Revoke role assignments checks the account's owning organization, so the assignee check does nothing
Problem
RevokeRoleAssignmentsService validates the assignee like this.
def same_organization?(assignee)
current_user.organization_id == assignee.organization_id
endThat compares the organization which owns the caller's account with the one which owns the assignee's account. It looks at neither the organization the revoke is scoped to nor anyone's membership.
Every account is owned by the default organization (confirmed by the Organizations team), so this is 1 == 1 for any two users and the check passes for everybody.
Why that matters
The check exists to hide information, per the comment at its call site.
# Treat a cross-organization assignee the same as a missing one so the
# error does not reveal whether the user exists in another organization.Because the comparison always passes, Assignee could not be found. is now returned only when the user genuinely does not exist. So the message discloses exactly what it was written to conceal. A member of an organization can probe whether a given user id exists, and whether they hold a role on a named resource, because a missing subject comes back as an error rather than a success.
Exposure is limited to disclosure rather than unwanted deletion. caller_in_organization? confines the caller to an organization they are a member of, and IAM keys a subject by organization plus user, so a delete can only remove a tuple that exists in that organization's identity space. Naming an arbitrary user removes nothing.
Why it was not simply converted to a membership check
The grant path now requires the assignee to be a member of the target organization. Applying the same rule to revoke makes things worse. Once someone is removed from an organization they would stop passing the check, so their role could never be revoked. Their existing tuples keep granting access, because Artifact Registry validates tokens locally and never calls back to Rails, so deleting the tuple is the only thing that cuts them off. That is the case #610042 describes as the dangerous one.
So the choice was between a check that does nothing and a check that blocks the cleanup that has to work. !252571 (merged) kept the first deliberately and recorded why in a comment.
Proposed direction
Remove the assignee organization check from revoke rather than converting it.
The caller is already confined to an organization they belong to, and the organization-scoped delete key already prevents removing anything outside it. The only property lost is the existence hiding, which is not working today and which user existence on GitLab largely gives away regardless.
If the disclosure is judged worth keeping, the check needs to be based on whether the assignee has or had a tuple in that organization, not on membership, so that a departed member stays revokable.
Why this needs deciding with the offboarding work
Whoever builds organization member removal has to remove the IAM tuples as part of it, and this check is what would stand in the way. It should be settled alongside #610042 rather than on its own.
Links
- !252571 (merged) where the check was left unchanged on purpose
- #610042 user offboarding does not revoke Artifact Registry role assignments
- artifact-registry#977 the home organization coupling this came out of