feat(notifications): deliver to native devices via Expo push - #241
Open
Waiel5 wants to merge 4 commits into
Open
feat(notifications): deliver to native devices via Expo push#241Waiel5 wants to merge 4 commits into
Waiel5 wants to merge 4 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.
The web UI is deployed from the same commit as the worker, so it never has to ask what the server supports. A third-party or native client is a separate artifact talking to whatever version an operator happens to be running, and had no way to find out: its only option was to attempt a capability and interpret the failure, which is indistinguishable from a bug, a misconfiguration, or a grant that was revoked. That matters most where the failure arrives late. A client can complete dynamic registration, walk the user through an authorization it cannot finish, and only discover on its first API call that this deployment does not accept tokens there — after the user has already approved something. `/api/config` now carries `apiVersion` and a `capabilities` object, so a client can check before it starts and say "this server needs a newer saasmail". The version is deliberately separate from the package version, which moves for reasons a client does not care about. Additive: existing consumers read the fields they name and ignore the rest — `src/lib/branding.tsx` type-guards each one individually.
Web Push is predicated on a browser `PushSubscription`: a push-service URL plus the P-256 ECDH public key and auth secret the `PushManager` generated, which the worker encrypts each payload to. React Native has no ServiceWorker and no `PushManager`, so a native client has no such keypair to offer — it has an opaque `ExponentPushToken` and nothing else. This is a second transport for the same event, not a variant of the existing one, and the Web Push path is unchanged. Registrations live in a new table rather than sharing `push_subscriptions`. Sharing would mean making `p256dh` and `auth` nullable, and SQLite cannot drop a NOT NULL constraint in place: it needs a create-copy-drop-rename of a live table that also carries a unique index and a foreign key. Migration 0033 is purely additive — one CREATE TABLE and two indexes, no ALTER, no DROP. Devices are keyed on a client-supplied installation id, not the token. Expo rotates tokens on reinstall and on restore to a new handset, and keying on the token would accumulate a dead row per rotation — every one of which still looks live until something tries to send to it. Re-registering updates the row in place, and the version counter moves only when the token actually changed. **Previews are off by default.** A native payload travels through Expo and then Apple or Google before reaching a lock screen, which is a different exposure from Web Push, where the payload is encrypted end-to-end to the browser. By default the notification says only that mail arrived and carries opaque routing ids; the client fetches the content over an authenticated connection. `PUSH_PREVIEWS="true"` opts in. This also matters because there is no way to authenticate the send. APNs and FCM credentials live on the Expo project that built the app, not on a self-hosted deployment, so the server POSTs to exp.host unauthenticated and Expo routes by token — which makes a token a bearer capability. Tokens are never returned by any route, and with previews off a stolen one cannot be used to read anything. Delivery is deliberately best-effort: a lost notification means the user opens the app and sees the mail, which the WebSocket stream and a pull-to-refresh already cover. It chunks to Expo's 100-message limit, stops on a 429 rather than hammering a rate limiter, and prunes only on `DeviceNotRegistered` — anything else is transient, and deleting a row over it would silence a device that is still there. Note that Expo reports per-message failure *inside* an HTTP 200, so the ticket array is read rather than the status code trusted. Two fixes fall out of this: - The VAPID guard sat at the top of the delivery path, so an unset key skipped every transport rather than just Web Push. A deployment serving only native clients now works with no VAPID configured at all. - TTL is 24 hours on the native path. The web path hardcodes 60 seconds, which is aggressive for a browser and simply wrong for a phone that is asleep, underground, or off overnight. `via: "push"` still means push was *attempted*, not that a send succeeded — that is the existing contract and callers key on it.
Owner
|
I'm not certain what this PR is trying to achieve - we already support notifications to iOS and Android devices using Web Push: see https://caniuse.com/push-api for compatibility. |
Contributor
Author
|
this is for the mobile app I was adding. You can check it here https://github.com/Waiel5/saasmail-mobile. All things are basically to ensure it would be ready for a mobile app, currently still working on perfecting it |
Owner
|
Appreciate you working on a mobile app for SaaSmail! This is super cool. At this time, we do not want to store expo tokens within saasmail. Wondering what the benefits are of having a mobile app vs just installing the web app to the Home Screen on iOS and Android |
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
2 participants
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
Web Push is predicated on a browser
PushSubscription— a push-service URL plus the P-256 keypair thePushManagergenerated, which the worker encrypts each payload to. React Native has no ServiceWorker and noPushManager, so a native client has no such keypair to offer; it has an opaqueExponentPushTokenand nothing else. This is a second transport for the same event, not a variant of the existing one. The Web Push path is unchanged.Stacked on #238, #239, #240.
Changes
expo_push_subscriptionstable (migration0033, purely additive) andPOST/DELETE /api/notifications/expo/subscribe.NotificationsHub.PUSH_PREVIEWS="true"to opt in.Release impact
Minor — new backwards-compatible behaviour
Feature
Test plan
yarn test worker/src/__tests__/expo-push.test.ts— 18 passedyarn db:generateproduced0033; verified it contains noALTER,DROPorRENAMEprettier --check .cleanNotes for reviewers
Why a separate table. Sharing
push_subscriptionsmeans makingp256dhandauthnullable, and SQLite cannot drop a NOT NULL constraint in place — it needs a create-copy-drop-rename of a live table that also carries a unique index and a foreign key. That is the riskiest migration in the repo for the least benefit. The new table is oneCREATE TABLEand two indexes.Why installation id rather than token. Expo rotates tokens on reinstall and on restore to a new handset. Keying on the token accumulates a dead row per rotation, and each one still looks live until something tries to send to it. Re-registering updates the row in place; the version counter moves only when the token actually changed, so it tracks rotations rather than app launches.
Previews default to off, and I'd argue that's the right default for this product. A native payload passes through Expo and then Apple or Google before it reaches a lock screen — a different exposure from Web Push, which is encrypted end-to-end to the browser. By default the notification says only that mail arrived and carries opaque routing ids.
PUSH_PREVIEWS="true"opts in per deployment.That matters more than usual here because the send cannot be authenticated. APNs and FCM credentials live on the Expo project that built the app, not on a self-hosted deployment, so the server POSTs to
exp.hostunauthenticated and Expo routes by token — which makes a token a bearer capability for anyone who obtains one. Tokens are stored server-side and never returned by any route, and with previews off a stolen token cannot be used to read anything. If your Expo project ever enables enhanced push security, unauthenticated sends stop working and this needs an access token; that is a deliberate either/or rather than something I've papered over.Deliberately best-effort. No durable job ledger, no cross-request retry. A lost notification means the user opens the app and sees the mail — the WebSocket stream and pull-to-refresh already cover that, and building a delivery-guarantee system for a wake-up ping seemed like the wrong trade. It chunks to Expo's 100-message limit, stops on a 429 rather than hammering a rate limiter, and prunes only on
DeviceNotRegistered. Receipts (Expo's delayed second signal) are not polled — worth adding if delivery problems show up, but tickets catch the case that matters, which is a dead token.One thing worth knowing if you review the code: Expo reports per-message failure inside an HTTP 200, so the ticket array has to be read rather than the status code trusted. There is a test for exactly that.
Two fixes fall out:
via: "push"still means push was attempted, not that a send succeeded — I changed that briefly and the existing test caught it.Checklist
yarn db:generate) —0033, additive onlyCHANGELOG.mdPUSH_PREVIEWSinwrangler.jsonc.example