From fdd8c8b37e8339635b7277dbe09b5508917a17cb Mon Sep 17 00:00:00 2001 From: Leendert de Borst Date: Wed, 2 Apr 2025 11:58:31 +0200 Subject: [PATCH] Add BadRequest handling to browser extension auth (#734) --- .../entrypoints/popup/components/Button.tsx | 2 +- .../src/entrypoints/popup/pages/Login.tsx | 36 +++++++--- .../entrypoints/popup/utils/SrpUtility.tsx | 69 +++++++++++++++++-- browser-extension/src/utils/WebApiService.ts | 56 ++++++++++----- .../src/utils/types/errors/ApiAuthError.ts | 14 ++++ .../utils/types/webapi/BadRequestResponse.ts | 9 +++ .../Main/Pages/Users/View/Index.razor | 8 +++ 7 files changed, 159 insertions(+), 35 deletions(-) create mode 100644 browser-extension/src/utils/types/errors/ApiAuthError.ts create mode 100644 browser-extension/src/utils/types/webapi/BadRequestResponse.ts diff --git a/browser-extension/src/entrypoints/popup/components/Button.tsx b/browser-extension/src/entrypoints/popup/components/Button.tsx index caabef46a..9f76b25d8 100644 --- a/browser-extension/src/entrypoints/popup/components/Button.tsx +++ b/browser-extension/src/entrypoints/popup/components/Button.tsx @@ -1,7 +1,7 @@ import React from 'react'; type ButtonProps = { - onClick: () => void; + onClick?: () => void; children: React.ReactNode; type?: 'button' | 'submit' | 'reset'; variant?: 'primary' | 'secondary'; diff --git a/browser-extension/src/entrypoints/popup/pages/Login.tsx b/browser-extension/src/entrypoints/popup/pages/Login.tsx index 408105aa6..d31d4430b 100644 --- a/browser-extension/src/entrypoints/popup/pages/Login.tsx +++ b/browser-extension/src/entrypoints/popup/pages/Login.tsx @@ -12,6 +12,8 @@ import { LoginResponse } from '../../../utils/types/webapi/Login'; import LoginServerInfo from '../components/LoginServerInfo'; import { AppInfo } from '../../../utils/AppInfo'; import { storage } from 'wxt/storage'; +import { ApiAuthError } from '../../../utils/types/errors/ApiAuthError'; + /** * Login page */ @@ -108,7 +110,7 @@ const Login: React.FC = () => { } // Try to get latest vault manually providing auth token. - const vaultResponseJson = await webApi.fetch('Vault', { method: 'GET', headers: { + const vaultResponseJson = await webApi.authFetch('Vault', { method: 'GET', headers: { 'Authorization': `Bearer ${validationResponse.token.token}` } }); @@ -130,8 +132,13 @@ const Login: React.FC = () => { // Show app. hideLoading(); - } catch { - setError('Could not reach AliasVault server. Please try again later or contact support if the problem persists.'); + } catch (err) { + // Show API authentication errors as-is. + if (err instanceof ApiAuthError) { + setError(err.message); + } else { + setError('Could not reach AliasVault server. Please try again later or contact support if the problem persists.'); + } hideLoading(); } }; @@ -143,13 +150,19 @@ const Login: React.FC = () => { e.preventDefault(); setError(null); - if (!passwordHashString || !passwordHashBase64 || !loginResponse) { - throw new Error('Required login data not found'); - } - try { showLoading(); + if (!passwordHashString || !passwordHashBase64 || !loginResponse) { + throw new Error('Required login data not found'); + } + + // Validate that 2FA code is a 6-digit number + const code = twoFactorCode.trim(); + if (!/^\d{6}$/.test(code)) { + throw new ApiAuthError('Please enter a valid 6-digit authentication code.'); + } + const validationResponse = await srpUtil.validateLogin2Fa( credentials.username, passwordHashString, @@ -164,7 +177,7 @@ const Login: React.FC = () => { } // Try to get latest vault manually providing auth token. - const vaultResponseJson = await webApi.fetch('Vault', { method: 'GET', headers: { + const vaultResponseJson = await webApi.authFetch('Vault', { method: 'GET', headers: { 'Authorization': `Bearer ${validationResponse.token.token}` } }); @@ -192,8 +205,13 @@ const Login: React.FC = () => { setLoginResponse(null); hideLoading(); } catch (err) { - setError('Invalid authentication code. Please try again.'); + // Show API authentication errors as-is. console.error('2FA error:', err); + if (err instanceof ApiAuthError) { + setError(err.message); + } else { + setError('Could not reach AliasVault server. Please try again later or contact support if the problem persists.'); + } hideLoading(); } }; diff --git a/browser-extension/src/entrypoints/popup/utils/SrpUtility.tsx b/browser-extension/src/entrypoints/popup/utils/SrpUtility.tsx index e903d5690..996bbea61 100644 --- a/browser-extension/src/entrypoints/popup/utils/SrpUtility.tsx +++ b/browser-extension/src/entrypoints/popup/utils/SrpUtility.tsx @@ -2,6 +2,8 @@ import srp from 'secure-remote-password/client' import { WebApiService } from '../../../utils/WebApiService'; import { LoginRequest, LoginResponse } from '../../../utils/types/webapi/Login'; import { ValidateLoginRequest, ValidateLoginRequest2Fa, ValidateLoginResponse } from '../../../utils/types/webapi/ValidateLogin'; +import BadRequestResponse from '@/utils/types/webapi/BadRequestResponse'; +import { ApiAuthError } from '../../../utils/types/errors/ApiAuthError'; /** * Utility class for SRP authentication operations. @@ -22,9 +24,27 @@ class SrpUtility { * Initiate login with server. */ public async initiateLogin(username: string): Promise { - return this.webApiService.post('Auth/login', { - username: username.toLowerCase().trim() + const model: LoginRequest = { + username: username.toLowerCase().trim(), + }; + + const response = await this.webApiService.rawFetch('Auth/login', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(model), }); + + // Check if response is a bad request (400) + if (response.status === 400) { + const badRequestResponse = await response.json() as BadRequestResponse; + throw new ApiAuthError(badRequestResponse.title); + } + + // For other responses, try to parse as LoginResponse + const loginResponse = await response.json() as LoginResponse; + return loginResponse; } /** @@ -51,12 +71,30 @@ class SrpUtility { privateKey ); - return this.webApiService.post('Auth/validate', { + const model: ValidateLoginRequest = { username: username.toLowerCase().trim(), rememberMe: rememberMe, clientPublicEphemeral: clientEphemeral.public, clientSessionProof: sessionProof.proof, + }; + + const response = await this.webApiService.rawFetch('Auth/validate', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(model), }); + + // Check if response is a bad request (400) + if (response.status === 400) { + const badRequestResponse = await response.json() as BadRequestResponse; + throw new ApiAuthError(badRequestResponse.title); + } + + // For other responses, try to parse as ValidateLoginResponse + const validateLoginResponse = await response.json() as ValidateLoginResponse; + return validateLoginResponse; } /** @@ -83,14 +121,31 @@ class SrpUtility { username, privateKey ); - - return this.webApiService.post('Auth/validate-2fa', { + const model: ValidateLoginRequest2Fa = { username: username.toLowerCase().trim(), - rememberMe: rememberMe, + rememberMe, clientPublicEphemeral: clientEphemeral.public, clientSessionProof: sessionProof.proof, - code2Fa: code2Fa, + code2Fa, + }; + + const response = await this.webApiService.rawFetch('Auth/validate-2fa', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(model), }); + + // Check if response is a bad request (400) + if (response.status === 400) { + const badRequestResponse = await response.json() as BadRequestResponse; + throw new ApiAuthError(badRequestResponse.title); + } + + // For other responses, try to parse as ValidateLoginResponse + const validateLoginResponse = await response.json() as ValidateLoginResponse; + return validateLoginResponse; } } diff --git a/browser-extension/src/utils/WebApiService.ts b/browser-extension/src/utils/WebApiService.ts index 673f6a6f8..431b9fec7 100644 --- a/browser-extension/src/utils/WebApiService.ts +++ b/browser-extension/src/utils/WebApiService.ts @@ -37,15 +37,13 @@ export class WebApiService { } /** - * Fetch data from the API. + * Fetch data from the API with authentication headers and access token refresh retry. */ - public async fetch( + public async authFetch( endpoint: string, options: RequestInit = {}, parseJson: boolean = true ): Promise { - const baseUrl = await this.getBaseUrl(); - const url = baseUrl + endpoint; const headers = new Headers(options.headers ?? {}); // Add authorization header if we have an access token @@ -54,22 +52,19 @@ export class WebApiService { headers.set('Authorization', `Bearer ${accessToken}`); } - // Add client version header - headers.set('X-AliasVault-Client', `${AppInfo.CLIENT_NAME}-${AppInfo.VERSION}`); - const requestOptions: RequestInit = { ...options, headers, }; try { - const response = await fetch(url, requestOptions); + const response = await this.rawFetch(endpoint, requestOptions); if (response.status === 401) { const newToken = await this.refreshAccessToken(); if (newToken) { headers.set('Authorization', `Bearer ${newToken}`); - const retryResponse = await fetch(url, { + const retryResponse = await this.rawFetch(endpoint, { ...requestOptions, headers, }); @@ -96,6 +91,34 @@ export class WebApiService { } } + /** + * Fetch data from the API without authentication headers and without access token refresh retry. + */ + public async rawFetch( + endpoint: string, + options: RequestInit = {} + ): Promise { + const baseUrl = await this.getBaseUrl(); + const url = baseUrl + endpoint; + const headers = new Headers(options.headers ?? {}); + + // Add client version header + headers.set('X-AliasVault-Client', `${AppInfo.CLIENT_NAME}-${AppInfo.VERSION}`); + + const requestOptions: RequestInit = { + ...options, + headers, + }; + + try { + const response = await fetch(url, requestOptions); + return response; + } catch (error) { + console.error('API request failed:', error); + throw error; + } + } + /** * Refresh the access token. */ @@ -106,14 +129,11 @@ export class WebApiService { } try { - const baseUrl = await this.getBaseUrl(); - - const response = await fetch(`${baseUrl}Auth/refresh`, { + const response = await this.rawFetch('Auth/refresh', { method: 'POST', headers: { 'Content-Type': 'application/json', 'X-Ignore-Failure': 'true', - 'X-AliasVault-Client': `${AppInfo.CLIENT_NAME}-${AppInfo.VERSION}`, }, body: JSON.stringify({ token: await this.getAccessToken(), @@ -138,7 +158,7 @@ export class WebApiService { * Issue GET request to the API. */ public async get(endpoint: string): Promise { - return this.fetch(endpoint, { method: 'GET' }); + return this.authFetch(endpoint, { method: 'GET' }); } /** @@ -146,7 +166,7 @@ export class WebApiService { */ public async downloadBlobAndConvertToBase64(endpoint: string): Promise { try { - const response = await this.fetch(endpoint, { + const response = await this.authFetch(endpoint, { method: 'GET', headers: { 'Accept': 'application/octet-stream', @@ -170,7 +190,7 @@ export class WebApiService { data: TRequest, parseJson: boolean = true ): Promise { - return this.fetch(endpoint, { + return this.authFetch(endpoint, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -183,7 +203,7 @@ export class WebApiService { * Issue PUT request to the API. */ public async put(endpoint: string, data: TRequest): Promise { - return this.fetch(endpoint, { + return this.authFetch(endpoint, { method: 'PUT', headers: { 'Content-Type': 'application/json', @@ -196,7 +216,7 @@ export class WebApiService { * Issue DELETE request to the API. */ public async delete(endpoint: string): Promise { - return this.fetch(endpoint, { method: 'DELETE' }, false); + return this.authFetch(endpoint, { method: 'DELETE' }, false); } /** diff --git a/browser-extension/src/utils/types/errors/ApiAuthError.ts b/browser-extension/src/utils/types/errors/ApiAuthError.ts new file mode 100644 index 000000000..83b91b46a --- /dev/null +++ b/browser-extension/src/utils/types/errors/ApiAuthError.ts @@ -0,0 +1,14 @@ +/** + * Custom error class for API authentication-related errors. + */ +export class ApiAuthError extends Error { + /** + * Creates a new instance of ApiAuthError. + * + * @param message - The error message. + */ + public constructor(message: string) { + super(message); + this.name = 'ApiAuthError'; + } +} diff --git a/browser-extension/src/utils/types/webapi/BadRequestResponse.ts b/browser-extension/src/utils/types/webapi/BadRequestResponse.ts new file mode 100644 index 000000000..cc4598f7d --- /dev/null +++ b/browser-extension/src/utils/types/webapi/BadRequestResponse.ts @@ -0,0 +1,9 @@ +type BadRequestResponse = { + type: string; + title: string; + status: number; + errors: Record; + traceId: string; +}; + +export default BadRequestResponse; diff --git a/src/AliasVault.Admin/Main/Pages/Users/View/Index.razor b/src/AliasVault.Admin/Main/Pages/Users/View/Index.razor index 93fc3cb98..f389b78f0 100644 --- a/src/AliasVault.Admin/Main/Pages/Users/View/Index.razor +++ b/src/AliasVault.Admin/Main/Pages/Users/View/Index.razor @@ -326,6 +326,14 @@ Do you want to proceed with the restoration?")) { if (User != null) { User.Blocked = !User.Blocked; + + // If user is unblocked by the admin, also reset any lockout status, which can be + // automatically triggered by the system when user has entered an incorrect password too many times. + if (!User.Blocked) { + User.AccessFailedCount = 0; + User.LockoutEnd = null; + } + await dbContext.SaveChangesAsync(); await RefreshData(); }