From 973bdd36c3569979a4375512299026a3b8aae726 Mon Sep 17 00:00:00 2001 From: alovladi007 <83262803+alovladi007@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:22:40 -0400 Subject: [PATCH] =?UTF-8?q?feat(auth):=20Phase=207a=20=E2=80=94=20email=20?= =?UTF-8?q?infra,=20password=20reset,=20change=20password?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform had no self-service account recovery: no email capability, no forgot-password, no change-password. After bootstrap_admin, a lost password was unrecoverable without SQL. Backend: - services/shared/mailer.py: env-driven SMTP sender (SMTP_HOST/PORT/ USERNAME/PASSWORD/STARTTLS/FROM, APP_BASE_URL). Without SMTP_HOST the message is logged at WARNING and send returns False — honest dev mode; the reset link is read from the service log. Named mailer.py, NOT email.py: the containers put services/shared on PYTHONPATH, where an email.py shadows the stdlib package and crashes smtplib itself (found live — the lims container died on reload). - password_reset_tokens table (migration 0024, tenant_isolation RLS, registered in the test_tenancy audit): sha256-only storage, purpose reset|invite (30 min / 7 days), single-use. - POST /api/auth/forgot-password: enumeration-safe 204 always, login rate-limit tier, OIDC accounts excluded (their IdP owns recovery). - POST /api/auth/reset-password: one generic 400 for unknown/expired/ used; consumes the token; 422 under the 12-char floor (shared with bootstrap_admin). - POST /api/auth/change-password (authed): verify current, set new; explicit 400 for SSO-only accounts. Frontend: - /forgot-password + /reset-password pages (public paths), the latter serving both reset and future invite links (?welcome=1). - "Forgot your password?" link on /login. - /system/account: identity summary + change-password form ("My Account" sidebar entry); authAPI in api-client (204-aware, surfaces backend detail messages, and does NOT treat change-password's 403 as a dead session). Proof: 12 new API tests (token lifecycle, reuse, expiry, enumeration safety, policy floors) — lims suite 80 passed; shared 88 passed; LIVE round-trip against the dev stack: forgot 204 → link from log → reset 204 → new password logs in, old 401, reuse 400; change-password 403 on wrong current / 204 / login with new; tsc clean; smoke 4/4; new pages render 200. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...0260803_1000_0024_password_reset_tokens.py | 96 ++++++++++ apps/web/src/app/ClientLayout.tsx | 2 +- apps/web/src/app/forgot-password/page.tsx | 103 ++++++++++ apps/web/src/app/login/page.tsx | 10 + apps/web/src/app/reset-password/page.tsx | 164 ++++++++++++++++ apps/web/src/app/system/account/page.tsx | 164 ++++++++++++++++ apps/web/src/components/layout/AppSidebar.tsx | 1 + apps/web/src/lib/api-client.ts | 51 +++++ services/lims/app/api/auth.py | 178 +++++++++++++++++- .../integration/test_auth_password_reset.py | 173 +++++++++++++++++ services/shared/db/models.py | 36 ++++ services/shared/mailer.py | 86 +++++++++ services/shared/tests/test_tenancy.py | 1 + 13 files changed, 1061 insertions(+), 4 deletions(-) create mode 100644 alembic/versions/20260803_1000_0024_password_reset_tokens.py create mode 100644 apps/web/src/app/forgot-password/page.tsx create mode 100644 apps/web/src/app/reset-password/page.tsx create mode 100644 apps/web/src/app/system/account/page.tsx create mode 100644 services/lims/tests/integration/test_auth_password_reset.py create mode 100644 services/shared/mailer.py diff --git a/alembic/versions/20260803_1000_0024_password_reset_tokens.py b/alembic/versions/20260803_1000_0024_password_reset_tokens.py new file mode 100644 index 00000000..bd9156d6 --- /dev/null +++ b/alembic/versions/20260803_1000_0024_password_reset_tokens.py @@ -0,0 +1,96 @@ +"""password_reset_tokens table + RLS (Phase 7 — account lifecycle) + +Before this, the platform had no self-service password recovery and no way +to invite a user with a set-your-own-password link: password_hash could +only be set by seed scripts or bootstrap_admin. This table backs both +flows: forgot-password (purpose='reset', 30 min) and admin invites +(purpose='invite', 7 days). Only the SHA-256 of the token is stored. + +Revision ID: 20260803_1000_0024 +Revises: 20260717_0900_0023 +Create Date: 2026-08-03 +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects.postgresql import UUID + +revision: str = "20260803_1000_0024" +down_revision: Union[str, Sequence[str], None] = "20260717_0900_0023" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + +GUC = "spectra.current_org_id" +ADMIN = "admin" +TABLE = "password_reset_tokens" + + +def _is_postgres() -> bool: + return op.get_bind().dialect.name == "postgresql" + + +def upgrade() -> None: + op.create_table( + TABLE, + sa.Column( + "id", UUID(as_uuid=True), primary_key=True, server_default=sa.text("gen_random_uuid()") + ), + sa.Column( + "organization_id", + UUID(as_uuid=True), + sa.ForeignKey("organizations.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column( + "user_id", + UUID(as_uuid=True), + sa.ForeignKey("users.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column("token_hash", sa.String(64), nullable=False), + sa.Column("purpose", sa.String(20), nullable=False, server_default="reset"), + sa.Column("expires_at", sa.TIMESTAMP(timezone=True), nullable=False), + sa.Column("used_at", sa.TIMESTAMP(timezone=True), nullable=True), + sa.Column( + "created_at", + sa.TIMESTAMP(timezone=True), + server_default=sa.func.now(), + nullable=False, + ), + sa.Column( + "updated_at", + sa.TIMESTAMP(timezone=True), + server_default=sa.func.now(), + nullable=False, + ), + ) + op.create_index("ix_password_reset_tokens_organization_id", TABLE, ["organization_id"]) + op.create_index("ix_password_reset_tokens_user_id", TABLE, ["user_id"]) + op.create_index("ix_password_reset_tokens_hash", TABLE, ["token_hash"], unique=True) + + if not _is_postgres(): + return + + # Same tenant_isolation shape as every other org-scoped table. + predicate = ( + f"current_setting('{GUC}', true) = '{ADMIN}'" + f" OR organization_id::text = current_setting('{GUC}', true)" + ) + op.execute( + f"ALTER TABLE {TABLE} ALTER COLUMN organization_id SET DEFAULT " + f"NULLIF(current_setting('{GUC}', true), '{ADMIN}')::uuid" + ) + op.execute(f"ALTER TABLE {TABLE} ENABLE ROW LEVEL SECURITY") + op.execute(f"ALTER TABLE {TABLE} FORCE ROW LEVEL SECURITY") + op.execute( + f"CREATE POLICY tenant_isolation ON {TABLE} " + f"USING ({predicate}) WITH CHECK ({predicate})" + ) + + +def downgrade() -> None: + if _is_postgres(): + op.execute(f"DROP POLICY IF EXISTS tenant_isolation ON {TABLE}") + op.drop_table(TABLE) diff --git a/apps/web/src/app/ClientLayout.tsx b/apps/web/src/app/ClientLayout.tsx index 385e685e..fadbc54f 100644 --- a/apps/web/src/app/ClientLayout.tsx +++ b/apps/web/src/app/ClientLayout.tsx @@ -18,7 +18,7 @@ import { useTokenRefresh } from '@/hooks/useTokenRefresh'; // a session — the // app's home (/) just redirects to /dashboard. Listed here so the guard below // can let the login page through without a redirect loop. -const PUBLIC_PATHS = new Set(['/login', '/auth/callback']); +const PUBLIC_PATHS = new Set(['/login', '/auth/callback', '/forgot-password', '/reset-password']); // Session 7.1: AuthProvider removed. The auth store hydrates from // localStorage at module load time (services/web/src/stores/useAuthStore.ts). diff --git a/apps/web/src/app/forgot-password/page.tsx b/apps/web/src/app/forgot-password/page.tsx new file mode 100644 index 00000000..ba6c0cbd --- /dev/null +++ b/apps/web/src/app/forgot-password/page.tsx @@ -0,0 +1,103 @@ +"use client"; + +/** + * Forgot-password page (Phase 7 — account lifecycle). + * + * Posts the email to /api/auth/forgot-password and shows the same neutral + * confirmation whatever the server found — the endpoint is deliberately + * enumeration-safe, and so is this UI. + */ + +import { useState } from 'react'; +import Link from 'next/link'; + +export default function ForgotPasswordPage() { + const [email, setEmail] = useState(''); + const [sent, setSent] = useState(false); + const [error, setError] = useState(''); + const [loading, setLoading] = useState(false); + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + setError(''); + setLoading(true); + try { + const res = await fetch('/api/v1/lims/auth/forgot-password', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email }), + }); + if (res.status === 429) { + setError('Too many attempts — wait a minute and try again.'); + } else if (!res.ok) { + setError('Something went wrong. Try again.'); + } else { + setSent(true); + } + } catch { + setError('Network error. Try again.'); + } finally { + setLoading(false); + } + }; + + return ( +
+
+
+

+ Reset your password +

+

+ Enter your account email and we'll send a reset link +

+
+ + {sent ? ( +
+ If an account exists for {email}, a reset link is on + its way. The link is valid for 30 minutes. +
+ ) : ( +
+ {error && ( +
+

{error}

+
+ )} +
+ + setEmail(e.target.value)} + disabled={loading} + /> +
+ +
+ )} + +

