Create permissions for a locked down user role #314
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nazywam
approved these changes
Dec 27, 2022
docs/users.md
Outdated
|
|
||
| This role names are considered stable, and will continue to work in the future. | ||
|
|
||
| User permissions are then split in more fine-grained permissions: |
There was a problem hiding this comment.
Suggested change
| User permissions are then split in more fine-grained permissions: | |
| User permissions are then split into more fine-grained permissions: |
| @@ -158,14 +158,14 @@ def __call__(self, user: User = Depends(current_user)): | |||
| default_roles = [ | |||
| role.strip() for role in auth_default_roles.split(",") | |||
| ] | |||
| all_user_roles = list(set(user_roles + default_roles)) | |||
| all_roles = list(set(user_roles + default_roles)) | |||
| all_roles = sum((expand_role(role) for role in all_roles), []) | |||
There was a problem hiding this comment.
Just making sure that the sum is here used on purpose? The usage looks super weird to me
There was a problem hiding this comment.
yes, this is basically concat (summing all the given lists). The idea is to get, well, list of all applicable permissions, instead a list of expanded permissions.
For example:
>>> test = [[1, 2, 3], [4, 5, 6], [7, 8]]
>>> sum(test, [])
[1, 2, 3, 4, 5, 6, 7, 8]
I'm not aware of a better idiom for concat in python. If that's unclear I can rewrite it to explicit for loop, or a bit complex list comprehension - I think this will work in this case too:
[perm for role in all_roles for perm in expand(role)]
Co-authored-by: Michał Praszmo <[email protected]>
Co-authored-by: Michał Praszmo <[email protected]>
Co-authored-by: Michał Praszmo <[email protected]>
Co-authored-by: Michał Praszmo <[email protected]>
Co-authored-by: Michał Praszmo <[email protected]>
Co-authored-by: Michał Praszmo <[email protected]>
yankovs
reviewed
Dec 28, 2022
Co-authored-by: Sharon Yankovich <[email protected]>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Your checklist for this pull request
What is the current behaviour?
There is no way to give more fine-grained permissions - everyone is
either an admin or a regulra user.
What is the new behaviour?
As documented in docs/users.md, the user role now is split into four sub-roles
can_view_queries: Can view a query with a given ID, and matched files.can_manage_queries: Can create, stop, and delete queries.can_list_queries: Can list queries (for "recent jobs" tab).can_download_files: Can download matched file contents.That list of sub-permissions is not considered stable yet - in future PR
I may decide to rename, remove or add some of them.
Test plan
Closing issues
fixes #302