diff --git a/addons/account/tests/__init__.py b/addons/account/tests/__init__.py index 5c822cca737..e4b29d36e38 100644 --- a/addons/account/tests/__init__.py +++ b/addons/account/tests/__init__.py @@ -19,4 +19,5 @@ from . import test_templates_consistency from . import test_account_fiscal_year from . import test_account_all_l10n from . import test_reconciliation_matching_rules -from . import test_account_onboarding \ No newline at end of file +from . import test_account_onboarding +from . import test_portal_attachment diff --git a/addons/portal/tests/test_attachment.py b/addons/account/tests/test_portal_attachment.py similarity index 64% rename from addons/portal/tests/test_attachment.py rename to addons/account/tests/test_portal_attachment.py index a3f8c3b11db..2aa1b4eff2c 100644 --- a/addons/portal/tests/test_attachment.py +++ b/addons/account/tests/test_portal_attachment.py @@ -12,18 +12,20 @@ class TestUi(tests.HttpCase): @mute_logger('odoo.addons.website.models.ir_http', 'odoo.http') def test_01_portal_attachment(self): + """Test the portal chatter attachment route.""" + base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url') - # For this test we need a parent document with an access_token field: - # attachment itself has an access_token so we can use it as parent too. - record = self.env['ir.attachment'].create({ + # For this test we need a parent document with an access_token field + # (in this case from portal.mixin) and also inheriting mail.thread. + invoice = self.env['account.move'].with_context(tracking_disable=True).create({ 'name': 'a record with an access_token field', }) # Test public user can't create attachment without token of document create_data = { 'name': "new attachment", - 'res_model': record._name, - 'res_id': record.id, + 'res_model': invoice._name, + 'res_id': invoice.id, 'csrf_token': http.WebRequest.csrf_token(self), } create_url = base_url + '/portal/attachment/add' @@ -33,7 +35,7 @@ class TestUi(tests.HttpCase): self.assertTrue("you do not have the rights" in res.text) # Test public user can create attachment with token - create_data['access_token'] = record.generate_access_token()[0] + create_data['access_token'] = invoice._portal_ensure_token() res = self.url_open(url=create_url, data=create_data, files=files) self.assertEqual(res.status_code, 200) create_res = json.loads(res.content.decode('utf-8')) @@ -82,21 +84,23 @@ class TestUi(tests.HttpCase): self.assertTrue(remove_res is True) # Test attachment can't be removed if not "pending" state + attachment = self.env['ir.attachment'].create({ + 'name': "an attachment", + 'access_token': self.env['ir.attachment']._generate_access_token(), + }) remove_data = { - 'attachment_id': record.id, - 'access_token': record.access_token, + 'attachment_id': attachment.id, + 'access_token': attachment.access_token, } res = self.opener.post(url=remove_url, json={'params': remove_data}) self.assertEqual(res.status_code, 200) - self.assertTrue(self.env['ir.attachment'].search([('id', '=', record.id)])) + self.assertTrue(self.env['ir.attachment'].search([('id', '=', attachment.id)])) self.assertTrue("not in a pending state" in res.text) # Test attachment can't be removed if attached to a message - attachment = self.env['ir.attachment'].create({ - 'name': 'a record with an access_token field', + attachment.write({ 'res_model': 'mail.compose.message', 'res_id': 0, - 'access_token': self.env['ir.attachment']._generate_access_token(), }) message = self.env['mail.message'].create({ 'attachment_ids': [(6, 0, attachment.ids)], @@ -111,42 +115,60 @@ class TestUi(tests.HttpCase): self.assertTrue("it is linked to a message" in res.text) message.unlink() - # Need a `mail.thread` record for the following test - thread_record = self.env['mail.channel'].create({ - 'name': 'channel', - 'public': 'public', - }) - # Authenticate because public user can't create `mail.message` without - # token, and `mail.channel` does not have an access_token. - self.authenticate('portal', 'portal') - - # Test attachment can't be associated if no token. + # Test attachment can't be associated if no attachment token. post_url = base_url + '/mail/chatter_post' post_data = { - 'res_model': thread_record._name, - 'res_id': thread_record.id, - 'message': "test message", + 'res_model': invoice._name, + 'res_id': invoice.id, + 'message': "test message 1", 'attachment_ids': attachment.id, 'attachment_tokens': 'false', 'csrf_token': http.WebRequest.csrf_token(self), } - self.assertFalse(thread_record.message_ids) res = self.url_open(url=post_url, data=post_data) - thread_record.invalidate_cache(fnames=['message_ids'], ids=thread_record.ids) - message = thread_record.message_ids[0] - self.assertFalse(message.attachment_ids) + self.assertEqual(res.status_code, 400) + self.assertIn("The attachment %s does not exist or you do not have the rights to access it." % attachment.id, res.text) + + # Test attachment can't be associated if no main document token + post_data['attachment_tokens'] = attachment.access_token + res = self.url_open(url=post_url, data=post_data) + self.assertEqual(res.status_code, 403) + self.assertIn("Sorry, you are not allowed to access documents of type 'Journal Entries' (account.move).", res.text) # Test attachment can't be associated if not "pending" state - post_data['attachment_tokens'] = attachment.access_token + post_data['token'] = invoice._portal_ensure_token() + self.assertFalse(invoice.message_ids) attachment.write({'res_model': 'model'}) res = self.url_open(url=post_url, data=post_data) - thread_record.invalidate_cache(fnames=['message_ids'], ids=thread_record.ids) - message = thread_record.message_ids[0] - self.assertFalse(message.attachment_ids) + self.assertEqual(res.status_code, 200) + invoice.invalidate_cache(fnames=['message_ids'], ids=invoice.ids) + self.assertEqual(len(invoice.message_ids), 1) + self.assertEqual(invoice.message_ids.body, "

