feat(Login): Migrate login page to frontend by msyavuz · Pull Request #33255 · apache/superset

msyavuz · 2025-04-28T10:38:22Z

SUMMARY

This PR tries to move the Login fab view to the frontend spa.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

AuthDB Login:

Before:

After:

TESTING INSTRUCTIONS

Follow the instructions on setting up auth methods in flask-appbuilder to see different auth methods working. The configuration should be something like this for different auth methods:

AUTH_TYPE = AUTH_OAUTH
OAUTH_PROVIDERS = [
    {
        "name": "google",
        "icon": "fa-google",
        "token_key": "access_token",
        "remote_app": {
            "client_id": GOOGLE_KEY,
            "client_secret": GOOGLE_SECRET,
            "api_base_url": "https://www.googleapis.com/oauth2/v2/",
            "client_kwargs": {"scope": "email profile"},
            "request_token_url": None,
            "access_token_url": "https://accounts.google.com/o/oauth2/token",
            "authorize_url": "https://accounts.google.com/o/oauth2/auth",
            "project_id": "symbolic-path-413013",
            "auth_uri": "https://accounts.google.com/o/oauth2/auth",
            "token_uri": "https://oauth2.googleapis.com/token",
            "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
            "redirect_uris": ["http://localhost:8088/oauth-authorized/google"],
            "javascript_origins": ["http://localhost:8088"],
            "jwks_uri": "https://www.googleapis.com/oauth2/v3/certs",
        },
    },
]

AUTH_TYPE = AUTH_OID
OPENID_PROVIDERS = [
    {"name": "Yahoo", "url": "https://me.yahoo.com"},
    {"name": "AOL", "url": "http://openid.aol.com/<username>"},
    {"name": "Flickr", "url": "http://www.flickr.com/<username>"},
    {"name": "MyOpenID", "url": "https://www.myopenid.com"},
]

AUTH_TYPE = AUTH_LDAP
AUTH_LDAP_SERVER = "ldap://ldap.example.org"
AUTH_LDAP_USE_TLS = False

# registration configs
AUTH_USER_REGISTRATION = True  # allow users who are not already in the FAB DB
AUTH_USER_REGISTRATION_ROLE = (
    "Public"  # this role will be given in addition to any AUTH_ROLES_MAPPING
)
AUTH_LDAP_FIRSTNAME_FIELD = "givenName"
AUTH_LDAP_LASTNAME_FIELD = "sn"
AUTH_LDAP_EMAIL_FIELD = (
    "mail"  # if null in LDAP, email is set to: "{username}@email.notfound"
)

# search configs
AUTH_LDAP_SEARCH = "ou=users,dc=example,dc=org"  # the LDAP search base
AUTH_LDAP_UID_FIELD = "uid"  # the username field
AUTH_LDAP_BIND_USER = (
    "uid=admin,dc=example,dc=org"  # the special bind username for search
)
AUTH_LDAP_BIND_PASSWORD = "admin_password"  # the special bind password for search

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

korbit-ai · 2025-04-28T10:38:26Z

Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment /korbit-review.

Your admin can change your review schedule in the Korbit Console

rusackas · 2025-04-29T14:54:21Z

Yay!!!! Is this the magic moment we've all been waiting for that kills the little gray close buttons that show up before the React frontend is built/loaded?

EnxDev

Thanks!

superset-frontend/src/pages/Login/index.tsx

EnxDev · 2025-05-08T15:36:17Z

superset-frontend/src/pages/Login/index.tsx

+                  { required: true, message: t('Please enter your username') },
+                ]}
+              >
+                <Input prefix={<Icons.UserOutlined size={1} />} />


Should it be one of these: 'xs', 's', 'm', 'l', 'xl', 'xxl'?

msyavuz · 2025-05-08T17:29:30Z

Yay!!!! Is this the magic moment we've all been waiting for that kills the little gray close buttons that show up before the React frontend is built/loaded?

Well, i think those will show up anyways until the frontend is loaded. Sorry!

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Unused OAuth Provider Parameter ▹ view	🧠 Incorrect
	Unmemoized Bootstrap Data Fetch ▹ view	🧠 Incorrect
	Form Submission Logic Not Separated ▹ view	🧠 Not in standard
	Incorrect Component for Logout Route ▹ view
	Unhandled URL Generation Error in Redirect ▹ view	🧠 Incorrect
	Missing Interface Documentation ▹ view	🧠 Not in standard
	Potential Circular Import Risk ▹ view	✅ Fix detected
	Unexplained Layout Constants ▹ view	🧠 Not in standard
	Unclear API Parameter ▹ view	🧠 Incorrect
	Unmemoized Form Handler ▹ view	🧠 Incorrect

