feat(notifications): let OAuth clients open the realtime stream - #239
Open
Waiel5 wants to merge 2 commits into
Open
feat(notifications): let OAuth clients open the realtime stream#239Waiel5 wants to merge 2 commits into
Waiel5 wants to merge 2 commits into
Conversation
This deployment already issues OAuth access tokens, but only `/mcp` accepted them. `/api/*` string-matched the `Bearer sk_` API-key prefix and rejected anything else, so a client could complete authorization, be granted `email:read`, and then find that the scope reached nothing. A third-party or native client had no usable path to the API at all. Tokens become a third credential alongside the session cookie and API keys. **One resolver, both surfaces.** Access tokens verify offline against the JWKS, so a signature proves only that this deployment minted the token at some point; everything that can revoke a live one — disabling the client, deleting or banning the user, removing their passkey — lives in the database and must be re-checked per request. `/mcp` already did all of that. Rather than write it twice, `resolveOAuthPrincipal` holds it and both callers use it, so the two bearer surfaces cannot drift apart. `/mcp` keeps its narrower audience and its `WWW-Authenticate` challenge; its handler loses about 2,700 characters of inline validation and behaves identically. **Scopes, not audience, are the boundary.** No `/api` audience is introduced. `@better-auth/oauth-provider` at the pinned version is subject to GHSA-p2fr-6hmx-4528, where the authorization-time resource is dropped and the token endpoint will mint a token for another allowlisted audience — so a distinct API audience would look like an authorization boundary without being one, which is worse than not having it. Scopes do the work instead, checked per route. **The policy classifies on method plus exact path.** Three routes send mail from under a router whose other routes do not — template send, sequence enroll and outbox retry — so a prefix rule would file them as `email:manage` and let a client that was never granted `email:send` send mail. Anything unclassified is denied, so adding a route without classifying it breaks an integration rather than quietly widening every existing token. **Two things are closed to tokens outright.** The credential surface (`/api/api-keys`, `/api/user/passkeys`, `/api/auth/*`), because minting an unscoped API key would convert a narrow mail consent into the user's whole account and destroy whatever key they already had. And a set of admin operations that escalate the principal or open a standing channel: rewriting inbox assignments (a token could grant itself every inbox, then read them), changing a user's role, minting or listing invite tokens, revoking OAuth clients, repointing the webhook, and setting an inbox's `forwardTo`. These are a different risk from an admin doing the same in a browser — a token is held by software, acts with no human present, and may be compromised without anyone noticing. `admin:manage` is for operating the deployment, not for rewriting who may operate it. Session and API-key callers are untouched and remain unscoped; the scope middleware returns immediately unless the request authenticated as `oauth`. Also here because the same clients need them: - `admin:manage`, never implied and required in addition to `role === "admin"`. - `GET /api/user/me`, so a client can decide whether to offer admin screens without probing an admin route and reading the 403. - `Authorization` in the CORS allow-list. Hono's default is empty, so a cross-origin client that preflighted a bearer request had the header stripped and the request blocked.
`GET /api/notifications/stream` rejects any upgrade whose `Origin` is not in `TRUSTED_ORIGINS`. A native client has no browser origin to send, so the realtime stream was unreachable for exactly the clients the OAuth surface exists to serve. The check defends against Cross-Site WebSocket Hijacking. That attack works because a browser attaches the session cookie to a cross-origin handshake by itself: an attacker's page opens the socket and the victim's credential rides along, which is why `Origin` — a header a page cannot forge — is the right control there. A bearer credential is not ambient. It has to be set explicitly on the request, and no attacker page can cause a victim's access token to be attached to a socket it opened, so the attack the check prevents cannot happen. The check now follows the credential it protects rather than the route: unchanged for session cookies, skipped for OAuth. Scoped to OAuth deliberately rather than to every bearer credential. The same argument applies to `sk_` API keys, but the suite asserts a 403 for them today and changing a tested contract is not needed to unblock a native client, so that behaviour is left alone rather than quietly widened. Also corrects this file's header, which said the 101 upgrade was not covered because it needed a real WebSocket client. The NOTIFICATIONS_HUB binding is present in the test config and the handshake does complete under miniflare — the new cases assert it.
This was referenced Aug 5, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
GET /api/notifications/streamrejects any upgrade whoseOriginis not inTRUSTED_ORIGINS. A native client has no browser origin to send, so the realtime stream is unreachable for exactly the clients the OAuth surface exists to serve.Stacked on #238 — it contains that PR's commit, so review this one's diff after it lands (or read the top commit only).
Changes
The
Origincheck now follows the credential it protects rather than the route: unchanged for session cookies, skipped for OAuth.Release impact
Minor — new backwards-compatible behaviour
Feature
Test plan
yarn test worker/src/__tests__/notifications-router.test.ts— 10 passed, including a real 101 upgradeprettier --checkcleanNotes for reviewers
The security argument, since this relaxes a control you added deliberately (
CHANGELOG.mdrecords it as CSWSH defence).Cross-Site WebSocket Hijacking works because a browser attaches the session cookie to a cross-origin handshake by itself: an attacker's page opens the socket and the victim's credential rides along.
Originis the right control there precisely because a page cannot forge it.A bearer credential is not ambient. It has to be set explicitly on the request, and no attacker page can cause a victim's access token to be attached to a socket it opened — so the attack the check prevents cannot occur for these callers, while the check itself is fatal to them.
Scoped to OAuth on purpose. The same argument applies to
sk_API keys, and I nearly extended it there — but the suite asserts a 403 for API-key callers today, and changing a tested contract is not needed to unblock a native client. Left alone rather than quietly widened. Say the word if you would rather it were consistent.One correction included: this file's header said the 101 upgrade was not covered because it needed a real WebSocket-capable client. The
NOTIFICATIONS_HUBbinding is in the test config and the handshake does complete under miniflare — the new cases assert it, and the comment is updated.Checklist
CHANGELOG.md