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/06/23 14:02:33 UTC
[jackrabbit-oak] branch trunk updated: OAK-9470 : Move
AutoMembershipPrincipals from ExternalGroupPrincipalProvider to top level
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new b1acba6 OAK-9470 : Move AutoMembershipPrincipals from ExternalGroupPrincipalProvider to top level
b1acba6 is described below
commit b1acba6292464f63df78d09b9c3a3acf22a7041b
Author: angela <an...@adobe.com>
AuthorDate: Wed Jun 23 16:01:48 2021 +0200
OAK-9470 : Move AutoMembershipPrincipals from ExternalGroupPrincipalProvider to top level
---
.../impl/principal/AutoMembershipPrincipals.java | 76 +++++++++++++++++
.../principal/ExternalGroupPrincipalProvider.java | 61 +-------------
.../impl/principal/AbstractAutoMembershipTest.java | 54 ++++++++++++
.../principal/AutoMembershipPrincipalsTest.java | 96 ++++++++++++++++++++++
4 files changed, 228 insertions(+), 59 deletions(-)
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipals.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipals.java
new file mode 100644
index 0000000..9fe1eb2
--- /dev/null
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipals.java
@@ -0,0 +1,76 @@
+package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.oak.spi.security.principal.GroupPrincipals;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.jcr.RepositoryException;
+import java.security.Principal;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+final class AutoMembershipPrincipals {
+
+ private static final Logger log = LoggerFactory.getLogger(AutoMembershipPrincipals.class);
+
+ private final UserManager userManager;
+ private final Map<String, String[]> autoMembershipMapping;
+ private final Map<String, Set<Principal>> principalMap;
+
+ AutoMembershipPrincipals(@NotNull UserManager userManager, @NotNull Map<String, String[]> autoMembershipMapping) {
+ this.userManager = userManager;
+ this.autoMembershipMapping = autoMembershipMapping;
+ this.principalMap = new ConcurrentHashMap<>(autoMembershipMapping.size());
+ }
+
+ @NotNull
+ Collection<Principal> getPrincipals(@Nullable String idpName) {
+ if (idpName == null) {
+ return ImmutableSet.of();
+ }
+
+ Set<Principal> principals;
+ if (!principalMap.containsKey(idpName)) {
+ principals = collectAutomembershipPrincipals(idpName);
+ principalMap.put(idpName, principals);
+ } else {
+ principals = principalMap.get(idpName);
+ }
+ return principals;
+ }
+
+ @NotNull
+ private Set<Principal> collectAutomembershipPrincipals(@NotNull String idpName) {
+ String[] vs = autoMembershipMapping.get(idpName);
+ if (vs == null) {
+ return ImmutableSet.of();
+ }
+
+ ImmutableSet.Builder<Principal> builder = ImmutableSet.builder();
+ for (String groupId : vs) {
+ try {
+ Authorizable gr = userManager.getAuthorizable(groupId);
+ if (gr != null && gr.isGroup()) {
+ Principal grPrincipal = gr.getPrincipal();
+ if (GroupPrincipals.isGroup(grPrincipal)) {
+ builder.add(grPrincipal);
+ } else {
+ log.warn("Principal of group {} is not of group type -> Ignoring", groupId);
+ }
+ } else {
+ log.warn("Configured auto-membership group {} does not exist -> Ignoring", groupId);
+ }
+ } catch (RepositoryException e) {
+ log.debug("Failed to retrieved 'auto-membership' group with id {}", groupId, e);
+ }
+ }
+ return builder.build();
+ }
+}
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
index c5e1c61..741b935 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
@@ -58,7 +58,6 @@ import javax.jcr.Value;
import javax.jcr.query.Query;
import java.security.Principal;
import java.text.ParseException;
-import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.Enumeration;
@@ -68,7 +67,6 @@ import java.util.Map;
import java.util.Set;
import java.util.Spliterator;
import java.util.Spliterators;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
@@ -114,7 +112,7 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
this.namePathMapper = namePathMapper;
userManager = uc.getUserManager(root, namePathMapper);
- autoMembershipPrincipals = new AutoMembershipPrincipals(autoMembershipMapping);
+ autoMembershipPrincipals = new AutoMembershipPrincipals(userManager, autoMembershipMapping);
}
//--------------------------------------------------< PrincipalProvider >---
@@ -231,7 +229,7 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
}
// add existing group principals as defined with the _autoMembership_ option.
- groupPrincipals.addAll(autoMembershipPrincipals.get(getIdpName(userTree)));
+ groupPrincipals.addAll(autoMembershipPrincipals.getPrincipals(getIdpName(userTree)));
return groupPrincipals;
}
}
@@ -453,59 +451,4 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
return null;
}
}
-
- private final class AutoMembershipPrincipals {
-
- private final Map<String, String[]> autoMembershipMapping;
- private final Map<String, Set<Principal>> principalMap;
-
- private AutoMembershipPrincipals(@NotNull Map<String, String[]> autoMembershipMapping) {
- this.autoMembershipMapping = autoMembershipMapping;
- this.principalMap = new ConcurrentHashMap<>(autoMembershipMapping.size());
- }
-
- @NotNull
- private Collection<Principal> get(@Nullable String idpName) {
- if (idpName == null) {
- return ImmutableSet.of();
- }
-
- Set<Principal> principals;
- if (!principalMap.containsKey(idpName)) {
- principals = collectAutomembershipPrincipals(idpName);
- principalMap.put(idpName, principals);
- } else {
- principals = principalMap.get(idpName);
- }
- return principals;
- }
-
- @NotNull
- private Set<Principal> collectAutomembershipPrincipals(@NotNull String idpName) {
- String[] vs = autoMembershipMapping.get(idpName);
- if (vs == null) {
- return ImmutableSet.of();
- }
-
- ImmutableSet.Builder<Principal> builder = ImmutableSet.builder();
- for (String groupId : vs) {
- try {
- Authorizable gr = userManager.getAuthorizable(groupId);
- if (gr != null && gr.isGroup()) {
- Principal grPrincipal = gr.getPrincipal();
- if (GroupPrincipals.isGroup(grPrincipal)) {
- builder.add(grPrincipal);
- } else {
- log.warn("Principal of group {} is not of group type -> Ignoring", groupId);
- }
- } else {
- log.warn("Configured auto-membership group {} does not exist -> Ignoring", groupId);
- }
- } catch (RepositoryException e) {
- log.debug("Failed to retrieved 'auto-membership' group with id {}", groupId, e);
- }
- }
- return builder.build();
- }
- }
}
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AbstractAutoMembershipTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AbstractAutoMembershipTest.java
new file mode 100644
index 0000000..d2e4311
--- /dev/null
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AbstractAutoMembershipTest.java
@@ -0,0 +1,54 @@
+package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.AbstractExternalAuthTest;
+import org.junit.After;
+import org.junit.Before;
+
+import java.util.Map;
+
+public abstract class AbstractAutoMembershipTest extends AbstractExternalAuthTest {
+
+ static final String IDP_VALID_AM = "idp1";
+ static final String IDP_INVALID_AM = "idp2";
+ static final String IDP_MIXED_AM = "idp3";
+
+ static final String AUTOMEMBERSHIP_GROUP_ID_1 = "autoMembershipGroupId_1";
+ static final String AUTOMEMBERSHIP_GROUP_ID_2 = "autoMembershipGroupId_2";
+ static final String NON_EXISTING_GROUP_ID = "nonExistingGroupId";
+
+ static final Map<String, String[]> MAPPING = ImmutableMap.of(
+ IDP_VALID_AM, new String[] {AUTOMEMBERSHIP_GROUP_ID_1, AUTOMEMBERSHIP_GROUP_ID_2},
+ IDP_INVALID_AM, new String[] {NON_EXISTING_GROUP_ID},
+ IDP_MIXED_AM, new String[] {AUTOMEMBERSHIP_GROUP_ID_1, NON_EXISTING_GROUP_ID});
+
+ UserManager userManager;
+ Group automembershipGroup1;
+ Group automembershipGroup2;
+
+ @Before
+ public void before() throws Exception {
+ super.before();
+ userManager = getUserManager(root);
+ automembershipGroup1 = userManager.createGroup(AUTOMEMBERSHIP_GROUP_ID_1);
+ automembershipGroup2 = userManager.createGroup(AUTOMEMBERSHIP_GROUP_ID_2);
+ root.commit();
+ }
+
+ @After
+ public void after() throws Exception {
+ try {
+ if (automembershipGroup1 != null) {
+ automembershipGroup1.remove();
+ }
+ if (automembershipGroup2 != null) {
+ automembershipGroup2.remove();
+ }
+ root.commit();
+ } finally {
+ super.after();
+ }
+ }
+}
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipalsTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipalsTest.java
new file mode 100644
index 0000000..ad48687
--- /dev/null
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipalsTest.java
@@ -0,0 +1,96 @@
+/*
+ * 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.principal;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.jackrabbit.api.security.user.UserManager;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.jcr.RepositoryException;
+import java.security.Principal;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+public class AutoMembershipPrincipalsTest extends AbstractAutoMembershipTest {
+
+ private AutoMembershipPrincipals amp;
+
+ @Before
+ public void before() throws Exception {
+ super.before();
+ amp = new AutoMembershipPrincipals(userManager, MAPPING);
+ }
+
+ @Test
+ public void testGetPrincipalsUnknownIdp() {
+ assertTrue(amp.getPrincipals("unknown").isEmpty());
+ }
+
+ @Test
+ public void testGetPrincipalsUnknownGroup() {
+ Collection<Principal> principals = amp.getPrincipals(IDP_INVALID_AM);
+ assertTrue(principals.isEmpty());
+ }
+
+ @Test
+ public void testGetPrincipalsMultipleGroups() throws Exception {
+ Collection<Principal> principals = amp.getPrincipals(IDP_VALID_AM);
+ assertFalse(principals.isEmpty());
+ assertEquals(ImmutableSet.of(automembershipGroup1.getPrincipal(), automembershipGroup2.getPrincipal()), ImmutableSet.copyOf(principals));
+ }
+
+ @Test
+ public void testGetPrincipalsMixed() throws Exception {
+ Collection<Principal> principals = amp.getPrincipals(IDP_MIXED_AM);
+ assertFalse(principals.isEmpty());
+ assertEquals(ImmutableSet.of(automembershipGroup1.getPrincipal()), ImmutableSet.copyOf(principals));
+ }
+
+ @Test
+ public void testGroupLookupFails() throws Exception {
+ UserManager um = spy(userManager);
+ when(um.getAuthorizable(anyString())).thenThrow(new RepositoryException());
+
+ AutoMembershipPrincipals amprincipals = new AutoMembershipPrincipals(um, MAPPING);
+ assertTrue(amprincipals.getPrincipals(IDP_VALID_AM).isEmpty());
+ }
+
+ @Test
+ public void testEmptyMapping() {
+ Map<String, String[]> m = spy(new HashMap<>());
+ AutoMembershipPrincipals amprincipals = new AutoMembershipPrincipals(userManager, m);
+
+ assertTrue(amprincipals.getPrincipals(null).isEmpty());
+ assertTrue(amprincipals.getPrincipals(IDP_VALID_AM).isEmpty());
+
+ verify(m, times(1)).size();
+ verify(m, times(1)).get(anyString());
+ verifyNoMoreInteractions(m);
+ }
+}
\ No newline at end of file