diff --git a/alembic/versions/022_password_resets.py b/alembic/versions/022_password_resets.py new file mode 100644 index 0000000..6eb7146 --- /dev/null +++ b/alembic/versions/022_password_resets.py @@ -0,0 +1,118 @@ +"""Real password changes and resets. + +- users.password_changed_at: refresh tokens issued before it are refused, so changing or + resetting a password signs out other sessions (within the access token's lifetime). + scirm_user_by_id returns it. +- password_resets: single-use reset links. Only the SHA-256 of the token is stored; it + expires (30 minutes by default) and a new request invalidates the user's earlier ones. + Both steps happen before the user is signed in, so they go through SECURITY DEFINER + functions (like the login lookup): scirm_password_reset_request creates a token for an + active account, and scirm_password_reset_consume sets the new password and uses the + token in one statement, so a token works exactly once. + +Revision ID: 022 +Revises: 021 +Create Date: 2026-10-23 +""" +from typing import Sequence, Union + +from alembic import op + +revision: str = "022" +down_revision: Union[str, None] = "021" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + +FUNCTIONS = ("scirm_password_reset_request(text, text, timestamptz)", "scirm_password_reset_consume(text, text)") + +USER_BY_ID = """ + CREATE FUNCTION scirm_user_by_id(p_id uuid) + RETURNS TABLE (id uuid, organization_id uuid, email text, name text, roles text[], + is_active boolean, mfa_secret text, mfa_enabled boolean{extra}) + LANGUAGE sql STABLE SECURITY DEFINER SET search_path = public, pg_temp AS $$ + SELECT u.id, u.organization_id, u.email, u.name, u.roles::text[], u.is_active, + u.mfa_secret, u.mfa_enabled{extra_select} + FROM users u WHERE u.id = p_id + $$ +""" + + +def _user_by_id(with_changed_at: bool) -> None: + op.execute("DROP FUNCTION IF EXISTS scirm_user_by_id(uuid)") + op.execute(USER_BY_ID.format(extra=", password_changed_at timestamptz" if with_changed_at else "", + extra_select=", u.password_changed_at" if with_changed_at else "")) + op.execute("REVOKE ALL ON FUNCTION scirm_user_by_id(uuid) FROM PUBLIC") + + +def upgrade() -> None: + op.execute("ALTER TABLE users ADD COLUMN password_changed_at timestamptz NULL") + _user_by_id(True) + op.execute(""" + CREATE TABLE password_resets ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + organization_id uuid NOT NULL REFERENCES organizations(id), + user_id uuid NOT NULL REFERENCES users(id), + token_hash text NOT NULL UNIQUE CHECK (token_hash ~ '^[0-9a-f]{64}$'), + expires_at timestamptz NOT NULL, + used_at timestamptz NULL, + created_at timestamptz NOT NULL DEFAULT now() + ) + """) + op.execute("CREATE INDEX ix_password_resets_user ON password_resets (user_id)") + # Enabled, not forced (like users): the application role sees nothing without its + # organization, while the definer functions (the owner) work before sign-in. + op.execute("ALTER TABLE password_resets ENABLE ROW LEVEL SECURITY") + op.execute(""" + CREATE POLICY password_resets_org_isolation ON password_resets + USING (organization_id = scirm_current_org()) + WITH CHECK (organization_id = scirm_current_org()) + """) + op.execute(""" + CREATE FUNCTION scirm_password_reset_request(p_email text, p_token_hash text, p_expires timestamptz) + RETURNS TABLE (user_id uuid, organization_id uuid, email text, name text) + LANGUAGE plpgsql SECURITY DEFINER SET search_path = public, pg_temp AS $$ + DECLARE + u users%ROWTYPE; + BEGIN + SELECT * INTO u FROM users WHERE lower(users.email) = lower(p_email) AND users.is_active; + IF NOT FOUND THEN + RETURN; + END IF; + -- Only the newest link works. + UPDATE password_resets SET used_at = now() WHERE password_resets.user_id = u.id AND used_at IS NULL; + INSERT INTO password_resets (organization_id, user_id, token_hash, expires_at) + VALUES (u.organization_id, u.id, p_token_hash, p_expires); + RETURN QUERY SELECT u.id, u.organization_id, u.email::text, u.name::text; + END + $$ + """) + op.execute(""" + CREATE FUNCTION scirm_password_reset_consume(p_token_hash text, p_password_hash text) + RETURNS TABLE (user_id uuid, organization_id uuid, email text) + LANGUAGE plpgsql SECURITY DEFINER SET search_path = public, pg_temp AS $$ + DECLARE + r password_resets%ROWTYPE; + BEGIN + UPDATE password_resets SET used_at = now() + WHERE token_hash = p_token_hash AND used_at IS NULL AND expires_at > now() + RETURNING * INTO r; + IF NOT FOUND THEN + RETURN; + END IF; + RETURN QUERY + UPDATE users SET hashed_password = p_password_hash, password_changed_at = now(), updated_at = now() + WHERE users.id = r.user_id AND users.is_active + RETURNING users.id, users.organization_id, users.email::text; + END + $$ + """) + for fn in FUNCTIONS: + op.execute(f"REVOKE ALL ON FUNCTION {fn} FROM PUBLIC") + + +def downgrade() -> None: + for fn in FUNCTIONS: + op.execute(f"DROP FUNCTION IF EXISTS {fn}") + op.execute("DROP TABLE IF EXISTS password_resets") + _user_by_id(False) + op.execute("ALTER TABLE users DROP COLUMN IF EXISTS password_changed_at") diff --git a/apps/api-gateway/main.py b/apps/api-gateway/main.py index 9d4e9ee..18cc665 100644 --- a/apps/api-gateway/main.py +++ b/apps/api-gateway/main.py @@ -331,7 +331,8 @@ def _is_sample_feature(path: str) -> bool: def _account_info(row: Dict[str, Any]) -> Dict[str, Any]: return {"id": str(row["id"]), "name": row["name"], "roles": list(row["roles"] or []), "org_id": str(row["organization_id"]), "mfa_secret": row.get("mfa_secret"), - "mfa_enabled": bool(row.get("mfa_enabled")), "stored": True} + "mfa_enabled": bool(row.get("mfa_enabled")), "password_changed_at": row.get("password_changed_at"), + "stored": True} def _mfa_on(user_info: Dict[str, Any]) -> bool: @@ -524,6 +525,17 @@ async def disable_mfa(body: MFACodeRequest, token: str = Depends(security)): return {"enabled": False} +def _issued_before_password_change(payload: Dict[str, Any], info: Dict[str, Any]) -> bool: + """A token issued before the account's last password change (change or reset).""" + changed = info.get("password_changed_at") + if not changed or "iat" not in payload: + return False + changed_at = datetime.fromisoformat(str(changed)) + if changed_at.tzinfo is None: + changed_at = changed_at.replace(tzinfo=timezone.utc) + return int(payload["iat"]) < int(changed_at.timestamp()) + + @app.post("/api/v1/auth/refresh") async def refresh_token(body: Dict[str, Any]): """Exchange a refresh token for a new access token.""" @@ -535,6 +547,8 @@ async def refresh_token(body: Dict[str, Any]): found = await _find_user_by_id(payload["sub"]) if not found: # unknown, or deactivated since the refresh token was issued raise HTTPException(status_code=401, detail="User not found") + if _issued_before_password_change(payload, found[1]): + raise HTTPException(status_code=401, detail="The password was changed: sign in again") tokens = _issue_tokens(*found) return {"access_token": tokens["access_token"], "token_type": "bearer"} @@ -1792,35 +1806,85 @@ async def export_full_report(token: str = Depends(security)): # --- Password Reset --- +PASSWORD_RESET_MINUTES = int(os.getenv("PASSWORD_RESET_MINUTES", "30")) +_RESET_SENT = "If an account exists for this email, a reset link has been sent. It works once, for 30 minutes." + + +class ForgotPasswordRequest(BaseModel): + email: str = Field(..., min_length=3, max_length=255) + + +class ResetPasswordRequest(BaseModel): + token: str = Field(..., min_length=20, max_length=200) + new_password: str + + +class ChangePasswordRequest(BaseModel): + current_password: str + new_password: str + + +async def _send_reset_email(account: Dict[str, Any], token_value: str) -> None: + link = f"{PUBLIC_APP_URL}/reset-password#{token_value}" if PUBLIC_APP_URL else f"/reset-password#{token_value}" + text = (f"Hello {account.get('name') or ''},\n\nSomeone (hopefully you) asked to reset the SCIRM password for " + f"{account['email']}. Open this link within {PASSWORD_RESET_MINUTES} minutes to choose a new one:\n\n" + f"{link}\n\nIf you did not ask for this, ignore this email; your password stays the same.\n") + html_body = (f"

