Merge pull request #599 from leepeuker/refactor-user-page-read-auth

Add middleware to verify valid auth for user page read access
leepeuker · Mar 3, 2024 · 1815eb8 · 1815eb8
2 parents a514ce2 + c2b3c10
commit 1815eb8
Show file tree

Hide file tree

Showing 25 changed files with 105 additions and 108 deletions.
diff --git a/settings/routes.php b/settings/routes.php
@@ -168,14 +168,14 @@ function addWebRoutes(RouterService $routerService, FastRoute\RouteCollector $ro
     ##############
     # User media #
     ##############
-    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/dashboard', [Web\DashboardController::class, 'render']);
-    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/history', [Web\HistoryController::class, 'renderHistory']);
-    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/watchlist', [Web\WatchlistController::class, 'renderWatchlist']);
-    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/movies', [Web\MoviesController::class, 'renderPage']);
-    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/actors', [Web\ActorsController::class, 'renderPage']);
-    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/directors', [Web\DirectorsController::class, 'renderPage']);
-    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/movies/{id:\d+}', [Web\Movie\MovieController::class, 'renderPage']);
-    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/persons/{id:\d+}', [Web\PersonController::class, 'renderPage']);
+    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/dashboard', [Web\DashboardController::class, 'render'], [Web\Middleware\IsAuthorizedToReadUserData::class]);
+    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/history', [Web\HistoryController::class, 'renderHistory'], [Web\Middleware\IsAuthorizedToReadUserData::class]);
+    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/watchlist', [Web\WatchlistController::class, 'renderWatchlist'], [Web\Middleware\IsAuthorizedToReadUserData::class]);
+    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/movies', [Web\MoviesController::class, 'renderPage'], [Web\Middleware\IsAuthorizedToReadUserData::class]);
+    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/actors', [Web\ActorsController::class, 'renderPage'], [Web\Middleware\IsAuthorizedToReadUserData::class]);
+    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/directors', [Web\DirectorsController::class, 'renderPage'], [Web\Middleware\IsAuthorizedToReadUserData::class]);
+    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/movies/{id:\d+}', [Web\Movie\MovieController::class, 'renderPage'], [Web\Middleware\IsAuthorizedToReadUserData::class]);
+    $routes->add('GET', '/users/{username:[a-zA-Z0-9]+}/persons/{id:\d+}', [Web\PersonController::class, 'renderPage'], [Web\Middleware\IsAuthorizedToReadUserData::class]);
     $routes->add('DELETE', '/users/{username:[a-zA-Z0-9]+}/movies/{id:\d+}/history', [
         Web\HistoryController::class,
         'deleteHistoryEntry'

diff --git a/src/Domain/User/Service/UserPageAuthorizationChecker.php b/src/Domain/User/Service/UserPageAuthorizationChecker.php
@@ -3,7 +3,6 @@
 namespace Movary\Domain\User\Service;
 
 use Movary\Domain\User\UserApi;
-use Movary\ValueObject\Http\Request;
 
 class UserPageAuthorizationChecker
 {
@@ -39,20 +38,4 @@ public function fetchAllVisibleUsernamesForCurrentVisitor() : array
 
         return $this->userApi->fetchAllInternVisibleUsernames();
     }
-
-    public function findUserIdIfCurrentVisitorIsAllowedToSeeUser(Request $request) : ?int
-    {
-        $requestUsername = (string)$request->getRouteParameters()['username'];
-
-        $requestedUser = $this->userApi->findUserByName($requestUsername);
-        if ($requestedUser === null) {
-            return null;
-        }
-
-        if ($this->authenticationService->isUserPageVisibleForWebRequest($requestedUser) === false) {
-            return null;
-        }
-
-        return $requestedUser->getId();
-    }
 }
diff --git a/src/HttpController/Web/ActorsController.php b/src/HttpController/Web/ActorsController.php
@@ -5,6 +5,7 @@
 use Movary\Domain\Movie\History\MovieHistoryApi;
 use Movary\Domain\User\Service\Authentication;
 use Movary\Domain\User\Service\UserPageAuthorizationChecker;
+use Movary\Domain\User\UserApi;
 use Movary\HttpController\Web\Mapper\PersonsRequestMapper;
 use Movary\Service\PaginationElementsCalculator;
 use Movary\ValueObject\Http\Request;
@@ -26,11 +27,6 @@ public function __construct(
 
     public function renderPage(Request $request) : Response
     {
-        $userId = $this->userPageAuthorizationChecker->findUserIdIfCurrentVisitorIsAllowedToSeeUser($request);
-        if ($userId === null) {
-            return Response::createNotFound();
-        }
-
         $requestData = $this->requestMapper->mapRenderPageRequest($request);
 
         $currentUserId = null;
@@ -41,7 +37,7 @@ public function renderPage(Request $request) : Response
         $personFilterUserId = $requestData->getSortBy() !== 'name' ? $currentUserId : null;
 
         $actors = $this->movieHistoryApi->fetchActors(
-            $userId,
+            $requestData->getUserId(),
             $requestData->getLimit(),
             $requestData->getPage(),
             $requestData->getSearchTerm(),
@@ -52,7 +48,7 @@ public function renderPage(Request $request) : Response
         );
 
         $actorsCount = $this->movieHistoryApi->fetchMostWatchedActorsCount(
-            $userId,
+            $requestData->getUserId(),
             $requestData->getSearchTerm(),
             $requestData->getGender(),
             $personFilterUserId,
@@ -70,7 +66,7 @@ public function renderPage(Request $request) : Response
                 'sortBy' => $requestData->getSortBy(),
                 'sortOrder' => (string)$requestData->getSortOrder(),
                 'filterGender' => (string)$requestData->getGender(),
-                'uniqueGenders' => $this->movieHistoryApi->fetchUniqueActorGenders($userId)
+                'uniqueGenders' => $this->movieHistoryApi->fetchUniqueActorGenders($requestData->getUserId())
             ]),
         );
     }

diff --git a/src/HttpController/Web/DashboardController.php b/src/HttpController/Web/DashboardController.php
@@ -31,10 +31,7 @@ public function __construct(
 
     public function render(Request $request) : Response
     {
-        $userId = $this->userPageAuthorizationChecker->findUserIdIfCurrentVisitorIsAllowedToSeeUser($request);
-        if ($userId === null) {
-            return Response::createForbiddenRedirect($request->getPath());
-        }
+        $userId = $this->userApi->fetchUserByName((string)$request->getRouteParameters()['username'])->getId();
 
         $currentUserId = null;
         if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {

diff --git a/src/HttpController/Web/DirectorsController.php b/src/HttpController/Web/DirectorsController.php
@@ -26,11 +26,6 @@ public function __construct(
 
     public function renderPage(Request $request) : Response
     {
-        $userId = $this->userPageAuthorizationChecker->findUserIdIfCurrentVisitorIsAllowedToSeeUser($request);
-        if ($userId === null) {
-            return Response::createNotFound();
-        }
-
         $requestData = $this->requestMapper->mapRenderPageRequest($request);
 
         $currentUserId = null;
@@ -41,7 +36,7 @@ public function renderPage(Request $request) : Response
         $personFilterUserId = $requestData->getSortBy() !== 'name' ? $currentUserId : null;
 
         $directors = $this->movieHistoryApi->fetchDirectors(
-            $userId,
+            $requestData->getUserId(),
             $requestData->getLimit(),
             $requestData->getPage(),
             $requestData->getSearchTerm(),
@@ -52,7 +47,7 @@ public function renderPage(Request $request) : Response
         );
 
         $directorsCount = $this->movieHistoryApi->fetchDirectorsCount(
-            $userId,
+            $requestData->getUserId(),
             $requestData->getSearchTerm(),
             $requestData->getGender(),
             personFilterUserId: $personFilterUserId,
@@ -70,7 +65,7 @@ public function renderPage(Request $request) : Response
                 'sortBy' => $requestData->getSortBy(),
                 'sortOrder' => (string)$requestData->getSortOrder(),
                 'filterGender' => (string)$requestData->getGender(),
-                'uniqueGenders' => $this->movieHistoryApi->fetchUniqueDirectorsGenders($userId)
+                'uniqueGenders' => $this->movieHistoryApi->fetchUniqueDirectorsGenders($requestData->getUserId())
             ]),
         );
     }

diff --git a/src/HttpController/Web/Dto/MoviesRequestDto.php b/src/HttpController/Web/Dto/MoviesRequestDto.php
@@ -8,7 +8,7 @@
 class MoviesRequestDto
 {
     private function __construct(
-        private readonly ?int $userId,
+        private readonly int $userId,
         private readonly ?string $searchTerm,
         private readonly int $page,
         private readonly int $limit,
@@ -24,7 +24,7 @@ private function __construct(
     }
 
     public static function createFromParameters(
-        ?int $userId,
+        int $userId,
         ?string $searchTerm,
         int $page,
         int $limit,
@@ -93,7 +93,7 @@ public function getSortOrder() : SortOrder
         return $this->sortOrder;
     }
 
-    public function getUserId() : ?int
+    public function getUserId() : int
     {
         return $this->userId;
     }

diff --git a/src/HttpController/Web/Dto/PersonsRequestDto.php b/src/HttpController/Web/Dto/PersonsRequestDto.php
@@ -8,7 +8,7 @@
 class PersonsRequestDto
 {
     private function __construct(
-        private readonly ?int $userId,
+        private readonly int $userId,
         private readonly ?string $searchTerm,
         private readonly int $page,
         private readonly int $limit,
@@ -19,7 +19,7 @@ private function __construct(
     }
 
     public static function createFromParameters(
-        ?int $userId,
+        int $userId,
         ?string $searchTerm,
         int $page,
         int $limit,
@@ -68,7 +68,7 @@ public function getSortOrder() : SortOrder
         return $this->sortOrder;
     }
 
-    public function getUserId() : ?int
+    public function getUserId() : int
     {
         return $this->userId;
     }

diff --git a/src/HttpController/Web/HistoryController.php b/src/HttpController/Web/HistoryController.php
@@ -118,11 +118,7 @@ public function logMovie(Request $request) : Response
 
     public function renderHistory(Request $request) : Response
     {
-        $userId = $this->userPageAuthorizationChecker->findUserIdIfCurrentVisitorIsAllowedToSeeUser($request);
-        if ($userId === null) {
-            return Response::createNotFound();
-        }
-
+        $userId = $this->userApi->fetchUserByName((string)$request->getRouteParameters()['username'])->getId();
         $searchTerm = $request->getGetParameters()['s'] ?? null;
         $page = $request->getGetParameters()['p'] ?? 1;
         $limit = self::DEFAULT_LIMIT;

diff --git a/src/HttpController/Web/Mapper/MoviesRequestMapper.php b/src/HttpController/Web/Mapper/MoviesRequestMapper.php
@@ -2,7 +2,7 @@
 
 namespace Movary\HttpController\Web\Mapper;
 
-use Movary\Domain\User\Service\UserPageAuthorizationChecker;
+use Movary\Domain\User\UserApi;
 use Movary\HttpController\Web\Dto\MoviesRequestDto;
 use Movary\ValueObject\Http\Request;
 use Movary\ValueObject\SortOrder;
@@ -26,14 +26,14 @@ class MoviesRequestMapper
     private const DEFAULT_SORT_BY = 'title';
 
     public function __construct(
-        private readonly UserPageAuthorizationChecker $userPageAuthorizationChecker,
+        private readonly UserApi $userApi,
     ) {
     }
 
     // phpcs:ignore Generic.Metrics.CyclomaticComplexity.TooHigh
     public function mapRenderPageRequest(Request $request) : MoviesRequestDto
     {
-        $userId = $this->userPageAuthorizationChecker->findUserIdIfCurrentVisitorIsAllowedToSeeUser($request);
+        $userId = $this->userApi->fetchUserByName((string)$request->getRouteParameters()['username'])->getId();
 
         $getParameters = $request->getGetParameters();
 

diff --git a/src/HttpController/Web/Mapper/PersonsRequestMapper.php b/src/HttpController/Web/Mapper/PersonsRequestMapper.php
@@ -2,7 +2,7 @@
 
 namespace Movary\HttpController\Web\Mapper;
 
-use Movary\Domain\User\Service\UserPageAuthorizationChecker;
+use Movary\Domain\User\UserApi;
 use Movary\HttpController\Web\Dto\PersonsRequestDto;
 use Movary\ValueObject\Gender;
 use Movary\ValueObject\Http\Request;
@@ -16,13 +16,13 @@ class PersonsRequestMapper
     private const DEFAULT_SORT_BY = 'uniqueAppearances';
 
     public function __construct(
-        private readonly UserPageAuthorizationChecker $userPageAuthorizationChecker,
+        private readonly UserApi $userApi,
     ) {
     }
 
     public function mapRenderPageRequest(Request $request) : PersonsRequestDto
     {
-        $userId = $this->userPageAuthorizationChecker->findUserIdIfCurrentVisitorIsAllowedToSeeUser($request);
+        $userId = $this->userApi->fetchUserByName((string)$request->getRouteParameters()['username'])->getId();
 
         $getParameters = $request->getGetParameters();
 

diff --git a/src/HttpController/Web/Mapper/WatchlistRequestMapper.php b/src/HttpController/Web/Mapper/WatchlistRequestMapper.php
@@ -2,7 +2,7 @@
 
 namespace Movary\HttpController\Web\Mapper;
 
-use Movary\Domain\User\Service\UserPageAuthorizationChecker;
+use Movary\Domain\User\UserApi;
 use Movary\HttpController\Web\Dto\MoviesRequestDto;
 use Movary\ValueObject\Http\Request;
 use Movary\ValueObject\SortOrder;
@@ -22,13 +22,13 @@ class WatchlistRequestMapper
     private const DEFAULT_SORT_BY = 'addedAt';
 
     public function __construct(
-        private readonly UserPageAuthorizationChecker $userPageAuthorizationChecker,
+        private readonly UserApi $userApi,
     ) {
     }
 
     public function mapRenderPageRequest(Request $request) : MoviesRequestDto
     {
-        $userId = $this->userPageAuthorizationChecker->findUserIdIfCurrentVisitorIsAllowedToSeeUser($request);
+        $userId = $this->userApi->fetchUserByName((string)$request->getRouteParameters()['username'])->getId();
 
         $getParameters = $request->getGetParameters();
 

diff --git a/src/HttpController/Web/Middleware/IsAuthorizedToReadUserData.php b/src/HttpController/Web/Middleware/IsAuthorizedToReadUserData.php
@@ -0,0 +1,33 @@
+<?php declare(strict_types=1);
+
+namespace Movary\HttpController\Web\Middleware;
+
+use Movary\Domain\User\Service\Authentication;
+use Movary\Domain\User\UserApi;
+use Movary\ValueObject\Http\Request;
+use Movary\ValueObject\Http\Response;
+
+class IsAuthorizedToReadUserData implements MiddlewareInterface
+{
+    public function __construct(
+        private readonly UserApi $userApi,
+        private readonly Authentication $authenticationService,
+    ) {
+    }
+
+    public function __invoke(Request $request) : ?Response
+    {
+        $requestedUsername = (string)$request->getRouteParameters()['username'];
+
+        $requestedUser = $this->userApi->findUserByName($requestedUsername);
+        if ($requestedUser === null) {
+            return Response::createNotFound();
+        }
+
+        if ($this->authenticationService->isUserPageVisibleForWebRequest($requestedUser) === false) {
+            return Response::createForbiddenRedirect($request->getPath());
+        }
+
+        return null;
+    }
+}
diff --git a/src/HttpController/Web/Middleware/MiddlewareInterface.php b/src/HttpController/Web/Middleware/MiddlewareInterface.php
@@ -2,9 +2,10 @@
 
 namespace Movary\HttpController\Web\Middleware;
 
+use Movary\ValueObject\Http\Request;
 use Movary\ValueObject\Http\Response;
 
 interface MiddlewareInterface
 {
-    public function __invoke() : ?Response;
+    public function __invoke(Request $request) : ?Response;
 }
diff --git a/src/HttpController/Web/Middleware/ServerHasNoUsers.php b/src/HttpController/Web/Middleware/ServerHasNoUsers.php
@@ -3,6 +3,7 @@
 namespace Movary\HttpController\Web\Middleware;
 
 use Movary\Domain\User\UserApi;
+use Movary\ValueObject\Http\Request;
 use Movary\ValueObject\Http\Response;
 
 class ServerHasNoUsers implements MiddlewareInterface
@@ -12,7 +13,8 @@ public function __construct(
     ) {
     }
 
-    public function __invoke() : ?Response
+    // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter
+    public function __invoke(Request $request) : ?Response
     {
         if ($this->userApi->hasUsers() === true) {
             return null;

diff --git a/src/HttpController/Web/Middleware/ServerHasRegistrationEnabled.php b/src/HttpController/Web/Middleware/ServerHasRegistrationEnabled.php
@@ -2,6 +2,7 @@
 
 namespace Movary\HttpController\Web\Middleware;
 
+use Movary\ValueObject\Http\Request;
 use Movary\ValueObject\Http\Response;
 
 class ServerHasRegistrationEnabled implements MiddlewareInterface
@@ -11,7 +12,8 @@ public function __construct(
     ) {
     }
 
-    public function __invoke() : ?Response
+    // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter
+    public function __invoke(Request $request) : ?Response
     {
         if ($this->registrationEnabled === false) {
             return null;

diff --git a/src/HttpController/Web/Middleware/ServerHasUsers.php b/src/HttpController/Web/Middleware/ServerHasUsers.php
@@ -3,6 +3,7 @@
 namespace Movary\HttpController\Web\Middleware;
 
 use Movary\Domain\User\UserApi;
+use Movary\ValueObject\Http\Request;
 use Movary\ValueObject\Http\Response;
 
 class ServerHasUsers implements MiddlewareInterface
@@ -12,7 +13,8 @@ public function __construct(
     ) {
     }
 
-    public function __invoke() : ?Response
+    // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter
+    public function __invoke(Request $request) : ?Response
     {
         if ($this->userApi->hasUsers() === false) {
             return null;