Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ELY-2340] Add the ability to allow query params in redirect URIs via a new system property #2135

Merged
merged 2 commits into from
May 16, 2024
Merged

[ELY-2340] Add the ability to allow query params in redirect URIs via a new system property #2135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b8abd8
[ELY-2340] Move some test methods to OidcBaseTest
fjuma May 1, 2024
dcbadba
[ELY-2340] Add the ability to allow query params in redirect URIs via…
fjuma May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java
View file
Open in desktop
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
21 changes: 20 additions & 1 deletion http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion http/oidc/src/test/java/org/wildfly/security/http/oidc/BearerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ private InputStream getOidcConfigurationInputStream(String authServerUrl) {
return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8));
}

private InputStream getOidcConfigurationInputStreamWithProviderUrl() {
protected InputStream getOidcConfigurationInputStreamWithProviderUrl() {
String oidcConfig = "{\n" +
" \"client-id\" : \"" + BEARER_ONLY_CLIENT_ID + "\",\n" +
" \"provider-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" +
Expand Down
84 changes: 84 additions & 0 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,14 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.wildfly.security.http.oidc.Oidc.OIDC_NAME;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -54,6 +59,7 @@
import org.wildfly.security.jose.util.JsonSerialization;

import com.gargoylesoftware.htmlunit.SilentCssErrorHandler;
import com.gargoylesoftware.htmlunit.TextPage;
import com.gargoylesoftware.htmlunit.WebClient;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlInput;
Expand All @@ -64,6 +70,7 @@
import okhttp3.mockwebserver.Dispatcher;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.QueueDispatcher;
import okhttp3.mockwebserver.RecordedRequest;

/**
Expand Down Expand Up @@ -332,4 +339,81 @@ 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, 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 clientUrl, String expectedLocation, String clientPageText,
String expectedScope, boolean checkInvalidScopeError) throws Exception {
try {
Map<String, Object> props = new HashMap<>();
OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(oidcConfig);
assertEquals(OidcClientConfiguration.RelativeUrlsUsed.NEVER, oidcClientConfiguration.getRelativeUrls());

OidcClientContext oidcClientContext = new OidcClientContext(oidcClientConfiguration);
oidcFactory = new OidcMechanismFactory(oidcClientContext);
HttpServerAuthenticationMechanism mechanism;
if (expectedScope == null) {
mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler());
} else {
mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler(true, expectedScope));
}

URI requestUri = new URI(clientUrl);
TestingHttpServerRequest request = new TestingHttpServerRequest(null, requestUri);
mechanism.evaluateRequest(request);
TestingHttpServerResponse response = request.getResponse();
assertEquals(loginToKeycloak ? HttpStatus.SC_MOVED_TEMPORARILY : HttpStatus.SC_FORBIDDEN, response.getStatusCode());
assertEquals(Status.NO_AUTH, request.getResult());
if (expectedScope != null) {
assertTrue(response.getFirstResponseHeaderValue("Location").contains("scope=" + expectedScope));
}

if (loginToKeycloak) {
client.setDispatcher(createAppResponse(mechanism, expectedDispatcherStatusCode, expectedLocation, clientPageText));

if (checkInvalidScopeError) {
WebClient webClient = getWebClient();
TextPage keycloakLoginPage = webClient.getPage(response.getLocation());
assertTrue(keycloakLoginPage.getWebResponse().getWebRequest().toString().contains("error_description=Invalid+scopes"));
} else {
TextPage page = loginToKeycloak(username, password, requestUri, response.getLocation(),
response.getCookies()).click();
assertTrue(page.getContent().contains(clientPageText));
}
}
} finally {
client.setDispatcher(new QueueDispatcher());
}
}

protected InputStream getOidcConfigurationInputStreamWithProviderUrl() {
String oidcConfig = "{\n" +
" \"resource\" : \"" + CLIENT_ID + "\",\n" +
" \"public-client\" : \"false\",\n" +
" \"provider-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" +
" \"ssl-required\" : \"EXTERNAL\",\n" +
" \"credentials\" : {\n" +
" \"secret\" : \"" + CLIENT_SECRET + "\"\n" +
" }\n" +
"}";
return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8));
}
}
93 changes: 0 additions & 93 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,37 +237,6 @@ public void testOpenIDWithMultipleScopeValue() throws Exception {
true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, expectedScope, false);
}

// Note: The tests will fail if `localhost` is not listed first in `/etc/hosts` file for the loopback addresses (IPv4 and IPv6).
private 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);
}

private void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak,
int expectedDispatcherStatusCode, String expectedLocation, String clientPageText,
CallbackHandler callbackHandler) throws Exception {
performAuthentication(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, expectedLocation,
clientPageText, null, false, callbackHandler);
}

@Test
public void testPrincipalAttribute() throws Exception {
// custom principal-attribute
performAuthentication(getOidcConfigurationInputStreamWithPrincipalAttribute("aud"), KeycloakConfiguration.ALICE,
KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT,
getCallbackHandler( "test-webapp"));

// standard principal-attribute
performAuthentication(getOidcConfigurationInputStreamWithPrincipalAttribute("given_name"), KeycloakConfiguration.ALICE,
KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT,
getCallbackHandler("Alice"));

// invalid principal-attribute, logging in should still succeed
performAuthentication(getOidcConfigurationInputStreamWithPrincipalAttribute("invalid_claim"), KeycloakConfiguration.ALICE,
KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT,
getCallbackHandler());
}

/*****************************************************************************************************************************************
* Tests for multi-tenancy.
*
Expand Down Expand Up @@ -527,55 +496,6 @@ private void performTenantRequest(String username, String password, String tenan
}
}

private 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, expectedLocation,
clientPageText, expectedScope, checkInvalidScopeError, getCallbackHandler(checkInvalidScopeError,
expectedScope, null));
}

private void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak,
int expectedDispatcherStatusCode, String expectedLocation, String clientPageText,
String expectedScope, boolean checkInvalidScopeError,
CallbackHandler callbackHandler) throws Exception {
try {
Map<String, Object> props = new HashMap<>();
OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(oidcConfig);
assertEquals(OidcClientConfiguration.RelativeUrlsUsed.NEVER, oidcClientConfiguration.getRelativeUrls());

OidcClientContext oidcClientContext = new OidcClientContext(oidcClientConfiguration);
oidcFactory = new OidcMechanismFactory(oidcClientContext);
HttpServerAuthenticationMechanism mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, callbackHandler);

URI requestUri = new URI(getClientUrl());
TestingHttpServerRequest request = new TestingHttpServerRequest(null, requestUri);
mechanism.evaluateRequest(request);
TestingHttpServerResponse response = request.getResponse();
assertEquals(loginToKeycloak ? HttpStatus.SC_MOVED_TEMPORARILY : HttpStatus.SC_FORBIDDEN, response.getStatusCode());
assertEquals(Status.NO_AUTH, request.getResult());
if (expectedScope != null) {
assertTrue(response.getFirstResponseHeaderValue("Location").contains("scope=" + expectedScope));
}

if (loginToKeycloak) {
client.setDispatcher(createAppResponse(mechanism, expectedDispatcherStatusCode, expectedLocation, clientPageText));

if (checkInvalidScopeError) {
WebClient webClient = getWebClient();
TextPage keycloakLoginPage = webClient.getPage(response.getLocation());
assertTrue(keycloakLoginPage.getWebResponse().getWebRequest().toString().contains("error_description=Invalid+scopes"));
} else {
TextPage page = loginToKeycloak(username, password, requestUri, response.getLocation(),
response.getCookies()).click();
assertTrue(page.getContent().contains(clientPageText));
}
}
} finally {
client.setDispatcher(new QueueDispatcher());
}
}

private InputStream getOidcConfigurationInputStream() {
return getOidcConfigurationInputStream(CLIENT_SECRET);
}
Expand All @@ -598,19 +518,6 @@ private InputStream getOidcConfigurationInputStream(String clientSecret, String
return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8));
}

private InputStream getOidcConfigurationInputStreamWithProviderUrl() {
String oidcConfig = "{\n" +
" \"resource\" : \"" + CLIENT_ID + "\",\n" +
" \"public-client\" : \"false\",\n" +
" \"provider-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" +
" \"ssl-required\" : \"EXTERNAL\",\n" +
" \"credentials\" : {\n" +
" \"secret\" : \"" + CLIENT_SECRET + "\"\n" +
" }\n" +
"}";
return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8));
}

private InputStream getOidcConfigurationInputStreamWithEnvironmentVariableExpression() {
String oidcConfig = "{\n" +
" \"resource\" : \"" + CLIENT_ID + "\",\n" +
Expand Down
61 changes: 61 additions & 0 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/QueryParamsBaseTest.java
View file
Open in desktop
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();
}
}

}
Loading
Loading