codeant-ai-for-open-source[bot] commented on code in PR #36857:
URL: https://github.com/apache/superset/pull/36857#discussion_r2652399921
##########
superset/views/base.py:
##########
@@ -479,7 +479,7 @@ def cached_common_bootstrap_data( # pylint:
disable=unused-argument
auth_type = app.config["AUTH_TYPE"]
auth_user_registration = app.config["AUTH_USER_REGISTRATION"]
frontend_config["AUTH_USER_REGISTRATION"] = auth_user_registration
- should_show_recaptcha = auth_user_registration and (auth_type !=
AUTH_OAUTH)
+ should_show_recaptcha = auth_user_registration and (auth_type !=
AUTH_OAUTH) and (auth_type != AUTH_LDAP)
Review Comment:
**Suggestion:** Computing `should_show_recaptcha` uses
`auth_user_registration` and auth type checks but does not validate that
`RECAPTCHA_PUBLIC_KEY` exists; this can lead to a KeyError later when the key
is accessed. Also ensure `auth_user_registration` is coerced to a boolean to
avoid surprising truthiness from non-boolean config values. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
should_show_recaptcha = (
bool(auth_user_registration)
and (auth_type != AUTH_OAUTH)
and (auth_type != AUTH_LDAP)
and bool(app.config.get("RECAPTCHA_PUBLIC_KEY"))
)
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This improves correctness: coercing auth_user_registration to bool avoids
surprising truthiness from non-boolean config values, and gating on the
presence of RECAPTCHA_PUBLIC_KEY prevents a later KeyError when the code
dereferences app.config[...] to set the frontend config. The change directly
addresses a real runtime risk visible in the PR hunk.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/views/base.py
**Line:** 482:482
**Comment:**
*Possible Bug: Computing `should_show_recaptcha` uses
`auth_user_registration` and auth type checks but does not validate that
`RECAPTCHA_PUBLIC_KEY` exists; this can lead to a KeyError later when the key
is accessed. Also ensure `auth_user_registration` is coerced to a boolean to
avoid surprising truthiness from non-boolean config values.
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>
--
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]
