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

[5.7] Cannot listen to Authentication events using queued listeners #25566

Closed
rodrigopedra opened this issue Sep 10, 2018 · 2 comments
Closed

Comments

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Sep 10, 2018

  • Laravel Version: 5.7.2
  • PHP Version: 7.2.9
  • Database Driver & Version: any

Description:

PR #25261 started passing the guard to some Authentication events as a constructor parameter.

Passing the whole guard breaks any queued listeners as the Guards in general relies on closures (SessionGuard for instance have the request, user provider and event dispatcher as properties, each of them have closure-based resolvers).

I have a queued listener that updates a timestamp in the user's record whenever the Login event is fired. Use it in every project (show a last logged at timestamp).

To migrate some of them to Laravel 5.7 I had to make this listener synchronous (remove the ShouldQueue interface) as with the change #25261 PR introduces an Serialization of 'Closure' is not allowed exception is thrown when the listener is queued.

I have some suggestions:

  1. Revert to the old behavior
  2. Pass only the guard name to the event, as it was originally intended in that PR
  3. Document it as a breaking change in the upgrade guide, something like:
    If you are listening for Authentication Events you should not use queued listeners anymore...

I can submit a PR for any of these alternatives as soon as we have a decision

Steps To Reproduce:

1, Create a new app:

$ laravel new test-app
$ cd test-app
$ php artisan make:auth
$ php artisan make:listener TestListener --queued --event=Illuminate\\Auth\\Events\\Login
$ # change the .env with database settings
$ php artisan migrate
  1. Register the listener on the EventServiceProvider:
// app/EventServiceProvider
protected $listen = [
    \Illuminate\Auth\Events\Login::class => [
        \App\Listeners\TestListener::class,
    ],
];
  1. Serve the application
$ php artisan serve
  1. Access the application on the default URL
  2. Register a new user.

As after user registration the Login event is fired it throws an exception at this point.

@rodrigopedra
Copy link
Contributor Author

Submitted PR #25568 that changes PR #25261 to its original intention and only pass the guard name to the event constructor.

@rodrigopedra
Copy link
Contributor Author

PR #25568 was merged

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

1 participant