diff --git a/backend/app/cli.py b/backend/app/cli.py index 2a870fa..65ae74d 100644 --- a/backend/app/cli.py +++ b/backend/app/cli.py @@ -7,6 +7,7 @@ created by an operator running this script directly on the app server. import argparse import asyncio import base64 +import getpass from pydantic import ValidationError @@ -15,6 +16,15 @@ from app.schemas.user import UserCreate from app.services.auth_service import DuplicateUserError, register_user +def _prompt_password() -> str: + while True: + password = getpass.getpass("Password: ") + confirm = getpass.getpass("Confirm password: ") + if password == confirm: + return password + print("Passwords didn't match -- try again.") + + async def _create_user(username: str, email: str, password: str, is_admin: bool) -> None: try: data = UserCreate(username=username, email=email, password=password) @@ -69,7 +79,12 @@ def main() -> None: create_user = subparsers.add_parser("create-user", help="Create a new user account") create_user.add_argument("username") create_user.add_argument("email") - create_user.add_argument("password") + create_user.add_argument( + "password", + nargs="?", + default=None, + help="If omitted, you'll be prompted interactively (hidden input, entered twice to confirm).", + ) create_user.add_argument("--admin", action="store_true", help="Grant is_site_admin") subparsers.add_parser("generate-vapid-keys", help="Generate a VAPID key pair for push notifications") @@ -77,7 +92,8 @@ def main() -> None: args = parser.parse_args() if args.command == "create-user": - asyncio.run(_create_user(args.username, args.email, args.password, args.admin)) + password = args.password if args.password is not None else _prompt_password() + asyncio.run(_create_user(args.username, args.email, password, args.admin)) elif args.command == "generate-vapid-keys": _generate_vapid_keys() diff --git a/backend/app/schemas/site_invite.py b/backend/app/schemas/site_invite.py index 82017a6..4fd02da 100644 --- a/backend/app/schemas/site_invite.py +++ b/backend/app/schemas/site_invite.py @@ -1,7 +1,7 @@ import uuid from datetime import datetime -from pydantic import BaseModel, ConfigDict, EmailStr, Field +from pydantic import BaseModel, ConfigDict, EmailStr, Field, model_validator from app.models import InviteStatus @@ -29,3 +29,15 @@ class SignupComplete(BaseModel): token: str username: str = Field(min_length=3, max_length=50) password: str = Field(min_length=8, max_length=200) + password_confirm: str + + # Backend backstop -- the signup form does its own client-side match + # check for immediate feedback (see SignupPage.tsx), but account + # creation is irreversible enough (a typo'd password with no recovery + # path until forgot-password) that the guarantee shouldn't rely on the + # client alone. + @model_validator(mode="after") + def _passwords_match(self) -> "SignupComplete": + if self.password != self.password_confirm: + raise ValueError("Passwords don't match") + return self diff --git a/backend/tests/test_cli.py b/backend/tests/test_cli.py new file mode 100644 index 0000000..c09b42e --- /dev/null +++ b/backend/tests/test_cli.py @@ -0,0 +1,78 @@ +import uuid + +import pytest +import pytest_asyncio +from sqlalchemy import select + +from app.cli import _create_user, _prompt_password +from app.database import async_session_factory +from app.database import engine as _cli_engine +from app.models import User + + +def _unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +@pytest_asyncio.fixture(autouse=True) +async def _dispose_cli_engine_between_tests(): + # _create_user (like the real CLI) uses app.database's module-level + # engine directly, not the db_session fixture's own per-test engine -- + # pytest-asyncio gives each test function a fresh event loop by + # default, and a pooled connection opened on a since-closed loop + # produces asyncpg "another operation is in progress" errors if reused + # by a later test. Disposing after every test forces a fresh connection + # next time instead of reusing a stale one. + yield + await _cli_engine.dispose() + + +async def _get_user(username: str) -> User | None: + async with async_session_factory() as session: + result = await session.execute(select(User).where(User.username == username)) + return result.scalar_one_or_none() + + +def test_prompt_password_matches_on_first_try(monkeypatch): + responses = iter(["correct-horse", "correct-horse"]) + monkeypatch.setattr("getpass.getpass", lambda prompt="": next(responses)) + + assert _prompt_password() == "correct-horse" + + +def test_prompt_password_retries_on_mismatch(monkeypatch, capsys): + responses = iter(["typo-password", "correct-password", "correct-password", "correct-password"]) + monkeypatch.setattr("getpass.getpass", lambda prompt="": next(responses)) + + assert _prompt_password() == "correct-password" + assert "didn't match" in capsys.readouterr().out + + +async def test_create_user_with_explicit_password_succeeds(): + username = _unique("alice") + await _create_user(username, f"{username}@example.com", "password123", False) + + user = await _get_user(username) + assert user is not None + assert user.is_site_admin is False + + +async def test_create_user_via_prompted_password(monkeypatch): + responses = iter(["prompted-pass", "prompted-pass"]) + monkeypatch.setattr("getpass.getpass", lambda prompt="": next(responses)) + + username = _unique("bob") + password = _prompt_password() + await _create_user(username, f"{username}@example.com", password, True) + + user = await _get_user(username) + assert user is not None + assert user.is_site_admin is True + + +async def test_create_user_rejects_short_password(): + username = _unique("shortpw") + with pytest.raises(SystemExit): + await _create_user(username, f"{username}@example.com", "short", False) + + assert await _get_user(username) is None diff --git a/backend/tests/test_site_invites.py b/backend/tests/test_site_invites.py index b6c5fa6..be35958 100644 --- a/backend/tests/test_site_invites.py +++ b/backend/tests/test_site_invites.py @@ -70,7 +70,7 @@ async def test_signup_flow_end_to_end(client, db_session, monkeypatch): complete = await client.post( "/api/signup", - json={"token": token, "username": "newperson", "password": "password123"}, + json={"token": token, "username": "newperson", "password": "password123", "password_confirm": "password123"}, ) assert complete.status_code == 200, complete.text assert complete.json()["email"] == "newperson@example.com" @@ -80,6 +80,32 @@ async def test_signup_flow_end_to_end(client, db_session, monkeypatch): assert me.json()["username"] == "newperson" +async def test_signup_rejects_mismatched_password_confirmation(client, db_session, monkeypatch): + calls = _fake_smtp(monkeypatch) + admin = await register_and_login(client, db_session, username="admin1") + await _make_admin(db_session, admin["id"]) + await _configure_smtp(client) + + await client.post("/api/admin/invites", json={"email": "typo@example.com"}) + token = _extract_token(calls[0]["message"].get_content()) + + complete = await client.post( + "/api/signup", + json={ + "token": token, + "username": "typouser", + "password": "password123", + "password_confirm": "password124", + }, + ) + assert complete.status_code == 422 + + # The mismatch must not have consumed the invite -- a typo shouldn't + # burn a single-use token. + validate = await client.get(f"/api/signup/validate?token={token}") + assert validate.status_code == 200 + + async def test_invalid_token_rejected(client, db_session): await register_and_login(client, db_session, username="alice") @@ -88,7 +114,7 @@ async def test_invalid_token_rejected(client, db_session): complete = await client.post( "/api/signup", - json={"token": "not-a-real-token", "username": "someone", "password": "password123"}, + json={"token": "not-a-real-token", "username": "someone", "password": "password123", "password_confirm": "password123"}, ) assert complete.status_code == 400 @@ -109,7 +135,7 @@ async def test_expired_token_rejected(client, db_session, monkeypatch): complete = await client.post( "/api/signup", - json={"token": token, "username": "late", "password": "password123"}, + json={"token": token, "username": "late", "password": "password123", "password_confirm": "password123"}, ) assert complete.status_code == 400 @@ -125,13 +151,13 @@ async def test_used_token_cannot_be_reused(client, db_session, monkeypatch): first = await client.post( "/api/signup", - json={"token": token, "username": "onceuser", "password": "password123"}, + json={"token": token, "username": "onceuser", "password": "password123", "password_confirm": "password123"}, ) assert first.status_code == 200 second = await client.post( "/api/signup", - json={"token": token, "username": "onceuser2", "password": "password123"}, + json={"token": token, "username": "onceuser2", "password": "password123", "password_confirm": "password123"}, ) assert second.status_code == 400 @@ -152,7 +178,7 @@ async def test_revoke_site_invite_prevents_signup(client, db_session, monkeypatc complete = await client.post( "/api/signup", - json={"token": token, "username": "revokeduser", "password": "password123"}, + json={"token": token, "username": "revokeduser", "password": "password123", "password_confirm": "password123"}, ) assert complete.status_code == 400 diff --git a/frontend/src/api/signup.ts b/frontend/src/api/signup.ts index e7a6a10..1ecbcdd 100644 --- a/frontend/src/api/signup.ts +++ b/frontend/src/api/signup.ts @@ -9,9 +9,10 @@ export function completeSignup( token: string, username: string, password: string, + passwordConfirm: string, ): Promise { return apiFetch('/api/signup', { method: 'POST', - body: JSON.stringify({ token, username, password }), + body: JSON.stringify({ token, username, password, password_confirm: passwordConfirm }), }) } diff --git a/frontend/src/pages/SignupPage.tsx b/frontend/src/pages/SignupPage.tsx index c517682..61acdeb 100644 --- a/frontend/src/pages/SignupPage.tsx +++ b/frontend/src/pages/SignupPage.tsx @@ -18,6 +18,7 @@ export function SignupPage() { const [username, setUsername] = useState('') const [password, setPassword] = useState('') + const [passwordConfirm, setPasswordConfirm] = useState('') const [error, setError] = useState(null) const [submitting, setSubmitting] = useState(false) @@ -40,9 +41,13 @@ export function SignupPage() { async function handleSubmit(e: FormEvent) { e.preventDefault() setError(null) + if (password !== passwordConfirm) { + setError("Passwords don't match") + return + } setSubmitting(true) try { - const newUser = await completeSignup(token, username, password) + const newUser = await completeSignup(token, username, password, passwordConfirm) updateUser(newUser) navigate('/rooms') } catch (err) { @@ -92,6 +97,18 @@ export function SignupPage() { onChange={(e) => setPassword(e.target.value)} required minLength={8} + autoComplete="new-password" + /> + + {error &&

{error}

}