From deecdb9acf0ac29469fa59c413c726870d613013 Mon Sep 17 00:00:00 2001 From: aahel Date: Fri, 18 Aug 2023 10:49:28 +0530 Subject: [PATCH 1/9] added check if anonymous token policy exists --- .../server-acl-init/anonymous_token.go | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/control-plane/subcommand/server-acl-init/anonymous_token.go b/control-plane/subcommand/server-acl-init/anonymous_token.go index 32c19ec208..b721c1b6fe 100644 --- a/control-plane/subcommand/server-acl-init/anonymous_token.go +++ b/control-plane/subcommand/server-acl-init/anonymous_token.go @@ -10,6 +10,13 @@ import ( // configureAnonymousPolicy sets up policies and tokens so that Consul DNS and // cross-datacenter Consul connect calls will work. func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { + exists, err := checkIfAnonymousTokenPolicyExists(consulClient) + if err != nil { + return err + } + if exists { + return nil + } anonRules, err := c.anonymousTokenRules() if err != nil { c.log.Error("Error templating anonymous token rules", "err", err) @@ -44,3 +51,26 @@ func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { return err }) } + +func checkIfAnonymousTokenPolicyExists(consulClient *api.Client) (bool, error) { + token, _, err := consulClient.ACL().TokenRead("00000000-0000-0000-0000-000000000002", nil) + if err != nil { + return false, err + } + existingPolicies, _, err := consulClient.ACL().PolicyList(&api.QueryOptions{}) + if err != nil { + return false, err + } + policyID := "" + for _, existingPolicy := range existingPolicies { + if existingPolicy.Name == "anonymous-token-policy" && existingPolicy.Description == "Anonymous token Policy" { + policyID = existingPolicy.ID + } + } + for _, policy := range token.Policies { + if policy.ID == policyID { + return true, nil + } + } + return false, nil +} From 8d8960145cdd4d94a5a41e3d6b4bd4ceb1a17051 Mon Sep 17 00:00:00 2001 From: aahel Date: Fri, 18 Aug 2023 11:24:39 +0530 Subject: [PATCH 2/9] changed checkIfAnonymousTokenPolicyExists impl --- .../server-acl-init/anonymous_token.go | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/control-plane/subcommand/server-acl-init/anonymous_token.go b/control-plane/subcommand/server-acl-init/anonymous_token.go index b721c1b6fe..5c57713625 100644 --- a/control-plane/subcommand/server-acl-init/anonymous_token.go +++ b/control-plane/subcommand/server-acl-init/anonymous_token.go @@ -7,6 +7,11 @@ import ( "github.com/hashicorp/consul/api" ) +const ( + AnonymousTokenPolicyName = "anonymous-token-policy" + AnonymousTokenAccessorID = "00000000-0000-0000-0000-000000000002" +) + // configureAnonymousPolicy sets up policies and tokens so that Consul DNS and // cross-datacenter Consul connect calls will work. func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { @@ -17,6 +22,7 @@ func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { if exists { return nil } + anonRules, err := c.anonymousTokenRules() if err != nil { c.log.Error("Error templating anonymous token rules", "err", err) @@ -25,7 +31,7 @@ func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { // Create policy for the anonymous token anonPolicy := api.ACLPolicy{ - Name: "anonymous-token-policy", + Name: AnonymousTokenPolicyName, Description: "Anonymous token Policy", Rules: anonRules, } @@ -40,7 +46,7 @@ func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { // Create token to get sent to TokenUpdate aToken := api.ACLToken{ - AccessorID: "00000000-0000-0000-0000-000000000002", + AccessorID: AnonymousTokenAccessorID, Policies: []*api.ACLTokenPolicyLink{{Name: anonPolicy.Name}}, } @@ -53,24 +59,16 @@ func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { } func checkIfAnonymousTokenPolicyExists(consulClient *api.Client) (bool, error) { - token, _, err := consulClient.ACL().TokenRead("00000000-0000-0000-0000-000000000002", nil) - if err != nil { - return false, err - } - existingPolicies, _, err := consulClient.ACL().PolicyList(&api.QueryOptions{}) + token, _, err := consulClient.ACL().TokenRead(AnonymousTokenAccessorID, nil) if err != nil { return false, err } - policyID := "" - for _, existingPolicy := range existingPolicies { - if existingPolicy.Name == "anonymous-token-policy" && existingPolicy.Description == "Anonymous token Policy" { - policyID = existingPolicy.ID - } - } + for _, policy := range token.Policies { - if policy.ID == policyID { + if policy.Name == AnonymousTokenPolicyName { return true, nil } } + return false, nil } From 10946edd493c4f019ea6ec962998c358b59ba275 Mon Sep 17 00:00:00 2001 From: aahel Date: Mon, 21 Aug 2023 10:44:26 +0530 Subject: [PATCH 3/9] made consts private --- .../subcommand/server-acl-init/anonymous_token.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/control-plane/subcommand/server-acl-init/anonymous_token.go b/control-plane/subcommand/server-acl-init/anonymous_token.go index 5c57713625..2a15d74777 100644 --- a/control-plane/subcommand/server-acl-init/anonymous_token.go +++ b/control-plane/subcommand/server-acl-init/anonymous_token.go @@ -8,8 +8,8 @@ import ( ) const ( - AnonymousTokenPolicyName = "anonymous-token-policy" - AnonymousTokenAccessorID = "00000000-0000-0000-0000-000000000002" + anonymousTokenPolicyName = "anonymous-token-policy" + anonymousTokenAccessorID = "00000000-0000-0000-0000-000000000002" ) // configureAnonymousPolicy sets up policies and tokens so that Consul DNS and @@ -31,7 +31,7 @@ func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { // Create policy for the anonymous token anonPolicy := api.ACLPolicy{ - Name: AnonymousTokenPolicyName, + Name: anonymousTokenPolicyName, Description: "Anonymous token Policy", Rules: anonRules, } @@ -46,7 +46,7 @@ func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { // Create token to get sent to TokenUpdate aToken := api.ACLToken{ - AccessorID: AnonymousTokenAccessorID, + AccessorID: anonymousTokenAccessorID, Policies: []*api.ACLTokenPolicyLink{{Name: anonPolicy.Name}}, } @@ -59,13 +59,13 @@ func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { } func checkIfAnonymousTokenPolicyExists(consulClient *api.Client) (bool, error) { - token, _, err := consulClient.ACL().TokenRead(AnonymousTokenAccessorID, nil) + token, _, err := consulClient.ACL().TokenRead(anonymousTokenAccessorID, nil) if err != nil { return false, err } for _, policy := range token.Policies { - if policy.Name == AnonymousTokenPolicyName { + if policy.Name == anonymousTokenPolicyName { return true, nil } } From 18c2824f9f588a8108874a091f99bee13602ea7a Mon Sep 17 00:00:00 2001 From: aahel Date: Wed, 23 Aug 2023 09:39:49 +0530 Subject: [PATCH 4/9] added test for configureAnonymousPolicy --- .../server-acl-init/anonymous_token_test.go | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 control-plane/subcommand/server-acl-init/anonymous_token_test.go diff --git a/control-plane/subcommand/server-acl-init/anonymous_token_test.go b/control-plane/subcommand/server-acl-init/anonymous_token_test.go new file mode 100644 index 0000000000..a3298427c2 --- /dev/null +++ b/control-plane/subcommand/server-acl-init/anonymous_token_test.go @@ -0,0 +1,53 @@ +package serveraclinit + +import ( + "strings" + "testing" + + "github.com/hashicorp/consul/api" + "github.com/mitchellh/cli" + "github.com/stretchr/testify/require" +) + +func Test_configureAnonymousPolicy(t *testing.T) { + + k8s, testClient := completeSetup(t) + consulHTTPAddr := testClient.TestServer.HTTPAddr + consulGRPCAddr := testClient.TestServer.GRPCAddr + + setUpK8sServiceAccount(t, k8s, ns) + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + } + cmd.init() + flags := []string{"-connect-inject"} + cmdArgs := append([]string{ + "-timeout=1m", + "-resource-prefix=" + resourcePrefix, + "-k8s-namespace=" + ns, + "-auth-method-host=https://my-kube.com", + "-addresses", strings.Split(consulHTTPAddr, ":")[0], + "-http-port", strings.Split(consulHTTPAddr, ":")[1], + "-grpc-port", strings.Split(consulGRPCAddr, ":")[1], + }, flags...) + responseCode := cmd.Run(cmdArgs) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + + bootToken := getBootToken(t, k8s, resourcePrefix, ns) + consul, err := api.NewClient(&api.Config{ + Address: consulHTTPAddr, + Token: bootToken, + }) + require.NoError(t, err) + + // creates new anonymous token policy + errx := cmd.configureAnonymousPolicy(consul) + require.NoError(t, errx) + + // does not create/update anonymous token policy + erry := cmd.configureAnonymousPolicy(consul) + require.NoError(t, erry) + +} From 78d17e77f9eb2e1237617191d5774666630d084c Mon Sep 17 00:00:00 2001 From: aahel Date: Wed, 23 Aug 2023 13:54:52 +0530 Subject: [PATCH 5/9] fixed unit test --- .../server-acl-init/anonymous_token_test.go | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/control-plane/subcommand/server-acl-init/anonymous_token_test.go b/control-plane/subcommand/server-acl-init/anonymous_token_test.go index a3298427c2..3ed7ea91b7 100644 --- a/control-plane/subcommand/server-acl-init/anonymous_token_test.go +++ b/control-plane/subcommand/server-acl-init/anonymous_token_test.go @@ -45,9 +45,30 @@ func Test_configureAnonymousPolicy(t *testing.T) { // creates new anonymous token policy errx := cmd.configureAnonymousPolicy(consul) require.NoError(t, errx) + var readOnlyPolicy = `acl = "read"` + _, _, err = consul.ACL().PolicyCreate(&api.ACLPolicy{ + Name: "acl-read-policy", + Rules: readOnlyPolicy, + }, nil) + require.NoError(t, err) + + resp, _, err := consul.ACL().TokenCreate(&api.ACLToken{ + Policies: []*api.ACLTokenPolicyLink{ + { + Name: "acl-read-policy", + }, + }, + }, nil) + require.NoError(t, err) + readToken := resp.SecretID + + readOnlyClient, errz := api.NewClient(&api.Config{ + Address: consulHTTPAddr, + Token: readToken, + }) + require.NoError(t, errz) // does not create/update anonymous token policy - erry := cmd.configureAnonymousPolicy(consul) + erry := cmd.configureAnonymousPolicy(readOnlyClient) require.NoError(t, erry) - } From 525e56bb5e6337668bc377fe589a0969a7f3f678 Mon Sep 17 00:00:00 2001 From: aahel Date: Wed, 23 Aug 2023 21:19:23 +0530 Subject: [PATCH 6/9] fixed test and minor refactoring --- .../server-acl-init/anonymous_token.go | 2 + .../server-acl-init/anonymous_token_test.go | 42 ++++++++----------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/control-plane/subcommand/server-acl-init/anonymous_token.go b/control-plane/subcommand/server-acl-init/anonymous_token.go index 2a15d74777..2f7ad6f513 100644 --- a/control-plane/subcommand/server-acl-init/anonymous_token.go +++ b/control-plane/subcommand/server-acl-init/anonymous_token.go @@ -17,9 +17,11 @@ const ( func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { exists, err := checkIfAnonymousTokenPolicyExists(consulClient) if err != nil { + c.log.Error("Error checking if anonymous token policy exists", "err", err) return err } if exists { + c.log.Info("skipping creating anonymous token since it already exists") return nil } diff --git a/control-plane/subcommand/server-acl-init/anonymous_token_test.go b/control-plane/subcommand/server-acl-init/anonymous_token_test.go index 3ed7ea91b7..d6e5a9a7fe 100644 --- a/control-plane/subcommand/server-acl-init/anonymous_token_test.go +++ b/control-plane/subcommand/server-acl-init/anonymous_token_test.go @@ -42,33 +42,27 @@ func Test_configureAnonymousPolicy(t *testing.T) { }) require.NoError(t, err) - // creates new anonymous token policy - errx := cmd.configureAnonymousPolicy(consul) - require.NoError(t, errx) - var readOnlyPolicy = `acl = "read"` + err = cmd.configureAnonymousPolicy(consul) + require.NoError(t, err) - _, _, err = consul.ACL().PolicyCreate(&api.ACLPolicy{ - Name: "acl-read-policy", - Rules: readOnlyPolicy, - }, nil) + policy, _, err := consul.ACL().PolicyReadByName(anonymousTokenPolicyName, nil) require.NoError(t, err) - resp, _, err := consul.ACL().TokenCreate(&api.ACLToken{ - Policies: []*api.ACLTokenPolicyLink{ - { - Name: "acl-read-policy", - }, - }, - }, nil) + testPolicy := api.ACLPolicy{ + ID: policy.ID, + Name: anonymousTokenPolicyName, + Description: "Anonymous token Policy", + Rules: `acl = "read"`, + } + updatedPolicy, _, err := consul.ACL().PolicyUpdate(&testPolicy, &api.WriteOptions{}) require.NoError(t, err) - readToken := resp.SecretID - readOnlyClient, errz := api.NewClient(&api.Config{ - Address: consulHTTPAddr, - Token: readToken, - }) - require.NoError(t, errz) - // does not create/update anonymous token policy - erry := cmd.configureAnonymousPolicy(readOnlyClient) - require.NoError(t, erry) + err = cmd.configureAnonymousPolicy(consul) + require.NoError(t, err) + + newPolicy, _, err := consul.ACL().PolicyReadByName(anonymousTokenPolicyName, nil) + require.NoError(t, err) + + // assert policy rule is still same. + require.Equal(t, updatedPolicy, newPolicy) } From 977050c2b4e7045f84c987e346457960bcd67d13 Mon Sep 17 00:00:00 2001 From: aahel Date: Wed, 23 Aug 2023 21:22:06 +0530 Subject: [PATCH 7/9] fix typo --- .../subcommand/server-acl-init/anonymous_token_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/subcommand/server-acl-init/anonymous_token_test.go b/control-plane/subcommand/server-acl-init/anonymous_token_test.go index d6e5a9a7fe..693c962829 100644 --- a/control-plane/subcommand/server-acl-init/anonymous_token_test.go +++ b/control-plane/subcommand/server-acl-init/anonymous_token_test.go @@ -63,6 +63,6 @@ func Test_configureAnonymousPolicy(t *testing.T) { newPolicy, _, err := consul.ACL().PolicyReadByName(anonymousTokenPolicyName, nil) require.NoError(t, err) - // assert policy rule is still same. + // assert policy is still same. require.Equal(t, updatedPolicy, newPolicy) } From 0d4e107875c3234464106cadf511156cfc13b58f Mon Sep 17 00:00:00 2001 From: aahel Date: Wed, 23 Aug 2023 22:13:32 +0530 Subject: [PATCH 8/9] changed some var names --- .../subcommand/server-acl-init/anonymous_token_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/control-plane/subcommand/server-acl-init/anonymous_token_test.go b/control-plane/subcommand/server-acl-init/anonymous_token_test.go index 693c962829..4a36c676e1 100644 --- a/control-plane/subcommand/server-acl-init/anonymous_token_test.go +++ b/control-plane/subcommand/server-acl-init/anonymous_token_test.go @@ -54,15 +54,15 @@ func Test_configureAnonymousPolicy(t *testing.T) { Description: "Anonymous token Policy", Rules: `acl = "read"`, } - updatedPolicy, _, err := consul.ACL().PolicyUpdate(&testPolicy, &api.WriteOptions{}) + readOnlyPolicy, _, err := consul.ACL().PolicyUpdate(&testPolicy, &api.WriteOptions{}) require.NoError(t, err) err = cmd.configureAnonymousPolicy(consul) require.NoError(t, err) - newPolicy, _, err := consul.ACL().PolicyReadByName(anonymousTokenPolicyName, nil) + actualPolicy, _, err := consul.ACL().PolicyReadByName(anonymousTokenPolicyName, nil) require.NoError(t, err) // assert policy is still same. - require.Equal(t, updatedPolicy, newPolicy) + require.Equal(t, readOnlyPolicy, actualPolicy) } From 905e681739e4d29bee57dc89226c93dc84d222f0 Mon Sep 17 00:00:00 2001 From: aahel Date: Wed, 30 Aug 2023 10:08:11 +0530 Subject: [PATCH 9/9] added changelog --- .changelog/2790.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/2790.txt diff --git a/.changelog/2790.txt b/.changelog/2790.txt new file mode 100644 index 0000000000..c16b55f74d --- /dev/null +++ b/.changelog/2790.txt @@ -0,0 +1,3 @@ +```release-note:improvement +control-plane: prevent updation of anonymous-token-policy and anonymous-token if anonymous-token-policy is already attached to the anonymous-token +``` \ No newline at end of file