From f552f215c7c3c85ffc875dec4bdbb0a1982dabd9 Mon Sep 17 00:00:00 2001 From: alovladi007 <83262803+alovladi007@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:35:07 -0400 Subject: [PATCH] =?UTF-8?q?feat(users):=20Phase=207b=20=E2=80=94=20admin?= =?UTF-8?q?=20user=20management=20+=20email=20invitations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After bootstrap_admin, an admin had no way to add a second user: the users API was GET-only. This completes org administration on the Phase 7a token/mailer foundation. Backend (services/lims/app/api/users.py): - POST /api/users/ (admin only): creates an ACTIVE, passwordless user and emails a 7-day single-use set-password link (the shared /reset-password page terminates both reset and invite flows via &welcome=1). Duplicate email in org → 409. Plain-str email shape check, NOT pydantic EmailStr — that type imports email-validator, which the lims image doesn't ship (found live: reload crash). - PATCH /api/users/{id} (admin only, own org): name/role/is_active. Self-protection: you cannot demote or deactivate your own account, so an org can never brick its last admin. Cross-org ids are 404. - issue_password_token imported RELATIVELY (.auth): the package is `app.api` in the container and `services.lims.app.api` under tests — the absolute spelling 500'd in the container (found live). Frontend (/system/users): - Invite dialog (email/name/role) on the real POST; role select + activate/deactivate in the details dialog on the real PATCH; backend errors (409, self-protection 400s) surface verbatim; react-query invalidation keeps the list fresh. Plumbing: - compose lims env: SMTP_* + APP_BASE_URL passthrough (unset = honest log-only dev mode). - k8s lims Deployment: APP_BASE_URL + SMTP_* from the OPTIONAL smtp-credentials secret (template + runbook decision row added) — the platform deploys and runs without email, it just logs instead. Proof: 10 new API tests (invite lifecycle incl. completion via the reset endpoint, RBAC 403s, 409, self-protection 400s, cross-org 404) — lims suite 90 passed. LIVE on the dev stack: invite 201 → link from log → set password 204 → invitee logs in 200; engineer invite 403. tsc clean; smoke 4/4; compose config valid; overlays render. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/web/src/app/system/users/page.tsx | 170 +++++++++++++++++- apps/web/src/lib/api-client.ts | 15 ++ docker-compose.yml | 10 ++ docs/deployment/PRODUCTION_RUNBOOK.md | 1 + .../secrets/smtp-credentials-template.yaml | 26 +++ k8s/base/services.yaml | 36 ++++ services/lims/app/api/users.py | 156 +++++++++++++++- .../tests/integration/test_users_admin.py | 143 +++++++++++++++ 8 files changed, 548 insertions(+), 9 deletions(-) create mode 100644 k8s/base/secrets/smtp-credentials-template.yaml create mode 100644 services/lims/tests/integration/test_users_admin.py diff --git a/apps/web/src/app/system/users/page.tsx b/apps/web/src/app/system/users/page.tsx index d5dd4804..89c1ee2a 100644 --- a/apps/web/src/app/system/users/page.tsx +++ b/apps/web/src/app/system/users/page.tsx @@ -1,10 +1,10 @@ 'use client' import { useState, useMemo } from 'react' -import { useQuery } from '@tanstack/react-query' +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import { Users, Search, Shield, CheckCircle2, XCircle, - Eye, Key, Clock, Loader2, AlertCircle + Eye, Key, Clock, Loader2, AlertCircle, UserPlus, Mail } from 'lucide-react' import { limsAPI, getErrorMessage, type LimsUser } from '@/lib/api-client' import { Button } from '@/components/ui/button' @@ -59,6 +59,49 @@ export default function UsersPage() { const [selectedUser, setSelectedUser] = useState(null) const [showDetailsDialog, setShowDetailsDialog] = useState(false) + // Phase 7b: admin actions (invite + role/status changes). The backend + // enforces admin-only and self-protection; UI errors surface verbatim. + const queryClient = useQueryClient() + const [showInviteDialog, setShowInviteDialog] = useState(false) + const [inviteEmail, setInviteEmail] = useState('') + const [inviteName, setInviteName] = useState('') + const [inviteRole, setInviteRole] = useState('viewer') + const [inviteSent, setInviteSent] = useState(false) + const [actionError, setActionError] = useState('') + + const invalidateUsers = () => queryClient.invalidateQueries({ queryKey: ['lims', 'users'] }) + + const inviteMutation = useMutation({ + mutationFn: () => + limsAPI.users.create({ email: inviteEmail, name: inviteName, role: inviteRole }), + onSuccess: () => { + setInviteSent(true) + setActionError('') + invalidateUsers() + }, + onError: (err) => setActionError(getErrorMessage(err)), + }) + + const updateMutation = useMutation({ + mutationFn: (vars: { id: string; data: { role?: string; is_active?: boolean } }) => + limsAPI.users.update(vars.id, vars.data), + onSuccess: (updated) => { + setActionError('') + setSelectedUser(toUserVM(updated)) + invalidateUsers() + }, + onError: (err) => setActionError(getErrorMessage(err)), + }) + + const openInvite = () => { + setInviteEmail('') + setInviteName('') + setInviteRole('viewer') + setInviteSent(false) + setActionError('') + setShowInviteDialog(true) + } + // Real users from the LIMS service (read-only list). const { data: rawUsers, @@ -145,6 +188,10 @@ export default function UsersPage() {

Manage user accounts and permissions

+ {/* Statistics */} @@ -364,11 +411,130 @@ export default function UsersPage() { )} + {selectedUser && ( +
+ + {actionError && ( +
+ {actionError} +
+ )} +
+ + +
+

+ Role and status changes apply immediately. You cannot demote or + deactivate your own account. +

+
+ )} + + + {/* Invite Dialog (Phase 7b) */} + + + + Invite a user + + {inviteSent ? ( +
+ + Invitation created for {inviteEmail}. They'll + receive a set-password link valid for 7 days. +
+ ) : ( +
+ {actionError && ( +
+ {actionError} +
+ )} +
+ + setInviteEmail(e.target.value)} + placeholder="person@yourlab.com" + /> +
+
+ + setInviteName(e.target.value)} + placeholder="Full name" + /> +
+
+ + +
+
+ )} + + {inviteSent ? ( + + ) : ( + <> + + + + )} + + + ) } diff --git a/apps/web/src/lib/api-client.ts b/apps/web/src/lib/api-client.ts index ef677dfd..e3127c0e 100644 --- a/apps/web/src/lib/api-client.ts +++ b/apps/web/src/lib/api-client.ts @@ -810,6 +810,21 @@ export const limsAPI = { // Users & roles directory (org-scoped). Backend: services/lims/app/api/users.py users: { list: () => fetchAPI('lims', '/api/v1/lims/users'), + // Phase 7b: admin-only. Create = invite (backend emails the + // set-password link); update covers name/role/is_active. + create: (data: { email: string; name: string; role: string }) => + fetchAPI('lims', '/api/v1/lims/users', { + method: 'POST', + body: JSON.stringify(data), + }), + update: ( + userId: string, + data: { name?: string; role?: string; is_active?: boolean } + ) => + fetchAPI('lims', `/api/v1/lims/users/${userId}`, { + method: 'PATCH', + body: JSON.stringify(data), + }), }, // 21 CFR Part 11 e-signature ledger. Backend: services/lims/app/api/signatures.py diff --git a/docker-compose.yml b/docker-compose.yml index bb050aee..9a779ffd 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -258,6 +258,16 @@ services: - JWT_SECRET=${JWT_SECRET:?JWT_SECRET env var is required (see .env.example)} - JWT_ALGORITHM=HS256 - JWT_ISSUER=spectra-lab + # Outbound email (Phase 7 — account lifecycle). Unset SMTP_HOST = + # dev mode: emails are logged, not sent (reset links readable via + # `docker logs spectra-lims`). + - SMTP_HOST=${SMTP_HOST:-} + - SMTP_PORT=${SMTP_PORT:-587} + - SMTP_USERNAME=${SMTP_USERNAME:-} + - SMTP_PASSWORD=${SMTP_PASSWORD:-} + - SMTP_STARTTLS=${SMTP_STARTTLS:-true} + - SMTP_FROM=${SMTP_FROM:-no-reply@spectra-lab.local} + - APP_BASE_URL=${APP_BASE_URL:-http://localhost:3012} # Phase 6.6: env-driven SSO. Off by default; flip via .env — see # infra/keycloak/README.md. Issuer/JWKS use the compose-internal # keycloak DNS so in-container JWKS fetches work; override for k8s. diff --git a/docs/deployment/PRODUCTION_RUNBOOK.md b/docs/deployment/PRODUCTION_RUNBOOK.md index 73c1d4ab..77bb36cf 100644 --- a/docs/deployment/PRODUCTION_RUNBOOK.md +++ b/docs/deployment/PRODUCTION_RUNBOOK.md @@ -26,6 +26,7 @@ Related docs: [SECRETS.md](SECRETS.md) (Sealed Secrets flow), | Object storage | Start with in-cluster MinIO (ships in `k8s/base/minio.yaml`). Swap `OBJECT_STORE_ENDPOINT` to managed S3 later — the app speaks plain S3 either way. | | SSO | Skip at first (`OIDC_ENABLED=false`, built-in auth). Add Keycloak later per AUTH.md. | | Domain | You need one you control, plus the ability to add DNS records. | +| Email (SMTP) | Optional at first: without an `smtp-credentials` secret, reset/invite emails are logged by the lims pod instead of sent. Any relay works (SES, Mailgun, your org's). See `k8s/base/secrets/smtp-credentials-template.yaml`. | Rough monthly cost at the small end (DO): 3-node cluster (~$72) + load balancer (~$12) + volumes (~$5) ≈ **$90/month**. diff --git a/k8s/base/secrets/smtp-credentials-template.yaml b/k8s/base/secrets/smtp-credentials-template.yaml new file mode 100644 index 00000000..687fd276 --- /dev/null +++ b/k8s/base/secrets/smtp-credentials-template.yaml @@ -0,0 +1,26 @@ +# TEMPLATE — do not apply as-is. OPTIONAL secret: without it the platform +# still runs, but reset/invite emails are logged by the lims pod instead of +# sent (`kubectl logs deploy/lims-service | grep "EMAIL NOT SENT"`). +# +# Any SMTP relay works (SES, Mailgun, Postmark, your org's relay). Create +# it with real values, or seal it per docs/deployment/SECRETS.md: +# +# kubectl create secret generic smtp-credentials \ +# --namespace spectra-lab \ +# --from-literal=host=smtp.example.com \ +# --from-literal=port=587 \ +# --from-literal=username= \ +# --from-literal=password= \ +# --from-literal=from=no-reply@yourdomain.com +apiVersion: v1 +kind: Secret +metadata: + name: smtp-credentials + namespace: spectra-lab +type: Opaque +stringData: + host: smtp.example.com + port: "587" + username: CHANGE_ME + password: CHANGE_ME_IN_PRODUCTION + from: no-reply@example.com diff --git a/k8s/base/services.yaml b/k8s/base/services.yaml index 6b558338..66a8e4db 100644 --- a/k8s/base/services.yaml +++ b/k8s/base/services.yaml @@ -153,6 +153,42 @@ spec: secretKeyRef: name: minio-credentials key: root-password + # Outbound email (Phase 7 — account lifecycle). APP_BASE_URL is + # the public web URL used in reset/invite links — set it to your + # real host (runbook step 4). SMTP creds come from the OPTIONAL + # smtp-credentials secret: absent = emails logged, not sent. + - name: APP_BASE_URL + value: "https://spectra-lab.example.com" + - name: SMTP_HOST + valueFrom: + secretKeyRef: + name: smtp-credentials + key: host + optional: true + - name: SMTP_PORT + valueFrom: + secretKeyRef: + name: smtp-credentials + key: port + optional: true + - name: SMTP_USERNAME + valueFrom: + secretKeyRef: + name: smtp-credentials + key: username + optional: true + - name: SMTP_PASSWORD + valueFrom: + secretKeyRef: + name: smtp-credentials + key: password + optional: true + - name: SMTP_FROM + valueFrom: + secretKeyRef: + name: smtp-credentials + key: from + optional: true resources: requests: memory: "256Mi" diff --git a/services/lims/app/api/users.py b/services/lims/app/api/users.py index 89089336..e6a50f4f 100644 --- a/services/lims/app/api/users.py +++ b/services/lims/app/api/users.py @@ -1,17 +1,26 @@ -"""User-directory endpoint — list the users in the caller's organization. +"""User-directory endpoints — list, invite, and administer org users. -Read-only, org-scoped (a user only ever sees their own org's directory). Backs -the System → Users & Roles page, which previously showed mock users. +Org-scoped (a user only ever sees their own org's directory). Backs the +System → Users & Roles page. Listing is open to any member; creating and +updating users is admin-only (Phase 7b — account lifecycle): + +- POST / invites a user: the account is created active with NO password; + a 7-day single-use invite token is emailed as a set-password link (the + same /reset-password page terminates both flows). +- PATCH /{id} updates name/role/is_active, with self-protection — an + admin cannot demote or deactivate their own account, so an org can + never lock out its last administrator by accident. """ -from fastapi import APIRouter, Depends, Query +from fastapi import APIRouter, Depends, HTTPException, Query, status from sqlalchemy.orm import Session from typing import List, Optional -from pydantic import BaseModel, UUID4 +from pydantic import BaseModel, UUID4, field_validator from datetime import datetime -from services.shared.db.deps import get_db, get_current_user -from services.shared.db.models import User +from services.shared.db.deps import get_db, get_current_user, require_admin +from services.shared.db.models import User, UserRole +from services.shared.mailer import app_base_url, send_email router = APIRouter(prefix="/api/users", tags=["users"]) @@ -30,6 +39,30 @@ class Config: from_attributes = True +class UserCreateRequest(BaseModel): + # Plain str + a minimal shape check, NOT pydantic's EmailStr — that + # type imports the email-validator package, which the lims image does + # not ship (adding a dependency for one field isn't worth it; a bad + # address just means the invite email won't deliver). + email: str + name: str + role: UserRole = UserRole.VIEWER + + @field_validator("email") + @classmethod + def _email_shape(cls, v: str) -> str: + v = v.strip().lower() + if "@" not in v[1:-1] or " " in v: + raise ValueError("not a valid email address") + return v + + +class UserUpdateRequest(BaseModel): + name: Optional[str] = None + role: Optional[UserRole] = None + is_active: Optional[bool] = None + + @router.get("/", response_model=List[UserResponse]) def list_users( skip: int = Query(0, ge=0), @@ -46,3 +79,112 @@ def list_users( .limit(limit) .all() ) + + +@router.post("/", response_model=UserResponse, status_code=status.HTTP_201_CREATED) +def create_user( + body: UserCreateRequest, + db: Session = Depends(get_db), + current_user: User = Depends(require_admin), +): + """Invite a user into the caller's organization (admin only). + + The account is created active with no password (password login is + impossible until they set one); a 7-day invite token is emailed as a + set-password link. Email is unique per org — a duplicate is 409. + """ + exists = ( + db.query(User) + .filter( + User.organization_id == current_user.organization_id, + User.email == body.email, + ) + .first() + ) + if exists is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="A user with this email already exists in your organization", + ) + + user = User( + organization_id=current_user.organization_id, + email=body.email, + name=body.name, + role=body.role, + password_hash=None, # set via the invite link + is_active=True, + ) + db.add(user) + db.flush() # user.id for the token row + + # Relative import: this package is `app.api` inside the container and + # `services.lims.app.api` in the repo-root test environment — a relative + # import is the only spelling correct in both. Local (not module-level) + # to keep the auth→users dependency one-directional at import time. + from .auth import issue_password_token + + raw = issue_password_token(db, user, purpose="invite") + db.commit() + db.refresh(user) + + send_email( + to=user.email, + subject=f"You've been invited to SPECTRA-Lab by {current_user.name}", + body_text=( + f"Hello {user.name},\n\n" + f"{current_user.name} ({current_user.email}) invited you to their\n" + f"SPECTRA-Lab organization as {user.role.value}. Set your password\n" + f"with the link below — it is valid for 7 days:\n\n" + f"{app_base_url()}/reset-password?token={raw}&welcome=1\n\n" + f"After setting a password, sign in at {app_base_url()}/login." + ), + ) + return user + + +@router.patch("/{user_id}", response_model=UserResponse) +def update_user( + user_id: UUID4, + body: UserUpdateRequest, + db: Session = Depends(get_db), + current_user: User = Depends(require_admin), +): + """Update a user's name, role, or active flag (admin only, own org). + + Self-protection: an admin cannot demote or deactivate THEMSELVES — + otherwise an org's only admin could brick its administration. Another + admin can still do either to them. + """ + user = ( + db.query(User) + .filter( + User.id == user_id, + User.organization_id == current_user.organization_id, + ) + .first() + ) + if user is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found") + + if user.id == current_user.id: + if body.role is not None and body.role != UserRole.ADMIN: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="You cannot demote your own admin account", + ) + if body.is_active is False: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="You cannot deactivate your own account", + ) + + if body.name is not None: + user.name = body.name + if body.role is not None: + user.role = body.role + if body.is_active is not None: + user.is_active = body.is_active + db.commit() + db.refresh(user) + return user diff --git a/services/lims/tests/integration/test_users_admin.py b/services/lims/tests/integration/test_users_admin.py new file mode 100644 index 00000000..e2979ef5 --- /dev/null +++ b/services/lims/tests/integration/test_users_admin.py @@ -0,0 +1,143 @@ +"""Admin user management + invite flow (Phase 7b — account lifecycle). + +Covers the invite path end-to-end (create → token row → set password via +the shared reset endpoint → login), RBAC (non-admins 403), duplicate +emails, updates, and the self-protection rules. +""" + +from __future__ import annotations + +import pytest + +from services.lims.tests.conftest import * # noqa: F401,F403 +from services.shared.db.models import PasswordResetToken, User + +INVITE = {"email": "newhire@acme.com", "name": "New Hire", "role": "engineer"} + + +@pytest.fixture(autouse=True) +def _reset_rate_limiter(): + """The invite completion path posts to /api/auth/reset-password, which + shares the 5/min login tier.""" + from services.shared.middleware.rate_limit import limiter + + limiter.reset() + yield + + +def test_invite_creates_active_passwordless_user( + lims_client, db_session, admin_user, admin_token, auth_header +): + r = lims_client.post("/api/users/", json=INVITE, headers=auth_header(admin_token)) + assert r.status_code == 201 + body = r.json() + assert body["email"] == INVITE["email"] + assert body["role"] == "engineer" + assert body["is_active"] is True + + user = db_session.query(User).filter(User.email == INVITE["email"]).one() + assert user.password_hash is None # no password until the link is used + + token = db_session.query(PasswordResetToken).filter(PasswordResetToken.user_id == user.id).one() + assert token.purpose == "invite" + assert token.used_at is None + + +def test_invited_user_cannot_login_before_setting_password(lims_client, admin_token, auth_header): + lims_client.post("/api/users/", json=INVITE, headers=auth_header(admin_token)) + r = lims_client.post( + "/api/auth/login", data={"username": INVITE["email"], "password": "anything-at-all-1"} + ) + assert r.status_code == 401 + + +def test_invite_completion_full_round_trip( + lims_client, db_session, admin_user, admin_token, auth_header +): + """Invite → set password through the shared reset endpoint → login.""" + lims_client.post("/api/users/", json=INVITE, headers=auth_header(admin_token)) + user = db_session.query(User).filter(User.email == INVITE["email"]).one() + + # Mint the raw link token the same way the endpoint does (raw tokens + # are never stored; see the password-reset suite for the rationale). + from services.lims.app.api.auth import issue_password_token + + raw = issue_password_token(db_session, user, purpose="invite") + db_session.commit() + + r = lims_client.post( + "/api/auth/reset-password", + json={"token": raw, "new_password": "welcome-aboard-pw-1"}, + ) + assert r.status_code == 204 + + r = lims_client.post( + "/api/auth/login", + data={"username": INVITE["email"], "password": "welcome-aboard-pw-1"}, + ) + assert r.status_code == 200 + + +def test_invite_requires_admin(lims_client, engineer_token, auth_header): + r = lims_client.post("/api/users/", json=INVITE, headers=auth_header(engineer_token)) + assert r.status_code == 403 + + +def test_invite_duplicate_email_409(lims_client, admin_token, auth_header, engineer_user): + dup = {**INVITE, "email": engineer_user.email} + r = lims_client.post("/api/users/", json=dup, headers=auth_header(admin_token)) + assert r.status_code == 409 + + +def test_update_role_and_deactivate(lims_client, admin_token, auth_header, engineer_user): + r = lims_client.patch( + f"/api/users/{engineer_user.id}", + json={"role": "pi", "is_active": False}, + headers=auth_header(admin_token), + ) + assert r.status_code == 200 + assert r.json()["role"] == "pi" + assert r.json()["is_active"] is False + + # Deactivated users cannot log in. + r = lims_client.post( + "/api/auth/login", data={"username": engineer_user.email, "password": "eng123"} + ) + assert r.status_code == 401 + + +def test_update_requires_admin(lims_client, engineer_token, auth_header, viewer_user): + r = lims_client.patch( + f"/api/users/{viewer_user.id}", + json={"role": "engineer"}, + headers=auth_header(engineer_token), + ) + assert r.status_code == 403 + + +def test_admin_cannot_demote_self(lims_client, admin_user, admin_token, auth_header): + r = lims_client.patch( + f"/api/users/{admin_user.id}", + json={"role": "viewer"}, + headers=auth_header(admin_token), + ) + assert r.status_code == 400 + + +def test_admin_cannot_deactivate_self(lims_client, admin_user, admin_token, auth_header): + r = lims_client.patch( + f"/api/users/{admin_user.id}", + json={"is_active": False}, + headers=auth_header(admin_token), + ) + assert r.status_code == 400 + + +def test_update_cross_org_user_404(lims_client, admin_token, auth_header, org2_engineer): + """Org isolation: an admin cannot even see another org's user.""" + r = lims_client.patch( + f"/api/users/{org2_engineer.id}", + json={"is_active": False}, + headers=auth_header(admin_token), + ) + assert r.status_code == 404