Files scanned

File Path	Reviewed
superset/connectors/sqla/init.py	✅
superset-frontend/cypress-base/cypress/utils/urls.ts	✅
superset/views/alerts.py	✅
superset/views/auth.py	✅
superset-frontend/src/components/Icons/AntdEnhanced.tsx	✅
superset-frontend/src/views/routes.tsx	✅
superset-frontend/src/pages/Register/index.tsx	✅
superset-frontend/src/pages/Login/index.tsx	✅
superset-frontend/cypress-base/cypress/support/e2e.ts	✅
superset/views/base.py	✅
superset/views/utils.py	✅
superset/initialization/init.py	✅
superset/security/manager.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

superset/views/alerts.py

    @has_access
    @permission_name("read")
    def list(self) -> FlaskResponse:
+        from superset import is_feature_enabled


korbit-ai · 2025-05-20T10:10:35Z

superset-frontend/src/views/routes.tsx

+  {
+    path: '/logout/',
+    Component: Login,
+  },


Incorrect Component for Logout Route

Tell me more

What is the issue?

The logout route is incorrectly using the Login component, which doesn't handle the logout functionality.

Why this matters

Users attempting to logout will be shown the login page without their session being terminated, leading to an inconsistent authentication state.

Suggested change ∙ Feature Preview

Create and use a dedicated Logout component that handles session termination before redirecting to the login page. Example:

const Logout = lazy( () => import(/* webpackChunkName: "Logout" */ 'src/pages/Logout'), ); // In routes array { path: '/logout/', Component: Logout, }

Provide feedback to improve future suggestions

_{💬 Looking for more details? Reply to this comment to chat with Korbit.}

superset/views/auth.py

+    @no_cache
+    def login(self, provider: Optional[str] = None) -> WerkzeugResponse:
+        if g.user is not None and g.user.is_authenticated:
+            return redirect(self.appbuilder.get_url_for_index)


superset-frontend/src/pages/Register/index.tsx

