feat: remove ApiScopes (#373)

Ongoing optimization, unnecessary stuff removal to reduce the number of requests during AppAPIAuth. --------- Signed-off-by: Alexander Piskun <[email protected]> Signed-off-by: Andrey Borysenko <[email protected]> Co-authored-by: Alexander Piskun <[email protected]>
nextcloud · Sep 4, 2024 · 1eab9f6 · 1eab9f6
1 parent e9d2442
commit 1eab9f6
Show file tree

Hide file tree

Showing 22 changed files with 50 additions and 442 deletions.
diff --git a/.github/workflows/tests-special.yml b/.github/workflows/tests-special.yml
@@ -108,32 +108,14 @@ jobs:
           sleep 5s
           php occ app_api:daemon:register manual_install "Manual Install" manual-install http localhost 0
           php occ app_api:app:register $APP_ID manual_install --json-info \
-            "{\"appid\":\"$APP_ID\",\"name\":\"$APP_ID\",\"daemon_config_name\":\"manual_install\",\"version\":\"$APP_VERSION\",\"secret\":\"$APP_SECRET\",\"port\":$APP_PORT,\"scopes\":[\"ALL\"],\"system_app\":1}" \
+            "{\"appid\":\"$APP_ID\",\"name\":\"$APP_ID\",\"daemon_config_name\":\"manual_install\",\"version\":\"$APP_VERSION\",\"secret\":\"$APP_SECRET\",\"port\":$APP_PORT}" \
             --force-scopes --wait-finish
           kill -15 $(cat /tmp/_install.pid)
           timeout 3m tail --pid=$(cat /tmp/_install.pid) -f /dev/null
 
       - name: Check logs
         run: grep -q 'Hello from ' data/nextcloud.log || error
 
-      - name: Test ALL Scope
-        run: python3 apps/${{ env.APP_NAME }}/tests/auth_scopes_system.py 0
-
-      - name: Re-Register App
-        run: |
-          php occ app_api:app:unregister $APP_ID --silent --force || true
-          python3 apps/${{ env.APP_NAME }}/tests/install_no_init.py &
-          echo $! > /tmp/_install.pid
-          sleep 5s
-          php occ app_api:app:register $APP_ID manual_install --json-info \
-            "{\"appid\":\"$APP_ID\",\"name\":\"$APP_ID\",\"daemon_config_name\":\"manual_install\",\"version\":\"$APP_VERSION\",\"secret\":\"$APP_SECRET\",\"port\":$APP_PORT,\"scopes\":[\"SYSTEM\"],\"system_app\":1}" \
-            --force-scopes --wait-finish
-          kill -15 $(cat /tmp/_install.pid)
-          timeout 3m tail --pid=$(cat /tmp/_install.pid) -f /dev/null
-
-      - name: Test NO ALL Scope
-        run: python3 apps/${{ env.APP_NAME }}/tests/auth_scopes_system.py 1
-
       - name: Upload NC logs
         if: always()
         uses: actions/upload-artifact@v3

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,28 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [3.2.0 - 2024-09-06]
+
+### Added
+
+- ExAppProxy: added bruteforce protection option for ExApp routes. #368
+
+### Changed
+
+- AppAPIAuth optimization: use throttler only when needed to lower the number of requests. #369
+- ExAppProxy: the order of checks of the ExApp routes was changed. #366
+- ExAppProxy: improve logic and logging with more explicit messages. #365
+- Drop support for Nextcloud 27. #374
+
+### Fixed
+
+- TaskProcessing: fixed bug when provider wasn't removed on unregister. #370
+
+### Removed
+
+- ApiScopes are deprecated and removed. #373
+
+
 ## [3.1.0 - 2024-08-15]
 
 **Breaking change**: Task processing API for NC30 AI API. (llm2 and translate2 apps) 

diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -43,7 +43,7 @@ to join us in shaping a more versatile, stable, and secure app landscape.
 *Your insights, suggestions, and contributions are invaluable to us.*
 
 	]]></description>
-	<version>3.1.1</version>
+	<version>3.2.0</version>
 	<licence>agpl</licence>
 	<author mail="[email protected]" homepage="https://github.com/andrey18106">Andrey Borysenko</author>
 	<author mail="[email protected]" homepage="https://github.com/bigcat88">Alexander Piskun</author>
