catalog: support `ResultsFilteredByACLs` flag/header (#11594)

agent/consul/acl.go

@@ -1242,25 +1242,32 @@ func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) bool {
 	return removed
 }
 
-// filterServices is used to filter a set of services based on ACLs.
+// filterServices is used to filter a set of services based on ACLs. Returns
-func (f *aclFilter) filterServices(services structs.Services, entMeta *structs.EnterpriseMeta) {
+// true if any elements were removed.
+func (f *aclFilter) filterServices(services structs.Services, entMeta *structs.EnterpriseMeta) bool {
 	var authzContext acl.AuthorizerContext
 	entMeta.FillAuthzContext(&authzContext)
 
+	var removed bool
+
 	for svc := range services {
 		if f.allowService(svc, &authzContext) {
 			continue
 		}
 		f.logger.Debug("dropping service from result due to ACLs", "service", svc)
+		removed = true
 		delete(services, svc)
 	}
+
+	return removed
 }
 
 // filterServiceNodes is used to filter a set of nodes for a given service
-// based on the configured ACL rules.
+// based on the configured ACL rules. Returns true if any elements were removed.
-func (f *aclFilter) filterServiceNodes(nodes *structs.ServiceNodes) {
+func (f *aclFilter) filterServiceNodes(nodes *structs.ServiceNodes) bool {
 	sn := *nodes
 	var authzContext acl.AuthorizerContext
+	var removed bool
 
 	for i := 0; i < len(sn); i++ {
 		node := sn[i]
@@ -1269,26 +1276,30 @@ func (f *aclFilter) filterServiceNodes(nodes *structs.ServiceNodes) {
 		if f.allowNode(node.Node, &authzContext) && f.allowService(node.ServiceName, &authzContext) {
 			continue
 		}
+		removed = true
 		f.logger.Debug("dropping node from result due to ACLs", "node", structs.NodeNameString(node.Node, &node.EnterpriseMeta))
 		sn = append(sn[:i], sn[i+1:]...)
 		i--
 	}
 	*nodes = sn
+	return removed
 }
 
 // filterNodeServices is used to filter services on a given node base on ACLs.
-func (f *aclFilter) filterNodeServices(services **structs.NodeServices) {
+// Returns true if any elements were removed
+func (f *aclFilter) filterNodeServices(services **structs.NodeServices) bool {
 	if *services == nil {
-		return
+		return false
 	}
 
 	var authzContext acl.AuthorizerContext
 	(*services).Node.FillAuthzContext(&authzContext)
 	if !f.allowNode((*services).Node.Node, &authzContext) {
 		*services = nil
-		return
+		return true
 	}
 
+	var removed bool
 	for svcName, svc := range (*services).Services {
 		svc.FillAuthzContext(&authzContext)
 
@@ -1296,44 +1307,45 @@ func (f *aclFilter) filterNodeServices(services **structs.NodeServices) {
 			continue
 		}
 		f.logger.Debug("dropping service from result due to ACLs", "service", svc.CompoundServiceID())
+		removed = true
 		delete((*services).Services, svcName)
 	}
+
+	return removed
 }
 
 // filterNodeServices is used to filter services on a given node base on ACLs.
-func (f *aclFilter) filterNodeServiceList(services **structs.NodeServiceList) {
+// Returns true if any elements were removed.
-	if services == nil || *services == nil {
+func (f *aclFilter) filterNodeServiceList(services *structs.NodeServiceList) bool {
-		return
+	if services.Node == nil {
+		return false
 	}
 
 	var authzContext acl.AuthorizerContext
-	(*services).Node.FillAuthzContext(&authzContext)
+	services.Node.FillAuthzContext(&authzContext)
-	if !f.allowNode((*services).Node.Node, &authzContext) {
+	if !f.allowNode(services.Node.Node, &authzContext) {
-		*services = nil
+		*services = structs.NodeServiceList{}
-		return
+		return true
 	}
 
-	svcs := (*services).Services
+	var removed bool
-	modified := false
+	svcs := services.Services
 	for i := 0; i < len(svcs); i++ {
 		svc := svcs[i]
 		svc.FillAuthzContext(&authzContext)
 
-		if f.allowNode((*services).Node.Node, &authzContext) && f.allowService(svc.Service, &authzContext) {
+		if f.allowService(svc.Service, &authzContext) {
 			continue
 		}
+
 		f.logger.Debug("dropping service from result due to ACLs", "service", svc.CompoundServiceID())
 		svcs = append(svcs[:i], svcs[i+1:]...)
 		i--
-		modified = true
+		removed = true
 	}
+	services.Services = svcs
 
-	if modified {
+	return removed
-		*services = &structs.NodeServiceList{
-			Node:     (*services).Node,
-			Services: svcs,
-		}
-	}
 }
 
 // filterCheckServiceNodes is used to filter nodes based on ACL rules. Returns
@@ -1520,11 +1532,13 @@ func (f *aclFilter) filterServiceDump(services *structs.ServiceDump) {
 }
 
 // filterNodes is used to filter through all parts of a node list and remove
-// elements the provided ACL token cannot access.
+// elements the provided ACL token cannot access. Returns true if any elements
-func (f *aclFilter) filterNodes(nodes *structs.Nodes) {
+// were removed.
+func (f *aclFilter) filterNodes(nodes *structs.Nodes) bool {
 	n := *nodes
 
 	var authzContext acl.AuthorizerContext
+	var removed bool
 
 	for i := 0; i < len(n); i++ {
 		n[i].FillAuthzContext(&authzContext)
@@ -1533,10 +1547,12 @@ func (f *aclFilter) filterNodes(nodes *structs.Nodes) {
 			continue
 		}
 		f.logger.Debug("dropping node from result due to ACLs", "node", structs.NodeNameString(node, n[i].GetEnterpriseMeta()))
+		removed = true
 		n = append(n[:i], n[i+1:]...)
 		i--
 	}
 	*nodes = n
+	return removed
 }
 
 // redactPreparedQueryTokens will redact any tokens unless the client has a
@@ -1769,14 +1785,16 @@ func (f *aclFilter) filterAuthMethods(methods *structs.ACLAuthMethods) {
 	*methods = ret
 }
 
-func (f *aclFilter) filterServiceList(services *structs.ServiceList) {
+func (f *aclFilter) filterServiceList(services *structs.ServiceList) bool {
 	ret := make(structs.ServiceList, 0, len(*services))
+	var removed bool
 	for _, svc := range *services {
 		var authzContext acl.AuthorizerContext
 
 		svc.FillAuthzContext(&authzContext)
 
 		if f.authorizer.ServiceRead(svc.Name, &authzContext) != acl.Allow {
+			removed = true
 			sid := structs.NewServiceID(svc.Name, &svc.EnterpriseMeta)
 			f.logger.Debug("dropping service from result due to ACLs", "service", sid.String())
 			continue
@@ -1786,10 +1804,13 @@ func (f *aclFilter) filterServiceList(services *structs.ServiceList) {
 	}
 
 	*services = ret
+	return removed
 }
 
 // filterGatewayServices is used to filter gateway to service mappings based on ACL rules.
-func (f *aclFilter) filterGatewayServices(mappings *structs.GatewayServices) {
+// Returns true if any elements were removed.
+func (f *aclFilter) filterGatewayServices(mappings *structs.GatewayServices) bool {
+	var removed bool
 	ret := make(structs.GatewayServices, 0, len(*mappings))
 	for _, s := range *mappings {
 		// This filter only checks ServiceRead on the linked service.
@@ -1799,11 +1820,13 @@ func (f *aclFilter) filterGatewayServices(mappings *structs.GatewayServices) {
 
 		if f.authorizer.ServiceRead(s.Service.Name, &authzContext) != acl.Allow {
 			f.logger.Debug("dropping service from result due to ACLs", "service", s.Service.String())
+			removed = true
 			continue
 		}
 		ret = append(ret, s)
 	}
 	*mappings = ret
+	return removed
 }
 
 func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, subj interface{}) {
@@ -1845,19 +1868,19 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
 		filt.filterServiceDump(&v.Dump)
 
 	case *structs.IndexedNodes:
-		filt.filterNodes(&v.Nodes)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterNodes(&v.Nodes)
 
 	case *structs.IndexedNodeServices:
-		filt.filterNodeServices(&v.NodeServices)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterNodeServices(&v.NodeServices)
 
-	case **structs.NodeServiceList:
+	case *structs.IndexedNodeServiceList:
-		filt.filterNodeServiceList(v)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterNodeServiceList(&v.NodeServices)
 
 	case *structs.IndexedServiceNodes:
-		filt.filterServiceNodes(&v.ServiceNodes)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterServiceNodes(&v.ServiceNodes)
 
 	case *structs.IndexedServices:
-		filt.filterServices(v.Services, &v.EnterpriseMeta)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterServices(v.Services, &v.EnterpriseMeta)
 
 	case *structs.IndexedSessions:
 		v.QueryMeta.ResultsFilteredByACLs = filt.filterSessions(&v.Sessions)
@@ -1898,10 +1921,18 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
 		filt.filterAuthMethod(v)
 
 	case *structs.IndexedServiceList:
-		filt.filterServiceList(&v.Services)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterServiceList(&v.Services)
 
-	case *structs.GatewayServices:
+	case *structs.IndexedGatewayServices:
-		filt.filterGatewayServices(v)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterGatewayServices(&v.Services)
+
+	case *structs.IndexedNodesWithGateways:
+		if filt.filterCheckServiceNodes(&v.Nodes) {
+			v.QueryMeta.ResultsFilteredByACLs = true
+		}
+		if filt.filterGatewayServices(&v.Gateways) {
+			v.QueryMeta.ResultsFilteredByACLs = true
+		}
 
 	default:
 		panic(fmt.Errorf("Unhandled type passed to ACL filter: %T %#v", subj, subj))

agent/consul/acl_test.go

@@ -2304,6 +2304,9 @@ func TestACL_filterIntentions(t *testing.T) {
 
 func TestACL_filterServices(t *testing.T) {
 	t.Parallel()
+
+	require := require.New(t)
+
 	// Create some services
 	services := structs.Services{
 		"service1": []string{},
@@ -2313,194 +2316,339 @@ func TestACL_filterServices(t *testing.T) {
 
 	// Try permissive filtering.
 	filt := newACLFilter(acl.AllowAll(), nil)
-	filt.filterServices(services, nil)
+	removed := filt.filterServices(services, nil)
-	if len(services) != 3 {
+	require.False(removed)
-		t.Fatalf("bad: %#v", services)
+	require.Len(services, 3)
-	}
 
 	// Try restrictive filtering.
 	filt = newACLFilter(acl.DenyAll(), nil)
-	filt.filterServices(services, nil)
+	removed = filt.filterServices(services, nil)
-	if len(services) != 0 {
+	require.True(removed)
-		t.Fatalf("bad: %#v", services)
+	require.Empty(services)
-	}
 }
 
 func TestACL_filterServiceNodes(t *testing.T) {
 	t.Parallel()
-	// Create some service nodes.
-	fill := func() structs.ServiceNodes {
-		return structs.ServiceNodes{
-			&structs.ServiceNode{
-				Node:        "node1",
-				ServiceName: "foo",
-			},
-		}
-	}
 
-	// Try permissive filtering.
+	logger := hclog.NewNullLogger()
-	{
-		nodes := fill()
-		filt := newACLFilter(acl.AllowAll(), nil)
-		filt.filterServiceNodes(&nodes)
-		if len(nodes) != 1 {
-			t.Fatalf("bad: %#v", nodes)
-		}
-	}
 
-	// Try restrictive filtering.
+	makeList := func() *structs.IndexedServiceNodes {
-	{
+		return &structs.IndexedServiceNodes{
-		nodes := fill()
+			ServiceNodes: structs.ServiceNodes{
-		filt := newACLFilter(acl.DenyAll(), nil)
+				{
-		filt.filterServiceNodes(&nodes)
+					Node:        "node1",
-		if len(nodes) != 0 {
+					ServiceName: "foo",
-			t.Fatalf("bad: %#v", nodes)
-		}
-	}
-
-	// Allowed to see the service but not the node.
-	policy, err := acl.NewPolicyFromSource(`
-service "foo" {
-  policy = "read"
-}
-`, acl.SyntaxLegacy, nil, nil)
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	perms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// But with version 8 the node will block it.
-	{
-		nodes := fill()
-		filt := newACLFilter(perms, nil)
-		filt.filterServiceNodes(&nodes)
-		if len(nodes) != 0 {
-			t.Fatalf("bad: %#v", nodes)
-		}
-	}
-
-	// Chain on access to the node.
-	policy, err = acl.NewPolicyFromSource(`
-node "node1" {
-  policy = "read"
-}
-`, acl.SyntaxLegacy, nil, nil)
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// Now it should go through.
-	{
-		nodes := fill()
-		filt := newACLFilter(perms, nil)
-		filt.filterServiceNodes(&nodes)
-		if len(nodes) != 1 {
-			t.Fatalf("bad: %#v", nodes)
-		}
-	}
-}
-
-func TestACL_filterNodeServices(t *testing.T) {
-	t.Parallel()
-	// Create some node services.
-	fill := func() *structs.NodeServices {
-		return &structs.NodeServices{
-			Node: &structs.Node{
-				Node: "node1",
-			},
-			Services: map[string]*structs.NodeService{
-				"foo": {
-					ID:      "foo",
-					Service: "foo",
 				},
 			},
 		}
 	}
 
-	// Try nil, which is a possible input.
+	t.Run("allowed", func(t *testing.T) {
-	{
+		require := require.New(t)
-		var services *structs.NodeServices
-		filt := newACLFilter(acl.AllowAll(), nil)
-		filt.filterNodeServices(&services)
-		if services != nil {
-			t.Fatalf("bad: %#v", services)
-		}
-	}
 
-	// Try permissive filtering.
+		policy, err := acl.NewPolicyFromSource(`
-	{
+			service "foo" {
-		services := fill()
+			  policy = "read"
-		filt := newACLFilter(acl.AllowAll(), nil)
+			}
-		filt.filterNodeServices(&services)
+			node "node1" {
-		if len(services.Services) != 1 {
+			  policy = "read"
-			t.Fatalf("bad: %#v", services.Services)
+			}
-		}
+		`, acl.SyntaxLegacy, nil, nil)
-	}
+		require.NoError(err)
 
-	// Try restrictive filtering.
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
-	{
+		require.NoError(err)
-		services := fill()
-		filt := newACLFilter(acl.DenyAll(), nil)
-		filt.filterNodeServices(&services)
-		if services != nil {
-			t.Fatalf("bad: %#v", *services)
-		}
-	}
 
-	// Allowed to see the service but not the node.
+		list := makeList()
-	policy, err := acl.NewPolicyFromSource(`
+		filterACLWithAuthorizer(logger, authz, list)
-service "foo" {
+
-  policy = "read"
+		require.Len(list.ServiceNodes, 1)
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	t.Run("allowed to read the service, but not the node", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			service "foo" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Empty(list.ServiceNodes)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("denied", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.DenyAll(), list)
+
+		require.Empty(list.ServiceNodes)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
 }
-`, acl.SyntaxLegacy, nil, nil)
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	perms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
 
-	// Node will block it.
+func TestACL_filterNodeServices(t *testing.T) {
-	{
+	t.Parallel()
-		services := fill()
+
-		filt := newACLFilter(perms, nil)
+	logger := hclog.NewNullLogger()
-		filt.filterNodeServices(&services)
+
-		if services != nil {
+	makeList := func() *structs.IndexedNodeServices {
-			t.Fatalf("bad: %#v", services)
+		return &structs.IndexedNodeServices{
+			NodeServices: &structs.NodeServices{
+				Node: &structs.Node{
+					Node: "node1",
+				},
+				Services: map[string]*structs.NodeService{
+					"foo": {
+						ID:      "foo",
+						Service: "foo",
+					},
+				},
+			},
 		}
 	}
 
-	// Chain on access to the node.
+	t.Run("nil input", func(t *testing.T) {
-	policy, err = acl.NewPolicyFromSource(`
+		require := require.New(t)
-node "node1" {
+
-  policy = "read"
+		list := &structs.IndexedNodeServices{
+			NodeServices: nil,
+		}
+		filterACLWithAuthorizer(logger, acl.AllowAll(), list)
+
+		require.Nil(list.NodeServices)
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	t.Run("allowed", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			service "foo" {
+			  policy = "read"
+			}
+			node "node1" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Len(list.NodeServices.Services, 1)
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	t.Run("allowed to read the service, but not the node", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			service "foo" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Nil(list.NodeServices)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("allowed to read the node, but not the service", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			node "node1" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Empty(list.NodeServices.Services)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("denied", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.DenyAll(), list)
+
+		require.Nil(list.NodeServices)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
 }
-`, acl.SyntaxLegacy, nil, nil)
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
 
-	// Now it should go through.
+func TestACL_filterNodeServiceList(t *testing.T) {
-	{
+	t.Parallel()
-		services := fill()
+
-		filt := newACLFilter(perms, nil)
+	logger := hclog.NewNullLogger()
-		filt.filterNodeServices(&services)
+
-		if len((*services).Services) != 1 {
+	makeList := func() *structs.IndexedNodeServiceList {
-			t.Fatalf("bad: %#v", (*services).Services)
+		return &structs.IndexedNodeServiceList{
+			NodeServices: structs.NodeServiceList{
+				Node: &structs.Node{
+					Node: "node1",
+				},
+				Services: []*structs.NodeService{
+					{Service: "foo"},
+				},
+			},
 		}
 	}
+
+	t.Run("empty NodeServices", func(t *testing.T) {
+		require := require.New(t)
+
+		var list structs.IndexedNodeServiceList
+		filterACLWithAuthorizer(logger, acl.AllowAll(), &list)
+
+		require.Empty(list)
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	t.Run("allowed", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			service "foo" {
+			  policy = "read"
+			}
+			node "node1" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Len(list.NodeServices.Services, 1)
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	t.Run("allowed to read the service, but not the node", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			service "foo" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Empty(list.NodeServices)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("allowed to read the node, but not the service", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			node "node1" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.NotEmpty(list.NodeServices.Node)
+		require.Empty(list.NodeServices.Services)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("denied", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.DenyAll(), list)
+
+		require.Empty(list.NodeServices)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+}
+
+func TestACL_filterGatewayServices(t *testing.T) {
+	t.Parallel()
+
+	logger := hclog.NewNullLogger()
+
+	makeList := func() *structs.IndexedGatewayServices {
+		return &structs.IndexedGatewayServices{
+			Services: structs.GatewayServices{
+				{Service: structs.ServiceName{Name: "foo"}},
+			},
+		}
+	}
+
+	t.Run("allowed", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			service "foo" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Len(list.Services, 1)
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	t.Run("denied", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.DenyAll(), list)
+
+		require.Empty(list.Services)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
 }
 
 func TestACL_filterCheckServiceNodes(t *testing.T) {
@@ -2982,6 +3130,9 @@ node "node1" {
 
 func TestACL_filterNodes(t *testing.T) {
 	t.Parallel()
+
+	require := require.New(t)
+
 	// Create a nodes list.
 	nodes := structs.Nodes{
 		&structs.Node{
@@ -2994,17 +3145,15 @@ func TestACL_filterNodes(t *testing.T) {
 
 	// Try permissive filtering.
 	filt := newACLFilter(acl.AllowAll(), nil)
-	filt.filterNodes(&nodes)
+	removed := filt.filterNodes(&nodes)
-	if len(nodes) != 2 {
+	require.False(removed)
-		t.Fatalf("bad: %#v", nodes)
+	require.Len(nodes, 2)
-	}
 
 	// Try restrictive filtering
 	filt = newACLFilter(acl.DenyAll(), nil)
-	filt.filterNodes(&nodes)
+	removed = filt.filterNodes(&nodes)
-	if len(nodes) != 0 {
+	require.True(removed)
-		t.Fatalf("bad: %#v", nodes)
+	require.Len(nodes, 0)
-	}
 }
 
 func TestACL_filterDatacenterCheckServiceNodes(t *testing.T) {
@@ -3268,6 +3417,39 @@ func TestACL_filterPreparedQueries(t *testing.T) {
 	}
 }
 
+func TestACL_filterServiceList(t *testing.T) {
+	logger := hclog.NewNullLogger()
+
+	makeList := func() *structs.IndexedServiceList {
+		return &structs.IndexedServiceList{
+			Services: structs.ServiceList{
+				{Name: "foo"},
+				{Name: "bar"},
+			},
+		}
+	}
+
+	t.Run("permissive filtering", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.AllowAll(), list)
+
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+		require.Len(list.Services, 2)
+	})
+
+	t.Run("restrictive filtering", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.DenyAll(), list)
+
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+		require.Empty(list.Services)
+	})
+}
+
 func TestACL_unhandledFilterType(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")

agent/consul/catalog_endpoint.go

@@ -488,16 +488,19 @@ func (c *Catalog) ListNodes(args *structs.DCSpecificRequest, reply *structs.Inde
 				return nil
 			}
 
-			if err := c.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
-
 			raw, err := filter.Execute(reply.Nodes)
 			if err != nil {
 				return err
 			}
 			reply.Nodes = raw.(structs.Nodes)
 
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return c.srv.sortNodesByDistanceFrom(args.Source, reply.Nodes)
 		})
 }
@@ -664,18 +667,20 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
 				reply.ServiceNodes = filtered
 			}
 
-			if err := c.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
-
 			// This is safe to do even when the filter is nil - its just a no-op then
 			raw, err := filter.Execute(reply.ServiceNodes)
 			if err != nil {
 				return err
 			}
-
 			reply.ServiceNodes = raw.(structs.ServiceNodes)
 
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return c.srv.sortNodesByDistanceFrom(args.Source, reply.ServiceNodes)
 		})
 
@@ -750,11 +755,7 @@ func (c *Catalog) NodeServices(args *structs.NodeSpecificRequest, reply *structs
 			if err != nil {
 				return err
 			}
-
 			reply.Index, reply.NodeServices = index, services
-			if err := c.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
 
 			if reply.NodeServices != nil {
 				raw, err := filter.Execute(reply.NodeServices.Services)
@@ -764,6 +765,13 @@ func (c *Catalog) NodeServices(args *structs.NodeSpecificRequest, reply *structs
 				reply.NodeServices.Services = raw.(map[string]*structs.NodeService)
 			}
 
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return nil
 		})
 }
@@ -802,11 +810,8 @@ func (c *Catalog) NodeServiceList(args *structs.NodeSpecificRequest, reply *stru
 				return err
 			}
 
-			if err := c.srv.filterACL(args.Token, &services); err != nil {
-				return err
-			}
-
 			reply.Index = index
+
 			if services != nil {
 				reply.NodeServices = *services
 
@@ -817,6 +822,13 @@ func (c *Catalog) NodeServiceList(args *structs.NodeSpecificRequest, reply *stru
 				reply.NodeServices.Services = raw.([]*structs.NodeService)
 			}
 
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return nil
 		})
 }
@@ -876,12 +888,11 @@ func (c *Catalog) GatewayServices(args *structs.ServiceSpecificRequest, reply *s
 			if err != nil {
 				return err
 			}
+			reply.Index, reply.Services = index, services
 
-			if err := c.srv.filterACL(args.Token, &services); err != nil {
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
-
-			reply.Index, reply.Services = index, services
 			return nil
 		})
 }

agent/consul/catalog_endpoint_test.go

@@ -1307,42 +1307,48 @@ func TestCatalog_ListNodes_ACLFilter(t *testing.T) {
 
 	testrpc.WaitForLeader(t, s1.RPC, "dc1")
 
-	// We scope the reply in each of these since msgpack won't clear out an
+	token := func(policy string) string {
-	// existing slice if the incoming one is nil, so it's best to start
+		rules := fmt.Sprintf(
-	// clean each time.
+			`node "%s" { policy = "%s" }`,
+			s1.config.NodeName,
+			policy,
+		)
+		return createTokenWithPolicyName(t, codec, policy, rules, "root")
+	}
 
-	// The node policy should not be ignored.
 	args := structs.DCSpecificRequest{
 		Datacenter: "dc1",
 	}
-	{
+
-		reply := structs.IndexedNodes{}
+	t.Run("deny", func(t *testing.T) {
+		args.Token = token("deny")
+
+		var reply structs.IndexedNodes
 		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply); err != nil {
 			t.Fatalf("err: %v", err)
 		}
 		if len(reply.Nodes) != 0 {
 			t.Fatalf("bad: %v", reply.Nodes)
 		}
-	}
+		if !reply.QueryMeta.ResultsFilteredByACLs {
+			t.Fatal("ResultsFilteredByACLs should be true")
+		}
+	})
 
-	rules := fmt.Sprintf(`
+	t.Run("allow", func(t *testing.T) {
-node "%s" {
+		args.Token = token("read")
-	policy = "read"
-}
-`, s1.config.NodeName)
-	id := createToken(t, codec, rules)
 
-	// Now try with the token and it will go through.
+		var reply structs.IndexedNodes
-	args.Token = id
-	{
-		reply := structs.IndexedNodes{}
 		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply); err != nil {
 			t.Fatalf("err: %v", err)
 		}
 		if len(reply.Nodes) != 1 {
 			t.Fatalf("bad: %v", reply.Nodes)
 		}
-	}
+		if reply.QueryMeta.ResultsFilteredByACLs {
+			t.Fatal("ResultsFilteredByACLs should not true")
+		}
+	})
 }
 
 func Benchmark_Catalog_ListNodes(t *testing.B) {
@@ -1422,6 +1428,7 @@ func TestCatalog_ListServices(t *testing.T) {
 		t.Fatalf("bad: %v", out)
 	}
 	require.False(t, out.QueryMeta.NotModified)
+	require.False(t, out.QueryMeta.ResultsFilteredByACLs)
 
 	t.Run("with option AllowNotModifiedResponse", func(t *testing.T) {
 		args.QueryOptions = structs.QueryOptions{
@@ -2474,6 +2481,7 @@ node "foo" {
 	require.Len(t, resp.ServiceNodes, 1)
 	v := resp.ServiceNodes[0]
 	require.Equal(t, "foo-proxy", v.ServiceName)
+	require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 }
 
 func TestCatalog_ListServiceNodes_ConnectNative(t *testing.T) {
@@ -2778,6 +2786,9 @@ func TestCatalog_ListServices_FilterACL(t *testing.T) {
 	if _, ok := reply.Services["bar"]; ok {
 		t.Fatalf("bad: %#v", reply.Services)
 	}
+	if !reply.QueryMeta.ResultsFilteredByACLs {
+		t.Fatal("ResultsFilteredByACLs should be true")
+	}
 }
 
 func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
@@ -2826,6 +2837,7 @@ func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
 			t.Fatalf("bad: %#v", reply.ServiceNodes)
 		}
 	}
+	require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 
 	// We've already proven that we call the ACL filtering function so we
 	// test node filtering down in acl.go for node cases. This also proves
@@ -2834,7 +2846,7 @@ func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
 	// for now until we change the sense of the version 8 ACL flag).
 }
 
-func TestCatalog_NodeServices_ACLDeny(t *testing.T) {
+func TestCatalog_NodeServices_ACL(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")
 	}
@@ -2853,43 +2865,46 @@ func TestCatalog_NodeServices_ACLDeny(t *testing.T) {
 
 	testrpc.WaitForTestAgent(t, s1.RPC, "dc1", testrpc.WithToken("root"))
 
-	// The node policy should not be ignored.
+	token := func(policy string) string {
+		rules := fmt.Sprintf(`
+				node "%s" { policy = "%s" }
+				service "consul" { policy = "%s" }
+			`,
+			s1.config.NodeName,
+			policy,
+			policy,
+		)
+		return createTokenWithPolicyName(t, codec, policy, rules, "root")
+	}
+
 	args := structs.NodeSpecificRequest{
 		Datacenter: "dc1",
 		Node:       s1.config.NodeName,
 	}
-	reply := structs.IndexedNodeServices{}
-	if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if reply.NodeServices != nil {
-		t.Fatalf("should not nil")
-	}
 
-	rules := fmt.Sprintf(`
+	t.Run("deny", func(t *testing.T) {
-node "%s" {
+		require := require.New(t)
-	policy = "read"
-}
-`, s1.config.NodeName)
-	id := createToken(t, codec, rules)
 
-	// Now try with the token and it will go through.
+		args.Token = token("deny")
-	args.Token = id
-	if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if reply.NodeServices == nil {
-		t.Fatalf("should not be nil")
-	}
 
-	// Make sure an unknown node doesn't cause trouble.
+		var reply structs.IndexedNodeServices
-	args.Node = "nope"
+		err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply)
-	if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply); err != nil {
+		require.NoError(err)
-		t.Fatalf("err: %v", err)
+		require.Nil(reply.NodeServices)
-	}
+		require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
-	if reply.NodeServices != nil {
+	})
-		t.Fatalf("should not nil")
+
-	}
+	t.Run("allow", func(t *testing.T) {
+		require := require.New(t)
+
+		args.Token = token("read")
+
+		var reply structs.IndexedNodeServices
+		err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply)
+		require.NoError(err)
+		require.NotNil(reply.NodeServices)
+		require.False(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
 }
 
 func TestCatalog_NodeServices_FilterACL(t *testing.T) {
@@ -3380,6 +3395,7 @@ service "gateway" {
 		var resp structs.IndexedGatewayServices
 		assert.Nil(r, msgpackrpc.CallWithCodec(codec, "Catalog.GatewayServices", &req, &resp))
 		assert.Len(r, resp.Services, 0)
+		assert.True(r, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 	})
 
 	rules = `
@@ -3403,6 +3419,7 @@ service "gateway" {
 		var resp structs.IndexedGatewayServices
 		assert.Nil(r, msgpackrpc.CallWithCodec(codec, "Catalog.GatewayServices", &req, &resp))
 		assert.Len(r, resp.Services, 2)
+		assert.True(r, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 
 		expect := structs.GatewayServices{
 			{

agent/consul/internal_endpoint.go

@@ -130,16 +130,19 @@ func (m *Internal) ServiceDump(args *structs.ServiceDumpRequest, reply *structs.
 			}
 			reply.Index = maxIdx
 
-			if err := m.srv.filterACL(args.Token, &reply.Gateways); err != nil {
-				return err
-			}
-
 			raw, err := filter.Execute(reply.Nodes)
 			if err != nil {
 				return err
 			}
-
 			reply.Nodes = raw.(structs.CheckServiceNodes)
+
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := m.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return nil
 		})
 }