feat(oauth): per-org oauth_redirect_url + opt-in white-label switch#398
Merged
Conversation
Replace the per-request white-label `redirect_uri` override and the
`oauth_callback_allowed_hosts` allow-list with a single admin-set per-org
`oauth_redirect_url`, opted into per request via a `use_org_redirect` bool.
Why a per-request opt-in (not a global per-org rule): a global override would
break overslash's own dashboard connect flows ("Try it"/Connect/reconnect/
upgrade-scopes), which rely on the default `/v1/oauth/callback` so the browser
completes the flow in place. The bool defaults to false, so those flows are
untouched in white-label orgs; only the partner sets `use_org_redirect: true`.
Backend:
- migration 081: add `orgs.oauth_redirect_url`, drop `oauth_callback_allowed_hosts`
- kernel: `use_org_redirect` resolves the org URL (400 if unconfigured), else the
default callback; flow-row contract unchanged (Some -> exchange, NULL -> callback)
- drop the per-request `redirect_uri` fields on `POST /v1/connections`,
`/upgrade_scopes`, and MCP `create_service` (now `use_org_redirect` /
`connect_use_org_redirect`); remove `callback_host_allowed`
- settings API: `GET/PATCH /v1/orgs/{id}/oauth-redirect-settings` (admin, audited,
URL validated on write via the shared `parse_redirect_uri`)
- keep `/v1/oauth/exchange`, `include_raw`/wrap-raw, default callback path
Dashboard:
- org settings: replace the OAuth callback hosts list with a single OAuth
redirect URL input; the dashboard's own Connect flows still use the default
callback
Tests: rewrite as `oauth_org_redirect_url.rs` (org-url-as-redirect, missing-URL
400, default-callback fallback, exchange/guard, upgrade-scopes, settings API).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Rename the stale 'OAuth callback hosts' screenshot script/helper to the new single 'OAuth redirect URL' card (setOauthRedirectUrl → oauth-redirect-settings). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
Author
Dashboard screenshotOrg Settings → OAuth redirect URL card, captured against the real e2e stack (
The single URL input replaces the former "OAuth callback hosts" allow-list. The dashboard's own Connect flows are unchanged — they never opt into |
angel-manuel
added a commit
that referenced
this pull request
Jun 15, 2026
…ctions/import) (#400) * feat(oauth): white-label connections as a token vault (POST /v1/connections/import) Partners that already own their OAuth (e.g. Overfolder) no longer route the dance through overslash. They run authorize + code-exchange themselves and `POST /v1/connections/import` the resulting tokens; overslash stores the connection (identical row to an orchestrated callback), refreshes it, and injects it at execution — and issues no `redirect_uri`. This dissolves the per-provider redirect problem entirely (overslash issues no redirect URI). Refresh mode is fixed per import: - self-refresh: a pinned `byoc_credential_id` (validated at import — a bad id 400s here, not at first refresh); overslash refreshes via the refresh-token grant, hard-pinned to that client (never the cascade). - integration-managed: a null `byoc_credential_id` flags `connections.integration_managed`. Overslash injects the stored token until expiry, then surfaces `reauth_required` marked integration-managed with NO reconnect link (auth_url omitted, provider + integration_managed: true) and fires a `connection.refresh_required` webhook — the partner refreshes and re-imports. It never refreshes and never borrows the env/org `OAUTH_*_CLIENT` cascade (a refresh token is valid only against its issuing client). Also covers opaque bearer tokens / PATs with no client at all. Re-import is idempotent, keyed on (identity, provider, account_email) — it updates the row in place rather than accreting duplicates each refresh cycle; a distinct account_email vaults a second account. Removed (now obsolete / dead once no flow can set a custom redirect): - all of #398: `orgs.oauth_redirect_url`, the `use_org_redirect` switch, the `/v1/orgs/{id}/oauth-redirect-settings` endpoints, the dashboard section. - `POST /v1/oauth/exchange` + the callback custom-redirect guard. - `include_raw` / the raw authorize-URL surface on `POST /v1/connections`, `/upgrade_scopes`, MCP `create_service`, the `raw` envelope fields, and the MCP chat-delivery strip — partners build their own authorize URLs now. - `oauth_connection_flows.redirect_uri`. Migration `082_connection_token_vault` adds `connections.integration_managed` and drops `orgs.oauth_redirect_url` + `oauth_connection_flows.redirect_uri`. Dashboard: connection detail surfaces the integration-managed credential source; org settings drops the OAuth redirect URL card. Tests: new `connection_import.rs` drives a fake integration partner through every path (integration-managed inject-then-reauth via a mock upstream, self-refresh BYOC validation, idempotent/multi-account re-import, expiry resolution, on_behalf_of binding, input validation). Reauth/oauth_x/ services-auto-connect suites updated for the no-`raw` envelope shape. Design: docs/design/white-label-token-vault.md; SPEC §7; DECISIONS D20. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(oauth): reject refresh-mode changes on connection re-import Seer review (HIGH): on re-import, a supplied `byoc_credential_id` that would change a connection's refresh mode was validated and then silently discarded (the update path only writes tokens/scopes), so a caller expecting self-refresh on an integration-managed row got a misleading 200 with the old mode. The mode is fixed at first import by design. Make that contract explicit: - omitting `byoc_credential_id` stays a token-only update preserving the mode (the hot path for integration-managed re-import); - supplying a `byoc_credential_id` that would flip the mode (integration-managed → self-refresh) or re-pin to a different client now returns 400 with a clear "delete and re-import to change" message. Adds a regression test (integration-managed → self-refresh re-import → 400) and documents the behavior in the design doc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(oauth): make import match mode-aware so it never overwrites orchestrated connections Seer review (HIGH): an emailless import resolves the re-import target via the `(identity, provider)` fallback in `find_for_import`, which matches on NULL account_email. An orchestrated connection whose userinfo fetch left account_email NULL could therefore be matched and have its tokens overwritten — and the previous mode guard only fired when `byoc_credential_id` was supplied, so an integration-managed import slipped through. Make the match mode-aware: - email-keyed match (caller named the account): an in-place update is intended, so a mode/client change is rejected with 400 (the prior fix, generalized). - emailless heuristic match: reuse the row ONLY when it is the same kind of vault connection (same mode + same pinned client). On a mismatch — notably an orchestrated connection — fall through to creating a fresh row instead of clobbering it. Adds a regression test (emailless integration-managed import leaves a NULL-email orchestrated connection untouched and creates a separate row) and documents the match semantics in the design doc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(oauth): preserve token_expires_at on a token-only connection re-import Seer review (MEDIUM): a re-import that carries no fresh `expires_at`/`expires_in` passed `None` to `update_tokens_and_scopes`, nulling `token_expires_at`. For an integration-managed connection that makes it look perpetually valid — it would never surface `reauth_required` and would keep injecting a token that has actually expired upstream. Fix surgically in the import kernel (not the shared repo fn the orchestrated upgrade callback also uses): on a re-import, fall back to the existing `token_expires_at` when the caller supplies no fresh expiry; a supplied expiry still overrides it. Adds a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(oauth): preserve existing scopes on a token-only connection re-import Seer review (CRITICAL): `ImportConnectionInput.scopes` defaults to `[]`, and the update path overwrote `scopes` unconditionally — so a token-only re-import that omitted `scopes` wiped the connection's granted scopes, 403ing every subsequent scope-gated action call. Mirror the expiry fix: on a re-import, preserve the existing scopes when the caller supplies none; a non-empty `scopes` still overrides. The effective scope set now also flows to the response, audit detail, and webhook payload. Adds a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(oauth): skip upgrade-URL mint for integration-managed missing_scopes Seer review (LOW): when an action needs a scope an integration-managed connection lacks, `check_required_scopes` called `mint_upgrade_auth_url` — which an integration-managed connection can't use (Overslash holds no client), doing wasted work and leaving a stray `oauth_connection_flows` row. Guard it: for integration-managed connections, return `missing_scopes` with `auth_url`/`short` omitted and no mint — the integration broadens the grant and re-imports. Adds a regression test (403 missing_scopes, no auth_url, zero flow rows created). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(oauth): reject /upgrade_scopes for integration-managed connections Seer review (MEDIUM): `POST /v1/connections/{id}/upgrade_scopes` didn't guard against integration-managed connections, so it would call kernel_create_connection and mint an orchestrated OAuth flow (incorrect when a fallback client exists, a generic 400 when not). The companion `check_required_scopes` fix already skips the mint for these connections, but the REST endpoint — which the missing_scopes `upgrade_url` points at — needed the same guard. Guard the handler: an integration-managed connection returns a clear 400 directing the caller to broaden the grant and re-import via POST /v1/connections/import. Adds a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(oauth): nullable connection scopes — unknown gets benefit of the doubt `connections.scopes` was `NOT NULL DEFAULT '{}'`, so an import that omitted `scopes` recorded an empty set — indistinguishable from "genuinely no scopes" — and the action scope-gate then 403'd `missing_scopes` on a token that was actually valid (Overslash can't know an imported token's real grants). Make scopes nullable to represent "unknown" distinctly (migration 083_connection_scopes_nullable; `ConnectionRow.scopes: Option<Vec<String>>`): - `POST /v1/connections/import` `scopes` now defaults to `null` (unknown), not `[]`. Re-import preserve-on-omit keeps the recorded set. - The scope-gate (`check_required_scopes`) and the dashboard credential-health badge (`derive_credentials_status` via a new `ScopeKnowledge` enum) treat `null` as benefit of the doubt — covering everything — so unknown-scope imports aren't falsely 403'd; a real shortfall still surfaces upstream. Orchestrated flows always record the concrete granted set, so they keep precise gating. - The `missing_scopes` envelope now reports both `required` (the action's full set) and `missing` (the delta), so a caller sees the target, not just the gap. Tests: import-without-scopes → recorded null + action executes (benefit of the doubt); known-but-insufficient scopes → 403 with required+missing; re-import preserves scopes; `ScopeKnowledge::Unknown` classifies Ok. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Factory <factory@overslash.dev> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.
Context
White-label integrations (consumer: overfolder) currently pick the OAuth provider
redirect_uriper request by passing aredirect_uristring, gated by a per-org allow-list (oauth_callback_allowed_hosts, #388/#392). A white-label org has exactly one partner-hosted callback, so an admin-set per-org URL is sufficient — the caller never needs to send (or host-validate) a URL on every request.This collapses both knobs into:
oauth_redirect_url(the partner callback), managed in Org Settings.use_org_redirectthat opts a flow into that org URL.Why a per-request opt-in (not a global per-org rule)
A global override would break overslash's own dashboard connect flows ("Try it"/Connect/reconnect/upgrade-scopes), which rely on the default
/v1/oauth/callbackso the browser completes the flow in place. Those flows send no redirect/white-label fields, so an opt-in bool (default = overslash callback) leaves them untouched in white-label orgs. Only the partner setsuse_org_redirect: true.The flow-row contract is unchanged:
flow.redirect_uri = Some(org_url)⇒ white-label ⇒ complete viaPOST /v1/oauth/exchange;NULL⇒ default ⇒ browserGET /v1/oauth/callback./v1/oauth/exchange,include_raw/wrap-raw, and the default callback path are all kept verbatim.Changes
Backend
081: addorgs.oauth_redirect_url, dropoauth_callback_allowed_hosts.platform_connections.rs):use_org_redirectresolves the org URL (400 if unconfigured/invalid) else the default callback;parse_redirect_urisimplified;callback_host_allowedremoved.redirect_urionPOST /v1/connections&/upgrade_scopes, andconnect_redirect_urion MCPcreate_service— nowuse_org_redirect/connect_use_org_redirect.GET/PATCH /v1/orgs/{id}/oauth-redirect-settings(admin-only, audited, URL validated on write via the sharedparse_redirect_uri).Dashboard
Testing
oauth_org_redirect_url.rs(rewritten, 13 tests): org-url-as-redirect,use_org_redirectwith missing URL → 400, default-callback fallback (no flag), exchange/callback guards, white-label upgrade-scopes, and the settings API (round-trip, clear, invalid-URL 400, admin-only).cargo clippyclean; dashboardsvelte-check0/0 and strict build pass.vet(agentic) reviewed the diff: no issues.🤖 Generated with Claude Code