[#2784] User email is treated as sensitive data

Cherry-picked from:
	bd860dc
	7fe6c76
	21ca66c

Conflicts:

	ckan/controllers/user.py
	ckan/tests/lib/test_dictization.py

ckan/controllers/user.py

@@ -329,12 +329,18 @@ def request_reset(self):
 
     def perform_reset(self, id):
         context = {'model': model, 'session': model.Session,
-                   'user': c.user}
+                   'user': c.user,
+                   'keep_sensitive_data': True}
 
         data_dict = {'id':id}
 
         try:
-            user_dict = get_action('user_show')(context,data_dict)
+            user_dict = get_action('user_show')(context, data_dict)
+
+            # Be a little paranoid, and get rid of sensitive data that's
+            # not needed.
+            user_dict.pop('apikey', None)
+            user_dict.pop('reset_key', None)
             user_obj = context['user_obj']
         except NotFound, e:
             abort(404, _('User not found'))

ckan/lib/dictization/model_dictize.py

@@ -7,6 +7,7 @@
                                   obj_dict_dictize,
                                   table_dictize)
 from ckan.logic import NotFound
+import ckan.authz
 import ckan.misc
 from ckan.lib.helpers import json
 
@@ -273,7 +274,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)
 
@@ -293,7 +293,17 @@ def user_dictize(user, context):
     result_dict['number_of_edits'] = user.number_of_edits()
     result_dict['number_administered_packages'] = user.number_administered_packages()
 
-    return result_dict 
+    requester = context['user']
+
+    if not (ckan.authz.Authorizer().is_sysadmin(unicode(requester)) or
+            requester == user.name or
+            context.get('keep_sensitive_data', False)):
+        # If not sysadmin or the same user, strip sensible info
+        result_dict.pop('apikey', None)
+        result_dict.pop('reset_key', None)
+        result_dict.pop('email', None)
+
+    return result_dict
 
 def task_status_dictize(task_status, context):
     return table_dictize(task_status, context)

ckan/logic/action/create.py

@@ -285,10 +285,22 @@ def user_create(context, data_dict):
     if not context.get('defer_commit'):
         model.repo.commit()
 
+    # A new context is required for dictizing the newly constructed user in
+    # order that all the new user's data is returned, in particular, the
+    # api_key.
+    #
+    # The context is copied so as not to clobber the caller's context dict.
+    user_dictize_context = context.copy()
+    user_dictize_context['keep_sensitive_data'] = True
+    user_dict = user_dictize(user, user_dictize_context)
+
+    # TODO: I don't know what the need for changing the context is here,
+    # probably caching of the domain object.  But it doesn't seem right given
+    # that usually context['user'] contains the user who made the request.
     context['user'] = user
     context['id'] = user.id
     log.debug('Created user %s' % str(user.name))
-    return user_dictize(user, context)
+    return user_dict
 
 ## Modifications for rest api
 

ckan/logic/action/get.py

@@ -285,7 +285,6 @@ def user_list(context, data_dict):
 
     for user in query.all():
         result_dict = user_dictize(user[0], context)
-        del result_dict['apikey']
         users_list.append(result_dict)
 
     return users_list
@@ -467,7 +466,6 @@ def tag_show(context, data_dict):
 def user_show(context, data_dict):
     '''Shows user details'''
     model = context['model']
-    user = context['user']
 
     id = data_dict.get('id',None)
     provided_user = data_dict.get('user_obj',None)
@@ -485,11 +483,6 @@ def user_show(context, data_dict):
 
     user_dict = 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/functional/api/test_user.py

@@ -1,5 +1,6 @@
 from nose.tools import assert_equal
 
+import ckan.logic as logic
 from ckan import model
 from ckan.lib.create_test_data import CreateTestData
 from ckan.tests import TestController as ControllerTestCase
@@ -50,3 +51,52 @@ def test_autocomplete_limit(self):
         print response.json
         assert_equal(len(response.json), 1)
 
+class TestUserActions(object):
+
+    @classmethod
+    def setup_class(cls):
+        CreateTestData.create()
+
+    @classmethod
+    def teardown_class(cls):
+        model.repo.rebuild_db()
+
+    def test_user_create_simple(self):
+        '''Simple creation of a new user by a non-sysadmin user.'''
+        context = {
+            'model': model,
+            'session': model.Session,
+            'user': 'tester'
+        }
+        data_dict = {
+            'name': 'a-new-user',
+            'email': 'a.person@example.com',
+            'password': 'supersecret',
+        }
+
+        user_dict = logic.get_action('user_create')(context, data_dict)
+
+        assert_equal(user_dict['name'], 'a-new-user')
+        assert 'email' in user_dict
+        assert 'apikey' in user_dict
+        assert 'password' not in user_dict
+
+    def test_user_update_simple(self):
+        '''Simple update of a user by themselves.'''
+        context = {
+            'model': model,
+            'session': model.Session,
+            'user': 'annafan',
+        }
+
+        data_dict = {
+            'id': 'annafan',
+            'email': 'anna@example.com',
+        }
+
+        user_dict = logic.get_action('user_update')(context, data_dict)
+
+        assert_equal(user_dict['email'], 'anna@example.com')
+        assert 'apikey' in user_dict
+        assert 'password' not in user_dict
+

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,
@@ -850,7 +851,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)
 
@@ -869,13 +871,11 @@ def test_16_group_dictized(self):
                     'users': [{'about': u'I love reading Annakarenina. My site: <a href="http://anna.com">anna.com</a>',
                               'display_name': u'annafan',
                               'capacity' : 'member',
-                              'email': None,
                               'email_hash': 'd41d8cd98f00b204e9800998ecf8427e',
                               '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',
                     'packages': [{'author': None,
@@ -999,3 +999,99 @@ def test_20_activity_save(self):
 
         # We didn't pass in any data so this should be empty.
         assert not got['data']
+
+    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
+        assert 'email' 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
+        assert 'email' 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 not available
+        assert 'apikey' not in user_dict
+        assert 'reset_key' not in user_dict
+        assert 'email' 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 not available
+        assert 'apikey' not in user_dict
+        assert 'reset_key' not in user_dict
+        assert 'email' not in user_dict
+
+        # Passwords should never be available
+        assert 'password' not in user_dict