HUE-1071 [core] Add option to force password change on first login

Review Request #5131 - Created April 15, 2015 and submitted

Chris Conner
hue
HUE-1071
HUE-1071
c60cdf0...
hue
enricoberti, erickt, romain
commit 710c57e8fb58da647a008fa23099dc8c1d4da14f
Author: Chris Conner <cconner@cloudera.com>
Date:   Wed Apr 15 14:00:46 2015 -0400

    HUE-1071 [core] Add option to force password change on first login

:100644 100644 acec7e7... 648f79a... M	apps/useradmin/src/useradmin/forms.py
:000000 100644 0000000... d53ce93... A	apps/useradmin/src/useradmin/migrations/0004_add_field_UserProfile_first_login.py
:100644 100644 86a5f2e... 24e970b... M	apps/useradmin/src/useradmin/models.py
:100644 100644 f44b413... 12f5705... M	apps/useradmin/src/useradmin/templates/edit_user.mako
:100644 100644 616cc17... 1d176c3... M	apps/useradmin/src/useradmin/views.py
:100644 100644 5bd158d... 345724d... M	desktop/core/src/desktop/auth/backend.py
:100644 100644 0ce8f6c... 0bb3247... M	desktop/core/src/desktop/auth/views.py
:100644 100644 a6040c4... 9100fe1... M	desktop/core/src/desktop/conf.py


  • 1
  • 0
  • 11
  • 3
  • 15
Description From Last Updated
I looked and cound not find a way to avoid adding a new field as we login they check if ... Romain Rigaux
  1. 
      
  2. could be compressed with a % elif ...

    1. I tested this and if we do an "elif" here, won't we lose "Step 3" when we go to edit user as a super user because we go into the if for "Step 2".

  3. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     

    minor nit: should have spaces around the =.

  4. apps/useradmin/src/useradmin/views.py (Diff revision 1)
     
     

    What's going on with this home directory? I can't tell why you're referencing it here and other places in the patch.

    1. Not sure where this line came from, not something I wanted:-).

  5. desktop/core/src/desktop/auth/views.py (Diff revision 1)
     
     

    What happens if the user closes the page before they change the password? Will they always be redirected back to the password prompt?

    1. I tested this and no matter how many times I close the page without changing the password, they get redirected back to the password prompt. My intent was that updating "first_login" after the redirect would make sure that they had to change their password before the redirects would stop.

  6. 
      
  1. Nice one!

    Not simple, bunch of comments to check for avoiding to bite us back

  2. apps/useradmin/src/useradmin/templates/edit_user.mako (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    how about we create a brand new view about changing the password instead with a dedicated python view and template?

    (this page starts to be un-manageable)

  3. apps/useradmin/src/useradmin/views.py (Diff revision 2)
     
     

    Just this line with?

    'if' --> 'return'

    1. Wasn't sure what you were looking for here, did you want the return and if all in one line or just dropping the else: False?

    2. you could just do

      return desktop.conf.AUTH.BACKEND.get() == 'desktop.auth.backend.AllowFirstUserDjangoBackend' and self.first_login and desktop.conf.AUTH.CHANGE_DEFAULT_PASSWORD.get():

  4. apps/useradmin/src/useradmin/views.py (Diff revision 2)
     
     

    why not use 'False' instead of 0?

  5. apps/useradmin/src/useradmin/views.py (Diff revision 2)
     
     
     

    Do we really need this case?

    1. Was trying to maintain the normal login of a superuser, if we don't care about that, then I can definitely drop this:-)

    2. @Romain, any thoughts here?

    3. the admin can check go to the home page and generally I think we should not duplicate the logic

      If you redirect to redirect(reverse('about:index')) in both case it will just do the correct thing automatically for free

      BTW: does it work if a new user opens it up with something like: /accounts/login/?next=/filebrowser/ ?
      (We don't need to fix it, just wondering)

  6. 0 --> False ?

  7. desktop/core/src/desktop/auth/views.py (Diff revision 2)
     
     
     

    should it be moved around #132 that way it won't mess up the ensure_home_directory?

  8. desktop/core/src/desktop/conf.py (Diff revision 2)
     
     

    update ini and ini template too?

  9. 
      
  1. 
      
  2. apps/useradmin/src/useradmin/models.py (Diff revision 4)
     
     

    I looked and cound not find a way to avoid adding a new field

    as we login they check if first time

    https://docs.djangoproject.com/en/1.8/ref/contrib/auth/#django.contrib.auth.models.User.last_login

    1. Nothing we can do here until Django 1.8, right?

    2. Even there, probably would not helped as the user will have a login_date updated to right now as he would have just logged-in!

  3. apps/useradmin/src/useradmin/views.py (Diff revision 4)
     
     

    how about just:

    if form.cleaned_data.get('ensure_home_directory'):

    same thing but cleaner.

    you changed it here because you pop-ed it the form

    JFYI if you wanted you could put it in a <span class="hide">${layout.render_field(form["ensure_home_directory"])}..
    but that's good with above

  4. desktop/core/src/desktop/auth/views.py (Diff revision 4)
     
     
     

    swap lines to keep the imports sorted?

  5. desktop/core/src/desktop/auth/views.py (Diff revision 4)
     
     

    0 --> False is cleaner

  6. 
      
  1. Nice!

  2. 
      
Review request changed

Status: Closed (submitted)

Loading...