optimization: system flag and ex_app_users removal (#323)

The system flag and extra table for that `ex_app_users` is removed to optimize database usage and simplify the system logic. --------- Signed-off-by: Andrey Borysenko <[email protected]>
nextcloud · Jul 11, 2024 · eea14dc · eea14dc
1 parent ec09c69
commit eea14dc
Show file tree

Hide file tree

Showing 29 changed files with 117 additions and 580 deletions.
diff --git a/.github/workflows/tests-special.yml b/.github/workflows/tests-special.yml
@@ -228,7 +228,7 @@ jobs:
       - name: Check logs
         run: grep -q 'Hello from ' data/nextcloud.log || error
 
-      - name: Test ALL Scope, System App
+      - name: Test ALL Scope
         run: python3 apps/${{ env.APP_NAME }}/tests/auth_scopes_system.py 0
 
       - name: Re-Register App
@@ -243,24 +243,9 @@ jobs:
           kill -15 $(cat /tmp/_install.pid)
           timeout 3m tail --pid=$(cat /tmp/_install.pid) -f /dev/null
 
-      - name: Test NO ALL Scope, System App
+      - name: Test NO ALL Scope
         run: python3 apps/${{ env.APP_NAME }}/tests/auth_scopes_system.py 1
 
-      - 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\":[\"ALL\"],\"system_app\":0}" \
-            --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, NO System App
-        run: python3 apps/${{ env.APP_NAME }}/tests/auth_scopes_system.py 2
-
       - name: Upload NC logs
         if: always()
         uses: actions/upload-artifact@v3

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,8 @@ Note: Nextcloud 27 is no longer supported since this version.
 ### Changed
 
 - Dropped support of Nextcloud 27. #322
+- ExApp system flag is now deprecated and removed to optimize performance and simplicity. #323
+- PublicFunctions changes: `exAppRequestWithUserInit` and `asyncExAppRequestWithUserInit` are now deprecated. #323
 
 ### Fixed
 

diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -89,7 +89,6 @@ to join us in shaping a more versatile, stable, and secure app landscape.
 		<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\ExApp\Users\ListUsers</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/Concepts.rst b/docs/Concepts.rst
@@ -9,15 +9,6 @@ Each application defines list of API groups it intends to access.
 This system easily allows you to increase the level of trust in applications.
 Even prior to installation, it's possible to ascertain the API groups to which an application will gain access.
 
-System & Non System apps
-------------------------
-
-The main difference between a System app and a regular app is that a System app can impersonate a user who hasn't interacted with the app before.
-And normal applications cannot impersonate a user and call any APIs in the context of the user if the user has never used this application before.
-
-Given that numerous applications do not require system-level status,
-this approach significantly heightens security and fosters greater trust in new and lesser-known applications.
-
 Extensible Deployment
 ---------------------
 

diff --git a/docs/ManagingExternalApplications.rst b/docs/ManagingExternalApplications.rst
@@ -103,18 +103,8 @@ ListExApps command will show all ExApps:
 
     ExApps:
     appid (Display Name): version [enabled/disabled]
-    to_gif (ToGif): 1.0.0 [enabled]
-    upscaler_demo (Upscaler Demo): 1.0.0 [enabled]
-
-List ExApp users
-----------------
-
-Command: ``app_api:app:users:list <appid>``
-
-System user
-***********
-
-System user (``[system user]``) in the list means that this ExApp was setup as a system ExApp.
+    to_gif_example (To Gif Example): 1.0.0 [enabled]
+    upscaler_example (Upscaler Example): 1.0.0 [enabled]
 
 Using the ExApp Management UI
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff --git a/docs/tech_details/Authentication.rst b/docs/tech_details/Authentication.rst
@@ -65,8 +65,6 @@ Authentication flow in details
 		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 interacted with ExApp
-		AppAPI-->>Nextcloud: Reject if user has not interacted with ExApp (attempt to bypass user)
 		AppAPI-->>AppAPI: Check if user is not empty and active
 		AppAPI-->>Nextcloud: Set active user
 		AppAPI->>-Nextcloud: Request accepted/rejected
@@ -87,13 +85,9 @@ After successful authentication AppAPI sets `app_api` session key to ``true``.
 .. code-block:: php
 
 	$this->session->set('app_api', true);
+	$this->session->set('app_api_system', true); // deprecated since AppAPI 2.8.0
 
-.. note:: The Nextcloud server verifies this session key and allows **CORS protection** and **Two-Factor authentication** to be bypassed for requests coming from ExApps.
+.. note::
 
-For ``System`` applications additional flag is set:
-
-.. code-block:: php
-
-	$this->session->set('app_api_system', true);
-
-.. note:: The Nextcloud Server skips rate limiting for requests coming from ``System`` ExApps.
+	The Nextcloud server verifies this session key and allows **CORS protection** and **Two-Factor authentication** to be bypassed for requests coming from ExApps.
+	Also the rate limit is not applied to requests coming from ExApps.
diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php
@@ -12,7 +12,6 @@
 use OCA\AppAPI\Listener\FileEventsListener;
 use OCA\AppAPI\Listener\LoadFilesPluginListener;
 use OCA\AppAPI\Listener\SabrePluginAuthInitListener;
-use OCA\AppAPI\Listener\UserDeletedListener;
 use OCA\AppAPI\Middleware\AppAPIAuthMiddleware;
 use OCA\AppAPI\Middleware\ExAppUIL10NMiddleware;
 use OCA\AppAPI\Middleware\ExAppUiMiddleware;
@@ -47,7 +46,6 @@
 use OCP\Settings\Events\DeclarativeSettingsGetValueEvent;
 use OCP\Settings\Events\DeclarativeSettingsRegisterFormEvent;
 use OCP\Settings\Events\DeclarativeSettingsSetValueEvent;
-use OCP\User\Events\UserDeletedEvent;
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\NotFoundExceptionInterface;
 use Throwable;
@@ -74,7 +72,6 @@ public function register(IRegistrationContext $context): void {
 		$context->registerMiddleware(ExAppUiMiddleware::class);
 		$context->registerMiddleware(ExAppUIL10NMiddleware::class, true);
 		$context->registerEventListener(SabrePluginAuthInitEvent::class, SabrePluginAuthInitListener::class);
-		$context->registerEventListener(UserDeletedEvent::class, UserDeletedListener::class);
 		$context->registerNotifierService(ExAppNotifier::class);
 		$context->registerNotifierService(ExAppAdminNotifier::class);
 

diff --git a/lib/Command/ExApp/Notify.php b/lib/Command/ExApp/Notify.php
@@ -53,7 +53,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
 		$route = $input->getArgument('route');
 		$userId = $input->getOption('user-id');
-		$response = $this->service->aeRequestToExApp($exApp, $route, $userId, params: $eventJsonData,
+		$response = $this->service->requestToExApp($exApp, $route, $userId, params: $eventJsonData,
 			options: ['timeout' => 2]);
 		if (is_array($response) && isset($response['error'])) {
 			$output->writeln(sprintf('Failed to notify ExApp %s: %s', $appId, $response['error']));

diff --git a/lib/Command/ExApp/Users/ListUsers.php b/lib/Command/ExApp/Users/ListUsers.php
diff --git a/lib/Controller/ExAppProxyController.php b/lib/Controller/ExAppProxyController.php
@@ -81,7 +81,7 @@ public function ExAppGet(string $appId, string $other): Response {
 			return new NotFoundResponse();
 		}
 
-		$response = $this->service->aeRequestToExApp2(
+		$response = $this->service->requestToExApp2(
 			$exApp, '/' . $other, $this->userId, 'GET', queryParams: $_GET, options: [
 				RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $this->service->getExAppDomain($exApp)),
 			],
@@ -110,7 +110,7 @@ public function ExAppPost(string $appId, string $other): Response {
 		}
 		$bodyParams = $this->prepareBodyParams($this->request->getParams());
 
-		$response = $this->service->aeRequestToExApp2(
+		$response = $this->service->requestToExApp2(
 			$exApp, '/' . $other, $this->userId,
 			queryParams: $_GET, bodyParams: $bodyParams, options: $options,
 			request: $this->request,
@@ -138,7 +138,7 @@ public function ExAppPut(string $appId, string $other): Response {
 		}
 		$bodyParams = $this->prepareBodyParams($this->request->getParams());
 
-		$response = $this->service->aeRequestToExApp2(
+		$response = $this->service->requestToExApp2(
 			$exApp, '/' . $other, $this->userId, 'PUT', queryParams: $_GET, bodyParams: $bodyParams,
 			options: $options,
 			request: $this->request,
@@ -158,7 +158,7 @@ public function ExAppDelete(string $appId, string $other): Response {
 		}
 
 		$bodyParams = $this->prepareBodyParams($this->request->getParams());
-		$response = $this->service->aeRequestToExApp2(
+		$response = $this->service->requestToExApp2(
 			$exApp, '/' . $other, $this->userId, 'DELETE', queryParams: $_GET, bodyParams: $bodyParams,
 			options: [
 				RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $this->service->getExAppDomain($exApp)),

diff --git a/lib/Controller/ExAppsPageController.php b/lib/Controller/ExAppsPageController.php
@@ -18,7 +18,6 @@
 use OCA\AppAPI\Service\DaemonConfigService;
 use OCA\AppAPI\Service\ExAppApiScopeService;
 use OCA\AppAPI\Service\ExAppService;
-use OCA\AppAPI\Service\ExAppUsersService;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
@@ -51,7 +50,6 @@ class ExAppsPageController extends Controller {
 	private IL10N $l10n;
 	private LoggerInterface $logger;
 	private IAppManager $appManager;
-	private ExAppUsersService $exAppUsersService;
 
 	public function __construct(
 		IRequest $request,
@@ -60,7 +58,6 @@ public function __construct(
 		AppAPIService $service,
 		DaemonConfigService $daemonConfigService,
 		ExAppApiScopeService $exAppApiScopeService,
-		ExAppUsersService $exAppUsersService,
 		DockerActions $dockerActions,
 		CategoryFetcher $categoryFetcher,
 		IFactory $l10nFactory,
@@ -84,7 +81,6 @@ public function __construct(
 		$this->exAppFetcher = $exAppFetcher;
 		$this->logger = $logger;
 		$this->appManager = $appManager;
-		$this->exAppUsersService = $exAppUsersService;
 	}
 
 	#[NoCSRFRequired]
@@ -243,7 +239,6 @@ private function getAppsForCategory(string $requestedCategory = ''): array {
 				'appstoreData' => $app,
 				'scopes' => $scopes,
 				'daemon' => $daemon,
-				'systemApp' => $exApp !== null && $this->exAppUsersService->exAppUserExists($exApp->getAppid(), ''),
 				'status' => $exApp !== null ? $exApp->getStatus() : [],
 				'error' => $exApp !== null ? $exApp->getStatus()['error'] ?? '' : '',
 			];
@@ -375,7 +370,6 @@ private function buildLocalAppsList(array $apps, array $exApps): array {
 					'appstoreData' => $app,
 					'scopes' => $scopes,
 					'daemon' => $daemon,
-					'systemApp' => $this->exAppUsersService->exAppUserExists($exApp->getAppid(), ''),
 					'exAppUrl' => $this->service->getExAppUrl($exApp, $exApp->getPort(), $auth),
 					'releases' => [],
 					'update' => null,

diff --git a/lib/Controller/OCSExAppController.php b/lib/Controller/OCSExAppController.php
@@ -98,6 +98,11 @@ public function requestToExApp(
 		]);
 	}
 
+	/**
+	 * TODO: remove later
+	 *
+	 * @deprecated since AppAPI 2.8.0
+	 */
 	#[NoCSRFRequired]
 	public function aeRequestToExApp(
 		string $appId,
@@ -111,7 +116,7 @@ public function aeRequestToExApp(
 		if ($exApp === null) {
 			return new DataResponse(['error' => sprintf('ExApp `%s` not found', $appId)]);
 		}
-		$response = $this->service->aeRequestToExApp($exApp, $route, $userId, $method, $params, $options, $this->request);
+		$response = $this->service->requestToExApp($exApp, $route, $userId, $method, $params, $options, $this->request);
 		if (is_array($response) && isset($response['error'])) {
 			return new DataResponse($response, Http::STATUS_BAD_REQUEST);
 		}

diff --git a/lib/Controller/TopMenuController.php b/lib/Controller/TopMenuController.php
@@ -6,7 +6,6 @@
 
 use OCA\AppAPI\AppInfo\Application;
 use OCA\AppAPI\Service\ExAppService;
-use OCA\AppAPI\Service\ExAppUsersService;
 use OCA\AppAPI\Service\UI\InitialStateService;
 use OCA\AppAPI\Service\UI\ScriptsService;
 use OCA\AppAPI\Service\UI\StylesService;
@@ -34,7 +33,6 @@ public function __construct(
 		private readonly InitialStateService $initialStateService,
 		private readonly ScriptsService      $scriptsService,
 		private readonly StylesService       $stylesService,
-		private readonly ExAppUsersService   $exAppUsersService,
 		private readonly ExAppService        $service,
 		private readonly ?string             $userId,
 		private readonly IGroupManager       $groupManager,
@@ -70,7 +68,6 @@ public function viewExAppPage(string $appId, string $name, string $other): Templ
 		$this->stylesService->applyExAppStyles($appId, 'top_menu', $menuEntry->getName());
 
 		$this->postprocess = true;
-		$this->exAppUsersService->setupExAppUser($exApp->getAppid(), $this->userId);
 		$response = new TemplateResponse(Application::APP_ID, 'embedded');
 		$csp = new ContentSecurityPolicy();
 		$csp->addAllowedScriptDomain($this->request->getServerHost());

diff --git a/lib/Db/ExApp.php b/lib/Db/ExApp.php
@@ -24,7 +24,6 @@
  * @method int getEnabled()
  * @method int getCreatedTime()
  * @method int getLastCheckTime()
- * @method int getIsSystem()
  * @method array getApiScopes()
  * @method array getDeployConfig()
  * @method string getAcceptsDeployId()
@@ -40,7 +39,6 @@
  * @method void setEnabled(int $enabled)
  * @method void setCreatedTime(int $createdTime)
  * @method void setLastCheckTime(int $lastCheckTime)
- * @method void setIsSystem(int $system)
  * @method void setApiScopes(array $apiScopes)
  * @method void setDeployConfig(array $deployConfig)
  * @method void setAcceptsDeployId(string $acceptsDeployId)
@@ -58,7 +56,6 @@ class ExApp extends Entity implements JsonSerializable {
 	protected $enabled;
 	protected $createdTime;
 	protected $lastCheckTime;
-	protected $isSystem;
 	protected $apiScopes;
 	protected $deployConfig;
 	protected $acceptsDeployId;
@@ -79,7 +76,6 @@ public function __construct(array $params = []) {
 		$this->addType('enabled', 'int');
 		$this->addType('createdTime', 'int');
 		$this->addType('lastCheckTime', 'int');
-		$this->addType('isSystem', 'int');
 		$this->addType('apiScopes', 'json');
 		$this->addType('deployConfig', 'json');
 		$this->addType('acceptsDeployId', 'string');
@@ -123,9 +119,6 @@ public function __construct(array $params = []) {
 		if (isset($params['last_check_time'])) {
 			$this->setLastCheckTime($params['last_check_time']);
 		}
-		if (isset($params['is_system'])) {
-			$this->setIsSystem($params['is_system']);
-		}
 		if (isset($params['api_scopes'])) {
 			$this->setApiScopes($params['api_scopes']);
 		} else {
@@ -154,7 +147,6 @@ public function jsonSerialize(): array {
 			'enabled' => $this->getEnabled(),
 			'created_time' => $this->getCreatedTime(),
 			'last_check_time' => $this->getLastCheckTime(),
-			'is_system' => $this->getIsSystem(),
 			'api_scopes' => $this->getApiScopes(),
 			'deploy_config' => $this->getDeployConfig(),
 			'accepts_deploy_id' => $this->getAcceptsDeployId(),

diff --git a/lib/Db/ExAppMapper.php b/lib/Db/ExAppMapper.php
@@ -114,8 +114,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 === 'is_system') {
-				$qb = $qb->set('is_system', $qb->createNamedParameter($exApp->getIsSystem(), IQueryBuilder::PARAM_INT));
 			} elseif ($field === 'api_scopes') {
 				$qb = $qb->set('api_scopes', $qb->createNamedParameter($exApp->getApiScopes(), IQueryBuilder::PARAM_JSON));
 			}