mariusz.pacek/keycloak
1
0 Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[KEYCLOAK-8168] - PEP is resolving claims twice under certain circumstances

Browse Source
This commit is contained in:
Pedro Igor committed 2018-09-25 11:47:50 -03:00
1 parent 0ff0dcecec
commit df311b60b4
4 files changed
+93 -26
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java
+7 -10
View File
@@ -112,8 +112,9 @@ public abstract class AbstractPolicyEnforcer {
}
MethodConfig methodConfig = getRequiredScopes(pathConfig, request);
Map<String, List<String>> claims = resolveClaims(pathConfig, httpFacade);
if (isAuthorized(pathConfig, methodConfig, accessToken, httpFacade)) {
if (isAuthorized(pathConfig, methodConfig, accessToken, httpFacade, claims)) {
try {
return createAuthorizationContext(accessToken, pathConfig);
} catch (Exception e) {
@@ -142,7 +143,7 @@ public abstract class AbstractPolicyEnforcer {
protected abstract boolean challenge(PathConfig pathConfig, MethodConfig methodConfig, OIDCHttpFacade facade);
protected boolean isAuthorized(PathConfig actualPathConfig, MethodConfig methodConfig, AccessToken accessToken, OIDCHttpFacade httpFacade) {
protected boolean isAuthorized(PathConfig actualPathConfig, MethodConfig methodConfig, AccessToken accessToken, OIDCHttpFacade httpFacade, Map<String, List<String>> claims) {
Request request = httpFacade.getRequest();
if (isDefaultAccessDeniedUri(request)) {
@@ -175,7 +176,7 @@ public abstract class AbstractPolicyEnforcer {
policyEnforcer.getPathMatcher().removeFromCache(getPath(request));
}
return hasValidClaims(actualPathConfig, permission, httpFacade, authorization);
return hasValidClaims(permission, claims);
}
}
} else {
@@ -197,12 +198,10 @@ public abstract class AbstractPolicyEnforcer {
return false;
}
private boolean hasValidClaims(PathConfig actualPathConfig, Permission permission, OIDCHttpFacade httpFacade, Authorization authorization) {
private boolean hasValidClaims(Permission permission, Map<String, List<String>> claims) {
Map<String, Set<String>> grantedClaims = permission.getClaims();
if (grantedClaims != null) {
Map<String, List<String>> claims = resolveClaims(actualPathConfig, httpFacade);
if (claims.isEmpty()) {
return false;
}
@@ -349,18 +348,16 @@ public abstract class AbstractPolicyEnforcer {
}
private Map<String, List<String>> getClaims(Map<String, Map<String, Object>>claimInformationPointConfig, HttpFacade httpFacade) {
Map<String, List<String>> claims = new HashMap<>();
if (claimInformationPointConfig != null) {
for (Entry<String, Map<String, Object>> claimDef : claimInformationPointConfig.entrySet()) {
ClaimInformationPointProviderFactory factory = getPolicyEnforcer().getClaimInformationPointProviderFactories().get(claimDef.getKey());
if (factory != null) {
claims.putAll(factory.create(claimDef.getValue()).resolve(httpFacade));
return factory.create(claimDef.getValue()).resolve(httpFacade);
}
}
}
return claims;
return new HashMap<>();
}
}
adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java
+5 -7
View File
@@ -55,14 +55,14 @@ public class KeycloakAdapterPolicyEnforcer extends AbstractPolicyEnforcer {
}
@Override
protected boolean isAuthorized(PathConfig pathConfig, PolicyEnforcerConfig.MethodConfig methodConfig, AccessToken accessToken, OIDCHttpFacade httpFacade) {
protected boolean isAuthorized(PathConfig pathConfig, PolicyEnforcerConfig.MethodConfig methodConfig, AccessToken accessToken, OIDCHttpFacade httpFacade, Map<String, List<String>> claims) {
AccessToken original = accessToken;
if (super.isAuthorized(pathConfig, methodConfig, accessToken, httpFacade)) {
if (super.isAuthorized(pathConfig, methodConfig, accessToken, httpFacade, claims)) {
return true;
}
accessToken = requestAuthorizationToken(pathConfig, methodConfig, httpFacade);
accessToken = requestAuthorizationToken(pathConfig, methodConfig, httpFacade, claims);
if (accessToken == null) {
return false;
@@ -90,7 +90,7 @@ public class KeycloakAdapterPolicyEnforcer extends AbstractPolicyEnforcer {
original.setAuthorization(authorization);
return super.isAuthorized(pathConfig, methodConfig, accessToken, httpFacade);
return super.isAuthorized(pathConfig, methodConfig, accessToken, httpFacade, claims);
}
@Override
@@ -133,7 +133,7 @@ public class KeycloakAdapterPolicyEnforcer extends AbstractPolicyEnforcer {
}
}
private AccessToken requestAuthorizationToken(PathConfig pathConfig, PolicyEnforcerConfig.MethodConfig methodConfig, OIDCHttpFacade httpFacade) {
private AccessToken requestAuthorizationToken(PathConfig pathConfig, PolicyEnforcerConfig.MethodConfig methodConfig, OIDCHttpFacade httpFacade, Map<String, List<String>> claims) {
if (getEnforcerConfig().getUserManagedAccess() != null) {
return null;
}
@@ -149,8 +149,6 @@ public class KeycloakAdapterPolicyEnforcer extends AbstractPolicyEnforcer {
authzRequest.addPermission(pathConfig.getId(), methodConfig.getScopes());
}
Map<String, List<String>> claims = resolveClaims(pathConfig, httpFacade);
if (!claims.isEmpty()) {
authzRequest.setClaimTokenFormat("urn:ietf:params:oauth:token-type:jwt");
authzRequest.setClaimToken(Base64.encodeBytes(JsonSerialization.writeValueAsBytes(claims)));
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/PolicyEnforcerTest.java
+54 -9
View File
@@ -33,8 +33,11 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -153,6 +156,34 @@ public class PolicyEnforcerTest extends AbstractKeycloakTest {
assertEquals(403, TestResponse.class.cast(httpFacade.getResponse()).getStatus());
}
@Test
public void testResolvingClaimsOnce() {
KeycloakDeployment deployment = KeycloakDeploymentBuilder.build(getAdapterConfiguration("enforcer-bearer-only-with-cip.json"));
PolicyEnforcer policyEnforcer = deployment.getPolicyEnforcer();
oauth.realm(REALM_NAME);
oauth.clientId("public-client-test");
oauth.doLogin("marta", "password");
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, null);
String token = response.getAccessToken();
OIDCHttpFacade httpFacade = createHttpFacade("/api/resourcea", token, new Function<String, String>() {
AtomicBoolean resolved = new AtomicBoolean();
@Override
public String apply(String s) {
Assert.assertTrue(resolved.compareAndSet(false, true));
return "claim-value";
}
});
AuthorizationContext context = policyEnforcer.enforce(httpFacade);
assertTrue(context.isGranted());
}
@Test
public void testOnDenyRedirectTo() {
KeycloakDeployment deployment = KeycloakDeploymentBuilder.build(getAdapterConfiguration("enforcer-on-deny-redirect.json"));
@@ -308,6 +339,10 @@ public class PolicyEnforcerTest extends AbstractKeycloakTest {
}
private OIDCHttpFacade createHttpFacade(String path, String method, String token, Map<String, List<String>> headers, Map<String, List<String>> parameters, InputStream requestBody, KeycloakDeployment deployment) {
return createHttpFacade(path, method, token, headers, parameters, requestBody, deployment, null);
}
private OIDCHttpFacade createHttpFacade(String path, String method, String token, Map<String, List<String>> headers, Map<String, List<String>> parameters, InputStream requestBody, KeycloakDeployment deployment, Function<String, String> parameterFunction) {
return new OIDCHttpFacade() {
Request request;
Response response;
@@ -329,7 +364,7 @@ public class PolicyEnforcerTest extends AbstractKeycloakTest {
@Override
public Request getRequest() {
if (request == null) {
request = createHttpRequest(path, method, headers, parameters, requestBody);
request = createHttpRequest(path, method, headers, parameters, requestBody, parameterFunction);
}
return request;
}
@@ -357,11 +392,27 @@ public class PolicyEnforcerTest extends AbstractKeycloakTest {
return createHttpFacade(path, null, null, new HashMap<>(), new HashMap<>(), null, null);
}
private OIDCHttpFacade createHttpFacade(String path, String token, Function<String, String> parameterFunction) {
return createHttpFacade(path, null, token, new HashMap<>(), new HashMap<>(), null, null, parameterFunction);
}
private Response createHttpResponse(Map<String, List<String>> headers) {
return new TestResponse(headers);
}
private Request createHttpRequest(String path, String method, Map<String, List<String>> headers, Map<String, List<String>> parameters, InputStream requestBody) {
private Request createHttpRequest(String path, String method, Map<String, List<String>> headers, Map<String, List<String>> parameters, InputStream requestBody, Function<String, String> parameterFunction) {
if (parameterFunction == null) {
parameterFunction = param -> {
List<String> values = parameters.getOrDefault(param, Collections.emptyList());
if (!values.isEmpty()) {
return values.get(0);
}
return null;
};
}
Function<String, String> finalParameterFunction = parameterFunction;
return new Request() {
private InputStream inputStream;
@@ -388,13 +439,7 @@ public class PolicyEnforcerTest extends AbstractKeycloakTest {
@Override
public String getFirstParam(String param) {
List<String> values = parameters.getOrDefault(param, Collections.emptyList());
if (!values.isEmpty()) {
return values.get(0);
}
return null;
return finalParameterFunction.apply(param);
}
@Override
testsuite/integration-arquillian/tests/base/src/test/resources/authorization-test/enforcer-bearer-only-with-cip.json
+27
View File
@@ -0,0 +1,27 @@
{
"realm": "authz-test",
"auth-server-url": "http://localhost:8180/auth",
"ssl-required": "external",
"resource": "resource-server-test",
"credentials": {
"secret": "secret"
},
"bearer-only": true,
"policy-enforcer": {
"claim-information-point": {
"claims": {
"claim-b": "claim-b"
}
},
"paths": [
{
"path": "/api/resourcea",
"claim-information-point": {
"claims": {
"claim-a": "{request.parameter['claim-a']}"
}
}
}
]
}
}