Open Bug 201069 Opened 21 years ago Updated 10 months ago

[LDAP] Cannot add user to a bug if user has not logged in once

Categories

(Bugzilla :: User Accounts, enhancement)

enhancement
Not set
normal

Tracking

()

People

(Reporter: morgamic, Unassigned)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3) Gecko/20030312
Build Identifier: cvs-tip

in LDAP mode, if a user has not logged in once, their account will not exist in
the profiles table, and they will not be able to be added to a bug in any
way/shape/form -- despite the fact that they exist in the LDAP directory.

james laska produced a patch/suggested contrib that was attached to bug 51191
that addressed this problem, although 51191 was created to address another
problem.  i've taken initiative and have decided to create another bug that
specifically addresses the problem james (and i) are/were trying to address.

james' code can be viewed below, and is also available by looking at bug 51191:

------- Additional Comment #2 From James A. Laska  2002-01-29 06:25 -------

I had the same problem with my LDAP enabled Bugzilla.  To get around it I
created a function (in similar form to those already in CGI.pl) called

CGI.pl::

# Wed Aug 29 15:27:31 EDT 2001 ------------------ jlaska
# SUB:          LDAPdn_to_cn
# ARGUMENTS:    $email   - string
#               $conn    - a valid LDAP connection string, if NULL new
connection is created
# RETURNS:      $cn      - a LDAP stored cn (certified name?)
sub LDAPemail_to_cn {
    my ($email,$LDAPconn) = (@_);
    my $LDAPserver        = Param("LDAPserver");
    my $LDAPport          = "389";  # default LDAP port
    my $dnEntry           = new Mozilla::LDAP::Entry();
    my $cn                = "";
    my $arg2_flag         = ($LDAPconn)?(1):(0);

    # is LDAP enabled by bugzilla?
    if ( Param("useLDAP") ) {

        # are the LDAP modules installed?
        if ( $have_ldap ) {
            # if a valid connection was not passed as an argument, we need to
connect
            if ( !$LDAPconn ) {
                if ( $LDAPserver =~ /:/ ) {
                    ($LDAPserver, $LDAPport) = split(":",$LDAPserver);
                }

                # connect to the server
                $LDAPconn = new Mozilla::LDAP::Conn($LDAPserver,$LDAPport);
            }

            # search for user
            my $filter  = "(". Param("LDAPmail") ."=$email)";

            if ( Param("LDAPadditional") ) {
               $filter = "(&$filter" . Param("LDAPadditional") . ")";
            }
            $dnEntry = $LDAPconn->search(Param("LDAPBaseDN"),"subtree", $filter);

            # if we found a LDAP DN for the email argument given, set the $cn
and return
            if ( $dnEntry ) {
                $cn = @{$dnEntry->{cn}}[0];
            }

            if ( $arg2_flag ) {
                $LDAPconn->close;
            }
        }
    }
    return $cn;
}

This function makes use of a Parameter I added to support installation
configurable LDAP filters in bug#122365.  The above function is then called from
globals.pl::DBNameToIdAndCheck().  This way, if a user is put on as a CC and
they don't exist in the database yet, it will go and grab their info from the
lDAP server.  The same is true for reassigning bugs, if the user hasn't ever
logged into bugzilla yet...it would grab their info from the LDAP server.  

globals.pl::DBNameToIdAndCheck

sub DBNameToIdAndCheck {
    my ($name, $forceok) = (@_);
    $name = html_quote($name);
    my $result = DBname_to_id($name);
    if ($result > 0) {
        return $result;
    }
    if ($forceok) {
        InsertNewUser($name, "");
        $result = DBname_to_id($name);
        if ($result > 0) {
            return $result;
        }
        print "Yikes; couldn't create user $name.  Please report problem to " .
Param("maintainer") ."\n";    } elsif ( Param("useLDAP") ) { # Are we using
LDAP, if so add this user
        my $realname = LDAPemail_to_cn($name); # search LDAP for the realname
attached to the email addr
        if ( $realname ) {
            InsertNewUser( $name, $realname );
            $result = DBname_to_id($name);
            if ($result > 0) {
             return $result;
            }
        } else {
            print "The email address <TT><font color=\"blue\">$name</font></TT>
could not be found.  Either you\n";
            print "misspelled it, or the person does not exist in the w3
bluepages directory.\n";
            print "If you think the email address <TT><font
color=\"blue\">$name</font></TT> exists in the w3 bluepages directory, please
report this problem to ".Param("maintainer") ."\n";
        }
    } else {
        print "\n";  # http://bugzilla.mozilla.org/show_bug.cgi?id=80045
        print "The name <TT>$name</TT> is not a valid username.  Either you\n";
        print "misspelled it, or the person has not registered for a\n";
        print "Bugzilla account.\n";
        print "<P>Please hit the <B>Back</B> button and try again.\n";
    }
    PutFooter(); # there was an error if this point has been reached, close the HTML
    exit(0);
}



