diff --git a/backend/testjam/routers/auth.py b/backend/testjam/routers/auth.py index e07f4be..da0ff73 100644 --- a/backend/testjam/routers/auth.py +++ b/backend/testjam/routers/auth.py @@ -25,6 +25,7 @@ from testjam.schemas.user import TokenResponse from testjam.services.password_reset import ( PasswordResetMisconfigured, + PasswordResetOutcome, confirm_password_reset, request_password_reset, ) @@ -106,9 +107,27 @@ def password_reset_request( return Response(status_code=status.HTTP_204_NO_CONTENT) +RESET_ERROR_DETAIL = { + PasswordResetOutcome.UNKNOWN: "Reset token is not recognized", + PasswordResetOutcome.USED: "Reset token has already been used", + PasswordResetOutcome.EXPIRED: "Reset token has expired", +} +RESET_ERROR_CODE = { + PasswordResetOutcome.UNKNOWN: "reset_token_invalid", + PasswordResetOutcome.USED: "reset_token_used", + PasswordResetOutcome.EXPIRED: "reset_token_expired", +} + + @router.post("/password-reset/confirm", status_code=status.HTTP_204_NO_CONTENT) def password_reset_confirm(body: PasswordResetConfirm, db: Session = Depends(get_db)): - succeeded = confirm_password_reset(db, body.token, body.new_password) - if not succeeded: - raise HTTPException(status_code=400, detail="Invalid or expired token") - return Response(status_code=status.HTTP_204_NO_CONTENT) + outcome = confirm_password_reset(db, body.token, body.new_password) + if outcome is PasswordResetOutcome.OK: + return Response(status_code=status.HTTP_204_NO_CONTENT) + raise HTTPException( + status_code=400, + detail={ + "code": RESET_ERROR_CODE[outcome], + "message": RESET_ERROR_DETAIL[outcome], + }, + ) diff --git a/backend/testjam/services/password_reset.py b/backend/testjam/services/password_reset.py index 1f41d74..dbdcee6 100644 --- a/backend/testjam/services/password_reset.py +++ b/backend/testjam/services/password_reset.py @@ -1,4 +1,5 @@ from datetime import datetime, timezone +from enum import Enum from fastapi import BackgroundTasks from sqlalchemy.orm import Session @@ -13,6 +14,13 @@ from testjam.services.settings import get_settings as get_app_settings +class PasswordResetOutcome(str, Enum): + OK = "ok" + UNKNOWN = "unknown" + USED = "used" + EXPIRED = "expired" + + def request_password_reset( db: Session, email: str, @@ -37,18 +45,28 @@ def issue_reset_email_for_user( _dispatch_reset_email(db, user, raw_token, background) -def confirm_password_reset(db: Session, raw_token: str, new_password: str) -> bool: - token = _load_active_token(db, raw_token) - if token is None: - return False - user = db.get(User, token.user_id) +def confirm_password_reset( + db: Session, raw_token: str, new_password: str, +) -> PasswordResetOutcome: + record = ( + db.query(PasswordResetToken) + .filter(PasswordResetToken.token_hash == PasswordResetToken.hash(raw_token)) + .first() + ) + if record is None: + return PasswordResetOutcome.UNKNOWN + if record.used_at is not None: + return PasswordResetOutcome.USED + if _expires_at_utc(record) <= datetime.now(timezone.utc): + return PasswordResetOutcome.EXPIRED + user = db.get(User, record.user_id) if user is None or not user.is_active or user.deleted_at is not None: - return False + return PasswordResetOutcome.UNKNOWN user.hashed_password = hash_password(new_password) - token.used_at = datetime.now(timezone.utc) + record.used_at = datetime.now(timezone.utc) db.commit() clear_lockout(db, user) - return True + return PasswordResetOutcome.OK def _create_reset_token(db: Session, user: User) -> tuple[str, str, datetime]: @@ -59,19 +77,6 @@ def _create_reset_token(db: Session, user: User) -> tuple[str, str, datetime]: return raw, token_hash, expires_at -def _load_active_token(db: Session, raw_token: str) -> PasswordResetToken | None: - record = ( - db.query(PasswordResetToken) - .filter(PasswordResetToken.token_hash == PasswordResetToken.hash(raw_token)) - .first() - ) - if record is None or record.used_at is not None: - return None - if _expires_at_utc(record) <= datetime.now(timezone.utc): - return None - return record - - def _expires_at_utc(token: PasswordResetToken) -> datetime: if token.expires_at.tzinfo is None: return token.expires_at.replace(tzinfo=timezone.utc) diff --git a/backend/tests/test_password_reset.py b/backend/tests/test_password_reset.py index bf218c8..a6f7780 100644 --- a/backend/tests/test_password_reset.py +++ b/backend/tests/test_password_reset.py @@ -146,6 +146,7 @@ def test_confirm_rejects_unknown_token(client): response = _confirm_reset(client, "garbage-token-value") assert response.status_code == 400 + assert response.json()["detail"]["code"] == "reset_token_invalid" def test_confirm_rejects_already_used_token(client, user_factory): @@ -157,6 +158,7 @@ def test_confirm_rejects_already_used_token(client, user_factory): assert first.status_code == 204 assert second.status_code == 400 + assert second.json()["detail"]["code"] == "reset_token_used" def test_confirm_rejects_expired_token(client, user_factory): @@ -174,6 +176,7 @@ def test_confirm_rejects_expired_token(client, user_factory): response = _confirm_reset(client, raw_token) assert response.status_code == 400 + assert response.json()["detail"]["code"] == "reset_token_expired" def test_confirm_rejects_weak_password(client, user_factory): diff --git a/frontend/src/__tests__/AppLayoutMobile.test.jsx b/frontend/src/__tests__/AppLayoutMobile.test.jsx index 19ccce0..d1bdd64 100644 --- a/frontend/src/__tests__/AppLayoutMobile.test.jsx +++ b/frontend/src/__tests__/AppLayoutMobile.test.jsx @@ -11,6 +11,7 @@ vi.mock("../hooks/useAuth", () => ({ }), useLogout: () => () => {}, useUpdateMe: () => ({ mutate: () => {}, isPending: false }), + useStorageAuthSync: () => {}, })) vi.mock("../hooks/useProjects", () => ({ diff --git a/frontend/src/__tests__/ResetPasswordPage.test.jsx b/frontend/src/__tests__/ResetPasswordPage.test.jsx index c9aa968..0c84320 100644 --- a/frontend/src/__tests__/ResetPasswordPage.test.jsx +++ b/frontend/src/__tests__/ResetPasswordPage.test.jsx @@ -79,4 +79,21 @@ describe("ResetPasswordPage", () => { expect(await screen.findByRole("alert")).toHaveTextContent(/invalid or expired token/i) }) + + it.each([ + ["reset_token_invalid", /not recognized/i], + ["reset_token_used", /already used/i], + ["reset_token_expired", /expired/i], + ])("maps backend error code %s to its localized message", async (code, pattern) => { + authApi.confirmPasswordReset.mockRejectedValue({ + response: { data: { detail: { code, message: "fallback" } } }, + }) + renderPage("/reset-password?token=bad") + + fireEvent.change(screen.getByLabelText(/new password/i), { target: { value: "fresh-secret-12" } }) + fireEvent.change(screen.getByLabelText(/confirm password/i), { target: { value: "fresh-secret-12" } }) + fireEvent.click(screen.getByRole("button", { name: /update password/i })) + + expect(await screen.findByRole("alert")).toHaveTextContent(pattern) + }) }) diff --git a/frontend/src/__tests__/useStorageAuthSync.test.jsx b/frontend/src/__tests__/useStorageAuthSync.test.jsx new file mode 100644 index 0000000..b1a4ddd --- /dev/null +++ b/frontend/src/__tests__/useStorageAuthSync.test.jsx @@ -0,0 +1,69 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import { renderHook } from "@testing-library/react" +import { QueryClient, QueryClientProvider } from "@tanstack/react-query" + +import { useStorageAuthSync } from "../hooks/useAuth" + + +function setup() { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const clearSpy = vi.spyOn(queryClient, "clear") + const wrapper = ({ children }) => ( + {children} + ) + renderHook(() => useStorageAuthSync(), { wrapper }) + return { clearSpy } +} + + +function fireStorageEvent({ key, newValue }) { + const event = new StorageEvent("storage", { + key, + newValue, + oldValue: "previous", + storageArea: window.localStorage, + }) + window.dispatchEvent(event) +} + + +describe("useStorageAuthSync", () => { + const originalHref = window.location.href + + beforeEach(() => { + delete window.location + window.location = { href: originalHref } + }) + + afterEach(() => { + delete window.location + window.location = { href: originalHref } + }) + + it("clears the query cache and redirects to /login when the token is removed in another tab", () => { + const { clearSpy } = setup() + + fireStorageEvent({ key: "token", newValue: null }) + + expect(clearSpy).toHaveBeenCalled() + expect(window.location.href).toBe("/login") + }) + + it("ignores storage events for unrelated keys", () => { + const { clearSpy } = setup() + + fireStorageEvent({ key: "theme", newValue: null }) + + expect(clearSpy).not.toHaveBeenCalled() + expect(window.location.href).toBe(originalHref) + }) + + it("ignores storage events that set a new token value (login in another tab)", () => { + const { clearSpy } = setup() + + fireStorageEvent({ key: "token", newValue: "fresh-token" }) + + expect(clearSpy).not.toHaveBeenCalled() + expect(window.location.href).toBe(originalHref) + }) +}) diff --git a/frontend/src/components/layout/AppLayout.jsx b/frontend/src/components/layout/AppLayout.jsx index d9b4c1f..7acac14 100644 --- a/frontend/src/components/layout/AppLayout.jsx +++ b/frontend/src/components/layout/AppLayout.jsx @@ -4,13 +4,14 @@ import { Menu } from 'lucide-react' import { useTranslation } from 'react-i18next' import { Sidebar } from './Sidebar' import { CommandPalette } from '../ui/command-palette' -import { useMe, useUpdateMe } from '../../hooks/useAuth' +import { useMe, useUpdateMe, useStorageAuthSync } from '../../hooks/useAuth' import { useProjectDeletionWatcher } from '../../hooks/useProjectDeletionWatcher' import { browserTimezone } from '../../lib/format' import i18n, { setLocale, SUPPORTED_LOCALES } from '../../i18n' export function AppLayout() { const { t } = useTranslation(['nav', 'common']) + useStorageAuthSync() const { data: user, isLoading, isError } = useMe() const updateMe = useUpdateMe() const detectedTimezoneBootstrapped = useRef(false) diff --git a/frontend/src/hooks/useAuth.js b/frontend/src/hooks/useAuth.js index 0191080..5c38844 100644 --- a/frontend/src/hooks/useAuth.js +++ b/frontend/src/hooks/useAuth.js @@ -1,6 +1,31 @@ +import { useEffect } from 'react' import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' import { authApi } from '../api/auth' + +const TOKEN_KEY = 'token' + + +// Bind to the browser storage event so a logout in tab A drops the +// cache + redirects tab B immediately, instead of waiting for tab B's +// next request to come back 401. Mounted once by AppLayout so the +// listener exists for the lifetime of an authenticated session. +export function useStorageAuthSync() { + const queryClient = useQueryClient() + useEffect(() => { + const handler = (event) => { + if (event.key !== TOKEN_KEY) return + if (event.newValue) return + if (event.storageArea !== window.localStorage) return + queryClient.clear() + window.location.href = '/login' + } + window.addEventListener('storage', handler) + return () => window.removeEventListener('storage', handler) + }, [queryClient]) +} + + export function useMe() { return useQuery({ queryKey: ['me'], @@ -14,7 +39,7 @@ export function useLogin() { return useMutation({ mutationFn: ({ username, password }) => authApi.login(username, password), onSuccess: (data) => { - localStorage.setItem('token', data.access_token) + localStorage.setItem(TOKEN_KEY, data.access_token) queryClient.invalidateQueries({ queryKey: ['me'] }) }, }) @@ -23,7 +48,7 @@ export function useLogin() { export function useLogout() { const queryClient = useQueryClient() return () => { - localStorage.removeItem('token') + localStorage.removeItem(TOKEN_KEY) queryClient.clear() window.location.href = '/login' } diff --git a/frontend/src/i18n/locales/ca/auth.json b/frontend/src/i18n/locales/ca/auth.json index 232ae95..7d16b86 100644 --- a/frontend/src/i18n/locales/ca/auth.json +++ b/frontend/src/i18n/locales/ca/auth.json @@ -35,6 +35,11 @@ "success": "Contrasenya actualitzada. Redirigint a l'inici de sessió…", "update": "Actualitza la contrasenya", "updating": "Actualitzant…", - "failure": "No s'ha pogut restablir. Demana un enllaç nou." + "failure": "No s'ha pogut restablir. Demana un enllaç nou.", + "errors": { + "reset_token_invalid": "Aquest enllaç de recuperació no és vàlid. Demana'n un de nou.", + "reset_token_used": "Aquest enllaç ja s'ha utilitzat. Demana'n un de nou per triar una altra contrasenya.", + "reset_token_expired": "Aquest enllaç ha caducat. Demana'n un de nou." + } } } diff --git a/frontend/src/i18n/locales/ca/cases.json b/frontend/src/i18n/locales/ca/cases.json index 90995dd..28d747b 100644 --- a/frontend/src/i18n/locales/ca/cases.json +++ b/frontend/src/i18n/locales/ca/cases.json @@ -71,6 +71,12 @@ "fieldSteps": "Passos" }, "discussion": { + "empty": "Encara no hi ha comentaris", + "add": "Afegeix comentari", + "edit": "Edita", + "delete": "Elimina", + "save": "Desa", + "cancel": "Cancel·la", "deleteTitle": "Eliminar aquest comentari?", "deleteDescription": "El comentari s'eliminarà de manera permanent." } diff --git a/frontend/src/i18n/locales/en/auth.json b/frontend/src/i18n/locales/en/auth.json index c9d3343..dcc42fc 100644 --- a/frontend/src/i18n/locales/en/auth.json +++ b/frontend/src/i18n/locales/en/auth.json @@ -35,6 +35,11 @@ "success": "Password updated. Redirecting to sign in…", "update": "Update password", "updating": "Updating…", - "failure": "Reset failed. Please request a new link." + "failure": "Reset failed. Please request a new link.", + "errors": { + "reset_token_invalid": "This reset link is not recognized. Please request a new one.", + "reset_token_used": "This reset link was already used. Request a new one to choose another password.", + "reset_token_expired": "This reset link has expired. Please request a new one." + } } } diff --git a/frontend/src/i18n/locales/es/auth.json b/frontend/src/i18n/locales/es/auth.json index 729a286..1874ea6 100644 --- a/frontend/src/i18n/locales/es/auth.json +++ b/frontend/src/i18n/locales/es/auth.json @@ -35,6 +35,11 @@ "success": "Contraseña actualizada. Redirigiendo al inicio de sesión…", "update": "Actualizar contraseña", "updating": "Actualizando…", - "failure": "No se pudo restablecer. Solicita un nuevo enlace." + "failure": "No se pudo restablecer. Solicita un nuevo enlace.", + "errors": { + "reset_token_invalid": "Este enlace de recuperación no es válido. Solicita uno nuevo.", + "reset_token_used": "Este enlace ya fue usado. Solicita uno nuevo para elegir otra contraseña.", + "reset_token_expired": "Este enlace ha caducado. Solicita uno nuevo." + } } } diff --git a/frontend/src/i18n/locales/eu/auth.json b/frontend/src/i18n/locales/eu/auth.json index 68fd534..9683ccf 100644 --- a/frontend/src/i18n/locales/eu/auth.json +++ b/frontend/src/i18n/locales/eu/auth.json @@ -35,6 +35,11 @@ "success": "Pasahitza eguneratuta. Saio-hasierara birbideratzen…", "update": "Eguneratu pasahitza", "updating": "Eguneratzen…", - "failure": "Ezin izan da berrezarri. Eskatu esteka berria." + "failure": "Ezin izan da berrezarri. Eskatu esteka berria.", + "errors": { + "reset_token_invalid": "Berrezartzeko esteka hau ez da baliozkoa. Eskatu berri bat.", + "reset_token_used": "Esteka hau dagoeneko erabili da. Eskatu berri bat beste pasahitz bat aukeratzeko.", + "reset_token_expired": "Esteka hau iraungi da. Eskatu berri bat." + } } } diff --git a/frontend/src/i18n/locales/eu/cases.json b/frontend/src/i18n/locales/eu/cases.json index c015bf9..760cc28 100644 --- a/frontend/src/i18n/locales/eu/cases.json +++ b/frontend/src/i18n/locales/eu/cases.json @@ -71,6 +71,12 @@ "fieldSteps": "Urratsak" }, "discussion": { + "empty": "Oraindik ez dago iruzkinik", + "add": "Gehitu iruzkina", + "edit": "Editatu", + "delete": "Ezabatu", + "save": "Gorde", + "cancel": "Utzi", "deleteTitle": "Iruzkin hau ezabatu?", "deleteDescription": "Iruzkina behin betiko ezabatuko da." } diff --git a/frontend/src/i18n/locales/gl/auth.json b/frontend/src/i18n/locales/gl/auth.json index 3b07c95..133d076 100644 --- a/frontend/src/i18n/locales/gl/auth.json +++ b/frontend/src/i18n/locales/gl/auth.json @@ -35,6 +35,11 @@ "success": "Contrasinal actualizado. Redirixindo ao inicio de sesión…", "update": "Actualizar contrasinal", "updating": "Actualizando…", - "failure": "Non se puido restablecer. Solicita unha nova ligazón." + "failure": "Non se puido restablecer. Solicita unha nova ligazón.", + "errors": { + "reset_token_invalid": "Esta ligazón de recuperación non é válida. Solicita unha nova.", + "reset_token_used": "Esta ligazón xa foi utilizada. Solicita unha nova para escoller outro contrasinal.", + "reset_token_expired": "Esta ligazón caducou. Solicita unha nova." + } } } diff --git a/frontend/src/i18n/locales/gl/cases.json b/frontend/src/i18n/locales/gl/cases.json index ff03bc0..fc05d73 100644 --- a/frontend/src/i18n/locales/gl/cases.json +++ b/frontend/src/i18n/locales/gl/cases.json @@ -71,6 +71,12 @@ "fieldSteps": "Pasos" }, "discussion": { + "empty": "Aínda non hai comentarios", + "add": "Engadir comentario", + "edit": "Editar", + "delete": "Eliminar", + "save": "Gardar", + "cancel": "Cancelar", "deleteTitle": "Eliminar este comentario?", "deleteDescription": "O comentario eliminarase de xeito permanente." } diff --git a/frontend/src/pages/ResetPasswordPage.jsx b/frontend/src/pages/ResetPasswordPage.jsx index 63879cc..8b209e1 100644 --- a/frontend/src/pages/ResetPasswordPage.jsx +++ b/frontend/src/pages/ResetPasswordPage.jsx @@ -53,7 +53,7 @@ export function ResetPasswordPage() {
{confirmReset.isError && (

- {confirmReset.error?.response?.data?.detail || t('reset.failure')} + {resetErrorMessage(t, confirmReset.error)}

)}