From dd2f7dea4196fb2547e2a2e8aa9b19dd8297db49 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Fri, 27 Jun 2025 12:05:26 +0200
Subject: prevent too many redirects and fail safe redirection after login, re
 #5593

Merge request studip/studip!4303
---
 lib/functions.php                               |  2 +-
 lib/middleware/AuthenticationMiddleware.php     |  8 ++++----
 lib/middleware/HandleAccessDeniedMiddleware.php | 11 ++++++-----
 lib/middleware/SeminarOpenMiddleware.php        | 11 ++++++-----
 lib/middleware/SessionMiddleware.php            |  5 +++--
 5 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/lib/functions.php b/lib/functions.php
index a567374..cd52968 100644
--- a/lib/functions.php
+++ b/lib/functions.php
@@ -1123,7 +1123,7 @@ function studip_default_exception_handler($exception) {
         $status = 403;
         $template = 'check_object_exception';
     } elseif ($exception instanceof LoginException) {
-        $_SESSION['redirect_after_login'] = Request::url();
+        $_SESSION['redirect_after_login'] ??= Request::url();
         sess()->save();
         header('Location: ' . URLHelper::getScriptURL('dispatch.php/login'));
         exit;
diff --git a/lib/middleware/AuthenticationMiddleware.php b/lib/middleware/AuthenticationMiddleware.php
index 5792e15..eb6ddd6 100644
--- a/lib/middleware/AuthenticationMiddleware.php
+++ b/lib/middleware/AuthenticationMiddleware.php
@@ -32,7 +32,7 @@ final class AuthenticationMiddleware implements MiddlewareInterface
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         if ($this->auth_manager->start()) {
-            if (isset($_SESSION['redirect_after_login'] )) {
+            if (isset($_SESSION['redirect_after_login'] ) && \User::findCurrent()) {
                 $redirect = $_SESSION['redirect_after_login'];
                 unset($_SESSION['redirect_after_login']);
 
@@ -43,12 +43,12 @@ final class AuthenticationMiddleware implements MiddlewareInterface
             return $handler->handle($request);
         } else {
             if (!match_route('dispatch.php/start')) {
-                $_SESSION['redirect_after_login'] = \Request::url();
+                $_SESSION['redirect_after_login'] ??= \Request::url();
             } else {
                 unset($_SESSION['redirect_after_login']);
             }
-            $response = $this->response_factory->createResponse(302);
-            return $response->withHeader('Location', \URLHelper::getURL('dispatch.php/login'));
+            return $this->response_factory->createResponse(302)
+                ->withHeader('Location', \URLHelper::getURL('dispatch.php/login'));
         }
     }
 }
diff --git a/lib/middleware/HandleAccessDeniedMiddleware.php b/lib/middleware/HandleAccessDeniedMiddleware.php
index d27e4b7..567eca8 100644
--- a/lib/middleware/HandleAccessDeniedMiddleware.php
+++ b/lib/middleware/HandleAccessDeniedMiddleware.php
@@ -12,8 +12,9 @@ use URLHelper;
 
 final class HandleAccessDeniedMiddleware implements MiddlewareInterface
 {
-    public function __construct(private ResponseFactoryInterface $responseFactory)
-    {
+    public function __construct(
+        private readonly ResponseFactoryInterface $responseFactory
+    ) {
     }
 
     /**
@@ -25,9 +26,9 @@ final class HandleAccessDeniedMiddleware implements MiddlewareInterface
         try {
             return $handler->handle($request);
         } catch (AccessDeniedException $ade) {
-            $_SESSION['redirect_after_login'] = Request::url();
-            $response = $this->responseFactory->createResponse(302);
-            return $response->withHeader('Location', URLHelper::getURL('dispatch.php/login'));
+            $_SESSION['redirect_after_login'] ??= Request::url();
+            return $this->responseFactory->createResponse(302)
+                ->withHeader('Location', URLHelper::getURL('dispatch.php/login'));
         }
     }
 }
diff --git a/lib/middleware/SeminarOpenMiddleware.php b/lib/middleware/SeminarOpenMiddleware.php
index 2d388e4..a20a3ba 100644
--- a/lib/middleware/SeminarOpenMiddleware.php
+++ b/lib/middleware/SeminarOpenMiddleware.php
@@ -25,8 +25,9 @@ use Psr\Http\Message\ResponseFactoryInterface;
 final class SeminarOpenMiddleware implements MiddlewareInterface
 {
 
-    public function __construct(private ResponseFactoryInterface $response_factory)
-    {
+    public function __construct(
+        private readonly ResponseFactoryInterface $response_factory
+    ) {
     }
 
     /**
@@ -159,9 +160,9 @@ final class SeminarOpenMiddleware implements MiddlewareInterface
             try {
                 \Context::set($course_id);
             } catch (\LoginException $e) {
-                $response = $this->response_factory->createResponse(302);
-                $_SESSION['redirect_after_login'] = \Request::url();
-                return $response->withHeader('Location', \URLHelper::getScriptURL('dispatch.php/login'));
+                $_SESSION['redirect_after_login'] ??= \Request::url();
+                return $this->response_factory->createResponse(302)
+                    ->withHeader('Location', \URLHelper::getScriptURL('dispatch.php/login'));
             }
             unset($course_id);
         }
diff --git a/lib/middleware/SessionMiddleware.php b/lib/middleware/SessionMiddleware.php
index 7c4f242..fb9155d 100644
--- a/lib/middleware/SessionMiddleware.php
+++ b/lib/middleware/SessionMiddleware.php
@@ -22,8 +22,9 @@ use Studip\Session\Manager;
 
 final class SessionMiddleware implements MiddlewareInterface
 {
-    public function __construct(private Manager $session_manager)
-    {
+    public function __construct(
+        private readonly Manager $session_manager
+    ) {
     }
 
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
-- 
cgit v1.0