fix: invalid-credentials code, no refresh on wrong pw

This commit is contained in:
2026-10-26 02:58:55 +01:00
parent 4714216093
commit bc3c9e3d23
10 changed files with 44 additions and 8 deletions

View File

@@ -8,6 +8,7 @@ use domain::errors::DomainError;
pub const NOT_FOUND: &str = "NOT_FOUND"; pub const NOT_FOUND: &str = "NOT_FOUND";
pub const INVALID_INPUT: &str = "INVALID_INPUT"; pub const INVALID_INPUT: &str = "INVALID_INPUT";
pub const UNAUTHORIZED: &str = "UNAUTHORIZED"; pub const UNAUTHORIZED: &str = "UNAUTHORIZED";
pub const INVALID_CREDENTIALS: &str = "INVALID_CREDENTIALS";
pub const FORBIDDEN: &str = "FORBIDDEN"; pub const FORBIDDEN: &str = "FORBIDDEN";
pub const CONFLICT: &str = "CONFLICT"; pub const CONFLICT: &str = "CONFLICT";
pub const RATE_LIMITED: &str = "RATE_LIMITED"; pub const RATE_LIMITED: &str = "RATE_LIMITED";
@@ -31,7 +32,9 @@ pub fn domain_error_status(e: &DomainError) -> StatusCode {
StatusCode::BAD_REQUEST StatusCode::BAD_REQUEST
} }
DomainError::NotFound(_) => StatusCode::NOT_FOUND, DomainError::NotFound(_) => StatusCode::NOT_FOUND,
DomainError::Unauthorized(_) => StatusCode::UNAUTHORIZED, DomainError::Unauthorized(_) | DomainError::InvalidCredentials(_) => {
StatusCode::UNAUTHORIZED
}
DomainError::Forbidden(_) => StatusCode::FORBIDDEN, DomainError::Forbidden(_) => StatusCode::FORBIDDEN,
DomainError::Conflict(_) => StatusCode::CONFLICT, DomainError::Conflict(_) => StatusCode::CONFLICT,
DomainError::InfrastructureError(_) => StatusCode::INTERNAL_SERVER_ERROR, 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::NotFound(_) => refuse(status, NOT_FOUND, self.0.to_string()),
DomainError::Unauthorized(_) => refuse(status, UNAUTHORIZED, 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::Forbidden(_) => refuse(status, FORBIDDEN, self.0.to_string()),
DomainError::Conflict(_) => refuse(status, CONFLICT, self.0.to_string()), DomainError::Conflict(_) => refuse(status, CONFLICT, self.0.to_string()),
DomainError::InfrastructureError(_) => { DomainError::InfrastructureError(_) => {

View File

@@ -30,7 +30,7 @@ pub async fn execute(
.verify(password, actor_user.password_hash()) .verify(password, actor_user.password_hash())
.await? .await?
{ {
return Err(DomainError::Unauthorized("incorrect password".into())); return Err(DomainError::InvalidCredentials("incorrect password".into()));
} }
let target_user = if is_self { let target_user = if is_self {

View File

@@ -138,7 +138,10 @@ async fn a_wrong_password_is_unauthorized_and_changes_nothing() {
.await .await
.unwrap_err(); .unwrap_err();
assert!(matches!(err, DomainError::Unauthorized(_)), "got {err:?}"); assert!(
matches!(err, DomainError::InvalidCredentials(_)),
"got {err:?}"
);
assert!(f.is_live(&alice).await); assert!(f.is_live(&alice).await);
assert!(f.store.recorded_events().is_empty()); 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") let err = request_account_deletion::execute(&f.deps, admin.clone(), bob.clone(), "bobs-pw")
.await .await
.unwrap_err(); .unwrap_err();
assert!(matches!(err, DomainError::Unauthorized(_)), "got {err:?}"); assert!(
matches!(err, DomainError::InvalidCredentials(_)),
"got {err:?}"
);
assert!(f.is_live(&bob).await); assert!(f.is_live(&bob).await);
request_account_deletion::execute(&f.deps, admin.clone(), bob.clone(), PASSWORD) request_account_deletion::execute(&f.deps, admin.clone(), bob.clone(), PASSWORD)

View File

@@ -62,6 +62,7 @@ async fn self_delete_with_a_wrong_password_is_refused_and_keeps_the_account() {
) )
.await; .await;
assert_eq!(status, 401, "{body}"); assert_eq!(status, 401, "{body}");
assert!(body.contains("\"INVALID_CREDENTIALS\""), "{body}");
let (status, _, _) = send(&router, get(&format!("/users/{}", alice.user_id), None)).await; let (status, _, _) = send(&router, get(&format!("/users/{}", alice.user_id), None)).await;
assert_eq!(status, 200); assert_eq!(status, 200);
} }
@@ -109,8 +110,9 @@ async fn an_admin_deletes_another_account() {
promote(&pool, &alice.user_id).await; promote(&pool, &alice.user_id).await;
let uri = format!("/api/v1/users/{}", bob.user_id); 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_eq!(status, 401, "wrong admin password");
assert!(body.contains("\"INVALID_CREDENTIALS\""), "{body}");
let (status, _, body) = send(&router, delete_json(&uri, &alice.token, PASSWORD)).await; let (status, _, body) = send(&router, delete_json(&uri, &alice.token, PASSWORD)).await;
assert_eq!(status, 204, "{body}"); assert_eq!(status, 204, "{body}");

View File

@@ -17,6 +17,9 @@ pub enum DomainError {
#[error("Unauthorized: {0}")] #[error("Unauthorized: {0}")]
Unauthorized(String), Unauthorized(String),
#[error("Unauthorized: {0}")]
InvalidCredentials(String),
#[error("Forbidden: {0}")] #[error("Forbidden: {0}")]
Forbidden(String), Forbidden(String),

View File

@@ -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 **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: 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 **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 error body is built. `ApiError` maps every `DomainError` variant to it with the status mapping that

View File

@@ -80,6 +80,7 @@ export function DeleteAccountDialog({
<Input <Input
id="delete-password" id="delete-password"
type="password" type="password"
autoFocus
autoComplete="current-password" autoComplete="current-password"
value={password} value={password}
onChange={(e) => setPassword(e.target.value)} onChange={(e) => setPassword(e.target.value)}

View File

@@ -242,7 +242,10 @@ export function useAdminDeleteUser() {
return useMutation({ return useMutation({
mutationFn: ({ id, password }: DeleteAccountRequest & { id: string }) => mutationFn: ({ id, password }: DeleteAccountRequest & { id: string }) =>
delWithBody(`/users/${id}`, { password }), 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: userKeys.all })
qc.invalidateQueries({ queryKey: ["activity-feed"] }) qc.invalidateQueries({ queryKey: ["activity-feed"] })
}, },

View File

@@ -68,6 +68,15 @@ async function tryRefresh(): Promise<boolean> {
} }
} }
async function isInvalidCredentials(res: Response): Promise<boolean> {
try {
const body = await res.clone().json()
return body?.code === "INVALID_CREDENTIALS"
} catch {
return false
}
}
async function request<T = void>( async function request<T = void>(
url: string, url: string,
options?: RequestInit, options?: RequestInit,
@@ -81,6 +90,11 @@ async function request<T = void>(
}) })
if (!res.ok) { if (!res.ok) {
if (res.status === 401) { 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")) { if (url.includes("/auth/refresh") || url.includes("/auth/login")) {
clearAuth() clearAuth()
window.location.href = "/app/login" window.location.href = "/app/login"

View File

@@ -211,7 +211,7 @@
"done": "User deleted", "done": "User deleted",
"errors": { "errors": {
"wrongPassword": "Wrong password", "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", "forbidden": "Only admins can delete other users",
"notFound": "User not found", "notFound": "User not found",
"generic": "Could not delete account" "generic": "Could not delete account"