account: convert change password form to colander schema and fix bug

where user could use same password as before when changing
password, fixes #2264
This commit is contained in:
Daniel Dourvaris 2016-08-05 06:55:35 +03:00
parent c9537b89c1
commit b51406ec1d
14 changed files with 306 additions and 107 deletions

View file

@ -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']})

View file

@ -32,6 +32,7 @@ from pylons.controllers.util import redirect
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):

View file

@ -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 <http://www.gnu.org/licenses/>.
#
# 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')

View file

@ -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):

View file

@ -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

View file

@ -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)

View file

@ -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 <http://www.gnu.org/licenses/>.
#
# 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

View file

@ -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)

View file

@ -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;

View file

@ -1,42 +1,5 @@
<div class="panel panel-default">
<div class="panel-heading">
<h3 class="panel-title">${_('Change Your Account Password')}</h3>
</div>
${h.secure_form(url('my_account_password'), method='post')}
<div class="panel-body">
<div class="fields">
<div class="field">
<div class="label">
<label for="current_password">${_('Current Password')}:</label>
</div>
<div class="input">
${h.password('current_password',class_='medium',autocomplete="off")}
</div>
</div>
<%namespace name="widgets" file="/widgets.html"/>
<div class="field">
<div class="label">
<label for="new_password">${_('New Password')}:</label>
</div>
<div class="input">
${h.password('new_password',class_='medium', autocomplete="off")}
</div>
</div>
<div class="field">
<div class="label">
<label for="password_confirmation">${_('Confirm New Password')}:</label>
</div>
<div class="input">
${h.password('new_password_confirmation',class_='medium', autocomplete="off")}
</div>
</div>
<div class="buttons">
${h.submit('save',_('Save'),class_="btn")}
${h.reset('reset',_('Reset'),class_="btn")}
</div>
</div>
</div>
${h.end_form()}
</div>
<%widgets:panel title="${_('Change Your Account Password')}">
${c.form.render() | n}
</%widgets:panel>

View file

@ -15,7 +15,6 @@ if getattr(c, 'rhodecode_user', None) and c.rhodecode_user.user_id:
c.template_context['visual']['default_renderer'] = h.get_visual_attr(c, 'default_renderer')
%>
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>${self.title()}</title>

View file

@ -0,0 +1,10 @@
<%def name="panel(title, class_='default')">
<div class="panel panel-${class_}">
<div class="panel-heading">
<h3 class="panel-title">${title}</h3>
</div>
<div class="panel-body">
${caller.body()}
</div>
</div>
</%def>

View file

@ -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(TestController):
('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(TestController):
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(TestController):
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(TestController):
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'))

View file

@ -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 <http://www.gnu.org/licenses/>.
#
# 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()