From 0dbad379f2a93a2e2153cfd2706640b536a4eb96 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 16:09:34 +1100 Subject: [PATCH] feat(auth): password change, admin reset and break-glass reset (#117) - POST /api/auth/password: change your own password; this session stays, all others end; a wrong current password is 400 and counts towards the lockout. - POST /api/auth/users/{username}/password: a tenant_admin resets a member of their own tenant; unlocks, ends that user's sessions and writes a password_reset row to the tenant's audit log. A user who is also in another tenant is refused (409). - scripts/reset_password.py: operator reset by username; clears the lockout and ends all sessions. - Frontend: change-password dialog from the header key icon. - Docs: ROLES.md, QUICKSTART, offline-install known limits, VCRM BR-36. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019YPoXzBSj89ZHgw7nfTmbB --- api/auth.py | 1 + api/routers/auth_routes.py | 58 ++++++- api/services/accounts.py | 52 ++++++ api/services/audit.py | 2 +- .../src/__tests__/change-password.test.tsx | 80 +++++++++ .../src/components/change-password.tsx | 121 ++++++++++++++ .../src/components/session-menu.tsx | 4 +- csv-table-hub-main/src/lib/session.ts | 22 +++ docs/QUICKSTART.md | 15 ++ docs/ROLES.md | 5 +- docs/VCRM.md | 2 +- docs/offline-install.md | 6 +- scripts/reset_password.py | 49 ++++++ tests/test_api_hardening.py | 2 +- tests/test_password_reset.py | 155 ++++++++++++++++++ tests/test_roles.py | 16 +- 16 files changed, 581 insertions(+), 9 deletions(-) create mode 100644 csv-table-hub-main/src/__tests__/change-password.test.tsx create mode 100644 csv-table-hub-main/src/components/change-password.tsx create mode 100644 scripts/reset_password.py create mode 100644 tests/test_password_reset.py diff --git a/api/auth.py b/api/auth.py index 90a3001..fcb084c 100644 --- a/api/auth.py +++ b/api/auth.py @@ -85,6 +85,7 @@ async def tenant_scope(authorization: str = Header(default=""), "list_te": frozenset({"uploader", "reviewer", "tenant_admin", "auditor"}), "delete_file": frozenset({"tenant_admin"}), "read_audit": frozenset({"tenant_admin", "auditor"}), + "reset_password": frozenset({"tenant_admin"}), } diff --git a/api/routers/auth_routes.py b/api/routers/auth_routes.py index a7af6ce..baf4d14 100644 --- a/api/routers/auth_routes.py +++ b/api/routers/auth_routes.py @@ -10,10 +10,12 @@ from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel, Field -from api.auth import bearer_token, require_session +from api.auth import PERMISSIONS, bearer_token, require_session from api.db import Conn from api import oidc from api.services import accounts +from api.services.audit import write_audit +from api.tenancy import schema_for, scoped router = APIRouter(prefix="/api/auth", tags=["auth"]) @@ -69,3 +71,57 @@ def logout(token: str = Depends(bearer_token)) -> dict: @router.get("/me") def me(session: dict = Depends(require_session)) -> dict: return session + + +_NewPassword = Field(min_length=accounts.MIN_PASSWORD, max_length=1024) + + +class ChangePasswordRequest(BaseModel): + currentPassword: str = Field(min_length=1, max_length=1024) + newPassword: str = _NewPassword + + +@router.post("/password") +def change_password(req: ChangePasswordRequest, token: str = Depends(bearer_token), + session: dict = Depends(require_session)) -> dict: + """Change your own password (#117). This session stays signed in; all others end. + + A wrong current password is 400, not 401, so the browser keeps the session, + and it counts towards the lockout like a failed sign-in. + """ + with Conn() as conn: + with conn.cursor() as cur: + ok = accounts.change_password(cur, session["userId"], token, + req.currentPassword, req.newPassword) + conn.commit() + if not ok: + raise HTTPException(status_code=400, detail="Current password is incorrect") + return {"status": "ok"} + + +class ResetPasswordRequest(BaseModel): + newPassword: str = _NewPassword + + +@router.post("/users/{username}/password") +def reset_password(username: str, req: ResetPasswordRequest, + session: dict = Depends(require_session)) -> dict: + """A tenant admin sets a member's password, unlocking the account and ending + its sessions (#117). Audited in the tenant's audit log.""" + if session["role"] not in PERMISSIONS["reset_password"]: + raise HTTPException(status_code=403, detail=f"Role '{session['role']}' may not reset password") + with Conn() as conn: + with conn.cursor() as cur: + result = accounts.reset_password(cur, session["tenantId"], username, req.newPassword) + if result == "ok": + # ponytail: audit_log has file columns only; target in file_name and actor + # in mode until M3's audit_events (#107) adds actor and object. + with scoped(schema_for(session["tenant"])): + write_audit(cur, "password_reset", None, username, None, f"by {session['username']}") + conn.commit() + if result == "missing": + raise HTTPException(status_code=404, detail="No such user in this tenant") + if result == "shared": + raise HTTPException(status_code=409, detail="This user is also in another tenant. " + "An operator must reset it with scripts/reset_password.py") + return {"status": "ok"} diff --git a/api/services/accounts.py b/api/services/accounts.py index 2932e03..f778dea 100644 --- a/api/services/accounts.py +++ b/api/services/accounts.py @@ -15,6 +15,7 @@ from api.config import settings ROLES = ("uploader", "reviewer", "tenant_admin", "auditor") +MIN_PASSWORD = 12 _N, _R, _P = 2**14, 8, 1 @@ -151,3 +152,54 @@ def session_for(cur, token: str) -> dict | None: def revoke(cur, token: str) -> None: cur.execute(_q("UPDATE {c}.sessions SET revoked_at = now() " "WHERE token_hash = %s AND revoked_at IS NULL"), (_token_hash(token),)) + + +def _set_password(cur, user_id: int, password: str, keep_token: str | None = None) -> None: + """New hash, lockout cleared, and every session revoked except ``keep_token`` (#117).""" + cur.execute(_q("UPDATE {c}.users SET password_hash = %s, failed_logins = 0, " + "first_failed_at = NULL, locked_until = NULL WHERE id = %s"), + (hash_password(password), user_id)) + cur.execute(_q("UPDATE {c}.sessions SET revoked_at = now() WHERE user_id = %s " + "AND revoked_at IS NULL AND token_hash IS DISTINCT FROM %s"), + (user_id, _token_hash(keep_token) if keep_token else None)) + + +def change_password(cur, user_id: int, token: str, current: str, new: str) -> bool: + """A signed-in user changes their own password; other sessions end. False if + ``current`` is wrong, which counts towards the lockout (#102).""" + cur.execute(_q("SELECT password_hash FROM {c}.users WHERE id = %s"), (user_id,)) + row = cur.fetchone() + if not (row and verify_password(current, row[0])): + if row: + _record_failure(cur, user_id) + return False + _set_password(cur, user_id, new, keep_token=token) + return True + + +def reset_password(cur, tenant_id: int, username: str, new: str) -> str: + """An admin sets a member's password; it also unlocks the account and ends all + of its sessions. Returns "ok", "missing" (not a member of ``tenant_id``) or + "shared": a user who is also in another tenant is refused, because the + password is global and one tenant's admin must not take over the others.""" + cur.execute(_q("SELECT u.id, EXISTS (SELECT 1 FROM {c}.memberships o " + "WHERE o.user_id = u.id AND o.tenant_id <> %s) " + "FROM {c}.users u JOIN {c}.memberships m " + "ON m.user_id = u.id AND m.tenant_id = %s WHERE u.username = %s"), + (tenant_id, tenant_id, username)) + row = cur.fetchone() + if not row: + return "missing" + if row[1]: + return "shared" + _set_password(cur, row[0], new) + return "ok" + + +def set_password(cur, username: str, new: str) -> bool: + """Break-glass reset by username, for an operator with database access (#117).""" + cur.execute(_q("SELECT id FROM {c}.users WHERE username = %s"), (username,)) + row = cur.fetchone() + if row: + _set_password(cur, row[0], new) + return bool(row) diff --git a/api/services/audit.py b/api/services/audit.py index 59941e3..5b35d82 100644 --- a/api/services/audit.py +++ b/api/services/audit.py @@ -35,7 +35,7 @@ # Actions the log is allowed to record. A typo becomes a loud failure here rather # than an unqueryable string in the table. -VALID_ACTIONS = frozenset({"upload", "delete"}) +VALID_ACTIONS = frozenset({"upload", "delete", "password_reset"}) def write_audit(cur, action: str, file_id: int | None, file_name: str | None, diff --git a/csv-table-hub-main/src/__tests__/change-password.test.tsx b/csv-table-hub-main/src/__tests__/change-password.test.tsx new file mode 100644 index 0000000..77d6caf --- /dev/null +++ b/csv-table-hub-main/src/__tests__/change-password.test.tsx @@ -0,0 +1,80 @@ +// src/__tests__/change-password.test.tsx — change your own password (#117). + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { ChangePasswordForm } from "@/components/change-password"; +import { clearSession, getSession } from "@/lib/session"; + +const json = (body: unknown, status = 200) => + new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json" } }); +const session = { + token: "tok-123", + expiresAt: new Date(Date.now() + 3_600_000).toISOString(), + username: "alice", + tenant: "acme", + role: "uploader", +}; + +async function fill(current: string, next: string, confirm = next) { + const user = userEvent.setup(); + await user.type(screen.getByLabelText("Current password"), current); + await user.type(screen.getByLabelText("New password"), next); + await user.type(screen.getByLabelText("Confirm new password"), confirm); + return user; +} +const submit = () => screen.getByRole("button", { name: /change password/i }); + +beforeEach(() => { + clearSession(); + sessionStorage.setItem("te.session", JSON.stringify(session)); +}); +afterEach(() => vi.unstubAllGlobals()); + +describe("ChangePasswordForm", () => { + it("keeps the button disabled while the new password is short or doesn't match", async () => { + render(); + await fill("old password!", "short"); + expect(submit()).toBeDisabled(); + expect(screen.getByText(/at least 12 characters/i)).toBeInTheDocument(); + }); + + it("flags a mismatched confirmation", async () => { + render(); + await fill("old password!", "a brand new passphrase", "a different passphrase"); + expect(screen.getByText(/don't match/i)).toBeInTheDocument(); + expect(submit()).toBeDisabled(); + }); + + it("posts current and new password with the bearer token, then confirms", async () => { + const fetchMock = vi.fn(async () => json({ status: "ok" })); + vi.stubGlobal("fetch", fetchMock); + const onDone = vi.fn(); + render(); + const user = await fill("old password!", "a brand new passphrase"); + await user.click(submit()); + expect(await screen.findByRole("status")).toHaveTextContent("Password changed"); + const [url, init] = fetchMock.mock.calls[0] as unknown as [string, RequestInit]; + expect(url).toMatch(/\/api\/auth\/password$/); + expect((init.headers as Record).Authorization).toBe("Bearer tok-123"); + expect(JSON.parse(init.body as string)).toEqual({ + currentPassword: "old password!", + newPassword: "a brand new passphrase", + }); + expect(onDone).toHaveBeenCalled(); + expect(getSession()?.token).toBe("tok-123"); + }); + + it("shows the server's message for a wrong current password and keeps the session", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => json({ detail: "Current password is incorrect" }, 400)), + ); + render(); + const user = await fill("wrong", "a brand new passphrase"); + await user.click(submit()); + expect(await screen.findByRole("alert")).toHaveTextContent("Current password is incorrect"); + expect(screen.getByLabelText("Current password")).toHaveValue(""); + expect(getSession()).not.toBeNull(); + }); +}); diff --git a/csv-table-hub-main/src/components/change-password.tsx b/csv-table-hub-main/src/components/change-password.tsx new file mode 100644 index 0000000..a47aa22 --- /dev/null +++ b/csv-table-hub-main/src/components/change-password.tsx @@ -0,0 +1,121 @@ +// Change your own password from the header (#117). +import { useState, type FormEvent } from "react"; +import { KeyRound } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogFooter, + DialogHeader, + DialogTitle, + DialogTrigger, +} from "@/components/ui/dialog"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { changePassword, MIN_PASSWORD } from "@/lib/session"; + +export function ChangePasswordForm({ onDone }: { onDone?: () => void }) { + const [current, setCurrent] = useState(""); + const [next, setNext] = useState(""); + const [confirm, setConfirm] = useState(""); + const [error, setError] = useState(null); + const [done, setDone] = useState(false); + const [busy, setBusy] = useState(false); + + const mismatch = confirm !== "" && next !== confirm; + const short = next !== "" && next.length < MIN_PASSWORD; + + async function onSubmit(e: FormEvent) { + e.preventDefault(); + setError(null); + setBusy(true); + try { + await changePassword(current, next); + setDone(true); + setCurrent(""); + setNext(""); + setConfirm(""); + onDone?.(); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + setCurrent(""); + } finally { + setBusy(false); + } + } + + if (done) { + return ( +

+ Password changed. You're still signed in here; any other sessions have been signed out. +

+ ); + } + + return ( +
+
+ + setCurrent(e.target.value)} + /> +
+
+ + setNext(e.target.value)} + /> + {short && ( +

At least {MIN_PASSWORD} characters.

+ )} +
+
+ + setConfirm(e.target.value)} + /> + {mismatch &&

The new passwords don't match.

} +
+ {error && ( +

+ {error} +

+ )} + + + +
+ ); +} + +export function ChangePasswordDialog() { + return ( + + + + + + + Change password + + + + + ); +} diff --git a/csv-table-hub-main/src/components/session-menu.tsx b/csv-table-hub-main/src/components/session-menu.tsx index b2a8e4a..7308b21 100644 --- a/csv-table-hub-main/src/components/session-menu.tsx +++ b/csv-table-hub-main/src/components/session-menu.tsx @@ -1,8 +1,9 @@ -// Signed-in user, tenant and role, with sign-out (#96). +// Signed-in user, tenant and role, with change password (#117) and sign-out (#96). import { useNavigate } from "@tanstack/react-router"; import { LogOut } from "lucide-react"; import { Button } from "@/components/ui/button"; import { getSession, signOut } from "@/lib/session"; +import { ChangePasswordDialog } from "@/components/change-password"; export function SessionMenu() { const navigate = useNavigate(); @@ -13,6 +14,7 @@ export function SessionMenu() { {s.username} · {s.tenant} +