Sync Organization Administrator role on owner changes in Users::UpdateService

Bojan Marjanovicrequested to merge
630819-sync-owner-role-on-user-update into master

What does this MR do and why?

Organization owners get an IAM organization_admin role tuple. Authz::Organizations::GrantOwnerRoleWorker and Authz::Organizations::RevokeOwnerRoleWorker keep it in sync. Every service that changes an owner has to enqueue one of them.

Two paths changed owners without enqueuing anything:

  1. Users::UpdateService accepts nested organization_users_attributes, saved through accepts_nested_attributes_for :organization_users on User. Both Admin::UsersController (admin user edit page) and Admin::Organizations::UsersController (organization admin area users page) use it. The second one is reachable by organization owners, not only instance admins. A promoted owner got no tuple. A demoted owner kept theirs.
  2. User has after_update_commit :update_home_organization_user, if: -> { saved_change_to_admin }. It rewrites the user's home organization membership row through Organizations::OrganizationUser.update_home_organization_record_for. When the admin flag is removed, that row drops from owner to default, and the tuple stayed in IAM.

The demotion cases are the security-relevant ones. Artifact Registry never calls back to Rails, so the tuple is the only thing that ends a demoted owner's access.

This MR closes the two paths in different places:

  1. Users::UpdateService#after_update gets sync_organization_admin_roles, which runs only after a successful save. For each loaded organization_users record whose access_level changed across the owner boundary, it enqueues GrantOwnerRoleWorker if the record is now owner, and RevokeOwnerRoleWorker otherwise, with current_user as the acting user. A new membership going from nil to default enqueues nothing, since that is not a demotion. The check is skipped when no organization_users_attributes were passed, so ordinary profile updates issue no extra query. This matches the existing hooks in Organizations::OrganizationUsers::UpdateService, Organizations::OrganizationUsers::CreateService, Organizations::OrganizationUsers::DestroyService, and Users::CreateService.
  2. Organizations::OrganizationUser.update_home_organization_record_for enqueues RevokeOwnerRoleWorker when the home organization row it rewrites actually drops from owner to default. The admin flag is also written by the OIDC and SAML sign-in flows and by the access_level= setter, none of which go through Users::UpdateService, so the revoke has to live where the row changes. There is no acting user there, so the demoted user is passed as their own actor and RevokeOwnerRoleWorker authorizes it through IAM's self-delete rule. A row that was already default enqueues nothing. The grant direction when the flag is added is intentionally not handled: GrantOwnerRoleWorker already skips an instance admin's owner row on their home organization by design, as agreed in the maintainer note linked below.

Both enqueues are gated on Authz::Organizations::OwnerRoleSync.enabled?.

This is the same kind of interim fix as the one for Users::CreateService. The long-term plan is to move the sync to the IAM transactional outbox. The team decided to fix this issue now with the worker pattern, because the outbox design needs more evolution before new entities are added to it.

Known gap, not addressed here: RevokeOwnerRoleWorker only honors the acting user when they currently own the organization, and unlike the grant worker it has no fallback actor. The nested-attributes revoke is reached mostly by instance admins demoting an owner from the admin area, who usually do not own that organization. In that case the worker logs and tracks an unauthorized-revoke error instead of revoking, and the tuple stays. The same applies to the existing hook in Organizations::OrganizationUsers::UpdateService. That acting-user problem is already tracked in the issue linked below and is not solved here. The admin flag path is not affected, since the demoted user acts for themselves.

Changes:

  • app/services/users/update_service.rb: the nested-attributes hook.
  • app/models/organizations/organization_user.rb: the home organization revoke.
  • spec/services/users/update_service_spec.rb: new describe 'Organization Administrator role sync' block. It covers:
    • owner to default enqueues revoke with the admin as actor
    • default to owner enqueues grant
    • a new membership created as owner enqueues grant
    • unchanged access level enqueues nothing
    • adding a new default membership enqueues nothing
    • a failed update enqueues nothing
    • sync disabled enqueues nothing
    • an organization owner (non-admin) demoting another owner enqueues revoke with the owner as actor
  • spec/models/organizations/organization_user_spec.rb: the home organization row going from owner to default enqueues revoke with the user as actor; default to owner, already default, newly created, and sync disabled enqueue nothing.

References

Screenshots or screen recordings

Not applicable. This change is backend only.

How to set up and validate locally

You need an EE instance with IAM data access configured (iam_data_access_service.secret_file in gitlab.yml) and a Cloud Connector key.

  1. In the admin area, edit a user who is an organization owner and change their organization role to default. Confirm one RevokeOwnerRoleWorker job is enqueued with the organization id, the user id, and the admin's id.
  2. Change the role back to owner. Confirm one GrantOwnerRoleWorker job is enqueued.
  3. Edit an admin user and uncheck the admin flag. Confirm one RevokeOwnerRoleWorker job is enqueued for the user's home organization with the user's own id as the acting user. Repeat on a user whose home organization row is already default and confirm nothing is enqueued.
  4. Update only a profile field, such as the name. Confirm nothing is enqueued.

RuboCop is clean on both files. RSpec was not run locally because the test database cluster for this checkout is down. The MR pipeline verifies the specs.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

🤖 Generated with Claude Code

Edited by Bojan Marjanovic

Merge request reports