test message 1

") + self.assertFalse(invoice.message_ids.attachment_ids) - # Test attachment can be associated if all good + # Test attachment can't be associated if not correct user attachment.write({'res_model': 'mail.compose.message'}) + post_data['message'] = "test message 2" res = self.url_open(url=post_url, data=post_data) - thread_record.invalidate_cache(fnames=['message_ids'], ids=thread_record.ids) - message = thread_record.message_ids[0] - self.assertEqual(len(message.attachment_ids), 1) + self.assertEqual(res.status_code, 200) + invoice.invalidate_cache(fnames=['message_ids'], ids=invoice.ids) + self.assertEqual(len(invoice.message_ids), 2) + self.assertEqual(invoice.message_ids[0].body, "

test message 2

") + self.assertFalse(invoice.message_ids.attachment_ids) + + # Test attachment can be associated if all good (complete flow) + create_data['name'] = "final attachment" + res = self.url_open(url=create_url, data=create_data, files=files) + self.assertEqual(res.status_code, 200) + create_res = json.loads(res.content.decode('utf-8')) + self.assertEqual(create_res['name'], "final attachment") + + post_data['message'] = "test message 3" + post_data['attachment_ids'] = create_res['id'] + post_data['attachment_tokens'] = create_res['access_token'] + res = self.url_open(url=post_url, data=post_data) + self.assertEqual(res.status_code, 200) + invoice.invalidate_cache(fnames=['message_ids'], ids=invoice.ids) + self.assertEqual(len(invoice.message_ids), 3) + self.assertEqual(invoice.message_ids[0].body, "

test message 3

