refactor(auth): split session and admin contracts behind neutral types#338
Conversation
Introduce a provider abstraction for server auth reads so the dashboard can move off direct Supabase session access ahead of the Ory migration.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview What looks wrong: setting Reviewed by Cursor Bugbot for commit 97f45f0. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “session boundary” for dashboard auth by adding an AuthSessionProvider abstraction (with a Supabase-backed implementation) and routing key server-side session/auth reads through a small facade. This sets the codebase up to swap the underlying auth provider later (e.g., Ory) while keeping current behavior mostly consistent.
Changes:
- Added an
AuthSessionProviderabstraction plus a Supabase implementation (SupabaseAuthSessionProvider) and a selector + facade API (getAuthContext,signOut, etc.). - Removed
getSessionInsecureand updated middleware/routes/server actions to use the new auth facade/provider. - Updated proxy middleware-like auth detection to use the new provider and added safer env var handling.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/proxy.ts | Uses the new session provider to determine auth state; adds env var guards. |
| src/core/server/trpc/init.ts | Replaces Supabase Session typing with a minimal session shape in TRPC context. |
| src/core/server/functions/auth/get-session.ts | Deletes getSessionInsecure helper. |
| src/core/server/auth/session.ts | Adds facade functions for auth context/access token/sign-out. |
| src/core/server/auth/session.supabase.ts | Adds Supabase-backed AuthSessionProvider implementation. |
| src/core/server/auth/session-selector.ts | Adds provider selection entrypoint (currently always Supabase). |
| src/core/server/auth/session-provider.ts | Adds base abstraction + MissingAuthSessionError. |
| src/core/server/api/middlewares/auth.ts | Switches TRPC auth middleware to use the new provider. |
| src/core/server/actions/client.ts | Switches safe-action auth to use getAuthContext and a minimal session shape. |
| src/core/server/actions/auth-actions.ts | Routes sign-out through facade; tweaks auth provider health check env usage. |
| src/app/sbx/new/route.ts | Uses getAuthContext() instead of direct Supabase calls + insecure session reads. |
| src/app/dashboard/route.ts | Uses getAuthContext() + signOut() facade for dashboard redirect logic. |
| src/app/dashboard/account/route.ts | Uses getAuthContext() + signOut() facade for account redirect logic. |
| src/app/dashboard/[teamSlug]/layout.tsx | Uses getAuthContext() to obtain token for user resolution. |
| src/app/dashboard/(resolvers)/inspect/sandbox/[sandboxId]/route.ts | Uses getAuthContext() for auth gating and token sourcing. |
| src/app/api/teams/[teamSlug]/metrics/route.ts | Uses getAuthContext() for auth gating and token sourcing. |
| src/app/(auth)/auth/cli/page.tsx | Uses getAuthContext() to supply email + token for CLI callback. |
Comments suppressed due to low confidence (1)
src/core/server/auth/session.supabase.ts:30
client.auth.getSession()is called without the previous warning suppression that existed ingetSessionInsecure, so Supabase's “getSession() is potentially insecure” warnings will likely reappear in logs. If the goal is still to avoid noisy production logs, consider reintroducing a narrowly-scoped suppression here (or switching to an approach that doesn’t callgetSession()when only presence/identity is needed).
const {
data: { session },
} = await client.auth.getSession()
if (!session?.access_token) {
return null
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b67f816e08
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Match the new session provider behavior in proxy integration tests by mocking both user and session reads.
Avoid silently bypassing proxy auth when Supabase env validation should already guarantee required values.
Use the existing process env typing directly in proxy Supabase client setup.
Replace standalone session helper exports with an authProvider facade and factory for injected auth clients.
Use an authContext getter on auth providers so call sites read from the selected provider object directly.
Align the provider API by using async getters for both auth context and access token.
Introduce typed ManagedSupabase and HostedOry provider adapters, and route Supabase auth SDK usage through the provider boundary.
Restructure the auth module into a flag-switchable provider contract that no longer leaks Supabase types into the app. Introduces an `AuthProvider` contract for session reads, an `AuthAdmin` contract for user lookups, and a neutral `AuthUser` model so consumers stop depending on `@supabase/supabase-js` shapes. Provider and admin implementations live under `supabase/` and `ory/` (stub) and are selected via the `AUTH_PROVIDER` flag in a single place.
Removes the leftover supabase/client.ts that was superseded by server-client.ts, deletes the unused createdAt field on AuthUser, collapses flags.ts to just isOryAuthEnabled(), and surfaces the Ory stubs as rejected promises so callers see consistent failure shapes instead of synchronous throws.
ben-fornefeld
changed the title
- Bake the resolved AuthUser into AuthContext so middleware/layouts no longer need a second admin getUser round-trip per authenticated request. - Make Ory stubs return null/empty instead of throwing so flag-flips can't fall through proxy's catch-all into protected routes. - Validate NEXT_PUBLIC_SUPABASE_URL alongside the anon key in the auth health probe.
Removes the empty ory/urls.ts stub and demotes type-only re-exports and internal builders that nothing imports today. Will be reintroduced when Ory hosted UI is wired.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 54af3c7. Configure here.
…ession-boundary-eng-3881 # Conflicts: # src/core/server/api/middlewares/auth.ts
…ession-boundary-eng-3881 # Conflicts: # src/core/server/actions/user-actions.ts
The method had three definition sites (interface + supabase + ory) and zero callers since AuthContext started carrying the resolved AuthUser directly. Cuts the per-provider implementation surface and removes a trap for the next contributor adding an Ory provider.
The new auth boundary (PR #338) swallowed Supabase failures and returned null, leaving the proxy, tRPC middleware, and route handlers unable to distinguish a real outage from a legitimate logged-out user. - Log getUser / getSession / signOut / health-check failures with stable keys (auth_provider:*) so prod issues are immediately visible. - Switch AuthProvider.signOut to return { error } instead of throwing, matching Supabase's value-style errors; updateUserAction now logs update_user_action:sign_out_others_failed when post-password-change session invalidation fails. - Log auth_admin:get_user_by_id:error and auth_admin:get_emails_by_ids:error inside supabaseAuthAdmin so degraded member listing is no longer silent. - Add email_callback:supabase_error so failed email changes are traceable. - Add trpc_auth_middleware:no_session warn alongside the existing span status, restoring boundary-level logging dropped during the refactor. - Make the Ory stub provider loud per request and return a stub error value instead of rejecting. - Add unit tests for the provider and admin logging paths.
Squashed baseline of ENG-3881 work to start ENG-4125 (Auth.js + hosted Ory) from a clean history. Will rebase onto main once PR #338 lands. Behaviorally identical to PR #338 / ENG-3881 latest: - AuthProvider (session) and AuthAdmin (lookups) interfaces with neutral AuthUser / AuthContext / SignOutOptions types - supabase/ implementation under provider, admin, flows, server-client, user converter; logging via auth_provider:* / auth_admin:* keys - ory/ stubs that fail-closed (return null / stub error) - AUTH_PROVIDER flag selects provider in src/core/server/auth/index.ts - All ~30 consumers migrated to auth.* / authAdmin.* / supabaseAuthFlows.* - User type from @supabase/supabase-js no longer leaks outside supabase/ - Single auth roundtrip per authenticated request (AuthContext carries user) - signOut returns { error } shape; observability hooks wired across middleware, actions, and route handlers
Summary
AuthProvider(session) andAuthAdmin(lookups) contracts with a neutralAuthUsertype.supabase/providers and flows.ory/stubs and anAUTH_PROVIDERflag inindex.tsso the provider is swapped in one place.