[5.4] Broadcasting Improvements #13653

leemason · 2016-05-22T20:20:31Z

The PR contains the following changes:

1. "de-vendors" and enhances the new broadcast authentication features of 5.3.

1.1. Cached socket/session ids are changed from pusher:socket:$id to broadcast:socket:$id.

1.2. Common broadcast driver functions are refactored into an external abstract class, namely auth(), check(), extractAuthParameters(), rememberSocket() and validAuthenticationResponse().

1.2.1. Specific drivers need only override validAuthenticationResponse() and/or rememberSocket() as the others are pretty general (but provided in the abstract class to prevent method not found issues, for example when testing). If not overwritten a default json response will be returned with status => success, user_id and user_info.

1.2.2 All drivers extending the abstract class (all existing core drivers) have been updated to require a container instance in there constructor, this is due to the abstract classes ability to ioc resolve classes mentioned in the next point.

This function returns the "member info" and anything else needed by a specific driver implementation. Pusher for example doesnt return anything, it does an api call, whereas Rebound (discussed further down) expects a response with the member info. Allowing this to be overwritten allows for different implementations without much hassle.

1.3. The auth('channel', function(){}) functionality has been extended to allow declaration just like other areas of laravel with the class@method syntax, or a callable. This gives the developer the choice to inject required classes via the class constructor (as it uses the ioc container to resolve the class before calling the "check" method.

1.3.1. Where just a class name is provided the class is expected to have an authenticate() method.

1.4. A new Illuminate\Foundation\Support\Providers\BroadcastServiceProvider has been created with the same functionality signature as the EventServiceProvider.

1.4.1 This provider is extended within laravel/laravel via another pull request: laravel/laravel#3784 and in addition to using auth('channel', function(){}); you can now declare channel_name => [AuthClass::class] for auth callbacks. This will be familiar to users as it exists with the EventServiceProvider.

~~2. A new core broadcast driver has been added, called Rebound. This is a package i have developed in response to Laravel Echo and the authentication features coming in 5.3.~~

~~Rebound is NOT built ontop of socket.io. it is a package built ontop engine.io.~~

For those who dont know engine.io is the work horse of socket.io. engine.io is responsible for the transport upgrading, sending messages, reconnections, etc. Socket.io provides the namespaces, rooms, etc functionality.

The default responses added are to allow a generic api to using the broadcasting routes, namely for a socket client i built, detailed below. As they are "default" responses unless overwritten by a specific driver they could be used by any client following the same conventions.

I chose to build Rebound to have an open source alternative to Pusher, that shares the same API as Laravel Echo.

The way pusher has "channels" and socket.io has namespaces/rooms just didnt make sense when trying to build a unified api.

Rebound works by adding messages to a redis db, which are handled by a node rebound-server proccess, and pushed out to all the connected sockets. (similar to socket.io and redis in 5.2).

The rebound-client has the same API as laravel echo, just a different engine underneath.

For example echo:

var socket = new Echo('api', {
....options
});

var channel = socket.channel('name');
channel.on('Event', function(data){
//..
});

var channel = socket.private('name');
channel.on('Event', function(data){
//..
});

var channel = socket.presence('name');
channel.here(function(members){
//..
});

and Rebound:

var socket = new rebound.Socket('domain:port', {
....options
});

var channel = socket.channel('name');
channel.on('Event', function(data){
//..
});

var channel = socket.private('name');
channel.on('Event', function(data){
//..
});

var channel = socket.presence('name');
channel.here(function(members){
//..
});

When a socket connects the server sends a post request to /broadcasting/socket just like pusher does to "remember" the socket against the session.

The way rebound authenticates channels is very similar to pusher. The node server gets a request from a browser socket to join. the node server then sends a post request to the laravel app (on the same urls used for pusher /broadcasting/auth). this is where the auth('channel', function(){}); functions take over and either return data, or throw a 403 error.

if no error is thrown and data returned, the node server puts the socket into a "channel" group. If an error is thrown they dont get added, this is important.

The node server also caches the responses in redis to prevent multiple post requests on every page load (its configurable for expiry).

When a message comes in via redis, the channel name is collected.

The node server then loops through all the browser sockets within the channel group of the same name and sends them the event.

whats key here is sockets that requested access to that channel have to be validated by laravel before they get put into the node servers channel group. Then when events on that channel get fired we only send to sockets in that channel group. there is no way for the client to bypass this check.

The client api will be mainatined along with Laravel echo, and the server is pretty opinionated so doesnt need a public api at present, so both are stable as we speak (active development still happening, but nothing is likley to need changing).

The "server" can be found here: https://github.com/leemason/rebound-server

And the "client" here: https://github.com/leemason/rebound-server

More docs to come with time.

…iance on cache keys, added rebound driver

taylorotwell · 2016-05-22T21:13:42Z

src/Illuminate/Broadcasting/BroadcastManager.php

        if (! $request->hasSession()) {
            return;
        }

        return $this->app['cache']->get(
-            'pusher:socket:'.$request->session()->getId()
+            'realtime:socket:'.$request->session()->getId()


Maybe just "broadcast" since that is the related component? broadcast:socket.

yeah, makes sense, ive just updated that now. also of note, ive updated my rebound package to have csrf, and socket remembering functionality, it now works as-is with this setup.

leemason · 2016-05-22T21:52:17Z

in the last commit ive added an abstract broadcaster to reduce duplication for checks and auth callbacks.

however log and basic redis dont/wont need this sort of stuff, for testing etc do you think the validateAuthenticationResponse function should be declared in the abstract class as well? but return false/null?

taylorotwell · 2016-05-22T21:57:02Z

If you use Skype or Slack would be interested in IMing about this. Send me an e-mail if you don't mind.

taylorotwell · 2016-05-22T21:59:50Z

If the abstract class calls a method it should probably exist on that abstract class.

leemason · 2016-05-22T22:06:52Z

makes sense, ive added that now, got it returning false as it wont really do anything for drivers that dont support it anyway.

tlaverdure · 2016-05-23T16:08:57Z

I already submitted a PR to add auth support to the RedisBroadcaster. #13639

Don't really think there would be breaking changes here since the RedisBroadcaster has only had the broadcast() method on it.

leemason · 2016-05-23T16:14:45Z

that was only on the redis broadcaster, didnt cater for log or any other broadcasters that may be added. this extracts the common methods to an abstract class, imagine using log driver when in tests and using the new Broadcast::auth() function in a provider that fails because the driver being used doesnt have the method.

tlaverdure · 2016-05-23T16:15:11Z

src/Illuminate/Broadcasting/Broadcasters/ReboundBroadcaster.php

+     */
+    public function validAuthenticationResponse($request, $result)
+    {
+        return ['status' => 'success', 'user_id' => $request->user()->id, 'user_info' => $request->user()];


I would prefer to use this in the BroadcastServericeProvider like in @taylorotwell's demo and allow the developer to choose what data is presented in the auth response. In different situations, the user model may contain information that isn't necessary and you may want to pass along additional data. I believe that is what the $result variable was intended for.

this is a custom broadcast driver, not the core redis driver. agreed what your saying is true, but this is a custom build package, the default redis broadcaster returns false. i will update this to reflect the same api.

leemason · 2016-05-23T17:45:23Z

just a quick note, dont think the style checks actually failed, i think github was down when the test was run, should clear up with other commits if needed.

…socket id issue

…tring

leemason · 2016-05-24T11:59:06Z

Just changed the ability for Broadcast::auth to use
Broadcast::auth('channel', function(){}); and
Broadcast::auth('channel', Class\Name::class); (defaults to method "authenticate") and
Broadcast::auth('channel', '\\Class\\Name@method');

Like event system.

…r, to be extended in laravel/laravel

taylorotwell · 2016-05-24T14:37:09Z

This entire PR needs to have a more thorough description and title of what it actually does. How "Rebound" works, and so on.

leemason · 2016-05-24T15:27:16Z

agreed, its gone beyond a small amendment, ill edit the main comment.

leemason · 2016-05-24T16:11:35Z

@taylorotwell does the overview comment suffice in detailing the PR well enough now do you think?

taylorotwell · 2016-05-27T13:36:59Z

src/Illuminate/Broadcasting/BroadcastManager.php

@@ -81,12 +82,20 @@ public function socket($request = null)
            return $request->header('X-Socket-Id');
        }

+        if ($request->hasCookie('io')) {


Where is this cookie sent from?

The original request sent from engine io includes a set cookie header for that. And in rebound I pass the original request headers to the save socket post. This includes the io header which is the socket I'd.

On 27 May 2016, at 14:37, Taylor Otwell [email protected] wrote:

In src/Illuminate/Broadcasting/BroadcastManager.php:

@@ -81,12 +82,20 @@ public function socket($request = null)
return $request->header('X-Socket-Id');
}

if ($request->hasCookie('io')) {
Where is this cookie sent from?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

taylorotwell · 2016-05-27T13:38:10Z

One question I have is why call the PHP Rebound Driver "ReboundDriver" at all. Nothing it does really needs to be specific to Rebound. It functions in a pretty generic way that potentially other libraries could leverage at some time in the future. Though, I'm not sure what to call it.

leemason · 2016-05-27T14:01:49Z

Yeah I suppose really it's just the redis driver with the extra methods to authenticate channels. Maybe I should just add the authentication methods to the redis driver and remove this?

It wouldn't break any existing redis driver users. Just add the new methods available for authenticating.

What do you think?

Sent from my iPhone

On 27 May 2016, at 14:38, Taylor Otwell [email protected] wrote:

One question I have is why call the PHP Rebound Driver "ReboundDriver" at all. Nothing it does really needs to be specific to Rebound. It functions in a pretty generic way that potentially other libraries could leverage at some time in the future. Though, I'm not sure what to call it.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

leemason · 2016-05-27T14:03:50Z

Or maybe add the auth/save methods to the abstract driver so all drivers inherit the methods. But can still overwrite them, like pusher? Thoughts?

Sent from my iPhone

On 27 May 2016, at 14:38, Taylor Otwell [email protected] wrote:

One question I have is why call the PHP Rebound Driver "ReboundDriver" at all. Nothing it does really needs to be specific to Rebound. It functions in a pretty generic way that potentially other libraries could leverage at some time in the future. Though, I'm not sure what to call it.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

taylorotwell · 2016-05-27T14:06:20Z

I agree that it should probably just be part of the Redis driver?

On May 27, 2016, at 9:03 AM, Lee Mason [email protected] wrote:

Or maybe add the auth/save methods to the abstract driver so all drivers inherit the methods. But can still overwrite them, like pusher? Thoughts?

Sent from my iPhone

On 27 May 2016, at 14:38, Taylor Otwell [email protected] wrote:

One question I have is why call the PHP Rebound Driver "ReboundDriver" at all. Nothing it does really needs to be specific to Rebound. It functions in a pretty generic way that potentially other libraries could leverage at some time in the future. Though, I'm not sure what to call it.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #13653 (comment), or mute the thread https://github.com/notifications/unsubscribe/AAcRfrehC-db4IVsgUyCHF4Izzl_qYBlks5qFvnKgaJpZM4IkEP_.

leemason · 2016-05-27T14:59:25Z

PR updated to use default responses unless otherwise provided, removed not needed new driver.

Ive added the methods to the abstract driver, not just redis as in the abstract driver they can be used as a "convension" for any client, not just redis.

This way any websocket client built for interaction with laravel, can expect to recieve the standard responses unless they need to overwrite them.

leemason · 2016-05-27T15:00:36Z

Also updated any reference to $request->user()->id to $request->user()->getKey() for edge cases where the pk isnt id

taylorotwell · 2016-05-28T02:15:45Z

@leemason one thing I'm a little confused about is it looks like on valid authentication responses you are returning user_info, but wouldn't that just be true or false, which is what the authenticators respond with when authenticating regular channels?

taylorotwell · 2016-05-28T02:34:10Z

Also curious how your implementation is handling the "dont broadcast to the current user" situation? This similar PR handles it: #13639

leemason · 2016-05-28T15:52:20Z

@taylorotwell the valid response returns user_info so that presence channels can populate member info.

the auth callbacks "can" just return true yes, but then the "members" info even with pusher will contain no data, i wasnt aware you were using it with just a true response. from the code and reading the pusher api its clear this response should contain user info.

how are you populating member info with the pusher api if not via returning data on the callback?

The auth callbacks as i understood it should return user info, or false.

As for the dont broadcast to current user. This pr doesnt need to anything with it. Its the clients job to handle that data.

Pusher looks for the socket property on the event, as does rebound. Its their job to "not" send them the events, the laravel side just sets the socket property in the event.

leemason · 2016-05-28T16:03:12Z

ahh i see what you mean, the socket is extracted from the event data in the other pr.

do you think thats needed? its really up to you wether we include in the event data, or pull it out seperatley.

There are no clients which consume this data yet (except rebound, which can be changed) so the data structure is up to you.

Personally i think keeping it as part of the payload has some benefits over extracting it.

it doesnt matter either way for clients, as none exist yet getting the data will just mean either root level "socket" or "data.socket".
including the socket id in the event data means if desired by the client, it can be passed down to the browsers and used (for example in presence channels each members socket id is known, so it could be matched up).

leemason · 2016-05-28T16:06:30Z

Talking about that, i think we also need to update the docs on events returning their own data structure.

As if they do the socket property wont get sent unless explicitly declared in the broadcastWith() method.

taylorotwell · 2016-05-28T18:41:43Z

I do return the user data for pusher but only when authenticating presence channels. When authenticating plain private channels you can just return true or false. Pusher doesn't need any user data for a plain private channel.

On Sat, May 28, 2016 at 8:52 AM -0700, "Lee Mason" [email protected] wrote:

@taylorotwell the valid response returns user_info so that presence channels can populate member info.

the auth callbacks "can" just return true yes, but then the "members" info even with pusher will contain no data, i wasnt aware you were using it with just a true response. from the code and reading the pusher api its clear this response should contain user info.

how are you populating member info with the pusher api if not via returning data on the callback?

The auth callbacks as i understood it should return user info, or false.

As for the dont broadcast to current user. This pr doesnt need to anything with it. Its the clients job to handle that data.

Pusher looks for the socket property on the event, as does rebound. Its their job to "not" send them the events, the laravel side just sets the socket property in the event.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

leemason · 2016-05-28T19:30:04Z

surely there is no reason to return false at all, as if the result isnt true a 403 http exception is thrown?

taylorotwell · 2016-05-28T19:31:09Z

I mean sure you could return the user info regardless.

On Sat, May 28, 2016 at 12:30 PM -0700, "Lee Mason" [email protected] wrote:

surely there is no reason to return false at all, as if the result isnt true a 403 http exception is thrown?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

leemason · 2016-05-28T19:37:02Z

i know user_info is generally for presence channels, but there could be implementations where a private channel "could" use the user info returned, it would depend on the client using the response.

its up to the auth callback what gets returned anyway, only allowing it for presence we can do for sure, but isnt it just a limitation that has no benefit, and wouldnt cause any issues keeping it as it is?

taylorotwell · 2016-05-28T20:57:26Z

Sure I agree

On Sat, May 28, 2016 at 12:37 PM -0700, "Lee Mason" [email protected] wrote:

i know user_info is generally for presence channels, but there could be implementations where a private channel "could" use the user info returned, it would depend on the client using the response.

its up to the auth callback what gets returned anyway, only allowing it for presence we can do for sure, but isnt it just a limitation that has no benefit, and wouldnt cause any issues keeping it as it is?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

taylorotwell · 2016-05-28T20:57:56Z

So can you explain in more detail how you implement the "don't broadcast to current user" stuff?

On Sat, May 28, 2016 at 12:37 PM -0700, "Lee Mason" [email protected] wrote:

i know user_info is generally for presence channels, but there could be implementations where a private channel "could" use the user info returned, it would depend on the client using the response.

its up to the auth callback what gets returned anyway, only allowing it for presence we can do for sure, but isnt it just a limitation that has no benefit, and wouldnt cause any issues keeping it as it is?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

leemason · 2016-05-28T21:22:09Z

The dont broadcast to current user isnt actually handled in any specific way like it is with pusher.

because pusher is an api driven implementation you explicitly send the socket id to the api.

However using the functionality via redis puts the implementation of this on top the consuming application, usually a node js proccess.

The Laravel apps only responsibility is to send along the socket id with the event info into a redis db.

Which it already does when using the trait by having a public property called $socket. we dont need to alter anything as it will automatically get added as part of the event payload in the data property.

When the event gets sent to redis, its sent to a channel like this:

{
    event: "eventname",
   data: {
        socket: "socketid",
        // other atts
   }
}

Its then up to the client to deal with this data and send it off to who needs it. For example in rebound it looks like this:

this.redis.on('pmessage', function(subscribed, channel, message) {
            //code reduced for berevity
            //sockets is object of all connected to channel
            for(let socket in sockets){

                //dont send if specified by the event itself via the socket property
                if(
                   sockets.hasOwnProperty(socket) && 
                   payload.data.socket == socket
                 ) {
                   continue;
                 }
                sockets[socket].send(payload);
            }

        });

before sending the event out, it loops through the connected sockets, and checks if the socket id matches the socket property on the event payload, if it does it wont send it to them.

GrahamCampbell · 2016-06-01T13:57:45Z

Just a quick note that our master branch is now L5.4.

leemason · 2016-06-01T14:27:58Z

so would this be delayed until 5.4? whats happened to 5.3? and should i create a new pull request to the right branch?

taylorotwell · 2016-06-01T14:28:21Z

It needs to be sent to 5.3 branch now.

Regards,
Taylor

On Jun 1, 2016, at 9:28 AM, Lee Mason [email protected] wrote:

so would this be delayed until 5.4? whats happened to 5.3? and should i create a new pull request to the right branch?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #13653 (comment), or mute the thread https://github.com/notifications/unsubscribe/AAcRfubZpcvf1HhQNGFugcdczeDLhf9sks5qHZbygaJpZM4IkEP_.

leemason · 2016-06-01T14:39:43Z

Closing to retarget 5.3

changed pusher: to realtime: in broadcast manager for less vendor rel…

4e839a1

…iance on cache keys, added rebound driver

taylorotwell reviewed May 22, 2016
View reviewed changes

leemason added 2 commits May 22, 2016 22:15

recommended rename of cache key

d171858

refactoring duplicate functions

cbc1ab1

style fixes

ee0975b

adding common auth response function

de02c67

GrahamCampbell changed the title ~~changed pusher: to realtime: in broadcast manager for less vendor rel…~~ [5.2] changed pusher: to realtime May 23, 2016

GrahamCampbell changed the title ~~[5.2] changed pusher: to realtime~~ [5.3] Changed pusher to realtime May 23, 2016

response changes to reflect package api similarities with pusher

31769d8

tlaverdure reviewed May 23, 2016
View reviewed changes

leemason added 3 commits May 23, 2016 17:22

updated resulting user info for custom driver

d00d91d

optionally allow broadcaster to return a response on socket save

4543060

spelling error

b0851f0

leemason added 2 commits May 23, 2016 21:05

allow for requests to send socket_id, preventing the multi-tab/wrong …

b908682

…socket id issue

stop styleci complaining about empty return

109ece7

leemason force-pushed the master branch from ed70f6f to 109ece7 Compare May 23, 2016 20:16

Allow Broadcast::auth() to receive a callable or \Class\Name@method s…

7681660

…tring

broadcast support provider works in same way as event service provide…

fd4a8bb

…r, to be extended in laravel/laravel

leemason mentioned this pull request May 24, 2016

[5.3] Amended broadcast service provider to extend foundation provider laravel/laravel#3784

Closed

leemason changed the title ~~[5.3] Changed pusher to realtime~~ [5.3] Broadcasting Improvements and Addition of a new core driver May 24, 2016

taylorotwell reviewed May 27, 2016
View reviewed changes

setup default responses, removed rebound driver for vanilla redis driver

cb2c839

leemason changed the title ~~[5.3] Broadcasting Improvements and Addition of a new core driver~~ [5.3] Broadcasting Improvements May 27, 2016

tlaverdure mentioned this pull request May 30, 2016

[5.4] Adding support for new Broadcast Support Provider #13639

Closed

GrahamCampbell changed the title ~~[5.3] Broadcasting Improvements~~ [5.4] Broadcasting Improvements Jun 1, 2016

leemason mentioned this pull request Jun 1, 2016

[5.3] Broadcasting Improvements #13814

Closed

leemason closed this Jun 1, 2016

[5.4] Broadcasting Improvements #13653

[5.4] Broadcasting Improvements #13653

Uh oh!

Conversation

leemason commented May 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taylorotwell May 22, 2016

Choose a reason for hiding this comment

Uh oh!

leemason May 22, 2016

Choose a reason for hiding this comment

Uh oh!

leemason commented May 22, 2016

Uh oh!

taylorotwell commented May 22, 2016

Uh oh!

taylorotwell commented May 22, 2016

Uh oh!

leemason commented May 22, 2016

Uh oh!

tlaverdure commented May 23, 2016

Uh oh!

leemason commented May 23, 2016

Uh oh!

tlaverdure May 23, 2016

Choose a reason for hiding this comment

Uh oh!

leemason May 23, 2016

Choose a reason for hiding this comment

Uh oh!

leemason commented May 23, 2016

Uh oh!

leemason commented May 24, 2016

Uh oh!

taylorotwell commented May 24, 2016

Uh oh!

leemason commented May 24, 2016

Uh oh!

leemason commented May 24, 2016

Uh oh!

taylorotwell May 27, 2016

Choose a reason for hiding this comment

Uh oh!

leemason May 27, 2016

Choose a reason for hiding this comment

Uh oh!

taylorotwell commented May 27, 2016

Uh oh!

leemason commented May 27, 2016

Uh oh!

leemason commented May 27, 2016

Uh oh!

taylorotwell commented May 27, 2016

Uh oh!

leemason commented May 27, 2016

Uh oh!

leemason commented May 27, 2016

Uh oh!

taylorotwell commented May 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taylorotwell commented May 28, 2016

Uh oh!

leemason commented May 28, 2016

Uh oh!

leemason commented May 28, 2016

Uh oh!

leemason commented May 28, 2016

Uh oh!

taylorotwell commented May 28, 2016

Uh oh!

leemason commented May 28, 2016

Uh oh!

taylorotwell commented May 28, 2016

Uh oh!

leemason commented May 28, 2016

Uh oh!

taylorotwell commented May 28, 2016

Uh oh!

leemason commented May 22, 2016 •

edited

Loading

taylorotwell commented May 28, 2016 •

edited

Loading