acl: remove ResolveTokenToIdentity

By exposing the AccessorID from the primary ResolveToken method we can
remove this duplication.

agent/acl.go

@@ -15,7 +15,7 @@ import (
 // critical purposes, such as logging. Therefore we interpret all errors as empty-string
 // so we can safely log it without handling non-critical errors at the usage site.
 func (a *Agent) aclAccessorID(secretID string) string {
-	ident, err := a.delegate.ResolveTokenToIdentity(secretID)
+	ident, err := a.delegate.ResolveTokenAndDefaultMeta(secretID, nil, nil)
 	if acl.IsErrNotFound(err) {
 		return ""
 	}
@@ -23,10 +23,7 @@ func (a *Agent) aclAccessorID(secretID string) string {
 		a.logger.Debug("non-critical error resolving acl token accessor for logging", "error", err)
 		return ""
 	}
-	if ident == nil {
+	return ident.AccessorID()
-		return ""
-	}
-	return ident.ID()
 }
 
 // vetServiceRegister makes sure the service registration action is allowed by

agent/acl_test.go

@@ -523,22 +523,3 @@ func TestACL_filterChecksWithAuthorizer(t *testing.T) {
 	_, ok = checks["my-other"]
 	require.False(t, ok)
 }
-
-// TODO: remove?
-func TestACL_ResolveIdentity(t *testing.T) {
-	t.Parallel()
-	a := NewTestACLAgent(t, t.Name(), TestACLConfig(), nil, catalogIdent)
-
-	// this test is meant to ensure we are calling the correct function
-	// which is ResolveTokenToIdentity on the Agent delegate. Our
-	// nil authz resolver will cause it to emit an error if used
-	ident, err := a.delegate.ResolveTokenToIdentity(nodeROSecret)
-	require.NoError(t, err)
-	require.NotNil(t, ident)
-
-	// just double checkingto ensure if we had used the wrong function
-	// that an error would be produced
-	_, err = a.delegate.ResolveTokenAndDefaultMeta(nodeROSecret, nil, nil)
-	require.Error(t, err)
-
-}

agent/agent.go

@@ -167,9 +167,6 @@ type delegate interface {
 	// RemoveFailedNode is used to remove a failed node from the cluster.
 	RemoveFailedNode(node string, prune bool, entMeta *structs.EnterpriseMeta) error
 
-	// TODO: replace this method with consul.ACLResolver
-	ResolveTokenToIdentity(token string) (structs.ACLIdentity, error)
-
 	// ResolveTokenAndDefaultMeta returns an acl.Authorizer which authorizes
 	// actions based on the permissions granted to the token.
 	// If either entMeta or authzContext are non-nil they will be populated with the

agent/consul/acl.go

@@ -1128,33 +1128,6 @@ func (a ACLResolveResult) AccessorID() string {
 	return a.ACLIdentity.ID()
 }
 
-// TODO: rename to AccessorIDFromToken. This method is only used to retrieve the
-// ACLIdentity.ID, so we don't need to return a full ACLIdentity. We could
-// return a much smaller type (instad of just a string) to allow for changes
-// in the future.
-func (r *ACLResolver) ResolveTokenToIdentity(token string) (structs.ACLIdentity, error) {
-	if !r.ACLsEnabled() {
-		return nil, nil
-	}
-
-	if acl.RootAuthorizer(token) != nil {
-		return nil, acl.ErrRootDenied
-	}
-
-	// handle the anonymous token
-	if token == "" {
-		token = anonymousToken
-	}
-
-	if ident, _, ok := r.resolveLocallyManagedToken(token); ok {
-		return ident, nil
-	}
-
-	defer metrics.MeasureSince([]string{"acl", "ResolveTokenToIdentity"}, time.Now())
-
-	return r.resolveIdentityFromToken(token)
-}
-
 func (r *ACLResolver) ACLsEnabled() bool {
 	// Whether we desire ACLs to be enabled according to configuration
 	if !r.config.ACLsEnabled {

agent/consul/acl_test.go

@@ -1534,36 +1534,6 @@ func TestACLResolver_Client(t *testing.T) {
 		require.Equal(t, policyResolves, int32(3))
 	})
 
-	t.Run("Resolve-Identity", func(t *testing.T) {
-		t.Parallel()
-
-		delegate := &ACLResolverTestDelegate{
-			enabled:       true,
-			datacenter:    "dc1",
-			legacy:        false,
-			localTokens:   false,
-			localPolicies: false,
-		}
-
-		delegate.tokenReadFn = delegate.plainTokenReadFn
-		delegate.policyResolveFn = delegate.plainPolicyResolveFn
-		delegate.roleResolveFn = delegate.plainRoleResolveFn
-
-		r := newTestACLResolver(t, delegate, nil)
-
-		ident, err := r.ResolveTokenToIdentity("found-policy-and-role")
-		require.NoError(t, err)
-		require.NotNil(t, ident)
-		require.Equal(t, "5f57c1f6-6a89-4186-9445-531b316e01df", ident.ID())
-		require.EqualValues(t, 0, delegate.localTokenResolutions)
-		require.EqualValues(t, 1, delegate.remoteTokenResolutions)
-		require.EqualValues(t, 0, delegate.localPolicyResolutions)
-		require.EqualValues(t, 0, delegate.remotePolicyResolutions)
-		require.EqualValues(t, 0, delegate.localRoleResolutions)
-		require.EqualValues(t, 0, delegate.remoteRoleResolutions)
-		require.EqualValues(t, 0, delegate.remoteLegacyResolutions)
-	})
-
 	t.Run("Concurrent-Token-Resolve", func(t *testing.T) {
 		t.Parallel()
 

agent/local/state.go

@@ -14,6 +14,7 @@ import (
 	"github.com/hashicorp/go-hclog"
 
 	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/agent/consul"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/agent/token"
 	"github.com/hashicorp/consul/api"
@@ -150,7 +151,7 @@ func (c *CheckState) CriticalFor() time.Duration {
 
 type rpc interface {
 	RPC(method string, args interface{}, reply interface{}) error
-	ResolveTokenToIdentity(secretID string) (structs.ACLIdentity, error)
+	ResolveTokenAndDefaultMeta(token string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error)
 }
 
 // State is used to represent the node's services,
@@ -1538,7 +1539,7 @@ func (l *State) notifyIfAliased(serviceID structs.ServiceID) {
 // critical purposes, such as logging. Therefore we interpret all errors as empty-string
 // so we can safely log it without handling non-critical errors at the usage site.
 func (l *State) aclAccessorID(secretID string) string {
-	ident, err := l.Delegate.ResolveTokenToIdentity(secretID)
+	ident, err := l.Delegate.ResolveTokenAndDefaultMeta(secretID, nil, nil)
 	if acl.IsErrNotFound(err) {
 		return ""
 	}
@@ -1546,8 +1547,5 @@ func (l *State) aclAccessorID(secretID string) string {
 		l.logger.Debug("non-critical error resolving acl token accessor for logging", "error", err)
 		return ""
 	}
-	if ident == nil {
+	return ident.AccessorID()
-		return ""
-	}
-	return ident.ID()
 }

agent/local/state_test.go

@@ -12,8 +12,10 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent"
 	"github.com/hashicorp/consul/agent/config"
+	"github.com/hashicorp/consul/agent/consul"
 	"github.com/hashicorp/consul/agent/local"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/agent/token"
@@ -2372,6 +2374,6 @@ func (f *fakeRPC) RPC(method string, args interface{}, reply interface{}) error
 	return nil
 }
 
-func (f *fakeRPC) ResolveTokenToIdentity(_ string) (structs.ACLIdentity, error) {
+func (f *fakeRPC) ResolveTokenAndDefaultMeta(string, *structs.EnterpriseMeta, *acl.AuthorizerContext) (consul.ACLResolveResult, error) {
-	return nil, nil
+	return consul.ACLResolveResult{}, nil
 }