diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index 301ef856fb6..61ccec97feb 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -512,8 +512,11 @@ def perform_reset(self, id): if request.method == 'POST': try: context['reset_password'] = True + user_state = user_dict['state'] new_password = self._get_form_password() - user_dict['password'] = new_password + username = request.params.get('name') + if (username is not None and username != ''): + user_dict['name'] = username user_dict['reset_key'] = c.reset_key user_dict['state'] = model.State.ACTIVE user = get_action('user_update')(context, user_dict) @@ -531,6 +534,7 @@ def perform_reset(self, id): h.flash_error(u'%r' % e.error_dict) except ValueError, ve: h.flash_error(unicode(ve)) + user_dict['state'] = user_state c.user_dict = user_dict return render('user/perform_reset.html') diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 0d1b249288c..3f82629ba1c 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -668,7 +668,7 @@ def user_update(context, data_dict): '''Update a user account. Normal users can only update their own user accounts. Sysadmins can update - any user account. + any user account. Can not modify exisiting user's name. For further parameters see :py:func:`~ckan.logic.action.create.user_create`. diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index 41e08e48d2f..e87a1ba6202 100644 --- a/ckan/logic/validators.py +++ b/ckan/logic/validators.py @@ -568,10 +568,9 @@ def user_name_validator(key, data, errors, context): raise Invalid(_('User names must be strings')) user = model.User.get(new_user_name) + user_obj_from_context = context.get('user_obj') if user is not None: # A user with new_user_name already exists in the database. - - user_obj_from_context = context.get('user_obj') if user_obj_from_context and user_obj_from_context.id == user.id: # If there's a user_obj in context with the same id as the user # found in the db, then we must be doing a user_update and not @@ -582,6 +581,12 @@ def user_name_validator(key, data, errors, context): # name, so you can create a new user with that name or update an # existing user's name to that name. errors[key].append(_('That login name is not available.')) + elif user_obj_from_context: + old_user = model.User.get(user_obj_from_context.id) + if old_user is not None and old_user.state != model.State.PENDING: + errors[key].append(_('That login name can not be modified.')) + else: + return def user_both_passwords_entered(key, data, errors, context): diff --git a/ckan/templates/user/edit_user_form.html b/ckan/templates/user/edit_user_form.html index 9643b35b0ea..dfdf4e8c51e 100644 --- a/ckan/templates/user/edit_user_form.html +++ b/ckan/templates/user/edit_user_form.html @@ -5,7 +5,7 @@
{{ _('Change details') }} - {{ form.input('name', label=_('Username'), id='field-username', value=data.name, error=errors.name, classes=['control-medium'], is_required=true) }} + {{ form.input('name', label=_('Username'), id='field-username', value=data.name, error=errors.name, classes=['control-medium'], attrs={'readonly': ''}) }} {{ form.input('fullname', label=_('Full name'), id='field-fullname', value=data.fullname, error=errors.fullname, placeholder=_('eg. Joe Bloggs'), classes=['control-medium']) }} diff --git a/ckan/templates/user/perform_reset.html b/ckan/templates/user/perform_reset.html index dc6d7f21e16..b4c67f0ff93 100644 --- a/ckan/templates/user/perform_reset.html +++ b/ckan/templates/user/perform_reset.html @@ -17,6 +17,11 @@

{% block form %}
{{ form.errors(error_summary) }} + {% if c.user_dict['state'] == 'pending' %} +

{{ _('You can also change username. It can not be modified later.') }}

+ {{ form.input("name", id="field-name", label=_("Username"), type="text", value=c.user_dict["name"], + error='', attrs={'autocomplete': 'no'}, classes=["control-medium"]) }} + {% endif %} {{ form.input("password1", id="field-password", label=_("Password"), type="password", value='', error='', attrs={'autocomplete': 'no'}, classes=["control-medium"]) }} {{ form.input("password2", id="field-confirm-password", label=_("Confirm"), type="password", value='', error='', attrs={'autocomplete': 'no'}, classes=["control-medium"]) }}
diff --git a/ckan/tests/controllers/test_user.py b/ckan/tests/controllers/test_user.py index dfc8087c519..7dabfd2e167 100644 --- a/ckan/tests/controllers/test_user.py +++ b/ckan/tests/controllers/test_user.py @@ -272,11 +272,9 @@ def test_edit_user_logged_in_username_change(self): # new values form['name'] = 'new-name' - response = submit_and_follow(app, form, name='save') - response = helpers.webtest_maybe_follow(response) - - expected_url = url_for(controller='user', action='read', id='new-name') - assert response.request.path == expected_url + env = {'REMOTE_USER': user['name'].encode('ascii')} + response = webtest_submit(form, 'save', status=200, extra_environ=env) + assert_true('That login name can not be modified' in response) def test_edit_user_logged_in_username_change_by_name(self): user_pass = 'pass' @@ -303,11 +301,9 @@ def test_edit_user_logged_in_username_change_by_name(self): # new values form['name'] = 'new-name' - response = submit_and_follow(app, form, name='save') - response = helpers.webtest_maybe_follow(response) - - expected_url = url_for(controller='user', action='read', id='new-name') - assert response.request.path == expected_url + env = {'REMOTE_USER': user['name'].encode('ascii')} + response = webtest_submit(form, 'save', status=200, extra_environ=env) + assert_true('That login name can not be modified' in response) def test_edit_user_logged_in_username_change_by_id(self): user_pass = 'pass' @@ -334,11 +330,9 @@ def test_edit_user_logged_in_username_change_by_id(self): # new values form['name'] = 'new-name' - response = submit_and_follow(app, form, name='save') - response = helpers.webtest_maybe_follow(response) - - expected_url = url_for(controller='user', action='read', id='new-name') - assert response.request.path == expected_url + env = {'REMOTE_USER': user['name'].encode('ascii')} + response = webtest_submit(form, 'save', status=200, extra_environ=env) + assert_true('That login name can not be modified' in response) def test_perform_reset_for_key_change(self): password = 'password' diff --git a/ckan/tests/logic/action/test_update.py b/ckan/tests/logic/action/test_update.py index 24a195f2c51..dac28949ad9 100644 --- a/ckan/tests/logic/action/test_update.py +++ b/ckan/tests/logic/action/test_update.py @@ -65,24 +65,12 @@ def test_user_update_name(self): # 1. Setup. user = factories.User() + user['name'] = 'updated' - # 2. Call the function that's being tested, once only. - # FIXME we have to pass the email address and password to user_update - # even though we're not updating those fields, otherwise validation - # fails. - helpers.call_action('user_update', id=user['name'], - email=user['email'], - password=factories.User.attributes()['password'], - name='updated', - ) - - # 3. Make assertions about the return value and/or side-effects. - updated_user = helpers.call_action('user_show', id=user['id']) - # Note that we check just the field we were trying to update, not the - # entire dict, only assert what we're actually testing. - assert updated_user['name'] == 'updated' - - # 4. Do nothing else! + # 2. Make assertions about the return value and/or side-effects. + assert_raises(logic.ValidationError, + helpers.call_action, 'user_update', + **user) ## END-BEFORE @@ -249,7 +237,7 @@ def test_user_update_activity_stream(self): helpers.call_action('user_update', id=user['name'], email=user['email'], password=factories.User.attributes()['password'], - name='updated', + fullname='updated full name', ) activity_stream = helpers.call_action('user_activity_list', @@ -292,7 +280,7 @@ def test_user_update_with_custom_schema(self): helpers.call_action('user_update', context={'schema': schema}, id=user['name'], email=user['email'], password=factories.User.attributes()['password'], - name='updated', + fullname='updated full name', ) # Since we passed user['name'] to user_update as the 'id' param, @@ -307,7 +295,6 @@ def test_user_update_multiple(self): params = { 'id': user['id'], - 'name': 'updated_name', 'fullname': 'updated full name', 'about': 'updated about', # FIXME: We shouldn't have to put email here since we're not @@ -321,7 +308,6 @@ def test_user_update_multiple(self): helpers.call_action('user_update', **params) updated_user = helpers.call_action('user_show', id=user['id']) - assert updated_user['name'] == 'updated_name' assert updated_user['fullname'] == 'updated full name' assert updated_user['about'] == 'updated about' @@ -333,7 +319,6 @@ def test_user_update_does_not_return_password(self): params = { 'id': user['id'], - 'name': 'updated_name', 'fullname': 'updated full name', 'about': 'updated about', 'email': user['email'], @@ -351,7 +336,6 @@ def test_user_update_does_not_return_apikey(self): params = { 'id': user['id'], - 'name': 'updated_name', 'fullname': 'updated full name', 'about': 'updated about', 'email': user['email'], @@ -373,7 +357,6 @@ def test_user_update_does_not_return_reset_key(self): params = { 'id': user['id'], - 'name': 'updated_name', 'fullname': 'updated full name', 'about': 'updated about', 'email': user['email'],