Azure DevOps #18038
Azure DevOps #18038
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
WalkthroughIntroduces dual-token authorization in the Azure DevOps app: requests can use OAuth or a Personal Access Token (PAT). Adds _personalAccessToken(), changes _headers(useOAuth) and _makeRequest(args) to accept useOAuth, and updates listAccounts to request OAuth. Bumps package version and dependency; minor source formatting change. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client as Pipedream Component
participant App as AzureDevOpsApp
participant Req as _makeRequest({ useOAuth })
participant H as _headers(useOAuth)
participant ADO as Azure DevOps API
User->>Client: Trigger action
Client->>App: call API method(args)
App->>Req: _makeRequest({ useOAuth })
Req->>H: _headers(useOAuth)
alt useOAuth = true
H->>App: _oauthAccessToken()
else useOAuth = false
H->>App: _personalAccessToken()
end
H-->>Req: Authorization header
Req->>ADO: HTTP request with header
ADO-->>Req: Response
Req-->>App: Data
App-->>Client: Result
sequenceDiagram
autonumber
participant App as AzureDevOpsApp
participant Req as _makeRequest({ useOAuth:true })
participant H as _headers(true)
participant OAuth as _oauthAccessToken()
participant ADO as Azure DevOps Accounts API
App->>Req: listAccounts({ useOAuth:true })
Req->>H: build headers (useOAuth:true)
H->>OAuth: fetch OAuth access token
OAuth-->>H: token
H-->>Req: Authorization: Bearer <token>
Req->>ADO: GET /accounts
ADO-->>Req: Accounts list
Req-->>App: Data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@vunguyenhung The docs here (https://learn.microsoft.com/en-us/rest/api/azure/devops/hooks/publishers/list-event-types?view=azure-devops-rest-5.0&tabs=HTTP) say to use a "personal access token". I tested and created this PR which is a work in progress, but shows that using a personal access token does work to get event types, create webhooks, and delete webhooks. Strangely, the endpoint to get organizations still seems to need the oauth_access_token. The first link I pasted above recommends migrating to Microsoft Entra ID OAuth. I think we should either do that or add a personal_access_token field to the $auth for Azure Devops. |
|
Hi @michelle0927, thank you for your information. Here's the next steps:
3. In the future, we can shift the Azure DevOps app to use Personal Access Token only, so that user will have 2 options for authentication
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
components/azure_devops/azure_devops.app.mjs (4)
31-37: Use Bearer for OAuth and Basic for PAT (align with Azure DevOps auth model)Azure DevOps REST APIs expect:
- OAuth: Authorization: Bearer <access_token>
- PAT: Authorization: Basic base64(:) (username can be empty)
Currently both paths use Basic. Recommend switching to Bearer for OAuth and using the empty-username pattern for PAT. Also add explicit checks to fail fast with helpful errors when tokens are missing.
Apply:
- _headers(useOAuth) { - const basicAuth = Buffer.from(`${this._oauthUid()}:${useOAuth - ? this._oauthAccessToken() - : this._personalAccessToken()}`).toString("base64"); - return { - Authorization: `Basic ${basicAuth}`, - }; - }, + _headers(useOAuth) { + if (useOAuth) { + const token = this._oauthAccessToken(); + if (!token) { + throw new Error("Azure DevOps OAuth access token is missing. Please reconnect your Azure DevOps account."); + } + return { Authorization: `Bearer ${token}` }; + } + const pat = this._personalAccessToken(); + if (!pat) { + throw new Error("Azure DevOps Personal Access Token is required for this operation. Add it to your Azure DevOps connection."); + } + // Basic auth with PAT: username can be blank + const basicAuth = Buffer.from(`:${pat}`).toString("base64"); + return { Authorization: `Basic ${basicAuth}` }; + },Optionally, confirm listAccounts still succeeds with Bearer headers against vssps.
53-59: Prevent accidental header override; merge user headers with auth headerSpreading otherArgs after headers allows a caller to unintentionally drop Authorization. Merge instead and still allow explicit override.
- const config = { - url: url || `${this._baseUrl()}${path}`, - headers: this._headers(useOAuth), - ...otherArgs, - }; + const { + headers: userHeaders, + ...restArgs + } = otherArgs; + const config = { + url: url || `${this._baseUrl()}${path}`, + headers: { + ...this._headers(useOAuth), + ...(userHeaders || {}), + }, + ...restArgs, + };
69-72: Validate OAuth presence for listAccounts and provide actionable errorIf users haven’t connected OAuth (but added only a PAT), this will fail in a non-obvious way. Add a guard with a clear message.
- const { value } = await this._makeRequest({ + if (!this._oauthUid() || !this._oauthAccessToken()) { + throw new Error("Listing organizations requires OAuth. Connect Azure DevOps via Microsoft Entra ID OAuth in the app settings."); + } + const { value } = await this._makeRequest({ url: `https://app.vssps.visualstudio.com/_apis/accounts?memberId=${this._oauthUid()}`, useOAuth: true, ...args, });Follow-up: Consider documenting which operations require PAT vs OAuth in the app description to reduce support pings.
45-47: Expose PAT accessor is fine; ensure friendly failure at call sitesGetter looks good. The more important part is surfacing a clear error if PAT is missing when required (addressed in the _headers() suggestion). No change needed here if you adopt that guard.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
components/azure_devops/azure_devops.app.mjs(3 hunks)components/azure_devops/package.json(2 hunks)components/azure_devops/sources/new-event/new-event.mjs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/azure_devops/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (4)
components/azure_devops/package.json (2)
3-3: Version bump aligns with auth behavior changeMinor bump to 0.1.0 makes sense given the auth semantics change (PAT support + flagged OAuth usage). No issues.
16-16: Verify major upgrade of @pipedream/platformYou’ve bumped
components/azure_devops/package.jsonfrom ^1.2.1 to ^3.1.0—a major release with potential breaking changes. Please confirm:
- All imports of
@pipedream/platformstill resolve correctly.- Direct calls to axios (via the platform or standalone) behave as expected.
- Any use of platform helpers (
$http,$step,$steps, etc.) hasn’t been deprecated or changed.- Your existing tests and any manual smoke tests around HTTP or step executions pass under v3.x.
components/azure_devops/sources/new-event/new-event.mjs (2)
5-5: Version bump acknowledgedSource version bump to 0.0.3 is appropriate; no functional deltas here.
43-48: No behavioral change; formatting is fineMulti-line invocation reads cleaner and doesn’t alter behavior. Assuming createSubscription uses PAT by default (per app changes), this should resolve the “event type” options issue end-to-end.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/azure_devops/azure_devops.app.mjs (1)
23-26: Bug: Mismatched prop name breaks event type options (organization vs organizationName)The eventType options function expects { organization }, but the prop is defined as organizationName. This passes undefined to listEventTypes and can produce the “Bad option response for prop type eventType”.
- async options({ organization }) { - const types = await this.listEventTypes(organization); + async options({ organizationName }) { + const types = await this.listEventTypes(organizationName); return types?.map((type) => type.id); },
🧹 Nitpick comments (1)
components/azure_devops/azure_devops.app.mjs (1)
89-94: Nit: Extra space afterreturnMinor style nit to remove the double-space.
- return this._makeRequest({ + return this._makeRequest({ method: "POST", path: `/${organization}/_apis/hooks/subscriptions`, ...args, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/azure_devops/azure_devops.app.mjs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (3)
components/azure_devops/azure_devops.app.mjs (3)
1-3: LGTM: Importing ConfigurationError is appropriate hereGood call importing ConfigurationError alongside axios for clear, actionable config errors.
51-53: LGTM: Accessor for PATSimple, clear method to access the PAT from $auth.
59-66: LGTM: Plumbs useOAuth through _makeRequestPassing the useOAuth flag into _headers centralizes auth behavior per endpoint neatly.
Resolves #17986
Summary by CodeRabbit