Private
Public Access
Require password confirmation on account creation (#40)
Both account-creation surfaces now require the password twice: - Web signup (invite-based self-service): SignupComplete gains a password_confirm field with a model_validator backstop server-side, plus a client-side match check in SignupPage.tsx for immediate feedback -- the client check is the primary UX, the server check is defense in depth so the guarantee doesn't rely on the client alone. - CLI (python -m app.cli create-user): password is now an optional positional argument. If omitted, prompts interactively via getpass (hidden input) twice, retrying on mismatch -- matching what "entered twice and verified" actually means for a human typing blind. Passing the password directly as before still works unchanged, for scripted/ automated provisioning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+18
-2
@@ -7,6 +7,7 @@ created by an operator running this script directly on the app server.
|
|||||||
import argparse
|
import argparse
|
||||||
import asyncio
|
import asyncio
|
||||||
import base64
|
import base64
|
||||||
|
import getpass
|
||||||
|
|
||||||
from pydantic import ValidationError
|
from pydantic import ValidationError
|
||||||
|
|
||||||
@@ -15,6 +16,15 @@ from app.schemas.user import UserCreate
|
|||||||
from app.services.auth_service import DuplicateUserError, register_user
|
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:
|
async def _create_user(username: str, email: str, password: str, is_admin: bool) -> None:
|
||||||
try:
|
try:
|
||||||
data = UserCreate(username=username, email=email, password=password)
|
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 = subparsers.add_parser("create-user", help="Create a new user account")
|
||||||
create_user.add_argument("username")
|
create_user.add_argument("username")
|
||||||
create_user.add_argument("email")
|
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")
|
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")
|
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()
|
args = parser.parse_args()
|
||||||
|
|
||||||
if args.command == "create-user":
|
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":
|
elif args.command == "generate-vapid-keys":
|
||||||
_generate_vapid_keys()
|
_generate_vapid_keys()
|
||||||
|
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import uuid
|
import uuid
|
||||||
from datetime import datetime
|
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
|
from app.models import InviteStatus
|
||||||
|
|
||||||
@@ -29,3 +29,15 @@ class SignupComplete(BaseModel):
|
|||||||
token: str
|
token: str
|
||||||
username: str = Field(min_length=3, max_length=50)
|
username: str = Field(min_length=3, max_length=50)
|
||||||
password: str = Field(min_length=8, max_length=200)
|
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
|
||||||
|
|||||||
@@ -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
|
||||||
@@ -70,7 +70,7 @@ async def test_signup_flow_end_to_end(client, db_session, monkeypatch):
|
|||||||
|
|
||||||
complete = await client.post(
|
complete = await client.post(
|
||||||
"/api/signup",
|
"/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.status_code == 200, complete.text
|
||||||
assert complete.json()["email"] == "newperson@example.com"
|
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"
|
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):
|
async def test_invalid_token_rejected(client, db_session):
|
||||||
await register_and_login(client, db_session, username="alice")
|
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(
|
complete = await client.post(
|
||||||
"/api/signup",
|
"/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
|
assert complete.status_code == 400
|
||||||
|
|
||||||
@@ -109,7 +135,7 @@ async def test_expired_token_rejected(client, db_session, monkeypatch):
|
|||||||
|
|
||||||
complete = await client.post(
|
complete = await client.post(
|
||||||
"/api/signup",
|
"/api/signup",
|
||||||
json={"token": token, "username": "late", "password": "password123"},
|
json={"token": token, "username": "late", "password": "password123", "password_confirm": "password123"},
|
||||||
)
|
)
|
||||||
assert complete.status_code == 400
|
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(
|
first = await client.post(
|
||||||
"/api/signup",
|
"/api/signup",
|
||||||
json={"token": token, "username": "onceuser", "password": "password123"},
|
json={"token": token, "username": "onceuser", "password": "password123", "password_confirm": "password123"},
|
||||||
)
|
)
|
||||||
assert first.status_code == 200
|
assert first.status_code == 200
|
||||||
|
|
||||||
second = await client.post(
|
second = await client.post(
|
||||||
"/api/signup",
|
"/api/signup",
|
||||||
json={"token": token, "username": "onceuser2", "password": "password123"},
|
json={"token": token, "username": "onceuser2", "password": "password123", "password_confirm": "password123"},
|
||||||
)
|
)
|
||||||
assert second.status_code == 400
|
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(
|
complete = await client.post(
|
||||||
"/api/signup",
|
"/api/signup",
|
||||||
json={"token": token, "username": "revokeduser", "password": "password123"},
|
json={"token": token, "username": "revokeduser", "password": "password123", "password_confirm": "password123"},
|
||||||
)
|
)
|
||||||
assert complete.status_code == 400
|
assert complete.status_code == 400
|
||||||
|
|
||||||
|
|||||||
@@ -9,9 +9,10 @@ export function completeSignup(
|
|||||||
token: string,
|
token: string,
|
||||||
username: string,
|
username: string,
|
||||||
password: string,
|
password: string,
|
||||||
|
passwordConfirm: string,
|
||||||
): Promise<User> {
|
): Promise<User> {
|
||||||
return apiFetch<User>('/api/signup', {
|
return apiFetch<User>('/api/signup', {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
body: JSON.stringify({ token, username, password }),
|
body: JSON.stringify({ token, username, password, password_confirm: passwordConfirm }),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ export function SignupPage() {
|
|||||||
|
|
||||||
const [username, setUsername] = useState('')
|
const [username, setUsername] = useState('')
|
||||||
const [password, setPassword] = useState('')
|
const [password, setPassword] = useState('')
|
||||||
|
const [passwordConfirm, setPasswordConfirm] = useState('')
|
||||||
const [error, setError] = useState<string | null>(null)
|
const [error, setError] = useState<string | null>(null)
|
||||||
const [submitting, setSubmitting] = useState(false)
|
const [submitting, setSubmitting] = useState(false)
|
||||||
|
|
||||||
@@ -40,9 +41,13 @@ export function SignupPage() {
|
|||||||
async function handleSubmit(e: FormEvent) {
|
async function handleSubmit(e: FormEvent) {
|
||||||
e.preventDefault()
|
e.preventDefault()
|
||||||
setError(null)
|
setError(null)
|
||||||
|
if (password !== passwordConfirm) {
|
||||||
|
setError("Passwords don't match")
|
||||||
|
return
|
||||||
|
}
|
||||||
setSubmitting(true)
|
setSubmitting(true)
|
||||||
try {
|
try {
|
||||||
const newUser = await completeSignup(token, username, password)
|
const newUser = await completeSignup(token, username, password, passwordConfirm)
|
||||||
updateUser(newUser)
|
updateUser(newUser)
|
||||||
navigate('/rooms')
|
navigate('/rooms')
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
@@ -92,6 +97,18 @@ export function SignupPage() {
|
|||||||
onChange={(e) => setPassword(e.target.value)}
|
onChange={(e) => setPassword(e.target.value)}
|
||||||
required
|
required
|
||||||
minLength={8}
|
minLength={8}
|
||||||
|
autoComplete="new-password"
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
<label>
|
||||||
|
Confirm password
|
||||||
|
<input
|
||||||
|
type="password"
|
||||||
|
value={passwordConfirm}
|
||||||
|
onChange={(e) => setPasswordConfirm(e.target.value)}
|
||||||
|
required
|
||||||
|
minLength={8}
|
||||||
|
autoComplete="new-password"
|
||||||
/>
|
/>
|
||||||
</label>
|
</label>
|
||||||
{error && <p className="login-error">{error}</p>}
|
{error && <p className="login-error">{error}</p>}
|
||||||
|
|||||||
Reference in New Issue
Block a user