Index: oak-doc/src/site/markdown/security/user.md IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-doc/src/site/markdown/security/user.md (revision 1692521) +++ oak-doc/src/site/markdown/security/user.md (revision ) @@ -295,14 +295,20 @@ | `PARAM_PASSWORD_MAX_AGE` | int | 0 | | `PARAM_PASSWORD_INITIAL_CHANGE` | boolean | false | | `PARAM_PASSWORD_HISTORY_SIZE` | int (upper limit: 1000) | 0 | +| `PARAM_CACHE_EXPIRATION` | long | 0 | | | | | The following configuration parameters present with the default implementation in Jackrabbit 2.x are no longer supported and will be ignored: -* 'compatibleJR16' -* 'autoExpandTree' -* 'autoExpandSize' -* 'groupMembershipSplitSize' +* `compatibleJR16` +* `autoExpandTree` +* `autoExpandSize` +* `groupMembershipSplitSize` + +The optional `cacheExpiration` configuration option listed above is discussed in +detail in section [Caching Results of Principal Resolution](principal/cache.html). +It is not related to user management s.str. but affects the implementation +specific `PrincipalProvider` implementation exposed by `UserConfiguration.getUserPrincipalProvider`. ### Pluggability Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java (revision 1692521) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java (revision ) @@ -25,6 +25,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import com.google.common.collect.ImmutableList; import org.apache.felix.scr.annotations.Activate; import org.apache.felix.scr.annotations.Component; import org.apache.felix.scr.annotations.Properties; @@ -109,7 +110,13 @@ @Property(name = UserConstants.PARAM_PASSWORD_HISTORY_SIZE, label = "Maximum Password History Size", description = "Maximum number of passwords recorded for a user after changing her password (NOTE: upper limit is 1000). When changing the password the new password must not be present in the password history. A value of 0 indicates no password history is recorded.", - intValue = UserConstants.PASSWORD_HISTORY_DISABLED_SIZE) + intValue = UserConstants.PASSWORD_HISTORY_DISABLED_SIZE), + @Property(name = UserPrincipalProvider.PARAM_CACHE_EXPIRATION, + label = "Cache Expiration", + description = "Optional configuration defining the number of milliseconds " + + "until the cache expires (currently only affecting principal resolution during authentication). " + + "If not set or equal/lower than zero no caches are created/evaluated.", + longValue = UserPrincipalProvider.EXPIRATION_NO_CACHE) }) public class UserConfigurationImpl extends ConfigurationBase implements UserConfiguration, SecurityConfiguration { @@ -162,7 +169,7 @@ @Nonnull @Override public List getValidators(@Nonnull String workspaceName, @Nonnull Set principals, @Nonnull MoveTracker moveTracker) { - return Collections.singletonList(new UserValidatorProvider(getParameters())); + return ImmutableList.of(new UserValidatorProvider(getParameters()), new CacheValidatorProvider(principals)); } @Nonnull Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheConstants.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheConstants.java (revision ) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheConstants.java (revision ) @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.jackrabbit.oak.security.user; + +/** + * Constants for persisted user management related caches. Currently this only + * includes a basic cache for group principals names that is used to populate + * the set of {@link java.security.Principal}s as present on the + * {@link javax.security.auth.Subject} in the commit phase of the authentication. + */ +interface CacheConstants { + + String NT_REP_CACHE = "rep:Cache"; + String REP_CACHE = "rep:cache"; + String REP_EXPIRATION = "rep:expiration"; + String REP_GROUP_PRINCIPAL_NAMES = "rep:groupPrincipalNames"; + +} \ No newline at end of file Index: oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java (revision 1692521) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java (revision ) @@ -16,9 +16,15 @@ */ package org.apache.jackrabbit.oak.security.user; +import java.security.Principal; +import java.util.Collections; import java.util.HashMap; +import java.util.List; +import com.google.common.collect.Lists; import org.apache.jackrabbit.oak.AbstractSecurityTest; +import org.apache.jackrabbit.oak.spi.commit.MoveTracker; +import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider; 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; @@ -28,6 +34,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class UserConfigurationImplTest extends AbstractSecurityTest { @@ -46,6 +53,23 @@ @Override protected ConfigurationParameters getSecurityConfigParameters() { return ConfigurationParameters.of(UserConfiguration.NAME, getParams()); + } + + @Test + public void testValidators() { + UserConfigurationImpl configuration = new UserConfigurationImpl(getSecurityProvider()); + List validators = configuration.getValidators(adminSession.getWorkspaceName(), Collections.emptySet(), new MoveTracker()); + assertEquals(2, validators.size()); + + List clNames = Lists.newArrayList( + UserValidatorProvider.class.getName(), + CacheValidatorProvider.class.getName()); + + for (ValidatorProvider vp : validators) { + clNames.remove(vp.getClass().getName()); + } + + assertTrue(clNames.isEmpty()); } @Test Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java (revision 1692521) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java (revision ) @@ -327,32 +327,37 @@ @Override public void propertiesCompleted(@Nonnull Tree protectedParent) throws RepositoryException { + if (isCacheNode(protectedParent)) { + // remove the cache if present + protectedParent.remove(); + } else { - Authorizable a = userManager.getAuthorizable(protectedParent); - if (a == null) { - // not an authorizable - return; - } + Authorizable a = userManager.getAuthorizable(protectedParent); + if (a == null) { + // not an authorizable + return; + } - // make sure the authorizable ID property is always set even if the - // authorizable defined by the imported XML didn't provide rep:authorizableID - if (!protectedParent.hasProperty(REP_AUTHORIZABLE_ID)) { - protectedParent.setProperty(REP_AUTHORIZABLE_ID, a.getID(), Type.STRING); - } + // make sure the authorizable ID property is always set even if the + // authorizable defined by the imported XML didn't provide rep:authorizableID + if (!protectedParent.hasProperty(REP_AUTHORIZABLE_ID)) { + protectedParent.setProperty(REP_AUTHORIZABLE_ID, a.getID(), Type.STRING); + } - /* - Execute authorizable actions for a NEW user at this point after - having set the password and the principal name (all protected properties - have been processed now). - */ - if (protectedParent.getStatus() == Tree.Status.NEW) { - if (a.isGroup()) { - userManager.onCreate((Group) a); - } else { - userManager.onCreate((User) a, currentPw); - } - } - currentPw = null; - } + /* + Execute authorizable actions for a NEW user at this point after + having set the password and the principal name (all protected properties + have been processed now). + */ + if (protectedParent.getStatus() == Tree.Status.NEW) { + if (a.isGroup()) { + userManager.onCreate((Group) a); + } else { + userManager.onCreate((User) a, currentPw); + } + } + currentPw = null; + } + } @Override public void processReferences() throws RepositoryException { @@ -516,6 +521,10 @@ } parent.setProperty(property); return true; + } + + private static boolean isCacheNode(@Nonnull Tree tree) { + return tree.exists() && CacheConstants.REP_CACHE.equals(tree.getName()) && CacheConstants.NT_REP_CACHE.equals(TreeUtil.getPrimaryTypeName(tree)); } /** \ No newline at end of file Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipal.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipal.java (revision 1692521) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AbstractGroupPrincipal.java (revision ) @@ -44,6 +44,10 @@ super(principalName, groupTree, namePathMapper); } + AbstractGroupPrincipal(@Nonnull String principalName, @Nonnull String groupPath, @Nonnull NamePathMapper namePathMapper) { + super(principalName, groupPath, namePathMapper); + } + abstract UserManager getUserManager(); abstract boolean isEveryone() throws RepositoryException; Index: oak-doc/src/site/markdown/security/principal.md IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-doc/src/site/markdown/security/principal.md (revision 1692521) +++ oak-doc/src/site/markdown/security/principal.md (revision ) @@ -41,6 +41,9 @@ - [CompositePrincipalProvider]: Implementation that combines different principals from different source providers. +See section [Implementations of the PrincipalProvider Interface](principal/principalprovider.html) +for details. + ##### Special Principals - [AdminPrincipal]: Marker interface to identify the principal associated with administrative user(s). - [EveryonePrincipal]: built-in group principal implementation that has every other valid principal as member. Index: oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/builtin_nodetypes.cnd IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/builtin_nodetypes.cnd (revision 1692521) +++ oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/builtin_nodetypes.cnd (revision ) @@ -287,6 +287,17 @@ - * (UNDEFINED) IGNORE + * (nt:base) = rep:Unstructured IGNORE +/** + * Unstructured base node type for protected repository internal information + * that is protected and must not be copied to the version store OPV + * + * @since oak 1.4 + */ +[rep:UnstructuredProtected] abstract +- * (UNDEFINED) protected multiple IGNORE +- * (UNDEFINED) protected IGNORE ++ * (rep:UnstructuredProtected) protected IGNORE + //------------------------------------------------------------------------------ // R E F E R E N C E A B L E //------------------------------------------------------------------------------ @@ -753,6 +764,12 @@ */ [rep:MemberReferencesList] + * (rep:MemberReferences) = rep:MemberReferences protected COPY + +/** + * @since oak 1.4 + */ +[rep:Cache] > rep:UnstructuredProtected + - rep:expiration (LONG) protected IGNORE // ----------------------------------------------------------------------------- // Privilege Management Index: oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/CacheValidatorProviderTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/CacheValidatorProviderTest.java (revision ) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/CacheValidatorProviderTest.java (revision ) @@ -0,0 +1,280 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.jackrabbit.oak.security.user; + +import java.security.PrivilegedExceptionAction; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import javax.annotation.Nonnull; +import javax.jcr.NoSuchWorkspaceException; +import javax.jcr.RepositoryException; +import javax.security.auth.Subject; +import javax.security.auth.login.LoginException; + +import org.apache.jackrabbit.JcrConstants; +import org.apache.jackrabbit.api.security.user.Authorizable; +import org.apache.jackrabbit.api.security.user.Group; +import org.apache.jackrabbit.oak.AbstractSecurityTest; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.ContentSession; +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Root; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; +import org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants; +import org.apache.jackrabbit.oak.spi.security.authentication.SystemSubject; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; +import org.apache.jackrabbit.oak.util.NodeUtil; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class CacheValidatorProviderTest extends AbstractSecurityTest { + + private Group testGroup; + private Authorizable[] authorizables; + + @Override + public void before() throws Exception { + super.before(); + + testGroup = getUserManager(root).createGroup("testGroup_" + UUID.randomUUID()); + root.commit(); + + authorizables = new Authorizable[] {getTestUser(), testGroup}; + } + + @Override + public void after() throws Exception { + try { + if (testGroup != null) { + testGroup.remove(); + root.commit(); + } + } finally { + super.after(); + } + } + + private Tree getAuthorizableTree(@Nonnull Authorizable authorizable) throws RepositoryException { + return root.getTree(authorizable.getPath()); + } + + private Tree getCache(@Nonnull Authorizable authorizable) throws Exception { + ContentSession cs = Subject.doAs(SystemSubject.INSTANCE, new PrivilegedExceptionAction() { + @Override + public ContentSession run() throws LoginException, NoSuchWorkspaceException { + return login(null); + + } + }); + try { + Root r = cs.getLatestRoot(); + NodeUtil n = new NodeUtil(r.getTree(authorizable.getPath())); + NodeUtil c = n.getOrAddChild(CacheConstants.REP_CACHE, CacheConstants.NT_REP_CACHE); + c.setLong(CacheConstants.REP_EXPIRATION, 1); + r.commit(CacheValidatorProvider.asCommitAttributes()); + } finally { + cs.close(); + } + + root.refresh(); + return root.getTree(authorizable.getPath()).getChild(CacheConstants.REP_CACHE); + } + + @Test + public void testCreateCacheByName() throws RepositoryException { + for (Authorizable a : authorizables) { + try { + NodeUtil node = new NodeUtil(getAuthorizableTree(a)); + node.addChild(CacheConstants.REP_CACHE, JcrConstants.NT_UNSTRUCTURED); + root.commit(); + fail("Creating rep:cache node below a user or group must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + } + } + } + + @Test + public void testCreateCacheByNodeType() throws RepositoryException { + for (Authorizable a : authorizables) { + try { + NodeUtil node = new NodeUtil(getAuthorizableTree(a)); + NodeUtil cache = node.addChild("childNode", CacheConstants.NT_REP_CACHE); + cache.setLong(CacheConstants.REP_EXPIRATION, 1); + root.commit(); + fail("Creating node with nt rep:Cache below a user or group must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + } + } + } + + @Test + public void testChangePrimaryType() throws RepositoryException { + for (Authorizable a : authorizables) { + try { + NodeUtil node = new NodeUtil(getAuthorizableTree(a)); + NodeUtil cache = node.addChild("childNode", JcrConstants.NT_UNSTRUCTURED); + root.commit(); + + cache.setName(JcrConstants.JCR_PRIMARYTYPE, CacheConstants.NT_REP_CACHE); + cache.setLong(CacheConstants.REP_EXPIRATION, 1); + root.commit(); + fail("Changing primary type of residual node below an user/group to rep:Cache must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + } + } + } + + @Test + public void testCreateCacheWithCommitInfo() throws RepositoryException { + for (Authorizable a : authorizables) { + try { + NodeUtil node = new NodeUtil(getAuthorizableTree(a)); + NodeUtil cache = node.addChild(CacheConstants.REP_CACHE, CacheConstants.NT_REP_CACHE); + cache.setLong(CacheConstants.REP_EXPIRATION, 1); + root.commit(CacheValidatorProvider.asCommitAttributes()); + fail("Creating rep:cache node below a user or group must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + } + } + } + + @Test + public void testCreateCacheBelowProfile() throws Exception { + try { + NodeUtil node = new NodeUtil(getAuthorizableTree(getTestUser())); + NodeUtil child = node.addChild("profile", NodeTypeConstants.NT_OAK_UNSTRUCTURED); + child.addChild(CacheConstants.REP_CACHE, CacheConstants.NT_REP_CACHE).setLong(CacheConstants.REP_EXPIRATION, 23); + root.commit(CacheValidatorProvider.asCommitAttributes()); + fail("Creating rep:cache node below a user or group must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + } + } + + @Test + public void testCreateCacheBelowPersistedProfile() throws Exception { + try { + NodeUtil node = new NodeUtil(getAuthorizableTree(getTestUser())); + NodeUtil child = node.addChild("profile", NodeTypeConstants.NT_OAK_UNSTRUCTURED); + root.commit(); + + child.addChild(CacheConstants.REP_CACHE, CacheConstants.NT_REP_CACHE).setLong(CacheConstants.REP_EXPIRATION, 23); + root.commit(CacheValidatorProvider.asCommitAttributes()); + fail("Creating rep:cache node below a user or group must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + } + } + + @Test + public void testModifyCache() throws Exception { + List props = new ArrayList(); + props.add(PropertyStates.createProperty(CacheConstants.REP_EXPIRATION, 25)); + props.add(PropertyStates.createProperty(CacheConstants.REP_GROUP_PRINCIPAL_NAMES, EveryonePrincipal.NAME)); + props.add(PropertyStates.createProperty(JcrConstants.JCR_PRIMARYTYPE, JcrConstants.NT_UNSTRUCTURED)); + props.add(PropertyStates.createProperty("residualProp", "anyvalue")); + + Tree cache = getCache(getTestUser()); + for (PropertyState prop : props) { + try { + cache.setProperty(prop); + root.commit(CacheValidatorProvider.asCommitAttributes()); + + fail("Modifying rep:cache node below a user or group must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + } + } + } + + @Test + public void testNestedCache() throws Exception { + NodeUtil cache = new NodeUtil(getCache(getTestUser())); + try { + NodeUtil c = cache.getOrAddChild(CacheConstants.REP_CACHE, CacheConstants.NT_REP_CACHE); + c.setLong(CacheConstants.REP_EXPIRATION, 223); + root.commit(CacheValidatorProvider.asCommitAttributes()); + + fail("Creating nested cache must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + } + } + + @Test + public void testRemoveCache() throws Exception { + Tree cache = getCache(getTestUser()); + cache.remove(); + root.commit(); + } + + @Test + public void testCreateCacheOutsideOfAuthorizable() throws Exception { + NodeUtil n = new NodeUtil(root.getTree("/")); + try { + NodeUtil child = n.addChild(CacheConstants.REP_CACHE, CacheConstants.NT_REP_CACHE); + child.setLong(CacheConstants.REP_EXPIRATION, 1); + root.commit(); + fail("Using rep:cache/rep:Cache outside a user or group must fail."); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + assertEquals(34, e.getCode()); + } finally { + root.refresh(); + Tree c = n.getTree().getChild(CacheConstants.REP_CACHE); + if (c.exists()) { + c.remove(); + root.commit(); + } + } + + } +} \ No newline at end of file Index: oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java (revision 1692521) +++ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java (revision ) @@ -469,7 +469,9 @@ // process properties importProperties(tree, propInfos, false); + if (tree.exists()) { - parents.push(tree); + parents.push(tree); + } } Index: oak-doc/src/site/markdown/security/principal/cache.md IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-doc/src/site/markdown/security/principal/cache.md (revision ) +++ oak-doc/src/site/markdown/security/principal/cache.md (revision ) @@ -0,0 +1,148 @@ + + +Caching Results of Principal Resolution +-------------------------------------------------------------------------------- + +### General + +Since Oak 1.3.4 this `UserPrincipalProvider` optionally allows for temporary +caching of the principal resolution mainly to optimize login performance (OAK-3003). + +This cache contains the result of the group principal resolution as performed by +`PrincipalProvider.getPrincipals(String userId)`and `PrincipalProvider.getGroupMembership(Principal)` +and will read from the cache upon subsequent calls for the configured expiration +time. + +### Configuration + +An administrator may enable the group principal caching via the +_org.apache.jackrabbit.oak.security.user.UserConfigurationImpl_ +OSGi configuration. By default caching is disabled. + +The following configuration option is supported: + +- Cache Expiration (`cacheExpiration`): Specifying a long greater 0 enables the + caching. + +NOTE: It is important that the configured expiration time balances between login +performance and cache invalidation to reflect changes made to the group membership. +An application that makes use of this cache, must be able to live with shot term +diverging of principal resolution and user management upon repository login. + +It is expected that the cache is used in scenarios where subsequent repository +login calls can (or even should) result in the creation of a `javax.security.auth.Subject` +with equal principal set irrespective of group membership changes. +See section Invalidation below for further details. + + +### How it works + +#### Caching Principal Names + +If the feature is enabled, evaluating `UserPrincipalProvider.getPrincipals(String userId)` +and `PrincipalProvider.getGroupMembership(Principal)` as well as the corresponding +calls on `PrincipalManager` will trigger the group principal names to be remembered +in a cache if the following conditions are met: + +- a valid expiration time is configured (i.e. > 0), +- the `PrincipalProvider` has been obtained for a system session (see below), +- the tree to hold the cache belongs to a user (i.e. tree with primary type + `rep:User` (i.e. no caches are created for groups) + +The cache itself consists of a tree named `rep:cache` with the built-in node type +`rep:Cache`, which defines a mandatory, protected `rep:expiration` property and +may have additional protected, residual properties. + +Subsequent calls will read the names of the group principals from the cache until +the cache expires. Once expired the default resolution will be performed again in +order to update the cache. + +##### Limitation to System Calls + +The creation and maintenance of this caches as well as the shortcut upon reading +is limited to system internal sessions for security reasons: The cache must always +be filled with the comprehensive list of group principals (as required upon login) +as must any subsequent call never expose principal information that might not +be accessible in the non-cache scenario where access to principals is protected +by regular permission evalution. + + +##### Validation + +The cache is system maintained, protected repository content that can only +be created and updated by the implementation. Any attempt to manipulate these +caches using JCR or Oak API calls will fail. Also the cache can only be created +or updated using the internal system subject. + +Also this validation is always enforce irrespective on whether the caching +feature is enabled or not, to prevent unintended manipulation. + +These constraints and the consistency of the cache structure is asserted by a +dedicated `CacheValidator`. The corresponding errors are all of type `Constraint` +with the following codes: + +| Code | Message | +|-------------------|----------------------------------------------------------| +| 0034 | Attempt to create or change the system maintained cache. | + +Note however, that the cache tree might be removed by any session that has +sufficient privileges to remove it. + + +##### Cache Invalidation + +The caches hold with the different user trees get invalidated once the expiration +time is reached. There is no explicit, forced invalidation if group membership +as reflected by the user management implementation is being changed. + +Consequently, system sessions which might read principal information from the cache +(if enabled) can be provided with a set of principals (as stored in the cache) +that might have diverged from the group membership stored in the repository +for the time until the cache expires. + +Applications that rely on principal resolution being _always_ in sync with the +revision associated with the system session that perform the repository login, +must not enable the cache. + +Similarly, applications that have due to their design have an extremely high +turnover wrt group membership might not be able to profit from this cache in +the expected way. + + +#### Interaction With User Management + +The cache is created and maintained by the `PrincipalProvider` implementation as +exposed by the optional `UserConfiguration.getUserPrincipalProvider` call and +will therefore only effect the results provided by the principal management API. + +Regular Jackrabbit user management API calls are not affected by this cache and +vice versa; i.e. changes made using the user management API have no immediate +effect on the cache and will not trigger it's invalidation. + +In other words user management API calls will always read from the revision of the +content repository that is associated with the give JCR `Session` (and Oak +`ContentSession`). The same is true for principal management API calls of all +non-system sessions. + +See the introduction and section Invalidation above for the expected behavior +for system sessions. + +##### XML Import + +When users are imported via JCR XML import, the protected cache structure will +be ignored (i.e. will not be imported). Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypePredicate.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypePredicate.java (revision 1692521) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypePredicate.java (revision ) @@ -25,8 +25,10 @@ import com.google.common.base.Predicate; import com.google.common.collect.Iterables; +import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry; import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.apache.jackrabbit.oak.util.TreeUtil; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Predicates.in; @@ -162,6 +164,21 @@ } if (mixinTypes != null && any(mixins, in(mixinTypes))) { return true; + } + return false; + } + + public boolean apply(@Nullable Tree input) { + if (input != null) { + init(); + if (primaryTypes != null + && primaryTypes.contains(TreeUtil.getPrimaryTypeName(input))) { + return true; + } + if (mixinTypes != null + && any(TreeUtil.getNames(input, JCR_MIXINTYPES), in(mixinTypes))) { + return true; + } } return false; } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/package-info.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/package-info.java (revision 1692521) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/package-info.java (revision ) @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("1.0") +@Version("1.1.0") @Export(optional = "provide:=true") package org.apache.jackrabbit.oak.plugins.nodetype; \ No newline at end of file Index: oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderWithCacheTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderWithCacheTest.java (revision ) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderWithCacheTest.java (revision ) @@ -0,0 +1,569 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.jackrabbit.oak.security.user; + +import java.security.Principal; +import java.security.PrivilegedExceptionAction; +import java.util.ArrayList; +import java.util.Enumeration; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import javax.annotation.Nullable; +import javax.jcr.NoSuchWorkspaceException; +import javax.jcr.SimpleCredentials; +import javax.security.auth.Subject; +import javax.security.auth.login.LoginException; + +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import org.apache.jackrabbit.JcrConstants; +import org.apache.jackrabbit.api.security.principal.PrincipalIterator; +import org.apache.jackrabbit.api.security.principal.PrincipalManager; +import org.apache.jackrabbit.api.security.user.Group; +import org.apache.jackrabbit.api.security.user.UserManager; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.ContentSession; +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Root; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; +import org.apache.jackrabbit.oak.spi.security.ConfigurationBase; +import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; +import org.apache.jackrabbit.oak.spi.security.authentication.SystemSubject; +import org.apache.jackrabbit.oak.spi.security.principal.AbstractPrincipalProviderTest; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; +import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl; +import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider; +import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration; +import org.apache.jackrabbit.oak.util.NodeUtil; +import org.apache.jackrabbit.oak.util.TreeUtil; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Testing the optional caching with the {@link org.apache.jackrabbit.oak.security.user.UserPrincipalProvider}. + */ +public class UserPrincipalProviderWithCacheTest extends AbstractPrincipalProviderTest { + + private String userId; + private String groupId; + private Group testGroup; + + private String groupId2; + private Group testGroup2; + + private ContentSession systemSession; + private Root systemRoot; + + @Override + public void before() throws Exception { + super.before(); + + userId = getTestUser().getID(); + + groupId = "testGroup" + UUID.randomUUID(); + testGroup = getUserManager(root).createGroup(groupId); + testGroup.addMember(getTestUser()); + + groupId2 = "testGroup2" + UUID.randomUUID(); + testGroup2 = getUserManager(root).createGroup(groupId2); + testGroup.addMember(testGroup2); + + root.commit(); + + systemSession = getSystemSession(); + systemRoot = systemSession.getLatestRoot(); + } + + @Override + public void after() throws Exception { + try { + if (systemSession != null) { + systemSession.close(); + } + + root.refresh(); + Group gr = getUserManager(root).getAuthorizable(groupId, Group.class); + if (gr != null) { + gr.remove(); + root.commit(); + } + } finally { + super.after(); + } + } + + @Override + protected ConfigurationParameters getSecurityConfigParameters() { + return ConfigurationParameters.of( + UserConfiguration.NAME, + ConfigurationParameters.of(UserPrincipalProvider.PARAM_CACHE_EXPIRATION, 3600 * 1000) + ); + } + + @Override + protected PrincipalProvider createPrincipalProvider() { + return createPrincipalProvider(root); + } + + private PrincipalProvider createPrincipalProvider(Root root) { + return new UserPrincipalProvider(root, getUserConfiguration(), namePathMapper); + } + + private ContentSession getSystemSession() throws Exception { + if (systemSession == null) { + systemSession = Subject.doAs(SystemSubject.INSTANCE, new PrivilegedExceptionAction() { + @Override + public ContentSession run() throws LoginException, NoSuchWorkspaceException { + return login(null); + + } + }); + } + return systemSession; + } + + private UserConfiguration changeUserConfiguration(ConfigurationParameters params) { + UserConfiguration userConfig = getUserConfiguration(); + ((ConfigurationBase) userConfig).setParameters(params); + return userConfig; + } + + private Tree getCacheTree(Root root) throws Exception { + return getCacheTree(root, getTestUser().getPath()); + } + + private Tree getCacheTree(Root root, String authorizablePath) throws Exception { + return root.getTree(authorizablePath + '/' + CacheConstants.REP_CACHE); + } + + private static void assertPrincipals(Set principals, Principal... expectedPrincipals) { + assertEquals(expectedPrincipals.length, principals.size()); + for (Principal principal : expectedPrincipals) { + assertTrue(principals.contains(principal)); + } + } + + @Test + public void testGetPrincipalsPopulatesCache() throws Exception { + PrincipalProvider pp = createPrincipalProvider(systemRoot); + + Set principals = pp.getPrincipals(userId); + assertPrincipals(principals, EveryonePrincipal.getInstance(), testGroup.getPrincipal(), getTestUser().getPrincipal()); + + root.refresh(); + + Tree principalCache = getCacheTree(root); + assertTrue(principalCache.exists()); + assertEquals(CacheConstants.NT_REP_CACHE, TreeUtil.getPrimaryTypeName(principalCache)); + + assertNotNull(principalCache.getProperty(CacheConstants.REP_EXPIRATION)); + + PropertyState ps = principalCache.getProperty(CacheConstants.REP_GROUP_PRINCIPAL_NAMES); + assertNotNull(ps); + + String val = ps.getValue(Type.STRING); + assertEquals(testGroup.getPrincipal().getName(), val); + } + + @Test + public void testGetGroupMembershipPopulatesCache() throws Exception { + PrincipalProvider pp = createPrincipalProvider(systemRoot); + + Set principals = pp.getGroupMembership(getTestUser().getPrincipal()); + assertPrincipals(principals, EveryonePrincipal.getInstance(), testGroup.getPrincipal()); + + root.refresh(); + + Tree principalCache = getCacheTree(root); + assertTrue(principalCache.exists()); + assertEquals(CacheConstants.NT_REP_CACHE, TreeUtil.getPrimaryTypeName(principalCache)); + + assertNotNull(principalCache.getProperty(CacheConstants.REP_EXPIRATION)); + + PropertyState ps = principalCache.getProperty(CacheConstants.REP_GROUP_PRINCIPAL_NAMES); + assertNotNull(ps); + + String val = ps.getValue(Type.STRING); + assertEquals(testGroup.getPrincipal().getName(), val); + } + + @Test + public void testPrincipalManagerGetGroupMembershipPopulatesCache() throws Exception { + PrincipalManager principalManager = getPrincipalManager(systemRoot); + + PrincipalIterator principalIterator = principalManager.getGroupMembership(getTestUser().getPrincipal()); + assertPrincipals(ImmutableSet.copyOf(principalIterator), EveryonePrincipal.getInstance(), testGroup.getPrincipal()); + + root.refresh(); + + Tree principalCache = getCacheTree(root); + assertTrue(principalCache.exists()); + assertEquals(CacheConstants.NT_REP_CACHE, TreeUtil.getPrimaryTypeName(principalCache)); + + assertNotNull(principalCache.getProperty(CacheConstants.REP_EXPIRATION)); + + PropertyState ps = principalCache.getProperty(CacheConstants.REP_GROUP_PRINCIPAL_NAMES); + assertNotNull(ps); + + String val = ps.getValue(Type.STRING); + assertEquals(testGroup.getPrincipal().getName(), val); + } + + @Test + public void testGetPrincipalsForGroups() throws Exception { + PrincipalProvider pp = createPrincipalProvider(systemRoot); + + Set principals = pp.getPrincipals(testGroup.getID()); + assertTrue(principals.isEmpty()); + + principals = pp.getPrincipals(testGroup2.getID()); + assertTrue(principals.isEmpty()); + + root.refresh(); + + Tree principalCache = getCacheTree(root, testGroup.getPath()); + assertFalse(principalCache.exists()); + + principalCache = getCacheTree(root, testGroup2.getPath()); + assertFalse(principalCache.exists()); + } + + @Test + public void testGetGroupMembershipForGroups() throws Exception { + PrincipalProvider pp = createPrincipalProvider(systemRoot); + + Set principals = pp.getGroupMembership(testGroup.getPrincipal()); + assertPrincipals(principals, EveryonePrincipal.getInstance()); + + principals = pp.getGroupMembership(testGroup2.getPrincipal()); + assertPrincipals(principals, EveryonePrincipal.getInstance(), testGroup.getPrincipal()); + + root.refresh(); + + Tree principalCache = getCacheTree(root, testGroup.getPath()); + assertFalse(principalCache.exists()); + + principalCache = getCacheTree(root, testGroup2.getPath()); + assertFalse(principalCache.exists()); + } + + @Test + public void testExtractPrincipalsFromCache() throws Exception { + // a) force the cache to be created + PrincipalProvider pp = createPrincipalProvider(systemRoot); + + // set of principals that read from user + membership-provider. + Set principals = pp.getPrincipals(userId); + assertPrincipals(principals, EveryonePrincipal.getInstance(), testGroup.getPrincipal(), getTestUser().getPrincipal()); + + // b) retrieve principals again (this time from the cache) + Set principalsAgain = pp.getPrincipals(userId); + + // make sure both sets are equal + assertEquals(principals, principalsAgain); + } + + @Test + public void testGroupPrincipals() throws Exception { + // a) force the cache to be created + PrincipalProvider pp = createPrincipalProvider(systemRoot); + Iterable principals = Iterables.filter(pp.getPrincipals(userId), new GroupPredicate()); + + for (Principal p : principals) { + String className = p.getClass().getName(); + assertEquals("org.apache.jackrabbit.oak.security.user.UserPrincipalProvider$GroupPrincipal", className); + } + + Principal testPrincipal = getTestUser().getPrincipal(); + + // b) retrieve principals again (this time from the cache) + // -> verify that they are a different implementation + Iterable principalsAgain = Iterables.filter(pp.getPrincipals(userId), new GroupPredicate()); + for (Principal p : principalsAgain) { + String className = p.getClass().getName(); + assertEquals("org.apache.jackrabbit.oak.security.user.UserPrincipalProvider$CachedGroupPrincipal", className); + + assertTrue(p instanceof TreeBasedPrincipal); + assertEquals(testGroup.getPath(), ((TreeBasedPrincipal) p).getPath()); + + java.security.acl.Group principalGroup = (java.security.acl.Group) p; + assertTrue(principalGroup.isMember(testPrincipal)); + + Enumeration members = principalGroup.members(); + assertTrue(members.hasMoreElements()); + assertEquals(testPrincipal, members.nextElement()); + assertEquals(testGroup2.getPrincipal(), members.nextElement()); + assertFalse(members.hasMoreElements()); + } + } + + @Test + public void testGroupPrincipalNameEscape() throws Exception { + String gId = null; + try { + Principal groupPrincipal = new PrincipalImpl(groupId + ",,%,%%"); + Group gr = getUserManager(root).createGroup(groupPrincipal); + gId = gr.getID(); + gr.addMember(getTestUser()); + root.commit(); + systemRoot.refresh(); + + PrincipalProvider pp = createPrincipalProvider(systemRoot); + Set principals = pp.getPrincipals(userId); + assertTrue(principals.contains(groupPrincipal)); + + principals = pp.getPrincipals(userId); + assertTrue(principals.contains(groupPrincipal)); + } finally { + root.refresh(); + if (gId != null) { + getUserManager(root).getAuthorizable(gId).remove(); + root.commit(); + } + } + } + + @Test + public void testMembershipChange() throws Exception { + PrincipalProvider pp = createPrincipalProvider(systemRoot); + + // set of principals that read from user + membership-provider. + Set principals = pp.getPrincipals(userId); + + // change group membership with a different root + UserManager uMgr = getUserManager(root); + Group gr = uMgr.getAuthorizable(groupId, Group.class); + assertTrue(gr.removeMember(uMgr.getAuthorizable(userId))); + root.commit(); + systemRoot.refresh(); + + Set principalsAgain = pp.getPrincipals(userId); + assertEquals(principals, principalsAgain); + + // disable the cache again + changeUserConfiguration(ConfigurationParameters.EMPTY); + pp = createPrincipalProvider(systemRoot); + + // now group principals must no longer be retrieved from the cache + assertPrincipals(pp.getPrincipals(userId), EveryonePrincipal.getInstance(), getTestUser().getPrincipal()); + } + + @Test + public void testCacheUpdate() throws Exception { + PrincipalProvider pp = createPrincipalProvider(systemRoot); + + // set of principals that read from user + membership-provider -> cache being filled + Set principals = pp.getPrincipals(userId); + assertTrue(getCacheTree(systemRoot).exists()); + + // change the group membership of the test user + UserManager uMgr = getUserConfiguration().getUserManager(systemRoot, namePathMapper); + Group gr = uMgr.getAuthorizable(groupId, Group.class); + assertTrue(gr.removeMember(uMgr.getAuthorizable(userId))); + systemRoot.commit(); + + // force cache expiration by manually setting the expiration time + Tree cache = getCacheTree(systemRoot); + cache.setProperty(CacheConstants.REP_EXPIRATION, 2); + systemRoot.commit(CacheValidatorProvider.asCommitAttributes()); + + // retrieve principals again to have cache updated + pp = createPrincipalProvider(systemRoot); + Set principalsAgain = pp.getPrincipals(userId); + assertFalse(principals.equals(principalsAgain)); + assertPrincipals(principalsAgain, EveryonePrincipal.getInstance(), getTestUser().getPrincipal()); + + // verify that the cache has really been updated + cache = getCacheTree(systemRoot); + assertNotSame(2, new NodeUtil(cache).getLong(CacheConstants.REP_EXPIRATION, 2)); + assertEquals("", TreeUtil.getString(cache, CacheConstants.REP_GROUP_PRINCIPAL_NAMES)); + } + + @Test + public void testOnlySystemCreatesCache() throws Exception { + Set principals = principalProvider.getPrincipals(getTestUser().getID()); + assertPrincipals(principals, EveryonePrincipal.getInstance(), testGroup.getPrincipal(), getTestUser().getPrincipal()); + + root.refresh(); + Tree userTree = root.getTree(getTestUser().getPath()); + + assertFalse(userTree.hasChild(CacheConstants.REP_CACHE)); + } + + @Test + public void testOnlySystemReadsFromCache() throws Exception { + String userId = getTestUser().getID(); + + PrincipalProvider systemPP = createPrincipalProvider(systemRoot); + Set principals = systemPP.getPrincipals(userId); + assertPrincipals(principals, EveryonePrincipal.getInstance(), testGroup.getPrincipal(), getTestUser().getPrincipal()); + + root.refresh(); + assertPrincipals(principalProvider.getPrincipals(userId), EveryonePrincipal.getInstance(), testGroup.getPrincipal(), getTestUser().getPrincipal()); + + testGroup.removeMember(getTestUser()); + root.commit(); + + assertPrincipals(principalProvider.getPrincipals(userId), EveryonePrincipal.getInstance(), getTestUser().getPrincipal()); + assertPrincipals(systemPP.getPrincipals(userId), EveryonePrincipal.getInstance(), testGroup.getPrincipal(), getTestUser().getPrincipal()); + } + + @Test + public void testInvalidExpiry() throws Exception { + long[] noCache = new long[] {0, -1, Long.MIN_VALUE}; + for (long exp : noCache) { + + changeUserConfiguration(ConfigurationParameters.of(UserPrincipalProvider.PARAM_CACHE_EXPIRATION, exp)); + + PrincipalProvider pp = createPrincipalProvider(systemRoot); + pp.getPrincipals(userId); + + root.refresh(); + Tree userTree = root.getTree(getTestUser().getPath()); + assertFalse(userTree.hasChild(CacheConstants.REP_CACHE)); + } + } + + @Test + public void testLongOverflow() throws Exception { + long[] maxCache = new long[] {Long.MAX_VALUE, Long.MAX_VALUE-1, Long.MAX_VALUE-10000}; + + Root systemRoot = getSystemSession().getLatestRoot(); + for (long exp : maxCache) { + changeUserConfiguration(ConfigurationParameters.of(UserPrincipalProvider.PARAM_CACHE_EXPIRATION, exp)); + + PrincipalProvider pp = createPrincipalProvider(systemRoot); + pp.getPrincipals(userId); + + Tree userTree = systemRoot.getTree(getTestUser().getPath()); + + Tree cache = userTree.getChild(CacheConstants.REP_CACHE); + assertTrue(cache.exists()); + + PropertyState propertyState = cache.getProperty(CacheConstants.REP_EXPIRATION); + assertNotNull(propertyState); + assertEquals(Long.MAX_VALUE, propertyState.getValue(Type.LONG).longValue()); + + cache.remove(); + systemRoot.commit(); + } + } + + @Test + public void testChangeCache() throws Exception { + PrincipalProvider pp = createPrincipalProvider(systemRoot); + pp.getPrincipals(userId); + + root.refresh(); + + List props = new ArrayList(); + props.add(PropertyStates.createProperty(CacheConstants.REP_EXPIRATION, 25)); + props.add(PropertyStates.createProperty(CacheConstants.REP_GROUP_PRINCIPAL_NAMES, EveryonePrincipal.NAME)); + props.add(PropertyStates.createProperty(JcrConstants.JCR_PRIMARYTYPE, JcrConstants.NT_UNSTRUCTURED)); + props.add(PropertyStates.createProperty("residualProp", "anyvalue")); + + // changing cache with (normally) sufficiently privileged session must not succeed + for (PropertyState ps : props) { + try { + Tree cache = getCacheTree(root); + cache.setProperty(ps); + root.commit(); + fail("Attempt to modify the cache tree must fail."); + } catch (CommitFailedException e) { + // success + } finally { + root.refresh(); + } + } + + // changing cache with system session must not succeed either + for (PropertyState ps : props) { + try { + Tree cache = getCacheTree(systemRoot); + cache.setProperty(ps); + systemRoot.commit(); + fail("Attempt to modify the cache tree must fail."); + } catch (CommitFailedException e) { + // success + } finally { + systemRoot.refresh(); + } + } + } + + @Test + public void testRemoveCache() throws Exception { + PrincipalProvider pp = createPrincipalProvider(systemRoot); + pp.getPrincipals(userId); + + // removing cache with sufficiently privileged session must succeed + root.refresh(); + Tree cache = getCacheTree(root); + cache.remove(); + root.commit(); + } + + @Test + public void testConcurrentLoginWithCacheRemoval() throws Exception { + changeUserConfiguration(ConfigurationParameters.of(UserPrincipalProvider.PARAM_CACHE_EXPIRATION, 1)); + + final List exceptions = new ArrayList(); + List threads = new ArrayList(); + for (int i = 0; i < 100; i++) { + threads.add(new Thread(new Runnable() { + public void run() { + try { + login(new SimpleCredentials(userId, userId.toCharArray())).close(); + } catch (Exception e) { + exceptions.add(e); + } + } + })); + } + for (Thread t : threads) { + t.start(); + } + for (Thread t : threads) { + t.join(); + } + for (Exception e : exceptions) { + e.printStackTrace(); + } + if (!exceptions.isEmpty()) { + fail(); + } + } + + //-------------------------------------------------------------------------- + + private static final class GroupPredicate implements Predicate { + @Override + public boolean apply(@Nullable Principal input) { + return (input instanceof java.security.acl.Group) && !EveryonePrincipal.getInstance().equals(input); + } + } +} \ No newline at end of file Index: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/user/UserImportCacheTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/user/UserImportCacheTest.java (revision ) +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/user/UserImportCacheTest.java (revision ) @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.jackrabbit.oak.jcr.security.user; + +import javax.jcr.Node; + +import org.apache.jackrabbit.api.security.user.Authorizable; +import org.apache.jackrabbit.oak.spi.security.user.UserConstants; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * Testing user import with default {@link org.apache.jackrabbit.oak.spi.xml.ImportBehavior} + * and pw-history content: test that the history is imported irrespective of the + * configuration. + */ +public class UserImportCacheTest extends AbstractImportTest { + + @Override + protected String getTargetPath() { + return USERPATH; + } + + @Override + protected String getImportBehavior() { + return null; + } + + /** + * @since Oak 1.3.4 + */ + @Test + public void testImportUserWithCache() throws Exception { + // import user + String xml = "\n" + + "" + + " " + + " rep:User" + + " " + + " " + + " 41529076-9594-360e-ae48-5922904f345d" + + " " + + " " + + " pw" + + " " + + " " + + " yPrincipal" + + " " + + " " + + " " + + " rep:Cache" + + " " + + " " + + " 123456789" + + " " + + " " + + " \"testGroup\"" + + " " + + " " + + ""; + + doImport(USERPATH, xml); + getImportSession().save(); + + Authorizable authorizable = getUserManager().getAuthorizable("y"); + Node userNode = getImportSession().getNode(authorizable.getPath()); + assertFalse(userNode.hasNode("rep:cache")); + } +} Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheValidatorProvider.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheValidatorProvider.java (revision ) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheValidatorProvider.java (revision ) @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.jackrabbit.oak.security.user; + +import java.security.Principal; +import java.util.Collections; +import java.util.Map; +import java.util.Set; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.plugins.nodetype.TypePredicate; +import org.apache.jackrabbit.oak.plugins.tree.TreeFactory; +import org.apache.jackrabbit.oak.spi.commit.CommitInfo; +import org.apache.jackrabbit.oak.spi.commit.DefaultValidator; +import org.apache.jackrabbit.oak.spi.commit.Validator; +import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider; +import org.apache.jackrabbit.oak.spi.commit.VisibleValidator; +import org.apache.jackrabbit.oak.spi.security.principal.SystemPrincipal; +import org.apache.jackrabbit.oak.spi.state.NodeState; + +import static com.google.common.base.Preconditions.checkNotNull; + +/** + * Validator provider to ensure that the principal-cache stored with a given + * user is only maintained by the {@link org.apache.jackrabbit.oak.security.user.UserPrincipalProvider} + * associated with a internal system session. + */ +class CacheValidatorProvider extends ValidatorProvider implements CacheConstants { + + private final boolean isSystem; + + CacheValidatorProvider(@Nonnull Set principals) { + super(); + isSystem = principals.contains(SystemPrincipal.INSTANCE); + } + + @CheckForNull + @Override + protected Validator getRootValidator(NodeState before, NodeState after, CommitInfo info) { + TypePredicate cachePredicate = new TypePredicate(after, NT_REP_CACHE); + boolean isValidCommitInfo = CommitMarker.isValidCommitInfo(info); + return new CacheValidator(TreeFactory.createReadOnlyTree(before), TreeFactory.createReadOnlyTree(after), cachePredicate, isValidCommitInfo); + } + + //-------------------------------------------------------------------------- + + static Map asCommitAttributes() { + return Collections.singletonMap(CommitMarker.KEY, CommitMarker.INSTANCE); + } + + private static final class CommitMarker { + + private static final String KEY = CommitMarker.class.getName(); + + private static final CommitMarker INSTANCE = new CommitMarker(); + + private static boolean isValidCommitInfo(@Nonnull CommitInfo commitInfo) { + return CommitMarker.INSTANCE == commitInfo.getInfo().get(CommitMarker.KEY); + } + + private CommitMarker() {} + } + + private static CommitFailedException constraintViolation(int code, @Nonnull String message) { + return new CommitFailedException(CommitFailedException.CONSTRAINT, code, message); + } + + //-----------------------------------------------------< CacheValidator >--- + private final class CacheValidator extends DefaultValidator { + + private final Tree parentBefore; + private final Tree parentAfter; + + private final TypePredicate cachePredicate; + private final boolean isValidCommitInfo; + + private final boolean isCache; + + private CacheValidator(@Nullable Tree parentBefore, @Nonnull Tree parentAfter, TypePredicate cachePredicate, boolean isValidCommitInfo) { + this.parentBefore = parentBefore; + this.parentAfter = parentAfter; + + this.cachePredicate = cachePredicate; + this.isValidCommitInfo = isValidCommitInfo; + + isCache = isCache(parentAfter); + } + + @Override + public void propertyAdded(PropertyState after) throws CommitFailedException { + if (isCache) { + checkValidCommit(); + } + } + + @Override + public void propertyChanged(PropertyState before, PropertyState after) throws CommitFailedException { + if (isCache) { + checkValidCommit(); + } + } + + @Override + public Validator childNodeChanged(String name, NodeState before, NodeState after) throws CommitFailedException { + Tree beforeTree = (parentBefore == null) ? null : parentBefore.getChild(name); + Tree afterTree = parentAfter.getChild(name); + + if (isCache || isCache(beforeTree) || isCache(afterTree)) { + checkValidCommit(); + } + + return new VisibleValidator(new CacheValidator(beforeTree, afterTree, cachePredicate, isValidCommitInfo), true, true); + } + + @Override + public Validator childNodeAdded(String name, NodeState after) throws CommitFailedException { + Tree tree = checkNotNull(parentAfter.getChild(name)); + if (isCache || isCache(tree)) { + checkValidCommit(); + } + return new VisibleValidator(new CacheValidator(null, tree, cachePredicate, isValidCommitInfo), true, true); + } + + private boolean isCache(@CheckForNull Tree tree) { + return tree != null && (REP_CACHE.equals(tree.getName()) || cachePredicate.apply(tree)); + } + + private void checkValidCommit() throws CommitFailedException { + if (!(isSystem && isValidCommitInfo)) { + throw constraintViolation(34, "Attempt to create or change the system maintained cache."); + } + } + } +} \ No newline at end of file Index: oak-doc/src/site/markdown/security/principal/principalprovider.md IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-doc/src/site/markdown/security/principal/principalprovider.md (revision ) +++ oak-doc/src/site/markdown/security/principal/principalprovider.md (revision ) @@ -0,0 +1,61 @@ + + +Implementations of the PrincipalProvider Interface +-------------------------------------------------------------------------------- + +Oak contains by default the following implementations of the `PrincipalProvider` +interface: + +### org.apache.jackrabbit.oak.security.principal.PrincipalProviderImpl + +This is the default implementation of the `PrincipalProvider`, which makes use +of the fact that `Authorizable`s as defined by the Jackrabbit user management +API are always paired with a `Principal`. + +The implementation is not tied to a particular user management implementation +and doesn't need to be rewritten if the security setup would be configured with +different implementation of `UserConfiguration`. + +### org.apache.jackrabbit.oak.security.user.UserPrincipalProvider + +The introduction of the optional `UserConfiguration.getUserPrincipalProvider` +extension allows for an optimized variant of the default principal provider, which +is reading principal information from user and group accounts always paired with +a `Principal` instance. + +This extension allows for a provider based implementation details of the user +management implementation it is associated with, offering the ability to optimize +the principal lookup without having to use regular Jackrabbit user management +API calls and the associated overhead in terms of user/group object creation. + +While the implementation is located along with the user management implementation +present in Oak this provider implementation should not be considered being +part of the user management API implementation. + +Another benefit of this optimized implementation is the ability to specifically +cache the results of the principal resolution in order to improve login performance. +See section [Caching Results of Principal Resolution](cache.html) for further details. + +### org.apache.jackrabbit.oak.spi.security.principal.CompositePrincipalProvider + +This implementation is a simple wrapper implementation that combines different +principals from different source providers. It is used in +`CompositePrincipalConfiguration` held by the default `SecurityProvider` to +collect all configured/plugged principal configurations i.e. the various +implementations of principal management. + Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProvider.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProvider.java (revision 1692521) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProvider.java (revision ) @@ -20,35 +20,44 @@ import java.security.acl.Group; import java.text.ParseException; import java.util.Collections; +import java.util.Date; import java.util.HashSet; import java.util.Iterator; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import javax.jcr.AccessDeniedException; import javax.jcr.RepositoryException; import com.google.common.base.Function; +import com.google.common.base.Joiner; import com.google.common.base.Predicate; import com.google.common.base.Predicates; +import com.google.common.collect.Iterables; import com.google.common.collect.Iterators; import org.apache.jackrabbit.api.security.principal.PrincipalManager; import org.apache.jackrabbit.api.security.user.Authorizable; import org.apache.jackrabbit.api.security.user.UserManager; +import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Result; import org.apache.jackrabbit.oak.api.ResultRow; import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.commons.LongUtils; import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.security.user.query.QueryUtil; import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl; import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider; +import org.apache.jackrabbit.oak.spi.security.principal.SystemPrincipal; import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType; 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.util.UserUtil; +import org.apache.jackrabbit.oak.util.NodeUtil; +import org.apache.jackrabbit.util.Text; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,6 +73,11 @@ private static final Logger log = LoggerFactory.getLogger(UserPrincipalProvider.class); + static final String PARAM_CACHE_EXPIRATION = "cacheExpiration"; + static final long EXPIRATION_NO_CACHE = 0; + + private static final long MEMBERSHIP_THRESHOLD = 0; + private final Root root; private final UserConfiguration config; private final NamePathMapper namePathMapper; @@ -71,6 +85,9 @@ private final UserProvider userProvider; private final MembershipProvider membershipProvider; + private final long expiration; + private final boolean cacheEnabled; + UserPrincipalProvider(@Nonnull Root root, @Nonnull UserConfiguration userConfiguration, @Nonnull NamePathMapper namePathMapper) { @@ -80,6 +97,9 @@ this.userProvider = new UserProvider(root, config.getParameters()); this.membershipProvider = new MembershipProvider(root, config.getParameters()); + + expiration = config.getParameters().getConfigValue(PARAM_CACHE_EXPIRATION, EXPIRATION_NO_CACHE); + cacheEnabled = (expiration > EXPIRATION_NO_CACHE && root.getContentSession().getAuthInfo().getPrincipals().contains(SystemPrincipal.INSTANCE)); } //--------------------------------------------------< PrincipalProvider >--- @@ -219,23 +239,108 @@ @Nonnull private Set getGroupMembership(@Nonnull Tree authorizableTree) { - Set groupPrincipals = new HashSet(); + Set groupPrincipals = null; + NodeUtil authorizableNode = new NodeUtil(authorizableTree); + boolean doCache = cacheEnabled && UserUtil.isType(authorizableTree, AuthorizableType.USER); + if (doCache) { + groupPrincipals = readGroupsFromCache(authorizableNode); + } + + // caching not configured or cache expired: use the membershipProvider to calculate + if (groupPrincipals == null) { + groupPrincipals = new HashSet(); - Iterator groupPaths = membershipProvider.getMembership(authorizableTree, true); - while (groupPaths.hasNext()) { - Tree groupTree = userProvider.getAuthorizableByPath(groupPaths.next()); - if (groupTree != null && UserUtil.isType(groupTree, AuthorizableType.GROUP)) { - Group gr = createGroupPrincipal(groupTree); - if (gr != null) { - groupPrincipals.add(createGroupPrincipal(groupTree)); - } - } - } + Iterator groupPaths = membershipProvider.getMembership(authorizableTree, true); + while (groupPaths.hasNext()) { + Tree groupTree = userProvider.getAuthorizableByPath(groupPaths.next()); + if (groupTree != null && UserUtil.isType(groupTree, AuthorizableType.GROUP)) { + Group gr = createGroupPrincipal(groupTree); + if (gr != null) { + groupPrincipals.add(createGroupPrincipal(groupTree)); + } + } + } + + // remember the regular groups in case caching is enabled + if (doCache) { + cacheGroups(authorizableNode, groupPrincipals); + } + } + // add the dynamic everyone principal group which is not included in // the 'getMembership' call. groupPrincipals.add(EveryonePrincipal.getInstance()); return groupPrincipals; } + private void cacheGroups(@Nonnull NodeUtil authorizableNode, @Nonnull Set groupPrincipals) { + try { + root.refresh(); + NodeUtil cache = authorizableNode.getChild(CacheConstants.REP_CACHE); + if (cache == null) { + if (groupPrincipals.size() <= MEMBERSHIP_THRESHOLD) { + log.debug("Omit cache creation for user without group membership at " + authorizableNode.getTree().getPath()); + return; + } else { + log.debug("Create new group membership cache at " + authorizableNode.getTree().getPath()); + cache = authorizableNode.addChild(CacheConstants.REP_CACHE, CacheConstants.NT_REP_CACHE); + } + } + + cache.setLong(CacheConstants.REP_EXPIRATION, LongUtils.calculateExpirationTime(expiration)); + String value = (groupPrincipals.isEmpty()) ? "" : Joiner.on(",").join(Iterables.transform(groupPrincipals, new Function() { + @Override + public String apply(Group input) { + return Text.escape(input.getName()); + } + })); + cache.setString(CacheConstants.REP_GROUP_PRINCIPAL_NAMES, value); + + root.commit(CacheValidatorProvider.asCommitAttributes()); + log.debug("Cached group membership at " + authorizableNode.getTree().getPath()); + + } catch (AccessDeniedException e) { + log.debug("Failed to cache group membership", e.getMessage()); + } catch (CommitFailedException e) { + log.debug("Failed to cache group membership", e.getMessage(), e); + } finally { + root.refresh(); + } + } + + @CheckForNull + private Set readGroupsFromCache(@Nonnull NodeUtil authorizableNode) { + NodeUtil principalCache = authorizableNode.getChild(CacheConstants.REP_CACHE); + if (principalCache == null) { + log.debug("No group cache at " + authorizableNode.getTree().getPath()); + return null; + } + + if (isValidCache(principalCache)) { + log.debug("Reading group membership at " + authorizableNode.getTree().getPath()); + + String str = principalCache.getString(CacheConstants.REP_GROUP_PRINCIPAL_NAMES, null); + if (str == null || str.isEmpty()) { + return new HashSet(1); + } + + Set groups = new HashSet(); + for (String s : Text.explode(str, ',')) { + final String name = Text.unescape(s); + groups.add(new CachedGroupPrincipal(name)); + } + return groups; + } else { + log.debug("Expired group cache for " + authorizableNode.getTree().getPath()); + return null; + } + } + + private static boolean isValidCache(NodeUtil principalCache) { + long expirationTime = principalCache.getLong(CacheConstants.REP_EXPIRATION, EXPIRATION_NO_CACHE); + long now = new Date().getTime(); + return expirationTime > EXPIRATION_NO_CACHE && now < expirationTime; + } + private static String buildSearchPattern(String nameHint) { if (nameHint == null) { return "%"; @@ -246,7 +351,6 @@ sb.append('%'); return sb.toString(); } - } private static boolean matchesEveryone(String nameHint, int searchType) { @@ -288,19 +392,22 @@ } } - /** - * Implementation of {@link AbstractGroupPrincipal} that reads the underlying - * authorizable group lazily in case the group membership must be retrieved. - */ - private final class GroupPrincipal extends AbstractGroupPrincipal { + //-------------------------------------------------------------------------- + // Group Principal implementations that retrieve member information on demand + //-------------------------------------------------------------------------- + private abstract class BaseGroupPrincipal extends AbstractGroupPrincipal { + private UserManager userManager; - private org.apache.jackrabbit.api.security.user.Group group; - GroupPrincipal(@Nonnull String principalName, @Nonnull Tree groupTree) { + BaseGroupPrincipal(@Nonnull String principalName, @Nonnull Tree groupTree) { super(principalName, groupTree, namePathMapper); } + BaseGroupPrincipal(@Nonnull String principalName, @Nonnull String groupPath) { + super(principalName, groupPath, namePathMapper); + } + @Override UserManager getUserManager() { if (userManager == null) { @@ -327,7 +434,25 @@ return (g == null) ? Iterators.emptyIterator() : g.getMembers(); } - private org.apache.jackrabbit.api.security.user.Group getGroup() throws RepositoryException { + @CheckForNull + abstract org.apache.jackrabbit.api.security.user.Group getGroup()throws RepositoryException; + } + + /** + * Implementation of {@link AbstractGroupPrincipal} that reads the underlying + * authorizable group lazily in case the group membership must be retrieved. + */ + private final class GroupPrincipal extends BaseGroupPrincipal { + + private org.apache.jackrabbit.api.security.user.Group group; + + GroupPrincipal(@Nonnull String principalName, @Nonnull Tree groupTree) { + super(principalName, groupTree); + } + + @Override + @CheckForNull + org.apache.jackrabbit.api.security.user.Group getGroup() throws RepositoryException { if (group == null) { Authorizable authorizable = getUserManager().getAuthorizable(this); if (authorizable != null && authorizable.isGroup()) { @@ -337,4 +462,44 @@ return group; } } + + private final class CachedGroupPrincipal extends BaseGroupPrincipal { + + private org.apache.jackrabbit.api.security.user.Group group; + + CachedGroupPrincipal(@Nonnull String principalName) { + super(principalName, ""); -} + } + + @Override + String getOakPath() { + String groupPath = getPath(); + return (groupPath == null) ? null : namePathMapper.getOakPath(getPath()); + } + + @Override + public String getPath() { + try { + org.apache.jackrabbit.api.security.user.Group gr = getGroup(); + return (gr == null) ? null : gr.getPath(); + } catch (RepositoryException e) { + log.error("Failed to retrieve path from group principal", e.getMessage()); + return null; + } + } + + @Override + @CheckForNull + org.apache.jackrabbit.api.security.user.Group getGroup() throws RepositoryException { + if (group == null) { + Authorizable authorizable = getUserManager().getAuthorizable(new PrincipalImpl(getName())); + if (authorizable != null && authorizable.isGroup()) { + group = (org.apache.jackrabbit.api.security.user.Group) authorizable; + } + } + return group; + } + } +} + +