Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Delegation Settings for more apps #29240

Merged
merged 1 commit into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/dav/lib/Controller/BirthdayCalendarController.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public function __construct($appName, IRequest $request,

/**
* @return Response
* @AuthorizedAdminSetting(settings=OCA\DAV\Settings\CalDAVSettings)
*/
public function enable() {
$this->config->setAppValue($this->appName, 'generateBirthdayCalendar', 'yes');
Expand All @@ -104,6 +105,7 @@ public function enable() {

/**
* @return Response
* @AuthorizedAdminSetting(settings=OCA\DAV\Settings\CalDAVSettings)
*/
public function disable() {
$this->config->setAppValue($this->appName, 'generateBirthdayCalendar', 'no');
Expand Down
29 changes: 20 additions & 9 deletions apps/dav/lib/Settings/CalDAVSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,23 @@
use OCP\AppFramework\Http\TemplateResponse;
use OCP\IConfig;
use OCP\AppFramework\Services\IInitialState;
use OCP\Settings\ISettings;
use OCP\Settings\IDelegatedSettings;

class CalDAVSettings implements ISettings {
class CalDAVSettings implements IDelegatedSettings {

/** @var IConfig */
private $config;

/** @var IInitialState */
private $initialState;

private const defaults = [
'sendInvitations' => 'yes',
'generateBirthdayCalendar' => 'yes',
'sendEventReminders' => 'yes',
'sendEventRemindersPush' => 'no',
];

/**
* CalDAVSettings constructor.
*
Expand All @@ -51,13 +58,7 @@ public function __construct(IConfig $config, IInitialState $initialState) {
}

public function getForm(): TemplateResponse {
$defaults = [
'sendInvitations' => 'yes',
'generateBirthdayCalendar' => 'yes',
'sendEventReminders' => 'yes',
'sendEventRemindersPush' => 'no',
];
foreach ($defaults as $key => $default) {
foreach (self::defaults as $key => $default) {
$value = $this->config->getAppValue(Application::APP_ID, $key, $default);
$this->initialState->provideInitialState($key, $value === 'yes');
}
Expand All @@ -77,4 +78,14 @@ public function getSection() {
public function getPriority() {
return 10;
}

public function getName(): ?string {
return null; // Only setting in this section
}

public function getAuthorizedAppConfig(): array {
return [
'dav' => ['/(' . implode('|', array_keys(self::defaults)) . ')/']
];
}
}
29 changes: 26 additions & 3 deletions apps/federatedfilesharing/lib/Settings/Admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,30 @@
use OCA\FederatedFileSharing\FederatedShareProvider;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\GlobalScale\IConfig;
use OCP\Settings\ISettings;
use OCP\IL10N;
use OCP\Settings\IDelegatedSettings;

class Admin implements ISettings {
class Admin implements IDelegatedSettings {

/** @var FederatedShareProvider */
private $fedShareProvider;

/** @var IConfig */
private $gsConfig;

/** @var IL10N */
private $l;

/**
* Admin constructor.
*
* @param FederatedShareProvider $fedShareProvider
* @param IConfig $globalScaleConfig
*/
public function __construct(FederatedShareProvider $fedShareProvider, IConfig $globalScaleConfig) {
public function __construct(FederatedShareProvider $fedShareProvider, IConfig $globalScaleConfig, IL10N $l) {
$this->fedShareProvider = $fedShareProvider;
$this->gsConfig = $globalScaleConfig;
$this->l = $l;
}

/**
Expand Down Expand Up @@ -83,4 +88,22 @@ public function getSection() {
public function getPriority() {
return 20;
}

public function getName(): ?string {
return $this->l->t('Federated Cloud Sharing');
}

public function getAuthorizedAppConfig(): array {
return [
'files_sharing' => [
'outgoing_server2server_share_enabled',
'incoming_server2server_share_enabled',
'federatedGroupSharingSupported',
'outgoingServer2serverGroupShareEnabled',
'incomingServer2serverGroupShareEnabled',
'lookupServerEnabled',
'lookupServerUploadEnabled',
],
];
}
}
4 changes: 3 additions & 1 deletion apps/federatedfilesharing/tests/Settings/AdminTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OCA\FederatedFileSharing\Settings\Admin;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\GlobalScale\IConfig;
use OCP\IL10N;
use Test\TestCase;

class AdminTest extends TestCase {
Expand All @@ -45,7 +46,8 @@ protected function setUp(): void {
$this->gsConfig = $this->createMock(IConfig::class);
$this->admin = new Admin(
$this->federatedShareProvider,
$this->gsConfig
$this->gsConfig,
$this->createMock(IL10N::class)
);
}

Expand Down
9 changes: 6 additions & 3 deletions apps/federation/lib/Controller/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ public function __construct($AppName,


/**
* add server to the list of trusted Nextclouds
* Add server to the list of trusted Nextclouds.
*
* @AuthorizedAdminSetting(settings=OCA\Federation\Settings\Admin)
* @param string $url
* @return DataResponse
* @throws HintException
Expand All @@ -76,8 +77,9 @@ public function addServer($url) {
}

/**
* add server to the list of trusted Nextclouds
* Add server to the list of trusted Nextclouds.
*
* @AuthorizedAdminSetting(settings=OCA\Federation\Settings\Admin)
* @param int $id
* @return DataResponse
*/
Expand All @@ -87,8 +89,9 @@ public function removeServer($id) {
}

/**
* check if the server should be added to the list of trusted servers or not
* Check if the server should be added to the list of trusted servers or not.
*
* @AuthorizedAdminSetting(settings=OCA\Federation\Settings\Admin)
* @param string $url
* @return bool
* @throws HintException
Expand Down
25 changes: 22 additions & 3 deletions apps/federation/lib/Settings/Admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,26 @@

use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\Settings\ISettings;
use OCP\IL10N;
use OCP\Settings\IDelegatedSettings;

class Admin implements ISettings {
class Admin implements IDelegatedSettings {

/** @var TrustedServers */
private $trustedServers;

public function __construct(TrustedServers $trustedServers) {
/** @var IL10N */
private $l;

/**
* Admin constructor.
*
* @param TrustedServers $trustedServers
* @param IL10N $l
*/
public function __construct(TrustedServers $trustedServers, IL10N $l) {
$this->trustedServers = $trustedServers;
$this->l = $l;
}

/**
Expand Down Expand Up @@ -63,4 +74,12 @@ public function getSection() {
public function getPriority() {
return 30;
}

public function getName(): ?string {
return $this->l->t("Trusted servers");
}

public function getAuthorizedAppConfig(): array {
return []; // Handled by custom controller
}
}
4 changes: 3 additions & 1 deletion apps/federation/tests/Settings/AdminTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use OCA\Federation\Settings\Admin;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\IL10N;
use Test\TestCase;

class AdminTest extends TestCase {
Expand All @@ -38,7 +39,8 @@ protected function setUp(): void {
parent::setUp();
$this->trustedServers = $this->getMockBuilder('\OCA\Federation\TrustedServers')->disableOriginalConstructor()->getMock();
$this->admin = new Admin(
$this->trustedServers
$this->trustedServers,
$this->createMock(IL10N::class)
);
}

Expand Down
66 changes: 65 additions & 1 deletion apps/provisioning_api/lib/Controller/AppConfigController.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,19 @@
*/
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 {

Expand All @@ -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
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -99,12 +126,23 @@ public function getValue(string $app, string $key, string $defaultValue = ''): D

/**
* @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);
Expand Down Expand Up @@ -170,4 +208,30 @@ protected function verifyConfigKey(string $app, string $key, string $value) {
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;
}
}
3 changes: 2 additions & 1 deletion apps/provisioning_api/lib/Controller/GroupsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,10 @@ public function getGroups(string $search = '', int $limit = null, int $offset =
}

/**
* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public function __construct(
* @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();
}
}
Expand Down
Loading