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 2019/10/16 11:48:23 UTC
svn commit: r1868505 - in /jackrabbit/oak/trunk/oak-auth-external/src:
main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/
test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/
test/java/org/apache/jackrab...
Author: angela
Date: Wed Oct 16 11:48:23 2019
New Revision: 1868505
URL: http://svn.apache.org/viewvc?rev=1868505&view=rev
Log:
OAK-8665 : DynamicSyncContext doesn't verify the same IDP
Modified:
jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java
jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java
jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/PrincipalResolutionTest.java
Modified: jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java?rev=1868505&r1=1868504&r2=1868505&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java (original)
+++ jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java Wed Oct 16 11:48:23 2019
@@ -22,6 +22,8 @@ import javax.jcr.RepositoryException;
import javax.jcr.Value;
import javax.jcr.ValueFactory;
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.UserManager;
@@ -145,6 +147,8 @@ public class DynamicSyncContext extends
* Recursively collect the principal names of the given declared group
* references up to the given depth.
*
+ * Note, that this method will filter out references that don't belong to the same IDP (see OAK-8665).
+ *
* @param principalNames The set used to collect the names of the group principals.
* @param declaredGroupIdRefs The declared group references for a user or a group.
* @param depth Configured membership nesting; the recursion will be stopped once depths is < 1.
@@ -152,7 +156,7 @@ public class DynamicSyncContext extends
*/
private void collectPrincipalNames(@NotNull Set<String> principalNames, @NotNull Iterable<ExternalIdentityRef> declaredGroupIdRefs, long depth) throws ExternalIdentityException {
boolean shortcut = (depth <= 1 && idp instanceof PrincipalNameResolver);
- for (ExternalIdentityRef ref : declaredGroupIdRefs) {
+ for (ExternalIdentityRef ref : Iterables.filter(declaredGroupIdRefs, externalIdentityRef -> isSameIDP(externalIdentityRef))) {
if (shortcut) {
principalNames.add(((PrincipalNameResolver) idp).fromExternalIdentityRef(ref));
} else {
Modified: jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java?rev=1868505&r1=1868504&r2=1868505&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java (original)
+++ jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java Wed Oct 16 11:48:23 2019
@@ -28,6 +28,7 @@ import javax.security.auth.login.LoginEx
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import org.jetbrains.annotations.NotNull;
public class TestIdentityProvider implements ExternalIdentityProvider {
@@ -216,17 +217,25 @@ public class TestIdentityProvider implem
return props;
}
- protected TestIdentity withProperty(String name, Object value) {
+ @NotNull
+ protected TestIdentity withProperty(@NotNull String name, @NotNull Object value) {
props.put(name, value);
return this;
}
- protected TestIdentity withGroups(String ... grps) {
+ @NotNull
+ protected TestIdentity withGroups(@NotNull String ... grps) {
for (String grp: grps) {
groups.add(new ExternalIdentityRef(grp, id.getProviderName()));
}
return this;
}
+
+ @NotNull
+ public TestIdentity withGroups(@NotNull ExternalIdentityRef... groups) {
+ this.groups.addAll(ImmutableSet.copyOf(groups));
+ return this;
+ }
}
public static class TestUser extends TestIdentity implements ExternalUser {
Modified: jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java?rev=1868505&r1=1868504&r2=1868505&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java (original)
+++ jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java Wed Oct 16 11:48:23 2019
@@ -23,7 +23,6 @@ import javax.jcr.RepositoryException;
import javax.jcr.Value;
import javax.jcr.ValueFactory;
-import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -49,13 +48,16 @@ import org.apache.jackrabbit.oak.spi.sec
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.After;
import org.junit.Before;
import org.junit.Test;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.TestIdentityProvider.ID_SECOND_USER;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.TestIdentityProvider.ID_TEST_USER;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
@@ -96,25 +98,27 @@ public class DynamicSyncContextTest exte
return sc;
}
- private void sync(@NotNull ExternalIdentity externalIdentity, @NotNull SyncResult.Status expectedStatus) throws Exception {
+ protected void sync(@NotNull ExternalIdentity externalIdentity, @NotNull SyncResult.Status expectedStatus) throws Exception {
SyncResult result = syncContext.sync(externalIdentity);
assertSame(expectedStatus, result.getStatus());
r.commit();
}
+ protected void assertDynamicMembership(@NotNull ExternalIdentity externalIdentity, long depth) throws Exception {
+ Authorizable a = userManager.getAuthorizable(externalIdentity.getId());
+ assertNotNull(a);
+ assertDynamicMembership(a, externalIdentity, depth);
+ }
+
private void assertDynamicMembership(@NotNull Authorizable a, @NotNull ExternalIdentity externalIdentity, long depth) throws Exception {
- Value[] vs = a.getProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES);
- Iterable<String> pNames = Iterables.transform(ImmutableList.copyOf(vs), new Function<Value, String>() {
- @Nullable
- @Override
- public String apply(Value input) {
- try {
- return input.getString();
- } catch (RepositoryException e) {
- fail(e.getMessage());
- return null;
- }
- };
+ Value[] vs = a.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
+ Iterable<String> pNames = Iterables.transform(ImmutableList.copyOf(vs), input -> {
+ try {
+ return input.getString();
+ } catch (RepositoryException e) {
+ fail(e.getMessage());
+ return null;
+ }
});
Set<String> expected = new HashSet<>();
@@ -165,7 +169,7 @@ public class DynamicSyncContextTest exte
sync(externalUser, SyncResult.Status.ADD);
Tree tree = r.getTree(userManager.getAuthorizable(USER_ID).getPath());
- PropertyState extPrincipalNames = tree.getProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES);
+ PropertyState extPrincipalNames = tree.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
assertNotNull(extPrincipalNames);
assertEquals(0, extPrincipalNames.count());
}
@@ -178,7 +182,7 @@ public class DynamicSyncContextTest exte
sync(externalUser, SyncResult.Status.ADD);
Tree tree = r.getTree(userManager.getAuthorizable(USER_ID).getPath());
- PropertyState extPrincipalNames = tree.getProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES);
+ PropertyState extPrincipalNames = tree.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
assertNotNull(extPrincipalNames);
Set<String> pNames = Sets.newHashSet(extPrincipalNames.getValue(Type.STRINGS));
@@ -196,7 +200,7 @@ public class DynamicSyncContextTest exte
sync(externalUser, SyncResult.Status.ADD);
Tree tree = r.getTree(userManager.getAuthorizable(USER_ID).getPath());
- PropertyState extPrincipalNames = tree.getProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES);
+ PropertyState extPrincipalNames = tree.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
assertNotNull(extPrincipalNames);
Set<String> pNames = Sets.newHashSet(extPrincipalNames.getValue(Type.STRINGS));
@@ -224,7 +228,7 @@ public class DynamicSyncContextTest exte
syncContext.sync(externalUser);
Tree t = r.getTree(a.getPath());
- assertFalse(t.hasProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES));
+ assertFalse(t.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
assertSyncedMembership(userManager, a, externalUser);
}
@@ -301,7 +305,7 @@ public class DynamicSyncContextTest exte
assertEquals(SyncResult.Status.UPDATE, result.getStatus());
Tree t = r.getTree(a.getPath());
- assertTrue(t.hasProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES));
+ assertTrue(t.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
}
@Test
@@ -323,7 +327,7 @@ public class DynamicSyncContextTest exte
Authorizable a = userManager.getAuthorizable(USER_ID);
Tree t = r.getTree(a.getPath());
- assertFalse(t.hasProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES));
+ assertFalse(t.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
assertSyncedMembership(userManager, a, externalUser);
}
@@ -336,7 +340,7 @@ public class DynamicSyncContextTest exte
sync(externalUser, SyncResult.Status.ADD);
Authorizable a = userManager.getAuthorizable(externalUser.getId());
- assertDynamicMembership(a, externalUser, nesting);
+ assertDynamicMembership(externalUser, nesting);
// verify that the membership is always reflected in the rep:externalPrincipalNames property
// 1. membership nesting = -1
@@ -416,11 +420,53 @@ public class DynamicSyncContextTest exte
Authorizable gr = userManager.getAuthorizable(externalGroup.getId());
syncContext.syncMembership(externalGroup, gr, 1);
- assertFalse(gr.hasProperty(ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES));
+ assertFalse(gr.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
assertFalse(r.hasPendingChanges());
}
@Test
+ public void testSyncMembershipWithForeignGroups() throws Exception {
+ TestIdentityProvider.TestUser testuser = (TestIdentityProvider.TestUser) idp.getUser(ID_TEST_USER);
+ Set<ExternalIdentityRef> sameIdpGroups = ImmutableSet.copyOf(testuser.getDeclaredGroups());
+
+ TestIdentityProvider.ForeignExternalGroup foreignGroup = new TestIdentityProvider.ForeignExternalGroup();
+ testuser.withGroups(foreignGroup.getExternalId());
+ assertFalse(Iterables.elementsEqual(sameIdpGroups, testuser.getDeclaredGroups()));
+
+ sync(testuser, SyncResult.Status.ADD);
+
+ Authorizable a = userManager.getAuthorizable(ID_TEST_USER);
+ assertTrue(a.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
+ Value[] extPrincipalNames = a.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
+
+ assertEquals(Iterables.size(sameIdpGroups), extPrincipalNames.length);
+ for (Value v : extPrincipalNames) {
+ assertNotEquals(foreignGroup.getPrincipalName(), v.getString());
+ }
+ }
+
+ @Test
+ public void testSyncMembershipWithUserRef() throws Exception {
+ TestIdentityProvider.TestUser testuser = (TestIdentityProvider.TestUser) idp.getUser(ID_TEST_USER);
+ Set<ExternalIdentityRef> groupRefs = ImmutableSet.copyOf(testuser.getDeclaredGroups());
+
+ ExternalUser second = idp.getUser(ID_SECOND_USER);
+ testuser.withGroups(second.getExternalId());
+ assertFalse(Iterables.elementsEqual(groupRefs, testuser.getDeclaredGroups()));
+
+ sync(testuser, SyncResult.Status.ADD);
+
+ Authorizable a = userManager.getAuthorizable(ID_TEST_USER);
+ assertTrue(a.hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
+ Value[] extPrincipalNames = a.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
+
+ assertEquals(Iterables.size(groupRefs), extPrincipalNames.length);
+ for (Value v : extPrincipalNames) {
+ assertNotEquals(second.getPrincipalName(), v.getString());
+ }
+ }
+
+ @Test
public void testAutoMembership() throws Exception {
Group gr = userManager.createGroup("group" + UUID.randomUUID());
r.commit();
Modified: jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/PrincipalResolutionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/PrincipalResolutionTest.java?rev=1868505&r1=1868504&r2=1868505&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/PrincipalResolutionTest.java (original)
+++ jackrabbit/oak/trunk/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/PrincipalResolutionTest.java Wed Oct 16 11:48:23 2019
@@ -16,13 +16,24 @@
*/
package org.apache.jackrabbit.oak.spi.security.authentication.external.impl;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentity;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityException;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityProvider;
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.PrincipalNameResolver;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncResult;
import org.apache.jackrabbit.oak.spi.security.authentication.external.TestIdentityProvider;
import org.jetbrains.annotations.NotNull;
+import org.junit.Test;
+
+import java.util.Set;
+
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.TestIdentityProvider.ID_SECOND_USER;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.TestIdentityProvider.ID_TEST_USER;
+import static org.junit.Assert.assertFalse;
public class PrincipalResolutionTest extends DynamicSyncContextTest {
@@ -44,4 +55,22 @@ public class PrincipalResolutionTest ext
}
}
}
+
+ /**
+ * With {@code PrincipalNameResolver} the extra verification for all member-refs being groups is omitted.
+ * @throws Exception
+ */
+ @Test
+ public void testSyncMembershipWithUserRef() throws Exception {
+ TestIdentityProvider.TestUser testuser = (TestIdentityProvider.TestUser) idp.getUser(ID_TEST_USER);
+ Set<ExternalIdentityRef> groupRefs = ImmutableSet.copyOf(testuser.getDeclaredGroups());
+
+ ExternalUser second = idp.getUser(ID_SECOND_USER);
+ testuser.withGroups(second.getExternalId());
+ assertFalse(Iterables.elementsEqual(groupRefs, testuser.getDeclaredGroups()));
+
+ sync(testuser, SyncResult.Status.ADD);
+
+ assertDynamicMembership(testuser, 1);
+ }
}