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

State ID check in Hybrid flow is not accurate #669

Closed
3 of 6 tasks
harjoben opened this issue May 12, 2022 · 2 comments
Closed
3 of 6 tasks

State ID check in Hybrid flow is not accurate #669

harjoben opened this issue May 12, 2022 · 2 comments
Labels
bug Something is not working.

Comments

@harjoben
Copy link
Contributor

Preflight checklist

Describe the bug

The flow_hybrid.go has this check here for state where it assigns the value from the fosite.AuthorizeRequester if there is no state present in the response parameters.

if resp.GetParameters().Get("state") == "" {
        resp.AddParameter("state", ar.GetState())
}

But the check will misbehave if there is empty string passed in as "state". In that scenario, response parameter will have "state" with two empty strings as values. This will result in the redirect uri having duplicate state keys.

The above if statement should be changed to

if !resp.GetParameters().Has("state") {
        resp.AddParameter("state", ar.GetState())
}

Reproducing the bug

Pass in empty state in the authorize request

Relevant log output

The redirect uri will look something like:
ui/oidcclient/redirect#expires_in=7199&token_type=bearer&state=&state=&scope=openid%20profile&id_token=<id_token>&code=<code>&access_token=<access_token>

Relevant configuration

No response

Version

v0.41.0

On which operating system are you observing this issue?

No response

In which environment are you deploying?

No response

Additional Context

No response

@harjoben harjoben added the bug Something is not working. label May 12, 2022
@aeneasr
Copy link
Member

aeneasr commented May 12, 2022

Nice find, PRs appreciated :)

@james-d-elliott
Copy link
Contributor

@aeneasr this was fixed by #670

@aeneasr aeneasr closed this as completed Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

No branches or pull requests

3 participants