Merge pull request #46810 from nextcloud/refactor/files_sharing/secur…

…ity-attributes
nextcloud · Aug 1, 2024 · 553224b · 553224b
2 parents 5bfe8df + 9128a23
commit 553224b
Show file tree

Hide file tree

Showing 10 changed files with 57 additions and 75 deletions.
diff --git a/apps/files_sharing/lib/Controller/AcceptController.php b/apps/files_sharing/lib/Controller/AcceptController.php
@@ -10,6 +10,8 @@
 
 use OCA\Files_Sharing\AppInfo\Application;
 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\NotFoundResponse;
 use OCP\AppFramework\Http\RedirectResponse;
@@ -40,10 +42,8 @@ public function __construct(IRequest $request, ShareManager $shareManager, IUser
 		$this->urlGenerator = $urlGenerator;
 	}
 
-	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 */
+	#[NoAdminRequired]
+	#[NoCSRFRequired]
 	public function accept(string $shareId): Response {
 		try {
 			$share = $this->shareManager->getShareById($shareId);

diff --git a/apps/files_sharing/lib/Controller/DeletedShareAPIController.php b/apps/files_sharing/lib/Controller/DeletedShareAPIController.php
@@ -11,6 +11,7 @@
 use OCA\Files_Sharing\ResponseDefinitions;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSException;
 use OCP\AppFramework\OCS\OCSNotFoundException;
@@ -158,14 +159,13 @@ private function formatShare(IShare $share): array {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Get a list of all deleted shares
 	 *
 	 * @return DataResponse<Http::STATUS_OK, Files_SharingDeletedShare[], array{}>
 	 *
 	 * 200: Deleted shares returned
 	 */
+	#[NoAdminRequired]
 	public function index(): DataResponse {
 		$groupShares = $this->shareManager->getDeletedSharedWith($this->userId, IShare::TYPE_GROUP, null, -1, 0);
 		$roomShares = $this->shareManager->getDeletedSharedWith($this->userId, IShare::TYPE_ROOM, null, -1, 0);
@@ -182,8 +182,6 @@ public function index(): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Undelete a deleted share
 	 *
 	 * @param string $id ID of the share
@@ -193,6 +191,7 @@ public function index(): DataResponse {
 	 *
 	 * 200: Share undeleted successfully
 	 */
+	#[NoAdminRequired]
 	public function undelete(string $id): DataResponse {
 		try {
 			$share = $this->shareManager->getShareById($id, $this->userId);

diff --git a/apps/files_sharing/lib/Controller/ExternalSharesController.php b/apps/files_sharing/lib/Controller/ExternalSharesController.php
@@ -7,6 +7,8 @@
 namespace OCA\Files_Sharing\Controller;
 
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\Http\Client\IClientService;
@@ -30,34 +32,34 @@ public function __construct(
 	}
 
 	/**
-	 * @NoAdminRequired
 	 * @NoOutgoingFederatedSharingRequired
 	 *
 	 * @return JSONResponse
 	 */
+	#[NoAdminRequired]
 	public function index() {
 		return new JSONResponse($this->externalManager->getOpenShares());
 	}
 
 	/**
-	 * @NoAdminRequired
 	 * @NoOutgoingFederatedSharingRequired
 	 *
 	 * @param int $id
 	 * @return JSONResponse
 	 */
+	#[NoAdminRequired]
 	public function create($id) {
 		$this->externalManager->acceptShare($id);
 		return new JSONResponse();
 	}
 
 	/**
-	 * @NoAdminRequired
 	 * @NoOutgoingFederatedSharingRequired
 	 *
 	 * @param integer $id
 	 * @return JSONResponse
 	 */
+	#[NoAdminRequired]
 	public function destroy($id) {
 		$this->externalManager->declineShare($id);
 		return new JSONResponse();
@@ -93,13 +95,13 @@ protected function testUrl($remote, $checkVersion = false) {
 	}
 
 	/**
-	 * @PublicPage
 	 * @NoOutgoingFederatedSharingRequired
 	 * @NoIncomingFederatedSharingRequired
 	 *
 	 * @param string $remote
 	 * @return DataResponse
 	 */
+	#[PublicPage]
 	public function testRemote($remote) {
 		if (str_contains($remote, '#') || str_contains($remote, '?') || str_contains($remote, ';')) {
 			return new DataResponse(false);

diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php
@@ -6,6 +6,8 @@
 namespace OCA\Files_Sharing\Controller;
 
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
+use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\PublicShareController;
@@ -60,9 +62,6 @@ protected function isPasswordProtected(): bool {
 
 
 	/**
-	 * @PublicPage
-	 * @NoCSRFRequired
-	 *
 	 * Get a preview for a shared file
 	 *
 	 * @param string $token Token of the share
@@ -77,6 +76,8 @@ protected function isPasswordProtected(): bool {
 	 * 403: Getting preview is not allowed
 	 * 404: Share or preview not found
 	 */
+	#[PublicPage]
+	#[NoCSRFRequired]
 	public function getPreview(
 		string $token,
 		string $file = '',
@@ -123,8 +124,6 @@ public function getPreview(
 	}
 
 	/**
-	 * @PublicPage
-	 * @NoCSRFRequired
 	 * @NoSameSiteCookieRequired
 	 *
 	 * Get a direct link preview for a shared file
@@ -137,6 +136,8 @@ public function getPreview(
 	 * 403: Getting preview is not allowed
 	 * 404: Share or preview not found
 	 */
+	#[PublicPage]
+	#[NoCSRFRequired]
 	public function directLink(string $token) {
 		// No token no image
 		if ($token === '') {

diff --git a/apps/files_sharing/lib/Controller/RemoteController.php b/apps/files_sharing/lib/Controller/RemoteController.php
@@ -9,6 +9,7 @@
 use OCA\Files_Sharing\External\Manager;
 use OCA\Files_Sharing\ResponseDefinitions;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\OCS\OCSNotFoundException;
@@ -21,8 +22,6 @@
  */
 class RemoteController extends OCSController {
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Remote constructor.
 	 *
 	 * @param string $appName
@@ -39,21 +38,18 @@ public function __construct(
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Get list of pending remote shares
 	 *
 	 * @return DataResponse<Http::STATUS_OK, Files_SharingRemoteShare[], array{}>
 	 *
 	 * 200: Pending remote shares returned
 	 */
+	#[NoAdminRequired]
 	public function getOpenShares() {
 		return new DataResponse($this->externalManager->getOpenShares());
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Accept a remote share
 	 *
 	 * @param int $id ID of the share
@@ -62,6 +58,7 @@ public function getOpenShares() {
 	 *
 	 * 200: Share accepted successfully
 	 */
+	#[NoAdminRequired]
 	public function acceptShare($id) {
 		if ($this->externalManager->acceptShare($id)) {
 			return new DataResponse();
@@ -74,8 +71,6 @@ public function acceptShare($id) {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Decline a remote share
 	 *
 	 * @param int $id ID of the share
@@ -84,6 +79,7 @@ public function acceptShare($id) {
 	 *
 	 * 200: Share declined successfully
 	 */
+	#[NoAdminRequired]
 	public function declineShare($id) {
 		if ($this->externalManager->declineShare($id)) {
 			return new DataResponse();
@@ -117,14 +113,13 @@ private static function extendShareInfo($share) {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Get a list of accepted remote shares
 	 *
 	 * @return DataResponse<Http::STATUS_OK, Files_SharingRemoteShare[], array{}>
 	 *
 	 * 200: Accepted remote shares returned
 	 */
+	#[NoAdminRequired]
 	public function getShares() {
 		$shares = $this->externalManager->getAcceptedShares();
 		$shares = array_map('self::extendShareInfo', $shares);
@@ -133,8 +128,6 @@ public function getShares() {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Get info of a remote share
 	 *
 	 * @param int $id ID of the share
@@ -143,6 +136,7 @@ public function getShares() {
 	 *
 	 * 200: Share returned
 	 */
+	#[NoAdminRequired]
 	public function getShare($id) {
 		$shareInfo = $this->externalManager->getShare($id);
 
@@ -155,8 +149,6 @@ public function getShare($id) {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Unshare a remote share
 	 *
 	 * @param int $id ID of the share
@@ -166,6 +158,7 @@ public function getShare($id) {
 	 *
 	 * 200: Share unshared successfully
 	 */
+	#[NoAdminRequired]
 	public function unshare($id) {
 		$shareInfo = $this->externalManager->getShare($id);
 

diff --git a/apps/files_sharing/lib/Controller/SettingsController.php b/apps/files_sharing/lib/Controller/SettingsController.php
@@ -10,6 +10,7 @@
 
 use OCA\Files_Sharing\AppInfo\Application;
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IConfig;
 use OCP\IRequest;
@@ -31,25 +32,19 @@ public function __construct(IRequest $request,
 		$this->userId = $userId;
 	}
 
-	/**
-	 * @NoAdminRequired
-	 */
+	#[NoAdminRequired]
 	public function setDefaultAccept(bool $accept): JSONResponse {
 		$this->config->setUserValue($this->userId, Application::APP_ID, 'default_accept', $accept ? 'yes' : 'no');
 		return new JSONResponse();
 	}
 
-	/**
-	 * @NoAdminRequired
-	 */
+	#[NoAdminRequired]
 	public function setUserShareFolder(string $shareFolder): JSONResponse {
 		$this->config->setUserValue($this->userId, Application::APP_ID, 'share_folder', $shareFolder);
 		return new JSONResponse();
 	}
 
-	/**
-	 * @NoAdminRequired
-	 */
+	#[NoAdminRequired]
 	public function resetUserShareFolder(): JSONResponse {
 		$this->config->deleteUserValue($this->userId, Application::APP_ID, 'share_folder');
 		return new JSONResponse();