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

refactor(settings): Replace security annotations with respective attributes #46816

Merged
merged 1 commit into from
Aug 14, 2024
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
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
come-nc marked this conversation as resolved.
Show resolved Hide resolved
* 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
Loading