UML-3121 make redirect uri dynamic (#2390)

* UML-3121 make redirect uri dynamic * Clean up authenticateOneLoginHandler * Fix redirect fail on welsh bug * revert url helper logic * Refactor work, rename redirect url and store ui_locale in authsession * Add behat test * Revert workflow changes * Refactor behat tests
ministryofjustice · Nov 3, 2023 · 46c77d1 · 46c77d1
1 parent e140372
commit 46c77d1
Show file tree

Hide file tree

Showing 11 changed files with 75 additions and 42 deletions.
diff --git a/.github/workflows/_build-and-push.yml b/.github/workflows/_build-and-push.yml
@@ -146,19 +146,9 @@ jobs:
           matrix.run_unit_tests == 'true' &&
           (inputs.specific_path == 'all' || inputs.specific_path == matrix.svc_prefix)
 
-      - name: ecr login
-        id: login_ecr
-        uses: aws-actions/amazon-ecr-login@f8672cc4c5410eabbb9df3d9bb4c7ad01fc4cc3e # [email protected]
-        with:
-          registries: 311462405659
-        if: |
-          inputs.push_to_ecr == 'true' &&
-          (inputs.specific_path == 'all' || inputs.specific_path == matrix.svc_prefix)
-
       - name: docker integration and acceptance tests
         env:
           SVC_PREFIX: ${{ matrix.svc_prefix }}
-          ECR_REGISTRY: ${{ steps.login_ecr.outputs.registry }}
         run: |
           if [[ "${SVC_PREFIX}" = "api" ]]; then
             docker run -d --rm --name lpa-codes-pact-mock pactfoundation/pact-cli:latest mock-service -p "80" --host "0.0.0.0" \
@@ -167,15 +157,10 @@ jobs:
               --pact-dir /tmp/pacts --consumer use_a_lasting_power_of_attorney --provider api-gateway
             docker run -d --rm --name iap-images-mock pactfoundation/pact-cli:latest mock-service -p "80" --host "0.0.0.0" \
               --pact-dir /tmp/pacts --consumer use_a_lasting_power_of_attorney --provider iap-api-gateway
-            docker run -d --rm --name mock-one-login $ECR_REGISTRY/use_an_lpa/mock_onelogin_app
 
             export DOCKER_REMOTE_LPA_PACT_IP="$(docker inspect --format='{{.NetworkSettings.IPAddress}}' lpa-codes-pact-mock)"
             export DOCKER_REMOTE_API_PACT_IP="$(docker inspect --format='{{.NetworkSettings.IPAddress}}' api-gateway-pact-mock)"
             export DOCKER_REMOTE_IAP_PACT_IP="$(docker inspect --format='{{.NetworkSettings.IPAddress}}' iap-images-mock)"
-            export DOCKER_REMOTE_MOCK_ONELOGIN_IP="$(docker inspect --format='{{.NetworkSettings.IPAddress}}' mock-one-login)"
-
-            curl -XGET $DOCKER_REMOTE_MOCK_ONELOGIN_IP:8080/token
-            docker logs mock-one-login
 
             docker run -d --name behattests \
               --add-host lpa-codes-pact-mock:$DOCKER_REMOTE_LPA_PACT_IP \
@@ -189,7 +174,6 @@ jobs:
             docker stop lpa-codes-pact-mock
             docker stop api-gateway-pact-mock
             docker stop iap-images-mock
-            docker stop mock-one-login
           elif [[ "${SVC_PREFIX}" = "front" ]]; then
             docker run -d --name behattests front-app:latest
 
@@ -272,6 +256,15 @@ jobs:
         with:
           sarif_file: 'trivy-results.sarif'
 
+      - name: ecr login
+        id: login_ecr
+        uses: aws-actions/amazon-ecr-login@f8672cc4c5410eabbb9df3d9bb4c7ad01fc4cc3e # [email protected]
+        with:
+          registries: 311462405659
+        if: |
+          inputs.push_to_ecr == 'true' &&
+          (inputs.specific_path == 'all' || inputs.specific_path == matrix.svc_prefix)
+
       - name: tag and push container
         env:
           ECR_REGISTRY: ${{ steps.login_ecr.outputs.registry }}

diff --git a/docker-compose.dependencies.yml b/docker-compose.dependencies.yml
@@ -109,4 +109,4 @@ services:
     environment:
       PUBLIC_URL: http://localhost:4013
       INTERNAL_URL: http://mock-one-login:8080
-      REDIRECT_URL: http://localhost:9002/auth/redirect
+      REDIRECT_URL: http://localhost:9002/home/login
diff --git a/service-api/app/src/App/src/Handler/OneLoginAuthenticationRequestHandler.php b/service-api/app/src/App/src/Handler/OneLoginAuthenticationRequestHandler.php
@@ -31,15 +31,22 @@ public function handle(ServerRequestInterface $request): ResponseInterface
     {
         $requestData = $request->getQueryParams();
 
+        if (empty($requestData['redirect_url'])) {
+            throw new BadRequestException('Redirect URL must be provided');
+        }
+
         if (empty($requestData['ui_locale'])) {
             throw new BadRequestException('Ui locale must be provided');
         }
 
-        $ui_locale = strtolower($requestData['ui_locale']);
+        $redirect_url = $requestData['redirect_url'];
+        $ui_locale    = strtolower($requestData['ui_locale']);
         if ($ui_locale !== 'en' and $ui_locale !== 'cy') {
             throw new BadRequestException('ui_locale is not set to en or cy');
         }
 
-        return new JsonResponse($this->authenticationRequestService->createAuthenticationRequest($ui_locale));
+        $authRequest = $this->authenticationRequestService->createAuthenticationRequest($ui_locale, $redirect_url);
+
+        return new JsonResponse($authRequest);
     }
 }
