From 7dea44e626816230b8bf12ef9730592eb305da3a Mon Sep 17 00:00:00 2001 From: Henrik Levkowetz Date: Thu, 9 Feb 2017 17:03:44 +0000 Subject: [PATCH] Added a change password page, and linked to it from the account profile page and user menu. Added zxcvbn-based browser-side password strength estimation on the various password setting, re-setting, and changing forms. Added a change password test. Changed ietfauth/urls.py to not use the deprecated string form for views in urlpatterns. - Legacy-Id: 12798 --- ietf/ietfauth/forms.py | 32 ++++- ietf/ietfauth/tests.py | 52 ++++++- ietf/ietfauth/urls.py | 27 ++-- ietf/ietfauth/views.py | 72 ++++++++-- ietf/ipr/views.py | 1 - ietf/static/ietf/js/password_strength.js | 130 ++++++++++++++++++ ietf/templates/base/menu_user.html | 7 +- .../registration/change_password.html | 56 ++++++-- .../registration/confirm_account.html | 7 + .../registration/password_change_email.txt | 10 ++ 10 files changed, 348 insertions(+), 46 deletions(-) create mode 100644 ietf/static/ietf/js/password_strength.js create mode 100644 ietf/templates/registration/password_change_email.txt diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index 08b0ee9d4..f30f8c103 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -1,4 +1,5 @@ import re +from unidecode import unidecode from django import forms from django.conf import settings @@ -8,7 +9,7 @@ from django.contrib.auth.models import User from django.utils.html import mark_safe from django.core.urlresolvers import reverse as urlreverse -from unidecode import unidecode +from django_password_strength.widgets import PasswordStrengthInput, PasswordConfirmationInput import debug # pyflakes:ignore @@ -31,8 +32,8 @@ class RegistrationForm(forms.Form): class PasswordForm(forms.Form): - password = forms.CharField(widget=forms.PasswordInput) - password_confirmation = forms.CharField(widget=forms.PasswordInput, + password = forms.CharField(widget=PasswordStrengthInput) + password_confirmation = forms.CharField(widget=PasswordConfirmationInput, help_text="Enter the same password as above, for verification.") def clean_password_confirmation(self): @@ -166,3 +167,28 @@ class WhitelistForm(forms.ModelForm): exclude = ['by', 'time' ] +from django import forms + + +class ChangePasswordForm(forms.Form): + current_password = forms.CharField(widget=forms.PasswordInput) + + + new_password = forms.CharField(widget=PasswordStrengthInput) + new_password_confirmation = forms.CharField(widget=PasswordConfirmationInput) + + def __init__(self, user, data=None): + self.user = user + super(ChangePasswordForm, self).__init__(data) + + def clean_current_password(self): + password = self.cleaned_data.get('current_password', None) + if not self.user.check_password(password): + raise ValidationError('Invalid password') + + def clean(self): + new_password = self.cleaned_data.get('new_password', None) + conf_password = self.cleaned_data.get('new_password_confirmation', None) + if not new_password == conf_password: + raise ValidationError("The password confirmation is different than the new password") + diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index eb7a143b0..b8dfc6440 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -6,11 +6,13 @@ from urlparse import urlsplit from pyquery import PyQuery from unittest import skipIf -from django.core.urlresolvers import reverse as urlreverse import django.contrib.auth.views +from django.core.urlresolvers import reverse as urlreverse from django.contrib.auth.models import User from django.conf import settings +import debug # pyflakes:ignore + from ietf.utils.test_utils import TestCase, login_testing_unauthorized, unicontent from ietf.utils.test_data import make_test_data, make_review_data from ietf.utils.mail import outbox, empty_outbox @@ -399,3 +401,51 @@ class IetfAuthTests(TestCase): update_htpasswd_file("foo", "passwd") self.assertTrue(self.username_in_htpasswd_file("foo")) + + def test_change_password(self): + + chpw_url = urlreverse(ietf.ietfauth.views.change_password) + prof_url = urlreverse(ietf.ietfauth.views.profile) + login_url = urlreverse(django.contrib.auth.views.login) + redir_url = '%s?next=%s' % (login_url, chpw_url) + + # get without logging in + r = self.client.get(chpw_url) + self.assertRedirects(r, redir_url) + + user = User.objects.create(username="someone@example.com", email="someone@example.com") + user.set_password("password") + user.save() + p = Person.objects.create(name="Some One", ascii="Some One", user=user) + Email.objects.create(address=user.username, person=p) + + # log in + r = self.client.post(redir_url, {"username":user.username, "password":"password"}) + self.assertRedirects(r, chpw_url) + + # wrong current password + r = self.client.post(chpw_url, {"current_password": "fiddlesticks", + "new_password": "foobar", + "new_password_confirmation": "foobar", + }) + self.assertEqual(r.status_code, 200) + self.assertFormError(r, 'form', 'current_password', 'Invalid password') + + # mismatching new passwords + r = self.client.post(chpw_url, {"current_password": "password", + "new_password": "foobar", + "new_password_confirmation": "barfoo", + }) + self.assertEqual(r.status_code, 200) + self.assertFormError(r, 'form', None, "The password confirmation is different than the new password") + + # correct password change + r = self.client.post(chpw_url, {"current_password": "password", + "new_password": "foobar", + "new_password_confirmation": "foobar", + }) + self.assertRedirects(r, prof_url) + # refresh user object + user = User.objects.get(username="someone@example.com") + self.assertTrue(user.check_password(u'foobar')) + diff --git a/ietf/ietfauth/urls.py b/ietf/ietfauth/urls.py index 1bc247554..ee278a4d4 100644 --- a/ietf/ietfauth/urls.py +++ b/ietf/ietfauth/urls.py @@ -3,23 +3,20 @@ from django.conf.urls import url from django.contrib.auth.views import login, logout -from ietf.ietfauth.views import add_account_whitelist +from ietf.ietfauth import views urlpatterns = [ - url(r'^$', 'ietf.ietfauth.views.index'), -# url(r'^login/$', 'ietf.ietfauth.views.ietf_login'), + url(r'^$', views.index), + url(r'^confirmnewemail/(?P[^/]+)/$', views.confirm_new_email), + url(r'^create/$', views.create_account), + url(r'^create/confirm/(?P[^/]+)/$', views.confirm_account), url(r'^login/$', login), url(r'^logout/$', logout), -# url(r'^loggedin/$', 'ietf.ietfauth.views.ietf_loggedin'), -# url(r'^loggedout/$', 'ietf.ietfauth.views.logged_out'), - url(r'^profile/$', 'ietf.ietfauth.views.profile'), -# (r'^login/(?P[a-z0-9.@]+)/(?P.+)$', 'ietf.ietfauth.views.url_login'), - url(r'^testemail/$', 'ietf.ietfauth.views.test_email'), - url(r'^create/$', 'ietf.ietfauth.views.create_account'), - url(r'^create/confirm/(?P[^/]+)/$', 'ietf.ietfauth.views.confirm_account'), - url(r'^reset/$', 'ietf.ietfauth.views.password_reset'), - url(r'^reset/confirm/(?P[^/]+)/$', 'ietf.ietfauth.views.confirm_password_reset'), - url(r'^confirmnewemail/(?P[^/]+)/$', 'ietf.ietfauth.views.confirm_new_email'), - url(r'whitelist/add/?$', add_account_whitelist), - url(r'^review/$', 'ietf.ietfauth.views.review_overview'), + url(r'^password/$', views.change_password), + url(r'^profile/$', views.profile), + url(r'^reset/$', views.password_reset), + url(r'^reset/confirm/(?P[^/]+)/$', views.confirm_password_reset), + url(r'^review/$', views.review_overview), + url(r'^testemail/$', views.test_email), + url(r'whitelist/add/?$', views.add_account_whitelist), ] diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index 8b3c18ff1..60e0c6cb9 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -32,24 +32,27 @@ # Copyright The IETF Trust 2007, All Rights Reserved +import importlib + from datetime import datetime as DateTime, timedelta as TimeDelta, date as Date from collections import defaultdict -from django.conf import settings -from django.http import Http404 #, HttpResponse, HttpResponseRedirect -from django.shortcuts import render, redirect, get_object_or_404 -#from django.contrib.auth import REDIRECT_FIELD_NAME, authenticate, login -from django.contrib.auth.decorators import login_required -#from django.utils.http import urlquote import django.core.signing -from django.contrib.sites.models import Site -from django.contrib.auth.models import User from django import forms +from django.contrib import messages +from django.conf import settings +from django.contrib.auth import update_session_auth_hash +from django.contrib.auth.decorators import login_required +from django.contrib.auth.models import User +from django.contrib.sites.models import Site +from django.core.urlresolvers import reverse as urlreverse +from django.http import Http404, HttpResponseRedirect #, HttpResponse, +from django.shortcuts import render, redirect, get_object_or_404 import debug # pyflakes:ignore from ietf.group.models import Role, Group -from ietf.ietfauth.forms import RegistrationForm, PasswordForm, ResetPasswordForm, TestEmailForm, WhitelistForm +from ietf.ietfauth.forms import RegistrationForm, PasswordForm, ResetPasswordForm, TestEmailForm, WhitelistForm, ChangePasswordForm from ietf.ietfauth.forms import get_person_form, RoleEmailForm, NewEmailForm from ietf.ietfauth.htpasswd import update_htpasswd_file from ietf.ietfauth.utils import role_required @@ -340,10 +343,14 @@ def confirm_password_reset(request, auth): else: form = PasswordForm() + hlibname, hashername = settings.PASSWORD_HASHERS[0].rsplit('.',1) + hlib = importlib.import_module(hlibname) + hasher = getattr(hlib, hashername) return render(request, 'registration/change_password.html', { 'form': form, - 'username': username, + 'user': user, 'success': success, + 'hasher': hasher, }) def test_email(request): @@ -465,3 +472,48 @@ def review_overview(request): 'review_wishes': review_wishes, 'review_wish_form': review_wish_form, }) + +@login_required +def change_password(request): + success = False + person = None + + try: + person = request.user.person + except Person.DoesNotExist: + return render(request, 'registration/missing_person.html') + + emails = [ e.address for e in Email.objects.filter(person=person, active=True).order_by('-primary','-time') ] + user = request.user + + if request.method == 'POST': + form = ChangePasswordForm(user, request.POST) + if form.is_valid(): + new_password = form.cleaned_data["new_password"] + + user.set_password(new_password) + user.save() + # password is also stored in htpasswd file + update_htpasswd_file(user.username, new_password) + # keep the session + update_session_auth_hash(request, user) + + send_mail(request, emails, None, "Datatracker password change notification", "registration/password_change_email.txt", {}) + + messages.success(request, "Your password was successfully changed") + return HttpResponseRedirect(urlreverse('ietf.ietfauth.views.profile')) + + else: + form = ChangePasswordForm(request.user) + + hlibname, hashername = settings.PASSWORD_HASHERS[0].rsplit('.',1) + hlib = importlib.import_module(hlibname) + hasher = getattr(hlib, hashername) + return render(request, 'registration/change_password.html', { + 'form': form, + 'user': user, + 'success': success, + 'hasher': hasher, + }) + + diff --git a/ietf/ipr/views.py b/ietf/ipr/views.py index cf59220de..c32a49d05 100644 --- a/ietf/ipr/views.py +++ b/ietf/ipr/views.py @@ -704,7 +704,6 @@ def get_details_tabs(ipr, selected): ('History', urlreverse('ipr_history', kwargs={ 'id': ipr.pk })) ]] -@debug.trace def show(request, id): """View of individual declaration""" ipr = get_object_or_404(IprDisclosureBase, id=id).get_child() diff --git a/ietf/static/ietf/js/password_strength.js b/ietf/static/ietf/js/password_strength.js new file mode 100644 index 000000000..a1fd393f5 --- /dev/null +++ b/ietf/static/ietf/js/password_strength.js @@ -0,0 +1,130 @@ +// Taken from django-password-strength, with changes to use the bower-managed zxcvbn.js The +// bower-managed zxcvbn.js is kept up-to-date to a larger extent than the copy packaged with +// the django-password-strength component. +(function($, window, document, undefined){ + window.djangoPasswordStrength = { + config: { + passwordClass: 'password_strength', + confirmationClass: 'password_confirmation' + }, + + init: function (config) { + var self = this; + // Setup configuration + if ($.isPlainObject(config)) { + $.extend(self.config, config); + } + + self.initListeners(); + }, + + initListeners: function() { + var self = this; + var body = $('body'); + + $('.' + self.config.passwordClass).on('keyup', function() { + var password_strength_bar = $(this).parent().find('.password_strength_bar'); + var password_strength_info = $(this).parent().find('.password_strength_info'); + + if( $(this).val() ) { + var result = zxcvbn( $(this).val() ); + + if( result.score < 3 ) { + password_strength_bar.removeClass('progress-bar-success').addClass('progress-bar-warning'); + password_strength_info.find('.label').removeClass('hidden'); + } else { + password_strength_bar.removeClass('progress-bar-warning').addClass('progress-bar-success'); + password_strength_info.find('.label').addClass('hidden'); + } + + password_strength_bar.width( ((result.score+1)/5)*100 + '%' ).attr('aria-valuenow', result.score + 1); + // henrik@levkowetz.com -- this is the only changed line: + password_strength_info.find('.password_strength_time').html(result.crack_times_display.online_no_throttling_10_per_second); + password_strength_info.removeClass('hidden'); + } else { + password_strength_bar.removeClass('progress-bar-success').addClass('progress-bar-warning'); + password_strength_bar.width( '0%' ).attr('aria-valuenow', 0); + password_strength_info.addClass('hidden'); + } + self.match_passwords($(this)); + }); + + var timer = null; + $('.' + self.config.confirmationClass).on('keyup', function() { + var password_field; + var confirm_with = $(this).data('confirm-with'); + + if( confirm_with ) { + password_field = $('#' + confirm_with); + } else { + password_field = $('.' + self.config.passwordClass); + } + + if (timer !== null) clearTimeout(timer); + + timer = setTimeout(function(){ + self.match_passwords(password_field); + }, 400); + }); + }, + + display_time: function(seconds) { + var minute = 60; + var hour = minute * 60; + var day = hour * 24; + var month = day * 31; + var year = month * 12; + var century = year * 100; + + // Provide fake gettext for when it is not available + if( typeof gettext !== 'function' ) { gettext = function(text) { return text; }; }; + + if( seconds < minute ) return gettext('only an instant'); + if( seconds < hour) return (1 + Math.ceil(seconds / minute)) + ' ' + gettext('minutes'); + if( seconds < day) return (1 + Math.ceil(seconds / hour)) + ' ' + gettext('hours'); + if( seconds < month) return (1 + Math.ceil(seconds / day)) + ' ' + gettext('days'); + if( seconds < year) return (1 + Math.ceil(seconds / month)) + ' ' + gettext('months'); + if( seconds < century) return (1 + Math.ceil(seconds / year)) + ' ' + gettext('years'); + + return gettext('centuries'); + }, + + match_passwords: function(password_field, confirmation_fields) { + var self = this; + // Optional parameter: if no specific confirmation field is given, check all + if( confirmation_fields === undefined ) { confirmation_fields = $('.' + self.config.confirmationClass) } + if( confirmation_fields === undefined ) { return; } + + var password = password_field.val(); + + confirmation_fields.each(function(index, confirm_field) { + var confirm_value = $(confirm_field).val(); + var confirm_with = $(confirm_field).data('confirm-with'); + + if( confirm_with && confirm_with == password_field.attr('id')) { + if( confirm_value && password ) { + if (confirm_value === password) { + $(confirm_field).parent().find('.password_strength_info').addClass('hidden'); + } else { + $(confirm_field).parent().find('.password_strength_info').removeClass('hidden'); + } + } else { + $(confirm_field).parent().find('.password_strength_info').addClass('hidden'); + } + } + }); + + // If a password field other than our own has been used, add the listener here + if( !password_field.hasClass(self.config.passwordClass) && !password_field.data('password-listener') ) { + password_field.on('keyup', function() { + self.match_passwords($(this)); + }); + password_field.data('password-listener', true); + } + } + }; + + // Call the init for backwards compatibility + djangoPasswordStrength.init(); + +})(jQuery, window, document); diff --git a/ietf/templates/base/menu_user.html b/ietf/templates/base/menu_user.html index 79f1a72c5..bf1232dfd 100644 --- a/ietf/templates/base/menu_user.html +++ b/ietf/templates/base/menu_user.html @@ -16,14 +16,17 @@ {% else %} {% if user.is_authenticated %}
  • Sign out
    • -
  • Edit profile
    • +
  • Account info
    • +
  • Change password
    • {% else %}
  • Sign in
  • Password reset
    • {% endif %} {% endif %} -
  • {% if request.user.is_authenticated %}Manage account{% else %}New account{% endif %}
    • + {% if not request.user.is_authenticated %} +
  • New account
    • + {% endif %}
  • Preferences
    • {% if user|has_role:"Reviewer" %} diff --git a/ietf/templates/registration/change_password.html b/ietf/templates/registration/change_password.html index 26321558a..c246425eb 100644 --- a/ietf/templates/registration/change_password.html +++ b/ietf/templates/registration/change_password.html @@ -3,30 +3,58 @@ {% load origin %} {% load bootstrap3 %} +{% load staticfiles %} {% block title %}Change password{% endblock %} +{% block js %} + {{ block.super }} + + +{% endblock %} + {% block content %} {% origin %} {% if success %} -

    Password change successful

    - -

    Your password has been updated.

    - Sign in - +

    Your password was successfully changed.

    + {% if not user.is_authenticated %} + Sign in + {% endif %} {% else %} -

    Change password

    +
    +
    +
    +

    Change password

    -

    You can change the password below for your user {{ username }} below.

    -
    - {% csrf_token %} - {% bootstrap_form form %} + + {% csrf_token %} + {% bootstrap_form form %} - {% buttons %} - - {% endbuttons %} -
    + {% buttons %} + + {% endbuttons %} + + +
    + This password form uses the + zxcvbn + password strength estimator to give an indication of password strength. + The crack times given assume online attack without rate limiting, + at a rate of 10 attempts per second. +
    + +
    + The datatracker currently uses a {{ hasher.algorithm }}-based + password hasher with + {% if hasher.iterations %}{{ hasher.iterations }} iterations{% else %}{{ hasher.rounds }} rounds{% endif %}. + Calculating offline attack time if password hashes would leak is left + as an excercise for the reader. +
    + +
    +
    +
    {% endif %} {% endblock %} diff --git a/ietf/templates/registration/confirm_account.html b/ietf/templates/registration/confirm_account.html index a43daa251..8cb03fed5 100644 --- a/ietf/templates/registration/confirm_account.html +++ b/ietf/templates/registration/confirm_account.html @@ -1,11 +1,18 @@ {% extends "base.html" %} {# Copyright The IETF Trust 2015, All Rights Reserved #} {% load origin %} +{% load staticfiles %} {% load bootstrap3 %} {% block title %}Complete account creation{% endblock %} +{% block js %} + {{ block.super }} + + +{% endblock %} + {% block content %} {% origin %} diff --git a/ietf/templates/registration/password_change_email.txt b/ietf/templates/registration/password_change_email.txt new file mode 100644 index 000000000..aca385b77 --- /dev/null +++ b/ietf/templates/registration/password_change_email.txt @@ -0,0 +1,10 @@ +{% autoescape off %} +Hello, + +{% filter wordwrap:73 %}The password for your datatracker account was just changed using the password change form. If this was not done by you, please contact the secretariat at ietf-action@ietf.org for assistance.{% endfilter %} + +Best regards, + + The datatracker account manager service + (for the IETF Secretariat) +{% endautoescape %}