ALL ApiScope (#190)

Added `ALL` ApiScope to allow the use of OCS Apis that we have not yet defined in the AppAPI. --------- Signed-off-by: Andrey Borysenko <[email protected]> Co-authored-by: Andrey Borysenko <[email protected]>
nextcloud · Jan 1, 2024 · a8863df · a8863df
1 parent 2ad52f0
commit a8863df
Show file tree

Hide file tree

Showing 8 changed files with 117 additions and 66 deletions.
diff --git a/.github/workflows/tests-special.yml b/.github/workflows/tests-special.yml
@@ -116,8 +116,8 @@ jobs:
           cd ..
           sleep 5s
           php occ app_api:daemon:register manual_install "Manual Install" manual-install 0 0 0
-          php occ app_api: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,\"scopes\":{\"required\":[\"SYSTEM\", \"FILES\", \"FILES_SHARING\"],\"optional\":[\"USER_INFO\", \"USER_STATUS\", \"NOTIFICATIONS\", \"WEATHER_STATUS\", \"TALK\"]},\"protocol\":\"http\",\"system_app\":1}" \
+          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\",\"host\":\"localhost\",\"port\":$APP_PORT,\"scopes\":{\"required\":[\"SYSTEM\", \"NOTIFICATIONS\"],\"optional\":[\"USER_INFO\"]},\"protocol\":\"http\",\"system_app\":1}" \
             --force-scopes --wait-finish
           kill -15 $(cat /tmp/_install.pid)
           timeout 3m tail --pid=$(cat /tmp/_install.pid) -f /dev/null
@@ -134,9 +134,9 @@ jobs:
           path: data/nextcloud.log
           if-no-files-found: warn
 
-  no-init-endpoint:
+  auth-tests-no-init:
     runs-on: ubuntu-22.04
-    name: Without Init Endpoint
+    name: Auth tests (no Init endpoint)
 
     services:
       postgres:
@@ -164,13 +164,6 @@ jobs:
           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 AppAPI
         uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
         with:
@@ -205,7 +198,6 @@ jobs:
           ./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 app:enable notifications
           ./occ app:enable --force ${{ env.APP_NAME }}
 
       - name: Run Nextcloud
@@ -221,18 +213,54 @@ jobs:
         working-directory: nc_py_api
         run: python3 -m pip -v install ".[dev]"
 
-      - name: Register NcPyApi
+      - name: Register App
         run: |
           python3 apps/${{ env.APP_NAME }}/tests/install_no_init.py &
           echo $! > /tmp/_install.pid
           sleep 5s
           php occ app_api:daemon:register manual_install "Manual Install" manual-install 0 0 0
-          php occ app_api: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,\"scopes\":{\"required\":[\"SYSTEM\", \"FILES\", \"FILES_SHARING\"],\"optional\":[\"USER_INFO\", \"USER_STATUS\", \"NOTIFICATIONS\", \"WEATHER_STATUS\", \"TALK\"]},\"protocol\":\"http\",\"system_app\":1}" \
+          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\",\"host\":\"localhost\",\"port\":$APP_PORT,\"scopes\":{\"required\":[\"ALL\"],\"optional\":[]},\"protocol\":\"http\",\"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: Check logs
+        run: grep -q 'Hello from ' data/nextcloud.log || error
+
+      - name: Test ALL Scope, System App
+        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 || 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\",\"host\":\"localhost\",\"port\":$APP_PORT,\"scopes\":{\"required\":[\"SYSTEM\"],\"optional\":[]},\"protocol\":\"http\",\"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, System App
+        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 || 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\",\"host\":\"localhost\",\"port\":$APP_PORT,\"scopes\":{\"required\":[\"ALL\"],\"optional\":[]},\"protocol\":\"http\",\"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
@@ -245,7 +273,7 @@ jobs:
     permissions:
       contents: none
     runs-on: ubuntu-22.04
-    needs: [app-version-higher, no-init-endpoint]
+    needs: [app-version-higher, auth-tests-no-init]
     name: TestsSpecial-OK
     steps:
       - run: echo "Tests special passed successfully"
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,11 +7,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
-## [1.4.5 - 202x-xx-xx]
+## [1.4.5 - 2024-01-02]
+
+### Added
+
+- Support for `ALL` API scope, that allows to call anyNextcloud endpoints bypassing API Scope check. #190
 
 ### Fixed
 
 - Fixed incorrect DeployConfig SSL params parsing. #188 (Thanks to @raudraido)
+- Incorrect HTTP status during invalid auth. #190
 
 ## [1.4.4 - 2023-12-21]
 

diff --git a/docs/tech_details/ApiScopes.rst b/docs/tech_details/ApiScopes.rst
@@ -20,18 +20,19 @@ but a subsequent version does, you can effortlessly specify the new API groups i
 
 The following API groups are currently supported:
 
-* ``2``   SYSTEM
-* ``10``  FILES
-* ``11``  FILES_SHARING
-* ``30``  USER_INFO
-* ``31``  USER_STATUS
-* ``32``  NOTIFICATIONS
-* ``33``  WEATHER_STATUS
-* ``50``  TALK
-* ``60``  TALK_BOT
-* ``61``  AI_PROVIDERS
-* ``110`` ACTIVITIES
-* ``120`` NOTES
+* ``2``     SYSTEM
+* ``10``    FILES
+* ``11``    FILES_SHARING
+* ``30``    USER_INFO
+* ``31``    USER_STATUS
+* ``32``    NOTIFICATIONS
+* ``33``    WEATHER_STATUS
+* ``50``    TALK
+* ``60``    TALK_BOT
+* ``61``    AI_PROVIDERS
+* ``110``   ACTIVITIES
+* ``120``   NOTES
+* ``9999``  ALL
 
 These groups are identified using names. As time progresses,
 the list will steadily expand, comprehensively encompassing all potential APIs provided by Nextcloud.

diff --git a/lib/Middleware/AppAPIAuthMiddleware.php b/lib/Middleware/AppAPIAuthMiddleware.php
@@ -23,26 +23,29 @@ class AppAPIAuthMiddleware extends Middleware {
 
 	public function __construct(
 		private AppAPIService   $service,
-		protected IRequest        $request,
+		protected IRequest      $request,
 		private IL10N           $l,
 		private LoggerInterface $logger,
 	) {
 	}
 
+	/**
+	 * @throws AppAPIAuthNotValidException when a security check fails
+	 * @throws \ReflectionException
+	 */
 	public function beforeController($controller, $methodName) {
 		$reflectionMethod = new ReflectionMethod($controller, $methodName);
 
 		$isAppAPIAuth = !empty($reflectionMethod->getAttributes(AppAPIAuth::class));
-
 		if ($isAppAPIAuth) {
 			if (!$this->service->validateExAppRequestToNC($this->request)) {
-				throw new AppAPIAuthNotValidException($this->l->t('AppAPIAuth authentication failed'), Http::STATUS_UNAUTHORIZED);
+				throw new AppAPIAuthNotValidException($this->l->t('AppAPI authentication failed'), Http::STATUS_UNAUTHORIZED);
 			}
 		}
 	}
 
 	/**
-	 * If an AEAuthNotValidException is being caught
+	 * If an AppAPIAuthNotValidException is being caught
 	 *
 	 * @param Controller $controller the controller that is being called
 	 * @param string $methodName the name of the method that will be called on
@@ -52,21 +55,11 @@ public function beforeController($controller, $methodName) {
 	 * @throws Exception the passed in exception if it can't handle it
 	 */
 	public function afterException($controller, $methodName, Exception $exception): Response {
-		if ($exception instanceof AppAPIAuth) {
-			$response = new JSONResponse([
-				'message' => $exception->getMessage(),
-			]);
-			if (stripos($this->request->getHeader('Accept'), 'html') === false) {
-				$response = new JSONResponse(
-					['message' => $exception->getMessage()],
-					$exception->getCode()
-				);
-			}
-
+		if ($exception instanceof AppAPIAuthNotValidException) {
 			$this->logger->debug($exception->getMessage(), [
 				'exception' => $exception,
 			]);
-			return $response;
+			return new JSONResponse(['message' => $exception->getMessage()], $exception->getCode());
 		}
 
 		throw $exception;

diff --git a/lib/Service/AppAPIService.php b/lib/Service/AppAPIService.php
@@ -38,7 +38,6 @@
 use SimpleXMLElement;
 
 class AppAPIService {
-	public const BASIC_API_SCOPE = 1;
 	public const CACHE_TTL = 60 * 60; // 1 hour
 
 	private ICache $cache;
@@ -737,21 +736,27 @@ public function validateExAppRequestToNC(IRequest $request, bool $isDav = false)
 			} else {
 				$path = '/dav/';
 			}
+
+			$allScopesFlag = (bool)$this->exAppScopesService->getByScope($exApp, ExAppApiScopeService::ALL_API_SCOPE);
 			$apiScope = $this->exAppApiScopeService->getApiScopeByRoute($path);
 
-			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->getScopeGroup() !== self::BASIC_API_SCOPE) {
-				if (!$this->exAppScopesService->passesScopeCheck($exApp, $apiScope->getScopeGroup())) {
-					$this->logger->error(sprintf('ExApp %s not passed scope group check %s', $exApp->getAppid(), $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->getScopeGroup() !== ExAppApiScopeService::BASIC_API_SCOPE) {
+					if (!$this->exAppScopesService->passesScopeCheck($exApp, $apiScope->getScopeGroup())) {
+						$this->logger->error(sprintf('ExApp %s not passed scope group check %s', $exApp->getAppid(), $path));
+						return false;
+					}
+				}
 			}
+
 			// For APIs that not assuming work under user context we do not check ExApp users
-			if ($apiScope->getUserCheck()) {
+			if (($apiScope === null) or ($apiScope->getUserCheck())) {
 				try {
 					if (!$this->exAppUsersService->exAppUserExists($exApp->getAppid(), $userId)) {
 						$this->logger->error(sprintf('ExApp %s user %s does not exist', $exApp->getAppid(), $userId));

diff --git a/lib/Service/ExAppApiScopeService.php b/lib/Service/ExAppApiScopeService.php
@@ -16,6 +16,8 @@
 use Psr\Log\LoggerInterface;
 
 class ExAppApiScopeService {
+	public const BASIC_API_SCOPE = 1;
+	public const ALL_API_SCOPE = 9999;
 	private ICache $cache;
 
 	public function __construct(
@@ -115,6 +117,9 @@ public function registerInitScopes(): bool {
 			['api_route' => '/apps/spreed/api/', 'scope_group' => 50, 'name' => 'TALK', 'user_check' => 1],
 			['api_route' => '/apps/activity/api/', 'scope_group' => 110, 'name' => 'ACTIVITIES', 'user_check' => 1],
 			['api_route' => '/apps/notes/api/', 'scope_group' => 120, 'name' => 'NOTES', 'user_check' => 1],
+
+			//ALL Scope
+			['api_route' => 'non-exist-all-api-route', 'scope_group' => self::ALL_API_SCOPE, 'name' => 'ALL', 'user_check' => 1],
 		];
 
 		$this->cache->clear('/all_api_scopes');

diff --git a/tests/auth_scopes_system.py b/tests/auth_scopes_system.py
@@ -0,0 +1,25 @@
+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()
diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml
@@ -96,17 +96,6 @@
       <code>IEventListener</code>
     </MissingTemplateParam>
   </file>
-  <file src="lib/Middleware/AppAPIAuthMiddleware.php">
-    <TypeDoesNotContainType>
-      <code>$exception instanceof AppAPIAuth</code>
-    </TypeDoesNotContainType>
-    <UndefinedMethod>
-      <code>getCode</code>
-      <code>getMessage</code>
-      <code>getMessage</code>
-      <code>getMessage</code>
-    </UndefinedMethod>
-  </file>
   <file src="lib/Profiler/AppAPIDataCollector.php">
     <UndefinedClass>
       <code>Request</code>