From 8581886944f08c59ad88a0111be9e0f2ef7360e7 Mon Sep 17 00:00:00 2001
From: Stefan Guilhen <sguilhen@redhat.com>
Date: Fri, 18 Oct 2024 12:49:10 -0300
Subject: [PATCH] Add validation for role and time policies

Closes #28978

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
---
 authz/policy/common/pom.xml                   |  5 +++
 .../role/RolePolicyProviderFactory.java       |  7 ++--
 .../time/TimePolicyProviderFactory.java       | 36 ++++++++++++++++---
 .../util/PolicyValidationException.java       | 29 +++++++++++++++
 .../admin/messages/messages_en.properties     |  2 +-
 5 files changed, 71 insertions(+), 8 deletions(-)
 create mode 100644 authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/util/PolicyValidationException.java

diff --git a/authz/policy/common/pom.xml b/authz/policy/common/pom.xml
index ec7c0f5ddc..eb1e18fbe7 100644
--- a/authz/policy/common/pom.xml
+++ b/authz/policy/common/pom.xml
@@ -51,6 +51,11 @@
             <artifactId>keycloak-server-spi-private</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>io.quarkus.resteasy.reactive</groupId>
+            <artifactId>resteasy-reactive-common</artifactId>
+            <scope>provided</scope>
+        </dependency>
         <dependency>
             <groupId>org.jboss.logging</groupId>
             <artifactId>jboss-logging</artifactId>
diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProviderFactory.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProviderFactory.java
index 2ebc91ee5a..8b9e3c1c96 100644
--- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProviderFactory.java
+++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProviderFactory.java
@@ -22,6 +22,7 @@ import org.keycloak.authorization.AuthorizationProvider;
 import org.keycloak.authorization.model.Policy;
 import org.keycloak.authorization.policy.provider.PolicyProvider;
 import org.keycloak.authorization.policy.provider.PolicyProviderFactory;
+import org.keycloak.authorization.policy.provider.util.PolicyValidationException;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
@@ -150,7 +151,7 @@ public class RolePolicyProviderFactory implements PolicyProviderFactory<RolePoli
 
     private void updateRoles(Policy policy, AuthorizationProvider authorization, Set<RolePolicyRepresentation.RoleDefinition> roles) {
         Set<RolePolicyRepresentation.RoleDefinition> updatedRoles = new HashSet<>();
-
+        Set<String> processedRoles = new HashSet<>();
         if (roles != null) {
             RealmModel realm = authorization.getRealm();
             for (RolePolicyRepresentation.RoleDefinition definition : roles) {
@@ -159,8 +160,10 @@ public class RolePolicyProviderFactory implements PolicyProviderFactory<RolePoli
                     continue;
                 }
 
+                if (!processedRoles.add(role.getId())) {
+                    throw new PolicyValidationException("Role can't be specified multiple times - " + role.getName());
+                }
                 definition.setId(role.getId());
-
                 updatedRoles.add(definition);
             }
         }
diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/time/TimePolicyProviderFactory.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/time/TimePolicyProviderFactory.java
index c1e3f4116a..90b86ad028 100644
--- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/time/TimePolicyProviderFactory.java
+++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/time/TimePolicyProviderFactory.java
@@ -1,6 +1,23 @@
+/*
+ * Copyright 2024 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.policy.provider.time;
 
 import java.text.SimpleDateFormat;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -9,6 +26,7 @@ import org.keycloak.authorization.AuthorizationProvider;
 import org.keycloak.authorization.model.Policy;
 import org.keycloak.authorization.policy.provider.PolicyProvider;
 import org.keycloak.authorization.policy.provider.PolicyProviderFactory;
+import org.keycloak.authorization.policy.provider.util.PolicyValidationException;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.representations.idm.authorization.PolicyRepresentation;
@@ -116,8 +134,7 @@ public class TimePolicyProviderFactory implements PolicyProviderFactory<TimePoli
         String noa = representation.getNotOnOrAfter();
 
         if (nbf != null && noa != null) {
-            validateFormat(nbf);
-            validateFormat(noa);
+            validateFormat(nbf, noa);
         }
 
         Map<String, String> config = new HashMap(policy.getConfig());
@@ -143,11 +160,20 @@ public class TimePolicyProviderFactory implements PolicyProviderFactory<TimePoli
         policy.setConfig(config);
     }
 
-    private void validateFormat(String date) {
+    private void validateFormat(String notBefore, String notOnOrAfter) {
+        Date nbf, noa;
         try {
-            new SimpleDateFormat(TimePolicyProvider.DEFAULT_DATE_PATTERN).parse(TimePolicyProvider.format(date));
+            nbf = new SimpleDateFormat(TimePolicyProvider.DEFAULT_DATE_PATTERN).parse(TimePolicyProvider.format(notBefore));
         } catch (Exception e) {
-            throw new RuntimeException("Could not parse a date using format [" + date + "]");
+            throw new PolicyValidationException("Unable not parse a date using format [" + notBefore + "]");
+        }
+        try {
+            noa = new SimpleDateFormat(TimePolicyProvider.DEFAULT_DATE_PATTERN).parse(TimePolicyProvider.format(notOnOrAfter));
+        } catch (Exception e) {
+            throw new PolicyValidationException("Unable not parse a date using format [" + notOnOrAfter + "]");
+        }
+        if (noa.before(nbf)) {
+            throw new PolicyValidationException("Expire time can't be set to a date before start time");
         }
     }
 }
diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/util/PolicyValidationException.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/util/PolicyValidationException.java
new file mode 100644
index 0000000000..b37ce91119
--- /dev/null
+++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/util/PolicyValidationException.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2024 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.policy.provider.util;
+
+import jakarta.ws.rs.BadRequestException;
+
+/**
+ * Exception that is thrown when validation errors are found when creating/updating policies.
+ */
+public class PolicyValidationException extends BadRequestException {
+
+    public PolicyValidationException(String message) {
+        super(message);
+    }
+}
diff --git a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties
index 8d138c0826..702164fab8 100644
--- a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties
+++ b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties
@@ -2085,7 +2085,7 @@ ldapGeneralOptionsSettingsDescription=This section contains a few basic options
 importSkipped_one=One record skipped.
 eventTypes.OAUTH2_DEVICE_AUTH.description=OAuth2 device authentication
 notBeforeClearedSuccess=Success\! "Not Before" cleared for realm.
-policySaveError=Could not update the policy due to {{error}}
+policySaveError=Could not update the policy: {{error}}
 experimental=Experimental
 idTokenSignatureAlgorithmHelp=JWA algorithm used for signing ID tokens.
 deleteResourceConfirm=If you delete this resource, some permissions will be affected.