feat(oauth): white-label connections as a token vault (POST /v1/connections/import)#400
Merged
Merged
Conversation
…ctions/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>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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>
…strated 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>
…mport 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>
…port 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>
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>
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>
… 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>
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.
What
White-label partners that already own their OAuth (e.g. Overfolder) no longer route the OAuth dance through overslash. They run authorize + code-exchange themselves and
POST /v1/connections/importthe resulting tokens; overslash becomes a token vault — it stores the connection (identical row to an orchestrated callback), refreshes it, injects it at execution, and issues noredirect_uri. This dissolves the per-provider redirect-URI problem that the earlier per-orgoauth_redirect_url(#398) and per-request override (#388/#392) approaches kept running into.Design:
docs/design/white-label-token-vault.md· SPEC §7 · DECISIONS D20.Refresh model (fixed at import)
byoc_credential_idconnections.integration_managed = true. Overslash injects the stored token until expiry, then returnsreauth_requiredmarked integration-managed with no reconnect link (auth_urlomitted;provider+integration_managed: true) and fires aconnection.refresh_requiredwebhook. It never refreshes and never borrows the env/orgOAUTH_*_CLIENTcascade (a refresh token is valid only against its issuing client). Also covers opaque bearer tokens / PATs.Re-import is idempotent, keyed on
(identity, provider, account_email)— updates the row in place rather than accreting a duplicate each refresh cycle; a distinctaccount_emailvaults a second account.Removed (obsolete / dead once no flow can set a custom redirect)
orgs.oauth_redirect_url, theuse_org_redirectswitch,/v1/orgs/{id}/oauth-redirect-settings, the dashboard card.POST /v1/oauth/exchange+ the callback custom-redirect guard.include_raw/ the raw authorize-URL surface onPOST /v1/connections,/upgrade_scopes, MCPcreate_service, therawenvelope fields, and the MCP chat-delivery strip — partners build their own authorize URLs now.oauth_connection_flows.redirect_uri.Migration
082_connection_token_vaultaddsconnections.integration_managed, dropsorgs.oauth_redirect_url+oauth_connection_flows.redirect_uri.Dashboard
Tests
connection_import.rsdrives a fake integration partner through every path: integration-managed inject-then-reauth (real action call through a mock upstream, asserting the token is injected thenreauth_required/integration_managed: true/noauth_url), self-refresh BYOC validation, idempotent + multi-account re-import,expires_at/expires_inresolution,on_behalf_ofbinding, input validation.oauth_x/ services-auto-connect suites updated for the no-rawenvelope shape.Verification
overslash-api+overslash-dbsuite green;cargo clippyclean;cargo fmt --checkclean.npm run check+build:strictpass.vetreview: no issues found.🤖 Generated with Claude Code