rusackas commented on PR #37773: URL: https://github.com/apache/superset/pull/37773#issuecomment-4965842358
Thanks for chasing this down, @jayvenn21. Think there seems to be a regression here though: `password_change.py:187` hardcodes `ResetMyPasswordView.this_form_get` as the redirect target for the forced-password-change flow (`ENABLE_FORCE_PASSWORD_CHANGE`). With the new flag defaulting `ENABLE_LEGACY_FAB_PASSWORD_VIEWS` to `False`, that endpoint no longer exists, so `url_for` fails and the hook falls back to logging the user out instead of letting them reset. Anyone with `password_must_change` set would just get bounced to login/logout in a loop, never able to actually change their password. Would be good to add a test that exercises both flags together. Also, @dpgaspar's suggestion of a cleaner FAB-side config flag (like `FAB_ADD_SECURITY_VIEWS`) doesn't seem to have been followed up on. The PR ended up monkeypatching `appbuilder.add_view_no_menu` instead. Did the FAB PR not happen, or is there a reason the local patch would be preferred? Would rather not merge a workaround like that without closing that loop first. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
