Skip to content

[5.8] Send LogoutOtherDevices event when request is made.#27865

Closed
crynobone wants to merge 1 commit into
laravel:5.8from
crynobone:logout-other-devices-event
Closed

[5.8] Send LogoutOtherDevices event when request is made.#27865
crynobone wants to merge 1 commit into
laravel:5.8from
crynobone:logout-other-devices-event

Conversation

@crynobone

Copy link
Copy Markdown
Member

This would allow developers to manages other authentications to react to this request
such as Passport, where the application may choose to revoke all users access_token etc.

Signed-off-by: Mior Muhammad Zaki crynobone@gmail.com

This would allow developers to manages other authentications to react to this request
such as `Passport`, where the application may choose to revoke all users access_token etc.

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
@crynobone crynobone force-pushed the logout-other-devices-event branch from 5c5f5b0 to 7e6c443 Compare March 12, 2019 22:53
protected function fireLogoutOtherDevicesEvent($user)
{
if (isset($this->events)) {
$this->events->dispatch(new Events\LogoutOtherDevices(

@laurencei laurencei Mar 12, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the namespace be imported at the top - then just new LogoutOtherDevices(... here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just following the other existing fire{EventName}Method structure in the class.

* @param \Illuminate\Contracts\Auth\Authenticatable $user
* @return void
*/
public function __construct($guard, $user)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason that the user parameter here and in the fireLogoutOtherDevicesEvent method isn't actually typehinted?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the existing Auth event has a type-hint. If you want that feel free to make a different PR to suggest those changes. I don't want to bloat this PR to add huge changes just to be rejected.

@taylorotwell

Copy link
Copy Markdown
Member

Merged.

@crynobone crynobone deleted the logout-other-devices-event branch September 14, 2019 06:41
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

Successfully merging this pull request may close these issues.

4 participants