chore(federation): cleanup SettingsController and legacy AddServerMid…

…dleware

Signed-off-by: skjnldsv <[email protected]>

apps/federatedfilesharing/lib/FederatedShareProvider.php

@@ -909,104 +909,90 @@ public function userDeletedFromGroup($uid, $gid) {
 	}
 
 	/**
-	 * check if users from other Nextcloud instances are allowed to mount public links share by this instance
-	 *
-	 * @return bool
+	 * Check if users from other Nextcloud instances are allowed to mount public links share by this instance
 	 */
-	public function isOutgoingServer2serverShareEnabled() {
+	public function isOutgoingServer2serverShareEnabled(): bool {
 		if ($this->gsConfig->onlyInternalFederation()) {
 			return false;
 		}
 		$result = $this->config->getAppValue('files_sharing', 'outgoing_server2server_share_enabled', 'yes');
-		return ($result === 'yes');
+		return $result === 'yes';
 	}
 
 	/**
-	 * check if users are allowed to mount public links from other Nextclouds
-	 *
-	 * @return bool
+	 * Check if users are allowed to mount public links from other Nextclouds
 	 */
-	public function isIncomingServer2serverShareEnabled() {
+	public function isIncomingServer2serverShareEnabled(): bool {
 		if ($this->gsConfig->onlyInternalFederation()) {
 			return false;
 		}
 		$result = $this->config->getAppValue('files_sharing', 'incoming_server2server_share_enabled', 'yes');
-		return ($result === 'yes');
+		return $result === 'yes';
 	}
 
 
 	/**
-	 * check if users from other Nextcloud instances are allowed to send federated group shares
-	 *
-	 * @return bool
+	 * Check if users from other Nextcloud instances are allowed to send federated group shares
 	 */
-	public function isOutgoingServer2serverGroupShareEnabled() {
+	public function isOutgoingServer2serverGroupShareEnabled(): bool {
 		if ($this->gsConfig->onlyInternalFederation()) {
 			return false;
 		}
 		$result = $this->config->getAppValue('files_sharing', 'outgoing_server2server_group_share_enabled', 'no');
-		return ($result === 'yes');
+		return $result === 'yes';
 	}
 
 	/**
-	 * check if users are allowed to receive federated group shares
-	 *
-	 * @return bool
+	 * Check if users are allowed to receive federated group shares
 	 */
-	public function isIncomingServer2serverGroupShareEnabled() {
+	public function isIncomingServer2serverGroupShareEnabled(): bool {
 		if ($this->gsConfig->onlyInternalFederation()) {
 			return false;
 		}
 		$result = $this->config->getAppValue('files_sharing', 'incoming_server2server_group_share_enabled', 'no');
-		return ($result === 'yes');
+		return $result === 'yes';
 	}
 
 	/**
-	 * check if federated group sharing is supported, therefore the OCM API need to be enabled
-	 *
-	 * @return bool
+	 * Check if federated group sharing is supported, therefore the OCM API need to be enabled
 	 */
-	public function isFederatedGroupSharingSupported() {
+	public function isFederatedGroupSharingSupported(): bool {
 		return $this->cloudFederationProviderManager->isReady();
 	}
 
 	/**
 	 * Check if querying sharees on the lookup server is enabled
-	 *
-	 * @return bool
 	 */
-	public function isLookupServerQueriesEnabled() {
+	public function isLookupServerQueriesEnabled(): bool {
 		// in a global scale setup we should always query the lookup server
 		if ($this->gsConfig->isGlobalScaleEnabled()) {
 			return true;
 		}
 		$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes');
-		return ($result === 'yes');
+		return $result === 'yes';
 	}
 
 
 	/**
 	 * Check if it is allowed to publish user specific data to the lookup server
-	 *
-	 * @return bool
 	 */
-	public function isLookupServerUploadEnabled() {
+	public function isLookupServerUploadEnabled(): bool {
 		// in a global scale setup the admin is responsible to keep the lookup server up-to-date
 		if ($this->gsConfig->isGlobalScaleEnabled()) {
 			return false;
 		}
 		$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes');
-		return ($result === 'yes');
+		return $result === 'yes';
 	}
 
-	public function isFederatedTrustedShareAutoAccept() {
+	/**
+	 * Check if auto accepting incoming shares from trusted servers is enabled
+	 */
+	public function isFederatedTrustedShareAutoAccept(): bool {
 		$result = $this->config->getAppValue('files_sharing', 'federatedTrustedShareAutoAccept', 'yes');
-		return ($result === 'yes');
+		return $result === 'yes';
 	}
 
-	/**
-	 * @inheritdoc
-	 */
 	public function getAccessList($nodes, $currentAccess) {
 		$ids = [];
 		foreach ($nodes as $node) {

apps/federation/appinfo/routes.php

@@ -31,20 +31,5 @@
 			'url' => '/shared-secret',
 			'verb' => 'POST',
 		],
-		[
-			'name' => 'Settings#getServers',
-			'url' => '/trusted-servers',
-			'verb' => 'GET'
-		],
-		[
-			'name' => 'Settings#addServer',
-			'url' => '/trusted-servers',
-			'verb' => 'POST'
-		],
-		[
-			'name' => 'Settings#removeServer',
-			'url' => '/trusted-servers/{id}',
-			'verb' => 'DELETE'
-		],
 	],
 ];

apps/federation/composer/composer/autoload_classmap.php

@@ -16,7 +16,6 @@
     'OCA\\Federation\\DAV\\FedAuth' => $baseDir . '/../lib/DAV/FedAuth.php',
     'OCA\\Federation\\DbHandler' => $baseDir . '/../lib/DbHandler.php',
     'OCA\\Federation\\Listener\\SabrePluginAuthInitListener' => $baseDir . '/../lib/Listener/SabrePluginAuthInitListener.php',
-    'OCA\\Federation\\Middleware\\AddServerMiddleware' => $baseDir . '/../lib/Middleware/AddServerMiddleware.php',
     'OCA\\Federation\\Migration\\Version1010Date20200630191302' => $baseDir . '/../lib/Migration/Version1010Date20200630191302.php',
     'OCA\\Federation\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',
     'OCA\\Federation\\SyncFederationAddressBooks' => $baseDir . '/../lib/SyncFederationAddressBooks.php',

apps/federation/composer/composer/autoload_static.php

@@ -31,7 +31,6 @@ class ComposerStaticInitFederation
         'OCA\\Federation\\DAV\\FedAuth' => __DIR__ . '/..' . '/../lib/DAV/FedAuth.php',
         'OCA\\Federation\\DbHandler' => __DIR__ . '/..' . '/../lib/DbHandler.php',
         'OCA\\Federation\\Listener\\SabrePluginAuthInitListener' => __DIR__ . '/..' . '/../lib/Listener/SabrePluginAuthInitListener.php',
-        'OCA\\Federation\\Middleware\\AddServerMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/AddServerMiddleware.php',
         'OCA\\Federation\\Migration\\Version1010Date20200630191302' => __DIR__ . '/..' . '/../lib/Migration/Version1010Date20200630191302.php',
         'OCA\\Federation\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
         'OCA\\Federation\\SyncFederationAddressBooks' => __DIR__ . '/..' . '/../lib/SyncFederationAddressBooks.php',

apps/federation/js/settings-admin.js

@@ -79,8 +79,11 @@
             OC.getRootPath() + '/ocs/v2.php/apps/federation/trusted-servers',
             {
                 url: url
-            }
-        ).done(function({data}) {
+            },
+            null,
+            'json'
+        ).done(function({ ocs }) {
+            var data = ocs.data;
             $("#serverUrl").attr('value', '');
             $("#listOfTrustedServers").prepend(
                 $('<li>')
@@ -92,7 +95,7 @@
             OC.msg.finishedSuccess('#ocFederationAddServer .msg', data.message);
         })
         .fail(function (jqXHR) {
-            OC.msg.finishedError('#ocFederationAddServer .msg', JSON.parse(jqXHR.responseText).data.message);
+            OC.msg.finishedError('#ocFederationAddServer .msg', JSON.parse(jqXHR.responseText).ocs.meta.message);
         });
     };
 

apps/federation/lib/AppInfo/Application.php

@@ -9,7 +9,6 @@
 
 use OCA\DAV\Events\SabrePluginAuthInitEvent;
 use OCA\Federation\Listener\SabrePluginAuthInitListener;
-use OCA\Federation\Middleware\AddServerMiddleware;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IBootContext;
 use OCP\AppFramework\Bootstrap\IBootstrap;
@@ -25,8 +24,6 @@ public function __construct($urlParams = []) {
 	}
 
 	public function register(IRegistrationContext $context): void {
-		$context->registerMiddleware(AddServerMiddleware::class);
-
 		$context->registerEventListener(SabrePluginAuthInitEvent::class, SabrePluginAuthInitListener::class);
 	}
 

apps/federation/lib/Controller/SettingsController.php

@@ -10,18 +10,23 @@
 use OCA\Federation\Settings\Admin;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\ApiRoute;
 use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
-use OCP\AppFramework\Http\JSONResponse;
+use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\OCS\OCSException;
+use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\OCSController;
 use OCP\IL10N;
 use OCP\IRequest;
+use Psr\Log\LoggerInterface;
 
 class SettingsController extends OCSController {
 	public function __construct(
 		string $AppName,
 		IRequest $request,
 		private IL10N $l,
 		private TrustedServers $trustedServers,
+		private LoggerInterface $logger,
 	) {
 		parent::__construct($AppName, $request);
 	}
@@ -31,67 +36,63 @@ public function __construct(
 	 * Add server to the list of trusted Nextcloud servers
 	 *
 	 * @param string $url The URL of the server to add
-	 * @return JSONResponse<Http::STATUS_OK, array{data: array{id: int, message: string, url: string}, status: 'ok'}, array{}>|JSONResponse<Http::STATUS_NOT_FOUND|Http::STATUS_CONFLICT, array{data: array{hint: string, message: string}, status: 'error'}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, array{id: int, message: string, url: string}, array{}>|DataResponse<Http::STATUS_NOT_FOUND|Http::STATUS_CONFLICT, array{message: string}, array{}>
 	 *
 	 * 200: Server added successfully
 	 * 404: Server not found at the given URL
 	 * 409: Server is already in the list of trusted servers
 	 */
 	#[AuthorizedAdminSetting(settings: Admin::class)]
-	public function addServer(string $url): JSONResponse {
-		$check = $this->checkServer(trim($url));
-		if ($check instanceof JSONResponse) {
-			return $check;
-		}
+	#[ApiRoute(verb: 'POST', url: '/trusted-servers')]
+	public function addServer(string $url): DataResponse {
+		$this->checkServer(trim($url));
 
 		// Add the server to the list of trusted servers, all is well
 		$id = $this->trustedServers->addServer(trim($url));
-		return new JSONResponse([
-			'status' => 'ok',
-			'data' => [
-				'url' => $url,
-				'id' => $id,
-				'message' => $this->l->t('Added to the list of trusted servers')
-			],
+		return new DataResponse([
+			'url' => $url,
+			'id' => $id,
+			'message' => $this->l->t('Added to the list of trusted servers')
 		]);
 	}
 
 	/**
 	 * Add server to the list of trusted Nextcloud servers
 	 *
 	 * @param int $id The ID of the trusted server to remove
-	 * @return JSONResponse<Http::STATUS_OK, array{data: array{id: int}, status: 'ok'}, array{}>|JSONResponse<Http::STATUS_NOT_FOUND, array{data: array{message: string}, status: 'error'}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, array{id: int}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{message: string}, array{}>
 	 *
 	 * 200: Server removed successfully
 	 * 404: Server not found at the given ID
 	 */
 	#[AuthorizedAdminSetting(settings: Admin::class)]
-	public function removeServer(int $id): JSONResponse {
+	#[ApiRoute(verb: 'DELETE', url: '/trusted-servers/{id}', requirements: ['id' => '\d+'])]
+	public function removeServer(int $id): DataResponse {
+		try {
+			$this->trustedServers->getServer($id);
+		} catch (\Exception $e) {
+			throw new OCSNotFoundException($this->l->t('No server found with ID: %s', [$id]));
+		}
+
 		try {
 			$this->trustedServers->removeServer($id);
-			return new JSONResponse([
-				'status' => 'ok',
-				'data' => ['id' => $id],
-			]);
+			return new DataResponse(['id' => $id]);
 		} catch (\Exception $e) {
-			return new JSONResponse([
-				'status' => 'error',
-				'data' => [
-					'message' => $e->getMessage(),
-				],
-			], Http::STATUS_NOT_FOUND);
+			$this->logger->error($e->getMessage(), ['app' => 'federation']);
+			throw new OCSException($this->l->t('Could not remove server'), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 	}
 
 	/**
 	 * List all trusted servers
 	 *
-	 * @return JSONResponse<Http::STATUS_OK, array{data: list<array{id: int, status: int, url: string}>, status: 'ok'}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, list<array{id: int, status: int, url: string}>, array{}>
 	 *
 	 * 200: List of trusted servers
 	 */
 	#[AuthorizedAdminSetting(settings: Admin::class)]
-	public function getServers(): JSONResponse {
+	#[ApiRoute(verb: 'GET', url: '/trusted-servers')]
+	public function getServers(): DataResponse {
 		$servers = $this->trustedServers->getServers();
 
 		// obfuscate the shared secret
@@ -104,47 +105,21 @@ public function getServers(): JSONResponse {
 		}, $servers);
 
 		// return the list of trusted servers
-		return new JSONResponse([
-			'status' => 'ok',
-			'data' => $servers,
-		]);
+		return new DataResponse($servers);
 	}
 
 
 	/**
 	 * Check if the server should be added to the list of trusted servers or not.
-	 *
-	 * @return JSONResponse<Http::STATUS_NOT_FOUND|Http::STATUS_CONFLICT, array{data: array{hint: string, message: string}, status: 'error'}, array{}>|null
-	 *
-	 * 404: Server not found at the given URL
-	 * 409: Server is already in the list of trusted servers
 	 */
 	#[AuthorizedAdminSetting(settings: Admin::class)]
-	protected function checkServer(string $url): ?JSONResponse {
+	protected function checkServer(string $url): void {
 		if ($this->trustedServers->isTrustedServer($url) === true) {
-			$message = 'Server is already in the list of trusted servers.';
-			$hint = $this->l->t('Server is already in the list of trusted servers.');
-			return new JSONResponse([
-				'status' => 'error',
-				'data' => [
-					'message' => $message,
-					'hint' => $hint,
-				],
-			], Http::STATUS_CONFLICT);
+			throw new OCSException($this->l->t('Server is already in the list of trusted servers.'), Http::STATUS_CONFLICT);
 		}
 
 		if ($this->trustedServers->isNextcloudServer($url) === false) {
-			$message = 'No server to federate with found';
-			$hint = $this->l->t('No server to federate with found');
-			return new JSONResponse([
-				'status' => 'error',
-				'data' => [
-					'message' => $message,
-					'hint' => $hint,
-				],
-			], Http::STATUS_NOT_FOUND);
+			throw new OCSNotFoundException($this->l->t('No server to federate with found'));
 		}
-
-		return null;
 	}
 }