[5.7] Add guard to authentication events

[Propaganistas]

I'm running into a situation where I need to fire some actions after logout, but only for a particular guard.

Currently there's no straightforward way to determine which guard was responsible for firing one of the authentication events.

[deleugpn]

Why not pass the whole Guard context ($this) instead of just the name ($this->name)?
Also, this should have some tests.

[Propaganistas]

Fair point about passing the whole Guard instance. I needed to add an additional getGuardName() method in order to easily get hold of the guard's config name. In this light, since the framework only fires the events in it's SessionGuard, I thought it might be appropriate to typehint $guard as the StatefulGuard contract.

Regarding tests: I couldn't find any checking the constructor arguments of the events.

[rodrigopedra]

Passing the whole guard breaks any queued listeners as the Guards in general (SessionGuard for instance) relies on closures (see the request, provider and events properties, each 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 this PR introduces it throws an Serialization of closures is not allowed exception.

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 this PR

3. Document it as a breaking change in the upgrade guide
   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

[devcircus]

If this is really breaking in this way, it needs to be reverted. We can't say "no queued listeners" when listening for authentication events.

[rodrigopedra]

I submitted issue #25566 with steps to reproduce. Also I imagine a new issue would get more attention than a comment on a merged PR

[deleugpn]

Given your complete knowledge on the issue, I think it would be beneficial if you open a PR with the original intent as it might solve the issue while still offering the original feature.

[rodrigopedra]

@deleugpn It seems the best option to me, but it would be a breaking change on a newly released version... I will send it anyway and maybe we can decide for a better approach later

[deleugpn]

I think we can advocate that this was a major breakage because it broke the feature. I would argue in favour of the breaking change given the state of the feature.
It's even better to do it now while 5.7 is a fresh release.
Perhaps even better would be to try and write a test that prevents this type of issue from happening again.

[rodrigopedra]

Sent the #25568 PR, working on the tests now.

[rodrigopedra]

Ping @Propaganistas , please check if PR #25568 breaks your workflow

[Propaganistas]

Thanks for the heads up.