pwa: never delete subscriptions (#85458)

Subscriptions must be garbage collected when notifications request
receive the 410 Gone status. Deleting all subscriptions would delete
subcriptions registered from another browser and there is no way to
relate existing subscription to a particular browser.

combo/apps/pwa/templates/combo/service-worker-registration.js

@@ -78,7 +78,6 @@ function combo_pwa_unsubscribe_user() {
     console.log('Error unsubscribing', error);
   })
   .then(function() {
-    combo_pwa_update_subscription_on_server(null);
     console.log('User is unsubscribed.');
     COMBO_PWA_USER_SUBSCRIPTION = false;
     $(document).trigger('combo:pwa-user-info');

combo/apps/pwa/views.py

@@ -96,13 +96,12 @@ def subscribe_push(request, *args, **kwargs):
     except json.JSONDecodeError:
         return HttpResponseBadRequest('bad json request: "%s"' % request.body)
 
-    if subscription_data is None:
-        PushSubscription.objects.filter(user=request.user).delete()
-    else:
-        subscription, dummy = PushSubscription.objects.get_or_create(
-            user=request.user, subscription_info=subscription_data
-        )
-        subscription.save()
+    if not isinstance(subscription_data, dict) or not (set(subscription_data) >= {'keys', 'endpoint'}):
+        return HttpResponseBadRequest('bad json request: "%s"' % subscription_data)
+    subscription, dummy = PushSubscription.objects.get_or_create(
+        user=request.user, subscription_info=subscription_data
+    )
+    subscription.save()
     return JsonResponse({'err': 0})
 
 

tests/test_pwa.py

@@ -49,22 +49,21 @@ def test_service_worker(app):
 
 
 def test_webpush_subscription(app, john_doe, jane_doe):
-    app.post_json(reverse('pwa-subscribe-push'), params={'sample': 'content'}, status=403)
+    app.post_json(reverse('pwa-subscribe-push'), params={'endpoint': 'endpoint1', 'keys': {}}, status=403)
     app.get(reverse('pwa-subscribe-push'), status=403)
     app = login(app, john_doe.username, john_doe.username)
-    app.post_json(reverse('pwa-subscribe-push'), params={'sample': 'content'}, status=200)
+    assert PushSubscription.objects.count() == 0
+    app.post_json(reverse('pwa-subscribe-push'), params={'endpoint': 'endpoint1', 'keys': {}}, status=200)
     assert PushSubscription.objects.count() == 1
-    app.post_json(reverse('pwa-subscribe-push'), params={'sample': 'content2'}, status=200)
+    app.post_json(reverse('pwa-subscribe-push'), params={'endpoint': 'endpoint1', 'keys': {}}, status=200)
+    assert PushSubscription.objects.count() == 1
+    app.post_json(reverse('pwa-subscribe-push'), params={'endpoint': 'endpoint2', 'keys': {}}, status=200)
     assert PushSubscription.objects.count() == 2
 
     app = login(app, jane_doe.username, jane_doe.username)
-    app.post_json(reverse('pwa-subscribe-push'), params={'sample': 'content'}, status=200)
+    app.post_json(reverse('pwa-subscribe-push'), params={'endpoint': 'endpoint1', 'keys': {}}, status=200)
     assert PushSubscription.objects.count() == 3
 
-    app = login(app, john_doe.username, john_doe.username)
-    app.post_json(reverse('pwa-subscribe-push'), params=None, status=200)
-    assert PushSubscription.objects.count() == 1
-
     app = login(app, john_doe.username, john_doe.username)
     resp = app.post(reverse('pwa-subscribe-push'), params='', status=400)
     assert 'bad json' in resp.text
@@ -76,7 +75,7 @@ def test_webpush_notification(app, john_doe, caplog):
     app = login(app, john_doe.username, john_doe.username)
     app.post_json(
         reverse('pwa-subscribe-push'),
-        params={'endpoint': 'https://push.example.com:1000/', 'sample': 'content'},
+        params={'endpoint': 'https://push.example.com:1000/', 'keys': {}},
         status=200,
     )
 
@@ -94,7 +93,7 @@ def test_webpush_notification(app, john_doe, caplog):
         webpusher.return_value.send.return_value.status_code = 200
         Notification.notify(john_doe, 'test', body='hello world')
         assert webpusher.mock_calls == [
-            mock.call({'sample': 'content', 'endpoint': 'https://push.example.com:1000/'}),
+            mock.call({'keys': {}, 'endpoint': 'https://push.example.com:1000/'}),
             mock.call().send(
                 data='{"summary": "test", "body": "hello world", "url": ""}',
                 headers={'Authorization': mock.ANY, 'Urgency': 'low'},
@@ -110,7 +109,7 @@ def test_webpush_notification(app, john_doe, caplog):
         Notification.notify(john_doe, 'test', body='hello world')
         assert vapid.mock_calls == []
         assert webpusher.mock_calls == [
-            mock.call({'sample': 'content', 'endpoint': 'https://push.example.com:1000/'}),
+            mock.call({'keys': {}, 'endpoint': 'https://push.example.com:1000/'}),
             mock.call().send(
                 data='{"summary": "test", "body": "hello world", "url": ""}',
                 headers={'Authorization': mock.ANY, 'Urgency': 'low'},
@@ -131,7 +130,7 @@ def test_webpush_notification(app, john_doe, caplog):
     caplog.clear()
     app.post_json(
         reverse('pwa-subscribe-push'),
-        params={'endpoint': 'https://push.example.com:1000/', 'sample': 'content'},
+        params={'endpoint': 'https://push.example.com:1000/', 'keys': {}},
         status=200,
     )
     with mock.patch('pywebpush.WebPusher') as webpusher: