From 333547424c205f832f91f42d29cc171f3c5e961d Mon Sep 17 00:00:00 2001 From: Alexander Piskun <13381981+bigcat88@users.noreply.github.com> Date: Thu, 22 Aug 2024 23:32:32 +0300 Subject: [PATCH 1/7] feat: started removing of api scopes Signed-off-by: Alexander Piskun --- .github/workflows/tests-special.yml | 20 +--- appinfo/info.xml | 1 - lib/Command/ExApp/Enable.php | 3 - lib/Command/ExApp/Register.php | 39 +------- lib/Command/ExApp/Scopes/ListScopes.php | 50 ---------- lib/Command/ExApp/Update.php | 31 +----- lib/Controller/DaemonConfigController.php | 2 +- lib/Controller/ExAppsPageController.php | 15 +-- lib/Db/ExApp.php | 10 -- lib/Db/ExAppMapper.php | 3 - lib/Service/AppAPIService.php | 52 +++------- lib/Service/ExAppApiScopeService.php | 112 ---------------------- lib/Service/ExAppService.php | 18 +--- tests/auth_scopes_system.py | 25 ----- 14 files changed, 20 insertions(+), 361 deletions(-) delete mode 100644 lib/Command/ExApp/Scopes/ListScopes.php delete mode 100644 lib/Service/ExAppApiScopeService.php delete mode 100644 tests/auth_scopes_system.py diff --git a/.github/workflows/tests-special.yml b/.github/workflows/tests-special.yml index 461eb200..271e4fba 100644 --- a/.github/workflows/tests-special.yml +++ b/.github/workflows/tests-special.yml @@ -108,7 +108,7 @@ 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 @@ -116,24 +116,6 @@ jobs: - 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/appinfo/info.xml b/appinfo/info.xml index 259a2889..b950ccbe 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -88,7 +88,6 @@ to join us in shaping a more versatile, stable, and secure app landscape. OCA\AppAPI\Command\ExApp\Disable OCA\AppAPI\Command\ExApp\ListExApps OCA\AppAPI\Command\ExApp\Notify - OCA\AppAPI\Command\ExApp\Scopes\ListScopes OCA\AppAPI\Command\ExAppConfig\GetConfig OCA\AppAPI\Command\ExAppConfig\SetConfig OCA\AppAPI\Command\ExAppConfig\DeleteConfig diff --git a/lib/Command/ExApp/Enable.php b/lib/Command/ExApp/Enable.php index b68169f3..cb9f50af 100644 --- 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 index 014030e7..87b6b71a 100644 --- 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 deleted file mode 100644 index 3a10dbe0..00000000 --- a/lib/Command/ExApp/Scopes/ListScopes.php +++ /dev/null @@ -1,50 +0,0 @@ -setName('app_api:app:scopes:list'); - $this->setDescription('List ExApp granted scopes'); - - $this->addArgument('appid', InputArgument::REQUIRED); - } - - protected function execute(InputInterface $input, OutputInterface $output): int { - $appId = $input->getArgument('appid'); - $exApp = $this->service->getExApp($appId); - if ($exApp === null) { - $output->writeln(sprintf('ExApp %s not found.', $appId)); - return 2; - } - - $scopes = $exApp->getApiScopes(); - if (empty($scopes)) { - $output->writeln(sprintf('No scopes granted for ExApp %s', $appId)); - return 0; - } - - $output->writeln(sprintf('ExApp %s scopes:', $exApp->getAppid())); - $mappedScopes = array_unique($this->exAppApiScopeService->mapScopeGroupsToNames($scopes)); - $output->writeln(join(', ', $mappedScopes)); - return 0; - } -} diff --git a/lib/Command/ExApp/Update.php b/lib/Command/ExApp/Update.php index 296688d6..7280d69b 100644 --- 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 index 2ea1a904..7b2cfd83 100644 --- 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); diff --git a/lib/Controller/ExAppsPageController.php b/lib/Controller/ExAppsPageController.php index 1dade990..8d3efd14 100644 --- a/lib/Controller/ExAppsPageController.php +++ b/lib/Controller/ExAppsPageController.php @@ -16,7 +16,6 @@ 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 OCP\App\IAppManager; use OCP\AppFramework\Controller; @@ -46,7 +45,6 @@ class ExAppsPageController extends Controller { private CategoryFetcher $categoryFetcher; private IFactory $l10nFactory; private ExAppFetcher $exAppFetcher; - private ExAppApiScopeService $exAppApiScopeService; private IL10N $l10n; private LoggerInterface $logger; private IAppManager $appManager; @@ -57,7 +55,6 @@ public function __construct( IInitialState $initialStateService, AppAPIService $service, DaemonConfigService $daemonConfigService, - ExAppApiScopeService $exAppApiScopeService, DockerActions $dockerActions, CategoryFetcher $categoryFetcher, IFactory $l10nFactory, @@ -73,7 +70,6 @@ public function __construct( $this->config = $config; $this->service = $service; $this->daemonConfigService = $daemonConfigService; - $this->exAppApiScopeService = $exAppApiScopeService; $this->dockerActions = $dockerActions; $this->categoryFetcher = $categoryFetcher; $this->l10nFactory = $l10nFactory; @@ -192,11 +188,8 @@ private function getAppsForCategory(string $requestedCategory = ''): array { $currentVersion = $app['releases'][0]['version']; } - $scopes = null; $daemon = null; - if ($exApp !== null) { - $scopes = $this->exAppApiScopeService->mapScopeGroupsToNames($exApp->getApiScopes()); $daemon = $this->daemonConfigService->getDaemonConfigByName($exApp->getDaemonConfigName()); } @@ -237,7 +230,6 @@ private function getAppsForCategory(string $requestedCategory = ''): array { 'needsDownload' => !$existsLocally, 'fromAppStore' => true, 'appstoreData' => $app, - 'scopes' => $scopes, 'daemon' => $daemon, 'status' => $exApp !== null ? $exApp->getStatus() : [], 'error' => $exApp !== null ? $exApp->getStatus()['error'] ?? '' : '', @@ -330,7 +322,6 @@ private function buildLocalAppsList(array $apps, array $exApps): array { if (!in_array($app['id'], $registeredAppsIds)) { $exApp = $this->exAppService->getExApp($app['id']); $daemon = $this->daemonConfigService->getDaemonConfigByName($exApp->getDaemonConfigName()); - $scopes = $this->exAppApiScopeService->mapScopeGroupsToNames($exApp->getApiScopes()); $formattedLocalApps[] = [ 'id' => $app['id'], @@ -368,7 +359,6 @@ private function buildLocalAppsList(array $apps, array $exApps): array { 'needsDownload' => false, 'fromAppStore' => false, 'appstoreData' => $app, - 'scopes' => $scopes, 'daemon' => $daemon, 'exAppUrl' => $this->service->getExAppUrl($exApp, $exApp->getPort(), $auth), 'releases' => [], @@ -387,7 +377,7 @@ public function enableApp(string $appId): JSONResponse { $exApp = $this->exAppService->getExApp($appId); // If ExApp is not registered - then it's a "Deploy and Enable" action. if (!$exApp) { - if (!$this->service->runOccCommand(sprintf("app_api:app:register --force-scopes --silent %s", $appId))) { + if (!$this->service->runOccCommand(sprintf("app_api:app:register --silent %s", $appId))) { return new JSONResponse(['data' => ['message' => $this->l10n->t('Error starting install of ExApp')]], Http::STATUS_INTERNAL_SERVER_ERROR); } $elapsedTime = 0; @@ -432,7 +422,6 @@ public function disableApp(string $appId): JSONResponse { /** * Default forced ExApp update process. * Update approval via password confirmation. - * Scopes approval does not applied in UI for now. */ #[PasswordConfirmationRequired] #[NoCSRFRequired] @@ -446,7 +435,7 @@ public function updateApp(string $appId): JSONResponse { } $exAppOldVersion = $this->exAppService->getExApp($appId)->getVersion(); - if (!$this->service->runOccCommand(sprintf("app_api:app:update --force-scopes --silent %s", $appId))) { + if (!$this->service->runOccCommand(sprintf("app_api:app:update --silent %s", $appId))) { return new JSONResponse(['data' => ['message' => $this->l10n->t('Error starting update of ExApp')]], Http::STATUS_INTERNAL_SERVER_ERROR); } diff --git a/lib/Db/ExApp.php b/lib/Db/ExApp.php index 576a1d46..13f087ad 100644 --- a/lib/Db/ExApp.php +++ b/lib/Db/ExApp.php @@ -24,7 +24,6 @@ * @method int getEnabled() * @method int getCreatedTime() * @method int getLastCheckTime() - * @method array getApiScopes() * @method array getDeployConfig() * @method string getAcceptsDeployId() * @method array getRoutes() @@ -40,7 +39,6 @@ * @method void setEnabled(int $enabled) * @method void setCreatedTime(int $createdTime) * @method void setLastCheckTime(int $lastCheckTime) - * @method void setApiScopes(array $apiScopes) * @method void setDeployConfig(array $deployConfig) * @method void setAcceptsDeployId(string $acceptsDeployId) * @method void setRoutes(array $routes) @@ -58,7 +56,6 @@ class ExApp extends Entity implements JsonSerializable { protected $enabled; protected $createdTime; protected $lastCheckTime; - protected $apiScopes; protected $deployConfig; protected $acceptsDeployId; protected $routes; @@ -79,7 +76,6 @@ public function __construct(array $params = []) { $this->addType('enabled', 'int'); $this->addType('createdTime', 'int'); $this->addType('lastCheckTime', 'int'); - $this->addType('apiScopes', 'json'); $this->addType('deployConfig', 'json'); $this->addType('acceptsDeployId', 'string'); $this->addType('routes', 'json'); @@ -123,11 +119,6 @@ public function __construct(array $params = []) { if (isset($params['last_check_time'])) { $this->setLastCheckTime($params['last_check_time']); } - if (isset($params['api_scopes'])) { - $this->setApiScopes($params['api_scopes']); - } else { - $this->setApiScopes([]); - } if (isset($params['deploy_config'])) { $this->setDeployConfig($params['deploy_config']); } @@ -154,7 +145,6 @@ public function jsonSerialize(): array { 'enabled' => $this->getEnabled(), 'created_time' => $this->getCreatedTime(), 'last_check_time' => $this->getLastCheckTime(), - 'api_scopes' => $this->getApiScopes(), 'deploy_config' => $this->getDeployConfig(), 'accepts_deploy_id' => $this->getAcceptsDeployId(), 'routes' => $this->getRoutes(), diff --git a/lib/Db/ExAppMapper.php b/lib/Db/ExAppMapper.php index 43c60335..6b9d2ec6 100644 --- a/lib/Db/ExAppMapper.php +++ b/lib/Db/ExAppMapper.php @@ -114,7 +114,6 @@ private function buildExAppWithRoutes(array $result): array { 'enabled' => $row['enabled'], 'created_time' => $row['created_time'], 'last_check_time' => $row['last_check_time'], - 'api_scopes' => $row['api_scopes'], 'deploy_config' => $row['deploy_config'], 'accepts_deploy_id' => $row['accepts_deploy_id'], 'routes' => [], @@ -184,8 +183,6 @@ public function updateExApp(ExApp $exApp, array $fields): int { $qb = $qb->set('enabled', $qb->createNamedParameter($exApp->getEnabled(), IQueryBuilder::PARAM_INT)); } elseif ($field === 'last_check_time') { $qb = $qb->set('last_check_time', $qb->createNamedParameter($exApp->getLastCheckTime(), IQueryBuilder::PARAM_INT)); - } elseif ($field === 'api_scopes') { - $qb = $qb->set('api_scopes', $qb->createNamedParameter($exApp->getApiScopes(), IQueryBuilder::PARAM_JSON)); } } return $qb->where($qb->expr()->eq('appid', $qb->createNamedParameter($exApp->getAppid())))->executeStatement(); diff --git a/lib/Service/AppAPIService.php b/lib/Service/AppAPIService.php index f101078d..d09c2d34 100644 --- a/lib/Service/AppAPIService.php +++ b/lib/Service/AppAPIService.php @@ -40,7 +40,6 @@ public function __construct( private readonly IUserManager $userManager, private readonly IFactory $l10nFactory, private readonly ExAppService $exAppService, - private readonly ExAppApiScopeService $exAppApiScopeService, private readonly DockerActions $dockerActions, private readonly ManualActions $manualActions, private readonly AppAPICommonService $commonService, @@ -277,7 +276,6 @@ private function getUriEncodedParams(array $params): string { * - checks if ExApp exists and is enabled * - checks if ExApp version changed and updates it in database * - checks if ExApp shared secret valid - * - checks ExApp scopes <-> ExApp API copes * * More info in docs: https://cloud-py-api.github.io/app_api/authentication.html */ @@ -316,36 +314,14 @@ public function validateExAppRequestToNC(IRequest $request, bool $isDav = false) $this->logger->error(sprintf('Error getting path info. Error: %s', $e->getMessage()), ['exception' => $e]); return false; } - } else { - $path = '/dav/'; - } - - if (($this->exAppApiScopeService->sanitizeOcsRoute($path) !== '/apps/app_api/ex-app/state') && !$exApp->getEnabled()) { - $this->logger->error(sprintf('ExApp with appId %s is disabled (%s)', $request->getHeader('EX-APP-ID'), $request->getRequestUri())); - return false; + if (($this->sanitizeOcsRoute($path) !== '/apps/app_api/ex-app/state') && !$exApp->getEnabled()) { + $this->logger->error(sprintf('ExApp with appId %s is disabled (%s)', $request->getHeader('EX-APP-ID'), $request->getRequestUri())); + return false; + } } if (!$this->handleExAppVersionChange($request, $exApp)) { return false; } - - $allScopesFlag = (bool)$this->getByScope($exApp, ExAppApiScopeService::ALL_API_SCOPE); - $apiScope = $this->exAppApiScopeService->getApiScopeByRoute($path); - - if (!$allScopesFlag) { - if ($apiScope === null) { - $this->logger->error(sprintf('Failed to check apiScope %s', $path)); - return false; - } - - // BASIC ApiScope is granted to all ExApps (all API routes with BASIC scope group). - if ($apiScope['scope_group'] !== ExAppApiScopeService::BASIC_API_SCOPE) { - if (!$this->passesScopeCheck($exApp, $apiScope['scope_group'])) { - $this->logger->error(sprintf('ExApp %s not passed scope group check %s', $exApp->getAppid(), $path)); - return false; - } - } - } - return $this->finalizeRequestToNC($exApp, $userId, $request, $delay); } else { $this->logger->error(sprintf('Invalid signature for ExApp: %s and user: %s.', $exApp->getAppid(), $userId !== '' ? $userId : 'null')); @@ -388,20 +364,14 @@ private function finalizeRequestToNC(ExApp $exApp, string $userId, IRequest $req return true; } - public function getByScope(ExApp $exApp, int $apiScope): ?int { - foreach ($exApp->getApiScopes() as $scope) { - if ($scope === $apiScope) { - return $scope; - } - } - return null; - } - - public function passesScopeCheck(ExApp $exApp, int $apiScope): bool { - if (in_array($apiScope, $exApp->getApiScopes(), true)) { - return true; + /** + * Check if the given route has ocs prefix and cut it off + */ + private function sanitizeOcsRoute(string $route): string { + if (preg_match("/\/ocs\/v([12])\.php/", $route, $matches)) { + return str_replace($matches[0], '', $route); } - return false; + return $route; } private function getCustomLogger(string $name): LoggerInterface { diff --git a/lib/Service/ExAppApiScopeService.php b/lib/Service/ExAppApiScopeService.php deleted file mode 100644 index 6b4d4197..00000000 --- a/lib/Service/ExAppApiScopeService.php +++ /dev/null @@ -1,112 +0,0 @@ -apiScopes = [ - // AppAPI scopes - ['api_route' => $aeApiV1Prefix . '/ui/files-actions-menu', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV2Prefix . '/ui/files-actions-menu', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/ui/top-menu', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/ui/initial-state', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/ui/script', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/ui/style', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/ui/settings', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/log', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/ex-app/config', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/ex-app/preference', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/routes', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => $aeApiV1Prefix . '/users', 'scope_group' => 2, 'name' => 'SYSTEM'], - ['api_route' => $aeApiV1Prefix . '/ex-app/all', 'scope_group' => 2, 'name' => 'SYSTEM'], - ['api_route' => $aeApiV1Prefix . '/ex-app/enabled', 'scope_group' => 2, 'name' => 'SYSTEM'], - ['api_route' => $aeApiV1Prefix . '/notification', 'scope_group' => 32, 'name' => 'NOTIFICATIONS'], - ['api_route' => $aeApiV1Prefix . '/talk_bot', 'scope_group' => 60, 'name' => 'TALK_BOT'], - ['api_route' => $aeApiV1Prefix . '/ai_provider/', 'scope_group' => 61, 'name' => 'AI_PROVIDERS'], - ['api_route' => $aeApiV1Prefix . '/events_listener', 'scope_group' => 62, 'name' => 'EVENTS_LISTENER'], - ['api_route' => $aeApiV1Prefix . '/occ_command', 'scope_group' => 63, 'name' => 'OCC_COMMAND'], - - // AppAPI internal scopes - ['api_route' => '/apps/app_api/apps/status', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => '/apps/app_api/ex-app/status', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => '/apps/app_api/ex-app/state', 'scope_group' => 1, 'name' => 'BASIC'], - - // Cloud scopes - ['api_route' => '/cloud/capabilities', 'scope_group' => 1, 'name' => 'BASIC'], - ['api_route' => '/cloud/apps', 'scope_group' => 2, 'name' => 'SYSTEM'], - ['api_route' => '/apps/provisioning_api/api/', 'scope_group' => 2, 'name' => 'SYSTEM'], - ['api_route' => '/settings/admin/log/', 'scope_group' => 2, 'name' => 'SYSTEM'], - ['api_route' => '/dav/', 'scope_group' => 10, 'name' => 'FILES'], - ['api_route' => '/apps/files/ajax/', 'scope_group' => 10, 'name' => 'FILES'], - ['api_route' => '/apps/files_sharing/api/', 'scope_group' => 11, 'name' => 'FILES_SHARING'], - ['api_route' => '/cloud/user', 'scope_group' => 30, 'name' => 'USER_INFO'], - ['api_route' => '/cloud/groups', 'scope_group' => 30, 'name' => 'USER_INFO'], - ['api_route' => '/apps/user_status/api/', 'scope_group' => 31, 'name' => 'USER_STATUS'], - ['api_route' => '/apps/notifications/api/', 'scope_group' => 32, 'name' => 'NOTIFICATIONS'], - ['api_route' => '/apps/weather_status/api/', 'scope_group' => 33, 'name' => 'WEATHER_STATUS'], - ['api_route' => '/apps/spreed/api/', 'scope_group' => 50, 'name' => 'TALK'], - ['api_route' => '/taskprocessing/', 'scope_group' => 61, 'name' => 'AI_PROVIDERS'], - ['api_route' => '/apps/activity/api/', 'scope_group' => 110, 'name' => 'ACTIVITIES'], - ['api_route' => '/apps/notes/api/', 'scope_group' => 120, 'name' => 'NOTES'], - ['api_route' => '/textprocessing/', 'scope_group' => 200, 'name' => 'TEXT_PROCESSING'], - ['api_route' => '/translation/', 'scope_group' => 210, 'name' => 'MACHINE_TRANSLATION'], - - //ALL Scope - ['api_route' => 'non-exist-all-api-route', 'scope_group' => self::ALL_API_SCOPE, 'name' => 'ALL'], - ]; - } - - public function getApiScopeByRoute(string $apiRoute): ?array { - foreach ($this->apiScopes as $apiScope) { - if (str_starts_with($this->sanitizeOcsRoute($apiRoute), $apiScope['api_route'])) { - return $apiScope; - } - } - return null; - } - - /** - * Check if the given route has ocs prefix and cut it off - */ - public function sanitizeOcsRoute(string $route): string { - if (preg_match("/\/ocs\/v([12])\.php/", $route, $matches)) { - return str_replace($matches[0], '', $route); - } - return $route; - } - - /** - * @param int[] $scopeGroups - * - * @return string[] - */ - public function mapScopeGroupsToNames(array $scopeGroups): array { - $apiScopes = array_values(array_filter($this->apiScopes, function (array $apiScope) use ($scopeGroups) { - return in_array($apiScope['scope_group'], $scopeGroups); - })); - return array_unique(array_map(function (array $apiScope) { - return $apiScope['name']; - }, $apiScopes)); - } - - public function mapScopeGroupsToNumbers(array $scopeGroups): array { - $apiScopes = array_values(array_filter($this->apiScopes, function (array $apiScope) use ($scopeGroups) { - return in_array($apiScope['name'], $scopeGroups); - })); - return array_unique(array_map(function (array $apiScope) { - return $apiScope['scope_group']; - }, $apiScopes)); - } -} diff --git a/lib/Service/ExAppService.php b/lib/Service/ExAppService.php index 204d90ec..112336da 100644 --- a/lib/Service/ExAppService.php +++ b/lib/Service/ExAppService.php @@ -40,7 +40,6 @@ public function __construct( private readonly ExAppFetcher $exAppFetcher, private readonly ExAppArchiveFetcher $exAppArchiveFetcher, private readonly ExAppMapper $exAppMapper, - private readonly ExAppApiScopeService $exAppApiScopeService, private readonly TopMenuService $topMenuService, private readonly InitialStateService $initialStateService, private readonly ScriptsService $scriptsService, @@ -89,7 +88,6 @@ public function registerExApp(array $appInfo): ?ExApp { 'status' => json_encode(['deploy' => 0, 'init' => 0, 'action' => '', 'type' => 'install', 'error' => '']), 'created_time' => time(), 'last_check_time' => time(), - 'api_scopes' => $appInfo['api_scopes'], ]); try { $this->exAppMapper->insert($exApp); @@ -194,9 +192,7 @@ public function formatExAppInfo(ExApp $exApp): array { 'version' => $exApp->getVersion(), 'enabled' => filter_var($exApp->getEnabled(), FILTER_VALIDATE_BOOLEAN), 'last_check_time' => $exApp->getLastCheckTime(), - 'system' => true, // TODO: Remove later 'status' => $exApp->getStatus(), - 'scopes' => $this->exAppApiScopeService->mapScopeGroupsToNames($exApp->getApiScopes()), ]; } @@ -209,14 +205,13 @@ public function getNCUsersList(): ?array { public function updateExAppInfo(ExApp $exApp, array $appInfo): bool { $exApp->setVersion($appInfo['version']); $exApp->setName($appInfo['name']); - $exApp->setApiScopes($appInfo['api_scopes']); - if (!$this->updateExApp($exApp, ['version', 'name', 'api_scopes'])) { + if (!$this->updateExApp($exApp, ['version', 'name'])) { return false; } return true; } - public function updateExApp(ExApp $exApp, array $fields = ['version', 'name', 'port', 'status', 'enabled', 'last_check_time', 'api_scopes']): bool { + public function updateExApp(ExApp $exApp, array $fields = ['version', 'name', 'port', 'status', 'enabled', 'last_check_time']): bool { try { $this->exAppMapper->updateExApp($exApp, $fields); $this->cache?->remove('/ex_apps'); @@ -266,7 +261,7 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo): # fill 'id' if it is missing(this field was called `appid` in previous versions in json) $appInfo['id'] = $appInfo['id'] ?? $appId; # during manual install JSON can have all values at root level - foreach (['docker-install', 'scopes', 'system_app', 'translations_folder', 'routes'] as $key) { + foreach (['docker-install', 'translations_folder', 'routes'] as $key) { if (isset($appInfo[$key])) { $appInfo['external-app'][$key] = $appInfo[$key]; unset($appInfo[$key]); @@ -285,13 +280,6 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo): } } $appInfo = json_decode(json_encode((array)$xmlAppInfo), true); - if (isset($appInfo['external-app']['scopes']['value'])) { - if (is_array($appInfo['external-app']['scopes']['value'])) { - $appInfo['external-app']['scopes'] = $appInfo['external-app']['scopes']['value']; - } else { - $appInfo['external-app']['scopes'] = [$appInfo['external-app']['scopes']['value']]; - } - } if (isset($appInfo['external-app']['routes']['route'])) { if (isset($appInfo['external-app']['routes']['route'][0])) { $appInfo['external-app']['routes'] = $appInfo['external-app']['routes']['route']; diff --git a/tests/auth_scopes_system.py b/tests/auth_scopes_system.py deleted file mode 100644 index ef21c54d..00000000 --- a/tests/auth_scopes_system.py +++ /dev/null @@ -1,25 +0,0 @@ -import sys -import pytest -import nc_py_api - -# sys.argv[1] = 0 -> System App, ALL Scope -# sys.argv[1] = 1 -> System App, No ALL Scope -# sys.argv[1] = 2 -> No System App, ALL Scope - -if __name__ == "__main__": - nc = nc_py_api.NextcloudApp(user="admin") - assert nc.capabilities - if int(sys.argv[1]) == 0: - nc.ocs("GET", "/ocs/v2.php/core/whatsnew") - else: - with pytest.raises(nc_py_api.NextcloudException) as e: - nc.ocs("GET", "/ocs/v2.php/core/whatsnew") - assert e.value.status_code == 401 - - if int(sys.argv[1]) == 2: - # as NextcloudApp was initialized with `user="admin"` this will fail for non-system app. - with pytest.raises(nc_py_api.NextcloudException) as e: - nc.users_list() - assert e.value.status_code == 401 - else: - assert nc.users_list() From 6b06a22d331048dfea7c36c6f9feefa9fb62cb22 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 4 Sep 2024 18:58:25 +0300 Subject: [PATCH 2/7] feat: remove api scopes Signed-off-by: Andrey Borysenko --- src/components/Apps/ScopesDetails.vue | 45 --------------------------- src/store/apps.js | 23 +------------- src/views/Apps.vue | 9 ------ 3 files changed, 1 insertion(+), 76 deletions(-) delete mode 100644 src/components/Apps/ScopesDetails.vue diff --git a/src/components/Apps/ScopesDetails.vue b/src/components/Apps/ScopesDetails.vue deleted file mode 100644 index e7b623ae..00000000 --- a/src/components/Apps/ScopesDetails.vue +++ /dev/null @@ -1,45 +0,0 @@ - - - - - diff --git a/src/store/apps.js b/src/store/apps.js index 3d0ca70d..8c7a2aed 100644 --- a/src/store/apps.js +++ b/src/store/apps.js @@ -1,6 +1,6 @@ import api from './api.js' import Vue from 'vue' -import { generateUrl, generateOcsUrl } from '@nextcloud/router' +import { generateUrl } from '@nextcloud/router' import { showError, showInfo } from '@nextcloud/dialogs' const state = { @@ -70,7 +70,6 @@ const mutations = { init: 0, deploy: 0, } - app.scopes = null } app.active = true app.canUnInstall = false @@ -94,7 +93,6 @@ const mutations = { state.apps.find(app => app.id === appId).canInstall = true state.apps.find(app => app.id === appId).daemon = null state.apps.find(app => app.id === appId).status = {} - state.apps.find(app => app.id === appId).scopes = null if (state.apps.find(app => app.id === appId).update !== null) { state.updateCount-- } @@ -112,7 +110,6 @@ const mutations = { init: 0, deploy: 0, } - app.scopes = null app.error = null state.updateCount-- }, @@ -153,13 +150,6 @@ const mutations = { app.status = status }, - setExAppInfo(state, { appId, exAppInfo }) { - const app = state.apps.find(app => app.id === appId) - if (exAppInfo.scopes) { - app.scopes = exAppInfo.scopes - } - }, - setIntervalUpdater(state, updater) { state.statusUpdater = updater }, @@ -359,12 +349,6 @@ const actions = { return context.state.gettingCategoriesPromise }, - getExAppInfo(context, { appId }) { - return api.get(generateOcsUrl(`/apps/app_api/api/v1/ex-app/info/${appId}`)).then((response) => { - context.commit('setExAppInfo', { appId, exAppInfo: response.data?.ocs.data }) - }) - }, - getAppStatus(context, { appId }) { return api.get(generateUrl(`/apps/app_api/apps/status/${appId}`)) .then((response) => { @@ -397,11 +381,6 @@ const actions = { console.debug('initializingOrDeployingApps', initializingOrDeployingApps) Array.from(initializingOrDeployingApps).forEach(app => { context.dispatch('getAppStatus', { appId: app.id }) - if ((app.status.deploy === 100 && app.status.init === 0) || app.status.type === 'update') { - console.debug('getExAppInfo', app.id) - // get ExApp info once app is deployed or during update - context.dispatch('getExAppInfo', { appId: app.id }) - } }) }, 2000)) }, diff --git a/src/views/Apps.vue b/src/views/Apps.vue index 015461cc..58d4839d 100644 --- a/src/views/Apps.vue +++ b/src/views/Apps.vue @@ -128,13 +128,6 @@ :order="1"> - - - @@ -160,7 +153,6 @@ import AppManagement from '../mixins/AppManagement.js' import AppScore from '../components/Apps/AppScore.vue' import Markdown from '../components/Apps/Markdown.vue' import DaemonDetails from '../components/Apps/DaemonDetails.vue' -import ScopesDetails from '../components/Apps/ScopesDetails.vue' import { APPS_SECTION_ENUM } from '../constants/AppsConstants.js' import { loadState } from '@nextcloud/initial-state' @@ -184,7 +176,6 @@ export default { NcContent, Markdown, DaemonDetails, - ScopesDetails, Alert, }, From 090d4815b2245b2320cc2182c27f9b80e0975fb1 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 4 Sep 2024 18:58:36 +0300 Subject: [PATCH 3/7] chore: update docs Signed-off-by: Andrey Borysenko --- docs/notes_for_developers/ExAppOverview.rst | 2 +- docs/tech_details/ApiScopes.rst | 4 ++++ docs/tech_details/Deployment.rst | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/notes_for_developers/ExAppOverview.rst b/docs/notes_for_developers/ExAppOverview.rst index 17719837..f67bfc9d 100644 --- a/docs/notes_for_developers/ExAppOverview.rst +++ b/docs/notes_for_developers/ExAppOverview.rst @@ -41,7 +41,7 @@ It should contain the following fields: cloud-py-api/skeleton latest - + // deprecated and removed since AppAPI 3.2.0 FILES AI_PROVIDERS ... diff --git a/docs/tech_details/ApiScopes.rst b/docs/tech_details/ApiScopes.rst index 6602bb1a..0ca1b7c3 100644 --- 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/Deployment.rst b/docs/tech_details/Deployment.rst index ecbaf434..37b0faed 100644 --- 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 cloud-py-api/talk_bot latest - + // deprecated since AppAPI 3.2.0 TALK TALK_BOT From 3352b600f91da00ce4494178ed732120ddd5a620 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 4 Sep 2024 18:58:43 +0300 Subject: [PATCH 4/7] chore: bump version Signed-off-by: Andrey Borysenko --- appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index b950ccbe..ee2408ad 100644 --- 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.* ]]> - 3.1.1 + 3.2.0 agpl Andrey Borysenko Alexander Piskun From 6ee9e024b9677deb3721f01107d870a4e3bfa32b Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 4 Sep 2024 19:33:05 +0300 Subject: [PATCH 5/7] chore: update changelog Signed-off-by: Andrey Borysenko --- CHANGELOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0b1c20c..65293f9d 100644 --- 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-0x] + +### 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) From 454741ca04957ff19170bd8a8e10b0e68e4d35c1 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 4 Sep 2024 19:52:15 +0300 Subject: [PATCH 6/7] chore(docs): update Authentication scheme Signed-off-by: Andrey Borysenko --- docs/tech_details/Authentication.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/tech_details/Authentication.rst b/docs/tech_details/Authentication.rst index 15576e08..0d07e022 100644 --- 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 From 38c6814ceccb8977a1a886e51dd3affd28cdd665 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 4 Sep 2024 20:00:10 +0300 Subject: [PATCH 7/7] chore: update changelog Signed-off-by: Andrey Borysenko --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65293f9d..0af74b85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ 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-0x] +## [3.2.0 - 2024-09-06] ### Added