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)
Bugzilla
User Accounts
Tracking
()
NEW
People
(Reporter: morgamic, Unassigned)
References
Details
Attachments
(1 file, 2 obsolete files)
7.16 KB,
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•21 years ago
|
||
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)
Reporter | ||
Comment 2•21 years ago
|
||
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.
Comment 3•21 years ago
|
||
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
Comment 5•21 years ago
|
||
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)
Reporter | ||
Comment 6•21 years ago
|
||
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.
Reporter | ||
Comment 7•21 years ago
|
||
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.
Comment 8•21 years ago
|
||
why are you calling Net::LDAP::Entry->new() and then not using the results anywhere?
Reporter | ||
Comment 9•21 years ago
|
||
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; }
Reporter | ||
Comment 10•21 years ago
|
||
if i'm doing this wrong, i apologize in advance.
Comment 11•21 years ago
|
||
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...
Comment 12•21 years ago
|
||
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.)
Updated•18 years ago
|
QA Contact: mattyt-bugzilla → default-qa
Comment 13•18 years ago
|
||
I bet this will be fixed by bug 51188.
Updated•18 years ago
|
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
Comment 14•13 years ago
|
||
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.
Comment 15•13 years ago
|
||
(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. :-)
Comment 16•12 years ago
|
||
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)
Comment 17•12 years ago
|
||
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 18•12 years ago
|
||
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-
Comment 19•11 years ago
|
||
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)
Comment 20•11 years ago
|
||
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 21•11 years ago
|
||
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)
Updated•10 years ago
|
Target Milestone: Bugzilla 5.0 → ---
Comment 22•9 years ago
|
||
@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)
Comment 23•9 years ago
|
||
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)
Comment 24•7 years ago
|
||
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?
Comment 25•7 years ago
|
||
(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.
Comment 26•6 years ago
|
||
(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.
Comment 27•6 years ago
|
||
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.
Updated•10 months ago
|
Assignee: timello → user-accounts
Status: ASSIGNED → NEW
You need to log in
before you can comment on or make changes to this bug.
Description
•