From 8002b41f1f0594fec05e96ad4a598a87e80bcb8d Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 20:54:57 +0100 Subject: [PATCH 1/4] Add auth service edge case tests for login, registration, token validation, and external login Tests cover: blank credentials, inactive user login, wrong password, concurrent JWT uniqueness, duplicate email registration, username length validation, invalid email format, malformed/expired/ wrong-key/wrong-issuer/wrong-audience token rejection, missing/non-GUID sub claim rejection, deleted/inactive user token validation, password change behavior, and external login edge cases including deleted linked account and username collision overflow (documents latent Substring bug). Linked to #707. --- .../AuthenticationServiceEdgeCaseTests.cs | 593 ++++++++++++++++++ 1 file changed, 593 insertions(+) create mode 100644 backend/tests/Taskdeck.Application.Tests/Services/AuthenticationServiceEdgeCaseTests.cs diff --git a/backend/tests/Taskdeck.Application.Tests/Services/AuthenticationServiceEdgeCaseTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/AuthenticationServiceEdgeCaseTests.cs new file mode 100644 index 000000000..c8f1bf2dd --- /dev/null +++ b/backend/tests/Taskdeck.Application.Tests/Services/AuthenticationServiceEdgeCaseTests.cs @@ -0,0 +1,593 @@ +using System.IdentityModel.Tokens.Jwt; +using System.Security.Claims; +using System.Text; +using FluentAssertions; +using Microsoft.IdentityModel.Tokens; +using Moq; +using Taskdeck.Application.DTOs; +using Taskdeck.Application.Interfaces; +using Taskdeck.Application.Services; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Exceptions; +using Xunit; + +namespace Taskdeck.Application.Tests.Services; + +/// +/// Edge-case tests for AuthenticationService: login, registration, password change, +/// token validation, and external login scenarios not covered by the happy-path suite. +/// Linked to #707 (TST-40). +/// +public class AuthenticationServiceEdgeCaseTests +{ + private static readonly JwtSettings DefaultJwtSettings = new() + { + SecretKey = "TaskdeckTestsOnlySecretKeyMustBeLongEnough123!", + Issuer = "TaskdeckTests", + Audience = "TaskdeckUsers", + ExpirationMinutes = 60 + }; + + private readonly Mock _unitOfWorkMock; + private readonly Mock _userRepoMock; + private readonly Mock _externalLoginRepoMock; + + public AuthenticationServiceEdgeCaseTests() + { + _unitOfWorkMock = new Mock(); + _userRepoMock = new Mock(); + _externalLoginRepoMock = new Mock(); + + _unitOfWorkMock.Setup(u => u.Users).Returns(_userRepoMock.Object); + _unitOfWorkMock.Setup(u => u.ExternalLogins).Returns(_externalLoginRepoMock.Object); + } + + // ───────────────────────────────────────────────────────── + // Login edge cases + // ───────────────────────────────────────────────────────── + + [Theory] + [InlineData(null, "password")] + [InlineData("", "password")] + [InlineData(" ", "password")] + public async Task LoginAsync_ShouldReturnValidationError_WhenUsernameOrEmailIsBlank( + string? usernameOrEmail, string password) + { + var service = CreateService(); + var result = await service.LoginAsync(new LoginDto(usernameOrEmail!, password)); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.ValidationError); + } + + [Fact] + public async Task LoginAsync_ShouldReturnForbidden_WhenOnlyMatchIsInactiveUser() + { + var password = "correctPassword1"; + var user = new User("inactiveuser", "inactive@test.com", BCrypt.Net.BCrypt.HashPassword(password)); + user.Deactivate(); + var service = CreateService(); + + _userRepoMock.Setup(r => r.GetByUsernameAsync("inactiveuser", default)).ReturnsAsync(user); + _userRepoMock.Setup(r => r.GetByEmailAsync("inactiveuser", default)).ReturnsAsync((User?)null); + + var result = await service.LoginAsync(new LoginDto("inactiveuser", password)); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Forbidden); + result.ErrorMessage.Should().Contain("inactive"); + } + + [Fact] + public async Task LoginAsync_ShouldReturnAuthFailed_WhenNoUserExists() + { + var service = CreateService(); + _userRepoMock.Setup(r => r.GetByUsernameAsync("nonexistent", default)).ReturnsAsync((User?)null); + _userRepoMock.Setup(r => r.GetByEmailAsync("nonexistent", default)).ReturnsAsync((User?)null); + + var result = await service.LoginAsync(new LoginDto("nonexistent", "password")); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.AuthenticationFailed); + } + + [Fact] + public async Task LoginAsync_ShouldSucceed_AfterPreviousFailedAttempt() + { + var password = "correctPassword1"; + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword(password)); + var service = CreateService(); + + _userRepoMock.Setup(r => r.GetByUsernameAsync("testuser", default)).ReturnsAsync(user); + _userRepoMock.Setup(r => r.GetByEmailAsync("testuser", default)).ReturnsAsync((User?)null); + + // First: wrong password + var failResult = await service.LoginAsync(new LoginDto("testuser", "wrongPassword")); + failResult.IsSuccess.Should().BeFalse(); + + // Then: correct password still works (no lockout) + var successResult = await service.LoginAsync(new LoginDto("testuser", password)); + successResult.IsSuccess.Should().BeTrue(); + successResult.Value.Token.Should().NotBeNullOrWhiteSpace(); + } + + [Fact] + public async Task LoginAsync_ConcurrentLogins_ShouldReturnDifferentJtis() + { + var password = "password123"; + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword(password)); + var service = CreateService(); + + _userRepoMock.Setup(r => r.GetByUsernameAsync("testuser", default)).ReturnsAsync(user); + + var result1 = await service.LoginAsync(new LoginDto("testuser", password)); + var result2 = await service.LoginAsync(new LoginDto("testuser", password)); + + result1.IsSuccess.Should().BeTrue(); + result2.IsSuccess.Should().BeTrue(); + + // Both tokens valid but have different JTI (unique token IDs) + var handler = new JwtSecurityTokenHandler(); + var jwt1 = handler.ReadJwtToken(result1.Value.Token); + var jwt2 = handler.ReadJwtToken(result2.Value.Token); + jwt1.Id.Should().NotBe(jwt2.Id); + } + + // ───────────────────────────────────────────────────────── + // Registration edge cases + // ───────────────────────────────────────────────────────── + + [Fact] + public async Task RegisterAsync_ShouldReturnConflict_WhenDuplicateEmail() + { + var service = CreateService(); + _userRepoMock.Setup(r => r.ExistsAsync("newuser", "existing@test.com", default)).ReturnsAsync(true); + + var result = await service.RegisterAsync(new CreateUserDto("newuser", "existing@test.com", "password123")); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Conflict); + } + + [Theory] + [InlineData("", "email@test.com", "password123")] + [InlineData(" ", "email@test.com", "password123")] + [InlineData("user", "", "password123")] + [InlineData("user", " ", "password123")] + public async Task RegisterAsync_ShouldReturnValidationError_WhenRequiredFieldsBlank( + string username, string email, string password) + { + var service = CreateService(); + + var result = await service.RegisterAsync(new CreateUserDto(username, email, password)); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.ValidationError); + } + + [Fact] + public async Task RegisterAsync_ShouldReturnError_WhenUsernameTooShort() + { + var service = CreateService(); + _userRepoMock.Setup(r => r.ExistsAsync("ab", "test@test.com", default)).ReturnsAsync(false); + _userRepoMock.Setup(r => r.AddAsync(It.IsAny(), default)) + .ReturnsAsync((User u, CancellationToken _) => u); + + // Username < 3 chars should be rejected by the User entity constructor + var result = await service.RegisterAsync(new CreateUserDto("ab", "test@test.com", "password123")); + + result.IsSuccess.Should().BeFalse(); + } + + [Fact] + public async Task RegisterAsync_ShouldReturnError_WhenUsernameTooLong() + { + var service = CreateService(); + var longUsername = new string('a', 51); + _userRepoMock.Setup(r => r.ExistsAsync(longUsername, "test@test.com", default)).ReturnsAsync(false); + _userRepoMock.Setup(r => r.AddAsync(It.IsAny(), default)) + .ReturnsAsync((User u, CancellationToken _) => u); + + var result = await service.RegisterAsync(new CreateUserDto(longUsername, "test@test.com", "password123")); + + result.IsSuccess.Should().BeFalse(); + } + + [Fact] + public async Task RegisterAsync_ShouldReturnError_WhenEmailInvalid() + { + var service = CreateService(); + _userRepoMock.Setup(r => r.ExistsAsync("validuser", "not-an-email", default)).ReturnsAsync(false); + _userRepoMock.Setup(r => r.AddAsync(It.IsAny(), default)) + .ReturnsAsync((User u, CancellationToken _) => u); + + var result = await service.RegisterAsync(new CreateUserDto("validuser", "not-an-email", "password123")); + + result.IsSuccess.Should().BeFalse(); + } + + // ───────────────────────────────────────────────────────── + // Token validation edge cases + // ───────────────────────────────────────────────────────── + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectMalformedToken() + { + var service = CreateService(); + + var result = await service.ValidateTokenAsync("not.a.jwt.token"); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectTokenSignedWithWrongKey() + { + // Create a valid-looking token but signed with a different key + var wrongKey = new SymmetricSecurityKey( + Encoding.UTF8.GetBytes("DifferentSecretKeyThatIsLongEnoughForHmac256!")); + var credentials = new SigningCredentials(wrongKey, SecurityAlgorithms.HmacSha256); + + var token = new JwtSecurityToken( + issuer: DefaultJwtSettings.Issuer, + audience: DefaultJwtSettings.Audience, + claims: new[] + { + new Claim(JwtRegisteredClaimNames.Sub, Guid.NewGuid().ToString()), + new Claim(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()), + }, + expires: DateTime.UtcNow.AddHours(1), + signingCredentials: credentials); + + var tokenString = new JwtSecurityTokenHandler().WriteToken(token); + var service = CreateService(); + + var result = await service.ValidateTokenAsync(tokenString); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectExpiredToken() + { + // Create a token that expired in the past + var key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(DefaultJwtSettings.SecretKey)); + var credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256); + + var token = new JwtSecurityToken( + issuer: DefaultJwtSettings.Issuer, + audience: DefaultJwtSettings.Audience, + claims: new[] + { + new Claim(JwtRegisteredClaimNames.Sub, Guid.NewGuid().ToString()), + new Claim(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()), + }, + notBefore: DateTime.UtcNow.AddHours(-2), + expires: DateTime.UtcNow.AddHours(-1), + signingCredentials: credentials); + + var tokenString = new JwtSecurityTokenHandler().WriteToken(token); + var service = CreateService(); + + var result = await service.ValidateTokenAsync(tokenString); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectTokenWithFutureNbf() + { + // Token not valid yet (notBefore in the future) + var key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(DefaultJwtSettings.SecretKey)); + var credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256); + + var token = new JwtSecurityToken( + issuer: DefaultJwtSettings.Issuer, + audience: DefaultJwtSettings.Audience, + claims: new[] + { + new Claim(JwtRegisteredClaimNames.Sub, Guid.NewGuid().ToString()), + new Claim(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()), + }, + notBefore: DateTime.UtcNow.AddHours(1), + expires: DateTime.UtcNow.AddHours(2), + signingCredentials: credentials); + + var tokenString = new JwtSecurityTokenHandler().WriteToken(token); + var service = CreateService(); + + var result = await service.ValidateTokenAsync(tokenString); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectTokenWithMissingSubClaim() + { + var key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(DefaultJwtSettings.SecretKey)); + var credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256); + + // No sub/NameIdentifier claim at all + var token = new JwtSecurityToken( + issuer: DefaultJwtSettings.Issuer, + audience: DefaultJwtSettings.Audience, + claims: new[] + { + new Claim(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()), + new Claim("username", "testuser"), + }, + expires: DateTime.UtcNow.AddHours(1), + signingCredentials: credentials); + + var tokenString = new JwtSecurityTokenHandler().WriteToken(token); + var service = CreateService(); + + var result = await service.ValidateTokenAsync(tokenString); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectTokenWithNonGuidSubClaim() + { + var key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(DefaultJwtSettings.SecretKey)); + var credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256); + + var token = new JwtSecurityToken( + issuer: DefaultJwtSettings.Issuer, + audience: DefaultJwtSettings.Audience, + claims: new[] + { + new Claim(JwtRegisteredClaimNames.Sub, "not-a-guid"), + new Claim(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()), + }, + expires: DateTime.UtcNow.AddHours(1), + signingCredentials: credentials); + + var tokenString = new JwtSecurityTokenHandler().WriteToken(token); + var service = CreateService(); + + var result = await service.ValidateTokenAsync(tokenString); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectTokenForDeletedUser() + { + var password = "password123"; + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword(password)); + var service = CreateService(); + + _userRepoMock.Setup(r => r.GetByUsernameAsync("testuser", default)).ReturnsAsync(user); + + // Login to get a valid token + var loginResult = await service.LoginAsync(new LoginDto("testuser", password)); + loginResult.IsSuccess.Should().BeTrue(); + + // Simulate user deletion: GetByIdAsync returns null + _userRepoMock.Setup(r => r.GetByIdAsync(user.Id, default)).ReturnsAsync((User?)null); + + var validateResult = await service.ValidateTokenAsync(loginResult.Value.Token); + + validateResult.IsSuccess.Should().BeFalse(); + validateResult.ErrorCode.Should().Be(ErrorCodes.NotFound); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectTokenForInactiveUser() + { + var password = "password123"; + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword(password)); + var service = CreateService(); + + _userRepoMock.Setup(r => r.GetByUsernameAsync("testuser", default)).ReturnsAsync(user); + _userRepoMock.Setup(r => r.GetByIdAsync(user.Id, default)).ReturnsAsync(user); + + var loginResult = await service.LoginAsync(new LoginDto("testuser", password)); + loginResult.IsSuccess.Should().BeTrue(); + + // Deactivate user after token was issued + user.Deactivate(); + + var validateResult = await service.ValidateTokenAsync(loginResult.Value.Token); + + validateResult.IsSuccess.Should().BeFalse(); + validateResult.ErrorCode.Should().Be(ErrorCodes.Forbidden); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectTokenWithWrongIssuer() + { + var key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(DefaultJwtSettings.SecretKey)); + var credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256); + + var token = new JwtSecurityToken( + issuer: "WrongIssuer", + audience: DefaultJwtSettings.Audience, + claims: new[] + { + new Claim(JwtRegisteredClaimNames.Sub, Guid.NewGuid().ToString()), + new Claim(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()), + }, + expires: DateTime.UtcNow.AddHours(1), + signingCredentials: credentials); + + var tokenString = new JwtSecurityTokenHandler().WriteToken(token); + var service = CreateService(); + + var result = await service.ValidateTokenAsync(tokenString); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + [Fact] + public async Task ValidateTokenAsync_ShouldRejectTokenWithWrongAudience() + { + var key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(DefaultJwtSettings.SecretKey)); + var credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256); + + var token = new JwtSecurityToken( + issuer: DefaultJwtSettings.Issuer, + audience: "WrongAudience", + claims: new[] + { + new Claim(JwtRegisteredClaimNames.Sub, Guid.NewGuid().ToString()), + new Claim(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()), + }, + expires: DateTime.UtcNow.AddHours(1), + signingCredentials: credentials); + + var tokenString = new JwtSecurityTokenHandler().WriteToken(token); + var service = CreateService(); + + var result = await service.ValidateTokenAsync(tokenString); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + // ───────────────────────────────────────────────────────── + // Password change edge cases + // ───────────────────────────────────────────────────────── + + [Fact] + public async Task ChangePasswordAsync_ShouldReturnNotFound_WhenUserDoesNotExist() + { + var service = CreateService(); + _userRepoMock.Setup(r => r.GetByIdAsync(It.IsAny(), default)).ReturnsAsync((User?)null); + + var result = await service.ChangePasswordAsync(Guid.NewGuid(), "old", "new"); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.NotFound); + } + + [Fact] + public async Task ChangePasswordAsync_ShouldReturnAuthFailed_WhenCurrentPasswordIsWrong() + { + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword("correct")); + var service = CreateService(); + _userRepoMock.Setup(r => r.GetByIdAsync(user.Id, default)).ReturnsAsync(user); + + var result = await service.ChangePasswordAsync(user.Id, "wrong", "newpassword"); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.AuthenticationFailed); + } + + [Fact] + public async Task ChangePasswordAsync_ShouldSucceed_AndUpdateHash() + { + var oldPassword = "oldPassword1"; + var newPassword = "newPassword1"; + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword(oldPassword)); + var service = CreateService(); + _userRepoMock.Setup(r => r.GetByIdAsync(user.Id, default)).ReturnsAsync(user); + + var result = await service.ChangePasswordAsync(user.Id, oldPassword, newPassword); + + result.IsSuccess.Should().BeTrue(); + // The user's password hash should now verify against the new password + BCrypt.Net.BCrypt.Verify(newPassword, user.PasswordHash).Should().BeTrue(); + BCrypt.Net.BCrypt.Verify(oldPassword, user.PasswordHash).Should().BeFalse(); + } + + [Fact] + public async Task PasswordChange_DoesNotInvalidateExistingTokens() + { + // Current behavior: password change does NOT set TokenInvalidatedAt. + // Existing JWTs remain valid until natural expiry. Document this behavior. + var oldPassword = "oldPassword1"; + var newPassword = "newPassword1"; + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword(oldPassword)); + var service = CreateService(); + + _userRepoMock.Setup(r => r.GetByUsernameAsync("testuser", default)).ReturnsAsync(user); + _userRepoMock.Setup(r => r.GetByIdAsync(user.Id, default)).ReturnsAsync(user); + + // Get a token before password change + var loginResult = await service.LoginAsync(new LoginDto("testuser", oldPassword)); + loginResult.IsSuccess.Should().BeTrue(); + + // Change password + var changeResult = await service.ChangePasswordAsync(user.Id, oldPassword, newPassword); + changeResult.IsSuccess.Should().BeTrue(); + + // Validate old token — it should still work (TokenInvalidatedAt not set) + var validateResult = await service.ValidateTokenAsync(loginResult.Value.Token); + validateResult.IsSuccess.Should().BeTrue(); + } + + // ───────────────────────────────────────────────────────── + // External login edge cases + // ───────────────────────────────────────────────────────── + + [Fact] + public async Task ExternalLoginAsync_ShouldReturnNotFound_WhenLinkedUserAccountDeleted() + { + var service = CreateService(); + var userId = Guid.NewGuid(); + var existingLogin = new ExternalLogin(userId, "GitHub", "12345"); + + _externalLoginRepoMock + .Setup(r => r.GetByProviderAsync("GitHub", "12345", default)) + .ReturnsAsync(existingLogin); + + // User no longer exists in DB + _userRepoMock.Setup(r => r.GetByIdAsync(userId, default)).ReturnsAsync((User?)null); + + var dto = new ExternalLoginDto("GitHub", "12345", "octocat", "octocat@github.com"); + var result = await service.ExternalLoginAsync(dto); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.NotFound); + } + + [Fact] + public async Task ExternalLoginAsync_ShouldReturnError_WhenAllUsernameVariantsTaken_ShortUsername() + { + // Known defect: when > 100 username variants are taken and the original + // username is short (< 18 chars), the GUID fallback generates a string + // shorter than 50 characters, causing Substring(0, 50) to throw + // ArgumentOutOfRangeException. The generic catch returns UnexpectedError. + // This documents the current behavior for future fix consideration. + var service = CreateService(); + + _externalLoginRepoMock + .Setup(r => r.GetByProviderAsync("GitHub", "99999", It.IsAny())) + .ReturnsAsync((ExternalLogin?)null); + + _userRepoMock.Setup(r => r.GetByEmailAsync("unique@example.com", It.IsAny())).ReturnsAsync((User?)null); + + // Every username variant returns a user (simulating all taken) + var takenUser = new User("taken", "taken@test.com", BCrypt.Net.BCrypt.HashPassword("p")); + _userRepoMock.Setup(r => r.GetByUsernameAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(takenUser); + + _userRepoMock.Setup(r => r.AddAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync((User u, CancellationToken _) => u); + + _externalLoginRepoMock.Setup(r => r.AddAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync((ExternalLogin l, CancellationToken _) => l); + + var dto = new ExternalLoginDto("GitHub", "99999", "popular", "unique@example.com"); + var result = await service.ExternalLoginAsync(dto); + + // Current behavior: returns UnexpectedError due to Substring overflow + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.UnexpectedError); + } + + private AuthenticationService CreateService(JwtSettings? jwtSettings = null) + { + return new AuthenticationService(_unitOfWorkMock.Object, jwtSettings ?? DefaultJwtSettings); + } +} From 34740ec733e348546fcb5fa29bbdc7ac707e9ab4 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 20:55:05 +0100 Subject: [PATCH 2/4] Add API-level auth edge case tests for controller, middleware, and OAuth code exchange Tests cover: empty/invalid/replayed/expired OAuth authorization codes, open redirect prevention, GitHub login when unconfigured, null/empty login body, account deletion during active session (TokenValidationMiddleware 401), token invalidation timestamp enforcement, re-authentication after invalidation, and missing userId claims passthrough. Linked to #707. --- .../AuthControllerEdgeCaseTests.cs | 373 ++++++++++++++++++ 1 file changed, 373 insertions(+) create mode 100644 backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs diff --git a/backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs b/backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs new file mode 100644 index 000000000..fecd0a827 --- /dev/null +++ b/backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs @@ -0,0 +1,373 @@ +using System.Collections.Concurrent; +using System.IdentityModel.Tokens.Jwt; +using System.Reflection; +using System.Security.Claims; +using System.Text.Json; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Taskdeck.Api.Contracts; +using Taskdeck.Api.Controllers; +using Taskdeck.Api.Middleware; +using Taskdeck.Application.DTOs; +using Taskdeck.Application.Interfaces; +using Taskdeck.Application.Services; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Exceptions; +using Xunit; + +namespace Taskdeck.Api.Tests; + +/// +/// Edge-case integration tests for AuthController, TokenValidationMiddleware, +/// and ActiveUserValidationMiddleware — verifying security properties around +/// OAuth flows, JWT lifecycle, and session invalidation. +/// Linked to #707 (TST-40). +/// +public class AuthControllerEdgeCaseTests +{ + private static readonly JwtSettings DefaultJwtSettings = new() + { + SecretKey = "TaskdeckTestsOnlySecretKeyMustBeLongEnough123!", + Issuer = "TaskdeckTests", + Audience = "TaskdeckUsers", + ExpirationMinutes = 60 + }; + + // ───────────────────────────────────────────────────────── + // OAuth code exchange edge cases + // ───────────────────────────────────────────────────────── + + [Fact] + public void ExchangeCode_ShouldReturn400_WhenCodeIsEmpty() + { + var controller = CreateAuthController(); + var result = controller.ExchangeCode(new ExchangeCodeRequest(string.Empty)); + + var badRequest = result.Should().BeOfType().Subject; + var error = badRequest.Value.Should().BeOfType().Subject; + error.ErrorCode.Should().Be(ErrorCodes.ValidationError); + } + + [Fact] + public void ExchangeCode_ShouldReturn401_WhenCodeIsInvalid() + { + var controller = CreateAuthController(); + var result = controller.ExchangeCode(new ExchangeCodeRequest("nonexistent-code")); + + var unauthorized = result.Should().BeOfType().Subject; + var error = unauthorized.Value.Should().BeOfType().Subject; + error.ErrorCode.Should().Be(ErrorCodes.AuthenticationFailed); + } + + [Fact] + public void ExchangeCode_ShouldPreventReplay_SecondUseOfSameCode() + { + // Insert a code into the static dictionary via reflection + var code = "test-replay-code"; + var authResult = new AuthResultDto("fake-token", new UserDto( + Guid.NewGuid(), "user", "user@test.com", + Domain.Enums.UserRole.Editor, true, + DateTimeOffset.UtcNow, DateTimeOffset.UtcNow)); + + InjectAuthCode(code, authResult, DateTimeOffset.UtcNow.AddSeconds(60)); + + var controller = CreateAuthController(); + + // First exchange — success + var first = controller.ExchangeCode(new ExchangeCodeRequest(code)); + first.Should().BeOfType(); + + // Second exchange with same code — should fail (code was consumed) + var second = controller.ExchangeCode(new ExchangeCodeRequest(code)); + second.Should().BeOfType(); + } + + [Fact] + public void ExchangeCode_ShouldReturn401_WhenCodeHasExpired() + { + var code = "test-expired-code"; + var authResult = new AuthResultDto("fake-token", new UserDto( + Guid.NewGuid(), "user", "user@test.com", + Domain.Enums.UserRole.Editor, true, + DateTimeOffset.UtcNow, DateTimeOffset.UtcNow)); + + // Inject code that expired 10 seconds ago + InjectAuthCode(code, authResult, DateTimeOffset.UtcNow.AddSeconds(-10)); + + var controller = CreateAuthController(); + var result = controller.ExchangeCode(new ExchangeCodeRequest(code)); + + // The expired code is removed by TryRemove, so the response is 401 "invalid or expired" + result.Should().BeOfType(); + } + + [Fact] + public void GetProviders_ShouldReturnGitHubStatus() + { + var controller = CreateAuthController(gitHubConfigured: true); + var result = controller.GetProviders(); + + var ok = result.Should().BeOfType().Subject; + ok.Value.Should().NotBeNull(); + } + + // ───────────────────────────────────────────────────────── + // GitHub login edge cases + // ───────────────────────────────────────────────────────── + + [Fact] + public void GitHubLogin_ShouldReturn404_WhenNotConfigured() + { + var controller = CreateAuthController(gitHubConfigured: false); + var result = controller.GitHubLogin(); + + var notFound = result.Should().BeOfType().Subject; + var error = notFound.Value.Should().BeOfType().Subject; + error.ErrorCode.Should().Be(ErrorCodes.NotFound); + } + + [Fact] + public void GitHubLogin_ShouldReturn400_WhenReturnUrlIsExternal() + { + var controller = CreateAuthController(gitHubConfigured: true); + + // The controller calls Url.IsLocalUrl which needs ActionContext setup. + // We set up a mock URL helper that rejects external URLs. + var urlHelper = new Mock(); + urlHelper.Setup(u => u.IsLocalUrl("https://evil.com/steal")).Returns(false); + controller.Url = urlHelper.Object; + + var result = controller.GitHubLogin(returnUrl: "https://evil.com/steal"); + + var badRequest = result.Should().BeOfType().Subject; + var error = badRequest.Value.Should().BeOfType().Subject; + error.ErrorCode.Should().Be(ErrorCodes.ValidationError); + error.Message.Should().Contain("Invalid return URL"); + } + + // ───────────────────────────────────────────────────────── + // TokenValidationMiddleware — account deletion during active session + // ───────────────────────────────────────────────────────── + + [Fact] + public async Task TokenValidationMiddleware_ShouldReturn401_WhenUserDeletedDuringSession() + { + var userId = Guid.NewGuid(); + var unitOfWorkMock = new Mock(); + var userRepoMock = new Mock(); + unitOfWorkMock.Setup(u => u.Users).Returns(userRepoMock.Object); + + // User is deleted — returns null + userRepoMock.Setup(r => r.GetByIdAsync(userId, It.IsAny())) + .ReturnsAsync((User?)null); + + var nextCalled = false; + RequestDelegate next = _ => { nextCalled = true; return Task.CompletedTask; }; + var middleware = new TokenValidationMiddleware(next, NullLogger.Instance); + + var context = CreateAuthenticatedContext(userId, DateTimeOffset.UtcNow); + context.Response.Body = new MemoryStream(); + + await middleware.InvokeAsync(context, unitOfWorkMock.Object); + + nextCalled.Should().BeFalse(); + context.Response.StatusCode.Should().Be(StatusCodes.Status401Unauthorized); + + var body = await ReadResponseBody(context); + body.ErrorCode.Should().Be(ErrorCodes.Unauthorized); + } + + [Fact] + public async Task TokenValidationMiddleware_ShouldReturn401_WhenTokenIssuedBeforeInvalidation() + { + var userId = Guid.NewGuid(); + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword("password")); + SetUserId(user, userId); + + // Token was issued 2 hours ago + var tokenIssuedAt = DateTimeOffset.UtcNow.AddHours(-2); + + // Invalidation happened 1 hour ago + user.InvalidateTokens(); + + var unitOfWorkMock = new Mock(); + var userRepoMock = new Mock(); + unitOfWorkMock.Setup(u => u.Users).Returns(userRepoMock.Object); + userRepoMock.Setup(r => r.GetByIdAsync(userId, It.IsAny())) + .ReturnsAsync(user); + + var nextCalled = false; + RequestDelegate next = _ => { nextCalled = true; return Task.CompletedTask; }; + var middleware = new TokenValidationMiddleware(next, NullLogger.Instance); + + var context = CreateAuthenticatedContext(userId, tokenIssuedAt); + context.Response.Body = new MemoryStream(); + + await middleware.InvokeAsync(context, unitOfWorkMock.Object); + + nextCalled.Should().BeFalse(); + context.Response.StatusCode.Should().Be(StatusCodes.Status401Unauthorized); + + var body = await ReadResponseBody(context); + body.Message.Should().Contain("invalidated"); + } + + [Fact] + public async Task TokenValidationMiddleware_ShouldPassThrough_WhenTokenIssuedAfterReauthentication() + { + // After invalidation, a freshly issued token should still work + var userId = Guid.NewGuid(); + var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword("password")); + SetUserId(user, userId); + + user.InvalidateTokens(); + + var unitOfWorkMock = new Mock(); + var userRepoMock = new Mock(); + unitOfWorkMock.Setup(u => u.Users).Returns(userRepoMock.Object); + userRepoMock.Setup(r => r.GetByIdAsync(userId, It.IsAny())) + .ReturnsAsync(user); + + var nextCalled = false; + RequestDelegate next = _ => { nextCalled = true; return Task.CompletedTask; }; + var middleware = new TokenValidationMiddleware(next, NullLogger.Instance); + + // Token issued 2 seconds after invalidation + var tokenIssuedAt = DateTimeOffset.UtcNow.AddSeconds(2); + var context = CreateAuthenticatedContext(userId, tokenIssuedAt); + + await middleware.InvokeAsync(context, unitOfWorkMock.Object); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task TokenValidationMiddleware_ShouldPassThrough_WhenClaimsHaveNoUserId() + { + // Token with authenticated identity but no parseable userId + var claims = new List { new("username", "testuser") }; + var identity = new ClaimsIdentity(claims, "Bearer"); + var principal = new ClaimsPrincipal(identity); + + var unitOfWorkMock = new Mock(); + var nextCalled = false; + RequestDelegate next = _ => { nextCalled = true; return Task.CompletedTask; }; + var middleware = new TokenValidationMiddleware(next, NullLogger.Instance); + + var context = new DefaultHttpContext { User = principal }; + + await middleware.InvokeAsync(context, unitOfWorkMock.Object); + + // Should pass through — let downstream handle it + nextCalled.Should().BeTrue(); + } + + // ───────────────────────────────────────────────────────── + // Login controller-level edge cases + // ───────────────────────────────────────────────────────── + + [Fact] + public async Task Login_ShouldReturn401_WhenBodyIsNull() + { + var authService = CreateMockAuthService(); + var controller = new AuthController(authService.Object, CreateGitHubSettings(false)); + + var result = await controller.Login(null); + + var unauthorized = result.Should().BeOfType().Subject; + var error = unauthorized.Value.Should().BeOfType().Subject; + error.ErrorCode.Should().Be(ErrorCodes.AuthenticationFailed); + } + + [Fact] + public async Task Login_ShouldReturn401_WhenFieldsEmpty() + { + var authService = CreateMockAuthService(); + var controller = new AuthController(authService.Object, CreateGitHubSettings(false)); + + var result = await controller.Login(new LoginDto("", "")); + + var unauthorized = result.Should().BeOfType().Subject; + var error = unauthorized.Value.Should().BeOfType().Subject; + error.ErrorCode.Should().Be(ErrorCodes.AuthenticationFailed); + } + + // ───────────────────────────────────────────────────────── + // Helpers + // ───────────────────────────────────────────────────────── + + private static AuthController CreateAuthController(bool gitHubConfigured = false) + { + var authServiceMock = CreateMockAuthService(); + var gitHubSettings = CreateGitHubSettings(gitHubConfigured); + return new AuthController(authServiceMock.Object, gitHubSettings); + } + + private static Mock CreateMockAuthService() + { + var unitOfWorkMock = new Mock(); + var userRepoMock = new Mock(); + unitOfWorkMock.Setup(u => u.Users).Returns(userRepoMock.Object); + unitOfWorkMock.Setup(u => u.ExternalLogins).Returns(new Mock().Object); + + // AuthenticationService is not sealed, but its constructor requires specific params + return new Mock(unitOfWorkMock.Object, DefaultJwtSettings) { CallBase = true }; + } + + private static GitHubOAuthSettings CreateGitHubSettings(bool configured) + { + return configured + ? new GitHubOAuthSettings { ClientId = "test-client", ClientSecret = "test-secret" } + : new GitHubOAuthSettings(); + } + + private static void InjectAuthCode(string code, AuthResultDto result, DateTimeOffset expiry) + { + // Access the static _authCodes field via reflection + var field = typeof(AuthController).GetField("_authCodes", + BindingFlags.NonPublic | BindingFlags.Static); + var dict = (ConcurrentDictionary)field!.GetValue(null)!; + dict[code] = (result, expiry); + } + + private static DefaultHttpContext CreateAuthenticatedContext(Guid userId, DateTimeOffset? iat) + { + var claims = new List + { + new(ClaimTypes.NameIdentifier, userId.ToString()) + }; + + if (iat.HasValue) + { + claims.Add(new Claim( + JwtRegisteredClaimNames.Iat, + iat.Value.ToUnixTimeSeconds().ToString(), + ClaimValueTypes.Integer64)); + } + + var identity = new ClaimsIdentity(claims, "Bearer"); + var principal = new ClaimsPrincipal(identity); + + return new DefaultHttpContext { User = principal }; + } + + private static void SetUserId(User user, Guid userId) + { + var idProperty = typeof(Domain.Common.Entity).GetProperty("Id"); + idProperty!.SetValue(user, userId); + } + + private static async Task ReadResponseBody(HttpContext context) + { + context.Response.Body.Seek(0, SeekOrigin.Begin); + using var reader = new StreamReader(context.Response.Body); + var json = await reader.ReadToEndAsync(); + return JsonSerializer.Deserialize(json, new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true + })!; + } +} From c99ec9fd8355aa8b43af005e64800f859f0574ca Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 21:45:36 +0100 Subject: [PATCH 3/4] Fix Substring overflow in ExternalLoginAsync GUID fallback The GUID-based username fallback used Substring(0, 50) which throws ArgumentOutOfRangeException when the original username is shorter than 17 characters. Use Math.Min(50, length) to safely truncate. --- .../src/Taskdeck.Application/Services/AuthenticationService.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/src/Taskdeck.Application/Services/AuthenticationService.cs b/backend/src/Taskdeck.Application/Services/AuthenticationService.cs index 773f227db..179f3b0ec 100644 --- a/backend/src/Taskdeck.Application/Services/AuthenticationService.cs +++ b/backend/src/Taskdeck.Application/Services/AuthenticationService.cs @@ -179,7 +179,8 @@ public async Task> ExternalLoginAsync(ExternalLoginDto dto suffix++; if (suffix > maxUsernameSuffixAttempts) { - candidateUsername = $"{normalizedUsername}-{Guid.NewGuid():N}".Substring(0, 50); + var guidFallback = $"{normalizedUsername}-{Guid.NewGuid():N}"; + candidateUsername = guidFallback[..Math.Min(50, guidFallback.Length)]; break; } candidateUsername = $"{normalizedUsername}{suffix}"; From 35d6c7b8497a8845ba23ec439f5f3e8508540869 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 21:45:43 +0100 Subject: [PATCH 4/4] Fix review findings in auth edge case tests - Update Substring overflow test to assert success after bug fix - Fix misleading comment on expired code test, add message assertion - Rename ConcurrentLogins test to SequentialLogins (tests are sequential) - Remove ActiveUserValidationMiddleware from class docstring (not tested) --- .../AuthControllerEdgeCaseTests.cs | 12 +++++++----- .../AuthenticationServiceEdgeCaseTests.cs | 18 ++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs b/backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs index fecd0a827..079e17837 100644 --- a/backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs +++ b/backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs @@ -21,9 +21,9 @@ namespace Taskdeck.Api.Tests; /// -/// Edge-case integration tests for AuthController, TokenValidationMiddleware, -/// and ActiveUserValidationMiddleware — verifying security properties around -/// OAuth flows, JWT lifecycle, and session invalidation. +/// Edge-case integration tests for AuthController and TokenValidationMiddleware, +/// verifying security properties around OAuth flows, JWT lifecycle, +/// and session invalidation. /// Linked to #707 (TST-40). /// public class AuthControllerEdgeCaseTests @@ -100,8 +100,10 @@ public void ExchangeCode_ShouldReturn401_WhenCodeHasExpired() var controller = CreateAuthController(); var result = controller.ExchangeCode(new ExchangeCodeRequest(code)); - // The expired code is removed by TryRemove, so the response is 401 "invalid or expired" - result.Should().BeOfType(); + // TryRemove succeeds (code existed), then the expiry check fires — returns "Code has expired" + var unauthorized = result.Should().BeOfType().Subject; + var error = unauthorized.Value.Should().BeOfType().Subject; + error.Message.Should().Contain("expired"); } [Fact] diff --git a/backend/tests/Taskdeck.Application.Tests/Services/AuthenticationServiceEdgeCaseTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/AuthenticationServiceEdgeCaseTests.cs index c8f1bf2dd..348f659c3 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/AuthenticationServiceEdgeCaseTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/AuthenticationServiceEdgeCaseTests.cs @@ -112,7 +112,7 @@ public async Task LoginAsync_ShouldSucceed_AfterPreviousFailedAttempt() } [Fact] - public async Task LoginAsync_ConcurrentLogins_ShouldReturnDifferentJtis() + public async Task LoginAsync_SequentialLogins_ShouldReturnDifferentJtis() { var password = "password123"; var user = new User("testuser", "test@example.com", BCrypt.Net.BCrypt.HashPassword(password)); @@ -552,13 +552,11 @@ public async Task ExternalLoginAsync_ShouldReturnNotFound_WhenLinkedUserAccountD } [Fact] - public async Task ExternalLoginAsync_ShouldReturnError_WhenAllUsernameVariantsTaken_ShortUsername() + public async Task ExternalLoginAsync_ShouldSucceedWithGuidFallback_WhenAllUsernameVariantsTaken_ShortUsername() { - // Known defect: when > 100 username variants are taken and the original - // username is short (< 18 chars), the GUID fallback generates a string - // shorter than 50 characters, causing Substring(0, 50) to throw - // ArgumentOutOfRangeException. The generic catch returns UnexpectedError. - // This documents the current behavior for future fix consideration. + // Regression test: previously Substring(0, 50) threw ArgumentOutOfRangeException + // when the GUID fallback string was shorter than 50 chars (username < 17 chars). + // Fixed to use Math.Min(50, length) so the fallback safely truncates. var service = CreateService(); _externalLoginRepoMock @@ -581,9 +579,9 @@ public async Task ExternalLoginAsync_ShouldReturnError_WhenAllUsernameVariantsTa var dto = new ExternalLoginDto("GitHub", "99999", "popular", "unique@example.com"); var result = await service.ExternalLoginAsync(dto); - // Current behavior: returns UnexpectedError due to Substring overflow - result.IsSuccess.Should().BeFalse(); - result.ErrorCode.Should().Be(ErrorCodes.UnexpectedError); + // With the fix, GUID fallback no longer throws — user is created successfully + result.IsSuccess.Should().BeTrue(); + result.Value.User.Username.Should().StartWith("popular-"); } private AuthenticationService CreateService(JwtSettings? jwtSettings = null)