Changeset - 8b47181750a8
[Not reviewed]
stable
0 3 0
Mads Kiilerich - 6 years ago 2020-01-09 12:28:33
mads@kiilerich.com
Grafted from: d80802bd5eb6
login: fix incorrect CSRF rejection of "Reset Your Password" form (Issue #350)

htmlfill would remove the CSRF token from the form when substituting the query
parameters, causing password reset to break.

By default, htmlfill will clear all input fields that doesn't have a new
"default" value provided. It could be fixed by setting force_defaults to False
- see http://www.formencode.org/en/1.2-branch/modules/htmlfill.html . It could
also be fixed by providing the CSRF token in the defaults to be substituted in
the form.

Instead, refactor password_reset_confirmation to have more explicitly safe
handling of query parameters. Replace htmlfill with the usual template
variables.

The URLs are generated in kallithea/model/user.py send_reset_password_email()
and should only contain email, timestamp (integer as digit string) and a hex
token from get_reset_password_token() .
3 files changed with 8 insertions and 10 deletions:
0 comments (0 inline, 0 general)
kallithea/controllers/login.py
Show inline comments
 
@@ -117,142 +117,140 @@ class LoginController(BaseController):
 
                               'hg.register.manual_activate')
 
    def register(self):
 
        def_user_perms = AuthUser(dbuser=User.get_default_user()).permissions['global']
 
        c.auto_active = 'hg.register.auto_activate' in def_user_perms
 

	
 
        settings = Setting.get_app_settings()
 
        captcha_private_key = settings.get('captcha_private_key')
 
        c.captcha_active = bool(captcha_private_key)
 
        c.captcha_public_key = settings.get('captcha_public_key')
 

	
 
        if request.POST:
 
            register_form = RegisterForm()()
 
            try:
 
                form_result = register_form.to_python(dict(request.POST))
 
                form_result['active'] = c.auto_active
 

	
 
                if c.captcha_active:
 
                    from kallithea.lib.recaptcha import submit
 
                    response = submit(request.POST.get('g-recaptcha-response'),
 
                                      private_key=captcha_private_key,
 
                                      remoteip=request.ip_addr)
 
                    if not response.is_valid:
 
                        _value = form_result
 
                        _msg = _('Bad captcha')
 
                        error_dict = {'recaptcha_field': _msg}
 
                        raise formencode.Invalid(_msg, _value, None,
 
                                                 error_dict=error_dict)
 

	
 
                UserModel().create_registration(form_result)
 
                h.flash(_('You have successfully registered with %s') % (c.site_name or 'Kallithea'),
 
                        category='success')
 
                Session().commit()
 
                raise HTTPFound(location=url('login_home'))
 

	
 
            except formencode.Invalid as errors:
 
                return htmlfill.render(
 
                    render('/register.html'),
 
                    defaults=errors.value,
 
                    errors=errors.error_dict or {},
 
                    prefix_error=False,
 
                    encoding="UTF-8",
 
                    force_defaults=False)
 
            except UserCreationError as e:
 
                # container auth or other auth functions that create users on
 
                # the fly can throw this exception signaling that there's issue
 
                # with user creation, explanation should be provided in
 
                # Exception itself
 
                h.flash(e, 'error')
 

	
 
        return render('/register.html')
 

	
 
    def password_reset(self):
 
        settings = Setting.get_app_settings()
 
        captcha_private_key = settings.get('captcha_private_key')
 
        c.captcha_active = bool(captcha_private_key)
 
        c.captcha_public_key = settings.get('captcha_public_key')
 

	
 
        if request.POST:
 
            password_reset_form = PasswordResetRequestForm()()
 
            try:
 
                form_result = password_reset_form.to_python(dict(request.POST))
 
                if c.captcha_active:
 
                    from kallithea.lib.recaptcha import submit
 
                    response = submit(request.POST.get('g-recaptcha-response'),
 
                                      private_key=captcha_private_key,
 
                                      remoteip=request.ip_addr)
 
                    if not response.is_valid:
 
                        _value = form_result
 
                        _msg = _('Bad captcha')
 
                        error_dict = {'recaptcha_field': _msg}
 
                        raise formencode.Invalid(_msg, _value, None,
 
                                                 error_dict=error_dict)
 
                redirect_link = UserModel().send_reset_password_email(form_result)
 
                h.flash(_('A password reset confirmation code has been sent'),
 
                            category='success')
 
                raise HTTPFound(location=redirect_link)
 

	
 
            except formencode.Invalid as errors:
 
                return htmlfill.render(
 
                    render('/password_reset.html'),
 
                    defaults=errors.value,
 
                    errors=errors.error_dict or {},
 
                    prefix_error=False,
 
                    encoding="UTF-8",
 
                    force_defaults=False)
 

	
 
        return render('/password_reset.html')
 

	
 
    def password_reset_confirmation(self):
 
        # This controller handles both GET and POST requests, though we
 
        # only ever perform the actual password change on POST (since
 
        # GET requests are not allowed to have side effects, and do not
 
        # receive automatic CSRF protection).
 

	
 
        # The template needs the email address outside of the form.
 
        c.email = request.params.get('email')
 

	
 
        c.timestamp = request.params.get('timestamp') or ''
 
        c.token = request.params.get('token') or ''
 
        if not request.POST:
 
            return htmlfill.render(
 
                render('/password_reset_confirmation.html'),
 
                defaults=dict(request.params),
 
                encoding='UTF-8')
 
            return render('/password_reset_confirmation.html')
 

	
 
        form = PasswordResetConfirmationForm()()
 
        try:
 
            form_result = form.to_python(dict(request.POST))
 
        except formencode.Invalid as errors:
 
            return htmlfill.render(
 
                render('/password_reset_confirmation.html'),
 
                defaults=errors.value,
 
                errors=errors.error_dict or {},
 
                prefix_error=False,
 
                encoding='UTF-8')
 

	
 
        if not UserModel().verify_reset_password_token(
 
            form_result['email'],
 
            form_result['timestamp'],
 
            form_result['token'],
 
        ):
 
            return htmlfill.render(
 
                render('/password_reset_confirmation.html'),
 
                defaults=form_result,
 
                errors={'token': _('Invalid password reset token')},
 
                prefix_error=False,
 
                encoding='UTF-8')
 

	
 
        UserModel().reset_password(form_result['email'], form_result['password'])
 
        h.flash(_('Successfully updated password'), category='success')
 
        raise HTTPFound(location=url('login_home'))
 

	
 
    def logout(self):
 
        session.delete()
 
        log.info('Logging out and deleting session for user')
 
        raise HTTPFound(location=url('home'))
 

	
 
    def session_csrf_secret_token(self):
 
        """Return the CSRF protection token for the session - just like it
 
        could have been screen scraped from a page with a form.
 
        Only intended for testing but might also be useful for other kinds
 
        of automation.
 
        """
 
        return h.session_csrf_secret_token()
