Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
javigonz opened this issue Sep 9, 2021 · 4 comments
Closed
Labels

Comments

@javigonz
Copy link

javigonz commented Sep 9, 2021

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 with token_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

@javigonz javigonz added the triage label Sep 9, 2021
@panva
Copy link
Owner

panva commented Sep 9, 2021

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 panva closed this as completed Sep 9, 2021
@panva panva added invalid and removed triage labels Sep 9, 2021
@javigonz
Copy link
Author

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
Copy link
Sponsor

@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! 🙏

@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants