#8274 Add optional HaveIBeenPwned checks for password changes

v1.11.0
closed
None
General
2019-06-17
2019-04-04
No

Discussion

  • Kenton Taylor

    Kenton Taylor - 2019-04-04
    • summary: Add optional HaveIBeenPwned checks for password c hanges --> Add optional HaveIBeenPwned checks for password changes
     
  • Kenton Taylor

    Kenton Taylor - 2019-04-04
    • status: in-progress --> review
     
  • Kenton Taylor

    Kenton Taylor - 2019-04-04

    kt/8274

     
  • Dave Brondsema

    Dave Brondsema - 2019-04-05
    • status: review --> open
    • Reviewer: Dave Brondsema
     
  • Dave Brondsema

    Dave Brondsema - 2019-04-05

    How about doing the check in PasswordChangeBase.to_python which is shared between all the forms usage? If we're lucky the existing error handling will just work too, and can clean up the url repetion for failure_redirect_url. And if you're able to undo the changes to controllers, that'll avoid conflicts with my TurboGears changes which tweaked controllers calling to_python.

    Careful adding __future__ to existing files, it may change behavior. Seems to be ok here though.

    User-Agent should probably use config['site_name']

    hibp_password_check config should go in development.ini rather than docker-dev, and add an explanation for it.

     

    Last edit: Dave Brondsema 2019-04-05
    • Kenton Taylor

      Kenton Taylor - 2019-04-08

      Fixup pushed. As discussed, I originally considered placing this in PasswordChangeBase, but that felt like too "core" of an area for it; also, placing the checks in the controller allows the controller to determine how to react, rather than it being an immutable behavior.

       
  • Kenton Taylor

    Kenton Taylor - 2019-04-08
    • status: open --> review
     
  • Dave Brondsema

    Dave Brondsema - 2019-04-09
    • status: review --> in-progress
     
  • Dave Brondsema

    Dave Brondsema - 2019-04-09
    • Some allura.tests.functional.test_auth tests are failing, I believe because hibp_password_check is set to true, and test.ini also inherits from development.ini. Perhaps best to set it to false by default? Or could set it to false in test.ini
    • For consistency, can you rename the config to auth.hibp_password_check and put it with the rest of the auth.* settings related to accounts & security?
     
  • Kenton Taylor

    Kenton Taylor - 2019-04-09
    • status: in-progress --> review
     
  • Kenton Taylor

    Kenton Taylor - 2019-04-09

    All set in new fixup.

     
  • Dave Brondsema

    Dave Brondsema - 2019-04-10
    • status: review --> closed
     
  • Dave Brondsema

    Dave Brondsema - 2019-06-17
    • Milestone: unreleased --> v1.11.0
     

Log in to post a comment.