diff --git a/agent/grpc-external/services/resource/list.go b/agent/grpc-external/services/resource/list.go index 803f64901b3..cc2b19026a1 100644 --- a/agent/grpc-external/services/resource/list.go +++ b/agent/grpc-external/services/resource/list.go @@ -10,29 +10,27 @@ import ( "google.golang.org/grpc/status" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/storage" "github.com/hashicorp/consul/proto-public/pbresource" ) func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbresource.ListResponse, error) { - if err := validateListRequest(req); err != nil { - return nil, err - } - - // check type - reg, err := s.resolveType(req.Type) + reg, err := s.validateListRequest(req) if err != nil { return nil, err } - // TODO(spatel): Refactor _ and entMeta in NET-4915 - authz, authzContext, err := s.getAuthorizer(tokenFromContext(ctx), acl.DefaultEnterpriseMeta()) + // v1 ACL subsystem is "wildcard" aware so just pass on through. + entMeta := v2TenancyToV1EntMeta(req.Tenancy) + token := tokenFromContext(ctx) + authz, authzContext, err := s.getAuthorizer(token, entMeta) if err != nil { return nil, err } - // check acls - err = reg.ACLs.List(authz, req.Tenancy) + // Check ACLs. + err = reg.ACLs.List(authz, authzContext) switch { case acl.IsErrPermissionDenied(err): return nil, status.Error(codes.PermissionDenied, err.Error()) @@ -40,6 +38,9 @@ func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbreso return nil, status.Errorf(codes.Internal, "failed list acl: %v", err) } + // Ensure we're defaulting correctly when request tenancy units are empty. + v1EntMetaToV2Tenancy(reg, entMeta, req.Tenancy) + resources, err := s.Backend.List( ctx, readConsistencyFrom(ctx), @@ -53,12 +54,21 @@ func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbreso result := make([]*pbresource.Resource, 0) for _, resource := range resources { - // filter out non-matching GroupVersion + // Filter out non-matching GroupVersion. if resource.Id.Type.GroupVersion != req.Type.GroupVersion { continue } - // filter out items that don't pass read ACLs + // Need to rebuild authorizer per resource since wildcard inputs may + // result in different tenancies. Consider caching per tenancy if this + // is deemed expensive. + entMeta = v2TenancyToV1EntMeta(resource.Id.Tenancy) + authz, authzContext, err = s.getAuthorizer(token, entMeta) + if err != nil { + return nil, err + } + + // Filter out items that don't pass read ACLs. err = reg.ACLs.Read(authz, authzContext, resource.Id) switch { case acl.IsErrPermissionDenied(err): @@ -71,15 +81,37 @@ func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbreso return &pbresource.ListResponse{Resources: result}, nil } -func validateListRequest(req *pbresource.ListRequest) error { +func (s *Server) validateListRequest(req *pbresource.ListRequest) (*resource.Registration, error) { var field string switch { case req.Type == nil: field = "type" case req.Tenancy == nil: field = "tenancy" - default: - return nil } - return status.Errorf(codes.InvalidArgument, "%s is required", field) + + if field != "" { + return nil, status.Errorf(codes.InvalidArgument, "%s is required", field) + } + + // Check type exists. + reg, err := s.resolveType(req.Type) + if err != nil { + return nil, err + } + + // Lowercase + resource.Normalize(req.Tenancy) + + // Error when partition scoped and namespace not empty. + if reg.Scope == resource.ScopePartition && req.Tenancy.Namespace != "" { + return nil, status.Errorf( + codes.InvalidArgument, + "partition scoped type %s cannot have a namespace. got: %s", + resource.ToGVK(req.Type), + req.Tenancy.Namespace, + ) + } + + return reg, nil } diff --git a/agent/grpc-external/services/resource/list_test.go b/agent/grpc-external/services/resource/list_test.go index 93a8d1c45ff..e640e389217 100644 --- a/agent/grpc-external/services/resource/list_test.go +++ b/agent/grpc-external/services/resource/list_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/grpc-external/testutils" + "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/internal/storage" "github.com/hashicorp/consul/proto-public/pbresource" @@ -31,12 +32,16 @@ func TestList_InputValidation(t *testing.T) { testCases := map[string]func(*pbresource.ListRequest){ "no type": func(req *pbresource.ListRequest) { req.Type = nil }, "no tenancy": func(req *pbresource.ListRequest) { req.Tenancy = nil }, + "partitioned resource provides non-empty namespace": func(req *pbresource.ListRequest) { + req.Type = demo.TypeV1RecordLabel + req.Tenancy.Namespace = "bad" + }, } for desc, modFn := range testCases { t.Run(desc, func(t *testing.T) { req := &pbresource.ListRequest{ Type: demo.TypeV2Album, - Tenancy: demo.TenancyDefault, + Tenancy: resource.DefaultNamespacedTenancy(), } modFn(req) @@ -53,7 +58,7 @@ func TestList_TypeNotFound(t *testing.T) { _, err := client.List(context.Background(), &pbresource.ListRequest{ Type: demo.TypeV2Artist, - Tenancy: demo.TenancyDefault, + Tenancy: resource.DefaultNamespacedTenancy(), NamePrefix: "", }) require.Error(t, err) @@ -70,7 +75,7 @@ func TestList_Empty(t *testing.T) { rsp, err := client.List(tc.ctx, &pbresource.ListRequest{ Type: demo.TypeV1Artist, - Tenancy: demo.TenancyDefault, + Tenancy: resource.DefaultNamespacedTenancy(), NamePrefix: "", }) require.NoError(t, err) @@ -102,7 +107,7 @@ func TestList_Many(t *testing.T) { rsp, err := client.List(tc.ctx, &pbresource.ListRequest{ Type: demo.TypeV2Artist, - Tenancy: demo.TenancyDefault, + Tenancy: resource.DefaultNamespacedTenancy(), NamePrefix: "", }) require.NoError(t, err) @@ -111,6 +116,120 @@ func TestList_Many(t *testing.T) { } } +func TestList_Tenancy_Defaults_And_Normalization(t *testing.T) { + // Test units of tenancy get defaulted correctly when empty. + ctx := context.Background() + testCases := map[string]struct { + typ *pbresource.Type + tenancy *pbresource.Tenancy + }{ + "namespaced type with empty partition": { + typ: demo.TypeV2Artist, + tenancy: &pbresource.Tenancy{ + Partition: "", + Namespace: resource.DefaultNamespaceName, + PeerName: "local", + }, + }, + "namespaced type with empty namespace": { + typ: demo.TypeV2Artist, + tenancy: &pbresource.Tenancy{ + Partition: resource.DefaultPartitionName, + Namespace: "", + PeerName: "local", + }, + }, + "namespaced type with empty partition and namespace": { + typ: demo.TypeV2Artist, + tenancy: &pbresource.Tenancy{ + Partition: "", + Namespace: "", + PeerName: "local", + }, + }, + "namespaced type with uppercase partition and namespace": { + typ: demo.TypeV2Artist, + tenancy: &pbresource.Tenancy{ + Partition: "DEFAULT", + Namespace: "DEFAULT", + PeerName: "local", + }, + }, + "namespaced type with wildcard partition and empty namespace": { + typ: demo.TypeV2Artist, + tenancy: &pbresource.Tenancy{ + Partition: "*", + Namespace: "", + PeerName: "local", + }, + }, + "namespaced type with empty partition and wildcard namespace": { + typ: demo.TypeV2Artist, + tenancy: &pbresource.Tenancy{ + Partition: "", + Namespace: "*", + PeerName: "local", + }, + }, + "partitioned type with empty partition": { + typ: demo.TypeV1RecordLabel, + tenancy: &pbresource.Tenancy{ + Partition: "", + Namespace: "", + PeerName: "local", + }, + }, + "partitioned type with uppercase partition": { + typ: demo.TypeV1RecordLabel, + tenancy: &pbresource.Tenancy{ + Partition: "DEFAULT", + Namespace: "", + PeerName: "local", + }, + }, + "partitioned type with wildcard partition": { + typ: demo.TypeV1RecordLabel, + tenancy: &pbresource.Tenancy{ + Partition: "*", + PeerName: "local", + }, + }, + } + for desc, tc := range testCases { + t.Run(desc, func(t *testing.T) { + server := testServer(t) + demo.RegisterTypes(server.Registry) + client := testClient(t, server) + + // Write partition scoped record label + recordLabel, err := demo.GenerateV1RecordLabel("LooneyTunes") + require.NoError(t, err) + recordLabelRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: recordLabel}) + require.NoError(t, err) + + // Write namespace scoped artist + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) + artistRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist}) + require.NoError(t, err) + + // List and verify correct resource returned for empty tenancy units. + listRsp, err := client.List(ctx, &pbresource.ListRequest{ + Type: tc.typ, + Tenancy: tc.tenancy, + }) + require.NoError(t, err) + require.Len(t, listRsp.Resources, 1) + if tc.typ == demo.TypeV1RecordLabel { + prototest.AssertDeepEqual(t, recordLabelRsp.Resource, listRsp.Resources[0]) + } else { + prototest.AssertDeepEqual(t, artistRsp.Resource, listRsp.Resources[0]) + } + }) + + } +} + func TestList_GroupVersionMismatch(t *testing.T) { for desc, tc := range listTestCases() { t.Run(desc, func(t *testing.T) { diff --git a/agent/grpc-external/services/resource/watch.go b/agent/grpc-external/services/resource/watch.go index 17cc48c094a..6f321fbc1ab 100644 --- a/agent/grpc-external/services/resource/watch.go +++ b/agent/grpc-external/services/resource/watch.go @@ -32,7 +32,7 @@ func (s *Server) WatchList(req *pbresource.WatchListRequest, stream pbresource.R } // check acls - err = reg.ACLs.List(authz, req.Tenancy) + err = reg.ACLs.List(authz, authzContext) switch { case acl.IsErrPermissionDenied(err): return status.Error(codes.PermissionDenied, err.Error()) diff --git a/internal/mesh/internal/types/proxy_state_template.go b/internal/mesh/internal/types/proxy_state_template.go index 404237717df..526e0e48cb5 100644 --- a/internal/mesh/internal/types/proxy_state_template.go +++ b/internal/mesh/internal/types/proxy_state_template.go @@ -52,7 +52,7 @@ func RegisterProxyStateTemplate(r resource.Registry) { // managed by a controller. return authorizer.ToAllowAuthorizer().OperatorWriteAllowed(authzContext) }, - List: func(authorizer acl.Authorizer, tenancy *pbresource.Tenancy) error { + List: func(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext) error { // No-op List permission as we want to default to filtering resources // from the list using the Read enforcement. return nil diff --git a/internal/resource/demo/demo.go b/internal/resource/demo/demo.go index 56d40d5ae85..c47d7e31e67 100644 --- a/internal/resource/demo/demo.go +++ b/internal/resource/demo/demo.go @@ -87,13 +87,13 @@ func RegisterTypes(r resource.Registry) { writeACL := func(authz acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error { key := fmt.Sprintf("resource/%s/%s", resource.ToGVK(res.Id.Type), res.Id.Name) - return authz.ToAllowAuthorizer().KeyWriteAllowed(key, &acl.AuthorizerContext{}) + return authz.ToAllowAuthorizer().KeyWriteAllowed(key, authzContext) } - makeListACL := func(typ *pbresource.Type) func(acl.Authorizer, *pbresource.Tenancy) error { - return func(authz acl.Authorizer, tenancy *pbresource.Tenancy) error { + makeListACL := func(typ *pbresource.Type) func(acl.Authorizer, *acl.AuthorizerContext) error { + return func(authz acl.Authorizer, authzContext *acl.AuthorizerContext) error { key := fmt.Sprintf("resource/%s", resource.ToGVK(typ)) - return authz.ToAllowAuthorizer().KeyListAllowed(key, &acl.AuthorizerContext{}) + return authz.ToAllowAuthorizer().KeyListAllowed(key, authzContext) } } diff --git a/internal/resource/registry.go b/internal/resource/registry.go index 84f889ed1c0..575bf6feafd 100644 --- a/internal/resource/registry.go +++ b/internal/resource/registry.go @@ -68,7 +68,7 @@ type ACLHooks struct { // List is used to authorize List RPCs. // // If it is omitted, we only filter the results using Read. - List func(acl.Authorizer, *pbresource.Tenancy) error + List func(acl.Authorizer, *acl.AuthorizerContext) error } // Resource type registry @@ -130,7 +130,7 @@ func (r *TypeRegistry) Register(registration Registration) { } } if registration.ACLs.List == nil { - registration.ACLs.List = func(authz acl.Authorizer, tenancy *pbresource.Tenancy) error { + registration.ACLs.List = func(authz acl.Authorizer, authzContext *acl.AuthorizerContext) error { return authz.ToAllowAuthorizer().OperatorReadAllowed(&acl.AuthorizerContext{}) } } diff --git a/internal/resource/registry_test.go b/internal/resource/registry_test.go index 1e89c9957bb..a7d3ec6a118 100644 --- a/internal/resource/registry_test.go +++ b/internal/resource/registry_test.go @@ -51,8 +51,8 @@ func TestRegister_Defaults(t *testing.T) { require.True(t, acl.IsErrPermissionDenied(reg.ACLs.Write(testutils.ACLNoPermissions(t), nil, artist))) // verify default list hook requires operator:read - require.NoError(t, reg.ACLs.List(testutils.ACLOperatorRead(t), artist.Id.Tenancy)) - require.True(t, acl.IsErrPermissionDenied(reg.ACLs.List(testutils.ACLNoPermissions(t), artist.Id.Tenancy))) + require.NoError(t, reg.ACLs.List(testutils.ACLOperatorRead(t), nil)) + require.True(t, acl.IsErrPermissionDenied(reg.ACLs.List(testutils.ACLNoPermissions(t), nil))) // verify default validate is a no-op require.NoError(t, reg.Validate(nil))