feat(api): list the inboxes the caller may send from - #242
Open
Waiel5 wants to merge 3 commits into
Open
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/admin/inboxes` is admin-only, and it was the only inbox list. A member therefore had no way to discover a `fromAddress` that `POST /api/send` would accept — a client could hardcode one, or build a picker by guessing and let the user find out from a 403. Neither is a contract, and it makes composing from a non-admin account impossible to implement correctly. `GET /api/inboxes` returns the caller's own sending identities: every inbox for an admin, the assigned ones for a member. It is built from the same universe the admin route builds (received recipients plus configured sender identities) and narrowed with `inboxScopeSql`, the same resolver the send route enforces with. That equivalence is the point and is asserted directly: a test sends from a listed address and an unlisted one and checks the pair 200s and 403s respectively, so the two cannot drift into a picker that offers addresses which fail. The response carries display name, display mode and signature — what a client needs to render and label a From row. It omits `forwardTo` and `assignedUserIds`: the first is a standing relay destination and admin configuration, the second discloses which other users hold an inbox. Tests cover the member path explicitly. `createTestUser` defaults to `role: "admin"` and `resolveAllowedInboxes` short-circuits for admins, so a suite that omits that setup asserts an unrestricted caller sees everything and passes without exercising the scoping at all. The empty grant is covered too: a member with no assignments must get `[]`, and an inlined `IN ()` is a SQLite syntax error, so the naive implementations fail either loudly or open. Classified `email:read` for OAuth callers. It discloses addresses the caller already controls and sends nothing, and unclassified routes are denied outright — leaving it unclassified would mean an OAuth client could read mail but never find a From address.
`GET /api/inboxes` declares its response with `.openapi("Inbox")`, which
registers a fifth named component. This assertion pins the exact set, so
adding a named schema fails it by design — extending the list is the
intended maintenance, not a weakening of the check.
Naming it is deliberate: an inlined response shape is anonymous in /doc,
which is worse for the integrators the route exists to serve.
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.
Stacked on #238 — the last commit here is the only new one; the scope-policy entry belongs with the route it governs, so it ships alongside rather than after. Review the tip commit.
The gap
GET /api/admin/inboxesis admin-only, and it was the only inbox list in the API.A member therefore had no way to discover a
fromAddressthatPOST /api/sendwould accept. A client could hardcode one, or build a From picker by guessing and let the user find out from a 403. Neither is a contract, and it makes composing from a non-admin account impossible to implement correctly — which is how I hit it, building a From picker in a native client.The route
GET /api/inboxesreturns the caller's own sending identities: every inbox for an admin, the assigned ones for a member.It is built from the same universe the admin route builds — received recipients
UNIONconfigured sender identities — and narrowed withinboxScopeSql, the same resolverPOST /api/sendenforces with. That equivalence is the whole point of the route, so it is asserted directly rather than assumed: one test sends from a listed address and an unlisted one and checks the pair does not 403 / does 403. The two cannot drift into a picker that offers addresses which fail.The response carries
displayName,displayModeandsignatureHtml— what a client needs to render and label a From row. It deliberately omits two fields the admin route returns:forwardTois a standing relay destination and admin configuration.assignedUserIdsdiscloses which other users hold an inbox.Tests
10 tests,
worker/src/__tests__/inboxes-router.test.ts.The member path is set up explicitly in every scoping test.
createTestUserdefaults torole: "admin"andresolveAllowedInboxesshort-circuits onisAdmin, so a suite that omits that setup asserts an unrestricted caller sees everything — green, and exercising none of the scoping.The empty grant gets its own test. A member with no assignments must get
[], and both plausible shortcuts fail: an inlinedIN ()is a SQLite syntax error (500), and skipping the filter when the list is empty scopes nothing and returns every inbox on the deployment.inboxScopeSqlis the one place that renders it as a false predicate, which is why the query goes through it rather than a hand-rolledIN.Also covered: case-insensitive matching against grants stored before insert-time canonicalization, the
chatdefault for an inbox with nosender_identitiesrow, 401 unauthenticated, and the scope classification.Scope
Classified
email:read. It discloses addresses the caller already controls and sends nothing, and a read-only client still needs it to label a message with the inbox that received it. Unclassified routes are denied outright by #238, so leaving it out would let an OAuth client read mail but never find a From address.Verification
(
inboxes-router,oauth-api-access,inbox-permission-enforcement.)yarn tsc --noEmitclean.