OAK-10466: Allow to configure anonymous user disablement #1882
Conversation
|
|
||
| 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); |
There was a problem hiding this 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?
There was a problem hiding this comment.
I also considered it but in that case I'd have to keep a reference to the configuration parameters on each UserImpl instance. Not sure if this could have a performance impact as well.
There was a problem hiding this comment.
@Amoratinos , it should be possible to retrieve the configuration params on the fly.... the usermanager field stored with the base class is an instance of UserManagerImpl, which comes with a method getConfig(), which are the config params obtained from UserConfiguration afaik.
so, i believe it's possible to move the evaluation to the validateDisableUser method and avoiding story isAnonymous and avoiding storing the config params again as they are already attached to the user manager
There was a problem hiding this comment.
You're right, changed to use getUserManager and removed those fields in favor of online evaluation.
oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImplTest.java
Outdated
Show resolved
Hide resolved
oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/UserConstants.java
Outdated
Show resolved
Hide resolved
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImpl.java
Outdated
Show resolved
Hide resolved
Configuration check moved to be performed when the disable method is called
|
@joerghoh , can you take another look to see if your concerns are addressed? ty. |
Created a configuration value that allows to prevent anonymous user to be disabled.
To support backward compatibility be default anonymous user can still be disabled