Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
feat(AppFramework): Add ExAppRequired attribute
Signed-off-by: provokateurin <[email protected]>
  • Loading branch information
provokateurin committed Jul 1, 2024
commit 5aefdc399eb17a86f3c2b59713ca6448479f99fd
2 changes: 2 additions & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
'OCP\\AppFramework\\Http\\Attribute\\AuthorizedAdminSetting' => $baseDir . '/lib/public/AppFramework/Http/Attribute/AuthorizedAdminSetting.php',
'OCP\\AppFramework\\Http\\Attribute\\BruteForceProtection' => $baseDir . '/lib/public/AppFramework/Http/Attribute/BruteForceProtection.php',
'OCP\\AppFramework\\Http\\Attribute\\CORS' => $baseDir . '/lib/public/AppFramework/Http/Attribute/CORS.php',
'OCP\\AppFramework\\Http\\Attribute\\ExAppRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/ExAppRequired.php',
'OCP\\AppFramework\\Http\\Attribute\\FrontpageRoute' => $baseDir . '/lib/public/AppFramework/Http/Attribute/FrontpageRoute.php',
'OCP\\AppFramework\\Http\\Attribute\\IgnoreOpenAPI' => $baseDir . '/lib/public/AppFramework/Http/Attribute/IgnoreOpenAPI.php',
'OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php',
Expand Down Expand Up @@ -887,6 +888,7 @@
'OC\\AppFramework\\Middleware\\Security\\CSPMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\ExAppRequiredException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/ExAppRequiredException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\LaxSameSiteCookieFailedException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/LaxSameSiteCookieFailedException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotAdminException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotAdminException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotConfirmedException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotConfirmedException.php',
Expand Down
2 changes: 2 additions & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OCP\\AppFramework\\Http\\Attribute\\AuthorizedAdminSetting' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/AuthorizedAdminSetting.php',
'OCP\\AppFramework\\Http\\Attribute\\BruteForceProtection' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/BruteForceProtection.php',
'OCP\\AppFramework\\Http\\Attribute\\CORS' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/CORS.php',
'OCP\\AppFramework\\Http\\Attribute\\ExAppRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/ExAppRequired.php',
'OCP\\AppFramework\\Http\\Attribute\\FrontpageRoute' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/FrontpageRoute.php',
'OCP\\AppFramework\\Http\\Attribute\\IgnoreOpenAPI' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/IgnoreOpenAPI.php',
'OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php',
Expand Down Expand Up @@ -920,6 +921,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\AppFramework\\Middleware\\Security\\CSPMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\ExAppRequiredException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/ExAppRequiredException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\LaxSameSiteCookieFailedException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/LaxSameSiteCookieFailedException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotAdminException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotAdminException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotConfirmedException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotConfirmedException.php',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\AppFramework\Middleware\Security\Exceptions;

use OCP\AppFramework\Http;

/**
* Class ExAppRequiredException is thrown when an endpoint can only be called by an ExApp but the caller is not an ExApp.
*/
class ExAppRequiredException extends SecurityException {
public function __construct() {
parent::__construct('ExApp required', Http::STATUS_PRECONDITION_FAILED);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,19 @@

use OC\AppFramework\Middleware\Security\Exceptions\AppNotEnabledException;
use OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException;
use OC\AppFramework\Middleware\Security\Exceptions\ExAppRequiredException;
use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
use OC\AppFramework\Middleware\Security\Exceptions\NotLoggedInException;
use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
use OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Settings\AuthorizedGroupMapper;
use OC\User\Session;
use OCP\App\AppPathNotFoundException;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
use OCP\AppFramework\Http\Attribute\ExAppRequired;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
Expand Down Expand Up @@ -127,7 +130,12 @@ public function beforeController($controller, $methodName) {

// security checks
$isPublicPage = $this->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class);
if (!$isPublicPage) {

if ($this->hasAnnotationOrAttribute($reflectionMethod, 'ExAppRequired', ExAppRequired::class)) {
if (!$this->userSession instanceof Session || $this->userSession->getSession()->get('app_api') !== true) {
throw new ExAppRequiredException();
}
} elseif (!$isPublicPage) {
if (!$this->isLoggedIn) {
throw new NotLoggedInException();
}
Expand Down
21 changes: 21 additions & 0 deletions lib/public/AppFramework/Http/Attribute/ExAppRequired.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCP\AppFramework\Http\Attribute;

use Attribute;

/**
* Attribute for controller methods that can only be accessed by ExApps
*
* @since 30.0.0
*/
#[Attribute]
class ExAppRequired {
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace Test\AppFramework\Middleware\Security\Mock;

use OCP\AppFramework\Http\Attribute\ExAppRequired;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
Expand Down Expand Up @@ -156,4 +157,14 @@ public function testAnnotationNoAdminRequiredNoCSRFRequiredPublicPage() {
#[PublicPage]
public function testAttributeNoAdminRequiredNoCSRFRequiredPublicPage() {
}

/**
* @ExAppRequired
*/
public function testAnnotationExAppRequired() {
}

#[ExAppRequired]
public function testAttributeExAppRequired() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@
use OC\AppFramework\Http\Request;
use OC\AppFramework\Middleware\Security\Exceptions\AppNotEnabledException;
use OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException;
use OC\AppFramework\Middleware\Security\Exceptions\ExAppRequiredException;
use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
use OC\AppFramework\Middleware\Security\Exceptions\NotLoggedInException;
use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
use OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException;
use OC\AppFramework\Middleware\Security\SecurityMiddleware;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Settings\AuthorizedGroupMapper;
use OC\User\Session;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\RedirectResponse;
Expand All @@ -27,6 +29,7 @@
use OCP\INavigationManager;
use OCP\IRequest;
use OCP\IRequestId;
use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -66,7 +69,7 @@ protected function setUp(): void {
parent::setUp();

$this->authorizedGroupMapper = $this->createMock(AuthorizedGroupMapper::class);
$this->userSession = $this->createMock(IUserSession::class);
$this->userSession = $this->createMock(Session::class);
$this->request = $this->createMock(IRequest::class);
$this->controller = new SecurityMiddlewareController(
'test',
Expand Down Expand Up @@ -167,6 +170,13 @@ public function dataNoCSRFRequiredSubAdminRequired(): array {
];
}

public static function dataExAppRequired(): array {
return [
['testAnnotationExAppRequired'],
['testAttributeExAppRequired'],
];
}

/**
* @dataProvider dataNoCSRFRequiredPublicPage
*/
Expand Down Expand Up @@ -682,4 +692,40 @@ public function testAfterAjaxExceptionReturnsJSONError() {

$this->assertTrue($response instanceof JSONResponse);
}

/**
* @dataProvider dataExAppRequired
*/
public function testExAppRequired(string $method): void {
$middleware = $this->getMiddleware(true, false, false);
$this->reader->reflect($this->controller, $method);

$session = $this->createMock(ISession::class);
$session->method('get')->with('app_api')->willReturn(true);
$this->userSession->method('getSession')->willReturn($session);

$this->request->expects($this->once())
->method('passesStrictCookieCheck')
->willReturn(true);
$this->request->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);

$middleware->beforeController($this->controller, $method);
}

/**
* @dataProvider dataExAppRequired
*/
public function testExAppRequiredError(string $method): void {
$middleware = $this->getMiddleware(true, false, false, false);
$this->reader->reflect($this->controller, $method);

$session = $this->createMock(ISession::class);
$session->method('get')->with('app_api')->willReturn(false);
$this->userSession->method('getSession')->willReturn($session);

$this->expectException(ExAppRequiredException::class);
$middleware->beforeController($this->controller, $method);
}
}