From e4e4dc18e6bb05f642d3fa783c074f831a62fdc7 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Wed, 1 Feb 2023 17:43:41 +0100 Subject: [PATCH] acl: fix a bug in token creation when parsing expiration TTLs. (#15999) 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. --- .changelog/15999.txt | 3 ++ api/acl.go | 47 +++++++++++++++++++++++ command/agent/acl_endpoint_test.go | 43 +++++++++++++++++++++ nomad/structs/acl.go | 51 +++++++++++++++++++++++++ website/content/api-docs/acl/tokens.mdx | 6 ++- 5 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 .changelog/15999.txt 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