fix: OAuth2 Basic Auth header encoding for special characters#6263
Conversation
WalkthroughTwo OAuth2 utility modules are updated to remove URL-encoding of client credentials before Base64 encoding in Basic Authentication headers. The clientId and clientSecret are now directly Base64 encoded across multiple OAuth2 authorization flows (authorizationCode, clientCredentials, passwordCredentials, refreshToken, and password grant paths). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-electron/src/utils/oauth2.js(4 hunks)packages/bruno-requests/src/auth/oauth2-helper.ts(2 hunks)
⏰ 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: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (4)
packages/bruno-requests/src/auth/oauth2-helper.ts (2)
148-148: Encoding fix correctly implements RFC 2617 Basic Authentication.Removing
encodeURIComponentbefore Base64 encoding is the correct implementation. Per RFC 2617, Basic Authentication credentials should be formatted asbase64(clientId:clientSecret)without URL encoding.
148-148: Add validation for clientSecret when using basic_auth_header credentials placement.The code uses a non-null assertion (
clientSecret!) without validating thatclientSecretis defined. SinceOAuth2ConfigdeclaresclientSecretas optional, this could cause a runtime error. Add an explicit check before constructing the Authorization header, or verify that all callers guaranteeclientSecretis defined whencredentialsPlacementis'basic_auth_header'.packages/bruno-electron/src/utils/oauth2.js (2)
453-455: Good validation pattern for clientSecret.This flow correctly validates that
clientSecretexists and is not empty before constructing the Basic Auth header. The encoding fix is correct per RFC 2617.
600-602: Proper validation and correct encoding.Like the client credentials flow, this correctly validates
clientSecretbefore use. The encoding fix aligns with RFC 2617 Basic Authentication standards.
|
Unit tests to be added in the PR #6186 related to current changes |
BRU-2240
Fixes: #6194
Removed URI encoding of client ID and secret before Base64 encoding. The credentials are now Base64-encoded directly as
clientId:clientSecret.