Aligns the logic in the welcome resources #23259

shawkins · 2023-09-14T14:38:22Z

Assumes the following:

KeycloakApplication.BOOTSTRAP_ADMIN_USER is not necessary. The WelcomeResource can localize the tracking using the same logic that was done for the QuarkusWelcomeResource
Changes made to the QuarkusWelcomeResource are applicable to the WelcomeResource, this includes adding underlying exceptions to rethrown exceptions, change to how headers are accessed, etc.
The only real change to QuarkusWelcomeResource is the use of the logger instance instead of the Quarkus Log for logging null themes, but this shouldn't matter

pedroigor

@shawkins Nice! Looking forward to try to remove the QuarkusWelcomeResource altogether :)

pedroigor · 2023-09-14T18:01:08Z

...me/src/main/java/org/keycloak/quarkus/runtime/services/resources/QuarkusWelcomeResource.java

-            throw new WebApplicationException(e, Response.Status.INTERNAL_SERVER_ERROR);
-        }
+    @Override
+    protected String getAdminCreationMessage() {


I think we can change the message on WelcomeResource and remove more code from this class. The message from WelcomeResource does not make sense anymore.

...me/src/main/java/org/keycloak/quarkus/runtime/services/resources/QuarkusWelcomeResource.java

ghost

Unreported flaky test detected, please review

ghost · 2023-09-14T21:49:17Z

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.ui.account2.UpdateEmailTest#updateUserInfoWithRegistrationEnabled

Keycloak CI - Account Console IT (firefox)

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.keycloak.testsuite.util.URLAssert URL expected to begin with: https://localhost:8543/auth/realms/test/protocol/openid-connect/auth ; actual URL: https://localhost:8543/auth/realms/test/account/#/personal-info within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
...

Report flaky test

org.keycloak.testsuite.ui.account2.UpdateEmailTest#updateEmailSucceeds

Keycloak CI - Account Console IT (firefox)

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.keycloak.testsuite.util.URLAssert URL expected to begin with: https://localhost:8543/auth/realms/test/protocol/openid-connect/auth ; actual URL: https://localhost:8543/auth/realms/test/account/#/personal-info within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
...

Report flaky test

vmuzikar

Thanks @shawkins.

~~LGTM but +1 to @pedroigor's comment around leveraging our Hostname SPI.~~
The changes were just pushed ;)

vmuzikar

LGTM.

ghost · 2023-09-15T17:07:25Z

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.VerifyProfileTest#testAttributeRequiredForDefaultScope

Keycloak CI - Forms IT (chrome)

java.lang.IllegalArgumentException: No enum constant org.keycloak.testsuite.pages.AppPage.RequestType.
	at java.base/java.lang.Enum.valueOf(Enum.java:273)
	at org.keycloak.testsuite.pages.AppPage$RequestType.valueOf(AppPage.java:56)
	at org.keycloak.testsuite.pages.AppPage.getRequestType(AppPage.java:49)
	at jdk.internal.reflect.GeneratedMethodAccessor552.invoke(Unknown Source)
...

Report flaky test

ghost

Unreported flaky test detected, please review

vmuzikar · 2023-09-18T15:40:18Z

@pedroigor Do you want to give it another look? :)

pedroigor · 2023-09-25T11:10:24Z

@shawkins Could you please rebase? LGTM.

shawkins · 2023-09-25T12:40:27Z

@pedroigor squashed and rebased

pedroigor

@shawkins Just found an issue when running your changes. Please, see the comment.

Basically, when accessing the local admin URL with an IP other than localhost the link in the page is built using the host as the scheme. I'm not 100% sure how to test this in our test suite as we need to resolve to a IP other than localhost.

...kus/runtime/src/main/java/org/keycloak/quarkus/runtime/hostname/DefaultHostnameProvider.java

pedroigor

@shawkins Thanks!

as a result the quarkus one can be removed closes keycloak#23243

…tname/DefaultHostnameProvider.java Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com>

ghost

Unreported flaky test detected, please review

ghost · 2023-09-28T14:16:56Z

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.webauthn.account.WebAuthnTransportLocaleTest#localizationTransportUSB

Keycloak CI - WebAuthn IT (chrome)

