From aed09a64b9010d7fc85e713e9ce1298971e9219d Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 2 Aug 2023 20:41:59 +0300 Subject: [PATCH 1/8] ExApp version check draft --- docs/authentication.rst | 1 + lib/Db/ExAppMapper.php | 12 +++++++++ lib/Service/AppEcosystemV2Service.php | 35 +++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/docs/authentication.rst b/docs/authentication.rst index f484b195..4c7b14cd 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -99,6 +99,7 @@ Authentication flow in details Nextcloud->>+AppEcosystemV2: Validate request AppEcosystemV2-->>AppEcosystemV2: Check if ExApp exists and enabled AppEcosystemV2-->>Nextcloud: Reject if ExApp not exists or disabled + AppEcosystemV2-->>AppEcosystemV2: Check if ExApp version changed AppEcosystemV2-->>AppEcosystemV2: Validate AE-SIGN-TIME AppEcosystemV2-->>Nextcloud: Reject if sign time diff > 5 min AppEcosystemV2-->>AppEcosystemV2: Generate and validate AE-SIGNATURE diff --git a/lib/Db/ExAppMapper.php b/lib/Db/ExAppMapper.php index 55fe14bd..20a2a205 100644 --- a/lib/Db/ExAppMapper.php +++ b/lib/Db/ExAppMapper.php @@ -102,4 +102,16 @@ public function updateLastCheckTime(ExApp $exApp): int { $qb->expr()->eq('appid', $qb->createNamedParameter($exApp->getAppid())) )->executeStatement(); } + + /** + * @throws Exception + */ + public function updateExAppVersion(ExApp $exApp): int { + $qb = $this->db->getQueryBuilder(); + return $qb->update($this->tableName) + ->set('version', $qb->createNamedParameter($exApp->getVersion(), IQueryBuilder::PARAM_INT)) + ->where( + $qb->expr()->eq('appid', $qb->createNamedParameter($exApp->getAppid())) + )->executeStatement(); + } } diff --git a/lib/Service/AppEcosystemV2Service.php b/lib/Service/AppEcosystemV2Service.php index 757ee191..ad3776be 100644 --- a/lib/Service/AppEcosystemV2Service.php +++ b/lib/Service/AppEcosystemV2Service.php @@ -448,6 +448,7 @@ private function generateDataHash(string $data): string { /** * AppEcosystem authentication request validation for Nextcloud: * - checks if ExApp exists and is enabled + * - checks if ExApp version changed and updates it in database * - validates request sign time (if it's complies with set time window) * - builds and checks request signature * - checks if request data hash is valid @@ -467,6 +468,10 @@ public function validateExAppRequestToNC(IRequest $request, bool $isDav = false) return false; } + if (!$this->handleExAppVersionChange($request, $exApp)) { + return false; + } + $enabled = $exApp->getEnabled(); if (!$enabled) { $this->logger->error(sprintf('ExApp with appId %s is disabled (%s)', $request->getHeader('EX-APP-ID'), $enabled)); @@ -605,6 +610,36 @@ public function updateExAppLastCheckTime(ExApp &$exApp): void { } } + public function updateExAppVersion(ExApp $exApp): bool { + try { + return $this->exAppMapper->updateExAppVersion($exApp) === 1; + } catch (Exception $e) { + $this->logger->error(sprintf('Failed to update ExApp %s version to %s', $exApp->getAppid(), $exApp->getVersion()), ['exception' => $e]); + return false; + } + } + + /** + * Check if ExApp version changed and update it in database + * + * @param IRequest $request + * @param ExApp $exApp + * + * @return bool + */ + public function handleExAppVersionChange(IRequest $request, ExApp &$exApp): bool { + $requestExAppVersion = $request->getHeader('EX-APP-VERSION'); + $versionValid = $exApp->getVersion() === $requestExAppVersion; + if (!$versionValid) { + // Update ExApp version + $exApp->setVersion($requestExAppVersion); + if (!$this->updateExAppVersion($exApp)) { + return false; + } + } + return true; + } + public function getExAppsList(bool $extended = false): array { try { $exApps = $this->exAppMapper->findAll(); From 15986282c39ed1e24773878ed652957731bb023d Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Tue, 8 Aug 2023 23:42:30 +0300 Subject: [PATCH 2/8] ExApp version change handling update Disable ExApp and send notifications to admins Added ExAppAdminNotifier Minor fixes --- lib/AppInfo/Application.php | 2 + lib/Db/ExAppMapper.php | 2 +- lib/Notifications/ExAppAdminNotifier.php | 62 ++++++++++++++++++++ lib/Notifications/ExAppNotifier.php | 5 +- lib/Notifications/ExNotificationsManager.php | 30 ++++++++-- lib/Service/AppEcosystemV2Service.php | 60 +++++++++++++------ lib/Settings/Admin.php | 2 +- lib/Settings/AdminSection.php | 2 +- 8 files changed, 137 insertions(+), 28 deletions(-) create mode 100644 lib/Notifications/ExAppAdminNotifier.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 64e03f16..bea6a9bd 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -9,6 +9,7 @@ use OCA\AppEcosystemV2\Listener\LoadFilesPluginListener; use OCA\AppEcosystemV2\Listener\SabrePluginAuthInitListener; use OCA\AppEcosystemV2\Middleware\AppEcosystemAuthMiddleware; +use OCA\AppEcosystemV2\Notifications\ExAppAdminNotifier; use OCA\AppEcosystemV2\Notifications\ExAppNotifier; use OCA\AppEcosystemV2\Profiler\AEDataCollector; use OCA\AppEcosystemV2\PublicCapabilities; @@ -45,6 +46,7 @@ public function register(IRegistrationContext $context): void { $context->registerMiddleware(AppEcosystemAuthMiddleware::class); $context->registerEventListener(SabrePluginAuthInitEvent::class, SabrePluginAuthInitListener::class); $context->registerNotifierService(ExAppNotifier::class); + $context->registerNotifierService(ExAppAdminNotifier::class); } public function boot(IBootContext $context): void { diff --git a/lib/Db/ExAppMapper.php b/lib/Db/ExAppMapper.php index 20a2a205..4d978764 100644 --- a/lib/Db/ExAppMapper.php +++ b/lib/Db/ExAppMapper.php @@ -109,7 +109,7 @@ public function updateLastCheckTime(ExApp $exApp): int { public function updateExAppVersion(ExApp $exApp): int { $qb = $this->db->getQueryBuilder(); return $qb->update($this->tableName) - ->set('version', $qb->createNamedParameter($exApp->getVersion(), IQueryBuilder::PARAM_INT)) + ->set('version', $qb->createNamedParameter($exApp->getVersion(), IQueryBuilder::PARAM_STR)) ->where( $qb->expr()->eq('appid', $qb->createNamedParameter($exApp->getAppid())) )->executeStatement(); diff --git a/lib/Notifications/ExAppAdminNotifier.php b/lib/Notifications/ExAppAdminNotifier.php new file mode 100644 index 00000000..41ed7053 --- /dev/null +++ b/lib/Notifications/ExAppAdminNotifier.php @@ -0,0 +1,62 @@ +factory = $factory; + $this->url = $urlGenerator; + $this->service = $service; + } + + public function getID(): string { + return Application::APP_ID; + } + + public function getName(): string { + return $this->factory->get(Application::APP_ID)->t('AppEcosystemV2 ExApp version update notifier'); + } + + public function prepare(INotification $notification, string $languageCode): INotification { + $exApp = $this->service->getExApp($notification->getApp()); + // TODO: Think about another possible admin ExApp notifications, make them unified + // TODO: Think about ExApp rich objects + if ($exApp === null || $notification->getSubject() !== 'ex_app_version_update') { + throw new \InvalidArgumentException(); + } + if ($exApp->getEnabled()) { + throw new \InvalidArgumentException('ExApp is probably already re-enabled'); + } + + $parameters = $notification->getSubjectParameters(); + + $notification->setLink($this->url->getAbsoluteURL('/index.php/settings/admin/app_ecosystem_v2')); + $notification->setIcon($this->url->imagePath(Application::APP_ID, 'app-dark.svg')); + + if (isset($parameters['rich_subject']) && isset($parameters['rich_subject_params'])) { + $notification->setRichSubject($parameters['rich_subject'], $parameters['rich_subject_params']); + } + if (isset($parameters['rich_message']) && isset($parameters['rich_message_params'])) { + $notification->setRichMessage($parameters['rich_message'], $parameters['rich_message_params']); + } + + return $notification; + } +} diff --git a/lib/Notifications/ExAppNotifier.php b/lib/Notifications/ExAppNotifier.php index 3772add6..f1e8bcb7 100644 --- a/lib/Notifications/ExAppNotifier.php +++ b/lib/Notifications/ExAppNotifier.php @@ -39,8 +39,9 @@ public function prepare(INotification $notification, string $languageCode): INot if ($exApp === null) { throw new \InvalidArgumentException(); } - // Only enabled ExApps can render notifications - if (!$exApp->getEnabled()) { + if ($notification->getSubject() === 'ex_app_version_update' && $exApp->getEnabled()) { + throw new \InvalidArgumentException('ExApp is probably already re-enabled'); + } elseif (!$exApp->getEnabled()) { // Only enabled ExApps can render notifications throw new \InvalidArgumentException('ExApp is disabled'); } diff --git a/lib/Notifications/ExNotificationsManager.php b/lib/Notifications/ExNotificationsManager.php index c5b00b8e..19b36f79 100644 --- a/lib/Notifications/ExNotificationsManager.php +++ b/lib/Notifications/ExNotificationsManager.php @@ -4,14 +4,17 @@ namespace OCA\AppEcosystemV2\Notifications; +use OCP\IGroupManager; use OCP\Notification\IManager; use OCP\Notification\INotification; class ExNotificationsManager { - private IManager $manager; + private IManager $notificationManager; + private IGroupManager $groupManager; - public function __construct(IManager $manager) { - $this->manager = $manager; + public function __construct(IManager $manager, IGroupManager $groupManager) { + $this->notificationManager = $manager; + $this->groupManager = $groupManager; } /** @@ -24,14 +27,31 @@ public function __construct(IManager $manager) { * @return INotification */ public function sendNotification(string $appId, ?string $userId = null, array $params = []): INotification { - $notification = $this->manager->createNotification(); + $notification = $this->notificationManager->createNotification(); $notification ->setApp($appId) ->setUser($userId) ->setDateTime(new \DateTime()) ->setObject($params['object'], $params['object_id']) ->setSubject($params['subject_type'], $params['subject_params']); - $this->manager->notify($notification); + $this->notificationManager->notify($notification); return $notification; } + + public function sendAdminsNotification(string $appId, array $params = []): array { + $admins = $this->groupManager->get("admin")->getUsers(); + $notifications = []; + foreach ($admins as $adminUser) { + $notification = $this->notificationManager->createNotification(); + $notification + ->setApp($appId) + ->setUser($adminUser->getUID()) + ->setDateTime(new \DateTime()) + ->setObject($params['object'], $params['object_id']) + ->setSubject($params['subject_type'], $params['subject_params']); + $this->notificationManager->notify($notification); + $notifications[] = $notification; + } + return $notifications; + } } diff --git a/lib/Service/AppEcosystemV2Service.php b/lib/Service/AppEcosystemV2Service.php index ad3776be..041305b5 100644 --- a/lib/Service/AppEcosystemV2Service.php +++ b/lib/Service/AppEcosystemV2Service.php @@ -8,6 +8,7 @@ use OCA\AppEcosystemV2\Db\ExApp; use OCA\AppEcosystemV2\Db\ExAppMapper; +use OCA\AppEcosystemV2\Notifications\ExNotificationsManager; use OCP\App\IAppManager; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -45,22 +46,24 @@ class AppEcosystemV2Service { private ExAppUsersService $exAppUsersService; private ExAppScopesService $exAppScopesService; private ExAppConfigService $exAppConfigService; + private ExNotificationsManager $exNotificationsManager; public function __construct( - LoggerInterface $logger, - ILogFactory $logFactory, - ICacheFactory $cacheFactory, - IConfig $config, - IClientService $clientService, - ExAppMapper $exAppMapper, - IAppManager $appManager, - ExAppUsersService $exAppUserService, + LoggerInterface $logger, + ILogFactory $logFactory, + ICacheFactory $cacheFactory, + IConfig $config, + IClientService $clientService, + ExAppMapper $exAppMapper, + IAppManager $appManager, + ExAppUsersService $exAppUserService, ExAppApiScopeService $exAppApiScopeService, - ExAppScopesService $exAppScopesService, - ISecureRandom $random, - IUserSession $userSession, - IUserManager $userManager, - ExAppConfigService $exAppConfigService, + ExAppScopesService $exAppScopesService, + ISecureRandom $random, + IUserSession $userSession, + IUserManager $userManager, + ExAppConfigService $exAppConfigService, + ExNotificationsManager $exNotificationsManager, ) { $this->logger = $logger; $this->logFactory = $logFactory; @@ -76,6 +79,7 @@ public function __construct( $this->exAppApiScopeService = $exAppApiScopeService; $this->exAppScopesService = $exAppScopesService; $this->exAppConfigService = $exAppConfigService; + $this->exNotificationsManager = $exNotificationsManager; } public function getExApp(string $appId): ?ExApp { @@ -468,10 +472,6 @@ public function validateExAppRequestToNC(IRequest $request, bool $isDav = false) return false; } - if (!$this->handleExAppVersionChange($request, $exApp)) { - return false; - } - $enabled = $exApp->getEnabled(); if (!$enabled) { $this->logger->error(sprintf('ExApp with appId %s is disabled (%s)', $request->getHeader('EX-APP-ID'), $enabled)); @@ -503,6 +503,9 @@ public function validateExAppRequestToNC(IRequest $request, bool $isDav = false) $signatureValid = $signature === $requestSignature; if ($signatureValid) { + if (!$this->handleExAppVersionChange($request, $exApp)) { + return false; + } if (!$this->verifyDataHash($dataHash)) { $this->logger->error(sprintf('Data hash %s is not valid', $dataHash)); return false; @@ -620,7 +623,13 @@ public function updateExAppVersion(ExApp $exApp): bool { } /** - * Check if ExApp version changed and update it in database + * Check if ExApp version changed and update it in database. + * Immediately disable ExApp and send notifications to the administrators (users of admins group). + * This handling only intentional case of manual ExApp update + * so the administrator must re-enable ExApp in UI or CLI after that. + * + * Ref: https://github.com/cloud-py-api/app_ecosystem_v2/pull/29 + * TODO: Add link to docs with warning and mark as not-recommended * * @param IRequest $request * @param ExApp $exApp @@ -632,10 +641,25 @@ public function handleExAppVersionChange(IRequest $request, ExApp &$exApp): bool $versionValid = $exApp->getVersion() === $requestExAppVersion; if (!$versionValid) { // Update ExApp version + $oldVersion = $exApp->getVersion(); $exApp->setVersion($requestExAppVersion); if (!$this->updateExAppVersion($exApp)) { return false; } + if ($this->disableExApp($exApp)) { + $this->exNotificationsManager->sendAdminsNotification($exApp->getAppid(), [ + 'object' => 'ex_app_update', + 'object_id' => $exApp->getAppid(), + 'subject_type' => 'ex_app_version_update', + 'subject_params' => [ + 'rich_subject' => 'ExApp updated, action required!', + 'rich_subject_params' => [], + 'rich_message' => sprintf('ExApp %s disabled due to update from %s to %s. Manual re-enable required.', $exApp->getAppid(), $oldVersion, $exApp->getVersion()), + 'rich_message_params' => [], + ], + ]); + } + return false; } return true; } diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index a6bbbda5..2b96ee9b 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -38,7 +38,7 @@ public function getForm(): TemplateResponse { } public function getSection(): string { - return 'app-ecosystem-v2'; + return Application::APP_ID; } public function getPriority(): int { diff --git a/lib/Settings/AdminSection.php b/lib/Settings/AdminSection.php index d8d4b6e4..c4f9ebec 100644 --- a/lib/Settings/AdminSection.php +++ b/lib/Settings/AdminSection.php @@ -29,7 +29,7 @@ public function __construct( * @inheritDoc */ public function getID(): string { - return 'app-ecosystem-v2'; + return 'app_ecosystem_v2'; } /** From 96170cb1d890536c29cda4cf3c5f41653943ed8e Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Wed, 9 Aug 2023 00:34:19 +0300 Subject: [PATCH 3/8] added test Signed-off-by: Alexander Piskun --- .github/workflows/tests-special.yml | 144 ++++++++++++++++++++++++++++ tests/app_version_higher.py | 26 +++++ 2 files changed, 170 insertions(+) create mode 100644 .github/workflows/tests-special.yml create mode 100644 tests/app_version_higher.py diff --git a/.github/workflows/tests-special.yml b/.github/workflows/tests-special.yml new file mode 100644 index 00000000..8904839a --- /dev/null +++ b/.github/workflows/tests-special.yml @@ -0,0 +1,144 @@ +name: Tests Special + +on: + pull_request: + push: + branches: [main] + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: tests-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + app-version-higher: + runs-on: ubuntu-22.04 + name: ExApp version higher + env: + NEXTCLOUD_URL: "http://localhost:8080/" + APP_ID: "nc_py_api" + APP_PORT: 9009 + APP_VERSION: "1.0.0" + APP_SECRET: "tC6vkwPhcppjMykD1r0n9NlI95uJMBYjs5blpIcA1PAdoPDmc5qoAjaBAkyocZ6E" + + services: + postgres: + image: ghcr.io/nextcloud/continuous-integration-postgres-14:latest + ports: + - 4444:5432/tcp + env: + POSTGRES_USER: root + POSTGRES_PASSWORD: rootpassword + POSTGRES_DB: nextcloud + options: --health-cmd pg_isready --health-interval 5s --health-timeout 2s --health-retries 5 + + steps: + - uses: actions/setup-python@v4 + with: + python-version: '3.11' + + - name: Set app env + run: echo "APP_NAME=${GITHUB_REPOSITORY##*/}" >> $GITHUB_ENV + + - name: Checkout server + uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + with: + submodules: true + repository: nextcloud/server + ref: 'stable27' + + - name: Checkout Notifications + uses: actions/checkout@v3 + with: + repository: nextcloud/notifications + ref: ${{ matrix.server-version }} + path: apps/notifications + + - name: Checkout AppEcosystemV2 + uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + with: + path: apps/${{ env.APP_NAME }} + + - name: Set up php + uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b # v2 + with: + php-version: '8.1' + extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, pgsql, pdo_pgsql + coverage: none + ini-file: development + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Check composer file existence + id: check_composer + uses: andstor/file-existence-action@20b4d2e596410855db8f9ca21e96fbe18e12930b # v2 + with: + files: apps/${{ env.APP_NAME }}/composer.json + + - name: Set up dependencies + if: steps.check_composer.outputs.files_exists == 'true' + working-directory: apps/${{ env.APP_NAME }} + run: composer i + + - name: Set up Nextcloud + env: + DB_PORT: 4444 + run: | + mkdir data + ./occ maintenance:install --verbose --database=pgsql --database-name=nextcloud --database-host=127.0.0.1 \ + --database-port=$DB_PORT --database-user=root --database-pass=rootpassword \ + --admin-user admin --admin-pass admin + ./occ config:system:set allow_local_remote_servers --value true + ./occ app:enable notifications + ./occ app:enable --force ${{ env.APP_NAME }} + patch -p 1 -i apps/${{ env.APP_NAME }}/base_php.patch + + - name: Run Nextcloud + run: php -S 127.0.0.1:8080 & + + - name: Checkout NcPyApi + uses: actions/checkout@v3 + with: + path: nc_py_api + repository: cloud-py-api/nc_py_api + + - name: Install NcPyApi + working-directory: nc_py_api + run: python3 -m pip -v install ".[dev]" + + - name: Register NcPyApi + run: | + cd nc_py_api + python3 tests/_install.py & + echo $! > /tmp/_install.pid + cd .. + sleep 5s + php occ app_ecosystem_v2:daemon:register manual_install "Manual Install" manual-install 0 0 0 + php occ app_ecosystem_v2:app:register nc_py_api manual_install --json-info \ + "{\"appid\":\"$APP_ID\",\"name\":\"$APP_ID\",\"daemon_config_name\":\"manual_install\",\"version\":\"$APP_VERSION\",\"secret\":\"$APP_SECRET\",\"host\":\"localhost\",\"port\":$APP_PORT,\"protocol\":\"http\",\"system_app\":1}" \ + -e --force-scopes + kill -15 $(cat /tmp/_install.pid) + timeout 3m tail --pid=$(cat /tmp/_install.pid) -f /dev/null + + - name: Run Manual App Update test + run: python3 tests/app_version_higher.py + + - name: Upload NC logs + if: always() + uses: actions/upload-artifact@v3 + with: + name: app_version_higher_nextcloud.log + path: data/nextcloud.log + if-no-files-found: warn + + tests-success: + permissions: + contents: none + runs-on: ubuntu-22.04 + needs: [app-version-higher] + name: TestsSpecial-OK + steps: + - run: echo "Tests special passed successfully" diff --git a/tests/app_version_higher.py b/tests/app_version_higher.py new file mode 100644 index 00000000..77ac59f8 --- /dev/null +++ b/tests/app_version_higher.py @@ -0,0 +1,26 @@ +import pytest + +from nc_py_api import Nextcloud, NextcloudApp, NextcloudException + + +if __name__ == "__main__": + nc_client = Nextcloud(nc_auth_user="admin", nc_auth_pass="admin") + assert nc_client.apps.is_disabled("nc_py_api") is False + nc_client.users.create("second_admin", password="2Very3Strong4", groups=["admin"]) + + nc_application = NextcloudApp(user="admin") + assert nc_application.users.get_details() # OCS call works + assert not nc_application.users.notifications.get_all() # there are no notifications + nc_application._session.cfg.app_version = "99.0.0" # change ExApp version + with pytest.raises(NextcloudException) as exc_info: + nc_application.users.get_details() # this call should be rejected by AppEcosystem + assert exc_info.value.status_code == 401 + + assert nc_client.apps.is_disabled("nc_py_api") is True + notifications = nc_client.users.notifications.get_all() + notification = [i for i in notifications if i.info.subject == "ex_app_version_update"] + assert len(notification) == 1 # only one notification for each admin + nc_client = Nextcloud(nc_auth_user="second_admin", nc_auth_pass="2Very3Strong4") + notifications = nc_client.users.notifications.get_all() + notification = [i for i in notifications if i.info.subject == "ex_app_version_update"] + assert len(notification) == 1 # only one notification for each admin From 707d3d160e5c09a4d87f229d3c75419d206c10c3 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Wed, 9 Aug 2023 00:47:41 +0300 Subject: [PATCH 4/8] fixed tests-special.yml Signed-off-by: Alexander Piskun --- .github/workflows/tests-special.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests-special.yml b/.github/workflows/tests-special.yml index 8904839a..d5ef570a 100644 --- a/.github/workflows/tests-special.yml +++ b/.github/workflows/tests-special.yml @@ -10,7 +10,7 @@ permissions: contents: read concurrency: - group: tests-${{ github.head_ref || github.run_id }} + group: tests-special-${{ github.head_ref || github.run_id }} cancel-in-progress: true jobs: @@ -124,6 +124,7 @@ jobs: timeout 3m tail --pid=$(cat /tmp/_install.pid) -f /dev/null - name: Run Manual App Update test + working-directory: apps/${{ env.APP_NAME }} run: python3 tests/app_version_higher.py - name: Upload NC logs From d2f3ba17e78181ac77f368d17313e0004927d8f0 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Wed, 9 Aug 2023 01:01:45 +0300 Subject: [PATCH 5/8] fixed tests-special.yml Signed-off-by: Alexander Piskun --- tests/app_version_higher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app_version_higher.py b/tests/app_version_higher.py index 77ac59f8..48ac8dea 100644 --- a/tests/app_version_higher.py +++ b/tests/app_version_higher.py @@ -11,7 +11,7 @@ nc_application = NextcloudApp(user="admin") assert nc_application.users.get_details() # OCS call works assert not nc_application.users.notifications.get_all() # there are no notifications - nc_application._session.cfg.app_version = "99.0.0" # change ExApp version + nc_application._session.adapter.headers.update({"EX-APP-VERSION": "99.0.0"}) # change ExApp version with pytest.raises(NextcloudException) as exc_info: nc_application.users.get_details() # this call should be rejected by AppEcosystem assert exc_info.value.status_code == 401 From 79a94ce233345113b3c931e8b6a9c84376f9122d Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 9 Aug 2023 17:25:53 +0300 Subject: [PATCH 6/8] api adjustments (#36) Resolves: #35. Return consistent ExApps list structure. --------- Signed-off-by: Alexander Piskun Co-authored-by: Alexander Piskun --- appinfo/routes.php | 2 +- lib/Controller/ExAppController.php | 11 ++++++--- lib/Controller/OCSApiController.php | 4 ---- lib/Service/AppEcosystemV2Service.php | 34 ++++++++++++++------------- lib/Service/ExAppApiScopeService.php | 1 + 5 files changed, 28 insertions(+), 24 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index 82422e36..5863970f 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -42,7 +42,7 @@ ['name' => 'OCSApi#getExAppUsers', 'url' => '/api/v1/users', 'verb' => 'GET'], // ExApps - ['name' => 'exApp#getExApps', 'url' => '/api/v1/ex-app/all', 'verb' => 'GET'], + ['name' => 'exApp#getExApps', 'url' => '/api/v1/ex-app/{list}', 'verb' => 'GET'], // Ex Apps registration (for Admin GUI, not implemented yet) // ['name' => 'OCSApi#registerExternalApp', 'url' => '/api/v1/ex-app', 'verb' => 'POST'], diff --git a/lib/Controller/ExAppController.php b/lib/Controller/ExAppController.php index cc0a2d43..ce0da3f2 100644 --- a/lib/Controller/ExAppController.php +++ b/lib/Controller/ExAppController.php @@ -10,6 +10,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCSController; use OCP\IRequest; @@ -28,12 +29,16 @@ public function __construct( /** * @NoCSRFRequired * - * @param bool $extended + * @param string $list * + * @throws OCSBadRequestException * @return DataResponse */ #[NoCSRFRequired] - public function getExApps(bool $extended = false): DataResponse { - return new DataResponse($this->service->getExAppsList($extended), Http::STATUS_OK); + public function getExApps(string $list = 'enabled'): DataResponse { + if (!in_array($list, ['all', 'enabled'])) { + throw new OCSBadRequestException(); + } + return new DataResponse($this->service->getExAppsList($list), Http::STATUS_OK); } } diff --git a/lib/Controller/OCSApiController.php b/lib/Controller/OCSApiController.php index 07a633b3..aff4b133 100644 --- a/lib/Controller/OCSApiController.php +++ b/lib/Controller/OCSApiController.php @@ -19,13 +19,11 @@ use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; -use OCP\IL10N; use OCP\IRequest; use Psr\Log\LoggerInterface; class OCSApiController extends OCSController { private LoggerInterface $logger; - private IL10N $l; private AppEcosystemV2Service $service; private ExFilesActionsMenuService $exFilesActionsMenuService; private ExAppApiScopeService $exAppApiScopeService; @@ -35,7 +33,6 @@ class OCSApiController extends OCSController { public function __construct( IRequest $request, ?string $userId, - IL10N $l, AppEcosystemV2Service $service, ExFilesActionsMenuService $exFilesActionsMenuService, LoggerInterface $logger, @@ -45,7 +42,6 @@ public function __construct( $this->request = $request; $this->logger = $logger; - $this->l = $l; $this->service = $service; $this->exFilesActionsMenuService = $exFilesActionsMenuService; $this->userId = $userId; diff --git a/lib/Service/AppEcosystemV2Service.php b/lib/Service/AppEcosystemV2Service.php index 041305b5..066eeb8e 100644 --- a/lib/Service/AppEcosystemV2Service.php +++ b/lib/Service/AppEcosystemV2Service.php @@ -664,25 +664,27 @@ public function handleExAppVersionChange(IRequest $request, ExApp &$exApp): bool return true; } - public function getExAppsList(bool $extended = false): array { + + public function getExAppsList(string $list = 'enabled'): array { try { $exApps = $this->exAppMapper->findAll(); - if ($extended) { - $exApps = array_map(function (ExApp $exApp) { - return [ - 'id' => $exApp->getAppid(), - 'name' => $exApp->getName(), - 'version' => $exApp->getVersion(), - 'enabled' => $exApp->getEnabled(), - 'last_check_time' => $exApp->getLastCheckTime(), - 'system' => $this->exAppUsersService->exAppUserExists($exApp->getAppid(), ''), - ]; - }, $exApps); - } else { - $exApps = array_map(function (ExApp $exApp) { - return $exApp->getAppid(); - }, $exApps); + + if ($list === 'enabled') { + $exApps = array_filter($exApps, function (ExApp $exApp) { + return $exApp->getEnabled() === 1; + }); } + + $exApps = array_map(function (ExApp $exApp) { + return [ + 'id' => $exApp->getAppid(), + 'name' => $exApp->getName(), + 'version' => $exApp->getVersion(), + 'enabled' => filter_var($exApp->getEnabled(), FILTER_VALIDATE_BOOLEAN), + 'last_check_time' => $exApp->getLastCheckTime(), + 'system' => $this->exAppUsersService->exAppUserExists($exApp->getAppid(), ''), + ]; + }, $exApps); } catch (Exception $e) { $this->logger->error(sprintf('Error while getting ExApps list. Error: %s', $e->getMessage()), ['exception' => $e]); $exApps = []; diff --git a/lib/Service/ExAppApiScopeService.php b/lib/Service/ExAppApiScopeService.php index 55143cdf..03abbc16 100644 --- a/lib/Service/ExAppApiScopeService.php +++ b/lib/Service/ExAppApiScopeService.php @@ -81,6 +81,7 @@ public function registerInitScopes(): bool { ['api_route' => $aeApiV1Prefix . '/ex-app/preference', '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' => 12, 'name' => 'NOTIFICATIONS'], // Cloud scopes From 9c5ed2abd05862ab1545774d169fc9b2640b02a6 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Wed, 9 Aug 2023 18:28:09 +0300 Subject: [PATCH 7/8] Fix array_filter output array indexing --- lib/Service/AppEcosystemV2Service.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Service/AppEcosystemV2Service.php b/lib/Service/AppEcosystemV2Service.php index f3f87ade..e90b3a5e 100644 --- a/lib/Service/AppEcosystemV2Service.php +++ b/lib/Service/AppEcosystemV2Service.php @@ -669,9 +669,9 @@ public function getExAppsList(string $list = 'enabled'): array { $exApps = $this->exAppMapper->findAll(); if ($list === 'enabled') { - $exApps = array_filter($exApps, function (ExApp $exApp) { + $exApps = array_values(array_filter($exApps, function (ExApp $exApp) { return $exApp->getEnabled() === 1; - }); + })); } $exApps = array_map(function (ExApp $exApp) { From 8c295ac9c730fc95949f21de9a91486a03512353 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Wed, 9 Aug 2023 18:40:12 +0300 Subject: [PATCH 8/8] fixed special test Signed-off-by: Alexander Piskun --- tests/app_version_higher.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/app_version_higher.py b/tests/app_version_higher.py index 48ac8dea..5497d55e 100644 --- a/tests/app_version_higher.py +++ b/tests/app_version_higher.py @@ -5,7 +5,7 @@ if __name__ == "__main__": nc_client = Nextcloud(nc_auth_user="admin", nc_auth_pass="admin") - assert nc_client.apps.is_disabled("nc_py_api") is False + assert nc_client.apps.ex_app_is_disabled("nc_py_api") is False nc_client.users.create("second_admin", password="2Very3Strong4", groups=["admin"]) nc_application = NextcloudApp(user="admin") @@ -16,11 +16,11 @@ nc_application.users.get_details() # this call should be rejected by AppEcosystem assert exc_info.value.status_code == 401 - assert nc_client.apps.is_disabled("nc_py_api") is True + assert nc_client.apps.ex_app_is_disabled("nc_py_api") is True notifications = nc_client.users.notifications.get_all() - notification = [i for i in notifications if i.info.subject == "ex_app_version_update"] + notification = [i for i in notifications if i.object_type == "ex_app_update"] assert len(notification) == 1 # only one notification for each admin nc_client = Nextcloud(nc_auth_user="second_admin", nc_auth_pass="2Very3Strong4") notifications = nc_client.users.notifications.get_all() - notification = [i for i in notifications if i.info.subject == "ex_app_version_update"] + notification = [i for i in notifications if i.object_type == "ex_app_update"] assert len(notification) == 1 # only one notification for each admin