From e913c1c9dbc545951344bdccbd00c0bc776c4b1f Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 25 Oct 2024 16:18:47 +0530 Subject: [PATCH] test: verify resource permissions are asserted correctly (#798) Config has additional flag to debug permission checks which can be enabled via `spicedb.check_trace:true` Signed-off-by: Kush Sharma --- cmd/serve.go | 2 +- config/sample.config.yaml | 3 + docs/docs/reference/configurations.md | 3 + internal/store/spicedb/config.go | 4 + internal/store/spicedb/relation_repository.go | 17 +- test/e2e/regression/api_test.go | 248 ++++++++++++++++++ 6 files changed, 274 insertions(+), 3 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 39362f575..16d6ca64b 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -331,7 +331,7 @@ func buildAPIDependencies( namespaceService := namespace.NewService(namespaceRepository) authzSchemaRepository := spicedb.NewSchemaRepository(logger, sdb) - authzRelationRepository := spicedb.NewRelationRepository(sdb, cfg.SpiceDB.FullyConsistent) + authzRelationRepository := spicedb.NewRelationRepository(sdb, cfg.SpiceDB.FullyConsistent, cfg.SpiceDB.CheckTrace) permissionRepository := postgres.NewPermissionRepository(dbc) permissionService := permission.NewService(permissionRepository) diff --git a/config/sample.config.yaml b/config/sample.config.yaml index 9981f01cf..10b36a7af 100644 --- a/config/sample.config.yaml +++ b/config/sample.config.yaml @@ -163,6 +163,9 @@ spicedb: # fully_consistent ensures APIs although slower than usual will result in responses always most consistent # suggested to keep it false for performance fully_consistent: false + # check_trace enables tracing in check api for spicedb, it adds considerable + # latency to the check calls and shouldn't be enabled in production + check_trace: false billing: # stripe key to be used for billing diff --git a/docs/docs/reference/configurations.md b/docs/docs/reference/configurations.md index f323703e3..56fd27faf 100644 --- a/docs/docs/reference/configurations.md +++ b/docs/docs/reference/configurations.md @@ -158,6 +158,9 @@ spicedb: # fully_consistent ensures APIs although slower than usual will result in responses always most consistent # suggested to keep it false for performance fully_consistent: false + # check_trace enables tracing in check api for spicedb, it adds considerable + # latency to the check calls and shouldn't be enabled in production + check_trace: false billing: # stripe key to be used for billing diff --git a/internal/store/spicedb/config.go b/internal/store/spicedb/config.go index a901239bd..5e327caba 100644 --- a/internal/store/spicedb/config.go +++ b/internal/store/spicedb/config.go @@ -7,4 +7,8 @@ type Config struct { // FullyConsistent ensures APIs although slower than usual will result in responses always most consistent FullyConsistent bool `yaml:"fully_consistent" mapstructure:"fully_consistent" default:"false"` + + // CheckTrace enables tracing in check api for spicedb, it adds considerable + // latency to the check calls and shouldn't be enabled in production + CheckTrace bool `yaml:"check_trace" mapstructure:"check_trace" default:"false"` } diff --git a/internal/store/spicedb/relation_repository.go b/internal/store/spicedb/relation_repository.go index a76303d48..f2e4fdb84 100644 --- a/internal/store/spicedb/relation_repository.go +++ b/internal/store/spicedb/relation_repository.go @@ -2,10 +2,14 @@ package spicedb import ( "context" + "encoding/json" "errors" "fmt" "io" + grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" + "go.uber.org/zap" + authzedpb "github.com/authzed/authzed-go/proto/authzed/api/v1" newrelic "github.com/newrelic/go-agent" "github.com/raystack/frontier/core/relation" @@ -18,6 +22,9 @@ type RelationRepository struct { // turning it on will result in slower API calls but useful in tests fullyConsistent bool + // tracing enables debug traces for check calls + tracing bool + // TODO(kushsharma): after every call, check if the response returns a relationship // snapshot(zedtoken/zookie), if it does, store it in a cache/db, and use it for subsequent calls // this will make the calls faster and avoid the use of fully consistent spiceDB @@ -25,10 +32,11 @@ type RelationRepository struct { const nrProductName = "spicedb" -func NewRelationRepository(spiceDB *SpiceDB, fullyConsistent bool) *RelationRepository { +func NewRelationRepository(spiceDB *SpiceDB, fullyConsistent bool, tracing bool) *RelationRepository { return &RelationRepository{ spiceDB: spiceDB, fullyConsistent: fullyConsistent, + tracing: tracing, } } @@ -92,7 +100,8 @@ func (r RelationRepository) Check(ctx context.Context, rel relation.Relation) (b }, OptionalRelation: rel.Subject.SubRelationName, }, - Permission: rel.RelationName, + Permission: rel.RelationName, + WithTracing: r.tracing, } nrCtx := newrelic.FromContext(ctx) @@ -110,6 +119,10 @@ func (r RelationRepository) Check(ctx context.Context, rel relation.Relation) (b if err != nil { return false, err } + if response.GetDebugTrace() != nil { + str, _ := json.Marshal(response.GetDebugTrace()) + grpczap.Extract(ctx).Info("CheckPermission", zap.String("trace", string(str))) + } return response.GetPermissionship() == authzedpb.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION, nil } diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index cbda09264..2fdc003fb 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -127,6 +127,7 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { }) s.Assert().NoError(err) s.Assert().Equal(1, len(orgCreatedPolicies.GetPolicies())) + s.Assert().True(!orgCreatedPolicies.GetPolicies()[0].GetCreatedAt().AsTime().IsZero()) }) s.Run("2. user attached to an org as member should have no basic permission other than membership", func() { createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, &frontierv1beta1.CreateOrganizationRequest{ @@ -1635,6 +1636,253 @@ func (s *APIRegressionTestSuite) TestResourceAPI() { s.Assert().NoError(err) s.Assert().False(deletePermResp.GetStatus()) }) + s.Run("3. run time permissions and roles assigned over resources should enforce correctly", func() { + // create org + createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, &frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{ + Title: "org 3", + Name: "org-resource-3", + }, + }) + s.Assert().NoError(err) + + // create users + user1Resp, err := s.testBench.Client.CreateUser(ctxOrgAdminAuth, &frontierv1beta1.CreateUserRequest{Body: &frontierv1beta1.UserRequestBody{ + Title: "member 1", + Email: "user-org-3-resource-1@raystack.org", + Name: "user_org_3_resource_1", + }}) + s.Assert().NoError(err) + user2Resp, err := s.testBench.Client.CreateUser(ctxOrgAdminAuth, &frontierv1beta1.CreateUserRequest{Body: &frontierv1beta1.UserRequestBody{ + Title: "member 2", + Email: "user-org-3-resource-2@raystack.org", + Name: "user_org_3_resource_2", + }}) + s.Assert().NoError(err) + + // create a project within org + createProjResp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, &frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{ + Name: "org-3-proj-1", + OrgId: createOrgResp.GetOrganization().GetId(), + }, + }) + s.Assert().NoError(err) + + // create permission for a resource type + resourceNamespace := "compute/network" + createdPermissions, err := s.testBench.AdminClient.CreatePermission(ctxOrgAdminAuth, &frontierv1beta1.CreatePermissionRequest{ + Bodies: []*frontierv1beta1.PermissionRequestBody{ + { + Key: "compute.network.create", + }, + { + Key: "compute.network.delete", + }, + }, + }) + s.Assert().NoError(err) + s.Assert().Len(createdPermissions.GetPermissions(), 2) + + // create a role at project level without resource access + projectViewerRoleResp, err := s.testBench.AdminClient.CreateRole(ctxOrgAdminAuth, &frontierv1beta1.CreateRoleRequest{ + Body: &frontierv1beta1.RoleRequestBody{ + Name: "project_viewer_custom", + Permissions: []string{ + "app_project_get", + "app_project_resourcelist", + }, + }, + }) + s.Assert().NoError(err) + s.Assert().NotNil(projectViewerRoleResp) + + // create a role at project level with resource create access + projectManagerRoleResp, err := s.testBench.AdminClient.CreateRole(ctxOrgAdminAuth, &frontierv1beta1.CreateRoleRequest{ + Body: &frontierv1beta1.RoleRequestBody{ + Name: "project_manager_custom", + Permissions: []string{ + "app_project_get", + "app_project_resourcelist", + "compute.network.create", + }, + }, + }) + s.Assert().NoError(err) + s.Assert().NotNil(projectManagerRoleResp) + + // create a role at project level with resource delete access + projectOwnerRoleResp, err := s.testBench.AdminClient.CreateRole(ctxOrgAdminAuth, &frontierv1beta1.CreateRoleRequest{ + Body: &frontierv1beta1.RoleRequestBody{ + Name: "project_owner_custom", + Permissions: []string{ + "app_project_get", + "app_project_resourcelist", + "compute.network.create", + "compute.network.delete", + }, + }, + }) + s.Assert().NoError(err) + s.Assert().NotNil(projectOwnerRoleResp) + + // create a resource under the project + createResource1Resp, err := s.testBench.Client.CreateProjectResource(ctxOrgAdminAuth, &frontierv1beta1.CreateProjectResourceRequest{ + ProjectId: createProjResp.GetProject().GetId(), + Body: &frontierv1beta1.ResourceRequestBody{ + Name: "res-1", + Namespace: resourceNamespace, + }, + }) + s.Assert().NoError(err) + s.Assert().NotNil(createResource1Resp) + + // assign project viewer role to user + _, err = s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, &frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{ + RoleId: projectViewerRoleResp.GetRole().GetId(), + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjResp.GetProject().GetId()), + Principal: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }, + }) + s.Assert().NoError(err) + + // by default no user should have access to it + checkCreatePermResp, err := s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(resourceNamespace, createResource1Resp.GetResource().GetId()), + Permission: "compute.network.create", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().False(checkCreatePermResp.GetStatus()) + checkCreatePermOnProjectResp, err := s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjResp.GetProject().GetId()), + Permission: "compute.network.create", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().False(checkCreatePermOnProjectResp.GetStatus()) + + // assign project manager to the user + _, err = s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, &frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{ + RoleId: projectManagerRoleResp.GetRole().GetId(), + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjResp.GetProject().GetId()), + Principal: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }, + }) + s.Assert().NoError(err) + + // user now should have access to create but not delete + checkCreatePermResp, err = s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjResp.GetProject().GetId()), + Permission: "compute.network.create", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().True(checkCreatePermResp.GetStatus()) + checkDeletePermResp, err := s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(resourceNamespace, createResource1Resp.GetResource().GetId()), + Permission: "compute.network.delete", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().False(checkDeletePermResp.GetStatus()) + + // make user project owner + _, err = s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, &frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{ + RoleId: projectOwnerRoleResp.GetRole().GetId(), + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjResp.GetProject().GetId()), + Principal: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }, + }) + s.Assert().NoError(err) + + // should have access to delete as well + checkDeletePermResp, err = s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(resourceNamespace, createResource1Resp.GetResource().GetId()), + Permission: "compute.network.delete", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().True(checkDeletePermResp.GetStatus()) + + // any other user shouldn't have access to it + checkCreatePermResp, err = s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(resourceNamespace, createResource1Resp.GetResource().GetId()), + Permission: "compute.network.create", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user2Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().False(checkCreatePermResp.GetStatus()) + + // remove permissions from owner role + projectOwnerUpdatedRoleResp, err := s.testBench.AdminClient.UpdateRole(ctxOrgAdminAuth, &frontierv1beta1.UpdateRoleRequest{ + Id: projectOwnerRoleResp.GetRole().GetId(), + Body: &frontierv1beta1.RoleRequestBody{ + Name: projectOwnerRoleResp.GetRole().GetName(), + Permissions: []string{ + "app_project_get", + "app_project_resourcelist", + }, + Metadata: projectOwnerRoleResp.GetRole().GetMetadata(), + Title: projectOwnerRoleResp.GetRole().GetTitle(), + Scopes: projectOwnerRoleResp.GetRole().GetScopes(), + }, + }) + s.Assert().NoError(err) + s.Assert().NotNil(projectOwnerUpdatedRoleResp) + + // user should not have access to delete anymore + checkDeletePermResp, err = s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(resourceNamespace, createResource1Resp.GetResource().GetId()), + Permission: "compute.network.delete", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user1Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().False(checkDeletePermResp.GetStatus()) + + // assigning updated owner role to user 2 should not give access to delete + _, err = s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, &frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{ + RoleId: projectOwnerUpdatedRoleResp.GetRole().GetId(), + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjResp.GetProject().GetId()), + Principal: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user2Resp.GetUser().GetId()), + }, + }) + s.Assert().NoError(err) + + checkCreatePermResp, err = s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(resourceNamespace, createResource1Resp.GetResource().GetId()), + Permission: "compute.network.create", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user2Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().False(checkCreatePermResp.GetStatus()) + + // if a user is owner of an org doesn't mean it will get access to other resources + ctxOrgUser2Auth := metadata.NewOutgoingContext(context.Background(), metadata.New(map[string]string{ + testbench.IdentityHeader: user2Resp.GetUser().GetEmail(), + })) + createUser2OrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgUser2Auth, &frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{ + Title: "org 3", + Name: "org-user-2-resource-3", + }, + }) + s.Assert().NoError(err) + s.Assert().NotEmpty(createUser2OrgResp.GetOrganization()) + + // should not have access to create + checkCreatePermResp, err = s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, &frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(resourceNamespace, createResource1Resp.GetResource().GetId()), + Permission: "compute.network.create", + Subject: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, user2Resp.GetUser().GetId()), + }) + s.Assert().NoError(err) + s.Assert().False(checkCreatePermResp.GetStatus()) + }) } func (s *APIRegressionTestSuite) TestPolicyAPI() {