Unified · Split
| @@ -527,6 +527,89 @@ describe("oauth.registerDynamicClient", () => { · 526 unmodified lines | |||
| 527 | ), | 527 | ), |
| 528 | ); | 528 | ); |
| 529 | 529 | ||
| 530 | // Repro: a recreated sandbox changes the executor's callback origin while | ||
| 531 | // the DCR client minted for the OLD callback survives in storage. Reuse is | ||
| 532 | // keyed on (owner, issuer, resource) and never compares the redirect URI the | ||
| 533 | // client was registered with, so `oauth.start` pairs the stale registration | ||
| 534 | // with the NEW callback — and a strict authorization server (the test AS | ||
| 535 | // included) rejects the authorize request with 400 `redirect_uri is not | ||
| 536 | // registered`. Contract: a changed flow redirect URI must trigger a fresh | ||
| 537 | // registration instead of reusing the stale client. | ||
| 538 | it.effect("re-registers instead of reusing the DCR client when the redirect URI changed", () => | ||
| 539 | Effect.scoped( | ||
| 540 | Effect.gen(function* () { | ||
| 541 | const server = yield* serveOAuthTestServer({ scopes: ["read"] }); | ||
| 542 | const { executor } = yield* makeTestWorkspaceHarness({ plugins }); | ||
| 543 | yield* executor.acme.seed(); | ||
| 544 | const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); | ||
| 545 | |||
| 546 | // Original sandbox: DCR registration carries the ORIGINAL callback. | ||
| 547 | yield* executor.oauth.registerDynamicClient({ | ||
| 548 | owner: "org", | ||
| 549 | slug: OAuthClientSlug.make("original-sandbox"), | ||
| 550 | issuer: probe.issuer, | ||
| 551 | registrationEndpoint: probe.registrationEndpoint!, | ||
| 552 | authorizationUrl: probe.authorizationUrl, | ||
| 553 | tokenUrl: probe.tokenUrl, | ||
| 554 | resource: probe.resource, | ||
| 555 | scopes: ["read"], | ||
| 556 | tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, | ||
| 557 | clientName: "Acme DCR", | ||
| 558 | redirectUri: FLOW_REDIRECT_URI, | ||
| 559 | originIntegration: INTEG, | ||
| 560 | }); | ||
| 561 | yield* server.clearRequests; | ||
| 562 | |||
| 563 | // Recreated sandbox: same persisted oauth_client rows, NEW callback | ||
| 564 | // origin. The register call runs again the way the connect flow does. | ||
| 565 | const recreatedRedirectUri = "https://localhost:6410/api/oauth/callback"; | ||
| 566 | const slug = yield* executor.oauth.registerDynamicClient({ | ||
| 567 | owner: "org", | ||
| 568 | slug: OAuthClientSlug.make("recreated-sandbox"), | ||
| 569 | issuer: probe.issuer, | ||
| 570 | registrationEndpoint: probe.registrationEndpoint!, | ||
| 571 | authorizationUrl: probe.authorizationUrl, | ||
| 572 | tokenUrl: probe.tokenUrl, | ||
| 573 | resource: probe.resource, | ||
| 574 | scopes: ["read"], | ||
| 575 | tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, | ||
| 576 | clientName: "Acme DCR", | ||
| 577 | redirectUri: recreatedRedirectUri, | ||
| 578 | originIntegration: INTEG, | ||
| 579 | }); | ||
| 580 | |||
| 581 | // The changed redirect URI must have minted a FRESH registration whose | ||
| 582 | // redirect_uris carry the recreated callback. Today this is 0: the | ||
| 583 | // stale client is reused and the authorize hop below 400s. | ||
| 584 | expect(registerRequestCount(yield* server.requests)).toBe(1); | ||
| 585 | |||
| 586 | const started = yield* executor.oauth.start({ | ||
| 587 | owner: "org", | ||
| 588 | client: slug, | ||
| 589 | clientOwner: "org", | ||
| 590 | name: ConnectionName.make("main"), | ||
| 591 | integration: INTEG, | ||
| 592 | template: TEMPLATE, | ||
| 593 | redirectUri: recreatedRedirectUri, | ||
| 594 | }); | ||
| 595 | expect(started.status).toBe("redirect"); | ||
| 596 | if (started.status !== "redirect") return; | ||
| 597 | |||
| 598 | // The strict AS accepts the authorize request only when the flow's | ||
| 599 | // redirect_uri matches the client's registration — with the stale | ||
| 600 | // reused client this hop is the reported 400. | ||
| 601 | const callback = yield* server.completeAuthorizationCodeFlow({ | ||
| 602 | authorizationUrl: started.authorizationUrl, | ||
| 603 | }); | ||
| 604 | const connection = yield* executor.oauth.complete({ | ||
| 605 | state: started.state, | ||
| 606 | code: callback.code, | ||
| 607 | }); | ||
| 608 | expect(String(connection.address)).toBe("tools.acme.org.main"); | ||
| 609 | }), | ||
| 610 | ), | ||
| 611 | ); | ||
| 612 | |||
| 530 | // Regression: issue #770. Vercel (and other RFC 8252-strict servers) only | 613 | // Regression: issue #770. Vercel (and other RFC 8252-strict servers) only |
| 531 | // approve loopback redirect URIs for anonymous DCR, so a hosted/tailnet/LAN | 614 | // approve loopback redirect URIs for anonymous DCR, so a hosted/tailnet/LAN |
| 532 | // origin trips `invalid_redirect_uri`. The failure must explain the loopback | 615 | // origin trips `invalid_redirect_uri`. The failure must explain the loopback |