diff --git a/adapters/oidc/adapter-core/pom.xml b/adapters/oidc/adapter-core/pom.xml index a92ff5fd58..19c7794446 100755 --- a/adapters/oidc/adapter-core/pom.xml +++ b/adapters/oidc/adapter-core/pom.xml @@ -95,6 +95,12 @@ httpclient provided + + org.mockito + mockito-core + 2.7.16 + test + diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeployment.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeployment.java index ba7bc5d759..1ea1d5bc47 100755 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeployment.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeployment.java @@ -35,6 +35,8 @@ import java.util.Map; /** * @author Bill Burke + * @author Brad Culley + * @author John D. Ament * @version $Revision: 1 $ */ public class KeycloakDeployment { @@ -87,6 +89,7 @@ public class KeycloakDeployment { // https://tools.ietf.org/html/rfc7636 protected boolean pkce = false; + protected boolean ignoreOAuthQueryParameter; public KeycloakDeployment() { } @@ -427,4 +430,11 @@ public class KeycloakDeployment { this.pkce = pkce; } + public void setIgnoreOAuthQueryParameter(boolean ignoreOAuthQueryParameter) { + this.ignoreOAuthQueryParameter = ignoreOAuthQueryParameter; + } + + public boolean isOAuthQueryParameterEnabled() { + return !this.ignoreOAuthQueryParameter; + } } diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeploymentBuilder.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeploymentBuilder.java index 2fd92760c6..6791edd955 100755 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeploymentBuilder.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/KeycloakDeploymentBuilder.java @@ -37,6 +37,8 @@ import java.security.PublicKey; /** * @author Bill Burke + * @author Brad Culley + * @author John D. Ament * @version $Revision: 1 $ */ public class KeycloakDeploymentBuilder { @@ -112,6 +114,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"); diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/QueryParamterTokenRequestAuthenticator.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/QueryParamterTokenRequestAuthenticator.java index 5ee6662c1f..d2cabb3d75 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/QueryParamterTokenRequestAuthenticator.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/QueryParamterTokenRequestAuthenticator.java @@ -22,6 +22,8 @@ import org.keycloak.adapters.spi.HttpFacade; /** * @author Christian Froehlich + * @author Brad Culley + * @author John D. Ament * @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()) { diff --git a/adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/KeycloakDeploymentBuilderTest.java b/adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/KeycloakDeploymentBuilderTest.java index 233c1ed2f0..0d723a3c4b 100644 --- a/adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/KeycloakDeploymentBuilderTest.java +++ b/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 Stian Thorgersen + * @author Brad Culley + * @author John D. Ament */ public class KeycloakDeploymentBuilderTest { @@ -57,6 +60,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()); diff --git a/adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/KeycloakDeploymentTest.java b/adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/KeycloakDeploymentTest.java new file mode 100644 index 0000000000..3bb5bce2a3 --- /dev/null +++ b/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 Brad Culley + * @author John D. Ament + */ +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()); + } +} \ No newline at end of file diff --git a/adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/QueryParamterTokenRequestAuthenticatorTest.java b/adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/QueryParamterTokenRequestAuthenticatorTest.java new file mode 100644 index 0000000000..0193a56766 --- /dev/null +++ b/adapters/oidc/adapter-core/src/test/java/org/keycloak/adapters/QueryParamterTokenRequestAuthenticatorTest.java @@ -0,0 +1,99 @@ +/* + * 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.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.keycloak.adapters.spi.AuthOutcome; +import org.keycloak.adapters.spi.HttpFacade; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author Brad Culley + * @author John D. Ament + */ +@RunWith(MockitoJUnitRunner.class) +public class QueryParamterTokenRequestAuthenticatorTest { + @Mock + private KeycloakDeployment keycloakDeployment; + @Mock + private HttpFacade httpFacade; + + private QueryParamterTokenRequestAuthenticator queryParamterTokenRequestAuthenticator; + + @Before + public void setup() { + when(keycloakDeployment.getRealm()).thenReturn("realm"); + when(keycloakDeployment.isOAuthQueryParameterEnabled()).thenReturn(true); + this.queryParamterTokenRequestAuthenticator = spy(new QueryParamterTokenRequestAuthenticator(keycloakDeployment)); + } + + @Test + public void shouldReturnNotAttemptedWhenQueryParameterNotSet() { + doReturn(null).when(queryParamterTokenRequestAuthenticator).getAccessTokenFromQueryParamter(httpFacade); + + AuthOutcome authOutcome = queryParamterTokenRequestAuthenticator.authenticate(httpFacade); + + assertEquals(authOutcome, AuthOutcome.NOT_ATTEMPTED); + } + + @Test + public void shouldReturnNotAttemptedWhenQueryParameterIsEmptyString() { + doReturn("").when(queryParamterTokenRequestAuthenticator).getAccessTokenFromQueryParamter(httpFacade); + + AuthOutcome authOutcome = queryParamterTokenRequestAuthenticator.authenticate(httpFacade); + + assertEquals(authOutcome, AuthOutcome.NOT_ATTEMPTED); + verify(queryParamterTokenRequestAuthenticator).getAccessTokenFromQueryParamter(httpFacade); + } + + @Test + public void shouldDelegateToAuthenticateTokenWhenAccessTokenSet() { + final String accessToken = "test"; + final AuthOutcome expectedAuthOutcome = AuthOutcome.FAILED; + doReturn(accessToken).when(queryParamterTokenRequestAuthenticator).getAccessTokenFromQueryParamter(httpFacade); + doReturn(expectedAuthOutcome).when(queryParamterTokenRequestAuthenticator).authenticateToken(httpFacade, accessToken); + + AuthOutcome authOutcome = queryParamterTokenRequestAuthenticator.authenticate(httpFacade); + + assertEquals(authOutcome, expectedAuthOutcome); + verify(queryParamterTokenRequestAuthenticator).authenticateToken(httpFacade, accessToken); + } + + @Test + public void shouldNotAttemptQueryParamterLogicWhenQueryParameterIsDisabled() { + given(keycloakDeployment.isOAuthQueryParameterEnabled()).willReturn(false); + + AuthOutcome authOutcome = queryParamterTokenRequestAuthenticator.authenticate(httpFacade); + + verify(queryParamterTokenRequestAuthenticator, never()).getAccessTokenFromQueryParamter(any(HttpFacade.class)); + verify(queryParamterTokenRequestAuthenticator, never()).authenticateToken(any(HttpFacade.class), anyString()); + assertEquals(authOutcome, AuthOutcome.NOT_ATTEMPTED); + } +} \ No newline at end of file diff --git a/adapters/oidc/adapter-core/src/test/resources/keycloak.json b/adapters/oidc/adapter-core/src/test/resources/keycloak.json index 9f0a204826..400128b6e4 100644 --- a/adapters/oidc/adapter-core/src/test/resources/keycloak.json +++ b/adapters/oidc/adapter-core/src/test/resources/keycloak.json @@ -31,5 +31,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 } \ No newline at end of file diff --git a/core/src/main/java/org/keycloak/representations/adapters/config/AdapterConfig.java b/core/src/main/java/org/keycloak/representations/adapters/config/AdapterConfig.java index f063962a55..265ba40772 100755 --- a/core/src/main/java/org/keycloak/representations/adapters/config/AdapterConfig.java +++ b/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 Bill Burke + * @author Brad Culley + * @author John D. Ament * @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; + } }