Skip to content

Conversation

sberyozkin
Copy link
Member

If GitHub provider fails to complete the authorization code flow, it will reply with 200, without the actual access token, causing a confusing error when Quarkus OIDC attempts to verify this non-existent token via the UserInfo injection...

This PR adds a couple of checks to fail early and an extra check to check the confusing error

Comment on lines 872 to 873
LOG.errorf(
"Neither ID token nor access tokens are available in the authorization code grant response");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything the dev can do to fix that? (Check some URL, setting, etc?)

Copy link
Member Author

@sberyozkin sberyozkin May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gastaldi It depends on the situation, for example, in this case we saw GitHub returning GitHub specific error in JSON format with 200 response type...
Perhaps I should add Please check the logs for more details ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be nice. Also if the data isn't visible in the logs, maybe point to a URL where you can find more info on how to fix that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is in the actual GitHub response, it includes link to the trouble shooting guide, the problem they return it with 200 which needs a debug level to be seen... Let me tune it a bit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a try :-)

This comment has been minimized.

@sberyozkin sberyozkin force-pushed the internal_idtoken_fix branch from 99446fe to 32da46e Compare May 7, 2025 17:05
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

quarkus-bot bot commented May 7, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 32da46e.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@sberyozkin sberyozkin merged commit 8bf9175 into quarkusio:main May 7, 2025
28 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.23 - main milestone May 7, 2025
@sberyozkin sberyozkin deleted the internal_idtoken_fix branch May 7, 2025 17:49
@gsmet gsmet modified the milestones: 3.23 - main, 3.22.3 May 13, 2025
@jmartisk jmartisk modified the milestones: 3.22.3, 3.20.2 Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants