Do some more checks when changing or deleting a member portrait.

Fixes http://dev.plone.org/ticket/5432

CHANGES.txt

@@ -4,6 +4,10 @@ Changelog
 4.0.11 (unreleased)
 -------------------
 
+- Do some more checks when changing or deleting a member portrait.
+  Fixes http://dev.plone.org/ticket/5432
+  [maurits]
+
 - Pass request along to getGroupsForPrincipal for caching purposes.
   [esteele]
 

Products/PlonePAS/tests/test_membershiptool.py

@@ -196,22 +196,63 @@ def testGetPersonalPortrait(self):
         # Should return the default portrait
         self.assertEqual(self.membership.getPersonalPortrait(default_user).getId(), 'defaultUser.png')
 
-    def testChangeMemberPortrait(self):
+    def testChangeOwnMemberPortrait(self):
         # Should change the portrait image
         # first we need a valid image
         image = self.makeRealImage()
         self.membership.changeMemberPortrait(image, default_user)
         self.assertEqual(self.membership.getPersonalPortrait(default_user).getId(), default_user)
         self.assertEqual(self.membership.getPersonalPortrait(default_user).meta_type, 'Image')
 
-    def testDeletePersonalPortrait(self):
+    def testCannotChangeOtherMemberPortrait(self):
+        # A normal member should not be able to change the portrait of
+        # another member.
+        image = self.makeRealImage()
+        self.membership.addMember('joe', 'secret', ['Member'], [])
+        self.assertRaises(Unauthorized, self.membership.changeMemberPortrait,
+                          image, 'joe')
+
+    def testChangeMemberPortraitAsManager(self):
+        # Managers should be able to change the portrait of another
+        # member.
+        image = self.makeRealImage()
+        self.membership.addMember('joe', 'secret', ['Member'], [])
+        self.setRoles(['Manager'])
+        # This should not raise Unauthorized:
+        self.membership.changeMemberPortrait(image, 'joe')
+        self.assertEqual(self.membership.getPersonalPortrait('joe').getId(), 'joe')
+        self.assertEqual(self.membership.getPersonalPortrait('joe').meta_type, 'Image')
+
+    def testDeleteOwnPersonalPortrait(self):
         # Should delete the portrait image
         image = self.makeRealImage()
         self.membership.changeMemberPortrait(image, default_user)
         self.assertEqual(self.membership.getPersonalPortrait(default_user).getId(), default_user)
         self.membership.deletePersonalPortrait(default_user)
         self.assertEqual(self.membership.getPersonalPortrait(default_user).getId(), 'defaultUser.png')
 
+    def testCannotDeleteOtherPersonalPortrait(self):
+        # A normal member should not be able to delete the portrait of
+        # another member.
+        image = self.makeRealImage()
+        self.membership.addMember('joe', 'secret', ['Member'], [])
+        self.setRoles(['Manager'])
+        self.membership.changeMemberPortrait(image, 'joe')
+        self.setRoles(['Member'])
+        self.assertRaises(Unauthorized, self.membership.deletePersonalPortrait,
+                          'joe')
+
+    def testDeleteOtherPersonalPortraitAsManager(self):
+        # Managers should be able to change the portrait of another
+        # member.
+        image = self.makeRealImage()
+        self.membership.addMember('joe', 'secret', ['Member'], [])
+        self.setRoles(['Manager'])
+        self.membership.changeMemberPortrait(image, 'joe')
+        self.membership.deletePersonalPortrait('joe')
+        self.assertEqual(self.membership.getPersonalPortrait('joe').getId(),
+                        'defaultUser.png')
+
     def testGetPersonalPortraitWithoutPassingId(self):
         # Should return the logged in users portrait if no id is given
         image = self.makeRealImage()

Products/PlonePAS/tools/membership.py

@@ -12,6 +12,7 @@
 
 from AccessControl import ClassSecurityInfo
 from AccessControl import getSecurityManager
+from AccessControl import Unauthorized
 from AccessControl.SecurityManagement import noSecurityManager
 from AccessControl.requestmethod import postonly
 from Acquisition import aq_get
@@ -468,11 +469,14 @@ def deletePersonalPortrait(self, id=None):
         Modified from CMFPlone version to URL-quote the member id.
         """
         safe_id = self._getSafeMemberId(id)
-        membertool = getToolByName(self, 'portal_memberdata')
-
+        authenticated_id = self.getAuthenticatedMember().getId()
         if not safe_id:
-            safe_id = self.getAuthenticatedMember().getId()
+            safe_id = authenticated_id
+        if safe_id != authenticated_id and not _checkPermission(
+                ManageUsers, self):
+            raise Unauthorized
 
+        membertool = getToolByName(self, 'portal_memberdata')
         return membertool._deletePortrait(safe_id)
 
 
@@ -483,9 +487,12 @@ def changeMemberPortrait(self, portrait, id=None):
         Modified from CMFPlone version to URL-quote the member id.
         """
         safe_id = self._getSafeMemberId(id)
+        authenticated_id = self.getAuthenticatedMember().getId()
         if not safe_id:
-            safe_id = self.getAuthenticatedMember().getId()
-
+            safe_id = authenticated_id
+        if safe_id != authenticated_id and not _checkPermission(
+                ManageUsers, self):
+            raise Unauthorized
         if portrait and portrait.filename:
             scaled, mimetype = scale_image(portrait)
             portrait = Image(id=safe_id, file=scaled, title='')