+      conf_password: values.confirmPassword,
+      'g-recaptcha-response': captchaResponse,
+    };
+    SupersetClient.postForm('/register/form', payload, '').finally(() => {


superset/views/auth.py

+
+    @expose("/")
+    @no_cache
+    def login(self, provider: Optional[str] = None) -> WerkzeugResponse:


superset-frontend/src/pages/Register/index.tsx

+  const [loading, setLoading] = useState(false);
+  const [captchaResponse, setCaptchaResponse] = useState<string | null>(null);
+
+  const bootstrapData = getBootstrapData();


superset-frontend/src/pages/Register/index.tsx

+  const onFinish = (values: RegisterForm) => {
+    setLoading(true);
+    const payload = {
+      username: values.username,
+      first_name: values.firstName,
+      last_name: values.lastName,
+      email: values.email,
+      password: values.password,
+      conf_password: values.confirmPassword,
+      'g-recaptcha-response': captchaResponse,
+    };


superset-frontend/src/pages/Register/index.tsx

+  const onFinish = (values: RegisterForm) => {
+    setLoading(true);
+    const payload = {
+      username: values.username,
+      first_name: values.firstName,
+      last_name: values.lastName,
+      email: values.email,
+      password: values.password,
+      conf_password: values.confirmPassword,
+      'g-recaptcha-response': captchaResponse,
+    };
+    SupersetClient.postForm('/register/form', payload, '').finally(() => {
+      setLoading(false);
+    });


superset-frontend/src/pages/Register/index.tsx

+interface RegisterForm {
+  username: string;
+  firstName: string;
+  lastName: string;
+  email: string;
+  password: string;
+  confirmPassword: string;
+}


superset-frontend/src/pages/Register/index.tsx

+const formItemLayout = {
+  labelCol: {
+    xs: { span: 24 },
+    sm: { span: 6 },
+  },
+  wrapperCol: {
+    xs: { span: 24 },
+    sm: { span: 24 },
+  },
+};


geido · 2025-05-20T14:06:26Z

superset-frontend/cypress-base/cypress/support/e2e.ts

 );

 before(() => {
+  if (Cypress.currentTest.title.toLowerCase().includes('login')) {


Can we make a small util for this?

geido · 2025-05-20T14:11:28Z

superset-frontend/src/pages/Login/index.tsx

+                    href={`/login/${provider.name}`}
+                    block
+                    iconPosition="start"
+                    icon={AuthIconMap[provider.name]}


Can new providers be introduced? What happens if the icon isn't available?

Since we can get the icon from user config as 'fa-google' etc, I can create an icon map that has all available providers, and maybe we can have fallback to <i> tag for new providers?

It seems that there is a pattern in naming convention for social icons on Ant Design. We can make a util function that gets the provider name, capitalizes it and adds either Outlined or Filled (to check with Kasia) and if it does not exist can fallback to a button or a fallback icon.

geido · 2025-05-20T14:12:56Z

superset-frontend/src/pages/Login/index.tsx

+                />
+              </Form.Item>
+              <Form.Item label={null}>
+                <Flex


You have a LoginContainer that is the same as this. I prefer inline, so maybe we can get rid of LoginContainer?

mistercrunch · 2025-05-20T23:09:49Z

YES! should the login box be vertical-algin: center;?

mistercrunch · 2025-05-21T20:39:50Z

While at it, wondering if we can reduce the width of the login card, and maybe remove the icon at the top-right, the word "Log in" speaks for itself, no need for an icon here. @kasiazjc may have some input here too.

alexandrusoare · 2025-05-22T08:45:17Z

superset-frontend/src/pages/Login/index.tsx

+
+const StyledCard = styled(Card)`
+  ${({ theme }) => css`
+    width: 40%;


I would avoid using percentages as I don't think it would look great on wide screens, let me know what you think

alexandrusoare · 2025-05-22T08:57:34Z

superset-frontend/src/pages/Register/index.tsx

+            label={t('Email')}
+            name="email"
+            rules={[{ required: true, message: t('Please enter your email') }]}
+          >


Suggested change

>

>rules={[

{ required: true, message: t('Email is required') },

{

type: 'email',

message: t('Please enter a valid email address'),

},

]}

So that it can check if t s valid format for the email adress as well

alexandrusoare · 2025-05-22T09:00:27Z

superset/views/utils.py

-    elif security_manager.is_guest_user(user):
+        user.roles = (current_app.appbuilder.sm.find_role("Public"),)
+    elif current_app.appbuilder.sm.is_guest_user(user):
        payload = {


Why do you need to use the current_app.app_builder.sm instead of security_manager

You are correct, these were measures i took to avoid circular imports but they are not necessary anymore

alexandrusoare · 2025-05-22T09:02:49Z

Great work @msyavuz ! The page is looking good, I just have a few nits here and there.

When you hover over the login and register buttons there's a slight difference, the register text gets underlined, maybe we can get rid of that:

I think we should either reduce a bit the height of the login card or to give a little bit more padding/margin to buttons on the top so that we can have some better proportions, let me know what you think:

Maybe center the inputs and labels or at least place them on the left

dpgaspar

Tested AUTH_DB and AUTH_OAUTH,
using OAUTH and AUTH_USER_REGISTRATION = True login breaks, it seems RECAPTCHA_PUBLIC_KEY is now required, although this is a good measure it was not a required config before.
This does not seem specifically related with this PR, but a possible issue

dpgaspar · 2025-05-26T09:25:05Z

superset/views/alerts.py

    @has_access
    @permission_name("read")
    def log(self, pk: int) -> FlaskResponse:  # pylint: disable=unused-argument
+        from superset import is_feature_enabled


nit: is it possible to move this import to the module level?

msyavuz · 2025-05-26T12:07:34Z

using OAUTH and AUTH_USER_REGISTRATION = True login breaks, it seems RECAPTCHA_PUBLIC_KEY is now required, although this is a good measure it was not a required config before.

@dpgaspar I think it was required when registration is enabled but it was implicit, Recaptcha would be broken and you couldn't register. I am not sure what was the intended flow with fab as https://flask-appbuilder.readthedocs.io/en/latest/user_registration.html doesn't mention oauth.

msyavuz · 2025-05-29T09:21:37Z

When you hover over the login and register buttons there's a slight difference, the register text gets underlined, maybe we can get rid of that:

That button is technically a link so not sure about removing the underline

I think we should either reduce a bit the height of the login card or to give a little bit more padding/margin to buttons on the top so that we can have some better proportions, let me know what you think:

I think looks pretty similar to the fab view we had now, we would again need a design input for this if we want to change more stuff.

Maybe center the inputs and labels or at least place them on the left

Thought about maybe having less padding on the left but decided to go with less custom styles

msyavuz · 2025-05-29T09:24:52Z

YES! should the login box be vertical-algin: center;?

While at it, wondering if we can reduce the width of the login card, and maybe remove the icon at the top-right, the word "Log in" speaks for itself, no need for an icon here. @kasiazjc may have some input here too.

I think input of @kasiazjc is relevant here. I tried to replicate what it was before but if want to change these stuff we can do that as well.

msyavuz added 2 commits April 23, 2025 22:46

chore: initial commit

22c7e8a

feat: add oauth views to login

78f4b89

pull-request-size bot added the size/L label Apr 28, 2025

github-actions bot added the api Related to the REST API label Apr 28, 2025

msyavuz changed the title ~~chore: initial commit~~ feat(Login): Migrate login page to frontend Apr 28, 2025

rusackas added the review:draft label Apr 28, 2025

msyavuz added 2 commits April 28, 2025 21:43

feat: somewhat working version

8befd87

feat(Login): oauth login and providers

b3cd4b8

refactor: use login logout routes

bfeeff5

github-actions bot removed the api Related to the REST API label Apr 29, 2025

msyavuz added 2 commits April 30, 2025 14:24

fix: remove logout view and check oauth

65fab15

fix: link for oauth

f54c36a

msyavuz requested a review from EnxDev May 8, 2025 10:09

EnxDev reviewed May 8, 2025

View reviewed changes

msyavuz added 13 commits May 9, 2025 10:44

fix: remove variant

60d12de

Merge branch 'template_less' into msyavuz/feat/login-view-to-react

32f6a8a

fix: correct imports

50e423a

fix: unused code

a79e08b

feat(Oid): migrate OID

d1c2e75

feat: LDAP and icons

bd2fb85

fix(Login): style

4210340

fix: working oauth

57914b4

Merge branch 'template_less' into msyavuz/feat/login-view-to-react

f45f013

fix(Login): button styles

8c97c27

Merge branch 'template_less' into msyavuz/feat/login-view-to-react

9c3a83d

fix: correct imports

c8e7126

chore: icon size

22fa679

msyavuz requested review from betodealmeida, eschutho, geido, mistercrunch and rusackas as code owners May 20, 2025 10:01

dosubot bot added the change:frontend Requires changing the frontend label May 20, 2025

korbit-ai bot reviewed May 20, 2025

View reviewed changes

geido reviewed May 20, 2025

View reviewed changes

msyavuz added 3 commits May 20, 2025 17:20

fix(Login): trailing slash

98827ef

fix(Login): attend reviews

7d6bd29

fix(Register): recaptcha fallback

c99cb34

rusackas removed the review:draft label May 20, 2025

msyavuz added 2 commits May 21, 2025 11:05

fix(Login): check cookies instead

b8e25a5

fix(Register): don't login and don't show captcha

45d8a51

alexandrusoare reviewed May 22, 2025

View reviewed changes

dpgaspar reviewed May 26, 2025

View reviewed changes

msyavuz added 3 commits May 26, 2025 15:16

feat(Login): import icon dynamically

c2877bf

fix(Register): add check to show recaptcha

1afecfa

refactor(Alerts): module level import

7df201d

msyavuz added 2 commits May 29, 2025 12:29

feat(Login): check valid email as well

b300d81

fix: revert unnecessary workaround

8e4a4ca

alexandrusoare approved these changes May 29, 2025

View reviewed changes

msyavuz merged commit 58435e3 into template_less May 29, 2025
47 checks passed

msyavuz deleted the msyavuz/feat/login-view-to-react branch May 29, 2025 11:22

-          >
+          >rules={[
+              { required: true, message: t('Email is required') },
+              {
+                type: 'email',
+                message: t('Please enter a valid email address'),
+              },
+            ]}

Conversation

msyavuz commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot commented Apr 28, 2025

Uh oh!

rusackas commented Apr 29, 2025

Uh oh!

EnxDev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msyavuz commented May 8, 2025

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Uh oh!

This comment was marked as resolved.

Uh oh!

korbit-ai bot May 20, 2025

Choose a reason for hiding this comment

Incorrect Component for Logout Route

What is the issue?

Why this matters

Suggested change ∙ Feature Preview

Provide feedback to improve future suggestions

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mistercrunch commented May 20, 2025

Uh oh!

mistercrunch commented May 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

msyavuz commented Apr 28, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading