Review request for patch for HUE-1071: Option to require users to change their password when they first login

Review Request #3191 - Created June 18, 2013 and discarded

Chris Conner
old-hue
HUE-1071
hue
abec, enricoberti, romain
Added a new URL change_pass and new mako for change_pass.  Added paramater:

[desktop]
[[oauth]]
change_default_password=true

When enabled, if it's the user's first login and the AllowFirstUserDjangoBackend is enabled, then user's are re-directed to change_pass.  once they change they're password, they are redirected to Home.
I have tested to confirm that they must know the user's current password to change their password.  They are redirected to change password only on first login.  They are only redirected on first login with AllowFirstUserDjangoBackend enabled.  I've confirmed that the password change occurs.
  • 3
  • 0
  • 10
  • 0
  • 13
Description From Last Updated
Why not use PasswordChangeForm directly? Romain Rigaux
is change_pass.mako missing? Abraham Elmahrek
I don't think we need to check for AllowFirstUserDjangoBackend. This should be true for any backend. The only gotcha I ... Abraham Elmahrek
  1. Overall, great first shot. A few comments, but almost there.
  2. apps/useradmin/src/useradmin/forms.py (Diff revision 1)
     
     
    get_username_re_rule() without the parenthesis and comma. IE: regex='^%s$' % get_username_re_rule(),
  3. apps/useradmin/src/useradmin/forms.py (Diff revision 1)
     
     
    _t("Old Password")
  4. apps/useradmin/src/useradmin/forms.py (Diff revision 1)
     
     
     
     
     
     
     
    try putting this in the 'clean' method instead. The order in which fields are validated are not guaranteed. 'clean' will be called after each individual 'clean_<field>' is called though.
  5. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     
    is change_pass.mako missing?
  6. desktop/core/src/desktop/auth/views.py (Diff revision 1)
     
     
     
     
     
     
     
     
    I don't think we need to check for AllowFirstUserDjangoBackend. This should be true for any backend.
    
    The only gotcha I see is if the user is an external user and the LdapBackend is being used to authenticate. Then changing the users' password seems useless. 
    
    To sum up... we should skip this step if its an external user, which is a property of the associate UserProfile.
  7. desktop/core/src/desktop/conf.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
    move this to desktop->auth
  8. 
      
  1. Nice!!!!
    
    Some additional comments!
  2. apps/useradmin/src/useradmin/forms.py (Diff revision 1)
     
     
    Maybe this is overkill but we could inherit from UserChangeForm and overhide what we need.
  3. It is not needed probably?
  4. apps/useradmin/src/useradmin/urls.py (Diff revision 1)
     
     
    change_pass --> change_password everywhere?
  5. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     
     
     
     
    try:
      instance = User.objects.get(username=username)
    except User.DoesNotExist:
     raise PopupException(_('User %s does not exist.) % username)
  6. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     
    Why not use PasswordChangeForm directly?
  7. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     
     
     
    no need, already checked above
  8. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     
     
    Let's get rid of the locks
  9. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     
     
     
     
    not needed to me?
  10. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     
    to remove
  11. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     
    We usually do this way now:
    
    {'form': form, 'username': username}
  12. desktop/core/src/desktop/auth/views.py (Diff revision 1)
     
     
     
     
     
     
    I think there is a flaw in the logic, e.g. if the user logs in for the first time, then fails to update its new login, the algo will think he did change his password even if he didn't.
    
    e.g.
    
          if auth_form.is_valid():
            # Must login by using the AuthenticationForm.
            # It provides 'backends' on the User object.
            user = auth_form.get_user()
            require_change_password = OAUTH.CHANGE_DEFAULT_PASSWORD.get() and users_first_login(user)
    
            login(request, user)
    
            if require_change_password:
              user.last_login = self.date_joined
              user.save()
              return HttpResponseRedirect(urlresolvers.reverse('useradmin.views.change_pass', kwargs={'username': user.username}))
              
              + need to update date of user.last_login in the view if if was a success
    
    
    
    
  13. 
      
Review request changed

Status: Discarded

Loading...