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

Chrome support #241

Closed
tiste opened this issue May 30, 2016 · 9 comments
Closed

Chrome support #241

tiste opened this issue May 30, 2016 · 9 comments

Comments

@tiste
Copy link

tiste commented May 30, 2016

Hey, I used node-gcm to deliver push notifications on Android devices and it works really well, good job :)

Now I moved to others devices, and I implemented browser push notifications (thanks to service workers), and it works as well on that devices. The thing is Chrome didn't accept payload. Now they are.

What do you guys think about adding push encryption to node-gcm, to allow payload on browsers?

@hypesystem
Copy link
Collaborator

This looks really cool!

We are currently in the process of moving to v1, making lots of breaking changes to the API (see #238), so I would suggest doing it as a part of the new interface, rather than the old.

I would love to see a PR with a suggestion. First off, though, we need to figure out what the interface would look like. How do we decide when to encrypt the payload?

@tiste
Copy link
Author

tiste commented May 30, 2016

😊
Actually, when a user subscribe to push notification, the subscription appears with an endpoint and two key: the auth one, the secret one, so that the interface crypt the payload thanks to the two previous key, and pass it to gcm.

So, I assume an endpoint and 2 keys uh?

@eladnava
Copy link
Collaborator

eladnava commented May 31, 2016

This is how web-push-encryption does it:
https://github.com/GoogleChrome/web-push-encryption/blob/4fc4326a2c3d6b56e8f607f6f79eefe9429fc516/src/push.js#L60-L119

And here:
https://github.com/GoogleChrome/web-push-encryption/blob/4fc4326a2c3d6b56e8f607f6f79eefe9429fc516/src/encrypt.js#L47-L124

Adding supports for this consists of sending additional headers, encrypting the message, etc. It isn't very easy to implement apparently, as the web-push-encryption Node.js module has quite some logic, mainly for the encryption.

@hypesystem
Copy link
Collaborator

Yeah it looks pretty complex. Maybe the best solution would be to implement it as a separate project that uses node-gcm?

The only hard part in this would be to circumvent the current validation of the message body, but we could provide a flag to the sender, that turns message validation off.

@eladnava
Copy link
Collaborator

eladnava commented Jun 3, 2016

@hypesystem @tiste Are we sure we want to reimplement a package that the Google Chrome team seem to have implemented nicely already?
https://github.com/GoogleChrome/web-push-encryption

The interface is very similar to ours.

@tiste
Copy link
Author

tiste commented Jun 3, 2016

@eladnava I didn't really had time to look at integrate payload from web-push-encryption to the current node-gcm.

If it's quite easy, we may add an example to the README, right?

@eladnava
Copy link
Collaborator

eladnava commented Jun 3, 2016

@tiste As discussed above, it actually looks quite complex to integrate the encryption and payload format required for sending to Chrome, and since there is already a package that does just that, we're not sure if we should be reimplementing the same functionality.

@hypesystem What do you think?

@hypesystem
Copy link
Collaborator

@eladnava well if they do a superset of what we do, then there is no reason to integrate it 😄

@eladnava
Copy link
Collaborator

eladnava commented Oct 3, 2016

Closing this as the solution here is simply to use the web-push-encryption package instead of reimplementing its logic in node-gcm.

@eladnava eladnava closed this as completed Oct 3, 2016
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

3 participants