Fix saving, getting and deleting the user portrait for non-standard u…

…ser ids.

For example 'bob-jones' or 'bob-jones+test@example.org'.

CHANGES.txt

@@ -4,6 +4,10 @@ Changelog
 4.1 (unreleased)
 ----------------
 
+- Fix saving, getting and deleting the user portrait for non-standard
+  user ids like 'bob-jones' or 'bob-jones+test@example.org'.
+  [maurits]
+
 - Fix the test for the current password if the user id differs from
   the login name.
   [maurits]

Products/PlonePAS/tests/test_membershiptool.py

@@ -294,6 +294,56 @@ def testGetPersonalPortraitWithoutPassingId(self):
         self.assertEqual(self.membership.getPersonalPortrait().meta_type,
                          'Image')
 
+    def testPortraitForNonStandardUserId(self):
+        # Some characters in a user id can give problems for getting
+        # or saving a portrait, especially '-', '+', '@'.
+        image = self.makeRealImage()
+        user_id = 'bob-jones+test@example.org'
+        safe_id = self.membership._getSafeMemberId(user_id)
+        self.assertEqual(safe_id, 'bob--jones-2Btest-40example.org')
+        self.membership.addMember(user_id, 'secret', ['Member'], [])
+        self.login(user_id)
+
+        # Should return the default portrait
+        self.assertEqual(
+            self.membership.getPersonalPortrait(user_id).getId(),
+            'defaultUser.png')
+
+        # Change your own portrait.
+        self.membership.changeMemberPortrait(image, user_id)
+        self.assertEqual(self.membership.getPersonalPortrait().getId(),
+                         safe_id)
+        self.assertEqual(self.membership.getPersonalPortrait().meta_type,
+                         'Image')
+
+        # Other users should be able to see your portrait.
+        self.login(default_user)
+        self.assertEqual(
+            self.membership.getPersonalPortrait(user_id).getId(),
+            safe_id)
+        self.assertEqual(
+            self.membership.getPersonalPortrait(user_id).meta_type,
+            'Image')
+
+        # You can delete your own portrait.
+        self.login(user_id)
+        self.membership.deletePersonalPortrait(user_id)
+        self.assertEqual(
+            self.membership.getPersonalPortrait(user_id).getId(),
+            'defaultUser.png')
+
+        # Managers should be able to change the portrait of another
+        # member and delete it.
+        manager_image = self.makeRealImage()
+        self.loginAsPortalOwner()
+        # This should not raise Unauthorized:
+        self.membership.changeMemberPortrait(manager_image, user_id)
+        self.assertEqual(self.membership.getPersonalPortrait(user_id).getId(),
+                         safe_id)
+        self.membership.deletePersonalPortrait(user_id)
+        self.assertEqual(self.membership.getPersonalPortrait(user_id).getId(),
+                        'defaultUser.png')
+
     def testListMembers(self):
         # Should return the members list
         members = self.membership.listMembers()

Products/PlonePAS/tools/membership.py

@@ -440,12 +440,10 @@ def getPersonalPortrait(self, id=None, verifyPermission=0):
 
         Modified from CMFPlone version to URL-quote the member id.
         """
+        if not id:
+            id = self.getAuthenticatedMember().getId()
         safe_id = self._getSafeMemberId(id)
         membertool = getToolByName(self, 'portal_memberdata')
-
-        if not safe_id:
-            safe_id = self.getAuthenticatedMember().getId()
-
         portrait = membertool._getPortrait(safe_id)
         if isinstance(portrait, str):
             portrait = None
@@ -462,18 +460,14 @@ def getPersonalPortrait(self, id=None, verifyPermission=0):
     security.declareProtected(SetOwnProperties, 'deletePersonalPortrait')
     def deletePersonalPortrait(self, id=None):
         """deletes the Portait of a member.
-
-        Modified from CMFPlone version to URL-quote the member id.
         """
-        safe_id = self._getSafeMemberId(id)
         authenticated_id = self.getAuthenticatedMember().getId()
-        safe_authenticated_id = self._getSafeMemberId(authenticated_id)
-        if not safe_id:
-            safe_id = safe_authenticated_id
-        if safe_authenticated_id and safe_id != safe_authenticated_id:
-            # Only Managers can delete portraits of others.
-            if not _checkPermission(ManageUsers, self):
-                raise Unauthorized
+        if not id:
+            id = authenticated_id
+        safe_id = self._getSafeMemberId(id)
+        if id != authenticated_id and not _checkPermission(
+                ManageUsers, self):
+            raise Unauthorized
 
         membertool = getToolByName(self, 'portal_memberdata')
         return membertool._deletePortrait(safe_id)
@@ -482,7 +476,7 @@ def deletePersonalPortrait(self, id=None):
     def changeMemberPortrait(self, portrait, id=None):
         """update the portait of a member.
 
-        Modified from CMFPlone version to URL-quote the member id.
+        We URL-quote the member id if needed.
 
         Note that this method might be called by an anonymous user who
         is getting registered.  This method will then be called from
@@ -491,13 +485,11 @@ def changeMemberPortrait(self, portrait, id=None):
         declareProtected line will kick in and prevent use of this
         method.
         """
-        safe_id = self._getSafeMemberId(id)
         authenticated_id = self.getAuthenticatedMember().getId()
-        safe_authenticated_id = self._getSafeMemberId(authenticated_id)
-        if not safe_id:
-            # The default is to change your own portrait.
-            safe_id = safe_authenticated_id
-        if safe_authenticated_id and safe_id != safe_authenticated_id:
+        if not id:
+            id = authenticated_id
+        safe_id = self._getSafeMemberId(id)
+        if authenticated_id and id != authenticated_id:
             # Only Managers can change portraits of others.
             if not _checkPermission(ManageUsers, self):
                 raise Unauthorized