feat(auth): add velodrome create-admin to bootstrap the first user

A fresh deployment could not be used. Registration requires a valid invite code, invites can only be issued by an existing admin, and a newly migrated database has neither -- so there was no way to create the first account. docs/PLAN.md always called for this command; it was never built during Phase 0, and D16 (single-container deploy) made the gap reachable. Adds a console script -- `[project.scripts]` -> velodrome.cli:main -- which installs into the same venv as alembic and uvicorn, so the deployed image already has it on PATH: docker exec -it velodrome velodrome create-admin --email you@example.com The account-creating logic is `auth.service.create_admin`, not something in cli.py, so that `db.auth_session` stays confined to auth/service.py as its docstring requires. Its lookup is an exact match on a unique key, which is the pattern db.py documents as safe on that session. Deliberate constraints, all covered by tests (see docs/DECISIONS.md D18): - Refuses an email that already exists rather than updating the row. An operator re-running a months-old command from shell history means "create", never "reset the password"; silently accepting would make this an undocumented password-reset tool that any container-exec grants. - Not restricted to "only when there are zero users". The restriction buys nothing -- reaching the command already requires process execution inside the container, which already permits rewriting the SQLite file directly -- while removing the cases that do happen: a second admin, and recovering an instance whose only admin was lost. - No --password flag. An argument lands in shell history, in ps output, and in the Docker daemon's record of the exec'd command. A TTY prompt (with confirmation) and --password-stdin are the two forms that avoid all three. - Pydantic's ValidationError is never printed verbatim: its rendering embeds the offending value, which for a short password prints the password itself. Only loc and msg are shown (CLAUDE.md invariant #5). role="admin" is recorded but nothing enforces it yet -- there is no admin-only endpoint until invite management in Phase 1. ROLE_ADMIN/ ROLE_MEMBER become named constants, and AuthenticatedSession carries the role for that future check. It is deliberately absent from UserOut, so no HTTP response and no OpenAPI contract changes. The register endpoint's password and display-name constraints move to named aliases in schemas/auth so the CLI applies exactly the same rules rather than a drifting copy. Verified: ruff check, ruff format --check, mypy --strict, and the full pytest suite (28 passed) from apps/api/; `alembic check` reports no model drift. Also smoke-tested end to end against a scratch database -- creation, the duplicate-email refusal, and the no-TTY message all behave as described. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R2ZKeWkZV7ehf7fivrAkkG
2026-09-21 22:30:42 -04:00
parent 3b80034f0e
commit b7b4c31296
7 changed files with 588 additions and 10 deletions
@@ -0,0 +1,308 @@
+"""Tests for the operator CLI (`velodrome create-admin`).
+
+These drive `cli.run()` directly against the same real SQLite database and real Argon2id hashing
+every other test uses — no mocks, per CLAUDE.md's test policy. That matters more than usual here:
+this command is the only way to create the first account on a fresh deployment, it is run exactly
+once by a human who has no way to debug it, and the failure mode of "it printed success but the
+password doesn't actually work" is indistinguishable from a broken deployment. So the central test
+below doesn't assert on a return code — it creates an admin through the CLI and then logs in as
+that admin over HTTP, proving the hash the CLI wrote is one the login path accepts.
+
+The other thing under test is what the CLI *refuses* to do: overwrite an existing account, and
+echo a rejected password back to the terminal (CLAUDE.md invariant #5).
+"""
+
+import io
+import sys
+import tomllib
+from pathlib import Path
+
+import httpx
+import pytest
+from sqlalchemy import text
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from velodrome import cli
+from velodrome.models import ROLE_ADMIN, ROLE_MEMBER
+
+_PASSWORD = "correct horse battery staple"
+_OTHER_PASSWORD = "an entirely different passphrase"
+
+
+class _FakeTty:
+    """Stands in for `sys.stdin` attached to a terminal, so `_read_password` takes the prompt
+    branch rather than the pipe branch. `readline` raises rather than returning a value: if the
+    prompt path ever silently starts reading stdin instead of calling getpass, that's a behaviour
+    change this should fail on, not absorb."""
+
+    def isatty(self) -> bool:
+        return True
+
+    def readline(self) -> str:
+        raise AssertionError("the prompt path must not read stdin directly")
+
+
+def _pipe(password: str, *, newline: str = "\n") -> io.StringIO:
+    """stdin as a pipe (isatty() is False on StringIO), carrying one line."""
+    return io.StringIO(f"{password}{newline}")
+
+
+async def test_create_admin_creates_an_account_that_can_actually_log_in(
+    client: httpx.AsyncClient, db_auth: AsyncSession, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The load-bearing test: bootstrap an admin through the CLI, then log in as them over HTTP.
+
+    This is deliberately an end-to-end assertion rather than "did a row appear with a hash in it".
+    The whole point of the command is to produce working credentials on a deployment where nobody
+    can yet log in to check, so the only assertion worth making is that the credentials work
+    through the same endpoint a real operator would use next.
+    """
+    monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD))
+    code = await cli.run(
+        ["create-admin", "--email", "boss@example.com", "--name", "Boss", "--password-stdin"]
+    )
+    assert code == 0
+
+    resp = await client.post(
+        "/api/v1/auth/login", json={"email": "boss@example.com", "password": _PASSWORD}
+    )
+    assert resp.status_code == 200, resp.text
+    assert "vd_session" in resp.cookies
+
+    me = await client.get("/api/v1/auth/me")
+    assert me.status_code == 200
+    assert me.json()["email"] == "boss@example.com"
+    assert me.json()["display_name"] == "Boss"
+
+
+async def test_create_admin_records_the_admin_role(
+    db_auth: AsyncSession, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The role is the one thing that distinguishes this from registration, and nothing enforces
+    it yet (docs/DECISIONS.md D17) — so nothing else in the suite would notice if it silently
+    wrote `member`. Asserted against the stored column directly, and against ROLE_MEMBER too, so
+    this fails loudly rather than passing vacuously if the default ever changes."""
+    monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD))
+    assert await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"]) == 0
+
+    role = (
+        await db_auth.execute(
+            text("SELECT role FROM users WHERE email = :email"), {"email": "boss@example.com"}
+        )
+    ).scalar_one()
+    await db_auth.commit()
+    assert role == ROLE_ADMIN
+    assert role != ROLE_MEMBER
+
+
+async def test_create_admin_defaults_display_name_to_the_email_local_part(
+    db_auth: AsyncSession, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD))
+    assert await cli.run(["create-admin", "--email", "benny@example.com", "--password-stdin"]) == 0
+
+    name = (
+        await db_auth.execute(
+            text("SELECT display_name FROM users WHERE email = :email"),
+            {"email": "benny@example.com"},
+        )
+    ).scalar_one()
+    await db_auth.commit()
+    assert name == "benny"
+
+
+async def test_create_admin_refuses_an_existing_email_without_touching_the_account(
+    client: httpx.AsyncClient, db_auth: AsyncSession, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Re-running the bootstrap command must not become an undocumented password reset.
+
+    "Refused" is asserted three ways, because exit code 1 alone would also be satisfied by a
+    command that failed *after* corrupting the row: the original password must still work, the new
+    one must not, and the display name must be unchanged.
+    """
+    monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD))
+    assert (
+        await cli.run(
+            ["create-admin", "--email", "boss@example.com", "--name", "Boss", "--password-stdin"]
+        )
+        == 0
+    )
+
+    monkeypatch.setattr(sys, "stdin", _pipe(_OTHER_PASSWORD))
+    second = await cli.run(
+        ["create-admin", "--email", "boss@example.com", "--name", "Impostor", "--password-stdin"]
+    )
+    assert second == 1
+
+    still_works = await client.post(
+        "/api/v1/auth/login", json={"email": "boss@example.com", "password": _PASSWORD}
+    )
+    assert still_works.status_code == 200, "the original password must survive a refused re-run"
+
+    rejected = await client.post(
+        "/api/v1/auth/login", json={"email": "boss@example.com", "password": _OTHER_PASSWORD}
+    )
+    assert rejected.status_code == 401, "the refused run's password must never become valid"
+
+    name = (
+        await db_auth.execute(
+            text("SELECT display_name FROM users WHERE email = :email"),
+            {"email": "boss@example.com"},
+        )
+    ).scalar_one()
+    await db_auth.commit()
+    assert name == "Boss"
+
+
+async def test_create_admin_error_message_never_echoes_the_password(
+    monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    """CLAUDE.md invariant #5, in the one place it's easy to breach by accident.
+
+    Pydantic's default rendering of a ValidationError embeds the offending value — for a
+    too-short password that means printing the password itself to the operator's terminal, and
+    into whatever captured that output (a CI log, a `script` session, a scrollback buffer shared
+    in a bug report). `_validate` strips it deliberately; this proves it stays stripped.
+    """
+    secret = "short"
+    monkeypatch.setattr(sys, "stdin", _pipe(secret))
+    code = await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"])
+    assert code == 1
+
+    captured = capsys.readouterr()
+    assert secret not in captured.out
+    assert secret not in captured.err
+    # ...while still being a useful message: it must name the offending field and the rule.
+    assert "password" in captured.err
+    assert "at least 8" in captured.err
+
+
+async def test_create_admin_rejects_a_malformed_email(
+    monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD))
+    code = await cli.run(["create-admin", "--email", "not-an-email", "--password-stdin"])
+    assert code == 1
+    assert "email" in capsys.readouterr().err
+
+
+async def test_password_stdin_rejects_an_empty_line(
+    monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    """An empty pipe is nearly always `$PASSWORD` being unset in the operator's shell. Failing
+    loudly beats creating an account whose password is the empty string."""
+    monkeypatch.setattr(sys, "stdin", io.StringIO(""))
+    code = await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"])
+    assert code == 1
+    assert "empty" in capsys.readouterr().err
+
+
+async def test_password_stdin_preserves_a_trailing_space(
+    client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Only the line ending is stripped, not surrounding whitespace — a password with a trailing
+    space is legitimate, and trimming it would create an account whose password can never be typed
+    back in. Proven through a real login rather than by inspecting the hash."""
+    padded = f"{_PASSWORD} "
+    monkeypatch.setattr(sys, "stdin", _pipe(padded, newline="\r\n"))
+    assert await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"]) == 0
+
+    resp = await client.post(
+        "/api/v1/auth/login", json={"email": "boss@example.com", "password": padded}
+    )
+    assert resp.status_code == 200, "the trailing space must be part of the stored password"
+
+    trimmed = await client.post(
+        "/api/v1/auth/login", json={"email": "boss@example.com", "password": _PASSWORD}
+    )
+    assert trimmed.status_code == 401
+
+
+async def test_without_a_tty_or_password_stdin_it_explains_how_to_run_it(
+    monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    """The most likely first-run mistake is `docker exec` without `-t`. getpass would otherwise
+    fail with a bare OSError, so the command catches it first and prints both working forms."""
+    monkeypatch.setattr(sys, "stdin", io.StringIO(""))
+    code = await cli.run(["create-admin", "--email", "boss@example.com"])
+    assert code == 1
+
+    err = capsys.readouterr().err
+    assert "docker exec -it" in err
+    assert "--password-stdin" in err
+
+
+async def test_prompt_path_requires_the_confirmation_to_match(
+    monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    """A typo in a password nobody can see, on the one account that can't be recovered by another
+    admin, is worth a second prompt."""
+    monkeypatch.setattr(sys, "stdin", _FakeTty())
+    answers = iter([_PASSWORD, _OTHER_PASSWORD])
+    monkeypatch.setattr(cli.getpass, "getpass", lambda prompt="": next(answers))
+
+    code = await cli.run(["create-admin", "--email", "boss@example.com"])
+    assert code == 1
+    assert "did not match" in capsys.readouterr().err
+
+
+async def test_prompt_path_creates_the_account_when_both_entries_match(
+    client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The interactive path is the one the docs tell operators to use, so it gets the same
+    end-to-end login proof as the piped path."""
+    monkeypatch.setattr(sys, "stdin", _FakeTty())
+    answers = iter([_PASSWORD, _PASSWORD])
+    monkeypatch.setattr(cli.getpass, "getpass", lambda prompt="": next(answers))
+
+    assert await cli.run(["create-admin", "--email", "boss@example.com"]) == 0
+
+    resp = await client.post(
+        "/api/v1/auth/login", json={"email": "boss@example.com", "password": _PASSWORD}
+    )
+    assert resp.status_code == 200
+
+
+async def test_success_output_never_contains_the_password(
+    monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    """The failure path is covered above; the success path prints more, so it gets its own check.
+    The realistic leak here is a well-meaning "created with password: ..." confirmation line."""
+    monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD))
+    assert await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"]) == 0
+
+    captured = capsys.readouterr()
+    assert _PASSWORD not in captured.out
+    assert _PASSWORD not in captured.err
+    assert "boss@example.com" in captured.out
+    assert ROLE_ADMIN in captured.out
+
+
+async def test_a_missing_required_option_exits_two_not_one(
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    """argparse's usage errors exit 2; a refusal from the command itself exits 1. A script driving
+    this should be able to tell "you called it wrong" from "it ran and declined"."""
+    with pytest.raises(SystemExit) as exc:
+        await cli.run(["create-admin"])
+    assert exc.value.code == 2
+    capsys.readouterr()
+
+
+async def test_no_subcommand_is_a_usage_error(capsys: pytest.CaptureFixture[str]) -> None:
+    with pytest.raises(SystemExit) as exc:
+        await cli.run([])
+    assert exc.value.code == 2
+    capsys.readouterr()
+
+
+def test_the_console_script_is_registered_under_the_name_the_docs_use() -> None:
+    """deploy/README.md and the CLI's own error messages tell operators to run
+    `docker exec -it velodrome velodrome create-admin`. That only works because pyproject declares
+    the console script, which nothing else in the test suite would exercise — an editable install
+    imports `velodrome.cli` fine whether or not the entry point exists. Asserted against the
+    manifest so renaming the module or the function fails here rather than on a deployment.
+    """
+    pyproject = Path(__file__).resolve().parents[1] / "pyproject.toml"
+    manifest = tomllib.loads(pyproject.read_text(encoding="utf-8"))
+    assert manifest["project"]["scripts"]["velodrome"] == "velodrome.cli:main"