diff --git a/.changelog/15999.txt b/.changelog/15999.txt new file mode 100644 index 000000000..872cd0617 --- /dev/null +++ b/.changelog/15999.txt @@ -0,0 +1,3 @@ +```release-note:bug +acl: Fixed a bug in token creation which failed to parse expiration TTLs correctly +``` diff --git a/api/acl.go b/api/acl.go index b8f32441d..1544d1a3c 100644 --- a/api/acl.go +++ b/api/acl.go @@ -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 diff --git a/command/agent/acl_endpoint_test.go b/command/agent/acl_endpoint_test.go index 391751150..cfc200445 100644 --- a/command/agent/acl_endpoint_test.go +++ b/command/agent/acl_endpoint_test.go @@ -1,6 +1,7 @@ package agent import ( + "bytes" "fmt" "net/http" "net/http/httptest" @@ -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) { diff --git a/nomad/structs/acl.go b/nomad/structs/acl.go index df184358c..158fa81eb 100644 --- a/nomad/structs/acl.go +++ b/nomad/structs/acl.go @@ -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. diff --git a/website/content/api-docs/acl/tokens.mdx b/website/content/api-docs/acl/tokens.mdx index 2dc2b4596..6b4d014a0 100644 --- a/website/content/api-docs/acl/tokens.mdx +++ b/website/content/api-docs/acl/tokens.mdx @@ -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 @@ -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