You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "schoel-bis (via GitHub)" <gi...@apache.org> on 2026/01/09 09:50:29 UTC
[PR] fix: do not require recaptcha public key for LDAP authentication [superset]
schoel-bis opened a new pull request, #37009:
URL: https://github.com/apache/superset/pull/37009
### SUMMARY
Fixes #37008.
In addition to OAuth, also ignores ReCaptcha public key for LDAP authentication.
### TESTING INSTRUCTIONS
1. Set up LDAP authentication with `AUTH_USER_REGISTRATION`
2. Run the application
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: #37008
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #37009:
URL: https://github.com/apache/superset/pull/37009#discussion_r3538273840
##########
superset/views/base.py:
##########
@@ -35,7 +35,7 @@
)
from flask_appbuilder import BaseView, Model, ModelView
from flask_appbuilder.actions import action
-from flask_appbuilder.const import AUTH_OAUTH
+from flask_appbuilder.const import AUTH_LDAP, AUTH_OAUTH
Review Comment:
AUTH_OAUTH and AUTH_SAML already come from that same import with no fallback, and AUTH_LDAP is just as old a FAB constant β wrapping only this one would be inconsistent.
##########
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 not in [AUTH_OAUTH, AUTH_LDAP])
Review Comment:
The existing check already compares against these constants directly with no normalization, so guarding just the LDAP case would be inconsistent. Leaving as-is.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #37009:
URL: https://github.com/apache/superset/pull/37009
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-4932606689
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-4931678822
Went ahead and pushed a commit here. The `AUTH_LDAP` exclusion was hiding recaptcha entirely, which broke the DB/LDAP test already on master (it expects recaptcha to show when a key IS configured). So instead I reverted that exclusion and made the `RECAPTCHA_PUBLIC_KEY` lookup itself safe with `.get()`, which is really what #37008 and #39364 were asking for. Also added a regression test for the missing-key case, since that had no coverage before.
Should be green now, will keep an eye on CI.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-3728148367
CodeAnt AI is reviewing your PR.
---
### Thanks for using CodeAnt! π
We're free for open-source projects. if you're enjoying it, help us grow by sharing.
[Share on X](https://twitter.com/intent/tweet?text=Just%20tried%20%40CodeAntAI%20for%20automated%20code%20review%20and%20I%27m%20impressed%21%20Free%20for%20open%20source%20with%20a%20free%20trial%20for%20private%20repos.%20Worth%20checking%20out%3A&url=https%3A//codeant.ai) Β·
[Reddit](https://www.reddit.com/submit?title=Check%20out%20CodeAnt%20for%20automated%20code%20review&text=Just%20tried%20CodeAnt%20for%20automated%20code%20review%20and%20I%27m%20impressed%21%20Free%20for%20open%20source%20with%20a%20free%20trial%20for%20private%20repos.%20Worth%20checking%20out%3A%20https%3A//codeant.ai) Β·
[LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcodeant.ai&mini=true&title=Check%20out%20CodeAnt%20for%20automated%20code%20review&summary=Just%20tried%20CodeAnt%20for%20automated%20code%20review%20and%20I%27m%20impressed%21%20Free%20for%20open%20source%20with%20a%20free%20trial%20for%20private%20repos)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-4817430715
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-4908140182
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #37009:
URL: https://github.com/apache/superset/pull/37009#discussion_r2675545370
##########
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 not in [AUTH_OAUTH, AUTH_LDAP])
Review Comment:
**Suggestion:** Type-safety and missing-constant handling: the one-line expression computing `should_show_recaptcha` assumes `AUTH_OAUTH` and `AUTH_LDAP` are present and that `auth_type`'s type matches those constants; normalize and guard comparisons so missing constants or mismatched types don't produce incorrect results. [logic error]
**Severity Level:** Minor β οΈ
```suggestion
# Build a tuple of available auth constants and perform a guarded comparison.
_auth_constants = tuple(c for c in (AUTH_OAUTH, AUTH_LDAP) if c is not None)
if _auth_constants:
should_show_recaptcha = bool(auth_user_registration) and (auth_type not in _auth_constants)
else:
# If constants aren't available, fall back to only checking registration flag
should_show_recaptcha = bool(auth_user_registration)
```
<details>
<summary><b>Why it matters? β </b></summary>
If you apply the import fallback above and AUTH_OAUTH/AUTH_LDAP can be None, this one-liner silently relies on those values being valid. Building the list/tuple of only-present constants and guarding the comparison is safer and avoids surprising results when constants are absent or of unexpected types.
</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:**
*Logic Error: Type-safety and missing-constant handling: the one-line expression computing `should_show_recaptcha` assumes `AUTH_OAUTH` and `AUTH_LDAP` are present and that `auth_type`'s type matches those constants; normalize and guard comparisons so missing constants or mismatched types don't produce incorrect results.
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>
##########
superset/views/base.py:
##########
@@ -35,7 +35,7 @@
)
from flask_appbuilder import BaseView, Model, ModelView
from flask_appbuilder.actions import action
-from flask_appbuilder.const import AUTH_OAUTH
+from flask_appbuilder.const import AUTH_LDAP, AUTH_OAUTH
Review Comment:
**Suggestion:** Import failure risk: directly importing `AUTH_LDAP` and `AUTH_OAUTH` from `flask_appbuilder.const` can raise an ImportError or AttributeError on different Flask-AppBuilder versions where these names may not be exported; perform a robust import with a fallback to avoid breaking application start-up. [possible bug]
**Severity Level:** Critical π¨
```suggestion
try:
from flask_appbuilder.const import AUTH_LDAP, AUTH_OAUTH
except Exception:
# Fallback to tolerate FAB versions that don't export these names directly.
# Use getattr on the module to avoid ImportError/AttributeError at import time.
from flask_appbuilder import const as _fab_const
AUTH_OAUTH = getattr(_fab_const, "AUTH_OAUTH", None)
AUTH_LDAP = getattr(_fab_const, "AUTH_LDAP", None)
```
<details>
<summary><b>Why it matters? β </b></summary>
This is a reasonable defensive change: if a downstream environment uses a FAB
variation that doesn't export those names directly, the current import would
raise at import time and crash the app. The proposed fallback using getattr
prevents that. The suggestion is actionable and fixes a real compatibility
failure mode.
</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:** 38:38
**Comment:**
*Possible Bug: Import failure risk: directly importing `AUTH_LDAP` and `AUTH_OAUTH` from `flask_appbuilder.const` can raise an ImportError or AttributeError on different Flask-AppBuilder versions where these names may not be exported; perform a robust import with a fallback to avoid breaking application start-up.
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-3728192298
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/37009?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 48.29%. Comparing base ([`413dfc9`](https://app.codecov.io/gh/apache/superset/commit/413dfc98ff17e4fafbdcbbe639925160cfd5bb14?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`af03eca`](https://app.codecov.io/gh/apache/superset/commit/af03eca9e948405e7daa686f4b61ce3effb0231c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #37009 +/- ##
===========================================
+ Coverage 0 48.29% +48.29%
===========================================
Files 0 642 +642
Lines 0 47885 +47885
Branches 0 5227 +5227
===========================================
+ Hits 0 23125 +23125
- Misses 0 24104 +24104
- Partials 0 656 +656
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/37009/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/37009/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `42.92% <100.00%> (?)` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/37009/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.50% <100.00%> (?)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/37009/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.28% <100.00%> (?)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/37009/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ΓΈ> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/37009?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-3728157185
## Nitpicks π
<table>
<tr><td>π <strong>No security issues identified</strong></td></tr>
<tr><td>β‘ <strong>Recommended areas for review</strong><br><br>
- [ ] <a href='https://github.com/apache/superset/pull/37009/files#diff-8a676fb002920d1d142fa9b19ded1ecb7b8b8d682cf9b9fe80f01b1a2314c86dR38-R38'><strong>Import risk</strong></a><br>The PR adds `AUTH_LDAP` to the top-level import from `flask_appbuilder.const`. If the installed Flask-AppBuilder version does not expose `AUTH_LDAP` at that location, importing it will raise ImportError at import time and cause the application to fail to start. Consider making the import robust or providing a fallback so older FAB versions remain compatible.<br>
</td></tr>
</table>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-3728148790
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #37009:
URL: https://github.com/apache/superset/pull/37009#discussion_r3556180675
##########
superset/views/base.py:
##########
@@ -525,8 +525,9 @@ def cached_common_bootstrap_data( # pylint: disable=unused-argument
frontend_config["AUTH_USER_REGISTRATION_ROLE"] = app.config[
"AUTH_USER_REGISTRATION_ROLE"
]
- if should_show_recaptcha:
- frontend_config["RECAPTCHA_PUBLIC_KEY"] = app.config["RECAPTCHA_PUBLIC_KEY"]
+ recaptcha_public_key = app.config.get("RECAPTCHA_PUBLIC_KEY")
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced local variable to satisfy the type-hint requirement. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The new local variable is introduced without a type annotation even though its type is inferable (`str | None` from `app.config.get(...)`). This matches the Python type-hint rule for newly added code that can be annotated.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a3795abb3d734fd0862a725615c57be3&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=a3795abb3d734fd0862a725615c57be3&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/views/base.py
**Line:** 528:528
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced local variable to satisfy the type-hint requirement.
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%2F37009&comment_hash=4f2742d5a7364372ded518d49925ac0b8d945007613053aba4c93358cc13f1a6&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37009&comment_hash=4f2742d5a7364372ded518d49925ac0b8d945007613053aba4c93358cc13f1a6&reaction=dislike'>π</a>
##########
tests/unit_tests/views/test_bootstrap_auth.py:
##########
@@ -135,6 +135,27 @@ def test_recaptcha_shown_for_non_federated_auth(
assert payload["conf"]["RECAPTCHA_PUBLIC_KEY"] == "test-key"
+@pytest.mark.parametrize(
+ "auth_type",
+ [AUTH_DB, AUTH_LDAP],
+)
+def test_bootstrap_does_not_crash_without_recaptcha_key(
+ app_context: None,
+ auth_type: int,
+) -> None:
+ """Missing RECAPTCHA_PUBLIC_KEY must not crash bootstrap (#37008/#39364)."""
+ from flask import current_app
+
+ current_app.config["AUTH_TYPE"] = auth_type
+ current_app.config["AUTH_USER_REGISTRATION"] = True
+ current_app.config["AUTH_USER_REGISTRATION_ROLE"] = "Public"
+ current_app.config.pop("RECAPTCHA_PUBLIC_KEY", None)
+
+ payload = _get_bootstrap()
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced local variable to satisfy the type-hint requirement for relevant variables. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The new test introduces a local variable whose type is clearly inferable and can be annotated, but it is left untyped. This matches the Python type-hint rule for relevant variables.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=17e362db53c149aea75cb02a1ee63b34&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=17e362db53c149aea75cb02a1ee63b34&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/unit_tests/views/test_bootstrap_auth.py
**Line:** 154:154
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced local variable to satisfy the type-hint requirement for relevant variables.
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%2F37009&comment_hash=f1fd2d7132413e8ab8c9f142aadd0bba4c09c9bd7083b27d761590e318ad766d&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37009&comment_hash=f1fd2d7132413e8ab8c9f142aadd0bba4c09c9bd7083b27d761590e318ad766d&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-4885086138
Thanks for the quick rebase @schoel-bis, LGTM. The unit-test failure looks unrelated to this change (mcp tests), so I'll kick CI again and hopefully we can merge. There are also a couple of old bot comments above to assess/address/resolve if you don't mind.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "sonfire186 (via GitHub)" <gi...@apache.org>.
sonfire186 commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-4604436804
!up
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-3728162667
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | af03eca9e948405e7daa686f4b61ce3effb0231c |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6960cf653aed4a000845a28b |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-37009--superset-docs-preview.netlify.app](https://deploy-preview-37009--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-37009--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-3728161543
CodeAnt AI finished reviewing your PR.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-3728824900
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: do not require recaptcha public key for LDAP authentication [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #37009:
URL: https://github.com/apache/superset/pull/37009#issuecomment-4815468165
Just needs a rebase to clear the conflict in `base.py` β I'll see if I can take care of that and get it green, then review.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org