web-push-libs / pywebpush

Python Webpush Data encryption library
Mozilla Public License 2.0
305 stars 52 forks source link

webpush modifies the vapid_claims parameter #79

Closed nglgzz closed 6 years ago

nglgzz commented 6 years ago

I have an object in my Django config for vapid claims and I'm using it in the webpush method.

I couldn't realize why after sending push messages on Firefox I was constantly getting "unauthorized registration" errors on Chrome, until I saw that the object passed as vapid_claims parameter gets modified from something like this:

{
    'sub': 'mailto:<my_email>'
}

to this :

{
    'aud': 'https://updates.push.services.mozilla.com',
    'exp': '1508309139',
    'sub': 'mailto:<my_email>'
}

I tried passing every time a new object and everything works fine, but now I'm wondering, is this expected behavior, and am I supposed to store that modified object?

jrconlin commented 6 years ago

Yes, this is expected.

Vapid requires the sub, and chrome also requires both the exp and aud. If you don't specify the aud parameter, it will use one derived from the endpoint. I'll note that you can't use endpoints interchangeably (e.g. you can't use an endpoint generated for firefox on chrome or chrome on edge, etc.).

You can specify your own values for aud and exp which will not be overwritten. Be aware that exp cannot be more than 24 hours in UTC. You may also wish to check that your server time is accurate, since that could produce clock skew, and make your VAPID header invalid.

nglgzz commented 6 years ago

Okay, thank you for the clarification 👍

cauethenorio commented 6 years ago

@jrconlin we had the same issue here using pywebpush.

We store a VAPID_CLAIMS dict in our django settings file and we noticed it was changed after calling the webpush func for the first time, making all subsquent calls to that function to fail.

I think in the most cases people which use this library aren't expecting it will mutate the dict sent as vapid_claims param.

It could be easily fixed on your end cloning the dict before adding the missing keys, what do you think?

jrconlin commented 6 years ago

Pushed py_vapid 1.4.0 https://pypi.org/project/py-vapid/1.4.0/#description

cauethenorio commented 6 years ago

@jrconlin thanks man