Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

acl: fix a bug in token creation when parsing expiration TTLs. #15999

Merged
merged 4 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -456,6 +457,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
jrasell marked this conversation as resolved.
Show resolved Hide resolved
// 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