Merge pull request #812 from Chris0Jeky/feature/oauth-pkce-account-linking

CLD-03 follow-up: DB-backed auth codes, PKCE, account linking

Author: Chris0Jeky

backend/src/Taskdeck.Api/Controllers/AuthController.cs

@@ -98,6 +98,11 @@ await context.Response.WriteAsJsonAsync(new ApiErrorResponse(
                 options.CallbackPath = "/api/auth/github/oauth-redirect";
                 options.SaveTokens = false;
 
+                // PKCE (Proof Key for Code Exchange) — defense-in-depth against
+                // authorization code interception attacks. GitHub supports PKCE
+                // and ASP.NET Core 8+ handles code_verifier/code_challenge automatically.
+                options.UsePkce = true;
+
                 options.Scope.Add("read:user");
                 options.Scope.Add("user:email");
 

backend/src/Taskdeck.Api/Extensions/AuthenticationRegistration.cs

@@ -37,3 +37,10 @@ public record ExternalLoginDto(
     string Email,
     string? DisplayName = null,
     string? AvatarUrl = null);
+
+public record LinkedAccountDto(
+    string Provider,
+    string ProviderUserId,
+    string? DisplayName,
+    string? AvatarUrl,
+    DateTimeOffset LinkedAt);

backend/src/Taskdeck.Application/DTOs/UserDtos.cs

@@ -0,0 +1,24 @@
+using Taskdeck.Domain.Entities;
+
+namespace Taskdeck.Application.Interfaces;
+
+public interface IOAuthAuthCodeRepository : IRepository<OAuthAuthCode>
+{
+    /// <summary>
+    /// Finds an auth code by its code string. Returns null if not found.
+    /// </summary>
+    Task<OAuthAuthCode?> GetByCodeAsync(string code, CancellationToken cancellationToken = default);
+
+    /// <summary>
+    /// Atomically consumes an auth code by setting IsConsumed = true where IsConsumed = false.
+    /// Returns true if the code was consumed (affected 1 row), false if already consumed or not found.
+    /// This provides single-use semantics safe for concurrent requests.
+    /// </summary>
+    Task<bool> TryConsumeAtomicAsync(string code, CancellationToken cancellationToken = default);
+
+    /// <summary>
+    /// Deletes all auth codes that have expired before the specified cutoff time.
+    /// Returns the number of codes removed.
+    /// </summary>
+    Task<int> DeleteExpiredAsync(DateTimeOffset cutoff, CancellationToken cancellationToken = default);
+}

backend/src/Taskdeck.Application/Interfaces/IOAuthAuthCodeRepository.cs

@@ -27,6 +27,7 @@ public interface IUnitOfWork
     IKnowledgeDocumentRepository KnowledgeDocuments { get; }
     IKnowledgeChunkRepository KnowledgeChunks { get; }
     IExternalLoginRepository ExternalLogins { get; }
+    IOAuthAuthCodeRepository OAuthAuthCodes { get; }
     IApiKeyRepository ApiKeys { get; }
 
     Task<int> SaveChangesAsync(CancellationToken cancellationToken = default);

backend/src/Taskdeck.Application/Interfaces/IUnitOfWork.cs

@@ -209,6 +209,83 @@ public async Task<Result<AuthResultDto>> ExternalLoginAsync(ExternalLoginDto dto
         }
     }
 
+    public async Task<Result<LinkedAccountDto>> CompleteAccountLinkAsync(Guid userId, string provider, string providerUserId, string? displayName, string? avatarUrl)
+    {
+        try
+        {
+            if (userId == Guid.Empty)
+                return Result.Failure<LinkedAccountDto>(ErrorCodes.ValidationError, "User ID is required");
+
+            var user = await _unitOfWork.Users.GetByIdAsync(userId);
+            if (user == null)
+                return Result.Failure<LinkedAccountDto>(ErrorCodes.NotFound, "User not found");
+
+            if (!user.IsActive)
+                return Result.Failure<LinkedAccountDto>(ErrorCodes.Forbidden, "User account is inactive");
+
+            // Check if this provider+userId combo is already linked to a different user
+            var existingLogin = await _unitOfWork.ExternalLogins.GetByProviderAsync(provider, providerUserId);
+            if (existingLogin != null)
+            {
+                if (existingLogin.UserId == userId)
+                    return Result.Failure<LinkedAccountDto>(ErrorCodes.Conflict, $"This {provider} account is already linked to your account");
+
+                return Result.Failure<LinkedAccountDto>(ErrorCodes.Conflict, $"This {provider} account is already linked to a different user");
+            }
+
+            // Check if user already has a linked account for this provider
+            var userLogins = await _unitOfWork.ExternalLogins.GetByUserIdAsync(userId);
+            if (userLogins.Any(l => l.Provider == provider))
+                return Result.Failure<LinkedAccountDto>(ErrorCodes.Conflict, $"Your account is already linked to a {provider} account");
+
+            var newLogin = new ExternalLogin(userId, provider, providerUserId, displayName, avatarUrl);
+            await _unitOfWork.ExternalLogins.AddAsync(newLogin);
+            await _unitOfWork.SaveChangesAsync();
+
+            return Result.Success(new LinkedAccountDto(
+                newLogin.Provider,
+                newLogin.ProviderUserId,
+                newLogin.ProviderDisplayName,
+                newLogin.AvatarUrl,
+                newLogin.CreatedAt));
+        }
+        catch (DomainException ex)
+        {
+            return Result.Failure<LinkedAccountDto>(ex.ErrorCode, ex.Message);
+        }
+        catch (Exception)
+        {
+            return Result.Failure<LinkedAccountDto>(ErrorCodes.UnexpectedError, "Account linking failed due to an unexpected error");
+        }
+    }
+
+    public async Task<Result> UnlinkExternalLoginAsync(Guid userId, string provider)
+    {
+        try
+        {
+            if (userId == Guid.Empty)
+                return Result.Failure(ErrorCodes.ValidationError, "User ID is required");
+
+            var user = await _unitOfWork.Users.GetByIdAsync(userId);
+            if (user == null)
+                return Result.Failure(ErrorCodes.NotFound, "User not found");
+
+            var logins = await _unitOfWork.ExternalLogins.GetByUserIdAsync(userId);
+            var loginToRemove = logins.FirstOrDefault(l => l.Provider == provider);
+            if (loginToRemove == null)
+                return Result.Failure(ErrorCodes.NotFound, $"No {provider} account is linked");
+
+            await _unitOfWork.ExternalLogins.DeleteAsync(loginToRemove);
+            await _unitOfWork.SaveChangesAsync();
+
+            return Result.Success();
+        }
+        catch (DomainException ex)
+        {
+            return Result.Failure(ex.ErrorCode, ex.Message);
+        }
+    }
+
     public async Task<Result> ChangePasswordAsync(Guid userId, string currentPassword, string newPassword)
     {
         try
@@ -290,7 +367,7 @@ public async Task<Result<UserDto>> ValidateTokenAsync(string token)
         }
     }
 
