Skip to content

Commit

Permalink
[ELY-2340] Add the ability to allow query params in redirect URIs via…
Browse files Browse the repository at this point in the history
… a new system property
  • Loading branch information
fjuma committed May 16, 2024
1 parent 7b8abd8 commit dcbadba
Show file tree
Hide file tree
Showing 6 changed files with 258 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Boolean>() {
@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;
Expand Down Expand Up @@ -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<String, String> rewriteRules = deployment.getRedirectRewriteRules();
if (ALLOW_QUERY_PARAMS_PROPERTY && (rewriteRules == null || rewriteRules.isEmpty())) {
return originalUri;
}
try {
URL url = new URL(originalUri);
Map.Entry<String, String> rule = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,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<String, Object> props = new HashMap<>();
OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(oidcConfig);
Expand All @@ -362,7 +376,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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <a href="mailto:[email protected]">Farah Juma</a>
*/
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();
}
}

}
Original file line number Diff line number Diff line change
@@ -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 <a href="mailto:[email protected]">Farah Juma</a>
*/
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);
}

}
Original file line number Diff line number Diff line change
@@ -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 <a href="mailto:[email protected]">Farah Juma</a>
*/
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);
}

}

0 comments on commit dcbadba

Please sign in to comment.