Deleting an application left its oidc_device_codes orphaned, tripping the
FK constraint and raising SQLite3::ConstraintException. Unlike the other
child tables, oidc_device_codes had neither a dependent: :destroy
association nor an on_delete: :cascade FK, so any app that had started a
device-authorization flow could not be deleted (regardless of groups/users,
since a pending device code has a null user_id).
Add the missing has_many :oidc_device_codes, dependent: :destroy and a
migration to cascade at the DB level, mirroring application_user_claims.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LARKs4cVsGrvKCkKZhPcy6
Regenerate the user_code when a freshly generated one collides with an existing
code (up to USER_CODE_MAX_ATTEMPTS), so a client never gets a uniqueness error
just because two codes happened to match; the DB unique index stays the final
guard against a concurrent-insert race.
Replace the model test that asserted Rails' uniqueness validator message
("has already been taken") — per CLAUDE.md we don't test framework behavior —
with one that exercises the app-specific property: generation retries past a
collision and yields a unique code.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq
The supported-scope list lived on OidcController, so the device authorization
controller (and a test) had to reach across into OidcController::SUPPORTED_SCOPES.
Move it to a shared OidcScopes::SUPPORTED module — one source of truth for the
OIDC controller, the device flow, and consent handling — and update every
reference. No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Q4ATZHoMCWqvSpE2yYoie
The consent record is unique on user+application and shared between the browser
authorization flow and device-flow approval. Centralize the upsert in
OidcUserConsent.record!(merge:) so a narrower device request unions its scopes
into any existing grant (merge: true) rather than overwriting it and wiping
stored claims, while the browser consent screen records exactly the approved
scopes (merge: false). scopes is now nil-safe for not-yet-saved records.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq
More device-flow security-review fixes.
Replay-revocation on device_code redemption (was: destroy on use):
- Redemption now marks the code redeemed_at and links the issued access/refresh
tokens back to it (new oidc_device_code_id FKs, on_delete: :nullify) instead of
destroying the row. A replayed redeemed code is detected as reuse — revoking
every descended token and returning a distinguishable "already been used"
invalid_grant rather than the generic "Invalid device_code" — mirroring the
authorization-code reuse semantics (RFC 6749 §4.1.2). The device_code FK is
carried forward across refresh rotation so revocation reaches the whole chain.
Redeemed codes are reaped by the existing expiry cleanup sweep.
user_code CSPRNG (RFC 8628 §6.1):
- The 8-char user_code is a credential (type it + Approve mints tokens), so draw
it from SecureRandom instead of Ruby's global Mersenne Twister (Array#sample).
Rate-limit the /device verification endpoint (RFC 8628 §5.1):
- Add the app's standard 10/min limit on show + verify so a signed-in user can't
brute-force the short code space to deny or hijack a pending authorization.
Also carried in this commit (other agent's working-tree change):
- Give each OidcController rate_limit a distinct name: so frequent device polling
on the token endpoint no longer shares one counter with (and 429s) unrelated
token/refresh/revoke/introspect calls.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Q4ATZHoMCWqvSpE2yYoie
Two more fixes from the device-flow security review:
slow_down interval grew without bound (OidcController):
- Each too-fast poll bumped the persisted interval by 5s with no ceiling, so a
client polling slightly fast — or an attacker spamming a known device_code —
could balloon it (5→10→15→…) past the 10-minute expiry and starve a legitimate
client of its token. Clamp the bump to OidcDeviceCode::MAX_INTERVAL (30s); a
client polling at the advertised interval is never throttled.
Expired device codes accumulated forever (OidcTokenCleanupJob):
- Anonymous callers can create device codes via /oauth/device_authorization, and
expired/denied/abandoned rows were never reaped. Sweep rows past expiry (with a
1-hour grace to stay clear of in-flight redemption); redeemed codes are already
destroyed at token issuance.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Q4ATZHoMCWqvSpE2yYoie
Fixes from review of the device flow / introspection / DCR work:
Introspection over-disclosure (RFC 7662):
- Restrict introspection to authorized callers — the client a token was issued
to, or a resource server registered (resource_identifiers) to serve the token's
bound RFC 8707 audience. Unauthorized callers get {active:false}, disclosing
nothing, instead of any confidential client reading any token.
- Scope-gate disclosed claims: username only with the email scope, groups only
with the groups scope (mirrors userinfo). ADR 0005.
Tokens minted for revoked users:
- Re-check application.user_allowed?(user) at mint time in the device, refresh,
and authorization-code grants (covers app-active, user-active, group
membership). A user deactivated or removed from the allowed group between
approval and the token request is refused with access_denied. The refresh
check runs before rotation so a denied refresh has no side effects.
Device authorization hardening:
- Require confidential clients to authenticate, and require PKCE (code_challenge)
up front for clients that require it, so an intercepted device_code plus a
known public client_id cannot redeem tokens.
- Merge (not overwrite) the shared consent record on device approval.
Tests: new oidc_introspection_test and oidc_mint_authorization_test; existing
PKCE/claims tests updated to grant app access (they created ungrouped apps and
relied on the token endpoint not checking authorization). Full suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq
Adds three OAuth surfaces to the OIDC provider so CLIs, terminal agents, and
MCP connectors can authenticate as real users instead of using static API keys.
Device Authorization Grant (RFC 8628):
- OidcDeviceCode model (HMAC device_code, short plaintext user_code, nullable
user until approval, slow_down polling state), mirroring OidcAuthorizationCode
- POST /oauth/device_authorization issues the code pair + verification URIs
- device_code grant on /oauth/token returns authorization_pending / slow_down /
access_denied / expired_token, then the standard token triple; single-use
- Authenticated /device approval page, gated by Application#user_allowed?
Token Introspection (RFC 7662):
- POST /oauth/introspect: confidential-caller-authenticated; returns active,
scope, and the user's groups so resource servers can authorize on membership
Dynamic Client Registration (RFC 7591):
- POST /oauth/register creates public/confidential clients (PKCE required)
- Runtime toggle via a new Setting store + admin switch on the Applications page;
off by default, env var CLINCH_DCR_ENABLED as bootstrap fallback
- New clients are default-deny (no allowed_groups) until an admin grants access
- RFC 8414 metadata alias at /.well-known/oauth-authorization-server;
registration_endpoint advertised only while the window is open
Discovery advertises all three grants/endpoints. Seeds add a clinch-cli public
client and a c2a2-introspection confidential client. ADRs in docs/decisions
record the opaque-vs-JWT, device-flow, and DCR-security decisions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq
Two problems with sign-count clone detection:
- suspicious_sign_count? flagged the case where both the stored and presented
counts are 0. Most synced passkeys (Apple/Google) report 0 every time, so every
legitimate sign-in was flagged — drowning real signals in noise. Per WebAuthn
§6.1.1 a 0 counter means "no counter"; only flag when BOTH counts are non-zero
and the new one does not advance.
- On a suspicious count the controller only logged a warning and then continued
to authenticate and overwrite the stored counter. A cloned credential therefore
worked indefinitely. webauthn_verify now rejects the sign-in (no session, no
counter update) and emails the user via a new SecurityMailer#suspicious_passkey_used.
Tests cover the corrected classification (synced/first-use/normal vs equal/
decreasing) and the new alert email.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
revoke_family! revoked only the refresh tokens in a rotation family. When reuse
of a revoked refresh token was detected (a token-theft signal), the access
tokens issued across that chain stayed valid at /userinfo until expiry — up to
the access-token TTL — so an attacker holding a stolen access token kept access.
revoke_family! now also revokes every access token referenced by the family's
refresh tokens. Adds a regression test: rotate once, reuse the revoked token,
and assert both the original and rotated-in access tokens are revoked.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
active? was only checked at the password step of sign-in. A user disabled
afterwards could (a) still complete the 2FA step and mint a valid session, and
(b) keep using any existing session until natural expiry, because per-request
auth only checked session expiry, not user status.
Three enforcement points:
- Mid-flow guard: verify_totp and webauthn_verify re-check active? before
start_new_session_for, clearing the pending session and rejecting if disabled.
- Request-time guard: find_session_by_cookie now uses Session.for_active_user,
so a session whose user is disabled no longer authenticates (authoritative,
catches any disable path including direct DB changes).
- Immediate cleanup: User#revoke_sessions_when_deactivated destroys a user's
sessions when status changes away from active, so access is revoked everywhere
at once rather than on the next request.
Tests cover the mid-flow TOTP rejection, request-time rejection of an existing
session after disable, session destruction on disable, and that unrelated
updates leave sessions intact.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
backchannel_logout_uri was validated only for scheme/HTTPS, so an admin (or a
compromised admin account) could point it at internal infrastructure — cloud
metadata (169.254.169.254), loopback, or RFC1918 hosts — and every user logout
would fire a server-side POST there.
Add PrivateAddressCheck (app/lib) and apply it as defense-in-depth:
- Application validation rejects URIs whose host is, or is a literal, internal
address (loopback / private / link-local / 0.0.0.0 / localhost / metadata
hostnames). Fast, DNS-free, immediate admin feedback.
- BackchannelLogoutJob re-checks at request time WITH DNS resolution and aborts
(no retry) if the host resolves to a non-public address — covering URIs that
predate the validation and public hostnames pointed at internal IPs.
Tests cover the address classification, the model validation, and updates an
existing test that used a localhost logout URI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
verify_totp called ROTP without `after:`, so a captured 6-digit code stayed
valid for the full ~90s drift window and could be replayed in a separate
sign-in. Add a last_otp_at column, pass it as ROTP's `after:`, and persist the
matched timestep on success so a code (or any earlier one) cannot be reused.
Also fixes a latent bug surfaced by the new replay path: enable_totp! did
`self.backup_codes = generate_backup_codes`, reassigning backup_codes to the
plaintext return value (generate_backup_codes already stores the BCrypt hashes
internally). That stored backup codes in plaintext and broke verification.
enable_totp! is test-only today, but it is public and backup_codes is not
encrypted, so this is a real footgun. Now it just calls generate_backup_codes.
Rewrites the mislabeled "TOTP code cannot be reused" test to actually assert
that replaying an accepted code is rejected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When an application has no icon attached, render a deterministic
monogram SVG instead of the generic picture-frame placeholder. Initials
are picked from capital letters in the name (ShelfLife -> SL); fall
back to the first two letters when fewer than two capitals exist
(Audiobookshelf -> AU). Background colour is hashed from the name for
stable per-app identity across visits.
Adds an optional second icon attachment, icon_dark, alongside the main
icon. When present, render a <picture> with a prefers-color-scheme:
dark source so the browser swaps automatically; when absent, the main
icon is used in both modes. The SVG sanitization, content-type fix,
and size/format validation now run over both attachments uniformly.
Bumps to 0.14.0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The sanitize_svg_icon before_validation callback called icon.download,
but Active Storage uploads pending blobs in before_save — so at
before_validation time the file only existed in the request tempfile,
not at the configured storage path. Read from the pending attachable
(UploadedFile / IO hash) instead. Guards against the recursive callback
that icon.attach would otherwise trigger by tracking the cleaned
attachable by object identity. Bumps to 0.13.1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the implicit "empty allowed_groups means public" rule with
explicit default-deny across both OIDC and ForwardAuth. Adds two boolean
flags on Group — auto_assign (Keycloak-style auto-join on user create)
and admin (members can reach the admin panel) — and drops the
users.admin column entirely. Adds "Users with access" and "Accessible
applications" panels with via-group badges on the application/user show
pages.
BEHAVIOR CHANGE: a ForwardAuth app with no allowed_groups previously
bypassed authentication entirely; it now returns 403 like any other
unauthorized request. The data migration seeds an "everyone" group and
attaches it to all previously group-less apps to preserve behavior on
existing installs. An "admins" group is seeded and backfilled from any
user with the old admin column.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Application#sanitize_svg_icon already runs a Loofah scrubber on every
icon upload, but the scrubber class itself was never tracked. Land it
along with tests covering the four shapes that matter:
- <script> elements stripped entirely
- on* event handlers (onload, onclick, …) removed but the carrying
element preserved
- attribute values pointing at javascript:/data: URIs rejected
- benign icons round-trip unchanged
Writing the benign-icon test caught a real bug: the attribute allowlist
holds canonical SVG case (viewBox, preserveAspectRatio, gradientUnits,
…) but safe_attribute? downcases the incoming name before comparing,
so legitimate icons were silently losing those attributes on upload.
Fix by comparing against a precomputed lowercase lookup set; the
constant stays readable as canonical SVG case for documentation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The replay handler previously used a created_at time-range filter to
target access tokens and called update_all(expires_at:), which left
revoked_at nil, skipped refresh tokens entirely, and could miss or
falsely catch tokens from concurrent flows. Add an oidc_authorization_code
FK on both token tables, carry it through refresh-token rotation, and
use the association to revoke every descendant via revoke! (which sets
revoked_at and cascades access -> refresh).
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Remove PKCE plain method support (S256 only), enforce openid scope requirement,
filter to supported scopes, strip reserved claims from custom claims as
defense-in-depth, sanitize SVG icons with Loofah, add global input padding,
switch session cookies to SameSite=Lax, use Session.active scope, and remove
unsafe-eval from CSP.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove duplicated app_allows_user_cached?/headers_for_user_cached methods; call model methods directly
- Fix sliding-window rate limit bug by using increment instead of write (avoids TTL reset)
- Use cached app lookup in validate_redirect_url instead of hitting DB on every unauthorized request
- Add cache busting to ApplicationGroup so group assignment changes invalidate the cache
- Eager-load user groups (includes(user: :groups)) to eliminate N+1 queries
- Replace pluck(:name) with map(&:name) to use already-loaded associations
- Remove hardcoded fallback domain, dead methods, and unnecessary comments
- Fix test indentation and make group-order assertions deterministic
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rate limit failed attempts (50/min per IP) with 429 + Retry-After.
Cache forward auth applications in a dedicated MemoryStore (8MB LRU)
to avoid loading all apps from SQLite on every request. Debounce
last_activity_at writes to at most once per minute per session.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enables server-to-server authentication for forward auth applications
(e.g., video players accessing WebDAV) where browser cookies aren't
available. API keys use clk_ prefixed tokens stored as HMAC hashes.
Bearer token auth is checked before cookie auth in /api/verify.
Invalid tokens return 401 JSON (no redirect). Requests without
bearer tokens fall through to existing cookie flow unchanged.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
