Handle rules translation when coming from the JSON compat HCL (#5662)

We were not handling some object keys when they were strings instead of identifiers. Now both are handled.

Fixes #5493

acl/policy.go

@@ -4,11 +4,13 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
+	"strconv"
 
 	"github.com/hashicorp/consul/sentinel"
 	"github.com/hashicorp/hcl"
 	"github.com/hashicorp/hcl/hcl/ast"
 	hclprinter "github.com/hashicorp/hcl/hcl/printer"
+	"github.com/hashicorp/hcl/hcl/token"
 	"golang.org/x/crypto/blake2b"
 )
 
@@ -818,22 +820,36 @@ func TranslateLegacyRules(policyBytes []byte) ([]byte, error) {
 
 	rewritten := ast.Walk(parsed, func(node ast.Node) (ast.Node, bool) {
 		switch n := node.(type) {
-		case *ast.ObjectKey:
-			switch n.Token.Text {
+		case *ast.ObjectItem:
+			if len(n.Keys) < 1 {
+				return node, true
+			}
+
+			txt := n.Keys[0].Token.Text
+			if n.Keys[0].Token.Type == token.STRING {
+				txt, err = strconv.Unquote(txt)
+				if err != nil {
+					return node, true
+				}
+			}
+
+			switch txt {
+			case "policy":
+				n.Keys[0].Token.Text = "policy"
 			case "agent":
-				n.Token.Text = "agent_prefix"
+				n.Keys[0].Token.Text = "agent_prefix"
 			case "key":
-				n.Token.Text = "key_prefix"
+				n.Keys[0].Token.Text = "key_prefix"
 			case "node":
-				n.Token.Text = "node_prefix"
+				n.Keys[0].Token.Text = "node_prefix"
 			case "query":
-				n.Token.Text = "query_prefix"
+				n.Keys[0].Token.Text = "query_prefix"
 			case "service":
-				n.Token.Text = "service_prefix"
+				n.Keys[0].Token.Text = "service_prefix"
 			case "session":
-				n.Token.Text = "session_prefix"
+				n.Keys[0].Token.Text = "session_prefix"
 			case "event":
-				n.Token.Text = "event_prefix"
+				n.Keys[0].Token.Text = "event_prefix"
 			}
 		}
 

acl/policy_test.go

@@ -1407,6 +1407,114 @@ operator = "write"
 	require.Equal(t, strings.Trim(expected, "\n"), string(output))
 }
 
+func TestRulesTranslate_GH5493(t *testing.T) {
+	input := `
+{
+	"key": {
+		"": {
+			"policy": "read"
+		},
+		"key": {
+			"policy": "read"
+		},
+		"policy": {
+			"policy": "read"
+		},
+		"privatething1/": {
+			"policy": "deny"
+		},
+		"anapplication/private/": {
+			"policy": "deny"
+		},
+		"privatething2/": {
+			"policy": "deny"
+		}
+	},
+	"session": {
+		"": {
+			"policy": "write"
+		}
+	},
+	"node": {
+		"": {
+			"policy": "read"
+		}
+	},
+	"agent": {
+		"": {
+			"policy": "read"
+		}
+	},
+	"service": {
+		"": {
+			"policy": "read"
+		}
+	},
+	"event": {
+		"": {
+			"policy": "read"
+		}
+	},
+	"query": {
+		"": {
+			"policy": "read"
+		}
+	}
+}`
+	expected := `
+key_prefix "" {
+  policy = "read"
+}
+
+key_prefix "key" {
+  policy = "read"
+}
+
+key_prefix "policy" {
+  policy = "read"
+}
+
+key_prefix "privatething1/" {
+  policy = "deny"
+}
+
+key_prefix "anapplication/private/" {
+  policy = "deny"
+}
+
+key_prefix "privatething2/" {
+  policy = "deny"
+}
+
+session_prefix "" {
+  policy = "write"
+}
+
+node_prefix "" {
+  policy = "read"
+}
+
+agent_prefix "" {
+  policy = "read"
+}
+
+service_prefix "" {
+  policy = "read"
+}
+
+event_prefix "" {
+  policy = "read"
+}
+
+query_prefix "" {
+  policy = "read"
+}
+`
+	output, err := TranslateLegacyRules([]byte(input))
+	require.NoError(t, err)
+	require.Equal(t, strings.Trim(expected, "\n"), string(output))
+}
+
 func TestPrecedence(t *testing.T) {
 	type testCase struct {
 		name     string

command/acl/rules/translate.go

@@ -53,6 +53,11 @@ func (c *cmd) Run(args []string) int {
 		return 1
 	}
 
+	if c.tokenSecret && c.tokenAccessor {
+		c.UI.Error(fmt.Sprintf("Error - cannot specify both -token-secret and -token-accessor"))
+		return 1
+	}
+
 	data, err := c.dataFromArgs(c.flags.Args())
 	if err != nil {
 		c.UI.Error(fmt.Sprintf("Error! %v", err))
@@ -69,6 +74,8 @@ func (c *cmd) Run(args []string) int {
 		// Trim whitespace and newlines (e.g. from echo without -n)
 		data = strings.TrimSpace(data)
 
+		// It is not a bug that this doesn't look at tokenAccessor. We already know that we want the rules from
+		// a token and just need to tell the helper function whether it should be retrieved by its secret or accessor
 		if rules, err := aclhelpers.GetRulesFromLegacyToken(client, data, c.tokenSecret); err != nil {
 			c.UI.Error(err.Error())
 			return 1

command/acl/rules/translate_test.go

@@ -9,10 +9,10 @@ import (
 
 	"github.com/hashicorp/consul/agent"
 	"github.com/hashicorp/consul/logger"
-	"github.com/hashicorp/consul/testrpc"
 	"github.com/hashicorp/consul/sdk/testutil"
+	"github.com/hashicorp/consul/testrpc"
 	"github.com/mitchellh/cli"
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestRulesTranslateCommand_noTabs(t *testing.T) {
@@ -25,7 +25,6 @@ func TestRulesTranslateCommand_noTabs(t *testing.T) {
 
 func TestRulesTranslateCommand(t *testing.T) {
 	t.Parallel()
-	assert := assert.New(t)
 
 	testDir := testutil.TempDir(t, "acl")
 	defer os.RemoveAll(testDir)
@@ -53,9 +52,9 @@ func TestRulesTranslateCommand(t *testing.T) {
 	expected := "service_prefix \"\" {\n  policy = \"write\"\n}"
 
 	// From a file
-	{
+	t.Run("file", func(t *testing.T) {
 		err := ioutil.WriteFile(testDir+"/rules.hcl", []byte(rules), 0644)
-		assert.NoError(err)
+		require.NoError(t, err)
 
 		args := []string{
 			"-http-addr=" + a.HTTPAddr(),
@@ -64,13 +63,13 @@ func TestRulesTranslateCommand(t *testing.T) {
 		}
 
 		code := cmd.Run(args)
-		assert.Equal(code, 0)
-		assert.Empty(ui.ErrorWriter.String())
-		assert.Contains(ui.OutputWriter.String(), expected)
-	}
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+		require.Contains(t, ui.OutputWriter.String(), expected)
+	})
 
 	// From stdin
-	{
+	t.Run("stdin", func(t *testing.T) {
 		go func() {
 			stdinW.Write([]byte(rules))
 			stdinW.Close()
@@ -83,13 +82,13 @@ func TestRulesTranslateCommand(t *testing.T) {
 		}
 
 		code := cmd.Run(args)
-		assert.Equal(code, 0)
-		assert.Empty(ui.ErrorWriter.String())
-		assert.Contains(ui.OutputWriter.String(), expected)
-	}
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+		require.Contains(t, ui.OutputWriter.String(), expected)
+	})
 
 	// From arg
-	{
+	t.Run("arg", func(t *testing.T) {
 		args := []string{
 			"-http-addr=" + a.HTTPAddr(),
 			"-token=root",
@@ -97,8 +96,23 @@ func TestRulesTranslateCommand(t *testing.T) {
 		}
 
 		code := cmd.Run(args)
-		assert.Equal(code, 0)
-		assert.Empty(ui.ErrorWriter.String())
-		assert.Contains(ui.OutputWriter.String(), expected)
-	}
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+		require.Contains(t, ui.OutputWriter.String(), expected)
+	})
+
+	// cannot specify both secret and accessor
+	t.Run("exclusive-options", func(t *testing.T) {
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-token-secret",
+			"-token-accessor",
+			`token "" { policy = "write" }`,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, 1, code, 0)
+		require.Equal(t, "Error - cannot specify both -token-secret and -token-accessor\n", ui.ErrorWriter.String())
+	})
 }