jazzband / django-push-notifications

Send push notifications to mobile devices through GCM or APNS in Django.
MIT License
2.26k stars 613 forks source link

Add WebPush support for Safari #674

Closed blighj closed 12 months ago

blighj commented 1 year ago

PR for #673 Stores the full endpoint url for web push given by the browser, in the registration_id of the model, instead of just the token, and use exactly what the browser gave us when we send the message. Still supports existing registration_id/browser combo's but rasies a warning.

Also:

The new docs address issues/questions raised in #647, #658, #665 The PR was inspired/informed by #487, #558, #602, #603, #604, #605

simonkern commented 1 year ago

tores the full endpoint url for web push given by the browser, in the registration_id of the model, instead of just the token, and use exactly what the browser gave us when we send the message.

I also thought about this aspect some time ago when I added push support and think this is a really good idea. This could also remove the need to extract the browser type via javascript, because we then already have the url of the endpoint we need to send the push to stored in the WebPushDevice object.

simonkern commented 1 year ago

iOS 16.4 just landed. What's the current status here? Are there still things that need to be done?

blighj commented 1 year ago

From my prespective, as a first time contributor, this is ready to go. I assume the next step is a code review.

simonkern commented 1 year ago

I am not a maintainer, but I could take this for a test spin. Do you know who could do a release? Maybe @alenzeinolov or @jamaalscarlett can help out?

ammarjmhor10 commented 1 year ago

Hi, is there any update please?

mamal72 commented 1 year ago

Hey guys. Any updates on this? Can we do one more push and have it released? Thanks in advance.

azmeuk commented 1 year ago

I tested this branch on several browsers, including Safari on iOS and I confirm this works!

azmeuk commented 1 year ago

For what it worth, instead of manually using py-vapid, I could do the key generation dynamically with:

from py_vapid import b64urlencode
from py_vapid import Vapid02 as Vapid
from cryptography.hazmat.primitives import serialization
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives.asymmetric import ec

def vapid_public_key(path):
    vapid = Vapid.from_file(path)
    pk = vapid.public_key.public_bytes(
        serialization.Encoding.X962, serialization.PublicFormat.UncompressedPoint
    )
    return b64urlencode(pk)

def generate_private_key(path):
    private_key = ec.generate_private_key(ec.SECP256R1, default_backend())
    private_pem = private_key.private_bytes(
        encoding=serialization.Encoding.PEM,
        format=serialization.PrivateFormat.PKCS8,
        encryption_algorithm=serialization.NoEncryption()
    )
    with open(path, "wb") as file:
        file.write(private_pem)
        file.close()

generate_private_key can be used if the private key does not exist, vapid_public_key can be used to get the base64 export of the public key and insert it dynamically in the templates.

jamaalscarlett commented 1 year ago

@blighj once you address @azmeuk I think this is good to merge. Nice work!

jamaalscarlett commented 1 year ago

@blighj looks like there are some newly failing tests

codecov[bot] commented 1 year ago

Codecov Report

Merging #674 (99fca51) into master (53525ca) will increase coverage by 0.84%. Report is 4 commits behind head on master. The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master     #674      +/-   ##
==========================================
+ Coverage   68.67%   69.51%   +0.84%     
==========================================
  Files          26       26              
  Lines        1111     1122      +11     
  Branches      243      245       +2     
==========================================
+ Hits          763      780      +17     
+ Misses        310      304       -6     
  Partials       38       38              
Files Coverage Δ
push_notifications/conf/app.py 75.15% <100.00%> (ø)
push_notifications/migrations/0001_initial.py 100.00% <ø> (ø)
...otifications/migrations/0002_auto_20160106_0850.py 100.00% <ø> (ø)
push_notifications/migrations/0003_wnsdevice.py 100.00% <ø> (ø)
push_notifications/migrations/0004_fcm.py 100.00% <ø> (ø)
...ush_notifications/migrations/0005_applicationid.py 100.00% <ø> (ø)
...ush_notifications/migrations/0006_webpushdevice.py 100.00% <ø> (ø)
push_notifications/webpush.py 100.00% <100.00%> (+33.33%) :arrow_up:
push_notifications/fields.py 64.06% <50.00%> (ø)
push_notifications/models.py 78.04% <0.00%> (ø)
... and 1 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

blighj commented 12 months ago

:) Thank you kindly I think you could now safely close PR #487 And the issues #457, #526, #629, #647, #657, and #665

a release to version 3.1.0 would be much appreciated too