diff --git a/src/Exceptionless.AppHost/Program.cs b/src/Exceptionless.AppHost/Program.cs index 983e877ca9..2071a61029 100644 --- a/src/Exceptionless.AppHost/Program.cs +++ b/src/Exceptionless.AppHost/Program.cs @@ -188,6 +188,7 @@ #pragma warning disable ASPIREBROWSERLOGS001 var app = builder.AddViteApp("App", "../Exceptionless.Web/ClientApp") + .WithNpm(install: !ciE2E) // CI already ran npm ci before starting the AppHost. .WithBrowserLogs() .WithReference(api) .WithEnvironment("PUBLIC_EXCEPTIONLESS_API_KEY", builder.Configuration["PUBLIC_EXCEPTIONLESS_API_KEY"]) @@ -216,10 +217,6 @@ .WithEnvironment("API_HTTPS", worktreePorts.ApiHttpsUrl); } - // The e2e workflow runs npm ci before starting the AppHost, so it skips Aspire's second install. - if (ciE2E) - app.WithNpm(install: false); - if (includeDevTools) { #pragma warning disable ASPIREDENO001 diff --git a/src/Exceptionless.Core/Extensions/UserExtensions.cs b/src/Exceptionless.Core/Extensions/UserExtensions.cs index c811e3be5b..4752e6966b 100644 --- a/src/Exceptionless.Core/Extensions/UserExtensions.cs +++ b/src/Exceptionless.Core/Extensions/UserExtensions.cs @@ -64,34 +64,48 @@ public static bool HasValidPasswordResetTokenExpiration(this User user, TimeProv public static void AddOAuthAccount(this User user, string providerName, string providerUserId, string username, SettingsDictionary? data = null) { - var account = new OAuthAccount + var accounts = user.OAuthAccounts.Where(account => String.Equals(account.Provider, providerName, StringComparison.Ordinal) + && String.Equals(account.ProviderUserId, providerUserId, StringComparison.Ordinal)).ToArray(); + var account = accounts.FirstOrDefault(); + if (account is null) + { + account = new OAuthAccount + { + Provider = providerName, + ProviderUserId = providerUserId, + Username = username + }; + user.OAuthAccounts.Add(account); + } + else { - Provider = providerName.ToLowerInvariant(), - ProviderUserId = providerUserId, - Username = username - }; + account.Username = username; + } if (data is not null) account.ExtraData.Apply(data); - user.OAuthAccounts.Add(account); + foreach (var duplicate in accounts.Skip(1)) + user.OAuthAccounts.Remove(duplicate); } public static bool RemoveOAuthAccount(this User user, string providerName, string providerUserId) { - var account = user.OAuthAccounts.FirstOrDefault(o => - String.Equals(o.Provider, providerName, StringComparison.OrdinalIgnoreCase) - && String.Equals(o.ProviderUserId, providerUserId, StringComparison.Ordinal)); - if (account is null) + var accounts = user.OAuthAccounts.Where(account => String.Equals(account.Provider, providerName, StringComparison.Ordinal) + && String.Equals(account.ProviderUserId, providerUserId, StringComparison.Ordinal)).ToArray(); + if (accounts.Length is 0) return true; var activeOAuthAccounts = user.OAuthAccounts - .Where(o => !String.Equals(o.Provider, "facebook", StringComparison.OrdinalIgnoreCase) - && !String.Equals(o.Provider, "windowslive", StringComparison.OrdinalIgnoreCase)) + .Where(account => !String.Equals(account.Provider, OAuthAccount.KnownProviders.Facebook, StringComparison.OrdinalIgnoreCase) + && !String.Equals(account.Provider, OAuthAccount.KnownProviders.WindowsLive, StringComparison.OrdinalIgnoreCase)) .ToArray(); - if (String.IsNullOrEmpty(user.Password) && activeOAuthAccounts.Length <= 1 && activeOAuthAccounts.Contains(account)) + if (String.IsNullOrEmpty(user.Password) && activeOAuthAccounts.Length <= accounts.Length && activeOAuthAccounts.Contains(accounts[0])) return false; - return user.OAuthAccounts.Remove(account); + foreach (var account in accounts) + user.OAuthAccounts.Remove(account); + + return true; } } diff --git a/src/Exceptionless.Core/Models/OAuthAccount.cs b/src/Exceptionless.Core/Models/OAuthAccount.cs index ef21a54fa0..161a377c7e 100644 --- a/src/Exceptionless.Core/Models/OAuthAccount.cs +++ b/src/Exceptionless.Core/Models/OAuthAccount.cs @@ -1,10 +1,26 @@ -namespace Exceptionless.Core.Models; +using System.ComponentModel.DataAnnotations; + +namespace Exceptionless.Core.Models; public record OAuthAccount { + public static class KnownProviders + { + public const string GitHub = "github"; + public const string Google = "google"; + public const string Microsoft = "microsoft"; + public const string Facebook = "facebook"; + public const string WindowsLive = "windowslive"; + } + + [Required] + [AllowedValues(KnownProviders.GitHub, KnownProviders.Google, KnownProviders.Microsoft, KnownProviders.Facebook, KnownProviders.WindowsLive, + ErrorMessage = "The provider name must be github, google, microsoft, facebook, or windowslive.")] public required string Provider { get; init; } + + [Required] public required string ProviderUserId { get; init; } - public required string Username { get; init; } + public required string Username { get; set; } public SettingsDictionary ExtraData { get; init; } = new(); diff --git a/src/Exceptionless.Core/Models/User.cs b/src/Exceptionless.Core/Models/User.cs index b234f57e25..8ef23bae9d 100644 --- a/src/Exceptionless.Core/Models/User.cs +++ b/src/Exceptionless.Core/Models/User.cs @@ -6,7 +6,7 @@ namespace Exceptionless.Core.Models; -public record User : IIdentity, IHaveDates, IValidatableObject +public record User : IIdentity, IHaveDates, IVersioned, IValidatableObject { /// /// Unique id that identifies an user. @@ -54,6 +54,12 @@ public record User : IIdentity, IHaveDates, IValidatableObject public string? VerifyEmailAddressToken { get; set; } public DateTime VerifyEmailAddressTokenExpiration { get; set; } + /// + /// Repository concurrency metadata used to reject stale writes, preserved in cached snapshots. + /// New users have an empty version until persisted. + /// + public string Version { get; set; } = String.Empty; + /// /// Gets or sets the users active state. /// diff --git a/src/Exceptionless.Core/Repositories/Interfaces/IUserRepository.cs b/src/Exceptionless.Core/Repositories/Interfaces/IUserRepository.cs index ce65da5524..ad52ae10ea 100644 --- a/src/Exceptionless.Core/Repositories/Interfaces/IUserRepository.cs +++ b/src/Exceptionless.Core/Repositories/Interfaces/IUserRepository.cs @@ -11,7 +11,7 @@ public interface IUserRepository : ISearchableRepository Task SetSavedViewOrdersAsync(User user, CommandOptionsDescriptor? options = null); Task GetByEmailAddressAsync(string emailAddress); Task GetByPasswordResetTokenAsync(string token); - Task GetUserByOAuthProviderAsync(string provider, string providerUserId); + Task> GetUsersByOAuthProviderAsync(string provider, string providerUserId, CancellationToken cancellationToken = default); Task GetByVerifyEmailAddressTokenAsync(string token); Task> GetByOrganizationIdAsync(string organizationId, CommandOptionsDescriptor? options = null); } diff --git a/src/Exceptionless.Core/Repositories/UserRepository.cs b/src/Exceptionless.Core/Repositories/UserRepository.cs index d82d9650b9..37ec4eded3 100644 --- a/src/Exceptionless.Core/Repositories/UserRepository.cs +++ b/src/Exceptionless.Core/Repositories/UserRepository.cs @@ -123,14 +123,27 @@ public Task SetSavedViewOrdersAsync(User user, CommandOptionsDescriptor GetUserByOAuthProviderAsync(string provider, string providerUserId) + public async Task> GetUsersByOAuthProviderAsync(string provider, string providerUserId, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); if (String.IsNullOrEmpty(provider) || String.IsNullOrEmpty(providerUserId)) - return null; - - provider = provider.ToLowerInvariant(); - var results = (await FindAsync(q => q.FieldEquals(u => u.OAuthAccounts.First().ProviderUserId, providerUserId))).Documents; - return results.FirstOrDefault(u => u.OAuthAccounts.Any(o => o.Provider == provider)); + return Array.Empty(); + + // OAuth accounts are mapped as objects, so indexed fields cannot enforce matching within one account. + var results = await FindAsync(q => q.FieldEquals(u => u.OAuthAccounts.First().ProviderUserId, providerUserId) + .FieldEquals(u => u.OAuthAccounts.First().Provider, provider).SortAscending(u => u.Id), + o => o.SearchAfterPaging().PageLimit(100)); + var matchingUsers = new List(); + do + { + cancellationToken.ThrowIfCancellationRequested(); + matchingUsers.AddRange(results.Documents.Where(u => u.OAuthAccounts.Any(account => + String.Equals(account.Provider, provider, StringComparison.Ordinal) && + String.Equals(account.ProviderUserId, providerUserId, StringComparison.Ordinal)))); + } while (!cancellationToken.IsCancellationRequested && await results.NextPageAsync()); + + cancellationToken.ThrowIfCancellationRequested(); + return matchingUsers; } public async Task GetByVerifyEmailAddressTokenAsync(string token) diff --git a/src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs b/src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs index a9db9da0cc..6632a0def3 100644 --- a/src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs +++ b/src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs @@ -28,6 +28,7 @@ public static IEndpointRouteBuilder MapAuthEndpoints(this IEndpointRouteBuilder .Accepts("application/json", "application/*+json") .Produces() .ProducesProblem(StatusCodes.Status401Unauthorized) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Login") .WithDescription(""" @@ -86,6 +87,7 @@ headers api_key input box. .Produces() .ProducesProblem(StatusCodes.Status401Unauthorized) .ProducesProblem(StatusCodes.Status403Forbidden) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Sign up") .WithMetadata(new EndpointDocumentation { @@ -104,13 +106,17 @@ headers api_key input box. .AllowAnonymous() .Accepts("application/json", "application/*+json") .Produces() + .ProducesProblem(StatusCodes.Status401Unauthorized) .ProducesProblem(StatusCodes.Status403Forbidden) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Sign in with GitHub") .WithMetadata(new EndpointDocumentation { ResponseDescriptions = new() { ["200"] = "User Authentication Token", + ["401"] = "Unauthorized", ["403"] = "Account Creation is currently disabled", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); @@ -122,13 +128,17 @@ headers api_key input box. .AllowAnonymous() .Accepts("application/json", "application/*+json") .Produces() + .ProducesProblem(StatusCodes.Status401Unauthorized) .ProducesProblem(StatusCodes.Status403Forbidden) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Sign in with Google") .WithMetadata(new EndpointDocumentation { ResponseDescriptions = new() { ["200"] = "User Authentication Token", + ["401"] = "Unauthorized", ["403"] = "Account Creation is currently disabled", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); @@ -140,31 +150,41 @@ headers api_key input box. .AllowAnonymous() .Accepts("application/json", "application/*+json") .Produces() + .ProducesProblem(StatusCodes.Status401Unauthorized) .ProducesProblem(StatusCodes.Status403Forbidden) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Sign in with Microsoft") .WithMetadata(new EndpointDocumentation { ResponseDescriptions = new() { ["200"] = "User Authentication Token", + ["401"] = "Unauthorized", ["403"] = "Account Creation is currently disabled", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); - group.MapPost("unlink/{providerName:minlength(1)}", async (string providerName, IMediator mediator, IMediatorResultMapper resultMapper, HttpContext httpContext, [FromBody] ValueFromBody providerUserId) - => (await mediator.InvokeAsync>(new AuthMessages.RemoveExternalLogin(providerName, providerUserId, httpContext))).ToHttpResult(resultMapper)) + group.MapPost("unlink/{providerName:minlength(1)}", async ([AsParameters] RemoveExternalLoginRequest request, IMediator mediator, IMediatorResultMapper resultMapper, HttpContext httpContext) + => (await mediator.InvokeAsync>(new AuthMessages.RemoveExternalLogin(request.ProviderName, request.ProviderUserId?.Value, httpContext))).ToHttpResult(resultMapper)) .Accepts>("application/json", "application/*+json") .Produces() .ProducesProblem(StatusCodes.Status400BadRequest) + .ProducesProblem(StatusCodes.Status401Unauthorized) + .ProducesProblem(StatusCodes.Status409Conflict) + .ProducesValidationProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Removes an external login provider from the account") .WithMetadata(new EndpointDocumentation { RequestBodyDescription = "The provider user id.", ParameterDescriptions = new() { - ["providerName"] = "The provider name.", + ["providerName"] = "The provider name: github, google, microsoft, facebook, or windowslive (case sensitive).", }, ResponseDescriptions = new() { ["200"] = "User Authentication Token", - ["400"] = "Invalid provider name.", + ["400"] = "Invalid provider user id, or the account would have no sign-in method.", + ["401"] = "Unauthorized", + ["409"] = "The account changed while the request was being processed.", + ["422"] = "Validation error", } }); @@ -175,10 +195,12 @@ headers api_key input box. .Accepts("application/json", "application/*+json") .Produces() .ProducesProblem(StatusCodes.Status422UnprocessableEntity) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Change password") .WithMetadata(new EndpointDocumentation { ResponseDescriptions = new() { ["200"] = "User Authentication Token", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); @@ -212,10 +234,12 @@ headers api_key input box. .Accepts("application/json", "application/*+json") .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Reset password") .WithMetadata(new EndpointDocumentation { ResponseDescriptions = new() { ["200"] = "Password reset email was sent.", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Invalid reset password model.", } }); @@ -231,6 +255,7 @@ headers api_key input box. .AllowAnonymous() .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status400BadRequest) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Cancel reset password") .WithMetadata(new EndpointDocumentation { ParameterDescriptions = new() { @@ -239,6 +264,7 @@ headers api_key input box. ResponseDescriptions = new() { ["200"] = "Password reset email was cancelled.", ["400"] = "Invalid password reset token.", + ["409"] = "The account changed while the request was being processed.", } }); diff --git a/src/Exceptionless.Web/Api/Endpoints/OrganizationEndpoints.cs b/src/Exceptionless.Web/Api/Endpoints/OrganizationEndpoints.cs index 5cd02efbe3..81f5ce842b 100644 --- a/src/Exceptionless.Web/Api/Endpoints/OrganizationEndpoints.cs +++ b/src/Exceptionless.Web/Api/Endpoints/OrganizationEndpoints.cs @@ -75,6 +75,7 @@ public static IEndpointRouteBuilder MapOrganizationEndpoints(this IEndpointRoute .Accepts("application/json", "application/*+json") .Produces(StatusCodes.Status201Created) .ProducesProblem(StatusCodes.Status400BadRequest) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Create") .WithMetadata(new EndpointDocumentation { @@ -190,6 +191,7 @@ public static IEndpointRouteBuilder MapOrganizationEndpoints(this IEndpointRoute .Produces(StatusCodes.Status202Accepted) .ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status500InternalServerError) .WithSummary("Remove") .WithMetadata(new EndpointDocumentation { @@ -281,6 +283,7 @@ public static IEndpointRouteBuilder MapOrganizationEndpoints(this IEndpointRoute => (await mediator.InvokeAsync>(new OrganizationMessages.AddOrganizationUser(id, email, httpContext))).ToHttpResult(resultMapper)) .Produces() .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status426UpgradeRequired) .WithSummary("Add user") .WithMetadata(new EndpointDocumentation { @@ -299,6 +302,7 @@ public static IEndpointRouteBuilder MapOrganizationEndpoints(this IEndpointRoute .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Remove user") .WithMetadata(new EndpointDocumentation { ParameterDescriptions = new() { diff --git a/src/Exceptionless.Web/Api/Endpoints/UserEndpoints.cs b/src/Exceptionless.Web/Api/Endpoints/UserEndpoints.cs index b1904356e7..9ca75dcd56 100644 --- a/src/Exceptionless.Web/Api/Endpoints/UserEndpoints.cs +++ b/src/Exceptionless.Web/Api/Endpoints/UserEndpoints.cs @@ -110,6 +110,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder .Produces() .ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Update") .WithMetadata(new EndpointDocumentation { RequestBodyDescription = "The changes", @@ -129,6 +130,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder .Produces() .ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Update") .WithMetadata(new EndpointDocumentation { RequestBodyDescription = "The changes", @@ -146,6 +148,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder .Accepts("multipart/form-data") .Produces() .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithMetadata( new MultipartFileUploadAttribute(), @@ -166,6 +169,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder group.MapDelete("users/{id:objectid}/avatar", DeleteAvatarAsync) .Produces() .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Remove avatar") .WithMetadata(new EndpointDocumentation { ParameterDescriptions = new() { @@ -229,6 +233,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder .Produces() .ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .ProducesProblem(StatusCodes.Status429TooManyRequests) .WithSummary("Update email address") @@ -248,6 +253,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder => (await mediator.InvokeAsync(new UserMessages.VerifyEmailAddress(token))).ToHttpResult(resultMapper)) .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Verify email address") .WithMetadata(new EndpointDocumentation { @@ -264,6 +270,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder => (await mediator.InvokeAsync(new UserMessages.ResendVerificationEmail(id))).ToHttpResult(resultMapper)) .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Resend verification email") .WithMetadata(new EndpointDocumentation { ParameterDescriptions = new() { @@ -285,6 +292,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder }) .RequireAuthorization(AuthorizationRoles.GlobalAdminPolicy) .Produces(StatusCodes.Status200OK) + .ProducesProblem(StatusCodes.Status409Conflict) .ExcludeFromDescription(); group.MapPost("users/{id:objectid}/admin-role", async (string id, IMediator mediator, IMediatorResultMapper resultMapper) @@ -292,6 +300,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder .RequireAuthorization(AuthorizationRoles.GlobalAdminPolicy) .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .ExcludeFromDescription(); group.MapDelete("users/{id:objectid}/admin-role", async (string id, IMediator mediator, IMediatorResultMapper resultMapper) @@ -299,6 +308,7 @@ public static IEndpointRouteBuilder MapUserEndpoints(this IEndpointRouteBuilder .RequireAuthorization(AuthorizationRoles.GlobalAdminPolicy) .Produces(StatusCodes.Status204NoContent) .ProducesProblem(StatusCodes.Status404NotFound) + .ProducesProblem(StatusCodes.Status409Conflict) .ExcludeFromDescription(); return endpoints; diff --git a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs index afbc11b311..6e0d01e100 100644 --- a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs @@ -15,8 +15,11 @@ using Exceptionless.Web.Models; using Exceptionless.Web.Security; using Foundatio.Caching; +using Foundatio.Lock; using Foundatio.Mediator; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; +using Foundatio.Repositories.Models; using Microsoft.IdentityModel.Tokens; using OAuth2.Models; @@ -31,17 +34,16 @@ public class AuthHandler( IOAuthTokenRepository oauthTokenRepository, IOAuthProviderClient oauthProviderClient, ICacheClient cacheClient, + ILockProvider lockProvider, AuthService authService, IMailer mailer, IDomainLoginProvider domainLoginProvider, TimeProvider timeProvider, ILogger logger) { - private const string LegacyMicrosoftOAuthProvider = "WindowsLive"; - private const string MicrosoftOAuthProvider = "Microsoft"; private readonly ScopedCacheClient _cache = new(cacheClient, "Auth"); - private static bool _isFirstUserChecked; private static readonly TimeSpan IntercomJwtLifetime = TimeSpan.FromMinutes(60); + private static readonly TimeSpan ExternalAccountAdmissionTimeout = TimeSpan.FromSeconds(15); public async Task> Handle(LoginMessage message) { @@ -285,19 +287,27 @@ public Task> Handle(MicrosoftLogin message) public async Task> Handle(RemoveExternalLogin message) { + if (String.IsNullOrWhiteSpace(message.ProviderUserId)) + return Result.BadRequest("Invalid Provider Name or Provider User Id."); + var httpContext = message.Context; var user = httpContext.Request.GetUser(); - using var _ = logger.BeginScope(new ExceptionlessState().Tag("External Login").Tag(message.ProviderName).Identity(user.EmailAddress).Property("Provider User Id", message.ProviderUserId?.Value).SetHttpContext(httpContext)); - - if (String.IsNullOrWhiteSpace(message.ProviderName) || String.IsNullOrWhiteSpace(message.ProviderUserId?.Value)) - { - logger.LogError("Remove external login failed for {EmailAddress}: Invalid Provider Name or Provider User Id", user.EmailAddress); - return Result.BadRequest("Invalid Provider Name or Provider User Id."); - } + using var _ = logger.BeginScope(new ExceptionlessState().Tag("External Login").Tag(message.ProviderName).Identity(user.EmailAddress).Property("Provider User Id", message.ProviderUserId).SetHttpContext(httpContext)); + using var operationCancellation = CancellationTokenSource.CreateLinkedTokenSource(httpContext.RequestAborted); + operationCancellation.CancelAfter(ExternalAccountAdmissionTimeout); try { - if (!user.RemoveOAuthAccount(message.ProviderName, message.ProviderUserId.Value)) + operationCancellation.Token.ThrowIfCancellationRequested(); + await using var userLock = await lockProvider.AcquireAsync(GetExternalAccountUserLockKey(user.Id), cancellationToken: operationCancellation.Token); + var storedUser = await userRepository.GetByIdAsync(user.Id, options => options.Cache(false)).WaitAsync(operationCancellation.Token); + if (storedUser is null || !storedUser.IsActive + || !String.Equals(user.AuthenticationVersion, storedUser.AuthenticationVersion, StringComparison.Ordinal)) + return Result.Unauthorized("Login failed."); + + user = storedUser; + CompleteExternalAccountAdmission(operationCancellation); + if (!user.RemoveOAuthAccount(message.ProviderName, message.ProviderUserId)) { logger.LogError("Remove external login failed for {EmailAddress}: You must set a local password before removing your external login", user.EmailAddress); return Result.BadRequest("You must set a local password before removing your external login."); @@ -305,17 +315,23 @@ public async Task> Handle(RemoveExternalLogin message) user.AuthenticationVersion = Core.Extensions.StringExtensions.GetNewToken(); await userRepository.SaveAsync(user, o => o.Cache()); + + await ResetUserTokensAsync(user, "RemoveExternalLoginAsync", httpContext); + + logger.UserRemovedExternalLogin(user.EmailAddress, message.ProviderName); + return new TokenResult { Token = await GetOrCreateAuthenticationTokenAsync(user) }; + } + catch (Exception ex) when (ex is LockAcquisitionTimeoutException + || ex is OperationCanceledException && operationCancellation.IsCancellationRequested) + { + logger.LogWarning(ex, "Remove external login failed for {EmailAddress}: {Message}", user.EmailAddress, ex.Message); + return Result.Unauthorized("Login failed."); } catch (Exception ex) { logger.LogCritical(ex, "Error removing external login for {EmailAddress}: {Message}", user.EmailAddress, ex.Message); throw; } - - await ResetUserTokensAsync(user, "RemoveExternalLoginAsync", httpContext); - - logger.UserRemovedExternalLogin(user.EmailAddress, message.ProviderName); - return new TokenResult { Token = await GetOrCreateAuthenticationTokenAsync(user) }; } public async Task> Handle(ChangePassword message) @@ -407,7 +423,14 @@ public async Task Handle(ForgotPassword message) } user.CreatePasswordResetToken(timeProvider); - await userRepository.SaveAsync(user, o => o.Cache()); + try + { + await userRepository.SaveAsync(user, o => o.Cache()); + } + catch (VersionConflictDocumentException) + { + return Result.Success(); + } await mailer.SendUserPasswordResetAsync(user); logger.UserForgotPassword(user.EmailAddress); @@ -479,16 +502,11 @@ public async Task Handle(CancelResetPassword message) return Result.Success(); } - private async Task AddGlobalAdminRoleIfFirstUserAsync(User user) + private async Task AddGlobalAdminRoleIfFirstUserAsync(User user, CancellationToken cancellationToken = default) { - if (_isFirstUserChecked) - return; - - bool isFirstUser = await userRepository.CountAsync() == 0; + bool isFirstUser = await userRepository.CountAsync().WaitAsync(cancellationToken) == 0; if (isFirstUser) user.Roles.Add(AuthorizationRoles.GlobalAdmin); - - _isFirstUserChecked = true; } private async Task> ExternalLoginAsync(ExternalAuthInfo authInfo, HttpContext httpContext, string? appId, string? appSecret, Func> getUserInfoAsync) @@ -509,10 +527,56 @@ private async Task> ExternalLoginAsync(ExternalAuthInfo auth throw; } - User? user; + // Bound admission reads only. Once admitted, complete account changes and token issuance inside the locks. + using var operationCancellation = CancellationTokenSource.CreateLinkedTokenSource(httpContext.RequestAborted); + operationCancellation.CancelAfter(ExternalAccountAdmissionTimeout); try { - user = await FromExternalLoginAsync(userInfo, authInfo.InviteToken, httpContext); + ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.Id); + ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.ProviderName); + ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.Email); + + // Lock the identity and normalized email before resolving or creating its owner. + string[] accountKeys = [ + $"external-account:identity:{userInfo.ProviderName}:{OAuthService.CreateTokenHash(userInfo.Id)}", + $"external-account:email:{OAuthService.CreateTokenHash(userInfo.Email.Trim().ToLowerInvariant())}" + ]; + operationCancellation.Token.ThrowIfCancellationRequested(); + await using var accountLock = await lockProvider.AcquireAsync(accountKeys, cancellationToken: operationCancellation.Token); + var owners = await userRepository.GetUsersByOAuthProviderAsync(userInfo.ProviderName, userInfo.Id, operationCancellation.Token) + .WaitAsync(operationCancellation.Token); + if (owners.Count > 1) + { + logger.LogWarning("External login failed for {EmailAddress}: The external account is linked to multiple users", userInfo.Email); + return Result.Unauthorized("Login failed."); + } + + var existingUser = owners.SingleOrDefault(); + // Admission reads may outlive cancellation; do not read or populate a stale email cache entry. + var emailUser = existingUser is null + ? (await userRepository.FindOneAsync(query => query.FieldEquals(user => user.EmailAddress, userInfo.Email.Trim().ToLowerInvariant()), + options => options.Cache(false)).WaitAsync(operationCancellation.Token))?.Document + : null; + string? currentUserId = String.IsNullOrWhiteSpace(authInfo.InviteToken) && httpContext.User.IsUserAuthType() + ? httpContext.Request.GetUser().Id : null; + var userKeys = new[] { existingUser?.Id, emailUser?.Id, currentUserId } + .OfType().Select(GetExternalAccountUserLockKey); + // Ordered user locks also protect transfers and different identities on the same user. + await using var userLock = await lockProvider.AcquireAsync(userKeys, cancellationToken: operationCancellation.Token); + operationCancellation.Token.ThrowIfCancellationRequested(); + var user = await FromExternalLoginAsync(userInfo, authInfo.InviteToken, httpContext, existingUser, emailUser, operationCancellation); + + if (!String.IsNullOrWhiteSpace(authInfo.InviteToken)) + await AddInvitedUserToOrganizationAsync(authInfo.InviteToken, user, httpContext); + + logger.UserLoggedIn(user.EmailAddress); + return new TokenResult { Token = await GetOrCreateAuthenticationTokenAsync(user) }; + } + catch (Exception ex) when (ex is UnauthorizedAccessException or LockAcquisitionTimeoutException + || ex is OperationCanceledException && operationCancellation.IsCancellationRequested) + { + logger.LogWarning(ex, "External login failed for {EmailAddress}: {Message}", userInfo.Email, ex.Message); + return Result.Unauthorized("Login failed."); } catch (ApplicationException ex) { @@ -524,30 +588,58 @@ private async Task> ExternalLoginAsync(ExternalAuthInfo auth logger.LogCritical(ex, "External login failed for {EmailAddress}: {Message}", userInfo.Email, ex.Message); throw; } + } - if (!String.IsNullOrWhiteSpace(authInfo.InviteToken)) - await AddInvitedUserToOrganizationAsync(authInfo.InviteToken, user, httpContext); + private static string GetExternalAccountUserLockKey(string userId) => $"external-account:user:{userId}"; - logger.UserLoggedIn(user.EmailAddress); - return new TokenResult { Token = await GetOrCreateAuthenticationTokenAsync(user) }; + private static void CompleteExternalAccountAdmission(CancellationTokenSource admissionCancellation) + { + admissionCancellation.Token.ThrowIfCancellationRequested(); + admissionCancellation.CancelAfter(Timeout.InfiniteTimeSpan); } - private async Task FromExternalLoginAsync(UserInfo userInfo, string? inviteToken, HttpContext httpContext) + private async Task FromExternalLoginAsync(UserInfo userInfo, string? inviteToken, HttpContext httpContext, User? existingUser, User? emailUser, CancellationTokenSource admissionCancellation) { + var cancellationToken = admissionCancellation.Token; ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.Id); ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.ProviderName); ArgumentException.ThrowIfNullOrWhiteSpace(userInfo.Email); - var existingUser = await userRepository.GetUserByOAuthProviderAsync(userInfo.ProviderName, userInfo.Id); + var accountData = new SettingsDictionary { ["email"] = userInfo.Email }; + string? fullName = userInfo.GetFullName(); + if (!String.IsNullOrWhiteSpace(fullName)) + accountData["name"] = fullName; + using var _ = logger.BeginScope(new ExceptionlessState().Tag("External Login").Tag(userInfo.ProviderName).Identity(userInfo.Email).SetHttpContext(httpContext)); + if (existingUser is not null) + { + existingUser = await userRepository.GetByIdAsync(existingUser.Id, options => options.Cache(false)).WaitAsync(cancellationToken) + ?? throw new UnauthorizedAccessException("The user no longer exists."); + EnsureUserIsActive(existingUser); + if (!existingUser.OAuthAccounts.Any(account => + String.Equals(account.Provider, userInfo.ProviderName, StringComparison.Ordinal) && + String.Equals(account.ProviderUserId, userInfo.Id, StringComparison.Ordinal))) + throw new UnauthorizedAccessException("The external account is no longer linked to this user."); + } + if (String.IsNullOrWhiteSpace(inviteToken) && httpContext.User.IsUserAuthType()) { - var currentUser = httpContext.Request.GetUser(); + var authenticatedUser = httpContext.Request.GetUser(); + var currentUser = await userRepository.GetByIdAsync(authenticatedUser.Id, options => options.Cache(false)).WaitAsync(cancellationToken) + ?? throw new UnauthorizedAccessException("The user no longer exists."); + EnsureUserIsActive(currentUser); + if (!String.Equals(authenticatedUser.AuthenticationVersion, currentUser.AuthenticationVersion, StringComparison.Ordinal)) + throw new UnauthorizedAccessException("The user session has been revoked."); + CompleteExternalAccountAdmission(admissionCancellation); + OAuthAccount? transferredAccount = null; if (existingUser is not null) { if (!String.Equals(existingUser.Id, currentUser.Id, StringComparison.Ordinal)) { + transferredAccount = existingUser.OAuthAccounts.First(account => + String.Equals(account.Provider, userInfo.ProviderName, StringComparison.Ordinal) + && String.Equals(account.ProviderUserId, userInfo.Id, StringComparison.Ordinal)); if (!existingUser.RemoveOAuthAccount(userInfo.ProviderName, userInfo.Id)) throw new Exception($"Unable to remove existing oauth account for existing user: {existingUser.EmailAddress}"); @@ -555,50 +647,55 @@ private async Task FromExternalLoginAsync(UserInfo userInfo, string? invit await userRepository.SaveAsync(existingUser, o => o.Cache()); await ResetUserTokensAsync(existingUser, "Link external account to another user", httpContext); } - else - { - if (RemoveLegacyMicrosoftOAuthAccounts(currentUser, userInfo.ProviderName)) - return await userRepository.SaveAsync(currentUser, o => o.Cache()); - - return currentUser; - } } - currentUser.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email); + currentUser.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email, accountData); RemoveLegacyMicrosoftOAuthAccounts(currentUser, userInfo.ProviderName); - return await userRepository.SaveAsync(currentUser, o => o.Cache()); - } - - if (existingUser is not null) - { - bool hasChanges = RemoveLegacyMicrosoftOAuthAccounts(existingUser, userInfo.ProviderName); - if (!existingUser.IsEmailAddressVerified) + try { - existingUser.MarkEmailAddressVerified(); - hasChanges = true; + return await userRepository.SaveAsync(currentUser, o => o.Cache()); } + catch (VersionConflictDocumentException) when (existingUser is not null && transferredAccount is not null) + { + // The destination write did not commit. Restore the link on fresh state, retaining session revocation. + bool restored = await userRepository.PatchAsync(existingUser.Id, new ActionPatch(user => + user.AddOAuthAccount(transferredAccount.Provider, transferredAccount.ProviderUserId, transferredAccount.Username, transferredAccount.ExtraData))); + if (!restored) + throw new InvalidOperationException("Unable to restore the previous external account owner."); - if (hasChanges) - await userRepository.SaveAsync(existingUser, o => o.Cache()); + throw; + } + } - return existingUser; + if (existingUser is not null) + { + CompleteExternalAccountAdmission(admissionCancellation); + existingUser.MarkEmailAddressVerified(); + existingUser.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email, accountData); + RemoveLegacyMicrosoftOAuthAccounts(existingUser, userInfo.ProviderName); + return await userRepository.SaveAsync(existingUser, o => o.Cache()); } - var user = !String.IsNullOrEmpty(userInfo.Email) ? await userRepository.GetByEmailAddressAsync(userInfo.Email) : null; + var user = emailUser is not null ? await userRepository.GetByIdAsync(emailUser.Id, options => options.Cache(false)).WaitAsync(cancellationToken) : null; + if (user is not null && !String.Equals(user.EmailAddress, userInfo.Email.Trim(), StringComparison.OrdinalIgnoreCase)) + throw new UnauthorizedAccessException("The user's email address has changed."); if (user is null) { - if (!await IsAccountCreationEnabledAsync(inviteToken)) + if (!await IsAccountCreationEnabledAsync(inviteToken, cancellationToken)) throw new ApplicationException("Account Creation is currently disabled."); user = new User { FullName = userInfo.GetFullName()!, EmailAddress = userInfo.Email }; user.Roles.Add(AuthorizationRoles.Client); user.Roles.Add(AuthorizationRoles.User); - await AddGlobalAdminRoleIfFirstUserAsync(user); + await AddGlobalAdminRoleIfFirstUserAsync(user, cancellationToken); } + EnsureUserIsActive(user); + CompleteExternalAccountAdmission(admissionCancellation); user.MarkEmailAddressVerified(); - user.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email); + user.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email, accountData); + RemoveLegacyMicrosoftOAuthAccounts(user, userInfo.ProviderName); if (String.IsNullOrEmpty(user.Id)) await userRepository.AddAsync(user, o => o.Cache()); @@ -608,13 +705,19 @@ private async Task FromExternalLoginAsync(UserInfo userInfo, string? invit return user; } + private static void EnsureUserIsActive(User user) + { + if (!user.IsActive) + throw new UnauthorizedAccessException("The user is inactive."); + } + private static bool RemoveLegacyMicrosoftOAuthAccounts(User user, string providerName) { - if (!String.Equals(providerName, MicrosoftOAuthProvider, StringComparison.OrdinalIgnoreCase)) + if (!String.Equals(providerName, OAuthAccount.KnownProviders.Microsoft, StringComparison.OrdinalIgnoreCase)) return false; var legacyAccounts = user.OAuthAccounts - .Where(account => String.Equals(account.Provider, LegacyMicrosoftOAuthProvider, StringComparison.OrdinalIgnoreCase)) + .Where(account => String.Equals(account.Provider, OAuthAccount.KnownProviders.WindowsLive, StringComparison.OrdinalIgnoreCase)) .ToArray(); foreach (var account in legacyAccounts) user.OAuthAccounts.Remove(account); @@ -622,7 +725,7 @@ private static bool RemoveLegacyMicrosoftOAuthAccounts(User user, string provide return legacyAccounts.Length > 0; } - private async Task IsAccountCreationEnabledAsync(string? token) + private async Task IsAccountCreationEnabledAsync(string? token, CancellationToken cancellationToken = default) { if (authOptions.EnableAccountCreation) return true; @@ -630,7 +733,7 @@ private async Task IsAccountCreationEnabledAsync(string? token) if (String.IsNullOrEmpty(token)) return false; - var organization = await organizationRepository.GetByInviteTokenAsync(token); + var organization = await organizationRepository.GetByInviteTokenAsync(token).WaitAsync(cancellationToken); return organization is not null; } diff --git a/src/Exceptionless.Web/Api/Messages/AuthMessages.cs b/src/Exceptionless.Web/Api/Messages/AuthMessages.cs index 134ee74ca7..fec97023ab 100644 --- a/src/Exceptionless.Web/Api/Messages/AuthMessages.cs +++ b/src/Exceptionless.Web/Api/Messages/AuthMessages.cs @@ -9,7 +9,7 @@ public record SignupMessage(Signup Model, HttpContext Context); public record GitHubLogin(ExternalAuthInfo AuthInfo, HttpContext Context); public record GoogleLogin(ExternalAuthInfo AuthInfo, HttpContext Context); public record MicrosoftLogin(ExternalAuthInfo AuthInfo, HttpContext Context); -public record RemoveExternalLogin(string ProviderName, ValueFromBody ProviderUserId, HttpContext Context); +public record RemoveExternalLogin(string ProviderName, string? ProviderUserId, HttpContext Context); public record ChangePassword(ChangePasswordModel Model, HttpContext Context); public record CheckEmailAddress(string Email, HttpContext Context); public record ForgotPassword(string Email, HttpContext Context); diff --git a/src/Exceptionless.Web/ClientApp/e2e/tests/password-recovery.e2e.ts b/src/Exceptionless.Web/ClientApp/e2e/tests/password-recovery.e2e.ts index 81bfd692c7..e14e34f765 100644 --- a/src/Exceptionless.Web/ClientApp/e2e/tests/password-recovery.e2e.ts +++ b/src/Exceptionless.Web/ClientApp/e2e/tests/password-recovery.e2e.ts @@ -5,6 +5,28 @@ const RESET_PASSWORD = `${E2E_TEST_PASSWORD}-reset`; test.skip(process.env.E2E_ENV === 'production', 'Password recovery requires local Mailpit.'); test.use({ e2eCleanupPassword: RESET_PASSWORD, e2eInjectBrowserToken: false, e2eUseInvitedUser: true }); +test('failed password-reset cancellation can be retried without reloading', async ({ page }) => { + let attempts = 0; + await page.route('**/api/v2/auth/cancel-reset-password/cancel-retry', async (route) => { + attempts++; + await route.fulfill({ + body: attempts === 1 ? JSON.stringify({ status: 409, title: 'User changed' }) : '', + contentType: 'application/problem+json', + status: attempts === 1 ? 409 : 200 + }); + }); + + await page.goto('/reset-password/cancel-retry?cancel=true'); + await expect(page.getByRole('button', { name: 'Try Again' })).toBeVisible(); + await expect(page.getByText('Cancelling password reset...')).toHaveCount(0); + await expect(page.getByRole('link', { name: 'Back to Login' })).toBeVisible(); + expect(attempts).toBe(1); + + await page.getByRole('button', { name: 'Try Again' }).click(); + await expect(page).toHaveURL(/\/login(?:[?#]|$)/); + expect(attempts).toBe(2); +}); + test('user can reset a forgotten password and log in @signup', async ({ e2eApi, e2eScenario, page }) => { await test.step('request a password reset through the UI', async () => { await page.goto('/forgot-password'); diff --git a/src/Exceptionless.Web/ClientApp/e2e/tests/stack-effects-chaos.e2e.ts b/src/Exceptionless.Web/ClientApp/e2e/tests/stack-effects-chaos.e2e.ts index a406b1e505..fbcf387ed4 100644 --- a/src/Exceptionless.Web/ClientApp/e2e/tests/stack-effects-chaos.e2e.ts +++ b/src/Exceptionless.Web/ClientApp/e2e/tests/stack-effects-chaos.e2e.ts @@ -38,6 +38,10 @@ test('stack effects stay bounded through background, paging, and navigation chao await installWebSocketTestHarness(page, { ignoreServerMessages: true }); const journey = ExceptionlessE2EJourney.fromScenario(page, e2eApi, e2eScenario); + const firstStackRow = page + .getByRole('row') + .filter({ has: page.getByRole('checkbox', { exact: true, name: 'Select row' }) }) + .first(); const diagnostics: RuntimeDiagnostics = { actionSamples: [], activeAction: 'setup', @@ -67,7 +71,7 @@ test('stack effects stay bounded through background, paging, and navigation chao await page.goto('/stack/all?limit=5'); expect((await response).ok()).toBe(true); await expect(page.getByRole('heading', { exact: true, name: 'All' })).toBeVisible(); - await expect(page.locator('tbody tr:visible').first()).toBeVisible(); + await expect(firstStackRow).toBeVisible(); await expect(page.getByRole('button', { name: 'Go to next page' })).toBeEnabled(); }); @@ -99,7 +103,7 @@ test('stack effects stay bounded through background, paging, and navigation chao }); await measureAction(diagnostics, 'stack timeline drag', async () => { - await page.locator('tbody tr:visible').first().click(); + await firstStackRow.click(); const dialog = page.getByRole('dialog'); await expect(dialog).toBeVisible(); const timeline = dialog.locator('[data-slot="chart"]'); @@ -147,7 +151,7 @@ test('stack effects stay bounded through background, paging, and navigation chao await measureAction(diagnostics, 'stack detail mount and teardown', async () => { for (let index = 0; index < 5; index++) { - await page.locator('tbody tr:visible').first().click(); + await firstStackRow.click(); await expect(page.getByRole('dialog')).toBeVisible(); await page.getByRole('button', { name: 'Close' }).click(); await expect(page.getByRole('dialog')).not.toBeVisible(); diff --git a/src/Exceptionless.Web/ClientApp/src/lib/features/auth/api.svelte.ts b/src/Exceptionless.Web/ClientApp/src/lib/features/auth/api.svelte.ts index 4e5a5568f3..0706b46f12 100644 --- a/src/Exceptionless.Web/ClientApp/src/lib/features/auth/api.svelte.ts +++ b/src/Exceptionless.Web/ClientApp/src/lib/features/auth/api.svelte.ts @@ -18,7 +18,7 @@ const queryKeys = { export async function cancelResetPassword(token: string) { const client = useFetchClient(); const response = await client.post(`auth/cancel-reset-password/${token}`, { - expectedStatusCodes: [400] + expectedStatusCodes: [400, 409] }); return response; @@ -33,7 +33,7 @@ export async function changePassword(currentPassword: string | undefined, newPas password: newPassword }, { - expectedStatusCodes: [422] + expectedStatusCodes: [409, 422] } ); @@ -89,7 +89,7 @@ export async function login(email: string, password: string, inviteToken?: null }; const client = useFetchClient(); const response = await client.postJSON('auth/login', data, { - expectedStatusCodes: [401, 422] + expectedStatusCodes: [401, 409, 422] }); if (response.ok && response.data?.token) { @@ -127,7 +127,7 @@ export async function resetPassword(passwordResetToken: string, password: string password_reset_token: passwordResetToken }, { - expectedStatusCodes: [422] + expectedStatusCodes: [409, 422] } ); @@ -145,7 +145,7 @@ export async function signup(name: string, email: string, password: string, invi password }, { - expectedStatusCodes: [401, 403, 422] + expectedStatusCodes: [401, 403, 409, 422] } ); @@ -166,7 +166,7 @@ export async function unlinkOAuthAccount(provider: string, providerUserId: strin provider_user_id: providerUserId }, { - expectedStatusCodes: [400] + expectedStatusCodes: [400, 409] } ); diff --git a/src/Exceptionless.Web/ClientApp/src/lib/features/auth/index.svelte.ts b/src/Exceptionless.Web/ClientApp/src/lib/features/auth/index.svelte.ts index a98bce3c7a..5bed54c731 100644 --- a/src/Exceptionless.Web/ClientApp/src/lib/features/auth/index.svelte.ts +++ b/src/Exceptionless.Web/ClientApp/src/lib/features/auth/index.svelte.ts @@ -176,7 +176,7 @@ async function oauthLogin(options: OAuthLoginOptions) { state: data.state }, { - expectedStatusCodes: [401, 403, 422] + expectedStatusCodes: [401, 403, 409, 422] } ); diff --git a/src/Exceptionless.Web/ClientApp/src/lib/features/auth/microsoft.test.ts b/src/Exceptionless.Web/ClientApp/src/lib/features/auth/microsoft.test.ts index 59fd715c6d..6c473ce446 100644 --- a/src/Exceptionless.Web/ClientApp/src/lib/features/auth/microsoft.test.ts +++ b/src/Exceptionless.Web/ClientApp/src/lib/features/auth/microsoft.test.ts @@ -72,7 +72,7 @@ describe('microsoftLogin', () => { redirectUri: 'http://localhost:7131', state }, - { expectedStatusCodes: [401, 403, 422] } + { expectedStatusCodes: [401, 403, 409, 422] } ); expect(accessToken.current).toBe('session-token'); expect(goto).toHaveBeenCalledExactlyOnceWith('/organization/invited'); @@ -97,8 +97,11 @@ describe('microsoftLogin', () => { expect(popup.close).toHaveBeenCalledOnce(); }); - it('displays the 403 with the application error callback enabled', async () => { - const message = 'Account Creation is currently disabled'; + it.each([ + { message: 'Account Creation is currently disabled', status: 403 }, + { message: 'The account changed while the request was being processed.', status: 409 } + ])('displays the $status with the application error callback enabled', async ({ message, status }) => { + // Arrange const client = new FetchClient({ baseUrl: 'http://localhost/api/v2', defaultRequestOptions: { @@ -107,17 +110,21 @@ describe('microsoftLogin', () => { } }, fetch: async () => - new Response(JSON.stringify({ status: 403, title: message }), { + new Response(JSON.stringify({ status, title: message }), { headers: { 'Content-Type': 'application/problem+json' }, - status: 403 + status }) }); postJSON.mockImplementation(client.postJSON.bind(client)); vi.stubGlobal('crypto', { randomUUID: () => 'expected-state' }); + + // Act const login = microsoftLogin(); const completed = expect(login).resolves.toBeUndefined(); popup.location = new URL('http://localhost:7131/?code=authorization-code&state=expected-state'); await vi.advanceTimersByTimeAsync(500); + + // Assert await completed; expect(toastError).toHaveBeenCalledExactlyOnceWith(message); expect(accessToken.current).toBeNull(); diff --git a/src/Exceptionless.Web/ClientApp/src/routes/(auth)/reset-password/[token]/+page.svelte b/src/Exceptionless.Web/ClientApp/src/routes/(auth)/reset-password/[token]/+page.svelte index cc1dca0b42..b9b48a1fe5 100644 --- a/src/Exceptionless.Web/ClientApp/src/routes/(auth)/reset-password/[token]/+page.svelte +++ b/src/Exceptionless.Web/ClientApp/src/routes/(auth)/reset-password/[token]/+page.svelte @@ -3,6 +3,7 @@ import { resolve } from '$app/paths'; import { page } from '$app/state'; import { createForm } from '@tanstack/svelte-form'; + import { untrack } from 'svelte'; import { toast } from 'svelte-sonner'; import ErrorMessage from '#comp/error-message.svelte'; @@ -19,6 +20,8 @@ const token = page.params.token; const shouldCancel = page.url.searchParams.get('cancel') === 'true'; + let isCancelling = $state(false); + let cancelFailed = $state(false); const form = createForm(() => ({ defaultValues: { @@ -49,14 +52,29 @@ })); async function handleCancelToken() { - if (shouldCancel && token) { - await cancelResetPassword(token); + if (!shouldCancel || !token || isCancelling) { + return; + } + + isCancelling = true; + cancelFailed = false; + try { + const response = await cancelResetPassword(token); + if (!response.ok) { + throw new Error('Password reset cancellation failed'); + } + await goto(resolve('/(auth)/login')); + } catch { + cancelFailed = true; + toast.error('An error occurred while trying to cancel your password reset. Please try again.'); + } finally { + isCancelling = false; } } $effect(() => { - handleCancelToken(); + untrack(handleCancelToken); }); @@ -64,12 +82,20 @@ - Cancelling password reset... + {cancelFailed ? 'Unable to cancel password reset' : 'Cancelling password reset...'} -
- -
+ {#if cancelFailed} + + +

+ Back to Login +

+ {:else} +
+ +
+ {/if} {:else} diff --git a/src/Exceptionless.Web/ClientApp/src/routes/(auth)/reset-password/[token]/reset-password-page.svelte.test.ts b/src/Exceptionless.Web/ClientApp/src/routes/(auth)/reset-password/[token]/reset-password-page.svelte.test.ts new file mode 100644 index 0000000000..a4bff57055 --- /dev/null +++ b/src/Exceptionless.Web/ClientApp/src/routes/(auth)/reset-password/[token]/reset-password-page.svelte.test.ts @@ -0,0 +1,58 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/svelte'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import ResetPasswordPage from './+page.svelte'; + +const mocks = vi.hoisted(() => ({ + cancelResetPassword: vi.fn(), + goto: vi.fn(), + toastError: vi.fn() +})); + +vi.mock('$app/navigation', () => ({ goto: mocks.goto })); +vi.mock('$app/paths', () => ({ resolve: (path: string) => path.replace('/(auth)', '') })); +vi.mock('$app/state', () => ({ page: { params: { token: 'reset-token' }, url: new URL('http://localhost/reset-password/reset-token?cancel=true') } })); +vi.mock('#features/auth/api.svelte.ts', () => ({ cancelResetPassword: mocks.cancelResetPassword, resetPassword: vi.fn() })); +vi.mock('svelte-sonner', () => ({ toast: { error: mocks.toastError } })); + +describe('password reset cancellation', () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it('CancelResetPassword_Conflict_OffersRetryAndNavigatesOnlyAfterSuccess', async () => { + // Arrange + mocks.cancelResetPassword.mockResolvedValueOnce({ ok: false, status: 409 }).mockResolvedValueOnce({ ok: true, status: 200 }); + render(ResetPasswordPage); + + // Act + const retry = await screen.findByRole('button', { name: 'Try Again' }); + + // Assert + expect(mocks.goto).not.toHaveBeenCalled(); + expect(mocks.cancelResetPassword).toHaveBeenCalledExactlyOnceWith('reset-token'); + expect(screen.queryByText('Cancelling password reset...')).not.toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Back to Login' })).toHaveAttribute('href', '/login'); + + // Act + await fireEvent.click(retry); + + // Assert + await waitFor(() => expect(mocks.goto).toHaveBeenCalledWith('/login')); + expect(mocks.cancelResetPassword).toHaveBeenCalledTimes(2); + }); + + it('CancelResetPassword_NetworkFailure_OffersRetry', async () => { + // Arrange + mocks.cancelResetPassword.mockRejectedValueOnce(new Error('Connection lost')); + render(ResetPasswordPage); + + // Act + const retry = await screen.findByRole('button', { name: 'Try Again' }); + + // Assert + expect(retry).toBeEnabled(); + expect(mocks.goto).not.toHaveBeenCalled(); + expect(mocks.cancelResetPassword).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/Exceptionless.Web/Models/Auth/RemoveExternalLoginRequest.cs b/src/Exceptionless.Web/Models/Auth/RemoveExternalLoginRequest.cs new file mode 100644 index 0000000000..f407e31ef1 --- /dev/null +++ b/src/Exceptionless.Web/Models/Auth/RemoveExternalLoginRequest.cs @@ -0,0 +1,18 @@ +using System.ComponentModel.DataAnnotations; +using Exceptionless.Core.Models; +using Microsoft.AspNetCore.Mvc; + +namespace Exceptionless.Web.Models; + +public sealed class RemoveExternalLoginRequest +{ + [FromRoute(Name = "providerName")] + [Required] + [AllowedValues(OAuthAccount.KnownProviders.GitHub, OAuthAccount.KnownProviders.Google, OAuthAccount.KnownProviders.Microsoft, + OAuthAccount.KnownProviders.Facebook, OAuthAccount.KnownProviders.WindowsLive, + ErrorMessage = "The provider name must be github, google, microsoft, facebook, or windowslive.")] + public string ProviderName { get; set; } = null!; + + [FromBody] + public ValueFromBody ProviderUserId { get; set; } = null!; +} diff --git a/src/Exceptionless.Web/Security/OAuthProviderClient.cs b/src/Exceptionless.Web/Security/OAuthProviderClient.cs index a9a1d71cc5..e8a9521bbd 100644 --- a/src/Exceptionless.Web/Security/OAuthProviderClient.cs +++ b/src/Exceptionless.Web/Security/OAuthProviderClient.cs @@ -22,7 +22,7 @@ public sealed class OAuthProviderClient(SlackService slackService) : IOAuthProvi { public Task GetGitHubUserInfoAsync(ExternalAuthInfo authInfo, string appId, string appSecret) { - return GetUserInfoAsync(authInfo, appId, appSecret, (factory, configuration) => + return GetUserInfoAsync(authInfo, appId, appSecret, OAuthAccount.KnownProviders.GitHub, (factory, configuration) => { configuration.Scope = "user:email"; return new GitHubClient(factory, configuration); @@ -31,7 +31,7 @@ public Task GetGitHubUserInfoAsync(ExternalAuthInfo authInfo, string a public Task GetGoogleUserInfoAsync(ExternalAuthInfo authInfo, string appId, string appSecret) { - return GetUserInfoAsync(authInfo, appId, appSecret, (factory, configuration) => + return GetUserInfoAsync(authInfo, appId, appSecret, OAuthAccount.KnownProviders.Google, (factory, configuration) => { configuration.Scope = "profile email"; return new GoogleClient(factory, configuration); @@ -40,7 +40,7 @@ public Task GetGoogleUserInfoAsync(ExternalAuthInfo authInfo, string a public Task GetMicrosoftUserInfoAsync(ExternalAuthInfo authInfo, string appId, string appSecret) { - return GetUserInfoAsync(authInfo, appId, appSecret, (factory, configuration) => + return GetUserInfoAsync(authInfo, appId, appSecret, OAuthAccount.KnownProviders.Microsoft, (factory, configuration) => { configuration.Scope = "User.Read"; return new MicrosoftClient(factory, configuration); @@ -52,10 +52,11 @@ public Task GetMicrosoftUserInfoAsync(ExternalAuthInfo authInfo, strin return slackService.GetAccessTokenAsync(code); } - private static Task GetUserInfoAsync( + private static async Task GetUserInfoAsync( ExternalAuthInfo authInfo, string appId, string appSecret, + string providerName, Func createClient ) where TClient : OAuth2Client { @@ -66,6 +67,8 @@ Func createClient RedirectUri = authInfo.RedirectUri }); - return client.GetUserInfoAsync(authInfo.Code, authInfo.RedirectUri); + var userInfo = await client.GetUserInfoAsync(authInfo.Code, authInfo.RedirectUri); + userInfo.ProviderName = providerName; + return userInfo; } } diff --git a/tests/Exceptionless.Tests/Api/Data/openapi.json b/tests/Exceptionless.Tests/Api/Data/openapi.json index 88be9cf072..8d00b548a9 100644 --- a/tests/Exceptionless.Tests/Api/Data/openapi.json +++ b/tests/Exceptionless.Tests/Api/Data/openapi.json @@ -1580,6 +1580,16 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -1718,6 +1728,16 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -1763,6 +1783,16 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "403": { "description": "Account Creation is currently disabled", "content": { @@ -1773,6 +1803,16 @@ } } }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -1818,6 +1858,16 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "403": { "description": "Account Creation is currently disabled", "content": { @@ -1828,6 +1878,16 @@ } } }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -1873,6 +1933,16 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "403": { "description": "Account Creation is currently disabled", "content": { @@ -1883,6 +1953,16 @@ } } }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -1906,10 +1986,9 @@ { "name": "providerName", "in": "path", - "description": "The provider name.", + "description": "The provider name: github, google, microsoft, facebook, or windowslive (case sensitive).", "required": true, "schema": { - "minLength": 1, "type": "string" } } @@ -1942,7 +2021,7 @@ } }, "400": { - "description": "Invalid provider name.", + "description": "Invalid provider user id, or the account would have no sign-in method.", "content": { "application/problem\u002Bjson": { "schema": { @@ -1950,6 +2029,36 @@ } } } + }, + "401": { + "description": "Unauthorized", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "422": { + "description": "Validation error", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/HttpValidationProblemDetails" + } + } + } } } } @@ -1995,6 +2104,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -2068,6 +2187,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -2103,6 +2232,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -4264,6 +4403,16 @@ } } } + }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } }, @@ -4330,6 +4479,16 @@ } } } + }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -4458,6 +4617,16 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "The image file is invalid.", "content": { @@ -4507,6 +4676,16 @@ } } } + }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -4677,6 +4856,16 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -4732,6 +4921,16 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Verify Email Address Token has expired.", "content": { @@ -4776,6 +4975,16 @@ } } } + }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -6413,6 +6622,16 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -6835,6 +7054,16 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "500": { "description": "An error occurred while deleting one or more organizations.", "content": { @@ -7206,6 +7435,16 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "426": { "description": "Please upgrade your plan to add an additional user.", "content": { @@ -7268,6 +7507,16 @@ } } } + }, + "409": { + "description": "Conflict", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -14126,6 +14375,7 @@ "email_notifications_enabled", "is_email_address_verified", "verify_email_address_token_expiration", + "version", "is_active", "roles", "created_utc", @@ -14236,6 +14486,10 @@ "type": "string", "format": "date-time" }, + "version": { + "type": "string", + "description": "Repository concurrency metadata used to reject stale writes, preserved in cached snapshots.\nNew users have an empty version until persisted." + }, "is_active": { "type": "boolean", "description": "Gets or sets the users active state." diff --git a/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs index e2a822b0cc..4543bbb50e 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs @@ -22,6 +22,7 @@ using FluentRest; using Foundatio.Queues; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; using Foundatio.Repositories.Utility; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; @@ -563,7 +564,7 @@ public async Task MicrosoftAsync_WithLinkedIdentity_ReturnsToken() const string code = "microsoft-user"; var user = new User { EmailAddress = TestOAuthProviderClient.GetEmailAddress(code), FullName = "Microsoft User" }; user.MarkEmailAddressVerified(); - user.AddOAuthAccount("Microsoft", code, user.EmailAddress); + user.AddOAuthAccount(OAuthAccount.KnownProviders.Microsoft, code, user.EmailAddress); await _userRepository.AddAsync(user, o => o.ImmediateConsistency()); // Act @@ -758,11 +759,15 @@ public async Task LoginValidAsync() Assert.False(String.IsNullOrEmpty(result.Token)); } - [Fact] - public async Task RemoveExternalLoginAsync_WithLinkedAccount_RemovesAccount() + [Theory] + [InlineData("github")] + [InlineData("google")] + [InlineData("microsoft")] + [InlineData("facebook")] + [InlineData("windowslive")] + public async Task RemoveExternalLoginAsync_WithLinkedAccount_RemovesAccount(string providerName) { // Arrange - const string providerName = "github"; const string providerUserId = "github-remove-user"; var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); Assert.NotNull(user); @@ -786,20 +791,79 @@ public async Task RemoveExternalLoginAsync_WithLinkedAccount_RemovesAccount() Assert.DoesNotContain(updatedUser.OAuthAccounts, account => account.Provider == providerName && account.ProviderUserId == providerUserId); } - [Fact] - public Task RemoveExternalLoginAsync_WithoutProviderUserId_ReturnsBadRequest() + [Theory] + [InlineData("unknown", "github-invalid-provider", "provider_name")] + [InlineData("GitHub", "github-invalid-provider", "provider_name")] + [InlineData("GITHUB", "github-invalid-provider", "provider_name")] + [InlineData(" github", "github-invalid-provider", "provider_name")] + [InlineData("github ", "github-invalid-provider", "provider_name")] + [InlineData("github", null, null)] + [InlineData("github", "", null)] + [InlineData("github", " ", null)] + public async Task RemoveExternalLoginAsync_InvalidIdentity_IsRejectedWithoutChangingAccount(string providerName, string? providerUserId, string? errorKey) { // Arrange - var providerUserId = new ValueFromBody(String.Empty); + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(user); + user.AddOAuthAccount("github", "github-invalid-provider", user.EmailAddress); + await _userRepository.SaveAsync(user, options => options.ImmediateConsistency().Cache()); + long originalTokenCount = (await _tokenRepository.CountAsync()).Total; - // Act & Assert - return SendRequestAsync(r => r - .Post() - .AsTestOrganizationUser() + // Act + var problem = await SendRequestAsAsync(request => + { + request.Post().AsTestOrganizationUser().AppendPaths("auth", "unlink", providerName) + .Content(new ValueFromBody(providerUserId!)); + if (errorKey is null) + request.StatusCodeShouldBeBadRequest(); + else + request.StatusCodeShouldBeUnprocessableEntity(); + }); + + // Assert + Assert.NotNull(problem); + if (errorKey is null) + Assert.Equal("Invalid Provider Name or Provider User Id.", problem.Title); + else + Assert.Contains(errorKey, problem.Errors.Keys); + + var storedUser = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); + Assert.NotNull(storedUser); + Assert.Equal(user.AuthenticationVersion, storedUser.AuthenticationVersion); + Assert.Equal(user.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username)), + storedUser.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username))); + Assert.Equal(originalTokenCount, (await _tokenRepository.CountAsync()).Total); + } + + [Theory] + [InlineData(null)] + [InlineData("facebook")] + [InlineData("windowslive")] + public async Task RemoveExternalLoginAsync_DuplicateEntriesForOnlySignInMethod_PreservesAccount(string? retiredProvider) + { + // Arrange + const string code = "github-only-duplicate-login"; + var login = await SendExternalLoginAsync("github", code); + Assert.NotNull(login); + var user = await _userRepository.GetByEmailAddressAsync(TestOAuthProviderClient.GetEmailAddress(code)); + Assert.NotNull(user); + Assert.Null(user.Password); + user.OAuthAccounts.Add(Assert.Single(user.OAuthAccounts)); + if (retiredProvider is not null) + user.AddOAuthAccount(retiredProvider, "retired-user", user.EmailAddress); + await _userRepository.SaveAsync(user, options => options.ImmediateConsistency()); + + // Act + await SendRequestAsync(request => request.Post().BearerToken(login.Token) .AppendPaths("auth", "unlink", "github") - .Content(providerUserId) - .StatusCodeShouldBeBadRequest() - ); + .Content(new ValueFromBody(code)).StatusCodeShouldBeBadRequest()); + + // Assert + var storedUser = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); + Assert.NotNull(storedUser); + Assert.Equal(retiredProvider is null ? 2 : 3, storedUser.OAuthAccounts.Count); + Assert.Equal(user.AuthenticationVersion, storedUser.AuthenticationVersion); + await SendRequestAsync(request => request.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeOk()); } [Fact] @@ -1342,6 +1406,8 @@ public async Task ChangePassword_StoredOldTokenSurvivesCleanup_CannotAuthenticat Assert.NotNull(login); var oldToken = await _tokenRepository.GetByIdAsync(login.Token, o => o.Cache(false)); Assert.NotNull(oldToken); + var staleUser = await _userRepository.GetByIdAsync(oldToken.UserId!, o => o.Cache(false)); + Assert.NotNull(staleUser); await SendRequestAsync(r => r.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeOk()); // Act @@ -1355,11 +1421,15 @@ public async Task ChangePassword_StoredOldTokenSurvivesCleanup_CannotAuthenticat // Assert Assert.NotNull(changed); Assert.NotEqual(login.Token, changed.Token); + staleUser.FullName = "Stale profile update"; + await Assert.ThrowsAsync(() => _userRepository.SaveAsync(staleUser)); await SendRequestAsync(r => r.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeUnauthorized()); await SendRequestAsync(r => r.BearerToken(changed.Token).AppendPath("users/me").StatusCodeShouldBeOk()); await SendRequestAsync(r => r.BasicAuthorization("client", login.Token).AppendPath("users/me").StatusCodeShouldBeUnauthorized()); var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); Assert.NotNull(user); + Assert.True(user.IsCorrectPassword(newPassword)); + Assert.False(user.IsCorrectPassword(SampleDataService.TEST_USER_PASSWORD)); Assert.False(String.IsNullOrEmpty(user.AuthenticationVersion)); Assert.Equal(user.AuthenticationVersion, (await _tokenRepository.GetByIdAsync(changed.Token))!.AuthenticationVersion); } @@ -2000,6 +2070,375 @@ await SendRequestAsync(r => r Assert.False(token.IsSuspended); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ExternalLogin_InactiveUser_IsRejectedWithoutChangingAccountOrInvite(bool hasExistingLink) + { + // Arrange + const string code = "github-inactive-user"; + string email = TestOAuthProviderClient.GetEmailAddress(code); + var user = new User + { + EmailAddress = email, + FullName = "Inactive User", + IsActive = false, + Roles = new HashSet([AuthorizationRoles.Client, AuthorizationRoles.User]) + }; + user.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider); + if (hasExistingLink) + user.AddOAuthAccount("github", code, email); + await _userRepository.AddAsync(user, options => options.ImmediateConsistency()); + var organization = (await _organizationRepository.GetAllAsync()).Documents.First(); + var invite = new Invite + { + Token = StringExtensions.GetNewToken(), + EmailAddress = email, + DateAdded = TimeProvider.GetUtcNow().UtcDateTime + }; + organization.Invites.Add(invite); + await _organizationRepository.SaveAsync(organization, options => options.ImmediateConsistency()); + + // Act + await SendRequestAsync(request => request + .Post() + .AppendPaths("auth", "github") + .Content(new ExternalAuthInfo + { + ClientId = "client-id", + Code = code, + InviteToken = invite.Token, + RedirectUri = "http://localhost/callback" + }) + .StatusCodeShouldBeUnauthorized()); + + // Assert + var storedUser = await _userRepository.GetByIdAsync(user.Id, options => options.ImmediateConsistency()); + Assert.NotNull(storedUser); + Assert.False(storedUser.IsActive); + Assert.False(storedUser.IsEmailAddressVerified); + Assert.Equal(user.VerifyEmailAddressToken, storedUser.VerifyEmailAddressToken); + Assert.Equal(user.VerifyEmailAddressTokenExpiration, storedUser.VerifyEmailAddressTokenExpiration); + Assert.Equal(user.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username)), + storedUser.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username))); + Assert.Empty(storedUser.OrganizationIds); + var tokens = await _tokenRepository.GetByTypeAndUserIdAsync(TokenType.Authentication, user.Id, options => options.ImmediateConsistency()); + Assert.Empty(tokens.Documents); + var storedOrganization = await _organizationRepository.GetByIdAsync(organization.Id, options => options.ImmediateConsistency()); + Assert.NotNull(storedOrganization); + Assert.Contains(storedOrganization.Invites, candidate => candidate.Token == invite.Token); + } + + [Fact] + public async Task ExternalLogin_InactiveProviderOwner_DoesNotTransferAccount() + { + // Arrange + const string code = "github-inactive-provider-owner"; + var owner = new User + { + EmailAddress = TestOAuthProviderClient.GetEmailAddress(code), + FullName = "Inactive Owner", + IsActive = false + }; + owner.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider); + owner.AddOAuthAccount("github", code, owner.EmailAddress); + await _userRepository.AddAsync(owner, options => options.ImmediateConsistency()); + var currentUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(currentUser); + var originalAccounts = currentUser.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username)).ToArray(); + long originalTokenCount = (await _tokenRepository.CountAsync()).Total; + + // Act + await SendRequestAsync(request => request + .Post() + .AsTestOrganizationUser() + .AppendPaths("auth", "github") + .Content(new ExternalAuthInfo + { + ClientId = "client-id", + Code = code, + RedirectUri = "http://localhost/callback" + }) + .StatusCodeShouldBeUnauthorized()); + + // Assert + var storedOwner = await _userRepository.GetByIdAsync(owner.Id, options => options.ImmediateConsistency()); + Assert.NotNull(storedOwner); + Assert.False(storedOwner.IsEmailAddressVerified); + Assert.Equal(owner.VerifyEmailAddressToken, storedOwner.VerifyEmailAddressToken); + Assert.Equal(code, Assert.Single(storedOwner.OAuthAccounts).ProviderUserId); + var storedCurrentUser = await _userRepository.GetByIdAsync(currentUser.Id, options => options.ImmediateConsistency()); + Assert.NotNull(storedCurrentUser); + Assert.Equal(originalAccounts, storedCurrentUser.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username))); + Assert.Equal(originalTokenCount, (await _tokenRepository.CountAsync()).Total); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ExternalLogin_InterruptedTransfer_CompletesTransferAndKeepsPreviousSessionRevoked(bool destinationCommitted) + { + // Arrange + const string code = "github-failed-transfer"; + var owner = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + var destination = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(owner); + Assert.NotNull(destination); + owner.AddOAuthAccount("github", code, owner.EmailAddress); + await _userRepository.SaveAsync(owner); + var login = await SendRequestAsAsync(request => request.Post().AppendPath("auth/login") + .Content(new Login { Email = owner.EmailAddress, Password = SampleDataService.TEST_USER_PASSWORD }) + .StatusCodeShouldBeOk()); + Assert.NotNull(login); + owner.OAuthAccounts.Clear(); + owner.AuthenticationVersion = StringExtensions.GetNewToken(); + await _userRepository.SaveAsync(owner); + if (destinationCommitted) + { + destination.AddOAuthAccount("github", code, owner.EmailAddress); + await _userRepository.SaveAsync(destination); + } + + var authInfo = new ExternalAuthInfo { ClientId = "client-id", Code = code, RedirectUri = "http://localhost/callback" }; + + // Act + var retry = await SendRequestAsAsync(request => request.Post().AsTestOrganizationUser().AppendPaths("auth", "github") + .Content(authInfo).StatusCodeShouldBeOk()); + + // Assert + var storedOwner = await _userRepository.GetByIdAsync(owner.Id); + Assert.NotNull(storedOwner); + Assert.DoesNotContain(storedOwner.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == code); + Assert.Equal(owner.AuthenticationVersion, storedOwner.AuthenticationVersion); + Assert.NotNull(retry); + var matchingUsers = await _userRepository.GetUsersByOAuthProviderAsync("github", code, TestCancellationToken); + Assert.Equal(destination.Id, Assert.Single(matchingUsers).Id); + await SendRequestAsync(request => request.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeUnauthorized()); + } + + [Fact] + public async Task ForgotPassword_ConcurrentUserUpdate_MatchesUnknownEmailResponse() + { + // Arrange + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(user); + var mailQueue = GetService>(); + long enqueued = (await mailQueue.GetQueueStatsAsync()).Enqueued; + bool changed = false; + using var subscription = _userRepository.DocumentsSaving.AddHandler(async (_, args) => + { + if (changed || !args.Documents.Any(document => document.Value.Id == user.Id)) + return; + + changed = true; + var current = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); + Assert.NotNull(current); + current.FullName = "Concurrent profile update"; + await _userRepository.SaveAsync(current); + }); + + // Act + var known = await SendRequestAsync(request => request.AppendPaths("auth", "forgot-password", user.EmailAddress).StatusCodeShouldBeOk()); + var unknown = await SendRequestAsync(request => request.AppendPaths("auth", "forgot-password", "unknown@localhost.test").StatusCodeShouldBeOk()); + var persisted = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); + + // Assert + Assert.True(changed); + Assert.Equal(HttpStatusCode.OK, known.StatusCode); + Assert.Equal(unknown.StatusCode, known.StatusCode); + Assert.Equal(await unknown.Content.ReadAsStringAsync(TestCancellationToken), await known.Content.ReadAsStringAsync(TestCancellationToken)); + Assert.NotNull(persisted); + Assert.Equal("Concurrent profile update", persisted.FullName); + Assert.Equal(user.PasswordResetToken, persisted.PasswordResetToken); + Assert.Equal(enqueued, (await mailQueue.GetQueueStatsAsync()).Enqueued); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ExternalLogin_ActiveProviderOwner_TransfersAccountAndRevokesPreviousSession(bool hasDuplicateEntries) + { + // Arrange + const string code = "github-transfer-account"; + var owner = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + var currentUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(owner); + Assert.NotNull(currentUser); + owner.AddOAuthAccount("github", code, owner.EmailAddress); + if (hasDuplicateEntries) + owner.OAuthAccounts.Add(owner.OAuthAccounts.Single(account => account.Provider == "github" && account.ProviderUserId == code)); + await _userRepository.SaveAsync(owner, options => options.ImmediateConsistency()); + var login = await SendRequestAsAsync(request => request.Post().AppendPath("auth/login") + .Content(new Login { Email = owner.EmailAddress, Password = SampleDataService.TEST_USER_PASSWORD }) + .StatusCodeShouldBeOk()); + Assert.NotNull(login); + await SendRequestAsync(request => request.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeOk()); + + // Act + var result = await SendRequestAsAsync(request => request + .Post() + .AsTestOrganizationUser() + .AppendPaths("auth", "github") + .Content(new ExternalAuthInfo { ClientId = "client-id", Code = code, RedirectUri = "http://localhost/callback" }) + .StatusCodeShouldBeOk()); + + // Assert + Assert.NotNull(result); + var token = await _tokenRepository.GetByIdAsync(result.Token); + Assert.NotNull(token); + Assert.Equal(currentUser.Id, token.UserId); + var storedOwner = await _userRepository.GetByIdAsync(owner.Id, options => options.ImmediateConsistency()); + Assert.NotNull(storedOwner); + Assert.DoesNotContain(storedOwner.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == code); + Assert.NotEqual(owner.AuthenticationVersion, storedOwner.AuthenticationVersion); + var matchingUsers = await _userRepository.GetUsersByOAuthProviderAsync("github", code, TestCancellationToken); + Assert.Equal(currentUser.Id, Assert.Single(matchingUsers).Id); + await SendRequestAsync(request => request.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeUnauthorized()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ExternalLogin_ExistingIdentity_RefreshesAccountMetadata(bool isAuthenticated) + { + // Arrange + const string code = "github-refresh-metadata"; + var owner = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(owner); + owner.MarkEmailAddressVerified(); + owner.AddOAuthAccount("github", code, "outdated@example.test", new SettingsDictionary + { + ["name"] = "Outdated name", ["email"] = "outdated@example.test", ["retained"] = "keep" + }); + int originalAccountCount = owner.OAuthAccounts.Count; + owner.OAuthAccounts.Add(new OAuthAccount + { + Provider = "github", ProviderUserId = code, Username = "stale-duplicate@example.test", + ExtraData = new SettingsDictionary { ["email"] = "stale-duplicate@example.test" } + }); + await _userRepository.SaveAsync(owner, options => options.ImmediateConsistency()); + + // Act + var result = await SendRequestAsAsync(request => + { + if (isAuthenticated) + request.AsTestOrganizationUser(); + + request.Post().AppendPaths("auth", "github") + .Content(new ExternalAuthInfo { ClientId = "client-id", Code = code, RedirectUri = "http://localhost/callback" }) + .StatusCodeShouldBeOk(); + }); + var stored = await _userRepository.GetByIdAsync(owner.Id, options => options.Cache(false)); + + // Assert + Assert.NotNull(result); + Assert.NotNull(stored); + var token = await _tokenRepository.GetByIdAsync(result.Token); + Assert.NotNull(token); + Assert.Equal(owner.Id, token.UserId); + Assert.Equal(originalAccountCount, stored.OAuthAccounts.Count); + var account = Assert.Single(stored.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == code); + Assert.Equal(TestOAuthProviderClient.GetEmailAddress(code), account.Username); + Assert.Equal("github User", account.ExtraData["name"]); + Assert.Equal(TestOAuthProviderClient.GetEmailAddress(code), account.ExtraData["email"]); + Assert.Equal("keep", account.ExtraData["retained"]); + Assert.DoesNotContain(stored.OAuthAccounts, account => account.EmailAddress() == "stale-duplicate@example.test"); + Assert.Equal(owner.EmailAddress, stored.EmailAddress); + Assert.Equal(owner.FullName, stored.FullName); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ExternalLogin_ConcurrentPasswordChange_DoesNotRestoreOldCredentials(bool isAuthenticated) + { + // Arrange + const string code = "github-concurrent-password-change"; + const string newPassword = "ChangedPassword2$"; + var owner = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(owner); + owner.AddOAuthAccount("github", code, owner.EmailAddress); + await _userRepository.SaveAsync(owner, options => options.ImmediateConsistency()); + bool passwordChanged = false; + TokenResult? replacementToken = null; + using var subscription = _userRepository.DocumentsSaving.AddHandler(async (_, args) => + { + if (passwordChanged || !args.Documents.Any(document => document.Value.Id == owner.Id)) + return; + + passwordChanged = true; + replacementToken = await SendRequestAsAsync(request => request.AsTestOrganizationUser() + .Post().AppendPath("auth/change-password") + .Content(new ChangePasswordModel { CurrentPassword = SampleDataService.TEST_ORG_USER_PASSWORD, Password = newPassword }) + .StatusCodeShouldBeOk()); + }); + using var client = _server.CreateClient(); + if (isAuthenticated) + client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Basic", + Convert.ToBase64String(Encoding.UTF8.GetBytes($"{SampleDataService.TEST_ORG_USER_EMAIL}:{SampleDataService.TEST_ORG_USER_PASSWORD}"))); + + // Act + using var response = await client.PostAsJsonAsync("api/v2/auth/github", new ExternalAuthInfo + { + ClientId = "client-id", Code = code, RedirectUri = "http://localhost/callback" + }, TestCancellationToken); + var stored = await _userRepository.GetByIdAsync(owner.Id, options => options.Cache(false)); + + // Assert + Assert.True(passwordChanged); + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); + Assert.NotNull(stored); + Assert.True(stored.IsCorrectPassword(newPassword)); + Assert.False(stored.IsCorrectPassword(SampleDataService.TEST_ORG_USER_PASSWORD)); + Assert.NotNull(replacementToken); + await SendRequestAsync(request => request.BearerToken(replacementToken.Token).AppendPath("users/me").StatusCodeShouldBeOk()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ExternalLogin_DuplicateProviderOwners_IsRejectedWithoutChangingAccountsOrTokens(bool isAuthenticated) + { + // Arrange + const string code = "github-duplicate-owners"; + var owners = new List(); + foreach (string email in new[] { SampleDataService.TEST_USER_EMAIL, SampleDataService.TEST_ORG_USER_EMAIL }) + { + var owner = await _userRepository.GetByEmailAddressAsync(email); + Assert.NotNull(owner); + owner.AddOAuthAccount("github", code, owner.EmailAddress); + owners.Add(owner); + } + await _userRepository.SaveAsync(owners, options => options.ImmediateConsistency()); + long originalTokenCount = (await _tokenRepository.CountAsync()).Total; + + // Act + var problem = await SendRequestAsAsync(request => + { + if (isAuthenticated) + request.AsTestOrganizationUser(); + + request.Post().AppendPaths("auth", "github") + .Content(new ExternalAuthInfo { ClientId = "client-id", Code = code, RedirectUri = "http://localhost/callback" }) + .StatusCodeShouldBeUnauthorized(); + }); + + // Assert + Assert.NotNull(problem); + Assert.Equal("Login failed.", problem.Title); + Assert.Equal(originalTokenCount, (await _tokenRepository.CountAsync()).Total); + Assert.Null(await _userRepository.GetByEmailAddressAsync(TestOAuthProviderClient.GetEmailAddress(code))); + foreach (var owner in owners) + { + var storedOwner = await _userRepository.GetByIdAsync(owner.Id, options => options.ImmediateConsistency()); + Assert.NotNull(storedOwner); + Assert.Equal(owner.AuthenticationVersion, storedOwner.AuthenticationVersion); + Assert.Equal(owner.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username)), + storedOwner.OAuthAccounts.Select(account => (account.Provider, account.ProviderUserId, account.Username))); + Assert.Equal(owner.IsEmailAddressVerified, storedOwner.IsEmailAddressVerified); + } + } + private async Task AssertExternalLoginAsync(TokenResult? result, string providerName, string providerUserId, bool isEmailVerified = true) { Assert.NotNull(result); diff --git a/tests/Exceptionless.Tests/Api/Endpoints/ExternalAccountConcurrencyTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/ExternalAccountConcurrencyTests.cs new file mode 100644 index 0000000000..44179c5062 --- /dev/null +++ b/tests/Exceptionless.Tests/Api/Endpoints/ExternalAccountConcurrencyTests.cs @@ -0,0 +1,299 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text; +using Exceptionless.Core.Extensions; +using Exceptionless.Core.Repositories; +using Exceptionless.Core.Repositories.Configuration; +using Exceptionless.Core.Models; +using Exceptionless.Tests.Utility; +using Exceptionless.Tests.Extensions; +using Exceptionless.Core.Services; +using Exceptionless.Core.Utility; +using Exceptionless.Web.Models; +using FluentRest; +using Foundatio.Repositories; +using Foundatio.Caching; +using Foundatio.Repositories.Models; +using Xunit; + +namespace Exceptionless.Tests.Api.Endpoints; + +public sealed class ExternalAccountConcurrencyTests : IntegrationTestsBase +{ + private readonly IUserRepository _users; + private readonly ITokenRepository _tokens; + + public ExternalAccountConcurrencyTests(ITestOutputHelper output, AppWebHostFactory factory) : base(output, factory) + { + _users = GetService(); + _tokens = GetService(); + } + + protected override async Task ResetDataAsync() + { + await base.ResetDataAsync(); + var options = GetService(); + options.EnableAccountCreation = true; + options.GitHubId = "github-client-id"; + options.GitHubSecret = "github-client-secret"; + await GetService().CreateDataAsync(); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ExternalLogin_ConcurrentRequestsForSameIdentity_KeepOneOwner(bool linkExistingUsers) + { + // Arrange + const string code = "concurrent-external-account"; + using var firstClient = _server.CreateClient(); + using var secondClient = _server.CreateClient(); + if (linkExistingUsers) + { + SetAuthorization(firstClient, SampleDataService.TEST_USER_EMAIL, SampleDataService.TEST_USER_PASSWORD); + SetAuthorization(secondClient, SampleDataService.TEST_ORG_USER_EMAIL, SampleDataService.TEST_ORG_USER_PASSWORD); + } + + // Act + var results = await RunConcurrentRequestsAsync(firstClient, code, secondClient, code); + + // Assert + Assert.All(results, result => Assert.NotNull(result)); + var candidates = await _users.FindAsync(query => query.FieldEquals(user => user.OAuthAccounts.First().ProviderUserId, code)); + var owner = Assert.Single(candidates.Documents); + Assert.Single(owner.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == code); + var tokens = await _tokens.GetByIdsAsync(results.Select(result => result!.Token).ToArray()); + Assert.Contains(tokens, token => token.UserId == owner.Id); + } + + [Theory] + [InlineData("identity")] + [InlineData("email")] + [InlineData("user")] + public async Task ExternalLogin_CanceledLookup_ReleasesLocksForNextRequest(string stage) + { + // Arrange + const string code = "canceled-external-account"; + using var client = _server.CreateClient(); + using var cancellation = CancellationTokenSource.CreateLinkedTokenSource(TestCancellationToken); + var users = Assert.IsType(_users); + if (stage == "user") + { + var user = new User { FullName = "Canceled lookup user", EmailAddress = TestOAuthProviderClient.GetEmailAddress(code) }; + user.MarkEmailAddressVerified(); + await users.AddAsync(user, options => options.ImmediateConsistency()); + } + + bool canceled = false; + int queries = 0; + using var subscription = users.BeforeQuery.AddHandler((_, _) => + { + queries++; + if (!canceled && ((stage == "identity" && queries == 1) || (stage == "email" && queries == 2))) + { + canceled = true; + cancellation.Cancel(); + } + + return Task.CompletedTask; + }); + + using var getSubscription = users.BeforeGet.AddHandler((_, _) => + { + if (!canceled && stage == "user") + { + canceled = true; + cancellation.Cancel(); + } + + return Task.CompletedTask; + }); + + // Act + var exception = await Record.ExceptionAsync(async () => + { + using var response = await client.PostAsJsonAsync("api/v2/auth/github", new ExternalAuthInfo + { + ClientId = "client-id", Code = code, RedirectUri = "http://localhost/callback" + }, cancellation.Token); + }); + var nextResult = await SendExternalLoginAsync(client, code); + + // Assert + Assert.True(canceled); + Assert.IsAssignableFrom(exception); + Assert.NotNull(nextResult); + Assert.NotNull(await _tokens.GetByIdAsync(nextResult.Token)); + } + + [Fact] + public async Task ExternalLogin_DestinationVersionConflict_PreservesPreviousOwnerAndConcurrentChanges() + { + // Arrange + const string code = "external-account-transfer-conflict"; + const string newPassword = "ChangedDestination2$"; + var owner = await _users.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + var destination = await _users.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(owner); + Assert.NotNull(destination); + owner.AddOAuthAccount("github", code, "original-provider-name", new SettingsDictionary { ["profile"] = "original" }); + await _users.SaveAsync(owner); + string? originalAuthenticationVersion = owner.AuthenticationVersion; + bool destinationChanged = false; + TokenResult? replacementToken = null; + using var subscription = _users.DocumentsSaving.AddHandler(async (_, args) => + { + if (destinationChanged || !args.Documents.Any(document => document.Value.Id == destination.Id)) + return; + + destinationChanged = true; + replacementToken = await SendRequestAsAsync(request => request.AsTestOrganizationUser() + .Post().AppendPath("auth/change-password") + .Content(new ChangePasswordModel { CurrentPassword = SampleDataService.TEST_ORG_USER_PASSWORD, Password = newPassword }) + .StatusCodeShouldBeOk()); + var currentOwner = await _users.GetByIdAsync(owner.Id, options => options.Cache(false)) + ?? throw new InvalidOperationException("The previous owner no longer exists."); + currentOwner.FullName = "Concurrent owner update"; + await _users.SaveAsync(currentOwner); + }); + using var client = _server.CreateClient(); + SetAuthorization(client, SampleDataService.TEST_ORG_USER_EMAIL, SampleDataService.TEST_ORG_USER_PASSWORD); + + // Act + using var response = await client.PostAsJsonAsync("api/v2/auth/github", new ExternalAuthInfo + { + ClientId = "client-id", Code = code, RedirectUri = "http://localhost/callback" + }, TestCancellationToken); + var storedOwner = await _users.GetByIdAsync(owner.Id, options => options.Cache(false)); + var storedDestination = await _users.GetByIdAsync(destination.Id, options => options.Cache(false)); + var owners = await _users.GetUsersByOAuthProviderAsync("github", code, TestCancellationToken); + + // Assert + Assert.True(destinationChanged); + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); + Assert.Equal(owner.Id, Assert.Single(owners).Id); + Assert.NotNull(storedOwner); + Assert.Equal("Concurrent owner update", storedOwner.FullName); + Assert.NotEqual(originalAuthenticationVersion, storedOwner.AuthenticationVersion); + var restoredAccount = Assert.Single(storedOwner.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == code); + Assert.Equal("original-provider-name", restoredAccount.Username); + Assert.Equal("original", restoredAccount.ExtraData["profile"]); + Assert.NotNull(storedDestination); + Assert.True(storedDestination.IsCorrectPassword(newPassword)); + Assert.DoesNotContain(storedDestination.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == code); + Assert.NotNull(replacementToken); + await SendRequestAsync(request => request.BearerToken(replacementToken.Token).AppendPath("users/me").StatusCodeShouldBeOk()); + } + + [Fact] + public async Task ExternalLogin_StaleCachedEmailMiss_LinksStoredUserWithoutCreatingDuplicate() + { + // Arrange + const string code = "external-account-stale-email-miss"; + var user = new User { FullName = "Existing email owner", EmailAddress = TestOAuthProviderClient.GetEmailAddress(code) }; + user.MarkEmailAddressVerified(); + await _users.AddAsync(user, options => options.ImmediateConsistency()); + var cache = new ScopedCacheClient(GetService().Cache, nameof(User)); + await cache.SetAsync>>($"Email:{user.EmailAddress}", []); + var staleEmailLookup = await _users.GetByEmailAddressAsync(user.EmailAddress); + long originalUserCount = (await _users.CountAsync()).Total; + using var client = _server.CreateClient(); + + // Act + var result = await SendExternalLoginAsync(client, code); + + // Assert + Assert.Null(staleEmailLookup); + Assert.NotNull(result); + var token = await _tokens.GetByIdAsync(result.Token); + Assert.NotNull(token); + Assert.Equal(user.Id, token.UserId); + Assert.Equal(originalUserCount, (await _users.CountAsync()).Total); + var storedUser = await _users.GetByIdAsync(user.Id, options => options.Cache(false)); + Assert.NotNull(storedUser); + Assert.Single(storedUser.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == code); + } + + [Fact] + public async Task ExternalLogin_IdentityUnlinkedAfterLookup_RejectsStaleOwner() + { + // Arrange + const string code = "external-account-unlinked-during-login"; + var user = await _users.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(user); + user.AddOAuthAccount("github", code, user.EmailAddress); + await _users.SaveAsync(user, options => options.ImmediateConsistency()); + using var loginClient = _server.CreateClient(); + using var unlinkClient = _server.CreateClient(); + SetAuthorization(unlinkClient, SampleDataService.TEST_ORG_USER_EMAIL, SampleDataService.TEST_ORG_USER_PASSWORD); + bool unlinked = false; + HttpStatusCode? unlinkStatus = null; + using var subscription = _users.AfterQuery.AddHandler(async (_, args) => + { + if (unlinked || args.Result is not FindResults result || !result.Documents.Any(candidate => candidate.Id == user.Id)) + return; + + unlinked = true; + using var response = await unlinkClient.PostAsJsonAsync("api/v2/auth/unlink/github", new ValueFromBody(code), TestCancellationToken); + unlinkStatus = response.StatusCode; + }); + + // Act + using var loginResponse = await loginClient.PostAsJsonAsync("api/v2/auth/github", new ExternalAuthInfo + { + ClientId = "client-id", Code = code, RedirectUri = "http://localhost/callback" + }, TestCancellationToken); + var storedUser = await _users.GetByIdAsync(user.Id, options => options.Cache(false)); + + // Assert + Assert.True(unlinked); + Assert.Equal(HttpStatusCode.OK, unlinkStatus); + Assert.Equal(HttpStatusCode.Unauthorized, loginResponse.StatusCode); + Assert.NotNull(storedUser); + Assert.DoesNotContain(storedUser.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == code); + } + + [Fact] + public async Task ExternalLogin_ConcurrentDifferentIdentitiesForSameUser_PreservesBothLinks() + { + // Arrange + using var firstClient = _server.CreateClient(); + using var secondClient = _server.CreateClient(); + SetAuthorization(firstClient, SampleDataService.TEST_ORG_USER_EMAIL, SampleDataService.TEST_ORG_USER_PASSWORD); + SetAuthorization(secondClient, SampleDataService.TEST_ORG_USER_EMAIL, SampleDataService.TEST_ORG_USER_PASSWORD); + var currentUser = await _users.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(currentUser); + + // Act + var results = await RunConcurrentRequestsAsync(firstClient, "concurrent-first-account", secondClient, "concurrent-second-account"); + + // Assert + Assert.All(results, result => Assert.NotNull(result)); + var storedUser = await _users.GetByIdAsync(currentUser.Id, options => options.Cache(false)); + Assert.NotNull(storedUser); + Assert.Single(storedUser.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == "concurrent-first-account"); + Assert.Single(storedUser.OAuthAccounts, account => account.Provider == "github" && account.ProviderUserId == "concurrent-second-account"); + } + + private Task RunConcurrentRequestsAsync(HttpClient firstClient, string firstCode, HttpClient secondClient, string secondCode) + { + return Task.WhenAll(SendExternalLoginAsync(firstClient, firstCode), SendExternalLoginAsync(secondClient, secondCode)); + } + + private static void SetAuthorization(HttpClient client, string email, string password) + { + client.DefaultRequestHeaders.Authorization = new("Basic", Convert.ToBase64String(Encoding.UTF8.GetBytes($"{email}:{password}"))); + } + + private async Task SendExternalLoginAsync(HttpClient client, string code) + { + using var response = await client.PostAsJsonAsync("api/v2/auth/github", new ExternalAuthInfo + { + ClientId = "client-id", + Code = code, + RedirectUri = "http://localhost/callback" + }, TestCancellationToken); + response.EnsureSuccessStatusCode(); + return await response.Content.ReadFromJsonAsync(TestCancellationToken); + } +} diff --git a/tests/Exceptionless.Tests/Api/Endpoints/MicrosoftAuthEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/MicrosoftAuthEndpointTests.cs index 2e5a025912..ebd5e3d96c 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/MicrosoftAuthEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/MicrosoftAuthEndpointTests.cs @@ -49,7 +49,7 @@ public async Task MicrosoftAsync_AuthenticatedLinkWithDifferentEmail_ReplacesLeg var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); Assert.NotNull(user); user.MarkEmailAddressVerified(); - user.AddOAuthAccount("WindowsLive", "legacy-user", user.EmailAddress); + user.AddOAuthAccount(OAuthAccount.KnownProviders.WindowsLive, "legacy-user", user.EmailAddress); await _userRepository.SaveAsync(user, o => o.ImmediateConsistency().Cache()); // Act @@ -72,8 +72,8 @@ public async Task MicrosoftAsync_ExistingModernIdentity_MarksEmailVerified() const string code = "existing-microsoft-user"; var user = CreateUser(TestOAuthProviderClient.GetEmailAddress(code)); user.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider); - user.AddOAuthAccount("WindowsLive", "legacy-user", user.EmailAddress); - user.AddOAuthAccount("Microsoft", code, user.EmailAddress); + user.AddOAuthAccount(OAuthAccount.KnownProviders.WindowsLive, "legacy-user", user.EmailAddress); + user.AddOAuthAccount(OAuthAccount.KnownProviders.Microsoft, code, user.EmailAddress); await _userRepository.AddAsync(user, o => o.ImmediateConsistency()); // Act @@ -163,7 +163,7 @@ public async Task MicrosoftAsync_UnverifiedAuthenticatedUser_CanLinkIdentity() var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); Assert.NotNull(user); user.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider); - user.AddOAuthAccount("WindowsLive", "legacy-user", user.EmailAddress); + user.AddOAuthAccount(OAuthAccount.KnownProviders.WindowsLive, "legacy-user", user.EmailAddress); await _userRepository.SaveAsync(user, o => o.ImmediateConsistency().Cache()); // Act diff --git a/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs index 890328f40f..7c2b1d1a79 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs @@ -1057,8 +1057,8 @@ public async Task UpdateEmailAddressAsync_ValidEmail_ReturnsResult() } [Theory] - [InlineData("Microsoft")] - [InlineData("Google")] + [InlineData(OAuthAccount.KnownProviders.Microsoft)] + [InlineData(OAuthAccount.KnownProviders.Google)] public async Task UpdateEmailAddressAsync_LinkedProviderEmail_MarksAddressVerified(string provider) { // Arrange diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index eac6a0f929..4d4fc31584 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -1,6 +1,7 @@ using System.Security.Claims; using Exceptionless.Core; using Exceptionless.Core.Authentication; +using Exceptionless.Core.Authorization; using Exceptionless.Core.Configuration; using Exceptionless.Core.Extensions; using Exceptionless.Core.Mail; @@ -15,8 +16,10 @@ using Exceptionless.Web.Models; using Exceptionless.Web.Security; using Foundatio.Caching; +using Foundatio.Lock; using Foundatio.Mediator; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; using Foundatio.Repositories.Models; using Microsoft.AspNetCore.Http; using Moq; @@ -29,11 +32,62 @@ public sealed class AuthHandlerTests : TestWithServices public AuthHandlerTests(ITestOutputHelper output) : base(output) { } [Theory] - [InlineData(false, false)] - [InlineData(true, false)] - [InlineData(false, true)] - [InlineData(true, true)] - public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOwnerSessions(bool cleanupFails, bool hasPreviousVersion) + [InlineData(false)] + [InlineData(true)] + public async Task Handle_ExternalAccountLockUnavailable_IsRejectedBeforeRepositoryAccess(bool unlink) + { + // Arrange + var users = new Mock(MockBehavior.Strict); + var tokens = new Mock(MockBehavior.Strict); + var oauthTokens = new Mock(MockBehavior.Strict); + var providerClient = new Mock(MockBehavior.Strict); + providerClient.Setup(client => client.GetGitHubUserInfoAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new OAuth2.Models.UserInfo { ProviderName = "github", Id = "locked-account", Email = "locked@example.com" }); + var locks = new Mock(MockBehavior.Strict); + locks.Setup(provider => provider.TryAcquireAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((ILock?)null); + locks.Setup(provider => provider.AcquireAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new LockAcquisitionTimeoutException("Account lock unavailable")); + var options = GetService().AuthOptions; + options.GitHubId = "test-client"; + options.GitHubSecret = "test-secret"; + var handler = CreateHandler(users.Object, tokens.Object, oauthTokens.Object, providerClient.Object, locks.Object); + + var context = new DefaultHttpContext(); + var user = new User { Id = TestConstants.UserId, EmailAddress = "locked@example.com", AuthenticationVersion = "initial" }; + user.AddOAuthAccount("github", "locked-account", user.EmailAddress); + context.Request.SetUser(user); + + // Act + var result = unlink + ? await handler.Handle(new RemoveExternalLogin("github", "locked-account", context)) + : await handler.Handle(new GitHubLogin(new ExternalAuthInfo(), context)); + + // Assert + Assert.Equal(ResultStatus.Unauthorized, result.Status); + users.VerifyNoOtherCalls(); + tokens.VerifyNoOtherCalls(); + oauthTokens.VerifyNoOtherCalls(); + if (unlink) + { + providerClient.VerifyNoOtherCalls(); + Assert.Equal("initial", user.AuthenticationVersion); + Assert.Single(user.OAuthAccounts); + } + else + { + providerClient.VerifyAll(); + } + } + + + [Theory] + [InlineData(false, false, false)] + [InlineData(true, false, false)] + [InlineData(false, true, false)] + [InlineData(true, true, false)] + [InlineData(false, true, true)] + public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOwnerSessions(bool cleanupFails, bool hasPreviousVersion, bool cancelAfterSave) { // Arrange const string provider = "github"; @@ -48,12 +102,15 @@ public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOw current.MarkEmailAddressVerified(); await GetService().ValidateAndThrowAsync(displaced); await GetService().ValidateAndThrowAsync(current); - var context = new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity(IdentityUtils.UserAuthenticationType)) }; + using var cancellation = CancellationTokenSource.CreateLinkedTokenSource(TestCancellationToken); + var context = new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity(IdentityUtils.UserAuthenticationType)), RequestAborted = cancellation.Token }; context.Request.SetUser(current); var calls = new List(); string? persistedVersion = null; var users = new Mock(MockBehavior.Strict); - users.Setup(repository => repository.GetUserByOAuthProviderAsync(provider, providerId)).ReturnsAsync(displaced); + users.Setup(repository => repository.GetUsersByOAuthProviderAsync(provider, providerId, It.IsAny())).ReturnsAsync(new[] { displaced }); + users.Setup(repository => repository.GetByIdAsync(displaced.Id, It.IsAny>())).ReturnsAsync(displaced); + users.Setup(repository => repository.GetByIdAsync(current.Id, It.IsAny>())).ReturnsAsync(current); users.Setup(repository => repository.SaveAsync(displaced, It.IsAny>())) .Callback(() => { @@ -61,7 +118,12 @@ public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOw calls.Add("save-displaced"); }).ReturnsAsync(displaced); users.Setup(repository => repository.SaveAsync(current, It.IsAny>())) - .Callback(() => calls.Add("save-current")).ReturnsAsync(current); + .Callback(() => + { + calls.Add("save-current"); + if (cancelAfterSave) + cancellation.Cancel(); + }).ReturnsAsync(current); var tokens = new Mock(MockBehavior.Strict); tokens.Setup(repository => repository.RemoveAllByUserIdAsync(displaced.Id, null)).Returns(() => { @@ -99,6 +161,105 @@ public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOw providerClient.VerifyAll(); } + [Fact] + public async Task Handle_TransferDestinationConflict_RestoresPreviousOwnerWithoutRestoringRevokedSessions() + { + // Arrange + const string subject = "transfer-destination-conflict"; + var owner = new User { Id = TestConstants.UserId, FullName = "Owner", EmailAddress = "owner@example.test", Password = "local-password", AuthenticationVersion = "old-owner-session" }; + owner.AddOAuthAccount("github", subject, "original-name", new SettingsDictionary { ["profile"] = "original" }); + var current = new User { Id = TestConstants.UserId2, FullName = "Destination", EmailAddress = "destination@example.test", AuthenticationVersion = "destination-session" }; + var storedOwner = owner; + var context = new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity(IdentityUtils.UserAuthenticationType)) }; + context.Request.SetUser(current); + var conflict = new VersionConflictDocumentException("Destination changed."); + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetUsersByOAuthProviderAsync("github", subject, It.IsAny())).ReturnsAsync(new[] { owner }); + users.Setup(repository => repository.GetByIdAsync(owner.Id, It.IsAny>())).ReturnsAsync(owner); + users.Setup(repository => repository.GetByIdAsync(current.Id, It.IsAny>())).ReturnsAsync(current); + users.Setup(repository => repository.SaveAsync(owner, It.IsAny>())) + .Callback(() => storedOwner = owner with { FullName = "Concurrent profile update", OAuthAccounts = owner.OAuthAccounts.ToHashSet() }).ReturnsAsync(owner); + users.Setup(repository => repository.SaveAsync(current, It.IsAny>())).ThrowsAsync(conflict); + users.Setup(repository => repository.PatchAsync(owner.Id, It.IsAny(), It.IsAny())) + .Callback((Id _, IPatchOperation operation, ICommandOptions _) => + { + foreach (var action in ((ActionPatch)operation).Actions) + action(storedOwner); + }).ReturnsAsync(true); + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.RemoveAllByUserIdAsync(owner.Id, null)).ReturnsAsync(1L); + var oauthTokens = new Mock(MockBehavior.Strict); + oauthTokens.Setup(repository => repository.RemoveAllByUserIdAsync(owner.Id, null)).ReturnsAsync(1L); + var providerClient = new Mock(MockBehavior.Strict); + providerClient.Setup(client => client.GetGitHubUserInfoAsync(It.IsAny(), "test-client", "test-secret")) + .ReturnsAsync(new OAuth2.Models.UserInfo { ProviderName = "github", Id = subject, Email = owner.EmailAddress }); + var options = GetService().AuthOptions; + options.GitHubId = "test-client"; + options.GitHubSecret = "test-secret"; + var handler = CreateHandler(users.Object, tokens.Object, oauthTokens.Object, providerClient.Object); + + // Act + var exception = await Record.ExceptionAsync(async () => + { + _ = await handler.Handle(new GitHubLogin(new ExternalAuthInfo(), context)); + }); + + // Assert + Assert.Same(conflict, exception); + var account = Assert.Single(storedOwner.OAuthAccounts); + Assert.Equal(subject, account.ProviderUserId); + Assert.Equal("original-name", account.Username); + Assert.Equal("original", account.ExtraData["profile"]); + Assert.Equal("Concurrent profile update", storedOwner.FullName); + Assert.NotEqual("old-owner-session", storedOwner.AuthenticationVersion); + users.VerifyAll(); + tokens.VerifyAll(); + oauthTokens.VerifyAll(); + } + + [Fact] + public async Task Handle_UnlinkCanceledAfterPersistence_CompletesTokenIssuance() + { + // Arrange + const string providerId = "unlink-after-persistence"; + string previousVersion = StringExtensions.GetNewToken(); + var user = GetService().GenerateUser(id: TestConstants.UserId, emailAddress: TestConstants.UserEmail) with + { + Password = "local-password", AuthenticationVersion = previousVersion + }; + user.AddOAuthAccount("github", providerId, user.EmailAddress); + using var cancellation = CancellationTokenSource.CreateLinkedTokenSource(TestCancellationToken); + var context = new DefaultHttpContext { RequestAborted = cancellation.Token }; + context.Request.SetUser(user); + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetByIdAsync(user.Id, It.IsAny>())).ReturnsAsync(user); + users.Setup(repository => repository.SaveAsync(user, It.IsAny>())) + .Callback(() => cancellation.Cancel()).ReturnsAsync(user); + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.RemoveAllByUserIdAsync(user.Id, null)).ReturnsAsync(1L); + tokens.Setup(repository => repository.GetByTypeAndUserIdAsync(TokenType.Authentication, user.Id, null)).ReturnsAsync(FindResults.Empty); + Token? issuedToken = null; + tokens.Setup(repository => repository.AddAsync(It.IsAny(), It.IsAny>())) + .Callback((Token token, CommandOptionsDescriptor _) => issuedToken = token) + .ReturnsAsync((Token token, CommandOptionsDescriptor _) => token); + var oauthTokens = new Mock(MockBehavior.Strict); + oauthTokens.Setup(repository => repository.RemoveAllByUserIdAsync(user.Id, null)).ReturnsAsync(1L); + var handler = CreateHandler(users.Object, tokens.Object, oauthTokens.Object, Mock.Of()); + + // Act + var result = await handler.Handle(new RemoveExternalLogin("github", providerId, context)); + + // Assert + Assert.Equal(ResultStatus.Success, result.Status); + Assert.Empty(user.OAuthAccounts); + Assert.NotEqual(previousVersion, user.AuthenticationVersion); + Assert.NotNull(issuedToken); + Assert.Equal(user.AuthenticationVersion, issuedToken.AuthenticationVersion); + users.VerifyAll(); + tokens.VerifyAll(); + oauthTokens.VerifyAll(); + } + [Theory] [MemberData(nameof(LoginRepositoryExceptions))] public async Task Handle_LoginRepositoryException_ReturnsUnauthorizedResult(Exception exception) @@ -128,29 +289,143 @@ public async Task Handle_LoginRepositoryException_ReturnsUnauthorizedResult(Exce new OperationCanceledException("Repository operation was canceled.") }; + [Fact] + public async Task Handle_FirstOAuthSignupCanceledAfterCount_RetryCreatesGlobalAdmin() + { + // Arrange + using var cancellation = CancellationTokenSource.CreateLinkedTokenSource(TestCancellationToken); + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetUsersByOAuthProviderAsync("github", "first-user", It.IsAny())) + .ReturnsAsync(Array.Empty()); + users.Setup(repository => repository.FindOneAsync(It.IsAny>(), It.IsAny>())) + .ReturnsAsync(FindHit.Empty); + int countCalls = 0; + users.Setup(repository => repository.CountAsync(It.IsAny())).Returns(() => + { + if (++countCalls is 1) + cancellation.Cancel(); + + return Task.FromResult(CountResult.Empty); + }); + User? storedUser = null; + users.Setup(repository => repository.AddAsync(It.IsAny(), It.IsAny>())) + .ReturnsAsync((User user, CommandOptionsDescriptor _) => + { + user.Id = TestConstants.UserId; + storedUser = user; + return user; + }); + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.GetByTypeAndUserIdAsync(TokenType.Authentication, TestConstants.UserId, null)) + .ReturnsAsync(FindResults.Empty); + tokens.Setup(repository => repository.AddAsync(It.IsAny(), It.IsAny>())) + .ReturnsAsync((Token token, CommandOptionsDescriptor _) => token); + var oauthTokens = new Mock(MockBehavior.Strict); + var providerClient = new Mock(MockBehavior.Strict); + providerClient.Setup(client => client.GetGitHubUserInfoAsync(It.IsAny(), "test-client", "test-secret")) + .ReturnsAsync(new OAuth2.Models.UserInfo { ProviderName = "github", Id = "first-user", Email = "first@example.test", FirstName = "First", LastName = "User" }); + var options = GetService().AuthOptions; + options.EnableAccountCreation = true; + options.GitHubId = "test-client"; + options.GitHubSecret = "test-secret"; + var handler = CreateHandler(users.Object, tokens.Object, oauthTokens.Object, providerClient.Object); + + // Act + var canceled = await handler.Handle(new GitHubLogin(new ExternalAuthInfo(), new DefaultHttpContext { RequestAborted = cancellation.Token })); + var userAfterCancellation = storedUser; + var retry = await handler.Handle(new GitHubLogin(new ExternalAuthInfo(), new DefaultHttpContext { RequestAborted = TestCancellationToken })); + + // Assert + Assert.Equal(ResultStatus.Unauthorized, canceled.Status); + Assert.Null(userAfterCancellation); + Assert.Equal(ResultStatus.Success, retry.Status); + Assert.NotNull(storedUser); + Assert.Contains(AuthorizationRoles.GlobalAdmin, storedUser.Roles); + users.Verify(repository => repository.CountAsync(It.IsAny()), Times.Exactly(2)); + users.Verify(repository => repository.AddAsync(It.IsAny(), It.IsAny>()), Times.Once); + } + + [Fact] + public async Task Handle_ExternalLoginEmailChangesAfterLookup_ReturnsUnauthorized() + { + // Arrange + const string providerId = "github-email-changed-after-lookup"; + const string previousEmail = "previous@example.test"; + const string currentEmail = "current@example.test"; + var emailUser = new User { Id = TestConstants.UserId, FullName = "Email owner", EmailAddress = previousEmail, Version = "previous-version" }; + emailUser.MarkEmailAddressVerified(); + var storedUser = emailUser with { EmailAddress = currentEmail, Version = "current-version" }; + storedUser.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider); + string? verificationToken = storedUser.VerifyEmailAddressToken; + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetUsersByOAuthProviderAsync("github", providerId, It.IsAny())) + .ReturnsAsync(Array.Empty()); + users.Setup(repository => repository.FindOneAsync(It.IsAny>(), It.IsAny>())) + .ReturnsAsync(new FindHit(emailUser.Id, emailUser, 0, emailUser.Version, null, null)); + users.Setup(repository => repository.GetByIdAsync(emailUser.Id, It.IsAny>())) + .ReturnsAsync(storedUser); + users.Setup(repository => repository.SaveAsync(storedUser, It.IsAny>())) + .ReturnsAsync(storedUser); + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.GetByTypeAndUserIdAsync(TokenType.Authentication, storedUser.Id, null)) + .ReturnsAsync(FindResults.Empty); + tokens.Setup(repository => repository.AddAsync(It.IsAny(), It.IsAny>())) + .ReturnsAsync((Token token, CommandOptionsDescriptor _) => token); + var oauthTokens = new Mock(MockBehavior.Strict); + var providerClient = new Mock(MockBehavior.Strict); + providerClient.Setup(client => client.GetGitHubUserInfoAsync(It.IsAny(), "test-client", "test-secret")) + .ReturnsAsync(new OAuth2.Models.UserInfo { ProviderName = "github", Id = providerId, Email = previousEmail }); + var options = GetService().AuthOptions; + options.GitHubId = "test-client"; + options.GitHubSecret = "test-secret"; + var handler = CreateHandler(users.Object, tokens.Object, oauthTokens.Object, providerClient.Object); + + // Act + var result = await handler.Handle(new GitHubLogin(new ExternalAuthInfo(), new DefaultHttpContext())); + + // Assert + Assert.Equal(ResultStatus.Unauthorized, result.Status); + Assert.Equal(currentEmail, storedUser.EmailAddress); + Assert.False(storedUser.IsEmailAddressVerified); + Assert.Equal(verificationToken, storedUser.VerifyEmailAddressToken); + Assert.Empty(storedUser.OAuthAccounts); + users.Verify(repository => repository.GetByIdAsync(emailUser.Id, It.IsAny>()), Times.Once); + users.Verify(repository => repository.SaveAsync(It.IsAny(), It.IsAny>()), Times.Never); + tokens.VerifyNoOtherCalls(); + oauthTokens.VerifyNoOtherCalls(); + } + [Theory] - [InlineData("facebook")] - [InlineData("windowslive")] - public async Task Handle_RemoveLastUsableExternalLogin_ReturnsBadRequest(string retiredProvider) + [InlineData("facebook", false)] + [InlineData("windowslive", false)] + [InlineData("facebook", true)] + [InlineData("windowslive", true)] + public async Task Handle_RemoveLastUsableExternalLogin_ReturnsBadRequest(string retiredProvider, bool hasDuplicateIdentity) { // Arrange var user = GetService().GenerateUser(id: TestConstants.UserId, emailAddress: TestConstants.UserEmail); user.Password = null; user.AddOAuthAccount("google", "google-user", user.EmailAddress); + if (hasDuplicateIdentity) + user.OAuthAccounts.Add(user.OAuthAccounts.First()); user.AddOAuthAccount(retiredProvider, "retired-user", user.EmailAddress); + string? authenticationVersion = user.AuthenticationVersion; var context = new DefaultHttpContext(); context.Request.SetUser(user); var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetByIdAsync(user.Id, It.IsAny>())).ReturnsAsync(user); var tokens = new Mock(MockBehavior.Strict); var oauthTokens = new Mock(MockBehavior.Strict); var handler = CreateHandler(users.Object, tokens.Object, oauthTokens.Object, GetService()); // Act - var result = await handler.Handle(new RemoveExternalLogin("google", new ValueFromBody("google-user"), context)); + var result = await handler.Handle(new RemoveExternalLogin("google", "google-user", context)); // Assert Assert.Equal(ResultStatus.BadRequest, result.Status); - Assert.Equal(2, user.OAuthAccounts.Count); + Assert.Equal(hasDuplicateIdentity ? 3 : 2, user.OAuthAccounts.Count); + Assert.Equal(authenticationVersion, user.AuthenticationVersion); + users.Verify(repository => repository.GetByIdAsync(user.Id, It.IsAny>()), Times.Once); users.VerifyNoOtherCalls(); tokens.VerifyNoOtherCalls(); oauthTokens.VerifyNoOtherCalls(); @@ -183,7 +458,7 @@ public async Task Handle_MicrosoftProviderFails_DoesNotAccessUserRepository() userRepository.VerifyNoOtherCalls(); } - private AuthHandler CreateHandler(IUserRepository userRepository, ITokenRepository tokenRepository, IOAuthTokenRepository oauthTokenRepository, IOAuthProviderClient oauthProvider) + private AuthHandler CreateHandler(IUserRepository userRepository, ITokenRepository tokenRepository, IOAuthTokenRepository oauthTokenRepository, IOAuthProviderClient oauthProvider, ILockProvider? lockProvider = null) { var appOptions = GetService(); appOptions.AuthOptions.MicrosoftId = "microsoft-client-id"; @@ -198,6 +473,7 @@ private AuthHandler CreateHandler(IUserRepository userRepository, ITokenReposito oauthTokenRepository, oauthProvider, GetService(), + lockProvider ?? GetService(), GetService(), GetService(), GetService(), diff --git a/tests/Exceptionless.Tests/Api/OpenApiSnapshotTests.cs b/tests/Exceptionless.Tests/Api/OpenApiSnapshotTests.cs index 06dd4f3b17..5f04d7b1c6 100644 --- a/tests/Exceptionless.Tests/Api/OpenApiSnapshotTests.cs +++ b/tests/Exceptionless.Tests/Api/OpenApiSnapshotTests.cs @@ -52,10 +52,32 @@ public async Task GetOpenApiJson_Default_ContainsExpectedRoutesOperationsAndResp Assert.True(loginPost.TryGetProperty("requestBody", out _)); AssertResponseCodes(loginPost, "200", "401", "422"); + AssertPathResponseCodes(paths, "/api/v2/auth/unlink/{providerName}", "post", "200", "400", "401", "422"); + Assert.True(paths.TryGetProperty("/api/v2/auth/logout", out var logoutPath)); Assert.True(logoutPath.TryGetProperty("get", out var logoutGet)); AssertResponseCodes(logoutGet, "200", "401", "403"); + foreach (var (path, method) in new[] { + ("/api/v2/organizations", "post"), + ("/api/v2/organizations/{ids}", "delete"), + ("/api/v2/organizations/{id}/users/{email}", "post"), + ("/api/v2/organizations/{id}/users/{email}", "delete"), + ("/api/v2/auth/login", "post"), + ("/api/v2/auth/signup", "post"), + ("/api/v2/users/{id}", "patch"), + ("/api/v2/users/{id}", "put"), + ("/api/v2/users/{id}/avatar", "post"), + ("/api/v2/users/{id}/avatar", "delete"), + ("/api/v2/users/{id}/email-address/{email}", "post"), + ("/api/v2/users/verify-email-address/{token}", "get"), + ("/api/v2/users/{id}/resend-verification-email", "get") + }) + AssertResponseCodes(paths.GetProperty(path).GetProperty(method), "409"); + + var forgotPassword = paths.GetProperty("/api/v2/auth/forgot-password/{email}").GetProperty("get"); + Assert.False(forgotPassword.GetProperty("responses").TryGetProperty("409", out _)); + Assert.True(paths.TryGetProperty("/api/v2/projects", out var projectsPath)); Assert.True(projectsPath.TryGetProperty("post", out var projectsPost)); Assert.True(projectsPost.TryGetProperty("requestBody", out _)); @@ -131,6 +153,9 @@ public async Task GetOpenApiJson_Default_ContainsExpectedSchemasAndSecuritySchem Assert.True(schemas.TryGetProperty("RecordProductTourResult", out var recordProductTourResult)); Assert.Equal("recorded_utc", Assert.Single(recordProductTourResult.GetProperty("properties").EnumerateObject()).Name); Assert.True(schemas.TryGetProperty("ViewOrganization", out _)); + var user = schemas.GetProperty("User"); + Assert.Contains(user.GetProperty("required").EnumerateArray(), property => property.GetString() == "version"); + Assert.Equal("string", user.GetProperty("properties").GetProperty("version").GetProperty("type").GetString()); var savedViewColumnProperties = savedViewColumnSettings.GetProperty("properties"); Assert.Equal("boolean", savedViewColumnProperties.GetProperty("auto_fill").GetProperty("type")[1].GetString()); diff --git a/tests/Exceptionless.Tests/AppHostTopologyTests.cs b/tests/Exceptionless.Tests/AppHostTopologyTests.cs new file mode 100644 index 0000000000..e88ca1f93f --- /dev/null +++ b/tests/Exceptionless.Tests/AppHostTopologyTests.cs @@ -0,0 +1,33 @@ +using Aspire.Hosting; +using Aspire.Hosting.ApplicationModel; +using Aspire.Hosting.JavaScript; +using Aspire.Hosting.Testing; +using Foundatio.Xunit; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Exceptionless.Tests; + +public sealed class AppHostTopologyTests(ITestOutputHelper output) : TestWithLoggingBase(output) +{ + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task BuildAsync_ClientPackageInstallation_RunsOnlyWhenNotAlreadyInstalledByCi(bool ciE2E) + { + // Arrange + var builder = await DistributedApplicationTestingBuilder.CreateAsync( + ciE2E ? ["--ci-e2e"] : [], TestCancellationToken); + + // Act + await using var application = await builder.BuildAsync(TestCancellationToken); + var model = application.Services.GetRequiredService(); + + // Assert + var installer = Assert.Single(model.Resources.OfType(), resource => resource.Name == "App-installer"); + var app = Assert.Single(model.Resources, resource => resource.Name == "App"); + Assert.Equal(ciE2E, installer.Annotations.OfType().Any()); + Assert.Equal(!ciE2E, app.Annotations.OfType().Any(wait => wait.Resource == installer)); + Assert.Contains(app.Annotations.OfType(), wait => wait.Resource.Name == "Api"); + } +} diff --git a/tests/Exceptionless.Tests/Extensions/UserExtensionsTests.cs b/tests/Exceptionless.Tests/Extensions/UserExtensionsTests.cs index 9ab481f500..529bcf7309 100644 --- a/tests/Exceptionless.Tests/Extensions/UserExtensionsTests.cs +++ b/tests/Exceptionless.Tests/Extensions/UserExtensionsTests.cs @@ -8,15 +8,53 @@ namespace Exceptionless.Tests.Extensions; public sealed class UserExtensionsTests(ITestOutputHelper output) : TestWithLoggingBase(output) { [Theory] - [InlineData("facebook")] - [InlineData("Facebook")] - [InlineData("windowslive")] - [InlineData("WindowsLive")] - public void RemoveOAuthAccount_OnlyRetiredProviderWouldRemain_PreservesActiveLogin(string retiredProvider) + [InlineData(false)] + [InlineData(true)] + public void AddOAuthAccount_ExistingIdentity_RefreshesMetadataWithoutAddingAccount(bool includeData) + { + // Arrange + var user = new User(); + user.AddOAuthAccount("github", "subject", "old@example.test", new SettingsDictionary + { + ["name"] = "Old name", ["retained"] = "keep" + }); + user.AddOAuthAccount("google", "subject", "other@example.test"); + var account = user.OAuthAccounts.First(); + user.OAuthAccounts.Add(new OAuthAccount + { + Provider = "github", ProviderUserId = "subject", Username = "stale-duplicate@example.test", + ExtraData = new SettingsDictionary { ["email"] = "stale-duplicate@example.test" } + }); + var data = includeData ? new SettingsDictionary { ["name"] = "New name", ["added"] = "new" } : null; + + // Act + user.AddOAuthAccount("github", "subject", "new@example.test", data); + + // Assert + Assert.Equal(2, user.OAuthAccounts.Count); + Assert.Same(account, user.OAuthAccounts.First()); + Assert.Equal("new@example.test", account.Username); + Assert.Equal(includeData ? "New name" : "Old name", account.ExtraData["name"]); + Assert.Equal("keep", account.ExtraData["retained"]); + Assert.Equal(includeData, account.ExtraData.ContainsKey("added")); + Assert.Equal("other@example.test", Assert.Single(user.OAuthAccounts, account => account.Provider == "google").Username); + Assert.DoesNotContain(user.OAuthAccounts, account => account.EmailAddress() == "stale-duplicate@example.test"); + } + + [Theory] + [InlineData("facebook", false)] + [InlineData("Facebook", false)] + [InlineData("windowslive", false)] + [InlineData("WindowsLive", false)] + [InlineData("facebook", true)] + [InlineData("windowslive", true)] + public void RemoveOAuthAccount_OnlyRetiredProviderWouldRemain_PreservesActiveLogin(string retiredProvider, bool hasDuplicateIdentity) { // Arrange var user = new User(); user.AddOAuthAccount("google", "active-user", "user@example.com"); + if (hasDuplicateIdentity) + user.OAuthAccounts.Add(user.OAuthAccounts.First()); user.OAuthAccounts.Add(new OAuthAccount { Provider = retiredProvider, ProviderUserId = "retired-user", Username = "user@example.com" }); // Act @@ -24,7 +62,7 @@ public void RemoveOAuthAccount_OnlyRetiredProviderWouldRemain_PreservesActiveLog // Assert Assert.False(removed); - Assert.Equal(2, user.OAuthAccounts.Count); + Assert.Equal(hasDuplicateIdentity ? 3 : 2, user.OAuthAccounts.Count); Assert.Contains(user.OAuthAccounts, account => account.ProviderUserId == "active-user"); } diff --git a/tests/Exceptionless.Tests/Jobs/WebHooksJobTests.cs b/tests/Exceptionless.Tests/Jobs/WebHooksJobTests.cs index 85b1f67dba..6e06765eab 100644 --- a/tests/Exceptionless.Tests/Jobs/WebHooksJobTests.cs +++ b/tests/Exceptionless.Tests/Jobs/WebHooksJobTests.cs @@ -200,6 +200,11 @@ public async Task ProcessQueueEntryAsync_GeneralWebHook_DeliversWithSafeTelemetr var requests = new ConcurrentQueue<(string Path, string Cookie, string TraceParent, string TraceState, string Baggage)>(); var activities = new ConcurrentQueue(); var sampledTraceIds = new ConcurrentDictionary(); + // Parallel application hosts keep a parent-based HTTP tracing provider active in this process. + using var applicationTracing = Sdk.CreateTracerProviderBuilder() + .SetSampler(new ParentBasedSampler(new AlwaysOnSampler())) + .AddHttpClientInstrumentation(options => options.EnrichWithHttpRequestMessage = WebHookDelivery.EnrichActivity) + .Build(); using var tracing = Sdk.CreateTracerProviderBuilder() .SetSampler(new TrackingSampler(sampled ? new AlwaysOnSampler() : new AlwaysOffSampler(), sampledTraceIds)) .AddHttpClientInstrumentation(options => options.EnrichWithHttpRequestMessage = WebHookDelivery.EnrichActivity) @@ -238,6 +243,8 @@ public async Task ProcessQueueEntryAsync_GeneralWebHook_DeliversWithSafeTelemetr HttpStatusCode internalStatus; var previousBaggage = Baggage.Current; Baggage.SetBaggage("webhook-test", "private-baggage"); + using var parentActivity = new Activity("webhook-delivery-test").SetIdFormat(ActivityIdFormat.W3C).Start(); + parentActivity.ActivityTraceFlags = sampled ? ActivityTraceFlags.Recorded : ActivityTraceFlags.None; try { // Act diff --git a/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs b/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs new file mode 100644 index 0000000000..1cee7b91a4 --- /dev/null +++ b/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs @@ -0,0 +1,311 @@ +using System.Text.Json; +using System.Text.Json.Nodes; +using Elastic.Clients.Elasticsearch; +using Exceptionless.Core.Extensions; +using Exceptionless.Core.Models; +using Exceptionless.Core.Repositories; +using Exceptionless.Core.Repositories.Configuration; +using Exceptionless.Core.Validation; +using Foundatio.Repositories; +using Foundatio.Repositories.Elasticsearch; +using Foundatio.Repositories.Exceptions; +using Foundatio.Repositories.Models; +using Xunit; + +namespace Exceptionless.Tests.Repositories; + +public sealed class UserRepositoryTests : IntegrationTestsBase +{ + private readonly IUserRepository _repository; + + public UserRepositoryTests(ITestOutputHelper output, AppWebHostFactory factory) : base(output, factory) + { + _repository = GetService(); + } + + [Theory] + [InlineData("id")] + [InlineData("ids")] + [InlineData("email")] + public async Task GetAsync_StoredUserWithoutVersion_ReadsVersionWithoutRewritingDocument(string lookup) + { + // Arrange + const string userId = "770000000000000000000096"; + const string emailAddress = "legacy@localhost.test"; + var configuration = GetService(); + var source = new JsonObject + { + ["id"] = userId, + ["email_address"] = emailAddress, + ["full_name"] = "Existing User" + }; + var indexed = await configuration.Client.IndexAsync(source, + request => request.Index(configuration.Users.VersionedName).Id(userId).Refresh(Refresh.WaitFor), + TestCancellationToken); + string expectedVersion = new ElasticDocumentVersion(indexed.PrimaryTerm.GetValueOrDefault(), indexed.SeqNo.GetValueOrDefault()); + + // Act + var loaded = lookup switch + { + "ids" => (await _repository.GetByIdsAsync(new[] { userId })).Single(), + "email" => await _repository.GetByEmailAddressAsync(emailAddress), + _ => await _repository.GetByIdAsync(userId) + }; + var stored = await configuration.Client.GetAsync( + configuration.Users.VersionedName, userId, TestCancellationToken); + + // Assert + Assert.True(indexed.IsValidResponse); + Assert.NotNull(loaded); + Assert.Equal(expectedVersion, loaded.Version); + Assert.Equal("Existing User", loaded.FullName); + Assert.True(stored.IsValidResponse); + Assert.Equal(indexed.SeqNo, stored.SeqNo); + Assert.False(stored.Source.TryGetProperty("version", out _)); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task PatchAsync_MembershipChanges_RefreshesCachedUserLookups(bool remove) + { + // Arrange + const string organizationId = "770000000000000000000001"; + var user = new User { FullName = "Cached User", EmailAddress = "cached@localhost.test" }; + user.MarkEmailAddressVerified(); + if (remove) + user.OrganizationIds.Add(organizationId); + await _repository.AddAsync(user, options => options.Cache()); + var before = await _repository.GetByIdAsync(user.Id, options => options.Cache()); + var beforeEmail = await _repository.GetByEmailAddressAsync(user.EmailAddress); + + // Act + bool patched = await _repository.PatchAsync(user.Id, new ActionPatch(current => + { + if (remove) + current.OrganizationIds.Remove(organizationId); + else + current.OrganizationIds.Add(organizationId); + }), options => options.Cache()); + var byId = await _repository.GetByIdAsync(user.Id, options => options.Cache()); + var byEmail = await _repository.GetByEmailAddressAsync(user.EmailAddress); + var stored = await _repository.GetByIdAsync(user.Id, options => options.Cache(false)); + + // Assert + Assert.NotNull(before); + Assert.NotNull(beforeEmail); + Assert.Equal(remove, before.OrganizationIds.Contains(organizationId)); + Assert.Equal(remove, beforeEmail.OrganizationIds.Contains(organizationId)); + Assert.True(patched); + Assert.NotNull(stored); + Assert.NotEqual(before.Version, stored.Version); + foreach (var loaded in new[] { byId, byEmail }) + { + Assert.NotNull(loaded); + Assert.Equal(!remove, loaded.OrganizationIds.Contains(organizationId)); + Assert.Equal(stored.Version, loaded.Version); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task UpdateProfileAsync_TourRecordedAfterSnapshot_PreservesProgressAndUnchangedFields(bool updateFullName) + { + // Arrange + var user = new User { FullName = "Existing User", EmailAddress = "profile@localhost.test" }; + user.MarkEmailAddressVerified(); + await _repository.AddAsync(user, options => options.Cache()); + var recordedUtc = TimeProvider.GetUtcNow().UtcDateTime; + await _repository.RecordProductTourAsync(user, "app_overview", recordedUtc); + await _repository.GetByIdAsync(user.Id, options => options.Cache()); + await _repository.GetByEmailAddressAsync(user.EmailAddress); + + // Act + var updated = await _repository.UpdateProfileAsync(user, updateFullName ? "Updated Name" : null, updateFullName ? null : false); + var byId = await _repository.GetByIdAsync(user.Id, options => options.Cache()); + var byEmail = await _repository.GetByEmailAddressAsync(user.EmailAddress); + + // Assert + Assert.Empty(user.ProductTours); + foreach (var result in new[] { updated, byId, byEmail }) + { + Assert.NotNull(result); + Assert.Equal(recordedUtc, result.ProductTours["app_overview"].GetDateTime()); + Assert.Equal(updateFullName ? "Updated Name" : user.FullName, result.FullName); + Assert.Equal(updateFullName && user.EmailNotificationsEnabled, result.EmailNotificationsEnabled); + Assert.Equal(user.EmailAddress, result.EmailAddress); + Assert.Equal(user.CreatedUtc, result.CreatedUtc); + Assert.Equal(user.Roles, result.Roles); + } + } + + [Fact] + public async Task UpdateProfileAsync_UserDeletedAfterSnapshot_DoesNotRecreateUser() + { + // Arrange + var user = new User { FullName = "Deleted User", EmailAddress = "deleted@localhost.test" }; + user.MarkEmailAddressVerified(); + await _repository.AddAsync(user, options => options.Cache()); + await _repository.RemoveAsync(user); + + // Act + var failure = await Record.ExceptionAsync(async () => + { + _ = await _repository.UpdateProfileAsync(user, "Updated Name", null); + }); + var stored = await _repository.GetByIdAsync(user.Id); + + // Assert + Assert.IsType(failure); + Assert.Null(stored); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SaveAsync_VersionChecks_RejectStaleOrMissingVersion(bool missingVersion) + { + // Arrange + var user = new User { FullName = "Versioned User", EmailAddress = "versioned@localhost.test" }; + user.MarkEmailAddressVerified(); + await _repository.AddAsync(user, options => options.ImmediateConsistency()); + var stale = user with { FullName = "Stale profile" }; + user.AuthenticationVersion = "rotated-generation"; + string originalVersion = user.Version; + var saved = await _repository.SaveAsync(user, options => options.ImmediateConsistency()); + if (missingVersion) + stale.Version = null!; + + // Act + var failure = await Record.ExceptionAsync(async () => + { + _ = await _repository.SaveAsync(stale); + }); + var persisted = await _repository.GetByIdAsync(user.Id); + + // Assert + Assert.IsAssignableFrom(failure); + if (!missingVersion) + Assert.IsType(failure); + Assert.Same(user, saved); + Assert.NotEqual(originalVersion, user.Version); + Assert.NotNull(persisted); + Assert.Equal("rotated-generation", persisted.AuthenticationVersion); + Assert.Equal(user.FullName, persisted.FullName); + } + + [Theory] + [InlineData("unknown", "new-subject")] + [InlineData("github", " ")] + public async Task SaveAsync_InvalidOAuthIdentity_RejectsChangesBeforePersistence(string provider, string providerUserId) + { + // Arrange + var user = new User { FullName = "Linked user", EmailAddress = "linked@localhost.test" }; + user.MarkEmailAddressVerified(); + user.AddOAuthAccount("github", "existing-subject", user.EmailAddress); + await _repository.AddAsync(user, options => options.ImmediateConsistency()); + user.AddOAuthAccount(provider, providerUserId, user.EmailAddress); + + // Act + var exception = await Record.ExceptionAsync(async () => + { + _ = await _repository.SaveAsync(user, options => options.ImmediateConsistency()); + }); + + // Assert + Assert.IsType(exception); + var storedUser = await _repository.GetByIdAsync(user.Id, options => options.Cache(false)); + Assert.NotNull(storedUser); + var account = Assert.Single(storedUser.OAuthAccounts); + Assert.Equal("github", account.Provider); + Assert.Equal("existing-subject", account.ProviderUserId); + } + + [Fact] + public async Task GetUsersByOAuthProviderAsync_DifferentAccountEntries_RequiresExactIdentityPair() + { + // Arrange + var user = new User { FullName = "Linked User", EmailAddress = "linked@localhost.test" }; + user.MarkEmailAddressVerified(); + user.AddOAuthAccount("google", "shared-subject", user.EmailAddress); + user.AddOAuthAccount("google", "shared-subject", user.EmailAddress); + user.AddOAuthAccount("github", "other-subject", user.EmailAddress); + await _repository.AddAsync(user, options => options.ImmediateConsistency()); + + // Act + var githubUser = await _repository.GetUsersByOAuthProviderAsync("github", "shared-subject", TestCancellationToken); + var googleUser = await _repository.GetUsersByOAuthProviderAsync("google", "shared-subject", TestCancellationToken); + var differentProviderCaseUser = await _repository.GetUsersByOAuthProviderAsync("GOOGLE", "shared-subject", TestCancellationToken); + var differentCaseUser = await _repository.GetUsersByOAuthProviderAsync("google", "SHARED-SUBJECT", TestCancellationToken); + + // Assert + Assert.Empty(githubUser); + var owner = Assert.Single(googleUser); + Assert.Equal(user.Id, owner.Id); + Assert.Equal(2, owner.OAuthAccounts.Count); + Assert.Empty(differentCaseUser); + Assert.Empty(differentProviderCaseUser); + } + + [Theory] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task GetUsersByOAuthProviderAsync_PagedCandidates_ValidatesIdentityAndCancellation(bool hasDuplicateOwner, bool cancelAfterFirstPage) + { + // Arrange + var users = Enumerable.Range(1, 101).Select(index => + { + var user = new User + { + Id = index.ToString("x24"), + FullName = $"Linked User {index}", + EmailAddress = $"linked-{index}@localhost.test" + }; + user.MarkEmailAddressVerified(); + user.AddOAuthAccount("google", "shared-subject", user.EmailAddress); + user.AddOAuthAccount("github", "other-subject", user.EmailAddress); + return user; + }).ToArray(); + users[^1].AddOAuthAccount("github", "shared-subject", users[^1].EmailAddress); + if (hasDuplicateOwner) + users[0].AddOAuthAccount("github", "shared-subject", users[0].EmailAddress); + await _repository.AddAsync(users, options => options.ImmediateConsistency()); + IReadOnlyCollection? matchingUsers = null; + using var cancellation = CancellationTokenSource.CreateLinkedTokenSource(TestCancellationToken); + int queryCount = 0; + using var queryHandler = _repository.AfterQuery.AddHandler((_, args) => + { + if (args.Result is FindResults) + { + queryCount++; + if (cancelAfterFirstPage) + cancellation.Cancel(); + } + + return Task.CompletedTask; + }); + + // Act + var exception = await Record.ExceptionAsync(async () => + { + matchingUsers = await _repository.GetUsersByOAuthProviderAsync("github", "shared-subject", cancellation.Token); + }); + + // Assert + if (cancelAfterFirstPage) + { + Assert.IsAssignableFrom(exception); + Assert.Null(matchingUsers); + Assert.Equal(1, queryCount); + } + else + { + Assert.Null(exception); + Assert.NotNull(matchingUsers); + Assert.Equal(hasDuplicateOwner ? 2 : 1, matchingUsers.Count); + Assert.Contains(matchingUsers, user => user.Id == users[^1].Id); + } + } +} diff --git a/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs b/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs index 37d82962e2..851da28111 100644 --- a/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs +++ b/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs @@ -1,5 +1,7 @@ using System.Text.Json; +using Exceptionless.Core.Extensions; using Exceptionless.Core.Models; +using Foundatio.Repositories.Elasticsearch; using Foundatio.Serializer; using Xunit; @@ -231,6 +233,7 @@ public void Deserialize_SnakeCaseJson_PreservesOrganizationIds() Assert.NotNull(user); Assert.Equal("user-from-cache", user.Id); Assert.Null(user.AuthenticationVersion); + Assert.Empty(user.Version); Assert.Equal(2, user.OrganizationIds.Count); Assert.Contains("cached-org-1", user.OrganizationIds); Assert.Contains("cached-org-2", user.OrganizationIds); @@ -260,11 +263,14 @@ public void Serialize_UserWithProductTourState_PreservesUiDefinedKeys() // Act string? json = _serializer.SerializeToString(original); + using var document = JsonDocument.Parse(json!); + string productToursJson = document.RootElement.GetProperty("product_tours").GetRawText(); + // Assert Assert.Contains("\"product_tours\":{\"app_overview\":\"2024-01-15T12:00:00Z\",\"future_guide_v2\":\"2024-01-15T12:01:00Z\"}", json); - Assert.DoesNotContain("status", json, StringComparison.OrdinalIgnoreCase); - Assert.DoesNotContain("version", json, StringComparison.OrdinalIgnoreCase); - Assert.DoesNotContain("event_investigate", json, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("status", productToursJson, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("version", productToursJson, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("event_investigate", productToursJson, StringComparison.OrdinalIgnoreCase); } [Fact] @@ -420,11 +426,13 @@ public void Deserialize_UserWithEmptyCollections_ReturnsEmptyCollections() public void Deserialize_UserWithAllProperties_PreservesAllValues() { // Arrange - const string authenticationVersion = "current-version"; + string authenticationVersion = StringExtensions.GetNewToken(); + string documentVersion = new ElasticDocumentVersion(primaryTerm: 1, sequenceNumber: 42); var original = new User { Id = "full-user", AuthenticationVersion = authenticationVersion, + Version = documentVersion, FullName = "Full Test User", EmailAddress = "full@example.com", Password = "hashedpassword", @@ -441,13 +449,14 @@ public void Deserialize_UserWithAllProperties_PreservesAllValues() original.Roles.Add("user"); // Act - string? json = _serializer.SerializeToString(original); + string json = _serializer.SerializeToString(original)!; var deserialized = _serializer.Deserialize(json); // Assert Assert.NotNull(deserialized); Assert.Equal("full-user", deserialized.Id); Assert.Equal(authenticationVersion, deserialized.AuthenticationVersion); + Assert.Equal(documentVersion, deserialized.Version); Assert.Equal("Full Test User", deserialized.FullName); Assert.Equal("full@example.com", deserialized.EmailAddress); Assert.Equal("hashedpassword", deserialized.Password); diff --git a/tests/Exceptionless.Tests/Utility/TestOAuthProviderClient.cs b/tests/Exceptionless.Tests/Utility/TestOAuthProviderClient.cs index 235953a8bb..c13e1156f4 100644 --- a/tests/Exceptionless.Tests/Utility/TestOAuthProviderClient.cs +++ b/tests/Exceptionless.Tests/Utility/TestOAuthProviderClient.cs @@ -9,17 +9,17 @@ public sealed class TestOAuthProviderClient : IOAuthProviderClient { public Task GetGitHubUserInfoAsync(ExternalAuthInfo authInfo, string appId, string appSecret) { - return GetUserInfoAsync("GitHub", authInfo); + return GetUserInfoAsync(OAuthAccount.KnownProviders.GitHub, authInfo); } public Task GetGoogleUserInfoAsync(ExternalAuthInfo authInfo, string appId, string appSecret) { - return GetUserInfoAsync("Google", authInfo); + return GetUserInfoAsync(OAuthAccount.KnownProviders.Google, authInfo); } public Task GetMicrosoftUserInfoAsync(ExternalAuthInfo authInfo, string appId, string appSecret) { - return GetUserInfoAsync("Microsoft", authInfo); + return GetUserInfoAsync(OAuthAccount.KnownProviders.Microsoft, authInfo); } public Task GetSlackAccessTokenAsync(string code) diff --git a/tests/Exceptionless.Tests/Validation/UserValidatorTests.cs b/tests/Exceptionless.Tests/Validation/UserValidatorTests.cs index 6f9a5c969a..ea5ca2ad4c 100644 --- a/tests/Exceptionless.Tests/Validation/UserValidatorTests.cs +++ b/tests/Exceptionless.Tests/Validation/UserValidatorTests.cs @@ -13,6 +13,39 @@ public UserValidatorTests(ITestOutputHelper output) : base(output) _validator = GetService(); } + [Theory] + [InlineData("github", "subject", true)] + [InlineData("google", "subject", true)] + [InlineData("microsoft", "subject", true)] + [InlineData("facebook", "subject", true)] + [InlineData("windowslive", "subject", true)] + [InlineData("unknown", "subject", false)] + [InlineData("GitHub", "subject", false)] + [InlineData("", "subject", false)] + [InlineData(null, "subject", false)] + [InlineData("github", null, false)] + [InlineData("github", "", false)] + [InlineData("github", " ", false)] + public async Task ValidateAsync_OAuthAccount_RequiresValidIdentity(string? providerName, string? providerUserId, bool expectedIsValid) + { + // Arrange + var user = new User + { + FullName = "Linked user", + EmailAddress = "linked@localhost.test", + IsEmailAddressVerified = true, + OAuthAccounts = [new OAuthAccount { Provider = providerName!, ProviderUserId = providerUserId!, Username = "linked@localhost.test" }] + }; + + // Act + var result = await _validator.ValidateAsync(user); + + // Assert + Assert.Equal(expectedIsValid, result.IsValid); + if (!expectedIsValid) + Assert.Contains(result.Errors.Keys, key => key.EndsWith(String.IsNullOrWhiteSpace(providerUserId) ? nameof(OAuthAccount.ProviderUserId) : nameof(OAuthAccount.Provider), StringComparison.Ordinal)); + } + [Fact] public async Task ValidateAsync_WhenFullNameIsValid_ReturnsSuccess() {