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.
This commit is contained in:
James Rasell 2023-02-01 17:43:41 +01:00 committed by GitHub
parent 67acfd9f6b
commit 9e8325d63c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 149 additions and 1 deletions

3
.changelog/15999.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
acl: Fixed a bug in token creation which failed to parse expiration TTLs correctly
```

View File

@ -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

View File

@ -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) {

View File

@ -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.

View File

@ -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