From bc3c9e3d2329b85cd86969aeba6df9db7dc0ac53 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Mon, 26 Oct 2026 02:58:55 +0100 Subject: [PATCH] fix: invalid-credentials code, no refresh on wrong pw --- crates/adapters/http-axum/src/errors.rs | 8 +++++++- .../src/users/request_account_deletion.rs | 2 +- .../tests/users/request_account_deletion_test.rs | 10 ++++++++-- crates/bootstrap/tests/account_deletion_test.rs | 4 +++- crates/domain/src/errors/mod.rs | 3 +++ .../adr/0014-http-adapter-and-one-refusal-shape.md | 3 ++- spa/src/components/delete-account-dialog.tsx | 1 + spa/src/features/users.ts | 5 ++++- spa/src/lib/api/client.ts | 14 ++++++++++++++ spa/src/locales/en.json | 2 +- 10 files changed, 44 insertions(+), 8 deletions(-) diff --git a/crates/adapters/http-axum/src/errors.rs b/crates/adapters/http-axum/src/errors.rs index 16f9fa29..d9b6b44e 100644 --- a/crates/adapters/http-axum/src/errors.rs +++ b/crates/adapters/http-axum/src/errors.rs @@ -8,6 +8,7 @@ use domain::errors::DomainError; pub const NOT_FOUND: &str = "NOT_FOUND"; pub const INVALID_INPUT: &str = "INVALID_INPUT"; pub const UNAUTHORIZED: &str = "UNAUTHORIZED"; +pub const INVALID_CREDENTIALS: &str = "INVALID_CREDENTIALS"; pub const FORBIDDEN: &str = "FORBIDDEN"; pub const CONFLICT: &str = "CONFLICT"; pub const RATE_LIMITED: &str = "RATE_LIMITED"; @@ -31,7 +32,9 @@ pub fn domain_error_status(e: &DomainError) -> StatusCode { StatusCode::BAD_REQUEST } DomainError::NotFound(_) => StatusCode::NOT_FOUND, - DomainError::Unauthorized(_) => StatusCode::UNAUTHORIZED, + DomainError::Unauthorized(_) | DomainError::InvalidCredentials(_) => { + StatusCode::UNAUTHORIZED + } DomainError::Forbidden(_) => StatusCode::FORBIDDEN, DomainError::Conflict(_) => StatusCode::CONFLICT, DomainError::InfrastructureError(_) => StatusCode::INTERNAL_SERVER_ERROR, @@ -65,6 +68,9 @@ impl IntoResponse for ApiError { } DomainError::NotFound(_) => refuse(status, NOT_FOUND, self.0.to_string()), DomainError::Unauthorized(_) => refuse(status, UNAUTHORIZED, self.0.to_string()), + DomainError::InvalidCredentials(_) => { + refuse(status, INVALID_CREDENTIALS, self.0.to_string()) + } DomainError::Forbidden(_) => refuse(status, FORBIDDEN, self.0.to_string()), DomainError::Conflict(_) => refuse(status, CONFLICT, self.0.to_string()), DomainError::InfrastructureError(_) => { diff --git a/crates/application/src/users/request_account_deletion.rs b/crates/application/src/users/request_account_deletion.rs index 2da7e595..de1546c0 100644 --- a/crates/application/src/users/request_account_deletion.rs +++ b/crates/application/src/users/request_account_deletion.rs @@ -30,7 +30,7 @@ pub async fn execute( .verify(password, actor_user.password_hash()) .await? { - return Err(DomainError::Unauthorized("incorrect password".into())); + return Err(DomainError::InvalidCredentials("incorrect password".into())); } let target_user = if is_self { diff --git a/crates/application/tests/users/request_account_deletion_test.rs b/crates/application/tests/users/request_account_deletion_test.rs index 456971fc..5fff6089 100644 --- a/crates/application/tests/users/request_account_deletion_test.rs +++ b/crates/application/tests/users/request_account_deletion_test.rs @@ -138,7 +138,10 @@ async fn a_wrong_password_is_unauthorized_and_changes_nothing() { .await .unwrap_err(); - assert!(matches!(err, DomainError::Unauthorized(_)), "got {err:?}"); + assert!( + matches!(err, DomainError::InvalidCredentials(_)), + "got {err:?}" + ); assert!(f.is_live(&alice).await); assert!(f.store.recorded_events().is_empty()); } @@ -166,7 +169,10 @@ async fn an_admin_deletes_another_user_with_their_own_password() { let err = request_account_deletion::execute(&f.deps, admin.clone(), bob.clone(), "bobs-pw") .await .unwrap_err(); - assert!(matches!(err, DomainError::Unauthorized(_)), "got {err:?}"); + assert!( + matches!(err, DomainError::InvalidCredentials(_)), + "got {err:?}" + ); assert!(f.is_live(&bob).await); request_account_deletion::execute(&f.deps, admin.clone(), bob.clone(), PASSWORD) diff --git a/crates/bootstrap/tests/account_deletion_test.rs b/crates/bootstrap/tests/account_deletion_test.rs index cfbb758d..4b285c02 100644 --- a/crates/bootstrap/tests/account_deletion_test.rs +++ b/crates/bootstrap/tests/account_deletion_test.rs @@ -62,6 +62,7 @@ async fn self_delete_with_a_wrong_password_is_refused_and_keeps_the_account() { ) .await; assert_eq!(status, 401, "{body}"); + assert!(body.contains("\"INVALID_CREDENTIALS\""), "{body}"); let (status, _, _) = send(&router, get(&format!("/users/{}", alice.user_id), None)).await; assert_eq!(status, 200); } @@ -109,8 +110,9 @@ async fn an_admin_deletes_another_account() { promote(&pool, &alice.user_id).await; let uri = format!("/api/v1/users/{}", bob.user_id); - let (status, _, _) = send(&router, delete_json(&uri, &alice.token, "nope")).await; + let (status, _, body) = send(&router, delete_json(&uri, &alice.token, "nope")).await; assert_eq!(status, 401, "wrong admin password"); + assert!(body.contains("\"INVALID_CREDENTIALS\""), "{body}"); let (status, _, body) = send(&router, delete_json(&uri, &alice.token, PASSWORD)).await; assert_eq!(status, 204, "{body}"); diff --git a/crates/domain/src/errors/mod.rs b/crates/domain/src/errors/mod.rs index 1dbda26f..a84f8998 100644 --- a/crates/domain/src/errors/mod.rs +++ b/crates/domain/src/errors/mod.rs @@ -17,6 +17,9 @@ pub enum DomainError { #[error("Unauthorized: {0}")] Unauthorized(String), + #[error("Unauthorized: {0}")] + InvalidCredentials(String), + #[error("Forbidden: {0}")] Forbidden(String), diff --git a/docs/adr/0014-http-adapter-and-one-refusal-shape.md b/docs/adr/0014-http-adapter-and-one-refusal-shape.md index 2b45a18e..9d03fbed 100644 --- a/docs/adr/0014-http-adapter-and-one-refusal-shape.md +++ b/docs/adr/0014-http-adapter-and-one-refusal-shape.md @@ -16,7 +16,8 @@ rate limiter's own 429 text. The refusals were documented without a body schema. **One body.** `api_types::ErrorResponse { error, code }`. `error` keeps its name and meaning (the SPA and existing clients read it); `code` is an additive, stable machine-readable field: -`NOT_FOUND`, `INVALID_INPUT`, `UNAUTHORIZED`, `FORBIDDEN`, `CONFLICT`, `RATE_LIMITED`, `INTERNAL`. +`NOT_FOUND`, `INVALID_INPUT`, `UNAUTHORIZED`, `INVALID_CREDENTIALS` (wrong password on a re-auth +check; unlike `UNAUTHORIZED` it must not trigger a token refresh), `FORBIDDEN`, `CONFLICT`, `RATE_LIMITED`, `INTERNAL`. **One constructor.** `errors::refuse(status, code, message) -> Response` is the only place a JSON error body is built. `ApiError` maps every `DomainError` variant to it with the status mapping that diff --git a/spa/src/components/delete-account-dialog.tsx b/spa/src/components/delete-account-dialog.tsx index d9c670c5..fa6b87b4 100644 --- a/spa/src/components/delete-account-dialog.tsx +++ b/spa/src/components/delete-account-dialog.tsx @@ -80,6 +80,7 @@ export function DeleteAccountDialog({ setPassword(e.target.value)} diff --git a/spa/src/features/users.ts b/spa/src/features/users.ts index 1e17d0a5..852297e3 100644 --- a/spa/src/features/users.ts +++ b/spa/src/features/users.ts @@ -242,7 +242,10 @@ export function useAdminDeleteUser() { return useMutation({ mutationFn: ({ id, password }: DeleteAccountRequest & { id: string }) => delWithBody(`/users/${id}`, { password }), - onSuccess: () => { + onSuccess: (_data, { id }) => { + qc.removeQueries({ + predicate: (q) => q.queryKey[0] === userKeys.all[0] && q.queryKey[1] === id, + }) qc.invalidateQueries({ queryKey: userKeys.all }) qc.invalidateQueries({ queryKey: ["activity-feed"] }) }, diff --git a/spa/src/lib/api/client.ts b/spa/src/lib/api/client.ts index c07fac1b..30ba0416 100644 --- a/spa/src/lib/api/client.ts +++ b/spa/src/lib/api/client.ts @@ -68,6 +68,15 @@ async function tryRefresh(): Promise { } } +async function isInvalidCredentials(res: Response): Promise { + try { + const body = await res.clone().json() + return body?.code === "INVALID_CREDENTIALS" + } catch { + return false + } +} + async function request( url: string, options?: RequestInit, @@ -81,6 +90,11 @@ async function request( }) if (!res.ok) { if (res.status === 401) { + // A failed re-auth check (wrong password) is not an expired session: + // no refresh, no retry, no logout. + if (await isInvalidCredentials(res)) { + throw new ApiError(res.status, await res.text()) + } if (url.includes("/auth/refresh") || url.includes("/auth/login")) { clearAuth() window.location.href = "/app/login" diff --git a/spa/src/locales/en.json b/spa/src/locales/en.json index 87908e25..27e2279b 100644 --- a/spa/src/locales/en.json +++ b/spa/src/locales/en.json @@ -211,7 +211,7 @@ "done": "User deleted", "errors": { "wrongPassword": "Wrong password", - "lastAdmin": "You are the last admin and cannot be deleted", + "lastAdmin": "The last admin account cannot be deleted", "forbidden": "Only admins can delete other users", "notFound": "User not found", "generic": "Could not delete account"