Skip to content

Commit

Permalink
UML-3106 Implement One Login callback handler on service-front (#2445)
Browse files Browse the repository at this point in the history
* UML-3106 Implement One Login callback handler on service-front

* Refactor switch statement to match

* add behat testing for access denied and temp unavailable errors

* add generic error behat tests

* Can redirect in Welsh

* Replace assertObjectHasAttributes with assertObjectHasProperty

* Callback method passes code, state, authsession, abstracthandler passes basepath

* add PR fixes

* replace testableAbstractHandler with an anoymous class
  • Loading branch information
MishNajam authored Dec 11, 2023
1 parent 1327c6f commit 66b939c
Show file tree
Hide file tree
Showing 13 changed files with 297 additions and 18 deletions.
31 changes: 31 additions & 0 deletions service-front/app/features/context/UI/AccountContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -2072,6 +2072,19 @@ public function iClickTheOneLoginButton(): void
$this->iDoFollowRedirects();
}

/**
* @When /^I have logged in to one login in (English|Welsh)$/
*/
public function iHaveLoggedInToOneLogin($language): void
{
$this->iAmOnTheTemporaryOneLoginPage();
$this->language = $language === 'English' ? 'en' : 'cy';
if ($this->language === 'cy') {
$this->iSelectTheWelshLanguage();
}
$this->iClickTheOneLoginButton();
}

/**
* @Then /^I am redirected to the redirect page in (English|Welsh)$/
*/
Expand All @@ -2096,4 +2109,22 @@ public function iSelectTheWelshLanguage(): void
$this->language = 'cy';
$this->ui->clickLink('Cymraeg');
}

/**
* @When /^One Login returns a "(.*)" error$/
*/
public function oneLoginReturnsAError($errorType): void
{
$this->ui->visit('/home/login?error=' . $errorType . '&state=fakestate');
}

/**
* @Then /^I am redirected to the login page with a "(.*)" error and "(.*)"$/
*/
public function iAmRedirectedToTheLanguageErrorPage($errorType, $errorMessage): void
{
$basePath = $this->language === 'cy' ? '/cy' : '';
$this->ui->assertPageAddress($basePath . '/home?error=' . $errorType);
$this->ui->assertPageContainsText($errorMessage);
}
}
2 changes: 1 addition & 1 deletion service-front/app/features/context/UI/CommonContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ public function iShouldBeOnTheWelshHomepageOfTheService()
/**
* @Then /^I should be shown an error page$/
*/
public function iShouldBeShownAnErrorPage()
public function iShouldBeShownAnErrorPage(): void
{
$this->ui->assertPageContainsText('Sorry, there is a problem with the service');
}
Expand Down
29 changes: 28 additions & 1 deletion service-front/app/features/one-login.feature
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,31 @@
Given I am on the temporary one login page
And I select the Welsh language
When I click the one login button
Then I am redirected to the redirect page in Welsh
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 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>"

Examples:
| language | error_type | error_message |
| English | access_denied | Tried to login however access is denied |
| English | temporarily_unavailable | One Login is temporarily unavailable |
| Welsh | access_denied | Mae problem |
| Welsh | temporarily_unavailable | Mae problem |

@ui @actor @ff:allow_gov_one_login:true
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

