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

Support setting client_secret to an empty string when using client_secret_basic or client_secret_post #609

Closed
Exidex opened this issue Jul 1, 2023 · 1 comment
Labels

Comments

@Exidex
Copy link
Contributor

Exidex commented Jul 1, 2023

This request is basically a copy of this issue only with different motivation.

I am trying to login into the Cloud Foundry UAA server using /oauth/token endpoint described here.
The exact request that works is:

POST https://login.cf.internal/oauth/token
Accept: application/json
Content-Type: application/x-www-form-urlencoded
Authorization: Basic Y2Y6

grant_type=password&password=your_password&username=your_username

Y2Y6 decodes to cf: where cf is client_id and and client_secret is empty string. These values are not documented in the CF UAA docs so I had to dig into CF CLI code to find them. And checked that was the only http request that worked. Removing Authorization header results in 401 error, therefore none token_endpoint_auth_method wouldn't work as proposed by the previous issue.

The following code fails with client_secret_basic client authentication method requires a client_secret error

const { Client } = await Issuer.discover("https://login.cf.internal");

const client = new Client({
  client_id: "cf", // there is no documentation about this, had to dig into cf cli code
  client_secret: "",
});

const tokenSet = await client.grant({
  grant_type: "password",
  username: "your_username",
  password: "your_password"
});

While the spec says that it should be required, the spec also acknowledges that the value can be empty string

Possible solutions:

  • Change the check to allow empty strings and not other falsy values (i think prefered)
  • Remove the check
  • Add options to explicitly allow empty string in secret

This library is the only one I found that is actually high quality compared to alternatives and rolling out my own implementation to do the same would not be a solution I would like to go with over this one small hick up. I hope you reconsider your previous decision based on the information listed above

Environment:

  • openid-client version: v5.4.2
  • node version: v18.16.0
@Exidex Exidex added the triage label Jul 1, 2023
@panva
Copy link
Owner

panva commented Jul 1, 2023

Change the check to allow empty strings and not other falsy values

I believe this would be the most appropriate alteration.

@panva panva closed this as completed in 402c711 Jul 6, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2023
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

2 participants