From 002dddfb65eff360dfd85f57b42d4e0094757542 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Wed, 21 Aug 2024 23:34:30 +0530 Subject: [PATCH 01/17] [chore] Create EmailTokenGenerator --- openwisp_notifications/tokens.py | 68 ++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 openwisp_notifications/tokens.py diff --git a/openwisp_notifications/tokens.py b/openwisp_notifications/tokens.py new file mode 100644 index 00000000..c60460ef --- /dev/null +++ b/openwisp_notifications/tokens.py @@ -0,0 +1,68 @@ +from django.contrib.auth.tokens import PasswordResetTokenGenerator +from django.utils.crypto import constant_time_compare +from django.utils.http import base36_to_int + + +class EmailTokenGenerator(PasswordResetTokenGenerator): + """ + Email token generator that extends the default PasswordResetTokenGenerator + with a fixed 7-day expiry period and a salt key. + """ + + key_salt = "openwisp_notifications.tokens.EmailTokenGenerator" + + def __init__(self): + super().__init__() + self.expiry_days = 7 + + def check_token(self, user, token): + """ + Check that a token is correct for a given user and has not expired. + """ + if not (user and token): + return False + + # Parse the token + try: + ts_b36, _ = token.split("-") + except ValueError: + return False + + try: + ts = base36_to_int(ts_b36) + except ValueError: + return False + + # Check that the timestamp/uid has not been tampered with + for secret in [self.secret, *self.secret_fallbacks]: + if constant_time_compare( + self._make_token_with_timestamp(user, ts, secret), + token, + ): + break + else: + return False + + # Check the timestamp is within the expiry limit. + if (self._num_seconds(self._now()) - ts) > self._expiry_seconds(): + return False + + return True + + def _make_hash_value(self, user, timestamp): + """ + Hash the user's primary key and password to produce a token that is + invalidated when the password is reset. + """ + email_field = user.get_email_field_name() + email = getattr(user, email_field, "") or "" + return f"{user.pk}{user.password}{timestamp}{email}" + + def _expiry_seconds(self): + """ + Returns the number of seconds representing the token's expiry period. + """ + return self.expiry_days * 24 * 3600 + + +email_token_generator = EmailTokenGenerator() From 7322a73bcb5441088841f687d0923426a8bde0ae Mon Sep 17 00:00:00 2001 From: Dhanus Date: Sat, 24 Aug 2024 12:51:17 +0530 Subject: [PATCH 02/17] [chore] Unsubscribe Implementation --- openwisp_notifications/base/views.py | 98 +++++++++++++++++++ .../css/unsubscribe.css | 67 +++++++++++++ openwisp_notifications/tasks.py | 23 ++++- .../openwisp_notifications/unsubscribe.html | 90 +++++++++++++++++ openwisp_notifications/urls.py | 4 +- openwisp_notifications/utils.py | 26 +++++ 6 files changed, 304 insertions(+), 4 deletions(-) create mode 100644 openwisp_notifications/base/views.py create mode 100644 openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css create mode 100644 openwisp_notifications/templates/openwisp_notifications/unsubscribe.html diff --git a/openwisp_notifications/base/views.py b/openwisp_notifications/base/views.py new file mode 100644 index 00000000..0863b938 --- /dev/null +++ b/openwisp_notifications/base/views.py @@ -0,0 +1,98 @@ +import base64 +import json + +from django.contrib.auth import get_user_model +from django.http import JsonResponse +from django.shortcuts import render +from django.utils.decorators import method_decorator +from django.views.decorators.csrf import csrf_exempt +from django.views.generic import TemplateView + +from openwisp_notifications.swapper import load_model + +from ..tokens import email_token_generator + +User = get_user_model() +NotificationSetting = load_model('NotificationSetting') + + +@method_decorator(csrf_exempt, name='dispatch') +class UnsubscribeView(TemplateView): + template_name = 'openwisp_notifications/unsubscribe.html' + + def get(self, request, *args, **kwargs): + encoded_token = request.GET.get('token') + if not encoded_token: + return render(request, self.template_name, {'valid': False}) + + user, valid = self._validate_token(encoded_token) + if not valid: + return render(request, self.template_name, {'valid': False}) + + notification_preference = self.get_user_preference(user) + + return render( + request, + self.template_name, + { + 'valid': True, + 'user': user, + 'is_subscribed': notification_preference.email, + }, + ) + + def post(self, request, *args, **kwargs): + encoded_token = request.GET.get('token') + if not encoded_token: + return JsonResponse( + {'success': False, 'message': 'No token provided'}, status=400 + ) + + user, valid = self._validate_token(encoded_token) + if not valid: + return JsonResponse( + {'success': False, 'message': 'Invalid or expired token'}, status=400 + ) + + subscribe = False + if request.body: + try: + data = json.loads(request.body) + subscribe = data.get('subscribe', False) + except json.JSONDecodeError: + return JsonResponse( + {'success': False, 'message': 'Invalid JSON data'}, status=400 + ) + + notification_preference = self.get_user_preference(user) + notification_preference.email = subscribe + notification_preference.save() + + status_message = 'subscribed' if subscribe else 'unsubscribed' + return JsonResponse( + {'success': True, 'message': f'Successfully {status_message}'} + ) + + def _validate_token(self, encoded_token): + try: + decoded_data = base64.urlsafe_b64decode(encoded_token).decode() + data = json.loads(decoded_data) + user_id = data.get('user_id') + token = data.get('token') + + user = User.objects.get(id=user_id) + if email_token_generator.check_token(user, token): + return user, True + except ( + User.DoesNotExist, + ValueError, + json.JSONDecodeError, + base64.binascii.Error, + ): + pass + + return None, False + + def get_user_preference(self, user): + # TODO: Should update this once the Notification Preferences Page PR is merged. + return NotificationSetting.objects.filter(user=user).first() diff --git a/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css b/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css new file mode 100644 index 00000000..4308d3d0 --- /dev/null +++ b/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css @@ -0,0 +1,67 @@ +body, +html { + height: 100%; + margin: 0; + background-color: #f0f4f8; +} +.container { + display: flex; + justify-content: center; + align-items: center; + flex-direction: column; + height: 100vh; +} +.content { + background: #ffffff; + padding: 40px; + border-radius: 12px; + text-align: center; + box-shadow: 0 6px 12px rgba(0, 0, 0, 0.1); + max-width: 500px; + width: 100%; +} +.logo { + width: 300px; + margin-bottom: 20px; +} +.icon { + font-size: 56px; + margin-bottom: 20px; +} +h1 { + font-size: 24px; +} +p { + color: #555; + font-size: 16px; + margin-bottom: 20px; + line-height: 1.5; +} +button { + background-color: #0077b5; + color: white; + padding: 10px 20px; + border: none; + border-radius: 5px; + cursor: pointer; + margin: 5px; +} +button:hover { + background-color: #005f8a; +} +a { + color: #0077b5; + text-decoration: none; + font-weight: bold; +} +a:hover { + text-decoration: underline; +} +.footer { + margin-top: 20px; +} +#confirmation-msg { + color: green; + margin-top: 20px; + font-weight: bold; +} diff --git a/openwisp_notifications/tasks.py b/openwisp_notifications/tasks.py index f1f6cdc4..b671122b 100644 --- a/openwisp_notifications/tasks.py +++ b/openwisp_notifications/tasks.py @@ -9,12 +9,16 @@ from django.db.utils import OperationalError from django.template.loader import render_to_string from django.utils import timezone +from django.utils.safestring import mark_safe from django.utils.translation import gettext as _ from openwisp_notifications import settings as app_settings from openwisp_notifications import types from openwisp_notifications.swapper import load_model, swapper_load_model -from openwisp_notifications.utils import send_notification_email +from openwisp_notifications.utils import ( + generate_unsubscribe_link, + send_notification_email, +) from openwisp_utils.admin_theme.email import send_email from openwisp_utils.tasks import OpenwispCeleryTask @@ -259,6 +263,8 @@ def send_batched_email_notifications(instance_id): .replace('pm', 'p.m.') ) + ' UTC' + user = User.objects.get(id=instance_id) + context = { 'notifications': unsent_notifications[:display_limit], 'notifications_count': notifications_count, @@ -266,10 +272,17 @@ def send_batched_email_notifications(instance_id): 'start_time': starting_time, } - extra_context = {} + unsubscribe_link = generate_unsubscribe_link(user) + + extra_context = { + 'footer': mark_safe( + 'To unsubscribe from these notifications, ' + f'click here.' + ), + } if notifications_count > display_limit: extra_context = { - 'call_to_action_url': f"https://{current_site.domain}/admin/#notifications", + 'call_to_action_url': f'https://{current_site.domain}/admin/#notifications', 'call_to_action_text': _('View all Notifications'), } context.update(extra_context) @@ -284,6 +297,10 @@ def send_batched_email_notifications(instance_id): body_html=html_content, recipients=[email_id], extra_context=extra_context, + headers={ + 'List-Unsubscribe-Post': 'List-Unsubscribe=One-Click', + 'List-Unsubscribe': f'<{unsubscribe_link}>', + }, ) unsent_notifications_query.update(emailed=True) diff --git a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html new file mode 100644 index 00000000..602581e4 --- /dev/null +++ b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html @@ -0,0 +1,90 @@ +{% load static %} + + + + + + + Manage Subscription Preferences + + + +
+ +
+
✉️
+

Manage Notification Preferences

+ {% if valid %} +

+ {% if is_subscribed %} + You are currently subscribed to notifications. + {% else %} + You are currently unsubscribed from notifications. + {% endif %} +

+ + + {% else %} +

Invalid or Expired Link

+

The link you used is invalid or expired. Please contact support.

+ {% endif %} +
+

Manage Other Preferences? Click here

+
+
+
+ + + + diff --git a/openwisp_notifications/urls.py b/openwisp_notifications/urls.py index efc6d2ba..a3916902 100644 --- a/openwisp_notifications/urls.py +++ b/openwisp_notifications/urls.py @@ -1,6 +1,7 @@ from django.urls import include, path from .api.urls import get_api_urls +from .base.views import UnsubscribeView def get_urls(api_views=None, social_views=None): @@ -10,7 +11,8 @@ def get_urls(api_views=None, social_views=None): api_views(optional): views for Notifications API """ urls = [ - path('api/v1/notifications/notification/', include(get_api_urls(api_views))) + path('api/v1/notifications/notification/', include(get_api_urls(api_views))), + path('unsubscribe/', UnsubscribeView.as_view(), name='unsubscribe'), ] return urls diff --git a/openwisp_notifications/utils.py b/openwisp_notifications/utils.py index a3778077..d920c81d 100644 --- a/openwisp_notifications/utils.py +++ b/openwisp_notifications/utils.py @@ -1,11 +1,17 @@ +import base64 +import json + from django.conf import settings from django.contrib.sites.models import Site from django.urls import NoReverseMatch, reverse +from django.utils.safestring import mark_safe from django.utils.translation import gettext as _ from openwisp_notifications.exceptions import NotificationRenderException from openwisp_utils.admin_theme.email import send_email +from .tokens import email_token_generator + def _get_object_link(obj, field, absolute_url=False, *args, **kwargs): related_obj = getattr(obj, field) @@ -53,6 +59,9 @@ def send_notification_email(notification): description += _('\n\nFor more information see %(target_url)s.') % { 'target_url': target_url } + + unsubscribe_link = generate_unsubscribe_link(notification.recipient) + send_email( subject, description, @@ -61,5 +70,22 @@ def send_notification_email(notification): extra_context={ 'call_to_action_url': target_url, 'call_to_action_text': _('Find out more'), + 'footer': mark_safe( + 'To unsubscribe from these notifications, ' + f'click here.' + ), + }, + headers={ + 'List-Unsubscribe-Post': 'List-Unsubscribe=One-Click', + 'List-Unsubscribe': f'<{unsubscribe_link}>', }, ) + + +def generate_unsubscribe_link(user): + token = email_token_generator.make_token(user) + data = json.dumps({'user_id': str(user.id), 'token': token}) + encoded_data = base64.urlsafe_b64encode(data.encode()).decode() + unsubscribe_url = reverse('notifications:unsubscribe') + current_site = Site.objects.get_current() + return f"https://{current_site.domain}{unsubscribe_url}?token={encoded_data}" From 6290e1671f3e275f685c06f839600204c2d68905 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Sun, 1 Sep 2024 13:50:09 +0530 Subject: [PATCH 03/17] [chore] Remove token time expiry --- openwisp_notifications/tokens.py | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/openwisp_notifications/tokens.py b/openwisp_notifications/tokens.py index c60460ef..170da853 100644 --- a/openwisp_notifications/tokens.py +++ b/openwisp_notifications/tokens.py @@ -6,18 +6,14 @@ class EmailTokenGenerator(PasswordResetTokenGenerator): """ Email token generator that extends the default PasswordResetTokenGenerator - with a fixed 7-day expiry period and a salt key. + without a fixed expiry period. """ key_salt = "openwisp_notifications.tokens.EmailTokenGenerator" - def __init__(self): - super().__init__() - self.expiry_days = 7 - def check_token(self, user, token): """ - Check that a token is correct for a given user and has not expired. + Check that a token is correct for a given user. """ if not (user and token): return False @@ -39,15 +35,9 @@ def check_token(self, user, token): self._make_token_with_timestamp(user, ts, secret), token, ): - break - else: - return False - - # Check the timestamp is within the expiry limit. - if (self._num_seconds(self._now()) - ts) > self._expiry_seconds(): - return False + return True - return True + return False def _make_hash_value(self, user, timestamp): """ @@ -58,11 +48,5 @@ def _make_hash_value(self, user, timestamp): email = getattr(user, email_field, "") or "" return f"{user.pk}{user.password}{timestamp}{email}" - def _expiry_seconds(self): - """ - Returns the number of seconds representing the token's expiry period. - """ - return self.expiry_days * 24 * 3600 - email_token_generator = EmailTokenGenerator() From cfff9aa7c92a1870d9c127328cff4394ab2028e2 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Sun, 1 Sep 2024 16:02:17 +0530 Subject: [PATCH 04/17] [chore] Handle logic for any one email setting type enabled even when global setting is disabled --- openwisp_notifications/base/views.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/openwisp_notifications/base/views.py b/openwisp_notifications/base/views.py index 0863b938..a8725355 100644 --- a/openwisp_notifications/base/views.py +++ b/openwisp_notifications/base/views.py @@ -29,7 +29,7 @@ def get(self, request, *args, **kwargs): if not valid: return render(request, self.template_name, {'valid': False}) - notification_preference = self.get_user_preference(user) + is_subscribed = self.get_user_preference(user) return render( request, @@ -37,7 +37,7 @@ def get(self, request, *args, **kwargs): { 'valid': True, 'user': user, - 'is_subscribed': notification_preference.email, + 'is_subscribed': is_subscribed, }, ) @@ -64,9 +64,7 @@ def post(self, request, *args, **kwargs): {'success': False, 'message': 'Invalid JSON data'}, status=400 ) - notification_preference = self.get_user_preference(user) - notification_preference.email = subscribe - notification_preference.save() + self.update_user_preferences(user, subscribe) status_message = 'subscribed' if subscribe else 'unsubscribed' return JsonResponse( @@ -94,5 +92,13 @@ def _validate_token(self, encoded_token): return None, False def get_user_preference(self, user): - # TODO: Should update this once the Notification Preferences Page PR is merged. - return NotificationSetting.objects.filter(user=user).first() + """ + Check if any of the user's notification settings have email notifications enabled. + """ + return NotificationSetting.objects.filter(user=user, email=True).exists() + + def update_user_preferences(self, user, subscribe): + """ + Update all of the user's notification settings to set email preference. + """ + NotificationSetting.objects.filter(user=user).update(email=subscribe) From 286b22bd3c4be9532d1a5ce08498957770ef68b8 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Sun, 1 Sep 2024 16:21:39 +0530 Subject: [PATCH 05/17] [chore] Translatable i18n and js file refactor --- .../openwisp-notifications/js/unsubscribe.js | 46 +++++++++++++ .../openwisp_notifications/unsubscribe.html | 69 ++++--------------- 2 files changed, 61 insertions(+), 54 deletions(-) create mode 100644 openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js diff --git a/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js b/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js new file mode 100644 index 00000000..f3ae796f --- /dev/null +++ b/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js @@ -0,0 +1,46 @@ +if (typeof gettext === 'undefined') { + var gettext = function(word) { return word; }; +} + +function updateSubscription(subscribe) { + const token = '{{ request.GET.token }}'; + + fetch(window.location.href, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ subscribe: subscribe }) + }) + .then(response => response.json()) + .then(data => { + if (data.success) { + const toggleBtn = document.getElementById('toggle-btn'); + const statusMessage = document.getElementById('status-message'); + const confirmationMsg = document.getElementById('confirmation-msg'); + + if (subscribe) { + statusMessage.textContent = gettext('You are currently subscribed to notifications.'); + toggleBtn.textContent = gettext('Unsubscribe'); + confirmationMsg.textContent = data.message; + } else { + statusMessage.textContent = gettext('You are currently unsubscribed from notifications.'); + toggleBtn.textContent = gettext('Subscribe'); + confirmationMsg.textContent = data.message; + } + + confirmationMsg.style.display = 'block'; + } else { + alert(data.message); + } + }); +} + +document.addEventListener('DOMContentLoaded', function() { + const toggleBtn = document.getElementById('toggle-btn'); + toggleBtn.addEventListener('click', function() { + const currentStatus = toggleBtn.textContent.trim().toLowerCase(); + const subscribe = currentStatus === gettext('subscribe').toLowerCase(); + updateSubscription(subscribe); + }); +}); diff --git a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html index 602581e4..a699aba5 100644 --- a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html +++ b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html @@ -1,90 +1,51 @@ {% load static %} +{% load i18n %} - + - Manage Subscription Preferences + {% trans 'Manage Subscription Preferences' %}
✉️
-

Manage Notification Preferences

+

{% trans 'Manage Notification Preferences' %}

{% if valid %}

{% if is_subscribed %} - You are currently subscribed to notifications. + {% trans 'You are currently subscribed to notifications.' %} {% else %} - You are currently unsubscribed from notifications. + {% trans 'You are currently unsubscribed from notifications.' %} {% endif %}

{% else %} -

Invalid or Expired Link

-

The link you used is invalid or expired. Please contact support.

+

{% trans 'Invalid or Expired Link' %}

+

{% trans 'The link you used is invalid or expired. Please contact support.' %}

{% endif %}
-

Manage Other Preferences? Click here

+

+ {% trans 'Manage Other Preferences?' %} {% trans 'Click here' %} +

- - + From b82f4417fe52b675073228b53569b1e72498d469 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Sun, 1 Sep 2024 19:35:45 +0530 Subject: [PATCH 06/17] [chore] Add tests --- .../openwisp-notifications/js/unsubscribe.js | 5 ++--- .../tests/test_notifications.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js b/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js index f3ae796f..5926f5b0 100644 --- a/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js +++ b/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js @@ -1,10 +1,9 @@ +'use strict'; if (typeof gettext === 'undefined') { var gettext = function(word) { return word; }; } function updateSubscription(subscribe) { - const token = '{{ request.GET.token }}'; - fetch(window.location.href, { method: 'POST', headers: { @@ -31,7 +30,7 @@ function updateSubscription(subscribe) { confirmationMsg.style.display = 'block'; } else { - alert(data.message); + window.alert(data.message); } }); } diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index 555eb060..940b09f5 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -30,6 +30,7 @@ register_notification_type, unregister_notification_type, ) +from openwisp_notifications.tokens import email_token_generator from openwisp_notifications.types import ( _unregister_notification_choice, get_notification_configuration, @@ -1047,6 +1048,24 @@ def test_that_the_notification_is_only_sent_once_to_the_user(self): self._create_notification() self.assertEqual(notification_queryset.count(), 1) + def test_email_unsubscribe_token(self): + token = email_token_generator.make_token(self.admin) + + with self.subTest('Valid token for the user'): + is_valid = email_token_generator.check_token(self.admin, token) + self.assertTrue(is_valid) + + with self.subTest('Token used with a different user'): + test_user = self._create_user(username='test') + is_valid = email_token_generator.check_token(test_user, token) + self.assertFalse(is_valid) + + with self.subTest('Token invalidated after password change'): + self.admin.set_password('new_password') + self.admin.save() + is_valid = email_token_generator.check_token(self.admin, token) + self.assertFalse(is_valid) + class TestTransactionNotifications(TestOrganizationMixin, TransactionTestCase): def setUp(self): From 3b962557b98b053b6dd2292382cb0a334f06b43f Mon Sep 17 00:00:00 2001 From: Dhanus Date: Sun, 1 Sep 2024 20:34:29 +0530 Subject: [PATCH 07/17] [chore] Handle check_token function for older django version --- openwisp_notifications/tokens.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/openwisp_notifications/tokens.py b/openwisp_notifications/tokens.py index 170da853..53da6b83 100644 --- a/openwisp_notifications/tokens.py +++ b/openwisp_notifications/tokens.py @@ -30,11 +30,17 @@ def check_token(self, user, token): return False # Check that the timestamp/uid has not been tampered with - for secret in [self.secret, *self.secret_fallbacks]: - if constant_time_compare( - self._make_token_with_timestamp(user, ts, secret), - token, - ): + if hasattr(self, 'secret_fallbacks'): + # For newer Django versions + for secret in [self.secret, *self.secret_fallbacks]: + if constant_time_compare( + self._make_token_with_timestamp(user, ts, secret), + token, + ): + return True + else: + # For older Django versions + if constant_time_compare(self._make_token_with_timestamp(user, ts), token): return True return False From 27952c7c4d0c5c8194a9df6acfb202ad37ebed34 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Sun, 1 Sep 2024 20:39:32 +0530 Subject: [PATCH 08/17] [ci] Add notification-preferences target branches PR to build actions --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 51b4f151..9dff2b0f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,6 +11,7 @@ on: - master - dev - gsoc24-rebased + - notification-preferences jobs: From 90a60562fe1f6fbe555fe0ffcb030efaa92330de Mon Sep 17 00:00:00 2001 From: Dhanus Date: Thu, 5 Sep 2024 11:39:49 +0530 Subject: [PATCH 09/17] [chore] Bump changes --- .../css/unsubscribe.css | 16 ++--- .../openwisp_notifications/unsubscribe.html | 4 +- openwisp_notifications/urls.py | 2 +- openwisp_notifications/utils.py | 5 +- openwisp_notifications/views.py | 65 +++++++++---------- tests/openwisp2/settings.py | 1 + 6 files changed, 43 insertions(+), 50 deletions(-) diff --git a/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css b/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css index 4308d3d0..85fa1957 100644 --- a/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css +++ b/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css @@ -21,7 +21,7 @@ html { width: 100%; } .logo { - width: 300px; + width: 150px; margin-bottom: 20px; } .icon { @@ -33,12 +33,11 @@ h1 { } p { color: #555; - font-size: 16px; margin-bottom: 20px; line-height: 1.5; } button { - background-color: #0077b5; + background-color: #333; color: white; padding: 10px 20px; border: none; @@ -47,15 +46,8 @@ button { margin: 5px; } button:hover { - background-color: #005f8a; -} -a { - color: #0077b5; - text-decoration: none; - font-weight: bold; -} -a:hover { - text-decoration: underline; + opacity: 0.7; + border-color: #333; } .footer { margin-top: 20px; diff --git a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html index aeb3d57f..62fb0325 100644 --- a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html +++ b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html @@ -7,6 +7,8 @@ {% trans 'Manage Subscription Preferences' %} + + @@ -37,7 +39,7 @@

{% trans 'Manage Notification Preferences' %}

{% else %}

{% trans 'Invalid or Expired Link' %}

-

{% trans 'The link you used is invalid or expired. Please contact support.' %}

+

{% trans 'The link you used is invalid or expired.' %}

{% endif %}

diff --git a/openwisp_notifications/urls.py b/openwisp_notifications/urls.py index ad708372..c303c3a1 100644 --- a/openwisp_notifications/urls.py +++ b/openwisp_notifications/urls.py @@ -22,7 +22,7 @@ def get_urls(api_views=None, social_views=None): notification_preference_view, name='user_notification_preference', ), - path('unsubscribe/', unsubscribe_view, name='unsubscribe'), + path('notifications/unsubscribe/', unsubscribe_view, name='unsubscribe'), ] return urls diff --git a/openwisp_notifications/utils.py b/openwisp_notifications/utils.py index d920c81d..64d5ac21 100644 --- a/openwisp_notifications/utils.py +++ b/openwisp_notifications/utils.py @@ -1,9 +1,10 @@ -import base64 import json from django.conf import settings from django.contrib.sites.models import Site from django.urls import NoReverseMatch, reverse +from django.utils.encoding import force_bytes +from django.utils.http import urlsafe_base64_encode from django.utils.safestring import mark_safe from django.utils.translation import gettext as _ @@ -85,7 +86,7 @@ def send_notification_email(notification): def generate_unsubscribe_link(user): token = email_token_generator.make_token(user) data = json.dumps({'user_id': str(user.id), 'token': token}) - encoded_data = base64.urlsafe_b64encode(data.encode()).decode() + encoded_data = urlsafe_base64_encode(force_bytes(data)) unsubscribe_url = reverse('notifications:unsubscribe') current_site = Site.objects.get_current() return f"https://{current_site.domain}{unsubscribe_url}?token={encoded_data}" diff --git a/openwisp_notifications/views.py b/openwisp_notifications/views.py index b243f8eb..11d95ded 100644 --- a/openwisp_notifications/views.py +++ b/openwisp_notifications/views.py @@ -6,6 +6,7 @@ from django.http import Http404, JsonResponse from django.shortcuts import render from django.utils.decorators import method_decorator +from django.utils.http import urlsafe_base64_decode from django.utils.translation import gettext as _ from django.views.decorators.csrf import csrf_exempt from django.views.generic import TemplateView @@ -57,52 +58,48 @@ def test_func(self): class UnsubscribeView(TemplateView): template_name = 'openwisp_notifications/unsubscribe.html' - def get(self, request, *args, **kwargs): - encoded_token = request.GET.get('token') - if not encoded_token: + def dispatch(self, request, *args, **kwargs): + self.encoded_token = request.GET.get('token') + if not self.encoded_token: + if request.method == 'POST': + return JsonResponse( + {'success': False, 'message': 'No token provided'}, status=400 + ) return render(request, self.template_name, {'valid': False}) - user, valid = self._validate_token(encoded_token) - if not valid: + self.user, self.valid = self._validate_token(self.encoded_token) + if not self.valid: + if request.method == 'POST': + return JsonResponse( + {'success': False, 'message': 'Invalid or expired token'}, + status=400, + ) return render(request, self.template_name, {'valid': False}) - is_subscribed = self.get_user_preference(user) + return super().dispatch(request, *args, **kwargs) - return render( - request, - self.template_name, + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + is_subscribed = self.get_user_preference(self.user) if self.valid else False + context.update( { - 'valid': True, - 'user': user, + 'valid': self.valid, + 'user': self.user, 'is_subscribed': is_subscribed, - }, + } ) + return context def post(self, request, *args, **kwargs): - encoded_token = request.GET.get('token') - if not encoded_token: - return JsonResponse( - {'success': False, 'message': 'No token provided'}, status=400 - ) - - user, valid = self._validate_token(encoded_token) - if not valid: + try: + data = json.loads(request.body) + subscribe = data.get('subscribe', False) + except json.JSONDecodeError: return JsonResponse( - {'success': False, 'message': 'Invalid or expired token'}, status=400 + {'success': False, 'message': 'Invalid JSON data'}, status=400 ) - subscribe = False - if request.body: - try: - data = json.loads(request.body) - subscribe = data.get('subscribe', False) - except json.JSONDecodeError: - return JsonResponse( - {'success': False, 'message': 'Invalid JSON data'}, status=400 - ) - - self.update_user_preferences(user, subscribe) - + self.update_user_preferences(self.user, subscribe) status_message = 'subscribed' if subscribe else 'unsubscribed' return JsonResponse( {'success': True, 'message': f'Successfully {status_message}'} @@ -110,7 +107,7 @@ def post(self, request, *args, **kwargs): def _validate_token(self, encoded_token): try: - decoded_data = base64.urlsafe_b64decode(encoded_token).decode() + decoded_data = urlsafe_base64_decode(encoded_token).decode('utf-8') data = json.loads(decoded_data) user_id = data.get('user_id') token = data.get('token') diff --git a/tests/openwisp2/settings.py b/tests/openwisp2/settings.py index 89394685..291c3392 100644 --- a/tests/openwisp2/settings.py +++ b/tests/openwisp2/settings.py @@ -27,6 +27,7 @@ 'django.contrib.staticfiles', 'django.contrib.sites', 'django_extensions', + 'openwisp_users.accounts', 'allauth', 'allauth.account', 'allauth.socialaccount', From 735c4d0518d8a2bd8a9f2e528ee3ef07e1cbe56c Mon Sep 17 00:00:00 2001 From: Dhanus Date: Thu, 5 Sep 2024 23:25:34 +0530 Subject: [PATCH 10/17] [chore] Add tests --- .../tests/test_notifications.py | 92 ++++++++++++++++++- openwisp_notifications/views.py | 8 +- 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index 619e4160..8fde9f01 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -1,3 +1,4 @@ +import json from datetime import datetime, timedelta from unittest.mock import patch from uuid import uuid4 @@ -36,7 +37,7 @@ _unregister_notification_choice, get_notification_configuration, ) -from openwisp_notifications.utils import _get_absolute_url +from openwisp_notifications.utils import _get_absolute_url, generate_unsubscribe_link from openwisp_users.tests.utils import TestOrganizationMixin from openwisp_utils.tests import capture_any_output @@ -1072,6 +1073,95 @@ def test_email_unsubscribe_token(self): is_valid = email_token_generator.check_token(self.admin, token) self.assertFalse(is_valid) + def test_email_unsubscribe_view(self): + unsubscribe_link_generated = generate_unsubscribe_link(self.admin) + token = unsubscribe_link_generated.split('?token=')[1] + local_unsubscribe_url = reverse('notifications:unsubscribe') + unsubscribe_url = f"{local_unsubscribe_url}?token={token}" + + with self.subTest('Test GET request with valid token'): + response = self.client.get(unsubscribe_url) + self.assertEqual(response.status_code, 200) + + with self.subTest('Test POST request with valid token'): + response = self.client.post(unsubscribe_url) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()['message'], 'Successfully unsubscribed') + + with self.subTest('Test GET request with invalid token'): + response = self.client.get(f"{local_unsubscribe_url}?token=invalid_token") + self.assertContains(response, 'Invalid or Expired Link') + self.assertEqual(response.status_code, 200) + + with self.subTest('Test POST request with invalid token'): + response = self.client.post(f"{local_unsubscribe_url}?token=invalid_token") + self.assertEqual(response.status_code, 400) + + with self.subTest('Test GET request with invalid user'): + tester = self._create_user(username='tester') + tester_link_generated = generate_unsubscribe_link(tester) + token = tester_link_generated.split('?token=')[1] + tester_unsubscribe_url = f"{local_unsubscribe_url}?token={token}" + tester.delete() + response = self.client.get(tester_unsubscribe_url) + self.assertContains(response, 'Invalid or Expired Link') + self.assertEqual(response.status_code, 200) + + with self.subTest('Test POST request with invalid user'): + tester = self._create_user(username='tester') + tester_link_generated = generate_unsubscribe_link(tester) + token = tester_link_generated.split('?token=')[1] + tester_unsubscribe_url = f"{local_unsubscribe_url}?token={token}" + tester.delete() + response = self.client.post(tester_unsubscribe_url) + self.assertEqual(response.status_code, 400) + + with self.subTest('Test GET request with no token'): + response = self.client.get(local_unsubscribe_url) + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Invalid or Expired Link') + self.assertFalse(response.context['valid']) + + with self.subTest('Test POST request with no token'): + response = self.client.post(local_unsubscribe_url) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json()['message'], 'No token provided') + + with self.subTest('Test POST request with empty JSON body'): + response = self.client.post( + unsubscribe_url, content_type='application/json' + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()['message'], 'Successfully unsubscribed') + + with self.subTest('Test POST request with subscribe=True in JSON'): + response = self.client.post( + unsubscribe_url, + data=json.dumps({'subscribe': True}), + content_type='application/json', + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()['message'], 'Successfully subscribed') + + with self.subTest('Test POST request with subscribe=False in JSON'): + response = self.client.post( + unsubscribe_url, + data=json.dumps({'subscribe': False}), + content_type='application/json', + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()['message'], 'Successfully unsubscribed') + + with self.subTest('Test POST request with invalid JSON'): + invalid_json = "{'data: invalid}" + response = self.client.post( + unsubscribe_url, + data=invalid_json, + content_type='application/json', + ) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json()['message'], 'Invalid JSON data') + def test_notification_preference_page(self): preference_page = 'notifications:user_notification_preference' tester = self._create_user(username='tester') diff --git a/openwisp_notifications/views.py b/openwisp_notifications/views.py index 11d95ded..79931e96 100644 --- a/openwisp_notifications/views.py +++ b/openwisp_notifications/views.py @@ -92,8 +92,12 @@ def get_context_data(self, **kwargs): def post(self, request, *args, **kwargs): try: - data = json.loads(request.body) - subscribe = data.get('subscribe', False) + if request.content_type == 'application/json': + data = json.loads(request.body) + subscribe = data.get('subscribe', False) + else: + # Unsubscribe by default + subscribe = False except json.JSONDecodeError: return JsonResponse( {'success': False, 'message': 'Invalid JSON data'}, status=400 From 7528ed71e314d2ec5cf3042bdbc7030cd9eaa468 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Fri, 6 Sep 2024 12:30:02 +0530 Subject: [PATCH 11/17] [chore] Add selenium tests --- .../tests/test_notifications.py | 2 +- openwisp_notifications/tests/test_selenium.py | 65 +++++++++++++++++++ openwisp_notifications/utils.py | 4 +- 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 openwisp_notifications/tests/test_selenium.py diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index 8fde9f01..d000c57e 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -1074,7 +1074,7 @@ def test_email_unsubscribe_token(self): self.assertFalse(is_valid) def test_email_unsubscribe_view(self): - unsubscribe_link_generated = generate_unsubscribe_link(self.admin) + unsubscribe_link_generated = generate_unsubscribe_link(self.admin, False) token = unsubscribe_link_generated.split('?token=')[1] local_unsubscribe_url = reverse('notifications:unsubscribe') unsubscribe_url = f"{local_unsubscribe_url}?token={token}" diff --git a/openwisp_notifications/tests/test_selenium.py b/openwisp_notifications/tests/test_selenium.py new file mode 100644 index 00000000..fc231bfc --- /dev/null +++ b/openwisp_notifications/tests/test_selenium.py @@ -0,0 +1,65 @@ +from django.contrib.staticfiles.testing import StaticLiveServerTestCase +from selenium.webdriver.common.by import By +from selenium.webdriver.support import expected_conditions as EC +from selenium.webdriver.support.ui import WebDriverWait + +from openwisp_notifications.utils import generate_unsubscribe_link +from openwisp_users.tests.utils import TestOrganizationMixin +from openwisp_utils.test_selenium_mixins import SeleniumTestMixin + + +class TestSelenium( + SeleniumTestMixin, + TestOrganizationMixin, + StaticLiveServerTestCase, +): + serve_static = True + + def setUp(self): + self.admin = self._create_admin( + username=self.admin_username, password=self.admin_password + ) + self.driver = self.web_driver + + def open(self, url, driver=None): + driver = self.driver or self.web_driver + driver.get(f'{self.live_server_url}{url}') + + def test_email_unsubscribe_page(self): + unsubscribe_link = generate_unsubscribe_link(self.admin, False) + self.open(unsubscribe_link) + WebDriverWait(self.web_driver, 10).until( + EC.visibility_of_element_located((By.ID, 'toggle-btn')) + ) + self.assertEqual( + self.driver.find_element(By.ID, 'toggle-btn').text, + 'Unsubscribe', + ) + + # Unsubscribe + self.driver.find_element(By.ID, 'toggle-btn').click() + WebDriverWait(self.web_driver, 10).until( + EC.visibility_of_element_located((By.ID, 'confirmation-msg')) + ) + self.assertEqual( + self.driver.find_element(By.ID, 'confirmation-msg').text, + 'Successfully unsubscribed', + ) + self.assertEqual( + self.driver.find_element(By.ID, 'toggle-btn').text, + 'Subscribe', + ) + + # Re-subscribe + self.driver.find_element(By.ID, 'toggle-btn').click() + WebDriverWait(self.web_driver, 10).until( + EC.visibility_of_element_located((By.ID, 'confirmation-msg')) + ) + self.assertEqual( + self.driver.find_element(By.ID, 'confirmation-msg').text, + 'Successfully subscribed', + ) + self.assertEqual( + self.driver.find_element(By.ID, 'toggle-btn').text, + 'Unsubscribe', + ) diff --git a/openwisp_notifications/utils.py b/openwisp_notifications/utils.py index 64d5ac21..31080695 100644 --- a/openwisp_notifications/utils.py +++ b/openwisp_notifications/utils.py @@ -83,10 +83,12 @@ def send_notification_email(notification): ) -def generate_unsubscribe_link(user): +def generate_unsubscribe_link(user, full_url=True): token = email_token_generator.make_token(user) data = json.dumps({'user_id': str(user.id), 'token': token}) encoded_data = urlsafe_base64_encode(force_bytes(data)) unsubscribe_url = reverse('notifications:unsubscribe') + if not full_url: + return f"{unsubscribe_url}?token={encoded_data}" current_site = Site.objects.get_current() return f"https://{current_site.domain}{unsubscribe_url}?token={encoded_data}" From 2f802be60ef7159ae5331518dd3cc8c9ce242f9f Mon Sep 17 00:00:00 2001 From: Dhanus Date: Sat, 7 Sep 2024 17:29:30 +0530 Subject: [PATCH 12/17] [chore] Bump changes --- .../static/openwisp-notifications/js/unsubscribe.js | 13 +++++++++---- .../openwisp_notifications/unsubscribe.html | 2 +- openwisp_notifications/views.py | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js b/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js index 5926f5b0..ef65cd5f 100644 --- a/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js +++ b/openwisp_notifications/static/openwisp-notifications/js/unsubscribe.js @@ -21,25 +21,30 @@ function updateSubscription(subscribe) { if (subscribe) { statusMessage.textContent = gettext('You are currently subscribed to notifications.'); toggleBtn.textContent = gettext('Unsubscribe'); - confirmationMsg.textContent = data.message; + toggleBtn.setAttribute('data-hasSubscribe', 'true'); + confirmationMsg.textContent = 'Successfully subscribed!'; } else { statusMessage.textContent = gettext('You are currently unsubscribed from notifications.'); toggleBtn.textContent = gettext('Subscribe'); - confirmationMsg.textContent = data.message; + toggleBtn.setAttribute('data-hasSubscribe', 'false'); + confirmationMsg.textContent = 'Successfully unsubscribed!'; } confirmationMsg.style.display = 'block'; } else { window.alert(data.message); } + }) + .catch(error => { + console.error('Error:', error); }); } document.addEventListener('DOMContentLoaded', function() { const toggleBtn = document.getElementById('toggle-btn'); toggleBtn.addEventListener('click', function() { - const currentStatus = toggleBtn.textContent.trim().toLowerCase(); - const subscribe = currentStatus === gettext('subscribe').toLowerCase(); + const isSubscribed = toggleBtn.getAttribute('data-hasSubscribe') === 'true'; + const subscribe = !isSubscribed; updateSubscription(subscribe); }); }); diff --git a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html index 62fb0325..41438570 100644 --- a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html +++ b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html @@ -29,7 +29,7 @@