@@ -88,7 +88,6 @@ to join us in shaping a more versatile, stable, and secure app landscape.
 		<command>OCA\AppAPI\Command\ExApp\Disable</command>
 		<command>OCA\AppAPI\Command\ExApp\ListExApps</command>
 		<command>OCA\AppAPI\Command\ExApp\Notify</command>
-		<command>OCA\AppAPI\Command\ExApp\Scopes\ListScopes</command>
 		<command>OCA\AppAPI\Command\ExAppConfig\GetConfig</command>
 		<command>OCA\AppAPI\Command\ExAppConfig\SetConfig</command>
 		<command>OCA\AppAPI\Command\ExAppConfig\DeleteConfig</command>

diff --git a/docs/notes_for_developers/ExAppOverview.rst b/docs/notes_for_developers/ExAppOverview.rst
@@ -41,7 +41,7 @@ It should contain the following fields:
 			<image>cloud-py-api/skeleton</image>
 			<image-tag>latest</image-tag>
 		</docker-install>
-		<scopes>
+		<scopes> // deprecated and removed since AppAPI 3.2.0
 			<value>FILES</value>
 			<value>AI_PROVIDERS</value>
 			...

diff --git a/docs/tech_details/ApiScopes.rst b/docs/tech_details/ApiScopes.rst
@@ -3,6 +3,10 @@
 Api Scopes
 ==========
 
+.. warning::
+
+	ApiScopes are deprecated and removed since AppAPI 3.2.0.
+
 AppAPI design's focus on simplicity and necessity highlights the benefits of defining API scopes.
 which simplify the development and integration of applications into the Nextcloud ecosystem.
 

diff --git a/docs/tech_details/Authentication.rst b/docs/tech_details/Authentication.rst
@@ -63,8 +63,6 @@ Authentication flow in details
 		AppAPI-->>Nextcloud: Reject if ExApp not exists or disabled
 		AppAPI-->>AppAPI: Validate shared secret from AUTHORIZATION-APP-API
 		AppAPI-->>Nextcloud: Reject if secret does not match
-		AppAPI-->>AppAPI: Check API scope
-		AppAPI-->>Nextcloud: Reject if API scope not match
 		AppAPI-->>AppAPI: Check if user is not empty and active
 		AppAPI-->>Nextcloud: Set active user
 		AppAPI->>-Nextcloud: Request accepted/rejected

diff --git a/docs/tech_details/Deployment.rst b/docs/tech_details/Deployment.rst
@@ -150,7 +150,7 @@ It has the same structure as Nextcloud appinfo/info.xml file, but with some addi
 			<image>cloud-py-api/talk_bot</image>
 			<image-tag>latest</image-tag>
 		</docker-install>
-		<scopes>
+		<scopes> // deprecated since AppAPI 3.2.0
 			<value>TALK</value>
 			<value>TALK_BOT</value>
 		</scopes>

diff --git a/lib/Command/ExApp/Enable.php b/lib/Command/ExApp/Enable.php
@@ -44,9 +44,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			$output->writeln(sprintf('ExApp %s successfully enabled.', $appId));
 			return 0;
 		}
-
-		// TODO: Add scopes check (from info.xml) and approval if needed
-
 		$output->writeln(sprintf('Failed to enable ExApp %s.', $appId));
 		return 1;
 	}

diff --git a/lib/Command/ExApp/Register.php b/lib/Command/ExApp/Register.php
@@ -10,26 +10,22 @@
 use OCA\AppAPI\Fetcher\ExAppArchiveFetcher;
 use OCA\AppAPI\Service\AppAPIService;
 use OCA\AppAPI\Service\DaemonConfigService;
-use OCA\AppAPI\Service\ExAppApiScopeService;
 use OCA\AppAPI\Service\ExAppService;
 
 use OCP\IConfig;
 use OCP\Security\ISecureRandom;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Console\Command\Command;
