KEYCLOAK-4758 Update Encode class using latest resteasy. Use encodeQueryParamAsIs instead of encodeQueryParam when encoding key=value pairs for URI query sections. Also fix a few callers who were relying on the bad behaviour of queryParam.
This commit is contained in:
Alex Szczuczko
parent
9be9e30ad6
commit
5d88c2b8be
7 changed files with 158 additions and 46 deletions
|
|
@ -170,7 +170,7 @@ public class OAuthRequestAuthenticator {
|
|||
KeycloakUriBuilder redirectUriBuilder = deployment.getAuthUrl().clone()
|
||||
.queryParam(OAuth2Constants.RESPONSE_TYPE, OAuth2Constants.CODE)
|
||||
.queryParam(OAuth2Constants.CLIENT_ID, deployment.getResourceName())
|
||||
.queryParam(OAuth2Constants.REDIRECT_URI, Encode.encodeQueryParamAsIs(url)) // Need to encode uri ourselves as queryParam() will not encode % characters.
|
||||
.queryParam(OAuth2Constants.REDIRECT_URI, url)
|
||||
.queryParam(OAuth2Constants.STATE, state)
|
||||
.queryParam("login", "true");
|
||||
if(loginHint != null && loginHint.length() > 0){
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import java.nio.ByteBuffer;
|
|||
import java.nio.charset.CharacterCodingException;
|
||||
import java.nio.charset.Charset;
|
||||
import java.nio.charset.CharsetDecoder;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
|
@ -36,7 +37,7 @@ import java.util.regex.Pattern;
|
|||
*/
|
||||
public class Encode
|
||||
{
|
||||
private static final String UTF_8 = "UTF-8";
|
||||
private static final String UTF_8 = StandardCharsets.UTF_8.name();
|
||||
|
||||
private static final Pattern PARAM_REPLACEMENT = Pattern.compile("_resteasy_uri_parameter");
|
||||
|
||||
|
|
@ -84,9 +85,7 @@ public class Encode
|
|||
case '@':
|
||||
continue;
|
||||
}
|
||||
StringBuffer sb = new StringBuffer();
|
||||
sb.append((char) i);
|
||||
pathEncoding[i] = URLEncoder.encode(sb.toString());
|
||||
pathEncoding[i] = URLEncoder.encode(String.valueOf((char) i));
|
||||
}
|
||||
pathEncoding[' '] = "%20";
|
||||
System.arraycopy(pathEncoding, 0, matrixParameterEncoding, 0, pathEncoding.length);
|
||||
|
|
@ -119,9 +118,7 @@ public class Encode
|
|||
queryNameValueEncoding[i] = "+";
|
||||
continue;
|
||||
}
|
||||
StringBuffer sb = new StringBuffer();
|
||||
sb.append((char) i);
|
||||
queryNameValueEncoding[i] = URLEncoder.encode(sb.toString());
|
||||
queryNameValueEncoding[i] = URLEncoder.encode(String.valueOf((char) i));
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -159,9 +156,7 @@ public class Encode
|
|||
queryStringEncoding[i] = "%20";
|
||||
continue;
|
||||
}
|
||||
StringBuffer sb = new StringBuffer();
|
||||
sb.append((char) i);
|
||||
queryStringEncoding[i] = URLEncoder.encode(sb.toString());
|
||||
queryStringEncoding[i] = URLEncoder.encode(String.valueOf((char) i));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -194,7 +189,7 @@ public class Encode
|
|||
*/
|
||||
public static String encodeFragment(String value)
|
||||
{
|
||||
return encodeValue(value, queryNameValueEncoding);
|
||||
return encodeValue(value, queryStringEncoding);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -221,18 +216,19 @@ public class Encode
|
|||
public static String decodePath(String path)
|
||||
{
|
||||
Matcher matcher = encodedCharsMulti.matcher(path);
|
||||
StringBuffer buf = new StringBuffer();
|
||||
int start=0;
|
||||
StringBuilder builder = new StringBuilder();
|
||||
CharsetDecoder decoder = Charset.forName(UTF_8).newDecoder();
|
||||
while (matcher.find())
|
||||
{
|
||||
builder.append(path, start, matcher.start());
|
||||
decoder.reset();
|
||||
String decoded = decodeBytes(matcher.group(1), decoder);
|
||||
decoded = decoded.replace("\\", "\\\\");
|
||||
decoded = decoded.replace("$", "\\$");
|
||||
matcher.appendReplacement(buf, decoded);
|
||||
builder.append(decoded);
|
||||
start = matcher.end();
|
||||
}
|
||||
matcher.appendTail(buf);
|
||||
return buf.toString();
|
||||
builder.append(path, start, path.length());
|
||||
return builder.toString();
|
||||
}
|
||||
|
||||
private static String decodeBytes(String enc, CharsetDecoder decoder)
|
||||
|
|
@ -264,7 +260,7 @@ public class Encode
|
|||
public static String encodeNonCodes(String string)
|
||||
{
|
||||
Matcher matcher = nonCodes.matcher(string);
|
||||
StringBuffer buf = new StringBuffer();
|
||||
StringBuilder builder = new StringBuilder();
|
||||
|
||||
|
||||
// FYI: we do not use the no-arg matcher.find()
|
||||
|
|
@ -276,29 +272,32 @@ public class Encode
|
|||
while (matcher.find(idx))
|
||||
{
|
||||
int start = matcher.start();
|
||||
buf.append(string.substring(idx, start));
|
||||
buf.append("%25");
|
||||
builder.append(string.substring(idx, start));
|
||||
builder.append("%25");
|
||||
idx = start + 1;
|
||||
}
|
||||
buf.append(string.substring(idx));
|
||||
return buf.toString();
|
||||
builder.append(string.substring(idx));
|
||||
return builder.toString();
|
||||
}
|
||||
|
||||
private static boolean savePathParams(String segment, StringBuffer newSegment, List<String> params)
|
||||
public static boolean savePathParams(String segment, StringBuilder newSegment, List<String> params)
|
||||
{
|
||||
boolean foundParam = false;
|
||||
// Regular expressions can have '{' and '}' characters. Replace them to do match
|
||||
segment = PathHelper.replaceEnclosedCurlyBraces(segment);
|
||||
Matcher matcher = PathHelper.URI_TEMPLATE_PATTERN.matcher(segment);
|
||||
int start = 0;
|
||||
while (matcher.find())
|
||||
{
|
||||
newSegment.append(segment, start, matcher.start());
|
||||
foundParam = true;
|
||||
String group = matcher.group();
|
||||
// Regular expressions can have '{' and '}' characters. Recover earlier replacement
|
||||
params.add(PathHelper.recoverEnclosedCurlyBraces(group));
|
||||
matcher.appendReplacement(newSegment, "_resteasy_uri_parameter");
|
||||
newSegment.append("_resteasy_uri_parameter");
|
||||
start = matcher.end();
|
||||
}
|
||||
matcher.appendTail(newSegment);
|
||||
newSegment.append(segment, start, segment.length());
|
||||
return foundParam;
|
||||
}
|
||||
|
||||
|
|
@ -309,11 +308,11 @@ public class Encode
|
|||
* @param encoding
|
||||
* @return
|
||||
*/
|
||||
private static String encodeValue(String segment, String[] encoding)
|
||||
public static String encodeValue(String segment, String[] encoding)
|
||||
{
|
||||
ArrayList<String> params = new ArrayList<String>();
|
||||
boolean foundParam = false;
|
||||
StringBuffer newSegment = new StringBuffer();
|
||||
StringBuilder newSegment = new StringBuilder();
|
||||
if (savePathParams(segment, newSegment, params))
|
||||
{
|
||||
foundParam = true;
|
||||
|
|
@ -411,21 +410,21 @@ public class Encode
|
|||
return encodeFromArray(nameOrValue, queryNameValueEncoding, true);
|
||||
}
|
||||
|
||||
private static String encodeFromArray(String segment, String[] encodingMap, boolean encodePercent)
|
||||
protected static String encodeFromArray(String segment, String[] encodingMap, boolean encodePercent)
|
||||
{
|
||||
StringBuffer result = new StringBuffer();
|
||||
StringBuilder result = new StringBuilder();
|
||||
for (int i = 0; i < segment.length(); i++)
|
||||
{
|
||||
if (!encodePercent && segment.charAt(i) == '%')
|
||||
char currentChar = segment.charAt(i);
|
||||
if (!encodePercent && currentChar == '%')
|
||||
{
|
||||
result.append(segment.charAt(i));
|
||||
result.append(currentChar);
|
||||
continue;
|
||||
}
|
||||
int idx = segment.charAt(i);
|
||||
String encoding = encode(idx, encodingMap);
|
||||
String encoding = encode(currentChar, encodingMap);
|
||||
if (encoding == null)
|
||||
{
|
||||
result.append(segment.charAt(i));
|
||||
result.append(currentChar);
|
||||
}
|
||||
else
|
||||
{
|
||||
|
|
@ -461,20 +460,20 @@ public class Encode
|
|||
return encoded;
|
||||
}
|
||||
|
||||
private static String pathParamReplacement(String segment, List<String> params)
|
||||
public static String pathParamReplacement(String segment, List<String> params)
|
||||
{
|
||||
StringBuffer newSegment = new StringBuffer();
|
||||
StringBuilder newSegment = new StringBuilder();
|
||||
Matcher matcher = PARAM_REPLACEMENT.matcher(segment);
|
||||
int i = 0;
|
||||
int start = 0;
|
||||
while (matcher.find())
|
||||
{
|
||||
newSegment.append(segment, start, matcher.start());
|
||||
String replacement = params.get(i++);
|
||||
// double encode slashes, so that slashes stay where they are
|
||||
replacement = replacement.replace("\\", "\\\\");
|
||||
replacement = replacement.replace("$", "\\$");
|
||||
matcher.appendReplacement(newSegment, replacement);
|
||||
newSegment.append(replacement);
|
||||
start = matcher.end();
|
||||
}
|
||||
matcher.appendTail(newSegment);
|
||||
newSegment.append(segment, start, segment.length());
|
||||
segment = newSegment.toString();
|
||||
return segment;
|
||||
}
|
||||
|
|
@ -506,6 +505,38 @@ public class Encode
|
|||
return decoded;
|
||||
}
|
||||
|
||||
/**
|
||||
* decode an encoded map
|
||||
*
|
||||
* @param map
|
||||
* @param charset
|
||||
* @return
|
||||
*/
|
||||
public static MultivaluedHashMap<String, String> decode(MultivaluedHashMap<String, String> map, String charset)
|
||||
{
|
||||
if (charset == null)
|
||||
{
|
||||
charset = UTF_8;
|
||||
}
|
||||
MultivaluedHashMap<String, String> decoded = new MultivaluedHashMap<String, String>();
|
||||
for (Map.Entry<String, List<String>> entry : map.entrySet())
|
||||
{
|
||||
List<String> values = entry.getValue();
|
||||
for (String value : values)
|
||||
{
|
||||
try
|
||||
{
|
||||
decoded.add(URLDecoder.decode(entry.getKey(), charset), URLDecoder.decode(value, charset));
|
||||
}
|
||||
catch (UnsupportedEncodingException e)
|
||||
{
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
}
|
||||
return decoded;
|
||||
}
|
||||
|
||||
public static MultivaluedHashMap<String, String> encode(MultivaluedHashMap<String, String> map)
|
||||
{
|
||||
MultivaluedHashMap<String, String> decoded = new MultivaluedHashMap<String, String>();
|
||||
|
|
|
|||
|
|
@ -614,7 +614,7 @@ public class KeycloakUriBuilder {
|
|||
if (value == null) throw new IllegalArgumentException("A passed in value was null");
|
||||
if (query == null) query = "";
|
||||
else query += "&";
|
||||
query += Encode.encodeQueryParam(name) + "=" + Encode.encodeQueryParam(value.toString());
|
||||
query += Encode.encodeQueryParamAsIs(name) + "=" + Encode.encodeQueryParamAsIs(value.toString());
|
||||
}
|
||||
return this;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -345,7 +345,7 @@ public class BaseSAML2BindingBuilder<T extends BaseSAML2BindingBuilder> {
|
|||
logger.debugv("saml document: {0}", documentAsString);
|
||||
byte[] responseBytes = documentAsString.getBytes(GeneralConstants.SAML_CHARSET);
|
||||
|
||||
return RedirectBindingUtil.deflateBase64URLEncode(responseBytes);
|
||||
return RedirectBindingUtil.deflateBase64Encode(responseBytes);
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -370,7 +370,7 @@ public class BaseSAML2BindingBuilder<T extends BaseSAML2BindingBuilder> {
|
|||
} catch (InvalidKeyException | SignatureException e) {
|
||||
throw new ProcessingException(e);
|
||||
}
|
||||
String encodedSig = RedirectBindingUtil.base64URLEncode(sig);
|
||||
String encodedSig = RedirectBindingUtil.base64Encode(sig);
|
||||
builder.queryParam(GeneralConstants.SAML_SIGNATURE_REQUEST_KEY, encodedSig);
|
||||
}
|
||||
return builder.build();
|
||||
|
|
|
|||
|
|
@ -60,6 +60,19 @@ public class RedirectBindingUtil {
|
|||
return URLDecoder.decode(str, GeneralConstants.SAML_CHARSET_NAME);
|
||||
}
|
||||
|
||||
/**
|
||||
* On the byte array, apply base64 encoding
|
||||
*
|
||||
* @param stringToEncode
|
||||
*
|
||||
* @return
|
||||
*
|
||||
* @throws IOException
|
||||
*/
|
||||
public static String base64Encode(byte[] stringToEncode) throws IOException {
|
||||
return Base64.encodeBytes(stringToEncode, Base64.DONT_BREAK_LINES);
|
||||
}
|
||||
|
||||
/**
|
||||
* On the byte array, apply base64 encoding following by URL encoding
|
||||
*
|
||||
|
|
|
|||
|
|
@ -91,7 +91,7 @@ public abstract class OIDCRedirectUriBuilder {
|
|||
|
||||
@Override
|
||||
public OIDCRedirectUriBuilder addParam(String paramName, String paramValue) {
|
||||
String param = paramName + "=" + Encode.encodeQueryParam(paramValue);
|
||||
String param = paramName + "=" + Encode.encodeQueryParamAsIs(paramValue);
|
||||
if (fragment == null) {
|
||||
fragment = new StringBuilder(param);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,68 @@
|
|||
/*
|
||||
* Copyright 2016 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.oauth;
|
||||
|
||||
import java.net.MalformedURLException;
|
||||
import java.net.URL;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.keycloak.protocol.oidc.utils.OIDCResponseType;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
|
||||
import org.keycloak.testsuite.Assert;
|
||||
import org.keycloak.testsuite.util.OAuthClient;
|
||||
|
||||
public class OAuthRedirectUriStateTest extends AbstractTestRealmKeycloakTest {
|
||||
|
||||
@Override
|
||||
public void configureTestRealm(RealmRepresentation testRealm) {
|
||||
}
|
||||
|
||||
@Before
|
||||
public void clientConfiguration() {
|
||||
oauth.clientId("test-app");
|
||||
oauth.responseType(OIDCResponseType.CODE);
|
||||
oauth.stateParamRandom();
|
||||
}
|
||||
|
||||
void assertStateReflected(String state) {
|
||||
oauth.stateParamHardcoded(state);
|
||||
|
||||
OAuthClient.AuthorizationEndpointResponse response = oauth.doLogin("test-user@localhost", "password");
|
||||
Assert.assertNotNull(response.getCode());
|
||||
|
||||
URL url;
|
||||
try {
|
||||
url = new URL(driver.getCurrentUrl());
|
||||
} catch (MalformedURLException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
Assert.assertTrue(url.getQuery().contains("state=" + state));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSimpleStateParameter() {
|
||||
assertStateReflected("VeryLittleGravitasIndeed");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testJsonStateParameter() {
|
||||
assertStateReflected("%7B%22csrf_token%22%3A%2B%22hlvZNIsWyqdkEhbjlQIia0ty2YY4TXat%22%2C%2B%22destination%22%3A%2B%22eyJhbGciOiJIUzI1NiJ9.Imh0dHA6Ly9sb2NhbGhvc3Q6NTAwMC9wcml2YXRlIg.T18WeIV29komDl8jav-3bSnUZDlMD8VOfIrd2ikP5zE%22%7D");
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue