From 398ae15d5d23c5416087260ac8a00b1fefdbbfb0 Mon Sep 17 00:00:00 2001
From: Alexander Klimetschek <aklimets@adobe.com>
Date: Mon, 26 Sep 2016 09:44:26 -0700
Subject: [PATCH] OAK-4845 - Regression: DefaultSyncContext does not sync
 membership to a local group

---
 .../external/basic/DefaultSyncContext.java         | 14 +++++++++-
 .../external/basic/DefaultSyncContextTest.java     | 32 ++++++++++++++++++++++
 2 files changed, 45 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 a46ff35..5fe1e84 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());
@@ -738,6 +738,18 @@ protected boolean isSameIDP(@Nullable Authorizable auth) throws RepositoryExcept
     }
 
     /**
+     * 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 a09a98a..7c092c4 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
@@ -64,6 +64,7 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class DefaultSyncContextTest extends AbstractExternalAuthTest {
 
@@ -636,6 +637,37 @@ 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();
+        while (declared.hasNext()) {
+            if (gr.getID().equals(declared.next().getID())) {
+                return;
+            }
+        }
+        fail();
+    }
+
     @Test
     public void testGetAuthorizableUser() throws Exception {
         ExternalIdentity extUser = idp.listUsers().next();