dcai / airnotifier

Push Notifications Server for Human Beings.
Other
593 stars 187 forks source link

APNS Production Mode Not Supported and Missing Header #215

Closed clsource closed 4 years ago

clsource commented 4 years ago

Hello, there seems to be some issues with APN.

As we can see in

https://github.com/airnotifier/airnotifier/blob/v3.0.0-alpha.2/pushservices/apns.py#L16

only development mode is supported.

BASE_URL = "api.development.push.apple.com:443"

Is recommended to support both endpoints.

https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns?language=objc

Also Apple recomends using 2197 port when 443 is behind a firewall. Supporting changing the port could be useful.

Maybe here https://github.com/airnotifier/airnotifier/blob/v3.0.0-alpha.2/pushservices/apns.py#L39

       self.http2 = HTTPConnection(BASE_URL + ":" + str(self.port))
       if kwargs["environment"] == "production":
          self.http2 = HTTPConnection(BASE_URL_PRODUCTION + ":" + str(self.port))

Also a header should be added to https://github.com/airnotifier/airnotifier/blob/v3.0.0-alpha.2/pushservices/apns.py#L61

apns-push-type

(Required for watchOS 6 and later; recommended for macOS, iOS, tvOS, and iPadOS) The value of this header must accurately reflect the contents of your notification’s payload. If there is a mismatch, or if the header is missing on required systems, APNs may return an error, delay the delivery of the notification, or drop it altogether.

The type of the notification. The value of this header is alert or background. Specify alert when the delivery of your notification displays an alert, plays a sound, or badges your app's icon. Specify background for silent notifications that do not interact with the user.

The value of this header must accurately reflect the contents of your notification's payload. If there is a mismatch, or if the header is missing on required systems, APNs may delay the delivery of the notification or drop it altogether.

Based on these docs https://github.com/node-apn/node-apn/blob/master/doc/notification.markdown#notificationpushtype

https://github.com/airnotifier/airnotifier/blob/v3.0.0-alpha.2/pushservices/apns.py#L60

return {
            "apns-expiration": "0",
            "apns-priority": "10",
            "apns-push-type": self.push_type # alert or background
            "apns-topic": self.bundle_id,
            "mutable-content": "1",
            "authorization": "bearer {0}".format(token),
        }

Thanks 👍

dcai commented 4 years ago

nice work, thanks @clsource 👍 ill find time to integrate your suggestion.

dcai commented 4 years ago

@clsource I have added a fix in master branch, let me know if you find any issues.