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

[KEYCLOAK-15893] - Incorrect resource match is returned for some cases when using wildcard in uri

Browse source
This commit is contained in:
Pedro Igor 2020-10-14 12:36:31 -03:00 committed by Stian Thorgersen
parent 80bf0b6bad
commit 2593c3dbc4
3 changed files with 89 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
common/src/main/java/org/keycloak/common/util/PathMatcher.java
View file

@ -42,7 +42,7 @@ public abstract class PathMatcher<P> {
String matchingUri = null;
if (exactMatch(expectedUri, targetUri, expectedUri)) {
if (exactMatch(expectedUri, targetUri)) {
matchingUri = expectedUri;
}
@ -53,7 +53,7 @@ public abstract class PathMatcher<P> {
int length = expectedUri.split("\\/").length;
int bracketsLength = expectedUri.split("\\{").length;
if (exactMatch(expectedUri, targetUri, templateUri) && (patternCount == 0 || length > patternCount || bracketsLength < bracketsPatternCount)) {
if (exactMatch(templateUri, targetUri) && (patternCount == 0 || length > patternCount || bracketsLength < bracketsPatternCount)) {
matchingUri = templateUri;
P resolved = resolvePathConfig(entry, targetUri);
@ -96,8 +96,14 @@ public abstract class PathMatcher<P> {
}
if (WILDCARD == expectedUri.charAt(expectedUri.length() - 1)) {
if (matchingAnyPath == null || getPath(matchingAnyPath).length() < matchingUri.length()) {
if (matchingAnyPath == null) {
matchingAnyPath = entry;
} else {
String resourcePath = getPath(matchingAnyPath);
if (resourcePath.split("/").length < matchingUri.split("/").length) {
matchingAnyPath = entry;
}
}
} else {
int suffixIndex = expectedUri.indexOf(WILDCARD + ".");
@ -128,9 +134,9 @@ public abstract class PathMatcher<P> {
protected abstract Collection<P> getPaths();
private boolean exactMatch(String expectedUri, String targetUri, String value) {
if (targetUri.equals(value)) {
return value.equals(targetUri);
private boolean exactMatch(String expectedUri, String targetUri) {
if (targetUri.equals(expectedUri)) {
return true;
}
if (endsWithWildcard(expectedUri)) {
@ -148,7 +154,8 @@ public abstract class PathMatcher<P> {
}
protected String buildUriFromTemplate(String template, String targetUri, boolean onlyFirstParam) {
String expectedUri = template;
StringBuilder uri = new StringBuilder(template);
String expectedUri = uri.toString();
int patternStartIndex = expectedUri.indexOf("{");
if (expectedUri.endsWith("/*")) {
@ -171,6 +178,7 @@ public abstract class PathMatcher<P> {
if (Arrays.equals(matchingUri, Arrays.copyOf(targetUri.toCharArray(), matchingUri.length))) {
matchingUri = Arrays.copyOf(matchingUri, targetUri.length());
int paramIndex = 0;
int lastPattern = 0;
for (int i = patternStartIndex; i < expectedUriChars.length; i++) {
if (matchingUriLastIndex >= matchingUri.length) {
@ -192,9 +200,21 @@ public abstract class PathMatcher<P> {
if (matchingUriLastIndex + paramLength > matchingUri.length) {
return null;
}
StringBuilder value = new StringBuilder();
for (int j = 0; j < paramLength; j++) {
matchingUri[matchingUriLastIndex++] = params[k].charAt(j);
char valueChar = params[k].charAt(j);
value.append(valueChar);
matchingUri[matchingUriLastIndex++] = valueChar;
}
if (c == '{') {
uri.replace(uri.indexOf("{", lastPattern), uri.indexOf("}", lastPattern) + 1, value.toString());
}
if (value.charAt(value.length() - 1) == '}') {
lastPattern = uri.indexOf(value.toString()) + value.length();
}
if (c == '*' && matchingUriLastIndex < matchingUri.length) {
@ -217,18 +237,7 @@ public abstract class PathMatcher<P> {
}
}
if (matchingUri[matchingUri.length - 1] == '\u0000') {
if (template.endsWith("*")) {
StringBuilder firstParam = new StringBuilder(String.valueOf(matchingUri).substring(0, matchingUriLastIndex));
firstParam.append(targetUri.substring(firstParam.length()));
return firstParam.toString();
}
return null;
}
return String.valueOf(matchingUri);
return uri.toString();
}
return null;

2
common/src/test/java/org/keycloak/common/util/PathMatcherTest.java
View file

@ -11,7 +11,7 @@ public class PathMatcherTest {
public void keycloak15833Test() {
TestingPathMatcher matcher = new TestingPathMatcher();
Assert.assertNull(matcher.customBuildUriFromTemplate("/api/v1/{clientId}/campaigns/*/excelFiles", "/api/v1/1/contentConnectorConfigs/29/contentConnectorContents", false));
Assert.assertEquals("/api/v1/1/campaigns/*/excelFiles", matcher.customBuildUriFromTemplate("/api/v1/{clientId}/campaigns/*/excelFiles", "/api/v1/1/contentConnectorConfigs/29/contentConnectorContents", false));
}
private static final class TestingPathMatcher extends PathMatcher<Object> {

59
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementWithAuthzClientTest.java
View file

@ -56,6 +56,11 @@ public class ResourceManagementWithAuthzClientTest extends ResourceManagementTes
doCreateResource(new ResourceRepresentation("/resources/{pattern}/{pattern}/*", Collections.emptySet(), "/resources/{pattern}/{pattern}/*", null));
doCreateResource(new ResourceRepresentation("/resources/{pattern}/sub-resources/{pattern}/*", Collections.emptySet(), "/resources/{pattern}/sub-resources/{pattern}/*", null));
doCreateResource(new ResourceRepresentation("/resources/{pattern}/sub-resource", Collections.emptySet(), "/resources/{pattern}/sub-resources/{pattern}/*", null));
doCreateResource(new ResourceRepresentation("/rest/{version}/loader/loadTwo", Collections.emptySet(), "/rest/{version}/loader/loadTwo", null));
doCreateResource(new ResourceRepresentation("/rest/{version}/loader/load", Collections.emptySet(), "/rest/{version}/loader/load", null));
doCreateResource(new ResourceRepresentation(
"/rest/{version}/carts/{cartId}/cartactions/{actionId}", Collections.emptySet(), "/rest/{version}/carts/{cartId}/cartactions/{actionId}", null));
doCreateResource(new ResourceRepresentation("/rest/v1/carts/{cartId}/cartactions/123", Collections.emptySet(), "/rest/v1/carts/{cartId}/cartactions/123", null));
AuthzClient authzClient = getAuthzClient();
@ -100,6 +105,60 @@ public class ResourceManagementWithAuthzClientTest extends ResourceManagementTes
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/resources/{pattern}/sub-resources/{pattern}/*", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/v1/loader/load");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/{version}/loader/load", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/v2/carts/123/cartactions/123");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/{version}/carts/{cartId}/cartactions/{actionId}", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/v2/carts/{cartId}/cartactions/123");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/{version}/carts/{cartId}/cartactions/{actionId}", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/{version}/carts/123/cartactions/123");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/{version}/carts/{cartId}/cartactions/{actionId}", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/{version}/carts/{cartId}/cartactions/123");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/{version}/carts/{cartId}/cartactions/{actionId}", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/v1/carts/123/cartactions/123");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/v1/carts/{cartId}/cartactions/123", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/v1/carts/{cartId}/cartactions/123");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/v1/carts/{cartId}/cartactions/123", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/v1/carts/345/cartactions/123");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/v1/carts/{cartId}/cartactions/123", resources.get(0).getUri());
resources = authzClient.protection().resource().findByMatchingUri("/rest/v2/carts/345/cartactions/123");
assertNotNull(resources);
assertEquals(1, resources.size());
assertEquals("/rest/{version}/carts/{cartId}/cartactions/{actionId}", resources.get(0).getUri());
}
@Test