panva / openid-client

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

Scope delimiter replace problem #626

Closed kg0r0 closed 1 year ago

kg0r0 commented 1 year ago

Describe the bug There is a problem in processing the delimiter in the scope parameter of the authorization request. The method for generating the authorization request URL has been changed by the following commit. This also seems to add processing to replace + with %20. However, it seems that only one space is replaced. https://github.com/panva/node-openid-client/commit/a9d3a87d2727bb37a535aeac9da9851ffdef8613#diff-ca407d959d33ce92a7f7efee354a5ea5e6e61fcd797d32c04205e429a074ed54

example:

let url = new URL("https://example.com?foo=1&bar=2");
undefined
url.searchParams.set("scope", "a b c")
undefined
url.toString()
"https://example.com/?foo=1&bar=2&scope=a+b+c"
url.href.replace("+", "%20")
"https://example.com/?foo=1&bar=2&scope=a%20b+c" 

I may be able to implement this bug fix.

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

  const url: string = client.authorizationUrl({
    response_type: 'code',
    scope: options.scope || 'tweet.read users.read offline.access',
    state: options.state,
    code_challenge: options.code_challenge,
    code_challenge_method: options.code_challenge_method
  });

=> Received: "https://localhost:5555/oauth2/authorize?client_id=TEST_CLIENT_ID&scope=tweet.read%20users.read+offline.access&response_type=code&redirect_uri=TEST_REDIRECT_URI&state=TEST_STATE&code_challenge=TEST_CODE_CHALLENGE&code_challenge_method=S256

Expected behaviour All delimiters are replaced correctly.

Environment:

Additional context Add any other context about the problem here.

panva commented 1 year ago

Ah yeah that is certainly an omission on my end. Question is, does your idp start complaining?

kg0r0 commented 1 year ago

Hi, I noticed this in a unit test on my end. I have not yet confirmed the behavior of any particular IdP.