Skip to content

Commit

Permalink
add PR fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
MishNajam committed Dec 7, 2023
1 parent 1d7b91f commit 25900b4
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 30 deletions.
18 changes: 2 additions & 16 deletions service-front/app/features/one-login.feature
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
Then I am redirected to the redirect page in Welsh

@ui @actor @ff:allow_gov_one_login:true @welsh
Scenario Outline: One Login returns a specific error
Scenario Outline: One Login returns a user error
Given I have logged in to one login in <language>
When One Login returns a "<error_type>" error
Then I am redirected to the login page with a "<error_type>" error and "<error_message>"
Expand All @@ -28,7 +28,7 @@
| Welsh | temporarily_unavailable | Mae problem |

@ui @actor @ff:allow_gov_one_login:true
Scenario Outline: One Login returns a generic error
Scenario Outline: One Login returns a system error
Given I have logged in to one login in English
When One Login returns a "<error_type>" error
Then I should be shown an error page
Expand All @@ -40,17 +40,3 @@
| invalid_scope |
| unsupported_response_type |
| server_error |

@ui @actor @ff:allow_gov_one_login:true @welsh
Scenario Outline: One Login returns a generic error in Welsh
Given I have logged in to one login in Welsh
When One Login returns a "<error_type>" error
Then I should be shown an error page in Welsh

Examples:
| error_type |
| unauthorized_client |
| invalid_request |
| invalid_scope |
| unsupported_response_type |
| server_error |
20 changes: 10 additions & 10 deletions service-front/app/src/Actor/src/Handler/OneLoginCallbackHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,18 @@ public function handle(ServerRequestInterface $request): ResponseInterface
$ui_locale = $authSession->getCustoms()['ui_locale'];

if (array_key_exists('error', $authParams)) {
$this->logger->notice('User attempted to login via OneLogin however there was an error');
return match ($authParams['error']) {
'access_denied' => $this->redirectToRoute(
$error = $authParams['error'];
$error === 'temporarily_unavailable' ?
$this->logger->warning('User attempted One Login but it is unavailable') :
$this->logger->notice(
'User attempted One Login but received an {error} error',
['error' => $error]
);
return match ($error) {
'access_denied', 'temporarily_unavailable' => $this->redirectToRoute(
'home',
[],
['error' => 'access_denied'],
$ui_locale === 'cy' ? $ui_locale : null
),
'temporarily_unavailable' => $this->redirectToRoute(
'home',
[],
['error' => 'temporarily_unavailable'],
['error' => $error],
$ui_locale === 'cy' ? $ui_locale : null
),
default => throw new RuntimeException('Error returned from OneLogin', 500)
Expand Down
3 changes: 1 addition & 2 deletions service-front/app/src/Common/src/Handler/AbstractHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,12 @@ abstract public function handle(ServerRequestInterface $request): ResponseInterf
* @param string|null $localeOverride
* @return RedirectResponse
*/
protected function redirectToRoute($route, $routeParams = [], $queryParams = [], ?string $localeOverride = null): RedirectResponse
public function redirectToRoute($route, $routeParams = [], $queryParams = [], ?string $localeOverride = null): RedirectResponse
{
//TODO: UML-3203 Identify if OneLogin can handle multiple redirect urls, then remove
if ($localeOverride !== null) {
$this->urlHelper->setBasePath($localeOverride);
}

return new RedirectResponse($this->urlHelper->generate($route, $routeParams, $queryParams));
}
}
72 changes: 72 additions & 0 deletions service-front/app/test/CommonTest/Handler/AbstractHandlerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

declare(strict_types=1);

namespace CommonTest\Handler;

use Common\Handler\AbstractHandler;
use Exception;
use Mezzio\Helper\UrlHelper;
use Mezzio\Template\TemplateRendererInterface;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Prophecy\Prophecy\ObjectProphecy;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;

class TestableAbstractHandler extends AbstractHandler {
public function handle(ServerRequestInterface $request): ResponseInterface
{
throw new Exception('Not implemented');
}
}

class AbstractHandlerTest extends TestCase
{
use ProphecyTrait;

/**
* @test
*/
public function testRedirectToRouteWithOverride(): void
{
$localOverride = 'cy';
/** @var UrlHelper|ObjectProphecy $urlHelperMock */
$urlHelperMock = $this->prophesize(UrlHelper::class);
$urlHelperMock->setBasePath($localOverride)->shouldBeCalledOnce();
$urlHelperMock->generate(
'fake-route',
['fake-route-parameters'],
['fake-query-parameters'],
)->willReturn('/cy/fake-route');

$renderer = $this->prophesize(TemplateRendererInterface::class);

$abstractHandler = new TestableAbstractHandler($renderer->reveal(), $urlHelperMock->reveal());
$response = $abstractHandler->redirectToRoute(
'fake-route',
['fake-route-parameters'],
['fake-query-parameters'],
$localOverride,
);
$this->assertEquals('/cy/fake-route', $response->getHeader('location')[0]);
}

/**
* @test
*/
public function testRedirectToRouteOverrideSetToNull(): void
{
/** @var UrlHelper|ObjectProphecy $urlHelperMock */
$urlHelperMock = $this->prophesize(UrlHelper::class);
$urlHelperMock->setBasePath(Argument::any())->shouldNotBeCalled();
$urlHelperMock->generate('fake-route', [], [])->willReturn('/fake-route');

$renderer = $this->prophesize(TemplateRendererInterface::class);

$abstractHandler = new TestableAbstractHandler($renderer->reveal(), $urlHelperMock->reveal());
$response = $abstractHandler->redirectToRoute('fake-route');
$this->assertEquals('/fake-route', $response->getHeader('location')[0]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public function can_get_callback_request_uri(): void
'Identity' => 'fake-sub-identity',
'Email' => '[email protected]',
'LastLogin' => $lastLogin,
'Birthday' => '01/01/1990'
'Birthday' => '1990-01-01'
]);

$oneLoginService = new OneLoginService($apiClientProphecy->reveal());
Expand All @@ -90,7 +90,7 @@ public function can_get_callback_request_uri(): void
'Identity' => 'fake-sub-identity',
'Email' => '[email protected]',
'LastLogin' => $lastLogin,
'Birthday' => '01/01/1990'
'Birthday' => '1990-01-01'
],
$response);
}
Expand Down

0 comments on commit 25900b4

Please sign in to comment.