diff --git a/docs/superpowers/specs/2026-08-03-multi-auth-providers-design.md b/docs/superpowers/specs/2026-08-03-multi-auth-providers-design.md index c96a505..5b1df95 100644 --- a/docs/superpowers/specs/2026-08-03-multi-auth-providers-design.md +++ b/docs/superpowers/specs/2026-08-03-multi-auth-providers-design.md @@ -48,7 +48,7 @@ type AuthProvider struct { ClientSecretEnc string `bson:"client_secret_enc,omitempty" json:"-"` Scopes []string `bson:"scopes" json:"scopes"` Enabled bool `bson:"enabled" json:"enabled"` - LegacyCallback bool `bson:"legacy_callback" json:"legacy_callback"` + CallbackNotice bool `bson:"callback_notice" json:"callback_notice"` Order int `bson:"order" json:"order"` CreatedAt time.Time `bson:"created_at" json:"created_at"` UpdatedAt time.Time `bson:"updated_at" json:"updated_at"` @@ -103,7 +103,10 @@ highest recorded today. For each document in `instance_oidc`, insert one and strands the SSO configuration. - `Scopes: ["openid", "profile", "email"]`, matching what `oidc.go` hardcodes today. -- `ProviderID` freshly generated, `LegacyCallback: true`. +- `ProviderID` freshly generated. +- `CallbackNotice: true` — this provider's redirect URI has changed and an + administrator has not yet acknowledged it. Set only by the migration; cleared + by the settings UI. New providers are created `false`. `instance_oidc` is left in place and no longer read. Idempotent by skipping any instance that already has an `auth_providers` document, so a re-run after a @@ -116,24 +119,26 @@ Routes: ``` GET /auth/oidc/:providerId/start GET /auth/oidc/:providerId/callback -GET /auth/oidc/callback # legacy, retained ``` +The old unparameterised `/auth/oidc/start` and `/auth/oidc/callback` are +**removed**, not retained. See Upgrade impact below — this breaks configured SSO +until the customer updates their IdP, and that is accepted deliberately rather +than carried as a compatibility path. + The state token in Redis stores `{instance_id, provider_id}` rather than the -bare instance ID. That is what lets the legacy callback keep working: a -migrated provider whose IdP still points at `/auth/oidc/callback` resolves its -provider from the consumed state, so **no customer has to touch their IdP -configuration at upgrade**. New providers are given the per-provider URL. Both -paths run identical code after state consumption. +bare instance ID. The callback resolves its provider from the consumed state +and cross-checks it against `:providerId` in the path, refusing a mismatch — +the path alone is attacker-controlled, and the state is the half that was +issued by the start handler. `providerForInstance` becomes `providerFor(ctx, c, instanceID, providerID)`. The `go-oidc` provider cache keys on `provider_id`, not instance. Saving, disabling or deleting a provider evicts that key. -`redirectURL(c)` gains the provider ID, and must return the same URL in the -start and callback halves of one flow or the token exchange is rejected — for a -legacy provider it therefore returns the legacy path. `LegacyCallback` is what -records that shape, and is set true only by the migration. +`redirectURL(c, providerID)` returns the one per-provider shape, and returns the +same URL in the start and callback halves of a flow — an IdP rejects the token +exchange if they differ. ### OIDC providers @@ -193,9 +198,9 @@ POST /auth/providers/:id/test with the stored credentials) and reports reachability. It does not sign anyone in. -`GET,PUT /api/org/oidc` is retained, reading and writing whichever provider -carries `legacy_callback`, so existing callers keep working. It creates nothing: -a PUT against an instance with no legacy provider answers 404. +`GET,PUT /api/org/oidc` is removed along with the old auth routes. Its only +caller is `OIDCCard.tsx`, which this work replaces, and a compatibility shim +over a one-of-many model would have to invent which provider it means. Every mutation writes an audit event, as every mutating path does. @@ -222,6 +227,12 @@ Add flow that asks for the preset first and then only the fields that preset needs, and the local-login toggle beneath the list. A guard violation surfaces the 409's message rather than a generic failure. +Every provider row shows its **callback URL** with click-to-copy — that is the +value the customer pastes into their IdP, it now differs per provider, and after +the upgrade every migrated provider needs it re-pasted. A migrated provider +additionally carries a warning until an administrator dismisses it, naming the +change and the URL. Dismissal is per provider, stored on the document. + ### Login page `web/app/login/page.tsx` calls `/auth/providers` on mount alongside the existing @@ -256,14 +267,39 @@ Errors keep the existing `/login?error=` redirect convention. - `local_login_enabled` absent decodes as enabled. - Guards: both 409 paths, and the enable/disable sequences that approach them without crossing. -- Legacy callback: a start on the legacy provider and a callback on - `/auth/oidc/callback` complete, and the redirect URL matches across the two - halves. - Per-provider callback: two providers in one instance, each resolving to its own configuration; a `provider_id` from another instance answers 404. +- A callback whose `:providerId` disagrees with the consumed state is refused, + and the state is consumed rather than left replayable. +- The removed routes (`/auth/oidc/start`, `/auth/oidc/callback`, + `/api/org/oidc`) answer 404. - GitHub: primary+verified selected; verified-only-absent refused. - `/auth/providers` response contains no issuer, client ID or secret. +## Upgrade impact + +**This release breaks configured SSO until each customer updates their identity +provider.** The old `/auth/oidc/callback` is gone, migrated providers are +reachable only at `/auth/oidc//callback`, and an IdP still pointing +at the old URL fails the flow. + +It is a deliberate trade: one callback shape rather than two, no +`legacy_callback` branch through `redirectURL`, and no permanently retained +route whose only purpose is a single past upgrade. + +Mitigations, in order of who sees them first: + +- The settings card shows the new callback URL per provider with click-to-copy, + and a migrated provider carries a dismissable warning naming the change. +- The failure is visible rather than silent: an IdP rejects the redirect URI + before Vantage is reached, so the customer sees their own provider's error. +- Local password login is unaffected, so no instance is locked out — an + administrator can always sign in to fix the URL. This is why + `local_login_enabled` defaults to true and why nothing in this migration + turns it off. +- Release notes and `docsite/docs/vantage/settings.md` state the required + action. + ## Deployment notes No new environment variables. No agent change. `KEY_ENCRYPTION_KEY` is already