-
Notifications
You must be signed in to change notification settings - Fork 16.4k
fix: disable recaptcha for LDAP authentication #36857
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #36857 +/- ##
==========================================
+ Coverage 60.48% 68.20% +7.72%
==========================================
Files 1931 639 -1292
Lines 76236 47602 -28634
Branches 8568 5195 -3373
==========================================
- Hits 46114 32469 -13645
+ Misses 28017 13853 -14164
+ Partials 2105 1280 -825
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
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 · |
Nitpicks 🔍
|
superset/views/base.py
Outdated
| 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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 🚨
| should_show_recaptcha = auth_user_registration and (auth_type != AUTH_OAUTH) and (auth_type != AUTH_LDAP) | |
| 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")) | |
| ) |
Why it matters? ⭐
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.
Prompt for AI Agent 🤖
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this is the exact problem I try to fix. I'm not sure if it's fine to define should_show_recaptcha logic based on RECAPTCHA_PUBLIC_KEY presence. I'll leave it for reviewers.
|
CodeAnt AI finished reviewing your PR. |
Code Review Agent Run #06b750Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Init process fails without RECAPTCHA_PUBLIC_KEY when AUTH_LDAP or AUTH_OAUTH is used. This change disables reCAPTCHA when LDAP or OAuth authentication is configured, preventing initialization failures that would otherwise require setting an empty RECAPTCHA_PUBLIC_KEY as a workaround.
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Code Review Agent Run #c93fc4Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
User description
Init process fails without RECAPTCHA_PUBLIC_KEY if AUTH_LDAP is used.
The problem can be solved by setting an empty
RECAPTCHA_PUBLIC_KEYbut it's more a hack than a solution.SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
CodeAnt-AI Description
Disable reCAPTCHA when using LDAP authentication to avoid startup failures
What Changed
Impact
✅ Prevents init failures for LDAP authentication✅ Fewer setup errors when RECAPTCHA_PUBLIC_KEY is not set✅ Clearer registration UI for LDAP deployments💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.