diff --git a/rhodecode/config/routing.py b/rhodecode/config/routing.py --- a/rhodecode/config/routing.py +++ b/rhodecode/config/routing.py @@ -531,9 +531,7 @@ def make_map(config): action='my_account_update', conditions={'method': ['POST']}) m.connect('my_account_password', '/my_account/password', - action='my_account_password', conditions={'method': ['GET']}) - m.connect('my_account_password', '/my_account/password', - action='my_account_password_update', conditions={'method': ['POST']}) + action='my_account_password', conditions={'method': ['GET', 'POST']}) m.connect('my_account_repos', '/my_account/repos', action='my_account_repos', conditions={'method': ['GET']}) diff --git a/rhodecode/controllers/admin/my_account.py b/rhodecode/controllers/admin/my_account.py --- a/rhodecode/controllers/admin/my_account.py +++ b/rhodecode/controllers/admin/my_account.py @@ -32,6 +32,7 @@ from pylons.controllers.util import redi from pylons.i18n.translation import _ from sqlalchemy.orm import joinedload +from rhodecode import forms from rhodecode.lib import helpers as h from rhodecode.lib import auth from rhodecode.lib.auth import ( @@ -39,10 +40,12 @@ from rhodecode.lib.auth import ( from rhodecode.lib.base import BaseController, render from rhodecode.lib.utils2 import safe_int, md5 from rhodecode.lib.ext_json import json + +from rhodecode.model.validation_schema.schemas import user_schema from rhodecode.model.db import ( Repository, PullRequest, PullRequestReviewers, UserEmailMap, User, UserFollowing) -from rhodecode.model.forms import UserForm, PasswordChangeForm +from rhodecode.model.forms import UserForm from rhodecode.model.scm import RepoList from rhodecode.model.user import UserModel from rhodecode.model.repo import RepoModel @@ -185,38 +188,44 @@ class MyAccountController(BaseController force_defaults=False ) - @auth.CSRFRequired() - def my_account_password_update(self): - c.active = 'password' - self.__load_data() - _form = PasswordChangeForm(c.rhodecode_user.username)() - try: - form_result = _form.to_python(request.POST) - UserModel().update_user(c.rhodecode_user.user_id, **form_result) - instance = c.rhodecode_user.get_instance() - instance.update_userdata(force_password_change=False) - Session().commit() - session.setdefault('rhodecode_user', {}).update( - {'password': md5(instance.password)}) - session.save() - h.flash(_("Successfully updated password"), category='success') - except formencode.Invalid as errors: - return htmlfill.render( - render('admin/my_account/my_account.html'), - defaults=errors.value, - errors=errors.error_dict or {}, - prefix_error=False, - encoding="UTF-8", - force_defaults=False) - except Exception: - log.exception("Exception updating password") - h.flash(_('Error occurred during update of user password'), - category='error') - return render('admin/my_account/my_account.html') - + @auth.CSRFRequired(except_methods=['GET']) def my_account_password(self): c.active = 'password' self.__load_data() + + schema = user_schema.ChangePasswordSchema().bind( + username=c.rhodecode_user.username) + + form = forms.Form(schema, + buttons=(forms.buttons.save, forms.buttons.reset)) + + if request.method == 'POST': + controls = request.POST.items() + try: + valid_data = form.validate(controls) + UserModel().update_user(c.rhodecode_user.user_id, **valid_data) + instance = c.rhodecode_user.get_instance() + instance.update_userdata(force_password_change=False) + Session().commit() + except forms.ValidationFailure as e: + request.session.flash( + _('Error occurred during update of user password'), + queue='error') + form = e + except Exception: + log.exception("Exception updating password") + request.session.flash( + _('Error occurred during update of user password'), + queue='error') + else: + session.setdefault('rhodecode_user', {}).update( + {'password': md5(instance.password)}) + session.save() + request.session.flash( + _("Successfully updated password"), queue='success') + return redirect(url('my_account_password')) + + c.form = form return render('admin/my_account/my_account.html') def my_account_repos(self): diff --git a/rhodecode/forms/__init__.py b/rhodecode/forms/__init__.py new file mode 100644 --- /dev/null +++ b/rhodecode/forms/__init__.py @@ -0,0 +1,33 @@ +# -*- coding: utf-8 -*- + +# Copyright (C) 2010-2016 RhodeCode GmbH +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License, version 3 +# (only), as published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +# This program is dual-licensed. If you wish to learn more about the +# RhodeCode Enterprise Edition, including its added features, Support services, +# and proprietary license terms, please see https://rhodecode.com/licenses/ + +""" +Base module for form rendering / validation - currently just a wrapper for +deform - later can be replaced with something custom. +""" + +from rhodecode.translation import _ +from deform import Button, Form, widget, ValidationFailure + + +class buttons: + save = Button(name='Save', type='submit') + reset = Button(name=_('Reset'), type='reset') + delete = Button(name=_('Delete'), type='submit') diff --git a/rhodecode/lib/auth.py b/rhodecode/lib/auth.py --- a/rhodecode/lib/auth.py +++ b/rhodecode/lib/auth.py @@ -1116,9 +1116,11 @@ class CSRFRequired(object): For use with the ``webhelpers.secure_form`` helper functions. """ - def __init__(self, token=csrf_token_key, header='X-CSRF-Token'): + def __init__(self, token=csrf_token_key, header='X-CSRF-Token', + except_methods=None): self.token = token self.header = header + self.except_methods = except_methods or [] def __call__(self, func): return get_cython_compat_decorator(self.__wrapper, func) @@ -1131,6 +1133,9 @@ class CSRFRequired(object): return supplied_token and supplied_token == cur_token def __wrapper(self, func, *fargs, **fkwargs): + if request.method in self.except_methods: + return func(*fargs, **fkwargs) + cur_token = get_csrf_token(save_if_missing=False) if self.check_csrf(request, cur_token): if request.POST.get(self.token): diff --git a/rhodecode/model/forms.py b/rhodecode/model/forms.py --- a/rhodecode/model/forms.py +++ b/rhodecode/model/forms.py @@ -102,20 +102,6 @@ def LoginForm(): return _LoginForm -def PasswordChangeForm(username): - class _PasswordChangeForm(formencode.Schema): - allow_extra_fields = True - filter_extra_fields = True - - current_password = v.ValidOldPassword(username)(not_empty=True) - new_password = All(v.ValidPassword(), v.UnicodeString(strip=False, min=6)) - new_password_confirmation = All(v.ValidPassword(), v.UnicodeString(strip=False, min=6)) - - chained_validators = [v.ValidPasswordsMatch('new_password', - 'new_password_confirmation')] - return _PasswordChangeForm - - def UserForm(edit=False, available_languages=[], old_data={}): class _UserForm(formencode.Schema): allow_extra_fields = True diff --git a/rhodecode/model/user.py b/rhodecode/model/user.py --- a/rhodecode/model/user.py +++ b/rhodecode/model/user.py @@ -147,7 +147,6 @@ class UserModel(BaseModel): # cleanups, my_account password change form kwargs.pop('current_password', None) kwargs.pop('new_password', None) - kwargs.pop('new_password_confirmation', None) # cleanups, user edit password change form kwargs.pop('password_confirmation', None) diff --git a/rhodecode/model/validation_schema/schemas/user_schema.py b/rhodecode/model/validation_schema/schemas/user_schema.py new file mode 100644 --- /dev/null +++ b/rhodecode/model/validation_schema/schemas/user_schema.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- + +# Copyright (C) 2016-2016 RhodeCode GmbH +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License, version 3 +# (only), as published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +# This program is dual-licensed. If you wish to learn more about the +# RhodeCode Enterprise Edition, including its added features, Support services, +# and proprietary license terms, please see https://rhodecode.com/licenses/ + +import colander + +from rhodecode import forms +from rhodecode.model.db import User +from rhodecode.translation import _ +from rhodecode.lib.auth import check_password + + +@colander.deferred +def deferred_user_password_validator(node, kw): + username = kw.get('username') + user = User.get_by_username(username) + + def _user_password_validator(node, value): + if not check_password(value, user.password): + msg = _('Password is incorrect') + raise colander.Invalid(node, msg) + return _user_password_validator + + +class ChangePasswordSchema(colander.Schema): + + current_password = colander.SchemaNode( + colander.String(), + missing=colander.required, + widget=forms.widget.PasswordWidget(redisplay=True), + validator=deferred_user_password_validator) + + new_password = colander.SchemaNode( + colander.String(), + missing=colander.required, + widget=forms.widget.CheckedPasswordWidget(redisplay=True), + validator=colander.Length(min=6)) + + + def validator(self, form, values): + if values['current_password'] == values['new_password']: + exc = colander.Invalid(form) + exc['new_password'] = _('New password must be different ' + 'to old password') + raise exc diff --git a/rhodecode/model/validation_schema/validators.py b/rhodecode/model/validation_schema/validators.py --- a/rhodecode/model/validation_schema/validators.py +++ b/rhodecode/model/validation_schema/validators.py @@ -13,7 +13,3 @@ def ip_addr_validator(node, value): except ValueError: msg = _(u'Please enter a valid IPv4 or IpV6 address') raise colander.Invalid(node, msg) - - - - diff --git a/rhodecode/public/css/deform.less b/rhodecode/public/css/deform.less --- a/rhodecode/public/css/deform.less +++ b/rhodecode/public/css/deform.less @@ -12,6 +12,7 @@ .control-label { width: 200px; + padding: 10px; float: left; } .control-inputs { @@ -26,6 +27,13 @@ .form-group { clear: left; + margin-bottom: 20px; + + &:after { /* clear fix */ + content: " "; + display: block; + clear: left; + } } .form-control { @@ -34,6 +42,11 @@ .error-block { color: red; + margin: 0; + } + + .help-block { + margin: 0; } .deform-seq-container .control-inputs { @@ -62,7 +75,9 @@ } } - .form-control.select2-container { height: 40px; } + .form-control.select2-container { + height: 40px; + } .deform-two-field-sequence .deform-seq-container .deform-seq-item label { display: none; @@ -74,7 +89,7 @@ display: none; } .deform-two-field-sequence .deform-seq-container .deform-seq-item.form-group { - background: red; + margin: 0; } .deform-two-field-sequence .deform-seq-container .deform-seq-item .deform-seq-item-group .form-group { width: 45%; padding: 0 2px; float: left; clear: none; diff --git a/rhodecode/templates/admin/my_account/my_account_password.html b/rhodecode/templates/admin/my_account/my_account_password.html --- a/rhodecode/templates/admin/my_account/my_account_password.html +++ b/rhodecode/templates/admin/my_account/my_account_password.html @@ -1,42 +1,5 @@ -
-
-

${_('Change Your Account Password')}

-
- ${h.secure_form(url('my_account_password'), method='post')} -
-
-
-
- -
-
- ${h.password('current_password',class_='medium',autocomplete="off")} -
-
+<%namespace name="widgets" file="/widgets.html"/> -
-
- -
-
- ${h.password('new_password',class_='medium', autocomplete="off")} -
-
- -
-
- -
-
- ${h.password('new_password_confirmation',class_='medium', autocomplete="off")} -
-
- -
- ${h.submit('save',_('Save'),class_="btn")} - ${h.reset('reset',_('Reset'),class_="btn")} -
-
-
- ${h.end_form()} -
\ No newline at end of file +<%widgets:panel title="${_('Change Your Account Password')}"> +${c.form.render() | n} + diff --git a/rhodecode/templates/base/root.html b/rhodecode/templates/base/root.html --- a/rhodecode/templates/base/root.html +++ b/rhodecode/templates/base/root.html @@ -15,7 +15,6 @@ if getattr(c, 'rhodecode_user', None) an c.template_context['visual']['default_renderer'] = h.get_visual_attr(c, 'default_renderer') %> - ${self.title()} diff --git a/rhodecode/templates/widgets.html b/rhodecode/templates/widgets.html new file mode 100644 --- /dev/null +++ b/rhodecode/templates/widgets.html @@ -0,0 +1,10 @@ +<%def name="panel(title, class_='default')"> +
+
+

${title}

+
+
+ ${caller.body()} +
+
+ diff --git a/rhodecode/tests/functional/test_admin_my_account.py b/rhodecode/tests/functional/test_admin_my_account.py --- a/rhodecode/tests/functional/test_admin_my_account.py +++ b/rhodecode/tests/functional/test_admin_my_account.py @@ -21,6 +21,7 @@ import pytest from rhodecode.lib import helpers as h +from rhodecode.lib.auth import check_password from rhodecode.model.db import User, UserFollowing, Repository, UserApiKeys from rhodecode.model.meta import Session from rhodecode.tests import ( @@ -34,6 +35,7 @@ fixture = Fixture() class TestMyAccountController(TestController): test_user_1 = 'testme' + test_user_1_password = '0jd83nHNS/d23n' destroy_users = set() @classmethod @@ -158,7 +160,8 @@ class TestMyAccountController(TestContro ('email', {'email': 'some@email.com'}), ]) def test_my_account_update(self, name, attrs): - usr = fixture.create_user(self.test_user_1, password='qweqwe', + usr = fixture.create_user(self.test_user_1, + password=self.test_user_1_password, email='testme@rhodecode.org', extern_type='rhodecode', extern_name=self.test_user_1, @@ -167,7 +170,8 @@ class TestMyAccountController(TestContro params = usr.get_api_data() # current user data user_id = usr.user_id - self.log_user(username=self.test_user_1, password='qweqwe') + self.log_user( + username=self.test_user_1, password=self.test_user_1_password) params.update({'password_confirmation': ''}) params.update({'new_password': ''}) @@ -321,8 +325,55 @@ class TestMyAccountController(TestContro response = response.follow() response.mustcontain(no=[api_key]) - def test_password_is_updated_in_session_on_password_change( - self, user_util): + def test_valid_change_password(self, user_util): + new_password = 'my_new_valid_password' + user = user_util.create_user(password=self.test_user_1_password) + session = self.log_user(user.username, self.test_user_1_password) + form_data = [ + ('current_password', self.test_user_1_password), + ('__start__', 'new_password:mapping'), + ('new_password', new_password), + ('new_password-confirm', new_password), + ('__end__', 'new_password:mapping'), + ('csrf_token', self.csrf_token), + ] + response = self.app.post(url('my_account_password'), form_data).follow() + assert 'Successfully updated password' in response + + # check_password depends on user being in session + Session().add(user) + try: + assert check_password(new_password, user.password) + finally: + Session().expunge(user) + + @pytest.mark.parametrize('current_pw,new_pw,confirm_pw', [ + ('', 'abcdef123', 'abcdef123'), + ('wrong_pw', 'abcdef123', 'abcdef123'), + (test_user_1_password, test_user_1_password, test_user_1_password), + (test_user_1_password, '', ''), + (test_user_1_password, 'abcdef123', ''), + (test_user_1_password, '', 'abcdef123'), + (test_user_1_password, 'not_the', 'same_pw'), + (test_user_1_password, 'short', 'short'), + ]) + def test_invalid_change_password(self, current_pw, new_pw, confirm_pw, + user_util): + user = user_util.create_user(password=self.test_user_1_password) + session = self.log_user(user.username, self.test_user_1_password) + old_password_hash = session['password'] + form_data = [ + ('current_password', current_pw), + ('__start__', 'new_password:mapping'), + ('new_password', new_pw), + ('new_password-confirm', confirm_pw), + ('__end__', 'new_password:mapping'), + ('csrf_token', self.csrf_token), + ] + response = self.app.post(url('my_account_password'), form_data) + assert 'Error occurred' in response + + def test_password_is_updated_in_session_on_password_change(self, user_util): old_password = 'abcdef123' new_password = 'abcdef124' @@ -330,12 +381,14 @@ class TestMyAccountController(TestContro session = self.log_user(user.username, old_password) old_password_hash = session['password'] - form_data = { - 'current_password': old_password, - 'new_password': new_password, - 'new_password_confirmation': new_password, - 'csrf_token': self.csrf_token - } + form_data = [ + ('current_password', old_password), + ('__start__', 'new_password:mapping'), + ('new_password', new_password), + ('new_password-confirm', new_password), + ('__end__', 'new_password:mapping'), + ('csrf_token', self.csrf_token), + ] self.app.post(url('my_account_password'), form_data) response = self.app.get(url('home')) diff --git a/rhodecode/tests/models/schemas/test_user_schema.py b/rhodecode/tests/models/schemas/test_user_schema.py new file mode 100644 --- /dev/null +++ b/rhodecode/tests/models/schemas/test_user_schema.py @@ -0,0 +1,72 @@ +# -*- coding: utf-8 -*- + +# Copyright (C) 2016-2016 RhodeCode GmbH +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License, version 3 +# (only), as published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +# This program is dual-licensed. If you wish to learn more about the +# RhodeCode Enterprise Edition, including its added features, Support services, +# and proprietary license terms, please see https://rhodecode.com/licenses/ + +import colander +import pytest + +from rhodecode.model import validation_schema +from rhodecode.model.validation_schema.schemas import user_schema + + +class TestChangePasswordSchema(object): + original_password = 'm092d903fnio0m' + + def test_deserialize_bad_data(self, user_regular): + schema = user_schema.ChangePasswordSchema().bind( + username=user_regular.username) + + with pytest.raises(validation_schema.Invalid) as exc_info: + schema.deserialize('err') + err = exc_info.value.asdict() + assert err[''] == '"err" is not a mapping type: ' \ + 'Does not implement dict-like functionality.' + + def test_validate_valid_change_password_data(self, user_util): + user = user_util.create_user(password=self.original_password) + schema = user_schema.ChangePasswordSchema().bind( + username=user.username) + + schema.deserialize({ + 'current_password': self.original_password, + 'new_password': '23jf04rm04imr' + }) + + @pytest.mark.parametrize( + 'current_password,new_password,key,message', [ + ('', 'abcdef123', 'current_password', 'required'), + ('wrong_pw', 'abcdef123', 'current_password', 'incorrect'), + (original_password, original_password, 'new_password', 'different'), + (original_password, '', 'new_password', 'Required'), + (original_password, 'short', 'new_password', 'minimum'), + ]) + def test_validate_invalid_change_password_data(self, current_password, + new_password, key, message, + user_util): + user = user_util.create_user(password=self.original_password) + schema = user_schema.ChangePasswordSchema().bind( + username=user.username) + + with pytest.raises(validation_schema.Invalid) as exc_info: + schema.deserialize({ + 'current_password': current_password, + 'new_password': new_password + }) + err = exc_info.value.asdict() + assert message.lower() in err[key].lower()