Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client handling should be seperated from communication plugin #2

Open
roybirger opened this issue Jan 8, 2016 · 7 comments
Open

client handling should be seperated from communication plugin #2

roybirger opened this issue Jan 8, 2016 · 7 comments

Comments

@roybirger
Copy link

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
Copy link
Owner

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
Copy link
Author

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

@juho-jaakkola
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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.

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

No branches or pull requests

2 participants