diff --git a/Bugzilla.pm b/Bugzilla.pm index 8ce44ec93..652fa2ec0 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -240,6 +240,26 @@ sub github_secret { sub assert_password_is_secure { my ($class, $password1) = @_; + my $reason; + + if (length($password1) < USER_PASSWORD_MIN_LENGTH) { + $reason = 'too short'; + } + elsif (Bugzilla->params->{password_complexity} eq 'bmo') { + my $features = 0; + $features++ if $password1 =~ /[a-z]/; + $features++ if $password1 =~ /[A-Z]/; + $features++ if $password1 =~ /[0-9]/; + $features++ if $password1 =~ /[^A-Za-z0-9]/; + $features++ if length($password1) > 12; + $reason = 'not complex enough' if $features < 3; + } + + # Allow an extension to apply a stricter policy. + Bugzilla::Hook::process('password_check', + {password => $password1, reason => \$reason}); + + ThrowUserError('password_insecure', {reason => $reason}) if $reason; } sub assert_passwords_match { diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index a44108fca..5e72c5d3f 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -1108,8 +1108,9 @@ sub update { # we only audit when bugs protected with a secure-mail enabled group # are made public if ( - !scalar @{$self->groups_in} && any { $old_groups{$_}->secure_mail } - @$removed_gr + !scalar @{$self->groups_in} + && Bugzilla->has_extension('SecureMail') + && any { $old_groups{$_}->secure_mail } @$removed_gr ) { Bugzilla->audit(sprintf( diff --git a/Bugzilla/Config/Auth.pm b/Bugzilla/Config/Auth.pm index 6bf49ade4..0f91db1c1 100644 --- a/Bugzilla/Config/Auth.pm +++ b/Bugzilla/Config/Auth.pm @@ -12,8 +12,6 @@ use strict; use warnings; use Bugzilla::Config::Common; -use Types::Standard qw(Tuple Maybe); -use Types::Common::Numeric qw(PositiveInt); our $sortkey = 300; @@ -95,47 +93,6 @@ sub get_param_list { {name => 'password_check_on_login', 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 => 'passwdqc_desc', - type => 'l', - default => 'The password must be complex.', - }, - {name => 'auth_delegation', type => 'b', default => 0,}, {name => 'duo_host', type => 't', default => '',}, @@ -161,52 +118,5 @@ 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/Hook.pm b/Bugzilla/Hook.pm index 07ef07fde..ee09b031b 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -1415,6 +1415,28 @@ your template. =back +=head2 password_check + +Allows an extension to apply a stricter password policy than the built-in +check. This is called after the core check has run. + +Params: + +=over + +=item C + +The plain-text password being validated. + +=item C + +A reference to a scalar. If the core check already rejected the password, +it contains the reason. Set it to a short reason string to reject a +password that the core check accepted. Leaving it unchanged accepts the +core result. + +=back + =head2 path_info_whitelist By default, Bugzilla removes the Path-Info information from URLs before diff --git a/qa/config/checksetup_answers.txt b/qa/config/checksetup_answers.txt index 79d9cb90c..6aba213bc 100644 --- a/qa/config/checksetup_answers.txt +++ b/qa/config/checksetup_answers.txt @@ -1,6 +1,6 @@ $answer{'ADMIN_EMAIL'} = 'admin@mozilla.bugs'; $answer{'ADMIN_OK'} = 'Y'; -$answer{'ADMIN_PASSWORD'} = 'password'; +$answer{'ADMIN_PASSWORD'} = 'password01!'; $answer{'ADMIN_REALNAME'} = 'QA Admin'; $answer{'NO_PAUSE'} = 1; $answer{'apache_size_limit'} = 700000; diff --git a/qa/config/generate_test_data.pl b/qa/config/generate_test_data.pl index c5ce33034..a948ba1d0 100644 --- a/qa/config/generate_test_data.pl +++ b/qa/config/generate_test_data.pl @@ -547,7 +547,7 @@ BEGIN Bugzilla::User->create({ login_name => $watch_user, - cryptpassword => "password1", + cryptpassword => "Password1", disable_mail => 1, }); diff --git a/t/903-passwdqc-conf.t b/t/903-passwdqc-conf.t deleted file mode 100644 index 0bc4954fe..000000000 --- a/t/903-passwdqc-conf.t +++ /dev/null @@ -1,40 +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 strict; -use warnings; -use 5.14.0; -use lib qw( . lib local/lib/perl5 ); -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/template/en/default/admin/params/auth.html.tmpl b/template/en/default/admin/params/auth.html.tmpl index ecadf0aaf..8101ef434 100644 --- a/template/en/default/admin/params/auth.html.tmpl +++ b/template/en/default/admin/params/auth.html.tmpl @@ -24,73 +24,6 @@ desc = "Set up your authentication policies" %] -[% desc_passwdqc_min = BLOCK %] - [Int0, Int1, Int2, Int3, Int4] -

- 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. - -

- 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. -

- Int1 is used for passwords consisting of characters from two character - classes that do not meet the requirements for a passphrase. -

- 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). -

- Int3 and Int4 are used for passwords consisting of characters from - three and four character classes, respectively. - -

- 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. - -

- 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. -

- 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 %] - -[% desc_passwdqc_desc = BLOCK %] - This should be a short paragraph describing the password requirements in plain English. - Limited HTML allowed. -[% END %] [% param_descs = { auth_env_id => "Environment variable used by external authentication system " _ @@ -201,13 +134,6 @@ "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, - passwdqc_desc => desc_passwdqc_desc, - password_complexity => "Set the complexity required for passwords. In all cases must the passwords " _ "be at least ${constants.USER_PASSWORD_MIN_LENGTH} characters long." _