Merge pull request #12143 from sberyozkin/oidc_drop_query_with_ssl_te…

…rminating_proxy Set HTTPS scheme in the final OIDC redirect URI
quarkusio · Sep 17, 2020 · 3108690 · 3108690
2 parents 7d1b2db + ff222c7
commit 3108690
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 11 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
@@ -318,7 +318,8 @@ public void accept(SecurityIdentity identity) {
 
                                         if (configContext.oidcConfig.authentication.isRemoveRedirectParameters()
                                                 && context.request().query() != null) {
-                                            String finalRedirectUri = buildUriWithoutQueryParams(context);
+                                            String finalRedirectUri = buildUriWithoutQueryParams(context,
+                                                    isForceHttps(configContext));
                                             if (finalUserQuery != null) {
                                                 finalRedirectUri += ("?" + finalUserQuery);
                                             }
@@ -429,9 +430,10 @@ private String buildUri(RoutingContext context, boolean forceHttps, String path)
                 .toString();
     }
 
-    private String buildUriWithoutQueryParams(RoutingContext context) {
+    private String buildUriWithoutQueryParams(RoutingContext context, boolean forceHttps) {
+        final String scheme = forceHttps ? "https" : context.request().scheme();
         URI absoluteUri = URI.create(context.request().absoluteURI());
-        return new StringBuilder(context.request().scheme()).append("://")
+        return new StringBuilder(scheme).append("://")
                 .append(absoluteUri.getAuthority())
                 .append(absoluteUri.getRawPath())
                 .toString();

diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantHttps.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantHttps.java
@@ -0,0 +1,16 @@
+package io.quarkus.it.keycloak;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+
+import io.quarkus.security.Authenticated;
+
+@Path("/tenant-https")
+public class TenantHttps {
+
+    @Authenticated
+    @GET
+    public String getTenant() {
+        return "tenant-https";
+    }
+}
diff --git a/integration-tests/oidc-code-flow/src/main/resources/application.properties b/integration-tests/oidc-code-flow/src/main/resources/application.properties
@@ -83,7 +83,6 @@ quarkus.oidc.tenant-https.auth-server-url=${keycloak.url}/realms/quarkus
 quarkus.oidc.tenant-https.client-id=quarkus-app
 quarkus.oidc.tenant-https.credentials.secret=secret
 quarkus.oidc.tenant-https.authentication.scopes=profile,email,phone
-quarkus.oidc.tenant-https.authentication.redirect-path=/web-app
 quarkus.oidc.tenant-https.authentication.cookie-path=/
 quarkus.oidc.tenant-https.authentication.extra-params.max-age=60
 quarkus.oidc.tenant-https.application-type=web-app
@@ -104,9 +103,6 @@ quarkus.http.auth.permission.logout.policy=authenticated
 quarkus.http.auth.permission.logout.paths=/tenant-autorefresh
 quarkus.http.auth.permission.logout.policy=authenticated
 
-quarkus.http.auth.permission.https.paths=/tenant-https
-quarkus.http.auth.permission.https.policy=authenticated
-
 quarkus.http.auth.permission.xhr.paths=/tenant-xhr
 quarkus.http.auth.permission.xhr.policy=authenticated
 

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
@@ -42,7 +42,7 @@ public void testCodeFlowNoConsent() throws IOException {
             webClient.getOptions().setRedirectEnabled(false);
             WebResponse webResponse = webClient
                     .loadWebResponse(new WebRequest(URI.create("http://localhost:8081/index.html").toURL()));
-            verifyLocationHeader(webClient, webResponse.getResponseHeaderValue("location"), null, false);
+            verifyLocationHeader(webClient, webResponse.getResponseHeaderValue("location"), null, "web-app", false);
 
             webClient.getOptions().setRedirectEnabled(true);
             HtmlPage page = webClient.getPage("http://localhost:8081/index.html");
@@ -75,15 +75,46 @@ public void testCodeFlowForceHttpsRedirectUri() throws IOException {
             WebResponse webResponse = webClient
                     .loadWebResponse(
                             new WebRequest(URI.create("http://localhost:8081/tenant-https").toURL()));
-            verifyLocationHeader(webClient, webResponse.getResponseHeaderValue("location"), "tenant-https", true);
+            String keycloakUrl = webResponse.getResponseHeaderValue("location");
+            verifyLocationHeader(webClient, keycloakUrl, "tenant-https", "tenant-https",
+                    true);
+            HtmlPage page = webClient.getPage(keycloakUrl);
+
+            assertEquals("Log in to quarkus", page.getTitleText());
+            HtmlForm loginForm = page.getForms().get(0);
+            loginForm.getInputByName("username").setValueAttribute("alice");
+            loginForm.getInputByName("password").setValueAttribute("alice");
+
+            webClient.getOptions().setThrowExceptionOnFailingStatusCode(false);
+            webResponse = loginForm.getInputByName("login").click().getWebResponse();
+            webClient.getOptions().setThrowExceptionOnFailingStatusCode(true);
+
+            // This is a redirect from the OIDC server to the endpoint
+            String endpointLocation = webResponse.getResponseHeaderValue("location");
+            assertTrue(endpointLocation.startsWith("https"));
+            endpointLocation = "http" + endpointLocation.substring(5);
+            URI endpointLocationUri = URI.create(endpointLocation);
+            assertNotNull(endpointLocationUri.getRawQuery());
+            webResponse = webClient.loadWebResponse(new WebRequest(endpointLocationUri.toURL()));
+
+            // This is a redirect from quarkus-oidc which drops the query parameters
+            String endpointLocation2 = webResponse.getResponseHeaderValue("location");
+            assertTrue(endpointLocation2.startsWith("https"));
+
+            endpointLocation2 = "http" + endpointLocation2.substring(5);
+            URI endpointLocationUri2 = URI.create(endpointLocation2);
+            assertNull(endpointLocationUri2.getRawQuery());
+
+            page = webClient.getPage(endpointLocationUri2.toURL());
+            assertEquals("tenant-https", page.getBody().asText());
             webClient.getCookieManager().clearCookies();
         }
     }
 
-    private void verifyLocationHeader(WebClient webClient, String loc, String tenant, boolean forceHttps) {
+    private void verifyLocationHeader(WebClient webClient, String loc, String tenant, String path, boolean forceHttps) {
         assertTrue(loc.startsWith("http://localhost:8180/auth/realms/quarkus/protocol/openid-connect/auth"));
         String scheme = forceHttps ? "https" : "http";
-        assertTrue(loc.contains("redirect_uri=" + scheme + "%3A%2F%2Flocalhost%3A8081%2Fweb-app"));
+        assertTrue(loc.contains("redirect_uri=" + scheme + "%3A%2F%2Flocalhost%3A8081%2F" + path));
         assertTrue(loc.contains("state=" + getStateCookieStateParam(webClient, tenant)));
         assertTrue(loc.contains("scope=openid+profile+email+phone"));
         assertTrue(loc.contains("response_type=code"));