Skip to content
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
60 changes: 44 additions & 16 deletions .github/workflows/tests-special.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
25 changes: 13 additions & 12 deletions docs/tech_details/ApiScopes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 9 additions & 16 deletions lib/Middleware/AppAPIAuthMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
25 changes: 15 additions & 10 deletions lib/Service/AppAPIService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down
5 changes: 5 additions & 0 deletions lib/Service/ExAppApiScopeService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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');
Expand Down
25 changes: 25 additions & 0 deletions tests/auth_scopes_system.py
Original file line number Diff line number Diff line change
@@ -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()
11 changes: 0 additions & 11 deletions tests/psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down