diff options
author | Joas Schilling <213943+nickvergessen@users.noreply.github.com> | 2023-04-25 12:11:37 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-04-25 12:11:37 +0200 |
commit | 2d8a0e4d8748e98a58713d9bd015c7057ac77c12 (patch) | |
tree | d024c4f7b5cb669d5895e1f0bd999f942c0e69e1 | |
parent | 3a2a5360a4d7cae79a8f16f9d67445512526e693 (diff) | |
parent | 2bef32969f4f00cec4bfed1592b15e4372248458 (diff) |
Merge pull request #9367 from nextcloud/techdebt/noid/controller-annotations
techdebt(controllers): Migrate controllers to Attributes
-rw-r--r-- | lib/Controller/FilesIntegrationController.php | 10 | ||||
-rw-r--r-- | lib/Controller/PageController.php | 26 | ||||
-rw-r--r-- | lib/Controller/RecordingController.php | 11 | ||||
-rw-r--r-- | lib/Controller/RoomController.php | 35 | ||||
-rw-r--r-- | lib/Controller/SignalingController.php | 22 | ||||
-rw-r--r-- | lib/Middleware/InjectionMiddleware.php | 26 |
6 files changed, 70 insertions, 60 deletions
diff --git a/lib/Controller/FilesIntegrationController.php b/lib/Controller/FilesIntegrationController.php index 4606eaf5a..0b47254e0 100644 --- a/lib/Controller/FilesIntegrationController.php +++ b/lib/Controller/FilesIntegrationController.php @@ -31,6 +31,8 @@ use OCA\Talk\Room; use OCA\Talk\Service\RoomService; use OCA\Talk\TalkSession; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; +use OCP\AppFramework\Http\Attribute\UseSession; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSNotFoundException; @@ -148,8 +150,6 @@ class FilesIntegrationController extends OCSController { /** * @PublicPage - * @UseSession - * @BruteForceProtection(action=shareinfo) * * Returns the token of the room associated to the file id of the given * share token. @@ -173,12 +173,14 @@ class FilesIntegrationController extends OCSController { * Besides the token of the room this also returns the current user ID and * display name, if any; this is needed by the Talk sidebar to know the * actual current user, as the public share page uses the incognito mode and - * thus logged in users as seen as guests. + * thus logged-in users as seen as guests. * * @param string $shareToken * @return DataResponse the status code is "200 OK" if a room is returned, * or "404 Not found" if the given share token was invalid. */ + #[UseSession] + #[BruteForceProtection(action: 'shareinfo')] public function getRoomByShareToken(string $shareToken): DataResponse { if ($this->config->getAppValue('spreed', 'conversations_files', '1') !== '1' || $this->config->getAppValue('spreed', 'conversations_files_public_shares', '1') !== '1') { @@ -195,7 +197,7 @@ class FilesIntegrationController extends OCSController { } } catch (ShareNotFound $e) { $response = new DataResponse([], Http::STATUS_NOT_FOUND); - $response->throttle(['token' => $shareToken]); + $response->throttle(['token' => $shareToken, 'action' => 'shareinfo']); return $response; } diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 88ddddf61..3c5781587 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -40,6 +40,8 @@ use OCA\Viewer\Event\LoadViewer; use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; +use OCP\AppFramework\Http\Attribute\UseSession; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Http\RedirectResponse; @@ -126,13 +128,13 @@ class PageController extends Controller { /** * @PublicPage * @NoCSRFRequired - * @UseSession - * @BruteForceProtection(action=talkRoomToken) * * @param string $token * @return Response * @throws HintException */ + #[UseSession] + #[BruteForceProtection(action: 'talkRoomToken')] public function showCall(string $token): Response { // This is the entry point from the `/call/{token}` URL which is hardcoded in the server. return $this->index($token); @@ -141,14 +143,14 @@ class PageController extends Controller { /** * @PublicPage * @NoCSRFRequired - * @UseSession - * @BruteForceProtection(action=talkRoomPassword) * * @param string $token * @param string $password * @return Response * @throws HintException */ + #[UseSession] + #[BruteForceProtection(action: 'talkRoomPassword')] public function authenticatePassword(string $token, string $password = ''): Response { // This is the entry point from the `/call/{token}` URL which is hardcoded in the server. return $this->index($token, '', $password); @@ -177,8 +179,6 @@ class PageController extends Controller { /** * @PublicPage * @NoCSRFRequired - * @UseSession - * @BruteForceProtection(action=talkRoomToken) * * @param string $token * @param string $callUser @@ -186,6 +186,8 @@ class PageController extends Controller { * @return TemplateResponse|RedirectResponse * @throws HintException */ + #[BruteForceProtection(action: 'talkRoomToken')] + #[UseSession] public function index(string $token = '', string $callUser = '', string $password = ''): Response { $bruteForceToken = $token; $user = $this->userSession->getUser(); @@ -256,7 +258,7 @@ class PageController extends Controller { $response = new RedirectResponse($passwordVerification['url']); } - $response->throttle(['token' => $token]); + $response->throttle(['token' => $token, 'action' => 'talkRoomPassword']); return $response; } } @@ -300,7 +302,7 @@ class PageController extends Controller { $response->setContentSecurityPolicy($csp); if ($throttle) { // Logged-in user tried to access a chat they can not access - $response->throttle(['token' => $bruteForceToken]); + $response->throttle(['token' => $bruteForceToken, 'action' => 'talkRoomToken']); } return $response; } @@ -308,17 +310,17 @@ class PageController extends Controller { /** * @PublicPage * @NoCSRFRequired - * @BruteForceProtection(action=talkRoomToken) * * @param string $token * @return TemplateResponse|NotFoundResponse */ + #[BruteForceProtection(action: 'talkRoomToken')] public function recording(string $token): Response { try { $room = $this->manager->getRoomByToken($token); } catch (RoomNotFoundException $e) { $response = new NotFoundResponse(); - $response->throttle(['token' => $token]); + $response->throttle(['token' => $token, 'action' => 'talkRoomToken']); return $response; } @@ -375,7 +377,7 @@ class PageController extends Controller { $response = new RedirectResponse($this->url->linkToRoute('core.login.showLoginForm', [ 'redirect_url' => $redirectUrl, ])); - $response->throttle(['token' => $token]); + $response->throttle(['token' => $token, 'action' => 'talkRoomToken']); return $response; } @@ -399,7 +401,7 @@ class PageController extends Controller { } else { $response = new RedirectResponse($passwordVerification['url']); } - $response->throttle(['token' => $token]); + $response->throttle(['token' => $token, 'action' => 'talkRoomPassword']); return $response; } } diff --git a/lib/Controller/RecordingController.php b/lib/Controller/RecordingController.php index 702f9bbc3..f4ab1a6b3 100644 --- a/lib/Controller/RecordingController.php +++ b/lib/Controller/RecordingController.php @@ -36,6 +36,7 @@ use OCA\Talk\Service\ParticipantService; use OCA\Talk\Service\RecordingService; use OCA\Talk\Service\RoomService; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; use OCP\AppFramework\Http\DataResponse; use OCP\Http\Client\IClientService; use OCP\IRequest; @@ -132,10 +133,10 @@ class RecordingController extends AEnvironmentAwareController { * Backend API to update recording status by backends. * * @PublicPage - * @BruteForceProtection(action=talkRecordingSecret) * * @return DataResponse */ + #[BruteForceProtection(action: 'talkRecordingSecret')] public function backend(): DataResponse { $json = $this->getInputStream(); if (!$this->validateBackendRequest($json)) { @@ -146,7 +147,7 @@ class RecordingController extends AEnvironmentAwareController { 'message' => 'The request could not be authenticated.', ], ], Http::STATUS_FORBIDDEN); - $response->throttle(); + $response->throttle(['action' => 'talkRecordingSecret']); return $response; } @@ -292,10 +293,8 @@ class RecordingController extends AEnvironmentAwareController { /** * @PublicPage * @RequireRoom - * @BruteForceProtection(action=talkRecordingSecret) - * - * @return DataResponse */ + #[BruteForceProtection(action: 'talkRecordingSecret')] public function store(string $owner): DataResponse { $data = $this->room->getToken(); if (!$this->validateBackendRequest($data)) { @@ -306,7 +305,7 @@ class RecordingController extends AEnvironmentAwareController { 'message' => 'The request could not be authenticated.', ], ], Http::STATUS_UNAUTHORIZED); - $response->throttle(); + $response->throttle(['action' => 'talkRecordingSecret']); return $response; } diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 0656d038c..8d4f9d2a4 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -53,6 +53,7 @@ use OCA\Talk\TalkSession; use OCA\Talk\Webinary; use OCP\App\IAppManager; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; @@ -284,10 +285,10 @@ class RoomController extends AEnvironmentAwareController { * * @NoAdminRequired * @RequireLoggedInParticipant - * @BruteForceProtection(action=talkRoomToken) * * @return DataResponse */ + #[BruteForceProtection(action: 'talkRoomToken')] public function getBreakoutRooms(): DataResponse { try { $rooms = $this->breakoutRoomService->getBreakoutRooms($this->room, $this->participant); @@ -312,20 +313,19 @@ class RoomController extends AEnvironmentAwareController { /** * @PublicPage - * @BruteForceProtection(action=talkRoomToken) * * @param string $token * @return DataResponse */ + #[BruteForceProtection(action: 'talkRoomToken')] + #[BruteForceProtection(action: 'talkSipBridgeSecret')] public function getSingleRoom(string $token): DataResponse { try { $isSIPBridgeRequest = $this->validateSIPBridgeRequest($token); } catch (UnauthorizedException $e) { - $ip = $this->request->getRemoteAddress(); - $action = 'talkSipBridgeSecret'; - $this->throttler->sleepDelay($ip, $action); - $this->throttler->registerAttempt($action, $ip); - return new DataResponse([], Http::STATUS_UNAUTHORIZED); + $response = new DataResponse([], Http::STATUS_UNAUTHORIZED); + $response->throttle(['action' => 'talkSipBridgeSecret']); + return $response; } // The SIP bridge only needs room details (public, sip enabled, lobby state, etc) @@ -365,7 +365,7 @@ class RoomController extends AEnvironmentAwareController { return new DataResponse($this->formatRoom($room, $participant, $statuses, $isSIPBridgeRequest), Http::STATUS_OK, $this->getTalkHashHeader()); } catch (RoomNotFoundException $e) { $response = new DataResponse([], Http::STATUS_NOT_FOUND); - $response->throttle(['token' => $token]); + $response->throttle(['token' => $token, 'action' => 'talkRoomToken']); return $response; } } @@ -1222,13 +1222,14 @@ class RoomController extends AEnvironmentAwareController { /** * @PublicPage - * @BruteForceProtection(action=talkRoomPassword) * * @param string $token * @param string $password * @param bool $force * @return DataResponse */ + #[BruteForceProtection(action: 'talkRoomPassword')] + #[BruteForceProtection(action: 'talkRoomToken')] public function joinRoom(string $token, string $password = '', bool $force = true): DataResponse { $sessionId = $this->session->getSessionForRoom($token); try { @@ -1284,11 +1285,11 @@ class RoomController extends AEnvironmentAwareController { $this->throttler->resetDelay($this->request->getRemoteAddress(), 'talkRoomToken', ['token' => $token]); } catch (InvalidPasswordException $e) { $response = new DataResponse([], Http::STATUS_FORBIDDEN); - $response->throttle(['token' => $token]); + $response->throttle(['token' => $token, 'action' => 'talkRoomPassword']); return $response; } catch (UnauthorizedException $e) { $response = new DataResponse([], Http::STATUS_NOT_FOUND); - $response->throttle(['token' => $token]); + $response->throttle(['token' => $token, 'action' => 'talkRoomToken']); return $response; } @@ -1305,21 +1306,21 @@ class RoomController extends AEnvironmentAwareController { /** * @PublicPage * @RequireRoom - * @BruteForceProtection(action=talkSipBridgeSecret) * * @param string $pin * @return DataResponse */ + #[BruteForceProtection(action: 'talkSipBridgeSecret')] public function getParticipantByDialInPin(string $pin): DataResponse { try { if (!$this->validateSIPBridgeRequest($this->room->getToken())) { $response = new DataResponse([], Http::STATUS_UNAUTHORIZED); - $response->throttle(); + $response->throttle(['action' => 'talkSipBridgeSecret']); return $response; } } catch (UnauthorizedException $e) { $response = new DataResponse([], Http::STATUS_UNAUTHORIZED); - $response->throttle(); + $response->throttle(['action' => 'talkSipBridgeSecret']); return $response; } @@ -1335,20 +1336,20 @@ class RoomController extends AEnvironmentAwareController { /** * @PublicPage * @RequireRoom - * @BruteForceProtection(action=talkSipBridgeSecret) * * @return DataResponse */ + #[BruteForceProtection(action: 'talkSipBridgeSecret')] public function createGuestByDialIn(): DataResponse { try { if (!$this->validateSIPBridgeRequest($this->room->getToken())) { $response = new DataResponse([], Http::STATUS_UNAUTHORIZED); - $response->throttle(); + $response->throttle(['action' => 'talkSipBridgeSecret']); return $response; } } catch (UnauthorizedException $e) { $response = new DataResponse([], Http::STATUS_UNAUTHORIZED); - $response->throttle(); + $response->throttle(['action' => 'talkSipBridgeSecret']); return $response; } diff --git a/lib/Controller/SignalingController.php b/lib/Controller/SignalingController.php index 40a8acd8a..4fb627b85 100644 --- a/lib/Controller/SignalingController.php +++ b/lib/Controller/SignalingController.php @@ -40,6 +40,7 @@ use OCA\Talk\Service\SessionService; use OCA\Talk\Signaling\Messages; use OCA\Talk\TalkSession; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCSController; use OCP\AppFramework\Utility\ITimeFactory; @@ -60,7 +61,6 @@ class SignalingController extends OCSController { public const EVENT_BACKEND_SIGNALING_ROOMS = self::class . '::signalingBackendRoom'; - private IConfig $serverConfig; private Config $talkConfig; private \OCA\Talk\Signaling\Manager $signalingManager; private TalkSession $session; @@ -73,7 +73,6 @@ class SignalingController extends OCSController { private IEventDispatcher $dispatcher; private ITimeFactory $timeFactory; private IClientService $clientService; - private IThrottler $throttler; private LoggerInterface $logger; private ?string $userId; @@ -98,7 +97,6 @@ class SignalingController extends OCSController { ?string $UserId, ) { parent::__construct($appName, $request); - $this->serverConfig = $serverConfig; $this->talkConfig = $talkConfig; $this->signalingManager = $signalingManager; $this->session = $session; @@ -111,7 +109,6 @@ class SignalingController extends OCSController { $this->dispatcher = $dispatcher; $this->timeFactory = $timeFactory; $this->clientService = $clientService; - $this->throttler = $throttler; $this->logger = $logger; $this->userId = $UserId; } @@ -145,21 +142,20 @@ class SignalingController extends OCSController { /** * @PublicPage - * @BruteForceProtection(action=talkRoomToken) * * @param string $token * @return DataResponse */ + #[BruteForceProtection(action: 'talkRoomToken')] + #[BruteForceProtection(action: 'talkRecordingSecret')] public function getSettings(string $token = ''): DataResponse { $isRecordingRequest = false; if (!empty($this->request->getHeader('Talk-Recording-Random')) || !empty($this->request->getHeader('Talk-Recording-Checksum'))) { if (!$this->validateRecordingBackendRequest('')) { - $ip = $this->request->getRemoteAddress(); - $action = 'talkRecordingSecret'; - $this->throttler->sleepDelay($ip, $action); - $this->throttler->registerAttempt($action, $ip); - return new DataResponse([], Http::STATUS_UNAUTHORIZED); + $response = new DataResponse([], Http::STATUS_UNAUTHORIZED); + $response->throttle(['action' => 'talkRecordingSecret']); + return $response; } $isRecordingRequest = true; @@ -176,7 +172,7 @@ class SignalingController extends OCSController { } } catch (RoomNotFoundException $e) { $response = new DataResponse([], Http::STATUS_NOT_FOUND); - $response->throttle(['token' => $token]); + $response->throttle(['token' => $token, 'action' => 'talkRoomToken']); return $response; } @@ -526,10 +522,10 @@ class SignalingController extends OCSController { * https://nextcloud-spreed-signaling.readthedocs.io/en/latest/standalone-signaling-api-v1/#backend-requests * * @PublicPage - * @BruteForceProtection(action=talkSignalingSecret) * * @return DataResponse */ + #[BruteForceProtection(action: 'talkSignalingSecret')] public function backend(): DataResponse { $json = $this->getInputStream(); if (!$this->validateBackendRequest($json)) { @@ -540,7 +536,7 @@ class SignalingController extends OCSController { 'message' => 'The request could not be authenticated.', ], ]); - $response->throttle(); + $response->throttle(['action' => 'talkSignalingSecret']); return $response; } diff --git a/lib/Middleware/InjectionMiddleware.php b/lib/Middleware/InjectionMiddleware.php index 67e10d566..e697cc201 100644 --- a/lib/Middleware/InjectionMiddleware.php +++ b/lib/Middleware/InjectionMiddleware.php @@ -39,6 +39,7 @@ use OCA\Talk\TalkSession; use OCA\Talk\Webinary; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; use OCP\AppFramework\Http\RedirectToDefaultAppResponse; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; @@ -263,15 +264,24 @@ class InjectionMiddleware extends Middleware { if ($exception instanceof RoomNotFoundException || $exception instanceof ParticipantNotFoundException) { if ($controller instanceof OCSController) { - $isBruteForceProtected = $this->reflector->hasAnnotation('BruteForceProtection'); - if ($isBruteForceProtected) { - $ip = $this->request->getRemoteAddress(); - $action = 'talkRoomToken'; - $this->throttler->sleepDelay($ip, $action); - $this->throttler->registerAttempt($action, $ip, [ - 'token' => $this->request->getParam('token') ?? '', - ]); + $reflectionMethod = new \ReflectionMethod($controller, $methodName); + $attributes = $reflectionMethod->getAttributes(BruteForceProtection::class); + + if (!empty($attributes)) { + foreach ($attributes as $attribute) { + /** @var BruteForceProtection $protection */ + $protection = $attribute->newInstance(); + $action = $protection->getAction(); + + if ('talkRoomToken' === $action) { + $this->throttler->sleepDelay($this->request->getRemoteAddress(), $action); + $this->throttler->registerAttempt($action, $this->request->getRemoteAddress(), [ |