Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require SMS recovery token authentication #290

Merged
merged 1 commit into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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)
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -32,6 +34,8 @@
* registration or not.
* 3. Remember the route where to return to after giving step up
* 4. Store the fact if step up was given for a given Recovery Token action.
*
* @SuppressWarnings(PHPMD.TooManyPublicMethods)
*/
class RecoveryTokenState
{
Expand All @@ -52,6 +56,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)
{
Expand All @@ -71,7 +76,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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
class SafeStoreService
{
/**
* @var SafeStoreState
* @var RecoveryTokenState
*/
private $stateStore;

Expand All @@ -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;
Expand Down Expand Up @@ -79,7 +79,7 @@ public function revokeRecoveryToken(RevokeRecoveryTokenCommand $command): Execut

public function wasSafeStoreTokenCreatedDuringSecondFactorRegistration(): bool
{
return $this->stateStore->wasSafeStoreTokenCreatedDuringSecondFactorRegistration();
return $this->stateStore->wasRecoveryTokenCreatedDuringSecondFactorRegistration();
}

/**
Expand All @@ -92,6 +92,6 @@ public function authenticate(string $secret, string $passwordHash)

public function forgetSafeStoreTokenCreatedDuringSecondFactorRegistration(): void
{
$this->stateStore->forgetSafeStoreTokenCreatedDuringSecondFactorRegistration();
$this->stateStore->forgetTokenCreatedDuringSecondFactorRegistration();
}
}
Loading