Someone (hopefully you) asked to reset the SCIRM password for {account['email']}.

" + f"

Choose a new password (works once, for {PASSWORD_RESET_MINUTES} minutes).

" + "

If you did not ask for this, ignore this email; your password stays the same.

") + try: + await reports.send_email([account["email"]], "Reset your SCIRM password", text, html_body) + except Exception as exc: + logger.error("Password reset email failed", error=type(exc).__name__) + + @app.post("/api/v1/auth/forgot-password") -async def forgot_password(body: Dict[str, Any]): - """Request a password reset email.""" - email = body.get("email", "") - # In production: generate token, send email via SMTP - # For dev: just acknowledge - logger.info("Password reset requested", email=email) - return {"message": "If an account exists for this email, a reset link has been sent."} +async def forgot_password(body: ForgotPasswordRequest): + """Email a single-use reset link. The answer is the same whether or not the account + exists, and the email is sent in the background so timing does not tell either.""" + if reports.smtp_config() is None: + raise HTTPException(status_code=503, detail="Password reset by email is not set up on this deployment. " + "Ask an administrator of your organization.") + import hashlib + token_value = secrets.token_urlsafe(32) + expires = (datetime.now(timezone.utc) + timedelta(minutes=PASSWORD_RESET_MINUTES)).isoformat() + account = await audit_store.request_password_reset(body.email.strip().lower(), + hashlib.sha256(token_value.encode()).hexdigest(), expires) + if account: + await _audit_event(str(account["organization_id"]), str(account["user_id"]), account["email"], + "auth.password_reset_requested") + asyncio.create_task(_send_reset_email(account, token_value)) + return {"message": _RESET_SENT} + @app.post("/api/v1/auth/reset-password") -async def reset_password(body: Dict[str, Any]): - """Reset password with a token.""" - token_val = body.get("token", "") - new_password = body.get("new_password", "") - if len(new_password) < 8: - raise HTTPException(status_code=400, detail="Password must be at least 8 characters") - # In production: validate token, update password in DB - return {"message": "Password has been reset successfully."} +async def reset_password(body: ResetPasswordRequest): + """Set a new password with a reset link's token. Other sessions are signed out.""" + import hashlib + _password_ok(body.new_password) + done = await audit_store.consume_password_reset(hashlib.sha256(body.token.encode()).hexdigest(), + get_password_hash(body.new_password)) + if not done: + raise HTTPException(status_code=400, detail="This reset link is not valid any more. Ask for a new one.") + await _audit_event(str(done["organization_id"]), str(done["user_id"]), done["email"], "auth.password_reset") + return {"message": "Your password has been reset. Sign in with the new one."} + @app.post("/api/v1/auth/change-password") -async def change_password(body: Dict[str, Any], token: str = Depends(security)): - """Change password for authenticated user.""" - user = await verify_token(token.credentials) - current = body.get("current_password", "") - new_pass = body.get("new_password", "") - if len(new_pass) < 8: - raise HTTPException(status_code=400, detail="Password must be at least 8 characters") - # In production: verify current password, update in DB - return {"message": "Password changed successfully."} +async def change_password(body: ChangePasswordRequest, token: str = Depends(security)): + """Change the caller's password. Other sessions are signed out; this one gets new tokens.""" + user = await verify_token(token.credentials) + if DEV_USERS.get(user.email.lower()): + raise HTTPException(status_code=400, detail="Demo accounts use DEV_PASSWORD and cannot change it.") + _password_ok(body.new_password) + if body.new_password == body.current_password: + raise HTTPException(status_code=400, detail="Choose a password different from the current one.") + if not await _check_password(user.email, body.current_password): + raise HTTPException(status_code=400, detail="The current password is not correct.") + if not await audit_store.set_password(user.organization_id, user.id, get_password_hash(body.new_password)): + raise HTTPException(status_code=404, detail="Account not found") + found = await _find_user_by_id(user.id) + return {"message": "Password changed. Other sessions are signed out.", **_issue_tokens(*found)} # --- Components --- diff --git a/apps/frontend/src/App.tsx b/apps/frontend/src/App.tsx index dfe1b9c..0093439 100644 --- a/apps/frontend/src/App.tsx +++ b/apps/frontend/src/App.tsx @@ -7,6 +7,7 @@ import SampleRoute from './components/SampleRoute'; import LoginPage from './pages/LoginPage'; import SignupPage from './pages/SignupPage'; import ForgotPasswordPage from './pages/ForgotPasswordPage'; +import ResetPasswordPage from './pages/ResetPasswordPage'; import NotFoundPage from './pages/NotFoundPage'; import SupplierScanPage from './pages/SupplierScanPage'; import SapImportPage from './pages/SapImportPage'; @@ -68,6 +69,7 @@ export default function App() { {/* Invitation links: public, whether or not someone is signed in on this browser */} } /> + } /> {/* 404 catch-all */} } /> diff --git a/apps/frontend/src/contexts/AuthContext.tsx b/apps/frontend/src/contexts/AuthContext.tsx index 8d23d1b..78b6b73 100644 --- a/apps/frontend/src/contexts/AuthContext.tsx +++ b/apps/frontend/src/contexts/AuthContext.tsx @@ -15,6 +15,8 @@ interface AuthContextType { verifyMfa: (mfaToken: string, code: string) => Promise; signup: (email: string, password: string, name: string) => Promise; acceptInvitation: (token: string, name: string, password: string) => Promise; + /** Changes the password; other sessions end, this one continues with new tokens. */ + changePassword: (current: string, next: string) => Promise; logout: () => void; } @@ -26,6 +28,7 @@ const AuthContext = createContext({ verifyMfa: async () => {}, signup: async () => {}, acceptInvitation: async () => {}, + changePassword: async () => {}, logout: () => {}, }); @@ -92,8 +95,12 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { begin(await api.acceptInvitation(token, name, password)); }; + const changePassword = async (current: string, next: string) => { + begin(await api.changePassword(current, next)); + }; + return ( - + {children} ); diff --git a/apps/frontend/src/i18n/locales/en.json b/apps/frontend/src/i18n/locales/en.json index f6f1c27..a431513 100644 --- a/apps/frontend/src/i18n/locales/en.json +++ b/apps/frontend/src/i18n/locales/en.json @@ -253,7 +253,7 @@ "newPassword": "New password (min 8 characters)", "confirmNewPassword": "Confirm new password", "updatePassword": "Update Password", - "passwordUpdated": "Your password has been changed successfully.", + "passwordUpdated": "Your password has been changed. Other sessions are signed out.", "dataExport": "Data Export", "dataExportDesc": "Download reports for board meetings and auditors", "exportRisks": "Export Risks (CSV)", diff --git a/apps/frontend/src/pages/ForgotPasswordPage.tsx b/apps/frontend/src/pages/ForgotPasswordPage.tsx index 5bd1d72..7677507 100644 --- a/apps/frontend/src/pages/ForgotPasswordPage.tsx +++ b/apps/frontend/src/pages/ForgotPasswordPage.tsx @@ -1,16 +1,26 @@ import { useState } from 'react'; import { Link } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; +import { api } from '../services/api'; export default function ForgotPasswordPage() { const [email, setEmail] = useState(''); - const [sent, setSent] = useState(false); + const [sent, setSent] = useState(''); + const [error, setError] = useState(''); + const [busy, setBusy] = useState(false); const { t } = useTranslation(); - const handleSubmit = (e: React.FormEvent) => { + const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); - // In production, call API to send reset email - setSent(true); + setError(''); + setBusy(true); + try { + setSent((await api.forgotPassword(email.trim())).message); + } catch (err: any) { + setError(err?.response?.data?.detail || t('auth.resetFailed', 'The request could not be sent. Try again.')); + } finally { + setBusy(false); + } }; return ( @@ -27,7 +37,7 @@ export default function ForgotPasswordPage() {
-

{t('auth.resetSent', "If an account exists for this email, you'll receive a password reset email shortly.")}

+

{sent}

{t('auth.backToSignIn', 'Back to Sign In')} ) : ( @@ -41,7 +51,8 @@ export default function ForgotPasswordPage() { placeholder={t('auth.emailPlaceholder', 'you@company.com')} /> -

diff --git a/apps/frontend/src/pages/ResetPasswordPage.tsx b/apps/frontend/src/pages/ResetPasswordPage.tsx new file mode 100644 index 0000000..a35212f --- /dev/null +++ b/apps/frontend/src/pages/ResetPasswordPage.tsx @@ -0,0 +1,63 @@ +import { FormEvent, useState } from 'react'; +import { Link } from 'react-router-dom'; +import { useTranslation } from 'react-i18next'; +import { api } from '../services/api'; + +// The link from the reset email: /reset-password#. The token stays in the fragment, +// so it is never sent to a server in a URL or a Referer header. +export default function ResetPasswordPage() { + const { t } = useTranslation(); + const [token] = useState(() => window.location.hash.slice(1)); + const [password, setPassword] = useState(''); + const [confirm, setConfirm] = useState(''); + const [done, setDone] = useState(''); + const [error, setError] = useState(''); + const [busy, setBusy] = useState(false); + + const submit = async (e: FormEvent) => { + e.preventDefault(); + setError(''); + if (password.length < 8) { setError(t('auth.passwordTooShort', 'Password must be at least 8 characters')); return; } + if (password !== confirm) { setError(t('auth.passwordMismatch', 'Passwords do not match')); return; } + setBusy(true); + try { + setDone((await api.resetPassword(token, password)).message); + window.history.replaceState(null, '', window.location.pathname); // drop the used token + } catch (err: any) { + setError(err?.response?.data?.detail || t('auth.resetFailed', 'The password could not be reset.')); + } finally { + setBusy(false); + } + }; + + const field = 'w-full px-3 py-2 border border-gray-300 dark:border-gray-600 rounded-lg outline-none focus:ring-2 focus:ring-blue-500 text-gray-900 dark:text-white bg-white dark:bg-gray-800'; + return ( +

+
+

{t('auth.choosePassword', 'Choose a new password')}

+ {done ? ( +
+

{done}

+ {t('auth.backToSignIn', 'Back to Sign In')} +
+ ) : !token ? ( +

+ {t('auth.resetNoToken', 'This page needs the link from the reset email.')}{' '} + {t('auth.askNewLink', 'Ask for a new link')} +

+ ) : ( +
+ setPassword(e.target.value)} + placeholder={t('auth.newPassword', 'New password')} className={field} /> + setConfirm(e.target.value)} + placeholder={t('auth.confirmPassword', 'Repeat the new password')} className={field} /> + {error &&

{error}

} + +
+ )} +
+
+ ); +} diff --git a/apps/frontend/src/pages/SettingsPage.tsx b/apps/frontend/src/pages/SettingsPage.tsx index 903d3f5..d1b4a6b 100644 --- a/apps/frontend/src/pages/SettingsPage.tsx +++ b/apps/frontend/src/pages/SettingsPage.tsx @@ -19,7 +19,7 @@ const LANGUAGES = [ ]; export default function SettingsPage() { - const { user } = useAuth(); + const { user, changePassword } = useAuth(); const { theme, toggleTheme } = useTheme(); const { addToast } = useToast(); const { t, i18n } = useTranslation(); @@ -38,7 +38,7 @@ export default function SettingsPage() { addToast('success', t('settings.languageUpdated', 'Language updated'), t('settings.languageSetTo', 'Language set to {{language}}', { language: LANGUAGES.find((l) => l.code === code)?.label })); }; - const handlePasswordChange = () => { + const handlePasswordChange = async () => { setPasswordError(''); if (newPassword.length < 8) { setPasswordError(t('auth.passwordTooShort', 'Password must be at least 8 characters')); @@ -48,8 +48,14 @@ export default function SettingsPage() { setPasswordError(t('auth.passwordMismatch', 'Passwords do not match')); return; } - // In production, call API to change password - addToast('success', t('settings.changePassword', 'Change Password'), t('settings.passwordUpdated', 'Your password has been changed successfully.')); + try { + await changePassword(currentPassword, newPassword); + } catch (err: any) { + setPasswordError(err?.response?.data?.detail || t('settings.passwordFailed', 'The password could not be changed.')); + return; + } + addToast('success', t('settings.changePassword', 'Change Password'), + t('settings.passwordUpdated', 'Your password has been changed. Other sessions are signed out.')); setCurrentPassword(''); setNewPassword(''); setConfirmPassword(''); diff --git a/apps/frontend/src/services/api.ts b/apps/frontend/src/services/api.ts index 0bbdba5..8bffd1a 100644 --- a/apps/frontend/src/services/api.ts +++ b/apps/frontend/src/services/api.ts @@ -525,6 +525,20 @@ export const api = { return response.data; }, + /** Returns new tokens: other sessions are signed out by the change. */ + async changePassword(currentPassword: string, newPassword: string): Promise { + return (await apiClient.post('/api/v1/auth/change-password', + { current_password: currentPassword, new_password: newPassword })).data; + }, + + async forgotPassword(email: string): Promise<{ message: string }> { + return (await apiClient.post('/api/v1/auth/forgot-password', { email })).data; + }, + + async resetPassword(token: string, newPassword: string): Promise<{ message: string }> { + return (await apiClient.post('/api/v1/auth/reset-password', { token, new_password: newPassword })).data; + }, + async verifyMfa(mfaToken: string, totpCode: string): Promise { const response = await apiClient.post('/api/v1/auth/mfa/verify', { mfa_token: mfaToken, totp_code: totpCode }); return response.data; diff --git a/docs/passwords.md b/docs/passwords.md new file mode 100644 index 0000000..45c92c4 --- /dev/null +++ b/docs/passwords.md @@ -0,0 +1,27 @@ +# Passwords + +**Change password** (Settings) checks the current password and stores the new one with +bcrypt. Your other sessions are signed out; the one you are using continues. + +**Forgot password** emails a reset link. The link: +- works **once**, for **30 minutes** (`PASSWORD_RESET_MINUTES`) +- stops working when a newer one is requested + +SCIRM stores only a hash of the link's token. The token travels in the URL fragment +(`/reset-password#…`), so it is never sent to a server in a URL or a Referer header. The +answer is the same whether or not an account exists, and the email is sent in the +background, so neither the answer nor its timing reveals which emails have accounts. + +Resetting signs out every session for that account: refresh tokens issued before the +change are refused (`users.password_changed_at`). An access token already issued keeps +working until it expires, at most 60 minutes. + +Password reset needs [email](scheduled-reports.md#email-settings). Without `SMTP_HOST`, +the forgot-password page says that reset by email is not set up and to ask an +administrator. + +Requests, resets and changes are recorded in the [audit log](audit-log.md) +(`auth.password_reset_requested`, `auth.password_reset`, `POST /api/v1/auth/change-password`). + +Two-step sign-in, when on, is still asked for after a reset: a reset changes only the +password. See [Two-step sign-in](two-step-sign-in.md) for a lost phone. diff --git a/libs/common/store.py b/libs/common/store.py index d3c401d..c226d61 100644 --- a/libs/common/store.py +++ b/libs/common/store.py @@ -252,6 +252,7 @@ def __init__(self, seed_demo: bool = True): self.webhook_deliveries: Dict[str, Dict[str, Any]] = {} self.notification_state: Dict[str, str] = {} self.report_schedules: Dict[str, Dict[str, Any]] = {} + self.password_resets: Dict[str, Dict[str, Any]] = {} async def close(self) -> None: return None @@ -661,6 +662,40 @@ async def record_login(self, org: str, user_id: str) -> None: if user and user["organization_id"] == org: user["last_login_at"] = _now() + async def set_password(self, org: str, user_id: str, hashed_password: str) -> bool: + user = self.users.get(str(user_id)) + if not user or user["organization_id"] != org: + return False + user.update(hashed_password=hashed_password, password_changed_at=await self.db_now()) + return True + + async def request_password_reset(self, email: str, token_hash: str, expires_at: str) -> Optional[Dict[str, Any]]: + """Before sign-in: a reset token for an active account (earlier ones stop working), + or None when there is no such account.""" + user = self._user_by_email(email) + if not user or not user["is_active"]: + return None + resets = self.password_resets + for r in resets.values(): + if r["user_id"] == user["id"] and r["used_at"] is None: + r["used_at"] = _now() + resets[token_hash] = {"user_id": user["id"], "organization_id": user["organization_id"], + "expires_at": expires_at, "used_at": None} + return {"user_id": user["id"], "organization_id": user["organization_id"], "email": user["email"], + "name": user["name"]} + + async def consume_password_reset(self, token_hash: str, hashed_password: str) -> Optional[Dict[str, Any]]: + """Set the new password and use the token, once; None if it is unknown, used or expired.""" + r = self.password_resets.get(token_hash) + if not r or r["used_at"] or datetime.fromisoformat(r["expires_at"]) <= datetime.now(timezone.utc): + return None + r["used_at"] = _now() + user = self.users.get(r["user_id"]) + if not user or not user["is_active"]: + return None + user.update(hashed_password=hashed_password, password_changed_at=await self.db_now()) + return {"user_id": user["id"], "organization_id": user["organization_id"], "email": user["email"]} + # --- feedback ----------------------------------------------------------- async def record_feedback(self, org: str, user_id: str, item: Dict[str, Any]) -> Dict[str, Any]: key = (org, str(user_id), item["target_type"], item["target_id"]) @@ -1870,6 +1905,27 @@ async def record_login(self, org: str, user_id: str) -> None: await self._exec(org, [("UPDATE users SET last_login_at = now() WHERE id = CAST(:i AS uuid)", {"i": user_id})]) + async def set_password(self, org: str, user_id: str, hashed_password: str) -> bool: + if not _is_uuid(user_id): + return False + rows = await self._all(org, ( + "UPDATE users SET hashed_password = :h, password_changed_at = now(), updated_at = now() " + "WHERE id = CAST(:i AS uuid) RETURNING id"), {"h": hashed_password, "i": user_id}) + return bool(rows) + + async def request_password_reset(self, email: str, token_hash: str, expires_at: str) -> Optional[Dict[str, Any]]: + """Before sign-in: a reset token for an active account (earlier ones stop working), + or None when there is no such account.""" + rows = await self._all(None, "SELECT * FROM scirm_password_reset_request(:e, :h, :x)", + {"e": (email or "").lower(), "h": token_hash, "x": datetime.fromisoformat(expires_at)}) + return _jsonable(rows[0]) if rows else None + + async def consume_password_reset(self, token_hash: str, hashed_password: str) -> Optional[Dict[str, Any]]: + """Set the new password and use the token, once; None if it is unknown, used or expired.""" + rows = await self._all(None, "SELECT * FROM scirm_password_reset_consume(:h, :p)", + {"h": token_hash, "p": hashed_password}) + return _jsonable(rows[0]) if rows else None + # --- feedback ----------------------------------------------------------- async def record_feedback(self, org: str, user_id: str, item: Dict[str, Any]) -> Dict[str, Any]: await self._ensure_org(org) diff --git a/mkdocs.yml b/mkdocs.yml index 85928e3..cfa1a45 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -19,6 +19,7 @@ nav: - Bedrock (EU) setup: bedrock-setup.md - Disaster exposure by site: site-exposure.md - Two-step sign-in: two-step-sign-in.md + - Passwords: passwords.md - Audit log: audit-log.md - Public API: public-api.md - Webhooks: webhooks.md diff --git a/scripts/db_setup.py b/scripts/db_setup.py index 5f39e73..24ed542 100644 --- a/scripts/db_setup.py +++ b/scripts/db_setup.py @@ -61,9 +61,10 @@ def ensure_app_role(owner_url: str, role: str, password: str) -> None: # Migrations are not for the application role. cur.execute(sql.SQL("REVOKE ALL ON alembic_version FROM {}").format(ident)) # The only lookups allowed before a tenant is known (login, refresh, invitations: migration - # 015; API keys: migration 019). + # 015; API keys: migration 019; password resets: migration 022). for fn in ("scirm_login_lookup(text)", "scirm_user_by_id(uuid)", "scirm_invitation_lookup(text)", - "scirm_accept_invitation(text, text, text)", "scirm_api_key_lookup(text)"): + "scirm_accept_invitation(text, text, text)", "scirm_api_key_lookup(text)", + "scirm_password_reset_request(text, text, timestamptz)", "scirm_password_reset_consume(text, text)"): cur.execute(sql.SQL("GRANT EXECUTE ON FUNCTION " + fn + " TO {}").format(ident)) conn.close() diff --git a/tests/db/test_database.py b/tests/db/test_database.py index dad82b7..3a9dd87 100644 --- a/tests/db/test_database.py +++ b/tests/db/test_database.py @@ -222,10 +222,11 @@ def test_lookups_work_when_the_owner_is_not_a_superuser(self, db): with db["owner"].cursor() as cur: cur.execute("DROP ROLE IF EXISTS scirm_plain_owner") cur.execute("CREATE ROLE scirm_plain_owner NOSUPERUSER NOBYPASSRLS") - for obj in ("TABLE users", "TABLE invitations", "TABLE organizations", "TABLE api_keys"): + for obj in ("TABLE users", "TABLE invitations", "TABLE organizations", "TABLE api_keys", "TABLE password_resets"): cur.execute(f"ALTER {obj} OWNER TO scirm_plain_owner") for fn in ("scirm_login_lookup(text)", "scirm_user_by_id(uuid)", "scirm_invitation_lookup(text)", - "scirm_accept_invitation(text, text, text)", "scirm_api_key_lookup(text)"): + "scirm_accept_invitation(text, text, text)", "scirm_api_key_lookup(text)", + "scirm_password_reset_request(text, text, timestamptz)", "scirm_password_reset_consume(text, text)"): cur.execute(f"ALTER FUNCTION {fn} OWNER TO scirm_plain_owner") try: rows = as_org(db["app"], "", "SELECT email FROM scirm_login_lookup('managed@example.com')") @@ -233,14 +234,20 @@ def test_lookups_work_when_the_owner_is_not_a_superuser(self, db): as_org(db["app"], db["a"], "INSERT INTO api_keys (organization_id, name, prefix, key_hash) " "VALUES (%s, 'm', 'scirm_ABCDEFGH', %s)", (db["a"], "9" * 64)) assert as_org(db["app"], "", "SELECT name FROM scirm_api_key_lookup(%s)", ("9" * 64,)) == [("m",)] + reset = "SELECT email FROM scirm_password_reset_request('managed@example.com', %s, now() + interval '5 minutes')" + assert as_org(db["app"], "", reset, ("8" * 64,)) == [("managed@example.com",)] + consume = "SELECT email FROM scirm_password_reset_consume(%s, 'new-hash')" + assert as_org(db["app"], "", consume, ("8" * 64,)) == [("managed@example.com",)] + assert as_org(db["app"], "", consume, ("8" * 64,)) == [] # once finally: with db["owner"].cursor() as cur: cur.execute("SELECT current_user") me = cur.fetchone()[0] - for obj in ("TABLE users", "TABLE invitations", "TABLE organizations", "TABLE api_keys"): + for obj in ("TABLE users", "TABLE invitations", "TABLE organizations", "TABLE api_keys", "TABLE password_resets"): cur.execute(f"ALTER {obj} OWNER TO {me}") for fn in ("scirm_login_lookup(text)", "scirm_user_by_id(uuid)", "scirm_invitation_lookup(text)", - "scirm_accept_invitation(text, text, text)", "scirm_api_key_lookup(text)"): + "scirm_accept_invitation(text, text, text)", "scirm_api_key_lookup(text)", + "scirm_password_reset_request(text, text, timestamptz)", "scirm_password_reset_consume(text, text)"): cur.execute(f"ALTER FUNCTION {fn} OWNER TO {me}") def test_an_invitation_can_be_accepted_only_once_even_concurrently(self, db): diff --git a/tests/security/test_passwords.py b/tests/security/test_passwords.py new file mode 100644 index 0000000..ae25739 --- /dev/null +++ b/tests/security/test_passwords.py @@ -0,0 +1,112 @@ +""" +Password change and reset actually change the password, reset links work once and expire, +answers do not reveal which accounts exist, and other sessions are signed out. +Runs on both backends, like the rest of the security suite. +""" + +import asyncio +import re +import time +import uuid +from datetime import datetime, timedelta, timezone + +import pytest + +from libs.integrations import reports + + +def bearer(token: str) -> dict: + return {"Authorization": f"Bearer {token}"} + + +@pytest.fixture +def member(client, sarah): + email = f"pw-{uuid.uuid4().hex[:8]}@example.com" + invited = client.post("/api/v1/org/invitations", json={"email": email, "roles": ["analyst"]}, + headers=bearer(sarah["access_token"])) + token = invited.json()["invite_url"].split("#", 1)[1] + tokens = client.post("/api/v1/invitations/accept", json={"token": token, "name": "P", "password": "first-password"}).json() + return {"email": email, "tokens": tokens} + + +@pytest.fixture +def outbox(monkeypatch): + sent = [] + monkeypatch.setenv("SMTP_HOST", "mail.example.com") + monkeypatch.setattr(reports, "_send_sync", lambda config, to, subject, text, html: sent.append( + {"to": to, "subject": subject, "text": text})) + return sent + + +def wait_for(outbox, n, timeout=5.0): + end = time.time() + timeout + while len(outbox) < n and time.time() < end: + time.sleep(0.05) + return outbox + + +class TestChange: + def test_change_password_really_changes_it_and_signs_out_other_sessions(self, client, login, member): + old_refresh = member["tokens"]["refresh_token"] + time.sleep(1.1) # token times are in whole seconds + changed = client.post("/api/v1/auth/change-password", headers=bearer(member["tokens"]["access_token"]), + json={"current_password": "first-password", "new_password": "second-password"}) + assert changed.status_code == 200, changed.text + assert "access_token" in changed.json() and "refresh_token" in changed.json() + assert login(member["email"], "first-password").status_code == 401 + assert login(member["email"], "second-password").status_code == 200 + assert client.post("/api/v1/auth/refresh", json={"refresh_token": old_refresh}).status_code == 401 + assert client.post("/api/v1/auth/refresh", json={"refresh_token": changed.json()["refresh_token"]}).status_code == 200 + + @pytest.mark.parametrize("body", [{"current_password": "wrong-password", "new_password": "second-password"}, + {"current_password": "first-password", "new_password": "first-password"}, + {"current_password": "first-password", "new_password": "short"}]) + def test_wrong_current_same_or_short_is_refused(self, client, login, member, body): + assert client.post("/api/v1/auth/change-password", headers=bearer(member["tokens"]["access_token"]), + json=body).status_code == 400 + assert login(member["email"], "first-password").status_code == 200 + + def test_demo_accounts_cannot(self, client, sarah): + assert client.post("/api/v1/auth/change-password", headers=bearer(sarah["access_token"]), + json={"current_password": "x", "new_password": "whatever-123"}).status_code == 400 + + +class TestReset: + def test_without_email_it_says_so(self, client, monkeypatch): + monkeypatch.delenv("SMTP_HOST", raising=False) + assert client.post("/api/v1/auth/forgot-password", json={"email": "anyone@example.com"}).status_code == 503 + + def test_the_answer_does_not_reveal_accounts(self, client, member, outbox): + known = client.post("/api/v1/auth/forgot-password", json={"email": member["email"]}) + unknown = client.post("/api/v1/auth/forgot-password", json={"email": f"nobody-{uuid.uuid4().hex[:6]}@example.com"}) + assert known.status_code == unknown.status_code == 200 and known.json() == unknown.json() + mails = wait_for(outbox, 1) + assert [m["to"] for m in mails] == [[member["email"]]] + + def test_a_reset_link_works_once_and_signs_out_other_sessions(self, client, login, member, outbox): + time.sleep(1.1) + client.post("/api/v1/auth/forgot-password", json={"email": member["email"]}) + token = re.search(r"/reset-password#([\w-]+)", wait_for(outbox, 1)[-1]["text"]).group(1) + done = client.post("/api/v1/auth/reset-password", json={"token": token, "new_password": "reset-password-1"}) + assert done.status_code == 200, done.text + assert login(member["email"], "reset-password-1").status_code == 200 + assert login(member["email"], "first-password").status_code == 401 + assert client.post("/api/v1/auth/refresh", json={"refresh_token": member["tokens"]["refresh_token"]}).status_code == 401 + again = client.post("/api/v1/auth/reset-password", json={"token": token, "new_password": "reset-password-2"}) + assert again.status_code == 400 + + def test_a_newer_request_invalidates_the_older_link(self, client, member, outbox): + client.post("/api/v1/auth/forgot-password", json={"email": member["email"]}) + first = re.search(r"#([\w-]+)", wait_for(outbox, 1)[-1]["text"]).group(1) + client.post("/api/v1/auth/forgot-password", json={"email": member["email"]}) + second = re.search(r"#([\w-]+)", wait_for(outbox, 2)[-1]["text"]).group(1) + assert client.post("/api/v1/auth/reset-password", json={"token": first, "new_password": "x-password-1"}).status_code == 400 + assert client.post("/api/v1/auth/reset-password", json={"token": second, "new_password": "x-password-2"}).status_code == 200 + + def test_an_expired_link_is_refused(self, client, apps, member): + import hashlib + token = "expired-" + uuid.uuid4().hex + past = (datetime.now(timezone.utc) - timedelta(minutes=1)).isoformat() + asyncio.run(apps["gateway"].audit_store.request_password_reset(member["email"], + hashlib.sha256(token.encode()).hexdigest(), past)) + assert client.post("/api/v1/auth/reset-password", json={"token": token, "new_password": "late-password"}).status_code == 400