[REF] mail: remove 'alias_user_id'

'alias_user_id' field allows to set a user when creating records through the
mail gateway. This is however quite wrong and eases spoofing. The alias owner
is not the creator of any record, nor responsible.

Current possible ways of being owner / responsible of records created through
the mailgateway

  * when you send an email to an alias: if you are recognized you are already
    set as creating user and logged message author;
  * it is possible to use alias_defaults notably to set fields like 'user_id'
    if you want to be notified / responsible of records created through this
    specific alias;

Those usages are therefore sufficient, no need to have another way to spoof
users. Moreover it is hidden in technical view of aliases, no model allows
to configure it by default. Moreover since odoo/odoo@3edf181 no default
value is given to alias_user_id as it adds more (ACLs / creator) issues than
really helping setting up mail gateway flows.

Task-3453482
Prepares Task-36879 (Mail: Multi-Domain Aliases)

closes odoo/odoo#138213

Related: odoo/upgrade#5259
Related: odoo/enterprise#48692
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
Thibault Delavallée
2023-10-11 17:02:14 +00:00
parent dea14bcd27
commit 84e2d6ea55
13 changed files with 82 additions and 121 deletions
-1
View File
@@ -2,7 +2,6 @@
<odoo><data noupdate="1">
<record id="sales_team.team_sales_department" model="crm.team" forcecreate="False">
<field name="alias_name">info</field>
<field name="alias_user_id" ref="base.user_admin"/>
</record>
<record id="sales_team.salesteam_website_sales" model="crm.team" forcecreate="False">
+2 -2
View File
@@ -2016,8 +2016,8 @@ class Lead(models.Model):
# remove default author when going through the mail gateway. Indeed we
# do not want to explicitly set an user as responsible. We prefer that
# assignment is done automatically (scoring) or manually. Otherwise it
# would always be either root (gateway user) either alias owner (through
# alias_user_id). It also allows to exclude portal / public users.
# would always be root (gateway user). It also allows to exclude portal
# and public users.
self = self.with_context(default_user_id=False)
if custom_values is None:
-4
View File
@@ -52,10 +52,6 @@ class Team(models.Model):
string='# Overdue Opportunities', compute='_compute_opportunities_overdue_data')
opportunities_overdue_amount = fields.Monetary(
string='Overdue Opportunities Revenues', compute='_compute_opportunities_overdue_data',)
# alias: improve fields coming from _inherits, use inherited to avoid replacing them
alias_user_id = fields.Many2one(
'res.users', related='alias_id.alias_user_id', readonly=False, inherited=True,
domain=lambda self: [('groups_id', 'in', self.env.ref('sales_team.group_sale_salesman_all_leads').id)])
# properties
lead_properties_definition = fields.PropertiesDefinition('Lead Properties')
@@ -5,7 +5,6 @@
<record id="mail_alias_expense" model="mail.alias">
<field name="alias_name">expense</field>
<field name="alias_model_id" ref="model_hr_expense"/>
<field name="alias_user_id" ref="base.user_admin"/>
<field name="alias_contact">employees</field>
</record>
</data>
-2
View File
@@ -221,8 +221,6 @@ class Job(models.Model):
def create(self, vals_list):
for vals in vals_list:
vals['favorite_user_ids'] = vals.get('favorite_user_ids', []) + [(4, self.env.uid)]
if vals.get('alias_name'):
vals['alias_user_id'] = False
jobs = super().create(vals_list)
utm_linkedin = self.env.ref("utm.utm_source_linkedin", raise_if_not_found=False)
if utm_linkedin:
+51 -40
View File
@@ -47,11 +47,6 @@ class Alias(models.Model):
# hack to only allow selecting mail_thread models (we might
# (have a few false positives, though)
domain="[('field_id.name', '=', 'message_ids')]")
alias_user_id = fields.Many2one('res.users', 'Owner',
help="The owner of records created upon receiving emails on this alias. "
"If this field is not set the system will attempt to find the right owner "
"based on the sender (From) address, or will use the Administrator account "
"if no system user is found for that address.")
alias_defaults = fields.Text('Default Values', required=True, default='{}',
help="A Python dictionary that will be evaluated to provide "
"default values when creating new records for this alias.")
@@ -306,7 +301,35 @@ class Alias(models.Model):
# MAIL GATEWAY
# ------------------------------------------------------------
def _get_alias_bounced_body(self, message_dict):
"""Get the body of the email return in case of bounced email when the
alias does not accept incoming email e.g. contact is not allowed.
:param dict message_dict: dictionary holding parsed message variables
:return: HTML to use as email body
"""
lang_author = False
if message_dict.get('author_id'):
try:
lang_author = self.env['res.partner'].browse(message_dict['author_id']).lang
except Exception:
pass
if lang_author:
self = self.with_context(lang=lang_author)
if not is_html_empty(self.alias_bounced_content):
body = self.alias_bounced_content
else:
body = self._get_alias_bounced_body_fallback(message_dict)
return self.env['ir.qweb']._render('mail.mail_bounce_alias_security', {
'body': body,
'message': message_dict
}, minimal_qcontext=True)
def _get_alias_bounced_body_fallback(self, message_dict):
""" Default body of bounced emails. See '_get_alias_bounced_body' """
contact_description = self._get_alias_contact_description()
default_email = self.env.company.partner_id.email_formatted if self.env.company.partner_id.email else self.env.company.name
content = Markup(
@@ -331,9 +354,12 @@ class Alias(models.Model):
return _('some specific addresses')
def _get_alias_invalid_body(self, message_dict):
"""Get the body of the bounced email returned when the alias is misconfigured (ex.: error in alias_defaults).
"""Get the body of the bounced email returned when the alias is incorrectly
configured e.g. error in alias_defaults.
:param message_dict: dictionary of mail values
:param dict message_dict: dictionary holding parsed message variables
:return: HTML to use as email body
"""
content = Markup(
_("""The message below could not be accepted by the address %(alias_display_name)s.
@@ -352,44 +378,29 @@ Please try again later or contact %(company_name)s instead."""
'message': message_dict
}, minimal_qcontext=True)
def _get_alias_bounced_body(self, message_dict):
"""Get the body of the email return in case of bounced email.
:param message_dict: dictionary of mail values
"""
lang_author = False
if message_dict.get('author_id'):
try:
lang_author = self.env['res.partner'].browse(message_dict['author_id']).lang
except:
pass
if lang_author:
self = self.with_context(lang=lang_author)
if not is_html_empty(self.alias_bounced_content):
body = self.alias_bounced_content
else:
body = self._get_alias_bounced_body_fallback(message_dict)
return self.env['ir.qweb']._render('mail.mail_bounce_alias_security', {
'body': body,
'message': message_dict
}, minimal_qcontext=True)
def _set_alias_invalid(self, message, message_dict):
def _alias_bounce_incoming_email(self, message, message_dict, set_invalid=True):
"""Set alias status to invalid and create bounce message to the sender
and the alias responsible.
This method must be called when a message received on the alias has
caused an error due to the mis-configuration of the alias.
:param EmailMessage message: email message that has caused the error
:param dict message_dict: dictionary of mail values
:param EmailMessage message: email message that is invalid and is about
to bounce;
:param dict message_dict: dictionary holding parsed message variables
:param bool set_invalid: set alias as invalid, to be done notably if
bounce is considered as coming from a configuration error instead of
being rejected due to alias rules;
"""
self.ensure_one()
self.alias_status = 'invalid'
body = self._get_alias_invalid_body(message_dict)
self.env['mail.thread']._routing_create_bounce_email(message_dict['email_from'], body, message,
references=message_dict['message_id'],
# add the alias responsible as recipient if set
recipient_ids=self.alias_user_id.partner_id.ids)
if set_invalid:
self.alias_status = 'invalid'
body = self._get_alias_invalid_body(message_dict)
else:
body = self._get_alias_bounced_body(message_dict)
self.env['mail.thread']._routing_create_bounce_email(
message_dict['email_from'], body, message,
references=message_dict['message_id'],
# add the alias creator as recipient if set
recipient_ids=self.create_uid.partner_id.ids if self.create_uid.active else [],
)
+4 -15
View File
@@ -803,7 +803,7 @@ class MailThread(models.AbstractModel):
if not model:
self._routing_warn(_('target model unspecified'), message_id, route, raise_exception)
return ()
elif model not in self.env:
if model not in self.env:
self._routing_warn(_('unknown target model %s', model), message_id, route, raise_exception)
return ()
record_set = self.env[model].browse(thread_id) if thread_id else self.env[model]
@@ -855,11 +855,7 @@ class MailThread(models.AbstractModel):
route,
False
)
if error.is_config_error:
alias._set_alias_invalid(message, message_dict)
else:
body = alias._get_alias_bounced_body(message_dict)
self._routing_create_bounce_email(email_from, body, message, references=message_id)
alias._alias_bounce_incoming_email(message, message_dict, set_invalid=error.is_config_error)
return False
return (model, thread_id, route[2], route[3], route[4])
@@ -1165,8 +1161,8 @@ class MailThread(models.AbstractModel):
except Exception:
if alias:
with self.pool.cursor() as new_cr:
self.with_env(self.env(cr=new_cr)).env['mail.alias'].browse(alias.id) \
._set_alias_invalid(message, message_dict)
self.with_env(self.env(cr=new_cr)).env['mail.alias'].browse(alias.id
)._alias_bounce_incoming_email(message, message_dict, set_invalid=True)
raise
else:
if alias and alias.alias_status != 'valid':
@@ -1845,13 +1841,6 @@ class MailThread(models.AbstractModel):
if not matching_user:
std_users = self.env['res.users'].sudo().search([('email_normalized', '=', normalized_email)], limit=1)
matching_user = std_users[0] if std_users else self.env['res.users']
if matching_user:
return matching_user
if not matching_user and alias and alias.alias_user_id:
matching_user = alias and alias.alias_user_id
if matching_user:
return matching_user
return matching_user
+3 -4
View File
@@ -38,7 +38,6 @@
<field name="alias_force_thread_id"/>
<field name="alias_defaults"/>
<field name="alias_contact"/>
<field name="alias_user_id"/>
<field name="alias_parent_model_id"/>
<field name="alias_parent_thread_id"/>
</group>
@@ -58,7 +57,6 @@
<field name="alias_name"/>
<field name="alias_model_id"/>
<field name="alias_force_thread_id" optional="hide"/>
<field name="alias_user_id"/>
<field name="alias_parent_model_id"/>
<field name="alias_parent_thread_id" optional="hide"/>
<field name="alias_defaults" optional="hide"/>
@@ -85,14 +83,15 @@
<search string="Search Alias">
<field name="alias_name"/>
<field name="alias_model_id"/>
<field name="create_uid"/>
<field name="alias_force_thread_id"/>
<field name="alias_parent_model_id"/>
<field name="alias_parent_thread_id"/>
<separator/>
<filter string="Active" name="active" domain="[('alias_name', '!=', False)]"/>
<group expand="0" string="Group By">
<filter string="Owner" name="groupby_user_id"
context="{'group_by': 'alias_user_id'}"/>
<filter string="Creator" name="groupby_create_uid"
context="{'group_by': 'create_uid'}"/>
<filter string="Document Model" name="groupby_alias_model_id"
context="{'group_by': 'alias_model_id'}"/>
<filter string="Container Model" name="groupby_alias_model_id"
+22 -48
View File
@@ -171,7 +171,6 @@ class TestMailgateway(MailCommon):
'alias_contact': 'everyone',
'alias_model_id': cls.mail_test_gateway_model.id,
'alias_name': 'groups',
'alias_user_id': False,
})
# Set a first message on public group to test update and hierarchy
@@ -413,7 +412,6 @@ class TestMailgateway(MailCommon):
self.env['mail.alias'].create({
'alias_model_id': self.env['ir.model']._get_id(test_record._name),
'alias_name': 'author-partner',
'alias_user_id': self.user_employee.id
})
test_record.message_subscribe((author_partner | self.user_employee.partner_id).ids)
@@ -519,47 +517,48 @@ class TestMailgateway(MailCommon):
@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.addons.mail.models.mail_mail', 'odoo.models', 'odoo.sql_db')
def test_message_process_alias_config_invalid_defaults(self):
"""Sending a mail to a misconfigured alias must change its status to invalid + notify sender and responsible."""
"""Sending a mail to a misconfigured alias must change its status to
invalid and notify sender and alias creator."""
test_model_track = self.env['ir.model']._get('mail.test.track')
container_custom = self.env['mail.test.container'].create({})
alias_valid = self.env['mail.alias'].create({
alias_valid = self.env['mail.alias'].with_user(self.user_admin).create({
'alias_name': 'valid',
'alias_user_id': self.user_employee.id,
'alias_model_id': test_model_track.id,
'alias_contact': 'everyone',
'alias_defaults': f"{{'container_id': {container_custom.id}}}",
})
self.assertEqual(alias_valid.create_uid, self.user_admin)
# Test that it works when the reference to container_id in alias default is not dangling.
self.assertEqual(alias_valid.alias_status, 'not_tested')
with self.mock_mail_gateway(), patch('odoo.addons.mail.models.mail_alias.Alias._set_alias_invalid',
autospec=True) as _set_alias_invalid_mock:
with self.mock_mail_gateway(), patch('odoo.addons.mail.models.mail_alias.Alias._alias_bounce_incoming_email',
autospec=True) as _alias_bounce_incoming_email_mock:
record = self.format_and_process(MAIL_TEMPLATE, self.email_from, f'valid@{self.alias_domain}', subject='Valid',
target_model=test_model_track.model)
_set_alias_invalid_mock.assert_not_called()
_alias_bounce_incoming_email_mock.assert_not_called()
self.assertNotSentEmail()
self.assertEqual(record.container_id, container_custom)
self.assertEqual(alias_valid.alias_status, 'valid')
# Test with a dangling reference that must trigger bounce emails and set the alias status to invalid.
container_custom.unlink()
with self.assertRaises(Exception), patch('odoo.addons.mail.models.mail_alias.Alias._set_alias_invalid',
autospec=True) as _set_alias_invalid_mock:
with self.assertRaises(Exception), patch('odoo.addons.mail.models.mail_alias.Alias._alias_bounce_incoming_email',
autospec=True) as _alias_bounce_incoming_email_mock:
self.format_and_process(MAIL_TEMPLATE, self.email_from, f'valid@{self.alias_domain}', subject='Invalid',
target_model=test_model_track.model)
# method executed in another transaction, so we cannot test its result directly but just below
_set_alias_invalid_mock.assert_called_once()
_alias_bounce_incoming_email_mock.assert_called_once()
# call notify_alias_invalid on the test transaction to validate its effect
alias, message, message_dict = _set_alias_invalid_mock.call_args.args
alias, message, message_dict = _alias_bounce_incoming_email_mock.call_args.args
with self.mock_mail_gateway():
alias = self.env['mail.alias'].browse(alias.id) # load alias in test transaction
alias._set_alias_invalid(message, message_dict)
alias._alias_bounce_incoming_email(message, message_dict)
self.assertEqual(alias_valid.alias_status, 'invalid')
self.assertSentEmail(f'"MAILER-DAEMON" <{self.alias_bounce}@{self.alias_domain}>',
[alias_valid.alias_user_id.partner_id.email_formatted],
[self.user_admin.email_formatted],
subject='Re: Invalid')
# Not sent to self.email_from because a return path is present in MAIL_TEMPLATE
self.assertSentEmail(f'"MAILER-DAEMON" <{self.alias_bounce}@{self.alias_domain}>',
@@ -568,49 +567,34 @@ class TestMailgateway(MailCommon):
body=alias_valid._get_alias_invalid_body(message_dict))
@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
@patch('odoo.addons.mail.models.mail_alias.Alias._set_alias_invalid')
def test_message_process_alias_defaults(self, _set_alias_invalid):
def test_message_process_alias_defaults(self):
""" Test alias defaults and inner values """
self.alias.write({
'alias_user_id': self.user_employee.id,
'alias_defaults': "{'custom_field': 'defaults_custom'}"
})
self.assertEqual(self.alias.alias_status, 'not_tested')
record = self.format_and_process(MAIL_TEMPLATE, self.email_from, f'groups@{self.alias_domain}', subject='Specific')
_set_alias_invalid.assert_not_called()
record = self.format_and_process(
MAIL_TEMPLATE, self.email_from, f'groups@{self.alias_domain}',
subject='Specific'
)
self.assertEqual(self.alias.alias_status, 'valid')
self.assertEqual(len(record), 1)
res = record.get_metadata()[0].get('create_uid') or [None]
self.assertEqual(res[0], self.user_employee.id)
self.assertEqual(record.name, 'Specific')
self.assertEqual(record.custom_field, 'defaults_custom')
self.alias.write({'alias_defaults': '""'})
self.assertEqual(self.alias.alias_status, 'not_tested', 'Updating alias_defaults must reset status')
record = self.format_and_process(MAIL_TEMPLATE, self.email_from, f'groups@{self.alias_domain}', subject='Specific2')
_set_alias_invalid.assert_not_called()
record = self.format_and_process(
MAIL_TEMPLATE, self.email_from, f'groups@{self.alias_domain}',
subject='Specific2'
)
self.assertEqual(len(record), 1)
res = record.get_metadata()[0].get('create_uid') or [None]
self.assertEqual(res[0], self.user_employee.id)
self.assertEqual(record.name, 'Specific2')
self.assertFalse(record.custom_field)
self.assertEqual(self.alias.alias_status, 'valid')
@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_process_alias_user_id(self):
""" Test alias ownership """
self.alias.write({'alias_user_id': self.user_employee.id})
record = self.format_and_process(MAIL_TEMPLATE, self.email_from, f'groups@{self.alias_domain}')
self.assertEqual(len(record), 1)
res = record.get_metadata()[0].get('create_uid') or [None]
self.assertEqual(res[0], self.user_employee.id)
@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_process_alias_everyone(self):
""" Incoming email: everyone: new record + message_new """
@@ -825,7 +809,6 @@ class TestMailgateway(MailCommon):
# test@.. will cause the creation of new mail.test
new_alias_2 = self.env['mail.alias'].create({
'alias_name': 'test',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.container').id,
'alias_contact': 'everyone',
})
@@ -852,7 +835,6 @@ class TestMailgateway(MailCommon):
# test@.. will cause the creation of new mail.test
new_alias_2 = self.env['mail.alias'].create({
'alias_name': 'test',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.container').id,
'alias_contact': 'everyone',
})
@@ -882,7 +864,6 @@ class TestMailgateway(MailCommon):
# test@.. will cause the creation of new mail.test
new_alias_2 = self.env['mail.alias'].create({
'alias_name': 'test',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.container').id,
'alias_contact': 'everyone',
})
@@ -908,7 +889,6 @@ class TestMailgateway(MailCommon):
# test@.. will cause the creation of new mail.test
new_alias_2 = self.env['mail.alias'].create({
'alias_name': 'test',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.container').id,
'alias_contact': 'everyone',
})
@@ -934,7 +914,6 @@ class TestMailgateway(MailCommon):
'alias_contact': 'everyone',
'alias_model_id': self.env['ir.model']._get_id('mail.test.container.mc'),
'alias_name': 'test',
'alias_user_id': False,
})
test_domain = 'hello.com'
@@ -1460,7 +1439,6 @@ class TestMailgateway(MailCommon):
""" Incoming email using references but with alias forward should not go into references destination """
self.env['mail.alias'].create({
'alias_name': 'test.alias',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.container').id,
'alias_contact': 'everyone',
})
@@ -1481,7 +1459,6 @@ class TestMailgateway(MailCommon):
""" Incoming email using references but with alias forward on same model should be considered as a reply """
self.env['mail.alias'].create({
'alias_name': 'test.alias',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.gateway').id,
'alias_contact': 'everyone',
})
@@ -1501,7 +1478,6 @@ class TestMailgateway(MailCommon):
""" Incoming email using references but with alias forward in CC should be considered as a repy (To > Cc) """
self.env['mail.alias'].create({
'alias_name': 'test.alias',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.container').id,
'alias_contact': 'everyone',
})
@@ -1724,7 +1700,6 @@ class TestMailgateway(MailCommon):
alias = self.env['mail.alias'].create({
'alias_name': 'test',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.container').id,
'alias_contact': 'everyone',
})
@@ -1880,7 +1855,6 @@ class TestMailThreadCC(MailCommon):
'alias_contact': 'everyone',
'alias_model_id': cls.env['ir.model']._get('mail.test.cc').id,
'alias_name': 'cc_record',
'alias_user_id': False,
})
@mute_logger('odoo.addons.mail.models.mail_thread')
@@ -294,7 +294,6 @@ class TestMessageValues(MailCommon):
test_record = self.env['mail.test.simple'].create({'name': 'Test', 'email_from': 'ignasse@example.com'})
alias = self.env['mail.alias'].create({
'alias_name': 'MegaLias',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.simple').id,
'alias_parent_model_id': self.env['ir.model']._get('mail.test.simple').id,
'alias_parent_thread_id': test_record.id,
@@ -45,7 +45,6 @@ class TestMailMCCommon(MailCommon, TestRecipients):
'alias_contact': 'everyone',
'alias_model_id': cls.test_model.id,
'alias_name': 'groups',
'alias_user_id': False,
})
# Set a first message on public group to test update and hierarchy
@@ -236,7 +236,6 @@ class TestTracking(MailCommon):
'mail_template': self.env.ref('test_mail.mail_test_ticket_tracking_tpl').id}
self.env['mail.alias'].create({
'alias_name': 'groups',
'alias_user_id': False,
'alias_model_id': test_model.id,
'alias_contact': 'everyone',
'alias_defaults': custom_values})
-1
View File
@@ -14,7 +14,6 @@ class TestMassMailCommon(MassSMSCommon):
cls.test_alias = cls.env['mail.alias'].create({
'alias_name': 'test.alias',
'alias_user_id': False,
'alias_model_id': cls.env['ir.model']._get('mailing.test.simple').id,
'alias_contact': 'everyone'
})