query: support ResultsFilteredByACLs in query list endpoint (#11620)

agent/consul/acl.go

@@ -1592,8 +1592,10 @@ func (f *aclFilter) redactPreparedQueryTokens(query **structs.PreparedQuery) {
 
 // filterPreparedQueries is used to filter prepared queries based on ACL rules.
 // We prune entries the user doesn't have access to, and we redact any tokens
-// if the user doesn't have a management token.
+// if the user doesn't have a management token. Returns true if any (named)
-func (f *aclFilter) filterPreparedQueries(queries *structs.PreparedQueries) {
+// queries were removed - un-named queries are meant to be ephemeral and can
+// only be enumerated by a management token
+func (f *aclFilter) filterPreparedQueries(queries *structs.PreparedQueries) bool {
 	var authzContext acl.AuthorizerContext
 	structs.DefaultEnterpriseMetaInDefaultPartition().FillAuthzContext(&authzContext)
 	// Management tokens can see everything with no filtering.
@@ -1601,17 +1603,22 @@ func (f *aclFilter) filterPreparedQueries(queries *structs.PreparedQueries) {
 	// the 1.4 ACL rewrite. The global-management token will provide unrestricted query privileges
 	// so asking for ACLWrite should be unnecessary.
 	if f.authorizer.ACLWrite(&authzContext) == acl.Allow {
-		return
+		return false
 	}
 
 	// Otherwise, we need to see what the token has access to.
+	var namedQueriesRemoved bool
 	ret := make(structs.PreparedQueries, 0, len(*queries))
 	for _, query := range *queries {
 		// If no prefix ACL applies to this query then filter it, since
 		// we know at this point the user doesn't have a management
 		// token, otherwise see what the policy says.
-		prefix, ok := query.GetACLPrefix()
+		prefix, hasName := query.GetACLPrefix()
-		if !ok || f.authorizer.PreparedQueryRead(prefix, &authzContext) != acl.Allow {
+		switch {
+		case hasName && f.authorizer.PreparedQueryRead(prefix, &authzContext) != acl.Allow:
+			namedQueriesRemoved = true
+			fallthrough
+		case !hasName:
 			f.logger.Debug("dropping prepared query from result due to ACLs", "query", query.ID)
 			continue
 		}
@@ -1623,6 +1630,7 @@ func (f *aclFilter) filterPreparedQueries(queries *structs.PreparedQueries) {
 		ret = append(ret, final)
 	}
 	*queries = ret
+	return namedQueriesRemoved
 }
 
 func (f *aclFilter) filterToken(token **structs.ACLToken) {
@@ -1847,6 +1855,9 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
 	case *structs.IndexedCheckServiceNodes:
 		v.QueryMeta.ResultsFilteredByACLs = filt.filterCheckServiceNodes(&v.Nodes)
 
+	case *structs.PreparedQueryExecuteResponse:
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterCheckServiceNodes(&v.Nodes)
+
 	case *structs.IndexedServiceTopology:
 		filtered := filt.filterServiceTopology(v.ServiceTopology)
 		if filtered {
@@ -1891,7 +1902,7 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
 		v.QueryMeta.ResultsFilteredByACLs = filt.filterSessions(&v.Sessions)
 
 	case *structs.IndexedPreparedQueries:
-		filt.filterPreparedQueries(&v.Queries)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterPreparedQueries(&v.Queries)
 
 	case **structs.PreparedQuery:
 		filt.redactPreparedQueryTokens(v)

agent/consul/acl_test.go

@@ -2752,6 +2752,108 @@ func TestACL_filterCheckServiceNodes(t *testing.T) {
 	})
 }
 
+func TestACL_filterPreparedQueryExecuteResponse(t *testing.T) {
+	t.Parallel()
+
+	logger := hclog.NewNullLogger()
+
+	makeList := func() *structs.PreparedQueryExecuteResponse {
+		return &structs.PreparedQueryExecuteResponse{
+			Nodes: structs.CheckServiceNodes{
+				{
+					Node: &structs.Node{
+						Node: "node1",
+					},
+					Service: &structs.NodeService{
+						ID:      "foo",
+						Service: "foo",
+					},
+					Checks: structs.HealthChecks{
+						{
+							Node:        "node1",
+							CheckID:     "check1",
+							ServiceName: "foo",
+						},
+					},
+				},
+			},
+		}
+	}
+
+	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.Nodes, 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.Nodes)
+		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.Nodes)
+		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.Nodes)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+}
+
 func TestACL_filterServiceTopology(t *testing.T) {
 	t.Parallel()
 	// Create some nodes.
@@ -3353,70 +3455,97 @@ func TestFilterACL_redactTokenSecrets(t *testing.T) {
 
 func TestACL_filterPreparedQueries(t *testing.T) {
 	t.Parallel()
-	queries := structs.PreparedQueries{
+
-		&structs.PreparedQuery{
+	logger := hclog.NewNullLogger()
-			ID: "f004177f-2c28-83b7-4229-eacc25fe55d1",
+
-		},
+	makeList := func() *structs.IndexedPreparedQueries {
-		&structs.PreparedQuery{
+		return &structs.IndexedPreparedQueries{
+			Queries: structs.PreparedQueries{
+				{ID: "f004177f-2c28-83b7-4229-eacc25fe55d1"},
+				{
 					ID:   "f004177f-2c28-83b7-4229-eacc25fe55d2",
 					Name: "query-with-no-token",
 				},
-		&structs.PreparedQuery{
+				{
 					ID:    "f004177f-2c28-83b7-4229-eacc25fe55d3",
 					Name:  "query-with-a-token",
 					Token: "root",
 				},
-	}
-
-	expected := structs.PreparedQueries{
-		&structs.PreparedQuery{
-			ID: "f004177f-2c28-83b7-4229-eacc25fe55d1",
-		},
-		&structs.PreparedQuery{
-			ID:   "f004177f-2c28-83b7-4229-eacc25fe55d2",
-			Name: "query-with-no-token",
-		},
-		&structs.PreparedQuery{
-			ID:    "f004177f-2c28-83b7-4229-eacc25fe55d3",
-			Name:  "query-with-a-token",
-			Token: "root",
 			},
 		}
-
-	// Try permissive filtering with a management token. This will allow the
-	// embedded token to be seen.
-	filt := newACLFilter(acl.ManageAll(), nil)
-	filt.filterPreparedQueries(&queries)
-	if !reflect.DeepEqual(queries, expected) {
-		t.Fatalf("bad: %#v", queries)
 	}
 
-	// Hang on to the entry with a token, which needs to survive the next
+	t.Run("management token", func(t *testing.T) {
-	// operation.
+		require := require.New(t)
-	original := queries[2]
 
-	// Now try permissive filtering with a client token, which should cause
+		list := makeList()
-	// the embedded token to get redacted, and the query with no name to get
+		filterACLWithAuthorizer(logger, acl.ManageAll(), list)
-	// filtered out.
-	filt = newACLFilter(acl.AllowAll(), nil)
-	filt.filterPreparedQueries(&queries)
-	expected[2].Token = redactedToken
-	expected = append(structs.PreparedQueries{}, expected[1], expected[2])
-	if !reflect.DeepEqual(queries, expected) {
-		t.Fatalf("bad: %#v", queries)
-	}
 
-	// Make sure that the original object didn't lose its token.
+		// Check we get the un-named query.
-	if original.Token != "root" {
+		require.Len(list.Queries, 3)
-		t.Fatalf("bad token: %s", original.Token)
-	}
 
-	// Now try restrictive filtering.
+		// Check we get the un-redacted token.
-	filt = newACLFilter(acl.DenyAll(), nil)
+		require.Equal("root", list.Queries[2].Token)
-	filt.filterPreparedQueries(&queries)
+
-	if len(queries) != 0 {
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
-		t.Fatalf("bad: %#v", queries)
+	})
+
+	t.Run("permissive filtering", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		queryWithToken := list.Queries[2]
+
+		filterACLWithAuthorizer(logger, acl.AllowAll(), list)
+
+		// Check the un-named query is filtered out.
+		require.Len(list.Queries, 2)
+
+		// Check the token is redacted.
+		require.Equal(redactedToken, list.Queries[1].Token)
+
+		// Check the original object is unmodified.
+		require.Equal("root", queryWithToken.Token)
+
+		// ResultsFilteredByACLs should not include un-named queries, which are only
+		// readable by a management token.
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	t.Run("limited access", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			query "query-with-a-token" {
+			  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)
+
+		// Check we only get the query we have access to.
+		require.Len(list.Queries, 1)
+
+		// Check the token is redacted.
+		require.Equal(redactedToken, list.Queries[0].Token)
+
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("restrictive filtering", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.DenyAll(), list)
+
+		require.Empty(list.Queries)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
 }
 
 func TestACL_filterServiceList(t *testing.T) {

agent/consul/prepared_query_endpoint.go

@@ -373,7 +373,7 @@ func (p *PreparedQuery) Execute(args *structs.PreparedQueryExecuteRequest,
 	if query.Token != "" {
 		token = query.Token
 	}
-	if err := p.srv.filterACL(token, &reply.Nodes); err != nil {
+	if err := p.srv.filterACL(token, reply); err != nil {
 		return err
 	}
 
@@ -500,7 +500,7 @@ func (p *PreparedQuery) ExecuteRemote(args *structs.PreparedQueryExecuteRemoteRe
 	if args.Query.Token != "" {
 		token = args.Query.Token
 	}
-	if err := p.srv.filterACL(token, &reply.Nodes); err != nil {
+	if err := p.srv.filterACL(token, reply); err != nil {
 		return err
 	}
 

agent/consul/prepared_query_endpoint_test.go

@@ -1167,6 +1167,31 @@ func TestPreparedQuery_List(t *testing.T) {
 		}
 	}
 
+	// Same for a token without access to the query.
+	{
+		token := createTokenWithPolicyName(t, codec, "deny-queries", `
+			query_prefix "" {
+				policy = "deny"
+			}
+		`, "root")
+
+		req := &structs.DCSpecificRequest{
+			Datacenter:   "dc1",
+			QueryOptions: structs.QueryOptions{Token: token},
+		}
+		var resp structs.IndexedPreparedQueries
+		if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.List", req, &resp); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		if len(resp.Queries) != 0 {
+			t.Fatalf("bad: %v", resp)
+		}
+		if !resp.QueryMeta.ResultsFilteredByACLs {
+			t.Fatal("ResultsFilteredByACLs should be true")
+		}
+	}
+
 	// But a management token should work, and be able to see the captured
 	// token.
 	query.Query.Token = "le-token"
@@ -2124,6 +2149,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 		require.NoError(t, msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply))
 
 		expectNodes(t, &query, &reply, 0)
+		require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 	})
 
 	t.Run("normal operation again with exec token", func(t *testing.T) {
@@ -2246,6 +2272,20 @@ func TestPreparedQuery_Execute(t *testing.T) {
 		expectFailoverNodes(t, &query, &reply, 0)
 	})
 
+	t.Run("nodes in response from dc2 are filtered by ACL token", func(t *testing.T) {
+		req := structs.PreparedQueryExecuteRequest{
+			Datacenter:    "dc1",
+			QueryIDOrName: query.Query.ID,
+			QueryOptions:  structs.QueryOptions{Token: execNoNodesToken},
+		}
+
+		var reply structs.PreparedQueryExecuteResponse
+		require.NoError(t, msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply))
+
+		expectFailoverNodes(t, &query, &reply, 0)
+		require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
 	// Bake the exec token into the query.
 	query.Query.Token = execToken
 	require.NoError(t, msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Apply", &query, &query.Query.ID))