panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Setting `client_secret` to an empty string does not comply with standards #402

Closed javigonz closed 3 years ago

javigonz commented 3 years ago

Describe the bug We are working in a provider which only supports client_secret_post or client_secret_basic for the token endpoint auth methods, and the authorization code grant which does not use a client_secret parameter.

As it said in the offical OAuth 2.0 Authorization Framework documentation (https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1), The client MAY omit the parameter if the client secret is an empty string.

But having a look to the code (lib/helpers/client.js), into authFor method this is not happens by default, it set client_secret parameter mandatory. I mean, if client_secret is an empty string it should return an object with just client_id and if not leave it as it is now, but that not happens in this code, so I think it does not conform to the standard as I mentioned before.

default: { // client_secret_basic
      if (!this.client_secret) {
        throw new TypeError('client_secret_basic client authentication method requires a client_secret');
      }
      const encoded = `${formUrlEncode(this.client_id)}:${formUrlEncode(this.client_secret)}`;
      const value = Buffer.from(encoded).toString('base64');
      return { headers: { Authorization: `Basic ${value}` } };
    }

To Reproduce Set a client withtoken_endpoint_auth_method: 'client_secret_basic', set the client_id and the client_secret to an empty string. Some kind of message like this is showed:

 message: 'client_secret_basic client authentication method requires a client_secret',

Environment: System: OS: macOS 11.5.2 CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz Memory: 326.37 MB / 16.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 12.13.0 - /usr/local/bin/node Yarn: 1.19.1 - ~/.yarn/bin/yarn npm: 6.9.0 - /usr/local/bin/npm Watchman: 4.9.0 - /usr/local/bin/watchman Browsers: Chrome: 95.0.4628.3 Safari: 14.1.2 npmPackages: next: ^11.1.0 => 11.1.0 next-auth: 4.0.0-beta.2 => 4.0.0-beta.2 react: ^17.0.2 => 17.0.2

panva commented 3 years ago

This makes little sense to me. If you don't have a secret issued use the token_endpoint_auth_method client property none. Otherwise, secret is required for the client_secret based authentication methods.

panva commented 3 years ago

https://github.com/panva/node-openid-client/blob/main/docs/README.md#client-authentication-methods

javigonz commented 3 years ago

According to your documentation, as described in RFC6749, client_idand client_secret are required but Client may omit client_secret if is a empty string, so I think that you are not according with the official documentation.

client_id
        REQUIRED.  The client identifier issued to the client during
        the registration process described by Section 2.2.

 client_secret
       REQUIRED.  The client secret.  The client MAY omit the
       parameter if the client secret is an empty string.
balazsorban44 commented 3 years ago

@panva is correct here, it might be due to next-auth not exposing an option you need. I'll look into how we could expose all relevant openid-client options on our side.

This lib is fantastic! 🙏