diff --git a/service-api/app/src/App/src/Service/Authentication/OneLoginAuthenticationRequestService.php b/service-api/app/src/App/src/Service/Authentication/OneLoginAuthenticationRequestService.php
@@ -22,7 +22,7 @@ public function __construct(
     ) {
     }
 
-    public function createAuthenticationRequest(string $uiLocale): array
+    public function createAuthenticationRequest(string $uiLocale, string $redirectURL): array
     {
 
         $cachedBuilder = new MetadataProviderBuilder();
@@ -59,7 +59,7 @@ public function createAuthenticationRequest(string $uiLocale): array
             [
                 'scope'        => 'openid email',
                 'state'        => $state,
-                'redirect_uri' => 'http://localhost:9002/auth/redirect', //TODO: use dynamic domain UML-3121
+                'redirect_uri' => $redirectURL,
                 'nonce'        => $nonce,
                 'vtr'          => '["Cl.Cm.P2"]',
                 'ui_locales'   => $uiLocale,

diff --git a/...-api/app/test/AppTest/Service/Authentication/OneLoginAuthenticationRequestServiceTest.php b/...-api/app/test/AppTest/Service/Authentication/OneLoginAuthenticationRequestServiceTest.php
@@ -53,15 +53,13 @@ public function create_authentication_request(): void
             $this->issuerBuilder->reveal(),
             $this->cacheFactory->reveal(),
         );
-        $authorisationRequest        = $authorisationRequestService->createAuthenticationRequest('en');
+        $fakeRedirect                = 'http://fakehost/auth/redirect';
+        $authorisationRequest        = $authorisationRequestService->createAuthenticationRequest('en', $fakeRedirect);
         $authorisationRequestUrl     = $authorisationRequest['url'];
         $this->assertStringContainsString('client_id=client-id', $authorisationRequestUrl);
         $this->assertStringContainsString('scope=openid+email', $authorisationRequestUrl);
         $this->assertStringContainsString('vtr=["Cl.Cm.P2"]', urldecode($authorisationRequestUrl));
         $this->assertStringContainsString('ui_locales=en', $authorisationRequestUrl);
-        $this->assertStringContainsString(
-            'redirect_uri=http://localhost:9002/auth/redirect',
-            urldecode($authorisationRequestUrl)
-        );
+        $this->assertStringContainsString('redirect_uri=' . $fakeRedirect, urldecode($authorisationRequestUrl));
     }
 }
diff --git a/service-front/app/config/routes.php b/service-front/app/config/routes.php
@@ -100,6 +100,7 @@
     $app->get('/session-expired', Actor\Handler\ActorSessionExpiredHandler::class, 'session-expired');
     $app->get('/session-check', Actor\Handler\ActorSessionCheckHandler::class, 'session-check');
     $app->get('/session-refresh', Common\Handler\SessionRefreshHandler::class, 'session-refresh');
+    $app->get('/home/login', Actor\Handler\LoginPageHandler::class, 'auth-redirect');
 
     $app->get(
         '/logout',

diff --git a/service-front/app/features/context/UI/AccountContext.php b/service-front/app/features/context/UI/AccountContext.php
@@ -26,6 +26,7 @@
  * @property string userEmailResetToken
  * @property string email
  * @property string password
+ * @property string language
  */
 class AccountContext implements Context
 {
@@ -44,6 +45,7 @@ class AccountContext implements Context
     private const USER_SERVICE_CAN_PASSWORD_RESET = 'UserService::canPasswordReset';
     private const USER_SERVICE_COMPLETE_PASSWORD_RESET = 'UserService::completePasswordReset';
     private const USER_SERVICE_DELETE_ACCOUNT = 'UserService::deleteAccount';
+    private const ONE_LOGIN_SERVICE_AUTHENTICATE = 'OneLoginService::authenticate';
 
     /**
      * @Then /^An account is created using (.*) (.*) (.*)$/
@@ -2037,6 +2039,7 @@ public function iReceiveAnEmailAndShownUniqueInstructionsOnHowToResetMyPassword(
      */
     public function iAmOnTheTemporaryOneLoginPage(): void
     {
+        $this->language = 'en';
         $this->ui->visit('/home');
         $this->ui->assertPageAddress('/home');
         $this->ui->assertElementContainsText('button[name=sign-in-one-login]', 'Sign in via One Login');
@@ -2057,26 +2060,37 @@ public function iClickTheOneLoginButton(): void
                         'url'   => 'http://fake.url/authorize',
                     ]
                 ),
-                self::USER_SERVICE_REQUEST_PASSWORD_RESET
+                self::ONE_LOGIN_SERVICE_AUTHENTICATE
             )
         );
 
         $this->iDoNotFollowRedirects();
         $this->ui->pressButton('Sign in via One Login');
         $this->iDoFollowRedirects();
-
-        $request = $this->apiFixtures->getLastRequest();
-        $params  = $request->getUri()->getQuery();
-        Assert::assertStringContainsString('ui_locale=en', $params);
     }
 
     /**
-     * @Then /^I am redirected to the redirect page$/
+     * @Then /^I am redirected to the redirect page in (English|Welsh)$/
      */
-    public function iAmRedirectedToTheRedirectPage(): void
+    public function iAmRedirectedToTheRedirectPage($language): void
     {
         $locationHeader = $this->ui->getSession()->getResponseHeader('Location');
+        $request        = $this->apiFixtures->getLastRequest();
+        $params         = $request->getUri()->getQuery();
+        $language       = $language === 'English' ? 'en' : 'cy';
+
         assert::assertTrue(isset($locationHeader));
         assert::assertEquals($locationHeader, 'http://fake.url/authorize');
+        assert::assertEquals($language, $this->language);
+        assert::assertStringContainsString('ui_locale=' . $this->language, $params);
+    }
+
+    /**
+     * @When /^I select the Welsh language$/
+     */
+    public function iSelectTheWelshLanguage(): void
+    {
+        $this->language = 'cy';
+        $this->ui->clickLink('Cymraeg');
     }
 }
diff --git a/service-front/app/features/one-login.feature b/service-front/app/features/one-login.feature
@@ -5,4 +5,11 @@
     Scenario: I initiate authentication via one login
       Given I am on the temporary one login page
       When I click the one login button
-      Then I am redirected to the redirect page
+      Then I am redirected to the redirect page in English
+
+    @ui @actor @ff:allow_gov_one_login:true @welsh
+    Scenario: I initiate authentication via one login in Welsh
+      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
diff --git a/service-front/app/src/Actor/src/Handler/AuthenticateOneLoginHandler.php b/service-front/app/src/Actor/src/Handler/AuthenticateOneLoginHandler.php
@@ -16,6 +16,7 @@
 use Facile\OpenIDClient\Session\AuthSession;
 use Laminas\Diactoros\Response\HtmlResponse;
 use Laminas\Diactoros\Response\RedirectResponse;
