feat(oauth): publish integration scopes in docs, and fix the Excel SharePoint scopes - #7440
feat(oauth): publish integration scopes in docs, and fix the Excel SharePoint scopes#7440mzxchandra wants to merge 14 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThe PR extracts OAuth scope metadata into a dependency-free catalog, publishes generated scope documentation, and adds the Microsoft Graph permissions Excel needs for SharePoint workbooks.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/oauth/scopes.ts | Introduces the canonical readonly OAuth scope catalog and description metadata, including the additional Excel SharePoint permissions. |
| apps/sim/lib/oauth/oauth.ts | Replaces inline scope arrays with absolute imports from the extracted catalog; the previously reported relative import is fixed. |
| scripts/generate-docs.ts | Generates integration scope tables and the self-hosting OAuth reference from canonical scope configuration. |
| apps/sim/lib/oauth/utils.ts | Moves scope-description data to the shared catalog and returns copied canonical scope arrays to callers. |
| apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx | Updates the OAuth connection UI to consume the centralized scope metadata. |
| apps/docs/content/docs/platform/self-hosting/integrations-oauth.mdx | Replaces manually maintained OAuth registration guidance with generated per-connector scope and provider references. |
Reviews (12): Last reviewed commit: "fix(docs): group the app reference by cl..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
e4b7842 to
cd9794c
Compare
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
cd9794c to
b2c817e
Compare
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
7d41b26 to
69e59f6
Compare
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 62 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
|
@cubic-dev-ai review this PR |
The Excel scope fix was incomplete. `connectors/microsoft-excel/meta.ts`
declares its own `requiredScopes: ['Files.ReadWrite']`, and the knowledge
base's connector card computes missing scopes from that list rather than
from the canonical set, so a connector credential predating this PR was
never flagged. That connector takes a `driveId` for a SharePoint document
library and builds `/drives/{id}/items/...` from it, which the basic grant
cannot reach, so it would have failed at sync time instead of prompting a
reconnect. It now lists `Files.ReadWrite.All` as well. `Sites.Read.All` is
not added: the connector takes the drive id as text and never lists sites.
The Dataverse caveat named the wrong audience. The host is normalized to
its `.api` form before the request, so `https://contoso.crm.dynamics.com`
is asked for as `https://contoso.api.crm.dynamics.com/.default`. The note
now says that, with the example.
The self-hosting reference listed one provider id per connector, but
Salesforce authenticates through a second authorization server for
sandboxes under `salesforce-sandbox`, and each provider id is its own
redirect URI. A self-hoster following the table registered half of what
they needed and the sandbox connect flow failed redirect-URI validation.
The table now lists every provider id a service authenticates through, and
generation refuses rather than silently dropping one it cannot read.
… names The failure collection added last round sat outside the config loop, so a throw on one config skipped every later config in the same file. No file declares more than one today, but `extractAllBlockConfigs` exists precisely for the v1-beside-v2 case, and a latent skip is one integration away from being real. The try now wraps each config, and the run still exits nonzero with the failure named. Verified by pointing a block at a nonexistent service: exit 1, block and config named. `read` and `write` are not Linear's alone. Trello issues both and Reddit issues `read`, so three published tables carried a label written for a fourth meaning: "Read access to connected account data" tells a Trello user nothing about the boards they are approving. Both now resolve through `PROVIDER_SCOPE_DESCRIPTIONS`, which exists for exactly this and already disambiguates `account` between Bitbucket and Reddit. Linear keeps the generic wording as the fallback.
abb839b to
34003e6
Compare
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 64 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 64 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
The generator imported `SALESFORCE_ADDITIONAL_PROVIDER_IDS` straight from the Salesforce module and matched that constant's identifier name in a regex against `oauth.ts`. That put one provider's name inside general scraper logic and gave the generator a second OAuth data source, so the next provider to run two authorization servers would have needed its own import and its own regex branch. The data itself is right where it belongs: `SALESFORCE_AUTH_SERVERS` is already the single source for Salesforce's connector registrations, refresh endpoints, `additionalProviderIds` and environment-picker labels, so moving one of those four derivations into the scope module would only detach it from the other three. So `scopes.ts` re-exports it instead, as `ADDITIONAL_PROVIDER_IDS` keyed by service id, and stays the one module a build script reads OAuth data from. `salesforce.ts` has no imports of its own, so the dependency-free graph that lets the generator load it at all is preserved. The scraper now only records that a service claims extras, and the values come from the map. A parity check compares the two in both directions, so a service that grows a second authorization server without the map following fails generation instead of publishing a redirect URI list that is missing one. Verified both directions: an entry in `oauth.ts` alone and an entry in `scopes.ts` alone each fail with the offending service named.
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 64 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Re-trigger cubic
I pushed back on this finding two rounds ago and I was wrong. I checked `lib/auth/connectors/providers.ts` and the better-auth bundle, found nothing appending `openid`, and said so. I never looked for a managed-OAuth path. There is one. `managed-oauth.ts:495` unions `getCanonicalScopesForProvider(providerId)` with the connector's own `additionalScopes`, and the Google managed connector sets `additionalScopes: ['openid']`. So on eight Google connectors a managed connection requests one scope more than the declared set, and the table printed from that set alone understated it. `MANAGED_OAUTH_ADDITIONAL_SCOPES` now carries it, keyed by provider id, which is what the connector factory matches on and what differs from the service id for Gmail. Both the integration pages and the self-hosting reference name it, deduplicated on the reference so an app-wide note reads once rather than once per connector. The eight provider ids existed only inside an eight-way `||` chain in the factory. They are now a set in `scopes.ts` that the factory matches against, matching the `MICROSOFT_MANAGED_OAUTH_PROVIDER_IDS.has(...)` line directly below it, so a Google service added to one is added to both.
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 65 files
Confidence score: 3/5
- In
apps/sim/lib/oauth/scopes.ts, the sharedmicrosoft-excelscope set requestsSites.Read.Alleven though the connector uses a directdriveIdand does not enumerate SharePoint sites, unnecessarily broadening OAuth consent and potentially blocking or discouraging authorization — remove the unused site-list scope from the Excel connector.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/oauth/scopes.ts">
<violation number="1" location="apps/sim/lib/oauth/scopes.ts:177">
P2: The shared `microsoft-excel` scope set makes the Excel connector request `Sites.Read.All` even though that connector accepts a direct `driveId` and never enumerates SharePoint sites. This adds an unnecessary site-listing permission to every connector consent flow and can cause consent failures; keep `Sites.Read.All` for the block surface that uses the SharePoint site picker, but omit it from the connector's authorization scope set.
(Based on your team's feedback about Excel scopes by surface.)</violation>
</file>
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Re-trigger cubic
| 'Files.ReadWrite', | ||
| 'Files.Read.All', | ||
| 'Files.ReadWrite.All', | ||
| 'Sites.Read.All', |
There was a problem hiding this comment.
P2: The shared microsoft-excel scope set makes the Excel connector request Sites.Read.All even though that connector accepts a direct driveId and never enumerates SharePoint sites. This adds an unnecessary site-listing permission to every connector consent flow and can cause consent failures; keep Sites.Read.All for the block surface that uses the SharePoint site picker, but omit it from the connector's authorization scope set.
(Based on your team's feedback about Excel scopes by surface.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/oauth/scopes.ts, line 177:
<comment>The shared `microsoft-excel` scope set makes the Excel connector request `Sites.Read.All` even though that connector accepts a direct `driveId` and never enumerates SharePoint sites. This adds an unnecessary site-listing permission to every connector consent flow and can cause consent failures; keep `Sites.Read.All` for the block surface that uses the SharePoint site picker, but omit it from the connector's authorization scope set.
(Based on your team's feedback about Excel scopes by surface.) </comment>
<file context>
@@ -0,0 +1,1252 @@
+ 'Files.ReadWrite',
+ 'Files.Read.All',
+ 'Files.ReadWrite.All',
+ 'Sites.Read.All',
+ 'offline_access',
+ ],
</file context>
There was a problem hiding this comment.
This is the third round for this one and my answer has not changed, so rather than restate it I am flagging it for a human decision.
The mechanism, from the two previous threads: the block and the connector share one microsoft-excel OAuth service and therefore one credential. The authorization request is registered once (lib/auth/connectors/providers.ts:784) and the connector's own connect modal passes that same canonical set (connectors-section.tsx:721). A connector-specific request would not mint a narrower token, it would mint two kinds of credential under one provider id, and the block's site picker would then 403 on a credential it believes it can use.
Sites.Read.All is required because the block ships a sharepoint.sites selector calling GET /sites?search=* (lib/selectors/server/providers/sharepoint.ts:170), a tenant-wide site search. The narrowing you are asking for is available, but only by dropping that picker, which is a product decision rather than a docs one.
Where the distinction can be expressed I have already applied it: connectors/microsoft-excel/meta.ts declares requiredScopes without Sites.Read.All, so a connector user is never prompted to reauthorize for a permission that surface does not use.
Leaving this open for a maintainer to settle rather than closing it as answered.
One OAuth app is one client id, and more than one connector can register against it. `manageengine-sdp` reads `ZOHO_CLIENT_ID` (`lib/auth/connectors/providers.ts:1267`) while being its own provider entry, so grouping by provider left it off the Zoho section entirely: a self-hoster following that page registered one redirect URI, missed `/api/auth/oauth2/callback/manageengine-sdp`, and that connector's connect flow then failed redirect-URI validation. Same class as the Salesforce sandbox URI, from the same wrong assumption that a capability id maps to exactly one provider. Connectors sharing an app's client id are now read from the registrations themselves, which is where the runtime decides which credentials a connector uses, and attached to that app. Additive rather than a replacement: Instagram, Salesforce, Shopify and Trello run custom flows that declare no `clientId: env.X` line, so a pure registration scrape would have dropped four apps off the page. Also generalized the admin-consent warning, which named only `microsoft-ad` while Teams and Planner both request `Group.ReadWrite.All`. It now points at Microsoft's reference rather than implying every `.All` permission needs an admin, since `Sites.Read.All` does not.
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 65 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
1 participant
Summary
Docs never said what an OAuth connection actually grants. A user hit the consent screen cold, and a self-hoster registering an app had to guess which permissions to add before anyone could connect. This publishes the real scope sets, straight from the config the runtime uses, and fixes an Excel scope gap found while doing it.
Three commits, each independently valid and
docs:check-clean:1.
refactor(oauth)— extract scope data into a dependency-free module.OAUTH_PROVIDERSimports ~120 React icons and readsenvat module load, so nothing outside Next can read the scopes it declares. Moved the 61 scope arrays out ofoauth.ts, and the label tables plusgetScopeDescriptionout ofutils.ts, intolib/oauth/scopes.tswith zero imports. Behavior-preserving: all 61 scope lists verified identical to staging (the one intentional difference is Excel, in commit 3).2.
feat(docs)— publish the scopes. A## Scopestable on 51 integration pages, plus a generated per-app reference on the self-hosting page (env vars, provider IDs, scopes grouped by connector).3.
fix(excel)— request the SharePoint scopes the file picker needs. See below.The Excel bug
The Excel block exposes a SharePoint site picker (
blocks/blocks/microsoft_excel.ts:795) and a drive picker that callsGET /sites/{id}/drives, and builds/drives/{driveId}/items/{id}for arbitrary drives. But the integration only requestedFiles.Read/Files.ReadWrite, which cover the signed-in user's own OneDrive. A workbook in a document library returned insufficient privileges.Added
Files.Read.All,Files.ReadWrite.All,Sites.Read.All. Word already carries the first two for exactly this reason. All are user-consentable, so Excel does not move behind admin consent.Existing connections must reconnect (Microsoft only grants new scopes on fresh authorization). No migration needed: the block's credential selector already derives
requiredScopesfromgetScopesForService('microsoft-excel'), so stale connections show as needing an update on their own.Drift this replaces
The hand-maintained self-hosting tables were already wrong:
microsoft-word. Anyone following it registered an app that could not connect Word. (Staging hand-patched this and the FAQ counts while this branch was in flight; the generated block supersedes both.)microsoft-adset needs tenant admin consent.Design notes
microsoft-adneeds.Type of Change
Testing
bun run check:audits— 45/45 pass, includingdocs:checkbun run type-check— cleanlib/oauth, the Microsoft blocks, and the connect modalscripts/generate-docs.test.tsdocs:checkverified green at each of the three commits independentlydocs:check, removing the page markers fails generationNew tests cover the module split (the two files must declare the same services, and every table scope must actually be requested), the generated section (env-flag scopes excluded so output cannot vary by deployment, empty-scope services skipped, unknown service throws), and the shared-scope hoisting rule.
Reviewers should focus on: the scope diff in
scopes.tsfor commit 3 (the only intended behavior change), and whether the per-connector grouping on the self-hosting page is the right call versus a union.Checklist
Screenshots/Videos
Integration page (SharePoint) and the self-hosting reference (Google, showing hoisted shared scopes) render as ordinary doc tables; verified locally at
localhost:3001.