Skip to content

Commit

Permalink
Merge pull request #46816 from nextcloud/refactor/settings/security-a…
Browse files Browse the repository at this point in the history
…ttributes
  • Loading branch information
provokateurin authored Aug 14, 2024
2 parents 8cb4679 + f012c99 commit 4673e1a
Show file tree
Hide file tree
Showing 13 changed files with 91 additions and 75 deletions.
5 changes: 3 additions & 2 deletions apps/settings/lib/Controller/AISettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
*/
namespace OCA\Settings\Controller;

use OCA\Settings\Settings\Admin\ArtificialIntelligence;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
use OCP\AppFramework\Http\DataResponse;
use OCP\IConfig;
use OCP\IRequest;
Expand All @@ -31,11 +33,10 @@ public function __construct(
/**
* Sets the email settings
*
* @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\ArtificialIntelligence)
*
* @param array $settings
* @return DataResponse
*/
#[AuthorizedAdminSetting(settings: ArtificialIntelligence::class)]
public function update($settings) {
$keys = ['ai.stt_provider', 'ai.textprocessing_provider_preferences', 'ai.taskprocessing_provider_preferences', 'ai.translation_provider_preferences', 'ai.text2image_provider'];
foreach ($keys as $key) {
Expand Down
6 changes: 4 additions & 2 deletions apps/settings/lib/Controller/AdminSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Services\IInitialState;
Expand Down Expand Up @@ -46,12 +48,12 @@ public function __construct(
}

/**
* @NoCSRFRequired
* @NoAdminRequired
* @NoSubAdminRequired
* We are checking the permissions in the getSettings method. If there is no allowed
* settings for the given section. The user will be gretted by an error message.
*/
#[NoAdminRequired]
#[NoCSRFRequired]
public function index(string $section): TemplateResponse {
return $this->getIndexResponse('admin', $section);
}
Expand Down
28 changes: 12 additions & 16 deletions apps/settings/lib/Controller/AppSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\JSONResponse;
Expand Down Expand Up @@ -74,10 +77,9 @@ public function __construct(
}

/**
* @NoCSRFRequired
*
* @return TemplateResponse
*/
#[NoCSRFRequired]
public function viewApps(): TemplateResponse {
$this->navigationManager->setActiveEntry('core_apps');

Expand All @@ -100,23 +102,21 @@ public function viewApps(): TemplateResponse {

/**
* Get all active entries for the app discover section
*
* @NoCSRFRequired
*/
#[NoCSRFRequired]
public function getAppDiscoverJSON(): JSONResponse {
$data = $this->discoverFetcher->get(true);
return new JSONResponse($data);
}

/**
* @PublicPage
* @NoCSRFRequired
*
* Get a image for the app discover section - this is proxied for privacy and CSP reasons
*
* @param string $image
* @throws \Exception
*/
#[PublicPage]
#[NoCSRFRequired]
public function getAppDiscoverMedia(string $fileName): Response {
$etag = $this->discoverFetcher->getETag() ?? date('Y-m');
$folder = null;
Expand Down Expand Up @@ -455,12 +455,11 @@ private function getAppsForCategory($requestedCategory = ''): array {
}

/**
* @PasswordConfirmationRequired
*
* @param string $appId
* @param array $groups
* @return JSONResponse
*/
#[PasswordConfirmationRequired]
public function enableApp(string $appId, array $groups = []): JSONResponse {
return $this->enableApps([$appId], $groups);
}
Expand All @@ -470,11 +469,11 @@ public function enableApp(string $appId, array $groups = []): JSONResponse {
*
* apps will be enabled for specific groups only if $groups is defined
*
* @PasswordConfirmationRequired
* @param array $appIds
* @param array $groups
* @return JSONResponse
*/
#[PasswordConfirmationRequired]
public function enableApps(array $appIds, array $groups = []): JSONResponse {
try {
$updateRequired = false;
Expand Down Expand Up @@ -522,21 +521,19 @@ private function getGroupList(array $groups) {
}

/**
* @PasswordConfirmationRequired
*
* @param string $appId
* @return JSONResponse
*/
#[PasswordConfirmationRequired]
public function disableApp(string $appId): JSONResponse {
return $this->disableApps([$appId]);
}

/**
* @PasswordConfirmationRequired
*
* @param array $appIds
* @return JSONResponse
*/
#[PasswordConfirmationRequired]
public function disableApps(array $appIds): JSONResponse {
try {
foreach ($appIds as $appId) {
Expand All @@ -551,11 +548,10 @@ public function disableApps(array $appIds): JSONResponse {
}

/**
* @PasswordConfirmationRequired
*
* @param string $appId
* @return JSONResponse
*/
#[PasswordConfirmationRequired]
public function uninstallApp(string $appId): JSONResponse {
$appId = OC_App::cleanAppId($appId);
$result = $this->installer->removeApp($appId);
Expand Down
14 changes: 8 additions & 6 deletions apps/settings/lib/Controller/AuthSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
use OCP\Activity\IManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
use OCP\AppFramework\Http\JSONResponse;
use OCP\Authentication\Exceptions\ExpiredTokenException;
use OCP\Authentication\Exceptions\InvalidTokenException;
Expand Down Expand Up @@ -88,13 +90,13 @@ public function __construct(string $appName,
}

/**
* @NoAdminRequired
* @NoSubAdminRequired
* @PasswordConfirmationRequired
*
* @param string $name
* @return JSONResponse
*/
#[NoAdminRequired]
#[PasswordConfirmationRequired]
public function create($name) {
if ($this->checkAppToken()) {
return $this->getServiceNotAvailableResponse();
Expand Down Expand Up @@ -169,12 +171,12 @@ private function checkAppToken(): bool {
}

/**
* @NoAdminRequired
* @NoSubAdminRequired
*
* @param int $id
* @return array|JSONResponse
*/
#[NoAdminRequired]
public function destroy($id) {
if ($this->checkAppToken()) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
Expand All @@ -195,14 +197,14 @@ public function destroy($id) {
}

/**
* @NoAdminRequired
* @NoSubAdminRequired
*
* @param int $id
* @param array $scope
* @param string $name
* @return array|JSONResponse
*/
#[NoAdminRequired]
public function update($id, array $scope, string $name) {
if ($this->checkAppToken()) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
Expand Down Expand Up @@ -276,15 +278,15 @@ private function findTokenByIdAndUser(int $id): IToken {
}

/**
* @NoAdminRequired
* @NoSubAdminRequired
* @PasswordConfirmationRequired
*
* @param int $id
* @return JSONResponse
* @throws InvalidTokenException
* @throws ExpiredTokenException
*/
#[NoAdminRequired]
#[PasswordConfirmationRequired]
public function wipe(int $id): JSONResponse {
if ($this->checkAppToken()) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
Expand Down
13 changes: 7 additions & 6 deletions apps/settings/lib/Controller/ChangePasswordController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
use OC\User\Session;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Attribute\BruteForceProtection;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
use OCP\AppFramework\Http\JSONResponse;
use OCP\HintException;
use OCP\IGroupManager;
Expand Down Expand Up @@ -49,10 +52,10 @@ public function __construct(string $appName,
}

/**
* @NoAdminRequired
* @NoSubAdminRequired
* @BruteForceProtection(action=changePersonalPassword)
*/
#[NoAdminRequired]
#[BruteForceProtection(action: 'changePersonalPassword')]
public function changePersonalPassword(string $oldpassword = '', ?string $newpassword = null): JSONResponse {
$loginName = $this->userSession->getLoginName();
/** @var IUser $user */
Expand Down Expand Up @@ -97,10 +100,8 @@ public function changePersonalPassword(string $oldpassword = '', ?string $newpas
]);
}

/**
* @NoAdminRequired
* @PasswordConfirmationRequired
*/
#[NoAdminRequired]
#[PasswordConfirmationRequired]
public function changeUserPassword(?string $username = null, ?string $password = null, ?string $recoveryPassword = null): JSONResponse {
if ($username === null) {
return new JSONResponse([
Expand Down
20 changes: 11 additions & 9 deletions apps/settings/lib/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@

use OC\AppFramework\Http;
use OC\IntegrityCheck\Checker;
use OCA\Settings\Settings\Admin\Overview;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Http\DataResponse;
Expand Down Expand Up @@ -54,30 +58,28 @@ public function __construct($AppName,
}

/**
* @NoAdminRequired
* @NoCSRFRequired
* @return DataResponse
*/
#[NoCSRFRequired]
#[NoAdminRequired]
public function setupCheckManager(): DataResponse {
return new DataResponse($this->setupCheckManager->runAll());
}

/**
* @NoCSRFRequired
* @return RedirectResponse
* @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview)
*/
#[NoCSRFRequired]
#[AuthorizedAdminSetting(settings: Overview::class)]
public function rescanFailedIntegrityCheck(): RedirectResponse {
$this->checker->runInstanceVerification();
return new RedirectResponse(
$this->urlGenerator->linkToRoute('settings.AdminSettings.index', ['section' => 'overview'])
);
}

/**
* @NoCSRFRequired
* @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview)
*/
#[NoCSRFRequired]
#[AuthorizedAdminSetting(settings: Overview::class)]
public function getFailedIntegrityCheckFiles(): DataDisplayResponse {
if (!$this->checker->isCodeCheckEnforced()) {
return new DataDisplayResponse('Integrity checker has been disabled. Integrity cannot be verified.');
Expand Down Expand Up @@ -137,8 +139,8 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse {

/**
* @return DataResponse
* @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview)
*/
#[AuthorizedAdminSetting(settings: Overview::class)]
public function check() {
return new DataResponse(
[
Expand Down
6 changes: 4 additions & 2 deletions apps/settings/lib/Controller/HelpController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
namespace OCA\Settings\Controller;

use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\TemplateResponse;
Expand Down Expand Up @@ -65,10 +67,10 @@ public function __construct(
/**
* @return TemplateResponse
*
* @NoCSRFRequired
* @NoAdminRequired
* @NoSubAdminRequired
*/
#[NoCSRFRequired]
#[NoAdminRequired]
public function help(string $mode = 'user'): TemplateResponse {
$this->navigationManager->setActiveEntry('help');
$pageTitle = $this->l10n->t('Administrator documentation');
Expand Down
4 changes: 2 additions & 2 deletions apps/settings/lib/Controller/LogSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use OC\Log;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\StreamResponse;
use OCP\IRequest;

Expand All @@ -26,14 +27,13 @@ public function __construct(string $appName, IRequest $request, Log $logger) {
/**
* download logfile
*
* @NoCSRFRequired
*
* @psalm-suppress MoreSpecificReturnType The value of Content-Disposition is not relevant
* @psalm-suppress LessSpecificReturnStatement The value of Content-Disposition is not relevant
* @return StreamResponse<Http::STATUS_OK, array{Content-Type: 'application/octet-stream', 'Content-Disposition': string}>
*
* 200: Logfile returned
*/
#[NoCSRFRequired]
public function download() {
if (!$this->log instanceof Log) {
throw new \UnexpectedValueException('Log file not available');
Expand Down
Loading

0 comments on commit 4673e1a

Please sign in to comment.