Merge pull request #46805 from nextcloud/refactor/federatedfilesharin…

…g/security-attributes
nextcloud · Jul 30, 2024 · 24a0d3e · 24a0d3e
2 parents 016fbb9 + e4e3783
commit 24a0d3e
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 30 deletions.
diff --git a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php
@@ -11,7 +11,11 @@
 use OCA\FederatedFileSharing\FederatedShareProvider;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\BruteForceProtection;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
+use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\Constants;
 use OCP\Federation\ICloudIdManager;
@@ -56,17 +60,16 @@ public function __construct(
 	/**
 	 * send federated share to a user of a public link
 	 *
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 * @BruteForceProtection(action=publicLink2FederatedShare)
-	 *
 	 * @param string $shareWith Username to share with
 	 * @param string $token Token of the share
 	 * @param string $password Password of the share
 	 * @return JSONResponse<Http::STATUS_OK, array{remoteUrl: string}, array{}>|JSONResponse<Http::STATUS_BAD_REQUEST, array{message: string}, array{}>
 	 * 200: Remote URL returned
 	 * 400: Creating share is not possible
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
+	#[BruteForceProtection(action: 'publicLink2FederatedShare')]
 	public function createFederatedShare($shareWith, $token, $password = '') {
 		if (!$this->federatedShareProvider->isOutgoingServer2serverShareEnabled()) {
 			return new JSONResponse(
@@ -125,8 +128,6 @@ public function createFederatedShare($shareWith, $token, $password = '') {
 	/**
 	 * ask other server to get a federated share
 	 *
-	 * @NoAdminRequired
-	 *
 	 * @param string $token
 	 * @param string $remote
 	 * @param string $password
@@ -135,6 +136,7 @@ public function createFederatedShare($shareWith, $token, $password = '') {
 	 * @param string $name (only for legacy reasons, can be removed with legacyMountPublicLink())
 	 * @return JSONResponse
 	 */
+	#[NoAdminRequired]
 	public function askForFederatedShare($token, $remote, $password = '', $owner = '', $ownerDisplayName = '', $name = '') {
 		// check if server admin allows to mount public links from other servers
 		if ($this->federatedShareProvider->isIncomingServer2serverShareEnabled() === false) {

diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php
@@ -12,7 +12,9 @@
 use OCA\FederatedFileSharing\Notifications;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
+use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\OCS\OCSBadRequestException;
 use OCP\AppFramework\OCS\OCSException;
 use OCP\AppFramework\OCSController;
@@ -100,9 +102,6 @@ public function __construct(string $appName,
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 *
 	 * create a new share
 	 *
 	 * @param string|null $remote Address of the remote
@@ -119,6 +118,8 @@ public function __construct(string $appName,
 	 *
 	 * 200: Share created successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
 	public function createShare(
 		?string $remote = null,
 		?string $token = null,
@@ -173,9 +174,6 @@ public function createShare(
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 *
 	 * create re-share on behalf of another user
 	 *
 	 * @param int $id ID of the share
@@ -188,6 +186,8 @@ public function createShare(
 	 *
 	 * 200: Remote share returned
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
 	public function reShare(int $id, ?string $token = null, ?string $shareWith = null, ?int $remoteId = 0) {
 		if ($token === null ||
 			$shareWith === null ||
@@ -223,9 +223,6 @@ public function reShare(int $id, ?string $token = null, ?string $shareWith = nul
 
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 *
 	 * accept server-to-server share
 	 *
 	 * @param int $id ID of the remote share
@@ -237,6 +234,8 @@ public function reShare(int $id, ?string $token = null, ?string $shareWith = nul
 	 *
 	 * 200: Share accepted successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
 	public function acceptShare(int $id, ?string $token = null) {
 		$notification = [
 			'sharedSecret' => $token,
@@ -259,9 +258,6 @@ public function acceptShare(int $id, ?string $token = null) {
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 *
 	 * decline server-to-server share
 	 *
 	 * @param int $id ID of the remote share
@@ -271,6 +267,8 @@ public function acceptShare(int $id, ?string $token = null) {
 	 *
 	 * 200: Share declined successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
 	public function declineShare(int $id, ?string $token = null) {
 		$notification = [
 			'sharedSecret' => $token,
@@ -293,9 +291,6 @@ public function declineShare(int $id, ?string $token = null) {
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 *
 	 * remove server-to-server share if it was unshared by the owner
 	 *
 	 * @param int $id ID of the share
@@ -305,6 +300,8 @@ public function declineShare(int $id, ?string $token = null) {
 	 *
 	 * 200: Share unshared successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
 	public function unshare(int $id, ?string $token = null) {
 		if (!$this->isS2SEnabled()) {
 			throw new OCSException('Server does not support federated cloud sharing', 503);
@@ -330,9 +327,6 @@ private function cleanupRemote($remote) {
 
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 *
 	 * federated share was revoked, either by the owner or the re-sharer
 	 *
 	 * @param int $id ID of the share
@@ -342,6 +336,8 @@ private function cleanupRemote($remote) {
 	 *
 	 * 200: Share revoked successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
 	public function revoke(int $id, ?string $token = null) {
 		try {
 			$provider = $this->cloudFederationProviderManager->getCloudFederationProvider('file');
@@ -372,9 +368,6 @@ private function isS2SEnabled($incoming = false) {
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 *
 	 * update share information to keep federated re-shares in sync
 	 *
 	 * @param int $id ID of the share
@@ -385,6 +378,8 @@ private function isS2SEnabled($incoming = false) {
 	 *
 	 * 200: Permissions updated successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
 	public function updatePermissions(int $id, ?string $token = null, ?int $permissions = null) {
 		$ncPermissions = $permissions;
 
@@ -428,9 +423,6 @@ protected function ncPermissions2ocmPermissions($ncPermissions) {
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 *
 	 * change the owner of a server-to-server share
 	 *
 	 * @param int $id ID of the share
@@ -442,6 +434,8 @@ protected function ncPermissions2ocmPermissions($ncPermissions) {
 	 *
 	 * 200: Share moved successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
 	public function move(int $id, ?string $token = null, ?string $remote = null, ?string $remote_id = null) {
 		if (!$this->isS2SEnabled()) {
 			throw new OCSException('Server does not support federated cloud sharing', 503);