-
Notifications
You must be signed in to change notification settings - Fork 2k
fix: cURL auth import for digest and ntlm #6292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: cURL auth import for digest and ntlm #6292
Conversation
WalkthroughAdds digest and NTLM authentication support across curl parsing, conversion-to-JSON, and code-snippet generation, plus tests covering parsing and curl export flag emission for these auth modes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
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(1 hunks)packages/bruno-app/src/utils/curl/curl-to-json.js(1 hunks)packages/bruno-app/src/utils/curl/parse-curl.js(5 hunks)packages/bruno-app/src/utils/curl/parse-curl.spec.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-app/src/utils/curl/curl-to-json.jspackages/bruno-app/src/utils/curl/parse-curl.spec.jspackages/bruno-app/src/utils/curl/parse-curl.jspackages/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.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
packages/bruno-app/src/utils/curl/parse-curl.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
🧠 Learnings (2)
📚 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/curl/curl-to-json.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
packages/bruno-app/src/utils/curl/parse-curl.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js
🧬 Code graph analysis (4)
packages/bruno-app/src/utils/curl/curl-to-json.js (4)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (2)
authMode(11-11)request(37-37)packages/bruno-app/src/utils/curl/parse-curl.js (4)
request(48-48)request(58-58)request(475-475)request(481-481)packages/bruno-app/src/utils/collections/index.js (1)
request(584-584)packages/bruno-app/src/utils/curl/index.js (1)
request(37-37)
packages/bruno-app/src/utils/curl/parse-curl.spec.js (1)
packages/bruno-app/src/utils/curl/parse-curl.js (1)
parseCurlCommand(45-51)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (1)
packages/bruno-app/src/utils/auth/index.js (3)
effectiveAuth(26-26)resolveInheritedAuth(7-43)resolveInheritedAuth(7-43)
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 (2)
result(72-72)generateSnippet(30-84)
🪛 Gitleaks (8.30.0)
packages/bruno-app/src/utils/curl/parse-curl.spec.js
[high] 278-278: Discovered a potential basic authorization token provided in a curl command, which could compromise the curl accessed resource.
(curl-auth-user)
[high] 298-298: Discovered a potential basic authorization token provided in a curl command, which could compromise the curl accessed resource.
(curl-auth-user)
[high] 318-318: Discovered a potential basic authorization token provided in a curl command, which could compromise the curl accessed resource.
(curl-auth-user)
⏰ 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 - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
🔇 Additional comments (11)
packages/bruno-app/src/utils/curl/curl-to-json.js (1)
186-213: LGTM! Clean implementation of digest and NTLM authentication support.The authentication handling is well-structured with consistent patterns across all three modes (basic, digest, ntlm). The use of optional chaining and the
repr()function ensures safe access to potentially undefined credential objects.packages/bruno-app/src/utils/curl/parse-curl.spec.js (1)
275-374: LGTM! Comprehensive test coverage for digest and NTLM authentication.The test cases appropriately cover various scenarios including different flag formats (
-uvs--user), flag ordering, and domain-qualified usernames for NTLM. The static analysis hints flagging curl-auth-user on lines 278, 298, and 318 are expected false positives for test credentials.packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (3)
4-4: LGTM! Import statement is correctly placed.
39-43: LGTM! Inherited authentication resolution is correctly implemented.The logic properly resolves authentication from the folder/collection hierarchy when the request uses
mode: 'inherit'.
72-77: LGTM! Post-processing approach for curl-specific auth flags is appropriate.The implementation correctly applies digest/NTLM authentication flags after HTTPSnippet conversion, which is necessary since HTTPSnippet doesn't natively support these curl-specific authentication modes.
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (1)
557-636: LGTM! Comprehensive test coverage for digest and NTLM curl export.The test suite appropriately covers digest authentication, NTLM authentication, and the edge case of username-only credentials. The regex assertions correctly verify flag order and presence in the generated curl commands.
packages/bruno-app/src/utils/curl/parse-curl.js (5)
29-30: LGTM! Flag definitions are correctly placed in the immediate action category.
154-160: LGTM! Flag handling follows the established pattern for immediate action flags.
212-236: LGTM! Authentication mode determination is correctly implemented.The logic properly determines the auth mode based on digest/ntlm flags, with a sensible default to 'basic'. The use of computed property syntax
[mode]cleanly handles the dynamic auth object structure.
456-456: LGTM! Comment accurately reflects the function's expanded responsibilities.
472-486: LGTM! Post-build auth mode correction properly handles flag ordering.The logic correctly addresses the scenario where the
-uflag appears before--digestor--ntlmin the curl command. The use of|| {}and|| ''safely handles missing auth objects and credentials.
...ts/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bruno-app/src/utils/curl/parse-curl.spec.js (1)
275-374: Digest/NTLM auth parsing tests are solid; consider one more edge case.The new tests nicely cover
--digest/--ntlmwith both-uand--user, including domain-style usernames and the--digestflag appearing after-u. To mirror the existing “username without password” basic-auth case and your curl export support for digest-without-password, you might add a small extra test forcurl --digest --user "myuser" ...to assertpassword: ''is handled consistently.Example addition (optional):
@@ it('should parse digest authentication with --user flag', () => { const result = parseCurlCommand(` curl --digest --user "admin:secret" https://api.example.com/secure `); @@ }); }); + it('should parse digest authentication with username only', () => { + const result = parseCurlCommand(` + curl --digest --user "myuser" https://api.example.com/digest + `); + + expect(result).toEqual({ + method: 'get', + auth: { + mode: 'digest', + digest: { + username: 'myuser', + password: '' + } + }, + isDigestAuth: true, + url: 'https://api.example.com/digest', + urlWithoutQuery: 'https://api.example.com/digest' + }); + });packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.js (1)
558-635: Curl export tests for digest/NTLM auth match the intended behavior.These tests validate the exact shapes we care about (
--digest/--ntlmplus--user 'username[:password]') and integrate cleanly with the existing HTTPSnippet mock. If you want to harden the inheritance path as well, you could add one more test whererequest.auth.mode === 'inherit'and the collection/root auth is digest or NTLM, asserting thatgenerateSnippetstill emits the correct flags viaresolveInheritedAuth.Example (optional) inherited-auth test:
@@ describe('generateSnippet – digest and NTLM auth curl export', () => { const language = { target: 'shell', client: 'curl' }; - const baseCollection = { + const baseCollection = { root: { request: { headers: [], auth: { mode: 'none' } } } }; @@ it('should handle digest auth with username only (no password)', () => { @@ expect(result).toMatch(/^curl --digest --user 'myuser'/); }); + + it('should use inherited digest auth from collection when mode is inherit', () => { + const collectionWithDigest = { + root: { + request: { + headers: [], + auth: { + mode: 'digest', + digest: { + username: 'inheritedUser', + password: 'inheritedPass' + } + } + } + } + }; + + const item = { + uid: 'inherit-digest-req', + request: { + method: 'GET', + url: 'https://example.com/api', + headers: [], + body: { mode: 'none' }, + auth: { mode: 'inherit' } + } + }; + + const result = generateSnippet({ language, item, collection: collectionWithDigest, shouldInterpolate: false }); + expect(result).toMatch(/^curl --digest --user 'inheritedUser:inheritedPass'/); + }); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
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(1 hunks)packages/bruno-app/src/utils/curl/curl-to-json.js(1 hunks)packages/bruno-app/src/utils/curl/parse-curl.js(5 hunks)packages/bruno-app/src/utils/curl/parse-curl.spec.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/bruno-app/src/utils/curl/curl-to-json.js
- packages/bruno-app/src/utils/curl/parse-curl.js
- packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/utils/curl/parse-curl.spec.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/utils/curl/parse-curl.spec.js
🧠 Learnings (1)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.spec.jspackages/bruno-app/src/utils/curl/parse-curl.spec.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 (2)
result(74-74)generateSnippet(32-86)
packages/bruno-app/src/utils/curl/parse-curl.spec.js (1)
packages/bruno-app/src/utils/curl/parse-curl.js (1)
parseCurlCommand(45-51)
🪛 Gitleaks (8.30.0)
packages/bruno-app/src/utils/curl/parse-curl.spec.js
[high] 278-278: Discovered a potential basic authorization token provided in a curl command, which could compromise the curl accessed resource.
(curl-auth-user)
[high] 298-298: Discovered a potential basic authorization token provided in a curl command, which could compromise the curl accessed resource.
(curl-auth-user)
[high] 318-318: Discovered a potential basic authorization token provided in a curl command, which could compromise the curl accessed resource.
(curl-auth-user)
⏰ 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: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
Description
This PR adds support for Digest and NTLM authentication in curl import and export operations.
JIRA
Contribution Checklist:
Screen.Recording.2025-12-03.at.6.42.52.PM.mov
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.