I hope you find this helpfull!  I certainly do :)

Reproducible: Always

Steps to Reproduce:
1. configure bugzilla to use ldap
2. log in successfully with your account
3. try to add another user who you know to exist in the ldap server to a
bugzilla ticket
4. you will not be able to add them because bugzilla does not query the ldap
server, it only queries the local profiles table
Actual Results:  
The name brewsteb@onid.orst.edu is not a valid username. Either you misspelled
it, or the person has not registered for a Bugzilla account. 

Expected Results:  
bug should be processed, and the user should be added to the profiles table
on-the-fly.
I think this is a feature - the set of LDAP users is nt necessarily teh set of
LDAP users. You want to be sure that they exist first, and that they want to use
bugzilla (the fact that this doesn't happen with 'normal' createaccount is a bug)
what i am talking about is not adding all ldap users, but rather making it so
that if you were trying to intentionally add a user to a bug who a) you know
exists in the ldap directory and b) has not logged into bugzilla yet, you would
be able to do so without having to either go ask that user to log in, or go
through the editusers.cgi just so you can add them as a cc: to a bug.

this may be something that i will have to deal with myself due to how specific
it is, but it could serve useful to others because there is a use for it...
james had a need for it, that's at least one other person...

my situation is pretty strange -- we have a support desk at a university, and
there is a central authentication server called ONID (at oregon state univ.)
that hosts an LDAP server.  using this server, students can authenticate to lab
computers, email, ssh, blackboard (online learning tool), ftp, etc.  long story
short, the users in LDAP will be the bugzilla population, as these people will
be the ones reporting problems to the database directly, or calling the support
desk and being tracked that way.  we see using LDAP as a great way to use the
university standard and avoid requiring all of these people to keep track of an
additional login/pw combination.

this may lead into a long conversation about whether or not i'm using bugzilla
in the proper environment... but so far it's working great, with a few
adjustments to template files, and the support desk is happy with the results.

eventually we want to have a situation where all students/staff who have this
ONID account can login to bugzilla (something that can already happen, because
of the help you gave me last friday in fixing bug 200742) and the technicians
will also be able to easily add an ONID user to any bug in those situations
where the user isn't accessing the system via a webpage, but rather calling the
desk over the phone.  cases like those are very common, and during the first few
months of testing, there are going to be a lot of users who meet condition a)
and b) as listed in the paragraph above.

so, i don't know -- maybe i'm filing a bs bug -- and if you guys think so,
that's cool.  i'll work on it on my own using james' contribution as a starting
point.  i posted this in order to help anyone else who may be needing the same
thing i am looking for...

sorry 'bout the essay.

I think it's a great idea personally.  We'd probably use it this way at
Syndicomm, too.  Perhaps what would satisfy bbaetz is if there were a param for
an attribute/value pair that must be present on the user's record to grant them
access to the Bugzilla system.  Probably similar to how the Host: attribute is
used for shell access to computers in some environments.

BTW, I'm pretty sure this is a dupe of bug 51192.  Since we have better and more
relevant discussion here, I think I'll mark the dupe the other direction though.
 Although the method proposed in that bug is different, the intended end result
