codeant-ai-for-open-source[bot] commented on code in PR #37773:
URL: https://github.com/apache/superset/pull/37773#discussion_r2776732423
##########
superset/security/manager.py:
##########
@@ -3022,6 +3022,21 @@ def register_views(self) -> None:
) in ["/roles", "/users", "/groups", "registrations"]:
self.appbuilder.baseviews.remove(view)
+ # When legacy FAB password views are disabled, unregister their routes
+ # so direct URL access to /superset/resetpassword and
/superset/resetmypassword
+ # is no longer possible (SPA handles password changes post-porting).
+ if not current_app.config.get("ENABLE_LEGACY_FAB_PASSWORD_VIEWS",
False):
+ from flask_appbuilder.security.views import (
+ ResetMyPasswordView,
+ ResetPasswordView,
+ )
+
+ for view in list(self.appbuilder.baseviews):
+ if isinstance(view, (ResetPasswordView, ResetMyPasswordView)):
+ if getattr(view, "blueprint", None) is not None:
+ current_app.unregister_blueprint(view.blueprint)
Review Comment:
**Suggestion:** Calling a non-existent `unregister_blueprint` method on the
Flask application object will raise an AttributeError at runtime on standard
Flask installations, breaking app startup when legacy FAB password views are
disabled; this should be guarded so it only executes if that method actually
exists. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Superset webserver fails to start with legacy views disabled.
- ❌ SecurityManager.register_views crashes during application initialization.
```
</details>
```suggestion
blueprint = getattr(view, "blueprint", None)
if blueprint is not None and hasattr(current_app,
"unregister_blueprint"):
current_app.unregister_blueprint(blueprint)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start Superset using the standard app factory in
`superset/app.py:create_app()` which
initializes Flask and Flask-AppBuilder, then invokes
`SupersetSecurityManager.register_views()` defined in
`superset/security/manager.py:3012`.
2. Ensure the configuration flag `ENABLE_LEGACY_FAB_PASSWORD_VIEWS` is unset
or explicitly
set to `False` (default behavior) in `superset/config.py`, so the block at
`superset/security/manager.py:3025-3038` executes.
3. During `register_views()`, after `super().register_views()` completes,
the code imports
`ResetPasswordView` and `ResetMyPasswordView` and iterates
`self.appbuilder.baseviews` at
`superset/security/manager.py:3034-3035`, finding one or both views whose
`blueprint`
attribute is non-None.
4. For each such view, the condition at `superset/security/manager.py:3036`
passes, and
`current_app.unregister_blueprint(view.blueprint)` is executed at line 3037;
since the
Flask app class used in Superset does not define an `unregister_blueprint`
method (only
`register_blueprint` is present in the Flask API and in Superset's app
subclass), Python
raises `AttributeError: 'Flask' object has no attribute
'unregister_blueprint'`, causing
application startup to fail whenever this configuration is used.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/manager.py
**Line:** 3036:3037
**Comment:**
*Possible Bug: Calling a non-existent `unregister_blueprint` method on
the Flask application object will raise an AttributeError at runtime on
standard Flask installations, breaking app startup when legacy FAB password
views are disabled; this should be guarded so it only executes if that method
actually exists.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37773&comment_hash=cbd5e0b645cf8dafee8d3e06d0817f0b10a4f385c33b0fc35ec68db3fdb36a59&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37773&comment_hash=cbd5e0b645cf8dafee8d3e06d0817f0b10a4f385c33b0fc35ec68db3fdb36a59&reaction=dislike'>👎</a>
--
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]
