Repository navigation
Add PKCE support and remove clientSecret from OIDC client - #6
Conversation
Implement PKCE (Proof Key for Code Exchange, RFC 7636) for enhanced security in OIDC Authorization Code flow. PKCE is now automatically enabled using S256 method for all authentication requests. Also remove clientSecret parameter as this client is designed for Public Clients (SPAs, Native Apps) where client secrets cannot be securely stored. The focus on Public Clients aligns with the testing use case and modern OAuth 2.0 Security Best Current Practice. Additionally fix OIDC spec compliance by adding client_id to Authorization Request, which was missing in the previous implementation. Changes: - Add automatic PKCE (S256) code_challenge/code_verifier generation - Remove clientSecret from OidcClientConfig (breaking change) - Add client_id to Authorization Request (bug fix) - Update documentation to recommend issuer-based Discovery - Add "Manual Endpoint Configuration" section for legacy providers BREAKING CHANGE: clientSecret parameter removed from OidcClientConfig
There was a problem hiding this comment.
Pull request overview
This pull request enhances the OIDC HTTP client by adding PKCE (Proof Key for Code Exchange) support for the Authorization Code flow, making it more suitable for Public Clients (SPAs, Native Apps) while removing the unnecessary clientSecret parameter. The changes improve OIDC spec compliance by including client_id in authorization requests and update documentation to recommend issuer-based Discovery over manual endpoint configuration.
Key Changes:
- Automatic PKCE (S256 method) support added to protect against authorization code interception attacks
- Removed
clientSecretparameter fromOidcClientConfiginterface (breaking change) - Fixed spec compliance by adding
client_idparameter to authorization requests
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return btoa(String.fromCharCode(...array)) | ||
| .replace(/\+/g, "-") | ||
| .replace(/\//g, "_") | ||
| .replace(/=/g, ""); |
There was a problem hiding this comment.
The base64url encoding implementation using btoa(String.fromCharCode(...array)) may not correctly handle byte values greater than 127 in all JavaScript environments. While this might work in Deno's runtime, it could cause issues with certain random byte sequences.
Consider using a more robust approach that properly handles all byte values, such as iterating through the array and building the string character by character, or using a dedicated base64url encoding function.
| return btoa(String.fromCharCode(...new Uint8Array(hash))) | ||
| .replace(/\+/g, "-") | ||
| .replace(/\//g, "_") | ||
| .replace(/=/g, ""); |
There was a problem hiding this comment.
The base64url encoding implementation using btoa(String.fromCharCode(...new Uint8Array(hash))) may not correctly handle byte values greater than 127 in all JavaScript environments. While this might work in Deno's runtime, it could cause issues with certain hash output byte sequences.
Consider using a more robust approach that properly handles all byte values, such as iterating through the array and building the string character by character, or using a dedicated base64url encoding function.
| // Step 1: Generate state and PKCE parameters | ||
| const state = crypto.randomUUID(); | ||
| const codeVerifier = generateCodeVerifier(); | ||
| const codeChallenge = await generateCodeChallenge(codeVerifier); |
There was a problem hiding this comment.
The newly added PKCE functionality is not covered by unit tests. The existing tests don't verify that:
code_challengeandcode_challenge_methodparameters are included in the authorization requestcode_verifierparameter is included in the token exchange request- The code challenge correctly corresponds to the code verifier (S256 method)
Consider adding test assertions to verify these PKCE parameters are present in the requests.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
clientSecretparameter (breaking change)client_idto Authorization RequestWhy
The OIDC HTTP client is designed for testing scenarios where it simulates end-user login flows. Supporting
clientSecretwas unnecessary since:PKCE (RFC 7636) is now automatically enabled using the S256 method, providing protection against authorization code interception attacks without requiring shared secrets.
The missing
client_idin Authorization Request was also a spec compliance bug that prevented proper operation with strict OIDC providers.Test Plan