libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Fix error response for invalid characters (#11533)

Browse source 
Fixes #11530
This commit is contained in:
Martin Bartoš 2022-04-20 11:26:08 +02:00 committed by GitHub
parent 7986872ae0
commit 3aa3db16ea
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 311 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
services/src/main/java/org/keycloak/services/error/KcUnrecognizedPropertyExceptionHandler.java Normal file
View file

@ -0,0 +1,46 @@
/*
* Copyright 2021 Red Hat, Inc. and/or its affiliates
* and other 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.keycloak.services.error;
import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper;
/**
* Override explicitly added ExceptionMapper for handling <code>UnrecognizedPropertyException</code> in RestEasy Jackson
*
* <code>org.jboss.resteasy.plugins.providers.jackson.UnrecognizedPropertyExceptionHandler</code>
*/
public class KcUnrecognizedPropertyExceptionHandler implements ExceptionMapper<UnrecognizedPropertyException> {
@Context
private HttpHeaders headers;
/**
* Return escaped original message
*/
@Override
public Response toResponse(UnrecognizedPropertyException exception) {
return KeycloakErrorHandler.getResponse(headers, new BadRequestException(exception.getMessage()));
}
}

17
services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java
View file

@ -4,7 +4,6 @@ import com.fasterxml.jackson.core.JsonParseException;
import org.jboss.logging.Logger;
import org.jboss.resteasy.spi.Failure;
import org.jboss.resteasy.spi.HttpResponse;
import org.jboss.resteasy.spi.ResteasyProviderFactory;
import org.keycloak.Config;
import org.keycloak.common.util.Resteasy;
import org.keycloak.forms.login.freemarker.model.UrlBean;
@ -29,7 +28,6 @@ import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;
import java.io.IOException;
import java.util.HashMap;
import java.util.Locale;
@ -55,6 +53,10 @@ public class KeycloakErrorHandler implements ExceptionMapper<Throwable> {
@Override
public Response toResponse(Throwable throwable) {
return getResponse(headers, throwable);
}
public static Response getResponse(HttpHeaders headers, Throwable throwable) {
KeycloakSession session = Resteasy.getContextData(KeycloakSession.class);
KeycloakTransaction tx = session.getTransactionManager();
tx.setRollbackOnly();
@ -63,8 +65,7 @@ public class KeycloakErrorHandler implements ExceptionMapper<Throwable> {
if (statusCode >= 500 && statusCode <= 599) {
logger.error(UNCAUGHT_SERVER_ERROR_TEXT, throwable);
}
else {
} else {
logger.debugv(throwable, ERROR_RESPONSE_TEXT, statusCode);
}
@ -99,7 +100,7 @@ public class KeycloakErrorHandler implements ExceptionMapper<Throwable> {
}
}
private int getStatusCode(Throwable throwable) {
private static int getStatusCode(Throwable throwable) {
int status = Response.Status.INTERNAL_SERVER_ERROR.getStatusCode();
if (throwable instanceof WebApplicationException) {
WebApplicationException ex = (WebApplicationException) throwable;
@ -120,7 +121,7 @@ public class KeycloakErrorHandler implements ExceptionMapper<Throwable> {
return status;
}
private String getErrorCode(Throwable throwable) {
private static String getErrorCode(Throwable throwable) {
if (throwable instanceof WebApplicationException && throwable.getMessage() != null) {
return throwable.getMessage();
}
@ -128,7 +129,7 @@ public class KeycloakErrorHandler implements ExceptionMapper<Throwable> {
return "unknown_error";
}
private RealmModel resolveRealm(KeycloakSession session) {
private static RealmModel resolveRealm(KeycloakSession session) {
String path = session.getContext().getUri().getPath();
Matcher m = realmNamePattern.matcher(path);
String realmName;
@ -149,7 +150,7 @@ public class KeycloakErrorHandler implements ExceptionMapper<Throwable> {
return realm;
}
private Map<String, Object> initAttributes(KeycloakSession session, RealmModel realm, Theme theme, Locale locale, int statusCode) throws IOException {
private static Map<String, Object> initAttributes(KeycloakSession session, RealmModel realm, Theme theme, Locale locale, int statusCode) throws IOException {
Map<String, Object> attributes = new HashMap<>();
Properties messagesBundle = theme.getMessages(locale);

2
services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java
View file

@ -41,6 +41,7 @@ import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.services.DefaultKeycloakSessionFactory;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.error.KeycloakErrorHandler;
import org.keycloak.services.error.KcUnrecognizedPropertyExceptionHandler;
import org.keycloak.services.filters.KeycloakSecurityHeadersFilter;
import org.keycloak.services.managers.ApplianceBootstrap;
import org.keycloak.services.managers.RealmManager;
@ -107,6 +108,7 @@ public class KeycloakApplication extends Application {
classes.add(KeycloakSecurityHeadersFilter.class);
classes.add(KeycloakErrorHandler.class);
classes.add(KcUnrecognizedPropertyExceptionHandler.class);
singletons.add(new ObjectMapperResolver());
singletons.add(new WelcomeResource());

13
testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java
View file

@ -57,6 +57,7 @@ import org.keycloak.representations.idm.AuthDetailsRepresentation;
import org.keycloak.representations.idm.AuthenticationFlowRepresentation;
import org.keycloak.representations.idm.EventRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.services.ErrorPage;
import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.resource.RealmResourceProvider;
import org.keycloak.services.scheduled.ClearExpiredUserSessions;
@ -1035,6 +1036,18 @@ public class TestingResourceProvider implements RealmResourceProvider {
}
/**
* Display message to Error Page - for testing purposes
*
* @param message message
*/
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/display-error-message")
public Response displayErrorMessage(@QueryParam("message") String message) {
return ErrorPage.error(session, session.getContext().getAuthenticationSession(), Response.Status.BAD_REQUEST, message == null ? "" : message);
}
private RealmModel getRealmByName(String realmName) {
RealmProvider realmProvider = session.getProvider(RealmProvider.class);
RealmModel realm = realmProvider.getRealmByName(realmName);

9
testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java
View file

@ -379,4 +379,13 @@ public interface TestingResource {
Response simulatePostRequest(@QueryParam("postRequestUrl") String postRequestUrl,
@QueryParam("encodedFormParameters") String encodedFormParameters);
/**
* Display message to Error Page - for testing purposes
*
* @param message message
*/
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/display-error-message")
Response displayErrorMessage(@QueryParam("message") String message);
}

4
testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/ErrorPage.java
View file

@ -16,8 +16,10 @@
*/
package org.keycloak.testsuite.pages;
import org.apache.commons.lang.StringEscapeUtils;
import org.jboss.arquillian.test.api.ArquillianResource;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.UIUtils;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;
@ -36,7 +38,7 @@ public class ErrorPage extends AbstractPage {
private WebElement backToApplicationLink;
public String getError() {
return errorMessage.getText();
return UIUtils.getTextFromElement(errorMessage);
}
public void clickBackToApplication() {

37
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java
View file

@ -17,6 +17,15 @@
package org.keycloak.testsuite.client;
import org.apache.http.Header;
import org.apache.http.HeaderElement;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils;
import org.hamcrest.CoreMatchers;
import org.hamcrest.Matchers;
import org.junit.Test;
import org.keycloak.client.registration.Auth;
@ -31,6 +40,7 @@ import org.keycloak.representations.idm.ProtocolMapperRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected;
import org.keycloak.util.JsonSerialization;
import javax.ws.rs.NotFoundException;
@ -47,6 +57,7 @@ import java.util.stream.Collectors;
import static java.util.Arrays.asList;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
@ -57,6 +68,7 @@ import static org.junit.Assert.fail;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import static org.keycloak.services.clientregistration.ErrorCodes.INVALID_CLIENT_METADATA;
import static org.keycloak.services.clientregistration.ErrorCodes.INVALID_REDIRECT_URI;
import static org.keycloak.utils.MediaType.APPLICATION_JSON;
/**
* @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@ -614,4 +626,29 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest {
}
}
@Test
@UncaughtServerErrorExpected
public void registerClientWithWrongCharacters() throws IOException {
try (CloseableHttpClient client = HttpClientBuilder.create().build()) {
HttpPost post = new HttpPost(suiteContext.getAuthServerInfo().getUriBuilder().path("/auth/realms/master/clients-registrations/openid-connect").build());
post.setEntity(new StringEntity("{\"<img src=alert(1)>\":1}"));
post.setHeader("Content-Type", APPLICATION_JSON);
try (CloseableHttpResponse response = client.execute(post)) {
assertThat(response.getStatusLine().getStatusCode(),is(400));
Header header = response.getFirstHeader("Content-Type");
assertThat(header, notNullValue());
// Verify the Content-Type is not text/html
assertThat(Arrays.stream(header.getElements())
.map(HeaderElement::getName)
.filter(Objects::nonNull)
.anyMatch(f -> f.equals(APPLICATION_JSON)), is(true));
// The alert is not executed
assertThat(EntityUtils.toString(response.getEntity()), CoreMatchers.containsString("Unrecognized field \\\"<img src=alert(1)>\\\""));
}
}
}
}

91
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/error/EscapeErrorPageTest.java Normal file
View file

@ -0,0 +1,91 @@
/*
* Copyright 2021 Red Hat, Inc. and/or its affiliates
* and other 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.keycloak.testsuite.error;
import org.hamcrest.CoreMatchers;
import org.jboss.arquillian.graphene.page.Page;
import org.junit.Test;
import org.keycloak.common.util.KeycloakUriBuilder;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.testsuite.AbstractKeycloakTest;
import org.keycloak.testsuite.pages.ErrorPage;
import java.net.URI;
import java.util.List;
import static org.hamcrest.MatcherAssert.assertThat;
public class EscapeErrorPageTest extends AbstractKeycloakTest {
@Page
public ErrorPage errorPage;
@Override
public void addTestRealms(List<RealmRepresentation> testRealms) {
}
@Test
public void innerScript() {
checkMessage("\"<img src=<script>alert(1)</script>/>\"", "\"alert(1)/>\"");
}
@Test
public void innerURL() {
checkMessage("\"See https://www.keycloak.org/docs\"", "\"See https://www.keycloak.org/docs\"");
}
@Test
public void URL() {
checkMessage("See https://www.keycloak.org/docs", "See https://www.keycloak.org/docs");
}
@Test
public void ampersandEscape() {
checkMessage("&lt;img src=&quot;something&quot;&gt;", "<img src=\"something\">");
}
@Test
public void hexEscape() {
checkMessage("&#x3C;img src&#61;something&#x2F;&#x3E;", "<img src=something/>");
}
@Test
public void plainText() {
checkMessage("It doesn't work", "It doesn't work");
}
@Test
public void textWithPlus() {
checkMessage("Fact: 1+1=2", "Fact: 1+1=2");
}
private void checkMessage(String queryParam, String expected) {
try {
final URI uri = KeycloakUriBuilder.fromUri(suiteContext.getAuthServerInfo().getContextRoot().toURI())
.path("/auth/realms/master/testing/display-error-message")
.queryParam("message", queryParam)
.build();
driver.navigate().to(uri.toURL());
errorPage.assertCurrent();
assertThat(errorPage.getError(), CoreMatchers.is(expected));
} catch (Exception e) {
e.printStackTrace();
}
}
}

43
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/error/UncaughtErrorPageTest.java
View file

@ -1,10 +1,14 @@
package org.keycloak.testsuite.error;
import org.apache.http.Header;
import org.apache.http.HeaderElement;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils;
import org.hamcrest.CoreMatchers;
import org.jboss.arquillian.graphene.page.Page;
import org.junit.Assert;
import org.junit.Test;
@ -26,10 +30,18 @@ import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URI;
import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import static org.junit.Assert.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.keycloak.utils.MediaType.APPLICATION_JSON;
public class UncaughtErrorPageTest extends AbstractKeycloakTest {
@ -104,6 +116,35 @@ public class UncaughtErrorPageTest extends AbstractKeycloakTest {
}
}
@Test
@UncaughtServerErrorExpected
public void uncaughtErrorAdminPropertyError() throws IOException {
try (CloseableHttpClient client = HttpClientBuilder.create().build()) {
String accessToken = adminClient.tokenManager().getAccessTokenString();
HttpPost post = new HttpPost(suiteContext.getAuthServerInfo().getUriBuilder().path("/auth/admin/realms").build());
post.setEntity(new StringEntity("{\"<img src=alert(1)>\":1}"));
post.setHeader("Authorization", "bearer " + accessToken);
post.setHeader("Content-Type", "application/json");
try (CloseableHttpResponse response = client.execute(post)) {
assertEquals(400, response.getStatusLine().getStatusCode());
Header header = response.getFirstHeader("Content-Type");
assertThat(header, notNullValue());
// Verify the Content-Type is not text/html
assertThat(Arrays.stream(header.getElements())
.map(HeaderElement::getName)
.filter(Objects::nonNull)
.anyMatch(f -> f.equals(APPLICATION_JSON)), is(true));
// The alert is not executed
assertThat(EntityUtils.toString(response.getEntity()), CoreMatchers.containsString("Unrecognized field \\\"<img src=alert(1)>\\\""));
}
}
}
@Test
@UncaughtServerErrorExpected
public void uncaughtError() throws MalformedURLException {

60
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/openshift/OpenShiftTokenReviewEndpointTest.java
View file

@ -1,11 +1,31 @@
/*
* Copyright 2018 Red Hat, Inc. and/or its affiliates
* and other 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.keycloak.testsuite.openshift;
import org.apache.http.Header;
import org.apache.http.HeaderElement;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@ -33,6 +53,7 @@ import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.UserBuilder;
@ -40,19 +61,26 @@ import org.keycloak.util.JsonSerialization;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.*;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.keycloak.common.Profile.Feature.OPENSHIFT_INTEGRATION;
import static org.keycloak.testsuite.util.ServerURLs.getAuthServerContextRoot;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
import static org.keycloak.utils.MediaType.APPLICATION_JSON;
@AuthServerContainerExclude({AuthServer.REMOTE})
@EnableFeature(value = OPENSHIFT_INTEGRATION, skipRestart = true)
@ -323,6 +351,32 @@ public class OpenShiftTokenReviewEndpointTest extends AbstractTestRealmKeycloakT
}
}
@Test
public void checkPropertyValidation() throws IOException {
try (CloseableHttpClient client = HttpClientBuilder.create().build()) {
String url = getAuthServerContextRoot() + "/auth/realms/" + "test" + "/protocol/openid-connect/ext/openshift-token-review/";
HttpPost post = new HttpPost(url);
post.setHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString());
post.setHeader(HttpHeaders.ACCEPT, ContentType.APPLICATION_JSON.toString());
post.setEntity(new StringEntity("{\"<img src=alert(1)>\":1}"));
try (CloseableHttpResponse response = client.execute(post)) {
Header header = response.getFirstHeader("Content-Type");
assertThat(header, notNullValue());
// Verify the Content-Type is not text/html
assertThat(Arrays.stream(header.getElements())
.map(HeaderElement::getName)
.filter(Objects::nonNull)
.anyMatch(f -> f.equals(APPLICATION_JSON)), is(true));
// OpenShiftTokenReviewRequestRepresentation ignore unknown attributes and is returned default representation
assertThat(EntityUtils.toString(response.getEntity()).contains("Unrecognized field \\\"<img src=alert(1)>\\\""), is(false));
}
}
}
private class Review {
private String realm = "test";

4
themes/src/main/resources/theme/base/login/error.ftl
View file

@ -1,10 +1,10 @@
<#import "template.ftl" as layout>
<@layout.registrationLayout displayMessage=false; section>
<#if section = "header">
${msg("errorTitle")}
${kcSanitize(msg("errorTitle"))?no_esc}
<#elseif section = "form">
<div id="kc-error-message">
<p class="instruction">${message.summary?no_esc}</p>
<p class="instruction">${kcSanitize(message.summary)?no_esc}</p>
<#if skipLink??>
<#else>
<#if client?? && client.baseUrl?has_content>