#bsq -- feat: implement URL-embedded credentials handling for authentication for digest-auth#7661
Open
Pragadesh-45 wants to merge 1 commit intousebruno:mainfrom
Open
#bsq -- feat: implement URL-embedded credentials handling for authentication for digest-auth#7661Pragadesh-45 wants to merge 1 commit intousebruno:mainfrom
Pragadesh-45 wants to merge 1 commit intousebruno:mainfrom
Conversation
Pragadesh-45
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
Contributor
WalkthroughURL-embedded credentials (user:pass@host) are now detected and parsed when no explicit per-request Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PrepareRequest
participant URLParser
participant AxiosConfig
Client->>PrepareRequest: prepareRequest(item)
PrepareRequest->>PrepareRequest: Check existing basicAuth / digestConfig
alt No explicit auth
PrepareRequest->>URLParser: Parse URL for embedded credentials
URLParser-->>PrepareRequest: username, password (or null)
alt Credentials found
PrepareRequest->>AxiosConfig: Set basicAuth (decoded creds)
PrepareRequest->>AxiosConfig: Set digestConfig (decoded creds)
end
end
PrepareRequest-->>Client: axiosRequest with auth config
sequenceDiagram
participant Request
participant DigestInterceptor
participant Server
participant Retry
Request->>DigestInterceptor: First request (may include URL creds)
DigestInterceptor->>Server: Send request
Server-->>DigestInterceptor: 401 + WWW-Authenticate: Digest
alt URL-embedded creds available and no digestConfig
DigestInterceptor->>DigestInterceptor: Remove Authorization header(s)
DigestInterceptor->>DigestInterceptor: Strip username:password from URL
DigestInterceptor->>DigestInterceptor: Compute Digest Authorization using decoded URL creds
end
DigestInterceptor->>Retry: Retry request with Digest header and cleaned URL
Retry->>Server: Send retried request
Server-->>Retry: 200 OK (or 401)
Retry-->>Request: Return response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Pragadesh-45
commented
Apr 2, 2026
Contributor
Author
Pragadesh-45
left a comment
Pragadesh-45
left a comment
There was a problem hiding this comment.
Self Review Done ✅
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/bruno-electron/src/ipc/network/prepare-request.js (1)
524-539: Duplicated logic with CLI, but acceptable here.This is identical to
packages/bruno-cli/src/runner/prepare-request.js(lines 449-464). Given these are in separate packages with different build contexts, and the logic is simple (~15 lines), the duplication is tolerable.If this pattern expands, consider extracting to a shared utility in
bruno-requests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/network/prepare-request.js` around lines 524 - 539, This block duplicates URL-embedded credential parsing (setting axiosRequest.basicAuth and axiosRequest.digestConfig) found in the CLI's prepare-request; leave the code as-is for now but when this pattern appears elsewhere extract it into a shared utility (e.g., a function named parseCredentialsFromUrl or extractUrlCredentials in a new bruno-requests helper) that takes a URL string and returns { username, password } and then replace the inline logic in prepare-request.js (and packages/bruno-cli) to call that helper and set axiosRequest.basicAuth and axiosRequest.digestConfig from its result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-requests/src/auth/digestauth-helper.spec.js`:
- Around line 140-173: The test currently inspects firstRequestHeaders (set in
axiosInstance.defaults.adapter) but Axios doesn’t populate URL-embedded Basic
credentials before the adapter runs, so the assertion is meaningless; update the
test to capture and assert headers from the retry (when callCount === 2) instead
of only the first call: in the adapter save both firstRequestHeaders and
retryRequestHeaders (or just retryRequestHeaders) and after awaiting
axiosInstance(request) assert that retryRequestHeaders.Authorization matches
/^Digest / (and optionally that firstRequestHeaders did not contain a Basic
header), keeping addDigestInterceptor, request, axiosInstance.defaults.adapter
and callCount as the locating symbols to modify.
---
Nitpick comments:
In `@packages/bruno-electron/src/ipc/network/prepare-request.js`:
- Around line 524-539: This block duplicates URL-embedded credential parsing
(setting axiosRequest.basicAuth and axiosRequest.digestConfig) found in the
CLI's prepare-request; leave the code as-is for now but when this pattern
appears elsewhere extract it into a shared utility (e.g., a function named
parseCredentialsFromUrl or extractUrlCredentials in a new bruno-requests helper)
that takes a URL string and returns { username, password } and then replace the
inline logic in prepare-request.js (and packages/bruno-cli) to call that helper
and set axiosRequest.basicAuth and axiosRequest.digestConfig from its result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f9934a6b-3992-463c-ad2a-3b93cff8b47d
📒 Files selected for processing (13)
packages/bruno-cli/src/runner/prepare-request.jspackages/bruno-cli/tests/runner/prepare-request.spec.jspackages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-requests/src/auth/digestauth-helper.jspackages/bruno-requests/src/auth/digestauth-helper.spec.jspackages/bruno-tests/collection/auth/basic/via url/Basic Auth 200.brupackages/bruno-tests/collection/auth/basic/via url/Basic Auth 401.brupackages/bruno-tests/collection/auth/digest/via auth/Digest Auth 200.brupackages/bruno-tests/collection/auth/digest/via auth/Digest Auth 401.brupackages/bruno-tests/collection/auth/digest/via auth/folder.brupackages/bruno-tests/collection/auth/digest/via url/Digest Auth 200.brupackages/bruno-tests/collection/auth/digest/via url/Digest Auth 401.brupackages/bruno-tests/collection/auth/digest/via url/folder.bru
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bruno-requests/src/auth/digestauth-helper.js (1)
36-36: Consider defensive destructuring fordigestConfig.If
addDigestInterceptoris ever called withrequest.digestConfigasundefinedornull, line 36 throws. While current call sites should always provide it, a simple fallback improves robustness:Suggested fix
- let { username, password } = request.digestConfig; + let { username, password } = request.digestConfig || {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-requests/src/auth/digestauth-helper.js` at line 36, The destructuring of request.digestConfig in addDigestInterceptor (let { username, password } = request.digestConfig) can throw if digestConfig is null/undefined; update the destructuring to use a safe fallback (e.g., default to an empty object) and then handle absent username/password appropriately (skip adding the interceptor or return an error) so the function no longer throws when request.digestConfig is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/bruno-requests/src/auth/digestauth-helper.js`:
- Line 36: The destructuring of request.digestConfig in addDigestInterceptor
(let { username, password } = request.digestConfig) can throw if digestConfig is
null/undefined; update the destructuring to use a safe fallback (e.g., default
to an empty object) and then handle absent username/password appropriately (skip
adding the interceptor or return an error) so the function no longer throws when
request.digestConfig is missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 361453ed-256c-4eb6-9adc-5926337ff879
📒 Files selected for processing (13)
packages/bruno-cli/src/runner/prepare-request.jspackages/bruno-cli/tests/runner/prepare-request.spec.jspackages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-requests/src/auth/digestauth-helper.jspackages/bruno-requests/src/auth/digestauth-helper.spec.jspackages/bruno-tests/collection/auth/basic/via url/Basic Auth 200.brupackages/bruno-tests/collection/auth/basic/via url/Basic Auth 401.brupackages/bruno-tests/collection/auth/digest/via auth/Digest Auth 200.brupackages/bruno-tests/collection/auth/digest/via auth/Digest Auth 401.brupackages/bruno-tests/collection/auth/digest/via auth/folder.brupackages/bruno-tests/collection/auth/digest/via url/Digest Auth 200.brupackages/bruno-tests/collection/auth/digest/via url/Digest Auth 401.brupackages/bruno-tests/collection/auth/digest/via url/folder.bru
✅ Files skipped from review due to trivial changes (9)
- packages/bruno-tests/collection/auth/digest/via auth/Digest Auth 401.bru
- packages/bruno-tests/collection/auth/basic/via url/Basic Auth 200.bru
- packages/bruno-tests/collection/auth/digest/via url/Digest Auth 401.bru
- packages/bruno-tests/collection/auth/digest/via url/Digest Auth 200.bru
- packages/bruno-tests/collection/auth/basic/via url/Basic Auth 401.bru
- packages/bruno-tests/collection/auth/digest/via url/folder.bru
- packages/bruno-tests/collection/auth/digest/via auth/folder.bru
- packages/bruno-electron/src/ipc/network/prepare-request.js
- packages/bruno-cli/tests/runner/prepare-request.spec.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-cli/src/runner/prepare-request.js
Pragadesh-45
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes: #7560
Description
This PR fixes Digest Authentication not working when credentials are embedded in the URL (e.g.
https://admin:password@host/path) withauth: none.When credentials are embedded in the URL, Bruno never populated
digestConfig, so the Digest interceptor was never registered and Digest challenges went unhandled. Even whendigestConfigwas setexplicitly,
axioswould auto-add anAuthorization: Basicheader from the URL credentials, which caused the interceptor'scontainsAuthorizationHeadercheck to bail out before the Digest retry couldhappen.
This PR fixes the issue by extracting URL-embedded credentials in
prepare-request.jsand setting bothbasicAuth(for preemptive Basic auth) anddigestConfig(to register the Digest interceptor as afallback). The URL credential stripping is moved inside the interceptor's error handler so it only fires after a Digest challenge is confirmed — preserving native Basic auth behaviour when the server
accepts credentials on the first request.
Contribution Checklist:
Summary by CodeRabbit
New Features
Tests