Merge branch 'master' of github.com:okfn/ckan into feature-2536-clean…

…-activity-stream

ckan/controllers/user.py

@@ -107,10 +107,6 @@ def read(self, id=None):
             abort(401, _('Not authorized to see this page'))
 
         context['with_related'] = True
-        try:
-            user_dict = get_action('user_show')(context,data_dict)
-        except NotFound:
-            h.redirect_to(controller='user', action='login', id=None)
 
         self._setup_template_variables(context, data_dict)
 
@@ -388,12 +384,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)
+
+            # 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

@@ -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,16 @@ 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 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)
+
     model = context['model']
     session = model.Session
 

ckan/logic/action/create.py

@@ -692,10 +692,19 @@ def user_create(context, data_dict):
     if not context.get('defer_commit'):
         model.repo.commit()
 
-    context['user'] = user
+    # 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 = model_dictize.user_dictize(user, user_dictize_context)
+
+    context['user_obj'] = 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

@@ -520,7 +520,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
@@ -816,7 +815,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)
@@ -834,11 +832,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/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,
@@ -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)
 
@@ -892,13 +894,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' : 'public',
-                              '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',
                     'image_url': u'',
@@ -1068,3 +1068,98 @@ 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
+        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