diff --git a/http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java b/http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java index 2052af1a0c..f42313b7f5 100644 --- a/http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java +++ b/http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java @@ -65,6 +65,7 @@ public class Oidc { public static final String FACES_REQUEST = "Faces-Request"; public static final String GRANT_TYPE = "grant_type"; public static final String INVALID_TOKEN = "invalid_token"; + public static final String ISSUER = "iss"; public static final String LOGIN_HINT = "login_hint"; public static final String DOMAIN_HINT = "domain_hint"; public static final String MAX_AGE = "max_age"; @@ -113,6 +114,7 @@ public class Oidc { static final String KEYCLOAK_QUERY_BEARER_TOKEN = "k_query_bearer_token"; static final String DEFAULT_TOKEN_SIGNATURE_ALGORITHM = "RS256"; public static final String DISABLE_TYP_CLAIM_VALIDATION_PROPERTY_NAME = "wildfly.elytron.oidc.disable.typ.claim.validation"; + public static final String ALLOW_QUERY_PARAMS_PROPERTY_NAME = "wildfly.elytron.oidc.allow.query.params"; public static final String X_REQUESTED_WITH = "X-Requested-With"; public static final String XML_HTTP_REQUEST = "XMLHttpRequest"; diff --git a/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java b/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java index dbb3f05687..bf67e93859 100644 --- a/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java +++ b/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java @@ -19,10 +19,12 @@ package org.wildfly.security.http.oidc; import static org.wildfly.security.http.oidc.ElytronMessages.log; +import static org.wildfly.security.http.oidc.Oidc.ALLOW_QUERY_PARAMS_PROPERTY_NAME; import static org.wildfly.security.http.oidc.Oidc.CLIENT_ID; import static org.wildfly.security.http.oidc.Oidc.CODE; import static org.wildfly.security.http.oidc.Oidc.DOMAIN_HINT; import static org.wildfly.security.http.oidc.Oidc.ERROR; +import static org.wildfly.security.http.oidc.Oidc.ISSUER; import static org.wildfly.security.http.oidc.Oidc.KC_IDP_HINT; import static org.wildfly.security.http.oidc.Oidc.LOGIN_HINT; import static org.wildfly.security.http.oidc.Oidc.MAX_AGE; @@ -43,6 +45,8 @@ import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; @@ -74,6 +78,17 @@ public class OidcRequestAuthenticator { protected String refreshToken; protected String strippedOauthParametersRequestUri; + static final boolean ALLOW_QUERY_PARAMS_PROPERTY; + + static { + ALLOW_QUERY_PARAMS_PROPERTY = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Boolean run() { + return Boolean.parseBoolean(System.getProperty(ALLOW_QUERY_PARAMS_PROPERTY_NAME, "false")); + } + }); + } + public OidcRequestAuthenticator(RequestAuthenticator requestAuthenticator, OidcHttpFacade facade, OidcClientConfiguration deployment, int sslRedirectPort, OidcTokenStore tokenStore) { this.reqAuthenticator = requestAuthenticator; this.facade = facade; @@ -375,11 +390,15 @@ protected AuthChallenge resolveCode(String code) { private static String stripOauthParametersFromRedirect(String uri) { uri = stripQueryParam(uri, CODE); uri = stripQueryParam(uri, STATE); - return stripQueryParam(uri, SESSION_STATE); + uri = stripQueryParam(uri, SESSION_STATE); + return stripQueryParam(uri, ISSUER); } private String rewrittenRedirectUri(String originalUri) { Map rewriteRules = deployment.getRedirectRewriteRules(); + if (ALLOW_QUERY_PARAMS_PROPERTY && (rewriteRules == null || rewriteRules.isEmpty())) { + return originalUri; + } try { URL url = new URL(originalUri); Map.Entry rule = null; diff --git a/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java b/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java index d2e4e4576e..f6374d4b5b 100644 --- a/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java +++ b/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java @@ -331,11 +331,25 @@ protected void checkForScopeClaims(Callback callback, String expectedScopes) thr // Note: The tests will fail if `localhost` is not listed first in `/etc/hosts` file for the loopback addresses (IPv4 and IPv6). protected void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, int expectedDispatcherStatusCode, String expectedLocation, String clientPageText) throws Exception { - performAuthentication(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, expectedLocation, clientPageText, null, false); + performAuthentication(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, getClientUrl(), expectedLocation, + clientPageText, null, false); + } + + protected void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, + int expectedDispatcherStatusCode, String clientUrl, String expectedLocation, String clientPageText) throws Exception { + performAuthentication(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, clientUrl, expectedLocation, + clientPageText, null, false); + } + + protected void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, int expectedDispatcherStatusCode, + String expectedLocation, String clientPageText, String expectedScope, boolean checkInvalidScopeError) throws Exception { + performAuthentication(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, getClientUrl(), expectedLocation, clientPageText, + expectedScope, checkInvalidScopeError); } private void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, - int expectedDispatcherStatusCode, String expectedLocation, String clientPageText, String expectedScope, boolean checkInvalidScopeError) throws Exception { + int expectedDispatcherStatusCode, String clientUrl, String expectedLocation, String clientPageText, + String expectedScope, boolean checkInvalidScopeError) throws Exception { try { Map props = new HashMap<>(); OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(oidcConfig); @@ -350,7 +364,7 @@ private void performAuthentication(InputStream oidcConfig, String username, Stri mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler(true, expectedScope)); } - URI requestUri = new URI(getClientUrl()); + URI requestUri = new URI(clientUrl); TestingHttpServerRequest request = new TestingHttpServerRequest(null, requestUri); mechanism.evaluateRequest(request); TestingHttpServerResponse response = request.getResponse(); diff --git a/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsBaseTest.java b/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsBaseTest.java new file mode 100644 index 0000000000..e6bb2762ed --- /dev/null +++ b/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsBaseTest.java @@ -0,0 +1,61 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2024 Red Hat, Inc., and individual 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.wildfly.security.http.oidc; + +import static org.junit.Assume.assumeTrue; + +import org.junit.AfterClass; +import org.junit.BeforeClass; + +import io.restassured.RestAssured; +import okhttp3.mockwebserver.MockWebServer; + +/** + * Tests for the {@code wildfly.elytron.oidc.allow.query.params} system property. + * + * @author Farah Juma + */ +public class QueryParamsBaseTest extends OidcBaseTest { + + @BeforeClass + public static void startTestContainers() throws Exception { + assumeTrue("Docker isn't available, OIDC tests will be skipped", isDockerAvailable()); + KEYCLOAK_CONTAINER = new KeycloakContainer(); + KEYCLOAK_CONTAINER.start(); + sendRealmCreationRequest(KeycloakConfiguration.getRealmRepresentation(TEST_REALM, CLIENT_ID, CLIENT_SECRET, CLIENT_HOST_NAME, CLIENT_PORT, CLIENT_APP, 3, 3, false, true)); + client = new MockWebServer(); + client.start(CLIENT_PORT); + } + + @AfterClass + public static void generalCleanup() throws Exception { + if (KEYCLOAK_CONTAINER != null) { + RestAssured + .given() + .auth().oauth2(KeycloakConfiguration.getAdminAccessToken(KEYCLOAK_CONTAINER.getAuthServerUrl())) + .when() + .delete(KEYCLOAK_CONTAINER.getAuthServerUrl() + "/admin/realms/" + TEST_REALM).then().statusCode(204); + KEYCLOAK_CONTAINER.stop(); + } + if (client != null) { + client.shutdown(); + } + } + +} diff --git a/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsDisabledTest.java b/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsDisabledTest.java new file mode 100644 index 0000000000..f32771d381 --- /dev/null +++ b/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsDisabledTest.java @@ -0,0 +1,74 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2024 Red Hat, Inc., and individual 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.wildfly.security.http.oidc; + +import static org.junit.Assume.assumeFalse; +import static org.wildfly.security.http.oidc.Oidc.ALLOW_QUERY_PARAMS_PROPERTY_NAME; + +import org.apache.http.HttpStatus; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * Tests for disabling query params via the {@code wildfly.elytron.oidc.allow.query.params} system property. + * + * @author Farah Juma + */ +public class QueryParamsDisabledTest extends QueryParamsBaseTest { + + @BeforeClass + public static void beforeClass() { + assumeFalse("wildfly.elytron.oidc.allow.query.params should default to false", + Boolean.parseBoolean(System.getProperty(ALLOW_QUERY_PARAMS_PROPERTY_NAME))); + } + + /** + * Test successfully logging in without query params included in the URL. + */ + @Test + public void testSuccessfulAuthenticationWithoutQueryParamsWithSystemPropertyDisabled() throws Exception { + String originalUrl = getClientUrl(); + String expectedUrlAfterRedirect = originalUrl; + performAuthentication(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, originalUrl, + expectedUrlAfterRedirect, CLIENT_PAGE_TEXT); + } + + /** + * Test successfully logging in with query params included in the URL. + * The query params should not be present upon redirect. + */ + @Test + public void testSuccessfulAuthenticationWithQueryParamsWithSystemPropertyDisabled() throws Exception { + String queryParams = "?myparam=abc"; + String originalUrl = getClientUrl() + queryParams; + String expectedUrlAfterRedirect = getClientUrl(); + performAuthentication(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, + originalUrl, expectedUrlAfterRedirect, CLIENT_PAGE_TEXT); + + queryParams = "?one=abc&two=def&three=ghi"; + originalUrl = getClientUrl() + queryParams; + expectedUrlAfterRedirect = getClientUrl(); + performAuthentication(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, originalUrl, + expectedUrlAfterRedirect, CLIENT_PAGE_TEXT); + } + +} diff --git a/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsEnabledTest.java b/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsEnabledTest.java new file mode 100644 index 0000000000..d16cc998ff --- /dev/null +++ b/http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsEnabledTest.java @@ -0,0 +1,84 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2024 Red Hat, Inc., and individual 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.wildfly.security.http.oidc; + +import static org.wildfly.security.http.oidc.Oidc.ALLOW_QUERY_PARAMS_PROPERTY_NAME; + +import org.apache.http.HttpStatus; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * Tests for enabling query params via the {@code wildfly.elytron.oidc.allow.query.params} system property. + * + * @author Farah Juma + */ +public class QueryParamsEnabledTest extends QueryParamsBaseTest { + + private static String ALLOW_QUERY_PARAMS_PROPERTY; + + @BeforeClass + public static void beforeClass() { + ALLOW_QUERY_PARAMS_PROPERTY = System.setProperty(ALLOW_QUERY_PARAMS_PROPERTY_NAME, "true"); + } + + @AfterClass + public static void afterClass() { + if (ALLOW_QUERY_PARAMS_PROPERTY == null) { + System.clearProperty(ALLOW_QUERY_PARAMS_PROPERTY_NAME); + } else { + System.setProperty(ALLOW_QUERY_PARAMS_PROPERTY_NAME, ALLOW_QUERY_PARAMS_PROPERTY); + } + } + + /** + * Test successfully logging in without query params included in the URL. + */ + @Test + public void testSuccessfulAuthenticationWithoutQueryParamsWithSystemPropertyEnabled() throws Exception { + String originalUrl = getClientUrl(); + String expectedUrlAfterRedirect = originalUrl; + performAuthentication(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, originalUrl, + expectedUrlAfterRedirect, CLIENT_PAGE_TEXT); + } + + /** + * Test successfully logging in with query params included in the URL. + * The query params should be present upon redirect. + */ + @Test + public void testSuccessfulAuthenticationWithQueryParamsWithSystemPropertyEnabled() throws Exception { + String queryParams = "?myparam=abc"; + String originalUrl = getClientUrl() + queryParams; + String expectedUrlAfterRedirect = originalUrl; + performAuthentication(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, originalUrl, + expectedUrlAfterRedirect, CLIENT_PAGE_TEXT); + + queryParams = "?one=abc&two=def&three=ghi"; + originalUrl = getClientUrl() + queryParams; + expectedUrlAfterRedirect = originalUrl; + performAuthentication(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, originalUrl, + expectedUrlAfterRedirect, CLIENT_PAGE_TEXT); + } + +}