Client_secret should not be URIEncoded as it breaks secrets with special characters

[hequ]

Describe the bug

Client secret with special characters should not be URIEncoded before tranformed to base64 string. This produces a wrong base64 encoded Basic auth string which results the token server to reject the authorization.

To Reproduce Issuer and Client configuration: (inline or gist) - Don't forget to redact your secrets.

// Issuer configuration (issuer.metadata) and how it's constructed (discovery or manual?)
// Constructed manually
{
  issuer: 'https://...redacted.../oic/v1/',
  authorization_endpoint: 'https://...redacted.../oic/v1/public/authorize',
  token_endpoint: 'https://...redacted.../oic/v1/backend/token',
  jwks_uri: 'https://...redacted.../oic/v1/backend/jwk' 
}

// Client configuration (client.metadata) and how it's constructed (fromUri or manual?)
// Issued manually

{
  client_id: clientId,
  client_secret: clientSecretContainingADollarSign
}

Steps to reproduce the behaviour:

1. Get authorization code by calling client.authorizationUrl({...})
2. Pass the code to client.authorizationCallback({...})

Expected behaviour Expected to get the autorization token response from the token server.

Environment:

• openid-client version: 2.4.5
• node version: 8.11.1

Additional context I pinpointed the behaviour to be in lib/client.js in function authFor which calls formUrlEncode to encode the client_id and client_secret. I think these parameters should not be URIEncoded, but to base64 encoded only.

• [x] the bug is happening on latest openid-client too.
• [x] i have searched the issues tracker on github for similar issues and couldn't find anything related.

[panva]

client_secret should not be URIEncoded as it breaks secrets with special characters

No it definitely should, see https://tools.ietf.org/html/rfc6749#section-2.3.1 and the related appendix, this was fixed (introduced) in https://github.com/panva/node-openid-client/releases/tag/v2.0.1 and will stay as such.

If the provider you're using openid-client with does not process your client_secret correctly then you should request them to fix it. If you can't get them to fix client_secret_basic see if you can use client_secret_post instead.

Note this behaviour is certified with OIDC Certification Suite where this behaviour was also missing until recently

[hequ]

Thanks for clarifying this.

[panva]

Happy to.