diff --git a/.changeset/hubspot-workspace-optional-scopes.md b/.changeset/hubspot-workspace-optional-scopes.md new file mode 100644 index 000000000..71af545f2 --- /dev/null +++ b/.changeset/hubspot-workspace-optional-scopes.md @@ -0,0 +1,5 @@ +--- +"@executor-js/sdk": patch +--- + +Send HubSpot optional permissions in `optional_scope` for workspace OAuth clients so accounts can connect without optional product features. diff --git a/apps/cloud/src/engine/first-party-oauth-clients.ts b/apps/cloud/src/engine/first-party-oauth-clients.ts index 9c646e5d5..aa1359c16 100644 --- a/apps/cloud/src/engine/first-party-oauth-clients.ts +++ b/apps/cloud/src/engine/first-party-oauth-clients.ts @@ -6,6 +6,7 @@ import { } from "@executor-js/plugin-openapi/providers/microsoft"; import { slackMcpUserScopes } from "@executor-js/react/lib/slack-mcp-oauth"; import { IntegrationSlug, type FirstPartyOAuthClientConfig } from "@executor-js/sdk"; +import { HUBSPOT_OPTIONAL_SCOPES } from "@executor-js/sdk/host-internal"; import { makeGoogleOAuthListing } from "../analytics/google-oauth-listing"; import { POSTHOG_INGEST_HOST } from "../edge/passthrough"; @@ -152,12 +153,6 @@ const HUBSPOT_REQUIRED_SCOPES = [ "timeline", ] as const; -const HUBSPOT_OPTIONAL_SCOPES = [ - "content", - "crm.objects.custom.read", - "crm.schemas.custom.read", -] as const; - const MICROSOFT_SCOPES = [ "User.Read", "Calendars.ReadWrite", diff --git a/apps/cloud/src/mcp/session-build-semaphore.test.ts b/apps/cloud/src/mcp/session-build-semaphore.test.ts index 3d4ad7634..584b65ee0 100644 --- a/apps/cloud/src/mcp/session-build-semaphore.test.ts +++ b/apps/cloud/src/mcp/session-build-semaphore.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, beforeEach } from "@effect/vitest"; +import { describe, expect, it, beforeEach, afterEach, vi } from "@effect/vitest"; import { acquireBuildSlot, @@ -13,6 +13,10 @@ describe("session-build-semaphore", () => { resetBuildSlotsForTest(); }); + afterEach(() => { + vi.useRealTimers(); + }); + it("grants up to the cap immediately, with no wait", async () => { const results = await Promise.all([ acquireBuildSlot().promise, @@ -214,6 +218,7 @@ describe("session-build-semaphore", () => { }); it("proceeds without a slot when the queue wait exceeds the timeout, and does not count it as active", async () => { + vi.useFakeTimers(); await Promise.all([ acquireBuildSlot().promise, acquireBuildSlot().promise, @@ -223,6 +228,10 @@ describe("session-build-semaphore", () => { expect(currentActiveBuildsForTest()).toBe(4); const timedOutHandle = acquireBuildSlot(10); + await vi.advanceTimersByTimeAsync(9); + expect(currentQueueLengthForTest()).toBe(1); + expect(currentActiveBuildsForTest()).toBe(4); + await vi.advanceTimersByTimeAsync(1); const result = await timedOutHandle.promise; expect(result).toEqual({ acquired: false, waitMs: expect.any(Number), timedOut: true }); diff --git a/e2e/selfhost/oauth-optional-scopes.test.ts b/e2e/selfhost/oauth-optional-scopes.test.ts new file mode 100644 index 000000000..b14dac127 --- /dev/null +++ b/e2e/selfhost/oauth-optional-scopes.test.ts @@ -0,0 +1,172 @@ +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { connectEmulator } from "@executor-js/emulate"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + OAuthClientSlug, +} from "@executor-js/sdk/shared"; + +import { createEmulatorInstance } from "../src/emulator-instance"; +import { scenario } from "../src/scenario"; +import { Api, Browser, Mcp, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +scenario( + "OAuth optional scopes · the integration partitions scopes and completes an authenticated connection", + { timeout: 180_000 }, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const mcp = yield* Mcp; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const session = mcp.session(identity); + expect((yield* session.call("execute", { code: "return true;" })).ok).toBe(true); + const base = yield* createEmulatorInstance("github", "optional-scopes"); + const emulator = yield* Effect.promise(() => + connectEmulator({ baseUrl: base, service: "github" }), + ); + yield* Effect.promise(() => emulator.seed({ users: [{ login: "optional-scope-user" }] })); + const slug = IntegrationSlug.make(`optional-${randomBytes(4).toString("hex")}`); + const app = OAuthClientSlug.make(`${slug}-app`); + yield* Effect.addFinalizer(() => + client.openapi.removeSpec({ params: { slug } }).pipe(Effect.orDie), + ); + yield* Effect.addFinalizer(() => + client.oauth + .removeClient({ params: { slug: app }, payload: { owner: "org" } }) + .pipe(Effect.orDie), + ); + // GitHub supplies the real OAuth transport and protected resource. The test + // verifies Executor's partitioning contract; it does not claim that GitHub + // implements HubSpot's optional-grant policy. + const authorizationUrl = `${base}/login/oauth/authorize`; + const tokenUrl = `${base}/login/oauth/access_token`; + yield* client.openapi.addSpec({ + payload: { + slug, + baseUrl: base, + spec: { + kind: "blob", + value: JSON.stringify({ + openapi: "3.0.3", + info: { title: "Optional scope API", version: "1" }, + paths: { + "/user": { + get: { + operationId: "getUser", + security: [{ oauth: ["read:user"] }], + responses: { "200": { description: "Authenticated user" } }, + }, + }, + }, + components: { + securitySchemes: { + oauth: { + type: "oauth2", + flows: { + authorizationCode: { + authorizationUrl, + tokenUrl, + scopes: { + "read:user": "Read user", + "user:email": "Read email when granted", + }, + }, + }, + }, + }, + }, + }), + }, + authenticationTemplate: [ + { + slug: "oauth", + kind: "oauth2", + authorizationUrl: `${authorizationUrl}?optional_scope=user%3Aemail`, + tokenUrl, + scopes: ["read:user", "user:email"], + }, + ], + }, + }); + yield* client.oauth.createClient({ + payload: { + owner: "org", + slug: app, + grant: "authorization_code", + authorizationUrl, + tokenUrl, + clientId: "optional-test-client", + clientSecret: "optional-test-secret", + originIntegration: slug, + }, + }); + const started = yield* client.oauth.start({ + payload: { + owner: "org", + client: app, + clientOwner: "org", + name: ConnectionName.make("main"), + integration: slug, + template: AuthTemplateSlug.make("oauth"), + }, + }); + if (started.status !== "redirect") + return yield* Effect.die("Expected authorization redirect"); + + yield* Effect.addFinalizer(() => + client.oauth.cancel({ payload: { state: started.state } }).pipe(Effect.orDie), + ); + const url = new URL(started.authorizationUrl); + expect(url.searchParams.get("scope")).toBe("read:user"); + expect(url.searchParams.get("optional_scope")).toBe("user:email"); + yield* browser.session(identity, async ({ page, step }) => { + await step("Review and approve the OAuth consent request", async () => { + await page.goto(started.authorizationUrl); + await page.getByRole("button", { name: /optional-scope-user/ }).click(); + await page.getByText("Connected", { exact: true }).waitFor({ timeout: 30_000 }); + }); + }); + yield* Effect.addFinalizer(() => + client.connections + .remove({ + params: { owner: "org", integration: slug, name: ConnectionName.make("main") }, + }) + .pipe(Effect.orDie), + ); + const catalog = yield* client.tools.list({ query: { integration: slug } }); + const tool = catalog.find((entry) => entry.name.endsWith("getUser")); + if (!tool) + return yield* Effect.die( + `Authenticated getUser tool missing: ${catalog.map((entry) => entry.name).join(", ")}`, + ); + let result = yield* session.call("execute", { + code: `const path = ${JSON.stringify(String(tool.address))}.split(".").slice(1); let call = tools; for (const part of path) call = call[part]; return await call({});`, + }); + for (let attempts = 0; result.text.includes("executionId:") && attempts < 10; attempts += 1) + result = yield* session.approvePaused(result.text); + expect(result.ok).toBe(true); + expect(result.text).toContain("optional-scope-user"); + const ledger = yield* Effect.promise(() => emulator.ledger.list()); + const authorize = ledger.find( + (entry) => entry.method === "GET" && entry.path.endsWith("/login/oauth/authorize"), + ); + expect(new URLSearchParams(authorize?.query).get("optional_scope")).toBe("user:email"); + expect( + ledger + .filter((entry) => entry.path === "/user" && entry.response.status === 200) + .map((entry) => entry.identity.user?.login), + ).toContain("optional-scope-user"); + }), + ), +); diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index efaf11921..9e48fc056 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -208,6 +208,7 @@ import { exchangeClientCredentials, isPermanentTokenRejection, isUnusableSuccessTokenResponse, + optionalScopesFromAuthorizationUrl, shouldRefreshToken, type OAuth2TokenResponse, type OAuthEndpointUrlPolicy, @@ -6635,7 +6636,14 @@ export const createExecutor = { // buildAuthorizationUrl // --------------------------------------------------------------------------- -describe("providerAuthorizeExtras (Google offline/consent quirk)", () => { +describe("providerAuthorizeExtras (provider authorization quirks)", () => { it("adds access_type=offline + prompt=consent for the Google authorize host", () => { expect(providerAuthorizeExtras("https://accounts.google.com/o/oauth2/v2/auth")).toEqual({ access_type: "offline", prompt: "consent", }); }); - it("adds nothing for non-Google hosts or an unparseable URL (token host ≠ authorize host)", () => { + + it("adds optional_scope for workspace-owned HubSpot OAuth clients", () => { + expect(providerAuthorizeExtras("https://app.hubspot.com/oauth/authorize")).toEqual({ + optional_scope: "content crm.objects.custom.read crm.schemas.custom.read", + }); + }); + + it("reads integration-declared optional_scope values from an authorization URL", () => { + expect( + optionalScopesFromAuthorizationUrl( + "https://app.hubspot.com/oauth/authorize?optional_scope=crm.objects.contacts.read+crm.objects.contacts.write+crm.objects.contacts.read", + ), + ).toEqual(["crm.objects.contacts.read", "crm.objects.contacts.write"]); + expect(optionalScopesFromAuthorizationUrl("not a url")).toEqual([]); + }); + + it("adds nothing for unrelated hosts, token hosts, or an unparseable URL", () => { expect(providerAuthorizeExtras("https://accounts.spotify.com/authorize")).toEqual({}); expect(providerAuthorizeExtras("https://oauth2.googleapis.com/token")).toEqual({}); + expect(providerAuthorizeExtras("https://api.hubapi.com/oauth/v3/token")).toEqual({}); expect(providerAuthorizeExtras("not a url")).toEqual({}); }); }); diff --git a/packages/core/sdk/src/oauth-helpers.ts b/packages/core/sdk/src/oauth-helpers.ts index 2f7c39fff..ba3a000c4 100644 --- a/packages/core/sdk/src/oauth-helpers.ts +++ b/packages/core/sdk/src/oauth-helpers.ts @@ -113,6 +113,16 @@ export const OAUTH2_REFRESH_SKEW_MS = 60_000; /** Default token-endpoint timeout. */ export const OAUTH2_DEFAULT_TIMEOUT_MS = 20_000; +/** HubSpot scopes that the registered app may grant but must receive through + * HubSpot's non-standard `optional_scope` authorize parameter. Keeping these + * out of the RFC `scope` parameter lets accounts without the corresponding + * product features complete consent while still granting them when present. */ +export const HUBSPOT_OPTIONAL_SCOPES = [ + "content", + "crm.objects.custom.read", + "crm.schemas.custom.read", +] as const; + /** RFC 8693 §2.1 token-exchange grant. */ export const TOKEN_EXCHANGE_GRANT_TYPE = "urn:ietf:params:oauth:grant-type:token-exchange"; @@ -247,7 +257,12 @@ export const buildAuthorizationUrl = (input: BuildAuthorizationUrlInput): string * re-consent can silently keep the old scope set. Do not add * `include_granted_scopes=true` here: with historical grants on the same Google * consent app, Google folds those unrelated scopes into the new consent flow and - * can fail inside accounts.google.com before returning to our callback. */ + * can fail inside accounts.google.com before returning to our callback. + * + * HubSpot: app scopes marked optional are ignored when they are omitted from + * the provider-specific `optional_scope` parameter. The OpenAPI auth template + * can only declare RFC scopes, so this host-level quirk must apply to both + * first-party and workspace-owned HubSpot OAuth clients. */ export const providerAuthorizeExtras = ( authorizationUrl: string, ): Readonly> => { @@ -257,12 +272,30 @@ export const providerAuthorizeExtras = ( if (host === "accounts.google.com") { return { access_type: "offline", prompt: "consent" }; } + if (host === "app.hubspot.com") { + return { optional_scope: HUBSPOT_OPTIONAL_SCOPES.join(" ") }; + } } catch { // Unparseable authorization URL — let buildAuthorizationUrl surface the error. } return {}; }; +/** Provider-specific scopes embedded in an integration's authorization + * endpoint. HubSpot models app-optional permissions with the non-standard + * `optional_scope` query parameter, so they are part of the integration's + * request contract rather than the registered OAuth app identity. */ +export const optionalScopesFromAuthorizationUrl = (authorizationUrl: string): readonly string[] => { + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: URL() throws on invalid input -> no optional scopes + try { + const value = new URL(authorizationUrl).searchParams.get("optional_scope"); + if (value == null) return []; + return [...new Set(value.split(/\s+/).filter(Boolean))]; + } catch { + return []; + } +}; + // --------------------------------------------------------------------------- // Regional token-endpoint rebind // diff --git a/packages/core/sdk/src/oauth-scope-union.test.ts b/packages/core/sdk/src/oauth-scope-union.test.ts index 6e7aa4166..655f62dd5 100644 --- a/packages/core/sdk/src/oauth-scope-union.test.ts +++ b/packages/core/sdk/src/oauth-scope-union.test.ts @@ -37,7 +37,10 @@ const DECLARED_SCOPES = ["calendar", "gmail", "drive", "sheets"] as const; * scopes (the MCP/no-template-scopes case). */ const makeScopePluginWithId = ( id: TId, - config: { readonly scopes: readonly string[] | null }, + config: { + readonly scopes: readonly string[] | null; + readonly authorizationUrl?: string; + }, options: { readonly discoversScopes?: boolean; readonly discoveryUrl?: string } = {}, ) => definePlugin(() => ({ @@ -49,7 +52,10 @@ const makeScopePluginWithId = ( }), invokeTool: ({ credential }) => Effect.succeed({ token: credential.value }), describeAuthMethods: (record: IntegrationRecord): readonly AuthMethodDescriptor[] => { - const cfg = record.config as { readonly scopes?: readonly string[] | null } | null; + const cfg = record.config as { + readonly scopes?: readonly string[] | null; + readonly authorizationUrl?: string; + } | null; const scopes = cfg?.scopes; if (scopes == null) { // No declared oauth scopes. Server-targeting methods (MCP) expose a @@ -73,7 +79,12 @@ const makeScopePluginWithId = ( label: "OAuth2", kind: "oauth", template: String(TEMPLATE), - oauth: { scopes }, + oauth: { + scopes, + ...(cfg?.authorizationUrl === undefined + ? {} + : { authorizationUrl: cfg.authorizationUrl }), + }, }, ]; }, @@ -82,13 +93,20 @@ const makeScopePluginWithId = ( ctx.core.integrations.register({ slug: INTEG, description: "Acme", - config: { scopes: config.scopes }, + config: { + scopes: config.scopes, + ...(config.authorizationUrl === undefined + ? {} + : { authorizationUrl: config.authorizationUrl }), + }, }), }), }))(); -const makeScopePlugin = (config: { readonly scopes: readonly string[] | null }) => - makeScopePluginWithId("acme", config); +const makeScopePlugin = (config: { + readonly scopes: readonly string[] | null; + readonly authorizationUrl?: string; +}) => makeScopePluginWithId("acme", config); const makeMcpScopePlugin = (config: { readonly scopes: readonly string[] | null }) => makeScopePluginWithId("mcp", config, { discoversScopes: true }); @@ -223,6 +241,57 @@ describe("oauth.start integration-driven scopes", () => { ), ); + it.effect("moves integration-declared optional scopes out of scope into optional_scope", () => + Effect.scoped( + Effect.gen(function* () { + const declared = [ + "oauth", + "crm.objects.contacts.read", + "crm.objects.companies.read", + ] as const; + const optional = [ + "crm.objects.contacts.read", + "crm.objects.companies.read", + "content", + ] as const; + const server = yield* serveOAuthTestServer({ scopes: [...declared] }); + const plugins = [ + memoryCredentialsPlugin(), + makeScopePlugin({ + scopes: declared, + authorizationUrl: `${server.authorizationEndpoint}?optional_scope=${optional.join("+")}`, + }), + ] as const; + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + yield* executor.oauth.createClient({ + owner: "org", + slug: CLIENT, + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "authorization_code", + clientId: "test-client", + clientSecret: "test-secret", + }); + + const started = yield* executor.oauth.start({ + owner: "org", + client: CLIENT, + clientOwner: "org", + name: ConnectionName.make("main"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + + const url = new URL(started.authorizationUrl); + expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual(["oauth"]); + expect(url.searchParams.get("optional_scope")?.split(/\s+/)).toEqual(optional); + }), + ), + ); + it.effect("filters stale declared scopes against authorization-server metadata", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index dd443144b..57ee153bf 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -199,7 +199,14 @@ const startErrorFromEnterpriseManaged = (cause: EnterpriseManagedMintError): OAu * 8707 resource: a user may clear the client's resource (Entra v2 rejects * the parameter, #1789) without losing scope discovery. */ export type OAuthScopePolicy = - | { readonly kind: "scopes"; readonly scopes: readonly string[] } + | { + readonly kind: "scopes"; + readonly scopes: readonly string[]; + /** Provider-specific scopes declared on the integration's authorization + * endpoint (HubSpot `optional_scope`). These must not also be sent in + * the RFC `scope` parameter. */ + readonly optionalScopes?: readonly string[]; + } | { readonly kind: "discover"; readonly discoveryUrl: string }; /** Everything the OAuth service needs from the executor: fuma access for the @@ -1971,10 +1978,22 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { : scopePolicy.kind === "discover" ? requestedScopes : yield* filterAuthorizationCodeScopes(client, requestedScopes); + const providerExtras = providerAuthorizeExtras(client.authorizationUrl); + const workspaceOptionalScopes = firstPartyFlow + ? [] + : dedupeScopes([ + ...(providerExtras.optional_scope ?? "").split(/\s+/).filter(Boolean), + ...(scopePolicy.kind === "scopes" ? (scopePolicy.optionalScopes ?? []) : []), + ]); + const workspaceOptionalScopeSet = new Set(workspaceOptionalScopes); const completeAuthorizationScopes = dedupeScopes([ - ...authorizationRequestedScopes, + ...authorizationRequestedScopes.filter((scope) => !workspaceOptionalScopeSet.has(scope)), ...(firstParty?.additionalAuthorizationScopes ?? []), ]); + const completeRequestedScopes = dedupeScopes([ + ...completeAuthorizationScopes, + ...workspaceOptionalScopes, + ]); // authorization_code: persist a session + build the authorize URL. const verifier = createPkceCodeVerifier(); @@ -2036,7 +2055,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { payload: { owner: input.owner, clientOwner: input.clientOwner, - requestedScopes: completeAuthorizationScopes, + requestedScopes: completeRequestedScopes, }, expires_at: expiresAt, created_at: now, @@ -2058,7 +2077,10 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { // without these Google returns no refresh token and won't re-consent // to widen scopes on reconnect. extraParams: { - ...providerAuthorizeExtras(client.authorizationUrl), + ...providerExtras, + ...(workspaceOptionalScopes.length > 0 + ? { optional_scope: workspaceOptionalScopes.join(" ") } + : {}), ...(firstParty?.authorizationExtraParams ?? {}), }, endpointUrlPolicy: deps.endpointUrlPolicy,