codeant-ai-for-open-source[bot] commented on code in PR #37773: URL: https://github.com/apache/superset/pull/37773#discussion_r3976431424
########## superset/config.py: ########## @@ -2076,6 +2076,13 @@ def allowed_schemas_for_csv_upload( # pylint: disable=unused-argument FAB_API_KEY_ENABLED = False FAB_API_KEY_PREFIXES = ["sst_"] +# When False (default), the legacy FAB SSR admin password reset route +# (/superset/resetpassword) is not registered. The self-service password reset +# route (/superset/resetmypassword) is also skipped unless forced password +# changes are enabled, since that flow still needs a reachable reset form. +# Set to True to re-enable direct URL access to those views (e.g. during migration). +ENABLE_LEGACY_FAB_PASSWORD_VIEWS: bool = False Review Comment: Yes—this needs an upgrade-path test. The current test only validates registration on a fresh app and does not cover persisted FAB metadata. When `ENABLE_LEGACY_FAB_PASSWORD_VIEWS` is false, `ResetPasswordView` should be excluded from role synchronization (and ideally removed from persisted permission/view-menu metadata), rather than merely skipping route registration. Otherwise an existing `ResetPasswordView` permission can remain assigned to `Admin`. A focused test should: 1. Seed or create the `ResetPasswordView` view-menu/permission and assign it to `Admin`. 2. Run the normal role synchronization with the flag disabled. 3. Assert that `Admin` no longer has the permission. 4. Assert that the stale view-menu/permission is removed if cleanup is part of the intended behavior. 5. Run the same test with the flag enabled and assert that the permission is retained. The implementation should apply the same feature-flag check in the synchronization path, not only in `register_views()`. This closes the upgrade gap while preserving legacy permissions for installations that explicitly re-enable the flag. -- 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]
