-
Notifications
You must be signed in to change notification settings - Fork 0
TST-40: OAuth and authentication edge case tests #737
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
8002b41
Add auth service edge case tests for login, registration, token valid…
Chris0Jeky 34740ec
Add API-level auth edge case tests for controller, middleware, and OA…
Chris0Jeky c99ec9f
Fix Substring overflow in ExternalLoginAsync GUID fallback
Chris0Jeky 35d6c7b
Fix review findings in auth edge case tests
Chris0Jeky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
375 changes: 375 additions & 0 deletions
375
backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,375 @@ | ||||||
| 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; | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// Edge-case integration tests for AuthController and TokenValidationMiddleware, | ||||||
| /// verifying security properties around OAuth flows, JWT lifecycle, | ||||||
| /// and session invalidation. | ||||||
| /// Linked to #707 (TST-40). | ||||||
| /// </summary> | ||||||
| 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<BadRequestObjectResult>().Subject; | ||||||
| var error = badRequest.Value.Should().BeOfType<ApiErrorResponse>().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<UnauthorizedObjectResult>().Subject; | ||||||
| var error = unauthorized.Value.Should().BeOfType<ApiErrorResponse>().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<OkObjectResult>(); | ||||||
|
|
||||||
| // Second exchange with same code — should fail (code was consumed) | ||||||
| var second = controller.ExchangeCode(new ExchangeCodeRequest(code)); | ||||||
| second.Should().BeOfType<UnauthorizedObjectResult>(); | ||||||
| } | ||||||
|
|
||||||
| [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)); | ||||||
|
|
||||||
| // TryRemove succeeds (code existed), then the expiry check fires — returns "Code has expired" | ||||||
| var unauthorized = result.Should().BeOfType<UnauthorizedObjectResult>().Subject; | ||||||
| var error = unauthorized.Value.Should().BeOfType<ApiErrorResponse>().Subject; | ||||||
| error.Message.Should().Contain("expired"); | ||||||
| } | ||||||
|
|
||||||
| [Fact] | ||||||
| public void GetProviders_ShouldReturnGitHubStatus() | ||||||
| { | ||||||
| var controller = CreateAuthController(gitHubConfigured: true); | ||||||
| var result = controller.GetProviders(); | ||||||
|
|
||||||
| var ok = result.Should().BeOfType<OkObjectResult>().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<NotFoundObjectResult>().Subject; | ||||||
| var error = notFound.Value.Should().BeOfType<ApiErrorResponse>().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<IUrlHelper>(); | ||||||
| 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<BadRequestObjectResult>().Subject; | ||||||
| var error = badRequest.Value.Should().BeOfType<ApiErrorResponse>().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<IUnitOfWork>(); | ||||||
| var userRepoMock = new Mock<IUserRepository>(); | ||||||
| unitOfWorkMock.Setup(u => u.Users).Returns(userRepoMock.Object); | ||||||
|
|
||||||
| // User is deleted — returns null | ||||||
| userRepoMock.Setup(r => r.GetByIdAsync(userId, It.IsAny<CancellationToken>())) | ||||||
| .ReturnsAsync((User?)null); | ||||||
|
|
||||||
| var nextCalled = false; | ||||||
| RequestDelegate next = _ => { nextCalled = true; return Task.CompletedTask; }; | ||||||
| var middleware = new TokenValidationMiddleware(next, NullLogger<TokenValidationMiddleware>.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<IUnitOfWork>(); | ||||||
| var userRepoMock = new Mock<IUserRepository>(); | ||||||
| unitOfWorkMock.Setup(u => u.Users).Returns(userRepoMock.Object); | ||||||
| userRepoMock.Setup(r => r.GetByIdAsync(userId, It.IsAny<CancellationToken>())) | ||||||
| .ReturnsAsync(user); | ||||||
|
|
||||||
| var nextCalled = false; | ||||||
| RequestDelegate next = _ => { nextCalled = true; return Task.CompletedTask; }; | ||||||
| var middleware = new TokenValidationMiddleware(next, NullLogger<TokenValidationMiddleware>.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<IUnitOfWork>(); | ||||||
| var userRepoMock = new Mock<IUserRepository>(); | ||||||
| unitOfWorkMock.Setup(u => u.Users).Returns(userRepoMock.Object); | ||||||
| userRepoMock.Setup(r => r.GetByIdAsync(userId, It.IsAny<CancellationToken>())) | ||||||
| .ReturnsAsync(user); | ||||||
|
|
||||||
| var nextCalled = false; | ||||||
| RequestDelegate next = _ => { nextCalled = true; return Task.CompletedTask; }; | ||||||
| var middleware = new TokenValidationMiddleware(next, NullLogger<TokenValidationMiddleware>.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<Claim> { new("username", "testuser") }; | ||||||
| var identity = new ClaimsIdentity(claims, "Bearer"); | ||||||
| var principal = new ClaimsPrincipal(identity); | ||||||
|
|
||||||
| var unitOfWorkMock = new Mock<IUnitOfWork>(); | ||||||
| var nextCalled = false; | ||||||
| RequestDelegate next = _ => { nextCalled = true; return Task.CompletedTask; }; | ||||||
| var middleware = new TokenValidationMiddleware(next, NullLogger<TokenValidationMiddleware>.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<UnauthorizedObjectResult>().Subject; | ||||||
| var error = unauthorized.Value.Should().BeOfType<ApiErrorResponse>().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<UnauthorizedObjectResult>().Subject; | ||||||
| var error = unauthorized.Value.Should().BeOfType<ApiErrorResponse>().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<AuthenticationService> CreateMockAuthService() | ||||||
| { | ||||||
| var unitOfWorkMock = new Mock<IUnitOfWork>(); | ||||||
| var userRepoMock = new Mock<IUserRepository>(); | ||||||
| unitOfWorkMock.Setup(u => u.Users).Returns(userRepoMock.Object); | ||||||
| unitOfWorkMock.Setup(u => u.ExternalLogins).Returns(new Mock<IExternalLoginRepository>().Object); | ||||||
|
|
||||||
| // AuthenticationService is not sealed, but its constructor requires specific params | ||||||
| return new Mock<AuthenticationService>(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<string, (AuthResultDto Result, DateTimeOffset Expiry)>)field!.GetValue(null)!; | ||||||
| dict[code] = (result, expiry); | ||||||
| } | ||||||
|
|
||||||
| private static DefaultHttpContext CreateAuthenticatedContext(Guid userId, DateTimeOffset? iat) | ||||||
| { | ||||||
| var claims = new List<Claim> | ||||||
| { | ||||||
| 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"); | ||||||
|
||||||
| var idProperty = typeof(Domain.Common.Entity).GetProperty("Id"); | |
| var idProperty = typeof(Taskdeck.Domain.Common.Entity).GetProperty("Id"); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines reference
Domain.Enums.UserRole, but this test file doesn't define aDomainalias/namespace (the enum isTaskdeck.Domain.Enums.UserRole). As written, the file won't compile. Update the references to the correct namespace (or add an explicit alias) so the tests build.