Skip to content

Commit

Permalink
acl: fix a bug in token creation when parsing expiration TTLs. (#15999)
Browse files Browse the repository at this point in the history
The ACL token decoding was not correctly handling time duration
syntax such as "1h" which forced people to use the nanosecond
representation via the HTTP API.

The change adds an unmarshal function which allows this syntax to
be used, along with other styles correctly.
  • Loading branch information
jrasell committed Feb 1, 2023
1 parent 0052596 commit e4e4dc1
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/15999.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
acl: Fixed a bug in token creation which failed to parse expiration TTLs correctly
```
47 changes: 47 additions & 0 deletions api/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,53 @@ type ACLTokenRoleLink struct {
Name string
}

// MarshalJSON implements the json.Marshaler interface and allows
// ACLToken.ExpirationTTL to be marshaled correctly.
func (a *ACLToken) MarshalJSON() ([]byte, error) {
type Alias ACLToken
exported := &struct {
ExpirationTTL string
*Alias
}{
ExpirationTTL: a.ExpirationTTL.String(),
Alias: (*Alias)(a),
}
if a.ExpirationTTL == 0 {
exported.ExpirationTTL = ""
}
return json.Marshal(exported)
}

// UnmarshalJSON implements the json.Unmarshaler interface and allows
// ACLToken.ExpirationTTL to be unmarshalled correctly.
func (a *ACLToken) UnmarshalJSON(data []byte) (err error) {
type Alias ACLToken
aux := &struct {
ExpirationTTL any
*Alias
}{
Alias: (*Alias)(a),
}

if err = json.Unmarshal(data, &aux); err != nil {
return err
}
if aux.ExpirationTTL != nil {
switch v := aux.ExpirationTTL.(type) {
case string:
if v != "" {
if a.ExpirationTTL, err = time.ParseDuration(v); err != nil {
return err
}
}
case float64:
a.ExpirationTTL = time.Duration(v)
}

}
return nil
}

type ACLTokenListStub struct {
AccessorID string
Name string
Expand Down
43 changes: 43 additions & 0 deletions command/agent/acl_endpoint_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package agent

import (
"bytes"
"fmt"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -471,6 +472,48 @@ func TestHTTP_ACLTokenCreate(t *testing.T) {
})
}

func TestHTTP_ACLTokenCreateExpirationTTL(t *testing.T) {
ci.Parallel(t)
httpACLTest(t, nil, func(s *TestAgent) {

// Generate an example token which has an expiration TTL in string
// format.
aclToken := `
{
"Name": "Readonly token",
"Type": "client",
"Policies": ["readonly"],
"ExpirationTTL": "10h",
"Global": false
}`

req, err := http.NewRequest("PUT", "/v1/acl/token", bytes.NewReader([]byte(aclToken)))
must.NoError(t, err)

respW := httptest.NewRecorder()
setToken(req, s.RootToken)

// Make the request.
obj, err := s.Server.ACLTokenSpecificRequest(respW, req)
must.NoError(t, err)
must.NotNil(t, obj)

// Ensure the returned token includes expiration.
createdTokenResp := obj.(*structs.ACLToken)
must.Eq(t, "10h0m0s", createdTokenResp.ExpirationTTL.String())
must.False(t, createdTokenResp.CreateTime.IsZero())

// Check for the index.
must.StrNotEqFold(t, "", respW.Result().Header.Get("X-Nomad-Index"))

// Check token was created and stored properly within state.
out, err := s.Agent.server.State().ACLTokenByAccessorID(nil, createdTokenResp.AccessorID)
must.NoError(t, err)
must.NotNil(t, out)
must.Eq(t, createdTokenResp, out)
})
}

func TestHTTP_ACLTokenDelete(t *testing.T) {
ci.Parallel(t)
httpACLTest(t, nil, func(s *TestAgent) {
Expand Down
51 changes: 51 additions & 0 deletions nomad/structs/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,57 @@ func (a *ACLToken) HasRoles(roleIDs []string) bool {
return true
}

// MarshalJSON implements the json.Marshaler interface and allows
// ACLToken.ExpirationTTL to be marshaled correctly.
func (a *ACLToken) MarshalJSON() ([]byte, error) {
type Alias ACLToken
exported := &struct {
ExpirationTTL string
*Alias
}{
ExpirationTTL: a.ExpirationTTL.String(),
Alias: (*Alias)(a),
}
if a.ExpirationTTL == 0 {
exported.ExpirationTTL = ""
}
return json.Marshal(exported)
}

// UnmarshalJSON implements the json.Unmarshaler interface and allows
// ACLToken.ExpirationTTL to be unmarshalled correctly.
func (a *ACLToken) UnmarshalJSON(data []byte) (err error) {
type Alias ACLToken
aux := &struct {
ExpirationTTL interface{}
Hash string
*Alias
}{
Alias: (*Alias)(a),
}

if err = json.Unmarshal(data, &aux); err != nil {
return err
}
if aux.ExpirationTTL != nil {
switch v := aux.ExpirationTTL.(type) {
case string:
if v != "" {
if a.ExpirationTTL, err = time.ParseDuration(v); err != nil {
return err
}
}
case float64:
a.ExpirationTTL = time.Duration(v)
}

}
if aux.Hash != "" {
a.Hash = []byte(aux.Hash)
}
return nil
}

// ACLRole is an abstraction for the ACL system which allows the grouping of
// ACL policies into a single object. ACL tokens can be created and linked to
// a role; the token then inherits all the permissions granted by the policies.
Expand Down
6 changes: 5 additions & 1 deletion website/content/api-docs/acl/tokens.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ The table below shows this endpoint's support for

- `ExpirationTTL` `(duration: 0s)` - This is a convenience field and if set will
initialize the `ExpirationTime` field to a value of `CreateTime` +
`ExpirationTTL`.
`ExpirationTTL`. This value must be between the [`token_min_expiration_ttl`][]
and [`token_max_expiration_ttl`][] ACL configuration parameters.

### Sample Payload

Expand Down Expand Up @@ -499,3 +500,6 @@ $ curl \
}
}
```

[`token_min_expiration_ttl`]: /nomad/docs/configuration/acl#token_min_expiration_ttl
[`token_max_expiration_ttl`]: /nomad/docs/configuration/acl#token_max_expiration_ttl

0 comments on commit e4e4dc1

Please sign in to comment.