From 03372d2f41dd09dfc853db0762369e26fd8064e3 Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Tue, 9 Jan 2024 14:59:20 +0100 Subject: [PATCH] Fix OfflineServletAdapterTest failures, and improve logging (#25724) Closes #25714 Closes #14448 Signed-off-by: Alexander Schwartz --- .../adapter/page/AbstractShowTokensPage.java | 25 ++++---- .../adapter/page/LogScreenContents.java | 41 +++++++++++++ .../adapter/AbstractServletsAdapterTest.java | 5 +- .../servlet/DemoServletsAdapterTest.java | 2 +- .../OIDCPublicKeyRotationAdapterTest.java | 3 +- .../servlet/OfflineServletsAdapterTest.java | 58 +++++++++---------- .../offline-client/WEB-INF/web.xml | 7 +++ .../secure-portal/WEB-INF/web.xml | 10 ++++ 8 files changed, 101 insertions(+), 50 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/LogScreenContents.java diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/AbstractShowTokensPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/AbstractShowTokensPage.java index e88aa81082c..94ecf892c0c 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/AbstractShowTokensPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/AbstractShowTokensPage.java @@ -20,6 +20,7 @@ package org.keycloak.testsuite.adapter.page; import org.keycloak.representations.AccessToken; import org.keycloak.representations.RefreshToken; import org.keycloak.testsuite.page.AbstractPageWithInjectedUrl; +import org.keycloak.testsuite.util.WaitUtils; import org.keycloak.util.JsonSerialization; import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; @@ -46,47 +47,43 @@ public abstract class AbstractShowTokensPage extends AbstractPageWithInjectedUrl public AccessToken getAccessToken() { try { + WaitUtils.waitUntilElement(accessToken).is().visible(); return JsonSerialization.readValue(accessToken.getText(), AccessToken.class); } catch (IOException e) { - e.printStackTrace(); + throw new RuntimeException(e); } catch (NoSuchElementException nsee) { - log.warn("No accessToken element found on the page"); + throw LogScreenContents.fail(driver, "No accessToken element found on the page", nsee); } - - return null; } public RefreshToken getRefreshToken() { try { + WaitUtils.waitUntilElement(refreshToken).is().visible(); return JsonSerialization.readValue(refreshToken.getText(), RefreshToken.class); } catch (IOException e) { - e.printStackTrace(); + throw new RuntimeException(e); } catch (NoSuchElementException nsee) { - log.warn("No refreshToken element found on the page"); + throw LogScreenContents.fail(driver, "No refreshToken element found on the page", nsee); } - - return null; } public String getAccessTokenString() { try { + WaitUtils.waitUntilElement(accessTokenString).is().visible(); return accessTokenString.getText(); } catch (NoSuchElementException nsee) { - log.warn("No accessTokenString element found on the page"); + throw LogScreenContents.fail(driver, "No accessTokenString element found on the page", nsee); } - - return null; } public String getRefreshTokenString() { try { + WaitUtils.waitUntilElement(refreshTokenString).is().visible(); return refreshTokenString.getText(); } catch (NoSuchElementException nsee) { - log.warn("No refreshTokenString element found on the page"); + throw LogScreenContents.fail(driver, "No refreshTokenString element found on the page", nsee); } - - return null; } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/LogScreenContents.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/LogScreenContents.java new file mode 100644 index 00000000000..f1de8aa87ce --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/LogScreenContents.java @@ -0,0 +1,41 @@ +/* + * Copyright 2023 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.testsuite.adapter.page; + +import org.jboss.logging.Logger; +import org.openqa.selenium.OutputType; +import org.openqa.selenium.TakesScreenshot; +import org.openqa.selenium.WebDriver; + +/** + * @author Alexander Schwartz + */ +public class LogScreenContents { + + private static final Logger log = Logger.getLogger(LogScreenContents.class); + + public static T fail(WebDriver webDriver, String message, T t) { + String screenShotBase64 = null; + if (webDriver instanceof TakesScreenshot) { + screenShotBase64 = ((TakesScreenshot) webDriver).getScreenshotAs(OutputType.BASE64); + } + log.error(message + ", url '" + webDriver.getCurrentUrl() + "', title: " + webDriver.getTitle() + ", source: " + webDriver.getPageSource() + + (screenShotBase64 != null ? ", screenshot: " + screenShotBase64 : "")); + return t; + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java index 8e85c3d4934..cb3141ea03b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java @@ -36,6 +36,9 @@ import java.util.List; import org.jboss.shrinkwrap.api.asset.UrlAsset; import org.junit.Assert; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; import static org.keycloak.testsuite.auth.page.AuthRealm.DEMO; import static org.keycloak.testsuite.util.WaitUtils.waitForPageToLoad; @@ -231,7 +234,7 @@ public abstract class AbstractServletsAdapterTest extends AbstractAdapterTest { DroneUtils.getCurrentDriver().navigate().to(timeOffsetUri); waitForPageToLoad(); String pageSource = DroneUtils.getCurrentDriver().getPageSource(); - log.info(pageSource); + assertThat(pageSource, containsString("Offset set successfully")); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java index 4ddbf386e95..806fe687eef 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java @@ -215,7 +215,7 @@ public class DemoServletsAdapterTest extends AbstractServletsAdapterTest { @Deployment(name = SecurePortal.DEPLOYMENT_NAME) protected static WebArchive securePortal() { - return servletDeployment(SecurePortal.DEPLOYMENT_NAME, CallAuthenticatedServlet.class); + return servletDeployment(SecurePortal.DEPLOYMENT_NAME, AdapterActionsFilter.class, CallAuthenticatedServlet.class); } @Deployment(name = SecurePortalRewriteRedirectUri.DEPLOYMENT_NAME) protected static WebArchive securePortalRewriteRedirectUri() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OIDCPublicKeyRotationAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OIDCPublicKeyRotationAdapterTest.java index 25c82174df2..e05994755b2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OIDCPublicKeyRotationAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OIDCPublicKeyRotationAdapterTest.java @@ -96,7 +96,7 @@ public class OIDCPublicKeyRotationAdapterTest extends AbstractServletsAdapterTes @Deployment(name = SecurePortal.DEPLOYMENT_NAME) protected static WebArchive securePortal() { - return servletDeployment(SecurePortal.DEPLOYMENT_NAME, CallAuthenticatedServlet.class); + return servletDeployment(SecurePortal.DEPLOYMENT_NAME, AdapterActionsFilter.class, CallAuthenticatedServlet.class); } @Deployment(name = TokenMinTTLPage.DEPLOYMENT_NAME) @@ -136,7 +136,6 @@ public class OIDCPublicKeyRotationAdapterTest extends AbstractServletsAdapterTes assertCurrentUrlStartsWithLoginUrlOf(testRealmPage); testRealmLoginPage.form().login("bburke@redhat.com", "password"); URLAssert.assertCurrentUrlStartsWith(tokenMinTTLPage.getInjectedUrl().toString()); - Assert.assertNull(tokenMinTTLPage.getAccessToken()); ApiUtil.findUserByUsernameId(adminClient.realm("demo"), "bburke@redhat.com").logout(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OfflineServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OfflineServletsAdapterTest.java index 27c596827ca..bfa2c0153a7 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OfflineServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/OfflineServletsAdapterTest.java @@ -34,9 +34,9 @@ import org.openqa.selenium.By; import java.io.Closeable; import java.io.IOException; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.keycloak.testsuite.auth.page.AuthRealm.TEST; @@ -72,6 +72,11 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest { private final String DEFAULT_PASSWORD = "password"; private final String OFFLINE_CLIENT_ID = "offline-client"; + /** + * URL in the deployment that doesn't require authentication and which therefore can be used to trigger activities in the actionfilter. + */ + private static final String UNSECURED_URL = "unsecured/foo"; + @Deployment(name = OfflineToken.DEPLOYMENT_NAME) protected static WebArchive offlineClient() { return servletDeployment(OfflineToken.DEPLOYMENT_NAME, AdapterActionsFilter.class, AbstractShowTokensServlet.class, OfflineTokenServlet.class, ErrorServlet.class, ServletTestUtils.class); @@ -114,7 +119,6 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest { assertCurrentUrlStartsWith(offlineTokenPage); RefreshToken refreshToken = offlineTokenPage.getRefreshToken(); - assertThat(refreshToken, notNullValue()); assertThat(TokenUtil.TOKEN_TYPE_OFFLINE, is(refreshToken.getType())); assertThat(refreshToken.getExp(), nullValue()); @@ -151,7 +155,7 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest { loginPage.assertCurrent(); } finally { events.clear(); - resetTimeOffsetAuthenticated(); + resetTimeOffset(); } } @@ -170,7 +174,6 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest { assertCurrentUrlStartsWith(offlineTokenPage); final RefreshToken refreshToken = offlineTokenPage.getRefreshToken(); - assertThat(refreshToken, notNullValue()); assertThat(refreshToken.getType(), is(TokenUtil.TOKEN_TYPE_OFFLINE)); // Assert refresh works with increased time @@ -200,7 +203,7 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest { loginPage.assertCurrent(); } finally { events.clear(); - resetTimeOffsetAuthenticated(); + resetTimeOffset(); } } @@ -235,7 +238,6 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest { assertCurrentUrlStartsWith(offlineTokenPage); RefreshToken refreshToken = offlineTokenPage.getRefreshToken(); - assertThat(refreshToken, notNullValue()); assertThat(refreshToken.getType(), is(TokenUtil.TOKEN_TYPE_OFFLINE)); // Check that the client scopes have been granted by the user @@ -247,39 +249,31 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest { AccountHelper.logout(adminClient.realm(TEST), DEFAULT_USERNAME); } finally { events.clear(); - resetTimeOffsetAuthenticated(); + resetTimeOffset(); } } private void setAdapterAndServerTimeOffset(int timeOffset) { - super.setAdapterAndServerTimeOffset(timeOffset, offlineTokenPage.toString()); + super.setAdapterAndServerTimeOffset(timeOffset, offlineTokenPage.toString() + UNSECURED_URL); } - private void resetTimeOffsetAuthenticated() { - resetTimeOffsetAuthenticated(DEFAULT_USERNAME, DEFAULT_PASSWORD); + @Override + public void resetTimeOffset() { + setAdapterServletTimeOffset(0, offlineTokenPage.toString() + UNSECURED_URL); + super.resetTimeOffset(); } - /** - * Reset time offset for remote environment. - * After the token expiration, process of re-authentication is necessary. - * - * @param username - * @param password - */ - private void resetTimeOffsetAuthenticated(String username, String password) { - if (testContext.getAppServerInfo().isUndertow()) { - setAdapterAndServerTimeOffset(0); - return; - } - super.setAdapterServletTimeOffset(0, offlineTokenPage.toString()); - if (loginPage.isCurrent()) { - loginPage.login(username, password); - waitForPageToLoad(); - AccountHelper.logout(adminClient.realm(TEST), DEFAULT_USERNAME); - } - setTimeOffset(0); - // Improve stability of the cleanup and have more time for synchronizing auth and app server living in different JVMs - pause(400); + @Override + protected void afterAbstractKeycloakTestRealmImport() { + // after each re-import, ensure that the information stored in JWKPublicKeyLocator is reset + String resetDeploymentUri = UriBuilder.fromUri(offlineTokenPage.toString() + UNSECURED_URL) + .queryParam(AdapterActionsFilter.RESET_DEPLOYMENT_PARAM, "true") + .build().toString(); + driver.navigate().to(resetDeploymentUri); + waitForPageToLoad(); + + assertThat(driver.getPageSource(), containsString("Restarted PublicKeyLocator")); + super.afterAbstractKeycloakTestRealmImport(); } - + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/offline-client/WEB-INF/web.xml b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/offline-client/WEB-INF/web.xml index 66acbedc6ca..e7a1003de51 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/offline-client/WEB-INF/web.xml +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/offline-client/WEB-INF/web.xml @@ -53,6 +53,13 @@ /error.html + + + Unsecured + /unsecured/* + + + Users diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/web.xml b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/web.xml index 80814292761..ace743619b1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/web.xml +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/secure-portal/WEB-INF/web.xml @@ -23,11 +23,21 @@ secure-portal + + AdapterActionsFilter + org.keycloak.testsuite.adapter.filter.AdapterActionsFilter + + Servlet org.keycloak.testsuite.adapter.servlet.CallAuthenticatedServlet + + AdapterActionsFilter + /* + + Servlet /*