From 6185f14807bf527652cd928bf040fd7c596f77ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 18 May 2022 08:32:30 +0000 Subject: [PATCH] [REF] mass_mailing: rename hash methods PURPOSE Overall cleaning of subscription and exclusion management code from portal. SPECIFICATIONS In this commit we rename methods generating tokens (for recipients and report management). Otherwise it is hard to distinguish them as naming is not really convenient to use * _unsubscribe_token -> was actually generating token for a given recipient on a given mailing; * _get_unsubscribe_token -> was actually generating token for mailing users to deactivate D+1 KPI reporting; Hash for mail tracking is also moved in its own method to avoid having the same code twice. Better delegate it to a tool sub method. Task-2710804 (Mail: Clean MailThread API) Task-2669037 (Mass Mailing: Subscription page code cleaning) Prepares Task-2150462 (Mass Mailing: Unsubscribe flow refactoring) Part-of: odoo/odoo#106568 --- addons/mass_mailing/controllers/main.py | 7 +++-- addons/mass_mailing/models/mail_mail.py | 11 +++++-- addons/mass_mailing/models/mailing.py | 31 +++++++++---------- .../test_mail_full/tests/test_mass_mailing.py | 4 +-- 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/addons/mass_mailing/controllers/main.py b/addons/mass_mailing/controllers/main.py index 839be83a5c7..73ec95a0c06 100644 --- a/addons/mass_mailing/controllers/main.py +++ b/addons/mass_mailing/controllers/main.py @@ -18,7 +18,7 @@ class MassMailController(http.Controller): if not (mailing_id and res_id and email and token): return False mailing = request.env['mailing.mailing'].sudo().browse(mailing_id) - return consteq(mailing._unsubscribe_token(res_id, email), token) + return consteq(mailing._generate_mailing_recipient_token(res_id, email), token) # ------------------------------------------------------------ # SUBSCRIPTION MANAGEMENT @@ -118,7 +118,8 @@ class MassMailController(http.Controller): @http.route('/mail/track///blank.gif', type='http', auth='public') def track_mail_open(self, mail_id, token, **post): """ Email tracking. """ - if not consteq(token, tools.hmac(request.env(su=True), 'mass_mailing-mail_mail-open', mail_id)): + mail = request.env['mail.mail'].sudo().browse(mail_id).exists() + if not mail or not consteq(token, mail._generate_mail_recipient_token()): raise BadRequest() request.env['mailing.trace'].sudo().set_opened(domain=[('mail_mail_id_int', 'in', [mail_id])]) @@ -151,7 +152,7 @@ class MassMailController(http.Controller): if not token or not user_id: raise werkzeug.exceptions.NotFound() user_id = int(user_id) - correct_token = consteq(token, request.env['mailing.mailing']._get_unsubscribe_token(user_id)) + correct_token = consteq(token, request.env['mailing.mailing']._generate_mailing_report_token(user_id)) user = request.env['res.users'].sudo().browse(user_id) if correct_token and user.has_group('mass_mailing.group_mass_mailing_user'): request.env['ir.config_parameter'].sudo().set_param('mass_mailing.mass_mailing_reports', False) diff --git a/addons/mass_mailing/models/mail_mail.py b/addons/mass_mailing/models/mail_mail.py index 71b4c2b6a8d..f0a64a3e961 100644 --- a/addons/mass_mailing/models/mail_mail.py +++ b/addons/mass_mailing/models/mail_mail.py @@ -25,8 +25,15 @@ class MailMail(models.Model): return mails def _get_tracking_url(self): - token = tools.hmac(self.env(su=True), 'mass_mailing-mail_mail-open', self.id) - return werkzeug.urls.url_join(self.get_base_url(), 'mail/track/%s/%s/blank.gif' % (self.id, token)) + token = self._generate_mail_recipient_token() + return werkzeug.urls.url_join( + self.get_base_url(), + f'mail/track/{self.id}/{token}/blank.gif' + ) + + def _generate_mail_recipient_token(self): + self.ensure_one() + return tools.hmac(self.env(su=True), 'mass_mailing-mail_mail-open', self.id) def _send_prepare_body(self): """ Override to add the tracking URL to the body and to add diff --git a/addons/mass_mailing/models/mailing.py b/addons/mass_mailing/models/mailing.py index 00ccf892457..3f7ca056b12 100644 --- a/addons/mass_mailing/models/mailing.py +++ b/addons/mass_mailing/models/mailing.py @@ -1022,7 +1022,7 @@ class MassMailing(models.Model): 'params': werkzeug.urls.url_encode({ 'res_id': res_id, 'email': email_to, - 'token': self._unsubscribe_token(res_id, email_to), + 'token': self._generate_mailing_recipient_token(res_id, email_to), }), } ) @@ -1035,7 +1035,7 @@ class MassMailing(models.Model): 'params': werkzeug.urls.url_encode({ 'res_id': res_id, 'email': email_to, - 'token': self._unsubscribe_token(res_id, email_to), + 'token': self._generate_mailing_recipient_token(res_id, email_to), }), } ) @@ -1167,7 +1167,7 @@ class MassMailing(models.Model): ** mailing._prepare_statistics_email_values(), } if mail_user.has_group('mass_mailing.group_mass_mailing_user'): - rendering_data['mailing_report_token'] = self._get_unsubscribe_token(mail_user.id) + rendering_data['mailing_report_token'] = self._generate_mailing_report_token(mail_user.id) rendering_data['user_id'] = mail_user.id rendered_body = self.env['ir.qweb']._render( @@ -1267,8 +1267,8 @@ class MassMailing(models.Model): def _get_pretty_mailing_type(self): return _('Emails') - def _get_unsubscribe_token(self, user_id): - """Generate a secure hash for this user. It allows to opt out from + def _generate_mailing_report_token(self, user_id): + """Generate a secure token for this user. It allows to opt out from mailing reports while keeping some security in that process. """ return tools.hmac(self.env(su=True), 'mailing-report-deactivated', user_id) @@ -1294,21 +1294,18 @@ class MassMailing(models.Model): mailing_domain = [('id', 'in', [])] return mailing_domain - def _unsubscribe_token(self, res_id, email): - """Generate a secure hash for this mailing list and parameters. + def _generate_mailing_recipient_token(self, document_id, email): + """Generate a secure token for a given mailing and recipient (based on + their email). This allows notably to unsubscribe from the mailing or + to blacklist their email entirely without need of a user account. - This is appended to the unsubscription URL and then checked at - unsubscription time to ensure no malicious unsubscriptions are - performed. - - :param int res_id: - ID of the resource that will be unsubscribed. - - :param str email: - Email of the resource that will be unsubscribed. + :param int document_id: ID of the business document on which mailing + is performed; + :param str email: recipient email, used to unsubscribe / blacklist; """ + self.ensure_one() secret = self.env["ir.config_parameter"].sudo().get_param("database.secret") - token = (self.env.cr.dbname, self.id, int(res_id), tools.ustr(email)) + token = (self.env.cr.dbname, self.id, int(document_id), tools.ustr(email)) return hmac.new(secret.encode('utf-8'), repr(token).encode('utf-8'), hashlib.sha512).hexdigest() def _convert_inline_images_to_urls(self, body_html): diff --git a/addons/test_mail_full/tests/test_mass_mailing.py b/addons/test_mail_full/tests/test_mass_mailing.py index b7d2b15313a..fa93f5f1352 100644 --- a/addons/test_mail_full/tests/test_mass_mailing.py +++ b/addons/test_mail_full/tests/test_mass_mailing.py @@ -82,7 +82,7 @@ class TestMassMailing(TestMailFullCommon): self.assertEqual(int(unsubscribe_params['res_id']), recipient.id) self.assertEqual(unsubscribe_params['email'], recipient.email_normalized) self.assertEqual( - mailing._unsubscribe_token(unsubscribe_params['res_id'], (unsubscribe_params['email'])), + mailing._generate_mailing_recipient_token(unsubscribe_params['res_id'], (unsubscribe_params['email'])), unsubscribe_params['token'] ) # rendered view @@ -95,7 +95,7 @@ class TestMassMailing(TestMailFullCommon): self.assertEqual(int(view_params['res_id']), recipient.id) self.assertEqual(view_params['email'], recipient.email_normalized) self.assertEqual( - mailing._unsubscribe_token(view_params['res_id'], (view_params['email'])), + mailing._generate_mailing_recipient_token(view_params['res_id'], (view_params['email'])), view_params['token'] )