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.

Sign up for GitHub

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

Jump to bottom

Do not require UserInfo when its injection point is detected for OIDC tenants without the UserInfo endpoint #40456

Merged
merged 1 commit into from
May 4, 2024
Merged

Do not require UserInfo when its injection point is detected for OIDC tenants without the UserInfo endpoint #40456

merged 1 commit into from
May 4, 2024

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented May 4, 2024 •
edited
Loading

fixes: #40434

@quarkus-bot quarkus-bot bot added the area/oidc label May 4, 2024
@michalvavrik michalvavrik force-pushed the feature/oidc-user-info-required-autoenablement-tweaks branch from 8a66605 to 22784f8 Compare May 4, 2024 10:42
@michalvavrik michalvavrik mentioned this pull request May 4, 2024
Closed
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@michalvavrik michalvavrik requested a review from sberyozkin May 4, 2024 11:23
@sberyozkin
Copy link
Member

Thanks @michalvavrik, a minor suggestion there to delay setting the property, set it only if it has not been set, lets continue next week, enjoy the weekend

@michalvavrik michalvavrik force-pushed the feature/oidc-user-info-required-autoenablement-tweaks branch from 22784f8 to 7c0ffd4 Compare May 4, 2024 20:02
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented May 4, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 7c0ffd4.

✅ 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.

@michalvavrik michalvavrik requested a review from sberyozkin May 4, 2024 20:44
@sberyozkin
Copy link
Member

Thanks Michal, I forgot enableUserInfo only does it if the user-info-required is set. LGTM, do enjoy the rest of your weekend

@sberyozkin
Copy link
Member

@markusdlugi, FYI, @michalvavrik helped out with getting all the tenants with different UserInfo capabilities play well together, thanks for raising the issue

@sberyozkin sberyozkin merged commit 251439e into quarkusio:main May 4, 2024
22 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone May 4, 2024
@sberyozkin
Copy link
Member

@gsmet, the reason I'd like this PR to be backported is to remove the breaking change label from #39486 (I made a mistake with the analysis related to that PR), there could be some OIDC tenant combinations with different level of UserInfo support in 3.10.x that will cause Quarkus fail to start. So please have it backported

@michalvavrik michalvavrik deleted the feature/oidc-user-info-required-autoenablement-tweaks branch May 5, 2024 07:02
@markusdlugi
Copy link
Contributor

@michalvavrik, @sberyozkin, thanks a lot for the super-fast fix guys!

@michalvavrik michalvavrik mentioned this pull request May 6, 2024
Closed
@gsmet gsmet modified the milestones: 3.11 - main, 3.10.1 May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sberyozkin sberyozkin sberyozkin approved these changes

Assignees
No one assigned
Labels
area/oidc kind/bugfix
Projects
None yet
Milestone
3.10.1
Development

Successfully merging this pull request may close these issues.

Quarkus 3.10 fails to start if some OIDC providers don't support UserInfo
4 participants
@michalvavrik @sberyozkin @markusdlugi @gsmet