From 57139cbefc0327ab8635b7eb36ddafd1583094b0 Mon Sep 17 00:00:00 2001
From: Pedro Igor <pigor.craveiro@gmail.com>
Date: Tue, 18 Jun 2024 12:49:47 -0300
Subject: [PATCH] Internal read-only attributes have precedence over unmanaged
 attribute policy

Closes #30240

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
---
 .../userprofile/DefaultAttributes.java        |  8 +++---
 ...ountRestServiceReadOnlyAttributesTest.java | 27 +++++++++++++++++++
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java
index 82609bceeb..d752bbe79e 100644
--- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java
+++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java
@@ -94,14 +94,14 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
 
     @Override
     public boolean isReadOnly(String name) {
-        if (!isManagedAttribute(name)) {
-            return !isAllowEditUnmanagedAttribute();
-        }
-
         if (isReadOnlyFromMetadata(name) || isReadOnlyInternalAttribute(name)) {
             return true;
         }
 
+        if (!isManagedAttribute(name)) {
+            return !isAllowEditUnmanagedAttribute();
+        }
+
         return getMetadata(name) == null;
     }
 
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceReadOnlyAttributesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceReadOnlyAttributesTest.java
index 2a0c3030c0..7f5cfa8f27 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceReadOnlyAttributesTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceReadOnlyAttributesTest.java
@@ -38,6 +38,7 @@ import org.keycloak.representations.idm.ErrorRepresentation;
 import org.keycloak.representations.userprofile.config.UPAttribute;
 import org.keycloak.representations.userprofile.config.UPAttributePermissions;
 import org.keycloak.representations.userprofile.config.UPConfig;
+import org.keycloak.representations.userprofile.config.UPConfig.UnmanagedAttributePolicy;
 import org.keycloak.services.messages.Messages;
 import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.broker.util.SimpleHttpDefault;
@@ -47,6 +48,7 @@ import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -144,6 +146,31 @@ public class AccountRestServiceReadOnlyAttributesTest extends AbstractRestServic
         testAccountUpdateAttributeExpectSuccess("deniedSomeAdmin");
     }
 
+    @Test
+    public void testUpdateProfileCannotUpdateReadOnlyAttributesUnmanagedEnabled() throws IOException {
+        UPConfig configuration = testRealm().users().userProfile().getConfiguration();
+        UnmanagedAttributePolicy unmanagedAttributePolicy = configuration.getUnmanagedAttributePolicy();
+        configuration.setUnmanagedAttributePolicy(UnmanagedAttributePolicy.ENABLED);
+        getCleanup().addCleanup(() -> {
+            configuration.setUnmanagedAttributePolicy(unmanagedAttributePolicy);
+            testRealm().users().userProfile().update(configuration);
+        });
+        testRealm().users().userProfile().update(configuration);
+        UserRepresentation user = SimpleHttpDefault.doGet(getAccountUrl(null), httpClient).auth(tokenUtil.getToken()).asJson(UserRepresentation.class);
+        UserResource adminUserResource = ApiUtil.findUserByUsernameId(testRealm(), user.getUsername());
+        org.keycloak.representations.idm.UserRepresentation adminUserRep = adminUserResource.toRepresentation();
+        adminUserRep.singleAttribute("deniedFoo", "foo");
+        adminUserResource.update(adminUserRep);
+        adminUserResource = ApiUtil.findUserByUsernameId(testRealm(), user.getUsername());
+        adminUserRep = adminUserResource.toRepresentation();
+        assertEquals("foo", adminUserRep.getAttributes().get("deniedFoo").get(0));
+        assertNull(user.getAttributes());
+        updateAndGet(user);
+        adminUserResource = ApiUtil.findUserByUsernameId(testRealm(), user.getUsername());
+        adminUserRep = adminUserResource.toRepresentation();
+        assertEquals("foo", adminUserRep.getAttributes().get("deniedFoo").get(0));
+    }
+
     private void testAccountUpdateAttributeExpectFailure(String attrName) throws IOException {
         testAccountUpdateAttributeExpectFailure(attrName, false);
     }