OAK-4845 - Regression: DefaultSyncContext does not sync membership to…

… a local group
alexkli · Sep 23, 2016 · 42c855c · 42c855c
1 parent 763a737
commit 42c855c
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 1 deletion.
diff --git a/.../apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java b/.../apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java
@@ -533,7 +533,7 @@ protected void syncMembership(@Nonnull ExternalIdentity external, @Nonnull Autho
             if (a == null) {
                 grp = createGroup(extGroup);
                 log.debug("- created new group");
-            } else if (a.isGroup() && isSameIDP(a)) {
+            } else if (a.isGroup() && isLocalOrSameIDP(a)) {
                 grp = (Group) a;
             } else {
                 log.warn("Existing authorizable '{}' is not a group from this IDP '{}'.", extGroup.getId(), idp.getName());
@@ -737,6 +737,18 @@ protected boolean isSameIDP(@Nullable Authorizable auth) throws RepositoryExcept
         return ref != null && idp.getName().equals(ref.getProviderName());
     }
 
+    /**
+     * Checks if the given authorizable was synced from the same IDP or does not have an external IDP
+     * reference in form of the {@value #REP_EXTERNAL_ID} property.
+     *
+     * @param auth the authorizable.
+     * @return {@code true} if local or same IDP.
+     */
+    protected boolean isLocalOrSameIDP(@Nullable Authorizable auth) throws RepositoryException {
+        ExternalIdentityRef ref = getIdentityRef(auth);
+        return ref == null || idp.getName().equals(ref.getProviderName());
+    }
+
     /**
      * Tests if the given {@link ExternalIdentityRef} refers to the same IDP
      * as associated with this context instance.

diff --git a/...che/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java b/...che/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java
@@ -636,6 +636,33 @@ public void testMembershipForExistingForeignGroup() throws Exception {
         }
     }
 
+    /**
+     * @see <a href="https://issues.apache.org/jira/browse/OAK-4845">OAK-4845</a>
+     */
+    @Test
+    public void testMembershipForExistingLocalGroup() throws Exception {
+        syncConfig.user().setMembershipNestingDepth(1).setMembershipExpirationTime(-1).setExpirationTime(-1);
+        syncConfig.group().setExpirationTime(-1);
+
+        ExternalUser externalUser = idp.getUser(USER_ID);
+        ExternalIdentityRef groupRef = externalUser.getDeclaredGroups().iterator().next();
+
+        // create the group locally (has no rep:externalId)
+        Group gr = userManager.createGroup(groupRef.getId());
+        root.commit();
+
+        sync(externalUser);
+
+        User user = userManager.getAuthorizable(externalUser.getId(), User.class);
+        assertNotNull(user);
+
+        // verify membership gets added
+        assertTrue(gr.isDeclaredMember(user));
+        Iterator<Group> declared = user.declaredMemberOf();
+        assertTrue(declared.hasNext());
+        assertTrue(gr.getID().equals(declared.next().getID()));
+    }
+
     @Test
     public void testGetAuthorizableUser() throws Exception {
         ExternalIdentity extUser = idp.listUsers().next();