aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthew Somerville <matthew@mysociety.org>2020-06-25 16:23:33 +0100
committerMatthew Somerville <matthew@mysociety.org>2020-07-07 15:03:49 +0100
commit9a12c0dac0b7677938f33f5abb639a296adff9c5 (patch)
treea909a3cc05e1ff7cac311a075ad7453257e58879
parentd597f5012f2db5408eb0a913a789bbe2de4d923a (diff)
Add option to check password on Have I Been Pwned.
If switched on, sends first five letters of the SHA1 hash of the entered password to HIBP's API, which then returns all matching hashes in their database of breached passwords. If we find a match, tell the user they need to pick a different password.
-rw-r--r--CHANGELOG.md1
-rw-r--r--conf/general.yml-example4
-rw-r--r--perllib/FixMyStreet/App/Controller/Auth.pm54
-rw-r--r--t/app/controller/auth.t17
4 files changed, 62 insertions, 14 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3d05be748..d851c5b0d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@
- Front end improvements:
- Add lazy image loading on list items.
- Improve Bing geocoder results.
+ - Add option of checking passwords against Have I Been Pwned.
- Changes:
- Mark user as active when sent an email alert.
- Bugfixes:
diff --git a/conf/general.yml-example b/conf/general.yml-example
index 26399bb81..e42a1433e 100644
--- a/conf/general.yml-example
+++ b/conf/general.yml-example
@@ -223,6 +223,10 @@ SMTP_PORT: ''
SMTP_USERNAME: ''
SMTP_PASSWORD: ''
+# Set if you want password setting to be checked (securely) against the
+# Have I Been Pwned dataset
+CHECK_HAVEIBEENPWNED: 0
+
# Gaze is a world-wide service for population density lookups. You can leave
# this as is.
GAZE_URL: 'https://gaze.mysociety.org/gaze'
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index beba6b235..0d0c2240a 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -460,7 +460,7 @@ sub no_csrf_token : Private {
=item common_password
-Returns 1/0 depending on if password is common or not.
+Returns 1/0 depending on if password is common/breached or not.
=cut
@@ -469,10 +469,8 @@ sub common_password : Local : Args(0) {
my $password = $c->get_param('password_register');
- my $return = JSON->true;
- if (!$c->cobrand->call_hook('bypass_password_checks') && found($password)) {
- $return = _('Please choose a less commonly-used password');
- }
+ my $pass = $c->forward('test_password', [ $password ]);
+ my $return = $pass ? JSON->true : $c->stash->{field_errors}->{password_register};
my $body = JSON->new->utf8->allow_nonref->encode($return);
$c->res->content_type('application/json; charset=utf-8');
@@ -491,22 +489,50 @@ sub test_password : Private {
return 1 if $c->cobrand->call_hook('bypass_password_checks');
- my @errors;
-
+ my $error;
my $min_length = $c->cobrand->password_minimum_length;
- push @errors, sprintf(_('Please make sure your password is at least %d characters long'), $min_length)
- if length($password) < $min_length;
-
- push @errors, _('Please choose a less commonly-used password')
- if found($password);
+ if (length($password) < $min_length) {
+ $error = sprintf(_('Please make sure your password is at least %d characters long'), $min_length);
+ } elsif (found($password)) {
+ $error = _('Please choose a less commonly-used password');
+ } elsif (hibp($password)) {
+ $error = _('That password has appeared in a known third-party data breach (<a href="https://haveibeenpwned.com/Passwords" target="_blank">more information</a>); please choose another');
+ }
- if (@errors) {
- $c->stash->{field_errors}->{password_register} = join('<br>', @errors);
+ if ($error) {
+ $c->stash->{field_errors}->{password_register} = $error;
return 0;
}
return 1;
}
+=item hibp
+
+Returns true if we should check Have I Been Pwned and the check
+comes back positive for a password that has been breached.
+
+=cut
+
+use Encode qw(encode);
+use Digest::SHA qw(sha1_hex);
+use LWP::Simple;
+use Unicode::Normalize;
+
+sub hibp : Private {
+ my $password = shift;
+
+ return 0 unless FixMyStreet->config('CHECK_HAVEIBEENPWNED');
+ my $sha1 = uc sha1_hex(encode('UTF-8', NFD($password)));
+ my $url = 'https://api.pwnedpasswords.com/range/' . substr($sha1, 0, 5);
+ my $response = LWP::Simple::get($url);
+ my $remainder = substr($sha1, 5);
+ foreach my $line (split /\r\n/, $response) {
+ my ($part, $count) = split /:/, $line;
+ return $count if $part eq $remainder;
+ }
+ return 0;
+}
+
=head2 sign_out
Log the user out. Tell them we've done so.
diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t
index 8b4b772fc..0326bbacd 100644
--- a/t/app/controller/auth.t
+++ b/t/app/controller/auth.t
@@ -288,6 +288,23 @@ subtest 'check common password AJAX call' => sub {
$mech->content_contains("true");
};
+subtest 'check hibp password call' => sub {
+ FixMyStreet::override_config {
+ CHECK_HAVEIBEENPWNED => 1,
+ }, sub {
+ my $lwp = Test::MockModule->new('LWP::Simple');
+ # Switch mock round from live site, so we know we're not testing live site by mistake
+ $lwp->mock(get => sub($) {
+ return '9958D0F0EE6744E7CCAFC84515FCFAD7B1B:10' if $_[0] =~ /6EF4D$/; # squirblewirble
+ return '';
+ });
+ $mech->post_ok('/auth/common_password', { password_register => 'p@ssword2' });
+ $mech->content_contains("true");
+ $mech->post_ok('/auth/common_password', { password_register => 'squirblewirble' });
+ $mech->content_contains("That password has appeared in a known");
+ };
+};
+
subtest 'test forgotten password page' => sub {
$mech->get_ok('/auth/forgot');
$mech->content_contains('Forgot password');