diff --git a/.env.example b/.env.example index afdc620..4999f7a 100644 --- a/.env.example +++ b/.env.example @@ -19,6 +19,11 @@ BACKEND_PORT=8000 # Generate a strong random value in your real .env (e.g. `openssl rand -hex 32`): SECRET_KEY=change-me-generate-a-random-secret +# ---- Authentication ---------------------------------------------------------- +# How long a session token from POST /auth/login stays valid. Short enough that a leaked +# token expires on its own, long enough not to interrupt a working day. +AUTH_TOKEN_TTL_HOURS=12 + # ---- Frontend ---------------------------------------------------------------- FRONTEND_PORT=3000 # The UI fetches findings in a Server Component, so this URL is resolved from inside the diff --git a/CHANGELOG.md b/CHANGELOG.md index 970d961..90653d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,51 @@ the console, and `make version-set` moves them together (issue #31). updating `docs/STATUS.md` and adding a `[Unreleased]` changelog entry. Mirrored in `.github/PULL_REQUEST_TEMPLATE.md`. +### Fixed +- **Validation errors no longer echo the submitted password or credential value.** FastAPI's + default handler returned Pydantic's error list verbatim, and a Pydantic `missing` error + carries the whole request object in `input`. `POST /auth/login` with the username omitted + returned the submitted password; `POST /engagements/{id}/credential` with `cred_type` + omitted returned the submitted scanning credential; `api_spec_inline` echoed the same way. + The credential case had been live for as long as the endpoint has existed (rule PX-SECRETS). + What comes back is the caller's own input, not stored state: validation runs before the + handler, so nothing is read from the database. It still matters, because a secret in a + response body ends up in proxy access logs, error trackers, and CI output (CWE-209). + A single `RequestValidationError` handler now serves every endpoint, keeping `type`/`loc`/ + `msg` and dropping `input`/`ctx`/`url`; the response shape and the CLI's per-field error + rendering are unchanged. See KI-010 in `docs/KNOWN_ISSUES.md`. + ### Added +- Authentication **foundation**: operator identity, roles, and sessions, proven on a small + set of endpoints rather than rolled out across the API. New `auth_user`, `auth_token`, + `auth_bootstrap`, and `auth_event` tables with a reversible migration (rule W-03). + Passwords are hashed with argon2id (`argon2-cffi`, MIT, at OWASP's 64 MiB / t=3 / p=4 + baseline) and are never recoverable — deliberately not built on the reversible + evidence-crypto layer, which exists for a different job. Session tokens are 256 random + bits stored as SHA-256 only, so a stolen database row cannot be replayed as a bearer + token. `POST /auth/login` issues one token delivered two ways, in the response body and + as an httpOnly / SameSite / Secure cookie (rule A-03); `POST /auth/logout` revokes the + presenting session; `GET /auth/me` reports identity and role and never the hash. + `POST /auth/bootstrap` creates the first admin on a deployment that has none and then + refuses permanently — guarded by a fixed-primary-key marker row, so two concurrent + first-run requests cannot both win and deleting the admin does not re-open the door. + Every authentication action, including a failed login, writes one append-only + `auth_event` row (rules PX-SECRETS, PX-EVIDENCE, D-05). +- Roles `admin` / `operator` / `viewer` as an enum, with the whole authorization policy + encoded as a capability table in `app/api/deps.py` rather than as checks scattered + through handlers, plus `require_role` and `require_capability` dependencies (rule + B-FA-03). **Enforced on `GET /auth/me`, `POST /auth/logout`, and the admin-only + `POST /users` and `GET /users` — and on nothing else.** Every engagement and scan route + is deliberately unchanged and remains unauthenticated; applying the gate across them is + the next change, and `docs/STATUS.md` lists exactly which routes are which. +- `provx login`, `provx logout`, and `provx admin create`, with an on-disk token store at + `$XDG_CONFIG_HOME/provx/credentials.json` created owner-readable only. No password flag + exists on any command — the secret comes from a hidden prompt, stdin, or + `--password-env VAR` (rule PX-SECRETS) — and the parser-introspection test that enforced + that for `credential set` now walks the entire command tree. `$PROVX_TOKEN` still works + and takes precedence over a stored session. +- `AUTH_TOKEN_TTL_HOURS` (default 12), and a first-run walkthrough in + `docs/QUICKSTART.md` §4. - One version for the whole monorepo, declared once per package and enforced by CI. Each Python package takes its version from its own `__init__.py` via `dynamic = ["version"]`, the new required `version-consistency` gate refuses any disagreement between declaration diff --git a/README.md b/README.md index da5e338..382ddaf 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,13 @@ The machine proposes findings; a human always confirms before anything is report > reachable from the `provx` CLI, the web console, and the HTTP API. A lab of vulnerable and > clean targets gates accuracy on TP/FP/FN in CI. > -> **Not built:** authentication on the API itself (do not expose it), the job queue, the +> **Partly built — authentication.** Operator accounts, argon2id passwords, admin/operator/ +> viewer roles, hashed session tokens, and `provx login` all exist. They are **enforced on +> `/auth/me`, `/auth/logout`, and `/users`, and on nothing else** — every engagement and scan +> route is still open. Applying the gate across them is the next change, so **do not expose +> this server**. [`docs/STATUS.md`](docs/STATUS.md) lists exactly which routes are which. +> +> **Not built:** the job queue, the > playbook execution engine, exploitation, Word/`.docx` export, dashboard depth beyond the > engagement list, and any AI feature. See [`docs/STATUS.md`](docs/STATUS.md) for the > plan-of-record, [`docs/ROADMAP.md`](docs/ROADMAP.md) §4 for MVP scope, and @@ -95,8 +101,10 @@ docker compose up --build ``` The API is at `http://localhost:8000` (interactive docs at `/docs`) and the web console at -`http://localhost:3000`. Migrations are applied on start. Note that **the API has no -authentication yet** — keep it on loopback and off shared networks. +`http://localhost:3000`. Migrations are applied on start. Note that **the engagement and +scan routes are still unauthenticated** — accounts and the role gate exist, but are enforced +only on `/auth/me`, `/auth/logout`, and `/users` so far. Keep the server on loopback and off +shared networks. Run `provx admin create --username root` on first start. **→ [`docs/QUICKSTART.md`](docs/QUICKSTART.md) walks the full first scan end to end** — a deliberately-vulnerable practice target you are unambiguously allowed to scan, then diff --git a/SECURITY.md b/SECURITY.md index 24867a7..76c7511 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -10,13 +10,15 @@ responsibly. ## Supported versions -Provx is pre-1.0 and under active development. Until the first tagged release, only -the `main` branch is supported. Once releases exist, this table will list the versions -that receive security fixes. +Provx is pre-1.0 and under active development. Security fixes land on `main` and reach +users in the next tagged release; there are no backports to older tags, because pre-1.0 +the upgrade path is forward. | Version | Supported | |---|---| -| `main` (unreleased) | ✅ | +| `main` | ✅ | +| `v0.1.1` (latest release) | ✅ | +| `v0.1.0` and earlier | ❌ upgrade to the latest tag | ## Reporting a vulnerability diff --git a/backend/alembic/versions/c3d4e5f6a7b8_add_auth_tables.py b/backend/alembic/versions/c3d4e5f6a7b8_add_auth_tables.py new file mode 100644 index 0000000..28c72cc --- /dev/null +++ b/backend/alembic/versions/c3d4e5f6a7b8_add_auth_tables.py @@ -0,0 +1,107 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +"""authentication foundation: users, tokens, bootstrap marker, audit trail + +Adds ``auth_user`` (operator identity + argon2id password hash + role), ``auth_token`` (live +sessions, stored as SHA-256 of the issued secret so a stolen row cannot be replayed), +``auth_bootstrap`` (the one-row marker that closes first-run admin creation permanently), and +``auth_event`` (the append-only trail of every authentication and user-management action - +rules PX-SECRETS, PX-EVIDENCE). + +No password, hash, or token value is stored in a recoverable form anywhere in this schema. + +Reversible (rule W-03): ``downgrade`` drops all four tables and the role enum type. + +Revision ID: c3d4e5f6a7b8 +Revises: a2b3c4d5e6f7 +Create Date: 2026-07-30 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +import sqlmodel +from sqlalchemy.dialects import postgresql + +from alembic import op + +revision: str = "c3d4e5f6a7b8" +down_revision: str | None = "a2b3c4d5e6f7" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +# Created with the table that uses it, mirroring how approvalstatus and findingstatus are +# handled. On SQLite this renders as VARCHAR; on PostgreSQL it is a real enum type, so the +# database itself refuses a role the application does not define. +ROLE = postgresql.ENUM("ADMIN", "OPERATOR", "VIEWER", name="role") + + +def upgrade() -> None: + """Create the identity, session, bootstrap-marker, and audit tables.""" + op.create_table( + "auth_user", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("username", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("email", sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column("password_hash", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("role", ROLE, nullable=False), + sa.Column("is_active", sa.Boolean(), nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index(op.f("ix_auth_user_username"), "auth_user", ["username"], unique=True) + op.create_index(op.f("ix_auth_user_email"), "auth_user", ["email"], unique=True) + + op.create_table( + "auth_token", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("user_id", sa.Uuid(), nullable=False), + sa.Column("token_hash", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("issued_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("revoked_at", sa.DateTime(timezone=True), nullable=True), + sa.ForeignKeyConstraint(["user_id"], ["auth_user.id"]), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index(op.f("ix_auth_token_token_hash"), "auth_token", ["token_hash"], unique=True) + op.create_index(op.f("ix_auth_token_user_id"), "auth_token", ["user_id"], unique=False) + + op.create_table( + "auth_bootstrap", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Uuid(), nullable=False), + sa.Column("completed_at", sa.DateTime(timezone=True), nullable=False), + sa.ForeignKeyConstraint(["user_id"], ["auth_user.id"]), + sa.PrimaryKeyConstraint("id"), + ) + + op.create_table( + "auth_event", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("event_type", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("outcome", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("actor_user_id", sa.Uuid(), nullable=True), + sa.Column("username_attempted", sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column("detail", sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.ForeignKeyConstraint(["actor_user_id"], ["auth_user.id"]), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + op.f("ix_auth_event_actor_user_id"), "auth_event", ["actor_user_id"], unique=False + ) + + +def downgrade() -> None: + """Drop the four tables in dependency order, then the role enum type.""" + op.drop_index(op.f("ix_auth_event_actor_user_id"), table_name="auth_event") + op.drop_table("auth_event") + op.drop_table("auth_bootstrap") + op.drop_index(op.f("ix_auth_token_user_id"), table_name="auth_token") + op.drop_index(op.f("ix_auth_token_token_hash"), table_name="auth_token") + op.drop_table("auth_token") + op.drop_index(op.f("ix_auth_user_email"), table_name="auth_user") + op.drop_index(op.f("ix_auth_user_username"), table_name="auth_user") + op.drop_table("auth_user") + ROLE.drop(op.get_bind(), checkfirst=True) diff --git a/backend/app/api/auth.py b/backend/app/api/auth.py new file mode 100644 index 0000000..c15fb99 --- /dev/null +++ b/backend/app/api/auth.py @@ -0,0 +1,149 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +Authentication routes: first-run bootstrap, login, logout, who-am-I. + +``/auth/login`` and ``/auth/bootstrap`` are open by necessity - a caller with no credential +has to be able to obtain one. ``/auth/logout`` and ``/auth/me`` require a valid token. + +Every response here is built from a whitelisting schema, and ``UserRead`` has no field a +password hash could occupy (rules B-FA-07, PX-SECRETS). Failures answer with a stable error +code and a generic message; a wrong password and an unknown username are indistinguishable, so +this endpoint cannot be used to enumerate accounts (rules PX-ERRORS, S-13). +""" + +from __future__ import annotations + +import logging +import uuid + +from fastapi import APIRouter, Depends, HTTPException, Response, status +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.api.deps import SESSION_COOKIE_NAME, current_token_id, current_user +from app.api.schemas import BootstrapRequest, LoginRead, LoginRequest, UserRead +from app.config import get_settings +from app.db import get_session +from app.models.tables import User +from app.security.passwords import PasswordPolicyError +from app.services.users import ( + BootstrapClosedError, + UsernameTakenError, + authenticate, + bootstrap_admin, + issue_token, + revoke_token, +) + +logger = logging.getLogger(__name__) + +router = APIRouter(prefix="/auth", tags=["auth"]) + +INVALID_CREDENTIALS = "invalid_credentials" +BOOTSTRAP_CLOSED = "bootstrap_closed" +USERNAME_TAKEN = "username_taken" +WEAK_PASSWORD = "weak_password" + + +def user_to_read(row: User) -> UserRead: + """Project a stored user onto its public shape, field by field. + + Hand-written rather than constructed from the row's attributes, matching every other + projector in this API: a column added to ``auth_user`` stays unpublished until someone + writes the line that publishes it (rule B-FA-01). + """ + return UserRead( + id=row.id, + username=row.username, + email=row.email, + role=str(row.role), + is_active=row.is_active, + created_at=row.created_at, + ) + + +@router.post("/bootstrap", response_model=UserRead, status_code=status.HTTP_201_CREATED) +async def bootstrap( + payload: BootstrapRequest, session: AsyncSession = Depends(get_session) +) -> UserRead: + """Create the first admin. Works only while no bootstrap has ever completed. + + This is not a backdoor: the marker row that records completion is permanent, so the route + refuses forever afterwards - including after the admin it created has been deleted. + """ + try: + admin = await bootstrap_admin( + session, + username=payload.username, + password=payload.password, + email=payload.email, + ) + except BootstrapClosedError as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={ + "error_code": BOOTSTRAP_CLOSED, + "message": "An administrator already exists. Ask them to create your account.", + }, + ) from exc + except PasswordPolicyError as exc: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={"error_code": WEAK_PASSWORD, "message": str(exc)}, + ) from exc + except UsernameTakenError as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={"error_code": USERNAME_TAKEN, "message": "That username is already in use."}, + ) from exc + logger.info("bootstrap admin created", extra={"username": admin.username}) + return user_to_read(admin) + + +@router.post("/login", response_model=LoginRead, status_code=status.HTTP_200_OK) +async def login( + payload: LoginRequest, response: Response, session: AsyncSession = Depends(get_session) +) -> LoginRead: + """Exchange a username and password for a session token. + + The token is returned in the body for the CLI and API, and set as an httpOnly, SameSite + cookie for the console (rule A-03). Both are the same secret and one logout ends both. + """ + user = await authenticate(session, username=payload.username, password=payload.password) + if user is None: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail={"error_code": INVALID_CREDENTIALS, "message": "Invalid username or password."}, + ) + issued = await issue_token(session, user) + settings = get_settings() + response.set_cookie( + SESSION_COOKIE_NAME, + issued.raw, + httponly=True, + # Off only in a development environment, where there is no TLS to require. Production + # fails to the safe setting because is_debug_env fails closed (rule PX-ERRORS). + secure=not settings.is_debug_env, + samesite="lax", + max_age=settings.auth_token_ttl_hours * 3600, + path="/", + ) + return LoginRead(token=issued.raw, expires_at=issued.expires_at, user=user_to_read(user)) + + +@router.post("/logout", status_code=status.HTTP_204_NO_CONTENT) +async def logout( + response: Response, + user: User = Depends(current_user), + token_id: uuid.UUID = Depends(current_token_id), + session: AsyncSession = Depends(get_session), +) -> None: + """Revoke the presenting token and clear the session cookie.""" + await revoke_token(session, token_id, user) + response.delete_cookie(SESSION_COOKIE_NAME, path="/") + + +@router.get("/me", response_model=UserRead, status_code=status.HTTP_200_OK) +async def me(user: User = Depends(current_user)) -> UserRead: + """Return the authenticated operator's identity and role - never the password hash.""" + return user_to_read(user) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py new file mode 100644 index 0000000..746fb3d --- /dev/null +++ b/backend/app/api/deps.py @@ -0,0 +1,218 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +Authentication and authorization dependencies (rules B-FA-03, A-01, A-02). + +Every protected route resolves its principal through :func:`current_user` and its permission +through :func:`require_role`. No handler parses an ``Authorization`` header or reads a role +itself: one place decides who a caller is, so reviewing that decision is reviewing the whole +of it. The role is read from the database row the token resolves to and never from anything +the client sent (rule A-02) - a caller can present a token, not a claim about itself. + +**What this PR enforces.** These dependencies are applied to ``/auth/me``, ``/auth/logout``, +and the ``/users`` routes only. The ``/engagements`` routes are deliberately unchanged and +remain unauthenticated; rolling enforcement across them is the next change, tracked in +docs/STATUS.md. The capability table below is written now, in full, because that next change +should be a matter of naming a capability rather than inventing the policy under deadline. +""" + +from __future__ import annotations + +import uuid +from collections.abc import Awaitable, Callable, Mapping +from datetime import UTC, datetime +from enum import StrEnum + +from fastapi import Depends, HTTPException, Request, status +from sqlmodel import select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.db import get_session +from app.models.tables import AuthToken, Role, User +from app.security.tokens import hash_token + +#: The cookie a browser session travels in. httpOnly and SameSite are set at issue time +#: (rule A-03); nothing here can read it from JavaScript. +SESSION_COOKIE_NAME = "provx_session" + +_BEARER_PREFIX = "bearer " + +UNAUTHENTICATED = "unauthenticated" +FORBIDDEN = "forbidden" + +#: Deliberately identical for a missing, malformed, unknown, revoked, and expired token. A +#: caller who is not authenticated learns that and nothing else (rules PX-ERRORS, S-13). +_UNAUTHENTICATED_MESSAGE = "Authentication required." +_FORBIDDEN_MESSAGE = "You do not have permission to perform this action." + + +class Capability(StrEnum): + """One thing an operator can do. Roles are named sets of these, never ad-hoc checks.""" + + READ = "read" + CREATE_ENGAGEMENT = "create_engagement" + RUN_SCAN = "run_scan" + APPROVE_ACTIVE_SCAN = "approve_active_scan" + MANAGE_USERS = "manage_users" + + +#: The whole authorization policy, as data. Encoded here rather than scattered across route +#: bodies so the answer to "what can an operator do?" is one table a reviewer can read, and so +#: the enforcement pass that follows this one adds arguments rather than logic. +ROLE_CAPABILITIES: Mapping[Role, frozenset[Capability]] = { + Role.ADMIN: frozenset(Capability), + Role.OPERATOR: frozenset( + { + Capability.READ, + Capability.CREATE_ENGAGEMENT, + Capability.RUN_SCAN, + # An operator approves an active run; that is the human in PX-ACTIVE's + # "recorded authorization", and it is the tester doing the work who holds it. + Capability.APPROVE_ACTIVE_SCAN, + } + ), + Role.VIEWER: frozenset({Capability.READ}), +} + + +def can(role: Role, capability: Capability) -> bool: + """Whether a role carries a capability. + + Args: + role: The authenticated user's role. + capability: The capability the action requires. + + Returns: + Whether the action is permitted. + """ + return capability in ROLE_CAPABILITIES[role] + + +def _unauthenticated() -> HTTPException: + """The single 401 raised anywhere in the auth path.""" + return HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail={"error_code": UNAUTHENTICATED, "message": _UNAUTHENTICATED_MESSAGE}, + headers={"WWW-Authenticate": "Bearer"}, + ) + + +def presented_token(request: Request) -> str | None: + """Extract the raw token a request carries, from the header or the session cookie. + + The header wins: a CLI run against a server the operator is also browsing should use the + token the CLI was given, not whichever session the browser happens to hold. + + Args: + request: The incoming request. + + Returns: + The raw token, or None when the request carries none. + """ + header = request.headers.get("Authorization", "") + if header.lower().startswith(_BEARER_PREFIX): + candidate = header[len(_BEARER_PREFIX) :].strip() + return candidate or None + return request.cookies.get(SESSION_COOKIE_NAME) or None + + +async def resolve_token(session: AsyncSession, raw_token: str) -> AuthToken | None: + """Look up a live token row by the presented secret. + + The presented value is hashed and matched against the stored digest by index, so the raw + token is never compared against anything and never has to exist in the database. + + Args: + session: The database session. + raw_token: The token as presented by the client. + + Returns: + The token row when it exists, is unrevoked, and has not expired; otherwise None. + """ + row = ( + await session.exec(select(AuthToken).where(AuthToken.token_hash == hash_token(raw_token))) + ).first() + if row is None or row.revoked_at is not None: + return None + expires_at = row.expires_at + # SQLite hands back naive datetimes even from a timezone-aware column; treat them as UTC + # rather than letting the comparison raise. + if expires_at.tzinfo is None: + expires_at = expires_at.replace(tzinfo=UTC) + if expires_at <= datetime.now(UTC): + return None + return row + + +async def current_user(request: Request, session: AsyncSession = Depends(get_session)) -> User: + """Resolve the authenticated operator, or refuse the request. + + Raises: + HTTPException: 401 when no valid token is presented, or when the user it names has + been deactivated. All of those cases answer identically. + """ + raw_token = presented_token(request) + if raw_token is None: + raise _unauthenticated() + token = await resolve_token(session, raw_token) + if token is None: + raise _unauthenticated() + user = await session.get(User, token.user_id) + if user is None or not user.is_active: + raise _unauthenticated() + return user + + +def require_role(*roles: Role) -> Callable[[User], Awaitable[User]]: + """Build a dependency that admits only the named roles. + + Args: + *roles: The roles permitted to call the route. + + Returns: + A FastAPI dependency yielding the authenticated user, or raising 403. + """ + permitted = frozenset(roles) + + async def checked(user: User = Depends(current_user)) -> User: + if user.role not in permitted: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={"error_code": FORBIDDEN, "message": _FORBIDDEN_MESSAGE}, + ) + return user + + return checked + + +def require_capability(capability: Capability) -> Callable[[User], Awaitable[User]]: + """Build a dependency admitting any role that carries a capability. + + The capability-shaped form of :func:`require_role`, for the enforcement pass that follows: + a route says what it needs rather than who may call it, so adding a role does not mean + revisiting every route. + + Args: + capability: The capability the route requires. + + Returns: + A FastAPI dependency yielding the authenticated user, or raising 403. + """ + return require_role(*(role for role in Role if can(role, capability))) + + +async def current_token_id( + request: Request, session: AsyncSession = Depends(get_session) +) -> uuid.UUID: + """Resolve the id of the token the request presented, for logout to revoke. + + Raises: + HTTPException: 401 when the request carries no valid token. + """ + raw_token = presented_token(request) + if raw_token is None: + raise _unauthenticated() + token = await resolve_token(session, raw_token) + if token is None: + raise _unauthenticated() + return token.id diff --git a/backend/app/api/engagements.py b/backend/app/api/engagements.py index 44ab0ce..396e05b 100644 --- a/backend/app/api/engagements.py +++ b/backend/app/api/engagements.py @@ -3,9 +3,13 @@ """ Engagement routes: create, list, scan, list findings, render the report. -No authentication yet - this is the walking skeleton, and RBAC is a later phase. Every -route declares an explicit response model and status code (rule B-FA-07), and errors leave -here as a stable code plus a generic message (rule PX-ERRORS). +**No route in this file is authenticated yet.** The identity, role, and token machinery +landed in ``feat/auth-foundation`` and is enforced on ``/auth`` and ``/users``; applying it +here is the next change, deliberately separated so the enforcement pass is reviewable on its +own. Until then every route below is open, and docs/STATUS.md says so explicitly. + +Every route declares an explicit response model and status code (rule B-FA-07), and errors +leave here as a stable code plus a generic message (rule PX-ERRORS). """ from __future__ import annotations diff --git a/backend/app/api/schemas.py b/backend/app/api/schemas.py index eab5a30..aff928a 100644 --- a/backend/app/api/schemas.py +++ b/backend/app/api/schemas.py @@ -318,6 +318,78 @@ class ScanRunRead(BaseModel): finished_at: datetime | None +#: An operator's username: letters, digits, dot, dash, underscore. Bounded because it is +#: written into the audit trail and shown in reports. +USERNAME_PATTERN = r"^[A-Za-z0-9._-]{3,64}$" + + +class UserRead(BaseModel): + """An operator as returned to clients. + + **There is no ``password_hash`` field here, and there must never be one.** The stored digest + is not merely omitted at serialization time - it has no field to travel through, so no + change to a handler or a projector can publish it by accident (rules B-FA-07, PX-SECRETS). + """ + + model_config = ConfigDict(extra="forbid") + + id: uuid.UUID + username: str + email: str | None + role: str + is_active: bool + created_at: datetime + + +class LoginRequest(BaseModel): + """Request body for POST /auth/login. + + The password is write-only in every sense: it is never returned, never logged, and never + recorded in the audit trail (rule PX-SECRETS). + """ + + model_config = ConfigDict(extra="forbid") + + username: str = Field(min_length=1, max_length=64) + password: str = Field(min_length=1, description="Write-only; never returned or logged") + + +class LoginRead(BaseModel): + """A successful login: the one and only time the raw token is available. + + The same token is also set as an httpOnly session cookie (rule A-03), so a caller may use + either. Only its SHA-256 is stored, so this value cannot be recovered from the database. + """ + + model_config = ConfigDict(extra="forbid") + + token: str + token_type: str = "bearer" + expires_at: datetime + user: UserRead + + +class BootstrapRequest(BaseModel): + """Request body for first-run admin creation. Accepted only while no admin exists.""" + + model_config = ConfigDict(extra="forbid") + + username: str = Field(pattern=USERNAME_PATTERN) + password: str = Field(min_length=1, description="Write-only; never returned or logged") + email: str | None = Field(default=None, max_length=320) + + +class UserCreate(BaseModel): + """Request body for creating an operator. Admin-only.""" + + model_config = ConfigDict(extra="forbid") + + username: str = Field(pattern=USERNAME_PATTERN) + password: str = Field(min_length=1, description="Write-only; never returned or logged") + role: str = Field(pattern="^(admin|operator|viewer)$") + email: str | None = Field(default=None, max_length=320) + + class ErrorResponse(BaseModel): """The user-safe error envelope (rules PX-ERRORS, B-FA-06, S-13).""" diff --git a/backend/app/api/users.py b/backend/app/api/users.py new file mode 100644 index 0000000..932206d --- /dev/null +++ b/backend/app/api/users.py @@ -0,0 +1,72 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +User-management routes. Admin-only, and the proof surface for the role gate. + +These two routes and the ``/auth`` pair are the **only** enforced endpoints in this change. +The ``/engagements`` routes are deliberately untouched and remain unauthenticated; rolling +enforcement across them is the next change (see docs/STATUS.md for the exact split). + +Authorization is a router-level dependency rather than a check inside each handler, so a route +added to this file is admin-only by construction rather than by remembering (rule B-FA-03). +""" + +from __future__ import annotations + +import logging + +from fastapi import APIRouter, Depends, HTTPException, status +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.api.auth import USERNAME_TAKEN, WEAK_PASSWORD, user_to_read +from app.api.deps import require_role +from app.api.schemas import UserCreate, UserRead +from app.db import get_session +from app.models.tables import Role, User +from app.security.passwords import PasswordPolicyError +from app.services.users import UsernameTakenError, create_user, list_users + +logger = logging.getLogger(__name__) + +#: The admin gate, built once. Attached to the router so every route in this file is gated by +#: construction, and reused as a handler argument where the handler needs to know *which* +#: admin acted - FastAPI resolves a repeated dependency once per request, so it runs once. +REQUIRE_ADMIN = Depends(require_role(Role.ADMIN)) + +router = APIRouter(prefix="/users", tags=["users"], dependencies=[REQUIRE_ADMIN]) + + +@router.post("", response_model=UserRead, status_code=status.HTTP_201_CREATED) +async def create( + payload: UserCreate, + admin: User = REQUIRE_ADMIN, + session: AsyncSession = Depends(get_session), +) -> UserRead: + """Create an operator account. The password is hashed on the way in and never returned.""" + try: + user = await create_user( + session, + username=payload.username, + password=payload.password, + role=Role(payload.role), + email=payload.email, + actor_user_id=admin.id, + ) + except PasswordPolicyError as exc: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={"error_code": WEAK_PASSWORD, "message": str(exc)}, + ) from exc + except UsernameTakenError as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={"error_code": USERNAME_TAKEN, "message": "That username is already in use."}, + ) from exc + logger.info("user created", extra={"username": user.username, "role": str(user.role)}) + return user_to_read(user) + + +@router.get("", response_model=list[UserRead], status_code=status.HTTP_200_OK) +async def index(session: AsyncSession = Depends(get_session)) -> list[UserRead]: + """List every operator. Identities and roles only - no password material exists to return.""" + return [user_to_read(row) for row in await list_users(session)] diff --git a/backend/app/config.py b/backend/app/config.py index 4c987ca..c40c7e4 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -40,6 +40,10 @@ class Settings(BaseSettings): redis_url: str = "" ai_enabled: bool = False + # How long a session token issued by POST /auth/login stays valid. Short enough that a + # leaked token expires on its own, long enough not to interrupt a working day. + auth_token_ttl_hours: int = 12 + # Client-report branding and handling marking. All optional: a report renders without # them (client name always comes from the engagement). The classification defaults to the # cautious marking so an unconfigured deployment still stamps reports as sensitive. diff --git a/backend/app/main.py b/backend/app/main.py index c6ad60b..762a156 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -4,8 +4,12 @@ Provx backend - FastAPI entrypoint. This is the walking skeleton's control plane: create an engagement, run one passive -adapter within scope, read the findings, render the report. Authentication, the workflow -engine, Active mode, and exploitation are later phases (see docs/ROADMAP.md §3-5). +adapter within scope, read the findings, render the report. The workflow engine and +exploitation are later phases (see docs/ROADMAP.md §3-5). + +Authentication exists as of ``feat/auth-foundation`` but is **not yet enforced everywhere**: +the ``/auth`` and ``/users`` routers are gated, and every ``/engagements`` route is still +open. That split is deliberate and temporary - see docs/STATUS.md for the exact list. **No AI runs here.** Every value the API returns is produced deterministically (rule PX-AI-OPTIONAL). @@ -17,12 +21,15 @@ from typing import Any from fastapi import FastAPI, Request, status -from fastapi.exceptions import HTTPException +from fastapi.encoders import jsonable_encoder +from fastapi.exceptions import HTTPException, RequestValidationError from fastapi.responses import JSONResponse from app import __version__ +from app.api.auth import router as auth_router from app.api.engagements import router as engagements_router from app.api.schemas import ErrorResponse +from app.api.users import router as users_router from app.config import get_settings logger = logging.getLogger(__name__) @@ -33,6 +40,12 @@ description="Governed automated security validation - control plane.", ) app.include_router(engagements_router) +# The authentication foundation. Only these two routers are gated: /auth/me and /auth/logout +# require any authenticated operator, /users requires an admin. The engagement and scan routes +# above are deliberately unchanged and remain open - enforcing them is the next change, and +# docs/STATUS.md carries the exact enforced/not-yet list. +app.include_router(auth_router) +app.include_router(users_router) @app.exception_handler(HTTPException) @@ -46,6 +59,36 @@ async def handle_http_exception(request: Request, exc: HTTPException) -> JSONRes return JSONResponse(status_code=exc.status_code, content=body.model_dump()) +@app.exception_handler(RequestValidationError) +async def handle_validation_error(request: Request, exc: RequestValidationError) -> JSONResponse: + """Answer a malformed request without handing the caller back what they sent. + + FastAPI's default handler returns Pydantic's error list verbatim, and a Pydantic error + carries the value that failed in ``input`` - for a ``missing`` error, the whole request + object. On any endpoint that accepts a secret, that hands the secret straight back: + ``POST /auth/login`` with the username omitted reflected the submitted password, and + ``POST /engagements/{id}/credential`` with ``cred_type`` omitted reflected the submitted + credential value (rule PX-SECRETS). Both were live before this handler existed. + + The reflected value is the caller's own input, not stored state - validation runs before + the handler, so nothing is read from the database. The harm is where a secret in a + response body comes to rest: proxy access logs, error trackers, CI output, devtools. + + ``type``, ``loc``, and ``msg`` survive because they are what makes a 422 actionable and + none of them carries caller input. ``input`` and ``ctx`` are dropped because both can, + and ``url`` is a docs link nobody here follows. The response keeps FastAPI's shape rather + than moving to :class:`ErrorResponse`: the CLI branches on the envelope first, so + switching shape would discard the per-field detail it renders today. + """ + sanitized: list[dict[str, Any]] = [ + {"type": error["type"], "loc": error["loc"], "msg": error["msg"]} for error in exc.errors() + ] + return JSONResponse( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + content=jsonable_encoder({"detail": sanitized}), + ) + + @app.exception_handler(Exception) async def handle_unexpected_exception(request: Request, exc: Exception) -> JSONResponse: """Log the real failure server-side and hand the client a generic message. diff --git a/backend/app/models/tables.py b/backend/app/models/tables.py index 1e94509..f327ac6 100644 --- a/backend/app/models/tables.py +++ b/backend/app/models/tables.py @@ -472,3 +472,114 @@ class ApprovalEventRow(SQLModel, table=True): actor: str | None = Field(default=None) note: str | None = Field(default=None) created_at: datetime = Field(default_factory=_now, sa_column=_timestamp_column()) + + +class Role(StrEnum): + """What an operator is allowed to do. The capability mapping lives in ``app.api.deps``. + + Three roles, not a permission matrix: a small firm's engagement has an owner who + administers the platform, testers who run the work, and reviewers who read the results. + """ + + ADMIN = "admin" + OPERATOR = "operator" + VIEWER = "viewer" + + +class User(SQLModel, table=True): + """A platform operator: who they are, what they may do, and how they prove it. + + ``password_hash`` holds an argon2id digest and is **write-only from the API's perspective** + - no response schema declares the field, so there is no serializer that could return it + (rules B-FA-07, PX-SECRETS). Unlike ``Credential``, nothing here is reversible: a password + is verified by re-hashing the candidate, never by decrypting the stored value. + + Named ``auth_user`` rather than ``user`` because ``user`` is a reserved word in PostgreSQL; + every reference to it would need quoting, including the ad-hoc ones a human types into psql. + """ + + __tablename__ = "auth_user" + + id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) + username: str = Field(unique=True, index=True) + # Optional: an air-gapped deployment has no mail. Unique when present - both PostgreSQL + # and SQLite permit repeated NULLs under a unique constraint. + email: str | None = Field(default=None, unique=True, index=True) + # argon2id digest, self-describing (algorithm, parameters, and salt are encoded in it). + password_hash: str + role: Role = Field(default=Role.VIEWER) + # Deactivation rather than deletion: an operator who has acted is referenced by the audit + # trail, and an audit trail with dangling actors is not an audit trail (rule PX-EVIDENCE). + is_active: bool = Field(default=True) + created_at: datetime = Field(default_factory=_now, sa_column=_timestamp_column()) + + +class AuthToken(SQLModel, table=True): + """A live credential proving a request comes from a given user. + + One login issues exactly one opaque token, delivered two ways - in the response body for the + CLI and API, and as an httpOnly cookie for the console (rule A-03). They are the same secret, + so a single row revokes both. + + Only the SHA-256 of that secret is stored. A stolen database row therefore cannot be replayed + as a bearer token, which is the whole point of the column (rule PX-SECRETS): the raw value + exists in the login response and nowhere else, ever again. + """ + + __tablename__ = "auth_token" + + id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) + user_id: uuid.UUID = Field(foreign_key="auth_user.id", index=True) + # SHA-256 hex of the issued token. Unique so a presented token resolves by exact index + # lookup - there is no stored secret to compare, and so no timing channel to defend. + token_hash: str = Field(unique=True, index=True) + issued_at: datetime = Field(default_factory=_now, sa_column=_timestamp_column()) + expires_at: datetime = Field(sa_column=_timestamp_column()) + # Set by logout. The row is kept rather than deleted so the audit trail can still name it. + revoked_at: datetime | None = Field(default=None, sa_column=_timestamp_column(nullable=True)) + + +#: The primary key every bootstrap marker row uses. Fixed, so a second insert collides. +BOOTSTRAP_ROW_ID = 1 + + +class AuthBootstrap(SQLModel, table=True): + """A one-row marker recording that first-run admin creation has already happened. + + The bootstrap endpoint is the only unauthenticated way to create a user, so it must close + permanently and cannot be allowed to race. A ``COUNT(admins) == 0`` check fails both tests: + two concurrent first-run requests each see zero, and deleting the admin re-opens the door. + A fixed primary key makes the second insert a primary-key violation on PostgreSQL and SQLite + alike, with no dialect-specific locking, and the marker survives the admin it created. + """ + + __tablename__ = "auth_bootstrap" + + id: int = Field(default=BOOTSTRAP_ROW_ID, primary_key=True) + user_id: uuid.UUID = Field(foreign_key="auth_user.id") + completed_at: datetime = Field(default_factory=_now, sa_column=_timestamp_column()) + + +class AuthEventRow(SQLModel, table=True): + """An append-only audit entry for one authentication or user-management action. + + Bootstrap, login (success *and* failure), logout, and user creation each write exactly one + row; nothing is updated or deleted (rules PX-SECRETS, PX-EVIDENCE, D-05). No password, hash, + or token value is ever recorded here - ``username_attempted`` is the identifier a caller + supplied, which is not a secret, and ``detail`` carries a fixed reason string, never input. + """ + + __tablename__ = "auth_event" + + id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) + # "bootstrap" | "login" | "logout" | "user_created". + event_type: str + # "success" | "failure". + outcome: str + # The authenticated principal, once one is known. Null for a failed login, where the caller + # proved nothing about who they are. + actor_user_id: uuid.UUID | None = Field(default=None, foreign_key="auth_user.id", index=True) + username_attempted: str | None = Field(default=None) + # A fixed reason string chosen from a closed set in the service layer, never caller input. + detail: str | None = Field(default=None) + created_at: datetime = Field(default_factory=_now, sa_column=_timestamp_column()) diff --git a/backend/app/security/passwords.py b/backend/app/security/passwords.py new file mode 100644 index 0000000..001ba50 --- /dev/null +++ b/backend/app/security/passwords.py @@ -0,0 +1,93 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +Operator password hashing (rules PX-SECRETS, PX-FREE, A-04). + +**This module is deliberately not built on the evidence-crypto layer next door.** Evidence +encryption is reversible by design so a scan credential can be replayed at scan time; a +password must never be recoverable by anyone holding the key, including us. The two live side +by side precisely so the distinction is visible rather than assumed. + +argon2id via ``argon2-cffi`` (MIT), the reference binding to the Argon2 implementation, at +OWASP's baseline parameters. The digest is self-describing - algorithm, version, parameters, +and salt are encoded in the string - so a future parameter change can verify old hashes while +issuing new ones without a schema migration. + +Neither the candidate password nor the stored digest is ever logged, returned, or placed in an +exception message. There is no function here that takes a hash and yields a password, because +no such function can exist. +""" + +from __future__ import annotations + +from argon2 import PasswordHasher +from argon2.exceptions import Argon2Error, InvalidHashError + +#: OWASP's argon2id baseline: 64 MiB of memory, 3 iterations, 4 lanes. Memory cost is the +#: parameter that actually prices a GPU attack, so it is the one held highest. +_TIME_COST = 3 +_MEMORY_COST_KIB = 64 * 1024 +_PARALLELISM = 4 +_HASH_BYTES = 32 +_SALT_BYTES = 16 + +_hasher = PasswordHasher( + time_cost=_TIME_COST, + memory_cost=_MEMORY_COST_KIB, + parallelism=_PARALLELISM, + hash_len=_HASH_BYTES, + salt_len=_SALT_BYTES, +) + + +class PasswordPolicyError(ValueError): + """The supplied password does not meet the minimum policy.""" + + +#: Long enough that argon2id's cost is not the only thing standing between a weak password and +#: an offline attacker. A denylist of common passwords (rule S-12) is a later addition. +MIN_PASSWORD_LENGTH = 12 + + +def validate_password(plain: str) -> None: + """Reject a password that is too short to be worth hashing. + + Raises: + PasswordPolicyError: With a message safe to show a caller - it describes the policy, + never the value that failed it (rules PX-ERRORS, S-13). + """ + if len(plain) < MIN_PASSWORD_LENGTH: + raise PasswordPolicyError(f"Password must be at least {MIN_PASSWORD_LENGTH} characters.") + + +def hash_password(plain: str) -> str: + """Hash a password for storage. + + Args: + plain: The candidate password. Never logged, never echoed back. + + Returns: + The argon2id digest, including its parameters and a fresh random salt. + """ + return _hasher.hash(plain) + + +def verify_password(stored_hash: str, plain: str) -> bool: + """Check a candidate password against a stored digest. + + Constant-time by the library's design. Every failure mode collapses to ``False`` - a + mismatch, a malformed digest, and a hash produced by parameters this build cannot run all + answer the same way, so neither a caller nor a traceback learns which one it was + (rules PX-ERRORS, S-13). + + Args: + stored_hash: The digest from ``auth_user.password_hash``. + plain: The candidate password. + + Returns: + Whether the candidate matches. + """ + try: + return _hasher.verify(stored_hash, plain) + except (Argon2Error, InvalidHashError): + return False diff --git a/backend/app/security/tokens.py b/backend/app/security/tokens.py new file mode 100644 index 0000000..4805140 --- /dev/null +++ b/backend/app/security/tokens.py @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +Session-token issue and lookup (rules PX-SECRETS, A-03, A-04). + +A token is 256 bits from ``secrets.token_urlsafe`` - never ``random``, which is seeded +predictably and is not a cryptographic source (rule A-04). The raw value is returned to the +caller once, at login, and is never stored: the database holds only its SHA-256, so a stolen +row cannot be replayed as a bearer token. + +**Why SHA-256 here and argon2id for passwords.** A password is low-entropy and guessable, so it +needs a deliberately slow KDF to price an offline dictionary attack. A token is 256 uniformly +random bits with no dictionary to try, so there is nothing for a slow hash to buy - and since +every authenticated request has to resolve one, a 64 MiB KDF would make each API call cost a +login. The lookup is an exact index match on the digest, so no stored secret is ever compared +and there is no timing channel to defend. +""" + +from __future__ import annotations + +import hashlib +import secrets +from datetime import UTC, datetime, timedelta + +#: 32 bytes of entropy, URL-safe encoded - safe in an Authorization header and a cookie alike. +_TOKEN_BYTES = 32 + + +def generate_token() -> str: + """Mint a new opaque session token. The only place a raw token comes into existence.""" + return secrets.token_urlsafe(_TOKEN_BYTES) + + +def hash_token(token: str) -> str: + """Return the stored form of a token. + + Args: + token: The raw token, as issued to or presented by a client. + + Returns: + Lowercase SHA-256 hex - the only form that is ever persisted. + """ + return hashlib.sha256(token.encode("utf-8")).hexdigest() + + +def expiry_from(ttl_hours: int, *, issued_at: datetime | None = None) -> datetime: + """Compute when a token issued now stops being accepted. + + Args: + ttl_hours: Lifetime in hours, from ``Settings.auth_token_ttl_hours``. + issued_at: Issue time, defaulting to now. Injectable so a test can age a token + without sleeping. + + Returns: + The timezone-aware expiry instant. + """ + return (issued_at or datetime.now(UTC)) + timedelta(hours=ttl_hours) diff --git a/backend/app/services/users.py b/backend/app/services/users.py new file mode 100644 index 0000000..ee7417c --- /dev/null +++ b/backend/app/services/users.py @@ -0,0 +1,286 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +Identity, session, and first-run bootstrap (rules PX-SECRETS, PX-EVIDENCE, D-05). + +The one seam between the API and the auth tables. Passwords enter here and become argon2id +digests; tokens are minted here and leave as a raw value exactly once, at login. Nothing in +this module returns, logs, or records a password, a digest, or a raw token - every audit entry +names a user or an attempted username, both of which are identifiers rather than secrets. + +Every state-changing action writes one append-only ``auth_event`` row, including a failed +login: a security platform that cannot say when someone tried and failed to get in is missing +the entry that matters most. +""" + +from __future__ import annotations + +import uuid +from datetime import UTC, datetime + +from sqlalchemy.exc import IntegrityError +from sqlmodel import col, select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.config import get_settings +from app.models.tables import AuthBootstrap, AuthEventRow, AuthToken, Role, User +from app.security.passwords import hash_password, validate_password, verify_password +from app.security.tokens import expiry_from, generate_token, hash_token + +EVENT_BOOTSTRAP = "bootstrap" +EVENT_LOGIN = "login" +EVENT_LOGOUT = "logout" +EVENT_USER_CREATED = "user_created" + +OUTCOME_SUCCESS = "success" +OUTCOME_FAILURE = "failure" + +#: Closed set of reasons an auth_event may record. Fixed strings, never caller input, so the +#: audit trail cannot become a place a secret is written by accident (rule PX-SECRETS). +REASON_NO_SUCH_USER = "no_such_user" +REASON_BAD_PASSWORD = "bad_password" +REASON_INACTIVE = "inactive" +REASON_BOOTSTRAP_CLOSED = "bootstrap_closed" + + +class BootstrapClosedError(RuntimeError): + """First-run admin creation has already happened and cannot happen again.""" + + +class UsernameTakenError(RuntimeError): + """The requested username or email is already in use.""" + + +class IssuedToken: + """A freshly issued session token: the raw secret, and when it stops working. + + The raw value exists here and in the login response, and nowhere else - not in the + database, not in a log. Deliberately not a dataclass with a default ``__repr__``: the + repr is overridden so an accidental ``logger.info(token)`` cannot print the secret. + """ + + __slots__ = ("raw", "expires_at") + + def __init__(self, raw: str, expires_at: datetime) -> None: + self.raw = raw + self.expires_at = expires_at + + def __repr__(self) -> str: + return f"IssuedToken(expires_at={self.expires_at!r})" + + +async def record_event( + session: AsyncSession, + *, + event_type: str, + outcome: str, + actor_user_id: uuid.UUID | None = None, + username_attempted: str | None = None, + detail: str | None = None, +) -> None: + """Append one audit row. Never updates, never deletes (rules PX-EVIDENCE, D-05).""" + session.add( + AuthEventRow( + event_type=event_type, + outcome=outcome, + actor_user_id=actor_user_id, + username_attempted=username_attempted, + detail=detail, + ) + ) + + +async def get_by_username(session: AsyncSession, username: str) -> User | None: + """Look up an operator by username. Case- and whitespace-normalized on the way in.""" + return ( + await session.exec(select(User).where(User.username == username.strip().lower())) + ).first() + + +async def list_users(session: AsyncSession) -> list[User]: + """Every operator, ordered by username for a stable listing.""" + return list((await session.exec(select(User).order_by(col(User.username)))).all()) + + +async def bootstrap_completed(session: AsyncSession) -> bool: + """Whether first-run admin creation has already happened.""" + return (await session.exec(select(AuthBootstrap))).first() is not None + + +async def create_user( + session: AsyncSession, + *, + username: str, + password: str, + role: Role, + email: str | None = None, + actor_user_id: uuid.UUID | None = None, +) -> User: + """Create an operator and audit it, in one transaction. + + Args: + session: The database session. + username: Desired username; normalized to lowercase. + password: The plaintext, validated then hashed. Never stored or logged. + role: The role to grant. + email: Optional contact address. + actor_user_id: The admin performing the creation, or None during bootstrap. + + Returns: + The persisted user. + + Raises: + PasswordPolicyError: The password is too short. + UsernameTakenError: The username or email is already in use. + """ + validate_password(password) + user = User( + username=username.strip().lower(), + email=(email or "").strip().lower() or None, + password_hash=hash_password(password), + role=role, + ) + session.add(user) + await record_event( + session, + event_type=EVENT_USER_CREATED, + outcome=OUTCOME_SUCCESS, + actor_user_id=actor_user_id, + username_attempted=user.username, + ) + try: + await session.commit() + except IntegrityError as exc: + await session.rollback() + raise UsernameTakenError("username or email already in use") from exc + await session.refresh(user) + return user + + +async def bootstrap_admin( + session: AsyncSession, *, username: str, password: str, email: str | None = None +) -> User: + """Create the first admin, once and only ever once. + + The marker row carries a fixed primary key, so two concurrent first-run requests cannot + both succeed: the loser hits a primary-key violation rather than a lost race. The marker + outlives the admin it created, so deleting that admin does not re-open the door. + + Raises: + BootstrapClosedError: Bootstrap has already completed. + PasswordPolicyError: The password is too short. + UsernameTakenError: The username or email is already in use. + """ + if await bootstrap_completed(session): + await record_event( + session, + event_type=EVENT_BOOTSTRAP, + outcome=OUTCOME_FAILURE, + username_attempted=username.strip().lower(), + detail=REASON_BOOTSTRAP_CLOSED, + ) + await session.commit() + raise BootstrapClosedError("bootstrap already completed") + + validate_password(password) + admin = User( + username=username.strip().lower(), + email=(email or "").strip().lower() or None, + password_hash=hash_password(password), + role=Role.ADMIN, + ) + session.add(admin) + await session.flush() + session.add(AuthBootstrap(user_id=admin.id)) + await record_event( + session, + event_type=EVENT_BOOTSTRAP, + outcome=OUTCOME_SUCCESS, + actor_user_id=admin.id, + username_attempted=admin.username, + ) + try: + await session.commit() + except IntegrityError as exc: + await session.rollback() + # Either the marker row already existed (the concurrent-bootstrap race, which this + # constraint exists to lose safely) or the username is taken. Both refuse; the + # marker is the more serious of the two and is reported as such. + if await bootstrap_completed(session): + raise BootstrapClosedError("bootstrap already completed") from exc + raise UsernameTakenError("username or email already in use") from exc + await session.refresh(admin) + return admin + + +async def authenticate(session: AsyncSession, *, username: str, password: str) -> User | None: + """Verify a username and password, auditing the attempt either way. + + An unknown username and a wrong password are indistinguishable to the caller - both + return None, and the endpoint turns both into the same message, so login is not an + account-enumeration oracle (rules PX-ERRORS, S-13). The reason is recorded in the audit + trail, where an operator may legitimately see it. + """ + attempted = username.strip().lower() + user = await get_by_username(session, attempted) + reason: str | None = None + if user is None: + reason = REASON_NO_SUCH_USER + elif not user.is_active: + reason = REASON_INACTIVE + elif not verify_password(user.password_hash, password): + reason = REASON_BAD_PASSWORD + + if reason is not None: + await record_event( + session, + event_type=EVENT_LOGIN, + outcome=OUTCOME_FAILURE, + actor_user_id=user.id if user is not None else None, + username_attempted=attempted, + detail=reason, + ) + await session.commit() + return None + return user + + +async def issue_token(session: AsyncSession, user: User) -> IssuedToken: + """Mint a session token for a user and store only its digest. + + Returns: + The raw token and its expiry. This is the one and only time the raw value is + available; the row keeps the SHA-256 (rule PX-SECRETS). + """ + raw = generate_token() + expires_at = expiry_from(get_settings().auth_token_ttl_hours) + session.add(AuthToken(user_id=user.id, token_hash=hash_token(raw), expires_at=expires_at)) + await record_event( + session, + event_type=EVENT_LOGIN, + outcome=OUTCOME_SUCCESS, + actor_user_id=user.id, + username_attempted=user.username, + ) + await session.commit() + return IssuedToken(raw, expires_at) + + +async def revoke_token(session: AsyncSession, token_id: uuid.UUID, user: User) -> None: + """Revoke one session token and audit the logout. + + Only the presenting token is revoked: an operator logged in from the CLI and the console + at once should not be signed out of both by ending one of them. + """ + token = await session.get(AuthToken, token_id) + if token is not None and token.revoked_at is None: + token.revoked_at = datetime.now(UTC) + session.add(token) + await record_event( + session, + event_type=EVENT_LOGOUT, + outcome=OUTCOME_SUCCESS, + actor_user_id=user.id, + username_attempted=user.username, + ) + await session.commit() diff --git a/backend/pyproject.toml b/backend/pyproject.toml index a0f6855..c2c1d2a 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -26,6 +26,11 @@ dependencies = [ "pydantic-settings>=2.3", # Evidence-at-rest encryption (AES-256-GCM). Apache-2.0 / BSD, PX-FREE clean. "cryptography>=42", + # Operator password hashing (argon2id). MIT, PX-FREE clean. The reference binding to + # the Argon2 reference implementation, so the parameters Provx sets are the ones the + # algorithm's authors define. Deliberately separate from `cryptography` above: + # evidence encryption is reversible by design, a password must never be. + "argon2-cffi>=23.1", # PDF rendering of the same report template. BSD-3, no headless browser, so the image # stays free of a Chromium payload (PX-FREE). Needs the pango system libraries, which # the Dockerfile installs; the renderer imports it lazily so a host without them can diff --git a/backend/requirements.lock b/backend/requirements.lock index 3ec6ee5..06f1354 100644 --- a/backend/requirements.lock +++ b/backend/requirements.lock @@ -4,13 +4,15 @@ # Regenerate: python3.12 -m venv .lock && .lock/bin/pip install "fastapi>=0.111" \ # "uvicorn[standard]>=0.30" "sqlmodel>=0.0.22" "alembic>=1.13" "asyncpg>=0.29" \ # "aiosqlite>=0.20" "greenlet>=3" "jinja2>=3.1" "pydantic-settings>=2.3" "httpx>=0.27" "pyyaml>=6" \ -# "cryptography>=42" "weasyprint>=62" \ +# "cryptography>=42" "argon2-cffi>=23.1" "weasyprint>=62" \ # && .lock/bin/pip freeze | grep -v "^provx" > backend/requirements.lock aiosqlite==0.22.1 alembic==1.18.5 annotated-doc==0.0.4 annotated-types==0.7.0 anyio==4.14.2 +argon2-cffi==25.1.0 +argon2-cffi-bindings==25.1.0 asyncpg==0.31.0 # WeasyPrint's font subsetter (fonttools[woff]) needs brotli and zopfli. brotli==1.2.0 diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index cd50845..8fdc8b1 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -94,6 +94,118 @@ async def session(database_url: str) -> AsyncIterator[AsyncSession]: await engine.dispose() +#: Passwords the auth fixtures plant. Long enough to clear the minimum policy, and distinctive +#: enough that a leak test searching for them cannot match something incidental. +ADMIN_PASSWORD = "correct-horse-battery-staple-admin" +OPERATOR_PASSWORD = "correct-horse-battery-staple-operator" +VIEWER_PASSWORD = "correct-horse-battery-staple-viewer" + + +class AuthHelper: + """Drives the real auth endpoints so tests get tokens the way a client would. + + No fixture writes a user row or a token row directly: the machinery under test is the + machinery that issues them. The cookie jar is cleared after every login so a test says + explicitly which credential it is presenting - otherwise TestClient's persistent jar would + silently authenticate requests that are meant to be anonymous. + """ + + def __init__(self, client: TestClient) -> None: + self.client = client + + def bootstrap(self, username: str = "root", password: str = ADMIN_PASSWORD) -> Any: + """Create the first admin through POST /auth/bootstrap.""" + response = self.client.post( + "/auth/bootstrap", json={"username": username, "password": password} + ) + assert response.status_code == 201, response.text + return response.json() + + def login(self, username: str, password: str) -> str: + """Log in and return the raw bearer token, leaving no session cookie behind.""" + response = self.client.post( + "/auth/login", json={"username": username, "password": password} + ) + assert response.status_code == 200, response.text + self.client.cookies.clear() + return str(response.json()["token"]) + + def create_user(self, admin_token: str, username: str, role: str, password: str) -> Any: + """Create an operator through the admin-only POST /users.""" + response = self.client.post( + "/users", + json={"username": username, "password": password, "role": role}, + headers=bearer(admin_token), + ) + assert response.status_code == 201, response.text + return response.json() + + +def bearer(token: str) -> dict[str, str]: + """The Authorization header for a raw token.""" + return {"Authorization": f"Bearer {token}"} + + +def with_session(database_url: str, work: Callable[[AsyncSession], Any]) -> Any: + """Run one coroutine against a throwaway session, from a synchronous test. + + Its own engine rather than the app's: TestClient drives the app on a portal thread with its + own event loop, and reusing a loop-bound engine from here is how a suite acquires flaky + cross-loop failures. + """ + + async def run() -> Any: + engine = create_async_engine(database_url, future=True) + maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + try: + async with maker() as db_session: + return await work(db_session) + finally: + await engine.dispose() + + return asyncio.run(run()) + + +def deactivate(database_url: str, username: str) -> None: + """Flip a user's ``is_active`` off. There is no deactivation endpoint in this change.""" + from sqlmodel import select + + from app.models.tables import User + + async def work(db_session: AsyncSession) -> None: + user = (await db_session.exec(select(User).where(User.username == username))).one() + user.is_active = False + db_session.add(user) + await db_session.commit() + + with_session(database_url, work) + + +@pytest.fixture +def auth(client: TestClient) -> AuthHelper: + """Helper that obtains tokens through the real endpoints.""" + return AuthHelper(client) + + +@pytest.fixture +def admin_token(auth: AuthHelper) -> str: + """A bootstrapped admin's bearer token.""" + auth.bootstrap() + return auth.login("root", ADMIN_PASSWORD) + + +@pytest.fixture +def role_tokens(auth: AuthHelper, admin_token: str) -> dict[str, str]: + """A live bearer token for each of the three roles, keyed by role name.""" + auth.create_user(admin_token, "opal", "operator", OPERATOR_PASSWORD) + auth.create_user(admin_token, "vera", "viewer", VIEWER_PASSWORD) + return { + "admin": admin_token, + "operator": auth.login("opal", OPERATOR_PASSWORD), + "viewer": auth.login("vera", VIEWER_PASSWORD), + } + + @pytest.fixture def make_engagement() -> EngagementFactory: """Build an Engagement without touching the database. diff --git a/backend/tests/support/__init__.py b/backend/tests/support/__init__.py new file mode 100644 index 0000000..c430d5c --- /dev/null +++ b/backend/tests/support/__init__.py @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +"""Shared test helpers. Not shipped; imported by the suites under ``backend/tests``.""" diff --git a/backend/tests/support/leakcheck.py b/backend/tests/support/leakcheck.py new file mode 100644 index 0000000..4f9360c --- /dev/null +++ b/backend/tests/support/leakcheck.py @@ -0,0 +1,108 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +A detector for secrets that escaped into somewhere a client or an operator can read. + +Used by the auth no-leak tests to assert that a planted password, its stored hash, and a raw +token appear in no response body, no response header, and no log record. + +Two things here are deliberate and worth not undoing: + +**Log records are searched through ``record.__dict__``, not ``record.getMessage()``.** The +existing credential no-leak test uses ``getMessage()``, which renders the format string and its +positional arguments but not the ``extra={...}`` payload - and ``extra=`` is exactly how this +codebase attaches structured context. A secret logged that way would pass a ``getMessage()`` +scan while sitting in the record. This scans the whole record. + +**The detector is itself under test.** ``test_auth_no_leak.py`` feeds it a payload that does +contain the secret and asserts it raises. A detector nobody has watched fail is an assertion +that something is absent from a place nobody checked. +""" + +from __future__ import annotations + +import json +import logging +from collections.abc import Iterable, Sequence +from typing import Any + +#: Record attributes that are logging's own bookkeeping and can never carry a payload. Excluded +#: so a false positive cannot come from, say, a pathname that happens to contain the needle. +_UNINTERESTING = frozenset({"created", "msecs", "relativeCreated", "thread", "process"}) + + +class SecretLeak(AssertionError): + """A secret was found somewhere it must never appear.""" + + +def _render(value: Any) -> str: + """Flatten any object into searchable text, falling back to repr for the exotic.""" + if isinstance(value, str): + return value + if isinstance(value, bytes): + return value.decode("utf-8", errors="replace") + try: + return json.dumps(value, default=repr, sort_keys=True) + except (TypeError, ValueError): + return repr(value) + + +def render_record(record: logging.LogRecord) -> str: + """Flatten a log record, including its ``extra=`` payload and any exception text.""" + parts = [record.getMessage()] + if record.exc_info is not None: + parts.append(logging.Formatter().formatException(record.exc_info)) + for key, value in record.__dict__.items(): + if key in _UNINTERESTING: + continue + parts.append(f"{key}={_render(value)}") + return "\n".join(parts) + + +def scan(secrets: Sequence[str], haystacks: Iterable[tuple[str, str]]) -> list[str]: + """Find every place a secret appears. + + Args: + secrets: The values that must be absent. Empty strings are ignored - an empty needle + matches everything and would make this assertion meaningless. + haystacks: ``(where, text)`` pairs, where ``where`` names the source for the message. + + Returns: + One human-readable line per (secret, place) hit; empty when nothing leaked. + """ + needles = [s for s in secrets if s] + return [ + f"{needle[:6]}... found in {where}" + for where, text in haystacks + for needle in needles + if needle in text + ] + + +def assert_absent(secrets: Sequence[str], haystacks: Iterable[tuple[str, str]]) -> None: + """Assert that no secret appears in any haystack. + + Raises: + SecretLeak: With every place the secret was found. The secret itself is truncated in + the message - a test failure should not be the thing that prints it. + """ + hits = scan(secrets, list(haystacks)) + if hits: + raise SecretLeak("secret material leaked:\n " + "\n ".join(hits)) + + +def from_response(label: str, response: Any) -> list[tuple[str, str]]: + """Searchable views of an HTTP response: its body, and its headers. + + Headers matter as much as the body here: a session cookie travels in ``Set-Cookie``, so a + test that scanned only the body would call a token-in-a-header case clean. + """ + return [ + (f"{label} body", response.text), + (f"{label} headers", _render(dict(response.headers))), + ] + + +def from_records(label: str, records: Iterable[logging.LogRecord]) -> list[tuple[str, str]]: + """Searchable views of captured log records, one entry per record.""" + return [(f"{label} log[{i}]", render_record(r)) for i, r in enumerate(records)] diff --git a/backend/tests/test_api_auth.py b/backend/tests/test_api_auth.py new file mode 100644 index 0000000..c699f24 --- /dev/null +++ b/backend/tests/test_api_auth.py @@ -0,0 +1,259 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +The authentication endpoints end to end, and the token-security properties behind them. + +The load-bearing assertions here are the negative ones: what a stolen database row cannot do, +what a revoked token stops doing, and what a failed login declines to tell the caller. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta + +import pytest +from fastapi.testclient import TestClient +from sqlmodel import select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.api.deps import SESSION_COOKIE_NAME +from app.models.tables import AuthEventRow, AuthToken, User +from app.security.tokens import hash_token +from tests.conftest import ADMIN_PASSWORD, AuthHelper, bearer, with_session + +pytestmark = pytest.mark.usefixtures("database_url") + + +def _tokens(database_url: str) -> list[AuthToken]: + async def work(db: AsyncSession) -> list[AuthToken]: + return list((await db.exec(select(AuthToken))).all()) + + return list(with_session(database_url, work)) + + +def _events(database_url: str) -> list[AuthEventRow]: + async def work(db: AsyncSession) -> list[AuthEventRow]: + return list((await db.exec(select(AuthEventRow))).all()) + + return list(with_session(database_url, work)) + + +def _expire(database_url: str, token: str) -> None: + """Backdate a token's expiry, so expiry is tested without a sleep.""" + + async def work(db: AsyncSession) -> None: + row = ( + await db.exec(select(AuthToken).where(AuthToken.token_hash == hash_token(token))) + ).one() + row.expires_at = datetime.now(UTC) - timedelta(seconds=1) + db.add(row) + await db.commit() + + with_session(database_url, work) + + +def test_login_returns_the_operator_and_a_token(client: TestClient, auth: AuthHelper) -> None: + auth.bootstrap() + + response = client.post("/auth/login", json={"username": "root", "password": ADMIN_PASSWORD}) + + assert response.status_code == 200 + body = response.json() + assert body["token_type"] == "bearer" + assert body["user"]["username"] == "root" + assert body["user"]["role"] == "admin" + assert body["token"] + + +def test_login_sets_an_httponly_samesite_session_cookie( + client: TestClient, auth: AuthHelper +) -> None: + """Rule A-03. Secure is off only because APP_ENV is `testing`, which is a debug env.""" + auth.bootstrap() + + response = client.post("/auth/login", json={"username": "root", "password": ADMIN_PASSWORD}) + + cookie = response.headers["set-cookie"].lower() + assert cookie.startswith(f"{SESSION_COOKIE_NAME}=") + assert "httponly" in cookie + assert "samesite=lax" in cookie + + +def test_the_session_cookie_authenticates_without_a_bearer_header( + client: TestClient, auth: AuthHelper +) -> None: + auth.bootstrap() + client.post("/auth/login", json={"username": "root", "password": ADMIN_PASSWORD}) + + response = client.get("/auth/me") + + assert response.status_code == 200 + assert response.json()["username"] == "root" + + +def test_me_returns_identity_and_role_only(client: TestClient, admin_token: str) -> None: + response = client.get("/auth/me", headers=bearer(admin_token)) + + assert response.status_code == 200 + assert set(response.json()) == {"id", "username", "email", "role", "is_active", "created_at"} + + +@pytest.mark.parametrize( + ("username", "password"), + [("root", "wrong-password-entirely"), ("nobody", ADMIN_PASSWORD)], +) +def test_a_bad_login_is_401_and_does_not_say_which_half_was_wrong( + client: TestClient, auth: AuthHelper, username: str, password: str +) -> None: + """A distinguishable answer here would make login an account-enumeration oracle.""" + auth.bootstrap() + + response = client.post("/auth/login", json={"username": username, "password": password}) + + assert response.status_code == 401 + assert response.json()["error_code"] == "invalid_credentials" + assert response.json()["message"] == "Invalid username or password." + + +def test_the_stored_row_is_not_the_issued_token(client: TestClient, database_url: str) -> None: + """A stolen database row is not a credential - the whole point of hashing the token.""" + auth = AuthHelper(client) + auth.bootstrap() + token = auth.login("root", ADMIN_PASSWORD) + + (row,) = _tokens(database_url) + + assert row.token_hash != token + assert token not in row.token_hash + + +def test_the_stored_hash_cannot_be_replayed_as_a_bearer_token( + client: TestClient, admin_token: str, database_url: str +) -> None: + (row,) = _tokens(database_url) + + response = client.get("/auth/me", headers=bearer(row.token_hash)) + + assert response.status_code == 401 + + +def test_logout_revokes_the_presenting_token(client: TestClient, admin_token: str) -> None: + assert client.get("/auth/me", headers=bearer(admin_token)).status_code == 200 + + assert client.post("/auth/logout", headers=bearer(admin_token)).status_code == 204 + + assert client.get("/auth/me", headers=bearer(admin_token)).status_code == 401 + + +def test_logout_leaves_the_operators_other_session_alone( + client: TestClient, auth: AuthHelper +) -> None: + """Ending a CLI session should not sign the same operator out of the console.""" + auth.bootstrap() + first = auth.login("root", ADMIN_PASSWORD) + second = auth.login("root", ADMIN_PASSWORD) + + client.post("/auth/logout", headers=bearer(first)) + + assert client.get("/auth/me", headers=bearer(second)).status_code == 200 + + +def test_an_expired_token_is_401(client: TestClient, admin_token: str, database_url: str) -> None: + _expire(database_url, admin_token) + + assert client.get("/auth/me", headers=bearer(admin_token)).status_code == 401 + + +@pytest.mark.parametrize( + "header", + [{"Authorization": "Bearer not-a-real-token"}, {"Authorization": "Basic cm9vdDpyb290"}], +) +def test_an_invalid_or_unsupported_credential_is_401( + client: TestClient, admin_token: str, header: dict[str, str] +) -> None: + assert client.get("/auth/me", headers=header).status_code == 401 + + +def test_a_bearer_header_wins_over_a_session_cookie(client: TestClient, auth: AuthHelper) -> None: + """A CLI run must use the token it was given, not whichever session the jar holds.""" + auth.bootstrap() + client.post("/auth/login", json={"username": "root", "password": ADMIN_PASSWORD}) + + response = client.get("/auth/me", headers={"Authorization": "Bearer not-a-real-token"}) + + assert response.status_code == 401 + + +def test_creating_a_user_hashes_the_password_and_never_stores_it( + client: TestClient, admin_token: str, database_url: str +) -> None: + password = "another-long-enough-password" + client.post( + "/users", + json={"username": "opal", "password": password, "role": "operator"}, + headers=bearer(admin_token), + ) + + async def work(db: AsyncSession) -> User: + return (await db.exec(select(User).where(User.username == "opal"))).one() + + stored = with_session(database_url, work) + assert stored.password_hash.startswith("$argon2id$") + assert password not in stored.password_hash + + +def test_a_duplicate_username_is_refused(client: TestClient, admin_token: str) -> None: + body = {"username": "opal", "password": "another-long-enough-password", "role": "viewer"} + assert client.post("/users", json=body, headers=bearer(admin_token)).status_code == 201 + + response = client.post("/users", json=body, headers=bearer(admin_token)) + + assert response.status_code == 409 + assert response.json()["error_code"] == "username_taken" + + +def test_a_short_password_is_refused_without_echoing_it( + client: TestClient, admin_token: str +) -> None: + response = client.post( + "/users", + json={"username": "opal", "password": "short", "role": "viewer"}, + headers=bearer(admin_token), + ) + + assert response.status_code == 400 + assert response.json()["error_code"] == "weak_password" + assert "short" not in response.json()["message"] + + +def test_every_authentication_action_writes_one_audit_row( + client: TestClient, auth: AuthHelper, database_url: str +) -> None: + """Rules PX-SECRETS, PX-EVIDENCE, D-05 - including the failed attempt.""" + auth.bootstrap() + token = auth.login("root", ADMIN_PASSWORD) + client.post("/auth/login", json={"username": "root", "password": "wrong-password-entirely"}) + client.post("/auth/logout", headers=bearer(token)) + + recorded = [(row.event_type, row.outcome) for row in _events(database_url)] + + assert ("bootstrap", "success") in recorded + assert ("login", "success") in recorded + assert ("login", "failure") in recorded + assert ("logout", "success") in recorded + + +def test_the_audit_trail_records_no_password_or_token( + client: TestClient, auth: AuthHelper, database_url: str +) -> None: + auth.bootstrap() + token = auth.login("root", ADMIN_PASSWORD) + + written = "\n".join( + f"{row.event_type} {row.outcome} {row.username_attempted} {row.detail}" + for row in _events(database_url) + ) + + assert ADMIN_PASSWORD not in written + assert token not in written + assert hash_token(token) not in written diff --git a/backend/tests/test_api_validation_errors.py b/backend/tests/test_api_validation_errors.py new file mode 100644 index 0000000..48de73d --- /dev/null +++ b/backend/tests/test_api_validation_errors.py @@ -0,0 +1,153 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +A 422 never hands the caller back what they sent (rules PX-SECRETS, PX-ERRORS). + +This file exists because the opposite was true and shipped. FastAPI's default validation +handler returns Pydantic's error list verbatim, and a Pydantic error carries the failing value +in ``input`` - for a ``missing`` error, the entire request object. Two live reflections came +out of that: ``POST /auth/login`` with the username omitted reflected the submitted password, +and ``POST /engagements/{id}/credential`` with ``cred_type`` omitted reflected the submitted +credential value. The second had been shipping since the endpoint existed. + +What is reflected is the caller's own input, not stored state - validation runs before the +handler, so nothing is read from the database. It still matters, because a secret in a response +body ends up in proxy logs, error trackers, and CI output. + +One handler covers every endpoint, so the guarantee is tested in one place rather than scattered +across the suites of the routes that happen to take a secret. Each case sends a body that +**contains** a planted secret and asserts the secret is absent from the answer - the same +planted-material discipline the auth no-leak tests use, through the same detector. +""" + +from __future__ import annotations + +import httpx +import pytest +from fastapi.testclient import TestClient +from provx_cli.api import _describe_validation + +from tests.conftest import bearer +from tests.support import leakcheck + +pytestmark = pytest.mark.usefixtures("database_url") + +#: Distinctive enough that a hit is unambiguous, and long enough to clear the password policy. +PLANTED_SECRET = "Zg7-planted-secret-must-never-echo-Zg7" + + +def _engagement(client: TestClient) -> str: + """An engagement to hang credential cases off.""" + response = client.post( + "/engagements", + json={ + "name": "validation probe", + "scope_allow": ["example.com"], + "targets": ["http://example.com"], + }, + ) + assert response.status_code == 201, response.text + return str(response.json()["id"]) + + +def _assert_no_echo(response: httpx.Response, label: str) -> None: + """Assert a 422 answered without repeating the planted secret.""" + assert response.status_code == 422, response.text + leakcheck.assert_absent([PLANTED_SECRET], leakcheck.from_response(label, response)) + + +#: The three shapes that leaked before the handler existed. A sibling field missing makes +#: Pydantic report the whole object as ``input``; a non-object body makes it report the body. +LEAKING_SHAPES = [ + ("sibling field missing", {"password": PLANTED_SECRET}), + ("body is a bare string", PLANTED_SECRET), + ("body is a list", [PLANTED_SECRET]), +] + + +@pytest.mark.parametrize(("label", "body"), LEAKING_SHAPES) +def test_a_malformed_login_never_echoes_the_password( + client: TestClient, label: str, body: object +) -> None: + _assert_no_echo(client.post("/auth/login", json=body), f"login {label}") + + +def test_a_malformed_bootstrap_never_echoes_the_password(client: TestClient) -> None: + _assert_no_echo(client.post("/auth/bootstrap", json={"password": PLANTED_SECRET}), "bootstrap") + + +def test_a_malformed_user_create_never_echoes_the_password( + client: TestClient, admin_token: str +) -> None: + """Admin-authed, because the role gate answers 401 before validation ever runs.""" + _assert_no_echo( + client.post("/users", json={"password": PLANTED_SECRET}, headers=bearer(admin_token)), + "user-create", + ) + + +def test_a_malformed_credential_never_echoes_the_value(client: TestClient) -> None: + """The PX-SECRETS case: this shape reflected a submitted scanning credential, on main.""" + engagement_id = _engagement(client) + + _assert_no_echo( + client.post(f"/engagements/{engagement_id}/credential", json={"value": PLANTED_SECRET}), + "credential", + ) + + +def test_a_malformed_engagement_never_echoes_the_inline_spec(client: TestClient) -> None: + """A pasted spec is untrusted operator content that no read path returns (PX-SECRETS).""" + _assert_no_echo( + client.post( + "/engagements", + json={ + "scope_allow": ["example.com"], + "targets": ["http://example.com"], + "api_spec_inline": PLANTED_SECRET, + }, + ), + "engagement", + ) + + +def test_the_error_keeps_what_is_actionable_and_drops_what_is_not(client: TestClient) -> None: + """``type``/``loc``/``msg`` survive; ``input``/``ctx``/``url`` do not.""" + response = client.post("/auth/login", json={"password": PLANTED_SECRET}) + + (error,) = response.json()["detail"] + + assert set(error) == {"type", "loc", "msg"} + assert error["type"] == "missing" + assert error["loc"] == ["body", "username"] + assert error["msg"] == "Field required" + + +def test_the_detector_fires_against_the_shape_this_handler_replaced(client: TestClient) -> None: + """Control. Without it, every assertion above could be passing for the wrong reason. + + This is verbatim what ``POST /auth/login`` answered before the handler existed. Feeding it + to the same detector must raise - if it does not, the assertions above prove nothing. + """ + unsanitized = ( + '{"detail":[{"type":"missing","loc":["body","username"],"msg":"Field required",' + f'"input":{{"password":"{PLANTED_SECRET}"}}}}]}}' + ) + + with pytest.raises(leakcheck.SecretLeak): + leakcheck.assert_absent([PLANTED_SECRET], [("pre-fix 422 body", unsanitized)]) + + +def test_the_cli_still_renders_a_field_level_message(client: TestClient) -> None: + """The sanitized shape has to stay useful, not just safe. + + The CLI reads ``loc`` and ``msg`` to turn a 422 into one readable sentence. Dropping + ``input`` was chosen partly because it leaves that path untouched; this is what says so. + """ + response = client.post("/auth/login", json={"password": PLANTED_SECRET}) + + rendered = _describe_validation(response.json()["detail"]) + + assert "username" in rendered + assert "Field required" in rendered + assert PLANTED_SECRET not in rendered diff --git a/backend/tests/test_auth_bootstrap.py b/backend/tests/test_auth_bootstrap.py new file mode 100644 index 0000000..4c97fc0 --- /dev/null +++ b/backend/tests/test_auth_bootstrap.py @@ -0,0 +1,110 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +First-run admin bootstrap - and, more importantly, its permanent closure. + +``POST /auth/bootstrap`` is the only unauthenticated way to create a user. That is defensible +exactly as long as it is genuinely one-shot, so most of this file is about the refusal rather +than the creation. +""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient +from sqlmodel import delete, select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.models.tables import AuthBootstrap, User +from tests.conftest import ADMIN_PASSWORD, AuthHelper, bearer, with_session + +pytestmark = pytest.mark.usefixtures("database_url") + + +def test_the_first_admin_can_be_created_with_no_credentials(client: TestClient) -> None: + response = client.post("/auth/bootstrap", json={"username": "root", "password": ADMIN_PASSWORD}) + + assert response.status_code == 201 + assert response.json()["role"] == "admin" + assert response.json()["is_active"] is True + + +def test_a_second_bootstrap_is_refused(client: TestClient) -> None: + body = {"username": "root", "password": ADMIN_PASSWORD} + client.post("/auth/bootstrap", json=body) + + response = client.post( + "/auth/bootstrap", json={"username": "root2", "password": ADMIN_PASSWORD} + ) + + assert response.status_code == 409 + assert response.json()["error_code"] == "bootstrap_closed" + + +def test_the_refusal_creates_nothing(client: TestClient, admin_token: str) -> None: + client.post("/auth/bootstrap", json={"username": "root2", "password": ADMIN_PASSWORD}) + + listing = client.get("/users", headers=bearer(admin_token)).json() + + assert {row["username"] for row in listing} == {"root"} + + +def test_deleting_the_admin_does_not_re_open_bootstrap( + client: TestClient, database_url: str +) -> None: + """The marker outlives the user it created; that is why it is a row and not a count.""" + client.post("/auth/bootstrap", json={"username": "root", "password": ADMIN_PASSWORD}) + + async def wipe_users(db: AsyncSession) -> None: + await db.exec(delete(User).where(User.username == "root")) # type: ignore[call-overload] + await db.commit() + + with_session(database_url, wipe_users) + + response = client.post( + "/auth/bootstrap", json={"username": "root2", "password": ADMIN_PASSWORD} + ) + + assert response.status_code == 409 + assert response.json()["error_code"] == "bootstrap_closed" + + +def test_the_marker_row_is_a_singleton(client: TestClient, database_url: str) -> None: + client.post("/auth/bootstrap", json={"username": "root", "password": ADMIN_PASSWORD}) + client.post("/auth/bootstrap", json={"username": "root2", "password": ADMIN_PASSWORD}) + + async def markers(db: AsyncSession) -> list[AuthBootstrap]: + return list((await db.exec(select(AuthBootstrap))).all()) + + rows = list(with_session(database_url, markers)) + + assert len(rows) == 1 + assert rows[0].id == 1 + + +def test_a_short_bootstrap_password_is_refused_and_leaves_bootstrap_open( + client: TestClient, +) -> None: + """A rejected attempt must not burn the one shot - the operator has to be able to retry.""" + assert ( + client.post("/auth/bootstrap", json={"username": "root", "password": "short"}).status_code + == 400 + ) + + assert ( + client.post( + "/auth/bootstrap", json={"username": "root", "password": ADMIN_PASSWORD} + ).status_code + == 201 + ) + + +def test_the_bootstrapped_admin_can_immediately_log_in_and_manage_users( + client: TestClient, auth: AuthHelper +) -> None: + auth.bootstrap() + token = auth.login("root", ADMIN_PASSWORD) + + created = auth.create_user(token, "opal", "operator", "another-long-enough-password") + + assert created["role"] == "operator" diff --git a/backend/tests/test_auth_no_leak.py b/backend/tests/test_auth_no_leak.py new file mode 100644 index 0000000..9c1b3c8 --- /dev/null +++ b/backend/tests/test_auth_no_leak.py @@ -0,0 +1,267 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +The deliverable: a password and a raw token are one-way and never recoverable. + +A known password is planted, used across the whole authentication surface, and then hunted for +in every place a client or an operator could read: response bodies, response headers, the +``/auth/me`` and ``/users`` payloads, every captured log record including its ``extra=`` +payload, and the evidence store. Its stored digest is hunted for in the same places. + +The detector is proven to work before it is trusted - see the control tests at the bottom. +Without them this file would assert that a secret is absent from places nobody verified were +being searched. +""" + +from __future__ import annotations + +import logging + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlmodel import select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.main import handle_unexpected_exception +from app.models.tables import FindingEvidenceRow, User +from app.security.tokens import hash_token +from tests.conftest import AuthHelper, bearer, with_session +from tests.support import leakcheck + +pytestmark = pytest.mark.usefixtures("database_url") + +#: Distinctive enough that a hit is a real hit, and long enough to clear the password policy. +PLANTED_PASSWORD = "Zg7-planted-password-never-recoverable-Zg7" + + +def _stored_hash(database_url: str, username: str) -> str: + async def work(db: AsyncSession) -> str: + return (await db.exec(select(User).where(User.username == username))).one().password_hash + + return str(with_session(database_url, work)) + + +def _exercise_the_whole_auth_surface( + client: TestClient, caplog: pytest.LogCaptureFixture +) -> tuple[str, list[tuple[str, str]]]: + """Drive bootstrap, login, me, user-create, list, a failed login, and logout. + + Returns: + The raw token, and every response as a labelled searchable haystack. + """ + with caplog.at_level(logging.DEBUG): + bootstrap = client.post( + "/auth/bootstrap", json={"username": "root", "password": PLANTED_PASSWORD} + ) + login = client.post("/auth/login", json={"username": "root", "password": PLANTED_PASSWORD}) + token = str(login.json()["token"]) + client.cookies.clear() + me = client.get("/auth/me", headers=bearer(token)) + created = client.post( + "/users", + json={"username": "opal", "password": PLANTED_PASSWORD, "role": "operator"}, + headers=bearer(token), + ) + listing = client.get("/users", headers=bearer(token)) + bad_login = client.post( + "/auth/login", json={"username": "root", "password": PLANTED_PASSWORD + "-wrong"} + ) + logout = client.post("/auth/logout", headers=bearer(token)) + after = client.get("/auth/me", headers=bearer(token)) + # The malformed shapes that used to hand the password straight back, before + # app.main.handle_validation_error existed. They ride along here so the file's + # existing absence assertions cover them without a second set of their own. + malformed = client.post("/auth/login", json={"password": PLANTED_PASSWORD}) + malformed_body = client.post("/auth/login", json=PLANTED_PASSWORD) + + assert (bootstrap.status_code, login.status_code, me.status_code) == (201, 200, 200) + assert (created.status_code, listing.status_code) == (201, 200) + assert (bad_login.status_code, logout.status_code, after.status_code) == (401, 204, 401) + assert (malformed.status_code, malformed_body.status_code) == (422, 422) + + responses = [ + *leakcheck.from_response("bootstrap", bootstrap), + *leakcheck.from_response("login", login), + *leakcheck.from_response("me", me), + *leakcheck.from_response("user-create", created), + *leakcheck.from_response("user-list", listing), + *leakcheck.from_response("bad-login", bad_login), + *leakcheck.from_response("logout", logout), + *leakcheck.from_response("me-after-logout", after), + *leakcheck.from_response("malformed-login", malformed), + *leakcheck.from_response("malformed-login-body", malformed_body), + ] + return token, responses + + +def test_the_password_and_the_raw_token_reach_no_response_and_no_log_at_all( + client: TestClient, caplog: pytest.LogCaptureFixture +) -> None: + """The headline claim, asserted against **every** captured record, third-party included. + + Nothing is excused here. The password is never persisted and never formatted into a + message, and the raw token exists only in the login response it was issued in - so neither + has any route into a log, whichever library is doing the logging. + """ + token, responses = _exercise_the_whole_auth_surface(client, caplog) + + everything = [*responses, *leakcheck.from_records("captured", caplog.records)] + + leakcheck.assert_absent([PLANTED_PASSWORD], everything) + # The raw token is legitimately in the login body and its Set-Cookie header, and nowhere + # else on earth. + leakcheck.assert_absent([token], [h for h in everything if not h[0].startswith("login ")]) + + +def test_the_stored_hash_reaches_no_response_and_no_provx_log( + client: TestClient, database_url: str, caplog: pytest.LogCaptureFixture +) -> None: + """The digest is never published and never logged by Provx's own loggers. + + Scoped to ``app`` and ``provx_sdk`` for the log half, matching the credential no-leak test + next door and for the same reason (KI-007): at DEBUG the database driver echoes bound + statement parameters, and a stored hash is one. That is a third-party debug facility Provx + does not enable in production - the response half below is unscoped and absolute. + """ + _, responses = _exercise_the_whole_auth_surface(client, caplog) + stored_hash = _stored_hash(database_url, "root") + provx_records = [r for r in caplog.records if r.name.startswith(("app", "provx_sdk"))] + + assert provx_records, "nothing was logged by Provx, so this assertion proves nothing" + leakcheck.assert_absent( + [stored_hash], [*responses, *leakcheck.from_records("provx", provx_records)] + ) + + +def test_no_api_payload_carries_a_password_hash_field(client: TestClient, auth: AuthHelper) -> None: + """Not "the value is absent" but "the field does not exist" (rule B-FA-07).""" + auth.bootstrap(password=PLANTED_PASSWORD) + token = auth.login("root", PLANTED_PASSWORD) + + payloads = [ + client.get("/auth/me", headers=bearer(token)).json(), + *client.get("/users", headers=bearer(token)).json(), + ] + + for payload in payloads: + assert "password_hash" not in payload + assert "password" not in payload + + +def test_the_password_never_reaches_the_evidence_store( + client: TestClient, auth: AuthHelper, database_url: str +) -> None: + """The auth path and the evidence path are unconnected, and this is what says so.""" + auth.bootstrap(password=PLANTED_PASSWORD) + auth.login("root", PLANTED_PASSWORD) + + async def work(db: AsyncSession) -> list[str]: + rows = (await db.exec(select(FindingEvidenceRow))).all() + return [row.evidence_tool_output or "" for row in rows] + + stored = "\n".join(with_session(database_url, work)) + + assert PLANTED_PASSWORD not in stored + + +def test_a_password_is_never_recoverable_from_what_is_stored( + client: TestClient, auth: AuthHelper, database_url: str +) -> None: + """The stored digest verifies the password and yields nothing else about it.""" + auth.bootstrap(password=PLANTED_PASSWORD) + + digest = _stored_hash(database_url, "root") + + assert PLANTED_PASSWORD not in digest + for length in range(6, len(PLANTED_PASSWORD)): + assert PLANTED_PASSWORD[:length] not in digest + + +def test_a_handler_that_leaks_the_password_is_caught_through_the_real_error_path( + caplog: pytest.LogCaptureFixture, +) -> None: + """The mutation control for the headline claim, and the reason it is worth anything. + + The other assertions in this file say a password is absent from responses and logs. On + their own that is unfalsifiable - a detector that found nothing anywhere would pass them + all. This mounts a deliberately leaky handler and shows the same detector, fed the same + way, refuses it. + + The leak travels the **production** error path: ``handle_unexpected_exception`` is the + shipped handler, registered here rather than reimplemented, and it echoes ``repr(exc)`` + whenever ``APP_ENV`` is a debug environment - which ``testing`` is. So this is not a + hypothetical leak shape, it is the one a raise-with-the-password would actually produce. + """ + probe = FastAPI() + probe.add_exception_handler(Exception, handle_unexpected_exception) + probe_logger = logging.getLogger("app.probe") + + @probe.get("/leaky") + async def leaky() -> dict[str, str]: + probe_logger.info("authenticating", extra={"password": PLANTED_PASSWORD}) + raise RuntimeError(f"could not authenticate with {PLANTED_PASSWORD}") + + with caplog.at_level(logging.DEBUG): + with TestClient(probe, raise_server_exceptions=False) as probe_client: + response = probe_client.get("/leaky") + + assert response.status_code == 500 + assert PLANTED_PASSWORD in response.text, "the probe did not actually leak; test is void" + + with pytest.raises(leakcheck.SecretLeak): + leakcheck.assert_absent([PLANTED_PASSWORD], leakcheck.from_response("leaky", response)) + with pytest.raises(leakcheck.SecretLeak): + leakcheck.assert_absent([PLANTED_PASSWORD], leakcheck.from_records("leaky", caplog.records)) + + +def test_the_detector_fails_on_a_planted_leak_in_a_body() -> None: + """Mutation control. A serializer that leaked the hash would look like this.""" + leaky = [("response body", f'{{"username": "root", "password_hash": "{PLANTED_PASSWORD}"}}')] + + with pytest.raises(leakcheck.SecretLeak): + leakcheck.assert_absent([PLANTED_PASSWORD], leaky) + + +def test_the_detector_fails_on_a_planted_leak_in_a_header() -> None: + """Mutation control for the header path, which a body-only scan would miss.""" + leaky = [("response headers", f'{{"set-cookie": "session={PLANTED_PASSWORD}"}}')] + + with pytest.raises(leakcheck.SecretLeak): + leakcheck.assert_absent([PLANTED_PASSWORD], leaky) + + +def test_the_detector_fails_on_a_secret_hidden_in_a_log_records_extra_payload() -> None: + """Mutation control for the gap a ``getMessage()``-based scan has. + + ``extra={...}`` is how this codebase attaches structured context, and it does not appear in + the rendered message. A detector that missed it would pass while the secret sat in the log. + """ + record = logging.LogRecord( + name="app.api.auth", + level=logging.INFO, + pathname=__file__, + lineno=1, + msg="login succeeded", + args=(), + exc_info=None, + ) + record.password = PLANTED_PASSWORD + + assert PLANTED_PASSWORD not in record.getMessage() + with pytest.raises(leakcheck.SecretLeak): + leakcheck.assert_absent([PLANTED_PASSWORD], leakcheck.from_records("captured", [record])) + + +def test_the_detector_finds_a_token_that_matches_its_own_stored_hash() -> None: + """Guards the shape of the token assertions: hash and raw value are distinct needles.""" + token = "a-token-value" + + assert hash_token(token) != token + with pytest.raises(leakcheck.SecretLeak): + leakcheck.assert_absent([hash_token(token)], [("row", hash_token(token))]) + + +def test_the_detector_ignores_an_empty_needle() -> None: + """An empty secret matches everything; treating it as a hit would make every test pass.""" + leakcheck.assert_absent([""], [("anywhere", "some text")]) diff --git a/backend/tests/test_auth_passwords.py b/backend/tests/test_auth_passwords.py new file mode 100644 index 0000000..e365116 --- /dev/null +++ b/backend/tests/test_auth_passwords.py @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +The password hashing primitive (rules PX-SECRETS, PX-FREE). + +The property under test is one-wayness, not correctness of a library: what matters is that the +stored digest neither contains nor yields the password, and that a wrong password is refused. +""" + +from __future__ import annotations + +import pytest + +from app.security.passwords import ( + MIN_PASSWORD_LENGTH, + PasswordPolicyError, + hash_password, + validate_password, + verify_password, +) + +PASSWORD = "correct-horse-battery-staple" + + +def test_the_digest_is_argon2id_and_does_not_contain_the_password() -> None: + digest = hash_password(PASSWORD) + + assert digest.startswith("$argon2id$") + assert PASSWORD not in digest + + +def test_the_same_password_hashes_differently_every_time() -> None: + """A fresh salt per hash, so two operators with the same password have unequal rows.""" + assert hash_password(PASSWORD) != hash_password(PASSWORD) + + +def test_the_correct_password_verifies() -> None: + assert verify_password(hash_password(PASSWORD), PASSWORD) is True + + +@pytest.mark.parametrize( + "candidate", + ["", "wrong", PASSWORD.upper(), PASSWORD + " ", " " + PASSWORD, PASSWORD[:-1]], +) +def test_a_wrong_password_is_refused(candidate: str) -> None: + assert verify_password(hash_password(PASSWORD), candidate) is False + + +@pytest.mark.parametrize("stored", ["", "not-a-hash", "$argon2id$v=19$garbage", "plaintext"]) +def test_a_malformed_digest_answers_false_rather_than_raising(stored: str) -> None: + """A corrupt row must not become a 500 that tells the caller the row is corrupt.""" + assert verify_password(stored, PASSWORD) is False + + +def test_the_parameters_are_the_owasp_baseline() -> None: + """Pinned so a future edit that weakens the cost has to change this line deliberately.""" + digest = hash_password(PASSWORD) + + assert "m=65536" in digest + assert "t=3" in digest + assert "p=4" in digest + + +def test_a_short_password_is_rejected_with_a_message_that_omits_it() -> None: + short = "x" * (MIN_PASSWORD_LENGTH - 1) + + with pytest.raises(PasswordPolicyError) as caught: + validate_password(short) + + assert short not in str(caught.value) + + +def test_a_long_enough_password_passes_the_policy() -> None: + validate_password("x" * MIN_PASSWORD_LENGTH) diff --git a/backend/tests/test_auth_roles.py b/backend/tests/test_auth_roles.py new file mode 100644 index 0000000..948614e --- /dev/null +++ b/backend/tests/test_auth_roles.py @@ -0,0 +1,149 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +The role gate (rules B-FA-03, A-01, A-02). + +Two things are proven here. First, that the admin-only routes admit an admin and refuse an +operator, a viewer, and an anonymous caller. Second - and this is the part that makes the first +worth anything - that the 403 is produced by ``require_role`` and not by something incidental. +The control mounts the same handler behind a no-op dependency and shows it answers 200. +""" + +from __future__ import annotations + +import pytest +from fastapi import Depends, FastAPI, status +from fastapi.testclient import TestClient + +from app.api.deps import Capability, can, current_user, require_capability, require_role +from app.models.tables import Role, User +from tests.conftest import AuthHelper, bearer, deactivate + +pytestmark = pytest.mark.usefixtures("database_url") + + +@pytest.mark.parametrize("path", ["/users", "/auth/me"]) +def test_an_anonymous_request_to_a_protected_route_is_401(client: TestClient, path: str) -> None: + response = client.get(path) + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.json()["error_code"] == "unauthenticated" + + +def test_an_admin_may_list_users(client: TestClient, role_tokens: dict[str, str]) -> None: + response = client.get("/users", headers=bearer(role_tokens["admin"])) + + assert response.status_code == status.HTTP_200_OK + assert {row["username"] for row in response.json()} == {"root", "opal", "vera"} + + +@pytest.mark.parametrize("role", ["operator", "viewer"]) +def test_a_non_admin_may_not_list_users( + client: TestClient, role_tokens: dict[str, str], role: str +) -> None: + response = client.get("/users", headers=bearer(role_tokens[role])) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.json()["error_code"] == "forbidden" + + +@pytest.mark.parametrize("role", ["operator", "viewer"]) +def test_a_non_admin_may_not_create_users( + client: TestClient, role_tokens: dict[str, str], role: str +) -> None: + response = client.post( + "/users", + json={"username": "intruder", "password": "a-long-enough-password", "role": "admin"}, + headers=bearer(role_tokens[role]), + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + # The refusal has to be real, not cosmetic: nothing was created. + listing = client.get("/users", headers=bearer(role_tokens["admin"])).json() + assert "intruder" not in {row["username"] for row in listing} + + +def test_the_403_is_produced_by_require_role_and_not_by_something_incidental( + client: TestClient, role_tokens: dict[str, str] +) -> None: + """The mutation control for the role gate. + + The same handler is mounted twice on a throwaway app - once behind ``require_role(ADMIN)`` + and once behind a dependency that waves everyone through. A viewer's token gets 403 from + the first and 200 from the second, so reverting ``require_role`` to a no-op is a change + this test notices rather than one it tolerates. + """ + probe = FastAPI() + guard = Depends(require_role(Role.ADMIN)) + no_op = Depends(current_user) + + @probe.get("/guarded") + async def guarded(user: User = guard) -> dict[str, str]: + return {"username": user.username} + + @probe.get("/ungated") + async def ungated(user: User = no_op) -> dict[str, str]: + return {"username": user.username} + + headers = bearer(role_tokens["viewer"]) + with TestClient(probe) as probe_client: + assert probe_client.get("/guarded", headers=headers).status_code == 403 + assert probe_client.get("/ungated", headers=headers).status_code == 200 + + +def test_require_capability_admits_exactly_the_roles_that_carry_it( + client: TestClient, role_tokens: dict[str, str] +) -> None: + """The capability form of the gate, which the enforcement pass will use. + + ``RUN_SCAN`` belongs to admin and operator, so a viewer is the one refused. + """ + probe = FastAPI() + may_scan = Depends(require_capability(Capability.RUN_SCAN)) + + @probe.get("/scan-ish") + async def scan_ish(user: User = may_scan) -> dict[str, str]: + return {"username": user.username} + + with TestClient(probe) as probe_client: + statuses = { + role: probe_client.get("/scan-ish", headers=bearer(token)).status_code + for role, token in role_tokens.items() + } + + assert statuses == {"admin": 200, "operator": 200, "viewer": 403} + + +@pytest.mark.parametrize( + ("role", "capability", "permitted"), + [ + (Role.ADMIN, Capability.MANAGE_USERS, True), + (Role.ADMIN, Capability.RUN_SCAN, True), + (Role.OPERATOR, Capability.MANAGE_USERS, False), + (Role.OPERATOR, Capability.CREATE_ENGAGEMENT, True), + (Role.OPERATOR, Capability.RUN_SCAN, True), + (Role.OPERATOR, Capability.APPROVE_ACTIVE_SCAN, True), + (Role.VIEWER, Capability.READ, True), + (Role.VIEWER, Capability.RUN_SCAN, False), + (Role.VIEWER, Capability.MANAGE_USERS, False), + (Role.VIEWER, Capability.APPROVE_ACTIVE_SCAN, False), + ], +) +def test_the_capability_table_says_what_the_brief_says( + role: Role, capability: Capability, permitted: bool +) -> None: + """The policy itself, asserted as data rather than inferred from route behaviour.""" + assert can(role, capability) is permitted + + +def test_a_deactivated_operator_is_refused_even_with_a_live_token( + client: TestClient, auth: AuthHelper, admin_token: str, database_url: str +) -> None: + """Deactivation takes effect on the next request, without waiting for the token to expire.""" + auth.create_user(admin_token, "opal", "operator", "a-long-enough-password") + operator_token = auth.login("opal", "a-long-enough-password") + assert client.get("/auth/me", headers=bearer(operator_token)).status_code == 200 + + deactivate(database_url, "opal") + + assert client.get("/auth/me", headers=bearer(operator_token)).status_code == 401 diff --git a/backend/tests/test_auth_tokens.py b/backend/tests/test_auth_tokens.py new file mode 100644 index 0000000..9356f27 --- /dev/null +++ b/backend/tests/test_auth_tokens.py @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +The token primitive: entropy, one-way storage, and expiry arithmetic (rules PX-SECRETS, A-04). +""" + +from __future__ import annotations + +import hashlib +import re +from datetime import UTC, datetime, timedelta + +from app.security.tokens import expiry_from, generate_token, hash_token + + +def test_tokens_are_unique_across_many_draws() -> None: + """Cheap smoke test for a real CSPRNG: 500 draws, no collision, no short values.""" + drawn = {generate_token() for _ in range(500)} + + assert len(drawn) == 500 + assert all(len(token) >= 40 for token in drawn) + + +def test_tokens_are_url_safe() -> None: + """They travel in a header and a cookie, so the alphabet has to be safe in both.""" + assert re.fullmatch(r"[A-Za-z0-9_-]+", generate_token()) + + +def test_the_stored_form_is_sha256_and_is_not_the_token() -> None: + token = generate_token() + + stored = hash_token(token) + + assert stored == hashlib.sha256(token.encode()).hexdigest() + assert stored != token + assert token not in stored + + +def test_hashing_is_deterministic_so_a_presented_token_resolves_by_lookup() -> None: + token = generate_token() + + assert hash_token(token) == hash_token(token) + + +def test_expiry_is_the_ttl_after_the_issue_instant() -> None: + issued = datetime(2026, 7, 30, 12, 0, tzinfo=UTC) + + assert expiry_from(12, issued_at=issued) == issued + timedelta(hours=12) + + +def test_expiry_defaults_to_now_and_is_timezone_aware() -> None: + expires = expiry_from(1) + + assert expires.tzinfo is not None + assert expires > datetime.now(UTC) diff --git a/backend/tests/test_integration_cli.py b/backend/tests/test_integration_cli.py index b1069c2..934e843 100644 --- a/backend/tests/test_integration_cli.py +++ b/backend/tests/test_integration_cli.py @@ -41,6 +41,9 @@ SECRET = "s3cr3t-bearer-token-value-xyz" +#: The operator password the auth walkthrough plants. Long enough to clear the policy. +OPERATOR_PASSWORD = "an-operator-password-nobody-should-see" + @pytest.fixture def target() -> Iterator[str]: @@ -269,3 +272,70 @@ def test_the_credential_value_never_leaves_the_request( connection.close() assert rows, "the credential should have been stored" assert all(SECRET not in str(row[0]) for row in rows) + + +def test_bootstrap_login_and_an_authenticated_command_work_end_to_end( + client: TestClient, + database_url: str, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """The first-run path an operator actually walks, with nothing stubbed. + + ``provx admin create`` against a fresh database, then ``provx login``, then a command that + presents the stored token to the real API. The CLI's own suite stubs the transport, so this + is what proves the request and response shapes on both sides genuinely agree. + """ + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "config")) + monkeypatch.delenv("PROVX_TOKEN", raising=False) + monkeypatch.setenv("PROVX_PASSWORD", OPERATOR_PASSWORD) + + created = run( + ["admin", "create", "--username", "root", "--password-env", "PROVX_PASSWORD"], http=client + ) + logged_in = run( + ["login", "--username", "root", "--password-env", "PROVX_PASSWORD"], http=client + ) + + assert (created, logged_in) == (exit_codes.OK, exit_codes.OK) + captured = capsys.readouterr() + assert OPERATOR_PASSWORD not in captured.out + captured.err + + # The stored token authenticates a real protected endpoint. + from provx_cli import store + + token = store.load_token("http://localhost:8000") + assert token is not None + assert client.get("/auth/me", headers={"Authorization": f"Bearer {token}"}).status_code == 200 + + # And a second bootstrap is refused, over the same live stack. + assert ( + run( + ["admin", "create", "--username", "root2", "--password-env", "PROVX_PASSWORD"], + http=client, + ) + == exit_codes.API_ERROR + ) + + +def test_logout_through_the_cli_revokes_the_token_at_the_api( + client: TestClient, + database_url: str, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "config")) + monkeypatch.delenv("PROVX_TOKEN", raising=False) + monkeypatch.setenv("PROVX_PASSWORD", OPERATOR_PASSWORD) + run(["admin", "create", "--username", "root", "--password-env", "PROVX_PASSWORD"], http=client) + run(["login", "--username", "root", "--password-env", "PROVX_PASSWORD"], http=client) + + from provx_cli import store + + token = store.load_token("http://localhost:8000") + assert run(["logout"], http=client) == exit_codes.OK + + assert store.load_token("http://localhost:8000") is None + assert token is not None + assert client.get("/auth/me", headers={"Authorization": f"Bearer {token}"}).status_code == 401 diff --git a/backend/tests/test_migrations.py b/backend/tests/test_migrations.py index 4ef1082..c9f5fcf 100644 --- a/backend/tests/test_migrations.py +++ b/backend/tests/test_migrations.py @@ -128,6 +128,73 @@ def test_the_api_spec_columns_reverse_cleanly(alembic_config: Config) -> None: assert not ({"api_spec_url", "api_spec_inline"} & columns) +def test_upgrade_creates_the_auth_tables(alembic_config: Config) -> None: + command.upgrade(alembic_config, "head") + + engine = create_engine(_sync_url(alembic_config)) + inspector = inspect(engine) + tables = set(inspector.get_table_names()) + user_columns = {c["name"] for c in inspector.get_columns("auth_user")} + engine.dispose() + + assert {"auth_user", "auth_token", "auth_bootstrap", "auth_event"} <= tables + assert {"username", "email", "password_hash", "role", "is_active"} <= user_columns + + +def test_the_username_and_token_hash_are_unique(alembic_config: Config) -> None: + """Two operators cannot share a name, and two sessions cannot share a token digest.""" + command.upgrade(alembic_config, "head") + + engine = create_engine(_sync_url(alembic_config)) + inspector = inspect(engine) + unique_columns = { + index["name"]: set(index["column_names"]) + for table in ("auth_user", "auth_token") + for index in inspector.get_indexes(table) + if index["unique"] + } + engine.dispose() + + assert {"username"} in unique_columns.values() + assert {"token_hash"} in unique_columns.values() + + +def test_the_auth_migration_reverses_cleanly(alembic_config: Config) -> None: + """Rule W-03: the auth tables undo to the pre-auth head, leaving the rest intact.""" + command.upgrade(alembic_config, "head") + command.downgrade(alembic_config, "a2b3c4d5e6f7") + + engine = create_engine(_sync_url(alembic_config)) + tables = set(inspect(engine).get_table_names()) + engine.dispose() + + assert not ({"auth_user", "auth_token", "auth_bootstrap", "auth_event"} & tables) + assert EXPECTED_TABLES <= tables + + +def test_the_auth_model_metadata_matches_the_migration(alembic_config: Config) -> None: + """Model and migration must agree, or every test runs against a schema production lacks. + + The suite builds its schema from ``SQLModel.metadata``, so a column declared only in the + migration would never be exercised and a column declared only on the model would never + reach a real database. This compares both directions for all four auth tables. + """ + from sqlmodel import SQLModel + + command.upgrade(alembic_config, "head") + + engine = create_engine(_sync_url(alembic_config)) + inspector = inspect(engine) + migrated = { + name: {c["name"] for c in inspector.get_columns(name)} + for name in ("auth_user", "auth_token", "auth_bootstrap", "auth_event") + } + engine.dispose() + + for name, columns in migrated.items(): + assert columns == {c.name for c in SQLModel.metadata.tables[name].columns}, name + + def test_downgrade_reverses_the_migration(alembic_config: Config) -> None: command.upgrade(alembic_config, "head") command.downgrade(alembic_config, "base") diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md index b717d3b..404d088 100644 --- a/docs/KNOWN_ISSUES.md +++ b/docs/KNOWN_ISSUES.md @@ -98,6 +98,15 @@ scope, so this issue's urgency trigger (an untrusted party supplying a hostname) reached. Unchanged and open; the pinned-resolution transport remains the fix and is out of scope for the credential work. +**Update (`feat/auth-foundation`):** identity, roles, and tokens now exist, and the trigger is +one change closer. It is **not** reached yet, on two counts: enforcement covers `/auth` and +`/users` only, so every engagement route — including the one that accepts a scope — is still +unauthenticated; and the only account a deployment has is the one its operator bootstrapped, so +there is no untrusted party to supply a hostname. **Both stop being true in the next change.** +The moment `POST /engagements` requires a role, a non-admin operator can put a hostname into +scope, and at that point this is the top item — the enforcement PR should either carry the +pinned-resolution transport or state in writing why it does not. + **Fix sketch:** resolve once, validate the resolved address, then connect to that address with the original `Host` header — a pinned-resolution transport rather than a pre-flight lookup. @@ -381,16 +390,123 @@ KI-003 (dangerous-range refusal on IP literals only) becomes materially more rel credentialed, crafted requests fly: it stays the top item to close the moment user-supplied scope and auth land. +## KI-010 — Validation errors echoed the submitted password and credential value + +**Severity:** High · **Status:** ✅ RESOLVED · **Source:** pre-merge audit of `feat/auth-foundation` +**Site:** [`backend/app/main.py`](../backend/app/main.py) — there was no `RequestValidationError` +handler, so FastAPI's default one served every 422. + +FastAPI's default validation handler returns Pydantic's error list verbatim, and a Pydantic error +carries the value that failed in `input`. For a `missing` error that value is the **whole request +object**. On any endpoint accepting a secret, a malformed request therefore handed the secret +straight back: + +``` +POST /auth/login {"password": ""} # username omitted +422 -> "input": {"password": ""} + +POST /engagements/{id}/credential {"value": ""} # cred_type omitted +422 -> "input": {"value": ""} +``` + +`api_spec_inline` echoed the same way. Three shapes leaked: a sibling field missing, a non-object +body, and a list body. Wrong-type, extra-field, and pattern failures did not. + +**What this is, and what it is not.** Validation runs *before* the route handler, so nothing is +read from the database: what comes back is the caller's own submitted value, reflected. This is +**not** a path for one party to read another's stored credentials, and calling it that would +overstate it. It is CWE-209 - a secret placed into a response body, where it comes to rest in +the places response bodies go: reverse-proxy and gateway access logs, error trackers, CI job +output, browser devtools. For a tool whose whole promise is that it protects its own secrets, +that is still a defect worth a fix and an advisory; it is a Moderate one, not a critical one. + +**This was live, and it predates the auth work.** No `RequestValidationError` handler existed on +`main` either, so `POST /engagements/{id}/credential` had been reflecting submitted scanning +credentials for as long as the endpoint has existed (rule PX-SECRETS). The authentication branch +did not introduce it; it added three more password-bearing fields to an already-leaking surface, +and the no-leak work is what found it. + +**Why it was not caught earlier.** The credential no-leak test +(`test_integration_authenticated.py`) checks evidence, findings, the report, and Provx's own logs +— the paths a credential was expected to travel. Nobody had asked what an endpoint returns when +the request is *malformed*, so the 422 path was never exercised with a secret in the body. The +lesson generalizes: a no-leak suite that only drives the happy path tests the paths you thought +of. + +**✅ Resolved.** `feat/auth-foundation` adds `handle_validation_error` in `app/main.py` — one +handler on the app, not per-router, because every endpoint taking a password, credential, or token +had the same exposure. `type`, `loc`, and `msg` survive; `input`, `ctx`, and `url` are dropped. +FastAPI's `{"detail": [...]}` shape is kept deliberately rather than moving to `ErrorResponse`: +the CLI branches on the envelope first, so changing shape would have discarded the per-field +message it renders. `test_api_validation_errors.py` sends bodies **containing** a planted secret +across `/auth/login`, `/auth/bootstrap`, `POST /users`, `POST /engagements/{id}/credential`, and +`POST /engagements`, and asserts absence through the same detector the auth tests use — with a +control proving that detector fires against the pre-fix shape, so a regression cannot pass +silently. + +**Residual, accepted.** `msg` is retained because it is what makes a 422 actionable. No validator +currently interpolates its input into a message, but a future one could. The guard is the test +above, which scans the whole response for planted material rather than trusting the message text. + +--- + +## Authentication foundation — deliberate limits after `feat/auth-foundation` + +The identity, role, and token machinery ships complete and proven. What it is **not yet** applied +to, and what it deliberately does not do, is recorded here rather than left to be discovered. + +- **Enforcement is not everywhere, and that is the whole shape of the change.** Authenticated: + `GET /auth/me`, `POST /auth/logout` (any operator), `POST /users` and `GET /users` (admin only). + **Still open:** every `/engagements/**` route — create, list, scan, credential, mode, + active-approvals, findings, transition, in-report, report — plus `GET /health` and `GET /`. + Rolling the gate across them is the next change and is a PR in its own right: it rewrites the + expectations of every existing API test, and doing that in the same commit as the crypto would + have meant neither half got read properly. **`docs/STATUS.md` carries the same list.** Until then + the API is, in practice, as open as it was — the foundation is real, the perimeter is not. +- **The bootstrap guard keys on the marker row, not on "an admin exists"** — tracked as + [issue #39](https://github.com/provx-sec/provx/issues/39). A database holding an admin but no + marker would permit re-bootstrap, which is a privilege-escalation path. Unreachable today, + because `/users` requires an admin and so no admin can exist without a prior bootstrap, but + the two facts are coupled implicitly rather than by the check itself. +- **No failed-login lockout (rule A-06).** Nothing rate-limits or locks an account after repeated + wrong passwords. argon2id at 64 MiB makes online guessing slow but not impossible. Every attempt + *is* recorded in `auth_event`, so the data a lockout needs already exists. Lands with, or + immediately after, the enforcement change. +- **No refresh-token rotation (rule A-05).** A token is issued at login and lives 12 hours; there is + no refresh flow to rotate, so there is nothing yet for A-05 to govern. Revisit when a console + session needs to outlive a working day. +- **No common-password denylist (rule S-12).** Password policy is a 12-character minimum and nothing + more. The repository already ships `wordlists/`, so this is a small addition deferred rather than + a missing capability. +- **No self-service password change, and no user deactivation endpoint.** An admin creates accounts; + changing a password or deactivating an operator is a database operation today. `is_active` is + honoured on every request, so a deactivation takes effect immediately once there is a way to set + it. +- **Sessions are opaque rows, not JWTs.** Revocation has to be immediate for a security product, and + a self-contained token cannot be revoked without the lookup a stateful token already does. The + cost is a database read per authenticated request; it is an indexed hash lookup. +- **At DEBUG, the database driver echoes bound statement parameters, including a password hash.** + The same class as KI-007, and the reason the hash no-leak assertion scopes its log half to the + `app` and `provx_sdk` loggers. Provx does not enable driver debug logging; the digest is not the + password and is not reversible; the password itself and the raw token appear in **no** log at any + level, which the unscoped half of that test asserts. + +--- + ## Not listed here -Absent features are not issues. Authentication, the job queue, the PX-DSL expression +Absent features are not issues. The job queue, the PX-DSL expression evaluator, exploitation (the sandboxed proof-of-exploit runner + per-finding approval), additional adapters, and **Word/`.docx` report export** are **declared scaffolding** — see [ROADMAP.md](ROADMAP.md) §4 for what is intentionally out of the current phase. Active mode was on this list until `feat/api-module-active`; it now ships as the engagement `mode` flag plus the per-run `ActiveScanApproval` queue that gates the intrusive API adapters. PDF reporting was on this list until `feat/dashboard-and-pdf`; it now ships as `GET /engagements/{id}/report?format=pdf` and -`provx report --format pdf`. +`provx report --format pdf`. Authentication was on this list until `feat/auth-foundation`; it now +ships as operator identity (argon2id), the admin/operator/viewer role gate, hashed session tokens +with a cookie and a bearer form, one-shot admin bootstrap, and `provx login` / `logout` / +`admin create` — **enforced on `/auth` and `/users` only**, with the engagement and scan routes +deliberately left open until the enforcement change that follows (see the entry below). Two operational notes that belong with the PDF work, neither of them a defect: diff --git a/docs/QUICKSTART.md b/docs/QUICKSTART.md index e5d1098..f792eac 100644 --- a/docs/QUICKSTART.md +++ b/docs/QUICKSTART.md @@ -70,11 +70,13 @@ curl -sS localhost:8000/health ``` > [!IMPORTANT] -> **The API and console have no authentication.** None. Anyone who can reach port 8000 can -> create engagements, launch scans, and read every finding and credential *label* on the -> server. Bind them to loopback only, keep them off shared networks, and do not put this on -> the public internet. Authentication is on the roadmap and is not built -> ([`ROADMAP.md`](ROADMAP.md) §4). +> **The engagement and scan routes still have no authentication.** Accounts, roles, and +> login exist (see §4), but the gate is applied only to `/auth/me`, `/auth/logout`, and +> `/users` so far. Anyone who can reach port 8000 can still create engagements, launch +> scans, and read every finding and credential *label* on the server. Bind it to loopback +> only, keep it off shared networks, and do not put this on the public internet. Enforcement +> across the remaining routes is the next change — [`STATUS.md`](STATUS.md) lists exactly +> which routes are gated today. `SAFE_MODE=true` is the shipped default. It means every intrusive adapter is refused outright, whatever else you ask for. Leave it alone for this walkthrough. @@ -116,6 +118,31 @@ pipx install ./packages/cli export PROVX_SERVER=http://localhost:8000 ``` +**First run only — create the administrator.** A fresh deployment has no accounts, so there +is one unauthenticated way in and it closes permanently the moment it is used: + +```bash +provx admin create --username root +# Password (input hidden): +provx login --username root +# Logged in to http://localhost:8000 as root (admin). Session stored in ~/.config/provx/credentials.json +``` + +There is no `--password` flag on either command, deliberately: a password given as a flag +lands in your shell history and in the process list where any local user can read it. Pipe it +on stdin or point `--password-env VAR` at an environment variable instead. A second +`provx admin create` is refused — permanently, and even if you delete the account it made. + +The password is stored as an argon2id hash and is not recoverable by anyone, including you. +The session token is stored on the server as a SHA-256 digest, so a stolen database row cannot +be replayed as a credential. `provx logout` revokes it. `$PROVX_TOKEN` still works and takes +precedence over the stored session, for CI. + +> **Only some endpoints require this today.** `/auth/me`, `/auth/logout`, and `/users` are +> authenticated; every `/engagements` route below is still open. Enforcing them is the next +> change — [`STATUS.md`](STATUS.md) carries the exact list. Log in anyway: the CLI presents +> the token everywhere, so nothing changes for you when enforcement lands. + `provx` is a thin client. It re-implements no scanning, scope check, or safety gate — every command is one HTTP call to the same endpoint the console uses, so the governance is enforced by the server either way. @@ -314,7 +341,12 @@ walks you through them: [`RESPONSIBLE_USE.md`](../RESPONSIBLE_USE.md) and [ADR-001](decisions/ADR-001-no-mutating-probes.md) before you turn any of it on. -Not built, so don't go looking: API authentication, a job queue, the playbook execution +- **Operator accounts.** An admin creates the rest with `POST /users` (`admin`, `operator`, + `viewer`). An operator creates engagements, runs scans, and approves active runs; a viewer + reads. The role gate is live on `/users` today and rolls out across the engagement and scan + routes in the next change. + +Not built, so don't go looking: a job queue, the playbook execution engine, exploitation, and any AI feature. [`KNOWN_ISSUES.md`](KNOWN_ISSUES.md) lists defects that are known and deferred. diff --git a/docs/STATUS.md b/docs/STATUS.md index e2aaae2..8d6c769 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -2,7 +2,7 @@ *The single source of truth for "where are we vs. the plan." Update this file as part of EVERY PR's Definition of Done. If it's not here, it's not tracked. Do not trust memory (human or AI) over this file.* -**Last updated:** branch `docs/contributing-accuracy` (issues #34 and #30 closed: CONTRIBUTING now describes the process the repo actually runs. §1/§3 claimed Conventional Commits "drives the automated changelog and version bumps" and the flow ended `→ auto changelog + version → next release` — machinery that was never built and, verified here, leaves no trace in the tree: no release-please config, no commitlint, no publish workflow. §3 now documents the real thing — `make version-set` writes all five declaration sites, the required `version-consistency` gate refuses a disagreement, `CHANGELOG.md` is hand-written under `[Unreleased]`, and tags are cut by hand with `release-check` as a post-hoc detector. §4's cookbook walked `build_command`/`parse_output` and told contributors to "copy the template adapter", which does not exist; it now walks the shipped `probe()` contract across all three protocols and — the part the old recipe never had — the two things a new adapter must add to be *scored*: the fixture pair and the lab positive/clean pair, including the three hand-maintained wiring lists an adapter can silently miss. **Automation was not built and is not implied**: the future item is named as future. The DoD gained the two steps the project already required in practice but never wrote down, STATUS.md and CHANGELOG, mirrored into the PR template. Also corrected where the same falsehood had propagated: `PROJECT_SETUP_PLAYBOOK` §B1/§B2, and `packages/adapters/README.md`, which was itself still calling the SDK "scaffolding", documenting `build_command` as the contract, omitting `ActiveApiAdapter` entirely, and pointing circularly back at the §4 it was supposed to be the authority for) · previously `fix/version-single-source` (issue #31 closed: Provx now declares **one version**, once per package, and CI refuses a disagreement — `/health` reported `0.0.0` against a `v0.1.1` tag because eight sites each declared a version and nothing held them equal. Source of truth is each package's own `__init__.py` via `dynamic = ["version"]`, because a root `VERSION` file and `git describe` are both unavailable where the packages are actually built — the image copies `packages/adapters` alone, with no `.git`. New required `version-consistency` gate plus a `release-check` tag assertion, `make version-set`, and `provx --version`. See the `## Versioning` section) · previously `docs/user-quickstart` (the first user-facing documentation: `docs/QUICKSTART.md`, written *after* executing every command it contains against a clean stack and a local OWASP Juice Shop target, and a README rewritten to lead with the identity and to state a status that no longer contradicts this file — it had been claiming Active mode and the API module were "absent by design" since before Phase 4 landed. Documentation is now tracked here, in the `## Documentation` section below, rather than being the one deliverable the plan-of-record never mentioned) · previously `ci/gate-workflow-edits` (issue #22 closed: CI configuration now gates changes to itself — a `.github/**` PR runs actionlint plus both CI self-tests, and the gate's own structure is asserted from `ci-required`, which a PR cannot switch off; `frontend-lint` became real, eslint + prettier, and joined the required set) · previously `ci/enforce-license-check` (issue #19 closed: the SPDX license-compatibility check named by PX-FREE and PX-LICENSE is a real, required CI gate rather than an `echo` stub — `pyphen`'s tri-license is now a pinned, machine-checked exception instead of a hand classification) · previously `docs/defer-mutating-tier` (ADR-001: the mutating-probe tier is **decided out**, not deferred — read-only active detection is the ceiling of the active tier, which completes Phase 4 rather than leaving it partial) · **Current phase:** Phase 4 — API module ✅ **complete** (Pass 1 passive ✅, Pass 2 active ✅, mutating tier decided out) alongside Phase 3 — Reporting (⏳ PDF ✅, dashboard list ✅, Word ⛔) · **Milestone:** v0.1.0 TAGGED (annotated tag at `9522dfa` / PR #12); the branded-PDF item in ROADMAP §11's v0.1 gate is now met, one tag late. +**Last updated:** branch `feat/auth-foundation` (**found and closed a live password- and credential-disclosure bug that predates this branch** — FastAPI's default validation handler returned Pydantic's error list verbatim, and a Pydantic `missing` error carries the whole request object in `input`, so `POST /auth/login` with the username omitted returned the submitted password and `POST /engagements/{id}/credential` with `cred_type` omitted reflected the submitted scanning credential. What comes back is the caller's own input, not stored state -- validation runs before the handler, so nothing is read from the database; the harm is that a secret in a response body ends up in proxy logs, error trackers, and CI output (CWE-209). No `RequestValidationError` handler existed on `main` either, so the credential case had been shipping for as long as the endpoint has; the auth work did not cause it, the auth work's planted-secret discipline is what found it. Fixed on this branch rather than split out, because "a password is never recoverable" is this branch's headline promise and a 422 that hands it back is a direct failure of it — see KI-010. Also: the authentication **foundation** — operator identity, argon2id passwords, admin/operator/viewer roles, hashed session tokens, one-shot admin bootstrap, and `provx login` / `logout` / `admin create` — **proven on a small endpoint set, not rolled out everywhere**. Enforcement covers `/auth/me`, `/auth/logout`, and both `/users` routes; every engagement route is deliberately untouched and still open, and the enforced/not-yet split is enumerated in the Authentication section below rather than left to be inferred. The split is the design: applying the gate across the API rewrites the expectations of every existing API test, and doing that in the same commit as new crypto means neither half gets read. The deliverable is not the endpoints, it is that a password and a raw token are one-way and never recoverable — planted, hunted for in every response body, response header, and log record including `extra=` payloads, and **the detector is itself proven to fail on a planted leak**. Three mutations were run by hand and each made the suite fail as intended) · previously branch `docs/contributing-accuracy` (issues #34 and #30 closed: CONTRIBUTING now describes the process the repo actually runs. §1/§3 claimed Conventional Commits "drives the automated changelog and version bumps" and the flow ended `→ auto changelog + version → next release` — machinery that was never built and, verified here, leaves no trace in the tree: no release-please config, no commitlint, no publish workflow. §3 now documents the real thing — `make version-set` writes all five declaration sites, the required `version-consistency` gate refuses a disagreement, `CHANGELOG.md` is hand-written under `[Unreleased]`, and tags are cut by hand with `release-check` as a post-hoc detector. §4's cookbook walked `build_command`/`parse_output` and told contributors to "copy the template adapter", which does not exist; it now walks the shipped `probe()` contract across all three protocols and — the part the old recipe never had — the two things a new adapter must add to be *scored*: the fixture pair and the lab positive/clean pair, including the three hand-maintained wiring lists an adapter can silently miss. **Automation was not built and is not implied**: the future item is named as future. The DoD gained the two steps the project already required in practice but never wrote down, STATUS.md and CHANGELOG, mirrored into the PR template. Also corrected where the same falsehood had propagated: `PROJECT_SETUP_PLAYBOOK` §B1/§B2, and `packages/adapters/README.md`, which was itself still calling the SDK "scaffolding", documenting `build_command` as the contract, omitting `ActiveApiAdapter` entirely, and pointing circularly back at the §4 it was supposed to be the authority for) · previously `fix/version-single-source` (issue #31 closed: Provx now declares **one version**, once per package, and CI refuses a disagreement — `/health` reported `0.0.0` against a `v0.1.1` tag because eight sites each declared a version and nothing held them equal. Source of truth is each package's own `__init__.py` via `dynamic = ["version"]`, because a root `VERSION` file and `git describe` are both unavailable where the packages are actually built — the image copies `packages/adapters` alone, with no `.git`. New required `version-consistency` gate plus a `release-check` tag assertion, `make version-set`, and `provx --version`. See the `## Versioning` section) · previously `docs/user-quickstart` (the first user-facing documentation: `docs/QUICKSTART.md`, written *after* executing every command it contains against a clean stack and a local OWASP Juice Shop target, and a README rewritten to lead with the identity and to state a status that no longer contradicts this file — it had been claiming Active mode and the API module were "absent by design" since before Phase 4 landed. Documentation is now tracked here, in the `## Documentation` section below, rather than being the one deliverable the plan-of-record never mentioned) · previously `ci/gate-workflow-edits` (issue #22 closed: CI configuration now gates changes to itself — a `.github/**` PR runs actionlint plus both CI self-tests, and the gate's own structure is asserted from `ci-required`, which a PR cannot switch off; `frontend-lint` became real, eslint + prettier, and joined the required set) · previously `ci/enforce-license-check` (issue #19 closed: the SPDX license-compatibility check named by PX-FREE and PX-LICENSE is a real, required CI gate rather than an `echo` stub — `pyphen`'s tri-license is now a pinned, machine-checked exception instead of a hand classification) · previously `docs/defer-mutating-tier` (ADR-001: the mutating-probe tier is **decided out**, not deferred — read-only active detection is the ceiling of the active tier, which completes Phase 4 rather than leaving it partial) · **Current phase:** Phase 4 — API module ✅ **complete** (Pass 1 passive ✅, Pass 2 active ✅, mutating tier decided out) alongside Phase 3 — Reporting (⏳ PDF ✅, dashboard list ✅, Word ⛔) · **Milestone:** v0.1.0 TAGGED (annotated tag at `9522dfa` / PR #12); the branded-PDF item in ROADMAP §11's v0.1 gate is now met, one tag late. --- @@ -11,6 +11,7 @@ | Phase | Scope | State | |---|---|---| | 0 — Foundations | Compose, Postgres, auth-less skeleton, scope engine, governance | ✅ done | +| **Platform security (ROADMAP §4)** | **RBAC, encrypted secrets, audit log** | ⏳ **partial** — identity + roles + hashed sessions + auth audit trail ✅ (`feat/auth-foundation`), secrets encrypted at rest ✅ (evidence + credentials), **RBAC enforced on 4 routes and no others** ⏳; the enforcement pass across the engagement routes is the next change | | 1 — Engagements & scope | Engagement CRUD, scope allow/deny, targets, walking skeleton | ✅ done | | **2 — Web module (MVP)** | **Passive adapters → findings pipeline → report** | **✅ done** — all 5 adapters run through `/scan`, findings dedup on the live path, report ships | | **3 — Reporting** | Branding, HTML→PDF/Word, dashboard | ⏳ **partial** (client-ready HTML ✅, branding ✅, **PDF ✅**, **dashboard engagement list + findings view ✅**; Word/`.docx` ⛔ deferred, dashboard depth beyond a list ⛔) | @@ -66,6 +67,25 @@ | **`GET /engagements` (list)** | ✅ | Branch `feat/list-engagements`: the engagements router gains its first collection read. New `EngagementSummaryRead` — `id`, `name`, `mode`, `target_count`, `created_at` and nothing more; `scope_allow` / `scope_deny` stay on the detail-shaped `EngagementRead`, because letting a caller pick an engagement out of a list does not require publishing what the operator is authorized to reach (PX-SECRETS). No credential existence, type, header name, or label. Proven by `test_list_engagements_exposes_no_credentials_or_scope`: a real credential is set, then the row's key set is pinned to exactly the five permitted names *and* the raw response text is checked to contain none of the token, the credential metadata field names, the scope field names, or the scope hostname. Ordered `created_at desc, id desc` so the output is reproducible even on a coarse clock (PX-DETERMINISM) — the collision test fails if the tiebreak is dropped. Target counts via one grouped query, not one per engagement. Auth-less like every sibling route (RBAC is still KI-003); **unpaginated by decision, not by omission**, and `GET /engagements/{id}` still deliberately absent. Closes KI-009. | +## Authentication + +*Added by `feat/auth-foundation`. The **foundation**, deliberately not the perimeter: this section says exactly which endpoints the gate is on and which it is not, because "auth landed" is the kind of claim that quietly becomes false in a reader's head.* + +| Item | State | Notes | +|---|---|---| +| **Operator identity + argon2id passwords** | ✅ | Branch `feat/auth-foundation`. New `auth_user` table — username, optional email, `password_hash`, role, `is_active`, `created_at` — behind `app/security/passwords.py` (argon2id via `argon2-cffi`, MIT, PX-FREE clean, at OWASP's 64 MiB / t=3 / p=4 baseline, pinned by test so weakening the cost is a deliberate edit). **Deliberately not built on `evidence_crypto.py` next door**: that layer is reversible AES-256-GCM so a scan credential can be replayed at scan time, and a password must never be — the two sit side by side so the distinction is visible rather than assumed, and `passwords.py` says so in its first line. `verify_password` collapses a mismatch, a malformed digest, and an unrunnable parameter set into the same `False`, so neither a caller nor a traceback learns which. Named `auth_user`, not `user`: `user` is reserved in PostgreSQL and would need quoting everywhere, including in a human's ad-hoc `psql` query. Password policy is a 12-character minimum and nothing more — the S-12 common-password denylist is a recorded deferral, not an oversight. | +| **A 422 no longer echoes what you sent** | ✅ | Same branch. One `RequestValidationError` handler on the app (`app/main.py`), not per-router, because every endpoint taking a password, credential, or token had the same exposure. `type`/`loc`/`msg` survive; `input`/`ctx`/`url` are dropped. FastAPI's `{"detail": [...]}` shape is kept deliberately rather than moved to `ErrorResponse` — the CLI branches on the envelope first, so changing shape would have discarded the per-field message it renders, and a fix that degrades the error UX invites being reverted. `test_api_validation_errors.py` sends bodies **containing** a planted secret across all five secret-bearing request models and asserts absence through the same detector the auth tests use, with a control proving that detector fires against the pre-fix shape. The malformed shapes also ride along in the no-leak walkthrough, so the file's existing absence assertions cover them. **This was a live disclosure, not a hardening pass** — see KI-010 for what leaked, for how long, and why the existing credential no-leak test did not catch it. | +| **The no-recoverable-secret guarantee** | ✅ | Same branch, and the actual deliverable. `UserRead` **has no `password_hash` field at all** — the digest is not omitted at serialization time, it has no field to travel through, so no handler edit can publish it (B-FA-07). Proven by `test_auth_no_leak.py`: a known password is planted, driven through bootstrap, login, me, user-create, list, a failed login, logout, and a post-logout call, then hunted for in every response body, **every response header** (the `Set-Cookie` path a body-only scan would miss), every API-returned row, the evidence store, and every captured log record. **Log records are searched through `record.__dict__`, not `getMessage()`** — which closes the gap the credential no-leak test documents: `extra={...}` is how this codebase attaches structured context and it does not render into the message, so a secret logged that way would have passed the old scan while sitting in the log. **The detector is itself under test.** Four control tests feed it planted leaks — in a body, in a header, in an `extra=` payload, and an empty needle — and assert it raises; a detector nobody has watched fail is an assertion that something is absent from a place nobody checked. Five mutations were also run by hand against **production** code, each reverted: adding `password_hash` to the real `UserRead` **failed** the leak tests; no-op'ing `require_role` **failed** six role tests; storing the raw token instead of its digest **failed** three token tests; and — closing the one gap a pre-merge audit found in this file — making the real login handler log the password via `extra=`, and making it echo the password in a response header, each **failed** `test_the_password_and_the_raw_token_reach_no_response_and_no_log_at_all`. That last pair is what moved the password needle from *asserted* to *demonstrated*; a permanent CI control now mounts a deliberately leaky handler behind the production 500 handler so it stays demonstrated. | +| **Roles + the capability table** | ✅ | Same branch. `Role` StrEnum (`admin`/`operator`/`viewer`) plus `ROLE_CAPABILITIES` in `app/api/deps.py` — **the whole authorization policy as one table of data**, not `if role ==` scattered through handlers, so the answer to "what may an operator do?" is a thing a reviewer reads rather than derives. admin: everything including `MANAGE_USERS`. operator: `READ`, `CREATE_ENGAGEMENT`, `RUN_SCAN`, and `APPROVE_ACTIVE_SCAN` — the human in PX-ACTIVE's *recorded authorization* is the tester doing the work. viewer: `READ`. Both `require_role(*roles)` and `require_capability(cap)` ship; the capability form is what the enforcement pass consumes, so that change adds arguments rather than logic. The table is asserted as data (ten parametrized cases) *and* through live routes, and the 403 is attributed: `test_the_403_is_produced_by_require_role_and_not_by_something_incidental` mounts the same handler behind a no-op dependency on a throwaway app and shows it answers 200. | +| **Hashed session tokens** | ✅ | Same branch. `secrets.token_urlsafe(32)` (rule A-04 — never `random`), stored as **SHA-256 only**, so a stolen `auth_token` row cannot be replayed as a bearer token; asserted directly, including a test that presents the stored digest and gets 401. **Why SHA-256 here and argon2id for passwords:** a password is low-entropy and needs a slow KDF to price an offline dictionary attack; a token is 256 uniformly random bits with no dictionary, and every authenticated request has to resolve one — a 64 MiB KDF would make each API call cost a login. Lookup is an exact index match on the digest, so no stored secret is ever compared and there is no timing channel to defend. **Opaque rows rather than JWTs**, decided: revocation has to be immediate for a security product, and a self-contained token cannot be revoked without the lookup a stateful token already does. Twelve-hour TTL from `AUTH_TOKEN_TTL_HOURS`; logout revokes **only the presenting token**, so ending a CLI session does not sign the same operator out of the console. | +| **Cookie *and* bearer, one secret** | ✅ | Same branch. One login issues one token, delivered two ways: in the response body for the CLI and API, and as an `httpOnly` / `SameSite=Lax` / `Secure` cookie for the console (rule A-03). `Secure` is off only in a debug `APP_ENV`, and `is_debug_env` fails closed, so production gets the safe setting by default. The header wins when both are present — a CLI run must use the token it was given, not whichever session a browser jar holds. The console consumes none of this yet (issue #29); the machinery ships complete and tested rather than half-built, so the console work is wiring rather than design. | +| **One-shot admin bootstrap** | ✅ | Same branch. `POST /auth/bootstrap` creates the first admin with no credentials — the only unauthenticated way to make a user — and closes **permanently**. The guard is a one-row `auth_bootstrap` table with a **fixed primary key**, not a `COUNT(admins) == 0` check, and that choice buys two properties a count cannot: two concurrent first-run requests cannot both win (the loser takes a primary-key violation, on SQLite and PostgreSQL alike, with no dialect-specific locking), and **deleting the admin does not re-open the door** — asserted by a test that wipes the user row and still gets `bootstrap_closed`. A rejected attempt (weak password) does **not** burn the one shot. Every attempt, successful or not, writes an `auth_event` row. | +| **Auth audit trail** | ✅ | Same branch. `auth_event` is append-only (PX-SECRETS, PX-EVIDENCE, D-05): bootstrap, login **success and failure**, logout, and user creation each write exactly one row, and there is no update or delete path. A security platform that cannot say when someone tried and failed to get in is missing the entry that matters most. No password, digest, or token value is recorded — `username_attempted` is an identifier, and `detail` is drawn from a closed set of fixed reason strings in the service layer, never from caller input. Asserted by a test that scans the whole trail for the planted password, the raw token, and the token's digest. | +| **`provx login` / `logout` / `admin create`** | ✅ | Same branch. New `provx_cli/store.py` — **stdlib only**, so `test_cli_no_bypass.py`'s assertion that `httpx` lives in exactly two modules still holds. Tokens land in `$XDG_CONFIG_HOME/provx/credentials.json` keyed by server URL (a lab instance and a client instance are two sessions, not one that overwrites), the file created `0600` **before** it is written rather than chmod'd after — a chmod that follows the write leaves a window where the token is world-readable on disk. `api.py:_headers()` stays the single place a token becomes a header; precedence is `$PROVX_TOKEN` then the stored token, so the pre-existing environment path keeps working and keeps winning, for CI. **No password flag exists on any command**, and the guard was widened to match: `test_cli_secrets.py` now walks the **entire** subparser tree rather than just `credential set`, with a control asserting the walk actually reaches `login` and `admin create` — otherwise it would have passed vacuously. `read_secret` gained a prompt argument rather than being copied (Q-11). `--json` login output **omits the token**: that stream is routinely redirected into a file, and a token there outlives the shell that made it. Logout clears the local token even when the server refuses. Proven unstubbed end to end in `test_integration_cli.py`, against the real app on a real database. | +| **Enforced endpoints (this change)** | ✅ | `GET /auth/me` and `POST /auth/logout` — any authenticated operator. `POST /users` and `GET /users` — **admin only**, via a router-level dependency so a route added to that file is gated by construction rather than by remembering (B-FA-03). An anonymous request to any of them is 401; a viewer or an operator on `/users` is 403. | +| **NOT enforced yet (deliberate)** | ⛔ | **Every engagement route**, listed rather than gestured at: `POST /engagements`, `GET /engagements`, `POST /{id}/scan`, `POST /{id}/credential`, `GET /{id}/credentials`, `GET /{id}/credential`, `DELETE /{id}/credential`, `POST /{id}/mode`, `POST /{id}/active-approvals`, `GET /{id}/active-approvals`, `POST /{id}/active-approvals/{approval_id}/decide`, `GET /{id}/findings`, `POST /{id}/findings/{finding_id}/transition`, `POST /{id}/findings/{finding_id}/in-report`, `GET /{id}/report`. Plus `GET /health` and `GET /` (meta, intentionally open). `POST /auth/login` and `POST /auth/bootstrap` are open by necessity — a caller with no credential must be able to obtain one. **`app/api/engagements.py` was not touched except for its docstring, which now says this in the file itself.** Every pre-existing API test passes **unmodified**, which is the evidence that the surface genuinely did not move. **Land with:** the next PR, which applies `require_capability` across these routes, binds the three free-text actor fields (`FindingEventRow.actor`, `ActiveScanApproval.authorized_by` / `decided_by`, `ApprovalEventRow.actor`) to a real principal, and must address KI-003 — see the issues table. | +| Failed-login lockout (A-06), refresh rotation (A-05), password denylist (S-12), self-service password change, deactivation endpoint, OIDC/SSO, multi-tenant | ⛔ | Recorded, not forgotten — the full list with reasoning is in [KNOWN_ISSUES.md](KNOWN_ISSUES.md) under *Authentication foundation — deliberate limits*. SSO and multi-tenant are ROADMAP §1 paid-edition features and do not belong in this repository at all. | + ## Documentation *Tracked here from `docs/user-quickstart` onward. Until then documentation was the one deliverable this file never mentioned, which is how the README came to contradict it.* @@ -77,7 +97,7 @@ | **API-authentication warning made prominent** | ✅ | Same branch. The API and console have no authentication and the README's old quickstart never said so at the point the reader binds ports. Now stated in the README status block and again in QUICKSTART §2 as an `IMPORTANT` callout next to the URLs. Not a new defect — a documented one, finally documented where it is load-bearing. | | CONTRIBUTING §4 adapter cookbook is stale | ✅ | **RESOLVED** by `docs/contributing-accuracy`, closing issue #30. §4 walked `build_command` / `parse_output` for a wrapped external binary and opened with "copy the template adapter" — a template that does not exist and never did. Rewritten to the shipped contract: pick one of **three** protocols by unit of work, declare the manifest (noting the gate treats anything not exactly `"passive"` as intrusive, so the classification fails closed), implement `probe`/`probe_endpoint` through `fetch_within_scope` and nothing else (PX-EGRESS), implement a pure `parse_output` returning `FindingDraft`, register one entry-point line, ship a fixture pair, ship a lab positive/clean pair. `build_command` is documented as what it actually is — the subprocess path for copyleft tools (PX-LICENSE), with exactly one of the two live per adapter — rather than deleted. **Two things the old recipe never said** and a contributor could not infer: `Evidence.matched_rule` gets its own step because the accuracy harness keys on it and raises without it, and the lab manifest's `adapter:` key **silently defaults to `security_headers`** if omitted. The scoring wiring is written down as the sharp edge it is: compose services + `depends_on`, the Makefile `up -d` list, and a `--adapter` run line are three hand-maintained lists nothing derives from the entry points, so an adapter can ship, pass its unit tests, and never be scored. Honest about enforcement, too: PX-ATTACK's technique ID is format-validated but its *presence* is not, and `cvss` is optional on the model — so §4 says that is on the reviewer instead of implying CI catches it. **This did not wait for the next adapter PR** — the deferral's stated condition — because the same PR was correcting §1/§3 and leaving the neighbouring section lying would have been a choice, not an omission. | | `/health` reports `"version":"0.0.0"` | ✅ | **RESOLVED** by `fix/version-single-source` (issue #31). Surfaced by the first-run verification. The endpoint was never the bug — it reads `app.__version__` correctly; the bug was that **eight** sites each declared a version and nothing held them equal, so a bump was a manual, silent, partial operation. See the `## Versioning` section below. | -| Console cannot create an engagement | ⛔ | Not a regression — the console has always been read-only, and its own empty state says to run `provx engagement create`. Recorded because it is the sharpest first-run edge: a user who reaches the UI first finds no way to start work, and the quickstart therefore has to route creation through the CLI or API and say why. **Land with:** Phase 3 dashboard depth, or whenever a write path in the UI is decided on its own terms (it needs the auth that does not exist yet — a create form on an unauthenticated API is a worse gap than no form). | +| Console cannot create an engagement | ⛔ | Not a regression — the console has always been read-only, and its own empty state says to run `provx engagement create`. Recorded because it is the sharpest first-run edge: a user who reaches the UI first finds no way to start work, and the quickstart therefore has to route creation through the CLI or API and say why. **Land with:** Phase 3 dashboard depth, or whenever a write path in the UI is decided on its own terms (it needs auth, which now **exists** as of `feat/auth-foundation` but is not yet enforced on `POST /engagements`; a create form still waits on the enforcement pass, because a form on a route anyone can call is the same gap wearing a login page). | | **CONTRIBUTING describes the real process** | ✅ | Branch `docs/contributing-accuracy`, closing **#34** and **#30** — see the two resolved rows above (this section's §4 row, and §1/§3 in `## Versioning`) for what changed and why. Method worth recording: every command and path in the new text was **executed or read out of the tree before being written**, not paraphrased from the docs it replaced — `make version-set` with no `VERSION` to confirm the usage string, the consistency script against the clean tree, and `importlib.metadata.entry_points` over all three groups to confirm the group names and that nine adapters resolve. That is the same discipline `docs/user-quickstart` used, and it is what separates this pass from the one that produced the claims being removed. | | Remaining known doc drift (not fixed here) | ⛔ | Three, all recorded rather than quietly left: (1) `packages/adapters/README.md`'s **Layout** tree lists four modules and omits `findings.py`, `registry.py`, `fetch.py`, `scope.py`, `auth.py`, `active.py`, `evidence.py`, and the whole `adapters/` subpackage — the contract sections were corrected here, the tree was not, because it is a separate mechanical pass. (2) `lab/README.md` still says "live for all six adapters / twelve nginx targets" against nine and fourteen, and `lab/expected.yml`'s index omits the three active pairs the harness actually globs. (3) `PROJECT_SETUP_PLAYBOOK` §"Release cadence" still describes automated daily dependency-bot commits and a bi-monthly release train, none of which runs — left because it reads as a *plan* rather than a claim about shipped machinery, unlike §B1/§B2 which asserted tooling and were corrected. **Land with:** the next PR that touches each file for its own reasons. | | No user-facing reference docs beyond the quickstart | ⛔ | No CLI reference, no configuration reference, no adapter catalogue. `--help`, `.env.example`, and `/docs` carry it for now. Deliberate: a reference that drifts is worse than a pointer to `--help`. **Land with:** demand, or the first release where a flag exists that `--help` cannot explain. | @@ -99,7 +119,7 @@ |---|---|---|---| | ADR-001 | mutating write/delete probes (proof-by-exploitation in the scan path) | nothing — **not an open item**. Listed here because it used to be tracked as one | ❌ **never, by decision.** Read-only active detection is the ceiling of the active tier; reversal needs a superseding ADR meeting four conditions (separate off-by-default mode, test-environments-only, per-object approval, reversible). See [ADR-001](decisions/ADR-001-no-mutating-probes.md) | | KI-002 | display_id race (fails safe via unique constraint) | nothing | when convenient | -| KI-003 | dangerous-range check = IP literals only; DNS-rebinding needs pinned-resolution transport | user-supplied scope | still open — auth landed **without** API RBAC / user-supplied scope, so its urgency trigger isn't reached yet | +| KI-003 | dangerous-range check = IP literals only; DNS-rebinding needs pinned-resolution transport | user-supplied scope | still open, and **one change from urgent**. `feat/auth-foundation` built the identity machinery but left every engagement route open, so there is still no non-admin operator who can put a hostname into scope. The enforcement PR creates exactly that, and must carry the pinned-resolution transport or state in writing why not | | SDK-004 | Evidence inline-seal design (envelope vs inline field) | nothing | adapter #6 / auth | | KI-004 residuals | request-side `Authorization`/`Cookie` + custom-header ✅ covered by boundary redaction; body-content ✅ best-effort (`redact_body`); URL-userinfo + KMS key still open | real credentials | ⏳ partly closed by `feat/authenticated-scanning` | | KI-006 | form-login/SSO/MFA/CSRF/session-record deferred; explicit creds only in v0.2 | nothing | when a real form-login need appears | @@ -139,6 +159,8 @@ - **Frontend:** ✅ **a runner now exists** — Vitest + Testing Library + jsdom in `frontend/`, `npm test` (`NODE_ENV=testing`), run by the `frontend-types` CI job and `make test`. 11 tests across two files. `dashboard.test.tsx`: the engagement list (rows + per-row links from a mocked client), the empty state, the HTML/PDF report links, the severity/lifecycle/PX-HUMAN surface, and `reportUrl` encoding — async Server Components are awaited as functions and their output rendered, so no Next server is started. `report-route.test.ts`: the proxy handler called directly with `fetch` stubbed — a malformed id and an unsupported format each return 404 **with `fetch` never called** (the guards run before egress, not merely before the response — S-05, PX-EGRESS), the default asks upstream for `format=html`, a PDF relays byte-identical through `arrayBuffer` with `no-store` (asserted with a non-UTF-8 fixture, so a regression to `.text()` fails), upstream 404 → 404, and any other upstream error → 502 carrying none of the upstream body (S-13, W-NEXT-09). Both guard tests were mutation-checked by neutering the guards. **Still uncovered:** `error.tsx` / `loading.tsx`. **Now blocking:** `frontend-types` is in `ci-required`'s `needs` (branch `ci/require-frontend-checks`), so these tests gate the merge on any PR that touches `frontend/` or `packages/client/`, and skip harmlessly on any PR that does not. - **CI gate logic:** ✅ `.github/scripts/assert_gate_results.test.sh` — an 11-vector truth table over `assert_gate_results.sh`, the script that decides whether the single required check passes. Drives the real script rather than re-implementing its loop, since a copied loop would prove nothing about the code CI runs. Covers both directions of the case that matters (`frontend-types` `skipped` → pass, `failure` → block), the paths-filter fail-open hole, `cancelled`, and the fail-closed cases (unknown token, empty input). Run by `make test` and by `ci-required` itself before it asserts anything real. - **Integration (no-stub), API module:** ✅ `test_integration_api_module.py` — a real loopback server publishing a real OpenAPI document and real endpoints, driving the shipping app and a real database. Covers the happy path (spec URL and inline spec both produce API findings through `/scan`, tagged `module=api`, sealed, with `PVX-` ids), the pipeline claims (a repeat scan corroborates rather than duplicates — `evidence_ref_count >= 2`; the lifecycle gate still refuses `new → validated` for an API finding, PX-HUMAN; findings reach the HTML report; a web and an API HSTS finding on one host stay distinct because their locations differ), and — the reason the file exists — the safety claims **asserted against the requests the server actually received**: a spec naming an off-scope host produces no findings and contacts nothing at all, an off-scope spec URL is refused without reaching it and surfaces no traceback or hostname, no writing method is ever sent, a declared TRACE is flagged without TRACE being sent, and `/users/{id}` is never guessed at. Plus the engagement surface: both spec sources → 422, a pasted spec is never echoed back, and the engagement list publishes no spec material. +- **Authentication:** ✅ `test_auth_passwords.py` / `test_auth_tokens.py` (the primitives: one-wayness, a fresh salt per hash, the pinned argon2id cost, a malformed digest answering `False` rather than raising, token entropy and URL-safety, `stored != issued`), `test_api_auth.py` (login, the httpOnly/SameSite cookie, cookie-authenticates, header-beats-cookie, the stored digest refused as a bearer token, logout revoking only the presenting session, expiry, the audit trail, and a wrong password and an unknown username answering identically so login is not an enumeration oracle), `test_auth_roles.py` (401 anonymous, 403 for operator and viewer, 200 for admin, the capability table as data, a deactivated operator refused mid-session), `test_auth_bootstrap.py` (one shot, refused twice, refused after the admin is deleted, and a rejected attempt not burning the shot), and `test_auth_no_leak.py` (the deliverable). **What makes these more than green ticks:** the no-leak detector is itself under test against planted leaks in a body, a header, and an `extra=` payload; the 403 is attributed by mounting the same handler behind a no-op dependency; and three by-hand mutations — leaking the hash through `UserRead`, no-op'ing `require_role`, storing the raw token — were each run and each made the suite fail. Migration up **and** down plus a model-vs-migration column comparison for all four tables (W-03). +- **Authentication, CLI:** ✅ `test_cli_auth.py` — the token store round-trips, the file is `0600` and its directory `0700`, the password never reaches argv or stdout, `--json` output omits the token, later commands present the stored token, `$PROVX_TOKEN` still wins, tokens are per-server, logout forgets the token even when the server refuses, and a corrupt store reads as no session. `test_cli_secrets.py`'s parser introspection now walks the **whole** command tree, with a control asserting the walk reaches `login` and `admin create`. End-to-end unstubbed in `test_integration_cli.py`: `admin create` → `login` → an authenticated call → a refused second bootstrap → `logout` invalidating the token at the API. - **Oracle/benchmark (OWASP Benchmark + ZAP/Nuclei diff):** ⛔ v0.5 *Phase 4 Pass 1 obeys the layer rule rather than breaching it: the API module is a new source of findings, but every pipeline its output flows into — dedup, lifecycle, report — was already finished in Phase 2, and none of them changed to accept it. What Pass 1 adds is a second producer for a completed pipeline, not a capability whose output lands somewhere unbuilt. Pass 2 obeys the same rule: it does add a new capability (crafted reads), but only behind machinery this pass builds first — Active-mode enablement and the per-run `ActiveScanApproval` queue — and its findings still flow into the same unchanged dedup/lifecycle/report pipeline.* diff --git a/packages/cli/README.md b/packages/cli/README.md index eb761e8..59ef26d 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -30,6 +30,9 @@ The only runtime dependency is `httpx` (rule PX-FREE). Everything else is the st ## Commands ```bash +provx admin create --username root [--email you@example.com] [--password-env VAR] +provx login --username root [--password-env VAR] +provx logout provx engagement create --name "Acme" --scope acme.com --target https://acme.com [--deny x.acme.com] [--mode passive] provx engagement list provx credential set --type bearer|cookie|header [--header-name X-API-Key] [--label staging] [--from-env VAR] @@ -70,9 +73,28 @@ provx credential set --type header --header-name X-API-Key --from-env PROVX On an interactive terminal the CLI prompts through `getpass`, so the value is not echoed. The API encrypts it at rest and never returns it, so nothing here can print it back. -A bearer token for a Provx server that requires one is read from `$PROVX_TOKEN` for the same -reason. The API ships without authentication today, so the header is sent only when that -variable is set. +**Your own password is handled exactly the same way.** `provx login` and `provx admin create` +have no `--password` flag either; the value comes from a hidden `getpass` prompt, from stdin, or +from `--password-env VAR`. A test walks the whole command tree and fails if any command anywhere +grows a flag that could carry a secret. + +`login` stores the issued token in `$XDG_CONFIG_HOME/provx/credentials.json` (else +`~/.config/provx/`), keyed by server URL, created `0600` in a `0700` directory. `logout` revokes +it server-side and forgets it locally — including when the server is unreachable, because a +credential on disk that this machine cannot use is a credential that should not be on disk. +`$PROVX_TOKEN` still works and **takes precedence** over the stored session, so a CI job can +authenticate without touching a config file. `--json` login output deliberately omits the token: +that stream is routinely redirected into a file, and a token there outlives the shell that made +it. + +`provx admin create` is a first-run step, not an administrator factory: the server refuses it +permanently once any bootstrap has completed, and deleting the account it made does not re-open +it. + +**Not every endpoint requires a token yet.** `/auth/me`, `/auth/logout`, and `/users` are +authenticated; the engagement, scan, findings, and report routes are still open. The CLI presents +the token on every request regardless, so nothing here changes when enforcement lands. See +[`../../docs/STATUS.md`](../../docs/STATUS.md) for the exact split. ## Exit codes diff --git a/packages/cli/src/provx_cli/api.py b/packages/cli/src/provx_cli/api.py index 61876de..3f26218 100644 --- a/packages/cli/src/provx_cli/api.py +++ b/packages/cli/src/provx_cli/api.py @@ -29,13 +29,15 @@ import httpx +from provx_cli import store + #: Where the CLI looks for the server when ``--server`` is not given. SERVER_ENV = "PROVX_SERVER" DEFAULT_SERVER = "http://localhost:8000" #: Bearer token for a server that requires one. Environment only, never a flag: a flag value -#: lands in shell history and in the process list (rule PX-SECRETS). The API ships without -#: auth today, so this is sent only when the operator sets it. +#: lands in shell history and in the process list (rule PX-SECRETS). Takes precedence over the +#: token ``provx login`` stores, so a CI job can authenticate without touching a config file. TOKEN_ENV = "PROVX_TOKEN" DEFAULT_TIMEOUT = 60.0 @@ -178,8 +180,17 @@ def close(self) -> None: self._http.close() def _headers(self) -> dict[str, str]: - """Build the request headers, adding a bearer token only when one is configured.""" - token = os.environ.get(TOKEN_ENV) + """Build the request headers, adding a bearer token only when one is available. + + Precedence is ``$PROVX_TOKEN`` first, then whatever ``provx login`` stored for this + server. The environment wins so a CI job or a one-off shell can override the logged-in + session without disturbing it, which is the same reason the variable existed before + there was a login command at all. + + Still the single place a token becomes a header (rule PX-SECRETS): a reviewer who reads + this method has read every way the CLI authenticates. + """ + token = os.environ.get(TOKEN_ENV) or store.load_token(self.base_url) return {"Authorization": f"Bearer {token}"} if token else {} def _request(self, method: str, path: str, *, json: dict[str, Any] | None = None) -> Any: @@ -370,3 +381,52 @@ def get_report(self, engagement_id: str, *, report_format: str = "html") -> byte params={"format": report_format}, ) return response.content + + def login(self, *, username: str, password: str) -> dict[str, Any]: + """Exchange a username and password for a session token. Wraps ``POST /auth/login``. + + The password is sent in the request body and is never placed in a URL, a query + parameter, or a log line (rule PX-SECRETS). + + Args: + username: The operator's username. + password: The operator's password. + + Returns: + The token, its expiry, and the operator it belongs to. + """ + result: dict[str, Any] = self._request( + "POST", "/auth/login", json={"username": username, "password": password} + ) + return result + + def logout(self) -> None: + """Revoke the presenting token server-side. Wraps ``POST /auth/logout``.""" + self._request("POST", "/auth/logout") + + def whoami(self) -> dict[str, Any]: + """Return the authenticated operator. Wraps ``GET /auth/me``.""" + result: dict[str, Any] = self._request("GET", "/auth/me") + return result + + def bootstrap_admin( + self, *, username: str, password: str, email: str | None = None + ) -> dict[str, Any]: + """Create the first admin. Wraps ``POST /auth/bootstrap``. + + The server refuses once any bootstrap has completed, so this is a one-shot call and + not a way to mint administrators. + + Args: + username: The admin's username. + password: The admin's password. + email: Optional contact address. + + Returns: + The created admin. + """ + body: dict[str, Any] = {"username": username, "password": password} + if email: + body["email"] = email + result: dict[str, Any] = self._request("POST", "/auth/bootstrap", json=body) + return result diff --git a/packages/cli/src/provx_cli/commands.py b/packages/cli/src/provx_cli/commands.py index 3903d2b..c4a0b95 100644 --- a/packages/cli/src/provx_cli/commands.py +++ b/packages/cli/src/provx_cli/commands.py @@ -18,16 +18,19 @@ from pathlib import Path from typing import Any -from provx_cli import exit_codes +from provx_cli import exit_codes, store from provx_cli.api import ProvxApiClient from provx_cli.output import emit_json, render_table, write_err, write_out #: The credential types the API accepts (its own regex is the authority; this only shapes help). CREDENTIAL_TYPES = ("bearer", "cookie", "header") -#: Prompt shown when a secret is read from an interactive terminal. +#: Prompt shown when a scanning credential is read from an interactive terminal. SECRET_PROMPT = "Credential value (input hidden): " +#: Prompt shown when an operator's own password is read from an interactive terminal. +PASSWORD_PROMPT = "Password (input hidden): " + #: The scan status the API reports when at least one adapter failed. COMPLETED_WITH_ERRORS = "completed_with_errors" @@ -40,15 +43,26 @@ class CredentialInputError(Exception): """Raised when no credential value could be read from stdin or the environment.""" -def read_secret(from_env: str | None) -> str: - """Read a credential value without it ever appearing on the command line. +def read_secret( + from_env: str | None, + *, + prompt: str = SECRET_PROMPT, + missing: str = "No credential value supplied. Pipe it on stdin or use --from-env VAR.", +) -> str: + """Read a secret without it ever appearing on the command line. + + A ``--value`` or ``--password`` flag would place the secret in shell history and in the + process list, where any local user can read it, so no command has one (rule PX-SECRETS). + The secret arrives either from a named environment variable or over stdin. - A ``--value`` flag would place the secret in shell history and in the process list, where - any local user can read it, so this command does not have one (rule PX-SECRETS). The - secret arrives either from a named environment variable or over stdin. + One function for scanning credentials and operator passwords alike (rule Q-11): they are + different secrets with identical handling requirements, and a second copy of this would be + a second place for that handling to drift. Args: from_env: Name of the environment variable holding the secret, or None to use stdin. + prompt: What to show on an interactive terminal. + missing: The error message when nothing was supplied. Returns: The secret, stripped of surrounding whitespace. @@ -62,15 +76,22 @@ def read_secret(from_env: str | None) -> str: raise CredentialInputError(f"Environment variable {from_env} is not set or is empty.") return value - raw = getpass.getpass(SECRET_PROMPT) if sys.stdin.isatty() else sys.stdin.read() + raw = getpass.getpass(prompt) if sys.stdin.isatty() else sys.stdin.read() value = raw.strip() if not value: - raise CredentialInputError( - "No credential value supplied. Pipe it on stdin or use --from-env VAR." - ) + raise CredentialInputError(missing) return value +def read_password(from_env: str | None) -> str: + """Read an operator's own password, by the same never-on-the-command-line rule.""" + return read_secret( + from_env, + prompt=PASSWORD_PROMPT, + missing="No password supplied. Pipe it on stdin or use --password-env VAR.", + ) + + def engagement_create(client: ProvxApiClient, args: argparse.Namespace) -> int: """Create an engagement and print its identifier. @@ -329,3 +350,104 @@ def _filter_findings( if status is not None: selected = [f for f in selected if str(f.get("status", "")).lower() == status.lower()] return selected + + +def login(client: ProvxApiClient, args: argparse.Namespace) -> int: + """Log in and store the issued token for later commands. + + The password is read from a terminal prompt, stdin, or a named environment variable - + never a flag (rule PX-SECRETS). Neither the password nor the token is printed. + + Args: + client: The API client. + args: Parsed arguments carrying the username, the password source, and the json flag. + + Returns: + An exit code. + """ + password = read_password(args.password_env) + session = client.login(username=args.username, password=password) + path = store.save_token( + client.base_url, + str(session["token"]), + username=str(session["user"]["username"]), + expires_at=str(session.get("expires_at") or "") or None, + ) + + if args.json: + # The token is deliberately not in this payload: --json output is routinely piped into + # a file or a log, and a token that lands there outlives the shell that made it. + emit_json( + { + "username": session["user"]["username"], + "role": session["user"]["role"], + "expires_at": session.get("expires_at"), + "stored_at": str(path), + } + ) + return exit_codes.OK + + write_out( + f"Logged in to {client.base_url} as {session['user']['username']} " + f"({session['user']['role']}). Session stored in {path}." + ) + return exit_codes.OK + + +def logout(client: ProvxApiClient, args: argparse.Namespace) -> int: + """Revoke the current session server-side and forget the stored token. + + The local token is cleared even when the server call fails: a token this machine can no + longer use is better left unusable here too, and an unreachable server is not a reason to + keep a credential on disk. + + Args: + client: The API client. + args: Parsed arguments carrying the json flag. + + Returns: + An exit code. + """ + username = store.stored_username(client.base_url) + try: + client.logout() + finally: + cleared = store.clear_token(client.base_url) + + if args.json: + emit_json({"server": client.base_url, "username": username, "cleared": cleared}) + return exit_codes.OK + + if not cleared and username is None: + write_out(f"No stored session for {client.base_url}.") + return exit_codes.OK + write_out(f"Logged out of {client.base_url}.") + return exit_codes.OK + + +def admin_create(client: ProvxApiClient, args: argparse.Namespace) -> int: + """Create the first administrator on a server that has none. + + Works exactly once per deployment: the server refuses afterwards, permanently. This is a + first-run step, not a way to mint administrators. + + Args: + client: The API client. + args: Parsed arguments carrying the username, optional email, the password source, + and the json flag. + + Returns: + An exit code. + """ + password = read_password(args.password_env) + admin = client.bootstrap_admin(username=args.username, password=password, email=args.email) + + if args.json: + emit_json(admin) + return exit_codes.OK + + write_out( + f"Created administrator {admin['username']} on {client.base_url}. " + f"Run 'provx login --username {admin['username']}' to sign in." + ) + return exit_codes.OK diff --git a/packages/cli/src/provx_cli/main.py b/packages/cli/src/provx_cli/main.py index 1d8f31b..549655f 100644 --- a/packages/cli/src/provx_cli/main.py +++ b/packages/cli/src/provx_cli/main.py @@ -90,6 +90,8 @@ def build_parser() -> argparse.ArgumentParser: ) subcommands = parser.add_subparsers(dest="command", required=True) + _add_auth(subcommands) + _add_admin(subcommands) _add_engagement(subcommands) _add_credential(subcommands) _add_scan(subcommands) @@ -99,6 +101,54 @@ def build_parser() -> argparse.ArgumentParser: return parser +def _add_auth(subcommands: SubCommands) -> None: + """Register ``login`` and ``logout``. + + As with ``credential set``, there is no password flag and there never will be: a secret on + the command line lands in shell history and the process list (rule PX-SECRETS). The + password comes from a hidden prompt, stdin, or a named environment variable. + """ + login = subcommands.add_parser( + "login", + help="Log in and store a session token. The password is never a flag.", + ) + login.add_argument("--username", required=True, help="Your operator username.") + login.add_argument( + "--password-env", + default=None, + metavar="VAR", + help="Read the password from this environment variable instead of prompting.", + ) + login.set_defaults(handler=commands.login) + + logout = subcommands.add_parser("logout", help="Revoke the session and forget the token.") + logout.set_defaults(handler=commands.logout) + + +def _add_admin(subcommands: SubCommands) -> None: + """Register the ``admin`` command group - first-run bootstrap only. + + ``admin create`` exists so a fresh deployment has a way in. The server refuses it once any + bootstrap has completed, permanently, so this is not an administrator factory. + """ + admin = subcommands.add_parser("admin", help="First-run administrator setup.") + actions = admin.add_subparsers(dest="action", required=True) + + create = actions.add_parser( + "create", + help="Create the first administrator. Refused once one exists.", + ) + create.add_argument("--username", required=True, help="Username for the administrator.") + create.add_argument("--email", default=None, help="Optional contact address.") + create.add_argument( + "--password-env", + default=None, + metavar="VAR", + help="Read the password from this environment variable instead of prompting.", + ) + create.set_defaults(handler=commands.admin_create) + + def _add_engagement(subcommands: SubCommands) -> None: """Register the ``engagement`` command group.""" engagement = subcommands.add_parser("engagement", help="Manage engagements.") diff --git a/packages/cli/src/provx_cli/store.py b/packages/cli/src/provx_cli/store.py new file mode 100644 index 0000000..847ec2c --- /dev/null +++ b/packages/cli/src/provx_cli/store.py @@ -0,0 +1,109 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +Where ``provx login`` keeps the token it was issued (rule PX-SECRETS). + +The file lives under the user's config directory with owner-only permissions, and the +directory is created owner-only too - a token written into a world-readable path is a token +every local account holds. Tokens are keyed by server URL, so an operator working against a +lab instance and a client instance does not have to keep logging back in. + +Deliberately stdlib-only. ``test_cli_no_bypass.py`` asserts that ``httpx`` is imported by the +API boundary and the entry point and nowhere else, and that assertion is worth more than the +convenience of reaching for a library here. +""" + +from __future__ import annotations + +import json +import os +import stat +from pathlib import Path +from typing import Any + +_DIR_MODE = stat.S_IRWXU +_FILE_MODE = stat.S_IRUSR | stat.S_IWUSR + + +def credentials_path() -> Path: + """Where the token file lives, honouring ``XDG_CONFIG_HOME`` when it is set.""" + base = os.environ.get("XDG_CONFIG_HOME", "").strip() + root = Path(base) if base else Path.home() / ".config" + return root / "provx" / "credentials.json" + + +def _read_all() -> dict[str, Any]: + """Every stored entry, or an empty mapping when the file is missing or unreadable. + + A corrupt file reads as empty rather than raising: the recovery from a mangled token cache + is to log in again, which is exactly what an empty cache produces. + """ + path = credentials_path() + try: + loaded = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return {} + return loaded if isinstance(loaded, dict) else {} + + +def _write_all(entries: dict[str, Any]) -> None: + """Persist the whole store, owner-readable only.""" + path = credentials_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.parent.chmod(_DIR_MODE) + # Create with the right mode before writing, rather than after: a chmod that follows the + # write leaves a window where the token is world-readable on disk. + descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, _FILE_MODE) + with os.fdopen(descriptor, "w", encoding="utf-8") as handle: + json.dump(entries, handle, indent=2, sort_keys=True) + path.chmod(_FILE_MODE) + + +def load_token(server: str) -> str | None: + """The stored token for a server, or None when there is none.""" + entry = _read_all().get(server) + if not isinstance(entry, dict): + return None + token = entry.get("token") + return token if isinstance(token, str) and token else None + + +def save_token(server: str, token: str, *, username: str, expires_at: str | None = None) -> Path: + """Store a token for a server, replacing any previous one. + + Args: + server: The server base URL the token authenticates against. + token: The raw token, as issued by ``POST /auth/login``. + username: Who the token belongs to, so ``provx logout`` can say whose session it ended. + expires_at: The API's stated expiry, kept for display only - the server decides. + + Returns: + The path written, so a caller can tell the operator where it went. + """ + entries = _read_all() + entries[server] = {"token": token, "username": username, "expires_at": expires_at} + _write_all(entries) + return credentials_path() + + +def clear_token(server: str) -> bool: + """Forget the stored token for a server. + + Returns: + Whether there was one to forget. + """ + entries = _read_all() + if server not in entries: + return False + del entries[server] + _write_all(entries) + return True + + +def stored_username(server: str) -> str | None: + """Who the stored token belongs to, for a human-readable status line.""" + entry = _read_all().get(server) + if not isinstance(entry, dict): + return None + username = entry.get("username") + return username if isinstance(username, str) else None diff --git a/packages/cli/tests/test_cli_auth.py b/packages/cli/tests/test_cli_auth.py new file mode 100644 index 0000000..c5724bb --- /dev/null +++ b/packages/cli/tests/test_cli_auth.py @@ -0,0 +1,298 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Solomon Nii Amu Darku +""" +``provx login`` / ``logout`` / ``admin create``, and the token store behind them. + +The properties that matter: the password never reaches argv or the terminal, the token lands +on disk owner-readable only, later commands present it, ``$PROVX_TOKEN`` still wins, and +logout forgets it even when the server is unreachable. + +Every test redirects ``XDG_CONFIG_HOME`` at a tmp_path, so a run never touches the developer's +real ``~/.config/provx``. +""" + +from __future__ import annotations + +import io +import json +import stat +from pathlib import Path + +import httpx +import pytest +from provx_cli import exit_codes, store +from provx_cli.main import run + +PASSWORD = "an-operator-password-nobody-should-see" +TOKEN = "issued-token-value-abc123" + +USER_BODY = { + "id": "aaaaaaaa-0000-0000-0000-000000000001", + "username": "root", + "email": None, + "role": "admin", + "is_active": True, + "created_at": "2026-07-30T10:00:00Z", +} +LOGIN_BODY = { + "token": TOKEN, + "token_type": "bearer", + "expires_at": "2026-07-30T22:00:00Z", + "user": USER_BODY, +} +SERVER = "http://provx.test" + + +@pytest.fixture(autouse=True) +def isolated_config(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Point the token store at a throwaway directory and clear any inherited token.""" + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "config")) + monkeypatch.delenv("PROVX_TOKEN", raising=False) + return tmp_path + + +def _recording_stub( + status_code: int = 200, payload: object = LOGIN_BODY +) -> tuple[httpx.Client, list[httpx.Request]]: + """A stub transport that answers with one body and records every request it saw.""" + seen: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request) + if status_code == 204: + return httpx.Response(204) + return httpx.Response(status_code, json=payload) + + return httpx.Client(transport=httpx.MockTransport(handler)), seen + + +def _login(monkeypatch: pytest.MonkeyPatch, http: httpx.Client) -> int: + """Run `provx login` with the password arriving on stdin.""" + monkeypatch.setattr("sys.stdin", io.StringIO(PASSWORD)) + return run(["--server", SERVER, "login", "--username", "root"], http=http) + + +def test_login_stores_the_token_and_says_so( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + http, seen = _recording_stub() + + assert _login(monkeypatch, http) == exit_codes.OK + + assert store.load_token(SERVER) == TOKEN + assert json.loads(seen[0].content)["password"] == PASSWORD + output = capsys.readouterr() + assert "root" in output.out + assert PASSWORD not in output.out + output.err + + +def test_the_stored_token_file_is_owner_readable_only(monkeypatch: pytest.MonkeyPatch) -> None: + """A token in a world-readable file is a token every local account holds (rule PX-SECRETS).""" + http, _ = _recording_stub() + + _login(monkeypatch, http) + + path = store.credentials_path() + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + assert stat.S_IMODE(path.parent.stat().st_mode) == 0o700 + + +def test_neither_the_password_nor_the_token_is_printed( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + http, _ = _recording_stub() + + _login(monkeypatch, http) + + printed = capsys.readouterr() + assert PASSWORD not in printed.out + printed.err + assert TOKEN not in printed.out + printed.err + + +def test_json_output_omits_the_token( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """``--json`` output is routinely redirected into a file; a token there outlives the shell.""" + http, _ = _recording_stub() + monkeypatch.setattr("sys.stdin", io.StringIO(PASSWORD)) + + run(["--server", SERVER, "--json", "login", "--username", "root"], http=http) + + payload = json.loads(capsys.readouterr().out) + assert payload["username"] == "root" + assert TOKEN not in json.dumps(payload) + + +def test_a_later_command_presents_the_stored_token(monkeypatch: pytest.MonkeyPatch) -> None: + http, seen = _recording_stub() + _login(monkeypatch, http) + + listing, listing_seen = _recording_stub(payload=[]) + run(["--server", SERVER, "engagement", "list"], http=listing) + + assert listing_seen[0].headers["Authorization"] == f"Bearer {TOKEN}" + + +def test_the_environment_variable_still_wins_over_the_stored_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The pre-existing $PROVX_TOKEN path keeps working, and keeps taking precedence.""" + http, _ = _recording_stub() + _login(monkeypatch, http) + monkeypatch.setenv("PROVX_TOKEN", "from-the-environment") + + listing, listing_seen = _recording_stub(payload=[]) + run(["--server", SERVER, "engagement", "list"], http=listing) + + assert listing_seen[0].headers["Authorization"] == "Bearer from-the-environment" + + +def test_tokens_are_kept_per_server(monkeypatch: pytest.MonkeyPatch) -> None: + """A lab instance and a client instance are different sessions, not one that overwrites.""" + http, _ = _recording_stub() + _login(monkeypatch, http) + + other, _ = _recording_stub(payload={**LOGIN_BODY, "token": "other-server-token"}) + monkeypatch.setattr("sys.stdin", io.StringIO(PASSWORD)) + run(["--server", "http://other.test", "login", "--username", "root"], http=other) + + assert store.load_token(SERVER) == TOKEN + assert store.load_token("http://other.test") == "other-server-token" + + +def test_logout_revokes_server_side_and_forgets_the_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + http, _ = _recording_stub() + _login(monkeypatch, http) + + revoke, revoke_seen = _recording_stub(status_code=204) + assert run(["--server", SERVER, "logout"], http=revoke) == exit_codes.OK + + assert revoke_seen[0].url.path == "/auth/logout" + assert revoke_seen[0].headers["Authorization"] == f"Bearer {TOKEN}" + assert store.load_token(SERVER) is None + + +def test_logout_forgets_the_token_even_when_the_server_refuses( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unreachable or unhappy server is not a reason to keep a credential on disk.""" + http, _ = _recording_stub() + _login(monkeypatch, http) + + failing, _ = _recording_stub( + status_code=401, payload={"error_code": "unauthenticated", "message": "Nope."} + ) + assert run(["--server", SERVER, "logout"], http=failing) == exit_codes.API_ERROR + + assert store.load_token(SERVER) is None + + +def test_logout_without_a_session_is_not_an_error( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + revoke, _ = _recording_stub(status_code=204) + + assert run(["--server", SERVER, "logout"], http=revoke) == exit_codes.OK + + assert "No stored session" in capsys.readouterr().out + + +def test_admin_create_sends_the_password_in_the_body_and_never_in_argv( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + http, seen = _recording_stub(status_code=201, payload=USER_BODY) + monkeypatch.setattr("sys.stdin", io.StringIO(PASSWORD)) + argv = ["--server", SERVER, "admin", "create", "--username", "root"] + + assert run(argv, http=http) == exit_codes.OK + + assert PASSWORD not in " ".join(argv) + assert seen[0].url.path == "/auth/bootstrap" + assert json.loads(seen[0].content)["password"] == PASSWORD + assert PASSWORD not in capsys.readouterr().out + + +def test_admin_create_surfaces_the_servers_refusal(monkeypatch: pytest.MonkeyPatch) -> None: + """Bootstrap closing is the server's decision; the CLI reports it and does not retry.""" + http, _ = _recording_stub( + status_code=409, + payload={"error_code": "bootstrap_closed", "message": "An administrator already exists."}, + ) + monkeypatch.setattr("sys.stdin", io.StringIO(PASSWORD)) + + exit_code = run(["--server", SERVER, "admin", "create", "--username", "root"], http=http) + + assert exit_code == exit_codes.API_ERROR + + +def test_a_password_read_from_an_environment_variable_never_touches_argv( + monkeypatch: pytest.MonkeyPatch, +) -> None: + http, seen = _recording_stub() + monkeypatch.setenv("PROVX_PASSWORD", PASSWORD) + + exit_code = run( + ["--server", SERVER, "login", "--username", "root", "--password-env", "PROVX_PASSWORD"], + http=http, + ) + + assert exit_code == exit_codes.OK + assert json.loads(seen[0].content)["password"] == PASSWORD + + +def test_an_unset_password_variable_is_a_usage_error_and_sends_nothing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + http, seen = _recording_stub() + monkeypatch.delenv("PROVX_PASSWORD", raising=False) + + exit_code = run( + ["--server", SERVER, "login", "--username", "root", "--password-env", "PROVX_PASSWORD"], + http=http, + ) + + assert exit_code == exit_codes.USAGE + assert not seen + + +def test_login_uses_a_hidden_prompt_on_a_terminal(monkeypatch: pytest.MonkeyPatch) -> None: + """On a TTY the password must go through getpass, not a visible ``input()``.""" + asked: list[str] = [] + + class _Tty(io.StringIO): + def isatty(self) -> bool: + return True + + def fake_getpass(prompt: str) -> str: + asked.append(prompt) + return PASSWORD + + monkeypatch.setattr("sys.stdin", _Tty()) + monkeypatch.setattr("provx_cli.commands.getpass.getpass", fake_getpass) + http, seen = _recording_stub() + + assert run(["--server", SERVER, "login", "--username", "root"], http=http) == exit_codes.OK + + assert asked and "hidden" in asked[0] + assert json.loads(seen[0].content)["password"] == PASSWORD + + +def test_a_corrupt_store_reads_as_no_session_rather_than_crashing() -> None: + """The recovery from a mangled cache is to log in again, which an empty cache produces.""" + path = store.credentials_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("{not json", encoding="utf-8") + + assert store.load_token(SERVER) is None + + +def test_the_store_round_trips_and_clears() -> None: + store.save_token(SERVER, TOKEN, username="root", expires_at=None) + + assert store.load_token(SERVER) == TOKEN + assert store.stored_username(SERVER) == "root" + assert store.clear_token(SERVER) is True + assert store.load_token(SERVER) is None + assert store.clear_token(SERVER) is False diff --git a/packages/cli/tests/test_cli_secrets.py b/packages/cli/tests/test_cli_secrets.py index cea14c6..e8b1eec 100644 --- a/packages/cli/tests/test_cli_secrets.py +++ b/packages/cli/tests/test_cli_secrets.py @@ -69,6 +69,30 @@ def _credential_parser_options() -> set[str]: return {option for action in setter._actions for option in action.option_strings} +def _walk(parser: argparse.ArgumentParser, path: tuple[str, ...] = ()) -> list[tuple[str, str]]: + """Every ``(command path, flag)`` pair in the entire command tree. + + The per-command form above covers ``credential set`` and nothing else, which was fine when + that was the only command handling a secret. ``login`` and ``admin create`` handle one too, + so the check walks the whole tree: a future command with a ``--password`` flag is caught + without anyone remembering to extend a list. + + Args: + parser: The parser to descend from. + path: The command names already descended through. + + Returns: + One entry per option string, labelled with the command that exposes it. + """ + label = " ".join(path) or "provx" + found = [(label, option) for action in parser._actions for option in action.option_strings] + for action in parser._actions: + if isinstance(action, argparse._SubParsersAction): + for name, child in action.choices.items(): + found.extend(_walk(child, (*path, name))) + return found + + def _capture_request() -> tuple[httpx.Client, dict[str, object]]: """Build a stub client that records the request body it was sent. @@ -93,6 +117,25 @@ def test_no_flag_can_carry_a_secret_on_the_command_line() -> None: ) +def test_no_command_anywhere_in_the_tree_can_carry_a_secret_on_the_command_line() -> None: + """The same rule, applied to every command rather than the one that had it first.""" + offenders = [ + f"{command} {flag}" for command, flag in _walk(build_parser()) if flag in FORBIDDEN_FLAGS + ] + + assert not offenders, ( + f"these commands expose a secret-carrying flag: {sorted(offenders)}; a secret passed as " + "a flag lands in shell history and the process list (rule PX-SECRETS)." + ) + + +def test_the_walk_actually_reaches_the_commands_that_handle_secrets() -> None: + """Control for the check above: a walk that found nothing would pass it vacuously.""" + reached = {command for command, _ in _walk(build_parser())} + + assert {"credential set", "login", "admin create"} <= reached + + def test_passing_a_value_flag_is_rejected_rather_than_silently_accepted() -> None: client, _ = _capture_request()