[DRAFT] feat(dashboard-api): integrate ory bootstrap and user profile provider#2809
[DRAFT] feat(dashboard-api): integrate ory bootstrap and user profile provider#2809ben-fornefeld wants to merge 25 commits into
Conversation
PR SummaryHigh Risk Overview A new admin Reviewed by Cursor Bugbot for commit ad40cb0. Bugbot is set up for automated code reviews on this repo. Configure here. |
❌ 8 Tests Failed:
View the full list of 8 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The resolveOIDCUserID function contains a security vulnerability by attempting to use the OIDC subject directly as an internal UserID if it is a valid UUID. This allows an external OIDC identity to potentially collide with or target existing internal accounts, leading to unauthorized account access. OIDC subjects must always be treated as external identifiers and should be deterministically mapped to the internal namespace using the hashing logic, ensuring isolation from other user identification schemes.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ben-fornefeld
requested review from
ValentaTomas,
dobrac and
jakubno
as code owners
|
![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: 145d6bc829
ℹ️ 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".
- Change UpsertPublicIdentity to return the canonical user_id and stop overwriting it on conflict so concurrent bootstraps converge on a single (issuer, subject) -> user_id mapping. - Move identity resolution inside the bootstrap transaction; clean up the orphan candidate public.users row when a concurrent request wins the race for a fresh identity. - Reject blank oidc_user_id / oidc_user_email in PostAdminUsersBootstrap so empty strings can't share a single placeholder identity. - Export userprofile.FirstNonEmpty and drop the duplicate helper in handlers.
Use errors.New for the static empty-input error instead of fmt.Errorf.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 849be2216f
ℹ️ 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".
Document that PostAdminUsersUserIdBootstrap is the legacy Supabase-only path and PostAdminUsersBootstrap is the entry point for generic OIDC provider deployments.
The OapiRequestValidator middleware enforces the OpenAPI required list, so the previous schema rejected bootstrap calls that omitted oidc_user_name even though the handler explicitly supports a missing or blank name by defaulting to "Default Team".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04c74aa6bd
ℹ️ 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".
Lets s.userProfiles resolve members and email lookups against an Ory identity API in addition to Supabase auth.users. USER_PROFILE_PROVIDER selects supabase, ory, or supabase-ory-fallback (dual provider that prefers supabase and falls back to ory for ids missing locally). The ory provider maps internal user UUIDs to Ory subject ids via two new sqlc queries against public.user_identities (by user_ids and by subjects), scoped to the configured oidc_iss so a future multi-issuer deployment cannot cross-resolve.
The bootstrap endpoint previously assumed exactly one configured OIDC issuer and silently planted the resulting identity under that single iss. The caller now passes oidc_issuer in the request body and the handler checks it against the configured AuthProvider.JWT list, rejecting unknown issuers with 400. This both unblocks multi-issuer deployments and prevents an admin-token holder from planting an identity under an arbitrary iss.
…ovider The ory userprofile provider previously borrowed its issuer from config.AuthProvider.JWT[0].Issuer.URL and required exactly one configured jwt issuer. That conflated two unrelated concerns: which issuers we accept incoming JWTs from (a verification concern) and which oidc_iss value we use to scope public.user_identities lookups (a property of the ory tenant deployment). Introduce ORY_ISSUER_URL as an explicit config field. Profile resolution now works regardless of how many - or zero - jwt issuers are configured, and is compatible with the multi-issuer bootstrap allow-list added in 2a4264e.
For the common deployments (default Ory Network and single-endpoint self-hosted setups) the issuer is the same string as the admin SDK URL. Default the issuer to the SDK URL so those callers only need to configure one knob. Custom-domain Ory Network and split Hydra/Kratos deployments can still override ORY_ISSUER_URL explicitly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f16a246d07
ℹ️ 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".
The Ory profile provider filters public.user_identities by oidc_iss, which gets stored at bootstrap as the auth-provider issuer URL. The SDK admin URL (e.g. https://tenant.projects.oryapis.com) is not the OIDC iss claim, so defaulting to it silently strands every user once the deployment uses a custom issuer URL. - Drop the silent ORY_ISSUER_URL = ORY_SDK_URL fallback. - When AUTH_PROVIDER_CONFIG has exactly one JWT issuer, default ORY_ISSUER_URL from it. - When AUTH_PROVIDER_CONFIG has any JWT entries, reject an ORY_ISSUER_URL that doesn't match any of them so the mismatch surfaces at startup instead of as missing profile lookups.
- Capture and drain *http.Response from Ory ListIdentitiesExecute so the connection is released (bodyclose). - Realign USER_PROFILE_PROVIDER env tag (tagalign). - Update local-dev seed to the new UpsertPublicIdentity :one signature after merging main.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a4f6eaf64
ℹ️ 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".
Plumbs the four new dashboard-api configuration env vars through the .env.gcp -> Makefile -> root terraform -> nomad module -> job module chain so that make plan-only-jobs/dashboard-api picks them up. - Add USER_PROFILE_PROVIDER, ORY_SDK_URL, ORY_ISSUER_URL as plain terraform variables. - Add ORY_PROJECT_API_TOKEN as a Google Secret Manager secret with a placeholder value + ignore_changes so operators set it out-of-band. - Pass them into module.dashboard_api and inject into the nomad job env only when non-empty so the binary keeps its supabase default.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 912eb1a40a
ℹ️ 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".
| secondary, err := p.secondary.FindProfilesByEmail(ctx, email) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
In supabase-ory-fallback mode, FindProfilesByEmail always calls the secondary provider and propagates its error, so a transient Ory failure turns profile/email resolution endpoints into 500s even when the primary (Supabase) already returned results. That breaks the expected availability behavior of a fallback mode for email-based lookups.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
fine, this is temporary
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 c41c17c. Configure here.
cfg permits Ory user-profile mode with ORY_ISSUER_URL set and no AUTH_PROVIDER_CONFIG.jwt entries, but requireConfiguredOIDCIssuer only consulted JWT issuers, so the configured issuer was rejected with 400 at bootstrap time. Additionally, in Ory user-profile mode the resolver only queries public.user_identities for OidcIss = OryIssuerURL, so identities created under any other configured JWT issuer would be stranded. When Ory mode is on, restrict bootstrap to ORY_ISSUER_URL only; otherwise allow any configured JWT issuer plus ORY_ISSUER_URL when set.
The Ory profile provider's identity resolver reads public.user_identities to translate between user_id and OIDC subject. Routing that through authDB.Read uses the read replica when AUTH_DB_READ_REPLICA_CONNECTION_STRING is set, but the OIDC bootstrap path writes those rows on the primary inside a transaction, so a profile lookup immediately after bootstrap can race replication lag and return no profile. Pin identity resolution to the primary; the wider profile/team queries keep using the replica.
The Ory project standardizes on flat traits regardless of upstream OIDC provider: name, email, profile_picture_url. Extend Profile with ProfilePictureURL and read it from traits.profile_picture_url. Drop the old multi-shape name fallback chain (nested first/last, given/family, full_name) now that the trait schema is committed.
| base http.RoundTripper | ||
| } | ||
|
|
||
| func (t *oryBearerTransport) RoundTrip(req *http.Request) (*http.Response, error) { |
There was a problem hiding this comment.
🔒 Agentic Security Review
Severity: HIGH
oryBearerTransport.RoundTrip injects the PAT into every outbound request at transport level, so if the Ory endpoint responds with a redirect to a different host the follow-up request can carry the same Authorization bearer token to that new origin. This turns redirect handling into a credential exfiltration path for the admin PAT rather than constraining it to the configured Ory host.
Impact: An attacker who can influence redirects on this path can capture the Ory admin token and use it to access or mutate identity data through privileged Ory APIs.
Commits the v1 trait schema applied to the Ory project so the contract between profileFromOryIdentity and Ory is visible in-repo. Adds profile_picture_url alongside email and name to match what the resolver reads. Pointer comment added on profileFromOryIdentity so the consuming code links to the schema.
Align with the OIDC standard claim name (picture). The internal Go field stays ProfilePictureURL for descriptiveness; only the Ory trait key, fixture schema, and test cases change.
Move user-profile enrichment out of the dashboard and onto dashboard-api so list-members is a single backend call instead of a per-member Ory admin fanout. - Extend userprofile.Profile with Providers []string. - Ory provider now requests credentials via include_credential=oidc and reads upstream providers from credentials.oidc.config.providers[] (with a fallback to identifier prefixes). - Supabase provider reads providers from raw_app_meta_data for parity. - TeamMember API schema gains name, profilePictureUrl, providers. - Handler projects the new profile fields through.
# Conflicts: # packages/dashboard-api/internal/api/api.gen.go # spec/openapi-dashboard.yml
The shared authenticator logged "authorization header is missing" at WARN on every per-scheme miss, which fires on each request in Ory mode because the Supabase schemes are evaluated first and never match. Demote to a span event so the trace still records which schemes were skipped, but no log line is emitted. The 401 status stamp is preserved with an inline note explaining why ErrorHandler depends on it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5304e8c845
ℹ️ 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".
POST /admin/users/bootstrap (static) and POST /admin/users/:userId/bootstrap (parameterized) coexist at the same path level. gin's router currently handles this correctly, but a future upgrade or router fork swap could regress it. The test asserts no-panic at registration and that both URLs resolve to their intended handler.
Drop forbidden gin.SetMode call (CI sets GIN_MODE=test), switch to httptest.NewRequestWithContext, and add t.Parallel() at both levels.
ben-fornefeld
changed the title
2 participants
Summary
POST /admin/users/bootstraproute that accepts OIDC user id/email/name and bootstraps the user without a user id path param.public.usersandpublic.user_identitiesfor OIDC users before creating/reusing the default team.userprofile.Providerwhile keeping the existing/admin/users/{userId}/bootstraproute unchanged.Test plan
go test ./packages/dashboard-api/internal/userprofile ./packages/dashboard-api ./packages/dashboard-api/internal/handlers ./packages/dashboard-api/internal/cfg