acl: rename ResolveTokenToIdentityAndAuthorizer to ResolveToken

This change allows us to remove one of the last remaining duplicate
resolve token methods (Server.ResolveToken).

With this change we are down to only 2, where the second one also
handles setting the default EnterpriseMeta from the token.

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

@@ -1051,13 +1051,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
@@ -1066,7 +1069,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())
@@ -1076,10 +1079,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
@@ -1092,7 +1096,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)
 
@@ -1100,15 +1104,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 {
@@ -1141,7 +1145,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
 	}
@@ -1152,8 +1156,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())
 	}
@@ -1161,7 +1165,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
@@ -1971,7 +1975,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

@@ -159,12 +159,6 @@ func (s *Server) ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error
 	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 {