is the same.  That bug proposes to have a script running in a cron job that
would detect new users in the LDAP directory and add them to Bugzilla.  I like
the method proposed here a little better...   Implementing what's described here
would essentially make the LDAP directory be the authoritative user directory
for Bugzilla, and the profiles table would only be consulted for
Bugzilla-specific data about that user.  This means for all user-matching code
we search LDAP instead of profiles to find out if a user exists (if LDAP is
enabled of course), and create the record for them in profiles as an
afterthought if one is missing.  We probably should add a field to the profiles
table for "External directory ID" which would be either their uid or DN for
LDAP, for easy matching of email changes, name changes, and so forth.
Status: UNCONFIRMED → NEW
Ever confirmed: true
*** Bug 51192 has been marked as a duplicate of this bug. ***
Well, the issue is that this is a bit haphazard ATM. We routinely lookup stuff
in various places, and so everywhere which looks up a user would have to do
this. Maybe if Bugzilla::User was used everywhere, but its not (and can't be til
I finish off my patch for that)
okay, then could i help in anyway with bug 180635?  might be over my head but if
i can help in anyway, let me know.

bbaetz: does the fix that james proposed conflict with what you are trying to do
in bug 180635?

the way i see it there are a couple of possible paths to take:

1) make this bug depend on bug 180635, and concentrate on Bugzilla::User first
2) fix this bug now and adjust it to accomodate bug 180635 once that is complete
-- maybe there's a way to do this without getting in the way of Bugzilla::User?

i have 8 hours a day to spend on this, so however i can help, let me know.  i
would really like to get this bug cleared, as it is the blocker for beta testing
-- but i don't want to contradict progress being made somewhere else.
been running into problems in CGI.pl.  i'm not initializing something right
because i'm dying at a given line in the name_to_cn function:

# Wed Aug 29 15:27:31 EDT 2001 ------------------ jlaska
# SUB:          LDAPdn_to_cn
# ARGUMENTS:    $email   - string
#               $conn    - a valid LDAP connection string, if NULL new
connection is created
# RETURNS:      $cn      - a LDAP stored cn (certified name?)

sub LDAPemail_to_cn {
          

    my ($email,$LDAPconn) = (@_);
    my $LDAPserver        = Param("LDAPserver");
    my $LDAPport          = "389";  # default LDAP port
    
    #call necessary mod for defining dnEntry
    use Net::LDAP::Entry;

    my $dnEntry = Net::LDAP::Entry->new;
    my $cn                = "";
    my $arg2_flag         = ($LDAPconn)?(1):(0);

    # is LDAP enabled by bugzilla?
    if ( Param("loginmethod") eq 'LDAP' ) {

      # if a valid connection was not passed as an argument, we need to connect
      if ( !$LDAPconn ) {
          if ( $LDAPserver =~ /:/ ) {
              ($LDAPserver, $LDAPport) = split(":",$LDAPserver);
          }

          # connect to the server
          my $LDAPconn = Net::LDAP->new($LDAPserver, port => $LDAPport, version
=> 3);
          $LDAPconn->start_tls(verify=>'none');
          if(!$LDAPconn) {
              return (AUTH_ERROR, undef, "connect_failed");
          } 
      }

      # search for user
      my $filter  = "(". Param("LDAPmailattribute") ."=$email)";  

      $dnEntry = $LDAPconn->search(Param("LDAPBaseDN"),"subtree", $filter);
#--------------------------------->  DIES HERE
     #ERROR MESSAGE:

     #Can't call method "search" on an undefined value at CGI.pl line 608.

      # if we found a LDAP DN for the email argument given, set the $cn and return
      if ( $dnEntry ) {
          $cn = @{$dnEntry->{cn}}[0];
      } else {
        #ERROR CHECKING --------------------------------------------------->
        print "<p>matching cn not found in ldap directory.  returning null . .
.</p>";
        }

      if ( $arg2_flag ) {
          $LDAPconn->close;
      }
    }
    return $cn;
}

any ideas?  i'm pulling hair out.
why are you calling Net::LDAP::Entry->new() and then not using the results anywhere?
i noticed that.  i redid everything and it works now.  here is the new code:

for globals.pl:

sub DBNameToIdAndCheck {
    my ($name) = (@_);
    my $result = DBname_to_id($name);
    if ($result > 0) {
        return $result;
    }
    if ( Param("loginmethod") eq 'LDAP' ) { 
      # Are we using LDAP, if so add this user

      my $realname = LDAPemail_to_cn($name); # search LDAP for the realname
attached to the email addr
      if ( $realname ) {
        InsertNewUser( $name, $realname );
        $result = DBname_to_id($name);
        if ($result > 0) {
          return $result;
          }
        } 
      } 
    ThrowUserError("invalid_username",
      { name => $name })
}

