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
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
kt/8274
How about doing the check in
PasswordChangeBase.to_pythonwhich 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 forfailure_redirect_url. And if you're able to undo the changes to controllers, that'll avoid conflicts with my TurboGears changes which tweaked controllers callingto_python.Careful adding
__future__to existing files, it may change behavior. Seems to be ok here though.User-Agentshould probably useconfig['site_name']hibp_password_checkconfig should go in development.ini rather than docker-dev, and add an explanation for it.Last edit: Dave Brondsema 2019-04-05
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.allura.tests.functional.test_authtests are failing, I believe becausehibp_password_checkis 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.iniauth.hibp_password_checkand put it with the rest of theauth.*settings related to accounts & security?All set in new fixup.