+ + Back to sign in + +

+
+
+ ); +} diff --git a/apps/web/src/app/login/page.tsx b/apps/web/src/app/login/page.tsx index 68e47c3a..9b668e17 100644 --- a/apps/web/src/app/login/page.tsx +++ b/apps/web/src/app/login/page.tsx @@ -103,6 +103,16 @@ export default function LoginPage() { {loading ? 'Signing in...' : 'Sign in'} + +

+ + Forgot your password? + +

{/* SSO entry point (Phase 6.6). Rendered only when the deploy sets diff --git a/apps/web/src/app/reset-password/page.tsx b/apps/web/src/app/reset-password/page.tsx new file mode 100644 index 00000000..79851e38 --- /dev/null +++ b/apps/web/src/app/reset-password/page.tsx @@ -0,0 +1,164 @@ +"use client"; + +/** + * Reset-password page (Phase 7 — account lifecycle). + * + * Terminal page for BOTH flows that email a set-password link: + * - forgot-password (?token=...) — "Reset your password" + * - admin invite (?token=...&welcome=1) — "Set your password" + * Posts to /api/auth/reset-password; the token is single-use and expiring, + * so a 400 sends the user back to request a fresh link. + */ + +import { Suspense, useState } from 'react'; +import Link from 'next/link'; +import { useRouter, useSearchParams } from 'next/navigation'; + +const MIN_LEN = 12; + +function ResetPasswordForm() { + const params = useSearchParams(); + const router = useRouter(); + const token = params.get('token') ?? ''; + const isWelcome = params.get('welcome') === '1'; + + const [password, setPassword] = useState(''); + const [confirm, setConfirm] = useState(''); + const [error, setError] = useState(''); + const [done, setDone] = useState(false); + const [loading, setLoading] = useState(false); + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + setError(''); + if (password.length < MIN_LEN) { + setError(`Password must be at least ${MIN_LEN} characters.`); + return; + } + if (password !== confirm) { + setError('Passwords do not match.'); + return; + } + setLoading(true); + try { + const res = await fetch('/api/v1/lims/auth/reset-password', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ token, new_password: password }), + }); + if (res.ok) { + setDone(true); + setTimeout(() => router.push('/login'), 2500); + } else if (res.status === 400) { + setError('This link is invalid or has expired. Request a new one.'); + } else if (res.status === 422) { + setError(`Password must be at least ${MIN_LEN} characters.`); + } else if (res.status === 429) { + setError('Too many attempts — wait a minute and try again.'); + } else { + setError('Something went wrong. Try again.'); + } + } catch { + setError('Network error. Try again.'); + } finally { + setLoading(false); + } + }; + + return ( +
+
+
+

+ {isWelcome ? 'Welcome — set your password' : 'Choose a new password'} +

+

+ At least {MIN_LEN} characters +

+
+ + {!token && ( +
+ This link is missing its token.{' '} + + Request a new reset link + + . +
+ )} + + {done ? ( +
+ Password set. Redirecting you to sign in… +
+ ) : token ? ( +
+ {error && ( +
+

{error}

+
+ )} +
+
+ + setPassword(e.target.value)} + disabled={loading} + /> +
+
+ + setConfirm(e.target.value)} + disabled={loading} + /> +
+
+ +
+ ) : null} + +

