Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage Alignment #29

Open
clausMeko opened this issue Apr 24, 2023 · 1 comment
Open

Usage Alignment #29

clausMeko opened this issue Apr 24, 2023 · 1 comment

Comments

@clausMeko
Copy link

Dear Maintainer,

I feel like something is not consistent but as you are in v2 I think there is a story behind.

When Assigning Permissions to Roles this is done via types func (role *StdRole) Assign(p Permission) error (Permission type).

When Inheriting Roles this is done via strings func (rbac *RBAC) SetParent(id string, parent string) error, i.e. you could have written a signature like func (rbac *RBAC) SetParent(child Role, parent Role) error.

Which would be:

  • more consistent
  • had more type safety (especially as you already use the types)

I will work around it by using roleA.ID() but this feels implicit when I need to check your code to notice IDs are the map stored things I need to use to get strings.

So please don´t read it as a complain your repo seems to be fine but is there a story behind that API design?

@mikespook
Copy link
Owner

Yes, you are right. If SetParent accepts Role as the params, it will be more consistent in the API.

Here some details of the design about current SetParent.

There are two typical phase for using this package.

  1. Preparing RBAC

In the phase, the role and permission instances are created and assigned to RBAC instance. This is the only chance that users need to touch the original role/permission instances. While the SetParent(s) belongs to Preparing phase, it should be fine to use role instances to set it up.

  1. Using RBAC to do the validation

When using RBAC to do the validation, the ID is the only thing we need to hold. In most case, the role ID is easier to get and hold internally in an application than the Role instance itself.

The most difficult part would be how to have the inheritance structure on the role instance. E.g. example here setting the inheritance by using ID is much more easier than using Role instances.

Of cause, I could save the calling of RBAC.Add and use RBAC.SetParent(s) to build the RBAC instance. I will work on it to find a better practical way.

Anyway, thank you for the valuable thought. It is really helpful.

@mikespook mikespook reopened this Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants