diff options
28 files changed, 299 insertions, 242 deletions
diff --git a/.circleci/checksetup_answers.legacy.txt b/.circleci/checksetup_answers.legacy.txt index 6bcdd2dcc..11d8c9131 100644 --- a/.circleci/checksetup_answers.legacy.txt +++ b/.circleci/checksetup_answers.legacy.txt @@ -1,6 +1,6 @@ $answer{'ADMIN_EMAIL'} = 'admin@mozilla.bugs'; $answer{'ADMIN_OK'} = 'Y'; -$answer{'ADMIN_PASSWORD'} = 'password'; +$answer{'ADMIN_PASSWORD'} = 'Te6Oovohch'; $answer{'ADMIN_REALNAME'} = 'QA Admin'; $answer{'NO_PAUSE'} = 1; $answer{'bugzilla_version'} = '1'; diff --git a/.circleci/checksetup_answers.txt b/.circleci/checksetup_answers.txt index bcdefa38e..8178854ac 100644 --- a/.circleci/checksetup_answers.txt +++ b/.circleci/checksetup_answers.txt @@ -1,6 +1,6 @@ $answer{'ADMIN_EMAIL'} = 'admin@mozilla.bugs'; $answer{'ADMIN_OK'} = 'Y'; -$answer{'ADMIN_PASSWORD'} = 'passWord1234!'; +$answer{'ADMIN_PASSWORD'} = 'Te6Oovohch'; $answer{'ADMIN_REALNAME'} = 'QA Admin'; $answer{'NO_PAUSE'} = 1; $answer{'bugzilla_version'} = '1'; diff --git a/.circleci/config.yml b/.circleci/config.yml index a0b13539f..18577fadb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -75,14 +75,16 @@ jobs: parallelism: 4 working_directory: /app docker: - - *bmo_slim_image + - <<: *bmo_slim_image + environment: *bmo_env steps: - checkout - *default_qa_setup - run: name: run sanity tests command: | - prove -qf $(circleci tests glob 't/*.t' | circleci tests split) | tee artifacts/$CIRCLE_JOB.txt + rm /app/localconfig + /app/scripts/entrypoint.pl prove -qf $(circleci tests glob 't/*.t' | circleci tests split) | tee artifacts/$CIRCLE_JOB.txt - store_artifacts: path: /app/artifacts @@ -156,14 +158,13 @@ workflows: version: 2 tests: jobs: - - test_bmo - test_sanity + - test_bmo - test_webservices - - test_selenium: - requires: - - test_bmo + - test_selenium - build: requires: - test_sanity - test_webservices - test_selenium + - test_bmo diff --git a/.circleci/selenium_test.conf b/.circleci/selenium_test.conf index a012ae957..511fc1cd5 100644 --- a/.circleci/selenium_test.conf +++ b/.circleci/selenium_test.conf @@ -22,28 +22,28 @@ 'test_bug_1' => 1, 'test_bug_2' => 2, 'admin_user_login' => 'admin@mozilla.test', - 'admin_user_passwd' => 'password', + 'admin_user_passwd' => 'bo6aazeKohch', 'admin_user_username' => 'QA Admin', 'admin_user_nick' => 'admin', 'permanent_user' => 'permanent_user@mozilla.test', 'permanent_user_login' => 'permanent_user@mozilla.test', - 'permanent_user_passwd' => 'password', + 'permanent_user_passwd' => 'bo6aazeKohch', 'unprivileged_user_login' => 'no-privs@mozilla.test', - 'unprivileged_user_passwd' => 'password', + 'unprivileged_user_passwd' => 'bo6aazeKohch', 'unprivileged_user_username' => 'no-privs', 'unprivileged_user_nick' => 'no-privs', 'unprivileged_user_login_truncated' => 'no-privs@mo', 'QA_Selenium_TEST_user_login' => 'QA-Selenium-TEST@mozilla.test', - 'QA_Selenium_TEST_user_passwd' => 'password', + 'QA_Selenium_TEST_user_passwd' => 'bo6aazeKohch', 'editbugs_user_login' => 'editbugs@mozilla.test', - 'editbugs_user_passwd' => 'password', + 'editbugs_user_passwd' => 'bo6aazeKohch', 'canconfirm_user_login' => 'canconfirm@mozilla.test', - 'canconfirm_user_passwd' => 'password', + 'canconfirm_user_passwd' => 'bo6aazeKohch', 'tweakparams_user_login' => 'tweakparams@mozilla.test', 'tweakparams_user_login_truncated' => 'tweakparams@mo', - 'tweakparams_user_passwd' => 'password', + 'tweakparams_user_passwd' => 'bo6aazeKohch', 'disabled_user_login' => 'disabled@mozilla.test', - 'disabled_user_passwd' => 'password', + 'disabled_user_passwd' => 'bo6aazeKohch', 'common_email' => '@mozilla.test', 'test_extensions' => 1, }; diff --git a/Bugzilla.pm b/Bugzilla.pm index 65508cb6f..0ffd63e04 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -322,6 +322,41 @@ sub github_secret { return $cache->{github_secret}; } +sub passwdqc { + my ($class) = @_; + require Data::Password::passwdqc; + + my $cache = $class->request_cache; + my $params = $class->params; + + return $cache->{passwdqc} if $cache->{passwdqc}; + + my @min = map { $_ eq 'undef' ? undef : $_ } + split( /\s*,\s*/, $params->{passwdqc_min} ); + + return $cache->{passwdqc} = Data::Password::passwdqc->new( + min => \@min, + max => $params->{passwdqc_max}, + passphrase_words => $params->{passwdqc_passphrase_words}, + match_length => $params->{passwdqc_match_length}, + random_bits => $params->{passwdqc_random_bits}, + ); +} + +sub assert_password_is_secure { + my ( $class, $password1 ) = @_; + + my $pwqc = $class->passwdqc; + ThrowUserError( 'password_insecure', { reason => $pwqc->reason } ) + unless $pwqc->validate_password($password1); +} + +sub assert_passwords_match { + my ( $class, $password1, $password2 ) = @_; + + ThrowUserError('password_mismatch') if $password1 ne $password2; +} + sub login { my ($class, $type) = @_; diff --git a/Bugzilla/Auth/Verify.pm b/Bugzilla/Auth/Verify.pm index 19d8dcc9e..5895534cd 100644 --- a/Bugzilla/Auth/Verify.pm +++ b/Bugzilla/Auth/Verify.pm @@ -72,7 +72,7 @@ sub create_or_update_user { || return { failure => AUTH_ERROR, error => 'auth_invalid_email', details => {addr => $username} }; - # Usually we'd call validate_password, but external authentication + # external authentication # systems might follow different standards than ours. So in this # place here, we call trick_taint without checks. trick_taint($password); diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm index ac6b71ac0..e46d1cd82 100644 --- a/Bugzilla/Auth/Verify/DB.pm +++ b/Bugzilla/Auth/Verify/DB.pm @@ -62,13 +62,15 @@ sub check_credentials { if (Bugzilla->usage_mode == USAGE_MODE_BROWSER && Bugzilla->params->{password_check_on_login}) { - my $check = validate_password_check($password); - if ($check) { - return { - failure => AUTH_ERROR, - user_error => $check, - details => { locked_user => $user } - } + my $pwqc = Bugzilla->passwdqc; + unless ($pwqc->validate_password($password)) { + my $reason = $pwqc->reason; + Bugzilla->audit(sprintf "%s logged in with a weak password (reason: %s)", $user->login, $reason); + $user->set_password_change_required(1); + $user->set_password_change_reason( + "You must change your password for the following reason: $reason" + ); + $user->update(); } } diff --git a/Bugzilla/Config/Auth.pm b/Bugzilla/Config/Auth.pm index dddedd819..58a3d3cd7 100644 --- a/Bugzilla/Config/Auth.pm +++ b/Bugzilla/Config/Auth.pm @@ -12,6 +12,8 @@ use strict; use warnings; use Bugzilla::Config::Common; +use Types::Standard qw(Tuple Maybe); +use Types::Common::Numeric qw(PositiveInt); our $sortkey = 300; @@ -119,6 +121,42 @@ sub get_param_list { type => 'b', default => '1' }, + + { + name => 'passwdqc_min', + type => 't', + default => 'undef, 24, 11, 8, 7', + checker => \&_check_passwdqc_min, + }, + + { + name => 'passwdqc_max', + type => 't', + default => '40', + checker => \&_check_passwdqc_max, + }, + + { + name => 'passwdqc_passphrase_words', + type => 't', + default => '3', + checker => \&check_numeric, + }, + + { + name => 'passwdqc_match_length', + type => 't', + default => '4', + checker => \&check_numeric, + }, + + { + name => 'passwdqc_random_bits', + type => 't', + default => '47', + checker => \&_check_passwdqc_random_bits, + }, + { name => 'auth_delegation', type => 'b', @@ -149,4 +187,51 @@ sub get_param_list { return @param_list; } +my $passwdqc_min = Tuple[ + Maybe[PositiveInt], + Maybe[PositiveInt], + Maybe[PositiveInt], + Maybe[PositiveInt], + Maybe[PositiveInt], +]; + +sub _check_passwdqc_min { + my ($value) = @_; + my @values = map { $_ eq 'undef' ? undef : $_ } split( /\s*,\s*/, $value ); + + unless ( $passwdqc_min->check( \@values ) ) { + return "must be list of five values, that are either integers > 0 or undef"; + } + + my ( $max, $max_pos ); + my $pos = 0; + foreach my $value (@values) { + if ( defined $max && defined $value ) { + if ( $value > $max ) { + return "Int$pos is larger than Int$max_pos ($max)"; + } + } + elsif ( defined $value ) { + $max = $value; + $max_pos = $pos; + } + $pos++; + } + return ""; +} + +sub _check_passwdqc_max { + my ($value) = @_; + return "must be a positive integer" unless PositiveInt->check($value); + return "must be greater than 8" unless $value > 8; + return ""; +} + +sub _check_passwdqc_random_bits { + my ($value) = @_; + return "must be a positive integer" unless PositiveInt->check($value); + return "must be between 24 and 85 inclusive" unless $value >= 24 && $value <= 85; + return ""; +} + 1; diff --git a/Bugzilla/Install.pm b/Bugzilla/Install.pm index 3f8d4bdfb..ced559111 100644 --- a/Bugzilla/Install.pm +++ b/Bugzilla/Install.pm @@ -500,9 +500,14 @@ sub _prompt_for_password { print "\n", get_text('install_confirm_password'), ' '; my $pass2 = <STDIN>; chomp $pass2; - eval { validate_password($password, $pass2); }; - if ($@) { - print "\n$@\n"; + my $pwqc = Bugzilla->passwdqc; + my $ok = $pwqc->validate_password($password); + if (!$ok) { + print "\n", $pwqc->reason, "\n"; + undef $password; + } + elsif ($password ne $pass2) { + print "\n", "passwords do not match\n"; undef $password; } system("stty","echo") unless ON_WINDOWS; diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 84fc1fb21..2d8256080 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -34,7 +34,7 @@ use Role::Tiny::With; use base qw(Bugzilla::Object Exporter); @Bugzilla::User::EXPORT = qw(is_available_username - login_to_id user_id_to_login validate_password validate_password_check + login_to_id user_id_to_login USER_MATCH_MULTIPLE USER_MATCH_FAILED USER_MATCH_SUCCESS MATCH_SKIP_CONFIRM ); @@ -417,7 +417,7 @@ sub _check_password { # authentication. return $pass if $pass eq '*'; - validate_password($pass); + Bugzilla->assert_password_is_secure($pass); my $cryptpassword = bz_crypt($pass); return $cryptpassword; } @@ -2712,40 +2712,6 @@ sub user_id_to_login { return $login || ''; } -sub validate_password { - my $check = validate_password_check(@_); - ThrowUserError($check) if $check; - return 1; -} - -sub validate_password_check { - my ($password, $matchpassword) = @_; - - if (length($password) < USER_PASSWORD_MIN_LENGTH) { - return 'password_too_short'; - } elsif ((defined $matchpassword) && ($password ne $matchpassword)) { - return 'passwords_dont_match'; - } - - my $complexity_level = Bugzilla->params->{password_complexity}; - if ($complexity_level eq 'bmo') { - my $features = 0; - - $features++ if $password =~ /[a-z]/; - $features++ if $password =~ /[A-Z]/; - $features++ if $password =~ /[0-9]/; - $features++ if $password =~ /[^A-Za-z0-9]/; - $features++ if length($password) > 12; - - return 'password_not_complex' if $features < 3; - } - - # Having done these checks makes us consider the password untainted. - trick_taint($_[0]); - return; -} - - 1; __END__ @@ -3369,26 +3335,6 @@ Returns the login name of the user account for the given user ID. If no valid user ID is given or the user has no entry in the profiles table, we return an empty string. -=item C<validate_password($passwd1, $passwd2)> - -Returns true if a password is valid (i.e. meets Bugzilla's -requirements for length and content), else throws an error. -Untaints C<$passwd1> if successful. - -If a second password is passed in, this function also verifies that -the two passwords match. - -=item C<validate_password_check($passwd1, $passwd2)> - -This sub routine is similair to C<validate_password>, except that it allows -the calling code to handle its own errors. - -Returns undef and untaints C<$passwd1> if a password is valid (i.e. meets -Bugzilla's requirements for length and content), else returns the error. - -If a second password is passed in, this function also verifies that -the two passwords match. - =item C<match_field($data, $fields, $behavior)> =over diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 3c4b2fb65..df2f78823 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -905,6 +905,7 @@ sub extract_nicks { return grep { defined $_ } @nicks; } + 1; __END__ diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index 3f5042f42..93fddfc2b 100644 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -143,8 +143,7 @@ use constant WS_ERROR_CODE => { auth_invalid_email => 302, extern_id_conflict => -303, auth_failure => 304, - password_too_short => 305, - password_not_complex => 305, + password_insecure => 305, api_key_not_valid => 306, api_key_revoked => 306, auth_invalid_token => 307, @@ -159,7 +158,7 @@ use constant WS_ERROR_CODE => { auth_cant_create_account => 501, account_creation_disabled => 501, account_creation_restricted => 501, - password_too_short => 502, + # Error 502 password_too_short no longer exists. # Error 503 password_too_long no longer exists. invalid_username => 504, # This is from strict_isolation, but it also basically means diff --git a/docs/en/rst/administering/parameters.rst b/docs/en/rst/administering/parameters.rst index 1452a9fb9..75974d388 100644 --- a/docs/en/rst/administering/parameters.rst +++ b/docs/en/rst/administering/parameters.rst @@ -179,14 +179,6 @@ emailsuffix createemailregexp This defines the (case-insensitive) regexp to use for email addresses that are permitted to self-register. The default (:paramval:`.*`) permits any account matching the emailregexp to be created. If this parameter is left blank, no users will be permitted to create their own accounts and all accounts will have to be created by an administrator. -password_complexity - Set the complexity required for passwords. In all cases must the passwords be at least 6 characters long. - - * :paramval:`no_constraints` - No complexity required. - * :paramval:`mixed_letters` - Passwords must contain at least one UPPER and one lower case letter. - * :paramval:`letters_numbers` - Passwords must contain at least one UPPER and one lower case letter and a number. - * :paramval:`letters_numbers_specialchars` - Passwords must contain at least one letter, a number and a special character. - password_check_on_login If set, Bugzilla will check that the password meets the current complexity rules and minimum length requirements when the user logs into the Bugzilla web interface. If it doesn't, the user would not be able to log in, and will receive a message to reset their password. diff --git a/qa/config/generate_test_data.pl b/qa/config/generate_test_data.pl index 62daef772..333bffa26 100644 --- a/qa/config/generate_test_data.pl +++ b/qa/config/generate_test_data.pl @@ -182,7 +182,8 @@ foreach my $username (@usernames) { } Bugzilla::User->create( - { login_name => $login, + { + login_name => $login, realname => $realname, cryptpassword => $password, %extra_args, @@ -498,7 +499,7 @@ foreach my $product (@products) { Bugzilla::User->create({ login_name => $watch_user, - cryptpassword => generate_random_password(), + cryptpassword => Bugzilla->passwdqc->generate_password(), disable_mail => 1, }); diff --git a/qa/config/selenium_test.conf b/qa/config/selenium_test.conf index 2a163d5f0..7fbfeffe3 100644 --- a/qa/config/selenium_test.conf +++ b/qa/config/selenium_test.conf @@ -22,28 +22,28 @@ 'test_bug_1' => 1, 'test_bug_2' => 2, 'admin_user_login' => 'admin@mozilla.test', - 'admin_user_passwd' => 'password', + 'admin_user_passwd' => 'bo6aazeKohch', 'admin_user_username' => 'QA Admin', 'admin_user_nick' => 'admin', 'permanent_user' => 'permanent_user@mozilla.test', 'permanent_user_login' => 'permanent_user@mozilla.test', - 'permanent_user_passwd' => 'password', + 'permanent_user_passwd' => 'bo6aazeKohch', 'unprivileged_user_login' => 'no-privs@mozilla.test', - 'unprivileged_user_passwd' => 'password', + 'unprivileged_user_passwd' => 'bo6aazeKohch', 'unprivileged_user_username' => 'no-privs', 'unprivileged_user_nick' => 'no-privs', 'unprivileged_user_login_truncated' => 'no-privs@mo', 'QA_Selenium_TEST_user_login' => 'QA-Selenium-TEST@mozilla.test', - 'QA_Selenium_TEST_user_passwd' => 'password', + 'QA_Selenium_TEST_user_passwd' => 'bo6aazeKohch', 'editbugs_user_login' => 'editbugs@mozilla.test', - 'editbugs_user_passwd' => 'password', + 'editbugs_user_passwd' => 'bo6aazeKohch', 'canconfirm_user_login' => 'canconfirm@mozilla.test', - 'canconfirm_user_passwd' => 'password', + 'canconfirm_user_passwd' => 'bo6aazeKohch', 'tweakparams_user_login' => 'tweakparams@mozilla.test', 'tweakparams_user_login_truncated' => 'tweakparams@mo', - 'tweakparams_user_passwd' => 'password', + 'tweakparams_user_passwd' => 'bo6aazeKohch', 'disabled_user_login' => 'disabled@mozilla.test', - 'disabled_user_passwd' => 'password', + 'disabled_user_passwd' => 'bo6aazeKohch', 'common_email' => '@mozilla.test', 'test_extensions' => 1, }; diff --git a/qa/t/test_bmo_enter_new_bug.t b/qa/t/test_bmo_enter_new_bug.t index 702d067a1..6e5753c74 100644 --- a/qa/t/test_bmo_enter_new_bug.t +++ b/qa/t/test_bmo_enter_new_bug.t @@ -413,7 +413,7 @@ sub _check_user { $sel->wait_for_page_to_load(WAIT_TIME); $sel->title_is('Add user'); $sel->type_ok('login', $user); - $sel->type_ok('password', 'password'); + $sel->type_ok('password', 'icohF1io2ohw'); $sel->click_ok("add"); $sel->wait_for_page_to_load(WAIT_TIME); $sel->is_text_present('regexp:The user account .* has been created successfully'); diff --git a/qa/t/test_password_complexity.t b/qa/t/test_password_complexity.t deleted file mode 100644 index 97b440ddd..000000000 --- a/qa/t/test_password_complexity.t +++ /dev/null @@ -1,97 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -# -# This Source Code Form is "Incompatible With Secondary Licenses", as -# defined by the Mozilla Public License, v. 2.0. - -use 5.10.1; -use strict; -use warnings; -use lib qw(lib ../../lib ../../local/lib/perl5); - -use Test::More "no_plan"; -use QA::Util; - -my ($sel, $config) = get_selenium(); -log_in($sel, $config, 'admin'); - -set_parameters($sel, {"Administrative Policies" => {"allowuserdeletion-on" => undef}, - "User Authentication" => {"createemailregexp" => {type => "text", value => '.*'}, - "emailsuffix" => {type => "text", value => ''}} }); - -# Set the password complexity to BMO. -# Password must contain at least one UPPER and one lowercase letter. -my @invalid_bmo = qw(lowercase UPPERCASE 1234567890 123lowercase 123UPPERCASE !@%&^lower !@&^UPPER); - -check_passwords($sel, 'bmo', \@invalid_bmo, ['Longerthan12chars', '%9rT#j22S']); - -# Set the password complexity to No Constraints. -check_passwords($sel, 'no_constraints', ['12xY!', 'aaaaa'], ['aaaaaaaa', '>F12Xy?#']); - -logout($sel); - -sub check_passwords { - my ($sel, $param, $invalid_passwords, $valid_passwords) = @_; - - set_parameters($sel, { "User Authentication" => {"password_complexity" => {type => "select", value => $param}} }); - my $new_user = 'selenium-' . random_string(10) . '@bugzilla.org'; - - go_to_admin($sel); - $sel->click_ok("link=Users"); - $sel->wait_for_page_to_load_ok(WAIT_TIME); - $sel->title_is('Search users'); - $sel->click_ok('link=add a new user'); - $sel->wait_for_page_to_load_ok(WAIT_TIME); - $sel->title_is('Add user'); - $sel->type_ok('login', $new_user); - - foreach my $password (@$invalid_passwords) { - $sel->type_ok('password', $password, 'Enter password'); - $sel->click_ok('add'); - $sel->wait_for_page_to_load_ok(WAIT_TIME); - if ($param eq 'no_constraints') { - $sel->title_is('Password Too Short'); - } - else { - $sel->title_is('Password Fails Requirements'); - } - - my $error_msg = trim($sel->get_text("error_msg")); - if ($param eq 'bmo') { - ok($error_msg =~ /must meet three of the following requirements/, - "Password fails requirement: $password"); - } - else { - ok($error_msg =~ /The password must be at least \d+ characters long/, - "Password Too Short: $password"); - } - $sel->go_back_ok(); - $sel->wait_for_page_to_load_ok(WAIT_TIME); - } - - my $created = 0; - - foreach my $password (@$valid_passwords) { - $sel->type_ok('password', $password, 'Enter password'); - $sel->click_ok($created ? 'update' : 'add'); - $sel->wait_for_page_to_load_ok(WAIT_TIME); - $sel->title_is($created ? "User $new_user updated" : "Edit user $new_user"); - my $msg = trim($sel->get_text('message')); - if ($created++) { - ok($msg =~ /A new password has been set/, 'Account updated'); - } - else { - ok($msg =~ /The user account $new_user has been created successfully/, 'Account created'); - } - } - - return unless $created; - - $sel->click_ok('delete'); - $sel->wait_for_page_to_load_ok(WAIT_TIME); - $sel->title_is("Confirm deletion of user $new_user"); - $sel->click_ok('delete'); - $sel->wait_for_page_to_load_ok(WAIT_TIME); - $sel->title_is("User $new_user deleted"); -} diff --git a/qa/t/test_user_groups.t b/qa/t/test_user_groups.t index 89fc2fd6d..0798a1b80 100644 --- a/qa/t/test_user_groups.t +++ b/qa/t/test_user_groups.t @@ -12,6 +12,7 @@ use lib qw(lib ../../lib ../../local/lib/perl5); use Test::More "no_plan"; use QA::Util; +use constant PASSWORD => 'uChoopoh1che'; my ($sel, $config) = get_selenium(); @@ -67,7 +68,7 @@ $sel->wait_for_page_to_load_ok(WAIT_TIME); $sel->title_is('Add user'); $sel->type_ok('login', 'master@selenium.bugzilla.org'); $sel->type_ok('name', 'master-user'); -$sel->type_ok('password', 'selenium', 'Enter password'); +$sel->type_ok('password', PASSWORD, 'Enter password'); $sel->type_ok('disabledtext', 'Not for common usage'); $sel->click_ok('add'); $sel->wait_for_page_to_load_ok(WAIT_TIME); @@ -83,7 +84,7 @@ $sel->wait_for_page_to_load_ok(WAIT_TIME); $sel->title_is('Add user'); $sel->type_ok('login', 'slave@selenium.bugzilla.org'); $sel->type_ok('name', 'slave-user'); -$sel->type_ok('password', 'selenium', 'Enter password'); +$sel->type_ok('password', PASSWORD, 'Enter password'); $sel->type_ok('disabledtext', 'Not for common usage'); $sel->click_ok('add'); $sel->wait_for_page_to_load_ok(WAIT_TIME); @@ -99,7 +100,7 @@ $sel->wait_for_page_to_load_ok(WAIT_TIME); $sel->title_is('Add user'); $sel->type_ok('login', 'reg@selenium.bugzilla.org'); $sel->type_ok('name', 'reg-user'); -$sel->type_ok('password', 'selenium', 'Enter password'); +$sel->type_ok('password', PASSWORD, 'Enter password'); $sel->type_ok('disabledtext', 'Not for common usage'); $sel->click_ok('add'); $sel->wait_for_page_to_load_ok(WAIT_TIME); diff --git a/qa/t/webservice_user_create.t b/qa/t/webservice_user_create.t index f82e71ae4..34b7a4896 100644 --- a/qa/t/webservice_user_create.t +++ b/qa/t/webservice_user_create.t @@ -16,7 +16,7 @@ use QA::Util; use Test::More tests => 75; my ($config, $xmlrpc, $jsonrpc, $jsonrpc_get) = get_rpc_clients(); -use constant NEW_PASSWORD => 'password'; +use constant NEW_PASSWORD => 'UiX1Shuuchid'; use constant NEW_FULLNAME => 'WebService Created User'; use constant PASSWORD_TOO_SHORT => 'a'; @@ -91,7 +91,7 @@ foreach my $rpc ($jsonrpc, $xmlrpc) { { user => 'admin', args => { email => new_login(), full_name => NEW_FULLNAME, password => PASSWORD_TOO_SHORT }, - error => 'password must be at least', + error => 'The password does not meet our security requirements for the following reason: too short', test => 'Password Too Short fails', }, { user => 'admin', diff --git a/reset_password.cgi b/reset_password.cgi index 3b0e36849..86ace9e12 100755 --- a/reset_password.cgi +++ b/reset_password.cgi @@ -17,7 +17,6 @@ use Bugzilla; use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Token; -use Bugzilla::User qw( validate_password ); use Bugzilla::Util qw( bz_crypt ); my $cgi = Bugzilla->cgi; @@ -51,7 +50,9 @@ if ($cgi->param('do_save')) { if ($old_password eq $password_1) { ThrowUserError('new_password_same'); } - validate_password($password_1, $password_2); + + Bugzilla->assert_password_is_secure($password_1); + Bugzilla->assert_passwords_match($password_1, $password_2); # update $dbh->bz_start_transaction; diff --git a/t/903-passwdqc-conf.t b/t/903-passwdqc-conf.t new file mode 100644 index 000000000..fe7ce9b53 --- /dev/null +++ b/t/903-passwdqc-conf.t @@ -0,0 +1,23 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. +use 5.10.1; +use strict; +use warnings; +use autodie; + +use Test::More 1.302; +use ok 'Bugzilla::Config::Auth'; + +ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("undef, 24, 11, 8, 7")) == 0, "default value is valid"); +ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("underf, 24, 11, 8, 7")) != 0, "underf is not valid"); +is(Bugzilla::Config::Auth::_check_passwdqc_min("undef, 24, 25, 8, 7"), "Int2 is larger than Int1 (24)", "25 can't come after 24"); +ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("")) != 0, "empty string is invalid"); +ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("24")) != 0, "24 is invalid"); +ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("-24")) != 0, "-24 is invalid"); +ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("10, 10, 10, 10, 0")) != 0, "10, 10, 10, 10, 0 is invalid"); + +done_testing; diff --git a/t/bmo/passwords.t b/t/bmo/passwords.t index d10eddff7..249cdfb3c 100644 --- a/t/bmo/passwords.t +++ b/t/bmo/passwords.t @@ -12,7 +12,7 @@ BEGIN { plan skip_all => "these tests only run in CI" unless $ENV{CI} && $ENV{CI use ok DRIVER; my $ADMIN_LOGIN = $ENV{BZ_TEST_ADMIN} // 'admin@mozilla.bugs'; -my $ADMIN_PW_OLD = $ENV{BZ_TEST_ADMIN_PASS} // 'passWord1234!'; +my $ADMIN_PW_OLD = $ENV{BZ_TEST_ADMIN_PASS} // 'Te6Oovohch'; my $ADMIN_PW_NEW = $ENV{BZ_TEST_ADMIN_NEWPASS} // 'she7Ka8t'; my @require_env = qw( @@ -33,7 +33,7 @@ eval { login_ok($sel, $ADMIN_LOGIN, $ADMIN_PW_OLD); - change_password($sel, $ADMIN_PW_OLD, 'newpassword', 'newpassword2'); + change_password($sel, $ADMIN_PW_OLD, 'Ju9shiePhie6', 'zeeKuj0leib7'); $sel->title_is("Passwords Don't Match"); $sel->body_text_contains('The two passwords you entered did not match.'); diff --git a/template/en/default/admin/params/auth.html.tmpl b/template/en/default/admin/params/auth.html.tmpl index 95db59022..99c52f759 100644 --- a/template/en/default/admin/params/auth.html.tmpl +++ b/template/en/default/admin/params/auth.html.tmpl @@ -24,6 +24,70 @@ desc = "Set up your authentication policies" %] +[% desc_passwdqc_min = BLOCK %] + [Int0, Int1, Int2, Int3, Int4] + <p> + The minimum allowed password lengths for different kinds of passwords + and passphrases. "undef" can be used to disallow passwords of a given + kind regardless of their length. Each subsequent number is required to + be no larger than the preceding one. + + <p> + Int0 is used for passwords consisting of characters from one character + class only. The character classes are: digits, lower-case letters, + upper-case letters, and other characters. There is also a special + class for non-ASCII characters, which could not be classified, but are + assumed to be non-digits. + <p> + Int1 is used for passwords consisting of characters from two character + classes that do not meet the requirements for a passphrase. + <p> + Int2 is used for passphrases. Note that besides meeting this length + requirement, a passphrase must also consist of a sufficient number of + words (see the "passphrase_words" option below). + <p> + Int3 and Int4 are used for passwords consisting of characters from + three and four character classes, respectively. + + <p> + When calculating the number of character classes, upper-case letters + used as the first character and digits used as the last character of a + password are not counted. + + <p> + In addition to being sufficiently long, passwords are required to + contain enough different characters for the character classes and the + minimum length they have been checked against. +[% END %] + +[% desc_passwdqc_max = BLOCK %] + The maximum allowed password length. This can be used to prevent users + from setting passwords that may be too long for some system services. + It must be larger than 8. +[% END %] + +[% desc_passwdqc_passphrase_words = BLOCK %] + The number of words required for a passphrase, or 0 to disable the + support for user-chosen passphrases. +[% END %] + +[% desc_passwdqc_match_length = BLOCK %] + The length of common substring required to conclude that a password is + at least partially based on information found in a character string, + or 0 to disable the substring search. Note that the password will not + be rejected once a weak substring is found; it will instead be + subjected to the usual strength requirements with the weak substring + partially discounted. + <p> + The substring search is case-insensitive and is able to detect and + remove a common substring spelled backwards. +[% END %] + +[% desc_random_bits = BLOCK %] + The size of randomly-generated passphrases in bits (24 to 85). +[% END %] + + [% param_descs = { auth_env_id => "Environment variable used by external authentication system " _ "to store a unique identifier for each user. Leave it blank " _ @@ -133,6 +197,12 @@ "will be permitted to create their own accounts and all accounts " _ "will have to be created by an administrator.", + passwdqc_min => desc_passwdqc_min, + passwdqc_max => desc_passwdqc_max + passwdqc_passphrase_words => desc_passwdqc_passphrase_words, + passwdqc_match_length => desc_passwdqc_match_length, + passwdqc_random_bits => desc_random_bits, + password_complexity => "Set the complexity required for passwords. In all cases must the passwords " _ "be at least ${constants.USER_PASSWORD_MIN_LENGTH} characters long." _ diff --git a/template/en/default/global/password-features.html.tmpl b/template/en/default/global/password-features.html.tmpl index 5d6c0f8c1..ab7ae1d81 100644 --- a/template/en/default/global/password-features.html.tmpl +++ b/template/en/default/global/password-features.html.tmpl @@ -10,7 +10,7 @@ style="display: none" class="[% class FILTER html %]" data-password-page="[% password_page FILTER html %]" - data-password-complexity="[% Param("password_complexity") FILTER html %]"> + data-password-complexity="no_constraints"> Password must be 8 characters or longer, and match at least 3 of the following requirements: @@ -24,4 +24,4 @@ <div id="password-msg"></div> <div id="password-meter-label" style="display: none">Strength: <span id="password-meter" class="meter"></span></div> -</div>
\ No newline at end of file +</div> diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index a622a5eee..3e4d7c4a0 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -1505,7 +1505,7 @@ [% title = "Password Change Requests Not Allowed" %] The system is not configured to allow password change requests. - [% ELSIF error == "passwords_dont_match" %] + [% ELSIF error == "password_mismatch" %] [% title = "Passwords Don't Match" %] The two passwords you entered did not match. @@ -1513,25 +1513,13 @@ [% title = "Incorrect Password" %] You did not enter your password correctly. - [% ELSIF error == "password_too_short" %] - [% title = "Password Too Short" %] - The password must be at least - [%+ constants.USER_PASSWORD_MIN_LENGTH FILTER html %] characters long. - [% IF locked_user %] - You must <a href="token.cgi?a=reqpw&loginname=[% locked_user.email FILTER uri %]&token=[% issue_hash_token(['reqpw']) FILTER uri %]"> - request a new password</a> in order to log in again. + [% ELSIF error == "password_insecure" %] + [% title = "Password Fails Requirements" %] + The password does not meet our security requirements + [% IF reason %] + for the following reason: [% reason FILTER html %] [% END %] - [% ELSIF error == "password_not_complex" %] - [% title = "Password Fails Requirements" %] - The Password must meet three of the following requirements - <ul> - <li>uppercase letters</li> - <li>lowercase letters</li> - <li>numbers</li> - <li>symbols</li> - <li>longer than 12 characters</li> - </ul> [% IF locked_user %] You must <a href="token.cgi?a=reqpw&loginname=[% locked_user.email FILTER uri %]&token=[% issue_hash_token(['reqpw']) FILTER uri %]"> request a new password</a> in order to log in again. @@ -118,11 +118,13 @@ if ( $action eq 'reqpw' ) { my $password; if ( $action eq 'chgpw' ) { $password = $cgi->param('password'); - defined $password - && defined $cgi->param('matchpassword') - || ThrowUserError("require_new_password"); + my $matchpassword = $cgi->param('matchpassword'); + ThrowUserError("require_new_password") + unless defined $password && defined $matchpassword; + + Bugzilla->assert_password_is_secure($password); + Bugzilla->assert_passwords_match($password, $matchpassword); - validate_password($password, $cgi->param('matchpassword')); # Make sure that these never show up in the UI under any circumstances. $cgi->delete('password', 'matchpassword'); } @@ -391,15 +393,17 @@ sub confirm_create_account { Bugzilla->user->check_account_creation_enabled; my (undef, undef, $login_name) = Bugzilla::Token::GetTokenData($token); - my $password = $cgi->param('passwd1') || ''; - validate_password($password, $cgi->param('passwd2') || ''); + my $password1 = $cgi->param('passwd1'); + my $password2 = $cgi->param('passwd2'); # Make sure that these never show up anywhere in the UI. $cgi->delete('passwd1', 'passwd2'); + Bugzilla->assert_password_is_secure($password1); + Bugzilla->assert_passwords_match($password1, $password2); my $otheruser = Bugzilla::User->create({ login_name => $login_name, realname => scalar $cgi->param('realname'), - cryptpassword => $password}); + cryptpassword => $password1}); # Now delete this token. delete_token($token); @@ -410,7 +414,7 @@ sub confirm_create_account { # Log in the new user using credentials he just gave. $cgi->param('Bugzilla_login', $otheruser->login); - $cgi->param('Bugzilla_password', $password); + $cgi->param('Bugzilla_password', $password1); Bugzilla->login(LOGIN_OPTIONAL); print $cgi->header(); diff --git a/userprefs.cgi b/userprefs.cgi index 00d266eef..7d6a66c6d 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -95,8 +95,9 @@ sub SaveAccount { } if ($pwd1 ne "" || $pwd2 ne "") { - $pwd1 || ThrowUserError("new_password_missing"); - validate_password($pwd1, $pwd2); + ThrowUserError("new_password_missing") unless $pwd1; + Bugzilla->assert_password_is_secure($pwd1); + Bugzilla->assert_passwords_match($pwd1, $pwd2); if ($oldpassword ne $pwd1) { if ($user->mfa) { diff --git a/vagrant_support/checksetup_answers.j2 b/vagrant_support/checksetup_answers.j2 index 19984dfde..683ef7252 100644 --- a/vagrant_support/checksetup_answers.j2 +++ b/vagrant_support/checksetup_answers.j2 @@ -17,7 +17,6 @@ $answer{'db_user'} = 'bugs'; $answer{'diffpath'} = '/usr/bin'; $answer{'index_html'} = 0; $answer{'interdiffbin'} = '/usr/bin/interdiff'; -$answer{'password_complexity'} = 'bmo'; $answer{'user_info_class'} = 'GitHubAuth,CGI'; $answer{'user_verify_class'} = 'GitHubAuth,DB'; $answer{'urlbase'} = "http://{{WEB_HOSTNAME}}/"; |