From 6bf65154a5698b9b3383b6b87130e97a1f69d981 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Thu, 9 Jan 2025 12:54:30 +0100 Subject: [PATCH] chore(federation): cleanup SettingsController and legacy AddServerMiddleware Signed-off-by: skjnldsv --- .../lib/FederatedShareProvider.php | 60 ++- apps/federation/appinfo/routes.php | 15 - .../composer/composer/autoload_classmap.php | 1 - .../composer/composer/autoload_static.php | 1 - apps/federation/js/settings-admin.js | 9 +- apps/federation/lib/AppInfo/Application.php | 3 - .../lib/Controller/SettingsController.php | 91 ++--- .../lib/Middleware/AddServerMiddleware.php | 55 --- apps/federation/lib/TrustedServers.php | 21 +- apps/federation/openapi-administration.json | 349 ++++++++++-------- apps/federation/openapi-full.json | 324 ++++++++-------- .../BackgroundJob/GetSharedSecretTest.php | 42 +-- .../Controller/SettingsControllerTest.php | 53 +-- .../Middleware/AddServerMiddlewareTest.php | 81 ---- apps/theming/openapi.json | 186 ---------- 15 files changed, 494 insertions(+), 797 deletions(-) delete mode 100644 apps/federation/lib/Middleware/AddServerMiddleware.php delete mode 100644 apps/federation/tests/Middleware/AddServerMiddlewareTest.php diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 4d2157c1b44da..2c836dfc090ab 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/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) { diff --git a/apps/federation/appinfo/routes.php b/apps/federation/appinfo/routes.php index 0fc6e5a16aef7..343870614b3b2 100644 --- a/apps/federation/appinfo/routes.php +++ b/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' - ], ], ]; diff --git a/apps/federation/composer/composer/autoload_classmap.php b/apps/federation/composer/composer/autoload_classmap.php index 1be343a65d1f3..d648b643c4642 100644 --- a/apps/federation/composer/composer/autoload_classmap.php +++ b/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', diff --git a/apps/federation/composer/composer/autoload_static.php b/apps/federation/composer/composer/autoload_static.php index 927aca6694924..29932e1dffc09 100644 --- a/apps/federation/composer/composer/autoload_static.php +++ b/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', diff --git a/apps/federation/js/settings-admin.js b/apps/federation/js/settings-admin.js index c2b9e206ad8c2..aa672e53e94b1 100644 --- a/apps/federation/js/settings-admin.js +++ b/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( $('
  • ') @@ -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); }); }; diff --git a/apps/federation/lib/AppInfo/Application.php b/apps/federation/lib/AppInfo/Application.php index c750392116452..358e3f68d508d 100644 --- a/apps/federation/lib/AppInfo/Application.php +++ b/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); } diff --git a/apps/federation/lib/Controller/SettingsController.php b/apps/federation/lib/Controller/SettingsController.php index 5b2ee257e14a0..1667e787ce72f 100644 --- a/apps/federation/lib/Controller/SettingsController.php +++ b/apps/federation/lib/Controller/SettingsController.php @@ -10,11 +10,15 @@ 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( @@ -22,6 +26,7 @@ public function __construct( IRequest $request, private IL10N $l, private TrustedServers $trustedServers, + private LoggerInterface $logger, ) { parent::__construct($AppName, $request); } @@ -31,28 +36,23 @@ public function __construct( * Add server to the list of trusted Nextcloud servers * * @param string $url The URL of the server to add - * @return JSONResponse|JSONResponse + * @return DataResponse|DataResponse * * 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') ]); } @@ -60,38 +60,39 @@ public function addServer(string $url): JSONResponse { * Add server to the list of trusted Nextcloud servers * * @param int $id The ID of the trusted server to remove - * @return JSONResponse|JSONResponse + * @return DataResponse|DataResponse * * 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, status: 'ok'}, array{}> + * @return DataResponse, 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|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; } } diff --git a/apps/federation/lib/Middleware/AddServerMiddleware.php b/apps/federation/lib/Middleware/AddServerMiddleware.php deleted file mode 100644 index 0e38dd8e7004d..0000000000000 --- a/apps/federation/lib/Middleware/AddServerMiddleware.php +++ /dev/null @@ -1,55 +0,0 @@ -logger->error($exception->getMessage(), [ - 'app' => $this->appName, - 'exception' => $exception, - ]); - if ($exception instanceof HintException) { - $message = $exception->getHint(); - } else { - $message = $exception->getMessage(); - } - - return new JSONResponse( - ['message' => $message], - Http::STATUS_BAD_REQUEST - ); - } -} diff --git a/apps/federation/lib/TrustedServers.php b/apps/federation/lib/TrustedServers.php index 231b892fc3e13..ca549586bfa96 100644 --- a/apps/federation/lib/TrustedServers.php +++ b/apps/federation/lib/TrustedServers.php @@ -98,13 +98,32 @@ public function removeServer(int $id): void { * @return list * @throws Exception */ - public function getServers() { + public function getServers(): ?array { if ($this->trustedServersCache === null) { $this->trustedServersCache = $this->dbHandler->getAllServer(); } return $this->trustedServersCache; } + /** + * Get a trusted server + * + * @return array{id: int, url: string, url_hash: string, shared_secret: ?string, status: int, sync_token: ?string} + * @throws Exception + */ + public function getServer(int $id): ?array { + if ($this->trustedServersCache === null) { + $this->trustedServersCache = $this->dbHandler->getAllServer(); + } + + $server = array_filter($this->trustedServersCache, fn ($server) => $server['id'] === $id); + if (empty($server)) { + throw new \Exception('No server found with ID: ' . $id); + } + + return $server[0]; + } + /** * Check if given server is a trusted Nextcloud server */ diff --git a/apps/federation/openapi-administration.json b/apps/federation/openapi-administration.json index f1ecd62f3d8b3..58f9293e0eed6 100644 --- a/apps/federation/openapi-administration.json +++ b/apps/federation/openapi-administration.json @@ -19,86 +19,35 @@ "scheme": "bearer" } }, - "schemas": {} - }, - "paths": { - "/ocs/v2.php/apps/federation/trusted-servers": { - "get": { - "operationId": "settings-get-servers", - "summary": "List all trusted servers", - "description": "This endpoint requires admin access", - "tags": [ - "settings" + "schemas": { + "OCSMeta": { + "type": "object", + "required": [ + "status", + "statuscode" ], - "security": [ - { - "bearer_auth": [] + "properties": { + "status": { + "type": "string" }, - { - "basic_auth": [] - } - ], - "parameters": [ - { - "name": "OCS-APIRequest", - "in": "header", - "description": "Required to be true for the API request to pass", - "required": true, - "schema": { - "type": "boolean", - "default": true - } - } - ], - "responses": { - "200": { - "description": "List of trusted servers", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "data", - "status" - ], - "properties": { - "data": { - "type": "array", - "items": { - "type": "object", - "required": [ - "id", - "status", - "url" - ], - "properties": { - "id": { - "type": "integer", - "format": "int64" - }, - "status": { - "type": "integer", - "format": "int64" - }, - "url": { - "type": "string" - } - } - } - }, - "status": { - "type": "string", - "enum": [ - "ok" - ] - } - } - } - } - } + "statuscode": { + "type": "integer" + }, + "message": { + "type": "string" + }, + "totalitems": { + "type": "string" + }, + "itemsperpage": { + "type": "string" } } - }, + } + } + }, + "paths": { + "/ocs/v2.php/apps/federation/trusted-servers": { "post": { "operationId": "settings-add-server", "summary": "Add server to the list of trusted Nextcloud servers", @@ -153,35 +102,40 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "id", - "message", - "url" + "meta", + "data" ], "properties": { - "id": { - "type": "integer", - "format": "int64" + "meta": { + "$ref": "#/components/schemas/OCSMeta" }, - "message": { - "type": "string" - }, - "url": { - "type": "string" + "data": { + "type": "object", + "required": [ + "id", + "message", + "url" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "message": { + "type": "string" + }, + "url": { + "type": "string" + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "ok" - ] } } } @@ -195,30 +149,31 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "hint", - "message" + "meta", + "data" ], "properties": { - "hint": { - "type": "string" + "meta": { + "$ref": "#/components/schemas/OCSMeta" }, - "message": { - "type": "string" + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "error" - ] } } } @@ -232,30 +187,112 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "hint", - "message" + "meta", + "data" ], "properties": { - "hint": { - "type": "string" + "meta": { + "$ref": "#/components/schemas/OCSMeta" }, - "message": { - "type": "string" + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + }, + "get": { + "operationId": "settings-get-servers", + "summary": "List all trusted servers", + "description": "This endpoint requires admin access", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "List of trusted servers", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "status", + "url" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "status": { + "type": "integer", + "format": "int64" + }, + "url": { + "type": "string" + } + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "error" - ] } } } @@ -311,27 +348,32 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "id" + "meta", + "data" ], "properties": { - "id": { - "type": "integer", - "format": "int64" + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "id" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "ok" - ] } } } @@ -345,26 +387,31 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "message" + "meta", + "data" ], "properties": { - "message": { - "type": "string" + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "error" - ] } } } diff --git a/apps/federation/openapi-full.json b/apps/federation/openapi-full.json index 63f22be74e26a..3b95b3656e889 100644 --- a/apps/federation/openapi-full.json +++ b/apps/federation/openapi-full.json @@ -502,82 +502,6 @@ } }, "/ocs/v2.php/apps/federation/trusted-servers": { - "get": { - "operationId": "settings-get-servers", - "summary": "List all trusted servers", - "description": "This endpoint requires admin access", - "tags": [ - "settings" - ], - "security": [ - { - "bearer_auth": [] - }, - { - "basic_auth": [] - } - ], - "parameters": [ - { - "name": "OCS-APIRequest", - "in": "header", - "description": "Required to be true for the API request to pass", - "required": true, - "schema": { - "type": "boolean", - "default": true - } - } - ], - "responses": { - "200": { - "description": "List of trusted servers", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "data", - "status" - ], - "properties": { - "data": { - "type": "array", - "items": { - "type": "object", - "required": [ - "id", - "status", - "url" - ], - "properties": { - "id": { - "type": "integer", - "format": "int64" - }, - "status": { - "type": "integer", - "format": "int64" - }, - "url": { - "type": "string" - } - } - } - }, - "status": { - "type": "string", - "enum": [ - "ok" - ] - } - } - } - } - } - } - } - }, "post": { "operationId": "settings-add-server", "summary": "Add server to the list of trusted Nextcloud servers", @@ -632,35 +556,40 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "id", - "message", - "url" + "meta", + "data" ], "properties": { - "id": { - "type": "integer", - "format": "int64" - }, - "message": { - "type": "string" + "meta": { + "$ref": "#/components/schemas/OCSMeta" }, - "url": { - "type": "string" + "data": { + "type": "object", + "required": [ + "id", + "message", + "url" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "message": { + "type": "string" + }, + "url": { + "type": "string" + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "ok" - ] } } } @@ -674,30 +603,31 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "hint", - "message" + "meta", + "data" ], "properties": { - "hint": { - "type": "string" + "meta": { + "$ref": "#/components/schemas/OCSMeta" }, - "message": { - "type": "string" + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "error" - ] } } } @@ -711,30 +641,112 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + }, + "get": { + "operationId": "settings-get-servers", + "summary": "List all trusted servers", + "description": "This endpoint requires admin access", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "List of trusted servers", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "hint", - "message" + "meta", + "data" ], "properties": { - "hint": { - "type": "string" + "meta": { + "$ref": "#/components/schemas/OCSMeta" }, - "message": { - "type": "string" + "data": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "status", + "url" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "status": { + "type": "integer", + "format": "int64" + }, + "url": { + "type": "string" + } + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "error" - ] } } } @@ -790,27 +802,32 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "id" + "meta", + "data" ], "properties": { - "id": { - "type": "integer", - "format": "int64" + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "id" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "ok" - ] } } } @@ -824,26 +841,31 @@ "schema": { "type": "object", "required": [ - "data", - "status" + "ocs" ], "properties": { - "data": { + "ocs": { "type": "object", "required": [ - "message" + "meta", + "data" ], "properties": { - "message": { - "type": "string" + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } } } - }, - "status": { - "type": "string", - "enum": [ - "error" - ] } } } diff --git a/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php b/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php index 021c8646cc74e..3da2e2e15e46b 100644 --- a/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php +++ b/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php @@ -20,6 +20,7 @@ use OCP\IConfig; use OCP\IURLGenerator; use OCP\OCS\IDiscoveryService; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; /** @@ -31,35 +32,16 @@ */ class GetSharedSecretTest extends TestCase { - /** @var \PHPUnit\Framework\MockObject\MockObject|IClient */ - private $httpClient; - - /** @var \PHPUnit\Framework\MockObject\MockObject|IClientService */ - private $httpClientService; - - /** @var \PHPUnit\Framework\MockObject\MockObject|IJobList */ - private $jobList; - - /** @var \PHPUnit\Framework\MockObject\MockObject|IURLGenerator */ - private $urlGenerator; - - /** @var \PHPUnit\Framework\MockObject\MockObject|TrustedServers */ - private $trustedServers; - - /** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */ - private $logger; - - /** @var \PHPUnit\Framework\MockObject\MockObject|IResponse */ - private $response; - - /** @var \PHPUnit\Framework\MockObject\MockObject|IDiscoveryService */ - private $discoverService; - - /** @var \PHPUnit\Framework\MockObject\MockObject|ITimeFactory */ - private $timeFactory; - - /** @var \PHPUnit\Framework\MockObject\MockObject|IConfig */ - private $config; + private MockObject&IClient $httpClient; + private MockObject&IClientService $httpClientService; + private MockObject&IJobList $jobList; + private MockObject&IURLGenerator $urlGenerator; + private MockObject&TrustedServers $trustedServers; + private MockObject&LoggerInterface $logger; + private MockObject&IResponse $response; + private MockObject&IDiscoveryService $discoverService; + private MockObject&ITimeFactory $timeFactory; + private MockObject&IConfig $config; private GetSharedSecret $getSharedSecret; @@ -89,7 +71,7 @@ protected function setUp(): void { $this->logger, $this->discoverService, $this->timeFactory, - $this->config + $this->config, ); } diff --git a/apps/federation/tests/Controller/SettingsControllerTest.php b/apps/federation/tests/Controller/SettingsControllerTest.php index 5652e2c055e52..c3e83945e9a8b 100644 --- a/apps/federation/tests/Controller/SettingsControllerTest.php +++ b/apps/federation/tests/Controller/SettingsControllerTest.php @@ -1,5 +1,4 @@ l10n = $this->getMockBuilder(IL10N::class)->getMock(); $this->trustedServers = $this->getMockBuilder(TrustedServers::class) ->disableOriginalConstructor()->getMock(); + $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); $this->controller = new SettingsController( 'SettingsControllerTest', $this->request, $this->l10n, - $this->trustedServers + $this->trustedServers, + $this->logger, ); } @@ -55,12 +56,12 @@ public function testAddServer(): void { ->willReturn(true); $result = $this->controller->addServer('url'); - $this->assertTrue($result instanceof JSONResponse); + $this->assertTrue($result instanceof DataResponse); $data = $result->getData(); $this->assertSame(200, $result->getStatus()); - $this->assertSame('url', $data['data']['url']); - $this->assertArrayHasKey('id', $data['data']); + $this->assertSame('url', $data['url']); + $this->assertArrayHasKey('id', $data); } /** @@ -78,15 +79,13 @@ public function testAddServerFail(bool $isTrustedServer, bool $isNextcloud): voi ->with('url') ->willReturn($isNextcloud); - $result = $this->controller->addServer('url'); - $this->assertTrue($result instanceof JSONResponse); if ($isTrustedServer) { - $this->assertSame(409, $result->getStatus()); + $this->expectException(OCSException::class); + } else { + $this->expectException(OCSNotFoundException::class); } - if (!$isNextcloud) { - $this->assertSame(404, $result->getStatus()); - } + $this->controller->addServer('url'); } public function testRemoveServer(): void { @@ -94,7 +93,7 @@ public function testRemoveServer(): void { ->method('removeServer') ->with(1); $result = $this->controller->removeServer(1); - $this->assertTrue($result instanceof JSONResponse); + $this->assertTrue($result instanceof DataResponse); $this->assertSame(200, $result->getStatus()); } @@ -110,8 +109,8 @@ public function testCheckServer(): void { ->with('url') ->willReturn(true); - $this->assertTrue( - $this->invokePrivate($this->controller, 'checkServer', ['url']) === null + $this->assertNull( + $this->invokePrivate($this->controller, 'checkServer', ['url']) ); } @@ -130,8 +129,14 @@ public function testCheckServerFail(bool $isTrustedServer, bool $isNextcloud): v ->with('url') ->willReturn($isNextcloud); + if ($isTrustedServer) { + $this->expectException(OCSException::class); + } else { + $this->expectException(OCSNotFoundException::class); + } + $this->assertTrue( - $this->invokePrivate($this->controller, 'checkServer', ['url']) instanceof JSONResponse + $this->invokePrivate($this->controller, 'checkServer', ['url']) ); } diff --git a/apps/federation/tests/Middleware/AddServerMiddlewareTest.php b/apps/federation/tests/Middleware/AddServerMiddlewareTest.php deleted file mode 100644 index 0ad619f8114d1..0000000000000 --- a/apps/federation/tests/Middleware/AddServerMiddlewareTest.php +++ /dev/null @@ -1,81 +0,0 @@ -logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); - $this->l10n = $this->getMockBuilder(IL10N::class)->getMock(); - $this->controller = $this->getMockBuilder(SettingsController::class) - ->disableOriginalConstructor()->getMock(); - - $this->middleware = new AddServerMiddleware( - 'AddServerMiddlewareTest', - $this->l10n, - $this->logger - ); - } - - /** - * @dataProvider dataTestAfterException - * - * @param \Exception $exception - * @param string $hint - */ - public function testAfterException($exception, $hint): void { - $this->logger->expects($this->once())->method('error'); - - $this->l10n->expects($this->any())->method('t') - ->willReturnCallback( - function (string $message): string { - return $message; - } - ); - - $result = $this->middleware->afterException($this->controller, 'method', $exception); - - $this->assertSame(Http::STATUS_BAD_REQUEST, - $result->getStatus() - ); - - $data = $result->getData(); - - $this->assertSame($hint, - $data['message'] - ); - } - - public function dataTestAfterException() { - return [ - [new HintException('message', 'hint'), 'hint'], - [new \Exception('message'), 'message'], - ]; - } -} diff --git a/apps/theming/openapi.json b/apps/theming/openapi.json index 7ad7242d74469..9057d26cb69b0 100644 --- a/apps/theming/openapi.json +++ b/apps/theming/openapi.json @@ -20,31 +20,6 @@ } }, "schemas": { - "Background": { - "type": "object", - "required": [ - "backgroundImage", - "backgroundColor", - "primaryColor", - "version" - ], - "properties": { - "backgroundImage": { - "type": "string", - "nullable": true - }, - "backgroundColor": { - "type": "string" - }, - "primaryColor": { - "type": "string" - }, - "version": { - "type": "integer", - "format": "int64" - } - } - }, "OCSMeta": { "type": "object", "required": [ @@ -627,18 +602,6 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "OCS-APIRequest", - "in": "header", - "description": "Required to be true for the API request to pass", - "required": true, - "schema": { - "type": "boolean", - "default": true - } - } - ], "responses": { "200": { "description": "Background image returned", @@ -664,155 +627,6 @@ } } }, - "/index.php/apps/theming/background/{type}": { - "post": { - "operationId": "user_theme-set-background", - "summary": "Set the background", - "tags": [ - "user_theme" - ], - "security": [ - { - "bearer_auth": [] - }, - { - "basic_auth": [] - } - ], - "requestBody": { - "required": false, - "content": { - "application/json": { - "schema": { - "type": "object", - "properties": { - "value": { - "type": "string", - "default": "", - "description": "Path of the background image" - }, - "color": { - "type": "string", - "nullable": true, - "description": "Color for the background" - } - } - } - } - } - }, - "parameters": [ - { - "name": "type", - "in": "path", - "description": "Type of background", - "required": true, - "schema": { - "type": "string" - } - }, - { - "name": "OCS-APIRequest", - "in": "header", - "description": "Required to be true for the API request to pass", - "required": true, - "schema": { - "type": "boolean", - "default": true - } - } - ], - "responses": { - "200": { - "description": "Background set successfully", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/Background" - } - } - } - }, - "400": { - "description": "Setting background is not possible", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "error" - ], - "properties": { - "error": { - "type": "string" - } - } - } - } - } - }, - "500": { - "description": "", - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "error" - ], - "properties": { - "error": { - "type": "string" - } - } - } - } - } - } - } - } - }, - "/index.php/apps/theming/background/custom": { - "delete": { - "operationId": "user_theme-delete-background", - "summary": "Delete the background", - "tags": [ - "user_theme" - ], - "security": [ - { - "bearer_auth": [] - }, - { - "basic_auth": [] - } - ], - "parameters": [ - { - "name": "OCS-APIRequest", - "in": "header", - "description": "Required to be true for the API request to pass", - "required": true, - "schema": { - "type": "boolean", - "default": true - } - } - ], - "responses": { - "200": { - "description": "Background deleted successfully", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/Background" - } - } - } - } - } - } - }, "/ocs/v2.php/apps/theming/api/v1/theme/{themeId}/enable": { "put": { "operationId": "user_theme-enable-theme",