search

juho-jaakkola / elgg-pusher

3 stars 1 forks source link

client handling should be seperated from communication plugin #2

Open roybirger opened 8 years ago

roybirger commented 8 years ago

The pusher plugin does not support client validation and authentication or alternatively clients subsrcription and handling should be performed by an external component or plugin.

juho-jaakkola commented 8 years ago

When I still hadn't moved communication features away from live_notifier plugin, I had the token approach for client authentication: https://github.com/juho-jaakkola/elgg-live_notifier/blob/72a3a480b590e27dcba1589c8857a095fdd655ea/classes/LiveNotifier/Pusher.php#L41

Do you think that kind of approach would be sufficient? Or do you have some ideas/requirements in mind?

roybirger commented 8 years ago

I think the Token approach could work, although it might be useful to configure it somehow (for example token lifetime etc.)

juho-jaakkola commented 8 years ago

Websocket message passes along the regular HTTP headers, so I suppose one option could also be to check whether there is a valid session in the users_sessions table.

roybirger commented 8 years ago

This will not be enough for me since I have users with valid sessions who are not authorized to send or recieve notificatios so I must have some form of custom authentication.

juho-jaakkola commented 8 years ago

I'm allowing anyone to subscribe, but a specific recipient is explicitly defined for each message:

I think I need to study your plugin a bit more to understand your use case better.

roybirger commented 8 years ago

I want to recieve notifications for a user once he is logged in, so that's when I'll provide the token. But the user can perform actions without being logged in so I don't want to send notifications to him although he may be a target of such.

juho-jaakkola commented 8 years ago

Sorry, by "anyone" I meant any logged in user. So I believe we in fact have the same goal.

On the initial page load, we can add the token conditionally:

if (elgg_is_logged_in()) {
    // Generate a token and add it to the page
}

And then javascript will open the websocket connection depending if there is a token or not.

Here's how live_notifier plugin was doing it earlier. If user is not logged in, the function returns immediately without adding the token: https://github.com/juho-jaakkola/elgg-live_notifier/blob/72a3a480b590e27dcba1589c8857a095fdd655ea/start.php#L33. Something similar could be added to the pusher plugin.