") + self.assertEqual(len(invoice.message_ids[0].attachment_ids), 1) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 701690039ca..68254b2bde0 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1619,7 +1619,7 @@ class MailThread(models.AbstractModel): lambda a: a.res_model == 'mail.compose.message' and a.create_uid.id == self._uid) if filtered_attachment_ids: filtered_attachment_ids.write({'res_model': model, 'res_id': res_id}) - m2m_attachment_ids += [(4, id) for id in attachment_ids] + m2m_attachment_ids += [(4, id) for id in filtered_attachment_ids.ids] # Handle attachments parameter, that is a dictionary of attachments if attachments: # generate diff --git a/addons/portal/controllers/mail.py b/addons/portal/controllers/mail.py index dec8fccff5c..f06f14a1b56 100644 --- a/addons/portal/controllers/mail.py +++ b/addons/portal/controllers/mail.py @@ -5,13 +5,13 @@ import werkzeug from werkzeug import urls from werkzeug.exceptions import NotFound, Forbidden -from odoo import http +from odoo import http, _ from odoo.http import request from odoo.osv import expression from odoo.tools import consteq, plaintext2html from odoo.addons.mail.controllers.main import MailController from odoo.addons.portal.controllers.portal import CustomerPortal -from odoo.exceptions import AccessError, MissingError +from odoo.exceptions import AccessError, MissingError, UserError def _check_special_access(res_model, res_id, token='', _hash='', pid=False): @@ -25,7 +25,7 @@ def _check_special_access(res_model, res_id, token='', _hash='', pid=False): raise Forbidden() -def _message_post_helper(res_model, res_id, message, token='', nosubscribe=True, **kw): +def _message_post_helper(res_model, res_id, message, token='', _hash=False, pid=False, nosubscribe=True, **kw): """ Generic chatter function, allowing to write on *any* object that inherits mail.thread. We distinguish 2 cases: 1/ If a token is specified, all logged in users will be able to write a message regardless @@ -55,9 +55,9 @@ def _message_post_helper(res_model, res_id, message, token='', nosubscribe=True, # check if user can post with special token/signed token. The "else" will try to post message with the # current user access rights (_mail_post_access use case). - if token or (kw.get('hash') and kw.get('pid')): - pid = int(kw['pid']) if kw.get('pid') else False - if _check_special_access(res_model, res_id, token=token, _hash=kw.get('hash'), pid=pid): + if token or (_hash and pid): + pid = int(pid) if pid else False + if _check_special_access(res_model, res_id, token=token, _hash=_hash, pid=pid): record = record.sudo() else: raise Forbidden() @@ -75,15 +75,9 @@ def _message_post_helper(res_model, res_id, message, token='', nosubscribe=True, if not author_id: raise NotFound() # Signed Token Case: author_id is forced - elif kw.get('hash') and kw.get('pid'): - author_id = int(kw.get('pid')) + elif _hash and pid: + author_id = pid - kw.pop('csrf_token', None) - kw.pop('redirect', None) - kw.pop('attachment_ids', None) - kw.pop('attachment_tokens', None) - kw.pop('hash', None) - kw.pop('pid', None) return record.with_context(mail_create_nosubscribe=nosubscribe).message_post( body=message, message_type=kw.pop('message_type', "comment"), subtype=kw.pop('subtype', "mt_comment"), author_id=author_id, **kw) @@ -91,45 +85,8 @@ def _message_post_helper(res_model, res_id, message, token='', nosubscribe=True, class PortalChatter(http.Controller): - def _post_process_portal_attachments(self, message, res_model, res_id, attachment_ids, attachment_tokens): - """Associate "pending" attachments to a message and its main record. - - The user must have the rights to all attachments or he must provide a - valid access_token for each of them. The attachments have to be on a - "pending" state as well: res_id=0 and res_model='mail.compose.message'. - - :param message: the related message on which to link the attachments - :type message: recordset of one `mail.message` - - :param res_model: the related model that will be saved on the attachment - :type res_model: string - - :param res_id: the id of the record that will be saved on the attachment - :type res_id: int - - :param attachment_ids: id of the attachments to associate - :type attachment_ids: iterable of int - - :param attachment_tokens: access_token of the attachments to associate. - Must always be the same length as `attachment_ids`, but only has to - contain a valid access_token if the user does not have the rights to - access the attachment without token. - :type attachment_tokens: iterable of string - """ - message.ensure_one() - attachments = request.env['ir.attachment'].sudo() - for (attachment_id, access_token) in zip(attachment_ids, attachment_tokens): - try: - attachment = CustomerPortal._document_check_access(self, 'ir.attachment', attachment_id, access_token) - if attachment.res_model == 'mail.compose.message' and attachment.res_id == 0: - attachments += attachment - except (AccessError, MissingError): - pass - attachments.write({'res_model': res_model, 'res_id': res_id}) - message.attachment_ids |= attachments - @http.route(['/mail/chatter_post'], type='http', methods=['POST'], auth='public', website=True) - def portal_chatter_post(self, res_model, res_id, message, redirect=None, attachment_ids=None, attachment_tokens=None, **kw): + def portal_chatter_post(self, res_model, res_id, message, redirect=None, attachment_ids='', attachment_tokens='', **kw): """Create a new `mail.message` with the given `message` and/or `attachment_ids` and redirect the user to the newly created message. @@ -138,18 +95,33 @@ class PortalChatter(http.Controller): must provide valid identifiers through `kw`. See `_message_post_helper`. """ url = redirect or (request.httprequest.referrer and request.httprequest.referrer + "#discussion") or '/my' - if message or (attachment_ids and attachment_tokens): - res_id = int(res_id) + + res_id = int(res_id) + + attachment_ids = [int(res_id) for res_id in attachment_ids.split(',')] + attachment_tokens = attachment_tokens.split(',') + if len(attachment_tokens) != len(attachment_ids): + raise UserError(_("An access token must be provided for each attachment.")) + for (attachment_id, access_token) in zip(attachment_ids, attachment_tokens): + try: + CustomerPortal._document_check_access(self, 'ir.attachment', attachment_id, access_token) + except (AccessError, MissingError): + raise UserError(_("The attachment %s does not exist or you do not have the rights to access it.") % attachment_id) + + if message or attachment_ids: # message is received in plaintext and saved in html if message: message = plaintext2html(message) - message = _message_post_helper(res_model=res_model, res_id=res_id, message=message, **kw) - - if attachment_ids and attachment_tokens and message: - attachment_ids = [int(res_id) for res_id in attachment_ids.split(',')] - attachment_tokens = attachment_tokens.split(',') - self._post_process_portal_attachments(message=message, res_model=res_model, res_id=res_id, - attachment_ids=attachment_ids, attachment_tokens=attachment_tokens) + message = _message_post_helper( + res_model=res_model, + res_id=res_id, + token=kw.get('token'), + _hash=kw.get('hash'), + pid=kw.get('pid'), + message=message, + send_after_commit=False, + attachment_ids=attachment_ids + ) return request.redirect(url) diff --git a/addons/portal/controllers/portal.py b/addons/portal/controllers/portal.py index 286cd970a0c..4425266df38 100644 --- a/addons/portal/controllers/portal.py +++ b/addons/portal/controllers/portal.py @@ -241,7 +241,7 @@ class CustomerPortal(Controller): # At this point the related message does not exist yet, so we assign # those specific res_model and res_is. They will be correctly set - # when the message is created: see `_post_process_portal_attachments`, + # when the message is created: see `portal_chatter_post`, # or garbage collected otherwise: see `_garbage_collect_attachments`. attachment = IrAttachment.create({ 'name': name, diff --git a/addons/portal/tests/__init__.py b/addons/portal/tests/__init__.py index 752ffa6f96c..06a8d4cd529 100644 --- a/addons/portal/tests/__init__.py +++ b/addons/portal/tests/__init__.py @@ -2,4 +2,3 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from . import test_load_process -from . import test_attachment diff --git a/addons/test_mail/tests/test_odoobot.py b/addons/test_mail/tests/test_odoobot.py index af73b325311..a494a84bf53 100644 --- a/addons/test_mail/tests/test_odoobot.py +++ b/addons/test_mail/tests/test_odoobot.py @@ -64,6 +64,7 @@ class TestOdoobot(BaseFunctionalTest, MockEmails, TestRecipients): attachment = self.env['ir.attachment'].with_user(self.user_employee).create({ 'datas': 'bWlncmF0aW9uIHRlc3Q=', 'name': 'picture_of_your_dog.doc', + 'res_model': 'mail.compose.message', }) kwargs['attachment_ids'] = [attachment.id] last_message = self.assertNextMessage(