From 74f649c9e3eb0690f5e48f939b4546b415b0777b Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Wed, 14 Aug 2013 21:42:22 -0300 Subject: [PATCH] [#1178] Admins can invite users The invited user starts in pending state, with the password reset key set. We still have to send an email to the user telling him/her to change the password and log in. I had to change authorization code to only automatically unauthorize deleted users, not pending. This was because the users needs to be able to perform the password reset when pending, to be able to become active. --- ckan/controllers/user.py | 2 +- ckan/logic/action/create.py | 37 ++++++++++++++++++++++ ckan/logic/auth/create.py | 2 ++ ckan/logic/schema.py | 1 + ckan/new_authz.py | 4 +-- ckan/tests/functional/test_user.py | 20 +++++++++++- ckan/tests/logic/test_action.py | 50 ++++++++++++++++++++++++++++++ ckan/tests/logic/test_auth.py | 22 ++++--------- dev-requirements.txt | 1 + 9 files changed, 119 insertions(+), 20 deletions(-) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index 4e76b098d37..000f3db5a0c 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -447,7 +447,7 @@ def perform_reset(self, id): # FIXME We should reset the reset key when it is used to prevent # reuse of the url context = {'model': model, 'session': model.Session, - 'user': c.user, + 'user': c.user or id, 'keep_sensitive_data': True} data_dict = {'id': id} diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 1fc0fb6e058..346fd30e680 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -1,6 +1,8 @@ '''API functions for adding data to CKAN.''' import logging +import random +import re from pylons import config import paste.deploy.converters @@ -15,6 +17,7 @@ import ckan.lib.dictization.model_dictize as model_dictize import ckan.lib.dictization.model_save as model_save import ckan.lib.navl.dictization_functions +import ckan.lib.navl.validators as validators from ckan.common import _ @@ -836,6 +839,40 @@ def user_create(context, data_dict): log.debug('Created user {name}'.format(name=user.name)) return user_dict +def user_invite(context, data_dict): + '''docstring''' + _check_access('user_invite', context, data_dict) + + user_invite_schema = { + 'email': [validators.not_empty, unicode] + } + _, errors = _validate(data_dict, user_invite_schema, context) + if errors: + raise ValidationError(errors) + + while True: + try: + import ckan.lib.mailer + name = _get_random_username_from_email(data_dict['email']) + password = str(random.SystemRandom().random()) + data_dict['name'] = name + data_dict['password'] = password + data_dict['state'] = ckan.model.State.PENDING + user_dict = _get_action('user_create')(context, data_dict) + user = ckan.model.User.get(user_dict['id']) + ckan.lib.mailer.create_reset_key(user) + return model_dictize.user_dictize(user, context) + except ValidationError as e: + if 'name' not in e.error_dict: + raise e + +def _get_random_username_from_email(email): + localpart = email.split('@')[0] + cleaned_localpart = re.sub(r'[^\w]', '', localpart) + random_number = random.SystemRandom().random() * 10000 + name = '%s-%d' % (cleaned_localpart, random_number) + return name + ## Modifications for rest api def package_create_rest(context, data_dict): diff --git a/ckan/logic/auth/create.py b/ckan/logic/auth/create.py index bf9c3d17ea3..fafbc5a2e4c 100644 --- a/ckan/logic/auth/create.py +++ b/ckan/logic/auth/create.py @@ -112,6 +112,8 @@ def user_create(context, data_dict=None): else: return {'success': True} +def user_invite(context, data_dict=None): + return {'success': False} def _check_group_auth(context, data_dict): # FIXME This code is shared amoung other logic.auth files and should be diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py index f8fe8df7fbd..a8cd4ce6779 100644 --- a/ckan/logic/schema.py +++ b/ckan/logic/schema.py @@ -395,6 +395,7 @@ def default_user_schema(): 'apikey': [ignore], 'reset_key': [ignore], 'activity_streams_email_notifications': [ignore_missing], + 'state': [ignore_missing], } return schema diff --git a/ckan/new_authz.py b/ckan/new_authz.py index b98434a9b45..62a4e88a978 100644 --- a/ckan/new_authz.py +++ b/ckan/new_authz.py @@ -151,8 +151,8 @@ def is_authorized(action, context, data_dict=None): user = _get_user(username) if user: - # inactive users are always unauthorized - if not user.is_active(): + # deleted users are always unauthorized + if user.is_deleted(): return {'success': False} # sysadmins can do anything unless the auth_sysadmins_check # decorator was used in which case they are treated like all other diff --git a/ckan/tests/functional/test_user.py b/ckan/tests/functional/test_user.py index f0ff38cfdac..fe30f53a505 100644 --- a/ckan/tests/functional/test_user.py +++ b/ckan/tests/functional/test_user.py @@ -959,7 +959,7 @@ def test_perform_reset_user_password_link_user_incorrect(self): def test_perform_reset_activates_pending_user(self): password = 'password' params = { 'password1': password, 'password2': password } - user = CreateTestData.create_user(name='username', + user = CreateTestData.create_user(name='pending_user', email='user@email.com') user.set_pending() create_reset_key(user) @@ -973,3 +973,21 @@ def test_perform_reset_activates_pending_user(self): user = model.User.get(user.id) assert user.is_active(), user + + def test_perform_reset_doesnt_activate_deleted_user(self): + password = 'password' + params = { 'password1': password, 'password2': password } + user = CreateTestData.create_user(name='deleted_user', + email='user@email.com') + user.delete() + create_reset_key(user) + assert user.is_deleted(), user.state + + offset = url_for(controller='user', + action='perform_reset', + id=user.id, + key=user.reset_key) + res = self.app.post(offset, params=params, status=302) + + user = model.User.get(user.id) + assert user.is_deleted(), user diff --git a/ckan/tests/logic/test_action.py b/ckan/tests/logic/test_action.py index fecbdf96b53..7bf17bd094e 100644 --- a/ckan/tests/logic/test_action.py +++ b/ckan/tests/logic/test_action.py @@ -6,6 +6,7 @@ from nose.plugins.skip import SkipTest from pylons import config import datetime +import mock import vdm.sqlalchemy import ckan @@ -561,6 +562,55 @@ def test_12_user_update_errors(self): for expected_message in test_call['messages']: assert expected_message[1] in ''.join(res_obj['error'][expected_message[0]]) + def test_user_invite(self): + email_username = 'invited_user$ckan' + email = '%s@email.com' % email_username + user_dict = {'email': email} + postparams = '%s=1' % json.dumps(user_dict) + extra_environ = {'Authorization': str(self.sysadmin_user.apikey)} + + res = self.app.post('/api/action/user_invite', params=postparams, + extra_environ=extra_environ) + + res_obj = json.loads(res.body) + user = model.User.get(res_obj['result']['id']) + expected_username = email_username.replace('$', '') + assert res_obj['success'] is True, res_obj + assert user.email == email, (user.email, email) + assert user.name.startswith(expected_username), (user.name, expected_username) + assert user.is_pending(), user + assert user.reset_key is not None, user + + def test_user_invite_without_email_raises_error(self): + user_dict = {} + postparams = '%s=1' % json.dumps(user_dict) + extra_environ = {'Authorization': str(self.sysadmin_user.apikey)} + + res = self.app.post('/api/action/user_invite', params=postparams, + extra_environ=extra_environ, + status=StatusCodes.STATUS_409_CONFLICT) + + res_obj = json.loads(res.body) + assert res_obj['success'] is False, res_obj + assert 'email' in res_obj['error'], res_obj + + @mock.patch('ckan.logic.action.create._get_random_username_from_email') + def test_user_invite_should_work_even_if_tried_username_already_exists(self, random_username_mock): + email = 'invited_user@email.com' + user_dict = {'email': email} + postparams = '%s=1' % json.dumps(user_dict) + extra_environ = {'Authorization': str(self.sysadmin_user.apikey)} + + usernames = ['first', 'first', 'second'] + random_username_mock.side_effect = lambda email: usernames.pop(0) + + for _ in range(2): + res = self.app.post('/api/action/user_invite', params=postparams, + extra_environ=extra_environ) + + res_obj = json.loads(res.body) + assert res_obj['success'] is True, res_obj + def test_user_delete(self): name = 'normal_user' CreateTestData.create_user(name) diff --git a/ckan/tests/logic/test_auth.py b/ckan/tests/logic/test_auth.py index 2b5204e878a..13e7b38f2a5 100644 --- a/ckan/tests/logic/test_auth.py +++ b/ckan/tests/logic/test_auth.py @@ -49,6 +49,12 @@ def create_user(self, name): class TestAuthUsers(TestAuth): + def test_only_sysadmins_can_invite_users(self): + username = 'normal_user' + self.create_user(username) + + assert not new_authz.is_authorized_boolean('user_invite', {'user': username}) + def test_only_sysadmins_can_delete_users(self): username = 'username' user = {'id': username} @@ -73,22 +79,6 @@ def test_auth_deleted_users_are_always_unauthorized(self): del new_authz._AuthFunctions._functions['always_success'] - def test_auth_pending_users_are_always_unauthorized(self): - always_success = lambda x,y: {'success': True} - new_authz._AuthFunctions._build() - new_authz._AuthFunctions._functions['always_success'] = always_success - # We can't reuse the username with the other tests because we can't - # rebuild_db(), because in the setup_class we get the sysadmin. If we - # rebuild the DB, we would delete the sysadmin as well. - username = 'pending_user' - self.create_user(username) - user = model.User.get(username) - user.state = model.State.PENDING - - assert not new_authz.is_authorized_boolean('always_success', {'user': username}) - - del new_authz._AuthFunctions._functions['always_success'] - class TestAuthOrgs(TestAuth): def test_01_create_users(self): diff --git a/dev-requirements.txt b/dev-requirements.txt index a445712d239..a781573b0b8 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -7,3 +7,4 @@ nose==1.3.0 pep8==1.4.6 Sphinx==1.2b1 polib==1.0.3 +mock==1.0.1