diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 5b894e69c65..f4737468716 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -87,9 +87,7 @@ use OCP\Defaults; use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; -use OCP\IGroupManager; use OCP\IServerContainer; -use OCP\IUserSession; use OCP\Settings\Events\DeclarativeSettingsGetValueEvent; use OCP\Settings\Events\DeclarativeSettingsSetValueEvent; use OCP\Settings\IManager; @@ -133,15 +131,6 @@ class Application extends App implements IBootstrap { /** * Core class wrappers */ - /** FIXME: Remove once OC_SubAdmin is non-static and mockable */ - $context->registerService('isSubAdmin', function () { - $userObject = \OCP\Server::get(IUserSession::class)->getUser(); - $isSubAdmin = false; - if ($userObject !== null) { - $isSubAdmin = \OCP\Server::get(IGroupManager::class)->getSubAdmin()->isSubAdmin($userObject); - } - return $isSubAdmin; - }); $context->registerService(IProvider::class, function (IAppContainer $appContainer) { /** @var IServerContainer $serverContainer */ $serverContainer = $appContainer->query(IServerContainer::class); diff --git a/apps/settings/lib/Middleware/SubadminMiddleware.php b/apps/settings/lib/Middleware/SubadminMiddleware.php index fe92661d24e..34f5b82f5f2 100644 --- a/apps/settings/lib/Middleware/SubadminMiddleware.php +++ b/apps/settings/lib/Middleware/SubadminMiddleware.php @@ -1,9 +1,13 @@ reflector = $reflector; + } + + private function isSubAdmin(): bool { + $userObject = $this->userSession->getUser(); + if ($userObject === null) { + return false; + } + return $this->subAdminManager->isSubAdmin($userObject); } /** @@ -43,7 +49,7 @@ class SubadminMiddleware extends Middleware { */ public function beforeController($controller, $methodName) { if (!$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->reflector->hasAnnotation('AuthorizedAdminSetting')) { - if (!$this->isSubAdmin) { + if (!$this->isSubAdmin()) { throw new NotAdminException($this->l10n->t('Logged in account must be a subadmin')); } } diff --git a/apps/settings/tests/Middleware/SubadminMiddlewareTest.php b/apps/settings/tests/Middleware/SubadminMiddlewareTest.php index dc725e2d377..2992810af6c 100644 --- a/apps/settings/tests/Middleware/SubadminMiddlewareTest.php +++ b/apps/settings/tests/Middleware/SubadminMiddlewareTest.php @@ -1,9 +1,13 @@ reflector = $this->getMockBuilder(ControllerMethodReflector::class) ->disableOriginalConstructor()->getMock(); - $this->controller = $this->getMockBuilder(Controller::class) - ->disableOriginalConstructor()->getMock(); + $this->userSession = $this->createMock(IUserSession::class); + $this->subAdminManager = $this->createMock(ISubAdmin::class); $this->l10n = $this->createMock(IL10N::class); - $this->subadminMiddlewareAsSubAdmin = new SubadminMiddleware($this->reflector, true, $this->l10n); - $this->subadminMiddleware = new SubadminMiddleware($this->reflector, false, $this->l10n); + $this->subadminMiddleware = new SubadminMiddleware( + $this->reflector, + $this->userSession, + $this->subAdminManager, + $this->l10n, + ); + + $this->controller = $this->getMockBuilder(Controller::class) + ->disableOriginalConstructor()->getMock(); + + $this->userSession + ->expects(self::any()) + ->method('getUser') + ->willReturn($this->createMock(IUser::class)); } - public function testBeforeControllerAsUserWithExemption(): void { + public function testBeforeControllerAsUserWithoutAnnotation(): void { $this->expectException(NotAdminException::class); $this->reflector @@ -54,20 +71,31 @@ class SubadminMiddlewareTest extends \Test\TestCase { ['NoSubAdminRequired'], ['AuthorizedAdminSetting'], )->willReturn(false); + + $this->subAdminManager + ->expects(self::once()) + ->method('isSubAdmin') + ->willReturn(false); + $this->subadminMiddleware->beforeController($this->controller, 'foo'); } - public function testBeforeControllerAsUserWithoutExemption(): void { + public function testBeforeControllerWithAnnotation(): void { $this->reflector ->expects($this->once()) ->method('hasAnnotation') ->with('NoSubAdminRequired') ->willReturn(true); + + $this->subAdminManager + ->expects(self::never()) + ->method('isSubAdmin'); + $this->subadminMiddleware->beforeController($this->controller, 'foo'); } - public function testBeforeControllerAsSubAdminWithoutExemption(): void { + public function testBeforeControllerAsSubAdminWithoutAnnotation(): void { $this->reflector ->expects($this->exactly(2)) ->method('hasAnnotation') @@ -75,16 +103,13 @@ class SubadminMiddlewareTest extends \Test\TestCase { ['NoSubAdminRequired'], ['AuthorizedAdminSetting'], )->willReturn(false); - $this->subadminMiddlewareAsSubAdmin->beforeController($this->controller, 'foo'); - } - public function testBeforeControllerAsSubAdminWithExemption(): void { - $this->reflector - ->expects($this->once()) - ->method('hasAnnotation') - ->with('NoSubAdminRequired') + $this->subAdminManager + ->expects(self::once()) + ->method('isSubAdmin') ->willReturn(true); - $this->subadminMiddlewareAsSubAdmin->beforeController($this->controller, 'foo'); + + $this->subadminMiddleware->beforeController($this->controller, 'foo'); } public function testAfterNotAdminException(): void {