From 6b9cc7af9592bb79e21e436fd9d93222df87df6d Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Mon, 12 Oct 2026 14:29:23 +0800 Subject: [PATCH] End a departed member's access through their credentials 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. --- .../Authorization/TeamStanding.cs | 43 + ...epartedMembersCredentialsCommandHandler.cs | 21 + ...dMemberCredentialRetirementRecurringJob.cs | 17 + .../CredentialRevocationService.cs | 176 ++++ .../Services/Credentials/CredentialService.cs | 82 +- .../ICredentialRevocationService.cs | 42 + .../Services/OAuth/OAuthFlowService.cs | 63 +- .../Identity/ActorIdentityResolver.cs | 56 +- .../Identity/IActorIdentityResolver.cs | 6 +- .../Identity/IUserProviderIdentityService.cs | 8 +- .../Identity/UserProviderIdentityService.cs | 44 +- .../RepositoryBindingService.cs | 3 +- .../Services/Teams/ITeamMemberService.cs | 17 + .../Services/Teams/TeamMemberService.cs | 85 +- ...RetireDepartedMembersCredentialsCommand.cs | 25 + .../Credentials/AddCredentialInput.cs | 2 +- .../Auth/DepartedMemberCredentialE2ETests.cs | 581 +++++++++++ .../Authorization/TeamStandingTests.cs | 125 +++ .../Binding/RepositoryBindingFlowTests.cs | 21 +- .../Binding/TestRepositoryProvider.cs | 3 + .../Credentials/CredentialOwnerReachTests.cs | 281 ++++++ .../DepartedMemberCredentialTests.cs | 930 ++++++++++++++++++ .../Providers/ActAsCandidatesFlowTests.cs | 55 ++ .../UserProviderIdentityServiceTests.cs | 43 + .../SweepCommandTransactionFlowTests.cs | 1 + .../RequestAuthorizationInventoryTests.cs | 1 + ...MemberCredentialRetirementDispatchTests.cs | 102 ++ 27 files changed, 2739 insertions(+), 94 deletions(-) create mode 100644 backend/src/CodeSpace.Core/Authorization/TeamStanding.cs create mode 100644 backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/RetireDepartedMembersCredentialsCommandHandler.cs create mode 100644 backend/src/CodeSpace.Core/Jobs/RecurringJobs/DepartedMemberCredentialRetirementRecurringJob.cs create mode 100644 backend/src/CodeSpace.Core/Services/Credentials/CredentialRevocationService.cs create mode 100644 backend/src/CodeSpace.Core/Services/Credentials/ICredentialRevocationService.cs create mode 100644 backend/src/CodeSpace.Messages/Commands/Credentials/RetireDepartedMembersCredentialsCommand.cs create mode 100644 backend/tests/CodeSpace.E2ETests/Auth/DepartedMemberCredentialE2ETests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Authorization/TeamStandingTests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialOwnerReachTests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Credentials/DepartedMemberCredentialTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Jobs/DepartedMemberCredentialRetirementDispatchTests.cs diff --git a/backend/src/CodeSpace.Core/Authorization/TeamStanding.cs b/backend/src/CodeSpace.Core/Authorization/TeamStanding.cs new file mode 100644 index 000000000..b40e8bd66 --- /dev/null +++ b/backend/src/CodeSpace.Core/Authorization/TeamStanding.cs @@ -0,0 +1,43 @@ +using System.Linq.Expressions; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Constants; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Query; + +namespace CodeSpace.Core.Authorization; + +/// +/// Standing in a team, asked of anyone rather than of the caller: a membership row in the team, or the instance Admin +/// role, which has the standing of an Owner in every team — what grants the caller. +/// The role counts while it is switched on, the same role_user → role join the caller's own roles are read +/// through. Whether the team is live is not part of it; the seams that care ask that separately. +/// +/// Defined once because several seams ask it of someone other than the caller, and must agree: the departed-token +/// sweep and a departure (what to retire), the holds a write in someone's name takes (whether to write it), and the +/// act-as picker and the write it feeds (whom to act as). Read as a membership row alone, the sweep retired every token +/// an instance Admin had lent a team without joining it. +/// +public static class TeamStanding +{ + /// + /// The rule as a filter over rows that each name a team and a person — a credential and whoever lent it, an identity + /// and its user — composed into one predicate EF translates. Both selectors must read from the same row. + /// + public static Expression> Of(CodeSpaceDbContext db, Expression> team, Expression> person) + { + var row = team.Parameters[0]; + var personOfRow = ReplacingExpressionVisitor.Replace(person.Parameters[0], row, person.Body); + var rule = Rule(db); + + return Expression.Lambda>(ReplacingExpressionVisitor.Replace(rule.Parameters, new[] { team.Body, personOfRow }, rule.Body), row); + } + + /// Whether has standing in as of now. Someone with no account has none. + public static async Task HeldAsync(CodeSpaceDbContext db, Guid teamId, Guid userId, CancellationToken cancellationToken) => + await db.User.IgnoreQueryFilters().AsNoTracking().Where(u => u.Id == userId).AnyAsync(Of(db, _ => teamId, u => u.Id), cancellationToken).ConfigureAwait(false); + + private static Expression> Rule(CodeSpaceDbContext db) => (teamId, userId) => + db.TeamMembership.Any(m => m.TeamId == teamId && m.UserId == userId) || + db.RoleUser.Any(ru => ru.UserId == userId && db.Role.Any(r => r.Id == ru.RoleId && r.Name == Roles.Admin && r.Status)); +} diff --git a/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/RetireDepartedMembersCredentialsCommandHandler.cs b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/RetireDepartedMembersCredentialsCommandHandler.cs new file mode 100644 index 000000000..e7ffb98de --- /dev/null +++ b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/RetireDepartedMembersCredentialsCommandHandler.cs @@ -0,0 +1,21 @@ +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Messages.Commands.Credentials; +using MediatR; + +namespace CodeSpace.Core.Handlers.CommandHandlers.Credentials; + +/// Thin dispatcher (Rule 16) — the production caller of . +public sealed class RetireDepartedMembersCredentialsCommandHandler : IRequestHandler +{ + private readonly ICredentialRevocationService _revocation; + + public RetireDepartedMembersCredentialsCommandHandler(ICredentialRevocationService revocation) + { + _revocation = revocation; + } + + public async Task Handle(RetireDepartedMembersCredentialsCommand request, CancellationToken cancellationToken) + { + return await _revocation.RetireDepartedAsync(request.BatchSize, cancellationToken).ConfigureAwait(false); + } +} diff --git a/backend/src/CodeSpace.Core/Jobs/RecurringJobs/DepartedMemberCredentialRetirementRecurringJob.cs b/backend/src/CodeSpace.Core/Jobs/RecurringJobs/DepartedMemberCredentialRetirementRecurringJob.cs new file mode 100644 index 000000000..c87016ab3 --- /dev/null +++ b/backend/src/CodeSpace.Core/Jobs/RecurringJobs/DepartedMemberCredentialRetirementRecurringJob.cs @@ -0,0 +1,17 @@ +using CodeSpace.Messages.Commands.Credentials; +using MediatR; + +namespace CodeSpace.Core.Jobs.RecurringJobs; + +/// Every five minutes: retire the personal tokens still live in a team their person no longer has standing in — every departure from before departures retired them, any an older pod serves during a rolling deploy, and an instance Admin's in a team they are not in once they lose the role (thin Rule-14 dispatcher). A retired token is no longer a candidate, so an idle tick is one small read. +public sealed class DepartedMemberCredentialRetirementRecurringJob : IRecurringJob +{ + private readonly IMediator _mediator; + + public DepartedMemberCredentialRetirementRecurringJob(IMediator mediator) { _mediator = mediator; } + + public string JobId => nameof(DepartedMemberCredentialRetirementRecurringJob); + public string CronExpression => "*/5 * * * *"; + + public async Task Execute() => await _mediator.Send(new RetireDepartedMembersCredentialsCommand()).ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialRevocationService.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialRevocationService.cs new file mode 100644 index 000000000..a96bcbed1 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialRevocationService.cs @@ -0,0 +1,176 @@ +using CodeSpace.Core.Authorization; +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Workflows; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed class CredentialRevocationService : ICredentialRevocationService, IScopedDependency +{ + private readonly CodeSpaceDbContext _db; + private readonly ILogger _logger; + + public CredentialRevocationService(CodeSpaceDbContext db, ILogger logger) + { + _db = db; + _logger = logger; + } + + public async Task CascadeAsync(Guid credentialId, CancellationToken cancellationToken) + { + var affectedRepoCount = await MarkDependentRepositoriesUnauthorisedAsync(credentialId, cancellationToken).ConfigureAwait(false); + + await SoftDeleteDependentIdentitiesAsync(credentialId, cancellationToken).ConfigureAwait(false); + + return affectedRepoCount; + } + + public async Task RetireAsync(Credential credential, CancellationToken cancellationToken) + { + MarkRetired(credential); + + return await CascadeAsync(credential.Id, cancellationToken).ConfigureAwait(false); + } + + /// + /// The sweep's own rule, narrowed to one person in one team, so a departure and the sweep can never disagree on what + /// goes. Every Personal credential the person lent the team goes, the already-disconnected ones included: the repair + /// sweep moves a disconnected credential's bindings to its owner's newest live one, so a single live token left behind + /// would hand the team back everything bound through the others. With none left, and no way to connect a new one in + /// the name of someone without standing in the team, nothing bound through them follows anywhere. A team-service + /// credential is the team's and survives membership changes — it is written owner-less, and is never retired here. + /// + public async Task RetireDepartedInTeamAsync(Guid teamId, Guid userId, CancellationToken cancellationToken) + { + var departed = await Departed() + .Where(c => c.TeamId == teamId && (c.OwnerUserId ?? c.CreatedBy) == userId) + .ToListAsync(cancellationToken).ConfigureAwait(false); + + foreach (var credential in departed) await RetireAsync(credential, cancellationToken).ConfigureAwait(false); + } + + public async Task RetireDepartedAsync(int batchSize, CancellationToken cancellationToken) + { + var departedIds = await LoadDepartedIdsAsync(batchSize, cancellationToken).ConfigureAwait(false); + + var retired = 0; + + foreach (var credentialId in departedIds) + { + try + { + if (await RetireDepartedOneAsync(credentialId, cancellationToken).ConfigureAwait(false)) retired++; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "Retiring credential {CredentialId}, whose person is no longer in its team, failed; the pass continues — the credential stays a candidate", credentialId); + } + } + + return retired; + } + + private async Task> LoadDepartedIdsAsync(int batchSize, CancellationToken cancellationToken) => + await Departed().AsNoTracking().OrderBy(c => c.Id).Select(c => c.Id).Take(batchSize).ToListAsync(cancellationToken).ConfigureAwait(false); + + /// + /// Re-read under the candidate rule and retired on its own save, so one failure undoes no other. A person who rejoined + /// since the candidate list was read keeps the token. + /// + private async Task RetireDepartedOneAsync(Guid credentialId, CancellationToken cancellationToken) + { + try + { + var credential = await Departed().SingleOrDefaultAsync(c => c.Id == credentialId, cancellationToken).ConfigureAwait(false); + + if (credential == null) return false; + + var brokenRepositoryCount = await RetireAsync(credential, cancellationToken).ConfigureAwait(false); + + await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + + _logger.LogInformation("Retired credential {CredentialId} in team {TeamId}: its person is no longer a member; {RepositoryCount} repositories bound through it now need a new credential", credential.Id, credential.TeamId, brokenRepositoryCount); + + return true; + } + finally + { + _db.ChangeTracker.Clear(); + } + } + + /// + /// Live Personal credentials whose person has no standing in the credential's team (): no + /// membership row there, and not an instance Admin. The person is the owner, or — for a token stored without one, as + /// adding a token without naming an owner did before the owner defaulted to the caller — whoever added it. + /// + /// Only a person with an account. An owner always has one (the column is a foreign key); a creator stamp is not + /// one — it can be empty, or name nobody when a script wrote the row — and then nothing says who lent the token or + /// whether they left. That token is kept: retiring it would break every repository bound through it on a guess. A + /// soft-deleted account is still somebody, and goes like anyone else without standing. + /// + private IQueryable Departed() => + _db.Credential + .Where(c => c.Ownership == CredentialOwnership.Personal && c.DeletedDate == null) + .Where(c => _db.User.IgnoreQueryFilters().Any(u => u.Id == (c.OwnerUserId ?? c.CreatedBy))) + .Where(ExpressionPredicate.Not(TeamStanding.Of(_db, c => c.TeamId, c => c.OwnerUserId ?? c.CreatedBy))); + + /// + /// Clear the token material — nothing in CodeSpace can present it once it is gone — and take the row out of the + /// team's credential list. A row retired earlier keeps the date it was first retired on. + /// + private static void MarkRetired(Credential credential) + { + credential.EncryptedPayload = string.Empty; + credential.Status = CredentialStatus.Revoked; + credential.LastValidatedDate = null; + credential.DeletedDate ??= DateTimeOffset.UtcNow; + } + + /// + /// Repositories bound through this credential lose their auth source the moment it's + /// revoked. We DON'T cascade-delete them (too destructive — disconnect is often part of + /// token rotation, and the user expects to come back), but we DO flip status to + /// with a concrete remediation message so the UI + /// can show "Needs new credential" instead of pretending the repo is still healthy. + /// + /// Webhook ingestion is deliberately untouched — webhooks authenticate via the secret we + /// registered, not via the OAuth token. Events keep flowing in, and once the user + /// re-links to a new credential the repo is fully healthy again with no event gap. + /// + private async Task MarkDependentRepositoriesUnauthorisedAsync(Guid credentialId, CancellationToken cancellationToken) + { + var affected = await _db.Repository + .Where(r => r.CredentialId == credentialId && r.DeletedDate == null && r.Status != RepositoryStatus.Error) + .ToListAsync(cancellationToken).ConfigureAwait(false); + + foreach (var repo in affected) + { + repo.Status = RepositoryStatus.Error; + repo.LastError = "Credential disconnected. Re-link this repository to an active credential of the same provider, or unbind it."; + } + + return affected.Count; + } + + /// + /// A (Model B act-as-user link) is unusable without its backing + /// credential, so revoking the credential — e.g. Disconnect on the Personal tab — must soft-delete the + /// linked identity too. Otherwise the Connected Identities surface keeps showing a dead link, out of + /// sync with the Personal tab. Tracked + saved by the same unit of work as the revoke, and idempotent + /// (no live identity ⇒ no-op) so the always-run cascade is safe on an already-revoked credential. + /// + private async Task SoftDeleteDependentIdentitiesAsync(Guid credentialId, CancellationToken cancellationToken) + { + var identities = await _db.UserProviderIdentity + .Where(i => i.CredentialId == credentialId && i.DeletedDate == null) + .ToListAsync(cancellationToken).ConfigureAwait(false); + + var now = DateTimeOffset.UtcNow; + foreach (var identity in identities) identity.DeletedDate = now; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs index 38626494d..e50fb9346 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs @@ -7,6 +7,7 @@ using CodeSpace.Core.Services.Providers.Capabilities; using CodeSpace.Core.Services.Providers.Modules; using CodeSpace.Core.Services.Providers.Scopes; +using CodeSpace.Core.Services.Teams; using CodeSpace.Messages.Commands.Credentials; using CodeSpace.Messages.Credentials; using CodeSpace.Messages.Dtos.Credentials; @@ -22,6 +23,7 @@ public sealed class CredentialService : ICredentialService, IScopedDependency { private readonly CodeSpaceDbContext _db; private readonly ICurrentTeam _currentTeam; + private readonly ICurrentUser _currentUser; private readonly IPayloadEncryptor _encryptor; private readonly ICredentialPayloadSerializer _serializer; private readonly IOAuthClientRegistry _oauthClients; @@ -29,12 +31,15 @@ public sealed class CredentialService : ICredentialService, IScopedDependency private readonly IProviderModuleCatalog _modules; private readonly IScopeChecker _scopeChecker; private readonly ICredentialSuccessionService _succession; + private readonly ICredentialRevocationService _revocation; + private readonly ITeamMemberService _members; private readonly ILogger _logger; - public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IOAuthClientRegistry oauthClients, IProviderRegistry registry, IProviderModuleCatalog modules, IScopeChecker scopeChecker, ICredentialSuccessionService succession, ILogger logger) + public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IOAuthClientRegistry oauthClients, IProviderRegistry registry, IProviderModuleCatalog modules, IScopeChecker scopeChecker, ICredentialSuccessionService succession, ICredentialRevocationService revocation, ITeamMemberService members, ILogger logger) { _db = db; _currentTeam = currentTeam; + _currentUser = currentUser; _encryptor = encryptor; _serializer = serializer; _oauthClients = oauthClients; @@ -42,6 +47,8 @@ public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, IPaylo _modules = modules; _scopeChecker = scopeChecker; _succession = succession; + _revocation = revocation; + _members = members; _logger = logger; } @@ -149,6 +156,10 @@ public async Task AddAsync(AddCredentialInput input, CancellationToken can // another team's instances. await EnsureProviderInstanceInTeamAsync(input.ProviderInstanceId, teamId, cancellationToken).ConfigureAwait(false); + var ownerUserId = OwnerOf(input); + + await EnsureOwnerWithinReachAsync(ownerUserId, cancellationToken).ConfigureAwait(false); + var json = _serializer.Serialize(input.Payload); var encrypted = _encryptor.Encrypt(json); @@ -157,8 +168,7 @@ public async Task AddAsync(AddCredentialInput input, CancellationToken can Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = input.ProviderInstanceId, - // A team-service credential belongs to the team, not a person — never carry an owner. - OwnerUserId = input.Ownership == CredentialOwnership.TeamService ? null : input.OwnerUserId, + OwnerUserId = ownerUserId, Ownership = input.Ownership, AuthType = input.Payload.Type, DisplayName = input.DisplayName, @@ -193,8 +203,7 @@ public async Task RevokeAsync(Guid credentialId, Cancell MarkRevokedLocally(credential, providerError); } - var affectedRepoCount = await MarkDependentRepositoriesUnauthorisedAsync(credential.Id, cancellationToken).ConfigureAwait(false); - await SoftDeleteDependentIdentitiesAsync(credential.Id, cancellationToken).ConfigureAwait(false); + var affectedRepoCount = await _revocation.CascadeAsync(credential.Id, cancellationToken).ConfigureAwait(false); return new RevokeCredentialResult { @@ -231,6 +240,25 @@ private async Task EnsureProviderInstanceInTeamAsync(Guid providerInstanceId, Gu if (!inTeam) throw new KeyNotFoundException($"Provider instance {providerInstanceId} not found."); } + /// + /// A team-service credential belongs to the team, not a person — it never carries an owner, whatever it was handed. A + /// personal one always does: the person named, or the caller when nobody is, as an OAuth connect already defaults it. + /// An owner-less personal token was nobody's to retire when the person who added it left. + /// + private Guid? OwnerOf(AddCredentialInput input) => input.Ownership == CredentialOwnership.TeamService ? null : input.OwnerUserId ?? _currentUser.Id; + + /// + /// Naming an owner is acting on that person: the team lists the token as theirs and its bindings follow their next + /// connect. So the owner must have standing in this team — a member, or an instance Admin — and the caller must be them + /// or outrank them; an unknown id and one from another team fail alike, before the foreign key could tell them apart. + /// + private async Task EnsureOwnerWithinReachAsync(Guid? ownerUserId, CancellationToken cancellationToken) + { + if (ownerUserId is not Guid ownerId) return; + + await _members.EnsureMayActOnAsync(ownerId, cancellationToken).ConfigureAwait(false); + } + private async Task LoadCredentialAsync(Guid id, CancellationToken cancellationToken) => await _db.Credential.Include(c => c.ProviderInstance).FirstOrDefaultAsync(c => c.Id == id && c.DeletedDate == null, cancellationToken).ConfigureAwait(false) ?? throw new KeyNotFoundException($"credential {id} not found"); @@ -312,48 +340,4 @@ private static void MarkRevokedLocally(Credential credential, string? providerEr credential.LastError = providerError; credential.LastValidatedDate = null; } - - /// - /// Repositories bound through this credential lose their auth source the moment it's - /// revoked. We DON'T cascade-delete them (too destructive — disconnect is often part of - /// token rotation, and the user expects to come back), but we DO flip status to - /// with a concrete remediation message so the UI - /// can show "Needs new credential" instead of pretending the repo is still healthy. - /// - /// Webhook ingestion is deliberately untouched — webhooks authenticate via the secret we - /// registered, not via the OAuth token. Events keep flowing in, and once the user - /// re-links to a new credential the repo is fully healthy again with no event gap. - /// - private async Task MarkDependentRepositoriesUnauthorisedAsync(Guid credentialId, CancellationToken cancellationToken) - { - var affected = await _db.Repository - .Where(r => r.CredentialId == credentialId && r.DeletedDate == null && r.Status != RepositoryStatus.Error) - .ToListAsync(cancellationToken).ConfigureAwait(false); - - foreach (var repo in affected) - { - repo.Status = RepositoryStatus.Error; - repo.LastError = "Credential disconnected. Re-link this repository to an active credential of the same provider, or unbind it."; - } - - return affected.Count; - } - - /// - /// A (Model B act-as-user link) is unusable without its backing - /// credential, so revoking the credential — e.g. Disconnect on the Personal tab — must soft-delete the - /// linked identity too. Otherwise the Connected Identities surface keeps showing a dead link, out of - /// sync with the Personal tab. This mirrors what UserProviderIdentityService.UnlinkAsync does - /// from the identity side; tracked + saved by the same unit of work as the revoke, and idempotent - /// (no live identity ⇒ no-op) so the always-run cascade is safe on an already-revoked credential. - /// - private async Task SoftDeleteDependentIdentitiesAsync(Guid credentialId, CancellationToken cancellationToken) - { - var identities = await _db.UserProviderIdentity - .Where(i => i.CredentialId == credentialId && i.DeletedDate == null) - .ToListAsync(cancellationToken).ConfigureAwait(false); - - var now = DateTimeOffset.UtcNow; - foreach (var identity in identities) identity.DeletedDate = now; - } } diff --git a/backend/src/CodeSpace.Core/Services/Credentials/ICredentialRevocationService.cs b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialRevocationService.cs new file mode 100644 index 000000000..ec138c71d --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialRevocationService.cs @@ -0,0 +1,42 @@ +using CodeSpace.Core.Persistence.Entities; + +namespace CodeSpace.Core.Services.Credentials; + +/// +/// What ending a credential does to everything that leans on it. A Disconnect, an identity unlink, and the owner leaving +/// the team each end one, and each used to clean up a different subset: an unlink left the repositories bound through the +/// token Active on a blanked payload, and a departure ended nothing at all. One cascade now serves all three. +/// +public interface ICredentialRevocationService +{ + /// + /// Repositories bound through the credential turn Error with a "needs a new credential" message, and the act-as + /// identities linked to it are soft-deleted. Idempotent. Tracked changes only — the caller's unit of work saves them. + /// Returns how many working repositories it broke. + /// + Task CascadeAsync(Guid credentialId, CancellationToken cancellationToken); + + /// + /// End the credential on this side — blank its token, mark it Revoked and soft-deleted — then cascade. The provider + /// is not asked. Tracked changes only. Returns how many working repositories it broke. + /// + Task RetireAsync(Credential credential, CancellationToken cancellationToken); + + /// + /// Retire every Personal credential lent — the ones they own, and + /// the owner-less ones they added before a token's owner defaulted to whoever added it — with the act-as identities + /// linked to them, unless they still have standing there: what a person lent a team stops being the team's when they + /// leave it, and an instance Admin, who stands in every team, has not left it. Their credentials in any other team are + /// untouched. Reads standing from the database, so the departure's own membership delete must be saved first. Tracked + /// changes only. + /// + Task RetireDepartedInTeamAsync(Guid teamId, Guid userId, CancellationToken cancellationToken); + + /// + /// Retire up to personal tokens whose person no longer has standing in the team they were + /// lent to — no membership there and not an instance Admin — left behind by departures from before departures retired + /// tokens, by an older pod, or by an instance Admin losing the role; each exactly as a departure retires it and saved + /// on its own. A token that cannot be attributed to an account is never one. Returns how many were retired. + /// + Task RetireDepartedAsync(int batchSize, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs b/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs index ed5528f21..63760c455 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs @@ -1,9 +1,11 @@ +using CodeSpace.Core.Authorization; using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.Identity; using CodeSpace.Core.Services.Providers.Identity; +using CodeSpace.Core.Services.Teams; using CodeSpace.Core.Settings.Application; using CodeSpace.Core.Settings.OAuth; using CodeSpace.Messages.Commands.Credentials; @@ -30,10 +32,11 @@ public sealed class OAuthFlowService : IOAuthFlowService, IScopedDependency private readonly ICurrentUser _currentUser; private readonly IUserProviderIdentityService _identityService; private readonly ICredentialSuccessionService _succession; + private readonly ITeamMemberService _members; private readonly OAuthCallbackUrlSetting _callbackUrlSetting; private readonly PublicBaseUrlSetting _publicBaseUrl; - public OAuthFlowService(CodeSpaceDbContext db, IOAuthClientRegistry oauthClients, IOAuthStateStore stateStore, IPkceGenerator pkce, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, ICurrentTeam currentTeam, ICurrentUser currentUser, IUserProviderIdentityService identityService, ICredentialSuccessionService succession, OAuthCallbackUrlSetting callbackUrlSetting, PublicBaseUrlSetting publicBaseUrl) + public OAuthFlowService(CodeSpaceDbContext db, IOAuthClientRegistry oauthClients, IOAuthStateStore stateStore, IPkceGenerator pkce, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, ICurrentTeam currentTeam, ICurrentUser currentUser, IUserProviderIdentityService identityService, ICredentialSuccessionService succession, ITeamMemberService members, OAuthCallbackUrlSetting callbackUrlSetting, PublicBaseUrlSetting publicBaseUrl) { _db = db; _oauthClients = oauthClients; @@ -45,6 +48,7 @@ public OAuthFlowService(CodeSpaceDbContext db, IOAuthClientRegistry oauthClients _currentUser = currentUser; _identityService = identityService; _succession = succession; + _members = members; _callbackUrlSetting = callbackUrlSetting; _publicBaseUrl = publicBaseUrl; } @@ -55,6 +59,7 @@ public async Task InitAsync(Guid providerInstanceId, var instance = await LoadOwnedInstanceAsync(providerInstanceId, cancellationToken).ConfigureAwait(false); var (clientId, _) = ReadOAuthClientCredentials(instance); + var ownerUserId = await ResolveOwnerWithinReachAsync(intendedOwnerUserId, cancellationToken).ConfigureAwait(false); var pkce = _pkce.Generate(); var browserNonce = OAuthBrowserNonce.Mint(); @@ -66,7 +71,7 @@ public async Task InitAsync(Guid providerInstanceId, InitiatorUserId = _currentUser.Id!.Value, CodeVerifier = pkce.Verifier, IntendedDisplayName = displayName, - IntendedOwnerUserId = intendedOwnerUserId ?? _currentUser.Id!.Value, + IntendedOwnerUserId = ownerUserId, ReturnUrl = returnUrl, BrowserNonceHash = OAuthBrowserNonce.Hash(browserNonce), RequestedScopes = scopes ?? instance.OauthDefaultScopes, @@ -94,7 +99,12 @@ public async Task CompleteAsync(string state, str var instance = await LoadInstanceAsync(pending.ProviderInstanceId, cancellationToken).ConfigureAwait(false); var (clientId, clientSecret) = ReadOAuthClientCredentials(instance); + await EnsureOwnerStillInTeamAsync(pending, cancellationToken).ConfigureAwait(false); + var token = await ExchangeCodeAsync(instance, clientId, clientSecret, code, pending.CodeVerifier, cancellationToken).ConfigureAwait(false); + + await HoldOwnerInTeamAsync(pending, cancellationToken).ConfigureAwait(false); + var credential = PersistCredential(pending, instance, token); await _succession.CarryForwardAsync(credential, cancellationToken).ConfigureAwait(false); @@ -142,6 +152,55 @@ private async Task LinkActorIdentityBestEffortAsync(ProviderInstance instance, C } } + /// + /// Who the credential will belong to — the caller unless they name someone — and whom an OAuth connect links as an + /// act-as identity. Naming them is acting on them, so they must have standing in this team and be within the caller's + /// reach, and that is checked for the caller too: a personal token in a team is lent by someone who stands in it — a + /// member, or an instance Admin, who stands in every team. + /// + private async Task ResolveOwnerWithinReachAsync(Guid? intendedOwnerUserId, CancellationToken cancellationToken) + { + var ownerUserId = intendedOwnerUserId ?? _currentUser.Id!.Value; + + await _members.EnsureMayActOnAsync(ownerUserId, cancellationToken).ConfigureAwait(false); + + return ownerUserId; + } + + /// + /// The callback can land up to after Init, and the owner may have left or been removed — or, an + /// instance Admin outside the team, lost the role — in between. A token minted for them now would be a live credential + /// of someone without standing in the team — one the team could bind through, and one the repair sweep would hand their + /// retired bindings to — so the code is not exchanged at all. Runs anonymous, so it asks standing directly + /// () rather than through the caller's reach. + /// + private async Task EnsureOwnerStillInTeamAsync(OAuthPendingState pending, CancellationToken cancellationToken) + { + if (pending.IntendedOwnerUserId is not Guid ownerId) return; + + var teamLive = await _db.Team.AsNoTracking().AnyAsync(t => t.Id == pending.TeamId && t.DeletedDate == null, cancellationToken).ConfigureAwait(false); + + if (!teamLive || !await TeamStanding.HeldAsync(_db, pending.TeamId, ownerId, cancellationToken).ConfigureAwait(false)) + throw OwnerLeftTheTeam(); + } + + /// + /// The check above runs before the provider round-trip, and the owner can leave during it. So the owner's standing is + /// asked again once the token is back, and a member's held until this callback commits: a departure that committed + /// during the exchange refuses the token here, and one that starts now waits for the commit and then retires the token + /// with the rest of theirs. The refused token is not revoked at the provider — revoking an OAuth grant there can end + /// the person's tokens elsewhere — it is simply never stored. + /// + private async Task HoldOwnerInTeamAsync(OAuthPendingState pending, CancellationToken cancellationToken) + { + if (pending.IntendedOwnerUserId is not Guid ownerId) return; + + if (!await _members.HoldMembershipAsync(pending.TeamId, ownerId, cancellationToken).ConfigureAwait(false)) + throw OwnerLeftTheTeam(); + } + + private static OAuthCallbackException OwnerLeftTheTeam() => new("the person this connection was started for no longer has standing in the team"); + private async Task LoadOwnedInstanceAsync(Guid id, CancellationToken cancellationToken) { var instance = await _db.ProviderInstance.AsNoTracking().FirstOrDefaultAsync(p => p.Id == id && p.DeletedDate == null, cancellationToken).ConfigureAwait(false) diff --git a/backend/src/CodeSpace.Core/Services/Providers/Identity/ActorIdentityResolver.cs b/backend/src/CodeSpace.Core/Services/Providers/Identity/ActorIdentityResolver.cs index 21bc41b92..9e4777d27 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/Identity/ActorIdentityResolver.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/Identity/ActorIdentityResolver.cs @@ -1,3 +1,5 @@ +using System.Linq.Expressions; +using CodeSpace.Core.Authorization; using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; @@ -16,15 +18,17 @@ public ActorIdentityResolver(CodeSpaceDbContext db) _db = db; } + /// A live identity on a live Active credential, in the live team that owns the provider instance. + private static readonly Expression> Usable = i => + i.DeletedDate == null && + i.Credential.DeletedDate == null && + i.Credential.Status == CredentialStatus.Active && + i.ProviderInstance.Team.DeletedDate == null; + public async Task ResolveAsync(Guid actorUserId, Guid providerInstanceId, CancellationToken cancellationToken) => - await _db.UserProviderIdentity - .Include(i => i.Credential) - .SingleOrDefaultAsync(i => - i.UserId == actorUserId && - i.ProviderInstanceId == providerInstanceId && - i.DeletedDate == null && - i.Credential.DeletedDate == null && - i.Credential.Status == CredentialStatus.Active, cancellationToken).ConfigureAwait(false); + await MayAct(_db.UserProviderIdentity.Include(i => i.Credential)) + .Where(i => i.UserId == actorUserId && i.ProviderInstanceId == providerInstanceId) + .SingleOrDefaultAsync(cancellationToken).ConfigureAwait(false); public async Task> ListCandidatesAsync(Guid repositoryId, Guid teamId, CancellationToken cancellationToken) { @@ -36,24 +40,10 @@ public async Task> ListCandidatesAsync(Guid if (providerInstanceId == null) return Array.Empty(); - // Team users — only they may be offered as an author. The owner used to be fetched separately - // and appended, because they could be recorded on the team row alone; they hold a membership - // row like everyone else now. - var teamUserIds = await _db.TeamMembership.AsNoTracking() - .Where(m => m.TeamId == teamId && m.Team.DeletedDate == null) - .Select(m => m.UserId) - .ToListAsync(cancellationToken).ConfigureAwait(false); - - if (teamUserIds.Count == 0) return Array.Empty(); - - // Every teammate with a LIVE, usable identity on this provider instance — the EXACT predicate - // ResolveAsync uses, so every candidate is guaranteed to resolve (no throw) at write time. - var identities = await _db.UserProviderIdentity.AsNoTracking() - .Where(i => i.ProviderInstanceId == providerInstanceId && - i.DeletedDate == null && - i.Credential.DeletedDate == null && - i.Credential.Status == CredentialStatus.Active && - teamUserIds.Contains(i.UserId)) + // Every teammate who may act on this provider instance — the EXACT predicate ResolveAsync uses, so every + // candidate is guaranteed to resolve (no throw) at write time. + var identities = await MayAct(_db.UserProviderIdentity.AsNoTracking()) + .Where(i => i.ProviderInstanceId == providerInstanceId) .Select(i => new { i.UserId, i.ProviderUsername, i.ProviderUserId, i.AvatarUrl }) .ToListAsync(cancellationToken).ConfigureAwait(false); @@ -80,4 +70,18 @@ public async Task> ListCandidatesAsync(Guid .OrderBy(c => c.Name) .ToList(); } + + /// + /// Whether an identity may act, and the ONLY place that says so: the picker and the write both filter through it, so + /// every candidate the picker offers resolves and nobody the picker hides can be written through. A usable identity, + /// and a user who still has standing () in the team that owns the provider instance — a + /// membership row there, or the instance Admin role, which stands in every team. + /// + /// The standing half, and the live team, are what the write used to lack; only the picker asked them. Leaving or + /// being removed deletes the membership row and nothing else could see it here, so a workflow any Member authored could + /// name a departed person in actAsUserId and write through their personal token. The instance is team-scoped, + /// so its team is the one whose standing counts — no caller has to pass it. + /// + private IQueryable MayAct(IQueryable identities) => + identities.Where(Usable).Where(TeamStanding.Of(_db, i => i.ProviderInstance.TeamId, i => i.UserId)); } diff --git a/backend/src/CodeSpace.Core/Services/Providers/Identity/IActorIdentityResolver.cs b/backend/src/CodeSpace.Core/Services/Providers/Identity/IActorIdentityResolver.cs index 6cb2f3ce1..0f1a53c32 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/Identity/IActorIdentityResolver.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/Identity/IActorIdentityResolver.cs @@ -16,8 +16,10 @@ public interface IActorIdentityResolver { /// /// The user's live, usable identity on the instance, or null. "Usable" = the identity is not - /// soft-deleted AND its linked credential is present and Active — so a revoked / expired / removed - /// credential resolves to null exactly like "never linked". + /// soft-deleted, its linked credential is present and Active, AND the user still has standing in the + /// live team that owns the instance — a membership row there, or the instance Admin role — so a + /// revoked / expired / removed credential, or a person who left the team, resolves to null exactly + /// like "never linked". /// Task ResolveAsync(Guid actorUserId, Guid providerInstanceId, CancellationToken cancellationToken); diff --git a/backend/src/CodeSpace.Core/Services/Providers/Identity/IUserProviderIdentityService.cs b/backend/src/CodeSpace.Core/Services/Providers/Identity/IUserProviderIdentityService.cs index 5f572d962..9d67d3a99 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/Identity/IUserProviderIdentityService.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/Identity/IUserProviderIdentityService.cs @@ -22,7 +22,7 @@ public interface IUserProviderIdentityService /// The caller's live linked identities (one per provider instance), newest first. Task> ListMineAsync(CancellationToken cancellationToken); - /// Unlink one of the caller's identities (soft-delete the identity + its credential). No-op if already gone. + /// Unlink one of the caller's identities: soft-delete it and retire its credential, cascading to the repositories bound through it. No-op if already gone. Task UnlinkAsync(Guid identityId, CancellationToken cancellationToken); /// @@ -30,8 +30,10 @@ public interface IUserProviderIdentityService /// backed by an ALREADY-created credential (the OAuth-complete path — the credential is built by /// the OAuth flow, not here). Probes the credential for the provider user, then either re-points /// the user's existing live identity to this credential or creates a new one. NON-destructive: it - /// never revokes another credential (unlike the PAT replace-in-place path). The caller persists — - /// no SaveChanges here. Throws if the probe fails (caller decides whether that's fatal). + /// never revokes another credential (unlike the PAT replace-in-place path). Does nothing unless + /// is who connected the credential — nobody links an identity for someone + /// else. The caller persists — no SaveChanges here. Throws if the probe fails (caller decides whether + /// that's fatal). /// Task EnsureIdentityForCredentialAsync(ProviderInstance instance, Credential credential, Guid userId, CancellationToken cancellationToken); } diff --git a/backend/src/CodeSpace.Core/Services/Providers/Identity/UserProviderIdentityService.cs b/backend/src/CodeSpace.Core/Services/Providers/Identity/UserProviderIdentityService.cs index 8e24fce15..ec7ed6faf 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/Identity/UserProviderIdentityService.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/Identity/UserProviderIdentityService.cs @@ -1,9 +1,11 @@ +using CodeSpace.Core.Authorization; using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.Identity; using CodeSpace.Core.Services.Providers.Capabilities; +using CodeSpace.Core.Services.Teams; using CodeSpace.Messages.Credentials; using CodeSpace.Messages.Dtos.Providers; using CodeSpace.Messages.Enums; @@ -20,8 +22,10 @@ public sealed class UserProviderIdentityService : IUserProviderIdentityService, private readonly ICredentialPayloadSerializer _serializer; private readonly IProviderRegistry _registry; private readonly ICredentialSuccessionService _succession; + private readonly ICredentialRevocationService _revocation; + private readonly ITeamMemberService _members; - public UserProviderIdentityService(CodeSpaceDbContext db, ICurrentUser currentUser, ICurrentTeam currentTeam, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IProviderRegistry registry, ICredentialSuccessionService succession) + public UserProviderIdentityService(CodeSpaceDbContext db, ICurrentUser currentUser, ICurrentTeam currentTeam, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IProviderRegistry registry, ICredentialSuccessionService succession, ICredentialRevocationService revocation, ITeamMemberService members) { _db = db; _currentUser = currentUser; @@ -30,6 +34,8 @@ public UserProviderIdentityService(CodeSpaceDbContext db, ICurrentUser currentUs _serializer = serializer; _registry = registry; _succession = succession; + _revocation = revocation; + _members = members; } public async Task LinkByPatAsync(Guid providerInstanceId, string accessToken, CancellationToken cancellationToken) @@ -44,6 +50,8 @@ public async Task LinkByPatAsync(Guid providerInsta var credential = BuildPersonalCredential(instance, userId, teamId, accessToken); var profile = await ProbeOrThrowAsync(instance, credential, cancellationToken).ConfigureAwait(false); + await HoldCallerInTeamAsync(userId, teamId, cancellationToken).ConfigureAwait(false); + credential.DisplayName = $"{instance.Provider} · {profile.AuthenticatedUserName}"; // Persist the token's real granted scopes (the probe reads them live) so capability warnings @@ -94,20 +102,20 @@ public async Task UnlinkAsync(Guid identityId, CancellationToken cancellationTok if (identity == null) return; - var now = DateTimeOffset.UtcNow; - identity.DeletedDate = now; + identity.DeletedDate = DateTimeOffset.UtcNow; - // Clear the token material + soft-delete the backing credential — an unlinked identity must - // leave nothing usable at rest. - identity.Credential.DeletedDate = now; - identity.Credential.Status = CredentialStatus.Revoked; - identity.Credential.EncryptedPayload = string.Empty; + // Retire the backing credential through the same cascade a Disconnect runs — an unlinked identity must leave + // nothing usable at rest, and a repository bound through its token must say it needs a new one rather than + // stay Active on a blanked payload. Still the caller's own token, from whichever team they ask. + await _revocation.RetireAsync(identity.Credential, cancellationToken).ConfigureAwait(false); await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); } public async Task EnsureIdentityForCredentialAsync(ProviderInstance instance, Credential credential, Guid userId, CancellationToken cancellationToken) { + if (!ConnectedBy(credential, userId)) return; + var profile = await ProbeOrThrowAsync(instance, credential, cancellationToken).ConfigureAwait(false); var existing = await _db.UserProviderIdentity @@ -129,6 +137,26 @@ public async Task EnsureIdentityForCredentialAsync(ProviderInstance instance, Cr await _db.UserProviderIdentity.AddAsync(identity, cancellationToken).ConfigureAwait(false); } + /// + /// The request was let in on the caller's standing, but the provider check runs between that and the insert, and the + /// caller can leave — or be removed — during it. Asked again and, for a member, held until this link commits: a + /// departure that already committed refuses the link, one that starts now waits and then retires the new token with + /// the rest of theirs. A personal token in a team is lent by someone who stands in it — a member, or an instance Admin, + /// who stands in every team (). + /// + private async Task HoldCallerInTeamAsync(Guid userId, Guid teamId, CancellationToken cancellationToken) + { + if (!await _members.HoldMembershipAsync(teamId, userId, cancellationToken).ConfigureAwait(false)) + throw new TenantAccessDeniedException(userId, teamId, "user is not a member of this team"); + } + + /// + /// An identity says "this provider account speaks for this person", and only the person can say that — by connecting + /// the account themselves. A token someone else connected in their name is whatever account THAT caller signed into + /// at the provider; linking it would let them choose who the person's writes are attributed to. + /// + private static bool ConnectedBy(Credential credential, Guid userId) => credential.CreatedBy == userId; + private async Task LoadInstanceAsync(Guid providerInstanceId, Guid teamId, CancellationToken cancellationToken) => await _db.ProviderInstance .SingleOrDefaultAsync(i => i.Id == providerInstanceId && i.TeamId == teamId, cancellationToken).ConfigureAwait(false) diff --git a/backend/src/CodeSpace.Core/Services/RepositoryBinding/RepositoryBindingService.cs b/backend/src/CodeSpace.Core/Services/RepositoryBinding/RepositoryBindingService.cs index 2a64102ea..9a79dc10e 100644 --- a/backend/src/CodeSpace.Core/Services/RepositoryBinding/RepositoryBindingService.cs +++ b/backend/src/CodeSpace.Core/Services/RepositoryBinding/RepositoryBindingService.cs @@ -259,9 +259,10 @@ private async Task LoadProviderInstanceAsync(BindContext ctx, Cance return ctx with { Instance = instance }; } + /// A soft-deleted credential — unlinked, or retired when its owner left the team — is gone, and nothing binds through it again. private async Task LoadCredentialAsync(BindContext ctx, CancellationToken cancellationToken) { - var credential = await _db.Credential.SingleOrDefaultAsync(c => c.Id == ctx.Request.CredentialId && c.TeamId == ctx.Request.TeamId, cancellationToken).ConfigureAwait(false) + var credential = await _db.Credential.SingleOrDefaultAsync(c => c.Id == ctx.Request.CredentialId && c.TeamId == ctx.Request.TeamId && c.DeletedDate == null, cancellationToken).ConfigureAwait(false) ?? throw new InvalidOperationException($"Credential {ctx.Request.CredentialId} not found in team {ctx.Request.TeamId}"); return ctx with { Credential = credential }; diff --git a/backend/src/CodeSpace.Core/Services/Teams/ITeamMemberService.cs b/backend/src/CodeSpace.Core/Services/Teams/ITeamMemberService.cs index 2901e1b2c..0a772ee8f 100644 --- a/backend/src/CodeSpace.Core/Services/Teams/ITeamMemberService.cs +++ b/backend/src/CodeSpace.Core/Services/Teams/ITeamMemberService.cs @@ -12,4 +12,21 @@ public interface ITeamMemberService Task LeaveAsync(CancellationToken cancellationToken); Task TransferOwnershipAsync(Guid toUserId, CancellationToken cancellationToken); + + /// + /// Throws unless has standing in the current team — a member, or an instance Admin, who + /// stands in every team as an Owner — and the caller may act on them: themselves, or someone below them. A user + /// without standing is refused exactly like an id that names nobody. Holds their standing as + /// does. + /// + Task EnsureMayActOnAsync(Guid userId, CancellationToken cancellationToken); + + /// + /// Whether has standing in the live team — a membership row there, + /// or the instance Admin role — and, for a member, keeps them one until the caller's transaction ends: their leaving or + /// removal waits for it to commit, then retires what it wrote in their name. For a write in someone's name that must + /// not land after they have gone, such as a token they own; the check before a slow provider call is not enough on its + /// own. Outside a transaction it holds nothing. + /// + Task HoldMembershipAsync(Guid teamId, Guid userId, CancellationToken cancellationToken); } diff --git a/backend/src/CodeSpace.Core/Services/Teams/TeamMemberService.cs b/backend/src/CodeSpace.Core/Services/Teams/TeamMemberService.cs index ff2a10e69..91691559f 100644 --- a/backend/src/CodeSpace.Core/Services/Teams/TeamMemberService.cs +++ b/backend/src/CodeSpace.Core/Services/Teams/TeamMemberService.cs @@ -2,6 +2,7 @@ using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.Identity; using CodeSpace.Messages.Authorization; using CodeSpace.Messages.Enums; @@ -30,13 +31,15 @@ public sealed class TeamMemberService : ITeamMemberService, IScopedDependency private readonly ICurrentUser _currentUser; private readonly ICurrentTeam _currentTeam; private readonly TeamMembershipResolver _membership; + private readonly ICredentialRevocationService _revocation; - public TeamMemberService(CodeSpaceDbContext db, ICurrentUser currentUser, ICurrentTeam currentTeam, TeamMembershipResolver membership) + public TeamMemberService(CodeSpaceDbContext db, ICurrentUser currentUser, ICurrentTeam currentTeam, TeamMembershipResolver membership, ICredentialRevocationService revocation) { _db = db; _currentUser = currentUser; _currentTeam = currentTeam; _membership = membership; + _revocation = revocation; } public async Task ChangeRoleAsync(Guid userId, TeamRole role, CancellationToken cancellationToken) @@ -80,6 +83,48 @@ public async Task LeaveAsync(CancellationToken cancellationToken) await RemoveMembershipAsync(teamId, membership, cancellationToken).ConfigureAwait(false); } + /// + /// The same reach as every other act on a member, asked by things that act on a person without changing their + /// membership — naming them as a credential's owner, say. They must have standing in this live team, and the caller + /// must be them or outrank them. Someone without standing fails exactly like an id that names nobody. Held until the + /// caller commits (), so what it writes in their name cannot land after they have gone. + /// + public async Task EnsureMayActOnAsync(Guid userId, CancellationToken cancellationToken) + { + var teamId = RequireTeam(); + + if (!await HoldMembershipAsync(teamId, userId, cancellationToken).ConfigureAwait(false)) + throw new KeyNotFoundException("that person is not a member of this team"); + + var actor = await _membership.ResolveRoleAsync(teamId, cancellationToken).ConfigureAwait(false); + var target = await ResolveStandingRoleAsync(teamId, userId, cancellationToken).ConfigureAwait(false); + + EnsureMayActOn(actor, target, isSelf: userId == RequireUser()); + } + + /// + /// Whether they have standing (), held until the caller's transaction ends. A member's + /// membership row is share-locked, after a share lock on the live team's row: it conflicts only with a departure's + /// delete of that membership row () and the owner guard's lock on the team row + /// (), so everything else in the team goes on beside it. Team row first, as an Owner's + /// departure takes them: holding only the membership row, the write's own insert would wait on the team row that + /// departure holds while the departure waits on the membership row. + /// + /// An instance Admin outside the team has no row to hold, and none is held for the role: nothing in the app + /// takes it away (role_user is written by migrations and by hand), so there is no departure to serialise with. + /// A demotion that commits before this answers is refused here; one that commits after leaves a token the departure + /// sweep retires on its next tick — the same window every token an ex-Admin lent already has between the demotion and + /// that tick, and one in which nobody can act as them, because the act-as resolver asks standing at the write. + /// + public async Task HoldMembershipAsync(Guid teamId, Guid userId, CancellationToken cancellationToken) + { + if (!await ShareLockLiveTeamAsync(teamId, cancellationToken).ConfigureAwait(false)) return false; + + await ShareLockMembershipAsync(teamId, userId, cancellationToken).ConfigureAwait(false); + + return await TeamStanding.HeldAsync(_db, teamId, userId, cancellationToken).ConfigureAwait(false); + } + /// /// Hands the team over in one step: the recipient becomes Owner and the outgoing owner becomes /// Admin. @@ -145,6 +190,22 @@ private static void EnsureMayGrant(TeamRole actor, TeamRole role) if (TeamRoleRank.Of(role) > TeamRoleRank.Of(actor)) throw new RoleOutranksActorException(actor, role); } + /// + /// Leaving and being removed end the same things, in the same transaction: the membership row, and every provider + /// token the person lent the team. A personal token is that person's account — kept after they go, the team could + /// still list it, browse what it reaches, bind more through it, and act as them. What is bound through it turns "needs + /// a new credential" rather than silently keep a token nobody here may use. + /// + /// The row's delete is saved before the retirement reads what goes, because what goes is decided by the + /// departure sweep's own rule — no standing left in the team — and an instance Admin still stands in it without the + /// row, so keeps what they lent. The mediator's transaction spans both saves. + /// + /// The delete is also what serialises a departure with every write in the person's name. It takes the row's + /// exclusive lock, so it waits for each write holding that row () to commit, and keeps + /// any new one out until this departure commits — after which the row is gone and the write refuses. The retirement + /// reads after the delete, so it sees a token such a write committed while the departure waited, and retires it with + /// the rest instead of leaving it live for someone outside the team. + /// private async Task RemoveMembershipAsync(Guid teamId, TeamMembership membership, CancellationToken cancellationToken) { if (membership.Role == TeamRole.Owner) await EnsureAnotherOwnerRemainsAsync(teamId, membership.UserId, cancellationToken).ConfigureAwait(false); @@ -152,6 +213,10 @@ private async Task RemoveMembershipAsync(Guid teamId, TeamMembership membership, _db.TeamMembership.Remove(membership); await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + + await _revocation.RetireDepartedInTeamAsync(teamId, membership.UserId, cancellationToken).ConfigureAwait(false); + + await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); } /// @@ -187,6 +252,24 @@ private async Task EnsureAnotherOwnerRemainsAsync(Guid teamId, Guid leavingUserI private async Task LockTeamAsync(Guid teamId, CancellationToken cancellationToken) => await _db.Database.ExecuteSqlInterpolatedAsync($"SELECT id FROM team WHERE id = {teamId} FOR UPDATE", cancellationToken).ConfigureAwait(false); + private async Task ShareLockLiveTeamAsync(Guid teamId, CancellationToken cancellationToken) => + (await _db.Database.SqlQuery($"SELECT 1 AS \"Value\" FROM team WHERE id = {teamId} AND deleted_date IS NULL FOR KEY SHARE").ToListAsync(cancellationToken).ConfigureAwait(false)).Count > 0; + + private async Task ShareLockMembershipAsync(Guid teamId, Guid userId, CancellationToken cancellationToken) => + await _db.Database.SqlQuery($"SELECT 1 AS \"Value\" FROM team_membership WHERE team_id = {teamId} AND user_id = {userId} FOR KEY SHARE").ToListAsync(cancellationToken).ConfigureAwait(false); + + /// + /// The rank a person with standing is acted on at: their membership's role, read as and + /// read it; or, for someone who stands in the team without one — only an instance Admin can — + /// an Owner's, the rank gives that role in every team. + /// + private async Task ResolveStandingRoleAsync(Guid teamId, Guid userId, CancellationToken cancellationToken) + { + var membership = await _db.TeamMembership.AsNoTracking().SingleOrDefaultAsync(m => m.TeamId == teamId && m.UserId == userId, cancellationToken).ConfigureAwait(false); + + return membership?.Role ?? TeamRole.Owner; + } + private async Task EnsureTeamLiveAsync(Guid teamId, CancellationToken cancellationToken) { var live = await _db.Team.AsNoTracking().AnyAsync(t => t.Id == teamId && t.DeletedDate == null, cancellationToken).ConfigureAwait(false); diff --git a/backend/src/CodeSpace.Messages/Commands/Credentials/RetireDepartedMembersCredentialsCommand.cs b/backend/src/CodeSpace.Messages/Commands/Credentials/RetireDepartedMembersCredentialsCommand.cs new file mode 100644 index 000000000..6f3ee8765 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Commands/Credentials/RetireDepartedMembersCredentialsCommand.cs @@ -0,0 +1,25 @@ +using CodeSpace.Messages.Mediation; + +namespace CodeSpace.Messages.Commands.Credentials; + +/// +/// Retire every personal token still live in a team its person no longer has standing in — no membership there, and not +/// an instance Admin, who stands in every team as an Owner does — the person being the token's owner, or, for a token +/// stored without one, whoever added it; a token whose creator stamp names no account is never retired on a guess. +/// Leaving or being removed retires them in the same transaction; this retires the ones a departure left behind anyway: +/// every departure from before departures retired anything, any an older pod serves during a rolling deploy, and every +/// token an instance Admin lent a team they are not in, once they lose the role — the app never takes the role away, so +/// no departure retires those. Each token is retired exactly as a departure retires it — blanked, Revoked, +/// soft-deleted, its repositories marked "needs a new credential", its act-as identities soft-deleted — and the provider +/// is not asked. Idempotent and bounded: a retired token is no longer a candidate, so the backlog only shrinks. NOT +/// tenant-scoped: a system-wide repair that runs without an actor context (mirrors RepairDisconnectedCredentialBindingsCommand). +/// +/// NOT transactional (): each token is retired and saved on its own, and a +/// token that fails stays a candidate while the pass carries on. One transaction around the pass would let one failing +/// token undo every other one's retirement. +/// +public sealed record RetireDepartedMembersCredentialsCommand : ICommand, INonTransactionalCommand +{ + /// Tokens retired per tick — bounds each pass. + public int BatchSize { get; init; } = 50; +} diff --git a/backend/src/CodeSpace.Messages/Credentials/AddCredentialInput.cs b/backend/src/CodeSpace.Messages/Credentials/AddCredentialInput.cs index ca1d33ca4..1cddac8d8 100644 --- a/backend/src/CodeSpace.Messages/Credentials/AddCredentialInput.cs +++ b/backend/src/CodeSpace.Messages/Credentials/AddCredentialInput.cs @@ -10,7 +10,7 @@ public sealed record AddCredentialInput { public required Guid ProviderInstanceId { get; init; } - /// The owning user for a Personal credential; ignored (forced null) for TeamService. + /// The owning user for a Personal credential — the caller when unset; ignored (forced null) for TeamService. public Guid? OwnerUserId { get; init; } public required string DisplayName { get; init; } diff --git a/backend/tests/CodeSpace.E2ETests/Auth/DepartedMemberCredentialE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Auth/DepartedMemberCredentialE2ETests.cs new file mode 100644 index 000000000..6f3ed3a87 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Auth/DepartedMemberCredentialE2ETests.cs @@ -0,0 +1,581 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using System.Text.Json; +using Autofac; +using Autofac.Extensions.DependencyInjection; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.IntegrationTests.Binding; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; + +namespace CodeSpace.E2ETests.Auth; + +/// +/// A person's own provider token, lent to a team while they belong to it, through the whole HTTP surface — and what is +/// left of it once they leave or are removed. +/// +/// Tier: 🟢 High-fidelity (Rule 12) — the real ASP.NET pipeline (routing, JWT auth, the X-Team-Id scope, +/// model binding, the controllers, the exception filter, the mediator and its authorization behaviors), the real +/// workflow engine drained in-process, and real Postgres. One documented seam: the provider is the Git test provider, +/// which answers whoami, lists three fixed repositories, and records every PR open in +/// — so "the write never reached the provider" is observed, not inferred — +/// with a loopback OAuth client for its authorize URL. Two teams per test: the departing person is a Member of one and +/// the Owner of their own. +/// +/// Must hold, for both ways out (removed, left): the token stops being the team's — not listed, not browsable, +/// not bindable, not usable to act as them, not even by a run launched before they went — while the token they lent +/// their other team keeps working; the same for a token added without naming an owner, and for a departure from before +/// departures retired tokens once the departure sweep has run; an owner id outside the caller's reach is refused on both +/// connect paths, with one answer for "elsewhere" and "nowhere"; and unlinking an identity breaks, visibly, the +/// repositories bound through its token. And an instance Admin, who stands in every team without a membership row, lends a +/// team a token that keeps working — sweep included — until the role is taken away. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class DepartedMemberCredentialE2ETests : IClassFixture +{ + private const int MaxSweepPasses = 10; + + /// The departure sweep is deployment-wide: every earlier test's leftover token of someone without standing is a candidate ahead of ours, fifty a pass. + private const int MaxDrainPasses = 50; + + private readonly GitProviderApiFactory _factory; + + public DepartedMemberCredentialE2ETests(GitProviderApiFactory factory) { _factory = factory; } + + public enum Departure { Removed, Left } + + [Theory] + [InlineData(Departure.Removed)] + [InlineData(Departure.Left)] + public async Task A_departed_members_token_stops_working_for_the_team_they_left(Departure departure) + { + var world = await SeedWorldAsync(); + var identityA = await LinkTokenAsync(world.DepartingId, world.TeamA, world.InstanceA, "departing-token-a"); + var identityB = await LinkTokenAsync(world.DepartingId, world.TeamB, world.InstanceB, "departing-token-b"); + var tokenA = await IdentityCredentialAsync(identityA); + var title = $"opened as a departed member {Guid.NewGuid():N}"; + + await ShouldSucceedAsync(await BindAsync(world.OwnerId, world.TeamA, world.InstanceA, tokenA, "acme/web"), "fixture check: while they are in the team, an Owner can bind through their token"); + await _factory.JobClient.DrainAsync(); + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.MemberId, world.TeamA), "fixture check: list")).ShouldContain(tokenA.ToString()); + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, $"/api/repositories/{world.ConnectionRepository}/act-as-candidates", world.MemberId, world.TeamA), "fixture check: picker")).ShouldContain(world.DepartingId.ToString()); + + var whileMember = await RunOpenPullRequestAsAsync(world, world.DepartingId, $"{title} while a member"); + whileMember.Status.ShouldBe(WorkflowRunStatus.Success, $"fixture check: while they are in the team, a workflow acts as them — node said: {whileMember.NodeError}"); + _factory.PullRequestOpens.All.Select(o => o.Title).ShouldContain($"{title} while a member", "fixture check: the act-as write reached the provider"); + + await DepartAsync(world, departure); + + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.MemberId, world.TeamA), "list")).ShouldNotContain(tokenA.ToString(), Case.Insensitive, $"{departure}: a Member no longer sees the departed member's token"); + + var browse = await SendAsync(HttpMethod.Get, $"/api/credentials/{tokenA}/accessible-repositories", world.MemberId, world.TeamA); + browse.StatusCode.ShouldBeOneOf(new[] { HttpStatusCode.Forbidden, HttpStatusCode.NotFound }, $"{departure}: a Member can no longer enumerate what the departed member's account reaches"); + + var bind = await BindAsync(world.OwnerId, world.TeamA, world.InstanceA, tokenA, "acme/cli"); + ((int)bind.StatusCode).ShouldBeInRange(400, 499, $"{departure}: an Owner can no longer bind through it — {await bind.Content.ReadAsStringAsync()}"); + + var run = await RunOpenPullRequestAsAsync(world, world.DepartingId, title); + ShouldBeRefusedForWantOfAnIdentity(run, $"{departure}: a workflow naming the departed member as its author"); + _factory.PullRequestOpens.All.Select(o => o.Title).ShouldNotContain(title, $"{departure}: the refusal came before any provider call — the departed member's token was never presented"); + + await ShouldBeRetiredAsync(tokenA, identityA, departure); + + var theirs = await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/me/identities", world.DepartingId, world.TeamB), "their identities, from the team they are still in"); + theirs.ShouldContain(identityB.ToString(), Case.Insensitive, $"{departure}: the identity they linked in their own team is untouched"); + theirs.ShouldNotContain(identityA.ToString(), Case.Insensitive, $"{departure}: nothing of theirs is left live on the team they left"); + } + + [Theory] + [InlineData(Departure.Removed)] + [InlineData(Departure.Left)] + public async Task A_run_launched_while_its_author_was_a_member_never_writes_through_their_token_after_they_go(Departure departure) + { + // The run is accepted and queued while the person it acts as still belongs to the team; it executes after they + // have gone. Whoever it acts as is decided when the write happens, not when the run was launched, so the queued + // act-as node meets the departure and is refused before the provider is asked. + var world = await SeedWorldAsync(); + await LinkTokenAsync(world.DepartingId, world.TeamA, world.InstanceA, "departing-token-a"); + var title = $"in flight across a departure {Guid.NewGuid():N}"; + + var runId = await LaunchOpenPullRequestAsAsync(world, world.DepartingId, title); + + await DepartAsync(world, departure); + await _factory.JobClient.DrainAsync(); + + ShouldBeRefusedForWantOfAnIdentity(await ReadRunAsync(runId), $"{departure}: the run launched before the departure"); + _factory.PullRequestOpens.All.Select(o => o.Title).ShouldNotContain(title, $"{departure}: the in-flight run never presented the departed member's token"); + } + + public enum ConnectPath { Token, OAuth } + + [Theory] + [InlineData(ConnectPath.Token)] + [InlineData(ConnectPath.OAuth)] + public async Task Naming_a_credential_owner_outside_your_reach_is_refused_with_one_answer_for_elsewhere_and_nowhere(ConnectPath path) + { + var world = await SeedWorldAsync(); + var elsewhere = await SeedUserInOwnTeamAsync("Victoria Elsewhere"); + + var foreign = await StartConnectForAsync(path, world, elsewhere); + var nowhere = await StartConnectForAsync(path, world, Guid.NewGuid()); + var aboveYou = await StartConnectForAsync(path, world, world.OwnerId); + var belowYou = await StartConnectForAsync(path, world, world.MemberId); + + foreign.StatusCode.ShouldBe(HttpStatusCode.NotFound, $"{path}: a user of another tenant is not someone this team can name"); + nowhere.StatusCode.ShouldBe(HttpStatusCode.NotFound, $"{path}: an invented id is refused before the foreign key can turn it into a 500"); + (await nowhere.Content.ReadAsStringAsync()).ShouldBe(await foreign.Content.ReadAsStringAsync(), $"{path}: elsewhere and nowhere must read the same — otherwise the endpoint tells a real account from an invented one"); + aboveYou.StatusCode.ShouldBe(HttpStatusCode.Forbidden, $"{path}: an Admin does not act on the Owner, so cannot connect a credential in the Owner's name"); + await ShouldSucceedAsync(belowYou, $"control: an Admin may start a {path} connect for a Member below them"); + + var listed = await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.AdminId, world.TeamA), "list"); + listed.ShouldNotContain(elsewhere.ToString(), Case.Insensitive, $"{path}: no credential carries the foreign account's id"); + listed.ShouldNotContain("Victoria Elsewhere", Case.Insensitive, $"{path}: nor discloses the foreign account's display name"); + (await PendingConnectsForAsync(elsewhere)).ShouldBe(0, $"{path}: no connect waits for a callback in the foreign account's name"); + } + + [Fact] + public async Task Unlinking_an_identity_breaks_the_repositories_bound_through_its_token() + { + var world = await SeedWorldAsync(); + var identity = await LinkTokenAsync(world.DepartingId, world.TeamA, world.InstanceA, "token-to-unlink"); + var token = await IdentityCredentialAsync(identity); + var bound = JsonDocument.Parse(await ShouldSucceedAsync(await BindAsync(world.OwnerId, world.TeamA, world.InstanceA, token, "acme/web"), "bind through the token")).RootElement.GetProperty("id").GetGuid(); + await _factory.JobClient.DrainAsync(); + + var unlink = await SendAsync(HttpMethod.Delete, $"/api/me/identities/{identity}", world.DepartingId, world.TeamA); + unlink.StatusCode.ShouldBe(HttpStatusCode.NoContent); + + var repo = await LoadRepositoryAsync(bound); + repo.Status.ShouldBe(RepositoryStatus.Error, "a repository bound through an unlinked token says it needs a new credential instead of failing at its next run"); + repo.LastError.ShouldNotBeNullOrWhiteSpace(); + } + + [Fact] + public async Task A_token_added_without_naming_an_owner_goes_when_whoever_added_it_leaves() + { + var world = await SeedWorldAsync(); + + await ShouldSucceedAsync(await SendAsync(HttpMethod.Patch, $"/api/teams/members/{world.DepartingId}", world.OwnerId, world.TeamA, new { role = "Admin" }), "fixture: the Owner makes them an Admin, who may add tokens"); + + var added = await ShouldSucceedAsync(await SendAsync(HttpMethod.Post, "/api/credentials/pat", world.DepartingId, world.TeamA, new { providerInstanceId = world.InstanceA, displayName = "my token", token = "glpat-mine" }), "add a token without naming an owner"); + var tokenId = JsonDocument.Parse(added).RootElement.GetProperty("id").GetGuid(); + + var listed = await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.MemberId, world.TeamA), "fixture check: list"); + listed.ShouldContain(tokenId.ToString(), Case.Insensitive, "fixture check: while they are in the team, the token is one of its credentials"); + listed.ShouldContain(world.DepartingId.ToString(), Case.Insensitive, "the token is listed as theirs — a personal token added without naming an owner is the caller's own"); + + await DepartAsync(world, Departure.Left); + + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.MemberId, world.TeamA), "list")).ShouldNotContain(tokenId.ToString(), Case.Insensitive, "a Member no longer sees the token of the person who added it and left"); + + var bind = await BindAsync(world.OwnerId, world.TeamA, world.InstanceA, tokenId, "acme/web"); + ((int)bind.StatusCode).ShouldBeInRange(400, 499, $"an Owner can no longer bind through it — {await bind.Content.ReadAsStringAsync()}"); + + var token = await LoadCredentialAsync(tokenId); + token.Status.ShouldBe(CredentialStatus.Revoked); + token.EncryptedPayload.ShouldBe(string.Empty); + token.DeletedDate.ShouldNotBeNull(); + } + + [Fact] + public async Task A_member_who_left_before_departures_retired_tokens_loses_them_once_the_sweep_runs() + { + // The state every departure before this change left behind, and every departure an older pod serves during the + // rolling deploy: the membership row is gone and the token is still the team's. + var world = await SeedWorldAsync(); + var identityA = await LinkTokenAsync(world.DepartingId, world.TeamA, world.InstanceA, "departing-token-a"); + var identityB = await LinkTokenAsync(world.DepartingId, world.TeamB, world.InstanceB, "departing-token-b"); + var tokenA = await IdentityCredentialAsync(identityA); + var tokenB = await IdentityCredentialAsync(identityB); + + await ShouldSucceedAsync(await BindAsync(world.OwnerId, world.TeamA, world.InstanceA, tokenA, "acme/web"), "fixture: an Owner binds through their token while they are in the team"); + await _factory.JobClient.DrainAsync(); + + await DeleteMembershipAsDeparturesUsedToAsync(world.TeamA, world.DepartingId); + + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.MemberId, world.TeamA), "fixture check: list")).ShouldContain(tokenA.ToString(), Case.Insensitive, "fixture check: a departure from before left the token listed"); + + await SweepDepartedUntilRetiredAsync(tokenA); + + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.MemberId, world.TeamA), "list")).ShouldNotContain(tokenA.ToString(), Case.Insensitive, "a Member no longer sees the departed member's token"); + + var browse = await SendAsync(HttpMethod.Get, $"/api/credentials/{tokenA}/accessible-repositories", world.MemberId, world.TeamA); + browse.StatusCode.ShouldBeOneOf(new[] { HttpStatusCode.Forbidden, HttpStatusCode.NotFound }, "a Member can no longer enumerate what the departed member's account reaches"); + + var bind = await BindAsync(world.OwnerId, world.TeamA, world.InstanceA, tokenA, "acme/cli"); + ((int)bind.StatusCode).ShouldBeInRange(400, 499, $"an Owner can no longer bind through it — {await bind.Content.ReadAsStringAsync()}"); + + (await LoadRepositoryAsync(await RepositoryBoundAsync(tokenA, "acme/web"))).Status.ShouldBe(RepositoryStatus.Error, "the repository bound through it says it needs a new credential"); + + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.DepartingId, world.TeamB), "their own team's list")).ShouldContain(tokenB.ToString(), Case.Insensitive, "the token they lent the team they are still in is untouched"); + (await LoadCredentialAsync(tokenB)).Status.ShouldBe(CredentialStatus.Active); + } + + [Fact] + public async Task An_instance_admin_outside_the_team_lends_it_a_token_that_stays_until_they_lose_the_role() + { + // A live instance Admin stands in every team as an Owner does — the pipeline lets them in without a membership row + // — and lends a team a token without joining it. That must keep working: connecting, binding through the token, a + // workflow acting as them, and the departure sweep leaving it alone on deploy. Once the role is taken away (by an + // operator: nothing in the app writes role_user) they stand nowhere, and the sweep retires what they lent. + var world = await SeedWorldAsync(); + var adminId = await SeedInstanceAdminAsync(); + var identity = await LinkTokenAsync(adminId, world.TeamA, world.InstanceA, "admins-token"); + var token = await IdentityCredentialAsync(identity); + var title = $"opened as an instance admin {Guid.NewGuid():N}"; + + var added = await ShouldSucceedAsync(await SendAsync(HttpMethod.Post, "/api/credentials/pat", adminId, world.TeamA, new { providerInstanceId = world.InstanceA, displayName = "admin's token", token = "glpat-admin" }), "the instance Admin adds a token without naming an owner"); + var addedId = JsonDocument.Parse(added).RootElement.GetProperty("id").GetGuid(); + await ShouldSucceedAsync(await SendAsync(HttpMethod.Post, "/api/credentials/oauth/init", adminId, world.TeamA, new { providerInstanceId = world.InstanceA, displayName = "admin's connect" }), "the instance Admin starts an OAuth connect of their own"); + (await PendingConnectsForAsync(adminId)).ShouldBe(1, "the connect waits for its callback in the instance Admin's name"); + + await ShouldSucceedAsync(await BindAsync(world.OwnerId, world.TeamA, world.InstanceA, token, "acme/web"), "the Owner binds through the instance Admin's token"); + await _factory.JobClient.DrainAsync(); + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, $"/api/repositories/{world.ConnectionRepository}/act-as-candidates", world.MemberId, world.TeamA), "picker")).ShouldContain(adminId.ToString(), Case.Insensitive, "the picker offers the instance Admin"); + + var control = await IdentityCredentialAsync(await LinkTokenAsync(world.DepartingId, world.TeamA, world.InstanceA, "departing-token-a")); + await DeleteMembershipAsDeparturesUsedToAsync(world.TeamA, world.DepartingId); + await SweepDepartedUntilRetiredAsync(control); + await DrainDepartureSweepAsync(); + + foreach (var keptId in new[] { token, addedId }) + (await LoadCredentialAsync(keptId)).Status.ShouldBe(CredentialStatus.Active, "the sweep leaves what an instance Admin lent a team alone — they stand in it"); + + (await LoadRepositoryAsync(await RepositoryBoundAsync(token, "acme/web"))).Status.ShouldBe(RepositoryStatus.Active, "the repository bound through the Admin's token keeps working across the sweep"); + + var asAdmin = await RunOpenPullRequestAsAsync(world, adminId, title); + asAdmin.Status.ShouldBe(WorkflowRunStatus.Success, $"a Member's workflow acts as the instance Admin — node said: {asAdmin.NodeError}"); + _factory.PullRequestOpens.All.Select(o => o.Title).ShouldContain(title, "the act-as write reached the provider through the Admin's token"); + + await RevokeInstanceAdminAsync(adminId); + await SweepDepartedUntilRetiredAsync(token); + await SweepDepartedUntilRetiredAsync(addedId); + + (await LoadRepositoryAsync(await RepositoryBoundAsync(token, "acme/web"))).Status.ShouldBe(RepositoryStatus.Error, "once the role is gone, the repository bound through their token says it needs a new credential"); + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, "/api/credentials", world.MemberId, world.TeamA), "list")).ShouldNotContain(token.ToString(), Case.Insensitive, "a Member no longer sees the ex-Admin's token"); + (await ShouldSucceedAsync(await SendAsync(HttpMethod.Get, $"/api/repositories/{world.ConnectionRepository}/act-as-candidates", world.MemberId, world.TeamA), "picker")).ShouldNotContain(adminId.ToString(), Case.Insensitive, "the picker no longer offers them"); + + var afterDemotion = await RunOpenPullRequestAsAsync(world, adminId, $"{title} after the role went"); + ShouldBeRefusedForWantOfAnIdentity(afterDemotion, "a workflow naming the ex-Admin as its author"); + } + + // ── Flow steps ──────────────────────────────────────────────────────────────── + + /// What departures did before they retired anything: the membership row goes, and nothing else does. + private async Task DeleteMembershipAsDeparturesUsedToAsync(Guid teamId, Guid userId) + { + using var scope = _factory.Services.CreateScope(); + + await scope.ServiceProvider.GetRequiredService().TeamMembership.Where(m => m.TeamId == teamId && m.UserId == userId).ExecuteDeleteAsync(); + } + + /// + /// The departure sweep's recurring command until a pass retires nothing, so every candidate it has has been retired — + /// for asserting a token is NOT one. + /// + private async Task DrainDepartureSweepAsync() + { + for (var pass = 1; pass <= MaxDrainPasses; pass++) + { + using var scope = _factory.Services.CreateScope(); + + if (await scope.ServiceProvider.GetRequiredService().Send(new RetireDepartedMembersCredentialsCommand()) == 0) return; + } + + throw new ShouldAssertException($"RetireDepartedMembersCredentialsCommand still retired tokens after {MaxDrainPasses} passes — check that a retired token stops being a candidate."); + } + + /// An account that signs in, holding the instance Admin role through a role_user row, and no membership anywhere. + private async Task SeedInstanceAdminAsync() + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var admin = NewUser("instance-admin", Guid.NewGuid().ToString("N")[..8]); + + db.User.Add(admin); + db.RoleUser.Add(new RoleUser { Id = Guid.NewGuid(), RoleId = await db.Role.Where(r => r.Name == Roles.Admin).Select(r => r.Id).SingleAsync(), UserId = admin.Id }); + await db.SaveChangesAsync(); + + return admin.Id; + } + + /// How the role is taken away: nothing in the app writes role_user, so an operator deletes the row. + private async Task RevokeInstanceAdminAsync(Guid userId) + { + using var scope = _factory.Services.CreateScope(); + + await scope.ServiceProvider.GetRequiredService().RoleUser.Where(r => r.UserId == userId).ExecuteDeleteAsync(); + } + + /// The departure sweep's recurring command through the app's own mediator, until the token is retired. + private async Task SweepDepartedUntilRetiredAsync(Guid credentialId) + { + for (var pass = 1; pass <= MaxSweepPasses; pass++) + { + using (var scope = _factory.Services.CreateScope()) + await scope.ServiceProvider.GetRequiredService().Send(new RetireDepartedMembersCredentialsCommand()); + + if ((await LoadCredentialAsync(credentialId)).DeletedDate != null) return; + } + + throw new ShouldAssertException($"RetireDepartedMembersCredentialsCommand had not retired credential {credentialId} after {MaxSweepPasses} passes — check that its candidate query finds a live Personal token whose owner holds no membership in its team."); + } + + private async Task LinkTokenAsync(Guid userId, Guid teamId, Guid instanceId, string token) + { + var body = await ShouldSucceedAsync(await SendAsync(HttpMethod.Post, "/api/me/identities/pat", userId, teamId, new { providerInstanceId = instanceId, accessToken = token }), "link a token"); + + return JsonDocument.Parse(body).RootElement.GetProperty("id").GetGuid(); + } + + private Task BindAsync(Guid userId, Guid teamId, Guid instanceId, Guid credentialId, string project) => + SendAsync(HttpMethod.Post, "/api/repositories/bind", userId, teamId, new { providerInstanceId = instanceId, credentialId, projectIdentifier = project }); + + /// The Admin starts a connect in 's name: a token added outright, or an OAuth connect begun and left waiting for its callback. + private Task StartConnectForAsync(ConnectPath path, World world, Guid ownerUserId) => path == ConnectPath.Token + ? SendAsync(HttpMethod.Post, "/api/credentials/pat", world.AdminId, world.TeamA, new { providerInstanceId = world.InstanceA, ownerUserId, displayName = "named token", token = "glpat-named" }) + : SendAsync(HttpMethod.Post, "/api/credentials/oauth/init", world.AdminId, world.TeamA, new { providerInstanceId = world.InstanceA, displayName = "named connect", intendedOwnerUserId = ownerUserId }); + + private async Task DepartAsync(World world, Departure departure) + { + var departed = departure == Departure.Removed + ? await SendAsync(HttpMethod.Delete, $"/api/teams/members/{world.DepartingId}", world.OwnerId, world.TeamA) + : await SendAsync(HttpMethod.Post, "/api/teams/members/leave", world.DepartingId, world.TeamA); + + departed.StatusCode.ShouldBe(HttpStatusCode.NoContent, $"{departure}: {await departed.Content.ReadAsStringAsync()}"); + } + + /// A Member authors a one-node workflow whose git.open_pr acts as , runs it, and drains the engine. + private async Task RunOpenPullRequestAsAsync(World world, Guid actAsUserId, string title) + { + var runId = await LaunchOpenPullRequestAsAsync(world, actAsUserId, title); + + await _factory.JobClient.DrainAsync(); + + return await ReadRunAsync(runId); + } + + /// A Member authors and launches the workflow; the run is accepted and queued, and nothing executes until the engine is drained. + private async Task LaunchOpenPullRequestAsAsync(World world, Guid actAsUserId, string title) + { + var definition = new + { + name = $"act-as-{Guid.NewGuid():N}"[..24], + definition = new + { + schemaVersion = 1, + nodes = new object[] + { + new { id = "start", typeKey = "trigger.manual", config = new { }, inputs = new { } }, + new { id = "act", typeKey = "git.open_pr", config = new { }, inputs = new Dictionary { ["repositoryId"] = world.ConnectionRepository.ToString(), ["title"] = title, ["sourceBranch"] = "feature", ["targetBranch"] = "main", ["actAsUserId"] = actAsUserId.ToString() } }, + new { id = "end", typeKey = "builtin.terminal", config = new { }, inputs = new { } }, + }, + edges = new object[] { new { from = "start", to = "act" }, new { from = "act", to = "end" } }, + }, + activations = Array.Empty(), + enabled = true, + }; + + var created = await ShouldSucceedAsync(await SendAsync(HttpMethod.Post, "/api/workflows", world.MemberId, world.TeamA, definition), "a Member authors the workflow"); + var workflowId = JsonDocument.Parse(created).RootElement.GetProperty("id").GetGuid(); + + var launched = await ShouldSucceedAsync(await SendAsync(HttpMethod.Post, $"/api/workflows/{workflowId}/run", world.MemberId, world.TeamA, new { }), "a Member runs it"); + + return JsonDocument.Parse(launched).RootElement.GetProperty("runId").GetGuid(); + } + + private async Task ReadRunAsync(Guid runId) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var status = await db.WorkflowRun.AsNoTracking().Where(r => r.Id == runId).Select(r => r.Status).SingleAsync(); + var nodeError = await db.WorkflowRunNode.AsNoTracking().Where(n => n.RunId == runId && n.NodeId == "act").Select(n => n.Error).SingleOrDefaultAsync(); + + return new RunOutcome(status, nodeError); + } + + private sealed record RunOutcome(WorkflowRunStatus Status, string? NodeError); + + /// Failed, and failed because nobody may be acted as — not for any other reason a run can fail. + private static void ShouldBeRefusedForWantOfAnIdentity(RunOutcome run, string what) + { + run.Status.ShouldBe(WorkflowRunStatus.Failure, $"{what} is refused — node said: {run.NodeError}"); + run.NodeError.ShouldNotBeNull(); + run.NodeError.ShouldContain("haven't linked one", Case.Insensitive, $"{what} fails for want of an identity to act as, not for some other reason"); + } + + private async Task ShouldBeRetiredAsync(Guid credentialId, Guid identityId, Departure departure) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var credential = await db.Credential.AsNoTracking().SingleAsync(c => c.Id == credentialId); + var identity = await db.UserProviderIdentity.AsNoTracking().SingleAsync(i => i.Id == identityId); + var boundRepo = await db.Repository.AsNoTracking().SingleAsync(r => r.CredentialId == credentialId && r.FullPath == "acme/web"); + + credential.Status.ShouldBe(CredentialStatus.Revoked, $"{departure}: the token is revoked"); + credential.EncryptedPayload.ShouldBe(string.Empty, $"{departure}: nothing usable is left at rest"); + credential.DeletedDate.ShouldNotBeNull(); + identity.DeletedDate.ShouldNotBeNull($"{departure}: the act-as identity goes with its token"); + boundRepo.Status.ShouldBe(RepositoryStatus.Error, $"{departure}: the repository bound through the token says it needs a new credential"); + } + + private async Task PendingConnectsForAsync(Guid ownerUserId) + { + using var scope = _factory.Services.CreateScope(); + + return await scope.ServiceProvider.GetRequiredService().OAuthPendingState.AsNoTracking().CountAsync(s => s.IntendedOwnerUserId == ownerUserId); + } + + private async Task LoadRepositoryAsync(Guid repositoryId) + { + using var scope = _factory.Services.CreateScope(); + + return await scope.ServiceProvider.GetRequiredService().Repository.AsNoTracking().SingleAsync(r => r.Id == repositoryId); + } + + private async Task LoadCredentialAsync(Guid credentialId) + { + using var scope = _factory.Services.CreateScope(); + + return await scope.ServiceProvider.GetRequiredService().Credential.AsNoTracking().SingleAsync(c => c.Id == credentialId); + } + + private async Task RepositoryBoundAsync(Guid credentialId, string fullPath) + { + using var scope = _factory.Services.CreateScope(); + + return await scope.ServiceProvider.GetRequiredService().Repository.AsNoTracking().Where(r => r.CredentialId == credentialId && r.FullPath == fullPath).Select(r => r.Id).SingleAsync(); + } + + private async Task IdentityCredentialAsync(Guid identityId) + { + using var scope = _factory.Services.CreateScope(); + + return await scope.ServiceProvider.GetRequiredService().UserProviderIdentity.AsNoTracking().Where(i => i.Id == identityId).Select(i => i.CredentialId).SingleAsync(); + } + + private async Task SendAsync(HttpMethod method, string url, Guid userId, Guid teamId, object? body = null) + { + var request = new HttpRequestMessage(method, url); + if (body != null) request.Content = JsonContent.Create(body); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", TestToken.Mint(userId, TestToken.SeedStamp)); + request.Headers.Add("X-Team-Id", teamId.ToString()); + + return await _factory.CreateClient().SendAsync(request); + } + + private static async Task ShouldSucceedAsync(HttpResponseMessage response, string what) + { + var body = await response.Content.ReadAsStringAsync(); + response.IsSuccessStatusCode.ShouldBeTrue($"{what}: {(int)response.StatusCode} {body}"); + + return body; + } + + // ── World ───────────────────────────────────────────────────────────────────── + + /// + /// Team A: an Owner, an Admin, a Member and the departing person (a Member), one Git instance, and a repository bound + /// through a team-service token. Team B: the departing person's own, with its own Git instance. GUID-suffixed throughout. + /// + private sealed record World(Guid TeamA, Guid TeamB, Guid OwnerId, Guid AdminId, Guid MemberId, Guid DepartingId, Guid InstanceA, Guid InstanceB, Guid ConnectionRepository); + + private async Task SeedWorldAsync() + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var encryptor = scope.ServiceProvider.GetRequiredService(); + var serializer = scope.ServiceProvider.GetRequiredService(); + var suffix = Guid.NewGuid().ToString("N")[..8]; + + var owner = NewUser("owner", suffix); + var admin = NewUser("admin", suffix); + var member = NewUser("member", suffix); + var departing = NewUser("departing", suffix); + var teamA = NewTeam("a", suffix); + var teamB = NewTeam("b", suffix); + var instanceA = NewInstance(teamA, suffix + "a", encryptor); + var instanceB = NewInstance(teamB, suffix + "b", encryptor); + var connection = new Credential { Id = Guid.NewGuid(), TeamId = teamA.Id, ProviderInstanceId = instanceA.Id, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.Pat, DisplayName = "team connection", EncryptedPayload = encryptor.Encrypt(serializer.Serialize(new PatPayload { Token = "team-token" })), Status = CredentialStatus.Active }; + var repo = new Repository { Id = Guid.NewGuid(), TeamId = teamA.Id, ProviderInstanceId = instanceA.Id, CredentialId = connection.Id, ExternalId = $"ext-{suffix}", NamespacePath = "acme", Name = "api", FullPath = "acme/api", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = "https://git.test/acme/api", Status = RepositoryStatus.Active }; + + db.User.AddRange(owner, admin, member, departing); + db.Team.AddRange(teamA, teamB); + db.TeamMembership.AddRange(Membership(teamA, owner, TeamRole.Owner), Membership(teamA, admin, TeamRole.Admin), Membership(teamA, member, TeamRole.Member), Membership(teamA, departing, TeamRole.Member), Membership(teamB, departing, TeamRole.Owner)); + db.ProviderInstance.AddRange(instanceA, instanceB); + db.Credential.Add(connection); + db.Repository.Add(repo); + await db.SaveChangesAsync(); + + return new World(teamA.Id, teamB.Id, owner.Id, admin.Id, member.Id, departing.Id, instanceA.Id, instanceB.Id, repo.Id); + } + + private async Task SeedUserInOwnTeamAsync(string name) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var suffix = Guid.NewGuid().ToString("N")[..8]; + var user = NewUser("elsewhere", suffix); + var team = NewTeam("elsewhere", suffix); + + user.Name = name; + db.User.Add(user); + db.Team.Add(team); + db.TeamMembership.Add(Membership(team, user, TeamRole.Owner)); + await db.SaveChangesAsync(); + + return user.Id; + } + + private static User NewUser(string label, string suffix) => new() { Id = Guid.NewGuid(), Email = $"{label}-{suffix}@departed.test", Name = $"{label}-{suffix}", SecurityStamp = TestToken.SeedStamp, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }; + + private static Team NewTeam(string label, string suffix) => new() { Id = Guid.NewGuid(), Slug = $"{label}-{suffix}", Name = label, Kind = TeamKind.Workspace, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }; + + private static TeamMembership Membership(Team team, User user, TeamRole role) => new() { Id = Guid.NewGuid(), TeamId = team.Id, UserId = user.Id, Role = role, CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }; + + private static ProviderInstance NewInstance(Team team, string suffix, IPayloadEncryptor encryptor) => + new() { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.Git, DisplayName = "Git", BaseUrl = $"https://git-{suffix}.test", OauthClientId = "client", OauthClientSecretEnc = encryptor.Encrypt("secret") }; + + /// + /// The app with one seam: the Git test provider stands in for GitHub/GitLab, recording every PR open, and a loopback + /// OAuth client stands in for its authorize endpoint. + /// + public sealed class GitProviderApiFactory : TaskLaunchApiFactory + { + public TestPullRequestOpenCapture PullRequestOpens => Services.GetAutofacRoot().Resolve(); + + protected override void ConfigureContractTestServices(ContainerBuilder builder) + { + builder.RegisterType().AsSelf().SingleInstance(); + builder.RegisterType().AsSelf().SingleInstance(); + builder.RegisterType().AsImplementedInterfaces().InstancePerLifetimeScope(); + builder.RegisterType().As().SingleInstance(); + } + } + + /// The Git provider's OAuth endpoints on loopback. Only the authorize URL is reached — no test here completes a connect, so the token endpoints refuse. + private sealed class LoopbackOAuthClient : IOAuthClient + { + public ProviderKind Kind => ProviderKind.Git; + + public Uri BuildAuthorizeUrl(OAuthAuthorizeInput input) => new($"http://127.0.0.1/oauth/authorize?state={input.State}"); + + public Task ExchangeCodeAsync(OAuthCodeExchangeInput input, CancellationToken cancellationToken) => throw new NotSupportedException("no test in this suite completes an OAuth connect"); + + public Task RefreshAsync(OAuthRefreshInput input, CancellationToken cancellationToken) => throw new NotSupportedException("no test in this suite refreshes an OAuth token"); + + public Task RevokeAsync(OAuthRevokeInput input, CancellationToken cancellationToken) => Task.CompletedTask; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamStandingTests.cs b/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamStandingTests.cs new file mode 100644 index 000000000..079615d17 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamStandingTests.cs @@ -0,0 +1,125 @@ +using Autofac; +using CodeSpace.Core.Authorization; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Authorization; + +/// +/// Who has standing in a team when the question is asked of someone other than the caller — the departed-token sweep, a +/// departure, a write held in someone's name, and the act-as picker and write all ask it. Everywhere else a live instance +/// Admin stands in every team as an Owner does (), so a membership row is not the +/// whole answer: an Admin who lent a team a token without joining it must not have it retired on deploy. +/// +/// Both forms the seams use are pinned to agree: the scalar ask, and the predicate composed into a query over rows +/// that name the team. +/// +/// The role counts only while it is switched on, as the caller's own roles are read (ApiUser.Roles). Counted +/// regardless, a switched-off Admin role would still keep tokens from the sweep and pass the holds, the OAuth re-check and +/// act-as for someone the caller-side resolver already refuses. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class TeamStandingTests : IAsyncLifetime +{ + private readonly PostgresFixture _fixture; + private bool _adminRoleSwitchedOff; + + public TeamStandingTests(PostgresFixture fixture) { _fixture = fixture; } + + public enum Person { Member, InstanceAdminOutsideTheTeam, InstanceAdminInTheTeam, InstanceAdminOutsideTheTeamWithTheRoleSwitchedOff, ExInstanceAdminOutsideTheTeam, DepartedMember, AnotherTeamsMember, NoAccount } + + public Task InitializeAsync() => Task.CompletedTask; + + /// + /// The Admin role is one row shared by the whole collection (its name is unique), so the case that switches it off + /// switches it back on whatever its assertions did. + /// + public async Task DisposeAsync() + { + if (_adminRoleSwitchedOff) await SwitchAdminRoleAsync(on: true); + } + + [Theory] + [InlineData(Person.Member, true)] + [InlineData(Person.InstanceAdminOutsideTheTeam, true)] + [InlineData(Person.InstanceAdminInTheTeam, true)] + [InlineData(Person.InstanceAdminOutsideTheTeamWithTheRoleSwitchedOff, false)] + [InlineData(Person.ExInstanceAdminOutsideTheTeam, false)] + [InlineData(Person.DepartedMember, false)] + [InlineData(Person.AnotherTeamsMember, false)] + [InlineData(Person.NoAccount, false)] + public async Task Standing_is_a_membership_row_or_the_instance_admin_role(Person person, bool stands) + { + var (teamId, userId) = await SeedAsync(person); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + var asked = await TeamStanding.HeldAsync(db, teamId, userId, CancellationToken.None); + var composed = await db.Team.AsNoTracking().Where(t => t.Id == teamId).AnyAsync(TeamStanding.Of(db, t => t.Id, _ => userId)); + + asked.ShouldBe(stands, $"{person}: {(stands ? "has" : "has no")} standing in the team"); + composed.ShouldBe(asked, $"{person}: the predicate a query composes answers exactly as the scalar ask does"); + } + + // ── World ───────────────────────────────────────────────────────────────────── + + private async Task<(Guid TeamId, Guid UserId)> SeedAsync(Person person) + { + var suffix = Guid.NewGuid().ToString("N")[..8]; + var team = new Team { Id = Guid.NewGuid(), Slug = $"standing-{suffix}", Name = "Standing", Kind = TeamKind.Workspace }; + var otherTeam = new Team { Id = Guid.NewGuid(), Slug = $"standing-other-{suffix}", Name = "Elsewhere", Kind = TeamKind.Workspace }; + var user = new User { Id = Guid.NewGuid(), Email = $"standing-{suffix}@standing.test", Name = $"standing-{suffix}" }; + + if (person == Person.NoAccount) return (await SeedTeamAsync(team), user.Id); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.Team.AddRange(team, otherTeam); + db.User.Add(user); + + if (person is Person.Member or Person.InstanceAdminInTheTeam or Person.DepartedMember) db.TeamMembership.Add(Membership(team, user)); + if (person == Person.AnotherTeamsMember) db.TeamMembership.Add(Membership(otherTeam, user)); + if (person is Person.InstanceAdminOutsideTheTeam or Person.InstanceAdminInTheTeam or Person.InstanceAdminOutsideTheTeamWithTheRoleSwitchedOff or Person.ExInstanceAdminOutsideTheTeam) db.RoleUser.Add(new RoleUser { Id = Guid.NewGuid(), RoleId = await AdminRoleIdAsync(db), UserId = user.Id }); + + await db.SaveChangesAsync(); + + if (person == Person.DepartedMember) await db.TeamMembership.Where(m => m.TeamId == team.Id && m.UserId == user.Id).ExecuteDeleteAsync(); + if (person == Person.ExInstanceAdminOutsideTheTeam) await db.RoleUser.Where(r => r.UserId == user.Id).ExecuteDeleteAsync(); + if (person == Person.InstanceAdminOutsideTheTeamWithTheRoleSwitchedOff) await SwitchAdminRoleAsync(on: false); + + return (team.Id, user.Id); + } + + private async Task SwitchAdminRoleAsync(bool on) + { + _adminRoleSwitchedOff = !on; + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + await db.Role.Where(r => r.Name == Roles.Admin).ExecuteUpdateAsync(r => r.SetProperty(x => x.Status, on)); + } + + private async Task SeedTeamAsync(Team team) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.Team.Add(team); + await db.SaveChangesAsync(); + + return team.Id; + } + + private static async Task AdminRoleIdAsync(CodeSpaceDbContext db) => await db.Role.Where(r => r.Name == Roles.Admin).Select(r => r.Id).SingleAsync(); + + private static TeamMembership Membership(Team team, User user) => new() { Id = Guid.NewGuid(), TeamId = team.Id, UserId = user.Id, Role = TeamRole.Member }; +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Binding/RepositoryBindingFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Binding/RepositoryBindingFlowTests.cs index cadb29c0d..80787463d 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Binding/RepositoryBindingFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Binding/RepositoryBindingFlowTests.cs @@ -317,9 +317,13 @@ public async Task AddCredentialCommand_encrypts_payload_and_persists() { var teamId = await SeedTeamAsync().ConfigureAwait(false); var providerInstanceId = await SeedProviderInstanceAsync(teamId).ConfigureAwait(false); + var adminId = await SeedInstanceAdminAsync().ConfigureAwait(false); + // An instance Admin from outside the team: they stand in every team as an Owner does, so the token is theirs to + // lend. A real account holding the role, because the caller becomes the token's owner, and both the owner column's + // foreign key and the standing check read that account from the database. Guid credentialId; - using (var scope = _fixture.BeginScopeAs(Guid.NewGuid(), teamId, Roles.Admin)) + using (var scope = _fixture.BeginScopeAs(adminId, teamId, Roles.Admin)) { var mediator = scope.Resolve(); credentialId = await mediator.Send(new AddCredentialCommand @@ -338,6 +342,7 @@ public async Task AddCredentialCommand_encrypts_payload_and_persists() var credential = await db.Credential.AsNoTracking().SingleAsync(c => c.Id == credentialId).ConfigureAwait(false); credential.AuthType.ShouldBe(AuthType.Pat); credential.Ownership.ShouldBe(CredentialOwnership.Personal, customMessage: "An ordinary add (no Ownership set) must default to Personal — adding the field is non-breaking."); + credential.OwnerUserId.ShouldBe(adminId, customMessage: "a personal token added without naming an owner is the caller's own"); credential.EncryptedPayload.ShouldNotContain("secret-token-xxx", customMessage: "payload must not be stored in plaintext"); var decryptedJson = encryptor.Decrypt(credential.EncryptedPayload); @@ -406,6 +411,20 @@ private async Task SeedTeamAsync() return team.Id; } + /// An account holding the instance Admin role through a role_user row, and no membership anywhere. + private async Task SeedInstanceAdminAsync() + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var admin = new User { Id = Guid.NewGuid(), Email = $"instance-admin-{Guid.NewGuid():N}@binding.test", Name = $"instance-admin-{Guid.NewGuid():N}" }; + + db.User.Add(admin); + db.RoleUser.Add(new RoleUser { Id = Guid.NewGuid(), RoleId = await db.Role.Where(r => r.Name == Roles.Admin).Select(r => r.Id).SingleAsync().ConfigureAwait(false), UserId = admin.Id }); + await db.SaveChangesAsync().ConfigureAwait(false); + + return admin.Id; + } + private async Task SeedProviderInstanceAsync(Guid teamId) { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.IntegrationTests/Binding/TestRepositoryProvider.cs b/backend/tests/CodeSpace.IntegrationTests/Binding/TestRepositoryProvider.cs index 3c626c2b1..1e9dbccec 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Binding/TestRepositoryProvider.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Binding/TestRepositoryProvider.cs @@ -122,6 +122,9 @@ public sealed class TestPullRequestOpenCapture public OpenPullRequestInput? Last { get { lock (_lock) { return _calls.LastOrDefault(); } } } + /// Every call so far, oldest first — for asserting a write NEVER reached the provider, which the latest call alone cannot show once another test has opened one. + public IReadOnlyList All { get { lock (_lock) { return _calls.ToList(); } } } + public void Record(OpenPullRequestInput input) { lock (_lock) { _calls.Add(input); } diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialOwnerReachTests.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialOwnerReachTests.cs new file mode 100644 index 000000000..272aea122 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialOwnerReachTests.cs @@ -0,0 +1,281 @@ +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.Core.Services.Providers.Identity; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.OAuth; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Commands.Identity; +using CodeSpace.Messages.Commands.Teams; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Credentials; + +/// +/// Whose name a credential may be connected under. Both connect paths take the owner from the request — a token's +/// ownerUserId, an OAuth connect's intendedOwnerUserId — and the owner is who the team then acts AS: the +/// token is listed as theirs, an OAuth connect links their act-as identity, and a disconnected credential's bindings +/// follow its owner's next one. So naming an owner is acting on that person, under the rule every other act on a +/// member follows: they must be in this team, and below you unless they are you. +/// +/// Before this, an Admin could connect a token "owned" by the Owner and re-point the Owner's act-as identity at +/// it, plant identities on users of other tenants (who then saw them in their own list), and tell a real user id from +/// an invented one by a 200 against a foreign-key 500. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class CredentialOwnerReachTests +{ + private readonly PostgresFixture _fixture; + + public CredentialOwnerReachTests(PostgresFixture fixture) { _fixture = fixture; } + + public enum ConnectPath { Token, OAuth } + + public enum NamedOwner { AnotherTeamsUser, NoSuchUser, DepartedMember, TheOwner, AnotherAdmin, InstanceAdminOutsideTheTeam } + + [Theory] + [InlineData(ConnectPath.Token, NamedOwner.AnotherTeamsUser)] + [InlineData(ConnectPath.Token, NamedOwner.NoSuchUser)] + [InlineData(ConnectPath.Token, NamedOwner.DepartedMember)] + [InlineData(ConnectPath.Token, NamedOwner.TheOwner)] + [InlineData(ConnectPath.Token, NamedOwner.AnotherAdmin)] + [InlineData(ConnectPath.Token, NamedOwner.InstanceAdminOutsideTheTeam)] + [InlineData(ConnectPath.OAuth, NamedOwner.AnotherTeamsUser)] + [InlineData(ConnectPath.OAuth, NamedOwner.NoSuchUser)] + [InlineData(ConnectPath.OAuth, NamedOwner.DepartedMember)] + [InlineData(ConnectPath.OAuth, NamedOwner.TheOwner)] + [InlineData(ConnectPath.OAuth, NamedOwner.AnotherAdmin)] + [InlineData(ConnectPath.OAuth, NamedOwner.InstanceAdminOutsideTheTeam)] + public async Task An_admin_cannot_connect_a_credential_in_the_name_of_someone_outside_their_reach(ConnectPath path, NamedOwner named) + { + var world = await SeedAsync(); + var ownerId = await NameAsync(world, named); + + var connect = () => StartConnectAsync(world, world.AdminId, path, ownerId); + + // An instance Admin stands in the team as an Owner does, row or not — so above a team Admin, not outside the team. + if (named is NamedOwner.TheOwner or NamedOwner.AnotherAdmin or NamedOwner.InstanceAdminOutsideTheTeam) + await connect.ShouldThrowAsync($"{path}: an Admin does not act on a {named}"); + else + await connect.ShouldThrowAsync($"{path}: a {named} is not in this team — and an id that exists elsewhere reads exactly like one that exists nowhere"); + + (await CredentialsOwnedByAsync(world.TeamId, ownerId)).ShouldBe(0, $"{path}: a refused owner leaves no credential behind"); + (await PendingConnectsForAsync(world.TeamId, ownerId)).ShouldBe(0, $"{path}: a refused owner leaves no connect waiting for its callback"); + } + + [Theory] + [InlineData(ConnectPath.Token, false)] + [InlineData(ConnectPath.Token, true)] + [InlineData(ConnectPath.OAuth, false)] + [InlineData(ConnectPath.OAuth, true)] + public async Task An_admin_still_connects_for_themselves_or_a_member_below_them(ConnectPath path, bool self) + { + var world = await SeedAsync(); + var ownerId = self ? world.AdminId : world.MemberId; + + await StartConnectAsync(world, world.AdminId, path, ownerId); + + var written = path == ConnectPath.Token ? await CredentialsOwnedByAsync(world.TeamId, ownerId) : await PendingConnectsForAsync(world.TeamId, ownerId); + written.ShouldBe(1, $"{path}: naming {(self ? "yourself" : "a Member")} is within an Admin's reach"); + } + + [Theory] + [InlineData(ConnectPath.Token, false)] + [InlineData(ConnectPath.Token, true)] + [InlineData(ConnectPath.OAuth, false)] + [InlineData(ConnectPath.OAuth, true)] + public async Task An_instance_admin_outside_the_team_connects_for_themselves(ConnectPath path, bool namesThemselves) + { + // A live instance Admin has the standing of an Owner in every team, membership row or not, as everywhere else in + // CodeSpace — so the token they lend a team they never joined is theirs to lend, named or defaulted. + var world = await SeedAsync(); + var instanceAdminId = await SeedInstanceAdminAsync(); + + await StartConnectAsync(world, instanceAdminId, path, namesThemselves ? instanceAdminId : null, Roles.Admin); + + var written = path == ConnectPath.Token ? await CredentialsOwnedByAsync(world.TeamId, instanceAdminId) : await PendingConnectsForAsync(world.TeamId, instanceAdminId); + written.ShouldBe(1, $"{path}: an instance Admin outside the team connects their own account"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task A_connect_someone_else_started_never_moves_the_owners_act_as_identity(bool ownerAlreadyLinked) + { + // The token an Admin connects "for" a member is whatever account the ADMIN signed into at the provider. Linking + // it as the member's act-as identity would let the Admin choose who the member's writes are attributed to — so + // only the person themselves, connecting, links or re-points their identity. + var world = await SeedAsync(); + var linkedCredential = ownerAlreadyLinked ? await LinkOwnTokenAsync(world, world.MemberId) : (Guid?)null; + + var connected = await ConnectOAuthAsync(world, world.AdminId, world.MemberId); + + (await CredentialsOwnedByAsync(world.TeamId, world.MemberId)).ShouldBe(ownerAlreadyLinked ? 2 : 1, "fixture check: the Admin's connect wrote a credential in the member's name"); + + var identity = await ResolveAsync(world.MemberId, world.InstanceId); + + if (!ownerAlreadyLinked) + { + identity.ShouldBeNull("nobody but the member creates the member's act-as identity"); + return; + } + + identity.ShouldNotBeNull(); + identity!.CredentialId.ShouldBe(linkedCredential!.Value, "the member's identity still acts through the token THEY linked, not the one the Admin connected"); + identity.CredentialId.ShouldNotBe(connected); + } + + // ── Flow steps ──────────────────────────────────────────────────────────────── + + /// The first half of each connect path: a token added outright, or an OAuth connect started (Init) and waiting for its callback. + private async Task StartConnectAsync(World world, Guid actorId, ConnectPath path, Guid? ownerId, params string[] roles) + { + using var scope = OAuthScope(actorId, world.TeamId, roles); + var mediator = scope.Resolve(); + + if (path == ConnectPath.Token) + { + await mediator.Send(new AddCredentialCommand { ProviderInstanceId = world.InstanceId, OwnerUserId = ownerId, DisplayName = "named token", Payload = new PatPayload { Token = "glpat-named" } }); + return; + } + + await mediator.Send(new InitCredentialOAuthCommand { ProviderInstanceId = world.InstanceId, DisplayName = "named connect", IntendedOwnerUserId = ownerId }); + } + + private async Task ConnectOAuthAsync(World world, Guid actorId, Guid ownerId) + { + InitCredentialOAuthResult init; + + using (var scope = OAuthScope(actorId, world.TeamId)) + init = await scope.Resolve().Send(new InitCredentialOAuthCommand { ProviderInstanceId = world.InstanceId, DisplayName = "connected for someone", IntendedOwnerUserId = ownerId }); + + using var callback = OAuthScope(userId: null, teamId: null); + + return (await callback.Resolve().Send(new CompleteCredentialOAuthCommand { State = init.State, Code = "code", BrowserNonce = init.BrowserNonce })).CredentialId; + } + + private async Task LinkOwnTokenAsync(World world, Guid userId) + { + using (var scope = _fixture.BeginScopeAs(userId, world.TeamId)) + await scope.Resolve().Send(new LinkProviderIdentityByPatCommand { ProviderInstanceId = world.InstanceId, AccessToken = "members-own-token" }); + + return (await ResolveAsync(userId, world.InstanceId))!.CredentialId; + } + + private ILifetimeScope OAuthScope(Guid? userId, Guid? teamId, params string[] roles) => + _fixture.BeginScope(b => + { + b.RegisterInstance(new TestCurrentUser(userId, "test", roles)).As().SingleInstance(); + b.RegisterInstance(new TestCurrentTeam(teamId)).As().SingleInstance(); + b.RegisterInstance(new OneClientRegistry(new StubOAuthClient(ProviderKind.Git, new OAuthTokenResponse { AccessToken = "admins-account-token", RefreshToken = "rt" }))).SingleInstance(); + }); + + private sealed class OneClientRegistry : IOAuthClientRegistry + { + private readonly IOAuthClient _client; + + public OneClientRegistry(IOAuthClient client) { _client = client; } + + public IOAuthClient Get(ProviderKind kind) => _client; + } + + // ── Reads ───────────────────────────────────────────────────────────────────── + + private async Task ResolveAsync(Guid userId, Guid instanceId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().ResolveAsync(userId, instanceId, CancellationToken.None); + } + + private async Task CredentialsOwnedByAsync(Guid teamId, Guid ownerId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Credential.AsNoTracking().CountAsync(c => c.TeamId == teamId && c.OwnerUserId == ownerId); + } + + private async Task PendingConnectsForAsync(Guid teamId, Guid ownerId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().OAuthPendingState.AsNoTracking().CountAsync(s => s.TeamId == teamId && s.IntendedOwnerUserId == ownerId); + } + + // ── World ───────────────────────────────────────────────────────────────────── + + /// A team with an Owner, two Admins and a Member on one OAuth-capable Git instance, beside another team with a user of its own. + private sealed record World(Guid TeamId, Guid InstanceId, Guid OwnerId, Guid AdminId, Guid PeerAdminId, Guid MemberId, Guid OtherTeamsUserId); + + private async Task SeedAsync() + { + var suffix = Guid.NewGuid().ToString("N")[..8]; + var team = new Team { Id = Guid.NewGuid(), Slug = $"reach-{suffix}", Name = "Reach", Kind = TeamKind.Workspace }; + var otherTeam = new Team { Id = Guid.NewGuid(), Slug = $"other-{suffix}", Name = "Other", Kind = TeamKind.Workspace }; + var owner = NewUser("owner", suffix); + var admin = NewUser("admin", suffix); + var peerAdmin = NewUser("peer-admin", suffix); + var member = NewUser("member", suffix); + var otherTeamsUser = NewUser("elsewhere", suffix); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.Git, DisplayName = "Git", BaseUrl = $"https://git-{suffix}.test", OauthClientId = "client", OauthClientSecretEnc = scope.Resolve().Encrypt("secret") }; + + db.Team.AddRange(team, otherTeam); + db.User.AddRange(owner, admin, peerAdmin, member, otherTeamsUser); + db.TeamMembership.AddRange(Membership(team, owner, TeamRole.Owner), Membership(team, admin, TeamRole.Admin), Membership(team, peerAdmin, TeamRole.Admin), Membership(team, member, TeamRole.Member), Membership(otherTeam, otherTeamsUser, TeamRole.Owner)); + db.ProviderInstance.Add(instance); + await db.SaveChangesAsync(); + + return new World(team.Id, instance.Id, owner.Id, admin.Id, peerAdmin.Id, member.Id, otherTeamsUser.Id); + } + + private async Task NameAsync(World world, NamedOwner named) => named switch + { + NamedOwner.AnotherTeamsUser => world.OtherTeamsUserId, + NamedOwner.NoSuchUser => Guid.NewGuid(), + NamedOwner.DepartedMember => await RemoveTheMemberAsync(world), + NamedOwner.TheOwner => world.OwnerId, + NamedOwner.AnotherAdmin => world.PeerAdminId, + NamedOwner.InstanceAdminOutsideTheTeam => await SeedInstanceAdminAsync(), + _ => throw new ArgumentOutOfRangeException(nameof(named), named, "every named owner needs a seat in this world") + }; + + private async Task RemoveTheMemberAsync(World world) + { + using var scope = _fixture.BeginScopeAs(world.OwnerId, world.TeamId); + + await scope.Resolve().Send(new RemoveTeamMemberCommand { UserId = world.MemberId }); + + return world.MemberId; + } + + /// An account holding the instance Admin role through a role_user row, and no membership anywhere. + private async Task SeedInstanceAdminAsync() + { + var admin = NewUser("instance-admin", Guid.NewGuid().ToString("N")[..8]); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.User.Add(admin); + db.RoleUser.Add(new RoleUser { Id = Guid.NewGuid(), RoleId = await db.Role.Where(r => r.Name == Roles.Admin).Select(r => r.Id).SingleAsync(), UserId = admin.Id }); + await db.SaveChangesAsync(); + + return admin.Id; + } + + private static User NewUser(string label, string suffix) => new() { Id = Guid.NewGuid(), Email = $"{label}-{suffix}@reach.test", Name = $"{label}-{suffix}" }; + + private static TeamMembership Membership(Team team, User user, TeamRole role) => new() { Id = Guid.NewGuid(), TeamId = team.Id, UserId = user.Id, Role = role }; +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/DepartedMemberCredentialTests.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/DepartedMemberCredentialTests.cs new file mode 100644 index 000000000..5b0230485 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/DepartedMemberCredentialTests.cs @@ -0,0 +1,930 @@ +using Autofac; +using CodeSpace.Core.Authorization; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Identity; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.Core.Services.Providers; +using CodeSpace.Core.Services.Providers.Capabilities; +using CodeSpace.Core.Services.Providers.Identity; +using CodeSpace.Core.Services.PullRequests; +using CodeSpace.Core.Services.Teams; +using CodeSpace.IntegrationTests.Binding; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.OAuth; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Commands.Identity; +using CodeSpace.Messages.Commands.Repositories; +using CodeSpace.Messages.Commands.Teams; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using CodeSpace.Messages.Queries.Credentials; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Credentials; + +/// +/// What a person's own provider tokens can still do for a team after they stop belonging to it. +/// +/// A Personal credential is one person's GitHub/GitLab account lent to the team while they are in it — the +/// contract on says it vanishes when they leave. Before this, leaving or being +/// removed deleted only the membership row: the token stayed Active, a Member could still list it and enumerate the +/// repositories it reaches, an Admin could bind more of them through it, and any workflow could act AS the departed +/// person by naming them in actAsUserId. Each test departs a member through the real mediator pipeline and +/// then asks one of those doors. +/// +/// Three ways a token could still outlive its person are pinned here too: a connect or token link in flight while +/// they leave (either order — the departure commits during the provider round-trip, or starts once the write holds their +/// membership), a token added without naming an owner, and every departure from before departures retired anything, +/// which the departure sweep retires. +/// +/// Real Postgres, the real pipeline and services; only the provider is the Git test provider, which answers +/// whoami and echoes PR opens into , so "never reached the provider" is +/// observed, not inferred. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class DepartedMemberCredentialTests +{ + private const int MaxSweepPasses = 20; + + /// The departure sweep is deployment-wide and this suite shares one database: every earlier test's leftover token of someone not in its team is a candidate ahead of ours, fifty a pass. + private const int MaxDepartureSweepPasses = 200; + + private static readonly TimeSpan LockWaitTimeout = TimeSpan.FromSeconds(15); + + private readonly PostgresFixture _fixture; + + public DepartedMemberCredentialTests(PostgresFixture fixture) { _fixture = fixture; } + + public enum Departure { Removed, Left } + + [Theory] + [InlineData(Departure.Removed)] + [InlineData(Departure.Left)] + public async Task Departing_retires_every_token_the_member_owns_in_that_team_and_nothing_else(Departure departure) + { + var world = await SeedWorldAsync(); + + (await ResolveAsync(world.DepartingId, world.InstanceA)).ShouldNotBeNull("fixture check: before departing, the member's own identity acts on the team's instance"); + + await DepartAsync(world, departure); + + var token = await LoadCredentialAsync(world.PersonalCredentialA); + token.Status.ShouldBe(CredentialStatus.Revoked, $"{departure}: the departed member's token is no longer the team's to use"); + token.EncryptedPayload.ShouldBe(string.Empty, $"{departure}: nothing usable is left at rest"); + token.DeletedDate.ShouldNotBeNull($"{departure}: the token leaves the team's credential list"); + + (await LoadIdentityAsync(world.IdentityA)).DeletedDate.ShouldNotBeNull($"{departure}: the act-as identity on the team's instance goes with its token"); + (await ResolveAsync(world.DepartingId, world.InstanceA)).ShouldBeNull($"{departure}: nobody can act as the departed member on this team's instance"); + + var personalRepo = await LoadRepositoryAsync(world.PersonalRepository); + personalRepo.Status.ShouldBe(RepositoryStatus.Error, $"{departure}: a repository bound through the departed member's token says it needs a new credential rather than keep a dead one"); + personalRepo.LastError.ShouldNotBeNullOrWhiteSpace(); + (await LoadRepositoryAsync(world.ConnectionRepository)).Status.ShouldBe(RepositoryStatus.Active, $"{departure}: the team's own connection was never the member's"); + + var otherTeamToken = await LoadCredentialAsync(world.PersonalCredentialB); + otherTeamToken.Status.ShouldBe(CredentialStatus.Active, $"{departure}: the token they lent their OTHER team is theirs, not this team's"); + otherTeamToken.EncryptedPayload.ShouldNotBeEmpty(); + otherTeamToken.DeletedDate.ShouldBeNull(); + (await ResolveAsync(world.DepartingId, world.InstanceB)).ShouldNotBeNull($"{departure}: they still act as themselves in the team they are still in"); + } + + [Fact] + public async Task An_act_as_write_through_a_departed_member_never_reaches_the_provider() + { + var world = await SeedWorldAsync(); + var title = $"opened as a departed member {Guid.NewGuid():N}"; + + await DepartAsync(world, Departure.Removed); + + var failure = await Should.ThrowAsync(() => OpenPullRequestAsAsync(world, world.DepartingId, title)); + + failure.ProviderInstanceId.ShouldBe(world.InstanceA); + OpenedPullRequestTitles().ShouldNotContain(title, "the write was refused before any provider call — the departed member's token was never presented"); + } + + [Fact] + public async Task The_team_can_no_longer_list_browse_or_bind_through_a_departed_members_token() + { + var world = await SeedWorldAsync(); + + (await ListCredentialIdsAsync(world.MemberId, world.TeamA)).ShouldContain(world.PersonalCredentialA, "fixture check: while they are in the team, their token is one of its credentials"); + + await DepartAsync(world, Departure.Removed); + + (await ListCredentialIdsAsync(world.MemberId, world.TeamA)).ShouldNotContain(world.PersonalCredentialA, "a Member no longer sees the departed member's token"); + + await Should.ThrowAsync(() => SendAsync(world.MemberId, world.TeamA, new ListAccessibleRepositoriesQuery { CredentialId = world.PersonalCredentialA })); + + await Should.ThrowAsync(() => SendAsync(world.OwnerId, world.TeamA, new BindRepositoryCommand { ProviderInstanceId = world.InstanceA, CredentialId = world.PersonalCredentialA, ProjectIdentifier = "acme/web" })); + (await RepositoriesBoundThroughAsync(world.PersonalCredentialA)).ShouldBe(1, "only the repository bound before the departure still names the token — the refused bind wrote nothing"); + } + + [Theory] + [InlineData(false, true)] + [InlineData(true, false)] + public async Task The_repair_sweep_never_moves_a_departed_members_bindings(bool departed, bool moves) + { + // The shape the sweep exists to heal: a repository left on a token the member disconnected, and a token they + // linked after it. The stay-in-the-team row is the control proving the sweep would move it. Departing retires the + // newer token too, so the member leaves nothing a binding could follow and the repository keeps naming the token + // that died. + var world = await SeedWorldAsync(); + var dead = await SeedDisconnectedTokenAsync(world); + var repoId = await SeedRepositoryAsync(world.TeamA, world.InstanceA, dead, "acme/stranded", RepositoryStatus.Error); + + if (departed) await DepartAsync(world, Departure.Removed); + + await SweepUntilDrainedAsync(); + + var repo = await LoadRepositoryAsync(repoId); + + if (!moves) + { + repo.CredentialId.ShouldBe(dead, "the departed member's bindings follow nothing — not even the token they linked before leaving"); + repo.Status.ShouldBe(RepositoryStatus.Error); + return; + } + + repo.CredentialId.ShouldBe(world.PersonalCredentialA, "fixture check: for a member still in the team, the sweep moves the stranded binding to the token they linked after the disconnect"); + repo.Status.ShouldBe(RepositoryStatus.Active); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task An_oauth_connect_whose_owner_left_before_the_callback_is_refused(bool startedByThemselves) + { + // The provider's callback can land up to the state's lifetime after Init. A member who is removed while a connect + // in their name is in flight — one they started as an Admin, or one the Owner started for them — must not come + // back as a fresh token the team can bind through, or that the repair sweep could hand their old bindings to. + var world = await SeedWorldAsync(); + var oauth = new StubOAuthClient(ProviderKind.Git, new OAuthTokenResponse { AccessToken = "at-after-leaving", RefreshToken = "rt" }); + + if (startedByThemselves) await SendAsync(world.OwnerId, world.TeamA, new ChangeTeamMemberRoleCommand { UserId = world.DepartingId, Role = TeamRole.Admin }); + + InitCredentialOAuthResult init; + using (var scope = OAuthScope(oauth, startedByThemselves ? world.DepartingId : world.OwnerId, world.TeamA)) + init = await scope.Resolve().Send(new InitCredentialOAuthCommand { ProviderInstanceId = world.InstanceA, DisplayName = "their GitLab", IntendedOwnerUserId = world.DepartingId }); + + await DepartAsync(world, Departure.Removed); + + using (var callback = OAuthScope(oauth, userId: null, teamId: null)) + await Should.ThrowAsync(() => callback.Resolve().Send(new CompleteCredentialOAuthCommand { State = init.State, Code = "code", BrowserNonce = init.BrowserNonce })); + + (await LiveCredentialsOwnedInAsync(world.TeamA, world.DepartingId)).ShouldBe(0, "no token of the departed member's is live in the team they left"); + oauth.LastExchange.ShouldBeNull("the code is never exchanged, so no token is minted for a connect that will not be kept"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task A_departure_that_commits_during_the_oauth_code_exchange_leaves_the_departed_member_no_token(bool startedByThemselves) + { + // The other side of the callback's window: the owner is still a member when the callback starts, and their removal + // commits while the provider is minting the token. Persisted, that token would be a live credential of someone + // outside the team, and the reconnect carry-forward would move the bindings the departure just broke onto it. + var world = await SeedWorldAsync(); + var oauth = new DepartingDuringExchangeOAuthClient(() => DepartAsync(world, Departure.Removed)); + + if (startedByThemselves) await SendAsync(world.OwnerId, world.TeamA, new ChangeTeamMemberRoleCommand { UserId = world.DepartingId, Role = TeamRole.Admin }); + + var init = await InitConnectAsync(oauth, startedByThemselves ? world.DepartingId : world.OwnerId, world); + + using (var callback = OAuthScope(oauth, userId: null, teamId: null)) + await Should.ThrowAsync(() => callback.Resolve().Send(new CompleteCredentialOAuthCommand { State = init.State, Code = "code", BrowserNonce = init.BrowserNonce })); + + oauth.Exchanged.ShouldBeTrue("fixture check: the departure ran inside the provider's code exchange, after the callback's first membership check"); + (await IsMemberAsync(world.TeamA, world.DepartingId)).ShouldBeFalse("fixture check: the departure committed"); + (await LiveCredentialsOwnedInAsync(world.TeamA, world.DepartingId)).ShouldBe(0, "the token minted while they were leaving is never kept"); + + var personalRepo = await LoadRepositoryAsync(world.PersonalRepository); + personalRepo.CredentialId.ShouldBe(world.PersonalCredentialA, "the repository the departure broke still names the token that died — nothing carried it forward"); + personalRepo.Status.ShouldBe(RepositoryStatus.Error); + } + + [Fact] + public async Task A_departure_that_commits_while_their_new_token_is_checked_leaves_the_departed_member_no_token() + { + // Linking a token checks it against the provider between the request's membership check and the insert. A + // departure that commits in that gap must not come back as a live token, nor have its broken bindings carried + // forward onto one. + var world = await SeedWorldAsync(); + var probe = new DepartingDuringProbe { DuringProbe = () => DepartAsync(world, Departure.Removed) }; + + using (var scope = LinkScope(probe, world.DepartingId, world.TeamA)) + await Should.ThrowAsync(() => scope.Resolve().Send(new LinkProviderIdentityByPatCommand { ProviderInstanceId = world.InstanceA, AccessToken = "linked-while-leaving" })); + + probe.Probed.ShouldBeTrue("fixture check: the departure ran inside the provider check, after the request's membership check"); + (await IsMemberAsync(world.TeamA, world.DepartingId)).ShouldBeFalse("fixture check: the departure committed"); + (await LiveCredentialsOwnedInAsync(world.TeamA, world.DepartingId)).ShouldBe(0, "the token they were linking as they left is never kept"); + + var personalRepo = await LoadRepositoryAsync(world.PersonalRepository); + personalRepo.CredentialId.ShouldBe(world.PersonalCredentialA, "the repository the departure broke still names the token that died — nothing carried it forward"); + personalRepo.Status.ShouldBe(RepositoryStatus.Error); + } + + public enum Holder { ConnectCallback, CallerNamingThem } + + [Theory] + [InlineData(Holder.ConnectCallback, TeamRole.Member)] + [InlineData(Holder.CallerNamingThem, TeamRole.Member)] + [InlineData(Holder.ConnectCallback, TeamRole.Owner)] + public async Task A_departure_waits_for_a_write_that_holds_the_membership_then_retires_what_it_wrote(Holder holder, TeamRole departingRole) + { + // The ordering the refusals above cannot cover: the write in their name has already confirmed they are a member + // when the departure starts, and commits a token for them while the departure waits — as a connect inserts its + // token at its final save. The departure then retires that token with every other, rather than read the token + // list before it exists. An Owner's departure locks the team row before the membership row; the write holds both, + // team row first, so neither ever waits on a row the other holds — holding the membership row alone, the write's + // insert waits on the team row the departure holds while the departure waits on it, and this case deadlocks. + var world = await SeedWorldAsync(); + var departureKind = departingRole == TeamRole.Owner ? Departure.Left : Departure.Removed; + Task departure; + Guid written; + + if (departingRole == TeamRole.Owner) await SendAsync(world.OwnerId, world.TeamA, new ChangeTeamMemberRoleCommand { UserId = world.DepartingId, Role = TeamRole.Owner }); + + using (var writer = holder == Holder.ConnectCallback ? _fixture.BeginScopeAs(null, null) : _fixture.BeginScopeAs(world.OwnerId, world.TeamA)) + { + var db = writer.Resolve(); + await using var transaction = await db.Database.BeginTransactionAsync(); + + await HoldMembershipAsync(writer, holder, world); + + departure = DepartAsync(world, departureKind); + await WaitUntilDepartureWaitsOnALockAsync(departure, holder); + + written = await WriteTokenInTheirNameAsync(db, world); + await transaction.CommitAsync(); + } + + await departure.WaitAsync(LockWaitTimeout); + + var token = await LoadCredentialAsync(written); + token.Status.ShouldBe(CredentialStatus.Revoked, $"{holder}/{departingRole}: the token written while the departure waited is retired with the rest"); + token.EncryptedPayload.ShouldBe(string.Empty); + token.DeletedDate.ShouldNotBeNull(); + (await IsMemberAsync(world.TeamA, world.DepartingId)).ShouldBeFalse($"{holder}/{departingRole}: the departure went through once the write committed"); + (await LiveCredentialsOwnedInAsync(world.TeamA, world.DepartingId)).ShouldBe(0, $"{holder}/{departingRole}: nothing of theirs is live in the team they left"); + } + + public enum Lent { AddedWithoutNamingAnOwner, WrittenOwnerlessBeforeOwnersDefaulted } + + [Theory] + [InlineData(Lent.AddedWithoutNamingAnOwner)] + [InlineData(Lent.WrittenOwnerlessBeforeOwnersDefaulted)] + public async Task A_token_added_without_naming_an_owner_goes_when_whoever_added_it_leaves(Lent lent) + { + // A personal token added without an owner used to be stored owner-less, so nothing keyed on its owner — the + // departure included — ever found it: the person who added it left and the team kept using their token. + var world = await SeedWorldAsync(); + + await SendAsync(world.OwnerId, world.TeamA, new ChangeTeamMemberRoleCommand { UserId = world.DepartingId, Role = TeamRole.Admin }); + + var tokenId = lent == Lent.AddedWithoutNamingAnOwner + ? await SendAsync(world.DepartingId, world.TeamA, new AddCredentialCommand { ProviderInstanceId = world.InstanceA, DisplayName = "my token", Payload = new PatPayload { Token = "glpat-mine" } }) + : await SeedOwnerlessTokenAsync(world, createdBy: world.DepartingId); + + if (lent == Lent.AddedWithoutNamingAnOwner) + (await LoadCredentialAsync(tokenId)).OwnerUserId.ShouldBe(world.DepartingId, "a personal token added without naming an owner is the caller's own"); + + (await ListCredentialIdsAsync(world.MemberId, world.TeamA)).ShouldContain(tokenId, "fixture check: while they are in the team, the token is one of its credentials"); + + await DepartAsync(world, Departure.Left); + + var token = await LoadCredentialAsync(tokenId); + token.Status.ShouldBe(CredentialStatus.Revoked, $"{lent}: the token goes with the person who added it"); + token.EncryptedPayload.ShouldBe(string.Empty); + token.DeletedDate.ShouldNotBeNull(); + (await ListCredentialIdsAsync(world.MemberId, world.TeamA)).ShouldNotContain(tokenId, $"{lent}: a Member no longer sees it"); + await Should.ThrowAsync(() => SendAsync(world.OwnerId, world.TeamA, new BindRepositoryCommand { ProviderInstanceId = world.InstanceA, CredentialId = tokenId, ProjectIdentifier = "acme/web" })); + } + + [Fact] + public async Task A_member_who_left_before_departures_retired_tokens_loses_every_token_they_left_behind_to_the_sweep() + { + // Departures used to delete the membership row and nothing else, and an older pod still does during a rolling + // deploy. Every such person's tokens are still live in the team they left — listed, browsable, bindable — until + // the sweep retires them exactly as a departure now does. + var world = await SeedWorldAsync(); + var ownerless = await SeedOwnerlessTokenAsync(world, createdBy: world.DepartingId); + var ownerlessOfAMember = await SeedOwnerlessTokenAsync(world, createdBy: world.MemberId); + var membersIdentity = await LinkTokenAsync(world.MemberId, world.TeamA, world.InstanceA, "member-token"); + var membersToken = await IdentityCredentialAsync(membersIdentity); + + await DeleteMembershipAsDeparturesUsedToAsync(world.TeamA, world.DepartingId); + + (await ListCredentialIdsAsync(world.MemberId, world.TeamA)).ShouldContain(world.PersonalCredentialA, "fixture check: a departure from before left the token live and listed"); + (await LoadRepositoryAsync(world.PersonalRepository)).Status.ShouldBe(RepositoryStatus.Active, "fixture check: and the repository bound through it working"); + + await SweepDepartedUntilRetiredAsync(world.PersonalCredentialA, ownerless); + + foreach (var retiredId in new[] { world.PersonalCredentialA, ownerless }) + { + var retired = await LoadCredentialAsync(retiredId); + retired.Status.ShouldBe(CredentialStatus.Revoked, "every token the departed member lent the team is retired"); + retired.EncryptedPayload.ShouldBe(string.Empty); + retired.DeletedDate.ShouldNotBeNull(); + } + + (await LoadIdentityAsync(world.IdentityA)).DeletedDate.ShouldNotBeNull("the act-as identity goes with its token"); + + var personalRepo = await LoadRepositoryAsync(world.PersonalRepository); + personalRepo.Status.ShouldBe(RepositoryStatus.Error, "a repository bound through a retired token says it needs a new credential"); + personalRepo.LastError.ShouldNotBeNullOrWhiteSpace(); + + (await ListCredentialIdsAsync(world.MemberId, world.TeamA)).ShouldNotContain(world.PersonalCredentialA, "a Member no longer sees the departed member's token"); + await Should.ThrowAsync(() => SendAsync(world.MemberId, world.TeamA, new ListAccessibleRepositoriesQuery { CredentialId = world.PersonalCredentialA })); + await Should.ThrowAsync(() => SendAsync(world.OwnerId, world.TeamA, new BindRepositoryCommand { ProviderInstanceId = world.InstanceA, CredentialId = world.PersonalCredentialA, ProjectIdentifier = "acme/web" })); + + foreach (var keptId in new[] { membersToken, ownerlessOfAMember, world.PersonalCredentialB }) + (await LoadCredentialAsync(keptId)).Status.ShouldBe(CredentialStatus.Active, "a token lent by someone still in its team is untouched — including the departed member's in the team they are still in"); + + (await LoadRepositoryAsync(world.ConnectionRepository)).Status.ShouldBe(RepositoryStatus.Active, "the team's own connection was never the member's"); + } + + [Fact] + public async Task What_an_instance_admin_lends_a_team_they_are_not_in_stays_the_teams_until_they_lose_the_role() + { + // Everywhere else a live instance Admin stands in every team as its Owner does, and may lend a team a token without + // joining it. A membership row is not the whole answer: read as one, the sweep retired every such token on deploy + // and broke every repository bound through it. Once the role goes, the person has no standing left, and the sweep + // retires what they lent exactly as it retires a departed member's. + var world = await SeedWorldAsync(); + var adminId = await SeedInstanceAdminAsync(); + var linked = await IdentityCredentialAsync(await LinkTokenAsync(adminId, world.TeamA, world.InstanceA, "admins-token", Roles.Admin)); + var added = await SendAsync(adminId, world.TeamA, new AddCredentialCommand { ProviderInstanceId = world.InstanceA, DisplayName = "admin's token", Payload = new PatPayload { Token = "glpat-admin" } }, Roles.Admin); + var repoId = await SeedRepositoryAsync(world.TeamA, world.InstanceA, linked, "acme/lent-by-an-admin", RepositoryStatus.Active); + var title = $"opened as an instance admin {Guid.NewGuid():N}"; + + (await LoadCredentialAsync(added)).OwnerUserId.ShouldBe(adminId, "fixture check: a token the Admin added without naming an owner is theirs"); + + await DeleteMembershipAsDeparturesUsedToAsync(world.TeamA, world.DepartingId); + await SweepDepartedUntilRetiredAsync(world.PersonalCredentialA); + await DrainDepartureSweepAsync(); + + foreach (var keptId in new[] { linked, added }) + (await LoadCredentialAsync(keptId)).Status.ShouldBe(CredentialStatus.Active, "an instance Admin has standing in every team, so what they lent one is not a departed person's token"); + + (await LoadRepositoryAsync(repoId)).Status.ShouldBe(RepositoryStatus.Active, "the repository bound through the Admin's token keeps working across the sweep"); + await OpenPullRequestAsAsync(world, adminId, title); + OpenedPullRequestTitles().ShouldContain(title, "a workflow in the team still acts as the instance Admin"); + + await RevokeInstanceAdminAsync(adminId); + await SweepDepartedUntilRetiredAsync(linked, added); + + var repo = await LoadRepositoryAsync(repoId); + repo.Status.ShouldBe(RepositoryStatus.Error, "once the role is gone, the repository bound through their token says it needs a new credential"); + repo.LastError.ShouldNotBeNullOrWhiteSpace(); + (await ResolveAsync(adminId, world.InstanceA)).ShouldBeNull("nobody acts as an ex-Admin in a team they were never in"); + await Should.ThrowAsync(() => OpenPullRequestAsAsync(world, adminId, $"{title} after the role went")); + } + + public enum Unattributable { CreatorIsEmpty, CreatorNamesNoAccount } + + [Theory] + [InlineData(Unattributable.CreatorIsEmpty)] + [InlineData(Unattributable.CreatorNamesNoAccount)] + public async Task The_sweep_never_retires_a_token_it_cannot_attribute_to_an_account(Unattributable unattributable) + { + // An owner-less token belongs, for retirement, to whoever added it — when that is someone. A creator stamp that is + // empty, or names no account (the column has no foreign key: a script could write anything), says nothing about + // who lent the token or whether they are still in the team. Retiring on that would be a guess, and a wrong one + // breaks every repository bound through it with no departure to explain it. + var world = await SeedWorldAsync(); + var tokenId = await SeedOwnerlessTokenAsync(world, createdBy: Guid.NewGuid()); + + if (unattributable == Unattributable.CreatorIsEmpty) await StampCreatorAsync(tokenId, Guid.Empty); + + await DeleteMembershipAsDeparturesUsedToAsync(world.TeamA, world.DepartingId); + await SweepDepartedUntilRetiredAsync(world.PersonalCredentialA); + await DrainDepartureSweepAsync(); + + var token = await LoadCredentialAsync(tokenId); + token.Status.ShouldBe(CredentialStatus.Active, $"{unattributable}: a token nobody can be named for is kept"); + token.EncryptedPayload.ShouldNotBeEmpty(); + token.DeletedDate.ShouldBeNull(); + } + + [Theory] + [InlineData(Departure.Removed)] + [InlineData(Departure.Left)] + public async Task An_instance_admin_who_leaves_a_team_keeps_what_they_lent_it(Departure departure) + { + // Leaving takes the membership row, not the role: an instance Admin still stands in the team afterwards, as in + // every other, so a departure has nothing of theirs to retire. + var world = await SeedWorldAsync(); + await GrantInstanceAdminAsync(world.DepartingId); + + await DepartAsync(world, departure); + + (await IsMemberAsync(world.TeamA, world.DepartingId)).ShouldBeFalse($"fixture check: {departure} took the membership row"); + (await LoadCredentialAsync(world.PersonalCredentialA)).Status.ShouldBe(CredentialStatus.Active, $"{departure}: the token an instance Admin lent the team is still the team's"); + (await LoadRepositoryAsync(world.PersonalRepository)).Status.ShouldBe(RepositoryStatus.Active, $"{departure}: and the repository bound through it keeps working"); + (await ResolveAsync(world.DepartingId, world.InstanceA)).ShouldNotBeNull($"{departure}: a workflow still acts as them"); + } + + [Fact] + public async Task An_instance_admin_outside_the_team_connects_their_own_account_through_oauth() + { + var world = await SeedWorldAsync(); + var adminId = await SeedInstanceAdminAsync(); + var oauth = new StubOAuthClient(ProviderKind.Git, new OAuthTokenResponse { AccessToken = "admins-oauth-token", RefreshToken = "rt" }); + + var init = await InitConnectAsync(oauth, adminId, world, ownerUserId: null, Roles.Admin); + + CompleteCredentialOAuthResult completed; + using (var callback = OAuthScope(oauth, userId: null, teamId: null)) + completed = await callback.Resolve().Send(new CompleteCredentialOAuthCommand { State = init.State, Code = "code", BrowserNonce = init.BrowserNonce }); + + var token = await LoadCredentialAsync(completed.CredentialId); + token.OwnerUserId.ShouldBe(adminId, "the connect is the instance Admin's own"); + token.Status.ShouldBe(CredentialStatus.Active, "an instance Admin has standing in the team, so the callback keeps their token"); + var identity = await ResolveAsync(adminId, world.InstanceA); + identity.ShouldNotBeNull("the connect links the instance Admin's act-as identity in the team"); + identity.CredentialId.ShouldBe(completed.CredentialId, "through the token they just connected"); + } + + [Fact] + public async Task A_demotion_that_commits_during_the_oauth_code_exchange_leaves_the_ex_admin_no_token() + { + // The callback re-asks standing once the provider answers. Taking the Admin role away is not an operation of this + // app — role_user is written by migrations and by hand — so there is no departure to wait on, only the answer at + // that moment: an ex-Admin outside the team is refused there, and the sweep catches a demotion that lands after it. + var world = await SeedWorldAsync(); + var adminId = await SeedInstanceAdminAsync(); + var oauth = new DepartingDuringExchangeOAuthClient(() => RevokeInstanceAdminAsync(adminId)); + + var init = await InitConnectAsync(oauth, adminId, world, ownerUserId: null, Roles.Admin); + + using (var callback = OAuthScope(oauth, userId: null, teamId: null)) + await Should.ThrowAsync(() => callback.Resolve().Send(new CompleteCredentialOAuthCommand { State = init.State, Code = "code", BrowserNonce = init.BrowserNonce })); + + oauth.Exchanged.ShouldBeTrue("fixture check: the role went inside the provider's code exchange"); + (await LiveCredentialsOwnedInAsync(world.TeamA, adminId)).ShouldBe(0, "no token of someone without standing in the team is kept"); + } + + // ── Flow steps ──────────────────────────────────────────────────────────────── + + private async Task DepartAsync(World world, Departure departure) + { + if (departure == Departure.Removed) + { + await SendAsync(world.OwnerId, world.TeamA, new RemoveTeamMemberCommand { UserId = world.DepartingId }); + return; + } + + await SendAsync(world.DepartingId, world.TeamA, new LeaveTeamCommand()); + } + + private async Task OpenPullRequestAsAsync(World world, Guid actorUserId, string title) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().OpenPullRequestAsync(world.ConnectionRepository, world.TeamA, new OpenPullRequestInput { Title = title, SourceBranch = "feature", TargetBranch = "main" }, actorUserId, CancellationToken.None); + } + + private IReadOnlyList OpenedPullRequestTitles() + { + using var scope = _fixture.BeginScope(); + + return scope.Resolve().All.Select(input => input.Title).ToList(); + } + + private async Task> ListCredentialIdsAsync(Guid userId, Guid teamId) + { + var credentials = await SendAsync(userId, teamId, new ListCredentialsQuery()); + + return credentials.Select(c => c.Id).ToList(); + } + + /// The recurring repair's tick, through the real mediator pipeline, until a pass moves nothing — the sweep is deployment-wide and the suite shares one database. + private async Task SweepUntilDrainedAsync() + { + for (var pass = 1; pass <= MaxSweepPasses; pass++) + { + using var scope = _fixture.BeginScope(); + + if (await scope.Resolve().Send(new RepairDisconnectedCredentialBindingsCommand()) == 0) return; + } + + throw new ShouldAssertException($"RepairDisconnectedCredentialBindingsCommand still repaired bindings after {MaxSweepPasses} passes — check that the sweep's candidate query drops a credential once nothing live is bound to it."); + } + + /// The departure sweep's tick, through the real mediator pipeline, until every named token is retired — never until the deployment-wide backlog drains, which other tests' leftovers in this shared database decide. + private async Task SweepDepartedUntilRetiredAsync(params Guid[] credentialIds) + { + for (var pass = 1; pass <= MaxDepartureSweepPasses; pass++) + { + using (var scope = _fixture.BeginScope()) + await scope.Resolve().Send(new RetireDepartedMembersCredentialsCommand()); + + if (await AllRetiredAsync(credentialIds)) return; + } + + throw new ShouldAssertException($"RetireDepartedMembersCredentialsCommand had not retired {string.Join(", ", credentialIds)} after {MaxDepartureSweepPasses} passes — check that the sweep's candidate query finds a live Personal token whose owner (or, owner-less, whose creator) holds no membership in its team."); + } + + /// What departures did before they retired anything: the membership row goes, and nothing else does. + private async Task DeleteMembershipAsDeparturesUsedToAsync(Guid teamId, Guid userId) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().TeamMembership.Where(m => m.TeamId == teamId && m.UserId == userId).ExecuteDeleteAsync(); + } + + /// + /// The departure sweep's tick until a pass retires nothing, so every candidate it has has been retired — for asserting + /// a token is NOT one. Called after a control token is retired, which proves the passes reach this test's rows. + /// + private async Task DrainDepartureSweepAsync() + { + for (var pass = 1; pass <= MaxDepartureSweepPasses; pass++) + { + using var scope = _fixture.BeginScope(); + + if (await scope.Resolve().Send(new RetireDepartedMembersCredentialsCommand()) == 0) return; + } + + throw new ShouldAssertException($"RetireDepartedMembersCredentialsCommand still retired tokens after {MaxDepartureSweepPasses} passes — check that a retired token stops being a candidate."); + } + + /// An account holding the instance Admin role through a role_user row, and no membership anywhere. + private async Task SeedInstanceAdminAsync() + { + var admin = NewUser("instance-admin", Guid.NewGuid().ToString("N")[..8]); + + using (var scope = _fixture.BeginScope()) + { + var db = scope.Resolve(); + + db.User.Add(admin); + await db.SaveChangesAsync(); + } + + await GrantInstanceAdminAsync(admin.Id); + + return admin.Id; + } + + private async Task GrantInstanceAdminAsync(Guid userId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.RoleUser.Add(new RoleUser { Id = Guid.NewGuid(), RoleId = await db.Role.Where(r => r.Name == Roles.Admin).Select(r => r.Id).SingleAsync(), UserId = userId }); + await db.SaveChangesAsync(); + } + + /// How the role is taken away: nothing in the app writes role_user, so an operator deletes the row. + private async Task RevokeInstanceAdminAsync(Guid userId) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().RoleUser.Where(r => r.UserId == userId).ExecuteDeleteAsync(); + } + + /// Rewrites the creator stamp past the audit pipeline, which fills an empty one in on insert. + private async Task StampCreatorAsync(Guid credentialId, Guid createdBy) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().Credential.Where(c => c.Id == credentialId).ExecuteUpdateAsync(s => s.SetProperty(c => c.CreatedBy, createdBy)); + } + + private static async Task HoldMembershipAsync(ILifetimeScope writer, Holder holder, World world) + { + var members = writer.Resolve(); + + if (holder == Holder.CallerNamingThem) + { + await members.EnsureMayActOnAsync(world.DepartingId, CancellationToken.None); + return; + } + + (await members.HoldMembershipAsync(world.TeamA, world.DepartingId, CancellationToken.None)).ShouldBeTrue("fixture check: they are a member when the write starts"); + } + + /// A live token owned by the departing member, written — not yet committed — inside the writer's transaction, as a connect writes one. + private static async Task WriteTokenInTheirNameAsync(CodeSpaceDbContext db, World world) + { + var token = new Credential { Id = Guid.NewGuid(), TeamId = world.TeamA, ProviderInstanceId = world.InstanceA, OwnerUserId = world.DepartingId, Ownership = CredentialOwnership.Personal, AuthType = AuthType.OAuth, DisplayName = "connected as they left", EncryptedPayload = "sealed", Status = CredentialStatus.Active }; + + db.Credential.Add(token); + await db.SaveChangesAsync(); + + return token.Id; + } + + /// + /// Until the departure's own connection is waiting on a row lock. Either it waits — the write's hold is what it is + /// waiting on, since nothing else in this database holds one — or it finishes first, which means it read what to + /// retire without waiting for the write and the token the write is about to commit is never retired. + /// + private async Task WaitUntilDepartureWaitsOnALockAsync(Task departure, Holder holder) + { + var deadline = DateTime.UtcNow + LockWaitTimeout; + + while (DateTime.UtcNow < deadline) + { + departure.IsCompleted.ShouldBeFalse($"{holder}: the departure finished while a write in the member's name still held their membership — it did not wait for it. Faulted: {departure.Exception?.GetBaseException().Message}"); + + if (await BackendsWaitingOnALockAsync() > 0) return; + + await Task.Delay(50); + } + + throw new ShouldAssertException($"{holder}: the departure neither finished nor waited on a lock within {LockWaitTimeout} — check pg_stat_activity for a backend whose wait_event_type is 'Lock' and what it is running."); + } + + private async Task BackendsWaitingOnALockAsync() + { + using var scope = _fixture.BeginScope(); + + return (await scope.Resolve().Database.SqlQuery($"SELECT count(*)::int AS \"Value\" FROM pg_stat_activity WHERE datname = current_database() AND wait_event_type = 'Lock'").ToListAsync()).Single(); + } + + private async Task SendAsync(Guid userId, Guid teamId, IRequest request, params string[] roles) + { + using var scope = _fixture.BeginScopeAs(userId, teamId, roles); + + return await scope.Resolve().Send(request); + } + + private async Task InitConnectAsync(IOAuthClient oauth, Guid initiatorId, World world) => await InitConnectAsync(oauth, initiatorId, world, world.DepartingId); + + private async Task InitConnectAsync(IOAuthClient oauth, Guid initiatorId, World world, Guid? ownerUserId, params string[] roles) + { + using var scope = OAuthScope(oauth, initiatorId, world.TeamA, roles); + + return await scope.Resolve().Send(new InitCredentialOAuthCommand { ProviderInstanceId = world.InstanceA, DisplayName = "their GitLab", IntendedOwnerUserId = ownerUserId }); + } + + private ILifetimeScope OAuthScope(IOAuthClient oauth, Guid? userId, Guid? teamId, params string[] roles) => + _fixture.BeginScope(b => + { + b.RegisterInstance(new TestCurrentUser(userId, "test", roles)).As().SingleInstance(); + b.RegisterInstance(new TestCurrentTeam(teamId)).As().SingleInstance(); + b.RegisterInstance(new OneClientRegistry(oauth)).SingleInstance(); + }); + + /// The real link flow as , with only the provider's token check swapped. + private ILifetimeScope LinkScope(ICredentialProbeCapability probe, Guid userId, Guid teamId) => + _fixture.BeginScope(b => + { + b.RegisterInstance(new TestCurrentUser(userId, "test")).As().SingleInstance(); + b.RegisterInstance(new TestCurrentTeam(teamId)).As().SingleInstance(); + b.RegisterInstance(new ProviderRegistry(new IProviderCapability[] { probe })).SingleInstance(); + }); + + private sealed class OneClientRegistry : IOAuthClientRegistry + { + private readonly IOAuthClient _client; + + public OneClientRegistry(IOAuthClient client) { _client = client; } + + public IOAuthClient Get(ProviderKind kind) => _client; + } + + /// A provider whose code exchange takes long enough for to commit inside it — run there, and awaited, before the token comes back. + private sealed class DepartingDuringExchangeOAuthClient : IOAuthClient + { + private readonly Func _depart; + + public DepartingDuringExchangeOAuthClient(Func depart) { _depart = depart; } + + public ProviderKind Kind => ProviderKind.Git; + + public bool Exchanged { get; private set; } + + public Uri BuildAuthorizeUrl(OAuthAuthorizeInput input) => new($"https://oauth.stub/authorize?state={input.State}"); + + public async Task ExchangeCodeAsync(OAuthCodeExchangeInput input, CancellationToken cancellationToken) + { + Exchanged = true; + await _depart(); + + return new OAuthTokenResponse { AccessToken = "at-minted-while-leaving", RefreshToken = "rt" }; + } + + public Task RefreshAsync(OAuthRefreshInput input, CancellationToken cancellationToken) => throw new NotSupportedException("no test here refreshes a token"); + + public Task RevokeAsync(OAuthRevokeInput input, CancellationToken cancellationToken) => Task.CompletedTask; + } + + /// + /// A provider token check during which runs — and is awaited — before it answers that the + /// token is valid. Parameterless, and answering exactly as does when nothing is set: + /// the fixture's capability scan activates every capability in this assembly into the real registry, so this one sits + /// beside the test provider in every test. + /// + private sealed class DepartingDuringProbe : ICredentialProbeCapability + { + public Func? DuringProbe { get; init; } + + public ProviderKind Kind => ProviderKind.Git; + + public bool Probed { get; private set; } + + public async Task ProbeCredentialAsync(ProviderContext context, CancellationToken cancellationToken) + { + Probed = true; + + if (DuringProbe != null) await DuringProbe(); + + return new CredentialProbeResult { IsValid = true, AuthenticatedUserExternalId = "test-user-id", AuthenticatedUserName = "Test User" }; + } + } + + // ── Reads ───────────────────────────────────────────────────────────────────── + + private async Task ResolveAsync(Guid userId, Guid instanceId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().ResolveAsync(userId, instanceId, CancellationToken.None); + } + + private async Task LoadCredentialAsync(Guid id) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Credential.AsNoTracking().SingleAsync(c => c.Id == id); + } + + private async Task LoadIdentityAsync(Guid id) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().UserProviderIdentity.AsNoTracking().SingleAsync(i => i.Id == id); + } + + private async Task LoadRepositoryAsync(Guid id) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Repository.AsNoTracking().SingleAsync(r => r.Id == id); + } + + private async Task RepositoriesBoundThroughAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Repository.AsNoTracking().CountAsync(r => r.CredentialId == credentialId && r.DeletedDate == null); + } + + private async Task LiveCredentialsOwnedInAsync(Guid teamId, Guid userId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Credential.AsNoTracking().CountAsync(c => c.TeamId == teamId && c.OwnerUserId == userId && c.Status == CredentialStatus.Active && c.DeletedDate == null); + } + + private async Task IsMemberAsync(Guid teamId, Guid userId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().TeamMembership.AsNoTracking().AnyAsync(m => m.TeamId == teamId && m.UserId == userId); + } + + private async Task AllRetiredAsync(Guid[] credentialIds) + { + using var scope = _fixture.BeginScope(); + + var retired = await scope.Resolve().Credential.AsNoTracking().CountAsync(c => credentialIds.Contains(c.Id) && c.Status == CredentialStatus.Revoked && c.DeletedDate != null); + + return retired == credentialIds.Length; + } + + // ── World ───────────────────────────────────────────────────────────────────── + + /// + /// Team A (Owner, Member, and the departing member as a Member) and team B, the departing member's own (they are its + /// Owner). Each team has a Git instance. Team A's connection repository is bound through a team-service token; its + /// other repository through the departing member's own token, as an Admin binding through it leaves it. The departing + /// member linked a token on BOTH instances through the real link flow. + /// + private sealed record World(Guid TeamA, Guid TeamB, Guid OwnerId, Guid MemberId, Guid DepartingId, Guid InstanceA, Guid InstanceB, Guid ConnectionRepository, Guid PersonalRepository, Guid PersonalCredentialA, Guid IdentityA, Guid PersonalCredentialB); + + private async Task SeedWorldAsync() + { + var suffix = Guid.NewGuid().ToString("N")[..8]; + var owner = NewUser("owner", suffix); + var member = NewUser("member", suffix); + var departing = NewUser("departing", suffix); + var teamA = new Team { Id = Guid.NewGuid(), Slug = $"a-{suffix}", Name = "Team A", Kind = TeamKind.Workspace }; + var teamB = new Team { Id = Guid.NewGuid(), Slug = $"b-{suffix}", Name = "Team B", Kind = TeamKind.Workspace }; + Guid instanceA, instanceB, connectionRepo; + + using (var scope = _fixture.BeginScope()) + { + var db = scope.Resolve(); + var encryptor = scope.Resolve(); + + db.User.AddRange(owner, member, departing); + db.Team.AddRange(teamA, teamB); + db.TeamMembership.AddRange(Membership(teamA, owner, TeamRole.Owner), Membership(teamA, member, TeamRole.Member), Membership(teamA, departing, TeamRole.Member), Membership(teamB, departing, TeamRole.Owner)); + + var a = NewInstance(teamA, suffix + "a", encryptor); + var b = NewInstance(teamB, suffix + "b", encryptor); + var connection = new Credential { Id = Guid.NewGuid(), TeamId = teamA.Id, ProviderInstanceId = a.Id, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.Pat, DisplayName = "connection", EncryptedPayload = encryptor.Encrypt(scope.Resolve().Serialize(new PatPayload { Token = "team-token" })), Status = CredentialStatus.Active }; + + db.ProviderInstance.AddRange(a, b); + db.Credential.Add(connection); + await db.SaveChangesAsync(); + + (instanceA, instanceB) = (a.Id, b.Id); + connectionRepo = await SeedRepositoryAsync(teamA.Id, a.Id, connection.Id, "acme/api", RepositoryStatus.Active); + } + + var identityA = await LinkTokenAsync(departing.Id, teamA.Id, instanceA, "departing-token-a"); + await LinkTokenAsync(departing.Id, teamB.Id, instanceB, "departing-token-b"); + + var personalA = await IdentityCredentialAsync(identityA); + var personalB = await LiveCredentialOwnedOnAsync(instanceB, departing.Id); + var personalRepo = await SeedRepositoryAsync(teamA.Id, instanceA, personalA, "acme/personal", RepositoryStatus.Active); + + return new World(teamA.Id, teamB.Id, owner.Id, member.Id, departing.Id, instanceA, instanceB, connectionRepo, personalRepo, personalA, identityA, personalB); + } + + /// A token the departing member connected thirty days ago and disconnected twenty days ago — revoked and blanked, as a disconnect leaves it. + private async Task SeedDisconnectedTokenAsync(World world) + { + var connected = DateTimeOffset.UtcNow.AddDays(-30); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var dead = new Credential { Id = Guid.NewGuid(), TeamId = world.TeamA, ProviderInstanceId = world.InstanceA, OwnerUserId = world.DepartingId, AuthType = AuthType.OAuth, DisplayName = "disconnected", Status = CredentialStatus.Revoked, EncryptedPayload = string.Empty, CreatedDate = connected, LastModifiedDate = connected.AddDays(10) }; + + db.Credential.Add(dead); + await db.SaveChangesAsync(); + + return dead.Id; + } + + /// + /// A personal token with no owner, as adding one without naming an owner stored it before the owner defaulted to the + /// caller: Personal, owner-less, created by . Production wrote exactly this row through + /// POST /api/credentials/pat without ownerUserId. + /// + private async Task SeedOwnerlessTokenAsync(World world, Guid createdBy) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var token = new Credential { Id = Guid.NewGuid(), TeamId = world.TeamA, ProviderInstanceId = world.InstanceA, Ownership = CredentialOwnership.Personal, AuthType = AuthType.Pat, DisplayName = "added without an owner", EncryptedPayload = scope.Resolve().Encrypt(scope.Resolve().Serialize(new PatPayload { Token = "glpat-ownerless" })), Status = CredentialStatus.Active, CreatedBy = createdBy, LastModifiedBy = createdBy }; + + db.Credential.Add(token); + await db.SaveChangesAsync(); + + return token.Id; + } + + private async Task LinkTokenAsync(Guid userId, Guid teamId, Guid instanceId, string token, params string[] roles) + { + var linked = await SendAsync(userId, teamId, new LinkProviderIdentityByPatCommand { ProviderInstanceId = instanceId, AccessToken = token }, roles); + + return linked.Id; + } + + private async Task IdentityCredentialAsync(Guid identityId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().UserProviderIdentity.AsNoTracking().Where(i => i.Id == identityId).Select(i => i.CredentialId).SingleAsync(); + } + + private async Task LiveCredentialOwnedOnAsync(Guid instanceId, Guid userId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Credential.AsNoTracking().Where(c => c.ProviderInstanceId == instanceId && c.OwnerUserId == userId && c.DeletedDate == null).Select(c => c.Id).SingleAsync(); + } + + private async Task SeedRepositoryAsync(Guid teamId, Guid instanceId, Guid credentialId, string fullPath, RepositoryStatus status) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var name = fullPath.Split('/')[1]; + var repo = new Repository { Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instanceId, CredentialId = credentialId, ExternalId = $"ext-{Guid.NewGuid():N}", NamespacePath = "acme", Name = name, FullPath = fullPath, DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = $"https://git.test/{fullPath}", Status = status }; + + db.Repository.Add(repo); + await db.SaveChangesAsync(); + + return repo.Id; + } + + private static User NewUser(string label, string suffix) => new() { Id = Guid.NewGuid(), Email = $"{label}-{suffix}@departed.test", Name = $"{label}-{suffix}" }; + + private static TeamMembership Membership(Team team, User user, TeamRole role) => new() { Id = Guid.NewGuid(), TeamId = team.Id, UserId = user.Id, Role = role }; + + private static ProviderInstance NewInstance(Team team, string suffix, IPayloadEncryptor encryptor) => + new() { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.Git, DisplayName = "Git", BaseUrl = $"https://git-{suffix}.test", OauthClientId = "client", OauthClientSecretEnc = encryptor.Encrypt("secret") }; +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Providers/ActAsCandidatesFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Providers/ActAsCandidatesFlowTests.cs index 8c62ecfbb..819777614 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Providers/ActAsCandidatesFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Providers/ActAsCandidatesFlowTests.cs @@ -4,8 +4,10 @@ using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.Providers.Identity; using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Constants; using CodeSpace.Messages.Credentials; using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; using Shouldly; namespace CodeSpace.IntegrationTests.Providers; @@ -84,6 +86,33 @@ public async Task Excludes_a_user_who_is_not_a_team_member() candidates.ShouldBeEmpty(); } + public enum Standing { Member, Departed, TeamDeleted, InstanceAdminOutsideTheTeam, ExInstanceAdminOutsideTheTeam } + + [Theory] + [InlineData(Standing.Member, true)] + [InlineData(Standing.Departed, false)] + [InlineData(Standing.TeamDeleted, false)] + [InlineData(Standing.InstanceAdminOutsideTheTeam, true)] + [InlineData(Standing.ExInstanceAdminOutsideTheTeam, false)] + public async Task The_picker_and_the_write_agree_on_who_may_act(Standing standing, bool acts) + { + // One predicate decides both, so the picker offers exactly the people a write can act as. The write used to skip + // the membership half: someone removed from the team dropped out of the picker, yet a workflow that named them + // still wrote through their token. Departed deletes the membership row and nothing else — a departure from before + // departures retired tokens — so the identity and its token are still live and only standing can refuse it. An + // instance Admin stands in every team without a row, as everywhere else; once the role goes, they stand nowhere. + var ctx = await SeedAsync().ConfigureAwait(false); + var actor = await AddMemberAsync(ctx.TeamId, "actor").ConfigureAwait(false); + await LinkIdentityAsync(ctx, actor, "bob", "777").ConfigureAwait(false); + await ApplyStandingAsync(ctx.TeamId, actor, standing).ConfigureAwait(false); + + var offered = (await ListAsync(ctx.RepositoryId, ctx.TeamId).ConfigureAwait(false)).Any(c => c.UserId == actor); + var resolves = await ResolveAsync(actor, ctx.ProviderInstanceId).ConfigureAwait(false) != null; + + offered.ShouldBe(acts, $"{standing}: the picker {(acts ? "offers" : "hides")} them"); + resolves.ShouldBe(acts, $"{standing}: a write naming them {(acts ? "acts as them" : "is refused")}, exactly as the picker says"); + } + [Fact] public async Task Empty_when_the_repository_is_not_the_teams() { @@ -155,6 +184,32 @@ private async Task SeedAsync() return new SeedCtx(team.Id, owner.Id, instance.Id, repo.Id); } + private async Task AddMemberAsync(Guid teamId, string name) + { + var userId = await AddUserAsync(name).ConfigureAwait(false); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = teamId, UserId = userId, Role = TeamRole.Member }); + await db.SaveChangesAsync().ConfigureAwait(false); + + return userId; + } + + private async Task ApplyStandingAsync(Guid teamId, Guid userId, Standing standing) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + if (standing is Standing.Departed or Standing.InstanceAdminOutsideTheTeam or Standing.ExInstanceAdminOutsideTheTeam) db.TeamMembership.RemoveRange(db.TeamMembership.Where(m => m.TeamId == teamId && m.UserId == userId)); + if (standing == Standing.TeamDeleted) (await db.Team.SingleAsync(t => t.Id == teamId).ConfigureAwait(false)).DeletedDate = DateTimeOffset.UtcNow; + if (standing is Standing.InstanceAdminOutsideTheTeam or Standing.ExInstanceAdminOutsideTheTeam) db.RoleUser.Add(new RoleUser { Id = Guid.NewGuid(), RoleId = await db.Role.Where(r => r.Name == Roles.Admin).Select(r => r.Id).SingleAsync().ConfigureAwait(false), UserId = userId }); + + await db.SaveChangesAsync().ConfigureAwait(false); + + if (standing == Standing.ExInstanceAdminOutsideTheTeam) await db.RoleUser.Where(r => r.UserId == userId).ExecuteDeleteAsync().ConfigureAwait(false); + } + private async Task AddUserAsync(string name) { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.IntegrationTests/Providers/UserProviderIdentityServiceTests.cs b/backend/tests/CodeSpace.IntegrationTests/Providers/UserProviderIdentityServiceTests.cs index c205aba97..7b0fe9cef 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Providers/UserProviderIdentityServiceTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Providers/UserProviderIdentityServiceTests.cs @@ -118,6 +118,29 @@ public async Task Unlink_soft_deletes_the_identity_and_clears_the_credential_tok identity.Credential.EncryptedPayload.ShouldBe(string.Empty, "unlink must leave no usable token at rest"); } + [Fact] + public async Task Unlink_breaks_the_repositories_bound_through_the_credential() + { + // Unlinking retires the token behind the identity, so a repository still bound through it has nothing left to + // authenticate with. It says so — the "needs a new credential" a Disconnect leaves — instead of staying Active + // until a run dies on the blanked token. + var (userId, teamId, instanceId) = await SeedAsync(); + + Guid identityId; + using (var scope = ScopeAs(userId, teamId, ValidProbe("8", "dana"))) + identityId = (await scope.Resolve().LinkByPatAsync(instanceId, "tok", CancellationToken.None)).Id; + + var repoId = await SeedRepositoryBoundThroughAsync(teamId, instanceId, await IdentityCredentialAsync(identityId)); + + using (var scope = ScopeAs(userId, teamId, ValidProbe("8", "dana"))) + await scope.Resolve().UnlinkAsync(identityId, CancellationToken.None); + + using var verify = _fixture.BeginScope(); + var repo = await verify.Resolve().Repository.AsNoTracking().SingleAsync(r => r.Id == repoId); + repo.Status.ShouldBe(RepositoryStatus.Error, "a repository bound through an unlinked token must say it needs a new credential"); + repo.LastError.ShouldNotBeNullOrWhiteSpace(); + } + [Fact] public async Task List_is_scoped_to_the_calling_user() { @@ -175,6 +198,26 @@ private static StubProbe ValidProbe(string externalId, string username) => return (user.Id, team.Id, instance.Id); } + private async Task IdentityCredentialAsync(Guid identityId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().UserProviderIdentity.AsNoTracking().Where(i => i.Id == identityId).Select(i => i.CredentialId).SingleAsync(); + } + + private async Task SeedRepositoryBoundThroughAsync(Guid teamId, Guid instanceId, Guid credentialId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var suffix = Guid.NewGuid().ToString("N")[..8]; + var repo = new Repository { Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instanceId, CredentialId = credentialId, ExternalId = $"ext-{suffix}", NamespacePath = "acme", Name = "web", FullPath = $"acme/web-{suffix}", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = "https://git.local/acme/web", Status = RepositoryStatus.Active }; + + db.Repository.Add(repo); + await db.SaveChangesAsync(); + + return repo.Id; + } + private async Task SeedUserAsync(Guid teamId) { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs index 576dc11b9..eeb5b3dce 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs @@ -139,6 +139,7 @@ public async Task Budget_settlement_releases_the_terminal_map_branch_reservation [InlineData(typeof(ReconcileStuckWebhookRegistrationsCommand))] [InlineData(typeof(RepairDisconnectedCredentialBindingsCommand))] [InlineData(typeof(ResumeAbandonedArtifactTransfersCommand))] + [InlineData(typeof(RetireDepartedMembersCredentialsCommand))] [InlineData(typeof(SweepBudgetSettlementCommand))] [InlineData(typeof(SweepCompletionShadowCommand))] [InlineData(typeof(TierStaleModelCapabilitiesCommand))] diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs index f7bf2b00a..c32a9fbd1 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs @@ -78,6 +78,7 @@ public class RequestAuthorizationInventoryTests ["SweepCompletionShadowCommand"] = "sweep", ["BackfillPackCloneUrlsCommand"] = "seals the clone URL of pack rows that still hold a pasted token in plaintext, across every team, in bounded batches; it changes where a pack's existing credential is kept, never which source a pack clones or what any user can reach, and a sealed row is no longer a candidate.", ["RepairDisconnectedCredentialBindingsCommand"] = "moves repositories and group hooks still bound to a disconnected credential onto the credential that credential's own owner connected afterwards on the same provider instance and team, across every team, in bounded batches; it never moves a binding off a live credential or onto another person's, so it only completes a reconnect the owner already made, and a moved binding is no longer a candidate.", + ["RetireDepartedMembersCredentialsCommand"] = "retires the personal tokens whose owner — or, for one stored without an owner, whose creator account — has no standing in the team the token was lent to (no membership there and not an instance Admin), across every team, in bounded batches; it ends access the person no longer has standing to lend, exactly as their departure does, never touches a team-service token, the token of anyone who still stands in its team, or one it cannot attribute to an account, and a retired token is no longer a candidate.", ["BackfillRunScorecardsCommand"] = "projects the observation-only north-star row for terminal runs that terminalized before the table existed, across every team, in bounded batches; a run that already has a row is not a candidate, so it can only ever add a measurement of a settled run — it authors nothing a user owns and changes no run's outcome.", ["SweepStaleAgentWorkspacesCommand"] = "sweep", ["TierStaleModelCapabilitiesCommand"] = "sweep", diff --git a/backend/tests/CodeSpace.UnitTests/Jobs/DepartedMemberCredentialRetirementDispatchTests.cs b/backend/tests/CodeSpace.UnitTests/Jobs/DepartedMemberCredentialRetirementDispatchTests.cs new file mode 100644 index 000000000..74e2b1576 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Jobs/DepartedMemberCredentialRetirementDispatchTests.cs @@ -0,0 +1,102 @@ +using CodeSpace.Core.Handlers.CommandHandlers.Credentials; +using CodeSpace.Core.Jobs.RecurringJobs; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Mediation; +using MediatR; +using Shouldly; + +namespace CodeSpace.UnitTests.Jobs; + +/// +/// 🟢 Unit: retiring the tokens of people no longer in the team they lent them to is the standard job → command → service +/// chain (Rule 14 + Rule 16) — the job holds no query, the handler holds no logic. Hand-rolled recording doubles, matching +/// the codebase convention. +/// +[Trait("Category", "Unit")] +public class DepartedMemberCredentialRetirementDispatchTests +{ + [Fact] + public async Task The_retirement_job_dispatches_its_command_every_five_minutes_and_holds_no_logic() + { + var mediator = new RecordingMediator(); + var job = new DepartedMemberCredentialRetirementRecurringJob(mediator); + + job.JobId.ShouldBe(nameof(DepartedMemberCredentialRetirementRecurringJob), "Hangfire indexes the schedule by this id — a rename strands the old one"); + job.CronExpression.ShouldBe("*/5 * * * *", "a committed cadence: a token left behind by a departure an older pod served stops being the team's within five minutes"); + + await job.Execute(); + + mediator.Sent.ShouldHaveSingleItem().ShouldBeOfType("the job is a thin dispatcher — it only sends the command"); + } + + [Fact] + public async Task The_retirement_handler_forwards_the_batch_size_and_returns_the_retired_count() + { + var revocation = new RecordingRevocation { ToReturn = 3 }; + var handler = new RetireDepartedMembersCredentialsCommandHandler(revocation); + + var retired = await handler.Handle(new RetireDepartedMembersCredentialsCommand { BatchSize = 7 }, CancellationToken.None); + + revocation.BatchSizes.ShouldHaveSingleItem().ShouldBe(7, "the handler delegates the whole pass to the service (Rule 16)"); + retired.ShouldBe(3, "the handler surfaces the service's count verbatim"); + } + + [Fact] + public void The_retirement_command_is_bounded_and_settles_each_credential_on_its_own() + { + new RetireDepartedMembersCredentialsCommand().BatchSize.ShouldBe(50, "an unbounded pass would let one tick retire every orphaned token in the deployment in one go"); + + typeof(INonTransactionalCommand).IsAssignableFrom(typeof(RetireDepartedMembersCredentialsCommand)) + .ShouldBeTrue("each credential is retired and saved on its own; one transaction around the pass would let one failing credential undo the rest"); + } + + /// Records the requests sent through the mediator; the rest of the surface is unreachable in these tests. + private sealed class RecordingMediator : IMediator + { + public List Sent { get; } = new(); + + public Task Send(IRequest request, CancellationToken cancellationToken = default) + { + Sent.Add(request); + return Task.FromResult(default(TResponse)!); + } + + public Task Send(object request, CancellationToken cancellationToken = default) + { + Sent.Add(request); + return Task.FromResult(null); + } + + public Task Send(TRequest request, CancellationToken cancellationToken = default) where TRequest : IRequest + { + Sent.Add(request!); + return Task.CompletedTask; + } + + public IAsyncEnumerable CreateStream(IStreamRequest request, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public IAsyncEnumerable CreateStream(object request, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task Publish(object notification, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task Publish(TNotification notification, CancellationToken cancellationToken = default) where TNotification : INotification => throw new NotSupportedException(); + } + + /// Records the batch sizes asked for + returns a canned count, so the handler test asserts pure delegation. + private sealed class RecordingRevocation : ICredentialRevocationService + { + public List BatchSizes { get; } = new(); + public int ToReturn; + + public Task CascadeAsync(Guid credentialId, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task RetireAsync(Credential credential, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task RetireDepartedInTeamAsync(Guid teamId, Guid userId, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task RetireDepartedAsync(int batchSize, CancellationToken cancellationToken) + { + BatchSizes.Add(batchSize); + return Task.FromResult(ToReturn); + } + } +}