{% trans 'Manage Notification Preferences' %}

{% trans 'You are currently unsubscribed from notifications.' %} {% endif %}

- - - {% else %} -

{% trans 'Invalid or Expired Link' %}

-

{% trans 'The link you used is invalid or expired.' %}

- {% endif %} -
-

- {% trans 'Manage Other Preferences?' %} {% trans 'Click here' %} -

-
+{% block content %} +
+ +
+
+

{% trans 'Manage Notification Preferences' %}

+ {% if valid %} +

+ {% if is_subscribed %} + {% trans 'You are currently subscribed to notifications.' %} + {% else %} + {% trans 'You are currently unsubscribed from notifications.' %} + {% endif %} +

+ + + {% else %} +

{% trans 'Invalid or Expired Link' %}

+

{% trans 'The link you used is invalid or expired.' %}

+ {% endif %} +
+

+ {% trans 'Manage Other Preferences?' %} {% trans 'Click here' %} +

- - - +
+{% endblock %} diff --git a/openwisp_notifications/views.py b/openwisp_notifications/views.py index 9c1bb757..d7503c6c 100644 --- a/openwisp_notifications/views.py +++ b/openwisp_notifications/views.py @@ -84,7 +84,6 @@ def get_context_data(self, **kwargs): context.update( { 'valid': self.valid, - 'user': self.user, 'is_subscribed': is_subscribed, } ) From 4171bd40191a997adc3326381a07e2ed15c61120 Mon Sep 17 00:00:00 2001 From: Dhanus Date: Wed, 2 Oct 2024 21:22:32 +0530 Subject: [PATCH 16/17] [chore] CSS changes --- .../css/unsubscribe.css | 31 +++---------------- .../openwisp_notifications/unsubscribe.html | 4 +-- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css b/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css index ac6b7e0d..d81b769d 100644 --- a/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css +++ b/openwisp_notifications/static/openwisp-notifications/css/unsubscribe.css @@ -1,5 +1,5 @@ #content, .content { - padding: 0 !important; + padding: 0 !important; } .unsubscribe-container { display: flex; @@ -7,10 +7,8 @@ align-items: center; flex-direction: column; height: 95vh; - overflow: none; } .unsubscribe-content { - background: #ffffff; padding: 40px; border-radius: 12px; text-align: center; @@ -19,39 +17,18 @@ width: 100%; } .unsubscribe-content h1 { - padding-top: 20px; + padding-top: 10px; } .logo { width: 200px; margin-bottom: 80px; - background-repeat: no-repeat; } -.icon { +.email-icon { background-image: url('../../openwisp-notifications/images/icons/icon-email.svg'); width: 50px; height: 50px; margin: 0 auto; -} -h1 { - font-size: 24px; -} -p { - color: #555; - margin-bottom: 20px; - line-height: 1.5; -} -button { - background-color: #333; - color: white; - padding: 10px 20px; - border: none; - border-radius: 5px; - cursor: pointer; - margin: 5px; -} -button:hover { - opacity: 0.7; - border-color: #333; + transform: scale(2) translate(25%, 25%); } .footer { margin-top: 20px; diff --git a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html index de511694..dbb54c7b 100644 --- a/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html +++ b/openwisp_notifications/templates/openwisp_notifications/unsubscribe.html @@ -23,7 +23,7 @@ class="logo" />
-
+

{% trans 'Manage Notification Preferences' %}

{% if valid %}

@@ -33,7 +33,7 @@

{% trans 'Manage Notification Preferences' %}

{% trans 'You are currently unsubscribed from notifications.' %} {% endif %}

-