Merge pull request #27968 from sberyozkin/flaky_oidc_code_flow_tests

Improve OIDC code flow tests
quarkusio · Sep 15, 2022 · 546568b · 546568b
2 parents 7f4d212 + 414519c
commit 546568b
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 30 deletions.
diff --git a/...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java b/...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java
@@ -250,7 +250,7 @@ public Uni<? extends SecurityIdentity> apply(Throwable t) {
                                             }
                                             if (!configContext.oidcConfig.token.refreshExpired) {
                                                 LOG.debug("Token has expired, token refresh is not allowed");
-                                                throw new AuthenticationCompletionException(t.getCause());
+                                                throw new AuthenticationFailedException(t.getCause());
                                             }
                                             LOG.debug("Token has expired, trying to refresh it");
                                             return refreshSecurityIdentity(configContext,
@@ -833,7 +833,7 @@ private Uni<SecurityIdentity> refreshSecurityIdentity(TenantConfigContext config
                     @Override
                     public Uni<SecurityIdentity> apply(final AuthorizationCodeTokens tokens, final Throwable t) {
                         if (t != null) {
-                            LOG.debugf("ID token refresh has failed: %s", t.getMessage());
+                            LOG.errorf("ID token refresh has failed: %s", t.getMessage());
                             if (autoRefresh) {
                                 LOG.debug("Using the current SecurityIdentity since the ID token is still valid");
                                 return Uni.createFrom().item(((TokenAutoRefreshException) t).getSecurityIdentity());

diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/ProtectedResource.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/ProtectedResource.java
@@ -174,12 +174,6 @@ public String getNameCallbackJwtNotUsedAfterRedirect() {
         throw new InternalServerErrorException("This method must not be invoked");
     }
 
-    @GET
-    @Path("tenant-logout")
-    public String getTenantLogout() {
-        return "Tenant Logout";
-    }
-
     @GET
     @Path("access")
     public String getAccessToken() {

diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantAutoRefresh.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantAutoRefresh.java
@@ -1,15 +1,20 @@
 package io.quarkus.it.keycloak;
 
+import javax.inject.Inject;
 import javax.ws.rs.GET;
 import javax.ws.rs.Path;
 
 import io.quarkus.security.Authenticated;
+import io.vertx.ext.web.RoutingContext;
 
 @Path("/tenant-autorefresh")
 public class TenantAutoRefresh {
+    @Inject
+    RoutingContext context;
+
     @Authenticated
     @GET
     public String getTenantLogout() {
-        return "Tenant AutoRefresh";
+        return "Tenant AutoRefresh, refreshed: " + (context.get("refresh_token_grant_response") != null);
     }
 }
diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantLogout.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantLogout.java
@@ -1,5 +1,6 @@
 package io.quarkus.it.keycloak;
 
+import javax.inject.Inject;
 import javax.ws.rs.GET;
 import javax.ws.rs.InternalServerErrorException;
 import javax.ws.rs.Path;
@@ -9,17 +10,21 @@
 import javax.ws.rs.core.HttpHeaders;
 
 import io.quarkus.security.Authenticated;
+import io.vertx.ext.web.RoutingContext;
 
 @Path("/tenant-logout")
 public class TenantLogout {
 
     @Context
     HttpHeaders headers;
 
+    @Inject
+    RoutingContext context;
+
     @Authenticated
     @GET
     public String getTenantLogout() {
-        return "Tenant Logout";
+        return "Tenant Logout, refreshed: " + (context.get("refresh_token_grant_response") != null);
     }
 
     // It is needed for the proactive-auth=false to work: /tenant-logout/logout should match a user initiated logout request

diff --git a/integration-tests/oidc-code-flow/src/main/resources/application.properties b/integration-tests/oidc-code-flow/src/main/resources/application.properties
@@ -86,6 +86,7 @@ quarkus.oidc.tenant-autorefresh.application-type=web-app
 quarkus.oidc.tenant-autorefresh.authentication.cookie-path=/tenant-autorefresh
 quarkus.oidc.tenant-autorefresh.token.refresh-expired=true
 quarkus.oidc.tenant-autorefresh.token.refresh-token-time-skew=30S
+quarkus.oidc.tenant-autorefresh.authentication.remove-redirect-parameters=false
 
 # Tenant which is used to test that the redirect_uri https scheme is enforced.
 quarkus.oidc.tenant-https.auth-server-url=${keycloak.url}/realms/quarkus

diff --git a/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java b/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java
@@ -413,7 +413,7 @@ public void testRPInitiatedLogout() throws IOException {
             loginForm.getInputByName("username").setValueAttribute("alice");
             loginForm.getInputByName("password").setValueAttribute("alice");
             page = loginForm.getInputByName("login").click();
-            assertTrue(page.asText().contains("Tenant Logout"));
+            assertEquals("Tenant Logout, refreshed: false", page.asText());
             assertNotNull(getSessionCookie(webClient, "tenant-logout"));
 
             page = webClient.getPage("http://localhost:8081/tenant-logout/logout");
@@ -422,11 +422,20 @@ public void testRPInitiatedLogout() throws IOException {
 
             page = webClient.getPage("http://localhost:8081/tenant-logout");
             assertEquals("Sign in to logout-realm", page.getTitleText());
-            loginForm = page.getForms().get(0);
+            webClient.getCookieManager().clearCookies();
+        }
+    }
+
+    @Test
+    public void testTokenRefresh() throws IOException {
+        try (final WebClient webClient = createWebClient()) {
+            HtmlPage page = webClient.getPage("http://localhost:8081/tenant-logout");
+            assertEquals("Sign in to logout-realm", page.getTitleText());
+            HtmlForm loginForm = page.getForms().get(0);
             loginForm.getInputByName("username").setValueAttribute("alice");
             loginForm.getInputByName("password").setValueAttribute("alice");
             page = loginForm.getInputByName("login").click();
-            assertTrue(page.asText().contains("Tenant Logout"));
+            assertEquals("Tenant Logout, refreshed: false", page.asText());
 
             Cookie sessionCookie = getSessionCookie(webClient, "tenant-logout");
             assertNotNull(sessionCookie);
@@ -445,13 +454,14 @@ public Boolean call() throws Exception {
                             // Should not redirect to OP but silently refresh token
                             Cookie newSessionCookie = getSessionCookie(webClient, "tenant-logout");
                             assertNotNull(newSessionCookie);
-                            return !idToken.equals(getIdToken(newSessionCookie));
+                            return webResponse.getContentAsString().equals("Tenant Logout, refreshed: true")
+                                    && !idToken.equals(getIdToken(newSessionCookie));
                         }
                     });
 
             // local session refreshed and still valid
             page = webClient.getPage("http://localhost:8081/tenant-logout");
-            assertTrue(page.asText().contains("Tenant Logout"));
+            assertEquals("Tenant Logout, refreshed: false", page.asText());
             assertNotNull(getSessionCookie(webClient, "tenant-logout"));
 
             //wait now so that we reach the refresh timeout
@@ -495,7 +505,7 @@ public void testTokenAutoRefresh() throws IOException {
             loginForm.getInputByName("username").setValueAttribute("alice");
             loginForm.getInputByName("password").setValueAttribute("alice");
             page = loginForm.getInputByName("login").click();
-            assertTrue(page.asText().contains("Tenant AutoRefresh"));
+            assertEquals("Tenant AutoRefresh, refreshed: false", page.asText());
 
             Cookie sessionCookie = getSessionCookie(webClient, "tenant-autorefresh");
             assertNotNull(sessionCookie);
@@ -504,24 +514,11 @@ public void testTokenAutoRefresh() throws IOException {
             // Auto-refresh-interval is 30 secs so every call auto-refreshes the token
             // Right now the original ID token is still valid but will be auto-refreshed
             page = webClient.getPage("http://localhost:8081/tenant-autorefresh");
-            assertTrue(page.getBody().asText().contains("Tenant AutoRefresh"));
+            assertEquals("Tenant AutoRefresh, refreshed: true", page.asText());
             sessionCookie = getSessionCookie(webClient, "tenant-autorefresh");
             assertNotNull(sessionCookie);
             String nextIdToken = getIdToken(sessionCookie);
             assertNotEquals(idToken, nextIdToken);
-            idToken = nextIdToken;
-
-            //wait now so that we reach the ID token timeout
-            await().atLeast(6, TimeUnit.SECONDS);
-
-            // ID token has expired, must be refreshed, auto-refresh-interval must not cause
-            // an auto-refresh loop even though it is larger than the ID token lifespan
-            page = webClient.getPage("http://localhost:8081/tenant-autorefresh");
-            assertTrue(page.getBody().asText().contains("Tenant AutoRefresh"));
-            sessionCookie = getSessionCookie(webClient, "tenant-autorefresh");
-            assertNotNull(sessionCookie);
-            nextIdToken = getIdToken(sessionCookie);
-            assertNotEquals(idToken, nextIdToken);
 
             webClient.getCookieManager().clearCookies();
         }