From 42c855cb583a85ef19ab839bf10d398967923d8c Mon Sep 17 00:00:00 2001
From: Alexander Klimetschek <aklimets@adobe.com>
Date: Thu, 22 Sep 2016 17:05:13 -0700
Subject: [PATCH] OAK-4845 - Regression: DefaultSyncContext does not sync
 membership to a local group

---
 .../external/basic/DefaultSyncContext.java    | 14 +++++++++-
 .../basic/DefaultSyncContextTest.java         | 27 +++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java
index a46ff354db1..5fe1e842a1c 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java
+++ b/oak-auth-external/src/main/java/org/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/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java
index a09a98aa829..45c1b4c72c6 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java
+++ b/oak-auth-external/src/test/java/org/apache/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();