From 18def69fc87a122e8e19639a0aad454ac9c8d48b Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Mon, 19 Oct 2026 08:45:09 +0800 Subject: [PATCH] Let a credential's owner reconnect it themselves Starting an OAuth connect took credentials.manage of everyone, so a Member or Viewer whose own sign-in had died could not bring it back: only an Admin could start it, and an Admin can only finish it by signing in as that person. Replacing a pasted token over the same credential already passed its owner on membership alone, so putting a sign-in right was harder than putting a token right. A connect that names a credential to re-authorize in place (ReplaceCredentialId) is now held to that credential's bar: its own person on membership, anyone else credentials.manage. A connect that names none mints a credential and keeps credentials.manage. Everything else a re-authorization checks is unchanged: the same provider account, a retired credential, an owner without standing, the rank reach over the person acted on, and the named owner being the credential's. A reconnect signs in AS the person, so it also keeps the reach over them (CredentialOwnerOrPermission.PermitsReconnect): an Admin cannot reconnect another Admin's or an Owner's sign-in, and nobody can for someone who has left the team. That was only the service's refusal, unknown to a button, so an Admin was offered a Reconnect on an Owner's sign-in that always answered 403, and a Reconnect or Replace token on the credential of someone who had left that no one could use. CallerStanding now models it, and the remedy names only the people who could do what the caller cannot. The bar is one definition, used by the pipeline (a marker and behavior), by CallerStanding for the remedy's buttons, by the credential list's new callerMayRepair, which the repository rows read, and by the OAuth callback, which asks it again of whoever started the connect before the code is exchanged: a starter demoted, removed from the team, or the person promoted past them in the ten minutes between start and callback never has a token written. The person's role is read one way, by TeamStanding.RolesActedOnAsync, which TeamMemberService now uses as well. callerMayRepair is additive: an older SPA ignores it and a newer one falls back to the role when a server sends none. No migration and no new setting. A pod from before this refuses the owner as it always did; one from after completes a connect an older pod started. --- .../CredentialOwnerOrPermission.cs | 62 ++- .../Authorization/CredentialPerson.cs | 17 + ...ialReauthorizationAuthorizationBehavior.cs | 25 + .../Authorization/TeamStanding.cs | 53 ++ .../Mediation/MediatorModule.cs | 4 + .../Credentials/CredentialService.Repair.cs | 28 ++ .../Services/Credentials/CredentialService.cs | 9 +- .../Credentials/Remedy/CallerStanding.cs | 40 +- .../Remedy/ProviderRemedyService.Assemble.cs | 41 +- .../Remedy/ProviderRemedyService.Context.cs | 5 + .../Remedy/ProviderRemedyService.Load.cs | 15 + .../Remedy/ProviderRemedyService.cs | 1 + .../Credentials/Remedy/RemedyActions.cs | 37 +- .../Credentials/Remedy/RemedyFacts.cs | 8 + .../Services/OAuth/OAuthFlowService.cs | 28 ++ .../Services/Teams/TeamMemberService.cs | 15 +- .../IRequireCredentialReauthorization.cs | 28 ++ .../Credentials/InitCredentialOAuthCommand.cs | 11 +- .../Dtos/Credentials/CredentialSummary.cs | 8 + .../Dtos/Credentials/ProviderRemedy.cs | 2 +- .../CredentialOwnerReconnectE2ETests.cs | 354 +++++++++++++ .../Credentials/ProviderRemedyHttpE2ETests.cs | 40 +- .../TeamCapabilityCoverageTests.cs | 7 +- .../Authorization/TeamStandingTests.cs | 36 ++ ...ReauthorizationFlowTests.OwnerReconnect.cs | 466 ++++++++++++++++++ .../CredentialReauthorizationFlowTests.cs | 6 +- ...roviderFailureThroughTheEngineFlowTests.cs | 12 + .../Credentials/ProviderRemedyFlowTests.cs | 233 ++++++++- .../TeamWritePermissionAdoptionTests.cs | 14 + .../CredentialOwnerOrPermissionTests.cs | 70 +++ .../Credentials/CredentialPersonTests.cs | 56 +++ .../Credentials/Remedy/RemedyActionsTests.cs | 170 +++++-- backend/tests/TESTING.md | 6 +- .../ai-code-space/connect-remote-modal.tsx | 13 +- frontend/src/api/types.ts | 7 + .../components/connect-remote-modal.test.tsx | 32 +- .../remedy/ProviderRemedyCard.test.tsx | 30 +- .../remedy/RepositoryConnectionIssue.tsx | 13 +- .../src/components/repository-list.test.tsx | 55 ++- 39 files changed, 1896 insertions(+), 161 deletions(-) create mode 100644 backend/src/CodeSpace.Core/Authorization/CredentialPerson.cs create mode 100644 backend/src/CodeSpace.Core/Authorization/CredentialReauthorizationAuthorizationBehavior.cs create mode 100644 backend/src/CodeSpace.Core/Services/Credentials/CredentialService.Repair.cs create mode 100644 backend/src/CodeSpace.Messages/Authorization/IRequireCredentialReauthorization.cs create mode 100644 backend/tests/CodeSpace.E2ETests/Credentials/CredentialOwnerReconnectE2ETests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.OwnerReconnect.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Credentials/CredentialOwnerOrPermissionTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Credentials/CredentialPersonTests.cs diff --git a/backend/src/CodeSpace.Core/Authorization/CredentialOwnerOrPermission.cs b/backend/src/CodeSpace.Core/Authorization/CredentialOwnerOrPermission.cs index d3ff3a80c..8f41a2c0f 100644 --- a/backend/src/CodeSpace.Core/Authorization/CredentialOwnerOrPermission.cs +++ b/backend/src/CodeSpace.Core/Authorization/CredentialOwnerOrPermission.cs @@ -2,17 +2,26 @@ using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Services.Identity; using CodeSpace.Messages.Authorization; +using CodeSpace.Messages.Constants; 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. +/// The rule every credential tier applies, with its own permission: the owner of a Personal credential passes on membership +/// alone; 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. +/// +/// Defined once, in , because every place that asks must agree: this class enforces it for a request +/// (, the spend tier and the OAuth connect's +/// ), CallerStanding decides a remedy's +/// buttons — and the credential list's callerMayRepair, which the repository rows read — by it, and the OAuth callback asks it +/// again of whoever started a reconnect — so a button is never shown to a person the endpoint behind it refuses, and a connect is +/// never finished under a permission that was taken back. +/// +/// A reconnect is held to one more thing, which is : the reach over the credential's person. /// public sealed class CredentialOwnerOrPermission : IScopedDependency { @@ -29,29 +38,46 @@ public CredentialOwnerOrPermission(CodeSpaceDbContext db, ICurrentTeam currentTe _resolver = resolver; } - public async Task EnsureAsync(Guid credentialId, string permission, CancellationToken cancellationToken) + /// + /// The rule: a credential's own person passes, and anyone else needs . Pure, so the same answer is the + /// one a request gets, a button shows and a callback asks again. is the credential + /// being Personal and the person being its owner — a team-service credential has none, so it is never true for one. + /// + public static bool Permits(TeamRole role, bool ownsThePersonalCredential, string permission) => ownsThePersonalCredential || TeamPermissionMatrix.IsGranted(role, permission); + + /// + /// A reconnect, which is the rule above and one more: it signs in AS the credential's person, so it acts on a person and not only on a + /// credential, and holds to the reach every act on a member keeps () — an Admin does not reach another + /// Admin or an Owner, nobody reaches an Owner but themselves. is the role the person stands as in the team + /// (), and null when they have none — they left, or the credential has no person — which nobody + /// can reconnect. What the service refuses with RoleOutranksActorException or KeyNotFoundException after the pipeline has let + /// the request in is this answer, so a button, the service and the OAuth callback all ask it here. + /// + public static bool PermitsReconnect(TeamRole role, bool ownsThePersonalCredential, TeamRole? ownerStanding) => + ownerStanding is { } owner && Permits(role, ownsThePersonalCredential, TeamPermissions.CredentialsManage) && TeamRoleRank.Reaches(role, owner, isOwn: ownsThePersonalCredential); + + /// + /// For the caller on this team. is null for a request that may name no credential — a connect that + /// mints one has no owner to pass, so it is the permission's alone. + /// + public async Task EnsureAsync(Guid? credentialId, string permission, CancellationToken cancellationToken) { - if (await IsCallersOwnPersonalCredentialAsync(credentialId, cancellationToken).ConfigureAwait(false)) return; + var teamId = _currentTeam.Id ?? throw TenantAccessDeniedException.NoTeamSelected(_currentUser.Id, HeaderCurrentTeam.HeaderName); - await EnsurePermissionAsync(permission, cancellationToken).ConfigureAwait(false); + var role = await _resolver.ResolveRoleAsync(teamId, cancellationToken).ConfigureAwait(false); + var owns = credentialId is Guid id && await IsCallersOwnPersonalCredentialAsync(id, teamId, cancellationToken).ConfigureAwait(false); + + if (!Permits(role, owns, permission)) + throw TenantAccessDeniedException.MissingTeamPermission(_currentUser.Id, teamId, role, permission); } - private async Task IsCallersOwnPersonalCredentialAsync(Guid credentialId, CancellationToken cancellationToken) + /// A live credential of the team being asked about: an owner's pass is for a credential of THIS team, whatever else the request proved. + private async Task IsCallersOwnPersonalCredentialAsync(Guid credentialId, Guid teamId, 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) + .AnyAsync(c => c.Id == credentialId && c.TeamId == teamId && 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/CredentialPerson.cs b/backend/src/CodeSpace.Core/Authorization/CredentialPerson.cs new file mode 100644 index 000000000..8a6165d52 --- /dev/null +++ b/backend/src/CodeSpace.Core/Authorization/CredentialPerson.cs @@ -0,0 +1,17 @@ +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Authorization; + +/// +/// The person a Personal credential belongs to, as the rules about acting on it see them: whether they are the one asking, and the role they +/// stand as in the team — null when they have none, because they left and the credential has not been retired yet. Built in one place, from the +/// credential's row and , by everything that says beforehand what a person may do about a +/// credential (the provider remedy and the credential list), so the two cannot read the same row two ways. A credential with no person — a +/// team's own, or a token stored with no owner — has none, and the callers pass null. +/// +public sealed record CredentialPerson(bool IsCaller, TeamRole? Standing) +{ + /// is 's answer for the credential's team; a person it does not name stands as no one. + public static CredentialPerson? Of(CredentialOwnership ownership, Guid? ownerUserId, Guid? callerId, IReadOnlyDictionary standing) => + ownership == CredentialOwnership.Personal && ownerUserId is { } owner ? new CredentialPerson(owner == callerId, standing.TryGetValue(owner, out var role) ? role : null) : null; +} diff --git a/backend/src/CodeSpace.Core/Authorization/CredentialReauthorizationAuthorizationBehavior.cs b/backend/src/CodeSpace.Core/Authorization/CredentialReauthorizationAuthorizationBehavior.cs new file mode 100644 index 000000000..dd65cba3c --- /dev/null +++ b/backend/src/CodeSpace.Core/Authorization/CredentialReauthorizationAuthorizationBehavior.cs @@ -0,0 +1,25 @@ +using CodeSpace.Messages.Authorization; +using CodeSpace.Messages.Constants; +using MediatR; + +namespace CodeSpace.Core.Authorization; + +/// +/// A connect that names a credential to re-authorize is held to the credential's bar — its owner on membership alone, anyone else +/// — and one that names none mints a credential, which is the permission's alone. +/// Runs inside , which has already proven the caller a member of +/// the header team; proves the named credential is that team's. +/// +public sealed class CredentialReauthorizationAuthorizationBehavior : IPipelineBehavior where TRequest : IRequireCredentialReauthorization +{ + private readonly CredentialOwnerOrPermission _rule; + + public CredentialReauthorizationAuthorizationBehavior(CredentialOwnerOrPermission rule) { _rule = rule; } + + public async Task Handle(TRequest request, RequestHandlerDelegate next, CancellationToken cancellationToken) + { + await _rule.EnsureAsync(request.ReplaceCredentialId, TeamPermissions.CredentialsManage, cancellationToken).ConfigureAwait(false); + + return await next().ConfigureAwait(false); + } +} diff --git a/backend/src/CodeSpace.Core/Authorization/TeamStanding.cs b/backend/src/CodeSpace.Core/Authorization/TeamStanding.cs index b40e8bd66..bb510f945 100644 --- a/backend/src/CodeSpace.Core/Authorization/TeamStanding.cs +++ b/backend/src/CodeSpace.Core/Authorization/TeamStanding.cs @@ -1,7 +1,9 @@ using System.Linq.Expressions; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Messages.Authorization; using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Query; @@ -37,6 +39,57 @@ public static Expression> Of(CodeSpaceDbContext db, Expression< public static async Task HeldAsync(CodeSpaceDbContext db, Guid teamId, Guid userId, CancellationToken cancellationToken) => await db.User.IgnoreQueryFilters().AsNoTracking().Where(u => u.Id == userId).AnyAsync(Of(db, _ => teamId, u => u.Id), cancellationToken).ConfigureAwait(false); + /// + /// What that standing amounts to as a role, asked of anyone — what answers for the + /// caller, for a request that has no caller to ask it of (the provider's OAuth callback is anonymous): an instance Admin stands as an + /// Owner, anyone else as the role of their membership row in a live team; null when they have neither. Agrees with + /// , which differs only in answering whether rather than what — and, for a member, in treating a deleted team as no + /// standing, as the resolver does. + /// + public static async Task RoleHeldAsync(CodeSpaceDbContext db, Guid teamId, Guid userId, CancellationToken cancellationToken) + { + var isInstanceAdmin = await db.RoleUser.AsNoTracking().AnyAsync(ru => ru.UserId == userId && db.Role.Any(r => r.Id == ru.RoleId && r.Name == Roles.Admin && r.Status), cancellationToken).ConfigureAwait(false); + + if (isInstanceAdmin) return TeamRole.Owner; + + return await db.Team.AsNoTracking() + .Where(t => t.Id == teamId && t.DeletedDate == null) + .SelectMany(t => t.Memberships.Where(m => m.UserId == userId).Select(m => (TeamRole?)m.Role)) + .FirstOrDefaultAsync(cancellationToken).ConfigureAwait(false); + } + + /// + /// What each person is to someone ACTING ON them — naming them as a credential's owner, signing in as them — not to someone asking + /// what they may do: the role of their membership row in a live team, else an instance Admin's standing as an Owner; absent when they have + /// neither. It is the target's side of every reach (), which reads a member's row before an instance + /// Admin's role — the opposite of , which answers for someone acting and so lets the role win. One definition for + /// the service that enforces the reach (TeamMemberService.EnsureMayActOnAsync), the callback that asks it again and the remedy + /// and the credential list that say, beforehand, whether it would hold — so a button and the endpoint behind it read the person alike. + /// Asked for many people at once because a list of credentials has one owner each. + /// + public static async Task> RolesActedOnAsync(CodeSpaceDbContext db, Guid teamId, IReadOnlyCollection userIds, CancellationToken cancellationToken) + { + var roles = new Dictionary(); + + if (userIds.Count == 0 || !await db.Team.AsNoTracking().AnyAsync(t => t.Id == teamId && t.DeletedDate == null, cancellationToken).ConfigureAwait(false)) return roles; + + var rows = await db.TeamMembership.AsNoTracking().Where(m => m.TeamId == teamId && userIds.Contains(m.UserId)).Select(m => new { m.UserId, m.Role }).ToListAsync(cancellationToken).ConfigureAwait(false); + var instanceAdmins = await db.RoleUser.AsNoTracking().Where(ru => userIds.Contains(ru.UserId) && db.Role.Any(r => r.Id == ru.RoleId && r.Name == Roles.Admin && r.Status)).Select(ru => ru.UserId).ToListAsync(cancellationToken).ConfigureAwait(false); + + foreach (var row in rows) roles[row.UserId] = row.Role; + foreach (var admin in instanceAdmins) roles.TryAdd(admin, TeamRole.Owner); + + return roles; + } + + /// for one person: null when they have no standing. + public static async Task RoleActedOnAsync(CodeSpaceDbContext db, Guid teamId, Guid userId, CancellationToken cancellationToken) + { + var roles = await RolesActedOnAsync(db, teamId, new[] { userId }, cancellationToken).ConfigureAwait(false); + + return roles.TryGetValue(userId, out var role) ? role : null; + } + private static Expression> Rule(CodeSpaceDbContext db) => (teamId, userId) => db.TeamMembership.Any(m => m.TeamId == teamId && m.UserId == userId) || db.RoleUser.Any(ru => ru.UserId == userId && db.Role.Any(r => r.Id == ru.RoleId && r.Name == Roles.Admin && r.Status)); diff --git a/backend/src/CodeSpace.Core/Mediation/MediatorModule.cs b/backend/src/CodeSpace.Core/Mediation/MediatorModule.cs index dae9a9b92..f56dc1650 100644 --- a/backend/src/CodeSpace.Core/Mediation/MediatorModule.cs +++ b/backend/src/CodeSpace.Core/Mediation/MediatorModule.cs @@ -42,6 +42,9 @@ protected override void Load(ContainerBuilder builder) // 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 + // CredentialReauthorization — inside TeamMembership (whose marker it extends): a connect that names a credential + // is held to the credential's bar (its owner on membership, anyone else + // credentials.manage); one that names none mints a credential and needs credentials.manage // BotVisibility — opts the request into seeing bot users (IBotInclusive); default excludes // Transactional — innermost of these, wraps DB writes only after auth has passed // @@ -59,6 +62,7 @@ protected override void Load(ContainerBuilder builder) 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(CredentialReauthorizationAuthorizationBehavior<,>)).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/Services/Credentials/CredentialService.Repair.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.Repair.cs new file mode 100644 index 000000000..b84d717b1 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.Repair.cs @@ -0,0 +1,28 @@ +using CodeSpace.Core.Authorization; +using CodeSpace.Core.Services.Credentials.Remedy; +using CodeSpace.Messages.Dtos.Credentials; +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.Credentials; + +/// What the person listing may do about each credential — the answer a client shows its Reconnect and Replace token buttons by. +public sealed partial class CredentialService +{ + /// + /// Says, for each listed credential, whether the caller may put it right in place: a sign-in reconnected or a pasted token replaced, by exactly the + /// rules the endpoints behind those buttons apply (), so the list never offers what the server would refuse — a + /// teammate's credential to someone without credentials.manage, another Admin's or an Owner's sign-in to an Admin, one whose person left. + /// One read of the standing of every owner on the list, however long it is. + /// + private async Task> WithCallerRepairAsync(IReadOnlyList summaries, Guid teamId, CancellationToken cancellationToken) + { + var role = await _membership.ResolveRoleAsync(teamId, cancellationToken).ConfigureAwait(false); + var owners = summaries.Where(s => s.Ownership == CredentialOwnership.Personal && s.OwnerUserId != null).Select(s => s.OwnerUserId!.Value).Distinct().ToList(); + var standing = await TeamStanding.RolesActedOnAsync(_db, teamId, owners, cancellationToken).ConfigureAwait(false); + + return summaries.Select(s => s with { CallerMayRepair = MayRepair(role, s, standing) }).ToList(); + } + + private bool MayRepair(TeamRole role, CredentialSummary credential, IReadOnlyDictionary standing) => + CallerStanding.Of(role, CredentialPerson.Of(credential.Ownership, credential.OwnerUserId, _currentUser.Id, standing)).MayRepair(CredentialHealthDerivation.IsStaticToken(credential.AuthType)); +} diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs index bf3abed1a..a9504aca3 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialService.cs @@ -1,3 +1,4 @@ +using CodeSpace.Core.Authorization; using CodeSpace.Core.DependencyInjection; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; @@ -35,9 +36,10 @@ public sealed partial class CredentialService : ICredentialService, IScopedDepen private readonly ICredentialHealthService _health; private readonly ICredentialReauthorizer _reauthorizer; private readonly ITeamMemberService _members; + private readonly TeamMembershipResolver _membership; private readonly ILogger _logger; - public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IOAuthGrantRevoker grants, IProviderRegistry registry, IProviderModuleCatalog modules, IScopeChecker scopeChecker, ICredentialSuccessionService succession, ICredentialRevocationService revocation, ICredentialHealthService health, ICredentialReauthorizer reauthorizer, ITeamMemberService members, ILogger logger) + public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurrentUser currentUser, IPayloadEncryptor encryptor, ICredentialPayloadSerializer serializer, IOAuthGrantRevoker grants, IProviderRegistry registry, IProviderModuleCatalog modules, IScopeChecker scopeChecker, ICredentialSuccessionService succession, ICredentialRevocationService revocation, ICredentialHealthService health, ICredentialReauthorizer reauthorizer, ITeamMemberService members, TeamMembershipResolver membership, ILogger logger) { _db = db; _currentTeam = currentTeam; @@ -53,6 +55,7 @@ public CredentialService(CodeSpaceDbContext db, ICurrentTeam currentTeam, ICurre _health = health; _reauthorizer = reauthorizer; _members = members; + _membership = membership; _logger = logger; } @@ -87,7 +90,9 @@ public async Task> ListAsync(Guid? providerInst }) .ToListAsync(cancellationToken).ConfigureAwait(false); - return await WithHealthAsync(summaries, teamId, cancellationToken).ConfigureAwait(false); + var withHealth = await WithHealthAsync(summaries, teamId, cancellationToken).ConfigureAwait(false); + + return await WithCallerRepairAsync(withHealth, teamId, cancellationToken).ConfigureAwait(false); } /// diff --git a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/CallerStanding.cs b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/CallerStanding.cs index db2ec5ec6..b6c90cb2b 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/CallerStanding.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/CallerStanding.cs @@ -1,3 +1,4 @@ +using CodeSpace.Core.Authorization; using CodeSpace.Messages.Authorization; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Enums; @@ -6,24 +7,37 @@ namespace CodeSpace.Core.Services.Credentials.Remedy; /// /// What the person asking may do about a provider failure, decided by the SAME rules the endpoints that do it apply — so a button -/// is enabled exactly when the request behind it would be accepted. Pure: the team role and whether the credential is the -/// caller's own are the only inputs. +/// is enabled exactly when the request behind it would be accepted. Pure: the team role and who the credential belongs to are the only inputs. /// /// /// repos.manage (Admin and Owner): re-link a repository, edit a provider's address. -/// credentials.manage (Admin and Owner): reconnect a sign-in. POST /api/credentials/oauth/init is a -/// credentials.manage request whoever the credential belongs to — owning the row does not pass it. -/// The credential's own person, or credentials.manage: replace a pasted token — the rule -/// applies to PUT /api/credentials/{id}/token. +/// Replace a pasted token (PUT /api/credentials/{id}/token): the credential's own person, or credentials.manage +/// (, which the pipeline applies) — and the person must still stand in the team, which the +/// service holds while it writes. +/// Reconnect a sign-in (POST /api/credentials/oauth/init naming the credential): the same bar, and one more because it signs in AS +/// the person — the reach over them (, which the service applies and the OAuth +/// callback asks again). An Admin reconnects a Member's sign-in, never another Admin's or an Owner's; their own person always may. /// runs.launch (Member and up): run it again. /// /// -public sealed record CallerStanding(bool MayManageRepositories, bool MayReconnectTheCredential, bool MayReplaceTheToken, bool MayLaunchRuns) +public sealed record CallerStanding(bool MayManageRepositories, bool MayReplaceTheToken, bool MayReconnectTheCredential, bool MayLaunchRuns) { - /// : the credential is Personal and the caller is its person — which passes the credential rule on membership alone. - public static CallerStanding Of(TeamRole role, bool ownsTheCredential) => new( - MayManageRepositories: TeamPermissionMatrix.IsGranted(role, TeamPermissions.ReposManage), - MayReconnectTheCredential: TeamPermissionMatrix.IsGranted(role, TeamPermissions.CredentialsManage), - MayReplaceTheToken: ownsTheCredential || TeamPermissionMatrix.IsGranted(role, TeamPermissions.CredentialsManage), - MayLaunchRuns: TeamPermissionMatrix.IsGranted(role, TeamPermissions.RunsLaunch)); + /// + /// is who the credential belongs to (), and null when it belongs to no one — a team's + /// own: its token is replaced by whoever holds credentials.manage, and it has no sign-in to reconnect. + /// + public static CallerStanding Of(TeamRole role, CredentialPerson? person) + { + var owns = person is { IsCaller: true }; + var personStands = person is null || person.Standing is not null; + + return new( + MayManageRepositories: TeamPermissionMatrix.IsGranted(role, TeamPermissions.ReposManage), + MayReplaceTheToken: personStands && CredentialOwnerOrPermission.Permits(role, owns, TeamPermissions.CredentialsManage), + MayReconnectTheCredential: CredentialOwnerOrPermission.PermitsReconnect(role, owns, person?.Standing), + MayLaunchRuns: TeamPermissionMatrix.IsGranted(role, TeamPermissions.RunsLaunch)); + } + + /// The fix for a credential of this kind: a pasted token is replaced, a sign-in is reconnected — the one pick makes. + public bool MayRepair(bool staticToken) => staticToken ? MayReplaceTheToken : MayReconnectTheCredential; } diff --git a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Assemble.cs b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Assemble.cs index 61e50d7e1..c8d28364c 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Assemble.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Assemble.cs @@ -1,5 +1,6 @@ using CodeSpace.Core.Services.Credentials; using CodeSpace.Core.Services.Redaction; +using CodeSpace.Messages.Authorization; using CodeSpace.Messages.Dtos.Credentials; using CodeSpace.Messages.Enums; @@ -42,10 +43,12 @@ private void StateFacts() Provider = _ctx.Instance?.Provider.ToString(), CredentialName = credential?.DisplayName ?? _ctx.Failure.Detail("credentialName"), OwnerName = _ctx.OwnerName, - CallerOwnsCredential = OwnsCredential(credential), + CallerOwnsCredential = _ctx.Person is { IsCaller: true }, + OwnerRole = _ctx.Person?.Standing, + CredentialOwnerLeft = _ctx.Person is { Standing: null }, CredentialRetired = credential?.DeletedDate != null, CredentialIsStaticToken = credential is { } c && CredentialHealthDerivation.IsStaticToken(c.AuthType), - CredentialIsTeamService = credential?.Ownership == CredentialOwnership.TeamService, + CredentialIsTeamService = credential is not null && _ctx.Person is null, Repositories = RepositoryPhrase(), RepositoryCount = _ctx.Repositories.Count, RedirectsTo = _ctx.Failure.Detail("finalOrigin"), @@ -67,8 +70,6 @@ private void StateFacts() ? RemedyResolution.RepairRedirect(_ctx.Failure.Detail("requestedOrigin"), _ctx.Failure.Detail("finalOrigin"), instance.BaseUrl, instance.ApiUrl, _ctx.ConnectedCredentials) : null; - private bool OwnsCredential(CredentialRow? credential) => credential is { Ownership: CredentialOwnership.Personal, OwnerUserId: { } owner } && owner == _ctx.CallerId; - private string? RepositoryPhrase() => _ctx.Repositories.Count switch { 0 => null, @@ -79,7 +80,7 @@ private void StateFacts() private void ChooseActions() { - var caller = CallerStanding.Of(_ctx.Role, OwnsCredential(_ctx.Credential)); + var caller = CallerStanding.Of(_ctx.Role, _ctx.Person); var subjects = new RemedyActions.ActionSubjects(HasCredential: _ctx.Credential is not null, HasRepositories: _ctx.Repositories.Count > 0, HasProviderInstance: _ctx.Instance is not null, OffersRerun: _ctx.OffersRerun); _ctx.Actions = RemedyActions.Build(_ctx.Code, _ctx.Facts, caller, subjects); @@ -108,30 +109,46 @@ private void ChooseActions() OwnerName = _ctx.OwnerName, AuthType = c.AuthType, Status = c.Status, - Health = CredentialHealthDerivation.Derive(c.Status, c.AuthType, c.ExpiresDate, ownerLeft: false, DateTimeOffset.UtcNow), + Health = CredentialHealthDerivation.Derive(c.Status, c.AuthType, c.ExpiresDate, ownerLeft: _ctx.Person is { Standing: null }, DateTimeOffset.UtcNow), Retired = c.DeletedDate != null, ProviderInstanceId = c.ProviderInstanceId }; private RemedyRepository ToRepository(RepositoryRow r) => new() { Id = r.Id, FullPath = r.FullPath, Status = r.Status, CredentialId = r.CredentialId, CredentialWorks = RepositoryWorks(r, DateTimeOffset.UtcNow) }; - /// Who to ask when something is not the caller's to do: the credential's own person for a repair only they can make, then the team's Owners and Admins. Empty when every action is the caller's — a list of people to ask beside buttons that work would only be noise. + /// + /// Who to ask when something is not the caller's to do: the credential's own person for a repair only they can make, then the team's Owners and + /// Admins who could do what the caller cannot. Empty when every action is the caller's — a list of people to ask beside buttons that work would only be noise. + /// private async Task> WhoToAskAsync(CancellationToken cancellationToken) { - if (!_ctx.Actions.Any(a => !a.Enabled && a.Kind is not RemedyActionKind.Rerun)) return Array.Empty(); + var refused = _ctx.Actions.Where(a => !a.Enabled && a.Kind is not RemedyActionKind.Rerun).ToList(); + + if (refused.Count == 0) return Array.Empty(); var people = new List(); - // Only a token replace can be the credential's own person's to make; a reconnect needs credentials.manage whoever owns it, so naming them would send the asker to someone who cannot do it either. - var repairIsTheirs = _ctx.Actions.Any(a => !a.Enabled && a.Kind is RemedyActionKind.ReplaceToken); + // Putting the credential right in place — a reconnect or a token replace — is its own person's to make, so they are named first, as long as they still stand in the team. + var repairIsTheirs = refused.Any(a => a.Kind is RemedyActionKind.Reconnect or RemedyActionKind.ReplaceToken); - if (repairIsTheirs && _ctx.Credential is { Ownership: CredentialOwnership.Personal, OwnerUserId: { } owner } && owner != _ctx.CallerId && _ctx.OwnerName is { } ownerName) + if (repairIsTheirs && _ctx.Person is { Standing: not null } && _ctx.Credential is { OwnerUserId: { } owner } && owner != _ctx.CallerId && _ctx.OwnerName is { } ownerName) people.Add(new RemedyPerson { UserId = owner, Name = ownerName, Role = "Owner of the credential" }); var administrators = await ReadTeamAdministratorsAsync(cancellationToken).ConfigureAwait(false); - people.AddRange(administrators.Where(a => a.UserId != _ctx.CallerId && people.All(p => p.UserId != a.UserId)).Select(a => new RemedyPerson { UserId = a.UserId, Name = a.Name, Role = a.Role.ToString() })); + people.AddRange(administrators.Where(a => a.UserId != _ctx.CallerId && people.All(p => p.UserId != a.UserId) && CouldDoWhatIsRefused(a.Role, refused)).Select(a => new RemedyPerson { UserId = a.UserId, Name = a.Name, Role = a.Role.ToString() })); return people.Take(MaxPeople).ToList(); } + + /// + /// Whether an Owner or Admin could do at least one of what the caller cannot: they hold every permission, but a reconnect also needs the reach over + /// the credential's person — an Admin cannot sign in as another Admin — and nobody repairs what has no person in the team to repair it for. + /// + private bool CouldDoWhatIsRefused(TeamRole administrator, IReadOnlyList refused) => refused.Any(action => action.Kind switch + { + RemedyActionKind.Reconnect => _ctx.Person is { Standing: { } person } && TeamRoleRank.Reaches(administrator, person, isOwn: false), + RemedyActionKind.ReplaceToken => _ctx.Person is null or { Standing: not null }, + _ => true + }); } diff --git a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Context.cs b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Context.cs index f4aea6338..7e6d1a681 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Context.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Context.cs @@ -1,3 +1,4 @@ +using CodeSpace.Core.Authorization; using CodeSpace.Messages.Dtos.Credentials; using CodeSpace.Messages.Enums; using CodeSpace.Messages.Failures; @@ -28,6 +29,10 @@ private sealed class RemedyContext public CredentialRow? Credential { get; set; } public string? OwnerName { get; set; } + + /// Who the credential belongs to as the rules about acting on it read them: null when it belongs to no person, and a person who stands as no one when they left (). + public CredentialPerson? Person { get; set; } + public InstanceRow? Instance { get; set; } /// Credentials still presented through the instance — read only for a redirect, where it decides whether editing the address is accepted. diff --git a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Load.cs b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Load.cs index 5798461ee..7ee3d3cb8 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Load.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.Load.cs @@ -1,4 +1,5 @@ using System.Linq.Expressions; +using CodeSpace.Core.Authorization; using CodeSpace.Core.Persistence.Entities; using CodeSpace.Messages.Authorization; using CodeSpace.Messages.Enums; @@ -95,6 +96,20 @@ private async Task ResolveCallerAsync(CancellationToken cancellationToken) _ctx.CallerId = _currentUser.Id; } + /// + /// Who the credential belongs to and the role they stand as, read the way the service that enforces a reconnect's reach reads it + /// () — so a person who has left, or one an Admin cannot reach, is known here before the button is, not after the endpoint refuses. + /// + private async Task LoadPersonAsync(CancellationToken cancellationToken) + { + if (_ctx.Credential is not { } credential) return; + + var owners = credential.OwnerUserId is { } owner ? new[] { owner } : Array.Empty(); + var standing = await TeamStanding.RolesActedOnAsync(_db, _ctx.TeamId, owners, cancellationToken).ConfigureAwait(false); + + _ctx.Person = CredentialPerson.Of(credential.Ownership, credential.OwnerUserId, _ctx.CallerId, standing); + } + private async Task> ReadTeamAdministratorsAsync(CancellationToken cancellationToken) { var rows = await _db.TeamMembership.AsNoTracking() diff --git a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.cs b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.cs index 07025d5b3..9cdd3787f 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/ProviderRemedyService.cs @@ -84,6 +84,7 @@ private async Task BuildAsync(CancellationToken cancellationToke await LoadProviderInstanceAsync(cancellationToken).ConfigureAwait(false); await LoadConnectedCredentialsAsync(cancellationToken).ConfigureAwait(false); await ResolveCallerAsync(cancellationToken).ConfigureAwait(false); + await LoadPersonAsync(cancellationToken).ConfigureAwait(false); DecideResolved(); StateFacts(); diff --git a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/RemedyActions.cs b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/RemedyActions.cs index 252c98943..6d0eea33d 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/RemedyActions.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/RemedyActions.cs @@ -1,5 +1,6 @@ using CodeSpace.Messages.Constants; using CodeSpace.Messages.Dtos.Credentials; +using CodeSpace.Messages.Enums; using CodeSpace.Messages.Failures; namespace CodeSpace.Core.Services.Credentials.Remedy; @@ -41,14 +42,15 @@ private static IEnumerable RepairsFor(string code, RemedyFacts fac var repair = subjects.HasCredential && !facts.CredentialRetired ? RepairCredential(facts, caller) : null; var relink = subjects.HasRepositories ? Relink(facts, caller) : null; - return (facts.CredentialRetired || repair is null ? new[] { relink, repair } : new[] { repair, relink }).OfType(); + // A credential whose person left is waiting to be retired, as one already retired is: nothing can put it right, so moving off it leads. + return (facts.CredentialRetired || facts.CredentialOwnerLeft || repair is null ? new[] { relink, repair } : new[] { repair, relink }).OfType(); } /// A renewal the provider did not answer (a refusal of the grant is credential_needs_reconnect) and a standing lookup that got no answer say nothing against the credential, so nobody is sent to reconnect, replace or move off it for them. private static bool SaysNothingAboutTheCredential(string code, RemedyFacts facts) => code == FailureCodes.OAuthExchangeFailed || (code == WorkflowRunRequestRejectionReasons.AuthorStandingUnknown && facts.Lookup == StandingLookupCauses.NoAnswer); - /// Reconnecting a sign-in and replacing a pasted token are two endpoints with two rules (), so each is enabled by its own. + /// Reconnecting a sign-in and replacing a pasted token are two endpoints with ONE bar (): the credential's own person, or credentials.manage — and the reconnect, which signs in as the person, keeps the reach over them too. private static RemedyAction RepairCredential(RemedyFacts facts, CallerStanding caller) => facts.CredentialIsStaticToken ? ReplaceToken(facts, caller) : Reconnect(facts, caller); private static RemedyAction ReplaceToken(RemedyFacts facts, CallerStanding caller) => new() @@ -56,7 +58,7 @@ private static bool SaysNothingAboutTheCredential(string code, RemedyFacts facts Kind = RemedyActionKind.ReplaceToken, Label = "Replace token", Enabled = caller.MayReplaceTheToken, - DisabledReason = caller.MayReplaceTheToken ? null : facts.CredentialIsTeamService ? NeedsAdmin : $"Only {facts.OwnerName ?? "the person who connected it"}, or a team Admin, can do this." + DisabledReason = caller.MayReplaceTheToken ? null : ReplaceDisabledReason(facts) }; private static RemedyAction Reconnect(RemedyFacts facts, CallerStanding caller) => new() @@ -64,9 +66,36 @@ private static bool SaysNothingAboutTheCredential(string code, RemedyFacts facts Kind = RemedyActionKind.Reconnect, Label = facts.Provider is { Length: > 0 } provider ? $"Reconnect {provider}" : "Reconnect", Enabled = caller.MayReconnectTheCredential, - DisabledReason = caller.MayReconnectTheCredential ? null : NeedsAdmin + DisabledReason = caller.MayReconnectTheCredential ? null : ReconnectDisabledReason(facts, caller) }; + /// Whose the repair is when it is not the caller's: a person's own credential is theirs, or an Admin's; a team's has no person, so only the role is named; one whose person left is nobody's. + private static string ReplaceDisabledReason(RemedyFacts facts) => + facts.CredentialOwnerLeft ? OwnerLeft(facts) : facts.CredentialIsTeamService ? NeedsAdmin : OnlyTheOwnerOrAnAdmin(facts); + + /// + /// As , and one more cause: a caller who holds credentials.manage () + /// and is still refused is refused for the person's rank — only the person, or whoever outranks them, signs in as them. + /// + private static string ReconnectDisabledReason(RemedyFacts facts, CallerStanding caller) + { + if (facts.CredentialIsTeamService) return caller.MayReplaceTheToken ? "A team's own credential has no one's sign-in to reconnect; replace its token instead." : NeedsAdmin; + + if (facts.CredentialOwnerLeft) return OwnerLeft(facts); + + return caller.MayReplaceTheToken ? OutrankedByThePerson(facts) : OnlyTheOwnerOrAnAdmin(facts); + } + + private static string OnlyTheOwnerOrAnAdmin(RemedyFacts facts) => $"Only {Person(facts)}, or a team Admin, can do this."; + + /// An Owner's sign-in is nobody's but their own; an Admin's is also an Owner's. Naming the Admin role to an Admin would send them to ask themselves. + private static string OutrankedByThePerson(RemedyFacts facts) => + facts.OwnerRole == TeamRole.Owner ? $"Only {Person(facts)} can reconnect their own sign-in." : $"Only {Person(facts)}, or a team Owner, can reconnect this sign-in."; + + private static string OwnerLeft(RemedyFacts facts) => $"{facts.OwnerName ?? "The person who connected it"} has left this team, so this cannot be repaired."; + + private static string Person(RemedyFacts facts) => facts.OwnerName ?? "the person who connected it"; + private static RemedyAction Relink(RemedyFacts facts, CallerStanding caller) => new() { Kind = RemedyActionKind.Relink, diff --git a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/RemedyFacts.cs b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/RemedyFacts.cs index 0cc0024ed..b1fdebbe1 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/Remedy/RemedyFacts.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/Remedy/RemedyFacts.cs @@ -1,3 +1,5 @@ +using CodeSpace.Messages.Enums; + namespace CodeSpace.Core.Services.Credentials.Remedy; /// @@ -18,6 +20,12 @@ public sealed record RemedyFacts /// The credential is a person's own and the asker is that person. public bool CallerOwnsCredential { get; init; } + /// The role the credential's person stands as in the team, for wording who outranks them; null for a team's own credential and for a person who left. + public TeamRole? OwnerRole { get; init; } + + /// The credential is a person's own and that person no longer stands in the team: it is waiting to be retired, and nobody can put it right in place — only move what leans on it. + public bool CredentialOwnerLeft { get; init; } + /// Soft-deleted: it cannot be reconnected, only replaced by moving what leans on it. public bool CredentialRetired { get; init; } diff --git a/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs b/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs index 780c9b6dc..8303de281 100644 --- a/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs +++ b/backend/src/CodeSpace.Core/Services/OAuth/OAuthFlowService.cs @@ -10,6 +10,7 @@ using CodeSpace.Core.Settings.CorsPolicy; using CodeSpace.Core.Settings.OAuth; using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Constants; using CodeSpace.Messages.Credentials; using CodeSpace.Messages.Enums; using CodeSpace.Messages.Exceptions; @@ -122,6 +123,7 @@ private async Task FinishAsync(OAuthPendingState var (clientId, clientSecret) = ReadOAuthClientCredentials(instance); await EnsureOwnerStillInTeamAsync(pending, cancellationToken).ConfigureAwait(false); + await EnsureStarterStillMayReauthorizeAsync(pending, cancellationToken).ConfigureAwait(false); var token = await ExchangeCodeAsync(instance, clientId, clientSecret, code, pending.CodeVerifier, cancellationToken).ConfigureAwait(false); @@ -237,6 +239,32 @@ private async Task EnsureOwnerStillInTeamAsync(OAuthPendingState pending, Cancel throw OwnerLeftTheTeam(); } + /// + /// A reconnect that named a credential was let in at the start on the credential's bar — its own person on membership alone, anyone else + /// credentials.manage and the reach over the person — and the callback can land up to later, when the starter may + /// have been demoted, removed, or the credential's person promoted past them. The same rule () + /// is asked again, of the starter's standing now and before the code is exchanged, so a token is never written into a credential under a + /// permission that was taken back. Runs anonymous, so it reads the starter's role directly () rather than + /// as the caller, and the person's the way the reach reads it (). A starter with no standing at all + /// — removed from the team — may do nothing. A connect that named no credential has nothing more to ask: the standing of the person it is for is + /// 's. + /// + private async Task EnsureStarterStillMayReauthorizeAsync(OAuthPendingState pending, CancellationToken cancellationToken) + { + if (pending.ReplaceCredentialId is not Guid replacedId) return; + + var owner = await _db.Credential.AsNoTracking() + .Where(c => c.Id == replacedId && c.TeamId == pending.TeamId && c.Ownership == CredentialOwnership.Personal) + .Select(c => c.OwnerUserId) + .FirstOrDefaultAsync(cancellationToken).ConfigureAwait(false); + + var role = await TeamStanding.RoleHeldAsync(_db, pending.TeamId, pending.InitiatorUserId, cancellationToken).ConfigureAwait(false); + var ownerStanding = owner is { } ownerId ? await TeamStanding.RoleActedOnAsync(_db, pending.TeamId, ownerId, cancellationToken).ConfigureAwait(false) : null; + + if (role is not { } held || !CredentialOwnerOrPermission.PermitsReconnect(held, owner == pending.InitiatorUserId, ownerStanding)) + throw new OAuthCallbackException("the person who started this connection may no longer reconnect that credential"); + } + /// /// The check above runs before the provider round-trip, and the owner can leave during it. So the owner's standing is /// asked again once the token is back, and a member's held until this callback commits: a departure that committed diff --git a/backend/src/CodeSpace.Core/Services/Teams/TeamMemberService.cs b/backend/src/CodeSpace.Core/Services/Teams/TeamMemberService.cs index 91691559f..526e4fbb6 100644 --- a/backend/src/CodeSpace.Core/Services/Teams/TeamMemberService.cs +++ b/backend/src/CodeSpace.Core/Services/Teams/TeamMemberService.cs @@ -97,7 +97,8 @@ public async Task EnsureMayActOnAsync(Guid userId, CancellationToken cancellatio throw new KeyNotFoundException("that person is not a member of this team"); var actor = await _membership.ResolveRoleAsync(teamId, cancellationToken).ConfigureAwait(false); - var target = await ResolveStandingRoleAsync(teamId, userId, cancellationToken).ConfigureAwait(false); + var target = await TeamStanding.RoleActedOnAsync(_db, teamId, userId, cancellationToken).ConfigureAwait(false) + ?? throw new KeyNotFoundException("that person is not a member of this team"); EnsureMayActOn(actor, target, isSelf: userId == RequireUser()); } @@ -258,18 +259,6 @@ private async Task ShareLockLiveTeamAsync(Guid teamId, CancellationToken c private async Task ShareLockMembershipAsync(Guid teamId, Guid userId, CancellationToken cancellationToken) => await _db.Database.SqlQuery($"SELECT 1 AS \"Value\" FROM team_membership WHERE team_id = {teamId} AND user_id = {userId} FOR KEY SHARE").ToListAsync(cancellationToken).ConfigureAwait(false); - /// - /// The rank a person with standing is acted on at: their membership's role, read as and - /// read it; or, for someone who stands in the team without one — only an instance Admin can — - /// an Owner's, the rank gives that role in every team. - /// - private async Task ResolveStandingRoleAsync(Guid teamId, Guid userId, CancellationToken cancellationToken) - { - var membership = await _db.TeamMembership.AsNoTracking().SingleOrDefaultAsync(m => m.TeamId == teamId && m.UserId == userId, cancellationToken).ConfigureAwait(false); - - return membership?.Role ?? TeamRole.Owner; - } - private async Task EnsureTeamLiveAsync(Guid teamId, CancellationToken cancellationToken) { var live = await _db.Team.AsNoTracking().AnyAsync(t => t.Id == teamId && t.DeletedDate == null, cancellationToken).ConfigureAwait(false); diff --git a/backend/src/CodeSpace.Messages/Authorization/IRequireCredentialReauthorization.cs b/backend/src/CodeSpace.Messages/Authorization/IRequireCredentialReauthorization.cs new file mode 100644 index 000000000..7f06cdd8e --- /dev/null +++ b/backend/src/CodeSpace.Messages/Authorization/IRequireCredentialReauthorization.cs @@ -0,0 +1,28 @@ +namespace CodeSpace.Messages.Authorization; + +/// +/// Marker — a connect that may name an existing credential to re-authorize in place. It carries two bars, chosen by whether it names one: +/// a connect that names none mints a credential, and that takes TeamPermissions.CredentialsManage of anyone; one that names a +/// credential puts a new token into THAT row, and that is 's bar — its own person on +/// membership alone, anyone else credentials.manage. +/// +/// Extends , so the X-Team-Id team is vetted and the caller a member of it first; this +/// tier only decides whether that is enough. It is not the credential marker (): its id is +/// optional, and the credential-keyed behaviors prove the team of one that is always there. The rule proves it itself — an owner's pass +/// is for a credential of THIS team. +/// +/// Why the owner's pass: a Personal credential is one person's own provider token, and putting a dead sign-in right adds nothing +/// the person could not do by pasting a token over it (PUT /api/credentials/{id}/token, the same bar) — the provider account that +/// signs in must still be the one the credential was connected as. Without it a Member whose sign-in expired could only ask an Admin, who +/// could only complete it by signing in as that Member. +/// +/// What this tier does not decide is the reach over the credential's person, which is not a permission: a reconnect signs in AS them, so +/// the service also refuses an Admin who would reconnect another Admin's or an Owner's sign-in (the rank every act on a member keeps), and a +/// person who has left. Both are the same answer a client is given beforehand — CredentialSummary.CallerMayRepair, the remedy's buttons — +/// so a button is shown only where the request would be accepted. +/// +public interface IRequireCredentialReauthorization : IRequireTeamMembership +{ + /// The credential this connect re-authorizes in place, or null when it creates one. + Guid? ReplaceCredentialId { get; } +} diff --git a/backend/src/CodeSpace.Messages/Commands/Credentials/InitCredentialOAuthCommand.cs b/backend/src/CodeSpace.Messages/Commands/Credentials/InitCredentialOAuthCommand.cs index 94995a59d..f2ca69125 100644 --- a/backend/src/CodeSpace.Messages/Commands/Credentials/InitCredentialOAuthCommand.cs +++ b/backend/src/CodeSpace.Messages/Commands/Credentials/InitCredentialOAuthCommand.cs @@ -1,5 +1,4 @@ using CodeSpace.Messages.Authorization; -using CodeSpace.Messages.Constants; using CodeSpace.Messages.Mediation; namespace CodeSpace.Messages.Commands.Credentials; @@ -7,11 +6,13 @@ namespace CodeSpace.Messages.Commands.Credentials; /// /// Begins a provider-credential OAuth flow. Returns the authorize URL the SPA should open /// in a popup or full redirect. The provider's callback hits CompleteCredentialOAuthCommand. +/// +/// : a connect that names no credential mints one and takes +/// credentials.manage; one that names re-authorizes it in place, which its own person may do on +/// membership alone — anyone else needs credentials.manage. /// -public sealed record InitCredentialOAuthCommand : ICommand, IRequireTeamPermission +public sealed record InitCredentialOAuthCommand : ICommand, IRequireCredentialReauthorization { - public string RequiredPermission => TeamPermissions.CredentialsManage; - /// Provider instance we're authorising against. Must belong to the current team. public required Guid ProviderInstanceId { get; init; } @@ -43,7 +44,7 @@ public sealed record InitCredentialOAuthCommand : ICommandcredential_account_mismatch. A pod from before this field ignores it and creates a new credential, as a - /// connect always did. + /// connect always did. Who may name one: its owner, or whoever holds credentials.manage (and reaches the owner by rank). /// public Guid? ReplaceCredentialId { get; init; } } diff --git a/backend/src/CodeSpace.Messages/Dtos/Credentials/CredentialSummary.cs b/backend/src/CodeSpace.Messages/Dtos/Credentials/CredentialSummary.cs index a34efb9b0..61eaa3ad0 100644 --- a/backend/src/CodeSpace.Messages/Dtos/Credentials/CredentialSummary.cs +++ b/backend/src/CodeSpace.Messages/Dtos/Credentials/CredentialSummary.cs @@ -35,6 +35,14 @@ public sealed record CredentialSummary /// Whole days until a static token expires at its provider; null for an OAuth credential (its access token renews itself) and for a token with no recorded expiry. public int? DaysToExpiry { get; init; } + /// + /// Whether the person asking may put this credential right IN PLACE — reconnect its sign-in, or replace its pasted token — decided by the rules the + /// endpoints behind those buttons apply, so a client shows the button exactly when the endpoint would accept the request: the credential's own person, + /// or credentials.manage, and for a sign-in also the reach over its person (an Admin cannot reconnect another Admin's or an Owner's). False when + /// its person has left the team. Null from a server that predates it: a client then falls back to what it can tell from the role. + /// + public bool? CallerMayRepair { get; init; } + /// How many live repositories are bound through the credential — what a person reconnecting, replacing or disconnecting it needs to know is at stake. Zero from a server that predates it. public int BoundRepositoryCount { get; init; } } diff --git a/backend/src/CodeSpace.Messages/Dtos/Credentials/ProviderRemedy.cs b/backend/src/CodeSpace.Messages/Dtos/Credentials/ProviderRemedy.cs index db72839a9..e4c7a194d 100644 --- a/backend/src/CodeSpace.Messages/Dtos/Credentials/ProviderRemedy.cs +++ b/backend/src/CodeSpace.Messages/Dtos/Credentials/ProviderRemedy.cs @@ -54,7 +54,7 @@ public enum RemedyActionKind /// Move the repository (or every repository the credential holds) onto a live credential. [JsonStringEnumMemberName("relink")] Relink, - /// Sign in again with the provider, into the SAME credential, so nothing bound through it changes. Needs credentials.manage whoever owns the credential: starting an OAuth connect is that request. + /// Sign in again with the provider, into the SAME credential, so nothing bound through it changes. The credential's own person, or credentials.manage: starting a connect that names the credential is held to the same bar as replacing its token. [JsonStringEnumMemberName("reconnect")] Reconnect, /// Paste a new token into the SAME credential, so nothing bound through it changes. The credential's own person, or credentials.manage. diff --git a/backend/tests/CodeSpace.E2ETests/Credentials/CredentialOwnerReconnectE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Credentials/CredentialOwnerReconnectE2ETests.cs new file mode 100644 index 000000000..45456647b --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Credentials/CredentialOwnerReconnectE2ETests.cs @@ -0,0 +1,354 @@ +using System.Net; +using System.Text.Json; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; + +namespace CodeSpace.E2ETests.Credentials; + +/// +/// 🟢 High fidelity — the person a provider credential belongs to reconnects it themselves, through the real HTTP surface: the real +/// ASP.NET pipeline (routing, JWT, the X-Team-Id scope, the authorization behaviors, the exception filter), cookies on the wire, +/// the real GitLab OAuth client and provider, real Postgres; the one fake is GitLab, a on a loopback port. +/// +/// The rule under test: POST /api/credentials/oauth/init naming a replaceCredentialId takes credentials.manage of +/// anyone but the credential's own person, who passes on membership alone — the bar PUT /api/credentials/{id}/token applies. So a +/// plain Member (or a Viewer) whose sign-in died brings it back in place, in the browser that started it and across sites through the +/// SPA's completion page; a teammate's credential still takes an Admin; and a connect that names no credential still takes +/// credentials.manage of everyone. The credential-side guards — the same provider account, a retired credential, a departed owner +/// — are those of and stay in force. +/// +[Trait("Category", "E2E")] +[Trait("Category", "ProviderE2E")] +[Trait("Surface", "Http")] +public sealed class CredentialOwnerReconnectE2ETests : IClassFixture +{ + private const string BindingCookie = "codespace_oauth_nonce"; + private const string CallbackPath = "/api/credentials/oauth/callback"; + private const string CompletePath = "/api/credentials/oauth/complete"; + + /// A dev SPA origin on another site than the API's http://localhost — on the CORS list in Development. + private const string CrossSiteSpa = "http://127.0.0.1:5180"; + + private readonly TaskLaunchApiFactory _factory; + private readonly TenantHarness _tenants; + + public CredentialOwnerReconnectE2ETests(TaskLaunchApiFactory factory) + { + _factory = factory; + _tenants = new TenantHarness(factory); + } + + [Theory] + [InlineData(TeamRole.Member)] + [InlineData(TeamRole.Viewer)] + public async Task The_credentials_own_person_reconnects_in_the_browser_that_started_it_whatever_their_role(TeamRole role) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + var (ownerId, account) = world.PersonOf(role); + var credentialId = world.CredentialOf(ownerId); + + var init = await InitAsync(world, ownerId, credentialId, "/settings/credentials").ConfigureAwait(false); + + init.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(init).ConfigureAwait(false)); + var callback = await CallbackAsync(await StateOfAsync(init).ConfigureAwait(false), $"authz:{account}", CookieValue(BindingCookieOf(init)!)).ConfigureAwait(false); + + callback.StatusCode.ShouldBe(HttpStatusCode.Redirect, await TenantHarness.DescribeAsync(callback).ConfigureAwait(false)); + callback.Headers.Location!.OriginalString.ShouldBe($"/settings/credentials?oauthCredentialId={credentialId}", "the popup is told the credential it already had"); + + var credential = await ReadCredentialAsync(credentialId).ConfigureAwait(false); + credential.Status.ShouldBe(CredentialStatus.Active); + credential.LastError.ShouldBeNull(); + credential.StoredPair.RefreshToken.ShouldBe(gitlab.LiveRefreshTokenOf(account), "the row holds the pair GitLab issued for this sign-in"); + (await _tenants.DbAsync(db => db.Credential.AsNoTracking().CountAsync(c => c.TeamId == world.TeamId)).ConfigureAwait(false)).ShouldBe(world.CredentialCount, "no second credential was minted"); + + var repository = await _tenants.DbAsync(db => db.Repository.AsNoTracking().SingleAsync(r => r.CredentialId == credentialId)).ConfigureAwait(false); + repository.Status.ShouldBe(RepositoryStatus.Active, "the Error its dead token had set is lifted, and it is still bound to the same credential"); + + var listed = await ReadListedAsync(ownerId, world.TeamId, credentialId).ConfigureAwait(false); + listed.GetProperty("health").GetString().ShouldBe("active"); + } + + [Theory] + [InlineData(TeamRole.Member)] + [InlineData(TeamRole.Viewer)] + public async Task A_cross_site_reconnect_by_the_credentials_own_person_is_finished_by_their_spa(TeamRole role) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + var (ownerId, account) = world.PersonOf(role); + var credentialId = world.CredentialOf(ownerId); + var state = await StateOfAsync(await InitAsync(world, ownerId, credentialId, $"{CrossSiteSpa}/oauth-callback").ConfigureAwait(false)).ConfigureAwait(false); + + // The SPA and the API are on different sites, so the binding cookie never comes back: the callback hands the code to the SPA + // origin the connect named, exchanging nothing. + var callback = await CallbackAsync(state, $"authz:{account}", bindingCookie: null).ConfigureAwait(false); + + callback.StatusCode.ShouldBe(HttpStatusCode.Redirect, await TenantHarness.DescribeAsync(callback).ConfigureAwait(false)); + callback.Headers.Location!.OriginalString.ShouldStartWith($"{CrossSiteSpa}/oauth-callback#oauthCode="); + gitlab.Count(StubEvent.Authorized).ShouldBe(0, "handing off exchanges nothing"); + + var finish = await CompleteAsync(ownerId, world.TeamId, $"authz:{account}", state).ConfigureAwait(false); + + finish.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(finish).ConfigureAwait(false)); + JsonDocument.Parse(await finish.Content.ReadAsStringAsync().ConfigureAwait(false)).RootElement.GetProperty("credentialId").GetGuid().ShouldBe(credentialId); + (await ReadCredentialAsync(credentialId).ConfigureAwait(false)).StoredPair.RefreshToken.ShouldBe(gitlab.LiveRefreshTokenOf(account)); + (await _tenants.DbAsync(db => db.OAuthPendingState.AsNoTracking().CountAsync(s => s.State == state)).ConfigureAwait(false)).ShouldBe(0, "finishing spends the state"); + } + + [Fact] + public async Task A_member_cannot_reconnect_a_teammates_credential_and_an_admin_can_start_to() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + var mayas = world.CredentialOf(world.MayaId); + + var byAnotherMember = await InitAsync(world, world.MiloId, mayas, "/settings/credentials").ConfigureAwait(false); + var byAViewer = await InitAsync(world, world.ViewerId, mayas, "/settings/credentials").ConfigureAwait(false); + var byAnAdmin = await InitAsync(world, world.BossId, mayas, "/settings/credentials").ConfigureAwait(false); + + byAnotherMember.StatusCode.ShouldBe(HttpStatusCode.Forbidden, await TenantHarness.DescribeAsync(byAnotherMember).ConfigureAwait(false)); + byAViewer.StatusCode.ShouldBe(HttpStatusCode.Forbidden, await TenantHarness.DescribeAsync(byAViewer).ConfigureAwait(false)); + (await byAnotherMember.Content.ReadAsStringAsync().ConfigureAwait(false)).ShouldContain("Your role on this team is Member", Case.Sensitive, "the refusal says what the person can act on: their own role"); + byAnAdmin.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(byAnAdmin).ConfigureAwait(false)); + (await _tenants.DbAsync(db => db.OAuthPendingState.AsNoTracking().CountAsync(s => s.TeamId == world.TeamId)).ConfigureAwait(false)).ShouldBe(1, "only the Admin's start left a pending connect"); + } + + [Fact] + public async Task An_admin_finishes_a_members_reconnect_only_by_signing_in_as_that_members_account() + { + // credentials.manage still reaches every credential below the Admin: the start is theirs, and the account that signs in must be Maya's. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + var mayas = world.CredentialOf(world.MayaId); + var before = await ReadCredentialAsync(mayas).ConfigureAwait(false); + + var asTheAdmin = await InitAsync(world, world.BossId, mayas, "/settings/credentials").ConfigureAwait(false); + var refused = await CallbackAsync(await StateOfAsync(asTheAdmin).ConfigureAwait(false), "authz:boss", CookieValue(BindingCookieOf(asTheAdmin)!)).ConfigureAwait(false); + + refused.StatusCode.ShouldBe(HttpStatusCode.Conflict, await TenantHarness.DescribeAsync(refused).ConfigureAwait(false)); + (await ReadCredentialAsync(mayas).ConfigureAwait(false)).EncryptedPayload.ShouldBe(before.EncryptedPayload, "the Admin's own account is not Maya's"); + + var again = await InitAsync(world, world.BossId, mayas, "/settings/credentials").ConfigureAwait(false); + var finished = await CallbackAsync(await StateOfAsync(again).ConfigureAwait(false), "authz:maya", CookieValue(BindingCookieOf(again)!)).ConfigureAwait(false); + + finished.StatusCode.ShouldBe(HttpStatusCode.Redirect, await TenantHarness.DescribeAsync(finished).ConfigureAwait(false)); + (await ReadCredentialAsync(mayas).ConfigureAwait(false)).StoredPair.RefreshToken.ShouldBe(gitlab.LiveRefreshTokenOf("maya")); + } + + [Fact] + public async Task An_admin_cannot_reconnect_an_owners_or_a_peer_admins_sign_in_and_the_list_never_offers_it() + { + // credentials.manage is the permission, and a reconnect signs in AS the person, so it also takes the reach over them: an Admin reaches the + // Members below, never another Admin or an Owner. What the list says (callerMayRepair — what the repository rows' Reconnect is shown by) + // is the answer the endpoint then gives, asker by asker. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + + var cases = new (Guid Asker, string AskerLabel, Guid Person, string PersonLabel, bool Accepted)[] + { + (world.BossId, "an Admin", world.MayaId, "a Member's", true), + (world.BossId, "an Admin", world.ViewerId, "a Viewer's", true), + (world.BossId, "an Admin", world.AdaId, "a peer Admin's", false), + (world.BossId, "an Admin", world.OliveId, "an Owner's", false), + (world.OliveId, "an Owner", world.AdaId, "an Admin's", true), + (world.OliveId, "an Owner", world.MayaId, "a Member's", true), + (world.OliveId, "an Owner", world.OliveId, "their own", true), + (world.AdaId, "an Admin", world.AdaId, "their own", true), + (world.MayaId, "a Member", world.MayaId, "their own", true), + (world.MayaId, "a Member", world.ViewerId, "a Viewer's", false), + (world.MiloId, "another Member", world.MayaId, "a Member's", false), + }; + + foreach (var (asker, askerLabel, person, personLabel, accepted) in cases) + { + var credentialId = world.CredentialOf(person); + var init = await InitAsync(world, asker, credentialId, "/settings/credentials").ConfigureAwait(false); + var listed = await ReadListedAsync(asker, world.TeamId, credentialId).ConfigureAwait(false); + + init.StatusCode.ShouldBe(accepted ? HttpStatusCode.OK : HttpStatusCode.Forbidden, $"{askerLabel} reconnecting {personLabel} sign-in: {await TenantHarness.DescribeAsync(init).ConfigureAwait(false)}"); + listed.GetProperty("callerMayRepair").GetBoolean().ShouldBe(accepted, $"the list must say what the endpoint says to {askerLabel} about {personLabel} sign-in"); + } + } + + [Fact] + public async Task A_credential_whose_person_left_is_offered_to_nobody_and_the_endpoint_refuses_it_while_it_waits_to_be_retired() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + var mayas = world.CredentialOf(world.MayaId); + + await _tenants.DbAsync(async db => + { + await db.TeamMembership.Where(m => m.TeamId == world.TeamId && m.UserId == world.MayaId).ExecuteDeleteAsync(); + return 0; + }).ConfigureAwait(false); + + var byAnOwner = await InitAsync(world, world.OliveId, mayas, "/settings/credentials").ConfigureAwait(false); + + byAnOwner.StatusCode.ShouldBe(HttpStatusCode.NotFound, await TenantHarness.DescribeAsync(byAnOwner).ConfigureAwait(false)); + (await ReadListedAsync(world.OliveId, world.TeamId, mayas).ConfigureAwait(false)).GetProperty("callerMayRepair").GetBoolean().ShouldBeFalse("there is no one left to sign in as, so no button is offered to anyone"); + (await _tenants.DbAsync(db => db.OAuthPendingState.AsNoTracking().CountAsync(s => s.TeamId == world.TeamId)).ConfigureAwait(false)).ShouldBe(0); + } + + [Fact] + public async Task A_connect_that_names_no_credential_is_still_an_admins() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + + foreach (var (userId, label, accepted) in new[] { (world.MayaId, "a member", false), (world.ViewerId, "a viewer", false), (world.BossId, "an admin", true) }) + { + var response = await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/oauth/init", userId, world.TeamId, new { providerInstanceId = world.InstanceId, displayName = "GitLab", returnUrl = "/settings/credentials" }).ConfigureAwait(false); + + response.StatusCode.ShouldBe(accepted ? HttpStatusCode.OK : HttpStatusCode.Forbidden, $"{label}: {await TenantHarness.DescribeAsync(response).ConfigureAwait(false)}"); + } + } + + [Fact] + public async Task A_person_who_left_the_team_cannot_reconnect_and_their_retired_credential_stays_retired_for_everyone() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + var mayas = world.CredentialOf(world.MayaId); + + // What a departure leaves: the membership gone, the token blanked, the row soft-deleted. + await _tenants.DbAsync(async db => + { + await db.TeamMembership.Where(m => m.TeamId == world.TeamId && m.UserId == world.MayaId).ExecuteDeleteAsync(); + await db.Credential.Where(c => c.Id == mayas).ExecuteUpdateAsync(s => s.SetProperty(c => c.EncryptedPayload, string.Empty).SetProperty(c => c.Status, CredentialStatus.Revoked).SetProperty(c => c.DeletedDate, DateTimeOffset.UtcNow)); + return 0; + }).ConfigureAwait(false); + + var byTheLeaver = await InitAsync(world, world.MayaId, mayas, "/settings/credentials").ConfigureAwait(false); + var byAnAdmin = await InitAsync(world, world.BossId, mayas, "/settings/credentials").ConfigureAwait(false); + + byTheLeaver.StatusCode.ShouldBe(HttpStatusCode.Forbidden, await TenantHarness.DescribeAsync(byTheLeaver).ConfigureAwait(false)); + byAnAdmin.StatusCode.ShouldBe(HttpStatusCode.Conflict, await TenantHarness.DescribeAsync(byAnAdmin).ConfigureAwait(false)); + JsonDocument.Parse(await byAnAdmin.Content.ReadAsStringAsync().ConfigureAwait(false)).RootElement.GetProperty("code").GetString().ShouldBe("credential_retired"); + (await _tenants.DbAsync(db => db.OAuthPendingState.AsNoTracking().CountAsync(s => s.TeamId == world.TeamId)).ConfigureAwait(false)).ShouldBe(0); + } + + [Fact] + public async Task A_members_reconnect_as_another_provider_account_is_refused_with_its_own_code_and_writes_nothing() + { + // The owner's pass is a pass to re-authorize THEIR credential: the account that signs in must still be the one on record. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedAsync(gitlab).ConfigureAwait(false); + var mayas = world.CredentialOf(world.MayaId); + var before = await ReadCredentialAsync(mayas).ConfigureAwait(false); + var init = await InitAsync(world, world.MayaId, mayas, "/settings/credentials").ConfigureAwait(false); + + var callback = await CallbackAsync(await StateOfAsync(init).ConfigureAwait(false), "authz:someone-else", CookieValue(BindingCookieOf(init)!)).ConfigureAwait(false); + + callback.StatusCode.ShouldBe(HttpStatusCode.Conflict, await TenantHarness.DescribeAsync(callback).ConfigureAwait(false)); + JsonDocument.Parse(await callback.Content.ReadAsStringAsync().ConfigureAwait(false)).RootElement.GetProperty("code").GetString().ShouldBe("credential_account_mismatch"); + (await ReadCredentialAsync(mayas).ConfigureAwait(false)).EncryptedPayload.ShouldBe(before.EncryptedPayload); + } + + // ─── Plumbing ────────────────────────────────────────────────────────────── + + /// A team of an Owner (olive), two Admins (boss, ada), two Members (maya, milo) and a Viewer (vera); a GitLab instance whose OAuth app is the stub's; a dead OAuth credential of maya's, vera's, olive's and ada's, each with a repository bound to it and the act-as identity that names it. + private sealed record World(Guid TeamId, Guid InstanceId, Guid BossId, Guid MayaId, Guid MiloId, Guid ViewerId, Guid OliveId, Guid AdaId, IReadOnlyDictionary Credentials) + { + public int CredentialCount => Credentials.Count; + + public Guid CredentialOf(Guid owner) => Credentials[owner]; + + public (Guid UserId, string Account) PersonOf(TeamRole role) => role == TeamRole.Member ? (MayaId, "maya") : (ViewerId, "vera"); + } + + private sealed record StoredCredential(CredentialStatus Status, string? LastError, string EncryptedPayload, OAuthPayload StoredPair); + + private async Task SeedAsync(GitLabDoorkeeperStub gitlab) + { + var boss = await _tenants.AccountAsync().ConfigureAwait(false); + var maya = await _tenants.AccountAsync().ConfigureAwait(false); + var milo = await _tenants.AccountAsync().ConfigureAwait(false); + var vera = await _tenants.AccountAsync().ConfigureAwait(false); + var olive = await _tenants.AccountAsync().ConfigureAwait(false); + var ada = await _tenants.AccountAsync().ConfigureAwait(false); + var team = await _tenants.TeamAsync((boss, TeamRole.Admin), (maya, TeamRole.Member), (milo, TeamRole.Member), (vera, TeamRole.Viewer), (olive, TeamRole.Owner), (ada, TeamRole.Admin)).ConfigureAwait(false); + + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var encryptor = scope.ServiceProvider.GetRequiredService(); + var serializer = scope.ServiceProvider.GetRequiredService(); + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team, Provider = ProviderKind.GitLab, DisplayName = $"gitlab-{Guid.NewGuid():N}"[..14], BaseUrl = gitlab.BaseUrl, ApiUrl = gitlab.BaseUrl, OauthClientId = GitLabDoorkeeperStub.DefaultClientId, OauthClientSecretEnc = encryptor.Encrypt(GitLabDoorkeeperStub.DefaultClientSecret), CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId }; + var credentials = new Dictionary(); + + db.ProviderInstance.Add(instance); + + foreach (var (userId, account) in new[] { (maya, "maya"), (vera, "vera"), (olive, "olive"), (ada, "ada") }) + { + // A pair GitLab never heard of: the dead grant the sign-in is there to replace. + var payload = new OAuthPayload { AccessToken = $"at-{account}", RefreshToken = $"rt-{account}", ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(-1) }; + var credential = new Credential { Id = Guid.NewGuid(), TeamId = team, ProviderInstanceId = instance.Id, OwnerUserId = userId, Ownership = CredentialOwnership.Personal, AuthType = AuthType.OAuth, DisplayName = $"GitLab · {account}", EncryptedPayload = encryptor.Encrypt(serializer.Serialize(payload)), ExpiresDate = payload.ExpiresAt, Status = CredentialStatus.Error, LastError = "401 Unauthorized", CreatedBy = userId, LastModifiedBy = userId }; + + db.Credential.Add(credential); + db.Repository.Add(new Repository { Id = Guid.NewGuid(), TeamId = team, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ExternalId = Random.Shared.Next(100_000, 999_999).ToString(), NamespacePath = "acme", Name = $"api-{account}", FullPath = $"acme/api-{account}", DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = $"{gitlab.BaseUrl}/acme/api-{account}", CloneUrlHttps = $"{gitlab.BaseUrl}/acme/api-{account}.git", Status = RepositoryStatus.Error, LastError = CredentialRevocationService.DisconnectedRepositoryError }); + db.UserProviderIdentity.Add(new UserProviderIdentity { Id = Guid.NewGuid(), UserId = userId, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ProviderUserId = gitlab.UserIdOf(account).ToString(), ProviderUsername = account }); + credentials[userId] = credential.Id; + } + + await db.SaveChangesAsync().ConfigureAwait(false); + + return new World(team, instance.Id, boss, maya, milo, vera, olive, ada, credentials); + } + + private async Task InitAsync(World world, Guid userId, Guid replaceCredentialId, string returnUrl) => + await _tenants.Client().SendAsync(_tenants.Request(HttpMethod.Post, "/api/credentials/oauth/init", userId, world.TeamId, new { providerInstanceId = world.InstanceId, displayName = "GitLab", returnUrl, replaceCredentialId })); + + private async Task CallbackAsync(string state, string code, string? bindingCookie) + { + var request = _tenants.Request(HttpMethod.Get, $"{CallbackPath}?code={Uri.EscapeDataString(code)}&state={Uri.EscapeDataString(state)}", userId: null, teamId: null); + + if (bindingCookie != null) request.Headers.Add("Cookie", $"{BindingCookie}={bindingCookie}"); + + return await _tenants.Client().SendAsync(request); + } + + private async Task CompleteAsync(Guid userId, Guid teamId, string code, string state) => + await _tenants.Client().SendAsync(_tenants.Request(HttpMethod.Post, CompletePath, userId, teamId, new { code, state })); + + /// The credential as GET /api/credentials says it — what the Providers list renders, with the health derived from the row. + private async Task ReadListedAsync(Guid userId, Guid teamId, Guid credentialId) + { + var response = await _tenants.SendAsync(HttpMethod.Get, "/api/credentials", userId, teamId).ConfigureAwait(false); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(response).ConfigureAwait(false)); + + using var json = JsonDocument.Parse(await response.Content.ReadAsStringAsync().ConfigureAwait(false)); + + return json.RootElement.EnumerateArray().Single(c => c.GetProperty("id").GetGuid() == credentialId).Clone(); + } + + private async Task ReadCredentialAsync(Guid credentialId) + { + var credential = await _tenants.DbAsync(db => db.Credential.AsNoTracking().SingleAsync(c => c.Id == credentialId)).ConfigureAwait(false); + + using var scope = _factory.Services.CreateScope(); + var pair = credential.EncryptedPayload.Length == 0 + ? new OAuthPayload { AccessToken = string.Empty } + : (OAuthPayload)scope.ServiceProvider.GetRequiredService().Deserialize(credential.AuthType, scope.ServiceProvider.GetRequiredService().Decrypt(credential.EncryptedPayload)); + + return new StoredCredential(credential.Status, credential.LastError, credential.EncryptedPayload, pair); + } + + private static async Task StateOfAsync(HttpResponseMessage init) => + JsonDocument.Parse(await init.Content.ReadAsStringAsync()).RootElement.GetProperty("state").GetString()!; + + private static string? BindingCookieOf(HttpResponseMessage response) => + response.Headers.TryGetValues("Set-Cookie", out var values) ? values.FirstOrDefault(c => c.StartsWith($"{BindingCookie}=", StringComparison.Ordinal)) : null; + + private static string CookieValue(string setCookie) => setCookie.Split(';')[0][(BindingCookie.Length + 1)..]; +} diff --git a/backend/tests/CodeSpace.E2ETests/Credentials/ProviderRemedyHttpE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Credentials/ProviderRemedyHttpE2ETests.cs index 44e06fba0..3ba108bfb 100644 --- a/backend/tests/CodeSpace.E2ETests/Credentials/ProviderRemedyHttpE2ETests.cs +++ b/backend/tests/CodeSpace.E2ETests/Credentials/ProviderRemedyHttpE2ETests.cs @@ -22,7 +22,7 @@ namespace CodeSpace.E2ETests.Credentials; /// (ProviderFailureThroughTheEngineFlowTests, with a real GitLab stub), which an HTTP test would only repeat slowly. /// /// Also pinned here: a button is enabled exactly when the endpoint behind it accepts the request (the credential's own Member is -/// offered no Reconnect, and the OAuth init answers them 403), a standing lookup the provider did not answer blames nobody while a row +/// offered Reconnect and the OAuth init starts it; another member is offered none and the init answers 403), a standing lookup the provider did not answer blames nobody while a row /// written before the cause was kept keeps the repair, a run recorded before failures were kept answers exactly as it did (no /// remedy, the same error), and another team's run answers nothing. /// @@ -82,21 +82,24 @@ public async Task The_run_detail_carries_a_remedy_computed_for_whoever_asks_an_o } [Fact] - public async Task A_button_is_enabled_exactly_when_the_endpoint_behind_it_accepts_the_request_for_the_credentials_own_member() + public async Task A_reconnect_button_is_enabled_exactly_when_the_endpoint_behind_it_accepts_the_request() { var world = await SeedWorldAsync(); + await ConfigureOAuthAppAsync(world); var runId = await SeedFailedRunAsync(world, new CredentialNeedsReconnectException(world.CredentialId, "GitLab · nacho", "invalid_grant"), recordFailure: true); + var init = new { providerInstanceId = world.InstanceId, displayName = "GitLab · nacho", replaceCredentialId = world.CredentialId }; - var detail = await _tenants.SendAsync(HttpMethod.Get, $"/api/workflows/runs/{runId}", world.CredentialOwner, world.Team); - var actions = JsonDocument.Parse(await detail.Content.ReadAsStringAsync()).RootElement.GetProperty("remedy").GetProperty("actions").EnumerateArray().ToDictionary(a => a.GetProperty("kind").GetString()!, a => a.GetProperty("enabled").GetBoolean()); + foreach (var (asker, label, accepted) in new[] { (world.CredentialOwner, "the credential's own member", true), (world.Owner, "the team Owner", true), (world.Member, "another member", false), (world.Viewer, "a viewer", false) }) + { + var detail = await _tenants.SendAsync(HttpMethod.Get, $"/api/workflows/runs/{runId}", asker, world.Team); + var actions = JsonDocument.Parse(await detail.Content.ReadAsStringAsync()).RootElement.GetProperty("remedy").GetProperty("actions").EnumerateArray().ToDictionary(a => a.GetProperty("kind").GetString()!, a => a); + var started = await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/oauth/init", asker, world.Team, init); - var init = new { providerInstanceId = world.InstanceId, displayName = "GitLab · nacho", replaceCredentialId = world.CredentialId }; - var asTheCredentialsPerson = await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/oauth/init", world.CredentialOwner, world.Team, init); - var asTheOwner = await _tenants.SendAsync(HttpMethod.Post, "/api/credentials/oauth/init", world.Owner, world.Team, init); + actions["reconnect"].GetProperty("enabled").GetBoolean().ShouldBe(accepted, $"{label}: the card's button"); + started.StatusCode.ShouldBe(accepted ? HttpStatusCode.OK : HttpStatusCode.Forbidden, $"{label}: the endpoint the button calls — {await TenantHarness.DescribeAsync(started)}"); - actions["reconnect"].ShouldBeFalse("the card must not offer the credential's own Member a button …"); - asTheCredentialsPerson.StatusCode.ShouldBe(HttpStatusCode.Forbidden, "… that this endpoint answers 403: it needs credentials.manage, which owning the row does not give"); - asTheOwner.StatusCode.ShouldNotBe(HttpStatusCode.Forbidden, "the team Owner passes authorization (the stub instance has no OAuth client, which is a different refusal)"); + if (!accepted) actions["reconnect"].GetProperty("disabledReason").GetString().ShouldStartWith("Only ", Case.Sensitive, $"{label}: the card says whose it is"); + } } [Fact] @@ -248,6 +251,23 @@ private async Task DeliveriesRemedyAsync(World world) return JsonDocument.Parse(body).RootElement.GetProperty("remedy").Clone(); } + /// The OAuth app a connect needs, so a start that authorization lets in is not then refused for the stub having none. + private async Task ConfigureOAuthAppAsync(World world) + { + using var scope = _factory.Services.CreateScope(); + var encryptor = scope.ServiceProvider.GetRequiredService(); + + await _tenants.DbAsync(async db => + { + var instance = await db.ProviderInstance.SingleAsync(i => i.Id == world.InstanceId); + + instance.OauthClientId = "remedy-app"; + instance.OauthClientSecretEnc = encryptor.Encrypt("remedy-secret"); + await db.SaveChangesAsync(); + return 0; + }); + } + private sealed record World(Guid Team, Guid Owner, Guid Member, Guid Viewer, Guid CredentialOwner, Guid InstanceId, Guid CredentialId, Guid RepositoryId); /// A team of an Owner, a Member and a Viewer; one GitLab connection owned by a fourth person; one repository running through it. diff --git a/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamCapabilityCoverageTests.cs b/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamCapabilityCoverageTests.cs index b18b7a824..438a21d9f 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamCapabilityCoverageTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamCapabilityCoverageTests.cs @@ -123,7 +123,9 @@ public async Task A_viewer_can_still_reach_the_writes_that_only_touch_their_own_ // The mirror of every assertion above: gating is not the goal, correct gating is. A Viewer who // cannot mark a conversation read has been locked out of their own account, not secured. var team = await SeedTeamAsync().ConfigureAwait(false); - var selfService = RequestTypes().Where(t => typeof(IRequireTeamMembership).IsAssignableFrom(t) && !typeof(IRequireTeamPermission).IsAssignableFrom(t)).ToList(); + // An OAuth connect carries its own marker: it is the Admin's to mint a credential, and the credential's own person's to re-authorize. + // The zeroed probe names no credential, so it is the connect, which is not self-service — see GatedWrites. + var selfService = RequestTypes().Where(t => typeof(IRequireTeamMembership).IsAssignableFrom(t) && !typeof(IRequireTeamPermission).IsAssignableFrom(t) && !typeof(IRequireCredentialReauthorization).IsAssignableFrom(t)).ToList(); selfService.ShouldNotBeEmpty("no self-service write was discovered — this assertion would pass vacuously"); @@ -166,6 +168,9 @@ private static IReadOnlyList GatedWrites() var writes = RequestTypes() .Where(t => typeof(IRequireTeamPermission).IsAssignableFrom(t)) .Select(t => new GatedWrite(t, ((IRequireTeamPermission)Probe(t)).RequiredPermission)) + // A connect that may re-authorize a credential is gated on credentials.manage as long as it names none — and a zeroed probe names none. + // Without these the OAuth connect would drop out of "every route to mint a credential is closed to a Viewer" the day it left IRequireTeamPermission. + .Concat(RequestTypes().Where(t => typeof(IRequireCredentialReauthorization).IsAssignableFrom(t)).Select(t => new GatedWrite(t, TeamPermissions.CredentialsManage))) .ToList(); writes.ShouldNotBeEmpty("the reflection scan found no permission-gated writes — every check in this class would pass vacuously"); diff --git a/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamStandingTests.cs b/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamStandingTests.cs index 079615d17..37540f262 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamStandingTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Authorization/TeamStandingTests.cs @@ -68,6 +68,42 @@ public async Task Standing_is_a_membership_row_or_the_instance_admin_role(Person composed.ShouldBe(asked, $"{person}: the predicate a query composes answers exactly as the scalar ask does"); } + [Theory] + [InlineData(Person.Member, TeamRole.Member)] + [InlineData(Person.InstanceAdminOutsideTheTeam, TeamRole.Owner)] + [InlineData(Person.InstanceAdminInTheTeam, TeamRole.Member)] // acted ON, the row they hold is read before the role — the opposite of RoleHeldAsync, where someone ACTING stands as the Owner their role makes them + [InlineData(Person.InstanceAdminOutsideTheTeamWithTheRoleSwitchedOff, null)] + [InlineData(Person.ExInstanceAdminOutsideTheTeam, null)] + [InlineData(Person.DepartedMember, null)] + [InlineData(Person.AnotherTeamsMember, null)] + [InlineData(Person.NoAccount, null)] + public async Task The_role_a_person_is_acted_on_as_exists_exactly_when_they_stand_and_is_the_row_before_the_role(Person person, TeamRole? role) + { + var (teamId, userId) = await SeedAsync(person); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + var actedOn = await TeamStanding.RoleActedOnAsync(db, teamId, userId, CancellationToken.None); + var stands = await TeamStanding.HeldAsync(db, teamId, userId, CancellationToken.None); + + actedOn.ShouldBe(role, $"{person}"); + (actedOn is not null).ShouldBe(stands, $"{person}: someone is acted on as a role exactly when they stand in a live team"); + } + + [Fact] + public async Task Many_people_are_read_in_one_ask_and_only_those_who_stand_are_named() + { + var (teamId, member) = await SeedAsync(Person.Member); + var (_, outsider) = await SeedAsync(Person.NoAccount); + + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + (await TeamStanding.RolesActedOnAsync(db, teamId, new[] { member, outsider, Guid.NewGuid() }, CancellationToken.None)).ShouldBe(new Dictionary { [member] = TeamRole.Member }); + (await TeamStanding.RolesActedOnAsync(db, teamId, Array.Empty(), CancellationToken.None)).ShouldBeEmpty(); + } + // ── World ───────────────────────────────────────────────────────────────────── private async Task<(Guid TeamId, Guid UserId)> SeedAsync(Person person) diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.OwnerReconnect.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.OwnerReconnect.cs new file mode 100644 index 000000000..042bb370b --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.OwnerReconnect.cs @@ -0,0 +1,466 @@ +using Autofac; +using CodeSpace.Core.Authorization; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.OAuth; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Commands.Credentials; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Credentials; + +/// +/// Who may start a reconnect, and who may finish it: the person a credential belongs to re-authorizes it on membership alone, anyone +/// else needs credentials.manage — the bar PUT /api/credentials/{id}/token already applies, so putting a sign-in +/// right is no harder than putting a pasted token right. A connect that names no credential mints one, and keeps its bar. +/// +/// Through the real mediator pipeline and the real callback, against real Postgres. Every other guard of a re-authorization — the +/// same provider account, a retired credential, an owner without standing, the rank reach over the person acted on — is unchanged and +/// pinned in the tests around these; the ones here are the guards that moved with the owner's pass. +/// +public partial class CredentialReauthorizationFlowTests +{ + // ─── The credential's own person ───────────────────────────────────────────────────────────────────────── + + [Theory] + [InlineData(TeamRole.Member)] + [InlineData(TeamRole.Viewer)] + public async Task A_credentials_own_person_reconnects_it_in_place_whatever_their_role(TeamRole role) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var (ownerId, account) = role == TeamRole.Member ? (world.MayaId, "maya") : (world.ViewerId, "viewer"); + var credentialId = await SeedOAuthCredentialAsync(world, ownerId, "at-0", "rt-0", status: CredentialStatus.Error, lastError: "OAuth exchange rejected: invalid_grant").ConfigureAwait(false); + var repositoryId = await SeedRepositoryAsync(world, credentialId, RepositoryStatus.Error).ConfigureAwait(false); + await SeedIdentityAsync(world, ownerId, credentialId, account).ConfigureAwait(false); + + var finished = await ReconnectAsync(world, ownerId, credentialId, $"authz:{account}").ConfigureAwait(false); + + finished.CredentialId.ShouldBe(credentialId, "the credential keeps its id: nothing bound through it moves"); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(1, "no second row was minted"); + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0, "finishing spends the state"); + + var row = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + row.Status.ShouldBe(CredentialStatus.Active); + row.LastError.ShouldBeNull(); + row.LastModifiedBy.ShouldBe(ownerId, "attributed to the person who reconnected"); + (await StoredPairAsync(credentialId).ConfigureAwait(false)).RefreshToken.ShouldBe(gitlab.LiveRefreshTokenOf(account), "the row holds the pair GitLab issued for this sign-in"); + (await LoadRepositoryAsync(repositoryId).ConfigureAwait(false)).ShouldSatisfyAllConditions(r => r.CredentialId.ShouldBe(credentialId), r => r.Status.ShouldBe(RepositoryStatus.Active)); + } + + [Theory] + [InlineData(TeamRole.Member)] + [InlineData(TeamRole.Viewer)] + public async Task A_credentials_own_person_finishes_it_as_the_initiator_when_the_callback_was_handed_to_the_spa(TeamRole role) + { + // The cross-site door: the callback exchanged nothing, and the signed-in starter posts the code to /complete. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var (ownerId, account) = role == TeamRole.Member ? (world.MayaId, "maya") : (world.ViewerId, "viewer"); + var credentialId = await SeedOAuthCredentialAsync(world, ownerId, "at-0", "rt-0", status: CredentialStatus.Error, lastError: "OAuth exchange rejected: invalid_grant").ConfigureAwait(false); + await SeedIdentityAsync(world, ownerId, credentialId, account).ConfigureAwait(false); + var init = await InitAsync(world, ownerId, credentialId).ConfigureAwait(false); + + using var scope = Scope(world, ownerId); + var finished = await scope.Resolve().Send(new CompleteCredentialOAuthAsInitiatorCommand { State = init.State, Code = $"authz:{account}" }).ConfigureAwait(false); + + finished.CredentialId.ShouldBe(credentialId); + (await LoadCredentialAsync(credentialId).ConfigureAwait(false)).Status.ShouldBe(CredentialStatus.Active); + (await CountCredentialsAsync(world).ConfigureAwait(false)).ShouldBe(1); + } + + // ─── Anyone else ───────────────────────────────────────────────────────────────────────────────────────── + + [Theory] + [InlineData(TeamRole.Member)] + [InlineData(TeamRole.Viewer)] + public async Task Someone_who_does_not_manage_credentials_cannot_start_a_reconnect_of_a_teammates_credential(TeamRole role) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var nachos = await SeedOAuthCredentialAsync(world, world.NachoId, "at-n", "rt-n").ConfigureAwait(false); + var asker = role == TeamRole.Member ? world.MayaId : world.ViewerId; + + var refusal = await Should.ThrowAsync(() => InitAsync(world, asker, nachos)).ConfigureAwait(false); + + refusal.ClientMessage.ShouldContain($"Your role on this team is {role}"); + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0, "a refused start leaves no pending connect behind"); + } + + [Fact] + public async Task A_team_service_credential_has_no_owner_to_pass_so_it_stays_the_admins() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var teamToken = await SeedOAuthCredentialAsync(world, null, "at-t", "rt-t", ownership: CredentialOwnership.TeamService).ConfigureAwait(false); + + await Should.ThrowAsync(() => InitAsync(world, world.MayaId, teamToken)).ConfigureAwait(false); + await Should.ThrowAsync(() => InitAsync(world, world.BossId, teamToken)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); + } + + [Fact] + public async Task An_admin_still_reconnects_a_members_credential_and_the_rank_reach_over_a_peer_is_unchanged() + { + // credentials.manage is the permission; reaching the person is the second guard, the one every act on a member keeps: an Admin + // may act on someone below them, never across. Reconnecting in place does not lower it. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var mayas = await SeedOAuthCredentialAsync(world, world.MayaId, "at-m", "rt-m").ConfigureAwait(false); + var nachos = await SeedOAuthCredentialAsync(world, world.NachoId, "at-n", "rt-n").ConfigureAwait(false); + + (await InitAsync(world, world.BossId, mayas).ConfigureAwait(false)).State.ShouldNotBeNullOrEmpty(); + await Should.ThrowAsync(() => InitAsync(world, world.BossId, nachos)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(1, "only the start that was allowed left a pending connect"); + } + + [Fact] + public async Task A_credential_of_another_team_is_not_an_owners_way_past_the_permission() + { + // The owner shortcut is for a credential of THIS team. Maya owns a credential in another team; naming it from hers gets the + // permission check she would have got before — she lacks it — rather than the owner's pass. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var elsewhere = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var foreign = await SeedOAuthCredentialAsync(elsewhere, world.MayaId, "at-f", "rt-f").ConfigureAwait(false); + + await Should.ThrowAsync(() => InitAsync(world, world.MayaId, foreign)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); + } + + // ─── What a connect that names no credential still needs ───────────────────────────────────────────────── + + [Theory] + [InlineData(TeamRole.Member)] + [InlineData(TeamRole.Viewer)] + public async Task A_plain_connect_still_takes_credentials_manage_even_for_oneself(TeamRole role) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var asker = role == TeamRole.Member ? world.MayaId : world.ViewerId; + + await Should.ThrowAsync(() => InitAsync(world, asker, replaceCredentialId: null)).ConfigureAwait(false); + await Should.ThrowAsync(() => InitAsync(world, asker, replaceCredentialId: null, intendedOwnerUserId: asker)).ConfigureAwait(false); + + (await InitAsync(world, world.BossId, replaceCredentialId: null).ConfigureAwait(false)).State.ShouldNotBeNullOrEmpty("an Admin's connect is as it always was"); + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(1); + } + + [Fact] + public async Task An_owner_cannot_use_their_own_reconnect_to_connect_for_someone_else() + { + // The guard stays: a name given with the credential must be the credential's owner. An owner's pass covers re-authorizing + // THEIR credential and nothing a connect could do in another person's name. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var mayas = await SeedOAuthCredentialAsync(world, world.MayaId, "at-m", "rt-m").ConfigureAwait(false); + + await Should.ThrowAsync(() => InitAsync(world, world.MayaId, mayas, intendedOwnerUserId: world.NachoId)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); + } + + // ─── Departed, retired ─────────────────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task An_owner_who_has_left_the_team_cannot_start_a_reconnect_and_an_admin_cannot_start_one_for_them() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var mayas = await SeedOAuthCredentialAsync(world, world.MayaId, "at-m", "rt-m").ConfigureAwait(false); + + await RemoveMembershipAsync(world, world.MayaId).ConfigureAwait(false); + + await Should.ThrowAsync(() => InitAsync(world, world.MayaId, mayas)).ConfigureAwait(false); + await Should.ThrowAsync(() => InitAsync(world, world.BossId, mayas)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); + } + + [Fact] + public async Task A_credential_retired_with_its_person_cannot_be_reconnected_by_them_or_anyone() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var mayas = await SeedOAuthCredentialAsync(world, world.MayaId, "at-m", "rt-m", disconnected: true, retired: true).ConfigureAwait(false); + + // Retired is a refusal of its own, not the permission's: the owner's pass does not reach a row that is no longer live, so the + // permission check speaks first for someone who lacks it, and the retirement for someone who has it. + await Should.ThrowAsync(() => InitAsync(world, world.MayaId, mayas)).ConfigureAwait(false); + await Should.ThrowAsync(() => InitAsync(world, world.BossId, mayas)).ConfigureAwait(false); + + (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); + } + + // ─── The callback asks again ───────────────────────────────────────────────────────────────────────────── + + [Theory] + [InlineData(CompletionDoor.BrowserBound)] + [InlineData(CompletionDoor.Initiator)] + public async Task A_reconnect_an_admin_started_for_someone_else_is_refused_at_the_end_when_they_are_no_longer_an_admin(CompletionDoor door) + { + // The start was allowed on credentials.manage, which is a role: the callback lands up to ten minutes later, and the person who + // started it may have been demoted since. The same bar is asked again — before the code is exchanged — of the starter's + // standing now, so a token is never written under a permission that was taken back. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0", status: CredentialStatus.Error, lastError: "OAuth exchange rejected: invalid_grant").ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + var init = await InitAsync(world, world.BossId, credentialId).ConfigureAwait(false); + var before = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + + await SetRoleAsync(world, world.BossId, TeamRole.Member).ConfigureAwait(false); + + await Should.ThrowAsync(() => FinishAsync(world, init, world.BossId, "authz:maya", door)).ConfigureAwait(false); + + gitlab.Count(StubEvent.Authorized).ShouldBe(0, "the code was never exchanged"); + (await LoadCredentialAsync(credentialId).ConfigureAwait(false)).EncryptedPayload.ShouldBe(before.EncryptedPayload, "nothing was written"); + } + + [Theory] + [InlineData(CompletionDoor.BrowserBound)] + [InlineData(CompletionDoor.Initiator)] + public async Task A_reconnect_an_admin_started_for_someone_else_still_finishes_while_they_remain_an_admin(CompletionDoor door) + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0", status: CredentialStatus.Error, lastError: "OAuth exchange rejected: invalid_grant").ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + var init = await InitAsync(world, world.BossId, credentialId).ConfigureAwait(false); + + var finished = await FinishAsync(world, init, world.BossId, "authz:maya", door).ConfigureAwait(false); + + finished.CredentialId.ShouldBe(credentialId); + (await LoadCredentialAsync(credentialId).ConfigureAwait(false)).Status.ShouldBe(CredentialStatus.Active); + } + + [Theory] + [InlineData(CompletionDoor.BrowserBound)] + [InlineData(CompletionDoor.Initiator)] + public async Task A_reconnect_an_admin_started_for_someone_else_is_refused_at_the_end_when_they_have_since_left_the_team(CompletionDoor door) + { + // Demoting is not the only way to lose the permission: a starter who is no longer in the team at all has no standing to ask the bar + // of, and must not be read as having some. The code is never exchanged and nothing is written. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0", status: CredentialStatus.Error, lastError: "OAuth exchange rejected: invalid_grant").ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + var init = await InitAsync(world, world.BossId, credentialId).ConfigureAwait(false); + var before = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + + await RemoveMembershipAsync(world, world.BossId).ConfigureAwait(false); + + await Should.ThrowAsync(() => FinishAsync(world, init, world.BossId, "authz:maya", door)).ConfigureAwait(false); + + gitlab.Count(StubEvent.Authorized).ShouldBe(0, "the code was never exchanged"); + (await LoadCredentialAsync(credentialId).ConfigureAwait(false)).EncryptedPayload.ShouldBe(before.EncryptedPayload, "nothing was written"); + } + + [Theory] + [InlineData(CompletionDoor.BrowserBound)] + [InlineData(CompletionDoor.Initiator)] + public async Task A_reconnect_an_admin_started_is_refused_at_the_end_when_the_credentials_person_has_since_been_promoted_to_their_rank(CompletionDoor door) + { + // The start was allowed because the person was below the Admin; a reconnect signs in AS them, so the reach is asked again with the roles as + // they are now — two Admins never reach across each other, however the second came to be one. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0", status: CredentialStatus.Error, lastError: "OAuth exchange rejected: invalid_grant").ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + var init = await InitAsync(world, world.BossId, credentialId).ConfigureAwait(false); + var before = await LoadCredentialAsync(credentialId).ConfigureAwait(false); + + await SetRoleAsync(world, world.MayaId, TeamRole.Admin).ConfigureAwait(false); + + await Should.ThrowAsync(() => FinishAsync(world, init, world.BossId, "authz:maya", door)).ConfigureAwait(false); + + gitlab.Count(StubEvent.Authorized).ShouldBe(0, "the code was never exchanged"); + (await LoadCredentialAsync(credentialId).ConfigureAwait(false)).EncryptedPayload.ShouldBe(before.EncryptedPayload, "nothing was written"); + } + + [Fact] + public async Task A_reconnect_an_owner_started_is_not_held_to_a_role_at_the_end_only_to_their_standing() + { + // The owner's pass is membership, so a demotion changes nothing for them: a Viewer who started it finishes it. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var credentialId = await SeedOAuthCredentialAsync(world, world.MayaId, "at-0", "rt-0", status: CredentialStatus.Error, lastError: "OAuth exchange rejected: invalid_grant").ConfigureAwait(false); + await SeedIdentityAsync(world, world.MayaId, credentialId, "maya").ConfigureAwait(false); + var init = await InitAsync(world, world.MayaId, credentialId).ConfigureAwait(false); + + await SetRoleAsync(world, world.MayaId, TeamRole.Viewer).ConfigureAwait(false); + + var finished = await CompleteAsync(world, init, "authz:maya").ConfigureAwait(false); + + finished.CredentialId.ShouldBe(credentialId); + } + + [Fact] + public async Task The_reach_over_a_credentials_person_is_decided_by_the_one_rule_for_every_kind_of_person() + { + // The agreement a button depends on: PermitsReconnect over the role TeamStanding gives the person is exactly what the service answers + // an Admin who starts a reconnect for them — for a member of each role, an instance Admin outside the team (who stands as an Owner), an + // instance Admin who also holds a lower row (the row is read first when acting ON someone, the role first when someone ACTS), and + // someone with no standing at all. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var owner = await SeedMemberAsync(world, TeamRole.Owner).ConfigureAwait(false); + var instanceAdminOutside = await SeedUserAsync().ConfigureAwait(false); + var instanceAdminWithARow = await SeedMemberAsync(world, TeamRole.Member).ConfigureAwait(false); + var outsider = await SeedUserAsync().ConfigureAwait(false); + await GrantInstanceAdminAsync(instanceAdminOutside).ConfigureAwait(false); + await GrantInstanceAdminAsync(instanceAdminWithARow).ConfigureAwait(false); + + var people = new (Guid Id, TeamRole? Standing)[] { (world.MayaId, TeamRole.Member), (world.ViewerId, TeamRole.Viewer), (world.NachoId, TeamRole.Admin), (owner, TeamRole.Owner), (instanceAdminOutside, TeamRole.Owner), (instanceAdminWithARow, TeamRole.Member), (outsider, null) }; + + foreach (var (personId, expectedStanding) in people) + { + var standing = await StandingOfAsync(world, personId).ConfigureAwait(false); + standing.ShouldBe(expectedStanding, $"person {personId}"); + + var credentialId = await SeedOAuthCredentialAsync(world, personId, "at-p", "rt-p").ConfigureAwait(false); + var started = await Record.ExceptionAsync(() => InitAsync(world, world.BossId, credentialId)).ConfigureAwait(false); + + (started is null).ShouldBe(CredentialOwnerOrPermission.PermitsReconnect(TeamRole.Admin, ownsThePersonalCredential: false, standing), $"an Admin starting a reconnect for person {personId} ({expectedStanding?.ToString() ?? "no standing"}): {started?.GetType().Name ?? "accepted"}"); + } + } + + [Fact] + public async Task A_team_that_is_gone_gives_nobody_a_standing_to_be_acted_on_as() + { + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var instanceAdmin = await SeedUserAsync().ConfigureAwait(false); + await GrantInstanceAdminAsync(instanceAdmin).ConfigureAwait(false); + + (await StandingOfAsync(world, world.MayaId).ConfigureAwait(false)).ShouldBe(TeamRole.Member, "fixture check: Maya stands in the live team"); + + await SoftDeleteTeamAsync(world).ConfigureAwait(false); + + (await StandingOfAsync(world, world.MayaId).ConfigureAwait(false)).ShouldBeNull("the service's hold refuses a team that is gone, so nobody is acted on in it"); + (await StandingOfAsync(world, instanceAdmin).ConfigureAwait(false)).ShouldBeNull(); + } + + private async Task StandingOfAsync(World world, Guid personId) + { + using var scope = _fixture.BeginScope(); + + return await TeamStanding.RoleActedOnAsync(scope.Resolve(), world.TeamId, personId, CancellationToken.None).ConfigureAwait(false); + } + + private async Task SoftDeleteTeamAsync(World world) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().Team.Where(t => t.Id == world.TeamId).ExecuteUpdateAsync(s => s.SetProperty(t => t.DeletedDate, DateTimeOffset.UtcNow)).ConfigureAwait(false); + } + + // ─── The pure rule and TeamStanding's role agree with what the pipeline decides ─────────────────────────── + + [Fact] + public async Task The_role_a_standing_amounts_to_is_the_one_the_membership_resolver_gives_the_caller() + { + // The callback is anonymous, so it cannot ask the resolver, which answers for the caller. TeamStanding answers for anyone; the two + // must never disagree, or the start of a reconnect and its end would judge the same person differently. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); + var outsider = await SeedUserAsync().ConfigureAwait(false); + var instanceAdmin = await SeedUserAsync().ConfigureAwait(false); + var adminWithARowBelowOwner = await SeedMemberAsync(world, TeamRole.Member).ConfigureAwait(false); + await GrantInstanceAdminAsync(instanceAdmin).ConfigureAwait(false); + await GrantInstanceAdminAsync(adminWithARowBelowOwner).ConfigureAwait(false); + + var everyone = new[] { world.NachoId, world.MayaId, world.BossId, world.ViewerId, outsider, instanceAdmin, adminWithARowBelowOwner }; + + await AssertTheyAgreeAsync(world, everyone, "in a live team").ConfigureAwait(false); + + (await RoleAsCallerAsync(world, instanceAdmin).ConfigureAwait(false)).ShouldBe(TeamRole.Owner, "fixture check: an instance Admin stands as an Owner in every team"); + (await RoleAsCallerAsync(world, adminWithARowBelowOwner).ConfigureAwait(false)).ShouldBe(TeamRole.Owner, "fixture check: and outranks the row they also hold"); + (await RoleAsCallerAsync(world, outsider).ConfigureAwait(false)).ShouldBeNull("fixture check: no row, no role, no standing"); + + await SoftDeleteTeamAsync(world).ConfigureAwait(false); + + // A team that is gone denies every member whatever rows outlive it, and only an instance Admin still stands — in this one as in every team. + await AssertTheyAgreeAsync(world, everyone, "in a deleted team").ConfigureAwait(false); + + (await RoleAsCallerAsync(world, world.MayaId).ConfigureAwait(false)).ShouldBeNull("fixture check: a member is denied once their team is deleted"); + } + + private async Task AssertTheyAgreeAsync(World world, IEnumerable users, string when) + { + foreach (var userId in users) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var asked = await TeamStanding.RoleHeldAsync(db, world.TeamId, userId, CancellationToken.None).ConfigureAwait(false); + + var asCaller = await RoleAsCallerAsync(world, userId).ConfigureAwait(false); + + asked.ShouldBe(asCaller, $"user {userId} {when}"); + } + } + + public enum CompletionDoor + { + /// The provider's redirect, in the browser that started the connect. + BrowserBound, + + /// The SPA's completion page, for a callback that came back without the binding cookie. + Initiator + } + + private async Task FinishAsync(World world, InitCredentialOAuthResult init, Guid starter, string code, CompletionDoor door) + { + if (door == CompletionDoor.BrowserBound) return await CompleteAsync(world, init, code).ConfigureAwait(false); + + using var scope = Scope(world, starter); + + return await scope.Resolve().Send(new CompleteCredentialOAuthAsInitiatorCommand { State = init.State, Code = code }).ConfigureAwait(false); + } + + private async Task SetRoleAsync(World world, Guid userId, TeamRole role) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().TeamMembership.Where(m => m.TeamId == world.TeamId && m.UserId == userId).ExecuteUpdateAsync(s => s.SetProperty(m => m.Role, role)).ConfigureAwait(false); + } + + private async Task GrantInstanceAdminAsync(Guid userId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var roleId = await db.Role.AsNoTracking().Where(r => r.Name == CodeSpace.Messages.Constants.Roles.Admin).Select(r => r.Id).SingleAsync().ConfigureAwait(false); + + db.RoleUser.Add(new CodeSpace.Core.Persistence.Entities.RoleUser { Id = Guid.NewGuid(), RoleId = roleId, UserId = userId }); + await db.SaveChangesAsync().ConfigureAwait(false); + } + + /// What the pipeline's resolver says the person is in this team when they are the caller — null when it refuses them. + private async Task RoleAsCallerAsync(World world, Guid userId) + { + using var scope = Scope(world, userId, configure: null, await RolesOfAsync(userId).ConfigureAwait(false)); + + try + { + return await scope.Resolve().ResolveRoleAsync(world.TeamId, CancellationToken.None).ConfigureAwait(false); + } + catch (TenantAccessDeniedException) + { + return null; + } + } + + private async Task RolesOfAsync(Guid userId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + return await db.RoleUser.AsNoTracking().Where(ru => ru.UserId == userId).Join(db.Role.AsNoTracking(), ru => ru.RoleId, r => r.Id, (ru, r) => r.Name).ToArrayAsync().ConfigureAwait(false); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.cs index aa4178a44..cff8b973c 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/CredentialReauthorizationFlowTests.cs @@ -328,14 +328,14 @@ public async Task A_credential_of_another_provider_instance_a_team_service_crede } [Fact] - public async Task Starting_a_reconnect_takes_credentials_manage_whoever_the_owner_is() + public async Task Starting_a_reconnect_takes_credentials_manage_unless_the_credential_is_the_askers_own() { - // The start of any OAuth connect is the Admin's (the marker on the command); naming a credential does not lower it. + // The start of any OAuth connect is the Admin's (the marker on the command); naming a credential lowers it to the credential's + // own person — and to no one else, whatever their role (the owner's own start is pinned in the OwnerReconnect tests). using var gitlab = new GitLabDoorkeeperStub(); var world = await SeedWorldAsync(gitlab).ConfigureAwait(false); var mayas = await SeedOAuthCredentialAsync(world, world.MayaId, "at-m", "rt-m").ConfigureAwait(false); - await Should.ThrowAsync(() => InitAsync(world, world.MayaId, mayas)).ConfigureAwait(false); await Should.ThrowAsync(() => InitAsync(world, world.ViewerId, mayas)).ConfigureAwait(false); (await CountPendingStatesAsync(world).ConfigureAwait(false)).ShouldBe(0); diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/ProviderFailureThroughTheEngineFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/ProviderFailureThroughTheEngineFlowTests.cs index 596124d29..fec36e6be 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Credentials/ProviderFailureThroughTheEngineFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/ProviderFailureThroughTheEngineFlowTests.cs @@ -50,6 +50,7 @@ public async Task A_dead_oauth_grant_is_recorded_on_the_failed_node_and_the_reme using var gitlab = new GitLabDoorkeeperStub(); // never heard of the pair the row holds: the refresh is answered invalid_grant var team = await WorkflowsTestSeed.SeedTeamAsync(_fixture); var seed = await GitLabCredentialSeed.SeedAsync(_fixture, gitlab.BaseUrl, "at-0", "rt-0", expired: true, existingTeam: team); + await RecordThePersonItWasConnectedForAsync(seed.CredentialId, team.UserId); var runId = await RunListPullRequestsAsync(team, seed.RepositoryId); @@ -364,6 +365,17 @@ private async Task ReadRecordedFailureAsync(Guid runId) return remedy; } + /// + /// What a connect always does and the shared seed leaves out: a sign-in is somebody's, so the credential names its person. A reconnect re-authorizes a + /// person's own connection — it refuses one with none — so the button the remedy shows for it is only honest on a row shaped as a connect shapes it. + /// + private async Task RecordThePersonItWasConnectedForAsync(Guid credentialId, Guid userId) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"UPDATE credential SET owner_user_id = {userId} WHERE id = {credentialId}"); + } + private async Task DisconnectAsync(Guid credentialId) { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.IntegrationTests/Credentials/ProviderRemedyFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Credentials/ProviderRemedyFlowTests.cs index f65c5c3a9..470aafd23 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Credentials/ProviderRemedyFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Credentials/ProviderRemedyFlowTests.cs @@ -10,6 +10,7 @@ using CodeSpace.IntegrationTests.Workflows.Infrastructure; using CodeSpace.Messages.Agents; using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Credentials; using CodeSpace.Messages.Dtos.Credentials; using CodeSpace.Messages.Enums; using CodeSpace.Messages.Exceptions; @@ -25,8 +26,8 @@ namespace CodeSpace.IntegrationTests.Credentials; /// written by the real from the real exceptions, so the shapes cannot drift from what the engine /// writes; the engine's own capture of them is . /// -/// Pins: the per-role matrix (an Owner, an Admin, a Member, a Viewer, and the credential's own person — who may replace their own -/// pasted token but not reconnect a sign-in, because that endpoint takes credentials.manage), a retired credential, a failure traced through +/// Pins: the per-role matrix (an Owner, an Admin, a Member, a Viewer, and the credential's own person — who may reconnect their own +/// sign-in or replace their own pasted token on membership alone, the one rule both endpoints apply, agreed with the real commands), a retired credential, a failure traced through /// the run's scope, an agent run's own account (only the LAST one, and only when the node that ended the run is the one that spawned it), a /// run whose handled provider failure is not why it ended, a push the remote refused on a run that otherwise finished, a run of another /// team, a redirect inside one origin (no address to edit), a base URL carrying userinfo (masked), and the refused-delivery remedy a @@ -68,8 +69,8 @@ public async Task The_same_failure_reads_differently_to_each_role_and_the_ones_w return; } - remedy.Actions.Single(a => a.Kind == RemedyActionKind.Reconnect).DisabledReason.ShouldBe("Needs the Admin or Owner role.", "the sign-in flow is a credentials.manage request whoever the credential belongs to"); - remedy.AskPeople.Select(p => p.Role).ShouldBe(new[] { "Owner", "Admin" }, "the credential's own person cannot reconnect it either, so only the team's Owners and Admins are named"); + remedy.Actions.Single(a => a.Kind == RemedyActionKind.Reconnect).DisabledReason.ShouldBe($"Only {world.CredentialOwnerName}, or a team Admin, can do this.", "someone else's sign-in is the credentials.manage bar, the same as someone else's pasted token"); + remedy.AskPeople.Select(p => p.Role).ShouldBe(new[] { "Owner of the credential", "Owner", "Admin" }, "the person whose sign-in it is can reconnect it, then the team's Owners and Admins"); remedy.AskPeople.ShouldAllBe(p => p.UserId != asker, "nobody is told to ask themselves"); } @@ -113,23 +114,176 @@ public async Task The_credentials_own_person_can_replace_their_pasted_token_as_a } [Fact] - public async Task The_credentials_own_person_is_not_offered_a_reconnect_of_their_oauth_sign_in_as_a_plain_member_because_the_endpoint_would_refuse_it() + public async Task The_credentials_own_person_is_offered_the_reconnect_of_their_oauth_sign_in_as_a_plain_member_and_the_endpoint_accepts_it() { var world = await SeedCredentialWorldAsync(owner: TeamRole.Member); + await ConfigureOAuthAppAsync(world.InstanceId); var runId = await SeedFailedRunAsync(world, new CredentialNeedsReconnectException(world.CredentialId, "GitLab · nacho", "invalid_grant")); var remedy = (await RemedyAsync(world.CredentialOwnerId, world.TeamId, runId))!; var reconnect = remedy.Actions.Single(a => a.Kind == RemedyActionKind.Reconnect); - reconnect.Enabled.ShouldBeFalse("POST /api/credentials/oauth/init needs credentials.manage"); - reconnect.DisabledReason.ShouldBe("Needs the Admin or Owner role."); - remedy.AskPeople.Select(p => p.Role).ShouldBe(new[] { "Owner", "Admin" }); + reconnect.Enabled.ShouldBeTrue("a person re-authorizes their own credential on membership alone"); + reconnect.DisabledReason.ShouldBeNull(); + remedy.Actions.Single(a => a.Kind == RemedyActionKind.Relink).Enabled.ShouldBeFalse("re-linking a repository is still the Admin's"); - // The probe the button's state stands for: the same asker sending the same command is refused by the same rule. + // The probe the button's state stands for: the same asker sending the same command is accepted by the same rule. using var scope = _fixture.BeginScopeAs(world.CredentialOwnerId, world.TeamId); - var refused = await Should.ThrowAsync(() => scope.Resolve().Send(new CodeSpace.Messages.Commands.Credentials.InitCredentialOAuthCommand { ProviderInstanceId = world.InstanceId, DisplayName = "GitLab · nacho", ReplaceCredentialId = world.CredentialId })); + var started = await scope.Resolve().Send(new CodeSpace.Messages.Commands.Credentials.InitCredentialOAuthCommand { ProviderInstanceId = world.InstanceId, DisplayName = "GitLab · nacho", ReplaceCredentialId = world.CredentialId }); + + started.State.ShouldNotBeNullOrEmpty(); + } + + /// + /// Every way a person can stand toward a credential: the asker's role, whether the credential is theirs (then the person's role is the + /// asker's own) or someone else's with a role of its own, and whether that person has since left the team without the credential + /// being retired yet. Each for a sign-in and a pasted token. + /// + public static TheoryData RepairMatrix() + { + var roles = new[] { TeamRole.Owner, TeamRole.Admin, TeamRole.Member, TeamRole.Viewer }; + var data = new TheoryData(); + + foreach (var authType in new[] { AuthType.OAuth, AuthType.Pat }) + foreach (var asker in roles) + { + data.Add(asker, true, asker, false, authType); + + foreach (var person in roles) data.Add(asker, false, person, false, authType); + + data.Add(asker, false, TeamRole.Member, true, authType); + } + + return data; + } + + [Theory] + [MemberData(nameof(RepairMatrix))] + public async Task A_repair_button_is_enabled_exactly_when_the_endpoint_behind_it_accepts_that_asker(TeamRole askerRole, bool owns, TeamRole personRole, bool personLeft, AuthType authType) + { + // The agreement the remedy exists to keep, measured against the real commands: what the remedy decides for a person is what the + // pipeline and the service answer that same person, and the Providers list says the same. Accepted means the command RETURNED — any + // exception, of any kind, is a refusal — so a guard the button does not know about cannot read as "accepted". + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedCredentialWorldAsync(owner: owns ? askerRole : personRole, authType, gitlab); + var asker = owns ? world.CredentialOwnerId : await AddMemberAsync(world.TeamId, askerRole, $"Asker {askerRole}"); + var runId = await SeedFailedRunAsync(world, new CredentialNeedsReconnectException(world.CredentialId, "GitLab · nacho", "invalid_grant")); + + if (personLeft) await RemoveMembershipAsync(world.TeamId, world.CredentialOwnerId); + + var remedy = (await RemedyAsync(asker, world.TeamId, runId))!; + var kind = authType == AuthType.OAuth ? RemedyActionKind.Reconnect : RemedyActionKind.ReplaceToken; + var offered = remedy.Actions.Single(a => a.Kind == kind).Enabled; + + var listed = (await ListedAsync(asker, world.TeamId)).Single(c => c.Id == world.CredentialId).CallerMayRepair; + + var accepted = await AcceptedByTheEndpointAsync(asker, world, authType); + + var who = $"a {askerRole} who {(owns ? "owns" : "does not own")} a credential whose person {(personLeft ? "has left" : $"is a {personRole}")}"; + offered.ShouldBe(accepted, $"the {kind} button for {who} must be enabled exactly when its endpoint accepts them"); + listed.ShouldBe(accepted, $"the Providers list, which the repository rows read, must say the same of {who}"); + } + + private async Task> ListedAsync(Guid asker, Guid teamId) + { + using var scope = _fixture.BeginScopeAs(asker, teamId); + + return await scope.Resolve().ListAsync(null, CancellationToken.None); + } + + [Fact] + public async Task An_admin_looking_at_an_owners_sign_in_is_told_it_is_the_owners_alone_and_is_not_sent_to_ask_a_peer() + { + var world = await SeedCredentialWorldAsync(owner: TeamRole.Owner); + var admin = await AddMemberAsync(world.TeamId, TeamRole.Admin, "Asker Admin"); + var runId = await SeedFailedRunAsync(world, new CredentialNeedsReconnectException(world.CredentialId, "GitLab · nacho", "invalid_grant")); + + var remedy = (await RemedyAsync(admin, world.TeamId, runId))!; + var reconnect = remedy.Actions.Single(a => a.Kind == RemedyActionKind.Reconnect); - refused.Message.ShouldContain("credentials.manage"); + reconnect.Enabled.ShouldBeFalse(); + reconnect.DisabledReason.ShouldBe($"Only {world.CredentialOwnerName} can reconnect their own sign-in."); + remedy.Actions.Single(a => a.Kind == RemedyActionKind.Relink).Enabled.ShouldBeTrue("an Admin can still move the repository off it"); + remedy.AskPeople.Select(p => p.UserId).ShouldBe(new[] { world.CredentialOwnerId }, "the Admin who is asking and the Admin beside them cannot sign in as an Owner, so only the person is named"); + remedy.AskPeople.Single().Role.ShouldBe("Owner of the credential"); + } + + [Fact] + public async Task A_peer_admins_sign_in_is_the_persons_or_an_owners_and_only_they_are_named() + { + var world = await SeedCredentialWorldAsync(owner: TeamRole.Admin); + var admin = await AddMemberAsync(world.TeamId, TeamRole.Admin, "Asker Admin"); + var runId = await SeedFailedRunAsync(world, new CredentialNeedsReconnectException(world.CredentialId, "GitLab · nacho", "invalid_grant")); + + var remedy = (await RemedyAsync(admin, world.TeamId, runId))!; + + remedy.Actions.Single(a => a.Kind == RemedyActionKind.Reconnect).DisabledReason.ShouldBe($"Only {world.CredentialOwnerName}, or a team Owner, can reconnect this sign-in."); + remedy.AskPeople.Select(p => (p.UserId, p.Role)).ShouldBe(new[] { (world.CredentialOwnerId, "Owner of the credential"), (world.OwnerId, "Owner") }, "Ada, the other Admin, could not sign in as a peer any more than the asker: only the person and the team's Owner can"); + } + + [Theory] + [InlineData(AuthType.OAuth)] + [InlineData(AuthType.Pat)] + public async Task A_credential_whose_person_left_is_nobodys_to_repair_and_names_nobody_for_it(AuthType authType) + { + var world = await SeedCredentialWorldAsync(owner: TeamRole.Member, authType); + var member = await AddMemberAsync(world.TeamId, TeamRole.Member, "Asker Member"); + var runId = await SeedFailedRunAsync(world, new CredentialNeedsReconnectException(world.CredentialId, "GitLab · nacho", "invalid_grant")); + await RemoveMembershipAsync(world.TeamId, world.CredentialOwnerId); + + var remedy = (await RemedyAsync(member, world.TeamId, runId))!; + var repair = remedy.Actions.Single(a => a.Kind is RemedyActionKind.Reconnect or RemedyActionKind.ReplaceToken); + + repair.DisabledReason.ShouldBe($"{world.CredentialOwnerName} has left this team, so this cannot be repaired."); + remedy.Actions.Select(a => a.Kind).First().ShouldBe(RemedyActionKind.Relink, "moving off it is the way out, as for a retired credential"); + remedy.AskPeople.Select(p => p.UserId).ShouldNotContain(world.CredentialOwnerId, "someone who left cannot be asked"); + remedy.AskPeople.Select(p => p.Role).ShouldBe(new[] { "Owner", "Admin" }, "the Admins who can re-link, which a Member cannot"); + } + + [Fact] + public async Task A_credential_whose_person_left_but_is_not_retired_yet_reads_as_owner_left_beside_the_failure() + { + // A 401 on a credential still marked Active: nothing records that it is refused, so what the card says about it is what the list would. + var world = await SeedCredentialWorldAsync(owner: TeamRole.Member); + await SetStatusAsync(world.CredentialId, CredentialStatus.Active); + var runId = await SeedFailedRunAsync(world, new ProviderApiException(ProviderKind.GitLab, 401, "ListPullRequestsAsync", "401", new Exception("x"))); + await RemoveMembershipAsync(world.TeamId, world.CredentialOwnerId); + + var remedy = (await RemedyAsync(world.OwnerId, world.TeamId, runId))!; + + remedy.Credential!.Health.ShouldBe(CredentialHealth.OwnerLeft); + (await ListedAsync(world.OwnerId, world.TeamId)).Single(c => c.Id == world.CredentialId).Health.ShouldBe(CredentialHealth.OwnerLeft, "the list and the card read the same row alike"); + } + + [Fact] + public async Task A_team_service_sign_in_is_refused_to_an_admin_by_the_endpoint_and_the_button_says_so_without_blaming_their_role() + { + // Not something the product mints — a connect always yields a Personal credential — but a row the server refuses to reconnect all the same. + using var gitlab = new GitLabDoorkeeperStub(); + var world = await SeedCredentialWorldAsync(owner: TeamRole.Member, AuthType.OAuth, gitlab); + await MakeTeamServiceAsync(world.CredentialId); + var admin = await AddMemberAsync(world.TeamId, TeamRole.Admin, "Asker Admin"); + var runId = await SeedFailedRunAsync(world, new CredentialNeedsReconnectException(world.CredentialId, "GitLab · nacho", "invalid_grant")); + + var reconnect = (await RemedyAsync(admin, world.TeamId, runId))!.Actions.Single(a => a.Kind == RemedyActionKind.Reconnect); + + reconnect.Enabled.ShouldBeFalse(); + reconnect.DisabledReason.ShouldBe("A team's own credential has no one's sign-in to reconnect; replace its token instead."); + (await AcceptedByTheEndpointAsync(admin, world, AuthType.OAuth)).ShouldBeFalse(); + (await ListedAsync(admin, world.TeamId)).Single(c => c.Id == world.CredentialId).CallerMayRepair.ShouldBe(false); + } + + /// The command the button runs, as the asker sends it: true when it returns, false for any refusal at all. + private async Task AcceptedByTheEndpointAsync(Guid asker, World world, AuthType authType) + { + using var scope = _fixture.BeginScopeAs(asker, world.TeamId); + var mediator = scope.Resolve(); + + var thrown = await Record.ExceptionAsync(() => authType == AuthType.OAuth + ? mediator.Send(new CodeSpace.Messages.Commands.Credentials.InitCredentialOAuthCommand { ProviderInstanceId = world.InstanceId, DisplayName = "GitLab · nacho", ReplaceCredentialId = world.CredentialId }) + : mediator.Send(new CodeSpace.Messages.Commands.Credentials.ReplaceCredentialTokenCommand { CredentialId = world.CredentialId, Token = FreshToken })); + + return thrown is null; } [Fact] @@ -419,8 +573,15 @@ public async Task The_credential_list_returns_every_state_a_person_may_need_to_r private sealed record World(Guid TeamId, Guid OwnerId, Guid CredentialOwnerId, string CredentialOwnerName, Guid InstanceId, Guid CredentialId, Guid RepositoryId); - /// A team with an Owner, an Admin, and a person ('s role) who owns the one credential, which a repository runs through. - private async Task SeedCredentialWorldAsync(TeamRole owner, AuthType authType = AuthType.OAuth) + /// The token a replace is sent, which the stub knows as the credential's own account's. + private const string FreshToken = "glpat-fresh"; + + /// + /// A team with an Owner, an Admin, and a person ('s role) who owns the one credential, which a repository runs + /// through. Given a , the provider connection is that stub's — with its OAuth app, the person's account on it and + /// as a token of theirs — so a connect or a token replace the remedy offers can be sent for real and succeed. + /// + private async Task SeedCredentialWorldAsync(TeamRole owner, AuthType authType = AuthType.OAuth, GitLabDoorkeeperStub? gitlab = null) { var team = await WorkflowsTestSeed.SeedTeamAsync(_fixture); var admin = await AddMemberAsync(team.TeamId, TeamRole.Admin, "Ada Admin"); @@ -429,11 +590,22 @@ private async Task SeedCredentialWorldAsync(TeamRole owner, AuthType auth using var scope = _fixture.BeginScope(); var db = scope.Resolve(); + var encryptor = scope.Resolve(); var suffix = Guid.NewGuid().ToString("N")[..8]; - var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.TeamId, Provider = ProviderKind.GitLab, DisplayName = "gitlab", BaseUrl = $"http://127.0.0.1:1/{suffix}", ApiUrl = $"http://127.0.0.1:1/{suffix}" }; + var address = gitlab?.BaseUrl ?? $"http://127.0.0.1:1/{suffix}"; + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.TeamId, Provider = ProviderKind.GitLab, DisplayName = "gitlab", BaseUrl = address, ApiUrl = address }; var credential = new Credential { Id = Guid.NewGuid(), TeamId = team.TeamId, ProviderInstanceId = instance.Id, OwnerUserId = nacho, Ownership = CredentialOwnership.Personal, AuthType = authType, DisplayName = "GitLab · nacho", EncryptedPayload = "x", Status = CredentialStatus.Error, LastError = "needs reconnect: invalid_grant" }; var repository = NewRepository(team.TeamId, instance.Id, credential.Id, "acme/api"); + if (gitlab is not null) + { + instance.OauthClientId = GitLabDoorkeeperStub.DefaultClientId; + instance.OauthClientSecretEnc = encryptor.Encrypt(GitLabDoorkeeperStub.DefaultClientSecret); + credential.EncryptedPayload = encryptor.Encrypt(scope.Resolve().Serialize(authType == AuthType.OAuth ? new OAuthPayload { AccessToken = "at-0", RefreshToken = "rt-0", ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(-1) } : new PatPayload { Token = "glpat-expired" })); + gitlab.SeedPersonalAccessToken("nacho", FreshToken); + db.UserProviderIdentity.Add(new UserProviderIdentity { Id = Guid.NewGuid(), UserId = nacho, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ProviderUserId = gitlab.UserIdOf("nacho").ToString(), ProviderUsername = "nacho" }); + } + db.ProviderInstance.Add(instance); db.Credential.Add(credential); db.Repository.Add(repository); @@ -444,6 +616,25 @@ private async Task SeedCredentialWorldAsync(TeamRole owner, AuthType auth return new World(team.TeamId, team.UserId, nacho, nachoName, instance.Id, credential.Id, repository.Id); } + private async Task RemoveMembershipAsync(Guid teamId, Guid userId) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().TeamMembership.Where(m => m.TeamId == teamId && m.UserId == userId).ExecuteDeleteAsync(); + } + + /// Gives the provider connection the OAuth app a connect needs, so a start that is let in by authorization is not then refused for having none. + private async Task ConfigureOAuthAppAsync(Guid instanceId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var instance = await db.ProviderInstance.SingleAsync(i => i.Id == instanceId); + + instance.OauthClientId = "remedy-app"; + instance.OauthClientSecretEnc = scope.Resolve().Encrypt("remedy-secret"); + await db.SaveChangesAsync(); + } + private async Task NameOfAsync(Guid userId) { using var scope = _fixture.BeginScope(); @@ -484,6 +675,20 @@ private async Task SetBaseUrlAsync(Guid instanceId, string baseUrl) await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"UPDATE provider_instance SET base_url = {baseUrl} WHERE id = {instanceId}"); } + private async Task SetStatusAsync(Guid credentialId, CredentialStatus status) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"UPDATE credential SET status = {status.ToString()}, last_error = NULL WHERE id = {credentialId}"); + } + + private async Task MakeTeamServiceAsync(Guid credentialId) + { + using var scope = _fixture.BeginScope(); + + await scope.Resolve().Database.ExecuteSqlInterpolatedAsync($"UPDATE credential SET ownership = 'TeamService', owner_user_id = NULL WHERE id = {credentialId}"); + } + private async Task RetireAsync(Guid credentialId) { using var scope = _fixture.BeginScope(); diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/TeamWritePermissionAdoptionTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/TeamWritePermissionAdoptionTests.cs index 6bc39d144..f0f8d3953 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/TeamWritePermissionAdoptionTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/TeamWritePermissionAdoptionTests.cs @@ -40,6 +40,7 @@ public void Every_team_write_declares_a_permission_or_is_self_service() var ungated = TeamWriteCommands() .Where(t => !typeof(IRequireTeamPermission).IsAssignableFrom(t)) .Where(t => !typeof(IRequireCredentialManagement).IsAssignableFrom(t)) + .Where(t => !typeof(IRequireCredentialReauthorization).IsAssignableFrom(t)) .Where(t => !SelfServiceByDesign.ContainsKey(t.Name)) .Select(t => t.FullName!) .OrderBy(n => n, StringComparer.Ordinal) @@ -79,6 +80,19 @@ public void Revoking_a_credential_takes_credentials_manage_unless_it_is_the_call typeof(IRequireCredentialManagement).IsAssignableFrom(type!).ShouldBeTrue("revoking a credential must be gated by IRequireCredentialManagement, not by its resource marker alone"); } + [Fact] + public void Starting_an_oauth_connect_takes_credentials_manage_unless_it_re_authorizes_the_callers_own_credential() + { + // A plain connect mints a credential, which is credentials.manage's. Naming a credential to re-authorize in place is the + // management rule's instead: its own person on membership alone, anyone else credentials.manage. The marker carries both + // bars, so the command must not also declare the permission — that would demand it of an owner reconnecting their own. + var type = TeamWriteCommands().SingleOrDefault(t => t.Name == "InitCredentialOAuthCommand"); + + type.ShouldNotBeNull("InitCredentialOAuthCommand is no longer seen as a team write — the scan below must keep covering the OAuth connect"); + typeof(IRequireCredentialReauthorization).IsAssignableFrom(type!).ShouldBeTrue("starting a connect must be gated by IRequireCredentialReauthorization, which holds both the connect's bar and the owner's"); + typeof(IRequireTeamPermission).IsAssignableFrom(type!).ShouldBeFalse("a plain IRequireTeamPermission would close the owner's own reconnect to everyone below Admin"); + } + [Fact] public void The_self_service_list_does_not_rot() { diff --git a/backend/tests/CodeSpace.UnitTests/Credentials/CredentialOwnerOrPermissionTests.cs b/backend/tests/CodeSpace.UnitTests/Credentials/CredentialOwnerOrPermissionTests.cs new file mode 100644 index 000000000..cf21cc50a --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Credentials/CredentialOwnerOrPermissionTests.cs @@ -0,0 +1,70 @@ +using CodeSpace.Core.Authorization; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.UnitTests.Credentials; + +/// +/// The one definition of "the credential's own person, or someone holding the permission" — what the credential pipeline tiers +/// enforce, what the provider remedy's CallerStanding decides its buttons by, and what the OAuth callback asks again of the +/// person who started a reconnect. Pure, so the whole table is pinned here; the pipeline's and the callback's use of it is pinned +/// against real rows in the integration tier. +/// +/// A reconnect adds the rank reach: it acts on a PERSON as well as on a credential, so holding the permission is not enough to +/// reconnect the sign-in of someone of one's own rank or above — the guard every act on a member keeps. +/// +[Trait("Category", "Unit")] +public class CredentialOwnerOrPermissionTests +{ + [Theory] + // role, owns the Personal credential, permission, permitted + [InlineData(TeamRole.Owner, false, TeamPermissions.CredentialsManage, true)] + [InlineData(TeamRole.Admin, false, TeamPermissions.CredentialsManage, true)] + [InlineData(TeamRole.Member, false, TeamPermissions.CredentialsManage, false)] + [InlineData(TeamRole.Viewer, false, TeamPermissions.CredentialsManage, false)] + [InlineData(TeamRole.Owner, true, TeamPermissions.CredentialsManage, true)] + [InlineData(TeamRole.Admin, true, TeamPermissions.CredentialsManage, true)] + [InlineData(TeamRole.Member, true, TeamPermissions.CredentialsManage, true)] + [InlineData(TeamRole.Viewer, true, TeamPermissions.CredentialsManage, true)] + [InlineData(TeamRole.Member, false, TeamPermissions.ReposManage, false)] + [InlineData(TeamRole.Member, true, TeamPermissions.ReposManage, true)] + [InlineData(TeamRole.Admin, false, TeamPermissions.ReposManage, true)] + [InlineData(TeamRole.Viewer, true, TeamPermissions.ReposManage, true)] + public void A_credentials_own_person_passes_on_membership_and_anyone_else_needs_the_permission(TeamRole role, bool owns, string permission, bool permitted) + { + CredentialOwnerOrPermission.Permits(role, owns, permission).ShouldBe(permitted); + } + + [Theory] + // role of whoever reconnects, whether the sign-in is theirs, the role its person stands as in the team, permitted + [InlineData(TeamRole.Owner, false, TeamRole.Admin, true)] + [InlineData(TeamRole.Owner, false, TeamRole.Member, true)] + [InlineData(TeamRole.Owner, false, TeamRole.Viewer, true)] + [InlineData(TeamRole.Owner, false, TeamRole.Owner, false)] // never across, and nobody ranks above an Owner + [InlineData(TeamRole.Admin, false, TeamRole.Member, true)] + [InlineData(TeamRole.Admin, false, TeamRole.Viewer, true)] + [InlineData(TeamRole.Admin, false, TeamRole.Admin, false)] // two Admins who could reconnect each other's sign-ins turn a disagreement into a race + [InlineData(TeamRole.Admin, false, TeamRole.Owner, false)] + [InlineData(TeamRole.Member, false, TeamRole.Viewer, false)] // credentials.manage is the permission, and a Member lacks it whoever the person is + [InlineData(TeamRole.Viewer, false, TeamRole.Viewer, false)] + [InlineData(TeamRole.Owner, true, TeamRole.Owner, true)] // yourself is always in reach of yourself + [InlineData(TeamRole.Admin, true, TeamRole.Admin, true)] + [InlineData(TeamRole.Member, true, TeamRole.Member, true)] + [InlineData(TeamRole.Viewer, true, TeamRole.Viewer, true)] + public void A_reconnect_needs_the_permission_or_the_credential_and_reach_over_the_person(TeamRole role, bool owns, TeamRole person, bool permitted) + { + CredentialOwnerOrPermission.PermitsReconnect(role, owns, person).ShouldBe(permitted); + } + + [Theory] + [InlineData(TeamRole.Owner, false)] + [InlineData(TeamRole.Admin, false)] + [InlineData(TeamRole.Member, true)] + [InlineData(TeamRole.Viewer, true)] + public void A_sign_in_whose_person_has_no_standing_in_the_team_is_reconnected_by_nobody(TeamRole role, bool owns) + { + // Someone who left has no one to sign in as, and a team's own credential has no person at all: the service refuses both, so no role does. + CredentialOwnerOrPermission.PermitsReconnect(role, owns, ownerStanding: null).ShouldBeFalse(); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Credentials/CredentialPersonTests.cs b/backend/tests/CodeSpace.UnitTests/Credentials/CredentialPersonTests.cs new file mode 100644 index 000000000..b6a183468 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Credentials/CredentialPersonTests.cs @@ -0,0 +1,56 @@ +using CodeSpace.Core.Authorization; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.UnitTests.Credentials; + +/// +/// Who a credential belongs to, as the rules about acting on it see them — built once, from the row and what the team says about its +/// person, by everything that decides a repair: the provider remedy, the credential list and (through ) +/// the OAuth callback. Pure, so the shapes are pinned here. +/// +[Trait("Category", "Unit")] +public class CredentialPersonTests +{ + private static readonly Guid Caller = Guid.NewGuid(); + private static readonly Guid Nacho = Guid.NewGuid(); + + [Fact] + public void A_team_service_credential_has_no_person() + { + CredentialPerson.Of(CredentialOwnership.TeamService, ownerUserId: null, Caller, new Dictionary()).ShouldBeNull(); + } + + [Fact] + public void A_personal_credential_stored_with_no_owner_has_no_person_to_ask_about() + { + CredentialPerson.Of(CredentialOwnership.Personal, ownerUserId: null, Caller, new Dictionary()).ShouldBeNull(); + } + + [Fact] + public void A_personal_credential_is_its_persons_and_carries_the_role_they_stand_as() + { + var person = CredentialPerson.Of(CredentialOwnership.Personal, Nacho, Caller, new Dictionary { [Nacho] = TeamRole.Admin })!; + + person.IsCaller.ShouldBeFalse(); + person.Standing.ShouldBe(TeamRole.Admin); + } + + [Fact] + public void The_caller_is_the_person_of_their_own_credential() + { + CredentialPerson.Of(CredentialOwnership.Personal, Caller, Caller, new Dictionary { [Caller] = TeamRole.Viewer })!.IsCaller.ShouldBeTrue(); + } + + [Fact] + public void A_person_the_team_knows_nothing_about_has_left_and_stands_as_no_one() + { + CredentialPerson.Of(CredentialOwnership.Personal, Nacho, Caller, new Dictionary { [Guid.NewGuid()] = TeamRole.Member })!.Standing.ShouldBeNull(); + } + + [Fact] + public void Nobody_is_the_caller_of_a_request_that_has_no_caller() + { + CredentialPerson.Of(CredentialOwnership.Personal, Nacho, callerId: null, new Dictionary { [Nacho] = TeamRole.Member })!.IsCaller.ShouldBeFalse(); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Credentials/Remedy/RemedyActionsTests.cs b/backend/tests/CodeSpace.UnitTests/Credentials/Remedy/RemedyActionsTests.cs index fcf255bd5..58009187a 100644 --- a/backend/tests/CodeSpace.UnitTests/Credentials/Remedy/RemedyActionsTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Credentials/Remedy/RemedyActionsTests.cs @@ -1,4 +1,5 @@ using System.Linq; +using CodeSpace.Core.Authorization; using CodeSpace.Core.Services.Credentials.Remedy; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Dtos.Credentials; @@ -26,24 +27,68 @@ public class RemedyActionsTests private static RemedyAction Of(IReadOnlyList actions, RemedyActionKind kind) => actions.Single(a => a.Kind == kind); + /// The caller's standing toward a credential whose person is a Member — below an Admin, so the rank reach over them never speaks unless a test says whose it is. A credential's own person stands as the role they are asking in. + private static CallerStanding Standing(TeamRole role, bool ownsTheCredential, TeamRole person = TeamRole.Member) => CallerStanding.Of(role, new CredentialPerson(ownsTheCredential, ownsTheCredential ? role : person)); + [Theory] - // role, owns the credential, may re-link, may reconnect (OAuth init), may replace a token, may launch runs - [InlineData(TeamRole.Owner, false, true, true, true, true)] - [InlineData(TeamRole.Admin, false, true, true, true, true)] - [InlineData(TeamRole.Member, false, false, false, false, true)] - [InlineData(TeamRole.Member, true, false, false, true, true)] // POST /api/credentials/oauth/init needs credentials.manage whoever owns the row; only the token replace passes an owner on membership alone - [InlineData(TeamRole.Viewer, false, false, false, false, false)] - [InlineData(TeamRole.Viewer, true, false, false, true, false)] // a Viewer's own token is still theirs to replace — and their sign-in is not theirs to reconnect - public void The_caller_standing_follows_the_rules_of_the_endpoints_behind_the_buttons(TeamRole role, bool owns, bool relink, bool reconnect, bool replace, bool launch) + // role, owns the credential, the role of the credential's person, may re-link, may replace its token, may reconnect its sign-in, may launch runs + [InlineData(TeamRole.Owner, false, TeamRole.Member, true, true, true, true)] + [InlineData(TeamRole.Owner, false, TeamRole.Admin, true, true, true, true)] + [InlineData(TeamRole.Owner, false, TeamRole.Owner, true, true, false, true)] // reconnecting acts on the PERSON: never across, never above + [InlineData(TeamRole.Admin, false, TeamRole.Member, true, true, true, true)] + [InlineData(TeamRole.Admin, false, TeamRole.Admin, true, true, false, true)] + [InlineData(TeamRole.Admin, false, TeamRole.Owner, true, true, false, true)] // while replacing a pasted token has no such reach: the endpoint asks only credentials.manage + [InlineData(TeamRole.Member, false, TeamRole.Member, false, false, false, true)] + [InlineData(TeamRole.Member, true, TeamRole.Member, false, true, true, true)] // a credential's own person repairs it on membership alone, either way + [InlineData(TeamRole.Viewer, false, TeamRole.Member, false, false, false, false)] + [InlineData(TeamRole.Viewer, true, TeamRole.Viewer, false, true, true, false)] // a Viewer's own credential is still theirs to put right + [InlineData(TeamRole.Admin, true, TeamRole.Admin, true, true, true, true)] // an Admin reaches themselves + [InlineData(TeamRole.Owner, true, TeamRole.Owner, true, true, true, true)] + public void The_caller_standing_follows_the_rules_of_the_endpoints_behind_the_buttons(TeamRole role, bool owns, TeamRole person, bool relink, bool replace, bool reconnect, bool launch) { - var standing = CallerStanding.Of(role, owns); + var standing = Standing(role, owns, person); standing.MayManageRepositories.ShouldBe(relink); - standing.MayReconnectTheCredential.ShouldBe(reconnect); standing.MayReplaceTheToken.ShouldBe(replace); + standing.MayReconnectTheCredential.ShouldBe(reconnect); standing.MayLaunchRuns.ShouldBe(launch); } + [Theory] + [InlineData(TeamRole.Owner)] + [InlineData(TeamRole.Admin)] + public void A_person_who_left_the_team_is_nobodys_to_repair_not_even_a_managers(TeamRole role) + { + // The services refuse both: a token cannot be put into a credential whose person has no standing, and nobody can sign in as them. + var standing = CallerStanding.Of(role, new CredentialPerson(IsCaller: false, Standing: null)); + + standing.MayReplaceTheToken.ShouldBeFalse(); + standing.MayReconnectTheCredential.ShouldBeFalse(); + standing.MayManageRepositories.ShouldBeTrue("moving what leans on it to another credential is still the Admin's"); + } + + [Theory] + [InlineData(TeamRole.Owner, true, false)] + [InlineData(TeamRole.Admin, true, false)] + [InlineData(TeamRole.Member, false, false)] + public void A_team_service_credential_has_a_token_to_replace_and_no_sign_in_to_reconnect(TeamRole role, bool replace, bool reconnect) + { + var standing = CallerStanding.Of(role, person: null); + + standing.MayReplaceTheToken.ShouldBe(replace); + standing.MayReconnectTheCredential.ShouldBe(reconnect); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Repairing_a_credential_means_what_its_kind_means_and_the_two_answers_come_from_one_standing(bool staticToken) + { + var standing = Standing(TeamRole.Admin, ownsTheCredential: false, person: TeamRole.Admin); // may replace a token, may not reconnect a peer's sign-in + + standing.MayRepair(staticToken).ShouldBe(staticToken); + } + [Theory] [InlineData(FailureCodes.CredentialDisconnected)] [InlineData(FailureCodes.CredentialNeedsReconnect)] @@ -51,7 +96,7 @@ public void The_caller_standing_follows_the_rules_of_the_endpoints_behind_the_bu [InlineData(FailureCodes.GitCredentialRejected)] public void An_owner_is_offered_every_credential_fix_enabled_and_a_live_credential_is_repaired_in_place_first(string code) { - var actions = RemedyActions.Build(code, Facts(), CallerStanding.Of(TeamRole.Owner, ownsTheCredential: false), Everything); + var actions = RemedyActions.Build(code, Facts(), Standing(TeamRole.Owner, ownsTheCredential: false), Everything); actions.Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.Reconnect, RemedyActionKind.Relink, RemedyActionKind.OpenProviders, RemedyActionKind.Rerun }, "repairing in place moves nothing, so it leads; re-linking is the fallback"); actions.Where(a => a.Kind != RemedyActionKind.Rerun).ShouldAllBe(a => a.Enabled); @@ -60,7 +105,7 @@ public void An_owner_is_offered_every_credential_fix_enabled_and_a_live_credenti [Fact] public void A_pasted_token_is_replaced_rather_than_reconnected() { - var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(staticToken: true), CallerStanding.Of(TeamRole.Owner, false), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(staticToken: true), Standing(TeamRole.Owner, false), Everything); actions.Select(a => a.Kind).ShouldContain(RemedyActionKind.ReplaceToken); actions.Select(a => a.Kind).ShouldNotContain(RemedyActionKind.Reconnect); @@ -69,7 +114,7 @@ public void A_pasted_token_is_replaced_rather_than_reconnected() [Fact] public void A_retired_credential_cannot_be_reconnected_so_re_linking_is_all_there_is() { - var actions = RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(retired: true), CallerStanding.Of(TeamRole.Owner, false), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(retired: true), Standing(TeamRole.Owner, false), Everything); actions.Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.Relink, RemedyActionKind.OpenProviders, RemedyActionKind.Rerun }); } @@ -77,13 +122,13 @@ public void A_retired_credential_cannot_be_reconnected_so_re_linking_is_all_ther [Fact] public void A_member_who_does_not_own_the_credential_sees_the_fix_disabled_with_who_can_do_it() { - var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(), CallerStanding.Of(TeamRole.Member, ownsTheCredential: false), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(), Standing(TeamRole.Member, ownsTheCredential: false), Everything); var reconnect = Of(actions, RemedyActionKind.Reconnect); reconnect.Enabled.ShouldBeFalse(); - reconnect.DisabledReason.ShouldBe("Needs the Admin or Owner role.", "the sign-in flow is a credentials.manage request whoever the credential belongs to"); + reconnect.DisabledReason.ShouldBe("Only Nacho, or a team Admin, can do this.", "someone else's sign-in is the credentials.manage bar, the same as someone else's pasted token"); - var replace = Of(RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(staticToken: true), CallerStanding.Of(TeamRole.Member, ownsTheCredential: false), Everything), RemedyActionKind.ReplaceToken); + var replace = Of(RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(staticToken: true), Standing(TeamRole.Member, ownsTheCredential: false), Everything), RemedyActionKind.ReplaceToken); replace.Enabled.ShouldBeFalse(); replace.DisabledReason.ShouldBe("Only Nacho, or a team Admin, can do this."); @@ -97,7 +142,7 @@ public void A_member_who_does_not_own_the_credential_sees_the_fix_disabled_with_ [Fact] public void A_member_whose_own_pasted_token_it_is_can_replace_it_but_not_re_link() { - var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(staticToken: true), CallerStanding.Of(TeamRole.Member, ownsTheCredential: true), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(staticToken: true), Standing(TeamRole.Member, ownsTheCredential: true), Everything); Of(actions, RemedyActionKind.ReplaceToken).Enabled.ShouldBeTrue("the token replace endpoint passes the credential's own person on membership alone"); Of(actions, RemedyActionKind.Relink).Enabled.ShouldBeFalse(); @@ -106,27 +151,78 @@ public void A_member_whose_own_pasted_token_it_is_can_replace_it_but_not_re_link [Theory] [InlineData(TeamRole.Member)] [InlineData(TeamRole.Viewer)] - public void A_non_admin_whose_own_oauth_sign_in_it_is_is_not_offered_a_reconnect_that_the_endpoint_would_refuse(TeamRole role) + public void A_credentials_own_person_is_offered_the_reconnect_of_their_own_sign_in_whatever_their_role(TeamRole role) { - var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(), CallerStanding.Of(role, ownsTheCredential: true), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(), Standing(role, ownsTheCredential: true), Everything); var reconnect = Of(actions, RemedyActionKind.Reconnect); - reconnect.Enabled.ShouldBeFalse("InitCredentialOAuthCommand requires credentials.manage; the owner-on-membership rule belongs to the token replace only"); - reconnect.DisabledReason.ShouldBe("Needs the Admin or Owner role."); + reconnect.Enabled.ShouldBeTrue("the owner of a Personal credential re-authorizes it on membership alone"); + reconnect.DisabledReason.ShouldBeNull(); } [Fact] - public void A_team_service_credential_names_the_role_that_can_repair_it_not_a_person() + public void An_admin_is_told_an_owners_sign_in_is_the_owners_alone_while_their_pasted_token_stays_the_admins_to_replace() + { + var facts = Facts() with { OwnerRole = TeamRole.Owner }; + var reconnect = Of(RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, facts, Standing(TeamRole.Admin, false, TeamRole.Owner), Everything), RemedyActionKind.Reconnect); + + reconnect.Enabled.ShouldBeFalse("an Admin does not reach an Owner, and a sign-in is signing in AS the person"); + reconnect.DisabledReason.ShouldBe("Only Nacho can reconnect their own sign-in.", "naming the Admin role would send an Admin to ask themselves"); + + var replace = Of(RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, facts with { CredentialIsStaticToken = true }, Standing(TeamRole.Admin, false, TeamRole.Owner), Everything), RemedyActionKind.ReplaceToken); + + replace.Enabled.ShouldBeTrue("the token replace endpoint asks credentials.manage and nothing about the person's rank"); + } + + [Fact] + public void A_peer_admins_sign_in_is_the_admins_own_or_a_team_owners() + { + var facts = Facts() with { OwnerRole = TeamRole.Admin }; + var reconnect = Of(RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, facts, Standing(TeamRole.Admin, false, TeamRole.Admin), Everything), RemedyActionKind.Reconnect); + + reconnect.Enabled.ShouldBeFalse(); + reconnect.DisabledReason.ShouldBe("Only Nacho, or a team Owner, can reconnect this sign-in."); + Of(RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, facts, Standing(TeamRole.Owner, false, TeamRole.Admin), Everything), RemedyActionKind.Reconnect).Enabled.ShouldBeTrue("an Owner outranks an Admin"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void A_credential_whose_person_left_is_repaired_by_nobody_says_so_and_leads_with_moving_off_it(bool staticToken) + { + var facts = Facts(staticToken: staticToken) with { CredentialOwnerLeft = true }; + var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, facts, CallerStanding.Of(TeamRole.Owner, new CredentialPerson(IsCaller: false, Standing: null)), Everything); + + var repair = Of(actions, staticToken ? RemedyActionKind.ReplaceToken : RemedyActionKind.Reconnect); + repair.Enabled.ShouldBeFalse(); + repair.DisabledReason.ShouldBe("Nacho has left this team, so this cannot be repaired."); + actions.Select(a => a.Kind).First().ShouldBe(RemedyActionKind.Relink, "like a retired credential, the one way out is to move what leans on it"); + } + + [Fact] + public void A_team_service_sign_in_that_an_admin_cannot_reconnect_is_not_blamed_on_their_role() + { + var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(teamService: true), CallerStanding.Of(TeamRole.Admin, person: null), Everything); + + var reconnect = Of(actions, RemedyActionKind.Reconnect); + reconnect.Enabled.ShouldBeFalse(); + reconnect.DisabledReason.ShouldBe("A team's own credential has no one's sign-in to reconnect; replace its token instead."); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void A_team_service_credential_names_the_role_that_can_repair_it_not_a_person(bool staticToken) { - var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(staticToken: true, teamService: true), CallerStanding.Of(TeamRole.Member, false), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialNeedsReconnect, Facts(staticToken: staticToken, teamService: true), Standing(TeamRole.Member, false), Everything); - Of(actions, RemedyActionKind.ReplaceToken).DisabledReason.ShouldBe("Needs the Admin or Owner role."); + Of(actions, staticToken ? RemedyActionKind.ReplaceToken : RemedyActionKind.Reconnect).DisabledReason.ShouldBe("Needs the Admin or Owner role."); } [Fact] public void A_viewer_gets_nothing_enabled_but_the_providers_dialog() { - var actions = RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(), CallerStanding.Of(TeamRole.Viewer, false), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(), Standing(TeamRole.Viewer, false), Everything); actions.Where(a => a.Enabled).Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.OpenProviders }); Of(actions, RemedyActionKind.Rerun).DisabledReason.ShouldBe("Fix the connection first, then run it again."); @@ -136,11 +232,11 @@ public void A_viewer_gets_nothing_enabled_but_the_providers_dialog() public void A_redirect_is_fixed_by_editing_the_providers_address_not_by_touching_a_credential() { var facts = Facts() with { RedirectFix = RedirectRepair.EditAddress }; - var actions = RemedyActions.Build(FailureCodes.ProviderBaseUrlRedirects, facts, CallerStanding.Of(TeamRole.Admin, false), Everything); + var actions = RemedyActions.Build(FailureCodes.ProviderBaseUrlRedirects, facts, Standing(TeamRole.Admin, false), Everything); actions.Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.EditProviderUrl, RemedyActionKind.OpenProviders, RemedyActionKind.Rerun }); Of(actions, RemedyActionKind.EditProviderUrl).Enabled.ShouldBeTrue(); - RemedyActions.Build(FailureCodes.ProviderBaseUrlRedirects, facts, CallerStanding.Of(TeamRole.Member, false), Everything).Single(a => a.Kind == RemedyActionKind.EditProviderUrl).Enabled.ShouldBeFalse(); + RemedyActions.Build(FailureCodes.ProviderBaseUrlRedirects, facts, Standing(TeamRole.Member, false), Everything).Single(a => a.Kind == RemedyActionKind.EditProviderUrl).Enabled.ShouldBeFalse(); } [Theory] @@ -148,7 +244,7 @@ public void A_redirect_is_fixed_by_editing_the_providers_address_not_by_touching [InlineData(RedirectRepair.HostSide)] public void A_redirect_the_server_would_refuse_to_edit_away_offers_no_edit_button_at_all(RedirectRepair fix) { - var actions = RemedyActions.Build(FailureCodes.ProviderBaseUrlRedirects, Facts() with { RedirectFix = fix }, CallerStanding.Of(TeamRole.Owner, false), Everything); + var actions = RemedyActions.Build(FailureCodes.ProviderBaseUrlRedirects, Facts() with { RedirectFix = fix }, Standing(TeamRole.Owner, false), Everything); actions.Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.OpenProviders, RemedyActionKind.Rerun }, "a button that the endpoint answers 409 to is not a fix; the cause says what is"); } @@ -156,17 +252,17 @@ public void A_redirect_the_server_would_refuse_to_edit_away_offers_no_edit_butto [Fact] public void Once_what_failed_works_again_running_it_again_leads_and_is_enabled_for_whoever_may_launch() { - var actions = RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(resolved: true), CallerStanding.Of(TeamRole.Member, false), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(resolved: true), Standing(TeamRole.Member, false), Everything); actions[0].Kind.ShouldBe(RemedyActionKind.Rerun); actions[0].Enabled.ShouldBeTrue(); - RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(resolved: true), CallerStanding.Of(TeamRole.Viewer, false), Everything)[0].DisabledReason.ShouldBe("Needs the Member role or above."); + RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(resolved: true), Standing(TeamRole.Viewer, false), Everything)[0].DisabledReason.ShouldBe("Needs the Member role or above."); } [Fact] public void A_provider_401_on_a_row_that_looks_fine_leaves_run_it_again_open_because_nobody_can_show_the_fix() { - var actions = RemedyActions.Build(FailureCodes.ProviderUnauthorized, Facts(observable: false), CallerStanding.Of(TeamRole.Member, false), Everything); + var actions = RemedyActions.Build(FailureCodes.ProviderUnauthorized, Facts(observable: false), Standing(TeamRole.Member, false), Everything); Of(actions, RemedyActionKind.Rerun).Enabled.ShouldBeTrue(); } @@ -175,8 +271,8 @@ public void A_provider_401_on_a_row_that_looks_fine_leaves_run_it_again_open_bec public void A_failure_that_says_nothing_about_the_credential_sends_nobody_to_repair_it() { // A renewal the provider did not answer (a final refusal would have been credential_needs_reconnect), and a standing lookup that got no answer. - var renewal = RemedyActions.Build(FailureCodes.OAuthExchangeFailed, Facts(observable: false), CallerStanding.Of(TeamRole.Owner, false), Everything); - var lookup = RemedyActions.Build(WorkflowRunRequestRejectionReasons.AuthorStandingUnknown, Facts() with { Lookup = StandingLookupCauses.NoAnswer }, CallerStanding.Of(TeamRole.Owner, false), Everything with { OffersRerun = false }); + var renewal = RemedyActions.Build(FailureCodes.OAuthExchangeFailed, Facts(observable: false), Standing(TeamRole.Owner, false), Everything); + var lookup = RemedyActions.Build(WorkflowRunRequestRejectionReasons.AuthorStandingUnknown, Facts() with { Lookup = StandingLookupCauses.NoAnswer }, Standing(TeamRole.Owner, false), Everything with { OffersRerun = false }); renewal.Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.OpenProviders, RemedyActionKind.Rerun }, "run it again; reconnecting a credential that was not refused is what to try only if that keeps failing"); Of(renewal, RemedyActionKind.Rerun).Enabled.ShouldBeTrue("nothing is recorded that could show it fixed, so it is not held back"); @@ -188,7 +284,7 @@ public void A_standing_lookup_with_no_credential_to_ask_with_offers_to_link_one_ { var subjects = new RemedyActions.ActionSubjects(HasCredential: false, HasRepositories: true, HasProviderInstance: true, OffersRerun: false); - RemedyActions.Build(WorkflowRunRequestRejectionReasons.AuthorStandingUnknown, Facts() with { Lookup = StandingLookupCauses.NoCredential }, CallerStanding.Of(TeamRole.Owner, false), subjects).Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.Relink, RemedyActionKind.OpenProviders }); + RemedyActions.Build(WorkflowRunRequestRejectionReasons.AuthorStandingUnknown, Facts() with { Lookup = StandingLookupCauses.NoCredential }, Standing(TeamRole.Owner, false), subjects).Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.Relink, RemedyActionKind.OpenProviders }); } [Fact] @@ -196,7 +292,7 @@ public void A_refused_trigger_delivery_has_no_run_to_run_again() { var subjects = Everything with { OffersRerun = false }; - RemedyActions.Build(WorkflowRunRequestRejectionReasons.CredentialRejected, Facts(), CallerStanding.Of(TeamRole.Owner, false), subjects).Select(a => a.Kind).ShouldNotContain(RemedyActionKind.Rerun); + RemedyActions.Build(WorkflowRunRequestRejectionReasons.CredentialRejected, Facts(), Standing(TeamRole.Owner, false), subjects).Select(a => a.Kind).ShouldNotContain(RemedyActionKind.Rerun); } [Fact] @@ -204,13 +300,13 @@ public void Actions_with_nothing_to_act_on_are_not_offered() { var subjects = new RemedyActions.ActionSubjects(HasCredential: false, HasRepositories: false, HasProviderInstance: false, OffersRerun: true); - RemedyActions.Build(FailureCodes.ProviderUnauthorized, Facts(), CallerStanding.Of(TeamRole.Owner, false), subjects).Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.OpenProviders, RemedyActionKind.Rerun }); + RemedyActions.Build(FailureCodes.ProviderUnauthorized, Facts(), Standing(TeamRole.Owner, false), subjects).Select(a => a.Kind).ShouldBe(new[] { RemedyActionKind.OpenProviders, RemedyActionKind.Rerun }); } [Fact] public void Several_repositories_make_the_re_link_say_so() { - var actions = RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(retired: true) with { RepositoryCount = 3 }, CallerStanding.Of(TeamRole.Admin, false), Everything); + var actions = RemedyActions.Build(FailureCodes.CredentialDisconnected, Facts(retired: true) with { RepositoryCount = 3 }, Standing(TeamRole.Admin, false), Everything); Of(actions, RemedyActionKind.Relink).Label.ShouldBe("Re-link repositories"); } diff --git a/backend/tests/TESTING.md b/backend/tests/TESTING.md index 968237aa0..e69807896 100644 --- a/backend/tests/TESTING.md +++ b/backend/tests/TESTING.md @@ -69,8 +69,10 @@ Provider-credential tests share two more pieces, both in `CodeSpace.IntegrationT `git.pr_review`, `git.create_issue`, `git.close_issue`, `git.comment_issue`), and asserts what the node's ledger record carries and what the remedy says. A repair the remedy offers is proved against the real command the button runs: the redirect test sends `UpdateProviderInstanceCommand` (refused while a token is connected, accepted once it is disconnected) and `InitCredentialOAuthCommand` - is sent as the credential's own Member to show the button's enabled state is the endpoint's answer — never an edit by SQL that - production would refuse. `ProviderRemedyFlowTests` pins the per-role matrix and every other source of a failure (a run that failed on + is sent as the credential's own Member, and as a teammate, to show the button's enabled state is the endpoint's answer (the credential's + own person passes on membership, anyone else needs `credentials.manage`; `CredentialReauthorizationFlowTests.OwnerReconnect` and + `CredentialOwnerReconnectE2ETests` pin the rest of who may start a reconnect and who may finish it) — never an edit by SQL that + production would refuse. `ProviderRemedyFlowTests` pins the per-role matrix — and, for every asker role against every role the credential's person can have (or has left the team), that the remedy's button, the credential list's `callerMayRepair` and the real command (a connect, or a token replace against the stub) all agree, a command counting as accepted only if it returns and as refused for any exception at all — and every other source of a failure (a run that failed on another node than the one that handled a provider failure has none; a standing lookup that got no answer blames nobody); `ProviderRemedyHttpE2ETests` pins the wire shape for an Owner, a Member and a Viewer. Those fixtures write the failure with the real `IRunRecordLogger` from the real exception through `ProviderFailureCapture`, never a hand-typed payload. diff --git a/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx b/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx index 1f2386746..9feaf668c 100644 --- a/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx +++ b/frontend/src/_imported/ai-code-space/connect-remote-modal.tsx @@ -390,9 +390,10 @@ function ProvidersStep({ providers, loading, error, myCredsByInstance, teamServi // which role holds it. const { can } = useTeamPermissions(); const mayAddProvider = can(TeamPermissions.ReposManage); - // Team tokens are nobody's own, so adding or revoking one takes credentials.manage, and so does starting an OAuth + // Team tokens are nobody's own, so adding or revoking one takes credentials.manage, and so does starting a NEW OAuth // connect (the server's init declares it) — unlike Disconnect on the Personal tab, which only ever lists the - // caller's own credentials, and "Use a token", which links a personal token: both stay theirs on membership alone. + // caller's own credentials, "Use a token", which links a personal token, and Reconnect, which re-authorizes one of those + // same credentials in place: all stay theirs on membership alone (the server's init passes a credential's own person). const mayManageCredentials = can(TeamPermissions.CredentialsManage); return ( <> @@ -451,7 +452,7 @@ function ProvidersStep({ providers, loading, error, myCredsByInstance, teamServi revokingId={revokingId} errorMsg={errors[instance.id]} onConnect={mayManageCredentials ? onConnect : null} - onReconnect={mayManageCredentials ? onReconnect : null} + onReconnect={onReconnect} onReplaceToken={onReplaceToken} onConnectPat={onConnectPat} onDisconnect={onDisconnect} @@ -506,8 +507,8 @@ interface ProviderRowProps { errorMsg: string | undefined; /** Null when the caller may not start an OAuth connect — the button is absent rather than present-and-refused. */ onConnect: ((instance: ProviderInstanceSummary) => void) | null; - /** Null when the caller may not start an OAuth connect — Reconnect starts one, so it is absent too. */ - onReconnect: ((instance: ProviderInstanceSummary, credential: CredentialSummary) => void) | null; + /** Reconnect re-authorizes one of the caller's own credentials in place, so it needs no more than membership — the row lists only theirs. */ + onReconnect: (instance: ProviderInstanceSummary, credential: CredentialSummary) => void; onReplaceToken: (credential: CredentialSummary, instance: ProviderInstanceSummary) => void; onConnectPat: (instance: ProviderInstanceSummary) => void; onDisconnect: (credential: CredentialSummary) => void; @@ -573,7 +574,7 @@ function ProviderRow({ instance, theme, myCreds, isConnecting, revokingId, error {/* Sign in again to the SAME credential (an OAuth sign-in), or paste a new token into it (a token): either way it keeps its id, so the repositories bound through it keep working. */} - {c.authType === "OAuth" && instance.oauthEnabled && onReconnect && ( + {c.authType === "OAuth" && instance.oauthEnabled && ( diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 00e629553..f96156cee 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -53,6 +53,13 @@ export interface CredentialSummary { daysToExpiry?: number | null; /** How many live repositories are bound through it — what reconnecting, replacing or disconnecting it puts at stake. Absent from a server that predates it; read it as zero. */ boundRepositoryCount?: number; + /** + * Whether the caller may put it right in place — reconnect its sign-in or replace its pasted token — by the rules the endpoints behind those + * buttons apply: its own person, or `credentials.manage`, and for a sign-in also the reach over its person (an Admin cannot reconnect another + * Admin's or an Owner's), and never once its person left. The server's answer, so a button is shown exactly when it would be accepted. Absent + * from a server that predates it: read the role then (`ownsIt || can(CredentialsManage)`), which is what every client did before. + */ + callerMayRepair?: boolean | null; } export interface InitOAuthResponse { diff --git a/frontend/src/components/connect-remote-modal.test.tsx b/frontend/src/components/connect-remote-modal.test.tsx index 55600484f..85b9f6947 100644 --- a/frontend/src/components/connect-remote-modal.test.tsx +++ b/frontend/src/components/connect-remote-modal.test.tsx @@ -13,9 +13,9 @@ import { ConnectRemoteModal } from "@/_imported/ai-code-space/connect-remote-mod * beyond membership. The modal must offer exactly that: a Member keeps Disconnect on the Personal tab — which lists * only their own credentials — and sees team tokens without a Revoke that could only answer 403. * - *

