Add permission checks #625
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
sevein
force-pushed
the
dev/issue-1485-permissions-take-two
branch
9 times, most recently
from
491ed6d
to
adc572d
Compare
sevein
force-pushed
the
dev/issue-1485-permissions-take-two
branch
7 times, most recently
from
93a13bb
to
f8d11e4
Compare
sevein
force-pushed
the
dev/issue-1485-permissions-take-two
branch
from
f8d11e4
to
e351c37
Compare
replaceafill
approved these changes
Jan 11, 2022
There was a problem hiding this comment.
This looks great to me @sevein! I just left some minor observations/questions. Feel free to address them as convenient.
storage_service/administration/templates/widgets/role_choice.html
Outdated
Show resolved
Hide resolved
This commit introduces a permission model where authenticated users have read-only access, but they can be promoted as reviewers, managers or administrators - implemented as Django groups with permissions associated. The database migration promotes existing users as managers unless they were already administrators. Reviewers can accept/reject package deletion requests, an action also available to managers and administrators.
sevein
force-pushed
the
dev/issue-1485-permissions-take-two
branch
from
d36eb8d
to
172de4c
Compare
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.
This pull request introduces the concept of user roles:
is_superuserflag enabled,approve_package_deletionpermission),The new
rolesmodule provides some functions to associate these roles to application users. A new database migration ensures that the new groups are created and have the permissions linked, as well as migrating existing users so they are grante dthe Managers role automatically.Permission checks have been added where needed, including the API. In particular, the package deletion approve/reject workflow is available for Reviewers, Managers and Administrators. Certain user interface elements are hidden from the user when they're not actionable due to lack of permissions, hence the updates in multiple templates.
Our supported authentication backends have been revisited to ensure that the migration to this new permission-role model is possible. To ensure backward compatibility, a new setting (
SS_AUTH_DEFAULT_USER_ROLE="reader") has been added to automatically promote users to managers that would become read-only users otherwise, in the absence of proper configuration to match other roles. Additionally:preservation-adminentitlement, we've addedpreservation-managerandpreservation-reviewer,AUTH_CAS_ADMIN_ATTRIBUTE{,_VALUE}vars, new settingsCAS_MANAGER_ATTRIBUTE{,_VALUE}andCAS_REVIEWER_ATTRIBUTE{,_VALUE}have been added,SS_AUTH_DEFAULT_USER_ROLEif you want your users to a role with more privileges. Group membership could be identified via claims in the future.Connects to archivematica/Issues#1486.
Build status: see last commit in Archivematica branch.