ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 208 forks source link

Chrome support #241

Closed tiste closed 7 years ago

tiste commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

:blush: 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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

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

eladnava commented 7 years ago

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