bug: check function not used by history, displays hidden fields

The user class uses check functions (own_record) to restrict viewing
of fields that are allowed to be seen if the user page is for the
current user. This display history showed the fields/oldvalues for all
users (with class access) if the permission had a check function.

Changed the history command to use item level and not class level
checks. Check functions are not used in class level checks.

Also needed to fix memorydb. It uses an in memory dict as its
database. The getjournal function returned the values in the
database. However testing modified the database copies. So added a
copy.deepcopy() around the returned journal so it can be modified
without changing the database.

Test added for the problem replacing a useless test.  Also fixed test
that was affected by addig additional security permissions.

Author: rouilj

CHANGES.txt

@@ -127,6 +127,8 @@ Fixed:
   John Rouillard)
 - Handle ConfigurationErrors in demo.py cleanly. Used to dump a full
   traceback. Now prints error and exits. (John Rouillard)
+- Do not display journal/history fields when user does not have access
+  to the property being changed. (John Rouillard)
 
 Features:
 

roundup/hyperdb.py

@@ -1392,13 +1392,18 @@ def history(self, nodeid, enforceperm=True, skipquiet=True):
                                      self.classname, key, j_repr)
                         del args[key]
                         continue
+                    # check if user can access the property on the
+                    # item. This allows the check function in the
+                    # property to deny access.
                     if enforceperm and not (perm("View",
                                                  uid,
                                                  self.classname,
+                                                 itemid=nodeid,
                                                  property=key) or
                                             perm("Edit",
                                                  uid,
                                                  self.classname,
+                                                 itemid=nodeid,
                                                  property=key)):
                         logger.debug("skipping unaccessible property "
                                      "%s::%s seen by user%s in %s",

roundup/test/memorydb.py

@@ -1,6 +1,7 @@
 '''Implement an in-memory hyperdb for testing purposes.
 '''
 
+import copy
 import os
 import shutil
 import time
@@ -439,7 +440,7 @@ def getjournal(self, classname, nodeid):
         # add any journal entries for transactions not committed to the
         # database
         for method, args in self.transactions:
-            if method != self.doSaveJournal:
+            if method not in (self.doSaveJournal, self.doSetJournal):
                 continue
             (cache_classname, cache_nodeid, cache_action, cache_params,
                 cache_creator, cache_creation) = args
@@ -455,7 +456,10 @@ def getjournal(self, classname, nodeid):
         except KeyError:
             if res: return res                                     # noqa: E701
             raise IndexError(nodeid)
-        return res
+        # use copy otherwise we are returning the actual in memory
+        # database entry. Hence changes to the journal (e.g. remove quiet
+        # properties) changes the actual database.
+        return copy.deepcopy(res)
 
     def pack(self, pack_before):
         """ Delete all journal entries except "create" before 'pack_before'.

test/db_test_base.py

@@ -165,6 +165,22 @@ def setupSchema(db, create, module):
 
     db.security.addPermissionToRole("User", v1)
 
+    # testFilteredJournal requires this. Allow age prop access if
+    # db user is accessing own record.
+    def own_record(db, userid, itemid):
+        '''Determine whether the userid matches the item being accessed.'''
+        return userid == itemid
+    
+    v2 = db.security.addPermission(
+        name='View', klass='user', check=own_record,
+        properties=['username', 'supervisor', 'assignable', 'age'],
+        description="Allow viewing of age by own user")
+
+    db.security.addPermissionToRole("User", v2)
+
+    # update testAdminOtherCommands do_security test
+    # if you add permissions
+    
 class MyTestCase(object):
     def tearDown(self):
         if hasattr(self, 'db'):
@@ -1146,8 +1162,41 @@ def testQuietChangenote(self):
         self.db.issue.properties['nosy'].quiet=True
         self.db.issue.properties['deadline'].quiet=True
 
-    def testViewPremJournal(self):
-        pass
+    def testFilteredJournal(self):
+        """Create two new users pete, david. Permissions do not allow
+           access to the user's age prop by any user except admin and
+           the same user.  Change david's age prop. Invoke
+           history/journal as pete, david and admin. Verify age
+           changes are not shown to pete, but are shown in other two
+           cases.
+
+        """
+        
+        new_pete=self.db.user.create(username="pete",
+                                     age=10, roles="User", assignable=False)
+        new_david=self.db.user.create(username="david",
+                                      age=10, roles="User", assignable=False)
+
+        self.db.user.set(new_david, age=20)
+        self.db.commit()
+  
+        self.db.setCurrentUser("pete")
+        journal = self.db.user.history(new_david, skipquiet=False)
+        entry = journal[-1]
+        self.assertIsInstance(entry[4], dict)
+        self.assertNotIn('age', entry[4])
+
+        self.db.setCurrentUser("david")
+        journal = self.db.user.history(new_david, skipquiet=False)
+        entry = journal[-1]
+        self.assertIsInstance(entry[4], dict)
+        self.assertIn('age', entry[4])
+
+        self.db.setCurrentUser("admin") # "1" is admin
+        journal = self.db.user.history(new_david, skipquiet=False)
+        entry = journal[-1]
+        self.assertIsInstance(entry[4], dict)
+        self.assertIn('age', entry[4])
 
     def testQuietJournal(self):
         ## This is an example of how to enable logging module
@@ -3565,7 +3614,8 @@ def stdoutwrite(s):
                           ' User may access the xmlrpc interface (Xmlrpc Access)\n',
                           'Role "anonymous":\n', 'Role "user":\n',
                           ' User is allowed to access msg (View for "msg" only)\n',
-                          ' Prevent users from seeing roles (View for "user": [\'username\', \'supervisor\', \'assignable\'] only)\n']
+                          ' Prevent users from seeing roles (View for "user": [\'username\', \'supervisor\', \'assignable\'] only)\n',
+                         ' Allow viewing of age by own user (View for "user": [\'username\', \'supervisor\', \'assignable\', \'age\'] only)\n']
 
             self.assertEqual(soutput, expected)