codeant-ai-for-open-source[bot] commented on code in PR #37773:
URL: https://github.com/apache/superset/pull/37773#discussion_r3580226811
##########
tests/integration_tests/core_tests.py:
##########
@@ -154,6 +158,14 @@ def assert_admin_view_menus_in(role_name, assert_func):
assert_admin_view_menus_in("Alpha", self.assertNotIn)
assert_admin_view_menus_in("Gamma", self.assertNotIn)
+ def test_legacy_fab_password_views_are_not_registered(self):
+ from flask import current_app
+
+ endpoints = {rule.endpoint for rule in
current_app.url_map.iter_rules()}
+
+ assert "ResetPasswordView.this_form_get" not in endpoints
+ assert "ResetMyPasswordView.this_form_get" not in endpoints
Review Comment:
**Suggestion:** The endpoint assertion for `ResetMyPasswordView` is
unconditional, but the security manager intentionally keeps this view
registered when forced password change is enabled. This will produce a false
test failure in configurations with `ENABLE_FORCE_PASSWORD_CHANGE=True`; gate
this assertion on config (or assert the opposite in that mode). [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Integration tests fail when forced password change enabled.
- ⚠️ Forced-password-change deployments cannot run full test suite.
- ⚠️ CI pipelines break for security-hardened configurations.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enable forced password change while keeping legacy FAB password views
disabled by
setting `ENABLE_FORCE_PASSWORD_CHANGE = True` and
`ENABLE_LEGACY_FAB_PASSWORD_VIEWS =
False` in `superset/config.py` (lines 372-382 and 13-16).
2. Start the Superset application and let the security manager register
views;
`SupersetSecurityManager._skip_legacy_fab_password_view_registration` in
`superset/security/manager.py` (lines 4320-4339) builds
`legacy_password_views` as
`(ResetPasswordView,)` only when `ENABLE_FORCE_PASSWORD_CHANGE` is True, so
`ResetMyPasswordView` remains registered.
3. Observe that the forced-password-change enforcement hook in
`superset/security/password_change.py` (lines 170-187) relies on the
`ResetMyPasswordView.this_form_get` endpoint as its primary redirect target,
so
`current_app.url_map.iter_rules()` includes
`"ResetMyPasswordView.this_form_get"` when
forced password change is enabled.
4. Run
`tests/integration_tests/core_tests.py::CoreTests::test_legacy_fab_password_views_are_not_registered`;
the test constructs `endpoints = {rule.endpoint for rule in
current_app.url_map.iter_rules()}` (lines 65-66) and then unconditionally
executes `assert
"ResetMyPasswordView.this_form_get" not in endpoints` (line 67-68), causing
a failure even
though this endpoint is intentionally present for valid
`ENABLE_FORCE_PASSWORD_CHANGE=True` configurations.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d09b9469bc8e420484013cfc7a6f4d32&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d09b9469bc8e420484013cfc7a6f4d32&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/core_tests.py
**Line:** 167:167
**Comment:**
*Logic Error: The endpoint assertion for `ResetMyPasswordView` is
unconditional, but the security manager intentionally keeps this view
registered when forced password change is enabled. This will produce a false
test failure in configurations with `ENABLE_FORCE_PASSWORD_CHANGE=True`; gate
this assertion on config (or assert the opposite in that mode).
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37773&comment_hash=2aa0b03b27955e528369c2ec70dff3a8614d8f79599d6e516119fc98fc5cef90&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37773&comment_hash=2aa0b03b27955e528369c2ec70dff3a8614d8f79599d6e516119fc98fc5cef90&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]
