From 121431bf1760d6350d2bcc5286a62f503a28a9cd Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 29 Sep 2021 17:43:36 -0400 Subject: [PATCH 1/9] acl: remove tests for resolving legacy tokens The code for this was already removed, which suggests this is not actually testing what it claims. I'm guessing these are still resolving because the tokens are converted to non-legacy tokens? --- agent/consul/acl_test.go | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index c1f4816b47..1fb9a0200a 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -2112,38 +2112,6 @@ func testACLResolver_variousTokens(t *testing.T, delegate *ACLResolverTestDelega require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil)) }) - runTwiceAndReset("legacy-management", func(t *testing.T) { - delegate.UseTestLocalData([]interface{}{ - &structs.ACLToken{ - AccessorID: "d109a033-99d1-47e2-a711-d6593373a973", - SecretID: "legacy-management", - Type: structs.ACLTokenTypeManagement, - }, - }) - authz, err := r.ResolveToken("legacy-management") - require.NotNil(t, authz) - require.NoError(t, err) - require.Equal(t, acl.Allow, authz.ACLWrite(nil)) - require.Equal(t, acl.Allow, authz.KeyRead("foo", nil)) - }) - - runTwiceAndReset("legacy-client", func(t *testing.T) { - delegate.UseTestLocalData([]interface{}{ - &structs.ACLToken{ - AccessorID: "b7375838-b104-4a25-b457-329d939bf257", - SecretID: "legacy-client", - Type: structs.ACLTokenTypeClient, - Rules: `service "" { policy = "read" }`, - }, - }) - authz, err := r.ResolveToken("legacy-client") - require.NoError(t, err) - require.NotNil(t, authz) - require.Equal(t, acl.Deny, authz.MeshRead(nil)) - require.Equal(t, acl.Deny, authz.OperatorRead(nil)) - require.Equal(t, acl.Allow, authz.ServiceRead("foo", nil)) - }) - runTwiceAndReset("service and intention wildcard write", func(t *testing.T) { delegate.UseTestLocalData([]interface{}{ &structs.ACLToken{ From c77e5747b1d0c2da050630931311050d2865a29c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 29 Sep 2021 17:54:54 -0400 Subject: [PATCH 2/9] acl: remove EmbeddedPolicy This method is no longer. It only existed for legacy tokens, which are no longer supported. --- agent/consul/acl.go | 9 ------- agent/structs/acl.go | 35 --------------------------- agent/structs/acl_test.go | 50 --------------------------------------- 3 files changed, 94 deletions(-) diff --git a/agent/consul/acl.go b/agent/consul/acl.go index a5c4010f12..cc2c4375ab 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -100,10 +100,6 @@ func (id *missingIdentity) RoleIDs() []string { return nil } -func (id *missingIdentity) EmbeddedPolicy() *structs.ACLPolicy { - return nil -} - func (id *missingIdentity) ServiceIdentityList() []*structs.ACLServiceIdentity { return nil } @@ -616,11 +612,6 @@ func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) ( ) if len(policyIDs) == 0 && len(serviceIdentities) == 0 && len(roleIDs) == 0 && len(nodeIdentities) == 0 { - policy := identity.EmbeddedPolicy() - if policy != nil { - return []*structs.ACLPolicy{policy}, nil - } - // In this case the default policy will be all that is in effect. return nil, nil } diff --git a/agent/structs/acl.go b/agent/structs/acl.go index f4b944dafa..15edc47f26 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -95,7 +95,6 @@ type ACLIdentity interface { SecretToken() string PolicyIDs() []string RoleIDs() []string - EmbeddedPolicy() *ACLPolicy ServiceIdentityList() []*ACLServiceIdentity NodeIdentityList() []*ACLNodeIdentity IsExpired(asOf time.Time) bool @@ -425,36 +424,6 @@ func (t *ACLToken) UsesNonLegacyFields() bool { t.AuthMethod != "" } -func (t *ACLToken) EmbeddedPolicy() *ACLPolicy { - // DEPRECATED (ACL-Legacy-Compat) - // - // For legacy tokens with embedded rules this provides a way to map those - // rules to an ACLPolicy. This function can just return nil once legacy - // acl compatibility is no longer needed. - // - // Additionally for management tokens we must embed the policy rules - // as well - policy := &ACLPolicy{} - if t.Type == ACLTokenTypeManagement { - hasher := fnv.New128a() - policy.ID = fmt.Sprintf("%x", hasher.Sum([]byte(ACLPolicyGlobalManagement))) - policy.Name = "legacy-management" - policy.Rules = ACLPolicyGlobalManagement - policy.Syntax = acl.SyntaxCurrent - } else if t.Rules != "" || t.Type == ACLTokenTypeClient { - hasher := fnv.New128a() - policy.ID = fmt.Sprintf("%x", hasher.Sum([]byte(t.Rules))) - policy.Name = fmt.Sprintf("legacy-policy-%s", policy.ID) - policy.Rules = t.Rules - policy.Syntax = acl.SyntaxLegacy - } else { - return nil - } - - policy.SetHash(true) - return policy -} - func (t *ACLToken) EnterpriseMetadata() *EnterpriseMeta { return &t.EnterpriseMeta } @@ -1799,10 +1768,6 @@ func (id *AgentMasterTokenIdentity) RoleIDs() []string { return nil } -func (id *AgentMasterTokenIdentity) EmbeddedPolicy() *ACLPolicy { - return nil -} - func (id *AgentMasterTokenIdentity) ServiceIdentityList() []*ACLServiceIdentity { return nil } diff --git a/agent/structs/acl_test.go b/agent/structs/acl_test.go index 64f59661e6..ec52edce0b 100644 --- a/agent/structs/acl_test.go +++ b/agent/structs/acl_test.go @@ -44,56 +44,6 @@ func TestStructs_ACLToken_PolicyIDs(t *testing.T) { }) } -func TestStructs_ACLToken_EmbeddedPolicy(t *testing.T) { - - t.Run("No Rules", func(t *testing.T) { - - token := &ACLToken{} - require.Nil(t, token.EmbeddedPolicy()) - }) - - t.Run("Legacy Client", func(t *testing.T) { - - // None of the other fields should be considered - token := &ACLToken{ - Type: ACLTokenTypeClient, - Rules: `acl = "read"`, - } - - policy := token.EmbeddedPolicy() - require.NotNil(t, policy) - require.NotEqual(t, "", policy.ID) - require.True(t, strings.HasPrefix(policy.Name, "legacy-policy-")) - require.Equal(t, token.Rules, policy.Rules) - require.Equal(t, policy.Syntax, acl.SyntaxLegacy) - require.NotNil(t, policy.Hash) - require.NotEqual(t, []byte{}, policy.Hash) - }) - - t.Run("Same Policy for Tokens with same Rules", func(t *testing.T) { - - token1 := &ACLToken{ - AccessorID: "f55b260c-5e05-418e-ab19-d421d1ab4b52", - SecretID: "b2165bac-7006-459b-8a72-7f549f0f06d6", - Description: "token 1", - Type: ACLTokenTypeClient, - Rules: `acl = "read"`, - } - - token2 := &ACLToken{ - AccessorID: "09d1c059-961a-46bd-a2e4-76adebe35fa5", - SecretID: "65e98e67-9b29-470c-8ffa-7c5a23cc67c8", - Description: "token 2", - Type: ACLTokenTypeClient, - Rules: `acl = "read"`, - } - - policy1 := token1.EmbeddedPolicy() - policy2 := token2.EmbeddedPolicy() - require.Equal(t, policy1, policy2) - }) -} - func TestStructs_ACLServiceIdentity_SyntheticPolicy(t *testing.T) { cases := []struct { From aea4cc5a6d538fcc02f7960308b34a846ecf78a3 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 29 Sep 2021 18:30:37 -0400 Subject: [PATCH 3/9] acl: remove legacy arg to store.ACLTokenSet And remove the tests for legacy=true --- agent/consul/state/acl.go | 16 +- agent/consul/state/acl_test.go | 163 +++++-------------- agent/consul/state/store_integration_test.go | 6 +- agent/rpc/subscribe/subscribe_test.go | 2 +- 4 files changed, 47 insertions(+), 140 deletions(-) diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index 7fe8c089f9..986fbc7ee7 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -411,18 +411,10 @@ func fixupRolePolicyLinks(tx ReadTxn, original *structs.ACLRole) (*structs.ACLRo return role, nil } -// ACLTokenSet is used to insert an ACL rule into the state store. -// Deprecated (ACL-Legacy-Compat) -func (s *Store) ACLTokenSet(idx uint64, token *structs.ACLToken, legacy bool) error { - tx := s.db.WriteTxn(idx) - defer tx.Abort() - - // Call set on the ACL - if err := aclTokenSetTxn(tx, idx, token, ACLTokenSetOptions{Legacy: legacy}); err != nil { - return err - } - - return tx.Commit() +// ACLTokenSet is used in many tests to set a single ACL token. It is now a shim +// for calling ACLTokenBatchSet with default options. +func (s *Store) ACLTokenSet(idx uint64, token *structs.ACLToken) error { + return s.ACLTokenBatchSet(idx, structs.ACLTokens{token}, ACLTokenSetOptions{}) } type ACLTokenSetOptions struct { diff --git a/agent/consul/state/acl_test.go b/agent/consul/state/acl_test.go index a58da3e6c2..99717745b8 100644 --- a/agent/consul/state/acl_test.go +++ b/agent/consul/state/acl_test.go @@ -46,7 +46,7 @@ func setupAnonymous(t *testing.T, s *Store) { Description: "Anonymous Token", } token.SetHash(true) - require.NoError(t, s.ACLTokenSet(1, &token, false)) + require.NoError(t, s.ACLTokenSet(1, &token)) } func testACLStateStore(t *testing.T) *Store { @@ -238,103 +238,6 @@ func TestStateStore_ACLBootstrap(t *testing.T) { require.Len(t, tokens, 2) } -func TestStateStore_ACLToken_SetGet_Legacy(t *testing.T) { - t.Parallel() - t.Run("Legacy - Existing With Policies", func(t *testing.T) { - t.Parallel() - s := testACLTokensStateStore(t) - - token := &structs.ACLToken{ - AccessorID: "c8d0378c-566a-4535-8fc9-c883a8cc9849", - SecretID: "6d48ce91-2558-4098-bdab-8737e4e57d5f", - Policies: []structs.ACLTokenPolicyLink{ - { - ID: testPolicyID_A, - }, - }, - } - - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) - - // legacy flag is set so it should disallow setting this token - err := s.ACLTokenSet(3, token.Clone(), true) - require.Error(t, err) - }) - - t.Run("Legacy - Empty Type", func(t *testing.T) { - t.Parallel() - s := testACLTokensStateStore(t) - token := &structs.ACLToken{ - AccessorID: "271cd056-0038-4fd3-90e5-f97f50fb3ac8", - SecretID: "c0056225-5785-43b3-9b77-3954f06d6aee", - } - - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) - - // legacy flag is set so it should disallow setting this token - err := s.ACLTokenSet(3, token.Clone(), true) - require.Error(t, err) - }) - - t.Run("Legacy - New", func(t *testing.T) { - t.Parallel() - s := testACLTokensStateStore(t) - token := &structs.ACLToken{ - SecretID: "2989e271-6169-4f34-8fec-4618d70008fb", - Type: structs.ACLTokenTypeClient, - Rules: `service "" { policy = "read" }`, - } - - require.NoError(t, s.ACLTokenSet(2, token.Clone(), true)) - - idx, rtoken, err := s.ACLTokenGetBySecret(nil, token.SecretID, nil) - require.NoError(t, err) - require.Equal(t, uint64(2), idx) - require.NotNil(t, rtoken) - require.Equal(t, "", rtoken.AccessorID) - require.Equal(t, "2989e271-6169-4f34-8fec-4618d70008fb", rtoken.SecretID) - require.Equal(t, "", rtoken.Description) - require.Len(t, rtoken.Policies, 0) - require.Equal(t, structs.ACLTokenTypeClient, rtoken.Type) - require.Equal(t, uint64(2), rtoken.CreateIndex) - require.Equal(t, uint64(2), rtoken.ModifyIndex) - }) - - t.Run("Legacy - Update", func(t *testing.T) { - t.Parallel() - s := testACLTokensStateStore(t) - original := &structs.ACLToken{ - SecretID: "2989e271-6169-4f34-8fec-4618d70008fb", - Type: structs.ACLTokenTypeClient, - Rules: `service "" { policy = "read" }`, - } - - require.NoError(t, s.ACLTokenSet(2, original.Clone(), true)) - - updatedRules := `service "" { policy = "read" } service "foo" { policy = "deny"}` - update := &structs.ACLToken{ - SecretID: "2989e271-6169-4f34-8fec-4618d70008fb", - Type: structs.ACLTokenTypeClient, - Rules: updatedRules, - } - - require.NoError(t, s.ACLTokenSet(3, update.Clone(), true)) - - idx, rtoken, err := s.ACLTokenGetBySecret(nil, original.SecretID, nil) - require.NoError(t, err) - require.Equal(t, uint64(3), idx) - require.NotNil(t, rtoken) - require.Equal(t, "", rtoken.AccessorID) - require.Equal(t, "2989e271-6169-4f34-8fec-4618d70008fb", rtoken.SecretID) - require.Equal(t, "", rtoken.Description) - require.Len(t, rtoken.Policies, 0) - require.Equal(t, structs.ACLTokenTypeClient, rtoken.Type) - require.Equal(t, updatedRules, rtoken.Rules) - require.Equal(t, uint64(2), rtoken.CreateIndex) - require.Equal(t, uint64(3), rtoken.ModifyIndex) - }) -} - func TestStateStore_ACLToken_SetGet(t *testing.T) { t.Parallel() t.Run("Missing Secret", func(t *testing.T) { @@ -344,7 +247,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { AccessorID: "39171632-6f34-4411-827f-9416403687f4", } - err := s.ACLTokenSet(2, token.Clone(), false) + err := s.ACLTokenSet(2, token.Clone()) require.Error(t, err) require.Equal(t, ErrMissingACLTokenSecret, err) }) @@ -356,7 +259,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { SecretID: "39171632-6f34-4411-827f-9416403687f4", } - err := s.ACLTokenSet(2, token.Clone(), false) + err := s.ACLTokenSet(2, token.Clone()) require.Error(t, err) require.Equal(t, ErrMissingACLTokenAccessor, err) }) @@ -372,7 +275,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -389,7 +292,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -406,7 +309,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token.Clone(), false) + err := s.ACLTokenSet(2, token.Clone()) require.Error(t, err) }) @@ -423,7 +326,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -440,7 +343,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token.Clone(), false) + err := s.ACLTokenSet(2, token.Clone()) require.Error(t, err) }) @@ -457,7 +360,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -470,7 +373,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { AuthMethod: "test", } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -497,7 +400,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) idx, rtoken, err := s.ACLTokenGetByAccessor(nil, "daf37c07-d04d-4fd5-9678-a8206a57d61a", nil) require.NoError(t, err) @@ -532,7 +435,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) updated := &structs.ACLToken{ AccessorID: "daf37c07-d04d-4fd5-9678-a8206a57d61a", @@ -554,7 +457,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(3, updated.Clone(), false)) + require.NoError(t, s.ACLTokenSet(3, updated.Clone())) idx, rtoken, err := s.ACLTokenGetByAccessor(nil, "daf37c07-d04d-4fd5-9678-a8206a57d61a", nil) require.NoError(t, err) @@ -588,7 +491,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) idx, rtoken, err := s.ACLTokenGetByAccessor(nil, "daf37c07-d04d-4fd5-9678-a8206a57d61a", nil) require.NoError(t, err) @@ -873,30 +776,42 @@ func TestStateStore_ACLTokens_ListUpgradeable(t *testing.T) { t.Parallel() s := testACLTokensStateStore(t) - require.NoError(t, s.ACLTokenSet(2, &structs.ACLToken{ + aclTokenSetLegacy := func(idx uint64, token *structs.ACLToken) error { + tx := s.db.WriteTxn(idx) + defer tx.Abort() + + opts := ACLTokenSetOptions{Legacy: true} + if err := aclTokenSetTxn(tx, idx, token, opts); err != nil { + return err + } + + return tx.Commit() + } + + require.NoError(t, aclTokenSetLegacy(2, &structs.ACLToken{ SecretID: "34ec8eb3-095d-417a-a937-b439af7a8e8b", Type: structs.ACLTokenTypeManagement, - }, true)) + })) - require.NoError(t, s.ACLTokenSet(3, &structs.ACLToken{ + require.NoError(t, aclTokenSetLegacy(3, &structs.ACLToken{ SecretID: "8de2dd39-134d-4cb1-950b-b7ab96ea20ba", Type: structs.ACLTokenTypeManagement, - }, true)) + })) - require.NoError(t, s.ACLTokenSet(4, &structs.ACLToken{ + require.NoError(t, aclTokenSetLegacy(4, &structs.ACLToken{ SecretID: "548bdb8e-c0d6-477b-bcc4-67fb836e9e61", Type: structs.ACLTokenTypeManagement, - }, true)) + })) - require.NoError(t, s.ACLTokenSet(5, &structs.ACLToken{ + require.NoError(t, aclTokenSetLegacy(5, &structs.ACLToken{ SecretID: "3ee33676-d9b8-4144-bf0b-92618cff438b", Type: structs.ACLTokenTypeManagement, - }, true)) + })) - require.NoError(t, s.ACLTokenSet(6, &structs.ACLToken{ + require.NoError(t, aclTokenSetLegacy(6, &structs.ACLToken{ SecretID: "fa9d658a-6e26-42ab-a5f0-1ea05c893dee", Type: structs.ACLTokenTypeManagement, - }, true)) + })) tokens, _, err := s.ACLTokenListUpgradeable(3) require.NoError(t, err) @@ -1246,7 +1161,7 @@ func TestStateStore_ACLToken_FixupPolicyLinks(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token, false)) + require.NoError(t, s.ACLTokenSet(2, token)) _, retrieved, err := s.ACLTokenGetByAccessor(nil, token.AccessorID, nil) require.NoError(t, err) @@ -1372,7 +1287,7 @@ func TestStateStore_ACLToken_FixupRoleLinks(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token, false)) + require.NoError(t, s.ACLTokenSet(2, token)) _, retrieved, err := s.ACLTokenGetByAccessor(nil, token.AccessorID, nil) require.NoError(t, err) @@ -1498,7 +1413,7 @@ func TestStateStore_ACLToken_Delete(t *testing.T) { Local: true, } - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) _, rtoken, err := s.ACLTokenGetByAccessor(nil, "f1093997-b6c7-496d-bfb8-6b1b1895641b", nil) require.NoError(t, err) diff --git a/agent/consul/state/store_integration_test.go b/agent/consul/state/store_integration_test.go index dc6cee8690..60574cb16d 100644 --- a/agent/consul/state/store_integration_test.go +++ b/agent/consul/state/store_integration_test.go @@ -52,7 +52,7 @@ func TestStore_IntegrationWithEventPublisher_ACLTokenUpdate(t *testing.T) { SecretID: "72e81982-7a0f-491f-a60e-c9c802ac1402", } token2.SetHash(false) - require.NoError(s.ACLTokenSet(3, token2.Clone(), false)) + require.NoError(s.ACLTokenSet(3, token2.Clone())) // Ensure there's no reset event. assertNoEvent(t, eventCh) @@ -64,7 +64,7 @@ func TestStore_IntegrationWithEventPublisher_ACLTokenUpdate(t *testing.T) { Description: "something else", } token3.SetHash(false) - require.NoError(s.ACLTokenSet(4, token3.Clone(), false)) + require.NoError(s.ACLTokenSet(4, token3.Clone())) // Ensure the reset event was sent. err = assertErr(t, eventCh) @@ -484,7 +484,7 @@ func createTokenAndWaitForACLEventPublish(t *testing.T, s *Store) *structs.ACLTo eventCh := testRunSub(sub) // Create the ACL token to be used in the subscription. - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) // Wait for the pre-subscription to be reset assertReset(t, eventCh, true) diff --git a/agent/rpc/subscribe/subscribe_test.go b/agent/rpc/subscribe/subscribe_test.go index d367a3ddbe..531d8ec820 100644 --- a/agent/rpc/subscribe/subscribe_test.go +++ b/agent/rpc/subscribe/subscribe_test.go @@ -857,7 +857,7 @@ node "node1" { SecretID: token, Rules: "", } - require.NoError(t, backend.store.ACLTokenSet(ids.Next("update"), aclToken, false)) + require.NoError(t, backend.store.ACLTokenSet(ids.Next("update"), aclToken)) select { case item := <-chEvents: From 32b4ad42acd2cf66b0d52480d8f7b675ac383f09 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 29 Sep 2021 18:32:15 -0400 Subject: [PATCH 4/9] acl: remove ACLTokenTypeClient, along with the last test referencing it. --- agent/structs/acl_legacy.go | 3 --- agent/structs/acl_test.go | 23 ----------------------- 2 files changed, 26 deletions(-) diff --git a/agent/structs/acl_legacy.go b/agent/structs/acl_legacy.go index bc3e4bc1d8..a0c879badd 100644 --- a/agent/structs/acl_legacy.go +++ b/agent/structs/acl_legacy.go @@ -7,9 +7,6 @@ package structs const ( - // ACLTokenTypeClient tokens have rules applied - ACLTokenTypeClient = "client" - // ACLTokenTypeManagement tokens have an always allow policy, so they can // make other tokens and can access all resources. ACLTokenTypeManagement = "management" diff --git a/agent/structs/acl_test.go b/agent/structs/acl_test.go index ec52edce0b..39fb5aefc0 100644 --- a/agent/structs/acl_test.go +++ b/agent/structs/acl_test.go @@ -155,7 +155,6 @@ func TestStructs_ACLToken_EstimateSize(t *testing.T) { } func TestStructs_ACLToken_Stub(t *testing.T) { - t.Run("Basic", func(t *testing.T) { token := ACLToken{ @@ -188,28 +187,6 @@ func TestStructs_ACLToken_Stub(t *testing.T) { require.Equal(t, token.ModifyIndex, stub.ModifyIndex) require.False(t, stub.Legacy) }) - - t.Run("Legacy", func(t *testing.T) { - token := ACLToken{ - AccessorID: "09d1c059-961a-46bd-a2e4-76adebe35fa5", - SecretID: "65e98e67-9b29-470c-8ffa-7c5a23cc67c8", - Description: "test", - Type: ACLTokenTypeClient, - Rules: `key "" { policy = "read" }`, - } - - stub := token.Stub() - require.Equal(t, token.AccessorID, stub.AccessorID) - require.Equal(t, token.SecretID, stub.SecretID) - require.Equal(t, token.Description, stub.Description) - require.Equal(t, token.Policies, stub.Policies) - require.Equal(t, token.Local, stub.Local) - require.Equal(t, token.CreateTime, stub.CreateTime) - require.Equal(t, token.Hash, stub.Hash) - require.Equal(t, token.CreateIndex, stub.CreateIndex) - require.Equal(t, token.ModifyIndex, stub.ModifyIndex) - require.True(t, stub.Legacy) - }) } func TestStructs_ACLTokens_Sort(t *testing.T) { From 3390f85ab44829007b595111fbb7d4fd2066806e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 29 Sep 2021 18:43:45 -0400 Subject: [PATCH 5/9] acl: remove ACLTokenTypeManagement --- agent/consul/acl_endpoint.go | 6 ++---- agent/consul/acl_endpoint_test.go | 2 -- agent/consul/fsm/snapshot_oss_test.go | 3 +-- agent/consul/leader.go | 9 +++------ agent/consul/state/acl_test.go | 16 +++++++--------- agent/structs/acl_legacy.go | 13 ------------- 6 files changed, 13 insertions(+), 36 deletions(-) delete mode 100644 agent/structs/acl_legacy.go diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index c9bbeaabc5..3579e245b8 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -235,10 +235,8 @@ func (a *ACL) BootstrapTokens(args *structs.DCSpecificRequest, reply *structs.AC ID: structs.ACLPolicyGlobalManagementID, }, }, - CreateTime: time.Now(), - Local: false, - // DEPRECATED (ACL-Legacy-Compat) - This is used so that the bootstrap token is still visible via the v1 acl APIs - Type: structs.ACLTokenTypeManagement, + CreateTime: time.Now(), + Local: false, EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), }, ResetIndex: specifiedIndex, diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index a7e24dc21a..bfe8a02200 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -48,7 +48,6 @@ func TestACLEndpoint_BootstrapTokens(t *testing.T) { require.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.BootstrapTokens", &arg, &out)) require.Equal(t, 36, len(out.AccessorID)) require.True(t, strings.HasPrefix(out.Description, "Bootstrap Token")) - require.Equal(t, out.Type, structs.ACLTokenTypeManagement) require.True(t, out.CreateIndex > 0) require.Equal(t, out.CreateIndex, out.ModifyIndex) @@ -69,7 +68,6 @@ func TestACLEndpoint_BootstrapTokens(t *testing.T) { require.Equal(t, 36, len(out.AccessorID)) require.NotEqual(t, oldID, out.AccessorID) require.True(t, strings.HasPrefix(out.Description, "Bootstrap Token")) - require.Equal(t, out.Type, structs.ACLTokenTypeManagement) require.True(t, out.CreateIndex > 0) require.Equal(t, out.CreateIndex, out.ModifyIndex) } diff --git a/agent/consul/fsm/snapshot_oss_test.go b/agent/consul/fsm/snapshot_oss_test.go index c4a7b3faa8..996cf2fd23 100644 --- a/agent/consul/fsm/snapshot_oss_test.go +++ b/agent/consul/fsm/snapshot_oss_test.go @@ -111,8 +111,7 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) { }, CreateTime: time.Now(), Local: false, - // DEPRECATED (ACL-Legacy-Compat) - This is used so that the bootstrap token is still visible via the v1 acl APIs - Type: structs.ACLTokenTypeManagement, + Type: "management", } require.NoError(t, fsm.state.ACLBootstrap(10, 0, token)) diff --git a/agent/consul/leader.go b/agent/consul/leader.go index e548414674..847db5c1a3 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -452,11 +452,8 @@ func (s *Server) initializeACLs(ctx context.Context) error { ID: structs.ACLPolicyGlobalManagementID, }, }, - CreateTime: time.Now(), - Local: false, - - // DEPRECATED (ACL-Legacy-Compat) - only needed for compatibility - Type: structs.ACLTokenTypeManagement, + CreateTime: time.Now(), + Local: false, EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), } @@ -599,7 +596,7 @@ func (s *Server) legacyACLTokenUpgrade(ctx context.Context) error { len(newToken.ServiceIdentities) == 0 && len(newToken.NodeIdentities) == 0 && len(newToken.Roles) == 0 && - newToken.Type == structs.ACLTokenTypeManagement { + newToken.Type == "management" { newToken.Policies = append(newToken.Policies, structs.ACLTokenPolicyLink{ID: structs.ACLPolicyGlobalManagementID}) } diff --git a/agent/consul/state/acl_test.go b/agent/consul/state/acl_test.go index 99717745b8..c86527cd1d 100644 --- a/agent/consul/state/acl_test.go +++ b/agent/consul/state/acl_test.go @@ -171,8 +171,6 @@ func TestStateStore_ACLBootstrap(t *testing.T) { }, CreateTime: time.Now(), Local: false, - // DEPRECATED (ACL-Legacy-Compat) - This is used so that the bootstrap token is still visible via the v1 acl APIs - Type: structs.ACLTokenTypeManagement, } token2 := &structs.ACLToken{ @@ -186,8 +184,6 @@ func TestStateStore_ACLBootstrap(t *testing.T) { }, CreateTime: time.Now(), Local: false, - // DEPRECATED (ACL-Legacy-Compat) - This is used so that the bootstrap token is still visible via the v1 acl APIs - Type: structs.ACLTokenTypeManagement, } s := testStateStore(t) @@ -788,29 +784,31 @@ func TestStateStore_ACLTokens_ListUpgradeable(t *testing.T) { return tx.Commit() } + const ACLTokenTypeManagement = "management" + require.NoError(t, aclTokenSetLegacy(2, &structs.ACLToken{ SecretID: "34ec8eb3-095d-417a-a937-b439af7a8e8b", - Type: structs.ACLTokenTypeManagement, + Type: ACLTokenTypeManagement, })) require.NoError(t, aclTokenSetLegacy(3, &structs.ACLToken{ SecretID: "8de2dd39-134d-4cb1-950b-b7ab96ea20ba", - Type: structs.ACLTokenTypeManagement, + Type: ACLTokenTypeManagement, })) require.NoError(t, aclTokenSetLegacy(4, &structs.ACLToken{ SecretID: "548bdb8e-c0d6-477b-bcc4-67fb836e9e61", - Type: structs.ACLTokenTypeManagement, + Type: ACLTokenTypeManagement, })) require.NoError(t, aclTokenSetLegacy(5, &structs.ACLToken{ SecretID: "3ee33676-d9b8-4144-bf0b-92618cff438b", - Type: structs.ACLTokenTypeManagement, + Type: ACLTokenTypeManagement, })) require.NoError(t, aclTokenSetLegacy(6, &structs.ACLToken{ SecretID: "fa9d658a-6e26-42ab-a5f0-1ea05c893dee", - Type: structs.ACLTokenTypeManagement, + Type: ACLTokenTypeManagement, })) tokens, _, err := s.ACLTokenListUpgradeable(3) diff --git a/agent/structs/acl_legacy.go b/agent/structs/acl_legacy.go deleted file mode 100644 index a0c879badd..0000000000 --- a/agent/structs/acl_legacy.go +++ /dev/null @@ -1,13 +0,0 @@ -// DEPRECATED (ACL-Legacy-Compat) -// -// Everything within this file is deprecated and related to the original ACL -// implementation. Once support for v1 ACLs are removed this whole file can -// be deleted. - -package structs - -const ( - // ACLTokenTypeManagement tokens have an always allow policy, so they can - // make other tokens and can access all resources. - ACLTokenTypeManagement = "management" -) From daba3c2309f610076537695104e67988a89bae62 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 4 Oct 2021 18:54:49 -0400 Subject: [PATCH 6/9] acl: remove legacy parameter to ACLDatacenter It is no longer used now that legacy ACLs have been removed. --- agent/consul/acl.go | 15 ++++----------- agent/consul/acl_client.go | 14 +++----------- agent/consul/acl_server.go | 2 +- agent/consul/acl_test.go | 2 +- 4 files changed, 9 insertions(+), 24 deletions(-) diff --git a/agent/consul/acl.go b/agent/consul/acl.go index cc2c4375ab..659fe3d23e 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -120,13 +120,6 @@ func (id *missingIdentity) EnterpriseMetadata() *structs.EnterpriseMeta { return structs.DefaultEnterpriseMetaInDefaultPartition() } -func minTTL(a time.Duration, b time.Duration) time.Duration { - if a < b { - return a - } - return b -} - type ACLRemoteError struct { Err error } @@ -145,7 +138,7 @@ func tokenSecretCacheID(token string) string { } type ACLResolverDelegate interface { - ACLDatacenter(legacy bool) string + ACLDatacenter() string ResolveIdentityFromToken(token string) (bool, structs.ACLIdentity, error) ResolvePolicyFromID(policyID string) (bool, *structs.ACLPolicy, error) ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error) @@ -361,7 +354,7 @@ func (r *ACLResolver) fetchAndCacheIdentityFromToken(token string, cached *struc cacheID := tokenSecretCacheID(token) req := structs.ACLTokenGetRequest{ - Datacenter: r.delegate.ACLDatacenter(false), + Datacenter: r.delegate.ACLDatacenter(), TokenID: token, TokenIDType: structs.ACLTokenSecret, QueryOptions: structs.QueryOptions{ @@ -449,7 +442,7 @@ func (r *ACLResolver) resolveIdentityFromToken(token string) (structs.ACLIdentit func (r *ACLResolver) fetchAndCachePoliciesForIdentity(identity structs.ACLIdentity, policyIDs []string, cached map[string]*structs.PolicyCacheEntry) (map[string]*structs.ACLPolicy, error) { req := structs.ACLPolicyBatchGetRequest{ - Datacenter: r.delegate.ACLDatacenter(false), + Datacenter: r.delegate.ACLDatacenter(), PolicyIDs: policyIDs, QueryOptions: structs.QueryOptions{ Token: identity.SecretToken(), @@ -504,7 +497,7 @@ func (r *ACLResolver) fetchAndCachePoliciesForIdentity(identity structs.ACLIdent func (r *ACLResolver) fetchAndCacheRolesForIdentity(identity structs.ACLIdentity, roleIDs []string, cached map[string]*structs.RoleCacheEntry) (map[string]*structs.ACLRole, error) { req := structs.ACLRoleBatchGetRequest{ - Datacenter: r.delegate.ACLDatacenter(false), + Datacenter: r.delegate.ACLDatacenter(), RoleIDs: roleIDs, QueryOptions: structs.QueryOptions{ Token: identity.SecretToken(), diff --git a/agent/consul/acl_client.go b/agent/consul/acl_client.go index c5666cb483..db647636db 100644 --- a/agent/consul/acl_client.go +++ b/agent/consul/acl_client.go @@ -23,17 +23,9 @@ var clientACLCacheConfig *structs.ACLCachesConfig = &structs.ACLCachesConfig{ Roles: 128, } -func (c *Client) ACLDatacenter(legacy bool) string { - // For resolution running on clients, when not in - // legacy mode the servers within the current datacenter - // must be queried first to pick up local tokens. When - // in legacy mode the clients should directly query the - // ACL Datacenter. When no ACL datacenter has been set - // then we assume that the local DC is the ACL DC - if legacy && c.config.PrimaryDatacenter != "" { - return c.config.PrimaryDatacenter - } - +func (c *Client) ACLDatacenter() string { + // For resolution running on clients servers within the current datacenter + // must be queried first to pick up local tokens. return c.config.Datacenter } diff --git a/agent/consul/acl_server.go b/agent/consul/acl_server.go index ecb6019aa2..33372bae8f 100644 --- a/agent/consul/acl_server.go +++ b/agent/consul/acl_server.go @@ -100,7 +100,7 @@ func (s *Server) LocalTokensEnabled() bool { return true } -func (s *Server) ACLDatacenter(legacy bool) string { +func (s *Server) ACLDatacenter() string { // For resolution running on servers the only option // is to contact the configured ACL Datacenter if s.config.PrimaryDatacenter != "" { diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index 1fb9a0200a..9ebf93bc27 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -613,7 +613,7 @@ func (d *ACLResolverTestDelegate) plainRoleResolveFn(args *structs.ACLRoleBatchG return nil } -func (d *ACLResolverTestDelegate) ACLDatacenter(legacy bool) string { +func (d *ACLResolverTestDelegate) ACLDatacenter() string { return d.datacenter } From 0784a31e8536c9e92260b5876e485f68f85b4bc1 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 5 Oct 2021 12:07:52 -0400 Subject: [PATCH 7/9] acl: remove init check for legacy anon token This token should always already be migrated from a previous version. --- agent/config/default.go | 5 ----- agent/consul/leader.go | 43 +++++++++++++++-------------------------- 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/agent/config/default.go b/agent/config/default.go index b916b6a93e..bb8ad905c8 100644 --- a/agent/config/default.go +++ b/agent/config/default.go @@ -18,11 +18,6 @@ func DefaultSource() Source { serfLAN := cfg.SerfLANConfig.MemberlistConfig serfWAN := cfg.SerfWANConfig.MemberlistConfig - // DEPRECATED (ACL-Legacy-Compat) - when legacy ACL support is removed these defaults - // the acl_* config entries here should be transitioned to their counterparts in the - // acl stanza for now we need to be able to detect the new entries not being set (not - // just set to the defaults here) so that we can use the old entries. So the true - // default still needs to reside in the original config values return FileSource{ Name: "default", Format: "hcl", diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 847db5c1a3..3c1633290b 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -498,35 +498,24 @@ func (s *Server) initializeACLs(ctx context.Context) error { } // Ignoring expiration times to avoid an insertion collision. if token == nil { - // DEPRECATED (ACL-Legacy-Compat) - Don't need to query for previous "anonymous" token - // check for legacy token that needs an upgrade - _, legacyToken, err := state.ACLTokenGetBySecret(nil, anonymousToken, nil) + token = &structs.ACLToken{ + AccessorID: structs.ACLTokenAnonymousID, + SecretID: anonymousToken, + Description: "Anonymous Token", + CreateTime: time.Now(), + EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), + } + token.SetHash(true) + + req := structs.ACLTokenBatchSetRequest{ + Tokens: structs.ACLTokens{token}, + CAS: false, + } + _, err := s.raftApply(structs.ACLTokenSetRequestType, &req) if err != nil { - return fmt.Errorf("failed to get anonymous token: %v", err) - } - // Ignoring expiration times to avoid an insertion collision. - - // the token upgrade routine will take care of upgrading the token if a legacy version exists - if legacyToken == nil { - token = &structs.ACLToken{ - AccessorID: structs.ACLTokenAnonymousID, - SecretID: anonymousToken, - Description: "Anonymous Token", - CreateTime: time.Now(), - EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), - } - token.SetHash(true) - - req := structs.ACLTokenBatchSetRequest{ - Tokens: structs.ACLTokens{token}, - CAS: false, - } - _, err := s.raftApply(structs.ACLTokenSetRequestType, &req) - if err != nil { - return fmt.Errorf("failed to create anonymous token: %v", err) - } - s.logger.Info("Created ACL anonymous token from configuration") + return fmt.Errorf("failed to create anonymous token: %v", err) } + s.logger.Info("Created ACL anonymous token from configuration") } // launch the upgrade go routine to generate accessors for everything s.startACLUpgrade(ctx) From 65d48e5042579ad2978a08388e6911fc2c60c3f6 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 5 Oct 2021 12:13:04 -0400 Subject: [PATCH 8/9] state: remove support for updating legacy ACL tokens --- agent/consul/state/acl.go | 6 +----- agent/structs/acl.go | 12 ------------ 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index 986fbc7ee7..9b84c6c16d 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -498,11 +498,7 @@ func aclTokenSetTxn(tx WriteTxn, idx uint64, token *structs.ACLToken, opts ACLTo } if opts.Legacy && original != nil { - if original.UsesNonLegacyFields() { - return fmt.Errorf("failed inserting acl token: cannot use legacy endpoint to modify a non-legacy token") - } - - token.AccessorID = original.AccessorID + return fmt.Errorf("legacy tokens can not be modified") } if err := aclTokenUpsertValidateEnterprise(tx, token, original); err != nil { diff --git a/agent/structs/acl.go b/agent/structs/acl.go index 15edc47f26..8f5e23c028 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -412,18 +412,6 @@ func (t *ACLToken) HasExpirationTime() bool { return t.ExpirationTime != nil && !t.ExpirationTime.IsZero() } -// TODO(ACL-Legacy-Compat): remove -func (t *ACLToken) UsesNonLegacyFields() bool { - return len(t.Policies) > 0 || - len(t.ServiceIdentities) > 0 || - len(t.NodeIdentities) > 0 || - len(t.Roles) > 0 || - t.Type == "" || - t.HasExpirationTime() || - t.ExpirationTTL != 0 || - t.AuthMethod != "" -} - func (t *ACLToken) EnterpriseMetadata() *EnterpriseMeta { return &t.EnterpriseMeta } From 5d41b4d2f4a2a042e0ad11cdf0dfa2db5a5581c4 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 25 Oct 2021 16:09:52 -0400 Subject: [PATCH 9/9] Update agent/consul/acl_client.go Co-authored-by: Freddy --- agent/consul/acl_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/consul/acl_client.go b/agent/consul/acl_client.go index db647636db..5ace9ce616 100644 --- a/agent/consul/acl_client.go +++ b/agent/consul/acl_client.go @@ -24,7 +24,7 @@ var clientACLCacheConfig *structs.ACLCachesConfig = &structs.ACLCachesConfig{ } func (c *Client) ACLDatacenter() string { - // For resolution running on clients servers within the current datacenter + // For resolution running on clients, servers within the current datacenter // must be queried first to pick up local tokens. return c.config.Datacenter }