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 1, 2024
1 parent 949c04e commit 5aba217
Show file tree
Hide file tree
Showing 6 changed files with 250 additions and 3 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.List;
Expand Down Expand Up @@ -72,6 +76,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 @@ -369,11 +384,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 @@ -299,7 +299,12 @@ protected String getCookieString(HttpServerCookie cookie) {
}

protected void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak,
int expectedDispatcherStatusCode, String expectedLocation, String clientPageText) throws Exception {
int expectedDispatcherStatusCode, String expectedLocation, String clientPageText) throws Exception {
performAuthentication(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, getClientUrl(), expectedLocation, clientPageText);
}

protected void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak,
int expectedDispatcherStatusCode, String clientUrl, String expectedLocation, String clientPageText) throws Exception {
try {
Map<String, Object> props = new HashMap<>();
OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(oidcConfig);
Expand All @@ -309,7 +314,7 @@ protected void performAuthentication(InputStream oidcConfig, String username, St
oidcFactory = new OidcMechanismFactory(oidcClientContext);
HttpServerAuthenticationMechanism mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler());

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:fjuma@redhat.com">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, 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,75 @@
/*
* 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:fjuma@redhat.com">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,85 @@
/*
* 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:fjuma@redhat.com">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 5aba217

Please sign in to comment.