Project

General

Profile

Bug #1322

login_password_profile template assumes openid user if there's no password

Added by Frédéric Péters over 7 years ago. Updated almost 4 years ago.

Status:
Rejeté
Priority:
Bas
Assignee:
-
Category:
-
Target version:
Start date:
28 Mar 2012
Due date:
% Done:

0%

Patch proposed:
No
Planning:
No

Description

login_password_profile.html has this snippet:

{% if user.password == '!' %}
 <p><a href="{% url authopenid_password_change %}">{% trans "Set password" %}</a></p>
{% else %}

However it will also be true in case of LDAP users, and then:

Caught NoReverseMatch while rendering: Reverse for 'authopenid_password_change' with arguments '()' and keyword arguments '{}' not found.

History

#1 Updated by Jean Christophe André over 7 years ago

I confirm I do hit this bug too, since I choose to base our authentication on our LDAP.

I would suggest to:
  1. save the source of the User creation somewhere
  2. create a class for each source type (LDAP, OpenID, registration, ...)
  3. define in these classes a method for determining if the password is changeable or not (the LDAP one would test for a “Allow password change” checkbox from the LDAP source form)
  4. use this method to test if the “Change/Set password” link should be displayed or not (would solve this bug)
  5. define in these classes a method for changing the password (the LDAP one would ask the user for old and new passwords and then call ldap.bind_s and ldap.password_s)

Just my 2 cents...

#2 Updated by Benjamin Dauvergne over 7 years ago

écrivait:

La demande #1322 a été mise à jour par Jean Christophe André.

I confirm I do hit this bug too, since I choose to base our authentication on our LDAP.

I would suggest to:
  1. save the source of the User creation somewhere
  2. create a class for each source type (LDAP, OpenID, registration, ...)
  3. define in these classes a method for determining if the password is changeable or not (the LDAP one would test for a “Allow password change” checkbox from the LDAP source form)
  4. define in these classes a method for changing the password (the LDAP one would ask the user for old and new passwords and then call ldap.bind_s and ldap.password_s)

Just my 2 cents...

For LDAP there is a complication, some LDAP directorie (from Microsoft,
you should know which one I'm talking about :) ) have special way of
changing the password and on some LDAP directories the userPasswd is not
accessible (sometimes because it's not event in the LDAP tree, sometimes
for security reasons), and in this case you must use the special command
PasswordModify. On those two occurrences you must send to the LDAP
server the old and the new password. The current password modification
form only works for setting the password without recalling the old
password.

So my idea is:
  • overload the LDAP backend to return User class object which a new
    synthesized set_password() method which use LDAP commands to update the
    passwd instead of modifying the local User object. Something like:
        def get_user(self, user_id):
            user = super(NewLdapBackend, self).get_user(user_id)
            def set_password(self, new_password):
                ldap_user = self.ldap_user
                ldap = ldap_user.ldap
                try:
                      self.ldap_user.connection.modify_s(ldap_user.dn.encode('utf-8'),
                          [(ldap.MOD_REPLACE, 'userPassword', new_password.encode('utf-8'))])
                except ldap.LDAPError:
                      pass
            user.set_password = set_password
            return user
    

    This code could have more features:
    • handle password hashing formats for LDAP
    • call the old set_password() to also set the local passwd, allowing local connection to work if LDAP is down
    • report LDAP errors to admin and maybe to user (a special logging domain with
      a handler able to post message to the user would be great for this)
  • add an LDAP_PASSWORD_MODIFY_BEHAVIOUR setting to set, if we are on
    openldap with full access to userAttribute, on AD or if we use
    PasswordModify, in case of PasswordModify or AD the set_password()
    method synthesized we have an extra parameter current_password.
  • modify the ChangePasswordForm to have an extra input field "Current
    password" if the set_password() method on the current user have the
    extra argument "current_password".

#3 Updated by Benjamin Dauvergne over 7 years ago

My answer is not really related to this ticket, I should open another one.

The correct answer for this ticket is to copy the view from django_authopenid.views as it is not really related to OpenID but to the case where an user does not have a password but is already logged. This new view should consider that in some cases the fact that the User does not have a usable password (i.e. stored password is '!') is not a sufficent reason to allow setting the password without giving the old (see the LDAP case). But this case maybe should be resolved by overloading has_usable_password() for LDAP users.

#4 Updated by Mikaël Ates over 7 years ago

  • Target version set to 2.0.2

#5 Updated by Mikaël Ates over 7 years ago

  • Target version changed from 2.0.2 to 2.0.3

#6 Updated by Benjamin Dauvergne almost 5 years ago

  • Priority changed from Normal to Bas

#7 Updated by Benjamin Dauvergne almost 5 years ago

  • Target version deleted (2.0.3)
  • Patch proposed set to No

#8 Updated by Benjamin Dauvergne almost 5 years ago

  • Target version set to future

#9 Updated by Benjamin Dauvergne almost 4 years ago

  • Status changed from Nouveau to Rejeté

authopenid is not part of authentic2 anymore.

Also available in: Atom PDF