* Allow no-auth connections and surface create rejections as typed 400s A no-auth (template "none") connection legitimately binds zero credential inputs; an empty item_ids map is its canonical shape. Exempt it from the empty-binding create guard and from the produce-time tool wipe. Connection-create validation failures (empty binding on a credentialed template, mixed origins, no user subject) now fail with a typed InvalidConnectionInputError mapped to HTTP 400 carrying the reason, instead of collapsing into an opaque 500. * Derive migrated OAuth authorize URLs instead of bare issuer origins The v1 provider state for some providers (Microsoft) stored only a bare issuer origin plus the token endpoint; v1 discovered the authorize endpoint at runtime. The migration's fallback wrote the bare origin as authorization_url, minting clients whose sign-in completes but never redirects back. When the grant has a browser leg and the token endpoint is a same-origin .../token URL, derive the .../authorize sibling. Client-credentials clients keep their (correct) empty URL. * Fix v1->v2 migration planner gaps - Plan the canonical workspace none-template connection for no-auth sources with no credential bindings; without it the migrated integration produces no tools (v2 tools are per-connection). - Skip secret-binding groups whose bindings reference no secrets instead of planning a credentialed connection with empty item_ids. - Warn on binding groups with nothing migratable instead of dropping them silently, and on orphan secrets with unparseable scopes. - Key text-binding item ids by source id so two sources binding the same slot in one scope cannot collide on one vault item. - Make authorization-server metadata resolution best-effort: a dead or unreachable metadata URL no longer aborts the whole local migration.
Unified · Split
| @@ -19,6 +19,7 @@ import { · 18 unmodified lines | ||
| 19 | 19 | IntegrationNotFoundError, |
| 20 | 20 | IntegrationSlug, |
| 21 | 21 | InternalError, |
| 22 | + InvalidConnectionInputError, | |
| 22 | 23 | OAuthClientSlug, |
| 23 | 24 | Owner, |
| 24 | 25 | ProviderItemId, |
| @@ -121,6 +122,9 @@ const IntegrationNotFound = IntegrationNotFoundError.annotate({ · 96 unmodified lines | ||
| 121 | 122 | const CredentialProviderNotRegistered = CredentialProviderNotRegisteredError.annotate({ |
| 122 | 123 | httpApiStatus: 409, |
| 123 | 124 | }); |
| 125 | +const InvalidConnectionInput = InvalidConnectionInputError.annotate({ | |
| 126 | + httpApiStatus: 400, | |
| 127 | +}); | |
| 124 | 128 | |
| 125 | 129 | // --------------------------------------------------------------------------- |
| 126 | 130 | // Group |
| @@ -138,7 +142,12 @@ export const ConnectionsApi = HttpApiGroup.make("connections") · 11 unmodified lines | ||
| 138 | 142 | HttpApiEndpoint.post("create", "/connections", { |
| 139 | 143 | payload: CreateConnectionPayload, |
| 140 | 144 | success: ConnectionResponse, |
| 141 | − error: [InternalError, IntegrationNotFound, CredentialProviderNotRegistered], | |
| 145 | + error: [ | |
| 146 | + InternalError, | |
| 147 | + IntegrationNotFound, | |
| 148 | + CredentialProviderNotRegistered, | |
| 149 | + InvalidConnectionInput, | |
| 150 | + ], | |
| 142 | 151 | }), |
| 143 | 152 | ) |
| 144 | 153 | .add( |
| @@ -162,15 +162,17 @@ describe("connections.create", () => { · 161 unmodified lines | ||
| 162 | 162 | }), |
| 163 | 163 | ); |
| 164 | 164 | |
| 165 | − // A connection is "born wired": it must reference at least one credential | |
| 166 | − // input. An empty binding (an empty `values`/`inputs` map) produces a | |
| 167 | − // credential with no credential — it persists, produces a full tool catalog, | |
| 168 | − // and then fails every invocation with `connection_value_missing`. These | |
| 169 | − // cases must be rejected at create. (An empty-STRING value is allowed — no-auth | |
| 170 | − // integrations bind one deliberately; and an external `from` that resolves to | |
| 171 | − // null is a supported case — both covered by their own tests — so neither is | |
| 172 | − // rejected here.) | |
| 173 | − it.effect("rejects an empty `values` map and persists nothing", () => | |
| 165 | + // A credentialed connection is "born wired": it must reference at least one | |
| 166 | + // credential input. An empty binding (an empty `values`/`inputs` map) produces | |
| 167 | + // a credential with no credential — it persists, produces a full tool catalog, | |
| 168 | + // and then fails every invocation with `connection_value_missing`. These cases | |
| 169 | + // must be rejected at create with a typed `InvalidConnectionInputError` (the | |
| 170 | + // HTTP edge answers 400 with the reason, not an opaque 500). The exception is | |
| 171 | + // the no-auth template ("none"), where zero inputs and an empty `item_ids` | |
| 172 | + // map are the canonical shape — covered below. (An empty-STRING value is also | |
| 173 | + // allowed, and an external `from` that resolves to null is a supported case — | |
| 174 | + // both covered by their own tests.) | |
| 175 | + it.effect("rejects an empty `values` map on a credentialed template and persists nothing", () => | |
| 174 | 176 | Effect.gen(function* () { |
| 175 | 177 | const executor = yield* setup(); |
| 176 | 178 | const result = yield* Effect.result( |
| @@ -183,13 +185,15 @@ describe("connections.create", () => { · 6 unmodified lines | ||
| 183 | 185 | }), |
| 184 | 186 | ); |
| 185 | 187 | expect(Result.isFailure(result)).toBe(true); |
| 188 | + if (!Result.isFailure(result)) return; | |
| 189 | + expect(Predicate.isTagged("InvalidConnectionInputError")(result.failure)).toBe(true); | |
| 186 | 190 | // No connection row and — critically — no tools were produced. |
| 187 | 191 | expect(yield* executor.connections.list()).toEqual([]); |
| 188 | 192 | expect(yield* executor.tools.list()).toEqual([]); |
| 189 | 193 | }), |
| 190 | 194 | ); |
| 191 | 195 | |
| 192 | − it.effect("rejects an empty `inputs` map", () => | |
| 196 | + it.effect("rejects an empty `inputs` map on a credentialed template", () => | |
| 193 | 197 | Effect.gen(function* () { |
| 194 | 198 | const executor = yield* setup(); |
| 195 | 199 | const result = yield* Effect.result( |
| @@ -202,10 +206,42 @@ describe("connections.create", () => { · 6 unmodified lines | ||
| 202 | 206 | }), |
| 203 | 207 | ); |
| 204 | 208 | expect(Result.isFailure(result)).toBe(true); |
| 209 | + if (!Result.isFailure(result)) return; | |
| 210 | + expect(Predicate.isTagged("InvalidConnectionInputError")(result.failure)).toBe(true); | |
| 205 | 211 | expect(yield* executor.connections.list()).toEqual([]); |
| 206 | 212 | }), |
| 207 | 213 | ); |
| 208 | 214 | |
| 215 | + // The no-auth template: public servers need no credential. The UI submits | |
| 216 | + // `values: {}` for them and the persisted row carries an empty `item_ids` | |
| 217 | + // map — that is the canonical shape (every migrated no-auth connection in | |
| 218 | + // prod has it), so it must create cleanly and keep its tools on refresh. | |
| 219 | + it.effect('creates a no-auth (`template: "none"`) connection from an empty `values` map', () => | |
| 220 | + Effect.gen(function* () { | |
| 221 | + const executor = yield* setup(); | |
| 222 | + const connection = yield* executor.connections.create({ | |
| 223 | + owner: "org", | |
| 224 | + name: ConnectionName.make("public"), | |
| 225 | + integration: INTEG, | |
| 226 | + template: AuthTemplateSlug.make("none"), | |
| 227 | + values: {}, | |
| 228 | + }); | |
| 229 | + expect(String(connection.address)).toBe("tools.vercel.org.public"); | |
| 230 | + | |
| 231 | + const tools = yield* executor.tools.list(); | |
| 232 | + expect(tools.map((t) => String(t.name)).sort()).toEqual(["deploy", "list"]); | |
| 233 | + | |
| 234 | + // Refresh must NOT treat the empty binding as invalid and wipe the tools. | |
| 235 | + const refreshed = yield* executor.connections.refresh({ | |
| 236 | + owner: "org", | |
| 237 | + integration: INTEG, | |
| 238 | + name: ConnectionName.make("public"), | |
| 239 | + }); | |
| 240 | + expect(refreshed.map((t) => String(t.name)).sort()).toEqual(["deploy", "list"]); | |
| 241 | + expect((yield* executor.tools.list()).length).toBe(2); | |
| 242 | + }), | |
| 243 | + ); | |
| 244 | + | |
| 209 | 245 | it.effect("allows an empty-string value (no-auth integrations bind one)", () => |
| 210 | 246 | Effect.gen(function* () { |
| 211 | 247 | const executor = yield* setup(); |
| @@ -91,6 +91,16 @@ export class ConnectionNotFoundError extends Schema.TaggedErrorClass<ConnectionN · 90 unmodified lines | ||
| 91 | 91 | }, |
| 92 | 92 | ) {} |
| 93 | 93 | |
| 94 | +/** A connection create request was rejected before anything was written: the | |
| 95 | + * input is structurally invalid (no credential inputs for a credentialed | |
| 96 | + * template, mixed pasted/external origins, …) or targets owner `user` in a | |
| 97 | + * context that has no user subject. The message says which — it is safe to | |
| 98 | + * show to the caller. */ | |
| 99 | +export class InvalidConnectionInputError extends Schema.TaggedErrorClass<InvalidConnectionInputError>()( | |
| 100 | + "InvalidConnectionInputError", | |
| 101 | + { message: Schema.String }, | |
| 102 | +) {} | |
| 103 | + | |
| 94 | 104 | /** A connection references a credential provider key that isn't registered on |
| 95 | 105 | * the executor. */ |
| 96 | 106 | export class CredentialProviderNotRegisteredError extends Schema.TaggedErrorClass<CredentialProviderNotRegisteredError>()( |
| @@ -51,6 +51,7 @@ import { · 50 unmodified lines | ||
| 51 | 51 | CredentialProviderNotRegisteredError, |
| 52 | 52 | CredentialResolutionError, |
| 53 | 53 | IntegrationNotFoundError, |
| 54 | + InvalidConnectionInputError, | |
| 54 | 55 | IntegrationRemovalNotAllowedError, |
| 55 | 56 | NoHandlerError, |
| 56 | 57 | PluginNotLoadedError, |
| @@ -64,6 +65,7 @@ import { · 7 unmodified lines | ||
| 64 | 65 | ConnectionAddress, |
| 65 | 66 | ConnectionName, |
| 66 | 67 | IntegrationSlug, |
| 68 | + NO_AUTH_TEMPLATE, | |
| 67 | 69 | OAuthClientSlug, |
| 68 | 70 | Owner, |
| 69 | 71 | PolicyId, |
| @@ -256,7 +258,10 @@ export type Executor<TPlugins extends readonly AnyPlugin[] = readonly []> = { · 186 unmodified lines | ||
| 256 | 258 | input: CreateConnectionInput, |
| 257 | 259 | ) => Effect.Effect< |
| 258 | 260 | Connection, |
| 259 | − IntegrationNotFoundError | CredentialProviderNotRegisteredError | StorageFailure | |
| 261 | + | IntegrationNotFoundError | |
| 262 | + | CredentialProviderNotRegisteredError | |
| 263 | + | InvalidConnectionInputError | |
| 264 | + | StorageFailure | |
| 260 | 265 | >; |
| 261 | 266 | readonly list: (filter?: { |
| 262 | 267 | readonly integration?: IntegrationSlug; |
| @@ -1764,15 +1769,18 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 1501 unmodified lines | ||
| 1764 | 1769 | ); |
| 1765 | 1770 | |
| 1766 | 1771 | // Defense in depth (and cleanup for rows created before the create-time |
| 1767 | − // guard, or emptied by an external edit): a non-OAuth connection with no | |
| 1768 | − // bound credential inputs can never resolve a value, so never advertise | |
| 1769 | − // tools for it — every call would fail with `connection_value_missing`. | |
| 1770 | − // OAuth connections resolve via refresh and carry their token outside | |
| 1771 | − // `item_ids`, so they are exempt. | |
| 1772 | + // guard, or emptied by an external edit): a credentialed non-OAuth | |
| 1773 | + // connection with no bound credential inputs can never resolve a value, | |
| 1774 | + // so never advertise tools for it — every call would fail with | |
| 1775 | + // `connection_value_missing`. OAuth connections resolve via refresh and | |
| 1776 | + // carry their token outside `item_ids`; no-auth (`"none"` template) | |
| 1777 | + // connections legitimately bind nothing (an empty `item_ids` is their | |
| 1778 | + // canonical shape) — both are exempt. | |
| 1772 | 1779 | const existingRow = yield* findConnectionRow(ref); |
| 1773 | 1780 | if ( |
| 1774 | 1781 | existingRow && |
| 1775 | 1782 | existingRow.oauth_client == null && |
| 1783 | + existingRow.template !== String(NO_AUTH_TEMPLATE) && | |
| 1776 | 1784 | Object.keys(connectionItemIds(existingRow)).length === 0 |
| 1777 | 1785 | ) { |
| 1778 | 1786 | yield* transaction( |
| @@ -1876,11 +1884,21 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 97 unmodified lines | ||
| 1876 | 1884 | input: CreateConnectionInput, |
| 1877 | 1885 | ): Effect.Effect< |
| 1878 | 1886 | Connection, |
| 1879 | − IntegrationNotFoundError | CredentialProviderNotRegisteredError | StorageFailure | |
| 1887 | + | IntegrationNotFoundError | |
| 1888 | + | CredentialProviderNotRegisteredError | |
| 1889 | + | InvalidConnectionInputError | |
| 1890 | + | StorageFailure | |
| 1880 | 1891 | > => |
| 1881 | 1892 | Effect.gen(function* () { |
| 1882 | 1893 | const name = connectionIdentifier(String(input.name)); |
| 1883 | − yield* requireUserSubject(input.owner); | |
| 1894 | + // Typed (not StorageError) so the HTTP edge can answer 400 with the | |
| 1895 | + // reason instead of an opaque 500 — callers can act on it. | |
| 1896 | + if (input.owner === "user" && subject == null) { | |
| 1897 | + return yield* new InvalidConnectionInputError({ | |
| 1898 | + message: | |
| 1899 | + 'Cannot create a personal connection: this context has no user subject. Create it with owner "org", or connect as a signed-in user.', | |
| 1900 | + }); | |
| 1901 | + } | |
| 1884 | 1902 | const integrationRow = yield* findIntegrationRow(input.integration); |
| 1885 | 1903 | if (!integrationRow) { |
| 1886 | 1904 | return yield* new IntegrationNotFoundError({ |
| @@ -1896,27 +1914,27 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 9 unmodified lines | ||
| 1896 | 1914 | const inputs = normalizeConnectionInputs(input); |
| 1897 | 1915 | const pasted = inputs.filter((i) => "value" in i.origin); |
| 1898 | 1916 | const external = inputs.filter((i) => "from" in i.origin); |
| 1899 | − // A connection is born wired: it must reference at least one credential | |
| 1900 | − // input. An empty binding (no inputs at all — e.g. an empty `values`/ | |
| 1901 | − // `inputs` map) is a credential with no credential: it would persist, | |
| 1902 | − // produce a full tool catalog, and then fail every invocation with | |
| 1903 | − // `connection_value_missing`. Reject it here. (An empty-string value is | |
| 1904 | − // NOT rejected — no-auth integrations like MCP deliberately bind one, and | |
| 1905 | − // it yields a non-empty `item_ids`. OAuth connections are minted via | |
| 1906 | − // `mintOAuthConnection`, not this path; an external `from` reference may | |
| 1907 | − // resolve to null and is surfaced at invoke time, not here.) | |
| 1908 | − if (inputs.length === 0) { | |
| 1909 | − return yield* new StorageError({ | |
| 1917 | + // A credentialed connection is born wired: it must reference at least | |
| 1918 | + // one credential input. An empty binding (no inputs at all — e.g. an | |
| 1919 | + // empty `values`/`inputs` map) is a credential with no credential: it | |
| 1920 | + // would persist, produce a full tool catalog, and then fail every | |
| 1921 | + // invocation with `connection_value_missing`. Reject it here — EXCEPT | |
| 1922 | + // for the no-auth template ("none"), where zero inputs and an empty | |
| 1923 | + // `item_ids` map are the canonical shape (public MCP servers; the UI | |
| 1924 | + // submits `values: {}` for them). OAuth connections are minted via | |
| 1925 | + // `mintOAuthConnection`, not this path; an external `from` reference | |
| 1926 | + // may resolve to null and is surfaced at invoke time, not here. | |
| 1927 | + const isNoAuth = String(input.template) === String(NO_AUTH_TEMPLATE); | |
| 1928 | + if (inputs.length === 0 && !isNoAuth) { | |
| 1929 | + return yield* new InvalidConnectionInputError({ | |
| 1910 | 1930 | message: "A connection must supply at least one credential input.", |
| 1911 | − cause: undefined, | |
| 1912 | 1931 | }); |
| 1913 | 1932 | } |
| 1914 | 1933 | let providerKey: string; |
| 1915 | 1934 | const itemIds: Record<string, string> = {}; |
| 1916 | 1935 | if (external.length > 0 && pasted.length > 0) { |
| 1917 | − return yield* new StorageError({ | |
| 1936 | + return yield* new InvalidConnectionInputError({ | |
| 1918 | 1937 | message: "A connection cannot mix pasted and external-provider inputs.", |
| 1919 | − cause: undefined, | |
| 1920 | 1938 | }); |
| 1921 | 1939 | } |
| 1922 | 1940 | if (external.length > 0) { |
| @@ -1924,9 +1942,8 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 1 unmodified line | ||
| 1924 | 1942 | external.map((i) => ("from" in i.origin ? String(i.origin.from.provider) : "")), |
| 1925 | 1943 | ); |
| 1926 | 1944 | if (providers.size > 1) { |
| 1927 | − return yield* new StorageError({ | |
| 1945 | + return yield* new InvalidConnectionInputError({ | |
| 1928 | 1946 | message: "A connection's inputs must all use the same external provider.", |
| 1929 | − cause: undefined, | |
| 1930 | 1947 | }); |
| 1931 | 1948 | } |
| 1932 | 1949 | const [only] = [...providers]; |
| @@ -12,6 +12,11 @@ export type IntegrationSlug = typeof IntegrationSlug.Type; · 11 unmodified lines | ||
| 12 | 12 | export const AuthTemplateSlug = Schema.String.pipe(Schema.brand("AuthTemplateSlug")); |
| 13 | 13 | export type AuthTemplateSlug = typeof AuthTemplateSlug.Type; |
| 14 | 14 | |
| 15 | +/** The sentinel template for integrations that require no credential (e.g. a | |
| 16 | + * public MCP server). Connections on it legitimately bind zero inputs — an | |
| 17 | + * empty `item_ids` map is their canonical persisted shape. */ | |
| 18 | +export const NO_AUTH_TEMPLATE = AuthTemplateSlug.make("none"); | |
| 19 | + | |
| 15 | 20 | /** A connection's name — the `<connection>` segment of an address, scoped under |
| 16 | 21 | * its integration + owner (so the same name can exist on two integrations). */ |
| 17 | 22 | export const ConnectionName = Schema.String.pipe(Schema.brand("ConnectionName")); |
| @@ -162,11 +162,19 @@ const resolveMigrationOAuthAuthorizationUrl = async ( · 161 unmodified lines | ||
| 162 | 162 | ): Promise<string | null> => { |
| 163 | 163 | const metadataUrl = client.authorizationServerMetadataUrl?.trim(); |
| 164 | 164 | if (metadataUrl) { |
| 165 | − return await fetchOAuthAuthorizationEndpoint( | |
| 166 | − validateMigrationOAuthUrl(metadataUrl, "authorizationServerMetadataUrl"), | |
| 167 | − fetchImpl, | |
| 168 | − timeoutMs, | |
| 169 | − ); | |
| 165 | + // Best-effort like the resource/issuer discovery below: a dead, invalid, | |
| 166 | + // or unreachable metadata endpoint (offline machine, archived server) | |
| 167 | + // must not abort the whole migration — fall through to discovery. | |
| 168 | + try { | |
| 169 | + const endpoint = await fetchOAuthAuthorizationEndpoint( | |
| 170 | + validateMigrationOAuthUrl(metadataUrl, "authorizationServerMetadataUrl"), | |
| 171 | + fetchImpl, | |
| 172 | + timeoutMs, | |
| 173 | + ); | |
| 174 | + if (endpoint) return endpoint; | |
| 175 | + } catch { | |
| 176 | + // fall through to issuer/resource discovery | |
| 177 | + } | |
| 170 | 178 | } |
| 171 | 179 | |
| 172 | 180 | if (client.grant !== "authorization_code") return null; |
| @@ -1117,6 +1117,179 @@ describe("planMigration (the weave)", () => { · 1116 unmodified lines | ||
| 1117 | 1117 | expect(plan.report.policies).toEqual({ ok: 2, static: 0, deadInert: 0 }); |
| 1118 | 1118 | }); |
| 1119 | 1119 | |
| 1120 | + // The prod Microsoft shape: v1 stored only a bare issuer origin plus the | |
| 1121 | + // full token endpoint (no discrete authorize endpoint — v1 discovered it at | |
| 1122 | + // runtime). A bare origin is a dead authorize URL (sign-in completes but | |
| 1123 | + // never redirects back), so the planner must derive the same-origin | |
| 1124 | + // `…/authorize` sibling of the token endpoint instead. | |
| 1125 | + it("derives the authorize endpoint from the token endpoint when v1 only stored a bare issuer", () => { | |
| 1126 | + const input: MigrationInput = { | |
| 1127 | + nowMs: now, | |
| 1128 | + sources: [ | |
| 1129 | + { scopeId: "org_X", id: "microsoft_graph", pluginId: "openapi", name: "Microsoft Graph" }, | |
| 1130 | + ], | |
| 1131 | + migratedConfigs: new Map([ | |
| 1132 | + [ | |
| 1133 | + "org_X microsoft_graph", | |
| 1134 | + cfg({ | |
| 1135 | + slotToTemplateSlug: { "oauth2:azureaddelegated:connection": "azureAdDelegated" }, | |
| 1136 | + slotToVariable: { "oauth2:azureaddelegated:connection": "token" }, | |
| 1137 | + }), | |
| 1138 | + ], | |
| 1139 | + ]), | |
| 1140 | + connections: [ | |
| 1141 | + { | |
| 1142 | + id: "openapi-oauth-microsoft-graph", | |
| 1143 | + scopeId: "org_X", | |
| 1144 | + provider: "oauth2", | |
| 1145 | + identityLabel: null, | |
| 1146 | + accessTokenSecretId: "ms-access", | |
| 1147 | + refreshTokenSecretId: null, | |
| 1148 | + expiresAt: null, | |
| 1149 | + providerState: { | |
| 1150 | + kind: "authorization-code", | |
| 1151 | + clientId: "cid-ms", | |
| 1152 | + issuerUrl: "https://login.microsoftonline.com", | |
| 1153 | + tokenEndpoint: "https://login.microsoftonline.com/common/oauth2/v2.0/token", | |
| 1154 | + scopes: ["User.Read"], | |
| 1155 | + }, | |
| 1156 | + }, | |
| 1157 | + ], | |
| 1158 | + bindings: [ | |
| 1159 | + { | |
| 1160 | + scopeId: "org_X", | |
| 1161 | + sourceId: "microsoft_graph", | |
| 1162 | + slotKey: "oauth2:azureaddelegated:connection", | |
| 1163 | + kind: "connection", | |
| 1164 | + secretId: null, | |
| 1165 | + connectionId: "openapi-oauth-microsoft-graph", | |
| 1166 | + textValue: null, | |
| 1167 | + }, | |
| 1168 | + ], | |
| 1169 | + secrets: [ | |
| 1170 | + { | |
| 1171 | + id: "ms-access", | |
| 1172 | + scopeId: "org_X", | |
| 1173 | + name: "", | |
| 1174 | + provider: "workos-vault", | |
| 1175 | + ownedByConnectionId: "openapi-oauth-microsoft-graph", | |
| 1176 | + }, | |
| 1177 | + ], | |
| 1178 | + policies: [], | |
| 1179 | + toolSourceIds: [], | |
| 1180 | + }; | |
| 1181 | + | |
| 1182 | + const plan = planMigration(input); | |
| 1183 | + | |
| 1184 | + expect(plan.oauthClients).toHaveLength(1); | |
| 1185 | + expect(plan.oauthClients[0]?.authorizationUrl).toBe( | |
| 1186 | + "https://login.microsoftonline.com/common/oauth2/v2.0/authorize", | |
| 1187 | + ); | |
| 1188 | + expect(plan.oauthClients[0]?.tokenUrl).toBe( | |
| 1189 | + "https://login.microsoftonline.com/common/oauth2/v2.0/token", | |
| 1190 | + ); | |
| 1191 | + }); | |
| 1192 | + | |
| 1193 | + // v2 produces tools per connection, so a no-auth source (no credential | |
| 1194 | + // bindings at all) must still get its canonical connection — template | |
| 1195 | + // "none", empty item_ids — or the migrated integration is dead. | |
| 1196 | + it("plans a workspace none-template connection for binding-less no-auth sources", () => { | |
| 1197 | + const input: MigrationInput = { | |
| 1198 | + nowMs: now, | |
| 1199 | + sources: [{ scopeId: "org_X", id: "context7", pluginId: "mcp", name: "Context7" }], | |
| 1200 | + migratedConfigs: new Map([ | |
| 1201 | + ["org_X context7", cfg({ config: { transport: "remote", auth: { kind: "none" } } })], | |
| 1202 | + ]), | |
| 1203 | + connections: [], | |
| 1204 | + bindings: [], | |
| 1205 | + secrets: [], | |
| 1206 | + policies: [], | |
| 1207 | + toolSourceIds: [], | |
| 1208 | + }; | |
| 1209 | + | |
| 1210 | + const plan = planMigration(input); | |
| 1211 | + | |
| 1212 | + expect(plan.connections).toHaveLength(1); | |
| 1213 | + const conn = plan.connections[0]; | |
| 1214 | + expect(conn?.row.integration).toBe("context7"); | |
| 1215 | + expect(conn?.row.name).toBe("workspace"); | |
| 1216 | + expect(conn?.row.template).toBe("none"); | |
| 1217 | + expect(conn?.row.owner).toBe("org"); | |
| 1218 | + expect(conn?.itemIds).toEqual({}); | |
| 1219 | + // An OAuth-protected source must NOT get one. | |
| 1220 | + const oauthInput: MigrationInput = { | |
| 1221 | + ...input, | |
| 1222 | + migratedConfigs: new Map([ | |
| 1223 | + ["org_X context7", cfg({ config: { transport: "remote", auth: { kind: "oauth2" } } })], | |
| 1224 | + ]), | |
| 1225 | + }; | |
| 1226 | + expect(planMigration(oauthInput).connections).toHaveLength(0); | |
| 1227 | + }); | |
| 1228 | + | |
| 1229 | + it("skips secret-binding groups whose bindings reference no secrets, with a warning", () => { | |
| 1230 | + const input: MigrationInput = { | |
| 1231 | + nowMs: now, | |
| 1232 | + sources: [{ scopeId: "org_X", id: "broken_api", pluginId: "openapi", name: "Broken" }], | |
| 1233 | + migratedConfigs: new Map(), | |
| 1234 | + connections: [], | |
| 1235 | + bindings: [ | |
| 1236 | + { | |
| 1237 | + scopeId: "org_X", | |
| 1238 | + sourceId: "broken_api", | |
| 1239 | + slotKey: "header:authorization", | |
| 1240 | + kind: "secret", | |
| 1241 | + secretId: null, | |
| 1242 | + connectionId: null, | |
| 1243 | + textValue: null, | |
| 1244 | + }, | |
| 1245 | + ], | |
| 1246 | + secrets: [], | |
| 1247 | + policies: [], | |
| 1248 | + toolSourceIds: [], | |
| 1249 | + }; | |
| 1250 | + | |
| 1251 | + const plan = planMigration(input); | |
| 1252 | + | |
| 1253 | + // No empty-item_ids ghost connection (the runtime would refuse it tools). | |
| 1254 | + expect(plan.connections).toHaveLength(0); | |
| 1255 | + expect(plan.report.warnings.some((w) => w.includes("reference no secrets"))).toBe(true); | |
| 1256 | + }); | |
| 1257 | + | |
| 1258 | + it("keys text-binding item ids by source so same-slot bindings do not collide", () => { | |
| 1259 | + const binding = (sourceId: string, textValue: string) => ({ | |
| 1260 | + scopeId: "org_X", | |
| 1261 | + sourceId, | |
| 1262 | + slotKey: "header:authorization", | |
| 1263 | + kind: "text" as const, | |
| 1264 | + secretId: null, | |
| 1265 | + connectionId: null, | |
| 1266 | + textValue, | |
| 1267 | + }); | |
| 1268 | + const input: MigrationInput = { | |
| 1269 | + nowMs: now, | |
| 1270 | + sources: [ | |
| 1271 | + { scopeId: "org_X", id: "api_one", pluginId: "mcp", name: "One" }, | |
| 1272 | + { scopeId: "org_X", id: "api_two", pluginId: "mcp", name: "Two" }, | |
| 1273 | + ], | |
| 1274 | + migratedConfigs: new Map(), | |
| 1275 | + connections: [], | |
| 1276 | + bindings: [binding("api_one", "value-one"), binding("api_two", "value-two")], | |
| 1277 | + secrets: [], | |
| 1278 | + policies: [], | |
| 1279 | + toolSourceIds: [], | |
| 1280 | + }; | |
| 1281 | + | |
| 1282 | + const plan = planMigration(input); | |
| 1283 | + | |
| 1284 | + const ids = plan.connections.map((c) => Object.values(c.itemIds)).flat(); | |
| 1285 | + expect(new Set(ids).size).toBe(2); | |
| 1286 | + const ops = plan.secretOps.filter((o) => o.role === "apikey"); | |
| 1287 | + expect(ops.map((o) => "fromText" in o && o.fromText).sort()).toEqual([ | |
| 1288 | + "value-one", | |
| 1289 | + "value-two", | |
| 1290 | + ]); | |
| 1291 | + }); | |
| 1292 | + | |
| 1120 | 1293 | it("plans a v1 client-credentials OAuth connection with secret-backed client credentials", () => { |
| 1121 | 1294 | const input: MigrationInput = { |
| 1122 | 1295 | nowMs: now, |
| @@ -1256,12 +1256,41 @@ const nonEmptyString = (value: string | null | undefined): string | null => { · 1255 unmodified lines | ||
| 1256 | 1256 | return trimmed ? trimmed : null; |
| 1257 | 1257 | }; |
| 1258 | 1258 | |
| 1259 | −const authorizationUrlFromProviderState = (ps: V1ProviderState | null): string => | |
| 1260 | − nonEmptyString(ps?.authorizationEndpoint) ?? | |
| 1261 | − nonEmptyString(ps?.authorizationServerMetadata?.authorization_endpoint) ?? | |
| 1262 | − nonEmptyString(ps?.authorizationServerUrl) ?? | |
| 1263 | − nonEmptyString(ps?.issuerUrl) ?? | |
| 1264 | − ""; | |
| 1259 | +/** True when the URL has no meaningful path — a bare origin like | |
| 1260 | + * `https://login.microsoftonline.com`. A bare origin is never a usable | |
| 1261 | + * authorize endpoint: redirecting there signs the user in and strands them | |
| 1262 | + * (observed in prod with migrated Microsoft clients). */ | |
| 1263 | +const isBareOrigin = (url: string): boolean => /^https?:\/\/[^/]+\/?$/.test(url); | |
| 1264 | + | |
| 1265 | +const authorizationUrlFromProviderState = ( | |
| 1266 | + ps: V1ProviderState | null, | |
| 1267 | + grant: MigrationGrant, | |
| 1268 | +): string => { | |
| 1269 | + const explicit = | |
| 1270 | + nonEmptyString(ps?.authorizationEndpoint) ?? | |
| 1271 | + nonEmptyString(ps?.authorizationServerMetadata?.authorization_endpoint); | |
| 1272 | + if (explicit) return explicit; | |
| 1273 | + const fallback = | |
| 1274 | + nonEmptyString(ps?.authorizationServerUrl) ?? nonEmptyString(ps?.issuerUrl) ?? ""; | |
| 1275 | + // Client-credentials clients have no browser leg — an empty authorization | |
| 1276 | + // URL is their correct shape; never derive one. | |
| 1277 | + if (grant === "client_credentials") return fallback; | |
| 1278 | + // v1 stored only an issuer/server origin for some providers and discovered | |
| 1279 | + // the authorize endpoint at runtime. v2 stores the endpoint itself, so a | |
| 1280 | + // bare origin would mint a broken client. When the token endpoint is a | |
| 1281 | + // same-origin `…/token` URL, its `…/authorize` sibling is the convention | |
| 1282 | + // (and exactly right for the Microsoft v2.0 endpoints that hit this path); | |
| 1283 | + // prefer that over a guaranteed-dead bare origin. | |
| 1284 | + const token = nonEmptyString(ps?.tokenEndpoint); | |
| 1285 | + if ( | |
| 1286 | + (fallback === "" || isBareOrigin(fallback)) && | |
| 1287 | + token?.endsWith("/token") && | |
| 1288 | + (fallback === "" || token.startsWith(fallback.replace(/\/$/, "") + "/")) | |
| 1289 | + ) { | |
| 1290 | + return token.replace(/\/token$/, "/authorize"); | |
| 1291 | + } | |
| 1292 | + return fallback; | |
| 1293 | +}; | |
| 1265 | 1294 | |
| 1266 | 1295 | const secretOpDedupeKey = (op: SecretOp): string => |
| 1267 | 1296 | `${op.targetProvider}\0${op.owner.tenant}\0${op.owner.owner}\0${op.owner.subject}\0${op.itemId}`; |
| @@ -1458,7 +1487,7 @@ export const planMigration = (input: MigrationInput): MigrationPlan => { · 190 unmodified lines | ||
| 1458 | 1487 | clientId: ps?.clientId ?? "", |
| 1459 | 1488 | clientIdSecretRef, |
| 1460 | 1489 | tokenUrl: ps?.tokenEndpoint ?? "", |
| 1461 | − authorizationUrl: authorizationUrlFromProviderState(ps), | |
| 1490 | + authorizationUrl: authorizationUrlFromProviderState(ps, grant), | |
| 1462 | 1491 | authorizationServerMetadataUrl: nonEmptyString(ps?.authorizationServerMetadataUrl), |
| 1463 | 1492 | grant, |
| 1464 | 1493 | resource: |
| @@ -1530,6 +1559,15 @@ export const planMigration = (input: MigrationInput): MigrationPlan => { · 65 unmodified lines | ||
| 1530 | 1559 | template = slotTemplate(b.slotKey); |
| 1531 | 1560 | consume(secretScopeId, b.secretId); |
| 1532 | 1561 | } |
| 1562 | + // All-null secret ids (malformed v1 rows) would plan a credentialed | |
| 1563 | + // connection with an empty `item_ids` map — a credential with no | |
| 1564 | + // credential that the runtime refuses to produce tools for. Skip loudly. | |
| 1565 | + if (Object.keys(itemIds).length === 0) { | |
| 1566 | + warnings.push( | |
| 1567 | + `Skipped binding group "${sourceId}" in scope "${scopeId}": its secret bindings reference no secrets.`, | |
| 1568 | + ); | |
| 1569 | + continue; | |
| 1570 | + } | |
| 1533 | 1571 | const provider = targetProvider ?? defaultWritableProvider; |
| 1534 | 1572 | if (providers.size > 1) { |
| 1535 | 1573 | warnings.push( |
| @@ -1567,7 +1605,11 @@ export const planMigration = (input: MigrationInput): MigrationPlan => { · 31 unmodified lines | ||
| 1567 | 1605 | let template = API_KEY_TEMPLATE_SLUG; |
| 1568 | 1606 | for (const b of textBindings) { |
| 1569 | 1607 | const variable = slotVar(b.slotKey); |
| 1570 | − const itemId = migratedItemId(scopeId, `text:${b.slotKey}`); | |
| 1608 | + // The source id is part of the key: two sources in one scope can bind | |
| 1609 | + // the same slot (e.g. `header:authorization`) with different values — | |
| 1610 | + // without it they'd collide on one item id and one would silently | |
| 1611 | + // read the other's secret. | |
| 1612 | + const itemId = migratedItemId(scopeId, `text:${sourceId}:${b.slotKey}`); | |
| 1571 | 1613 | secretOps.push({ |
| 1572 | 1614 | itemId, |
| 1573 | 1615 | role: "apikey", |
| @@ -1603,6 +1645,52 @@ export const planMigration = (input: MigrationInput): MigrationPlan => { · 29 unmodified lines | ||
| 1603 | 1645 | refreshItemId: null, |
| 1604 | 1646 | }); |
| 1605 | 1647 | } |
| 1648 | + } else { | |
| 1649 | + // Nothing migratable in the group: a connection binding without a | |
| 1650 | + // connection id, or only OAuth client-credential slots (the app config | |
| 1651 | + // migrates via the bound connection's provider state — an app that was | |
| 1652 | + // configured but never connected has nothing to attach to). Say so | |
| 1653 | + // instead of dropping the group silently. | |
| 1654 | + warnings.push( | |
| 1655 | + `Skipped binding group "${sourceId}" in scope "${scopeId}": no migratable credential binding (its secrets fall to the orphan re-key).`, | |
| 1656 | + ); | |
| 1657 | + } | |
| 1658 | + } | |
| 1659 | + | |
| 1660 | + // --- No-auth sources (mcp/graphql `auth.kind === "none"`) have no credential | |
| 1661 | + // bindings, but v2 produces tools per CONNECTION — without one the migrated | |
| 1662 | + // integration is dead (no tools, nothing to invoke). Plan the canonical | |
| 1663 | + // no-auth connection: template "none", empty item_ids. (This is the planner | |
| 1664 | + // fix for the gap that required the prod `{}` backfill.) | |
| 1665 | + for (const source of input.sources) { | |
| 1666 | + if (groups.has(sourceKey(source.scopeId, source.id))) continue; | |
| 1667 | + const migrated = input.migratedConfigs.get(sourceKey(source.scopeId, source.id)); | |
| 1668 | + const auth = (migrated?.config as { auth?: { kind?: string } } | undefined)?.auth; | |
| 1669 | + if (auth?.kind !== "none") continue; | |
| 1670 | + const row = planConnectionRow({ | |
| 1671 | + scopeId: source.scopeId, | |
| 1672 | + integration: source.id, | |
| 1673 | + name: "workspace", | |
| 1674 | + template: "none", | |
| 1675 | + provider: defaultWritableProvider, | |
| 1676 | + identityLabel: null, | |
| 1677 | + grant: "authorization_code", | |
| 1678 | + v1ExpiresAt: null, | |
| 1679 | + oauthScopes: [], | |
| 1680 | + oauthClientSlug: null, | |
| 1681 | + oauthClientOwner: null, | |
| 1682 | + nowMs: input.nowMs, | |
| 1683 | + ownerForScope, | |
| 1684 | + }); | |
| 1685 | + if (row) { | |
| 1686 | + connections.push({ | |
| 1687 | + credentialScopeId: source.scopeId, | |
| 1688 | + sourceScopeId: source.scopeId, | |
| 1689 | + sourceId: source.id, | |
| 1690 | + row, | |
| 1691 | + itemIds: {}, | |
| 1692 | + refreshItemId: null, | |
| 1693 | + }); | |
| 1606 | 1694 | } |
| 1607 | 1695 | } |
| 1608 | 1696 | |
| @@ -1630,7 +1718,10 @@ export const planMigration = (input: MigrationInput): MigrationPlan => { · 21 unmodified lines | ||
| 1630 | 1718 | for (const s of input.secrets) { |
| 1631 | 1719 | if (consumed.has(`${s.scopeId} ${s.id}`)) continue; |
| 1632 | 1720 | const owner = ownerForScope(s.scopeId); |
| 1633 | − if (!owner) continue; // unparseable scope orphan — skip + (counted via warnings below) | |
| 1721 | + if (!owner) { | |
| 1722 | + warnings.push(`Skipped orphan secret "${s.id}": unparseable scope "${s.scopeId}".`); | |
| 1723 | + continue; | |
| 1724 | + } | |
| 1634 | 1725 | secretOps.push({ |
| 1635 | 1726 | itemId: migratedItemId(s.scopeId, s.id), |
| 1636 | 1727 | role: "orphan", |
| @@ -36,6 +36,7 @@ import type { · 35 unmodified lines | ||
| 36 | 36 | CredentialProviderNotRegisteredError, |
| 37 | 37 | IntegrationNotFoundError, |
| 38 | 38 | IntegrationRemovalNotAllowedError, |
| 39 | + InvalidConnectionInputError, | |
| 39 | 40 | } from "./errors"; |
| 40 | 41 | import type { OAuthService } from "./oauth-client"; |
| 41 | 42 | import type { CredentialProvider, ProviderEntry } from "./provider"; |
| @@ -140,7 +141,10 @@ export interface PluginCtx<TStore = unknown> { · 98 unmodified lines | ||
| 140 | 141 | input: CreateConnectionInput, |
| 141 | 142 | ) => Effect.Effect< |
| 142 | 143 | Connection, |
| 143 | − IntegrationNotFoundError | CredentialProviderNotRegisteredError | StorageFailure | |
| 144 | + | IntegrationNotFoundError | |
| 145 | + | CredentialProviderNotRegisteredError | |
| 146 | + | InvalidConnectionInputError | |
| 147 | + | StorageFailure | |
| 144 | 148 | >; |
| 145 | 149 | readonly list: (filter?: { |
| 146 | 150 | readonly integration?: IntegrationSlug; |
| @@ -57,6 +57,7 @@ export { · 56 unmodified lines | ||
| 57 | 57 | IntegrationAlreadyExistsError, |
| 58 | 58 | IntegrationRemovalNotAllowedError, |
| 59 | 59 | ConnectionNotFoundError, |
| 60 | + InvalidConnectionInputError, | |
| 60 | 61 | CredentialProviderNotRegisteredError, |
| 61 | 62 | CredentialResolutionError, |
| 62 | 63 | type ExecuteError, |