IdentityPython/idpy-oidc

Do not urlencode credentials

viincenb opened this issue · 10 comments

viincenb commented

Hi, I'm implementing an oidc client and I noticed that the client_secret is sent like this

# l.120 oidcrp.client_auth.py
credentials = "{}:{}".format(quote_plus(user), quote_plus(passwd))

But the server does not seem to decode the secret as it should and I got an invalid_client.

Is there a way to avoid the quote_plus in order to send the raw secret ?

Reference

rohe commented

The short answer: no!

According to the HTTP Basic authentication scheme the credentials is expected to be base64 encoded.

I would say that if the server isn't able to decode the credentials properly then it should be fixed.
I know this is problematic if the server belongs to someone else.
But in that case they are not adhering to the standard and should be told so.

c00kiemon5ter commented

some suggestions:

  • you may want to try client_secret_post instead of client_secret_basic
  • you may try with a client_id/secret that is not changed when url-encoded
viincenb commented

My problem is from the url-encode thing, not the base64 defined in the RFC 2617. My secret has specials characters and the quote_plus changes the value of the secret.

you may want to try client_secret_post instead of client_secret_basic

I got the same error

you may try with a client_id/secret that is not changed when url-encoded

I thought about it, but the secret is not mine so I will have to ask for an other secret and I don't know if I can have a "clean" one

rohe commented

Are you telling me that if you start with a secret, base64 encode it then base64 decode it the resulting string will be different from the input ?
I don't really get what the problem is.

viincenb commented

What I am saying is slightly different: the lib performs a quote_plus just before the base64 encode (l.120 oidcrp.client_auth.py), this step changes the value of my secret because it has some special characters (+ becomes %2B).

rohe commented

Ah, now I get it. Sorry about being so dense :-)
So, why are we using quote_plus at all ?
It seems a bit counterproductive.
It works perfectly well as long as you don't have one of the reserved characters (https://datatracker.ietf.org/doc/html/rfc3986.html#section-2.2) in the user name or the secret.
But as you have found out it breaks things if you do.
I'm leaning toward removing quote_plus() completely.
Will check with the rest of the gang.

rohe commented

Fixed in release 1.2.1

viincenb commented

Thank you for this quick fix, but is the urlsafe_b64encode mandatory too (l.126 client.client_auth.py) ? My base64 encoded Bearer has a / which is replaced by a _ now, it is corrupting my credentials.

rohe commented

Fixed in release 1.2.2

viincenb commented

My credentials are successfully sent now, thank you guys for the help 🙏

Have a nice day