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

New authorization for Promgen #562

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

hoangpn
Copy link
Contributor

@hoangpn hoangpn commented Feb 6, 2025

Using django-guardian to implement per object permission for Promgen.

  • Add owner field to the Farm object.
  • New default permissions for Service, Project, and Farm objects:
    • The edit permission will be used for editing the data of the object and its children, such as Notifier, Rule, etc.
    • The manage permission will be used for sensitive actions, like granting permission to other users.
    • By default, the owner will have manage permission on the object.
  • Apply logic to check permissions in Promgen Web.
  • Support the Administrator to assign/remove permissions via the Admin site.
  • Support normal users to assign/remove permissions via a new panel on the object's detail screen.

Some screenshots:
image

image

@hoangpn hoangpn requested review from kfdm and a team as code owners February 6, 2025 10:18
@hoangpn hoangpn force-pushed the feature/new_auth branch 2 times, most recently from e845dd8 to 74a5024 Compare February 7, 2025 03:05
@vincent-olivert-riera
Copy link
Contributor

vincent-olivert-riera commented Feb 14, 2025

@hoangpn , commit b2b3dd3 is adding a missing migration script that was forgotten on a previous pull request.

Please do that on a separate pull request, since those changes are not related to "New authorization for Promgen" pull request that you are doing here.

Amend the commit message to state very clearly that the migration script was forgotten on a previous pull request (you can also add the link).

@hoangpn hoangpn force-pushed the feature/new_auth branch 2 times, most recently from d70d5f9 to 53570cd Compare February 14, 2025 10:28
@hoangpn
Copy link
Contributor Author

hoangpn commented Feb 18, 2025

Thank you for your hard work! I created another pull request to add the missing migration script. I also updated this branch after that pull request was merged to the master branch.🙇

@hoangpn hoangpn force-pushed the feature/new_auth branch 2 times, most recently from de05743 to 2ad4b33 Compare February 26, 2025 01:33
@hoangpn
Copy link
Contributor Author

hoangpn commented Feb 26, 2025

Rebased new_auth onto master.

hoangpn added 5 commits March 14, 2025 14:53
We add an 'owner' field to the Farm object.
By default, the user who creates a Farm will become the owner.
Farm's owner could be changed.
To implement per-object permissions for Promgen, we add the django-guardian library
and some necessary configurations.
https://django-guardian.readthedocs.io/
We replace default permissions by new two permissions: manage and edit.
- The edit permission will be used for editing the data of the object and its children.
- The manage permission will be used for sensitive actions like granting permission to other users
or deleting the object, beside editing the data.

By default, the owner will have the manage permission on the object.
Replace Django's ModelAdmin with django-guardian's GuardedModelAdmin to allow administrators
to assign/remove permissions on the Admin Site on Serive/Project/Farm object.
https://django-guardian.readthedocs.io/en/stable/userguide/admin-integration.html
Create a mixin class to check permission logic:
- For Service/Farm, the object being checked is itself.
- For Project, the object being checked is itself or its parent Service.
- For Host, the object being checked is its parent Farm.
- For Exporter/URL, the object being checked is its parent Project
or the Service that is the parent of the Project.
- For Rule/Sender, the object being checked is its parent Service/Project.
- Other cases only have permission if the user being checked is a superuser.

Apply the mixin class to View classes:
- 'View' classes (List, Detail) are not applied.
- The 'ServiceRegister' class is not applied.
- 'Update' classes require the user to have EDIT or MANAGE permissions.
- 'Delete' classes require the user to have MANAGE permissions.
Add a panel to the UI of each detail screen of Service/Project/Farm object
to assign or remove permissions for users. User needs to have MANAGE permissions
to assign or remove permissions for users through this panel.
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

Successfully merging this pull request may close these issues.

2 participants