upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-02-03 20:41:22 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 13

feat(AppFramework): Add missing NoSameSiteCookieRequired attribute

Browse source 
Allow to replace the old annotation.

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
This commit is contained in:
Carl Schwan 2025-09-17 15:54:17 +02:00 committed by Carl Schwan
parent b040fb1c73
commit 6408ed0b51
No known key found for this signature in database
GPG key ID: 02325448204E452A
15 changed files with 115 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
apps/files_sharing/lib/Controller/PublicPreviewController.php
View file

@ -8,6 +8,7 @@ namespace OCA\Files_Sharing\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\DataResponse;
@ -144,8 +145,6 @@ class PublicPreviewController extends PublicShareController {
}
/**
* @NoSameSiteCookieRequired
*
* Get a direct link preview for a shared file
*
* @param string $token Token of the share
@ -159,6 +158,7 @@ class PublicPreviewController extends PublicShareController {
#[PublicPage]
#[NoCSRFRequired]
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
#[NoSameSiteCookieRequired]
public function directLink(string $token) {
// No token no image
if ($token === '') {

11
apps/files_sharing/lib/Controller/ShareController.php
View file

@ -15,12 +15,12 @@ use OCA\Files_Sharing\Event\ShareLinkAccessedEvent;
use OCP\Accounts\IAccountManager;
use OCP\AppFramework\AuthPublicShareController;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\NotFoundResponse;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\Constants;
use OCP\Defaults;
@ -343,18 +343,13 @@ class ShareController extends AuthPublicShareController {
}
/**
* @NoSameSiteCookieRequired
*
* @param string $token
* @param string|null $files
* @param string $path
* @return void|Response
* @throws NotFoundException
* @deprecated 31.0.0 Users are encouraged to use the DAV endpoint
*/
#[PublicPage]
#[NoCSRFRequired]
public function downloadShare($token, $files = null, $path = '') {
#[NoSameSiteCookieRequired]
public function downloadShare(string $token, ?string $files = null, string $path = ''): NotFoundResponse|RedirectResponse|DataResponse {
\OC_User::setIncognitoMode(true);
$share = $this->shareManager->getShareByToken($token);

4
apps/theming/lib/Controller/ThemingController.php
View file

@ -17,6 +17,7 @@ use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
use OCP\AppFramework\Http\Attribute\BruteForceProtection;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
@ -377,8 +378,6 @@ class ThemingController extends Controller {
}
/**
* @NoSameSiteCookieRequired
*
* Get the CSS stylesheet for a theme
*
* @param string $themeId ID of the theme
@ -393,6 +392,7 @@ class ThemingController extends Controller {
#[NoCSRFRequired]
#[NoTwoFactorRequired]
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
#[NoSameSiteCookieRequired]
public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false) {
$themes = $this->themesService->getThemes();
if (!in_array($themeId, array_keys($themes))) {

7
core/Controller/AvatarController.php
View file

@ -13,6 +13,7 @@ use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\FileDisplayResponse;
@ -50,8 +51,6 @@ class AvatarController extends Controller {
}
/**
* @NoSameSiteCookieRequired
*
* Get the dark avatar
*
* @param string $userId ID of the user
@ -67,6 +66,7 @@ class AvatarController extends Controller {
#[PublicPage]
#[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}/dark')]
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
#[NoSameSiteCookieRequired]
public function getAvatarDark(string $userId, int $size, bool $guestFallback = false) {
if ($size <= 64) {
if ($size !== 64) {
@ -102,8 +102,6 @@ class AvatarController extends Controller {
/**
* @NoSameSiteCookieRequired
*
* Get the avatar
*
* @param string $userId ID of the user
@ -119,6 +117,7 @@ class AvatarController extends Controller {
#[PublicPage]
#[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}')]
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
#[NoSameSiteCookieRequired]
public function getAvatar(string $userId, int $size, bool $guestFallback = false) {
if ($size <= 64) {
if ($size !== 64) {

5
core/Controller/ClientFlowLoginController.php
View file

@ -17,6 +17,7 @@ use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
@ -171,13 +172,11 @@ class ClientFlowLoginController extends Controller {
return $response;
}
/**
* @NoSameSiteCookieRequired
*/
#[NoAdminRequired]
#[NoCSRFRequired]
#[UseSession]
#[FrontpageRoute(verb: 'GET', url: '/login/flow/grant')]
#[NoSameSiteCookieRequired]
public function grantPage(
string $stateToken = '',
string $clientIdentifier = '',

5
core/Controller/ClientFlowLoginV2Controller.php
View file

@ -18,6 +18,7 @@ use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
@ -142,14 +143,12 @@ class ClientFlowLoginV2Controller extends Controller {
);
}
/**
* @NoSameSiteCookieRequired
*/
#[NoAdminRequired]
#[NoCSRFRequired]
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
#[UseSession]
#[FrontpageRoute(verb: 'GET', url: '/login/v2/grant')]
#[NoSameSiteCookieRequired]
public function grantPage(?string $stateToken, int $direct = 0): StandaloneTemplateResponse {
if ($stateToken === null) {
return $this->stateTokenMissingResponse();

10
core/Controller/CssController.php
View file

@ -13,11 +13,11 @@ use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\NotFoundResponse;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Files\IAppData;
use OCP\Files\NotFoundException;
@ -41,21 +41,19 @@ class CssController extends Controller {
}
/**
* @NoSameSiteCookieRequired
*
* @param string $fileName css filename with extension
* @param string $appName css folder name
* @return FileDisplayResponse|NotFoundResponse
*/
#[PublicPage]
#[NoCSRFRequired]
#[FrontpageRoute(verb: 'GET', url: '/css/{appName}/{fileName}')]
public function getCss(string $fileName, string $appName): Response {
#[NoSameSiteCookieRequired]
public function getCss(string $fileName, string $appName): FileDisplayResponse|NotFoundResponse {
try {
$folder = $this->appData->getFolder($appName);
$gzip = false;
$file = $this->getFile($folder, $fileName, $gzip);
} catch (NotFoundException $e) {
} catch (NotFoundException) {
return new NotFoundResponse();
}

5
core/Controller/JsController.php
View file

@ -13,12 +13,12 @@ use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\NotFoundResponse;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Files\IAppData;
use OCP\Files\NotFoundException;
@ -42,8 +42,6 @@ class JsController extends Controller {
}
/**
* @NoSameSiteCookieRequired
*
* @param string $fileName js filename with extension
* @param string $appName js folder name
*/
@ -51,6 +49,7 @@ class JsController extends Controller {
#[NoCSRFRequired]
#[FrontpageRoute(verb: 'GET', url: '/js/{appName}/{fileName}')]
#[NoTwoFactorRequired]
#[NoSameSiteCookieRequired]
public function getJs(string $fileName, string $appName): FileDisplayResponse|NotFoundResponse {
try {
$folder = $this->appData->getFolder($appName);

1
lib/composer/composer/autoload_classmap.php
View file

@ -93,6 +93,7 @@ return array(
'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',
'OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php',
'OCP\\AppFramework\\Http\\Attribute\\NoSameSiteCookieRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php',
'OCP\\AppFramework\\Http\\Attribute\\NoTwoFactorRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php',
'OCP\\AppFramework\\Http\\Attribute\\OpenAPI' => $baseDir . '/lib/public/AppFramework/Http/Attribute/OpenAPI.php',
'OCP\\AppFramework\\Http\\Attribute\\PasswordConfirmationRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php',

1
lib/composer/composer/autoload_static.php
View file

@ -134,6 +134,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'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',
'OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php',
'OCP\\AppFramework\\Http\\Attribute\\NoSameSiteCookieRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php',
'OCP\\AppFramework\\Http\\Attribute\\NoTwoFactorRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php',
'OCP\\AppFramework\\Http\\Attribute\\OpenAPI' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/OpenAPI.php',
'OCP\\AppFramework\\Http\\Attribute\\PasswordConfirmationRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php',

2
lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php
View file

@ -12,7 +12,7 @@ namespace OC\AppFramework\Http\Attributes;
use Attribute;
#[Attribute]
#[Attribute(Attribute::TARGET_METHOD)]
class TwoFactorSetUpDoneRequired {
}

34
lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php
View file

@ -1,5 +1,7 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
@ -10,13 +12,17 @@ use OC\AppFramework\Http\Request;
use OC\AppFramework\Middleware\Security\Exceptions\LaxSameSiteCookieFailedException;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Middleware;
use Psr\Log\LoggerInterface;
use ReflectionMethod;
class SameSiteCookieMiddleware extends Middleware {
public function __construct(
private Request $request,
private ControllerMethodReflector $reflector,
private readonly Request $request,
private readonly ControllerMethodReflector $reflector,
private readonly LoggerInterface $logger,
) {
}
@ -29,7 +35,8 @@ class SameSiteCookieMiddleware extends Middleware {
return;
}
$noSSC = $this->reflector->hasAnnotation('NoSameSiteCookieRequired');
$reflectionMethod = new ReflectionMethod($controller, $methodName);
$noSSC = $this->hasAnnotationOrAttribute($reflectionMethod, 'NoSameSiteCookieRequired', NoSameSiteCookieRequired::class);
if ($noSSC) {
return;
}
@ -80,4 +87,25 @@ class SameSiteCookieMiddleware extends Middleware {
);
}
}
/**
* @template T
*
* @param ReflectionMethod $reflectionMethod
* @param ?string $annotationName
* @param class-string<T> $attributeClass
* @return boolean
*/
protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, ?string $annotationName, string $attributeClass): bool {
if (!empty($reflectionMethod->getAttributes($attributeClass))) {
return true;
}
if ($annotationName && $this->reflector->hasAnnotation($annotationName)) {
$this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead');
return true;
}
return false;
}
}

25
lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php Normal file
View file

@ -0,0 +1,25 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH
* SPDX-FileContributor: Carl Schwan
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCP\AppFramework\Http\Attribute;
use Attribute;
use OCP\AppFramework\Attribute\Consumable;
/**
* Attribute for controller methods that want to disable the same site cookies
* requirements.
*
* @since 33.0.0
*/
#[Attribute(Attribute::TARGET_METHOD)]
#[Consumable(since: '33.0.0')]
class NoSameSiteCookieRequired {
}

7
lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php
View file

@ -11,15 +11,20 @@ declare(strict_types=1);
namespace OCP\AppFramework\Http\Attribute;
use Attribute;
use OCP\AppFramework\Attribute\Consumable;
/**
* Attribute for controller methods that want to disable the two factor
* authentification requirements.
*
* A user can access the page before the two-factor challenge has been passed
* (use this wisely and only in two-factor auth apps, e.g. to allow setup during
* login).
*
* @since 33.0.0
*/
#[Attribute]
#[Attribute(Attribute::TARGET_METHOD)]
#[Consumable(since: '33.0.0')]
class NoTwoFactorRequired {
}

46
tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php
View file

@ -14,31 +14,45 @@ use OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Response;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;
class HasAnnotationController extends Controller {
#[NoSameSiteCookieRequired]
public function foo(): Response {
return new Response();
}
}
class NoAnnotationController extends Controller {
public function foo(): Response {
return new Response();
}
}
class SameSiteCookieMiddlewareTest extends TestCase {
/** @var SameSiteCookieMiddleware */
private $middleware;
/** @var Request|\PHPUnit\Framework\MockObject\MockObject */
private $request;
/** @var ControllerMethodReflector|\PHPUnit\Framework\MockObject\MockObject */
private $reflector;
private SameSiteCookieMiddleware $middleware;
private Request&MockObject $request;
private ControllerMethodReflector&MockObject $reflector;
private LoggerInterface&MockObject $logger;
protected function setUp(): void {
parent::setUp();
$this->request = $this->createMock(Request::class);
$this->reflector = $this->createMock(ControllerMethodReflector::class);
$this->middleware = new SameSiteCookieMiddleware($this->request, $this->reflector);
$this->logger = $this->createMock(LoggerInterface::class);
$this->middleware = new SameSiteCookieMiddleware($this->request, $this->reflector, $this->logger);
}
public function testBeforeControllerNoIndex(): void {
$this->request->method('getScriptName')
->willReturn('/ocs/v2.php');
$this->middleware->beforeController($this->createMock(Controller::class), 'foo');
$this->middleware->beforeController(new NoAnnotationController('foo', $this->request), 'foo');
$this->addToAssertionCount(1);
}
@ -50,7 +64,7 @@ class SameSiteCookieMiddlewareTest extends TestCase {
->with('NoSameSiteCookieRequired')
->willReturn(true);
$this->middleware->beforeController($this->createMock(Controller::class), 'foo');
$this->middleware->beforeController(new HasAnnotationController('foo', $this->request), 'foo');
$this->addToAssertionCount(1);
}
@ -65,7 +79,7 @@ class SameSiteCookieMiddlewareTest extends TestCase {
$this->request->method('passesLaxCookieCheck')
->willReturn(true);
$this->middleware->beforeController($this->createMock(Controller::class), 'foo');
$this->middleware->beforeController(new NoAnnotationController('foo', $this->request), 'foo');
$this->addToAssertionCount(1);
}
@ -82,14 +96,14 @@ class SameSiteCookieMiddlewareTest extends TestCase {
$this->request->method('passesLaxCookieCheck')
->willReturn(false);
$this->middleware->beforeController($this->createMock(Controller::class), 'foo');
$this->middleware->beforeController(new NoAnnotationController('foo', $this->request), 'foo');
}
public function testAfterExceptionNoLaxCookie(): void {
$ex = new SecurityException();
try {
$this->middleware->afterException($this->createMock(Controller::class), 'foo', $ex);
$this->middleware->afterException(new NoAnnotationController('foo', $this->request), 'foo', $ex);
$this->fail();
} catch (\Exception $e) {
$this->assertSame($ex, $e);
@ -103,14 +117,14 @@ class SameSiteCookieMiddlewareTest extends TestCase {
->willReturn('/myrequri');
$middleware = $this->getMockBuilder(SameSiteCookieMiddleware::class)
->setConstructorArgs([$this->request, $this->reflector])
->setConstructorArgs([$this->request, $this->reflector, $this->logger])
->onlyMethods(['setSameSiteCookie'])
->getMock();
$middleware->expects($this->once())
->method('setSameSiteCookie');
$resp = $middleware->afterException($this->createMock(Controller::class), 'foo', $ex);
$resp = $middleware->afterException(new NoAnnotationController('foo', $this->request), 'foo', $ex);
$this->assertSame(Http::STATUS_FOUND, $resp->getStatus());