diff --git a/SecurityService.BusinessLogic/RequestHandlers/UserRequestHandler.cs b/SecurityService.BusinessLogic/RequestHandlers/UserRequestHandler.cs index 943c9351..30cbeea2 100644 --- a/SecurityService.BusinessLogic/RequestHandlers/UserRequestHandler.cs +++ b/SecurityService.BusinessLogic/RequestHandlers/UserRequestHandler.cs @@ -2,6 +2,7 @@ using MessagingService.Client; using MessagingService.DataTransferObjects; using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -33,7 +34,8 @@ public sealed class UserRequestHandler : IRequestHandler, IRequestHandler, IRequestHandler>, - IRequestHandler + IRequestHandler, + IRequestHandler { private readonly UserManager UserManager; private readonly IPasswordHasher PasswordHasher; @@ -121,19 +123,18 @@ private SendEmailRequest BuildEmailConfirmationRequest(ApplicationUser user, return request; } - private SendEmailRequest BuildWelcomeEmail(string emailAddress, - string password) + private SendEmailRequest BuildActivationEmail(string emailAddress, + string activationUri) { StringBuilder mesasgeBuilder = new StringBuilder(); mesasgeBuilder.AppendLine(""); mesasgeBuilder.AppendLine("

Welcome to Transaction Processing System

"); mesasgeBuilder.AppendLine("

"); - mesasgeBuilder.AppendLine("

Please find below your user details:

"); + mesasgeBuilder.AppendLine("

Please activate your account by choosing a password:

"); mesasgeBuilder.AppendLine(""); mesasgeBuilder.AppendLine(""); mesasgeBuilder.AppendLine($""); - mesasgeBuilder.AppendLine(""); - mesasgeBuilder.AppendLine($""); + mesasgeBuilder.AppendLine($""); mesasgeBuilder.AppendLine("
User Name
{emailAddress}
Password
{password}
Activate account
"); mesasgeBuilder.AppendLine(""); @@ -178,16 +179,18 @@ public async Task Handle(SecurityServiceCommands.CreateUserCommand comma RegistrationDateTime = DateTime.UtcNow, }; - Result passwordValueResult = String.IsNullOrEmpty(command.Password) ? PasswordGenerator.GenerateRandomPassword(this.Options.Value.PasswordOptions) : command.Password; + bool hasEmailAddress = String.IsNullOrWhiteSpace(command.EmailAddress) == false; + if (hasEmailAddress == false) + { + Result passwordValueResult = String.IsNullOrEmpty(command.Password) ? PasswordGenerator.GenerateRandomPassword(this.Options.Value.PasswordOptions) : command.Password; - if (passwordValueResult.IsFailed) - return ResultHelpers.CreateFailure(passwordValueResult); + if (passwordValueResult.IsFailed) + return ResultHelpers.CreateFailure(passwordValueResult); - // Hash the default password - newIdentityUser.PasswordHash = this.PasswordHasher.HashPassword(newIdentityUser, passwordValueResult.Data); + newIdentityUser.PasswordHash = this.PasswordHasher.HashPassword(newIdentityUser, passwordValueResult.Data); - if (String.IsNullOrEmpty(newIdentityUser.PasswordHash)) { - return Result.Failure("Error generating password hash value, hash was null or empty"); + if (String.IsNullOrEmpty(newIdentityUser.PasswordHash)) + return Result.Failure("Error generating password hash value, hash was null or empty"); } // Create the User @@ -197,7 +200,7 @@ public async Task Handle(SecurityServiceCommands.CreateUserCommand comma Result result = (createResult.IsSuccess, addRolesToUserResult.IsSuccess, addClaimsToUserResult.IsSuccess) switch { - (true, true, true) => await SendConfirmationEmail(newIdentityUser, cancellationToken), + (true, true, true) => hasEmailAddress ? await SendConfirmationEmail(newIdentityUser, cancellationToken) : Result.Success(), _ => await this.DeleteUser(createResult, addRolesToUserResult, addClaimsToUserResult, newIdentityUser) }; return result; } @@ -376,34 +379,7 @@ public async Task Handle(SecurityServiceCommands.SendWelcomeEmailCommand return Result.NotFound($"No user found with username {command.Username}"); } - IdentityResult removePasswordResult = await this.UserManager.RemovePasswordAsync(user); - if (removePasswordResult.Succeeded == false) - { - return Result.Failure($"Errors removing password for user [{command.Username}]"); - } - - Result generatedPasswordResult = PasswordGenerator.GenerateRandomPassword(this.Options.Value.PasswordOptions); - if (generatedPasswordResult.IsFailed) - { - return ResultHelpers.CreateFailure(generatedPasswordResult); - } - - IdentityResult addPasswordResult = await this.UserManager.AddPasswordAsync(user, generatedPasswordResult.Data); - if (addPasswordResult.Succeeded == false) - { - return Result.Failure($"Errors adding password for user [{command.Username}]"); - } - - TokenResponse token = await this.GetToken(); - string emailAddress = user.Email ?? user.UserName ?? string.Empty; - SendEmailRequest emailRequest = this.BuildWelcomeEmail(emailAddress, generatedPasswordResult.Data); - Result sendEmailResult = await this.MessagingServiceClient.SendEmail(token.AccessToken, emailRequest, cancellationToken); - if (sendEmailResult.IsFailed) - { - return ResultHelpers.CreateFailure(sendEmailResult); - } - - return Result.Success(); + return await this.SendActivationEmail(user, cancellationToken); } public async Task Handle(SecurityServiceCommands.ResendWelcomeEmailCommand command, @@ -415,32 +391,43 @@ public async Task Handle(SecurityServiceCommands.ResendWelcomeEmailComma return Result.NotFound($"No user found with username {command.Username}"); } - IdentityResult removePasswordResult = await this.UserManager.RemovePasswordAsync(user); - if (removePasswordResult.Succeeded == false) - { - return Result.Failure($"Errors removing password for user [{command.Username}]"); - } + if (user.EmailConfirmed == false) + return await this.SendConfirmationEmail(user, cancellationToken); - Result generatedPasswordResult = PasswordGenerator.GenerateRandomPassword(this.Options.Value.PasswordOptions); - if (generatedPasswordResult.IsFailed) - { - return ResultHelpers.CreateFailure(generatedPasswordResult); - } + return await this.SendActivationEmail(user, cancellationToken); + } - IdentityResult addPasswordResult = await this.UserManager.AddPasswordAsync(user, generatedPasswordResult.Data); - if (addPasswordResult.Succeeded == false) - { - return Result.Failure($"Errors adding password for user [{command.Username}]"); - } + private async Task SendActivationEmail(ApplicationUser user, CancellationToken cancellationToken) + { + String activationToken = await this.UserManager.GeneratePasswordResetTokenAsync(user); + String emailAddress = user.Email ?? user.UserName ?? string.Empty; + String uri = QueryHelpers.AddQueryString( + $"{this.Options.Value.PublicOrigin}/Account/ActivateAccount", + new Dictionary + { + ["userName"] = user.UserName, + ["activationToken"] = activationToken + }); TokenResponse token = await this.GetToken(); - string emailAddress = user.Email ?? user.UserName ?? string.Empty; - SendEmailRequest emailRequest = this.BuildWelcomeEmail(emailAddress, generatedPasswordResult.Data); + SendEmailRequest emailRequest = this.BuildActivationEmail(emailAddress, uri); Result sendEmailResult = await this.MessagingServiceClient.SendEmail(token.AccessToken, emailRequest, cancellationToken); if (sendEmailResult.IsFailed) - { return ResultHelpers.CreateFailure(sendEmailResult); - } + + return Result.Success(); + } + + public async Task Handle(SecurityServiceCommands.ProcessAccountActivationCommand command, + CancellationToken cancellationToken) + { + ApplicationUser? user = await this.UserManager.FindByNameAsync(command.Username); + if (user == null) + return Result.NotFound(); + + IdentityResult result = await this.UserManager.ResetPasswordAsync(user, command.Token, command.Password); + if (result.Succeeded == false) + return Result.Failure("The account activation link is invalid or has expired."); return Result.Success(); } @@ -565,8 +552,14 @@ public async Task Handle(SecurityServiceCommands.ProcessPasswordResetReq // User has been found so send an email with reset details String resetToken = await this.UserManager.GeneratePasswordResetTokenAsync(user); - resetToken = UrlEncoder.Default.Encode(resetToken); - String uri = $"{this.Options.Value.PublicOrigin}/Account/ResetPassword?userName={user.UserName}&resetToken={resetToken}&clientId={command.ClientId}"; + String uri = QueryHelpers.AddQueryString( + $"{this.Options.Value.PublicOrigin}/Account/ResetPassword", + new Dictionary + { + ["userName"] = user.UserName, + ["resetToken"] = resetToken, + ["clientId"] = command.ClientId + }); TokenResponse token = await this.GetToken(); SendEmailRequest emailRequest = this.BuildPasswordResetEmailRequest(user, uri); diff --git a/SecurityService.BusinessLogic/Requests/SecurityServiceCommands.cs b/SecurityService.BusinessLogic/Requests/SecurityServiceCommands.cs index edf0b21a..09d322cf 100644 --- a/SecurityService.BusinessLogic/Requests/SecurityServiceCommands.cs +++ b/SecurityService.BusinessLogic/Requests/SecurityServiceCommands.cs @@ -57,6 +57,9 @@ public record ConfirmUserEmailAddressCommand(String UserName, String ConfirmEmai public record SendWelcomeEmailCommand(String Username) : IRequest; public record ResendWelcomeEmailCommand(String Username) : IRequest; + public record ProcessAccountActivationCommand(String Username, + String Token, + String Password) : IRequest; public record ProcessPasswordResetRequestCommand(String Username, String EmailAddress, String ClientId) : IRequest; diff --git a/SecurityService.IntegrationTesting.Helpers/ReqnrollExtensions.cs b/SecurityService.IntegrationTesting.Helpers/ReqnrollExtensions.cs index 5e30f06c..8e26eda7 100644 --- a/SecurityService.IntegrationTesting.Helpers/ReqnrollExtensions.cs +++ b/SecurityService.IntegrationTesting.Helpers/ReqnrollExtensions.cs @@ -259,6 +259,7 @@ public static List ToCreateUserRequests(this DataTableRows ta String roles = ReqnrollTableHelper.GetStringRowValue(tableRow, "Roles"); CreateUserRequest createUserRequest = new CreateUserRequest{ + UserName = ReqnrollTableHelper.GetStringRowValue(tableRow, "User Name"), EmailAddress = ReqnrollTableHelper.GetStringRowValue(tableRow, "Email Address"), FamilyName = ReqnrollTableHelper.GetStringRowValue(tableRow, "Family Name"), GivenName = ReqnrollTableHelper.GetStringRowValue(tableRow, "Given Name"), @@ -322,4 +323,4 @@ private static List SplitString(this String stringToSplit){ return result; } } -} \ No newline at end of file +} diff --git a/SecurityService.IntegrationTesting.Helpers/SecurityServiceSteps.cs b/SecurityService.IntegrationTesting.Helpers/SecurityServiceSteps.cs index 774b8e45..76da2ac5 100644 --- a/SecurityService.IntegrationTesting.Helpers/SecurityServiceSteps.cs +++ b/SecurityService.IntegrationTesting.Helpers/SecurityServiceSteps.cs @@ -288,7 +288,10 @@ public async Task WhenIGetTheRolesRolesDetailsAreReturnedAsFollows(List>? user = await this.SecurityServiceClient.GetUsers(createUserRequest.EmailAddress, cancellationToken); user.IsSuccess.ShouldBeTrue(); - results.Add((createUserRequest.EmailAddress, user.Data.Single().UserId)); + String userKey = String.IsNullOrWhiteSpace(createUserRequest.UserName) + ? createUserRequest.EmailAddress ?? String.Empty + : createUserRequest.UserName; + results.Add((userKey, user.Data.Single().UserId)); } return results; } @@ -341,4 +344,4 @@ public async Task WhenIGetTheUserWithUserNameTheUserDetailsAreReturnedAsFollows( //userDetails.Data.RegistrationDateTime.Date.ShouldBe(expectedRecord.RegistrationDateTime.Date); } -} \ No newline at end of file +} diff --git a/SecurityService.IntegrationTests/Token/Token.feature b/SecurityService.IntegrationTests/Token/Token.feature index c5446ba3..d571a866 100644 --- a/SecurityService.IntegrationTests/Token/Token.feature +++ b/SecurityService.IntegrationTests/Token/Token.feature @@ -26,8 +26,8 @@ Background: | merchantClient | Merchant Client | Secret1 | transactionProcessorAcl | password | Given I create the following users - | Email Address | Password | Phone Number | Given Name | Middle Name | Family Name | Claims | Roles | - | merchantuser@testmerchant1.co.uk | 123456 | 123456789 | Test | | User 1 | EstateId:1,MerchantId:2 | Merchant | + | User Name | Email Address | Password | Phone Number | Given Name | Middle Name | Family Name | Claims | Roles | + | merchantuser | | 123456 | 123456789 | Test | | User 1 | EstateId:1,MerchantId:2 | Merchant | @PRTest Scenario: Get Tokens @@ -37,7 +37,7 @@ Scenario: Get Tokens Then my token is returned When I request a password token with the following values | ClientId | ClientSecret | Username | Password | - | merchantClient | Secret1 | merchantuser@testmerchant1.co.uk | 123456 | + | merchantClient | Secret1 | merchantuser | 123456 | Then my token is returned diff --git a/SecurityService.IntegrationTests/Token/Token.feature.cs b/SecurityService.IntegrationTests/Token/Token.feature.cs index 319c470b..78ab1a15 100644 --- a/SecurityService.IntegrationTests/Token/Token.feature.cs +++ b/SecurityService.IntegrationTests/Token/Token.feature.cs @@ -121,83 +121,84 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa { #line 4 #line hidden - global::Reqnroll.Table table22 = new global::Reqnroll.Table(new string[] { + global::Reqnroll.Table table1 = new global::Reqnroll.Table(new string[] { "Role Name"}); - table22.AddRow(new string[] { + table1.AddRow(new string[] { "Estate"}); - table22.AddRow(new string[] { + table1.AddRow(new string[] { "Merchant"}); #line 6 - await testRunner.GivenAsync("I create the following roles", ((string)(null)), table22, "Given "); + await testRunner.GivenAsync("I create the following roles", ((string)(null)), table1, "Given "); #line hidden - global::Reqnroll.Table table23 = new global::Reqnroll.Table(new string[] { + global::Reqnroll.Table table2 = new global::Reqnroll.Table(new string[] { "Name", "DisplayName", "Description"}); - table23.AddRow(new string[] { + table2.AddRow(new string[] { "estateManagement", "estateManagement Scope", "A scope for estateManagement"}); - table23.AddRow(new string[] { + table2.AddRow(new string[] { "transactionProcessor", "transactionProcessor Scope", "A scope for transactionProcessor"}); - table23.AddRow(new string[] { + table2.AddRow(new string[] { "transactionProcessorAcl", "transactionProcessorAcl Scope", "A scope for transactionProcessorAcl"}); #line 11 - await testRunner.GivenAsync("I create the following api scopes", ((string)(null)), table23, "Given "); + await testRunner.GivenAsync("I create the following api scopes", ((string)(null)), table2, "Given "); #line hidden - global::Reqnroll.Table table24 = new global::Reqnroll.Table(new string[] { + global::Reqnroll.Table table3 = new global::Reqnroll.Table(new string[] { "Name", "DisplayName", "Secret", "Scopes", "UserClaims"}); - table24.AddRow(new string[] { + table3.AddRow(new string[] { "estateManagement", "Estate Managememt REST", "Secret1", "estateManagement", "MerchantId, EstateId, role"}); - table24.AddRow(new string[] { + table3.AddRow(new string[] { "transactionProcessor", "Transaction Processor REST", "Secret1", "transactionProcessor", ""}); - table24.AddRow(new string[] { + table3.AddRow(new string[] { "transactionProcessorAcl", "Transaction Processor ACL REST", "Secret1", "transactionProcessorAcl", "MerchantId, EstateId, role"}); #line 17 - await testRunner.GivenAsync("I create the following api resources", ((string)(null)), table24, "Given "); + await testRunner.GivenAsync("I create the following api resources", ((string)(null)), table3, "Given "); #line hidden - global::Reqnroll.Table table25 = new global::Reqnroll.Table(new string[] { + global::Reqnroll.Table table4 = new global::Reqnroll.Table(new string[] { "ClientId", "Name", "Secret", "Scopes", "GrantTypes"}); - table25.AddRow(new string[] { + table4.AddRow(new string[] { "serviceClient", "Service Client", "Secret1", "estateManagement,transactionProcessor,transactionProcessorAcl", "client_credentials"}); - table25.AddRow(new string[] { + table4.AddRow(new string[] { "merchantClient", "Merchant Client", "Secret1", "transactionProcessorAcl", "password"}); #line 23 - await testRunner.GivenAsync("I create the following clients", ((string)(null)), table25, "Given "); + await testRunner.GivenAsync("I create the following clients", ((string)(null)), table4, "Given "); #line hidden - global::Reqnroll.Table table26 = new global::Reqnroll.Table(new string[] { + global::Reqnroll.Table table5 = new global::Reqnroll.Table(new string[] { + "User Name", "Email Address", "Password", "Phone Number", @@ -206,8 +207,9 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "Family Name", "Claims", "Roles"}); - table26.AddRow(new string[] { - "merchantuser@testmerchant1.co.uk", + table5.AddRow(new string[] { + "merchantuser", + "", "123456", "123456789", "Test", @@ -216,7 +218,7 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "EstateId:1,MerchantId:2", "Merchant"}); #line 28 - await testRunner.GivenAsync("I create the following users", ((string)(null)), table26, "Given "); + await testRunner.GivenAsync("I create the following users", ((string)(null)), table5, "Given "); #line hidden } @@ -250,30 +252,30 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa #line 4 await this.FeatureBackgroundAsync(); #line hidden - global::Reqnroll.Table table27 = new global::Reqnroll.Table(new string[] { + global::Reqnroll.Table table6 = new global::Reqnroll.Table(new string[] { "ClientId", "ClientSecret"}); - table27.AddRow(new string[] { + table6.AddRow(new string[] { "serviceClient", "Secret1"}); #line 34 - await testRunner.WhenAsync("I request a client token with the following values", ((string)(null)), table27, "When "); + await testRunner.WhenAsync("I request a client token with the following values", ((string)(null)), table6, "When "); #line hidden #line 37 await testRunner.ThenAsync("my token is returned", ((string)(null)), ((global::Reqnroll.Table)(null)), "Then "); #line hidden - global::Reqnroll.Table table28 = new global::Reqnroll.Table(new string[] { + global::Reqnroll.Table table7 = new global::Reqnroll.Table(new string[] { "ClientId", "ClientSecret", "Username", "Password"}); - table28.AddRow(new string[] { + table7.AddRow(new string[] { "merchantClient", "Secret1", - "merchantuser@testmerchant1.co.uk", + "merchantuser", "123456"}); #line 38 - await testRunner.WhenAsync("I request a password token with the following values", ((string)(null)), table28, "When "); + await testRunner.WhenAsync("I request a password token with the following values", ((string)(null)), table7, "When "); #line hidden #line 41 await testRunner.ThenAsync("my token is returned", ((string)(null)), ((global::Reqnroll.Table)(null)), "Then "); diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs index 861b274e..e7b91aa3 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs @@ -149,15 +149,32 @@ public async Task ThenIGetAWelcomeEmailWithMyLoginDetails() var emailNode = doc.DocumentNode.SelectNodes("//*[@id='username']"); - var passwordNode = doc.DocumentNode.SelectNodes("//*[@id='password']"); + var activationNode = doc.DocumentNode.SelectNodes("//a[@href]"); emailNode.ShouldHaveSingleItem(); emailNode.Single().InnerText.ShouldNotBeNullOrEmpty(); - passwordNode.ShouldHaveSingleItem(); - passwordNode.Single().InnerText.ShouldNotBeNullOrEmpty(); + activationNode.ShouldHaveSingleItem(); this.TestingContext.EmailAddress = emailNode.Single().InnerText.Trim(); - this.TestingContext.Password = passwordNode.Single().InnerText.Trim(); + this.TestingContext.Password = "Pa55word!"; + + await BrowserNavigation.NavigateWithDiagnosticsAsync( + () => + { + this.WebDriver.Navigate().GoToUrl(activationNode.Single().GetAttributeValue("href", string.Empty)); + return Task.CompletedTask; + }, + message => this.TestingContext.DockerHelper.Logger.LogInformation(message)); + + this.WebDriver.FillIn("Input.Password", this.TestingContext.Password); + this.WebDriver.FillIn("Input.ConfirmPassword", this.TestingContext.Password); + await this.WebDriver.ClickButton("Activate account"); + + await Retry.For(async () => + { + IWebElement activationMessage = this.WebDriver.FindElement(By.Id("activationMessage")); + activationMessage.Text.ShouldBe("Your account has been activated. You can now log in."); + }); } @@ -178,7 +195,7 @@ public async Task ThenIAmPresentedWithTheConfirmEmailAddressSuccessfulScreen() { await Retry.For(async () => { IWebElement webElement = this.WebDriver.FindElement(By.Id("userMessage")); - webElement.Text.ShouldBe("Thanks for confirming your email address, you should receive a welcome email soon."); + webElement.Text.ShouldBe("Thanks for confirming your email address, you should receive an account activation email soon."); }); } diff --git a/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs b/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs index 2f78b176..59004625 100644 --- a/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs +++ b/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs @@ -1,4 +1,5 @@ using System.Security.Cryptography; +using Microsoft.AspNetCore.DataProtection; using MessagingService.Client; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; @@ -12,7 +13,7 @@ namespace SecurityService.UnitTests.Infrastructure; public static class TestServiceProviderFactory { - public static ServiceProvider Create(string databaseName, UserManager? userManager = null, SignInManager? signInManager = null, Action? configureOptions = null) + public static ServiceProvider Create(string databaseName, UserManager? userManager = null, SignInManager? signInManager = null, Action? configureOptions = null, TimeSpan? passwordTokenLifespan = null) { var rsa = RSA.Create(2048); var key = new RsaSecurityKey(rsa); @@ -30,6 +31,9 @@ public static ServiceProvider Create(string databaseName, UserManager(); services.AddLogging(); + services.AddDataProtection().UseEphemeralDataProtectionProvider(); + if (passwordTokenLifespan.HasValue) + services.Configure(options => options.TokenLifespan = passwordTokenLifespan.Value); services.AddDbContext(options => { options.UseInMemoryDatabase(databaseName); diff --git a/SecurityService.UnitTests/Pages/ConfirmEmailPageModelTests.cs b/SecurityService.UnitTests/Pages/ConfirmEmailPageModelTests.cs index 4ee9862f..a2199e0c 100644 --- a/SecurityService.UnitTests/Pages/ConfirmEmailPageModelTests.cs +++ b/SecurityService.UnitTests/Pages/ConfirmEmailPageModelTests.cs @@ -27,10 +27,8 @@ public async Task OnGet_WithQueryValues_PopulatesInput() .ReturnsAsync(user); userManager.ConfirmEmailAsync(user, "token-123") .ReturnsAsync(IdentityResult.Success); - userManager.RemovePasswordAsync(user) - .ReturnsAsync(IdentityResult.Success); - userManager.AddPasswordAsync(user, Arg.Any()) - .ReturnsAsync(IdentityResult.Success); + userManager.GeneratePasswordResetTokenAsync(user) + .ReturnsAsync("activation-token"); var signInManager = IdentityMocks.CreateSignInManager(userManager); using var provider = CreateProvider(nameof(this.OnGet_WithQueryValues_PopulatesInput), userManager, signInManager); @@ -90,8 +88,7 @@ public async Task OnGet_WhenConfirmationFails_ReturnsPageWithFailureMessage() model.Input.Username.ShouldBe("alice"); model.Input.Token.ShouldBe("token-123"); userManager.ConfirmEmailAsync(user, "token-123").Called(Count.Once()); - userManager.RemovePasswordAsync(Arg.Any()).Called(Count.Never()); - userManager.AddPasswordAsync(Arg.Any(), Arg.Any()).Called(Count.Never()); + userManager.GeneratePasswordResetTokenAsync(Arg.Any()).Called(Count.Never()); } [Fact] @@ -107,10 +104,8 @@ public async Task OnGet_WhenConfirmationSucceeds_ReturnsPageAndSendsExpectedComm .ReturnsAsync(user); userManager.ConfirmEmailAsync(user, "token-123") .ReturnsAsync(IdentityResult.Success); - userManager.RemovePasswordAsync(user) - .ReturnsAsync(IdentityResult.Success); - userManager.AddPasswordAsync(user, Arg.Any()) - .ReturnsAsync(IdentityResult.Success); + userManager.GeneratePasswordResetTokenAsync(user) + .ReturnsAsync("activation-token"); var signInManager = IdentityMocks.CreateSignInManager(userManager); using var provider = CreateProvider(nameof(this.OnGet_WhenConfirmationSucceeds_ReturnsPageAndSendsExpectedCommands), userManager, signInManager); @@ -125,13 +120,14 @@ public async Task OnGet_WhenConfirmationSucceeds_ReturnsPageAndSendsExpectedComm result.ShouldBeOfType(); model.Input.Username.ShouldBe("alice"); model.Input.Token.ShouldBe("token-123"); - model.View.UserMessage.ShouldBe("Thanks for confirming your email address, you should receive a welcome email soon."); + model.View.UserMessage.ShouldBe("Thanks for confirming your email address, you should receive an account activation email soon."); userManager.ConfirmEmailAsync(user, "token-123").Called(Count.Once()); - userManager.RemovePasswordAsync(user).Called(Count.Once()); - userManager.AddPasswordAsync(user, Arg.Any()).Called(Count.Once()); + userManager.GeneratePasswordResetTokenAsync(user).Called(Count.Once()); messagingClient.LastEmailRequest.ShouldNotBeNull(); messagingClient.LastEmailRequest.Subject.ShouldBe("Welcome to Transaction Processing"); messagingClient.LastEmailRequest.ToAddresses.ShouldContain("alice@example.com"); + messagingClient.LastEmailRequest.Body.ShouldContain("activation-token"); + messagingClient.LastEmailRequest.Body.ShouldNotContain("Password"); } private static ServiceProvider CreateProvider(string databaseName, diff --git a/SecurityService.UnitTests/RequestHandlers/AccountActivationTokenTests.cs b/SecurityService.UnitTests/RequestHandlers/AccountActivationTokenTests.cs new file mode 100644 index 00000000..e2b1a2c2 --- /dev/null +++ b/SecurityService.UnitTests/RequestHandlers/AccountActivationTokenTests.cs @@ -0,0 +1,44 @@ +using MediatR; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection; +using SecurityService.BusinessLogic.Requests; +using SecurityService.Database.Entities; +using SecurityService.UnitTests.Infrastructure; +using Shouldly; + +namespace SecurityService.UnitTests.RequestHandlers; + +public sealed class AccountActivationTokenTests +{ + [Fact] + public async Task ActivationToken_CannotBeReusedAfterPasswordIsSet() + { + using var provider = TestServiceProviderFactory.Create(nameof(this.ActivationToken_CannotBeReusedAfterPasswordIsSet)); + using var scope = provider.CreateScope(); + var userManager = scope.ServiceProvider.GetRequiredService>(); + var user = new ApplicationUser { UserName = "alice", Email = "alice@example.com", EmailConfirmed = true }; + + (await userManager.CreateAsync(user)).Succeeded.ShouldBeTrue(); + var token = await userManager.GeneratePasswordResetTokenAsync(user); + + (await userManager.ResetPasswordAsync(user, token, "NewPassword1!")).Succeeded.ShouldBeTrue(); + (await userManager.ResetPasswordAsync(user, token, "AnotherPassword1!")).Succeeded.ShouldBeFalse(); + } + + [Fact] + public async Task ActivationToken_ExpiresAccordingToConfiguredLifetime() + { + using var provider = TestServiceProviderFactory.Create( + nameof(this.ActivationToken_ExpiresAccordingToConfiguredLifetime), + passwordTokenLifespan: TimeSpan.FromMilliseconds(1)); + using var scope = provider.CreateScope(); + var userManager = scope.ServiceProvider.GetRequiredService>(); + var user = new ApplicationUser { UserName = "alice", Email = "alice@example.com", EmailConfirmed = true }; + + (await userManager.CreateAsync(user)).Succeeded.ShouldBeTrue(); + var token = await userManager.GeneratePasswordResetTokenAsync(user); + await Task.Delay(25); + + (await userManager.ResetPasswordAsync(user, token, "NewPassword1!")).Succeeded.ShouldBeFalse(); + } +} diff --git a/SecurityService.UnitTests/RequestHandlers/UserRequestHandlerTests.cs b/SecurityService.UnitTests/RequestHandlers/UserRequestHandlerTests.cs index f2fe619a..6426576b 100644 --- a/SecurityService.UnitTests/RequestHandlers/UserRequestHandlerTests.cs +++ b/SecurityService.UnitTests/RequestHandlers/UserRequestHandlerTests.cs @@ -14,6 +14,28 @@ namespace SecurityService.UnitTests.RequestHandlers; public class UserRequestHandlerTests { + [Fact] + public async Task CreateUser_WithEmailAddress_DoesNotCreateAnInitialPassword() + { + using var provider = TestServiceProviderFactory.Create(nameof(this.CreateUser_WithEmailAddress_DoesNotCreateAnInitialPassword)); + var mediator = provider.GetRequiredService(); + + var result = await mediator.Send(new SecurityServiceCommands.CreateUserCommand( + "Alice", null, "Tester", "alice", "SuppliedPassword1!", "alice@example.com", null, + new Dictionary(), new List())); + + result.IsSuccess.ShouldBeTrue(); + var messagingClient = provider.GetRequiredService().ShouldBeOfType(); + messagingClient.LastEmailRequest.ShouldNotBeNull(); + messagingClient.LastEmailRequest.Body.ShouldNotContain("SuppliedPassword1!"); + messagingClient.LastEmailRequest.Body.ShouldNotContain("Password"); + using var scope = provider.CreateScope(); + var userManager = scope.ServiceProvider.GetRequiredService>(); + var user = await userManager.FindByNameAsync("alice"); + user.ShouldNotBeNull(); + user.PasswordHash.ShouldBeNull(); + } + [Fact] public async Task UserLifecycle_CreateGetAndList_Works() { @@ -164,60 +186,51 @@ public async Task SendWelcomeEmail_UsesResolvedMediatorAndSendsEmail() { var user = new ApplicationUser { - UserName = "alice", + UserName = "alice+ops@example.com", Email = "alice@example.com" }; var userManager = IdentityMocks.CreateUserManager(); - userManager.FindByNameAsync("alice") + userManager.FindByNameAsync("alice+ops@example.com") .ReturnsAsync(user); - userManager.RemovePasswordAsync(user) - .ReturnsAsync(IdentityResult.Success); - userManager.AddPasswordAsync(user, Arg.Any()) - .ReturnsAsync(IdentityResult.Success); + userManager.GeneratePasswordResetTokenAsync(user) + .ReturnsAsync("activation-token"); var signInManager = IdentityMocks.CreateSignInManager(userManager); using var provider = TestServiceProviderFactory.Create(nameof(this.SendWelcomeEmail_UsesResolvedMediatorAndSendsEmail), userManager.Instance(), signInManager.Instance()); var mediator = provider.GetRequiredService(); - var result = await mediator.Send(new SecurityServiceCommands.SendWelcomeEmailCommand("alice")); + var result = await mediator.Send(new SecurityServiceCommands.SendWelcomeEmailCommand("alice+ops@example.com")); var messagingClient = provider.GetRequiredService().ShouldBeOfType(); result.IsSuccess.ShouldBeTrue(); - userManager.FindByNameAsync("alice").Called(Count.Once()); - userManager.RemovePasswordAsync(user).Called(Count.Once()); - userManager.AddPasswordAsync(user, Arg.Any()).Called(Count.Once()); + userManager.FindByNameAsync("alice+ops@example.com").Called(Count.Once()); + userManager.GeneratePasswordResetTokenAsync(user).Called(Count.Once()); messagingClient.LastEmailRequest.ShouldNotBeNull(); messagingClient.LastEmailRequest.Subject.ShouldBe("Welcome to Transaction Processing"); messagingClient.LastEmailRequest.ToAddresses.ShouldContain("alice@example.com"); + messagingClient.LastEmailRequest.Body.ShouldContain("userName=alice%2Bops@example.com"); + messagingClient.LastEmailRequest.Body.ShouldContain("activation-token"); + messagingClient.LastEmailRequest.Body.ShouldNotContain("Password"); } [Fact] - public async Task SendWelcomeEmail_WhenRemovePasswordFails_ReturnsFailure() + public async Task SendWelcomeEmail_WhenUserMissing_ReturnsNotFound() { - var user = new ApplicationUser - { - UserName = "alice", - Email = "alice@example.com" - }; var userManager = IdentityMocks.CreateUserManager(); - userManager.FindByNameAsync("alice") - .ReturnsAsync(user); - userManager.RemovePasswordAsync(user) - .ReturnsAsync(IdentityResult.Failed(new IdentityError { Description = "cannot remove" })); + userManager.FindByNameAsync("alice").ReturnsAsync((ApplicationUser?)null); var signInManager = IdentityMocks.CreateSignInManager(userManager); - using var provider = TestServiceProviderFactory.Create(nameof(this.SendWelcomeEmail_WhenRemovePasswordFails_ReturnsFailure), userManager.Instance(), signInManager.Instance()); + using var provider = TestServiceProviderFactory.Create(nameof(this.SendWelcomeEmail_WhenUserMissing_ReturnsNotFound), userManager.Instance(), signInManager.Instance()); var mediator = provider.GetRequiredService(); var result = await mediator.Send(new SecurityServiceCommands.SendWelcomeEmailCommand("alice")); - result.IsFailed.ShouldBeTrue(); - result.Status.ShouldBe(SimpleResults.ResultStatus.Failure); - userManager.AddPasswordAsync(Arg.Any(), Arg.Any()).Called(Count.Never()); + result.Status.ShouldBe(SimpleResults.ResultStatus.NotFound); + userManager.GeneratePasswordResetTokenAsync(Arg.Any()).Called(Count.Never()); } [Fact] - public async Task SendWelcomeEmail_WhenAddPasswordFails_ReturnsFailure() + public async Task ProcessAccountActivation_UsesTokenToSetPassword() { var user = new ApplicationUser { @@ -227,19 +240,36 @@ public async Task SendWelcomeEmail_WhenAddPasswordFails_ReturnsFailure() var userManager = IdentityMocks.CreateUserManager(); userManager.FindByNameAsync("alice") .ReturnsAsync(user); - userManager.RemovePasswordAsync(user) + userManager.ResetPasswordAsync(user, "activation-token", "NewPassword1!") .ReturnsAsync(IdentityResult.Success); - userManager.AddPasswordAsync(user, Arg.Any()) - .ReturnsAsync(IdentityResult.Failed(new IdentityError { Description = "cannot add" })); var signInManager = IdentityMocks.CreateSignInManager(userManager); - using var provider = TestServiceProviderFactory.Create(nameof(this.SendWelcomeEmail_WhenAddPasswordFails_ReturnsFailure), userManager.Instance(), signInManager.Instance()); + using var provider = TestServiceProviderFactory.Create(nameof(this.ProcessAccountActivation_UsesTokenToSetPassword), userManager.Instance(), signInManager.Instance()); var mediator = provider.GetRequiredService(); - var result = await mediator.Send(new SecurityServiceCommands.SendWelcomeEmailCommand("alice")); + var result = await mediator.Send(new SecurityServiceCommands.ProcessAccountActivationCommand("alice", "activation-token", "NewPassword1!")); + + result.IsSuccess.ShouldBeTrue(); + userManager.ResetPasswordAsync(user, "activation-token", "NewPassword1!").Called(Count.Once()); + } + + [Fact] + public async Task ProcessAccountActivation_WhenTokenIsInvalid_ReturnsFailure() + { + var user = new ApplicationUser { UserName = "alice", Email = "alice@example.com" }; + var userManager = IdentityMocks.CreateUserManager(); + userManager.FindByNameAsync("alice").ReturnsAsync(user); + userManager.ResetPasswordAsync(user, "invalid-token", "NewPassword1!") + .ReturnsAsync(IdentityResult.Failed(new IdentityError { Description = "invalid token" })); + + var signInManager = IdentityMocks.CreateSignInManager(userManager); + using var provider = TestServiceProviderFactory.Create(nameof(this.ProcessAccountActivation_WhenTokenIsInvalid_ReturnsFailure), userManager.Instance(), signInManager.Instance()); + var mediator = provider.GetRequiredService(); + + var result = await mediator.Send(new SecurityServiceCommands.ProcessAccountActivationCommand("alice", "invalid-token", "NewPassword1!")); result.IsFailed.ShouldBeTrue(); - result.Status.ShouldBe(SimpleResults.ResultStatus.Failure); + result.Message.ShouldBe("The account activation link is invalid or has expired."); } [Fact] @@ -247,11 +277,11 @@ public async Task ProcessPasswordResetRequest_UsesResolvedMediatorAndSendsEmail( { var user = new ApplicationUser { - UserName = "alice", + UserName = "alice+ops@example.com", Email = "alice@example.com" }; var userManager = IdentityMocks.CreateUserManager(); - userManager.FindByNameAsync("alice") + userManager.FindByNameAsync("alice+ops@example.com") .ReturnsAsync(user); userManager.GeneratePasswordResetTokenAsync(user) .ReturnsAsync("token+/="); @@ -260,16 +290,16 @@ public async Task ProcessPasswordResetRequest_UsesResolvedMediatorAndSendsEmail( using var provider = TestServiceProviderFactory.Create(nameof(this.ProcessPasswordResetRequest_UsesResolvedMediatorAndSendsEmail), userManager.Instance(), signInManager.Instance()); var mediator = provider.GetRequiredService(); - var result = await mediator.Send(new SecurityServiceCommands.ProcessPasswordResetRequestCommand("alice", "alice", "test-client-id")); + var result = await mediator.Send(new SecurityServiceCommands.ProcessPasswordResetRequestCommand("alice+ops@example.com", "alice", "test-client-id")); var messagingClient = provider.GetRequiredService().ShouldBeOfType(); result.IsSuccess.ShouldBeTrue(); - userManager.FindByNameAsync("alice").Called(Count.Once()); + userManager.FindByNameAsync("alice+ops@example.com").Called(Count.Once()); userManager.GeneratePasswordResetTokenAsync(user).Called(Count.Once()); messagingClient.LastEmailRequest.ShouldNotBeNull(); messagingClient.LastEmailRequest.Subject.ShouldBe("Password Reset Requested"); messagingClient.LastEmailRequest.ToAddresses.ShouldContain("alice@example.com"); - messagingClient.LastEmailRequest.Body.ShouldContain("/Account/ResetPassword?userName=alice&resetToken=token%2B%2F%3D&clientId=test-client-id"); + messagingClient.LastEmailRequest.Body.ShouldContain("/Account/ResetPassword?userName=alice%2Bops@example.com&resetToken=token%2B%2F%3D&clientId=test-client-id"); } [Fact] diff --git a/SecurityService/Pages/Account/ActivateAccount/Index.cshtml b/SecurityService/Pages/Account/ActivateAccount/Index.cshtml new file mode 100644 index 00000000..66426ddc --- /dev/null +++ b/SecurityService/Pages/Account/ActivateAccount/Index.cshtml @@ -0,0 +1,43 @@ +@page +@model SecurityService.Pages.Account.ActivateAccount.IndexModel +@{ + ViewData["Title"] = "Activate account"; +} + +
+
+

Activate account

+

Choose a password for your account.

+
+ + + + @if (string.IsNullOrWhiteSpace(Model.View.UserMessage) == false) + { +

@Model.View.UserMessage

+ Return to login + } + else + { +
+
+
+ + + +
+ + +
+ +
+ + +
+ + +
+
+
+ } +
diff --git a/SecurityService/Pages/Account/ActivateAccount/Index.cshtml.cs b/SecurityService/Pages/Account/ActivateAccount/Index.cshtml.cs new file mode 100644 index 00000000..1fc0ad7d --- /dev/null +++ b/SecurityService/Pages/Account/ActivateAccount/Index.cshtml.cs @@ -0,0 +1,72 @@ +using MediatR; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.RazorPages; +using SecurityService.BusinessLogic.Requests; +using SimpleResults; +using System.ComponentModel.DataAnnotations; + +namespace SecurityService.Pages.Account.ActivateAccount; + +public sealed class IndexModel : PageModel +{ + private readonly IMediator _mediator; + + public ViewModel View { get; private set; } = new(); + + [BindProperty] + public InputModel Input { get; set; } = new(); + + public IndexModel(IMediator mediator) + { + this._mediator = mediator; + } + + public IActionResult OnGet() + { + this.Input = new InputModel + { + Username = this.Request.Query["userName"].FirstOrDefault() ?? string.Empty, + Token = this.Request.Query["activationToken"].FirstOrDefault() ?? string.Empty + }; + return this.Page(); + } + + public async Task OnPostAsync(CancellationToken cancellationToken) + { + if (this.ModelState.IsValid == false || String.Equals(this.Input.Password, this.Input.ConfirmPassword, StringComparison.Ordinal) == false) + { + if (String.Equals(this.Input.Password, this.Input.ConfirmPassword, StringComparison.Ordinal) == false) + this.ModelState.AddModelError(string.Empty, "Password does not match Confirm Password"); + return this.Page(); + } + + Result result = await this._mediator.Send( + new SecurityServiceCommands.ProcessAccountActivationCommand(this.Input.Username, this.Input.Token, this.Input.Password), + cancellationToken); + + this.View = new ViewModel + { + UserMessage = result.IsSuccess + ? "Your account has been activated. You can now log in." + : "The account activation link is invalid or has expired." + }; + return this.Page(); + } + + public sealed class ViewModel + { + public string UserMessage { get; set; } = string.Empty; + } + + public sealed class InputModel + { + public string Username { get; set; } = string.Empty; + public string Token { get; set; } = string.Empty; + + [Required] + public string Password { get; set; } = string.Empty; + + [Required] + public string ConfirmPassword { get; set; } = string.Empty; + } +} diff --git a/SecurityService/Pages/Account/ConfirmEmail/Index.cshtml.cs b/SecurityService/Pages/Account/ConfirmEmail/Index.cshtml.cs index 332bda83..269797de 100644 --- a/SecurityService/Pages/Account/ConfirmEmail/Index.cshtml.cs +++ b/SecurityService/Pages/Account/ConfirmEmail/Index.cshtml.cs @@ -39,7 +39,7 @@ public async Task OnGet(CancellationToken cancellationToken) } else { - this.View.UserMessage = $"Thanks for confirming your email address, you should receive a welcome email soon."; + this.View.UserMessage = $"Thanks for confirming your email address, you should receive an account activation email soon."; SecurityServiceCommands.SendWelcomeEmailCommand command = new(this.Input.Username); await this._mediator.Send(command, cancellationToken); } diff --git a/docs/superpowers/plans/2026-10-05-account-activation.md b/docs/superpowers/plans/2026-10-05-account-activation.md new file mode 100644 index 00000000..e5e9c20c --- /dev/null +++ b/docs/superpowers/plans/2026-10-05-account-activation.md @@ -0,0 +1,26 @@ +# Account activation instead of emailed passwords + +## Goal + +Ensure emailed user passwords are never generated, persisted for normal email-backed users, or included in email payloads. New users receive a short-lived Identity password token and choose their password through `/Account/ActivateAccount`. + +## Tasks + +1. Add failing unit tests for activation email content, activation processing, and user creation without a password hash. +2. Change `UserRequestHandler` to create email-backed users without a password, send activation links, and resend either email confirmation or activation depending on account state. +3. Add the activation command and Razor Page; use `ResetPasswordAsync` so Identity enforces token expiry and single-use semantics. +4. Update existing confirmation, resend, unit, and integration expectations from welcome-password behavior to activation behavior. +5. Run focused tests, the complete solution test suite, and build verification. + +## Design decisions + +- Existing authentication/token endpoints remain unchanged. +- The existing password-reset data-protection provider and configured two-hour lifetime are reused for activation tokens; successful password reset rotates the security stamp, making the token single-use. +- Users without an email address retain the legacy direct-password path because there is no email channel through which to activate them. +- The public `CreateUserRequest.Password` property remains temporarily for compatibility, but is ignored for email-backed users and never returned or emailed. + +## Review focus + +- No plaintext password reaches an email builder, log message, result, or response. +- Invalid, expired, and reused activation tokens fail without changing the password. +- Existing forgot-password and login flows remain compatible.