fix(generate-code): preserve resolved auth for snippet-generation (cURL inherits folder Basic Auth) #6338
Conversation
- Ensure resolved auth from resolveInheritedAuth is preserved when building finalItem for GenerateCodeItem. - Explicitly set request.auth to resolvedRequest.auth to prevent 'inherit' mode overwriting resolved auth. - Make snippet generator consume resolved auth and use getAuthHeaders accordingly. - Add unit test ensuring code generation uses folder-level Basic Auth. Fixes regression introduced after v2.12.0 where generated cURL omitted inherited Basic Auth.
prashantdubeypng
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
|
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. WalkthroughThis PR fixes authentication resolution precedence in code generation. Changes ensure resolved/inherited auth from collections takes priority over raw request auth data, with updates to auth header generation function signatures and logic to support both legacy and new auth resolution workflows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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 (3)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (1)
20-35: Defensive fallback is well-implemented.The fallback logic at lines 28-34 handles edge cases where auth might still be
'inherit'after upstream resolution. Good defensive programming.Minor: Consider also checking
collectionAuth.mode !== 'inherit'at line 31 for clarity, thoughgetAuthHeadersalready handles this internally:- if (collectionAuth && collectionAuth.mode !== 'none') { + if (collectionAuth && collectionAuth.mode !== 'none' && collectionAuth.mode !== 'inherit') {packages/bruno-app/src/utils/codegenerator/auth.spec.js (1)
134-155: Good regression test for folder-inherited auth.This test validates the core fix. Minor: the
asynckeyword on line 135 is unnecessary since noawaitis used.- it('should use folder basic auth when request inherits from folder', async () => { + it('should use folder basic auth when request inherits from folder', () => {packages/bruno-app/src/utils/codegenerator/auth.js (1)
21-46: Heuristic-based parameter detection works but is fragile.The logic correctly handles both legacy and new usage patterns. However, the heuristics rely on the shape of the second parameter:
isLegacySecondParam:requestOrRequestAuth?.mode !== undefinedisNewUsageSecondParam:requestOrRequestAuth?.auth?.mode !== undefinedIf an object has both
modeandauth.modeproperties, the legacy path takes precedence (checked first). This works for current callers but could break if future code passes an unexpected shape.Consider adding a brief comment noting this precedence, or logging a warning in development if both conditions are true.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js(1 hunks)packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js(1 hunks)packages/bruno-app/src/utils/auth/index.spec.js(1 hunks)packages/bruno-app/src/utils/codegenerator/auth.js(2 hunks)packages/bruno-app/src/utils/codegenerator/auth.spec.js(1 hunks)
🧰 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/utils/auth/index.spec.jspackages/bruno-app/src/utils/codegenerator/auth.spec.jspackages/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/index.js
🧠 Learnings (3)
📓 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.299Z
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:32.975Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:32.975Z
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/utils/auth/index.spec.jspackages/bruno-app/src/utils/codegenerator/auth.spec.js
📚 Learning: 2025-12-02T07:24:50.299Z
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.299Z
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/utils/codegenerator/auth.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.jspackages/bruno-app/src/utils/codegenerator/auth.js
🧬 Code graph analysis (3)
packages/bruno-app/src/utils/auth/index.spec.js (1)
packages/bruno-app/src/utils/auth/index.js (2)
resolveInheritedAuth(7-43)resolveInheritedAuth(7-43)
packages/bruno-app/src/utils/codegenerator/auth.spec.js (1)
packages/bruno-app/src/utils/codegenerator/auth.js (2)
getAuthHeaders(12-93)getAuthHeaders(12-93)
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(12-93)getAuthHeaders(12-93)
🔇 Additional comments (5)
packages/bruno-app/src/utils/auth/index.spec.js (1)
79-96: LGTM! Solid regression test.Good coverage for the resolved auth scenario. The assertions properly verify that
mode !== 'inherit'after resolution and that credentials are present for header generation.packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js (1)
114-124: Fix correctly addresses the root cause.The explicit
authassignment on line 121 ensures resolved auth takes precedence over the raw request auth, fixing the regression where{ mode: 'inherit' }was overwriting the resolved credentials.One consideration: the fallback chain
resolvedRequest.auth || requestData.request.authmeans ifresolvedRequest.authis{ mode: 'none' }(truthy), it won't fall back torequestData.request.auth. This is likely correct behavior since resolved auth should be authoritative, but worth confirming this is intended.packages/bruno-app/src/utils/codegenerator/auth.spec.js (1)
1-132: Comprehensive test coverage.Tests thoroughly cover new usage patterns (resolved auth), legacy usage (collection vs request auth), and edge cases. The assertions properly verify header structure including
enabled,name, andvalueproperties.packages/bruno-app/src/utils/codegenerator/auth.js (2)
3-11: Good JSDoc documentation.Clear documentation of the dual-purpose parameters and return type. Helpful for maintainability.
48-51: Guard clause correctly handles unresolved/invalid auth.Returning early for
noneandinheritmodes prevents generating headers for unresolved authentication.
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where the "Generate Code" feature for cURL snippets omitted Basic Auth credentials when a request inherited authentication from a parent folder or collection. The root cause was an object spread ordering issue that allowed unresolved auth (mode: 'inherit') to overwrite the properly resolved auth object.
Key changes:
- Fixed object spread order in GenerateCodeItem to preserve resolved auth
- Refactored
getAuthHeaders()to support both resolved auth objects and legacy usage patterns - Added defensive fallback logic in snippet-generator for edge cases
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js |
Fixed object spread order and explicitly set auth from resolvedRequest to prevent overwriting |
packages/bruno-app/src/utils/codegenerator/auth.js |
Refactored getAuthHeaders to accept resolved auth while maintaining backward compatibility |
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js |
Updated to consume resolved auth with defensive fallback for unresolved cases |
packages/bruno-app/src/utils/codegenerator/auth.spec.js |
Added comprehensive unit tests for new and legacy getAuthHeaders usage patterns |
packages/bruno-app/src/utils/auth/index.spec.js |
Added regression test to ensure resolved auth is suitable for code generation |
package-lock.json |
Contains unrelated dependency metadata changes (dev → devOptional) that should be reviewed separately |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const resolvedAuth = request.auth; | ||
| if (resolvedAuth && resolvedAuth.mode && resolvedAuth.mode !== 'none' && resolvedAuth.mode !== 'inherit') { | ||
| // getAuthHeaders should accept resolved auth directly | ||
| const authHeaders = getAuthHeaders(resolvedAuth, request); | ||
| headers = [...headers, ...authHeaders]; | ||
| } else if (resolvedAuth && resolvedAuth.mode === 'inherit') { |
There was a problem hiding this comment.
The variable name resolvedAuth could be misleading since it's assigned request.auth which may or may not actually be resolved at this point (hence the defensive checks). Consider a more descriptive name like requestAuth to avoid confusion:
const requestAuth = request.auth;
if (requestAuth && requestAuth.mode && requestAuth.mode !== 'none' && requestAuth.mode !== 'inherit') {
const authHeaders = getAuthHeaders(requestAuth, request);
// ...
} else if (requestAuth && requestAuth.mode === 'inherit') {
// ...
}| const resolvedAuth = request.auth; | |
| if (resolvedAuth && resolvedAuth.mode && resolvedAuth.mode !== 'none' && resolvedAuth.mode !== 'inherit') { | |
| // getAuthHeaders should accept resolved auth directly | |
| const authHeaders = getAuthHeaders(resolvedAuth, request); | |
| headers = [...headers, ...authHeaders]; | |
| } else if (resolvedAuth && resolvedAuth.mode === 'inherit') { | |
| const requestAuth = request.auth; | |
| if (requestAuth && requestAuth.mode && requestAuth.mode !== 'none' && requestAuth.mode !== 'inherit') { | |
| // getAuthHeaders should accept resolved auth directly | |
| const authHeaders = getAuthHeaders(requestAuth, request); | |
| headers = [...headers, ...authHeaders]; | |
| } else if (requestAuth && requestAuth.mode === 'inherit') { |
| resolvedAuthOrCollectionAuth.mode !== 'inherit' && | ||
| resolvedAuthOrCollectionAuth.mode !== 'none') { |
There was a problem hiding this comment.
The condition on line 36-38 has a logical issue. When checking if the first parameter has valid resolved auth, it checks if the mode is not 'inherit' AND not 'none'. However, if resolvedAuthOrCollectionAuth has mode 'none', this means it was explicitly set to 'none', which should be respected. The function should use it instead of falling through to request.auth.
The condition should only exclude 'inherit' (unresolved case):
if (resolvedAuthOrCollectionAuth?.mode && resolvedAuthOrCollectionAuth.mode !== 'inherit') {
auth = resolvedAuthOrCollectionAuth;
}The check for 'none' is already handled later at lines 49-51 where it returns an empty array.
| resolvedAuthOrCollectionAuth.mode !== 'inherit' && | |
| resolvedAuthOrCollectionAuth.mode !== 'none') { | |
| resolvedAuthOrCollectionAuth.mode !== 'inherit') { |
| }); | ||
| }); | ||
|
|
||
| describe('generate code uses resolved folder basic auth for cURL', () => { |
There was a problem hiding this comment.
The describe block name "generate code uses resolved folder basic auth for cURL" is inconsistent with the naming pattern used in the rest of the file. It should follow the pattern of the other describe blocks which reference the module/function being tested.
Suggest renaming to:
describe('codegenerator/auth - folder auth inheritance', () => {or
describe('getAuthHeaders - folder auth resolution', () => {| describe('generate code uses resolved folder basic auth for cURL', () => { | |
| describe('codegenerator/auth - folder auth inheritance', () => { |
| }); | ||
|
|
||
| describe('generate code uses resolved folder basic auth for cURL', () => { | ||
| it('should use folder basic auth when request inherits from folder', async () => { |
There was a problem hiding this comment.
The test function is marked as async but doesn't contain any await expressions or return a promise. Remove the async keyword:
it('should use folder basic auth when request inherits from folder', () => {| it('should use folder basic auth when request inherits from folder', async () => { | |
| it('should use folder basic auth when request inherits from folder', () => { |
| * Get authentication headers for code generation. | ||
| * | ||
| * @param {Object} resolvedAuthOrCollectionAuth - Either the resolved auth object (new usage) | ||
| * or collection root auth (legacy usage) | ||
| * @param {Object} requestOrRequestAuth - Either the request object (new usage) | ||
| * or request auth (legacy usage) | ||
| * @returns {Array} Array of header objects with enabled, name, and value properties |
There was a problem hiding this comment.
The JSDoc comment should document the backward compatibility behavior more clearly. Consider adding an @example section to illustrate both legacy and new usage patterns:
/**
* Get authentication headers for code generation.
* Supports both legacy (collectionAuth, requestAuth) and new (resolvedAuth, request) usage patterns.
*
* @param {Object} resolvedAuthOrCollectionAuth - Either the resolved auth object (new usage)
* or collection root auth (legacy usage)
* @param {Object} requestOrRequestAuth - Either the request object (new usage)
* or request auth (legacy usage)
* @returns {Array} Array of header objects with enabled, name, and value properties
*
* @example
* // New usage (resolved auth)
* getAuthHeaders({ mode: 'basic', basic: {...} }, request)
*
* @example
* // Legacy usage (collection + request auth)
* getAuthHeaders(collectionAuth, { mode: 'inherit' })
*/| * Get authentication headers for code generation. | |
| * | |
| * @param {Object} resolvedAuthOrCollectionAuth - Either the resolved auth object (new usage) | |
| * or collection root auth (legacy usage) | |
| * @param {Object} requestOrRequestAuth - Either the request object (new usage) | |
| * or request auth (legacy usage) | |
| * @returns {Array} Array of header objects with enabled, name, and value properties | |
| * Get authentication headers for code generation. | |
| * Supports both legacy (collectionAuth, requestAuth) and new (resolvedAuth, request) usage patterns. | |
| * | |
| * @param {Object} resolvedAuthOrCollectionAuth - Either the resolved auth object (new usage) | |
| * or collection root auth (legacy usage) | |
| * @param {Object} requestOrRequestAuth - Either the request object (new usage) | |
| * or request auth (legacy usage) | |
| * @returns {Array} Array of header objects with enabled, name, and value properties | |
| * | |
| * @example | |
| * // New usage (resolved auth and request object) | |
| * const resolvedAuth = { mode: 'basic', basic: { username: 'user', password: 'pass' } }; | |
| * const request = { ... }; // request object | |
| * getAuthHeaders(resolvedAuth, request); | |
| * | |
| * @example | |
| * // Legacy usage (collectionAuth and requestAuth) | |
| * const collectionAuth = { mode: 'basic', basic: { username: 'user', password: 'pass' } }; | |
| * const requestAuth = { mode: 'inherit' }; | |
| * getAuthHeaders(collectionAuth, requestAuth); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isLegacySecondParam = requestOrRequestAuth?.mode !== undefined; | ||
| const isNewUsageSecondParam = requestOrRequestAuth?.auth?.mode !== undefined; |
There was a problem hiding this comment.
The detection logic for isLegacySecondParam and isNewUsageSecondParam has an issue: if the second parameter is an object with both mode and auth.mode properties, the function will incorrectly classify it as legacy usage (line 25) because isLegacySecondParam is checked first.
This could happen if a request object has a mode property at the top level for some other purpose. While unlikely in the current codebase, this creates ambiguous parameter detection. Consider checking for the new usage pattern first, or using a more robust detection method (e.g., checking if the first parameter has properties that indicate it's collection auth vs resolved auth).
| } else if (resolvedAuth && resolvedAuth.mode === 'inherit') { | ||
| // Fallback: if auth is still 'inherit', try collection auth | ||
| const collectionAuth = collection?.draft?.root ? get(collection, 'draft.root.request.auth', null) : get(collection, 'root.request.auth', null); | ||
| if (collectionAuth && collectionAuth.mode !== 'none') { | ||
| const authHeaders = getAuthHeaders(collectionAuth, request); | ||
| headers = [...headers, ...authHeaders]; | ||
| } | ||
| } |
There was a problem hiding this comment.
The fallback logic for 'inherit' mode (lines 28-35) should theoretically never execute if the auth is properly resolved by GenerateCodeItem before being passed here. This defensive code is good to have, but it indicates a potential code smell.
Consider adding a console.warn or logging when this fallback is triggered, so developers know when auth resolution didn't work as expected upstream. This would help catch bugs during development.
| }); | ||
|
|
||
| describe('generate code uses resolved folder basic auth for cURL', () => { | ||
| it('should use folder basic auth when request inherits from folder', async () => { |
There was a problem hiding this comment.
The test is marked as async but doesn't use any async operations or await. The async keyword should be removed from the test function.
| it('should use folder basic auth when request inherits from folder', async () => { | |
| it('should use folder basic auth when request inherits from folder', () => { |
1 participant
Summary
This fixes a regression where Generate Code → cURL omitted Basic Auth inherited from a folder/collection. The root cause was resolved auth being overwritten when constructing the final request used for code generation.
Changes
Preserve resolved auth in GenerateCodeItem by explicitly setting request.auth = resolvedRequest.auth.
Make snippet-generator consume resolved auth defensively and fallback in edge cases.
Refactor getAuthHeaders to accept resolved auth objects while maintaining backward compatibility with legacy usage.
Added unit tests to prevent regression and validate folder-level Basic Auth is included in generated code.
Why
The object spread order allowed requestData.request (with auth: { mode: 'inherit' }) to overwrite resolvedRequest.auth. The code generator then emitted no Authorization header.
How to test
Set folder Basic Auth (basic username/password).
Create request set to Inherit Auth.
Execute the request (should work).
Generate Code → cURL and confirm header appears:
--header 'authorization: Basic '
Notes
This preserves backward compatibility for getAuthHeaders legacy usages.
Unit tests were added/updated: packages/bruno-app/src/utils/codegenerator/auth.spec.js, packages/bruno-app/src/utils/auth/index.spec.js.
Closes: #6269
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.