client_secret being passed in post data in Basic auth

[carlbarrdahl]

Describe the bug
We're trying to integrate Signicat OpenID with Basic auth and get this message:

{
  statusCode: 400,
  data: '{"error":"invalid_request","error_description":"Your client is configured to authenticate using CLIENT_SECRET_BASIC"}'
}

This is because client_secret is sent as post data here, even if its null or undefined:

next-auth/src/server/lib/oauth/callback.js

Line 225 in 9dbd372

const postData = querystring.stringify(params)

With empty client secret (note the: &client_secret=):

// formData is: grant_type=authorization_code&client_id=client_id&client_secret=&code=code&redirect_uri=uri
{
  statusCode: 400,
  data: `{"error":"invalid_request","error_description":"Form body malformed! (Body is not a set of key-value pairs separated by '=', delimited by '&' characters)"}`
}

Here are some possible solutions:

• Use this querystring library and strip nulls in the query object. https://github.com/sindresorhus/query-string#skipnull
• Change this code to not set the params.client_secret at all.

if (!params.client_secret) {
  if (provider.clientSecretCallback) {
    params.client_secret = yield provider.clientSecretCallback(provider.clientSecret);
  } else {
    params.client_secret = provider.clientSecret;
  }
}

Steps to reproduce
Use custom oauth2 config:

{
  id: "signicat",
  name: "Signicat BankID",
  type: "oauth",
  version: "2.0",
  scope: "openid profile signicat.national_id signicat.certificate",
  params: { grant_type: "authorization_code" },
  accessTokenUrl: `https://${config.signicatHost}/oidc/token`,
  requestTokenUrl: `https://${config.signicatHost}/oidc/token`,
  authorizationUrl: `https://${config.signicatHost}/oidc/authorize?response_type=code&response_mode=form_post&acr_values=urn:signicat:oidc:method:sbid`,
  profileUrl: `https://${config.signicatHost}/oidc/userinfo`,
  clientId: config.signicatClientId,
  clientSecret: config.signicatClientSecret,
  idToken: true,
  state: false,
  clientSecretCallback: () => null, // Remove client_secret from post data in request
  headers: {
    Authorization: `Basic ${Buffer.from(config.signicatClientId + ":" + config.signicatClientSecret, "ascii").toString("base64")}`,
  }
}

Expected behavior
client_secret should be omitted from postData if headers.Authorization.includes("Basic")

Feedback
Documentation refers to searching through online documentation, code comments and issue history. The example project refers to next-auth-example.

• Found the documentation helpful
• Found documentation but was incomplete
• Could not find relevant documentation
• Found the example project helpful
• Did not find the example project helpful

[balazsorban44]

Hi there!

Just out of curiosity, since I'm not familiar with Signicat, have you actually tried to just provide a secret and see if sending it in post data would work?

Is this the documentation of what you are referring to?: https://developer.signicat.com/documentation/authentication/protocols/openid-connect/endpoints/

I think we have to see if we want to/need to expand the public facing API as you suggested above, but I have to discuss this further first.

I'll get back to you with more information,

Thanks!

[carlbarrdahl]

Hi @balazsorban44. Thanks for your response.

Yes when sending the client_secret Signicat throws me the first error (Your client is configured to authenticate using CLIENT_SECRET_BASIC).

I was able to get around this by monkeypatching querystring to filter out empty keys:

const _stringify = querystring.stringify;
querystring.stringify = (...args) => {
  args[0] = Object.keys(args[0]).reduce(
    (obj, x) => (args[0][x] ? { ...obj, [x]: args[0][x] } : obj),
    {}
  );
  return _stringify.call(null, ...args);
};

However, I got stuck again when trying to parse the profileData:

next-auth/src/server/lib/oauth/callback.js

Line 144 in 9dbd372

if (typeof profileData === 'string' || profileData instanceof String) { profileData = JSON.parse(profileData) }

Since we're using encryption Signicat is sending back an encrypted JWT which obviously fails to JSON.parse.

This is unfortunate because if I could only get past that line, I would be able to decode it in the provider.profile function.

next-auth/src/server/lib/oauth/callback.js

Line 153 in 8827950

profile = await provider.profile(profileData)

I really wish to use this excellent library but for now I don't see how to get it working without doing a fork and rewrite parts of:
/src/server/lib/oauth/callback.js to change:

• Omit client_secret from post data if headers.Authorization is set in _getOAuthAccessToken
• Allow encrypted profileData (don't throw if JSON.parse fails) in _getProfile

[balazsorban44]

Hmm, if you have the time and interest, I think you could try create the changes and propose it in a PR, and we can discuss further what we could incorporate into the core package. I think if it is a general enough solution that we can back up with links to official OAuth and/or OIDC specs and doesn't alter the user facing api much (or at all), I am pretty sure we could (and should) accept such a change.

[carlbarrdahl]

After playing around with the code base and with panva/node-openid-client, I think perhaps this could be a solution:

• Provide a way to add a private keystore to provider config
• Set provider.idToken to an object with encryption algorithms
• Decrypt idToken if keystore and algorithms are present

Here is the relevant code in panva/node-openid
https://github.com/panva/node-openid-client/blob/82382e8e8904e9cfb87228d11fd01ca7c425af9d/lib/client.js#L449-L457

Basically there needs to be a way to decrypt tokens before they get decoded if keys are present.

[mod-flux]

@balazsorban44 I've just come across this issue with an OAuth service that doesn't accept via POST. As per the most up-to-date IETF OAuth 2.0 Authorization Framework spec, providing the client credentials via the request-body (POST) is explicitly called out as NOT RECOMMENDED and only to be used in situations where clients are unable to directly utilize the HTTP Basic authentication scheme, which doesn't cover this library's primary use case. Here is the snippet in question in the RFC:

Including the client credentials in the request-body using the two
parameters is NOT RECOMMENDED and SHOULD be limited to clients unable
to directly utilize the HTTP Basic authentication scheme (or other
password-based HTTP authentication schemes). The parameters can only
be transmitted in the request-body and MUST NOT be included in the
request URI.

Reference: https://tools.ietf.org/html/rfc6749#section-2.3.1

Not having the ability to choose between Basic Auth and POST by easily stripping out the client_id and client_secret from the request body and provide it via Basic Auth is a blocker for us as our OAuth provider is following the latest OAuth specifications and doesn't marry with the support available with NextAuth. :-(

As per your previous comments with regards to links, does this cover that requirement?

[Fronix]

@carlbarrdahl We have the exact same issue using Signicat's OpenID-connect solution. Did you manage to get it working with the modifications?