# SESSION-0026.1 — AuthN/AuthZ security audit subsession (rfc-app v0.24.0)

Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
Date: 2026-05-28T13-46 → 2026-05-28T13-50 (America/Los_Angeles)
Role: subsession 0026.1 of OHM security-audit driver session 0026.
Mode: READ-ONLY. No edits, no deploys, no mutations. Only file written is this transcript.

## Goal

Audit the authentication / authorization surface of the rfc-app codebase
(`/Users/benstull/git/rfc-app`, tag v0.24.0): the email/OTC login flow,
passcodes, device-trust, Gitea OAuth + core auth, session/cookie security,
the admin role model, and per-endpoint authz guards across every
`api*.py` write/admin/owner endpoint. Verify deployment guarantee #4
("anonymous users are off-limits for any write/mutation"). Hunt for missing
guards, IDOR, and OTC/passcode brute-force/replay.

## Outcome

One High finding (OTC verify has no attempt limit / lockout — asymmetric
with the passcode path which does). Two Low/Info findings (session cookie
`https_only=False`; email-bounce webhook unauthenticated when env unset —
both documented dev contracts). Every enumerated write/admin/owner endpoint
enforces auth; no missing guards, no IDOR found. Admin role model is sound.

## Pre-state (what I found at the start)

- `auth.py` defines the dependency ladder: `require_user` (401 if anon) →
  `require_contributor` (adds muted + permission_state='granted' gate) →
  `require_admin` (role in owner/admin). Plus per-RFC predicates
  (`can_contribute_to_rfc`, `can_discuss_rfc`, `can_invite_to_rfc`,
  `is_rfc_owner`). `current_user` re-reads role + permission_state from DB
  every request (so grants/revokes take effect next request — good).
- Credentials: OTC 6-digit codes (bcrypt at rest, 10-min TTL, 60s request
  cooldown, single live code per email); passcodes (bcrypt, 5-fail/15-min
  per-account lockout, denylist, 4–20 chars); device-trust (256-bit CSPRNG,
  bcrypt, 30-day, HttpOnly+Secure+SameSite=Lax cookie); invite tokens
  (256-bit CSPRNG, bcrypt, 7-day, single-use).

## Turn-by-turn arc

1. Read auth.py + config.py. Dependency ladder is clean. Noted SECRET_KEY
   is `_required` (good — no insecure default). OAuth provisioning sets
   owner role only via OWNER_GITEA_LOGIN match; OTC/invite users get
   gitea_login="" / gitea_id=0 coalesced.

2. Read otc.py + passcode.py side by side. **Caught the asymmetry**:
   `passcode.verify_passcode` has a per-account failed-attempts counter +
   15-min lockout; `otc.verify_code` has NOTHING — it walks the last 5
   rows, bcrypt-checks, and returns "wrong" with no counter, no lockout,
   no per-IP throttle. The 60s cooldown lives in `request_code`, not
   `verify_code`. This is the headline finding.

3. Read device_trust.py + email_otc.py. Device-trust is solid (constant-time
   bcrypt walk, no early-exit, token never logged, cookie flags correct,
   user-scoped revoke). email_otc always returns 202 / logs failures —
   no email enumeration via the request path.

4. Read main.py (middleware + all /auth endpoints). Confirmed:
   - SessionMiddleware uses `https_only=False` (line 150) — itsdangerous-
     signed cookie, HttpOnly by default, but Secure flag off. Dev parity
     comment present. Behind HTTPS+proxy in prod this is low-impact but
     worth flagging.
   - `/auth/otc/verify` has NO rate-limit dependency, confirming finding #1
     at the endpoint layer too.
   - Turnstile gates `/auth/otc/request` ONLY, not verify.

5. Refutation pass on finding #1: Is there a global limiter? Grepped for
   RateLimit/slowapi/limiter/429 across backend/app — only tag_suggest.py
   (per-user, in-memory) and the OTC *request* cooldown 429. No global
   middleware, no limiter on verify. SPEC §ahead-of (line ~4100) discusses
   per-IP throttle for the *passcode* verify (which already has a per-
   account lockout) but never notes the OTC verify has no per-account limit
   at all. Quantified: single live code per email, bcrypt ~tens of ms,
   10^6 keyspace, 10-min TTL. Full sweep not possible in one TTL, but
   attacker re-requests (60s cooldown) to keep a live code and runs
   parallel verifies; expected ~500k guesses. bcrypt cost + TTL + single-
   live-code = real friction but NO attempt cap. Downgraded Critical→High.

6. Read api_admin.py end to end. Every endpoint calls require_admin; the
   user-search typeahead intentionally uses require_user (no privileged
   data). Role escalation well-guarded: only owner grants/changes owner;
   self-downgrade, self-mute, self-permission-flip, self-invite all refused;
   admins/owners not write-mutable. No spoof/escalation path. SOLID.

7. Enumerated routes+guards across api.py, api_prs.py, api_branches.py,
   api_graduation.py, api_invitations.py, api_notifications.py,
   api_discussion.py via grep. Every write/mutation route has require_user
   / require_contributor / require_admin, and the per-RFC write routes
   additionally call can_contribute_to_rfc / can_discuss_rfc /
   can_invite_to_rfc. GET read routes use current_user (nullable) — anon
   read is by design (guarantee #4 is about writes).

8. IDOR hunt — checked every owner/id-comparing helper:
   - api_prs `_can_merge`/`_can_withdraw`/`_can_edit_pr_text`: compare
     viewer.gitea_login against RFC frontmatter owners/arbiters cache and
     pr_row["opened_by"] — server-side data, not user-supplied. Safe.
   - api_branches `_require_branch_owner`/`_can_contribute`/
     `_can_read_branch`: branch grants keyed on grantee_user_id (session
     id), creator compared to gitea_login. Safe.
   - notification mute (`/api/users/{user_id}/notification-mute`): SQL
     scoped to viewer.user_id; only the {user_id} *target* is user-supplied
     and it's just the mute subject, not an ownership pivot. Self-mute
     refused. Safe.
   - invitations accept: requires auth + case-insensitive email match +
     refuses empty email; collaborator row keyed on viewer.user_id. Safe.
   - Considered: OTC/invite users have gitea_login="". Could "" match an
     owners list? Only if frontmatter literally contained "" — impossible
     from gitea_login frontmatter. `"" in []` is False. Killed as non-issue.

9. Checked anonymous-reachable mutation surfaces:
   - `/api/email/unsubscribe` (GET+POST): itsdangerous URLSafeSerializer
     signed token (SECRET_KEY, salted) → user_id+category not forgeable;
     idempotent, low-stakes; no expiry but URLSafeSerializer is fine here.
   - `/api/webhooks/email-bounce`: optional X-Webhook-Secret (constant-time
     compare) when WEBHOOK_EMAIL_BOUNCE_SECRET set; UNAUTHENTICATED when
     unset (documented v1 dev contract). Impact: flip email opt-out flags +
     mark outbound rows bounced. Low.
   - `/api/webhooks/...` Gitea receiver: mandatory HMAC-SHA256, constant-
     time compare, refuses to start without secret unless
     RFC_APP_INSECURE_WEBHOOKS=1. SOLID.
   - `/api/invites/claim`, `/auth/device-trust/start`: anon by design (they
     establish the session); both gated by 256-bit bcrypt-hashed tokens.

10. Confirmed db is a single shared sqlite connection (check_same_thread=
    False, isolation_level=None) — no incidental concurrency limiter that
    would mitigate the OTC brute-force.

## Cut state (final findings)

### High
- **F1 — OTC verify has no attempt limit / lockout / per-IP throttle.**
  `backend/app/otc.py:211 verify_code` (endpoint `backend/app/main.py:279
  /auth/otc/verify`). The passcode path (`passcode.py:262`) has a 5-fail/
  15-min per-account lockout; the OTC path has none. 60s cooldown is on
  *request* only. 6-digit code (10^6), 10-min TTL, single live code per
  email. Remediation: add a per-account failed-OTC-attempt counter +
  lockout mirroring passcode.py, and/or a per-IP 429 throttle on verify;
  consider invalidating the live code after N failures. Confidence: HIGH
  the control is absent (verified across module + endpoint + global grep).
  Severity argued down from Critical to High because bcrypt cost + 10-min
  TTL + single-live-code give real friction; still a genuine gap and an
  inconsistency with the passcode path.

### Low / Info
- **F2 — Session cookie `https_only=False`.** `backend/app/main.py:150`
  SessionMiddleware. Cookie is itsdangerous-signed + HttpOnly, but Secure
  is off "for dev parity." The device-trust cookie correctly sets
  Secure=True. In prod behind HTTPS the session cookie should also be
  Secure. Remediation: drive https_only from an env flag, true in prod.
  Confidence: HIGH it's set false; impact Low given HTTPS + signed cookie.
- **F3 — Email-bounce webhook unauthenticated when env unset.**
  `backend/app/api_notifications.py:543`. Documented v1 dev contract;
  impact limited to email opt-out flag flips + outbound-row status. Info/
  Low. Confidence HIGH on behavior, low on impact.

### What's solid (controls correctly implemented)
- Three-tier dependency ladder; `current_user` re-reads role +
  permission_state from DB every request (revokes take effect immediately).
- Admin role model: owner-only owner grants, self-downgrade/self-mute/
  self-permission/self-invite refusals, admins/owners not write-mutable.
  No escalation/spoof path. Every /api/admin/* endpoint require_admin.
- Every write/mutation endpoint across all api*.py files enforces auth;
  per-RFC writes additionally enforce can_*_to_rfc. No missing guard.
- No IDOR: owner/permission checks compare against server-side frontmatter
  cache / session user_id / pr opened_by, never a trusted user-supplied id.
- Passcode lockout (5/15min), denylist; OTC single-live-code + consume-
  before-provision (no replay, no double-sign-in).
- All tokens (OTC, passcode, device-trust, invite) bcrypt-hashed at rest;
  device-trust/invite tokens 256-bit CSPRNG; constant-time bcrypt walks,
  no early-exit timing leak; raw tokens never logged.
- Gitea webhook mandatory HMAC-SHA256 + constant-time compare; OAuth state
  param checked with secrets.compare_digest.
- SECRET_KEY is _required (no insecure default); unsubscribe tokens signed
  with it (salted, not forgeable).
- OTC request returns 202 regardless (no email enumeration); verify
  collapses wrong/unknown to one 400 (no enumeration); passcode verify
  collapses wrong/unknown likewise.

## What the driver needs to know

- F1 is the only finding worth a remediation slice. It is a real,
  exploitable-with-effort gap AND an internal inconsistency (passcode has a
  lockout, OTC doesn't). Recommend the driver fold it in at High; if the
  driver judges the bcrypt+TTL friction insufficient given OHM is publicly
  reachable, it could argue back toward Critical — I left it at High after
  refutation.
- F2/F3 are documented dev contracts; flag them but they are low priority.
  Worth a one-line confirmation that OHM's prod overlay sets
  WEBHOOK_EMAIL_BOUNCE_SECRET and serves over HTTPS (the device-trust
  cookie is Secure-only, so if OHM weren't HTTPS, device trust would
  already be broken — implying HTTPS is in fact present, which de-risks F2).
- The authz guard coverage is genuinely complete — guarantee #4 holds for
  every write/admin/owner endpoint I enumerated. No anon-write hole found.

## §19.2 candidates surfaced

- Per-account OTC-verify lockout + per-IP verify throttle (pairs with the
  SPEC's already-listed per-IP passcode-verify candidate). This is the
  remediation for F1 and is a flotilla-irrelevant, rfc-app-core change.