The token endpoint wrapped parse_request in try/except but
called process_request and do_response unguarded, so a parseable-but-invalid request (e.g. a refresh_token grant missing client_id, or an
unknown token) made idpyoidc raise and surfaced as a 500. Wrap both so failures return a clean 400 invalid_request and log the traceback
server-side. Adds a regression test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The CSRF middleware read the request body via request.form() to extract
the csrf_token form field, then handed the already-consumed ASGI receive
to the downstream app. The endpoint's own request.form()/body() then
blocked indefinitely waiting for body bytes that would never arrive,
hanging the request until the client disconnected (ClientDisconnect).
Only native form POSTs hit this path: htmx requests carry the token in
the X-CSRF-Token header and skip the body read. The OIDC consent form is
a plain form with the token in the body, so authorization consent hung.
Buffer the body when falling back to the form token and replay it to the
downstream app via a wrapped receive. Header-token requests are
unchanged. Adds a regression test where the endpoint reads the body after
body-token CSRF validation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
idpyoidc already gates refresh-token issuance on the offline_access scope
(verified by test), but the refresh-token grant was configured to also mint
fresh ID tokens. Drop id_token from the refresh_token grant's supports_minting
so refreshing yields only access (and a rotated refresh) token; ID tokens come
from authentication. Refresh-token rotation is retained.
Refs: porchlight-553
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No security headers were set. Add SecurityHeadersMiddleware applying
Content-Security-Policy (configurable), X-Content-Type-Options: nosniff,
X-Frame-Options: DENY, Referrer-Policy, and Strict-Transport-Security on
HTTPS deployments. Verified HTMX/WebAuthn/forms still work under the CSP.
Refs: porchlight-1ph
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Private JWK files were written under the default umask (observed 0664 — group
and world readable). Create the key directory 0700, chmod private key files
(private_jwks.json, token_jwks.json) to 0600 after they are written, and
refuse to start if a pre-existing private key is group/world accessible.
Tests now use an isolated per-test key directory.
Refs: porchlight-91i
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The session cookie relied on Starlette's default max_age (two weeks), which is
easy to miss and longer than an OP session should live. Add a session_max_age
setting (default 8 hours) and pass it to SessionMiddleware.
Refs: porchlight-1lg
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
session_secret defaulted to a random per-process value, which silently
invalidates all sessions on restart and rotates the management client secret.
Add _resolve_session_secret(): use the configured secret; allow a generated
one only in debug or for a localhost issuer; otherwise fail startup. The
management client secret is now tied to the resolved session secret.
Refs: porchlight-wvx
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two WebAuthn client/route hardening fixes:
- GET /login/webauthn/begin wrote a challenge to the session on a safe method
(login-flow DoS / CSRF surface). Make it POST with CSRF, matching the
registration-begin endpoint; update webauthn.js and tests accordingly.
- webauthn.js rendered dynamic error text (err.message, server error fields)
via innerHTML — an XSS-prone sink. Add showAlert() that sets textContent;
route all dynamic error messages through it. The trusted server-rendered
credentials partial is still injected as markup.
Refs: porchlight-cog, porchlight-t7y
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign counters were stored but never checked, so a cloned authenticator or a
replayed assertion with an equal/lower counter was accepted. Reject the
authentication when the presented counter does not exceed the stored one,
while allowing counter-less/synced passkeys that always report 0.
Refs: porchlight-3cr
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The webauthn_credentials primary key is (user_id, credential_id), which does
not stop the same credential_id from existing under two users. Usernameless
authentication looks up the credential by id alone, so a duplicate could
resolve to the wrong account. Add a unique index on credential_id (migration
003); duplicate registration now raises DuplicateError.
Refs: porchlight-as2
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
User verification was hardcoded to PREFERRED. Add a webauthn_user_verification
setting (default "preferred") wired into WebAuthnService for both registration
and authentication, so identity-provider deployments can require it.
Refs: porchlight-is8
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GET /register/{token} consumed the magic-link token and created a session, so
a side-effecting state change happened on a safe method — link prefetchers,
email scanners, or a cross-site GET could trigger account setup/login.
Split the flow: GET validates the token (without consuming) and renders a
confirmation form; POST /register/{token} consumes the token, runs the
existing checks, and establishes the session. The POST carries a CSRF token
and the session is reset on login as for other auth paths.
Refs: porchlight-9k0
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The limiter keyed solely on the direct peer address, so behind a reverse
proxy every request shares the proxy's IP (collapsing all users into one
bucket). Blindly trusting X-Forwarded-For would instead let clients spoof it.
Add a trusted_proxy_count setting (default 0). When > 0, the client IP is read
from X-Forwarded-For counting that many hops from the right (ProxyFix-style);
when 0, the header is ignored and the peer address is used.
Refs: porchlight-8qj
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Admins could remove the admin group from, deactivate, or delete the last
active admin, locking the system out of all administration. Add a
count_active_admins() repo method and a _is_last_active_admin() guard, and
block all three operations when they would leave zero active admins.
Refs: porchlight-yq7
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Admin credential deletion removed password/WebAuthn credentials with no
last-credential check, so an admin could delete a user's only credential and
lock them out. Use the atomic delete_*_if_not_last repo methods; on refusal
re-render the credentials section unchanged with an explanatory alert.
Refs: porchlight-lg7
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The self-service credential delete handlers counted credentials and then
deleted in separate steps, so concurrent deletes could each see >1 and both
proceed, removing the user's last credential and locking them out.
Add atomic delete_password_if_not_last / delete_webauthn_if_not_last repo
methods (count + delete in one conditional statement) and use them in the
manage delete handlers. Removes the now-unused _count_credentials helper.
Refs: porchlight-2nv
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both password and WebAuthn login wrote the authenticated identity onto the
existing pre-auth session, so a fixed/planted session could be elevated to an
authenticated one. Add _establish_authenticated_session() which clears the
session (preserving only a pending OIDC authorization request) before setting
the identity, used by both login paths.
Tests that reused a pre-login CSRF token now re-fetch it after login, matching
real client behavior.
Refs: porchlight-vxr
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
OIDC error responses interpolated parse-error/exception and error_description
text straight into HTML. idpyoidc currently emits canned messages, but this is
the same reflected-XSS class as the validation-error fix; relying on upstream
not to echo input is fragile.
Add a shared _error_page() helper that HTML-escapes the message and route all
six dynamic error responses through it.
Refs: porchlight-8iw
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
idpyoidc advertised PKCE support but did not actually verify the code
challenge at the token endpoint, so a sent code_challenge provided no
protection. Enable the PKCE add-on restricted to S256.
Configured as non-essential: relying parties that do not send a
code_challenge continue to work (no breaking change), but any RP that uses
PKCE must use S256, and the code_verifier is verified at token exchange.
Flip essential=True (or per-client pkce_essential) to require PKCE once all
clients have migrated.
Refs: porchlight-s48
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A registration/re-invite link auto-established a session for any existing
active user, so re-inviting a fully set-up user acted as a passwordless
login. Invite links are for account setup only.
After consuming the token, refuse to establish a session when the target
account already has a password or WebAuthn credential. Credential-less
accounts (e.g. freshly created by initial-admin) can still complete setup.
Account recovery for set-up accounts must use a separate, authenticated flow.
Refs: porchlight-a3a
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Validation and marking-used were two separate steps, so two concurrent
requests for the same registration token could both pass validation before
either marked it used — a replay window.
Add an atomic consume() at the repository (conditional UPDATE ... WHERE
used = 0 AND not expired, gated on rowcount) and service layers, and switch
the /register handler to consume() instead of validate()+mark_used().
Refs: porchlight-ur7
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Magic-link tokens were persisted in plaintext, so a database read disclosed
usable login/invite tokens. The service now hashes tokens (HMAC-SHA256 when a
pepper is configured, else SHA-256 of the high-entropy token) and persists
only the hash; the raw token is exposed solely in the registration URL and is
re-attached to objects returned to callers.
Refs: porchlight-42h
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The /consent POST handler trusted the scope values submitted in the form,
so a forged consent submission could approve (and persist consent for)
scopes that were never part of the originating authorization request —
a scope-escalation vector.
Intersect the submitted scopes with the originally requested set stored in
the session before saving consent and completing the flow.
Refs: porchlight-a03
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
format_validation_errors interpolated Pydantic error messages directly into
HTML. Some messages echo user input (e.g. "Invalid group name '<name>'"), so
a crafted group name was reflected as raw HTML — a stored/reflected XSS.
HTML-escape each formatted message before interpolation.
Refs: porchlight-due
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The admin nav had no way back to the account management area. Add a
Profile link to /manage/profile, mirroring the Admin link already present
in the manage nav.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 6-column user table could not shrink below its content width at the
40rem main column, so the Created column was clipped past the card border.
Tighten cell padding to fit all columns, and add overflow-x: auto on the
table container as a safety net for longer content.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The full Playwright suite authenticates ~100 times in a few minutes, far
over the login endpoint's 5/minute limit, so most specs failed at the
beforeEach login with 429s.
Add an OIDC_OP_RATE_LIMIT_ENABLED setting (default True) wired to the
slowapi limiter's enabled flag, and set it to false in tests/e2e/run.sh.
Production behavior is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The password setup/change form used hx-target="#password-section" with
hx-swap="innerHTML", but that div wraps the form itself. On a validation
error the route returns only an alert div, so the swap replaced the entire
form — the password inputs disappeared. Most visible during registration's
"set password" step.
Retarget the form to a dedicated #password-error div outside the form
(mirrors the working login form's #login-error pattern), so the form and
its inputs survive errors while messages still render inside #password-section.
Also fix pre-existing broken e2e tests: they omitted the required
current_password fill and used passwords below the zxcvbn strength
threshold (score 1 < MIN_PASSWORD_STRENGTH=2).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Use Annotated[str, Form()] for FastAPI dependencies (FAST002)
- Add missing type annotations across src/ and tests/ (ANN001/003/201/202)
- Reduce function arguments via request.form() reads (PLR0913)
- Combine return paths to reduce return statements (PLR0911)
- Use anyio.Path for async-safe filesystem operations (ASYNC240)
- Extract constants, helpers, and dict comprehensions for clarity
- Move inline imports to top-level (PLC0415)
- Use raw strings for regex match patterns (RUF043)
- Fix redundant get_session_user call in delete_user (not-iterable)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use isinstance check instead of bool flag to help ty resolve
the current_password attribute on the validated model.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use PasswordChange model (requires current password) for users with
existing passwords and PasswordSet for first-time setup. Add zxcvbn
strength validation and current password field to credentials template.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace manual validation error formatting with shared helper in both
admin and manage profile routes. Add UsernameInput validation to invite
route and GroupListInput validation to groups route.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add hidden CSRF token inputs to admin profile, groups, and invite
forms. Add maxlength, pattern, and title attributes to invite input.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add slowapi-based rate limiting: 5/min on password login, 10/min on
WebAuthn login. Includes shared rate limiter reset fixture for tests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add active-user checks to password login, WebAuthn login, and magic
link registration to prevent deactivated accounts from authenticating.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add BCP 47 locale validator to ProfileUpdate, UsernameInput model,
GroupListInput model, PasswordSet/PasswordChange with zxcvbn strength
checking, and shared format_validation_errors HTML helper.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 7 new e2e tests verifying profile form validation in both manage
and admin UIs: invalid phone number, phone normalization, E.164 hint
attributes, and admin-side email/phone/picture URL validation errors.
Fix 3 pre-existing test failures:
- Replace invalid seeded phone number (+1234567890) with valid E.164
(+12025551234) that was causing profile update tests to fail
- Update email validation error assertion to match actual pydantic
message (value_error type uses raw message, not label-prefixed)