-use Symfony\Component\Console\Helper\QuestionHelper;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
-use Symfony\Component\Console\Question\ConfirmationQuestion;
 
 class Register extends Command {
 
 	public function __construct(
 		private readonly AppAPIService  	  $service,
 		private readonly DaemonConfigService  $daemonConfigService,
-		private readonly ExAppApiScopeService $exAppApiScopeService,
 		private readonly DockerActions        $dockerActions,
 		private readonly ManualActions        $manualActions,
 		private readonly IConfig              $config,
@@ -48,7 +44,7 @@ protected function configure(): void {
 		$this->addArgument('appid', InputArgument::REQUIRED);
 		$this->addArgument('daemon-config-name', InputArgument::OPTIONAL);
 
-		$this->addOption('force-scopes', null, InputOption::VALUE_NONE, 'Force scopes approval');
+		$this->addOption('force-scopes', null, InputOption::VALUE_NONE, 'Force scopes approval[deprecated]');
 		$this->addOption('info-xml', null, InputOption::VALUE_REQUIRED, 'Path to ExApp info.xml file (url or local absolute path)');
 		$this->addOption('json-info', null, InputOption::VALUE_REQUIRED, 'ExApp info.xml in JSON format');
 		$this->addOption('wait-finish', null, InputOption::VALUE_NONE, 'Wait until finish');
@@ -109,32 +105,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			return 2;
 		}
 
-		$confirmRequiredScopes = (bool) $input->getOption('force-scopes');
-		if (!$confirmRequiredScopes && $input->isInteractive()) {
-			/** @var QuestionHelper $helper */
-			$helper = $this->getHelper('question');
-
-			// Prompt to approve required ExApp scopes
-			if (count($appInfo['external-app']['scopes']) > 0) {
-				$output->writeln(
-					sprintf('ExApp %s requested required scopes: %s', $appId, implode(', ', $appInfo['external-app']['scopes']))
-				);
-				$question = new ConfirmationQuestion('Do you want to approve it? [y/N] ', false);
-				$confirmRequiredScopes = $helper->ask($input, $output, $question);
-			} else {
-				$confirmRequiredScopes = true;
-			}
-		}
-
-		if (!$confirmRequiredScopes && count($appInfo['external-app']['scopes']) > 0) {
-			$output->writeln(sprintf('ExApp %s required scopes not approved.', $appId));
-			return 1;
-		}
-
 		$appInfo['port'] = $appInfo['port'] ?? $this->exAppService->getExAppFreePort();
 		$appInfo['secret'] = $appInfo['secret'] ?? $this->random->generate(128);
 		$appInfo['daemon_config_name'] = $appInfo['daemon_config_name'] ?? $daemonConfigName;
-		$appInfo['api_scopes'] = array_values($this->exAppApiScopeService->mapScopeGroupsToNumbers($appInfo['external-app']['scopes']));
 		$exApp = $this->exAppService->registerExApp($appInfo);
 		if (!$exApp) {
 			$this->logger->error(sprintf('Error during registering ExApp %s.', $appId));
@@ -143,16 +116,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			}
 			return 3;
 		}
-		if (count($appInfo['external-app']['scopes']) > 0) {
-			$this->logger->info(
-				sprintf('ExApp %s scope groups successfully set: %s', $exApp->getAppid(), implode(', ', $appInfo['external-app']['scopes']))
-			);
-			if ($outputConsole) {
-				$output->writeln(
-					sprintf('ExApp %s scope groups successfully set: %s', $exApp->getAppid(), implode(', ', $appInfo['external-app']['scopes']))
-				);
-			}
-		}
 
 		if (!empty($appInfo['external-app']['translations_folder'])) {
 			$result = $this->exAppArchiveFetcher->installTranslations($appId, $appInfo['external-app']['translations_folder']);

diff --git a/lib/Command/ExApp/Scopes/ListScopes.php b/lib/Command/ExApp/Scopes/ListScopes.php
diff --git a/lib/Command/ExApp/Update.php b/lib/Command/ExApp/Update.php
@@ -10,24 +10,20 @@
 use OCA\AppAPI\Fetcher\ExAppFetcher;
 use OCA\AppAPI\Service\AppAPIService;
 use OCA\AppAPI\Service\DaemonConfigService;
-use OCA\AppAPI\Service\ExAppApiScopeService;
 
 use OCA\AppAPI\Service\ExAppService;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Console\Command\Command;
-use Symfony\Component\Console\Helper\QuestionHelper;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
-use Symfony\Component\Console\Question\ConfirmationQuestion;
 
 class Update extends Command {
 
 	public function __construct(
 		private readonly AppAPIService  	  $service,
 		private readonly ExAppService         $exAppService,
-		private readonly ExAppApiScopeService $exAppApiScopeService,
 		private readonly DaemonConfigService  $daemonConfigService,
 		private readonly DockerActions        $dockerActions,
 		private readonly ManualActions        $manualActions,
@@ -46,7 +42,7 @@ protected function configure(): void {
 
 		$this->addOption('info-xml', null, InputOption::VALUE_REQUIRED, 'Path to ExApp info.xml file (url or local absolute path)');
 		$this->addOption('json-info', null, InputOption::VALUE_REQUIRED, 'ExApp info.xml in JSON format');
-		$this->addOption('force-scopes', null, InputOption::VALUE_NONE, 'Force new ExApp scopes approval');
+		$this->addOption('force-scopes', null, InputOption::VALUE_NONE, 'Force new ExApp scopes approval[deprecated]');
 		$this->addOption('wait-finish', null, InputOption::VALUE_NONE, 'Wait until finish');
 		$this->addOption('silent', null, InputOption::VALUE_NONE, 'Do not print to console');
 		$this->addOption('all', null, InputOption::VALUE_NONE, 'Update all updatable apps');
@@ -138,30 +134,6 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str
 			return 0;
 		}
 
-		// Default scopes approval process (compare new ExApp scopes)
-		$currentExAppScopes = $exApp->getApiScopes();
-		// Prepare for prompt of newly requested ExApp scopes
-		$requiredScopes = array_values(array_diff($this->exAppApiScopeService->mapScopeGroupsToNumbers($appInfo['external-app']['scopes']), $currentExAppScopes));
-
-		$confirmScopes = (bool) $input->getOption('force-scopes');
-		if (!$confirmScopes && $input->isInteractive()) {
-			/** @var QuestionHelper $helper */
-			$helper = $this->getHelper('question');
-
-			if (count($requiredScopes) > 0) {
-				$output->writeln(sprintf('ExApp %s requested scopes: %s', $appId, implode(', ',
-					$this->exAppApiScopeService->mapScopeGroupsToNames($requiredScopes))));
-				$question = new ConfirmationQuestion('Do you want to approve it? [y/N] ', false);
-				$confirmScopes = $helper->ask($input, $output, $question);
-			} else {
-				$confirmScopes = true;
-			}
-		}
-		if (!$confirmScopes && count($requiredScopes) > 0) {
-			$output->writeln(sprintf('ExApp %s required scopes not approved. Failed to finish ExApp update.', $appId));
-			return 1;
-		}
-
 		$status = $exApp->getStatus();
 		$status['type'] = 'update';
 		$status['error'] = '';
@@ -197,7 +169,6 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str
 			}
 		}
 
-		$appInfo['api_scopes'] = array_values($this->exAppApiScopeService->mapScopeGroupsToNumbers($appInfo['external-app']['scopes']));
 		if (!$this->exAppService->updateExAppInfo($exApp, $appInfo)) {
 			$this->logger->error(sprintf('Failed to update ExApp %s info', $appId));
 			if ($outputConsole) {

diff --git a/lib/Controller/DaemonConfigController.php b/lib/Controller/DaemonConfigController.php
@@ -125,7 +125,7 @@ public function startTestDeploy(string $name): Response {
 		}
 
 		if (!$this->service->runOccCommand(
-			sprintf("app_api:app:register --force-scopes --silent %s %s --info-xml %s --test-deploy-mode",
+			sprintf("app_api:app:register --silent %s %s --info-xml %s --test-deploy-mode",
 				Application::TEST_DEPLOY_APPID, $daemonConfig->getName(), Application::TEST_DEPLOY_INFO_XML)
 		)) {
 			return new JSONResponse(['error' => $this->l10n->t('Error starting install of ExApp')], Http::STATUS_INTERNAL_SERVER_ERROR);