fix: getAuthMode returns 'none' for AWS, Basic Auth, NTLM#7882
Open
prateek-bruno wants to merge 6 commits intousebruno:mainfrom
Open
fix: getAuthMode returns 'none' for AWS, Basic Auth, NTLM#7882prateek-bruno wants to merge 6 commits intousebruno:mainfrom
prateek-bruno wants to merge 6 commits intousebruno:mainfrom
Conversation
Contributor
WalkthroughThis refactoring decouples authentication configuration retention from transport cleanup by moving auth object deletion from immediate post-usage cleanup to a dedicated axios request interceptor. Auth config properties are now preserved on the request object for script access, then stripped before transport. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Co-authored-by: Prateek Sunal <prtksunal@gmail.com>
prateek-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/auth/auth-mode/auth-mode-runner.spec.ts (1)
28-37: ⚡ Quick winConsider
test.stepfor clearer Playwright reporting.This test flow is already clean; wrapping setup/run/assert into
test.stepblocks would make failures easier to localize in reports.Suggested refactor
for (const mode of ['safe', 'developer'] as const) { test(`detects auth modes in ${mode} mode`, async ({ pageWithUserData: page }) => { - await setSandboxMode(page, COLLECTION_NAME, mode); - await runCollection(page, COLLECTION_NAME); - - await validateRunnerResults(page, { - totalRequests: EXPECTED_REQUESTS.length, - passed: EXPECTED_REQUESTS.length, - failed: 0, - skipped: 0 - }); + await test.step('configure sandbox mode', async () => { + await setSandboxMode(page, COLLECTION_NAME, mode); + }); + + await test.step('run auth-mode collection', async () => { + await runCollection(page, COLLECTION_NAME); + }); + + await test.step('validate runner results', async () => { + await validateRunnerResults(page, { + totalRequests: EXPECTED_REQUESTS.length, + passed: EXPECTED_REQUESTS.length, + failed: 0, + skipped: 0 + }); + }); }); }As per coding guidelines: "Promote the use of
test.stepas much as possible so the generated reports are easier to read."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/auth-mode/auth-mode-runner.spec.ts` around lines 28 - 37, Wrap the three logical phases of the test `test('detects auth modes in ${mode} mode'...)` into Playwright `test.step` calls to improve reporting: create steps around the setup (call to setSandboxMode with COLLECTION_NAME), the execution (call to runCollection with page and COLLECTION_NAME), and the assertions (call to validateRunnerResults with EXPECTED_REQUESTS.length and the result shape). Update the test body to call these functions inside descriptive `test.step` blocks so failures are reported per step while keeping existing calls to setSandboxMode, runCollection, and validateRunnerResults intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/auth/auth-mode/auth-mode-runner.spec.ts`:
- Around line 28-37: Wrap the three logical phases of the test `test('detects
auth modes in ${mode} mode'...)` into Playwright `test.step` calls to improve
reporting: create steps around the setup (call to setSandboxMode with
COLLECTION_NAME), the execution (call to runCollection with page and
COLLECTION_NAME), and the assertions (call to validateRunnerResults with
EXPECTED_REQUESTS.length and the result shape). Update the test body to call
these functions inside descriptive `test.step` blocks so failures are reported
per step while keeping existing calls to setSandboxMode, runCollection, and
validateRunnerResults intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac8409ae-47f0-4eed-bf92-77e40ed47d93
📒 Files selected for processing (30)
packages/bruno-cli/src/runner/run-single-request.jspackages/bruno-cli/src/utils/axios-instance.jspackages/bruno-cli/tests/utils/axios-instance.spec.jspackages/bruno-electron/src/ipc/network/axios-instance.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/tests/network/axios-instance.spec.jspackages/bruno-js/src/bruno-request.jspackages/bruno-js/tests/bruno-request-auth-mode.spec.jspackages/bruno-requests/src/auth/oauth1-request-authorization.tstests/auth/apikey/apikey-runner.spec.tstests/auth/auth-mode/auth-mode-runner.spec.tstests/auth/auth-mode/fixtures/collections/auth-mode-test/api-key-header.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/api-key-query.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/aws-sigv4.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/basic-auth.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/bearer.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/bruno.jsontests/auth/auth-mode/fixtures/collections/auth-mode-test/digest.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/inherit-aws-sigv4/folder.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/inherit-aws-sigv4/inherited-aws-sigv4.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/inherit-basic-auth/folder.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/inherit-basic-auth/inherited-basic-auth.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/inherit-ntlm/folder.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/inherit-ntlm/inherited-ntlm.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/no-auth.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/ntlm.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/oauth1.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/oauth2.brutests/auth/auth-mode/fixtures/collections/auth-mode-test/wsse.brutests/auth/auth-mode/init-user-data/preferences.json
💤 Files with no reviewable changes (4)
- packages/bruno-requests/src/auth/oauth1-request-authorization.ts
- packages/bruno-cli/src/runner/run-single-request.js
- tests/auth/apikey/apikey-runner.spec.ts
- packages/bruno-electron/src/ipc/network/index.js
prateek-bruno
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.
Description
Jira
getAuthMode wasn't working correctly for these types:
This PR along with fixing the getAuthMode for them, adds proper E2E test for all types of auth mode, so getAuthMode works correctly for all.
Contribution Checklist:
Summary by CodeRabbit
New Features
Tests