From 719dbafd1339702a170f04ebbc4f20e80d45e8c9 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 14 Oct 2021 15:07:14 +0200 Subject: Add support for Delegation Settings for more apps * This adds support for the sharing, groupware, theming and user_ldap app * This adds some code who disapeared during a rebase in the initial delegation PR (provisioning_api) Signed-off-by: Carl Schwan --- .../lib/Controller/AppConfigController.php | 66 +++++++++++++++++++++- .../lib/Controller/GroupsController.php | 3 +- .../lib/Middleware/ProvisioningApiMiddleware.php | 3 +- .../tests/Controller/AppConfigControllerTest.php | 39 ++++++++++++- .../Middleware/ProvisioningApiMiddlewareTest.php | 34 +++++++---- 5 files changed, 131 insertions(+), 14 deletions(-) (limited to 'apps/provisioning_api') diff --git a/apps/provisioning_api/lib/Controller/AppConfigController.php b/apps/provisioning_api/lib/Controller/AppConfigController.php index cd8dba9e5b8..12b6a7d1370 100644 --- a/apps/provisioning_api/lib/Controller/AppConfigController.php +++ b/apps/provisioning_api/lib/Controller/AppConfigController.php @@ -26,12 +26,19 @@ declare(strict_types=1); */ namespace OCA\Provisioning_API\Controller; +use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCSController; use OCP\IAppConfig; use OCP\IConfig; +use OCP\IGroupManager; +use OCP\IL10N; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; +use OCP\Settings\IDelegatedSettings; +use OCP\Settings\IManager; class AppConfigController extends OCSController { @@ -41,6 +48,18 @@ class AppConfigController extends OCSController { /** @var IAppConfig */ protected $appConfig; + /** @var IUserSession */ + private $userSession; + + /** @var IL10N */ + private $l10n; + + /** @var IGroupManager */ + private $groupManager; + + /** @var IManager */ + private $settingManager; + /** * @param string $appName * @param IRequest $request @@ -50,10 +69,18 @@ class AppConfigController extends OCSController { public function __construct(string $appName, IRequest $request, IConfig $config, - IAppConfig $appConfig) { + IAppConfig $appConfig, + IUserSession $userSession, + IL10N $l10n, + IGroupManager $groupManager, + IManager $settingManager) { parent::__construct($appName, $request); $this->config = $config; $this->appConfig = $appConfig; + $this->userSession = $userSession; + $this->l10n = $l10n; + $this->groupManager = $groupManager; + $this->settingManager = $settingManager; } /** @@ -99,12 +126,23 @@ class AppConfigController extends OCSController { /** * @PasswordConfirmationRequired + * @NoSubAdminRequired + * @NoAdminRequired * @param string $app * @param string $key * @param string $value * @return DataResponse */ public function setValue(string $app, string $key, string $value): DataResponse { + $user = $this->userSession->getUser(); + if ($user === null) { + throw new \Exception("User is not logged in."); // Should not happen, since method is guarded by middleware + } + + if (!$this->isAllowedToChangedKey($user, $app, $key)) { + throw new NotAdminException($this->l10n->t('Logged in user must be an admin or have authorization to edit this setting.')); + } + try { $this->verifyAppId($app); $this->verifyConfigKey($app, $key, $value); @@ -170,4 +208,30 @@ class AppConfigController extends OCSController { throw new \InvalidArgumentException('The given key can not be set, unlimited quota is forbidden on this instance'); } } + + private function isAllowedToChangedKey(IUser $user, string $app, string $key): bool { + // Admin right verification + $isAdmin = $this->groupManager->isAdmin($user->getUID()); + if ($isAdmin) { + return true; + } + + $settings = $this->settingManager->getAllAllowedAdminSettings($user); + foreach ($settings as $setting) { + if (!($setting instanceof IDelegatedSettings)) { + continue; + } + $allowedKeys = $setting->getAuthorizedAppConfig(); + if (!array_key_exists($app, $allowedKeys)) { + continue; + } + foreach ($allowedKeys[$app] as $regex) { + if ($regex === $key + || (str_starts_with($regex, '/') && preg_match($regex, $key) === 1)) { + return true; + } + } + } + return false; + } } diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index 7b6e5319c2a..e7e2a666b7b 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -96,9 +96,10 @@ class GroupsController extends AUserData { } /** - * returns a list of groups details with ids and displaynames + * Returns a list of groups details with ids and displaynames * * @NoAdminRequired + * @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Sharing) * * @param string $search * @param int $limit diff --git a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php index 8dbee85360b..02fd0469513 100644 --- a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php +++ b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php @@ -70,7 +70,8 @@ class ProvisioningApiMiddleware extends Middleware { * @throws NotSubAdminException */ public function beforeController($controller, $methodName) { - if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin) { + // If AuthorizedAdminSetting, the check will be done in the SecurityMiddleware + if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin && !$this->reflector->hasAnnotation('AuthorizedAdminSetting')) { throw new NotSubAdminException(); } } diff --git a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php index 3abc46dc5fc..f55f842debc 100644 --- a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php +++ b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php @@ -30,7 +30,12 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\IAppConfig; use OCP\IConfig; +use OCP\IGroupManager; +use OCP\IL10N; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; +use OCP\Settings\IManager; use Test\TestCase; /** @@ -44,12 +49,24 @@ class AppConfigControllerTest extends TestCase { private $config; /** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */ private $appConfig; + /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ + private $userSession; + /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ + private $l10n; + /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ + private $settingManager; + /** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */ + private $groupManager; protected function setUp(): void { parent::setUp(); $this->config = $this->createMock(IConfig::class); $this->appConfig = $this->createMock(IAppConfig::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->l10n = $this->createMock(IL10N::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->settingManager = $this->createMock(IManager::class); } /** @@ -64,7 +81,11 @@ class AppConfigControllerTest extends TestCase { 'provisioning_api', $request, $this->config, - $this->appConfig + $this->appConfig, + $this->userSession, + $this->l10n, + $this->groupManager, + $this->settingManager ); } else { return $this->getMockBuilder(AppConfigController::class) @@ -73,6 +94,10 @@ class AppConfigControllerTest extends TestCase { $request, $this->config, $this->appConfig, + $this->userSession, + $this->l10n, + $this->groupManager, + $this->settingManager ]) ->setMethods($methods) ->getMock(); @@ -200,6 +225,18 @@ class AppConfigControllerTest extends TestCase { * @param int $status */ public function testSetValue($app, $key, $value, $appThrows, $keyThrows, $status) { + $adminUser = $this->createMock(IUser::class); + $adminUser->expects($this->once()) + ->method('getUid') + ->willReturn('admin'); + + $this->userSession->expects($this->once()) + ->method('getUser') + ->willReturn($adminUser); + $this->groupManager->expects($this->once()) + ->method('isAdmin') + ->with('admin') + ->willReturn(true); $api = $this->getInstance(['verifyAppId', 'verifyConfigKey']); if ($appThrows instanceof \Exception) { $api->expects($this->once()) diff --git a/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php b/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php index ea3d44cc907..37ab03f758c 100644 --- a/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php +++ b/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php @@ -45,13 +45,20 @@ class ProvisioningApiMiddlewareTest extends TestCase { public function dataAnnotation() { return [ - [false, false, false, false], - [false, false, true, false], - [false, true, true, false], - [ true, false, false, true], - [ true, false, true, false], - [ true, true, false, false], - [ true, true, true, false], + [false, false, false, false, false], + [false, false, true, false, false], + [false, true, true, false, false], + [ true, false, false, false, true], + [ true, false, true, false, false], + [ true, true, false, false, false], + [ true, true, true, false, false], + [false, false, false, true, false], + [false, false, true, true, false], + [false, true, true, true, false], + [ true, false, false, true, false], + [ true, false, true, true, false], + [ true, true, false, true, false], + [ true, true, true, true, false], ]; } @@ -63,7 +70,7 @@ class ProvisioningApiMiddlewareTest extends TestCase { * @param bool $isSubAdmin * @param bool $shouldThrowException */ - public function testBeforeController($subadminRequired, $isAdmin, $isSubAdmin, $shouldThrowException) { + public function testBeforeController($subadminRequired, $isAdmin, $isSubAdmin, $hasSettingAuthorizationAnnotation, $shouldThrowException) { $middleware = new ProvisioningApiMiddleware( $this->reflector, $isAdmin, @@ -71,8 +78,15 @@ class ProvisioningApiMiddlewareTest extends TestCase { ); $this->reflector->method('hasAnnotation') - ->with('NoSubAdminRequired') - ->willReturn(!$subadminRequired); + ->willReturnCallback(function ($annotation) use ($subadminRequired, $hasSettingAuthorizationAnnotation) { + if ($annotation === 'NoSubAdminRequired') { + return !$subadminRequired; + } + if ($annotation === 'AuthorizedAdminSetting') { + return $hasSettingAuthorizationAnnotation; + } + return false; + }); try { $middleware->beforeController( -- cgit v1.2.3