googlearchive / web-push-encryption

[Deprecated] Encryption Utilities for Web Push protocol
Apache License 2.0
84 stars 23 forks source link

Allow sending with no payload, plus some small fixes #17

Closed wibblymat closed 8 years ago

wibblymat commented 8 years ago

@gauntface

Fixes #10

gauntface commented 8 years ago

cc @petele

Awesome - thanks for doing this.

Only concern is still with the auth token, I can see people doing:

subscriptions.forEach(subscription => {
    if (!subscription.aesgcm) {
        return;
    }
    pushEncryption.sendWebPush('Totes rad msg', subscription, GCM_API_KEY);
});

Which means they are sharing their API key with the world :(

If we changed back to a method, we can be smarter and only include the header for GCM requests.

pushEncryption.setGCMAPIKey(GCM_API_KEY);
subscriptions.forEach(subscription => {
    if (!subscription.aesgcm) {
        return;
    }
    pushEncryption.sendWebPush('Totes rad msg', subscription);
});
addyosmani commented 8 years ago

If we changed back to a method, we can be smarter and only include the header for GCM requests.

I would be strongly in favour of this if we can push for it. I had similar concerns to @gauntface about API keys being shared with the world and can see folks tripping up here :/

wibblymat commented 8 years ago

@gauntface OK, done

gauntface commented 8 years ago

One small nit on TTL, other than that - LGTM

wibblymat commented 8 years ago

@gauntface you're right, spec says TTL. No idea why I thought otherwise!