feat(plugins): add managed authType to Corsair-managed oauth2 providers - #571
feat(plugins): add managed authType to Corsair-managed oauth2 providers#571yuvrxj-afk wants to merge 2 commits into
Conversation
|
@yuvrxj-afk is attempting to deploy a commit to the corsair Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughManaged authentication is added to thirteen plugins. Each plugin declares managed account fields, accepts the ChangesManaged authentication
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant PluginKeyBuilder
participant CorsairHub
participant ManagedRefreshAuth
PluginKeyBuilder->>CorsairHub: retrieve managed access token
CorsairHub-->>PluginKeyBuilder: return access token
PluginKeyBuilder->>ManagedRefreshAuth: attach refresh behavior
ManagedRefreshAuth-->>PluginKeyBuilder: return authenticated key
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryAdds managed OAuth authentication to thirteen provider plugins.
Confidence Score: 4/5This PR is not yet safe to merge because managed GitLab credentials can still be sent to a noncanonical port and the previously reported managed-webhook paths remain broken. The GitLab hostname-only check accepts and preserves arbitrary ports in authenticated request URLs, while the OneDrive, Outlook, Teams, Dropbox, SharePoint, and Zoho Mail key builders continue to reject managed webhook deliveries without separately supplied signing configuration. Files Needing Attention: packages/gitlab/client.ts, packages/onedrive/index.ts, packages/outlook/index.ts, packages/teams/index.ts, packages/dropbox/index.ts, packages/sharepoint/index.ts, packages/zohomail/index.ts
|
| Filename | Overview |
|---|---|
| packages/gitlab/client.ts | Adds hostname normalization for managed GitLab authentication, but accepts non-default ports that are preserved in authenticated requests. |
| packages/gitlab/index.ts | Adds managed authentication and a GitLab.com guard; its safety depends on the incomplete helper validation. |
| packages/onedrive/index.ts | Adds managed authentication while retaining unconditional rejection of managed webhook verification without an explicit plugin secret. |
| packages/dropbox/index.ts | Adds managed authentication while retaining managed webhook rejection without separately configured signing data. |
| packages/sharepoint/index.ts | Adds managed authentication, but managed webhook processing without explicit client state still falls through to an authentication error. |
| packages/zohomail/index.ts | Adds managed authentication while retaining managed webhook rejection without an explicit signing secret. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Plugin configured with managed auth] --> B[Key builder]
B --> C[Fetch tenant token from Hub]
C --> D[Attach managed refresh handler]
D --> E[Call provider endpoint]
B --> F{Webhook request?}
F -->|Yes| G[Use explicit signing option or reject]
B --> H{GitLab host accepted?}
H -->|Hostname is gitlab.com| I[Send token to configured GitLab URL]
Reviews (3): Last reviewed commit: "feat(plugins): add managed authType to C..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (ctx.authType !== 'oauth_2') { | ||
| throw new AuthMissingError('dropbox', 'oauth_2'); | ||
| } |
There was a problem hiding this comment.
When a Dropbox, SharePoint, or Zoho Mail webhook is processed for a managed tenant without separately configured signing data, the key builder rejects managed mode or falls through to an OAuth auth-missing error, causing the incoming event to be dropped instead of verified and delivered.
Plugin PR scorecard —
|
| Check | Status | Notes |
|---|---|---|
| R1 — Scope: one plugin per PR | ❌ | This PR touches: asana, dropbox, gitlab, hubspot, linear, notion, onedrive, outlook, sharepoint, slack, spotify, teams, zohomail |
| R2 — Tests with assertions | ✅ | |
| R3 — Description | ❌ | Description section is empty or placeholder |
| R3 — Linked issue / claim | No "Fixes #…" or claim link — add one if this PR has a claim or issue | |
| R4 — Demo video / recording | ❌ | Required in "Screenshots / Demos" before a maintainer reviews |
Rules: PLUGIN_PR_RULES.md · re-runs on every push
|
Hey @yuvrxj-afk, thanks for the contribution! 🏴☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push. Must fix
How this was verified: The guard ignores the parsed port while the API URL retains the configured port for authenticated requests. PR requirements (rules)
If anything remains after your next push, a bot commit will clean it up; a maintainer always does the final review and merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/asana/index.ts (1)
1104-1121: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the duplicated managed-auth branch into a Hub helper. The identical eighteen-line block appears in all seven files and, per the PR description, in thirteen plugins. Only the plugin id changes. A single exported helper in
corsair/hub, for exampleresolveManagedAuth(ctx, 'asana'), keeps the Hub-missing message, the context shape, and the refresh attachment in one place. This also keeps each plugin self-contained, because the shared logic stays inpackages/corsair.
packages/asana/index.ts#L1104-L1121: replace the block withreturn (await resolveManagedAuth(ctx, 'asana')).accessToken;.packages/dropbox/index.ts#L385-L402: replace the block with the helper call using the plugin id'dropbox'.packages/gitlab/index.ts#L922-L939: replace the block with the helper call using the plugin id'gitlab'.packages/hubspot/index.ts#L689-L706: replace the block with the helper call using the plugin id'hubspot'.packages/spotify/index.ts#L613-L630: replace the block with the helper call using the plugin id'spotify'.packages/teams/index.ts#L526-L543: replace the block with the helper call using the plugin id'teams'.packages/zohomail/index.ts#L439-L456: replace the block with the helper call using the plugin id'zohomail'.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/asana/index.ts` around lines 1104 - 1121, The managed-auth branch is duplicated across seven plugin files; extract it into one exported corsair/hub helper, such as resolveManagedAuth, preserving Hub validation, managed context construction, token resolution, and refresh attachment. Update packages/asana/index.ts lines 1104-1121, packages/dropbox/index.ts lines 385-402, packages/gitlab/index.ts lines 922-939, packages/hubspot/index.ts lines 689-706, packages/spotify/index.ts lines 613-630, packages/teams/index.ts lines 526-543, and packages/zohomail/index.ts lines 439-456 to call the helper with their respective plugin ids and return its access token.Source: Coding guidelines
packages/linear/index.ts (1)
525-543: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated managed-auth boilerplate into a shared
corsair/hubhelper. Each plugin repeats the same ~15-line block — checkctx.hub, buildmanagedContext, callgetManagedAccessToken, callattachManagedRefreshAuth, return the token — differing only by theplugin:literal and the return-value formatting (Bearer prefix or plain token).
packages/linear/index.ts#L525-L543: replace the block with a shared helper call, e.g.resolveManagedAuth(ctx, 'linear'), keeping theBearer ${token}formatting local to this plugin.packages/notion/index.ts#L404-L422: replace the block with the same shared helper call using'notion'.packages/onedrive/index.ts#L917-L935: replace the block with the same shared helper call using'onedrive'.packages/outlook/index.ts#L684-L702: replace the block with the same shared helper call using'outlook'.packages/sharepoint/index.ts#L1459-L1481: replace the block with the same shared helper call using'sharepoint', keeping the existingsource === 'endpoint'guard around the call.packages/slack/index.ts#L796-L814: replace the block with the same shared helper call using'slack'.Centralizing this in
packages/corsair/hubis consistent with the plugin self-containment rule, since that path is excluded from the per-plugin package requirement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/linear/index.ts` around lines 525 - 543, Extract the repeated managed-auth flow into a shared resolveManagedAuth helper under packages/corsair/hub, centralizing hub validation, managedContext construction, token retrieval, and refresh-auth attachment. Update packages/linear/index.ts (525-543), packages/notion/index.ts (404-422), packages/onedrive/index.ts (917-935), packages/outlook/index.ts (684-702), packages/sharepoint/index.ts (1459-1481), and packages/slack/index.ts (796-814) to call it with their respective plugin names; preserve Linear’s Bearer formatting, each plugin’s token formatting, and SharePoint’s source === 'endpoint' guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gitlab/index.ts`:
- Around line 922-939: Update the GitLab webhook secret retrieval path to
explicitly reject managed authentication when no stored signature is available,
rather than returning an empty string. Use the existing auth-type/context
symbols near the webhook handler and preserve normal secret handling for
non-managed tenants; ensure verification fails closed consistently with the
other managed integrations.
In `@packages/spotify/index.ts`:
- Line 57: Inspect the Spotify plugin definition and Hub auth-resolution path to
confirm whether managed account fields require an exported auth config. If
required, add a spotifyAuthConfig with a managed entry mirroring Spotify’s OAuth
account fields and assign it to the plugin object’s authConfig; otherwise
preserve the current declaration and ensure managed resolution works without it.
In `@packages/teams/index.ts`:
- Around line 475-479: Update the managed Teams onboarding flow associated with
teamsSubscribe to require and set an explicit options.clientState before webhook
key requests reach the authType === 'managed' validation. Document that managed
Teams tenants must configure clientState, while preserving the existing error
for any managed configuration that remains unset.
---
Nitpick comments:
In `@packages/asana/index.ts`:
- Around line 1104-1121: The managed-auth branch is duplicated across seven
plugin files; extract it into one exported corsair/hub helper, such as
resolveManagedAuth, preserving Hub validation, managed context construction,
token resolution, and refresh attachment. Update packages/asana/index.ts lines
1104-1121, packages/dropbox/index.ts lines 385-402, packages/gitlab/index.ts
lines 922-939, packages/hubspot/index.ts lines 689-706,
packages/spotify/index.ts lines 613-630, packages/teams/index.ts lines 526-543,
and packages/zohomail/index.ts lines 439-456 to call the helper with their
respective plugin ids and return its access token.
In `@packages/linear/index.ts`:
- Around line 525-543: Extract the repeated managed-auth flow into a shared
resolveManagedAuth helper under packages/corsair/hub, centralizing hub
validation, managedContext construction, token retrieval, and refresh-auth
attachment. Update packages/linear/index.ts (525-543), packages/notion/index.ts
(404-422), packages/onedrive/index.ts (917-935), packages/outlook/index.ts
(684-702), packages/sharepoint/index.ts (1459-1481), and packages/slack/index.ts
(796-814) to call it with their respective plugin names; preserve Linear’s
Bearer formatting, each plugin’s token formatting, and SharePoint’s source ===
'endpoint' guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 498cc5e5-b2e4-4414-9b0d-fddb4f5bd4b4
📒 Files selected for processing (13)
packages/asana/index.tspackages/dropbox/index.tspackages/gitlab/index.tspackages/hubspot/index.tspackages/linear/index.tspackages/notion/index.tspackages/onedrive/index.tspackages/outlook/index.tspackages/sharepoint/index.tspackages/slack/index.tspackages/spotify/index.tspackages/teams/index.tspackages/zohomail/index.ts
| if (ctx.authType === 'managed') { | ||
| throw new Error( | ||
| '[auth-missing:teams:managed]: webhook signature is not available in managed mode', | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Managed tenants now require an explicit clientState.
The plugin registers subscribe: teamsSubscribe, and Microsoft Graph notifications carry a clientState value. With authType: 'managed' and no options.clientState, every webhook key request throws. Confirm that the managed onboarding path sets clientState, and document this requirement for managed Teams tenants.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/teams/index.ts` around lines 475 - 479, Update the managed Teams
onboarding flow associated with teamsSubscribe to require and set an explicit
options.clientState before webhook key requests reach the authType === 'managed'
validation. Document that managed Teams tenants must configure clientState,
while preserving the existing error for any managed configuration that remains
unset.
0b6c9dd to
064f1d9
Compare
064f1d9 to
c6f41e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/gitlab/client.test.ts (1)
5-26: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd negative cases for non-HTTPS schemes.
URL.hostnameis independent of the URL protocol, so hostname equality alone acceptshttp://gitlab.comandftp://gitlab.com. (url.spec.whatwg.org) Add these inputs to the rejected list after the implementation requireshttps:. This prevents regressions in managed-token transport validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/gitlab/client.test.ts` around lines 5 - 26, Update the managed GitLab host validation used by isManagedGitlabHost to require the URL protocol to be https: in addition to the existing hostname checks, then add http://gitlab.com and ftp://gitlab.com to the rejected test inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gitlab/client.ts`:
- Around line 23-31: Update isManagedGitlabHost in packages/gitlab/client.ts to
require both hostname gitlab.com and url.protocol === 'https:'; continue
returning false for invalid URLs. In packages/gitlab/client.test.ts lines 5-26,
add coverage asserting http://gitlab.com and ftp://gitlab.com are rejected while
the HTTPS host remains accepted.
---
Nitpick comments:
In `@packages/gitlab/client.test.ts`:
- Around line 5-26: Update the managed GitLab host validation used by
isManagedGitlabHost to require the URL protocol to be https: in addition to the
existing hostname checks, then add http://gitlab.com and ftp://gitlab.com to the
rejected test inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a20a4d5-fefe-4d3d-8e3a-31e222506fd7
📒 Files selected for processing (28)
packages/asana/index.tspackages/asana/jest.config.cjspackages/dropbox/index.tspackages/dropbox/jest.config.cjspackages/gitlab/client.test.tspackages/gitlab/client.tspackages/gitlab/index.tspackages/gitlab/jest.config.cjspackages/hubspot/index.tspackages/hubspot/jest.config.cjspackages/linear/index.tspackages/linear/jest.config.cjspackages/notion/index.tspackages/notion/jest.config.cjspackages/onedrive/index.tspackages/onedrive/jest.config.cjspackages/outlook/index.tspackages/outlook/jest.config.cjspackages/sharepoint/index.tspackages/sharepoint/jest.config.cjspackages/slack/index.tspackages/slack/jest.config.cjspackages/spotify/index.tspackages/spotify/jest.config.cjspackages/teams/index.tspackages/teams/jest.config.cjspackages/zohomail/index.tspackages/zohomail/jest.config.cjs
🚧 Files skipped from review as they are similar to previous changes (26)
- packages/gitlab/jest.config.cjs
- packages/hubspot/jest.config.cjs
- packages/teams/jest.config.cjs
- packages/dropbox/jest.config.cjs
- packages/linear/jest.config.cjs
- packages/notion/jest.config.cjs
- packages/sharepoint/jest.config.cjs
- packages/outlook/jest.config.cjs
- packages/slack/jest.config.cjs
- packages/asana/jest.config.cjs
- packages/zohomail/jest.config.cjs
- packages/onedrive/index.ts
- packages/gitlab/index.ts
- packages/outlook/index.ts
- packages/asana/index.ts
- packages/spotify/jest.config.cjs
- packages/spotify/index.ts
- packages/notion/index.ts
- packages/dropbox/index.ts
- packages/onedrive/jest.config.cjs
- packages/hubspot/index.ts
- packages/linear/index.ts
- packages/zohomail/index.ts
- packages/teams/index.ts
- packages/slack/index.ts
- packages/sharepoint/index.ts
| new URL(baseUrl).hostname.replace(/\.$/, '').toLowerCase() === | ||
| 'gitlab.com' | ||
| ); | ||
| } catch { |
There was a problem hiding this comment.
When managed authentication uses a non-default port such as https://gitlab.com:8443, this hostname-only check accepts the URL and the request builder preserves port 8443, causing the managed bearer token to be sent to a noncanonical service where authentication fails or the credential is disclosed.
How this was verified: The guard ignores the parsed port while the API URL retains the configured port for authenticated requests.
isManagedGitlabHost compared only the hostname, so https://gitlab.com:8443 passed and Corsair's managed bearer token (valid only for canonical gitlab.com) would be sent to a noncanonical service. Require https on the default port, so a custom port or scheme is treated as a custom host and rejected.
2 participants
Declares the
managedauthType on the 13 plugins Corsair has registered a managed OAuth app for, so tenant end-users can connect them without bringing their own OAuth credentials.Each plugin gains
'managed'in its authType union, amanaged:authConfig block mirroring itsoauth_2account fields, and a keyBuilder branch that resolves the tenant token from the Hub.Scoped on purpose:
managedonly enters the union of registered providers, so the SDK never offers it where Corsair doesn't own the OAuth app —authType: 'managed'on an unregistered plugin is a compile error.Plugins: asana, dropbox, gitlab, hubspot, linear, notion, onedrive, outlook, sharepoint, slack, spotify, teams, zohomail.
Excluded: airtable + jira (api_key-only, need oauth2 support first); google + stripe (not published yet).
Summary by CodeRabbit