Commit 34a324c0 authored by Ahmed Raza's avatar Ahmed Raza
Browse files

Code improvements and bug fixes

parent 785b7e68
Loading
Loading
Loading
Loading
+43 −4
Changes for alogin.module: 43 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -5,18 +5,30 @@
 * Contains alogin.module.
 */

use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\alogin\AuthenticatorService;
use Drupal\alogin\Event\TwoFactorLogin;
use Drupal\Core\Ajax\AjaxResponse;
use Drupal\Core\Ajax\HtmlCommand;
use Drupal\Core\Ajax\ReplaceCommand;
use Drupal\Core\Ajax\RedirectCommand;
use Drupal\alogin\AuthenticatorService;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\TempStore\PrivateTempStoreFactory;
use Drupal\Core\Url;

/**
 * Implements hook_help().
 *
 * This function provides help text for the alogin module.
 *
 * @param string $route_name
 *   The name of the route being accessed.
 * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
 *   The route match object.
 *
 * @return string
 *   The help text to be displayed.
 */
function alogin_help($route_name, RouteMatchInterface $route_match) {
  switch ($route_name) {
@@ -31,6 +43,18 @@ function alogin_help($route_name, RouteMatchInterface $route_match) {
  }
}

/**
 * Implements hook_form_alter().
 *
 * This function alters the user login form to add AJAX functionality.
 *
 * @param array $form
 *   The form structure.
 * @param \Drupal\Core\Form\FormStateInterface $form_state
 *   The current state of the form.
 * @param string $form_id
 *   The ID of the form being altered.
 */
function alogin_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  if ($form_id == 'user_login_form') {
    $form['#cache'] = ['max-age' => 0];
@@ -42,10 +66,25 @@ function alogin_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  }
}

/**
 * Ajax callback for the user login form.
 *
 * This function handles the AJAX response when a user attempts to log in.
 * It checks for errors and redirects the user accordingly.
 *
 * @param array $form
 *   The form structure.
 * @param \Drupal\Core\Form\FormStateInterface $form_state
 *   The current state of the form.
 *
 * @return \Drupal\Core\Ajax\AjaxResponse
 *   The AJAX response object.
 */
