From 211c29ff9293c6c9f76264053246a89704db3ec5 Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Fri, 14 Jul 2023 14:03:28 -0500 Subject: [PATCH] ensure permissions checks are fully consistent (#141) Signed-off-by: Mike Mason --- internal/api/permissions.go | 2 +- internal/query/mock/mock.go | 2 +- internal/query/relations.go | 15 ++++++++------- internal/query/relations_test.go | 4 ++-- internal/query/service.go | 2 +- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/api/permissions.go b/internal/api/permissions.go index b0811baa..29005012 100644 --- a/internal/api/permissions.go +++ b/internal/api/permissions.go @@ -59,7 +59,7 @@ func (r *Router) checkAction(c echo.Context) error { } // Check the permissions - err = r.engine.SubjectHasPermission(ctx, subjectResource, action, resource, "") + err = r.engine.SubjectHasPermission(ctx, subjectResource, action, resource) if err != nil && errors.Is(err, query.ErrActionNotAssigned) { msg := fmt.Sprintf("subject '%s' does not have permission to perform action '%s' on resource '%s'", subject, action, resourceIDStr) diff --git a/internal/query/mock/mock.go b/internal/query/mock/mock.go index e52f1859..72b1a6ad 100644 --- a/internal/query/mock/mock.go +++ b/internal/query/mock/mock.go @@ -139,7 +139,7 @@ func (e *Engine) GetResourceType(name string) *types.ResourceType { } // SubjectHasPermission returns nil to satisfy the Engine interface. -func (e *Engine) SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource, queryToken string) error { +func (e *Engine) SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource) error { e.Called() return nil diff --git a/internal/query/relations.go b/internal/query/relations.go index 0b5ab81d..82c177ef 100644 --- a/internal/query/relations.go +++ b/internal/query/relations.go @@ -60,8 +60,13 @@ func resourceToSpiceDBRef(namespace string, r types.Resource) *pb.ObjectReferenc } // SubjectHasPermission checks if the given subject can do the given action on the given resource -func (e *engine) SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource, queryToken string) error { +func (e *engine) SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource) error { req := &pb.CheckPermissionRequest{ + Consistency: &pb.Consistency{ + Requirement: &pb.Consistency_FullyConsistent{ + FullyConsistent: true, + }, + }, Resource: resourceToSpiceDBRef(e.namespace, resource), Permission: action, Subject: &pb.SubjectReference{ @@ -69,7 +74,7 @@ func (e *engine) SubjectHasPermission(ctx context.Context, subject types.Resourc }, } - return e.checkPermission(ctx, req, queryToken) + return e.checkPermission(ctx, req) } // AssignSubjectRole assigns the given role to the given subject. @@ -170,11 +175,7 @@ func (e *engine) subjectRoleRelDelete(subject types.Resource, role types.Role) * } } -func (e *engine) checkPermission(ctx context.Context, req *pb.CheckPermissionRequest, queryToken string) error { - if queryToken != "" { - req.Consistency = &pb.Consistency{Requirement: &pb.Consistency_AtLeastAsFresh{AtLeastAsFresh: &pb.ZedToken{Token: queryToken}}} - } - +func (e *engine) checkPermission(ctx context.Context, req *pb.CheckPermissionRequest) error { resp, err := e.client.CheckPermission(ctx, req) if err != nil { return err diff --git a/internal/query/relations_test.go b/internal/query/relations_test.go index 376d1bf5..4a79ad00 100644 --- a/internal/query/relations_test.go +++ b/internal/query/relations_test.go @@ -523,7 +523,7 @@ func TestSubjectActions(t *testing.T) { }, ) assert.NoError(t, err) - queryToken, err := e.AssignSubjectRole(ctx, subjRes, role) + _, err = e.AssignSubjectRole(ctx, subjRes, role) assert.NoError(t, err) type testInput struct { @@ -565,7 +565,7 @@ func TestSubjectActions(t *testing.T) { } testFn := func(ctx context.Context, input testInput) testingx.TestResult[any] { - err := e.SubjectHasPermission(ctx, subjRes, input.action, input.resource, queryToken) + err := e.SubjectHasPermission(ctx, subjRes, input.action, input.resource) return testingx.TestResult[any]{ Err: err, diff --git a/internal/query/service.go b/internal/query/service.go index e15b7e17..a802cb44 100644 --- a/internal/query/service.go +++ b/internal/query/service.go @@ -25,7 +25,7 @@ type Engine interface { DeleteRelationships(ctx context.Context, resource types.Resource) (string, error) NewResourceFromID(id gidx.PrefixedID) (types.Resource, error) GetResourceType(name string) *types.ResourceType - SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource, queryToken string) error + SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource) error } type engine struct {