istio-ecosystem / authservice

Move OIDC token acquisition out of your app code and into the Istio mesh
Apache License 2.0
220 stars 62 forks source link

Support `:` in client ID #263

Closed mjnagel closed 1 month ago

mjnagel commented 3 months ago

Currently authservice chains do not work if the client id contains a :. This is due to the usage of the basic auth header and limitations on the user id not allowing : for basic auth.

I'm not familiar with the alternative options here but I suspect switching from basic auth to would allow this to be supported. Alternatively this should be documented as a limitation with a clear error in the logs/when the config is read in. In the current state I see a failure on the token exchange with my IDP and Invalid client or Invalid client credentials in Authservice's logs. On the IDP side I am able to see the request come in with only part of my client id (it is trimmed at the first :).

This should be easy to reproduce by using a client id with a URI, such as http://example.com.

nacx commented 1 month ago

according to the spec clients may authenticate using basic auth or sending the client id and secret as body parameters, but the latter is not recommended, so I think the best thing is to validate the configuration and return a proper error message.