diff --git a/backend/src/CodeSpace.Core/Authorization/CredentialManagementAuthorizationBehavior.cs b/backend/src/CodeSpace.Core/Authorization/CredentialManagementAuthorizationBehavior.cs index 244f6ca25..efd7d346e 100644 --- a/backend/src/CodeSpace.Core/Authorization/CredentialManagementAuthorizationBehavior.cs +++ b/backend/src/CodeSpace.Core/Authorization/CredentialManagementAuthorizationBehavior.cs @@ -1,10 +1,6 @@ -using CodeSpace.Core.Persistence.Db; -using CodeSpace.Core.Services.Identity; using CodeSpace.Messages.Authorization; using CodeSpace.Messages.Constants; -using CodeSpace.Messages.Enums; using MediatR; -using Microsoft.EntityFrameworkCore; namespace CodeSpace.Core.Authorization; @@ -15,44 +11,14 @@ namespace CodeSpace.Core.Authorization; /// public sealed class CredentialManagementAuthorizationBehavior : IPipelineBehavior where TRequest : IRequireCredentialManagement { - private readonly CodeSpaceDbContext _db; - private readonly ICurrentTeam _currentTeam; - private readonly ICurrentUser _currentUser; - private readonly TeamMembershipResolver _resolver; + private readonly CredentialOwnerOrPermission _rule; - public CredentialManagementAuthorizationBehavior(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, TeamMembershipResolver resolver) - { - _db = db; - _currentTeam = currentTeam; - _currentUser = currentUser; - _resolver = resolver; - } + public CredentialManagementAuthorizationBehavior(CredentialOwnerOrPermission rule) { _rule = rule; } public async Task Handle(TRequest request, RequestHandlerDelegate next, CancellationToken cancellationToken) { - if (await IsCallersOwnPersonalCredentialAsync(request.CredentialId, cancellationToken).ConfigureAwait(false)) return await next().ConfigureAwait(false); - - await EnsureCredentialsManageAsync(cancellationToken).ConfigureAwait(false); + await _rule.EnsureAsync(request.CredentialId, TeamPermissions.CredentialsManage, cancellationToken).ConfigureAwait(false); return await next().ConfigureAwait(false); } - - private async Task IsCallersOwnPersonalCredentialAsync(Guid credentialId, CancellationToken cancellationToken) - { - if (_currentUser.Id is not { } callerId) return false; - - return await _db.Credential.AsNoTracking() - .AnyAsync(c => c.Id == credentialId && c.DeletedDate == null && c.Ownership == CredentialOwnership.Personal && c.OwnerUserId == callerId, cancellationToken) - .ConfigureAwait(false); - } - - private async Task EnsureCredentialsManageAsync(CancellationToken cancellationToken) - { - var teamId = _currentTeam.Id ?? throw TenantAccessDeniedException.NoTeamSelected(_currentUser.Id, HeaderCurrentTeam.HeaderName); - - var role = await _resolver.ResolveRoleAsync(teamId, cancellationToken).ConfigureAwait(false); - - if (!TeamPermissionMatrix.IsGranted(role, TeamPermissions.CredentialsManage)) - throw TenantAccessDeniedException.MissingTeamPermission(_currentUser.Id, teamId, role, TeamPermissions.CredentialsManage); - } } diff --git a/backend/src/CodeSpace.Core/Authorization/CredentialOwnerOrPermission.cs b/backend/src/CodeSpace.Core/Authorization/CredentialOwnerOrPermission.cs new file mode 100644 index 000000000..d3ff3a80c --- /dev/null +++ b/backend/src/CodeSpace.Core/Authorization/CredentialOwnerOrPermission.cs @@ -0,0 +1,57 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.Identity; +using CodeSpace.Messages.Authorization; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; + +namespace CodeSpace.Core.Authorization; + +/// +/// The rule every credential tier inside applies, +/// with its own permission: the owner of a Personal credential passes on the membership that behavior already proved; +/// anyone else — on a teammate's credential, or on the team's own, which has no owner — needs the named team permission. +/// An instance Admin resolves as an Owner (), the standing +/// gives them in every team. +/// +public sealed class CredentialOwnerOrPermission : IScopedDependency +{ + private readonly CodeSpaceDbContext _db; + private readonly ICurrentTeam _currentTeam; + private readonly ICurrentUser _currentUser; + private readonly TeamMembershipResolver _resolver; + + public CredentialOwnerOrPermission(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, TeamMembershipResolver resolver) + { + _db = db; + _currentTeam = currentTeam; + _currentUser = currentUser; + _resolver = resolver; + } + + public async Task EnsureAsync(Guid credentialId, string permission, CancellationToken cancellationToken) + { + if (await IsCallersOwnPersonalCredentialAsync(credentialId, cancellationToken).ConfigureAwait(false)) return; + + await EnsurePermissionAsync(permission, cancellationToken).ConfigureAwait(false); + } + + private async Task IsCallersOwnPersonalCredentialAsync(Guid credentialId, CancellationToken cancellationToken) + { + if (_currentUser.Id is not { } callerId) return false; + + return await _db.Credential.AsNoTracking() + .AnyAsync(c => c.Id == credentialId && c.DeletedDate == null && c.Ownership == CredentialOwnership.Personal && c.OwnerUserId == callerId, cancellationToken) + .ConfigureAwait(false); + } + + private async Task EnsurePermissionAsync(string permission, CancellationToken cancellationToken) + { + var teamId = _currentTeam.Id ?? throw TenantAccessDeniedException.NoTeamSelected(_currentUser.Id, HeaderCurrentTeam.HeaderName); + + var role = await _resolver.ResolveRoleAsync(teamId, cancellationToken).ConfigureAwait(false); + + if (!TeamPermissionMatrix.IsGranted(role, permission)) + throw TenantAccessDeniedException.MissingTeamPermission(_currentUser.Id, teamId, role, permission); + } +} diff --git a/backend/src/CodeSpace.Core/Authorization/CredentialSpendAuthorizationBehavior.cs b/backend/src/CodeSpace.Core/Authorization/CredentialSpendAuthorizationBehavior.cs new file mode 100644 index 000000000..a593a0e29 --- /dev/null +++ b/backend/src/CodeSpace.Core/Authorization/CredentialSpendAuthorizationBehavior.cs @@ -0,0 +1,25 @@ +using CodeSpace.Messages.Authorization; +using CodeSpace.Messages.Constants; +using MediatR; + +namespace CodeSpace.Core.Authorization; + +/// +/// The owner of a Personal credential may present it to the provider on membership alone; every other caller needs +/// , the bar of the bind such a listing feeds. Runs inside +/// , which has already proven the credential is the +/// header team's and the caller a member of it. +/// +public sealed class CredentialSpendAuthorizationBehavior : IPipelineBehavior where TRequest : IRequireCredentialSpend +{ + private readonly CredentialOwnerOrPermission _rule; + + public CredentialSpendAuthorizationBehavior(CredentialOwnerOrPermission rule) { _rule = rule; } + + public async Task Handle(TRequest request, RequestHandlerDelegate next, CancellationToken cancellationToken) + { + await _rule.EnsureAsync(request.CredentialId, TeamPermissions.ReposManage, cancellationToken).ConfigureAwait(false); + + return await next().ConfigureAwait(false); + } +} diff --git a/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/ShredRetiredSecretsCommandHandler.cs b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/ShredRetiredSecretsCommandHandler.cs new file mode 100644 index 000000000..7afe0556b --- /dev/null +++ b/backend/src/CodeSpace.Core/Handlers/CommandHandlers/Credentials/ShredRetiredSecretsCommandHandler.cs @@ -0,0 +1,21 @@ +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Messages.Commands.Credentials; +using MediatR; + +namespace CodeSpace.Core.Handlers.CommandHandlers.Credentials; + +/// Thin dispatcher (Rule 16) — the production caller of . +public sealed class ShredRetiredSecretsCommandHandler : IRequestHandler +{ + private readonly IRetiredSecretShredService _shred; + + public ShredRetiredSecretsCommandHandler(IRetiredSecretShredService shred) + { + _shred = shred; + } + + public async Task Handle(ShredRetiredSecretsCommand request, CancellationToken cancellationToken) + { + return await _shred.ShredAsync(cancellationToken).ConfigureAwait(false); + } +} diff --git a/backend/src/CodeSpace.Core/Jobs/RecurringJobs/RetiredSecretShredRecurringJob.cs b/backend/src/CodeSpace.Core/Jobs/RecurringJobs/RetiredSecretShredRecurringJob.cs new file mode 100644 index 000000000..9ecf52261 --- /dev/null +++ b/backend/src/CodeSpace.Core/Jobs/RecurringJobs/RetiredSecretShredRecurringJob.cs @@ -0,0 +1,17 @@ +using CodeSpace.Messages.Commands.Credentials; +using MediatR; + +namespace CodeSpace.Core.Jobs.RecurringJobs; + +/// Every hour: blank the secrets a build older than migration 0249 retired without blanking — any an older pod retires during a rolling deploy, and every one retired after a rollback — since 0249's own backfill ran once (thin Rule-14 dispatcher). A blanked row is no longer a candidate, so an idle tick changes nothing. +public sealed class RetiredSecretShredRecurringJob : IRecurringJob +{ + private readonly IMediator _mediator; + + public RetiredSecretShredRecurringJob(IMediator mediator) { _mediator = mediator; } + + public string JobId => nameof(RetiredSecretShredRecurringJob); + public string CronExpression => "0 * * * *"; + + public async Task Execute() => await _mediator.Send(new ShredRetiredSecretsCommand()).ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Mediation/MediatorModule.cs b/backend/src/CodeSpace.Core/Mediation/MediatorModule.cs index aa750d826..dae9a9b92 100644 --- a/backend/src/CodeSpace.Core/Mediation/MediatorModule.cs +++ b/backend/src/CodeSpace.Core/Mediation/MediatorModule.cs @@ -40,6 +40,8 @@ protected override void Load(ContainerBuilder builder) // already vetted, this adds the role check for write/action requests // CredentialManagement — inside Credential (whose marker it extends): the owner of a Personal // credential passes on membership, anyone else needs credentials.manage + // CredentialSpend — the same shape for presenting a credential to its provider: the owner of a + // Personal credential passes on membership, anyone else needs repos.manage // BotVisibility — opts the request into seeing bot users (IBotInclusive); default excludes // Transactional — innermost of these, wraps DB writes only after auth has passed // @@ -56,6 +58,7 @@ protected override void Load(ContainerBuilder builder) builder.RegisterGeneric(typeof(RepositoryAccessAuthorizationBehavior<,>)).As(typeof(IPipelineBehavior<,>)).InstancePerLifetimeScope(); builder.RegisterGeneric(typeof(CredentialAccessAuthorizationBehavior<,>)).As(typeof(IPipelineBehavior<,>)).InstancePerLifetimeScope(); builder.RegisterGeneric(typeof(CredentialManagementAuthorizationBehavior<,>)).As(typeof(IPipelineBehavior<,>)).InstancePerLifetimeScope(); + builder.RegisterGeneric(typeof(CredentialSpendAuthorizationBehavior<,>)).As(typeof(IPipelineBehavior<,>)).InstancePerLifetimeScope(); builder.RegisterGeneric(typeof(BotVisibilityBehavior<,>)).As(typeof(IPipelineBehavior<,>)).InstancePerLifetimeScope(); builder.RegisterGeneric(typeof(TransactionalBehavior<,>)).As(typeof(IPipelineBehavior<,>)).InstancePerLifetimeScope(); diff --git a/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0249_shred_retired_secrets.sql b/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0249_shred_retired_secrets.sql new file mode 100644 index 000000000..58a50384a --- /dev/null +++ b/backend/src/CodeSpace.Core/Persistence/DbUpFiles/0249_shred_retired_secrets.sql @@ -0,0 +1,223 @@ +-- 0249_shred_retired_secrets.sql +-- +-- A retired secret keeps no ciphertext. Until now four kinds of retirement left theirs decryptable forever, readable by +-- anyone holding a database copy and the key: +-- +-- * deleting a git provider connection marked its credentials Revoked and soft-deleted, but kept each token in +-- credential.encrypted_payload, the connection's OAuth client secret in provider_instance.oauth_client_secret_enc, +-- and each group hook's HMAC secret in connection_webhook.secret_enc; +-- * deleting a variable kept a Secret's AES-GCM envelope in variable.value_encrypted, because +-- chk_variable_value_exclusive (0012) required one on every Secret row, deleted or not; +-- * revoking a storage credential kept every revision's envelope in storage_credential_revision.encrypted_payload, +-- because storage_credential_revision_guard (0126) refused every UPDATE; +-- * rotating or clearing a deployment storage default's secret repointed the template and kept the envelope it +-- replaced in storage_default_credential.encrypted_payload, because storage_default_credential_guard (0173) refused +-- every UPDATE. +-- +-- The application now blanks each of these when it retires one. This script relaxes the three guards just enough to +-- let it, and blanks what retirement already left behind. It changes no live secret: +-- +-- * credential: blanked only when Revoked or soft-deleted. Every other retirement path (Disconnect, unlinking an +-- identity, a departure) already blanked the payload; the rows left holding one are a deleted connection's. Nothing +-- reads a Revoked or soft-deleted credential's token: credential succession reads only ids and dates. +-- * provider_instance: the client secret is blanked only on a soft-deleted connection. Nothing restores one. +-- * connection_webhook: a soft-deleted connection's hooks are taken out of service (Cancelled) and their secrets +-- blanked. Ingestion refuses a Cancelled hook before it reads the secret, the registrar only ever claims an +-- Enqueued row, and a deleted connection's hooks could never be retired by the application: their registering +-- token is already blanked, so a hook still registered at the provider has to be removed there by hand. Its +-- external_id and owner_path stay, as the record of which hook that is. +-- * variable: a Secret may hold no ciphertext once deleted_date is set — and only then; a live Secret must still +-- carry one. Every read filters on deleted_date IS NULL and nothing restores a deleted variable. +-- * storage_credential_revision: exactly one UPDATE is admitted per revision — the one that blanks its payload and +-- changes nothing else — and only once its credential is Revoked. A Revoked credential resolves for nobody (the +-- secret resolver answers NotActive before it reads an envelope), revocation is terminal, and it is refused while a +-- stored location or a live profile still resolves through the credential. A superseded revision of an ACTIVE +-- credential is not blanked and may not be: a storage profile pins an exact revision (db:{id}:{revision}), so it +-- still decrypts the bytes stored through it, and routed bytes are permanent. A revision is never inserted blank. +-- * storage_default_credential: exactly one UPDATE is admitted per envelope — the one that blanks its payload and +-- changes nothing else — and only while no storage_default names it. The materializer is the only reader, and it +-- decrypts only the envelope its template names, read under a share lock on that template; an envelope is only +-- ever named by the template that appended it. Fingerprint and hint stay as history. An envelope is never inserted +-- blank. +-- +-- Safe on a populated database: the new checks are weaker than the old ones, so every existing row passes them; the +-- backfills touch only retired rows. Never refuses. +-- +-- Rollback. The blanked ciphertexts do not come back — that is the point — and so 0012's, 0126's and 0173's checks +-- cannot be put back as they were: once anything is shredded, by this script or by the application, a deleted Secret +-- variable holds NULL and a shredded revision or envelope holds '', exactly the rows those checks reject, so re-adding +-- them as written fails with 23514. Re-add them NOT VALID (enforced on every row written from then on, not on the rows +-- already shredded), and put 0173's trigger back on UPDATE and DELETE only: +-- +-- ALTER TABLE variable DROP CONSTRAINT chk_variable_value_exclusive; +-- ALTER TABLE variable ADD CONSTRAINT chk_variable_value_exclusive CHECK ((value_type = 'Secret' AND value_encrypted IS NOT NULL AND value_plain IS NULL) OR (value_type <> 'Secret' AND value_plain IS NOT NULL AND value_encrypted IS NULL)) NOT VALID; +-- ALTER TABLE storage_credential_revision DROP CONSTRAINT ck_storage_credential_revision_encrypted_payload; +-- ALTER TABLE storage_credential_revision ADD CONSTRAINT ck_storage_credential_revision_encrypted_payload CHECK (btrim(encrypted_payload) <> '') NOT VALID; +-- ALTER TABLE storage_default_credential DROP CONSTRAINT ck_storage_default_credential_encrypted_payload; +-- ALTER TABLE storage_default_credential ADD CONSTRAINT ck_storage_default_credential_encrypted_payload CHECK (btrim(encrypted_payload) <> '') NOT VALID; +-- DROP TRIGGER storage_default_credential_enforce_append_only ON storage_default_credential; +-- CREATE TRIGGER storage_default_credential_enforce_append_only BEFORE UPDATE OR DELETE ON storage_default_credential FOR EACH ROW EXECUTE FUNCTION storage_default_credential_guard(); +-- +-- then re-run, verbatim, 0126's CREATE OR REPLACE FUNCTION storage_credential_revision_guard() block and 0173's +-- CREATE OR REPLACE FUNCTION storage_default_credential_guard() block. Restoring those functions is safe on any +-- database: a trigger judges only the rows written after it. The hooks this script cancelled stay cancelled. +-- +-- Rolling the application back needs none of this: the previous build runs on this schema as it stands. Once the old +-- checks are back, though, this build cannot retire a secret — deleting a Secret variable fails with 23514 and revoking +-- a storage credential or editing a default's secret is refused — so before rolling forward again, delete this script's +-- journal row and let the next start apply it once more; every statement in it is safe to run twice: +-- +-- DELETE FROM schemaversions WHERE scriptname = '0249_shred_retired_secrets.sql'; + +-- ── Variables ──────────────────────────────────────────────────────────────── + +ALTER TABLE variable DROP CONSTRAINT IF EXISTS chk_variable_value_exclusive; + +ALTER TABLE variable ADD CONSTRAINT chk_variable_value_exclusive CHECK ( + (value_type = 'Secret' AND value_plain IS NULL AND (value_encrypted IS NOT NULL OR deleted_date IS NOT NULL)) + OR + (value_type <> 'Secret' AND value_plain IS NOT NULL AND value_encrypted IS NULL) +); + +UPDATE variable SET value_encrypted = NULL WHERE deleted_date IS NOT NULL AND value_encrypted IS NOT NULL; + +-- ── Storage credential revisions ───────────────────────────────────────────── + +ALTER TABLE storage_credential_revision DROP CONSTRAINT IF EXISTS ck_storage_credential_revision_encrypted_payload; + +ALTER TABLE storage_credential_revision ADD CONSTRAINT ck_storage_credential_revision_encrypted_payload CHECK (encrypted_payload = '' OR btrim(encrypted_payload) <> ''); + +COMMENT ON COLUMN storage_credential_revision.encrypted_payload IS + 'Complete IPayloadEncryptor/Data Protection ciphertext envelope for one provider SecretSchema payload; never plaintext. Empty once the credential is Revoked (0249).'; + +CREATE OR REPLACE FUNCTION storage_credential_revision_guard() RETURNS trigger AS $$ +DECLARE + credential_state VARCHAR(16); + latest_revision INTEGER; +BEGIN + -- The one UPDATE a revision ever admits: its payload blanked, every other column as it was, its credential Revoked. + -- Compared as whole rows minus the payload, so a column added later is covered too. + IF TG_OP = 'UPDATE' THEN + IF OLD.encrypted_payload <> '' AND NEW.encrypted_payload = '' + AND to_jsonb(NEW) - 'encrypted_payload' = to_jsonb(OLD) - 'encrypted_payload' THEN + SELECT state INTO credential_state + FROM storage_credential + WHERE team_id = OLD.team_id AND id = OLD.storage_credential_id; + + IF credential_state = 'Revoked' THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION + 'storage_credential_revision payload may be blanked only once its credential is Revoked — a profile pins an exact revision, and an Active credential still decrypts it for the bytes stored through it (credential_id=%, revision=%).', + OLD.storage_credential_id, OLD.revision; + END IF; + END IF; + + IF TG_OP <> 'INSERT' THEN + RAISE EXCEPTION + 'storage_credential_revision is immutable — % rejected (credential_id=%, revision=%). Append a new revision instead.', + TG_OP, OLD.storage_credential_id, OLD.revision; + END IF; + + IF NEW.encrypted_payload = '' THEN + RAISE EXCEPTION 'storage_credential_revision payload is required — only a revoked credential''s revision is ever blank (credential_id=%).', NEW.storage_credential_id; + END IF; + + -- Serialize append with revocation. If revocation commits first, this append fails; if append commits first, the + -- revocation follows it and becomes the terminal boundary. The tenant-bound FK remains the final missing-parent guard. + SELECT state INTO credential_state + FROM storage_credential + WHERE team_id = NEW.team_id AND id = NEW.storage_credential_id + FOR UPDATE; + + IF FOUND AND credential_state = 'Revoked' THEN + RAISE EXCEPTION 'storage_credential Revoked state is terminal — revision append rejected (id=%).', NEW.storage_credential_id; + END IF; + + IF FOUND THEN + SELECT COALESCE(MAX(revision), 0) INTO latest_revision + FROM storage_credential_revision + WHERE team_id = NEW.team_id AND storage_credential_id = NEW.storage_credential_id; + + IF NEW.revision <> latest_revision + 1 THEN + RAISE EXCEPTION + 'storage_credential_revision is a contiguous append-only sequence (credential_id=%, latest=%, attempted=%).', + NEW.storage_credential_id, latest_revision, NEW.revision; + END IF; + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +UPDATE storage_credential_revision AS revision + SET encrypted_payload = '' + FROM storage_credential AS credential + WHERE credential.team_id = revision.team_id + AND credential.id = revision.storage_credential_id + AND credential.state = 'Revoked' + AND revision.encrypted_payload <> ''; + +-- ── Git provider credentials and connections ───────────────────────────────── + +UPDATE credential SET encrypted_payload = '' WHERE encrypted_payload <> '' AND (deleted_date IS NOT NULL OR status = 'Revoked'); + +UPDATE provider_instance SET oauth_client_secret_enc = NULL WHERE deleted_date IS NOT NULL AND oauth_client_secret_enc IS NOT NULL; + +UPDATE connection_webhook AS hook + SET secret_enc = '', registration_status = 'Cancelled' + FROM provider_instance AS instance + WHERE instance.id = hook.provider_instance_id + AND instance.deleted_date IS NOT NULL + AND (hook.secret_enc <> '' OR hook.registration_status <> 'Cancelled'); + +-- ── Deployment storage default envelopes ───────────────────────────────────── + +ALTER TABLE storage_default_credential DROP CONSTRAINT IF EXISTS ck_storage_default_credential_encrypted_payload; + +ALTER TABLE storage_default_credential ADD CONSTRAINT ck_storage_default_credential_encrypted_payload CHECK (encrypted_payload = '' OR btrim(encrypted_payload) <> ''); + +COMMENT ON COLUMN storage_default_credential.encrypted_payload IS + 'Complete IPayloadEncryptor/Data Protection ciphertext envelope for one provider SecretSchema payload; never plaintext. Empty once no template names the envelope (0249).'; + +CREATE OR REPLACE FUNCTION storage_default_credential_guard() RETURNS trigger AS $$ +BEGIN + IF TG_OP = 'INSERT' THEN + IF NEW.encrypted_payload = '' THEN + RAISE EXCEPTION 'storage_default_credential payload is required -- only an envelope no template names is ever blank (id=%).', NEW.id; + END IF; + + RETURN NEW; + END IF; + + -- The one UPDATE an envelope ever admits: its payload blanked, every other column as it was, no template naming it. + -- Compared as whole rows minus the payload, so a column added later is covered too. + IF TG_OP = 'UPDATE' + AND OLD.encrypted_payload <> '' AND NEW.encrypted_payload = '' + AND to_jsonb(NEW) - 'encrypted_payload' = to_jsonb(OLD) - 'encrypted_payload' THEN + IF NOT EXISTS (SELECT 1 FROM storage_default WHERE credential_id = OLD.id) THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION + 'storage_default_credential payload may be blanked only once no template names it -- the materializer decrypts the envelope its template points at (id=%).', + OLD.id; + END IF; + + RAISE EXCEPTION + 'storage_default_credential is append-only -- % rejected (id=%). Insert a new envelope and repoint storage_default.credential_id.', + TG_OP, OLD.id; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS storage_default_credential_enforce_append_only ON storage_default_credential; + +CREATE TRIGGER storage_default_credential_enforce_append_only + BEFORE INSERT OR UPDATE OR DELETE ON storage_default_credential + FOR EACH ROW EXECUTE FUNCTION storage_default_credential_guard(); + +UPDATE storage_default_credential AS envelope + SET encrypted_payload = '' + WHERE envelope.encrypted_payload <> '' + AND NOT EXISTS (SELECT 1 FROM storage_default AS template WHERE template.credential_id = envelope.id); diff --git a/backend/src/CodeSpace.Core/Persistence/Entities/StorageCredentialRevision.cs b/backend/src/CodeSpace.Core/Persistence/Entities/StorageCredentialRevision.cs index d4378450c..71e36d13f 100644 --- a/backend/src/CodeSpace.Core/Persistence/Entities/StorageCredentialRevision.cs +++ b/backend/src/CodeSpace.Core/Persistence/Entities/StorageCredentialRevision.cs @@ -18,7 +18,8 @@ public class StorageCredentialRevision : IEntity /// /// Self-contained ciphertext produced by the established payload-encryption primitive. It is never plaintext JSON, - /// an individual key/token, or a model-credential reference. + /// an individual key/token, or a model-credential reference. Empty once the credential is Revoked — the one change a + /// revision ever admits (0249). /// public string EncryptedPayload { get; set; } = string.Empty; diff --git a/backend/src/CodeSpace.Core/Persistence/Entities/StorageDefaultCredential.cs b/backend/src/CodeSpace.Core/Persistence/Entities/StorageDefaultCredential.cs index b83a6912c..3db934cca 100644 --- a/backend/src/CodeSpace.Core/Persistence/Entities/StorageDefaultCredential.cs +++ b/backend/src/CodeSpace.Core/Persistence/Entities/StorageDefaultCredential.cs @@ -8,8 +8,9 @@ namespace CodeSpace.Core.Persistence.Entities; /// primitive already in the build. /// /// Rotation appends a new row and repoints ; the superseded envelope -/// stays as history. No runtime path decrypts this yet — the materializer lane is the intended reader, and it will -/// re-encrypt the secret into the team's own before anything resolves through it. +/// stays as history, its payload blanked once no template names it (0249). The materializer is the only reader: it +/// decrypts the envelope its template names and re-encrypts the secret into the team's own +/// before anything resolves through it. /// public class StorageDefaultCredential : IEntity { @@ -18,7 +19,10 @@ public class StorageDefaultCredential : IEntity /// Canonical major-versioned storage provider key, for example aliyun-oss/v1. public string ProviderTypeKey { get; set; } = string.Empty; - /// Self-contained ciphertext produced by the established payload-encryption primitive. Never plaintext JSON or an individual key. + /// + /// Self-contained ciphertext produced by the established payload-encryption primitive. Never plaintext JSON or an + /// individual key. Empty once no template names the envelope — the one change it ever admits (0249). + /// public string EncryptedPayload { get; set; } = string.Empty; /// Optional pre-sanitized operator hint (for example a short masked tail); never consumed at runtime. @@ -27,7 +31,7 @@ public class StorageDefaultCredential : IEntity /// SHA-256 of the encrypted envelope, allowing safe diagnostics without logging or indexing the envelope. public string EnvelopeFingerprint { get; set; } = string.Empty; - /// Immutable append timestamp and actor. There are no last-modified fields because mutation is illegal. + /// Immutable append timestamp and actor. There are no last-modified fields: the only change admitted is blanking the payload. public DateTimeOffset CreatedDate { get; set; } public Guid CreatedBy { get; set; } } diff --git a/backend/src/CodeSpace.Core/Persistence/Entities/Variable.cs b/backend/src/CodeSpace.Core/Persistence/Entities/Variable.cs index c0bcd21e5..b52ef400c 100644 --- a/backend/src/CodeSpace.Core/Persistence/Entities/Variable.cs +++ b/backend/src/CodeSpace.Core/Persistence/Entities/Variable.cs @@ -15,8 +15,9 @@ namespace CodeSpace.Core.Persistence.Entities; /// else routes through . /// /// The DB CHECK constraint chk_variable_value_exclusive guarantees exactly one -/// value column is populated, matching the ValueType. Application code does NOT need to -/// defensively null-check both columns — the constraint makes inconsistent rows impossible. +/// value column is populated, matching the ValueType — except on a deleted Secret, whose +/// ciphertext is dropped (0249) and which no read ever reaches. Application code does NOT +/// need to defensively null-check both columns of a live row. /// /// is denormalised even for Workflow scope so tenant-filter /// sweeps (the X-Team-Id middleware checks) work without joining workflow. @@ -54,7 +55,8 @@ public class Variable : IEntity, IAuditable /// /// AES-256-GCM envelope [nonce(12) || ciphertext || tag(16)] when - /// is . Null otherwise. + /// is . Null otherwise, and on a + /// deleted Secret (see VariableRetirement). /// public byte[]? ValueEncrypted { get; set; } diff --git a/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/StorageCredentialConfiguration.cs b/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/StorageCredentialConfiguration.cs index b1c9a9547..36667845a 100644 --- a/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/StorageCredentialConfiguration.cs +++ b/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/StorageCredentialConfiguration.cs @@ -40,7 +40,7 @@ public void Configure(EntityTypeBuilder builder) { builder.ToTable("storage_credential_revision", table => { - table.HasCheckConstraint("ck_storage_credential_revision_encrypted_payload", "btrim(encrypted_payload) <> ''"); + table.HasCheckConstraint("ck_storage_credential_revision_encrypted_payload", "encrypted_payload = '' OR btrim(encrypted_payload) <> ''"); table.HasCheckConstraint("ck_storage_credential_revision_envelope_fingerprint", "envelope_fingerprint ~ '^sha256:[0-9a-f]{64}$'"); table.HasCheckConstraint("ck_storage_credential_revision_number", "revision > 0"); table.HasCheckConstraint("ck_storage_credential_revision_provider_type_key", "provider_type_key ~ '^[a-z0-9][a-z0-9.-]*/v[1-9][0-9]*$'"); diff --git a/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/StorageDefaultConfiguration.cs b/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/StorageDefaultConfiguration.cs index a018206eb..8758f2dbe 100644 --- a/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/StorageDefaultConfiguration.cs +++ b/backend/src/CodeSpace.Core/Persistence/EntityConfigurations/StorageDefaultConfiguration.cs @@ -43,7 +43,7 @@ public void Configure(EntityTypeBuilder builder) { builder.ToTable("storage_default_credential", table => { - table.HasCheckConstraint("ck_storage_default_credential_encrypted_payload", "btrim(encrypted_payload) <> ''"); + table.HasCheckConstraint("ck_storage_default_credential_encrypted_payload", "encrypted_payload = '' OR btrim(encrypted_payload) <> ''"); table.HasCheckConstraint("ck_storage_default_credential_envelope_fingerprint", "envelope_fingerprint ~ '^sha256:[0-9a-f]{64}$'"); table.HasCheckConstraint("ck_storage_default_credential_provider_type_key", "provider_type_key ~ '^[a-z0-9][a-z0-9.-]*/v[1-9][0-9]*$'"); table.HasCheckConstraint("ck_storage_default_credential_safe_hint", "safe_hint IS NULL OR (char_length(safe_hint) BETWEEN 1 AND 32 AND btrim(safe_hint) <> '' AND safe_hint !~ '[[:cntrl:]]')"); diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs index e50fb9346..4c6c0b3fe 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs @@ -26,7 +26,7 @@ public sealed class CredentialService : ICredentialService, IScopedDependency private readonly ICurrentUser _currentUser; private readonly IPayloadEncryptor _encryptor; private readonly ICredentialPayloadSerializer _serializer; - private readonly IOAuthClientRegistry _oauthClients; + private readonly IOAuthGrantRevoker _grants; private readonly IProviderRegistry _registry; private readonly IProviderModuleCatalog _modules; private readonly IScopeChecker _scopeChecker; @@ -35,14 +35,14 @@ public sealed class CredentialService : ICredentialService, IScopedDependency private readonly ITeamMemberService _members; private readonly ILogger _logger; - public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IOAuthClientRegistry oauthClients, IProviderRegistry registry, IProviderModuleCatalog modules, IScopeChecker scopeChecker, ICredentialSuccessionService succession, ICredentialRevocationService revocation, ITeamMemberService members, ILogger logger) + public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IOAuthGrantRevoker grants, IProviderRegistry registry, IProviderModuleCatalog modules, IScopeChecker scopeChecker, ICredentialSuccessionService succession, ICredentialRevocationService revocation, ITeamMemberService members, ILogger logger) { _db = db; _currentTeam = currentTeam; _currentUser = currentUser; _encryptor = encryptor; _serializer = serializer; - _oauthClients = oauthClients; + _grants = grants; _registry = registry; _modules = modules; _scopeChecker = scopeChecker; @@ -272,13 +272,13 @@ await _db.Credential.Include(c => c.ProviderInstance).FirstOrDefaultAsync(c => c if (string.IsNullOrWhiteSpace(credential.ProviderInstance.OauthClientId) || string.IsNullOrWhiteSpace(credential.ProviderInstance.OauthClientSecretEnc)) return (Ok: false, Error: "provider instance missing OAuth client credentials"); - var payload = TryReadOAuthPayload(credential); + var payload = _grants.ReadPayload(credential); if (payload == null) return (Ok: false, Error: "credential payload is not a valid OAuth payload"); try { - await RevokeOAuthTokensAsync(credential.ProviderInstance, payload, cancellationToken).ConfigureAwait(false); + await _grants.RevokeAsync(credential.ProviderInstance, _encryptor.Decrypt(credential.ProviderInstance.OauthClientSecretEnc!), payload, wholeGrant: true, cancellationToken).ConfigureAwait(false); return (Ok: true, Error: null); } catch (Exception ex) @@ -288,49 +288,6 @@ await _db.Credential.Include(c => c.ProviderInstance).FirstOrDefaultAsync(c => c } } - private OAuthPayload? TryReadOAuthPayload(Credential credential) - { - try - { - var json = _encryptor.Decrypt(credential.EncryptedPayload); - return _serializer.Deserialize(AuthType.OAuth, json) as OAuthPayload; - } - catch - { - return null; - } - } - - private async Task RevokeOAuthTokensAsync(ProviderInstance instance, OAuthPayload payload, CancellationToken cancellationToken) - { - var client = _oauthClients.Get(instance.Provider); - var clientSecret = _encryptor.Decrypt(instance.OauthClientSecretEnc!); - - // Revoke refresh_token first (more durable — kills the grant on most providers), then - // access_token. Both are idempotent on the provider side; the caller's catch clears - // local payload regardless. - if (!string.IsNullOrEmpty(payload.RefreshToken)) - { - await client.RevokeAsync(new OAuthRevokeInput - { - Instance = instance, - ClientId = instance.OauthClientId!, - ClientSecret = clientSecret, - Token = payload.RefreshToken, - TokenTypeHint = "refresh_token" - }, cancellationToken).ConfigureAwait(false); - } - - await client.RevokeAsync(new OAuthRevokeInput - { - Instance = instance, - ClientId = instance.OauthClientId!, - ClientSecret = clientSecret, - Token = payload.AccessToken, - TokenTypeHint = "access_token" - }, cancellationToken).ConfigureAwait(false); - } - private static void MarkRevokedLocally(Credential credential, string? providerError) { // Clear encrypted payload — even if the provider call failed, removing token material diff --git a/backend/src/CodeSpace.Core/Services/Credentials/IRetiredSecretShredService.cs b/backend/src/CodeSpace.Core/Services/Credentials/IRetiredSecretShredService.cs new file mode 100644 index 000000000..660bab1d7 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/IRetiredSecretShredService.cs @@ -0,0 +1,16 @@ +namespace CodeSpace.Core.Services.Credentials; + +/// +/// Blanks the ciphertext of secrets that are retired but still decryptable. Every retirement in this build blanks its own +/// secret; this catches what a build older than migration 0249 retires without blanking — during a rolling deploy, or +/// after a rollback — since 0249's own backfill ran once, before those rows existed. +/// +public interface IRetiredSecretShredService +{ + /// + /// Blank every retired secret still holding ciphertext: exactly the rows 0249's backfill blanks. Idempotent and safe on + /// several workers at once — a blanked row is no longer a candidate, and each statement re-checks its row as it + /// writes. Returns how many rows this pass blanked. + /// + Task ShredAsync(CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/RetiredSecretShredService.cs b/backend/src/CodeSpace.Core/Services/Credentials/RetiredSecretShredService.cs new file mode 100644 index 000000000..63174a2f7 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/RetiredSecretShredService.cs @@ -0,0 +1,85 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Credentials; + +/// +/// The pass behind : 0249's backfill (0249_shred_retired_secrets.sql), kind +/// 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. +/// +/// 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 +/// by hand — and its OAuth grants are not revoked at the provider. Only this build's delete asks the provider, while the +/// tokens it needs still open. +/// +public sealed class RetiredSecretShredService : IRetiredSecretShredService, IScopedDependency +{ + private readonly CodeSpaceDbContext _db; + private readonly ILogger _logger; + + public RetiredSecretShredService(CodeSpaceDbContext db, ILogger logger) + { + _db = db; + _logger = logger; + } + + public async Task ShredAsync(CancellationToken cancellationToken) + { + var shredded = await ShredDeletedSecretVariablesAsync(cancellationToken).ConfigureAwait(false); + shredded += await ShredRevokedStorageCredentialRevisionsAsync(cancellationToken).ConfigureAwait(false); + shredded += await ShredRetiredGitCredentialsAsync(cancellationToken).ConfigureAwait(false); + shredded += await ShredDeletedConnectionClientSecretsAsync(cancellationToken).ConfigureAwait(false); + shredded += await RetireDeletedConnectionWebhooksAsync(cancellationToken).ConfigureAwait(false); + shredded += await ShredUnnamedStorageDefaultEnvelopesAsync(cancellationToken).ConfigureAwait(false); + + if (shredded > 0) _logger.LogInformation("Shredded {SecretCount} retired secrets that were still decryptable, retired by a build older than migration 0249", shredded); + + return shredded; + } + + private async Task ShredDeletedSecretVariablesAsync(CancellationToken cancellationToken) => + await _db.Variable + .Where(v => v.DeletedDate != null && v.ValueEncrypted != null) + .ExecuteUpdateAsync(s => s.SetProperty(v => v.ValueEncrypted, (byte[]?)null), cancellationToken) + .ConfigureAwait(false); + + private async Task ShredRevokedStorageCredentialRevisionsAsync(CancellationToken cancellationToken) => + await _db.StorageCredentialRevision + .Where(r => r.EncryptedPayload != string.Empty && _db.StorageCredential.Any(c => c.TeamId == r.TeamId && c.Id == r.StorageCredentialId && c.State == StorageCredentialState.Revoked)) + .ExecuteUpdateAsync(s => s.SetProperty(r => r.EncryptedPayload, string.Empty), cancellationToken) + .ConfigureAwait(false); + + private async Task ShredRetiredGitCredentialsAsync(CancellationToken cancellationToken) => + await _db.Credential + .Where(c => c.EncryptedPayload != string.Empty && (c.DeletedDate != null || c.Status == CredentialStatus.Revoked)) + .ExecuteUpdateAsync(s => s.SetProperty(c => c.EncryptedPayload, string.Empty), cancellationToken) + .ConfigureAwait(false); + + private async Task ShredDeletedConnectionClientSecretsAsync(CancellationToken cancellationToken) => + await _db.ProviderInstance + .Where(p => p.DeletedDate != null && p.OauthClientSecretEnc != null) + .ExecuteUpdateAsync(s => s.SetProperty(p => p.OauthClientSecretEnc, (string?)null), cancellationToken) + .ConfigureAwait(false); + + /// A deleted connection's group hooks: out of service and holding no secret, the provider's hook id kept as the record of what to remove there by hand. + private async Task RetireDeletedConnectionWebhooksAsync(CancellationToken cancellationToken) => + await _db.ConnectionWebhook + .Where(w => (w.SecretEnc != string.Empty || w.RegistrationStatus != RepositoryWebhookRegistrationStatus.Cancelled) && _db.ProviderInstance.Any(p => p.Id == w.ProviderInstanceId && p.DeletedDate != null)) + .ExecuteUpdateAsync(s => s.SetProperty(w => w.SecretEnc, string.Empty).SetProperty(w => w.RegistrationStatus, RepositoryWebhookRegistrationStatus.Cancelled), cancellationToken) + .ConfigureAwait(false); + + private async Task ShredUnnamedStorageDefaultEnvelopesAsync(CancellationToken cancellationToken) => + await _db.StorageDefaultCredential + .Where(e => e.EncryptedPayload != string.Empty && !_db.StorageDefault.Any(t => t.CredentialId == e.Id)) + .ExecuteUpdateAsync(s => s.SetProperty(e => e.EncryptedPayload, string.Empty), cancellationToken) + .ConfigureAwait(false); +} diff --git a/backend/src/CodeSpace.Core/Services/OAuth/CredentialPayloadReader.cs b/backend/src/CodeSpace.Core/Services/OAuth/CredentialPayloadReader.cs index 0b23d5748..917ec1abc 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/CredentialPayloadReader.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/CredentialPayloadReader.cs @@ -10,16 +10,22 @@ public sealed class CredentialPayloadReader : ICredentialPayloadReader, IScopedD public CredentialPayloadReader(CodeSpaceConnectionString connectionString) { _connectionString = connectionString; } - public async Task ReadEncryptedPayloadAsync(Guid credentialId, CancellationToken cancellationToken) + public async Task ReadEncryptedPayloadAsync(Guid credentialId, CancellationToken cancellationToken) => + await ReadScalarAsync("SELECT encrypted_payload FROM credential WHERE id = @id", credentialId, cancellationToken).ConfigureAwait(false) as string + ?? throw new InvalidOperationException($"Credential {credentialId} no longer exists"); + + public async Task IsRetiredAsync(Guid credentialId, CancellationToken cancellationToken) => + (bool)(await ReadScalarAsync("SELECT NOT EXISTS (SELECT 1 FROM credential WHERE id = @id AND deleted_date IS NULL)", credentialId, cancellationToken).ConfigureAwait(false))!; + + private async Task ReadScalarAsync(string sql, Guid credentialId, CancellationToken cancellationToken) { await using var conn = new NpgsqlConnection(_connectionString.Value); await conn.OpenAsync(cancellationToken).ConfigureAwait(false); await using var cmd = conn.CreateCommand(); - cmd.CommandText = "SELECT encrypted_payload FROM credential WHERE id = @id"; + cmd.CommandText = sql; cmd.Parameters.AddWithValue("@id", credentialId); - return await cmd.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false) as string - ?? throw new InvalidOperationException($"Credential {credentialId} no longer exists"); + return await cmd.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false); } } diff --git a/backend/src/CodeSpace.Core/Services/OAuth/GitHubOAuthClient.cs b/backend/src/CodeSpace.Core/Services/OAuth/GitHubOAuthClient.cs index 29c2e099b..ae848d236 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/GitHubOAuthClient.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/GitHubOAuthClient.cs @@ -88,14 +88,20 @@ public async Task RefreshAsync(OAuthRefreshInput input, Canc return await PostTokenAsync(input.Instance, form, cancellationToken).ConfigureAwait(false); } + /// + /// DELETE /applications/{client_id}/grant for a revoke: it ends the + /// user's grant — every access and refresh token GitHub ever issued that user under the client id, including one + /// another team connected through the same OAuth App holds. Otherwise DELETE /applications/{client_id}/token, + /// which ends the one access token presented. GitHub has no endpoint that ends a refresh token on its own, so outside + /// a whole-grant revoke a refresh token is not sent at all. Both endpoints authenticate as the app and take the token + /// as access_token; both are idempotent — a token already gone answers 404 or 422. + /// public async Task RevokeAsync(OAuthRevokeInput input, CancellationToken cancellationToken) { - // DELETE /applications/{client_id}/grant revokes the user's grant entirely — kills - // every access + refresh token GitHub ever issued under our client_id for that user. - // Surgical alternative is /token (revoke single token) but for a CodeSpace credential - // we always want the whole grant gone. Idempotent: subsequent calls return 404. + if (!input.WholeGrant && input.TokenTypeHint == "refresh_token") return; + var apiBase = ResolveApiBase(input.Instance); - var url = new Uri($"{apiBase}/applications/{Uri.EscapeDataString(input.ClientId)}/grant"); + var url = new Uri($"{apiBase}/applications/{Uri.EscapeDataString(input.ClientId)}/{(input.WholeGrant ? "grant" : "token")}"); using var http = _httpClientFactory.CreateClient(); using var request = new HttpRequestMessage(HttpMethod.Delete, url) diff --git a/backend/src/CodeSpace.Core/Services/OAuth/ICredentialPayloadReader.cs b/backend/src/CodeSpace.Core/Services/OAuth/ICredentialPayloadReader.cs index eadec5a92..c4522665a 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/ICredentialPayloadReader.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/ICredentialPayloadReader.cs @@ -10,4 +10,11 @@ namespace CodeSpace.Core.Services.OAuth; public interface ICredentialPayloadReader { Task ReadEncryptedPayloadAsync(Guid credentialId, CancellationToken cancellationToken); + + /// + /// True once the credential is soft-deleted — a retirement: a connection delete, a member's departure or a relink, + /// each of which blanks the token and takes the row out of the team's list — or gone. False while the row is listed, + /// a Disconnected one included: a Disconnect blanks the token and leaves the row Revoked in the list. + /// + Task IsRetiredAsync(Guid credentialId, CancellationToken cancellationToken); } diff --git a/backend/src/CodeSpace.Core/Services/OAuth/IOAuthClient.cs b/backend/src/CodeSpace.Core/Services/OAuth/IOAuthClient.cs index 95360e949..4f0ae92c3 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/IOAuthClient.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/IOAuthClient.cs @@ -65,6 +65,14 @@ public sealed record OAuthRevokeInput /// "access_token" or "refresh_token" — providers use this only as a hint. public string? TokenTypeHint { get; init; } + + /// + /// True ends the user's whole grant to the OAuth app where a provider revokes per grant: on GitHub, every token it ever + /// issued that user under , wherever it is held — including a token another team connected + /// through the same app. False ends only . A provider that revokes per token (GitLab, RFC 7009) + /// sends the same request either way. Default true: a Disconnect ends the grant, as it always has. + /// + public bool WholeGrant { get; init; } = true; } public sealed record OAuthTokenResponse diff --git a/backend/src/CodeSpace.Core/Services/OAuth/IOAuthGrantRevoker.cs b/backend/src/CodeSpace.Core/Services/OAuth/IOAuthGrantRevoker.cs new file mode 100644 index 000000000..4ca178dfd --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OAuth/IOAuthGrantRevoker.cs @@ -0,0 +1,25 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Credentials; + +namespace CodeSpace.Core.Services.OAuth; + +/// +/// The provider side of ending an OAuth credential: a Disconnect asks it for one credential, deleting a connection for +/// every credential connected through it. +/// +public interface IOAuthGrantRevoker +{ + /// The OAuth tokens a credential holds, or null when its stored payload does not open or does not read as OAuth tokens. + OAuthPayload? ReadPayload(Credential credential); + + /// + /// Ask the provider names to revoke the refresh token, when there is one, then the access + /// token, presenting the connection's OAuth app as OauthClientId and . The secret + /// is handed in, not read off the connection, so a caller that has already blanked it can still revoke. + /// is on every request: a Disconnect ends the + /// user's grant to the app; a connection delete ends only these tokens, because the grant also carries the user's + /// tokens in every other team connected through the same app. Throws what the provider answers; the caller decides + /// that the revoke is best effort. + /// + Task RevokeAsync(ProviderInstance instance, string clientSecret, OAuthPayload payload, bool wholeGrant, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/OAuth/OAuthGrantRevoker.cs b/backend/src/CodeSpace.Core/Services/OAuth/OAuthGrantRevoker.cs new file mode 100644 index 000000000..58ba0a543 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/OAuth/OAuthGrantRevoker.cs @@ -0,0 +1,61 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.OAuth; + +/// +/// Scoped, not singleton, so a scope that swaps the OAuth client registry (a test standing in for the provider) is the +/// registry this revoker asks. +/// +public sealed class OAuthGrantRevoker : IOAuthGrantRevoker, IScopedDependency +{ + private readonly IOAuthClientRegistry _oauthClients; + private readonly IPayloadEncryptor _encryptor; + private readonly ICredentialPayloadSerializer _serializer; + + public OAuthGrantRevoker(IOAuthClientRegistry oauthClients, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer) + { + _oauthClients = oauthClients; + _encryptor = encryptor; + _serializer = serializer; + } + + public OAuthPayload? ReadPayload(Credential credential) + { + try + { + var json = _encryptor.Decrypt(credential.EncryptedPayload); + return _serializer.Deserialize(AuthType.OAuth, json) as OAuthPayload; + } + catch + { + return null; + } + } + + public async Task RevokeAsync(ProviderInstance instance, string clientSecret, OAuthPayload payload, bool wholeGrant, CancellationToken cancellationToken) + { + var client = _oauthClients.Get(instance.Provider); + + // Revoke refresh_token first (more durable — kills the grant on most providers), then + // access_token. Both are idempotent on the provider side; the caller's catch clears + // local payload regardless. + if (!string.IsNullOrEmpty(payload.RefreshToken)) + await client.RevokeAsync(Input(instance, clientSecret, payload.RefreshToken, "refresh_token", wholeGrant), cancellationToken).ConfigureAwait(false); + + await client.RevokeAsync(Input(instance, clientSecret, payload.AccessToken, "access_token", wholeGrant), cancellationToken).ConfigureAwait(false); + } + + private static OAuthRevokeInput Input(ProviderInstance instance, string clientSecret, string token, string tokenTypeHint, bool wholeGrant) => new() + { + Instance = instance, + ClientId = instance.OauthClientId!, + ClientSecret = clientSecret, + Token = token, + TokenTypeHint = tokenTypeHint, + WholeGrant = wholeGrant + }; +} diff --git a/backend/src/CodeSpace.Core/Services/OAuth/OAuthTokenRefresher.cs b/backend/src/CodeSpace.Core/Services/OAuth/OAuthTokenRefresher.cs index 1ae5e3b43..4bba84745 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/OAuthTokenRefresher.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/OAuthTokenRefresher.cs @@ -137,15 +137,20 @@ private OAuthPayload Adopt(Credential credential, string storedPayload) } /// - /// The pair the provider issued for a credential revoked meanwhile is stored nowhere; revoke it there too, so the grant - /// the user disconnected does not stay live. Best effort, like the revocation itself. + /// The pair the provider issued for a credential revoked meanwhile is stored nowhere; revoke it there too, the way the + /// write that emptied the row ends tokens. A Disconnect ends the user's grant, so the pair goes with it and the grant + /// the user disconnected does not stay live. A retirement — a connection delete, a departure, a relink — soft-deletes + /// the row and ends tokens, never the grant: on GitHub the grant also carries the user's tokens in every other team + /// connected through the same OAuth App. Best effort, like the revocation itself. /// private async Task RevokeIssuedTokensAsync(ProviderContext context, OAuthPayload issued) { try { - await RevokeAtProviderAsync(context.Instance, issued.RefreshToken!, "refresh_token").ConfigureAwait(false); - await RevokeAtProviderAsync(context.Instance, issued.AccessToken, "access_token").ConfigureAwait(false); + var wholeGrant = !await _payloadReader.IsRetiredAsync(context.Credential.Id, CancellationToken.None).ConfigureAwait(false); + + await RevokeAtProviderAsync(context.Instance, issued.RefreshToken!, "refresh_token", wholeGrant).ConfigureAwait(false); + await RevokeAtProviderAsync(context.Instance, issued.AccessToken, "access_token", wholeGrant).ConfigureAwait(false); } catch (Exception ex) { @@ -153,9 +158,9 @@ private async Task RevokeIssuedTokensAsync(ProviderContext context, OAuthPayload } } - private async Task RevokeAtProviderAsync(ProviderInstance instance, string token, string tokenTypeHint) + private async Task RevokeAtProviderAsync(ProviderInstance instance, string token, string tokenTypeHint, bool wholeGrant) { - var input = new OAuthRevokeInput { Instance = instance, ClientId = instance.OauthClientId!, ClientSecret = _encryptor.Decrypt(instance.OauthClientSecretEnc!), Token = token, TokenTypeHint = tokenTypeHint }; + var input = new OAuthRevokeInput { Instance = instance, ClientId = instance.OauthClientId!, ClientSecret = _encryptor.Decrypt(instance.OauthClientSecretEnc!), Token = token, TokenTypeHint = tokenTypeHint, WholeGrant = wholeGrant }; await _oauthClients.Get(instance.Provider).RevokeAsync(input, CancellationToken.None).ConfigureAwait(false); } diff --git a/backend/src/CodeSpace.Core/Services/Projects/ProjectService.cs b/backend/src/CodeSpace.Core/Services/Projects/ProjectService.cs index 0a9cd1a07..d23c101dc 100644 --- a/backend/src/CodeSpace.Core/Services/Projects/ProjectService.cs +++ b/backend/src/CodeSpace.Core/Services/Projects/ProjectService.cs @@ -2,6 +2,7 @@ using System.Text.RegularExpressions; using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Services.Slugs; +using CodeSpace.Core.Services.Variables; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; using CodeSpace.Messages.Constants; @@ -324,17 +325,13 @@ public async Task DeleteAsync(Guid teamId, Guid projectId, Guid actorUserId, Can project.LastModifiedBy = actorUserId; // Cascade: soft-delete every project-scoped variable so {{project.{Slug}.X}} references - // resolve to null going forward. We don't hard-delete — operator audit needs the trail. + // resolve to null going forward. We don't hard-delete — operator audit needs the trail — + // but a Secret's ciphertext goes, as on every variable delete. var variableRows = await _db.Variable .Where(v => v.Scope == VariableScope.Project && v.ScopeId == projectId && v.DeletedDate == null) .ToListAsync(cancellationToken).ConfigureAwait(false); - foreach (var v in variableRows) - { - v.DeletedDate = now; - v.LastModifiedDate = now; - v.LastModifiedBy = actorUserId; - } + foreach (var v in variableRows) VariableRetirement.Retire(v, now, actorUserId); await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); _logger.LogInformation( diff --git a/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.Retirement.cs b/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.Retirement.cs new file mode 100644 index 000000000..62b889336 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.Retirement.cs @@ -0,0 +1,169 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.ProviderInstances; + +/// +/// What deleting a connection does to the hooks and tokens connected through it. Its group hooks come down first, while +/// the tokens that registered them still open (). Then each token is retired +/// the way every other retirement retires one (): token +/// blanked, Revoked, soft-deleted, repositories bound through it told they need a new credential, act-as identities +/// linked to it gone. The connection's own OAuth client secret is blanked by the delete. Nothing it held stays +/// decryptable in the database. +/// +/// Each OAuth credential's tokens are then revoked at the provider, best effort, and only once the delete has +/// committed: a provider asked from inside the transaction would have ended them even when the delete rolled back, leaving +/// Active credentials on a token the provider no longer honours. The tokens and the client secret that revoke needs are +/// read before they are blanked and held in memory until then. Only those tokens, never the user's whole grant: the grant +/// is the user's, per OAuth app, and another team connected through the same app holds tokens under it that this delete +/// must leave working — on GitHub, the grant delete would end them all. +/// +public sealed partial class ProviderInstanceService +{ + /// + /// The longest a delete waits on the provider for all of its revokes together. The delete has committed by then and + /// the caller is still waiting for its answer; a provider that never answers — often the very reason a connection is + /// being removed — must not hold that answer for a socket timeout per token. + /// + internal static readonly TimeSpan ProviderRevocationBudget = TimeSpan.FromSeconds(30); + + private const string RetiredByDelete = "Provider instance deleted"; + + /// + /// Takes the connection's group hooks out of service the way a switch back to per-repository scope does: each is + /// deleted at the provider best effort, the Registered rows go, the rest are cancelled. It has to happen here, before + /// : a hook is deleted with the token that registered it, and once that token is + /// blanked nothing could ever remove a hook still delivering the group's events. Like a forced unbind's hook teardown, + /// the provider is asked from inside the transaction; a delete that then rolled back would leave a hook row naming a + /// hook that is gone, never a live hook with no way to remove it. + /// + /// Then every hook row the connection keeps loses its secret. None is in service, so no delivery is verified + /// against it again — ingestion refuses a retired hook before it reads the secret. + /// + private async Task RetireConnectionWebhooksAsync(ProviderInstance instance, CancellationToken cancellationToken) + { + var retired = await _connectionHooks.RetireAllAsync(instance, cancellationToken).ConfigureAwait(false); + + await ShredConnectionWebhookSecretsAsync(instance.Id, cancellationToken).ConfigureAwait(false); + + if (retired > 0) _logger.LogInformation("Retired {ConnectionWebhookCount} group hooks of provider instance {ProviderInstanceId} before deleting it", retired, instance.Id); + } + + private async Task ShredConnectionWebhookSecretsAsync(Guid providerInstanceId, CancellationToken cancellationToken) => + await _db.ConnectionWebhook + .Where(w => w.ProviderInstanceId == providerInstanceId && w.SecretEnc != string.Empty) + .ExecuteUpdateAsync(s => s.SetProperty(w => w.SecretEnc, string.Empty), cancellationToken) + .ConfigureAwait(false); + + private async Task RetireCredentialsAsync(ProviderInstance instance, CancellationToken cancellationToken) + { + var credentials = await LoadUnretiredCredentialsAsync(instance.Id, cancellationToken).ConfigureAwait(false); + var grants = ReadGrants(instance, credentials); + + foreach (var credential in credentials) await RetireAsync(credential, cancellationToken).ConfigureAwait(false); + + await RevokeGrantsAfterCommitAsync(instance, grants, cancellationToken).ConfigureAwait(false); + + return credentials.Count; + } + + /// Every credential of the connection that still holds a token: not soft-deleted, and not already Revoked — a Disconnect blanked those. + private async Task> LoadUnretiredCredentialsAsync(Guid providerInstanceId, CancellationToken cancellationToken) => + await _db.Credential + .Where(c => c.ProviderInstanceId == providerInstanceId && c.DeletedDate == null && c.Status != CredentialStatus.Revoked) + .ToListAsync(cancellationToken).ConfigureAwait(false); + + private async Task RetireAsync(Credential credential, CancellationToken cancellationToken) + { + await _revocation.RetireAsync(credential, cancellationToken).ConfigureAwait(false); + + credential.LastError = RetiredByDelete; + } + + /// + /// What revoking each OAuth credential's tokens at the provider takes, read while the credentials and the connection still hold it. + /// Nothing when the connection has no OAuth app (only personal and group tokens, which no provider endpoint revokes), + /// or when its client secret no longer opens. + /// + private OAuthGrants ReadGrants(ProviderInstance instance, IReadOnlyList credentials) + { + var clientSecret = ReadClientSecret(instance); + + if (clientSecret == null) return OAuthGrants.None; + + var tokens = credentials + .Where(c => c.AuthType == AuthType.OAuth) + .Select(c => (c.Id, Payload: _grants.ReadPayload(c))) + .Where(grant => grant.Payload != null) + .Select(grant => (grant.Id, grant.Payload!)) + .ToList(); + + return new OAuthGrants(clientSecret, tokens); + } + + private string? ReadClientSecret(ProviderInstance instance) + { + if (string.IsNullOrWhiteSpace(instance.OauthClientId) || string.IsNullOrWhiteSpace(instance.OauthClientSecretEnc)) return null; + + try + { + return _encryptor.Decrypt(instance.OauthClientSecretEnc); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "The OAuth client secret of provider instance {ProviderInstanceId} does not open; its credentials are retired here but cannot be revoked at the provider", instance.Id); + return null; + } + } + + private async Task RevokeGrantsAfterCommitAsync(ProviderInstance instance, OAuthGrants grants, CancellationToken cancellationToken) + { + if (grants.Tokens.Count == 0) return; + + await _postCommit.RunAfterCommitAsync(ct => RevokeGrantsAsync(instance, grants, ct), cancellationToken).ConfigureAwait(false); + } + + private async Task RevokeGrantsAsync(ProviderInstance instance, OAuthGrants grants, CancellationToken cancellationToken) + { + using var budget = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + budget.CancelAfter(ProviderRevocationBudget); + + foreach (var (credentialId, payload) in grants.Tokens) await RevokeGrantAsync(instance, grants.ClientSecret, credentialId, payload, budget.Token).ConfigureAwait(false); + } + + private async Task RevokeGrantAsync(ProviderInstance instance, string clientSecret, Guid credentialId, OAuthPayload payload, CancellationToken cancellationToken) + { + try + { + await _grants.RevokeAsync(instance, clientSecret, payload, wholeGrant: false, cancellationToken).ConfigureAwait(false); + + _logger.LogInformation("Revoked the OAuth tokens of credential {CredentialId} at the provider after its provider instance {ProviderInstanceId} was deleted", credentialId, instance.Id); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Revoking the OAuth tokens of credential {CredentialId} at the provider failed after its provider instance {ProviderInstanceId} was deleted; the token is already gone from CodeSpace and stays valid at the provider until it expires or is revoked there", credentialId, instance.Id); + } + } + + /// + /// The connection's client secret and each OAuth credential's tokens, in memory only, between the delete's read of + /// them and the provider's revoke. A class, not a record: a record's generated ToString would print the secret. + /// + private sealed class OAuthGrants + { + public static readonly OAuthGrants None = new(string.Empty, Array.Empty<(Guid, OAuthPayload)>()); + + public OAuthGrants(string clientSecret, IReadOnlyList<(Guid CredentialId, OAuthPayload Payload)> tokens) + { + ClientSecret = clientSecret; + Tokens = tokens; + } + + public string ClientSecret { get; } + + public IReadOnlyList<(Guid CredentialId, OAuthPayload Payload)> Tokens { get; } + } +} diff --git a/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs b/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs index c0eabf03b..0cad3147e 100644 --- a/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs +++ b/backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs @@ -3,9 +3,11 @@ using CodeSpace.Core.Persistence.Entities; using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.Identity; +using CodeSpace.Core.Services.OAuth; using CodeSpace.Core.Middlewares.Transactional; using CodeSpace.Core.Services.Redaction; using CodeSpace.Core.Services.RepositoryBinding; +using CodeSpace.Core.Services.Webhooks.Registration; using CodeSpace.Core.Services.Webhooks.Scope; using CodeSpace.Messages.Commands.ProviderInstances; using CodeSpace.Messages.Dtos.ProviderInstances; @@ -17,7 +19,7 @@ namespace CodeSpace.Core.Services.ProviderInstances; -public sealed class ProviderInstanceService : IProviderInstanceService, IScopedDependency +public sealed partial class ProviderInstanceService : IProviderInstanceService, IScopedDependency { private readonly CodeSpaceDbContext _db; private readonly ICurrentTeam _currentTeam; @@ -25,11 +27,14 @@ public sealed class ProviderInstanceService : IProviderInstanceService, IScopedD private readonly IPayloadEncryptor _encryptor; private readonly IRepositoryBindingService _binding; private readonly IWebhookScopeTransitionService _webhookScope; + private readonly IConnectionWebhookProvisioner _connectionHooks; + private readonly ICredentialRevocationService _revocation; + private readonly IOAuthGrantRevoker _grants; private readonly IPostCommitActions _postCommit; private readonly TimeProvider _clock; private readonly ILogger _logger; - public ProviderInstanceService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, IRepositoryBindingService binding, IWebhookScopeTransitionService webhookScope, IPostCommitActions postCommit, TimeProvider clock, ILogger logger) + public ProviderInstanceService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, IRepositoryBindingService binding, IWebhookScopeTransitionService webhookScope, IConnectionWebhookProvisioner connectionHooks, ICredentialRevocationService revocation, IOAuthGrantRevoker grants, IPostCommitActions postCommit, TimeProvider clock, ILogger logger) { _db = db; _currentTeam = currentTeam; @@ -37,6 +42,9 @@ public ProviderInstanceService(CodeSpaceDbContext db, ICurrentTeam currentTeam, _encryptor = encryptor; _binding = binding; _webhookScope = webhookScope; + _connectionHooks = connectionHooks; + _revocation = revocation; + _grants = grants; _postCommit = postCommit; _clock = clock; _logger = logger; @@ -230,8 +238,10 @@ public async Task DeleteAsync(Guid providerInstanc var instance = await LoadOwnedInstanceAsync(providerInstanceId, teamId, cancellationToken).ConfigureAwait(false); var unboundCount = await ResolveBoundRepositoriesAsync(instance, force, cancellationToken).ConfigureAwait(false); - var revokedCount = await CascadeRevokeCredentialsAsync(instance.Id, cancellationToken).ConfigureAwait(false); + await RetireConnectionWebhooksAsync(instance, cancellationToken).ConfigureAwait(false); + var revokedCount = await RetireCredentialsAsync(instance, cancellationToken).ConfigureAwait(false); + instance.OauthClientSecretEnc = null; instance.DeletedDate = _clock.GetUtcNow(); return new DeleteProviderInstanceResult @@ -355,24 +365,6 @@ private async Task ResolveBoundRepositoriesAsync(ProviderInstance instance, return activeRepoIds.Count; } - private async Task CascadeRevokeCredentialsAsync(Guid providerInstanceId, CancellationToken cancellationToken) - { - var now = _clock.GetUtcNow(); - - var activeCredentials = await _db.Credential - .Where(c => c.ProviderInstanceId == providerInstanceId && c.DeletedDate == null && c.Status == CredentialStatus.Active) - .ToListAsync(cancellationToken).ConfigureAwait(false); - - foreach (var credential in activeCredentials) - { - credential.Status = CredentialStatus.Revoked; - credential.DeletedDate = now; - credential.LastError = "Provider instance deleted"; - } - - return activeCredentials.Count; - } - /// Empty / whitespace-only client IDs collapse to null so the form's "not yet configured" state and a literal empty string aren't treated as distinct identities by the unique index. private static string? NormaliseClientId(string? raw) => string.IsNullOrWhiteSpace(raw) ? null : raw.Trim(); diff --git a/backend/src/CodeSpace.Core/Services/Variables/VariableRetirement.cs b/backend/src/CodeSpace.Core/Services/Variables/VariableRetirement.cs new file mode 100644 index 000000000..a8a9823bc --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Variables/VariableRetirement.cs @@ -0,0 +1,21 @@ +using CodeSpace.Core.Persistence.Entities; + +namespace CodeSpace.Core.Services.Variables; + +/// +/// What deleting a variable leaves: the row, for the audit trail, and nothing a secret could be recovered from. Every +/// read of a variable filters on deleted_date IS NULL and nothing restores a deleted one, so a deleted Secret's +/// ciphertext could only ever be opened by someone holding a copy of the database and the master key. A plain value is +/// no secret and stays as the record of what the variable held. One rule for every way a variable is deleted — on its +/// own, or with the project it belongs to. +/// +public static class VariableRetirement +{ + public static void Retire(Variable row, DateTimeOffset now, Guid actorUserId) + { + row.DeletedDate = now; + row.LastModifiedDate = now; + row.LastModifiedBy = actorUserId; + row.ValueEncrypted = null; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Variables/VariableService.cs b/backend/src/CodeSpace.Core/Services/Variables/VariableService.cs index d79531e1b..a2a3b284f 100644 --- a/backend/src/CodeSpace.Core/Services/Variables/VariableService.cs +++ b/backend/src/CodeSpace.Core/Services/Variables/VariableService.cs @@ -84,10 +84,7 @@ public async Task DeleteAsync(VariableScope scope, Guid scopeId, Guid expectedTe var row = await LoadActiveRowAsync(scope, scopeId, name, cancellationToken).ConfigureAwait(false); if (row == null) return; // idempotent — no-op on missing-name - var now = DateTimeOffset.UtcNow; - row.DeletedDate = now; - row.LastModifiedDate = now; - row.LastModifiedBy = actorUserId; + VariableRetirement.Retire(row, DateTimeOffset.UtcNow, actorUserId); await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Artifacts/Credentials/StorageCredentialService.cs b/backend/src/CodeSpace.Core/Services/Workflows/Artifacts/Credentials/StorageCredentialService.cs index 2619d4d9a..83b0bb6f3 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Artifacts/Credentials/StorageCredentialService.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Artifacts/Credentials/StorageCredentialService.cs @@ -130,9 +130,26 @@ public async Task CreateAsync(Guid teamId, Guid actor credential.RevokedBy = actorId; await SaveConcurrentAsync("The storage credential changed before it could be revoked.", cancellationToken).ConfigureAwait(false); + await ShredRevisionsAsync(teamId, credential.Id, cancellationToken).ConfigureAwait(false); return Metadata(Projection(credential, current)); } + /// + /// Blank the encrypted payload of every revision of a credential that has just been revoked; revision, provider, + /// hint, envelope fingerprint and authorship stay for the audit trail. Nothing reads them again — the resolver admits + /// only an Active credential, revocation is terminal, and it was refused while a stored location or a live profile + /// still resolved through the credential — so the ciphertext could only ever be opened from a database copy. + /// + /// After the revocation is saved, and as its own statement, because the revision guard admits the blanking only + /// once it sees the credential Revoked. Superseded revisions of an Active credential are never blanked: a profile + /// pins an exact revision, so each of them still decrypts the bytes stored through it. + /// + private async Task ShredRevisionsAsync(Guid teamId, Guid credentialId, CancellationToken cancellationToken) => + await _db.StorageCredentialRevision + .Where(value => value.TeamId == teamId && value.StorageCredentialId == credentialId && value.EncryptedPayload != string.Empty) + .ExecuteUpdateAsync(setters => setters.SetProperty(value => value.EncryptedPayload, string.Empty), cancellationToken) + .ConfigureAwait(false); + /// /// Revocation is terminal and the secret resolver admits none but an Active credential, so revoking one a stored /// object still depends on makes those bytes permanently unreadable — the same stranding the profile lifecycle diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Artifacts/Defaults/StorageDefaultService.cs b/backend/src/CodeSpace.Core/Services/Workflows/Artifacts/Defaults/StorageDefaultService.cs index c8daaf98b..d5436e0ff 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Artifacts/Defaults/StorageDefaultService.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Artifacts/Defaults/StorageDefaultService.cs @@ -73,12 +73,15 @@ public async Task CreateAsync(Guid actorId, CreateStorageD var prepared = PrepareTemplate(Input(command, template.DataClassTypeKey)); var now = _clock.GetUtcNow(); + var previousCredentialId = template.CredentialId; var credentialId = await ResolveCredentialAsync(template, command, actorId, now).ConfigureAwait(false); Apply(template, prepared, actorId, now); template.Revision = checked(template.Revision + 1); template.CredentialId = credentialId; await SaveAsync("The deployment storage default changed before this edit could be applied.", cancellationToken).ConfigureAwait(false); + await ShredReplacedEnvelopeAsync(previousCredentialId, credentialId, cancellationToken).ConfigureAwait(false); + return await RequireAsync(template.Id, cancellationToken).ConfigureAwait(false); } @@ -140,7 +143,8 @@ private PreparedTemplate PrepareTemplate(TemplateInput input) /// /// Which envelope the edited template points at: a newly appended one when a secret is supplied, none when the - /// operator clears it, otherwise the one it already had. Superseded envelopes are never overwritten in place. + /// operator clears it, otherwise the one it already had. A superseded envelope is never rewritten, only blanked once + /// the edit has saved (). /// /// A RETAINED envelope must still belong to the template's provider. Repointing a template at a different /// provider while silently keeping the old provider's secret would leave a template that reads as complete and @@ -158,6 +162,24 @@ private PreparedTemplate PrepareTemplate(TemplateInput input) return retained; } + /// + /// Blanks the envelope an edit stopped naming: the one a new secret replaced, or the one a clear detached. Only the + /// materializer decrypts an envelope, and only the one its template names, under a share lock on that template, so + /// this one has no reader left; kept, it would leave a deployment-wide cloud key decryptable from every copy of the + /// database for good. Its fingerprint and hint stay as history. Runs after the repoint has saved, because 0249's guard + /// admits the blanking only once no template names the envelope, and asks the same here so an envelope some other + /// template still names is left alone rather than failing the edit. + /// + private async Task ShredReplacedEnvelopeAsync(Guid? previousCredentialId, Guid? credentialId, CancellationToken cancellationToken) + { + if (previousCredentialId is not { } replaced || replaced == credentialId) return; + + await _db.StorageDefaultCredential + .Where(envelope => envelope.Id == replaced && envelope.EncryptedPayload != string.Empty && !_db.StorageDefault.Any(template => template.CredentialId == envelope.Id)) + .ExecuteUpdateAsync(setters => setters.SetProperty(envelope => envelope.EncryptedPayload, string.Empty), cancellationToken) + .ConfigureAwait(false); + } + private async Task EnsureRetainedCredentialMatchesAsync(Guid credentialId, string providerTypeKey) { var module = RequireModule(providerTypeKey); diff --git a/backend/src/CodeSpace.Messages/Authorization/IRequireCredentialSpend.cs b/backend/src/CodeSpace.Messages/Authorization/IRequireCredentialSpend.cs new file mode 100644 index 000000000..28e8cad4a --- /dev/null +++ b/backend/src/CodeSpace.Messages/Authorization/IRequireCredentialSpend.cs @@ -0,0 +1,15 @@ +namespace CodeSpace.Messages.Authorization; + +/// +/// Marker — a request that presents one credential, named by its id, to its provider on the caller's behalf. Its owner +/// may spend their own Personal credential; anyone else needs TeamPermissions.ReposManage. +/// +/// Extends , so the credential is first proven to be the X-Team-Id +/// team's and the caller a member of it; this tier only decides whether membership is enough. It is not: a provider +/// answers with everything the token's account can see, so on membership alone any Viewer could spend the Owner's +/// personal token and read back private repositories outside the team. The listings such a request serves feed a bind, +/// and a bind takes repos.manage — the same bar here, for a teammate's credential and for the team's own. +/// +public interface IRequireCredentialSpend : IRequireCredentialAccess +{ +} diff --git a/backend/src/CodeSpace.Messages/Commands/Credentials/ShredRetiredSecretsCommand.cs b/backend/src/CodeSpace.Messages/Commands/Credentials/ShredRetiredSecretsCommand.cs new file mode 100644 index 000000000..42c1aa068 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Commands/Credentials/ShredRetiredSecretsCommand.cs @@ -0,0 +1,19 @@ +using CodeSpace.Messages.Mediation; + +namespace CodeSpace.Messages.Commands.Credentials; + +/// +/// Blank the ciphertext of every secret that is retired but still decryptable — the rows migration 0249 blanks on upgrade: +/// deleted Secret variables, revoked or soft-deleted git tokens, a deleted connection's OAuth client secret and group +/// hooks, every revision of a revoked storage credential, and deployment-default envelopes no template names. Retiring +/// one blanks it in this build; 0249 blanks what earlier builds left, but runs once, so anything a build older than 0249 +/// retires afterwards — an older pod still serving during a rolling deploy, or the whole fleet after a rollback — keeps +/// its ciphertext until this runs. Idempotent: a blanked row is no longer a candidate, so an idle pass changes nothing. +/// NOT tenant-scoped: a system-wide repair that runs without an actor context (mirrors +/// RetireDepartedMembersCredentialsCommand). +/// +/// NOT transactional (): each kind of secret is blanked by one statement that +/// commits on its own, so a statement that fails leaves the others' work in place and its own rows candidates for the +/// next pass. One transaction around the pass would hold every candidate row's lock until the last statement. +/// +public sealed record ShredRetiredSecretsCommand : ICommand, INonTransactionalCommand; diff --git a/backend/src/CodeSpace.Messages/Commands/ProviderInstances/DeleteProviderInstanceCommand.cs b/backend/src/CodeSpace.Messages/Commands/ProviderInstances/DeleteProviderInstanceCommand.cs index 70f68e8a1..41d377af6 100644 --- a/backend/src/CodeSpace.Messages/Commands/ProviderInstances/DeleteProviderInstanceCommand.cs +++ b/backend/src/CodeSpace.Messages/Commands/ProviderInstances/DeleteProviderInstanceCommand.cs @@ -16,8 +16,12 @@ namespace CodeSpace.Messages.Commands.ProviderInstances; /// "Unbind all and remove" one-click path the UI offers when the operator explicitly /// confirms the impact. /// -/// Active credentials are always cascade-revoked in the same transaction — they're -/// useless once the provider is gone, and leaving them as Active would lie to the UI. +/// The connection's group hooks are deleted at the provider first, best effort, while the +/// tokens that registered them still work; the hook rows it keeps are out of service and lose +/// their secrets. Active credentials are then retired in the same transaction — token blanked, +/// Revoked, soft-deleted — and the instance's OAuth client secret is blanked with it, so nothing +/// the connection held stays decryptable. Each OAuth grant is then revoked at the provider, best +/// effort, after the transaction commits. /// public sealed record DeleteProviderInstanceCommand : ICommand, IRequireTeamPermission { diff --git a/backend/src/CodeSpace.Messages/Commands/Storage/StorageDefaultCommands.cs b/backend/src/CodeSpace.Messages/Commands/Storage/StorageDefaultCommands.cs index 369f64dae..d4eb7786f 100644 --- a/backend/src/CodeSpace.Messages/Commands/Storage/StorageDefaultCommands.cs +++ b/backend/src/CodeSpace.Messages/Commands/Storage/StorageDefaultCommands.cs @@ -37,7 +37,8 @@ public sealed record CreateStorageDefaultCommand : ICommand /// Replaces the template's configuration and advances its revision. Omitting Secret keeps the attached /// envelope; supplying one appends a new envelope and repoints the template at it; ClearCredential detaches -/// the envelope for a provider that needs none. Superseded envelopes are never overwritten in place. +/// the envelope for a provider that needs none. A replaced or detached envelope is never rewritten: it keeps its +/// fingerprint and hint as history, and its payload is blanked, since nothing reads it any more. /// public sealed record UpdateStorageDefaultCommand : ICommand, IRequireGlobalPermission { diff --git a/backend/src/CodeSpace.Messages/Queries/Credentials/ListAccessibleRepositoriesQuery.cs b/backend/src/CodeSpace.Messages/Queries/Credentials/ListAccessibleRepositoriesQuery.cs index 477af217c..f5e540338 100644 --- a/backend/src/CodeSpace.Messages/Queries/Credentials/ListAccessibleRepositoriesQuery.cs +++ b/backend/src/CodeSpace.Messages/Queries/Credentials/ListAccessibleRepositoriesQuery.cs @@ -4,7 +4,12 @@ namespace CodeSpace.Messages.Queries.Credentials; -public sealed record ListAccessibleRepositoriesQuery : IQuery, IRequireCredentialAccess +/// +/// Lists the repositories a credential's account can see at its provider — the picker a bind feeds from. It presents the +/// credential to the provider, so it is : the owner of a Personal credential, or a +/// repos.manage holder; never a member spending a teammate's or the team's token on membership alone. +/// +public sealed record ListAccessibleRepositoriesQuery : IQuery, IRequireCredentialSpend { /// Default page size — matches the PR-list default and keeps the picker snappy even on accounts with thousands of repos. public const int DefaultPerPage = 30; diff --git a/backend/tests/CodeSpace.E2ETests/Credentials/AccessibleRepositoriesGateE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Credentials/AccessibleRepositoriesGateE2ETests.cs new file mode 100644 index 000000000..dc6b1962e --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Credentials/AccessibleRepositoriesGateE2ETests.cs @@ -0,0 +1,95 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.E2ETests.Credentials; + +/// +/// Listing what a credential reaches (GET /api/credentials/{id}/accessible-repositories) presents that credential to the +/// provider. On membership alone a Viewer could make the platform call GitLab with the Owner's personal token and read +/// back every project the Owner's account can see — private ones outside the team included. Now a Personal credential +/// lists for its owner or a repos.manage holder, and a team-service credential for a repos.manage holder; everyone else +/// is refused before the token leaves the platform. +/// +/// Driven through the real HTTP surface (POST /api/provider-instances, POST /api/credentials/pat and +/// /group-access-token, GET accessible-repositories), the real mediator pipeline and the real GitLab provider over +/// NGitLab, against a GitLab on a loopback port that records which token each request carried. +/// +/// Fidelity: HIGH. Only GitLab is a stub, and it is a real socket the production client speaks HTTP to. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class AccessibleRepositoriesGateE2ETests : IClassFixture +{ + private readonly TenantHarness _tenants; + + public AccessibleRepositoriesGateE2ETests(TaskLaunchApiFactory factory) { _tenants = new TenantHarness(factory); } + + public enum Holder { OwnersPersonal, MembersPersonal, Team } + + [Theory] + [InlineData(TeamRole.Viewer, Holder.OwnersPersonal, HttpStatusCode.Forbidden)] + [InlineData(TeamRole.Member, Holder.OwnersPersonal, HttpStatusCode.Forbidden)] + [InlineData(TeamRole.Member, Holder.Team, HttpStatusCode.Forbidden)] + [InlineData(TeamRole.Viewer, Holder.Team, HttpStatusCode.Forbidden)] + [InlineData(TeamRole.Member, Holder.MembersPersonal, HttpStatusCode.OK)] + [InlineData(TeamRole.Owner, Holder.OwnersPersonal, HttpStatusCode.OK)] + [InlineData(TeamRole.Admin, Holder.OwnersPersonal, HttpStatusCode.OK)] + [InlineData(TeamRole.Admin, Holder.Team, HttpStatusCode.OK)] + public async Task Only_the_owner_of_a_personal_credential_or_a_repos_manage_holder_lists_what_it_reaches(TeamRole caller, Holder holder, HttpStatusCode expected) + { + await using var gitLab = RecordingProviderStub.Start(); + var world = await SeedAsync(gitLab.BaseUrl); + var (credentialId, token) = world.Credential(holder); + + var listing = await _tenants.SendAsync(HttpMethod.Get, $"/api/credentials/{credentialId}/accessible-repositories", world.User(caller), world.Team); + + listing.StatusCode.ShouldBe(expected, $"{caller} listing through {holder} — {await TenantHarness.DescribeAsync(listing)}"); + + if (expected == HttpStatusCode.OK) + { + gitLab.Carrying(token).ShouldNotBeEmpty("fixture check: an allowed listing presents the token to GitLab"); + return; + } + + gitLab.Carrying(token).ShouldBeEmpty($"{caller} was refused before the {holder} token was ever presented to the provider"); + } + + private sealed record World(Guid Team, Guid Owner, Guid Admin, Guid Member, Guid Viewer, (Guid, string) OwnersPat, (Guid, string) MembersPat, (Guid, string) TeamToken) + { + public Guid User(TeamRole role) => role switch { TeamRole.Owner => Owner, TeamRole.Admin => Admin, TeamRole.Member => Member, _ => Viewer }; + + public (Guid Id, string Token) Credential(Holder holder) => holder switch { Holder.OwnersPersonal => OwnersPat, Holder.MembersPersonal => MembersPat, _ => TeamToken }; + } + + /// A team of four roles on one GitLab connection: the Owner's personal token, the Member's (added for them by the Owner), and a team-service group token. + private async Task SeedAsync(string gitLabUrl) + { + var owner = await _tenants.AccountAsync(); + var admin = await _tenants.AccountAsync(); + var member = await _tenants.AccountAsync(); + var viewer = await _tenants.AccountAsync(); + var team = await _tenants.TeamAsync((owner, TeamRole.Owner), (admin, TeamRole.Admin), (member, TeamRole.Member), (viewer, TeamRole.Viewer)); + var canary = Guid.NewGuid().ToString("N"); + var instanceId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/provider-instances", owner, team, new { provider = "GitLab", displayName = "gitlab", baseUrl = gitLabUrl })); + + var ownersPat = $"glpat-owner-{canary}"; + var membersPat = $"glpat-member-{canary}"; + var teamToken = $"glgat-team-{canary}"; + var ownersId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/pat", owner, team, new { providerInstanceId = instanceId, displayName = "owner pat", token = ownersPat })); + var membersId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/pat", owner, team, new { providerInstanceId = instanceId, ownerUserId = member, displayName = "member pat", token = membersPat })); + var teamId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/group-access-token", owner, team, new { providerInstanceId = instanceId, displayName = "team token", token = teamToken })); + + return new World(team, owner, admin, member, viewer, (ownersId, ownersPat), (membersId, membersPat), (teamId, teamToken)); + } + + private static async Task CreatedIdAsync(HttpResponseMessage response) + { + response.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(response)); + + return (await response.Content.ReadFromJsonAsync()).GetProperty("id").GetGuid(); + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Credentials/DeletedVariableShredE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Credentials/DeletedVariableShredE2ETests.cs new file mode 100644 index 000000000..2172a7ea3 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Credentials/DeletedVariableShredE2ETests.cs @@ -0,0 +1,51 @@ +using System.Net; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.E2ETests.Credentials; + +/// +/// DELETE /api/team-variables/{name} on a Secret leaves its row for the audit trail and no ciphertext in it. Before, the +/// row kept its AES-GCM envelope forever, so a deleted secret stayed recoverable from any database copy by whoever held +/// the variable master key. +/// +/// Driven through the real HTTP surface (PUT, DELETE and GET /api/team-variables), the real mediator pipeline and +/// its transaction, real encryption and Postgres. Fidelity: HIGH — nothing is stubbed. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class DeletedVariableShredE2ETests : IClassFixture +{ + private readonly TenantHarness _tenants; + + public DeletedVariableShredE2ETests(TaskLaunchApiFactory factory) { _tenants = new TenantHarness(factory); } + + [Fact] + public async Task Deleting_a_secret_team_variable_leaves_its_row_without_ciphertext_and_frees_the_name() + { + var owner = await _tenants.AccountAsync(); + var team = await _tenants.TeamAsync((owner, TeamRole.Owner)); + var body = new { name = "DEPLOY_TOKEN", valueType = "Secret", value = $"CANARY-deleted-variable-{Guid.NewGuid():N}" }; + + await ShouldSucceedAsync(await _tenants.SendAsync(HttpMethod.Put, "/api/team-variables/DEPLOY_TOKEN", owner, team, body), "set"); + await ShouldSucceedAsync(await _tenants.SendAsync(HttpMethod.Delete, "/api/team-variables/DEPLOY_TOKEN", owner, team), "delete"); + + var deleted = await _tenants.DbAsync(db => db.Variable.AsNoTracking().SingleAsync(v => v.TeamId == team && v.Name == "DEPLOY_TOKEN" && v.DeletedDate != null)); + deleted.ValueEncrypted.ShouldBeNull("the deleted secret's ciphertext must not outlive the delete — check VariableService.DeleteAsync if this fails"); + deleted.ValueType.ShouldBe(VariableValueType.Secret, "the row itself stays as the audit trail"); + + await ShouldSucceedAsync(await _tenants.SendAsync(HttpMethod.Put, "/api/team-variables/DEPLOY_TOKEN", owner, team, body), "set again under the freed name"); + (await ShouldSucceedAsync(await _tenants.SendAsync(HttpMethod.Get, "/api/team-variables", owner, team), "list")).ShouldContain("DEPLOY_TOKEN"); + } + + private static async Task ShouldSucceedAsync(HttpResponseMessage response, string step) + { + var text = await response.Content.ReadAsStringAsync(); + + response.IsSuccessStatusCode.ShouldBeTrue($"{step}: got {(int)response.StatusCode}: {text}"); + + return text; + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Infrastructure/RecordingProviderStub.cs b/backend/tests/CodeSpace.E2ETests/Infrastructure/RecordingProviderStub.cs new file mode 100644 index 000000000..b480a3a43 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Infrastructure/RecordingProviderStub.cs @@ -0,0 +1,144 @@ +using System.Collections.Concurrent; +using System.Net; +using System.Net.Sockets; +using System.Text; +using System.Text.Json; + +namespace CodeSpace.E2ETests.Infrastructure; + +/// +/// A git provider on a loopback port, for tests that need to see which token reached the provider and through which call. +/// It answers GitLab's token-revocation endpoint (POST /oauth/revoke) with 200, GitHub's OAuth App revocation +/// endpoints (DELETE /api/v3/applications/{client_id}/grant and /token) with 204 as GitHub does, GitHub's +/// token endpoint as sets it, and every other request with an empty JSON list — enough for the +/// real GitLab provider to list repositories — and records each request whole: method, path and query, headers and body. +/// The production clients speak real HTTP to it. Requests are served concurrently. +/// +internal sealed class RecordingProviderStub : IAsyncDisposable +{ + private readonly HttpListener _listener = new(); + private readonly CancellationTokenSource _stopping = new(); + private readonly ConcurrentQueue _requests = new(); + private readonly Task _accept; + private RefreshAnswer? _refresh; + + private RecordingProviderStub() + { + // Reserve an ephemeral loopback port; retry binding only if another process won the close/bind race. + for (var attempt = 0; ; attempt++) + { + var port = FreeLoopbackPort(); + BaseUrl = $"http://127.0.0.1:{port}"; + _listener.Prefixes.Clear(); + _listener.Prefixes.Add($"{BaseUrl}/"); + try { _listener.Start(); break; } + catch (HttpListenerException) when (attempt < 4) { } + } + + _accept = Task.Run(AcceptAsync); + } + + public string BaseUrl { get; } + + public IReadOnlyCollection Requests => _requests; + + /// Every request that carried anywhere — its query, a header, or its body. + public IReadOnlyList Carrying(string secret) => _requests.Where(r => r.Whole.Contains(secret, StringComparison.Ordinal)).ToList(); + + public static RecordingProviderStub Start() => new(); + + /// + /// Answer GitHub's token endpoint (POST /login/oauth/access_token) with the pair , + /// once has run — a write that lands while GitHub + /// holds a refresh. Requests are served concurrently, so that write may reach this stub itself. + /// + public void AnswerRefresh(string accessToken, string refreshToken, Func whileRefreshing) => _refresh = new RefreshAnswer(accessToken, refreshToken, whileRefreshing); + + /// A loopback port with nothing listening on it — a provider that is down. Allocated by the OS, then released. + public static int FreeLoopbackPort() + { + using var probe = new TcpListener(IPAddress.Loopback, 0); + probe.Start(); + var port = ((IPEndPoint)probe.LocalEndpoint).Port; + probe.Stop(); + return port; + } + + private async Task AcceptAsync() + { + try + { + while (!_stopping.IsCancellationRequested) + { + var context = await _listener.GetContextAsync().WaitAsync(_stopping.Token); + _ = ServeAsync(context); + } + } + catch (Exception) when (_stopping.IsCancellationRequested) { } + } + + private async Task ServeAsync(HttpListenerContext context) + { + try + { + using var reader = new StreamReader(context.Request.InputStream, Encoding.UTF8); + var body = await reader.ReadToEndAsync(_stopping.Token); + var headers = string.Join("\n", context.Request.Headers.AllKeys.Select(key => $"{key}: {context.Request.Headers[key]}")); + var pathAndQuery = context.Request.Url!.PathAndQuery; + + _requests.Enqueue(new RecordedRequest(context.Request.HttpMethod, pathAndQuery, headers, body)); + + var (status, answer) = await AnswerAsync(pathAndQuery); + var reply = Encoding.UTF8.GetBytes(answer); + context.Response.StatusCode = status; + context.Response.ContentType = "application/json"; + context.Response.ContentLength64 = reply.Length; + await context.Response.OutputStream.WriteAsync(reply, _stopping.Token); + } + catch (Exception) when (_stopping.IsCancellationRequested) { } + finally + { + context.Response.Close(); + } + } + + private async Task<(int Status, string Body)> AnswerAsync(string pathAndQuery) + { + if (pathAndQuery.StartsWith("/api/v3/applications/", StringComparison.Ordinal)) return (204, string.Empty); + if (pathAndQuery.StartsWith("/oauth/revoke", StringComparison.Ordinal)) return (200, "{}"); + if (_refresh != null && pathAndQuery.StartsWith("/login/oauth/access_token", StringComparison.Ordinal)) return (200, await RefreshAsync(_refresh)); + + return (200, "[]"); + } + + private static async Task RefreshAsync(RefreshAnswer refresh) + { + await refresh.WhileRefreshing(); + + return JsonSerializer.Serialize(new { access_token = refresh.AccessToken, refresh_token = refresh.RefreshToken, expires_in = 28800, token_type = "bearer", scope = "" }); + } + + public async ValueTask DisposeAsync() + { + _stopping.Cancel(); + _listener.Stop(); + _listener.Close(); + + try { await _accept; } + catch (Exception) { /* the accept loop ends with the listener */ } + + _stopping.Dispose(); + } + + private sealed record RefreshAnswer(string AccessToken, string RefreshToken, Func WhileRefreshing); + + internal sealed record RecordedRequest(string Method, string PathAndQuery, string Headers, string Body) + { + public string Whole => $"{Method} {PathAndQuery}\n{Headers}\n\n{Body}"; + + /// The form fields of a url-encoded body, decoded. + public IReadOnlyDictionary Form => Body.Split('&', StringSplitOptions.RemoveEmptyEntries) + .Select(pair => pair.Split('=', 2)) + .ToDictionary(pair => WebUtility.UrlDecode(pair[0]), pair => pair.Length > 1 ? WebUtility.UrlDecode(pair[1]) : string.Empty, StringComparer.Ordinal); + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Repositories/ProviderInstanceDeleteRevokeE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Repositories/ProviderInstanceDeleteRevokeE2ETests.cs new file mode 100644 index 000000000..1869a0481 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Repositories/ProviderInstanceDeleteRevokeE2ETests.cs @@ -0,0 +1,274 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; + +namespace CodeSpace.E2ETests.Repositories; + +/// +/// Deleting a git provider connection ends every token connected through it, here and at the provider. Before, the +/// delete marked them Revoked and left the tokens and the connection's OAuth client secret decryptable in the database, +/// and never told the provider, so an OAuth refresh token stayed good at GitLab while the UI said "Revoked". +/// +/// Driven through the real HTTP surface (POST and DELETE /api/provider-instances, POST /api/credentials/pat, +/// GET /api/credentials), the real mediator pipeline and its transaction, the real GitLabOAuthClient speaking +/// RFC 7009 over HTTP, and Postgres, against a GitLab on a loopback port that records every request. One seam: the OAuth +/// credential is written straight to the database, sealed by the production encryptor, as an OAuth connect would leave it +/// — the connect flow is not what is under test. +/// +/// Fidelity: HIGH. Only GitLab (and GitHub, for the shared-OAuth-App case) is a stub, and it is a real socket the +/// production client speaks HTTP to. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class ProviderInstanceDeleteRevokeE2ETests : IClassFixture +{ + private readonly TaskLaunchApiFactory _factory; + private readonly TenantHarness _tenants; + + public ProviderInstanceDeleteRevokeE2ETests(TaskLaunchApiFactory factory) + { + _factory = factory; + _tenants = new TenantHarness(factory); + } + + [Fact] + public async Task Deleting_a_connection_revokes_its_oauth_grants_at_the_provider_and_leaves_no_secret_decryptable() + { + await using var gitLab = RecordingProviderStub.Start(); + var canary = Guid.NewGuid().ToString("N"); + var world = await SeedConnectionAsync(gitLab.BaseUrl, canary); + + var delete = await _tenants.SendAsync(HttpMethod.Delete, $"/api/provider-instances/{world.InstanceId}", world.Admin, world.Team); + + delete.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(delete)); + (await delete.Content.ReadFromJsonAsync()).GetProperty("revokedCredentialCount").GetInt32().ShouldBe(2); + + var revokes = gitLab.Requests.Where(r => r.Method == "POST" && r.PathAndQuery == "/oauth/revoke").ToList(); + revokes.Select(r => (r.Form["token"], r.Form["token_type_hint"])).ShouldBe(new[] { ($"rt-{canary}", "refresh_token"), ($"at-{canary}", "access_token") }, "GitLab must be asked to revoke the refresh token, then the access token — check the post-commit revoke in ProviderInstanceService and the stub's request log if this fails"); + revokes.ShouldAllBe(r => r.Form["client_id"] == world.ClientId && r.Form["client_secret"] == $"client-secret-{canary}"); + gitLab.Carrying($"glpat-{canary}").ShouldBeEmpty("a personal access token has no revocation endpoint and is never sent anywhere by a delete"); + + var stored = await StoredAsync(world); + stored.Payloads.ShouldAllBe(p => p == string.Empty, "a deleted connection's tokens must not stay decryptable in the database"); + stored.ClientSecret.ShouldBeNull("nor its OAuth client secret"); + + var listed = await _tenants.SendAsync(HttpMethod.Get, "/api/credentials", world.Owner, world.Team); + (await listed.Content.ReadAsStringAsync()).ShouldNotContain(world.OAuthCredentialId.ToString(), Case.Insensitive, "a retired credential is out of the team's list"); + } + + [Fact] + public async Task Deleting_a_connection_whose_provider_is_down_still_retires_everything_here() + { + var down = $"http://127.0.0.1:{RecordingProviderStub.FreeLoopbackPort()}"; + var canary = Guid.NewGuid().ToString("N"); + var world = await SeedConnectionAsync(down, canary); + + var delete = await _tenants.SendAsync(HttpMethod.Delete, $"/api/provider-instances/{world.InstanceId}", world.Admin, world.Team); + + delete.StatusCode.ShouldBe(HttpStatusCode.OK, $"the provider-side revoke is best effort; a provider that does not answer must not fail the delete — {await TenantHarness.DescribeAsync(delete)}"); + + var stored = await StoredAsync(world); + stored.Payloads.ShouldAllBe(p => p == string.Empty); + stored.ClientSecret.ShouldBeNull(); + } + + /// + /// Two teams connect GitHub through the same OAuth App and the same person authorizes it in both. GitHub's grant delete + /// ends every token the person holds under the app, the other team's included, so deleting one team's connection asks + /// GitHub to end only the access token that team held — and sends no refresh token, which GitHub cannot end alone. A + /// Disconnect still ends the whole grant, as it always has. + /// + [Fact] + public async Task Deleting_a_GitHub_connection_ends_only_its_own_token_while_a_disconnect_still_ends_the_grant() + { + await using var gitHub = RecordingProviderStub.Start(); + var canary = Guid.NewGuid().ToString("N"); + var clientId = $"shared-app-{canary}"; + var deleted = await SeedGitHubConnectionAsync(gitHub.BaseUrl, clientId, $"A-{canary}"); + var kept = await SeedGitHubConnectionAsync(gitHub.BaseUrl, clientId, $"B-{canary}"); + + var delete = await _tenants.SendAsync(HttpMethod.Delete, $"/api/provider-instances/{deleted.InstanceId}", deleted.Admin, deleted.Team); + + delete.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(delete)); + var revokes = gitHub.Requests.Where(r => r.Method == "DELETE" && r.PathAndQuery.StartsWith("/api/v3/applications/", StringComparison.Ordinal)).ToList(); + revokes.Select(r => (r.PathAndQuery, r.Body)).ShouldBe(new[] { ($"/api/v3/applications/{clientId}/token", $"{{\"access_token\":\"at-A-{canary}\"}}") }, $"a delete ends the one access token its team held, never the person's grant, which team B's token lives under too — check the post-commit revoke in ProviderInstanceService; requests seen: {string.Join(", ", gitHub.Requests.Select(r => $"{r.Method} {r.PathAndQuery}"))}"); + gitHub.Carrying($"rt-A-{canary}").ShouldBeEmpty("GitHub has no endpoint that ends a refresh token alone, so none is sent"); + gitHub.Carrying($"at-B-{canary}").ShouldBeEmpty("team B's token is never presented by team A's delete"); + + var disconnect = await _tenants.SendAsync(HttpMethod.Post, $"/api/credentials/{kept.OAuthCredentialId}/revoke", kept.Owner, kept.Team); + + disconnect.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(disconnect)); + gitHub.Carrying($"at-B-{canary}").ShouldHaveSingleItem().PathAndQuery.ShouldBe($"/api/v3/applications/{clientId}/grant", "a Disconnect ends the person's whole grant to the app, unchanged"); + } + + /// + /// Team A's token is inside the refresh window and a listing refreshes it; team A's connection is deleted while GitHub + /// holds that refresh. The pair GitHub then issues is stored nowhere, so the refresher ends it at GitHub — by token, as + /// the delete itself ends team A's token: a grant delete would end the person's token in team B too. + /// + [Fact] + public async Task A_GitHub_connection_deleted_while_its_token_refreshes_ends_only_the_pair_GitHub_issued() + { + await using var gitHub = RecordingProviderStub.Start(); + var canary = Guid.NewGuid().ToString("N"); + var clientId = $"shared-app-{canary}"; + var deleted = await SeedGitHubConnectionAsync(gitHub.BaseUrl, clientId, $"A-{canary}", DateTimeOffset.UtcNow.AddSeconds(30)); + await SeedGitHubConnectionAsync(gitHub.BaseUrl, clientId, $"B-{canary}"); + HttpStatusCode? deleteStatus = null; + gitHub.AnswerRefresh($"at-A2-{canary}", $"rt-A2-{canary}", whileRefreshing: async () => deleteStatus = (await _tenants.SendAsync(HttpMethod.Delete, $"/api/provider-instances/{deleted.InstanceId}", deleted.Admin, deleted.Team)).StatusCode); + + var listing = await _tenants.SendAsync(HttpMethod.Get, $"/api/credentials/{deleted.OAuthCredentialId}/accessible-repositories", deleted.Owner, deleted.Team); + + deleteStatus.ShouldBe(HttpStatusCode.OK, $"fixture check: the delete landed while GitHub held the refresh; requests seen: {string.Join(", ", gitHub.Requests.Select(r => $"{r.Method} {r.PathAndQuery}"))}"); + listing.IsSuccessStatusCode.ShouldBeFalse("a listing whose connection was deleted under it gets no token"); + var revokes = gitHub.Requests.Where(r => r.Method == "DELETE" && r.PathAndQuery.StartsWith("/api/v3/applications/", StringComparison.Ordinal)).ToList(); + revokes.Select(r => (r.PathAndQuery, r.Body)).ShouldBe(new[] { ($"/api/v3/applications/{clientId}/token", $"{{\"access_token\":\"at-A-{canary}\"}}"), ($"/api/v3/applications/{clientId}/token", $"{{\"access_token\":\"at-A2-{canary}\"}}") }, $"the delete ends the token team A held, then the refresher ends the pair GitHub issued meanwhile — each by token, never the person's grant, which team B's token lives under too; check OAuthTokenRefresher.RevokeIssuedTokensAsync. Requests seen: {string.Join(", ", gitHub.Requests.Select(r => $"{r.Method} {r.PathAndQuery}"))}"); + gitHub.Carrying($"at-B-{canary}").ShouldBeEmpty("team B's token is never presented by team A's delete or its refresh"); + } + + /// + /// A connection-wide connection whose group hook was registered at GitLab with the owner's personal access token. The + /// delete blanks that token, so the hook must come down first, while the token still opens: afterwards nothing could + /// remove it, and it would go on delivering the group's events with its HMAC secret still decryptable here. A hook + /// row retired earlier stays as the record it is, holding no secret. + /// + [Fact] + public async Task Deleting_a_connection_wide_connection_takes_its_group_hook_down_at_the_provider_before_its_token_goes() + { + await using var gitLab = RecordingProviderStub.Start(); + var canary = Guid.NewGuid().ToString("N"); + var world = await SeedConnectionAsync(gitLab.BaseUrl, canary); + var retiredEarlier = await SeedGroupHooksAsync(world, $"acme-{canary}", $"hook-{canary}", canary); + + var delete = await _tenants.SendAsync(HttpMethod.Delete, $"/api/provider-instances/{world.InstanceId}", world.Admin, world.Team); + + delete.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(delete)); + + var hookDeletes = gitLab.Requests.Where(r => r.Method == "DELETE" && r.PathAndQuery == $"/api/v4/groups/acme-{canary}/hooks/hook-{canary}").ToList(); + hookDeletes.Count.ShouldBe(1, $"GitLab must be asked to delete the group hook — check the hook retirement in ProviderInstanceService.DeleteAsync; requests seen: {string.Join(", ", gitLab.Requests.Select(r => $"{r.Method} {r.PathAndQuery}"))}"); + hookDeletes[0].Headers.ShouldContain($"glpat-{canary}", Case.Sensitive, "the hook is deleted with the token that registered it, before the delete blanks that token"); + + var hooks = await _tenants.DbAsync(db => db.ConnectionWebhook.AsNoTracking().Where(w => w.ProviderInstanceId == world.InstanceId).ToListAsync()); + hooks.Select(h => h.Id).ShouldBe(new[] { retiredEarlier }, "the Registered row described the hook that is now gone"); + hooks.ShouldAllBe(h => h.RegistrationStatus == RepositoryWebhookRegistrationStatus.Cancelled && h.SecretEnc == string.Empty, "a deleted connection keeps no hook secret decryptable"); + } + + private sealed record Connection(Guid Owner, Guid Admin, Guid Team, Guid InstanceId, string ClientId, Guid OAuthCredentialId, Guid PatCredentialId); + + /// + /// Puts the connection in connection-wide scope with a Registered group hook on , registered + /// by the owner's personal access token, beside a Cancelled one an earlier scope switch left. Written directly, as the + /// registrar leaves them — the registration is not what is under test. Returns the Cancelled row's id. + /// + private async Task SeedGroupHooksAsync(Connection world, string ownerPath, string externalId, string canary) + { + using var scope = _factory.Services.CreateScope(); + var encryptor = scope.ServiceProvider.GetRequiredService(); + var registered = Guid.NewGuid(); + var cancelled = Guid.NewGuid(); + + await _tenants.DbAsync(async db => + { + var instance = await db.ProviderInstance.SingleAsync(p => p.Id == world.InstanceId); + instance.WebhookScope = ProviderWebhookScope.Connection; + + db.ConnectionWebhook.Add(GroupHook(registered, world, ownerPath, externalId, RepositoryWebhookRegistrationStatus.Registered, encryptor.Encrypt($"hook-secret-{canary}"))); + db.ConnectionWebhook.Add(GroupHook(cancelled, world, $"{ownerPath}-old", externalId: null, RepositoryWebhookRegistrationStatus.Cancelled, encryptor.Encrypt($"old-hook-secret-{canary}"))); + + return await db.SaveChangesAsync(); + }); + + return cancelled; + } + + private static ConnectionWebhook GroupHook(Guid id, Connection world, string ownerPath, string? externalId, RepositoryWebhookRegistrationStatus status, string secretEnc) => new() + { + Id = id, ProviderInstanceId = world.InstanceId, CredentialId = world.PatCredentialId, OwnerPath = ownerPath, ExternalId = externalId, + CallbackUrl = $"https://codespace.test/api/webhooks/connection/{id}", SecretEnc = secretEnc, SubscribedEvents = new List { "push" }, RegistrationStatus = status, + }; + + /// + /// The Owner adds a GitLab connection with an OAuth app and a personal access token through the API; the Owner's OAuth + /// credential is sealed by the production encryptor and written directly, as a completed connect leaves it. + /// + private async Task SeedConnectionAsync(string gitLabUrl, string canary) + { + var owner = await _tenants.AccountAsync(); + var admin = await _tenants.AccountAsync(); + var team = await _tenants.TeamAsync((owner, TeamRole.Owner), (admin, TeamRole.Admin)); + var clientId = $"client-{canary}"; + var instanceId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/provider-instances", owner, team, new { provider = "GitLab", displayName = "gitlab", baseUrl = gitLabUrl, oauthClientId = clientId, oauthClientSecret = $"client-secret-{canary}" })); + var patId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/pat", owner, team, new { providerInstanceId = instanceId, displayName = "owner pat", token = $"glpat-{canary}" })); + var oauthId = Guid.NewGuid(); + + using (var scope = _factory.Services.CreateScope()) + { + var payload = scope.ServiceProvider.GetRequiredService().Serialize(new OAuthPayload { AccessToken = $"at-{canary}", RefreshToken = $"rt-{canary}" }); + var sealedPayload = scope.ServiceProvider.GetRequiredService().Encrypt(payload); + + await _tenants.DbAsync(async db => + { + db.Credential.Add(new Credential { Id = oauthId, TeamId = team, ProviderInstanceId = instanceId, OwnerUserId = owner, Ownership = CredentialOwnership.Personal, AuthType = AuthType.OAuth, DisplayName = "owner oauth", EncryptedPayload = sealedPayload, Status = CredentialStatus.Active }); + return await db.SaveChangesAsync(); + }); + } + + return new Connection(owner, admin, team, instanceId, clientId, oauthId, patId); + } + + private sealed record GitHubConnection(Guid Owner, Guid Admin, Guid Team, Guid InstanceId, Guid OAuthCredentialId); + + /// + /// A team whose Owner adds a GitHub connection on through the API with the OAuth App + /// , and the Owner's OAuth credential (at-{tag}, rt-{tag}, expiring at + /// when given) sealed by the production encryptor and written directly, as a completed + /// connect leaves it. + /// + private async Task SeedGitHubConnectionAsync(string gitHubUrl, string clientId, string tag, DateTimeOffset? expiresAt = null) + { + var owner = await _tenants.AccountAsync(); + var admin = await _tenants.AccountAsync(); + var team = await _tenants.TeamAsync((owner, TeamRole.Owner), (admin, TeamRole.Admin)); + var instanceId = await CreatedIdAsync(await _tenants.SendAsync(HttpMethod.Post, "/api/provider-instances", owner, team, new { provider = "GitHub", displayName = "github", baseUrl = gitHubUrl, oauthClientId = clientId, oauthClientSecret = "shared-app-secret" })); + var oauthId = Guid.NewGuid(); + + using (var scope = _factory.Services.CreateScope()) + { + var payload = scope.ServiceProvider.GetRequiredService().Serialize(new OAuthPayload { AccessToken = $"at-{tag}", RefreshToken = $"rt-{tag}", ExpiresAt = expiresAt }); + var sealedPayload = scope.ServiceProvider.GetRequiredService().Encrypt(payload); + + await _tenants.DbAsync(async db => + { + db.Credential.Add(new Credential { Id = oauthId, TeamId = team, ProviderInstanceId = instanceId, OwnerUserId = owner, Ownership = CredentialOwnership.Personal, AuthType = AuthType.OAuth, DisplayName = "owner oauth", EncryptedPayload = sealedPayload, ExpiresDate = expiresAt, Status = CredentialStatus.Active }); + return await db.SaveChangesAsync(); + }); + } + + return new GitHubConnection(owner, admin, team, instanceId, oauthId); + } + + private async Task<(IReadOnlyList Payloads, string? ClientSecret)> StoredAsync(Connection world) + { + var payloads = await _tenants.DbAsync(db => db.Credential.AsNoTracking().Where(c => c.Id == world.OAuthCredentialId || c.Id == world.PatCredentialId).Select(c => c.EncryptedPayload).ToListAsync()); + var clientSecret = await _tenants.DbAsync(db => db.ProviderInstance.AsNoTracking().Where(p => p.Id == world.InstanceId).Select(p => p.OauthClientSecretEnc).SingleAsync()); + + payloads.Count.ShouldBe(2, "fixture check: both credentials were seeded"); + + return (payloads, clientSecret); + } + + private static async Task CreatedIdAsync(HttpResponseMessage response) + { + response.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(response)); + + return (await response.Content.ReadFromJsonAsync()).GetProperty("id").GetGuid(); + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Storage/StorageCredentialsEndpointE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Storage/StorageCredentialsEndpointE2ETests.cs index 53da4acea..5f421dca6 100644 --- a/backend/tests/CodeSpace.E2ETests/Storage/StorageCredentialsEndpointE2ETests.cs +++ b/backend/tests/CodeSpace.E2ETests/Storage/StorageCredentialsEndpointE2ETests.cs @@ -85,21 +85,12 @@ public async Task Admin_creates_lists_gets_rotates_and_revokes_without_exposing_ rotated.GetProperty("xmin").GetUInt32().ShouldNotBe(created.GetProperty("xmin").GetUInt32()); AssertSafeWire(rotatedWire, rotatedSecret); - var revoke = await SendAsync(world.UserId, world.TeamId, HttpMethod.Post, $"/api/storage/credentials/{credentialId}/revoke", new - { - expectedXmin = rotated.GetProperty("xmin").GetUInt32(), expectedCurrentRevision = 2, - }); - revoke.StatusCode.ShouldBe(HttpStatusCode.OK, await DescribeAsync(revoke)); - var revoked = Json(await revoke.Content.ReadAsStringAsync()); - revoked.GetProperty("state").GetString().ShouldBe("Revoked"); - revoked.GetProperty("revokedDate").ValueKind.ShouldBe(JsonValueKind.String); - using var scope = _application.Services.CreateScope(); var db = scope.ServiceProvider.GetRequiredService(); var revisions = await db.StorageCredentialRevision.AsNoTracking().Where(value => value.StorageCredentialId == credentialId).OrderBy(value => value.Revision).ToListAsync(); revisions.Count.ShouldBe(2); var encryptor = scope.ServiceProvider.GetRequiredService(); - encryptor.Decrypt(revisions[0].EncryptedPayload).ShouldBe("""{"accessKey":"AK-ABCD","mode":"write"}"""); + encryptor.Decrypt(revisions[0].EncryptedPayload).ShouldBe("""{"accessKey":"AK-ABCD","mode":"write"}""", "while the credential is Active the superseded revision stays readable: a profile may pin it"); encryptor.Decrypt(revisions[1].EncryptedPayload).ShouldBe("""{"accessKey":"AK-WXYZ","mode":"read"}"""); revisions[0].EncryptedPayload.ShouldNotContain(firstSecret); revisions[1].EncryptedPayload.ShouldNotContain(rotatedSecret); @@ -107,6 +98,19 @@ public async Task Admin_creates_lists_gets_rotates_and_revokes_without_exposing_ revisions[1].EnvelopeFingerprint.ShouldBe(Fingerprint(revisions[1].EncryptedPayload)); createdWire.ShouldNotContain(revisions[0].EncryptedPayload); rotatedWire.ShouldNotContain(revisions[1].EncryptedPayload); + + var revoke = await SendAsync(world.UserId, world.TeamId, HttpMethod.Post, $"/api/storage/credentials/{credentialId}/revoke", new + { + expectedXmin = rotated.GetProperty("xmin").GetUInt32(), expectedCurrentRevision = 2, + }); + revoke.StatusCode.ShouldBe(HttpStatusCode.OK, await DescribeAsync(revoke)); + var revoked = Json(await revoke.Content.ReadAsStringAsync()); + revoked.GetProperty("state").GetString().ShouldBe("Revoked"); + revoked.GetProperty("revokedDate").ValueKind.ShouldBe(JsonValueKind.String); + + var retired = await db.StorageCredentialRevision.AsNoTracking().Where(value => value.StorageCredentialId == credentialId).OrderBy(value => value.Revision).ToListAsync(); + retired.ShouldAllBe(value => value.EncryptedPayload == string.Empty, "a revoked credential resolves for nobody, so none of its revisions may stay decryptable"); + retired.Select(value => value.EnvelopeFingerprint).ShouldBe(revisions.Select(value => value.EnvelopeFingerprint), "the fingerprints stay for the audit trail"); } [Fact] diff --git a/backend/tests/CodeSpace.E2ETests/Storage/StorageDefaultSecretShredE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Storage/StorageDefaultSecretShredE2ETests.cs new file mode 100644 index 000000000..0b05939ed --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Storage/StorageDefaultSecretShredE2ETests.cs @@ -0,0 +1,134 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +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.Messages.Constants; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; + +namespace CodeSpace.E2ETests.Storage; + +/// +/// Rotating a deployment storage default's secret, and clearing it, through the real admin HTTP surface. Each rotation +/// appends a new envelope; the one it replaced used to stay decryptable forever, so a cloud key an operator rotated out +/// because it leaked stayed recoverable from every database copy. Now the envelope a template stops naming is blanked +/// in the same transaction, and only the one it names still opens. +/// +/// Fidelity: HIGH. The real ApiUser capability resolution, MediatR pipeline and its transaction, the +/// production encryptor, and Postgres with 0249's guard. Its own database (one fixture per class), because a template is +/// one row per data class for the whole deployment and can never be deleted. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class StorageDefaultSecretShredE2ETests : IClassFixture +{ + private readonly TaskLaunchApiFactory _factory; + + public StorageDefaultSecretShredE2ETests(TaskLaunchApiFactory factory) { _factory = factory; } + + [Fact] + public async Task Rotating_then_clearing_a_templates_secret_leaves_only_the_envelope_it_names_decryptable() + { + var operatorId = await SeedCapabilityHolderAsync(); + var canary = Guid.NewGuid().ToString("N"); + + var created = await SendAsync(operatorId, HttpMethod.Post, "/api/admin/storage-defaults", Template(new { accessKeyId = "AK", accessKeySecret = $"first-{canary}" })); + var defaultId = created.GetProperty("id").GetGuid(); + var first = await NamedEnvelopeAsync(defaultId); + + var rotated = await SendAsync(operatorId, HttpMethod.Put, $"/api/admin/storage-defaults/{defaultId}", Edit(created, secret: new { accessKeyId = "AK", accessKeySecret = $"second-{canary}" })); + var second = await NamedEnvelopeAsync(defaultId); + + second.ShouldNotBe(first, "fixture check: a supplied secret appends a new envelope"); + (await EnvelopeAsync(first!.Value)).ShouldBe(string.Empty, "the rotated-out key has no reader left and must not stay decryptable — check StorageDefaultService.UpdateAsync and 0249's storage_default_credential_guard"); + (await DecryptAsync(second!.Value)).ShouldContain($"second-{canary}", Case.Sensitive, "the envelope the template names is the one the materializer decrypts"); + + var cleared = await SendAsync(operatorId, HttpMethod.Put, $"/api/admin/storage-defaults/{defaultId}", Edit(rotated, clearCredential: true)); + + cleared.GetProperty("hasCredential").GetBoolean().ShouldBeFalse(); + (await NamedEnvelopeAsync(defaultId)).ShouldBeNull(); + (await EnvelopeAsync(second.Value)).ShouldBe(string.Empty, "a cleared key has no reader left either"); + } + + private static object Template(object secret) => new + { + dataClassTypeKey = "agent-run-log/v1", + providerTypeKey = "aliyun-oss/v1", + nonSecretConfig = new { endpoint = "oss-cn-hangzhou.aliyuncs.com", bucket = "codespace-artifacts" }, + namespaceRoot = "codespace-defaults/", + adoptionPolicy = "Automatic", + isEnabled = true, + secret, + }; + + private static object Edit(JsonElement current, object? secret = null, bool clearCredential = false) => new + { + expectedXmin = current.GetProperty("xmin").GetUInt32(), + expectedRevision = current.GetProperty("revision").GetInt32(), + providerTypeKey = "aliyun-oss/v1", + nonSecretConfig = new { endpoint = "oss-cn-hangzhou.aliyuncs.com", bucket = "codespace-artifacts" }, + namespaceRoot = "codespace-defaults/", + adoptionPolicy = "Automatic", + secret, + clearCredential, + }; + + private async Task SendAsync(Guid userId, HttpMethod method, string path, object body) + { + var request = new HttpRequestMessage(method, path) { Content = JsonContent.Create(body) }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", TestToken.Mint(userId, TestToken.SeedStamp)); + + var response = await _factory.CreateClient().SendAsync(request); + var text = await response.Content.ReadAsStringAsync(); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, $"{method} {path} got {(int)response.StatusCode}: {text}"); + + return JsonDocument.Parse(text).RootElement.Clone(); + } + + private async Task NamedEnvelopeAsync(Guid defaultId) => + await DbAsync(db => db.StorageDefault.AsNoTracking().Where(row => row.Id == defaultId).Select(row => row.CredentialId).SingleAsync()); + + private async Task EnvelopeAsync(Guid envelopeId) => + await DbAsync(db => db.StorageDefaultCredential.AsNoTracking().Where(row => row.Id == envelopeId).Select(row => row.EncryptedPayload).SingleAsync()); + + private async Task DecryptAsync(Guid envelopeId) + { + var envelope = await EnvelopeAsync(envelopeId); + + using var scope = _factory.Services.CreateScope(); + + return scope.ServiceProvider.GetRequiredService().Decrypt(envelope); + } + + private async Task DbAsync(Func> read) + { + using var scope = _factory.Services.CreateScope(); + + return await read(scope.ServiceProvider.GetRequiredService()); + } + + /// A deployment admin: an account holding storage.defaults.manage through a real role row, as ApiUser resolves it. + private async Task SeedCapabilityHolderAsync() + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var suffix = Guid.NewGuid().ToString("N")[..8]; + var permissionId = await db.Permission.AsNoTracking().Where(value => value.Name == Permissions.StorageDefaultsManage).Select(value => value.Id).SingleAsync(); + var user = new User { Id = Guid.NewGuid(), SecurityStamp = TestToken.SeedStamp, Email = $"shred-operator-{suffix}@test.local", Name = "Deployment Admin", CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }; + var role = new Role { Id = Guid.NewGuid(), Name = $"storage-defaults-shred-{suffix}", IsSystem = false, Status = true }; + + db.User.Add(user); + db.Role.Add(role); + db.RolePermission.Add(new RolePermission { Id = Guid.NewGuid(), RoleId = role.Id, PermissionId = permissionId }); + db.RoleUser.Add(new RoleUser { Id = Guid.NewGuid(), RoleId = role.Id, UserId = user.Id }); + await db.SaveChangesAsync(); + + return user.Id; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamPermissionEnforcementTests.cs b/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamPermissionEnforcementTests.cs index 2722425cf..7d7acfa47 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamPermissionEnforcementTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamPermissionEnforcementTests.cs @@ -14,6 +14,7 @@ using CodeSpace.Messages.Credentials; using CodeSpace.Messages.Dtos.Workflows; using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Queries.Credentials; using CodeSpace.Messages.Queries.Workflows; using MediatR; using Microsoft.EntityFrameworkCore; @@ -213,6 +214,41 @@ public async Task Below_Admin_only_the_callers_own_personal_credential_can_be_re (await LoadCredentialStatusAsync(credentialId).ConfigureAwait(false)).ShouldBe(CredentialStatus.Revoked); } + /// + /// Listing what a credential reaches presents it to the provider, so on membership alone a Viewer could spend the + /// Owner's personal token to enumerate everything the Owner's account can see there — private repositories outside + /// the team included. Below Admin only the caller's own Personal credential lists; a teammate's and the team's own + /// take repos.manage, the bar of the bind the listing feeds. The allow path reaches the provider lookup, which the Git + /// kind cannot answer — anything but a denial is past the gate. + /// + [Theory] + [MemberData(nameof(RevokesByRoleAndHolder))] + public async Task Below_Admin_only_the_callers_own_personal_credential_lists_what_it_reaches(TeamRole role, CredentialHolder holder, bool denied) + { + var team = await SeedTeamAsync().ConfigureAwait(false); + var caller = team.UserFor(role); + var teammate = role == TeamRole.Owner ? team.Admin : team.Owner; + var credentialId = await SeedCredentialAsync(team.TeamId, holder switch { CredentialHolder.Caller => caller, CredentialHolder.Teammate => teammate, _ => null }).ConfigureAwait(false); + + using var scope = _fixture.BeginScopeAs(caller, team.TeamId); + var thrown = await Record.ExceptionAsync(() => scope.Resolve().Send(new ListAccessibleRepositoriesQuery { CredentialId = credentialId })).ConfigureAwait(false); + + AssertDenial(thrown, denied, role, TeamPermissions.ReposManage); + } + + [Fact] + public async Task An_instance_Admin_outside_the_team_lists_through_a_members_personal_credential() + { + // The instance Admin role has an Owner's standing in every team (TeamStanding), membership row or not. + var team = await SeedTeamAsync().ConfigureAwait(false); + var credentialId = await SeedCredentialAsync(team.TeamId, team.Member).ConfigureAwait(false); + + using var scope = _fixture.BeginScopeAs(Guid.NewGuid(), team.TeamId, Roles.Admin); + var thrown = await Record.ExceptionAsync(() => scope.Resolve().Send(new ListAccessibleRepositoriesQuery { CredentialId = credentialId })).ConfigureAwait(false); + + thrown.ShouldNotBeOfType(); + } + [Fact] public async Task The_global_Admin_role_still_bypasses() { diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/RetiredSecretShredSweepFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/RetiredSecretShredSweepFlowTests.cs new file mode 100644 index 000000000..2090df06b --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/RetiredSecretShredSweepFlowTests.cs @@ -0,0 +1,237 @@ +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Workflows.Artifacts.Providers.Local; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Enums; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Credentials; + +/// +/// A rolling deploy runs the previous build beside this one after 0249 has run, and a rollback runs it alone. That build +/// retires secrets without blanking them: its provider-instance delete marks tokens Revoked and keeps them, its variable +/// delete keeps the ciphertext, its storage revoke keeps every revision, its storage-default rotation keeps the envelope it +/// replaced. 0249 ran once, before those rows existed. The recurring sweep must blank them on its next tick — and touch +/// nothing live. +/// +/// Mixed-version, simulated: the previous build's retirements are its own statements, replayed as raw SQL against +/// this build's schema (0249's relaxed guards included), on rows seeded through the real model. The sweep is sent through +/// the real mediator — the pipeline its recurring job uses — and every assertion reads rows this test owns in a fresh +/// scope, never the sweep's global count: this suite shares one database. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class RetiredSecretShredSweepFlowTests +{ + private readonly PostgresFixture _fixture; + + public RetiredSecretShredSweepFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Fact] + public async Task What_the_previous_build_retires_after_0249_is_shredded_by_the_next_sweep_and_nothing_live_is() + { + var world = await SeedAsync(); + + await RetireAsThePreviousBuildDoesAsync(world); + (await PayloadAsync(world.RetiredCredentialId)).ShouldNotBeEmpty("fixture check: the previous build's delete keeps the token"); + + await SweepAsync(); + + await ShouldHoldNoRetiredSecretAndEveryLiveOneAsync(world); + } + + [Fact] + public async Task A_second_sweep_finds_nothing_left_to_change() + { + var world = await SeedAsync(); + await RetireAsThePreviousBuildDoesAsync(world); + await SweepAsync(); + + await Should.NotThrowAsync(SweepAsync, "a blanked revision or envelope is no longer a candidate — the guards refuse a second blanking, so a sweep that re-selected one would fail every tick"); + + await ShouldHoldNoRetiredSecretAndEveryLiveOneAsync(world); + } + + private async Task SweepAsync() + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().Send(new ShredRetiredSecretsCommand()); + } + + private async Task ShouldHoldNoRetiredSecretAndEveryLiveOneAsync(World world) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + (await PayloadAsync(world.RetiredCredentialId)).ShouldBe(string.Empty, "the previous build's provider delete left the token Revoked and decryptable"); + (await PayloadAsync(world.LiveCredentialId)).ShouldBe("CfDJ8-CANARY-live-token", "a live connection's token is untouched"); + (await db.ProviderInstance.AsNoTracking().Where(p => p.Id == world.DeletedInstanceId).Select(p => p.OauthClientSecretEnc).SingleAsync()).ShouldBeNull("a deleted connection's client secret"); + (await db.ProviderInstance.AsNoTracking().Where(p => p.Id == world.LiveInstanceId).Select(p => p.OauthClientSecretEnc).SingleAsync()).ShouldBe("CfDJ8-CANARY-live-client-secret"); + + var deletedHook = await db.ConnectionWebhook.AsNoTracking().SingleAsync(w => w.ProviderInstanceId == world.DeletedInstanceId); + deletedHook.ShouldSatisfyAllConditions( + h => h.RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Cancelled, "a deleted connection's hook is out of service — ingestion refuses it before it reads a secret"), + h => h.SecretEnc.ShouldBe(string.Empty), + h => h.ExternalId.ShouldBe("hook-777", "the provider's hook id stays, as the record of what to remove at the provider by hand")); + (await db.ConnectionWebhook.AsNoTracking().SingleAsync(w => w.ProviderInstanceId == world.LiveInstanceId)).ShouldSatisfyAllConditions( + h => h.RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Registered), + h => h.SecretEnc.ShouldBe("CfDJ8-CANARY-live-hook-secret", "a live hook keeps the secret its deliveries are verified against")); + + (await db.Variable.AsNoTracking().SingleAsync(v => v.Id == world.DeletedVariableId)).ValueEncrypted.ShouldBeNull("the previous build's variable delete kept the ciphertext"); + (await db.Variable.AsNoTracking().SingleAsync(v => v.Id == world.LiveVariableId)).ValueEncrypted.ShouldBe(new byte[] { 7, 7, 7, 7 }); + + (await RevisionsAsync(db, world.RevokedStorageCredentialId)).ShouldBe(new[] { string.Empty, string.Empty }, ignoreOrder: false, customMessage: "the previous build's storage revoke kept every revision"); + (await RevisionsAsync(db, world.ActiveStorageCredentialId)).ShouldBe(new[] { "CfDJ8-CANARY-active-rev-1", "CfDJ8-CANARY-active-rev-2" }, ignoreOrder: false, customMessage: "an Active credential's superseded revision may be pinned by a profile and is kept"); + + (await EnvelopeAsync(db, world.ReplacedEnvelopeId)).ShouldBe(string.Empty, "the previous build's rotation kept the envelope it replaced"); + (await EnvelopeAsync(db, world.RotatedEnvelopeId)).ShouldBe("CfDJ8-CANARY-rotated-envelope", "the envelope the template now names is the one the materializer decrypts"); + } + + /// + /// The previous build's retirements, statement for statement: ProviderInstanceService.DeleteAsync's + /// cascade-revoke then soft delete, VariableService.DeleteAsync, StorageCredentialService.RevokeAsync, + /// and StorageDefaultService.UpdateAsync appending a new envelope and repointing the template. None of them + /// blanks anything. Each must still be accepted by this build's schema. + /// + private async Task RetireAsThePreviousBuildDoesAsync(World world) + { + await using var connection = new NpgsqlConnection(_fixture.ConnectionString); + await connection.OpenAsync(); + await using var command = new NpgsqlCommand( + """ + UPDATE credential SET status = 'Revoked', deleted_date = now(), last_error = 'Provider instance deleted' WHERE provider_instance_id = @deleted AND deleted_date IS NULL AND status = 'Active'; + UPDATE provider_instance SET deleted_date = now() WHERE id = @deleted; + UPDATE variable SET deleted_date = now(), last_modified_date = now(), last_modified_by = @actor WHERE id = @variable; + UPDATE storage_credential SET state = 'Revoked', revoked_date = now(), revoked_by = @actor WHERE id = @storage; + INSERT INTO storage_default_credential (id, provider_type_key, encrypted_payload, envelope_fingerprint, created_date, created_by) VALUES (@rotated, @provider, 'CfDJ8-CANARY-rotated-envelope', @fingerprint, now(), @actor); + UPDATE storage_default SET credential_id = @rotated, revision = revision + 1, last_modified_date = now(), last_modified_by = @actor WHERE id = @template; + """, connection); + + command.Parameters.AddWithValue("deleted", world.DeletedInstanceId); + command.Parameters.AddWithValue("actor", world.ActorId); + command.Parameters.AddWithValue("variable", world.DeletedVariableId); + command.Parameters.AddWithValue("storage", world.RevokedStorageCredentialId); + command.Parameters.AddWithValue("rotated", world.RotatedEnvelopeId); + command.Parameters.AddWithValue("provider", LocalRwxArtifactStorageDriverFactory.TypeKey); + command.Parameters.AddWithValue("fingerprint", $"sha256:{new string('d', 64)}"); + command.Parameters.AddWithValue("template", world.TemplateId); + + await command.ExecuteNonQueryAsync(); + } + + private sealed record World(Guid ActorId, Guid DeletedInstanceId, Guid LiveInstanceId, Guid RetiredCredentialId, Guid LiveCredentialId, Guid DeletedVariableId, Guid LiveVariableId, Guid RevokedStorageCredentialId, Guid ActiveStorageCredentialId, Guid TemplateId, Guid ReplacedEnvelopeId, Guid RotatedEnvelopeId); + + /// Everything live, as this build or the previous one creates it: two connections with a token and a group hook each, two Secret variables, two storage credentials rotated once, and a deployment default naming its envelope. + private async Task SeedAsync() + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var suffix = Guid.NewGuid().ToString("N")[..10]; + var actor = new User { Id = Guid.NewGuid(), Email = $"sweep-{suffix}@test.local", Name = $"sweep-{suffix}" }; + var team = new Team { Id = Guid.NewGuid(), Slug = $"sweep-{suffix}", Name = "Sweep Team" }; + + db.User.Add(actor); + db.Team.Add(team); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = team.Id, UserId = actor.Id, Role = TeamRole.Owner }); + + var deletedInstance = Instance(team.Id, $"https://gone-{suffix}.example", "CfDJ8-CANARY-deleted-client-secret"); + var liveInstance = Instance(team.Id, $"https://live-{suffix}.example", "CfDJ8-CANARY-live-client-secret"); + var retired = Token(team.Id, deletedInstance.Id, "CfDJ8-CANARY-retired-token"); + var live = Token(team.Id, liveInstance.Id, "CfDJ8-CANARY-live-token"); + + db.ProviderInstance.AddRange(deletedInstance, liveInstance); + db.Credential.AddRange(retired, live); + + var deletedVariable = SecretVariable(team.Id, "DELETED_SECRET", new byte[] { 1, 2, 3, 4 }); + var liveVariable = SecretVariable(team.Id, "LIVE_SECRET", new byte[] { 7, 7, 7, 7 }); + + db.Variable.AddRange(deletedVariable, liveVariable); + await db.SaveChangesAsync(); + + db.ConnectionWebhook.AddRange(Hook(deletedInstance.Id, retired.Id, "hook-777", "CfDJ8-CANARY-deleted-hook-secret"), Hook(liveInstance.Id, live.Id, "hook-888", "CfDJ8-CANARY-live-hook-secret")); + + var revoked = await StorageCredentialAsync(db, team.Id, actor.Id, "revoked"); + var active = await StorageCredentialAsync(db, team.Id, actor.Id, "active"); + var replaced = new StorageDefaultCredential { Id = Guid.NewGuid(), ProviderTypeKey = LocalRwxArtifactStorageDriverFactory.TypeKey, EncryptedPayload = "CfDJ8-CANARY-replaced-envelope", EnvelopeFingerprint = $"sha256:{new string('c', 64)}", CreatedDate = DateTimeOffset.UtcNow, CreatedBy = actor.Id }; + var template = Template(actor.Id, replaced.Id); + + db.StorageDefaultCredential.Add(replaced); + db.StorageDefault.Add(template); + await db.SaveChangesAsync(); + + return new World(actor.Id, deletedInstance.Id, liveInstance.Id, retired.Id, live.Id, deletedVariable.Id, liveVariable.Id, revoked.Id, active.Id, template.Id, replaced.Id, Guid.NewGuid()); + } + + private static ProviderInstance Instance(Guid teamId, string baseUrl, string clientSecretEnc) => new() + { + Id = Guid.NewGuid(), TeamId = teamId, Provider = ProviderKind.GitLab, DisplayName = "gitlab", BaseUrl = baseUrl, OauthClientId = "client", OauthClientSecretEnc = clientSecretEnc, WebhookScope = ProviderWebhookScope.Connection + }; + + private static Credential Token(Guid teamId, Guid instanceId, string payload) => new() + { + Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instanceId, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.OAuth, DisplayName = "token", EncryptedPayload = payload, Status = CredentialStatus.Active + }; + + private static Variable SecretVariable(Guid teamId, string name, byte[] ciphertext) => new() + { + Id = Guid.NewGuid(), Scope = VariableScope.Team, ScopeId = teamId, TeamId = teamId, Name = name, ValueType = VariableValueType.Secret, ValueEncrypted = ciphertext + }; + + private static ConnectionWebhook Hook(Guid instanceId, Guid credentialId, string externalId, string secretEnc) + { + var id = Guid.NewGuid(); + + return new ConnectionWebhook + { + Id = id, ProviderInstanceId = instanceId, CredentialId = credentialId, OwnerPath = "acme", ExternalId = externalId, CallbackUrl = $"https://codespace.test/api/webhooks/connection/{id}", + SecretEnc = secretEnc, SubscribedEvents = new List { "push" }, RegistrationStatus = RepositoryWebhookRegistrationStatus.Registered + }; + } + + /// A template on a data class of its own — a data class is instance-wide, so this test claims none another test uses. + private static StorageDefault Template(Guid actorId, Guid credentialId) => new() + { + Id = Guid.NewGuid(), DataClassTypeKey = $"t{Guid.NewGuid():N}/v1", Revision = 1, ProviderTypeKey = LocalRwxArtifactStorageDriverFactory.TypeKey, NonSecretConfigJson = "{}", NamespaceRoot = "codespace-defaults/", + CredentialId = credentialId, AdoptionPolicy = StorageDefaultAdoptionPolicy.Explicit, IsEnabled = true, CreatedDate = DateTimeOffset.UtcNow, CreatedBy = actorId, LastModifiedDate = DateTimeOffset.UtcNow, LastModifiedBy = actorId + }; + + /// A storage credential rotated once, through the append-only path its guard admits. + private static async Task StorageCredentialAsync(CodeSpaceDbContext db, Guid teamId, Guid actorId, string name) + { + var credential = new StorageCredential { Id = Guid.NewGuid(), TeamId = teamId, StableName = $"{name}-{Guid.NewGuid():N}", CurrentRevision = 1, State = StorageCredentialState.Active, CreatedDate = DateTimeOffset.UtcNow, CreatedBy = actorId }; + credential.Revisions.Add(StorageRevision(credential, actorId, 1, $"CfDJ8-CANARY-{name}-rev-1")); + db.StorageCredential.Add(credential); + await db.SaveChangesAsync(); + + db.StorageCredentialRevision.Add(StorageRevision(credential, actorId, 2, $"CfDJ8-CANARY-{name}-rev-2")); + credential.CurrentRevision = 2; + await db.SaveChangesAsync(); + + return credential; + } + + private static StorageCredentialRevision StorageRevision(StorageCredential credential, Guid actorId, int revision, string payload) => new() + { + Id = Guid.NewGuid(), TeamId = credential.TeamId, StorageCredentialId = credential.Id, Revision = revision, ProviderTypeKey = LocalRwxArtifactStorageDriverFactory.TypeKey, + EncryptedPayload = payload, EnvelopeFingerprint = $"sha256:{new string((char)('a' + revision), 64)}", CreatedDate = DateTimeOffset.UtcNow, CreatedBy = actorId + }; + + private async Task PayloadAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Credential.AsNoTracking().Where(c => c.Id == credentialId).Select(c => c.EncryptedPayload).SingleAsync(); + } + + private static async Task> RevisionsAsync(CodeSpaceDbContext db, Guid credentialId) => + await db.StorageCredentialRevision.AsNoTracking().Where(r => r.StorageCredentialId == credentialId).OrderBy(r => r.Revision).Select(r => r.EncryptedPayload).ToListAsync(); + + private static async Task EnvelopeAsync(CodeSpaceDbContext db, Guid envelopeId) => + await db.StorageDefaultCredential.AsNoTracking().Where(e => e.Id == envelopeId).Select(e => e.EncryptedPayload).SingleAsync(); +} diff --git a/backend/tests/CodeSpace.IntegrationTests/OAuth/CredentialPayloadReaderFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/OAuth/CredentialPayloadReaderFlowTests.cs new file mode 100644 index 000000000..97bdce049 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/OAuth/CredentialPayloadReaderFlowTests.cs @@ -0,0 +1,65 @@ +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.IntegrationTests.OAuth; + +/// +/// A refresh overtaken by a write that blanked its row ends the pair it was issued the way that write ends tokens, and +/// tells the two writes apart by : a retirement (connection delete, +/// departure, relink) soft-deletes the row; a Disconnect leaves it listed, Revoked. A row that is gone counts as retired, +/// so the refresher never ends a grant on a guess. Read through the production reader's detached connection against +/// Postgres, on rows written as each path leaves them. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class CredentialPayloadReaderFlowTests +{ + private readonly PostgresFixture _fixture; + + public CredentialPayloadReaderFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + public enum StoredState { Active, Disconnected, Retired, Gone } + + [Theory] + [InlineData(StoredState.Active, false)] + [InlineData(StoredState.Disconnected, false)] + [InlineData(StoredState.Retired, true)] + [InlineData(StoredState.Gone, true)] + public async Task A_credential_is_retired_once_soft_deleted_or_gone_and_not_while_listed(StoredState state, bool expectedRetired) + { + var credentialId = state == StoredState.Gone ? Guid.NewGuid() : await SeedCredentialAsync(state); + + using var scope = _fixture.BeginScope(); + var retired = await scope.Resolve().IsRetiredAsync(credentialId, CancellationToken.None); + + retired.ShouldBe(expectedRetired, $"a {state} credential decides whether a refresh it overtook ends the user's whole grant — check the query in CredentialPayloadReader.IsRetiredAsync"); + } + + /// A credential as each path leaves it: a Disconnect blanks and revokes it in place; a retirement also soft-deletes it. + private async Task SeedCredentialAsync(StoredState state) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var team = new Team { Id = Guid.NewGuid(), Slug = $"reader-{Guid.NewGuid().ToString("N")[..8]}", Name = "Reader" }; + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.GitHub, DisplayName = "GitHub", BaseUrl = "https://github.example" }; + var credential = new Credential + { + Id = Guid.NewGuid(), TeamId = team.Id, ProviderInstanceId = instance.Id, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.OAuth, DisplayName = "octocat", + EncryptedPayload = state == StoredState.Active ? "CfDJ8-CANARY-live-token" : string.Empty, + Status = state == StoredState.Active ? CredentialStatus.Active : CredentialStatus.Revoked, + DeletedDate = state == StoredState.Retired ? DateTimeOffset.UtcNow : null + }; + + db.Team.Add(team); + db.ProviderInstance.Add(instance); + db.Credential.Add(credential); + await db.SaveChangesAsync(); + + return credential.Id; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Persistence/RetiredSecretShredMigrationTests.cs b/backend/tests/CodeSpace.IntegrationTests/Persistence/RetiredSecretShredMigrationTests.cs new file mode 100644 index 000000000..da52b6cb3 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Persistence/RetiredSecretShredMigrationTests.cs @@ -0,0 +1,410 @@ +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Workflows.Artifacts.Providers.Local; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Enums; +using DbUp; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Persistence; + +/// +/// 0249 over a database that already holds what retirement used to leave behind: a deleted connection's Revoked tokens, +/// OAuth client secret and group-hook secrets, a deleted Secret variable's ciphertext, every revision of a revoked storage +/// credential, and the deployment-default envelopes a rotation or a clear left unnamed — all still decryptable. The +/// upgrade blanks exactly those and nothing live, relaxes the three schema guards that forbade it, and must not refuse: +/// a migration that throws is a deployment that does not start. +/// +/// Driven through the production over its own database, migrated up to (not including) +/// 0249 and seeded through the real model — the schema a deployment upgrading across 0249 meets. Dropped on dispose; the +/// shared test database is never touched. +/// +/// 0249 runs once. The same rows written after it — by a build older than 0249 still serving, or after a rollback — +/// are the recurring sweep's (), and it is held to the same assertions. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class RetiredSecretShredMigrationTests +{ + private const string Shredding = "0249"; + + private readonly PostgresFixture _fixture; + + public RetiredSecretShredMigrationTests(PostgresFixture fixture) { _fixture = fixture; } + + [Fact] + public async Task Upgrading_shreds_what_retirement_left_behind_and_leaves_every_live_secret_alone() + { + await using var database = await PreUpgradeDatabase.CreateAsync(_fixture.ConnectionString, Shredding); + var world = await SeedAsync(database.ConnectionString); + + new DbUpRunner(database.ConnectionString).Run(); + + await using var db = Context(database.ConnectionString); + + await ShouldHoldNoRetiredSecretAndEveryLiveOneAsync(db, world); + } + + /// + /// The same rows written AFTER the upgrade — what a build older than 0249 leaves when it retires something while it + /// still serves during a rolling deploy, or after a rollback. 0249 has already run and does not run again; the + /// recurring sweep must shred exactly what it would have. Same seed, same assertions as the upgrade above, so the two + /// cannot drift apart on this corpus: a kind or a predicate the sweep misses, or one it over-reaches, fails here. + /// + [Fact] + public async Task The_sweep_shreds_what_an_older_build_retires_after_the_upgrade_exactly_as_the_upgrade_does() + { + await using var database = await PreUpgradeDatabase.CreateAsync(_fixture.ConnectionString, Shredding); + + new DbUpRunner(database.ConnectionString).Run(); + + var world = await SeedAsync(database.ConnectionString); + + await using (var sweep = Context(database.ConnectionString)) + { + (await new RetiredSecretShredService(sweep, NullLogger.Instance).ShredAsync(CancellationToken.None)).ShouldBeGreaterThan(0, "fixture check: the older build's retirements were written after 0249 and left their ciphertext"); + } + + await using var db = Context(database.ConnectionString); + + await ShouldHoldNoRetiredSecretAndEveryLiveOneAsync(db, world); + } + + private static async Task ShouldHoldNoRetiredSecretAndEveryLiveOneAsync(CodeSpaceDbContext db, World world) + { + (await PayloadAsync(db, world.RetiredCredentialId)).ShouldBe(string.Empty, "a deleted connection's Revoked token must not stay decryptable"); + (await PayloadAsync(db, world.SoftDeletedCredentialId)).ShouldBe(string.Empty, "a soft-deleted token is out of every list and every read; nothing presents it again"); + (await PayloadAsync(db, world.DisconnectedCredentialId)).ShouldBe(string.Empty, "a Revoked token on a live connection is no more usable than one on a deleted connection"); + (await PayloadAsync(db, world.LiveCredentialId)).ShouldBe("CfDJ8-CANARY-live-token", "a live token is untouched"); + (await ClientSecretAsync(db, world.DeletedInstanceId)).ShouldBeNull("a deleted connection's OAuth client secret must not stay decryptable"); + (await ClientSecretAsync(db, world.LiveInstanceId)).ShouldBe("CfDJ8-CANARY-live-client-secret", "a live connection keeps its client secret"); + + (await VariableAsync(db, world.DeletedSecretVariableId)).ValueEncrypted.ShouldBeNull("a deleted Secret variable's ciphertext must not survive the upgrade"); + (await VariableAsync(db, world.LiveSecretVariableId)).ValueEncrypted.ShouldBe(world.LiveCiphertext, "a live Secret keeps its ciphertext byte for byte"); + (await VariableAsync(db, world.DeletedPlainVariableId)).ValuePlain.ShouldBe("\"plain-kept\"", "a plain value is no secret and stays"); + + (await RevisionPayloadsAsync(db, world.RevokedStorageCredentialId)).ShouldBe(new[] { string.Empty, string.Empty }, ignoreOrder: false, customMessage: "every revision of a revoked storage credential is blanked"); + (await RevisionPayloadsAsync(db, world.ActiveStorageCredentialId)).ShouldBe(new[] { "CfDJ8-CANARY-active-rev-1", "CfDJ8-CANARY-active-rev-2" }, ignoreOrder: false, customMessage: "an Active credential's revisions — the superseded one too, which a profile may pin — are untouched"); + + var deletedHooks = await HooksAsync(db, world.DeletedInstanceId); + deletedHooks.Count.ShouldBe(3, "fixture check: a Registered, a Pending and a Cancelled hook were seeded on the deleted connection"); + deletedHooks.ShouldAllBe(h => h.RegistrationStatus == RepositoryWebhookRegistrationStatus.Cancelled && h.SecretEnc == string.Empty, "a deleted connection's group hooks keep no secret, and none stays in service with a secret it no longer holds"); + deletedHooks.Single(h => h.OwnerPath == "acme").ExternalId.ShouldBe("hook-777", "the provider's hook id stays: it is what an operator removes by hand, since no token is left to remove it"); + (await HooksAsync(db, world.LiveInstanceId)).ShouldHaveSingleItem().ShouldSatisfyAllConditions( + h => h.RegistrationStatus.ShouldBe(RepositoryWebhookRegistrationStatus.Registered, "a live connection's hook stays in service"), + h => h.SecretEnc.ShouldBe("CfDJ8-CANARY-live-hook-secret", "and keeps the secret its deliveries are verified against")); + + (await EnvelopeAsync(db, world.SupersededEnvelopeId)).ShouldBe(string.Empty, "a deployment-default envelope no template names has no reader left"); + (await EnvelopeAsync(db, world.NamedEnvelopeId)).ShouldBe("CfDJ8-CANARY-named-envelope", "the envelope a template names is the one the materializer decrypts"); + } + + [Fact] + public async Task After_the_upgrade_the_guards_still_refuse_everything_but_a_retirement() + { + await using var database = await PreUpgradeDatabase.CreateAsync(_fixture.ConnectionString, Shredding); + var world = await SeedAsync(database.ConnectionString); + + new DbUpRunner(database.ConnectionString).Run(); + + (await RefusalAsync(database.ConnectionString, $"UPDATE storage_credential_revision SET encrypted_payload = '' WHERE storage_credential_id = '{world.ActiveStorageCredentialId}' AND revision = 1")).ShouldContain("Revoked"); + (await RefusalAsync(database.ConnectionString, $"UPDATE storage_credential_revision SET envelope_fingerprint = 'sha256:{new string('f', 64)}' WHERE storage_credential_id = '{world.RevokedStorageCredentialId}'")).ShouldContain("immutable"); + (await RefusalAsync(database.ConnectionString, $"UPDATE variable SET value_encrypted = NULL WHERE id = '{world.LiveSecretVariableId}'")).ShouldContain("chk_variable_value_exclusive"); + (await RefusalAsync(database.ConnectionString, $"UPDATE storage_default_credential SET encrypted_payload = '' WHERE id = '{world.NamedEnvelopeId}'")).ShouldContain("no template names it"); + (await RefusalAsync(database.ConnectionString, $"UPDATE storage_default_credential SET safe_hint = 'rewritten' WHERE id = '{world.SupersededEnvelopeId}'")).ShouldContain("append-only"); + (await RefusalAsync(database.ConnectionString, $"DELETE FROM storage_default_credential WHERE id = '{world.SupersededEnvelopeId}'")).ShouldContain("append-only"); + (await RefusalAsync(database.ConnectionString, $"INSERT INTO storage_default_credential (id, provider_type_key, encrypted_payload, envelope_fingerprint, created_date, created_by) SELECT gen_random_uuid(), provider_type_key, '', envelope_fingerprint, now(), created_by FROM storage_default_credential WHERE id = '{world.NamedEnvelopeId}'")).ShouldContain("payload is required"); + } + + /// + /// The rollback the script's header documents, run as written on a database the script has shredded. The old checks + /// reject exactly the rows a shred leaves — a deleted Secret with no ciphertext, a blank revision or envelope — so a + /// note that restored them as they were would fail with 23514 during the very incident it is followed in. The + /// statements are read out of the header itself, so the note cannot drift from what this proves. + /// + [Fact] + public async Task The_documented_rollback_runs_on_a_shredded_database_and_restores_the_old_rules_for_new_rows() + { + await using var database = await PreUpgradeDatabase.CreateAsync(_fixture.ConnectionString, Shredding); + var world = await SeedAsync(database.ConnectionString); + + new DbUpRunner(database.ConnectionString).Run(); + + var rollback = RollbackScript(); + + rollback.ShouldContain("NOT VALID", customMessage: "fixture check: the statements were read out of 0249's header"); + + await ExecuteAsync(database.ConnectionString, rollback); + + (await RefusalAsync(database.ConnectionString, $"UPDATE storage_credential_revision SET encrypted_payload = '' WHERE storage_credential_id = '{world.ActiveStorageCredentialId}' AND revision = 1")).ShouldContain("immutable"); + (await RefusalAsync(database.ConnectionString, $"UPDATE storage_default_credential SET encrypted_payload = 'x' WHERE id = '{world.NamedEnvelopeId}'")).ShouldContain("append-only"); + (await RefusalAsync(database.ConnectionString, $"UPDATE variable SET value_encrypted = NULL, deleted_date = now() WHERE id = '{world.LiveSecretVariableId}'")).ShouldContain("chk_variable_value_exclusive"); + } + + /// + /// Rolling forward after that rollback, the way the header says: drop the script's journal row and start again. With + /// the old checks back, this build could not retire a secret — a Secret variable's delete would fail with 23514 and + /// the recurring sweep would fail every tick — so the script has to apply a second time over its own shredded rows, + /// and admit the application's blanking again. + /// + [Fact] + public async Task Rolling_forward_after_the_documented_rollback_applies_the_script_again_and_retirement_blanks_again() + { + await using var database = await PreUpgradeDatabase.CreateAsync(_fixture.ConnectionString, Shredding); + var world = await SeedAsync(database.ConnectionString); + + new DbUpRunner(database.ConnectionString).Run(); + await ExecuteAsync(database.ConnectionString, RollbackScript()); + + var rollForward = HeaderStatements("DELETE FROM schemaversions ").ShouldHaveSingleItem("fixture check: the roll-forward statement was read out of 0249's header"); + + await ExecuteAsync(database.ConnectionString, rollForward); + new DbUpRunner(database.ConnectionString).Run(); + + await ExecuteAsync(database.ConnectionString, $"UPDATE variable SET value_encrypted = NULL, deleted_date = now() WHERE id = '{world.LiveSecretVariableId}'"); + + await using var db = Context(database.ConnectionString); + + (await VariableAsync(db, world.LiveSecretVariableId)).ValueEncrypted.ShouldBeNull("a Secret variable's delete blanks it again, as VariableRetirement does"); + (await new RetiredSecretShredService(db, NullLogger.Instance).ShredAsync(CancellationToken.None)).ShouldBe(0, "the second run's backfill found the rows the first left blank, and the sweep has nothing left to do"); + } + + /// + /// 0249's rollback: the statements its header lists, then 0126's and 0173's guard functions verbatim, as the header + /// says to. + /// + private static string RollbackScript() + { + var statements = HeaderStatements("ALTER TABLE ", "DROP TRIGGER ", "CREATE TRIGGER "); + + return string.Join("\n", statements.Append(FunctionBlock("0126_", "storage_credential_revision_guard")).Append(FunctionBlock("0173_", "storage_default_credential_guard"))); + } + + /// The statements 0249's header comment lists that start with one of . + private static IReadOnlyList HeaderStatements(params string[] starts) => + File.ReadLines(Script("0249_")) + .TakeWhile(line => line.StartsWith("--", StringComparison.Ordinal)) + .Select(line => line.Length > 2 ? line[2..].Trim() : string.Empty) + .Where(line => starts.Any(start => line.StartsWith(start, StringComparison.Ordinal))) + .ToList(); + + private static string FunctionBlock(string scriptPrefix, string function) + { + var script = File.ReadAllText(Script(scriptPrefix)); + var start = script.IndexOf($"CREATE OR REPLACE FUNCTION {function}()", StringComparison.Ordinal); + const string end = "$$ LANGUAGE plpgsql;"; + + start.ShouldBeGreaterThanOrEqualTo(0, $"fixture check: {scriptPrefix} defines {function}"); + + return script[start..(script.IndexOf(end, start, StringComparison.Ordinal) + end.Length)]; + } + + private static string Script(string prefix) => + Directory.GetFiles(Path.Combine(Path.GetDirectoryName(typeof(DbUpRunner).Assembly.Location)!, DbUpRunner.ScriptFolder), $"{prefix}*.sql").ShouldHaveSingleItem(); + + private static async Task ExecuteAsync(string connectionString, string sql) + { + await using var connection = new NpgsqlConnection(connectionString); + await connection.OpenAsync(); + await using var command = new NpgsqlCommand(sql, connection); + + await command.ExecuteNonQueryAsync(); + } + + private static async Task PayloadAsync(CodeSpaceDbContext db, Guid credentialId) => + await db.Credential.AsNoTracking().Where(c => c.Id == credentialId).Select(c => c.EncryptedPayload).SingleAsync(); + + private static async Task ClientSecretAsync(CodeSpaceDbContext db, Guid instanceId) => + await db.ProviderInstance.AsNoTracking().Where(p => p.Id == instanceId).Select(p => p.OauthClientSecretEnc).SingleAsync(); + + private static async Task VariableAsync(CodeSpaceDbContext db, Guid variableId) => + await db.Variable.AsNoTracking().SingleAsync(v => v.Id == variableId); + + private static async Task> RevisionPayloadsAsync(CodeSpaceDbContext db, Guid credentialId) => + await db.StorageCredentialRevision.AsNoTracking().Where(r => r.StorageCredentialId == credentialId).OrderBy(r => r.Revision).Select(r => r.EncryptedPayload).ToListAsync(); + + private static async Task> HooksAsync(CodeSpaceDbContext db, Guid instanceId) => + await db.ConnectionWebhook.AsNoTracking().Where(w => w.ProviderInstanceId == instanceId).ToListAsync(); + + private static async Task EnvelopeAsync(CodeSpaceDbContext db, Guid envelopeId) => + await db.StorageDefaultCredential.AsNoTracking().Where(e => e.Id == envelopeId).Select(e => e.EncryptedPayload).SingleAsync(); + + private static async Task RefusalAsync(string connectionString, string sql) + { + await using var connection = new NpgsqlConnection(connectionString); + await connection.OpenAsync(); + await using var command = new NpgsqlCommand(sql, connection); + + var refused = await Should.ThrowAsync(() => command.ExecuteNonQueryAsync()); + + return $"{refused.ConstraintName} {refused.MessageText}"; + } + + private sealed record World(Guid DeletedInstanceId, Guid LiveInstanceId, Guid RetiredCredentialId, Guid SoftDeletedCredentialId, Guid DisconnectedCredentialId, Guid LiveCredentialId, Guid DeletedSecretVariableId, Guid LiveSecretVariableId, Guid DeletedPlainVariableId, byte[] LiveCiphertext, Guid RevokedStorageCredentialId, Guid ActiveStorageCredentialId, Guid SupersededEnvelopeId, Guid NamedEnvelopeId); + + /// + /// The rows the pre-0249 code leaves: what its provider-instance delete, variable delete, storage revoke and + /// deployment-default rotation write, beside live ones of each kind. + /// + private static async Task SeedAsync(string connectionString) + { + await using var db = Context(connectionString); + var now = DateTimeOffset.UtcNow; + var suffix = Guid.NewGuid().ToString("N")[..10]; + var actor = new User { Id = Guid.NewGuid(), Email = $"shred-{suffix}@test.local", Name = $"shred-{suffix}" }; + var team = new Team { Id = Guid.NewGuid(), Slug = $"shred-{suffix}", Name = "Shred Team", Kind = TeamKind.Workspace }; + + db.User.Add(actor); + db.Team.Add(team); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = team.Id, UserId = actor.Id, Role = TeamRole.Owner }); + + var deletedInstance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.GitLab, DisplayName = "gone", BaseUrl = $"https://gone-{suffix}.example", OauthClientId = "client", OauthClientSecretEnc = "CfDJ8-CANARY-deleted-client-secret", DeletedDate = now }; + var liveInstance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.GitLab, DisplayName = "live", BaseUrl = $"https://live-{suffix}.example", OauthClientId = "client", OauthClientSecretEnc = "CfDJ8-CANARY-live-client-secret" }; + var retired = Token(team.Id, deletedInstance.Id, "CfDJ8-CANARY-retired-token", CredentialStatus.Revoked, now); + var softDeleted = Token(team.Id, liveInstance.Id, "CfDJ8-CANARY-soft-deleted-token", CredentialStatus.Active, now); + var disconnected = Token(team.Id, liveInstance.Id, "CfDJ8-CANARY-disconnected-token", CredentialStatus.Revoked, deletedDate: null); + var live = Token(team.Id, liveInstance.Id, "CfDJ8-CANARY-live-token", CredentialStatus.Active, deletedDate: null); + + db.ProviderInstance.AddRange(deletedInstance, liveInstance); + db.Credential.AddRange(retired, softDeleted, disconnected, live); + + var liveCiphertext = new byte[] { 7, 7, 7, 7 }; + var deletedSecret = Variable(team.Id, "DELETED_SECRET", VariableValueType.Secret, plain: null, encrypted: new byte[] { 1, 2, 3, 4 }, now); + var liveSecret = Variable(team.Id, "LIVE_SECRET", VariableValueType.Secret, plain: null, encrypted: liveCiphertext, deletedDate: null); + var deletedPlain = Variable(team.Id, "DELETED_PLAIN", VariableValueType.String, plain: "\"plain-kept\"", encrypted: null, now); + + db.Variable.AddRange(deletedSecret, liveSecret, deletedPlain); + await db.SaveChangesAsync(); + + var revoked = await StorageCredentialAsync(db, team.Id, actor.Id, "revoked"); + var active = await StorageCredentialAsync(db, team.Id, actor.Id, "active"); + + revoked.State = StorageCredentialState.Revoked; + revoked.RevokedDate = now; + revoked.RevokedBy = actor.Id; + + db.ConnectionWebhook.AddRange( + Hook(deletedInstance.Id, retired.Id, "acme", RepositoryWebhookRegistrationStatus.Registered, "hook-777", "CfDJ8-CANARY-deleted-hook-secret"), + Hook(deletedInstance.Id, retired.Id, "acme/web", RepositoryWebhookRegistrationStatus.Pending, externalId: null, "CfDJ8-CANARY-pending-hook-secret"), + Hook(deletedInstance.Id, retired.Id, "acme/old", RepositoryWebhookRegistrationStatus.Cancelled, externalId: null, "CfDJ8-CANARY-cancelled-hook-secret"), + Hook(liveInstance.Id, live.Id, "acme", RepositoryWebhookRegistrationStatus.Registered, "hook-888", "CfDJ8-CANARY-live-hook-secret")); + + var superseded = Envelope(actor.Id, "CfDJ8-CANARY-superseded-envelope"); + var named = Envelope(actor.Id, "CfDJ8-CANARY-named-envelope"); + + db.StorageDefaultCredential.AddRange(superseded, named); + db.StorageDefault.Add(Template(actor.Id, named.Id, now)); + await db.SaveChangesAsync(); + + return new World(deletedInstance.Id, liveInstance.Id, retired.Id, softDeleted.Id, disconnected.Id, live.Id, deletedSecret.Id, liveSecret.Id, deletedPlain.Id, liveCiphertext, revoked.Id, active.Id, superseded.Id, named.Id); + } + + private static ConnectionWebhook Hook(Guid instanceId, Guid credentialId, string ownerPath, RepositoryWebhookRegistrationStatus status, string? externalId, string secretEnc) + { + var id = Guid.NewGuid(); + + return new ConnectionWebhook + { + Id = id, ProviderInstanceId = instanceId, CredentialId = credentialId, OwnerPath = ownerPath, ExternalId = externalId, CallbackUrl = $"https://codespace.test/api/webhooks/connection/{id}", + SecretEnc = secretEnc, SubscribedEvents = new List { "push" }, RegistrationStatus = status, + }; + } + + private static StorageDefaultCredential Envelope(Guid actorId, string payload) => new() + { + Id = Guid.NewGuid(), ProviderTypeKey = LocalRwxArtifactStorageDriverFactory.TypeKey, EncryptedPayload = payload, EnvelopeFingerprint = $"sha256:{new string('c', 64)}", CreatedDate = DateTimeOffset.UtcNow, CreatedBy = actorId, + }; + + /// A template on a key of its own — a data class is instance-wide, so the shred test claims none another test uses. + private static StorageDefault Template(Guid actorId, Guid credentialId, DateTimeOffset now) => new() + { + Id = Guid.NewGuid(), DataClassTypeKey = $"t{Guid.NewGuid():N}/v1", Revision = 2, ProviderTypeKey = LocalRwxArtifactStorageDriverFactory.TypeKey, NonSecretConfigJson = "{}", NamespaceRoot = "codespace-defaults/", + CredentialId = credentialId, AdoptionPolicy = StorageDefaultAdoptionPolicy.Explicit, IsEnabled = true, CreatedDate = now, CreatedBy = actorId, LastModifiedDate = now, LastModifiedBy = actorId, + }; + + private static Credential Token(Guid teamId, Guid instanceId, string payload, CredentialStatus status, DateTimeOffset? deletedDate) => new() + { + Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instanceId, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.OAuth, + DisplayName = "token", EncryptedPayload = payload, Status = status, DeletedDate = deletedDate, + }; + + private static Variable Variable(Guid teamId, string name, VariableValueType type, string? plain, byte[]? encrypted, DateTimeOffset? deletedDate) => new() + { + Id = Guid.NewGuid(), Scope = VariableScope.Team, ScopeId = teamId, TeamId = teamId, Name = name, ValueType = type, ValuePlain = plain, ValueEncrypted = encrypted, DeletedDate = deletedDate, + }; + + /// A storage credential rotated once, through the append-only path its guard admits. + private static async Task StorageCredentialAsync(CodeSpaceDbContext db, Guid teamId, Guid actorId, string name) + { + var credential = new StorageCredential { Id = Guid.NewGuid(), TeamId = teamId, StableName = $"{name}-{Guid.NewGuid():N}", CurrentRevision = 1, State = StorageCredentialState.Active, CreatedDate = DateTimeOffset.UtcNow, CreatedBy = actorId }; + credential.Revisions.Add(StorageRevision(credential, actorId, 1, $"CfDJ8-CANARY-{name}-rev-1")); + db.StorageCredential.Add(credential); + await db.SaveChangesAsync(); + + db.StorageCredentialRevision.Add(StorageRevision(credential, actorId, 2, $"CfDJ8-CANARY-{name}-rev-2")); + credential.CurrentRevision = 2; + await db.SaveChangesAsync(); + + return credential; + } + + private static StorageCredentialRevision StorageRevision(StorageCredential credential, Guid actorId, int revision, string payload) => new() + { + Id = Guid.NewGuid(), TeamId = credential.TeamId, StorageCredentialId = credential.Id, Revision = revision, ProviderTypeKey = LocalRwxArtifactStorageDriverFactory.TypeKey, + EncryptedPayload = payload, EnvelopeFingerprint = $"sha256:{new string((char)('a' + revision), 64)}", CreatedDate = DateTimeOffset.UtcNow, CreatedBy = actorId, + }; + + private static CodeSpaceDbContext Context(string connectionString) => + new(new DbContextOptionsBuilder().UseNpgsql(connectionString).UseSnakeCaseNamingConvention().Options); + + /// + /// Its own database, migrated only up to (not including) firstPendingScript, so the runner meets exactly the + /// schema a deployment upgrading across that script meets. The name carries a GUID; dropped on dispose. + /// + private sealed class PreUpgradeDatabase : IAsyncDisposable + { + private readonly string _name; + + private PreUpgradeDatabase(string connectionString, string name) { ConnectionString = connectionString; _name = name; } + + public string ConnectionString { get; } + + public static async Task CreateAsync(string sharedConnectionString, string firstPendingScript) + { + var name = $"codespace_shred_{Guid.NewGuid():N}"; + var database = new PreUpgradeDatabase(new NpgsqlConnectionStringBuilder(sharedConnectionString) { Database = name }.ConnectionString, name); + + await database.AdminAsync($"CREATE DATABASE \"{name}\""); + + try + { + var scripts = Path.Combine(Path.GetDirectoryName(typeof(DbUpRunner).Assembly.Location)!, DbUpRunner.ScriptFolder); + var upgrade = DeployChanges.To.PostgresqlDatabase(database.ConnectionString) + .WithScriptsFromFileSystem(scripts, path => string.CompareOrdinal(Path.GetFileName(path)[..4], firstPendingScript) < 0) + .WithVariablesDisabled().WithTransaction().LogToNowhere().Build().PerformUpgrade(); + + if (!upgrade.Successful) throw upgrade.Error; + + return database; + } + catch + { + await database.DisposeAsync(); + throw; + } + } + + public async ValueTask DisposeAsync() => await AdminAsync($"DROP DATABASE IF EXISTS \"{_name}\" WITH (FORCE)"); + + private async Task AdminAsync(string sql) + { + await using var admin = new NpgsqlConnection(new NpgsqlConnectionStringBuilder(ConnectionString) { Database = "postgres" }.ConnectionString); + await admin.OpenAsync(); + await using var command = new NpgsqlCommand(sql, admin); + await command.ExecuteNonQueryAsync(); + } + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Persistence/StorageDefaultPersistenceTests.cs b/backend/tests/CodeSpace.IntegrationTests/Persistence/StorageDefaultPersistenceTests.cs index 0ced825d6..0b37f96fd 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Persistence/StorageDefaultPersistenceTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Persistence/StorageDefaultPersistenceTests.cs @@ -160,6 +160,48 @@ public async Task Credential_envelope_is_append_only() } } + /// + /// The one change an envelope ever admits (0249): its payload blanked, every other column as it was, once no template + /// names it — the materializer decrypts only the envelope its template points at, so a rotated-out or cleared one has + /// no reader left, and keeping it would leave a deployment-wide cloud key readable from every backup forever. The + /// envelope a template still names is refused, a second blanking is refused, and an envelope is never inserted blank. + /// + [Fact] + public async Task Credential_envelope_admits_one_blanking_and_only_once_no_template_names_it() + { + var (_, actorId) = await SeedTeamAsync(); + var named = Credential(actorId, "CfDJ8-CANARY-named-envelope"); + var superseded = Credential(actorId, "CfDJ8-CANARY-superseded-envelope"); + + using (var scope = _fixture.BeginScope()) + { + var db = scope.Resolve(); + db.StorageDefaultCredential.AddRange(named, superseded); + db.StorageDefault.Add(Template(UniqueDataClass(), actorId, named.Id)); + await db.SaveChangesAsync(); + } + + (await SetEnvelopeAsync(superseded.Id)).ShouldBe(1, "an envelope no template names has no reader left and may be blanked"); + (await EnvelopeAsync(superseded.Id)).ShouldBe(string.Empty); + + foreach (var payload in new[] { string.Empty, "CfDJ8-CANARY-refilled" }) + { + var again = await Should.ThrowAsync(() => SetEnvelopeAsync(superseded.Id, payload)); + again.MessageText.ShouldContain("append-only", customMessage: "a blanked envelope is final — it is blanked once and nothing writes into it again"); + } + + var stillNamed = await Should.ThrowAsync(() => SetEnvelopeAsync(named.Id)); + stillNamed.MessageText.ShouldContain("no template names it", customMessage: "the materializer still decrypts the envelope its template points at"); + (await EnvelopeAsync(named.Id)).ShouldBe("CfDJ8-CANARY-named-envelope"); + + using (var scope = _fixture.BeginScope()) + { + var db = scope.Resolve(); + db.StorageDefaultCredential.Add(Credential(actorId, string.Empty)); + (await db.SaveChangesAsync().ShouldThrowAsync()).InnerException?.Message.ShouldContain("payload is required"); + } + } + /// /// One adoption per (team, data class), and the profile it names must belong to that same team — a materialization /// that recorded another team's profile would be provenance pointing at storage the team does not own. @@ -255,6 +297,25 @@ private async Task SeedProfileAsync(Guid teamId, Guid actorId) return profile.Id; } + /// Sets an envelope's payload the way a raw UPDATE would — by default to empty, the one change 0249 admits. + private async Task SetEnvelopeAsync(Guid credentialId, string payload = "") + { + await using var connection = new NpgsqlConnection(_fixture.ConnectionString); + await connection.OpenAsync(); + await using var command = new NpgsqlCommand("UPDATE storage_default_credential SET encrypted_payload = @payload WHERE id = @id", connection); + command.Parameters.AddWithValue("payload", payload); + command.Parameters.AddWithValue("id", credentialId); + + return await command.ExecuteNonQueryAsync(); + } + + private async Task EnvelopeAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().StorageDefaultCredential.AsNoTracking().Where(value => value.Id == credentialId).Select(value => value.EncryptedPayload).SingleAsync(); + } + private async Task<(Guid TeamId, Guid ActorId)> SeedTeamAsync() { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/SharedOAuthAppRevocationFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/SharedOAuthAppRevocationFlowTests.cs new file mode 100644 index 000000000..8c21fde87 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/SharedOAuthAppRevocationFlowTests.cs @@ -0,0 +1,316 @@ +using System.Collections.Concurrent; +using System.Text; +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Providers; +using CodeSpace.Core.Services.Providers.Auth; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Commands.ProviderInstances; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.ProviderInstances; + +/// +/// Two teams connect GitHub through the same OAuth App — the unique index is per team, so nothing stops them sharing a +/// client id — and the same person authorizes it in both. GitHub keeps one grant per user and app: its grant delete ends +/// every token issued under it, the other team's included. Deleting one team's connection must end only the tokens that +/// team held; a Disconnect keeps ending the whole grant, as it always has. The same holds for the pair GitHub issues a +/// refresh that a retirement or a Disconnect overtakes. +/// +/// Drives the real chain: the mediator pipeline and its transaction, ProviderInstanceService and +/// CredentialService, the post-commit revoke, the real GitHubOAuthClient speaking HTTP, and Postgres — +/// against a GitHub on a loopback port that keeps grants the way GitHub does, so what each call ends is read off the +/// provider's own state rather than inferred from which URL was hit. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class SharedOAuthAppRevocationFlowTests +{ + private const string ClientSecret = "shared-app-secret"; + + private readonly PostgresFixture _fixture; + + public SharedOAuthAppRevocationFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + /// What lands while team A's refresh is at GitHub and blanks the token it read. + public enum RetiringWrite + { + /// This build's connection delete: it retires the credential (blanked, Revoked, soft-deleted) and ends team A's token by token. + ConnectionDelete, + + /// The previous build's connection delete — Revoked and soft-deleted, token kept, nothing asked of GitHub — then this build's sweep blanking it: a rolling deploy or a rollback. + PreviousBuildDeleteThenSweep + } + + [Fact] + public async Task Deleting_one_teams_connection_ends_only_that_teams_token_and_not_the_grant_another_team_shares() + { + using var gitHub = new StubProviderHost(); + var world = await SeedSharedAppAsync(gitHub, gitHub); + + await DeleteConnectionAsync(world.TeamA, world.InstanceA); + + world.Grants.IsLive(world.AccessTokenA).ShouldBeFalse($"team A's token must end at GitHub — check the post-commit revoke in ProviderInstanceService; GitHub saw: {Seen(gitHub)}"); + world.Grants.IsLive(world.AccessTokenB).ShouldBeTrue($"team B connected through the same OAuth App and must keep working; a grant delete would have ended its token too. GitHub saw: {Seen(gitHub)}"); + gitHub.Requests.ShouldNotContain(r => r.PathAndQuery.Contains("/grant"), "a connection delete never asks GitHub for the user's whole grant"); + gitHub.Requests.ShouldNotContain(r => r.Body.Contains(world.RefreshTokenA), "GitHub has no endpoint that ends a refresh token alone, so none is sent"); + (await StoredAccessTokenAsync(world.CredentialB)).ShouldBe(world.AccessTokenB, "the other team's credential is untouched here too"); + } + + [Fact] + public async Task A_disconnect_still_ends_the_whole_grant() + { + using var gitHub = new StubProviderHost(); + var world = await SeedSharedAppAsync(gitHub, gitHub); + + var result = await DisconnectAsync(world.TeamB, world.CredentialB); + + result.ProviderAcknowledged.ShouldBeTrue($"GitHub answered the grant delete; saw: {Seen(gitHub)}"); + gitHub.Requests.ShouldContain(r => r.Method == "DELETE" && r.PathAndQuery.EndsWith($"/applications/{world.ClientId}/grant", StringComparison.Ordinal), "a Disconnect ends the user's grant to the app, unchanged"); + world.Grants.IsLive(world.AccessTokenB).ShouldBeFalse(); + world.Grants.IsLive(world.AccessTokenA).ShouldBeFalse("the grant is the user's, not the team's: ending it ends the token the other team holds too — which is why a connection delete must not ask for it"); + } + + /// + /// Team A's token is inside the refresh window, and the connection is retired while GitHub holds the refresh: the + /// rotated pair GitHub then issues is stored nowhere, so the refresher ends it at GitHub itself. A retirement ends + /// tokens, never the person's grant — the same rule the delete's own revoke follows — so team B, connected through the + /// same OAuth App by the same person, keeps working. The refresher tells a retirement from a Disconnect by the + /// credential's soft delete, which only a retirement writes. + /// + [Theory] + [InlineData(RetiringWrite.ConnectionDelete)] + [InlineData(RetiringWrite.PreviousBuildDeleteThenSweep)] + public async Task A_connection_retired_while_its_refresh_is_at_GitHub_ends_only_the_pair_GitHub_issued(RetiringWrite retirement) + { + using var gitHub = new StubProviderHost(); + using var signIn = new StubProviderHost(); + var world = await SeedSharedAppAsync(gitHub, signIn); + world.Grants.ServeRefresh(signIn, "octocat", world.RotatedAccessTokenA, world.RotatedRefreshTokenA, whileRefreshing: () => RetireAsync(world, retirement).GetAwaiter().GetResult()); + + var failure = await Should.ThrowAsync(() => ResolveTokenAsync(world.CredentialA)); + + failure.Message.ShouldContain("was revoked", customMessage: "fixture check: the retirement blanked the row while GitHub answered, so the refresh found it empty"); + world.Grants.IsLive(world.RotatedAccessTokenA).ShouldBeFalse($"the pair issued while the connection was retired is held by nothing and must end at GitHub; GitHub saw: {Seen(gitHub)}"); + gitHub.Requests.ShouldNotContain(r => r.PathAndQuery.Contains("/grant"), $"a retirement racing a refresh never asks GitHub for the user's whole grant — check OAuthTokenRefresher.RevokeIssuedTokensAsync; GitHub saw: {Seen(gitHub)}"); + world.Grants.IsLive(world.AccessTokenB).ShouldBeTrue("team B connected through the same OAuth App and must keep working"); + gitHub.Requests.ShouldNotContain(r => r.Body.Contains(world.RotatedRefreshTokenA), "GitHub has no endpoint that ends a refresh token alone, so none is sent"); + } + + /// The sibling of the retirement race: a Disconnect landing while the refresh is at GitHub still ends the grant, the pair just issued included. + [Fact] + public async Task A_disconnect_while_a_refresh_is_at_GitHub_still_ends_the_whole_grant() + { + using var gitHub = new StubProviderHost(); + using var signIn = new StubProviderHost(); + var world = await SeedSharedAppAsync(gitHub, signIn); + world.Grants.ServeRefresh(signIn, "octocat", world.RotatedAccessTokenA, world.RotatedRefreshTokenA, whileRefreshing: () => DisconnectAsync(world.TeamA, world.CredentialA).GetAwaiter().GetResult()); + + var failure = await Should.ThrowAsync(() => ResolveTokenAsync(world.CredentialA)); + + failure.Message.ShouldContain("was revoked", customMessage: "fixture check: the Disconnect blanked the row while GitHub answered, so the refresh found it empty"); + gitHub.Requests.ShouldContain(r => r.Method == "DELETE" && r.PathAndQuery.EndsWith($"/applications/{world.ClientId}/grant", StringComparison.Ordinal) && r.Body.Contains(world.RotatedAccessTokenA), $"the pair issued during a Disconnect ends the way the Disconnect does — the whole grant; GitHub saw: {Seen(gitHub)}"); + world.Grants.IsLive(world.RotatedAccessTokenA).ShouldBeFalse(); + } + + private sealed record SharedApp(GitHubGrants Grants, string ClientId, Guid TeamA, Guid InstanceA, Guid CredentialA, string AccessTokenA, string RefreshTokenA, string RotatedAccessTokenA, string RotatedRefreshTokenA, Guid TeamB, Guid InstanceB, Guid CredentialB, string AccessTokenB); + + /// + /// The same person connected in two teams, each team with its own GitHub connection on the same host and OAuth App — + /// two credentials, two access tokens, one grant at GitHub. is GitHub's API, where tokens are + /// revoked; its sign-in host, where they are refreshed — two listeners, so a write landing + /// while a refresh is answered can reach the API. Team A's token is inside the refresh window; team B's is not. + /// + private async Task SeedSharedAppAsync(StubProviderHost gitHub, StubProviderHost signIn) + { + var canary = Guid.NewGuid().ToString("N")[..10]; + var clientId = $"shared-app-{canary}"; + var grants = GitHubGrants.Serve(gitHub, clientId, ClientSecret); + var (teamA, instanceA) = await SeedConnectionAsync(signIn.BaseUrl, gitHub.BaseUrl, clientId).ConfigureAwait(false); + var (teamB, instanceB) = await SeedConnectionAsync(signIn.BaseUrl, gitHub.BaseUrl, clientId).ConfigureAwait(false); + var credentialA = await SeedOAuthCredentialAsync(teamA, instanceA, $"at-A-{canary}", $"rt-A-{canary}", DateTimeOffset.UtcNow.AddSeconds(30)).ConfigureAwait(false); + var credentialB = await SeedOAuthCredentialAsync(teamB, instanceB, $"at-B-{canary}", $"rt-B-{canary}", DateTimeOffset.UtcNow.AddHours(8)).ConfigureAwait(false); + + grants.Issue("octocat", $"at-A-{canary}", $"at-B-{canary}"); + + return new SharedApp(grants, clientId, teamA, instanceA, credentialA, $"at-A-{canary}", $"rt-A-{canary}", $"at-A2-{canary}", $"rt-A2-{canary}", teamB, instanceB, credentialB, $"at-B-{canary}"); + } + + private async Task<(Guid TeamId, Guid InstanceId)> SeedConnectionAsync(string signInUrl, string apiUrl, string clientId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var suffix = Guid.NewGuid().ToString("N")[..8]; + var team = new Team { Id = Guid.NewGuid(), Slug = $"shared-app-{suffix}", Name = "Shared app" }; + var instance = new ProviderInstance + { + Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.GitHub, DisplayName = "GitHub", BaseUrl = signInUrl, ApiUrl = apiUrl, + OauthClientId = clientId, OauthClientSecretEnc = scope.Resolve().Encrypt(ClientSecret) + }; + + db.Team.Add(team); + db.ProviderInstance.Add(instance); + await db.SaveChangesAsync().ConfigureAwait(false); + + return (team.Id, instance.Id); + } + + private async Task SeedOAuthCredentialAsync(Guid teamId, Guid instanceId, string accessToken, string refreshToken, DateTimeOffset expiresAt) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var payload = scope.Resolve().Serialize(new OAuthPayload { AccessToken = accessToken, RefreshToken = refreshToken, ExpiresAt = expiresAt }); + var credential = new Credential + { + Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instanceId, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.OAuth, + DisplayName = "octocat", EncryptedPayload = scope.Resolve().Encrypt(payload), ExpiresDate = expiresAt, Status = CredentialStatus.Active + }; + + db.Credential.Add(credential); + await db.SaveChangesAsync().ConfigureAwait(false); + + return credential.Id; + } + + private async Task DeleteConnectionAsync(Guid teamId, Guid instanceId) + { + using var scope = _fixture.BeginScopeAs(Guid.NewGuid(), teamId, Roles.Admin); + + await scope.Resolve().Send(new DeleteProviderInstanceCommand { ProviderInstanceId = instanceId }).ConfigureAwait(false); + } + + private async Task DisconnectAsync(Guid teamId, Guid credentialId) + { + using var scope = _fixture.BeginScopeAs(Guid.NewGuid(), teamId, Roles.Admin); + + return await scope.Resolve().Send(new RevokeCredentialCommand { CredentialId = credentialId }).ConfigureAwait(false); + } + + private Task RetireAsync(SharedApp world, RetiringWrite retirement) => retirement switch + { + RetiringWrite.ConnectionDelete => DeleteConnectionAsync(world.TeamA, world.InstanceA), + RetiringWrite.PreviousBuildDeleteThenSweep => DeleteAsThePreviousBuildThenSweepAsync(world.InstanceA), + _ => throw new ArgumentOutOfRangeException(nameof(retirement), retirement, null) + }; + + /// + /// The previous build's ProviderInstanceService.DeleteAsync, statement for statement — cascade-revoke then soft + /// delete, every token kept, nothing asked of GitHub — replayed as raw SQL against this build's schema, then this + /// build's recurring sweep through the real mediator, which blanks the token that delete kept. + /// + private async Task DeleteAsThePreviousBuildThenSweepAsync(Guid instanceId) + { + await using (var connection = new NpgsqlConnection(_fixture.ConnectionString)) + { + await connection.OpenAsync().ConfigureAwait(false); + await using var command = new NpgsqlCommand( + """ + UPDATE credential SET status = 'Revoked', deleted_date = now(), last_error = 'Provider instance deleted' WHERE provider_instance_id = @deleted AND deleted_date IS NULL AND status = 'Active'; + UPDATE provider_instance SET deleted_date = now() WHERE id = @deleted; + """, connection); + command.Parameters.AddWithValue("deleted", instanceId); + await command.ExecuteNonQueryAsync().ConfigureAwait(false); + } + + using var scope = _fixture.BeginScope(); + await scope.Resolve().Send(new ShredRetiredSecretsCommand()).ConfigureAwait(false); + } + + /// A run resolving the credential's token: the real auth resolver → GitHub OAuth strategy → refresher chain, on the row as a run loads it. + private async Task ResolveTokenAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + var credential = await scope.Resolve().Credential.AsNoTracking().Include(c => c.ProviderInstance).SingleAsync(c => c.Id == credentialId).ConfigureAwait(false); + var auth = await scope.Resolve().ResolveAsync(new ProviderContext(credential.ProviderInstance, credential), CancellationToken.None).ConfigureAwait(false); + + return auth.Token; + } + + private async Task StoredAccessTokenAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + var stored = await scope.Resolve().Credential.AsNoTracking().Where(c => c.Id == credentialId).Select(c => c.EncryptedPayload).SingleAsync().ConfigureAwait(false); + var payload = scope.Resolve().Deserialize(AuthType.OAuth, scope.Resolve().Decrypt(stored)); + + return payload.ShouldBeOfType().AccessToken; + } + + private static string Seen(StubProviderHost gitHub) => string.Join(", ", gitHub.Requests.Select(r => $"{r.Method} {r.PathAndQuery}")); + + /// + /// A GitHub OAuth App's tokens as GitHub keeps them: each access token issued to one user, all of a user's tokens one + /// grant. DELETE /applications/{client_id}/grant ends every token of the user the presented token belongs to; + /// DELETE /applications/{client_id}/token ends the presented token alone. Both answer 204, 404 for a token + /// GitHub does not know, and 401 unless the request authenticates as the app. + /// + private sealed class GitHubGrants + { + private readonly ConcurrentDictionary _userByLiveToken = new(StringComparer.Ordinal); + private readonly string _basicAuth; + + private GitHubGrants(string clientId, string clientSecret) { _basicAuth = "Basic " + Convert.ToBase64String(Encoding.ASCII.GetBytes($"{clientId}:{clientSecret}")); } + + public static GitHubGrants Serve(StubProviderHost host, string clientId, string clientSecret) + { + var grants = new GitHubGrants(clientId, clientSecret); + + host.Answer("DELETE", $"/applications/{clientId}/grant", grants.DeleteGrant); + host.Answer("DELETE", $"/applications/{clientId}/token", grants.DeleteToken); + + return grants; + } + + public void Issue(string user, params string[] accessTokens) + { + foreach (var token in accessTokens) _userByLiveToken[token] = user; + } + + public bool IsLive(string accessToken) => _userByLiveToken.ContainsKey(accessToken); + + /// + /// GitHub's token endpoint answering a refresh: runs while GitHub holds the + /// request — a write that lands before its answer — then the pair GitHub issues is live and + /// returned. The sign-in host is not the API host, so that write can reach the API meanwhile. + /// + public void ServeRefresh(StubProviderHost signIn, string user, string accessToken, string refreshToken, Action whileRefreshing) + { + signIn.Answer("POST", "/login/oauth/access_token", _ => + { + whileRefreshing(); + Issue(user, accessToken); + + return new StubProviderHost.StubReply(200, JsonSerializer.Serialize(new { access_token = accessToken, refresh_token = refreshToken, expires_in = 28800, refresh_token_expires_in = 15897600, token_type = "bearer", scope = "" })); + }); + } + + private StubProviderHost.StubReply DeleteGrant(StubProviderHost.RecordedRequest request) + { + if (request.AuthorizationHeader != _basicAuth) return new StubProviderHost.StubReply(401, """{"message":"Requires authentication"}"""); + if (!_userByLiveToken.TryGetValue(AccessToken(request), out var user)) return new StubProviderHost.StubReply(404, """{"message":"Not Found"}"""); + + foreach (var token in _userByLiveToken.Where(entry => entry.Value == user).Select(entry => entry.Key).ToList()) _userByLiveToken.TryRemove(token, out _); + + return new StubProviderHost.StubReply(204, string.Empty); + } + + private StubProviderHost.StubReply DeleteToken(StubProviderHost.RecordedRequest request) + { + if (request.AuthorizationHeader != _basicAuth) return new StubProviderHost.StubReply(401, """{"message":"Requires authentication"}"""); + + return _userByLiveToken.TryRemove(AccessToken(request), out _) ? new StubProviderHost.StubReply(204, string.Empty) : new StubProviderHost.StubReply(404, """{"message":"Not Found"}"""); + } + + private static string AccessToken(StubProviderHost.RecordedRequest request) => JsonDocument.Parse(request.Body).RootElement.GetProperty("access_token").GetString()!; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/UpdateAndDeleteProviderInstanceFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/UpdateAndDeleteProviderInstanceFlowTests.cs index 1836c2170..5aeb21ee8 100644 --- a/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/UpdateAndDeleteProviderInstanceFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/ProviderInstances/UpdateAndDeleteProviderInstanceFlowTests.cs @@ -2,10 +2,14 @@ using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.Core.Services.Providers; +using CodeSpace.Core.Services.Providers.Capabilities; using CodeSpace.IntegrationTests.Infrastructure; using CodeSpace.Messages.Commands.ProviderInstances; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Dtos.Providers; using CodeSpace.Messages.Enums; using MediatR; using Microsoft.EntityFrameworkCore; @@ -186,25 +190,100 @@ public async Task Delete_soft_deletes_provider_when_no_dependencies() saved.DeletedDate.ShouldNotBeNull(); } + /// + /// Deleting a connection ends every token connected through it the way any other retirement does — blanked, Revoked, + /// soft-deleted, the act-as identities linked to it gone — and drops the connection's OAuth client secret, so none of + /// it stays decryptable in the database. Each OAuth grant is then revoked at the provider, but only once the delete + /// has committed: a provider asked from inside the transaction would have lost the grant even if the delete rolled + /// back. The recorder reads the credential through a connection of its own at the moment the provider is asked, which + /// is what anyone outside the delete's transaction would see. + /// [Fact] - public async Task Delete_cascade_revokes_active_credentials() + public async Task Delete_retires_its_credentials_and_client_secret_then_revokes_each_grant_at_the_provider_after_commit() { - // The cleanup motivation: a deleted provider's tokens are useless. If we left them - // Active in the DB, ListCredentials would still show them and the UI would lie. var (teamId, instanceId) = await SeedProviderAsync(withOAuth: true).ConfigureAwait(false); var credentialId = await SeedCredentialAsync(teamId, instanceId).ConfigureAwait(false); + var identityId = await SeedIdentityAsync(teamId, instanceId, credentialId).ConfigureAwait(false); + var provider = new RevocationRecorder(() => ReadCommittedPayloadAsync(credentialId)); - using var scope = _fixture.BeginScopeAs(Guid.NewGuid(), teamId, Roles.Admin); - var mediator = scope.Resolve(); + var result = await DeleteAsync(teamId, instanceId, provider).ConfigureAwait(false); - await mediator.Send(new DeleteProviderInstanceCommand { ProviderInstanceId = instanceId }).ConfigureAwait(false); + result.RevokedCredentialCount.ShouldBe(1); using var verify = _fixture.BeginScope(); var db = verify.Resolve(); var cred = await db.Credential.AsNoTracking().IgnoreQueryFilters().SingleAsync(c => c.Id == credentialId).ConfigureAwait(false); + var instance = await db.ProviderInstance.AsNoTracking().SingleAsync(p => p.Id == instanceId).ConfigureAwait(false); + var identity = await db.UserProviderIdentity.AsNoTracking().SingleAsync(i => i.Id == identityId).ConfigureAwait(false); cred.Status.ShouldBe(CredentialStatus.Revoked); cred.DeletedDate.ShouldNotBeNull(); + cred.EncryptedPayload.ShouldBe(string.Empty, "a deleted connection's tokens must not stay decryptable in the database"); + cred.LastError.ShouldBe("Provider instance deleted"); + instance.OauthClientSecretEnc.ShouldBeNull("the deleted connection's OAuth client secret must not stay decryptable either"); + instance.OauthClientId.ShouldBe("client-id", "the client id is no secret and stays for the audit trail"); + identity.DeletedDate.ShouldNotBeNull("an act-as identity is unusable without its credential and goes with it, as on every other retirement"); + + provider.Revokes.Select(r => (r.Input.Token, r.Input.TokenTypeHint)).ShouldBe(new[] { ("rt-CANARY", "refresh_token"), ("at-CANARY", "access_token") }); + provider.Revokes.ShouldAllBe(r => r.Input.ClientId == "client-id" && r.Input.ClientSecret == "original-secret"); + provider.Revokes.ShouldAllBe(r => !r.Input.WholeGrant, "a delete ends the tokens this connection held, never the user's whole grant: another team connected through the same OAuth App holds tokens under it too"); + provider.Revokes.ShouldAllBe(r => r.CommittedPayload == string.Empty, "the provider was asked while the delete was uncommitted — a rolled-back delete would leave Active credentials on a grant the provider already ended"); + } + + [Fact] + public async Task Delete_retires_its_credentials_even_when_the_provider_refuses_the_revoke() + { + var (teamId, instanceId) = await SeedProviderAsync(withOAuth: true).ConfigureAwait(false); + var credentialId = await SeedCredentialAsync(teamId, instanceId).ConfigureAwait(false); + var provider = new RevocationRecorder(() => ReadCommittedPayloadAsync(credentialId)) { Refuse = true }; + + var result = await DeleteAsync(teamId, instanceId, provider).ConfigureAwait(false); + + result.RevokedCredentialCount.ShouldBe(1, "the provider's answer is best effort; the delete stands without it"); + provider.Revokes.ShouldNotBeEmpty("fixture check: the provider was asked"); + (await ReadCommittedPayloadAsync(credentialId).ConfigureAwait(false)).ShouldBe(string.Empty); + } + + [Fact] + public async Task Delete_blanks_a_personal_access_token_without_asking_the_provider() + { + // A PAT has no revocation endpoint — only its owner deleting it on the provider's side ends it there. + var (teamId, instanceId) = await SeedProviderAsync(withOAuth: true).ConfigureAwait(false); + var credentialId = await SeedCredentialAsync(teamId, instanceId, new PatPayload { Token = "glpat-CANARY" }).ConfigureAwait(false); + var provider = new RevocationRecorder(() => ReadCommittedPayloadAsync(credentialId)); + + await DeleteAsync(teamId, instanceId, provider).ConfigureAwait(false); + + provider.Revokes.ShouldBeEmpty(); + (await ReadCommittedPayloadAsync(credentialId).ConfigureAwait(false)).ShouldBe(string.Empty); + } + + /// + /// A connection-wide connection registered its group hooks at the provider with one of its own credentials, and the + /// delete blanks every one of those. So the hooks come down first, while that token still opens: afterwards nothing + /// could remove them, and each would go on delivering the group's events with its HMAC secret still decryptable here. + /// A provider that refuses the delete changes nothing here — the row goes either way, as on a scope switch — and the + /// hook rows the connection keeps are out of service and hold no secret. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Delete_takes_its_group_hooks_down_with_a_live_token_and_keeps_no_hook_secret(bool providerRefuses) + { + var (teamId, instanceId) = await SeedProviderAsync(withOAuth: true, webhookScope: ProviderWebhookScope.Connection).ConfigureAwait(false); + var credentialId = await SeedCredentialAsync(teamId, instanceId, new PatPayload { Token = "ghp-HOOK-CANARY" }).ConfigureAwait(false); + await SeedConnectionHookAsync(instanceId, credentialId, "acme", RepositoryWebhookRegistrationStatus.Registered, "hook-777").ConfigureAwait(false); + var retiredEarlier = await SeedConnectionHookAsync(instanceId, credentialId, "acme/old", RepositoryWebhookRegistrationStatus.Cancelled, externalId: null).ConfigureAwait(false); + var groupHooks = new GroupHookRecorder { Refuse = providerRefuses }; + + await DeleteAsync(teamId, instanceId, new RevocationRecorder(() => ReadCommittedPayloadAsync(credentialId)), groupHooks: groupHooks).ConfigureAwait(false); + + groupHooks.Deletes.Select(d => (d.OwnerPath, d.ExternalId, ReadPatToken(d.Payload))).ShouldBe(new[] { ("acme", "hook-777", "ghp-HOOK-CANARY") }, ignoreOrder: false, customMessage: "the group hook must be deleted at the provider with the token that registered it, before the delete blanks that token — check the hook retirement in ProviderInstanceService.DeleteAsync"); + + var hooks = await LoadConnectionHooksAsync(instanceId).ConfigureAwait(false); + + hooks.Select(h => h.Id).ShouldBe(new[] { retiredEarlier }, "the Registered row described a hook that is gone (or that the operator now removes by hand); keeping it would claim coverage for a deleted connection"); + hooks.ShouldAllBe(h => h.RegistrationStatus == RepositoryWebhookRegistrationStatus.Cancelled && h.SecretEnc == string.Empty, "a deleted connection keeps no hook in service and no hook secret decryptable"); } [Fact] @@ -235,13 +314,7 @@ public async Task Delete_with_force_cascades_unbind_then_removes_provider() var repoIdA = await SeedRepositoryAsync(teamId, instanceId, credentialId, name: "api").ConfigureAwait(false); var repoIdB = await SeedRepositoryAsync(teamId, instanceId, credentialId, name: "web").ConfigureAwait(false); - DeleteProviderInstanceResult result; - - using (var scope = _fixture.BeginScopeAs(Guid.NewGuid(), teamId, Roles.Admin)) - { - var mediator = scope.Resolve(); - result = await mediator.Send(new DeleteProviderInstanceCommand { ProviderInstanceId = instanceId, Force = true }).ConfigureAwait(false); - } + var result = await DeleteAsync(teamId, instanceId, new RevocationRecorder(() => ReadCommittedPayloadAsync(credentialId)), force: true).ConfigureAwait(false); result.UnboundRepositoryCount.ShouldBe(2); result.RevokedCredentialCount.ShouldBe(1); @@ -258,10 +331,11 @@ public async Task Delete_with_force_cascades_unbind_then_removes_provider() repoRows.Count.ShouldBe(2); repoRows.ShouldAllBe(r => r.DeletedDate != null); - // Credential: revoked. + // Credential: revoked, and its token gone. var credRow = await db.Credential.AsNoTracking().IgnoreQueryFilters().SingleAsync(c => c.Id == credentialId).ConfigureAwait(false); credRow.Status.ShouldBe(CredentialStatus.Revoked); credRow.DeletedDate.ShouldNotBeNull(); + credRow.EncryptedPayload.ShouldBe(string.Empty); } [Fact] @@ -301,7 +375,7 @@ private async Task SeedTeamAsync() return team.Id; } - private async Task<(Guid TeamId, Guid InstanceId)> SeedProviderAsync(bool withOAuth, string? baseUrl = null, Guid? existingTeamId = null) + private async Task<(Guid TeamId, Guid InstanceId)> SeedProviderAsync(bool withOAuth, string? baseUrl = null, Guid? existingTeamId = null, ProviderWebhookScope webhookScope = ProviderWebhookScope.Repository) { var teamId = existingTeamId ?? await SeedTeamAsync().ConfigureAwait(false); @@ -317,7 +391,8 @@ private async Task SeedTeamAsync() DisplayName = "GitHub", BaseUrl = baseUrl ?? "https://github.com", OauthClientId = withOAuth ? "client-id" : null, - OauthClientSecretEnc = withOAuth ? encryptor.Encrypt("original-secret") : null + OauthClientSecretEnc = withOAuth ? encryptor.Encrypt("original-secret") : null, + WebhookScope = webhookScope }; db.ProviderInstance.Add(instance); @@ -326,21 +401,21 @@ private async Task SeedTeamAsync() return (teamId, instance.Id); } - private async Task SeedCredentialAsync(Guid teamId, Guid instanceId) + private async Task SeedCredentialAsync(Guid teamId, Guid instanceId, CredentialPayload? token = null) { using var scope = _fixture.BeginScope(); var db = scope.Resolve(); var encryptor = scope.Resolve(); var serializer = scope.Resolve(); - var payload = new OAuthPayload { AccessToken = "at", RefreshToken = "rt" }; + var payload = token ?? new OAuthPayload { AccessToken = "at-CANARY", RefreshToken = "rt-CANARY" }; var credential = new Credential { Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instanceId, - AuthType = AuthType.OAuth, - DisplayName = "OAuth", + AuthType = payload.Type, + DisplayName = payload.Type.ToString(), EncryptedPayload = encryptor.Encrypt(serializer.Serialize(payload)), Status = CredentialStatus.Active }; @@ -351,6 +426,148 @@ private async Task SeedCredentialAsync(Guid teamId, Guid instanceId) return credential.Id; } + /// The team owner's act-as link to the credential, as connecting through OAuth leaves one. + private async Task SeedIdentityAsync(Guid teamId, Guid instanceId, Guid credentialId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var ownerId = await db.TeamMembership.AsNoTracking().Where(m => m.TeamId == teamId).Select(m => m.UserId).SingleAsync().ConfigureAwait(false); + var identity = new UserProviderIdentity { Id = Guid.NewGuid(), UserId = ownerId, ProviderInstanceId = instanceId, CredentialId = credentialId, ProviderUserId = "42", ProviderUsername = "owner" }; + + db.UserProviderIdentity.Add(identity); + await db.SaveChangesAsync().ConfigureAwait(false); + + return identity.Id; + } + + /// + /// The delete as an Admin sends it, with the provider's revocation endpoint replaced by and, + /// when given, its group-hook endpoint by . + /// + private async Task DeleteAsync(Guid teamId, Guid instanceId, RevocationRecorder provider, bool force = false, GroupHookRecorder? groupHooks = null) + { + using var scope = _fixture.BeginScopeAs(Guid.NewGuid(), teamId, Roles.Admin); + using var delete = scope.BeginLifetimeScope(b => + { + b.RegisterInstance(provider).SingleInstance(); + + if (groupHooks != null) b.RegisterInstance(new ProviderRegistry(new IProviderCapability[] { groupHooks })).SingleInstance(); + }); + + return await delete.Resolve().Send(new DeleteProviderInstanceCommand { ProviderInstanceId = instanceId, Force = force }).ConfigureAwait(false); + } + + /// The credential's payload as a connection outside any open transaction reads it — the committed state. + private async Task ReadCommittedPayloadAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().Credential.AsNoTracking().Where(c => c.Id == credentialId).Select(c => c.EncryptedPayload).SingleAsync().ConfigureAwait(false); + } + + /// + /// Stands in for the provider's revocation endpoint: records each revoke with the credential's committed payload at + /// that moment, and refuses every one when told to, as a provider that is down or rejects the client would. + /// + private sealed class RevocationRecorder : IOAuthClient, IOAuthClientRegistry + { + private readonly Func> _readCommittedPayload; + + public RevocationRecorder(Func> readCommittedPayload) { _readCommittedPayload = readCommittedPayload; } + + public List<(OAuthRevokeInput Input, string CommittedPayload)> Revokes { get; } = new(); + + public bool Refuse { get; init; } + + public ProviderKind Kind => ProviderKind.GitHub; + + public IOAuthClient Get(ProviderKind kind) => this; + + public Uri BuildAuthorizeUrl(OAuthAuthorizeInput input) => throw new NotSupportedException(); + + public Task ExchangeCodeAsync(OAuthCodeExchangeInput input, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task RefreshAsync(OAuthRefreshInput input, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public async Task RevokeAsync(OAuthRevokeInput input, CancellationToken cancellationToken) + { + Revokes.Add((input, await _readCommittedPayload().ConfigureAwait(false))); + + if (Refuse) throw new HttpRequestException("stub provider: revocation refused"); + } + } + + /// A group hook of the connection, registered by , in . + private async Task SeedConnectionHookAsync(Guid instanceId, Guid credentialId, string ownerPath, RepositoryWebhookRegistrationStatus status, string? externalId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var id = Guid.NewGuid(); + + db.ConnectionWebhook.Add(new ConnectionWebhook + { + Id = id, + ProviderInstanceId = instanceId, + CredentialId = credentialId, + OwnerPath = ownerPath, + ExternalId = externalId, + CallbackUrl = $"https://codespace.test/api/webhooks/connection/{id}", + SecretEnc = scope.Resolve().Encrypt($"hook-secret-CANARY-{id:N}"), + SubscribedEvents = new List { "push" }, + RegistrationStatus = status + }); + + await db.SaveChangesAsync().ConfigureAwait(false); + + return id; + } + + private async Task> LoadConnectionHooksAsync(Guid instanceId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().ConnectionWebhook.AsNoTracking().Where(w => w.ProviderInstanceId == instanceId).ToListAsync().ConfigureAwait(false); + } + + /// The personal access token a sealed payload holds, or empty when the payload has been blanked. + private string ReadPatToken(string encryptedPayload) + { + if (string.IsNullOrEmpty(encryptedPayload)) return string.Empty; + + using var scope = _fixture.BeginScope(); + var json = scope.Resolve().Decrypt(encryptedPayload); + + return scope.Resolve().Deserialize(AuthType.Pat, json).Token; + } + + /// + /// Stands in for the provider's group-hook endpoint: records each delete with the sealed payload of the credential + /// the call presents at that moment, and refuses every one when told to, as a provider that is down would. + /// + /// Parameterless and inert on purpose: the fixture's capability scan activates every capability class in this + /// assembly into the container's registry, where the production GitHub provider, registered first, answers before + /// it. Only the delete test's own registry reaches it. + /// + private sealed class GroupHookRecorder : IConnectionWebhookRegistrationCapability + { + public List<(string OwnerPath, string ExternalId, string Payload)> Deletes { get; } = new(); + + public bool Refuse { get; init; } + + public ProviderKind Kind => ProviderKind.GitHub; + + public Task FindConnectionWebhookByCallbackUrlAsync(ProviderContext context, string ownerPath, string callbackUrl, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task RegisterConnectionWebhookAsync(ProviderContext context, string ownerPath, WebhookRegistration request, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task DeleteConnectionWebhookAsync(ProviderContext context, string ownerPath, string externalWebhookId, CancellationToken cancellationToken) + { + Deletes.Add((ownerPath, externalWebhookId, context.Credential.EncryptedPayload)); + + return Refuse ? throw new HttpRequestException("stub provider: group hook delete refused") : Task.CompletedTask; + } + } + private async Task SeedRepositoryAsync(Guid teamId, Guid instanceId, Guid credentialId, string name = "api") { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.IntegrationTests/Variables/VariableShredFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Variables/VariableShredFlowTests.cs new file mode 100644 index 000000000..2bf96daf4 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Variables/VariableShredFlowTests.cs @@ -0,0 +1,95 @@ +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.Variables; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Commands.Projects; +using CodeSpace.Messages.Commands.Variables; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Variables; + +/// +/// A deleted variable keeps its row for the audit trail and nothing a secret could be recovered from. Every read of a +/// variable filters on deleted_date IS NULL and nothing restores one, so a deleted Secret's ciphertext could only +/// ever be opened by someone holding a copy of the database and the master key. Both ways a variable is deleted — on its +/// own, and with the project it belongs to — drop it, and 0249's check admits a NULL ciphertext on a deleted Secret only. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class VariableShredFlowTests +{ + private readonly PostgresFixture _fixture; + + public VariableShredFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Theory] + [InlineData(VariableValueType.Secret, "\"CANARY-variable-secret\"")] + [InlineData(VariableValueType.String, "\"plain-kept\"")] + public async Task Deleting_a_variable_drops_a_secrets_ciphertext_and_the_name_is_free_again(VariableValueType valueType, string value) + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + using var scope = _fixture.BeginScope(); + var variables = scope.Resolve(); + + await variables.SetAsync(VariableScope.Team, teamId, teamId, "DEPLOY_TOKEN", valueType, Json(value), null, userId, CancellationToken.None); + await variables.DeleteAsync(VariableScope.Team, teamId, teamId, "DEPLOY_TOKEN", userId, CancellationToken.None); + + var deleted = await DeletedRowAsync(VariableScope.Team, teamId, "DEPLOY_TOKEN"); + deleted.ValueEncrypted.ShouldBeNull("a deleted variable must hold no ciphertext anyone could open later"); + deleted.ValuePlain.ShouldBe(valueType == VariableValueType.Secret ? null : value, "a plain value is no secret and stays as the record of what the variable held"); + + await variables.SetAsync(VariableScope.Team, teamId, teamId, "DEPLOY_TOKEN", valueType, Json(value), null, userId, CancellationToken.None); + (await variables.GetAllForEngineAsync(VariableScope.Team, teamId, CancellationToken.None)).Single(v => v.Name == "DEPLOY_TOKEN").Value.GetRawText().ShouldBe(value, "the name is free again and the new row reads back"); + } + + [Fact] + public async Task Deleting_a_project_drops_the_ciphertext_of_its_secret_variables() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + Guid projectId; + + using (var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin)) + { + var mediator = scope.Resolve(); + projectId = await mediator.Send(new CreateProjectCommand { Name = $"Shred {Guid.NewGuid():N}" }); + await mediator.Send(new SetProjectVariableCommand { ProjectId = projectId, Name = "API_KEY", ValueType = VariableValueType.Secret, Value = Json("\"CANARY-project-secret\"") }); + } + + using (var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin)) + await scope.Resolve().Send(new DeleteProjectCommand { ProjectId = projectId }); + + (await DeletedRowAsync(VariableScope.Project, projectId, "API_KEY")).ValueEncrypted.ShouldBeNull("the project cascade retires its variables the same way a delete does"); + } + + [Fact] + public async Task A_live_secret_still_must_carry_its_ciphertext() + { + // 0249 relaxed the value check for deleted rows only. A live Secret with no ciphertext is still a programmer + // error the database refuses — otherwise the engine would read NULL where a secret should be. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + using (var scope = _fixture.BeginScope()) + await scope.Resolve().SetAsync(VariableScope.Team, teamId, teamId, "LIVE_SECRET", VariableValueType.Secret, Json("\"CANARY-live\""), null, userId, CancellationToken.None); + + using var verify = _fixture.BeginScope(); + var db = verify.Resolve(); + var refused = await Should.ThrowAsync(() => db.Database.ExecuteSqlInterpolatedAsync($"UPDATE variable SET value_encrypted = NULL WHERE team_id = {teamId} AND name = 'LIVE_SECRET' AND deleted_date IS NULL")); + + refused.ConstraintName.ShouldBe("chk_variable_value_exclusive"); + } + + private async Task DeletedRowAsync(VariableScope scope, Guid scopeId, string name) + { + using var verify = _fixture.BeginScope(); + + return await verify.Resolve().Variable.AsNoTracking().SingleAsync(v => v.Scope == scope && v.ScopeId == scopeId && v.Name == name && v.DeletedDate != null); + } + + private static JsonElement Json(string raw) => JsonDocument.Parse(raw).RootElement.Clone(); +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Artifacts/Credentials/StorageCredentialShredTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Artifacts/Credentials/StorageCredentialShredTests.cs new file mode 100644 index 000000000..d7c9aa4dd --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Artifacts/Credentials/StorageCredentialShredTests.cs @@ -0,0 +1,153 @@ +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Workflows.Artifacts.Credentials; +using CodeSpace.Core.Services.Workflows.Artifacts.Providers.Local; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Commands.Storage; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Workflows.Artifacts.Credentials; + +/// +/// Revoking a storage credential blanks the encrypted payload of every one of its revisions, and only a revoked +/// credential's revisions may ever be blanked. +/// +/// Why revoked and not merely superseded: the secret resolver reads the exact revision a storage profile pins +/// (db:{id}:{revision}), so while a credential is Active every revision of it is a live key — a profile revision +/// that still holds stored bytes resolves through it, and a new profile revision may pin it +/// (StorageCredentialSecretResolverTests.Exact_old_revision_never_falls_forward_to_the_current_secret). Blanking +/// one would make routed bytes permanently unreadable. A Revoked credential resolves for nobody: the resolver answers +/// NotActive before it reads any envelope, revocation is terminal, and it is refused while a stored location or a +/// live profile still resolves through the credential. So its ciphertext is dead weight, recoverable only by someone +/// holding a database copy and the key ring. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class StorageCredentialShredTests +{ + private readonly PostgresFixture _fixture; + + public StorageCredentialShredTests(PostgresFixture fixture) => _fixture = fixture; + + [Fact] + public async Task Revoking_blanks_every_revisions_payload_and_keeps_what_the_audit_trail_reads() + { + var world = await SeedRotatedCredentialAsync(); + var before = await RevisionsAsync(world); + + await RevokeAsync(world); + + var after = await RevisionsAsync(world); + after.ShouldAllBe(r => r.EncryptedPayload == string.Empty, "a revoked credential's revisions must not stay decryptable"); + after.Select(Audit).ShouldBe(before.Select(Audit), "revision, provider, hint, envelope fingerprint and authorship stay for the audit trail"); + + using var scope = _fixture.BeginScope(); + var resolver = scope.Resolve(); + foreach (var revision in new[] { 1, 2 }) + (await resolver.ResolveAsync(new StorageCredentialSecretRequest(world.TeamId, world.CredentialId, revision, LocalRwxArtifactStorageDriverFactory.TypeKey), CancellationToken.None)) + .ShouldBe(new StorageCredentialSecretResolution.NotActive(StorageCredentialState.Revoked), "a revoked credential resolves for nobody, so nothing reads the blanked envelope"); + } + + [Fact] + public async Task A_superseded_revision_of_an_active_credential_keeps_its_payload() + { + var world = await SeedRotatedCredentialAsync(); + + var refused = await Should.ThrowAsync(() => ExecuteAsync($"UPDATE storage_credential_revision SET encrypted_payload = '' WHERE storage_credential_id = '{world.CredentialId}' AND revision = 1")); + + refused.MessageText.ShouldContain("Revoked", Case.Sensitive, "a profile may still pin revision 1 of an Active credential; blanking it would strand the bytes stored through it"); + (await RevisionsAsync(world)).ShouldAllBe(r => r.EncryptedPayload != string.Empty); + } + + [Theory] + [InlineData("UPDATE storage_credential_revision SET envelope_fingerprint = 'sha256:" + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "' WHERE storage_credential_id = '{0}'")] + [InlineData("UPDATE storage_credential_revision SET encrypted_payload = 'CfDJ8-restored' WHERE storage_credential_id = '{0}'")] + [InlineData("UPDATE storage_credential_revision SET encrypted_payload = '' WHERE storage_credential_id = '{0}'")] + [InlineData("DELETE FROM storage_credential_revision WHERE storage_credential_id = '{0}'")] + public async Task On_a_revoked_credential_the_one_blanking_update_is_all_the_guard_ever_admits(string statement) + { + var world = await SeedRotatedCredentialAsync(); + await RevokeAsync(world); + + var refused = await Should.ThrowAsync(() => ExecuteAsync(string.Format(statement, world.CredentialId))); + + refused.MessageText.ShouldContain("immutable", Case.Sensitive, "a blanked revision is final — no second blanking, no restore, no other edit, no delete"); + } + + [Fact] + public async Task A_revision_never_arrives_blank() + { + var world = await SeedRotatedCredentialAsync(); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var credential = await db.StorageCredential.SingleAsync(c => c.Id == world.CredentialId); + db.StorageCredentialRevision.Add(Revision(world, 3, string.Empty)); + credential.CurrentRevision = 3; + + var refused = await Should.ThrowAsync(() => db.SaveChangesAsync()); + + refused.InnerException.ShouldBeOfType().MessageText.ShouldContain("payload is required", Case.Sensitive); + } + + private static object Audit(StorageCredentialRevision r) => new { r.Id, r.Revision, r.ProviderTypeKey, r.SafeHint, r.EnvelopeFingerprint, r.CreatedDate, r.CreatedBy }; + + private async Task RevokeAsync(World world) + { + using var scope = _fixture.BeginScope(); + var credential = await scope.Resolve().StorageCredential.AsNoTracking().SingleAsync(c => c.Id == world.CredentialId); + + await scope.Resolve().RevokeAsync(world.TeamId, world.ActorId, new RevokeStorageCredentialCommand { CredentialId = world.CredentialId, ExpectedXmin = credential.Xmin, ExpectedCurrentRevision = credential.CurrentRevision }, CancellationToken.None); + } + + private async Task> RevisionsAsync(World world) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().StorageCredentialRevision.AsNoTracking().Where(r => r.StorageCredentialId == world.CredentialId).OrderBy(r => r.Revision).ToListAsync(); + } + + private async Task ExecuteAsync(string sql) + { + await using var connection = new NpgsqlConnection(_fixture.ConnectionString); + await connection.OpenAsync(); + await using var command = new NpgsqlCommand(sql, connection); + await command.ExecuteNonQueryAsync(); + } + + /// An Active credential rotated once — revision 1 superseded, revision 2 current — that no profile references. + private async Task SeedRotatedCredentialAsync() + { + var world = new World(Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid()); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + db.User.Add(new User { Id = world.ActorId, Email = $"shred-{world.ActorId:N}@test.local", Name = $"shred-{world.ActorId:N}" }); + db.Team.Add(new Team { Id = world.TeamId, Slug = $"shred-{world.TeamId:N}", Name = "Storage Shred Team", Kind = TeamKind.Workspace }); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = world.TeamId, UserId = world.ActorId, Role = TeamRole.Owner }); + + var credential = new StorageCredential { Id = world.CredentialId, TeamId = world.TeamId, StableName = $"shred-{Guid.NewGuid():N}", CurrentRevision = 1, State = StorageCredentialState.Active, CreatedDate = DateTimeOffset.UtcNow, CreatedBy = world.ActorId }; + credential.Revisions.Add(Revision(world, 1, "CfDJ8-CANARY-storage-revision-1")); + db.StorageCredential.Add(credential); + await db.SaveChangesAsync(); + + db.StorageCredentialRevision.Add(Revision(world, 2, "CfDJ8-CANARY-storage-revision-2")); + credential.CurrentRevision = 2; + await db.SaveChangesAsync(); + + return world; + } + + private static StorageCredentialRevision Revision(World world, int revision, string encryptedPayload) => new() + { + Id = Guid.NewGuid(), TeamId = world.TeamId, StorageCredentialId = world.CredentialId, Revision = revision, + ProviderTypeKey = LocalRwxArtifactStorageDriverFactory.TypeKey, EncryptedPayload = encryptedPayload, SafeHint = $"rev-{revision}", + EnvelopeFingerprint = $"sha256:{new string((char)('a' + revision), 64)}", CreatedDate = DateTimeOffset.UtcNow, CreatedBy = world.ActorId, + }; + + private sealed record World(Guid TeamId, Guid ActorId, Guid CredentialId); +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Artifacts/Defaults/StorageDefaultMaterializerFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Artifacts/Defaults/StorageDefaultMaterializerFlowTests.cs index 4d7c535ac..87068e9f3 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Artifacts/Defaults/StorageDefaultMaterializerFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Artifacts/Defaults/StorageDefaultMaterializerFlowTests.cs @@ -226,8 +226,66 @@ public async Task An_unknown_team_is_refused_by_name_rather_than_read_as_a_missi outcome.ShouldBeOfType("authoring a template and fixing the caller are opposite responses"); } + /// + /// Rotating the template's secret appends a new envelope and blanks the one it replaced; clearing it blanks the one it + /// named. The materializer decrypts only the envelope its template points at, so neither old one has a reader left, + /// and a deployment-wide cloud key kept decryptable forever is what rotating it was meant to end. A team materialized + /// after the rotation is the proof the blanking took nothing a reader needs. + /// + [Fact] + public async Task Rotating_or_clearing_the_templates_secret_blanks_the_envelope_it_replaced() + { + await AuthorTemplateAsync(NewRoot()); + var replaced = await SetTemplateSecretAsync(clear: false); + var current = await SetTemplateSecretAsync(clear: false); + + current.ShouldNotBe(replaced, "fixture check: a supplied secret appends a new envelope"); + (await EnvelopeAsync(replaced!.Value)).ShouldBe(string.Empty, "the rotated-out envelope has no reader left and must not stay decryptable"); + (await EnvelopeAsync(current!.Value)).ShouldNotBeNullOrEmpty("the envelope the template names is the one the materializer decrypts"); + + (await MaterializeAsync(await SeedTeamAsync(), automatic: true)).ShouldBeOfType("a team materialized after a rotation reads the new envelope; a reader of the blanked one would halt as DestinationUnusable"); + + (await SetTemplateSecretAsync(clear: true)).ShouldBeNull("fixture check: the credential was cleared"); + (await EnvelopeAsync(current.Value)).ShouldBe(string.Empty, "a cleared envelope has no reader left either"); + } + // ─── World + helpers ───────────────────────────────────────────────────── + /// + /// Supplies a new secret to the shared template through its own service — or clears the one it has — keeping the rest + /// of it as it is, and returns the envelope it names afterwards. The local provider takes an empty secret object. + /// + private async Task SetTemplateSecretAsync(bool clear) + { + using var scope = _fixture.BeginScope(); + var service = scope.Resolve(); + var actorId = await SeedOperatorAsync(scope); + var existing = (await service.ListAsync(CancellationToken.None)).Single(row => row.DataClassTypeKey == AgentRunLogClass); + var template = (await service.GetAsync(existing.Id, CancellationToken.None))!; + + await service.UpdateAsync(actorId, new UpdateStorageDefaultCommand + { + DefaultId = template.Id, + ExpectedXmin = template.Xmin, + ExpectedRevision = template.Revision, + ProviderTypeKey = template.ProviderTypeKey, + NonSecretConfig = template.NonSecretConfig, + NamespaceRoot = template.NamespaceRoot, + AdoptionPolicy = template.AdoptionPolicy, + Secret = clear ? null : JsonDocument.Parse("{}").RootElement, + ClearCredential = clear, + }, CancellationToken.None); + + return await scope.Resolve().StorageDefault.AsNoTracking().Where(row => row.Id == template.Id).Select(row => row.CredentialId).SingleAsync(); + } + + private async Task EnvelopeAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().StorageDefaultCredential.AsNoTracking().Where(row => row.Id == credentialId).Select(row => row.EncryptedPayload).SingleAsync(); + } + private async Task MaterializeAsync(World world, bool automatic, string? dataClass = null) { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs index eeb5b3dce..e2795d7a7 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/SweepCommandTransactionFlowTests.cs @@ -140,6 +140,7 @@ public async Task Budget_settlement_releases_the_terminal_map_branch_reservation [InlineData(typeof(RepairDisconnectedCredentialBindingsCommand))] [InlineData(typeof(ResumeAbandonedArtifactTransfersCommand))] [InlineData(typeof(RetireDepartedMembersCredentialsCommand))] + [InlineData(typeof(ShredRetiredSecretsCommand))] [InlineData(typeof(SweepBudgetSettlementCommand))] [InlineData(typeof(SweepCompletionShadowCommand))] [InlineData(typeof(TierStaleModelCapabilitiesCommand))] diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/CredentialSpendGateTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/CredentialSpendGateTests.cs new file mode 100644 index 000000000..6e29a8b2f --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Architecture/CredentialSpendGateTests.cs @@ -0,0 +1,51 @@ +using System.IO; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Authorization; +using CodeSpace.Messages.Queries.Credentials; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Shouldly; + +namespace CodeSpace.UnitTests.Architecture; + +/// +/// Requests that present one credential to its provider on the caller's behalf. The credential marker alone proves the +/// credential is the caller's team's and the caller a member, and no more — so on membership alone a Viewer could spend +/// another member's personal token to enumerate everything that person's account can see at the provider, private +/// repositories outside the team included. Listing what a credential reaches is now its owner's (a Personal credential) +/// or a repos.manage holder's, the bar of the bind the listing feeds. +/// +[Trait("Category", "Unit")] +public sealed class CredentialSpendGateTests +{ + [Fact] + public void Listing_what_a_credential_reaches_takes_the_spend_gate() + { + typeof(IRequireCredentialSpend).IsAssignableFrom(typeof(ListAccessibleRepositoriesQuery)).ShouldBeTrue("listing what a credential reaches presents it to the provider — gate it on whose it is, not on membership alone"); + } + + [Fact] + public void The_spend_gate_runs_on_a_credential_already_proven_to_be_the_callers_teams() + { + typeof(IRequireCredentialAccess).IsAssignableFrom(typeof(IRequireCredentialSpend)).ShouldBeTrue("the spend tier decides only whether membership is enough; the team match must already hold"); + } + + /// + /// DbUp owns the schema; the EF model only describes it. 0249 relaxed each payload check so a revoked credential's + /// revision, and a deployment-default envelope no template names, can hold an empty payload, and the model's copy of + /// each check must say the same thing. + /// + [Theory] + [InlineData(typeof(StorageCredentialRevision), "ck_storage_credential_revision_encrypted_payload")] + [InlineData(typeof(StorageDefaultCredential), "ck_storage_default_credential_encrypted_payload")] + public void The_models_payload_check_matches_the_one_0249_installed(Type entity, string constraint) + { + using var db = new CodeSpaceDbContext(new DbContextOptionsBuilder().UseNpgsql("Host=127.0.0.1;Port=1;Database=unused;Username=unused;Password=unused").UseSnakeCaseNamingConvention().Options); + var check = db.GetService().Model.FindEntityType(entity)!.GetCheckConstraints().Single(c => c.Name == constraint); + var migration = File.ReadAllText(Path.Combine(AppContext.BaseDirectory, "Persistence", "DbUpFiles", "0249_shred_retired_secrets.sql")); + + migration.ShouldContain($"ADD CONSTRAINT {constraint} CHECK ({check.Sql})", Case.Sensitive); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs index c32a9fbd1..a7c09b8bc 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/RequestAuthorizationInventoryTests.cs @@ -79,6 +79,7 @@ public class RequestAuthorizationInventoryTests ["BackfillPackCloneUrlsCommand"] = "seals the clone URL of pack rows that still hold a pasted token in plaintext, across every team, in bounded batches; it changes where a pack's existing credential is kept, never which source a pack clones or what any user can reach, and a sealed row is no longer a candidate.", ["RepairDisconnectedCredentialBindingsCommand"] = "moves repositories and group hooks still bound to a disconnected credential onto the credential that credential's own owner connected afterwards on the same provider instance and team, across every team, in bounded batches; it never moves a binding off a live credential or onto another person's, so it only completes a reconnect the owner already made, and a moved binding is no longer a candidate.", ["RetireDepartedMembersCredentialsCommand"] = "retires the personal tokens whose owner — or, for one stored without an owner, whose creator account — has no standing in the team the token was lent to (no membership there and not an instance Admin), across every team, in bounded batches; it ends access the person no longer has standing to lend, exactly as their departure does, never touches a team-service token, the token of anyone who still stands in its team, or one it cannot attribute to an account, and a retired token is no longer a candidate.", + ["ShredRetiredSecretsCommand"] = "blanks the ciphertext of secrets that are already retired — deleted variables, revoked or soft-deleted git tokens, deleted connections' client and hook secrets, revoked storage credentials' revisions, deployment-default envelopes no template names — across every team; it never touches a live secret, ends no access anyone still has, and a blanked row is no longer a candidate, so it only finishes a retirement an older build left decryptable.", ["BackfillRunScorecardsCommand"] = "projects the observation-only north-star row for terminal runs that terminalized before the table existed, across every team, in bounded batches; a run that already has a row is not a candidate, so it can only ever add a measurement of a settled run — it authors nothing a user owns and changes no run's outcome.", ["SweepStaleAgentWorkspacesCommand"] = "sweep", ["TierStaleModelCapabilitiesCommand"] = "sweep", diff --git a/backend/tests/CodeSpace.UnitTests/Jobs/RetiredSecretShredDispatchTests.cs b/backend/tests/CodeSpace.UnitTests/Jobs/RetiredSecretShredDispatchTests.cs new file mode 100644 index 000000000..5b1861342 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Jobs/RetiredSecretShredDispatchTests.cs @@ -0,0 +1,93 @@ +using CodeSpace.Core.Handlers.CommandHandlers.Credentials; +using CodeSpace.Core.Jobs.RecurringJobs; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Mediation; +using MediatR; +using Shouldly; + +namespace CodeSpace.UnitTests.Jobs; + +/// +/// 🟢 Unit: shredding the secrets an older build retired without blanking them is the standard job → command → service +/// chain (Rule 14 + Rule 16) — the job holds no query, the handler holds no logic. Hand-rolled recording doubles, matching +/// the codebase convention. +/// +[Trait("Category", "Unit")] +public class RetiredSecretShredDispatchTests +{ + [Fact] + public async Task The_shred_job_dispatches_its_command_every_hour_and_holds_no_logic() + { + var mediator = new RecordingMediator(); + var job = new RetiredSecretShredRecurringJob(mediator); + + job.JobId.ShouldBe(nameof(RetiredSecretShredRecurringJob), "Hangfire indexes the schedule by this id — a rename strands the old one"); + job.CronExpression.ShouldBe("0 * * * *", "a committed cadence: a secret an older pod retires during a rollout stays decryptable at most an hour after the new build takes over"); + + await job.Execute(); + + mediator.Sent.ShouldHaveSingleItem().ShouldBeOfType("the job is a thin dispatcher — it only sends the command"); + } + + [Fact] + public async Task The_shred_handler_forwards_to_the_service_and_returns_its_count() + { + var shred = new RecordingShred { ToReturn = 4 }; + var handler = new ShredRetiredSecretsCommandHandler(shred); + + var shredded = await handler.Handle(new ShredRetiredSecretsCommand(), CancellationToken.None); + + shred.Calls.ShouldBe(1, "the handler delegates the whole pass to the service (Rule 16)"); + shredded.ShouldBe(4, "the handler surfaces the service's count verbatim"); + } + + [Fact] + public void The_shred_command_settles_each_kind_of_secret_on_its_own() + { + typeof(INonTransactionalCommand).IsAssignableFrom(typeof(ShredRetiredSecretsCommand)) + .ShouldBeTrue("each kind is blanked by one statement that commits on its own; one transaction around the pass would hold every candidate row's lock until the last statement and let one failing statement undo the rest"); + } + + /// Records the requests sent through the mediator; the rest of the surface is unreachable in these tests. + private sealed class RecordingMediator : IMediator + { + public List Sent { get; } = new(); + + public Task Send(IRequest request, CancellationToken cancellationToken = default) + { + Sent.Add(request); + return Task.FromResult(default(TResponse)!); + } + + public Task Send(object request, CancellationToken cancellationToken = default) + { + Sent.Add(request); + return Task.FromResult(null); + } + + public Task Send(TRequest request, CancellationToken cancellationToken = default) where TRequest : IRequest + { + Sent.Add(request!); + return Task.CompletedTask; + } + + public IAsyncEnumerable CreateStream(IStreamRequest request, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public IAsyncEnumerable CreateStream(object request, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task Publish(object notification, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task Publish(TNotification notification, CancellationToken cancellationToken = default) where TNotification : INotification => throw new NotSupportedException(); + } + + /// Counts the passes asked for + returns a canned count, so the handler test asserts pure delegation. + private sealed class RecordingShred : IRetiredSecretShredService + { + public int Calls; + public int ToReturn; + + public Task ShredAsync(CancellationToken cancellationToken) + { + Calls++; + return Task.FromResult(ToReturn); + } + } +} diff --git a/backend/tests/CodeSpace.UnitTests/OAuth/OAuthClientRevokeTests.cs b/backend/tests/CodeSpace.UnitTests/OAuth/OAuthClientRevokeTests.cs new file mode 100644 index 000000000..c831034f3 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OAuth/OAuthClientRevokeTests.cs @@ -0,0 +1,139 @@ +using System.Net; +using System.Text; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.UnitTests.OAuth; + +/// +/// What each provider's client sends when CodeSpace ends a token. GitHub revokes at two endpoints: the grant delete ends +/// every token GitHub issued the user under the OAuth App — the ones another team connected through the same app holds +/// too — so it is sent only for a whole-grant revoke (a Disconnect); otherwise the token delete ends the one token. +/// GitLab revokes per token (RFC 7009) and sends the same request either way. The URL, method, body and client +/// authentication are asserted as they leave the client, through a recording . +/// +[Trait("Category", "Unit")] +public sealed class OAuthClientRevokeTests +{ + private static readonly ProviderInstance GitHubInstance = new() { Id = Guid.NewGuid(), TeamId = Guid.NewGuid(), Provider = ProviderKind.GitHub, DisplayName = "github", BaseUrl = "https://github.com", OauthClientId = "shared-app" }; + + private static readonly ProviderInstance GitLabInstance = new() { Id = Guid.NewGuid(), TeamId = Guid.NewGuid(), Provider = ProviderKind.GitLab, DisplayName = "gitlab", BaseUrl = "https://gitlab.example", OauthClientId = "shared-app" }; + + [Fact] + public void A_revoke_ends_the_whole_grant_unless_told_otherwise() + { + var input = new OAuthRevokeInput { Instance = GitHubInstance, ClientId = "shared-app", ClientSecret = "app-secret", Token = "tok" }; + + input.WholeGrant.ShouldBeTrue("a Disconnect, and every caller that does not say, keeps the grant-level revoke it always sent"); + } + + [Theory] + [InlineData(true, "access_token", "/applications/shared-app/grant")] // a Disconnect: the user's whole grant to the app + [InlineData(true, "refresh_token", "/applications/shared-app/grant")] // unchanged: a Disconnect sends both tokens to the grant delete + [InlineData(false, "access_token", "/applications/shared-app/token")] // a connection delete: this access token only + public async Task GitHub_deletes_the_grant_only_for_a_whole_grant_revoke_and_otherwise_the_one_token(bool wholeGrant, string tokenTypeHint, string expectedPath) + { + var handler = new RecordingHandler(HttpStatusCode.NoContent); + + await GitHub(handler).RevokeAsync(Input(GitHubInstance, wholeGrant, "tok-CANARY", tokenTypeHint), CancellationToken.None); + + var request = handler.Requests.ShouldHaveSingleItem(); + request.Method.ShouldBe(HttpMethod.Delete); + request.Uri.ShouldBe(new Uri("https://api.github.com" + expectedPath)); + request.Body.ShouldBe("{\"access_token\":\"tok-CANARY\"}"); + request.Authorization.ShouldBe("Basic " + Convert.ToBase64String(Encoding.ASCII.GetBytes("shared-app:app-secret")), "both endpoints authenticate as the OAuth App"); + } + + [Fact] + public async Task GitHub_sends_nothing_for_a_refresh_token_outside_a_whole_grant_revoke() + { + var handler = new RecordingHandler(HttpStatusCode.NoContent); + + await GitHub(handler).RevokeAsync(Input(GitHubInstance, wholeGrant: false, "rt-CANARY", "refresh_token"), CancellationToken.None); + + handler.Requests.ShouldBeEmpty("GitHub has no endpoint that ends a refresh token on its own; the only one that does is the grant delete, which would end the user's tokens in every other team on the same OAuth App"); + } + + [Theory] + [InlineData(HttpStatusCode.NoContent)] // revoked + [InlineData(HttpStatusCode.NotFound)] // already gone + [InlineData(HttpStatusCode.UnprocessableContent)] // not a token GitHub knows + public async Task GitHub_treats_a_token_revoke_that_finds_nothing_left_to_end_as_done(HttpStatusCode status) + { + var handler = new RecordingHandler(status); + + await Should.NotThrowAsync(() => GitHub(handler).RevokeAsync(Input(GitHubInstance, wholeGrant: false, "tok", "access_token"), CancellationToken.None)); + } + + [Fact] + public async Task GitHub_surfaces_a_token_revoke_the_provider_failed() + { + var handler = new RecordingHandler(HttpStatusCode.InternalServerError); + + var failure = await Should.ThrowAsync(() => GitHub(handler).RevokeAsync(Input(GitHubInstance, wholeGrant: false, "tok", "access_token"), CancellationToken.None)); + + failure.Error.ShouldBe("http_500", "the caller logs a failed revoke as best effort; it must not read as done"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task GitLab_revokes_the_one_token_whether_or_not_the_whole_grant_is_asked_for(bool wholeGrant) + { + var handler = new RecordingHandler(HttpStatusCode.OK); + + await GitLab(handler).RevokeAsync(Input(GitLabInstance, wholeGrant, "rt-CANARY", "refresh_token"), CancellationToken.None); + + var request = handler.Requests.ShouldHaveSingleItem(); + request.Method.ShouldBe(HttpMethod.Post); + request.Uri.ShouldBe(new Uri("https://gitlab.example/oauth/revoke")); + request.Body.ShouldBe("client_id=shared-app&client_secret=app-secret&token=rt-CANARY&token_type_hint=refresh_token", "RFC 7009 ends exactly the token presented"); + } + + private static OAuthRevokeInput Input(ProviderInstance instance, bool wholeGrant, string token, string tokenTypeHint) => new() + { + Instance = instance, + ClientId = "shared-app", + ClientSecret = "app-secret", + Token = token, + TokenTypeHint = tokenTypeHint, + WholeGrant = wholeGrant + }; + + private static GitHubOAuthClient GitHub(RecordingHandler handler) => new(new SingleClientFactory(handler), TimeProvider.System); + + private static GitLabOAuthClient GitLab(RecordingHandler handler) => new(new SingleClientFactory(handler), TimeProvider.System); + + private sealed record RecordedRequest(HttpMethod Method, Uri Uri, string Body, string? Authorization); + + /// Answers every request with one status and records it whole, as the provider would receive it. + private sealed class RecordingHandler : HttpMessageHandler + { + private readonly HttpStatusCode _status; + + public RecordingHandler(HttpStatusCode status) { _status = status; } + + public List Requests { get; } = new(); + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + var body = request.Content == null ? string.Empty : await request.Content.ReadAsStringAsync(cancellationToken); + + Requests.Add(new RecordedRequest(request.Method, request.RequestUri!, body, request.Headers.Authorization?.ToString())); + + return new HttpResponseMessage(_status) { Content = new StringContent("{}") }; + } + } + + /// Hands out clients over the recording handler; the handler outlives each client the provider client disposes. + private sealed class SingleClientFactory : IHttpClientFactory + { + private readonly HttpMessageHandler _handler; + + public SingleClientFactory(HttpMessageHandler handler) { _handler = handler; } + + public HttpClient CreateClient(string name) => new(_handler, disposeHandler: false); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/OAuth/OAuthGrantRevokerTests.cs b/backend/tests/CodeSpace.UnitTests/OAuth/OAuthGrantRevokerTests.cs new file mode 100644 index 000000000..cd93888d6 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/OAuth/OAuthGrantRevokerTests.cs @@ -0,0 +1,119 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.UnitTests.OAuth; + +/// +/// The provider side of ending an OAuth credential, shared by a Disconnect and by deleting the connection the credential +/// was made through. The refresh token goes first — on most providers it is the grant — then the access token. The +/// client secret is the one it is handed rather than one it reads off the connection: a delete reads the secret before +/// it blanks it, and revokes only after its transaction commits, when the connection no longer holds it. Whether the +/// provider is asked to end the user's whole grant or only these tokens is the caller's to say, on every request. +/// +[Trait("Category", "Unit")] +public sealed class OAuthGrantRevokerTests +{ + private static readonly ProviderInstance Instance = new() { Id = Guid.NewGuid(), TeamId = Guid.NewGuid(), Provider = ProviderKind.GitLab, DisplayName = "gitlab", BaseUrl = "https://gitlab.example", OauthClientId = "client-1" }; + + [Fact] + public async Task Revokes_the_refresh_token_then_the_access_token_with_the_client_secret_it_is_handed() + { + var client = new RecordingOAuthClient(); + + await Revoker(client).RevokeAsync(Instance, "client-secret-CANARY", new OAuthPayload { AccessToken = "at-CANARY", RefreshToken = "rt-CANARY" }, wholeGrant: true, CancellationToken.None); + + client.Revokes.Select(r => (r.Token, r.TokenTypeHint)).ShouldBe(new[] { ("rt-CANARY", "refresh_token"), ("at-CANARY", "access_token") }); + client.Revokes.ShouldAllBe(r => r.ClientId == "client-1" && r.ClientSecret == "client-secret-CANARY" && r.Instance == Instance); + } + + [Fact] + public async Task Revokes_only_the_access_token_of_a_grant_without_a_refresh_token() + { + var client = new RecordingOAuthClient(); + + await Revoker(client).RevokeAsync(Instance, "client-secret", new OAuthPayload { AccessToken = "at-only" }, wholeGrant: true, CancellationToken.None); + + client.Revokes.Select(r => r.Token).ShouldBe(new[] { "at-only" }); + } + + [Theory] + [InlineData(true)] // a Disconnect: the user's whole grant to the OAuth App + [InlineData(false)] // a connection delete: only the tokens this credential held + public async Task Asks_for_the_scope_it_is_told_on_every_token(bool wholeGrant) + { + var client = new RecordingOAuthClient(); + + await Revoker(client).RevokeAsync(Instance, "client-secret", new OAuthPayload { AccessToken = "at", RefreshToken = "rt" }, wholeGrant, CancellationToken.None); + + client.Revokes.Count.ShouldBe(2, "fixture check: both tokens were revoked"); + client.Revokes.ShouldAllBe(r => r.WholeGrant == wholeGrant, "the provider client decides what each scope means at its endpoints; the revoker must pass the caller's choice through untouched"); + } + + [Theory] + [InlineData(true)] // the stored envelope opens and holds OAuth tokens + [InlineData(false)] // it does not open — a rotated key ring, a blanked row + public void Reads_the_tokens_of_a_readable_OAuth_payload_and_nothing_from_an_unreadable_one(bool readable) + { + var serializer = new CredentialPayloadSerializer(); + var envelope = readable ? PrefixEncryptor.Seal(serializer.Serialize(new OAuthPayload { AccessToken = "at-read", RefreshToken = "rt-read" })) : "not-an-envelope"; + var credential = new Credential { Id = Guid.NewGuid(), AuthType = AuthType.OAuth, DisplayName = "oauth", EncryptedPayload = envelope }; + + var payload = Revoker(new RecordingOAuthClient()).ReadPayload(credential); + + if (!readable) + { + payload.ShouldBeNull(); + return; + } + + payload.ShouldNotBeNull(); + payload.AccessToken.ShouldBe("at-read"); + payload.RefreshToken.ShouldBe("rt-read"); + } + + private static OAuthGrantRevoker Revoker(RecordingOAuthClient client) => new(new SingleClientRegistry(client), new PrefixEncryptor(), new CredentialPayloadSerializer()); + + private sealed class RecordingOAuthClient : IOAuthClient + { + public List Revokes { get; } = new(); + + public ProviderKind Kind => ProviderKind.GitLab; + + public Uri BuildAuthorizeUrl(OAuthAuthorizeInput input) => throw new NotSupportedException(); + + public Task ExchangeCodeAsync(OAuthCodeExchangeInput input, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task RefreshAsync(OAuthRefreshInput input, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task RevokeAsync(OAuthRevokeInput input, CancellationToken cancellationToken) + { + Revokes.Add(input); + return Task.CompletedTask; + } + } + + private sealed class SingleClientRegistry : IOAuthClientRegistry + { + private readonly IOAuthClient _client; + + public SingleClientRegistry(IOAuthClient client) { _client = client; } + + public IOAuthClient Get(ProviderKind kind) => kind == _client.Kind ? _client : throw new NotSupportedException($"No OAuth client registered for provider {kind}"); + } + + /// An envelope that opens only when it carries the prefix it was sealed with, as a real key ring opens only its own. + private sealed class PrefixEncryptor : IPayloadEncryptor + { + private const string Prefix = "sealed:"; + + public static string Seal(string plaintext) => Prefix + plaintext; + + public string Encrypt(string plaintext) => Seal(plaintext); + + public string Decrypt(string ciphertext) => ciphertext.StartsWith(Prefix, StringComparison.Ordinal) ? ciphertext[Prefix.Length..] : throw new InvalidOperationException("the envelope does not open"); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/OAuth/OAuthTokenRefresherTests.cs b/backend/tests/CodeSpace.UnitTests/OAuth/OAuthTokenRefresherTests.cs index 12c8ad438..de07ad6af 100644 --- a/backend/tests/CodeSpace.UnitTests/OAuth/OAuthTokenRefresherTests.cs +++ b/backend/tests/CodeSpace.UnitTests/OAuth/OAuthTokenRefresherTests.cs @@ -172,21 +172,29 @@ public async Task A_refused_refresh_with_nothing_newer_stored_rethrows_the_refus refusal.Error.ShouldBe("invalid_grant"); } - [Fact] - public async Task A_credential_revoked_while_the_provider_rotated_is_not_written_back() + /// + /// Revocation empties the row without taking the refresh lock — here while the provider answers the refresh. The pair + /// it issued must not land in the revoked row, and since nothing stores it, it is revoked at the provider — the way the + /// write that emptied the row ends tokens. A Disconnect ends the user's grant, so the pair goes with the grant. A + /// retirement (connection delete, departure, relink) also soft-deletes the row and ends tokens, never the grant, which + /// on GitHub also carries the user's tokens in every other team connected through the same OAuth App. + /// + [Theory] + [InlineData(false, true)] // Disconnect: blanked, still listed → the whole grant + [InlineData(true, false)] // retirement: blanked and soft-deleted → only the pair + public async Task A_credential_revoked_while_the_provider_rotated_is_not_written_back(bool retired, bool expectedWholeGrant) { - // Revocation empties the row without taking the refresh lock — here while the provider answers the refresh. The - // pair it issued must not land in the revoked row, and since nothing stores it, it is revoked at the provider. var ctx = BuildContext(); var stale = new OAuthPayload { AccessToken = "old", RefreshToken = "r-0", ExpiresAt = Now.AddSeconds(-1) }; var row = new StoredRow(stale); - var provider = new RecordingOAuthClient("new", "r-1", Now.AddHours(2), onRefreshing: row.Revoke); + var provider = new RecordingOAuthClient("new", "r-1", Now.AddHours(2), onRefreshing: retired ? row.Retire : row.Revoke); var failure = await Should.ThrowAsync(() => RunAsync(stale, ctx, row, provider)); failure.Message.ShouldContain("was revoked"); row.IsEmpty.ShouldBeTrue(); provider.RevokedTokens.ShouldBe(new[] { "r-1", "new" }); + provider.WholeGrants.ShouldBe(new[] { expectedWholeGrant, expectedWholeGrant }, ignoreOrder: false, customMessage: "the issued pair ends the way the write that emptied the row ends tokens — check OAuthTokenRefresher.RevokeIssuedTokensAsync"); } [Fact] @@ -283,14 +291,26 @@ private sealed class StoredRow : ICredentialPayloadReader public void Store(CredentialPayload payload) => _ciphertext = Serializer.Serialize(payload); + private bool _retired; + + /// A Disconnect: the token blanked, the row still in the team's list. public void Revoke() => _ciphertext = string.Empty; + /// A retirement — connection delete, departure, relink: the token blanked and the row soft-deleted. + public void Retire() + { + Revoke(); + _retired = true; + } + public bool IsEmpty => _ciphertext.Length == 0; public OAuthPayload Read() => (OAuthPayload)Serializer.Deserialize(AuthType.OAuth, _ciphertext); public Task ReadEncryptedPayloadAsync(Guid credentialId, CancellationToken cancellationToken) => Task.FromResult(_ciphertext); + public Task IsRetiredAsync(Guid credentialId, CancellationToken cancellationToken) => Task.FromResult(_retired); + /// The writer's compare-and-swap: stores only if the row still holds . public bool StoreIfUnchanged(string expected, CredentialPayload payload) { @@ -336,6 +356,9 @@ public RecordingOAuthClient(string accessToken, string? refreshToken, DateTimeOf public List RevokedTokens { get; } = new(); + /// of each revoke, in the order of . + public List WholeGrants { get; } = new(); + public ProviderKind Kind => ProviderKind.GitLab; public Uri BuildAuthorizeUrl(OAuthAuthorizeInput input) => throw new NotSupportedException(); @@ -366,6 +389,7 @@ public Task RefreshAsync(OAuthRefreshInput input, Cancellati public Task RevokeAsync(OAuthRevokeInput input, CancellationToken cancellationToken) { RevokedTokens.Add(input.Token); + WholeGrants.Add(input.WholeGrant); return Task.CompletedTask; } } diff --git a/backend/tests/CodeSpace.UnitTests/Variables/VariableRetirementTests.cs b/backend/tests/CodeSpace.UnitTests/Variables/VariableRetirementTests.cs new file mode 100644 index 000000000..9d00cca42 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Variables/VariableRetirementTests.cs @@ -0,0 +1,40 @@ +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Variables; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.UnitTests.Variables; + +/// +/// Deleting a variable keeps its row for the audit trail and drops the only thing in it that is still a secret: a +/// Secret's ciphertext. Nothing reads a deleted row's value — every read filters on deleted_date IS NULL and +/// nothing restores one — so the envelope could only ever be opened by someone holding the database and the key. A +/// plain value is not a secret and stays, as the history of what the variable was. +/// +[Trait("Category", "Unit")] +public sealed class VariableRetirementTests +{ + private static readonly DateTimeOffset Now = new(2030, 1, 1, 12, 0, 0, TimeSpan.Zero); + + [Theory] + [InlineData(VariableValueType.Secret)] + [InlineData(VariableValueType.String)] + public void Retiring_stamps_the_deletion_and_drops_a_secrets_ciphertext_only(VariableValueType valueType) + { + var actor = Guid.NewGuid(); + var row = new Variable + { + Id = Guid.NewGuid(), Name = "TOKEN", ValueType = valueType, + ValueEncrypted = valueType == VariableValueType.Secret ? new byte[] { 1, 2, 3 } : null, + ValuePlain = valueType == VariableValueType.Secret ? null : "\"kept\"", + }; + + VariableRetirement.Retire(row, Now, actor); + + row.DeletedDate.ShouldBe(Now); + row.LastModifiedDate.ShouldBe(Now); + row.LastModifiedBy.ShouldBe(actor); + row.ValueEncrypted.ShouldBeNull("a deleted variable holds no ciphertext anyone could open later"); + row.ValuePlain.ShouldBe(valueType == VariableValueType.Secret ? null : "\"kept\"", "a plain value is no secret and stays as the record of what the variable held"); + } +}