-    private string GenerateJwtToken(User user)
+    public string GenerateJwtToken(User user)
     {
         if (!TryValidateJwtSettings(out var jwtValidationError))
             throw new DomainException(ErrorCodes.UnexpectedError, jwtValidationError);

backend/src/Taskdeck.Application/Services/AuthenticationService.cs

@@ -14,4 +14,6 @@ public interface IAuthenticationService
     Task<Result> ChangePasswordAsync(Guid userId, string currentPassword, string newPassword);
     Task<Result<UserDto>> ValidateTokenAsync(string token);
     Task<Result<AuthResultDto>> ExternalLoginAsync(ExternalLoginDto dto);
+    Task<Result<LinkedAccountDto>> CompleteAccountLinkAsync(Guid userId, string provider, string providerUserId, string? displayName, string? avatarUrl);
+    Task<Result> UnlinkExternalLoginAsync(Guid userId, string provider);
 }

backend/src/Taskdeck.Application/Services/IAuthenticationService.cs

@@ -0,0 +1,144 @@
+using Taskdeck.Domain.Common;
+using Taskdeck.Domain.Exceptions;
+
+namespace Taskdeck.Domain.Entities;
+
+/// <summary>
+/// A short-lived, single-use authorization code issued after OAuth callback.
+/// Stored in the database to survive restarts and support multi-instance deployments.
+/// Supports both login (Token populated) and account-linking (ProviderData populated) flows.
+/// </summary>
+public class OAuthAuthCode : Entity
+{
+    private string _code = string.Empty;
+
+    public string Code
+    {
+        get => _code;
+        private set
+        {
+            if (string.IsNullOrWhiteSpace(value))
+                throw new DomainException(ErrorCodes.ValidationError, "Auth code cannot be empty");
+
+            if (value.Length > 512)
+                throw new DomainException(ErrorCodes.ValidationError, "Auth code cannot exceed 512 characters");
+
+            _code = value;
+        }
+    }
+
+    /// <summary>
+    /// The user ID this code authenticates (login flow) or Guid.Empty (link flow).
+    /// </summary>
+    public Guid UserId { get; private set; }
+
+    /// <summary>
+    /// Legacy field kept for schema compatibility. No longer populated with real JWTs --
+    /// tokens are re-issued at exchange time from the stored UserId.
+    /// </summary>
+    public string Token { get; private set; } = string.Empty;
+
+    /// <summary>
+    /// The purpose of this code: "login" or "link".
+    /// </summary>
+    public string Purpose { get; private set; } = "login";
+
+    /// <summary>
+    /// JSON-serialized provider identity data for account linking flows.
+    /// Contains provider, providerUserId, displayName, avatarUrl.
+    /// </summary>
+    public string? ProviderData { get; private set; }
+
+    /// <summary>
+    /// When this code expires and can no longer be exchanged.
+    /// </summary>
+    public DateTimeOffset ExpiresAt { get; private set; }
+
+    /// <summary>
+    /// Whether this code has been consumed (exchanged for a token).
+    /// </summary>
+    public bool IsConsumed { get; private set; }
+
+    /// <summary>
+    /// When the code was consumed, if applicable.
+    /// </summary>
+    public DateTimeOffset? ConsumedAt { get; private set; }
+
+    private OAuthAuthCode() : base() { }
+
+    /// <summary>
+    /// Creates an auth code for the login flow (token exchange).
+    /// Token parameter is accepted for backward compatibility but is no longer stored;
+    /// JWTs are re-issued at exchange time from the UserId.
+    /// </summary>
+    public OAuthAuthCode(string code, Guid userId, string token, DateTimeOffset expiresAt)
+        : base()
+    {
+        if (userId == Guid.Empty)
+            throw new DomainException(ErrorCodes.ValidationError, "User ID cannot be empty");
+
+        if (string.IsNullOrWhiteSpace(token))
+            throw new DomainException(ErrorCodes.ValidationError, "Token cannot be empty");
+
+        if (expiresAt <= DateTimeOffset.UtcNow)
+            throw new DomainException(ErrorCodes.ValidationError, "Expiry must be in the future");
+
+        Code = code;
+        UserId = userId;
+        Token = string.Empty; // Never store actual JWT in DB — re-issue at exchange time
+        Purpose = "login";
+        ExpiresAt = expiresAt;
+    }
+
+    /// <summary>
+    /// Creates an auth code for the account linking flow (provider identity exchange).
+    /// The initiatingUserId binds this code to the user who started the link flow,
+    /// preventing CSRF attacks where an attacker's GitHub is linked to a victim's account.
+    /// </summary>
+    public static OAuthAuthCode CreateForLinking(string code, Guid initiatingUserId, string providerData, DateTimeOffset expiresAt)
+    {
+        if (initiatingUserId == Guid.Empty)
+            throw new DomainException(ErrorCodes.ValidationError, "Initiating user ID is required for linking");
+
+        if (string.IsNullOrWhiteSpace(providerData))
+            throw new DomainException(ErrorCodes.ValidationError, "Provider data cannot be empty for linking");
+
+        if (expiresAt <= DateTimeOffset.UtcNow)
+            throw new DomainException(ErrorCodes.ValidationError, "Expiry must be in the future");
+
+        var entity = new OAuthAuthCode
+        {
+            Code = code,
+            UserId = initiatingUserId,
+            Token = string.Empty,
+            Purpose = "link",
+            ProviderData = providerData,
+            ExpiresAt = expiresAt
+        };
+        return entity;
+    }
+
+    /// <summary>
+    /// Returns true if this code has expired.
+    /// </summary>
+    public bool IsExpired => DateTimeOffset.UtcNow > ExpiresAt;
+
+    /// <summary>
+    /// Returns true if this is a linking code (not a login code).
+    /// </summary>
+    public bool IsLinkingCode => Purpose == "link";
+
+    /// <summary>
+    /// Attempts to consume this code. Returns false if already consumed or expired.
+    /// </summary>
+    public bool TryConsume()
+    {
+        if (IsConsumed || IsExpired)
+            return false;
+
+        IsConsumed = true;
+        ConsumedAt = DateTimeOffset.UtcNow;
+        Touch();
+        return true;
+    }
+}

backend/src/Taskdeck.Domain/Entities/OAuthAuthCode.cs

@@ -43,6 +43,7 @@ public static IServiceCollection AddInfrastructure(this IServiceCollection servi
         services.AddScoped<IKnowledgeDocumentRepository, KnowledgeDocumentRepository>();
         services.AddScoped<IKnowledgeChunkRepository, KnowledgeChunkRepository>();
         services.AddScoped<IExternalLoginRepository, ExternalLoginRepository>();
+        services.AddScoped<IOAuthAuthCodeRepository, OAuthAuthCodeRepository>();
         services.AddScoped<IApiKeyRepository, ApiKeyRepository>();
         services.AddScoped<IKnowledgeSearchService, Taskdeck.Infrastructure.Services.KnowledgeFtsSearchService>();
         services.AddScoped<IUnitOfWork, UnitOfWork>();