and CGI.pl:

#------------------------------------------------------------------------------------------
LDAPemail_to_cn


# Michael Morgan 4/15/2003
# SUB:          LDAPdn_to_cn
# ARGUMENTS:    $email   - string
#               $conn    - a valid LDAP connection string, if NULL new
connection is created
# RETURNS:      $cn      - a LDAP stored cn representing the real name of a user

sub LDAPemail_to_cn {
     
  my ($email,$LDAPconn) = (@_);
  
  #check LDAPserver param existence
  my $LDAPserver = Param("LDAPserver");
  if ($LDAPserver eq "") {
      return (AUTH_ERROR, undef, "server_not_defined");
  }
  
  #default LDAP port
  my $LDAPport = "389";  
  if($LDAPserver =~ /:/) {
      ($LDAPserver, $LDAPport) = split(":",$LDAPserver);
  }

  #connect to LDAP server
  $LDAPconn = Net::LDAP->new($LDAPserver, port => $LDAPport, version => 3);
  if(!$LDAPconn) {
      return (AUTH_ERROR, undef, "connect_failed");
  }
  
  #declare var that will hold search results
  my $mesg;

  #use a specific bind if the binddn param is set
  if (Param("LDAPbinddn")) {
      my ($LDAPbinddn,$LDAPbindpass) = split(":",Param("LDAPbinddn"));
      $mesg = $LDAPconn->bind($LDAPbinddn, password => $LDAPbindpass);
  }

  #if not binddn, then do an anonymous bind by default
  else {
      $mesg = $LDAPconn->bind();
  }

  #if the connection/bind fails, retrieve the error message from the results and
dislplay to user
  if($mesg->code) {
      return "";
  }

  #if we get here, we've got our anonymous bind;  let's look up this user's real
name...
  $mesg = $LDAPconn->search( base   => Param("LDAPBaseDN"),
                             scope  => "sub",
                             filter => Param("LDAPmailattribute") . "=$email"
                           );
  
  #still not sure what this does -- but it works, so i'll look it up tomorrow.  ;)
  my $user_entry = $mesg->shift_entry if !$mesg->code && $mesg->count;

  if(!$user_entry || !$user_entry->exists(Param("LDAPmailattribute"))) {
      return "";
  }
  
  #get the user's real name so we can spit it back into DBNameToIdAndCheck
  my $userRealName = $user_entry->get_value("cn");

  # we're done, so disconnect
  $LDAPconn->unbind;

  return $userRealName;

}
Attached patch proposed? please take a look. (obsolete) — Splinter Review
if i'm doing this wrong, i apologize in advance.
I would argue that LDAPemail_to_cn() belongs in Bugzilla::Auth::LDAP rather than
in CGI.pl.

Does Bugzilla::Auth have an AUTOLOAD that falls through to
Bugzilla::Auth::<current_auth_module> ?  or do the auth modules set their ISA to
Bugzilla::Auth or anything like that?  If so, maybe we might want to come up
with some generic name for it in relation to external directory lookups, and the
routine by that name in Bugzilla::Auth::DB.pm can always return undef since it
doesn't use an external directory...
Actually, Bugzilla::Auth sets its ISA to the current auth module. Its a bit odd,
but it makes sense if you think of it as mix-in rather than inheritance. (Its
also what allows the generic code using Bugzilla::Auth to not have to care about
the current auth module.)
QA Contact: mattyt-bugzilla → default-qa
I bet this will be fixed by bug 51188.
Assignee: myk → user-accounts
Depends on: 51188
Whiteboard: [blocker will fix]
Summary: in LDAP mode: cannot add user to a bug if user has not logged in once → [LDAP] Cannot add user to a bug if user has not logged in once
Has anything else come of this since 2006? This bug still exists as of 3.4, and from what I can see it also exists in the latest version 4.0.
(In reply to comment #14)
> Has anything else come of this since 2006? This bug still exists as of 3.4, and
> from what I can see it also exists in the latest version 4.0.

  Nope, nothing has come of it yet! There's some code in bug 51188 that attempted to address it, though. If you want to contribute something here, we'd be happy to have it. :-)
Attached patch v1 (obsolete) — Splinter Review
Hi mkanat,

