From 79ca722b49b04f723aa35668b77d8e7f5da4d04f Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Tue, 25 Sep 2018 10:39:23 -0300 Subject: [PATCH] [KEYCLOAK-7605] - Make sure Evaluation API is read-only --- .../StoreFactoryCacheSession.java | 28 ++++++++- .../jpa/store/JPAStoreFactory.java | 11 ++++ .../jpa/store/PolicyAdapter.java | 18 +++++- .../jpa/store/ResourceAdapter.java | 14 ++++- .../jpa/store/ResourceServerAdapter.java | 6 +- .../authorization/jpa/store/ScopeAdapter.java | 7 ++- .../authorization/AuthorizationProvider.java | 10 +++ .../model/AbstractAuthorizationModel.java | 34 ++++++++++ .../IterablePermissionEvaluator.java | 8 +++ .../policy/evaluation/DefaultEvaluation.java | 3 +- .../authorization/store/StoreFactory.java | 15 +++++ .../admin/PolicyEvaluationService.java | 5 +- .../AuthorizationTokenService.java | 22 +++---- .../common/DefaultEvaluationContext.java | 17 ++++- .../common/KeycloakEvaluationContext.java | 61 ------------------ .../testsuite/authz/PolicyEvaluationTest.java | 62 +++++++++++++++++-- 16 files changed, 230 insertions(+), 91 deletions(-) create mode 100644 server-spi-private/src/main/java/org/keycloak/authorization/model/AbstractAuthorizationModel.java delete mode 100644 services/src/main/java/org/keycloak/authorization/common/KeycloakEvaluationContext.java diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java index 89ae8d6ada..7689816eef 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java @@ -146,6 +146,16 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider { return permissionTicketCache; } + @Override + public void setReadOnly(boolean readOnly) { + getDelegate().setReadOnly(readOnly); + } + + @Override + public boolean isReadOnly() { + return getDelegate().isReadOnly(); + } + public void close() { if (delegate != null) { delegate.close(); @@ -724,7 +734,14 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider { Long loaded = cache.getCurrentRevision(cacheKey); List model = resultSupplier.get(); if (model == null) return null; - if (invalidations.contains(cacheKey)) return model; + if (invalidations.contains(cacheKey)) { + if (consumer != null) { + for (R policy: model) { + consumer.accept(policy); + } + } + return model; + }; query = querySupplier.apply(loaded, model); cache.addRevisioned(query, startupRevision); if (consumer != null) { @@ -930,7 +947,14 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider { Long loaded = cache.getCurrentRevision(cacheKey); List model = resultSupplier.get(); if (model == null) return null; - if (invalidations.contains(cacheKey)) return model; + if (invalidations.contains(cacheKey)) { + if (consumer != null) { + for (R policy: model) { + consumer.accept(policy); + } + } + return model; + }; query = querySupplier.apply(loaded, model); cache.addRevisioned(query, startupRevision); if (consumer != null) { diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAStoreFactory.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAStoreFactory.java index cd08f0c8e6..b78a471171 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAStoreFactory.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAStoreFactory.java @@ -38,6 +38,7 @@ public class JPAStoreFactory implements StoreFactory { private final ResourceStore resourceStore; private final ScopeStore scopeStore; private final JPAPermissionTicketStore permissionTicketStore; + private boolean readOnly; public JPAStoreFactory(EntityManager entityManager, AuthorizationProvider provider) { policyStore = new JPAPolicyStore(entityManager, provider); @@ -76,4 +77,14 @@ public class JPAStoreFactory implements StoreFactory { public void close() { } + + @Override + public void setReadOnly(boolean readOnly) { + this.readOnly = readOnly; + } + + @Override + public boolean isReadOnly() { + return readOnly; + } } diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/PolicyAdapter.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/PolicyAdapter.java index a761370d1e..4746b20be5 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/PolicyAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/PolicyAdapter.java @@ -19,6 +19,7 @@ package org.keycloak.authorization.jpa.store; import org.keycloak.authorization.jpa.entities.PolicyEntity; import org.keycloak.authorization.jpa.entities.ResourceEntity; import org.keycloak.authorization.jpa.entities.ScopeEntity; +import org.keycloak.authorization.model.AbstractAuthorizationModel; import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; @@ -39,12 +40,13 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class PolicyAdapter implements Policy, JpaModel { +public class PolicyAdapter extends AbstractAuthorizationModel implements Policy, JpaModel { private PolicyEntity entity; private EntityManager em; private StoreFactory storeFactory; public PolicyAdapter(PolicyEntity entity, EntityManager em, StoreFactory storeFactory) { + super(storeFactory); this.entity = entity; this.em = em; this.storeFactory = storeFactory; @@ -72,6 +74,7 @@ public class PolicyAdapter implements Policy, JpaModel { @Override public void setDecisionStrategy(DecisionStrategy decisionStrategy) { + throwExceptionIfReadonly(); entity.setDecisionStrategy(decisionStrategy); } @@ -83,6 +86,7 @@ public class PolicyAdapter implements Policy, JpaModel { @Override public void setLogic(Logic logic) { + throwExceptionIfReadonly(); entity.setLogic(logic); } @@ -95,6 +99,7 @@ public class PolicyAdapter implements Policy, JpaModel { @Override public void setConfig(Map config) { + throwExceptionIfReadonly(); if (entity.getConfig() == null) { entity.setConfig(new HashMap<>()); } else { @@ -105,6 +110,7 @@ public class PolicyAdapter implements Policy, JpaModel { @Override public void removeConfig(String name) { + throwExceptionIfReadonly(); if (entity.getConfig() == null) { return; } @@ -113,6 +119,7 @@ public class PolicyAdapter implements Policy, JpaModel { @Override public void putConfig(String name, String value) { + throwExceptionIfReadonly(); if (entity.getConfig() == null) { entity.setConfig(new HashMap<>()); } @@ -127,6 +134,7 @@ public class PolicyAdapter implements Policy, JpaModel { @Override public void setName(String name) { + throwExceptionIfReadonly(); entity.setName(name); } @@ -138,6 +146,7 @@ public class PolicyAdapter implements Policy, JpaModel { @Override public void setDescription(String description) { + throwExceptionIfReadonly(); entity.setDescription(description); } @@ -176,38 +185,45 @@ public class PolicyAdapter implements Policy, JpaModel { @Override public void addScope(Scope scope) { + throwExceptionIfReadonly(); entity.getScopes().add(ScopeAdapter.toEntity(em, scope)); } @Override public void removeScope(Scope scope) { + throwExceptionIfReadonly(); entity.getScopes().remove(ScopeAdapter.toEntity(em, scope)); } @Override public void addAssociatedPolicy(Policy associatedPolicy) { + throwExceptionIfReadonly(); entity.getAssociatedPolicies().add(toEntity(em, associatedPolicy)); } @Override public void removeAssociatedPolicy(Policy associatedPolicy) { + throwExceptionIfReadonly(); entity.getAssociatedPolicies().remove(toEntity(em, associatedPolicy)); } @Override public void addResource(Resource resource) { + throwExceptionIfReadonly(); entity.getResources().add(ResourceAdapter.toEntity(em, resource)); } @Override public void removeResource(Resource resource) { + throwExceptionIfReadonly(); entity.getResources().remove(ResourceAdapter.toEntity(em, resource)); } @Override public void setOwner(String owner) { + throwExceptionIfReadonly(); entity.setOwner(owner); } diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/ResourceAdapter.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/ResourceAdapter.java index 82735ed5cb..a7a295e92c 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/ResourceAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/ResourceAdapter.java @@ -19,6 +19,7 @@ package org.keycloak.authorization.jpa.store; import org.keycloak.authorization.jpa.entities.ResourceAttributeEntity; import org.keycloak.authorization.jpa.entities.ResourceEntity; import org.keycloak.authorization.jpa.entities.ScopeEntity; +import org.keycloak.authorization.model.AbstractAuthorizationModel; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; @@ -43,13 +44,14 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class ResourceAdapter implements Resource, JpaModel { +public class ResourceAdapter extends AbstractAuthorizationModel implements Resource, JpaModel { private ResourceEntity entity; private EntityManager em; private StoreFactory storeFactory; public ResourceAdapter(ResourceEntity entity, EntityManager em, StoreFactory storeFactory) { + super(storeFactory); this.entity = entity; this.em = em; this.storeFactory = storeFactory; @@ -77,6 +79,7 @@ public class ResourceAdapter implements Resource, JpaModel { @Override public void setDisplayName(String name) { + throwExceptionIfReadonly(); entity.setDisplayName(name); } @@ -87,11 +90,13 @@ public class ResourceAdapter implements Resource, JpaModel { @Override public void updateUris(Set uri) { + throwExceptionIfReadonly(); entity.setUris(uri); } @Override public void setName(String name) { + throwExceptionIfReadonly(); entity.setName(name); } @@ -103,6 +108,7 @@ public class ResourceAdapter implements Resource, JpaModel { @Override public void setType(String type) { + throwExceptionIfReadonly(); entity.setType(type); } @@ -124,6 +130,7 @@ public class ResourceAdapter implements Resource, JpaModel { @Override public void setIconUri(String iconUri) { + throwExceptionIfReadonly(); entity.setIconUri(iconUri); } @@ -145,11 +152,13 @@ public class ResourceAdapter implements Resource, JpaModel { @Override public void setOwnerManagedAccess(boolean ownerManagedAccess) { + throwExceptionIfReadonly(); entity.setOwnerManagedAccess(ownerManagedAccess); } @Override public void updateScopes(Set toUpdate) { + throwExceptionIfReadonly(); Set ids = new HashSet<>(); for (Scope scope : toUpdate) { ids.add(scope.getId()); @@ -171,7 +180,7 @@ public class ResourceAdapter implements Resource, JpaModel { for (ResourceAttributeEntity attr : entity.getAttributes()) { result.add(attr.getName(), attr.getValue()); } - return result; + return Collections.unmodifiableMap(result); } @Override @@ -213,6 +222,7 @@ public class ResourceAdapter implements Resource, JpaModel { @Override public void removeAttribute(String name) { + throwExceptionIfReadonly(); Query query = em.createNamedQuery("deleteResourceAttributesByNameAndResource"); query.setParameter("name", name); diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/ResourceServerAdapter.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/ResourceServerAdapter.java index 72c7cc1c92..a3098ef1f7 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/ResourceServerAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/ResourceServerAdapter.java @@ -17,6 +17,7 @@ package org.keycloak.authorization.jpa.store; import org.keycloak.authorization.jpa.entities.ResourceServerEntity; +import org.keycloak.authorization.model.AbstractAuthorizationModel; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.store.StoreFactory; import org.keycloak.models.jpa.JpaModel; @@ -28,12 +29,13 @@ import javax.persistence.EntityManager; * @author Bill Burke * @version $Revision: 1 $ */ -public class ResourceServerAdapter implements ResourceServer, JpaModel { +public class ResourceServerAdapter extends AbstractAuthorizationModel implements ResourceServer, JpaModel { private ResourceServerEntity entity; private EntityManager em; private StoreFactory storeFactory; public ResourceServerAdapter(ResourceServerEntity entity, EntityManager em, StoreFactory storeFactory) { + super(storeFactory); this.entity = entity; this.em = em; this.storeFactory = storeFactory; @@ -56,6 +58,7 @@ public class ResourceServerAdapter implements ResourceServer, JpaModelBill Burke * @version $Revision: 1 $ */ -public class ScopeAdapter implements Scope, JpaModel { +public class ScopeAdapter extends AbstractAuthorizationModel implements Scope, JpaModel { private ScopeEntity entity; private EntityManager em; private StoreFactory storeFactory; public ScopeAdapter(ScopeEntity entity, EntityManager em, StoreFactory storeFactory) { + super(storeFactory); this.entity = entity; this.em = em; this.storeFactory = storeFactory; @@ -56,6 +58,7 @@ public class ScopeAdapter implements Scope, JpaModel { @Override public void setName(String name) { + throwExceptionIfReadonly(); entity.setName(name); } @@ -67,6 +70,7 @@ public class ScopeAdapter implements Scope, JpaModel { @Override public void setDisplayName(String name) { + throwExceptionIfReadonly(); entity.setDisplayName(name); } @@ -77,6 +81,7 @@ public class ScopeAdapter implements Scope, JpaModel { @Override public void setIconUri(String iconUri) { + throwExceptionIfReadonly(); entity.setIconUri(iconUri); } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java b/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java index 50828a6470..bd1d88d088 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java @@ -224,6 +224,16 @@ public final class AuthorizationProvider implements Provider { public void close() { storeFactory.close(); } + + @Override + public void setReadOnly(boolean readOnly) { + storeFactory.setReadOnly(readOnly); + } + + @Override + public boolean isReadOnly() { + return storeFactory.isReadOnly(); + } }; } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/model/AbstractAuthorizationModel.java b/server-spi-private/src/main/java/org/keycloak/authorization/model/AbstractAuthorizationModel.java new file mode 100644 index 0000000000..2f35aeaa39 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/authorization/model/AbstractAuthorizationModel.java @@ -0,0 +1,34 @@ +/* + * 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.authorization.model; + +import org.keycloak.authorization.store.StoreFactory; + +public abstract class AbstractAuthorizationModel { + + private StoreFactory storeFactory; + + public AbstractAuthorizationModel(StoreFactory storeFactory) { + this.storeFactory = storeFactory; + } + + protected void throwExceptionIfReadonly() { + if (storeFactory.isReadOnly()) { + throw new IllegalStateException("Instance marked as read-only"); + } + } +} diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/permission/evaluator/IterablePermissionEvaluator.java b/server-spi-private/src/main/java/org/keycloak/authorization/permission/evaluator/IterablePermissionEvaluator.java index 959b27741a..b4410e67ae 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/permission/evaluator/IterablePermissionEvaluator.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/permission/evaluator/IterablePermissionEvaluator.java @@ -30,6 +30,7 @@ import org.keycloak.authorization.permission.ResourcePermission; import org.keycloak.authorization.policy.evaluation.DecisionPermissionCollector; import org.keycloak.authorization.policy.evaluation.EvaluationContext; import org.keycloak.authorization.policy.evaluation.PolicyEvaluator; +import org.keycloak.authorization.store.StoreFactory; import org.keycloak.representations.idm.authorization.AuthorizationRequest; import org.keycloak.representations.idm.authorization.Permission; @@ -52,9 +53,13 @@ class IterablePermissionEvaluator implements PermissionEvaluator { @Override public Decision evaluate(Decision decision) { + StoreFactory storeFactory = authorizationProvider.getStoreFactory(); + try { Map> decisionCache = new HashMap<>(); + storeFactory.setReadOnly(true); + while (this.permissions.hasNext()) { this.policyEvaluator.evaluate(this.permissions.next(), authorizationProvider, executionContext, decision, decisionCache); } @@ -62,7 +67,10 @@ class IterablePermissionEvaluator implements PermissionEvaluator { decision.onComplete(); } catch (Throwable cause) { decision.onError(cause); + } finally { + storeFactory.setReadOnly(false); } + return decision; } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultEvaluation.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultEvaluation.java index 2bc912bd04..6a456b6dc6 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultEvaluation.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultEvaluation.java @@ -263,8 +263,7 @@ public class DefaultEvaluation implements Evaluation { @Override public Map> getUserAttributes(String id) { - Map> attributes = getUser(id, authorizationProvider.getKeycloakSession()).getAttributes(); - return attributes; + return Collections.unmodifiableMap(getUser(id, authorizationProvider.getKeycloakSession()).getAttributes()); } }; } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/store/StoreFactory.java b/server-spi-private/src/main/java/org/keycloak/authorization/store/StoreFactory.java index a1123a94ac..d95f9e85be 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/store/StoreFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/store/StoreFactory.java @@ -64,4 +64,19 @@ public interface StoreFactory extends Provider { * @return the permission ticket store */ PermissionTicketStore getPermissionTicketStore(); + + /** + * Sets whether or not changes to instances returned from this factory are supported. Once marked as read-only, any attempt to + * change state will throw an {@link IllegalStateException}. + * + * @param readOnly if true, changes are not supported + */ + void setReadOnly(boolean readOnly); + + /** + * Indicates if instances returned from storage are read-only. + * + * @return if true, instances only support reads. + */ + boolean isReadOnly(); } diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java index 1ee4d9136a..42d64eccf0 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java @@ -41,14 +41,13 @@ import org.keycloak.OAuthErrorException; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.admin.representation.PolicyEvaluationResponseBuilder; import org.keycloak.authorization.attribute.Attributes; -import org.keycloak.authorization.common.KeycloakEvaluationContext; +import org.keycloak.authorization.common.DefaultEvaluationContext; import org.keycloak.authorization.common.KeycloakIdentity; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.permission.ResourcePermission; import org.keycloak.authorization.policy.evaluation.DecisionPermissionCollector; -import org.keycloak.authorization.policy.evaluation.DecisionResultCollector; import org.keycloak.authorization.policy.evaluation.EvaluationContext; import org.keycloak.authorization.policy.evaluation.Result; import org.keycloak.authorization.store.ScopeStore; @@ -132,7 +131,7 @@ public class PolicyEvaluationService { } private EvaluationContext createEvaluationContext(PolicyEvaluationRequest representation, KeycloakIdentity identity) { - return new KeycloakEvaluationContext(identity, this.authorization.getKeycloakSession()) { + return new DefaultEvaluationContext(identity, this.authorization.getKeycloakSession()) { @Override public Attributes getAttributes() { Map> attributes = new HashMap<>(super.getAttributes().toMap()); diff --git a/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java b/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java index 26f3a16544..bc61c3a8c1 100644 --- a/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java +++ b/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java @@ -42,12 +42,13 @@ import org.jboss.logging.Logger; import org.jboss.resteasy.spi.HttpRequest; import org.keycloak.OAuthErrorException; import org.keycloak.authorization.AuthorizationProvider; -import org.keycloak.authorization.common.KeycloakEvaluationContext; +import org.keycloak.authorization.common.DefaultEvaluationContext; import org.keycloak.authorization.common.KeycloakIdentity; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.permission.ResourcePermission; +import org.keycloak.authorization.policy.evaluation.EvaluationContext; import org.keycloak.authorization.policy.evaluation.PermissionTicketAwareDecisionResultCollector; import org.keycloak.authorization.store.ResourceServerStore; import org.keycloak.authorization.store.ResourceStore; @@ -92,7 +93,7 @@ public class AuthorizationTokenService { private static final String RESPONSE_MODE_DECISION = "decision"; private static final String RESPONSE_MODE_PERMISSIONS = "permissions"; private static final String RESPONSE_MODE_DECISION_RESULT = "result"; - private static Map> SUPPORTED_CLAIM_TOKEN_FORMATS; + private static Map> SUPPORTED_CLAIM_TOKEN_FORMATS; static { SUPPORTED_CLAIM_TOKEN_FORMATS = new HashMap<>(); @@ -103,7 +104,7 @@ public class AuthorizationTokenService { try { Map claims = JsonSerialization.readValue(Base64Url.decode(authorizationRequest.getClaimToken()), Map.class); authorizationRequest.setClaims(claims); - return new KeycloakEvaluationContext(new KeycloakIdentity(authorization.getKeycloakSession(), Tokens.getAccessToken(authorizationRequest.getSubjectToken(), authorization.getKeycloakSession())), claims, authorization.getKeycloakSession()); + return new DefaultEvaluationContext(new KeycloakIdentity(authorization.getKeycloakSession(), Tokens.getAccessToken(authorizationRequest.getSubjectToken(), authorization.getKeycloakSession())), claims, authorization.getKeycloakSession()); } catch (IOException cause) { throw new RuntimeException("Failed to map claims from claim token [" + claimToken + "]", cause); } @@ -114,7 +115,6 @@ public class AuthorizationTokenService { SUPPORTED_CLAIM_TOKEN_FORMATS.put(CLAIM_TOKEN_FORMAT_ID_TOKEN, (authorizationRequest, authorization) -> { try { KeycloakSession keycloakSession = authorization.getKeycloakSession(); - RealmModel realm = authorization.getRealm(); String accessToken = authorizationRequest.getSubjectToken(); if (accessToken == null) { @@ -122,7 +122,7 @@ public class AuthorizationTokenService { } IDToken idToken = new TokenManager().verifyIDTokenSignature(keycloakSession, accessToken); - return new KeycloakEvaluationContext(new KeycloakIdentity(keycloakSession, idToken), authorizationRequest.getClaims(), keycloakSession); + return new DefaultEvaluationContext(new KeycloakIdentity(keycloakSession, idToken), authorizationRequest.getClaims(), keycloakSession); } catch (OAuthErrorException cause) { throw new RuntimeException("Failed to verify ID token", cause); } @@ -151,7 +151,7 @@ public class AuthorizationTokenService { request.setClaims(ticket.getClaims()); ResourceServer resourceServer = getResourceServer(ticket, request); - KeycloakEvaluationContext evaluationContext = createEvaluationContext(request); + EvaluationContext evaluationContext = createEvaluationContext(request); KeycloakIdentity identity = KeycloakIdentity.class.cast(evaluationContext.getIdentity()); Collection permissions; @@ -213,21 +213,21 @@ public class AuthorizationTokenService { return request.getClaimToken() != null && request.getKeycloakSession().getContext().getClient().isPublicClient() && request.getTicket() == null; } - private Collection evaluatePermissions(KeycloakAuthorizationRequest request, PermissionTicketToken ticket, ResourceServer resourceServer, KeycloakEvaluationContext evaluationContext, KeycloakIdentity identity) { + private Collection evaluatePermissions(KeycloakAuthorizationRequest request, PermissionTicketToken ticket, ResourceServer resourceServer, EvaluationContext evaluationContext, KeycloakIdentity identity) { AuthorizationProvider authorization = request.getAuthorization(); return authorization.evaluators() .from(createPermissions(ticket, request, resourceServer, identity, authorization), evaluationContext) .evaluate(resourceServer, request); } - private Collection evaluateUserManagedPermissions(KeycloakAuthorizationRequest request, PermissionTicketToken ticket, ResourceServer resourceServer, KeycloakEvaluationContext evaluationContext, KeycloakIdentity identity) { + private Collection evaluateUserManagedPermissions(KeycloakAuthorizationRequest request, PermissionTicketToken ticket, ResourceServer resourceServer, EvaluationContext evaluationContext, KeycloakIdentity identity) { AuthorizationProvider authorization = request.getAuthorization(); return authorization.evaluators() .from(createPermissions(ticket, request, resourceServer, identity, authorization), evaluationContext) .evaluate(new PermissionTicketAwareDecisionResultCollector(request, ticket, identity, resourceServer, authorization)).results(); } - private Collection evaluateAllPermissions(KeycloakAuthorizationRequest request, ResourceServer resourceServer, KeycloakEvaluationContext evaluationContext, KeycloakIdentity identity) { + private Collection evaluateAllPermissions(KeycloakAuthorizationRequest request, ResourceServer resourceServer, EvaluationContext evaluationContext, KeycloakIdentity identity) { AuthorizationProvider authorization = request.getAuthorization(); return authorization.evaluators() .from(Permissions.all(resourceServer, identity, authorization, request), evaluationContext) @@ -340,14 +340,14 @@ public class AuthorizationTokenService { return resourceServer; } - private KeycloakEvaluationContext createEvaluationContext(KeycloakAuthorizationRequest request) { + private EvaluationContext createEvaluationContext(KeycloakAuthorizationRequest request) { String claimTokenFormat = request.getClaimTokenFormat(); if (claimTokenFormat == null) { claimTokenFormat = CLAIM_TOKEN_FORMAT_ID_TOKEN; } - BiFunction evaluationContextProvider = SUPPORTED_CLAIM_TOKEN_FORMATS.get(claimTokenFormat); + BiFunction evaluationContextProvider = SUPPORTED_CLAIM_TOKEN_FORMATS.get(claimTokenFormat); if (evaluationContextProvider == null) { throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Claim token format [" + claimTokenFormat + "] not supported", Status.BAD_REQUEST); diff --git a/services/src/main/java/org/keycloak/authorization/common/DefaultEvaluationContext.java b/services/src/main/java/org/keycloak/authorization/common/DefaultEvaluationContext.java index e740cdf393..3131566043 100644 --- a/services/src/main/java/org/keycloak/authorization/common/DefaultEvaluationContext.java +++ b/services/src/main/java/org/keycloak/authorization/common/DefaultEvaluationContext.java @@ -22,6 +22,7 @@ import org.keycloak.authorization.attribute.Attributes; import org.keycloak.authorization.identity.Identity; import org.keycloak.authorization.policy.evaluation.EvaluationContext; import org.keycloak.models.KeycloakSession; +import org.keycloak.representations.AccessToken; import java.text.SimpleDateFormat; import java.util.Arrays; @@ -40,6 +41,7 @@ public class DefaultEvaluationContext implements EvaluationContext { protected final KeycloakSession keycloakSession; protected final Identity identity; private final Map> claims; + private Attributes attributes; public DefaultEvaluationContext(Identity identity, KeycloakSession keycloakSession) { this(identity, null, keycloakSession); @@ -56,7 +58,7 @@ public class DefaultEvaluationContext implements EvaluationContext { return identity; } - public Map> getBaseAttributes() { + protected Map> getBaseAttributes() { Map> attributes = new HashMap<>(); attributes.put("kc.time.date_time", Arrays.asList(new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").format(new Date()))); @@ -77,11 +79,22 @@ public class DefaultEvaluationContext implements EvaluationContext { } } + if (KeycloakIdentity.class.isInstance(identity)) { + AccessToken accessToken = KeycloakIdentity.class.cast(this.identity).getAccessToken(); + + if (accessToken != null) { + attributes.put("kc.client.id", Arrays.asList(accessToken.getIssuedFor())); + } + } + return attributes; } @Override public Attributes getAttributes() { - return Attributes.from(getBaseAttributes()); + if (attributes == null) { + attributes = Attributes.from(getBaseAttributes()); + } + return attributes; } } diff --git a/services/src/main/java/org/keycloak/authorization/common/KeycloakEvaluationContext.java b/services/src/main/java/org/keycloak/authorization/common/KeycloakEvaluationContext.java deleted file mode 100644 index bbb4218c1c..0000000000 --- a/services/src/main/java/org/keycloak/authorization/common/KeycloakEvaluationContext.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * JBoss, Home of Professional Open Source. - * Copyright 2016 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.keycloak.authorization.common; - -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Map; - -import org.keycloak.authorization.identity.Identity; -import org.keycloak.models.KeycloakSession; -import org.keycloak.representations.AccessToken; - -/** - * @author Pedro Igor - */ -public class KeycloakEvaluationContext extends DefaultEvaluationContext { - - private final KeycloakIdentity identity; - - public KeycloakEvaluationContext(KeycloakIdentity identity, KeycloakSession keycloakSession) { - this(identity, null, keycloakSession); - } - - public KeycloakEvaluationContext(KeycloakIdentity identity, Map> claims, KeycloakSession keycloakSession) { - super(identity, claims, keycloakSession); - this.identity = identity; - } - - @Override - public Identity getIdentity() { - return this.identity; - } - - @Override - public Map> getBaseAttributes() { - Map> attributes = super.getBaseAttributes(); - AccessToken accessToken = this.identity.getAccessToken(); - - if (accessToken != null) { - attributes.put("kc.client.id", Arrays.asList(accessToken.getIssuedFor())); - } - return attributes; - } -} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationTest.java index 0b3795d6db..fedc6e8adb 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationTest.java @@ -19,8 +19,10 @@ package org.keycloak.testsuite.authz; import java.text.SimpleDateFormat; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.function.Function; @@ -28,11 +30,9 @@ import java.util.stream.Collectors; import org.jboss.arquillian.container.test.api.Deployment; import org.jboss.shrinkwrap.api.spec.WebArchive; -import org.jetbrains.annotations.NotNull; import org.junit.Assert; import org.junit.Test; import org.keycloak.authorization.AuthorizationProvider; -import org.keycloak.authorization.Decision; import org.keycloak.authorization.Decision.Effect; import org.keycloak.authorization.attribute.Attributes; import org.keycloak.authorization.common.DefaultEvaluationContext; @@ -40,9 +40,10 @@ import org.keycloak.authorization.identity.Identity; import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; +import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.permission.ResourcePermission; +import org.keycloak.authorization.permission.evaluator.PermissionEvaluator; import org.keycloak.authorization.policy.evaluation.DefaultEvaluation; -import org.keycloak.authorization.policy.evaluation.Evaluation; import org.keycloak.authorization.policy.provider.PolicyProvider; import org.keycloak.authorization.store.StoreFactory; import org.keycloak.models.ClientModel; @@ -57,6 +58,7 @@ import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.authorization.JSPolicyRepresentation; +import org.keycloak.representations.idm.authorization.ResourcePermissionRepresentation; import org.keycloak.representations.idm.authorization.TimePolicyRepresentation; import org.keycloak.testsuite.runonserver.RunOnServerDeployment; import org.keycloak.testsuite.util.ClientBuilder; @@ -630,6 +632,52 @@ public class PolicyEvaluationTest extends AbstractAuthzTest { Assert.assertEquals(Effect.PERMIT, evaluation.getEffect()); } + @Test + public void testCheckReadOnlyInstances() { + testingClient.server().run(PolicyEvaluationTest::testCheckReadOnlyInstances); + } + + public static void testCheckReadOnlyInstances(KeycloakSession session) { + session.getContext().setRealm(session.realms().getRealmByName("authz-test")); + AuthorizationProvider authorization = session.getProvider(AuthorizationProvider.class); + ClientModel clientModel = session.realms().getClientByClientId("resource-server-test", session.getContext().getRealm()); + StoreFactory storeFactory = authorization.getStoreFactory(); + ResourceServer resourceServer = storeFactory.getResourceServerStore().findById(clientModel.getId()); + JSPolicyRepresentation policyRepresentation = new JSPolicyRepresentation(); + + policyRepresentation.setName("testCheckReadOnlyInstances"); + StringBuilder builder = new StringBuilder(); + + builder.append("$evaluation.getPermission().getResource().setName('test')"); + + policyRepresentation.setCode(builder.toString()); + + Policy policy = storeFactory.getPolicyStore().create(policyRepresentation, resourceServer); + + Resource resource = storeFactory.getResourceStore().create("Resource A", resourceServer, resourceServer.getId()); + Scope scope = storeFactory.getScopeStore().create("Scope A", resourceServer); + + resource.updateScopes(new HashSet<>(Arrays.asList(scope))); + + ResourcePermissionRepresentation permission = new ResourcePermissionRepresentation(); + + permission.setName("testCheckReadOnlyInstances permission"); + permission.addPolicy(policy.getId()); + permission.addResource(resource.getId()); + + storeFactory.getPolicyStore().create(permission, resourceServer); + + session.getTransactionManager().commit(); + + PermissionEvaluator evaluator = authorization.evaluators().from(Arrays.asList(new ResourcePermission(resource, Arrays.asList(scope), resourceServer)), createEvaluationContext(session, Collections.emptyMap())); + + try { + evaluator.evaluate(resourceServer, null); + Assert.fail("Instances should be marked as read-only"); + } catch (Exception ignore) { + } + } + private static DefaultEvaluation createEvaluation(KeycloakSession session, AuthorizationProvider authorization, ResourceServer resourceServer, Policy policy) { return createEvaluation(session, authorization, null, resourceServer, policy); } @@ -641,7 +689,11 @@ public class PolicyEvaluationTest extends AbstractAuthzTest { private static DefaultEvaluation createEvaluation(KeycloakSession session, AuthorizationProvider authorization, Resource resource, ResourceServer resourceServer, Policy policy, Map> contextAttributes) { - return new DefaultEvaluation(new ResourcePermission(resource, null, resourceServer), new DefaultEvaluationContext(new Identity() { + return new DefaultEvaluation(new ResourcePermission(resource, null, resourceServer), createEvaluationContext(session, contextAttributes), policy, evaluation -> {}, authorization, null); + } + + private static DefaultEvaluationContext createEvaluationContext(KeycloakSession session, Map> contextAttributes) { + return new DefaultEvaluationContext(new Identity() { @Override public String getId() { return null; @@ -664,6 +716,6 @@ public class PolicyEvaluationTest extends AbstractAuthzTest { } return baseAttributes; } - }, policy, evaluation -> {}, authorization, null); + }; } }