joerghoh commented on code in PR #1882:
URL: https://github.com/apache/jackrabbit-oak/pull/1882#discussion_r1862429517
##########
oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImplTest.java:
##########
@@ -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'td have reached this point");
Review Comment:
```suggestion
fail("Shouldn't have reached this point");
```
##########
oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/UserConstants.java:
##########
@@ -265,4 +265,12 @@ 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.
+ *
+ * @since Oak 1.73.0
Review Comment:
there is no Oak 1.73.0; Oak uses the odd-even versioning scheme, so there is
an official 1.70.0, 1.72.0, 1.74.0 etc.
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImpl.java:
##########
@@ -42,12 +42,16 @@
class UserImpl extends AuthorizableImpl implements User {
private final boolean isAdmin;
+ private final boolean isAnonymous;
private final PasswordHistory pwHistory;
+ private final boolean allowDisableAnonymous;
UserImpl(String id, Tree tree, UserManagerImpl userManager) throws
RepositoryException {
super(id, tree, userManager);
isAdmin = UserUtil.isAdmin(userManager.getConfig(), id);
+ isAnonymous =
UserUtil.getAnonymousId(userManager.getConfig()).equals(id);
+ allowDisableAnonymous =
userManager.getConfig().getConfigValue(UserConstants.PARAM_ALLOW_DISABLE_ANONYMOUS,
true);
Review Comment:
Having this in the constructor will impose a small performance penalty on
every construction of a user object. Is it possible to move this code down into
the ``canDisableUser`` method, so it is executed only when it is needed?
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImpl.java:
##########
@@ -148,6 +150,16 @@ public void disable(@Nullable String reason) throws
RepositoryException {
}
}
+ private void canDisableUser() throws RepositoryException {
Review Comment:
nit: I am not super-happy with the name of this method, as it makes me think
that it returns a ``boolean``. But I don't have a good name for it either.
(Feel free to ignore this, if you disagree with it.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
