Skip to content

Commit

Permalink
Fix and enable firefox-strict-cookies profile adapter tests
Browse files Browse the repository at this point in the history
Closes keycloak#34853

Signed-off-by: rmartinc <[email protected]>
  • Loading branch information
rmartinc authored and mposolda committed Nov 18, 2024
1 parent 8d559d5 commit e4509a7
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 33 deletions.
46 changes: 44 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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()
Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion testsuite/integration-arquillian/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
<arquillian-infinispan-container.version>1.2.0.Beta3</arquillian-infinispan-container.version>
<undertow.version>${undertow-jakarta.version}</undertow.version>
<undertow-embedded.version>1.0.0.Final</undertow-embedded.version>
<version.org.wildfly.extras.creaper>1.6.1</version.org.wildfly.extras.creaper>
<version.org.wildfly.extras.creaper>2.0.3</version.org.wildfly.extras.creaper>
<jakarta.persistence-legacy.version>2.2.3</jakarta.persistence-legacy.version>
<smallrye.jandex.version>3.0.5</smallrye.jandex.version>
<commons.validator.version>1.8.0</commons.validator.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -30,20 +29,18 @@
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;
import org.wildfly.extras.creaper.core.online.operations.OperationException;
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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1952,6 +1954,8 @@ private List<Cookie> 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<UserRepresentation> users = realm.users().search("bburke", true);
Assert.assertNotNull(users);
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
samesite-cookie(mode=None, cookie-pattern=JSESSIONID)

0 comments on commit e4509a7

Please sign in to comment.