java.lang.AssertionError: Expected OIDCLogin but was  (https://localhost:8543/auth/realms/test/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Ftest%2Faccount%2F%23%2Fsecurity%2Fsigningin&state=f94ca8aa-f884-4b79-88f0-e87ce185970f&response_mode=fragment&response_type=code&scope=openid&nonce=3999b083-21ab-4463-8bfa-464bfbc4b865&code_challenge=DcZ_zMHgtDPu8CxCSjDH-5s6vY4aawhKZSolD6fXFko&code_challenge_method=S256)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.page.AbstractPage.assertCurrent(AbstractPage.java:110)
	at jdk.internal.reflect.GeneratedMethodAccessor628.invoke(Unknown Source)
...

Report flaky test

ghost

Unreported flaky test detected, please review

ghost · 2023-09-28T22:33:18Z

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.webauthn.account.WebAuthnTransportLocaleTest#localizationTransportUSB

Keycloak CI - WebAuthn IT (chrome)

java.lang.AssertionError: Expected OIDCLogin but was  (https://localhost:8543/auth/realms/test/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Ftest%2Faccount%2F%23%2Fsecurity%2Fsigningin&state=f94ca8aa-f884-4b79-88f0-e87ce185970f&response_mode=fragment&response_type=code&scope=openid&nonce=3999b083-21ab-4463-8bfa-464bfbc4b865&code_challenge=DcZ_zMHgtDPu8CxCSjDH-5s6vY4aawhKZSolD6fXFko&code_challenge_method=S256)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.page.AbstractPage.assertCurrent(AbstractPage.java:110)
	at jdk.internal.reflect.GeneratedMethodAccessor628.invoke(Unknown Source)
...

Report flaky test

cypress · 2023-09-29T00:40:55Z

1 flaky test on run #9144 ↗︎

0	1054	96	0	1

Details:

Update quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/hostname/Defau...
Project: Keycloak Admin UI	Commit: `b07391b538`
Status: Passed	Duration: 12:19 💡
Started: Sep 29, 2023 12:28 AM	Ended: Sep 29, 2023 12:40 AM

cypress/e2e/realm_settings_general_tab_test.spec.ts • 1 flaky test • chrome

View Output Video

Test		Artifacts
Realm settings general tab tests > Test all general tab switches		`Test Replay` `Output` `Screenshots`

Review all test suite changes for PR #23259 ↗︎

jonkoops · 2023-10-25T09:55:48Z

Thanks for all involved in landing this, it's much appreciated!

ghost added the flaky-test label Sep 14, 2023

This was referenced Sep 14, 2023

Flaky test: org.keycloak.testsuite.ui.account2.PermissionsTest#manageAccountRoleRequired #20586

Closed

Flaky test: org.keycloak.testsuite.forms.BrowserFlowTest#testAlternativeNonInteractiveExecutorInSubflow #22644

Closed

shawkins force-pushed the iss23243 branch from baefda8 to a8a809a Compare September 14, 2023 16:46

shawkins marked this pull request as ready for review September 14, 2023 16:51

shawkins requested review from a team as code owners September 14, 2023 16:51

ghost added team/cloud-native labels Sep 14, 2023

ghost mentioned this pull request Sep 14, 2023

Flaky test: org.keycloak.testsuite.forms.RecoveryAuthnCodesAuthenticatorTest#test03AuthenticateRecoveryAuthnCodes #22691

Closed

pedroigor reviewed Sep 14, 2023

View reviewed changes

shawkins force-pushed the iss23243 branch from a8a809a to 38ff01e Compare September 14, 2023 20:10

ghost mentioned this pull request Sep 14, 2023

Flaky test: org.keycloak.testsuite.adapter.servlet.OfflineServletsAdapterTest#testServlet #21536

Closed

ghost reviewed Sep 14, 2023

View reviewed changes

vmuzikar reviewed Sep 15, 2023

View reviewed changes

vmuzikar previously approved these changes Sep 15, 2023

View reviewed changes

vmuzikar requested a review from pedroigor September 15, 2023 15:31

ghost mentioned this pull request Sep 15, 2023

Flaky test: org.keycloak.testsuite.forms.ResetPasswordTest#resetPasswordLink #21422

Closed

ghost reviewed Sep 15, 2023

View reviewed changes

shawkins dismissed vmuzikar’s stale review via c0de003 September 25, 2023 12:39

shawkins force-pushed the iss23243 branch from 4cb3aa5 to c0de003 Compare September 25, 2023 12:39

vmuzikar previously approved these changes Sep 25, 2023

View reviewed changes

pedroigor approved these changes Sep 25, 2023

View reviewed changes

pedroigor requested changes Sep 25, 2023

View reviewed changes

...kus/runtime/src/main/java/org/keycloak/quarkus/runtime/hostname/DefaultHostnameProvider.java Outdated Show resolved Hide resolved

shawkins dismissed vmuzikar’s stale review via 3015f90 September 25, 2023 19:30

vmuzikar requested a review from pedroigor September 26, 2023 07:41

vmuzikar approved these changes Sep 26, 2023

View reviewed changes

pedroigor approved these changes Sep 27, 2023

View reviewed changes

pedroigor enabled auto-merge (rebase) September 27, 2023 11:15

This was referenced Sep 27, 2023

Flaky test: org.keycloak.testsuite.ui.account2.DeleteAccountTest#deleteOwnAccountForbiddenWithoutClientRole #23365

Closed

Flaky test: org.keycloak.testsuite.ui.account2.UpdateEmailTest#updateUserInfoWithRegistrationEnabled #23409

Closed

shawkins and others added 2 commits September 28, 2023 08:30

Aligns the logic in the welcome resources

9a1dd17

as a result the quarkus one can be removed closes keycloak#23243

Update quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/hos…

07d2083

…tname/DefaultHostnameProvider.java Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com>

shawkins force-pushed the iss23243 branch from 3015f90 to 07d2083 Compare September 28, 2023 12:30

ghost reviewed Sep 28, 2023

View reviewed changes

pedroigor merged commit b07391b into keycloak:main Sep 28, 2023

ghost reviewed Sep 28, 2023

View reviewed changes

shawkins mentioned this pull request Apr 22, 2025

fix: relaxes the admin root redirect check #39095

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Aligns the logic in the welcome resources #23259

Aligns the logic in the welcome resources #23259

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Review all test suite changes for PR #23259 ↗︎

Uh oh!

Uh oh!

Uh oh!

Aligns the logic in the welcome resources #23259

Aligns the logic in the welcome resources #23259

Uh oh!

Conversation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.ui.account2.UpdateEmailTest#updateUserInfoWithRegistrationEnabled

org.keycloak.testsuite.ui.account2.UpdateEmailTest#updateEmailSucceeds

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.forms.VerifyProfileTest#testAttributeRequiredForDefaultScope

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.webauthn.account.WebAuthnTransportLocaleTest#localizationTransportUSB

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.webauthn.account.WebAuthnTransportLocaleTest#localizationTransportUSB

Uh oh!

1 flaky test on run #9144 ↗︎

Review all test suite changes for PR #23259 ↗︎

Uh oh!

Uh oh!

Uh oh!