When using broadcasting, /broadcasting/auth returns 500 if authenticatable model that is not eloquent does not implement getKey

[tdondich]

• Laravel Version: 5.4.15 (Or any that implements Broadcasts)
• PHP Version: Any
• Database Driver & Version: MySQL

Description:

When implementing a Authenticatable for authentication, if the implementation does not also implement getKey, then usage of Event Broadcasting will fail.

The getKey method on the Authenticated user is called in PusherBroadcaster.php in method validAuthenticationResponse as well as RedisBroadcaster.php.

Steps To Reproduce:

Implement a bare bones class that implements Authenticatable then call Auth::login with this instance. Enable Broadcasting per documentation. Attempt to call join or private (to attempt to call /broadcasting/auth). Server response is 500 due to no method getKey on the Authenticated user.

[jerguslejko]

Well, if your class implements Illuminate\Auth\Authenticatable, you need to at least provide getKeyName() and getKey() methods.

I do not think it's supported to have a class which implements Authenticable and is not an eloquent model.

[tdondich]

@jerguslejko absolutely you can. We do. We utilize Propel for our model classes and we've implemented the Authenticatable contract per documentation at https://laravel.com/docs/5.4/authentication#the-authenticatable-contract . That works great. For reference, the Authenticatable contract interface documentation is here: https://laravel.com/api/5.4/Illuminate/Contracts/Auth/Authenticatable.html . There is no mention of getKey or getKeyName.

[tdondich]

And I don't know what you are talking about regarding Illuminate\Auth\Authenticatable. There is no such class/interface. You might be confused with the use alias that is inside the default User.php in the laravel/laravel package.

Anyways, my recommendation in resolving this is to replace the getKey references in the Broadcasters with getAuthIdentifier() . This is meant to be unique per Authenticatable documentation and is suitable for Broadcaster purposes.

[jerguslejko]

@tdondich oh I get you now. Yeah, you are right. I was looking at Illuminate\Auth\Authenticatable trait.

I guess calls in:
https://github.com/laravel/framework/blob/5.4/src/Illuminate/Broadcasting/Broadcasters/PusherBroadcaster.php#L69
https://github.com/laravel/framework/blob/5.4/src/Illuminate/Broadcasting/Broadcasters/RedisBroadcaster.php#L75

need to change as you're suggesting.

[tdondich]

I'll create a pull request.

[tdondich]

Thanks @jerguslejko . Appreciate the back and forth. Maybe we can merge in that PR? First time contributing to the project, so want to do it right. Let me know if I need to change anything per process.

[jerguslejko]

It's not up to us. You'll need to wait for Taylor to review the change.

[tdondich]

It's okay. I'm on a first name basis with him. Get it? Get it?

[themsaid]

Closing since the PR was merged. Thanks :)