Skip to content

Conversation

@lunika
Copy link
Member

@lunika lunika commented Nov 12, 2025

Purpose

Via the ask_for_access viewset, it is possible for an administrator to escaladate an other user to an owner role and then take the control of the document by removing all other owner.

This PR ensure that an administrator can not accept role higher than the one he/she already have. Also, the serializer does not accept the owner role anymore.

Proposal

  • 🔒️(backend) role in ask_for_access must be lower than user role
  • 🔒️(backend) remove owner as valid role for ask_for_access serializer

@lunika lunika requested review from AntoLC and qbey November 12, 2025 11:04
@lunika lunika self-assigned this Nov 12, 2025
@lunika lunika added the enhancement improve an existing feature label Nov 12, 2025
@lunika lunika force-pushed the fix/ask-to-access-escalation branch from 02d300d to b37f2aa Compare November 12, 2025 12:27
@github-actions
Copy link

github-actions bot commented Nov 14, 2025

Size Change: +23 B (0%)

Total Size: 4.07 MB

Filename Size Change
apps/impress/out/_next/static/4d8f7a5a/_buildManifest.js 0 B -883 B (removed) 🏆
apps/impress/out/_next/static/572d983d/_buildManifest.js 883 B +883 B (new file) 🆕

compressed-size-action

@AntoLC AntoLC self-requested a review November 14, 2025 10:54
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.

Lgtm

We check that the role set in a ask_for_access is not higher than the
user's role accepting the request. We prevent case where ad min will
grant a user owner in order to take control of the document. Only owner
can accept an owner role.
When a ask_for_access creation is made, we explicitly remove the owner
role to prevent role escalation.
@lunika lunika force-pushed the fix/ask-to-access-escalation branch from 35e2235 to 3ce8864 Compare November 17, 2025 07:47
lunika and others added 2 commits November 17, 2025 08:48
Like in other abilities, we compute a set_role_to property on the
abilities. This set_role_to contains all the roles lower or equal than
the current user role. We rely on this propoerty to validate the accept
endpoint and it will be used by the front allpication to built the role
select list.
We disable roles that the current user is not allowed
to assign when sharing a document. This prevents
users from selecting roles they cannot actually
assign, improving the user experience and reducing
confusion.
@lunika lunika force-pushed the fix/ask-to-access-escalation branch from 3ce8864 to b069310 Compare November 17, 2025 07:48
@lunika lunika merged commit b069310 into main Nov 17, 2025
21 of 22 checks passed
@lunika lunika deleted the fix/ask-to-access-escalation branch November 17, 2025 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement improve an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants