Commit 82c51e7e authored by Jérome Perrin's avatar Jérome Perrin

notification_tool: fix Unauthorized when sending message to person user cannot access

When a user triggers `NotificationTool.sendMessage(recipient=user_id)` to a recipient she does not have access permission on, it now causes this problem (the caller context is a custom script with manager proxy role):

```
  Module Products.ERP5.Tool.NotificationTool, line 322, in sendMessage
    person_value = getUserValueByUserId(person)
  Module Products.ERP5.Tool.NotificationTool, line 291, in getUserValueByUserId
    return portal.restrictedTraverse(user['path'])
  Module OFS.Traversable, line 317, in restrictedTraverse
    return self.unrestrictedTraverse(path, default, restricted=True)
  Module OFS.Traversable, line 251, in unrestrictedTraverse
   - __traceback_info__: (['redacted_person_id'], 'person_module')
    next = guarded_getattr(obj, name)
Unauthorized: You are not allowed to access 'person_module' in this context
```

This is a regression caused by 62d8d3ac .

That particular case was working before, because the person was looked up using [catalog]( https://lab.nexedi.com/nexedi/erp5/blob/882f0022c7af4f36c2f31643498ac0b5d82c2217/product/ERP5/Tool/NotificationTool.py#L321-322) so the proxy role from the caller script was taken in to account.

Now, we can say that the approach suggested here is not correct and document that the current logged in user must have permission to access the person documents involved as sender or recipient in the notification.

Then, if we need to send message to persons the current user does not have access permission, instead  of using:
```python
portal.portal_notifications.sendMessage(recipient=person.getUserId())
```

just do:
```python
portal.portal_notifications.sendMessage(recipient=person)
```

but the later does not allow for using activities.

/cc @vpelletier @gabriel 


/reviewed-on !395
parents d5f616d9 9fdef63c
......@@ -32,6 +32,7 @@ from Products.CMFCore.utils import getToolByName
from Products.ERP5Type.Tool.BaseTool import BaseTool
from Products.ERP5Type import Permissions
from AccessControl import ModuleSecurityInfo
from zExceptions import Unauthorized
from Products.ERP5 import _dtmldir
from mimetypes import guess_type
......@@ -237,7 +238,8 @@ class NotificationTool(BaseTool):
check_consistency=False,
message_text_format='text/plain',
event_keyword_argument_dict=None,
portal_type_list=None):
portal_type_list=None,
REQUEST=None):
"""
This method provides a common API to send messages to erp5 users
from object actions of workflow scripts.
......@@ -284,11 +286,13 @@ class NotificationTool(BaseTool):
TODO: support default notification email
"""
if REQUEST is not None:
raise Unauthorized()
portal = self.getPortalObject()
searchUsers = self.acl_users.searchUsers
def getUserValueByUserId(user_id):
user, = searchUsers(id=user_id, exact_match=True)
return portal.restrictedTraverse(user['path'])
return portal.unrestrictedTraverse(user['path'])
if notifier_list is None:
# XXX TODO: Use priority_level. Need to implement default notifier query system.
......
......@@ -485,6 +485,29 @@ Yes, I will go."""
sequence_list.addSequenceString(sequence_string)
sequence_list.play(self)
def stepCheckNotificationWithoutPermissionOnRecipient(self, sequence=None):
"""
Check that notification is send by user who cannot see recipient
"""
self.logout()
self.portal.portal_notifications.sendMessage(
recipient=sequence['user_a_id'], subject='Subject', message='Message')
last_message = self.portal.MailHost._last_message
self.assertNotEquals((), last_message)
def test_permission_on_recipient_not_needed(self):
"""Notification Tool can be used to send Messages even when user does not
have permission on sender or recipent documents.
"""
sequence_list = SequenceList()
sequence_string = '\
AddUserA \
Tic \
CheckNotificationWithoutPermissionOnRecipient \
'
sequence_list.addSequenceString(sequence_string)
sequence_list.play(self)
class TestNotificationToolWithCRM(TestNotificationTool):
"""Make sure that notification tool works with crm"""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment