Skip to content

Conversation

@Khrol
Copy link
Contributor

@Khrol Khrol commented Dec 29, 2025

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_KEY but it's more a hack than a solution.

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

CodeAnt-AI Description

Disable reCAPTCHA when using LDAP authentication to avoid startup failures

What Changed

  • The registration UI no longer requires or exposes the reCAPTCHA key when AUTH_TYPE is LDAP
  • The app will not attempt to read or send RECAPTCHA_PUBLIC_KEY to the frontend for LDAP-based setups
  • Users with LDAP-only auth no longer hit initialization errors if RECAPTCHA_PUBLIC_KEY is missing

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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

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.

@Khrol Khrol changed the title Disable recaptcha for LDAP authentication fix: disable recaptcha for LDAP authentication Dec 29, 2025
@codecov
Copy link

codecov bot commented Dec 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.20%. Comparing base (76d897e) to head (4a876a5).
⚠️ Report is 3233 commits behind head on master.

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     
Flag Coverage Δ
hive 43.13% <100.00%> (-6.03%) ⬇️
javascript ?
mysql 66.21% <100.00%> (?)
postgres 66.26% <100.00%> (?)
presto 46.73% <100.00%> (-7.07%) ⬇️
python 68.17% <100.00%> (+4.67%) ⬆️
sqlite 65.97% <100.00%> (?)
unit 100.00% <ø> (+42.36%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Khrol Khrol marked this pull request as ready for review December 30, 2025 06:38
@dosubot dosubot bot added the authentication Related to authentication label Dec 30, 2025
@codeant-ai-for-open-source
Copy link
Contributor

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 ·
Reddit ·
LinkedIn

@codeant-ai-for-open-source codeant-ai-for-open-source bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Dec 30, 2025
@codeant-ai-for-open-source
Copy link
Contributor

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Compatibility risk
    Adding AUTH_LDAP to the import may break installations that use older
    versions of Flask-AppBuilder where AUTH_LDAP is not defined. This can
    raise ImportError at startup. Consider guarding the import or providing
    a fallback to preserve compatibility.

  • Potential KeyError
    The code still uses app.config["RECAPTCHA_PUBLIC_KEY"] when
    should_show_recaptcha is True. If the key is missing from config this
    will raise a KeyError at runtime. Consider using app.config.get(...)
    or a safe check prior to reading the value.

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)
Copy link
Contributor

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 🚨

Suggested change
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.

Copy link
Contributor Author

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-for-open-source
Copy link
Contributor

CodeAnt AI finished reviewing your PR.

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Dec 30, 2025

Code Review Agent Run #06b750

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: e10dfce..fb8b1d0
    • superset/views/base.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

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-for-open-source
Copy link
Contributor

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 ·
Reddit ·
LinkedIn

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Dec 30, 2025

Code Review Agent Run #c93fc4

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 4a876a5..4a876a5
    • superset/views/base.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

authentication Related to authentication size/XS size:XS This PR changes 0-9 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant