Require SMS recovery token authentication

When recovering a SF token using your recovery token, the SMS option did
not require an SMS authentication. That was fixed in this PR

During registration, the SMS auth step is not required as you just
registered the SMS recovery token in that case. And that is enough proof
of possession at that point. But during recovery of a SF token, you are
required to prove possession of your recovery token.

For the safe-store RT that would already work.

See: https://www.pivotaltracker.com/story/show/185099092

src/Surfnet/StepupSelfService/SelfServiceBundle/Command/SendRecoveryTokenSmsAuthenticationChallengeCommand.php

@@ -0,0 +1,54 @@
+<?php
+
+/**
+ * Copyright 2022 SURFnet bv
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace Surfnet\StepupSelfService\SelfServiceBundle\Command;
+
+use Surfnet\StepupBundle\Value\PhoneNumber\InternationalPhoneNumber;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\SmsRecoveryTokenService;
+
+/**
+ * @SuppressWarnings(PHPMD.LongClassName)
+ */
+class SendRecoveryTokenSmsAuthenticationChallengeCommand implements SendSmsChallengeCommandInterface
+{
+    /**
+     * @var InternationalPhoneNumber
+     */
+    public $identifier;
+
+    /**
+     * The requesting identity's ID (not name ID).
+     *
+     * @var string
+     */
+    public $identity;
+
+    /**
+     * The requesting identity's institution.
+     *
+     * @var string
+     */
+    public $institution;
+
+    /**
+     * An arbitrary token id, not recorded in Middleware.
+     * This is used to do a preliminary proof of phone possession.
+     * @var string
+     */
+    public $recoveryTokenId = SmsRecoveryTokenService::REGISTRATION_RECOVERY_TOKEN_ID;
+}

src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/RecoveryTokenControllerTrait.php

@@ -128,6 +128,9 @@ private function handleSmsProofOfPossession(
         if ($form->isSubmitted() && $form->isValid()) {
             $result = $this->smsService->provePossession($command);
             if ($result->isSuccessful()) {
+                $this->smsService->forgetRecoveryTokenState();
+                $this->smsService->tokenCreatedDuringSecondFactorRegistration();
+
                 $this->smsService->clearSmsVerificationState(SmsRecoveryTokenService::REGISTRATION_RECOVERY_TOKEN_ID);
                 $urlParameter = [];
                 if (isset($secondFactorId)) {

src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/SelfAssertedTokensController.php

@@ -20,11 +20,15 @@
 
 use Psr\Log\LoggerInterface;
 use Surfnet\StepupBundle\Service\LoaResolutionService;
+use Surfnet\StepupBundle\Value\PhoneNumber\InternationalPhoneNumber;
 use Surfnet\StepupSelfService\SelfServiceBundle\Command\PromiseSafeStorePossessionCommand;
 use Surfnet\StepupSelfService\SelfServiceBundle\Command\SafeStoreAuthenticationCommand;
 use Surfnet\StepupSelfService\SelfServiceBundle\Command\SelfAssertedTokenRegistrationCommand;
+use Surfnet\StepupSelfService\SelfServiceBundle\Command\SendRecoveryTokenSmsAuthenticationChallengeCommand;
+use Surfnet\StepupSelfService\SelfServiceBundle\Command\VerifySmsRecoveryTokenChallengeCommand;
 use Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\AuthenticateSafeStoreType;
 use Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\PromiseSafeStorePossessionType;
+use Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\VerifySmsChallengeType;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\SecondFactorService;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\AuthenticationRequestFactory;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\RecoveryTokenService;
@@ -168,20 +172,35 @@ public function selfAssertedTokenRegistrationRecoveryTokenAction(
 
         switch ($token->type) {
             case "sms":
-                $secondFactor = $this->secondFactorService->findOneVerified($secondFactorId);
+                if ($this->smsService->wasTokenCreatedDuringSecondFactorRegistration()) {
+                    // Forget we created the recovery token during token registration. Next time Identity
+                    // must fill its password.
+                    $this->smsService->forgetTokenCreatedDuringSecondFactorRegistration();
+                    $secondFactor = $this->secondFactorService->findOneVerified($secondFactorId);
 
-                $command = new SelfAssertedTokenRegistrationCommand();
-                $command->identity = $this->getIdentity();
-                $command->secondFactor = $secondFactor;
-                $command->recoveryTokenId = $recoveryTokenId;
+                    $command = new SelfAssertedTokenRegistrationCommand();
+                    $command->identity = $this->getIdentity();
+                    $command->secondFactor = $secondFactor;
+                    $command->recoveryTokenId = $recoveryTokenId;
 
-                if ($this->secondFactorService->registerSelfAssertedToken($command)) {
-                    $this->addFlash('success', 'ss.self_asserted_tokens.second_factor.alert.successful');
-                } else {
-                    $this->addFlash('error', 'ss.self_asserted_tokens.second_factor.alert.failed');
+                    if ($this->secondFactorService->registerSelfAssertedToken($command)) {
+                        $this->addFlash('success', 'ss.self_asserted_tokens.second_factor.alert.successful');
+                    } else {
+                        $this->addFlash('error', 'ss.self_asserted_tokens.second_factor.alert.failed');
+                    }
+                    return $this->redirectToRoute('ss_second_factor_list');
                 }
-
-                return $this->redirectToRoute('ss_second_factor_list');
+                $number = InternationalPhoneNumber::fromStringFormat($token->identifier);
+                $command = new SendRecoveryTokenSmsAuthenticationChallengeCommand();
+                $command->identifier = $number;
+                $command->institution = $identity->institution;
+                $command->recoveryTokenId = $recoveryTokenId;
+                $command->identity = $identity->id;
+                $this->smsService->authenticate($command);
+                return $this->redirectToRoute(
+                    'ss_second_factor_self_asserted_tokens_recovery_token_sms',
+                    ['secondFactorId' => $secondFactorId, 'recoveryTokenId' => $recoveryTokenId]
+                );
             case "safe-store":
                 // No authentication of the safe store token is required if created during SF token registration
                 if ($this->safeStoreService->wasSafeStoreTokenCreatedDuringSecondFactorRegistration()) {
@@ -247,6 +266,75 @@ public function newRecoveryTokenAction($secondFactorId)
         );
     }
 
+    /**
+     * Self-asserted token registration: Authenticate a SMS recovery token
+     *
+     * The previous action (selfAssertedTokenRegistrationRecoveryTokenAction)
+     * sent the Identity a OTP via SMS. The Identity must reproduce that OTP
+     * in this action. Proving possession of the recovery token.
+     */
+    public function selfAssertedTokenRecoveryTokenSmsAuthenticationAction(
+        Request $request,
+        string $secondFactorId,
+        string $recoveryTokenId
+    ): Response {
+        $identity = $this->getIdentity();
+        $this->assertSecondFactorInPossession($secondFactorId, $identity);
+        $this->assertRecoveryTokenInPossession($recoveryTokenId, $identity);
+
+        // Then render the authentication (proof of possession screen
+        if (!$this->smsService->hasSmsVerificationState($recoveryTokenId)) {
+            $this->get('session')->getFlashBag()->add('notice', 'ss.registration.sms.alert.no_verification_state');
+            return $this->redirectToRoute(
+                'ss_second_factor_self_asserted_tokens',
+                ['secondFactorId' => $secondFactorId]
+            );
+        }
+
+        $secondFactor = $this->secondFactorService->findOneVerified($secondFactorId);
+
+        $command = new VerifySmsRecoveryTokenChallengeCommand();
+        $command->identity = $identity->id;
+        $command->resendRouteParameters = ['secondFactorId' => $secondFactorId, 'recoveryTokenId' => $recoveryTokenId];
+
+        $form = $this->createForm(VerifySmsChallengeType::class, $command)->handleRequest($request);
+
+        if ($form->isSubmitted() && $form->isValid()) {
+            $command->recoveryTokenId = $recoveryTokenId;
+            $result = $this->smsService->verifyAuthentication($command);
+            if ($result->authenticated()) {
+                $this->smsService->clearSmsVerificationState($recoveryTokenId);
+
+                $command = new SelfAssertedTokenRegistrationCommand();
+                $command->identity = $this->getIdentity();
+                $command->secondFactor = $secondFactor;
+                $command->recoveryTokenId = $recoveryTokenId;
+
+                if ($this->secondFactorService->registerSelfAssertedToken($command)) {
+                    $this->addFlash('success', 'ss.self_asserted_tokens.second_factor.alert.successful');
+                } else {
+                    $this->addFlash('error', 'ss.self_asserted_tokens.second_factor.alert.failed');
+                }
+
+                return $this->redirectToRoute('ss_second_factor_list');
+            } elseif ($result->wasIncorrectChallengeResponseGiven()) {
+                $this->addFlash('error', 'ss.prove_phone_possession.incorrect_challenge_response');
+            } elseif ($result->hasChallengeExpired()) {
+                $this->addFlash('error', 'ss.prove_phone_possession.challenge_expired');
+            } elseif ($result->wereTooManyAttemptsMade()) {
+                $this->addFlash('error', 'ss.prove_phone_possession.too_many_attempts');
+            } else {
+                $this->addFlash('error', 'ss.prove_phone_possession.proof_of_possession_failed');
+            }
+        }
+        return $this->render(
+            '@SurfnetStepupSelfServiceSelfService/registration/self_asserted_tokens/registration_sms_prove_possession.html.twig',
+            [
+                'form' => $form->createView(),
+            ]
+        );
+    }
+
     /**
      * Self-asserted token registration: Create Recovery Token (safe-store)
      *

src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/config/routing.yml

@@ -70,6 +70,11 @@ ss_second_factor_self_asserted_tokens_recovery_token:
     methods:  [GET,POST]
     defaults: { _controller: SurfnetStepupSelfServiceSelfServiceBundle:SelfAssertedTokens:selfAssertedTokenRegistrationRecoveryToken }
 
+ss_second_factor_self_asserted_tokens_recovery_token_sms:
+    path:     /second-factor/{secondFactorId}/self-asserted-token-registration/{recoveryTokenId}/authenticate
+    methods:  [GET,POST]
+    defaults: { _controller: SurfnetStepupSelfServiceSelfServiceBundle:SelfAssertedTokens:selfAssertedTokenRecoveryTokenSmsAuthentication }
+
 ss_second_factor_new_recovery_token:
     path:     /second-factor/{secondFactorId}/new-recovery-token
     methods:  [GET]

src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/config/services.yml

@@ -120,6 +120,7 @@ services:
             - '@Surfnet\StepupBundle\Service\SmsRecoveryTokenService'
             - "@translator"
             - "@surfnet_stepup_self_service_self_service.service.command"
+            - '@Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\RecoveryTokenState'
 
     surfnet_stepup_self_service_self_service.service.gssf:
         class: Surfnet\StepupSelfService\SelfServiceBundle\Service\GssfService
@@ -162,17 +163,13 @@ services:
             - '@Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\RecoveryTokenConfig'
             - '@logger'
 
-    Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\SafeStoreState:
-        arguments:
-            - '@session'
-
     Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\RecoveryTokenState:
         arguments:
             - '@session'
 
     Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\SafeStoreService:
         arguments:
-            - '@Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\SafeStoreState'
+            - '@Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\RecoveryTokenState'
             - '@surfnet_stepup_self_service_self_service.service.command'
 
     Surfnet\StepupSelfService\SelfServiceBundle\Service\AuthorizationService:

src/Surfnet/StepupSelfService/SelfServiceBundle/Service/SecondFactorService.php

@@ -126,8 +126,7 @@ public function registerSelfAssertedToken(SelfAssertedTokenRegistrationCommand $
         $apiCommand->authorityId = $command->identity->id;
         $apiCommand->authoringRecoveryTokenId = $command->recoveryTokenId;
 
-        $result = $this->commandService->execute($apiCommand);
-        return $result->isSuccessful();
+        return $this->commandService->execute($apiCommand)->isSuccessful();
     }
 
     /**

src/Surfnet/StepupSelfService/SelfServiceBundle/Service/SelfAssertedTokens/RecoveryTokenService.php

@@ -131,6 +131,9 @@ public function getAvailableTokens(Identity $identity, VerifiedSecondFactor $sec
                 unset($tokens['sms']);
             }
         }
+        if (empty($tokens)) {
+            $this->logger->info('No recovery tokens are available for second factor registration');
+        }
         return $tokens;
     }
 

src/Surfnet/StepupSelfService/SelfServiceBundle/Service/SelfAssertedTokens/RecoveryTokenState.php

@@ -19,6 +19,8 @@
 namespace Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens;
 
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\Dto\ReturnTo;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\Dto\SafeStoreSecret;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\Exception\SafeStoreSecretNotFoundException;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 
 /**
@@ -52,6 +54,7 @@ class RecoveryTokenState
 
     public const RECOVERY_TOKEN_RETURN_TO_CREATE_SMS = 'ss_recovery_token_sms';
 
+    private const SAFE_STORE_SESSION_NAME = 'safe_store_secret';
 
     public function __construct(SessionInterface $session)
     {
@@ -71,7 +74,25 @@ public function wasRecoveryTokenCreatedDuringSecondFactorRegistration(): bool
         return false;
     }
 
-    public function forgetSafeStoreTokenCreatedDuringSecondFactorRegistration(): void
+    public function retrieveSecret(): SafeStoreSecret
+    {
+        if ($this->session->has(self::SAFE_STORE_SESSION_NAME)) {
+            return $this->session->get(self::SAFE_STORE_SESSION_NAME);
+        }
+        throw new SafeStoreSecretNotFoundException('Unable to retrieve SafeStore secret, it was not found in state');
+    }
+
+    public function store(SafeStoreSecret $secret): void
+    {
+        $this->session->set(self::SAFE_STORE_SESSION_NAME, $secret);
+    }
+
+    public function forget(): void
+    {
+        $this->session->remove(self::SAFE_STORE_SESSION_NAME);
+    }
+
+    public function forgetTokenCreatedDuringSecondFactorRegistration(): void
     {
         $this->session->remove(self::RECOVERY_TOKEN_REGISTRATION_IDENTIFIER);
     }

src/Surfnet/StepupSelfService/SelfServiceBundle/Service/SelfAssertedTokens/SafeStoreService.php

@@ -31,7 +31,7 @@
 class SafeStoreService
 {
     /**
-     * @var SafeStoreState
+     * @var RecoveryTokenState
      */
     private $stateStore;
 
@@ -40,7 +40,7 @@ class SafeStoreService
      */
     private $commandService;
 
-    public function __construct(SafeStoreState $stateStore, CommandService $commandService)
+    public function __construct(RecoveryTokenState $stateStore, CommandService $commandService)
     {
         $this->stateStore = $stateStore;
         $this->commandService = $commandService;
@@ -79,7 +79,7 @@ public function revokeRecoveryToken(RevokeRecoveryTokenCommand $command): Execut
 
     public function wasSafeStoreTokenCreatedDuringSecondFactorRegistration(): bool
     {
-        return $this->stateStore->wasSafeStoreTokenCreatedDuringSecondFactorRegistration();
+        return $this->stateStore->wasRecoveryTokenCreatedDuringSecondFactorRegistration();
     }
 
     /**
@@ -92,6 +92,6 @@ public function authenticate(string $secret, string $passwordHash)
 
     public function forgetSafeStoreTokenCreatedDuringSecondFactorRegistration(): void
     {
-        $this->stateStore->forgetSafeStoreTokenCreatedDuringSecondFactorRegistration();
+        $this->stateStore->forgetTokenCreatedDuringSecondFactorRegistration();
     }
 }