diff --git a/agent/acl_test.go b/agent/acl_test.go index a26798bba8..5ad4880ed0 100644 --- a/agent/acl_test.go +++ b/agent/acl_test.go @@ -86,16 +86,13 @@ func (a *TestACLAgent) ResolveToken(secretID string) (acl.Authorizer, error) { return authz, err } -func (a *TestACLAgent) ResolveTokenToIdentityAndAuthorizer(secretID string) (structs.ACLIdentity, acl.Authorizer, error) { - if a.resolveAuthzFn == nil { - return nil, nil, fmt.Errorf("ResolveTokenToIdentityAndAuthorizer call is unexpected - no authz resolver callback set") +func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error) { + authz, err := a.ResolveToken(secretID) + if err != nil { + return consul.ACLResolveResult{}, err } - return a.resolveAuthzFn(secretID) -} - -func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error) { - identity, authz, err := a.ResolveTokenToIdentityAndAuthorizer(secretID) + identity, err := a.resolveIdentFn(secretID) if err != nil { return consul.ACLResolveResult{}, err } diff --git a/agent/consul/acl.go b/agent/consul/acl.go index 846e02d26c..e18feb4150 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -1053,13 +1053,16 @@ func (r *ACLResolver) resolveLocallyManagedToken(token string) (structs.ACLIdent return r.resolveLocallyManagedEnterpriseToken(token) } -func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs.ACLIdentity, acl.Authorizer, error) { +// ResolveToken to an acl.Authorizer and structs.ACLIdentity. The acl.Authorizer +// can be used to check permissions granted to the token, and the ACLIdentity +// describes the token and any defaults applied to it. +func (r *ACLResolver) ResolveToken(token string) (ACLResolveResult, error) { if !r.ACLsEnabled() { - return nil, acl.ManageAll(), nil + return ACLResolveResult{Authorizer: acl.ManageAll()}, nil } if acl.RootAuthorizer(token) != nil { - return nil, nil, acl.ErrRootDenied + return ACLResolveResult{}, acl.ErrRootDenied } // handle the anonymous token @@ -1068,7 +1071,7 @@ func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs } if ident, authz, ok := r.resolveLocallyManagedToken(token); ok { - return ident, authz, nil + return ACLResolveResult{Authorizer: authz, ACLIdentity: ident}, nil } defer metrics.MeasureSince([]string{"acl", "ResolveToken"}, time.Now()) @@ -1078,10 +1081,11 @@ func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs r.handleACLDisabledError(err) if IsACLRemoteError(err) { r.logger.Error("Error resolving token", "error", err) - return &missingIdentity{reason: "primary-dc-down", token: token}, r.down, nil + ident := &missingIdentity{reason: "primary-dc-down", token: token} + return ACLResolveResult{Authorizer: r.down, ACLIdentity: ident}, nil } - return nil, nil, err + return ACLResolveResult{}, err } // Build the Authorizer @@ -1094,7 +1098,7 @@ func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs authz, err := policies.Compile(r.cache, &conf) if err != nil { - return nil, nil, err + return ACLResolveResult{}, err } chain = append(chain, authz) @@ -1102,15 +1106,15 @@ func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs if err != nil { if IsACLRemoteError(err) { r.logger.Error("Error resolving identity defaults", "error", err) - return identity, r.down, nil + return ACLResolveResult{Authorizer: r.down, ACLIdentity: identity}, nil } - return nil, nil, err + return ACLResolveResult{}, err } else if authz != nil { chain = append(chain, authz) } chain = append(chain, acl.RootAuthorizer(r.config.ACLDefaultPolicy)) - return identity, acl.NewChainedAuthorizer(chain), nil + return ACLResolveResult{Authorizer: acl.NewChainedAuthorizer(chain), ACLIdentity: identity}, nil } type ACLResolveResult struct { @@ -1143,7 +1147,7 @@ func (r *ACLResolver) ACLsEnabled() bool { } func (r *ACLResolver) ResolveTokenAndDefaultMeta(token string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (ACLResolveResult, error) { - identity, authz, err := r.ResolveTokenToIdentityAndAuthorizer(token) + result, err := r.ResolveToken(token) if err != nil { return ACLResolveResult{}, err } @@ -1154,8 +1158,8 @@ func (r *ACLResolver) ResolveTokenAndDefaultMeta(token string, entMeta *structs. // Default the EnterpriseMeta based on the Tokens meta or actual defaults // in the case of unknown identity - if identity != nil { - entMeta.Merge(identity.EnterpriseMetadata()) + if result.ACLIdentity != nil { + entMeta.Merge(result.ACLIdentity.EnterpriseMetadata()) } else { entMeta.Merge(structs.DefaultEnterpriseMetaInDefaultPartition()) } @@ -1163,7 +1167,7 @@ func (r *ACLResolver) ResolveTokenAndDefaultMeta(token string, entMeta *structs. // Use the meta to fill in the ACL authorization context entMeta.FillAuthzContext(authzContext) - return ACLResolveResult{Authorizer: authz, ACLIdentity: identity}, err + return result, err } // aclFilter is used to filter results from our state store based on ACL rules @@ -1973,7 +1977,7 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub // not authorized for read access will be removed from subj. func filterACL(r *ACLResolver, token string, subj interface{}) error { // Get the ACL from the token - _, authorizer, err := r.ResolveTokenToIdentityAndAuthorizer(token) + authorizer, err := r.ResolveToken(token) if err != nil { return err } diff --git a/agent/consul/acl_server.go b/agent/consul/acl_server.go index 30187f9f12..ba44b5606c 100644 --- a/agent/consul/acl_server.go +++ b/agent/consul/acl_server.go @@ -165,12 +165,6 @@ func (s *serverACLResolverBackend) ResolveRoleFromID(roleID string) (bool, *stru return s.InPrimaryDatacenter() || index > 0, role, acl.ErrNotFound } -// TODO: remove -func (s *Server) ResolveToken(token string) (acl.Authorizer, error) { - _, authz, err := s.ACLResolver.ResolveTokenToIdentityAndAuthorizer(token) - return authz, err -} - func (s *Server) filterACL(token string, subj interface{}) error { return filterACL(s.ACLResolver, token, subj) } diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index 3219cebb4f..d453d50a01 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -46,10 +46,11 @@ type asyncResolutionResult struct { err error } -func verifyAuthorizerChain(t *testing.T, expected acl.Authorizer, actual acl.Authorizer) { - expectedChainAuthz, ok := expected.(*acl.ChainedAuthorizer) +func verifyAuthorizerChain(t *testing.T, expected ACLResolveResult, actual ACLResolveResult) { + t.Helper() + expectedChainAuthz, ok := expected.Authorizer.(*acl.ChainedAuthorizer) require.True(t, ok, "expected Authorizer is not a ChainedAuthorizer") - actualChainAuthz, ok := actual.(*acl.ChainedAuthorizer) + actualChainAuthz, ok := actual.Authorizer.(*acl.ChainedAuthorizer) require.True(t, ok, "actual Authorizer is not a ChainedAuthorizer") expectedChain := expectedChainAuthz.AuthorizerChain() @@ -65,19 +66,13 @@ func verifyAuthorizerChain(t *testing.T, expected acl.Authorizer, actual acl.Aut } func resolveTokenAsync(r *ACLResolver, token string, ch chan *asyncResolutionResult) { - _, authz, err := r.ResolveTokenToIdentityAndAuthorizer(token) + authz, err := r.ResolveToken(token) ch <- &asyncResolutionResult{authz: authz, err: err} } -// Deprecated: use resolveToken or ACLResolver.ResolveTokenToIdentityAndAuthorizer instead -func (r *ACLResolver) ResolveToken(token string) (acl.Authorizer, error) { - _, authz, err := r.ResolveTokenToIdentityAndAuthorizer(token) - return authz, err -} - func resolveToken(t *testing.T, r *ACLResolver, token string) acl.Authorizer { t.Helper() - _, authz, err := r.ResolveTokenToIdentityAndAuthorizer(token) + authz, err := r.ResolveToken(token) require.NoError(t, err) return authz } @@ -739,7 +734,7 @@ func TestACLResolver_Disabled(t *testing.T) { r := newTestACLResolver(t, delegate, nil) authz, err := r.ResolveToken("does not exist") - require.Equal(t, acl.ManageAll(), authz) + require.Equal(t, ACLResolveResult{Authorizer: acl.ManageAll()}, authz) require.Nil(t, err) } @@ -753,22 +748,19 @@ func TestACLResolver_ResolveRootACL(t *testing.T) { r := newTestACLResolver(t, delegate, nil) t.Run("Allow", func(t *testing.T) { - authz, err := r.ResolveToken("allow") - require.Nil(t, authz) + _, err := r.ResolveToken("allow") require.Error(t, err) require.True(t, acl.IsErrRootDenied(err)) }) t.Run("Deny", func(t *testing.T) { - authz, err := r.ResolveToken("deny") - require.Nil(t, authz) + _, err := r.ResolveToken("deny") require.Error(t, err) require.True(t, acl.IsErrRootDenied(err)) }) t.Run("Manage", func(t *testing.T) { - authz, err := r.ResolveToken("manage") - require.Nil(t, authz) + _, err := r.ResolveToken("manage") require.Error(t, err) require.True(t, acl.IsErrRootDenied(err)) }) @@ -817,7 +809,11 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz, err := r.ResolveToken("foo") require.NoError(t, err) require.NotNil(t, authz) - require.Equal(t, authz, acl.DenyAll()) + expected := ACLResolveResult{ + Authorizer: acl.DenyAll(), + ACLIdentity: &missingIdentity{reason: "primary-dc-down", token: "foo"}, + } + require.Equal(t, expected, authz) requireIdentityCached(t, r, tokenSecretCacheID("foo"), false, "not present") }) @@ -841,7 +837,11 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz, err := r.ResolveToken("foo") require.NoError(t, err) require.NotNil(t, authz) - require.Equal(t, authz, acl.AllowAll()) + expected := ACLResolveResult{ + Authorizer: acl.AllowAll(), + ACLIdentity: &missingIdentity{reason: "primary-dc-down", token: "foo"}, + } + require.Equal(t, expected, authz) requireIdentityCached(t, r, tokenSecretCacheID("foo"), false, "not present") }) @@ -958,7 +958,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { config.Config.ACLDownPolicy = "extend-cache" }) - _, authz, err := r.ResolveTokenToIdentityAndAuthorizer("not-found") + authz, err := r.ResolveToken("not-found") require.NoError(t, err) require.NotNil(t, authz) require.Equal(t, acl.Deny, authz.NodeWrite("foo", nil)) @@ -1255,10 +1255,9 @@ func TestACLResolver_DownPolicy(t *testing.T) { // the go routine spawned will eventually return and this will be a not found error retry.Run(t, func(t *retry.R) { - authz3, err := r.ResolveToken("found") + _, err := r.ResolveToken("found") assert.Error(t, err) assert.True(t, acl.IsErrNotFound(err)) - assert.Nil(t, authz3) }) requireIdentityCached(t, r, tokenSecretCacheID("found"), false, "no longer cached") @@ -1526,7 +1525,6 @@ func TestACLResolver_Client(t *testing.T) { // policies within the cache) authz, err = r.ResolveToken("a1a54629-5050-4d17-8a4e-560d2423f835") require.EqualError(t, err, acl.ErrNotFound.Error()) - require.Nil(t, authz) require.True(t, modified) require.True(t, deleted) @@ -1675,8 +1673,7 @@ func testACLResolver_variousTokens(t *testing.T, delegate *ACLResolverTestDelega runTwiceAndReset("Missing Identity", func(t *testing.T) { delegate.UseTestLocalData(nil) - authz, err := r.ResolveToken("doesn't exist") - require.Nil(t, authz) + _, err := r.ResolveToken("doesn't exist") require.Error(t, err) require.True(t, acl.IsErrNotFound(err)) }) @@ -3929,12 +3926,12 @@ func TestACLResolver_AgentRecovery(t *testing.T) { tokens.UpdateAgentRecoveryToken("9a184a11-5599-459e-b71a-550e5f9a5a23", token.TokenSourceConfig) - ident, authz, err := r.ResolveTokenToIdentityAndAuthorizer("9a184a11-5599-459e-b71a-550e5f9a5a23") + authz, err := r.ResolveToken("9a184a11-5599-459e-b71a-550e5f9a5a23") require.NoError(t, err) - require.NotNil(t, ident) - require.Equal(t, "agent-recovery:foo", ident.ID()) - require.NotNil(t, authz) - require.Equal(t, r.agentRecoveryAuthz, authz) + require.NotNil(t, authz.ACLIdentity) + require.Equal(t, "agent-recovery:foo", authz.ACLIdentity.ID()) + require.NotNil(t, authz.Authorizer) + require.Equal(t, r.agentRecoveryAuthz, authz.Authorizer) require.Equal(t, acl.Allow, authz.AgentWrite("foo", nil)) require.Equal(t, acl.Allow, authz.NodeRead("bar", nil)) require.Equal(t, acl.Deny, authz.NodeWrite("bar", nil)) @@ -3998,7 +3995,7 @@ func TestACLResolver_ACLsEnabled(t *testing.T) { } -func TestACLResolver_ResolveTokenToIdentityAndAuthorizer_UpdatesPurgeTheCache(t *testing.T) { +func TestACLResolver_ResolveToken_UpdatesPurgeTheCache(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") } @@ -4035,7 +4032,7 @@ func TestACLResolver_ResolveTokenToIdentityAndAuthorizer_UpdatesPurgeTheCache(t require.NoError(t, err) runStep(t, "first resolve", func(t *testing.T) { - _, authz, err := srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(token) + authz, err := srv.ACLResolver.ResolveToken(token) require.NoError(t, err) require.NotNil(t, authz) require.Equal(t, acl.Allow, authz.KeyRead("foo", nil)) @@ -4054,7 +4051,7 @@ func TestACLResolver_ResolveTokenToIdentityAndAuthorizer_UpdatesPurgeTheCache(t err := msgpackrpc.CallWithCodec(codec, "ACL.PolicySet", &reqPolicy, &structs.ACLPolicy{}) require.NoError(t, err) - _, authz, err := srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(token) + authz, err := srv.ACLResolver.ResolveToken(token) require.NoError(t, err) require.NotNil(t, authz) require.Equal(t, acl.Deny, authz.KeyRead("foo", nil)) @@ -4070,7 +4067,7 @@ func TestACLResolver_ResolveTokenToIdentityAndAuthorizer_UpdatesPurgeTheCache(t err := msgpackrpc.CallWithCodec(codec, "ACL.TokenDelete", &req, &resp) require.NoError(t, err) - _, _, err = srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(token) + _, err = srv.ACLResolver.ResolveToken(token) require.True(t, acl.IsErrNotFound(err), "Error %v is not acl.ErrNotFound", err) }) } diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index 2031641caa..1fb8946623 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -100,21 +100,13 @@ func (s *Intention) Apply(args *structs.IntentionRequest, reply *string) error { } // Get the ACL token for the request for the checks below. - // TODO: use ResolveTokenAndDefaultMeta - identity, authz, err := s.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token) + var entMeta structs.EnterpriseMeta + authz, err := s.srv.ACLResolver.ResolveTokenAndDefaultMeta(args.Token, &entMeta, nil) if err != nil { return err } - var accessorID string - var entMeta structs.EnterpriseMeta - if identity != nil { - entMeta.Merge(identity.EnterpriseMetadata()) - accessorID = identity.ID() - } else { - entMeta.Merge(structs.DefaultEnterpriseMetaInDefaultPartition()) - } - + accessorID := authz.AccessorID() var ( mut *structs.IntentionMutation legacyWrite bool diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index 5213349e79..44b6af5aaf 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -433,11 +433,11 @@ func (m *Internal) KeyringOperation( } // Check ACLs - identity, authz, err := m.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token) + authz, err := m.srv.ACLResolver.ResolveToken(args.Token) if err != nil { return err } - if err := m.srv.validateEnterpriseToken(identity); err != nil { + if err := m.srv.validateEnterpriseToken(authz.ACLIdentity); err != nil { return err } switch args.Operation { diff --git a/agent/consul/operator_autopilot_endpoint.go b/agent/consul/operator_autopilot_endpoint.go index d600df83cb..f4a3db65e6 100644 --- a/agent/consul/operator_autopilot_endpoint.go +++ b/agent/consul/operator_autopilot_endpoint.go @@ -17,11 +17,11 @@ func (op *Operator) AutopilotGetConfiguration(args *structs.DCSpecificRequest, r } // This action requires operator read access. - identity, authz, err := op.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token) + authz, err := op.srv.ACLResolver.ResolveToken(args.Token) if err != nil { return err } - if err := op.srv.validateEnterpriseToken(identity); err != nil { + if err := op.srv.validateEnterpriseToken(authz.ACLIdentity); err != nil { return err } if authz.OperatorRead(nil) != acl.Allow { @@ -49,11 +49,11 @@ func (op *Operator) AutopilotSetConfiguration(args *structs.AutopilotSetConfigRe } // This action requires operator write access. - identity, authz, err := op.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token) + authz, err := op.srv.ACLResolver.ResolveToken(args.Token) if err != nil { return err } - if err := op.srv.validateEnterpriseToken(identity); err != nil { + if err := op.srv.validateEnterpriseToken(authz.ACLIdentity); err != nil { return err } if authz.OperatorWrite(nil) != acl.Allow { @@ -84,11 +84,11 @@ func (op *Operator) ServerHealth(args *structs.DCSpecificRequest, reply *structs } // This action requires operator read access. - identity, authz, err := op.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token) + authz, err := op.srv.ACLResolver.ResolveToken(args.Token) if err != nil { return err } - if err := op.srv.validateEnterpriseToken(identity); err != nil { + if err := op.srv.validateEnterpriseToken(authz.ACLIdentity); err != nil { return err } if authz.OperatorRead(nil) != acl.Allow { @@ -151,11 +151,11 @@ func (op *Operator) AutopilotState(args *structs.DCSpecificRequest, reply *autop } // This action requires operator read access. - identity, authz, err := op.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token) + authz, err := op.srv.ACLResolver.ResolveToken(args.Token) if err != nil { return err } - if err := op.srv.validateEnterpriseToken(identity); err != nil { + if err := op.srv.validateEnterpriseToken(authz.ACLIdentity); err != nil { return err } if authz.OperatorRead(nil) != acl.Allow { diff --git a/agent/consul/operator_raft_endpoint.go b/agent/consul/operator_raft_endpoint.go index 72ae5b195b..431b455c08 100644 --- a/agent/consul/operator_raft_endpoint.go +++ b/agent/consul/operator_raft_endpoint.go @@ -81,11 +81,11 @@ func (op *Operator) RaftRemovePeerByAddress(args *structs.RaftRemovePeerRequest, // This is a super dangerous operation that requires operator write // access. - identity, authz, err := op.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token) + authz, err := op.srv.ACLResolver.ResolveToken(args.Token) if err != nil { return err } - if err := op.srv.validateEnterpriseToken(identity); err != nil { + if err := op.srv.validateEnterpriseToken(authz.ACLIdentity); err != nil { return err } if authz.OperatorWrite(nil) != acl.Allow { @@ -134,11 +134,11 @@ func (op *Operator) RaftRemovePeerByID(args *structs.RaftRemovePeerRequest, repl // This is a super dangerous operation that requires operator write // access. - identity, authz, err := op.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token) + authz, err := op.srv.ACLResolver.ResolveToken(args.Token) if err != nil { return err } - if err := op.srv.validateEnterpriseToken(identity); err != nil { + if err := op.srv.validateEnterpriseToken(authz.ACLIdentity); err != nil { return err } if authz.OperatorWrite(nil) != acl.Allow {