2 files changed, 105 insertions, 79 deletions

diff --git a/apps/federation/lib/Controller/OCSAuthAPIController.php b/apps/federation/lib/Controller/OCSAuthAPIController.php
index b4849b274b8..16b401be251 100644
--- a/apps/federation/lib/Controller/OCSAuthAPIController.php
+++ b/apps/federation/lib/Controller/OCSAuthAPIController.php
@@ -10,7 +10,10 @@ namespace OCA\Federation\Controller;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\BruteForceProtection;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
+use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\OCSController;
@@ -30,50 +33,33 @@ use Psr\Log\LoggerInterface;
  */
 #[OpenAPI(scope: OpenAPI::SCOPE_FEDERATION)]
 class OCSAuthAPIController extends OCSController {
-	private ISecureRandom $secureRandom;
-	private IJobList $jobList;
-	private TrustedServers $trustedServers;
-	private DbHandler $dbHandler;
-	private LoggerInterface $logger;
-	private ITimeFactory $timeFactory;
-	private IThrottler $throttler;
-
 	public function __construct(
 		string $appName,
 		IRequest $request,
-		ISecureRandom $secureRandom,
-		IJobList $jobList,
-		TrustedServers $trustedServers,
-		DbHandler $dbHandler,
-		LoggerInterface $logger,
-		ITimeFactory $timeFactory,
-		IThrottler $throttler
+		private ISecureRandom $secureRandom,
+		private IJobList $jobList,
+		private TrustedServers $trustedServers,
+		private DbHandler $dbHandler,
+		private LoggerInterface $logger,
+		private ITimeFactory $timeFactory,
+		private IThrottler $throttler,
 	) {
 		parent::__construct($appName, $request);
-
-		$this->secureRandom = $secureRandom;
-		$this->jobList = $jobList;
-		$this->trustedServers = $trustedServers;
-		$this->dbHandler = $dbHandler;
-		$this->logger = $logger;
-		$this->timeFactory = $timeFactory;
-		$this->throttler = $throttler;
 	}
 
 	/**
 	 * Request received to ask remote server for a shared secret, for legacy end-points
 	 *
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 * @BruteForceProtection(action=federationSharedSecret)
-	 *
 	 * @param string $url URL of the server
 	 * @param string $token Token of the server
-	 * @return DataResponse<Http::STATUS_OK, array<empty>, array{}>
+	 * @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
 	 * @throws OCSForbiddenException Requesting shared secret is not allowed
 	 *
 	 * 200: Shared secret requested successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
+	#[BruteForceProtection(action: 'federationSharedSecret')]
 	public function requestSharedSecretLegacy(string $url, string $token): DataResponse {
 		return $this->requestSharedSecret($url, $token);
 	}
@@ -82,10 +68,6 @@ class OCSAuthAPIController extends OCSController {
 	/**
 	 * Create shared secret and return it, for legacy end-points
 	 *
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 * @BruteForceProtection(action=federationSharedSecret)
-	 *
 	 * @param string $url URL of the server
 	 * @param string $token Token of the server
 	 * @return DataResponse<Http::STATUS_OK, array{sharedSecret: string}, array{}>
@@ -93,6 +75,9 @@ class OCSAuthAPIController extends OCSController {
 	 *
 	 * 200: Shared secret returned
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
+	#[BruteForceProtection(action: 'federationSharedSecret')]
 	public function getSharedSecretLegacy(string $url, string $token): DataResponse {
 		return $this->getSharedSecret($url, $token);
 	}
@@ -100,21 +85,20 @@ class OCSAuthAPIController extends OCSController {
 	/**
 	 * Request received to ask remote server for a shared secret
 	 *
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 * @BruteForceProtection(action=federationSharedSecret)
-	 *
 	 * @param string $url URL of the server
 	 * @param string $token Token of the server
-	 * @return DataResponse<Http::STATUS_OK, array<empty>, array{}>
+	 * @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
 	 * @throws OCSForbiddenException Requesting shared secret is not allowed
 	 *
 	 * 200: Shared secret requested successfully
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
+	#[BruteForceProtection(action: 'federationSharedSecret')]
 	public function requestSharedSecret(string $url, string $token): DataResponse {
 		if ($this->trustedServers->isTrustedServer($url) === false) {
 			$this->throttler->registerAttempt('federationSharedSecret', $this->request->getRemoteAddress());
-			$this->logger->error('remote server not trusted (' . $url . ') while requesting shared secret', ['app' => 'federation']);
+			$this->logger->error('remote server not trusted (' . $url . ') while requesting shared secret');
 			throw new OCSForbiddenException();
 		}
 
@@ -123,8 +107,7 @@ class OCSAuthAPIController extends OCSController {
 		$localToken = $this->dbHandler->getToken($url);
 		if (strcmp($localToken, $token) > 0) {
 			$this->logger->info(
-				'remote server (' . $url . ') presented lower token. We will initiate the exchange of the shared secret.',
-				['app' => 'federation']
+				'remote server (' . $url . ') presented lower token. We will initiate the exchange of the shared secret.'
 			);
 			throw new OCSForbiddenException();
 		}
@@ -144,10 +127,6 @@ class OCSAuthAPIController extends OCSController {
 	/**
 	 * Create shared secret and return it
 	 *
-	 * @NoCSRFRequired
-	 * @PublicPage
-	 * @BruteForceProtection(action=federationSharedSecret)
-	 *
 	 * @param string $url URL of the server
 	 * @param string $token Token of the server
 	 * @return DataResponse<Http::STATUS_OK, array{sharedSecret: string}, array{}>
@@ -155,10 +134,13 @@ class OCSAuthAPIController extends OCSController {
 	 *
 	 * 200: Shared secret returned
 	 */
+	#[NoCSRFRequired]
+	#[PublicPage]
+	#[BruteForceProtection(action: 'federationSharedSecret')]
 	public function getSharedSecret(string $url, string $token): DataResponse {
 		if ($this->trustedServers->isTrustedServer($url) === false) {
 			$this->throttler->registerAttempt('federationSharedSecret', $this->request->getRemoteAddress());
-			$this->logger->error('remote server not trusted (' . $url . ') while getting shared secret', ['app' => 'federation']);
+			$this->logger->error('remote server not trusted (' . $url . ') while getting shared secret');
 			throw new OCSForbiddenException();
 		}
 
@@ -166,8 +148,7 @@ class OCSAuthAPIController extends OCSController {
 			$this->throttler->registerAttempt('federationSharedSecret', $this->request->getRemoteAddress());
 			$expectedToken = $this->dbHandler->getToken($url);
 			$this->logger->error(
-				'remote server (' . $url . ') didn\'t send a valid token (got "' . $token . '" but expected "'. $expectedToken . '") while getting shared secret',
-				['app' => 'federation']
+				'remote server (' . $url . ') didn\'t send a valid token (got "' . $token . '" but expected "' . $expectedToken . '") while getting shared secret'
 			);
 			throw new OCSForbiddenException();
 		}
diff --git a/apps/federation/lib/Controller/SettingsController.php b/apps/federation/lib/Controller/SettingsController.php
index f5cc7eae8ba..27341eba815 100644
--- a/apps/federation/lib/Controller/SettingsController.php
+++ b/apps/federation/lib/Controller/SettingsController.php
@@ -7,38 +7,48 @@
  */
 namespace OCA\Federation\Controller;
 
+use OCA\Federation\Settings\Admin;
 use OCA\Federation\TrustedServers;
-use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\ApiRoute;
+use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
 use OCP\AppFramework\Http\DataResponse;
-use OCP\HintException;
+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 Controller {
-	private IL10N $l;
-	private TrustedServers $trustedServers;
-
-	public function __construct(string $AppName,
+class SettingsController extends OCSController {
+	public function __construct(
+		string $AppName,
 		IRequest $request,
-		IL10N $l10n,
-		TrustedServers $trustedServers
+		private IL10N $l,
+		private TrustedServers $trustedServers,
+		private LoggerInterface $logger,
 	) {
 		parent::__construct($AppName, $request);
-		$this->l = $l10n;
-		$this->trustedServers = $trustedServers;
 	}
 
 
 	/**
-	 * Add server to the list of trusted Nextclouds.
+	 * Add server to the list of trusted Nextcloud servers
+	 *
+	 * @param string $url The URL of the server to add
+	 * @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{}>
 	 *
-	 * @AuthorizedAdminSetting(settings=OCA\Federation\Settings\Admin)
-	 * @throws HintException
+	 * 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)]
+	#[ApiRoute(verb: 'POST', url: '/trusted-servers')]
 	public function addServer(string $url): DataResponse {
-		$this->checkServer($url);
-		$id = $this->trustedServers->addServer($url);
+		$this->checkServer(trim($url));
 
+		// Add the server to the list of trusted servers, all is well
+		$id = $this->trustedServers->addServer(trim($url));
 		return new DataResponse([
 			'url' => $url,
 			'id' => $id,
@@ -47,34 +57,69 @@ class SettingsController extends Controller {
 	}
 
 	/**
-	 * Add server to the list of trusted Nextclouds.
+	 * Add server to the list of trusted Nextcloud servers
+	 *
+	 * @param int $id The ID of the trusted server to remove
+	 * @return DataResponse<Http::STATUS_OK, array{id: int}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{message: string}, array{}>
 	 *
-	 * @AuthorizedAdminSetting(settings=OCA\Federation\Settings\Admin)
+	 * 200: Server removed successfully
+	 * 404: Server not found at the given ID
 	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
+	#[ApiRoute(verb: 'DELETE', url: '/trusted-servers/{id}', requirements: ['id' => '\d+'])]
 	public function removeServer(int $id): DataResponse {
-		$this->trustedServers->removeServer($id);
-		return new 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 DataResponse(['id' => $id]);
+		} catch (\Exception $e) {
+			$this->logger->error($e->getMessage(), ['e' => $e]);
+			throw new OCSException($this->l->t('Could not remove server'), Http::STATUS_INTERNAL_SERVER_ERROR);
+		}
 	}
 
 	/**
-	 * Check if the server should be added to the list of trusted servers or not.
+	 * List all trusted servers
 	 *
-	 * @AuthorizedAdminSetting(settings=OCA\Federation\Settings\Admin)
-	 * @throws HintException
+	 * @return DataResponse<Http::STATUS_OK, list<array{id: int, status: int, url: string}>, array{}>
+	 *
+	 * 200: List of trusted servers
 	 */
-	protected function checkServer(string $url): bool {
+	#[AuthorizedAdminSetting(settings: Admin::class)]
+	#[ApiRoute(verb: 'GET', url: '/trusted-servers')]
+	public function getServers(): DataResponse {
+		$servers = $this->trustedServers->getServers();
+
+		// obfuscate the shared secret
+		$servers = array_map(function ($server) {
+			return [
+				'url' => $server['url'],
+				'id' => $server['id'],
+				'status' => $server['status'],
+			];
+		}, $servers);
+
+		// return the list of trusted servers
+		return new DataResponse($servers);
+	}
+
+
+	/**
+	 * Check if the server should be added to the list of trusted servers or not.
+	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
+	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.');
-			throw new HintException($message, $hint);
+			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');
-			throw new HintException($message, $hint);
+			throw new OCSNotFoundException($this->l->t('No server to federate with found'));
 		}
-
-		return true;
 	}
 }