Merge pull request #12167 from hashicorp/dnephin/acl-resolve-token-3

acl: rename ResolveTokenToIdentityAndAuthorizer to ResolveToken

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) {
+func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error) {
-	if a.resolveAuthzFn == nil {
+	authz, err := a.ResolveToken(secretID)
-		return nil, nil, fmt.Errorf("ResolveTokenToIdentityAndAuthorizer call is unexpected - no authz resolver callback set")
+	if err != nil {
+		return consul.ACLResolveResult{}, err
 	}
 
-	return a.resolveAuthzFn(secretID)
+	identity, err := a.resolveIdentFn(secretID)
-}
-
-func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error) {
-	identity, authz, err := a.ResolveTokenToIdentityAndAuthorizer(secretID)
 	if err != nil {
 		return consul.ACLResolveResult{}, err
 	}

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 {
+	if result.ACLIdentity != nil {
-		entMeta.Merge(identity.EnterpriseMetadata())
+		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
 	}

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

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) {
+func verifyAuthorizerChain(t *testing.T, expected ACLResolveResult, actual ACLResolveResult) {
-	expectedChainAuthz, ok := expected.(*acl.ChainedAuthorizer)
+	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")
+		_, err := r.ResolveToken("allow")
-		require.Nil(t, authz)
 		require.Error(t, err)
 		require.True(t, acl.IsErrRootDenied(err))
 	})
 
 	t.Run("Deny", func(t *testing.T) {
-		authz, err := r.ResolveToken("deny")
+		_, err := r.ResolveToken("deny")
-		require.Nil(t, authz)
 		require.Error(t, err)
 		require.True(t, acl.IsErrRootDenied(err))
 	})
 
 	t.Run("Manage", func(t *testing.T) {
-		authz, err := r.ResolveToken("manage")
+		_, err := r.ResolveToken("manage")
-		require.Nil(t, authz)
 		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")
+		_, err := r.ResolveToken("doesn't exist")
-		require.Nil(t, authz)
 		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.NotNil(t, authz.ACLIdentity)
-	require.Equal(t, "agent-recovery:foo", ident.ID())
+	require.Equal(t, "agent-recovery:foo", authz.ACLIdentity.ID())
-	require.NotNil(t, authz)
+	require.NotNil(t, authz.Authorizer)
-	require.Equal(t, r.agentRecoveryAuthz, authz)
+	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)
 	})
 }

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
+	var entMeta structs.EnterpriseMeta
-	identity, authz, err := s.srv.ACLResolver.ResolveTokenToIdentityAndAuthorizer(args.Token)
+	authz, err := s.srv.ACLResolver.ResolveTokenAndDefaultMeta(args.Token, &entMeta, nil)
 	if err != nil {
 		return err
 	}
 
-	var accessorID string
+	accessorID := authz.AccessorID()
-	var entMeta structs.EnterpriseMeta
-	if identity != nil {
-		entMeta.Merge(identity.EnterpriseMetadata())
-		accessorID = identity.ID()
-	} else {
-		entMeta.Merge(structs.DefaultEnterpriseMetaInDefaultPartition())
-	}
-
 	var (
 		mut         *structs.IntentionMutation
 		legacyWrite bool

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 {

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 {

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 {