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

Bump Keycloak version to 22.0.0 #34740

Merged
merged 2 commits into from
Jul 14, 2023
Merged

Conversation

pedroigor
Copy link
Contributor

No description provided.

@pedroigor pedroigor requested a review from sberyozkin July 13, 2023 19:34
@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/keycloak labels Jul 13, 2023
@sberyozkin
Copy link
Member

Great stuff thanks @pedroigor

@sberyozkin
Copy link
Member

Checking locally

@sberyozkin
Copy link
Member

CodeFlowTest.testAccessAndRefreshTokenInjectionWithoutIndexHtmlAndListenerMultiTab is failing, I'll have a look

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

@pedroigor Many thanks for the PR, minor issue in the test related to the multi-tab setup which I fixed, I'll check it outside the test setup - but as far as Quarkus is concerned it can't affect how KC responds in such cases so we can chat about it afterwards and see what/if might need to be tweaked further

@sberyozkin
Copy link
Member

sberyozkin commented Jul 14, 2023

@pedroigor For Keycloak 20, 21, 22 I get now 401 as well in the browser, with Cookie not found. Please make sure cookies are enabled in your browser., While test gets an expected You are already logged in. It feels like it is the browser related configuration which influences it. I'm pretty sure that specifically Keycloak users confirmed multi-tab was working fine for them and I did try it in the quickstart before writing a test - as I needed to know what Keycloak would return in the first tab to verify it in the test.

But in any case - this is definitely not something Quarkus can influence, it is between the browser and Keycloak, so I'm not worried. If I see users documenting how this should be worked around then I can add some advice to our docs...

Thanks

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 14, 2023

Failing Jobs - Building 152f93b

Status Name Step Failures Logs Raw logs
JVM Tests - JDK 11 Build ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 17
✔️ JVM Tests - JDK 19
✔️ Maven Tests - JDK 11
Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs

@sberyozkin
Copy link
Member

Tests look ok. Keycloak authorization is the only integration point. For dev services changing a KC image name is easy

@sberyozkin sberyozkin merged commit 5ddba07 into quarkusio:main Jul 14, 2023
@quarkus-bot quarkus-bot bot added this to the 3.3 - main milestone Jul 14, 2023
@gsmet
Copy link
Member

gsmet commented Jul 18, 2023

Is it something that we want in 3.2?

@maxandersen
Copy link
Member

So 22.0 is what is supposed tonight be used in 3.2 - but there is an open question about forward and backwards compatibility of the client that the Keycloak team is going to investigate next week.

@sberyozkin
Copy link
Member

Added a breaking-change label due to #35572 - even though it is not a keycloak-authorization specific breaking change

@sberyozkin
Copy link
Member

@maxandersen @gsmet See #35572, for it to be backported to 3.2, keycloak-adapter-core dependency which is no longer needed after the update to KC 22.0.1, would have to be re-introduced as users may be depending on some code from this dependency

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.

4 participants