From e4509a71991437d8b09f5441b88a0818a1c99e33 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Thu, 14 Nov 2024 12:41:03 +0100 Subject: [PATCH] Fix and enable firefox-strict-cookies profile adapter tests Closes #34853 Signed-off-by: rmartinc --- .github/workflows/ci.yml | 46 ++++++++++++++++++- testsuite/integration-arquillian/pom.xml | 2 +- .../arquillian/AppServerTestEnricher.java | 35 +++----------- .../servlet/SAMLServletAdapterTest.java | 17 ++++++- .../samesite/undertow-handlers.conf | 1 + 5 files changed, 68 insertions(+), 33 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/samesite/undertow-handlers.conf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b84ad02e20b..867482a7f5d1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -143,7 +143,7 @@ jobs: name: Adapter IT needs: build runs-on: ubuntu-latest - timeout-minutes: 100 + timeout-minutes: 30 steps: - uses: actions/checkout@v4 @@ -161,7 +161,7 @@ jobs: run: | TESTS="org.keycloak.testsuite.adapter.**" echo "Tests: $TESTS" - ./mvnw test ${{ env.SUREFIRE_RETRY }} -Pauth-server-quarkus -Papp-server-wildfly -Dtest=$TESTS -pl testsuite/integration-arquillian/tests/base 2>&1 | misc/log/trimmer.sh + ./mvnw test ${{ env.SUREFIRE_RETRY }} -Pauth-server-quarkus -Papp-server-wildfly -Dtest=$TESTS -Dapp.server.ssl.required=true -pl testsuite/integration-arquillian/tests/base 2>&1 | misc/log/trimmer.sh - name: Upload JVM Heapdumps if: always() @@ -180,6 +180,47 @@ jobs: with: job-id: adapter-integration-tests + adapter-integration-tests-strict-cookies: + name: Adapter IT Strict Cookies + needs: build + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@v4 + + - id: integration-test-setup + name: Integration test setup + uses: ./.github/actions/integration-test-setup + + - name: Build adapter distributions + run: ./mvnw install -DskipTests -f distribution/pom.xml + + - name: Build app servers + run: ./mvnw install -DskipTests -Pbuild-app-servers -f testsuite/integration-arquillian/servers/app-server/pom.xml + + - name: Run adapter tests + run: | + TESTS="org.keycloak.testsuite.adapter.**" + echo "Tests: $TESTS" + ./mvnw test ${{ env.SUREFIRE_RETRY }} -Pfirefox-strict-cookies -Pauth-server-quarkus -Papp-server-wildfly -Dtest=$TESTS -Dapp.server.ssl.required=true "-Dwebdriver.gecko.driver=$GECKOWEBDRIVER/geckodriver" -pl testsuite/integration-arquillian/tests/base 2>&1 | misc/log/trimmer.sh + + - name: Upload JVM Heapdumps + if: always() + uses: ./.github/actions/upload-heapdumps + + - uses: ./.github/actions/upload-flaky-tests + name: Upload flaky tests + env: + GH_TOKEN: ${{ github.token }} + with: + job-name: Base IT + + - name: Surefire reports + if: always() + uses: ./.github/actions/archive-surefire-reports + with: + job-id: adapter-integration-tests-strict-cookies + quarkus-unit-tests: name: Quarkus UT needs: [build, conditional] @@ -1044,6 +1085,7 @@ jobs: - unit-tests - base-integration-tests - adapter-integration-tests + - adapter-integration-tests-strict-cookies - quarkus-unit-tests - quarkus-integration-tests - jdk-integration-tests diff --git a/testsuite/integration-arquillian/pom.xml b/testsuite/integration-arquillian/pom.xml index 371fd693d3c4..519cd78ba676 100644 --- a/testsuite/integration-arquillian/pom.xml +++ b/testsuite/integration-arquillian/pom.xml @@ -53,7 +53,7 @@ 1.2.0.Beta3 ${undertow-jakarta.version} 1.0.0.Final - 1.6.1 + 2.0.3 2.2.3 3.0.5 1.8.0 diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/AppServerTestEnricher.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/AppServerTestEnricher.java index e2565035fb18..3ff3e80330bf 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/AppServerTestEnricher.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/AppServerTestEnricher.java @@ -17,7 +17,6 @@ package org.keycloak.testsuite.arquillian; -import java.io.File; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.StringUtils; import org.jboss.arquillian.container.test.api.ContainerController; @@ -30,13 +29,10 @@ import org.keycloak.testsuite.arquillian.annotation.AppServerContainer; import org.keycloak.testsuite.arquillian.annotation.AppServerContainers; import org.keycloak.testsuite.arquillian.containers.SelfManagedAppContainerLifecycle; -import org.wildfly.extras.creaper.commands.web.AddConnector; -import org.wildfly.extras.creaper.commands.web.AddConnectorSslConfig; import org.wildfly.extras.creaper.core.CommandFailedException; import org.wildfly.extras.creaper.core.ManagementClient; import org.wildfly.extras.creaper.core.online.CliException; import org.wildfly.extras.creaper.core.online.ManagementProtocol; -import org.wildfly.extras.creaper.core.online.ModelNodeResult; import org.wildfly.extras.creaper.core.online.OnlineManagementClient; import org.wildfly.extras.creaper.core.online.OnlineOptions; import org.wildfly.extras.creaper.core.online.operations.Address; @@ -44,6 +40,7 @@ import org.wildfly.extras.creaper.core.online.operations.Operations; import org.wildfly.extras.creaper.core.online.operations.admin.Administration; +import java.io.File; import java.io.IOException; import java.lang.reflect.Method; import java.net.MalformedURLException; @@ -57,9 +54,7 @@ import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; -import static org.keycloak.testsuite.arquillian.ServerTestEnricherUtil.addHttpsListenerAppServer; import static org.keycloak.testsuite.arquillian.ServerTestEnricherUtil.reloadOrRestartTimeoutClient; -import static org.keycloak.testsuite.arquillian.ServerTestEnricherUtil.removeHttpsListener; import static org.keycloak.testsuite.util.ServerURLs.getAppServerContextRoot; import static org.keycloak.testsuite.util.ServerURLs.getAuthServerContextRoot; @@ -214,38 +209,22 @@ public static void enableHTTPSForManagementClient(OnlineManagementClient client) Administration administration = new Administration(client); Operations operations = new Operations(client); - boolean isElytronConfigured = false; - try { - // check if the eap is configured to use elytron - ModelNodeResult result = operations.readAttribute(Address.subsystem("undertow") - .and("server", "default-server").and("https-listener", "https"), "ssl-context"); - if (!result.isFailed() && result.hasDefinedValue()) { - isElytronConfigured = true; - } - } catch (IOException e) { - log.debug("Error reading 'ssl-context' attribute in undertow, assuming not elytron", e); - } - - if (isElytronConfigured && !operations.exists(Address.subsystem("elytron").and("server-ssl-context", "KCSslContext"))) { + if (!operations.exists(Address.subsystem("elytron").and("server-ssl-context", "KCSslContext"))) { client.execute("/subsystem=elytron/key-store=KCKeyStore:add(path=adapter.jks,relative-to=jboss.server.config.dir,credential-reference={clear-text=secret},type=JKS)"); client.execute("/subsystem=elytron/key-manager=KCKeyManager:add(key-store=KCKeyStore,credential-reference={clear-text=secret,algorithm=PKIX})"); client.execute("/subsystem=elytron/key-store=KCTrustStore:add(relative-to=jboss.server.config.dir,path=keycloak.truststore,credential-reference={clear-text=secret},type=JKS)"); client.execute("/subsystem=elytron/trust-manager=KCTrustManager:add(key-store=KCTrustStore)"); client.execute("/subsystem=elytron/server-ssl-context=KCSslContext:add(key-manager=KCKeyManager,trust-manager=KCTrustManager)"); - client.execute("/subsystem=undertow/server=default-server/https-listener=https:write-attribute(name=ssl-context,value=KCSslContext)"); - } else if (!operations.exists(Address.coreService("management").and("security-realm", "UndertowRealm"))) { - client.execute("/core-service=management/security-realm=UndertowRealm:add()"); - client.execute("/core-service=management/security-realm=UndertowRealm/server-identity=ssl:add(keystore-relative-to=jboss.server.config.dir,keystore-password=secret,keystore-path=adapter.jks"); + if (operations.exists(Address.subsystem("undertow").and("server", "default-server").and("https-listener", "https"))) { + client.execute("/subsystem=undertow/server=default-server/https-listener=https:write-attribute(name=ssl-context,value=KCSslContext)"); + } else { + client.execute("/subsystem=undertow/server=default-server/https-listener=https:add(socket-binding=https, enable-http2=true, ssl-context=KCSslContext)"); + } } client.execute("/system-property=javax.net.ssl.trustStore:add(value=${jboss.server.config.dir}/keycloak.truststore)"); client.execute("/system-property=javax.net.ssl.trustStorePassword:add(value=secret)"); - if (!isElytronConfigured) { - removeHttpsListener(client, administration); - addHttpsListenerAppServer(client); - } - reloadOrRestartTimeoutClient(administration); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java index 8b1f276d53d1..855a2cdb40e6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java @@ -106,6 +106,7 @@ import org.jboss.shrinkwrap.api.spec.WebArchive; import org.junit.Assert; +import org.junit.Assume; import org.junit.Test; import org.keycloak.admin.client.Keycloak; @@ -167,6 +168,7 @@ import org.keycloak.testsuite.updaters.Creator; import org.keycloak.testsuite.updaters.UserAttributeUpdater; import org.keycloak.testsuite.util.AdminClientUtil; +import org.keycloak.testsuite.util.BrowserDriverUtil; import org.keycloak.testsuite.util.BrowserTabUtil; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.SamlClient; @@ -1952,6 +1954,8 @@ private List impersonate(String admin, String adminPassword, String user @Test public void testImpersonationForSaml() throws IOException { + Assume.assumeFalse("The firefox driver does not allow to set the cookies", BrowserDriverUtil.isDriverFirefox(driver)); + RealmResource realm = adminClient.realm(SAMLSERVLETDEMO); List users = realm.users().search("bburke", true); Assert.assertNotNull(users); @@ -1987,12 +1991,21 @@ public void testImpersonationForSaml() throws IOException { checkLoggedOut(salesPostSigEmailServletPage, testRealmSAMLPostLoginPage); } + private String getCookieValue(String name, String path) { + return driver.manage().getCookies() + .stream() + .filter(c -> name.equals(c.getName()) && path.equals(c.getPath())) + .findAny() + .map(Cookie::getValue) + .orElse(null); + } + @Test public void testChangeSessionID() throws Exception { // login in the employeeDom application assertSuccessfulLogin(employeeDomServletPage, bburkeUser, testRealmSAMLPostLoginPage, "principal=bburke"); assertSuccessfullyLoggedIn(employeeDomServletPage, "principal=bburke"); - String sessionId = driver.manage().getCookieNamed("JSESSIONID").getValue(); + String sessionId = getCookieValue("JSESSIONID", "/employee-dom"); // retrieve the saml document driver.navigate().to(employeeDomServletPage.getUriBuilder().clone().path("getAssertionFromDocument").build().toURL()); @@ -2003,7 +2016,7 @@ public void testChangeSessionID() throws Exception { // change the session id driver.navigate().to(employeeDomServletPage.getUriBuilder().clone().path("change-session-id").build().toURL()); waitForPageToLoad(); - Assert.assertNotEquals("SessionID has not been changed at login", sessionId, driver.manage().getCookieNamed("JSESSIONID").getValue()); + Assert.assertNotEquals("SessionID has not been changed at login", sessionId, getCookieValue("JSESSIONID", "/employee-dom")); // retrieve again the saml document and should be the same as login should be maintained driver.navigate().to(employeeDomServletPage.getUriBuilder().clone().path("getAssertionFromDocument").build().toURL()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/samesite/undertow-handlers.conf b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/samesite/undertow-handlers.conf new file mode 100644 index 000000000000..692c64016d20 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/samesite/undertow-handlers.conf @@ -0,0 +1 @@ +samesite-cookie(mode=None, cookie-pattern=JSESSIONID) \ No newline at end of file