The same for connecting: adding a team token and starting an OAuth connect take credentials.manage, and editing - * or removing a provider takes repos.manage. Linking a personal access token takes only membership, so "Use a token" - * stays for everyone.

+ *

The same for connecting: adding a team token and starting a NEW OAuth connect (which mints a credential) take + * credentials.manage, and editing or removing a provider takes repos.manage. Linking a personal access token takes only + * membership, so "Use a token" stays for everyone. Reconnecting a credential you own is not a new connect — see below.

*/ describe("connect remote modal credential controls", () => { const team = (role: MeTeam["role"], permissions: string[]): MeTeam => ({ @@ -138,8 +138,9 @@ describe("connect remote modal credential controls", () => { /** * A credential that stopped working is brought back IN PLACE: Reconnect signs in again to the same credential (its id goes * to the API as replaceCredentialId, so what is bound through it needs no moving), and Replace token pastes a new token into - * it. Starting an OAuth connect takes credentials.manage, so Reconnect is absent for a role that would only be refused; - * replacing the token of your own credential takes only membership, and a team token takes credentials.manage. + * it. Both are the credential's own person's on membership alone, or credentials.manage's for anyone else's: the Personal tab lists + * only the caller's own credentials, so both are always offered there whatever the role. A plain Connect with OAuth mints a + * credential and stays credentials.manage's. */ describe("connect remote modal re-authorizing a credential in place", () => { const admin: MeTeam = { @@ -209,13 +210,28 @@ describe("connect remote modal re-authorizing a credential in place", () => { expect(JSON.stringify(init.body)).not.toContain("replaceCredentialId"); }); - it("offers a member no Reconnect, which would only be refused, and still offers Replace token on their own token", async () => { - renderAs(member, [credential("oauth-1", "OAuth"), credential("pat-1", "Pat")]); + it.each([ + ["Member", member], + ["Viewer", { ...admin, role: "Viewer", permissions: [] } as MeTeam], + ] as const)("offers a %s Reconnect on their own sign-in and Replace token on their own token: the credential's own person passes on membership", async (_role, asker) => { + vi.spyOn(window, "open").mockReturnValue(null); + const calls = renderAs(asker, [credential("oauth-1", "OAuth"), credential("pat-1", "Pat")], { "/api/credentials/oauth/init": { authorizeUrl: "https://gitlab.test/oauth/authorize", state: "s" } }); await waitFor(() => expect(screen.getAllByRole("button", { name: "Disconnect" })).toHaveLength(2)); - expect(screen.queryByRole("button", { name: "Reconnect" })).toBeNull(); expect(screen.getAllByRole("button", { name: "Replace token" })).toHaveLength(1); + fireEvent.click(screen.getByRole("button", { name: "Reconnect" })); + + await waitFor(() => expect(calls.some((c) => c.url.endsWith("/api/credentials/oauth/init"))).toBe(true)); + expect(calls.find((c) => c.url.endsWith("/api/credentials/oauth/init"))!.body).toMatchObject({ providerInstanceId: "pi1", replaceCredentialId: "oauth-1" }); + }); + + it("still offers a member no plain Connect with OAuth, which would mint a credential and answer 403", async () => { + renderAs(member, []); + + await waitFor(() => expect(screen.getByRole("button", { name: /Use a token/ })).toBeTruthy()); + + expect(screen.queryByRole("button", { name: /Connect with OAuth/ })).toBeNull(); }); it("puts a pasted token into the credential in place and closes", async () => { diff --git a/frontend/src/components/remedy/ProviderRemedyCard.test.tsx b/frontend/src/components/remedy/ProviderRemedyCard.test.tsx index d793d3650..0c2e3b42b 100644 --- a/frontend/src/components/remedy/ProviderRemedyCard.test.tsx +++ b/frontend/src/components/remedy/ProviderRemedyCard.test.tsx @@ -141,12 +141,12 @@ describe("ProviderRemedyCard — a failure that is nobody's credential to repair describe("ProviderRemedyCard — who may press what", () => { const memberView = remedy({ actions: [ - action("reconnect", { enabled: false, disabledReason: "Needs the Admin or Owner role." }), + action("reconnect", { enabled: false, disabledReason: "Only Nacho, or a team Admin, can do this." }), action("relink", { enabled: false, disabledReason: "Needs the Admin or Owner role." }), action("open_providers"), action("rerun", { enabled: false, disabledReason: "Fix the connection first, then run it again." }), ], - askPeople: [{ userId: "u3", name: "Olive", role: "Owner" }, { userId: "u2", name: "Ada", role: "Admin" }], + askPeople: [{ userId: "u1", name: "Nacho", role: "Owner of the credential" }, { userId: "u3", name: "Olive", role: "Owner" }, { userId: "u2", name: "Ada", role: "Admin" }], }); it("gives a Member no dead buttons: what is not theirs is a line saying which fix, what it takes and who to ask", () => { @@ -155,9 +155,28 @@ describe("ProviderRemedyCard — who may press what", () => { expect(screen.getAllByRole("button").map((b) => b.textContent?.trim())).toEqual(["Open Providers"]); // no disabled Reconnect, Re-link or Re-run const ask = screen.getByRole("note"); - expect(ask.textContent).toContain("Reconnect GitLab — Needs the Admin or Owner role."); + expect(ask.textContent).toContain("Reconnect GitLab — Only Nacho, or a team Admin, can do this."); expect(ask.textContent).toContain("Re-link repository — Needs the Admin or Owner role."); - expect(ask.textContent).toContain("Ask Olive (Owner) or Ada (Admin)."); + expect(ask.textContent).toContain("Ask Nacho (Owner of the credential), Olive (Owner) or Ada (Admin)."); + }); + + it("tells an Admin an Owner's sign-in is the Owner's alone, keeps their re-link, and names the Owner — not a peer who could not do it either", () => { + // An Admin holds credentials.manage and still cannot sign in as an Owner; what the server sends is shown as sent. + renderCard(remedy({ + actions: [ + action("reconnect", { enabled: false, disabledReason: "Only Nacho can reconnect their own sign-in." }), + action("relink"), + action("open_providers"), + action("rerun", { enabled: false, disabledReason: "Fix the connection first, then run it again." }), + ], + askPeople: [{ userId: "u9", name: "Nacho", role: "Owner of the credential" }], + })); + + expect(screen.getAllByRole("button").filter((b) => !(b as HTMLButtonElement).disabled).map((b) => b.textContent?.trim())).toEqual(["Re-link repository", "Open Providers"]); + + const ask = screen.getByRole("note"); + expect(ask.textContent).toContain("Reconnect GitLab — Only Nacho can reconnect their own sign-in."); + expect(ask.textContent).toContain("Ask Nacho (Owner of the credential)."); }); it("says nothing about who to ask when every fix is the caller's", () => { @@ -176,7 +195,7 @@ describe("ProviderRemedyCard — who may press what", () => { describe("ProviderRemedyCard — the same failure for an Owner, a Member and a Viewer", () => { const forRole = (reconnect: boolean, relink: boolean, askPeople: ProviderRemedy["askPeople"]) => remedy({ actions: [ - action("reconnect", { enabled: reconnect, disabledReason: reconnect ? null : "Needs the Admin or Owner role." }), + action("reconnect", { enabled: reconnect, disabledReason: reconnect ? null : "Only Nacho, or a team Admin, can do this." }), action("relink", { enabled: relink, disabledReason: relink ? null : "Needs the Admin or Owner role." }), action("open_providers"), ], @@ -188,6 +207,7 @@ describe("ProviderRemedyCard — the same failure for an Owner, a Member and a V ["Owner", forRole(true, true, []), ["Reconnect GitLab", "Re-link repository", "Open Providers"], false], ["Member", forRole(false, false, who), ["Open Providers"], true], ["Viewer", forRole(false, false, who), ["Open Providers"], true], + ["Member whose credential it is", forRole(true, false, who), ["Reconnect GitLab", "Open Providers"], true], // the credential's own person reconnects it; re-linking the repository is still not theirs ] as const)("%s: presses only what is enabled, and is told who to ask when something is not theirs", (_role, r, enabledButtons, asked) => { renderCard(r); diff --git a/frontend/src/components/remedy/RepositoryConnectionIssue.tsx b/frontend/src/components/remedy/RepositoryConnectionIssue.tsx index 45079eca0..40a83cb93 100644 --- a/frontend/src/components/remedy/RepositoryConnectionIssue.tsx +++ b/frontend/src/components/remedy/RepositoryConnectionIssue.tsx @@ -19,11 +19,12 @@ export function ConnectionIssueChip({ issue }: { issue: ConnectionIssue }) { } /** - * The way out, on the repository itself: Replace token for a pasted token (the credential's own person, or anyone who manages - * credentials), Reconnect for a sign-in (only who manages credentials: starting an OAuth connect takes `credentials.manage` whoever - * owns the row), and Re-link to another credential (when the caller manages repositories). The same fixes the run page offers, done - * by the same code. The server stays the authority: these only decide which buttons are worth showing, so a person is never offered - * one that could only answer 403. + * The way out, on the repository itself: Replace token for a pasted token and Reconnect for a sign-in — the credential's own + * person's (on membership alone) or anyone's who manages credentials, and for a sign-in only over someone the caller reaches: an Admin + * cannot sign in as another Admin or an Owner — and Re-link to another credential (when the caller manages repositories). + * The same fixes the run page offers, done by the same code. The server stays the authority: it says whether the caller may repair this + * credential (`callerMayRepair`, decided by the rules the two endpoints apply), so a person is never offered a button that could only answer + * 403. A server that predates the flag leaves the role to say: the credential's own person, or `credentials.manage`. * * It stops clicks reaching the row it sits in: a repository row opens the repository, and a dialog's portal still bubbles React events. */ @@ -35,7 +36,7 @@ export function RepositoryConnectionFix({ repository, issue, onConnectNew }: { r const mayRelink = can(TeamPermissions.ReposManage); const ownsIt = credential?.ownership === "Personal" && credential.ownerUserId != null && credential.ownerUserId === me.data?.id; - const mayRepair = credential != null && (isStaticToken(credential) ? ownsIt || can(TeamPermissions.CredentialsManage) : can(TeamPermissions.CredentialsManage)); + const mayRepair = credential != null && (credential.callerMayRepair ?? (ownsIt || can(TeamPermissions.CredentialsManage))); // A credential that merely looks fine (the repository is in Error for another reason) or whose person left is moved off, not repaired. const repairable = credential != null && (issue.kind === "rejected" || issue.kind === "disconnected"); diff --git a/frontend/src/components/repository-list.test.tsx b/frontend/src/components/repository-list.test.tsx index e7f1c6775..13595fb8d 100644 --- a/frontend/src/components/repository-list.test.tsx +++ b/frontend/src/components/repository-list.test.tsx @@ -145,8 +145,54 @@ describe("repository list controls", () => { expect(screen.getByText("Ask a team Admin or Owner to fix this connection.")).toBeTruthy(); }); - it("offers a member no Reconnect on their own refused sign-in, because starting an OAuth connect takes credentials.manage and would answer 403", async () => { + it.each([ + ["Member", ["workflows.write"]], + ["Viewer", []], + ] as const)("offers a %s Reconnect on their own refused sign-in but not on a teammate's: starting a reconnect passes the credential's own person on membership, anyone else needs credentials.manage", async (role, permissions) => { const mine = credential("mine", { status: "Error", authType: "OAuth", ownerUserId: "u1" }); + const theirs = credential("theirs", { status: "Error", authType: "OAuth", ownerUserId: "u2", ownerUserName: "Ada" }); + renderAs(team(role, [...permissions]), { credentials: [mine, theirs], repositories: [onCredential("mine"), onCredential("theirs")] }); + + await waitFor(() => expect(screen.getByText("acme/theirs")).toBeTruthy()); + + expect(screen.getAllByRole("button", { name: "Reconnect" })).toHaveLength(1); // only the one that is theirs + expect(screen.queryByRole("button", { name: /Re-link credential/ })).toBeNull(); + expect(screen.getByText("Ask a team Admin or Owner to fix this connection.")).toBeTruthy(); // for the teammate's row + }); + + it("offers an admin a Reconnect on a teammate's refused sign-in, because credentials.manage reaches every credential of the team", async () => { + const theirs = credential("theirs", { status: "Error", authType: "OAuth", ownerUserId: "u2", ownerUserName: "Ada" }); + renderAs(admin(), { credentials: [theirs], repositories: [onCredential("theirs")] }); + + await waitFor(() => expect(screen.getByRole("button", { name: "Reconnect" })).toBeTruthy()); + }); + + // ── What the server says the caller may repair beats what the role suggests: the reach over a credential's person is the server's to know ── + + it("offers a Reconnect only where the server says the caller may — an admin has credentials.manage and still cannot sign in as a peer Admin or an Owner", async () => { + const peers = credential("peers", { status: "Error", authType: "OAuth", ownerUserId: "u2", ownerUserName: "Ada", callerMayRepair: false }); + const members = credential("members", { status: "Error", authType: "OAuth", ownerUserId: "u3", ownerUserName: "Milo", callerMayRepair: true }); + renderAs(admin(), { credentials: [peers, members], repositories: [onCredential("peers"), onCredential("members")] }); + + await waitFor(() => expect(screen.getByText("acme/peers")).toBeTruthy()); + + expect(screen.getAllByRole("button", { name: "Reconnect" })).toHaveLength(1); // only the Member's sign-in + expect(screen.getAllByRole("button", { name: /Re-link credential/ })).toHaveLength(2); // moving off a credential is still the admin's, for both rows + }); + + it("offers no Replace token on a token whose person left, however the role reads, and no Reconnect on a sign-in the server refuses", async () => { + const left = credential("left", { status: "Error", authType: "Pat", ownerUserId: "u2", ownerUserName: "Ada", callerMayRepair: false }); + const outranked = credential("outranked", { status: "Error", authType: "OAuth", ownerUserId: "u3", ownerUserName: "Olive", callerMayRepair: false }); + renderAs(admin(), { credentials: [left, outranked], repositories: [onCredential("left"), onCredential("outranked")] }); + + await waitFor(() => expect(screen.getByText("acme/outranked")).toBeTruthy()); + + expect(screen.queryByRole("button", { name: "Replace token" })).toBeNull(); + expect(screen.queryByRole("button", { name: "Reconnect" })).toBeNull(); + }); + + it("tells a member who may not repair a credential who to ask, whatever they own, when the server says no", async () => { + const mine = credential("mine", { status: "Error", authType: "OAuth", ownerUserId: "u1", callerMayRepair: false }); renderAs(team("Member", ["workflows.write"]), { credentials: [mine], repositories: [onCredential("mine")] }); await waitFor(() => expect(screen.getByText("acme/mine")).toBeTruthy()); @@ -154,4 +200,11 @@ describe("repository list controls", () => { expect(screen.queryByRole("button", { name: "Reconnect" })).toBeNull(); expect(screen.getByText("Ask a team Admin or Owner to fix this connection.")).toBeTruthy(); }); + + it("offers a teammate's sign-in to a member the server says may repair it, as it says nothing else about roles", async () => { + const theirs = credential("theirs", { status: "Error", authType: "OAuth", ownerUserId: "u2", ownerUserName: "Ada", callerMayRepair: true }); + renderAs(team("Member", ["workflows.write"]), { credentials: [theirs], repositories: [onCredential("theirs")] }); + + await waitFor(() => expect(screen.getByRole("button", { name: "Reconnect" })).toBeTruthy()); + }); });