Skip to content

Commit

Permalink
authorization-app-api, auth simplification (#69)
Browse files Browse the repository at this point in the history
Signed-off-by: Andrey Borysenko <andrey18106x@gmail.com>
Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
Co-authored-by: Alexander Piskun <bigcat88@icloud.com>
  • Loading branch information
andrey18106 and bigcat88 authored Sep 13, 2023
1 parent 8b3f129 commit 9c68821
Show file tree
Hide file tree
Showing 15 changed files with 73 additions and 245 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/tests-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
ref: ${{ matrix.server-version }}
path: apps/notifications

- name: Checkout AppEcosystemV2
- name: Checkout AppAPI
uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
with:
path: apps/${{ env.APP_NAME }}
Expand Down Expand Up @@ -159,7 +159,7 @@ jobs:
sudo chmod 766 /var/run/docker.sock
sleep 120s
- name: Install AppEcosystemV2
- name: Install AppAPI
run: |
docker exec -w /var/www/html/apps nextcloud git clone https://github.com/cloud-py-api/${{ env.APP_NAME }}.git
docker exec -w /var/www/html/apps/${{ env.APP_NAME }} nextcloud git fetch origin $GITHUB_REF
Expand Down Expand Up @@ -235,7 +235,7 @@ jobs:
-v `pwd`/certs:/data/certs kekru/docker-remote-api-tls:master
sleep 30s
- name: Install AppEcosystemV2
- name: Install AppAPI
run: |
docker exec -w /var/www/html/apps nextcloud git clone https://github.com/cloud-py-api/${{ env.APP_NAME }}.git
docker exec -w /var/www/html/apps/${{ env.APP_NAME }} nextcloud git fetch origin $GITHUB_REF
Expand Down Expand Up @@ -312,7 +312,7 @@ jobs:
-v `pwd`/certs:/data/certs kekru/docker-remote-api-tls:master
sleep 30s
- name: Install AppEcosystemV2
- name: Install AppAPI
run: |
docker exec -w /var/www/html/apps nextcloud git clone https://github.com/cloud-py-api/${{ env.APP_NAME }}.git
docker exec -w /var/www/html/apps/${{ env.APP_NAME }} nextcloud git fetch origin $GITHUB_REF
Expand Down Expand Up @@ -414,7 +414,7 @@ jobs:
repository: nextcloud/server
ref: ${{ matrix.server-version }}

- name: Checkout AppEcosystemV2
- name: Checkout AppAPI
uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
with:
path: apps/${{ env.APP_NAME }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests-special.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
ref: ${{ matrix.server-version }}
path: apps/notifications

- name: Checkout AppEcosystemV2
- name: Checkout AppAPI
uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
with:
path: apps/${{ env.APP_NAME }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ jobs:
ref: ${{ matrix.server-version }}
path: apps/notifications

- name: Checkout AppEcosystemV2
- name: Checkout AppAPI
uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
with:
path: apps/${{ env.APP_NAME }}
Expand Down
57 changes: 11 additions & 46 deletions base_php.patch
Original file line number Diff line number Diff line change
@@ -1,72 +1,37 @@
From 1ed62d007ce19934edf71740099665e661fbb368 Mon Sep 17 00:00:00 2001
From: Andrey Borysenko <andrey18106x@gmail.com>
Date: Tue, 27 Jun 2023 15:05:52 +0300
Subject: [PATCH 1/2] Added app_ecosystem_v2 auth to base.php

---
lib/base.php | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/lib/base.php b/lib/base.php
index 09ec5be441b5..b49491e61b02 100644
index 732fc55be3e..db2c2866b7b 100644
--- a/lib/base.php
+++ b/lib/base.php
@@ -1133,6 +1133,9 @@ public static function handleLogin(OCP\IRequest $request): bool {
@@ -1132,6 +1132,9 @@ class OC {
if (OC_User::handleApacheAuth()) {
return true;
}
+ if (self::tryAppEcosystemV2Login($request)) {
+ if (self::tryAppAPILogin($request)) {
+ return true;
+ }
if ($userSession->tryTokenLogin($request)) {
return true;
}
@@ -1170,6 +1173,21 @@ protected static function handleAuthHeaders(): void {
@@ -1169,6 +1172,22 @@ class OC {
}
}
}
+
+ protected static function tryAppEcosystemV2Login(OCP\IRequest $request): bool {
+ protected static function tryAppAPILogin(OCP\IRequest $request): bool {
+ $appManager = Server::get(OCP\App\IAppManager::class);
+ if (!$request->getHeader('AE-SIGNATURE')) {
+ if (!$request->getHeader('AUTHORIZATION-APP-API')) {
+ return false;
+ }
+ if (!$appManager->isAppLoaded('app_ecosystem_v2')) {
+ if (!$appManager->isInstalled('app_api')) {
+ return false;
+ }
+ if (!$appManager->isInstalled('app_ecosystem_v2')) {
+ try {
+ $appAPIService = Server::get(OCA\AppAPI\Service\AppAPIService::class);
+ return $appAPIService->validateExAppRequestToNC($request);
+ } catch (\Psr\Container\NotFoundExceptionInterface|\Psr\Container\ContainerExceptionInterface $e) {
+ return false;
+ }
+ $appEcosystemV2Service = Server::get(OCA\AppEcosystemV2\Service\AppEcosystemV2Service::class);
+ return $appEcosystemV2Service->validateExAppRequestToNC($request);
+ }
}

OC::init();

From b8f1a58f7daf2ade87f2600d37b1926fdfbd949a Mon Sep 17 00:00:00 2001
From: Alexander Piskun <13381981+bigcat88@users.noreply.github.com>
Date: Thu, 13 Jul 2023 14:17:29 +0300
Subject: [PATCH 2/2] Update base.php

Support 26 Nextcloud

Signed-off-by: Alexander Piskun <13381981+bigcat88@users.noreply.github.com>
---
lib/base.php | 3 ---
1 file changed, 3 deletions(-)

diff --git a/lib/base.php b/lib/base.php
index b49491e61b02..27556a10acc3 100644
--- a/lib/base.php
+++ b/lib/base.php
@@ -1179,9 +1179,6 @@ protected static function tryAppEcosystemV2Login(OCP\IRequest $request): bool {
if (!$request->getHeader('AE-SIGNATURE')) {
return false;
}
- if (!$appManager->isAppLoaded('app_ecosystem_v2')) {
- return false;
- }
if (!$appManager->isInstalled('app_ecosystem_v2')) {
return false;
}
49 changes: 11 additions & 38 deletions docs/tech_details/Authentication.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ Authentication
==============

AppAPI introduces a distinct method of authentication for external apps.

This authentication relies on a shared secret between Nextcloud and the external app, which generates a unique signature for each request.
This authentication relies on a shared secret between Nextcloud and the external app

Authentication flow
^^^^^^^^^^^^^^^^^^^
Expand All @@ -30,35 +29,12 @@ Authentication flow
Authentication headers
^^^^^^^^^^^^^^^^^^^^^^

Each ExApp request to secured API with AppEcosystemAuth must contain the following headers (order is important):
Each ExApp request to secured API with AppAPIAuth must contain the following headers:

1. ``AE-VERSION`` - minimal version of the AppAPI
1. ``AA-VERSION`` - minimal version of the AppAPI
2. ``EX-APP-ID``- ID of the ExApp
3. ``EX-APP-VERSION`` - version of the ExApp
4. ``NC-USER-ID`` - the user under which the request is made, can be empty in case of system apps (see :ref:`api_scopes`)
5. ``AE-DATA-HASH`` - hash of the request body (see details in `ae_signature`_ section)
6. ``AE-SIGN-TIME`` - Unix timestamp of the request
7. ``AE-SIGNATURE`` - signature of the request (see details `ae_signature`_ section)


AE_SIGNATURE
************

AppApi signature (AE-SIGNATURE) is a HMAC-SHA256 hash of the request signed with the shared secret.

The signature is calculated from the following data:

* method
* uri (with urlencoded query parameters)
* headers (``AE-VERSION``, ``EX-APP-ID``, ``EX-APP-VERSION``, ``NC-USER-ID``, ``AE-DATA-HASH``, ``AE-SIGN-TIME``)
* xxh64 hash from request body (post data, json, files, etc.)

AE_DATA_HASH
************

``AE-DATA-HASH`` header must contain a xxh64 hash of the request body.
It's calculated even if the request body is empty (e.g. empty hash: ``ef46db3751d8e999``).

4. ``AUTHORIZATION-APP-API`` - base64 encoded ``userid:secret``

Authentication flow in details
******************************
Expand All @@ -74,20 +50,17 @@ Authentication flow in details
participant AppApi
end
ExApp->>+Nextcloud: Request to API
Nextcloud->>Nextcloud: Check if AE-SIGNATURE header exists
Nextcloud-->>ExApp: Reject if AE-SIGNATURE header not exists
Nextcloud->>Nextcloud: Check if AppApi enabled
Nextcloud-->>ExApp: Reject if AppApi not enabled
Nextcloud->>Nextcloud: Check if AUTHORIZATION-APP-API header exists
Nextcloud-->>ExApp: Reject if AUTHORIZATION-APP-API header not exists
Nextcloud->>Nextcloud: Check if AppApi app is enabled
Nextcloud-->>ExApp: Reject if AppApi is not exists or disabled
Nextcloud->>+AppApi: Validate request
AppApi-->>AppApi: Check if ExApp exists and enabled
AppApi-->>Nextcloud: Reject if ExApp not exists or disabled
AppApi-->>AppApi: Check if ExApp version changed
AppApi-->>AppApi: Validate AE-SIGN-TIME
AppApi-->>Nextcloud: Reject if sign time diff > 5 min
AppApi-->>AppApi: Generate and validate AE-SIGNATURE
AppApi-->>Nextcloud: Reject if signature not match
AppApi-->>AppApi: Validate AE-DATA-HASH
AppApi-->>Nextcloud: Reject if data hash not match
AppApi-->>Nextcloud: Disable ExApp and notify admins if version changed
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 interacted with ExApp
Expand Down
6 changes: 3 additions & 3 deletions docs/tech_details/api/index.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
===================
AppEcosystemV2 APIs
===================
=====================
AppAPI Nextcloud APIs
=====================

.. note::

Expand Down
4 changes: 2 additions & 2 deletions lib/AppAPIAuthBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ public function __construct(
}

public function check(RequestInterface $request, ResponseInterface $response) {
if ($this->request->getHeader('AE-SIGNATURE')) {
if ($this->request->getHeader('AUTHORIZATION-APP-API')) {
$davAuthenticated = $this->session->get(Auth::DAV_AUTHENTICATED);
$userIdHeader = $this->request->getHeader('NC-USER-ID');
$userIdHeader = explode(':', base64_decode($this->request->getHeader('AUTHORIZATION-APP-API')), 2)[0];
$sessionUserId = $this->session->get('user_id');
if ($sessionUserId === $userIdHeader && $davAuthenticated === $sessionUserId) {
$authString = 'principals/' . Application::APP_ID . '/' . $this->session->get('user_id');
Expand Down
4 changes: 2 additions & 2 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use OCA\AppAPI\Middleware\AppAPIAuthMiddleware;
use OCA\AppAPI\Notifications\ExAppAdminNotifier;
use OCA\AppAPI\Notifications\ExAppNotifier;
use OCA\AppAPI\Profiler\AEDataCollector;
use OCA\AppAPI\Profiler\AppAPIDataCollector;
use OCA\AppAPI\PublicCapabilities;

use OCA\DAV\Events\SabrePluginAuthInitEvent;
Expand Down Expand Up @@ -54,7 +54,7 @@ public function boot(IBootContext $context): void {
try {
$profiler = $server->get(IProfiler::class);
if ($profiler->isEnabled()) {
$profiler->add(new AEDataCollector());
$profiler->add(new AppAPIDataCollector());
}
} catch (NotFoundExceptionInterface|ContainerExceptionInterface) {
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/NotificationsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function __construct(
#[NoCSRFRequired]
public function sendNotification(array $params): Response {
$appId = $this->request->getHeader('EX-APP-ID');
$userId = $this->request->getHeader('NC-USER-ID');
$userId = explode(':', base64_decode($this->request->getHeader('AUTHORIZATION-APP-API')), 2)[0];
$notification = $this->exNotificationsManager->sendNotification($appId, $userId, $params);
return new DataResponse($this->notificationToArray($notification), Http::STATUS_OK);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/DavPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ public function initialize(Server $server) {
}

public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
if ($this->request->getHeader('AE-SIGNATURE')) {
if ($this->request->getHeader('AUTHORIZATION-APP-API')) {
if ($this->service->validateExAppRequestToNC($this->request, true)) {
$this->session->set(Auth::DAV_AUTHENTICATED, $this->request->getHeader('NC-USER-ID'));
$this->session->set(Auth::DAV_AUTHENTICATED, explode(':', base64_decode($this->request->getHeader('AUTHORIZATION-APP-API')), 2)[0]);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,29 @@
/**
* @psalm-suppress UndefinedClass
*/
class AEDataCollector extends AbstractDataCollector {
class AppAPIDataCollector extends AbstractDataCollector {
public function getName(): string {
return Application::APP_ID;
}

public function collect(Request $request, Response $response, \Throwable $exception = null): void {
// TODO: Check why DAV requests missing AE headers data
$headers = [];
$aeHeadersList = [
'AE-VERSION',
'AA-VERSION',
'EX-APP-ID',
'EX-APP-VERSION',
'NC-USER-ID',
'AE-DATA-HASH',
'AE-SIGN-TIME',
'AE-SIGNATURE',
'AE-REQUEST-ID',
'AUTHORIZATION-APP-API',
'AA-REQUEST-ID',
];
foreach ($aeHeadersList as $header) {
if ($request->getHeader($header) !== '') {
if ($header === 'AUTHORIZATION-APP-API') {
$authorization = $request->getHeader($header);
$headers[$header] = $authorization;
$headers['NC-USER-ID'] = explode(':', base64_decode($authorization), 2)[0];
$headers['EX-APP-SECRET'] = explode(':', base64_decode($authorization), 2)[1];
continue;
}
$headers[$header] = $request->getHeader($header);
}
}
Expand Down
Loading

0 comments on commit 9c68821

Please sign in to comment.