-
Notifications
You must be signed in to change notification settings - Fork 393
Open
Description
> I've submitted a fix for this issue in PR #466
Description
The current implementation of Basic Authentication header construction in OpenIDConnectClient incorrectly URL-encodes the client credentials before base64 encoding them. This causes issues when client_secret contains special characters (like '/'), as they get double-encoded. This violates both RFC 2617 Section 2 and RFC 7617 specifications for Basic Authentication.
Current Implementation
'Authorization: Basic ' . base64_encode(urlencode($clientId) . ':' . urlencode($clientSecret))
This implementation appears in multiple places in the codebase, including:
- requestTokens() method
- requestResourceOwnerToken() method
- requestTokenExchange() method
- refreshToken() method
- introspectToken method
- revokeToken() method
Related old Issues
- Url encoding of client_id and client_secret in fetching access token Url encoding of client_id and client_secret in fetching access token #248
Problem
- The current implementation URL-encodes the credentials before base64 encoding
- This contradicts both:
- RFC 2617 Section 2: "the client sends the userid and password, separated by a single colon (":") character, within a base64 encoded string"
- RFC 7617 (which supersedes RFC 2617 Section 2) maintaining the same encoding requirements - When client_secret contains special characters (e.g., '/'), they get URL-encoded (e.g., '%2F') before base64 encoding, resulting in incorrect authentication headers
The correct format should be:
base64-encoded(userid:password)
Expected Behavior
According to RFC 2617 Section 2, the Basic Authentication header should be constructed by:
- Combining the credentials with a colon separator
- Applying base64 encoding to the combined string
- NO URL encoding should be performed on the credentials
Correct implementation:
'Authorization: Basic ' . base64_encode($clientId . ':' . $clientSecret)
Example of the Issue
When client_secret = "my/secret":
- Current output: Authorization: Basic {base64(urlencode("client_id:my%2Fsecret"))}
- Expected output: Authorization: Basic {base64("client_id:my/secret")}
Impact
- Authentication failures with OAuth 2.0/OpenID Connect providers that expect exact client_secret values
- Incompatibility with providers that require special characters in client_secret
- Non-compliance with RFC 2617 Section 2 and RFC 7617 specifications
Proposed Solution
Remove the urlencode() calls from the Basic Authentication header construction:
$headers = ['Authorization: Basic ' . base64_encode($this->clientID . ':' . $this->clientSecret)];
Additional Information
- PHP Version: [8.2]
- OpenID-Connect-PHP Version: [v1.0.2]
- Related RFCs:
ricklambrechts and ovsep404
Metadata
Metadata
Assignees
Labels
No labels