diff --git a/command/acl/rules/translate.go b/command/acl/rules/translate.go index 71b59040f4..a44091218c 100644 --- a/command/acl/rules/translate.go +++ b/command/acl/rules/translate.go @@ -135,5 +135,5 @@ Usage: consul acl translate-rules [options] TRANSLATE Translate rules for a legacy ACL token using its AccessorID: - $ consul acl translate-rules 429cd746-03d5-4bbb-a83a-18b164171c89 + $ consul acl translate-rules -token-accessor 429cd746-03d5-4bbb-a83a-18b164171c89 ` diff --git a/command/acl/token/update/token_update.go b/command/acl/token/update/token_update.go index 4a97855027..09df170b5a 100644 --- a/command/acl/token/update/token_update.go +++ b/command/acl/token/update/token_update.go @@ -28,6 +28,7 @@ type cmd struct { description string mergePolicies bool showMeta bool + upgradeLegacy bool } func (c *cmd) init() { @@ -44,6 +45,12 @@ func (c *cmd) init() { "policy to use for this token. May be specified multiple times") c.flags.Var((*flags.AppendSliceValue)(&c.policyNames), "policy-name", "Name of a "+ "policy to use for this token. May be specified multiple times") + c.flags.BoolVar(&c.upgradeLegacy, "upgrade-legacy", false, "Add new polices "+ + "to a legacy token replacing all existing rules. This will cause the legacy "+ + "token to behave exactly like a new token but keep the same Secret.\n"+ + "WARNING: you must ensure that the new policy or policies specified grant "+ + "equivalent or appropriate access for the existing clients using this token.") + c.http = &flags.HTTPFlags{} flags.Merge(c.flags, c.http.ClientFlags()) flags.Merge(c.flags, c.http.ServerFlags()) @@ -78,7 +85,27 @@ func (c *cmd) Run(args []string) int { return 1 } - token.Description = c.description + if c.upgradeLegacy { + if token.Rules == "" { + // This is just for convenience it should actually be harmless to allow it + // to go through anyway. + c.UI.Error(fmt.Sprintf("Can't use -upgrade-legacy on a non-legacy token")) + return 1 + } + // Reset the rules to nothing forcing this to be updated as a non-legacy + // token but with same secret. + token.Rules = "" + } + + if c.description != "" { + // Only update description if the user specified a new one. This does make + // it impossible to completely clear descriptions from CLI but that seems + // better than silently deleting descriptions when using command without + // manually giving the new description. If it's a real issue we can always + // add another explicit `-remove-description` flag but it feels like an edge + // case that's not going to be critical to anyone. + token.Description = c.description + } if c.mergePolicies { for _, policyName := range c.policyNames { diff --git a/command/acl/token/update/token_update_test.go b/command/acl/token/update/token_update_test.go index 3fd04c2bdf..b48603e5f6 100644 --- a/command/acl/token/update/token_update_test.go +++ b/command/acl/token/update/token_update_test.go @@ -5,11 +5,14 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/agent" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/logger" "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/consul/testutil" + "github.com/hashicorp/consul/testutil/retry" "github.com/mitchellh/cli" "github.com/stretchr/testify/assert" ) @@ -25,6 +28,8 @@ func TestTokenUpdateCommand_noTabs(t *testing.T) { func TestTokenUpdateCommand(t *testing.T) { t.Parallel() assert := assert.New(t) + // Alias because we need to access require package in Retry below + req := require.New(t) testDir := testutil.TempDir(t, "acl") defer os.RemoveAll(testDir) @@ -44,7 +49,6 @@ func TestTokenUpdateCommand(t *testing.T) { testrpc.WaitForLeader(t, a.RPC, "dc1") ui := cli.NewMockUi() - cmd := New(ui) // Create a policy client := a.Client() @@ -53,17 +57,31 @@ func TestTokenUpdateCommand(t *testing.T) { &api.ACLPolicy{Name: "test-policy"}, &api.WriteOptions{Token: "root"}, ) - assert.NoError(err) + req.NoError(err) // create a token token, _, err := client.ACL().TokenCreate( &api.ACLToken{Description: "test"}, &api.WriteOptions{Token: "root"}, ) - assert.NoError(err) + req.NoError(err) + + // create a legacy token + legacyTokenSecretID, _, err := client.ACL().Create(&api.ACLEntry{ + Name: "Legacy token", + Type: "client", + Rules: "service \"test\" { policy = \"write\" }", + }, + &api.WriteOptions{Token: "root"}, + ) + req.NoError(err) + + // We fetch the legacy token later to give server time to async background + // upgrade it. // update with policy by name { + cmd := New(ui) args := []string{ "-http-addr=" + a.HTTPAddr(), "-id=" + token.AccessorID, @@ -86,6 +104,7 @@ func TestTokenUpdateCommand(t *testing.T) { // update with policy by id { + cmd := New(ui) args := []string{ "-http-addr=" + a.HTTPAddr(), "-id=" + token.AccessorID, @@ -105,4 +124,68 @@ func TestTokenUpdateCommand(t *testing.T) { assert.NoError(err) assert.NotNil(token) } + + // update with no description shouldn't delete the current description + { + cmd := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-id=" + token.AccessorID, + "-token=root", + "-policy-name=" + policy.Name, + } + + code := cmd.Run(args) + assert.Equal(code, 0) + assert.Empty(ui.ErrorWriter.String()) + + token, _, err := client.ACL().TokenRead( + token.AccessorID, + &api.QueryOptions{Token: "root"}, + ) + assert.NoError(err) + assert.NotNil(token) + assert.Equal("test token", token.Description) + } + + // Need legacy token now, hopefully server had time to generate an accessor ID + // in the background but wait for it if not. + var legacyToken *api.ACLToken + retry.Run(t, func(r *retry.R) { + // Fetch the legacy token via new API so we can use it's accessor ID + legacyToken, _, err = client.ACL().TokenReadSelf( + &api.QueryOptions{Token: legacyTokenSecretID}) + r.Check(err) + require.NotEmpty(r, legacyToken.AccessorID) + }) + + // upgrade legacy token should replace rules and leave token in a "new" state! + { + cmd := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-id=" + legacyToken.AccessorID, + "-token=root", + "-policy-name=" + policy.Name, + "-upgrade-legacy", + } + + code := cmd.Run(args) + assert.Equal(code, 0) + assert.Empty(ui.ErrorWriter.String()) + + gotToken, _, err := client.ACL().TokenRead( + legacyToken.AccessorID, + &api.QueryOptions{Token: "root"}, + ) + assert.NoError(err) + assert.NotNil(gotToken) + // Description shouldn't change + assert.Equal("Legacy token", gotToken.Description) + assert.Len(gotToken.Policies, 1) + // Rules should now be empty meaning this is no longer a legacy token + assert.Empty(gotToken.Rules) + // Secret should not have changes + assert.Equal(legacyToken.SecretID, gotToken.SecretID) + } }