From 8ec8da0491ad89604700b3e29a227966f6d84ba1 Mon Sep 17 00:00:00 2001 From: Perl Tidy Date: Wed, 5 Dec 2018 15:38:52 -0500 Subject: no bug - reformat all the code using the new perltidy rules --- Bugzilla/Auth/Verify/DB.pm | 152 ++++++++++++------------- Bugzilla/Auth/Verify/LDAP.pm | 253 ++++++++++++++++++++++------------------- Bugzilla/Auth/Verify/RADIUS.pm | 58 +++++----- Bugzilla/Auth/Verify/Stack.pm | 107 ++++++++--------- 4 files changed, 296 insertions(+), 274 deletions(-) (limited to 'Bugzilla/Auth/Verify') diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm index e46d1cd82..9251fa893 100644 --- a/Bugzilla/Auth/Verify/DB.pm +++ b/Bugzilla/Auth/Verify/DB.pm @@ -19,97 +19,97 @@ use Bugzilla::Util; use Bugzilla::User; sub check_credentials { - my ($self, $login_data) = @_; - my $dbh = Bugzilla->dbh; + my ($self, $login_data) = @_; + my $dbh = Bugzilla->dbh; - my $username = $login_data->{username}; - my $user = new Bugzilla::User({ name => $username }); + my $username = $login_data->{username}; + my $user = new Bugzilla::User({name => $username}); - return { failure => AUTH_NO_SUCH_USER } unless $user; + return {failure => AUTH_NO_SUCH_USER} unless $user; - $login_data->{user} = $user; - $login_data->{bz_username} = $user->login; + $login_data->{user} = $user; + $login_data->{bz_username} = $user->login; - if ($user->account_is_locked_out) { - return { failure => AUTH_LOCKOUT, user => $user }; - } - - my $password = $login_data->{password}; - return { failure => AUTH_NODATA } unless defined $login_data->{password}; - my $real_password_crypted = $user->cryptpassword; - - # Using the internal crypted password as the salt, - # crypt the password the user entered. - my $entered_password_crypted = bz_crypt($password, $real_password_crypted); + if ($user->account_is_locked_out) { + return {failure => AUTH_LOCKOUT, user => $user}; + } - if ($entered_password_crypted ne $real_password_crypted) { - # Record the login failure - $user->note_login_failure(); + my $password = $login_data->{password}; + return {failure => AUTH_NODATA} unless defined $login_data->{password}; + my $real_password_crypted = $user->cryptpassword; - # Immediately check if we are locked out - if ($user->account_is_locked_out) { - return { failure => AUTH_LOCKOUT, user => $user, - just_locked_out => 1 }; - } + # Using the internal crypted password as the salt, + # crypt the password the user entered. + my $entered_password_crypted = bz_crypt($password, $real_password_crypted); - return { failure => AUTH_LOGINFAILED, - failure_count => scalar(@{ $user->account_ip_login_failures }), - }; - } + if ($entered_password_crypted ne $real_password_crypted) { - # Force the user to change their password if it does not meet the current - # criteria. This should usually only happen if the criteria has changed. - if (Bugzilla->usage_mode == USAGE_MODE_BROWSER && - Bugzilla->params->{password_check_on_login}) - { - 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(); - } - } + # Record the login failure + $user->note_login_failure(); - # The user's credentials are okay, so delete any outstanding - # password tokens or login failures they may have generated. - Bugzilla::Token::DeletePasswordTokens($user->id, "user_logged_in"); - $user->clear_login_failures(); - - # If their old password was using crypt() or some different hash - # than we're using now, convert the stored password to using - # whatever hashing system we're using now. - my $current_algorithm = PASSWORD_DIGEST_ALGORITHM; - if ($real_password_crypted !~ /{\Q$current_algorithm\E}$/) { - # We can't call $user->set_password because we don't want the password - # complexity rules to apply here. - $user->{cryptpassword} = bz_crypt($password); - $user->update(); + # Immediately check if we are locked out + if ($user->account_is_locked_out) { + return {failure => AUTH_LOCKOUT, user => $user, just_locked_out => 1}; } - if (i_am_webservice() && $user->settings->{api_key_only}->{value} eq 'on') { - # api-key verification happens in Auth/Login/APIKey - # token verification happens in Auth/Login/Cookie - # if we get here from an api call then we must be using user/pass - return { - failure => AUTH_ERROR, - user_error => 'invalid_auth_method', - }; + return { + failure => AUTH_LOGINFAILED, + failure_count => scalar(@{$user->account_ip_login_failures}), + }; + } + + # Force the user to change their password if it does not meet the current + # criteria. This should usually only happen if the criteria has changed. + if ( Bugzilla->usage_mode == USAGE_MODE_BROWSER + && Bugzilla->params->{password_check_on_login}) + { + 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(); } - - return $login_data; + } + + # The user's credentials are okay, so delete any outstanding + # password tokens or login failures they may have generated. + Bugzilla::Token::DeletePasswordTokens($user->id, "user_logged_in"); + $user->clear_login_failures(); + + # If their old password was using crypt() or some different hash + # than we're using now, convert the stored password to using + # whatever hashing system we're using now. + my $current_algorithm = PASSWORD_DIGEST_ALGORITHM; + if ($real_password_crypted !~ /{\Q$current_algorithm\E}$/) { + + # We can't call $user->set_password because we don't want the password + # complexity rules to apply here. + $user->{cryptpassword} = bz_crypt($password); + $user->update(); + } + + if (i_am_webservice() && $user->settings->{api_key_only}->{value} eq 'on') { + + # api-key verification happens in Auth/Login/APIKey + # token verification happens in Auth/Login/Cookie + # if we get here from an api call then we must be using user/pass + return {failure => AUTH_ERROR, user_error => 'invalid_auth_method',}; + } + + return $login_data; } sub change_password { - my ($self, $user, $password) = @_; - my $dbh = Bugzilla->dbh; - my $cryptpassword = bz_crypt($password); - $dbh->do("UPDATE profiles SET cryptpassword = ? WHERE userid = ?", - undef, $cryptpassword, $user->id); - Bugzilla->memcached->clear({ table => 'profiles', id => $user->id }); + my ($self, $user, $password) = @_; + my $dbh = Bugzilla->dbh; + my $cryptpassword = bz_crypt($password); + $dbh->do("UPDATE profiles SET cryptpassword = ? WHERE userid = ?", + undef, $cryptpassword, $user->id); + Bugzilla->memcached->clear({table => 'profiles', id => $user->id}); } 1; diff --git a/Bugzilla/Auth/Verify/LDAP.pm b/Bugzilla/Auth/Verify/LDAP.pm index de88f9a43..e9aca17e8 100644 --- a/Bugzilla/Auth/Verify/LDAP.pm +++ b/Bugzilla/Auth/Verify/LDAP.pm @@ -13,7 +13,7 @@ use warnings; use base qw(Bugzilla::Auth::Verify); use fields qw( - ldap + ldap ); use Bugzilla::Constants; @@ -28,126 +28,139 @@ use constant admin_can_create_account => 0; use constant user_can_create_account => 0; sub check_credentials { - my ($self, $params) = @_; - my $dbh = Bugzilla->dbh; - - # We need to bind anonymously to the LDAP server. This is - # because we need to get the Distinguished Name of the user trying - # to log in. Some servers (such as iPlanet) allow you to have unique - # uids spread out over a subtree of an area (such as "People"), so - # just appending the Base DN to the uid isn't sufficient to get the - # user's DN. For servers which don't work this way, there will still - # be no harm done. + my ($self, $params) = @_; + my $dbh = Bugzilla->dbh; + + # We need to bind anonymously to the LDAP server. This is + # because we need to get the Distinguished Name of the user trying + # to log in. Some servers (such as iPlanet) allow you to have unique + # uids spread out over a subtree of an area (such as "People"), so + # just appending the Base DN to the uid isn't sufficient to get the + # user's DN. For servers which don't work this way, there will still + # be no harm done. + $self->_bind_ldap_for_search(); + + # Now, we verify that the user exists, and get a LDAP Distinguished + # Name for the user. + my $username = $params->{username}; + my $dn_result + = $self->ldap->search(_bz_search_params($username), attrs => ['dn']); + return { + failure => AUTH_ERROR, + error => "ldap_search_error", + details => {errstr => $dn_result->error, username => $username} + } + if $dn_result->code; + + return {failure => AUTH_NO_SUCH_USER} if !$dn_result->count; + + my $dn = $dn_result->shift_entry->dn; + + # Check the password. + my $pw_result = $self->ldap->bind($dn, password => $params->{password}); + return {failure => AUTH_LOGINFAILED} if $pw_result->code; + + # And now we fill in the user's details. + + # First try the search as the (already bound) user in question. + my $user_entry; + my $error_string; + my $detail_result = $self->ldap->search(_bz_search_params($username)); + if ($detail_result->code) { + + # Stash away the original error, just in case + $error_string = $detail_result->error; + } + else { + $user_entry = $detail_result->shift_entry; + } + + # If that failed (either because the search failed, or returned no + # results) then try re-binding as the initial search user, but only + # if the LDAPbinddn parameter is set. + if (!$user_entry && Bugzilla->params->{"LDAPbinddn"}) { $self->_bind_ldap_for_search(); - # Now, we verify that the user exists, and get a LDAP Distinguished - # Name for the user. - my $username = $params->{username}; - my $dn_result = $self->ldap->search(_bz_search_params($username), - attrs => ['dn']); - return { failure => AUTH_ERROR, error => "ldap_search_error", - details => {errstr => $dn_result->error, username => $username} - } if $dn_result->code; - - return { failure => AUTH_NO_SUCH_USER } if !$dn_result->count; - - my $dn = $dn_result->shift_entry->dn; - - # Check the password. - my $pw_result = $self->ldap->bind($dn, password => $params->{password}); - return { failure => AUTH_LOGINFAILED } if $pw_result->code; - - # And now we fill in the user's details. - - # First try the search as the (already bound) user in question. - my $user_entry; - my $error_string; - my $detail_result = $self->ldap->search(_bz_search_params($username)); - if ($detail_result->code) { - # Stash away the original error, just in case - $error_string = $detail_result->error; - } else { - $user_entry = $detail_result->shift_entry; + $detail_result = $self->ldap->search(_bz_search_params($username)); + if (!$detail_result->code) { + $user_entry = $detail_result->shift_entry; } + } - # If that failed (either because the search failed, or returned no - # results) then try re-binding as the initial search user, but only - # if the LDAPbinddn parameter is set. - if (!$user_entry && Bugzilla->params->{"LDAPbinddn"}) { - $self->_bind_ldap_for_search(); - - $detail_result = $self->ldap->search(_bz_search_params($username)); - if (!$detail_result->code) { - $user_entry = $detail_result->shift_entry; - } + # If we *still* don't have anything in $user_entry then give up. + return { + failure => AUTH_ERROR, + error => "ldap_search_error", + details => {errstr => $error_string, username => $username} } + if !$user_entry; - # If we *still* don't have anything in $user_entry then give up. - return { failure => AUTH_ERROR, error => "ldap_search_error", - details => {errstr => $error_string, username => $username} - } if !$user_entry; + my $mail_attr = Bugzilla->params->{"LDAPmailattribute"}; + if ($mail_attr) { + if (!$user_entry->exists($mail_attr)) { + return { + failure => AUTH_ERROR, + error => "ldap_cannot_retrieve_attr", + details => {attr => $mail_attr} + }; + } - my $mail_attr = Bugzilla->params->{"LDAPmailattribute"}; - if ($mail_attr) { - if (!$user_entry->exists($mail_attr)) { - return { failure => AUTH_ERROR, - error => "ldap_cannot_retrieve_attr", - details => {attr => $mail_attr} }; - } + my @emails = $user_entry->get_value($mail_attr); - my @emails = $user_entry->get_value($mail_attr); + # Default to the first email address returned. + $params->{bz_username} = $emails[0]; - # Default to the first email address returned. - $params->{bz_username} = $emails[0]; + if (@emails > 1) { - if (@emails > 1) { - # Cycle through the adresses and check if they're Bugzilla logins. - # Use the first one that returns a valid id. - foreach my $email (@emails) { - if ( login_to_id($email) ) { - $params->{bz_username} = $email; - last; - } - } + # Cycle through the adresses and check if they're Bugzilla logins. + # Use the first one that returns a valid id. + foreach my $email (@emails) { + if (login_to_id($email)) { + $params->{bz_username} = $email; + last; } - - } else { - $params->{bz_username} = $username; + } } - $params->{realname} ||= $user_entry->get_value("displayName"); - $params->{realname} ||= $user_entry->get_value("cn"); + } + else { + $params->{bz_username} = $username; + } + + $params->{realname} ||= $user_entry->get_value("displayName"); + $params->{realname} ||= $user_entry->get_value("cn"); - $params->{extern_id} = $username; + $params->{extern_id} = $username; - return $params; + return $params; } sub _bz_search_params { - my ($username) = @_; - $username = escape_filter_value($username); - return (base => Bugzilla->params->{"LDAPBaseDN"}, - scope => "sub", - filter => '(&(' . Bugzilla->params->{"LDAPuidattribute"} - . "=$username)" - . Bugzilla->params->{"LDAPfilter"} . ')'); + my ($username) = @_; + $username = escape_filter_value($username); + return ( + base => Bugzilla->params->{"LDAPBaseDN"}, + scope => "sub", + filter => '(&(' + . Bugzilla->params->{"LDAPuidattribute"} + . "=$username)" + . Bugzilla->params->{"LDAPfilter"} . ')' + ); } sub _bind_ldap_for_search { - my ($self) = @_; - my $bind_result; - if (Bugzilla->params->{"LDAPbinddn"}) { - my ($LDAPbinddn,$LDAPbindpass) = - split(":",Bugzilla->params->{"LDAPbinddn"}); - $bind_result = - $self->ldap->bind($LDAPbinddn, password => $LDAPbindpass); - } - else { - $bind_result = $self->ldap->bind(); - } - ThrowCodeError("ldap_bind_failed", {errstr => $bind_result->error}) - if $bind_result->code; + my ($self) = @_; + my $bind_result; + if (Bugzilla->params->{"LDAPbinddn"}) { + my ($LDAPbinddn, $LDAPbindpass) = split(":", Bugzilla->params->{"LDAPbinddn"}); + $bind_result = $self->ldap->bind($LDAPbinddn, password => $LDAPbindpass); + } + else { + $bind_result = $self->ldap->bind(); + } + ThrowCodeError("ldap_bind_failed", {errstr => $bind_result->error}) + if $bind_result->code; } # We can't just do this in new(), because we're not allowed to throw any @@ -156,27 +169,27 @@ sub _bind_ldap_for_search { # to fix his mistake. (Because Bugzilla->login always calls # Bugzilla::Auth->new, and almost every page calls Bugzilla->login.) sub ldap { - my ($self) = @_; - return $self->{ldap} if $self->{ldap}; - - my @servers = split(/[\s,]+/, Bugzilla->params->{"LDAPserver"}); - ThrowCodeError("ldap_server_not_defined") unless @servers; - - foreach (@servers) { - $self->{ldap} = new Net::LDAP(trim($_)); - last if $self->{ldap}; - } - ThrowCodeError("ldap_connect_failed", { server => join(", ", @servers) }) - unless $self->{ldap}; - - # try to start TLS if needed - if (Bugzilla->params->{"LDAPstarttls"}) { - my $mesg = $self->{ldap}->start_tls(); - ThrowCodeError("ldap_start_tls_failed", { error => $mesg->error() }) - if $mesg->code(); - } - - return $self->{ldap}; + my ($self) = @_; + return $self->{ldap} if $self->{ldap}; + + my @servers = split(/[\s,]+/, Bugzilla->params->{"LDAPserver"}); + ThrowCodeError("ldap_server_not_defined") unless @servers; + + foreach (@servers) { + $self->{ldap} = new Net::LDAP(trim($_)); + last if $self->{ldap}; + } + ThrowCodeError("ldap_connect_failed", {server => join(", ", @servers)}) + unless $self->{ldap}; + + # try to start TLS if needed + if (Bugzilla->params->{"LDAPstarttls"}) { + my $mesg = $self->{ldap}->start_tls(); + ThrowCodeError("ldap_start_tls_failed", {error => $mesg->error()}) + if $mesg->code(); + } + + return $self->{ldap}; } 1; diff --git a/Bugzilla/Auth/Verify/RADIUS.pm b/Bugzilla/Auth/Verify/RADIUS.pm index ad0778e39..a2a54b944 100644 --- a/Bugzilla/Auth/Verify/RADIUS.pm +++ b/Bugzilla/Auth/Verify/RADIUS.pm @@ -23,33 +23,37 @@ use constant admin_can_create_account => 0; use constant user_can_create_account => 0; sub check_credentials { - my ($self, $params) = @_; - my $dbh = Bugzilla->dbh; - my $address_suffix = Bugzilla->params->{'RADIUS_email_suffix'}; - my $username = $params->{username}; - - # If we're using RADIUS_email_suffix, we may need to cut it off from - # the login name. - if ($address_suffix) { - $username =~ s/\Q$address_suffix\E$//i; - } - - # Create RADIUS object. - my $radius = - new Authen::Radius(Host => Bugzilla->params->{'RADIUS_server'}, - Secret => Bugzilla->params->{'RADIUS_secret'}) - || return { failure => AUTH_ERROR, error => 'radius_preparation_error', - details => {errstr => Authen::Radius::strerror() } }; - - # Check the password. - $radius->check_pwd($username, $params->{password}, - Bugzilla->params->{'RADIUS_NAS_IP'} || undef) - || return { failure => AUTH_LOGINFAILED }; - - # Build the user account's e-mail address. - $params->{bz_username} = $username . $address_suffix; - - return $params; + my ($self, $params) = @_; + my $dbh = Bugzilla->dbh; + my $address_suffix = Bugzilla->params->{'RADIUS_email_suffix'}; + my $username = $params->{username}; + + # If we're using RADIUS_email_suffix, we may need to cut it off from + # the login name. + if ($address_suffix) { + $username =~ s/\Q$address_suffix\E$//i; + } + + # Create RADIUS object. + my $radius = new Authen::Radius( + Host => Bugzilla->params->{'RADIUS_server'}, + Secret => Bugzilla->params->{'RADIUS_secret'} + ) + || return { + failure => AUTH_ERROR, + error => 'radius_preparation_error', + details => {errstr => Authen::Radius::strerror()} + }; + + # Check the password. + $radius->check_pwd($username, $params->{password}, + Bugzilla->params->{'RADIUS_NAS_IP'} || undef) + || return {failure => AUTH_LOGINFAILED}; + + # Build the user account's e-mail address. + $params->{bz_username} = $username . $address_suffix; + + return $params; } 1; diff --git a/Bugzilla/Auth/Verify/Stack.pm b/Bugzilla/Auth/Verify/Stack.pm index 3e5db3cec..9a9412915 100644 --- a/Bugzilla/Auth/Verify/Stack.pm +++ b/Bugzilla/Auth/Verify/Stack.pm @@ -13,8 +13,8 @@ use warnings; use base qw(Bugzilla::Auth::Verify); use fields qw( - _stack - successful + _stack + successful ); use Bugzilla::Hook; @@ -23,70 +23,75 @@ use Hash::Util qw(lock_keys); use List::MoreUtils qw(any); sub new { - my $class = shift; - my $list = shift; - my $self = $class->SUPER::new(@_); - my %methods = map { $_ => "Bugzilla/Auth/Verify/$_.pm" } split(',', $list); - lock_keys(%methods); - Bugzilla::Hook::process('auth_verify_methods', { modules => \%methods }); - - $self->{_stack} = []; - foreach my $verify_method (split(',', $list)) { - my $module = $methods{$verify_method}; - require $module; - $module =~ s|/|::|g; - $module =~ s/.pm$//; - push(@{$self->{_stack}}, $module->new(@_)); - } - return $self; + my $class = shift; + my $list = shift; + my $self = $class->SUPER::new(@_); + my %methods = map { $_ => "Bugzilla/Auth/Verify/$_.pm" } split(',', $list); + lock_keys(%methods); + Bugzilla::Hook::process('auth_verify_methods', {modules => \%methods}); + + $self->{_stack} = []; + foreach my $verify_method (split(',', $list)) { + my $module = $methods{$verify_method}; + require $module; + $module =~ s|/|::|g; + $module =~ s/.pm$//; + push(@{$self->{_stack}}, $module->new(@_)); + } + return $self; } sub can_change_password { - my ($self) = @_; - # We return true if any method can change passwords. - foreach my $object (@{$self->{_stack}}) { - return 1 if $object->can_change_password; - } - return 0; + my ($self) = @_; + + # We return true if any method can change passwords. + foreach my $object (@{$self->{_stack}}) { + return 1 if $object->can_change_password; + } + return 0; } sub check_credentials { - my $self = shift; - my $result; - foreach my $object (@{$self->{_stack}}) { - $result = $object->check_credentials(@_); - $self->{successful} = $object; - last if !$result->{failure}; - # So that if none of them succeed, it's undef. - $self->{successful} = undef; - } - # Returns the result at the bottom of the stack if they all fail. - return $result; + my $self = shift; + my $result; + foreach my $object (@{$self->{_stack}}) { + $result = $object->check_credentials(@_); + $self->{successful} = $object; + last if !$result->{failure}; + + # So that if none of them succeed, it's undef. + $self->{successful} = undef; + } + + # Returns the result at the bottom of the stack if they all fail. + return $result; } sub create_or_update_user { - my $self = shift; - my $result; - foreach my $object (@{$self->{_stack}}) { - $result = $object->create_or_update_user(@_); - last if !$result->{failure}; - } - # Returns the result at the bottom of the stack if they all fail. - return $result; + my $self = shift; + my $result; + foreach my $object (@{$self->{_stack}}) { + $result = $object->create_or_update_user(@_); + last if !$result->{failure}; + } + + # Returns the result at the bottom of the stack if they all fail. + return $result; } sub user_can_create_account { - my ($self) = @_; - # We return true if any method allows the user to create an account. - foreach my $object (@{$self->{_stack}}) { - return 1 if $object->user_can_create_account; - } - return 0; + my ($self) = @_; + + # We return true if any method allows the user to create an account. + foreach my $object (@{$self->{_stack}}) { + return 1 if $object->user_can_create_account; + } + return 0; } sub extern_id_used { - my ($self) = @_; - return any { $_->extern_id_used } @{ $self->{_stack} }; + my ($self) = @_; + return any { $_->extern_id_used } @{$self->{_stack}}; } 1; -- cgit v1.2.3-24-g4f1b