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.

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 #2136

Merged
merged 3 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -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
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 All @@ -49,6 +54,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 @@ -59,6 +65,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 @@ -291,4 +298,50 @@ protected String getCookieString(HttpServerCookie cookie) {
return header.toString();
}

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);
}

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);
assertEquals(OidcClientConfiguration.RelativeUrlsUsed.NEVER, oidcClientConfiguration.getRelativeUrls());

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

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 (loginToKeycloak) {
client.setDispatcher(createAppResponse(mechanism, expectedDispatcherStatusCode, expectedLocation, clientPageText));
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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,13 @@ public void testWrongPassword() throws Exception {

@Test
public void testWrongAuthServerUrl() throws Exception {
loginToAppMultiTenancy(getOidcConfigurationInputStream(CLIENT_SECRET, "http://fakeauthserver/auth"), KeycloakConfiguration.ALICE,
performAuthentication(getOidcConfigurationInputStream(CLIENT_SECRET, "http://fakeauthserver/auth"), KeycloakConfiguration.ALICE,
KeycloakConfiguration.ALICE_PASSWORD, false, -1, null, null);
}

@Test
public void testWrongClientSecret() throws Exception {
loginToAppMultiTenancy(getOidcConfigurationInputStream("WRONG_CLIENT_SECRET"), KeycloakConfiguration.ALICE,
performAuthentication(getOidcConfigurationInputStream("WRONG_CLIENT_SECRET"), KeycloakConfiguration.ALICE,
KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_FORBIDDEN, null,"Forbidden");
}

Expand All @@ -149,19 +149,19 @@ public void testMissingRequiredConfigurationOption() {

@Test
public void testSucessfulAuthenticationWithAuthServerUrl() throws Exception {
loginToAppMultiTenancy(getOidcConfigurationInputStream(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
performAuthentication(getOidcConfigurationInputStream(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT);
}

@Test
public void testSucessfulAuthenticationWithProviderUrl() throws Exception {
loginToAppMultiTenancy(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
performAuthentication(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT);
}

@Test
public void testSucessfulAuthenticationWithProviderUrlTrailingSlash() throws Exception {
loginToAppMultiTenancy(getOidcConfigurationInputStreamWithProviderUrlTrailingSlash(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
performAuthentication(getOidcConfigurationInputStreamWithProviderUrlTrailingSlash(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT);
}

Expand All @@ -171,20 +171,20 @@ public void testSucessfulAuthenticationWithEnvironmentVariableExpression() throw
String providerUrlEnv = System.getenv("OIDC_PROVIDER_URL_ENV");
assertEquals(oidcProviderUrl, providerUrlEnv);

loginToAppMultiTenancy(getOidcConfigurationInputStreamWithEnvironmentVariableExpression(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
performAuthentication(getOidcConfigurationInputStreamWithEnvironmentVariableExpression(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT);
}

@Test
public void testSucessfulAuthenticationWithSystemPropertyExpression() throws Exception {
loginToAppMultiTenancy(getOidcConfigurationInputStreamWithSystemPropertyExpression(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
performAuthentication(getOidcConfigurationInputStreamWithSystemPropertyExpression(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT);
}

@Test
public void testTokenSignatureAlgorithm() throws Exception {
// keycloak uses RS256
loginToAppMultiTenancy(getOidcConfigurationInputStreamWithTokenSignatureAlgorithm(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
performAuthentication(getOidcConfigurationInputStreamWithTokenSignatureAlgorithm(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT);
}

Expand Down Expand Up @@ -363,35 +363,6 @@ private void testNonExistingUser(String username, String password, String tenant
assertTrue(page.getBody().asText().contains("Invalid username or password"));
}

private void loginToAppMultiTenancy(InputStream oidcConfig, String username, String password, boolean loginToKeycloak,
int expectedDispatcherStatusCode, String expectedLocation, String clientPageText) 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, getCallbackHandler());

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 (loginToKeycloak) {
client.setDispatcher(createAppResponse(mechanism, expectedDispatcherStatusCode, expectedLocation, clientPageText));
TextPage page = loginToKeycloak(username, password, requestUri, response.getLocation(),
response.getCookies()).click();
assertTrue(page.getContent().contains(clientPageText));
}
} finally {
client.setDispatcher(new QueueDispatcher());
}
}

private void performTenantRequestWithAuthServerUrl(String username, String password, String tenant, String otherTenant) throws Exception {
performTenantRequest(username, password, tenant, otherTenant, true);
}
Expand Down Expand Up @@ -467,19 +438,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
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();
}
}

}