Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove ApiScopes #373

Merged
merged 7 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions .github/workflows/tests-special.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion docs/notes_for_developers/ExAppOverview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>
...
Expand Down
4 changes: 4 additions & 0 deletions docs/tech_details/ApiScopes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 0 additions & 2 deletions docs/tech_details/Authentication.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/tech_details/Deployment.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
3 changes: 0 additions & 3 deletions lib/Command/ExApp/Enable.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
39 changes: 1 addition & 38 deletions lib/Command/ExApp/Register.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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');
Expand Down Expand Up @@ -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));
Expand All @@ -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']);
Expand Down
50 changes: 0 additions & 50 deletions lib/Command/ExApp/Scopes/ListScopes.php

This file was deleted.

31 changes: 1 addition & 30 deletions lib/Command/ExApp/Update.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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');
Expand Down Expand Up @@ -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'] = '';
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/DaemonConfigController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading
Loading