diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | conf/general.yml-example | 4 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth.pm | 54 | ||||
| -rw-r--r-- | t/app/controller/auth.t | 17 |
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'); |