Skip to content

Conversation

@robhudson
Copy link
Contributor

See the docs/access_controls.md and docs/cli.md files here for a bit more details.

This doesn't currently add any permissions or permission dependencies to any routes, that will come later.

@robhudson robhudson requested a review from a team as a code owner February 27, 2025 01:10
@robhudson robhudson added the enhancement New feature or request label Feb 27, 2025
@robhudson
Copy link
Contributor Author

@leplatrem or @grahamalama: Curious if either of you would have time for a quick review? I know +2,312 lines isn't quick but perhaps a high level review?

Copy link
Contributor

@leplatrem leplatrem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry Rob for the delay.

This is great! Super clean 👌

I have one question about the permissions. Since permissions are meant to be used in code, do you think it's relevant to expose their manipulation to admins? Intuitively I would either create them on the fly when a view refers to it, and put their creation in migration code so that they get created automatically when a new version is deployed. What do you think?

@robhudson
Copy link
Contributor Author

I have one question about the permissions. Since permissions are meant to be used in code, do you think it's relevant to expose their manipulation to admins?

I think having a CLI to manage permissions and roles is valuable, as it provides flexibility for admins when needed. That said, I agree that the most common approach would be to add permissions via migrations alongside the code that uses them. This ensures they are always created when deploying a new version, keeping them in sync with the application logic.

@robhudson robhudson force-pushed the one-role-to-rule-them-all branch 2 times, most recently from 748e42c to 8384c91 Compare March 11, 2025 17:21
@robhudson robhudson force-pushed the one-role-to-rule-them-all branch from 8384c91 to 31ded95 Compare March 13, 2025 23:14
@robhudson robhudson merged commit cb81c7c into main Mar 13, 2025
5 checks passed
@robhudson robhudson deleted the one-role-to-rule-them-all branch March 13, 2025 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants