feat: add permission manager #1507
Conversation
kian99
force-pushed
the
permission-manager
branch
from
4bf6903 to
b9343c4
Compare
kian99
changed the title
kian99
force-pushed
the
permission-manager
branch
from
dabd03c to
18f62a8
Compare
Moved group methods to group package Moved createTestControllerEnvironment function to jimmtest package
Listing offers test was failing because model owner was only expected to have consume access on offers but should have admin access.
kian99
force-pushed
the
permission-manager
branch
from
d515fbb to
64db740
Compare
| } | ||
|
|
||
| // GetUserControllerAccess returns the user's level of access to the desired controller. | ||
| func (j *permissionManager) GetUserControllerAccess(ctx context.Context, user *openfga.User, controller names.ControllerTag) (string, error) { |
There was a problem hiding this comment.
suggestion: keep all the permissionManager method implementations in permissionmanager.go
There was a problem hiding this comment.
Do you mean just move all the methods in access.go to permissionmanager.go? What about relations.go which contains some more methods but these ones are generic OpenFGA ones.
There was a problem hiding this comment.
yes, i think it would be much cleaner to have all methods of a type in a single file..
There was a problem hiding this comment.
Let's re-organise in a separate PR then, there's a couple of ways of chopping it up.
Also updated constructor from NewPermissionManager to NewManager
| @@ -76,7 +76,7 @@ func (j *JIMM) CheckRelation(ctx context.Context, user *openfga.User, tuple apip | |||
|
|
|||
| // ListRelationshipTuples checks user permission and lists relationship tuples based of tuple struct with pagination. | |||
| // Listing filters can be relaxed: optionally exclude tuple.Relation or tuple.Object or specify only tuple.TargetObject.Kind. | |||
| func (j *JIMM) ListRelationshipTuples(ctx context.Context, user *openfga.User, tuple apiparams.RelationshipTuple, pageSize int32, continuationToken string) ([]openfga.Tuple, string, error) { | |||
| func (j *permissionManager) ListRelationshipTuples(ctx context.Context, user *openfga.User, tuple apiparams.RelationshipTuple, pageSize int32, continuationToken string) ([]openfga.Tuple, string, error) { | |||
There was a problem hiding this comment.
remembering our discussion the other day about exposing dbmodel types in manager method, this is doing something similar, but exposing the underlying openfga.Tuple type. Eventually we might want to consider decoupling..
| } | ||
|
|
||
| // GetUserControllerAccess returns the user's level of access to the desired controller. | ||
| func (j *permissionManager) GetUserControllerAccess(ctx context.Context, user *openfga.User, controller names.ControllerTag) (string, error) { |
There was a problem hiding this comment.
yes, i think it would be much cleaner to have all methods of a type in a single file..
Description
This PR builds on #1506.
This PR introduces a permission manager package dedicated to granting/revoking access and checking a user's permissions.
Some notable changes:
jujuauthnow uses a factory pattern since this is a stateful object that needs to be recreated for each websocket connection. After introducing the permission manager package this simplifies how much access the API layer needs to the underlying business layer structs.yaml-> DB approach for creating test objects.The rest of the changes are largely mechanical but unfortunately very long because we had access related logic in many different places.
The changes are easier to review commit-by-commit (or at least the first commit on its own then the rest separately).
Fixes JUJU-7252
Engineering checklist