[LDAP] Add support for changing passwords




User Accounts
12 years ago
5 years ago


(Reporter: timeless, Unassigned)





(1 attachment, 1 obsolete attachment)

10.38 KB, patch
Max Kanat-Alexander
: review-
Details | Diff | Splinter Review


12 years ago
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.


12 years ago
Blocks: 344275

Comment 1

12 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

Comment 2

12 years ago
Created attachment 245655 [details] [diff] [review]
patch 4 templates and AUTH

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

12 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

12 years ago
Created attachment 246009 [details] [diff] [review]
initial LDAP Support 4 BZ

remove module Net::LDAP in userprefs
Attachment #245655 - Attachment is obsolete: true
Attachment #246009 - Flags: review?(mkanat)
Attachment #245655 - Flags: review?(mkanat)

Comment 5

12 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:


  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.