fix: get certs and proxy config based on oauth2 token and refresh urls instead of resource url#6164
Conversation
bru-2005certs and proxy config based on oauth2 token and refresh urls instead of resource url bru-2005
867e5d7 to
ac1458f
Compare
ac1458f to
3b52481
Compare
WalkthroughThis PR applies per-URL TLS and proxy resolution to OAuth2 token and refresh flows by interpolating token/refresh URLs (including prompt/collection/env variables) and deriving URL-specific certsAndProxyConfig values, then using those configs when fetching or refreshing OAuth2 tokens across IPC/network and oauth2 utility layers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant IPC_Network as "IPC / Network"
participant Interpolator as "interpolateString"
participant CertResolver as "getCertsAndProxyConfig"
participant OAuth2 as "oauth2 helpers"
participant AuthServer as "Token Endpoint"
Client->>IPC_Network: Request requiring OAuth2 token
IPC_Network->>Interpolator: interpolate accessTokenUrl / refreshTokenUrl (envs, collection, promptVariables)
Interpolator-->>IPC_Network: interpolated URLs
IPC_Network->>CertResolver: compute certsAndProxyConfigForTokenUrl / ForRefreshUrl (per-URL)
CertResolver-->>IPC_Network: certs/proxy configs
IPC_Network->>OAuth2: getOAuth2TokenUsing* (pass per-URL certs/proxy)
OAuth2->>AuthServer: token request (uses passed certs/proxy)
AuthServer-->>OAuth2: token response (or error)
OAuth2-->>IPC_Network: credentials or error
IPC_Network-->>Client: proceed using token or surface error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 0
🧹 Nitpick comments (2)
packages/bruno-electron/src/utils/oauth2.js (1)
631-682: VerifyrefreshOauth2Tokenreceives correct config.The
refreshOauth2Tokenfunction receivescertsAndProxyConfigas parameter (line 631) and uses it at line 669. Callers now passcertsAndProxyConfigForRefreshUrlas this argument. Ensure consistency in naming - the parameter iscertsAndProxyConfiginternally but callers pass the refresh-specific config. This works correctly but the internal naming doesn't reflect the intent.Consider renaming the parameter to
certsAndProxyConfigForRefreshUrlfor clarity:-const refreshOauth2Token = async ({ requestCopy, collectionUid, certsAndProxyConfig }) => { +const refreshOauth2Token = async ({ requestCopy, collectionUid, certsAndProxyConfig: certsAndProxyConfigForRefreshUrl }) => {packages/bruno-electron/src/ipc/collection.js (1)
1261-1262: Initialize configs conditionally instead of to null for clarity.The
certsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrlvariables are initialized tonull, differing fromindex.jsandws-event-handlers.jswhich default to the main request's config. While this works because implicit grants don't use them and other grant types always compute them before use, consider initializing tocertsAndProxyConfig(like the other handlers) for consistency and to avoid the null state entirely.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/bruno-electron/src/ipc/collection.js(6 hunks)packages/bruno-electron/src/ipc/network/index.js(3 hunks)packages/bruno-electron/src/ipc/network/prepare-grpc-request.js(4 hunks)packages/bruno-electron/src/ipc/network/ws-event-handlers.js(5 hunks)packages/bruno-electron/src/utils/oauth2.js(11 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-electron/src/ipc/network/prepare-grpc-request.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-electron/src/utils/oauth2.js
🧠 Learnings (1)
📚 Learning: 2025-12-02T07:24:50.311Z
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.
Applied to files:
packages/bruno-electron/src/ipc/network/prepare-grpc-request.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-electron/src/utils/oauth2.js
🧬 Code graph analysis (4)
packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (4)
packages/bruno-electron/src/ipc/network/index.js (11)
requestCopy(162-162)requestCopy(163-163)certsAndProxyConfigForTokenUrl(166-166)certsAndProxyConfig(117-126)certsAndProxyConfig(147-147)certsAndProxyConfigForRefreshUrl(167-167)request(374-374)collection(146-146)credentials(212-212)envVars(346-346)processEnvVars(355-355)packages/bruno-electron/src/utils/oauth2.js (11)
requestCopy(135-135)requestCopy(358-358)requestCopy(485-485)certsAndProxyConfig(49-49)request(731-731)credentials(26-26)credentials(636-636)credentials(856-862)getOAuth2TokenUsingAuthorizationCode(131-292)getOAuth2TokenUsingClientCredentials(357-480)getOAuth2TokenUsingPasswordCredentials(484-629)packages/bruno-electron/src/ipc/network/interpolate-string.js (1)
interpolateString(4-58)packages/bruno-electron/src/ipc/network/cert-utils.js (2)
getCertsAndProxyConfig(12-126)collection(44-44)
packages/bruno-electron/src/ipc/collection.js (4)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (10)
interpolateVars(5-5)requestCopy(90-90)requestCopy(91-91)envVars(65-65)processEnvVars(66-66)certsAndProxyConfigForTokenUrl(94-94)certsAndProxyConfigForRefreshUrl(95-95)tokenUrlForRefresh(118-118)interpolatedRefreshUrl(120-126)refreshRequestForConfig(127-127)packages/bruno-electron/src/ipc/network/interpolate-vars.js (1)
interpolateVars(21-365)packages/bruno-electron/src/ipc/network/interpolate-string.js (1)
interpolateString(4-58)packages/bruno-electron/src/ipc/network/cert-utils.js (1)
getCertsAndProxyConfig(12-126)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
packages/bruno-electron/src/ipc/network/index.js (13)
collection(146-146)requestCopy(162-162)requestCopy(163-163)certsAndProxyConfigForTokenUrl(166-166)certsAndProxyConfig(117-126)certsAndProxyConfig(147-147)certsAndProxyConfigForRefreshUrl(167-167)interpolatedTokenUrl(170-176)request(374-374)tokenRequestForConfig(177-177)tokenUrlForRefresh(190-190)interpolatedRefreshUrl(192-198)refreshRequestForConfig(199-199)packages/bruno-electron/src/ipc/network/cert-utils.js (1)
getCertsAndProxyConfig(12-126)
packages/bruno-electron/src/utils/oauth2.js (3)
packages/bruno-electron/src/ipc/network/index.js (1)
certsAndProxyConfigForTokenUrl(166-166)packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (1)
certsAndProxyConfigForTokenUrl(75-75)packages/bruno-electron/src/ipc/network/ws-event-handlers.js (1)
certsAndProxyConfigForTokenUrl(94-94)
🔇 Additional comments (15)
packages/bruno-electron/src/utils/oauth2.js (4)
131-131: Updated function signatures to accept per-URL cert/proxy configs.The signature changes consistently add
certsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrlacross all three OAuth2 grant type functions. This aligns with the PR objective of retrieving certs/proxy based on token/refresh URLs.Also applies to: 357-357, 484-484
450-452: Good: Authorization header now requires non-empty clientSecret.This change correctly handles the case where
clientSecretis optional for client_credentials flow. The Basic Auth header is only set whenclientSecretis present and non-empty. Based on learnings, this aligns with community requests for optional clientSecret support.
51-51: Defensive initialization ofrequestDetails.Initializing
requestDetailsto{ request: {}, response: {} }ensures the object is always defined, even if an exception occurs before the try block sets it. This prevents potential undefined access issues.
111-111: Timeline now sourced fromerror?.timelinein non-response errors.For error codes without an HTTP response (e.g., network failures), the timeline is attached directly to the error object, not nested under
error.response. This is the correct path.packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
93-138: Per-URL cert/proxy config retrieval for WebSocket OAuth2 flows.The implementation correctly:
- Defaults to the passed
certsAndProxyConfig(which may be{})- Interpolates URLs before config lookup
- Handles the refresh URL fallback to access token URL
This mirrors the pattern used in
index.jsandprepare-grpc-request.js.
150-155: OAuth2 token calls updated with per-URL configs.All three grant types (
authorization_code,client_credentials,password) now pass the distinctcertsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrlparameters.Also applies to: 181-186, 212-217
packages/bruno-electron/src/ipc/network/index.js (2)
163-210: Per-URL cert/proxy config resolution inconfigureRequest.The implementation correctly:
- Extracts
refreshTokenUrlalong with other OAuth2 properties- Defaults to existing
certsAndProxyConfigfor non-implicit grant types- Uses
interpolateStringwith all variable sources includingpromptVariables- Retrieves URL-specific configs via
getCertsAndProxyConfigThe logic is consistent with the ws-event-handlers and prepare-grpc-request implementations.
216-216: OAuth2 token retrieval calls updated.All non-implicit grant types now receive
certsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrl. The implicit grant (line 230) correctly remains unchanged since it doesn't make server-to-server token requests.Also applies to: 244-244, 258-258
packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (3)
3-3: Added required imports for per-URL config resolution.New imports for
interpolateStringandgetCertsAndProxyConfigsupport the per-URL cert/proxy configuration feature.Also applies to: 8-8
72-119: Per-URL cert/proxy config for gRPC OAuth2 flows.The implementation follows the same pattern as
index.jsandws-event-handlers.js:
- Extract OAuth2 URLs from request
- Default configs to passed
certsAndProxyConfig- Interpolate URLs and fetch URL-specific configs when applicable
- Skip for implicit grant type
125-125: OAuth2 token calls updated for gRPC requests.All three grant types pass the URL-specific cert/proxy configurations consistently.
Also applies to: 139-139, 153-153
packages/bruno-electron/src/ipc/collection.js (4)
60-60: Added import forinterpolateString.Required for interpolating OAuth2 URLs before config resolution.
1264-1306: Per-URL cert/proxy config computation for credential fetch.The implementation correctly:
- Only computes configs for non-implicit grant types
- Uses
interpolateStringwith all variable sources- Creates a synthetic request object with the interpolated URL for config lookup
- Handles refresh URL fallback to access token URL
1433-1459: Per-URL cert/proxy config for refresh token handler.The refresh handler correctly:
- Derives token URL from
refreshTokenUrl || accessTokenUrl- Interpolates the URL before config lookup
- Passes the computed config to
refreshOauth2TokenThis is consistent with how refresh configs are handled elsewhere in the PR.
1318-1324: OAuth2 token calls updated with per-URL configs.All grant types (
authorization_code,client_credentials,password) now receive URL-specific cert/proxy configurations. The implicit grant (lines 1346-1352) correctly doesn't receive these configs.Also applies to: 1328-1334, 1338-1344
3b52481 to
20a2d08
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/bruno-electron/src/ipc/collection.js (1)
1419-1460: Guard against nullcertsAndProxyConfigForRefreshUrlwhen refreshing OAuth2 credentialsIn
renderer:refresh-oauth2-credentialsyou:
- Derive
tokenUrl = refreshTokenUrl || accessTokenUrl- Only call
getCertsAndProxyConfigwhentokenUrlis truthy- Initialize
certsAndProxyConfigForRefreshUrltonulland pass it directly torefreshOauth2TokenInside
refreshOauth2Token, the call togetCredentialsFromTokenUrlexpects a non-nullcertsAndProxyConfigand immediately destructures it. IftokenUrlis falsy but there is still a storedrefresh_token(e.g. config changed after caching credentials), this will lead to a runtime error whencertsAndProxyConfigForRefreshUrlisnull.I recommend hardening this in tandem with
getCredentialsFromTokenUrlby ensuring it always receives an object, for example:
- In
oauth2.js, treatcertsAndProxyConfigdefensively:-const getCredentialsFromTokenUrl = async ({ requestConfig, certsAndProxyConfig }) => { - const { proxyMode, proxyConfig, httpsAgentRequestFields, interpolationOptions } = certsAndProxyConfig; +const getCredentialsFromTokenUrl = async ({ requestConfig, certsAndProxyConfig }) => { + const safeConfig = certsAndProxyConfig || {}; + const { proxyMode, proxyConfig, httpsAgentRequestFields, interpolationOptions } = safeConfig;This keeps
renderer:refresh-oauth2-credentialssafe even iftokenUrlis missing or future callers forget to supply a config object.packages/bruno-electron/src/utils/oauth2.js (3)
48-127: Defensively handlecertsAndProxyConfigingetCredentialsFromTokenUrl
getCredentialsFromTokenUrlassumescertsAndProxyConfigis a non-null object:const { proxyMode, proxyConfig, httpsAgentRequestFields, interpolationOptions } = certsAndProxyConfig;However, some callers (e.g. the new
renderer:refresh-oauth2-credentialshandler) can passnullwhen no token/refresh URL is available, which will throw at destructuring time.To make this robust and avoid hard crashes on edge cases (e.g. cached credentials but missing URLs), treat the config defensively:
-const getCredentialsFromTokenUrl = async ({ requestConfig, certsAndProxyConfig }) => { - const { proxyMode, proxyConfig, httpsAgentRequestFields, interpolationOptions } = certsAndProxyConfig; +const getCredentialsFromTokenUrl = async ({ requestConfig, certsAndProxyConfig }) => { + const safeConfig = certsAndProxyConfig || {}; + const { proxyMode, proxyConfig, httpsAgentRequestFields, interpolationOptions } = safeConfig;This preserves normal behavior when a full config is passed, while falling back to axios/makeAxiosInstance defaults when it is not.
443-480: Basic auth header in client credentials flow should not require a non-empty clientSecretFor the client credentials flow you now have:
if (credentialsPlacement === 'basic_auth_header' && clientSecret && clientSecret.trim() !== '') { axiosRequestConfig.headers['Authorization'] = `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`; }With
credentialsPlacement === 'basic_auth_header'and an empty/missing secret, this sends no client credentials at all (neither header nor body), even though we want to support public clients with onlyclientId. Per the retrieved learning, Basic headers should tolerate missing secrets by usingclientSecret || ''. Based on learnings, this should be aligned withoauth2-helper.ts.A safer pattern:
- if (credentialsPlacement === 'basic_auth_header' && clientSecret && clientSecret.trim() !== '') { - axiosRequestConfig.headers['Authorization'] = `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`; - } + if (credentialsPlacement === 'basic_auth_header') { + const safeSecret = clientSecret || ''; + axiosRequestConfig.headers['Authorization'] = `Basic ${Buffer.from(`${clientId}:${safeSecret}`).toString('base64')}`; + }Same applies to the password flow below.
484-629: Password flow: mirror Basic auth behavior and per-URL config usageThe password grant flow mirrors client credentials:
- It correctly accepts and uses
certsAndProxyConfigForTokenUrl/certsAndProxyConfigForRefreshUrl.- It calls
refreshOauth2Tokenwith the refresh config when auto-refreshing.- It uses
getCredentialsFromTokenUrlwith the token config when fetching a new token.However, the Basic auth header has the same issue as the client credentials flow:
if (credentialsPlacement === 'basic_auth_header' && clientSecret && clientSecret.trim() !== '') { axiosRequestConfig.headers['Authorization'] = `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`; }To support clients with an empty/omitted secret, please apply the same
clientSecret || ''pattern here as in the previous comment.Based on learnings, aligning Basic auth behavior for all grant types will avoid regressions for clientId-only configurations and keeps this module consistent with
oauth2-helper.ts.
🧹 Nitpick comments (4)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
65-85: Consider propagatingpromptVariablesthrough WebSocket interpolation as wellYou now pull
promptVariablesfrom the collection and use them for token/refresh URL interpolation, butinterpolateVarsis still called without the 5thpromptVariablesargument (both forrequestCopyin the OAuth2 switch and forwsRequestat the end). If you want WS OAuth2 and message interpolation to behave like the HTTP path, consider:
- Passing
promptVariablesintointerpolateVars(requestCopy, envVars, runtimeVariables, processEnvVars, promptVariables)in each grant-type case- Passing
promptVariablesinto the finalinterpolateVars(wsRequest, envVars, runtimeVariables, processEnvVars, promptVariables)This keeps prompt-variable handling consistent across HTTP and WS flows.
89-138: Per-token/refresh certs & proxy config wiring for WS OAuth2 looks correctThe new logic that:
- Extracts
grantType,accessTokenUrl,refreshTokenUrlfromrequestCopy.oauth2- Interpolates the token/refresh URLs with
globalEnvironmentVariables,envVars,runtimeVariables,processEnvVars, andpromptVariables- Builds synthetic requests with
urlpointing at the OAuth2 endpoint- Calls
getCertsAndProxyConfigto derivecertsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrlis consistent with the HTTP implementation in
network/index.jsand preserves implicit-flow behavior (no per-URL config). Passing these configs into the OAuth2 helpers below is also aligned with the updated signatures.No correctness issues spotted here.
If you want to avoid duplicate work when
refreshTokenUrlequalsaccessTokenUrl, you could reusecertsAndProxyConfigForTokenUrlinstead of callinggetCertsAndProxyConfiga second time for the refresh case.packages/bruno-electron/src/ipc/network/index.js (1)
161-210: HTTP OAuth2 per-token/refresh cert/proxy resolution is wired correctlyThe new block that:
- Extracts OAuth2 fields (including
accessTokenUrlandrefreshTokenUrl)- Computes
certsAndProxyConfigForTokenUrl/certsAndProxyConfigForRefreshUrlviainterpolateString+getCertsAndProxyConfig- Uses
tokenUrlForRefresh = refreshTokenUrl || accessTokenUrl- Skips this work for the implicit grant
is consistent with
cert-utils.jsand mirrors the collection/WS flows. PassingpromptVariablesinto the interpolation context keeps behavior aligned with the rest of the request pipeline.This section looks solid.
You could avoid a second
getCertsAndProxyConfigcall whentokenUrlForRefresh === accessTokenUrlby reusingcertsAndProxyConfigForTokenUrl.packages/bruno-electron/src/ipc/collection.js (1)
1245-1363: Align prompt-variable interpolation and per-URL certs/proxy forfetch-oauth2-credentialsThe new logic in
renderer:fetch-oauth2-credentialscorrectly:
- Pulls
promptVariablesfrom the collection- Interpolates
accessTokenUrl/refreshTokenUrlwith env + runtime + process + prompt variables- Computes
certsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrlviagetCertsAndProxyConfigon synthetic requests pointing at the OAuth2 endpoints- Passes these configs into the OAuth2 helper functions
Two improvements to keep behavior in sync with the HTTP/WS paths:
interpolateVarsis still called withoutpromptVariables(both before the grant-type switch and inside each case). To get full prompt-variable support for OAuth2 parameters when fetching credentials from the UI, consider:- interpolateVars(requestCopy, envVars, runtimeVariables, processEnvVars); + interpolateVars(requestCopy, envVars, runtimeVariables, processEnvVars, promptVariables);
- As with the HTTP path, you could skip recomputing
certsAndProxyConfigForRefreshUrlwhentokenUrlForRefresh === accessTokenUrlby reusingcertsAndProxyConfigForTokenUrl.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/bruno-electron/src/ipc/collection.js(6 hunks)packages/bruno-electron/src/ipc/network/index.js(3 hunks)packages/bruno-electron/src/ipc/network/ws-event-handlers.js(5 hunks)packages/bruno-electron/src/utils/oauth2.js(11 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-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-electron/src/utils/oauth2.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/collection.js
🧠 Learnings (1)
📚 Learning: 2025-12-02T07:24:50.311Z
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.
Applied to files:
packages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-electron/src/utils/oauth2.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/collection.js
🧬 Code graph analysis (2)
packages/bruno-electron/src/ipc/network/index.js (5)
packages/bruno-electron/src/utils/oauth2.js (13)
requestCopy(135-135)requestCopy(358-358)requestCopy(485-485)certsAndProxyConfig(49-49)credentials(26-26)credentials(636-636)credentials(856-862)debugInfo(471-471)debugInfo(620-620)debugInfo(667-667)getOAuth2TokenUsingAuthorizationCode(131-292)getOAuth2TokenUsingClientCredentials(357-480)getOAuth2TokenUsingPasswordCredentials(484-629)packages/bruno-electron/src/ipc/network/prepare-request.js (2)
grantType(75-75)grantType(192-192)packages/bruno-electron/src/ipc/network/interpolate-string.js (1)
interpolateString(4-58)packages/bruno-electron/src/ipc/network/cert-utils.js (1)
getCertsAndProxyConfig(12-126)packages/bruno-electron/src/ipc/network/interpolate-vars.js (1)
interpolateVars(21-365)
packages/bruno-electron/src/ipc/collection.js (6)
packages/bruno-electron/src/ipc/network/index.js (18)
require(9-9)require(10-10)require(11-11)require(12-12)require(13-13)require(14-14)require(15-15)require(16-16)require(17-17)require(19-19)require(21-21)require(22-22)require(23-23)require(24-24)require(25-25)interpolateVars(20-20)envVars(346-346)processEnvVars(355-355)packages/bruno-electron/src/ipc/network/interpolate-vars.js (1)
interpolateVars(21-365)packages/bruno-app/src/components/CollectionSettings/Auth/OAuth2/index.js (1)
grantType(21-21)packages/bruno-app/src/components/RequestPane/Auth/OAuth2/index.js (1)
grantType(21-21)packages/bruno-electron/src/ipc/network/interpolate-string.js (1)
interpolateString(4-58)packages/bruno-electron/src/ipc/network/cert-utils.js (1)
getCertsAndProxyConfig(12-126)
⏰ 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 - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
🔇 Additional comments (5)
packages/bruno-electron/src/ipc/network/index.js (1)
213-270: Good: OAuth2 flows now receive per-URL cert/proxy configs and promptVariablesIn each non-implicit grant case you:
- Re-run
interpolateVars(requestCopy, envVars, runtimeVariables, processEnvVars, promptVariables)- Pass
certsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrlinto the respective OAuth2 helpers- Only set Authorization header when
tokenPlacement === 'header' && credentials?.access_token, trimming the final valueThis aligns HTTP execution with the updated OAuth2 utilities and avoids sending
Authorization: Bearer undefinedwhen the token fetch fails.No issues here.
packages/bruno-electron/src/ipc/collection.js (1)
59-65: NewinterpolateStringimport is used appropriately for OAuth2 token URLsThe added
interpolateStringimport is used later in the OAuth2 handlers to interpolate access/refresh token URLs before computing cert/proxy config, which is consistent with howinterpolateStringis used in the network layer.No issues here.
packages/bruno-electron/src/utils/oauth2.js (3)
243-279: Authorization Code flow: per-token cert/proxy config usage looks correctIn
getOAuth2TokenUsingAuthorizationCode, the changes to:
- Accept
certsAndProxyConfigForTokenUrl/certsAndProxyConfigForRefreshUrl- Use
certsAndProxyConfigForRefreshUrlwhen auto-refreshing viarefreshOauth2Token- Use
certsAndProxyConfigForTokenUrlwhen fetching a new token viagetCredentialsFromTokenUrlare consistent with the intended per-URL behavior. Debug info enrichment and credential persistence still work as before.
No functional issues here.
631-681: Refresh flow correctly delegates per-URL cert/proxy config
refreshOauth2Tokennow delegates token refresh HTTP calls togetCredentialsFromTokenUrlusing the injectedcertsAndProxyConfig. Combined with the earlier suggestion to defensively handlecertsAndProxyConfiginsidegetCredentialsFromTokenUrl, this cleanly centralizes all TLS/proxy handling for refresh operations.The rest of the refresh logic (cache lookup, credential clearing on failure, persistence on success) remains unchanged.
Looks good once the defensive config handling is added.
131-292: Ensure all call sites pass the new cert/proxy config parametersThe signatures for the OAuth2 helpers now include per-URL cert/proxy configs:
getOAuth2TokenUsingAuthorizationCode({ request, collectionUid, forceFetch, certsAndProxyConfigForTokenUrl, certsAndProxyConfigForRefreshUrl })getOAuth2TokenUsingClientCredentials({ ...same })getOAuth2TokenUsingPasswordCredentials({ ...same })You’ve updated the major call sites in
network/index.js,ws-event-handlers.js, andcollection.js, but any missed call will now fail at runtime whengetCredentialsFromTokenUrldestructurescertsAndProxyConfig.To be safe, please verify all usages with something like:
#!/bin/bash # Verify all call sites of the updated OAuth2 helpers pass the new config arguments rg -n "getOAuth2TokenUsingAuthorizationCode\(" . rg -n "getOAuth2TokenUsingClientCredentials\(" . rg -n "getOAuth2TokenUsingPasswordCredentials\(" .This should show that every call provides both
certsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrl.
20a2d08 to
9945466
Compare
…s instead of resource url
9945466 to
8289bf3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (1)
93-144: Per-URL cert/proxy config logic is consistent with other files.The interpolation and configuration retrieval pattern aligns well with
index.jsandprepare-grpc-request.js. The fallback to the defaultcertsAndProxyConfigwhen conditions aren't met is appropriate.One consideration: this block (lines 93-144) is nearly identical across three files. If this pattern grows or changes frequently, consider extracting it into a shared helper to reduce maintenance burden.
packages/bruno-electron/src/ipc/network/index.js (1)
163-216: Per-URL cert/proxy configuration logic is correct.The interpolation and per-URL configuration retrieval are properly implemented. The fallback to the default
certsAndProxyConfigwhenaccessTokenUrlis not present or grant type isimplicitis appropriate.This block is duplicated in
ws-event-handlers.jsandprepare-grpc-request.js. Consider extracting this into a shared utility if the pattern needs to be maintained across multiple locations.packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (1)
35-90: Per-URL cert/proxy config logic is consistent.Same pattern as
index.jsandws-event-handlers.js. The implementation correctly adapts to the gRPC context by extractingcollectionUidandcollectionPathfrom the collection object.Consider extracting the shared interpolation and config retrieval logic into a reusable helper function to reduce code duplication across the three files.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/bruno-electron/src/ipc/collection.js(6 hunks)packages/bruno-electron/src/ipc/network/index.js(3 hunks)packages/bruno-electron/src/ipc/network/prepare-grpc-request.js(2 hunks)packages/bruno-electron/src/ipc/network/ws-event-handlers.js(5 hunks)packages/bruno-electron/src/utils/oauth2.js(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-electron/src/ipc/collection.js
🧰 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-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/network/prepare-grpc-request.jspackages/bruno-electron/src/utils/oauth2.js
🧠 Learnings (1)
📚 Learning: 2025-12-02T07:24:50.311Z
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.
Applied to files:
packages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/network/prepare-grpc-request.jspackages/bruno-electron/src/utils/oauth2.js
🧬 Code graph analysis (3)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
packages/bruno-electron/src/ipc/network/index.js (4)
collection(146-146)requestCopy(162-162)requestCopy(163-163)request(380-380)packages/bruno-electron/src/ipc/network/cert-utils.js (1)
getCertsAndProxyConfig(12-126)
packages/bruno-electron/src/ipc/network/index.js (6)
packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (8)
requestCopy(34-34)requestCopy(36-36)certsAndProxyConfigForTokenUrl(40-40)certsAndProxyConfigForRefreshUrl(41-41)credentials(37-37)interpolateVars(2-2)envVars(146-146)processEnvVars(145-145)packages/bruno-electron/src/utils/oauth2.js (11)
requestCopy(135-135)requestCopy(358-358)requestCopy(485-485)certsAndProxyConfig(49-49)credentials(26-26)credentials(636-636)credentials(856-862)debugInfo(471-471)debugInfo(620-620)debugInfo(667-667)getOAuth2TokenUsingAuthorizationCode(131-292)packages/bruno-electron/src/ipc/network/interpolate-string.js (1)
interpolateString(4-58)packages/bruno-electron/src/ipc/network/cert-utils.js (1)
getCertsAndProxyConfig(12-126)packages/bruno-electron/src/ipc/network/interpolate-vars.js (1)
interpolateVars(21-365)packages/bruno-cli/src/runner/interpolate-vars.js (1)
interpolateVars(16-272)
packages/bruno-electron/src/utils/oauth2.js (3)
packages/bruno-electron/src/ipc/network/index.js (2)
certsAndProxyConfigForRefreshUrl(167-167)certsAndProxyConfigForTokenUrl(166-166)packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (2)
certsAndProxyConfigForRefreshUrl(41-41)certsAndProxyConfigForTokenUrl(40-40)packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
certsAndProxyConfigForRefreshUrl(95-95)certsAndProxyConfigForTokenUrl(94-94)
🔇 Additional comments (21)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (3)
159-161: LGTM!Correctly passes the per-URL configurations to
getOAuth2TokenUsingAuthorizationCode.
190-192: LGTM!Correctly passes the per-URL configurations to
getOAuth2TokenUsingClientCredentials.
221-223: LGTM!Correctly passes the per-URL configurations to
getOAuth2TokenUsingPasswordCredentials.packages/bruno-electron/src/ipc/network/index.js (3)
222-222: LGTM!Single-line destructuring with per-URL configs for authorization_code flow.
250-250: LGTM!Correctly passes per-URL configs for client_credentials flow.
264-264: LGTM!Correctly passes per-URL configs for password flow.
packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (4)
7-8: LGTM!Required imports added for per-URL configuration handling.
96-96: LGTM!Correctly passes per-URL configs for authorization_code flow.
102-102: LGTM!Correctly passes per-URL configs for client_credentials flow.
108-108: LGTM!Correctly passes per-URL configs for password flow.
packages/bruno-electron/src/utils/oauth2.js (11)
51-51: Good defensive initialization.Initializing
requestDetailswith default structures ensures the debug info object always has valid structure, even if the request fails before populating it.
111-111: Verify timeline propagation change.Changed from
error?.response?.timelinetoerror?.timeline. This affects how timeline data is captured for non-response errors (e.g., network errors with an error code but no response).Ensure this change is intentional and that
error.timelineis populated in the axios error scenarios whereerror.responseis not available.
202-202: LGTM!Correctly uses
certsAndProxyConfigForRefreshUrlfor token refresh operations.
277-277: LGTM!Correctly uses
certsAndProxyConfigForTokenUrlfor token URL requests.
357-357: LGTM!
getOAuth2TokenUsingClientCredentialssignature updated consistently.
405-405: LGTM!Correctly uses
certsAndProxyConfigForRefreshUrlfor token refresh in client_credentials flow.
473-473: LGTM!Correctly uses
certsAndProxyConfigForTokenUrlfor token URL requests in client_credentials flow.
484-484: LGTM!
getOAuth2TokenUsingPasswordCredentialssignature updated consistently.
551-551: LGTM!Correctly uses
certsAndProxyConfigForRefreshUrlfor token refresh in password flow.
622-622: LGTM!Correctly uses
certsAndProxyConfigForTokenUrlfor token URL requests in password flow.
131-131: Function signature updated for per-URL configs.
getOAuth2TokenUsingAuthorizationCodenow acceptscertsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrlinstead of a singlecertsAndProxyConfig. All call sites have been properly updated.
There was a problem hiding this comment.
Actionable comments posted: 0
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/utils/oauth2.js (1)
256-258: Inconsistent Authorization header logic across grant types.In
getOAuth2TokenUsingAuthorizationCode, the Authorization header is set without verifying thatclientSecretexists and is non-empty:if (credentialsPlacement === 'basic_auth_header') { axiosRequestConfig.headers['Authorization'] = `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`; }However, in
getOAuth2TokenUsingClientCredentials(line 461) andgetOAuth2TokenUsingPasswordCredentials(line 608), the check includesclientSecretvalidation:if (credentialsPlacement === 'basic_auth_header' && clientSecret && clientSecret.trim() !== '') {Based on learnings, clientSecret is optional. When
clientSecretis empty/undefined withbasic_auth_headerplacement, the Authorization header would encodeclientId:undefinedorclientId:, which could cause authentication issues.Apply this diff to align with the other grant types:
- if (credentialsPlacement === 'basic_auth_header') { + if (credentialsPlacement === 'basic_auth_header' && clientSecret && clientSecret.trim() !== '') { axiosRequestConfig.headers['Authorization'] = `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`; }Based on learnings, clientSecret handling should use
clientSecret || ''pattern.
🧹 Nitpick comments (1)
packages/bruno-electron/src/ipc/collection.js (1)
1262-1313: Consider extracting per-URL cert/proxy config logic to a shared utility.The logic for computing
certsAndProxyConfigForTokenUrlandcertsAndProxyConfigForRefreshUrlis duplicated across multiple files (ws-event-handlers.js, collection.js, and likely others). This creates maintainability concerns and increases the risk of inconsistencies.Consider extracting this logic into a shared utility function, e.g.,
computeOAuth2CertsAndProxyConfigs:// utils/oauth2-cert-utils.js const computeOAuth2CertsAndProxyConfigs = async ({ oauth2, certsAndProxyConfig, collectionUid, collection, envVars, runtimeVariables, processEnvVars, collectionPath, globalEnvironmentVariables, collectionVariables, folderVariables, requestVariables, promptVariables }) => { const { grantType, accessTokenUrl, refreshTokenUrl } = oauth2; let certsAndProxyConfigForTokenUrl = certsAndProxyConfig; let certsAndProxyConfigForRefreshUrl = certsAndProxyConfig; if (accessTokenUrl && grantType !== 'implicit') { // ... token URL interpolation and config logic } const tokenUrlForRefresh = refreshTokenUrl || accessTokenUrl; if (tokenUrlForRefresh && grantType !== 'implicit') { // ... refresh URL interpolation and config logic } return { certsAndProxyConfigForTokenUrl, certsAndProxyConfigForRefreshUrl }; };Then use it in all three locations (ws-event-handlers.js, collection.js, prepare-grpc-request.js).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bruno-electron/src/ipc/collection.js(5 hunks)packages/bruno-electron/src/ipc/network/ws-event-handlers.js(5 hunks)packages/bruno-electron/src/utils/oauth2.js(11 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-electron/src/utils/oauth2.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/ipc/network/ws-event-handlers.js
🧠 Learnings (1)
📚 Learning: 2025-12-02T07:24:50.311Z
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.
Applied to files:
packages/bruno-electron/src/utils/oauth2.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/ipc/network/ws-event-handlers.js
🧬 Code graph analysis (1)
packages/bruno-electron/src/utils/oauth2.js (4)
packages/bruno-electron/src/utils/oauth2-protocol-handler.js (1)
error(70-70)packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
certsAndProxyConfigForRefreshUrl(95-95)certsAndProxyConfigForTokenUrl(94-94)packages/bruno-electron/src/ipc/network/prepare-grpc-request.js (2)
certsAndProxyConfigForRefreshUrl(41-41)certsAndProxyConfigForTokenUrl(40-40)packages/bruno-electron/src/ipc/network/index.js (2)
certsAndProxyConfigForRefreshUrl(167-167)certsAndProxyConfigForTokenUrl(166-166)
⏰ 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: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
certs and proxy config based on oauth2 token and refresh urls instead of resource url bru-2005certs and proxy config based on oauth2 token and refresh urls instead of resource url
…s instead of resource url (usebruno#6164)
jira
Description
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
New Features
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.