diff --git a/backend/src/CodeSpace.Api/Controllers/CredentialsController.cs b/backend/src/CodeSpace.Api/Controllers/CredentialsController.cs index da13c1049..8e72e1711 100644 --- a/backend/src/CodeSpace.Api/Controllers/CredentialsController.cs +++ b/backend/src/CodeSpace.Api/Controllers/CredentialsController.cs @@ -102,6 +102,19 @@ public async Task Revoke([FromRoute] Guid credentialId, [FromBody return Ok(result); } + /// + /// Replace a credential's token in place: the credential keeps its id, so everything bound through it keeps working and + /// what its dead token had broken comes back. The new token must be the same provider account's. The token travels in + /// the body only. Route's credentialId is authoritative, merged into the command with with so a body that + /// disagrees cannot redirect the replacement to another credential. + /// + [HttpPut("{credentialId:guid}/token")] + public async Task ReplaceToken([FromRoute] Guid credentialId, [FromBody] ReplaceCredentialTokenCommand command, CancellationToken cancellationToken) + { + var result = await _mediator.Send(command with { CredentialId = credentialId }, cancellationToken).ConfigureAwait(false); + return Ok(result); + } + /// /// Move every repository and group hook still bound through a dead credential onto another credential in one action. /// Route's fromCredentialId is authoritative; the body carries toCredentialId. diff --git a/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/InitCredentialOAuthCommandHandler.cs b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/InitCredentialOAuthCommandHandler.cs index fac47b74c..b25262311 100644 --- a/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/InitCredentialOAuthCommandHandler.cs +++ b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/InitCredentialOAuthCommandHandler.cs @@ -11,5 +11,5 @@ public sealed class InitCredentialOAuthCommandHandler : IRequestHandler Handle(InitCredentialOAuthCommand request, CancellationToken cancellationToken) => - await _flow.InitAsync(request.ProviderInstanceId, request.DisplayName, request.IntendedOwnerUserId, request.ReturnUrl, request.Scopes, cancellationToken).ConfigureAwait(false); + await _flow.InitAsync(request, cancellationToken).ConfigureAwait(false); } diff --git a/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/ReplaceCredentialTokenCommandHandler.cs b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/ReplaceCredentialTokenCommandHandler.cs new file mode 100644 index 000000000..35c0a9441 --- /dev/null +++ b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/ReplaceCredentialTokenCommandHandler.cs @@ -0,0 +1,15 @@ +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Messages.Commands.Credentials; +using MediatR; + +namespace CodeSpace.Core.Handlers.CommandHandlers.Credentials; + +public sealed class ReplaceCredentialTokenCommandHandler : IRequestHandler +{ + private readonly ICredentialService _credentials; + + public ReplaceCredentialTokenCommandHandler(ICredentialService credentials) { _credentials = credentials; } + + public async Task Handle(ReplaceCredentialTokenCommand request, CancellationToken cancellationToken) => + await _credentials.ReplaceTokenAsync(request.CredentialId, request.Token, cancellationToken).ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0250_oauth_pending_state_replace_credential.sql b/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0250_oauth_pending_state_replace_credential.sql new file mode 100644 index 000000000..d771291ba --- /dev/null +++ b/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0250_oauth_pending_state_replace_credential.sql @@ -0,0 +1,21 @@ +-- 0250_oauth_pending_state_replace_credential.sql +-- +-- Lets a pending OAuth connect name a credential to re-authorize in place. +-- +-- Every reconnect used to mint a NEW credential row, and everything bound through the old one — repositories, group +-- hooks, the act-as identity — pointed at a row whose token was dead until a whole succession subsystem moved it. A +-- connect that names the credential it replaces writes the new token into that row instead: same id, nothing to move. +-- /init stores the credential's id here; the callback reads it back and re-authorizes that row. +-- +-- Nullable and additive, for the rolling deploy: a pod still on the previous version inserts rows without the column +-- (NULL here) and ignores it on the rows a newer pod wrote — it then completes the connect as a new credential, exactly as +-- it always did. A row with NULL is a plain connect for every version. +-- +-- No foreign key, on purpose. A pending row lives ten minutes and is swept by the expiry job; the reference is checked +-- again against (id, team) when the callback uses it, because the credential can be retired in between, and a hard +-- constraint would only make a row that can never complete a reason for something unrelated to fail. + +ALTER TABLE oauth_pending_state ADD COLUMN IF NOT EXISTS replace_credential_id UUID NULL; + +COMMENT ON COLUMN oauth_pending_state.replace_credential_id IS + 'The credential this connect re-authorizes in place (its token is replaced, the id kept); NULL connects a new credential. Not a foreign key: re-checked against (id, team) when the callback uses it.'; diff --git a/backend/src/CodeSpace.Core/Persistence/Entities/OAuthPendingState.cs b/backend/src/CodeSpace.Core/Persistence/Entities/OAuthPendingState.cs index 81e1ee324..a577c938a 100644 --- a/backend/src/CodeSpace.Core/Persistence/Entities/OAuthPendingState.cs +++ b/backend/src/CodeSpace.Core/Persistence/Entities/OAuthPendingState.cs @@ -18,6 +18,13 @@ public class OAuthPendingState : IAuditable public Guid? IntendedOwnerUserId { get; set; } public string? ReturnUrl { get; set; } + /// + /// The credential the callback re-authorizes in place instead of creating another, or null for a plain connect. Written + /// by a version from migration 0250 on; a row an older pod wrote reads null, and an older pod completing a newer row + /// ignores it and creates a new credential. + /// + public Guid? ReplaceCredentialId { get; set; } + /// /// SHA-256 of the nonce the starting browser holds in its binding cookie. A callback that presents that nonce finishes /// the connect on the spot; one that does not consumes nothing and is handed to the SPA, where only diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialHealthDerivation.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialHealthDerivation.cs index 620dea2ac..f649d623b 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/CredentialHealthDerivation.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialHealthDerivation.cs @@ -1,3 +1,4 @@ +using CodeSpace.Messages.Dtos.Providers; using CodeSpace.Messages.Enums; namespace CodeSpace.Core.Services.Credentials; @@ -34,6 +35,16 @@ public static CredentialHealth Derive(CredentialStatus status, AuthType authType /// The credentials whose provider-side expiry CodeSpace records: access tokens a person created, not tokens the provider issued for a grant. public static bool IsStaticToken(AuthType authType) => authType is AuthType.Pat or AuthType.ProjectAccessToken or AuthType.GroupAccessToken; + /// + /// What to record about a static token's expiry from what its provider said about the token: the expiry as reported — + /// recorded even when it is "none" — but only when the provider described the token ( + /// is set). Null leaves the column alone: nothing was described, or the credential is an OAuth one, whose expiry is its + /// access token's and is the refresher's to write. One rule for every writer that has just probed a token — the sweep, a + /// credential being added, a token that replaces another — so they cannot disagree about what "expires" means. + /// + public static ExpiryRecord? RecordedExpiry(AuthType authType, CredentialProbeResult? probe) => + IsStaticToken(authType) && probe is { Active: not null } ? new ExpiryRecord(probe.ExpiresAt) : null; + private static bool HasExpired(AuthType authType, DateTimeOffset? expiresDate, DateTimeOffset now) => IsStaticToken(authType) && expiresDate is { } at && at <= now; private static bool IsExpiringSoon(AuthType authType, DateTimeOffset? expiresDate, DateTimeOffset now) => IsStaticToken(authType) && expiresDate is { } at && at - now <= ExpiringWithin; diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialHealthService.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialHealthService.cs index a41e94670..76b6fb5b9 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/CredentialHealthService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialHealthService.cs @@ -12,7 +12,7 @@ namespace CodeSpace.Core.Services.Credentials; -public sealed class CredentialHealthService : ICredentialHealthService, IScopedDependency +public sealed class CredentialHealthService : ICredentialHealthService, IBoundedCredentialProbe, IScopedDependency { private const string SweepName = "credential-health-probe"; @@ -140,7 +140,7 @@ private async Task ProbeOneAsync(DueCredential candidate, CancellationToke /// verdict in its row stops anything resolving its token, and the probe is how the verdict is lifted. The copy is read /// without tracking and never saved, so the pretence reaches no row. /// - private async Task ProbeAsync(ProviderInstance instance, Credential credential, CancellationToken cancellationToken) + public async Task ProbeAsync(ProviderInstance instance, Credential credential, CancellationToken cancellationToken) { var provider = instance.Provider; var probe = _registry.Require(provider); @@ -176,7 +176,7 @@ private async Task RecordAsync(Credential credential, CredentialProbeResul if (result.IsValid) { - await _writer.MarkValidatedAsync(credential.Id, credential.EncryptedPayload, _clock.GetUtcNow(), ExpiryOf(credential, result), cancellationToken).ConfigureAwait(false); + await _writer.MarkValidatedAsync(credential.Id, credential.EncryptedPayload, _clock.GetUtcNow(), CredentialHealthDerivation.RecordedExpiry(credential.AuthType, result), cancellationToken).ConfigureAwait(false); return true; } @@ -200,7 +200,7 @@ public async Task ValidateNewAsync(Credential credential, ProviderInstance insta credential.LastValidatedDate = _clock.GetUtcNow(); - if (ExpiryOf(credential, result) is { } expiry) credential.ExpiresDate = expiry.ExpiresAt; + if (CredentialHealthDerivation.RecordedExpiry(credential.AuthType, result) is { } expiry) credential.ExpiresDate = expiry.ExpiresAt; } /// The probe of a credential that is not saved yet, or null when it could not be asked: a failure here is the provider's, and adding the credential does not wait on it or fail with it. The caller's own cancellation still ends the add. @@ -218,9 +218,5 @@ public async Task ValidateNewAsync(Credential credential, ProviderInstance insta } } - /// A static token's expiry as its provider reports it — recorded even when it is "none" — but only when the provider described the token; an OAuth credential's expiry is its access token's and is not this sweep's to write. - private static ExpiryRecord? ExpiryOf(Credential credential, CredentialProbeResult result) => - CredentialHealthDerivation.IsStaticToken(credential.AuthType) && result.Active.HasValue ? new ExpiryRecord(result.ExpiresAt) : null; - private sealed record DueCredential(Guid Id) : ISweepCandidate; } diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizationWriter.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizationWriter.cs new file mode 100644 index 000000000..40f4d893e --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizationWriter.cs @@ -0,0 +1,44 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Settings.Database; +using CodeSpace.Messages.Credentials; +using Npgsql; +using NpgsqlTypes; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed class CredentialReauthorizationWriter : ICredentialReauthorizationWriter, IScopedDependency +{ + private const string Sql = @"UPDATE credential +SET encrypted_payload = @enc, auth_type = @authType, scopes = @scopes, expires_date = @exp, + display_name = COALESCE(@name, display_name), + status = 'Active', last_error = NULL, last_validated_date = @now, + last_modified_date = @now, last_modified_by = @user +WHERE id = @id AND deleted_date IS NULL AND encrypted_payload = @expected"; + + private readonly CodeSpaceConnectionString _connectionString; + + public CredentialReauthorizationWriter(CodeSpaceConnectionString connectionString) { _connectionString = connectionString; } + + public async Task TryReplaceAsync(CredentialReplacement replacement, CancellationToken cancellationToken) + { + // Detached Npgsql connection — must NOT join an outer EF transaction. The new token is the only copy of an + // authorization the provider will not issue again from the same code; if an outer rollback discarded it the person + // would have to authorize again. + await using var conn = new NpgsqlConnection(_connectionString.Value); + await conn.OpenAsync(cancellationToken).ConfigureAwait(false); + + await using var cmd = conn.CreateCommand(); + cmd.CommandText = Sql; + cmd.Parameters.AddWithValue("@enc", replacement.EncryptedPayload); + cmd.Parameters.AddWithValue("@authType", replacement.AuthType.ToString()); + cmd.Parameters.Add(new NpgsqlParameter("@scopes", NpgsqlDbType.Array | NpgsqlDbType.Text) { Value = replacement.Scopes?.ToArray() ?? (object)DBNull.Value }); + cmd.Parameters.Add(new NpgsqlParameter("@exp", NpgsqlDbType.TimestampTz) { Value = (object?)replacement.ExpiresDate?.ToUniversalTime() ?? DBNull.Value }); + cmd.Parameters.Add(new NpgsqlParameter("@name", NpgsqlDbType.Text) { Value = (object?)replacement.DisplayName ?? DBNull.Value }); + cmd.Parameters.AddWithValue("@now", DateTimeOffset.UtcNow); + cmd.Parameters.AddWithValue("@user", replacement.ModifiedBy); + cmd.Parameters.AddWithValue("@id", replacement.CredentialId); + cmd.Parameters.AddWithValue("@expected", replacement.ExpectedEncryptedPayload); + + return await cmd.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false) == 1; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Bindings.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Bindings.cs new file mode 100644 index 000000000..54a00517e --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Bindings.cs @@ -0,0 +1,43 @@ +using CodeSpace.Messages.Enums; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed partial class CredentialReauthorizer +{ + /// What the credential's dead token had broken comes back — the repositories it turned Error, the hooks it buried — with nothing re-pointed, because nothing moved. + private async Task ReviveBindingsAsync(CancellationToken cancellationToken) + { + var revived = await _bindings.ReviveBindingsAsync(_ctx.Credential, cancellationToken).ConfigureAwait(false); + + if (revived.Any) + _logger.LogInformation("Re-authorized credential {CredentialId} in place; {RepositoryCount} repositories and {ConnectionWebhookCount} group hooks bound through it came back", _ctx.Credential.Id, revived.RepositoryCount, revived.ConnectionWebhookCount); + } + + /// + /// The owner's act-as identity is pointed at this credential again — created if their Disconnect had ended it — as a + /// connect does. Only when the owner is the one who re-authorized: an identity says "this account speaks for this + /// person", and only the person can say that. Best-effort, as at connect: the credential is already valid for repository + /// operations, and a probe hiccup right after a valid token must not undo a re-authorization that has been written. + /// + private async Task RelinkActorIdentityBestEffortAsync(CancellationToken cancellationToken) + { + if (!ActorIsTheOwner(out var ownerId)) return; + + try + { + await _identities.EnsureIdentityForCredentialAsync(_ctx.Instance, _ctx.Credential, ownerId, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (!cancellationToken.IsCancellationRequested) + { + _logger.LogWarning(ex, "Re-authorized credential {CredentialId} but could not relink the act-as identity of user {UserId}; act-as stays unavailable until they re-authorize again", _ctx.Credential.Id, ownerId); + } + } + + private bool ActorIsTheOwner(out Guid ownerId) + { + ownerId = _ctx.Credential.OwnerUserId ?? Guid.Empty; + + return _ctx.Credential.Ownership == CredentialOwnership.Personal && ownerId != Guid.Empty && ownerId == _ctx.Request.ActorUserId; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Context.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Context.cs new file mode 100644 index 000000000..042242654 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Context.cs @@ -0,0 +1,38 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed partial class CredentialReauthorizer +{ + private static ReauthorizationContext BuildContext(CredentialReauthorization request) => new() { Request = request }; + + /// The state one re-authorization builds up, step by step, and the next step reads. One per call. + private sealed class ReauthorizationContext + { + public required CredentialReauthorization Request { get; init; } + + /// The row, read untracked: as it was before the write until brings it up to date. + public Credential Credential { get; set; } = default!; + + public ProviderInstance Instance => Credential.ProviderInstance; + + /// The auth type the row had before this re-authorization — what its replaced payload must be read as. + public AuthType PreviousAuthType { get; set; } + + public string NewEncryptedPayload { get; set; } = string.Empty; + + /// The provider's answer for the new token; null when the provider has no probe or the probe could not be run. + public CredentialProbeResult? Profile { get; set; } + + /// The provider account the credential was connected as, when something durable says so. + public KnownAccount? Known { get; set; } + + /// What the row held when it was replaced — the payload the write was compared against. + public string ReplacedEncryptedPayload { get; set; } = string.Empty; + } + + private sealed record KnownAccount(string ProviderUserId, string ProviderUsername); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Filter.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Filter.cs new file mode 100644 index 000000000..598595473 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Filter.cs @@ -0,0 +1,44 @@ +using CodeSpace.Messages.Credentials; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed partial class CredentialReauthorizer +{ + public enum AccountMatch + { + /// Nothing both sides know to compare — the provider gave no id and no name, or the credential has none on record. + Unknown, + Same, + Different + } + + /// + /// Whether the account that signed in is the account on record. The provider's own id is authoritative where both sides + /// have one — handles are renamed, ids are not — and the handle, compared without regard to case as the providers do, is + /// the fallback when either side lacks an id. + /// + internal static AccountMatch Compare(string? knownId, string? knownName, string? presentedId, string? presentedName) + { + if (!string.IsNullOrWhiteSpace(knownId) && !string.IsNullOrWhiteSpace(presentedId)) return knownId == presentedId ? AccountMatch.Same : AccountMatch.Different; + + if (!string.IsNullOrWhiteSpace(knownName) && !string.IsNullOrWhiteSpace(presentedName)) return string.Equals(knownName, presentedName, StringComparison.OrdinalIgnoreCase) ? AccountMatch.Same : AccountMatch.Different; + + return AccountMatch.Unknown; + } + + /// + /// The OAuth tokens the re-authorization replaced and the provider should be asked to end, or null when there is nothing to + /// end: the row held no OAuth tokens, or the provider handed the new authorization the very tokens it already held — one + /// grant reused for a second sign-in, as GitLab through 15.4 does — and revoking those would revoke the new ones. + /// + internal static OAuthPayload? ReplacedPairToRevoke(CredentialPayload? replaced, CredentialPayload next) + { + if (replaced is not OAuthPayload pair) return null; + + if (next is not OAuthPayload fresh) return pair; + + var sharesAToken = pair.AccessToken == fresh.AccessToken || (!string.IsNullOrEmpty(pair.RefreshToken) && pair.RefreshToken == fresh.RefreshToken); + + return sharesAToken ? null : pair; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Prepare.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Prepare.cs new file mode 100644 index 000000000..b786a7b89 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Prepare.cs @@ -0,0 +1,53 @@ +using CodeSpace.Core.Authorization; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using Microsoft.EntityFrameworkCore; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed partial class CredentialReauthorizer +{ + /// + /// The credential by (id, the request's team), retired or not: a row of another team reads as not found, exactly as an + /// id that names nothing, and a retired one is told apart below because it deserves its own answer. + /// + private async Task LoadCredentialAsync(CancellationToken cancellationToken) + { + var request = _ctx.Request; + + _ctx.Credential = await _db.Credential.AsNoTracking().Include(c => c.ProviderInstance) + .FirstOrDefaultAsync(c => c.Id == request.CredentialId && c.TeamId == request.TeamId, cancellationToken).ConfigureAwait(false) + ?? throw new KeyNotFoundException($"credential {request.CredentialId} not found"); + + _ctx.PreviousAuthType = _ctx.Credential.AuthType; + + if (_ctx.Credential.ProviderInstanceId != request.ProviderInstanceId) + throw new ArgumentException($"credential {request.CredentialId} belongs to another provider instance than the authorization was obtained from", nameof(request)); + } + + /// + /// A soft-deleted row was retired — its connection removed, its person gone, its token shredded — and so is one whose + /// person has no standing in the team () and which the departed-token sweep retires next. + /// Writing a live token into either would hand the team a token of someone without standing, and strand whatever the + /// revival just brought back when the sweep takes it again. + /// + private async Task EnsureNotRetiredAsync(CancellationToken cancellationToken) + { + var credential = _ctx.Credential; + + if (credential.DeletedDate != null) + throw CredentialRetiredException.SoftDeleted(credential.Id, credential.DisplayName); + + if (credential.Ownership != CredentialOwnership.Personal || credential.OwnerUserId is not Guid ownerId) return; + + if (await TeamStanding.HeldAsync(_db, credential.TeamId, ownerId, cancellationToken).ConfigureAwait(false)) return; + + throw Retired("the person it belongs to no longer has standing in the team, so the departed-token sweep would retire it again."); + } + + private CredentialRetiredException Retired(string reason) => new(_ctx.Credential.Id, _ctx.Credential.DisplayName, reason); + + /// Sealed before anything is asked of the provider or any lock is held, so a key ring that cannot seal fails here and not with the lock held. + private void SealNewPayload() => _ctx.NewEncryptedPayload = _encryptor.Encrypt(_serializer.Serialize(_ctx.Request.Payload)); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Probe.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Probe.cs new file mode 100644 index 000000000..d7325c4e6 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Probe.cs @@ -0,0 +1,102 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Providers.Capabilities; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using Microsoft.EntityFrameworkCore; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed partial class CredentialReauthorizer +{ + /// + /// Asks the provider whose account the new token is — and, for a pasted token, whether it works at all, and when it + /// expires. Probed as a row of its own that exists nowhere: the credential being replaced still holds the old pair, and an + /// auth strategy that finds the new token near its expiry would otherwise refresh THAT row's stored pair with the old + /// refresh token. Asked within the probe deadline the sweep and the add keep (), + /// so a provider that stops answering costs this request its deadline and not the thread it holds. + /// + private async Task ProbeNewAuthorizationAsync(CancellationToken cancellationToken) + { + if (!_registry.TryGet(_ctx.Instance.Provider, out _)) return; + + try + { + _ctx.Profile = await _probe.ProbeAsync(_ctx.Instance, TransientCredentialHolding(_ctx.NewEncryptedPayload), cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (!cancellationToken.IsCancellationRequested) + { + _ctx.Profile = new CredentialProbeResult { IsValid = false, Error = ex.Message }; + } + } + + private Credential TransientCredentialHolding(string encryptedPayload) => new() + { + Id = Guid.NewGuid(), + TeamId = _ctx.Credential.TeamId, + ProviderInstanceId = _ctx.Credential.ProviderInstanceId, + OwnerUserId = _ctx.Credential.OwnerUserId, + Ownership = _ctx.Credential.Ownership, + AuthType = _ctx.Request.Payload.Type, + DisplayName = _ctx.Credential.DisplayName, + EncryptedPayload = encryptedPayload, + Status = CredentialStatus.Active + }; + + /// + /// A token the person pasted must work. One the provider just issued for the code the person approved is presumed to, + /// so a probe that could not run does not refuse it — unless the account has to be compared (). + /// + private void EnsureNewAuthorizationIsUsable() + { + if (_ctx.Request.Payload is OAuthPayload || _ctx.Profile is { IsValid: true }) return; + + throw new InvalidOperationException($"Could not validate the token against {_ctx.Instance.Provider}: {_ctx.Profile?.Error ?? "this provider cannot check a token"}"); + } + + /// + /// The account a credential was connected as is remembered by the act-as identity that names it — which a Disconnect + /// soft-deletes but keeps. An identity is one per (person, provider instance), though, and every connect re-points it at + /// the newest credential, so an older credential of the same person — a token linked first and OAuth connected after, the + /// pair the Providers modal itself produces — is named by nothing. The product's own model is one account per person per + /// instance, so the account on record for it is the one the person's identity speaks for there: the identity that names + /// this credential if any does, else the live one, else the newest that ended. A team-service credential is no person's + /// account, and a person who never linked an identity has none on record: nothing to compare, and the identity this + /// re-authorization links is what records it from now on. + /// + private async Task LoadKnownAccountAsync(CancellationToken cancellationToken) + { + var credential = _ctx.Credential; + + if (credential.Ownership != CredentialOwnership.Personal || credential.OwnerUserId is not Guid ownerId) return; + + var identity = await _db.UserProviderIdentity.AsNoTracking() + .Where(i => i.CredentialId == credential.Id || (i.UserId == ownerId && i.ProviderInstanceId == credential.ProviderInstanceId)) + .OrderByDescending(i => i.CredentialId == credential.Id) + .ThenByDescending(i => i.DeletedDate == null) + .ThenByDescending(i => i.CreatedDate) + .FirstOrDefaultAsync(cancellationToken).ConfigureAwait(false); + + if (identity != null) _ctx.Known = new KnownAccount(identity.ProviderUserId, identity.ProviderUsername); + } + + /// + /// Another account is a new connection, not a re-authorization: everything bound through this credential acts as the + /// account it was connected as. A provider that cannot say which account signed in cannot be compared, so when an account + /// is on record the re-authorization waits for an answer rather than guess. + /// + private void EnsureSameProviderAccount() + { + if (_ctx.Known is not { } known) return; + + if (_ctx.Profile is not { IsValid: true } profile) + throw new InvalidOperationException($"Could not confirm which {_ctx.Instance.Provider} account this authorization belongs to ({_ctx.Profile?.Error ?? "the provider gave no answer"}). Nothing was changed; try again."); + + if (Compare(known.ProviderUserId, known.ProviderUsername, profile.AuthenticatedUserExternalId, profile.AuthenticatedUserName) != AccountMatch.Different) return; + + throw new CredentialAccountMismatchException(_ctx.Credential.Id, _ctx.Credential.DisplayName, Name(known.ProviderUsername, known.ProviderUserId), Name(profile.AuthenticatedUserName, profile.AuthenticatedUserExternalId)); + } + + private static string Name(string? username, string? externalId) => !string.IsNullOrWhiteSpace(username) ? username : externalId ?? "an unnamed account"; +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Revoke.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Revoke.cs new file mode 100644 index 000000000..0addbc8aa --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Revoke.cs @@ -0,0 +1,64 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Credentials; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed partial class CredentialReauthorizer +{ + /// + /// Once the unit of work commits, the tokens this re-authorization replaced are ended at the provider: leaving a live + /// access token and a refresh token behind that nothing in CodeSpace holds any longer is a credential nobody can revoke. + /// Only those tokens — never the user's whole grant, which on GitHub also carries their tokens in every other team + /// connected through the same OAuth App and would end the new token as well — and never a token the new authorization + /// shares. Best effort: the new token is stored and the old one is gone from the row whatever the provider answers. + /// + /// After the commit, not at the write: a provider call inside the unit of work would hold what it holds — the + /// owner's membership, for one — for as long as the provider takes to answer. The cost is the rare unit of work that fails + /// after the write landed: the person repeats the re-authorization, which ends the pair that one wrote, and the pair + /// the failed one replaced is left to expire at the provider. + /// + private async Task RevokeReplacedTokensAfterCommitAsync(CancellationToken cancellationToken) + { + var replaced = ReplacedPairToRevoke(ReadReplacedPayload(), _ctx.Request.Payload); + + if (replaced == null || string.IsNullOrWhiteSpace(_ctx.Instance.OauthClientSecretEnc)) return; + + var instance = _ctx.Instance; + var clientSecret = _encryptor.Decrypt(instance.OauthClientSecretEnc); + var credentialId = _ctx.Credential.Id; + + await _postCommit.RunAfterCommitAsync(ct => RevokeAtProviderBestEffortAsync(instance, clientSecret, replaced, credentialId, ct), cancellationToken).ConfigureAwait(false); + } + + private async Task RevokeAtProviderBestEffortAsync(ProviderInstance instance, string clientSecret, OAuthPayload replaced, Guid credentialId, CancellationToken cancellationToken) + { + try + { + await _grants.RevokeAsync(instance, clientSecret, replaced, wholeGrant: false, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (!cancellationToken.IsCancellationRequested) + { + _logger.LogWarning(ex, "Could not revoke at the provider the tokens credential {CredentialId} held before it was re-authorized; the credential holds the new ones", credentialId); + } + } + + /// What the row held when it was replaced, read as the type it was then. A blank payload is a Disconnect that already ended its tokens; one that does not open leaves nothing to revoke with. + private CredentialPayload? ReadReplacedPayload() + { + if (string.IsNullOrEmpty(_ctx.ReplacedEncryptedPayload)) return null; + + try + { + return _serializer.Deserialize(_ctx.PreviousAuthType, _encryptor.Decrypt(_ctx.ReplacedEncryptedPayload)); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Could not read the payload credential {CredentialId} held before it was re-authorized, so its tokens are not revoked at the provider", _ctx.Credential.Id); + + return null; + } + } + + private CredentialReauthorizationResult BuildResult() => new() { CredentialId = _ctx.Credential.Id, ProviderUsername = _ctx.Profile?.AuthenticatedUserName ?? _ctx.Known?.ProviderUsername }; +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Write.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Write.cs new file mode 100644 index 000000000..1fa075762 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.Write.cs @@ -0,0 +1,88 @@ +using CodeSpace.Core.Services.OAuth; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.Credentials; + +public sealed partial class CredentialReauthorizer +{ + /// How many times the write may find the row changed — a Disconnect blanking it — and read it again, before the person is told to retry. + private const int MaxReplaceRounds = 3; + + /// + /// The one durable write, under the lock a refresh takes — the in-process semaphore, then the advisory lock. Holding it, + /// no refresher is between asking the provider and storing its answer, so none can rotate the pair being replaced or put + /// a rotated pair over the new one; and the row is read AFTER the lock, because what it held before is stale by + /// definition. The write is a compare-and-swap on that payload: a Disconnect does not take the lock and may blank the + /// row meanwhile, which is read again, and a retirement refuses. The caller's cancellation is honoured while waiting for + /// the lock and no longer once it is held — the authorization code is spent, and the token is the only copy of it. A pair + /// this process kept because it could not store a rotation of the pair just replaced is dropped with it, under the same + /// lock (): it belongs to the grant that no longer is the credential's. + /// + private async Task ReplaceUnderRefreshLockAsync(CancellationToken cancellationToken) + { + await using var held = await CredentialRefreshLock.AcquireAsync(_crossProcessLock, _ctx.Credential.Id, cancellationToken).ConfigureAwait(false); + + for (var round = 0; round < MaxReplaceRounds; round++) + { + _ctx.ReplacedEncryptedPayload = await _payloadReader.ReadEncryptedPayloadAsync(_ctx.Credential.Id, CancellationToken.None).ConfigureAwait(false); + + if (await _writer.TryReplaceAsync(BuildReplacement(), CancellationToken.None).ConfigureAwait(false)) + { + MarkReauthorized(); + _keptPairs.Discard(_ctx.Credential.Id); + + return; + } + + await EnsureNotRetiredNowAsync().ConfigureAwait(false); + } + + throw new InvalidOperationException($"The credential '{_ctx.Credential.DisplayName}' kept changing while its new authorization was being stored, so nothing was changed. Try again."); + } + + private CredentialReplacement BuildReplacement() => new() + { + CredentialId = _ctx.Credential.Id, + ExpectedEncryptedPayload = _ctx.ReplacedEncryptedPayload, + EncryptedPayload = _ctx.NewEncryptedPayload, + AuthType = _ctx.Request.Payload.Type, + Scopes = _ctx.Request.Scopes ?? _ctx.Profile?.GrantedScopes, + ExpiresDate = ExpiryOfNewAuthorization(), + DisplayName = string.IsNullOrWhiteSpace(_ctx.Request.DisplayName) ? null : _ctx.Request.DisplayName, + ModifiedBy = _ctx.Request.ActorUserId + }; + + /// + /// The expiry the row is written with: an OAuth pair's is its access token's; a static token's is the one its provider just + /// reported for it (), so the health the list derives is of THIS + /// token — an expired one replaced by a good one reads fine at once, and a new one that expires soon says so. When the + /// provider did not describe the token the expiry is unknown, which is null, and never the replaced token's. + /// + private DateTimeOffset? ExpiryOfNewAuthorization() => _ctx.Request.Payload is OAuthPayload oauth + ? oauth.ExpiresAt + : CredentialHealthDerivation.RecordedExpiry(_ctx.Request.Payload.Type, _ctx.Profile)?.ExpiresAt; + + /// A write that found nothing to change either lost to a retirement, which is the answer, or to a row that is merely different now, which is read again. + private async Task EnsureNotRetiredNowAsync() + { + if (await _payloadReader.IsRetiredAsync(_ctx.Credential.Id, CancellationToken.None).ConfigureAwait(false)) + throw Retired("it was retired while its new authorization was being stored."); + } + + /// Brings the untracked copy up to what the row now holds, for the steps that hand it to the identity link. + private void MarkReauthorized() + { + var replacement = BuildReplacement(); + var credential = _ctx.Credential; + + credential.EncryptedPayload = replacement.EncryptedPayload; + credential.AuthType = replacement.AuthType; + credential.Scopes = replacement.Scopes?.ToList(); + credential.ExpiresDate = replacement.ExpiresDate; + credential.DisplayName = replacement.DisplayName ?? credential.DisplayName; + credential.Status = CredentialStatus.Active; + credential.LastError = null; + credential.LastValidatedDate = DateTimeOffset.UtcNow; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.cs new file mode 100644 index 000000000..a4d86be5a --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialReauthorizer.cs @@ -0,0 +1,74 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Middlewares.Transactional; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.Core.Services.Providers.Capabilities; +using CodeSpace.Core.Services.Providers.Identity; +using CodeSpace.Messages.Credentials; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Credentials; + +/// +/// The pipeline of : each step enriches _ctx or throws the exception that names +/// why nothing was changed. Nothing is written until the new authorization has been sealed, probed and compared with the +/// account the credential belongs to; the one durable write happens under the refresh lock; what follows is tracked. +/// +public sealed partial class CredentialReauthorizer : ICredentialReauthorizer, IScopedDependency +{ + private readonly CodeSpaceDbContext _db; + private readonly IProviderRegistry _registry; + private readonly IBoundedCredentialProbe _probe; + private readonly IPayloadEncryptor _encryptor; + private readonly ICredentialPayloadSerializer _serializer; + private readonly ICredentialPayloadReader _payloadReader; + private readonly ICrossProcessLock _crossProcessLock; + private readonly IKeptOAuthPairs _keptPairs; + private readonly ICredentialReauthorizationWriter _writer; + private readonly ICredentialBindingReviver _bindings; + private readonly IUserProviderIdentityService _identities; + private readonly IOAuthGrantRevoker _grants; + private readonly IPostCommitActions _postCommit; + private readonly ILogger _logger; + + private ReauthorizationContext _ctx = default!; + + public CredentialReauthorizer(CodeSpaceDbContext db, IProviderRegistry registry, IBoundedCredentialProbe probe, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, ICredentialPayloadReader payloadReader, ICrossProcessLock crossProcessLock, IKeptOAuthPairs keptPairs, ICredentialReauthorizationWriter writer, ICredentialBindingReviver bindings, IUserProviderIdentityService identities, IOAuthGrantRevoker grants, IPostCommitActions postCommit, ILogger logger) + { + _db = db; + _registry = registry; + _probe = probe; + _encryptor = encryptor; + _serializer = serializer; + _payloadReader = payloadReader; + _crossProcessLock = crossProcessLock; + _keptPairs = keptPairs; + _writer = writer; + _bindings = bindings; + _identities = identities; + _grants = grants; + _postCommit = postCommit; + _logger = logger; + } + + public async Task ReauthorizeAsync(CredentialReauthorization reauthorization, CancellationToken cancellationToken) + { + _ctx = BuildContext(reauthorization); + + await LoadCredentialAsync(cancellationToken).ConfigureAwait(false); // KeyNotFoundException, ArgumentException + await EnsureNotRetiredAsync(cancellationToken).ConfigureAwait(false); // CredentialRetiredException + SealNewPayload(); + + await ProbeNewAuthorizationAsync(cancellationToken).ConfigureAwait(false); + EnsureNewAuthorizationIsUsable(); // InvalidOperationException + await LoadKnownAccountAsync(cancellationToken).ConfigureAwait(false); + EnsureSameProviderAccount(); // CredentialAccountMismatchException + + await ReplaceUnderRefreshLockAsync(cancellationToken).ConfigureAwait(false); // CredentialRetiredException + await ReviveBindingsAsync(cancellationToken).ConfigureAwait(false); + await RelinkActorIdentityBestEffortAsync(cancellationToken).ConfigureAwait(false); + await RevokeReplacedTokensAfterCommitAsync(cancellationToken).ConfigureAwait(false); + + return BuildResult(); + } +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.ReplaceToken.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.ReplaceToken.cs new file mode 100644 index 000000000..da0340a2b --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.ReplaceToken.cs @@ -0,0 +1,67 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using Microsoft.EntityFrameworkCore; + +namespace CodeSpace.Core.Services.Credentials; + +/// Replacing a credential's token in place — the pasted-token door onto . +public sealed partial class CredentialService +{ + public async Task ReplaceTokenAsync(Guid credentialId, string token, CancellationToken cancellationToken) + { + if (string.IsNullOrWhiteSpace(token)) + throw new ArgumentException("token must not be empty", nameof(token)); + + var credential = await LoadTeamCredentialAsync(credentialId, cancellationToken).ConfigureAwait(false); + + await HoldOwnerInTeamAsync(credential, cancellationToken).ConfigureAwait(false); + + var result = await _reauthorizer.ReauthorizeAsync(new CredentialReauthorization + { + CredentialId = credential.Id, + TeamId = credential.TeamId, + ProviderInstanceId = credential.ProviderInstanceId, + Payload = StaticTokenPayload(credential, token.Trim()), + ActorUserId = _currentUser.Id!.Value + }, cancellationToken).ConfigureAwait(false); + + return new ReplaceCredentialTokenResult { CredentialId = result.CredentialId, ProviderUsername = result.ProviderUsername }; + } + + /// + /// By (id, the header team), whatever its state: the authorization marker already refuses a credential of another team, + /// but not for an instance Admin, who passes it by role — so the team the caller works in is checked here too. A retired + /// credential is found, and refused by the re-authorization with the answer it deserves. + /// + private async Task LoadTeamCredentialAsync(Guid credentialId, CancellationToken cancellationToken) => + await _db.Credential.AsNoTracking().FirstOrDefaultAsync(c => c.Id == credentialId && c.TeamId == _currentTeam.Id!.Value, cancellationToken).ConfigureAwait(false) + ?? throw new KeyNotFoundException($"credential {credentialId} not found"); + + /// + /// A person's token is theirs while they stand in the team, and the standing is held until this unit of work commits: a + /// departure that starts now waits and then retires the new token with the rest of theirs, as it does for a link or a + /// connect. A team-service credential has no person to ask about. + /// + private async Task HoldOwnerInTeamAsync(Credential credential, CancellationToken cancellationToken) + { + if (credential.Ownership != CredentialOwnership.Personal || credential.OwnerUserId is not Guid ownerId) return; + + if (!await _members.HoldMembershipAsync(credential.TeamId, ownerId, cancellationToken).ConfigureAwait(false)) + throw new CredentialRetiredException(credential.Id, credential.DisplayName, "the person it belongs to no longer has standing in the team."); + } + + /// + /// The payload a pasted token becomes: another of the kind the row already is when it is a static token — a group token + /// stays a group token — and a personal access token when it replaces an OAuth sign-in. A key or a login is not a token. + /// + private static CredentialPayload StaticTokenPayload(Credential credential, string token) => credential.AuthType switch + { + AuthType.OAuth or AuthType.Pat => new PatPayload { Token = token }, + AuthType.ProjectAccessToken => new ProjectAccessTokenPayload { Token = token }, + AuthType.GroupAccessToken => new GroupAccessTokenPayload { Token = token }, + _ => throw new ArgumentException($"credential {credential.Id} is a {credential.AuthType} credential, which a pasted token cannot replace", nameof(credential)) + }; +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs index fbc7537cb..02c4edfdf 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs @@ -33,10 +33,11 @@ public sealed partial class CredentialService : ICredentialService, IScopedDepen private readonly ICredentialSuccessionService _succession; private readonly ICredentialRevocationService _revocation; private readonly ICredentialHealthService _health; + private readonly ICredentialReauthorizer _reauthorizer; private readonly ITeamMemberService _members; private readonly ILogger _logger; - public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IOAuthGrantRevoker grants, IProviderRegistry registry, IProviderModuleCatalog modules, IScopeChecker scopeChecker, ICredentialSuccessionService succession, ICredentialRevocationService revocation, ICredentialHealthService health, ITeamMemberService members, ILogger logger) + public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IOAuthGrantRevoker grants, IProviderRegistry registry, IProviderModuleCatalog modules, IScopeChecker scopeChecker, ICredentialSuccessionService succession, ICredentialRevocationService revocation, ICredentialHealthService health, ICredentialReauthorizer reauthorizer, ITeamMemberService members, ILogger logger) { _db = db; _currentTeam = currentTeam; @@ -50,6 +51,7 @@ public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurre _succession = succession; _revocation = revocation; _health = health; + _reauthorizer = reauthorizer; _members = members; _logger = logger; } diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.Revive.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.Revive.cs new file mode 100644 index 000000000..4b11ea925 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.Revive.cs @@ -0,0 +1,26 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Dtos.Credentials; +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.Credentials; + +/// The same lift-the-error and revive-the-hooks an adoption performs, for a credential that stays where it is. +public sealed partial class CredentialSuccessionService : ICredentialBindingReviver +{ + public async Task ReviveBindingsAsync(Credential credential, CancellationToken cancellationToken) + { + var own = new List { credential.Id }; + + var repositories = await LoadBoundRepositoriesAsync(credential, own, cancellationToken).ConfigureAwait(false); + var hooks = await LoadBoundConnectionWebhooksAsync(credential, own, cancellationToken).ConfigureAwait(false); + + var revived = new BindingsRevived { RepositoryCount = repositories.Count(r => r.Status == RepositoryStatus.Error), ConnectionWebhookCount = hooks.Count(h => Revivable.Contains(h.RegistrationStatus)) }; + + foreach (var repository in repositories) Revive(repository); + foreach (var hook in hooks) Revive(hook); + + await DispatchAfterCommitAsync(hooks.Where(h => h.RegistrationStatus == RepositoryWebhookRegistrationStatus.Pending).Select(h => h.Id).ToList(), cancellationToken).ConfigureAwait(false); + + return revived; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.cs index 298910a36..464958a1a 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.cs @@ -170,26 +170,38 @@ await _db.ConnectionWebhook .Where(w => w.ProviderInstanceId == successor.ProviderInstanceId && predecessorIds.Contains(w.CredentialId) && WebhookRegistrationLifecycle.InService.Contains(w.RegistrationStatus)) .ToListAsync(cancellationToken).ConfigureAwait(false); - /// The inverse of the disconnect cascade: bind the live credential and lift the Error it set. Any other status (Paused, Unreachable) is the repository's own and stays. + /// Bind the live credential, and lift the Error the dead one's disconnect set (). private static void Adopt(Repository repository, Credential successor) { repository.CredentialId = successor.Id; + Revive(repository); + } + + /// The inverse of the disconnect cascade: lift the Error it set. Any other status (Paused, Unreachable) is the repository's own and stays. + private static void Revive(Repository repository) + { if (repository.Status != RepositoryStatus.Error) return; repository.Status = RepositoryStatus.Active; repository.LastError = null; } - /// - /// Bind the live credential, and revive a hook the dead one failed — attempts reset, due now — exactly as a bind - /// revives it: a reconnect is the operator intervention that fixes what buried it. Registered, Enqueued and - /// Registering keep their state; they only need a live credential for their next call. - /// + /// Bind the live credential, and revive a hook the dead one failed (). private static void Adopt(ConnectionWebhook hook, Credential successor) { hook.CredentialId = successor.Id; + Revive(hook); + } + + /// + /// Revive a hook a dead credential failed — attempts reset, due now — exactly as a bind revives it: a reconnect is the + /// operator intervention that fixes what buried it. Registered, Enqueued and Registering keep their state; they only + /// need a live credential for their next call. + /// + private static void Revive(ConnectionWebhook hook) + { if (!Revivable.Contains(hook.RegistrationStatus)) return; hook.RegistrationStatus = RepositoryWebhookRegistrationStatus.Pending; diff --git a/backend/src/CodeSpace.Core/Services/Credentials/IBoundedCredentialProbe.cs b/backend/src/CodeSpace.Core/Services/Credentials/IBoundedCredentialProbe.cs new file mode 100644 index 000000000..ff37e3aef --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/IBoundedCredentialProbe.cs @@ -0,0 +1,21 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Dtos.Providers; + +namespace CodeSpace.Core.Services.Credentials; + +/// +/// Asks a provider about a credential within the probe deadline (). +/// A sibling of , which keeps recorded health in step with it: whatever must ask about a +/// token BEFORE it is stored — the one a person pasted, the pair a sign-in was just issued — is bounded exactly as the sweep and +/// the add are, without having any use for the sweep. +/// +public interface IBoundedCredentialProbe +{ + /// + /// What the provider says about right now. A provider that does not answer in time is "no + /// answer" ( false, not rejected), however it ignores the caller's token. Nothing + /// is read from or written to the database, and the credential may be a copy that exists nowhere. Its Status is set to + /// Active on the instance given, so that a credential recorded as Error is asked as if it were not: pass one that is never saved. + /// + Task ProbeAsync(ProviderInstance instance, Credential credential, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/ICredentialBindingReviver.cs b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialBindingReviver.cs new file mode 100644 index 000000000..c7e6f4857 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialBindingReviver.cs @@ -0,0 +1,21 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Dtos.Credentials; + +namespace CodeSpace.Core.Services.Credentials; + +/// +/// What a credential coming back to life in place brings back with it. Re-authorizing a credential keeps its id, so nothing +/// bound through it moves; what changes is that the repositories its dead token had turned Error work again, and the group +/// hooks its failures had buried are tried again. A sibling of rather than another +/// member of it: that one moves bindings between credentials, and nothing moves here. +/// +public interface ICredentialBindingReviver +{ + /// + /// Lift the "needs a new credential" error from every live repository bound through and + /// revive every in-service group hook its failures buried (attempts reset, due now), handing them to the registrar once + /// the caller's unit of work commits — exactly as a bind or a reconnect revives them. A repository that is Paused or + /// Unreachable keeps that: it is its own state, not the credential's. Tracked changes only. Returns what came back. + /// + Task ReviveBindingsAsync(Credential credential, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/ICredentialReauthorizationWriter.cs b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialReauthorizationWriter.cs new file mode 100644 index 000000000..929a32819 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialReauthorizationWriter.cs @@ -0,0 +1,20 @@ +using CodeSpace.Messages.Credentials; + +namespace CodeSpace.Core.Services.Credentials; + +/// +/// Writes a re-authorization into the credential row — the payload and everything that says the credential is alive again — +/// in one compare-and-swap statement on a connection of its own, like ICredentialPayloadWriter does for a refresh and +/// for the same reason: the new token comes from a one-time authorization code the provider has already spent, so it must +/// not be lost to a rollback of whatever unit of work the caller is in. +/// +public interface ICredentialReauthorizationWriter +{ + /// + /// Replaces the row's payload, auth type, scopes and expiry, sets it Active with its last error cleared and + /// LastValidatedDate now, and stamps the modifier — but only while the row is live and still holds + /// . Returns whether it did. False means the row changed + /// under the caller (a Disconnect blanked it; a retirement soft-deleted it), and nothing was written. + /// + Task TryReplaceAsync(CredentialReplacement replacement, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/ICredentialReauthorizer.cs b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialReauthorizer.cs new file mode 100644 index 000000000..5520e5fa8 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialReauthorizer.cs @@ -0,0 +1,33 @@ +using CodeSpace.Messages.Credentials; + +namespace CodeSpace.Core.Services.Credentials; + +/// +/// Puts a new authorization into a credential row that already exists. Everything that references a credential — +/// repository.credential_id, connection_webhook.credential_id, user_provider_identity.credential_id — +/// references its id, and a reconnect used to mint a new row, so a whole succession subsystem had to move those bindings +/// afterwards, with edge cases for every shape of owner, instance and timing. Writing the new token into the SAME row removes +/// the cause: nothing moves, so nothing can be left behind. +/// +/// The row is rewritten under the lock a refresh takes, so a refresher cannot rotate the pair being replaced while it +/// is replaced, nor write its rotated pair over the new one. The new authorization must belong to the same provider account +/// the credential was connected as; one that does not is a different connection and is refused. A credential retired as its +/// person left the team or its connection was removed stays retired. +/// +/// It is not a connect: no row is created, so what a connect does about the owner's other dead credentials — carry their +/// bindings to the new one — does not happen here. A verdict about the replaced token that lands later (a sweep's, a +/// refresher's) is a compare-and-swap on the payload it judged, which the row no longer holds, so it changes nothing; and a +/// pair this process kept for the grant that was replaced is dropped under the same lock, never stored over the new one. +/// +public interface ICredentialReauthorizer +{ + /// + /// Replaces the credential's token, auth type, scopes, expiry (a static token's as its provider reports it, so the health the + /// list derives is of the NEW token) and (when given) display name; sets it Active with its last error cleared — a credential + /// its provider had refused included — and LastValidatedDate now; revives the repositories and group hooks its dead + /// token had broken; relinks the owner's act-as identity; and, once the caller's unit of work commits, asks the provider to + /// revoke the tokens that were replaced — those tokens only, never the user's grant. The row's write is durable at once, on a + /// connection of its own; the rest are tracked changes the caller's unit of work saves. + /// + Task ReauthorizeAsync(CredentialReauthorization reauthorization, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/ICredentialService.cs b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialService.cs index e3a6b12cf..84c5ee1b3 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/ICredentialService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/ICredentialService.cs @@ -30,5 +30,15 @@ public interface ICredentialService /// the provider is asked to revoke anything. /// Task RevokeAsync(Guid credentialId, Guid? moveBindingsToCredentialId, CancellationToken cancellationToken); + + /// + /// Put a new token into the credential, in place: it keeps its id, so everything bound through it keeps working and what + /// its dead token had broken comes back. The token is probed first and must be the same provider account's + /// (). Any credential this caller manages, whatever it was before: a token replaces an + /// OAuth sign-in as readily as another token, and the row's auth type follows. Tracked changes beyond the row's own write — + /// the caller's unit of work saves them. + /// + Task ReplaceTokenAsync(Guid credentialId, string token, CancellationToken cancellationToken); + Task ListAccessibleRepositoriesAsync(Guid credentialId, string? search, int page, int perPage, CancellationToken cancellationToken); } diff --git a/backend/src/CodeSpace.Core/Services/Credentials/RetiredSecretShredService.cs b/backend/src/CodeSpace.Core/Services/Credentials/RetiredSecretShredService.cs index 63174a2f7..94d0d860a 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/RetiredSecretShredService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/RetiredSecretShredService.cs @@ -12,9 +12,14 @@ namespace CodeSpace.Core.Services.Credentials; /// for kind and predicate for predicate, run as one conditional UPDATE per kind. Each predicate is the row's retirement — /// deleted, Revoked, or no longer named — so a live secret is never a candidate, and each matches the relaxed guard that /// admits the blanking: a revision only once its credential is Revoked, an envelope only while no template names it, a -/// Secret variable's NULL only once it is deleted. Each retirement is terminal — nothing restores a deleted row, revives a -/// Revoked one, or names an envelope again — so a row once a candidate stays one; Postgres re-checks a row's own predicate -/// when a concurrent writer changed it first, and the guards re-check theirs as each row is written. +/// Secret variable's NULL only once it is deleted. Each retirement is terminal — nothing restores a deleted row or names an +/// envelope again — so a row once a candidate stays one; Postgres re-checks a row's own predicate when a concurrent writer +/// changed it first, and the guards re-check theirs as each row is written. +/// +/// One Revoked row does come back: a git credential its person Disconnected and, not deleted, later re-authorized in +/// place (). It was never a candidate while Revoked — a Disconnect blanks the payload, +/// and only a row still holding one is — and the write that revives it sets it Active in the same statement that gives it +/// a token, so it is not one afterwards. A deleted credential is never revived. /// /// What it does not do is what 0249 does not do: a connection an older build deleted keeps its group hooks registered /// at the provider — they are Cancelled here with their secrets blanked, refused at ingestion, and removed at the provider diff --git a/backend/src/CodeSpace.Core/Services/OAuth/CredentialRefreshLock.cs b/backend/src/CodeSpace.Core/Services/OAuth/CredentialRefreshLock.cs new file mode 100644 index 000000000..e5c8aae6a --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OAuth/CredentialRefreshLock.cs @@ -0,0 +1,70 @@ +using System.Collections.Concurrent; + +namespace CodeSpace.Core.Services.OAuth; + +/// +/// The lock everything that rewrites one credential's token material takes, so two of them never write at once: a refresh +/// () and a re-authorization (ICredentialReauthorizer) are the same kind of act — +/// each replaces the stored pair, and each is only sound against the pair it read. Two tiers, always taken in this order: +/// 1. a per-credential in-process — cheap, and blocks every sibling in this process; +/// 2. the (a Postgres advisory lock) — blocks every other pod. +/// One definition, because the semaphores are process-wide state: a second dictionary beside this one would be a second lock +/// that excludes nobody. +/// +internal static class CredentialRefreshLock +{ + private static readonly ConcurrentDictionary InProcess = new(); + + /// + /// Stable int64 derived from the credential GUID. pg_advisory_lock takes int8 (bigint). + /// We take the first 8 bytes of the GUID — uniform distribution, collision probability + /// is the same as a 64-bit random key. Same credential id always maps to same key. + /// + internal static long KeyOf(Guid credentialId) => BitConverter.ToInt64(credentialId.ToByteArray(), 0); + + /// Waits for both tiers. Disposing the handle releases the advisory lock first and the semaphore after it. + internal static async Task AcquireAsync(ICrossProcessLock crossProcess, Guid credentialId, CancellationToken cancellationToken) + { + var semaphore = InProcess.GetOrAdd(credentialId, _ => new SemaphoreSlim(1, 1)); + + await semaphore.WaitAsync(cancellationToken).ConfigureAwait(false); + + try + { + return new Held(semaphore, await crossProcess.AcquireAsync(KeyOf(credentialId), cancellationToken).ConfigureAwait(false)); + } + catch + { + semaphore.Release(); + throw; + } + } + + private sealed class Held : IAsyncDisposable + { + private readonly SemaphoreSlim _semaphore; + private readonly IAsyncDisposable _crossProcess; + private bool _released; + + public Held(SemaphoreSlim semaphore, IAsyncDisposable crossProcess) + { + _semaphore = semaphore; + _crossProcess = crossProcess; + } + + public async ValueTask DisposeAsync() + { + if (_released) return; + _released = true; + + try + { + await _crossProcess.DisposeAsync().ConfigureAwait(false); + } + finally + { + _semaphore.Release(); + } + } + } +} diff --git a/backend/src/CodeSpace.Core/Services/OAuth/IKeptOAuthPairs.cs b/backend/src/CodeSpace.Core/Services/OAuth/IKeptOAuthPairs.cs new file mode 100644 index 000000000..98fda4bf9 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OAuth/IKeptOAuthPairs.cs @@ -0,0 +1,18 @@ +namespace CodeSpace.Core.Services.OAuth; + +/// +/// The pairs a process kept because it could not store them (), seen from +/// the one place that makes them obsolete: a re-authorization, which replaces a credential's token material wholesale. A pair +/// the provider rotated for the grant that was replaced must never be stored over the new authorization — a later store +/// settles a row that changed meanwhile by which pair expires later, and a new authorization is not always the later one. +/// A sibling of rather than a member: nothing that recovers a refused token has any use for it. +/// +public interface IKeptOAuthPairs +{ + /// + /// Forgets the pair this process kept for the credential, if it kept one. Called by the re-authorization while it holds + /// the credential's refresh locks, which is also what every use of a kept pair holds, so none is half-way through + /// storing it. Another process's kept pair is not reachable from here; its store is decided by the row it finds. + /// + void Discard(Guid credentialId); +} diff --git a/backend/src/CodeSpace.Core/Services/OAuth/IOAuthFlowService.cs b/backend/src/CodeSpace.Core/Services/OAuth/IOAuthFlowService.cs index 2e18987e1..1c16f90a6 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/IOAuthFlowService.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/IOAuthFlowService.cs @@ -8,7 +8,9 @@ namespace CodeSpace.Core.Services.OAuth; /// provider's authorize URL the SPA opens in a popup. /// 2. runs from the callback redirect — anonymous, with the /// state token and the starting browser's nonce as proof — consumes the state, -/// exchanges the auth code, and persists a Credential. Without the nonce it consumes +/// exchanges the auth code, and persists a Credential — or, when the connect named a +/// credential to replace (), +/// writes the token into that credential in place. Without the nonce it consumes /// nothing and hands the callback to the SPA's completion page instead. /// 3. is that page finishing it: the same consume, /// exchange and persist, with the signed-in caller — who must have started the @@ -16,7 +18,7 @@ namespace CodeSpace.Core.Services.OAuth; /// public interface IOAuthFlowService { - Task InitAsync(Guid providerInstanceId, string displayName, Guid? intendedOwnerUserId, string? returnUrl, IReadOnlyList? scopes, CancellationToken cancellationToken); + Task InitAsync(InitCredentialOAuthCommand command, CancellationToken cancellationToken); Task CompleteAsync(string state, string code, string? browserNonce, CancellationToken cancellationToken); Task CompleteAsInitiatorAsync(string state, string code, CancellationToken cancellationToken); } diff --git a/backend/src/CodeSpace.Core/Services/OAuth/IOAuthStateStore.cs b/backend/src/CodeSpace.Core/Services/OAuth/IOAuthStateStore.cs index c0edbb147..8c319769c 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/IOAuthStateStore.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/IOAuthStateStore.cs @@ -38,6 +38,7 @@ public sealed record OAuthPendingStateInput public required string IntendedDisplayName { get; init; } public Guid? IntendedOwnerUserId { get; init; } public string? ReturnUrl { get; init; } + public Guid? ReplaceCredentialId { get; init; } public required string BrowserNonceHash { get; init; } public IReadOnlyList? RequestedScopes { get; init; } public required TimeSpan Ttl { get; init; } diff --git a/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs b/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs index 017ccdd26..780c9b6dc 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs @@ -12,6 +12,7 @@ using CodeSpace.Messages.Commands.Credentials; using CodeSpace.Messages.Credentials; using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; using Microsoft.EntityFrameworkCore; namespace CodeSpace.Core.Services.OAuth; @@ -33,11 +34,12 @@ public sealed class OAuthFlowService : IOAuthFlowService, IScopedDependency private readonly ICurrentUser _currentUser; private readonly IUserProviderIdentityService _identityService; private readonly ICredentialSuccessionService _succession; + private readonly ICredentialReauthorizer _reauthorizer; private readonly ITeamMemberService _members; private readonly OAuthCallbackUrlSetting _callbackUrlSetting; private readonly OAuthReturnOriginsSetting _returnOrigins; - 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, OAuthReturnOriginsSetting returnOrigins) + public OAuthFlowService(CodeSpaceDbContext db, IOAuthClientRegistry oauthClients, IOAuthStateStore stateStore, IPkceGenerator pkce, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, ICurrentTeam currentTeam, ICurrentUser currentUser, IUserProviderIdentityService identityService, ICredentialSuccessionService succession, ICredentialReauthorizer reauthorizer, ITeamMemberService members, OAuthCallbackUrlSetting callbackUrlSetting, OAuthReturnOriginsSetting returnOrigins) { _db = db; _oauthClients = oauthClients; @@ -49,18 +51,20 @@ public OAuthFlowService(CodeSpaceDbContext db, IOAuthClientRegistry oauthClients _currentUser = currentUser; _identityService = identityService; _succession = succession; + _reauthorizer = reauthorizer; _members = members; _callbackUrlSetting = callbackUrlSetting; _returnOrigins = returnOrigins; } - public async Task InitAsync(Guid providerInstanceId, string displayName, Guid? intendedOwnerUserId, string? returnUrl, IReadOnlyList? scopes, CancellationToken cancellationToken) + public async Task InitAsync(InitCredentialOAuthCommand command, CancellationToken cancellationToken) { - EnsureReturnUrlAllowed(returnUrl); + EnsureReturnUrlAllowed(command.ReturnUrl); - var instance = await LoadOwnedInstanceAsync(providerInstanceId, cancellationToken).ConfigureAwait(false); + var instance = await LoadOwnedInstanceAsync(command.ProviderInstanceId, cancellationToken).ConfigureAwait(false); var (clientId, _) = ReadOAuthClientCredentials(instance); - var ownerUserId = await ResolveOwnerWithinReachAsync(intendedOwnerUserId, cancellationToken).ConfigureAwait(false); + var replaced = await LoadReplacedCredentialAsync(command, cancellationToken).ConfigureAwait(false); + var ownerUserId = await ResolveOwnerWithinReachAsync(replaced?.OwnerUserId ?? command.IntendedOwnerUserId, cancellationToken).ConfigureAwait(false); var pkce = _pkce.Generate(); var browserNonce = OAuthBrowserNonce.Mint(); @@ -71,11 +75,12 @@ public async Task InitAsync(Guid providerInstanceId, TeamId = _currentTeam.Id!.Value, InitiatorUserId = _currentUser.Id!.Value, CodeVerifier = pkce.Verifier, - IntendedDisplayName = displayName, + IntendedDisplayName = command.DisplayName, IntendedOwnerUserId = ownerUserId, - ReturnUrl = returnUrl, + ReturnUrl = command.ReturnUrl, + ReplaceCredentialId = replaced?.Id, BrowserNonceHash = OAuthBrowserNonce.Hash(browserNonce), - RequestedScopes = scopes ?? instance.OauthDefaultScopes, + RequestedScopes = command.Scopes ?? instance.OauthDefaultScopes, Ttl = StateTtl }, cancellationToken).ConfigureAwait(false); @@ -122,6 +127,23 @@ private async Task FinishAsync(OAuthPendingState await HoldOwnerInTeamAsync(pending, cancellationToken).ConfigureAwait(false); + var credentialId = pending.ReplaceCredentialId is Guid replacedId + ? await ReauthorizeAsync(pending, replacedId, token, cancellationToken).ConfigureAwait(false) + : await ConnectNewAsync(pending, instance, token, cancellationToken).ConfigureAwait(false); + + return new CompleteCredentialOAuthResult + { + CredentialId = credentialId, + TeamId = pending.TeamId, + // Checked again rather than trusted from init: the configured origins can change while a connect is pending. + ReturnUrl = OAuthReturnUrl.AllowedOrFallback(pending.ReturnUrl, _returnOrigins.Value), + BrowserNonceCookiePath = ResolveCallbackUri().AbsolutePath + }; + } + + /// A plain connect: a new credential, the bindings of the owner's disconnected ones carried onto it, and the owner's act-as identity linked. + private async Task ConnectNewAsync(OAuthPendingState pending, ProviderInstance instance, OAuthTokenResponse token, CancellationToken cancellationToken) + { var credential = PersistCredential(pending, instance, token); await _succession.CarryForwardAsync(credential, cancellationToken).ConfigureAwait(false); @@ -129,14 +151,28 @@ private async Task FinishAsync(OAuthPendingState if (pending.IntendedOwnerUserId is Guid ownerId) await LinkActorIdentityBestEffortAsync(instance, credential, ownerId, cancellationToken).ConfigureAwait(false); - return new CompleteCredentialOAuthResult + return credential.Id; + } + + /// + /// A connect that named the credential it replaces: the token goes into THAT row, so what is bound through it needs no + /// moving. The account that signed in must be the one the credential was connected as, and a credential retired since + /// the connect began is refused — both by the re-authorization itself, which knows the credential. + /// + private async Task ReauthorizeAsync(OAuthPendingState pending, Guid replacedId, OAuthTokenResponse token, CancellationToken cancellationToken) + { + var result = await _reauthorizer.ReauthorizeAsync(new CredentialReauthorization { - CredentialId = credential.Id, + CredentialId = replacedId, TeamId = pending.TeamId, - // Checked again rather than trusted from init: the configured origins can change while a connect is pending. - ReturnUrl = OAuthReturnUrl.AllowedOrFallback(pending.ReturnUrl, _returnOrigins.Value), - BrowserNonceCookiePath = ResolveCallbackUri().AbsolutePath - }; + ProviderInstanceId = pending.ProviderInstanceId, + Payload = PayloadOf(token), + Scopes = token.GrantedScopes?.ToList(), + DisplayName = pending.IntendedDisplayName, + ActorUserId = pending.InitiatorUserId + }, cancellationToken).ConfigureAwait(false); + + return result.CredentialId; } /// @@ -228,6 +264,35 @@ private async Task LoadOwnedInstanceAsync(Guid id, Cancellatio return instance; } + /// + /// The credential a connect re-authorizes in place, or null for a plain connect. Named by id, and a credential id says + /// nothing about its team, so it is looked up by (id, the header team): another team's reads as not found. It must be + /// a person's own connection on this very provider instance — an OAuth sign-in is somebody's account, and writing one into + /// a team-service credential would make a team credential out of a person who can leave — and it is the owner's to + /// re-authorize, by the same reach that connecting for them takes. + /// + private async Task LoadReplacedCredentialAsync(InitCredentialOAuthCommand command, CancellationToken cancellationToken) + { + if (command.ReplaceCredentialId is not Guid replacedId) return null; + + var credential = await _db.Credential.AsNoTracking().FirstOrDefaultAsync(c => c.Id == replacedId && c.TeamId == _currentTeam.Id!.Value, cancellationToken).ConfigureAwait(false) + ?? throw new KeyNotFoundException($"credential {replacedId} not found"); + + if (credential.DeletedDate != null) + throw CredentialRetiredException.SoftDeleted(credential.Id, credential.DisplayName); + + if (credential.ProviderInstanceId != command.ProviderInstanceId) + throw new ArgumentException("replaceCredentialId belongs to another provider instance than providerInstanceId", nameof(command)); + + if (credential.Ownership != CredentialOwnership.Personal || credential.OwnerUserId == null) + throw new ArgumentException("replaceCredentialId must be a person's own connection; replace a team token with a new token instead", nameof(command)); + + if (command.IntendedOwnerUserId is Guid named && named != credential.OwnerUserId) + throw new ArgumentException("intendedOwnerUserId, when given with replaceCredentialId, must be the credential's owner", nameof(command)); + + return credential; + } + // Complete-internals (callback runs anonymous, no current-team check). /// @@ -287,16 +352,16 @@ private async Task ExchangeCodeAsync(ProviderInstance instan }, cancellationToken).ConfigureAwait(false); } - private Credential PersistCredential(OAuthPendingState state, ProviderInstance instance, OAuthTokenResponse token) + private static OAuthPayload PayloadOf(OAuthTokenResponse token) => new() { - var payload = new OAuthPayload - { - AccessToken = token.AccessToken, - RefreshToken = token.RefreshToken, - ExpiresAt = token.ExpiresAt - }; + AccessToken = token.AccessToken, + RefreshToken = token.RefreshToken, + ExpiresAt = token.ExpiresAt + }; - var json = _serializer.Serialize(payload); + private Credential PersistCredential(OAuthPendingState state, ProviderInstance instance, OAuthTokenResponse token) + { + var json = _serializer.Serialize(PayloadOf(token)); var encrypted = _encryptor.Encrypt(json); var credential = new Credential diff --git a/backend/src/CodeSpace.Core/Services/OAuth/OAuthStateStore.cs b/backend/src/CodeSpace.Core/Services/OAuth/OAuthStateStore.cs index 54edbcf3d..f8ca23e91 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/OAuthStateStore.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/OAuthStateStore.cs @@ -25,6 +25,7 @@ public async Task CreateAsync(OAuthPendingStateInput input, C IntendedDisplayName = input.IntendedDisplayName, IntendedOwnerUserId = input.IntendedOwnerUserId, ReturnUrl = input.ReturnUrl, + ReplaceCredentialId = input.ReplaceCredentialId, BrowserNonceHash = input.BrowserNonceHash, RequestedScopes = input.RequestedScopes?.ToList(), ExpiresDate = _clock.GetUtcNow() + input.Ttl diff --git a/backend/src/CodeSpace.Core/Services/OAuth/OAuthTokenRefresher.cs b/backend/src/CodeSpace.Core/Services/OAuth/OAuthTokenRefresher.cs index 54ce77d09..4d1b9f7d1 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/OAuthTokenRefresher.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/OAuthTokenRefresher.cs @@ -17,7 +17,8 @@ public interface IOAuthTokenRefresher } /// -/// Shared refresh logic for any provider's OAuth strategy. Two-tier lock: +/// Shared refresh logic for any provider's OAuth strategy. Two-tier lock (, the same one a +/// re-authorization takes): /// 1. Per-credential in-process — cheap, blocks sibling /// requests in the same process. /// 2. — coordinates across multiple API instances. Held @@ -48,9 +49,8 @@ public interface IOAuthTokenRefresher /// refresh token has marked the credential Error by then, and the fail-fast on that verdict would otherwise stand in /// front of the one live pair. /// -public sealed class OAuthTokenRefresher : IOAuthTokenRefresher, IOAuthTokenRecovery, IScopedDependency +public sealed class OAuthTokenRefresher : IOAuthTokenRefresher, IOAuthTokenRecovery, IKeptOAuthPairs, IScopedDependency { - private static readonly ConcurrentDictionary RefreshLocks = new(); private static readonly TimeSpan RefreshBuffer = TimeSpan.FromMinutes(2); /// Waits between the attempts to store a rotated pair: one store and two retries, each on a connection of its own. @@ -73,7 +73,7 @@ public sealed class OAuthTokenRefresher : IOAuthTokenRefresher, IOAuthTokenRecov /// /// Pairs the provider issued that no attempt could store, by credential: what the row held when the refresh began /// (the compare-and-swap's expected value) and the pair. Only this process knows them; the next use of the credential in - /// it stores them. Refreshes of one credential run one at a time per process, under . + /// it stores them. Refreshes of one credential run one at a time per process, under . /// private static readonly ConcurrentDictionary KeptRotations = new(); @@ -106,19 +106,7 @@ public async Task RefreshIfNeededAsync(ProviderContext context, OA { if (!ShouldRefresh(current)) return current; - var sem = RefreshLocks.GetOrAdd(context.Credential.Id, _ => new SemaphoreSlim(1, 1)); - await sem.WaitAsync(cancellationToken).ConfigureAwait(false); - - try - { - await using var crossLock = await _crossProcessLock.AcquireAsync(CredentialIdToLockKey(context.Credential.Id), cancellationToken).ConfigureAwait(false); - - return await RefreshStoredPayloadAsync(context, cancellationToken).ConfigureAwait(false); - } - finally - { - sem.Release(); - } + return await UnderRefreshLockAsync(context, ct => RefreshStoredPayloadAsync(context, ct), cancellationToken).ConfigureAwait(false); } /// @@ -145,22 +133,18 @@ public async Task StoreKeptPairAsync(ProviderContext context, CancellationToken await UnderRefreshLockAsync(context, async ct => await ReadStoredAfterKeptPairAsync(context, ct).ConfigureAwait(false), cancellationToken).ConfigureAwait(false); } + public void Discard(Guid credentialId) + { + if (KeptRotations.TryRemove(credentialId, out _)) + _logger.LogWarning("Credential {CredentialId} was re-authorized; the rotated pair this process had kept for the grant it replaced is dropped, not stored over the new authorization", credentialId); + } + /// The credential's two refresh locks — this process's, then every process's — held for . private async Task UnderRefreshLockAsync(ProviderContext context, Func> work, CancellationToken cancellationToken) { - var sem = RefreshLocks.GetOrAdd(context.Credential.Id, _ => new SemaphoreSlim(1, 1)); - await sem.WaitAsync(cancellationToken).ConfigureAwait(false); + await using var held = await CredentialRefreshLock.AcquireAsync(_crossProcessLock, context.Credential.Id, cancellationToken).ConfigureAwait(false); - try - { - await using var crossLock = await _crossProcessLock.AcquireAsync(CredentialIdToLockKey(context.Credential.Id), cancellationToken).ConfigureAwait(false); - - return await work(cancellationToken).ConfigureAwait(false); - } - finally - { - sem.Release(); - } + return await work(cancellationToken).ConfigureAwait(false); } /// @@ -451,12 +435,5 @@ private async Task ExchangeRefreshAsync(ProviderContext context, O }; } - /// - /// Stable int64 derived from the credential GUID. pg_advisory_lock takes int8 (bigint). - /// We take the first 8 bytes of the GUID — uniform distribution, collision probability - /// is the same as a 64-bit random key. Same credential id always maps to same key. - /// - internal static long CredentialIdToLockKey(Guid credentialId) => BitConverter.ToInt64(credentialId.ToByteArray(), 0); - private sealed record KeptRotation(string Expected, OAuthPayload Issued); } diff --git a/backend/src/CodeSpace.Messages/Commands/Credentials/InitCredentialOAuthCommand.cs b/backend/src/CodeSpace.Messages/Commands/Credentials/InitCredentialOAuthCommand.cs index 7a8f810da..94995a59d 100644 --- a/backend/src/CodeSpace.Messages/Commands/Credentials/InitCredentialOAuthCommand.cs +++ b/backend/src/CodeSpace.Messages/Commands/Credentials/InitCredentialOAuthCommand.cs @@ -35,6 +35,17 @@ public sealed record InitCredentialOAuthCommand : ICommandOverride the provider_instance's default scope list. Usually null. public IReadOnlyList? Scopes { get; init; } + + /// + /// The credential this connect re-authorizes IN PLACE instead of creating another: when the callback completes, the new + /// token is written into this row — same id, so what is bound through it needs no moving — and what its dead token had + /// turned Error comes back. Null (what a client that predates this field sends) connects a new credential exactly as + /// before. It must be a Personal credential of this team and provider instance, owned by the person; the sign-in must + /// be the same provider account the credential was connected as, or the callback is refused with + /// credential_account_mismatch. A pod from before this field ignores it and creates a new credential, as a + /// connect always did. + /// + public Guid? ReplaceCredentialId { get; init; } } public sealed record InitCredentialOAuthResult diff --git a/backend/src/CodeSpace.Messages/Commands/Credentials/ReplaceCredentialTokenCommand.cs b/backend/src/CodeSpace.Messages/Commands/Credentials/ReplaceCredentialTokenCommand.cs new file mode 100644 index 000000000..803ed1898 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Commands/Credentials/ReplaceCredentialTokenCommand.cs @@ -0,0 +1,35 @@ +using CodeSpace.Messages.Authorization; +using CodeSpace.Messages.Mediation; + +namespace CodeSpace.Messages.Commands.Credentials; + +/// +/// Puts a new token into a credential that already exists, in place: the credential keeps its id, so every repository, group +/// hook and act-as identity bound through it keeps working, and the ones its dead token had turned Error come back. For a +/// token that expired or was revoked at the provider, and for a replacement of an OAuth sign-in by a token — the row's auth +/// type follows the new token. +/// +/// The token is probed against the provider before anything is written, and must belong to the SAME provider account +/// the credential was connected as: another account is a new connection, not a re-authorization, and is refused with +/// credential_account_mismatch. A credential retired when its person left the team or its connection was removed is +/// refused with credential_retired. +/// +/// : the owner of a Personal credential on membership alone; anyone else — +/// a teammate's or a team-service credential — needs credentials.manage. +/// +public sealed record ReplaceCredentialTokenCommand : ICommand, IRequireCredentialManagement +{ + /// Set by the controller from the route segment via `command with { CredentialId = ... }`. Non-required so System.Text.Json doesn't 400-fail when the body omits it (URL is authoritative). + public Guid CredentialId { get; init; } + + /// The new token. Carried in the request BODY only; never logged or echoed back. + public required string Token { get; init; } +} + +public sealed record ReplaceCredentialTokenResult +{ + public required Guid CredentialId { get; init; } + + /// The provider account the new token belongs to, when the provider could say — what the person can check it against. + public string? ProviderUsername { get; init; } +} diff --git a/backend/src/CodeSpace.Messages/Credentials/CredentialReauthorization.cs b/backend/src/CodeSpace.Messages/Credentials/CredentialReauthorization.cs new file mode 100644 index 000000000..5fa781473 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Credentials/CredentialReauthorization.cs @@ -0,0 +1,63 @@ +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Messages.Credentials; + +/// +/// A fresh authorization to put into a credential row that already exists, in place — the same id, so everything that +/// references it (repositories, group hooks, act-as identities) keeps working without anything being moved. Handed over by +/// the OAuth callback finishing a connect that named a credential to replace, and by a person pasting a replacement token. +/// +public sealed record CredentialReauthorization +{ + public required Guid CredentialId { get; init; } + + /// The team the request was made in. A credential of another team reads as not found. + public required Guid TeamId { get; init; } + + /// The provider instance the authorization was obtained from — must be the credential's own. + public required Guid ProviderInstanceId { get; init; } + + /// The new token material. Its type becomes the row's auth type: a token can replace an OAuth sign-in and the other way round. + public required CredentialPayload Payload { get; init; } + + /// The scopes the new authorization was granted, when the provider said. Left out, the provider's own probe of the token supplies them, or the row records them as unknown. + public IReadOnlyList? Scopes { get; init; } + + /// A new display name; left out, the row keeps the one it has. + public string? DisplayName { get; init; } + + /// Who is re-authorizing — written as the row's last modifier, and the only person whose act-as identity the re-authorization relinks (their own). + public required Guid ActorUserId { get; init; } +} + +public sealed record CredentialReauthorizationResult +{ + public required Guid CredentialId { get; init; } + + /// The provider account the new authorization belongs to, when the provider could say. + public string? ProviderUsername { get; init; } +} + +/// +/// The columns a re-authorization writes in one statement, and the payload the row must still hold for the write to land. +/// +public sealed record CredentialReplacement +{ + public required Guid CredentialId { get; init; } + + /// The encrypted payload the row held when it was read under the refresh lock. The write lands only while the row still holds it. + public required string ExpectedEncryptedPayload { get; init; } + + public required string EncryptedPayload { get; init; } + public required AuthType AuthType { get; init; } + + /// Null records the scopes as unknown, which the capability check reads as "no false warning" rather than "none granted". + public IReadOnlyList? Scopes { get; init; } + + public DateTimeOffset? ExpiresDate { get; init; } + + /// Null keeps the row's name. + public string? DisplayName { get; init; } + + public required Guid ModifiedBy { get; init; } +} diff --git a/backend/src/CodeSpace.Messages/Dtos/Credentials/BindingsRevived.cs b/backend/src/CodeSpace.Messages/Dtos/Credentials/BindingsRevived.cs new file mode 100644 index 000000000..88d143441 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Dtos/Credentials/BindingsRevived.cs @@ -0,0 +1,13 @@ +namespace CodeSpace.Messages.Dtos.Credentials; + +/// What a credential coming back to life brought back with it: the repositories its disconnect had turned Error, and the group hooks its failures had buried. +public sealed record BindingsRevived +{ + public static BindingsRevived None { get; } = new(); + + public int RepositoryCount { get; init; } + + public int ConnectionWebhookCount { get; init; } + + public bool Any => RepositoryCount + ConnectionWebhookCount > 0; +} diff --git a/backend/src/CodeSpace.Messages/Exceptions/CredentialAccountMismatchException.cs b/backend/src/CodeSpace.Messages/Exceptions/CredentialAccountMismatchException.cs new file mode 100644 index 000000000..37130d7cb --- /dev/null +++ b/backend/src/CodeSpace.Messages/Exceptions/CredentialAccountMismatchException.cs @@ -0,0 +1,32 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Messages.Exceptions; + +/// +/// Thrown when a credential is asked to take an authorization that belongs to a different provider account than the one it +/// was connected as. A credential is one person's connection of one provider account, and everything bound through it acts +/// as that account; putting another account's token into the same row would change who those repositories, hooks and +/// act-as writes are attributed to without anyone having said so. Signing in as another account is a new connection. +/// +public sealed class CredentialAccountMismatchException : Exception, IFailure +{ + public FailureKind Kind => FailureKind.Conflict; + + public string Code => FailureCodes.CredentialAccountMismatch; + + public IReadOnlyDictionary? Details => new Dictionary { ["credentialId"] = CredentialId, ["expectedAccount"] = ExpectedAccount, ["presentedAccount"] = PresentedAccount }; + + public Guid CredentialId { get; } + + public string ExpectedAccount { get; } + + public string PresentedAccount { get; } + + public CredentialAccountMismatchException(Guid credentialId, string displayName, string expectedAccount, string presentedAccount) + : base($"The credential '{displayName}' belongs to the {expectedAccount} account, and the authorization you gave is for {presentedAccount}. Re-authorizing means the same account signing in again; connect {presentedAccount} as its own credential instead, or sign in as {expectedAccount}.") + { + CredentialId = credentialId; + ExpectedAccount = expectedAccount; + PresentedAccount = presentedAccount; + } +} diff --git a/backend/src/CodeSpace.Messages/Exceptions/CredentialRetiredException.cs b/backend/src/CodeSpace.Messages/Exceptions/CredentialRetiredException.cs new file mode 100644 index 000000000..ee1ab3403 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Exceptions/CredentialRetiredException.cs @@ -0,0 +1,31 @@ +using CodeSpace.Messages.Failures; + +namespace CodeSpace.Messages.Exceptions; + +/// +/// Thrown when a credential is asked to take a new authorization in place after it was retired — soft-deleted when its +/// person left the team or its connection was removed, or held by someone who no longer stands in the team and so about to +/// be. A retired credential keeps no token and nothing is allowed to bring it back: it would be a live token of someone +/// without standing, which the departed-token sweep then retires again together with whatever it had just taken in. The +/// remedy is a new connection. +/// +public sealed class CredentialRetiredException : Exception, IFailure +{ + public FailureKind Kind => FailureKind.Conflict; + + public string Code => FailureCodes.CredentialRetired; + + public IReadOnlyDictionary? Details => new Dictionary { ["credentialId"] = CredentialId }; + + public Guid CredentialId { get; } + + public CredentialRetiredException(Guid credentialId, string displayName, string reason) + : base($"The credential '{displayName}' cannot be re-authorized: {reason} Connect the provider again to create a new credential.") + { + CredentialId = credentialId; + } + + /// The row is soft-deleted: the retirement itself, as opposed to one that is about to happen. + public static CredentialRetiredException SoftDeleted(Guid credentialId, string displayName) => + new(credentialId, displayName, "its connection was removed or its person left the team, and a retired credential stays retired."); +} diff --git a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs index 570f71e86..bcb45adb7 100644 --- a/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs +++ b/backend/src/CodeSpace.Messages/Failures/FailureCodes.cs @@ -58,6 +58,12 @@ public static class FailureCodes /// A credential's grant or token was refused by its provider and cannot be used again as it is — an OAuth refresh answered invalid_grant, a static token the provider rejects or has expired. Remedy: reconnect the provider (or add the token again); a retry as-is is refused the same way, so nothing bound through the credential authenticates until then. public const string CredentialNeedsReconnect = "credential_needs_reconnect"; + /// A credential was asked to take a new authorization in place after it was retired — its person left the team, or its connection was removed — and a retired row stays retired. Remedy: connect again, which creates a new credential; a retry as-is reads the same retired row. + public const string CredentialRetired = "credential_retired"; + + /// A credential was asked to take an authorization that belongs to a different provider account than the one it was connected as. Re-authorizing means the same account signing in again; another account is a new connection. Remedy: sign in as the account the credential is for, or connect the other account separately. + public const string CredentialAccountMismatch = "credential_account_mismatch"; + /// The remote refused the git credential a push carried even after it was resolved again immediately before the push (or the credential was disconnected while the run worked), so the agent's branch was not published. Remedy: reconnect the provider, or re-link the repository to an active credential, then re-run — a retry as-is is refused the same way. public const string GitCredentialRejected = "git_credential_rejected"; diff --git a/backend/tests/CodeSpace.E2ETests/Credentials/CredentialReauthorizationE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Credentials/CredentialReauthorizationE2ETests.cs new file mode 100644 index 000000000..c830d496b --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Credentials/CredentialReauthorizationE2ETests.cs @@ -0,0 +1,364 @@ +using System.Net; +using System.Text.Json; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using Xunit.Abstractions; + +namespace CodeSpace.E2ETests.Credentials; + +/// +/// 🟢 High fidelity — a credential re-authorized IN PLACE through the real HTTP surface: the real ASP.NET pipeline (routing, +/// JWT, the X-Team-Id scope, model binding, the credential authorization behaviors, the exception filter), cookies on the +/// wire, the real GitLab OAuth client and GitLab provider, real Postgres. The one fake is GitLab, a +/// on a loopback port. +/// +/// What this pins end to end: POST /api/credentials/oauth/init naming a replaceCredentialId, then the +/// provider's callback — completing in the browser that started it, or handed to the SPA's completion page and finished by +/// POST /api/credentials/oauth/complete when the SPA and the API are on different sites — leaves the SAME credential +/// id holding the new pair, with its repository still bound to it and now working; PUT /api/credentials/{id}/token does +/// the same for a pasted token; and each refusal reaches a client as its own status and code. +/// +[Trait("Category", "E2E")] +[Trait("Category", "ProviderE2E")] +[Trait("Surface", "Http")] +public sealed class CredentialReauthorizationE2ETests : IClassFixture +{ + private const string BindingCookie = "codespace_oauth_nonce"; + private const string CallbackPath = "/api/credentials/oauth/callback"; + private const string CompletePath = "/api/credentials/oauth/complete"; + + /// A dev SPA origin on another site than the API's http://localhost — on the CORS list in Development. + private const string CrossSiteSpa = "http://127.0.0.1:5180"; + + private readonly TaskLaunchApiFactory _factory; + private readonly TenantHarness _tenants; + private readonly ITestOutputHelper _output; + + public CredentialReauthorizationE2ETests(TaskLaunchApiFactory factory, ITestOutputHelper output) + { + _factory = factory; + _tenants = new TenantHarness(factory); + _output = output; + } + + [Fact] + public async Task A_reconnect_completes_in_the_starting_browser_on_the_same_credential_and_the_repository_works_again() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab, deadPair: true).ConfigureAwait(false); + (await ReadListedAsync(world.OwnerId, world.TeamId, world.CredentialId).ConfigureAwait(false)).GetProperty("health").GetString().ShouldBe("rejected", "fixture check: the Providers list shows the refused credential as one to reconnect"); + + var init = await InitAsync(world, "/settings/credentials", world.CredentialId).ConfigureAwait(false); + + init.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(init).ConfigureAwait(false)); + var callback = await CallbackAsync(await StateOfAsync(init).ConfigureAwait(false), "authz:nacho", CookieValue(BindingCookieOf(init)!)).ConfigureAwait(false); + + callback.StatusCode.ShouldBe(HttpStatusCode.Redirect, await TenantHarness.DescribeAsync(callback).ConfigureAwait(false)); + callback.Headers.Location!.OriginalString.ShouldBe($"/settings/credentials?oauthCredentialId={world.CredentialId}", "the popup is told the credential it already had"); + + Dump(gitlab); + var credential = await ReadCredentialAsync(world.CredentialId).ConfigureAwait(false); + credential.Status.ShouldBe(CredentialStatus.Active); + credential.LastError.ShouldBeNull(); + credential.StoredPair.RefreshToken.ShouldBe(gitlab.LiveRefreshTokenOf("nacho"), "the row holds the pair GitLab issued for this sign-in"); + (await _tenants.DbAsync(db => db.Credential.AsNoTracking().CountAsync(c => c.TeamId == world.TeamId)).ConfigureAwait(false)).ShouldBe(1, "no second credential was minted"); + + var repository = await _tenants.DbAsync(db => db.Repository.AsNoTracking().SingleAsync(r => r.Id == world.RepositoryId)).ConfigureAwait(false); + repository.CredentialId.ShouldBe(world.CredentialId); + repository.Status.ShouldBe(RepositoryStatus.Active, "the Error its dead token had set is lifted"); + + var served = await _tenants.SendAsync(HttpMethod.Get, $"/api/credentials/{world.CredentialId}/accessible-repositories", world.OwnerId, world.TeamId).ConfigureAwait(false); + served.StatusCode.ShouldBe(HttpStatusCode.OK, $"the same credential id is served with the new pair — {await TenantHarness.DescribeAsync(served).ConfigureAwait(false)}"); + gitlab.Count(StubEvent.Refused).ShouldBe(0, $"nothing presented a consumed refresh token: {string.Join(", ", gitlab.RefusedRefreshTokens())}"); + + var listed = await ReadListedAsync(world.OwnerId, world.TeamId, world.CredentialId).ConfigureAwait(false); + listed.GetProperty("health").GetString().ShouldBe("active", "the list reads the credential as the re-authorization left it, with no sweep in between"); + listed.GetProperty("status").GetString().ShouldBe("Active"); + listed.GetProperty("lastError").ValueKind.ShouldBe(JsonValueKind.Null); + listed.GetProperty("lastValidatedDate").ValueKind.ShouldNotBe(JsonValueKind.Null); + } + + [Fact] + public async Task A_cross_site_reconnect_is_finished_by_its_starters_spa_on_the_same_credential() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab, deadPair: true).ConfigureAwait(false); + var init = await InitAsync(world, $"{CrossSiteSpa}/oauth-callback", world.CredentialId).ConfigureAwait(false); + var state = await StateOfAsync(init).ConfigureAwait(false); + + // The SPA and the API are on different sites, so the binding cookie never comes back: the callback hands the code to + // the SPA origin the connect named, exchanging nothing. + var callback = await CallbackAsync(state, "authz:nacho", bindingCookie: null).ConfigureAwait(false); + + callback.StatusCode.ShouldBe(HttpStatusCode.Redirect, await TenantHarness.DescribeAsync(callback).ConfigureAwait(false)); + callback.Headers.Location!.OriginalString.ShouldStartWith($"{CrossSiteSpa}/oauth-callback#oauthCode="); + gitlab.Count(StubEvent.Authorized).ShouldBe(0, "handing off exchanges nothing"); + + var finish = await CompleteAsync(world.OwnerId, world.TeamId, "authz:nacho", state).ConfigureAwait(false); + + finish.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(finish).ConfigureAwait(false)); + JsonDocument.Parse(await finish.Content.ReadAsStringAsync().ConfigureAwait(false)).RootElement.GetProperty("credentialId").GetGuid().ShouldBe(world.CredentialId); + (await ReadCredentialAsync(world.CredentialId).ConfigureAwait(false)).StoredPair.RefreshToken.ShouldBe(gitlab.LiveRefreshTokenOf("nacho")); + (await _tenants.DbAsync(db => db.Credential.AsNoTracking().CountAsync(c => c.TeamId == world.TeamId)).ConfigureAwait(false)).ShouldBe(1); + (await _tenants.DbAsync(db => db.OAuthPendingState.AsNoTracking().CountAsync(s => s.State == state)).ConfigureAwait(false)).ShouldBe(0, "finishing spends the state"); + } + + [Fact] + public async Task A_forwarded_cross_site_reconnect_is_refused_to_everyone_but_its_starter() + { + // The reconnect names a credential, so a victim who finishes it would put THEIR token into the starter's credential row. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab, deadPair: true).ConfigureAwait(false); + var victim = await _tenants.AccountAsync().ConfigureAwait(false); + await _tenants.AddMemberAsync(world.TeamId, victim, TeamRole.Owner).ConfigureAwait(false); + var state = await StateOfAsync(await InitAsync(world, $"{CrossSiteSpa}/oauth-callback", world.CredentialId).ConfigureAwait(false)).ConfigureAwait(false); + var before = await ReadCredentialAsync(world.CredentialId).ConfigureAwait(false); + + var finish = await CompleteAsync(victim, world.TeamId, "authz:victim", state).ConfigureAwait(false); + + finish.StatusCode.ShouldBe(HttpStatusCode.BadRequest, await TenantHarness.DescribeAsync(finish).ConfigureAwait(false)); + gitlab.Count(StubEvent.Authorized).ShouldBe(0, "the victim's code never reached the token endpoint"); + (await ReadCredentialAsync(world.CredentialId).ConfigureAwait(false)).EncryptedPayload.ShouldBe(before.EncryptedPayload); + } + + [Fact] + public async Task A_reconnect_as_another_account_is_refused_with_its_own_status_and_code_on_both_doors() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab, deadPair: true).ConfigureAwait(false); + var before = await ReadCredentialAsync(world.CredentialId).ConfigureAwait(false); + + var sameSite = await InitAsync(world, "/settings/credentials", world.CredentialId).ConfigureAwait(false); + var refusedAtCallback = await CallbackAsync(await StateOfAsync(sameSite).ConfigureAwait(false), "authz:someone-else", CookieValue(BindingCookieOf(sameSite)!)).ConfigureAwait(false); + + var crossSiteState = await StateOfAsync(await InitAsync(world, $"{CrossSiteSpa}/oauth-callback", world.CredentialId).ConfigureAwait(false)).ConfigureAwait(false); + var refusedAtComplete = await CompleteAsync(world.OwnerId, world.TeamId, "authz:someone-else", crossSiteState).ConfigureAwait(false); + + foreach (var refusal in new[] { refusedAtCallback, refusedAtComplete }) + { + refusal.StatusCode.ShouldBe(HttpStatusCode.Conflict, await TenantHarness.DescribeAsync(refusal).ConfigureAwait(false)); + var body = JsonDocument.Parse(await refusal.Content.ReadAsStringAsync().ConfigureAwait(false)).RootElement; + body.GetProperty("code").GetString().ShouldBe("credential_account_mismatch"); + body.GetProperty("expectedAccount").GetString().ShouldBe("nacho"); + body.GetProperty("presentedAccount").GetString().ShouldBe("someone-else"); + body.GetProperty("message").GetString().ShouldContain("connect someone-else as its own credential", Case.Insensitive); + } + + (await ReadCredentialAsync(world.CredentialId).ConfigureAwait(false)).EncryptedPayload.ShouldBe(before.EncryptedPayload, "neither door wrote anything"); + } + + [Fact] + public async Task Starting_a_reconnect_names_a_credential_of_this_team_only() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab, deadPair: true).ConfigureAwait(false); + var elsewhere = await SeedAsync(gitlab, deadPair: true).ConfigureAwait(false); + + var init = await InitAsync(world, "/settings/credentials", elsewhere.CredentialId).ConfigureAwait(false); + + init.StatusCode.ShouldBe(HttpStatusCode.NotFound, await TenantHarness.DescribeAsync(init).ConfigureAwait(false)); + (await _tenants.DbAsync(db => db.OAuthPendingState.AsNoTracking().CountAsync(s => s.TeamId == world.TeamId)).ConfigureAwait(false)).ShouldBe(0); + } + + [Fact] + public async Task Starting_a_reconnect_of_a_retired_credential_is_refused_as_retired_and_starts_nothing() + { + // What a departure or a connection delete leaves: the token gone, the row soft-deleted. It stays retired. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab, deadPair: true).ConfigureAwait(false); + await _tenants.DbAsync(async db => await db.Credential.Where(c => c.Id == world.CredentialId).ExecuteUpdateAsync(s => s.SetProperty(c => c.EncryptedPayload, string.Empty).SetProperty(c => c.Status, CredentialStatus.Revoked).SetProperty(c => c.DeletedDate, DateTimeOffset.UtcNow))).ConfigureAwait(false); + + var init = await InitAsync(world, "/settings/credentials", world.CredentialId).ConfigureAwait(false); + + init.StatusCode.ShouldBe(HttpStatusCode.Conflict, await TenantHarness.DescribeAsync(init).ConfigureAwait(false)); + JsonDocument.Parse(await init.Content.ReadAsStringAsync().ConfigureAwait(false)).RootElement.GetProperty("code").GetString().ShouldBe("credential_retired"); + (await _tenants.DbAsync(db => db.OAuthPendingState.AsNoTracking().CountAsync(s => s.TeamId == world.TeamId)).ConfigureAwait(false)).ShouldBe(0); + } + + [Fact] + public async Task A_pasted_token_replaces_the_credentials_token_in_place_and_reaches_nothing_at_rest_in_the_clear() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab, deadPair: false, tokenCredential: true).ConfigureAwait(false); + var fresh = $"glpat-fresh-{Guid.NewGuid():N}"; + gitlab.AddAccessToken(fresh, "maya", DateTimeOffset.UtcNow.AddDays(90)); + (await ReadListedAsync(world.MayaId, world.TeamId, world.CredentialId).ConfigureAwait(false)).GetProperty("health").GetString().ShouldBe("rejected", "fixture check: the refused token reads as one to replace"); + + var replaced = await ReplaceTokenAsync(world.CredentialId, world.MayaId, world.TeamId, fresh).ConfigureAwait(false); + + replaced.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(replaced).ConfigureAwait(false)); + var body = await replaced.Content.ReadAsStringAsync().ConfigureAwait(false); + body.ShouldNotContain(fresh, Case.Sensitive, "the token is carried in the request body only and never echoed back"); + var json = JsonDocument.Parse(body).RootElement; + json.GetProperty("credentialId").GetGuid().ShouldBe(world.CredentialId); + json.GetProperty("providerUsername").GetString().ShouldBe("maya"); + + var credential = await ReadTokenCredentialAsync(world.CredentialId).ConfigureAwait(false); + credential.Status.ShouldBe(CredentialStatus.Active); + credential.LastError.ShouldBeNull(); + credential.Token.ShouldBe(fresh); + var repository = await _tenants.DbAsync(db => db.Repository.AsNoTracking().SingleAsync(r => r.Id == world.RepositoryId)).ConfigureAwait(false); + repository.CredentialId.ShouldBe(world.CredentialId); + repository.Status.ShouldBe(RepositoryStatus.Active); + + using var scope = _factory.Services.CreateScope(); + var hits = await SecretCanarySweep.FindAsync(_factory.ConnectionString, scope.ServiceProvider.GetRequiredService(), scope.ServiceProvider.GetRequiredService(), world.TeamId, fresh).ConfigureAwait(false); + hits.ShouldBeEmpty("the new token is stored sealed and appears in no table in the clear"); + + var listed = await ReadListedAsync(world.MayaId, world.TeamId, world.CredentialId).ConfigureAwait(false); + listed.GetProperty("health").GetString().ShouldBe("active", "the token that replaced the refused one is what the list reads"); + listed.GetProperty("status").GetString().ShouldBe("Active"); + listed.GetProperty("lastError").ValueKind.ShouldBe(JsonValueKind.Null); + listed.GetProperty("expiresDate").ValueKind.ShouldNotBe(JsonValueKind.Null, "the new token's own expiry, as GitLab reports it"); + listed.GetProperty("daysToExpiry").GetInt32().ShouldBeInRange(88, 90); + } + + [Fact] + public async Task A_replacement_token_is_refused_with_the_status_and_code_the_client_branches_on() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab, deadPair: false, tokenCredential: true).ConfigureAwait(false); + var otherMember = await _tenants.AccountAsync().ConfigureAwait(false); + await _tenants.AddMemberAsync(world.TeamId, otherMember, TeamRole.Member).ConfigureAwait(false); + var elsewhere = await SeedAsync(gitlab, deadPair: false, tokenCredential: true).ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("nacho", "glpat-nachos"); + gitlab.SeedPersonalAccessToken("maya", "glpat-fresh"); + + var anotherAccount = await ReplaceTokenAsync(world.CredentialId, world.MayaId, world.TeamId, "glpat-nachos").ConfigureAwait(false); + var notWorking = await ReplaceTokenAsync(world.CredentialId, world.MayaId, world.TeamId, "glpat-typo").ConfigureAwait(false); + var notYours = await ReplaceTokenAsync(world.CredentialId, otherMember, world.TeamId, "glpat-fresh").ConfigureAwait(false); + var notThisTeams = await ReplaceTokenAsync(elsewhere.CredentialId, world.MayaId, world.TeamId, "glpat-fresh").ConfigureAwait(false); + var anonymous = await _tenants.SendAsync(HttpMethod.Put, $"/api/credentials/{world.CredentialId}/token", userId: null, world.TeamId, new { token = "glpat-fresh" }).ConfigureAwait(false); + + anotherAccount.StatusCode.ShouldBe(HttpStatusCode.Conflict, await TenantHarness.DescribeAsync(anotherAccount).ConfigureAwait(false)); + JsonDocument.Parse(await anotherAccount.Content.ReadAsStringAsync().ConfigureAwait(false)).RootElement.GetProperty("code").GetString().ShouldBe("credential_account_mismatch"); + notWorking.StatusCode.ShouldBe(HttpStatusCode.BadRequest, await TenantHarness.DescribeAsync(notWorking).ConfigureAwait(false)); + (await notWorking.Content.ReadAsStringAsync().ConfigureAwait(false)).ShouldContain("Could not validate the token against GitLab"); + notYours.StatusCode.ShouldBe(HttpStatusCode.Forbidden, await TenantHarness.DescribeAsync(notYours).ConfigureAwait(false)); + notThisTeams.StatusCode.ShouldBe(HttpStatusCode.Forbidden, await TenantHarness.DescribeAsync(notThisTeams).ConfigureAwait(false)); + anonymous.StatusCode.ShouldBe(HttpStatusCode.Unauthorized, await TenantHarness.DescribeAsync(anonymous).ConfigureAwait(false)); + + (await ReadTokenCredentialAsync(world.CredentialId).ConfigureAwait(false)).Token.ShouldBe(world.TokenOfCredential, "every refusal left the credential as it was"); + (await ReadTokenCredentialAsync(elsewhere.CredentialId).ConfigureAwait(false)).Token.ShouldBe(elsewhere.TokenOfCredential); + } + + // ─── Plumbing ────────────────────────────────────────────────────────────── + + private sealed record World(Guid OwnerId, Guid MayaId, Guid TeamId, Guid InstanceId, Guid CredentialId, Guid RepositoryId, string TokenOfCredential); + + private sealed record StoredCredential(CredentialStatus Status, string? LastError, string EncryptedPayload, OAuthPayload StoredPair); + + private sealed record StoredToken(CredentialStatus Status, string? LastError, string Token); + + /// + /// A team with an Admin (nacho) and a Member (maya), a GitLab instance whose OAuth app is the stub's, and a credential + /// with a repository bound to it, the Error its dead token set already on it. makes it + /// maya's personal access token; otherwise nacho's OAuth pair — which GitLab never heard of when . + /// + private async Task SeedAsync(GitLabDoorkeeperStub gitlab, bool deadPair, bool tokenCredential = false) + { + var owner = await _tenants.AccountAsync().ConfigureAwait(false); + var maya = await _tenants.AccountAsync().ConfigureAwait(false); + var viewer = await _tenants.AccountAsync().ConfigureAwait(false); + var team = await _tenants.TeamAsync((owner, TeamRole.Admin), (maya, TeamRole.Member), (viewer, TeamRole.Viewer)).ConfigureAwait(false); + + if (!deadPair && !tokenCredential) gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-0", "rt-0"); + + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var encryptor = scope.ServiceProvider.GetRequiredService(); + var serializer = scope.ServiceProvider.GetRequiredService(); + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team, Provider = ProviderKind.GitLab, DisplayName = $"gitlab-{Guid.NewGuid():N}"[..14], BaseUrl = gitlab.BaseUrl, ApiUrl = gitlab.BaseUrl, OauthClientId = GitLabDoorkeeperStub.DefaultClientId, OauthClientSecretEnc = encryptor.Encrypt(GitLabDoorkeeperStub.DefaultClientSecret), CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }; + + var ownerId = tokenCredential ? maya : owner; + var token = $"glpat-expired-{Guid.NewGuid():N}"; + CredentialPayload payload = tokenCredential ? new PatPayload { Token = token } : new OAuthPayload { AccessToken = "at-0", RefreshToken = "rt-0", ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(-1) }; + var credential = new Credential { Id = Guid.NewGuid(), TeamId = team, ProviderInstanceId = instance.Id, OwnerUserId = ownerId, Ownership = CredentialOwnership.Personal, AuthType = payload.Type, DisplayName = tokenCredential ? "GitLab · maya" : "GitLab · nacho", EncryptedPayload = encryptor.Encrypt(serializer.Serialize(payload)), ExpiresDate = (payload as OAuthPayload)?.ExpiresAt, Status = CredentialStatus.Error, LastError = "401 Unauthorized", CreatedBy = ownerId, LastModifiedBy = ownerId }; + var repository = new Repository { Id = Guid.NewGuid(), TeamId = team, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ExternalId = Random.Shared.Next(100_000, 999_999).ToString(), NamespacePath = "acme", Name = "api", FullPath = "acme/api", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = $"{gitlab.BaseUrl}/acme/api", CloneUrlHttps = $"{gitlab.BaseUrl}/acme/api.git", Status = RepositoryStatus.Error, LastError = CredentialRevocationService.DisconnectedRepositoryError }; + var identity = new UserProviderIdentity { Id = Guid.NewGuid(), UserId = ownerId, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ProviderUserId = gitlab.UserIdOf(tokenCredential ? "maya" : "nacho").ToString(), ProviderUsername = tokenCredential ? "maya" : "nacho" }; + + db.ProviderInstance.Add(instance); + db.Credential.Add(credential); + db.Repository.Add(repository); + db.UserProviderIdentity.Add(identity); + await db.SaveChangesAsync().ConfigureAwait(false); + + return new World(owner, maya, team, instance.Id, credential.Id, repository.Id, token); + } + + private async Task InitAsync(World world, string returnUrl, Guid replaceCredentialId) => + await _tenants.Client().SendAsync(_tenants.Request(HttpMethod.Post, "/api/credentials/oauth/init", world.OwnerId, world.TeamId, new { providerInstanceId = world.InstanceId, displayName = "GitLab · nacho", returnUrl, replaceCredentialId })); + + private async Task CallbackAsync(string state, string code, string? bindingCookie) + { + var request = _tenants.Request(HttpMethod.Get, $"{CallbackPath}?code={Uri.EscapeDataString(code)}&state={Uri.EscapeDataString(state)}", userId: null, teamId: null); + + if (bindingCookie != null) request.Headers.Add("Cookie", $"{BindingCookie}={bindingCookie}"); + + return await _tenants.Client().SendAsync(request); + } + + private async Task CompleteAsync(Guid userId, Guid teamId, string code, string state) => + await _tenants.Client().SendAsync(_tenants.Request(HttpMethod.Post, CompletePath, userId, teamId, new { code, state })); + + private async Task ReplaceTokenAsync(Guid credentialId, Guid userId, Guid teamId, string token) => + await _tenants.SendAsync(HttpMethod.Put, $"/api/credentials/{credentialId}/token", userId, teamId, new { token }); + + /// The credential as GET /api/credentials says it — what the Providers list renders, with the health derived from the row. + private async Task ReadListedAsync(Guid userId, Guid teamId, Guid credentialId) + { + var response = await _tenants.SendAsync(HttpMethod.Get, "/api/credentials", userId, teamId).ConfigureAwait(false); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(response).ConfigureAwait(false)); + + using var json = JsonDocument.Parse(await response.Content.ReadAsStringAsync().ConfigureAwait(false)); + + return json.RootElement.EnumerateArray().Single(c => c.GetProperty("id").GetGuid() == credentialId).Clone(); + } + + private async Task ReadCredentialAsync(Guid credentialId) + { + var credential = await _tenants.DbAsync(db => db.Credential.AsNoTracking().SingleAsync(c => c.Id == credentialId)).ConfigureAwait(false); + + return new StoredCredential(credential.Status, credential.LastError, credential.EncryptedPayload, (OAuthPayload)Open(credential)); + } + + private async Task ReadTokenCredentialAsync(Guid credentialId) + { + var credential = await _tenants.DbAsync(db => db.Credential.AsNoTracking().SingleAsync(c => c.Id == credentialId)).ConfigureAwait(false); + + return new StoredToken(credential.Status, credential.LastError, ((PatPayload)Open(credential)).Token); + } + + /// What the row holds, opened as stored — not through , which refuses a credential its provider refused (Error), and a refused re-authorization leaves exactly that. + private CredentialPayload Open(Credential credential) + { + using var scope = _factory.Services.CreateScope(); + + return scope.ServiceProvider.GetRequiredService().Deserialize(credential.AuthType, scope.ServiceProvider.GetRequiredService().Decrypt(credential.EncryptedPayload)); + } + + private void Dump(GitLabDoorkeeperStub gitlab) + { + _output.WriteLine($"GitLab saw: rotated={gitlab.Count(StubEvent.Rotated)} refused={gitlab.Count(StubEvent.Refused)} authorized={gitlab.Count(StubEvent.Authorized)} revoked={gitlab.Count(StubEvent.Revoked)} api-200={gitlab.Count(StubEvent.Api200)} api-401={gitlab.Count(StubEvent.Api401)}"); + + foreach (var entry in gitlab.Log) _output.WriteLine($" {entry.At:HH:mm:ss.fff} {entry.Kind}: {entry.Detail}"); + } + + private static async Task StateOfAsync(HttpResponseMessage init) => + JsonDocument.Parse(await init.Content.ReadAsStringAsync()).RootElement.GetProperty("state").GetString()!; + + private static string? BindingCookieOf(HttpResponseMessage response) => + response.Headers.TryGetValues("Set-Cookie", out var values) ? values.FirstOrDefault(c => c.StartsWith($"{BindingCookie}=", StringComparison.Ordinal)) : null; + + private static string CookieValue(string setCookie) => setCookie.Split(';')[0][(BindingCookie.Length + 1)..]; +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.Guards.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.Guards.cs new file mode 100644 index 000000000..05095adeb --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.Guards.cs @@ -0,0 +1,215 @@ +using System.Data.Common; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Credentials; + +/// +/// The guards a re-authorization keeps for itself. Every door in front of it — the Init lookup, the owner's standing at the +/// callback, the credential marker on PUT /token — refuses first, so a test that goes through a door passes whether or not the +/// service behind it checks anything; is documented as reusable across entry points, and +/// the tests here call it directly (or break the commit under it) so that each guard is the thing that refuses. Plus the +/// account on record for a credential the act-as identity no longer names, which no door can see at all. +/// +public partial class CredentialReauthorizationFlowTests +{ + /// The two ways a person re-authorizes a credential: a token pasted over it, or OAuth signed in again. + public enum Door + { + PastedToken, + Reconnect + } + + // ─── The account on record when the act-as identity names a newer credential ───────────────────────────── + + [Theory] + [InlineData(Door.PastedToken)] + [InlineData(Door.Reconnect)] + public async Task A_credential_no_identity_names_any_more_still_refuses_another_account(Door door) + { + // An identity is one per (person, provider instance) and every connect re-points it at the newest credential, so an + // older credential of the same person — the "token next to OAuth" state the Providers modal itself produces — is named + // by nothing. Its account is still on record: it is the account the person's identity speaks for on that instance. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var (older, newest) = await SeedOlderTokenThenConnectOAuthAsync(world).ConfigureAwait(false); + + var refusal = await Should.ThrowAsync(() => ReauthorizeAsNachoAsync(world, door, older, "maya")).ConfigureAwait(false); + + refusal.ShouldSatisfyAllConditions(r => r.ExpectedAccount.ShouldBe("nacho"), r => r.PresentedAccount.ShouldBe("maya")); + (await TokenOfAsync(older).ConfigureAwait(false)).ShouldBe("glpat-nacho-old", "the row was not touched"); + (await LoadCredentialAsync(older).ConfigureAwait(false)).Status.ShouldBe(CredentialStatus.Active); + + var identity = (await LoadIdentitiesAsync(world.NachoId).ConfigureAwait(false)).Single(i => i.DeletedDate == null); + identity.ShouldSatisfyAllConditions(i => i.CredentialId.ShouldBe(newest), i => i.ProviderUsername.ShouldBe("nacho"), i => i.ProviderUserId.ShouldBe(gitlab.UserIdOf("nacho").ToString())); + } + + [Theory] + [InlineData(Door.PastedToken)] + [InlineData(Door.Reconnect)] + public async Task The_same_account_re_authorizes_a_credential_no_identity_names_any_more_and_the_identity_still_speaks_for_that_account(Door door) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var (older, _) = await SeedOlderTokenThenConnectOAuthAsync(world).ConfigureAwait(false); + var before = await LoadCredentialAsync(older).ConfigureAwait(false); + + await ReauthorizeAsNachoAsync(world, door, older, "nacho").ConfigureAwait(false); + + var after = await LoadCredentialAsync(older).ConfigureAwait(false); + after.EncryptedPayload.ShouldNotBe(before.EncryptedPayload, "the new authorization is in the same row"); + after.LastModifiedBy.ShouldBe(world.NachoId); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(2, "no row was minted"); + + var identity = (await LoadIdentitiesAsync(world.NachoId).ConfigureAwait(false)).Single(i => i.DeletedDate == null); + identity.ShouldSatisfyAllConditions(i => i.ProviderUsername.ShouldBe("nacho"), i => i.ProviderUserId.ShouldBe(gitlab.UserIdOf("nacho").ToString())); + } + + // ─── The guards the reauthorizer keeps when nothing stands in front of it ──────────────────────────────── + + [Fact] + public async Task The_reauthorizer_itself_reads_a_credential_of_another_team_as_not_found() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var elsewhere = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + + await Should.ThrowAsync(() => ReauthorizeInWorldAsync(world, credentialId, FreshPair("at-new"), teamId: elsewhere.TeamId)).ConfigureAwait(false); + + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldBe("at-0", "another team's request wrote nothing into this team's credential"); + gitlab.Count(StubEvent.Api200).ShouldBe(0, "and asked the provider nothing"); + } + + [Fact] + public async Task The_reauthorizer_itself_refuses_a_credential_whose_person_no_longer_stands_in_the_team() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0").ConfigureAwait(false); + + await RemoveMembershipAsync(world, world.MayaId).ConfigureAwait(false); + + var refusal = await Should.ThrowAsync(() => ReauthorizeInWorldAsync(world, credentialId, FreshPair("at-new"), actorUserId: world.BossId)).ConfigureAwait(false); + + refusal.Message.ShouldContain("standing"); + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldBe("at-0", "the departed-token sweep retires it; a pair written into it now would be retired with it"); + } + + [Theory] + [InlineData(true, "Could not confirm which GitLab account", "at-0")] // an account is on record → comparing takes an answer, and none is a refusal + [InlineData(false, null, "at-unknown")] // nothing to compare → a pair the provider just issued is presumed good + public async Task A_pair_the_provider_cannot_answer_for_is_refused_only_when_an_account_has_to_be_compared(bool accountOnRecord, string? expectedRefusal, string expectedStoredAccessToken) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + + if (accountOnRecord) await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + + // A token GitLab never issued: its probe is a 401, so the provider says nothing about whose it is. + var thrown = await Record.ExceptionAsync(() => ReauthorizeInWorldAsync(world, credentialId, FreshPair("at-unknown"))).ConfigureAwait(false); + + if (expectedRefusal == null) thrown.ShouldBeNull(); + else thrown.ShouldBeOfType().Message.ShouldContain(expectedRefusal); + + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldBe(expectedStoredAccessToken); + } + + [Fact] + public async Task A_unit_of_work_that_fails_after_the_new_pair_was_stored_revokes_nothing_at_the_provider() + { + // The new pair is the only copy of an authorization the provider will not issue again, so its write is durable on a + // connection of its own and survives the rollback. The replaced pair is ended only once the unit of work commits: a + // failed commit means the person repeats the re-authorization, and nothing was revoked on the strength of one that did + // not finish. The commit is failed for real — the transaction interceptor throws as the pipeline commits. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-0", "rt-0"); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + var options = OptionsWhoseCommitFails(); + + await Should.ThrowAsync(() => CompleteAsync(world, init, "authz:nacho", b => b.RegisterInstance(options).As>().SingleInstance())).ConfigureAwait(false); + + Dump(gitlab); + var stored = await StoredPairAsync(credentialId).ConfigureAwait(false); + stored.AccessToken.ShouldBe(gitlab.LiveAccessTokenOf("nacho"), "the new pair was stored before the commit and stays"); + gitlab.Count(StubEvent.Authorized).ShouldBe(1); + gitlab.Count(StubEvent.Revoked).ShouldBe(0, "nothing was revoked: the unit of work that would have justified it never committed"); + gitlab.IsAccessTokenLive("at-0").ShouldBeTrue("the replaced pair is left to expire at the provider, as the service documents"); + } + + // ─── Plumbing ────────────────────────────────────────────────────────────────────────────────────────── + + /// + /// A person with a token linked first and OAuth connected after, as production leaves them: the token's credential and the + /// identity that names it (what LinkByPat writes), then a plain OAuth connect — the real flow — which re-points that one + /// identity at the new credential. Checked, because a seed that left the identity on the older credential would be a + /// state the product cannot produce and the tests above would pass for the wrong reason. + /// + private async Task<(Guid Older, Guid Newest)> SeedOlderTokenThenConnectOAuthAsync(World world) + { + var older = await SeedTokenCredentialAsync(world, world.NachoId, "glpat-nacho-old").ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, older, "nacho").ConfigureAwait(false); + + var init = await InitAsync(world, world.NachoId, replaceCredentialId: null).ConfigureAwait(false); + var newest = (await CompleteAsync(world, init, "authz:nacho").ConfigureAwait(false)).CredentialId!.Value; + + var live = (await LoadIdentitiesAsync(world.NachoId).ConfigureAwait(false)).Single(i => i.DeletedDate == null); + live.CredentialId.ShouldBe(newest, "the plain connect re-pointed the one live identity at the newest credential, so nothing names the older one"); + + return (older, newest); + } + + /// Nacho re-authorizes through with an authorization of . + private async Task ReauthorizeAsNachoAsync(World world, Door door, Guid credentialId, string account) + { + if (door == Door.Reconnect) + { + await ReconnectAsync(world, world.NachoId, credentialId, $"authz:{account}").ConfigureAwait(false); + + return; + } + + var token = $"glpat-{account}-{Guid.NewGuid():N}"; + world.GitLab.SeedPersonalAccessToken(account, token); + + await ReplaceTokenAsync(world, world.NachoId, credentialId, token).ConfigureAwait(false); + } + + private static OAuthPayload FreshPair(string accessToken) => new() { AccessToken = accessToken, RefreshToken = $"rt-{accessToken}", ExpiresAt = DateTimeOffset.UtcNow.AddHours(2) }; + + /// The service with nothing in front of it: the request as any entry point would build it, the team and actor chosen by the test. + private async Task ReauthorizeInWorldAsync(World world, Guid credentialId, OAuthPayload payload, Guid? teamId = null, Guid? actorUserId = null) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().ReauthorizeAsync(new CredentialReauthorization { CredentialId = credentialId, TeamId = teamId ?? world.TeamId, ProviderInstanceId = world.InstanceId, Payload = payload, ActorUserId = actorUserId ?? world.NachoId }, CancellationToken.None).ConfigureAwait(false); + } + + private DbContextOptions OptionsWhoseCommitFails() + { + DbContextOptions production; + + using (var probe = _fixture.BeginScope()) + production = probe.Resolve>(); + + return new DbContextOptionsBuilder(production).AddInterceptors(new FailingCommit()).Options; + } + + /// Fails the commit of the first transaction a scope's context opens — the unit of work the mediator pipeline wraps around a command. + private sealed class FailingCommit : DbTransactionInterceptor + { + public override ValueTask TransactionCommittingAsync(DbTransaction transaction, TransactionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) => + throw new InvalidOperationException("simulated: the unit of work could not commit"); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.Health.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.Health.cs new file mode 100644 index 000000000..7a8c625cc --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.Health.cs @@ -0,0 +1,345 @@ +using Autofac; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Settings.Database; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Dtos.Credentials; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using MediatR; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Credentials; + +/// +/// Where an in-place re-authorization meets what the credential health work records about a credential: a refusal marks it +/// Error and the list derives health from the row, so a re-authorization has to leave the row saying what is true +/// of the NEW token — active, validated now, expiring when the provider says so — and nothing that judged the old one may land +/// afterwards. The one fake is GitLab, a ; the rest is production, from the mediator down to +/// Postgres, with the credential read the way a run and the Providers list read it. +/// +public partial class CredentialReauthorizationFlowTests +{ + // ─── What the list and a run say once the credential is re-authorized ───────────────────────────────────── + + [Fact] + public async Task A_grant_the_provider_refused_is_reconnected_in_place_and_reads_active_and_resolves_at_once() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + // A pair GitLab never issued, past its expiry: the first run's refresh is refused for good, as a revoked grant is. + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-dead", "rt-dead", expired: true).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Active).ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + + await Should.ThrowAsync(() => CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId)).ConfigureAwait(false); + + gitlab.Count(StubEvent.Refused).ShouldBe(1, "fixture check: GitLab refused the dead refresh token, which is what marks the credential"); + var refused = await ListedAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + refused.ShouldSatisfyAllConditions(c => c.Status.ShouldBe(CredentialStatus.Error), c => c.LastError.ShouldBe("needs reconnect: invalid_grant"), c => c.Health.ShouldBe(CredentialHealth.Rejected)); + + var finished = await ReconnectAsync(world, world.NachoId, credentialId, "authz:nacho").ConfigureAwait(false); + + Dump(gitlab); + finished.CredentialId.ShouldBe(credentialId, "the same row: nothing moved, so the connect-time rule for a refused credential's bindings had nothing to do"); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(1); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.ShouldSatisfyAllConditions(r => r.Status.ShouldBe(CredentialStatus.Active), r => r.LastError.ShouldBeNull(), r => r.DeletedDate.ShouldBeNull(), r => r.LastValidatedDate.ShouldNotBeNull().ShouldBeGreaterThan(DateTimeOffset.UtcNow.AddMinutes(-2)), r => r.ExpiresDate.ShouldNotBeNull().ShouldBeGreaterThan(DateTimeOffset.UtcNow.AddHours(1))); + + var listed = await ListedAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + listed.ShouldSatisfyAllConditions(c => c.Health.ShouldBe(CredentialHealth.Active, "derived from the row as the re-authorization left it: no sweep had to look first"), c => c.Status.ShouldBe(CredentialStatus.Active), c => c.LastError.ShouldBeNull(), c => c.DaysToExpiry.ShouldBeNull("an OAuth credential's access token renews itself")); + + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).ShouldSatisfyAllConditions(r => r.CredentialId.ShouldBe(credentialId), r => r.Status.ShouldBe(RepositoryStatus.Active)); + + var stored = await StoredPairAsync(credentialId).ConfigureAwait(false); + (await CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId).ConfigureAwait(false)).ShouldBe(stored.AccessToken, "a run bound through the repository resolves the new pair — the resolver no longer fails fast on it"); + gitlab.Count(StubEvent.Rotated).ShouldBe(0, "nothing refreshed: the pair the sign-in issued is fresh"); + gitlab.Count(StubEvent.Refused).ShouldBe(1, "nothing was refused after the re-authorization"); + } + + [Theory] + [InlineData(CredentialStatus.Error, null, CredentialHealth.Active)] // GitLab says the new token never expires + [InlineData(CredentialStatus.Active, 90, CredentialHealth.Active)] // still Active in the row, yet the list already read the lapsed token as rejected + [InlineData(CredentialStatus.Error, 10, CredentialHealth.Expiring)] // a new token that ends soon says so, as one that was added would + public async Task Replacing_the_lapsed_token_of_a_credential_reads_as_the_new_token_does(CredentialStatus before, int? daysLeft, CredentialHealth expected) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var lapsed = DateTimeOffset.UtcNow.AddDays(-3); + var credentialId = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-lapsed", status: before, lastError: before == CredentialStatus.Error ? "token rejected by GitLab" : null, expires: lapsed).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Active).ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + var expiresAt = daysLeft is { } days ? DateTimeOffset.UtcNow.AddDays(days) : (DateTimeOffset?)null; + gitlab.AddAccessToken("glpat-fresh", "maya", expiresAt); + + (await ListedAsync(world, world.MayaId, credentialId).ConfigureAwait(false)).Health.ShouldBe(CredentialHealth.Rejected, "fixture check: the lapsed token reads as refused whatever the row's status says"); + + await ReplaceTokenAsync(world, world.MayaId, credentialId, "glpat-fresh").ConfigureAwait(false); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.ShouldSatisfyAllConditions(r => r.Status.ShouldBe(CredentialStatus.Active), r => r.LastError.ShouldBeNull(), r => r.LastValidatedDate.ShouldNotBeNull().ShouldBeGreaterThan(DateTimeOffset.UtcNow.AddMinutes(-2))); + DateTimeOffset? recorded = expiresAt is { } at ? new DateTimeOffset(at.UtcDateTime.Date, TimeSpan.Zero) : null; + row.ExpiresDate.ShouldBe(recorded, "the new token's own expiry as GitLab reports it (a date), never the lapsed token's"); + + var listed = await ListedAsync(world, world.MayaId, credentialId).ConfigureAwait(false); + listed.Health.ShouldBe(expected); + + if (daysLeft is { } left) listed.DaysToExpiry.ShouldNotBeNull().ShouldBeInRange(left - 1, left); + else listed.DaysToExpiry.ShouldBeNull(); + + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).CredentialId.ShouldBe(credentialId); + (await CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId).ConfigureAwait(false)).ShouldBe("glpat-fresh", "a run resolves the new token"); + } + + [Fact] + public async Task A_pair_the_provider_refuses_at_its_first_use_is_marked_and_not_left_looking_active() + { + // With no account on record there is nothing to compare, so a pair the provider just issued is presumed good and is stored. + // If the provider then refuses it for good — here a pair it never issued — the first thing to use it is the act-as relink + // that follows the write, and that refusal is recorded like any other: the credential is Error, saying why, and the list + // says rejected. A re-authorization does not get to leave a dead token reading Active. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + + await ReauthorizeInWorldAsync(world, credentialId, FreshPair("at-unknown")).ConfigureAwait(false); + + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldBe("at-unknown", "the pair was stored: nothing could say it was wrong when it was written"); + var listed = await ListedAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + listed.ShouldSatisfyAllConditions(c => c.Status.ShouldBe(CredentialStatus.Error), c => c.LastError.ShouldBe("needs reconnect: invalid_grant"), c => c.Health.ShouldBe(CredentialHealth.Rejected)); + } + + // ─── The connect-time rule for a refused credential's bindings is for a NEW row ────────────────────────── + + [Theory] + [InlineData(Door.Reconnect)] + [InlineData(Door.PastedToken)] + public async Task Re_authorizing_one_refused_credential_in_place_moves_nothing_that_hangs_on_another_and_a_plain_connect_still_does(Door door) + { + // A plain connect treats every refused credential its owner holds as dead and carries its bindings to the new one. A + // re-authorization creates no row, so there is nothing to carry them to: what hangs on a different refused credential + // stays there until the person connects it again deliberately — which the second half shows is the connect that moves it. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var target = door == Door.Reconnect + ? await SeedOAuthCredentialAsync(world, world.NachoId, "at-dead", "rt-dead", status: CredentialStatus.Error, lastError: "needs reconnect: invalid_grant").ConfigureAwait(false) + : await SeedTokenCredentialAsync(world, world.NachoId, "glpat-dead", status: CredentialStatus.Error, lastError: "token rejected by GitLab").ConfigureAwait(false); + var other = await SeedOAuthCredentialAsync(world, world.NachoId, "at-9", "rt-9", status: CredentialStatus.Error, lastError: "needs reconnect: invalid_grant").ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, target, "nacho").ConfigureAwait(false); + var onTarget = await SeedRepositoryAsync(world, target, RepositoryStatus.Active).ConfigureAwait(false); + var onOther = await SeedRepositoryAsync(world, other, RepositoryStatus.Active).ConfigureAwait(false); + var otherHook = await SeedHookAsync(world, other, RepositoryWebhookRegistrationStatus.Failed).ConfigureAwait(false); + + await ReauthorizeAsNachoAsync(world, door, target, "nacho").ConfigureAwait(false); + + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(2, "no row was minted"); + (await LoadCredentialAsync(target).ConfigureAwait(false)).Status.ShouldBe(CredentialStatus.Active); + (await LoadRepositoryAsync(onTarget).ConfigureAwait(false)).CredentialId.ShouldBe(target); + (await LoadRepositoryAsync(onOther).ConfigureAwait(false)).CredentialId.ShouldBe(other, "the rule for a refused credential's bindings is a connect's, and this was not one"); + (await LoadHookAsync(otherHook).ConfigureAwait(false)).ShouldSatisfyAllConditions(h => h.CredentialId.ShouldBe(other), h => h.RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Failed)); + (await LoadCredentialAsync(other).ConfigureAwait(false)).Status.ShouldBe(CredentialStatus.Error); + + var plain = await CompleteAsync(world, await InitAsync(world, world.NachoId, replaceCredentialId: null).ConfigureAwait(false), "authz:nacho").ConfigureAwait(false); + + plain.CredentialId.ShouldNotBe(target); + (await LoadRepositoryAsync(onOther).ConfigureAwait(false)).CredentialId.ShouldBe(plain.CredentialId, "control: a plain connect does carry a refused credential's bindings to the new one"); + (await LoadRepositoryAsync(onTarget).ConfigureAwait(false)).CredentialId.ShouldBe(target, "the re-authorized credential is live again, so it has no successor"); + } + + // ─── A verdict about the replaced token cannot land on the new one ──────────────────────────────────────── + + [Theory] + [InlineData(false)] // GitLab refuses the replaced token: the sweep's verdict is "rejected" + [InlineData(true)] // GitLab still accepts it, with an expiry soon: the sweep's verdict is "validated", with that expiry + public async Task A_sweep_verdict_about_the_replaced_token_that_lands_after_the_replacement_is_not_written(bool oldTokenStillLive) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-old").ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + gitlab.AddAccessToken("glpat-old", "maya", oldTokenStillLive ? DateTimeOffset.UtcNow.AddDays(2) : null); + if (!oldTokenStillLive) gitlab.RevokeAccessToken("glpat-old"); + var newExpiry = DateTimeOffset.UtcNow.AddDays(60); + gitlab.AddAccessToken("glpat-new", "maya", newExpiry); + await GitLabCredentialSeed.QuietEveryOtherCredentialAsync(_fixture, credentialId).ConfigureAwait(false); + + // The sweep has asked GitLab about glpat-old and holds a verdict on it when the person's replacement lands. + var verdicts = new InterleavingHealthWriter(async () => await ReplaceTokenAsync(world, world.MayaId, credentialId, "glpat-new").ConfigureAwait(false)); + + using (var scope = _fixture.BeginScope(b => b.Register(c => verdicts.Over(new CredentialHealthWriter(c.Resolve()))).As().InstancePerLifetimeScope())) + await scope.Resolve().Send(new ProbeCredentialsCommand { BatchSize = 25 }).ConfigureAwait(false); + + Dump(gitlab); + var verdict = verdicts.Verdicts.ShouldHaveSingleItem("fixture check: the sweep reached a verdict on the credential and tried to write it"); + verdict.Kind.ShouldBe(oldTokenStillLive ? "validated" : "rejected"); + verdict.Landed.ShouldBeFalse("it judged a payload the row no longer holds"); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.ShouldSatisfyAllConditions(r => r.Status.ShouldBe(CredentialStatus.Active), r => r.LastError.ShouldBeNull(), r => r.ExpiresDate.ShouldBe(new DateTimeOffset(newExpiry.UtcDateTime.Date, TimeSpan.Zero), "the new token's expiry, not the replaced one's")); + (await TokenOfAsync(credentialId).ConfigureAwait(false)).ShouldBe("glpat-new"); + (await ListedAsync(world, world.MayaId, credentialId).ConfigureAwait(false)).Health.ShouldBe(CredentialHealth.Active); + } + + [Fact] + public async Task A_refresh_waiting_behind_a_re_authorization_does_not_present_the_dead_grant_and_does_not_mark_the_new_pair_error() + { + // The row holds a grant GitLab has ended. A refresh that ran now would be refused for good and would mark the credential + // Error — over the very row the person is re-authorizing. It takes the lock the re-authorization holds, so it waits, then + // finds the new pair fresh and has nothing to refresh. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-dead", "rt-dead", expired: true).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Active).ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + var writer = new GatedReauthorizationWriter(); + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + + var reconnect = Task.Run(() => CompleteAsync(world, init, "authz:nacho", b => b.Register(c => writer.Over(new CredentialReauthorizationWriter(c.Resolve()))).As().InstancePerLifetimeScope())); + await WithinPatienceAsync(writer.Entered, "the re-authorization reached its write", credentialId).ConfigureAwait(false); + + var run = Task.Run(() => CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId)); + await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false); + + run.IsCompleted.ShouldBeFalse("the refresh waits for the lock the re-authorization holds"); + gitlab.Count(StubEvent.Refused).ShouldBe(0, "the dead refresh token has not been presented to GitLab"); + + writer.Release(); + await WithinPatienceAsync(reconnect, "the re-authorization finished once released", credentialId).ConfigureAwait(false); + var token = await WithinPatienceAsync(run, "the run that waited for it finished", credentialId).ConfigureAwait(false); + + Dump(gitlab); + var stored = await StoredPairAsync(credentialId).ConfigureAwait(false); + token.ShouldBe(stored.AccessToken, "the run read the row after the lock and found the new pair"); + gitlab.Count(StubEvent.Refused).ShouldBe(0, "no refusal was ever recorded against the credential, so none could mark it"); + (await LoadCredentialAsync(credentialId).ConfigureAwait(false)).ShouldSatisfyAllConditions(r => r.Status.ShouldBe(CredentialStatus.Active), r => r.LastError.ShouldBeNull()); + (await ListedAsync(world, world.NachoId, credentialId).ConfigureAwait(false)).Health.ShouldBe(CredentialHealth.Active); + } + + [Fact] + public async Task A_pair_this_process_kept_for_the_replaced_grant_is_dropped_and_never_stored_over_the_new_authorization() + { + // GitLab rotated rt-0 and every store failed, so this process keeps the pair it was issued, to store it on the credential's + // next use. The person then re-authorizes. A kept pair that survived would, on that next use, find the row changed and settle + // it by which pair expires later — and a new authorization whose tokens live shorter would lose to the pair of the grant it + // replaced. The re-authorization drops the kept pair under the lock it already holds. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-0", "rt-0"); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", expired: true).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Active).ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + + await Should.ThrowAsync(() => CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId, inner => new FailingCredentialPayloadWriter(inner, int.MaxValue))).ConfigureAwait(false); + + gitlab.Count(StubEvent.Rotated).ShouldBe(1, "fixture check: GitLab rotated rt-0 and this process kept the pair it issued, a two-hour one"); + gitlab.ExpiresInSeconds = 900; + + await ReconnectAsync(world, world.NachoId, credentialId, "authz:nacho").ConfigureAwait(false); + + var reauthorized = await StoredPairAsync(credentialId).ConfigureAwait(false); + reauthorized.ExpiresAt.ShouldNotBeNull().ShouldBeLessThan(DateTimeOffset.UtcNow.AddMinutes(20), "fixture check: the new authorization's pair lives for fifteen minutes, shorter than the kept pair's two hours"); + + var token = await CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId).ConfigureAwait(false); + + Dump(gitlab); + token.ShouldBe(reauthorized.AccessToken, "the next run uses the new authorization, not the pair kept for the grant it replaced"); + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldBe(reauthorized.AccessToken, "and the kept pair was not stored over it"); + gitlab.Count(StubEvent.Rotated).ShouldBe(1, "nothing was refreshed after the re-authorization"); + } + + // ─── The provider that stops answering ─────────────────────────────────────────────────────────────────── + + [Fact] + public async Task A_provider_that_never_answers_costs_a_pasted_token_its_probe_deadline_and_changes_nothing() + { + // Asking about the new token is bounded as the sweep's and the add's probes are: a provider that accepts the connection and + // never replies costs this request the probe deadline, not a thread for the HTTP client's five minutes and three attempts. + using var gitlab = new GitLabDoorkeeperStub(); + using var silent = new SilentProviderHost(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var silentInstance = await SeedInstanceAtAsync(world, silent.BaseUrl).ConfigureAwait(false); + var credentialId = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-dead", status: CredentialStatus.Error, lastError: "token rejected by GitLab", instanceId: silentInstance).ConfigureAwait(false); + var before = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + using var scope = Scope(world, world.MayaId, b => b.RegisterInstance(new CredentialHealthDeadlines(Probe: TimeSpan.FromSeconds(1), Pass: TimeSpan.FromMinutes(1))).SingleInstance()); + + var replace = Task.Run(() => scope.Resolve().Send(new ReplaceCredentialTokenCommand { CredentialId = credentialId, Token = "glpat-fresh" })); + var finished = await Task.WhenAny(replace, Task.Delay(TimeSpan.FromSeconds(30))).ConfigureAwait(false); + + finished.ShouldBe(replace, "the replacement was still waiting on a silent provider after 30 seconds — nothing bounded it"); + var refusal = await Should.ThrowAsync(() => replace).ConfigureAwait(false); + + refusal.Message.ShouldContain("Could not validate the token against GitLab"); + refusal.Message.ShouldContain("did not answer within"); + silent.Connections.ShouldBe(1, "a call given up on at its deadline is not sent again"); + var after = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + after.EncryptedPayload.ShouldBe(before.EncryptedPayload, "no answer is not an acceptance: the row was not touched"); + after.ShouldSatisfyAllConditions(c => c.Status.ShouldBe(CredentialStatus.Error), c => c.LastError.ShouldBe("token rejected by GitLab")); + } + + // ─── Plumbing ────────────────────────────────────────────────────────────────────────────────────────── + + /// The credential as the Providers list reads it, with the health derived from its row. + private async Task ListedAsync(World world, Guid userId, Guid credentialId) + { + using var scope = Scope(world, userId); + var listed = await scope.Resolve().ListAsync(null, CancellationToken.None).ConfigureAwait(false); + + return listed.Single(c => c.Id == credentialId); + } + + /// + /// Stands between the sweep and the credential row: the first verdict it is handed is held until beforeFirstVerdict + /// has run — the person's replacement landing after GitLab answered about the old token and before the answer is written — + /// then goes through to the real writer, whose compare-and-swap is what decides it. Records each verdict and whether it landed. + /// + private sealed class InterleavingHealthWriter : ICredentialHealthWriter + { + private readonly Func _beforeFirstVerdict; + private readonly List _verdicts = new(); + private ICredentialHealthWriter _inner = default!; + private int _first; + + public InterleavingHealthWriter(Func beforeFirstVerdict) { _beforeFirstVerdict = beforeFirstVerdict; } + + public IReadOnlyList Verdicts { get { lock (_verdicts) { return _verdicts.ToList(); } } } + + public InterleavingHealthWriter Over(ICredentialHealthWriter inner) + { + _inner = inner; + + return this; + } + + public async Task MarkRejectedAsync(Guid credentialId, string expectedEncryptedPayload, string lastError, CancellationToken cancellationToken) + { + await InterleaveOnceAsync().ConfigureAwait(false); + + return Record("rejected", await _inner.MarkRejectedAsync(credentialId, expectedEncryptedPayload, lastError, cancellationToken).ConfigureAwait(false)); + } + + public async Task MarkValidatedAsync(Guid credentialId, string expectedEncryptedPayload, DateTimeOffset validatedAt, ExpiryRecord? expiry, CancellationToken cancellationToken) + { + await InterleaveOnceAsync().ConfigureAwait(false); + + return Record("validated", await _inner.MarkValidatedAsync(credentialId, expectedEncryptedPayload, validatedAt, expiry, cancellationToken).ConfigureAwait(false)); + } + + private async Task InterleaveOnceAsync() + { + if (Interlocked.Increment(ref _first) == 1) await _beforeFirstVerdict().ConfigureAwait(false); + } + + private bool Record(string kind, bool landed) + { + lock (_verdicts) _verdicts.Add(new Verdict(kind, landed)); + + return landed; + } + + public sealed record Verdict(string Kind, bool Landed); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.cs new file mode 100644 index 000000000..aa4178a44 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.cs @@ -0,0 +1,1212 @@ +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.Webhooks.Registration; +using CodeSpace.Core.Settings.Database; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Infrastructure.Jobs; +using CodeSpace.IntegrationTests.OAuth; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using CodeSpace.Messages.Failures; +using CodeSpace.StorageTestWorker; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Npgsql; +using Shouldly; +using Xunit.Abstractions; + +namespace CodeSpace.IntegrationTests.Credentials; + +/// +/// 🟢 High fidelity — a credential's token re-authorized IN PLACE, through the real mediator pipeline (the authorization +/// behaviors, the unit of work, the post-commit drain), the real OAuthFlowService and CredentialReauthorizer, +/// the real GitLab OAuth client and GitLab provider (the probe that says whose account signed in), the real refresher and +/// its Postgres advisory lock, against real Postgres. The one fake is GitLab, a : refresh +/// tokens are single use, a refresh kills the access token it replaces, an authorization code authz:<user> issues +/// that user a pair, and every user has an id of their own. +/// +/// The claim: a reconnect used to mint a NEW credential row, so everything bound through the old one — repositories, +/// group hooks, the act-as identity — pointed at a dead row until a succession subsystem moved it. Re-authorization writes the +/// new token into the SAME row, so nothing moves and nothing can be left behind: the credential id never changes, what the +/// dead token broke comes back, the replaced tokens are ended at the provider (those tokens, never the user's grant), another +/// account is refused, and a refresh that races it is serialised by the very lock the refresher takes. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public partial class CredentialReauthorizationFlowTests +{ + private static readonly TimeSpan Patience = TimeSpan.FromSeconds(30); + + private readonly PostgresFixture _fixture; + private readonly ITestOutputHelper _output; + + public CredentialReauthorizationFlowTests(PostgresFixture fixture, ITestOutputHelper output) + { + _fixture = fixture; + _output = output; + } + + // ─── The OAuth door: Reconnect ──────────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task Reconnecting_a_disconnected_credential_keeps_its_id_and_brings_back_everything_its_dead_token_broke() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", disconnected: true).ConfigureAwait(false); + var broken = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Error).ConfigureAwait(false); + var healthy = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Active).ConfigureAwait(false); + var paused = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Paused).ConfigureAwait(false); + var failedHook = await SeedHookAsync(world, credentialId, RepositoryWebhookRegistrationStatus.Failed).ConfigureAwait(false); + var buriedHook = await SeedHookAsync(world, credentialId, RepositoryWebhookRegistrationStatus.DeadLettered).ConfigureAwait(false); + var registeredHook = await SeedHookAsync(world, credentialId, RepositoryWebhookRegistrationStatus.Registered).ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho", deleted: true).ConfigureAwait(false); + + var finished = await ReconnectAsync(world, world.NachoId, credentialId, "authz:nacho").ConfigureAwait(false); + + finished.CredentialId.ShouldBe(credentialId, "the credential keeps its id"); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(1, "no second row was minted"); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.Status.ShouldBe(CredentialStatus.Active); + row.LastError.ShouldBeNull(); + row.LastValidatedDate.ShouldNotBeNull().ShouldBeGreaterThan(DateTimeOffset.UtcNow.AddMinutes(-2)); + row.DeletedDate.ShouldBeNull(); + row.AuthType.ShouldBe(AuthType.OAuth); + row.ExpiresDate.ShouldNotBeNull().ShouldBeGreaterThan(DateTimeOffset.UtcNow.AddHours(1)); + row.LastModifiedBy.ShouldBe(world.NachoId, "attributed to the person who reconnected, not to the anonymous callback"); + (await StoredPairAsync(credentialId).ConfigureAwait(false)).RefreshToken.ShouldBe(gitlab.LiveRefreshTokenOf("nacho"), "the row holds the pair GitLab issued for this sign-in"); + + (await LoadRepositoryAsync(broken).ConfigureAwait(false)).ShouldSatisfyAllConditions(r => r.CredentialId.ShouldBe(credentialId), r => r.Status.ShouldBe(RepositoryStatus.Active), r => r.LastError.ShouldBeNull()); + (await LoadRepositoryAsync(healthy).ConfigureAwait(false)).CredentialId.ShouldBe(credentialId); + (await LoadRepositoryAsync(paused).ConfigureAwait(false)).ShouldSatisfyAllConditions(r => r.CredentialId.ShouldBe(credentialId), r => r.Status.ShouldBe(RepositoryStatus.Paused, "a repository paused for its own reasons stays paused")); + + foreach (var hookId in new[] { failedHook, buriedHook }) + (await LoadHookAsync(hookId).ConfigureAwait(false)).ShouldSatisfyAllConditions(h => h.CredentialId.ShouldBe(credentialId), h => h.RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Enqueued, "revived to Pending, then handed to the registrar once the unit of work committed"), h => h.Attempts.ShouldBe(0), h => WasDispatched(h.Id).ShouldBeTrue()); + + (await LoadHookAsync(registeredHook).ConfigureAwait(false)).RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Registered, "a hook that works only needs a live credential for its next call"); + + var identity = (await LoadIdentitiesAsync(world.NachoId).ConfigureAwait(false)).Single(i => i.DeletedDate == null); + identity.CredentialId.ShouldBe(credentialId, "the Disconnect ended the owner's act-as identity; reconnecting links it again, to the same credential"); + identity.ProviderUserId.ShouldBe(gitlab.UserIdOf("nacho").ToString()); + } + + [Fact] + public async Task Reconnecting_a_failing_credential_replaces_its_pair_and_ends_only_the_tokens_it_replaced() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-0", "rt-0"); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", status: CredentialStatus.Error, lastError: "OAuth exchange rejected: invalid_grant").ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Active).ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + + await ReconnectAsync(world, world.NachoId, credentialId, "authz:nacho").ConfigureAwait(false); + + Dump(gitlab); + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.Status.ShouldBe(CredentialStatus.Active); + row.LastError.ShouldBeNull("the failure the row carried is cleared by a sign-in that works"); + + var stored = await StoredPairAsync(credentialId).ConfigureAwait(false); + stored.AccessToken.ShouldNotBe("at-0"); + gitlab.IsAccessTokenLive(stored.AccessToken).ShouldBeTrue("the new pair is the live one"); + gitlab.IsAccessTokenLive("at-0").ShouldBeFalse("the pair it replaced is ended at the provider"); + gitlab.IsRefreshTokenLive("rt-0").ShouldBeFalse(); + gitlab.Count(StubEvent.Revoked).ShouldBe(2, "the replaced refresh token and the replaced access token — and nothing else"); + gitlab.Count(StubEvent.Refused).ShouldBe(0); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).CredentialId.ShouldBe(credentialId); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(1); + (await CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId).ConfigureAwait(false)).ShouldBe(stored.AccessToken, "a run bound through the same repository resolves the new token, with no refresh"); + } + + [Fact] + public async Task The_tokens_a_reconnect_replaces_are_revoked_at_token_level_and_never_as_a_whole_grant() + { + // On GitHub a grant revoke ends every token the user holds under the OAuth App — the new one included, and the user's + // tokens in every other team connected through the same app — so a re-authorization must only ever end its own. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + var revoker = new RecordingGrantRevoker(() => StoredPairAsync(credentialId).GetAwaiter().GetResult().AccessToken); + + await ReconnectAsync(world, world.NachoId, credentialId, "authz:nacho", b => b.RegisterInstance(revoker).As()).ConfigureAwait(false); + + var revoked = revoker.Revocations.ShouldHaveSingleItem(); + revoked.WholeGrant.ShouldBeFalse("only the replaced tokens may be ended"); + revoked.Payload.AccessToken.ShouldBe("at-0"); + revoked.Payload.RefreshToken.ShouldBe("rt-0"); + revoked.StoredAccessTokenWhenAsked.ShouldBe((await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken, "asked only once the new pair is stored and the unit of work committed"); + } + + [Fact] + public async Task A_provider_that_fails_to_revoke_the_replaced_tokens_does_not_undo_the_reconnect() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Error).ConfigureAwait(false); + + await ReconnectAsync(world, world.NachoId, credentialId, "authz:nacho", b => b.RegisterInstance(new RecordingGrantRevoker { Fails = true }).As()).ConfigureAwait(false); + + (await LoadCredentialAsync(credentialId).ConfigureAwait(false)).Status.ShouldBe(CredentialStatus.Active); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).Status.ShouldBe(RepositoryStatus.Active); + } + + [Fact] + public async Task A_reconnect_does_not_carry_other_credentials_bindings_onto_the_one_it_re_authorizes() + { + // A plain connect claims every binding of the owner's disconnected credentials. Re-authorizing one credential is not a + // new connection: what hangs on a different dead credential stays there, for whoever relinks it deliberately. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", disconnected: true).ConfigureAwait(false); + var other = await SeedOAuthCredentialAsync(world, world.NachoId, "at-9", "rt-9", disconnected: true).ConfigureAwait(false); + var strandedOnOther = await SeedRepositoryAsync(world, other, RepositoryStatus.Error).ConfigureAwait(false); + + await ReconnectAsync(world, world.NachoId, credentialId, "authz:nacho").ConfigureAwait(false); + + (await LoadRepositoryAsync(strandedOnOther).ConfigureAwait(false)).ShouldSatisfyAllConditions(r => r.CredentialId.ShouldBe(other), r => r.Status.ShouldBe(RepositoryStatus.Error)); + } + + [Fact] + public async Task A_different_provider_account_is_refused_and_nothing_changes() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", disconnected: true).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Error).ConfigureAwait(false); + var hookId = await SeedHookAsync(world, credentialId, RepositoryWebhookRegistrationStatus.Failed).ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho", deleted: true).ConfigureAwait(false); + gitlab.UserIdOf("nacho"); + var before = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + + var refusal = await Should.ThrowAsync(() => ReconnectAsync(world, world.NachoId, credentialId, "authz:someone-else")).ConfigureAwait(false); + + refusal.Code.ShouldBe(FailureCodes.CredentialAccountMismatch); + refusal.ExpectedAccount.ShouldBe("nacho"); + refusal.PresentedAccount.ShouldBe("someone-else"); + refusal.Message.ShouldContain("connect someone-else as its own credential", Case.Insensitive, "the message says what to do instead"); + + var after = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + after.EncryptedPayload.ShouldBe(before.EncryptedPayload, "the row was not touched"); + after.Status.ShouldBe(CredentialStatus.Revoked); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).Status.ShouldBe(RepositoryStatus.Error); + (await LoadHookAsync(hookId).ConfigureAwait(false)).RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Failed); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(1, "a different account is a new connection, which this is not"); + (await LoadIdentitiesAsync(world.NachoId).ConfigureAwait(false)).ShouldAllBe(i => i.DeletedDate != null, "no identity was linked for the other account"); + } + + [Fact] + public async Task A_credential_with_no_account_on_record_takes_the_first_one_and_records_it() + { + // Nothing to compare: the credential was connected before identities were linked, or its identity link failed. Refusing + // would make a dead credential like that impossible to bring back; the identity linked now is what records the account. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", disconnected: true).ConfigureAwait(false); + + await ReconnectAsync(world, world.NachoId, credentialId, "authz:nacho").ConfigureAwait(false); + + var identity = (await LoadIdentitiesAsync(world.NachoId).ConfigureAwait(false)).ShouldHaveSingleItem(); + identity.ShouldSatisfyAllConditions(i => i.CredentialId.ShouldBe(credentialId), i => i.ProviderUsername.ShouldBe("nacho"), i => i.DeletedDate.ShouldBeNull()); + + var refusal = await Should.ThrowAsync(() => ReconnectAsync(world, world.NachoId, credentialId, "authz:someone-else")).ConfigureAwait(false); + refusal.ExpectedAccount.ShouldBe("nacho", "from now on the credential has an account on record"); + } + + [Fact] + public async Task A_teammate_with_authority_over_a_credential_re_authorizes_it_without_taking_over_its_identity() + { + // An Admin reconnects Maya's credential: the account that signs in must still be Maya's, and the act-as identity is + // linked only by the person it speaks for — Maya — never by whoever happened to click. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0", disconnected: true).ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya", deleted: true).ConfigureAwait(false); + + await Should.ThrowAsync(() => ReconnectAsync(world, world.BossId, credentialId, "authz:boss")).ConfigureAwait(false); + + var finished = await ReconnectAsync(world, world.BossId, credentialId, "authz:maya").ConfigureAwait(false); + + finished.CredentialId.ShouldBe(credentialId); + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.OwnerUserId.ShouldBe(world.MayaId, "the credential is still Maya's"); + row.LastModifiedBy.ShouldBe(world.BossId); + (await LoadIdentitiesAsync(world.MayaId).ConfigureAwait(false)).ShouldAllBe(i => i.DeletedDate != null, "Maya did not sign in, so nobody spoke for her"); + } + + // ─── Refusals at the start and at the end of a connect ─────────────────────────────────────────────────── + + [Fact] + public async Task A_credential_retired_as_a_departed_members_is_refused_at_the_start_and_stays_retired() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", disconnected: true, retired: true).ConfigureAwait(false); + + var refusal = await Should.ThrowAsync(() => InitAsync(world, world.NachoId, credentialId)).ConfigureAwait(false); + + refusal.Code.ShouldBe(FailureCodes.CredentialRetired); + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0, "a refused start leaves no pending connect behind"); + } + + [Fact] + public async Task A_credential_retired_while_the_connect_was_pending_is_refused_at_the_callback_and_not_brought_back() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + + await RetireAsync(credentialId).ConfigureAwait(false); + + await Should.ThrowAsync(() => CompleteAsync(world, init, "authz:nacho")).ConfigureAwait(false); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.EncryptedPayload.ShouldBeEmpty("a retired row keeps no token"); + row.DeletedDate.ShouldNotBeNull(); + row.Status.ShouldBe(CredentialStatus.Revoked); + } + + [Fact] + public async Task A_credential_whose_person_no_longer_stands_in_the_team_is_refused_before_the_sweep_gets_to_it() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0").ConfigureAwait(false); + var init = await InitAsync(world, world.BossId, credentialId).ConfigureAwait(false); + + await RemoveMembershipAsync(world, world.MayaId).ConfigureAwait(false); + + await Should.ThrowAsync(() => CompleteAsync(world, init, "authz:maya")).ConfigureAwait(false); + + (await LoadCredentialAsync(credentialId).ConfigureAwait(false)).EncryptedPayload.ShouldNotBeEmpty("it is the sweep's to retire, and this did not resurrect or rewrite it"); + gitlab.Count(StubEvent.Authorized).ShouldBe(0, "the owner's standing is asked before the code is exchanged, so no token was minted for someone without standing"); + } + + [Fact] + public async Task A_credential_of_another_team_reads_as_not_found_and_no_state_is_created() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var elsewhere = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var foreign = await SeedOAuthCredentialAsync(elsewhere, elsewhere.NachoId, "at-f", "rt-f").ConfigureAwait(false); + + await Should.ThrowAsync(() => InitAsync(world, world.NachoId, foreign)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); + (await StoredPairAsync(foreign).ConfigureAwait(false)).AccessToken.ShouldBe("at-f"); + } + + [Fact] + public async Task A_credential_of_another_provider_instance_a_team_service_credential_and_a_name_that_is_not_the_owner_are_refused_at_the_start() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var otherInstance = await SeedInstanceAsync(world, gitlab, clientId: "another-app").ConfigureAwait(false); + var onOtherInstance = await SeedOAuthCredentialAsync(world, world.NachoId, "at-o", "rt-o", instanceId: otherInstance).ConfigureAwait(false); + var teamToken = await SeedOAuthCredentialAsync(world, null, "at-t", "rt-t", ownership: CredentialOwnership.TeamService).ConfigureAwait(false); + var teamTokenWithAnOwnerRecorded = await SeedOAuthCredentialAsync(world, world.NachoId, "at-u", "rt-u", ownership: CredentialOwnership.TeamService).ConfigureAwait(false); + var nachos = await SeedOAuthCredentialAsync(world, world.NachoId, "at-n", "rt-n").ConfigureAwait(false); + + await Should.ThrowAsync(() => InitAsync(world, world.NachoId, onOtherInstance)).ConfigureAwait(false); + await Should.ThrowAsync(() => InitAsync(world, world.NachoId, teamToken)).ConfigureAwait(false); + await Should.ThrowAsync(() => InitAsync(world, world.NachoId, teamTokenWithAnOwnerRecorded)).ConfigureAwait(false); + await Should.ThrowAsync(() => InitAsync(world, world.NachoId, nachos, intendedOwnerUserId: world.MayaId)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); + } + + [Fact] + public async Task Starting_a_reconnect_takes_credentials_manage_whoever_the_owner_is() + { + // The start of any OAuth connect is the Admin's (the marker on the command); naming a credential does not lower it. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var mayas = await SeedOAuthCredentialAsync(world, world.MayaId, "at-m", "rt-m").ConfigureAwait(false); + + await Should.ThrowAsync(() => InitAsync(world, world.MayaId, mayas)).ConfigureAwait(false); + await Should.ThrowAsync(() => InitAsync(world, world.ViewerId, mayas)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); + } + + // ─── Mixed versions ───────────────────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task A_pending_connect_an_older_pod_wrote_without_the_column_completes_as_a_new_credential_exactly_as_it_always_did() + { + // A pod from before migration 0250 inserts the row with the columns it knows — no replace_credential_id — so the + // column is NULL, which is a plain connect for every version. Written as that pod writes it, not through this model. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var existing = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", disconnected: true).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, existing, RepositoryStatus.Error).ConfigureAwait(false); + var nonce = OAuthBrowserNonce.Mint(); + var state = $"old-pod-{Guid.NewGuid():N}"; + + await InsertPreMigrationStateAsync(world, state, world.NachoId, OAuthBrowserNonce.Hash(nonce)).ConfigureAwait(false); + + var finished = await CompleteAsync(world, new InitCredentialOAuthResult { AuthorizeUrl = new Uri("https://unused.test/"), State = state, BrowserNonce = nonce, Lifetime = TimeSpan.FromMinutes(10), BrowserNonceCookiePath = "/" }, "authz:nacho").ConfigureAwait(false); + + finished.CredentialId.ShouldNotBe(existing, "no replace id, so a connect mints a new credential"); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(2); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).CredentialId.ShouldBe(finished.CredentialId, "and the succession that always followed a connect moved the repository onto it"); + } + + [Fact] + public async Task A_connect_that_names_no_credential_is_the_connect_it_always_was() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var existing = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", disconnected: true).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, existing, RepositoryStatus.Error).ConfigureAwait(false); + + var init = await InitAsync(world, world.NachoId, replaceCredentialId: null).ConfigureAwait(false); + (await LoadPendingStateAsync(init.State).ConfigureAwait(false)).ReplaceCredentialId.ShouldBeNull(); + + var finished = await CompleteAsync(world, init, "authz:nacho").ConfigureAwait(false); + + finished.CredentialId.ShouldNotBe(existing); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).CredentialId.ShouldBe(finished.CredentialId); + } + + // ─── The pasted-token door: Replace token ──────────────────────────────────────────────────────────────── + + [Fact] + public async Task Replacing_a_token_keeps_the_credential_and_brings_back_what_its_dead_token_broke() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-expired", status: CredentialStatus.Error, lastError: "401 Unauthorized").ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Error).ConfigureAwait(false); + var hookId = await SeedHookAsync(world, credentialId, RepositoryWebhookRegistrationStatus.DeadLettered).ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("maya", "glpat-fresh"); + + var result = await ReplaceTokenAsync(world, world.MayaId, credentialId, "glpat-fresh").ConfigureAwait(false); + + result.CredentialId.ShouldBe(credentialId); + result.ProviderUsername.ShouldBe("maya"); + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.ShouldSatisfyAllConditions(r => r.Status.ShouldBe(CredentialStatus.Active), r => r.LastError.ShouldBeNull(), r => r.AuthType.ShouldBe(AuthType.Pat), r => r.ExpiresDate.ShouldBeNull(), r => r.LastValidatedDate.ShouldNotBeNull(), r => r.LastModifiedBy.ShouldBe(world.MayaId)); + row.Scopes.ShouldBe(GitLabDoorkeeperStub.PersonalAccessTokenScopes.Split(','), ignoreOrder: false, customMessage: "the token's real scopes, as the provider reports them"); + (await TokenOfAsync(credentialId).ConfigureAwait(false)).ShouldBe("glpat-fresh"); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).ShouldSatisfyAllConditions(r => r.CredentialId.ShouldBe(credentialId), r => r.Status.ShouldBe(RepositoryStatus.Active)); + (await LoadHookAsync(hookId).ConfigureAwait(false)).RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Enqueued); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(1); + gitlab.Count(StubEvent.Revoked).ShouldBe(0, "a personal access token is not something CodeSpace can revoke at the provider"); + } + + [Fact] + public async Task A_token_replaces_an_oauth_sign_in_in_place_and_the_replaced_oauth_tokens_are_ended() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "maya", "at-0", "rt-0"); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0").ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("maya", "glpat-fresh"); + + await ReplaceTokenAsync(world, world.MayaId, credentialId, "glpat-fresh").ConfigureAwait(false); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.AuthType.ShouldBe(AuthType.Pat, "bindings reference only the id, so the kind of token behind it may change"); + row.ExpiresDate.ShouldBeNull("an OAuth expiry must not outlive the OAuth token it described"); + (await TokenOfAsync(credentialId).ConfigureAwait(false)).ShouldBe("glpat-fresh"); + gitlab.IsAccessTokenLive("at-0").ShouldBeFalse("the OAuth pair the token replaced is ended at the provider"); + (await LoadIdentitiesAsync(world.MayaId).ConfigureAwait(false)).ShouldHaveSingleItem().CredentialId.ShouldBe(credentialId); + } + + [Fact] + public async Task A_replacement_token_the_provider_rejects_changes_nothing() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-expired", status: CredentialStatus.Error, lastError: "401 Unauthorized").ConfigureAwait(false); + var before = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + + var refusal = await Should.ThrowAsync(() => ReplaceTokenAsync(world, world.MayaId, credentialId, "glpat-typo")).ConfigureAwait(false); + + refusal.Message.ShouldContain("Could not validate the token against GitLab"); + var after = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + after.EncryptedPayload.ShouldBe(before.EncryptedPayload); + after.Status.ShouldBe(CredentialStatus.Error); + after.LastError.ShouldBe("401 Unauthorized"); + } + + [Fact] + public async Task A_replacement_token_of_another_account_is_refused() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-expired", status: CredentialStatus.Error).ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("nacho", "glpat-nachos"); + + var refusal = await Should.ThrowAsync(() => ReplaceTokenAsync(world, world.MayaId, credentialId, "glpat-nachos")).ConfigureAwait(false); + + refusal.ExpectedAccount.ShouldBe("maya"); + refusal.PresentedAccount.ShouldBe("nacho"); + (await TokenOfAsync(credentialId).ConfigureAwait(false)).ShouldBe("glpat-expired"); + } + + [Fact] + public async Task A_team_service_token_is_replaced_by_another_token_of_its_kind_whoever_it_belongs_to() + { + // A group access token's account is a bot that a new token for the same group does not share, so there is no person's + // account to compare — and it stays a group access token, owned by the team. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedTokenCredentialAsync(world, null, "glpat-group-old", authType: AuthType.GroupAccessToken, ownership: CredentialOwnership.TeamService, status: CredentialStatus.Error).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Error).ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("group_42_bot", "glpat-group-new"); + + await ReplaceTokenAsync(world, world.BossId, credentialId, "glpat-group-new").ConfigureAwait(false); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.ShouldSatisfyAllConditions(r => r.AuthType.ShouldBe(AuthType.GroupAccessToken), r => r.Ownership.ShouldBe(CredentialOwnership.TeamService), r => r.OwnerUserId.ShouldBeNull(), r => r.Status.ShouldBe(CredentialStatus.Active)); + (await TokenOfAsync(credentialId).ConfigureAwait(false)).ShouldBe("glpat-group-new"); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).Status.ShouldBe(RepositoryStatus.Active); + } + + [Fact] + public async Task Only_the_owner_of_a_personal_credential_or_someone_who_manages_credentials_may_replace_its_token() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-expired", status: CredentialStatus.Error).ConfigureAwait(false); + var otherMember = await SeedMemberAsync(world, TeamRole.Member).ConfigureAwait(false); + var outsider = await SeedUserAsync().ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("maya", "glpat-fresh"); + + await Should.ThrowAsync(() => ReplaceTokenAsync(world, otherMember, credentialId, "glpat-fresh")).ConfigureAwait(false); + await Should.ThrowAsync(() => ReplaceTokenAsync(world, world.ViewerId, credentialId, "glpat-fresh")).ConfigureAwait(false); + await Should.ThrowAsync(() => ReplaceTokenAsync(world, outsider, credentialId, "glpat-fresh")).ConfigureAwait(false); + (await TokenOfAsync(credentialId).ConfigureAwait(false)).ShouldBe("glpat-expired", "every refusal left the credential as it was"); + + await ReplaceTokenAsync(world, world.BossId, credentialId, "glpat-fresh").ConfigureAwait(false); + + (await TokenOfAsync(credentialId).ConfigureAwait(false)).ShouldBe("glpat-fresh"); + } + + [Fact] + public async Task A_token_is_refused_for_a_credential_whose_person_no_longer_stands_in_the_team() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-expired", status: CredentialStatus.Error).ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("maya", "glpat-fresh"); + + await RemoveMembershipAsync(world, world.MayaId).ConfigureAwait(false); + + var refusal = await Should.ThrowAsync(() => ReplaceTokenAsync(world, world.BossId, credentialId, "glpat-fresh")).ConfigureAwait(false); + + refusal.Message.ShouldContain("standing"); + (await TokenOfAsync(credentialId).ConfigureAwait(false)).ShouldBe("glpat-expired", "the departed-token sweep retires it; a token written into it now would be retired with it"); + } + + [Fact] + public async Task A_credential_of_another_team_is_not_reachable_even_by_an_instance_admin_working_in_this_one() + { + // The credential marker lets an instance Admin through on role alone, so the team they work in is checked by the service. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var elsewhere = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var foreign = await SeedTokenCredentialAsync(elsewhere, elsewhere.MayaId, "glpat-foreign").ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("maya", "glpat-fresh"); + var instanceAdmin = await SeedUserAsync().ConfigureAwait(false); + + await Should.ThrowAsync(() => ReplaceTokenAsync(world, instanceAdmin, foreign, "glpat-fresh", Roles.Admin)).ConfigureAwait(false); + + (await TokenOfAsync(foreign).ConfigureAwait(false)).ShouldBe("glpat-foreign"); + } + + [Fact] + public async Task A_retired_credential_cannot_take_a_token_and_a_blank_token_is_refused_before_anything_is_asked() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var retired = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-old", status: CredentialStatus.Revoked, retired: true).ConfigureAwait(false); + var live = await SeedTokenCredentialAsync(world, world.MayaId, "glpat-live").ConfigureAwait(false); + var instanceAdmin = await SeedUserAsync().ConfigureAwait(false); + gitlab.SeedPersonalAccessToken("maya", "glpat-fresh"); + + // The credential marker already reads a retired credential as not accessible to anyone on the team; an instance Admin + // passes it by role and reaches the service, which says what it is. + await Should.ThrowAsync(() => ReplaceTokenAsync(world, world.BossId, retired, "glpat-fresh")).ConfigureAwait(false); + var refusal = await Should.ThrowAsync(() => ReplaceTokenAsync(world, instanceAdmin, retired, "glpat-fresh", Roles.Admin)).ConfigureAwait(false); + await Should.ThrowAsync(() => ReplaceTokenAsync(world, world.MayaId, live, " ")).ConfigureAwait(false); + + refusal.Code.ShouldBe(FailureCodes.CredentialRetired); + (await LoadCredentialAsync(retired).ConfigureAwait(false)).EncryptedPayload.ShouldBeEmpty(); + (await TokenOfAsync(live).ConfigureAwait(false)).ShouldBe("glpat-live"); + gitlab.Count(StubEvent.Api200).ShouldBe(0, "a refused replacement asks the provider nothing"); + } + + // ─── Concurrency: the refresh lock ─────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task A_refresh_that_arrives_while_a_re_authorization_is_writing_waits_and_then_finds_the_new_pair_and_does_not_rotate_it() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-0", "rt-0"); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", expired: true).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Active).ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + var writer = new GatedReauthorizationWriter(); + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + + var reconnect = Task.Run(() => CompleteAsync(world, init, "authz:nacho", b => b.Register(c => writer.Over(new CredentialReauthorizationWriter(c.Resolve()))).As().InstancePerLifetimeScope())); + await WithinPatienceAsync(writer.Entered, "the re-authorization reached its write", credentialId).ConfigureAwait(false); + + var refresh = Task.Run(() => CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId)); + await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false); + + refresh.IsCompleted.ShouldBeFalse("the refresher takes the same lock the re-authorization holds, so it must wait for the write"); + gitlab.Count(StubEvent.Rotated).ShouldBe(0, "a refresh that rotated rt-0 now would have consumed the pair being replaced"); + + writer.Release(); + await WithinPatienceAsync(reconnect, "the re-authorization finished once released", credentialId).ConfigureAwait(false); + var token = await WithinPatienceAsync(refresh, "the refresh that waited for it finished", credentialId).ConfigureAwait(false); + + Dump(gitlab); + var stored = await StoredPairAsync(credentialId).ConfigureAwait(false); + token.ShouldBe(stored.AccessToken, "the refresher read the row after the lock and found the new pair, fresh"); + gitlab.Count(StubEvent.Rotated).ShouldBe(0); + gitlab.Count(StubEvent.Refused).ShouldBe(0); + gitlab.IsAccessTokenLive(stored.AccessToken).ShouldBeTrue(); + } + + [Fact] + public async Task A_re_authorization_that_arrives_while_a_refresh_is_at_the_provider_waits_and_replaces_the_pair_the_refresh_stored() + { + using var gitlab = new GitLabDoorkeeperStub { RotateReplyDelay = TimeSpan.FromSeconds(3) }; + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-0", "rt-0"); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", expired: true).ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Active).ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + + var refresh = Task.Run(() => CredentialRefreshHarness.ResolveTokenAsync(_fixture, repositoryId)); + await WaitForAsync(() => gitlab.Count(StubEvent.Rotated) == 1, "GitLab rotated rt-0 for the refresher").ConfigureAwait(false); + + var reconnect = Task.Run(() => CompleteAsync(world, init, "authz:nacho")); + await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false); + + reconnect.IsCompleted.ShouldBeFalse("the re-authorization waits for the lock the refresher holds while its pair is at the provider"); + + var rotatedToken = await WithinPatienceAsync(refresh, "the refresh that held the lock finished", credentialId).ConfigureAwait(false); + await WithinPatienceAsync(reconnect, "the re-authorization that waited for it finished", credentialId).ConfigureAwait(false); + + Dump(gitlab); + var stored = await StoredPairAsync(credentialId).ConfigureAwait(false); + stored.AccessToken.ShouldNotBe(rotatedToken, "the re-authorization wrote over the pair the refresh had just stored — and read it first, so it replaced what was really there"); + gitlab.Count(StubEvent.Refused).ShouldBe(0, "nobody presented a consumed refresh token"); + gitlab.IsAccessTokenLive(rotatedToken).ShouldBeFalse("the pair the refresh stored is the one replaced, so it is the one ended at the provider"); + gitlab.IsAccessTokenLive(stored.AccessToken).ShouldBeTrue(); + } + + [Fact] + public async Task A_re_authorization_waits_for_the_advisory_lock_another_pod_holds_as_well_as_for_this_processs_own() + { + // The in-process semaphore only excludes this pod. Another pod's refresher holds the Postgres advisory lock for the + // credential while its pair is at the provider, and the key must be the one the refresher derives — the previous build + // derives it the same way — or a re-authorization on this pod would write over a rotation on that one. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + IAsyncDisposable otherPod; + + using (var holder = _fixture.BeginScope()) + otherPod = await holder.Resolve().AcquireAsync(CredentialRefreshLock.KeyOf(credentialId), CancellationToken.None).ConfigureAwait(false); + + var reconnect = Task.Run(() => CompleteAsync(world, init, "authz:nacho")); + await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false); + + try + { + reconnect.IsCompleted.ShouldBeFalse("another pod holds the credential's advisory lock, so this re-authorization must wait for it"); + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldBe("at-0", "nothing was written while the lock was held elsewhere"); + } + finally + { + await otherPod.DisposeAsync().ConfigureAwait(false); + } + + await WithinPatienceAsync(reconnect, "the re-authorization finished once the other pod let go", credentialId).ConfigureAwait(false); + + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldNotBe("at-0"); + } + + [Theory] + [InlineData(PodBuild.This)] + [InlineData(PodBuild.Previous)] + public async Task A_re_authorization_waits_for_a_refresh_running_in_another_process_of_either_build_and_then_replaces_the_pair_it_stored(PodBuild build) + { + // A rolling deploy or a rollback runs pods of this build beside pods of the one before it. The previous build's refresher + // (pinned in CodeSpace.StorageTestWorker) takes the advisory lock on the same key, so a re-authorization on this build + // cannot write over a rotation it has under way — the one case an in-process test cannot make. + using var gitlab = new GitLabDoorkeeperStub { RotateReplyDelay = TimeSpan.FromSeconds(4) }; + using var pods = new RefreshPodHarness(_fixture.ConnectionString); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-0", "rt-0"); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-new", "rt-new"); + var credentialId = await pods.SeedCredentialAsync(gitlab.BaseUrl, "at-0", "rt-0").ConfigureAwait(false); + var (teamId, instanceId) = await TeamAndInstanceOfAsync(credentialId).ConfigureAwait(false); + + var podRun = Task.Run(() => pods.RunAsync(credentialId, new PodSpec("pod", build, 1))); + await WaitForAsync(() => gitlab.Count(StubEvent.Rotated) == 1, $"GitLab rotated rt-0 for the {build} pod, which now holds the credential's lock while its answer is delayed").ConfigureAwait(false); + + var reauthorization = Task.Run(() => ReauthorizeDirectlyAsync(credentialId, teamId, instanceId, new OAuthPayload { AccessToken = "at-new", RefreshToken = "rt-new", ExpiresAt = DateTimeOffset.UtcNow.AddHours(2) })); + await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false); + + reauthorization.IsCompleted.ShouldBeFalse($"the {build} pod holds the credential's advisory lock while its pair is at the provider"); + + var callers = await WithinPatienceAsync(podRun, $"the {build} pod finished its refresh", credentialId).ConfigureAwait(false); + await WithinPatienceAsync(reauthorization, "the re-authorization finished once the pod let go", credentialId).ConfigureAwait(false); + + Dump(gitlab); + callers.ShouldAllBe(c => c.Ok, string.Join(" | ", callers.Select(c => c.ToString()))); + gitlab.Count(StubEvent.Refused).ShouldBe(0, $"nobody presented a consumed refresh token: {string.Join(", ", gitlab.RefusedRefreshTokens())}"); + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldBe("at-new", "the re-authorization wrote last, over the pair the pod's refresh had stored"); + } + + [Fact] + public async Task Two_reconnects_racing_leave_the_credential_holding_one_live_pair_and_end_the_other() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + gitlab.Seed(GitLabDoorkeeperStub.DefaultClientId, "nacho", "at-0", "rt-0"); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + await SeedIdentityAsync(world, world.NachoId, credentialId, "nacho").ConfigureAwait(false); + var first = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + var second = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + + await WithinPatienceAsync(Task.WhenAll(Task.Run(() => CompleteAsync(world, first, "authz:nacho")), Task.Run(() => CompleteAsync(world, second, "authz:nacho"))), "both racing re-authorizations finished", credentialId).ConfigureAwait(false); + + Dump(gitlab); + var stored = await StoredPairAsync(credentialId).ConfigureAwait(false); + gitlab.IsAccessTokenLive(stored.AccessToken).ShouldBeTrue("what the row holds is live"); + gitlab.Rows.Count(r => r.User == "nacho" && r.RevokedAt == null).ShouldBe(1, "the seeded pair and the pair the loser wrote were each replaced, and so ended; one pair is live and it is the row's"); + gitlab.Count(StubEvent.Authorized).ShouldBe(2); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(1); + (await LoadIdentitiesAsync(world.NachoId).ConfigureAwait(false)).Count(i => i.DeletedDate == null).ShouldBe(1, "one live act-as identity, as the unique index requires"); + } + + [Fact] + public async Task A_retirement_that_lands_while_the_new_pair_is_being_written_is_not_undone() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Error).ConfigureAwait(false); + var writer = new GatedReauthorizationWriter(); + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + + var reconnect = Task.Run(() => CompleteAsync(world, init, "authz:nacho", b => b.Register(c => writer.Over(new CredentialReauthorizationWriter(c.Resolve()))).As().InstancePerLifetimeScope())); + await WithinPatienceAsync(writer.Entered, "the re-authorization reached its write", credentialId).ConfigureAwait(false); + + await RetireAsync(credentialId).ConfigureAwait(false); + writer.Release(); + + await Should.ThrowAsync(() => WithinPatienceAsync(reconnect, "the re-authorization finished once released", credentialId)).ConfigureAwait(false); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.EncryptedPayload.ShouldBeEmpty("the compare-and-swap found the row retired and wrote nothing"); + row.DeletedDate.ShouldNotBeNull(); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).Status.ShouldBe(RepositoryStatus.Error, "nothing was revived for a credential that stays retired"); + } + + [Fact] + public async Task A_retirement_that_lands_on_a_row_a_disconnect_had_already_blanked_is_not_undone() + { + // The case the payload comparison cannot see: a Disconnect leaves the row blank but listed, so the re-authorization + // reads '' and the write expects ''. A retirement that lands in between blanks nothing further — only the soft-delete + // tells the write the row is gone, and that is the one thing standing between it and a live token in a retired row. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0", disconnected: true).ConfigureAwait(false); + var writer = new GatedReauthorizationWriter(); + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + + var reconnect = Task.Run(() => CompleteAsync(world, init, "authz:nacho", b => b.Register(c => writer.Over(new CredentialReauthorizationWriter(c.Resolve()))).As().InstancePerLifetimeScope())); + await WithinPatienceAsync(writer.Entered, "the re-authorization reached its write", credentialId).ConfigureAwait(false); + + await RetireAsync(credentialId).ConfigureAwait(false); + writer.Release(); + + await Should.ThrowAsync(() => WithinPatienceAsync(reconnect, "the re-authorization finished once released", credentialId)).ConfigureAwait(false); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.EncryptedPayload.ShouldBeEmpty("the row was already blank, so only the soft-delete could refuse the write — and it did"); + row.DeletedDate.ShouldNotBeNull(); + row.Status.ShouldBe(CredentialStatus.Revoked); + } + + [Fact] + public async Task A_disconnect_that_blanks_the_row_while_the_new_pair_is_being_written_is_read_again_and_the_write_lands() + { + // A Disconnect does not take the refresh lock. It blanks the payload between the read and the write; the write is a + // compare-and-swap on what was read, so it finds the row changed, reads it again — blank — and lands. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.NachoId, "at-0", "rt-0").ConfigureAwait(false); + var interfering = new GatedReauthorizationWriter { BeforeFirstWrite = () => DisconnectBehindItsBackAsync(credentialId) }; + var init = await InitAsync(world, world.NachoId, credentialId).ConfigureAwait(false); + + var finished = await CompleteAsync(world, init, "authz:nacho", b => b.Register(c => interfering.Over(new CredentialReauthorizationWriter(c.Resolve()))).As().InstancePerLifetimeScope()).ConfigureAwait(false); + + finished.CredentialId.ShouldBe(credentialId); + interfering.Writes.ShouldBe(2, "the first write lost to the Disconnect, the second landed"); + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.Status.ShouldBe(CredentialStatus.Active); + (await StoredPairAsync(credentialId).ConfigureAwait(false)).AccessToken.ShouldBe(gitlab.LiveAccessTokenOf("nacho")); + gitlab.Count(StubEvent.Revoked).ShouldBe(0, "the row held nothing when it was replaced, so there was nothing to end"); + } + + // ─── Plumbing ────────────────────────────────────────────────────────────────────────────────────────── + + private sealed record World(Guid TeamId, Guid InstanceId, Guid NachoId, Guid MayaId, Guid BossId, Guid ViewerId, GitLabDoorkeeperStub GitLab); + + private async Task SeedWorldAsync(GitLabDoorkeeperStub gitlab) + { + var suffix = Guid.NewGuid().ToString("N")[..8]; + var team = new Team { Id = Guid.NewGuid(), Slug = $"t-{suffix}", Name = "Team" }; + var users = new[] { "nacho", "maya", "boss", "viewer" }.Select(label => new User { Id = Guid.NewGuid(), Email = $"{label}-{suffix}@x", Name = $"{label}-{suffix}" }).ToArray(); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.Team.Add(team); + db.User.AddRange(users); + db.TeamMembership.AddRange(Membership(team.Id, users[0].Id, TeamRole.Admin), Membership(team.Id, users[1].Id, TeamRole.Member), Membership(team.Id, users[2].Id, TeamRole.Admin), Membership(team.Id, users[3].Id, TeamRole.Viewer)); + await db.SaveChangesAsync().ConfigureAwait(false); + + var world = new World(team.Id, Guid.Empty, users[0].Id, users[1].Id, users[2].Id, users[3].Id, gitlab); + + return world with { InstanceId = await SeedInstanceAsync(world, gitlab).ConfigureAwait(false) }; + } + + /// A GitLab instance whose OAuth app is the stub's — or, named, another app on the same host, which is what lets one team hold two instances of one provider. + private async Task SeedInstanceAsync(World world, GitLabDoorkeeperStub gitlab, string clientId = GitLabDoorkeeperStub.DefaultClientId) => await SeedInstanceAtAsync(world, gitlab.BaseUrl, clientId).ConfigureAwait(false); + + /// The same instance at any address — a provider that has stopped answering is one too. + private async Task SeedInstanceAtAsync(World world, string baseUrl, string clientId = GitLabDoorkeeperStub.DefaultClientId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = world.TeamId, Provider = ProviderKind.GitLab, DisplayName = $"gitlab-{Guid.NewGuid():N}"[..14], BaseUrl = baseUrl, ApiUrl = baseUrl, OauthClientId = clientId, OauthClientSecretEnc = scope.Resolve().Encrypt(GitLabDoorkeeperStub.DefaultClientSecret) }; + + db.ProviderInstance.Add(instance); + await db.SaveChangesAsync().ConfigureAwait(false); + + return instance.Id; + } + + private static TeamMembership Membership(Guid teamId, Guid userId, TeamRole role) => new() { Id = Guid.NewGuid(), TeamId = teamId, UserId = userId, Role = role }; + + /// + /// An OAuth credential as the connect that made it leaves it. is what a Disconnect leaves: + /// Revoked, the payload blanked, the row still listed; is what a departure or a connection + /// delete leaves on top of that — soft-deleted. + /// + private async Task SeedOAuthCredentialAsync(World world, Guid? ownerId, string accessToken, string refreshToken, bool disconnected = false, bool retired = false, bool expired = false, CredentialStatus status = CredentialStatus.Active, string? lastError = null, CredentialOwnership ownership = CredentialOwnership.Personal, Guid? instanceId = null) + { + using var scope = _fixture.BeginScope(); + var payload = new OAuthPayload { AccessToken = accessToken, RefreshToken = refreshToken, ExpiresAt = expired ? DateTimeOffset.UtcNow.AddMinutes(-1) : DateTimeOffset.UtcNow.AddHours(2) }; + var sealedPayload = scope.Resolve().Encrypt(scope.Resolve().Serialize(payload)); + + return await InsertCredentialAsync(scope, world, ownerId, AuthType.OAuth, sealedPayload, payload.ExpiresAt, disconnected || retired, retired, status, lastError, ownership, instanceId).ConfigureAwait(false); + } + + private async Task SeedTokenCredentialAsync(World world, Guid? ownerId, string token, AuthType authType = AuthType.Pat, CredentialOwnership ownership = CredentialOwnership.Personal, CredentialStatus status = CredentialStatus.Active, string? lastError = null, bool retired = false, DateTimeOffset? expires = null, Guid? instanceId = null) + { + using var scope = _fixture.BeginScope(); + CredentialPayload payload = authType == AuthType.GroupAccessToken ? new GroupAccessTokenPayload { Token = token } : new PatPayload { Token = token }; + var sealedPayload = scope.Resolve().Encrypt(scope.Resolve().Serialize(payload)); + + return await InsertCredentialAsync(scope, world, ownerId, authType, retired ? string.Empty : sealedPayload, expires, retired, retired, status, lastError, ownership, instanceId).ConfigureAwait(false); + } + + private static async Task InsertCredentialAsync(ILifetimeScope scope, World world, Guid? ownerId, AuthType authType, string encryptedPayload, DateTimeOffset? expires, bool blank, bool retired, CredentialStatus status, string? lastError, CredentialOwnership ownership, Guid? instanceId) + { + var db = scope.Resolve(); + var actor = ownerId ?? world.BossId; + + var credential = new Credential + { + Id = Guid.NewGuid(), + TeamId = world.TeamId, + ProviderInstanceId = instanceId ?? world.InstanceId, + OwnerUserId = ownerId, + Ownership = ownership, + AuthType = authType, + DisplayName = ownerId == world.MayaId ? "GitLab · maya" : "GitLab · nacho", + EncryptedPayload = blank ? string.Empty : encryptedPayload, + ExpiresDate = expires, + Status = blank ? CredentialStatus.Revoked : status, + LastError = lastError, + DeletedDate = retired ? DateTimeOffset.UtcNow.AddMinutes(-5) : null, + CreatedBy = actor, + LastModifiedBy = actor + }; + + db.Credential.Add(credential); + await db.SaveChangesAsync().ConfigureAwait(false); + + return credential.Id; + } + + private async Task SeedIdentityAsync(World world, Guid userId, Guid credentialId, string account, bool deleted = false) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.UserProviderIdentity.Add(new UserProviderIdentity { Id = Guid.NewGuid(), UserId = userId, ProviderInstanceId = world.InstanceId, CredentialId = credentialId, ProviderUserId = world.GitLab.UserIdOf(account).ToString(), ProviderUsername = account, DeletedDate = deleted ? DateTimeOffset.UtcNow.AddMinutes(-5) : null }); + await db.SaveChangesAsync().ConfigureAwait(false); + } + + private async Task SeedRepositoryAsync(World world, Guid credentialId, RepositoryStatus status) + { + var name = $"repo-{Guid.NewGuid():N}"[..13]; + var repository = new Repository { Id = Guid.NewGuid(), TeamId = world.TeamId, ProviderInstanceId = InstanceOf(world, credentialId), CredentialId = credentialId, ExternalId = $"acme/{name}", NamespacePath = "acme", Name = name, FullPath = $"acme/{name}", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = $"{world.GitLab.BaseUrl}/acme/{name}", CloneUrlHttps = $"{world.GitLab.BaseUrl}/acme/{name}.git", Status = status, LastError = status == RepositoryStatus.Error ? CredentialRevocationService.DisconnectedRepositoryError : null }; + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.Repository.Add(repository); + await db.SaveChangesAsync().ConfigureAwait(false); + + return repository.Id; + } + + private Guid InstanceOf(World world, Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + return scope.Resolve().Credential.AsNoTracking().Where(c => c.Id == credentialId).Select(c => c.ProviderInstanceId).Single(); + } + + private async Task SeedHookAsync(World world, Guid credentialId, RepositoryWebhookRegistrationStatus status) + { + var id = Guid.NewGuid(); + var failed = status is RepositoryWebhookRegistrationStatus.Failed or RepositoryWebhookRegistrationStatus.DeadLettered; + var registered = status == RepositoryWebhookRegistrationStatus.Registered; + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.ConnectionWebhook.Add(new ConnectionWebhook { Id = id, ProviderInstanceId = world.InstanceId, CredentialId = credentialId, OwnerPath = $"acme-{id:N}"[..13], CallbackUrl = $"https://codespace.test/api/webhooks/connection/{id}", SecretEnc = scope.Resolve().Encrypt("hook-secret"), SubscribedEvents = new List { "push" }, RegistrationStatus = status, Attempts = 3, NextAttemptAt = DateTimeOffset.UtcNow.AddHours(1), LastError = failed ? "401 Unauthorized" : null, ExternalId = registered ? "4242" : null, RegisteredAt = registered ? DateTimeOffset.UtcNow.AddDays(-20) : null }); + await db.SaveChangesAsync().ConfigureAwait(false); + + return id; + } + + private async Task SeedMemberAsync(World world, TeamRole role) + { + var userId = await SeedUserAsync().ConfigureAwait(false); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.TeamMembership.Add(Membership(world.TeamId, userId, role)); + await db.SaveChangesAsync().ConfigureAwait(false); + + return userId; + } + + private async Task SeedUserAsync() + { + var suffix = Guid.NewGuid().ToString("N")[..8]; + var user = new User { Id = Guid.NewGuid(), Email = $"u-{suffix}@x", Name = $"u-{suffix}" }; + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + db.User.Add(user); + await db.SaveChangesAsync().ConfigureAwait(false); + + return user.Id; + } + + private async Task RemoveMembershipAsync(World world, Guid userId) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().TeamMembership.Where(m => m.TeamId == world.TeamId && m.UserId == userId).ExecuteDeleteAsync().ConfigureAwait(false); + } + + /// A scope for one request as in the world's team — anonymous when null, as the provider's callback is. Everything but GitLab is production's. + private ILifetimeScope Scope(World world, Guid? userId, Action? configure = null, params string[] roles) => _fixture.BeginScope(b => + { + if (userId is Guid id) + { + b.RegisterInstance(new TestCurrentUser(id, "tester", roles)).As().SingleInstance(); + b.RegisterInstance(new TestCurrentTeam(world.TeamId)).As().SingleInstance(); + } + + configure?.Invoke(b); + }); + + /// The Providers modal's "Reconnect": Init naming the credential, then the provider's anonymous callback. + private async Task ReconnectAsync(World world, Guid userId, Guid credentialId, string code, Action? configure = null) + { + var init = await InitAsync(world, userId, credentialId).ConfigureAwait(false); + + return await CompleteAsync(world, init, code, configure).ConfigureAwait(false); + } + + private async Task InitAsync(World world, Guid userId, Guid? replaceCredentialId, Guid? intendedOwnerUserId = null) + { + using var scope = Scope(world, userId); + + return await scope.Resolve().Send(new InitCredentialOAuthCommand { ProviderInstanceId = world.InstanceId, DisplayName = "GitLab · nacho", ReplaceCredentialId = replaceCredentialId, IntendedOwnerUserId = intendedOwnerUserId }).ConfigureAwait(false); + } + + private async Task CompleteAsync(World world, InitCredentialOAuthResult init, string code, Action? configure = null) + { + using var callback = Scope(world, userId: null, configure); + + return await callback.Resolve().Send(new CompleteCredentialOAuthCommand { State = init.State, Code = code, BrowserNonce = init.BrowserNonce }).ConfigureAwait(false); + } + + private async Task ReplaceTokenAsync(World world, Guid userId, Guid credentialId, string token, params string[] roles) + { + using var scope = Scope(world, userId, configure: null, roles); + + return await scope.Resolve().Send(new ReplaceCredentialTokenCommand { CredentialId = credentialId, Token = token }).ConfigureAwait(false); + } + + private async Task<(Guid TeamId, Guid InstanceId)> TeamAndInstanceOfAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + var credential = await scope.Resolve().Credential.AsNoTracking().SingleAsync(c => c.Id == credentialId).ConfigureAwait(false); + + return (credential.TeamId, credential.ProviderInstanceId); + } + + /// The re-authorization itself, with no connect around it — for a credential a process harness seeded, which has no team members or pending state. + private async Task ReauthorizeDirectlyAsync(Guid credentialId, Guid teamId, Guid instanceId, OAuthPayload payload) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().ReauthorizeAsync(new CredentialReauthorization { CredentialId = credentialId, TeamId = teamId, ProviderInstanceId = instanceId, Payload = payload, ActorUserId = Guid.NewGuid() }, CancellationToken.None).ConfigureAwait(false); + } + + private async Task LoadCredentialAsync(Guid credentialId) => await GitLabCredentialSeed.ReadCredentialAsync(_fixture, credentialId).ConfigureAwait(false); + + private async Task StoredPairAsync(Guid credentialId) => await GitLabCredentialSeed.ReadStoredPayloadAsync(_fixture, credentialId).ConfigureAwait(false); + + /// The static token the row holds, read as stored — past the resolver's refusal of an Error credential, since a refused replacement leaves the credential as it was. + private async Task TokenOfAsync(Guid credentialId) => await GitLabCredentialSeed.ReadPayloadAsync(_fixture, credentialId).ConfigureAwait(false) switch + { + PatPayload pat => pat.Token, + GroupAccessTokenPayload group => group.Token, + var other => throw new InvalidOperationException($"the credential holds a {other.Type} payload, not a static token") + }; + + private async Task LoadRepositoryAsync(Guid repositoryId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Repository.AsNoTracking().SingleAsync(r => r.Id == repositoryId).ConfigureAwait(false); + } + + private async Task LoadHookAsync(Guid hookId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().ConnectionWebhook.AsNoTracking().SingleAsync(w => w.Id == hookId).ConfigureAwait(false); + } + + private async Task> LoadIdentitiesAsync(Guid userId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().UserProviderIdentity.AsNoTracking().Where(i => i.UserId == userId).OrderBy(i => i.CreatedDate).ToListAsync().ConfigureAwait(false); + } + + private async Task LoadPendingStateAsync(string state) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().OAuthPendingState.AsNoTracking().SingleAsync(s => s.State == state).ConfigureAwait(false); + } + + private async Task CountCredentialsAsync(World world) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Credential.AsNoTracking().CountAsync(c => c.TeamId == world.TeamId).ConfigureAwait(false); + } + + private async Task CountPendingStatesAsync(World world) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().OAuthPendingState.AsNoTracking().CountAsync(s => s.TeamId == world.TeamId).ConfigureAwait(false); + } + + private bool WasDispatched(Guid hookId) + { + using var scope = _fixture.BeginScope(); + + return scope.Resolve().Calls.Any(c => c.ServiceType == typeof(IConnectionWebhookRegistrar) && Equals(c.FirstArgument, hookId)); + } + + /// What a departure or a connection delete leaves: the token gone, the row soft-deleted and Revoked. + private async Task RetireAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().Credential.Where(c => c.Id == credentialId).ExecuteUpdateAsync(s => s.SetProperty(c => c.EncryptedPayload, string.Empty).SetProperty(c => c.Status, CredentialStatus.Revoked).SetProperty(c => c.DeletedDate, DateTimeOffset.UtcNow)).ConfigureAwait(false); + } + + /// A Disconnect, which takes no refresh lock: it blanks the payload and marks the row Revoked, and that is all that matters to a write waiting on the old payload. + private async Task DisconnectBehindItsBackAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().Credential.Where(c => c.Id == credentialId).ExecuteUpdateAsync(s => s.SetProperty(c => c.EncryptedPayload, string.Empty).SetProperty(c => c.Status, CredentialStatus.Revoked)).ConfigureAwait(false); + } + + /// The row a pod from before migration 0250 writes: only the columns it knows. + private async Task InsertPreMigrationStateAsync(World world, string state, Guid initiatorId, string nonceHash) + { + await using var conn = new NpgsqlConnection(_fixture.ConnectionString); + await conn.OpenAsync().ConfigureAwait(false); + + await using var cmd = new NpgsqlCommand("INSERT INTO oauth_pending_state (state, provider_instance_id, team_id, initiator_user_id, code_verifier, intended_display_name, intended_owner_user_id, return_url, browser_nonce_hash, requested_scopes, expires_date, created_date, created_by, last_modified_date, last_modified_by) VALUES (@state, @instance, @team, @user, 'verifier', 'GitLab · nacho', @user, NULL, @hash, NULL, now() + interval '10 minutes', now(), @user, now(), @user)", conn); + cmd.Parameters.AddWithValue("state", state); + cmd.Parameters.AddWithValue("instance", world.InstanceId); + cmd.Parameters.AddWithValue("team", world.TeamId); + cmd.Parameters.AddWithValue("user", initiatorId); + cmd.Parameters.AddWithValue("hash", nonceHash); + + await cmd.ExecuteNonQueryAsync().ConfigureAwait(false); + } + + /// + /// Bounds a wait that hangs, if it hangs, on the refresh semaphore or the Postgres advisory lock — and says how to see which. + /// + private static async Task WithinPatienceAsync(Task work, string signal, Guid credentialId) + { + await WithinPatienceAsync((Task)work, signal, credentialId).ConfigureAwait(false); + + return await work.ConfigureAwait(false); + } + + private static async Task WithinPatienceAsync(Task work, string signal, Guid credentialId) + { + try + { + await work.WaitAsync(Patience).ConfigureAwait(false); + } + catch (TimeoutException timeout) + { + throw new TimeoutException($"{signal} within {Patience.TotalSeconds}s — most likely stuck on the in-process refresh semaphore or the Postgres advisory lock (key {CredentialRefreshLock.KeyOf(credentialId)}) for credential {credentialId}. While the test hangs, inspect `SELECT pid, pg_blocking_pids(pid), wait_event_type, state, query FROM pg_stat_activity WHERE datname = current_database()`.", timeout); + } + } + + private static async Task WaitForAsync(Func condition, string signal) + { + var deadline = DateTimeOffset.UtcNow + Patience; + + while (!condition()) + { + if (DateTimeOffset.UtcNow > deadline) throw new TimeoutException($"{signal} within {Patience.TotalSeconds}s"); + + await Task.Delay(20).ConfigureAwait(false); + } + } + + private void Dump(GitLabDoorkeeperStub gitlab) + { + _output.WriteLine($"GitLab saw: rotated={gitlab.Count(StubEvent.Rotated)} refused={gitlab.Count(StubEvent.Refused)} authorized={gitlab.Count(StubEvent.Authorized)} revoked={gitlab.Count(StubEvent.Revoked)} api-200={gitlab.Count(StubEvent.Api200)} api-401={gitlab.Count(StubEvent.Api401)}"); + + foreach (var entry in gitlab.Log) _output.WriteLine($" {entry.At:HH:mm:ss.fff} {entry.Kind}: {entry.Detail}"); + } + + /// Records every revoke it is asked for, with what the credential row held at that moment — so "after the new pair is stored" is a fact the test reads, not an inference. + private sealed class RecordingGrantRevoker : IOAuthGrantRevoker + { + private readonly Func _storedAccessToken; + private readonly List _revocations = new(); + + public RecordingGrantRevoker(Func? storedAccessToken = null) { _storedAccessToken = storedAccessToken ?? (() => null); } + + public bool Fails { get; init; } + + public IReadOnlyList Revocations { get { lock (_revocations) { return _revocations.ToList(); } } } + + public OAuthPayload? ReadPayload(Credential credential) => null; + + public Task RevokeAsync(ProviderInstance instance, string clientSecret, OAuthPayload payload, bool wholeGrant, CancellationToken cancellationToken) + { + lock (_revocations) { _revocations.Add(new Revocation(payload, wholeGrant, _storedAccessToken())); } + + return Fails ? throw new InvalidOperationException("simulated: the provider is down") : Task.CompletedTask; + } + + public sealed record Revocation(OAuthPayload Payload, bool WholeGrant, string? StoredAccessTokenWhenAsked); + } + + /// + /// Wraps the real writer so a test can hold a re-authorization inside the lock — after the row was read and before it is + /// written — or let something else change the row in that gap. completes the moment the first write + /// is reached; the write waits for . + /// + private sealed class GatedReauthorizationWriter : ICredentialReauthorizationWriter + { + private readonly TaskCompletionSource _entered = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly TaskCompletionSource _release = new(TaskCreationOptions.RunContinuationsAsynchronously); + private ICredentialReauthorizationWriter _inner = default!; + private int _writes; + + public Task Entered => _entered.Task; + + /// Runs once, before the first write is attempted — what happens to the row while the re-authorization is between its read and its write. + public Func? BeforeFirstWrite { get; init; } + + public int Writes => Volatile.Read(ref _writes); + + public void Release() => _release.TrySetResult(); + + public ICredentialReauthorizationWriter Over(ICredentialReauthorizationWriter inner) + { + _inner = inner; + + return this; + } + + public async Task TryReplaceAsync(CredentialReplacement replacement, CancellationToken cancellationToken) + { + var attempt = Interlocked.Increment(ref _writes); + + if (attempt == 1 && BeforeFirstWrite != null) + { + await BeforeFirstWrite().ConfigureAwait(false); + } + else if (attempt == 1) + { + _entered.TrySetResult(); + await _release.Task.ConfigureAwait(false); + } + + return await _inner.TryReplaceAsync(replacement, cancellationToken).ConfigureAwait(false); + } + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabCredentialSeed.cs b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabCredentialSeed.cs index 5cd4f9d78..f29fdec62 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabCredentialSeed.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabCredentialSeed.cs @@ -108,13 +108,16 @@ public static async Task SetLastValidatedAsync(PostgresFixture fixture, Guid cre await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"UPDATE credential SET last_validated_date = {at} WHERE id = {credentialId}").ConfigureAwait(false); } - /// What the credential row holds now, decrypted — what the next refresh would present. - public static async Task ReadStoredPayloadAsync(PostgresFixture fixture, Guid credentialId) + /// What the credential row holds now, decrypted — what the next refresh would present. Read as the row stands, not through , which refuses a credential its provider refused (Error): a test asks what is stored, whatever the row says about its health. + public static async Task ReadStoredPayloadAsync(PostgresFixture fixture, Guid credentialId) => (OAuthPayload)await ReadPayloadAsync(fixture, credentialId).ConfigureAwait(false); + + /// The payload the row holds, as the type its auth type says — read past the resolver's health gate (see ). + public static async Task ReadPayloadAsync(PostgresFixture fixture, Guid credentialId) { using var scope = fixture.BeginScope(); var credential = await scope.Resolve().Credential.AsNoTracking().SingleAsync(c => c.Id == credentialId).ConfigureAwait(false); - return (OAuthPayload)scope.Resolve().Resolve(credential); + return scope.Resolve().Deserialize(credential.AuthType, scope.Resolve().Decrypt(credential.EncryptedPayload)); } public static async Task ReadCredentialAsync(PostgresFixture fixture, Guid credentialId) diff --git a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabDoorkeeperStub.cs b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabDoorkeeperStub.cs index 75ccccff0..eb6bb10c8 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabDoorkeeperStub.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabDoorkeeperStub.cs @@ -49,14 +49,18 @@ public enum StubEvent /// Access tokens. A personal, project or group access token is not part of an OAuth grant: it is added with /// , travels as PRIVATE-TOKEN on REST (GitLab resolves that header as exactly that, whatever /// the token's length), answers 401 once it is revoked () or past the expiry it was given, and -/// describes itself — scopes, expires_at as a date, active — at GET /api/v4/personal_access_tokens/self, -/// which an OAuth token cannot call (404). +/// describes itself — scopes (), expires_at as a date, active — at +/// GET /api/v4/personal_access_tokens/self, which an OAuth token cannot call (404). +/// is the short form for a token that never expires. /// /// Endpoints. POST /oauth/token (refresh_token and authorization_code grants; the code /// authz:<user> issues <user> a first pair), POST /oauth/revoke (RFC 7009: 200 whatever /// it was given), GET /api/v4/user and GET /api/v4/projects/:id/members/all/:user_id (Bearer only — GitLab /// resolves an OAuth token from Authorization: Bearer and looks PRIVATE-TOKEN up as a personal access -/// token), any other GET /api/v4/** as an empty list, and git smart-HTTP GET **/info/refs for both services. +/// token; an access token is read as PRIVATE-TOKEN alone, see above), GET /api/v4/personal_access_tokens/self +/// (an access token's own scopes and expiry), any other GET /api/v4/** as an empty list, and git smart-HTTP +/// GET **/info/refs for both services. Every user has an id of their own, stable for the stub's life +/// (), so a test can tell two accounts apart the way a provider does. /// The git endpoint takes an OAuth access token as HTTP Basic with login oauth2 — what GitLab through 17.x /// requires of an OAuth token — or as Authorization: Bearer; anything else is a 401 with the /// WWW-Authenticate: Basic challenge that makes git ask again. A live token gets an empty repository's @@ -84,6 +88,7 @@ public sealed class GitLabDoorkeeperStub : IDisposable private readonly List _log = new(); private readonly Dictionary _applications = new() { [DefaultClientId] = DefaultClientSecret }; private readonly Dictionary _accessTokens = new(); + private readonly Dictionary _userIds = new(); private int _sequence; private int _membersFailureStatus; private int _membersFailuresLeft; @@ -152,6 +157,23 @@ public void FailMembers(int status, int times = int.MaxValue) } } + /// The scopes a personal access token reports for itself at /api/v4/personal_access_tokens/self. + public const string PersonalAccessTokenScopes = "api,read_repository"; + + /// A personal access token created at GitLab: for a token that never expires. A test that wants a dead one revokes it, or presents a token this never heard of. + public void SeedPersonalAccessToken(string user, string token) => AddAccessToken(token, user); + + /// The id GitLab knows by: the first one asked about gets 1001, the next 1002, and so on. + public long UserIdOf(string user) + { + lock (_gate) + { + if (!_userIds.TryGetValue(user, out var id)) _userIds[user] = id = 1001 + _userIds.Count; + + return id; + } + } + /// A pair issued before the test began: what the credential row held when the story starts. public TokenRow Seed(string app, string user, string accessToken, string refreshToken) { @@ -174,6 +196,12 @@ public bool IsRefreshTokenLive(string refreshToken) lock (_gate) { return _rows.LastOrDefault(r => r.User == user && r.RevokedAt == null)?.RefreshToken; } } + /// The access token GitLab would accept right now for — the newest one not revoked. + public string? LiveAccessTokenOf(string user) + { + lock (_gate) { return _rows.LastOrDefault(r => r.User == user && r.RevokedAt == null)?.AccessToken; } + } + public void Dispose() { try { _listener.Close(); } @@ -360,7 +388,7 @@ private Reply TokenApi(string privateToken, string path) } private static string SelfBody(AccessToken token) => - JsonSerializer.Serialize(new { id = 1, name = "stub", scopes = new[] { "api" }, active = true, revoked = false, expires_at = token.ExpiresAt?.UtcDateTime.ToString("yyyy-MM-dd") }); + JsonSerializer.Serialize(new { id = 1, name = "stub", scopes = PersonalAccessTokenScopes.Split(','), active = true, revoked = false, expires_at = token.ExpiresAt?.UtcDateTime.ToString("yyyy-MM-dd") }); /// The members lookup's scripted failure, while any are left (). private Reply? ScriptedMembersFailure(string path) @@ -375,7 +403,7 @@ private static string SelfBody(AccessToken token) => private string ApiBody(string user, string path) { - if (path == "/api/v4/user") return JsonSerializer.Serialize(new { id = 1, username = user }); + if (path == "/api/v4/user") return JsonSerializer.Serialize(new { id = UserIdOf(user), username = user }); var member = path.IndexOf("/members/all/", StringComparison.Ordinal); diff --git a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabDoorkeeperStubTests.cs b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabDoorkeeperStubTests.cs index 8a24d5522..a628248d7 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabDoorkeeperStubTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/GitLabDoorkeeperStubTests.cs @@ -144,6 +144,41 @@ public async Task The_api_honours_an_oauth_token_only_as_a_bearer_and_answers_th _gitlab.Count(StubEvent.Api401).ShouldBe(1); } + [Fact] + public async Task Every_user_has_an_id_of_their_own_that_the_user_endpoint_reports_for_whoever_holds_the_token() + { + _gitlab.Seed("app1", "nacho", "at-nacho", "rt-nacho"); + _gitlab.Seed("app1", "maya", "at-maya", "rt-maya"); + _gitlab.SeedPersonalAccessToken("maya", "glpat-maya"); + + var nacho = await GetAsync("/api/v4/user", ("Authorization", "Bearer at-nacho")).ConfigureAwait(false); + var maya = await GetAsync("/api/v4/user", ("Authorization", "Bearer at-maya")).ConfigureAwait(false); + var mayaByToken = await GetAsync("/api/v4/user", ("PRIVATE-TOKEN", "glpat-maya")).ConfigureAwait(false); + + nacho.Json.GetProperty("id").GetInt64().ShouldBe(_gitlab.UserIdOf("nacho")); + maya.Json.GetProperty("id").GetInt64().ShouldBe(_gitlab.UserIdOf("maya")); + mayaByToken.Json.GetProperty("id").GetInt64().ShouldBe(_gitlab.UserIdOf("maya"), "one account, one id, whichever kind of token asked"); + _gitlab.UserIdOf("nacho").ShouldNotBe(_gitlab.UserIdOf("maya"), "a re-authorization is refused for another account by its id, so two users must not share one"); + maya.Json.GetProperty("username").GetString().ShouldBe("maya"); + } + + [Fact] + public async Task A_seeded_personal_access_token_is_an_access_token_that_never_expires_and_reports_its_own_scopes() + { + _gitlab.SeedPersonalAccessToken("nacho", "glpat-live"); + + var asPrivateToken = await GetAsync("/api/v4/user", ("PRIVATE-TOKEN", "glpat-live")).ConfigureAwait(false); + var asBearer = await GetAsync("/api/v4/user", ("Authorization", "Bearer glpat-live")).ConfigureAwait(false); + var unknown = await GetAsync("/api/v4/user", ("PRIVATE-TOKEN", "glpat-never-heard-of")).ConfigureAwait(false); + var self = await GetAsync("/api/v4/personal_access_tokens/self", ("PRIVATE-TOKEN", "glpat-live")).ConfigureAwait(false); + + asPrivateToken.Status.ShouldBe(HttpStatusCode.OK); + asBearer.Status.ShouldBe(HttpStatusCode.Unauthorized, "an access token is not an OAuth token: GitLab looks a Bearer up as OAuth alone"); + unknown.Status.ShouldBe(HttpStatusCode.Unauthorized); + self.Json.GetProperty("scopes").EnumerateArray().Select(e => e.GetString()).ShouldBe(GitLabDoorkeeperStub.PersonalAccessTokenScopes.Split(',')); + self.Json.GetProperty("expires_at").ValueKind.ShouldBe(JsonValueKind.Null); + } + [Theory] [InlineData("oauth2", true, HttpStatusCode.OK)] [InlineData("nacho", true, HttpStatusCode.Unauthorized)] diff --git a/backend/tests/CodeSpace.IntegrationTests/OAuth/OAuthTokenRefreshRaceFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/OAuth/OAuthTokenRefreshRaceFlowTests.cs index 184135346..1501ac7cf 100644 --- a/backend/tests/CodeSpace.IntegrationTests/OAuth/OAuthTokenRefreshRaceFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/OAuth/OAuthTokenRefreshRaceFlowTests.cs @@ -300,7 +300,7 @@ private async Task LoseRefreshLockToAnotherPodAsync(Guid credentialId, OAuthPayl private async Task TerminateRefreshLockSessionAsync(Guid credentialId) { - var key = OAuthTokenRefresher.CredentialIdToLockKey(credentialId); + var key = CredentialRefreshLock.KeyOf(credentialId); await using var conn = new NpgsqlConnection(_fixture.ConnectionString); await conn.OpenAsync().ConfigureAwait(false); @@ -326,7 +326,7 @@ private static async Task WithinPatienceAsync(Task work, string signal, } catch (TimeoutException timeout) { - throw new TimeoutException($"{signal} within {Patience.TotalSeconds}s — most likely stuck on the in-process refresh semaphore or the Postgres advisory lock (key {OAuthTokenRefresher.CredentialIdToLockKey(credentialId)}) for credential {credentialId}. While the test hangs, inspect `SELECT pid, pg_blocking_pids(pid), wait_event_type, state, query FROM pg_stat_activity WHERE datname = current_database()`.", timeout); + throw new TimeoutException($"{signal} within {Patience.TotalSeconds}s — most likely stuck on the in-process refresh semaphore or the Postgres advisory lock (key {CredentialRefreshLock.KeyOf(credentialId)}) for credential {credentialId}. While the test hangs, inspect `SELECT pid, pg_blocking_pids(pid), wait_event_type, state, query FROM pg_stat_activity WHERE datname = current_database()`.", timeout); } } @@ -484,7 +484,7 @@ public static async Task AcquireAsync(string connectionString await connection.OpenAsync().ConfigureAwait(false); await using var cmd = new NpgsqlCommand("SELECT pg_advisory_lock(@key), pg_backend_pid()", connection); - cmd.Parameters.AddWithValue("key", OAuthTokenRefresher.CredentialIdToLockKey(credentialId)); + cmd.Parameters.AddWithValue("key", CredentialRefreshLock.KeyOf(credentialId)); await using var reader = await cmd.ExecuteReaderAsync().ConfigureAwait(false); await reader.ReadAsync().ConfigureAwait(false); diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs index c96f35c82..4c734a5e9 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs @@ -76,6 +76,8 @@ public void The_wire_codes_are_pinned() FailureCodes.RateLimited.ShouldBe("rate_limited"); FailureCodes.CredentialDisconnected.ShouldBe("credential_disconnected"); FailureCodes.CredentialNeedsReconnect.ShouldBe("credential_needs_reconnect"); + FailureCodes.CredentialRetired.ShouldBe("credential_retired"); + FailureCodes.CredentialAccountMismatch.ShouldBe("credential_account_mismatch"); FailureCodes.GitCredentialRejected.ShouldBe("git_credential_rejected"); FailureCodes.ModelEndpointKeyRequired.ShouldBe("model_endpoint_key_required"); FailureCodes.ProviderEndpointCredentialsBound.ShouldBe("provider_endpoint_credentials_bound"); diff --git a/backend/tests/CodeSpace.UnitTests/Credentials/CredentialHealthDerivationTests.cs b/backend/tests/CodeSpace.UnitTests/Credentials/CredentialHealthDerivationTests.cs index 5b30e28ac..0bba5a56c 100644 --- a/backend/tests/CodeSpace.UnitTests/Credentials/CredentialHealthDerivationTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Credentials/CredentialHealthDerivationTests.cs @@ -2,6 +2,7 @@ using System.Text.Json.Serialization; using CodeSpace.Core.Services.Credentials; using CodeSpace.Messages.Dtos.Credentials; +using CodeSpace.Messages.Dtos.Providers; using CodeSpace.Messages.Enums; using Shouldly; @@ -111,4 +112,29 @@ public void Every_health_has_its_pinned_wire_name(CredentialHealth health, strin JsonSerializer.Serialize(health, options).ShouldBe($"\"{wire}\"", "a rename is a breaking change to every client that branches on it"); JsonSerializer.Deserialize($"\"{wire}\"", options).ShouldBe(health); } + + [Theory] + // auth type, did the provider describe the token, what it said about the expiry (days; null = never), expected record (null = leave the column alone) + [InlineData(AuthType.Pat, true, 90, true, 90)] + [InlineData(AuthType.ProjectAccessToken, true, 3, true, 3)] + [InlineData(AuthType.GroupAccessToken, true, 400, true, 400)] + [InlineData(AuthType.Pat, true, null, true, null)] // described, and "never expires" is recorded as no expiry: the replaced token's must not outlive it + [InlineData(AuthType.Pat, false, null, false, null)] // not described: nothing is known, so the column is left alone + [InlineData(AuthType.Pat, false, 90, false, null)] // an expiry the provider did not describe the token with is not trusted + [InlineData(AuthType.OAuth, true, 90, false, null)] // an OAuth credential's expiry is its access token's, written by the refresher + public void A_static_tokens_expiry_is_recorded_only_when_the_provider_described_the_token(AuthType type, bool described, int? days, bool recorded, int? expectedDays) + { + var probe = new CredentialProbeResult { IsValid = true, Active = described ? true : null, ExpiresAt = days is { } d ? Now.AddDays(d) : null }; + + var record = CredentialHealthDerivation.RecordedExpiry(type, probe); + + if (!recorded) record.ShouldBeNull(); + else record.ShouldNotBeNull().ExpiresAt.ShouldBe(expectedDays is { } e ? Now.AddDays(e) : null); + } + + [Fact] + public void No_probe_records_nothing() + { + CredentialHealthDerivation.RecordedExpiry(AuthType.Pat, null).ShouldBeNull(); + } } diff --git a/backend/tests/CodeSpace.UnitTests/Credentials/CredentialReauthorizerFilterTests.cs b/backend/tests/CodeSpace.UnitTests/Credentials/CredentialReauthorizerFilterTests.cs new file mode 100644 index 000000000..9aee6938c --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Credentials/CredentialReauthorizerFilterTests.cs @@ -0,0 +1,83 @@ +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Messages.Credentials; +using Shouldly; +using static CodeSpace.Core.Services.Credentials.CredentialReauthorizer; + +namespace CodeSpace.UnitTests.Credentials; + +/// +/// 🟢 High fidelity — the two pure decisions a re-authorization makes before it touches anything: whether the account that +/// signed in is the account the credential was connected as, and which of the tokens it replaced the provider is asked to +/// end. Everything around them (the lock, the write, the revival) is exercised against real Postgres in +/// CredentialReauthorizationFlowTests. +/// +[Trait("Category", "Unit")] +public class CredentialReauthorizerFilterTests +{ + [Theory] + [InlineData("1001", "nacho", "1001", "NACHO", AccountMatch.Same)] // ids agree; the handle's case is no evidence + [InlineData("1001", "nacho", "1002", "nacho", AccountMatch.Different)] // the provider's id is authoritative: a handle can be reused by someone else + [InlineData("1001", "nacho", "1001", "renamed", AccountMatch.Same)] // the same account after a rename + [InlineData("", "nacho", "1002", "NACHO", AccountMatch.Same)] // no id on record: the handle decides + [InlineData("1001", "nacho", null, "someone-else", AccountMatch.Different)] // the provider gave no id: the handle decides + [InlineData("1001", null, "1001", null, AccountMatch.Same)] + [InlineData(null, null, "1001", "nacho", AccountMatch.Unknown)] // nothing on record to compare + [InlineData("1001", "nacho", null, null, AccountMatch.Unknown)] // the provider said nothing about who signed in + [InlineData(" ", " ", "1001", "nacho", AccountMatch.Unknown)] // blank is not a name + public void The_account_that_signed_in_is_compared_with_the_account_on_record(string? knownId, string? knownName, string? presentedId, string? presentedName, AccountMatch expected) + { + Compare(knownId, knownName, presentedId, presentedName).ShouldBe(expected); + } + + [Fact] + public void The_tokens_a_re_authorization_replaced_are_the_ones_to_end() + { + var replaced = new OAuthPayload { AccessToken = "at-old", RefreshToken = "rt-old", ExpiresAt = DateTimeOffset.UtcNow.AddHours(-1) }; + var fresh = new OAuthPayload { AccessToken = "at-new", RefreshToken = "rt-new", ExpiresAt = DateTimeOffset.UtcNow.AddHours(2) }; + + ReplacedPairToRevoke(replaced, fresh).ShouldBeSameAs(replaced); + } + + [Theory] + [InlineData("at-same", "rt-old", "at-same", "rt-new")] // the provider reused the access token for the second sign-in + [InlineData("at-old", "rt-same", "at-new", "rt-same")] // ... or the refresh token: GitLab through 15.4 hands the same grant to a second authorization + [InlineData("at-same", "rt-same", "at-same", "rt-same")] // ... or both + public void A_token_the_new_authorization_shares_is_never_revoked_because_revoking_it_would_end_the_new_one(string oldAccess, string oldRefresh, string newAccess, string newRefresh) + { + var replaced = new OAuthPayload { AccessToken = oldAccess, RefreshToken = oldRefresh }; + var fresh = new OAuthPayload { AccessToken = newAccess, RefreshToken = newRefresh }; + + ReplacedPairToRevoke(replaced, fresh).ShouldBeNull("the row's old pair and its new one are one grant, so there is no old pair left to end"); + } + + [Fact] + public void Two_tokens_without_refresh_tokens_are_compared_by_their_access_tokens_alone() + { + // A GitHub OAuth App token has no refresh token, and "no refresh token" is not a token both sides share. + var replaced = new OAuthPayload { AccessToken = "gho-old" }; + + ReplacedPairToRevoke(replaced, new OAuthPayload { AccessToken = "gho-new" }).ShouldBeSameAs(replaced); + ReplacedPairToRevoke(replaced, new OAuthPayload { AccessToken = "gho-old" }).ShouldBeNull(); + } + + [Fact] + public void A_replaced_personal_access_token_has_nothing_to_end_at_the_provider() + { + ReplacedPairToRevoke(new PatPayload { Token = "glpat-old" }, new PatPayload { Token = "glpat-new" }).ShouldBeNull("a token the provider issued as a PAT is not one CodeSpace can revoke"); + ReplacedPairToRevoke(new PatPayload { Token = "glpat-old" }, new OAuthPayload { AccessToken = "at-new", RefreshToken = "rt-new" }).ShouldBeNull(); + } + + [Fact] + public void A_blank_row_has_nothing_to_end() + { + ReplacedPairToRevoke(null, new OAuthPayload { AccessToken = "at-new", RefreshToken = "rt-new" }).ShouldBeNull("a Disconnect already ended the tokens it blanked"); + } + + [Fact] + public void An_oauth_pair_replaced_by_a_pasted_token_is_ended() + { + var replaced = new OAuthPayload { AccessToken = "at-old", RefreshToken = "rt-old" }; + + ReplacedPairToRevoke(replaced, new PatPayload { Token = "glpat-new" }).ShouldBeSameAs(replaced); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/OAuth/CredentialRefreshLockTests.cs b/backend/tests/CodeSpace.UnitTests/OAuth/CredentialRefreshLockTests.cs new file mode 100644 index 000000000..ab1986021 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OAuth/CredentialRefreshLockTests.cs @@ -0,0 +1,147 @@ +using CodeSpace.Core.Services.OAuth; +using Shouldly; + +namespace CodeSpace.UnitTests.OAuth; + +/// +/// 🟢 High fidelity — the production and its real in-process semaphores; only the +/// cross-process tier is a recording fake, because ordering against it is the claim. A refresh and a re-authorization both +/// take this one lock, and what they share is process-wide state, so these pin the properties that make "the same lock" +/// true: the same credential excludes, another does not, and the two tiers are taken and given back in one fixed order. +/// +[Trait("Category", "Unit")] +public class CredentialRefreshLockTests +{ + private static readonly TimeSpan Patience = TimeSpan.FromSeconds(5); + private static readonly TimeSpan ShortWait = TimeSpan.FromMilliseconds(250); + + [Fact] + public void The_advisory_key_is_the_first_eight_bytes_of_the_credential_id() + { + // Another pod running the previous build derives the same key from the same id, so a change here would put a refresh + // and a re-authorization on different advisory locks. Pinned against the bytes, not against the function. + var id = Guid.Parse("00112233-4455-6677-8899-aabbccddeeff"); + + CredentialRefreshLock.KeyOf(id).ShouldBe(BitConverter.ToInt64(id.ToByteArray(), 0)); + CredentialRefreshLock.KeyOf(id).ShouldBe(BitConverter.ToInt64(new byte[] { 0x33, 0x22, 0x11, 0x00, 0x55, 0x44, 0x77, 0x66 }, 0)); + } + + [Fact] + public async Task A_second_holder_of_the_same_credential_waits_until_the_first_lets_go() + { + var id = Guid.NewGuid(); + var crossProcess = new RecordingCrossProcessLock(); + var first = await CredentialRefreshLock.AcquireAsync(crossProcess, id, CancellationToken.None); + + var second = CredentialRefreshLock.AcquireAsync(crossProcess, id, CancellationToken.None); + + await Task.Delay(ShortWait); + second.IsCompleted.ShouldBeFalse("a re-authorization and a refresh of one credential must never both be inside it"); + + await first.DisposeAsync(); + + await using var handle = await second.WaitAsync(Patience); + crossProcess.Events.ShouldBe(new[] { "acquire", "release", "acquire" }, ignoreOrder: false, customMessage: "the second holder takes the advisory lock only after the first has given it back"); + } + + [Fact] + public async Task Another_credential_is_not_held_up() + { + var crossProcess = new RecordingCrossProcessLock(); + await using var first = await CredentialRefreshLock.AcquireAsync(crossProcess, Guid.NewGuid(), CancellationToken.None); + + await using var other = await CredentialRefreshLock.AcquireAsync(crossProcess, Guid.NewGuid(), CancellationToken.None).WaitAsync(Patience); + + crossProcess.Events.ShouldBe(new[] { "acquire", "acquire" }); + } + + [Fact] + public async Task The_advisory_lock_is_given_back_before_the_semaphore_so_a_waiter_never_overlaps_it() + { + var id = Guid.NewGuid(); + var crossProcess = new RecordingCrossProcessLock(); + var first = await CredentialRefreshLock.AcquireAsync(crossProcess, id, CancellationToken.None); + var second = CredentialRefreshLock.AcquireAsync(crossProcess, id, CancellationToken.None); + await Task.Delay(ShortWait); + + await first.DisposeAsync(); + await using var handle = await second.WaitAsync(Patience); + + crossProcess.Events.IndexOf("release").ShouldBeLessThan(crossProcess.Events.LastIndexOf("acquire"), "if the semaphore went first the waiter would acquire while the advisory lock is still held"); + } + + [Fact] + public async Task A_wait_for_the_advisory_lock_that_fails_gives_the_semaphore_back() + { + var id = Guid.NewGuid(); + + var failure = await Should.ThrowAsync(async () => await CredentialRefreshLock.AcquireAsync(new RecordingCrossProcessLock { FailAcquire = true }, id, CancellationToken.None)); + + failure.Message.ShouldContain("advisory"); + await using var again = await CredentialRefreshLock.AcquireAsync(new RecordingCrossProcessLock(), id, CancellationToken.None).WaitAsync(Patience); + } + + [Fact] + public async Task A_cancelled_wait_for_the_semaphore_never_reaches_the_advisory_lock() + { + var id = Guid.NewGuid(); + var crossProcess = new RecordingCrossProcessLock(); + await using var holder = await CredentialRefreshLock.AcquireAsync(crossProcess, id, CancellationToken.None); + using var cancellation = new CancellationTokenSource(ShortWait); + + await Should.ThrowAsync(async () => await CredentialRefreshLock.AcquireAsync(crossProcess, id, cancellation.Token)); + + crossProcess.Events.ShouldBe(new[] { "acquire" }, ignoreOrder: false, customMessage: "the cancelled caller asked for the advisory lock without ever holding the semaphore"); + } + + [Fact] + public async Task Disposing_a_handle_twice_releases_once() + { + var id = Guid.NewGuid(); + var crossProcess = new RecordingCrossProcessLock(); + var first = await CredentialRefreshLock.AcquireAsync(crossProcess, id, CancellationToken.None); + + await first.DisposeAsync(); + await first.DisposeAsync(); + + // A second release of the semaphore would let two holders in; one acquire proves the count stayed at one. + await using var second = await CredentialRefreshLock.AcquireAsync(crossProcess, id, CancellationToken.None).WaitAsync(Patience); + var third = CredentialRefreshLock.AcquireAsync(crossProcess, id, CancellationToken.None); + + await Task.Delay(ShortWait); + third.IsCompleted.ShouldBeFalse(); + } + + private sealed class RecordingCrossProcessLock : ICrossProcessLock + { + private readonly object _gate = new(); + private readonly List _events = new(); + + public bool FailAcquire { get; init; } + + public List Events { get { lock (_gate) { return _events.ToList(); } } } + + public Task AcquireAsync(long key, CancellationToken cancellationToken) + { + if (FailAcquire) throw new InvalidOperationException("simulated: the advisory lock could not be taken"); + + lock (_gate) { _events.Add("acquire"); } + + return Task.FromResult(new Release(this)); + } + + private sealed class Release : IAsyncDisposable + { + private readonly RecordingCrossProcessLock _owner; + + public Release(RecordingCrossProcessLock owner) { _owner = owner; } + + public ValueTask DisposeAsync() + { + lock (_owner._gate) { _owner._events.Add("release"); } + + return ValueTask.CompletedTask; + } + } + } +} diff --git a/backend/tests/TESTING.md b/backend/tests/TESTING.md index 915250155..550a3e82d 100644 --- a/backend/tests/TESTING.md +++ b/backend/tests/TESTING.md @@ -54,7 +54,9 @@ Provider-credential tests share two more pieces, both in `CodeSpace.IntegrationT refresh revokes the access token it replaces, and the REST and git smart-HTTP endpoints honour only live tokens. It also stands in for personal, project and group access tokens (`AddAccessToken` / `RevokeAccessToken`: `PRIVATE-TOKEN`, a date-valued `expires_at` at `personal_access_tokens/self`, 401 once revoked or expired) and can script the members lookup to fail - (`FailMembers(status, times)`). Its counters (`Count(StubEvent.Refused)`, …) are how a test says "GitLab was never shown a consumed refresh token". + (`FailMembers(status, times)`). Every user has an id of their own (`UserIdOf`) and a personal access token can be seeded for one + (`SeedPersonalAccessToken`), so a test can tell two accounts apart and drive a pasted token, as the in-place re-authorization tests do. + Its counters (`Count(StubEvent.Refused)`, …) are how a test says "GitLab was never shown a consumed refresh token". `GitLabDoorkeeperStubTests` pins what it does, so a red test built on it is the product's and not the stub's. - `SilentProviderHost` (`CodeSpace.IntegrationTests/Webhooks`, linked into the unit project like `StubProviderHost`) — a loopback provider that accepts the connection and never answers, for what a hung GitLab costs the caller. A call to it only returns when the caller gives diff --git a/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx b/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx index 4af431a8e..c3936f398 100644 --- a/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx +++ b/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx @@ -5,6 +5,7 @@ import { ApiError, oauthApi, type AddProviderInstanceRequest, type UpdateProvide import type { CredentialSummary, ProviderInstanceSummary, ProviderKind } from "@/api/types"; import { useAlert, useConfirm } from "@/components/dialog"; import { IdentityLinkModal } from "@/components/identities/IdentityLinkModal"; +import { ReplaceTokenModal } from "@/components/identities/ReplaceTokenModal"; import { useAddGroupAccessToken, useAddProviderInstance, useCredentialCapabilities, useCredentials, useDeleteProviderInstance, useProviderDefaults, useProviderInstances, useRevokeCredential, useUpdateProviderInstance } from "@/hooks/use-credentials"; import { useMe } from "@/hooks/use-me"; import { TeamPermissions, useTeamPermissions } from "@/hooks/use-team-management"; @@ -74,6 +75,40 @@ function capabilityLabel(key: string): string { ?? key.replace(/^I/, "").replace(/Capability$/, "").replace(/([a-z])([A-Z])/g, "$1 $2"); } +/** The provider refused the credential, or its token has lapsed: it needs reconnecting (OAuth) or a new token. `status` alone is not enough — a token whose expiry passed reads rejected before anything has marked it. */ +function isRefused(credential: CredentialSummary): boolean { + return credential.status === "Error" || credential.health === "rejected"; +} + +/** What a credential's health says to the person, or nothing when it is fine: a refused one how to come back, a token about to expire how long it has. */ +function CredentialHealthChip({ credential }: { credential: CredentialSummary }) { + if (isRefused(credential)) { + return ( + + {credential.authType === "OAuth" ? "needs reconnect" : "needs a new token"} + + ); + } + + if (credential.health !== "expiring") return null; + + const days = credential.daysToExpiry ?? 0; + + return ( + + {days === 0 ? "expires today" : `expires in ${days} ${days === 1 ? "day" : "days"}`} + + ); +} + +/** + * A credential the lists show: a working one, and one the provider has refused — which is the one Reconnect and Replace + * token exist for, so it must stay reachable. A disconnected (Revoked) one is not listed: connecting again is how it comes back. + */ +function isListed(credential: CredentialSummary): boolean { + return credential.status === "Active" || credential.status === "Error"; +} + export function ConnectRemoteModal({ onClose }: ConnectRemoteModalProps) { const [step, setStep] = useState("list"); const [editingInstance, setEditingInstance] = useState(null); @@ -89,6 +124,8 @@ export function ConnectRemoteModal({ onClose }: ConnectRemoteModalProps) { const runOAuth = useOAuthFlow(); // The instance to connect a personal token (PAT) to — opens the shared IdentityLinkModal. const [patInstance, setPatInstance] = useState(null); + // The credential whose token is being replaced in place — opens ReplaceTokenModal. + const [replacing, setReplacing] = useState<{ credential: CredentialSummary; instance: ProviderInstanceSummary } | null>(null); const confirm = useConfirm(); // Show ALL providers (no oauthEnabled filter). Rows distinguish OAuth-ready vs @@ -105,7 +142,7 @@ export function ConnectRemoteModal({ onClose }: ConnectRemoteModalProps) { if (!myId) return map; const mine = (credentials.data ?? []) - .filter(c => c.ownerUserId === myId && c.status === "Active") + .filter(c => c.ownerUserId === myId && isListed(c)) .sort((a, b) => b.createdDate.localeCompare(a.createdDate)); for (const c of mine) { @@ -118,7 +155,7 @@ export function ConnectRemoteModal({ onClose }: ConnectRemoteModalProps) { // Active team-service credentials (owned by the team, not a person). Shown in the modal's // "Team" tab, grouped under the provider each belongs to — a distinct identity from a personal sign-in. const teamServiceCreds = useMemo( - () => (credentials.data ?? []).filter(c => c.ownership === "TeamService" && c.status === "Active"), + () => (credentials.data ?? []).filter(c => c.ownership === "TeamService" && isListed(c)), [credentials.data], ); @@ -128,13 +165,15 @@ export function ConnectRemoteModal({ onClose }: ConnectRemoteModalProps) { return () => window.removeEventListener("keydown", handler); }, [onClose]); - const connect = async (instance: ProviderInstanceSummary) => { + // `reauthorizing` names the credential a Reconnect brings back to life IN PLACE: same credential, same id, so the + // repositories bound through it keep working. Without it this is a plain connect, which creates a new credential. + const connect = async (instance: ProviderInstanceSummary, reauthorizing?: CredentialSummary) => { setConnectingId(instance.id); setErrorByInstance(prev => ({ ...prev, [instance.id]: "" })); try { - const displayName = `${me.data?.name ?? "user"}'s ${instance.displayName}`; - await runOAuth({ providerInstanceId: instance.id, displayName }); + const displayName = reauthorizing?.displayName ?? `${me.data?.name ?? "user"}'s ${instance.displayName}`; + await runOAuth({ providerInstanceId: instance.id, displayName, replaceCredentialId: reauthorizing?.id }); // Hook invalidates ['credentials']; row re-renders with "Connected" automatically. } catch (err) { const message = @@ -222,6 +261,8 @@ export function ConnectRemoteModal({ onClose }: ConnectRemoteModalProps) { revokingId={revoke.isPending ? revoke.variables ?? null : null} errors={errorByInstance} onConnect={connect} + onReconnect={connect} + onReplaceToken={(credential, instance) => setReplacing({ credential, instance })} onConnectPat={connectPat} onDisconnect={disconnect} onAddProvider={() => setStep("addProvider")} @@ -266,6 +307,13 @@ export function ConnectRemoteModal({ onClose }: ConnectRemoteModalProps) { onLinked={() => { void credentials.refetch(); }} /> )} + {replacing && ( + setReplacing(null)} + /> + )} , document.body, ); @@ -285,6 +333,8 @@ interface ProvidersStepProps { revokingId: string | null; errors: Record; onConnect: (instance: ProviderInstanceSummary) => void; + onReconnect: (instance: ProviderInstanceSummary, credential: CredentialSummary) => void; + onReplaceToken: (credential: CredentialSummary, instance: ProviderInstanceSummary) => void; onConnectPat: (instance: ProviderInstanceSummary) => void; onDisconnect: (credential: CredentialSummary) => void; onAddProvider: () => void; @@ -294,7 +344,7 @@ interface ProvidersStepProps { onClose: () => void; } -function ProvidersStep({ providers, loading, error, myCredsByInstance, teamServiceCreds, tab, onTabChange, connectingId, revokingId, errors, onConnect, onConnectPat, onDisconnect, onAddProvider, onEditProvider, onAddTeamToken, onRevokeTeamCred, onClose }: ProvidersStepProps) { +function ProvidersStep({ providers, loading, error, myCredsByInstance, teamServiceCreds, tab, onTabChange, connectingId, revokingId, errors, onConnect, onReconnect, onReplaceToken, onConnectPat, onDisconnect, onAddProvider, onEditProvider, onAddTeamToken, onRevokeTeamCred, onClose }: ProvidersStepProps) { // Absent rather than present-and-refusing. The form this opens asks for an OAuth client secret // before it can be submitted, so a member who cannot add a provider would type one into a request // that was never going to land. Asked as a permission — the server's matrix stays the only copy of @@ -362,6 +412,8 @@ function ProvidersStep({ providers, loading, error, myCredsByInstance, teamServi revokingId={revokingId} errorMsg={errors[instance.id]} onConnect={mayManageCredentials ? onConnect : null} + onReconnect={mayManageCredentials ? onReconnect : null} + onReplaceToken={onReplaceToken} onConnectPat={onConnectPat} onDisconnect={onDisconnect} onEdit={mayAddProvider ? onEditProvider : null} @@ -383,6 +435,7 @@ function ProvidersStep({ providers, loading, error, myCredsByInstance, teamServi revokingId={revokingId} onAdd={mayManageCredentials ? onAddTeamToken : null} onRevoke={mayManageCredentials ? onRevokeTeamCred : null} + onReplaceToken={mayManageCredentials ? onReplaceToken : null} /> ))} @@ -414,13 +467,16 @@ interface ProviderRowProps { errorMsg: string | undefined; /** Null when the caller may not start an OAuth connect — the button is absent rather than present-and-refused. */ onConnect: ((instance: ProviderInstanceSummary) => void) | null; + /** Null when the caller may not start an OAuth connect — Reconnect starts one, so it is absent too. */ + onReconnect: ((instance: ProviderInstanceSummary, credential: CredentialSummary) => void) | null; + onReplaceToken: (credential: CredentialSummary, instance: ProviderInstanceSummary) => void; onConnectPat: (instance: ProviderInstanceSummary) => void; onDisconnect: (credential: CredentialSummary) => void; /** Null when the caller may not edit or remove the provider — its menu and "Configure OAuth" are absent. */ onEdit: ((instance: ProviderInstanceSummary) => void) | null; } -function ProviderRow({ instance, theme, myCreds, isConnecting, revokingId, errorMsg, onConnect, onConnectPat, onDisconnect, onEdit }: ProviderRowProps) { +function ProviderRow({ instance, theme, myCreds, isConnecting, revokingId, errorMsg, onConnect, onReconnect, onReplaceToken, onConnectPat, onDisconnect, onEdit }: ProviderRowProps) { const connected = myCreds.length > 0; const hasOAuth = myCreds.some(c => c.authType === "OAuth"); const hasPat = myCreds.some(c => c.authType === "Pat"); @@ -470,6 +526,17 @@ function ProviderRow({ instance, theme, myCreds, isConnecting, revokingId, error {c.authType === "OAuth" ? <> OAuth : <> Token} connected {formatRelative(c.createdDate)} + + {/* Sign in again to the SAME credential (an OAuth sign-in), or paste a new token into it (a token): either way + it keeps its id, so the repositories bound through it keep working. */} + {c.authType === "OAuth" && instance.oauthEnabled && onReconnect && ( + + )} + {c.authType !== "OAuth" && ( + + )} @@ -516,7 +583,7 @@ function ProviderRow({ instance, theme, myCreds, isConnecting, revokingId, error * can mint a group token in place ("+ Add team token"); others show why they can't yet (GitHub * needs an App installation — not built). Each existing token carries its own Revoke. */ -function TeamProviderGroup({ instance, theme, creds, revokingId, onAdd, onRevoke }: { +function TeamProviderGroup({ instance, theme, creds, revokingId, onAdd, onRevoke, onReplaceToken }: { instance: ProviderInstanceSummary; theme: { initials: string; label: string }; creds: CredentialSummary[]; @@ -525,6 +592,8 @@ function TeamProviderGroup({ instance, theme, creds, revokingId, onAdd, onRevoke onAdd: ((instance: ProviderInstanceSummary) => void) | null; /** Null when the caller may not revoke team tokens — the button is absent rather than present-and-refused. */ onRevoke: ((credential: CredentialSummary) => void) | null; + /** Null when the caller may not manage team tokens — the button is absent rather than present-and-refused. */ + onReplaceToken: ((credential: CredentialSummary, instance: ProviderInstanceSummary) => void) | null; }) { const supportsTeamToken = providerSupportsTeamServiceCredential(instance.provider); @@ -542,6 +611,10 @@ function TeamProviderGroup({ instance, theme, creds, revokingId, onAdd, onRevoke
{c.displayName} + + {onReplaceToken && ( + + )} {onRevoke && ( +
+ +
+
+
+ New access token + setToken(e.target.value)} + onKeyDown={(e) => { if (e.key === "Enter") submit(); }} + placeholder="glpat-… / ghp_…" + autoFocus + /> + {error && {error}} +
+
+
+ +
+ + +
+ + , + document.body, + ); +} diff --git a/frontend/src/components/settings/ProvidersSettings.test.tsx b/frontend/src/components/settings/ProvidersSettings.test.tsx new file mode 100644 index 000000000..5e3bfd1b1 --- /dev/null +++ b/frontend/src/components/settings/ProvidersSettings.test.tsx @@ -0,0 +1,61 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import type { ProviderInstanceSummary } from "@/api/types"; +import { DialogProvider } from "@/components/dialog"; +import { ProvidersSettings } from "@/components/settings/ProvidersSettings"; + +/** + * The Providers summary agrees with the modal it opens: a provider the person has a working credential on is connected, one + * whose credential the provider refused says so — the modal lists that credential with the way to bring it back — and only a + * provider with nothing at all reads as not connected. + */ +describe("providers settings summary", () => { + const instance: ProviderInstanceSummary = { id: "pi1", teamId: "t1", provider: "GitLab", displayName: "Acme GitLab", baseUrl: "https://gitlab.test", createdDate: "2026-01-01T00:00:00Z", oauthEnabled: true }; + const credential = (id: string, extra: Record = {}) => ({ id, teamId: "t1", providerInstanceId: "pi1", ownerUserId: "u1", ownerUserName: "Maya", ownership: "Personal", authType: "Pat", displayName: `Maya's ${id}`, status: "Active", createdDate: "2026-01-01T00:00:00Z", ...extra }); + + function renderWith(held: unknown[]) { + localStorage.setItem("codespace.jwt", "test-jwt"); + localStorage.setItem("codespace.activeTeamId", "t1"); + + const routes: Record = { + "/api/users/me": { id: "u1", email: "u@test.local", name: "Maya", teams: [], permissions: [], passwordMustChange: false }, + "/api/provider-instances": [instance], + "/api/credentials": held, + }; + + vi.stubGlobal("fetch", vi.fn(async (input: RequestInfo | URL) => { + const url = typeof input === "string" ? input : input.toString(); + const key = Object.keys(routes).find((k) => url.includes(k)); + const body = key === undefined ? undefined : routes[key]; + return new Response(body === undefined ? "" : JSON.stringify(body), { status: body === undefined ? 404 : 200, headers: { "Content-Type": "application/json" } }); + })); + + const client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } }); + + render( + + + , + ); + } + + afterEach(() => { localStorage.clear(); vi.unstubAllGlobals(); }); + + it.each([ + { what: "a working credential", held: [credential("ok")], expected: "connected" }, + { what: "a credential the provider refused", held: [credential("dead", { status: "Error" })], expected: "needs reconnect" }, + { what: "a token whose expiry passed, though its row is still Active", held: [credential("lapsed", { health: "rejected" })], expected: "needs reconnect" }, + { what: "a refused one beside a working one", held: [credential("dead", { status: "Error" }), credential("ok")], expected: "connected" }, + { what: "only someone else's credential", held: [credential("theirs", { ownerUserId: "u2" })], expected: "not connected" }, + { what: "nothing", held: [], expected: "not connected" }, + ])("reads $what as '$expected'", async ({ held, expected }) => { + renderWith(held); + + await waitFor(() => expect(screen.getByText("Acme GitLab")).toBeTruthy()); + await waitFor(() => expect(screen.getByText(expected)).toBeTruthy()); + + for (const other of ["connected", "needs reconnect", "not connected"].filter((label) => label !== expected)) expect(screen.queryByText(other)).toBeNull(); + }); +}); diff --git a/frontend/src/components/settings/ProvidersSettings.tsx b/frontend/src/components/settings/ProvidersSettings.tsx index b0bddb056..d6feba46c 100644 --- a/frontend/src/components/settings/ProvidersSettings.tsx +++ b/frontend/src/components/settings/ProvidersSettings.tsx @@ -24,16 +24,20 @@ export function ProvidersSettings() { const loading = instances.isLoading || credentials.isLoading || me.isLoading; const rows = instances.data ?? []; - // Per-instance, whether the current user has an active personal credential on it — same "connected for me" - // signal the modal's Personal tab shows, so the summary agrees with what they'll see when they open it. - const connectedInstanceIds = useMemo(() => { - const set = new Set(); + // Per-instance, whether the current user has a working personal credential on it — and, failing that, one the provider + // refused, which the modal's Personal tab lists with the way to bring it back. The same signals the modal shows, so the + // summary agrees with what they'll see when they open it. + const { connectedInstanceIds, refusedInstanceIds } = useMemo(() => { + const connected = new Set(); + const refused = new Set(); const myId = me.data?.id; - if (!myId) return set; + if (!myId) return { connectedInstanceIds: connected, refusedInstanceIds: refused }; for (const c of credentials.data ?? []) { - if (c.ownerUserId === myId && c.status === "Active") set.add(c.providerInstanceId); + if (c.ownerUserId !== myId) continue; + if (c.status === "Error" || c.health === "rejected") refused.add(c.providerInstanceId); + else if (c.status === "Active") connected.add(c.providerInstanceId); } - return set; + return { connectedInstanceIds: connected, refusedInstanceIds: refused }; }, [credentials.data, me.data?.id]); // Rendered inside the Settings layout (it owns the "Settings" header + tab strip), so this is body-only. @@ -91,7 +95,9 @@ export function ProvidersSettings() { )} {connected ? connected - : not connected} + : refusedInstanceIds.has(inst.id) + ? needs reconnect + : not connected}
{inst.baseUrl}
diff --git a/frontend/src/hooks/use-credentials.ts b/frontend/src/hooks/use-credentials.ts index d8d302138..e86ea6bc2 100644 --- a/frontend/src/hooks/use-credentials.ts +++ b/frontend/src/hooks/use-credentials.ts @@ -264,6 +264,22 @@ export function useRevokeCredential() { }); } +/** + * Replace a credential's token in place. The credential keeps its id, so the repositories its dead token had turned Error + * come back — invalidate what shows them, not only the credential list. + */ +export function useReplaceCredentialToken() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: ({ credentialId, token }: { credentialId: string; token: string }) => oauthApi.replaceCredentialToken(credentialId, token), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["credentials"] }); + queryClient.invalidateQueries({ queryKey: ["repositories"] }); + queryClient.invalidateQueries({ queryKey: ["credential-capabilities"] }); + }, + }); +} + /** Backend-driven defaults for the "Add provider" form. Cached aggressively — the source * is a static IProviderModule, so the values change only on a backend deploy. */ export function useProviderDefaults(provider: ProviderKind | null) { diff --git a/frontend/src/hooks/use-oauth-flow.ts b/frontend/src/hooks/use-oauth-flow.ts index 0f156f311..7418ead98 100644 --- a/frontend/src/hooks/use-oauth-flow.ts +++ b/frontend/src/hooks/use-oauth-flow.ts @@ -114,10 +114,11 @@ export function useOAuthFlow() { try { const outcome = await awaitOutcome(popup); - // Successful OAuth always lands a new credential row. Invalidate so any open - // modal / sidebar showing connection status re-fetches immediately rather than - // waiting for the next mount. + // A successful OAuth lands a new credential row — or, for a reconnect that named one (`replaceCredentialId`), + // brings the same row back to life along with the repositories its dead token had turned Error. Invalidate so any + // open modal / sidebar showing connection status re-fetches immediately rather than waiting for the next mount. await queryClient.invalidateQueries({ queryKey: ["credentials"] }); + await queryClient.invalidateQueries({ queryKey: ["repositories"] }); return outcome; } finally { try { if (!popup.closed) popup.close(); } catch { /* cross-origin during provider visit; harmless */ }