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

Scope delimiter replace problem #626

Comments

@kg0r0
Copy link
Contributor

kg0r0 commented Oct 9, 2023

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.
a9d3a87#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:

  • openid-client version: 5.6.0
  • node version: v20,18,16

Additional context
Add any other context about the problem here.

  • the bug is happening on latest openid-client too.
  • i have searched the issues tracker on github for similar issues and couldn't find anything related.
@kg0r0 kg0r0 added the triage label Oct 9, 2023
@panva
Copy link
Owner

panva commented Oct 9, 2023

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

@kg0r0
Copy link
Contributor Author

kg0r0 commented Oct 9, 2023

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

@panva panva closed this as completed in ad68223 Oct 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2024
@panva panva removed the triage label Oct 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.