+ + Back to sign in + +

+
+
+ ); +} + +export default function ResetPasswordPage() { + // useSearchParams needs a Suspense boundary in the app router. + return ( + + + + ); +} diff --git a/apps/web/src/app/system/account/page.tsx b/apps/web/src/app/system/account/page.tsx new file mode 100644 index 00000000..95e20b07 --- /dev/null +++ b/apps/web/src/app/system/account/page.tsx @@ -0,0 +1,164 @@ +'use client' + +/** + * My Account (Phase 7 — account lifecycle). + * + * Personal account page: identity summary from /api/auth/me plus the + * change-password form (POST /api/auth/change-password). Org-level admin + * lives in /system/users; this page is strictly the caller's own account. + */ + +import { useState } from 'react' +import { useQuery } from '@tanstack/react-query' +import { KeyRound, Loader2, ShieldCheck, UserCircle2 } from 'lucide-react' +import { authAPI, getErrorMessage } from '@/lib/api-client' +import { Button } from '@/components/ui/button' +import { Card } from '@/components/ui/card' +import { Input } from '@/components/ui/input' +import { Label } from '@/components/ui/label' + +const MIN_LEN = 12 + +export default function AccountPage() { + const { data: me, isLoading } = useQuery({ + queryKey: ['auth', 'me'], + queryFn: authAPI.me, + }) + + const [current, setCurrent] = useState('') + const [next, setNext] = useState('') + const [confirm, setConfirm] = useState('') + const [error, setError] = useState('') + const [saved, setSaved] = useState(false) + const [saving, setSaving] = useState(false) + + const submit = async (e: React.FormEvent) => { + e.preventDefault() + setError('') + setSaved(false) + if (next.length < MIN_LEN) { + setError(`New password must be at least ${MIN_LEN} characters.`) + return + } + if (next !== confirm) { + setError('New passwords do not match.') + return + } + setSaving(true) + try { + await authAPI.changePassword(current, next) + setSaved(true) + setCurrent('') + setNext('') + setConfirm('') + } catch (err) { + setError(getErrorMessage(err)) + } finally { + setSaving(false) + } + } + + return ( +
+
+

+ + My Account +

+

+ Your identity and credentials on this SPECTRA-Lab organization +

+
+ + + {isLoading ? ( + + ) : me ? ( +
+
+
Name
+
{me.name}
+
+
+
Email
+
{me.email}
+
+
+
Role
+
+ + {me.role} +
+
+
+
Organization
+
{me.organization_id}
+
+
+ ) : ( +

Could not load your account.

+ )} + + + +

+ + Change password +

+
+ {error && ( +
+ {error} +
+ )} + {saved && ( +
+ Password changed. +
+ )} +
+ + setCurrent(e.target.value)} + disabled={saving} + /> +
+
+ + setNext(e.target.value)} + disabled={saving} + /> +
+
+ + setConfirm(e.target.value)} + disabled={saving} + /> +
+ +
+ +
+ ) +} diff --git a/apps/web/src/components/layout/AppSidebar.tsx b/apps/web/src/components/layout/AppSidebar.tsx index de1d8ae6..09724d45 100644 --- a/apps/web/src/components/layout/AppSidebar.tsx +++ b/apps/web/src/components/layout/AppSidebar.tsx @@ -213,6 +213,7 @@ const navigation: NavItem[] = [ children: [ { name: 'Instruments', href: '/system/instruments' }, { name: 'Users & Roles', href: '/system/users' }, + { name: 'My Account', href: '/system/account' }, { name: 'Settings', href: '/system/settings' }, ], }, diff --git a/apps/web/src/lib/api-client.ts b/apps/web/src/lib/api-client.ts index 6ef06e19..ef677dfd 100644 --- a/apps/web/src/lib/api-client.ts +++ b/apps/web/src/lib/api-client.ts @@ -1178,6 +1178,57 @@ export const processControlAPI = { // ==================== Helper Functions ==================== +// ==================== Auth API (Phase 7 — account lifecycle) ==================== +// Bespoke (not fetchAPI): these endpoints return 204 No Content on success, +// and their error bodies carry a `detail` the user must actually see +// ("Current password is incorrect"), not a generic statusText. + +export interface AuthMe { + id: string + email: string + name: string + role: string + organization_id: string +} + +async function authFetch(endpoint: string, options: RequestInit = {}): Promise { + const response = await fetch(endpoint, { + ...options, + headers: { + 'Content-Type': 'application/json', + ...authHeader(), + ...options.headers, + }, + }) + if (!response.ok) { + // NOT handleUnauthorized() here: a 403 from change-password is a wrong + // current password, not a dead session. + let detail = response.statusText + try { + const body = await response.json() + if (typeof body?.detail === 'string') detail = body.detail + } catch { + /* non-JSON body — keep statusText */ + } + if (response.status === 401) handleUnauthorized() + throw new APIError(response.status, detail) + } + return response +} + +export const authAPI = { + me: async (): Promise => { + const r = await authFetch('/api/v1/lims/auth/me') + return r.json() + }, + changePassword: async (currentPassword: string, newPassword: string): Promise => { + await authFetch('/api/v1/lims/auth/change-password', { + method: 'POST', + body: JSON.stringify({ current_password: currentPassword, new_password: newPassword }), + }) + }, +} + export function isAPIError(error: unknown): error is APIError { return error instanceof APIError } diff --git a/services/lims/app/api/auth.py b/services/lims/app/api/auth.py index f0d1584b..d10b6764 100644 --- a/services/lims/app/api/auth.py +++ b/services/lims/app/api/auth.py @@ -17,28 +17,90 @@ found nothing. Login identity now == DB identity, by construction. """ +import hashlib +import secrets +from datetime import datetime, timedelta, timezone from typing import Optional from fastapi import APIRouter, Depends, HTTPException, Request, Response, status from fastapi.security import OAuth2PasswordRequestForm from jose import JWTError -from pydantic import BaseModel +from pydantic import BaseModel, Field from sqlalchemy.orm import Session -from services.shared.auth.jwt import create_token_pair, decode_token, verify_password +from services.shared.auth.jwt import ( + create_token_pair, + decode_token, + hash_password, + verify_password, +) from services.shared.auth.revocation import revoke_jti from services.shared.db.deps import get_current_user, get_db -from services.shared.db.models import User +from services.shared.db.models import PasswordResetToken, User +from services.shared.mailer import app_base_url, send_email from services.shared.middleware.rate_limit import login_limit from services.shared.tenancy import with_admin_tenancy router = APIRouter(prefix="/api/auth", tags=["Authentication"]) +# Matches bootstrap_admin.py's floor — one policy everywhere. +MIN_PASSWORD_LEN = 12 + +RESET_TOKEN_TTL = timedelta(minutes=30) +INVITE_TOKEN_TTL = timedelta(days=7) + class RefreshRequest(BaseModel): refresh_token: str +class ForgotPasswordRequest(BaseModel): + email: str + + +class ResetPasswordRequest(BaseModel): + token: str + new_password: str = Field(min_length=1) + + +class ChangePasswordRequest(BaseModel): + current_password: str + new_password: str = Field(min_length=1) + + +def _hash_token(raw: str) -> str: + return hashlib.sha256(raw.encode()).hexdigest() + + +def _validate_new_password(pw: str) -> None: + if len(pw) < MIN_PASSWORD_LEN: + raise HTTPException( + status_code=422, + detail=f"Password must be at least {MIN_PASSWORD_LEN} characters", + ) + + +def issue_password_token(db: Session, user: User, *, purpose: str = "reset") -> str: + """Create a single-use password token row and return the RAW token. + + Shared by forgot-password (purpose='reset') and the admin invite flow + (purpose='invite'). The raw token goes into the emailed link only — + the DB stores its SHA-256. + """ + raw = secrets.token_urlsafe(32) + ttl = INVITE_TOKEN_TTL if purpose == "invite" else RESET_TOKEN_TTL + db.add( + PasswordResetToken( + organization_id=user.organization_id, + user_id=user.id, + token_hash=_hash_token(raw), + purpose=purpose, + expires_at=datetime.now(timezone.utc) + ttl, + ) + ) + return raw + + class LogoutRequest(BaseModel): # Optional: the client sends its refresh token so BOTH halves of the # session die at logout, not just the 15-minute access token. @@ -196,6 +258,116 @@ async def logout( return Response(status_code=status.HTTP_204_NO_CONTENT) +@router.post("/forgot-password", status_code=status.HTTP_204_NO_CONTENT) +@login_limit +async def forgot_password( + request: Request, + body: ForgotPasswordRequest, + db: Session = Depends(get_db), +): + """Start self-service password recovery. + + ALWAYS returns 204, whether or not the email matches a user — + anything else is an account-enumeration oracle. When it does match an + active password-auth user, a 30-minute single-use token is created and + the reset link is emailed (or logged, when SMTP is unconfigured — the + dev flow reads the link from the lims log). Rate-limited like login: + this endpoint sends email on attacker-controlled input. + """ + with with_admin_tenancy(db): + user = db.query(User).filter(User.email == body.email, User.is_active.is_(True)).first() + # OIDC users have no password to reset — their IdP owns recovery. + if user is not None and user.password_hash: + raw = issue_password_token(db, user, purpose="reset") + db.commit() + send_email( + to=user.email, + subject="SPECTRA-Lab password reset", + body_text=( + f"Hello {user.name},\n\n" + f"A password reset was requested for this address. The link below\n" + f"is valid for 30 minutes and can be used once:\n\n" + f"{app_base_url()}/reset-password?token={raw}\n\n" + f"If you did not request this, ignore this email — your password\n" + f"is unchanged." + ), + ) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + +@router.post("/reset-password", status_code=status.HTTP_204_NO_CONTENT) +@login_limit +async def reset_password( + request: Request, + body: ResetPasswordRequest, + db: Session = Depends(get_db), +): + """Set a new password with a valid reset/invite token. + + One generic 400 for unknown, expired, and already-used tokens — the + error must not reveal which. On success the token is consumed and the + user's password hash replaced; existing sessions stay valid until + their tokens expire (revocation is per-jti, and we don't know them). + """ + _validate_new_password(body.new_password) + now = datetime.now(timezone.utc) + with with_admin_tenancy(db): + token = ( + db.query(PasswordResetToken) + .filter(PasswordResetToken.token_hash == _hash_token(body.token)) + .first() + ) + expires_at = token.expires_at if token else None + if ( + token is None + or token.used_at is not None + or ( + expires_at.replace(tzinfo=timezone.utc) if expires_at.tzinfo is None else expires_at + ) + < now + ): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid or expired reset token", + ) + user = db.query(User).filter(User.id == token.user_id, User.is_active.is_(True)).first() + if user is None: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid or expired reset token", + ) + user.password_hash = hash_password(body.new_password) + token.used_at = now + db.commit() + return Response(status_code=status.HTTP_204_NO_CONTENT) + + +@router.post("/change-password", status_code=status.HTTP_204_NO_CONTENT) +async def change_password( + body: ChangePasswordRequest, + current_user: User = Depends(get_current_user), + db: Session = Depends(get_db), +): + """Authenticated password change: verify the current password, set the + new one. OIDC-only accounts (no password_hash) get an explicit 400 — + their identity provider owns the password. + """ + if not current_user.password_hash: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="This account signs in via SSO and has no local password", + ) + if not verify_password(body.current_password, current_user.password_hash): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Current password is incorrect", + ) + _validate_new_password(body.new_password) + current_user.password_hash = hash_password(body.new_password) + db.commit() + return Response(status_code=status.HTTP_204_NO_CONTENT) + + @router.get("/me") async def read_current_user(current_user: User = Depends(get_current_user)): """Return the authenticated user. Validates the bearer token for real diff --git a/services/lims/tests/integration/test_auth_password_reset.py b/services/lims/tests/integration/test_auth_password_reset.py new file mode 100644 index 00000000..1c465c36 --- /dev/null +++ b/services/lims/tests/integration/test_auth_password_reset.py @@ -0,0 +1,173 @@ +"""Password reset + change-password flows (Phase 7 — account lifecycle). + +Covers the full token lifecycle (request → email link → set new password → +login with it), the misuse space (reuse, expiry, garbage tokens, weak +passwords), enumeration safety, and the authed change-password path. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +from sqlalchemy.orm import Session + +import pytest + +from services.lims.tests.conftest import * # noqa: F401,F403 +from services.shared.db.models import PasswordResetToken, User + +NEW_PW = "brand-new-password-42" + + +@pytest.fixture(autouse=True) +def _reset_rate_limiter(): + """forgot/reset share the 5/min login tier; this suite makes more than + five calls per minute by design. Reset the in-memory bucket per test — + the limit itself stays enforced in production.""" + from services.shared.middleware.rate_limit import limiter + + limiter.reset() + yield + + +def _request_reset(lims_client, email: str) -> None: + r = lims_client.post("/api/auth/forgot-password", json={"email": email}) + assert r.status_code == 204 + + +def _mint_token_via_api(lims_client, db_session: Session, user: User) -> str: + """Mint a token through the same issuing helper the endpoints use. + + The raw token only exists inside the emailed link (the DB stores its + one-way hash), so tests get it from issue_password_token directly; + the endpoint's own row-creation behavior is asserted separately.""" + from services.lims.app.api.auth import issue_password_token + + raw = issue_password_token(db_session, user, purpose="reset") + db_session.commit() + return raw + + +def test_forgot_password_always_204_and_no_enumeration(lims_client, engineer_user): + # Real user and nonexistent user are indistinguishable. + r1 = lims_client.post("/api/auth/forgot-password", json={"email": engineer_user.email}) + r2 = lims_client.post("/api/auth/forgot-password", json={"email": "ghost@nowhere.example"}) + assert r1.status_code == r2.status_code == 204 + assert r1.content == r2.content + + +def test_forgot_password_creates_token_row(lims_client, db_session, engineer_user): + _request_reset(lims_client, engineer_user.email) + rows = ( + db_session.query(PasswordResetToken) + .filter(PasswordResetToken.user_id == engineer_user.id) + .all() + ) + assert len(rows) == 1 + assert rows[0].purpose == "reset" + assert rows[0].used_at is None + # Stored value is a 64-hex sha256, not the raw token. + assert len(rows[0].token_hash) == 64 + + +def test_forgot_password_no_token_for_unknown_email(lims_client, db_session, engineer_user): + _request_reset(lims_client, "ghost@nowhere.example") + assert db_session.query(PasswordResetToken).count() == 0 + + +def test_reset_password_full_round_trip(lims_client, db_session, engineer_user): + raw = _mint_token_via_api(lims_client, db_session, engineer_user) + + r = lims_client.post("/api/auth/reset-password", json={"token": raw, "new_password": NEW_PW}) + assert r.status_code == 204 + + # Old password dead, new password lives. + r_old = lims_client.post( + "/api/auth/login", data={"username": engineer_user.email, "password": "eng123"} + ) + assert r_old.status_code == 401 + r_new = lims_client.post( + "/api/auth/login", data={"username": engineer_user.email, "password": NEW_PW} + ) + assert r_new.status_code == 200 + assert "access_token" in r_new.json() + + +def test_reset_token_single_use(lims_client, db_session, engineer_user): + raw = _mint_token_via_api(lims_client, db_session, engineer_user) + assert ( + lims_client.post( + "/api/auth/reset-password", json={"token": raw, "new_password": NEW_PW} + ).status_code + == 204 + ) + r = lims_client.post( + "/api/auth/reset-password", json={"token": raw, "new_password": "another-long-pw-1"} + ) + assert r.status_code == 400 + + +def test_reset_token_expired(lims_client, db_session, engineer_user): + raw = _mint_token_via_api(lims_client, db_session, engineer_user) + row = db_session.query(PasswordResetToken).one() + row.expires_at = datetime.now(timezone.utc) - timedelta(minutes=1) + db_session.commit() + r = lims_client.post("/api/auth/reset-password", json={"token": raw, "new_password": NEW_PW}) + assert r.status_code == 400 + + +def test_reset_password_garbage_token(lims_client, engineer_user): + r = lims_client.post( + "/api/auth/reset-password", + json={"token": "not-a-real-token", "new_password": NEW_PW}, + ) + assert r.status_code == 400 + + +def test_reset_password_rejects_short_password(lims_client, db_session, engineer_user): + raw = _mint_token_via_api(lims_client, db_session, engineer_user) + r = lims_client.post("/api/auth/reset-password", json={"token": raw, "new_password": "short"}) + assert r.status_code == 422 + # Token NOT consumed by the failed attempt. + assert db_session.query(PasswordResetToken).one().used_at is None + + +def test_change_password_round_trip(lims_client, engineer_user, engineer_token, auth_header): + r = lims_client.post( + "/api/auth/change-password", + json={"current_password": "eng123", "new_password": NEW_PW}, + headers=auth_header(engineer_token), + ) + assert r.status_code == 204 + assert ( + lims_client.post( + "/api/auth/login", data={"username": engineer_user.email, "password": NEW_PW} + ).status_code + == 200 + ) + + +def test_change_password_wrong_current(lims_client, engineer_token, auth_header): + r = lims_client.post( + "/api/auth/change-password", + json={"current_password": "wrong-password", "new_password": NEW_PW}, + headers=auth_header(engineer_token), + ) + assert r.status_code == 403 + + +def test_change_password_requires_auth(lims_client): + r = lims_client.post( + "/api/auth/change-password", + json={"current_password": "x", "new_password": NEW_PW}, + ) + assert r.status_code in (401, 403) + + +def test_change_password_rejects_short(lims_client, engineer_token, auth_header): + r = lims_client.post( + "/api/auth/change-password", + json={"current_password": "eng123", "new_password": "short"}, + headers=auth_header(engineer_token), + ) + assert r.status_code == 422 diff --git a/services/shared/db/models.py b/services/shared/db/models.py index c9570019..9d2b877e 100644 --- a/services/shared/db/models.py +++ b/services/shared/db/models.py @@ -1198,6 +1198,42 @@ def __repr__(self): return f"" +class PasswordResetToken(Base, UUIDMixin, TimestampMixin): + """Single-use token for password reset and user-invite flows (Phase 7). + + Only the SHA-256 hash of the token is stored — the raw value goes into + the emailed link and is never persisted. ``purpose`` distinguishes a + forgot-password reset (30-minute expiry) from an admin invite + (7-day expiry); both terminate in the same set-password endpoint. + ``used_at`` non-NULL means consumed. RLS (tenant_isolation, migration + 0024) scopes rows; the pre-auth reset endpoints run in admin tenancy + exactly like login does. + """ + + __tablename__ = "password_reset_tokens" + __table_args__ = (Index("ix_password_reset_tokens_hash", "token_hash", unique=True),) + + organization_id = Column( + UUID(as_uuid=True), + ForeignKey("organizations.id", ondelete="CASCADE"), + nullable=False, + index=True, + ) + user_id = Column( + UUID(as_uuid=True), + ForeignKey("users.id", ondelete="CASCADE"), + nullable=False, + index=True, + ) + token_hash = Column(String(64), nullable=False) + purpose = Column(String(20), nullable=False, default="reset") # reset | invite + expires_at = Column(TIMESTAMP(timezone=True), nullable=False) + used_at = Column(TIMESTAMP(timezone=True), nullable=True) + + def __repr__(self): + return f"" + + # ============================================================================ # Audit Trail # ============================================================================ diff --git a/services/shared/mailer.py b/services/shared/mailer.py new file mode 100644 index 00000000..d20e199f --- /dev/null +++ b/services/shared/mailer.py @@ -0,0 +1,86 @@ +"""Outbound email for the platform (Phase 7 — account lifecycle). + +Named mailer.py, NOT email.py: the services containers put +services/shared directly on PYTHONPATH, where email.py would shadow +the stdlib `email` package and crash smtplib itself on import +(ModuleNotFoundError: 'email' is not a package). + +One honest module, env-driven: + + SMTP_HOST — when unset, emails are NOT sent; the full message is + logged at INFO instead (dev/CI mode). No fake success: + send_email() returns False so callers know. + SMTP_PORT — default 587. + SMTP_USERNAME / SMTP_PASSWORD — optional (unauthenticated relay if unset). + SMTP_STARTTLS — "true" (default) upgrades the connection; "false" for + already-TLS or plaintext relays (e.g. in-cluster). + SMTP_FROM — sender address; default no-reply@spectra-lab.local. + APP_BASE_URL — public URL of the web app, used by callers to build + links (e.g. https://lab.example.com). Default + http://localhost:3012 (the compose web container). + +Sending is synchronous smtplib on purpose: the only senders are low-volume +account flows (password reset, invites). If email ever becomes high-volume +(report schedules, alert digests), move sends onto the celery queue. +""" + +from __future__ import annotations + +import logging +import os +import smtplib +from email.message import EmailMessage + +logger = logging.getLogger(__name__) + + +def app_base_url() -> str: + """Public base URL of the web app, no trailing slash.""" + return os.getenv("APP_BASE_URL", "http://localhost:3012").rstrip("/") + + +def email_enabled() -> bool: + """True when a real SMTP relay is configured.""" + return bool(os.getenv("SMTP_HOST")) + + +def send_email(to: str, subject: str, body_text: str) -> bool: + """Send a plain-text email. Returns True only on a real SMTP send. + + Without SMTP_HOST the message is logged (so dev flows are inspectable — + the reset link appears in the lims service log) and False is returned. + Failures raise: account flows must surface a 5xx rather than pretend + the email went out. + """ + if not email_enabled(): + # warning, not info: an email that could not be delivered (no SMTP + # relay configured) deserves the louder level. In dev this is also + # how the reset/invite link is read out of the service log. + logger.warning( + "EMAIL NOT SENT (SMTP_HOST unset)\nTo: %s\nSubject: %s\n%s", + to, + subject, + body_text, + ) + return False + + msg = EmailMessage() + msg["From"] = os.getenv("SMTP_FROM", "no-reply@spectra-lab.local") + msg["To"] = to + msg["Subject"] = subject + msg.set_content(body_text) + + host = os.environ["SMTP_HOST"] + port = int(os.getenv("SMTP_PORT", "587")) + starttls = os.getenv("SMTP_STARTTLS", "true").lower() != "false" + username = os.getenv("SMTP_USERNAME") + password = os.getenv("SMTP_PASSWORD") + + with smtplib.SMTP(host, port, timeout=15) as smtp: + if starttls: + smtp.starttls() + if username and password: + smtp.login(username, password) + smtp.send_message(msg) + logger.info("email sent to %s: %s", to, subject) + return True diff --git a/services/shared/tests/test_tenancy.py b/services/shared/tests/test_tenancy.py index 109d9547..7b3402f1 100644 --- a/services/shared/tests/test_tenancy.py +++ b/services/shared/tests/test_tenancy.py @@ -125,6 +125,7 @@ def _load(fname): src_0019 = fh.read() covered |= set(_re.findall(r'"((?:pvd|cmp)_\w+)"', src_0019)) covered |= {_load("20260716_1200_0021_notifications.py").TABLE} # Phase 6.2 + covered |= {_load("20260803_1000_0024_password_reset_tokens.py").TABLE} # Phase 7 tenant_in_migration = covered