Allow ACL legacy migration via CLI (#4882)

* Adds a flag to `consul acl token update` that allows legacy ACLs to be upgraded via the CLI.

Also fixes a bug where descriptions are deleted if not specified.

* Remove debug

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
 `

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 {

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)
+	}
 }