Open
Bug 344271
Opened 18 years ago
Updated 11 years ago
[LDAP] Add support for changing passwords
Categories
(Bugzilla :: User Accounts, enhancement)
Tracking
()
NEW
People
(Reporter: timeless, Unassigned)
References
()
Details
Attachments
(1 file, 1 obsolete file)
10.38 KB,
patch
|
mkanat
:
review-
|
Details | Diff | Splinter Review |
from bugzilla-meeting. we need the ability to change passwords using the bugzilla ui when bugzilla is configured to use ldap authentication. the ui for the end user shouldn't change, but the password should be changed in the ldap db. if the ldap db refuses to allow the change, an error should be delivered to the user, and perhaps a message logged or sent to an admin.
Comment 1•18 years ago
|
||
Net::LDAP would have to support this, though. Maybe it does.
OS: Windows XP → All
Hardware: PC → All
Summary: add support for changing passwords when bugzilla is using the ldap authentication module → [LDAP] Add support for changing passwords
Updated•18 years ago
|
Updated•18 years ago
|
Comment 2•18 years ago
|
||
Andrea a member of the support team our company made some changes at the interface and backend code to support changing ldap passwords. In adjustment with Max we added a new password field if ldap is configured on the confirm.html.tmpl page. Changes at the E-Mail address in LDAP only allowed if the user can successful authenticated against the LDAP server
Attachment #245655 -
Flags: review?(mkanat)
Comment 3•18 years ago
|
||
Comment on attachment 245655 [details] [diff] [review] patch 4 templates and AUTH >--- bugzilla-2.23.3+/userprefs.cgi 2006-11-15 16:13:06.406668144 +0100 >+++ bugzilla/userprefs.cgi 2006-11-15 15:56:39.128302992 +0100 >+use Bugzilla::Auth::Verify::LDAP; >+use Net::LDAP; I don't have Net::LDAP installed. I still want to be able to use userprefs.cgi.
Attachment #245655 -
Flags: review-
Comment 4•18 years ago
|
||
remove module Net::LDAP in userprefs
Attachment #245655 -
Attachment is obsolete: true
Attachment #246009 -
Flags: review?(mkanat)
Attachment #245655 -
Flags: review?(mkanat)
Comment 5•18 years ago
|
||
Comment on attachment 246009 [details] [diff] [review] initial LDAP Support 4 BZ >+++ bugzilla/Bugzilla/Auth/Verify/LDAP.pm 2006-11-17 11:27:27.253563048 +0100 >-use constant user_can_create_account => 0; >+use constant user_can_create_account => 1; That's not true. Users can't create their own accounts. Why'd you change that? If you need a user_can_change_password, then add that. >@@ -56,18 +59,8 @@ >+ my $dn = $self->get_dn($username); get_dn can return an AUTH_ERROR code, but you never check for that, anywhere. >@@ -150,4 +143,67 @@ >+sub change_password { >+ my ($self, $cgi, $pwd1, $pwd2) = @_; That is not the interface of change_password. The interface is ($user, $password). Read the perldoc of Verify.pm. You may change it to be $user, $new_password, $old_password, if you need to. >+ my $bz_pwd = $cgi->param('Bugzilla_password'); >+ my $bz_login = $cgi->param('Bugzilla_login'); One thing you absolutely cannot do is access those values here, or use the CGI in any way inside of Verify.pm. >+ if ($bz_pwd ne "" || $pwd1 ne "" || $pwd2 ne "") >+ { >+ $self->_bind_ldap_anonymously(); >+ my $dn = $self->get_dn($bz_login); If you always need to bind to get the dn, then you should bind inside get_dn, but only if $self->ldap is not already bound. >+ # Check the password. >+ my $pw_result = $self->ldap->bind($dn, password => $bz_pwd); >+ return { failure => AUTH_LOGINFAILED } if $pw_result->code; >+ validate_password($pwd1, $pwd2); That should have been done by the caller. >+ if ($bz_pwd ne $pwd1) { >+ my $result = $self->ldap->modify($dn, replace=>{userPassword=>$pwd1}); >+ } Net::LDAP has a very long section in its FAQ about changing the password for a user: http://search.cpan.org/dist/perl-ldap/lib/Net/LDAP/FAQ.pod#Ho_do_I_reset_a_user%27s_password_... You'll want to check if the server actually supports the operation you're doing above, before doing it. For example, that will fail on Active Directory. We don't have to support Active Directory in this bug, but we shouldn't *claim* to support it if we don't. >+sub change_login { Let's do that in a separate bug. >+sub get_dn { >+ my ($self, $username) = @_; >+ >+# Now, we verify that the user exists, and get a LDAP Distinguished >+# Name for the user. The comment needs to be indented just like the code. >+++ bugzilla/template/en/default/account/email/confirm.html.tmpl 2006-11-10 15:04:55.000000000 +0100 >@@ -39,6 +39,11 @@ > <td><input type="text" name="email" size="36"></td> > </tr> > <tr> >+ [% IF Param('user_verify_class') != 'DB' %] != 'DB' doesn't make sense. There could be other Verify methods in the future. >+++ bugzilla/userprefs.cgi 2006-11-17 09:51:16.522585704 +0100 >+ if (Bugzilla->params->{'user_verify_class'} eq "DB") { This whole section: No, no. Absolutely not. $user->authorizer->change_password -- that exists for a reason. Anyhow, there may be other user_verify_class options in the future, besides just LDAP and DB. But use the change_password interface, don't write your own interface here in userprefs.cgi itself.
Attachment #246009 -
Flags: review?(mkanat) → review-
You need to log in
before you can comment on or make changes to this bug.
Description
•