You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2021/05/19 16:12:30 UTC

[jackrabbit-filevault] 01/01: JCRVLT-515 evaluate bound principals in addition to user ids

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch feature/JCRVLT-515-evaluate-bound-principals
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git

commit 6faaf38099e9d7b5d14204fcc3c0b750ee441371
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Wed May 5 09:33:32 2021 +0200

    JCRVLT-515 evaluate bound principals in addition to user ids
---
 vault-core/pom.xml                                 |   6 ++
 .../packaging/impl/AdminPermissionChecker.java     | 100 ++++++++++++++++-----
 .../vault/packaging/impl/PackagingImpl.java        |   4 +-
 .../packaging/impl/AdminPermissionCheckerIT.java   |  39 +++++++-
 4 files changed, 119 insertions(+), 30 deletions(-)

diff --git a/vault-core/pom.xml b/vault-core/pom.xml
index 4466650..c00bc5d 100644
--- a/vault-core/pom.xml
+++ b/vault-core/pom.xml
@@ -207,6 +207,12 @@
             <optional>true</optional>
         </dependency>
 
+        <!-- for parsing Maven versions -->
+        <dependency>
+            <groupId>org.apache.maven</groupId>
+            <artifactId>maven-artifact</artifactId>
+            <version>3.8.1</version>
+        </dependency>
         <!-- test deps -->
         <dependency>
             <groupId>junit</groupId>
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/AdminPermissionChecker.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/AdminPermissionChecker.java
index 6cdda54..9a879e2 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/AdminPermissionChecker.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/AdminPermissionChecker.java
@@ -17,24 +17,27 @@
 
 package org.apache.jackrabbit.vault.packaging.impl;
 
+import java.security.Principal;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
+import javax.jcr.Repository;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 
 import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.maven.artifact.versioning.ComparableVersion;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Utility class to check if a user/session has administrative permissions (e.g. to check if a package can be installed)
