From 6d93c725649c8f511c9ce5bc3a1abc1503d2ebd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Geelen=20=28sge=29?= Date: Mon, 11 May 2020 08:44:12 +0000 Subject: [PATCH] [IMP] base : improve messages text in access_error pop-ups * Improve layout of access error messages from group restriction * Improve layout of access error messages from security records rules * Use the error title in the js crash manager when possible * adapt unit tests closes odoo/odoo#50369 Taskid: 2206787 Signed-off-by: Yannick Tivisse (yti) --- .../account/tests/test_portal_attachment.py | 2 +- .../static/src/js/services/crash_manager.js | 7 +- odoo/addons/base/models/ir_model.py | 36 +++--- odoo/addons/base/models/ir_rule.py | 69 +++++++---- .../test_access_rights/tests/test_feedback.py | 111 +++++++++++++----- 5 files changed, 156 insertions(+), 69 deletions(-) diff --git a/addons/account/tests/test_portal_attachment.py b/addons/account/tests/test_portal_attachment.py index 999f504e128..c36daf3086a 100644 --- a/addons/account/tests/test_portal_attachment.py +++ b/addons/account/tests/test_portal_attachment.py @@ -140,7 +140,7 @@ class TestUi(tests.HttpCase): 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 Entry' (account.move).", res.text) + self.assertIn("You are not allowed to access 'Journal Entry' (account.move) records.", res.text) # Test attachment can't be associated if not "pending" state post_data['token'] = invoice._portal_ensure_token() diff --git a/addons/web/static/src/js/services/crash_manager.js b/addons/web/static/src/js/services/crash_manager.js index ea0927a9f28..cb7e5c3b054 100644 --- a/addons/web/static/src/js/services/crash_manager.js +++ b/addons/web/static/src/js/services/crash_manager.js @@ -222,7 +222,12 @@ var CrashManager = AbstractService.extend({ return; } var message = error.data ? error.data.message : error.message; - var title = _.str.capitalize(error.type) || _t("Something went wrong !"); + var title = _t("Something went wrong !"); + if (error.type) { + title = _.str.capitalize(error.type); + } else if (error.data && error.data.title) { + title = _.str.capitalize(error.data.title); + } return this._displayWarning(message, title, options); }, show_error: function (error) { diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index 85bf6f97c05..f807e0cf675 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -1718,26 +1718,34 @@ class IrModelAccess(models.Model): if not r and raise_exception: groups = '\n'.join('\t- %s' % g for g in self.group_names_with_access(model, mode)) + document_kind = self.env['ir.model']._get(model).name or model msg_heads = { # Messages are declared in extenso so they are properly exported in translation terms - 'read': _("Sorry, you are not allowed to access documents of type '%(document_kind)s' (%(document_model)s)."), - 'write': _("Sorry, you are not allowed to modify documents of type '%(document_kind)s' (%(document_model)s)."), - 'create': _("Sorry, you are not allowed to create documents of type '%(document_kind)s' (%(document_model)s)."), - 'unlink': _("Sorry, you are not allowed to delete documents of type '%(document_kind)s' (%(document_model)s)."), - } - msg_params = { - 'document_kind': self.env['ir.model']._get(model).name or model, - 'document_model': model, + 'read': _("You are not allowed to access '%(document_kind)s' (%(document_model)s) records.", document_kind=document_kind, document_model=model), + 'write': _("You are not allowed to modify '%(document_kind)s' (%(document_model)s) records.", document_kind=document_kind, document_model=model), + 'create': _("You are not allowed to create '%(document_kind)s' (%(document_model)s) records.", document_kind=document_kind, document_model=model), + 'unlink': _("You are not allowed to delete '%(document_kind)s' (%(document_model)s) records.", document_kind=document_kind, document_model=model), } + operation_error = msg_heads[mode] + if groups: - msg_tail = _("This operation is allowed for the groups:\n%(groups_list)s") - msg_params['groups_list'] = groups + group_info = _("This operation is allowed for the following groups:\n%(groups_list)s", groups_list=groups) else: - msg_tail = _("No group currently allows this operation.") - msg_tail += u' - ({} {}, {} {})'.format(_('Operation:'), mode, _('User:'), self._uid) + group_info = _("No group currently allows this operation.") + + resolution_info = _("Contact your administrator to request access if necessary.") + _logger.info('Access Denied by ACLs for operation: %s, uid: %s, model: %s', mode, self._uid, model) - msg = '%s %s' % (msg_heads[mode], msg_tail) - raise AccessError(msg % msg_params) + msg = """{operation_error} + +{group_info} + +{resolution_info}""".format( + operation_error=operation_error, + group_info=group_info, + resolution_info=resolution_info) + + raise AccessError(msg) return bool(r) diff --git a/odoo/addons/base/models/ir_rule.py b/odoo/addons/base/models/ir_rule.py index 6f524137847..519468f63c7 100644 --- a/odoo/addons/base/models/ir_rule.py +++ b/odoo/addons/base/models/ir_rule.py @@ -220,36 +220,61 @@ class IrRule(models.Model): model = records._name description = self.env['ir.model']._get(model).name or model - if not self.env.user.has_group('base.group_no_one'): - return AccessError(_('The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: "%(document_kind)s" (%(document_model)s), Operation: %(operation)s)') % { - 'document_kind': description, - 'document_model': model, - 'operation': operation, - }) + msg_heads = { + # Messages are declared in extenso so they are properly exported in translation terms + 'read': _("Due to security restrictions, you are not allowed to access '%(document_kind)s' (%(document_model)s) records.", document_kind=description, document_model=model), + 'write': _("Due to security restrictions, you are not allowed to modify '%(document_kind)s' (%(document_model)s) records.", document_kind=description, document_model=model), + 'create': _("Due to security restrictions, you are not allowed to create '%(document_kind)s' (%(document_model)s) records.", document_kind=description, document_model=model), + 'unlink': _("Due to security restrictions, you are not allowed to delete '%(document_kind)s' (%(document_model)s) records.", document_kind=description, document_model=model) + } + operation_error = msg_heads[operation] + resolution_info = _("Contact your administrator to request access if necessary.") + + if not self.env.user.has_group('base.group_no_one') or not self.env.user.has_group('base.group_user'): + msg = """{operation_error} + +{resolution_info}""".format( + operation_error=operation_error, + resolution_info=resolution_info) + return AccessError(msg) # This extended AccessError is only displayed in debug mode. # Note that by default, public and portal users do not have # the group "base.group_no_one", even if debug mode is enabled, - # so it is relatively safe here to include the list of rules and - # record names. + # so it is relatively safe here to include the list of rules and record names. rules = self._get_failing(records, mode=operation).sudo() - error = AccessError(_("""The requested operation ("%(operation)s" on "%(document_kind)s" (%(document_model)s)) was rejected because of the following rules: -%(rules_list)s -%(multi_company_warning)s -(Records: %(example_records)s, User: %(user_id)s)""") % { - 'operation': operation, - 'document_kind': description, - 'document_model': model, - 'rules_list': '\n'.join('- %s' % rule.name for rule in rules), - 'multi_company_warning': ('\n' + _('Note: this might be a multi-company issue.') + '\n') if any( - 'company_id' in (r.domain_force or []) for r in rules) else '', - 'example_records': ' - '.join(['%s (id=%s)' % (rec.display_name, rec.id) for rec in records[:6].sudo()]), - 'user_id': '%s (id=%s)' % (self.env.user.name, self.env.user.id), - }) + + records_description = ', '.join(['%s (id=%s)' % (rec.display_name, rec.id) for rec in records[:6].sudo()]) + failing_records = _("Records: %s", records_description) + + user_description = '%s (id=%s)' % (self.env.user.name, self.env.user.id) + failing_user = _("User: %s", user_description) + + rules_description = '\n'.join('- %s' % rule.name for rule in rules) + failing_rules = _("This restriction is due to the following rules:\n%s", rules_description) + if any('company_id' in (r.domain_force or []) for r in rules): + failing_rules += "\n\n" + _('Note: this might be a multi-company issue.') + + msg = """{operation_error} + +{failing_records} +{failing_user} + +{failing_rules} + +{resolution_info}""".format( + operation_error=operation_error, + failing_records=failing_records, + failing_user=failing_user, + failing_rules=failing_rules, + resolution_info=resolution_info) + # clean up the cache of records prefetched with display_name above for record in records[:6]: record._cache.clear() - return error + + return AccessError(msg) + # # Hack for field 'global': this field cannot be defined like others, because diff --git a/odoo/addons/test_access_rights/tests/test_feedback.py b/odoo/addons/test_access_rights/tests/test_feedback.py index 2dc4a00d7a5..0e0afbe5b50 100644 --- a/odoo/addons/test_access_rights/tests/test_feedback.py +++ b/odoo/addons/test_access_rights/tests/test_feedback.py @@ -117,7 +117,11 @@ class TestACLFeedback(Feedback): self.record.with_user(self.user).write({'val': 10}) self.assertEqual( ctx.exception.args[0], - """Sorry, you are not allowed to modify documents of type 'Object For Test Access Right' (test_access_right.some_obj). No group currently allows this operation. - (Operation: write, User: %d)""" % self.user.id + """You are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records. + +No group currently allows this operation. + +Contact your administrator to request access if necessary.""" ) def test_one_group(self): @@ -127,12 +131,20 @@ class TestACLFeedback(Feedback): }) self.assertEqual( ctx.exception.args[0], - """Sorry, you are not allowed to create documents of type 'Object For Test Access Right' (test_access_right.some_obj). This operation is allowed for the groups:\n\t- Group 0 - (Operation: create, User: %d)""" % self.user.id + """You are not allowed to create 'Object For Test Access Right' (test_access_right.some_obj) records. + +This operation is allowed for the following groups:\n\t- Group 0 + +Contact your administrator to request access if necessary.""" ) def test_two_groups(self): r = self.record.with_user(self.user) - expected = """Sorry, you are not allowed to access documents of type 'Object For Test Access Right' (test_access_right.some_obj). This operation is allowed for the groups:\n\t- Group 0\n\t- Group 1 - (Operation: read, User: %d)""" % self.user.id + expected = """You are not allowed to access 'Object For Test Access Right' (test_access_right.some_obj) records. + +This operation is allowed for the following groups:\n\t- Group 0\n\t- Group 1 + +Contact your administrator to request access if necessary.""" with self.assertRaises(AccessError) as ctx: # noinspection PyStatementEffect r.val @@ -171,19 +183,26 @@ class TestIRRuleFeedback(Feedback): self.record.write({'val': 1}) self.assertEqual( ctx.exception.args[0], - 'The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: "Object For Test Access Right" (test_access_right.some_obj), Operation: write)' - ) + """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records. + +Contact your administrator to request access if necessary.""") # debug mode self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]}) + self.env.ref('base.group_user').write({'users': [(4, self.user.id)]}) with self.assertRaises(AccessError) as ctx: self.record.write({'val': 1}) self.assertEqual( ctx.exception.args[0], - """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules: + """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records. + +Records: %s (id=%s) +User: %s (id=%s) + +This restriction is due to the following rules: - rule 0 -(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) +Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) ) @@ -196,58 +215,73 @@ class TestIRRuleFeedback(Feedback): p.with_user(self.user).write({'val': 1}) def test_locals(self): - self.env.ref('base.group_no_one').write( - {'users': [(4, self.user.id)]}) + self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]}) + self.env.ref('base.group_user').write({'users': [(4, self.user.id)]}) self._make_rule('rule 0', '[("val", "=", 42)]') self._make_rule('rule 1', '[("val", "=", 78)]') with self.assertRaises(AccessError) as ctx: self.record.write({'val': 1}) self.assertEqual( ctx.exception.args[0], - """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules: + """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records. + +Records: %s (id=%s) +User: %s (id=%s) + +This restriction is due to the following rules: - rule 0 - rule 1 -(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) +Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) ) def test_globals_all(self): - self.env.ref('base.group_no_one').write( - {'users': [(4, self.user.id)]}) + self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]}) + self.env.ref('base.group_user').write({'users': [(4, self.user.id)]}) self._make_rule('rule 0', '[("val", "=", 42)]', global_=True) self._make_rule('rule 1', '[("val", "=", 78)]', global_=True) with self.assertRaises(AccessError) as ctx: self.record.write({'val': 1}) self.assertEqual( ctx.exception.args[0], - """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules: + """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records. + +Records: %s (id=%s) +User: %s (id=%s) + +This restriction is due to the following rules: - rule 0 - rule 1 -(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) +Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) ) def test_globals_any(self): """ Global rules are AND-eded together, so when an access fails it might be just one of the rules, and we want an exact listing """ - self.env.ref('base.group_no_one').write( - {'users': [(4, self.user.id)]}) + self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]}) + self.env.ref('base.group_user').write({'users': [(4, self.user.id)]}) self._make_rule('rule 0', '[("val", "=", 42)]', global_=True) self._make_rule('rule 1', '[(1, "=", 1)]', global_=True) with self.assertRaises(AccessError) as ctx: self.record.write({'val': 1}) self.assertEqual( ctx.exception.args[0], - """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules: + """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records. + +Records: %s (id=%s) +User: %s (id=%s) + +This restriction is due to the following rules: - rule 0 -(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) +Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) ) def test_combination(self): - self.env.ref('base.group_no_one').write( - {'users': [(4, self.user.id)]}) + self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]}) + self.env.ref('base.group_user').write({'users': [(4, self.user.id)]}) self._make_rule('rule 0', '[("val", "=", 42)]', global_=True) self._make_rule('rule 1', '[(1, "=", 1)]', global_=True) self._make_rule('rule 2', '[(0, "=", 1)]') @@ -256,53 +290,68 @@ class TestIRRuleFeedback(Feedback): self.record.write({'val': 1}) self.assertEqual( ctx.exception.args[0], - """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules: + """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records. + +Records: %s (id=%s) +User: %s (id=%s) + +This restriction is due to the following rules: - rule 0 - rule 2 - rule 3 -(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) +Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) ) def test_warn_company(self): """ If one of the failing rules mentions company_id, add a note that this might be a multi-company issue. """ - self.env.ref('base.group_no_one').write( - {'users': [(4, self.user.id)]}) + self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]}) + self.env.ref('base.group_user').write({'users': [(4, self.user.id)]}) self._make_rule('rule 0', "[('company_id', '=', user.company_id.id)]") self._make_rule('rule 1', '[("val", "=", 0)]', global_=True) with self.assertRaises(AccessError) as ctx: self.record.write({'val': 1}) self.assertEqual( ctx.exception.args[0], - """The requested operation ("write" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules: + """Due to security restrictions, you are not allowed to modify 'Object For Test Access Right' (test_access_right.some_obj) records. + +Records: %s (id=%s) +User: %s (id=%s) + +This restriction is due to the following rules: - rule 0 Note: this might be a multi-company issue. -(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) +Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) ) def test_read(self): """ because of prefetching, read() goes through a different codepath to apply rules """ - self.env.ref('base.group_no_one').write( - {'users': [(4, self.user.id)]}) + self.env.ref('base.group_no_one').write({'users': [(4, self.user.id)]}) + self.env.ref('base.group_user').write({'users': [(4, self.user.id)]}) self._make_rule('rule 0', "[('company_id', '=', user.company_id.id)]", attr='read') self._make_rule('rule 1', '[("val", "=", 1)]', global_=True, attr='read') with self.assertRaises(AccessError) as ctx: _ = self.record.val self.assertEqual( ctx.exception.args[0], - """The requested operation ("read" on "Object For Test Access Right" (test_access_right.some_obj)) was rejected because of the following rules: + """Due to security restrictions, you are not allowed to access 'Object For Test Access Right' (test_access_right.some_obj) records. + +Records: %s (id=%s) +User: %s (id=%s) + +This restriction is due to the following rules: - rule 0 - rule 1 Note: this might be a multi-company issue. -(Records: %s (id=%s), User: %s (id=%s))""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) +Contact your administrator to request access if necessary.""" % (self.record.display_name, self.record.id, self.user.name, self.user.id) ) p = self.env['test_access_right.parent'].create({'obj_id': self.record.id})