From 9011dd21d64cfd1c95d0013f258caaae736c30eb Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 11 Oct 2023 12:13:56 -0600 Subject: [PATCH] mesh,catalog: add missing ACL hooks --- agent/grpc-external/services/resource/read.go | 2 +- .../types/computed_traffic_permissions.go | 8 +- .../internal/types/traffic_permissions.go | 10 +- .../auth/internal/types/workload_identity.go | 12 +- .../internal/types/workload_identity_test.go | 4 +- .../helpers/acl_hooks_test_helpers.go | 21 +++ internal/catalog/exports.go | 6 + .../controllers/nodehealth/controller_test.go | 2 +- .../testhelpers/acl_hooks_test_helpers.go | 103 ++++++++++++ internal/catalog/internal/types/acl_hooks.go | 56 +++++++ internal/catalog/internal/types/dns_policy.go | 1 + .../catalog/internal/types/dns_policy_test.go | 18 ++ .../catalog/internal/types/failover_policy.go | 8 +- .../internal/types/failover_policy_test.go | 129 ++++---------- .../catalog/internal/types/health_checks.go | 1 + .../internal/types/health_checks_test.go | 11 ++ .../catalog/internal/types/health_status.go | 35 ++++ .../internal/types/health_status_test.go | 86 ++++++++++ internal/catalog/internal/types/node.go | 14 ++ internal/catalog/internal/types/node_test.go | 46 +++++ internal/catalog/internal/types/service.go | 1 + .../internal/types/service_endpoints.go | 10 ++ .../internal/types/service_endpoints_test.go | 44 +++++ .../catalog/internal/types/service_test.go | 11 ++ internal/catalog/internal/types/types.go | 8 +- internal/catalog/internal/types/types_test.go | 4 +- .../catalog/internal/types/virtual_ips.go | 23 ++- .../internal/types/virtual_ips_test.go | 45 +++++ internal/catalog/internal/types/workload.go | 35 ++++ .../internal/types/workload_selecting.go | 16 ++ .../catalog/internal/types/workload_test.go | 158 ++++++++++++++++++ .../workload_selection_mapper.go | 14 +- .../mesh/internal/types/destination_policy.go | 8 +- internal/mesh/internal/types/destinations.go | 1 + .../types/destinations_configuration.go | 4 +- .../types/destinations_configuration_test.go | 11 ++ .../mesh/internal/types/destinations_test.go | 13 +- .../internal/types/proxy_configuration.go | 5 +- .../types/proxy_configuration_test.go | 11 ++ .../internal/types/proxy_state_template.go | 6 +- internal/mesh/internal/types/types.go | 3 +- internal/mesh/internal/types/types_test.go | 3 +- internal/mesh/internal/types/xroute.go | 10 +- internal/mesh/internal/types/xroute_test.go | 2 +- internal/resource/acls.go | 13 ++ internal/resource/demo/demo.go | 2 +- internal/resource/errors.go | 14 ++ internal/resource/registry.go | 4 +- internal/resource/resourcetest/acls.go | 81 +++++++++ 49 files changed, 953 insertions(+), 180 deletions(-) create mode 100644 internal/catalog/catalogtest/helpers/acl_hooks_test_helpers.go create mode 100644 internal/catalog/internal/testhelpers/acl_hooks_test_helpers.go create mode 100644 internal/catalog/internal/types/acl_hooks.go create mode 100644 internal/catalog/internal/types/workload_selecting.go create mode 100644 internal/resource/acls.go create mode 100644 internal/resource/resourcetest/acls.go diff --git a/agent/grpc-external/services/resource/read.go b/agent/grpc-external/services/resource/read.go index 351a50385655..b6cec3725456 100644 --- a/agent/grpc-external/services/resource/read.go +++ b/agent/grpc-external/services/resource/read.go @@ -50,7 +50,7 @@ func (s *Server) Read(ctx context.Context, req *pbresource.ReadRequest) (*pbreso authzNeedsData := false err = reg.ACLs.Read(authz, authzContext, req.Id, nil) switch { - case errors.Is(err, resource.ErrNeedData): + case errors.Is(err, resource.ErrNeedResource): authzNeedsData = true err = nil case acl.IsErrPermissionDenied(err): diff --git a/internal/auth/internal/types/computed_traffic_permissions.go b/internal/auth/internal/types/computed_traffic_permissions.go index 0ba88427233b..0a32e13d2926 100644 --- a/internal/auth/internal/types/computed_traffic_permissions.go +++ b/internal/auth/internal/types/computed_traffic_permissions.go @@ -19,7 +19,7 @@ func RegisterComputedTrafficPermission(r resource.Registry) { ACLs: &resource.ACLHooks{ Read: aclReadHookComputedTrafficPermissions, Write: aclWriteHookComputedTrafficPermissions, - List: aclListHookComputedTrafficPermissions, + List: resource.NoOpACLListHook, }, Validate: ValidateComputedTrafficPermissions, Scope: resource.ScopeNamespace, @@ -71,9 +71,3 @@ func aclReadHookComputedTrafficPermissions(authorizer acl.Authorizer, authzConte func aclWriteHookComputedTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error { return authorizer.ToAllowAuthorizer().TrafficPermissionsWriteAllowed(res.Id.Name, authzContext) } - -func aclListHookComputedTrafficPermissions(_ acl.Authorizer, _ *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/auth/internal/types/traffic_permissions.go b/internal/auth/internal/types/traffic_permissions.go index fb1de7acff90..78d53c70c628 100644 --- a/internal/auth/internal/types/traffic_permissions.go +++ b/internal/auth/internal/types/traffic_permissions.go @@ -19,7 +19,7 @@ func RegisterTrafficPermissions(r resource.Registry) { ACLs: &resource.ACLHooks{ Read: aclReadHookTrafficPermissions, Write: aclWriteHookTrafficPermissions, - List: aclListHookTrafficPermissions, + List: resource.NoOpACLListHook, }, Validate: ValidateTrafficPermissions, Mutate: MutateTrafficPermissions, @@ -273,7 +273,7 @@ func isLocalPeer(p string) bool { func aclReadHookTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, _ *pbresource.ID, res *pbresource.Resource) error { if res == nil { - return resource.ErrNeedData + return resource.ErrNeedResource } return authorizeDestination(res, func(dest string) error { return authorizer.ToAllowAuthorizer().TrafficPermissionsReadAllowed(dest, authzContext) @@ -286,12 +286,6 @@ func aclWriteHookTrafficPermissions(authorizer acl.Authorizer, authzContext *acl }) } -func aclListHookTrafficPermissions(_ acl.Authorizer, _ *acl.AuthorizerContext) error { - // No-op List permission as we want to default to filtering resources - // from the list using the Read enforcement - return nil -} - func authorizeDestination(res *pbresource.Resource, intentionAllowed func(string) error) error { tp, err := resource.Decode[*pbauth.TrafficPermissions](res) if err != nil { diff --git a/internal/auth/internal/types/workload_identity.go b/internal/auth/internal/types/workload_identity.go index 5379d256ba4f..17334e66099e 100644 --- a/internal/auth/internal/types/workload_identity.go +++ b/internal/auth/internal/types/workload_identity.go @@ -18,7 +18,7 @@ func RegisterWorkloadIdentity(r resource.Registry) { ACLs: &resource.ACLHooks{ Read: aclReadHookWorkloadIdentity, Write: aclWriteHookWorkloadIdentity, - List: aclListHookWorkloadIdentity, + List: resource.NoOpACLListHook, }, Validate: nil, }) @@ -36,7 +36,7 @@ func aclReadHookWorkloadIdentity( if res != nil { return authorizer.ToAllowAuthorizer().IdentityReadAllowed(res.Id.Name, authzCtx) } - return resource.ErrNeedData + return resource.ErrNeedResource } func aclWriteHookWorkloadIdentity( @@ -44,13 +44,7 @@ func aclWriteHookWorkloadIdentity( authzCtx *acl.AuthorizerContext, res *pbresource.Resource) error { if res == nil { - return resource.ErrNeedData + return resource.ErrNeedResource } return authorizer.ToAllowAuthorizer().IdentityWriteAllowed(res.Id.Name, authzCtx) } - -func aclListHookWorkloadIdentity(authorizer acl.Authorizer, context *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/auth/internal/types/workload_identity_test.go b/internal/auth/internal/types/workload_identity_test.go index 1ca59952ecee..8dfb22bc74a2 100644 --- a/internal/auth/internal/types/workload_identity_test.go +++ b/internal/auth/internal/types/workload_identity_test.go @@ -82,8 +82,8 @@ func TestWorkloadIdentityACLs(t *testing.T) { checkF(t, tc.listOK, err) }) t.Run("errors", func(t *testing.T) { - require.ErrorIs(t, reg.ACLs.Read(authz, &acl.AuthorizerContext{}, nil, nil), resource.ErrNeedData) - require.ErrorIs(t, reg.ACLs.Write(authz, &acl.AuthorizerContext{}, nil), resource.ErrNeedData) + require.ErrorIs(t, reg.ACLs.Read(authz, &acl.AuthorizerContext{}, nil, nil), resource.ErrNeedResource) + require.ErrorIs(t, reg.ACLs.Write(authz, &acl.AuthorizerContext{}, nil), resource.ErrNeedResource) }) } diff --git a/internal/catalog/catalogtest/helpers/acl_hooks_test_helpers.go b/internal/catalog/catalogtest/helpers/acl_hooks_test_helpers.go new file mode 100644 index 000000000000..097647ed08d1 --- /dev/null +++ b/internal/catalog/catalogtest/helpers/acl_hooks_test_helpers.go @@ -0,0 +1,21 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package helpers + +import ( + "testing" + + "github.com/hashicorp/consul/internal/catalog" + "github.com/hashicorp/consul/internal/catalog/internal/testhelpers" + "github.com/hashicorp/consul/internal/resource" + pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" + "github.com/hashicorp/consul/proto-public/pbresource" +) + +func RunWorkloadSelectingTypeACLsTests[T catalog.WorkloadSelecting](t *testing.T, typ *pbresource.Type, + getData func(selector *pbcatalog.WorkloadSelector) T, + registerFunc func(registry resource.Registry), +) { + testhelpers.RunWorkloadSelectingTypeACLsTests[T](t, typ, getData, registerFunc) +} diff --git a/internal/catalog/exports.go b/internal/catalog/exports.go index c4e70ffbefe1..671a8c56208b 100644 --- a/internal/catalog/exports.go +++ b/internal/catalog/exports.go @@ -48,6 +48,12 @@ var ( FailoverStatusConditionAcceptedUsingMeshDestinationPortReason = failover.UsingMeshDestinationPortReason ) +type WorkloadSelecting = types.WorkloadSelecting + +func ACLHooksForWorkloadSelectingType[T WorkloadSelecting]() *resource.ACLHooks { + return types.ACLHooksForWorkloadSelectingType[T]() +} + // RegisterTypes adds all resource types within the "catalog" API group // to the given type registry func RegisterTypes(r resource.Registry) { diff --git a/internal/catalog/internal/controllers/nodehealth/controller_test.go b/internal/catalog/internal/controllers/nodehealth/controller_test.go index 30989b479b0f..b21c52e521f8 100644 --- a/internal/catalog/internal/controllers/nodehealth/controller_test.go +++ b/internal/catalog/internal/controllers/nodehealth/controller_test.go @@ -73,7 +73,7 @@ type nodeHealthControllerTestSuite struct { } func (suite *nodeHealthControllerTestSuite) SetupTest() { - suite.resourceClient = svctest.RunResourceService(suite.T(), types.Register) + suite.resourceClient = svctest.RunResourceService(suite.T(), types.Register, types.RegisterDNSPolicy) suite.runtime = controller.Runtime{Client: suite.resourceClient, Logger: testutil.Logger(suite.T())} // The rest of the setup will be to prime the resource service with some data diff --git a/internal/catalog/internal/testhelpers/acl_hooks_test_helpers.go b/internal/catalog/internal/testhelpers/acl_hooks_test_helpers.go new file mode 100644 index 000000000000..c1a00edf53ef --- /dev/null +++ b/internal/catalog/internal/testhelpers/acl_hooks_test_helpers.go @@ -0,0 +1,103 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package testhelpers + +import ( + "testing" + + "google.golang.org/protobuf/proto" + + "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/internal/resource/resourcetest" + pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" + "github.com/hashicorp/consul/proto-public/pbresource" +) + +// WorkloadSelecting denotes a resource type that uses workload selectors. +type WorkloadSelecting interface { + proto.Message + GetWorkloads() *pbcatalog.WorkloadSelector +} + +func RunWorkloadSelectingTypeACLsTests[T WorkloadSelecting](t *testing.T, typ *pbresource.Type, + getData func(selector *pbcatalog.WorkloadSelector) T, + registerFunc func(registry resource.Registry), +) { + // Wire up a registry to generically invoke hooks + registry := resource.NewRegistry() + registerFunc(registry) + + cases := map[string]resourcetest.ACLTestCase{ + "no rules": { + Rules: ``, + Data: getData(&pbcatalog.WorkloadSelector{Names: []string{"workload"}}), + Typ: typ, + ReadOK: resourcetest.DENY, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test read": { + Rules: `service "test" { policy = "read" }`, + Data: getData(&pbcatalog.WorkloadSelector{Names: []string{"workload"}}), + Typ: typ, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test write with named selectors and insufficient policy": { + Rules: `service "test" { policy = "write" }`, + Data: getData(&pbcatalog.WorkloadSelector{Names: []string{"workload"}}), + Typ: typ, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test write with prefixed selectors and insufficient policy": { + Rules: `service "test" { policy = "write" }`, + Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"workload"}}), + Typ: typ, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test write with named selectors": { + Rules: `service "test" { policy = "write" } service "workload" { policy = "read" }`, + Data: getData(&pbcatalog.WorkloadSelector{Names: []string{"workload"}}), + Typ: typ, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.ALLOW, + ListOK: resourcetest.DEFAULT, + }, + "service test write with prefixed selectors": { + Rules: `service "test" { policy = "write" } service_prefix "workload-" { policy = "read" }`, + Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"workload-"}}), + Typ: typ, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.ALLOW, + ListOK: resourcetest.DEFAULT, + }, + "service test write with prefixed selectors and a policy with more specific than the selector": { + Rules: `service "test" { policy = "write" } service_prefix "workload-" { policy = "read" }`, + Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"wor"}}), + Typ: typ, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test write with prefixed selectors and a policy with less specific than the selector": { + Rules: `service "test" { policy = "write" } service_prefix "wor" { policy = "read" }`, + Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"workload-"}}), + Typ: typ, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.ALLOW, + ListOK: resourcetest.DEFAULT, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + resourcetest.RunACLTestCase(t, tc, registry) + }) + } +} diff --git a/internal/catalog/internal/types/acl_hooks.go b/internal/catalog/internal/types/acl_hooks.go new file mode 100644 index 000000000000..3752e2b8de35 --- /dev/null +++ b/internal/catalog/internal/types/acl_hooks.go @@ -0,0 +1,56 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package types + +import ( + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/proto-public/pbresource" +) + +func aclReadHookResourceWithWorkloadSelector(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, id *pbresource.ID, _ *pbresource.Resource) error { + return authorizer.ToAllowAuthorizer().ServiceReadAllowed(id.GetName(), authzContext) +} + +func aclWriteHookResourceWithWorkloadSelector[T WorkloadSelecting](authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error { + if res == nil { + return resource.ErrNeedResource + } + + decodedService, err := resource.Decode[T](res) + if err != nil { + return resource.ErrNeedResource + } + + // First check service:write on the name. + err = authorizer.ToAllowAuthorizer().ServiceWriteAllowed(res.GetId().GetName(), authzContext) + if err != nil { + return err + } + + // Then also check whether we're allowed to select a service. + for _, name := range decodedService.GetData().GetWorkloads().GetNames() { + err = authorizer.ToAllowAuthorizer().ServiceReadAllowed(name, authzContext) + if err != nil { + return err + } + } + + for _, prefix := range decodedService.GetData().GetWorkloads().GetPrefixes() { + err = authorizer.ToAllowAuthorizer().ServiceReadAllowed(prefix, authzContext) + if err != nil { + return err + } + } + + return nil +} + +func ACLHooksForWorkloadSelectingType[T WorkloadSelecting]() *resource.ACLHooks { + return &resource.ACLHooks{ + Read: aclReadHookResourceWithWorkloadSelector, + Write: aclWriteHookResourceWithWorkloadSelector[T], + List: resource.NoOpACLListHook, + } +} diff --git a/internal/catalog/internal/types/dns_policy.go b/internal/catalog/internal/types/dns_policy.go index 809dd6f880c2..8e9dd864a957 100644 --- a/internal/catalog/internal/types/dns_policy.go +++ b/internal/catalog/internal/types/dns_policy.go @@ -19,6 +19,7 @@ func RegisterDNSPolicy(r resource.Registry) { Proto: &pbcatalog.DNSPolicy{}, Scope: resource.ScopeNamespace, Validate: ValidateDNSPolicy, + ACLs: ACLHooksForWorkloadSelectingType[*pbcatalog.DNSPolicy](), }) } diff --git a/internal/catalog/internal/types/dns_policy_test.go b/internal/catalog/internal/types/dns_policy_test.go index 3a611171070a..9f74366592e5 100644 --- a/internal/catalog/internal/types/dns_policy_test.go +++ b/internal/catalog/internal/types/dns_policy_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/known/anypb" + "github.com/hashicorp/consul/internal/catalog/internal/testhelpers" "github.com/hashicorp/consul/internal/resource" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" @@ -161,3 +162,20 @@ func TestValidateDNSPolicy_EmptySelector(t *testing.T) { require.ErrorAs(t, err, &actual) require.Equal(t, expected, actual) } + +func TestDNSPolicyACLs(t *testing.T) { + // Wire up a registry to generically invoke hooks + registry := resource.NewRegistry() + RegisterDNSPolicy(registry) + + testhelpers.RunWorkloadSelectingTypeACLsTests[*pbcatalog.DNSPolicy](t, pbcatalog.DNSPolicyType, + func(selector *pbcatalog.WorkloadSelector) *pbcatalog.DNSPolicy { + return &pbcatalog.DNSPolicy{ + Workloads: selector, + Weights: &pbcatalog.Weights{Passing: 1, Warning: 0}, + } + }, + func(registry resource.Registry) { + RegisterDNSPolicy(registry) + }) +} diff --git a/internal/catalog/internal/types/failover_policy.go b/internal/catalog/internal/types/failover_policy.go index 4dc8b1bd8eb0..fa1d5c3f32ed 100644 --- a/internal/catalog/internal/types/failover_policy.go +++ b/internal/catalog/internal/types/failover_policy.go @@ -25,7 +25,7 @@ func RegisterFailoverPolicy(r resource.Registry) { ACLs: &resource.ACLHooks{ Read: aclReadHookFailoverPolicy, Write: aclWriteHookFailoverPolicy, - List: aclListHookFailoverPolicy, + List: resource.NoOpACLListHook, }, }) } @@ -371,9 +371,3 @@ func aclWriteHookFailoverPolicy(authorizer acl.Authorizer, authzContext *acl.Aut return nil } - -func aclListHookFailoverPolicy(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/catalog/internal/types/failover_policy_test.go b/internal/catalog/internal/types/failover_policy_test.go index 8abe5d5cb74d..923d260580e0 100644 --- a/internal/catalog/internal/types/failover_policy_test.go +++ b/internal/catalog/internal/types/failover_policy_test.go @@ -10,8 +10,6 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" - "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" @@ -685,105 +683,52 @@ func TestFailoverPolicyACLs(t *testing.T) { registry := resource.NewRegistry() Register(registry) - type testcase struct { - rules string - check func(t *testing.T, authz acl.Authorizer, res *pbresource.Resource) - readOK string - writeOK string - listOK string - } - - const ( - DENY = "deny" - ALLOW = "allow" - DEFAULT = "default" - ) - - checkF := func(t *testing.T, expect string, got error) { - switch expect { - case ALLOW: - if acl.IsErrPermissionDenied(got) { - t.Fatal("should be allowed") - } - case DENY: - if !acl.IsErrPermissionDenied(got) { - t.Fatal("should be denied") - } - case DEFAULT: - require.Nil(t, got, "expected fallthrough decision") - default: - t.Fatalf("unexpected expectation: %q", expect) - } - } - - reg, ok := registry.Resolve(pbcatalog.FailoverPolicyType) - require.True(t, ok) - - run := func(t *testing.T, tc testcase) { - failoverData := &pbcatalog.FailoverPolicy{ - Config: &pbcatalog.FailoverConfig{ - Destinations: []*pbcatalog.FailoverDestination{ - {Ref: newRef(pbcatalog.ServiceType, "api-backup")}, - }, + failoverData := &pbcatalog.FailoverPolicy{ + Config: &pbcatalog.FailoverConfig{ + Destinations: []*pbcatalog.FailoverDestination{ + {Ref: newRef(pbcatalog.ServiceType, "api-backup")}, }, - } - res := resourcetest.Resource(pbcatalog.FailoverPolicyType, "api"). - WithTenancy(resource.DefaultNamespacedTenancy()). - WithData(t, failoverData). - Build() - resourcetest.ValidateAndNormalize(t, registry, res) - - config := acl.Config{ - WildcardName: structs.WildcardSpecifier, - } - authz, err := acl.NewAuthorizerFromRules(tc.rules, &config, nil) - require.NoError(t, err) - authz = acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.DenyAll()}) - - t.Run("read", func(t *testing.T) { - err := reg.ACLs.Read(authz, &acl.AuthorizerContext{}, res.Id, nil) - checkF(t, tc.readOK, err) - }) - t.Run("write", func(t *testing.T) { - err := reg.ACLs.Write(authz, &acl.AuthorizerContext{}, res) - checkF(t, tc.writeOK, err) - }) - t.Run("list", func(t *testing.T) { - err := reg.ACLs.List(authz, &acl.AuthorizerContext{}) - checkF(t, tc.listOK, err) - }) + }, } - cases := map[string]testcase{ + cases := map[string]resourcetest.ACLTestCase{ "no rules": { - rules: ``, - readOK: DENY, - writeOK: DENY, - listOK: DEFAULT, - }, - "service api read": { - rules: `service "api" { policy = "read" }`, - readOK: ALLOW, - writeOK: DENY, - listOK: DEFAULT, - }, - "service api write": { - rules: `service "api" { policy = "write" }`, - readOK: ALLOW, - writeOK: DENY, - listOK: DEFAULT, - }, - "service api write and api-backup read": { - rules: `service "api" { policy = "write" } service "api-backup" { policy = "read" }`, - readOK: ALLOW, - writeOK: ALLOW, - listOK: DEFAULT, + Rules: ``, + Data: failoverData, + Typ: pbcatalog.FailoverPolicyType, + ReadOK: resourcetest.DENY, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test read": { + Rules: `service "test" { policy = "read" }`, + Data: failoverData, + Typ: pbcatalog.FailoverPolicyType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test write": { + Rules: `service "test" { policy = "write" }`, + Data: failoverData, + Typ: pbcatalog.FailoverPolicyType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test write and api-backup read": { + Rules: `service "test" { policy = "write" } service "api-backup" { policy = "read" }`, + Data: failoverData, + Typ: pbcatalog.FailoverPolicyType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.ALLOW, + ListOK: resourcetest.DEFAULT, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - run(t, tc) + resourcetest.RunACLTestCase(t, tc, registry) }) } } diff --git a/internal/catalog/internal/types/health_checks.go b/internal/catalog/internal/types/health_checks.go index b470be331feb..1333e2368d88 100644 --- a/internal/catalog/internal/types/health_checks.go +++ b/internal/catalog/internal/types/health_checks.go @@ -17,6 +17,7 @@ func RegisterHealthChecks(r resource.Registry) { Proto: &pbcatalog.HealthChecks{}, Scope: resource.ScopeNamespace, Validate: ValidateHealthChecks, + ACLs: ACLHooksForWorkloadSelectingType[*pbcatalog.HealthChecks](), }) } diff --git a/internal/catalog/internal/types/health_checks_test.go b/internal/catalog/internal/types/health_checks_test.go index 8af0ffde9fac..26c09a419d1d 100644 --- a/internal/catalog/internal/types/health_checks_test.go +++ b/internal/catalog/internal/types/health_checks_test.go @@ -12,6 +12,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" + "github.com/hashicorp/consul/internal/catalog/internal/testhelpers" "github.com/hashicorp/consul/internal/resource" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" @@ -196,3 +197,13 @@ func TestValidateHealthChecks_EmptySelector(t *testing.T) { require.ErrorAs(t, err, &actual) require.Equal(t, expected, actual) } + +func TestHealthChecksACLs(t *testing.T) { + testhelpers.RunWorkloadSelectingTypeACLsTests[*pbcatalog.HealthChecks](t, pbcatalog.HealthChecksType, + func(selector *pbcatalog.WorkloadSelector) *pbcatalog.HealthChecks { + return &pbcatalog.HealthChecks{Workloads: selector} + }, + func(registry resource.Registry) { + RegisterHealthChecks(registry) + }) +} diff --git a/internal/catalog/internal/types/health_status.go b/internal/catalog/internal/types/health_status.go index 99b153895c1f..fe92e858b025 100644 --- a/internal/catalog/internal/types/health_status.go +++ b/internal/catalog/internal/types/health_status.go @@ -6,6 +6,7 @@ package types import ( "github.com/hashicorp/go-multierror" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" @@ -17,6 +18,11 @@ func RegisterHealthStatus(r resource.Registry) { Proto: &pbcatalog.HealthStatus{}, Scope: resource.ScopeNamespace, Validate: ValidateHealthStatus, + ACLs: &resource.ACLHooks{ + Read: aclReadHookHealthStatus, + Write: aclWriteHookHealthStatus, + List: resource.NoOpACLListHook, + }, }) } @@ -66,3 +72,32 @@ func ValidateHealthStatus(res *pbresource.Resource) error { return err } + +func aclReadHookHealthStatus(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, _ *pbresource.ID, res *pbresource.Resource) error { + if res == nil { + return resource.ErrNeedResource + } + // For a health status of a workload we need to check service:read perms. + if res.GetOwner() != nil && resource.EqualType(res.GetOwner().GetType(), pbcatalog.WorkloadType) { + return authorizer.ToAllowAuthorizer().ServiceReadAllowed(res.GetOwner().GetName(), authzContext) + } + + if res.GetOwner() != nil && resource.EqualType(res.GetOwner().GetType(), pbcatalog.NodeType) { + return authorizer.ToAllowAuthorizer().NodeReadAllowed(res.GetOwner().GetName(), authzContext) + } + + return acl.PermissionDenied("cannot read catalog.HealthStatus because there is no owner") +} + +func aclWriteHookHealthStatus(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error { + // For a health status of a workload we need to check service:write perms. + if res.GetOwner() != nil && resource.EqualType(res.GetOwner().GetType(), pbcatalog.WorkloadType) { + return authorizer.ToAllowAuthorizer().ServiceWriteAllowed(res.GetOwner().GetName(), authzContext) + } + + if res.GetOwner() != nil && resource.EqualType(res.GetOwner().GetType(), pbcatalog.NodeType) { + return authorizer.ToAllowAuthorizer().NodeWriteAllowed(res.GetOwner().GetName(), authzContext) + } + + return acl.PermissionDenied("cannot write catalog.HealthStatus because there is no owner") +} diff --git a/internal/catalog/internal/types/health_status_test.go b/internal/catalog/internal/types/health_status_test.go index 654573d24a53..644d9effb347 100644 --- a/internal/catalog/internal/types/health_status_test.go +++ b/internal/catalog/internal/types/health_status_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" ) @@ -214,3 +215,88 @@ func TestValidateHealthStatus_InvalidOwner(t *testing.T) { }) } } + +func TestHealthStatusACLs(t *testing.T) { + registry := resource.NewRegistry() + Register(registry) + + workload := resourcetest.Resource(pbcatalog.WorkloadType, "test").ID() + node := resourcetest.Resource(pbcatalog.NodeType, "test").ID() + + healthStatusData := &pbcatalog.HealthStatus{ + Type: "tcp", + Status: pbcatalog.Health_HEALTH_PASSING, + } + + cases := map[string]resourcetest.ACLTestCase{ + "no rules": { + Rules: ``, + Data: healthStatusData, + Owner: workload, + Typ: pbcatalog.HealthStatusType, + ReadOK: resourcetest.DENY, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test read": { + Rules: `service "test" { policy = "read" }`, + Data: healthStatusData, + Owner: workload, + Typ: pbcatalog.HealthStatusType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test write": { + Rules: `service "test" { policy = "write" }`, + Data: healthStatusData, + Owner: workload, + Typ: pbcatalog.HealthStatusType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.ALLOW, + ListOK: resourcetest.DEFAULT, + }, + "service test read with node owner": { + Rules: `service "test" { policy = "read" }`, + Data: healthStatusData, + Owner: node, + Typ: pbcatalog.HealthStatusType, + ReadOK: resourcetest.DENY, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "service test write with node owner": { + Rules: `service "test" { policy = "write" }`, + Data: healthStatusData, + Owner: node, + Typ: pbcatalog.HealthStatusType, + ReadOK: resourcetest.DENY, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "node test read with node owner": { + Rules: `node "test" { policy = "read" }`, + Data: healthStatusData, + Owner: node, + Typ: pbcatalog.HealthStatusType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "node test write with node owner": { + Rules: `node "test" { policy = "write" }`, + Data: healthStatusData, + Owner: node, + Typ: pbcatalog.HealthStatusType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.ALLOW, + ListOK: resourcetest.DEFAULT, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + resourcetest.RunACLTestCase(t, tc, registry) + }) + } +} diff --git a/internal/catalog/internal/types/node.go b/internal/catalog/internal/types/node.go index 9c59228a49b6..42ac833c6e7d 100644 --- a/internal/catalog/internal/types/node.go +++ b/internal/catalog/internal/types/node.go @@ -6,6 +6,7 @@ package types import ( "github.com/hashicorp/go-multierror" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" @@ -22,6 +23,11 @@ func RegisterNode(r resource.Registry) { // Until that time, Node will remain namespace scoped. Scope: resource.ScopeNamespace, Validate: ValidateNode, + ACLs: &resource.ACLHooks{ + Read: aclReadHookNode, + Write: aclWriteHookNode, + List: resource.NoOpACLListHook, + }, }) } @@ -80,3 +86,11 @@ func validateNodeAddress(addr *pbcatalog.NodeAddress) error { return nil } + +func aclReadHookNode(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, id *pbresource.ID, _ *pbresource.Resource) error { + return authorizer.ToAllowAuthorizer().NodeReadAllowed(id.GetName(), authzContext) +} + +func aclWriteHookNode(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error { + return authorizer.ToAllowAuthorizer().NodeWriteAllowed(res.GetId().GetName(), authzContext) +} diff --git a/internal/catalog/internal/types/node_test.go b/internal/catalog/internal/types/node_test.go index 130551fad2b6..5a678745e3a3 100644 --- a/internal/catalog/internal/types/node_test.go +++ b/internal/catalog/internal/types/node_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" ) @@ -127,3 +128,48 @@ func TestValidateNode_AddressMissingHost(t *testing.T) { require.ErrorAs(t, err, &actual) require.Equal(t, expected, actual) } + +func TestNodeACLs(t *testing.T) { + registry := resource.NewRegistry() + Register(registry) + + nodeData := &pbcatalog.Node{ + Addresses: []*pbcatalog.NodeAddress{ + { + Host: "1.1.1.1", + }, + }, + } + cases := map[string]resourcetest.ACLTestCase{ + "no rules": { + Rules: ``, + Data: nodeData, + Typ: pbcatalog.NodeType, + ReadOK: resourcetest.DENY, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "node test read": { + Rules: `node "test" { policy = "read" }`, + Data: nodeData, + Typ: pbcatalog.NodeType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.DENY, + ListOK: resourcetest.DEFAULT, + }, + "node test write": { + Rules: `node "test" { policy = "write" }`, + Data: nodeData, + Typ: pbcatalog.NodeType, + ReadOK: resourcetest.ALLOW, + WriteOK: resourcetest.ALLOW, + ListOK: resourcetest.DEFAULT, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + resourcetest.RunACLTestCase(t, tc, registry) + }) + } +} diff --git a/internal/catalog/internal/types/service.go b/internal/catalog/internal/types/service.go index 4cefb362e78f..54e2aaaab093 100644 --- a/internal/catalog/internal/types/service.go +++ b/internal/catalog/internal/types/service.go @@ -20,6 +20,7 @@ func RegisterService(r resource.Registry) { Scope: resource.ScopeNamespace, Validate: ValidateService, Mutate: MutateService, + ACLs: ACLHooksForWorkloadSelectingType[*pbcatalog.Service](), }) } diff --git a/internal/catalog/internal/types/service_endpoints.go b/internal/catalog/internal/types/service_endpoints.go index 8008ada845b4..9e16d0338a5d 100644 --- a/internal/catalog/internal/types/service_endpoints.go +++ b/internal/catalog/internal/types/service_endpoints.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/go-multierror" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" @@ -20,6 +21,15 @@ func RegisterServiceEndpoints(r resource.Registry) { Scope: resource.ScopeNamespace, Validate: ValidateServiceEndpoints, Mutate: MutateServiceEndpoints, + ACLs: &resource.ACLHooks{ + Read: func(authorizer acl.Authorizer, context *acl.AuthorizerContext, id *pbresource.ID, _ *pbresource.Resource) error { + return authorizer.ToAllowAuthorizer().ServiceReadAllowed(id.GetName(), context) + }, + Write: func(authorizer acl.Authorizer, context *acl.AuthorizerContext, p *pbresource.Resource) error { + return authorizer.ToAllowAuthorizer().ServiceWriteAllowed(p.GetId().GetName(), context) + }, + List: resource.NoOpACLListHook, + }, }) } diff --git a/internal/catalog/internal/types/service_endpoints_test.go b/internal/catalog/internal/types/service_endpoints_test.go index d210ba1aaafd..7a298e397949 100644 --- a/internal/catalog/internal/types/service_endpoints_test.go +++ b/internal/catalog/internal/types/service_endpoints_test.go @@ -258,3 +258,47 @@ func TestMutateServiceEndpoints_PopulateOwner(t *testing.T) { require.True(t, resource.EqualTenancy(res.Owner.Tenancy, defaultEndpointTenancy)) require.Equal(t, res.Owner.Name, res.Id.Name) } + +func TestServiceEndpointsACLs(t *testing.T) { + registry := resource.NewRegistry() + Register(registry) + + service := rtest.Resource(pbcatalog.ServiceType, "test"). + WithTenancy(resource.DefaultNamespacedTenancy()).ID() + serviceEndpointsData := &pbcatalog.ServiceEndpoints{} + cases := map[string]rtest.ACLTestCase{ + "no rules": { + Rules: ``, + Data: serviceEndpointsData, + Owner: service, + Typ: pbcatalog.ServiceEndpointsType, + ReadOK: rtest.DENY, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test read": { + Rules: `service "test" { policy = "read" }`, + Data: serviceEndpointsData, + Owner: service, + Typ: pbcatalog.ServiceEndpointsType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test write": { + Rules: `service "test" { policy = "write" }`, + Data: serviceEndpointsData, + Owner: service, + Typ: pbcatalog.ServiceEndpointsType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.ALLOW, + ListOK: rtest.DEFAULT, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + rtest.RunACLTestCase(t, tc, registry) + }) + } +} diff --git a/internal/catalog/internal/types/service_test.go b/internal/catalog/internal/types/service_test.go index b47c02218405..1de151568e12 100644 --- a/internal/catalog/internal/types/service_test.go +++ b/internal/catalog/internal/types/service_test.go @@ -10,6 +10,7 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/known/anypb" + "github.com/hashicorp/consul/internal/catalog/internal/testhelpers" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" @@ -275,3 +276,13 @@ func TestValidateService_InvalidVIP(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, errNotIPAddress) } + +func TestServiceACLs(t *testing.T) { + testhelpers.RunWorkloadSelectingTypeACLsTests[*pbcatalog.Service](t, pbcatalog.ServiceType, + func(selector *pbcatalog.WorkloadSelector) *pbcatalog.Service { + return &pbcatalog.Service{Workloads: selector} + }, + func(registry resource.Registry) { + RegisterService(registry) + }) +} diff --git a/internal/catalog/internal/types/types.go b/internal/catalog/internal/types/types.go index 6ce29a265963..15ed6b148de7 100644 --- a/internal/catalog/internal/types/types.go +++ b/internal/catalog/internal/types/types.go @@ -13,8 +13,10 @@ func Register(r resource.Registry) { RegisterServiceEndpoints(r) RegisterNode(r) RegisterHealthStatus(r) - RegisterHealthChecks(r) - RegisterDNSPolicy(r) - RegisterVirtualIPs(r) RegisterFailoverPolicy(r) + + // todo (v2): re-register once these resources are implemented. + //RegisterHealthChecks(r) + //RegisterDNSPolicy(r) + //RegisterVirtualIPs(r) } diff --git a/internal/catalog/internal/types/types_test.go b/internal/catalog/internal/types/types_test.go index ba4243b62805..4facd921c368 100644 --- a/internal/catalog/internal/types/types_test.go +++ b/internal/catalog/internal/types/types_test.go @@ -24,9 +24,9 @@ func TestTypeRegistration(t *testing.T) { pbcatalog.ServiceEndpointsKind, pbcatalog.NodeKind, pbcatalog.HealthStatusKind, - pbcatalog.HealthChecksKind, - pbcatalog.DNSPolicyKind, // todo (ishustava): uncomment once we implement these + //pbcatalog.HealthChecksKind, + //pbcatalog.DNSPolicyKind, //pbcatalog.VirtualIPsKind, } diff --git a/internal/catalog/internal/types/virtual_ips.go b/internal/catalog/internal/types/virtual_ips.go index 7a4cee276ae6..9c7a06547405 100644 --- a/internal/catalog/internal/types/virtual_ips.go +++ b/internal/catalog/internal/types/virtual_ips.go @@ -6,19 +6,28 @@ package types import ( "github.com/hashicorp/go-multierror" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" ) func RegisterVirtualIPs(r resource.Registry) { - // todo (ishustava): uncomment when we implement it - //r.Register(resource.Registration{ - // Type: pbcatalog.VirtualIPsV2Beta1Type, - // Proto: &pbcatalog.VirtualIPs{}, - // Scope: resource.ScopeNamespace, - // Validate: ValidateVirtualIPs, - //}) + r.Register(resource.Registration{ + Type: pbcatalog.VirtualIPsType, + Proto: &pbcatalog.VirtualIPs{}, + Scope: resource.ScopeNamespace, + Validate: ValidateVirtualIPs, + ACLs: &resource.ACLHooks{ + Read: func(authorizer acl.Authorizer, context *acl.AuthorizerContext, id *pbresource.ID, p *pbresource.Resource) error { + return authorizer.ToAllowAuthorizer().ServiceReadAllowed(id.GetName(), context) + }, + Write: func(authorizer acl.Authorizer, context *acl.AuthorizerContext, p *pbresource.Resource) error { + return authorizer.ToAllowAuthorizer().ServiceWriteAllowed(p.GetId().GetName(), context) + }, + List: resource.NoOpACLListHook, + }, + }) } func ValidateVirtualIPs(res *pbresource.Resource) error { diff --git a/internal/catalog/internal/types/virtual_ips_test.go b/internal/catalog/internal/types/virtual_ips_test.go index c7ed70972530..0107e1cfd94f 100644 --- a/internal/catalog/internal/types/virtual_ips_test.go +++ b/internal/catalog/internal/types/virtual_ips_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" "github.com/hashicorp/consul/internal/resource" + rtest "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" ) @@ -81,3 +82,47 @@ func TestValidateVirtualIPs_InvalidIP(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, errNotIPAddress) } + +func TestVirtualIPsACLs(t *testing.T) { + registry := resource.NewRegistry() + RegisterVirtualIPs(registry) + + service := rtest.Resource(pbcatalog.ServiceType, "test"). + WithTenancy(resource.DefaultNamespacedTenancy()).ID() + virtualIPsData := &pbcatalog.VirtualIPs{} + cases := map[string]rtest.ACLTestCase{ + "no rules": { + Rules: ``, + Data: virtualIPsData, + Owner: service, + Typ: pbcatalog.VirtualIPsType, + ReadOK: rtest.DENY, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test read": { + Rules: `service "test" { policy = "read" }`, + Data: virtualIPsData, + Owner: service, + Typ: pbcatalog.VirtualIPsType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test write": { + Rules: `service "test" { policy = "write" }`, + Data: virtualIPsData, + Owner: service, + Typ: pbcatalog.VirtualIPsType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.ALLOW, + ListOK: rtest.DEFAULT, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + rtest.RunACLTestCase(t, tc, registry) + }) + } +} diff --git a/internal/catalog/internal/types/workload.go b/internal/catalog/internal/types/workload.go index 961a85346c4f..7e064ecb05b1 100644 --- a/internal/catalog/internal/types/workload.go +++ b/internal/catalog/internal/types/workload.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/go-multierror" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" @@ -20,6 +21,11 @@ func RegisterWorkload(r resource.Registry) { Proto: &pbcatalog.Workload{}, Scope: resource.ScopeNamespace, Validate: ValidateWorkload, + ACLs: &resource.ACLHooks{ + Read: aclReadHookWorkload, + Write: aclWriteHookWorkload, + List: resource.NoOpACLListHook, + }, }) } @@ -145,3 +151,32 @@ func ValidateWorkload(res *pbresource.Resource) error { return err } + +func aclReadHookWorkload(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, id *pbresource.ID, _ *pbresource.Resource) error { + return authorizer.ToAllowAuthorizer().ServiceReadAllowed(id.GetName(), authzContext) +} + +func aclWriteHookWorkload(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error { + decodedWorkload, err := resource.Decode[*pbcatalog.Workload](res) + if err != nil { + return resource.ErrNeedResource + } + + // First check service:write on the workload name. + err = authorizer.ToAllowAuthorizer().ServiceWriteAllowed(res.GetId().GetName(), authzContext) + if err != nil { + return err + } + + // Check node:read permissions if node is specified. + if decodedWorkload.GetData().GetNodeName() != "" { + return authorizer.ToAllowAuthorizer().NodeReadAllowed(decodedWorkload.GetData().GetNodeName(), authzContext) + } + + // Check identity:read permissions if identity is specified. + if decodedWorkload.GetData().GetIdentity() != "" { + return authorizer.ToAllowAuthorizer().IdentityReadAllowed(decodedWorkload.GetData().GetIdentity(), authzContext) + } + + return nil +} diff --git a/internal/catalog/internal/types/workload_selecting.go b/internal/catalog/internal/types/workload_selecting.go new file mode 100644 index 000000000000..6d129bfaa693 --- /dev/null +++ b/internal/catalog/internal/types/workload_selecting.go @@ -0,0 +1,16 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package types + +import ( + "google.golang.org/protobuf/proto" + + pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" +) + +// WorkloadSelecting denotes a resource type that uses workload selectors. +type WorkloadSelecting interface { + proto.Message + GetWorkloads() *pbcatalog.WorkloadSelector +} diff --git a/internal/catalog/internal/types/workload_test.go b/internal/catalog/internal/types/workload_test.go index e55d9a44fd2b..1c7f7b825594 100644 --- a/internal/catalog/internal/types/workload_test.go +++ b/internal/catalog/internal/types/workload_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" "github.com/hashicorp/consul/internal/resource" + rtest "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" ) @@ -304,3 +305,160 @@ func TestValidateWorkload_Locality(t *testing.T) { require.ErrorAs(t, err, &actual) require.Equal(t, expected, actual) } + +func TestWorkloadACLs(t *testing.T) { + registry := resource.NewRegistry() + Register(registry) + + cases := map[string]rtest.ACLTestCase{ + "no rules": { + Rules: ``, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.DENY, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test read": { + Rules: `service "test" { policy = "read" }`, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test write": { + Rules: `service "test" { policy = "write" }`, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.ALLOW, + ListOK: rtest.DEFAULT, + }, + "service test write with node": { + Rules: `service "test" { policy = "write" }`, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + NodeName: "test-node", + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test write with workload identity": { + Rules: `service "test" { policy = "write" }`, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + Identity: "test-identity", + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test write with workload identity and node": { + Rules: `service "test" { policy = "write" }`, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + NodeName: "test-node", + Identity: "test-identity", + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.DENY, + ListOK: rtest.DEFAULT, + }, + "service test write with node and node policy": { + Rules: `service "test" { policy = "write" } node "test-node" { policy = "read" }`, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + NodeName: "test-node", + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.ALLOW, + ListOK: rtest.DEFAULT, + }, + "service test write with workload identity and identity policy ": { + Rules: `service "test" { policy = "write" } identity "test-identity" { policy = "read" }`, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + Identity: "test-identity", + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.ALLOW, + ListOK: rtest.DEFAULT, + }, + "service test write with workload identity and node with both node and identity policy": { + Rules: `service "test" { policy = "write" } identity "test-identity" { policy = "read" } node "test-node" { policy = "read" }`, + Data: &pbcatalog.Workload{ + Addresses: []*pbcatalog.WorkloadAddress{ + {Host: "1.1.1.1"}, + }, + Ports: map[string]*pbcatalog.WorkloadPort{ + "tcp": {Port: 8080}, + }, + NodeName: "test-node", + Identity: "test-identity", + }, + Typ: pbcatalog.WorkloadType, + ReadOK: rtest.ALLOW, + WriteOK: rtest.ALLOW, + ListOK: rtest.DEFAULT, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + rtest.RunACLTestCase(t, tc, registry) + }) + } +} diff --git a/internal/mesh/internal/mappers/workloadselectionmapper/workload_selection_mapper.go b/internal/mesh/internal/mappers/workloadselectionmapper/workload_selection_mapper.go index 533474e6522a..7b064248414b 100644 --- a/internal/mesh/internal/mappers/workloadselectionmapper/workload_selection_mapper.go +++ b/internal/mesh/internal/mappers/workloadselectionmapper/workload_selection_mapper.go @@ -6,29 +6,21 @@ package workloadselectionmapper import ( "context" - "google.golang.org/protobuf/proto" - + "github.com/hashicorp/consul/internal/catalog" "github.com/hashicorp/consul/internal/controller" "github.com/hashicorp/consul/internal/mesh/internal/mappers/common" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/mappers/selectiontracker" "github.com/hashicorp/consul/lib/stringslice" - pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" ) -// WorkloadSelecting denotes a resource type that uses workload selectors. -type WorkloadSelecting interface { - proto.Message - GetWorkloads() *pbcatalog.WorkloadSelector -} - -type Mapper[T WorkloadSelecting] struct { +type Mapper[T catalog.WorkloadSelecting] struct { workloadSelectionTracker *selectiontracker.WorkloadSelectionTracker computedType *pbresource.Type } -func New[T WorkloadSelecting](computedType *pbresource.Type) *Mapper[T] { +func New[T catalog.WorkloadSelecting](computedType *pbresource.Type) *Mapper[T] { if computedType == nil { panic("computed type is required") } diff --git a/internal/mesh/internal/types/destination_policy.go b/internal/mesh/internal/types/destination_policy.go index 75a6b9f18fb0..68b37345baf3 100644 --- a/internal/mesh/internal/types/destination_policy.go +++ b/internal/mesh/internal/types/destination_policy.go @@ -24,7 +24,7 @@ func RegisterDestinationPolicy(r resource.Registry) { ACLs: &resource.ACLHooks{ Read: aclReadHookDestinationPolicy, Write: aclWriteHookDestinationPolicy, - List: aclListHookDestinationPolicy, + List: resource.NoOpACLListHook, }, }) } @@ -233,9 +233,3 @@ func aclWriteHookDestinationPolicy(authorizer acl.Authorizer, authzContext *acl. // Check service:write permissions on the service this is controlling. return authorizer.ToAllowAuthorizer().ServiceWriteAllowed(serviceName, authzContext) } - -func aclListHookDestinationPolicy(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/mesh/internal/types/destinations.go b/internal/mesh/internal/types/destinations.go index 657aa33cb0a0..f469d1b14074 100644 --- a/internal/mesh/internal/types/destinations.go +++ b/internal/mesh/internal/types/destinations.go @@ -20,6 +20,7 @@ func RegisterDestinations(r resource.Registry) { Scope: resource.ScopeNamespace, Mutate: MutateDestinations, Validate: ValidateDestinations, + ACLs: catalog.ACLHooksForWorkloadSelectingType[*pbmesh.Destinations](), }) } diff --git a/internal/mesh/internal/types/destinations_configuration.go b/internal/mesh/internal/types/destinations_configuration.go index b5de19d029b5..fedbe40df48c 100644 --- a/internal/mesh/internal/types/destinations_configuration.go +++ b/internal/mesh/internal/types/destinations_configuration.go @@ -7,17 +7,19 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/consul/internal/catalog" + "github.com/hashicorp/consul/internal/resource" pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" ) -func RegisterUpstreamsConfiguration(r resource.Registry) { +func RegisterDestinationsConfiguration(r resource.Registry) { r.Register(resource.Registration{ Type: pbmesh.DestinationsConfigurationType, Proto: &pbmesh.DestinationsConfiguration{}, Scope: resource.ScopeNamespace, Validate: ValidateDestinationsConfiguration, + ACLs: catalog.ACLHooksForWorkloadSelectingType[*pbmesh.DestinationsConfiguration](), }) } diff --git a/internal/mesh/internal/types/destinations_configuration_test.go b/internal/mesh/internal/types/destinations_configuration_test.go index 29c7c7cae0f1..5a6d3ff73289 100644 --- a/internal/mesh/internal/types/destinations_configuration_test.go +++ b/internal/mesh/internal/types/destinations_configuration_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" + catalogtesthelpers "github.com/hashicorp/consul/internal/catalog/catalogtest/helpers" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" @@ -16,6 +17,16 @@ import ( "github.com/hashicorp/consul/sdk/testutil" ) +func TestDestinationsConfigurationACLs(t *testing.T) { + catalogtesthelpers.RunWorkloadSelectingTypeACLsTests[*pbmesh.DestinationsConfiguration](t, pbmesh.DestinationsConfigurationType, + func(selector *pbcatalog.WorkloadSelector) *pbmesh.DestinationsConfiguration { + return &pbmesh.DestinationsConfiguration{Workloads: selector} + }, + func(registry resource.Registry) { + RegisterDestinationsConfiguration(registry) + }) +} + func TestValidateDestinationsConfiguration(t *testing.T) { type testcase struct { data *pbmesh.DestinationsConfiguration diff --git a/internal/mesh/internal/types/destinations_test.go b/internal/mesh/internal/types/destinations_test.go index 2601e884df07..89c44963e854 100644 --- a/internal/mesh/internal/types/destinations_test.go +++ b/internal/mesh/internal/types/destinations_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" + catalogtesthelpers "github.com/hashicorp/consul/internal/catalog/catalogtest/helpers" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" @@ -17,7 +18,7 @@ import ( "github.com/hashicorp/consul/sdk/testutil" ) -func TestMutateUpstreams(t *testing.T) { +func TestMutateDestinations(t *testing.T) { type testcase struct { tenancy *pbresource.Tenancy data *pbmesh.Destinations @@ -248,3 +249,13 @@ func TestValidateUpstreams(t *testing.T) { }) } } + +func TestDestinationsACLs(t *testing.T) { + catalogtesthelpers.RunWorkloadSelectingTypeACLsTests[*pbmesh.Destinations](t, pbmesh.DestinationsType, + func(selector *pbcatalog.WorkloadSelector) *pbmesh.Destinations { + return &pbmesh.Destinations{Workloads: selector} + }, + func(registry resource.Registry) { + RegisterDestinations(registry) + }) +} diff --git a/internal/mesh/internal/types/proxy_configuration.go b/internal/mesh/internal/types/proxy_configuration.go index 0c9ac05147e1..081324d72167 100644 --- a/internal/mesh/internal/types/proxy_configuration.go +++ b/internal/mesh/internal/types/proxy_configuration.go @@ -6,10 +6,10 @@ package types import ( "math" - "github.com/hashicorp/go-multierror" - "github.com/hashicorp/consul/internal/catalog" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/consul/internal/resource" pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" @@ -23,6 +23,7 @@ func RegisterProxyConfiguration(r resource.Registry) { Scope: resource.ScopeNamespace, Mutate: MutateProxyConfiguration, Validate: ValidateProxyConfiguration, + ACLs: catalog.ACLHooksForWorkloadSelectingType[*pbmesh.ProxyConfiguration](), }) } diff --git a/internal/mesh/internal/types/proxy_configuration_test.go b/internal/mesh/internal/types/proxy_configuration_test.go index c504ff2bd03d..3d332ca0d377 100644 --- a/internal/mesh/internal/types/proxy_configuration_test.go +++ b/internal/mesh/internal/types/proxy_configuration_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/structpb" + catalogtesthelpers "github.com/hashicorp/consul/internal/catalog/catalogtest/helpers" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" @@ -20,6 +21,16 @@ import ( "github.com/hashicorp/consul/sdk/testutil" ) +func TestProxyConfigurationACLs(t *testing.T) { + catalogtesthelpers.RunWorkloadSelectingTypeACLsTests[*pbmesh.ProxyConfiguration](t, pbmesh.ProxyConfigurationType, + func(selector *pbcatalog.WorkloadSelector) *pbmesh.ProxyConfiguration { + return &pbmesh.ProxyConfiguration{Workloads: selector} + }, + func(registry resource.Registry) { + RegisterProxyConfiguration(registry) + }) +} + func TestMutateProxyConfiguration(t *testing.T) { cases := map[string]struct { data *pbmesh.ProxyConfiguration diff --git a/internal/mesh/internal/types/proxy_state_template.go b/internal/mesh/internal/types/proxy_state_template.go index 010d0f9591b3..b84d0e9b45cb 100644 --- a/internal/mesh/internal/types/proxy_state_template.go +++ b/internal/mesh/internal/types/proxy_state_template.go @@ -44,11 +44,7 @@ func RegisterProxyStateTemplate(r resource.Registry) { // managed by a controller. return authorizer.ToAllowAuthorizer().OperatorWriteAllowed(authzContext) }, - 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 - }, + List: resource.NoOpACLListHook, }, }) } diff --git a/internal/mesh/internal/types/types.go b/internal/mesh/internal/types/types.go index 20b23604fc6e..cf1443aabf18 100644 --- a/internal/mesh/internal/types/types.go +++ b/internal/mesh/internal/types/types.go @@ -12,11 +12,12 @@ func Register(r resource.Registry) { RegisterComputedProxyConfiguration(r) RegisterDestinations(r) RegisterComputedExplicitDestinations(r) - RegisterUpstreamsConfiguration(r) RegisterProxyStateTemplate(r) RegisterHTTPRoute(r) RegisterTCPRoute(r) RegisterGRPCRoute(r) RegisterDestinationPolicy(r) RegisterComputedRoutes(r) + // todo (v2): uncomment once we implement it. + //RegisterDestinationsConfiguration(r) } diff --git a/internal/mesh/internal/types/types_test.go b/internal/mesh/internal/types/types_test.go index 631e1b4be8d0..801d3de01846 100644 --- a/internal/mesh/internal/types/types_test.go +++ b/internal/mesh/internal/types/types_test.go @@ -21,13 +21,14 @@ func TestTypeRegistration(t *testing.T) { requiredKinds := []string{ pbmesh.ProxyConfigurationKind, pbmesh.DestinationsKind, - pbmesh.DestinationsConfigurationKind, pbmesh.ProxyStateTemplateKind, pbmesh.HTTPRouteKind, pbmesh.TCPRouteKind, pbmesh.GRPCRouteKind, pbmesh.DestinationPolicyKind, pbmesh.ComputedRoutesKind, + // todo (v2): re-enable once we implement it. + //pbmesh.DestinationsConfigurationKind, } r := resource.NewRegistry() diff --git a/internal/mesh/internal/types/xroute.go b/internal/mesh/internal/types/xroute.go index 1c60bdcb1c54..619c9cb68243 100644 --- a/internal/mesh/internal/types/xroute.go +++ b/internal/mesh/internal/types/xroute.go @@ -290,7 +290,7 @@ func xRouteACLHooks[R XRouteData]() *resource.ACLHooks { hooks := &resource.ACLHooks{ Read: aclReadHookXRoute[R], Write: aclWriteHookXRoute[R], - List: aclListHookXRoute[R], + List: resource.NoOpACLListHook, } return hooks @@ -298,7 +298,7 @@ func xRouteACLHooks[R XRouteData]() *resource.ACLHooks { func aclReadHookXRoute[R XRouteData](authorizer acl.Authorizer, _ *acl.AuthorizerContext, _ *pbresource.ID, res *pbresource.Resource) error { if res == nil { - return resource.ErrNeedData + return resource.ErrNeedResource } dec, err := resource.Decode[R](res) @@ -351,9 +351,3 @@ func aclWriteHookXRoute[R XRouteData](authorizer acl.Authorizer, _ *acl.Authoriz return nil } - -func aclListHookXRoute[R XRouteData](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/mesh/internal/types/xroute_test.go b/internal/mesh/internal/types/xroute_test.go index 09806bea8c7e..cd6d6d766327 100644 --- a/internal/mesh/internal/types/xroute_test.go +++ b/internal/mesh/internal/types/xroute_test.go @@ -458,7 +458,7 @@ func testXRouteACLs[R XRouteData](t *testing.T, newRoute func(t *testing.T, pare require.True(t, ok) err = reg.ACLs.Read(authz, &acl.AuthorizerContext{}, tc.res.Id, nil) - require.ErrorIs(t, err, resource.ErrNeedData, "read hook should require the data payload") + require.ErrorIs(t, err, resource.ErrNeedResource, "read hook should require the data payload") checkF(t, "read", tc.readOK, reg.ACLs.Read(authz, &acl.AuthorizerContext{}, tc.res.Id, tc.res)) checkF(t, "write", tc.writeOK, reg.ACLs.Write(authz, &acl.AuthorizerContext{}, tc.res)) diff --git a/internal/resource/acls.go b/internal/resource/acls.go new file mode 100644 index 000000000000..55a5872fc0de --- /dev/null +++ b/internal/resource/acls.go @@ -0,0 +1,13 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package resource + +import "github.com/hashicorp/consul/acl" + +// NoOpACLListHook is a common function that can be used if no special list permission is required for a resource. +func NoOpACLListHook(_ acl.Authorizer, _ *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 b6a9263842d3..8e978c9fb49a 100644 --- a/internal/resource/demo/demo.go +++ b/internal/resource/demo/demo.go @@ -97,7 +97,7 @@ func RegisterTypes(r resource.Registry) { readACL := func(authz acl.Authorizer, authzContext *acl.AuthorizerContext, id *pbresource.ID, res *pbresource.Resource) error { if resource.EqualType(TypeV1RecordLabel, id.Type) { if res == nil { - return resource.ErrNeedData + return resource.ErrNeedResource } } key := fmt.Sprintf("resource/%s/%s", resource.ToGVK(id.Type), id.Name) diff --git a/internal/resource/errors.go b/internal/resource/errors.go index 2003d86cbf71..279f276b2a99 100644 --- a/internal/resource/errors.go +++ b/internal/resource/errors.go @@ -136,6 +136,20 @@ type ErrOwnerTenantInvalid struct { } func (err ErrOwnerTenantInvalid) Error() string { + if err.ResourceTenancy == nil && err.OwnerTenancy != nil { + return fmt.Sprintf( + "empty resource tenancy cannot be owned by a resource in partition %s, namespace %s and peer %s", + err.OwnerTenancy.Partition, err.OwnerTenancy.Namespace, err.OwnerTenancy.PeerName, + ) + } + + if err.ResourceTenancy != nil && err.OwnerTenancy == nil { + return fmt.Sprintf( + "resource in partition %s, namespace %s and peer %s cannot be owned by a resource with empty tenancy", + err.ResourceTenancy.Partition, err.ResourceTenancy.Namespace, err.ResourceTenancy.PeerName, + ) + } + return fmt.Sprintf( "resource in partition %s, namespace %s and peer %s cannot be owned by a resource in partition %s, namespace %s and peer %s", err.ResourceTenancy.Partition, err.ResourceTenancy.Namespace, err.ResourceTenancy.PeerName, diff --git a/internal/resource/registry.go b/internal/resource/registry.go index 2b004b6b4c0c..20c1f4dc41a8 100644 --- a/internal/resource/registry.go +++ b/internal/resource/registry.go @@ -68,14 +68,14 @@ type Registration struct { Scope Scope } -var ErrNeedData = errors.New("authorization check requires resource data") +var ErrNeedResource = errors.New("authorization check requires the entire resource") type ACLHooks struct { // Read is used to authorize Read RPCs and to filter results in List // RPCs. // // It can be called an ID and possibly a Resource. The check will first - // attempt to use the ID and if the hook returns ErrNeedData, then the + // attempt to use the ID and if the hook returns ErrNeedResource, then the // check will be deferred until the data is fetched from the storage layer. // // If it is omitted, `operator:read` permission is assumed. diff --git a/internal/resource/resourcetest/acls.go b/internal/resource/resourcetest/acls.go new file mode 100644 index 000000000000..c73f3c4752d4 --- /dev/null +++ b/internal/resource/resourcetest/acls.go @@ -0,0 +1,81 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package resourcetest + +import ( + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/reflect/protoreflect" + + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/proto-public/pbresource" +) + +const ( + DENY = "deny" + ALLOW = "allow" + DEFAULT = "default" +) + +var checkF = func(t *testing.T, expect string, got error) { + switch expect { + case ALLOW: + if acl.IsErrPermissionDenied(got) { + t.Fatal("should be allowed") + } + case DENY: + if !acl.IsErrPermissionDenied(got) { + t.Fatal("should be denied") + } + case DEFAULT: + require.Nil(t, got, "expected fallthrough decision") + default: + t.Fatalf("unexpected expectation: %q", expect) + } +} + +type ACLTestCase struct { + Rules string + Data protoreflect.ProtoMessage + Owner *pbresource.ID + Typ *pbresource.Type + ReadOK string + WriteOK string + ListOK string +} + +func RunACLTestCase(t *testing.T, tc ACLTestCase, registry resource.Registry) { + reg, ok := registry.Resolve(tc.Typ) + require.True(t, ok) + + res := Resource(tc.Typ, "test"). + WithTenancy(resource.DefaultNamespacedTenancy()). + WithOwner(tc.Owner). + WithData(t, tc.Data). + Build() + ValidateAndNormalize(t, registry, res) + + config := acl.Config{ + WildcardName: structs.WildcardSpecifier, + } + authz, err := acl.NewAuthorizerFromRules(tc.Rules, &config, nil) + require.NoError(t, err) + authz = acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.DenyAll()}) + + t.Run("read", func(t *testing.T) { + err := reg.ACLs.Read(authz, &acl.AuthorizerContext{}, res.Id, res) + checkF(t, tc.ReadOK, err) + }) + t.Run("write", func(t *testing.T) { + err := reg.ACLs.Write(authz, &acl.AuthorizerContext{}, res) + checkF(t, tc.WriteOK, err) + }) + t.Run("list", func(t *testing.T) { + err := reg.ACLs.List(authz, &acl.AuthorizerContext{}) + checkF(t, tc.ListOK, err) + }) +}