301634 - Authentication errors when LDAP user does not exist in Bugzilla database

Status: RESOLVED WORKSFORME

(Bugzilla :: User Accounts, defect)

Description

[Alex Yamauchi]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050513 Fedora/1.0.4-1.3.1 Firefox/1.0.4
Build Identifier: Bugzilla 2.19.3

When using an LDAP login to access Bugzilla for the first time, Bugzilla errors
out in two different modules (at least for my set up) when trying to set up the
the Bugzilla user account from the LDAP data.  Specifically
Bugzilla::Auth::Verify::LDAP and Bugzilla::User.  See "Additional Information"
for the error log entries and the fixes I used to get my installation working.

Also, in the process of fixing these problems for my installation, I was quite
disturbed to see that Bugzilla dumps cleartext usernames and passwords into
error log message bodies in a publicly readable log file when the Auth fails.

Reproducible: Always

Steps to Reproduce:
Log in to Bugzilla using an LDAP account that has never accessed Bugzilla before.
Actual Results:  
The first error occurs in Bugzilla::Auth::Verify::LDAP.  The "displayName"
attribute is not set in my LDAP configuration, so $RealUserName is not defined
on the first pass.  $RealUserName is then evaluated against "" in a conditional
in the next command, which fails because $RealUserName is undefined and "use
strict" has been specified.

The second error occurs in Bugzilla::User - it is unable to find the function
bz_crypt.

Expected Results:  
Authenticated the user and created a corresponding user in the Bugzilla
database, without error.

Here are the original error log messages (host and IPs removed):
[Wed Jul 20 17:41:29 2005] [error] [client X.X.X.X] [Wed Jul 20 17:41:29 2005]
index.cgi: Use of uninitialized value in string eq at
Bugzilla/Auth/Verify/LDAP.pm line 150, <DATA> line 283., referer: http://XXX/
[Wed Jul 20 17:41:29 2005] [error] [client X.X.X.X] [Wed Jul 20 17:41:29 2005]
index.cgi: Undefined subroutine &Bugzilla::User::bz_crypt called at
Bugzilla/User.pm line 1085, <DATA> line 283., referer: http://XXX/

Here are the diffs from how I fixed this on my installation.

LDAP.pm diff:
-------------
150,153c150,152
<         if($userRealName eq "") {
<             $userRealName = $user_entry->get_value("cn");
<         }
<         insert_new_user($username, $userRealName);
---
>               $userRealName ||= $user_entry->get_value("cn");
>
>               insert_new_user($username, $userRealName);

User.pm diff
------------
1085c1085
<     my $cryptpassword = bz_crypt($password);
---
>     my $cryptpassword = &Bugzilla::Auth::bz_crypt($password);

Comment 1

[Alex Yamauchi]

Attachment: Bugzilla/Auth/Verify/LDAP.pm diff

Bugzilla/Auth/Verify/LDAP.pm diff

Comment 2

[Alex Yamauchi]

Attachment: Bugzilla/User.pm diff

Comment 3

[Marc Schumann [:Wurblzap]]

The missing bz_crypt issue has been fixed in bug 300336.

Comment 4

[A. Karl Kornel]

Suggest confirming.  I don't know how problematic this can be, but the mentioned
code is definitely not catching the proper return values.

$userRealName (line 150 of Bugzilla::Auth::Verify::LDAP) holds the result of a
call to the get_value method of the Net::LDAP::Entry object, which (in scalar
context) returns a single value or undef.  The test on line 151 should be
explicitly looking for an undef (as performed in the patch).

Reporter, in the future patches should be submitted as a single unified diff
file. Also, at least one of your patches is no longer needed (see comment 3).

Comment 5

[Max Kanat-Alexander]

Confirmed based on karl's code inspection.

Comment 6

[Max Kanat-Alexander]

I suspect that this was fixed by the Auth re-write. Looking at the new code, we should deal properly with the case where displayName is empty. We also check cn for the realname now.