Skip to content

Commit

Permalink
Merge pull request #517 from leepeuker/improve-unauthenticated-users-…
Browse files Browse the repository at this point in the history
…response

Improve the handling of unauthenticated users
  • Loading branch information
leepeuker authored Oct 5, 2023
2 parents 31aeca5 + 56d9cc8 commit 797306d
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 7 deletions.
20 changes: 15 additions & 5 deletions src/HttpController/Web/AuthenticationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,21 @@ public function login(Request $request) : Response
} catch (InvalidCredentials) {
$this->sessionWrapper->set('failedLogin', true);
}
$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($_SERVER['HTTP_REFERER'])],
[Header::createLocation($targetRelativeUrl)],
);
}

Expand All @@ -56,20 +66,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,
Expand Down
2 changes: 1 addition & 1 deletion src/HttpController/Web/DashboardController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/HttpController/Web/Middleware/UserIsAuthenticated.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ public function __invoke() : ?Response
return null;
}

return Response::createForbidden();
return Response::createForbiddenRedirect($_SERVER['REQUEST_URI']);
}
}
6 changes: 6 additions & 0 deletions src/ValueObject/Http/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()]);
Expand Down
7 changes: 7 additions & 0 deletions templates/page/login.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@
<div class="alert alert-danger" role="alert" style="margin-bottom: 0.7rem!important;">
Invalid credentials
</div>
{% else %}
{% if redirect != false %}
<div class="alert alert-danger" role="alert" style="margin-bottom: 0.7rem!important;">
Sign in to access page
</div>
<input type="hidden" value="{{ redirect }}" name="redirect" />
{% endif %}
{% endif %}
<button class="w-100 btn btn-lg btn-primary mb-3" type="submit">Sign in</button>
{% if registrationEnabled == true %}
Expand Down

0 comments on commit 797306d

Please sign in to comment.