OAK-10466: Allow to configure anonymous user disablement (#1882)

* OAK-10466: Allow to configure anonymous user disablement * OAK-10466: Apply PR comments * OAK-10466: Change anonymous evaluation Configuration check moved to be performed when the disable method is called
apache · Dec 12, 2024 · 9bc52a5 · 9bc52a5
1 parent da01d4a
commit 9bc52a5
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 19 deletions.
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java
@@ -195,6 +195,12 @@ public class UserConfigurationImpl extends ConfigurationBase implements UserConf
                 name = "RFC7613 Username Comparison Profile",
                 description = "Enable the UsercaseMappedProfile defined in RFC7613 for username comparison.")
         boolean enableRFC7613UsercaseMappedProfile() default false;
+
+        @AttributeDefinition(
+                name = "Allow Disable Anonymous User",
+                description = "By default the anonymous user can be disabled. By changing this option to false trying "
+                        + "to disable the anonymous user will throw an exception.")
+        boolean allowDisableAnonymous() default true;
     }
 
     private static final UserAuthenticationFactory DEFAULT_AUTH_FACTORY = new UserAuthenticationFactoryImpl();

diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImpl.java
@@ -25,6 +25,7 @@
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.jackrabbit.oak.spi.security.user.UserIdCredentials;
@@ -47,8 +48,9 @@ class UserImpl extends AuthorizableImpl implements User {
     UserImpl(String id, Tree tree, UserManagerImpl userManager) throws RepositoryException {
         super(id, tree, userManager);
 
-        isAdmin = UserUtil.isAdmin(userManager.getConfig(), id);
-        pwHistory = new PasswordHistory(userManager.getConfig());
+        ConfigurationParameters configurationParameters = userManager.getConfig();
+        isAdmin = UserUtil.isAdmin(configurationParameters, id);
+        pwHistory = new PasswordHistory(configurationParameters);
     }
 
     //---------------------------------------------------< AuthorizableImpl >---
@@ -131,9 +133,7 @@ public void changePassword(@Nullable String password, @NotNull String oldPasswor
 
     @Override
     public void disable(@Nullable String reason) throws RepositoryException {
-        if (isAdmin) {
-            throw new RepositoryException("The administrator user cannot be disabled.");
-        }
+        validateDisableUser();
 
         getUserManager().onDisable(this, reason);
 
@@ -148,6 +148,18 @@ public void disable(@Nullable String reason) throws RepositoryException {
         }
     }
 
+    private void validateDisableUser() throws RepositoryException {
+        if (isAdmin) {
+            throw new RepositoryException("The administrator user cannot be disabled.");
+        }
+
+        boolean isAnonymous = UserUtil.getAnonymousId(getUserManager().getConfig()).equals(getID());
+        boolean allowDisableAnonymous = getUserManager().getConfig().getConfigValue(UserConstants.PARAM_ALLOW_DISABLE_ANONYMOUS, true);
+        if (isAnonymous && !allowDisableAnonymous) {
+            throw new RepositoryException("The anonymous user cannot be disabled.");
+        }
+    }
+
     @Override
     public boolean isDisabled() {
         return getTree().hasProperty(REP_DISABLED);

diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java
@@ -204,6 +204,13 @@ protected UserManager getUserManager(Root root) {
         }
     }
 
+    /*
+     * Useful when needing to re-create the user manager instance while updating configuration
+     */
+    protected void cleanUserManager() {
+        userManager = null;
+    }
+
     protected PrincipalManager getPrincipalManager(Root root) {
         return getConfig(PrincipalConfiguration.class).getPrincipalManager(root, getNamePathMapper());
     }

diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImplTest.java
@@ -16,15 +16,27 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
+import static javax.jcr.Property.JCR_PRIMARY_TYPE;
+import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.NT_REP_GROUP;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 import javax.jcr.Credentials;
 import javax.jcr.RepositoryException;
-
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.UUIDUtils;
 import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.jackrabbit.oak.spi.security.user.UserIdCredentials;
 import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil;
@@ -33,19 +45,10 @@
 import org.junit.Before;
 import org.junit.Test;
 
-import static javax.jcr.Property.JCR_PRIMARY_TYPE;
-import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.NT_REP_GROUP;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
 public class UserImplTest extends AbstractSecurityTest {
 
     private User user;
+    private boolean allowDisableAnonymous = true;
 
     @Override
     @Before
@@ -71,9 +74,19 @@ private User getAdminUser() throws Exception {
         return admin;
     }
 
+    @NotNull
+    private User getAnonymousUser() throws Exception {
+        User anonymous = getUserManager(root).getAuthorizable(UserConstants.DEFAULT_ANONYMOUS_ID, User.class);
+        assertNotNull(anonymous);
+        return anonymous;
+    }
+
     @Test(expected = IllegalArgumentException.class)
     public void testCreateFromInvalidTree() throws Exception {
-        Tree t = when(mock(Tree.class).getProperty(JCR_PRIMARY_TYPE)).thenReturn(PropertyStates.createProperty(JCR_PRIMARY_TYPE, NT_REP_GROUP, Type.NAME)).getMock();
+        Tree t = when(mock(Tree.class).getProperty(JCR_PRIMARY_TYPE)).thenReturn(PropertyStates.createProperty(
+                JCR_PRIMARY_TYPE,
+                NT_REP_GROUP,
+                Type.NAME)).getMock();
         User u = new UserImpl("uid", t, (UserManagerImpl) getUserManager(root));
     }
 
@@ -165,6 +178,36 @@ public void testDisableNullReason() throws Exception {
         assertFalse(user.isDisabled());
     }
 
+    @Test
+    public void testDisableAnonymous() throws Exception {
+        User anonymous = getAnonymousUser();
+        assertFalse(anonymous.isDisabled());
+
+        anonymous.disable("Test anonymous disable");
+
+        assertNotNull(anonymous.getDisabledReason());
+        assertEquals("Test anonymous disable", anonymous.getDisabledReason());
+        assertTrue(anonymous.isDisabled());
+    }
+
+    @Test(expected = RepositoryException.class)
+    public void testDisableAnonymousNotAllowed() throws Exception {
+        // Dirty hack to prevent anonymous user from being disabled
+        // and initialize the repo again
+        allowDisableAnonymous = false;
+        securityProvider = null;
+
+        cleanUserManager();
+        before();
+
+        User anonymous = getAnonymousUser();
+        assertFalse(anonymous.isDisabled());
+
+        anonymous.disable("Test anonymous disable");
+
+        fail("Shouldn't have reached this point");
+    }
+
     @Test(expected = RepositoryException.class)
     public void testDisableAdministrator() throws Exception {
         getAdminUser().disable("reason");
@@ -182,10 +225,21 @@ public void testGetCredentials() throws Exception {
 
     @Test
     public void testGetCredentialsUserWithoutPassword() throws Exception {
-        User u = getUserManager(root).createUser("u"+ UUIDUtils.generateUUID(), null);
+        User u = getUserManager(root).createUser("u" + UUIDUtils.generateUUID(), null);
 
         Credentials creds = u.getCredentials();
         assertTrue(creds instanceof UserIdCredentials);
         assertEquals(u.getID(), ((UserIdCredentials) creds).getUserId());
     }
+
+    @Override
+    protected ConfigurationParameters getSecurityConfigParameters() {
+        if (allowDisableAnonymous) {
+            return super.getSecurityConfigParameters();
+        } else {
+            ConfigurationParameters userConfig = ConfigurationParameters.of(
+                    UserConstants.PARAM_ALLOW_DISABLE_ANONYMOUS, false);
+            return ConfigurationParameters.of(UserConfiguration.NAME, userConfig);
+        }
+    }
 }
diff --git a/...security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/UserConstants.java b/...security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/UserConstants.java
@@ -265,4 +265,11 @@ public interface UserConstants {
      * @since Oak 1.3.3
      */
     int PASSWORD_HISTORY_DISABLED_SIZE = 0;
+
+    /**
+     * Optional configuration parameter indicating if the anonymous user can be disabled or not.
+     * By default, the anonymous user can be disabled.
+     *
+     */
+    String PARAM_ALLOW_DISABLE_ANONYMOUS = "allowDisableAnonymous";
 }
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/package-info.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/package-info.java
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-@Version("2.7.0")
+@Version("2.8.0")
 package org.apache.jackrabbit.oak.spi.security.user;
 
 import org.osgi.annotation.versioning.Version;