feat: enhance OAuth2 support in snippet generation#6592
feat: enhance OAuth2 support in snippet generation#6592bijin-bruno merged 8 commits intousebruno:mainfrom
Conversation
* Updated getAuthHeaders function to handle OAuth2 authentication, including token retrieval and placement. * Added tests for OAuth2 scenarios, ensuring correct Authorization header generation and handling of edge cases. * Improved error handling for access token retrieval from stored credentials.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughgetAuthHeaders gained optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Snippet Generator
participant Auth as getAuthHeaders
participant Store as Persisted Credentials Store
participant HAR as HAR Builder
rect rgb(240,248,255)
note right of UI: Trigger: generate snippet\n(with collection & item)
UI->>Auth: getAuthHeaders(collectionAuth, requestAuth,\ncollection, item)
end
rect rgb(255,250,240)
Auth->>Store: interpolate auth/accessToken URL\n(using collection+item variables)
alt matching credentials found
Store-->>Auth: credentials (access_token)
else not found / error
Store-->>Auth: none / error
end
Auth-->>Auth: build token string\napply tokenHeaderPrefix (if any)\nrespect tokenPlacement
Auth-->>UI: headers[] (or empty)
end
rect rgb(240,255,245)
UI->>HAR: attach headers (unless tokenPlacement == url)
HAR-->>UI: final HAR request
UI-->>User: generated code snippet
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds OAuth2 support to the code snippet generation feature, enabling generated code snippets to include OAuth2 access tokens in Authorization headers. The implementation retrieves stored OAuth2 credentials from the collection and includes them in the generated snippets.
- Enhanced the
getAuthHeadersfunction to handle OAuth2 authentication with token retrieval from stored credentials - Added comprehensive test coverage for various OAuth2 scenarios including different token placements and header prefixes
- Updated the snippet generator to pass collection and item context to enable credential lookup
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/bruno-app/src/utils/codegenerator/auth.js | Added OAuth2 case handling with token retrieval logic, supporting both header and URL token placement, with fallback to placeholder tokens |
| packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js | Updated getAuthHeaders call to include collection and item parameters for OAuth2 credential lookup |
| packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js | Added comprehensive test suite for OAuth2 authentication scenarios covering header placement, custom prefixes, URL placement, and edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...debar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (1)
590-746: Consider adding a test for OAuth2 auth inheritance.Tests cover direct OAuth2 auth well, but there's no coverage for when
request.auth.mode === 'inherit'with OAuth2 configured at the collection level. This is an edge case worth validating.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/utils/codegenerator/auth.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.jspackages/bruno-app/src/utils/codegenerator/auth.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
🧠 Learnings (5)
📓 Common learnings
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.311Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
📚 Learning: 2025-12-02T07:24:50.311Z
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.311Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.jspackages/bruno-app/src/utils/codegenerator/auth.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.jspackages/bruno-app/src/utils/codegenerator/auth.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
🧬 Code graph analysis (3)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (1)
packages/bruno-app/src/utils/codegenerator/auth.js (2)
getAuthHeaders(6-118)getAuthHeaders(6-118)
packages/bruno-app/src/utils/codegenerator/auth.js (3)
packages/bruno-cli/src/runner/prepare-request.js (1)
get(1-1)packages/bruno-electron/src/ipc/network/prepare-request.js (2)
grantType(75-75)grantType(192-192)packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (1)
variables(12-12)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (3)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (3)
require(10-10)collectionAuth(22-22)generateSnippet(7-50)packages/bruno-app/src/utils/auth/index.js (1)
collectionAuth(25-25)packages/bruno-app/src/utils/codegenerator/auth.js (4)
oauth2Config(55-55)tokenHeaderPrefix(57-57)accessToken(62-62)headerValue(101-103)
🪛 Biome (2.1.2)
packages/bruno-app/src/utils/codegenerator/auth.js
[error] 55-55: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 56-56: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 57-57: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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). (8)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: SSL Tests - Linux
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
🔇 Additional comments (4)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (1)
21-24: LGTM!The updated call to
getAuthHeaderscorrectly passescollectionanditemcontext, enabling OAuth2 credential resolution. Both variables are properly in scope.packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (1)
558-588: Good test coverage for OAuth2 scenarios.The mock implementation correctly mirrors the production logic for OAuth2 header generation. Tests effectively validate the header output through
buildHarRequestmock calls.packages/bruno-app/src/utils/codegenerator/auth.js (2)
2-6: LGTM!New imports are all used in the OAuth2 implementation, and the function signature maintains backward compatibility with null defaults.
66-77: Good implementation of OAuth2 token resolution.The logic correctly differentiates between implicit grant (using
authorizationUrl) and other grant types (usingaccessTokenUrl). URL interpolation ensures variable substitution matches persisted credentials accurately.
* Updated comparison operators in the getAuthHeaders function to use strict equality (===) for improved consistency and reliability in credential checks.
* Added missing block structure for the 'oauth2' case in the getAuthHeaders function to ensure proper execution flow and maintain code clarity.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Updated getAuthHeaders function to support retrieval of stored OAuth2 credentials based on collection and item context. * Improved access token handling by checking for existing credentials before falling back to default values. * Enhanced test coverage for OAuth2 scenarios to ensure accurate token management and error handling.
* Updated snippet-generator.spec.js to ensure that the tokenHeaderPrefix from OAuth2 configuration is preserved, allowing for empty string scenarios. * Default to 'Bearer' only if the tokenHeaderPrefix is undefined, enhancing flexibility in token management.
…andling * Updated getAuthHeaders function to always trim the final result of the authorization header for consistent formatting. * Adjusted snippet-generator.spec.js to reflect the same trimming logic for the access token, enhancing test reliability.
* Updated comments in the getAuthHeaders function to specify that when tokenPlacement is 'url', no auth headers are added, and that token placement in the URL/query params must be managed separately.
…ction * Updated getAuthHeaders function to default to an empty array when accessing oauth2Credentials, preventing potential errors when no credentials are available.
|
@stupidly-logical is attempting to deploy a commit to the Bruno Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (1)
619-650: Consider adding a test for stored OAuth2 credentials lookup.The tests cover direct accessToken from config, but don't exercise the credential lookup path where
collection.oauth2Credentialsis populated and matched by URL/collectionUid/credentialsId. This is a key feature of the implementation.🔎 Example test structure
it('should use stored credentials when available', () => { const collectionWithCreds = { ...baseCollection, uid: 'test-collection-uid', oauth2Credentials: [ { url: 'https://auth.example.com/token', collectionUid: 'test-collection-uid', credentialsId: 'credentials', credentials: { access_token: 'stored-token-789' } } ] }; const item = { uid: 'oauth-req-stored', request: { method: 'GET', url: 'https://api.example.com/users', headers: [], auth: { mode: 'oauth2', oauth2: { grantType: 'client_credentials', accessTokenUrl: 'https://auth.example.com/token', tokenPlacement: 'header', tokenHeaderPrefix: 'Bearer' } } } }; generateSnippet({ language, item, collection: collectionWithCreds, shouldInterpolate: false }); const harUtils = require('utils/codegenerator/har'); const harCall = harUtils.buildHarRequest.mock.calls[0][0]; expect(harCall.headers).toEqual( expect.arrayContaining([ expect.objectContaining({ name: 'Authorization', value: 'Bearer stored-token-789' }) ]) ); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/utils/codegenerator/auth.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/utils/codegenerator/auth.js
🧠 Learnings (5)
📓 Common learnings
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.311Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
📚 Learning: 2025-12-02T07:24:50.311Z
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.311Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/utils/codegenerator/auth.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/utils/codegenerator/auth.js
🧬 Code graph analysis (2)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (2)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (2)
require(10-10)generateSnippet(7-50)packages/bruno-app/src/utils/codegenerator/auth.js (8)
tokenPlacement(56-56)tokenHeaderPrefix(57-57)accessToken(62-62)grantType(66-66)urlToLookup(68-70)credentialsId(71-71)collectionUid(72-72)headerValue(102-106)
packages/bruno-app/src/utils/codegenerator/auth.js (5)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/CodeView/index.js (1)
collection(24-32)packages/bruno-cli/src/runner/prepare-request.js (2)
get(1-1)find(4-4)packages/bruno-electron/src/ipc/network/prepare-request.js (2)
grantType(75-75)grantType(192-192)packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (1)
variables(12-12)packages/bruno-js/src/sandbox/quickjs/shims/bru.js (1)
interpolate(32-34)
⏰ 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). (6)
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (5)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (1)
562-617: Mock implementation is comprehensive and well-structured.The mock correctly handles the full getAuthHeaders signature
(collectionRootAuth, requestAuth, collection, item)and mirrors the actual OAuth2 logic including credential lookup, tokenPlacement handling, and prefix formatting.One observation: the mock duplicates significant logic from the actual implementation. While this ensures the test validates integration behavior, it also means changes to
auth.jsrequire corresponding mock updates. Consider whether a partial mock or spy approach might reduce maintenance burden for future changes.packages/bruno-app/src/utils/codegenerator/auth.js (4)
1-4: Imports look good.New imports for
find,interpolate, andgetAllVariablesare correctly added to support the OAuth2 credential lookup functionality.
6-6: Signature extension maintains backward compatibility.Default values for
collectionanditemensure existing callers continue to work without modification.
54-119: OAuth2 implementation is well-structured.Block scope, strict equality, consistent trimming, and safe fallback for
oauth2Credentialsare all properly implemented. The try-catch with console.error provides graceful degradation when credential lookup fails.One minor note: the
tokenHeaderPrefixdefault handling withget(oauth2Config, 'tokenHeaderPrefix', 'Bearer')correctly preserves an explicit empty string''since lodashgetonly applies the default when the value isundefined, not when it's falsy.
66-71: The grant type URL selection logic is correct and properly covers all OAuth2 flows. The schema defines four grant types:client_credentials,password,authorization_code, andimplicit. The code correctly usesauthorizationUrlfor implicit (which has no separate token endpoint) andaccessTokenUrlfor the other three types (all requiring token exchange). This aligns with OAuth2 specification and the schema validation rules.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...debar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
Show resolved
Hide resolved
| import { find } from 'lodash'; | ||
| import { interpolate } from '@usebruno/common'; | ||
| import { getAllVariables } from 'utils/collections/index'; | ||
|
|
There was a problem hiding this comment.
The getAuthHeaders function signature has been extended with two new optional parameters (collection and item), but there's no JSDoc or inline documentation explaining when these parameters are needed and what their purpose is. Since other auth modes (basic, bearer, apikey) don't use these parameters, it would be helpful to document that these are specifically needed for OAuth2 token resolution from stored credentials.
| /** | |
| * Builds authorization headers for a request based on collection-level and request-level auth settings. | |
| * | |
| * For most auth modes (basic, bearer, apikey), only {@link collectionRootAuth} and {@link requestAuth} | |
| * are required. The {@link collection} and {@link item} parameters are optional and are used | |
| * specifically for OAuth2 flows where the access token needs to be resolved from stored | |
| * credentials or variables defined at the collection/request level. | |
| * | |
| * @param {Object|null} collectionRootAuth Auth configuration defined at the collection root. | |
| * @param {Object|null} requestAuth Auth configuration defined on the request (may inherit from collection). | |
| * @param {Object|null} [collection=null] Optional collection object used for resolving OAuth2 tokens | |
| * from stored credentials/variables. Ignored by non-OAuth2 modes. | |
| * @param {Object|null} [item=null] Optional request/item object used for resolving OAuth2 tokens | |
| * from stored credentials/variables. Ignored by non-OAuth2 modes. | |
| * @returns {Array<Object>} An array of header objects to apply to the request. | |
| */ |
| } | ||
| } catch (error) { | ||
| console.error('Error retrieving OAuth2 access token:', error); | ||
| // Fall back to placeholder if lookup fails |
There was a problem hiding this comment.
The error handling catches all errors during OAuth2 token retrieval but only logs them to the console. In a production environment, this might make debugging difficult if token lookup fails silently. Consider whether additional error handling is needed, such as setting a more descriptive placeholder value like '<error_retrieving_token>' to help users understand that token retrieval failed, or exposing this error through the UI in some way.
| // Fall back to placeholder if lookup fails | |
| // Indicate that an error occurred during token retrieval | |
| accessToken = '<error_retrieving_token>'; |
| describe('generateSnippet with OAuth2 authentication', () => { | ||
| const language = { target: 'shell', client: 'curl' }; | ||
| const baseCollection = { root: { request: { auth: { mode: 'none' }, headers: [] } } }; | ||
|
|
||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| // Mock getAuthHeaders to return OAuth2 headers based on the auth config | ||
| const authUtils = require('utils/codegenerator/auth'); | ||
| authUtils.getAuthHeaders.mockImplementation((collectionRootAuth, requestAuth, collection = null, item = null) => { | ||
| if (requestAuth?.mode === 'oauth2') { | ||
| const oauth2Config = requestAuth.oauth2 || {}; | ||
| const tokenPlacement = oauth2Config.tokenPlacement || 'header'; | ||
| // Use the actual value from config, defaulting to 'Bearer' only if undefined | ||
| // Empty string should be preserved to test no-prefix scenarios | ||
| const tokenHeaderPrefix = oauth2Config.tokenHeaderPrefix !== undefined | ||
| ? oauth2Config.tokenHeaderPrefix | ||
| : 'Bearer'; | ||
| let accessToken = oauth2Config.accessToken || '<access_token>'; | ||
|
|
||
| // If collection and item are provided, try to look up stored credentials | ||
| if (collection && item && collection.oauth2Credentials) { | ||
| const grantType = oauth2Config.grantType || ''; | ||
| const urlToLookup = grantType === 'implicit' | ||
| ? oauth2Config.authorizationUrl || '' | ||
| : oauth2Config.accessTokenUrl || ''; | ||
| const credentialsId = oauth2Config.credentialsId || 'credentials'; | ||
| const collectionUid = collection.uid; | ||
|
|
||
| if (urlToLookup && collectionUid) { | ||
| // Look up stored credentials (simplified - assumes URL is already interpolated in test data) | ||
| const credentialsData = collection.oauth2Credentials.find( | ||
| (creds) => | ||
| creds?.url === urlToLookup | ||
| && creds?.collectionUid === collectionUid | ||
| && creds?.credentialsId === credentialsId | ||
| ); | ||
|
|
||
| if (credentialsData?.credentials?.access_token) { | ||
| accessToken = credentialsData.credentials.access_token; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (tokenPlacement === 'header') { | ||
| // Always trim the final result for consistent formatting | ||
| const headerValue = tokenHeaderPrefix | ||
| ? `${tokenHeaderPrefix} ${accessToken}`.trim() | ||
| : accessToken.trim(); | ||
| return [ | ||
| { | ||
| enabled: true, | ||
| name: 'Authorization', | ||
| value: headerValue | ||
| } | ||
| ]; | ||
| } | ||
| } | ||
| return []; | ||
| }); | ||
| }); | ||
|
|
||
| it('should include OAuth2 Bearer token in Authorization header when tokenPlacement is header', () => { | ||
| const item = { | ||
| uid: 'oauth-req', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'header', | ||
| tokenHeaderPrefix: 'Bearer', | ||
| accessToken: 'test-access-token-123' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| expect(harCall.headers).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'Bearer test-access-token-123' | ||
| }) | ||
| ]) | ||
| ); | ||
| }); | ||
|
|
||
| it('should use custom tokenHeaderPrefix when provided', () => { | ||
| const item = { | ||
| uid: 'oauth-req-custom', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'header', | ||
| tokenHeaderPrefix: 'OAuth', | ||
| accessToken: 'custom-token-456' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| expect(harCall.headers).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'OAuth custom-token-456' | ||
| }) | ||
| ]) | ||
| ); | ||
| }); | ||
|
|
||
| it('should not include Authorization header when tokenPlacement is url', () => { | ||
| const item = { | ||
| uid: 'oauth-req-url', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'url', | ||
| tokenQueryKey: 'access_token', | ||
| accessToken: 'token-in-url' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| const authHeader = harCall.headers.find((h) => h.name === 'Authorization'); | ||
| expect(authHeader).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('should use placeholder when accessToken is not available', () => { | ||
| const item = { | ||
| uid: 'oauth-req-placeholder', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'header', | ||
| tokenHeaderPrefix: 'Bearer' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| expect(harCall.headers).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'Bearer <access_token>' | ||
| }) | ||
| ]) | ||
| ); | ||
| }); | ||
|
|
||
| it('should handle empty tokenHeaderPrefix', () => { | ||
| const item = { | ||
| uid: 'oauth-req-no-prefix', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'header', | ||
| tokenHeaderPrefix: '', | ||
| accessToken: 'token-without-prefix' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| expect(harCall.headers).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'token-without-prefix' | ||
| }) | ||
| ]) | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The tests don't cover the scenario where OAuth2 credentials are retrieved from stored credentials in the collection (collection.oauth2Credentials). All test cases provide the accessToken directly in the oauth2Config. Consider adding a test case that verifies the credential lookup logic works correctly, including testing with interpolated URLs and matching collectionUid, credentialsId, and URL values. This would ensure the core feature of retrieving tokens from persisted credentials is properly tested.
| describe('generateSnippet with OAuth2 authentication', () => { | ||
| const language = { target: 'shell', client: 'curl' }; | ||
| const baseCollection = { root: { request: { auth: { mode: 'none' }, headers: [] } } }; | ||
|
|
||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| // Mock getAuthHeaders to return OAuth2 headers based on the auth config | ||
| const authUtils = require('utils/codegenerator/auth'); | ||
| authUtils.getAuthHeaders.mockImplementation((collectionRootAuth, requestAuth, collection = null, item = null) => { | ||
| if (requestAuth?.mode === 'oauth2') { | ||
| const oauth2Config = requestAuth.oauth2 || {}; | ||
| const tokenPlacement = oauth2Config.tokenPlacement || 'header'; | ||
| // Use the actual value from config, defaulting to 'Bearer' only if undefined | ||
| // Empty string should be preserved to test no-prefix scenarios | ||
| const tokenHeaderPrefix = oauth2Config.tokenHeaderPrefix !== undefined | ||
| ? oauth2Config.tokenHeaderPrefix | ||
| : 'Bearer'; | ||
| let accessToken = oauth2Config.accessToken || '<access_token>'; | ||
|
|
||
| // If collection and item are provided, try to look up stored credentials | ||
| if (collection && item && collection.oauth2Credentials) { | ||
| const grantType = oauth2Config.grantType || ''; | ||
| const urlToLookup = grantType === 'implicit' | ||
| ? oauth2Config.authorizationUrl || '' | ||
| : oauth2Config.accessTokenUrl || ''; | ||
| const credentialsId = oauth2Config.credentialsId || 'credentials'; | ||
| const collectionUid = collection.uid; | ||
|
|
||
| if (urlToLookup && collectionUid) { | ||
| // Look up stored credentials (simplified - assumes URL is already interpolated in test data) | ||
| const credentialsData = collection.oauth2Credentials.find( | ||
| (creds) => | ||
| creds?.url === urlToLookup | ||
| && creds?.collectionUid === collectionUid | ||
| && creds?.credentialsId === credentialsId | ||
| ); | ||
|
|
||
| if (credentialsData?.credentials?.access_token) { | ||
| accessToken = credentialsData.credentials.access_token; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (tokenPlacement === 'header') { | ||
| // Always trim the final result for consistent formatting | ||
| const headerValue = tokenHeaderPrefix | ||
| ? `${tokenHeaderPrefix} ${accessToken}`.trim() | ||
| : accessToken.trim(); | ||
| return [ | ||
| { | ||
| enabled: true, | ||
| name: 'Authorization', | ||
| value: headerValue | ||
| } | ||
| ]; | ||
| } | ||
| } | ||
| return []; | ||
| }); | ||
| }); | ||
|
|
||
| it('should include OAuth2 Bearer token in Authorization header when tokenPlacement is header', () => { | ||
| const item = { | ||
| uid: 'oauth-req', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'header', | ||
| tokenHeaderPrefix: 'Bearer', | ||
| accessToken: 'test-access-token-123' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| expect(harCall.headers).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'Bearer test-access-token-123' | ||
| }) | ||
| ]) | ||
| ); | ||
| }); | ||
|
|
||
| it('should use custom tokenHeaderPrefix when provided', () => { | ||
| const item = { | ||
| uid: 'oauth-req-custom', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'header', | ||
| tokenHeaderPrefix: 'OAuth', | ||
| accessToken: 'custom-token-456' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| expect(harCall.headers).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'OAuth custom-token-456' | ||
| }) | ||
| ]) | ||
| ); | ||
| }); | ||
|
|
||
| it('should not include Authorization header when tokenPlacement is url', () => { | ||
| const item = { | ||
| uid: 'oauth-req-url', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'url', | ||
| tokenQueryKey: 'access_token', | ||
| accessToken: 'token-in-url' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| const authHeader = harCall.headers.find((h) => h.name === 'Authorization'); | ||
| expect(authHeader).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('should use placeholder when accessToken is not available', () => { | ||
| const item = { | ||
| uid: 'oauth-req-placeholder', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'header', | ||
| tokenHeaderPrefix: 'Bearer' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| expect(harCall.headers).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'Bearer <access_token>' | ||
| }) | ||
| ]) | ||
| ); | ||
| }); | ||
|
|
||
| it('should handle empty tokenHeaderPrefix', () => { | ||
| const item = { | ||
| uid: 'oauth-req-no-prefix', | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com/users', | ||
| headers: [], | ||
| auth: { | ||
| mode: 'oauth2', | ||
| oauth2: { | ||
| grantType: 'client_credentials', | ||
| tokenPlacement: 'header', | ||
| tokenHeaderPrefix: '', | ||
| accessToken: 'token-without-prefix' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| generateSnippet({ language, item, collection: baseCollection, shouldInterpolate: false }); | ||
|
|
||
| const harUtils = require('utils/codegenerator/har'); | ||
| const harCall = harUtils.buildHarRequest.mock.calls[0][0]; | ||
| expect(harCall.headers).toEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'token-without-prefix' | ||
| }) | ||
| ]) | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The tests don't cover the scenario where grantType is 'implicit', which causes the code to look up credentials using authorizationUrl instead of accessTokenUrl. Consider adding a test case that verifies this logic path works correctly to ensure both credential lookup mechanisms are tested.
* feat: enhance OAuth2 support in snippet generation * Updated getAuthHeaders function to handle OAuth2 authentication, including token retrieval and placement. * Added tests for OAuth2 scenarios, ensuring correct Authorization header generation and handling of edge cases. * Improved error handling for access token retrieval from stored credentials. * refactor: standardize comparison operators in getAuthHeaders function * Updated comparison operators in the getAuthHeaders function to use strict equality (===) for improved consistency and reliability in credential checks. * fix: correct block structure in OAuth2 case of getAuthHeaders function * Added missing block structure for the 'oauth2' case in the getAuthHeaders function to ensure proper execution flow and maintain code clarity. * feat: enhance OAuth2 credential retrieval in getAuthHeaders function * Updated getAuthHeaders function to support retrieval of stored OAuth2 credentials based on collection and item context. * Improved access token handling by checking for existing credentials before falling back to default values. * Enhanced test coverage for OAuth2 scenarios to ensure accurate token management and error handling. * fix: preserve tokenHeaderPrefix value in OAuth2 configuration * Updated snippet-generator.spec.js to ensure that the tokenHeaderPrefix from OAuth2 configuration is preserved, allowing for empty string scenarios. * Default to 'Bearer' only if the tokenHeaderPrefix is undefined, enhancing flexibility in token management. * fix: ensure consistent formatting of authorization header in OAuth2 handling * Updated getAuthHeaders function to always trim the final result of the authorization header for consistent formatting. * Adjusted snippet-generator.spec.js to reflect the same trimming logic for the access token, enhancing test reliability. * fix: clarify token placement handling in getAuthHeaders function * Updated comments in the getAuthHeaders function to specify that when tokenPlacement is 'url', no auth headers are added, and that token placement in the URL/query params must be managed separately. * fix: ensure safe handling of OAuth2 credentials in getAuthHeaders function * Updated getAuthHeaders function to default to an empty array when accessing oauth2Credentials, preventing potential errors when no credentials are available.
Description
This PR closes #6591
Adds support to OAuth 2.0 token inclusion in generated code snippet.
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.