You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2021/10/14 15:52:52 UTC
[jackrabbit-oak] 01/01: OAK-9599 : Enforce dynamic membership upon
user login
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch OAK-9599
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 3945733b8c38f0259a52e67398e2a5ce68ed56f7
Author: angela <an...@adobe.com>
AuthorDate: Thu Oct 14 17:52:35 2021 +0200
OAK-9599 : Enforce dynamic membership upon user login
---
.../external/basic/DefaultSyncConfig.java | 27 +++-
.../external/basic/package-info.java | 2 +-
.../external/impl/DynamicSyncContext.java | 38 ++++--
.../external/basic/DefaultSyncConfigTest.java | 14 ++
.../external/impl/DynamicSyncContextTest.java | 20 +--
.../impl/EnforceDynamicMembershipTest.java | 151 +++++++++++++++++++++
6 files changed, 232 insertions(+), 20 deletions(-)
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java
index 54f752e..b77e02d 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java
@@ -277,6 +277,8 @@ public class DefaultSyncConfig {
private long membershipNestingDepth;
private boolean dynamicMembership;
+
+ private boolean enforceDynamicMembership;
private boolean disableMissing;
@@ -343,7 +345,7 @@ public class DefaultSyncConfig {
}
/**
- * Enable or disable the dynamic group membership. If turned external
+ * Enable or disable the dynamic group membership. If turned on external
* identities and their group membership will be synchronized such that the
* membership information is generated dynamically. External groups may
* or may not be synchronized into the repository if this option is turned
@@ -361,6 +363,29 @@ public class DefaultSyncConfig {
}
/**
+ * Returns {@code true} if a dynamic group membership must be enforced for users that have been synchronized
+ * previously. Note that this option has no effect if {@link #getDynamicMembership()} returns {@code false}.
+ *
+ * @return {@code true} if dynamic group membership for external user identities must be enforced for previously
+ * synced users; {@code false} otherwise. This option only takes effect if {@link #getDynamicMembership()} is enabled.
+ */
+ public boolean getEnforceDynamicMembership() {
+ return enforceDynamicMembership;
+ }
+
+ /**
+ * Enable or disable the enforcement of dynamic group membership.
+ *
+ * @param enforceDynamicMembership Boolean flag to define if dynamic group management is enforced for previously synced users.
+ * @return {@code this}
+ * @see #getEnforceDynamicMembership() () for details.
+ */
+ public User setEnforceDynamicMembership(boolean enforceDynamicMembership) {
+ this.enforceDynamicMembership = enforceDynamicMembership;
+ return this;
+ }
+
+ /**
* Controls the behavior for users that no longer exist on the external provider. The default is to delete the repository users
* if they no longer exist on the external provider. If set to true, they will be disabled instead, and re-enabled once they appear
* again.
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/package-info.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/package-info.java
index a82ca08..e70a09e 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/package-info.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/package-info.java
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-@Version("1.4.0")
+@Version("1.5.0")
package org.apache.jackrabbit.oak.spi.security.authentication.external.basic;
import org.osgi.annotation.versioning.Version;
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java
index fe44edc..7cbfd1d 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java
@@ -16,12 +16,6 @@
*/
package org.apache.jackrabbit.oak.spi.security.authentication.external.impl;
-import java.util.HashSet;
-import java.util.Set;
-import javax.jcr.RepositoryException;
-import javax.jcr.Value;
-import javax.jcr.ValueFactory;
-
import com.google.common.collect.Iterables;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
@@ -43,6 +37,13 @@ import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.ValueFactory;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
/**
* Extension of the {@code DefaultSyncContext} that doesn't synchronize group
* membership of new external users into the user management of the repository.
@@ -114,8 +115,9 @@ public class DynamicSyncContext extends DefaultSyncContext {
return;
}
- if (auth.hasProperty(REP_LAST_SYNCED) && !auth.hasProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES)) {
- // user has been synchronized before dynamic membership has been turned on
+ boolean groupsSyncedBefore = groupsSyncedBefore(auth);
+ if (groupsSyncedBefore && !config.user().getEnforceDynamicMembership()) {
+ // user has been synchronized before dynamic membership has been turned on and dynamic membership is not enforced
super.syncMembership(external, auth, depth);
} else {
// retrieve membership of the given external user (up to the configured
@@ -131,6 +133,9 @@ public class DynamicSyncContext extends DefaultSyncContext {
vs = createValues(principalsNames);
}
auth.setProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES, vs);
+ if (groupsSyncedBefore) {
+ clearGroupMembership(auth);
+ }
} catch (ExternalIdentityException e) {
log.error("Failed to synchronize membership information for external identity {}", external.getId(), e);
}
@@ -173,4 +178,21 @@ public class DynamicSyncContext extends DefaultSyncContext {
}
}
}
+
+ private void clearGroupMembership(@NotNull Authorizable authorizable) throws RepositoryException {
+ Iterator<Group> grpIter = authorizable.declaredMemberOf();
+ while (grpIter.hasNext()) {
+ Group grp = grpIter.next();
+ if (isSameIDP(grp)) {
+ grp.removeMember(authorizable);
+ if (!grp.getDeclaredMembers().hasNext()) {
+ grp.remove();
+ }
+ }
+ }
+ }
+
+ private static boolean groupsSyncedBefore(@NotNull Authorizable authorizable) throws RepositoryException {
+ return authorizable.hasProperty(REP_LAST_SYNCED) && !authorizable.hasProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES);
+ }
}
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfigTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfigTest.java
index 0d8dbd2..6703a47 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfigTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfigTest.java
@@ -27,6 +27,7 @@ import org.jetbrains.annotations.NotNull;
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.assertSame;
import static org.junit.Assert.assertTrue;
@@ -93,6 +94,19 @@ public class DefaultSyncConfigTest {
assertEquals(5, userConfig.getMembershipNestingDepth());
assertEquals(0, userConfig.setMembershipExpirationTime(0).getMembershipExpirationTime());
}
+
+ @Test
+ public void testUserDynamicMembership() {
+ DefaultSyncConfig.User userConfig = config.user();
+
+ assertFalse(userConfig.getDynamicMembership());
+ assertSame(userConfig, userConfig.setDynamicMembership(true));
+ assertTrue(userConfig.getDynamicMembership());
+
+ assertFalse(userConfig.getEnforceDynamicMembership());
+ assertSame(userConfig, userConfig.setEnforceDynamicMembership(true));
+ assertTrue(userConfig.getEnforceDynamicMembership());
+ }
@Test
public void testGroupConfig() {
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
index 9afc5af..aa55c29 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
@@ -77,11 +77,11 @@ import static org.mockito.Mockito.when;
public class DynamicSyncContextTest extends AbstractExternalAuthTest {
- private Root r;
- private UserManager userManager;
- private ValueFactory valueFactory;
+ Root r;
+ UserManager userManager;
+ ValueFactory valueFactory;
- private DynamicSyncContext syncContext;
+ DynamicSyncContext syncContext;
@Before
public void before() throws Exception {
@@ -149,9 +149,9 @@ public class DynamicSyncContextTest extends AbstractExternalAuthTest {
}
}
- private static void assertSyncedMembership(@NotNull UserManager userManager,
- @NotNull Authorizable a,
- @NotNull ExternalIdentity externalIdentity) throws Exception {
+ static void assertSyncedMembership(@NotNull UserManager userManager,
+ @NotNull Authorizable a,
+ @NotNull ExternalIdentity externalIdentity) throws Exception {
for (ExternalIdentityRef ref : externalIdentity.getDeclaredGroups()) {
Group gr = userManager.getAuthorizable(ref.getId(), Group.class);
assertNotNull(gr);
@@ -398,7 +398,7 @@ public class DynamicSyncContextTest extends AbstractExternalAuthTest {
syncContext.syncMembership(mod, a, nesting);
assertDynamicMembership(a, mod, nesting);
- // 2. set with different groups that defined on IDP
+ // 2. set with different groups than defined on IDP
mod = new TestUserWithGroupRefs(externalUser, ImmutableSet.of(
idp.getGroup("a").getExternalId(),
idp.getGroup("aa").getExternalId(),
@@ -526,11 +526,11 @@ public class DynamicSyncContextTest extends AbstractExternalAuthTest {
assertFalse(gr.isMember(u));
}
- private static final class TestUserWithGroupRefs extends TestIdentityProvider.TestIdentity implements ExternalUser {
+ static final class TestUserWithGroupRefs extends TestIdentityProvider.TestIdentity implements ExternalUser {
private final Iterable<ExternalIdentityRef> declaredGroupRefs;
- private TestUserWithGroupRefs(@NotNull ExternalUser base, @NotNull Iterable<ExternalIdentityRef> declaredGroupRefs) {
+ TestUserWithGroupRefs(@NotNull ExternalUser base, @NotNull Iterable<ExternalIdentityRef> declaredGroupRefs) {
super(base);
this.declaredGroupRefs = declaredGroupRefs;
}
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/EnforceDynamicMembershipTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/EnforceDynamicMembershipTest.java
new file mode 100644
index 0000000..94c5973
--- /dev/null
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/EnforceDynamicMembershipTest.java
@@ -0,0 +1,151 @@
+/*
+ * 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.spi.security.authentication.external.impl;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentity;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalUser;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncConfig;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncContext;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class EnforceDynamicMembershipTest extends DynamicSyncContextTest {
+
+ @Override
+ @NotNull
+ protected DefaultSyncConfig createSyncConfig() {
+ DefaultSyncConfig sc = super.createSyncConfig();
+ sc.user().setDynamicMembership(true).setEnforceDynamicMembership(true);
+ return sc;
+ }
+
+ @Test
+ public void testSyncMembershipWithChangedExistingGroups() throws Exception {
+ long nesting = 1;
+ syncConfig.user().setMembershipNestingDepth(nesting);
+
+ ExternalUser externalUser = idp.getUser(USER_ID);
+
+ DefaultSyncContext ctx = new DefaultSyncContext(syncConfig, idp, userManager, valueFactory);
+ ctx.sync(externalUser);
+ ctx.close();
+
+ Authorizable a = userManager.getAuthorizable(externalUser.getId());
+ assertSyncedMembership(userManager, a, externalUser);
+ r.commit();
+
+ // set with different groups than defined on IDP
+ TestUserWithGroupRefs mod = new TestUserWithGroupRefs(externalUser, ImmutableSet.of(
+ idp.getGroup("a").getExternalId(),
+ idp.getGroup("aa").getExternalId(),
+ idp.getGroup("secondGroup").getExternalId()));
+ syncContext.syncMembership(mod, a, nesting);
+
+ Tree t = r.getTree(a.getPath());
+ assertTrue(t.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
+ assertMigratedGroups(userManager, mod, null);
+ assertMigratedGroups(userManager, externalUser, null);
+ }
+
+ @Test
+ public void testSyncExternalUserExistingGroups() throws Exception {
+ syncConfig.user().setMembershipNestingDepth(1);
+
+ ExternalUser externalUser = idp.getUser(USER_ID);
+
+ DefaultSyncContext ctx = new DefaultSyncContext(syncConfig, idp, userManager, valueFactory);
+ ctx.sync(externalUser);
+ ctx.close();
+
+ Authorizable a = userManager.getAuthorizable(USER_ID);
+ assertSyncedMembership(userManager, a, externalUser);
+ // add an addition member to one group
+ ExternalIdentityRef grRef = externalUser.getDeclaredGroups().iterator().next();
+ Group gr = userManager.getAuthorizable(grRef.getId(), Group.class);
+ gr.addMember(userManager.createGroup("someOtherMember"));
+ r.commit();
+
+ syncContext.setForceUserSync(true);
+ syncConfig.user().setMembershipExpirationTime(-1);
+ syncContext.sync(externalUser);
+
+ // membership must have been migrated from group to rep:externalPrincipalNames
+ // groups that have no other members left must be deleted.
+ Tree t = r.getTree(a.getPath());
+ assertTrue(t.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
+ assertDynamicMembership(externalUser, 1);
+ assertMigratedGroups(userManager, externalUser, grRef);
+ }
+
+ @Test
+ public void testGroupFromDifferentIDP() throws Exception {
+ syncConfig.user().setMembershipNestingDepth(1);
+
+ ExternalUser externalUser = idp.getUser(USER_ID);
+
+ DefaultSyncContext ctx = new DefaultSyncContext(syncConfig, idp, userManager, valueFactory);
+ ctx.sync(externalUser);
+ ctx.close();
+
+ Authorizable a = userManager.getAuthorizable(USER_ID);
+ assertSyncedMembership(userManager, a, externalUser);
+ // add as member to a group from a different IDP
+ Group gr = userManager.createGroup("anotherGroup");
+ gr.addMember(a);
+ r.commit();
+
+ syncContext.setForceUserSync(true);
+ syncConfig.user().setMembershipExpirationTime(-1);
+ syncContext.sync(externalUser);
+
+ // membership must have been migrated from group to rep:externalPrincipalNames
+ // groups that have no other members left must be deleted.
+ Tree t = r.getTree(a.getPath());
+ assertTrue(t.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
+ assertDynamicMembership(externalUser, 1);
+ assertMigratedGroups(userManager, externalUser, null);
+
+ gr = userManager.getAuthorizable("anotherGroup", Group.class);
+ assertNotNull(gr);
+ assertTrue(gr.isMember(a));
+ }
+
+ private static void assertMigratedGroups(@NotNull UserManager userManager,
+ @NotNull ExternalIdentity externalIdentity,
+ @Nullable ExternalIdentityRef grRef) throws Exception {
+ for (ExternalIdentityRef ref : externalIdentity.getDeclaredGroups()) {
+ Group gr = userManager.getAuthorizable(ref.getId(), Group.class);
+ if (ref.equals(grRef)) {
+ assertNotNull(gr);
+ } else {
+ assertNull(gr);
+ }
+ }
+ }
+}
\ No newline at end of file