Merge pull request #4066 from johnament/KEYCLOAK-4765

KEYCLOAK-4765 - Add ability to disable Query Parameter parsing.

adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeployment.java

@@ -35,6 +35,8 @@ import java.util.Map;
 
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @author <a href="mailto:brad.culley@spartasystems.com">Brad Culley</a>
+ * @author <a href="mailto:john.ament@spartasystems.com">John D. Ament</a>
  * @version $Revision: 1 $
  */
 public class KeycloakDeployment {
@@ -88,6 +90,7 @@ public class KeycloakDeployment {
 
     // https://tools.ietf.org/html/rfc7636
     protected boolean pkce = false;
+    protected boolean ignoreOAuthQueryParameter;
 
     public KeycloakDeployment() {
     }
@@ -436,4 +439,11 @@ public class KeycloakDeployment {
         this.pkce = pkce;
     }
 
+    public void setIgnoreOAuthQueryParameter(boolean ignoreOAuthQueryParameter) {
+        this.ignoreOAuthQueryParameter = ignoreOAuthQueryParameter;
+    }
+
+    public boolean isOAuthQueryParameterEnabled() {
+        return !this.ignoreOAuthQueryParameter;
+    }
 }

adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeploymentBuilder.java

@@ -37,6 +37,8 @@ import java.security.PublicKey;
 
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @author <a href="mailto:brad.culley@spartasystems.com">Brad Culley</a>
+ * @author <a href="mailto:john.ament@spartasystems.com">John D. Ament</a>
  * @version $Revision: 1 $
  */
 public class KeycloakDeploymentBuilder {
@@ -113,6 +115,7 @@ public class KeycloakDeploymentBuilder {
         deployment.setTokenMinimumTimeToLive(adapterConfig.getTokenMinimumTimeToLive());
         deployment.setMinTimeBetweenJwksRequests(adapterConfig.getMinTimeBetweenJwksRequests());
         deployment.setPublicKeyCacheTtl(adapterConfig.getPublicKeyCacheTtl());
+        deployment.setIgnoreOAuthQueryParameter(adapterConfig.isIgnoreOAuthQueryParameter());
 
         if (realmKeyPem == null && adapterConfig.isBearerOnly() && adapterConfig.getAuthServerUrl() == null) {
             throw new IllegalArgumentException("For bearer auth, you must set the realm-public-key or auth-server-url");

adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/QueryParamterTokenRequestAuthenticator.java

@@ -22,6 +22,8 @@ import org.keycloak.adapters.spi.HttpFacade;
 
 /**
  * @author <a href="mailto:froehlich.ch@gmail.com">Christian Froehlich</a>
+ * @author <a href="mailto:brad.culley@spartasystems.com">Brad Culley</a>
+ * @author <a href="mailto:john.ament@spartasystems.com">John D. Ament</a>
  * @version $Revision: 1 $
  */
 public class QueryParamterTokenRequestAuthenticator extends BearerTokenRequestAuthenticator {
@@ -33,6 +35,9 @@ public class QueryParamterTokenRequestAuthenticator extends BearerTokenRequestAu
     }
 
     public AuthOutcome authenticate(HttpFacade exchange) {
+        if(!deployment.isOAuthQueryParameterEnabled()) {
+            return AuthOutcome.NOT_ATTEMPTED;
+        }
         tokenString = null;
         tokenString = getAccessTokenFromQueryParamter(exchange);
         if (tokenString == null || tokenString.trim().isEmpty()) {

adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/KeycloakDeploymentBuilderTest.java

@@ -29,10 +29,13 @@ import org.keycloak.common.util.PemUtils;
 import org.keycloak.enums.TokenStore;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
+ * @author <a href="mailto:brad.culley@spartasystems.com">Brad Culley</a>
+ * @author <a href="mailto:john.ament@spartasystems.com">John D. Ament</a>
  */
 public class KeycloakDeploymentBuilderTest {
 
@@ -58,6 +61,7 @@ public class KeycloakDeploymentBuilderTest {
         assertTrue(deployment.isPublicClient());
         assertTrue(deployment.isEnableBasicAuth());
         assertTrue(deployment.isExposeToken());
+        assertFalse(deployment.isOAuthQueryParameterEnabled());
         assertEquals("234234-234234-234234", deployment.getResourceCredentials().get("secret"));
         assertEquals(ClientIdAndSecretCredentialsProvider.PROVIDER_ID, deployment.getClientAuthenticator().getId());
         assertEquals(20, ((ThreadSafeClientConnManager) deployment.getClient().getConnectionManager()).getMaxTotal());

adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/KeycloakDeploymentTest.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2016 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.adapters;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author <a href="mailto:brad.culley@spartasystems.com">Brad Culley</a>
+ * @author <a href="mailto:john.ament@spartasystems.com">John D. Ament</a>
+ */
+public class KeycloakDeploymentTest {
+    @Test
+    public void shouldNotEnableOAuthQueryParamWhenIgnoreIsTrue() {
+        KeycloakDeployment keycloakDeployment = new KeycloakDeployment();
+        keycloakDeployment.setIgnoreOAuthQueryParameter(true);
+        assertFalse(keycloakDeployment.isOAuthQueryParameterEnabled());
+    }
+
+    @Test
+    public void shouldEnableOAuthQueryParamWhenIgnoreIsFalse() {
+        KeycloakDeployment keycloakDeployment = new KeycloakDeployment();
+        keycloakDeployment.setIgnoreOAuthQueryParameter(false);
+        assertTrue(keycloakDeployment.isOAuthQueryParameterEnabled());
+    }
+
+    @Test
+    public void shouldEnableOAuthQueryParamWhenIgnoreNotSet() {
+        KeycloakDeployment keycloakDeployment = new KeycloakDeployment();
+
+        assertTrue(keycloakDeployment.isOAuthQueryParameterEnabled());
+    }
+}

adapters/oidc/adapter-core/src/test/resources/keycloak.json

@@ -32,5 +32,6 @@
     "principal-attribute": "email",
     "token-minimum-time-to-live": 10,
     "min-time-between-jwks-requests": 20,
-    "public-key-cache-ttl": 120
+    "public-key-cache-ttl": 120,
+    "ignore-oauth-query-parameter": true
 }

adapters/oidc/wildfly/wf8-subsystem/src/main/java/org/keycloak/subsystem/wf8/extension/SharedAttributeDefinitons.java

@@ -178,7 +178,14 @@ public class SharedAttributeDefinitons {
             .setXmlName("autodetect-bearer-only")
             .setAllowExpression(true)
             .setDefaultValue(new ModelNode(false))
-            .build(); 
+            .build();
+
+    protected static final SimpleAttributeDefinition IGNORE_OAUTH_QUERY_PARAMETER =
+            new SimpleAttributeDefinitionBuilder("ignore-oauth-query-parameter", ModelType.BOOLEAN, true)
+            .setXmlName("ignore-oauth-query-parameter")
+            .setAllowExpression(true)
+            .setDefaultValue(new ModelNode(false))
+            .build();
 
     protected static final List<SimpleAttributeDefinition> ATTRIBUTES = new ArrayList<SimpleAttributeDefinition>();
     static {
@@ -206,6 +213,7 @@ public class SharedAttributeDefinitons {
         ATTRIBUTES.add(TOKEN_STORE);
         ATTRIBUTES.add(PRINCIPAL_ATTRIBUTE);
         ATTRIBUTES.add(AUTODETECT_BEARER_ONLY);
+        ATTRIBUTES.add(IGNORE_OAUTH_QUERY_PARAMETER);
     }
 
     /**

adapters/oidc/wildfly/wf8-subsystem/src/main/resources/org/keycloak/subsystem/wf8/extension/LocalDescriptions.properties

@@ -48,6 +48,7 @@ keycloak.realm.register-node-period=how often to re-register node
 keycloak.realm.token-store=cookie or session storage for auth session data
 keycloak.realm.principal-attribute=token attribute to use to set Principal name
 keycloak.realm.autodetect-bearer-only=autodetect bearer-only requests
+keycloak.realm.ignore-oauth-query-parameter=disable query parameter parsing for access_token
 
 keycloak.secure-deployment=A deployment secured by Keycloak
 keycloak.secure-deployment.add=Add a deployment to be secured by Keycloak
@@ -86,6 +87,7 @@ keycloak.secure-deployment.turn-off-change-session-id-on-login=The session id is
 keycloak.secure-deployment.token-minimum-time-to-live=The adapter will refresh the token if the current token is expired OR will expire in 'token-minimum-time-to-live' seconds or less
 keycloak.secure-deployment.min-time-between-jwks-requests=If adapter recognize token signed by unknown public key, it will try to download new public key from keycloak server. However it won't try to download if already tried it in less than 'min-time-between-jwks-requests' seconds
 keycloak.secure-deployment.autodetect-bearer-only=autodetect bearer-only requests
+keycloak.secure-deployment.ignore-oauth-query-parameter=disable query parameter parsing for access_token
 
 keycloak.secure-deployment.credential=Credential value
 

adapters/oidc/wildfly/wf8-subsystem/src/main/resources/schema/wildfly-keycloak_1_1.xsd

@@ -67,6 +67,7 @@
             <xs:element name="token-store" type="xs:string" minOccurs="0" maxOccurs="1"/>
             <xs:element name="principal-attribute" type="xs:string" minOccurs="0" maxOccurs="1"/>
             <xs:element name="autodetect-bearer-only" type="xs:boolean" minOccurs="0" maxOccurs="1"/>
+            <xs:element name="ignore-oauth-query-parameter" type="xs:boolean" minOccurs="0" maxOccurs="1"/>
         </xs:all>
         <xs:attribute name="name" type="xs:string" use="required">
             <xs:annotation>
@@ -111,6 +112,7 @@
             <xs:element name="token-minimum-time-to-live" type="xs:integer" minOccurs="0" maxOccurs="1"/>
             <xs:element name="min-time-between-jwks-requests" type="xs:integer" minOccurs="0" maxOccurs="1"/>
             <xs:element name="autodetect-bearer-only" type="xs:boolean" minOccurs="0" maxOccurs="1"/>
+            <xs:element name="ignore-oauth-query-parameter" type="xs:boolean" minOccurs="0" maxOccurs="1"/>
         </xs:all>
         <xs:attribute name="name" type="xs:string" use="required">
             <xs:annotation>

adapters/oidc/wildfly/wildfly-subsystem/src/main/java/org/keycloak/subsystem/adapter/extension/SharedAttributeDefinitons.java

@@ -179,6 +179,12 @@ public class SharedAttributeDefinitons {
             .setDefaultValue(new ModelNode(false))
             .build();
 
+    protected static final SimpleAttributeDefinition IGNORE_OAUTH_QUERY_PARAMETER =
+            new SimpleAttributeDefinitionBuilder("ignore-oauth-query-parameter", ModelType.BOOLEAN, true)
+            .setXmlName("ignore-oauth-query-parameter")
+            .setAllowExpression(true)
+            .setDefaultValue(new ModelNode(false))
+            .build();
 
 
 
@@ -209,6 +215,7 @@ public class SharedAttributeDefinitons {
         ATTRIBUTES.add(TOKEN_STORE);
         ATTRIBUTES.add(PRINCIPAL_ATTRIBUTE);
         ATTRIBUTES.add(AUTODETECT_BEARER_ONLY);
+        ATTRIBUTES.add(IGNORE_OAUTH_QUERY_PARAMETER);
     }
 
     private static boolean isSet(ModelNode attributes, SimpleAttributeDefinition def) {

adapters/oidc/wildfly/wildfly-subsystem/src/main/resources/org/keycloak/subsystem/adapter/extension/LocalDescriptions.properties

@@ -48,6 +48,7 @@ keycloak.realm.register-node-period=how often to re-register node
 keycloak.realm.token-store=cookie or session storage for auth session data
 keycloak.realm.principal-attribute=token attribute to use to set Principal name
 keycloak.realm.autodetect-bearer-only=autodetect bearer-only requests
+keycloak.realm.ignore-oauth-query-parameter=disable query parameter parsing for access_token
 
 keycloak.secure-deployment=A deployment secured by Keycloak
 keycloak.secure-deployment.add=Add a deployment to be secured by Keycloak
@@ -86,6 +87,7 @@ keycloak.secure-deployment.principal-attribute=token attribute to use to set Pri
 keycloak.secure-deployment.turn-off-change-session-id-on-login=The session id is changed by default on a successful login.  Change this to true if you want to turn this off
 keycloak.secure-deployment.token-minimum-time-to-live=The adapter will refresh the token if the current token is expired OR will expire in 'token-minimum-time-to-live' seconds or less
 keycloak.secure-deployment.min-time-between-jwks-requests=If adapter recognize token signed by unknown public key, it will try to download new public key from keycloak server. However it won't try to download if already tried it in less than 'min-time-between-jwks-requests' seconds
+keycloak.secure-deployment.ignore-oauth-query-parameter=disable query parameter parsing for access_token
 
 keycloak.secure-deployment.credential=Credential value
 

adapters/oidc/wildfly/wildfly-subsystem/src/main/resources/schema/wildfly-keycloak_1_1.xsd

@@ -67,6 +67,7 @@
             <xs:element name="token-store" type="xs:string" minOccurs="0" maxOccurs="1"/>
             <xs:element name="principal-attribute" type="xs:string" minOccurs="0" maxOccurs="1"/>
             <xs:element name="autodetect-bearer-only" type="xs:boolean" minOccurs="0" maxOccurs="1"/>
+            <xs:element name="ignore-oauth-query-parameter" type="xs:boolean" minOccurs="0" maxOccurs="1"/>
         </xs:all>
         <xs:attribute name="name" type="xs:string" use="required">
             <xs:annotation>
@@ -111,6 +112,7 @@
             <xs:element name="token-minimum-time-to-live" type="xs:integer" minOccurs="0" maxOccurs="1"/>
             <xs:element name="min-time-between-jwks-requests" type="xs:integer" minOccurs="0" maxOccurs="1"/>
             <xs:element name="autodetect-bearer-only" type="xs:boolean" minOccurs="0" maxOccurs="1"/>
+            <xs:element name="ignore-oauth-query-parameter" type="xs:boolean" minOccurs="0" maxOccurs="1"/>
         </xs:all>
         <xs:attribute name="name" type="xs:string" use="required">
             <xs:annotation>

core/src/main/java/org/keycloak/representations/adapters/config/AdapterConfig.java

@@ -24,6 +24,8 @@ import com.fasterxml.jackson.annotation.JsonPropertyOrder;
  * Configuration for Java based adapters
  *
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @author <a href="mailto:brad.culley@spartasystems.com">Brad Culley</a>
+ * @author <a href="mailto:john.ament@spartasystems.com">John D. Ament</a>
  * @version $Revision: 1 $
  */
 @JsonPropertyOrder({"realm", "realm-public-key", "auth-server-url", "ssl-required",
@@ -38,7 +40,7 @@ import com.fasterxml.jackson.annotation.JsonPropertyOrder;
         "register-node-at-startup", "register-node-period", "token-store", "principal-attribute",
         "proxy-url", "turn-off-change-session-id-on-login", "token-minimum-time-to-live",
         "min-time-between-jwks-requests", "public-key-cache-ttl",
-        "policy-enforcer"
+        "policy-enforcer", "ignore-oauth-query-parameter"
 })
 public class AdapterConfig extends BaseAdapterConfig implements AdapterHttpClientConfig {
 
@@ -81,6 +83,8 @@ public class AdapterConfig extends BaseAdapterConfig implements AdapterHttpClien
     // https://tools.ietf.org/html/rfc7636
     @JsonProperty("enable-pkce")
     protected boolean pkce = false;
+    @JsonProperty("ignore-oauth-query-parameter")
+    protected boolean ignoreOAuthQueryParameter = false;
 
     /**
      * The Proxy url to use for requests to the auth-server, configurable via the adapter config property {@code proxy-url}.
@@ -257,4 +261,11 @@ public class AdapterConfig extends BaseAdapterConfig implements AdapterHttpClien
         this.pkce = pkce;
     }
 
+    public boolean isIgnoreOAuthQueryParameter() {
+        return ignoreOAuthQueryParameter;
+    }
+
+    public void setIgnoreOAuthQueryParameter(boolean ignoreOAuthQueryParameter) {
+        this.ignoreOAuthQueryParameter = ignoreOAuthQueryParameter;
+    }
 }

testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTest.java

@@ -19,20 +19,18 @@ package org.keycloak.testsuite.adapter;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
-import org.keycloak.common.util.Encode;
-import org.keycloak.common.util.KeycloakUriBuilder;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.services.managers.RealmManager;
 import org.keycloak.testsuite.rule.AbstractKeycloakRule;
 
 import java.net.URL;
-import java.security.PublicKey;
 
 /**
  * Tests Undertow Adapter
  *
  * @author <a href="mailto:bburke@redhat.com">Bill Burke</a>
+ * @author <a href="mailto:john.ament@spartasystems.com">John Ament</a>
  */
 public class AdapterTest {
 
@@ -93,6 +91,12 @@ public class AdapterTest {
                     .name("input-portal").contextPath("/input-portal")
                     .servletClass(InputServlet.class).adapterConfigPath(url.getPath())
                     .role("user").constraintUrl("/secured/*").deployApplication();
+
+            url = getClass().getResource("/adapter-test/no-access-token.json");
+            createApplicationDeployment()
+                    .name("no-access-token").contextPath("/no-access-token")
+                    .servletClass(InputServlet.class).adapterConfigPath(url.getPath())
+                    .role("user").constraintUrl("/secured/*").deployApplication();
         }
     };
 
@@ -237,4 +241,9 @@ public class AdapterTest {
         testStrategy.testRestCallWithAccessTokenAsQueryParameter();
 
     }
+
+    @Test
+    public void testCallURLWithAccessToken() throws Exception {
+        testStrategy.checkThatAccessTokenCanBeSentPublicly();
+    }
 }

testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java

@@ -67,6 +67,7 @@ import java.util.concurrent.atomic.AtomicInteger;
  * Tests Undertow Adapter
  *
  * @author <a href="mailto:bburke@redhat.com">Bill Burke</a>
+ * @author <a href="mailto:john.ament@spartasystems.com">John Ament</a>
  */
 public class AdapterTestStrategy extends ExternalResource {
 
@@ -814,4 +815,13 @@ public class AdapterTestStrategy extends ExternalResource {
 
     }
 
+    void checkThatAccessTokenCanBeSentPublicly() {
+        // test login to customer-portal which does a bearer request to customer-db
+        final String applicationURL = APP_SERVER_BASE_URL + "/no-access-token?access_token=invalid_token";
+        driver.navigate().to(applicationURL);
+        System.out.println("Current url: " + driver.getCurrentUrl());
+        Assert.assertEquals(applicationURL, driver.getCurrentUrl());
+        inputPage.execute("hello");
+    }
+
 }

testsuite/integration/src/test/resources/adapter-test/no-access-token.json

@@ -0,0 +1,11 @@
+{
+  "realm" : "demo",
+  "resource" : "no-access-token",
+  "realm-public-key" : "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCrVrCuTtArbgaZzL1hvh0xtL5mc7o0NqPVnYXkLvgcwiC3BjLGw1tGEGoJaXDuSaRllobm53JBhjx33UNv+5z/UMG4kytBWxheNVKnL6GgqlNabMaFfPLPCF8kAgKnsi79NMo+n6KnSY8YeUmec/p2vjO2NjsSAVcWEQMVhJ31LwIDAQAB",
+  "auth-server-url" : "http://${my.host.name}:8081/auth",
+  "ssl-required" : "external",
+  "credentials" : {
+    "secret": "password"
+  },
+  "ignore-oauth-query-parameter": true
+}