[#2784] Hand control of user's sensitive data to user_dictize.

Only a sysadmin or the owner of the account may view a user's apikey
or reset_key.

This does not change behaviour of the actions; just moves the logic
form the actions to the model dictize layer.

ckan/lib/dictization/model_dictize.py

@@ -2,6 +2,7 @@
 from pylons import config
 from sqlalchemy.sql import select
 import datetime
+import ckan.authz
 import ckan.model
 import ckan.misc
 import ckan.logic as logic
@@ -351,7 +352,6 @@ def user_list_dictize(obj_list, context,
 
     for obj in obj_list:
         user_dict = user_dictize(obj, context)
-        user_dict.pop('apikey')
         result_list.append(user_dict)
     return sorted(result_list, key=sort_key, reverse=reverse)
 
@@ -373,6 +373,14 @@ def user_dictize(user, context):
     result_dict['number_of_edits'] = user.number_of_edits()
     result_dict['number_administered_packages'] = user.number_administered_packages()
 
+    requester = context['user']
+
+    if not (ckan.authz.Authorizer().is_sysadmin(unicode(requester)) or
+            requester == user.name):
+        # If not sysadmin or the same user, strip sensible info
+        result_dict.pop('apikey', None)
+        result_dict.pop('reset_key', None)
+
     model = context['model']
     session = model.Session
 

ckan/logic/action/create.py

@@ -684,10 +684,17 @@ def user_create(context, data_dict):
     if not context.get('defer_commit'):
         model.repo.commit()
 
+    # Construct the user dict before changing the context.
+    #
+    # TODO: I don't know what the need for changing the context is, probably
+    # caching of the domain object.  But it doesn't seem right given that
+    # usually context['user'] contains the user who made the request.
+    user_dict = model_dictize.user_dictize(user, context)
+
     context['user'] = user
     context['id'] = user.id
     log.debug('Created user %s' % str(user.name))
-    return model_dictize.user_dictize(user, context)
+    return user_dict
 
 ## Modifications for rest api
 

ckan/logic/action/get.py

@@ -522,7 +522,6 @@ def user_list(context, data_dict):
 
     for user in query.all():
         result_dict = model_dictize.user_dictize(user[0], context)
-        del result_dict['apikey']
         users_list.append(result_dict)
 
     return users_list
@@ -818,7 +817,6 @@ def user_show(context, data_dict):
 
     '''
     model = context['model']
-    user = context['user']
 
     id = data_dict.get('id',None)
     provided_user = data_dict.get('user_obj',None)
@@ -836,11 +834,6 @@ def user_show(context, data_dict):
 
     user_dict = model_dictize.user_dictize(user_obj,context)
 
-    if not (Authorizer().is_sysadmin(unicode(user)) or user == user_obj.name):
-        # If not sysadmin or the same user, strip sensible info
-        del user_dict['apikey']
-        del user_dict['reset_key']
-
     revisions_q = model.Session.query(model.Revision
             ).filter_by(author=user_obj.name)
 

ckan/tests/lib/test_dictization.py

@@ -13,6 +13,7 @@
                                                 activity_dictize,
                                                 package_to_api1,
                                                 package_to_api2,
+                                                user_dictize,
                                                )
 from ckan.lib.dictization.model_save import (package_dict_save,
                                              resource_dict_save,
@@ -871,7 +872,8 @@ def test_16_group_dictized(self):
         group = model.Session.query(model.Group).filter_by(name=u'help').one()
 
         context = {"model": model,
-                  "session": model.Session}
+                  "session": model.Session,
+                  "user": None}
 
         group_dictized = group_dictize(group, context)
 
@@ -897,8 +899,7 @@ def test_16_group_dictized(self):
                               'fullname': None,
                               'name': u'annafan',
                               'number_administered_packages': 1L,
-                              'number_of_edits': 0L,
-                              'reset_key': None}],
+                              'number_of_edits': 0L}],
                     'name': u'help',
                     'display_name': u'help',
                     'image_url': u'',
@@ -1068,3 +1069,94 @@ def test_21_package_dictization_with_deleted_group(self):
         assert_not_in('test-group-2', [ g['name'] for g in result['groups'] ])
         assert_in('test-group-1', [ g['name'] for g in result['groups'] ])
 
+    def test_22_user_dictize_as_sysadmin(self):
+        '''Sysadmins should be allowed to see certain sensitive data.'''
+        context = {
+            'model': model,
+            'session': model.Session,
+            'user': 'testsysadmin',
+        }
+
+        user = model.User.by_name('tester')
+
+        user_dict = user_dictize(user, context)
+
+        # Check some of the non-sensitive data
+        assert 'name' in user_dict
+        assert 'about' in user_dict
+
+        # Check sensitive data is available
+        assert 'apikey' in user_dict
+        assert 'reset_key' in user_dict
+
+        # Passwords should never be available
+        assert 'password' not in user_dict
+
+    def test_23_user_dictize_as_same_user(self):
+        '''User should be able to see their own sensitive data.'''
+        context = {
+            'model': model,
+            'session': model.Session,
+            'user': 'tester',
+        }
+
+        user = model.User.by_name('tester')
+
+        user_dict = user_dictize(user, context)
+
+        # Check some of the non-sensitive data
+        assert 'name' in user_dict
+        assert 'about' in user_dict
+
+        # Check sensitive data is available
+        assert 'apikey' in user_dict
+        assert 'reset_key' in user_dict
+
+        # Passwords should never be available
+        assert 'password' not in user_dict
+
+    def test_24_user_dictize_as_other_user(self):
+        '''User should not be able to see other's sensitive data.'''
+        context = {
+            'model': model,
+            'session': model.Session,
+            'user': 'annafan',
+        }
+
+        user = model.User.by_name('tester')
+
+        user_dict = user_dictize(user, context)
+
+        # Check some of the non-sensitive data
+        assert 'name' in user_dict
+        assert 'about' in user_dict
+
+        # Check sensitive data is available
+        assert 'apikey' not in user_dict
+        assert 'reset_key' not in user_dict
+
+        # Passwords should never be available
+        assert 'password' not in user_dict
+
+    def test_25_user_dictize_as_anonymous(self):
+        '''Anonymous should not be able to see other's sensitive data.'''
+        context = {
+            'model': model,
+            'session': model.Session,
+            'user': '',
+        }
+
+        user = model.User.by_name('tester')
+
+        user_dict = user_dictize(user, context)
+
+        # Check some of the non-sensitive data
+        assert 'name' in user_dict
+        assert 'about' in user_dict
+
+        # Check sensitive data is available
+        assert 'apikey' not in user_dict
+        assert 'reset_key' not in user_dict
+
+        # Passwords should never be available
+        assert 'password' not in user_dict