You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sadpandajoe (via GitHub)" <gi...@apache.org> on 2026/09/17 03:01:19 UTC
[PR] fix(auth): gate registration UI by auth type, not just the flag [superset]
sadpandajoe opened a new pull request, #44368:
URL: https://github.com/apache/superset/pull/44368
### SUMMARY
LDAP, SAML and `AUTH_REMOTE_USER` deployments have to set
`AUTH_USER_REGISTRATION = True` — Flask-AppBuilder refuses to provision a user on
first login without it (`auth_user_ldap`: `if (not user) and (not self.auth_user_registration): return None`),
and our own docs tell operators to set it. Superset gated its self-registration UI
on that flag alone, with no auth-type check, in two places:
- `superset/security/manager.py` registered `SupersetRegisterUserView` (the public,
undecorated `GET /register/` SPA page) whenever the `register_superset_registeruser_view`
class attribute was set — never consulting `auth_user_registration` or `auth_type`.
- `superset-frontend/src/pages/Login/index.tsx` rendered the "Register" button on
`{authRegistration && ...}`; the surrounding block covers `AuthDB || AuthLDAP`, so LDAP
logins got a Register button next to the login form.
So enabling the flag purely for first-login provisioning published what looks like open
registration — and it could not work: Flask-AppBuilder only instantiates a register-user
view (and with it the `PublicFormView` `/register/form` POST handler the page submits to)
for `AUTH_DB` and `AUTH_OAUTH`, so on LDAP the form navigated the user to a 404. This is a
regression from the SPA login rewrite (#31590): FAB's own `login_ldap.html` has no register
link at all, while `login_db.html` gates one on `auth_user_registration`.
This mirrors Flask-AppBuilder's own `register_views` gate: register the page only for the
auth types that offer self-registration, and only when `AUTH_USER_REGISTRATION` is enabled;
show the login-page button only for `AUTH_DB`. First-login provisioning is untouched.
Out of scope on purpose: splitting `AUTH_USER_REGISTRATION` into two flags (a maintainer
back-compat call), and the OAuth `/register/form` POST behaviour, which is Flask-AppBuilder's
documented upstream behaviour rather than a defect.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no running Superset available to capture; covered by unit tests instead.
### TESTING INSTRUCTIONS
Automated:
- `pytest tests/unit_tests/security/manager_test.py -k register_views`
- `cd superset-frontend && npm run test -- src/pages/Login/Login.test.tsx`
Manual, with `AUTH_TYPE = AUTH_LDAP` and `AUTH_USER_REGISTRATION = True`:
1. Load `/login/` — no "Register" button next to "Sign in" (it used to appear).
2. Visit `/register/` directly — 404 (it used to render a username/password
registration form whose submit navigated to a 404).
3. Log in with an LDAP user that has no Superset row — the user is still
provisioned on first login with `AUTH_USER_REGISTRATION_ROLE`.
4. With `AUTH_TYPE = AUTH_DB` and `AUTH_USER_REGISTRATION = True`, `/register/`
and the Register button still work exactly as before.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #37100
- [ ] Required feature flags:
- [x] 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(auth): gate registration UI by auth type, not just the flag [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44368:
URL: https://github.com/apache/superset/pull/44368#issuecomment-5759612707
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 7a5d61fa13ebf2294fbfaea8e3eddd252def2387 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ab111403872a100088b0c37 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44368--superset-docs-preview.netlify.app](https://deploy-preview-44368--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> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84AsktzSooSi1NLdNMyK3SNzQ0NDICyKakFOfmVnilAebPEJENDQxMDYwtzo0SgrIGFRZJBsrG5Ui0A) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44368--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(auth): gate registration UI by auth type, not just the flag [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44368:
URL: https://github.com/apache/superset/pull/44368#issuecomment-5721244219
<!-- Bito Reply -->
The inclusion of `AUTH_OAUTH` in the registration guard is deliberate. The PR addresses the issue where the self-registration page (`/register/`) was exposed for auth types that do not support self-registration (like LDAP, SAML, and `AUTH_REMOTE_USER`), even when `AUTH_USER_REGISTRATION` was enabled for first-login provisioning.
By restricting the registration view to `AUTH_DB` and `AUTH_OAUTH` (as seen in `superset/security/manager.py`), the change ensures that the registration page is only served when the underlying auth mechanism actually supports the registration form handler, preventing 404 errors for other auth types.
**superset/security/manager.py**
```
if (
self.register_superset_registeruser_view
and self.auth_user_registration
and self.auth_type in (AUTH_DB, AUTH_OAUTH)
):
self.registeruser_view = self.appbuilder.add_view_no_menu(
SupersetRegisterUserView
)
```
--
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(auth): gate registration UI by auth type, not just the flag [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #44368:
URL: https://github.com/apache/superset/pull/44368
--
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(auth): gate registration UI by auth type, not just the flag [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44368:
URL: https://github.com/apache/superset/pull/44368#issuecomment-5707811596
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(auth): gate registration UI by auth type, not just the flag [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44368:
URL: https://github.com/apache/superset/pull/44368#issuecomment-5707844537
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44368?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `50.00000%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.37%. Comparing base ([`4c77b0b`](https://app.codecov.io/gh/apache/superset/commit/4c77b0b924a9bd770110915eacd68f5619e8241d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ddc4e69`](https://app.codecov.io/gh/apache/superset/commit/ddc4e69bbf171c29c94b20892a97a6d1a6b4e3ec?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/44368?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/security/manager.py](https://app.codecov.io/gh/apache/superset/pull/44368?src=pr&el=tree&filepath=superset%2Fsecurity%2Fmanager.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | 50.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/superset/pull/44368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (4c77b0b) and HEAD (ddc4e69). Click for more details.
>
> <details><summary>HEAD has 69 uploads less than BASE</summary>
>
>| Flag | BASE (4c77b0b) | HEAD (ddc4e69) |
>|------|------|------|
>|python|61|1|
>|presto|10|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44368 +/- ##
===========================================
- Coverage 80.40% 57.37% -23.04%
===========================================
Files 2932 2932
Lines 174525 174524 -1
Branches 40548 40547 -1
===========================================
- Hits 140319 100125 -40194
- Misses 31537 73444 +41907
+ Partials 2669 955 -1714
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44368/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/44368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/44368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/44368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.22% <50.00%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/44368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.22% <50.00%> (-45.53%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/44368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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 Harness](https://app.codecov.io/gh/apache/superset/pull/44368?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(auth): gate registration UI by auth type, not just the flag [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #44368:
URL: https://github.com/apache/superset/pull/44368#issuecomment-5769776619
@sadpandajoe I should have caught this before I hit approve: @aminghadersohi's question above about OAuth is still open, and it seems like a fair one. FAB forces AUTH_USER_REGISTRATION for OAuth provisioning the same way it does for LDAP, so the AUTH_OAUTH carve-out here might keep exactly the exposure this PR's trying to close. Want to make sure that's deliberate before this actually merges.
Also, pre-commit's failing on a stray trailing-whitespace line in the UPDATING.md addition, easy fix, I'll push a commit.
Babel-extract's red too, but that looks like unrelated pot drift from master (screenshot/retry strings), not this diff.
--
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(auth): gate registration UI by auth type, not just the flag [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #44368:
URL: https://github.com/apache/superset/pull/44368#discussion_r4041449963
##########
superset/security/manager.py:
##########
@@ -6020,7 +6020,17 @@ def register_views(self) -> None:
# FlaskAppBuilder's AuthRemoteUserView actually claims the route.
if self.register_superset_auth_view and self.auth_type != AUTH_REMOTE_USER:
self.auth_view = self.appbuilder.add_view_no_menu(SupersetAuthView)
- if self.register_superset_registeruser_view:
+ # AUTH_USER_REGISTRATION is what makes FlaskAppBuilder provision users on
+ # first login, so LDAP/SAML/AUTH_REMOTE_USER deployments have to enable
+ # it; that must not publish the "/register/" self-registration page.
+ # FlaskAppBuilder only wires the "/register/form" handler that page posts
+ # to for AUTH_DB and AUTH_OAUTH (see its own register_views), so for any
+ # other auth type the page is a registration form that submits to a 404.
+ if (
+ self.register_superset_registeruser_view
+ and self.auth_user_registration
+ and self.auth_type in (AUTH_DB, AUTH_OAUTH)
Review Comment:
FAB forces `AUTH_USER_REGISTRATION` for OAuth provisioning too (`auth_user_oauth` manager.py:1509, same guard as `auth_user_ldap`:1247), and OAuth's `/register/form` does resolve — so it keeps a live unlinked self-registration page. #37100 names OAuth and `Fixes #37100` auto-closes it. Deliberate?
--
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