fix(ios): make the organization explicit everywhere the app enters a screen - #545
Open
Makisuo wants to merge 3 commits into
Open
fix(ios): make the organization explicit everywhere the app enters a screen#545Makisuo wants to merge 3 commits into
Makisuo wants to merge 3 commits into
Conversation
…screen The app treated "which organization" as ambient state. It lived only in the Clerk session token's active-organization claim, and nothing else carried it, so every entry point resolved against whichever organization happened to be selected. The reported symptom was notifications. `MobilePushService` has always sent `maple_org_id`; nothing on the device read it, so tapping an alert for org B while org A was active pushed the incident straight onto the Alerts stack, where the request went out under org A's token and came back 404 — the app said the incident did not exist. Home Screen widgets had the same shape: one snapshot per surface, re-pointed silently on every switch. Push, widget and Live Activity taps now go through one entry point, `DestinationOpener`, which switches organization *before* navigating. Order matters: `select` bumps `dataGeneration` and every detail screen keys its load on it, so pushing first would build the screen under the old generation, fire the 404, and only then re-run — the user would watch the bug go past. A short toast acknowledges the switch; an organization the user has left is refused rather than opened onto an error screen. The subtle case is cold start. A tap launches the app and fires `didReceive` before `RootView` has loaded memberships, so a membership-first ordering would tell every cold cross-organization tap that the user is not a member. `DestinationResolver` parks instead and `RootView` re-asks once the session settles; it is a pure function in `MapleWidgetData` with a test per rule, because the app target has no test bundle. Widgets can now be pinned to an organization. A Clerk token carries exactly one, and `setActive` is global session state the foreground is using, so the API had to accept an explicit one: `x-maple-org-id`, verified against the caller's Clerk memberships. Two invariants hold it together. Naming the organization you already have is free — no verifier call — which is what lets a client send the header unconditionally instead of branching. And everywhere membership cannot be proven (self-hosted, `MAPLE_ORG_ID_OVERRIDE`, API keys, no verifier wired) the header is a 403, never a silent ignore: ignoring it is precisely the failure where a widget renders one organization's incidents under another's name and nobody notices. Membership lookups are cached per *user*, never per (user, organization). The organization arrives in a request header, so a per-pair key would turn that header into a Clerk-request amplifier; caching the whole set gives negative answers for free and makes header rotation cost nothing. The shared TTL is the revocation lag — five minutes, documented on the constant, with a Clerk webhook the proper fix. On the device, snapshots move to per-organization keys, `IssuesWidget` becomes an `AppIntentConfiguration` (the `kind` string is untouched, so placed widgets migrate rather than disappear), and the publish set is driven by which organizations actually have a widget placed, capped at three. Publishing every membership would be 48 requests a round for an account in twelve, most of them for organizations nobody pinned — and iOS answers that appetite with less background time, so the widgets would end up less current, not more. `IncidentActivityAttributes` gains an optional `organization_id`, and optional is load-bearing: attributes are the static half of a Live Activity, so one already running can never gain the field, and a required one would make iOS silently drop every start push from a server that has not deployed yet. Both sides tolerate its absence, so they can ship in either order.
`ios:openapi:check` regenerates the pruned spec and compares, so the two failures `AuthorizationV2` gained — the 403 for an unverifiable organization selection and the 503 when the membership directory cannot be reached — have to appear on every v2 operation's documented responses. Description text only; no schema or operation changed, so the generated Swift client is unaffected.
Unrelated to this branch: `anti-slop(no-reflect-get)` has been failing the lint job on main since a4973a8, and it blocks every PR that runs the whole-repo lint. The rule's advice — parse dynamic input into a named domain type first — does not apply to a function whose entire job is describing an arbitrary thrown value, so the fix is the narrowing that does the same thing without the reflection: `"message" in error` checks the prototype chain exactly as `Reflect.get` walked it, and does not invoke a getter. The read that can throw stays inside the try, which is the property the original comment was protecting. No behaviour change; `describeThrown`'s invariant test still passes.
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.
Why
The mobile app treated which organization as ambient state: it lived only in the Clerk session token's active-organization claim, and nothing else carried it. Every entry point therefore resolved against whichever organization happened to be selected.
The reported symptom was notifications.
MobilePushServicehas always sentmaple_org_id— nothing on the device read it. Tapping an alert for org B while org A was active pushed the incident onto the Alerts stack, the request went out under org A's token, and the app said the incident did not exist. Home Screen widgets had the same shape: one snapshot per surface, silently re-pointed on every switch. Live Activities andmaple://links carried no organization at all.What changed
Push / widget / Live Activity taps now go through one entry point,
DestinationOpener, which switches organization before navigating. Order matters:selectbumpsdataGenerationand every detail screen keys its load on it, so navigating first would build the screen under the old generation, fire the 404, and only then re-run — the user would watch the bug go past. A short toast acknowledges the switch; an organization the user has left is refused rather than opened onto an error screen.The subtle case is cold start: a tap launches the app and fires
didReceivebeforeRootViewhas loaded memberships, so a membership-first ordering would tell every cold cross-organization tap that the user is not a member.DestinationResolverparks instead, andRootViewre-asks once the session settles. It is a pure function inMapleWidgetDatawith a test per rule — the app target has no test bundle, so anything worth testing has to live in the package.Widgets can be pinned to an organization. A Clerk token carries exactly one, and
setActiveis global session state the foreground is using, so the API had to accept an explicit one:x-maple-org-id, verified against the caller's Clerk memberships.Also:
maple://links carry?org=(a query item, so every notification already in Notification Center and every running activity still parses), andIncidentActivityAttributesgains an optionalorganization_id.Review notes
Three decisions worth a second opinion:
The header is a 403 wherever membership cannot be proven — self-hosted,
MAPLE_ORG_ID_OVERRIDE, API keys, no verifier wired — never a silent ignore. Silently ignoring it is precisely the failure where a widget renders one organization's incidents under another's name and nobody notices. Naming the organization you already have stays a free no-op (verifier not called), which is what lets a client send the header unconditionally.Membership is cached per user, never per (user, organization). The organization arrives in an attacker-controllable header, so a per-pair key would make it a Clerk-request amplifier; the whole-set key gives negative answers for free and makes header rotation cost zero outbound calls. Revocation lag is the shared TTL — 5 minutes, documented on the constant. A Clerk
organizationMembership.deletedwebhook callingedgeCache.invalidateis the proper fix and is not in this PR.organization_idon the Live Activity must stay optional. Attributes are the static half of an activity, so one already running can never gain the field, and a required one would make iOS silently drop every start push from a server that has not deployed yet. Both sides tolerate its absence, so they ship in either order.Nothing that iOS persists was renamed:
IssuesWidgetKind.identifier,SelectServiceIntent(gained a parameter rather than being replaced),ServiceEntity.id, and theIncidentActivityAttributestype name are all untouched — each one silently orphans or unconfigures something on a user's device.IssuesWidgetmoves fromStaticConfigurationtoAppIntentConfiguration; placed widgets are migrated by iOS rather than removed, and a one-shotreloadAllTimelinesper new build makes them resolve an organization immediately instead of an hour later.The publish budget is capped at three organizations, driven by which ones actually have a widget placed. Publishing every membership would be 48 requests a round for an account in twelve, mostly for organizations nobody pinned — and iOS answers that appetite with less background time, so the widgets would end up less current, not more.
Verification
bun typecheck— clean, 40/40 packagesbun run --cwd apps/api test— 2197 passedbun run --cwd packages/auth test— 68 passed (new: no-op costs no verifier call, role adoption, no-active-org cold case, and a 403 for each rejecting credential)bun run --cwd packages/domain test— 539 passedbun run --cwd apps/electric-sync test— 71 passedswift testinapps/ios/Packages/MapleAPI— 81 passedNot run: the manual simulator matrix (cold-start tap on a foreign-org push, two widgets pinned to different organizations, a widget placed before the update). It needs a signed build and a two-organization test account.
Lint reports one pre-existing error in
apps/cli/src/server/serve.ts, untouched here.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.