+use Mezzio\Helper\ServerUrlHelper;
 use Mezzio\Helper\UrlHelper;
 use Mezzio\Session\SessionMiddleware;
 use Mezzio\Template\TemplateRendererInterface;
@@ -39,6 +40,7 @@ public function __construct(
         UrlHelper $urlHelper,
         LoggerInterface $logger,
         private OneLoginService $authenticateOneLoginService,
+        private ServerUrlHelper $serverUrlHelper,
     ) {
         parent::__construct($renderer, $urlHelper, $logger);
     }
@@ -48,12 +50,20 @@ public function handle(ServerRequestInterface $request): ResponseInterface
         $form = new OneLoginForm($this->getCsrfGuard($request));
 
         if ($request->getMethod() === 'POST') {
-            $url      = $this->urlHelper->generate();
-            $uiLocale = (str_contains($url, '/cy/') ? 'cy' : 'en');
-            $result   = $this->authenticateOneLoginService->authenticate($uiLocale);
+            $signInLink = $this->serverUrlHelper->generate($this->urlHelper->generate('auth-redirect'));
+            $uiLocale   = \Locale::getPrimaryLanguage($request->getAttribute('locale'));
+
+            if ($uiLocale === 'cy') {
+                $signInLink = str_replace('/cy', '', $signInLink);
+            }
+
+            $result            = $this->authenticateOneLoginService->authenticate($uiLocale, $signInLink);
+            $result['customs'] = ['ui_locale' => $uiLocale];
+
             $this
                 ->getSession($request, SessionMiddleware::SESSION_ATTRIBUTE)
                 ?->set(self::OIDC_AUTH_INTERFACE, AuthSession::fromArray($result));
+
             return new RedirectResponse($result['url']);
         }
 

diff --git a/service-front/app/src/Common/src/Service/OneLogin/OneLoginService.php b/service-front/app/src/Common/src/Service/OneLogin/OneLoginService.php
@@ -12,10 +12,11 @@ public function __construct(private ApiClient $apiClient)
     {
     }
 
-    public function authenticate(string $uiLocale): ?array
+    public function authenticate(string $uiLocale, string $redirectUrl): ?array
     {
         return $this->apiClient->httpGet('/v1/auth-one-login', [
-            'ui_locale' => $uiLocale,
+            'ui_locale'    => $uiLocale,
+            'redirect_url' => $redirectUrl,
         ]);
     }
 }
diff --git a/service-front/app/test/CommonTest/Service/OneLogin/OneLoginServiceTest.php b/service-front/app/test/CommonTest/Service/OneLogin/OneLoginServiceTest.php
@@ -18,12 +18,13 @@ public function can_get_authentication_request_uri(): void
     {
         $state = 'STATE';
         $nonce = 'aEwkamaos5B';
+        $redirect = 'FAKE_REDIRECT';
         $uri   = '/authorize?response_type=code
             &scope=YOUR_SCOPES
             &client_id=YOUR_CLIENT_ID
             &state=' . $state .
-            '&redirect_uri=YOUR_REDIRECT_URI
-            &nonce=' . $nonce .
+            '&redirect_uri=' . $redirect .
+            '&nonce=' . $nonce .
             '&vtr=["Cl.Cm"]
             &ui_locales=en';
 
@@ -34,11 +35,12 @@ public function can_get_authentication_request_uri(): void
                 '/v1/auth-one-login',
                 [
                     'ui_locale' => 'en',
+                    'redirect_url' => $redirect,
                 ]
             )->willReturn(['state' => $state, 'nonce' => $nonce, 'url' => $uri]);
 
         $oneLoginService = new OneLoginService($apiClientProphecy->reveal());
-        $response        = $oneLoginService->authenticate('en');
+        $response        = $oneLoginService->authenticate('en', $redirect);
         $this->assertEquals(['state' => $state, 'nonce' => $nonce, 'url' => $uri], $response);
     }
 }