kallithea/templates/password_reset_confirmation.html
Show inline comments
 
## -*- coding: utf-8 -*-
 
<%inherit file="base/root.html"/>
 

	
 
<%block name="title">
 
    ${_('Reset Your Password')}
 
</%block>
 

	
 
<%include file="/base/flash_msg.html"/>
 

	
 
<div class="container">
 
<div class="row">
 
<div class="centered-column">
 
<div id="register" class="panel panel-primary">
 
    <div class="panel-heading">
 
        %if c.site_name:
 
            <h5>${_('Reset Your Password to %s') % c.site_name}</h5>
 
        %else:
 
            <h5>${_('Reset Your Password')}</h5>
 
        %endif
 
    </div>
 
    <div class="panel-body">
 
        ${h.form(h.url('reset_password_confirmation'), method='post')}
 
        <p>${_('You are about to set a new password for the email address %s.') % c.email}</p>
 
        <p>${_('Note that you must use the same browser session for this as the one used to request the password reset.')}</p>
 
        ${h.hidden('email')}
 
        ${h.hidden('timestamp')}
 
        ${h.hidden('email', value=c.email)}
 
        ${h.hidden('timestamp', value=c.timestamp)}
 
        <div class="form">
 
                <div class="form-group">
 
                    <label class="control-label" for="token">${_('Code you received in the email')}:</label>
 
                    <div>
 
                        ${h.text('token', class_='form-control')}
 
                        ${h.text('token', value=c.token, class_='form-control')}
 
                    </div>
 
                </div>
 

	
 
                <div class="form-group">
 
                    <label class="control-label" for="password">${_('New Password')}:</label>
 
                    <div>
 
                        ${h.password('password',class_='form-control')}
 
                    </div>
 
                </div>
 

	
 
                <div class="form-group">
 
                    <label class="control-label" for="password_confirm">${_('Confirm New Password')}:</label>
 
                    <div>
 
                        ${h.password('password_confirm',class_='form-control')}
 
                    </div>
 
                </div>
 

	
 
                <div class="form-group">
 
                    <div class="buttons">
 
                        ${h.submit('send',_('Confirm'),class_="btn btn-default")}
 
                    </div>
 
                </div>
 
        </div>
 
        ${h.end_form()}
 
    </div>
 
   </div>
 