Examples:
| error_type |
| unauthorized_client |
| invalid_request |
| invalid_scope |
| unsupported_response_type |
| server_error |
13 changes: 13 additions & 0 deletions service-front/app/src/Actor/src/Form/OneLoginForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@ class OneLoginForm extends AbstractForm
{
public const FORM_NAME = 'one_login';

public const ACCESS_DENIED_ERROR = 'access_denied';
public const TEMPORARILY_UNAVAILABLE_ERROR = 'temporarily_unavailable';

/**
* Error messages
*
* @var string[]
*/
protected array $messageTemplates = [
self::ACCESS_DENIED_ERROR => 'Tried to login however access is denied.',
self::TEMPORARILY_UNAVAILABLE_ERROR => 'One Login is temporarily unavailable.',
];

public function __construct(CsrfGuardInterface $csrfGuard)
{
parent::__construct(self::FORM_NAME, $csrfGuard);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ class AuthenticateOneLoginHandler extends AbstractHandler implements CsrfGuardAw
use Logger;
use Session;

public const OIDC_AUTH_INTERFACE = 'oidcauthinterface';

public function __construct(
TemplateRendererInterface $renderer,
UrlHelper $urlHelper,
Expand All @@ -54,20 +52,29 @@ public function handle(ServerRequestInterface $request): ResponseInterface
$signInLink = $this->serverUrlHelper->generate($this->urlHelper->generate('auth-redirect'));
$uiLocale = Locale::getPrimaryLanguage($request->getAttribute('locale'));

//TODO: UML-3203 Identify if OneLogin can handle multiple redirect urls, then remove
if ($uiLocale === 'cy') {
$signInLink = str_replace('/cy', '', $signInLink);
}

$result = $this->authenticateOneLoginService->authenticate($uiLocale, $signInLink);
$result['customs'] = ['ui_locale' => $uiLocale];
$result['customs'] = [
'ui_locale' => $uiLocale,
'redirect_uri' => $signInLink,
];

$this
->getSession($request, SessionMiddleware::SESSION_ATTRIBUTE)
?->set(self::OIDC_AUTH_INTERFACE, AuthSession::fromArray($result));
?->set(OneLoginService::OIDC_AUTH_INTERFACE, AuthSession::fromArray($result));

return new RedirectResponse($result['url']);
}

$params = $request->getQueryParams();
if (array_key_exists('error', $params)) {
$form->addErrorMessage($params['error']);
}

return new HtmlResponse($this->renderer->render('actor::one-login', [
'form' => $form,
]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,80 @@
namespace Actor\Handler;

use Common\Handler\AbstractHandler;
use Common\Handler\LoggerAware;
use Common\Handler\SessionAware;
use Common\Handler\Traits\Logger;
use Common\Handler\Traits\Session;
use Common\Service\OneLogin\OneLoginService;
use Facile\OpenIDClient\Session\AuthSession;
use Laminas\Diactoros\Response\HtmlResponse;
use Mezzio\Helper\UrlHelper;
use Mezzio\Session\SessionMiddleware;
use Mezzio\Template\TemplateRendererInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Log\LoggerInterface;
use RuntimeException;

/**
* @codeCoverageIgnore
*/
class OneLoginCallbackHandler extends AbstractHandler
class OneLoginCallbackHandler extends AbstractHandler implements LoggerAware, SessionAware
{
use Logger;
use Session;

public function __construct(
private OneLoginService $oneLoginService,
TemplateRendererInterface $renderer,
UrlHelper $urlHelper,
LoggerInterface $logger,
) {
parent::__construct($renderer, $urlHelper, $logger);
}

public function handle(ServerRequestInterface $request): ResponseInterface
{
$authParams = $request->getQueryParams();

$session = $this->getSession($request, SessionMiddleware::SESSION_ATTRIBUTE);
$authSession = AuthSession::fromArray($session->get(OneLoginService::OIDC_AUTH_INTERFACE));
$ui_locale = $authSession->getCustoms()['ui_locale'];

if (array_key_exists('error', $authParams)) {
$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' => $error],
$ui_locale === 'cy' ? $ui_locale : null
),
default => throw new RuntimeException('Error returned from OneLogin', 500)
};
}

if (!array_key_exists('code', $authParams) || !array_key_exists('state', $authParams)) {
throw new RuntimeException('Required parameters not passed for authentication', 500);
}

return new HtmlResponse('<h1>Hello World</h1>');

//TODO: UML-3078
// $user = $this->oneLoginService->callback($authParams['code'], $authParams['state'], $authSession);
// //Add user to session
// if (! is_null($user)) {
// if (empty($user->getDetail('LastLogin'))) {
// return $this->redirectToRoute('lpa.add');
// } else {
// return $this->redirectToRoute('lpa.dashboard');
// }
// }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<div class="govuk-width-container">
<main class="govuk-main-wrapper" id="main-content" role="main">
<div class="govuk-grid-row">
{{ govuk_error_summary(form) }}
{{ govuk_form_open(form) }}

<button data-prevent-double-click="true" name="sign-in-one-login" type="submit" class="govuk-button">{% trans %}Sign in via One Login{% endtrans %}</button>
Expand Down
21 changes: 14 additions & 7 deletions service-front/app/src/Common/src/Handler/AbstractHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
use Psr\Http\Server\RequestHandlerInterface;
use Psr\Log\LoggerInterface;

/**
* @codeCoverageIgnore
*/
abstract class AbstractHandler implements RequestHandlerInterface
{
public function __construct(
Expand All @@ -30,17 +33,21 @@ public function __construct(
abstract public function handle(ServerRequestInterface $request): ResponseInterface;

/**
* Redirect to the specified route
* Handles a redirect to route
* $basePathOverride used to override from English to Welsh only
*
* @param $route
* @param array $routeParams
* @param array $queryParams
* @param $routeParams
* @param $queryParams
* @param string|null $basePathOverride
* @return RedirectResponse
*/
protected function redirectToRoute($route, $routeParams = [], $queryParams = []): RedirectResponse
public function redirectToRoute($route, $routeParams = [], $queryParams = [], ?string $basePathOverride = null): RedirectResponse
{
return new RedirectResponse(
$this->urlHelper->generate($route, $routeParams, $queryParams)
);
//TODO: UML-3203 Identify if OneLogin can handle multiple redirect urls, then remove
if ($basePathOverride !== null) {
$this->urlHelper->setBasePath($basePathOverride);
}
return new RedirectResponse($this->urlHelper->generate($route, $routeParams, $queryParams));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
namespace Common\Service\OneLogin;

use Common\Service\ApiClient\Client as ApiClient;
use Facile\OpenIDClient\Session\AuthSession;

class OneLoginService
{
public const OIDC_AUTH_INTERFACE = 'oidcauthinterface';

public function __construct(private ApiClient $apiClient)
{
}
Expand All @@ -19,4 +22,13 @@ public function authenticate(string $uiLocale, string $redirectUrl): ?array
'redirect_url' => $redirectUrl,
]);
}

public function callback(string $code, string $state, AuthSession $authCredentials): ?array
{
return $this->apiClient->httpPost('/v1/auth/callback', [
'code' => $code,
'state' => $state,
'auth_session' => $authCredentials,
]);
}
}
74 changes: 74 additions & 0 deletions service-front/app/test/CommonTest/Handler/AbstractHandlerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
<?php

declare(strict_types=1);

namespace CommonTest\Handler;

use Common\Handler\AbstractHandler;
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 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 class ($renderer->reveal(), $urlHelperMock->reveal()) extends AbstractHandler {
public function handle(ServerRequestInterface $request): ResponseInterface
{
throw new Exception('Not implemented');
}
};
$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 class ($renderer->reveal(), $urlHelperMock->reveal()) extends AbstractHandler {
public function handle(ServerRequestInterface $request): ResponseInterface
{
throw new Exception('Not implemented');
}
};
$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 @@ -91,7 +91,7 @@ public function it_correctly_parses_an_lpa_api_response(): void
$this->lpaId => $this->lpaData,
]
);
$this->assertObjectHasAttribute($this->lpaId, $result);
$this->assertObjectHasProperty($this->lpaId, $result);
$this->assertEquals($this->lpa, $result->{$this->lpaId}->lpa);
$this->assertEquals($this->actor, $result->{$this->lpaId}->actor['details']);
$this->assertEquals($this->iapImages, $result->{$this->lpaId}->iap);
Expand Down
Loading

0 comments on commit 66b939c

Please sign in to comment.