From 1fb86668aa9cbdc4c0ded2d0ffa4b3ab17786ed7 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Sun, 24 May 2026 18:35:15 +0200 Subject: [PATCH 1/2] progress --- API/Controller/OAuth/SignupFinalize.cs | 12 +- .../OAuth/SignupLinkWithPassword.cs | 135 ++++++++++++++++++ .../Requests/OAuthLinkPasswordRequest.cs | 9 ++ API/OAuth/OAuthError.cs | 10 ++ API/Services/Account/AccountService.cs | 48 ++++++- API/Services/Account/IAccountService.cs | 14 +- Common/Errors/AccountError.cs | 10 ++ 7 files changed, 225 insertions(+), 13 deletions(-) create mode 100644 API/Controller/OAuth/SignupLinkWithPassword.cs create mode 100644 API/Models/Requests/OAuthLinkPasswordRequest.cs diff --git a/API/Controller/OAuth/SignupFinalize.cs b/API/Controller/OAuth/SignupFinalize.cs index 4f0d5ef4..16b9732e 100644 --- a/API/Controller/OAuth/SignupFinalize.cs +++ b/API/Controller/OAuth/SignupFinalize.cs @@ -93,11 +93,15 @@ public async Task OAuthSignupFinalize( isEmailTrusted ); - if (!created.TryPickT0(out var newUser, out _)) + if (!created.TryPickT0(out var newUser, out var conflict)) { - // Username or email already exists — conflict. - // Do NOT clear the flow cookie so the frontend can retry with a different username. - return Problem(SignupError.UsernameOrEmailExists); + // Do NOT clear the flow cookie on conflict — the frontend may retry signup-finalize + // with a different username, or upgrade the flow to a link via signup-link-password. + return conflict.Match( + _ => Problem(AccountError.UsernameTaken), + email => Problem(email.HasPassword + ? AccountError.EmailTakenLinkAvailable + : AccountError.EmailTakenLinkUnavailable)); } // Authenticate the client if its activated (create session and set session cookie) diff --git a/API/Controller/OAuth/SignupLinkWithPassword.cs b/API/Controller/OAuth/SignupLinkWithPassword.cs new file mode 100644 index 00000000..e50e4626 --- /dev/null +++ b/API/Controller/OAuth/SignupLinkWithPassword.cs @@ -0,0 +1,135 @@ +using System.Net.Mime; +using System.Security.Claims; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.RateLimiting; +using OpenShock.API.Models.Requests; +using OpenShock.API.Models.Response; +using OpenShock.API.OAuth; +using OpenShock.API.Services.OAuthConnection; +using OpenShock.Common.Errors; +using OpenShock.Common.Extensions; +using OpenShock.Common.Problems; + +namespace OpenShock.API.Controller.OAuth; + +public sealed partial class OAuthController +{ + /// + /// Link an in-flight OAuth signup to an existing OpenShock account by authenticating with the + /// account's password. Used when POST /{provider}/signup-finalize returned + /// Account.Email.TakenLinkAvailable: the user holds a valid OAuth flow cookie, the + /// provider-supplied email matches an existing account that has a password, and the user proves + /// ownership of that account here. + /// + /// + /// Authenticates via the temporary OAuth flow cookie (anonymous user) and the account password. + /// On success the OAuth connection is attached to the existing account, a session is created, + /// and the flow cookie is cleared. + /// + [EnableRateLimiting("auth")] + [HttpPost("{provider}/signup-link-password")] + [Consumes(MediaTypeNames.Application.Json)] + [ProducesResponseType(StatusCodes.Status200OK, MediaTypeNames.Application.Json)] + [ProducesResponseType(StatusCodes.Status401Unauthorized, MediaTypeNames.Application.ProblemJson)] + [ProducesResponseType(StatusCodes.Status409Conflict, MediaTypeNames.Application.ProblemJson)] + [ApiExplorerSettings(IgnoreApi = true)] + public async Task OAuthSignupLinkWithPassword( + [FromRoute] string provider, + [FromBody] OAuthLinkPasswordRequest body, + [FromServices] IOAuthConnectionService connectionService, + CancellationToken cancellationToken) + { + var domain = GetCurrentCookieDomain(); + if (string.IsNullOrEmpty(domain)) + { + await HttpContext.SignOutAsync(OAuthConstants.FlowScheme); + return Problem(OAuthError.InternalError); + } + + var result = await ValidateOAuthFlowAsync(); + if (!result.TryPickT0(out var auth, out var error)) + { + return error switch + { + OAuthValidationError.FlowStateMissing => Problem(OAuthError.FlowNotFound), + _ => Problem(OAuthError.InternalError) + }; + } + + if (User.HasOpenShockUserIdentity()) + { + return Problem(OAuthError.FlowRequiresAnonymous); + } + + // Defense-in-depth: ensure the flow's provider matches the route and the flow type is right. + if (!string.Equals(auth.Provider, provider, StringComparison.OrdinalIgnoreCase) || auth.Flow != OAuthFlow.LoginOrCreate) + { + await HttpContext.SignOutAsync(OAuthConstants.FlowScheme); + return Problem(OAuthError.FlowMismatch); + } + + // The provider-supplied email is the only email we trust here — the client does not pass + // an email of their own. This prevents the endpoint from being used to probe arbitrary + // addresses for password validity. + var providerEmail = auth.Principal.FindFirst(ClaimTypes.Email)?.Value; + if (string.IsNullOrWhiteSpace(auth.ExternalAccountId) || string.IsNullOrWhiteSpace(providerEmail)) + { + return Problem(OAuthError.FlowMissingData); + } + + // No Turnstile here: the flow cookie already proves a completed OAuth round-trip with the + // provider, which is stronger bot-gating than Turnstile. Password brute-force is bounded + // by the shared `auth` rate-limit bucket. + + // Reject if the external identity got linked by a parallel flow between the conflict on + // signup-finalize and now. + var existing = await connectionService.GetByProviderExternalIdAsync(provider, auth.ExternalAccountId, cancellationToken); + if (existing is not null) + { + await HttpContext.SignOutAsync(OAuthConstants.FlowScheme); + return Problem(OAuthError.ExternalAlreadyLinked); + } + + // Authenticate using the provider's email as the lookup key. Map failures to the same + // problems LoginV2 returns so error shape & timing don't differ from a normal login attempt. + var getAccountResult = await _accountService.GetAccountByCredentialsAsync(providerEmail, body.Password, cancellationToken); + if (!getAccountResult.TryPickT0(out var account, out var loginErrors)) + { + return loginErrors.Match( + notFound => Problem(LoginError.InvalidCredentials), + deactivated => Problem(AccountError.AccountDeactivated), + notActivated => Problem(AccountError.AccountNotActivated), + oauthOnly => Problem(AccountError.AccountOAuthOnly) + ); + } + + // One connection per (user, provider). If this account already has this provider attached, + // surface a distinct error so the frontend can route the user to settings/connections. + if (await connectionService.HasConnectionAsync(account.Id, provider, cancellationToken)) + { + return Problem(OAuthError.ProviderAlreadyLinkedToAccount); + } + + var linked = await connectionService.TryAddConnectionAsync( + account.Id, + provider, + auth.ExternalAccountId, + auth.ExternalAccountDisplayName ?? auth.ExternalAccountName, + cancellationToken); + if (!linked) + { + // Race: either the external id or the (user, provider) pair was claimed in parallel. + return Problem(OAuthError.LinkFailed); + } + + _logger.LogInformation( + "Linked OAuth provider {Provider} (external id {ExternalId}) to existing user {UserId} via password", + provider, auth.ExternalAccountId, account.Id); + + await CreateSession(account.Id, domain); + await HttpContext.SignOutAsync(OAuthConstants.FlowScheme); + + return Ok(LoginV2OkResponse.FromUser(account)); + } +} diff --git a/API/Models/Requests/OAuthLinkPasswordRequest.cs b/API/Models/Requests/OAuthLinkPasswordRequest.cs new file mode 100644 index 00000000..0fb4fcc9 --- /dev/null +++ b/API/Models/Requests/OAuthLinkPasswordRequest.cs @@ -0,0 +1,9 @@ +using System.ComponentModel.DataAnnotations; + +namespace OpenShock.API.Models.Requests; + +public sealed class OAuthLinkPasswordRequest +{ + [Required(AllowEmptyStrings = false)] + public required string Password { get; set; } +} diff --git a/API/OAuth/OAuthError.cs b/API/OAuth/OAuthError.cs index 123cfa1e..2563e469 100644 --- a/API/OAuth/OAuthError.cs +++ b/API/OAuth/OAuthError.cs @@ -58,6 +58,16 @@ public static class OAuthError "This external account is already linked to another user", HttpStatusCode.Conflict); + public static OpenShockProblem ProviderAlreadyLinkedToAccount => new( + "OAuth.Provider.AlreadyLinkedToAccount", + "This provider is already linked to the target account", + HttpStatusCode.Conflict); + + public static OpenShockProblem LinkFailed => new( + "OAuth.Link.Failed", + "Failed to link the external account", + HttpStatusCode.Conflict); + // Misc / generic public static OpenShockProblem InternalError => new( "OAuth.InternalError", diff --git a/API/Services/Account/AccountService.cs b/API/Services/Account/AccountService.cs index dd17b919..5095c90c 100644 --- a/API/Services/Account/AccountService.cs +++ b/API/Services/Account/AccountService.cs @@ -122,7 +122,7 @@ await _emailService.ActivateAccount(new Contact(email, username), return new Success(user); } - public async Task, AccountWithEmailOrUsernameExists>> CreateOAuthOnlyAccountAsync( + public async Task, UsernameAlreadyTaken, EmailAlreadyTaken>> CreateOAuthOnlyAccountAsync( string email, string username, string provider, @@ -133,13 +133,23 @@ public async Task, AccountWithEmailOrUsernameExists>> Create email = email.ToLowerInvariant(); provider = provider.ToLowerInvariant(); - // Reuse your existing guards + // Reuse existing guards. Blacklist hits are reported as username-taken so we don't leak + // the existence of the blacklist or the validity of the email domain to anonymous callers. if (await IsUserNameBlacklisted(username) || await IsEmailProviderBlacklisted(email)) - return new AccountWithEmailOrUsernameExists(); + return new UsernameAlreadyTaken(); + + // Fast uniqueness pre-check (optimistic; race handled by unique constraints below). + // Prefer reporting the email collision when both are taken — only the email path lets the + // user recover by signing in to link the provider to the existing account. + var emailOwner = await _db.Users + .Where(u => u.Email == email) + .Select(u => new { HasPassword = u.PasswordHash != null }) + .FirstOrDefaultAsync(); + if (emailOwner is not null) + return new EmailAlreadyTaken { HasPassword = emailOwner.HasPassword }; - // Fast uniqueness check (optimistic; race handled by unique constraints below) - var exists = await _db.Users.AnyAsync(u => u.Email == email || u.Name == username); - if (exists) return new AccountWithEmailOrUsernameExists(); + if (await _db.Users.AnyAsync(u => u.Name == username)) + return new UsernameAlreadyTaken(); await using var tx = await _db.Database.BeginTransactionAsync(); @@ -205,7 +215,31 @@ await _emailService.ActivateAccount( catch (DbUpdateException ex) when (ex.InnerException is PostgresException { SqlState: "23505" }) { await tx.RollbackAsync(); - return new AccountWithEmailOrUsernameExists(); + + // Map known unique indexes to specific outcomes. + switch (pgEx.ConstraintName) + { + case "IX_users_email": + { + var hasPassword = await _db.Users + .Where(u => u.Email == email) + .Select(u => (bool?)(u.PasswordHash != null)) + .FirstOrDefaultAsync() ?? false; + return new EmailAlreadyTaken { HasPassword = hasPassword }; + } + case "IX_users_name": + return new UsernameAlreadyTaken(); + } + + // Ambiguous constraint — re-query both. Prefer the email outcome. + var emailRow = await _db.Users + .Where(u => u.Email == email) + .Select(u => new { HasPassword = u.PasswordHash != null }) + .FirstOrDefaultAsync(); + if (emailRow is not null) + return new EmailAlreadyTaken { HasPassword = emailRow.HasPassword }; + + return new UsernameAlreadyTaken(); } } diff --git a/API/Services/Account/IAccountService.cs b/API/Services/Account/IAccountService.cs index 4cbd2596..bc01bc42 100644 --- a/API/Services/Account/IAccountService.cs +++ b/API/Services/Account/IAccountService.cs @@ -39,8 +39,12 @@ public interface IAccountService /// external subject/id from provider /// display name from provider /// - /// Success with the created user, or AccountWithEmailOrUsernameExists when taken/blocked. - Task, AccountWithEmailOrUsernameExists>> CreateOAuthOnlyAccountAsync(string email, string username, string provider, string providerAccountId, string? providerAccountName, bool isEmailTrusted); + /// + /// Success with the created user, or a conflict marker distinguishing username vs. email collision. + /// When both collide, prefer (the email path enables linking via + /// password login; username conflict can only be retried). + /// + Task, UsernameAlreadyTaken, EmailAlreadyTaken>> CreateOAuthOnlyAccountAsync(string email, string username, string provider, string providerAccountId, string? providerAccountName, bool isEmailTrusted); /// /// @@ -141,6 +145,12 @@ public interface IAccountService public readonly struct AccountNotActivated; public readonly struct AccountDeactivated; public readonly struct AccountWithEmailOrUsernameExists; +public readonly struct UsernameAlreadyTaken; +public readonly struct EmailAlreadyTaken +{ + /// True if the existing user has a password hash and can therefore log in to link this provider. + public bool HasPassword { get; init; } +} public readonly struct CannotDeactivatePrivilegedAccount; public readonly struct AccountDeactivationAlreadyInProgress; public readonly struct CannotDeletePrivilegedAccount; diff --git a/Common/Errors/AccountError.cs b/Common/Errors/AccountError.cs index 0f76aab9..09820058 100644 --- a/Common/Errors/AccountError.cs +++ b/Common/Errors/AccountError.cs @@ -37,6 +37,16 @@ public static class AccountError public static OpenShockProblem EmailChangeAlreadyInUse => new OpenShockProblem("Account.Email.AlreadyInUse", "This email address is already in use", HttpStatusCode.Conflict); + public static OpenShockProblem EmailTakenLinkAvailable => new OpenShockProblem( + "Account.Email.TakenLinkAvailable", + "An account with this email already exists. Sign in with your password to link this provider to your existing account.", + HttpStatusCode.Conflict); + + public static OpenShockProblem EmailTakenLinkUnavailable => new OpenShockProblem( + "Account.Email.TakenLinkUnavailable", + "An account with this email already exists but cannot be linked via password.", + HttpStatusCode.Conflict); + public static OpenShockProblem EmailChangeUnchanged => new OpenShockProblem("Account.Email.Unchanged", "The new email address is the same as the current one", HttpStatusCode.BadRequest); From 9084ee8e13a171ddda2b8e3f9ddaddbfde5d30ae Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Sun, 24 May 2026 19:52:57 +0200 Subject: [PATCH 2/2] Update AccountService.cs --- API/Services/Account/AccountService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/API/Services/Account/AccountService.cs b/API/Services/Account/AccountService.cs index 5095c90c..f5971d2d 100644 --- a/API/Services/Account/AccountService.cs +++ b/API/Services/Account/AccountService.cs @@ -212,7 +212,7 @@ await _emailService.ActivateAccount( return new Success(user); } - catch (DbUpdateException ex) when (ex.InnerException is PostgresException { SqlState: "23505" }) + catch (DbUpdateException ex) when (ex.InnerException is PostgresException { SqlState: "23505" } pgEx) { await tx.RollbackAsync();