You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Trust & Compliance: this lets people connect third-party apps of their own choosing to their Fizzy data, a new flow of customer data to third parties at the user's initiative, with no new subprocessor. It's on the T&C On Call board: Fizzy OAuth: people can connect third-party apps to their Fizzy data. OAuth ships dark (OAUTH_ACCEPTANCE_ENABLED unset) until that review is done.
Builds on existing Identity::AccessToken (optionally belongs to an OAuth client)
Provides everything MCP clients need for dynamic registration and auth
Adds a Connected Apps listing to user profiles to view and revoke granted tokens
No refresh tokens; no token expiry; authorization code + PKCE only.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a minimal OAuth 2.1-style authorization code + PKCE stack to Fizzy, including dynamic client registration and a “Connected Apps” UI built on the existing Identity::AccessToken model (now optionally associated to an OAuth client).
Rebased oauth onto current main (738 commits, one trivial conflict in config/routes.rb) and force-pushed: 4a047d8. Full suite green after rebase: 1644 tests, 0 failures.
One follow-up commit landed on this branch during review convergence: 840ec75 accepts form-encoded requests at the token/revocation/registration endpoints — conforming OAuth clients POST application/x-www-form-urlencoded, which the app's forgery protection rejected over https. The endpoints are cookie-free so CSRF doesn't apply; the browser-facing consent form keeps its protection. (Three resulting CodeQL rb/csrf-protection-disabled alerts dismissed with that reasoning.)
Connector-grade gaps closed as a stack of three PRs on top of this branch, one gap each:
All four PRs converged through Copilot/Codex review (17 findings: 15 fixed, 2 declined with reasoning in-thread) and are CI-green. Full suite on the top of the stack: 1683 tests, 0 failures. Merging this PR first, then the stack in order, keeps each diff reviewable.
Still open, deliberately: consent/Connected Apps UI design polish — the screens work (and now name the redirect host for hosted clients) but haven't had a design pass.
Single-use codes (RFC 6749 §4.1.2, MUST). Codes were stateless blobs that could be redeemed again within their 60s life, and each redemption minted a separate grant. Each code now carries a jti. The grant it mints is stamped with that jti in identity_access_tokens.authorization_code_jti, which has a unique index. A second redemption fails the insert, returns invalid_grant, and destroys the grant the first redemption issued (the SHOULD). Replay is detected only after PKCE, redirect_uri and client_id all pass, so someone holding a leaked code without the verifier can't revoke the grant. I used a DB unique index rather than SolidCache because the cache is per-region, and #12535 showed its failsafe can report a store error as "already written". bc3 persists code rows with consumed_at instead. If phase B moves Fizzy to persisted codes for audience, the jti column still provides revoke-on-reuse.
client_id required and checked on the code grant (RFC 6749 §4.1.3, MUST). Until now the client was taken from the code alone, so client_id could be left out or could name a different client.
Missing parameters return invalid_request (RFC 6749 §5.2). A missing grant_type, code, code_verifier, redirect_uri or client_id used to return unsupported_grant_type or invalid_grant, and a client reads those as a dead grant.
Known limit: if a grant is revoked within the code's 60s life, its row is deleted, so a replay inside that window could redeem the code once more. It still needs the verifier. I'm leaving it as is rather than adding tombstones.
Codes minted before deploy have no jti and are rejected. They expire within 60s anyway.
I rebased the train on top of this. #3081 gets one more commit applying the same parameter checks to refresh. #3082's ordering commit now authenticates the client before the shared client_id check for both grants.
Conformance fix on the authorization endpoint: 83e9118
Consent form-action (CSP). The app-wide form-action 'self' is enforced in OSS and in SaaS production. Browsers check form-action on every redirect that follows a form submission, so Chrome blocked the 302 that Authorize and Deny send to the client's loopback or https callback. The flow stopped on the consent page. Oauth::AuthorizationsController now adds a form-action source for the request's validated redirect target, ported from bc3:
http/https: the exact origin, including the loopback port the request presented (RFC 8252 §7.3).
No CSP host-source spelling: if the target can't be written as a CSP host-source, the source is just its scheme. That covers IPv6 [::1], percent-encoded hosts and registered native schemes.
Unvalidated redirect_uri: nothing is added; the page keeps 'self' only.
The source is built only from a redirect that allows_redirect? accepted, and only in a shape that matches a strict pattern, so nothing raw from params goes into the header. content_security_policy runs only when an app policy exists, so DISABLE_CSP still means no CSP. The new Chrome system test test/system/oauth_consent_test.rb fails without the override (I checked) and passes with it.
RFC 9207 iss. Success and error redirects now carry iss: the code redirect, access_denied, and every redirect_with_error redirect. The metadata advertises authorization_response_iss_parameter_supported: true. The metadata issuer, the protected-resource authorization_servers entry and iss all come from one oauth_issuer helper, so they can't drift (§2 requires them to be identical). The rendered error page for an unvalidated redirect doesn't redirect and is unchanged. This is the mix-up defense RFC 9700 §4.4.2 calls for when a client talks to several authorization servers, which is the normal case for MCP clients.
Canonical scope.Oauth.canonical_scope reports a grant that includes write as "read write" everywhere.
Consent screen: a request for "read write" matched neither option, so the browser preselected Read. A user who just clicked Authorize got a read-only grant. The Read + Write option's value is now read write and is preselected for read write or write.
Registration: DCR registering write also registers read, which write already implies.
Unchanged: a read-only client still never sees the write option.
I rebased #3080, #3081 and #3082 and force-pushed each with a lease. On #3081 there is one new commit (52e2e56) that routes granted_scopes and the refresh echo through the same helper.
RFC 8414 metadata. Now advertises revocation_endpoint_auth_methods_supported: ["none"] and response_modes_supported: ["query"]. When those keys are left out, the spec defaults are client_secret_basic and ["query", "fragment"], and this server does neither. If revocation later authenticates confidential clients (the open RFC 7009 §2.1 thread on Support confidential clients with client_secret_post #3082), the value becomes ["none", "client_secret_post"] in the same change.
RFC 7591 §3.2.2.client_name has to be a String; any other type falls back to "MCP Client". It is also capped at 255 characters. Before, a 256-character name hit the column limit and returned a 500 (ValueTooLong on MySQL, CheckViolation on SQLite). Now it returns 400 invalid_client_metadata.
RFC 8252 §7.3. Loopback matching now requires the exact host and query, so only the port may vary. This brings back the query fix from the resolved Copilot thread that had been lost. It also stops treating 127.0.0.1, localhost and ::1 as the same host, which matches bc3's loopback_match?.
RFC 7009 §2.2.1. A missing or blank token now returns invalid_request as JSON, instead of Rails' generic ParameterMissing 400.
I wrote the tests first and saw them fail. The train has been rebased (#3080, #3081 and #3082) and force-pushed with lease. In #3080 the matcher combines Oauth.loopback_host? with exact host and query matching and the fragment guard. In #3081, revocation keeps the refresh-token lookup behind the presence check. At the tip, the full OSS SQLite suite passes: 1754 tests, 0 failures.
Availability switch: the AS now merges dark (2b6ea64)
Merging this PR lit OAuth on app.fizzy.do and in every OSS install, because the routes were unconditional. That contradicts fizzy-mcp-server docs/hosted-plan.md Phase A and Decision 6: the train merges dark, and OAuth in OSS core is off by default. This commit ports bc3's Oauth::Availability, trimmed to what Fizzy has: no IAT cohorts and no CIMD.
Switch
Env (production, beta, staging)
Default
Effect
acceptance
OAUTH_ACCEPTANCE_ENABLED=true
dark (fail-closed, == true)
Dark: both .well-known documents, DCR, authorize and token answer 404, and OAuth-issued bearer tokens are refused. PATs are unaffected.
issuance
OAUTH_ISSUANCE_ENABLED=false pauses
follows acceptance
Paused: discovery stays up; DCR, authorize and token answer 503; issued access tokens keep working
pilot
OAUTH_PILOT_CLIENT_IDS=a,b
empty
The named client_ids are exempt from both switches at authorize, token and acceptance. Discovery and DCR name no client, so they stay dark.
Never gated: revocation, Connected Apps, and consent denials (error=access_denied), so people can always shed access.
Gate runs before sign-in: a dark authorize endpoint never sends anyone to the sign-in page.
Departures from bc3:
Acceptance is fail-closed (== true); bc3 reads nil as lit. An install that never mentions OAuth stays dark.
A dark server answers 404, where bc3 answers 503, because a dark Fizzy should look like a Fizzy without an AS.
Protected-resource metadata also 404s while dark. bc3 keeps it live because it points at Launchpad; Fizzy has no other AS to name.
Environments: lit in development (OAUTH_DARK=1 previews dark) and in test. docs/docker-deployment.md documents the switches.
Incident rule (also in the module comment): a pause must also empty OAUTH_PILOT_CLIENT_IDS.
Tests:test/integration/oauth_availability_test.rb and test/models/oauth/availability_test.rb. They failed first: 12 failures and 6 errors before the change.
Local runs at the tip: the full OSS SQLite suite passed (1784 tests, 0 failures, 1 skip), along with the Chrome consent system test, rubocop and brakeman.
To light production: set OAUTH_ACCEPTANCE_ENABLED=true in saas/config/deploy.production.yml when the hosted plan's light-up comes. That is a separate change, not part of this train.
Follow-up commit bf477a7: registration stays dark even when a request names a pilot client_id. Before this, the gate read the claimed client_id at DCR too, so anyone who knew a pilot's id could register clients on a dark server. Registration creates a client and doesn't act for one, so ClientsController#piloting_client_id is nil. There is a failing-first test. #3080, #3081 and #3082 are rebased again; their new heads are 7163917, 870e173 (refresh pin d05407c → 870e173) and 3a8d94e (confidential pin).
1402fa7: the token endpoint's require_params now rejects any grant parameter that isn't a single non-blank string with invalid_request (RFC 6749 §5.2). Before, a JSON array or object as code or code_verifier raised and answered 500.
Grants belong to the identity. One whose only membership ended was
sent to the session menu and could never revoke them, though joining
an account again would restore their reach.
…ters
Registration is unauthenticated, and the rate limit only counts
requests, so each one could store an arbitrarily large list. The list
is now checked before any URI is parsed, and the model holds the same
bound for every way a client is created.
Presence let an array through, which the code then carried until its
exchange compared it with the verifier and raised. A challenge is now
the 43-character unpadded base64url SHA-256 of a verifier, which also
bounds what the code carries.
An oversized or non-string state rejected by validate_state is still echoed in the error redirect. A consent POST with a very large state therefore produces a similarly large Location header, defeating the bound and potentially preventing delivery of the error. Echo state only when valid_state? succeeds; this also protects errors raised before state validation.
Use valid verifiers to test digest mismatches
app/models/oauth/authorization_code.rb:30
The wrong-verifier model test and the token-exchange and replay tests use wrong_verifier, which is shorter than 43 characters and fails before reaching this comparison. They would still pass if the digest comparison always returned true. Use distinct, valid-format verifiers to test an actual digest mismatch, assert that exchange issues no token, and verify that a mismatched replay leaves the existing token intact.
Test pilot token acceptance without session authentication
test/integration/oauth_availability_test.rb:154
This request still carries the session cookie from sign_in_as. Authentication#require_authentication resumes that session before checking the bearer token, so the assertion passes even if pilot-issued tokens are rejected while OAuth is disabled. Save the issued token and sign out before this request so it exercises the pilot token acceptance exemption.
* commit 'f64ab634ab6ea1737e48722b83dbefabc13299de': (22 commits)
Issue a code only for an S256-shaped code_challenge
Bound what open registration stores: ten redirect URIs of 2000 characters
Reach Connected Apps without an account, as personal access tokens are
Accept an authorization request without state, and bound one that has it
Filter authorization codes, PKCE verifiers and DPoP proofs from logs
Keep credentials that outlive a grant away from OAuth apps
Keep OAuth grants out of email changes too
Keep OAuth grants out of every credential and consent surface
Refuse a PKCE verifier outside 43 to 128 unreserved characters
Refuse personal access token management to OAuth grants
Point a client's access_tokens at oauth_client_id, and drop the stray foreign key
Vary the port only for http loopback redirects
Reject non-string token request parameters as malformed
Keep registration dark when a request names a pilot client
Ship the OAuth authorization server dark behind availability switches
Describe revocation and response modes honestly, bound client names, and vary only the loopback port
Let consent redirect past CSP, name the issuer, and grant canonical scopes
Redeem each authorization code once, for the client it was issued to
Accept form-encoded requests at the OAuth protocol endpoints
Connected Apps UI for OAuth grant management
...
MySQL commits each DDL statement on its own, so a migration of several statements that stops partway (a lock wait, a lost connection) leaves the first ones applied and its version unrecorded. Running it again then fails on the first one. Each step now skips itself when it's already done: create_table, add_column, add_reference and add_index with if_not_exists. The DDL they emit is unchanged.
rate_limit is a before_action, and it came before require_issuance_enabled, so a dark server answered registration's eleventh request, and the token endpoint's twenty-first, with 429 slow_down. The availability gate now runs first.
Main runs Ruby 4.0, which drops CGI.parse, so the OAuth flow tests read redirect queries with URI.decode_www_form like main's own. Main's SQL redaction tests look a token up through Identity::AccessToken.find_permissable, the bearer path here.
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
…readable
Main filters code by partial match for the magic link and join codes, which also caught OAuth's public code_challenge. Match it whole, nested paths included, and it covers the authorization code too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Note
Trust & Compliance: this lets people connect third-party apps of their own choosing to their Fizzy data, a new flow of customer data to third parties at the user's initiative, with no new subprocessor. It's on the T&C On Call board: Fizzy OAuth: people can connect third-party apps to their Fizzy data. OAuth ships dark (
OAUTH_ACCEPTANCE_ENABLEDunset) until that review is done.No refresh tokens; no token expiry; authorization code + PKCE only.
Needs design: Authorization
Needs design: Connected Apps