From b4b2dc3fe7ada7ee53b3087d5b9ebb8a2768d7ca Mon Sep 17 00:00:00 2001 From: Leendert de Borst Date: Thu, 21 Nov 2024 19:26:16 +0100 Subject: [PATCH] Fix refresh token revoke bug (#363) --- .../Controllers/AuthController.cs | 28 +++++++++++++------ .../Components/QuickVaultUnlockSection.razor | 2 +- .../Services/Auth/AuthService.cs | 4 +-- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/AliasVault.Api/Controllers/AuthController.cs b/src/AliasVault.Api/Controllers/AuthController.cs index 731e2d489..3fe3ca8a0 100644 --- a/src/AliasVault.Api/Controllers/AuthController.cs +++ b/src/AliasVault.Api/Controllers/AuthController.cs @@ -137,8 +137,6 @@ public class AuthController(IDbContextFactory dbContextFac return Ok(new ValidateLoginResponse(true, string.Empty, null)); } - // If 2FA is not required, then it means the user is successfully authenticated at this point. - // Reset failed login attempts. await userManager.ResetAccessFailedCountAsync(user); @@ -284,6 +282,12 @@ public class AuthController(IDbContextFactory dbContextFac { await using var context = await dbContextFactory.CreateDbContextAsync(); + // If the token is not provided, return bad request. + if (string.IsNullOrWhiteSpace(model.RefreshToken)) + { + return BadRequest("Refresh token is required."); + } + var principal = GetPrincipalFromToken(model.Token); if (principal.FindFirst(ClaimTypes.NameIdentifier)?.Value == null) { @@ -297,16 +301,18 @@ public class AuthController(IDbContextFactory dbContextFac } // Check if the refresh token is valid. - var deviceIdentifier = GenerateDeviceIdentifier(Request); - var existingToken = await context.AliasVaultUserRefreshTokens.FirstOrDefaultAsync(t => t.UserId == user.Id && t.DeviceIdentifier == deviceIdentifier); - if (existingToken == null || existingToken.Value != model.RefreshToken) + var providedTokenExists = await context.AliasVaultUserRefreshTokens.AnyAsync(t => t.UserId == user.Id && t.Value == model.RefreshToken); + if (!providedTokenExists) { await authLoggingService.LogAuthEventFailAsync(user.UserName!, AuthEventType.Logout, AuthFailureReason.InvalidRefreshToken); return Unauthorized("Invalid refresh token"); } - // Remove the existing refresh token. - context.AliasVaultUserRefreshTokens.Remove(existingToken); + // Remove the provided refresh token and any other existing refresh tokens that are issued to the current device ID. + // This to make sure all tokens are revoked for this device that user is "logging out" from. + var deviceIdentifier = GenerateDeviceIdentifier(Request); + var allDeviceTokens = await context.AliasVaultUserRefreshTokens.Where(t => t.UserId == user.Id && (t.Value == model.RefreshToken || t.DeviceIdentifier == deviceIdentifier)).ToListAsync(); + context.AliasVaultUserRefreshTokens.RemoveRange(allDeviceTokens); await context.SaveChangesAsync(); await authLoggingService.LogAuthEventSuccessAsync(user.UserName!, AuthEventType.Logout); @@ -705,8 +711,14 @@ public class AuthController(IDbContextFactory dbContextFac // New refresh token lifetime is the same as the existing one. var existingTokenLifetime = existingToken.ExpireDate - existingToken.CreatedAt; + // Retrieve new refresh token. + var newRefreshToken = await GenerateRefreshToken(user, existingTokenLifetime, existingToken.Value); + + // After successfully retrieving new refresh token, remove the existing one by saving changes. + await context.SaveChangesAsync(); + // Return new refresh token. - return await GenerateRefreshToken(user, existingTokenLifetime, existingToken.Value); + return newRefreshToken; } finally { diff --git a/src/AliasVault.Client/Main/Pages/Settings/Security/Components/QuickVaultUnlockSection.razor b/src/AliasVault.Client/Main/Pages/Settings/Security/Components/QuickVaultUnlockSection.razor index cfbfb469c..1441340ef 100644 --- a/src/AliasVault.Client/Main/Pages/Settings/Security/Components/QuickVaultUnlockSection.razor +++ b/src/AliasVault.Client/Main/Pages/Settings/Security/Components/QuickVaultUnlockSection.razor @@ -85,7 +85,7 @@ /// public async Task DisableWebAuthn() { - await AuthService.SetWebAuthnEnabledAsync(false, string.Empty, string.Empty, string.Empty); + await AuthService.SetWebAuthnEnabledAsync(false); GlobalNotificationService.AddSuccessMessage("Quick Vault Unlock is successfully disabled.", true); await LoadData(); } diff --git a/src/AliasVault.Client/Services/Auth/AuthService.cs b/src/AliasVault.Client/Services/Auth/AuthService.cs index f8f6fbbde..641a5acdb 100644 --- a/src/AliasVault.Client/Services/Auth/AuthService.cs +++ b/src/AliasVault.Client/Services/Auth/AuthService.cs @@ -194,7 +194,7 @@ public sealed class AuthService(HttpClient httpClient, ILocalStorageService loca /// WebAuthn salt. /// WebAuthn credential derived key. /// Task. - public async Task SetWebAuthnEnabledAsync(bool enabled, string? webauthCredentialId, string? webauthSalt, string? webauthCredentialDerivedKey) + public async Task SetWebAuthnEnabledAsync(bool enabled, string? webauthCredentialId = null, string? webauthSalt = null, string? webauthCredentialDerivedKey = null) { await localStorage.SetItemAsStringAsync("webAuthnEnabled", enabled.ToString().ToLower()); @@ -311,7 +311,7 @@ public sealed class AuthService(HttpClient httpClient, ILocalStorageService loca private async Task RevokeTokenAsync() { // Remove webauthn enabled flag. - await SetWebAuthnEnabledAsync(false, null, null, null); + await SetWebAuthnEnabledAsync(false); var tokenInput = new TokenModel {