Add 'None' option to 'Add token to' options in the OAuth2 settings#7898
Open
suchmememanyskill wants to merge 1 commit intousebruno:mainfrom
Open
Add 'None' option to 'Add token to' options in the OAuth2 settings#7898suchmememanyskill wants to merge 1 commit intousebruno:mainfrom
suchmememanyskill wants to merge 1 commit intousebruno:mainfrom
Conversation
suchmememanyskill
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a “None” option for OAuth2 “Add token to” behavior so Bruno can fetch/store OAuth2 credentials without automatically injecting the token into request headers or URL—enabling fully manual token handling via pre-request scripts.
Changes:
- Extend OAuth2 schema typing to allow
tokenPlacement: 'none'. - Update Bruno language stringifiers to only emit
token_header_prefix/token_query_keywhen applicable. - Update Electron HTTP/WS/gRPC request preparation to skip token injection unless placement is explicitly
headerorurl, and add tests + UI dropdown options for “None”.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/bruno-schema-types/src/common/auth.ts | Types tokenPlacement as `'header' |
| packages/bruno-lang/v2/src/jsonToBru.js | Only emits token_query_key when placement is url. |
| packages/bruno-lang/v2/src/jsonToCollectionBru.js | Same conditional emission change for collection BRU output. |
| packages/bruno-lang/v2/tests/jsonToBru.spec.js | Adds coverage for tokenPlacement: 'none' output behavior. |
| packages/bruno-electron/src/ipc/network/index.js | HTTP request configuration now only injects token when placement is header or url. |
| packages/bruno-electron/src/ipc/network/ws-event-handlers.js | WS request handling now only injects token when placement is header or url. |
| packages/bruno-electron/src/ipc/network/prepare-grpc-request.js | gRPC token placement helper now only injects token when placement is header or url. |
| packages/bruno-electron/tests/network/index.spec.js | Adds test ensuring none doesn’t inject token for HTTP configureRequest. |
| packages/bruno-electron/tests/network/prepare-grpc-request.spec.js | Adds test ensuring none doesn’t inject token for gRPC flow. |
| packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js | Adds “None” option + hides URL query key input when not url. |
| packages/bruno-app/src/components/RequestPane/Auth/OAuth2/ClientCredentials/index.js | Adds “None” option + hides URL query key input when not url. |
| packages/bruno-app/src/components/RequestPane/Auth/OAuth2/PasswordCredentials/index.js | Adds “None” option + hides URL query key input when not url. |
| packages/bruno-app/src/components/RequestPane/Auth/OAuth2/Implicit/index.js | Adds “None” option + hides URL query key input when not url. |
Comments suppressed due to low confidence (8)
packages/bruno-electron/src/ipc/network/index.js:294
- This check only places the token in the URL when
tokenPlacement == 'url'. If existing requests can still contain legacytokenPlacement: 'query', tokens will no longer be appended to the URL. Consider treating'query'as an alias for'url'(or normalizingtokenPlacementbefore this switch) so older data continues to work.
const tokenValue = tokenSource === 'id_token' ? credentials?.id_token : credentials?.access_token;
if (tokenPlacement == 'header' && tokenValue) {
request.headers['Authorization'] = `${tokenHeaderPrefix} ${tokenValue}`.trim();
} else if (tokenPlacement == 'url' && tokenValue) {
try {
const url = new URL(request.url);
url.searchParams.set(tokenQueryKey, tokenValue);
request.url = url.toString();
} catch (error) { }
packages/bruno-electron/src/ipc/network/ws-event-handlers.js:215
- This logic now only appends the token to the URL when
tokenPlacement == 'url'. If any existing OAuth2 configs use legacytokenPlacement: 'query', they will no longer get the token appended. Consider supporting'query'as an alias for URL placement (or normalizing the value before this check).
if (tokenPlacement == 'header') {
wsRequest.headers['Authorization'] = `${tokenHeaderPrefix} ${credentials?.access_token}`;
} else if (tokenPlacement == 'url') {
try {
const url = new URL(request.url);
url?.searchParams?.set(tokenQueryKey, credentials?.access_token);
request.url = url?.toString();
} catch (error) {}
packages/bruno-electron/src/ipc/network/ws-event-handlers.js:246
- This logic now only appends the token to the URL when
tokenPlacement == 'url'. If any existing OAuth2 configs use legacytokenPlacement: 'query', they will no longer get the token appended. Consider supporting'query'as an alias for URL placement (or normalizing the value before this check).
if (tokenPlacement == 'header') {
wsRequest.headers['Authorization'] = `${tokenHeaderPrefix} ${credentials?.access_token}`;
} else if (tokenPlacement == 'url') {
try {
const url = new URL(request.url);
url?.searchParams?.set(tokenQueryKey, credentials?.access_token);
request.url = url?.toString();
} catch (error) {}
packages/bruno-electron/src/ipc/network/prepare-grpc-request.js:25
placeOAuth2Tokennow only appends to the URL whentokenPlacement === 'url'. If any existing OAuth2 configs still use legacytokenPlacement: 'query', the token will no longer be placed. Consider supporting'query'as an alias for URL placement (or normalizing tokenPlacement before calling this helper).
const placeOAuth2Token = (grpcRequest, credentials, tokenPlacement, tokenHeaderPrefix, tokenQueryKey) => {
if (tokenPlacement === 'header') {
grpcRequest.headers['Authorization'] = `${tokenHeaderPrefix} ${credentials?.access_token}`;
} else if (tokenPlacement === 'url') {
try {
const url = new URL(grpcRequest.url);
url?.searchParams?.set(tokenQueryKey, credentials?.access_token);
grpcRequest.url = url?.toString();
packages/bruno-electron/src/ipc/network/index.js:277
- This check only places the token in the URL when
tokenPlacement == 'url'. If existing requests can still contain legacytokenPlacement: 'query', tokens will no longer be appended to the URL. Consider treating'query'as an alias for'url'(or normalizingtokenPlacementbefore this switch) so older data continues to work.
const tokenValue = tokenSource === 'id_token' ? credentials?.id_token : credentials?.access_token;
if (tokenPlacement == 'header' && tokenValue) {
request.headers['Authorization'] = `${tokenHeaderPrefix} ${tokenValue}`.trim();
} else if (tokenPlacement == 'url' && tokenValue) {
try {
const url = new URL(request.url);
url.searchParams.set(tokenQueryKey, tokenValue);
request.url = url.toString();
} catch (error) { }
packages/bruno-electron/src/ipc/network/ws-event-handlers.js:184
- This logic now only appends the token to the URL when
tokenPlacement == 'url'. If any existing OAuth2 configs use legacytokenPlacement: 'query', they will no longer get the token appended. Consider supporting'query'as an alias for URL placement (or normalizing the value before this check).
if (tokenPlacement == 'header') {
wsRequest.headers['Authorization'] = `${tokenHeaderPrefix} ${credentials?.access_token}`;
} else if (tokenPlacement == 'url') {
try {
const url = new URL(request.url);
url?.searchParams?.set(tokenQueryKey, credentials?.access_token);
request.url = url?.toString();
} catch (error) {}
packages/bruno-electron/src/ipc/network/index.js:243
- This check only places the token in the URL when
tokenPlacement == 'url'. If existing requests can still contain legacytokenPlacement: 'query', tokens will no longer be appended to the URL. Consider treating'query'as an alias for'url'(or normalizingtokenPlacementbefore this switch) so older data continues to work.
const tokenValue = tokenSource === 'id_token' ? credentials?.id_token : credentials?.access_token;
if (tokenPlacement == 'header' && tokenValue) {
request.headers['Authorization'] = `${tokenHeaderPrefix} ${tokenValue}`.trim();
} else if (tokenPlacement == 'url' && tokenValue) {
try {
const url = new URL(request.url);
url.searchParams.set(tokenQueryKey, tokenValue);
request.url = url.toString();
} catch (error) { }
packages/bruno-electron/src/ipc/network/index.js:260
- This check only places the token in the URL when
tokenPlacement == 'url'. If existing requests can still contain legacytokenPlacement: 'query', tokens will no longer be appended to the URL. Consider treating'query'as an alias for'url'(or normalizingtokenPlacementbefore this switch) so older data continues to work.
const tokenValue = tokenSource === 'id_token' ? credentials?.id_token : credentials?.access_token;
if (tokenPlacement == 'header' && tokenValue) {
request.headers['Authorization'] = `${tokenHeaderPrefix} ${tokenValue}`.trim();
} else if (tokenPlacement == 'url' && tokenValue) {
try {
const url = new URL(request.url);
url.searchParams.set(tokenQueryKey, tokenValue);
request.url = url.toString();
} catch (error) { }
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds a new OAuth2 token placement value Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI
participant Renderer as Renderer (app)
participant Main as Electron Main / IPC
participant Network as Network Configurer
participant TokenSvc as OAuth2 Token Service
participant Request as Outbound Request
UI->>Renderer: user selects tokenPlacement = none
Renderer->>Main: send configured request + oauth2 settings
Main->>TokenSvc: request access token (grant-specific)
TokenSvc-->>Main: return access_token
Main->>Network: configureRequest with oauth2 + access_token
alt tokenPlacement == 'header'
Network->>Request: set Authorization header
else tokenPlacement == 'url'
Network->>Request: append tokenQueryKey to URL
else tokenPlacement == 'none'
Network->>Request: do NOT modify headers or URL (store token in credentials)
end
Network->>Request: dispatch request
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/bruno-app/src/components/RequestPane/Auth/OAuth2/Implicit/index.js (1)
212-264:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the fallback label singular.
The
nonebranch and the newurlgating look right, but the selected label still falls back to “Headers” while the dropdown item is “Header”. That mismatch will show the wrong text for stale or unexpected values.Fix
- {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Headers'} + {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Header'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth2/Implicit/index.js` around lines 212 - 264, The displayed fallback label in the MenuDropdown's token-placement-label currently shows "Headers" while the menu item uses "Header", causing mismatches for stale values; update the ternary inside the MenuDropdown child (where tokenPlacement is rendered) to use 'Header' instead of 'Headers' so it reads: tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Header' (reference symbols: tokenPlacement, MenuDropdown, token-placement-label).packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js (1)
277-331:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the fallback label singular.
The
nonehandling is correct, but the selected label still falls back to “Headers” while the dropdown item is “Header”. That copy mismatch will show up for stale or unexpected values.Fix
- {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Headers'} + {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Header'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js` around lines 277 - 331, The selected label fallback uses the plural "Headers" while the dropdown item is "Header", causing a copy mismatch; in the JSX where tokenPlacement is rendered inside MenuDropdown (look for tokenPlacement, MenuDropdown and the token-placement-label div), replace the fallback string "Headers" with the singular "Header" so the displayed label matches the dropdown item for 'header' and for stale/unknown values.packages/bruno-app/src/components/RequestPane/Auth/OAuth2/PasswordCredentials/index.js (1)
171-225:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the fallback label singular.
The
nonehandling is fine, but the selected label still falls back to “Headers” while the dropdown item is “Header”. That mismatch will surface the wrong copy for stale or unexpected values.Fix
- {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Headers'} + {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Header'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth2/PasswordCredentials/index.js` around lines 171 - 225, The selected label fallback currently renders "Headers" for any value other than 'url' or 'none', causing a mismatch with the dropdown item "Header"; update the rendering logic inside the MenuDropdown label (where tokenPlacement is read) so it uses the singular "Header" when tokenPlacement === 'header', "URL" when 'url', and "None" when 'none' (refer to tokenPlacement, MenuDropdown, and the token-placement-label block) to ensure the displayed copy matches the dropdown items.packages/bruno-app/src/components/RequestPane/Auth/OAuth2/ClientCredentials/index.js (1)
167-221:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the fallback label singular.
The new
nonebranch is fine, but the selected label still falls back to “Headers” while the dropdown item is “Header”. That mismatch will surface stale/unknown values with the wrong copy.Fix
- {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Headers'} + {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Header'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth2/ClientCredentials/index.js` around lines 167 - 221, The dropdown selected label falls back to "Headers" while the dropdown item is "Header", causing a mismatch; update the rendered label logic in the MenuDropdown child (where tokenPlacement is read) to use the singular "Header" instead of "Headers" (i.e., change the ternary that renders {tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' : 'Headers'} to return 'Header' for the default case) so the displayed selection matches the MenuDropdown item IDs and labels.packages/bruno-electron/src/ipc/network/ws-event-handlers.js (1)
177-185:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrite the updated URL back to
wsRequest.
prepareWsRequest()returnswsRequest, but these branches mutaterequest.url. That means the OAuth2 query param never reaches the WebSocket client, sotokenPlacement: 'url'still behaves likenone.Suggested fix
- const url = new URL(request.url); - url?.searchParams?.set(tokenQueryKey, credentials?.access_token); - request.url = url?.toString(); + const url = new URL(wsRequest.url); + url.searchParams.set(tokenQueryKey, credentials?.access_token); + wsRequest.url = url.toString();Also applies to: 208-216, 239-247
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/network/ws-event-handlers.js` around lines 177 - 185, In prepareWsRequest() the 'url' tokenPlacement branch mutates the local request.url but never writes that back into the object returned to the caller (wsRequest), so the OAuth2 query param never reaches the WebSocket client; update the handler so after constructing the new URL you assign the stringified URL back onto the outgoing wsRequest (e.g., set wsRequest.url or the appropriate property that the WS client reads) instead of only mutating request.url; apply the same fix for the other occurrences referenced (the blocks around lines 208-216 and 239-247) so each branch writes the updated URL into the wsRequest returned by prepareWsRequest().
🧹 Nitpick comments (1)
packages/bruno-lang/v2/tests/jsonToBru.spec.js (1)
140-175: ⚡ Quick winBroaden the regression to the other grant types.
This only exercises
client_credentials, but the same serializer branch changed forauthorization_code,implicit, andpasswordas well. A small table-driven case would catch regressions in the repeated template.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/tests/jsonToBru.spec.js` around lines 140 - 175, Add table-driven tests to extend the existing oauth2 token placement spec so it covers all grant types (client_credentials, authorization_code, implicit, password) rather than only client_credentials: refactor the test case using a loop or array of grantType values and for each build the input object (same shape as the existing test) but substitute oauth2.grantType accordingly, call stringify(input) and assert the same expectations on token_placement, absence of token_header_prefix and token_query_key; update or add test(s) around the existing describe('oauth2 token placement') and the stringify usage so the serializer branch for all grant types is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js`:
- Around line 277-331: The selected label fallback uses the plural "Headers"
while the dropdown item is "Header", causing a copy mismatch; in the JSX where
tokenPlacement is rendered inside MenuDropdown (look for tokenPlacement,
MenuDropdown and the token-placement-label div), replace the fallback string
"Headers" with the singular "Header" so the displayed label matches the dropdown
item for 'header' and for stale/unknown values.
In
`@packages/bruno-app/src/components/RequestPane/Auth/OAuth2/ClientCredentials/index.js`:
- Around line 167-221: The dropdown selected label falls back to "Headers" while
the dropdown item is "Header", causing a mismatch; update the rendered label
logic in the MenuDropdown child (where tokenPlacement is read) to use the
singular "Header" instead of "Headers" (i.e., change the ternary that renders
{tokenPlacement == 'url' ? 'URL' : tokenPlacement == 'none' ? 'None' :
'Headers'} to return 'Header' for the default case) so the displayed selection
matches the MenuDropdown item IDs and labels.
In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth2/Implicit/index.js`:
- Around line 212-264: The displayed fallback label in the MenuDropdown's
token-placement-label currently shows "Headers" while the menu item uses
"Header", causing mismatches for stale values; update the ternary inside the
MenuDropdown child (where tokenPlacement is rendered) to use 'Header' instead of
'Headers' so it reads: tokenPlacement == 'url' ? 'URL' : tokenPlacement ==
'none' ? 'None' : 'Header' (reference symbols: tokenPlacement, MenuDropdown,
token-placement-label).
In
`@packages/bruno-app/src/components/RequestPane/Auth/OAuth2/PasswordCredentials/index.js`:
- Around line 171-225: The selected label fallback currently renders "Headers"
for any value other than 'url' or 'none', causing a mismatch with the dropdown
item "Header"; update the rendering logic inside the MenuDropdown label (where
tokenPlacement is read) so it uses the singular "Header" when tokenPlacement ===
'header', "URL" when 'url', and "None" when 'none' (refer to tokenPlacement,
MenuDropdown, and the token-placement-label block) to ensure the displayed copy
matches the dropdown items.
In `@packages/bruno-electron/src/ipc/network/ws-event-handlers.js`:
- Around line 177-185: In prepareWsRequest() the 'url' tokenPlacement branch
mutates the local request.url but never writes that back into the object
returned to the caller (wsRequest), so the OAuth2 query param never reaches the
WebSocket client; update the handler so after constructing the new URL you
assign the stringified URL back onto the outgoing wsRequest (e.g., set
wsRequest.url or the appropriate property that the WS client reads) instead of
only mutating request.url; apply the same fix for the other occurrences
referenced (the blocks around lines 208-216 and 239-247) so each branch writes
the updated URL into the wsRequest returned by prepareWsRequest().
---
Nitpick comments:
In `@packages/bruno-lang/v2/tests/jsonToBru.spec.js`:
- Around line 140-175: Add table-driven tests to extend the existing oauth2
token placement spec so it covers all grant types (client_credentials,
authorization_code, implicit, password) rather than only client_credentials:
refactor the test case using a loop or array of grantType values and for each
build the input object (same shape as the existing test) but substitute
oauth2.grantType accordingly, call stringify(input) and assert the same
expectations on token_placement, absence of token_header_prefix and
token_query_key; update or add test(s) around the existing describe('oauth2
token placement') and the stringify usage so the serializer branch for all grant
types is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6e8f87c-086f-4508-af1c-a1c82675eee7
📒 Files selected for processing (13)
packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.jspackages/bruno-app/src/components/RequestPane/Auth/OAuth2/ClientCredentials/index.jspackages/bruno-app/src/components/RequestPane/Auth/OAuth2/Implicit/index.jspackages/bruno-app/src/components/RequestPane/Auth/OAuth2/PasswordCredentials/index.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/network/prepare-grpc-request.jspackages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-electron/tests/network/index.spec.jspackages/bruno-electron/tests/network/prepare-grpc-request.spec.jspackages/bruno-lang/v2/src/jsonToBru.jspackages/bruno-lang/v2/src/jsonToCollectionBru.jspackages/bruno-lang/v2/tests/jsonToBru.spec.jspackages/bruno-schema-types/src/common/auth.ts
suchmememanyskill
force-pushed
the
sims/oauth2-dont-add-token-to-request
branch
from
a7ec3f0 to
a561c22
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (1)
179-184:⚠️ Potential issue | 🟠 Major | ⚡ Quick winURL token placement mutates the wrong object
In all three OAuth2 grant branches, Line 183/214/245 assigns to
request.urlinstead ofwsRequest.url.prepareWsRequest()returnswsRequest, so URL token placement can be lost.Suggested patch
- const url = new URL(request.url); + const url = new URL(wsRequest.url); url?.searchParams?.set(tokenQueryKey, credentials?.access_token); - request.url = url?.toString(); + wsRequest.url = url?.toString();Apply this in each branch (
authorization_code,client_credentials,password).Also applies to: 210-215, 241-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/network/ws-event-handlers.js` around lines 179 - 184, The URL token-placement branch mutates the wrong object: code sets request.url instead of the prepared wsRequest returned by prepareWsRequest(), so the added query param is dropped; update each branch (authorization_code, client_credentials, password) to operate on wsRequest (use wsRequest.url and assign wsRequest.url = url.toString()) when tokenPlacement == 'url', ensuring you still use tokenQueryKey and credentials.access_token and keep the try/catch around URL parsing.
🤖 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-electron/tests/network/prepare-grpc-request.spec.js`:
- Around line 144-146: The test is asserting result.headers.authorization
(lowercase) which will miss the actual header key added as 'Authorization' and
can yield a false positive; update the assertion to check the exact header key
used by the code (result.headers.Authorization) or better assert header presence
case-insensitively (e.g., normalize keys before asserting) so the test reliably
detects whether the OAuth header was added; update the assertion referencing
result.headers.authorization to use result.headers.Authorization (or a
normalized lookup) and keep the other expectations (result.url and
result.oauth2Credentials.credentials.access_token) unchanged.
---
Outside diff comments:
In `@packages/bruno-electron/src/ipc/network/ws-event-handlers.js`:
- Around line 179-184: The URL token-placement branch mutates the wrong object:
code sets request.url instead of the prepared wsRequest returned by
prepareWsRequest(), so the added query param is dropped; update each branch
(authorization_code, client_credentials, password) to operate on wsRequest (use
wsRequest.url and assign wsRequest.url = url.toString()) when tokenPlacement ==
'url', ensuring you still use tokenQueryKey and credentials.access_token and
keep the try/catch around URL parsing.
🪄 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: a61100b4-aa54-425d-a530-9c4971a7f6d4
📒 Files selected for processing (12)
packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.jspackages/bruno-app/src/components/RequestPane/Auth/OAuth2/ClientCredentials/index.jspackages/bruno-app/src/components/RequestPane/Auth/OAuth2/Implicit/index.jspackages/bruno-app/src/components/RequestPane/Auth/OAuth2/PasswordCredentials/index.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/network/prepare-grpc-request.jspackages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-electron/tests/network/index.spec.jspackages/bruno-electron/tests/network/prepare-grpc-request.spec.jspackages/bruno-lang/v2/src/jsonToBru.jspackages/bruno-lang/v2/src/jsonToCollectionBru.jspackages/bruno-lang/v2/tests/jsonToBru.spec.js
✅ Files skipped from review due to trivial changes (1)
- packages/bruno-lang/v2/tests/jsonToBru.spec.js
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/bruno-electron/src/ipc/network/prepare-grpc-request.js
- packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js
- packages/bruno-electron/src/ipc/network/index.js
- packages/bruno-lang/v2/src/jsonToBru.js
- packages/bruno-app/src/components/RequestPane/Auth/OAuth2/PasswordCredentials/index.js
- packages/bruno-app/src/components/RequestPane/Auth/OAuth2/Implicit/index.js
- packages/bruno-electron/tests/network/index.spec.js
suchmememanyskill
force-pushed
the
sims/oauth2-dont-add-token-to-request
branch
from
a561c22 to
cc2b3d7
Compare
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
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
Hi!
While setting up a particular set of endpoints for testing an application of mine, i need to send an OAuth2 token in a bit of a weird way (required to authenticate again). I implemented this with the use of pre-request scripts. Currently in Bruno, when using OAuth2, it's not possible to disable sending the token in the request. I'd prefer to not send an invalid (for my workflow) token to the server.
This PR adds the ability to disable sending the OAuth2 token automatically within the request. This allows me to purely use pre-requests to manipulate and send the OAuth2 token.
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.
Summary by CodeRabbit
New Features
Bug Fixes
Tests