From add4ea97dce2cea1d95aee4711bf6048aeec963e Mon Sep 17 00:00:00 2001 From: James Rasell Date: Wed, 20 Jul 2022 10:06:23 +0200 Subject: [PATCH] acl: enable configuration and visualisation of token expiration for users (#13846) * api: add ACL token expiry params to HTTP API * cli: allow setting and displaying ACL token expiry --- api/acl.go | 44 ++++++++++---- api/acl_test.go | 101 ++++++++++++++++++++++--------- command/acl_bootstrap.go | 9 +++ command/acl_bootstrap_test.go | 2 + command/acl_token_create.go | 21 ++++++- command/acl_token_create_test.go | 29 ++++++--- command/acl_token_list.go | 13 +++- 7 files changed, 166 insertions(+), 53 deletions(-) diff --git a/api/acl.go b/api/acl.go index 4a289c666..07b51deb4 100644 --- a/api/acl.go +++ b/api/acl.go @@ -221,24 +221,42 @@ type ACLPolicy struct { // ACLToken represents a client token which is used to Authenticate type ACLToken struct { - AccessorID string - SecretID string - Name string - Type string - Policies []string - Global bool - CreateTime time.Time + AccessorID string + SecretID string + Name string + Type string + Policies []string + Global bool + CreateTime time.Time + + // ExpirationTime represents the point after which a token should be + // considered revoked and is eligible for destruction. The zero value of + // time.Time does not respect json omitempty directives, so we must use a + // pointer. + ExpirationTime *time.Time `json:",omitempty"` + + // ExpirationTTL is a convenience field for helping set ExpirationTime to a + // value of CreateTime+ExpirationTTL. This can only be set during token + // creation. This is a string version of a time.Duration like "2m". + ExpirationTTL time.Duration `json:",omitempty"` + CreateIndex uint64 ModifyIndex uint64 } type ACLTokenListStub struct { - AccessorID string - Name string - Type string - Policies []string - Global bool - CreateTime time.Time + AccessorID string + Name string + Type string + Policies []string + Global bool + CreateTime time.Time + + // ExpirationTime represents the point after which a token should be + // considered revoked and is eligible for destruction. A nil value + // indicates no expiration has been set on the token. + ExpirationTime *time.Time `json:"expiration_time,omitempty"` + CreateIndex uint64 ModifyIndex uint64 } diff --git a/api/acl_test.go b/api/acl_test.go index 0b7dbc025..7a0434b54 100644 --- a/api/acl_test.go +++ b/api/acl_test.go @@ -2,9 +2,11 @@ package api import ( "testing" + "time" "github.com/hashicorp/nomad/api/internal/testutil" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestACLPolicies_ListUpsert(t *testing.T) { @@ -118,15 +120,10 @@ func TestACLTokens_List(t *testing.T) { // Expect out bootstrap token result, qm, err := at.List(nil) - if err != nil { - t.Fatalf("err: %s", err) - } - if qm.LastIndex == 0 { - t.Fatalf("bad index: %d", qm.LastIndex) - } - if n := len(result); n != 1 { - t.Fatalf("expected 1 token, got: %d", n) - } + require.NoError(t, err) + require.NotEqual(t, 0, qm.LastIndex) + require.Len(t, result, 1) + require.Nil(t, result[0].ExpirationTime) } func TestACLTokens_CreateUpdate(t *testing.T) { @@ -156,31 +153,81 @@ func TestACLTokens_CreateUpdate(t *testing.T) { // Verify the change took hold assert.Equal(t, out.Name, out2.Name) + + // Try updating the token to include a TTL which is not allowed. + out2.ExpirationTTL = 10 * time.Minute + out3, _, err := at.Update(out2, nil) + require.Error(t, err) + require.Nil(t, out3) } func TestACLTokens_Info(t *testing.T) { testutil.Parallel(t) - c, s, _ := makeACLClient(t, nil, nil) - defer s.Stop() - at := c.ACLTokens() - token := &ACLToken{ - Name: "foo", - Type: "client", - Policies: []string{"foo1"}, + testClient, testServer, _ := makeACLClient(t, nil, nil) + defer testServer.Stop() + + testCases := []struct { + name string + testFn func(client *Client) + }{ + { + name: "token without expiry", + testFn: func(client *Client) { + + token := &ACLToken{ + Name: "foo", + Type: "client", + Policies: []string{"foo1"}, + } + + // Create the token + out, wm, err := client.ACLTokens().Create(token, nil) + require.Nil(t, err) + assertWriteMeta(t, wm) + require.NotNil(t, out) + + // Query the token + out2, qm, err := client.ACLTokens().Info(out.AccessorID, nil) + require.Nil(t, err) + assertQueryMeta(t, qm) + require.Equal(t, out, out2) + }, + }, + { + name: "token with expiry", + testFn: func(client *Client) { + + token := &ACLToken{ + Name: "token-with-expiry", + Type: "client", + Policies: []string{"foo1"}, + ExpirationTTL: 10 * time.Minute, + } + + // Create the token + out, wm, err := client.ACLTokens().Create(token, nil) + require.Nil(t, err) + assertWriteMeta(t, wm) + require.NotNil(t, out) + + // Query the token and ensure it matches what was returned + // during the creation as well as ensuring the expiration time + // is set. + out2, qm, err := client.ACLTokens().Info(out.AccessorID, nil) + require.Nil(t, err) + assertQueryMeta(t, qm) + require.Equal(t, out, out2) + require.NotNil(t, out2.ExpirationTime) + }, + }, } - // Create the token - out, wm, err := at.Create(token, nil) - assert.Nil(t, err) - assertWriteMeta(t, wm) - assert.NotNil(t, out) - - // Query the token - out2, qm, err := at.Info(out.AccessorID, nil) - assert.Nil(t, err) - assertQueryMeta(t, qm) - assert.Equal(t, out, out2) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.testFn(testClient) + }) + } } func TestACLTokens_Self(t *testing.T) { diff --git a/command/acl_bootstrap.go b/command/acl_bootstrap.go index f8970f938..f367f9cb9 100644 --- a/command/acl_bootstrap.go +++ b/command/acl_bootstrap.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "os" "strings" + "time" "github.com/hashicorp/nomad/api" "github.com/posener/complete" @@ -163,8 +164,16 @@ func formatKVACLToken(token *api.ACLToken) string { // Add the generic output output = append(output, fmt.Sprintf("Create Time|%v", token.CreateTime), + fmt.Sprintf("Expiry Time |%s", expiryTimeString(token.ExpirationTime)), fmt.Sprintf("Create Index|%d", token.CreateIndex), fmt.Sprintf("Modify Index|%d", token.ModifyIndex), ) return formatKV(output) } + +func expiryTimeString(t *time.Time) string { + if t == nil || t.IsZero() { + return "" + } + return t.String() +} diff --git a/command/acl_bootstrap_test.go b/command/acl_bootstrap_test.go index c91588b23..96f489e02 100644 --- a/command/acl_bootstrap_test.go +++ b/command/acl_bootstrap_test.go @@ -36,6 +36,7 @@ func TestACLBootstrapCommand(t *testing.T) { out := ui.OutputWriter.String() assert.Contains(out, "Secret ID") + require.Contains(t, out, "Expiry Time = ") } // If a bootstrap token has already been created, attempts to create more should @@ -116,6 +117,7 @@ func TestACLBootstrapCommand_WithOperatorFileBootstrapToken(t *testing.T) { out := ui.OutputWriter.String() assert.Contains(t, out, mockToken.SecretID) + require.Contains(t, out, "Expiry Time = ") } // Attempting to bootstrap the server with an invalid operator provided token in a file should diff --git a/command/acl_token_create.go b/command/acl_token_create.go index eadb66ee6..de1962849 100644 --- a/command/acl_token_create.go +++ b/command/acl_token_create.go @@ -3,6 +3,7 @@ package command import ( "fmt" "strings" + "time" "github.com/hashicorp/nomad/api" "github.com/posener/complete" @@ -36,6 +37,11 @@ Create Options: -policy="" Specifies a policy to associate with the token. Can be specified multiple times, but only with client type tokens. + + -ttl + Specifies the time-to-live of the created ACL token. This takes the form of + a time duration such as "5m" and "1h". By default, tokens will be created + without a TTL and therefore never expire. ` return strings.TrimSpace(helpText) } @@ -47,6 +53,7 @@ func (c *ACLTokenCreateCommand) AutocompleteFlags() complete.Flags { "type": complete.PredictAnything, "global": complete.PredictNothing, "policy": complete.PredictAnything, + "ttl": complete.PredictAnything, }) } @@ -61,7 +68,7 @@ func (c *ACLTokenCreateCommand) Synopsis() string { func (c *ACLTokenCreateCommand) Name() string { return "acl token create" } func (c *ACLTokenCreateCommand) Run(args []string) int { - var name, tokenType string + var name, tokenType, ttl string var global bool var policies []string flags := c.Meta.FlagSet(c.Name(), FlagSetClient) @@ -69,6 +76,7 @@ func (c *ACLTokenCreateCommand) Run(args []string) int { flags.StringVar(&name, "name", "", "") flags.StringVar(&tokenType, "type", "client", "") flags.BoolVar(&global, "global", false, "") + flags.StringVar(&ttl, "ttl", "", "") flags.Var((funcVar)(func(s string) error { policies = append(policies, s) return nil @@ -93,6 +101,17 @@ func (c *ACLTokenCreateCommand) Run(args []string) int { Global: global, } + // If the user set a TTL flag value, convert this to a time duration and + // add it to our token request object. + if ttl != "" { + ttlDuration, err := time.ParseDuration(ttl) + if err != nil { + c.Ui.Error(fmt.Sprintf("Failed to parse TTL as time duration: %s", err)) + return 1 + } + tk.ExpirationTTL = ttlDuration + } + // Get the HTTP client client, err := c.Meta.Client() if err != nil { diff --git a/command/acl_token_create_test.go b/command/acl_token_create_test.go index e24e4c507..8eb782686 100644 --- a/command/acl_token_create_test.go +++ b/command/acl_token_create_test.go @@ -1,18 +1,17 @@ package command import ( - "strings" "testing" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/command/agent" "github.com/mitchellh/cli" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestACLTokenCreateCommand(t *testing.T) { ci.Parallel(t) - assert := assert.New(t) + config := func(c *agent.Config) { c.ACL.Enabled = true } @@ -22,22 +21,32 @@ func TestACLTokenCreateCommand(t *testing.T) { // Bootstrap an initial ACL token token := srv.RootToken - assert.NotNil(token, "failed to bootstrap ACL token") + require.NotNil(t, token, "failed to bootstrap ACL token") ui := cli.NewMockUi() cmd := &ACLTokenCreateCommand{Meta: Meta{Ui: ui, flagAddress: url}} // Request to create a new token without providing a valid management token code := cmd.Run([]string{"-address=" + url, "-token=foo", "-policy=foo", "-type=client"}) - assert.Equal(1, code) + require.Equal(t, 1, code) - // Request to create a new token with a valid management token + // Request to create a new token with a valid management token that does + // not have an expiry set. code = cmd.Run([]string{"-address=" + url, "-token=" + token.SecretID, "-policy=foo", "-type=client"}) - assert.Equal(0, code) + require.Equal(t, 0, code) // Check the output out := ui.OutputWriter.String() - if !strings.Contains(out, "[foo]") { - t.Fatalf("bad: %v", out) - } + require.Contains(t, out, "[foo]") + require.Contains(t, out, "Expiry Time = ") + + ui.OutputWriter.Reset() + ui.ErrorWriter.Reset() + + // Create a new token that has an expiry TTL set and check the response. + code = cmd.Run([]string{"-address=" + url, "-token=" + token.SecretID, "-type=management", "-ttl=10m"}) + require.Equal(t, 0, code) + + out = ui.OutputWriter.String() + require.NotContains(t, out, "Expiry Time = ") } diff --git a/command/acl_token_list.go b/command/acl_token_list.go index 9e3abe4c5..5946302fd 100644 --- a/command/acl_token_list.go +++ b/command/acl_token_list.go @@ -3,6 +3,7 @@ package command import ( "fmt" "strings" + "time" "github.com/hashicorp/nomad/api" "github.com/posener/complete" @@ -108,9 +109,17 @@ func formatTokens(tokens []*api.ACLTokenListStub) string { } output := make([]string, 0, len(tokens)+1) - output = append(output, "Name|Type|Global|Accessor ID") + output = append(output, "Name|Type|Global|Accessor ID|Expired") for _, p := range tokens { - output = append(output, fmt.Sprintf("%s|%s|%t|%s", p.Name, p.Type, p.Global, p.AccessorID)) + expired := false + if p.ExpirationTime != nil && !p.ExpirationTime.IsZero() { + if p.ExpirationTime.Before(time.Now().UTC()) { + expired = true + } + } + + output = append(output, fmt.Sprintf( + "%s|%s|%t|%s|%v", p.Name, p.Type, p.Global, p.AccessorID, expired)) } return formatList(output)