From 1d54fa1f4d0b951ad3f9e58cce0d7eb69d02e910 Mon Sep 17 00:00:00 2001 From: Ted Wollman <25165500+TheTedder@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:53:30 -0400 Subject: [PATCH] Test Recovery Code Endpoint (#196) * Remove unused import. * Rename AccountRecoveryTests. * Add RecoverAccount route. * Create test acount recovery tests. * Add new result. * Implement TestRecovery. * Add GET /account/recover/{id} endpoint. * Update openapi.json. * Fix test file naming. * Fix missing Test attributes. * Don't recreate the client for each test. * formatting * Fix incorrect role checking. * Allow admins to recover their accounts. * Generalize role-based test. * Delete Old result. * Only banned users cannot recover their account. * Use OneTimeSetUp instead of constructor. --- LeaderboardBackend.Test/Consts.cs | 5 +- ...tRecoveryTests.cs => SendRecoveryTests.cs} | 2 +- .../Features/Users/TestRecoveryTests.cs | 161 ++++++++++++++++++ .../Controllers/AccountController.cs | 25 +++ .../Models/Entities/AccountRecovery.cs | 1 - .../Services/IAccountRecoveryService.cs | 5 + .../Services/Impl/AccountRecoveryService.cs | 44 +++++ LeaderboardBackend/openapi.json | 45 +++++ 8 files changed, 285 insertions(+), 3 deletions(-) rename LeaderboardBackend.Test/Features/Users/{AccountRecoveryTests.cs => SendRecoveryTests.cs} (99%) create mode 100644 LeaderboardBackend.Test/Features/Users/TestRecoveryTests.cs diff --git a/LeaderboardBackend.Test/Consts.cs b/LeaderboardBackend.Test/Consts.cs index 1956f427..258e19df 100644 --- a/LeaderboardBackend.Test/Consts.cs +++ b/LeaderboardBackend.Test/Consts.cs @@ -8,5 +8,8 @@ internal static class Routes public const string REGISTER = "/account/register"; public const string RESEND_CONFIRMATION = "/account/confirm"; public const string RECOVER_ACCOUNT = "/account/recover"; - public static string ConfirmAccount(Guid id) => $"/account/confirm/{id.ToUrlSafeBase64String()}"; + public static string ConfirmAccount(string id) => $"/account/confirm/{id}"; + public static string ConfirmAccount(Guid id) => ConfirmAccount(id.ToUrlSafeBase64String()); + public static string RecoverAccount(string id) => $"/account/recover/{id}"; + public static string RecoverAccount(Guid id) => RecoverAccount(id.ToUrlSafeBase64String()); } diff --git a/LeaderboardBackend.Test/Features/Users/AccountRecoveryTests.cs b/LeaderboardBackend.Test/Features/Users/SendRecoveryTests.cs similarity index 99% rename from LeaderboardBackend.Test/Features/Users/AccountRecoveryTests.cs rename to LeaderboardBackend.Test/Features/Users/SendRecoveryTests.cs index e62cb4c5..e722ad57 100644 --- a/LeaderboardBackend.Test/Features/Users/AccountRecoveryTests.cs +++ b/LeaderboardBackend.Test/Features/Users/SendRecoveryTests.cs @@ -17,7 +17,7 @@ namespace LeaderboardBackend.Test.Features.Users; [TestFixture] -public class AccountRecoveryTests : IntegrationTestsBase +public class SendRecoveryTests : IntegrationTestsBase { private IServiceScope _scope = null!; diff --git a/LeaderboardBackend.Test/Features/Users/TestRecoveryTests.cs b/LeaderboardBackend.Test/Features/Users/TestRecoveryTests.cs new file mode 100644 index 00000000..0811b2c8 --- /dev/null +++ b/LeaderboardBackend.Test/Features/Users/TestRecoveryTests.cs @@ -0,0 +1,161 @@ +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using LeaderboardBackend.Models.Entities; +using LeaderboardBackend.Test.Fixtures; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using NodaTime; +using NodaTime.Testing; +using NUnit.Framework; + +namespace LeaderboardBackend.Test.Features.Users; + +public class TestRecoveryTests : IntegrationTestsBase +{ + private IServiceScope _scope = null!; + private HttpClient _client = null!; + private readonly FakeClock _clock = new(Instant.FromUnixTimeSeconds(1)); + + [OneTimeSetUp] + public void OneTimeSetUp() + { + _client = _factory.WithWebHostBuilder(builder => + builder.ConfigureTestServices(services => + services.AddSingleton(_ => _clock) + ) + ).CreateClient(); + } + + [SetUp] + public void Init() + { + _factory.ResetDatabase(); + _scope = _factory.Services.CreateScope(); + } + + [TearDown] + public void TearDown() => _scope.Dispose(); + + [TestCase("not_a_guid")] + [TestCase("L8msfy9wd0qWbDJMZwwgQg")] + public async Task TestRecovery_BadRecoveryId(string id) + { + HttpResponseMessage res = await _client.GetAsync(Routes.RecoverAccount(id)); + res.Should().HaveStatusCode(HttpStatusCode.NotFound); + } + + [Test] + public async Task TestRecovery_Expired() + { + _clock.Reset(Instant.FromUnixTimeSeconds(1) + Duration.FromHours(2)); + ApplicationContext context = _scope.ServiceProvider.GetRequiredService(); + + AccountRecovery recovery = new() + { + CreatedAt = Instant.FromUnixTimeSeconds(0), + ExpiresAt = Instant.FromUnixTimeSeconds(0).Plus(Duration.FromHours(1)), + User = new() + { + Email = "test@email.com", + Password = "password", + Username = "username", + Role = UserRole.Confirmed + } + }; + + await context.AccountRecoveries.AddAsync(recovery); + await context.SaveChangesAsync(); + HttpResponseMessage res = await _client.GetAsync(Routes.RecoverAccount(recovery.Id)); + res.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + + [Test] + public async Task TestRecovery_Old() + { + _clock.Reset(Instant.FromUnixTimeSeconds(10)); + ApplicationContext context = _scope.ServiceProvider.GetRequiredService(); + + User user = new() + { + Email = "test@email.com", + Password = "password", + Username = "username", + Role = UserRole.Confirmed + }; + await context.Users.AddAsync(user); + + AccountRecovery recovery1 = new() + { + CreatedAt = Instant.FromUnixTimeSeconds(0), + ExpiresAt = Instant.FromUnixTimeSeconds(0).Plus(Duration.FromHours(1)), + User = user + }; + + AccountRecovery recovery2 = new() + { + CreatedAt = Instant.FromUnixTimeSeconds(5), + ExpiresAt = Instant.FromUnixTimeSeconds(5).Plus(Duration.FromHours(1)), + User = user + }; + + await context.AccountRecoveries.AddRangeAsync(recovery1, recovery2); + await context.SaveChangesAsync(); + HttpResponseMessage res = await _client.GetAsync(Routes.RecoverAccount(recovery1.Id)); + res.Should().HaveStatusCode(HttpStatusCode.NotFound); + } + + [Test] + public async Task TestRecovery_Used() + { + _clock.Reset(Instant.FromUnixTimeSeconds(10)); + ApplicationContext context = _scope.ServiceProvider.GetRequiredService(); + + AccountRecovery recovery = new() + { + CreatedAt = Instant.FromUnixTimeSeconds(0), + ExpiresAt = Instant.FromUnixTimeSeconds(0).Plus(Duration.FromHours(1)), + UsedAt = Instant.FromUnixTimeSeconds(5), + User = new() + { + Email = "test@email.com", + Password = "password", + Username = "username", + Role = UserRole.Confirmed + } + }; + + await context.AccountRecoveries.AddAsync(recovery); + await context.SaveChangesAsync(); + HttpResponseMessage res = await _client.GetAsync(Routes.RecoverAccount(recovery.Id)); + res.Should().HaveStatusCode(HttpStatusCode.NotFound); + } + + [TestCase(UserRole.Administrator, HttpStatusCode.OK)] + [TestCase(UserRole.Banned, HttpStatusCode.NotFound)] + [TestCase(UserRole.Confirmed, HttpStatusCode.OK)] + [TestCase(UserRole.Registered, HttpStatusCode.OK)] + public async Task TestRecovery_Roles(UserRole role, HttpStatusCode expected) + { + _clock.Reset(Instant.FromUnixTimeSeconds(1)); + ApplicationContext context = _scope.ServiceProvider.GetRequiredService(); + + AccountRecovery recovery = new() + { + CreatedAt = Instant.FromUnixTimeSeconds(0), + ExpiresAt = Instant.FromUnixTimeSeconds(0).Plus(Duration.FromHours(1)), + User = new() + { + Email = "test@email.com", + Password = "password", + Username = "username", + Role = role + } + }; + + await context.AccountRecoveries.AddAsync(recovery); + await context.SaveChangesAsync(); + HttpResponseMessage res = await _client.GetAsync(Routes.RecoverAccount(recovery.Id)); + res.Should().HaveStatusCode(expected); + } +} diff --git a/LeaderboardBackend/Controllers/AccountController.cs b/LeaderboardBackend/Controllers/AccountController.cs index cce08398..7e2989d7 100644 --- a/LeaderboardBackend/Controllers/AccountController.cs +++ b/LeaderboardBackend/Controllers/AccountController.cs @@ -222,4 +222,29 @@ public async Task ConfirmAccount(Guid id, [FromServices] IAccountC expired => NotFound() ); } + + /// + /// Tests an account recovery token for validity. + /// + /// The recovery token. + /// IAccountRecoveryService dependency. + /// The token provided is valid. + /// The token provided is invalid or expired, or the user is banned. + [AllowAnonymous] + [HttpGet("recover/{id}")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + [ProducesResponseType(StatusCodes.Status404NotFound)] + public async Task TestRecovery(Guid id, [FromServices] IAccountRecoveryService recoveryService) + { + RecoverAccountResult result = await recoveryService.TestRecovery(id); + + return result.Match( + alreadyUsed => NotFound(), + badRole => NotFound(), + expired => NotFound(), + notFound => NotFound(), + success => Ok() + ); + } } diff --git a/LeaderboardBackend/Models/Entities/AccountRecovery.cs b/LeaderboardBackend/Models/Entities/AccountRecovery.cs index f55e554a..35c07655 100644 --- a/LeaderboardBackend/Models/Entities/AccountRecovery.cs +++ b/LeaderboardBackend/Models/Entities/AccountRecovery.cs @@ -1,5 +1,4 @@ using System.ComponentModel.DataAnnotations; -using System.Text.Json.Serialization; using NodaTime; namespace LeaderboardBackend.Models.Entities; diff --git a/LeaderboardBackend/Services/IAccountRecoveryService.cs b/LeaderboardBackend/Services/IAccountRecoveryService.cs index 4a29d889..d8b6cde9 100644 --- a/LeaderboardBackend/Services/IAccountRecoveryService.cs +++ b/LeaderboardBackend/Services/IAccountRecoveryService.cs @@ -1,13 +1,18 @@ using LeaderboardBackend.Models.Entities; using LeaderboardBackend.Result; using OneOf; +using OneOf.Types; namespace LeaderboardBackend.Services; public interface IAccountRecoveryService { Task CreateRecoveryAndSendEmail(User user); + Task TestRecovery(Guid id); } [GenerateOneOf] public partial class CreateRecoveryResult : OneOfBase { }; + +[GenerateOneOf] +public partial class RecoverAccountResult : OneOfBase { }; diff --git a/LeaderboardBackend/Services/Impl/AccountRecoveryService.cs b/LeaderboardBackend/Services/Impl/AccountRecoveryService.cs index 79d19a4a..01217760 100644 --- a/LeaderboardBackend/Services/Impl/AccountRecoveryService.cs +++ b/LeaderboardBackend/Services/Impl/AccountRecoveryService.cs @@ -1,7 +1,9 @@ using LeaderboardBackend.Models.Entities; using LeaderboardBackend.Result; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; using NodaTime; +using OneOf.Types; namespace LeaderboardBackend.Services; @@ -70,4 +72,46 @@ private string GenerateAccountRecoveryEmailBody(User user, AccountRecovery recov return $@"Hi {user.Username},

Click here to reset your password."; } + + public async Task TestRecovery(Guid id) + { + AccountRecovery? recovery = await _applicationContext.AccountRecoveries.Include(ar => ar.User).SingleOrDefaultAsync(ar => ar.Id == id); + + if (recovery is null) + { + return new NotFound(); + } + + if (recovery.User.Role is UserRole.Banned) + { + return new BadRole(); + } + + if (recovery.UsedAt is not null) + { + return new AlreadyUsed(); + } + + Instant now = _clock.GetCurrentInstant(); + + if (recovery.ExpiresAt <= now) + { + return new Expired(); + } + + IQueryable latest = + from rec in _applicationContext.AccountRecoveries + where rec.UserId == recovery.UserId + orderby rec.CreatedAt descending + select rec.Id; + + Guid latestId = await latest.FirstAsync(); + + if (latestId != id) + { + return new Expired(); + } + + return new Success(); + } } diff --git a/LeaderboardBackend/openapi.json b/LeaderboardBackend/openapi.json index 295f442a..d51c22c9 100644 --- a/LeaderboardBackend/openapi.json +++ b/LeaderboardBackend/openapi.json @@ -261,6 +261,51 @@ } } }, + "/Account/recover/{id}": { + "get": { + "tags": [ + "Account" + ], + "summary": "Tests an account recovery token for validity.", + "parameters": [ + { + "name": "id", + "in": "path", + "description": "The recovery token.", + "required": true, + "schema": { + "pattern": "^[a-zA-Z0-9-_]{22}$", + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "The token provided is valid." + }, + "400": { + "description": "Bad Request", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "404": { + "description": "The token provided is invalid or expired, or the user is banned.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/Categories/{id}": { "get": { "tags": [