This is a short patch that implements a way to add ldap users (in case they haven't logged in once) when adding their username in fields like, cc/qa contact, assignee, etc.
Assignee: user-accounts → timello
Attachment #120654 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #587319 - Flags: review?(mkanat)
Actually, I think this one fixed the blocker as well. I'll remove the whiteboard, but we can decide if we are gonna rename them, mark one as dup of another, whatever.
Whiteboard: [blocker will fix]
Comment on attachment 587319 [details] [diff] [review]
v1

>=== modified file 'Bugzilla.pm'

>+use Net::LDAP;

It's not ok to require Net::LDAP in Bugzilla. This Perl module is optional, not required. Also, this patch seems to duplicate a lot of code from bug 343614, or vice versa.
Attachment #587319 - Flags: review?(mkanat) → review-
Attached patch v2Splinter Review
Net::LDAP is no longer being required in Bugzilla.pm since it's optional. It has been moved to 'ldap' method.
Attachment #587319 - Attachment is obsolete: true
Attachment #696759 - Flags: review?(LpSolit)
This bug is marked as depending on bug 51188? Is that still true? If not, the dependency should be removed.
Target Milestone: --- → Bugzilla 5.0
Comment on attachment 696759 [details] [diff] [review]
v2

Moving this patch out of my radar, because I never played with LDAP. If one day I become familiar with LDAP, I will review this patch. But this is not for the near future.
Attachment #696759 - Flags: review?(LpSolit) → review?(emmanuel)
Target Milestone: Bugzilla 5.0 → ---
@justdave: about LDAP specifically, does your bug 51188 comment 18 still apply (we shouldn't leak the external ID)? In comment 3 of this bug, you seem to like the idea, though. Or maybe should we add a parameter to allow/forbid disclosing LDAP accounts, or more generally the extern_id values? Or assume that the existing LDAPfilter parameter is enough to filter accounts which can be disclosed (probably not a good idea)?
Flags: needinfo?(justdave)
I think the concerns were specifically about the extern_id values.  As the system exists today, implementing this would probably search LDAP by the user's email address, not the extern_id.  Once login name separation happens, we'd search by that.
Flags: needinfo?(justdave)
Can we have a status update on this bug?  The last comment suggests that the original objection is no longer valid and the target Milestone is set to bugzilla 5 which has been out for how long?  Is this a wontfix now?  it was reported 15 years ago.  is the patch any good any more?  Is it worth the time for someone (me?) to implement and submit a new patch?
(In reply to JT Moree from comment #24)
> Can we have a status update on this bug?  The last comment suggests that the

I have applied the changes to my test 5.x bugzilla but it does not allow me to choose users that dont exist.  Seems that the patch does not work or I dont understand what it was doing to test properly.  I will play around with it.
(In reply to JT Moree from comment #25)
> (In reply to JT Moree from comment #24)
> > Can we have a status update on this bug?  The last comment suggests that the
> 
> I have applied the changes to my test 5.x bugzilla but it does not allow me
> to choose users that dont exist.  Seems that the patch does not work or I
> dont understand what it was doing to test properly.  I will play around with
> it.

Do you have a patch for that on 5.0.4? 

I have looked into the code on editcomponent.cgi, there is a call during update action to Bugzilla::User::match_field(), which processes the default_cc field (initialcc, and others). So you can look into Bugzilla/User.pm and you will see there a match_field() method, which expands the field and then checks every user with the match() method. This method looks into the database for an existing user profile, which is sometimes not available, because the user did not log into Bugzilla.

I think, it could work to enhance the "exact match" option. At this point (there is a comment on line 1774:try an exact match), we could check for an LDAP user and add this user to the database, if there is one.

To add a new user I have looked into the edituser.cgi, which has an action="new", where the method Bugzilla::User::create() has been called. Password can be set to '*', extern_id is not clear, maybe it can be empty (I have seen, that extern_id is set to the LDAPuidattribute). You will get an User object, which you have to push into @users, the return value of the match() method.

For the LDAP access you could re-use Bugzilla/Auth/Verify/LDAP.pm, it must be enhanced to search for the LDAPmailattribute and return the real name and uid.
I have not had time to work on this.  The patch I tried was the one posted to this bug.  I would be interested in working on an updated patch but still have some time constraints.  Please post here if you start work on one.
Assignee: timello → user-accounts
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.