Update handlers.go #3302
Update handlers.go #3302
Conversation
Compare secrets just on not public clients Signed-off-by: Miguel Martinez <martinezcanteromiguel@gmail.com>
| @@ -813,7 +813,7 @@ func (s *Server) withClientFromStorage(w http.ResponseWriter, r *http.Request, h | |||
| return | |||
| } | |||
|
|
|||
| if subtle.ConstantTimeCompare([]byte(client.Secret), []byte(clientSecret)) != 1 { | |||
| if !client.Public && subtle.ConstantTimeCompare([]byte(client.Secret), []byte(clientSecret)) != 1 { | |||
There was a problem hiding this comment.
Could you please elaborate more on the problem?
If I get it right, there is a clientSecret in the public client config, and Dex tries to validate it.
So, the solution to the problem is to remove the clientSecret from the config. Why do you want to change the default behavior?
I'd tried to find that validation MUST be skipped for public clients, but didn't manage to find the right RFC. If you had a link, it would also be cool.
There was a problem hiding this comment.
Proposed Changes:
Do not validate the client_secret for public clients in the public OAuth flow. Clarify that the client relies on credentials stored in the storage and does not use explicit client credentials in requests. The client_secret is exclusively used for communication between the OIDC server and the custom conector with API.
Justification:
In situations involving public clients, the validation of the client_secret poses challenges as the confidentiality of the secret cannot be guaranteed. Public clients are intentionally designed to operate without explicit client credentials in their requests. Instead, they rely on credentials securely stored in the storage. The client_secret is reserved for secure communication solely between the OIDC server and the API. Validating the client_secret for public clients contradicts established security best practices outlined in OAuth 2.0 and OpenID Connect. This change ensures that the server does not attempt to validate a non-confidential secret in cases where explicit client credentials are not utilized.
References:
- RFC 6749 - Section 2.1.1 and Section 2.3.1
- OAuth 2.0 Security Best Practices
Compare secrets just on not public clients
Overview
Removed client.Secret verification for public clients, maintaining the same behavior for private clients.
This PR addresses a specific need in our custom connector. Our custom connector relies on the secret as an authentication header. In the case of public clients, this header is also necessary for proper authentication. With this small change, the connector now functions seamlessly for both public and private clients. It's worth noting that this adjustment doesn't appear to impact code coverage or the expected behavior of the system.
Special notes for your reviewer