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

where user could use same password as before when changing
password, fixes  #2264

rhodecode/forms/__init__.py

@@ -0,0 +1,33 b''
+# -*- 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')

rhodecode/model/validation_schema/schemas/user_schema.py

@@ -0,0 +1,61 b''
+# -*- 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
+
+
[e-mail]
+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

rhodecode/templates/widgets.html

@@ -0,0 +1,10 b''
+<%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>

rhodecode/tests/models/schemas/test_user_schema.py

@@ -0,0 +1,72 b''
+# -*- 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()

rhodecode/config/routing.py

@@ -531,9 +531,7 b' 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']})

rhodecode/controllers/admin/my_account.py

@@ -32,6 +32,7 b' 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 b' 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 b' class MyAccountController(BaseController'
             force_defaults=False
         )
 
-    @auth.CSRFRequired()
-    def my_account_password_update(self):
+    @auth.CSRFRequired(except_methods=['GET'])
+    def my_account_password(self):
         c.active = 'password'
         self.__load_data()
-        _form = PasswordChangeForm(c.rhodecode_user.username)()
+
+        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:
-            form_result = _form.to_python(request.POST)
-            UserModel().update_user(c.rhodecode_user.user_id, **form_result)
+                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()
-            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')
+                request.session.flash(
+                    _("Successfully updated password"), queue='success')
+                return redirect(url('my_account_password'))
 
-    def my_account_password(self):
-        c.active = 'password'
-        self.__load_data()
+        c.form = form
         return render('admin/my_account/my_account.html')
 
     def my_account_repos(self):

rhodecode/lib/auth.py

@@ -1116,9 +1116,11 b' 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 b' 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):

rhodecode/model/forms.py

@@ -102,20 +102,6 b' 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

rhodecode/model/user.py

@@ -147,7 +147,6 b' 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)

rhodecode/model/validation_schema/validators.py

@@ -13,7 +13,3 b' def ip_addr_validator(node, value):'
     except ValueError:
         msg = _(u'Please enter a valid IPv4 or IpV6 address')
         raise colander.Invalid(node, msg)
-
-
-
-

rhodecode/public/css/deform.less

@@ -12,6 +12,7 b''
 
   .control-label {
     width: 200px;
+    padding: 10px;
     float: left;
   }
   .control-inputs {
@@ -26,6 +27,13 b''
 
   .form-group {
     clear: left;
+    margin-bottom: 20px;
+
+    &:after { /* clear fix */
+      content: " ";
+      display: block;
+      clear: left;
+    }
   }
 
   .form-control {
@@ -34,6 +42,11 b''
 
   .error-block {
     color: red;
+    margin: 0;
+  }
+
+  .help-block {
+    margin: 0;
   }
 
   .deform-seq-container .control-inputs {
@@ -62,7 +75,9 b''
       }
   }
 
-  .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 b''
       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;

rhodecode/templates/admin/my_account/my_account_password.html

@@ -1,42 +1,5 b''
-<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>
  No newline at end of file
+<%widgets:panel title="${_('Change Your Account Password')}">
+${c.form.render() | n}
+</%widgets:panel>

rhodecode/templates/base/root.html

@@ -15,7 +15,6 b" if getattr(c, 'rhodecode_user', None) an"
 
 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>

rhodecode/tests/functional/test_admin_my_account.py

@@ -21,6 +21,7 b''
 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 b' fixture = Fixture()'
 
 class TestMyAccountController(TestController):
     test_user_1 = 'testme'
+    test_user_1_password = '0jd83nHNS/d23n'
     destroy_users = set()
 
     @classmethod
@@ -158,7 +160,8 b' class TestMyAccountController(TestContro'
             ('email', {'email': '[e-mail]'}),
         ])
     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='[e-mail]',
                                   extern_type='rhodecode',
                                   extern_name=self.test_user_1,
@@ -167,7 +170,8 b' 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 b' 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 b' 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'))