fix: Code Generation for Basic Auth#6474
fix: Code Generation for Basic Auth#6474bijin-bruno merged 10 commits intousebruno:fix/oauth-code-genfrom
Conversation
# Conflicts: # packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js
WalkthroughRefactors interpolation by replacing 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)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.js(1 hunks)packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.spec.js(1 hunks)packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js(3 hunks)packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js(7 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/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js
🧠 Learnings (4)
📓 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-16T07:16:08.934Z
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:08.934Z
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.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.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-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/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js
🧬 Code graph analysis (2)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (1)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (3)
require(10-10)generateSnippet(7-45)harRequest(32-35)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (5)
packages/bruno-app/src/components/FolderSettings/Auth/index.js (2)
request(30-30)request(50-50)packages/bruno-app/src/components/RequestPane/Auth/index.js (4)
request(35-37)item(21-21)requestTreePath(32-32)getTreePathFromCollectionToItem(19-27)packages/bruno-cli/src/utils/collection.js (3)
getTreePathFromCollectionToItem(368-376)headers(123-123)mergeHeaders(122-153)packages/bruno-electron/src/utils/collection.js (3)
getTreePathFromCollectionToItem(274-282)mergeHeaders(8-50)variables(586-586)packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.js (2)
interpolateObject(7-38)interpolateObject(7-38)
🔇 Additional comments (7)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (3)
1-1: LGTM - Import aligns with test requirements.The import of
getAuthHeadersis necessary for the new basic authentication test at line 456 where the actual implementation is used to validate header generation.
61-63: LGTM - Mock variables support the new test.Adding
userandpassvariables to the mockgetAllVariablesoutput enables the basic authentication test case to function correctly.
153-154: LGTM - Compact JSON aligns with interpolation behavior.The switch from pretty-printed to compact JSON strings in test expectations correctly reflects the new
interpolateObjectfunction's behavior, which performs string interpolation on the JSON without reformatting.Also applies to: 207-208, 270-270, 370-370, 421-424
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.js (1)
1-38: LGTM - Clean, robust implementation.The
interpolateObjectfunction provides a solid foundation for recursive variable interpolation:
- Circular reference detection prevents infinite loops
- Handles all common value types (strings, arrays, plain objects, null, primitives)
- Uses appropriate lodash-es utilities for object traversal
The WeakSet approach for cycle detection is efficient and memory-safe since it doesn't prevent garbage collection of tracked objects.
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (2)
14-22: Critical fix correctly applied.The sequence is now correct for basic auth interpolation:
- Line 14:
requestchanged toletto allow reassignment- Lines 20-22: Request interpolation happens before auth header generation
- Lines 24-29: Auth headers are generated from the interpolated request
This ensures that
{{username}}and{{password}}placeholders inrequest.authare replaced with actual values before base64 encoding occurs ingetAuthHeaders. This is the core fix described in the PR objectives.
3-3: The shortened import path is valid.The import from
'utils/collections'correctly resolves to'utils/collections/index.js'. All three functions (getAllVariables,getTreePathFromCollectionToItem,mergeHeaders) are properly exported as named exports from the module.packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/interpolation.spec.js (1)
1-95: LGTM - Comprehensive test coverage.The test suite thoroughly validates the new
interpolateObjectimplementation:
- Complex nested structures with multiple data types (lines 5-60)
- Circular reference protection (lines 62-69)
- Missing variable handling with placeholder preservation (lines 71-82)
- Edge cases including null inputs (lines 93-95)
The tests are well-structured with clear naming and follow AAA (Arrange-Act-Assert) pattern.
...ts/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js
Outdated
Show resolved
Hide resolved
| it('should interpolate basic auth credentials correctly', () => { | ||
| const item = { | ||
| request: { | ||
| method: 'GET', | ||
| url: 'https://api.example.com', | ||
| auth: { | ||
| mode: 'basic', | ||
| basic: { | ||
| username: '{{user}}', | ||
| password: '{{pass}}' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const collection = { | ||
| root: { | ||
| request: { | ||
| vars: { | ||
| req: [ | ||
| { name: 'user', value: 'admin', enabled: true }, | ||
| { name: 'pass', value: 'secret123', enabled: true } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const { HTTPSnippet: mockedHTTPSnippet } = require('httpsnippet'); | ||
| const { getAuthHeaders: actualGetAuthHeaders } = jest.requireActual('utils/codegenerator/auth'); | ||
| getAuthHeaders.mockImplementation(actualGetAuthHeaders); | ||
|
|
||
| const language = { target: 'shell', client: 'curl' }; | ||
|
|
||
| generateSnippet({ | ||
| language, | ||
| item, | ||
| collection, | ||
| shouldInterpolate: true | ||
| }); | ||
|
|
||
| const harRequest = mockedHTTPSnippet.mock.calls[0][0]; | ||
|
|
||
| // "admin:secret123" encoded is "YWRtaW46c2VjcmV0MTIz" | ||
| expect(harRequest.headers).toContainEqual( | ||
| expect.objectContaining({ | ||
| name: 'Authorization', | ||
| value: 'Basic YWRtaW46c2VjcmV0MTIz' | ||
| }) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The test validates basic auth interpolation correctly, but consider clarifying the data flow.
The test depends on the real getAuthHeaders implementation, which is appropriate since basic auth processing involves interpolating template values from collection vars before encoding. However, the actual interpolation and header generation happen in the request preparation phase (via _interpolate), not within getAuthHeaders itself. The test is correct—interpolated credentials do reach the auth handler with resolved values (e.g., 'admin' and 'secret123' instead of '{{user}}' and '{{pass}}'), and they're properly base64-encoded as shown in the assertion.
| @@ -24,14 +28,6 @@ const generateSnippet = ({ language, item, collection, shouldInterpolate = false | |||
| headers = [...headers, ...authHeaders]; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
We need to interpolate the merged headers too.
Description
Fix basic auth variable interpolation in generated code snippets. Code generation currently mishandles requests that use Basic Auth credentials defined via variables (e.g. {{username}}, {{password}}). The generator base64-encodes the literal variable placeholders instead of interpolating them first. Demo of the issue and the fix has been included below with the current behavior on the left pane and the behavior present in this PR on the right.
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
Please see here for more information.
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.