Unified · Split
| @@ -274,3 +274,78 @@ scenario( · 273 unmodified lines | |||
| 274 | ); | 274 | ); |
| 275 | }), | 275 | }), |
| 276 | ); | 276 | ); |
| 277 | |||
| 278 | scenario( | ||
| 279 | "Google · OAuth catalog connection without a health check is healthy from the grant", | ||
| 280 | { timeout: 300_000 }, | ||
| 281 | Effect.gen(function* () { | ||
| 282 | const target = yield* Target; | ||
| 283 | const browser = yield* Browser; | ||
| 284 | const { client: makeClient } = yield* Api; | ||
| 285 | const identity = yield* target.newIdentity(); | ||
| 286 | const client = yield* makeClient(api, identity); | ||
| 287 | const emulator = yield* createGoogleEmulator; | ||
| 288 | const slug = IntegrationSlug.make("google_sheets"); | ||
| 289 | const oauthClient = OAuthClientSlug.make(unique("google_sheets_oauth")); | ||
| 290 | |||
| 291 | yield* Effect.ensuring( | ||
| 292 | Effect.gen(function* () { | ||
| 293 | yield* addGooglePresetFromCatalog(browser, identity, "Google Sheets", String(slug)); | ||
| 294 | |||
| 295 | const stored = yield* client.integrations.healthCheckGet({ params: { slug } }); | ||
| 296 | expect(stored, "Google Sheets catalog preset declares no health check").toBeNull(); | ||
| 297 | |||
| 298 | yield* connectGoogleAccount({ | ||
| 299 | client, | ||
| 300 | emulator: emulator.client, | ||
| 301 | emulatorBaseUrl: emulator.baseUrl, | ||
| 302 | target, | ||
| 303 | integration: slug, | ||
| 304 | oauthClient, | ||
| 305 | }); | ||
| 306 | |||
| 307 | const connections = yield* client.connections.list({ | ||
| 308 | query: { owner: "org", integration: slug }, | ||
| 309 | }); | ||
| 310 | const connected = connections.find((connection) => connection.name === CONNECTION); | ||
| 311 | expect( | ||
| 312 | connected?.identityLabel, | ||
| 313 | "Google Sheets stores grant identity before any probe is configured", | ||
| 314 | ).toBe(GOOGLE_EMULATOR_ACCOUNT_EMAIL); | ||
| 315 | expect( | ||
| 316 | connected?.lastHealth, | ||
| 317 | "Google Sheets has not run a health check before the explicit check", | ||
| 318 | ).toBeNull(); | ||
| 319 | |||
| 320 | const health = yield* client.connections.checkHealth({ | ||
| 321 | params: { owner: "org", integration: slug, name: CONNECTION }, | ||
| 322 | query: { ifStaleMs: 0 }, | ||
| 323 | }); | ||
| 324 | expect( | ||
| 325 | health.status, | ||
| 326 | `Google Sheets no-probe health is healthy: ${JSON.stringify(health)}`, | ||
| 327 | ).toBe("healthy"); | ||
| 328 | expect(health.detail).toBe("Credential resolved (no probe configured)."); | ||
| 329 | |||
| 330 | const refreshed = yield* client.connections.list({ | ||
| 331 | query: { owner: "org", integration: slug }, | ||
| 332 | }); | ||
| 333 | expect( | ||
| 334 | refreshed.find((connection) => connection.name === CONNECTION)?.identityLabel, | ||
| 335 | "Google Sheets still shows the OAuth grant identity after no-probe health", | ||
| 336 | ).toBe(GOOGLE_EMULATOR_ACCOUNT_EMAIL); | ||
| 337 | }), | ||
| 338 | Effect.gen(function* () { | ||
| 339 | yield* client.connections | ||
| 340 | .remove({ | ||
| 341 | params: { owner: "org", integration: slug, name: CONNECTION }, | ||
| 342 | }) | ||
| 343 | .pipe(Effect.ignore); | ||
| 344 | yield* client.oauth | ||
| 345 | .removeClient({ params: { slug: oauthClient }, payload: { owner: "org" } }) | ||
| 346 | .pipe(Effect.ignore); | ||
| 347 | yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); | ||
| 348 | }), | ||
| 349 | ); | ||
| 350 | }), | ||
| 351 | ); | ||
| @@ -58,6 +58,7 @@ const ConnectionResponse = Schema.Struct({ · 57 unmodified lines | |||
| 58 | oauthClient: Schema.NullOr(OAuthClientSlug), | 58 | oauthClient: Schema.NullOr(OAuthClientSlug), |
| 59 | oauthClientOwner: Schema.NullOr(Owner), | 59 | oauthClientOwner: Schema.NullOr(Owner), |
| 60 | oauthScope: Schema.NullOr(Schema.String), | 60 | oauthScope: Schema.NullOr(Schema.String), |
| 61 | missingOAuthScopes: Schema.Array(Schema.String), | ||
| 61 | // Last persisted health-check verdict (written by every checkHealth run), | 62 | // Last persisted health-check verdict (written by every checkHealth run), |
| 62 | // so the list can show alive/expired at a glance without probing. | 63 | // so the list can show alive/expired at a glance without probing. |
| 63 | lastHealth: Schema.NullOr(HealthCheckResult), | 64 | lastHealth: Schema.NullOr(HealthCheckResult), |
| @@ -28,6 +28,7 @@ const toResponse = (c: Connection) => ({ · 27 unmodified lines | |||
| 28 | oauthClient: c.oauthClient ?? null, | 28 | oauthClient: c.oauthClient ?? null, |
| 29 | oauthClientOwner: c.oauthClientOwner ?? null, | 29 | oauthClientOwner: c.oauthClientOwner ?? null, |
| 30 | oauthScope: c.oauthScope ?? null, | 30 | oauthScope: c.oauthScope ?? null, |
| 31 | missingOAuthScopes: c.missingOAuthScopes ?? [], | ||
| 31 | lastHealth: c.lastHealth ?? null, | 32 | lastHealth: c.lastHealth ?? null, |
| 32 | }); | 33 | }); |
| 33 | 34 | ||
| @@ -45,6 +45,7 @@ const connectionToResponse = (c: Connection) => ({ · 44 unmodified lines | |||
| 45 | oauthClient: c.oauthClient ?? null, | 45 | oauthClient: c.oauthClient ?? null, |
| 46 | oauthClientOwner: c.oauthClientOwner ?? null, | 46 | oauthClientOwner: c.oauthClientOwner ?? null, |
| 47 | oauthScope: c.oauthScope ?? null, | 47 | oauthScope: c.oauthScope ?? null, |
| 48 | missingOAuthScopes: c.missingOAuthScopes ?? [], | ||
| 48 | }); | 49 | }); |
| 49 | 50 | ||
| 50 | const startResultToResponse = (result: ConnectResult) => | 51 | const startResultToResponse = (result: ConnectResult) => |
| @@ -50,6 +50,7 @@ const ConnectionResponse = Schema.Struct({ · 49 unmodified lines | |||
| 50 | oauthClient: Schema.NullOr(OAuthClientSlug), | 50 | oauthClient: Schema.NullOr(OAuthClientSlug), |
| 51 | oauthClientOwner: Schema.NullOr(Owner), | 51 | oauthClientOwner: Schema.NullOr(Owner), |
| 52 | oauthScope: Schema.NullOr(Schema.String), | 52 | oauthScope: Schema.NullOr(Schema.String), |
| 53 | missingOAuthScopes: Schema.Array(Schema.String), | ||
| 53 | }); | 54 | }); |
| 54 | 55 | ||
| 55 | // --------------------------------------------------------------------------- | 56 | // --------------------------------------------------------------------------- |
| @@ -54,6 +54,9 @@ export interface Connection { · 53 unmodified lines | |||
| 54 | * declared scopes to decide whether this connection must reconnect to grant | 54 | * declared scopes to decide whether this connection must reconnect to grant |
| 55 | * newly-needed access. Null for static creds / when the AS omitted `scope`. */ | 55 | * newly-needed access. Null for static creds / when the AS omitted `scope`. */ |
| 56 | readonly oauthScope?: string | null; | 56 | readonly oauthScope?: string | null; |
| 57 | /** Requested authorization-code scopes the provider did not grant, excluding | ||
| 58 | * identity/refresh scopes. Informational only: connect still succeeds. */ | ||
| 59 | readonly missingOAuthScopes?: readonly string[]; | ||
| 57 | /** Last health-check verdict, persisted by every `checkHealth` run. Answers | 60 | /** Last health-check verdict, persisted by every `checkHealth` run. Answers |
| 58 | * "has this expired?" at a glance in the connections list without probing. | 61 | * "has this expired?" at a glance in the connections list without probing. |
| 59 | * Null/absent = never checked. */ | 62 | * Null/absent = never checked. */ |
| @@ -618,6 +618,29 @@ describe("tool catalog sync safety", () => { · 617 unmodified lines | |||
| 618 | ); | 618 | ); |
| 619 | }); | 619 | }); |
| 620 | 620 | ||
| 621 | describe("connections.checkHealth", () => { | ||
| 622 | it.effect("keeps API-key connections without a probe unknown", () => | ||
| 623 | Effect.gen(function* () { | ||
| 624 | const executor = yield* setup(); | ||
| 625 | yield* executor.connections.create({ | ||
| 626 | owner: "org", | ||
| 627 | name: ConnectionName.make("main"), | ||
| 628 | integration: INTEG, | ||
| 629 | template: TEMPLATE, | ||
| 630 | value: "secret-token", | ||
| 631 | }); | ||
| 632 | |||
| 633 | const result = yield* executor.connections.checkHealth({ | ||
| 634 | owner: "org", | ||
| 635 | integration: INTEG, | ||
| 636 | name: ConnectionName.make("main"), | ||
| 637 | }); | ||
| 638 | |||
| 639 | expect(result.status).toBe("unknown"); | ||
| 640 | }), | ||
| 641 | ); | ||
| 642 | }); | ||
| 643 | |||
| 621 | describe("execute over a connection", () => { | 644 | describe("execute over a connection", () => { |
| 622 | it.effect("resolves the credential value and hands it to invokeTool", () => | 645 | it.effect("resolves the credential value and hands it to invokeTool", () => |
| 623 | Effect.gen(function* () { | 646 | Effect.gen(function* () { |
| @@ -550,6 +550,15 @@ const rowToIntegrationRecord = ( · 549 unmodified lines | |||
| 550 | const decodeLastHealth = Schema.decodeUnknownOption(HealthCheckResult); | 550 | const decodeLastHealth = Schema.decodeUnknownOption(HealthCheckResult); |
| 551 | const decodeHealthCheckSpec = Schema.decodeUnknownOption(HealthCheckSpec); | 551 | const decodeHealthCheckSpec = Schema.decodeUnknownOption(HealthCheckSpec); |
| 552 | 552 | ||
| 553 | const missingOAuthScopesFromProviderState = (value: unknown): readonly string[] => { | ||
| 554 | const decoded = decodeJsonColumn(value); | ||
| 555 | if (decoded == null || typeof decoded !== "object" || Array.isArray(decoded)) return []; | ||
| 556 | const scopes = (decoded as Record<string, unknown>).missingOAuthScopes; | ||
| 557 | return Array.isArray(scopes) | ||
| 558 | ? scopes.filter((scope): scope is string => typeof scope === "string") | ||
| 559 | : []; | ||
| 560 | }; | ||
| 561 | |||
| 553 | const rowToConnection = (row: ConnectionRow): Connection => { | 562 | const rowToConnection = (row: ConnectionRow): Connection => { |
| 554 | const owner = row.owner as Owner; | 563 | const owner = row.owner as Owner; |
| 555 | const integration = IntegrationSlug.make(row.integration); | 564 | const integration = IntegrationSlug.make(row.integration); |
| @@ -568,6 +577,7 @@ const rowToConnection = (row: ConnectionRow): Connection => { · 12 unmodified lines | |||
| 568 | oauthClientOwner: | 577 | oauthClientOwner: |
| 569 | row.oauth_client_owner == null ? null : (String(row.oauth_client_owner) as Owner), | 578 | row.oauth_client_owner == null ? null : (String(row.oauth_client_owner) as Owner), |
| 570 | oauthScope: row.oauth_scope == null ? null : String(row.oauth_scope), | 579 | oauthScope: row.oauth_scope == null ? null : String(row.oauth_scope), |
| 580 | missingOAuthScopes: missingOAuthScopesFromProviderState(row.provider_state), | ||
| 571 | lastHealth: Option.getOrNull(decodeLastHealth(row.last_health)), | 581 | lastHealth: Option.getOrNull(decodeLastHealth(row.last_health)), |
| 572 | }; | 582 | }; |
| 573 | }; | 583 | }; |
| @@ -2447,6 +2457,10 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 1873 unmodified lines | |||
| 2447 | expires_at: input.expiresAt, | 2457 | expires_at: input.expiresAt, |
| 2448 | oauth_scope: input.oauthScope, | 2458 | oauth_scope: input.oauthScope, |
| 2449 | oauth_token_url: input.oauthTokenUrl ?? null, | 2459 | oauth_token_url: input.oauthTokenUrl ?? null, |
| 2460 | provider_state: | ||
| 2461 | input.missingOAuthScopes && input.missingOAuthScopes.length > 0 | ||
| 2462 | ? { missingOAuthScopes: input.missingOAuthScopes } | ||
| 2463 | : null, | ||
| 2450 | updated_at: now, | 2464 | updated_at: now, |
| 2451 | }; | 2465 | }; |
| 2452 | if (existing) { | 2466 | if (existing) { |
| @@ -2479,7 +2493,10 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 26 unmodified lines | |||
| 2479 | expires_at: input.expiresAt, | 2493 | expires_at: input.expiresAt, |
| 2480 | oauth_scope: input.oauthScope, | 2494 | oauth_scope: input.oauthScope, |
| 2481 | oauth_token_url: input.oauthTokenUrl ?? null, | 2495 | oauth_token_url: input.oauthTokenUrl ?? null, |
| 2482 | provider_state: null, | 2496 | provider_state: |
| 2497 | input.missingOAuthScopes && input.missingOAuthScopes.length > 0 | ||
| 2498 | ? { missingOAuthScopes: input.missingOAuthScopes } | ||
| 2499 | : null, | ||
| 2483 | created_at: now, | 2500 | created_at: now, |
| 2484 | updated_at: now, | 2501 | updated_at: now, |
| 2485 | }); | 2502 | }); |
| @@ -2513,7 +2530,10 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 27 unmodified lines | |||
| 2513 | expires_at: input.expiresAt, | 2530 | expires_at: input.expiresAt, |
| 2514 | oauth_scope: input.oauthScope, | 2531 | oauth_scope: input.oauthScope, |
| 2515 | oauth_token_url: input.oauthTokenUrl ?? null, | 2532 | oauth_token_url: input.oauthTokenUrl ?? null, |
| 2516 | provider_state: null, | 2533 | provider_state: |
| 2534 | input.missingOAuthScopes && input.missingOAuthScopes.length > 0 | ||
| 2535 | ? { missingOAuthScopes: input.missingOAuthScopes } | ||
| 2536 | : null, | ||
| 2517 | created_at: now, | 2537 | created_at: now, |
| 2518 | updated_at: now, | 2538 | updated_at: now, |
| 2519 | } as ConnectionRow); | 2539 | } as ConnectionRow); |
| @@ -2688,6 +2708,35 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 168 unmodified lines | |||
| 2688 | }), | 2708 | }), |
| 2689 | ); | 2709 | ); |
| 2690 | 2710 | ||
| 2711 | const healthFromCredentialResolutionFailure = ( | ||
| 2712 | failure: StorageFailure | CredentialResolutionError, | ||
| 2713 | ): HealthCheckResult => | ||
| 2714 | Predicate.isTagged("CredentialResolutionError")(failure) && failure.reauthRequired === true | ||
| 2715 | ? { | ||
| 2716 | status: "expired", | ||
| 2717 | checkedAt: Date.now(), | ||
| 2718 | // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: CredentialResolutionError carries a typed `message` field | ||
| 2719 | detail: failure.message, | ||
| 2720 | } | ||
| 2721 | : { | ||
| 2722 | status: "degraded", | ||
| 2723 | checkedAt: Date.now(), | ||
| 2724 | // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: failures here carry typed safe messages | ||
| 2725 | detail: failure.message, | ||
| 2726 | }; | ||
| 2727 | |||
| 2728 | const oauthCredentialHealthWithoutProbe = ( | ||
| 2729 | row: ConnectionRow, | ||
| 2730 | ): Effect.Effect<HealthCheckResult, never> => | ||
| 2731 | resolveConnectionValues(row).pipe( | ||
| 2732 | Effect.as({ | ||
| 2733 | status: "healthy" as const, | ||
| 2734 | checkedAt: Date.now(), | ||
| 2735 | detail: "Credential resolved (no probe configured).", | ||
| 2736 | }), | ||
| 2737 | Effect.catch((failure) => Effect.succeed(healthFromCredentialResolutionFailure(failure))), | ||
| 2738 | ); | ||
| 2739 | |||
| 2691 | // Resolve an in-flight credential's value map (key-first validation) without | 2740 | // Resolve an in-flight credential's value map (key-first validation) without |
| 2692 | // saving anything. Mirrors `resolveConnectionValues` for the saved-row path: | 2741 | // saving anything. Mirrors `resolveConnectionValues` for the saved-row path: |
| 2693 | // pasted `value`/`values` are used directly; `from` origins resolve through | 2742 | // pasted `value`/`values` are used directly; `from` origins resolve through |
| @@ -2747,6 +2796,12 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 53 unmodified lines | |||
| 2747 | const runtime = runtimes.get(integrationRow.plugin_id); | 2796 | const runtime = runtimes.get(integrationRow.plugin_id); |
| 2748 | const check = runtime?.plugin.checkHealth; | 2797 | const check = runtime?.plugin.checkHealth; |
| 2749 | if (!runtime || !check) return unknownHealth(); | 2798 | if (!runtime || !check) return unknownHealth(); |
| 2799 | const spec = describeHealthCheckForRow(integrationRow) ?? undefined; | ||
| 2800 | if (spec === undefined && connectionRow.oauth_client != null) { | ||
| 2801 | const result = yield* oauthCredentialHealthWithoutProbe(connectionRow); | ||
| 2802 | yield* persistHealthResult(ref, result); | ||
| 2803 | return result; | ||
| 2804 | } | ||
| 2750 | 2805 | ||
| 2751 | const result = yield* Effect.gen(function* () { | 2806 | const result = yield* Effect.gen(function* () { |
| 2752 | const values = yield* resolveConnectionValues(connectionRow); | 2807 | const values = yield* resolveConnectionValues(connectionRow); |
| @@ -2765,7 +2820,6 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 12 unmodified lines | |||
| 2765 | }; | 2820 | }; |
| 2766 | // Core resolves the declared spec (its own column) and hands it to the | 2821 | // Core resolves the declared spec (its own column) and hands it to the |
| 2767 | // plugin; plugins no longer read it out of their config. | 2822 | // plugin; plugins no longer read it out of their config. |
| 2768 | const spec = describeHealthCheckForRow(integrationRow) ?? undefined; | ||
| 2769 | return yield* foldPluginFailure( | 2823 | return yield* foldPluginFailure( |
| 2770 | check({ ctx: runtime.ctx, integration: record, credential, spec }), | 2824 | check({ ctx: runtime.ctx, integration: record, credential, spec }), |
| 2771 | `Health check for connection "${ref.name}" failed.`, | 2825 | `Health check for connection "${ref.name}" failed.`, |
| @@ -849,6 +849,170 @@ describe("oauth token refresh in resolveConnectionValue", () => { · 848 unmodified lines | |||
| 849 | }), | 849 | }), |
| 850 | ), | 850 | ), |
| 851 | ); | 851 | ); |
| 852 | |||
| 853 | it.effect( | ||
| 854 | "checkHealth reports healthy from OAuth credential resolution when no probe is configured", | ||
| 855 | () => | ||
| 856 | Effect.scoped( | ||
| 857 | Effect.gen(function* () { | ||
| 858 | const server = yield* serveOAuthTestServer({ scopes: ["read"] }); | ||
| 859 | const { executor } = yield* makeTestWorkspaceHarness({ plugins }); | ||
| 860 | yield* executor.acme.seed(); | ||
| 861 | |||
| 862 | yield* executor.oauth.createClient({ | ||
| 863 | owner: "org", | ||
| 864 | slug: CLIENT, | ||
| 865 | authorizationUrl: server.authorizationEndpoint, | ||
| 866 | tokenUrl: server.tokenEndpoint, | ||
| 867 | grant: "authorization_code", | ||
| 868 | clientId: "test-client", | ||
| 869 | clientSecret: "test-secret", | ||
| 870 | resource: server.mcpResourceUrl, | ||
| 871 | }); | ||
| 872 | |||
| 873 | const started = yield* executor.oauth.start({ | ||
| 874 | owner: "org", | ||
| 875 | client: CLIENT, | ||
| 876 | clientOwner: "org", | ||
| 877 | name: ConnectionName.make("main"), | ||
| 878 | integration: INTEG, | ||
| 879 | template: TEMPLATE, | ||
| 880 | }); | ||
| 881 | expect(started.status).toBe("redirect"); | ||
| 882 | if (started.status !== "redirect") return; | ||
| 883 | const callback = yield* server.completeAuthorizationCodeFlow({ | ||
| 884 | authorizationUrl: started.authorizationUrl, | ||
| 885 | }); | ||
| 886 | yield* executor.oauth.complete({ | ||
| 887 | state: started.state, | ||
| 888 | code: callback.code, | ||
| 889 | }); | ||
| 890 | yield* server.clearRequests; | ||
| 891 | |||
| 892 | const result = yield* executor.connections.checkHealth({ | ||
| 893 | owner: "org", | ||
| 894 | integration: INTEG, | ||
| 895 | name: ConnectionName.make("main"), | ||
| 896 | }); | ||
| 897 | |||
| 898 | expect(result).toMatchObject({ | ||
| 899 | status: "healthy", | ||
| 900 | detail: "Credential resolved (no probe configured).", | ||
| 901 | }); | ||
| 902 | expect(result.identity).toBeUndefined(); | ||
| 903 | const requests = yield* server.requests; | ||
| 904 | expect(requests).toEqual([]); | ||
| 905 | }), | ||
| 906 | ), | ||
| 907 | ); | ||
| 908 | |||
| 909 | it.effect("records missing authorization-code scopes without blocking the connection", () => | ||
| 910 | Effect.scoped( | ||
| 911 | Effect.gen(function* () { | ||
| 912 | const server = yield* serveOAuthTestServer({ | ||
| 913 | scopes: ["openid", "email", "profile", "offline_access", "read", "write"], | ||
| 914 | omitTokenResponseScopes: ["email", "profile", "write"], | ||
| 915 | }); | ||
| 916 | const { executor, config } = yield* makeTestWorkspaceHarness({ plugins }); | ||
| 917 | yield* executor.acme.seed([ | ||
| 918 | "openid", | ||
| 919 | "email", | ||
| 920 | "profile", | ||
| 921 | "offline_access", | ||
| 922 | "read", | ||
| 923 | "write", | ||
| 924 | ]); | ||
| 925 | |||
| 926 | yield* executor.oauth.createClient({ | ||
| 927 | owner: "org", | ||
| 928 | slug: CLIENT, | ||
| 929 | authorizationUrl: server.authorizationEndpoint, | ||
| 930 | tokenUrl: server.tokenEndpoint, | ||
| 931 | grant: "authorization_code", | ||
| 932 | clientId: "test-client", | ||
| 933 | clientSecret: "test-secret", | ||
| 934 | resource: server.mcpResourceUrl, | ||
| 935 | }); | ||
| 936 | |||
| 937 | const started = yield* executor.oauth.start({ | ||
| 938 | owner: "org", | ||
| 939 | client: CLIENT, | ||
| 940 | clientOwner: "org", | ||
| 941 | name: ConnectionName.make("main"), | ||
| 942 | integration: INTEG, | ||
| 943 | template: TEMPLATE, | ||
| 944 | }); | ||
| 945 | expect(started.status).toBe("redirect"); | ||
| 946 | if (started.status !== "redirect") return; | ||
| 947 | const callback = yield* server.completeAuthorizationCodeFlow({ | ||
| 948 | authorizationUrl: started.authorizationUrl, | ||
| 949 | }); | ||
| 950 | const connection = yield* executor.oauth.complete({ | ||
| 951 | state: started.state, | ||
| 952 | code: callback.code, | ||
| 953 | }); | ||
| 954 | |||
| 955 | expect(connection.missingOAuthScopes).toEqual(["write"]); | ||
| 956 | const row = yield* Effect.promise(() => | ||
| 957 | config.db.findFirst("connection", { | ||
| 958 | where: (b) => b("name", "=", "main"), | ||
| 959 | }), | ||
| 960 | ); | ||
| 961 | expect(row?.provider_state).toEqual({ missingOAuthScopes: ["write"] }); | ||
| 962 | const listed = yield* executor.connections.list({ integration: INTEG }); | ||
| 963 | expect(listed[0]?.missingOAuthScopes).toEqual(["write"]); | ||
| 964 | }), | ||
| 965 | ), | ||
| 966 | ); | ||
| 967 | |||
| 968 | it.effect( | ||
| 969 | "does not persist missing scopes when the authorization-code grant covers requested scopes", | ||
| 970 | () => | ||
| 971 | Effect.scoped( | ||
| 972 | Effect.gen(function* () { | ||
| 973 | const server = yield* serveOAuthTestServer({ scopes: ["read", "write"] }); | ||
| 974 | const { executor, config } = yield* makeTestWorkspaceHarness({ plugins }); | ||
| 975 | yield* executor.acme.seed(["read", "write"]); | ||
| 976 | |||
| 977 | yield* executor.oauth.createClient({ | ||
| 978 | owner: "org", | ||
| 979 | slug: CLIENT, | ||
| 980 | authorizationUrl: server.authorizationEndpoint, | ||
| 981 | tokenUrl: server.tokenEndpoint, | ||
| 982 | grant: "authorization_code", | ||
| 983 | clientId: "test-client", | ||
| 984 | clientSecret: "test-secret", | ||
| 985 | resource: server.mcpResourceUrl, | ||
| 986 | }); | ||
| 987 | |||
| 988 | const started = yield* executor.oauth.start({ | ||
| 989 | owner: "org", | ||
| 990 | client: CLIENT, | ||
| 991 | clientOwner: "org", | ||
| 992 | name: ConnectionName.make("main"), | ||
| 993 | integration: INTEG, | ||
| 994 | template: TEMPLATE, | ||
| 995 | }); | ||
| 996 | expect(started.status).toBe("redirect"); | ||
| 997 | if (started.status !== "redirect") return; | ||
| 998 | const callback = yield* server.completeAuthorizationCodeFlow({ | ||
| 999 | authorizationUrl: started.authorizationUrl, | ||
| 1000 | }); | ||
| 1001 | const connection = yield* executor.oauth.complete({ | ||
| 1002 | state: started.state, | ||
| 1003 | code: callback.code, | ||
| 1004 | }); | ||
| 1005 | |||
| 1006 | expect(connection.missingOAuthScopes).toEqual([]); | ||
| 1007 | const row = yield* Effect.promise(() => | ||
| 1008 | config.db.findFirst("connection", { | ||
| 1009 | where: (b) => b("name", "=", "main"), | ||
| 1010 | }), | ||
| 1011 | ); | ||
| 1012 | expect(row?.provider_state).toBeNull(); | ||
| 1013 | }), | ||
| 1014 | ), | ||
| 1015 | ); | ||
| 852 | }); | 1016 | }); |
| 853 | 1017 | ||
| 854 | // Multi-site providers (Datadog) statically advertise one region's token | 1018 | // Multi-site providers (Datadog) statically advertise one region's token |
| @@ -91,6 +91,7 @@ export interface MintOAuthConnectionInput { · 90 unmodified lines | |||
| 91 | readonly refreshItemId: string | null; | 91 | readonly refreshItemId: string | null; |
| 92 | readonly expiresAt: number | null; | 92 | readonly expiresAt: number | null; |
| 93 | readonly oauthScope: string | null; | 93 | readonly oauthScope: string | null; |
| 94 | readonly missingOAuthScopes?: readonly string[]; | ||
| 94 | /** Per-connection override for the token endpoint, persisted only when the | 95 | /** Per-connection override for the token endpoint, persisted only when the |
| 95 | * code was redeemed at a region other than the client's configured token | 96 | * code was redeemed at a region other than the client's configured token |
| 96 | * host (Datadog multi-site). Null means refresh uses the client's token URL. */ | 97 | * host (Datadog multi-site). Null means refresh uses the client's token URL. */ |
| @@ -203,6 +204,34 @@ const recordedOAuthScope = ( · 106 unmodified lines | |||
| 203 | return recorded.join(" ") || null; | 204 | return recorded.join(" ") || null; |
| 204 | }; | 205 | }; |
| 205 | 206 | ||
| 207 | const OAUTH_SCOPE_ALIASES: Readonly<Record<string, string>> = { | ||
| 208 | "https://www.googleapis.com/auth/userinfo.email": "email", | ||
| 209 | "https://www.googleapis.com/auth/userinfo.profile": "profile", | ||
| 210 | }; | ||
| 211 | |||
| 212 | const informationalOAuthScopes = new Set(["openid", "email", "profile", "offline_access"]); | ||
| 213 | |||
| 214 | const canonicalOAuthScope = (scope: string): string => OAUTH_SCOPE_ALIASES[scope] ?? scope; | ||
| 215 | |||
| 216 | const normalizedOAuthScopeSet = (scopes: readonly string[]): ReadonlySet<string> => | ||
| 217 | new Set(scopes.map((scope) => canonicalOAuthScope(scope.trim())).filter(Boolean)); | ||
| 218 | |||
| 219 | const missingGrantedOAuthScopes = ( | ||
| 220 | requestedScopes: readonly string[], | ||
| 221 | recordedScope: string | null, | ||
| 222 | ): readonly string[] => { | ||
| 223 | const granted = normalizedOAuthScopeSet(recordedScope?.split(/\s+/).filter(Boolean) ?? []); | ||
| 224 | const seen = new Set<string>(); | ||
| 225 | const out: string[] = []; | ||
| 226 | for (const raw of requestedScopes) { | ||
| 227 | const scope = canonicalOAuthScope(raw.trim()); | ||
| 228 | if (scope.length === 0 || informationalOAuthScopes.has(scope) || seen.has(scope)) continue; | ||
| 229 | seen.add(scope); | ||
| 230 | if (!granted.has(scope)) out.push(scope); | ||
| 231 | } | ||
| 232 | return out; | ||
| 233 | }; | ||
| 234 | |||
| 206 | const decodeJsonPayload = Schema.decodeUnknownOption(Schema.UnknownFromJsonString); | 235 | const decodeJsonPayload = Schema.decodeUnknownOption(Schema.UnknownFromJsonString); |
| 207 | 236 | ||
| 208 | /** Extract the persisted `requestedScopes` from an `oauth_session.payload`. The | 237 | /** Extract the persisted `requestedScopes` from an `oauth_session.payload`. The |
| @@ -1278,6 +1307,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 1069 unmodified lines | |||
| 1278 | yield* provider.set(ProviderItemId.make(refreshItemId), token.refresh_token); | 1307 | yield* provider.set(ProviderItemId.make(refreshItemId), token.refresh_token); |
| 1279 | } | 1308 | } |
| 1280 | 1309 | ||
| 1310 | const oauthScope = recordedOAuthScope(token, requestedScopes); | ||
| 1281 | return yield* deps.mintOAuthConnection({ | 1311 | return yield* deps.mintOAuthConnection({ |
| 1282 | owner: target.owner, | 1312 | owner: target.owner, |
| 1283 | name: target.name, | 1313 | name: target.name, |
| @@ -1294,7 +1324,11 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 10 unmodified lines | |||
| 1294 | // Microsoft, issue a refresh token for `offline_access` but omit that | 1324 | // Microsoft, issue a refresh token for `offline_access` but omit that |
| 1295 | // non-resource scope from the token `scope` string, so preserve it when | 1325 | // non-resource scope from the token `scope` string, so preserve it when |
| 1296 | // the refresh token proves it was granted. | 1326 | // the refresh token proves it was granted. |
| 1297 | oauthScope: recordedOAuthScope(token, requestedScopes), | 1327 | oauthScope, |
| 1328 | missingOAuthScopes: | ||
| 1329 | client.grant === "authorization_code" | ||
| 1330 | ? missingGrantedOAuthScopes(requestedScopes, oauthScope) | ||
| 1331 | : [], | ||
| 1298 | oauthTokenUrl, | 1332 | oauthTokenUrl, |
| 1299 | }); | 1333 | }); |
| 1300 | }); | 1334 | }); |
| @@ -1,6 +1,20 @@ | |||
| 1 | import { describe, expect, it } from "@effect/vitest"; | 1 | import { describe, expect, it } from "@effect/vitest"; |
| 2 | import * as React from "react"; | ||
| 2 | 3 | ||
| 3 | import { baseUrlFromSpecInput, openApiPreviewFailureMessage } from "./AddOpenApiSource"; | 4 | import { |
| 5 | AddOpenApiHealthCheckSection, | ||
| 6 | baseUrlFromSpecInput, | ||
| 7 | openApiPreviewFailureMessage, | ||
| 8 | } from "./AddOpenApiSource"; | ||
| 9 | |||
| 10 | const visibleText = (node: React.ReactNode): string => { | ||
| 11 | if (node === null || node === undefined || typeof node === "boolean") return ""; | ||
| 12 | if (typeof node === "string" || typeof node === "number") return String(node); | ||
| 13 | if (Array.isArray(node)) return node.map(visibleText).join(""); | ||
| 14 | if (!React.isValidElement(node)) return ""; | ||
| 15 | const props = node.props as { readonly children?: React.ReactNode }; | ||
| 16 | return visibleText(props.children); | ||
| 17 | }; | ||
| 4 | 18 | ||
| 5 | describe("baseUrlFromSpecInput", () => { | 19 | describe("baseUrlFromSpecInput", () => { |
| 6 | it("defaults URL-hosted specs to their origin", () => { | 20 | it("defaults URL-hosted specs to their origin", () => { |
| @@ -27,3 +41,56 @@ describe("openApiPreviewFailureMessage", () => { · 20 unmodified lines | |||
| 27 | ); | 41 | ); |
| 28 | }); | 42 | }); |
| 29 | }); | 43 | }); |
| 44 | |||
| 45 | describe("AddOpenApiHealthCheckSection", () => { | ||
| 46 | const candidate = { | ||
| 47 | operation: "getCurrentUser", | ||
| 48 | method: "get", | ||
| 49 | requiredArgCount: 0, | ||
| 50 | destructive: false, | ||
| 51 | summary: "Get current user", | ||
| 52 | parameters: [], | ||
| 53 | responseFields: [{ path: "email", type: "string" }], | ||
| 54 | }; | ||
| 55 | |||
| 56 | it("renders a read-only catalog line when the preset has a health check", () => { | ||
| 57 | const text = visibleText( | ||
| 58 | AddOpenApiHealthCheckSection({ | ||
| 59 | presetHealthCheck: { operation: "getCurrentUser" }, | ||
| 60 | candidates: [candidate], | ||
| 61 | selected: candidate, | ||
| 62 | operation: "", | ||
| 63 | onOperationChange: () => {}, | ||
| 64 | identityField: "", | ||
| 65 | onIdentityFieldChange: () => {}, | ||
| 66 | args: {}, | ||
| 67 | onArgChange: () => {}, | ||
| 68 | disabled: false, | ||
| 69 | }), | ||
| 70 | ); | ||
| 71 | |||
| 72 | expect(text).toContain("getCurrentUser"); | ||
| 73 | expect(text).toContain("From the catalog. Change it later from the integration page."); | ||
| 74 | expect(text).not.toContain("Health check (optional)"); | ||
| 75 | }); | ||
| 76 | |||
| 77 | it("renders the full optional editor when there is no preset health check", () => { | ||
| 78 | const text = visibleText( | ||
| 79 | AddOpenApiHealthCheckSection({ | ||
| 80 | candidates: [candidate], | ||
| 81 | selected: candidate, | ||
| 82 | operation: "getCurrentUser", | ||
| 83 | onOperationChange: () => {}, | ||
| 84 | identityField: "", | ||
| 85 | onIdentityFieldChange: () => {}, | ||
| 86 | args: {}, | ||
| 87 | onArgChange: () => {}, | ||
| 88 | disabled: false, | ||
| 89 | }), | ||
| 90 | ); | ||
| 91 | |||
| 92 | expect(text).toContain("Health check (optional)"); | ||
| 93 | expect(text).toContain("OAuth connections, validity and identity come from the OAuth grant"); | ||
| 94 | expect(text).not.toContain("From the catalog."); | ||
| 95 | }); | ||
| 96 | }); | ||
| @@ -83,6 +83,58 @@ export const baseUrlFromSpecInput = (input: string): string => { · 82 unmodified lines | |||
| 83 | export const openApiPreviewFailureMessage = (message: string | null | undefined): string => | 83 | export const openApiPreviewFailureMessage = (message: string | null | undefined): string => |
| 84 | `Couldn't load or parse this spec: ${message && message.trim().length > 0 ? message : "unknown error"}`; | 84 | `Couldn't load or parse this spec: ${message && message.trim().length > 0 ? message : "unknown error"}`; |
| 85 | 85 | ||
| 86 | export function AddOpenApiHealthCheckSection(props: { | ||
| 87 | readonly presetHealthCheck?: HealthCheckSpec; | ||
| 88 | readonly candidates: readonly HealthCheckCandidate[]; | ||
| 89 | readonly selected: HealthCheckCandidate | null; | ||
| 90 | readonly operation: string; | ||
| 91 | readonly onOperationChange: (operation: string) => void; | ||
| 92 | readonly identityField: string; | ||
| 93 | readonly onIdentityFieldChange: (field: string) => void; | ||
| 94 | readonly args: Record<string, string>; | ||
| 95 | readonly onArgChange: (name: string, value: string) => void; | ||
| 96 | readonly disabled: boolean; | ||
| 97 | }) { | ||
| 98 | if (props.presetHealthCheck) { | ||
| 99 | return ( | ||
| 100 | <section className="space-y-2"> | ||
| 101 | <h3 className="text-sm font-medium text-foreground">Health check</h3> | ||
| 102 | <p className="text-[11px] text-muted-foreground"> | ||
| 103 | <span className="font-mono text-foreground">{props.presetHealthCheck.operation}</span>{" "} | ||
| 104 | From the catalog. Change it later from the integration page. | ||
| 105 | </p> | ||
| 106 | </section> | ||
| 107 | ); | ||
| 108 | } | ||
| 109 | |||
| 110 | if (props.candidates.length === 0) return null; | ||
| 111 | |||
| 112 | return ( | ||
| 113 | <section className="space-y-4"> | ||
| 114 | <div className="space-y-1"> | ||
| 115 | <h3 className="text-sm font-medium text-foreground">Health check (optional)</h3> | ||
| 116 | <p className="text-[11px] text-muted-foreground"> | ||
| 117 | An optional read-only probe adds live upstream depth. For OAuth connections, validity and | ||
| 118 | identity come from the OAuth grant itself. For non-OAuth methods, the probe can validate | ||
| 119 | the credential and identify the account. | ||
| 120 | </p> | ||
| 121 | </div> | ||
| 122 | <HealthCheckConfigFields | ||
| 123 | candidates={props.candidates} | ||
| 124 | selected={props.selected} | ||
| 125 | operation={props.operation} | ||
| 126 | onOperationChange={props.onOperationChange} | ||
| 127 | identityField={props.identityField} | ||
| 128 | onIdentityFieldChange={props.onIdentityFieldChange} | ||
| 129 | args={props.args} | ||
| 130 | onArgChange={props.onArgChange} | ||
| 131 | disabled={props.disabled} | ||
| 132 | idPrefix="add-health-check" | ||
| 133 | /> | ||
| 134 | </section> | ||
| 135 | ); | ||
| 136 | } | ||
| 137 | |||
| 86 | // --------------------------------------------------------------------------- | 138 | // --------------------------------------------------------------------------- |
| 87 | // Component: single progressive form. Post-redesign: preview -> addSpec | 139 | // Component: single progressive form. Post-redesign: preview -> addSpec |
| 88 | // (register the integration catalog entry with ALL detected auth methods) → | 140 | // (register the integration catalog entry with ALL detected auth methods) → |
| @@ -491,30 +543,20 @@ export default function AddOpenApiSource(props: { · 402 unmodified lines | |||
| 491 | /> | 543 | /> |
| 492 | )} | 544 | )} |
| 493 | 545 | ||
| 494 | {preview && healthCheckCandidates.length > 0 && ( | 546 | {preview ? ( |
| 495 | <section className="space-y-4"> | 547 | <AddOpenApiHealthCheckSection |
| 496 | <div className="space-y-1"> | 548 | presetHealthCheck={initialPreset?.healthCheck} |
| 497 | <h3 className="text-sm font-medium text-foreground">Health check (optional)</h3> | 549 | candidates={healthCheckCandidates} |
| 498 | <p className="text-[11px] text-muted-foreground"> | 550 | selected={hcSelected} |
| 499 | One read-only call Executor runs to tell whether a connection's credential is still | 551 | operation={hcOperation} |
| 500 | alive and, optionally, whose account it is. You can change this later, and run a live | 552 | onOperationChange={onHcOperationChange} |
| 501 | preview against a key, from the integration page. | 553 | identityField={hcIdentityField} |
| 502 | </p> | 554 | onIdentityFieldChange={setHcIdentityField} |
| 503 | </div> | 555 | args={hcArgs} |
| 504 | <HealthCheckConfigFields | 556 | onArgChange={onHcArgChange} |
| 505 | candidates={healthCheckCandidates} | 557 | disabled={adding} |
| 506 | selected={hcSelected} | 558 | /> |
| 507 | operation={hcOperation} | 559 | ) : null} |
| 508 | onOperationChange={onHcOperationChange} | ||
| 509 | identityField={hcIdentityField} | ||
| 510 | onIdentityFieldChange={setHcIdentityField} | ||
| 511 | args={hcArgs} | ||
| 512 | onArgChange={onHcArgChange} | ||
| 513 | disabled={adding} | ||
| 514 | idPrefix="add-health-check" | ||
| 515 | /> | ||
| 516 | </section> | ||
| 517 | )} | ||
| 518 | 560 | ||
| 519 | {preview && slugAlreadyExists && !adding && <SlugCollisionAlert slug={resolvedSourceId} />} | 561 | {preview && slugAlreadyExists && !adding && <SlugCollisionAlert slug={resolvedSourceId} />} |
| 520 | 562 | ||
| @@ -366,6 +366,7 @@ export const addConnectionOptimistic = Atom.family((owner: Owner) => · 365 unmodified lines | |||
| 366 | oauthClient: null, | 366 | oauthClient: null, |
| 367 | oauthClientOwner: null, | 367 | oauthClientOwner: null, |
| 368 | oauthScope: null, | 368 | oauthScope: null, |
| 369 | missingOAuthScopes: [], | ||
| 369 | lastHealth: null, | 370 | lastHealth: null, |
| 370 | }; | 371 | }; |
| 371 | return [optimistic, ...rows]; | 372 | return [optimistic, ...rows]; |
| @@ -95,6 +95,7 @@ function AccountRow(props: { · 94 unmodified lines | |||
| 95 | const displayLabel = identity ?? String(connection.name); | 95 | const displayLabel = identity ?? String(connection.name); |
| 96 | 96 | ||
| 97 | const expired = status === "expired"; | 97 | const expired = status === "expired"; |
| 98 | const missingOAuthScopes = connection.missingOAuthScopes ?? []; | ||
| 98 | 99 | ||
| 99 | const handleCheck = async () => { | 100 | const handleCheck = async () => { |
| 100 | if (checking) return; | 101 | if (checking) return; |
| @@ -150,6 +151,11 @@ function AccountRow(props: { · 49 unmodified lines | |||
| 150 | This connection wasn't granted all the access this integration now needs. | 151 | This connection wasn't granted all the access this integration now needs. |
| 151 | </CardStackEntryDescription> | 152 | </CardStackEntryDescription> |
| 152 | ) : null} | 153 | ) : null} |
| 154 | {missingOAuthScopes.length > 0 ? ( | ||
| 155 | <CardStackEntryDescription className="mt-1 text-xs text-muted-foreground"> | ||
| 156 | Missing scopes: {missingOAuthScopes.join(", ")} | ||
| 157 | </CardStackEntryDescription> | ||
| 158 | ) : null} | ||
| 153 | </CardStackEntryContent> | 159 | </CardStackEntryContent> |
| 154 | <CardStackEntryActions className="self-start pt-0.5"> | 160 | <CardStackEntryActions className="self-start pt-0.5"> |
| 155 | {props.showOwnerLabel ? ( | 161 | {props.showOwnerLabel ? ( |
| @@ -477,8 +477,8 @@ function HealthCheckEditorSheet(props: { · 476 unmodified lines | |||
| 477 | <SheetHeader> | 477 | <SheetHeader> |
| 478 | <SheetTitle>Health check</SheetTitle> | 478 | <SheetTitle>Health check</SheetTitle> |
| 479 | <SheetDescription> | 479 | <SheetDescription> |
| 480 | One read-only call Executor runs to tell whether a connection's credential is still | 480 | Optional read-only depth for live upstream checks. OAuth validity and identity come from |
| 481 | alive and, optionally, whose account it is. A 401 or 403 marks the connection expired. | 481 | the grant itself. For non-OAuth methods, a 401 or 403 marks the connection expired. |
| 482 | </SheetDescription> | 482 | </SheetDescription> |
| 483 | </SheetHeader> | 483 | </SheetHeader> |
| 484 | 484 | ||
| @@ -553,8 +553,8 @@ export function HealthCheckEditor(props: { · 68 unmodified lines | |||
| 553 | <div className="space-y-1"> | 553 | <div className="space-y-1"> |
| 554 | <h3 className="text-sm font-medium">Health check</h3> | 554 | <h3 className="text-sm font-medium">Health check</h3> |
| 555 | <p className="text-xs text-muted-foreground"> | 555 | <p className="text-xs text-muted-foreground"> |
| 556 | A read-only call Executor runs to tell whether a connection's credential is still alive | 556 | Optional read-only depth for live upstream checks. OAuth identity comes from the account |
| 557 | and, optionally, whose account it is. | 557 | grant, while probe identity fields are useful for non-OAuth methods. |
| 558 | </p> | 558 | </p> |
| 559 | </div> | 559 | </div> |
| 560 | <div className="flex items-center justify-between gap-3 rounded-md border border-border/60 bg-muted/30 px-3 py-2.5"> | 560 | <div className="flex items-center justify-between gap-3 rounded-md border border-border/60 bg-muted/30 px-3 py-2.5"> |