An OAuth 2.1 authorization server, so Claude can connect from a phone #168
No reviewers
Labels
No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
lz/agent-nexus!168
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/oauth-as"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Nexus issues its own OAuth 2.1 tokens, so Claude — on a phone, on the desktop, or in another Nexus worker — can authorize against it as a custom connector. Closes #151.
oidc-provider@9.12.2(OpenID Certified) runs as a Koa app on the raw Node handler insrc/server.ts, beside the existing WebSocket upgrade listener. Storage is a Kysely adapter over one SQLite table. Consent is an ordinary SvelteKit page behind the normal cookie gate; only the protocol endpoints sit outside it.Deliberately not in this PR
Say so here rather than quietly doing it:
cimdAllowlistand advertises the discovery flag from it; nothing populates it, so the flag shipsfalseand Claude Code cannot yet complete a flow againstmain. That is the correct state — advertising support we cannot honour makes Claude pick a mechanism that then fails./api/mcp, protected-resource metadata and the bearer challenge (#154).Gates
main)Also verified against the built server (
pnpm build,node build/server.js) on an isolatedDATA_DIR:/.well-known/oauth-authorization-serverreturns503 {"error":"public_url is not configured"}unset, and200with a matchingissueronce set.Guards proven able to fail
Every one of these was broken deliberately and a named test watched go red, then restored from a file copy. This repo has shipped six tests that could not fail, so the proof is the point:
!(expires_at > now)→expires_at <= nowconsume()→destroy()'s bodyrevokeByGrantIdby model.where('model',…)fromfindByUid/findByUserCode/destroyMath.floorfromreviveconsumed_attodoUpdateSetdoUpdateSet→doNothing()trueallowDcr === true→allowDcr !== falseroutes: {}jwks:/ deleteadapter:routeOwner)decodeURIComponentas the first line ofrouteOwnerstartsWith('/oauth/')→startsWith('/oauth')MCP_PATH::ffff:branch.some(isLoopback)→.everydecideConsent→!== 'deny'consentSubmitFailureexpired branch → 500app.use('/oauth', …)res.headersSentcheckfinishConsentFailuretestsexpires_at > now→>= nowloginresultaddResourceScopeThe four bugs that made this feature not work
The headline finding. Every gate was green and the authorization flow could not have completed once. Four defects sat between "operator clicks Approve" and "token issued", layered so that each hid the next. Two were found by reading the library; the other two only surfaced once a test actually drove the flow.
The mutation sequence tells the story better than prose:
login:no_sessionconsent:rs_scopes_missingconsent:op_scopes_missingscope = nexus:read nexus:writeThe login prompt was never resolved.
config.tssets onlyinteractions.url, neverinteractions.policy, andoidc-providerdeep-merges configuration — so the default two-prompt policy (login, thenconsent) was still in force.handleConsentSubmitreturned{ consent: { grantId } }only, soresume.jsnever calledsession.loginAccount,session.accountIdwas never set, and theno_sessioncheck re-fired forever. Note this is the library's own_sessioncookie, unrelated to Nexus's operator session — being unlocked does not satisfy it.Scopes were written to the wrong bucket.
addOIDCScopewritesgrant.openid; thers_scopes_missingcheck readsgetResourceScopeEncountered(), written only byaddResourceScope. WithresourceIndicatorsenabled and adefaultResource, that check runs on every authorization.They are needed in both buckets. Because
config.tsalso declares the Nexus scopes in the top-levelscopeslist,requestParamOIDCScopesincludes them, soop_scopes_missingdemands them in the openid bucket too. Routing them to the resource bucket alone — which is what I instructed — still looped, just on a different check. The implementing agent proved that empirically rather than taking my word for it.JWT access tokens could not be signed.
accessTokenFormat: 'jwt'takes its algorithm fromclientDefaults.id_token_signed_response_alg, which defaults toRS256, whilejwks.tsprovisions only EC P-256. Every resource-bound token exchange 500'd. This was unreachable dead code until bugs 1–3 were fixed. One line:clientDefaults: { id_token_signed_response_alg: 'ES256' }.In this configuration there is no path that terminates with a wrong-but-valid token — every incomplete bucket combination loops instead, because the scopes are double-declared. The fix is covered by
flow.test.ts, which drives a real PKCE authorization → consent → resume → token exchange against a real provider and SQLite adapter, asserts the issued token's scope, and asserts the flow terminates in exactly one approval.Bugs 1 and 2 came from the plan, not from the implementation. Nothing in 1906 passing unit tests touched any of the four, because every test asserted against the configuration object or a pure function and none drove the protocol.
Other things found by building it
Each of these changed the code, and none would have failed a green gate.
An undefined
jwksdoes not throw.initialize_keystore.js:282silently substitutesDEV_KEYSTORE— a fixed keypair (kid: 'keystore-CHANGE-ME') shipped in every install ofoidc-provider, behind awarn. Bothjwksandadapterare optional in theConfigurationtype, so TypeScript would not stop it either, and neither had a test. Anyone who has read the library source could have forged tokens. Both are now asserted.An expired consent form crashed Nexus.
handleConsentSubmitwas called unawaited;interactionDetails()throwsSessionNotFoundon a missing or expired interaction, and Node 22 turns an unhandled rejection into a process exit. Leaving the consent page open and clicking Approve would have restarted the control plane, dropping every session terminal and preview with it. Reproduced live against the built server, then fixed and re-verified.oidc-providerdoes not implement RFC 8414 §3.1. The discovery route is a literal constant (initialize_app.js:180), so a path-carrying issuer would not move the document — it would leave the library serving at the root while a compliant client looked at the path-inserted location and found nothing. The spec said the opposite; corrected in80a628f. The regression that does move it is an express-style mount, which is what the placement test's mutation exercises.Replay detection rested on an unpinned assumption.
upsertwroteconsumed_at: nullunconditionally, including on the conflict path. No live bug in 9.12.2 — verified by reading all threeconsume()call sites — but a version bump that re-saved a consumed code would have removed the defence with every test green. Now structural.The
::ffff:loopback form dodged the consent warning. The URL parser normaliseshttp://[::ffff:127.0.0.1]to::ffff:7f00:1, which matched neither the::1nor the127.check. A loopback redirect URI written that way is loopback in fact and escaped the warning — the same laundering the "any registered URI" rule exists to stop, through a different door.Decisions worth knowing
The issuer is path-less.
oidc-provider's endpointroutesare configurable independently of the issuer, so the issuer stays at the origin and the endpoints live under/oauth/*. Metadata therefore sits at the standard well-known path.req.urlis not rewritten — an express-style mount would break every endpoint.The dispatcher uses a prefix, not the named allowlist this repo uses elsewhere (
AGENT_CALLBACKS,TOKEN_ROUTES). The library mounts dynamic sub-paths:${routes.authorization}/:uidis the interaction resume endpoint the browser returns to after consent, so an exact set would 404 the consent flow's own return path. It does not decode —/%6fauth/tokenis not/oauth/token, which is the bug class of #120.The consent warning keys on the whole registered redirect set, while the displayed host is this request's
redirect_uri. The asymmetry is deliberate:redirect_uriappears nowhere inoidc-provider's consent interaction policy, so once a Grant covers the scopes a later authorization with a different registered URI does not re-prompt. A hosted URI really can launder a loopback one past a consent already given.The consent form carries no CSRF token and does not need one. The interaction cookie is
sameSite: 'lax', which withholds it on cross-site POST, and both branches funnel through the same#getInteraction— approve viainteractionDetails, deny viainteractionFinished→interactionResult. A forged submission of either kind arrives cookie-less and lands on the 400.features.clientIdMetadataDocumentstays off on purpose.Client.find()resolves static clients → adapter → CIMD, so #152 can register a client row keyed by the URL at allowlist time and never enable the feature. Enabling it would add a request-time fetch of a client-supplied URL withallowFetch/allowClientdefaulting toasync () => true— an SSRF primitive next toDOCKER_HOST=tcp://127.0.0.1:2375. Recorded on #152.The signing key lives in
meta, notinstance_settings.listInstance()returns every instance-scoped setting, so the plan's original placement would have served the private key fromGET /api/settings/instanceand rendered it in the Settings panel.Rejected review findings
model/idout ofdoUpdateSet(row). They are the conflict target and writing them back is a no-op, but passing the whole row stays correct automatically when a column is added; an explicit list is a second place to forget one.revocationandintrospectionfeatures so their configured routes resolve. Out of scope; discovery correctly omits both endpoints, so no compliant client tries them. Commented as scaffolding.findByUid/findByUserCodereturn payloads without being given the id, so a hashed key could not restore thejtithey must carry — and more decisively, the signing key sits unsealed in the same database, so hashing the rows buys nothing beside it. Threat model now stated honestly; the sealing trade-off is #166.oauth_payloadsrows. Real, but not a correctness bug and not worth widening this PR — filed as #167 with the trap noted (a sweep must not delete consumed-but-unexpired rows, or replay detection degrades from "revoke the grant" to a plain refusal).new URL's slash-collapsing inderiveIssuer. Identical pre-existing properties ofpublic_url's own validator; consistency is the right outcome and changing the shared validator is a separate change.Not verified
Each writes its own where('model', ...) clause rather than sharing it the way expiry is shared through revive(), so removing any one of them silently collapses two models onto the same row. Also make the consumed-at rounding assertion exercise the floor by using a NOW that isn't an exact multiple of 1000, and assert the literal expected seconds instead of restating the implementation.An IPv4-mapped IPv6 loopback address (::ffff:127.0.0.1) parses to a normalised hex hostname the ANY-redirect loopback check never matched, letting it escape the consent-screen warning the same way a bare startsWith('127.') would have. decideConsent is now a standalone deny-by-default function so the security-relevant half of handleConsentSubmit — anything but the exact string "approve" denies — is unit-testable without a Provider double. Also pins that a mixed redirect set displays the first URI's host while still raising the loopback warning, so a later change to that asymmetry is deliberate rather than silent.e10387b332b2e285fc97rotateRefreshToken was configured for a token that was never issued. lib/actions/authorization/scopes.js:25-33 strips offline_access from any request that does not carry prompt=consent, and the default issueRefreshToken then requires code.scopes.has('offline_access'). Measured against the built server: scope goes in as "openid nexus:read nexus:write offline_access", the stored Grant reads "openid nexus:read nexus:write", no refresh token comes back, and the access token expires 59 minutes later with no way to renew it — so the operator re-approves hourly and the spec's own refresh behaviour ("revoking a client drops its refresh token", "an invalid refresh token returns invalid_grant", "refresh within 30s") is unreachable. issueRefreshToken now keys off the client's allowed grant types instead of a request parameter Nexus does not control, so it holds whatever Claude sends. The OIDC rule it sidesteps exists so offline access is never granted without explicit consent; Nexus satisfies that structurally, since consent.ts is the only thing that ever creates a Grant. expiresWithSession is forced false. The default returns !scopes.has('offline_access'), which — offline_access having been stripped — bound every token to the operator's browser session, revoking a headless client when they closed the tab. ttl is deliberately left at the library defaults: RefreshTokenTTL is not a constant, and for a rotating public web client with clientAuthMethod 'none' it returns the rotated token's remainingTTL so a chain still dies 14 days after the original grant. Restating that by hand to silence an advisory notice risks minting immortal refresh tokens. Proven by mutation: dropping issueRefreshToken, flipping rotateRefreshToken to false, and dropping expiresWithSession each fail a named test.