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: [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]
