Skip to content

Commit

Permalink
Merge pull request #1986 from stloyd/remove-twig
Browse files Browse the repository at this point in the history
Remove Twig usage from `AuthenticationFailureHandler`
  • Loading branch information
stloyd committed Feb 24, 2024
2 parents 0c75237 + 1349409 commit c68853a
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 31 deletions.
1 change: 0 additions & 1 deletion src/Resources/config/oauth.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
->args([
service('request_stack'),
service('router'),
service('twig'),
'%hwi_oauth.connect%',
]);
};
48 changes: 38 additions & 10 deletions src/Security/Http/Authentication/AuthenticationFailureHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,44 @@
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Routing\RouterInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
use Twig\Environment;
use Symfony\Component\Security\Http\ParameterBagUtils;

final class AuthenticationFailureHandler implements AuthenticationFailureHandlerInterface
{
private array $defaultOptions = [
'failure_path' => 'hwi_oauth_connect_registration',
'failure_forward' => false,
'login_path' => '/login',
'failure_path_parameter' => '_failure_path',
];

public function __construct(
private readonly RequestStack $requestStack,
private readonly RouterInterface $router,
private readonly Environment $twig,
private readonly bool $connect
private readonly bool $connect,
) {
}

public function setOptions(array $options): void
{
$this->defaultOptions = array_merge($this->defaultOptions, $options);
}

public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response
{
$options = $this->defaultOptions;

$failureUrl = ParameterBagUtils::getRequestParameterValue($request, $options['failure_path_parameter']);

if (\is_string($failureUrl) && (str_starts_with($failureUrl, '/') || str_starts_with($failureUrl, 'http'))) {
$options['failure_path'] = $failureUrl;
}

$options['failure_path'] ??= $options['login_path'];

$error = $exception->getPrevious();

if ($this->connect && $error instanceof AccountNotLinkedException) {
Expand All @@ -50,9 +70,13 @@ public function onAuthenticationFailure(Request $request, AuthenticationExceptio
$session->set('_hwi_oauth.registration_error.'.$key, $error);
}

return new RedirectResponse(
$this->router->generate('hwi_oauth_connect_registration', ['key' => $key], UrlGeneratorInterface::ABSOLUTE_PATH)
);
if ('/' === $options['failure_path'][0]) {
$failurePath = $request->getUriForPath($options['failure_path'][0]);
} else {
$failurePath = $this->router->generate($options['failure_path'], ['key' => $key]);
}

return new RedirectResponse($failurePath);
}

if ($error instanceof AuthenticationException) {
Expand All @@ -61,9 +85,13 @@ public function onAuthenticationFailure(Request $request, AuthenticationExceptio
$error = $exception->getMessageKey();
}

return new Response(
$this->twig->render('@HWIOAuth/Connect/login.html.twig', ['error' => $error])
);
if ('/' === $options['login_path'][0]) {
$loginPath = $request->getUriForPath($options['login_path'][0]);
} else {
$loginPath = $this->router->generate($options['login_path'], ['error' => $error]);
}

return new RedirectResponse($loginPath);
}

private function getSession(): ?SessionInterface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@
use Symfony\Component\HttpFoundation\Session\SessionInterface;
use Symfony\Component\Routing\RouterInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Twig\Environment;

final class AuthenticationFailureHandlerTest extends TestCase
{
public function testRendersLoginPageByDefault()
public function testRedirectsToLoginPathWhenNotSet()
{
$request = Request::create('/login');
$requestStack = new RequestStack();
Expand All @@ -36,16 +35,35 @@ public function testRendersLoginPageByDefault()
$handler = new AuthenticationFailureHandler(
$requestStack,
$router = $this->createMock(RouterInterface::class),
$twig = $this->createMock(Environment::class),
false
);

$router->expects($this->never())
->method('generate');

$twig->expects($this->once())
->method('render')
->with('@HWIOAuth/Connect/login.html.twig', ['error' => 'An authentication exception occurred.']);
$this->assertInstanceOf(
Response::class,
$handler->onAuthenticationFailure($request, new AuthenticationException())
);
}

public function testRedirectsToLoginRoute()
{
$request = Request::create('/login');
$requestStack = new RequestStack();
$requestStack->push($request);

$handler = new AuthenticationFailureHandler(
$requestStack,
$router = $this->createMock(RouterInterface::class),
false,
);
$handler->setOptions(['login_path' => 'route_name']);

$router->expects($this->once())
->method('generate')
->with('route_name', ['error' => 'An authentication exception occurred.'], 1)
->willReturn('/path');

$this->assertInstanceOf(
Response::class,
Expand All @@ -62,17 +80,12 @@ public function testDoesNothingWhenConnectIsDisabled()
$handler = new AuthenticationFailureHandler(
$requestStack,
$router = $this->createMock(RouterInterface::class),
$twig = $this->createMock(Environment::class),
false
);

$router->expects($this->never())
->method('generate');

$twig->expects($this->once())
->method('render')
->with('@HWIOAuth/Connect/login.html.twig', ['error' => 'An authentication exception occurred.']);

$this->assertInstanceOf(
Response::class,
$handler->onAuthenticationFailure($request, new AuthenticationException())
Expand All @@ -88,17 +101,12 @@ public function testDoesNothingWhenExceptionIsNotOAuthOne()
$handler = new AuthenticationFailureHandler(
$requestStack,
$router = $this->createMock(RouterInterface::class),
$twig = $this->createMock(Environment::class),
true
);

$router->expects($this->never())
->method('generate');

$twig->expects($this->once())
->method('render')
->with('@HWIOAuth/Connect/login.html.twig', ['error' => 'An authentication exception occurred.']);

$this->assertInstanceOf(
Response::class,
$handler->onAuthenticationFailure($request, new AuthenticationException())
Expand All @@ -116,7 +124,6 @@ public function testRedirectsToRegistrationPage()
$handler = new AuthenticationFailureHandler(
$requestStack,
$router = $this->createMock(RouterInterface::class),
$twig = $this->createMock(Environment::class),
true
);

Expand All @@ -125,9 +132,6 @@ public function testRedirectsToRegistrationPage()
->with('hwi_oauth_connect_registration', ['key' => $key = time()])
->willReturn('https://localhost/register/'.$key);

$twig->expects($this->never())
->method('render');

$this->assertInstanceOf(
RedirectResponse::class,
$handler->onAuthenticationFailure($request, new AuthenticationException(previous: new AccountNotLinkedException()))
Expand Down

0 comments on commit c68853a

Please sign in to comment.