Commit c08142c0 authored by Mugesh S's avatar Mugesh S
Browse files

Improve login and two-factor enrollment handling

parent bc914ebb
Loading
Loading
Loading
Loading
Loading
+11 −8
Changes for README.md: 11 added lines, 8 removed lines.
Original line number Diff line number Diff line
@@ -87,8 +87,9 @@ drush en auth_login_plus
     themselves, whether backup codes are issued, how many, whether the
     emailed self-service reset link is offered, and where to redirect after
     a successful challenge.
   - **Enforcement** — force any user without 2FA configured to the setup
     page, and the message shown when that happens.
   - **Enforcement** — force any user without 2FA enabled (never enrolled,
     or enrolled and later disabled) to the setup page, and the message
     shown when that happens.
3. Set a permanent [encryption key](#encryption-key) in `settings.php` before
   going to production.
4. Grant the `manage user 2fa` permission to whichever role should be able to
@@ -127,11 +128,12 @@ auto-generated fallback key but never touches a key supplied via
  visitor is put into a pending state (tracked server-side, not in a cookie)
  and sent to the OTP challenge (`/2fa/login`). The Drupal session is only
  granted once the code is verified.
- If 2FA is enforced sitewide and the account has never configured it, the
  visitor is instead sent to first-time enrollment (`/user/{uid}/2fa-plus`)
  as part of the same pending pre-login state.
- Once authenticated, if enforcement is on and the user still hasn't
  configured 2FA, every request to a route an anonymous visitor couldn't
- If 2FA is enforced sitewide and the account does not have it enabled
  (never configured, or configured and later disabled), the visitor is
  instead sent to enrollment (`/user/{uid}/2fa-plus`) as part of the same
  pending pre-login state.
- Once authenticated, if enforcement is on and the user still doesn't have
  2FA enabled, every request to a route an anonymous visitor couldn't
  reach anyway (i.e. anything actually gated behind a permission) redirects
  to the setup page until they do.
- The JSON/REST login endpoint (`user.login.http`) is routed through a
@@ -174,7 +176,8 @@ module.
- **Enable/disable**: if self-service is allowed, use the checkbox on your
  2FA page. Turning 2FA on/off (when it changes state) requires confirming
  with a current code, unless you're acting as an admin via
  `manage user 2fa`.
  `manage user 2fa`. While enforcement is on, users without the bypass
  permission cannot turn 2FA off for themselves.
- **Set up a new authenticator**: use "Set up a new authenticator" on your
  2FA page. Your old authenticator keeps working until the new one is
  confirmed with a code.
+2 −0
Changes for auth_login_plus.services.yml: 2 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -48,6 +48,7 @@ services:
      - '@config.factory'
      - '@datetime.time'
      - '@request_stack'
      - '@current_user'

  auth_login_plus.own_settings_access_check:
    class: Drupal\auth_login_plus\Access\OwnSettingsAccessCheck
@@ -78,3 +79,4 @@ services:
  # can resolve these for Drupal\auth_login_plus\Drush\Commands\AuthLoginPlusCommands.
  Drupal\auth_login_plus\Service\SecretStorageInterface: '@auth_login_plus.secret_storage'
  Drupal\auth_login_plus\Service\UserTotpManager: '@auth_login_plus.user_totp_manager'
  Drupal\auth_login_plus\Service\LoginChallengeManager: '@auth_login_plus.login_challenge_manager'
+6 −0
Changes for src/Controller/ResetRequestController.php: 6 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -89,6 +89,12 @@ final class ResetRequestController extends ControllerBase {
      throw new AccessDeniedHttpException();
    }

    // A link mailed while email reset was on must stop working once an
    // admin turns it off, rather than staying valid until it expires.
    if (!$this->challengeManager->isEmailResetEnabled()) {
      throw new AccessDeniedHttpException();
    }

    if (!$this->totpManager->consumeResetToken($uid, $token)) {
      $this->messenger()->addError($this->t('This reset link is invalid or has expired. Please request a new one.'));
      return $this->redirect('user.login');
+17 −0
Changes for src/Controller/RestLoginController.php: 17 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Drupal\auth_login_plus\Controller;

use Drupal\auth_login_plus\Exception\TotpFloodException;
use Drupal\auth_login_plus\Service\LoginChallengeManager;
use Drupal\auth_login_plus\Service\UserTotpManager;
use Drupal\user\Controller\UserAuthenticationController;
use Drupal\user\UserInterface;
@@ -35,6 +36,11 @@ final class RestLoginController extends UserAuthenticationController {
   */
  private RequestStack $requestStack;

  /**
   * The Authenticator Login Plus login challenge manager.
   */
  private LoginChallengeManager $challengeManager;

  /**
   * {@inheritdoc}
   */
@@ -43,6 +49,7 @@ final class RestLoginController extends UserAuthenticationController {
    $instance = parent::create($container);
    $instance->totpManager = $container->get('auth_login_plus.user_totp_manager');
    $instance->requestStack = $container->get('request_stack');
    $instance->challengeManager = $container->get('auth_login_plus.login_challenge_manager');

    return $instance;
  }
@@ -51,6 +58,13 @@ final class RestLoginController extends UserAuthenticationController {
   * {@inheritdoc}
   */
  protected function userLoginFinalize(UserInterface $user): void {
    // Enrollment needs the browser-based setup page, which this endpoint
    // can't offer, so refuse the session rather than grant one that only
    // LoginEnforcementSubscriber would later confine.
    if ($this->challengeManager->isEnrollmentRequired($user)) {
      throw new AccessDeniedHttpException('Two-factor authentication must be set up for this account before logging in here. Log in through the website to set it up.');
    }

    if (!$this->totpManager->isGloballyEnabled() || !$this->totpManager->isEnabled((int) $user->id())) {
      parent::userLoginFinalize($user);
      return;
@@ -79,6 +93,9 @@ final class RestLoginController extends UserAuthenticationController {
      throw new AccessDeniedHttpException('Invalid or expired two-factor authentication code.');
    }

    // Otherwise AuthLoginPlusHooks::userLogin() would treat this as a login
    // that skipped 2FA.
    $this->challengeManager->allowVerifiedLogin($user);
    parent::userLoginFinalize($user);
  }

+4 −2
Changes for src/EventSubscriber/LoginEnforcementSubscriber.php: 4 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -164,7 +164,7 @@ final class LoginEnforcementSubscriber implements EventSubscriberInterface {
  }

  /**
   * Redirects to the enrollment page if 2FA is enforced but not configured.
   * Redirects to the enrollment page if 2FA is enforced but not enabled.
   */
  private function enforceEnrollment(RequestEvent $event): void {
    $config = $this->configFactory->get('auth_login_plus.settings');
@@ -173,7 +173,9 @@ final class LoginEnforcementSubscriber implements EventSubscriberInterface {
    }

    $uid = (int) $this->currentUser->id();
    if ($this->totpManager->exists($uid)) {
    // Enabled, not merely enrolled: see
    // LoginChallengeManager::isEnrollmentRequired().
    if ($this->totpManager->isEnabled($uid)) {
      return;
    }

Loading