function alogin_ajax_callback(&$form, FormStateInterface $form_state) {
  $response = new AjaxResponse();
  // Validate for errors and flood control (presence of UID).
  if ($form_state->getErrors() || !$form_state->get('uid')) {
    \Drupal::logger('alogin')->info('User @name logged in successfully.');
    unset($form['#prefix']);
    unset($form['#suffix']);
    $form['status_messages'] = [
@@ -60,7 +99,7 @@ function alogin_ajax_callback(&$form, FormStateInterface $form_state) {
  $tempstorePrivate = \Drupal::service('tempstore.private');
  $tempstorePrivate->get('alogin')->set('uid', $account->id());
  $authenticator = \Drupal::service('alogin.authenticator');
  if ($authenticator->is_enabled($account->id())) {
  if ($authenticator->isEnabled($account->id())) {
    $tempstorePrivate->get('alogin')->set('uid', $account->id());
    $response->addCommand(new RedirectCommand(Url::fromRoute('alogin.two_fa_form')->toString()));
    return $response;
+4 −0
Changes for alogin.services.yml: 4 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -10,3 +10,7 @@ services:
    arguments: ['@current_user', '@alogin.authenticator', '@current_route_match', '@config.factory', '@messenger', '@entity_type.manager']
    tags:
      - { name: event_subscriber }
  alogin.route_subscriber:
    class: Drupal\alogin\Routing\AloginRouteSubscriber
    tags:
      - { name: event_subscriber }
+24 −3
Changes for src/AuthenticatorService.php: 24 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -54,6 +54,26 @@ class AuthenticatorService
        return $this->twofa->verifyKey($this->secret, $code);
    }

    /**
     * Verify a code for a specific user.
     *
     * @param string $code
     *   The MFA code to verify.
     * @param int $uid
     *   The user ID.
     *
     * @return bool
     *   TRUE if the code is valid, FALSE otherwise.
     */
    public function verifyCode($code, $uid)
    {
        $secret = $this->getSecret($uid);
        if (!$secret) {
            return FALSE;
        }
        return $this->twofa->verifyKey($secret, $code);
    }

    public function store($enable)
    {
        if ($this->exists($this->currentUser->id())) {
@@ -62,11 +82,12 @@ class AuthenticatorService
        return $this->new($enable);
    }

    public function exists()
    public function exists($uid = NULL)
    {
        $user_id = $uid ?? $this->currentUser->id();
        $exists = $this->database->select($this->table, 'a')
            ->fields('a')
            ->condition('uid', $this->currentUser->id(), '=')
            ->condition('uid', $user_id, '=')
            ->execute()
            ->fetchAssoc();
        return $exists;
@@ -101,7 +122,7 @@ class AuthenticatorService
        return $update;
    }

    public function is_enabled($uid)
    public function isEnabled($uid)
    {
        $enabled = $this->database->select($this->table, 'a')
            ->fields('a', ['enabled'])
+207 −0
Changes for src/Controller/MfaLoginController.php: 207 added lines, 0 removed lines.
Original line number Diff line number Diff line
<?php

namespace Drupal\alogin\Controller;

use Drupal\Core\Access\CsrfTokenGenerator;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Routing\RouteProviderInterface;
use Drupal\alogin\AuthenticatorService;
use Drupal\user\Controller\UserAuthenticationController;
use Drupal\user\UserAuthenticationInterface;
use Drupal\user\UserAuthInterface;
use Drupal\user\UserFloodControlInterface;
use Drupal\user\UserStorageInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\Serializer\Serializer;

/**
 * Controller for handling MFA-protected login requests.
 */
class MfaLoginController extends UserAuthenticationController implements ContainerInjectionInterface {

  /**
   * The config factory.
   *
   * @var \Drupal\Core\Config\ConfigFactoryInterface
   */
  protected $configFactory;

  /**
   * The authenticator service.
   *
   * @var \Drupal\alogin\AuthenticatorService
   */
  protected $authenticatorService;

  /**
   * The entity type manager.
   *
   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   */
  protected $entityTypeManager;

  /**
   * Constructs a MfaLoginController object.
   *
   * @param \Drupal\user\UserFloodControlInterface $user_flood_control
   *   The user flood control service.
   * @param \Drupal\user\UserStorageInterface $user_storage
   *   The user storage.
   * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token
   *   The CSRF token generator.
   * @param \Drupal\user\UserAuthenticationInterface|\Drupal\user\UserAuthInterface $user_auth
   *   The user authentication service.
   * @param \Drupal\Core\Routing\RouteProviderInterface $route_provider
   *   The route provider.
   * @param \Symfony\Component\Serializer\Serializer $serializer
   *   The serializer service.
   * @param array $serialization_formats
   *   The available serialization formats.
   * @param \Psr\Log\LoggerInterface $logger
   *   The logger service.
   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
   *   The config factory.
   * @param \Drupal\alogin\AuthenticatorService $authenticator_service
   *   The authenticator service.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager.
   */
  public function __construct(UserFloodControlInterface $user_flood_control, UserStorageInterface $user_storage, CsrfTokenGenerator $csrf_token, UserAuthenticationInterface|UserAuthInterface $user_auth, RouteProviderInterface $route_provider, Serializer $serializer, array $serialization_formats, LoggerInterface $logger, ConfigFactoryInterface $config_factory, AuthenticatorService $authenticator_service, EntityTypeManagerInterface $entity_type_manager) {
    parent::__construct($user_flood_control, $user_storage, $csrf_token, $user_auth, $route_provider, $serializer, $serialization_formats, $logger);
    $this->configFactory = $config_factory;
    $this->authenticatorService = $authenticator_service;
    $this->entityTypeManager = $entity_type_manager;
  }

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container) {
    if ($container->hasParameter('serializer.formats') && $container->has('serializer')) {
      $serializer = $container->get('serializer');
      $formats = $container->getParameter('serializer.formats');
    }
    else {
      $formats = ['json'];
      $encoders = [new \Symfony\Component\Serializer\Encoder\JsonEncoder()];
      $serializer = new \Symfony\Component\Serializer\Serializer([], $encoders);
    }

    return new static(
      $container->get('user.flood_control'),
      $container->get('entity_type.manager')->getStorage('user'),
      $container->get('csrf_token'),
      $container->get('user.auth'),
      $container->get('router.route_provider'),
      $serializer,
      $formats,
      $container->get('logger.factory')->get('user'),
      $container->get('config.factory'),
      $container->get('alogin.authenticator'),
      $container->get('entity_type.manager')
    );
  }

  /**
   * Custom login validation with MFA check.
   *
   * @param \Symfony\Component\HttpFoundation\Request $request
   *   The request object.
   *
   * @return \Symfony\Component\HttpFoundation\Response
   *   The response object.
   *
   * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
   */
  public function validateLoginRequest(Request $request) {
    $format = $this->getRequestFormat($request);
    $content = $request->getContent();

    if (empty($content)) {
      throw new BadRequestHttpException('Empty request body.');
    }

    $credentials = $this->serializer->decode($content, $format);

    if (empty($credentials['name'])) {
      throw new BadRequestHttpException('Missing credentials.name.');
    }

    if (empty($credentials['pass'])) {
      throw new BadRequestHttpException('Missing credentials.pass.');
    }

    // Load user by username.
    /** @var \Drupal\user\UserInterface[] $users */
    $users = $this->userStorage->loadByProperties(['name' => $credentials['name']]);

    if (count($users) !== 1) {
      throw new BadRequestHttpException('Sorry, unrecognized username or password.');
    }

    $user = reset($users);

    // Check if user is active.
    if (!$user->isActive()) {
      throw new AccessDeniedHttpException('The user has not been activated or is blocked.');
    }

    // Check if user has MFA configured.
    $user_has_mfa = $this->authenticatorService->exists($user->id());

    // If MFA is enabled but user doesn't have it configured, check if it's enforced.
    if (!$user_has_mfa) {
      $allow_enable_disable = $config->get('allow_enable_disable') ?? TRUE;
      $user_can_bypass = $user->hasPermission('alogin bypass enforced redirect');

      // If MFA is enforced and user can't bypass, deny access.
      if (!$allow_enable_disable && !$user_can_bypass) {
        throw new AccessDeniedHttpException('Two-factor authentication is required but not configured for this user.');
      }

      // If user can disable MFA or has bypass permission, allow normal login.
      return parent::login($request);
    }

    // If user has MFA configured, require MFA validation.
    // For API requests, we need to check if MFA token is provided.
    if (empty($credentials['mfa_token'])) {
      throw new AccessDeniedHttpException('Two-factor authentication token is required.');
    }

    // Validate the MFA token.
    $mfa_valid = $this->authenticatorService->verifyCode($credentials['mfa_token'], $user->id());

    if (!$mfa_valid) {
      throw new AccessDeniedHttpException('Invalid two-factor authentication token.');
    }

    // If MFA validation passes, proceed with normal login.
    return parent::login($request);
  }

  /**
   * Gets the format of the current request.
   *
   * @param \Symfony\Component\HttpFoundation\Request $request
   *   The current request.
   *
   * @return string
   *   The format of the request.
   */
  protected function getRequestFormat(Request $request) {
    $format = $request->getRequestFormat();
    if (!in_array($format, $this->serializerFormats)) {
      throw new BadRequestHttpException("Unrecognized format: $format.");
    }
    return $format;
  }

}
+5 −6
Changes for src/EventSubscriber/MfaRedirectSubscriber.php: 5 added lines, 6 removed lines.
Original line number Diff line number Diff line
@@ -5,7 +5,6 @@ namespace Drupal\alogin\EventSubscriber;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
// use Symfony\Contracts\EventDispatcher\Event;
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\Routing\CurrentRouteMatch;
use Drupal\Core\Session\AccountInterface;
@@ -90,7 +89,7 @@ class MfaRedirectSubscriber implements EventSubscriberInterface {
   *   In Drupal 10 this will be a \Symfony\Contracts\EventDispatcher\Event.
   */
  public function check2fa(object $event) {
    // dump($this->routeMatch->getRouteName());

    if ($this->currentUser->isAuthenticated()) {
      $account = $this->entityTypeManager->getStorage('user')->load($this->currentUser->id());
      $config = $this->configFactory->get('alogin.config');
@@ -102,13 +101,13 @@ class MfaRedirectSubscriber implements EventSubscriberInterface {
        'system.css_asset',
        'system.js_asset',
      ];
      if (
      $bypass_routes = (
        !$account->hasPermission('alogin bypass enforced redirect') &&
        !$config->get('allow_enable_disable') &&
        !$this->alogin->exists($this->currentUser->id()) &&
        !in_array($this->routeMatch->getRouteName(), $bypass_routes) &&
        $config->get('redirect')
      ) {
        !in_array($this->routeMatch->getRouteName(), $bypass_routes)
      );
      if ($bypass_routes && $config->get('redirect')) {
        $this->messenger->addMessage($config->get('redirect_message'), $config->get('message_type'), TRUE);
        $event->setResponse(
          new RedirectResponse(Url::fromRoute(
Loading