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

Lockout event is never fired #336

Closed
alperyazgan opened this issue Dec 25, 2021 · 3 comments
Closed

Lockout event is never fired #336

alperyazgan opened this issue Dec 25, 2021 · 3 comments

Comments

@alperyazgan
Copy link

alperyazgan commented Dec 25, 2021

  • Fortify Version: 1.8.6
  • Laravel Version: 8.77.1
  • PHP Version: 8.1.1

What is aimed

Enable a configurable option for the app to use either the throttling or user lockout.

What is failing

Throttling is working fine but the lockout event is never fired so never fetched, lockout option could not be setup.

Description:

With 2 Different Case Scenarios, during Authentication (with same user, wrong password) lockout event is expected to be fired but never happened;

First Scenario:
config/fortify.php has the following limiters set
'limiters' => [ 'login' => 'login', 'two-factor' => 'two-factor', ],
And inside the boot method of "FortifyServiceProvider" we have authenticateUsing and authenticateThrough methods configured and the rate limiters.

public function boot()
  {
    Fortify::viewPrefix('app.auth.');

    Fortify::createUsersUsing(CreateNewUser::class);
    Fortify::updateUserProfileInformationUsing(UpdateUserProfileInformation::class);
    Fortify::updateUserPasswordsUsing(UpdateUserPassword::class);
    Fortify::resetUserPasswordsUsing(ResetUserPassword::class);

    Fortify::authenticateUsing(function (Request $request) {
      $loginUsername = config('fortify.username');

      $user = User::where([
                            [$loginUsername, '=', strtolower($request->input($loginUsername))],
                            ['active', '=', true],
                            ['locked', '=', false],
                          ])->first();
      if ($user && Hash::check($request->password, $user->password)) return $user;
    });

    Fortify::authenticateThrough(function (Request $request) {
      return [
        EnsureLoginIsNotThrottled::class,
        AttemptToAuthenticate::class,
        PrepareAuthenticatedSession::class,
      ];
    });

    RateLimiter::for('login', function (Request $request) {
      $loginUsername = config('fortify.username');

      return Limit::perMinute(config('app-settings.max_login_attempts'))->by($request->$loginUsername . $request->ip());
    });

    RateLimiter::for('two-factor', function (Request $request) {
      return Limit::perMinute(5)->by($request->session()->get('login.id'));
    });
  }

In this scenario we can easily follow that login request goes thru AuthenticatedSessionController::loginPipe method and inside the method, first block is evaluated to be true and executed, so the EnsureLoginIsNotThrottled::handle but after too many attempts we get 429 error and get to the relevant error-page but event is never fired.

    public function handle($request, $next)
    {
        if (! $this->limiter->tooManyAttempts($request)) {
            return $next($request);
        }

        event(new Lockout($request));  <--- **This line is never reached !!!** 

        return app(LockoutResponse::class);
    }

Second Scenario:

config/fortify.php has the following limiters set
'limiters' => [ 'login' => null, 'two-factor' => null, ],
And inside the boot method of "FortifyServiceProvider" we have only authenticateUsing method configured.

public function boot()
  {
    Fortify::viewPrefix('app.auth.');

    Fortify::createUsersUsing(CreateNewUser::class);
    Fortify::updateUserProfileInformationUsing(UpdateUserProfileInformation::class);
    Fortify::updateUserPasswordsUsing(UpdateUserPassword::class);
    Fortify::resetUserPasswordsUsing(ResetUserPassword::class);

    Fortify::authenticateUsing(function (Request $request) {
      $loginUsername = config('fortify.username');

      $user = User::where([
                            [$loginUsername, '=', strtolower($request->input($loginUsername))], 
                            ['active', '=', true],
                            ['locked', '=', false],
                          ])->first();
      if ($user && Hash::check($request->password, $user->password)) return $user;
    });
  }

In this scenario we can easily follow that login request goes thru AuthenticatedSessionController::loginPipe method and third block is executed, so the EnsureLoginIsNotThrottled::handle but after too many attempts we get 429 error and get to the relevant error-page but event again is never fired.

Actually in both scenarios, during the too many attempt phase, EnsureLoginIsNotThrottled::handle could not find a chance to be executed, throttling control prevent it beforehand.

Maybe, my approach to the case is lacking, or missing something; therefore on this issue your assistance will be greatly appreciated.

Kind Regards,

@alperyazgan
Copy link
Author

alperyazgan commented Dec 31, 2021

@alperyazgan
Copy link
Author

Conclusion : Laravel framework 8.x request throttling feature prevents (basically interrupts the flow beforehand) the lockout event to be fired (which is already seemed to be a removed feature).

If the lockout option is desired it could be implemented, but not a mere use of this event type.

@brnquester
Copy link

@alperyazgan - I understand this issue is old, but the Lockout Fortify own handler is still present but never reached due to the Laravel middleware. However, with simple steps, I was able to get it working back again. This to me seems like an existing bug. The Laravel middleware should be an option that we enable or not, but never the only option. Any support in my current issue is appreciated.
#543

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants