Skip to content

Fix HubSpot optional scopes for workspace OAuth - #1898

Open
LloydVickeryASI wants to merge 3 commits into
UsefulSoftwareCo:mainfrom
LloydVickeryASI:codex/hubspot-optional-scope-workspace
Open

Fix HubSpot optional scopes for workspace OAuth#1898
LloydVickeryASI wants to merge 3 commits into
UsefulSoftwareCo:mainfrom
LloydVickeryASI:codex/hubspot-optional-scope-workspace

Conversation

@LloydVickeryASI

@LloydVickeryASI LloydVickeryASI commented Aug 31, 2026

Copy link
Copy Markdown

Summary

  • keep HubSpot's deployment-default optional scopes in the generated optional_scope parameter for workspace-owned OAuth clients
  • honor additional per-integration optional_scope values embedded in an OAuth auth template, remove those values from the RFC scope parameter, and record the complete requested set for callback fallback
  • preserve the first-party HubSpot client's separate registration contract
  • add regression coverage for both provider defaults and integration-declared scope partitioning

Why

HubSpot decides whether each app permission is required or optional in the app registration. Optional permissions are ignored when sent only in the RFC scope parameter; they must be sent through HubSpot's non-standard optional_scope parameter.

The ASI NZ workspace app demonstrates why this classification must be integration-specific: its contacts, companies, deals, and tickets permissions are configured as optional, while Executor's shared first-party app has a different required/optional split. Hard-coding one larger global optional set would break apps with another registration contract.

An OpenAPI OAuth auth template can already carry provider query parameters on its authorization URL. This change makes the selected template authoritative for optional_scope: any matching declared scopes are moved out of scope, provider defaults are merged, and first-party configuration still overrides the workspace path.

Live verification against the ASI NZ workspace confirmed that scope=oauth plus the app's optional CRM scopes completed consent and produced HTTP 200 account-info, company, contact, and deal reads on the intended production portal. The previous generated request left those CRM permissions in scope and returned HubSpot MISSING_SCOPES.

Test plan

  • bun run --cwd packages/core/sdk test -- src/oauth-helpers.test.ts src/oauth-scope-union.test.ts (95 passed)
  • bun run --cwd apps/cloud test -- src/engine/first-party-oauth-clients.test.ts (6 passed)
  • bun run format
  • bun run lint
  • bun run typecheck (45 packages)

@LloydVickeryASI

LloydVickeryASI commented Aug 31, 2026

Copy link
Copy Markdown
Author

Final CI is green on a637902f: all 40 checks passed, including format, lint, typecheck, unit tests, builds, all 16 cloud E2E shards, all 10 self-hosted E2E shards, and both local E2E shards.

Live ASI NZ verification also confirms the reason for the expanded patch: this workspace app classifies its contact, company, deal, and ticket permissions as optional. With scope=oauth and those values in optional_scope, the personal OAuth connection now returns HTTP 200 for account info, companies, contacts, and deals on production portal 45856450. The saved OpenAPI auth template carries that per-app split; this PR makes generated workspace OAuth URLs honor it without changing the first-party HubSpot registration contract.

@devin-ai-integration

Copy link
Copy Markdown

Verdict: needs a decision from Rhys — the template-declared optional_scope half is sound; the provider-default half is a behavior change for every BYO HubSpot app that I don't think we want. Open ~2 days; 7 commits behind main, applies cleanly.

Ran on the branch (all green, tree clean, no bun.lock churn): bun run lint, bun run format:check, bun run typecheck (45/45), bun run --cwd packages/core/sdk test -- src/oauth-helpers.test.ts src/oauth-scope-union.test.ts (95 passed), bun run --cwd apps/cloud test -- src/engine/first-party-oauth-clients.test.ts (6 passed), bun run --filter @executor-js/sdk test (863 passed). Not exercised against the emulators — there is no HubSpot emulator and the new test already asserts the wire URL against a local AS.

Blocking issue — packages/core/sdk/src/oauth-service.ts L1731-1736 / oauth-helpers.ts L275-277:
workspaceOptionalScopes merges HUBSPOT_OPTIONAL_SCOPES (content, crm.objects.custom.read, crm.schemas.custom.read — the first-party app's optional split) into every workspace-owned client on app.hubspot.com. Two consequences:

  1. If a BYO app has any of those three registered as required and declares them in its template, this PR now strips them from scope → HubSpot rejects the authorize request (per HubSpot docs, required scopes must appear in scope). That's exactly the "hard-coding one global optional set breaks apps with another registration contract" argument the PR description itself makes.
  2. HubSpot's token response carries no scope, so recordedOAuthScope falls back to requestedScopes (oauth-service.ts L331), which now = completeRequestedScopes. Every workspace HubSpot connection will record those three scopes as granted even when the app never registered them. Before this PR workspace connections recorded only their declared scopes.

Suggested fix, keeping the design: make the template authoritative and drop the provider default for workspace clients —

const workspaceOptionalScopes = firstPartyFlow
  ? []
  : dedupeScopes(scopePolicy.kind === "scopes" ? (scopePolicy.optionalScopes ?? []) : []);

and revert the app.hubspot.com branch in providerAuthorizeExtras + the HUBSPOT_OPTIONAL_SCOPES move to host-internal (first-party already sends it via authorizationExtraParams). The "provider defaults" regression test in oauth-scope-union.test.ts would go with it. If Rhys does want a host-level HubSpot default, it should at minimum be dropped when the integration template declares its own optional_scope.

Non-blocking: providerAuthorizeExtras only matches app.hubspot.com, not app-eu1.hubspot.com; moot if the default is removed.

Nothing pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant