Skip to content

Commit

Permalink
Merge 43cb086 into 0ad2f8a
Browse files Browse the repository at this point in the history
  • Loading branch information
wardi committed Sep 17, 2015
2 parents 0ad2f8a + 43cb086 commit 94722c1
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 10 deletions.
8 changes: 5 additions & 3 deletions ckan/lib/dictization/model_dictize.py
Expand Up @@ -556,15 +556,15 @@ def user_list_dictize(obj_list, context,
def member_dictize(member, context):
return d.table_dictize(member, context)

def user_dictize(user, context):
def user_dictize(user, context, include_password_hash=False):

if context.get('with_capacity'):
user, capacity = user
result_dict = d.table_dictize(user, context, capacity=capacity)
else:
result_dict = d.table_dictize(user, context)

del result_dict['password']
password_hash = result_dict.pop('password')
del result_dict['reset_key']

result_dict['display_name'] = user.display_name
Expand All @@ -590,11 +590,13 @@ def user_dictize(user, context):
result_dict['apikey'] = apikey
result_dict['email'] = email

## this should not really really be needed but tests need it
if authz.is_sysadmin(requester):
result_dict['apikey'] = apikey
result_dict['email'] = email

if include_password_hash:
result_dict['password_hash'] = password_hash

model = context['model']
session = model.Session

Expand Down
5 changes: 5 additions & 0 deletions ckan/logic/action/create.py
Expand Up @@ -23,6 +23,7 @@
import ckan.lib.datapreview

from ckan.common import _
from ckan import authz

# FIXME this looks nasty and should be shared better
from ckan.logic.action.update import _update_package_relationship
Expand Down Expand Up @@ -1016,6 +1017,10 @@ def user_create(context, data_dict):
session.rollback()
raise ValidationError(errors)

# allow importing password_hash from another ckan
if authz.is_sysadmin(context['user']) and 'password_hash' in data:
data['_password'] = data.pop('password_hash')

user = model_save.user_dict_save(data, context)

# Flush the session to cause user.id to be initialised, because
Expand Down
22 changes: 15 additions & 7 deletions ckan/logic/action/get.py
Expand Up @@ -1479,8 +1479,11 @@ def user_show(context, data_dict):
(optional, default:``False``, limit:50)
:type include_datasets: boolean
:param include_num_followers: Include the number of followers the user has
(optional, default:``False``)
(optional, default:``False``)
:type include_num_followers: boolean
:param include_password_hash: Include the stored password hash
(sysadmin only, optional, default:``False``)
:type include_password_hash: boolean
:returns: the details of the user. Includes email_hash, number_of_edits and
number_created_packages (which excludes draft or private datasets
Expand Down Expand Up @@ -1508,24 +1511,29 @@ def user_show(context, data_dict):

# include private and draft datasets?
requester = context.get('user')
sysadmin = False
if requester:
sysadmin = authz.is_sysadmin(requester)
requester_looking_at_own_account = requester == user_obj.name
include_private_and_draft_datasets = \
authz.is_sysadmin(requester) or \
requester_looking_at_own_account
include_private_and_draft_datasets = (
sysadmin or requester_looking_at_own_account)
else:
include_private_and_draft_datasets = False
context['count_private_and_draft_datasets'] = \
include_private_and_draft_datasets

user_dict = model_dictize.user_dictize(user_obj, context)
include_password_hash = sysadmin and asbool(
data_dict.get('include_password_hash', False))

user_dict = model_dictize.user_dictize(
user_obj, context, include_password_hash)

if context.get('return_minimal'):
log.warning('Use of the "return_minimal" in user_show is '
'deprecated.')
return user_dict

if data_dict.get('include_datasets', False):
if asbool(data_dict.get('include_datasets', False)):
user_dict['datasets'] = []

fq = "+creator_user_id:{0}".format(user_dict['id'])
Expand All @@ -1543,7 +1551,7 @@ def user_show(context, data_dict):
data_dict=search_dict) \
.get('results')

if data_dict.get('include_num_followers', False):
if asbool(data_dict.get('include_num_followers', False)):
user_dict['num_followers'] = logic.get_action('user_follower_count')(
{'model': model, 'session': model.Session},
{'id': user_dict['id']})
Expand Down
5 changes: 5 additions & 0 deletions ckan/logic/action/update.py
Expand Up @@ -25,6 +25,7 @@


from ckan.common import _, request
from ckan import authz

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -699,6 +700,10 @@ def user_update(context, data_dict):
session.rollback()
raise ValidationError(errors)

# allow importing password_hash from another ckan
if authz.is_sysadmin(context['user']) and 'password_hash' in data:
data['_password'] = data.pop('password_hash')

user = model_save.user_dict_save(data, context)

activity_dict = {
Expand Down
1 change: 1 addition & 0 deletions ckan/logic/schema.py
Expand Up @@ -430,6 +430,7 @@ def default_user_schema():
'name': [not_empty, name_validator, user_name_validator, unicode],
'fullname': [ignore_missing, unicode],
'password': [user_password_validator, user_password_not_empty, ignore_missing, unicode],
'password_hash': [ignore_missing, unicode],
'email': [not_empty, unicode],
'about': [ignore_missing, user_about_validator, unicode],
'created': [ignore],
Expand Down
5 changes: 5 additions & 0 deletions ckan/logic/validators.py
Expand Up @@ -619,6 +619,11 @@ def user_password_not_empty(key, data, errors, context):
'''Only check if password is present if the user is created via action API.
If not, user_both_passwords_entered will handle the validation'''

# sysadmin may provide password_hash directly for importing users
if (data.get(('password_hash',), missing) is not missing and
authz.is_sysadmin(context.get('user'))):
return

if not ('password1',) in data and not ('password2',) in data:
password = data.get(('password',),None)
if not password:
Expand Down
36 changes: 36 additions & 0 deletions ckan/tests/logic/action/test_create.py
Expand Up @@ -636,3 +636,39 @@ def test_create_matches_show(self):
assert sorted(created.keys()) == sorted(shown.keys())
for k in created.keys():
assert created[k] == shown[k], k


class TestUserCreate(helpers.FunctionalTestBase):

def test_user_create_with_password_hash(self):
sysadmin = factories.Sysadmin()
context = {
'user': sysadmin['name'],
}

user = helpers.call_action(
'user_create',
context=context,
email='test@example.com',
name='test',
password_hash='pretend-this-is-a-valid-hash')

user_obj = model.User.get(user['id'])
assert user_obj.password == 'pretend-this-is-a-valid-hash'

def test_user_create_password_hash_not_for_normal_users(self):
normal_user = factories.User()
context = {
'user': normal_user['name'],
}

user = helpers.call_action(
'user_create',
context=context,
email='test@example.com',
name='test',
password='required',
password_hash='pretend-this-is-a-valid-hash')

user_obj = model.User.get(user['id'])
assert user_obj.password != 'pretend-this-is-a-valid-hash'
28 changes: 28 additions & 0 deletions ckan/tests/logic/action/test_get.py
Expand Up @@ -568,6 +568,7 @@ def test_user_show_default_values(self):
assert 'apikey' not in got_user
assert 'email' not in got_user
assert 'datasets' not in got_user
assert 'password_hash' not in got_user

def test_user_show_keep_email(self):

Expand Down Expand Up @@ -595,6 +596,16 @@ def test_user_show_keep_apikey(self):
assert 'password' not in got_user
assert 'reset_key' not in got_user

def test_user_show_normal_user_no_password_hash(self):

user = factories.User()

got_user = helpers.call_action('user_show',
id=user['id'],
include_password_hash=True)

assert 'password_hash' not in got_user

def test_user_show_for_myself(self):

user = factories.User()
Expand Down Expand Up @@ -623,6 +634,23 @@ def test_user_show_sysadmin_values(self):
assert 'password' not in got_user
assert 'reset_key' not in got_user

def test_user_show_sysadmin_password_hash(self):

user = factories.User(password='test')

sysadmin = factories.User(sysadmin=True)

got_user = helpers.call_action('user_show',
context={'user': sysadmin['name']},
id=user['id'],
include_password_hash=True)

assert got_user['email'] == user['email']
assert got_user['apikey'] == user['apikey']
assert 'password_hash' in got_user
assert 'password' not in got_user
assert 'reset_key' not in got_user

def test_user_show_include_datasets(self):

user = factories.User()
Expand Down
37 changes: 37 additions & 0 deletions ckan/tests/logic/action/test_update.py
Expand Up @@ -10,6 +10,7 @@
import ckan.plugins as p
import ckan.tests.helpers as helpers
import ckan.tests.factories as factories
from ckan import model

assert_equals = nose.tools.assert_equals
assert_raises = nose.tools.assert_raises
Expand Down Expand Up @@ -748,3 +749,39 @@ def test_app_globals_set_if_defined(self):
assert hasattr(app_globals.app_globals, globals_key)

assert_equals(getattr(app_globals.app_globals, globals_key), value)


class TestUserUpdate(helpers.FunctionalTestBase):

def test_user_update_with_password_hash(self):
sysadmin = factories.Sysadmin()
context = {
'user': sysadmin['name'],
}

user = helpers.call_action(
'user_update',
context=context,
email='test@example.com',
id=sysadmin['name'],
password_hash='pretend-this-is-a-valid-hash')

user_obj = model.User.get(user['id'])
assert user_obj.password == 'pretend-this-is-a-valid-hash'

def test_user_create_password_hash_not_for_normal_users(self):
normal_user = factories.User()
context = {
'user': normal_user['name'],
}

user = helpers.call_action(
'user_update',
context=context,
email='test@example.com',
id=normal_user['name'],
password='required',
password_hash='pretend-this-is-a-valid-hash')

user_obj = model.User.get(user['id'])
assert user_obj.password != 'pretend-this-is-a-valid-hash'

0 comments on commit 94722c1

Please sign in to comment.