diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index 4a0578a3dee..ca85c85597b 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -388,12 +388,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')) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 17ac70a0205..cda8b7f69c9 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/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 diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 94b5f74ce32..9ed5a1ea586 100644 --- a/ckan/logic/action/create.py +++ b/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 diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index f74843e835b..6308a9cd0c7 100644 --- a/ckan/logic/action/get.py +++ b/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) diff --git a/ckan/tests/functional/api/test_user.py b/ckan/tests/functional/api/test_user.py index 13ce706f2f4..06dd35dd7fc 100644 --- a/ckan/tests/functional/api/test_user.py +++ b/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 + diff --git a/ckan/tests/lib/test_dictization.py b/ckan/tests/lib/test_dictization.py index ae95f0220ee..a7806087b13 100644 --- a/ckan/tests/lib/test_dictization.py +++ b/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: anna.com', '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