Skip to content

Commit

Permalink
REFACTOR: Only use Session and Middleware for 2FA
Browse files Browse the repository at this point in the history
  • Loading branch information
JamesAlias committed May 2, 2023
1 parent 4ead71e commit 773c3b9
Show file tree
Hide file tree
Showing 15 changed files with 242 additions and 321 deletions.
22 changes: 0 additions & 22 deletions Classes/Controller/AuthenticationController.php

This file was deleted.

88 changes: 66 additions & 22 deletions Classes/Controller/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@
use Neos\Flow\Configuration\ConfigurationManager;
use Neos\Flow\Mvc\Controller\ActionController;
use Neos\Flow\Mvc\FlashMessage\FlashMessageService;
use Neos\Flow\Security\Authentication\AuthenticationManagerInterface;
use Neos\Flow\Security\Account;
use Neos\Flow\Security\Context as SecurityContext;
use Neos\Flow\Session\SessionManagerInterface;
use Neos\Fusion\View\FusionView;
use Neos\Neos\Domain\Repository\DomainRepository;
use Neos\Neos\Domain\Repository\SiteRepository;
use Sandstorm\NeosTwoFactorAuthentication\Domain\Model\SecondFactor;
use Sandstorm\NeosTwoFactorAuthentication\Domain\Repository\SecondFactorRepository;
use Sandstorm\NeosTwoFactorAuthentication\Security\Authentication\Token\UsernameAndPasswordWithSecondFactor;
use Sandstorm\NeosTwoFactorAuthentication\Http\Middleware\SecondFactorMiddleware;
use Sandstorm\NeosTwoFactorAuthentication\Service\TOTPService;

class LoginController extends ActionController
Expand All @@ -36,12 +37,6 @@ class LoginController extends ActionController
*/
protected $securityContext;

/**
* @var AuthenticationManagerInterface
* @Flow\Inject
*/
protected $authenticationManager;

/**
* @var DomainRepository
* @Flow\Inject
Expand All @@ -66,13 +61,19 @@ class LoginController extends ActionController
*/
protected $secondFactorRepository;

/**
* @Flow\Inject(lazy=false)
* @var SessionManagerInterface
*/
protected $sessionManager;

/**
* This action decides which tokens are already authenticated
* and decides which is next to authenticate
*
* ATTENTION: this code is copied from the Neos.Neos:LoginController
*/
public function askForSecondFactorAction(?string $username = null, bool $unauthorized = false)
public function askForSecondFactorAction(?string $username = null)
{
$currentDomain = $this->domainRepository->findOneByActiveRequest();
$currentSite = $currentDomain !== null ? $currentDomain->getSite() : $this->siteRepository->findDefault();
Expand All @@ -83,6 +84,51 @@ public function askForSecondFactorAction(?string $username = null, bool $unautho
'site' => $currentSite,
'flashMessages' => $this->flashMessageService->getFlashMessageContainerForRequest($this->request)->getMessagesAndFlush(),
]);

// TODO: should we safe redirect to original request?
}

public function checkOtpAction(string $otp)
{
$account = $this->securityContext->getAccount();

$isValidOtp = $this->enteredTokenMatchesAnySecondFactor($otp, $account);

// WHY: We need to check the OTP here and set the authentication status on the Session Object of 2FA package
// see Sandstorm/NeosTwoFactor
if ($isValidOtp) {
$this->sessionManager->getCurrentSession()->putData(
SecondFactorMiddleware::SESSION_OBJECT_ID,
[SecondFactorMiddleware::SESSION_OBJECT_AUTH_STATUS => SecondFactorMiddleware::SECOND_FACTOR_AUTHENTICATED]
);
} else {
// FIXME: not visible in View!
$this->addFlashMessage('Invalid otp!', 'Error', Message::SEVERITY_ERROR);
}

// TODO: should we safe redirect to original request?
$this->redirect('index', 'Backend\Backend', 'Neos.Neos');
}

/**
* Check if the given token matches any registered second factor
*
* @param string $enteredSecondFactor
* @param Account $account
* @return bool
*/
private function enteredTokenMatchesAnySecondFactor(string $enteredSecondFactor, Account $account): bool
{
/** @var SecondFactor[] $secondFactors */
$secondFactors = $this->secondFactorRepository->findByAccount($account);
foreach ($secondFactors as $secondFactor) {
$isValid = TOTPService::checkIfOtpIsValid($secondFactor->getSecret(), $enteredSecondFactor);
if ($isValid) {
return true;
}
}

return false;
}

/**
Expand All @@ -91,7 +137,7 @@ public function askForSecondFactorAction(?string $username = null, bool $unautho
*
* ATTENTION: this code is copied from the Neos.Neos:LoginController
*/
public function setupSecondFactorAction(?string $username = null, bool $unauthorized = false)
public function setupSecondFactorAction(?string $username = null)
{
$otp = TOTPService::generateNewTotp();
$secret = $otp->getSecret();
Expand All @@ -101,11 +147,7 @@ public function setupSecondFactorAction(?string $username = null, bool $unauthor
$currentSiteName = $currentSite->getName();
$urlEncodedSiteName = urlencode($currentSiteName);

$secondFactorAuthenticationTokens = $this->securityContext->getAuthenticationTokensOfType(UsernameAndPasswordWithSecondFactor::class);
// TODO: error if empty
// TODO: check token status (is authentication successful)

$userIdentifier = $secondFactorAuthenticationTokens[0]->getAccount()->getAccountIdentifier();
$userIdentifier = $this->securityContext->getAccount()->getAccountIdentifier();

$oauthData = "otpauth://totp/$userIdentifier?secret=$secret&period=30&issuer=$urlEncodedSiteName";
$qrCode = (new QRCode(new QROptions([
Expand All @@ -131,19 +173,14 @@ public function setupSecondFactorAction(?string $username = null, bool $unauthor
*/
public function createSecondFactorAction(string $secret, string $secondFactorFromApp)
{
// TODO: validate Token
$isValid = TOTPService::checkIfOtpIsValid($secret, $secondFactorFromApp);

if (!$isValid) {
$this->addFlashMessage('Submitted OTP was not correct', '', Message::SEVERITY_WARNING);
$this->redirect('setupSecondFactor');
}

$secondFactorAuthenticationTokens = $this->securityContext->getAuthenticationTokensOfType(UsernameAndPasswordWithSecondFactor::class);
// TODO: error if empty
// TODO: check token status (is authentication successful)

$account = $secondFactorAuthenticationTokens[0]->getAccount();
$account = $this->securityContext->getAccount();

$secondFactor = new SecondFactor();
$secondFactor->setAccount($account);
Expand All @@ -153,7 +190,14 @@ public function createSecondFactorAction(string $secret, string $secondFactorFro
$this->persistenceManager->persistAll();

$this->addFlashMessage('Successfully created otp');
// TODO: login because 2fa is set up with valid otp or force re-login with new otp

// TODO: Discuss: we could skip this to force the user to enter a otp again directly after setup
$this->sessionManager->getCurrentSession()->putData(
SecondFactorMiddleware::SESSION_OBJECT_ID,
[SecondFactorMiddleware::SESSION_OBJECT_AUTH_STATUS => SecondFactorMiddleware::SECOND_FACTOR_AUTHENTICATED]
);

// TODO: should we safe redirect to original request?
$this->redirect('index', 'Backend\Backend', 'Neos.Neos');
}

Expand Down
12 changes: 0 additions & 12 deletions Classes/Error/SecondFactorEnforcedSetupException.php

This file was deleted.

12 changes: 0 additions & 12 deletions Classes/Error/SecondFactorRequiredException.php

This file was deleted.

162 changes: 162 additions & 0 deletions Classes/Http/Middleware/SecondFactorMiddleware.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
<?php

namespace Sandstorm\NeosTwoFactorAuthentication\Http\Middleware;

use GuzzleHttp\Psr7\Response;
use Neos\Flow\Security\Context as SecurityContext;
use Neos\Flow\Session\SessionManagerInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Psr\Log\LoggerInterface;
use Sandstorm\NeosTwoFactorAuthentication\Domain\Repository\SecondFactorRepository;
use Neos\Flow\Annotations as Flow;

class SecondFactorMiddleware implements MiddlewareInterface
{
const SESSION_OBJECT_ID = 'Sandstorm/NeosTwoFactorAuthentication';
const SESSION_OBJECT_AUTH_STATUS = 'authenticationStatus';

const SECOND_FACTOR_AUTHENTICATION_NEEDED = 'SECOND_FACTOR_AUTHENTICATION_NEEDED';
const SECOND_FACTOR_AUTHENTICATED = 'SECOND_FACTOR_AUTHENTICATED';

/**
* TODO: Why lazy false?
* @Flow\Inject(lazy=false)
* @var SessionManagerInterface
*/
protected $sessionManager;

/**
* @Flow\Inject(lazy=false)
* @var SecurityContext
*/
protected $securityContext;

/**
* @Flow\Inject(lazy=false)
* @var SecondFactorRepository
*/
protected $secondFactorRepository;

/**
* @Flow\InjectConfiguration(path="enforceTwoFactorAuthentication")
* @var bool
*/
protected $enforceTwoFactorAuthentication;

/**
* @Flow\Inject(name="Neos.Flow:SecurityLogger")
* @var LoggerInterface
*/
protected $securityLogger;

// TODO: break up into smaller functions to remove complexity
public function process(ServerRequestInterface $request, RequestHandlerInterface $next): ResponseInterface
{
$authenticationTokens = $this->securityContext->getAuthenticationTokens();

if (empty($authenticationTokens)) {
$this->securityLogger->info(
'Sandstorm/NeosTwoFactorAuthentication: ' .
'No authentication tokens found, skipping second factor'
);
return $next->handle($request);
}

// WHY: we currently only support 'Neos.Neos:Backend' provider (which ever is used) because the
// second factor feature is currently only build for Neos Editors use case
if (!array_key_exists('Neos.Neos:Backend', $authenticationTokens)) {
$this->securityLogger->error(
'Sandstorm/NeosTwoFactorAuthentication: ' .
'No authentication token for "Neos.Neos:Backend" found, skipping second factor'
);
return $next->handle($request);
}

$isAuthenticated = $authenticationTokens['Neos.Neos:Backend']->isAuthenticated();

// ignore unauthenticated requests
if (!$isAuthenticated) {
$this->securityLogger->info(
'Sandstorm/NeosTwoFactorAuthentication: ' .
'Not authenticated on "Neos.Neos:Backend" auth provider, skipping second factor'
);
return $next->handle($request);
}

$account = $this->securityContext->getAccount();

// ignore if second factor is not enabled for account and second factor is not enforced
if (
!$this->secondFactorRepository->isEnabledForAccount($account)
&& !$this->enforceTwoFactorAuthentication
) {
$this->securityLogger->debug(
'Sandstorm/NeosTwoFactorAuthentication: ' .
'Second factor not enforced and set up for account, skipping second factor'
);
return $next->handle($request);
}

// get 2FA data from session
$currentSession = $this->sessionManager->getCurrentSession();
// TODO: ValueObject/DTO
$twoFactorData = $currentSession->getData(self::SESSION_OBJECT_ID);

// if session has no 2FA data object, we initialize a default
if (empty($twoFactorData)) {
$currentSession->putData(
self::SESSION_OBJECT_ID,
[
self::SESSION_OBJECT_AUTH_STATUS => self::SECOND_FACTOR_AUTHENTICATION_NEEDED,
]
);
$twoFactorData = $currentSession->getData(self::SESSION_OBJECT_ID);
}

// already authenticated
if ($twoFactorData[self::SESSION_OBJECT_AUTH_STATUS] === self::SECOND_FACTOR_AUTHENTICATED) {
return $next->handle($request);
}

if (
$this->secondFactorRepository->isEnabledForAccount($account)
&& $twoFactorData[self::SESSION_OBJECT_AUTH_STATUS] === self::SECOND_FACTOR_AUTHENTICATION_NEEDED
) {
// TODO: discuss
// WHY: We use the request URI as part of the state
$isAskingFor2FA = str_ends_with($request->getUri()->getPath(), 'neos/two-factor-login');
if ($isAskingFor2FA) {
return $next->handle($request);
}

if ($request->getMethod() === 'POST') {
return new Response(401);
}

return new Response(303, ['Location' => '/neos/two-factor-login']);
}

if (
$this->enforceTwoFactorAuthentication &&
!$this->secondFactorRepository->isEnabledForAccount($account)
) {
// ignore if setup is in progress
$isSettingUp2FA = str_ends_with($request->getUri()->getPath(), 'neos/setup-second-factor');
if ($isSettingUp2FA) {
return $next->handle($request);
}

if ($request->getMethod() === 'POST') {
return new Response(401);
}

return new Response(303, ['Location' => '/neos/setup-second-factor']);
}

// TODO: Throw here?
die('this should not happen^^');
}
}
Loading

0 comments on commit 773c3b9

Please sign in to comment.