From 6a1c6ecec6e39ae1f66c9750d5136cf83faa6417 Mon Sep 17 00:00:00 2001
From: bergice <andre.kiste@silverstripe.com>
Date: Mon, 16 Sep 2019 17:41:21 +1200
Subject: [PATCH] Fix administrators not being able to see files that are
 restricted to groups

Resolves https://github.com/silverstripe/silverstripe-asset-admin/issues/777
---
 src/Security/InheritedPermissions.php         | 34 +++++++++++--------
 .../php/Security/InheritedPermissionsTest.php |  5 +++
 .../php/Security/InheritedPermissionsTest.yml |  8 +++++
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/src/Security/InheritedPermissions.php b/src/Security/InheritedPermissions.php
index d320793f6..544f32993 100644
--- a/src/Security/InheritedPermissions.php
+++ b/src/Security/InheritedPermissions.php
@@ -357,21 +357,25 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher
         $baseTable = DataObject::getSchema()->baseDataTable($this->getBaseClass());
 
         if ($member && $member->ID) {
-            // Determine if this member matches any of the group or other rules
-            $groupJoinTable = $this->getJoinTable($type);
-            $uninheritedPermissions = $stageRecords
-                ->where([
-                    "(\"$typeField\" IN (?, ?) OR " . "(\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL))"
-                    => [
-                        self::ANYONE,
-                        self::LOGGED_IN_USERS,
-                        self::ONLY_THESE_USERS
-                    ]
-                ])
-                ->leftJoin(
-                    $groupJoinTable,
-                    "\"$groupJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " . "\"$groupJoinTable\".\"GroupID\" IN ($groupIDsSQLList)"
-                )->column('ID');
+            if (!Permission::checkMember($member, 'ADMIN')) {
+                // Determine if this member matches any of the group or other rules
+                $groupJoinTable = $this->getJoinTable($type);
+                $uninheritedPermissions = $stageRecords
+                    ->where([
+                        "(\"$typeField\" IN (?, ?) OR " . "(\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL))"
+                        => [
+                            self::ANYONE,
+                            self::LOGGED_IN_USERS,
+                            self::ONLY_THESE_USERS
+                        ]
+                    ])
+                    ->leftJoin(
+                        $groupJoinTable,
+                        "\"$groupJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " . "\"$groupJoinTable\".\"GroupID\" IN ($groupIDsSQLList)"
+                    )->column('ID');
+            } else {
+                $uninheritedPermissions = $stageRecords->column('ID');
+            }
         } else {
             // Only view pages with ViewType = Anyone if not logged in
             $uninheritedPermissions = $stageRecords
diff --git a/tests/php/Security/InheritedPermissionsTest.php b/tests/php/Security/InheritedPermissionsTest.php
index f5a0804d2..5aa32de65 100644
--- a/tests/php/Security/InheritedPermissionsTest.php
+++ b/tests/php/Security/InheritedPermissionsTest.php
@@ -151,6 +151,8 @@ class InheritedPermissionsTest extends SapphireTest
         $protected = $this->objFromFixture(TestPermissionNode::class, 'protected');
         $protectedChild = $this->objFromFixture(TestPermissionNode::class, 'protected-child');
         $editor = $this->objFromFixture(Member::class, 'editor');
+        $restricted = $this->objFromFixture(TestPermissionNode::class, 'restricted-page');
+        $admin = $this->objFromFixture(Member::class, 'admin');
 
         // Not logged in user can only access Inherit or Anyone pages
         Member::actAs(
@@ -182,6 +184,9 @@ class InheritedPermissionsTest extends SapphireTest
         $this->rootPermissions->setCanView(false);
 
         $this->assertFalse($history->canView($editor));
+
+        // Ensure admins can view everything, even if only a certain group is allowed to view it
+        $this->assertTrue($restricted->canView($admin));
     }
 
     public function testUnstagedViewPermissions()
diff --git a/tests/php/Security/InheritedPermissionsTest.yml b/tests/php/Security/InheritedPermissionsTest.yml
index 19fa1cd3a..92141c361 100644
--- a/tests/php/Security/InheritedPermissionsTest.yml
+++ b/tests/php/Security/InheritedPermissionsTest.yml
@@ -100,6 +100,10 @@ SilverStripe\Security\Tests\InheritedPermissionsTest\TestPermissionNode:
     Title: Child
     CanViewType: Inherit
     Parent: =>SilverStripe\Security\Tests\InheritedPermissionsTest\TestPermissionNode.protected
+  restricted-page:
+    Title: Restricted Page
+    CanViewType: OnlyTheseUsers
+    ViewerGroups: =>SilverStripe\Security\Group.allsections
 
 SilverStripe\Security\Tests\InheritedPermissionsTest\UnstagedNode:
   about:
@@ -167,3 +171,7 @@ SilverStripe\Security\Tests\InheritedPermissionsTest\UnstagedNode:
     Title: Child
     CanViewType: Inherit
     Parent: =>SilverStripe\Security\Tests\InheritedPermissionsTest\UnstagedNode.protected
+  restricted-page:
+    Title: Restricted Page
+    CanViewType: OnlyTheseUsers
+    ViewerGroups: =>SilverStripe\Security\Group.allsections