Access to OIDC Token endpoint don't fully respect OAuth Specifications

[grrolland]

Hello,

The Basic Authentification for the OIDC Token Endpoint don't fully respect the OAuth 2.0 specification (cf. https://tools.ietf.org/html/rfc6749#section-2.3.1) :

Clients in possession of a client password MAY use the HTTP Basic
authentication scheme as defined in [RFC2617] to authenticate with
the authorization server. The client identifier is encoded using the
"application/x-www-form-urlencoded" encoding algorithm per
Appendix B, and the encoded value is used as the username; the client
password is encoded using the same algorithm and used as the
password. The authorization server MUST support the HTTP Basic
authentication scheme for authenticating clients that were issued a
client password.

Actually, client_id and client secret are only base64 encoded.

Regards.
Grégoire Rolland

[zandbelt]

Agreed that this should be corrected. I'm not sure we should keep backwards compatibility: that would be for broken OPs or for setups where a url-encoded secret has been configured to accommodate for lua-resty-openidc not implementing url-encoding itself properly ? I think I'd be willing to forget about these and add a warning in the upcoming release notes for those edge cases. I feel that the vast majority of deployments would have configured a URL safe secret anyhow. Thoughts?

[bodewig]

I'm in favor of fixing it unconditionally as well.

[grrolland]

Hello,

For my use case and probably for other situation, I need to keep backward copatibility because I have a multi-tenant implementation with many OP. Some of the OPs implement strictly the RFC, and others don't (typically my internal OP - OpenAM 13.0.0).
So I need a configuration parameter to distinguish the behaviour of the token endpoint authentication.

What do you think about this ?

Regards.

[zandbelt]

I'd prefer not to include workarounds for broken OPs: would it be an option to change your secret for that Provider to something that is URL-safe in itself?

[grrolland]

OK, this option could be possible. I modify the pull request consequently.