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

Fixes https://github.com/dexidp/dex/issues/2537 #2538

Merged
merged 2 commits into from
May 30, 2022
Merged

Fixes https://github.com/dexidp/dex/issues/2537 #2538

merged 2 commits into from
May 30, 2022

Conversation

ShivanshVij
Copy link
Contributor

@ShivanshVij ShivanshVij commented May 26, 2022

Signed-off-by: Shivansh Vij [email protected]

Overview

What this PR does / why we need it

Closes #2537

This is a bug that was introduced in this PR, and is where the handleDeviceCode handler does not set the content-type header in the response to application/json - because of this, the flow does not conform to the RFC (https://datatracker.ietf.org/doc/html/rfc8628#section-3.3 and https://datatracker.ietf.org/doc/html/rfc6749#section-5.1).

I've also added a test case to check for this exact bug.

Special notes for your reviewer

Does this PR introduce a user-facing change?

Device Code Flow does not return application/json in Content-Type header

Signed-off-by: Shivansh Vij <[email protected]>
Fixes #2537

Signed-off-by: Shivansh Vij <[email protected]>
@sagikazarmark sagikazarmark requested a review from nabokihms May 30, 2022 10:52
@sagikazarmark sagikazarmark added this to the v2.32.0 milestone May 30, 2022
@nabokihms
Copy link
Member

Because our implementation is not compliant with the spec, I think we can accept the PR without worrying about being a breaking change.

@ShivanshVij, good catch! Thank you for your contribution.

@nabokihms nabokihms merged commit a858ffb into dexidp:master May 30, 2022
@ShivanshVij ShivanshVij deleted the 2537-fix-json-response branch May 30, 2022 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Device Code Flow does not return application/json in Content-Type header
3 participants