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

♻️(backend) fix access rights on the invitations API endpoint #369

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

sampaccoud
Copy link
Member

@sampaccoud sampaccoud commented Oct 21, 2024

Purpose

Only users who have the rights to manage accesses on a document should be allowed to see and manipulate invitations on this document.

Other users should be able to see access rights on the document but only when the corresponding user/team has actually been granted access, not during invitation.

Proposal

Refactor the invitation API endpoint to fix this issue concerning access rights on invitations.

As advised by @qbey, I took this refactoring opportunity to move any code related to permissions on invitations from the serializer to a permission class.

Tests on the invitation API endpoint were copied a bit too fast from https://github.com/numerique-gouv/people and did not respect the code style and testing strategy used in this project so I normalized them while modifying them to fix them following changes in the API code.

@sampaccoud sampaccoud requested review from qbey and AntoLC October 21, 2024 22:35
@sampaccoud sampaccoud self-assigned this Oct 21, 2024
@sampaccoud sampaccoud added bug Something isn't working python Pull requests that update Python code backend refacto labels Oct 21, 2024
@sampaccoud sampaccoud changed the title ♻️(backend) fix invitations API endpoint access rights ♻️(backend) fix access rights on the invitations API endpoint Oct 21, 2024
@sampaccoud sampaccoud force-pushed the fix-get-abilities branch 3 times, most recently from 5f809f3 to 878e4cc Compare October 22, 2024 07:30
src/backend/core/api/permissions.py Outdated Show resolved Hide resolved
src/backend/core/api/permissions.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@AntoLC AntoLC left a comment

Choose a reason for hiding this comment

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

From a frontend perspective, it looks good ✅

@sampaccoud sampaccoud requested a review from qbey October 22, 2024 12:17
Copy link
Collaborator

@qbey qbey left a comment

Choose a reason for hiding this comment

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

Please consider my comment on the permission name (you may agree or not), otherwise everything looks good to me :) Thanks for the permission aspect cleanup ❤️

Only users who have the rights to manage accesses on the document should
be allowed to see and manipulate invitations. Other users can see access
rights on the document but only when the corresponding user/team has
actually been granted access.

We added a parameter in document abilities so the frontend knows when
the logged-in user can invite another user with the owner role or not.
@sampaccoud sampaccoud enabled auto-merge (rebase) October 22, 2024 16:44
@sampaccoud
Copy link
Member Author

@qbey @AntoLC thank you for the fast reviews!
Shippiiiiing 🚢

@sampaccoud sampaccoud merged commit 0f0f812 into main Oct 22, 2024
15 of 16 checks passed
@sampaccoud sampaccoud deleted the fix-get-abilities branch October 22, 2024 17:40
@AntoLC AntoLC mentioned this pull request Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend bug Something isn't working python Pull requests that update Python code refacto
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants