codeant-ai-for-open-source[bot] commented on code in PR #39469:
URL: https://github.com/apache/superset/pull/39469#discussion_r3534758717
##########
superset/cli/reset.py:
##########
@@ -68,7 +69,11 @@ def factory_reset(
# Validate the user
password = click.prompt("Admin Password", hide_input=True)
user = security_manager.find_user(username)
- if not user or not check_password_hash(user.password, password):
+ if (
+ not user
+ or not user.is_active
+ or not verify_auth_db_password(user.password, password)
Review Comment:
**Suggestion:** This direct hash check bypasses the security manager
authentication flow, so failed-attempt accounting/timing-balancing logic is
skipped and this command behaves differently from normal login protections.
Route credential validation through the security manager auth method so
lockout/rate-limit and failure bookkeeping are consistently enforced. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Factory reset CLI ignores AUTH_DB login failure tracking.
- ⚠️ Brute-force attempts via CLI bypass lockout protections.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Invoke the Superset CLI entrypoint `superset` defined in
`superset/cli/main.py:55-73`,
which auto-loads subcommands from the `superset.cli` package.
2. Ensure the factory reset command is enabled by setting
`FEATURE_FLAGS["ENABLE_FACTORY_RESET_COMMAND"] = True` so the check at
`superset/cli/reset.py:55-57` passes.
3. Run `superset factory_reset` from the CLI, enter an existing admin
username and an
incorrect password at the prompts implemented in
`superset/cli/reset.py:69-70`; execution
reaches the validation block at lines 72-76.
4. In `superset/cli/reset.py:72-76`, the condition `or not
verify_auth_db_password(user.password, password)` (line 75) directly checks
the stored
hash and, on failure, prints "Invalid credentials" and calls `sys.exit(1)`
(lines 77-78)
without ever invoking `SupersetSecurityManager.auth_user_db()` in
`superset/security/manager.py:84-115`, so the usual failure bookkeeping via
`update_user_auth_stat()` and fake-hash timing-balancing logic are skipped
for this
command.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=370cb74fe52e4433b8d64505913d74da&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=370cb74fe52e4433b8d64505913d74da&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:** superset/cli/reset.py
**Line:** 75:75
**Comment:**
*Security: This direct hash check bypasses the security manager
authentication flow, so failed-attempt accounting/timing-balancing logic is
skipped and this command behaves differently from normal login protections.
Route credential validation through the security manager auth method so
lockout/rate-limit and failure bookkeeping are consistently enforced.
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%2F39469&comment_hash=02b72feb614f7b5a4d8aa123552b7d05d9abeb4b33c4b2d0b80d08bd1eb58dca&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39469&comment_hash=02b72feb614f7b5a4d8aa123552b7d05d9abeb4b33c4b2d0b80d08bd1eb58dca&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]