Remove web logout endpoint to only use api logout

leepeuker · Feb 25, 2024 · fbebf02 · fbebf02
1 parent 884ba28
commit fbebf02
Show file tree

Hide file tree

Showing 20 changed files with 55 additions and 51 deletions.
diff --git a/public/js/app.js b/public/js/app.js
@@ -498,3 +498,11 @@ function addAlert(parentDivId, message, color, withCloseButton = true, marginBot
 function removeAlert(parentDivId) {
     document.getElementById(parentDivId).innerHTML = ''
 }
+
+async function logout() {
+    await fetch('/api/authentication/token', {
+        method: 'DELETE',
+    });
+
+    window.location.href = '/'
+}
diff --git a/public/js/login.js b/public/js/login.js
@@ -11,6 +11,11 @@ async function submitCredentials() {
         return;
     }
 
+    const forbiddenPageAlert = document.getElementById('forbiddenPageAlert');
+    if (forbiddenPageAlert) {
+        forbiddenPageAlert.classList.add('d-none');
+    }
+
     if (response.status === 400) {
         const error = await response.json();
 

diff --git a/settings/routes.php b/settings/routes.php
@@ -19,8 +19,6 @@ function addWebRoutes(RouterService $routerService, FastRoute\RouteCollector $ro
 
     $routes->add('GET', '/', [Web\LandingPageController::class, 'render'], [Web\Middleware\UserIsUnauthenticated::class, Web\Middleware\ServerHasNoUsers::class]);
     $routes->add('GET', '/login', [Web\AuthenticationController::class, 'renderLoginPage'], [Web\Middleware\UserIsUnauthenticated::class]);
-    $routes->add('POST', '/login', [Web\AuthenticationController::class, 'login']);
-    $routes->add('GET', '/logout', [Web\AuthenticationController::class, 'logout']);
     $routes->add('POST', '/create-user', [Web\CreateUserController::class, 'createUser'], [
         Web\Middleware\UserIsUnauthenticated::class,
         Web\Middleware\ServerHasUsers::class,
@@ -203,7 +201,7 @@ function addApiRoutes(RouterService $routerService, FastRoute\RouteCollector $ro
 
     $routes->add('GET', '/openapi', [Api\OpenApiController::class, 'getSchema']);
     $routes->add('POST', '/authentication/token', [Api\AuthenticationController::class, 'createToken']);
-    $routes->add('DELETE', '/authentication/token', [Api\AuthenticationController::class, 'destroyToken'], [Api\Middleware\IsAuthenticated::class]);
+    $routes->add('DELETE', '/authentication/token', [Api\AuthenticationController::class, 'destroyToken']);
 
     $routeUserHistory = '/users/{username:[a-zA-Z0-9]+}/history/movies';
     $routes->add('GET', $routeUserHistory, [Api\HistoryController::class, 'getHistory'], [Api\Middleware\IsAuthorizedToReadUserData::class]);

diff --git a/src/Domain/User/Service/Authentication.php b/src/Domain/User/Service/Authentication.php
@@ -113,7 +113,7 @@ public function getUserIdByApiToken(Request $request) : ?int
         return $this->userApi->findByToken($apiToken)?->getId();
     }
 
-    public function isUserAuthenticated() : bool
+    public function isUserAuthenticatedWithCookie() : bool
     {
         $token = filter_input(INPUT_COOKIE, self::AUTHENTICATION_COOKIE_NAME);
 
@@ -152,11 +152,11 @@ public function isUserPageVisibleForCurrentUser(int $privacyLevel, int $userId)
             return true;
         }
 
-        if ($privacyLevel === 1 && $this->isUserAuthenticated() === true) {
+        if ($privacyLevel === 1 && $this->isUserAuthenticatedWithCookie() === true) {
             return true;
         }
 
-        return $this->isUserAuthenticated() === true && $this->getCurrentUserId() === $userId;
+        return $this->isUserAuthenticatedWithCookie() === true && $this->getCurrentUserId() === $userId;
     }
 
     public function isValidToken(string $token) : bool

diff --git a/src/Domain/User/Service/UserPageAuthorizationChecker.php b/src/Domain/User/Service/UserPageAuthorizationChecker.php
@@ -15,7 +15,7 @@ public function __construct(
 
     public function fetchAllHavingWatchedMovieVisibleUsernamesForCurrentVisitor(int $movieId) : array
     {
-        if ($this->authenticationService->isUserAuthenticated() === false) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === false) {
             return $this->userApi->fetchAllHavingWatchedMoviePublicVisibleUsernames($movieId);
         }
 
@@ -24,7 +24,7 @@ public function fetchAllHavingWatchedMovieVisibleUsernamesForCurrentVisitor(int
 
     public function fetchAllHavingWatchedMovieWithPersonVisibleUsernamesForCurrentVisitor(int $personId) : array
     {
-        if ($this->authenticationService->isUserAuthenticated() === false) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === false) {
             return $this->userApi->fetchAllHavingWatchedMovieWithPersonPublicVisibleUsernames($personId);
         }
 
@@ -33,7 +33,7 @@ public function fetchAllHavingWatchedMovieWithPersonVisibleUsernamesForCurrentVi
 
     public function fetchAllVisibleUsernamesForCurrentVisitor() : array
     {
-        if ($this->authenticationService->isUserAuthenticated() === false) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === false) {
             return $this->userApi->fetchAllPublicVisibleUsernames();
         }
 

diff --git a/src/Factory.php b/src/Factory.php
@@ -279,7 +279,7 @@ public static function createTwigEnvironment(ContainerInterface $container) : Tw
         $currentRequest = $container->get(Request::class);
         $routeUsername = $currentRequest->getRouteParameters()['username'] ?? null;
 
-        $userAuthenticated = $container->get(Authentication::class)->isUserAuthenticated();
+        $userAuthenticated = $container->get(Authentication::class)->isUserAuthenticatedWithCookie();
 
         $twig->addGlobal('loggedIn', $userAuthenticated);
 

diff --git a/src/HttpController/Api/AuthenticationController.php b/src/HttpController/Api/AuthenticationController.php
@@ -16,7 +16,6 @@ class AuthenticationController
 {
     public function __construct(
         private readonly Authentication $authenticationService,
-        private readonly UserApi $userApi
     ) {
     }
 
@@ -94,16 +93,25 @@ public function createToken(Request $request) : Response
 
     public function destroyToken(Request $request) : Response
     {
-        if($this->authenticationService->isUserAuthenticated() === true) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
             $this->authenticationService->logout();
-        } 
-        else {
-            $apiToken = $this->authenticationService->getUserIdByApiToken($request);
-            if($apiToken === null) {
-                return Response::createForbidden();
-            }
-            $this->userApi->deleteApiToken($apiToken);
+
+            return Response::CreateNoContent();
+        }
+
+        $apiToken = $request->getHeaders()['X-Auth-Token'] ?? null;
+        if ($apiToken === null) {
+            return Response::createBadRequest(
+                Json::encode([
+                    'error' => 'MissingAuthToken',
+                    'message' => 'Authentication token to delete in headers missing'
+                ]),
+                [Header::createContentTypeJson()],
+            );
         }
+
+        $this->authenticationService->deleteToken($apiToken);
+
         return Response::CreateNoContent();
     }
 }
diff --git a/src/HttpController/Web/ActorsController.php b/src/HttpController/Web/ActorsController.php
@@ -34,7 +34,7 @@ public function renderPage(Request $request) : Response
         $requestData = $this->requestMapper->mapRenderPageRequest($request);
 
         $currentUserId = null;
-        if ($this->authenticationService->isUserAuthenticated() === true) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
             $currentUserId = $this->authenticationService->getCurrentUserId();
         }
 

diff --git a/src/HttpController/Web/AuthenticationController.php b/src/HttpController/Web/AuthenticationController.php
@@ -3,12 +3,7 @@
 namespace Movary\HttpController\Web;
 
 use Movary\Domain\SessionService;
-use Movary\Domain\User\Exception\InvalidCredentials;
-use Movary\Domain\User\Exception\InvalidTotpCode;
-use Movary\Domain\User\Exception\MissingTotpCode;
-use Movary\Domain\User\Service;
 use Movary\Util\SessionWrapper;
-use Movary\ValueObject\Http\Header;
 use Movary\ValueObject\Http\Request;
 use Movary\ValueObject\Http\Response;
 use Movary\ValueObject\Http\StatusCode;
@@ -18,22 +13,10 @@ class AuthenticationController
 {
     public function __construct(
         private readonly Environment $twig,
-        private readonly Service\Authentication $authenticationService,
         private readonly SessionWrapper $sessionWrapper,
     ) {
     }
 
-    public function logout() : Response
-    {
-        $this->authenticationService->logout();
-
-        return Response::create(
-            StatusCode::createSeeOther(),
-            null,
-            [Header::createLocation('/')],
-        );
-    }
-
     public function renderLoginPage(Request $request) : Response
     {
         $failedLogin = $this->sessionWrapper->has('failedLogin');

diff --git a/src/HttpController/Web/DashboardController.php b/src/HttpController/Web/DashboardController.php
@@ -37,7 +37,7 @@ public function render(Request $request) : Response
         }
 
         $currentUserId = null;
-        if ($this->authenticationService->isUserAuthenticated() === true) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
             $currentUserId = $this->authenticationService->getCurrentUserId();
         }
 

diff --git a/src/HttpController/Web/DirectorsController.php b/src/HttpController/Web/DirectorsController.php
@@ -34,7 +34,7 @@ public function renderPage(Request $request) : Response
         $requestData = $this->requestMapper->mapRenderPageRequest($request);
 
         $currentUserId = null;
-        if ($this->authenticationService->isUserAuthenticated() === true) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
             $currentUserId = $this->authenticationService->getCurrentUserId();
         }
 

diff --git a/src/HttpController/Web/Middleware/UserIsAuthenticated.php b/src/HttpController/Web/Middleware/UserIsAuthenticated.php
@@ -14,7 +14,7 @@ public function __construct(
 
     public function __invoke() : ?Response
     {
-        if ($this->authenticationService->isUserAuthenticated() === true) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
             return null;
         }
 

diff --git a/src/HttpController/Web/Middleware/UserIsUnauthenticated.php b/src/HttpController/Web/Middleware/UserIsUnauthenticated.php
@@ -14,7 +14,7 @@ public function __construct(
 
     public function __invoke() : ?Response
     {
-        if ($this->authenticationService->isUserAuthenticated() === false) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === false) {
             return null;
         }
 

diff --git a/src/HttpController/Web/Movie/MovieController.php b/src/HttpController/Web/Movie/MovieController.php
@@ -69,7 +69,7 @@ public function renderPage(Request $request) : Response
         }
 
         $currentUser = null;
-        if ($this->authenticationService->isUserAuthenticated() === true) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
             $currentUser = $this->authenticationService->getCurrentUser();
         }
 

diff --git a/src/HttpController/Web/PersonController.php b/src/HttpController/Web/PersonController.php
@@ -98,7 +98,7 @@ public function renderPage(Request $request) : Response
         }
 
         $isHiddenInTopLists = false;
-        if ($this->authenticationService->isUserAuthenticated() === true) {
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === true) {
             $userId = $this->authenticationService->getCurrentUserId();
             $isHiddenInTopLists = $this->userApi->hasHiddenPerson($userId, $personId);
         }

diff --git a/src/HttpController/Web/UserController.php b/src/HttpController/Web/UserController.php
@@ -22,7 +22,7 @@ public function __construct(
 
     public function createUser(Request $request) : Response
     {
-        if ($this->authenticationService->isUserAuthenticated() === false
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === false
             && $this->authenticationService->getCurrentUser()->isAdmin() === false) {
             return Response::createForbidden();
         }
@@ -65,7 +65,7 @@ public function deleteUser(Request $request) : Response
 
     public function fetchUsers() : Response
     {
-        if ($this->authenticationService->isUserAuthenticated() === false
+        if ($this->authenticationService->isUserAuthenticatedWithCookie() === false
             && $this->authenticationService->getCurrentUser()->isAdmin() === false) {
             return Response::createForbidden();
         }

diff --git a/templates/component/navbar.html.twig b/templates/component/navbar.html.twig
@@ -48,7 +48,7 @@
                 </li>
                 {% if loggedIn == true %}
                     <li><a class="dropdown-item {% if requestUrlPath starts with '/settings/' %}active{% endif %}" href="/settings/account/general">Settings</a></li>
-                    <li><a class="dropdown-item" href="/logout">Logout</a></li>
+                    <li><button class="dropdown-item" onclick="logout()" style="cursor: pointer">Logout</button></li>
                 {% else %}
                     <li><a class="dropdown-item" href="/login">Login</a></li>
                 {% endif %}

diff --git a/templates/page/login.html.twig b/templates/page/login.html.twig
@@ -44,7 +44,7 @@
                     </div>
                 {% endif %}
                 {% if redirect != false %}
-                    <div class="alert alert-danger" role="alert" style="margin-bottom: 0.7rem!important;">
+                    <div class="alert alert-danger" role="alert" id="forbiddenPageAlert">
                         Sign in to access page
                     </div>
                     <input type="hidden" value="{{ redirect }}" name="redirect" />

diff --git a/tests/rest/api/authentication.http b/tests/rest/api/authentication.http
@@ -4,13 +4,13 @@ Cache-Control: no-cache
 Content-Type: application/json
 X-Movary-Client: RestAPI Test
 
-{"email" : "{{username}}", "password" : "{{password}}", "rememberMe" : 1, "totpCode" : 123456}
+{"email" : "{{email}}", "password" : "{{password}}", "rememberMe" : 1, "totpCode" : 123456}
 
 ###
 
-GET http://127.0.0.1/api/authentication/destroy-token
+DELETE http://127.0.0.1/api/authentication/token
 Accept: */*
 Cache-Control: no-cache
 Content-Type: application/json
 X-Movary-Client: RestAPI Test
-X-Auth-Token: {{xAuthToken}}
+X-Auth-Token: {{xAuthToken}}
diff --git a/tests/rest/api/http-client.env.json b/tests/rest/api/http-client.env.json
@@ -1,6 +1,8 @@
 {
   "testUser": {
     "username": "testUser",
-    "xAuthToken": "4f0fbe93e2752932e5700e14ffa49f67"
+    "xAuthToken": "4f0fbe93e2752932e5700e14ffa49f67",
+    "email": "[email protected]",
+    "password": "password1234"
   }
 }