fix: Fix SupersetClient.postForm calls

superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts

@@ -27,6 +27,7 @@ import {
   Headers,
   Host,
   Mode,
+  PostFormConfig,
   Protocol,
   RequestConfig,
   ParseMethod,
@@ -115,19 +116,18 @@ export default class SupersetClientClass {
     return this.getCSRFToken();
   }
 
-  async postForm(
-    endpoint: string,
-    payload: Record<string, any>,
-    target = '_blank',
-  ) {
-    if (endpoint) {
+  async postForm(postFormConfig: PostFormConfig) {
+    if (postFormConfig.endpoint || postFormConfig.url) {
       await this.ensureAuth();
       const hiddenForm = document.createElement('form');
-      hiddenForm.action = this.getUrl({ endpoint });
+      hiddenForm.action = this.getUrl({
+        endpoint: postFormConfig.endpoint,
+        url: postFormConfig.url,
+      });
       hiddenForm.method = 'POST';
-      hiddenForm.target = target;
+      hiddenForm.target = postFormConfig.target ?? '_blank';
       const payloadWithToken: Record<string, any> = {
-        ...payload,
+        ...postFormConfig.payload,
         csrf_token: this.csrfToken!,
       };
 

superset-frontend/packages/superset-ui-core/src/connection/types.ts

@@ -117,6 +117,23 @@ export interface RequestWithUrl extends RequestBase {
 // this make sure at least one of `url` or `endpoint` is set
 export type RequestConfig = RequestWithEndpoint | RequestWithUrl;
 
+export interface PostFormBase {
+  payload: Record<string, any>;
+  target?: string;
+}
+
+export interface PostFormWithEndpoint extends PostFormBase {
+  endpoint: Endpoint;
+  url?: Url;
+}
+
+export interface PostFormWithUrl extends PostFormBase {
+  url: Url;
+  endpoint?: Endpoint;
+}
+
+export type PostFormConfig = PostFormWithEndpoint | PostFormWithUrl;
+
 export interface JsonResponse {
   response: Response;
   json: JsonObject;

superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts

@@ -666,7 +666,7 @@ describe('SupersetClientClass', () => {
           authSpy = jest.spyOn(SupersetClientClass.prototype, 'ensureAuth');
           await client.init();
         }
-        await client.postForm(mockPostFormEndpoint, {});
+        await client.postForm({ endpoint: mockPostFormEndpoint, payload: {} });
 
         const hiddenForm = createElement.mock.results[0].value;
         const csrfTokenInput = createElement.mock.results[1].value;
@@ -693,7 +693,7 @@ describe('SupersetClientClass', () => {
       client = new SupersetClientClass({ protocol, host, guestToken });
       await client.init();
 
-      await client.postForm(mockPostFormUrl, {});
+      await client.postForm({ endpoint: mockPostFormUrl, payload: {} });
 
       const guestTokenInput = createElement.mock.results[2].value;
 
@@ -709,7 +709,10 @@ describe('SupersetClientClass', () => {
     });
 
     it('makes postForm request with payload', async () => {
-      await client.postForm(mockPostFormUrl, { form_data: postFormPayload });
+      await client.postForm({
+        url: mockPostFormUrl,
+        payload: { form_data: postFormPayload },
+      });
 
       const postFormPayloadInput = createElement.mock.results[1].value;
 
@@ -726,7 +729,7 @@ describe('SupersetClientClass', () => {
     });
 
     it('should do nothing when url is empty string', async () => {
-      const result = await client.postForm('', {});
+      const result = await client.postForm({ url: '', payload: {} });
       expect(result).toBeUndefined();
       expect(createElement.mock.calls).toHaveLength(0);
       expect(appendChild.mock.calls).toHaveLength(0);

superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/ExploreCtasResultsButton.test.tsx

@@ -68,9 +68,12 @@ describe('ExploreCtasResultsButton', () => {
 
     await waitFor(() => {
       expect(postFormSpy).toHaveBeenCalledTimes(1);
-      expect(postFormSpy).toHaveBeenCalledWith('http://localhost/explore/', {
-        form_data:
-          '{"datasource":"1234__table","metrics":["count"],"groupby":[],"viz_type":"table","since":"100 years ago","all_columns":[],"row_limit":1000}',
+      expect(postFormSpy).toHaveBeenCalledWith({
+        url: 'http://localhost/explore/',
+        payload: {
+          form_data:
+            '{"datasource":"1234__table","metrics":["count"],"groupby":[],"viz_type":"table","since":"100 years ago","all_columns":[],"row_limit":1000}',
+        },
       });
     });
   });

superset-frontend/src/components/Chart/chartAction.js

@@ -41,7 +41,6 @@ import { Logger, LOG_ACTIONS_LOAD_CHART } from 'src/logger/LogUtils';
 import { allowCrossDomain as domainShardingEnabled } from 'src/utils/hostNamesConfig';
 import { updateDataMask } from 'src/dataMask/actions';
 import { waitForAsyncData } from 'src/middleware/asyncEvent';
-import { ensureAppRoot } from 'src/utils/pathUtils';
 import { safeStringify } from 'src/utils/safeStringify';
 import { extendedDayjs } from '@superset-ui/core/utils/dates';
 
@@ -574,8 +573,11 @@ export function redirectSQLLab(formData, history) {
             },
           });
         } else {
-          SupersetClient.postForm(ensureAppRoot(redirectUrl), {
-            form_data: safeStringify(payload),
+          SupersetClient.postForm({
+            endpoint: redirectUrl,
+            payload: {
+              form_data: safeStringify(payload),
+            },
           });
         }
       })

superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx

@@ -261,8 +261,11 @@ class DatasourceControl extends PureComponent {
             datasourceKey: `${datasource.id}__${datasource.type}`,
             sql: datasource.sql,
           };
-          SupersetClient.postForm('/sqllab/', {
-            form_data: safeStringify(payload),
+          SupersetClient.postForm({
+            endpoint: '/sqllab/',
+            payload: {
+              form_data: safeStringify(payload),
+            },
           });
         }
         break;

superset-frontend/src/explore/components/controls/ViewQueryModalFooter.tsx

@@ -55,7 +55,7 @@ const ViewQueryModalFooter: FC<ViewQueryModalFooterProps> = (props: {
       sql,
     };
     if (openInNewWindow) {
-      SupersetClient.postForm('/sqllab/', payload);
+      SupersetClient.postForm({ endpoint: '/sqllab/', payload });
     } else {
       history.push({
         pathname: '/sqllab',

superset-frontend/src/explore/exploreUtils/index.js

@@ -252,6 +252,7 @@ export const exportChart = async ({
   force = false,
   ownState = {},
 }) => {
+  let endpoint;
   let url;
   let payload;
   const [useLegacyApi, parseMethod] = getQuerySettings(formData);
@@ -264,7 +265,7 @@ export const exportChart = async ({
     });
     payload = formData;
   } else {
-    url = ensureAppRoot('/api/v1/chart/data');
+    endpoint = '/api/v1/chart/data';
     payload = await buildV1ChartDataPayload({
       formData,
       force,
@@ -275,7 +276,11 @@ export const exportChart = async ({
     });
   }
 
-  SupersetClient.postForm(url, { form_data: safeStringify(payload) });
+  SupersetClient.postForm({
+    endpoint,
+    url,
+    payload: { form_data: safeStringify(payload) },
+  });
 };
 
 export const exploreChart = (formData, requestParams) => {
@@ -285,7 +290,10 @@ export const exploreChart = (formData, requestParams) => {
     allowDomainSharding: false,
     requestParams,
   });
-  SupersetClient.postForm(url, { form_data: safeStringify(formData) });
+  SupersetClient.postForm({
+    url,
+    payload: { form_data: safeStringify(formData) },
+  });
 };
 
 export const useDebouncedEffect = (effect, delay, deps) => {

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

@@ -16,24 +16,36 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { render, screen } from 'spec/helpers/testing-library';
+import { render, screen, waitFor } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
+import { SupersetClient } from '@superset-ui/core';
+import getBootstrapData, { applicationRoot } from 'src/utils/getBootstrapData';
 import Login from './index';
 
+const defaultBootstrapData = (authUserRegistration: boolean = false) => ({
+  common: {
+    conf: {
+      AUTH_TYPE: 1,
+      AUTH_PROVIDERS: [],
+      AUTH_USER_REGISTRATION: authUserRegistration,
+    },
+    feature_flags: {},
+  },
+});
+
 jest.mock('src/utils/getBootstrapData', () => ({
   __esModule: true,
-  default: () => ({
-    common: {
-      conf: {
-        AUTH_TYPE: 1,
-        AUTH_PROVIDERS: [],
-        AUTH_USER_REGISTRATION: false,
-      },
-    },
-  }),
+  default: jest.fn(() => defaultBootstrapData()),
+  applicationRoot: jest.fn(() => ''),
 }));
 
+const mockGetBootstrapData = getBootstrapData as jest.Mock;
+const mockApplicationRoot = applicationRoot as jest.Mock;
+
+const renderLogin = () => render(<Login />, { useRedux: true });
+
 test('should render login form elements', () => {
-  render(<Login />);
+  renderLogin();
   expect(screen.getByTestId('login-form')).toBeInTheDocument();
   expect(screen.getByTestId('username-input')).toBeInTheDocument();
   expect(screen.getByTestId('password-input')).toBeInTheDocument();
@@ -42,14 +54,100 @@ test('should render login form elements', () => {
 });
 
 test('should render username and password labels', () => {
-  render(<Login />);
+  renderLogin();
   expect(screen.getByText('Username:')).toBeInTheDocument();
   expect(screen.getByText('Password:')).toBeInTheDocument();
 });
 
 test('should render form instruction text', () => {
-  render(<Login />);
+  renderLogin();
   expect(
     screen.getByText('Enter your login and password below:'),
   ).toBeInTheDocument();
 });
+
+test('should render registration button with correct app root URL when authRegister=true', () => {
+  mockGetBootstrapData.mockReturnValue(defaultBootstrapData(true));
+  mockApplicationRoot.mockReturnValue('/superset');
+
+  renderLogin();
+
+  const registerButton = screen.getByTestId('register-button');
+  expect(registerButton).toHaveAttribute('href', '/superset/register/');
+});
+
+test.each([['', '/superset']])(
+  'should render OAuth providers with app root %s',
+  (app_root: string) => {
+    mockGetBootstrapData.mockReturnValue({
+      common: {
+        conf: {
+          AUTH_TYPE: 4, // AuthType.AuthOauth
+          AUTH_PROVIDERS: [
+            { name: 'google', icon: 'google' },
+            { name: 'github', icon: 'github' },
+          ],
+          AUTH_USER_REGISTRATION: false,
+        },
+      },
+    });
+
+    mockApplicationRoot.mockReturnValue(app_root);
+
+    renderLogin();
+
+    const googleButton = screen.getByRole('link', {
+      name: /Sign in with Google/i,
+    });
+    const githubButton = screen.getByRole('link', {
+      name: /Sign in with Github/i,
+    });
+
+    expect(googleButton).toHaveAttribute('href', `${app_root}/login/google`);
+    expect(githubButton).toHaveAttribute('href', `${app_root}/login/github`);
+  },
+);
+
+test.each([[1, 2]])(
+  'should call SupersetClient.postForm with correct endpoint for AuthDB/AuthLDAP',
+  async (authType: number) => {
+    mockGetBootstrapData.mockReturnValue({
+      common: {
+        conf: {
+          AUTH_TYPE: authType,
+          AUTH_PROVIDERS: [],
+          AUTH_USER_REGISTRATION: false,
+        },
+      },
+    });
+    mockApplicationRoot.mockReturnValue('/superset');
+
+    const postFormSpy = jest
+      .spyOn(SupersetClient, 'postForm')
+      .mockResolvedValue();
+
+    renderLogin();
+
+    // Fill in the form
+    const usernameInput = screen.getByTestId('username-input');
+    const passwordInput = screen.getByTestId('password-input');
+    const loginButton = screen.getByTestId('login-button');
+
+    await userEvent.type(usernameInput, 'testuser');
+    await userEvent.type(passwordInput, 'testpass');
+    await userEvent.click(loginButton);
+
+    await waitFor(() => {
+      expect(postFormSpy).toHaveBeenCalledWith(
+        // Should be bare endpoint, not /superset/login/
+        {
+          endpoint: '/login/',
+          payload: { username: 'testuser', password: 'testpass' },
+          target: '',
+        },
+      );
+    });
+
+    postFormSpy.mockRestore();
+  },
+);

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

@@ -30,6 +30,7 @@ import {
 import { useState, useMemo } from 'react';
 import { capitalize } from 'lodash/fp';
 import getBootstrapData from 'src/utils/getBootstrapData';
+import { ensureAppRoot } from 'src/utils/pathUtils';
 
 type OAuthProvider = {
   name: string;
@@ -94,7 +95,7 @@ export default function Login() {
   );
 
   const buildProviderLoginUrl = (providerName: string) => {
-    const base = `/login/${providerName}`;
+    const base = ensureAppRoot(`/login/${providerName}`);
     return nextUrl
       ? `${base}${base.includes('?') ? '&' : '?'}next=${encodeURIComponent(nextUrl)}`
       : base;
@@ -107,7 +108,11 @@ export default function Login() {
 
   const onFinish = (values: LoginForm) => {
     setLoading(true);
-    SupersetClient.postForm(loginEndpoint, values, '').finally(() => {
+    SupersetClient.postForm({
+      endpoint: loginEndpoint,
+      payload: values,
+      target: '',
+    }).finally(() => {
       setLoading(false);
     });
   };
@@ -232,7 +237,7 @@ export default function Login() {
                     <Button
                       block
                       type="default"
-                      href="/register/"
+                      href={ensureAppRoot('/register/')}
                       data-test="register-button"
                     >
                       {t('Register')}