Merge pull request #10737 from hashicorp/dnephin/remove-authorizer-nil-checks

acl: remove authz == nil checks

agent/acl.go

@@ -40,10 +40,6 @@ func (a *Agent) vetServiceRegister(token string, service *structs.NodeService) e
 }
 
 func (a *Agent) vetServiceRegisterWithAuthorizer(authz acl.Authorizer, service *structs.NodeService) error {
-	if authz == nil {
-		return nil
-	}
-
 	var authzContext acl.AuthorizerContext
 	service.FillAuthzContext(&authzContext)
 	// Vet the service itself.
@@ -73,19 +69,6 @@ func (a *Agent) vetServiceRegisterWithAuthorizer(authz acl.Authorizer, service *
 	return nil
 }
 
-// vetServiceUpdate makes sure the service update action is allowed by the given
-// token.
-// TODO: move to test package
-func (a *Agent) vetServiceUpdate(token string, serviceID structs.ServiceID) error {
-	// Resolve the token and bail if ACLs aren't enabled.
-	authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
-	if err != nil {
-		return err
-	}
-
-	return a.vetServiceUpdateWithAuthorizer(authz, serviceID)
-}
-
 func (a *Agent) vetServiceUpdateWithAuthorizer(authz acl.Authorizer, serviceID structs.ServiceID) error {
 	var authzContext acl.AuthorizerContext
 
@@ -103,23 +86,7 @@ func (a *Agent) vetServiceUpdateWithAuthorizer(authz acl.Authorizer, serviceID s
 	return nil
 }
 
-// vetCheckRegister makes sure the check registration action is allowed by the
-// given token.
-func (a *Agent) vetCheckRegister(token string, check *structs.HealthCheck) error {
-	// Resolve the token and bail if ACLs aren't enabled.
-	authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
-	if err != nil {
-		return err
-	}
-
-	return a.vetCheckRegisterWithAuthorizer(authz, check)
-}
-
 func (a *Agent) vetCheckRegisterWithAuthorizer(authz acl.Authorizer, check *structs.HealthCheck) error {
-	if authz == nil {
-		return nil
-	}
-
 	var authzContext acl.AuthorizerContext
 	check.FillAuthzContext(&authzContext)
 	// Vet the check itself.
@@ -149,22 +116,7 @@ func (a *Agent) vetCheckRegisterWithAuthorizer(authz acl.Authorizer, check *stru
 	return nil
 }
 
-// vetCheckUpdate makes sure that a check update is allowed by the given token.
-func (a *Agent) vetCheckUpdate(token string, checkID structs.CheckID) error {
-	// Resolve the token and bail if ACLs aren't enabled.
-	authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
-	if err != nil {
-		return err
-	}
-
-	return a.vetCheckUpdateWithAuthorizer(authz, checkID)
-}
-
 func (a *Agent) vetCheckUpdateWithAuthorizer(authz acl.Authorizer, checkID structs.CheckID) error {
-	if authz == nil {
-		return nil
-	}
-
 	var authzContext acl.AuthorizerContext
 	checkID.FillAuthzContext(&authzContext)
 
@@ -193,9 +145,6 @@ func (a *Agent) filterMembers(token string, members *[]serf.Member) error {
 	if err != nil {
 		return err
 	}
-	if rule == nil {
-		return nil
-	}
 
 	var authzContext acl.AuthorizerContext
 	structs.DefaultEnterpriseMetaInDefaultPartition().FillAuthzContext(&authzContext)
@@ -215,21 +164,7 @@ func (a *Agent) filterMembers(token string, members *[]serf.Member) error {
 	return nil
 }
 
-// filterServices redacts services that the token doesn't have access to.
-func (a *Agent) filterServices(token string, services *map[structs.ServiceID]*structs.NodeService) error {
-	// Resolve the token and bail if ACLs aren't enabled.
-	authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
-	if err != nil {
-		return err
-	}
-
-	return a.filterServicesWithAuthorizer(authz, services)
-}
-
 func (a *Agent) filterServicesWithAuthorizer(authz acl.Authorizer, services *map[structs.ServiceID]*structs.NodeService) error {
-	if authz == nil {
-		return nil
-	}
 	var authzContext acl.AuthorizerContext
 	// Filter out services based on the service policy.
 	for id, service := range *services {
@@ -243,22 +178,7 @@ func (a *Agent) filterServicesWithAuthorizer(authz acl.Authorizer, services *map
 	return nil
 }
 
-// filterChecks redacts checks that the token doesn't have access to.
-func (a *Agent) filterChecks(token string, checks *map[structs.CheckID]*structs.HealthCheck) error {
-	// Resolve the token and bail if ACLs aren't enabled.
-	authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
-	if err != nil {
-		return err
-	}
-
-	return a.filterChecksWithAuthorizer(authz, checks)
-}
-
 func (a *Agent) filterChecksWithAuthorizer(authz acl.Authorizer, checks *map[structs.CheckID]*structs.HealthCheck) error {
-	if authz == nil {
-		return nil
-	}
-
 	var authzContext acl.AuthorizerContext
 	// Filter out checks based on the node or service policy.
 	for id, check := range *checks {

agent/acl_endpoint.go

@@ -1156,8 +1156,6 @@ func (s *HTTPHandlers) ACLAuthorize(resp http.ResponseWriter, req *http.Request)
 		authz, err := s.agent.delegate.ResolveTokenAndDefaultMeta(request.Token, nil, nil)
 		if err != nil {
 			return nil, err
-		} else if authz == nil {
-			return nil, fmt.Errorf("Failed to initialize authorizer")
 		}
 
 		responses, err = structs.CreateACLAuthorizationResponses(authz, request.Requests)

agent/acl_test.go

@@ -294,12 +294,21 @@ func TestACL_vetServiceRegister(t *testing.T) {
 	require.True(t, acl.IsErrPermissionDenied(err))
 }
 
-func TestACL_vetServiceUpdate(t *testing.T) {
+func TestACL_vetServiceUpdateWithAuthorizer(t *testing.T) {
 	t.Parallel()
 	a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent)
 
+	vetServiceUpdate := func(token string, serviceID structs.ServiceID) error {
+		authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
+		if err != nil {
+			return err
+		}
+
+		return a.vetServiceUpdateWithAuthorizer(authz, serviceID)
+	}
+
 	// Update a service that doesn't exist.
-	err := a.vetServiceUpdate(serviceRWSecret, structs.NewServiceID("my-service", nil))
+	err := vetServiceUpdate(serviceRWSecret, structs.NewServiceID("my-service", nil))
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "Unknown service")
 
@@ -308,21 +317,29 @@ func TestACL_vetServiceUpdate(t *testing.T) {
 		ID:      "my-service",
 		Service: "service",
 	}, "")
-	err = a.vetServiceUpdate(serviceRWSecret, structs.NewServiceID("my-service", nil))
+	err = vetServiceUpdate(serviceRWSecret, structs.NewServiceID("my-service", nil))
 	require.NoError(t, err)
 
 	// Update without write privs.
-	err = a.vetServiceUpdate(serviceROSecret, structs.NewServiceID("my-service", nil))
+	err = vetServiceUpdate(serviceROSecret, structs.NewServiceID("my-service", nil))
 	require.Error(t, err)
 	require.True(t, acl.IsErrPermissionDenied(err))
 }
 
-func TestACL_vetCheckRegister(t *testing.T) {
+func TestACL_vetCheckRegisterWithAuthorizer(t *testing.T) {
 	t.Parallel()
 	a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent)
 
+	vetCheckRegister := func(token string, check *structs.HealthCheck) error {
+		authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
+		if err != nil {
+			return err
+		}
+		return a.vetCheckRegisterWithAuthorizer(authz, check)
+	}
+
 	// Register a new service check with write privs.
-	err := a.vetCheckRegister(serviceRWSecret, &structs.HealthCheck{
+	err := vetCheckRegister(serviceRWSecret, &structs.HealthCheck{
 		CheckID:     types.CheckID("my-check"),
 		ServiceID:   "my-service",
 		ServiceName: "service",
@@ -330,7 +347,7 @@ func TestACL_vetCheckRegister(t *testing.T) {
 	require.NoError(t, err)
 
 	// Register a new service check without write privs.
-	err = a.vetCheckRegister(serviceROSecret, &structs.HealthCheck{
+	err = vetCheckRegister(serviceROSecret, &structs.HealthCheck{
 		CheckID:     types.CheckID("my-check"),
 		ServiceID:   "my-service",
 		ServiceName: "service",
@@ -339,13 +356,13 @@ func TestACL_vetCheckRegister(t *testing.T) {
 	require.True(t, acl.IsErrPermissionDenied(err))
 
 	// Register a new node check with write privs.
-	err = a.vetCheckRegister(nodeRWSecret, &structs.HealthCheck{
+	err = vetCheckRegister(nodeRWSecret, &structs.HealthCheck{
 		CheckID: types.CheckID("my-check"),
 	})
 	require.NoError(t, err)
 
 	// Register a new node check without write privs.
-	err = a.vetCheckRegister(nodeROSecret, &structs.HealthCheck{
+	err = vetCheckRegister(nodeROSecret, &structs.HealthCheck{
 		CheckID: types.CheckID("my-check"),
 	})
 	require.Error(t, err)
@@ -362,7 +379,7 @@ func TestACL_vetCheckRegister(t *testing.T) {
 		ServiceID:   "my-service",
 		ServiceName: "other",
 	}, "")
-	err = a.vetCheckRegister(serviceRWSecret, &structs.HealthCheck{
+	err = vetCheckRegister(serviceRWSecret, &structs.HealthCheck{
 		CheckID:     types.CheckID("my-check"),
 		ServiceID:   "my-service",
 		ServiceName: "service",
@@ -374,7 +391,7 @@ func TestACL_vetCheckRegister(t *testing.T) {
 	a.State.AddCheck(&structs.HealthCheck{
 		CheckID: types.CheckID("my-node-check"),
 	}, "")
-	err = a.vetCheckRegister(serviceRWSecret, &structs.HealthCheck{
+	err = vetCheckRegister(serviceRWSecret, &structs.HealthCheck{
 		CheckID:     types.CheckID("my-node-check"),
 		ServiceID:   "my-service",
 		ServiceName: "service",
@@ -383,12 +400,21 @@ func TestACL_vetCheckRegister(t *testing.T) {
 	require.True(t, acl.IsErrPermissionDenied(err))
 }
 
-func TestACL_vetCheckUpdate(t *testing.T) {
+func TestACL_vetCheckUpdateWithAuthorizer(t *testing.T) {
 	t.Parallel()
 	a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent)
 
+	vetCheckUpdate := func(token string, checkID structs.CheckID) error {
+		authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
+		if err != nil {
+			return err
+		}
+
+		return a.vetCheckUpdateWithAuthorizer(authz, checkID)
+	}
+
 	// Update a check that doesn't exist.
-	err := a.vetCheckUpdate(nodeRWSecret, structs.NewCheckID("my-check", nil))
+	err := vetCheckUpdate(nodeRWSecret, structs.NewCheckID("my-check", nil))
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "Unknown check")
 
@@ -402,11 +428,11 @@ func TestACL_vetCheckUpdate(t *testing.T) {
 		ServiceID:   "my-service",
 		ServiceName: "service",
 	}, "")
-	err = a.vetCheckUpdate(serviceRWSecret, structs.NewCheckID("my-service-check", nil))
+	err = vetCheckUpdate(serviceRWSecret, structs.NewCheckID("my-service-check", nil))
 	require.NoError(t, err)
 
 	// Update service check without write privs.
-	err = a.vetCheckUpdate(serviceROSecret, structs.NewCheckID("my-service-check", nil))
+	err = vetCheckUpdate(serviceROSecret, structs.NewCheckID("my-service-check", nil))
 	require.Error(t, err)
 	require.True(t, acl.IsErrPermissionDenied(err), "not permission denied: %s", err.Error())
 
@@ -414,11 +440,11 @@ func TestACL_vetCheckUpdate(t *testing.T) {
 	a.State.AddCheck(&structs.HealthCheck{
 		CheckID: types.CheckID("my-node-check"),
 	}, "")
-	err = a.vetCheckUpdate(nodeRWSecret, structs.NewCheckID("my-node-check", nil))
+	err = vetCheckUpdate(nodeRWSecret, structs.NewCheckID("my-node-check", nil))
 	require.NoError(t, err)
 
 	// Update without write privs.
-	err = a.vetCheckUpdate(nodeROSecret, structs.NewCheckID("my-node-check", nil))
+	err = vetCheckUpdate(nodeROSecret, structs.NewCheckID("my-node-check", nil))
 	require.Error(t, err)
 	require.True(t, acl.IsErrPermissionDenied(err))
 }
@@ -442,31 +468,49 @@ func TestACL_filterMembers(t *testing.T) {
 	require.Equal(t, members[1].Name, "Node 2")
 }
 
-func TestACL_filterServices(t *testing.T) {
+func TestACL_filterServicesWithAuthorizer(t *testing.T) {
 	t.Parallel()
 	a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent)
 
+	filterServices := func(token string, services *map[structs.ServiceID]*structs.NodeService) error {
+		authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
+		if err != nil {
+			return err
+		}
+
+		return a.filterServicesWithAuthorizer(authz, services)
+	}
+
 	services := make(map[structs.ServiceID]*structs.NodeService)
-	require.NoError(t, a.filterServices(nodeROSecret, &services))
+	require.NoError(t, filterServices(nodeROSecret, &services))
 
 	services[structs.NewServiceID("my-service", nil)] = &structs.NodeService{ID: "my-service", Service: "service"}
 	services[structs.NewServiceID("my-other", nil)] = &structs.NodeService{ID: "my-other", Service: "other"}
-	require.NoError(t, a.filterServices(serviceROSecret, &services))
+	require.NoError(t, filterServices(serviceROSecret, &services))
 	require.Contains(t, services, structs.NewServiceID("my-service", nil))
 	require.NotContains(t, services, structs.NewServiceID("my-other", nil))
 }
 
-func TestACL_filterChecks(t *testing.T) {
+func TestACL_filterChecksWithAuthorizer(t *testing.T) {
 	t.Parallel()
 	a := NewTestACLAgent(t, t.Name(), TestACLConfig(), catalogPolicy, catalogIdent)
 
+	filterChecks := func(token string, checks *map[structs.CheckID]*structs.HealthCheck) error {
+		authz, err := a.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
+		if err != nil {
+			return err
+		}
+
+		return a.filterChecksWithAuthorizer(authz, checks)
+	}
+
 	checks := make(map[structs.CheckID]*structs.HealthCheck)
-	require.NoError(t, a.filterChecks(nodeROSecret, &checks))
+	require.NoError(t, filterChecks(nodeROSecret, &checks))
 
 	checks[structs.NewCheckID("my-node", nil)] = &structs.HealthCheck{}
 	checks[structs.NewCheckID("my-service", nil)] = &structs.HealthCheck{ServiceName: "service"}
 	checks[structs.NewCheckID("my-other", nil)] = &structs.HealthCheck{ServiceName: "other"}
-	require.NoError(t, a.filterChecks(serviceROSecret, &checks))
+	require.NoError(t, filterChecks(serviceROSecret, &checks))
 	_, ok := checks[structs.NewCheckID("my-node", nil)]
 	require.False(t, ok)
 	_, ok = checks[structs.NewCheckID("my-service", nil)]
@@ -477,7 +521,7 @@ func TestACL_filterChecks(t *testing.T) {
 	checks[structs.NewCheckID("my-node", nil)] = &structs.HealthCheck{}
 	checks[structs.NewCheckID("my-service", nil)] = &structs.HealthCheck{ServiceName: "service"}
 	checks[structs.NewCheckID("my-other", nil)] = &structs.HealthCheck{ServiceName: "other"}
-	require.NoError(t, a.filterChecks(nodeROSecret, &checks))
+	require.NoError(t, filterChecks(nodeROSecret, &checks))
 	_, ok = checks[structs.NewCheckID("my-node", nil)]
 	require.True(t, ok)
 	_, ok = checks[structs.NewCheckID("my-service", nil)]

agent/agent_endpoint.go

@@ -175,7 +175,7 @@ func (s *HTTPHandlers) AgentMetricsStream(resp http.ResponseWriter, req *http.Re
 	switch {
 	case err != nil:
 		return nil, err
-	case rule != nil && rule.AgentRead(s.agent.config.NodeName, nil) != acl.Allow:
+	case rule.AgentRead(s.agent.config.NodeName, nil) != acl.Allow:
 		return nil, acl.ErrPermissionDenied
 	}
 

agent/agent_endpoint_test.go

@@ -1459,7 +1459,7 @@ func TestHTTPHandlers_AgentMetricsStream(t *testing.T) {
 	bd.Tokens = new(tokenStore.Store)
 	sink := metrics.NewInmemSink(20*time.Millisecond, time.Second)
 	bd.MetricsHandler = sink
-	d := fakeResolveTokenDelegate{}
+	d := fakeResolveTokenDelegate{authorizer: acl.ManageAll()}
 	agent := &Agent{
 		baseDeps: bd,
 		delegate: d,

agent/connect_auth.go

@@ -132,14 +132,6 @@ func (a *Agent) ConnectAuthorize(token string,
 		return false, reason, &meta, nil
 	}
 
-	// No match, we need to determine the default behavior. We do this by
-	// fetching the default intention behavior from the resolved authorizer. The
-	// default behavior if ACLs are disabled is to allow connections to mimic the
-	// behavior of Consul itself: everything is allowed if ACLs are disabled.
-	if authz == nil {
-		// ACLs not enabled at all, the default is allow all.
-		return true, "ACLs disabled, access is allowed by default", &meta, nil
-	}
 	reason = "Default behavior configured by ACLs"
 	return authz.IntentionDefaultAllow(nil) == acl.Allow, reason, &meta, nil
 }

agent/consul/acl_endpoint.go

@@ -293,7 +293,7 @@ func (a *ACL) TokenRead(args *structs.ACLTokenGetRequest, reply *structs.ACLToke
 		// secrets will be redacted
 		if authz, err = a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 			return err
-		} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+		} else if authz.ACLRead(&authzContext) != acl.Allow {
 			return acl.ErrPermissionDenied
 		}
 	}
@@ -358,7 +358,7 @@ func (a *ACL) TokenClone(args *structs.ACLTokenSetRequest, reply *structs.ACLTok
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.ACLToken.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -429,7 +429,7 @@ func (a *ACL) TokenSet(args *structs.ACLTokenSetRequest, reply *structs.ACLToken
 	var authzContext acl.AuthorizerContext
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.ACLToken.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -835,7 +835,7 @@ func (a *ACL) TokenDelete(args *structs.ACLTokenDeleteRequest, reply *string) er
 	var authzContext acl.AuthorizerContext
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -924,7 +924,7 @@ func (a *ACL) TokenList(args *structs.ACLTokenListRequest, reply *structs.ACLTok
 	// merge the token default meta into the requests meta
 	args.EnterpriseMeta.Merge(&requestMeta)
 	args.EnterpriseMeta.FillAuthzContext(&authzContext)
-	if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -981,8 +981,6 @@ func (a *ACL) TokenBatchRead(args *structs.ACLTokenBatchGetRequest, reply *struc
 	authz, err := a.srv.ResolveToken(args.Token)
 	if err != nil {
 		return err
-	} else if authz == nil {
-		return acl.ErrPermissionDenied
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -1035,7 +1033,7 @@ func (a *ACL) PolicyRead(args *structs.ACLPolicyGetRequest, reply *structs.ACLPo
 	var authzContext acl.AuthorizerContext
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	} else if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1073,8 +1071,6 @@ func (a *ACL) PolicyBatchRead(args *structs.ACLPolicyBatchGetRequest, reply *str
 	authz, err := a.srv.ResolveToken(args.Token)
 	if err != nil {
 		return err
-	} else if authz == nil {
-		return acl.ErrPermissionDenied
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -1115,7 +1111,7 @@ func (a *ACL) PolicySet(args *structs.ACLPolicySetRequest, reply *structs.ACLPol
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.Policy.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1246,7 +1242,7 @@ func (a *ACL) PolicyDelete(args *structs.ACLPolicyDeleteRequest, reply *string)
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1297,7 +1293,7 @@ func (a *ACL) PolicyList(args *structs.ACLPolicyListRequest, reply *structs.ACLP
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	} else if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1469,7 +1465,7 @@ func (a *ACL) RoleRead(args *structs.ACLRoleGetRequest, reply *structs.ACLRoleRe
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	} else if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1507,8 +1503,6 @@ func (a *ACL) RoleBatchRead(args *structs.ACLRoleBatchGetRequest, reply *structs
 	authz, err := a.srv.ResolveToken(args.Token)
 	if err != nil {
 		return err
-	} else if authz == nil {
-		return acl.ErrPermissionDenied
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -1549,7 +1543,7 @@ func (a *ACL) RoleSet(args *structs.ACLRoleSetRequest, reply *structs.ACLRole) e
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.Role.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1707,7 +1701,7 @@ func (a *ACL) RoleDelete(args *structs.ACLRoleDeleteRequest, reply *string) erro
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1754,7 +1748,7 @@ func (a *ACL) RoleList(args *structs.ACLRoleListRequest, reply *structs.ACLRoleL
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	} else if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1853,7 +1847,7 @@ func (a *ACL) BindingRuleRead(args *structs.ACLBindingRuleGetRequest, reply *str
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	} else if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -1894,7 +1888,7 @@ func (a *ACL) BindingRuleSet(args *structs.ACLBindingRuleSetRequest, reply *stru
 	// Verify token is permitted to modify ACLs
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.BindingRule.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -2023,16 +2017,15 @@ func (a *ACL) BindingRuleDelete(args *structs.ACLBindingRuleDeleteRequest, reply
 	// Verify token is permitted to modify ACLs
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
 	_, rule, err := a.srv.fsm.State().ACLBindingRuleGetByID(nil, args.BindingRuleID, &args.EnterpriseMeta)
-	if err != nil {
+	switch {
+	case err != nil:
 		return err
-	}
+	case rule == nil:
-
-	if rule == nil {
 		return nil
 	}
 
@@ -2072,7 +2065,7 @@ func (a *ACL) BindingRuleList(args *structs.ACLBindingRuleListRequest, reply *st
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	} else if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -2111,7 +2104,7 @@ func (a *ACL) AuthMethodRead(args *structs.ACLAuthMethodGetRequest, reply *struc
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	} else if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -2156,7 +2149,7 @@ func (a *ACL) AuthMethodSet(args *structs.ACLAuthMethodSetRequest, reply *struct
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.AuthMethod.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -2268,7 +2261,7 @@ func (a *ACL) AuthMethodDelete(args *structs.ACLAuthMethodDeleteRequest, reply *
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz == nil || authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if authz.ACLWrite(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -2322,7 +2315,7 @@ func (a *ACL) AuthMethodList(args *structs.ACLAuthMethodListRequest, reply *stru
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz == nil || authz.ACLRead(&authzContext) != acl.Allow {
+	} else if authz.ACLRead(&authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -2566,8 +2559,6 @@ func (a *ACL) Authorize(args *structs.RemoteACLAuthorizationRequest, reply *[]st
 	authz, err := a.srv.ResolveToken(args.Token)
 	if err != nil {
 		return err
-	} else if authz == nil {
-		return fmt.Errorf("Failed to initialize authorizer")
 	}
 
 	responses, err := structs.CreateACLAuthorizationResponses(authz, args.Requests)

agent/consul/acl_endpoint_legacy.go

@@ -169,7 +169,7 @@ func (a *ACL) Apply(args *structs.ACLRequest, reply *string) error {
 	// NOTE: We will not support enterprise authorizer contexts with legacy ACLs
 	if rule, err := a.srv.ResolveToken(args.Token); err != nil {
 		return err
-	} else if rule == nil || rule.ACLWrite(nil) != acl.Allow {
+	} else if rule.ACLWrite(nil) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -261,7 +261,7 @@ func (a *ACL) List(args *structs.DCSpecificRequest,
 	// and this check for ACLWrite is basically what it did before.
 	if rule, err := a.srv.ResolveToken(args.Token); err != nil {
 		return err
-	} else if rule == nil || rule.ACLWrite(nil) != acl.Allow {
+	} else if rule.ACLWrite(nil) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 

agent/consul/internal_endpoint.go

@@ -452,23 +452,21 @@ func (m *Internal) KeyringOperation(
 	if err := m.srv.validateEnterpriseToken(identity); err != nil {
 		return err
 	}
-	if rule != nil {
+	switch args.Operation {
-		switch args.Operation {
+	case structs.KeyringList:
-		case structs.KeyringList:
+		if rule.KeyringRead(nil) != acl.Allow {
-			if rule.KeyringRead(nil) != acl.Allow {
+			return fmt.Errorf("Reading keyring denied by ACLs")
-				return fmt.Errorf("Reading keyring denied by ACLs")
-			}
-		case structs.KeyringInstall:
-			fallthrough
-		case structs.KeyringUse:
-			fallthrough
-		case structs.KeyringRemove:
-			if rule.KeyringWrite(nil) != acl.Allow {
-				return fmt.Errorf("Modifying keyring denied due to ACLs")
-			}
-		default:
-			panic("Invalid keyring operation")
 		}
+	case structs.KeyringInstall:
+		fallthrough
+	case structs.KeyringUse:
+		fallthrough
+	case structs.KeyringRemove:
+		if rule.KeyringWrite(nil) != acl.Allow {
+			return fmt.Errorf("Modifying keyring denied due to ACLs")
+		}
+	default:
+		panic("Invalid keyring operation")
 	}
 
 	if args.LocalOnly || args.Forwarded || m.srv.serfWAN == nil {

agent/consul/kvs_endpoint.go

@@ -157,7 +157,7 @@ func (k *KVS) Get(args *structs.KeyRequest, reply *structs.IndexedDirEntries) er
 			if err != nil {
 				return err
 			}
-			if authz != nil && authz.KeyRead(args.Key, &authzContext) != acl.Allow {
+			if authz.KeyRead(args.Key, &authzContext) != acl.Allow {
 				return acl.ErrPermissionDenied
 			}
 
@@ -194,7 +194,7 @@ func (k *KVS) List(args *structs.KeyRequest, reply *structs.IndexedDirEntries) e
 		return err
 	}
 
-	if authz != nil && k.srv.config.ACLEnableKeyListPolicy && authz.KeyList(args.Key, &authzContext) != acl.Allow {
+	if k.srv.config.ACLEnableKeyListPolicy && authz.KeyList(args.Key, &authzContext) != acl.Allow {
 		return acl.ErrPermissionDenied
 	}
 
@@ -206,9 +206,7 @@ func (k *KVS) List(args *structs.KeyRequest, reply *structs.IndexedDirEntries) e
 			if err != nil {
 				return err
 			}
-			if authz != nil {
+			ent = FilterDirEnt(authz, ent)
-				ent = FilterDirEnt(authz, ent)
-			}
 
 			if len(ent) == 0 {
 				// Must provide non-zero index to prevent blocking

agent/rpc/subscribe/subscribe.go

@@ -75,9 +75,7 @@ func (h *Server) Subscribe(req *pbsubscribe.SubscribeRequest, serverStream pbsub
 			return err
 		}
 
-		var ok bool
+		if !event.Payload.HasReadPermission(authz) {
-		event, ok = filterByAuth(authz, event)
-		if !ok {
 			continue
 		}
 
@@ -126,16 +124,6 @@ func forwardToDC(
 	}
 }
 
-// filterByAuth to only those Events allowed by the acl token.
-func filterByAuth(authz acl.Authorizer, event stream.Event) (stream.Event, bool) {
-	// authz will be nil when ACLs are disabled
-	if authz == nil {
-		return event, true
-	}
-
-	return event, event.Payload.HasReadPermission(authz)
-}
-
 func newEventFromStreamEvent(event stream.Event) *pbsubscribe.Event {
 	e := &pbsubscribe.Event{Index: event.Index}
 	switch {

agent/structs/intention.go

@@ -298,9 +298,6 @@ func (x *Intention) Validate() error {
 }
 
 func (ixn *Intention) CanRead(authz acl.Authorizer) bool {
-	if authz == nil {
-		return true
-	}
 	var authzContext acl.AuthorizerContext
 
 	// Read access on either end of the intention allows you to read the
@@ -325,11 +322,12 @@ func (ixn *Intention) CanRead(authz acl.Authorizer) bool {
 }
 
 func (ixn *Intention) CanWrite(authz acl.Authorizer) bool {
-	if authz == nil || authz == acl.ManageAll() {
+	if authz == acl.ManageAll() {
 		return true
 	}
 	var authzContext acl.AuthorizerContext
 
+	// TODO: this line seems to require checking 'authz == acl.ManageAll()' above
 	if ixn.DestinationName == "" {
 		return false
 	}

command/acl/bindingrule/read/bindingrule_read.go

@@ -5,10 +5,11 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/mitchellh/cli"
+
 	"github.com/hashicorp/consul/command/acl"
 	"github.com/hashicorp/consul/command/acl/bindingrule"
 	"github.com/hashicorp/consul/command/flags"
-	"github.com/mitchellh/cli"
 )
 
 func New(ui cli.Ui) *cmd {
@@ -85,10 +86,11 @@ func (c *cmd) Run(args []string) int {
 	}
 
 	rule, _, err := client.ACL().BindingRuleRead(ruleID, nil)
-	if err != nil {
+	switch {
+	case err != nil:
 		c.UI.Error(fmt.Sprintf("Error reading binding rule %q: %v", ruleID, err))
 		return 1
-	} else if rule == nil {
+	case rule == nil:
 		c.UI.Error(fmt.Sprintf("Binding rule not found with ID %q", ruleID))
 		return 1
 	}