Skip to content

End a departed member's access through their credentials - #2106

Merged
ppXD merged 1 commit into
mainfrom
fix/end-a-departed-members-access-through-their-credentials
Oct 12, 2026
Merged

ppXD merged 1 commit into
mainfrom
fix/end-a-departed-members-access-through-their-credentials

Conversation

@ppXD

@ppXD ppXD commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Summary

  • Leaving or being removed from a team used to delete only the membership row. The person's personal provider token stayed live there: the team could list it, browse what it reaches, bind more repositories through it, and act as the person through actAsUserId. Now a person's tokens stop being the team's once they no longer have standing in it.
  • Standing is a membership row in the team, or the instance Admin role while that role is switched on (role.status, as the caller's own roles are read). Everywhere else in CodeSpace that role has an Owner's standing in every team, as TeamMembershipResolver short-circuits for the caller. TeamStanding (backend/src/CodeSpace.Core/Authorization/TeamStanding.cs) defines it once. Every seam below uses it:
    • the departure sweep and a departure;
    • the holds a write in someone's name takes;
    • the OAuth callback;
    • the act-as picker and the act-as write.
  • A departure and every write in a member's name serialize on the membership row:
    • TeamMemberService.RemoveMembershipAsync deletes the row before deciding what to retire. The delete takes the row's exclusive lock, so it waits for any write that holds the row.
    • The OAuth callback asks standing before the code exchange and again after it (OAuthFlowService.HoldOwnerInTeamAsync). PAT linking asks again after the provider check, and naming an owner (EnsureMayActOnAsync) holds the row too. Each holds ITeamMemberService.HoldMembershipAsync until it commits.
    • So a departure that commits during a connect refuses its token, and one that starts during a connect waits for it, then retires the new token.
    • An instance Admin outside the team has no row to lock, and nothing in the app removes the role (role_user is written by migrations and SQL). Their writes ask standing again instead. A demotion that lands after that check leaves a token the sweep retires on its next tick; act-as refuses it at once.
  • One revocation cascade (ICredentialRevocationService) now serves Disconnect, identity unlink, departure and the sweep. A recurring job runs it for departures the request path never saw: DepartedMemberCredentialRetirementRecurringJob → RetireDepartedMembersCredentialsCommand → RetireDepartedAsync. It runs every 5 minutes, 50 tokens a batch, each token saved on its own.

Behaviour changes

  1. Leave and remove (POST /api/teams/members/leave, DELETE /api/teams/members/{userId}) now also retire every Personal credential the person lent that team: the ones they own, and owner-less ones they created. This happens in the same transaction, unless they hold the instance Admin role. A retired token:
    • has its payload blanked, status Revoked, and is soft-deleted;
    • turns every repository bound through it to Error, with "Credential disconnected. Re-link this repository to an active credential of the same provider, or unbind it.";
    • soft-deletes the act-as identities linked to it.
      The provider is not called. Team-service credentials and the person's credentials in other teams are untouched.
  2. New recurring job, DepartedMemberCredentialRetirementRecurringJob. It retires, as in 1, every live Personal credential whose person has no standing in the credential's team. The person is the owner, or, for an owner-less token, the account that created it.
    • On its first ticks after deploy, it retires the tokens of everyone who left a team before this change, and repositories bound through them turn Error.
    • It keeps tokens an instance Admin lent a team they are not a member of.
    • It keeps tokens whose creator stamp names no account (created_by empty, or not an app_user id): it never guesses who lent them.
    • Later it retires an ex-Admin's tokens in teams they are not a member of, once their role_user row is removed. Switching the Admin role off (role.status) does the same to every Admin's tokens in teams they are not a member of. Nothing in the app does either; only SQL does.
  3. Act-as: resolving actAsUserId (IActorIdentityResolver.ResolveAsync) now requires standing in the team that owns the provider instance. A workflow that names a departed person, including a run queued before they left, fails with ActorIdentityRequiredException before any provider call.
    • It also refuses when the team that owns the provider instance is soft-deleted. Before, only the picker checked that. No app path soft-deletes a team; only SQL does.
  4. Act-as picker: GET /api/repositories/{id}/act-as-candidates uses the same filter as the write. It now also offers an instance Admin outside the team who linked an identity there; before, the picker hid them while the write accepted them. Departed members stay hidden.
  5. Default owner: POST /api/credentials/pat with Personal ownership and no ownerUserId now stores the caller as the owner. Before, the token was stored with no owner.
  6. Named owners: a request-supplied owner (ownerUserId on POST /api/credentials/pat, intendedOwnerUserId on POST /api/credentials/oauth/init) must have standing in the current team and be the caller or rank below them. An instance Admin without a membership row ranks as Owner.
    • An unknown id and another team's user both get the same 404. Before, any id was accepted, and an id naming no account was a foreign-key 500.
    • An owner at or above the caller's rank gets 403.
  7. OAuth callback: the callback is refused with OAuthCallbackException and no credential is stored when the intended owner no longer has standing in the team. This is checked before the code exchange and again after it.
  8. OAuth act-as identity: an OAuth connect creates or re-points an act-as identity only for the person who started it. A connect an Admin starts for a member no longer touches the member's identity.
  9. Identity unlink: DELETE /api/me/identities/{id} now also turns the repositories bound through that identity's token Error.
  10. Binding: POST /api/repositories/bind refuses a soft-deleted credential.
  11. Departure waits: a departure now waits for an in-flight write that holds the departing member's row, such as an OAuth callback, a PAT link, or a token added in their name. When the write commits, the departure retires what it wrote.

Unchanged: an instance Admin outside a team can still link, add and OAuth-connect their own personal token there, bind through it, and be acted as there. The sweep leaves those tokens alone while the Admin holds the role.

Test plan

  • Unit: full suite, 13275 passed and 1 skipped. Includes the request-authorization, resource-access-marker and team-write-permission inventories, and the job → command → service dispatch tests.
  • Integration (real Postgres): TeamStandingTests. One theory checks both forms of the predicate across eight cases: member, instance Admin outside the team, instance Admin in the team, instance Admin outside the team whose Admin role is switched off, ex-Admin outside the team, departed member, another team's member, and no account.
  • Integration: DepartedMemberCredentialTests
    • leave and remove retire the member's tokens, and an instance Admin who leaves keeps theirs
    • the sweep keeps an Admin's tokens in a team they are not in, then retires them once the role is removed, and act-as follows
    • the sweep never retires an unattributable token (empty creator, or a creator that names no account)
    • OAuth callback for an Admin outside the team, and a demotion during the code exchange
    • departures racing an OAuth exchange, a PAT link and a held write
    • owner-less tokens, and the legacy-departure sweep
  • Integration: CredentialOwnerReachTests (an Admin outside the team connects for themselves; a team Admin cannot name one as owner), ActAsCandidatesFlowTests (picker and write agree for Admins, ex-Admins and a soft-deleted team), RepositoryBindingFlowTests (an instance Admin outside the team adds a token and owns it)
  • Integration: OAuth, Invitations, Auth, Authorization, Credentials, Users, Providers, ProviderInstances, Binding, Repositories, Chat, Persistence, Mediation, Middlewares, Projects, Webhooks, Hangfire, and the sweep-transaction theory, 1092 passed in total
  • E2E over HTTP with two teams (DepartedMemberCredentialE2ETests):
    • removed or left, including a run queued before the departure
    • owners outside the caller's reach
    • identity unlink, and a PAT added without an owner followed by leave
    • a pre-change departure followed by the sweep
    • an instance Admin outside the team links, adds, starts an OAuth connect and is acted as; the sweep keeps the token until the role is removed, then retires it
  • E2E: the Auth, OAuth-connect, resource-keyed-permission and claim-board classes; RecurringJobWorkerSmokeE2ETests fires the new job through the real worker host (107 passed)
  • Staging: after deploy, check the sweep's "Retired credential" log lines and which repositories turn Error. Confirm no token lent by an instance Admin is among them.

Leaving or being removed from a team deleted the membership row and
nothing else. The person's personal provider token stayed Active in the
team they left: a Member could list it and enumerate what it reaches,
an Admin could bind more repositories through it, and any workflow could
act as the departed person by naming them in actAsUserId, because
act-as resolution never asked whether the user was still in the team.

Standing in a team is a membership row there, or the instance Admin
role, which everywhere else in CodeSpace has an Owner's standing in
every team. One predicate, TeamStanding, answers that for anyone, and
every seam below asks it, so a departure, the sweep, the holds and the
act-as picker and write cannot disagree about who has left.

- Act-as resolution and the act-as picker share one filter: live
  identity, live Active credential, live team, and standing in the team
  that owns the provider instance. The write used to ask neither the
  team's liveness nor standing. A departed person resolves to null, so
  the write fails with ActorIdentityRequiredException before any
  provider call, including a run queued before they left.
- Leaving and removal retire every Personal credential the person lent
  that team: token blanked, Revoked, soft-deleted, bound repositories
  marked "needs a new credential", linked act-as identities soft-deleted.
  The departure saves its membership delete and then applies the
  sweep's own rule to that one person, in the same transaction, so an
  instance Admin who leaves a team keeps what they lent it. Team-service
  credentials and the person's credentials in other teams are untouched.
  The provider is not asked: revoking an OAuth grant there can end the
  person's tokens elsewhere.
- A departure and every write in a member's name serialize on their
  membership row. The departure deletes it before reading what to
  retire, and the delete waits for any write that holds the row; an
  OAuth callback (re-checked once the code exchange returns), a token
  link (once the provider check returns) and adding a token in someone's
  name hold it until they commit. A departure that commits during a
  connect refuses its token; one that starts during it waits, then
  retires the new token with the rest. Without this, a removal landing
  during the code exchange left a live token of a non-member, and the
  reconnect carry-forward moved the bindings the departure had just
  broken onto it. The hold takes the team row first, as an Owner's
  departure does, so the two cannot deadlock. An instance Admin outside
  the team has no row to hold, and nothing in the app takes the role
  away, so those writes re-ask standing instead; a demotion that lands
  after the re-check leaves a token the sweep retires on its next tick.
- A personal token added without naming an owner is now the caller's
  own, as an OAuth connect already was. One stored owner-less before
  this belongs, for retirement, to whoever added it.
- A recurring sweep retires the personal tokens of people with no
  standing left in the team they lent them to: every departure from
  before this change, any an older pod serves during the rolling deploy,
  and an instance Admin's in teams they are not in once they lose the
  role. What an instance Admin lent a team without joining it is kept.
  A token whose creator stamp names no account is never retired, since
  nothing says who lent it. Repositories bound through a retired token
  turn "needs a new credential" on the sweep's first ticks after deploy.
- One revocation cascade now serves Disconnect, identity unlink,
  departure and the sweep; unlink used to skip the repository half and
  leave bound repositories Active on a blanked token.
- Binding refuses a soft-deleted credential.
- A request-supplied credential owner (token ownerUserId, OAuth
  intendedOwnerUserId) must have standing in the current team and be
  someone the caller may act on: themselves or someone below them, an
  instance Admin outside the team ranking as an Owner. Unknown and
  foreign ids get the same 404, before the foreign key can turn one
  into a 500.
- An OAuth connect links or re-points an act-as identity only for the
  person who started it, so an Admin cannot choose whose provider
  account a member's writes are attributed to.

With every token they lent retired and no way to connect a new one in
the name of someone without standing, the reconnect carry-forward and
its repair sweep, both keyed on the same owner, have nothing to move a
departed member's bindings to.
@ppXD
ppXD merged commit 8a8ad97 into main Oct 12, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant