From 0fef612a780c29cec550fb97c458ff465f73a997 Mon Sep 17 00:00:00 2001 From: JVT038 <47184046+JVT038@users.noreply.github.com> Date: Sat, 30 Sep 2023 18:51:49 +0200 Subject: [PATCH 1/4] Automatically redirect user if not authenticated and redirect the user back if they have logged in --- src/HttpController/Web/AuthenticationController.php | 13 +++++++------ .../Web/Middleware/UserIsAuthenticated.php | 2 +- src/ValueObject/Http/Response.php | 6 ++++++ templates/page/login.html.twig | 6 ++++++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/HttpController/Web/AuthenticationController.php b/src/HttpController/Web/AuthenticationController.php index edaf6084..68bc2837 100644 --- a/src/HttpController/Web/AuthenticationController.php +++ b/src/HttpController/Web/AuthenticationController.php @@ -37,11 +37,12 @@ public function login(Request $request) : Response } catch (InvalidCredentials) { $this->sessionWrapper->set('failedLogin', true); } - + $redirect = $postParameters['redirect']; + $target = $redirect ?? $_SERVER['HTTP_REFERER']; return Response::create( StatusCode::createSeeOther(), null, - [Header::createLocation($_SERVER['HTTP_REFERER'])], + [Header::createLocation($target)], ); } @@ -56,20 +57,20 @@ public function logout() : Response ); } - public function renderLoginPage() : Response + public function renderLoginPage(Request $request) : Response { $failedLogin = $this->sessionWrapper->has('failedLogin'); + $redirect = $request->getGetParameters()['redirect'] ?? false; $this->sessionWrapper->unset('failedLogin'); $renderedTemplate = $this->twig->render( 'page/login.html.twig', [ - 'failedLogin' => $failedLogin + 'failedLogin' => $failedLogin, + 'redirect' => $redirect ], ); - $this->sessionWrapper->unset('failedLogin'); - return Response::create( StatusCode::createOk(), $renderedTemplate, diff --git a/src/HttpController/Web/Middleware/UserIsAuthenticated.php b/src/HttpController/Web/Middleware/UserIsAuthenticated.php index ddc0af62..104a6bcf 100644 --- a/src/HttpController/Web/Middleware/UserIsAuthenticated.php +++ b/src/HttpController/Web/Middleware/UserIsAuthenticated.php @@ -18,6 +18,6 @@ public function __invoke() : ?Response return null; } - return Response::createForbidden(); + return Response::createForbiddenRedirect($_SERVER['REQUEST_URI']); } } diff --git a/src/ValueObject/Http/Response.php b/src/ValueObject/Http/Response.php index 26dbb7e1..89dbb0ef 100644 --- a/src/ValueObject/Http/Response.php +++ b/src/ValueObject/Http/Response.php @@ -34,6 +34,12 @@ public static function createForbidden() : self return new self(StatusCode::createForbidden()); } + public static function createForbiddenRedirect(string $redirectTarget) : self + { + $query = urlencode($redirectTarget); + return new self(StatusCode::createForbidden(), null, [Header::createLocation('/login?redirect='.$query)]); + } + public static function createJson(string $body) : self { return new self(StatusCode::createOk(), $body, [Header::createContentTypeJson()]); diff --git a/templates/page/login.html.twig b/templates/page/login.html.twig index ee5c3896..bf3fa7b9 100644 --- a/templates/page/login.html.twig +++ b/templates/page/login.html.twig @@ -43,6 +43,12 @@ Invalid credentials {% endif %} + {% if redirect != false %} + + + {% endif %} {% if registrationEnabled == true %} Create new user From 190ecc061f14100678b51bc592f33fc3ce4d7152 Mon Sep 17 00:00:00 2001 From: Lee Peuker Date: Thu, 5 Oct 2023 20:29:44 +0200 Subject: [PATCH 2/4] Minor additional improvements --- src/HttpController/Web/DashboardController.php | 2 +- templates/page/login.html.twig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/HttpController/Web/DashboardController.php b/src/HttpController/Web/DashboardController.php index 348c2a5c..b446f200 100644 --- a/src/HttpController/Web/DashboardController.php +++ b/src/HttpController/Web/DashboardController.php @@ -31,7 +31,7 @@ public function render(Request $request) : Response { $userId = $this->userPageAuthorizationChecker->findUserIdIfCurrentVisitorIsAllowedToSeeUser((string)$request->getRouteParameters()['username']); if ($userId === null) { - return Response::createSeeOther('/'); + return Response::createForbiddenRedirect($request->getPath()); } $dashboardRows = $this->dashboardFactory->createDashboardRowsForUser($this->userApi->fetchUser($userId)); diff --git a/templates/page/login.html.twig b/templates/page/login.html.twig index bf3fa7b9..c4a7a707 100644 --- a/templates/page/login.html.twig +++ b/templates/page/login.html.twig @@ -45,7 +45,7 @@ {% endif %} {% if redirect != false %} {% endif %} From a3180ee6df1d55f296bb68c5dbe1e02d7b946c39 Mon Sep 17 00:00:00 2001 From: Lee Peuker Date: Thu, 5 Oct 2023 20:35:17 +0200 Subject: [PATCH 3/4] Fix issue with error message display --- templates/page/login.html.twig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/templates/page/login.html.twig b/templates/page/login.html.twig index c4a7a707..0fcb6002 100644 --- a/templates/page/login.html.twig +++ b/templates/page/login.html.twig @@ -42,13 +42,14 @@ - {% endif %} + {% else %} {% if redirect != false %} {% endif %} + {% endif %} {% if registrationEnabled == true %} Create new user From 56d9cc862940254270e8e19fff4696b130cda90e Mon Sep 17 00:00:00 2001 From: Lee Peuker Date: Thu, 5 Oct 2023 21:06:34 +0200 Subject: [PATCH 4/4] Only allow relative urls for auth redirects --- src/HttpController/Web/AuthenticationController.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/HttpController/Web/AuthenticationController.php b/src/HttpController/Web/AuthenticationController.php index 68bc2837..faa2dd71 100644 --- a/src/HttpController/Web/AuthenticationController.php +++ b/src/HttpController/Web/AuthenticationController.php @@ -39,10 +39,19 @@ public function login(Request $request) : Response } $redirect = $postParameters['redirect']; $target = $redirect ?? $_SERVER['HTTP_REFERER']; + + $urlParts = parse_url($target); + if (is_array($urlParts) === false) { + $urlParts = ['path' => '/']; + } + + /* @phpstan-ignore-next-line */ + $targetRelativeUrl = $urlParts['path'] . $urlParts['query'] ?? ''; + return Response::create( StatusCode::createSeeOther(), null, - [Header::createLocation($target)], + [Header::createLocation($targetRelativeUrl)], ); }