fix(dashboard-api): return user_id from /admin/users/bootstrap and formalize response schema#3247
fix(dashboard-api): return user_id from /admin/users/bootstrap and formalize response schema#3247AdaAibaby wants to merge 6 commits into
Conversation
…igration - Add DB-based email fallback in GetTeamsTeamIDMembers and PostTeamsTeamIDMembers when the Ory admin API is unavailable (Hydra-only or CDN-blocked /admin/ paths). Emails are resolved from each user's default team record in public.teams. - Add email-match fallback in bootstrapUserWithIdentity for pre-Ory users. When no user_identities row exists for an incoming OIDC sub, check whether an existing user's default-team email matches the OIDC email and that user has no Ory identity yet. If so, reuse the existing user_id and write the new user_identities row, preserving API keys and template sandboxes across the auth migration. - Add user_id to PostAdminUsersBootstrap response so callers can read the canonical public.users id after bootstrap. - Add authdb.Client helpers GetUserEmailsByUserIDs and FindUserIDByEmail that query public.teams / public.users_teams directly, used by both fallback paths. Closes e2b-dev#3222 Closes e2b-dev#3223
…ion fallbacks Tests cover: - bootstrapOIDCUser email-match path reuses existing pre-Ory user account - email-match is blocked when user already has an Ory identity (prevents account hijack) - second login after email-match follows normal identity-lookup path - PostAdminUsersBootstrap response includes user_id field - GetTeamsTeamIDMembers falls back to DB email when Ory admin API is unavailable - PostTeamsTeamIDMembers falls back to DB email lookup when Ory admin API is unavailable - PostTeamsTeamIDMembers returns 404 when Ory is unavailable and email is not in DB
Three issues raised in review: 1. Propagate DB errors in email-match fallback Previously, FindUserIDByEmail and GetUserIdentitiesByUserIDs errors were silently swallowed. A transient DB fault would cause bootstrap to skip the migration path and create a new user, permanently splitting the account. Errors are now returned immediately. 2. Serialize concurrent email-based identity claims with a row lock Two concurrent bootstraps with different OIDC subs but the same email could both observe zero user_identities rows and both claim the pre-Ory user_id. Fix: take LockPublicUserForUpdate on the candidate user before re-checking identities. The losing goroutine waits, then sees the row written by the winner and falls through to new-user creation. 3. Add functional index for case-insensitive email lookup FindUserIDByEmail uses lower(t.email) = lower($1) which causes a full table scan. Migration 20260708120000 adds CONCURRENTLY an index on lower(email). Tests: add TestBootstrapOIDCUser_ConcurrentEmailMatchOnlyFirstSubMerges to verify the lock-based serialization under concurrent load.
The /admin/users/bootstrap endpoint was returning user_id in the JSON body but the spec still referenced TeamResolveResponse (id + slug only), creating a mismatch between the live contract and the generated client. Changes: - Add AdminUserBootstrapResponse schema (id, slug, user_id) to openapi-dashboard.yml - Point /admin/users/bootstrap 200 response at the new schema instead of TeamResolveResponse - Regenerate api.gen.go with the AdminUserBootstrapResponse struct - Use the typed struct in PostAdminUsersBootstrap instead of gin.H - Add -- +goose NO TRANSACTION to 20260708120000_add_teams_email_lower_index.sql so CREATE INDEX CONCURRENTLY does not fail when the migration test harness wraps migrations in a transaction block
There was a problem hiding this comment.
Code Review
This pull request introduces database fallback mechanisms for user email and ID lookups when the Ory admin API is unreachable, ensuring that team member management and user bootstrapping remain functional. It also updates the bootstrapping response to include the internal user ID and adds a database migration to index team emails. The review feedback highlights a compilation error in the test suite due to an invalid sync.WaitGroup.Go() call, and suggests refactoring the email lookup fallback to retrieve all matching user IDs instead of using LIMIT 1, thereby explicitly handling potential email ambiguity to prevent incorrect account merging or team associations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Resolves modify/delete conflict in handlers/utils_team_provisioning.go: the file was deleted upstream as part of the provisioning package refactor (PR e2b-dev#3189). All bootstrap logic has been ported to the new internal/provisioning package. PR additions ported to the new structure: - provisioning.ProvisionedTeam.UserID field for user_id in API response - Pre-Ory user migration via email-match in bootstrap.go - Non-fatal backfillIdentityExternalID for Hydra-only deployments - Updated bootstrap_test.go and ory_fallback_test.go for new interfaces - Fixed team_members.go: replace removed userprofile import with identity
6838ea0 to
a8b822d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8b822d535
ℹ️ 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".
…back
Two issues raised in review:
1. FindUserIDByEmail used LIMIT 1, silently returning an arbitrary user when
multiple accounts share the same email address. Replace with FindUserIDsByEmail
(returns all matches) and require exactly one result before acting:
- In team_members.go DB fallback: 0 -> 404, 1 -> proceed, 2+ -> 409 Conflict
- In bootstrap.go email-match migration: skip if 0 or 2+ matches, only
reuse the existing user_id when there is an unambiguous single match
2. PostTeamsTeamIDMembers DB fallback returned early before the SSO organization
check that guards the normal Ory path. When the identity provider is
unreachable, the DB fallback now fails closed for SSO-managed teams instead
of bypassing the membership check.
|
Fixed in e289eb4. Three issues addressed: Gemini high — compile error: Not a bug. was added in Go 1.25; this repo runs Go 1.26.3. Gemini/Codex P2 — LIMIT 1 ambiguity: Replaced with (returns all matches, no LIMIT). Callers now require exactly one result:
Codex P1 — SSO team bypass in DB fallback: Fixed. DB fallback now fails closed when — returns 503 instead of bypassing the org membership check. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e289eb47f5
ℹ️ 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".
Problem
The
/admin/users/bootstrapendpoint omitsuser_idfrom its declared OpenAPI response (TeamResolveResponseonly hasidandslug). The Next.js dashboard reads this field to populate the session cookie; when it is absent the session is created without auserId,getAuthContext()returns null on every request, and the user is redirected back to/sign-inin a loop — visible asERR_TOO_MANY_REDIRECTSin incognito/fresh sessions.Changes
spec/openapi-dashboard.ymlAdminUserBootstrapResponseschema with required fieldsid,slug, anduser_id/admin/users/bootstrap200 response at the new schema instead ofTeamResolveResponse(
TeamResolveResponseitself is unchanged to avoid affecting/admin/teams/bootstrapand/teams/resolve)packages/dashboard-api/internal/api/api.gen.gogo generate ./internal/api/to include the newAdminUserBootstrapResponsestructpackages/dashboard-api/internal/handlers/admin_users_bootstrap.goapi.AdminUserBootstrapResponse{Id, Slug, UserId}instead of an untypedgin.Hmappackages/db/migrations/20260708120000_add_teams_email_lower_index.sql-- +goose NO TRANSACTIONdirective soCREATE INDEX CONCURRENTLYdoes not fail when the migration test harness wraps migrations in a transaction blockTest plan
TestPostAdminUsersBootstrap_ResponseIncludesUserIDpassesgo build ./...inpackages/dashboard-apisucceeds