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:51 UTC
[jackrabbit-oak] branch OAK-9599 created (now 3945733)
This is an automated email from the ASF dual-hosted git repository.
angela pushed a change to branch OAK-9599
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git.
at 3945733 OAK-9599 : Enforce dynamic membership upon user login
This branch includes the following new commits:
new 3945733 OAK-9599 : Enforce dynamic membership upon user login
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[jackrabbit-oak] 01/01: OAK-9599 : Enforce dynamic membership upon
user login
Posted by an...@apache.org.
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