</div>
 
</div>
 
</div>
kallithea/tests/functional/test_login.py
Show inline comments
 
@@ -366,170 +366,170 @@ class TestLoginController(TestController
 

	
 
        ret = Session().query(User).filter(User.username == 'test_regular4').one()
 
        assert ret.username == username
 
        assert check_password(password, ret.password) == True
 
        assert ret.email == email
 
        assert ret.name == name
 
        assert ret.lastname == lastname
 
        assert ret.api_key is not None
 
        assert ret.admin == False
 

	
 
    #==========================================================================
 
    # PASSWORD RESET
 
    #==========================================================================
 

	
 
    def test_forgot_password_wrong_mail(self):
 
        bad_email = 'username%wrongmail.org'
 
        response = self.app.post(
 
                        url(controller='login', action='password_reset'),
 
                            {'email': bad_email,
 
                             '_session_csrf_secret_token': self.session_csrf_secret_token()})
 

	
 
        response.mustcontain('An email address must contain a single @')
 

	
 
    def test_forgot_password(self):
 
        response = self.app.get(url(controller='login',
 
                                    action='password_reset'))
 
        assert response.status == '200 OK'
 

	
 
        username = 'test_password_reset_1'
 
        password = 'qweqwe'
 
        email = 'username@example.com'
 
        name = u'passwd'
 
        lastname = u'reset'
 
        timestamp = int(time.time())
 

	
 
        new = User()
 
        new.username = username
 
        new.password = password
 
        new.email = email
 
        new.name = name
 
        new.lastname = lastname
 
        new.api_key = generate_api_key()
 
        Session().add(new)
 
        Session().commit()
 

	
 
        token = UserModel().get_reset_password_token(
 
            User.get_by_username(username), timestamp, self.session_csrf_secret_token())
 

	
 
        collected = []
 
        def mock_send_email(recipients, subject, body='', html_body='', headers=None, author=None):
 
            collected.append((recipients, subject, body, html_body))
 

	
 
        with mock.patch.object(kallithea.lib.celerylib.tasks, 'send_email', mock_send_email):
 
            response = self.app.post(url(controller='login',
 
                                         action='password_reset'),
 
                                     {'email': email,
 
                                      '_session_csrf_secret_token': self.session_csrf_secret_token()})
 

	
 
        self.checkSessionFlash(response, 'A password reset confirmation code has been sent')
 

	
 
        ((recipients, subject, body, html_body),) = collected
 
        assert recipients == ['username@example.com']
 
        assert subject == 'Password reset link'
 
        assert '\n%s\n' % token in body
 
        (confirmation_url,) = (line for line in body.splitlines() if line.startswith('http://'))
 
        assert ' href="%s"' % confirmation_url.replace('&', '&amp;').replace('@', '%40') in html_body
 

	
 
        d = urlparse.parse_qs(urlparse.urlparse(confirmation_url).query)
 
        assert d['token'] == [token]
 
        assert d['timestamp'] == [str(timestamp)]
 
        assert d['email'] == [email]
 

	
 
        response = response.follow()
 

	
 
        # BAD TOKEN
 

	
 
        bad_token = "bad"
 

	
 
        response = self.app.post(url(controller='login',
 
                                     action='password_reset_confirmation'),
 
                                 {'email': email,
 
                                  'timestamp': timestamp,
 
                                  'password': "p@ssw0rd",
 
                                  'password_confirm': "p@ssw0rd",
 
                                  'token': bad_token,
 
                                  '_session_csrf_secret_token': self.session_csrf_secret_token(),
 
                                 })
 
        assert response.status == '200 OK'
 
        response.mustcontain('Invalid password reset token')
 

	
 
        # GOOD TOKEN
 

	
 
        response = self.app.get(confirmation_url)
 
        assert response.status == '200 OK'
 
        response.mustcontain("You are about to set a new password for the email address %s" % email)
 
        response.mustcontain('<form action="%s" method="post">' % url(controller='login', action='password_reset_confirmation'))
 
        response.mustcontain(no='value="%s"' % self.session_csrf_secret_token())  # BUG making actual password_reset_confirmation POST fail
 
        response.mustcontain('value="%s"' % self.session_csrf_secret_token())
 
        response.mustcontain('value="%s"' % token)
 
        response.mustcontain('value="%s"' % timestamp)
 
        response.mustcontain('value="username@example.com"')
 

	
 
        # fake a submit of that form (*with* csrf token)
 
        # fake a submit of that form
 
        response = self.app.post(url(controller='login',
 
                                     action='password_reset_confirmation'),
 
                                 {'email': email,
 
                                  'timestamp': timestamp,
 
                                  'password': "p@ssw0rd",
 
                                  'password_confirm': "p@ssw0rd",
 
                                  'token': token,
 
                                  '_session_csrf_secret_token': self.session_csrf_secret_token(),
 
                                 })
 
        assert response.status == '302 Found'
 
        self.checkSessionFlash(response, 'Successfully updated password')
 

	
 
        response = response.follow()
 

	
 
    #==========================================================================
 
    # API
 
    #==========================================================================
 

	
 
    def _api_key_test(self, api_key, status):
 
        """Verifies HTTP status code for accessing an auth-requiring page,
 
        using the given api_key URL parameter as well as using the API key
 
        with bearer authentication.
 

	
 
        If api_key is None, no api_key is passed at all. If api_key is True,
 
        a real, working API key is used.
 
        """
 
        with fixture.anon_access(False):
 
            if api_key is None:
 
                params = {}
 
                headers = {}
 
            else:
 
                if api_key is True:
 
                    api_key = User.get_first_admin().api_key
 
                params = {'api_key': api_key}
 
                headers = {'Authorization': 'Bearer ' + str(api_key)}
 

	
 
            self.app.get(url(controller='changeset', action='changeset_raw',
 
                             repo_name=HG_REPO, revision='tip', **params),
 
                         status=status)
 

	
 
            self.app.get(url(controller='changeset', action='changeset_raw',
 
                             repo_name=HG_REPO, revision='tip'),
 
                         headers=headers,
 
                         status=status)
 

	
 
    @parametrize('test_name,api_key,code', [
 
        ('none', None, 302),
 
        ('empty_string', '', 403),
 
        ('fake_number', '123456', 403),
 
        ('fake_not_alnum', 'a-z', 403),
 
        ('fake_api_key', '0123456789abcdef0123456789ABCDEF01234567', 403),
 
        ('proper_api_key', True, 200)
 
    ])
 
    def test_access_page_via_api_key(self, test_name, api_key, code):
 
        self._api_key_test(api_key, code)
 

	
 
    def test_access_page_via_extra_api_key(self):
 
        new_api_key = ApiKeyModel().create(TEST_USER_ADMIN_LOGIN, u'test')
 
        Session().commit()
 
        self._api_key_test(new_api_key.api_key, status=200)
 

	
 
    def test_access_page_via_expired_api_key(self):
 
        new_api_key = ApiKeyModel().create(TEST_USER_ADMIN_LOGIN, u'test')
 
        Session().commit()
 
        # patch the API key and make it expired
 
        new_api_key.expires = 0
 
        Session().commit()
 
        self._api_key_test(new_api_key.api_key, status=403)
0 comments (0 inline, 0 general)