Skip to content

Ask for access #1081

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Ask for access #1081

wants to merge 7 commits into from

Conversation

lunika
Copy link
Member

@lunika lunika commented Jun 18, 2025

Purpose

A user without access to a document can request the access to it. Also, a user with an existing access can request to change its role.
The backend introduces a new Model responsible to manage this request and to create or update a DocumentAccess

Proposal

  • ✨(back) document as for access CRUD
  • ✨(back) accept for a owner the request to access a document
  • send email to owners

lunika added 2 commits June 18, 2025 15:18
We introduce a new model for user wanted to access a document or upgrade
their role if they already have access.
The viewsets does not implement PUT and PATCH, we don't need it for now.
Add the action accepting a request to access a document. It is possible
to override the role from the request and also update an existing
DocumentAccess

roles = set(document.get_roles(self.request.user))
is_owner_or_admin = bool(roles.intersection(set(models.PRIVILEGED_ROLES)))
# self.is_current_user_owner_or_admin = is_owner_or_admin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# self.is_current_user_owner_or_admin = is_owner_or_admin

lebaudantoine and others added 5 commits June 19, 2025 15:47
Stop retry attempts when receiving 401 Unauthorized from /me endpoint since
this clearly indicates authentication status. The original purpose of the /me
call is simply to determine if user is authenticated, and a 401 provides
sufficient information.

Prevents unnecessary network requests caused by React Query's automatic retry
behavior when re-raising exceptions, which was hiding the 401 status. Improves
performance and reduces server load during authentication failures.
Reduce unnecessary fetch requests when retrieving documents with permission
or authentication issues. Previous implementation was triggering multiple
document requests despite having sufficient error information from initial
attempt to determine appropriate user redirection.

Additionally, fix issue where resetting the auth cache was triggering redundant
authentication verification requests. The responsibility for checking auth
status should belong to the 401 page component on mount, rather than being
triggered by cache resets during error handling.

Known limitations:
- Not waiting for async  function completion makes code harder to
 maintain
- Added loading spinner as temporary solution to prevent UI flicker
- Future improvement should implement consistent error-based redirects rather
 than rendering error messages directly on document page
When a user is redirected on the 403 page,
they can now request access to the document.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants