Skip to content

[18.0][ADD] auth_user_role#928

Open
astirpe wants to merge 1 commit intoOCA:18.0from
astirpe:18_add_auth_user_role
Open

[18.0][ADD] auth_user_role#928
astirpe wants to merge 1 commit intoOCA:18.0from
astirpe:18_add_auth_user_role

Conversation

@astirpe
Copy link
Copy Markdown
Member

@astirpe astirpe commented Apr 17, 2026

No description provided.

@astirpe astirpe force-pushed the 18_add_auth_user_role branch from 96db213 to f9f973d Compare April 20, 2026 07:28
@astirpe astirpe marked this pull request as ready for review April 20, 2026 07:32
Copy link
Copy Markdown
Contributor

@CasVissers-360ERP CasVissers-360ERP left a comment

Choose a reason for hiding this comment

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

Functional review

roles_to_add = self._get_mapped_roles(identity_payload)

existing_lines = self.role_line_ids
existing_role_ids = set(existing_lines.mapped("role_id").ids)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@astirpe In a full sync I would expect also res.groups to be removed since we want a full sync with roles setup for that user.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@CasVissers-360ERP it's already the case, see this line:

if strict_sync:
    self.set_groups_from_roles(force=True)

Since "force=True" is passed, Odoo performs the following:

  • Calculates all groups implied by the user's current active roles.
  • Wipes any groups currently on the user that are not in that calculated list.
  • Ensures only the groups defined by the SAML-mapped roles remain.

This scenario is already covered by test_13_strict_sync_removes_native_groups().

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@astirpe what if the user has no roles?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@CasVissers-360ERP In case a user ends up with no roles, here is how set_groups_from_roles() behaves:

The following statement is bypassed, because force is true:

for user in self:
    if not user.role_line_ids and not force:
        continue

https://github.com/OCA/server-backend/blob/18.0/base_user_role/models/user.py#L92-L93

In this case:

groups_to_add will be an empty list;

groups_to_remove = list(set(user.groups_id.ids) - set([])). This means every single group the user currently has is flagged for removal;

Odoo executes the (3, ID) commands for all groups_to_remove, so that the user is stripped of all permissions. The user might be able to log in, but they will see a blank screen because they don't even have the basic base.group_user

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@astirpe My bad, thnx for the explanation!

@astirpe astirpe force-pushed the 18_add_auth_user_role branch from f9f973d to f59e143 Compare May 6, 2026 14:29
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