1581 lines
47 KiB
Markdown
1581 lines
47 KiB
Markdown
# Form Validation Hardening Implementation Plan
|
|
|
|
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
|
|
|
**Goal:** Harden form validation across the application — fix security gaps (inactive user login, rate limiting, password max length), add missing validation (username, groups, locale, password strength), deduplicate profile validation, require current password on change, add CSRF tokens to admin forms, and display all validation errors.
|
|
|
|
**Architecture:** Extend `src/porchlight/validation.py` with new Pydantic models and a shared error-formatting helper. Add `slowapi` middleware for rate limiting. Add `zxcvbn` for password strength. Update route handlers to use the new models and helper. Update templates for CSRF tokens and password UI.
|
|
|
|
**Tech Stack:** Pydantic v2, `slowapi` (rate limiting), `zxcvbn` (password strength), FastAPI, HTMX.
|
|
|
|
---
|
|
|
|
### Task 1: Add `slowapi` and `zxcvbn` dependencies
|
|
|
|
**Files:**
|
|
- Modify: `pyproject.toml:7-22`
|
|
|
|
- [ ] **Step 1: Add dependencies**
|
|
|
|
In `pyproject.toml`, add to the `dependencies` list:
|
|
|
|
```toml
|
|
"slowapi>=0.1.9",
|
|
"zxcvbn>=4.5",
|
|
```
|
|
|
|
- [ ] **Step 2: Install**
|
|
|
|
Run: `uv sync`
|
|
|
|
- [ ] **Step 3: Verify imports**
|
|
|
|
Run:
|
|
```bash
|
|
uv run python -c "from slowapi import Limiter; print('slowapi OK')"
|
|
uv run python -c "from zxcvbn import zxcvbn; print('zxcvbn OK')"
|
|
```
|
|
Expected: Both print OK.
|
|
|
|
- [ ] **Step 4: Commit**
|
|
|
|
```bash
|
|
git add pyproject.toml uv.lock
|
|
git commit -m "build: add slowapi and zxcvbn dependencies"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 2: Add locale BCP 47 validator to `ProfileUpdate`
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/validation.py:1-42`
|
|
- Modify: `tests/test_validation.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Add to `tests/test_validation.py`:
|
|
|
|
```python
|
|
class TestProfileUpdateLocale:
|
|
def test_valid_locale_language_only(self) -> None:
|
|
p = ProfileUpdate(locale="en")
|
|
assert p.locale == "en"
|
|
|
|
def test_valid_locale_language_region(self) -> None:
|
|
p = ProfileUpdate(locale="sv-SE")
|
|
assert p.locale == "sv-SE"
|
|
|
|
def test_valid_locale_language_script_region(self) -> None:
|
|
p = ProfileUpdate(locale="zh-Hans-CN")
|
|
assert p.locale == "zh-Hans-CN"
|
|
|
|
def test_valid_locale_three_letter(self) -> None:
|
|
p = ProfileUpdate(locale="gsw")
|
|
assert p.locale == "gsw"
|
|
|
|
def test_empty_locale_allowed(self) -> None:
|
|
p = ProfileUpdate(locale="")
|
|
assert p.locale == ""
|
|
|
|
def test_invalid_locale_rejected(self) -> None:
|
|
with pytest.raises(ValidationError, match="locale"):
|
|
ProfileUpdate(locale="not_a_locale!")
|
|
|
|
def test_invalid_locale_numbers(self) -> None:
|
|
with pytest.raises(ValidationError, match="locale"):
|
|
ProfileUpdate(locale="12345")
|
|
|
|
def test_whitespace_locale_becomes_empty(self) -> None:
|
|
p = ProfileUpdate(locale=" ")
|
|
assert p.locale == ""
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestProfileUpdateLocale -v`
|
|
Expected: FAIL (no locale validation exists yet)
|
|
|
|
- [ ] **Step 3: Add locale validator**
|
|
|
|
In `src/porchlight/validation.py`, add at the top:
|
|
|
|
```python
|
|
import re
|
|
```
|
|
|
|
And add a validator to `ProfileUpdate`:
|
|
|
|
```python
|
|
@field_validator("locale", mode="before")
|
|
@classmethod
|
|
def validate_locale(cls, v: str) -> str:
|
|
if isinstance(v, str):
|
|
v = v.strip()
|
|
if v == "":
|
|
return ""
|
|
# BCP 47 simplified: language(-Script)?(-REGION)?
|
|
if not re.match(r"^[a-z]{2,3}(-[A-Z][a-z]{3})?(-[A-Z]{2})?$", v):
|
|
raise ValueError("Locale must be a valid BCP 47 language tag (e.g. en, sv-SE, zh-Hans-CN)")
|
|
return v
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestProfileUpdateLocale -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/validation.py tests/test_validation.py
|
|
git commit -m "feat: add BCP 47 locale validation to ProfileUpdate"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 3: Add username validation model
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/validation.py`
|
|
- Modify: `tests/test_validation.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Add to `tests/test_validation.py`:
|
|
|
|
```python
|
|
from porchlight.validation import UsernameInput
|
|
|
|
|
|
class TestUsernameInput:
|
|
def test_valid_simple(self) -> None:
|
|
u = UsernameInput(username="alice")
|
|
assert u.username == "alice"
|
|
|
|
def test_valid_with_dots_dashes_underscores(self) -> None:
|
|
u = UsernameInput(username="alice.bob_charlie-1")
|
|
assert u.username == "alice.bob_charlie-1"
|
|
|
|
def test_valid_email_style(self) -> None:
|
|
u = UsernameInput(username="user@example.com")
|
|
assert u.username == "user@example.com"
|
|
|
|
def test_valid_uppercase(self) -> None:
|
|
u = UsernameInput(username="Alice")
|
|
assert u.username == "Alice"
|
|
|
|
def test_empty_rejected(self) -> None:
|
|
with pytest.raises(ValidationError, match="username"):
|
|
UsernameInput(username="")
|
|
|
|
def test_whitespace_only_rejected(self) -> None:
|
|
with pytest.raises(ValidationError, match="username"):
|
|
UsernameInput(username=" ")
|
|
|
|
def test_too_long_rejected(self) -> None:
|
|
with pytest.raises(ValidationError):
|
|
UsernameInput(username="a" * 256)
|
|
|
|
def test_spaces_rejected(self) -> None:
|
|
with pytest.raises(ValidationError, match="username"):
|
|
UsernameInput(username="alice bob")
|
|
|
|
def test_special_chars_rejected(self) -> None:
|
|
with pytest.raises(ValidationError, match="username"):
|
|
UsernameInput(username="alice<script>")
|
|
|
|
def test_strips_whitespace(self) -> None:
|
|
u = UsernameInput(username=" alice ")
|
|
assert u.username == "alice"
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestUsernameInput -v`
|
|
Expected: FAIL (UsernameInput doesn't exist)
|
|
|
|
- [ ] **Step 3: Add UsernameInput model**
|
|
|
|
In `src/porchlight/validation.py`, add:
|
|
|
|
```python
|
|
class UsernameInput(BaseModel):
|
|
username: str = Field(max_length=255)
|
|
|
|
@field_validator("username", mode="before")
|
|
@classmethod
|
|
def validate_username(cls, v: str) -> str:
|
|
if isinstance(v, str):
|
|
v = v.strip()
|
|
if not v:
|
|
raise ValueError("Username is required")
|
|
if not re.match(r"^[a-zA-Z0-9_.@-]+$", v):
|
|
raise ValueError("Username may only contain letters, digits, dots, hyphens, underscores, and @")
|
|
return v
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestUsernameInput -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/validation.py tests/test_validation.py
|
|
git commit -m "feat: add UsernameInput validation model"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 4: Add group name validation model
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/validation.py`
|
|
- Modify: `tests/test_validation.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Add to `tests/test_validation.py`:
|
|
|
|
```python
|
|
from porchlight.validation import GroupListInput
|
|
|
|
|
|
class TestGroupListInput:
|
|
def test_valid_single_group(self) -> None:
|
|
g = GroupListInput(groups="users")
|
|
assert g.group_list == ["users"]
|
|
|
|
def test_valid_multiple_groups(self) -> None:
|
|
g = GroupListInput(groups="users, admin, staff")
|
|
assert g.group_list == ["users", "admin", "staff"]
|
|
|
|
def test_empty_string_gives_empty_list(self) -> None:
|
|
g = GroupListInput(groups="")
|
|
assert g.group_list == []
|
|
|
|
def test_whitespace_only_gives_empty_list(self) -> None:
|
|
g = GroupListInput(groups=" , , ")
|
|
assert g.group_list == []
|
|
|
|
def test_valid_with_hyphens_underscores(self) -> None:
|
|
g = GroupListInput(groups="my-group, my_group")
|
|
assert g.group_list == ["my-group", "my_group"]
|
|
|
|
def test_invalid_group_name_spaces(self) -> None:
|
|
with pytest.raises(ValidationError, match="group"):
|
|
GroupListInput(groups="bad group")
|
|
|
|
def test_invalid_group_name_special_chars(self) -> None:
|
|
with pytest.raises(ValidationError, match="group"):
|
|
GroupListInput(groups="admin, bad!group")
|
|
|
|
def test_invalid_group_name_uppercase(self) -> None:
|
|
with pytest.raises(ValidationError, match="group"):
|
|
GroupListInput(groups="Admin")
|
|
|
|
def test_group_name_too_long(self) -> None:
|
|
with pytest.raises(ValidationError, match="group"):
|
|
GroupListInput(groups="a" * 65)
|
|
|
|
def test_deduplicates(self) -> None:
|
|
g = GroupListInput(groups="users, users, admin")
|
|
assert g.group_list == ["users", "admin"]
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestGroupListInput -v`
|
|
Expected: FAIL (GroupListInput doesn't exist)
|
|
|
|
- [ ] **Step 3: Add GroupListInput model**
|
|
|
|
In `src/porchlight/validation.py`, add:
|
|
|
|
```python
|
|
class GroupListInput(BaseModel):
|
|
groups: str = ""
|
|
|
|
@property
|
|
def group_list(self) -> list[str]:
|
|
"""Parse comma-separated groups into a deduplicated list."""
|
|
seen: set[str] = set()
|
|
result: list[str] = []
|
|
for g in (g.strip() for g in self.groups.split(",") if g.strip()):
|
|
if g not in seen:
|
|
seen.add(g)
|
|
result.append(g)
|
|
return result
|
|
|
|
@field_validator("groups", mode="before")
|
|
@classmethod
|
|
def validate_groups(cls, v: str) -> str:
|
|
if isinstance(v, str):
|
|
names = [g.strip() for g in v.split(",") if g.strip()]
|
|
for name in names:
|
|
if not re.match(r"^[a-z0-9_-]{1,64}$", name):
|
|
raise ValueError(
|
|
f"Invalid group name '{name}'. "
|
|
"Groups must be 1-64 lowercase letters, digits, hyphens, or underscores."
|
|
)
|
|
return v
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestGroupListInput -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/validation.py tests/test_validation.py
|
|
git commit -m "feat: add GroupListInput validation model"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 5: Add password validation models with zxcvbn
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/validation.py`
|
|
- Modify: `tests/test_validation.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Add to `tests/test_validation.py`:
|
|
|
|
```python
|
|
from porchlight.validation import PasswordSet, PasswordChange
|
|
|
|
|
|
class TestPasswordSet:
|
|
def test_valid_password(self) -> None:
|
|
p = PasswordSet(password="strongP@ss123", confirm="strongP@ss123")
|
|
assert p.password == "strongP@ss123"
|
|
|
|
def test_mismatch_rejected(self) -> None:
|
|
with pytest.raises(ValidationError, match="match"):
|
|
PasswordSet(password="strongP@ss123", confirm="different")
|
|
|
|
def test_too_short_rejected(self) -> None:
|
|
with pytest.raises(ValidationError):
|
|
PasswordSet(password="short", confirm="short")
|
|
|
|
def test_too_long_rejected(self) -> None:
|
|
long_pw = "a" * 257
|
|
with pytest.raises(ValidationError):
|
|
PasswordSet(password=long_pw, confirm=long_pw)
|
|
|
|
def test_weak_password_rejected(self) -> None:
|
|
with pytest.raises(ValidationError, match="[Ww]eak\\b|[Ss]trength|easily guessed"):
|
|
PasswordSet(password="password", confirm="password")
|
|
|
|
def test_common_password_rejected(self) -> None:
|
|
with pytest.raises(ValidationError, match="[Ww]eak\\b|[Ss]trength|easily guessed"):
|
|
PasswordSet(password="12345678", confirm="12345678")
|
|
|
|
|
|
class TestPasswordChange:
|
|
def test_valid_change(self) -> None:
|
|
p = PasswordChange(
|
|
current_password="oldPassword1",
|
|
password="newStrongP@ss99",
|
|
confirm="newStrongP@ss99",
|
|
)
|
|
assert p.current_password == "oldPassword1"
|
|
assert p.password == "newStrongP@ss99"
|
|
|
|
def test_missing_current_password(self) -> None:
|
|
with pytest.raises(ValidationError, match="current"):
|
|
PasswordChange(
|
|
current_password="",
|
|
password="newStrongP@ss99",
|
|
confirm="newStrongP@ss99",
|
|
)
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestPasswordSet -v`
|
|
Expected: FAIL (PasswordSet doesn't exist)
|
|
|
|
- [ ] **Step 3: Add password models**
|
|
|
|
In `src/porchlight/validation.py`, add at the top:
|
|
|
|
```python
|
|
from zxcvbn import zxcvbn
|
|
```
|
|
|
|
And add the models:
|
|
|
|
```python
|
|
class PasswordSet(BaseModel):
|
|
password: str = Field(min_length=8, max_length=256)
|
|
confirm: str
|
|
|
|
@model_validator(mode="after")
|
|
def validate_password(self) -> "PasswordSet":
|
|
if self.password != self.confirm:
|
|
raise ValueError("Passwords do not match")
|
|
result = zxcvbn(self.password)
|
|
if result["score"] < 2:
|
|
feedback = result.get("feedback", {})
|
|
warning = feedback.get("warning", "")
|
|
suggestions = feedback.get("suggestions", [])
|
|
msg = "Password is too easily guessed."
|
|
if warning:
|
|
msg += f" {warning}."
|
|
if suggestions:
|
|
msg += " " + " ".join(suggestions)
|
|
raise ValueError(msg)
|
|
return self
|
|
|
|
|
|
class PasswordChange(PasswordSet):
|
|
current_password: str
|
|
|
|
@field_validator("current_password", mode="before")
|
|
@classmethod
|
|
def validate_current_password(cls, v: str) -> str:
|
|
if isinstance(v, str) and v.strip() == "":
|
|
raise ValueError("Current password is required")
|
|
return v
|
|
```
|
|
|
|
Note: also add `model_validator` to the pydantic import at the top of the file:
|
|
|
|
```python
|
|
from pydantic import BaseModel, EmailStr, Field, field_validator, model_validator
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestPasswordSet tests/test_validation.py::TestPasswordChange -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/validation.py tests/test_validation.py
|
|
git commit -m "feat: add password validation models with zxcvbn strength check"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 6: Add shared validation error formatting helper
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/validation.py`
|
|
- Modify: `tests/test_validation.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Add to `tests/test_validation.py`:
|
|
|
|
```python
|
|
from porchlight.validation import format_validation_errors
|
|
|
|
|
|
class TestFormatValidationErrors:
|
|
def test_single_error_no_list(self) -> None:
|
|
try:
|
|
ProfileUpdate(email="bad")
|
|
except ValidationError as exc:
|
|
result = format_validation_errors(exc)
|
|
assert '<div role="alert">' in result
|
|
assert "<ul>" not in result # single error, no list
|
|
assert "Email" in result
|
|
|
|
def test_multiple_errors_uses_list(self) -> None:
|
|
try:
|
|
ProfileUpdate(email="bad", phone_number="bad")
|
|
except ValidationError as exc:
|
|
result = format_validation_errors(exc)
|
|
assert '<div role="alert">' in result
|
|
assert "<ul>" in result
|
|
assert "<li>" in result
|
|
assert "Email" in result
|
|
assert "Phone" in result
|
|
|
|
def test_value_error_strips_prefix(self) -> None:
|
|
try:
|
|
ProfileUpdate(picture="ftp://bad.url")
|
|
except ValidationError as exc:
|
|
result = format_validation_errors(exc)
|
|
assert "Picture URL must be a valid HTTP or HTTPS URL" in result
|
|
assert "Value error" not in result
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestFormatValidationErrors -v`
|
|
Expected: FAIL (format_validation_errors doesn't exist)
|
|
|
|
- [ ] **Step 3: Add the helper function**
|
|
|
|
In `src/porchlight/validation.py`, add:
|
|
|
|
```python
|
|
# Field label mapping for user-friendly error messages
|
|
FIELD_LABELS: dict[str, str] = {
|
|
"given_name": "Given name",
|
|
"family_name": "Family name",
|
|
"preferred_username": "Display name",
|
|
"email": "Email",
|
|
"phone_number": "Phone number",
|
|
"picture": "Picture URL",
|
|
"locale": "Locale",
|
|
"username": "Username",
|
|
"groups": "Groups",
|
|
"password": "Password",
|
|
"confirm": "Confirm password",
|
|
"current_password": "Current password",
|
|
}
|
|
|
|
|
|
def format_validation_errors(exc: ValidationError) -> str:
|
|
"""Format Pydantic ValidationError into user-friendly HTML.
|
|
|
|
Returns a single <div role="alert"> with either a plain message (one error)
|
|
or an unordered list (multiple errors).
|
|
"""
|
|
messages: list[str] = []
|
|
for error in exc.errors():
|
|
field = str(error["loc"][-1]) if error["loc"] else "input"
|
|
label = FIELD_LABELS.get(field, field)
|
|
msg = error["msg"]
|
|
if error["type"] == "value_error":
|
|
display_msg = msg.removeprefix("Value error, ")
|
|
else:
|
|
display_msg = f"{label}: {msg}"
|
|
messages.append(display_msg)
|
|
|
|
if len(messages) == 1:
|
|
return f'<div role="alert">{messages[0]}</div>'
|
|
|
|
items = "".join(f"<li>{m}</li>" for m in messages)
|
|
return f'<div role="alert"><ul>{items}</ul></div>'
|
|
```
|
|
|
|
Also add `ValidationError` to the pydantic import:
|
|
|
|
```python
|
|
from pydantic import BaseModel, EmailStr, Field, ValidationError, field_validator, model_validator
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_validation.py::TestFormatValidationErrors -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/validation.py tests/test_validation.py
|
|
git commit -m "feat: add shared validation error formatting helper"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 7: Block inactive users from logging in
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/authn/routes.py:30-58,69-91,104-153`
|
|
- Create: `tests/test_authn_active.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Create `tests/test_authn_active.py`:
|
|
|
|
```python
|
|
from datetime import UTC, datetime
|
|
|
|
import pytest
|
|
from httpx import AsyncClient
|
|
|
|
from porchlight.authn.password import PasswordHasher, PasswordService
|
|
from porchlight.models import PasswordCredential, User
|
|
|
|
from tests.conftest import get_csrf_token
|
|
|
|
|
|
async def _create_inactive_user_with_password(client: AsyncClient) -> None:
|
|
"""Create an inactive user with a password credential."""
|
|
app = client._transport.app
|
|
user_repo = app.state.user_repo
|
|
cred_repo = app.state.credential_repo
|
|
|
|
user = User(
|
|
userid="inactive-01",
|
|
username="inactive",
|
|
active=False,
|
|
created_at=datetime.now(UTC),
|
|
updated_at=datetime.now(UTC),
|
|
)
|
|
await user_repo.create(user)
|
|
|
|
svc = PasswordService(hasher=PasswordHasher(time_cost=1, memory_cost=8192))
|
|
await cred_repo.create_password(
|
|
PasswordCredential(user_id=user.userid, password_hash=svc.hash("password123!Secure"))
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_inactive_user_cannot_login_password(client: AsyncClient) -> None:
|
|
await _create_inactive_user_with_password(client)
|
|
token = await get_csrf_token(client)
|
|
|
|
response = await client.post(
|
|
"/login/password",
|
|
data={"username": "inactive", "password": "password123!Secure"},
|
|
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
|
)
|
|
|
|
assert "Invalid username or password" in response.text
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_inactive_user_cannot_register_magic_link(client: AsyncClient) -> None:
|
|
"""If an inactive user exists, magic link registration should reject them."""
|
|
app = client._transport.app
|
|
user_repo = app.state.user_repo
|
|
magic_link_service = app.state.magic_link_service
|
|
|
|
user = User(
|
|
userid="inactive-02",
|
|
username="deactivated",
|
|
active=False,
|
|
created_at=datetime.now(UTC),
|
|
updated_at=datetime.now(UTC),
|
|
)
|
|
await user_repo.create(user)
|
|
|
|
link = await magic_link_service.create(
|
|
username="deactivated", created_by="admin", note="test"
|
|
)
|
|
|
|
response = await client.get(f"/register/{link.token}", follow_redirects=False)
|
|
|
|
# Should not redirect to credentials setup — should return error
|
|
assert response.status_code == 400 or "deactivated" in response.text.lower() or "Invalid" in response.text
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_authn_active.py -v`
|
|
Expected: FAIL (inactive user can currently log in)
|
|
|
|
- [ ] **Step 3: Add active check to login_password**
|
|
|
|
In `src/porchlight/authn/routes.py`, in the `login_password` function, add after the password verify check (after line 51, before setting session):
|
|
|
|
```python
|
|
if not user.active:
|
|
return HTMLResponse(error_html)
|
|
```
|
|
|
|
- [ ] **Step 4: Add active check to login_webauthn_complete**
|
|
|
|
In `src/porchlight/authn/routes.py`, in the `login_webauthn_complete` function, after the line `user = await user_repo.get_by_userid(userid)` (line 146) and the None check (line 147-148), add:
|
|
|
|
```python
|
|
if not user.active:
|
|
return JSONResponse({"error": "Authentication failed"}, status_code=400)
|
|
```
|
|
|
|
- [ ] **Step 5: Add active check to register_magic_link**
|
|
|
|
In `src/porchlight/authn/routes.py`, in the `register_magic_link` function, after checking for existing user (after line 80, inside the `if existing_user is not None` block), add:
|
|
|
|
```python
|
|
if existing_user is not None:
|
|
if not existing_user.active:
|
|
return HTMLResponse("<p>This account has been deactivated.</p>", status_code=400)
|
|
user = existing_user
|
|
```
|
|
|
|
- [ ] **Step 6: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_authn_active.py -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 7: Run all tests**
|
|
|
|
Run: `uv run pytest -v`
|
|
Expected: All pass
|
|
|
|
- [ ] **Step 8: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/authn/routes.py tests/test_authn_active.py
|
|
git commit -m "fix: block inactive users from all authentication paths"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 8: Add rate limiting middleware
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/app.py:1-153`
|
|
- Modify: `src/porchlight/authn/routes.py:30-58,104-153`
|
|
- Create: `tests/test_rate_limit.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Create `tests/test_rate_limit.py`:
|
|
|
|
```python
|
|
import pytest
|
|
from httpx import AsyncClient
|
|
|
|
from tests.conftest import get_csrf_token
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_password_login_rate_limited(client: AsyncClient) -> None:
|
|
"""After 5 failed attempts, the 6th should be rate-limited."""
|
|
token = await get_csrf_token(client)
|
|
|
|
for _ in range(5):
|
|
await client.post(
|
|
"/login/password",
|
|
data={"username": "nobody", "password": "wrong"},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
|
|
response = await client.post(
|
|
"/login/password",
|
|
data={"username": "nobody", "password": "wrong"},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
|
|
assert response.status_code == 429
|
|
```
|
|
|
|
- [ ] **Step 2: Run test to verify it fails**
|
|
|
|
Run: `uv run pytest tests/test_rate_limit.py -v`
|
|
Expected: FAIL (no rate limiting yet)
|
|
|
|
- [ ] **Step 3: Create rate_limit module**
|
|
|
|
Create `src/porchlight/rate_limit.py`:
|
|
|
|
```python
|
|
from slowapi import Limiter
|
|
from slowapi.util import get_remote_address
|
|
|
|
limiter = Limiter(key_func=get_remote_address)
|
|
```
|
|
|
|
- [ ] **Step 4: Wire rate limiter into app.py**
|
|
|
|
In `src/porchlight/app.py`, add imports:
|
|
|
|
```python
|
|
from slowapi.errors import RateLimitExceeded
|
|
from starlette.responses import HTMLResponse as StarletteHTMLResponse
|
|
|
|
from porchlight.rate_limit import limiter
|
|
```
|
|
|
|
In the `create_app` function, after the middleware section (after line 124), add:
|
|
|
|
```python
|
|
# Rate limiting
|
|
app.state.limiter = limiter
|
|
|
|
@app.exception_handler(RateLimitExceeded)
|
|
async def rate_limit_handler(request: Request, exc: RateLimitExceeded) -> StarletteHTMLResponse:
|
|
return StarletteHTMLResponse(
|
|
'<div role="alert">Too many attempts. Please try again later.</div>',
|
|
status_code=429,
|
|
)
|
|
```
|
|
|
|
- [ ] **Step 5: Add rate limit decorators to auth routes**
|
|
|
|
In `src/porchlight/authn/routes.py`, add import:
|
|
|
|
```python
|
|
from porchlight.rate_limit import limiter
|
|
```
|
|
|
|
Add `@limiter.limit("5/minute")` decorator to `login_password` (between `@router.post(...)` and `async def`):
|
|
|
|
```python
|
|
@router.post("/login/password", response_class=HTMLResponse)
|
|
@limiter.limit("5/minute")
|
|
async def login_password(
|
|
request: Request,
|
|
username: str = Form(),
|
|
password: str = Form(),
|
|
) -> Response:
|
|
...
|
|
```
|
|
|
|
Add `@limiter.limit("10/minute")` decorator to `login_webauthn_complete`:
|
|
|
|
```python
|
|
@router.post("/login/webauthn/complete")
|
|
@limiter.limit("10/minute")
|
|
async def login_webauthn_complete(request: Request) -> Response:
|
|
...
|
|
```
|
|
|
|
- [ ] **Step 6: Run test to verify it passes**
|
|
|
|
Run: `uv run pytest tests/test_rate_limit.py -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 7: Run all tests**
|
|
|
|
Run: `uv run pytest -v`
|
|
Expected: All pass
|
|
|
|
- [ ] **Step 8: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/rate_limit.py src/porchlight/app.py src/porchlight/authn/routes.py tests/test_rate_limit.py
|
|
git commit -m "feat: add rate limiting middleware for authentication endpoints"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 9: Deduplicate profile validation and use shared error helper
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/manage/routes.py:200-262`
|
|
- Modify: `src/porchlight/admin/routes.py:127-191`
|
|
- Modify: `tests/test_manage_profile.py` (verify existing tests still pass)
|
|
|
|
- [ ] **Step 1: Update manage/routes.py to use shared helper**
|
|
|
|
In `src/porchlight/manage/routes.py`, update the import:
|
|
|
|
```python
|
|
from porchlight.validation import ProfileUpdate, format_validation_errors
|
|
```
|
|
|
|
Replace the `except ValidationError` block in `update_profile` (lines 227-244) with:
|
|
|
|
```python
|
|
except ValidationError as exc:
|
|
return HTMLResponse(format_validation_errors(exc))
|
|
```
|
|
|
|
Remove the now-unused `ValidationError` import from pydantic (it's re-exported from validation.py), or keep it — the import from validation.py is cleaner. Actually, `ValidationError` is still needed from pydantic for the `except` clause. The import is fine.
|
|
|
|
- [ ] **Step 2: Update admin/routes.py to use shared helper**
|
|
|
|
In `src/porchlight/admin/routes.py`, update the import:
|
|
|
|
```python
|
|
from porchlight.validation import ProfileUpdate, format_validation_errors
|
|
```
|
|
|
|
Replace the `except ValidationError` block in `update_user_profile` (lines 157-172) with:
|
|
|
|
```python
|
|
except ValidationError as exc:
|
|
return HTMLResponse(format_validation_errors(exc))
|
|
```
|
|
|
|
- [ ] **Step 3: Run existing tests**
|
|
|
|
Run: `uv run pytest tests/test_manage_profile.py -v`
|
|
Expected: All pass (behavior is backward-compatible, though now multiple errors may be shown)
|
|
|
|
- [ ] **Step 4: Run all tests**
|
|
|
|
Run: `uv run pytest -v`
|
|
Expected: All pass
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/manage/routes.py src/porchlight/admin/routes.py
|
|
git commit -m "refactor: deduplicate profile validation using shared error formatter"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 10: Wire username validation into admin invite route
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/admin/routes.py:99-123`
|
|
- Create: `tests/test_admin_invite_validation.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Create `tests/test_admin_invite_validation.py`:
|
|
|
|
```python
|
|
from datetime import UTC, datetime
|
|
|
|
import pytest
|
|
from httpx import AsyncClient
|
|
|
|
from porchlight.authn.password import PasswordHasher, PasswordService
|
|
from porchlight.models import PasswordCredential, User
|
|
|
|
from tests.conftest import get_csrf_token
|
|
|
|
|
|
async def _login_admin(client: AsyncClient) -> str:
|
|
"""Create and login as admin user, return CSRF token."""
|
|
app = client._transport.app
|
|
user_repo = app.state.user_repo
|
|
cred_repo = app.state.credential_repo
|
|
|
|
user = User(
|
|
userid="admin-01",
|
|
username="admin",
|
|
groups=["admin", "users"],
|
|
created_at=datetime.now(UTC),
|
|
updated_at=datetime.now(UTC),
|
|
)
|
|
await user_repo.create(user)
|
|
|
|
svc = PasswordService(hasher=PasswordHasher(time_cost=1, memory_cost=8192))
|
|
await cred_repo.create_password(
|
|
PasswordCredential(user_id=user.userid, password_hash=svc.hash("AdminPass123!"))
|
|
)
|
|
|
|
token = await get_csrf_token(client)
|
|
await client.post(
|
|
"/login/password",
|
|
data={"username": "admin", "password": "AdminPass123!"},
|
|
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
|
)
|
|
return token
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_invite_valid_username(client: AsyncClient) -> None:
|
|
token = await _login_admin(client)
|
|
response = await client.post(
|
|
"/admin/invite",
|
|
data={"username": "newuser@example.com"},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert response.status_code == 200
|
|
assert "Invite created" in response.text
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_invite_empty_username_rejected(client: AsyncClient) -> None:
|
|
token = await _login_admin(client)
|
|
response = await client.post(
|
|
"/admin/invite",
|
|
data={"username": ""},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "alert" in response.text
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_invite_invalid_username_rejected(client: AsyncClient) -> None:
|
|
token = await _login_admin(client)
|
|
response = await client.post(
|
|
"/admin/invite",
|
|
data={"username": "bad user<script>"},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "alert" in response.text
|
|
assert "letters" in response.text.lower() or "Username" in response.text
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_admin_invite_validation.py -v`
|
|
Expected: The invalid username test fails (currently any non-empty string is accepted)
|
|
|
|
- [ ] **Step 3: Wire UsernameInput into create_invite route**
|
|
|
|
In `src/porchlight/admin/routes.py`, add import:
|
|
|
|
```python
|
|
from porchlight.validation import UsernameInput, format_validation_errors
|
|
```
|
|
|
|
Update `create_invite` to use the model:
|
|
|
|
```python
|
|
@router.post("/invite", response_class=HTMLResponse)
|
|
async def create_invite(
|
|
request: Request,
|
|
username: str = Form(),
|
|
) -> Response:
|
|
session_user = get_session_user(request)
|
|
if session_user is None:
|
|
return RedirectResponse("/login", status_code=303)
|
|
|
|
admin = await _get_admin_user(request)
|
|
if admin is None:
|
|
return HTMLResponse("Forbidden", status_code=403)
|
|
|
|
try:
|
|
validated = UsernameInput(username=username)
|
|
except ValidationError as exc:
|
|
return HTMLResponse(format_validation_errors(exc))
|
|
|
|
magic_link_service = request.app.state.magic_link_service
|
|
settings = request.app.state.settings
|
|
link = await magic_link_service.create(
|
|
username=validated.username, created_by=admin.username, note="admin invite"
|
|
)
|
|
url = f"{settings.issuer}/register/{link.token}"
|
|
|
|
return HTMLResponse(
|
|
f'<div role="status">Invite created for <strong>{validated.username}</strong>:</div>'
|
|
f'<div class="invite-url">{url}</div>'
|
|
)
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_admin_invite_validation.py -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/admin/routes.py tests/test_admin_invite_validation.py
|
|
git commit -m "feat: validate username format in admin invite route"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 11: Wire group validation into admin groups route
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/admin/routes.py:195-216`
|
|
- Create: `tests/test_admin_groups_validation.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Create `tests/test_admin_groups_validation.py`:
|
|
|
|
```python
|
|
from datetime import UTC, datetime
|
|
|
|
import pytest
|
|
from httpx import AsyncClient
|
|
|
|
from porchlight.authn.password import PasswordHasher, PasswordService
|
|
from porchlight.models import PasswordCredential, User
|
|
|
|
from tests.conftest import get_csrf_token
|
|
|
|
|
|
async def _setup_admin_and_target(client: AsyncClient) -> tuple[str, str]:
|
|
"""Create admin + target user, login as admin, return (token, target_userid)."""
|
|
app = client._transport.app
|
|
user_repo = app.state.user_repo
|
|
cred_repo = app.state.credential_repo
|
|
|
|
admin = User(
|
|
userid="admin-g01",
|
|
username="admin_g",
|
|
groups=["admin", "users"],
|
|
created_at=datetime.now(UTC),
|
|
updated_at=datetime.now(UTC),
|
|
)
|
|
await user_repo.create(admin)
|
|
|
|
target = User(
|
|
userid="target-g01",
|
|
username="target_g",
|
|
groups=["users"],
|
|
created_at=datetime.now(UTC),
|
|
updated_at=datetime.now(UTC),
|
|
)
|
|
await user_repo.create(target)
|
|
|
|
svc = PasswordService(hasher=PasswordHasher(time_cost=1, memory_cost=8192))
|
|
await cred_repo.create_password(
|
|
PasswordCredential(user_id=admin.userid, password_hash=svc.hash("AdminPass123!"))
|
|
)
|
|
|
|
token = await get_csrf_token(client)
|
|
await client.post(
|
|
"/login/password",
|
|
data={"username": "admin_g", "password": "AdminPass123!"},
|
|
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
|
)
|
|
return token, target.userid
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_valid_groups(client: AsyncClient) -> None:
|
|
token, userid = await _setup_admin_and_target(client)
|
|
response = await client.post(
|
|
f"/admin/users/{userid}/groups",
|
|
data={"groups": "users, staff"},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "Groups updated" in response.text
|
|
|
|
app = client._transport.app
|
|
user = await app.state.user_repo.get_by_userid(userid)
|
|
assert user.groups == ["users", "staff"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_invalid_group_name_rejected(client: AsyncClient) -> None:
|
|
token, userid = await _setup_admin_and_target(client)
|
|
response = await client.post(
|
|
f"/admin/users/{userid}/groups",
|
|
data={"groups": "users, Bad Group!"},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "alert" in response.text
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_empty_groups_clears(client: AsyncClient) -> None:
|
|
token, userid = await _setup_admin_and_target(client)
|
|
response = await client.post(
|
|
f"/admin/users/{userid}/groups",
|
|
data={"groups": ""},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "Groups updated" in response.text
|
|
|
|
app = client._transport.app
|
|
user = await app.state.user_repo.get_by_userid(userid)
|
|
assert user.groups == []
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_admin_groups_validation.py -v`
|
|
Expected: The invalid group name test fails
|
|
|
|
- [ ] **Step 3: Wire GroupListInput into update_user_groups**
|
|
|
|
In `src/porchlight/admin/routes.py`, update imports to include `GroupListInput`:
|
|
|
|
```python
|
|
from porchlight.validation import GroupListInput, ProfileUpdate, UsernameInput, format_validation_errors
|
|
```
|
|
|
|
Update `update_user_groups`:
|
|
|
|
```python
|
|
@router.post("/users/{userid}/groups", response_class=HTMLResponse)
|
|
async def update_user_groups(
|
|
request: Request,
|
|
userid: str,
|
|
groups: str = Form(""),
|
|
) -> Response:
|
|
session_user = get_session_user(request)
|
|
if session_user is None:
|
|
return RedirectResponse("/login", status_code=303)
|
|
admin = await _get_admin_user(request)
|
|
if admin is None:
|
|
return HTMLResponse("Forbidden", status_code=403)
|
|
|
|
try:
|
|
validated = GroupListInput(groups=groups)
|
|
except ValidationError as exc:
|
|
return HTMLResponse(format_validation_errors(exc))
|
|
|
|
user_repo = request.app.state.user_repo
|
|
user = await user_repo.get_by_userid(userid)
|
|
if user is None:
|
|
return HTMLResponse("User not found", status_code=404)
|
|
|
|
updated = user.model_copy(update={"groups": validated.group_list})
|
|
await user_repo.update(updated)
|
|
return HTMLResponse('<div role="status">Groups updated</div>')
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_admin_groups_validation.py -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/admin/routes.py tests/test_admin_groups_validation.py
|
|
git commit -m "feat: validate group names in admin groups route"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 12: Require current password when changing password
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/manage/routes.py:53-81`
|
|
- Modify: `src/porchlight/templates/manage/credentials.html:41-52`
|
|
- Modify: `tests/test_manage_profile.py` or create `tests/test_password_change.py`
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Create `tests/test_password_change.py`:
|
|
|
|
```python
|
|
from datetime import UTC, datetime
|
|
|
|
import pytest
|
|
from httpx import AsyncClient
|
|
|
|
from porchlight.authn.password import PasswordHasher, PasswordService
|
|
from porchlight.models import PasswordCredential, User
|
|
|
|
from tests.conftest import get_csrf_token
|
|
|
|
|
|
async def _login_user_with_password(client: AsyncClient) -> str:
|
|
"""Create user with password, login, return CSRF token."""
|
|
app = client._transport.app
|
|
user_repo = app.state.user_repo
|
|
cred_repo = app.state.credential_repo
|
|
|
|
user = User(
|
|
userid="pw-user-01",
|
|
username="pwuser",
|
|
created_at=datetime.now(UTC),
|
|
updated_at=datetime.now(UTC),
|
|
)
|
|
await user_repo.create(user)
|
|
|
|
svc = PasswordService(hasher=PasswordHasher(time_cost=1, memory_cost=8192))
|
|
await cred_repo.create_password(
|
|
PasswordCredential(user_id=user.userid, password_hash=svc.hash("OldPass123!ok"))
|
|
)
|
|
|
|
token = await get_csrf_token(client)
|
|
await client.post(
|
|
"/login/password",
|
|
data={"username": "pwuser", "password": "OldPass123!ok"},
|
|
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
|
)
|
|
return token
|
|
|
|
|
|
async def _login_user_without_password(client: AsyncClient) -> str:
|
|
"""Create user without password, simulate session login, return CSRF token."""
|
|
app = client._transport.app
|
|
user_repo = app.state.user_repo
|
|
|
|
user = User(
|
|
userid="pw-user-02",
|
|
username="newuser",
|
|
created_at=datetime.now(UTC),
|
|
updated_at=datetime.now(UTC),
|
|
)
|
|
await user_repo.create(user)
|
|
|
|
# Simulate session login (as if registered via magic link)
|
|
token = await get_csrf_token(client)
|
|
# We need to set session directly — do a magic link registration
|
|
magic_link_service = app.state.magic_link_service
|
|
link = await magic_link_service.create(username="newuser", created_by="admin", note="test")
|
|
await client.get(f"/register/{link.token}", follow_redirects=False)
|
|
# Re-fetch CSRF token after session change
|
|
token = await get_csrf_token(client)
|
|
return token
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_change_password_requires_current(client: AsyncClient) -> None:
|
|
token = await _login_user_with_password(client)
|
|
|
|
response = await client.post(
|
|
"/manage/credentials/password",
|
|
data={
|
|
"password": "NewStrong!Pass99",
|
|
"confirm": "NewStrong!Pass99",
|
|
},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "alert" in response.text
|
|
assert "urrent password" in response.text
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_change_password_wrong_current_rejected(client: AsyncClient) -> None:
|
|
token = await _login_user_with_password(client)
|
|
|
|
response = await client.post(
|
|
"/manage/credentials/password",
|
|
data={
|
|
"current_password": "WrongPassword",
|
|
"password": "NewStrong!Pass99",
|
|
"confirm": "NewStrong!Pass99",
|
|
},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "alert" in response.text
|
|
assert "incorrect" in response.text.lower() or "Invalid" in response.text or "current" in response.text.lower()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_change_password_correct_current_succeeds(client: AsyncClient) -> None:
|
|
token = await _login_user_with_password(client)
|
|
|
|
response = await client.post(
|
|
"/manage/credentials/password",
|
|
data={
|
|
"current_password": "OldPass123!ok",
|
|
"password": "NewStrong!Pass99",
|
|
"confirm": "NewStrong!Pass99",
|
|
},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "updated" in response.text.lower() or "status" in response.text
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_set_password_no_current_needed(client: AsyncClient) -> None:
|
|
"""First-time password setup should not require current password."""
|
|
token = await _login_user_without_password(client)
|
|
|
|
response = await client.post(
|
|
"/manage/credentials/password",
|
|
data={
|
|
"password": "FirstPass!Strong99",
|
|
"confirm": "FirstPass!Strong99",
|
|
},
|
|
headers={"X-CSRF-Token": token},
|
|
)
|
|
assert "updated" in response.text.lower() or "status" in response.text
|
|
```
|
|
|
|
- [ ] **Step 2: Run tests to verify they fail**
|
|
|
|
Run: `uv run pytest tests/test_password_change.py -v`
|
|
Expected: FAIL (current password not required yet)
|
|
|
|
- [ ] **Step 3: Update set_password route**
|
|
|
|
In `src/porchlight/manage/routes.py`, update imports:
|
|
|
|
```python
|
|
from porchlight.validation import PasswordChange, PasswordSet, ProfileUpdate, format_validation_errors
|
|
```
|
|
|
|
Replace `set_password`:
|
|
|
|
```python
|
|
@router.post("/credentials/password", response_class=HTMLResponse)
|
|
async def set_password(
|
|
request: Request,
|
|
password: str = Form(),
|
|
confirm: str = Form(),
|
|
current_password: str = Form(""),
|
|
) -> Response:
|
|
session_user = get_session_user(request)
|
|
if session_user is None:
|
|
return RedirectResponse("/login", status_code=303)
|
|
|
|
userid, _username = session_user
|
|
cred_repo = request.app.state.credential_repo
|
|
password_service = request.app.state.password_service
|
|
|
|
existing = await cred_repo.get_password_by_user(userid)
|
|
has_password = existing is not None
|
|
|
|
# Validate input
|
|
try:
|
|
if has_password:
|
|
validated = PasswordChange(
|
|
current_password=current_password,
|
|
password=password,
|
|
confirm=confirm,
|
|
)
|
|
else:
|
|
validated = PasswordSet(password=password, confirm=confirm)
|
|
except ValidationError as exc:
|
|
return HTMLResponse(format_validation_errors(exc))
|
|
|
|
# Verify current password if changing
|
|
if has_password:
|
|
if not password_service.verify(existing.password_hash, validated.current_password):
|
|
return HTMLResponse('<div role="alert">Current password is incorrect</div>')
|
|
|
|
# Store new password
|
|
password_hash = password_service.hash(validated.password)
|
|
|
|
if existing is not None:
|
|
await cred_repo.delete_password(userid)
|
|
|
|
await cred_repo.create_password(PasswordCredential(user_id=userid, password_hash=password_hash))
|
|
|
|
return HTMLResponse('<div role="status">Password updated successfully</div>')
|
|
```
|
|
|
|
- [ ] **Step 4: Update credentials template**
|
|
|
|
In `src/porchlight/templates/manage/credentials.html`, update the password form section (lines 41-52) to conditionally show a current password field:
|
|
|
|
```html
|
|
<form hx-post="/manage/credentials/password" hx-target="#password-section" hx-swap="innerHTML">
|
|
<input type="hidden" name="csrf_token" value="{{ csrf_token_processor(request) }}">
|
|
{% if has_password %}
|
|
<div>
|
|
<label for="current_password">Current password</label>
|
|
<input type="password" id="current_password" name="current_password" required autocomplete="current-password">
|
|
</div>
|
|
{% endif %}
|
|
<div>
|
|
<label for="password">{{ "New password" if has_password else "Set password" }}</label>
|
|
<input type="password" id="password" name="password" required minlength="8" maxlength="256" autocomplete="new-password">
|
|
</div>
|
|
<div>
|
|
<label for="confirm">Confirm password</label>
|
|
<input type="password" id="confirm" name="confirm" required minlength="8" maxlength="256" autocomplete="new-password">
|
|
</div>
|
|
<button type="submit">{{ "Change password" if has_password else "Set password" }}</button>
|
|
</form>
|
|
```
|
|
|
|
Note: also adds `maxlength="256"` to the password fields.
|
|
|
|
- [ ] **Step 5: Run tests to verify they pass**
|
|
|
|
Run: `uv run pytest tests/test_password_change.py -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 6: Run all tests**
|
|
|
|
Run: `uv run pytest -v`
|
|
Expected: All pass
|
|
|
|
- [ ] **Step 7: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/manage/routes.py src/porchlight/templates/manage/credentials.html tests/test_password_change.py
|
|
git commit -m "feat: require current password when changing password, add zxcvbn strength check"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 13: Add CSRF tokens to admin form templates
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/templates/admin/user_detail.html:13,50`
|
|
- Modify: `src/porchlight/templates/admin/users.html:10`
|
|
|
|
- [ ] **Step 1: Add CSRF token to admin profile form**
|
|
|
|
In `src/porchlight/templates/admin/user_detail.html`, add after line 13 (after the `<form>` opening tag):
|
|
|
|
```html
|
|
<input type="hidden" name="csrf_token" value="{{ csrf_token_processor(request) }}">
|
|
```
|
|
|
|
- [ ] **Step 2: Add CSRF token to admin groups form**
|
|
|
|
In `src/porchlight/templates/admin/user_detail.html`, add after line 50 (after the groups `<form>` opening tag):
|
|
|
|
```html
|
|
<input type="hidden" name="csrf_token" value="{{ csrf_token_processor(request) }}">
|
|
```
|
|
|
|
- [ ] **Step 3: Add CSRF token to admin invite form**
|
|
|
|
In `src/porchlight/templates/admin/users.html`, add after line 10 (after the invite `<form>` opening tag):
|
|
|
|
```html
|
|
<input type="hidden" name="csrf_token" value="{{ csrf_token_processor(request) }}">
|
|
```
|
|
|
|
- [ ] **Step 4: Run all tests**
|
|
|
|
Run: `uv run pytest -v`
|
|
Expected: All pass
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/templates/admin/user_detail.html src/porchlight/templates/admin/users.html
|
|
git commit -m "fix: add explicit CSRF tokens to admin form templates"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 14: Add username validation HTML attributes to invite form
|
|
|
|
**Files:**
|
|
- Modify: `src/porchlight/templates/admin/users.html:12`
|
|
|
|
- [ ] **Step 1: Update invite username input**
|
|
|
|
In `src/porchlight/templates/admin/users.html`, update the username input (line 12) to add validation hints:
|
|
|
|
```html
|
|
<input type="text" name="username" placeholder="Username or email for new invite" required
|
|
maxlength="255" pattern="[a-zA-Z0-9_.@-]+"
|
|
title="Letters, digits, dots, hyphens, underscores, and @">
|
|
```
|
|
|
|
- [ ] **Step 2: Commit**
|
|
|
|
```bash
|
|
git add src/porchlight/templates/admin/users.html
|
|
git commit -m "style: add HTML5 validation hints to admin invite form"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 15: Run full check suite
|
|
|
|
**Files:** None (verification only)
|
|
|
|
- [ ] **Step 1: Run formatter**
|
|
|
|
Run: `make reformat`
|
|
|
|
- [ ] **Step 2: Run linter**
|
|
|
|
Run: `make lint`
|
|
|
|
- [ ] **Step 3: Run type checker**
|
|
|
|
Run: `make typecheck`
|
|
|
|
- [ ] **Step 4: Run all tests**
|
|
|
|
Run: `make test`
|
|
|
|
- [ ] **Step 5: Fix any issues**
|
|
|
|
If any check fails, fix the issue and re-run.
|
|
|
|
- [ ] **Step 6: Final commit if any formatting/lint changes**
|
|
|
|
```bash
|
|
git add -A
|
|
git commit -m "style: apply formatting and lint fixes"
|
|
```
|
|
|
|
---
|
|
|
|
## Summary of Changes
|
|
|
|
| Task | File(s) | What |
|
|
|------|---------|------|
|
|
| 1 | `pyproject.toml` | Add slowapi + zxcvbn deps |
|
|
| 2 | `validation.py`, tests | Locale BCP 47 validation |
|
|
| 3 | `validation.py`, tests | UsernameInput model |
|
|
| 4 | `validation.py`, tests | GroupListInput model |
|
|
| 5 | `validation.py`, tests | PasswordSet + PasswordChange with zxcvbn |
|
|
| 6 | `validation.py`, tests | Shared `format_validation_errors` helper |
|
|
| 7 | `authn/routes.py`, tests | Block inactive users from all auth paths |
|
|
| 8 | `rate_limit.py`, `app.py`, `authn/routes.py`, tests | Rate limiting on login endpoints |
|
|
| 9 | `manage/routes.py`, `admin/routes.py` | Deduplicate profile validation |
|
|
| 10 | `admin/routes.py`, tests | Username validation in invite |
|
|
| 11 | `admin/routes.py`, tests | Group name validation |
|
|
| 12 | `manage/routes.py`, template, tests | Require current password + zxcvbn UI |
|
|
| 13 | Admin templates | CSRF tokens in forms |
|
|
| 14 | Admin template | HTML5 validation hints |
|
|
| 15 | — | Full check suite |
|