+ * Utility class to check if a session has administrative permissions (e.g. to check if a package can be installed)
  */
 public class AdminPermissionChecker {
 
@@ -43,6 +46,12 @@ public class AdminPermissionChecker {
     private static final String SYSTEM_USER = "system";
     private static final String ADMINISTRATORS_GROUP = "administrators";
 
+    public static final ComparableVersion VERSION_OAK_140 = new ComparableVersion("1.40");
+
+    private AdminPermissionChecker() {
+        // static methods only
+    }
+
     /**
      * Checks if the user who opened the session has administrative permissions
      *
@@ -50,37 +59,80 @@ public class AdminPermissionChecker {
      * @return whether the passed session is an admin session
      * @throws RepositoryException If an error occurrs.
      */
-    public static boolean hasAdministrativePermissions(@NotNull Session session, String... additionalAdminAuthorizableIds) throws RepositoryException {
+    public static boolean hasAdministrativePermissions(@NotNull Session session, String... additionalAdminAuthorizableIdsOrPrincipalNames) throws RepositoryException {
+        List<String> additionalAdminIdsOrPrincipalNames = Arrays.asList(Optional.ofNullable(additionalAdminAuthorizableIdsOrPrincipalNames).orElse(new String[0]));
+        final JackrabbitSession jackrabbitSession;
+        if (session instanceof JackrabbitSession) {
+            jackrabbitSession = (JackrabbitSession) session;
+            if (isOakVersionExposingBoundPrincipals(session.getRepository())) {
+                if (hasAdministrativePermissionsWithPrincipals(jackrabbitSession, additionalAdminIdsOrPrincipalNames)) {
+                    return true;
+                }
+            }
+        } else {
+            jackrabbitSession = null;
+        }
+        // then evaluate user id
         String userId = session.getUserID();
-        if (ADMIN_USER.equals(userId) || SYSTEM_USER.equals(userId)) {
+        if (hasAdministrativePermissionsWithAuthorizableId(userId, additionalAdminIdsOrPrincipalNames)) {
             return true;
         }
-        List<String> additionalAdminIds = Arrays.asList(Optional.ofNullable(additionalAdminAuthorizableIds).orElse(new String[0]));
-        if (additionalAdminIds.contains(userId)) {
+        if (jackrabbitSession != null) {
+            Authorizable authorizable = jackrabbitSession.getUserManager().getAuthorizable(userId);
+            if (authorizable == null) {
+                return false;
+            }
+    
+            Iterator<Group> groupIterator = authorizable.memberOf();
+            while (groupIterator.hasNext()) {
+                String groupId = groupIterator.next().getID();
+                if (hasAdministrativePermissionsWithAuthorizableId(groupId, additionalAdminIdsOrPrincipalNames)) {
+                    return true;
+                }
+            }
+        } else {
+            log.warn("could not evaluate group permissions but just user name");
+        }
+        return false;
+    }
+    
+    static boolean hasAdministrativePermissionsWithPrincipals(@NotNull Session session, List<String> additionalAdminPrincipalNames) {
+        Set<Principal> boundPrincipals = (Set<Principal>)session.getAttribute("oak.bound-principals");
+        if (boundPrincipals != null) {
+            for (Principal principal : boundPrincipals) {
+                if (additionalAdminPrincipalNames.contains(principal.getName())) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    static boolean hasAdministrativePermissionsWithAuthorizableId(@NotNull String authorizableId, List<String> additionalAdminIds) {
+        if (ADMIN_USER.equals(authorizableId) || SYSTEM_USER.equals(authorizableId) || ADMINISTRATORS_GROUP.equals(authorizableId)) {
             return true;
         }
-        if (!(session instanceof JackrabbitSession)) {
-            log.warn("could not evaluate group permissions but just user name");
-            return false;
+        if (additionalAdminIds.contains(authorizableId)) {
+            return true;
         }
+        return false;
+    }
 
-        JackrabbitSession jackrabbitSession = (JackrabbitSession) session;
-        Authorizable authorizable = jackrabbitSession.getUserManager().getAuthorizable(userId);
-        if (authorizable == null) {
+    /**
+     * Checks if the repository is Oak 1.40 or newer. Compare with <a href="https://issues.apache.org/jira/browse/OAK-9415">OAK-9415</a>.
+     * @param session
+     * @return {@code true} if Oak repository >= 1.40.0 is used, otherwise {@code false}
+     */
+    static boolean isOakVersionExposingBoundPrincipals(@NotNull Repository repository) {
+        // first check repository
+        if (!"Apache Jackrabbit Oak".equals(repository.getDescriptor(Repository.REP_NAME_DESC))) {
             return false;
         }
-
-        Iterator<Group> groupIterator = authorizable.memberOf();
-        while (groupIterator.hasNext()) {
-            String groupId = groupIterator.next().getID();
-            if (ADMINISTRATORS_GROUP.equals(groupId)) {
-                return true;
-            }
-            if (additionalAdminIds.contains(groupId)) {
-                return true;
-            }
+        String version = repository.getDescriptor(Repository.REP_VERSION_DESC);
+        if (version == null) {
+            return false;
         }
-
-        return false;
+        // parse version according to Maven standards
+        return new ComparableVersion(version).compareTo(VERSION_OAK_140) >= 0;
     }
 }
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackagingImpl.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackagingImpl.java
index 6de235c..80250e7 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackagingImpl.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackagingImpl.java
@@ -94,10 +94,10 @@ public class PackagingImpl implements Packaging {
         @AttributeDefinition(description = "The locations in the repository which are used by the package manager")
         String[] packageRoots() default {"/etc/packages"};
         
-        @AttributeDefinition(description = "The authorizable ids which are allowed to execute hooks (in addition to 'admin', 'administrators' and 'system'")
+        @AttributeDefinition(description = "The authorizable ids or principal names which are allowed to execute hooks (in addition to 'admin', 'administrators' and 'system'")
         String[] authIdsForHookExecution();
         
-        @AttributeDefinition(description = "The authorizable ids which are allowed to install packages with the 'requireRoot' flag (in addition to 'admin', 'administrators' and 'system'")
+        @AttributeDefinition(description = "The authorizable ids or principal names which are allowed to install packages with the 'requireRoot' flag (in addition to 'admin', 'administrators' and 'system'")
         String[] authIdsForRootInstallation();
         
         @AttributeDefinition(description = "The default value for strict imports (i.e. whether it just logs certain errors or always throws exceptions")
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/impl/AdminPermissionCheckerIT.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/impl/AdminPermissionCheckerIT.java
index e04c3db..0a9d7bc 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/impl/AdminPermissionCheckerIT.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/impl/AdminPermissionCheckerIT.java
@@ -17,7 +17,17 @@
 
 package org.apache.jackrabbit.vault.packaging.impl;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.security.Principal;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
 import javax.jcr.AccessDeniedException;
+import javax.jcr.LoginException;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.SimpleCredentials;
@@ -27,14 +37,11 @@ import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.vault.packaging.integration.IntegrationTestBase;
 import org.junit.After;
 import org.junit.Test;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-
 /**
  * Testcase for {@link AdminPermissionChecker}
  */
@@ -143,4 +150,28 @@ public class AdminPermissionCheckerIT extends IntegrationTestBase {
             session.logout();
         }
     }
+
+    @Test
+    public void testBoundPrincipalIsAdmin() throws LoginException, RepositoryException {
+        assertFalse(AdminPermissionChecker.hasAdministrativePermissionsWithPrincipals(admin, Collections.singletonList("myadmin")));
+        // emulate Oak 1.40 with custom attribute
+        SimpleCredentials creds = new SimpleCredentials("admin", "admin".toCharArray());
+        Set<Principal> principals = new HashSet<>();
+        principals.add(new PrincipalImpl("someprincipalname"));
+        principals.add(new PrincipalImpl("myadmin"));
+        creds.setAttribute("oak.bound-principals", principals);
+        Session newSession = repository.login(creds);
+        try {
+            assertFalse(AdminPermissionChecker.hasAdministrativePermissionsWithPrincipals(newSession, Collections.singletonList("myadmin2")));
+            assertTrue(AdminPermissionChecker.hasAdministrativePermissionsWithPrincipals(newSession, Collections.singletonList("myadmin")));
+        } finally {
+            newSession.logout();
+        }
+    }
+
+    @Test
+    public void testIsOak140() {
+        // this test must be adjusted once the Oak dependency is updated to 1.40 or newer
+        assertFalse(AdminPermissionChecker.isOakVersionExposingBoundPrincipals(repository));
+    }
 }