Skip to content

Prevent concurrent admin revocations from leaving a team without an administrator #1985

Description

@dimitri-tombroff

Description

Context

FRED enforces the invariant that every team must retain at least one user with the team_admin role.

Before revoking an administrator role, the Control Plane currently:

  1. reads the current team administrators;
  2. verifies that more than one administrator remains;
  3. deletes the requested role relation.

Relevant code:

  • apps/control-plane-backend/control_plane_backend/teams/service.py
  • _ensure_team_keeps_at_least_one_admin()
  • revoke_team_member_role()
  • remove_team_member()

Reported during the review of PR #1957:

#1957 (comment)

Problem

The check and the deletion are not serialized.

If a team has exactly two administrators, two concurrent requests may attempt to revoke or remove them:

  1. request A reads two administrators and passes the check;
  2. request B also reads two administrators and passes the check;
  3. request A removes the first administrator;
  4. request B removes the second administrator;
  5. the team is left without any administrator.

Each request is valid when evaluated independently, but their interleaving violates the team invariant.

Expected behavior

The operation that verifies the last-administrator constraint and removes an administrator must be atomic or serialized per team.

At most one concurrent revocation should succeed when a team has only two administrators. The other request should observe that only one administrator remains and fail with the existing last-administrator constraint error.

This protection must apply to every operation capable of removing the team_admin relation, including:

  • revoking only the team_admin role;
  • removing a team member who holds the team_admin role.

Suggested approach

Use a team-scoped PostgreSQL advisory lock, or the repository’s existing equivalent serialization mechanism, around the complete critical section:

  1. acquire the lock for the target team;
  2. read the current direct administrator relations;
  3. enforce the last-administrator invariant;
  4. delete the requested relation or member relations;
  5. release the lock when the transaction or operation completes.

The existing locking approach used by rescue_team_admin should be inspected and reused where appropriate.

Avoid process-local locks, because concurrent requests may be handled by different replicas.

Acceptance criteria

  • Two concurrent revocations against the last two administrators cannot both succeed.
  • Two concurrent member removals against the last two administrators cannot both succeed.
  • The same guarantee holds when one request revokes a role while another removes a member.
  • Exactly one request succeeds and the other receives the existing domain error when only one administrator would remain.
  • Revoking an administrator remains possible when another administrator safely remains.
  • Operations affecting different teams do not unnecessarily block one another.
  • Existing support for users holding multiple simultaneous roles is preserved.
  • Recovery through rescue_team_admin remains functional.
  • Concurrency regression tests demonstrate the failing interleaving and verify the fix.

Scope

This issue is intentionally tracked separately from PR #1957 so that the current authorization consolidation can proceed without broadening its scope.

8:37 AM

Metadata

Metadata

Assignees

No one assigned

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions