fix(ui): forever loading for oidc auth refresh failure (#19854)

* fix(ui): forever loading for oidc auth refresh failure

* fix redirect login issue with iframe
added tests for iframe vs popup login
added tests for clearing localState for refresh api failures

openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/MsalAuthenticator.test.tsx

@@ -10,42 +10,160 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
-
-import { render, screen } from '@testing-library/react';
+import { InteractionStatus } from '@azure/msal-browser';
+import { useMsal } from '@azure/msal-react';
+import { act, render, screen } from '@testing-library/react';
 import React from 'react';
-import { MemoryRouter } from 'react-router-dom';
+import { msalLoginRequest } from '../../../utils/AuthProvider.util';
+import { AuthenticatorRef } from '../AuthProviders/AuthProvider.interface';
 import MsalAuthenticator from './MsalAuthenticator';
 
-jest.mock('../../../hooks/useApplicationStore', () => {
-  return {
-    useApplicationStore: jest.fn(() => ({
-      authConfig: {},
-      getOidcToken: jest.fn(),
-      setOidcToken: jest.fn(),
-    })),
-  };
-});
+// Mock MSAL hooks and utilities
+jest.mock('@azure/msal-react', () => ({
+  useMsal: jest.fn(),
+  useAccount: jest.fn(),
+}));
+
+jest.mock('../../../utils/AuthProvider.util', () => ({
+  msalLoginRequest: {
+    scopes: ['test.scope'],
+  },
+  parseMSALResponse: jest.fn().mockImplementation((response) => ({
+    id_token: 'mock-id-token',
+    ...response,
+  })),
+}));
+
+const mockInstance = {
+  loginPopup: jest.fn(),
+  loginRedirect: jest.fn(),
+  handleRedirectPromise: jest.fn(),
+  ssoSilent: jest.fn(),
+};
+
+const mockProps = {
+  children: <div>Test Children</div>,
+  onLoginSuccess: jest.fn(),
+  onLoginFailure: jest.fn(),
+  onLogoutSuccess: jest.fn(),
+};
+
+describe('MsalAuthenticator', () => {
+  let authenticatorRef: AuthenticatorRef | null = null;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    // Default mock implementation for useMsal
+    (useMsal as jest.Mock).mockReturnValue({
+      instance: mockInstance,
+      accounts: [{ username: 'test@example.com' }],
+      inProgress: InteractionStatus.None,
+    });
+  });
+
+  it('should handle login in iframe using popup', async () => {
+    // Mock window.self !== window.top for iframe detection
+    Object.defineProperty(window, 'self', {
+      value: { location: {} },
+      writable: true,
+    });
+    Object.defineProperty(window, 'top', {
+      value: { location: {} },
+      writable: true,
+    });
+
+    mockInstance.loginPopup.mockResolvedValueOnce({
+      account: { username: 'test@example.com' },
+    });
 
-describe('Test MsalAuthenticator component', () => {
-  it('Checks if the MsalAuthenticator renders', async () => {
-    const onLogoutMock = jest.fn();
-    const onLoginMock = jest.fn();
-    const onFailedLoginMock = jest.fn();
-    const authenticatorRef = null;
     render(
       <MsalAuthenticator
-        ref={authenticatorRef}
-        onLoginFailure={onFailedLoginMock}
-        onLoginSuccess={onLoginMock}
-        onLogoutSuccess={onLogoutMock}>
-        <p data-testid="children" />
-      </MsalAuthenticator>,
-      {
-        wrapper: MemoryRouter,
-      }
+        {...mockProps}
+        ref={(ref) => (authenticatorRef = ref)}
+      />
     );
-    const children = screen.getByTestId('children');
 
-    expect(children).toBeInTheDocument();
+    await act(async () => {
+      authenticatorRef?.invokeLogin();
+    });
+
+    expect(mockInstance.loginPopup).toHaveBeenCalledWith(msalLoginRequest);
+    expect(mockProps.onLoginSuccess).toHaveBeenCalled();
+  });
+
+  it('should handle login in normal window using redirect', async () => {
+    // Mock window.self === window.top for normal window detection
+    Object.defineProperty(window, 'self', {
+      value: window,
+      writable: true,
+    });
+    Object.defineProperty(window, 'top', {
+      value: window,
+      writable: true,
+    });
+
+    render(
+      <MsalAuthenticator
+        {...mockProps}
+        ref={(ref) => (authenticatorRef = ref)}
+      />
+    );
+
+    await act(async () => {
+      authenticatorRef?.invokeLogin();
+    });
+
+    expect(mockInstance.loginRedirect).toHaveBeenCalledWith(msalLoginRequest);
+  });
+
+  it('should handle logout', async () => {
+    render(
+      <MsalAuthenticator
+        {...mockProps}
+        ref={(ref) => (authenticatorRef = ref)}
+      />
+    );
+
+    await act(async () => {
+      authenticatorRef?.invokeLogout();
+    });
+
+    expect(mockProps.onLogoutSuccess).toHaveBeenCalled();
+  });
+
+  it('should handle renewIdToken successfully', async () => {
+    mockInstance.ssoSilent.mockResolvedValueOnce({
+      account: { username: 'test@example.com' },
+      idToken: 'new-token',
+    });
+
+    render(
+      <MsalAuthenticator
+        {...mockProps}
+        ref={(ref) => (authenticatorRef = ref)}
+      />
+    );
+
+    const result = await authenticatorRef?.renewIdToken();
+
+    expect(mockInstance.ssoSilent).toHaveBeenCalled();
+    expect(result).toBe('mock-id-token');
+  });
+
+  it('should show loader when interaction is in progress', () => {
+    (useMsal as jest.Mock).mockReturnValue({
+      instance: mockInstance,
+      accounts: [{ username: 'test@example.com' }],
+      inProgress: InteractionStatus.Login,
+    });
+
+    render(
+      <MsalAuthenticator
+        {...mockProps}
+        ref={(ref) => (authenticatorRef = ref)}
+      />
+    );
+
+    expect(screen.getByTestId('loader')).toBeInTheDocument();
   });
 });

openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/MsalAuthenticator.tsx

@@ -63,8 +63,17 @@ const MsalAuthenticator = forwardRef<AuthenticatorRef, Props>(
 
     const login = async () => {
       try {
-        // Use login with redirect to avoid popup issue with maximized browser window
-        await instance.loginRedirect();
+        const isInIframe = window.self !== window.top;
+
+        if (isInIframe) {
+          // Use popup login when in iframe to avoid redirect issues
+          const response = await instance.loginPopup(msalLoginRequest);
+
+          onLoginSuccess(parseMSALResponse(response));
+        } else {
+          // Use login with redirect for normal window context
+          await instance.loginRedirect(msalLoginRequest);
+        }
       } catch (error) {
         onLoginFailure(error as AxiosError);
       }

openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/OidcAuthenticator.test.tsx

@@ -0,0 +1,251 @@
+/*
+ *  Copyright 2025 Collate.
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+import { act, render, screen } from '@testing-library/react';
+import { User } from 'oidc-client';
+import React from 'react';
+import { Callback } from 'react-oidc';
+import { MemoryRouter } from 'react-router-dom';
+import { ROUTES } from '../../../constants/constants';
+import { useApplicationStore } from '../../../hooks/useApplicationStore';
+import TokenService from '../../../utils/Auth/TokenService/TokenServiceUtil';
+import { setOidcToken } from '../../../utils/LocalStorageUtils';
+import {
+  AuthenticatorRef,
+  OidcUser,
+} from '../AuthProviders/AuthProvider.interface';
+import OidcAuthenticator from './OidcAuthenticator';
+
+const mockOnLoginSuccess = jest.fn();
+const mockOnLoginFailure = jest.fn();
+const mockOnLogoutSuccess = jest.fn();
+
+// Mock dependencies
+jest.mock('../../../utils/LocalStorageUtils');
+jest.mock('../../../utils/Auth/TokenService/TokenServiceUtil');
+jest.mock('../../../hooks/useApplicationStore');
+jest.mock('react-oidc', () => ({
+  ...jest.requireActual('react-oidc'),
+  Callback: jest.fn().mockImplementation(() => {
+    return <div>Callback</div>;
+  }),
+}));
+
+const mockHistoryPush = jest.fn();
+jest.mock('react-router-dom', () => ({
+  ...jest.requireActual('react-router-dom'),
+  useHistory: () => ({
+    push: mockHistoryPush,
+  }),
+}));
+
+describe('OidcAuthenticator - Silent SignIn Tests', () => {
+  const mockUpdateAxiosInterceptors = jest.fn();
+  const mockTokenServiceInstance = {
+    clearRefreshInProgress: jest.fn(),
+  };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    (TokenService.getInstance as jest.Mock).mockReturnValue(
+      mockTokenServiceInstance
+    );
+    (useApplicationStore as unknown as jest.Mock).mockReturnValue({
+      updateAxiosInterceptors: mockUpdateAxiosInterceptors,
+    });
+  });
+
+  it('should handle silent sign-in success', async () => {
+    const mockUser = {
+      id_token: 'mock-id-token',
+    } as User;
+
+    render(
+      <MemoryRouter initialEntries={[ROUTES.SILENT_CALLBACK]}>
+        <OidcAuthenticator
+          childComponentType={() => <div>Child Component</div>}
+          userConfig={{}}
+          onLoginFailure={mockOnLoginFailure}
+          onLoginSuccess={mockOnLoginSuccess}
+          onLogoutSuccess={mockOnLogoutSuccess}>
+          <div>Protected Content</div>
+        </OidcAuthenticator>
+      </MemoryRouter>
+    );
+
+    expect(Callback).toHaveBeenCalled();
+
+    // Get the Callback component's onSuccess prop and call it
+    const callbackProps = (Callback as jest.Mock).mock.calls[0][0];
+    await act(async () => {
+      callbackProps.onSuccess(mockUser);
+    });
+
+    // Verify the success flow
+    expect(setOidcToken).toHaveBeenCalledWith('mock-id-token');
+    expect(mockUpdateAxiosInterceptors).toHaveBeenCalled();
+    expect(mockTokenServiceInstance.clearRefreshInProgress).toHaveBeenCalled();
+  });
+
+  it('should handle silent sign-in failure', async () => {
+    const mockError = new Error('Silent sign-in failed');
+
+    render(
+      <MemoryRouter initialEntries={[ROUTES.SILENT_CALLBACK]}>
+        <OidcAuthenticator
+          childComponentType={() => <div>Child Component</div>}
+          userConfig={{}}
+          onLoginFailure={mockOnLoginFailure}
+          onLoginSuccess={mockOnLoginSuccess}
+          onLogoutSuccess={mockOnLogoutSuccess}>
+          <div>Protected Content</div>
+        </OidcAuthenticator>
+      </MemoryRouter>
+    );
+
+    // Get the Callback component's onError prop and call it
+    const callbackProps = (Callback as jest.Mock).mock.calls[0][0];
+    await act(async () => {
+      callbackProps.onError(mockError);
+    });
+
+    // Verify the failure flow
+    expect(mockTokenServiceInstance.clearRefreshInProgress).toHaveBeenCalled();
+    expect(mockOnLogoutSuccess).toHaveBeenCalled();
+    expect(mockHistoryPush).toHaveBeenCalledWith(ROUTES.SIGNIN);
+  });
+});
+
+describe('OidcAuthenticator - Login Tests', () => {
+  const mockUpdateAxiosInterceptors = jest.fn();
+  const mockSetIsSigningUp = jest.fn();
+  const mockOnLoginSuccess = jest.fn();
+  const mockOnLoginFailure = jest.fn();
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    (useApplicationStore as unknown as jest.Mock).mockReturnValue({
+      updateAxiosInterceptors: mockUpdateAxiosInterceptors,
+      setIsSigningUp: mockSetIsSigningUp,
+    });
+  });
+
+  it('should handle login success', async () => {
+    const mockUser = {
+      id_token: 'mock-id-token',
+      profile: {
+        email: 'test@test.com',
+        name: 'Test User',
+      },
+    } as OidcUser;
+
+    render(
+      <MemoryRouter initialEntries={[ROUTES.CALLBACK]}>
+        <OidcAuthenticator
+          childComponentType={() => <div>Child Component</div>}
+          userConfig={{}}
+          onLoginFailure={mockOnLoginFailure}
+          onLoginSuccess={mockOnLoginSuccess}
+          onLogoutSuccess={mockOnLogoutSuccess}>
+          <div>Protected Content</div>
+        </OidcAuthenticator>
+      </MemoryRouter>
+    );
+
+    // Get the Callback component's onSuccess prop and call it
+    const callbackProps = (Callback as jest.Mock).mock.calls[0][0];
+    await act(async () => {
+      callbackProps.onSuccess(mockUser);
+    });
+
+    // Verify the success flow
+    expect(setOidcToken).toHaveBeenCalledWith('mock-id-token');
+    expect(mockOnLoginSuccess).toHaveBeenCalledWith(mockUser);
+  });
+
+  it('should handle login failure', async () => {
+    const mockError = new Error('Login failed');
+
+    render(
+      <MemoryRouter initialEntries={[ROUTES.CALLBACK]}>
+        <OidcAuthenticator
+          childComponentType={() => <div>Child Component</div>}
+          userConfig={{}}
+          onLoginFailure={mockOnLoginFailure}
+          onLoginSuccess={mockOnLoginSuccess}
+          onLogoutSuccess={mockOnLogoutSuccess}>
+          <div>Protected Content</div>
+        </OidcAuthenticator>
+      </MemoryRouter>
+    );
+
+    // Get the Callback component's onError prop and call it
+    const callbackProps = (Callback as jest.Mock).mock.calls[0][0];
+    await act(async () => {
+      callbackProps.onError(mockError);
+    });
+
+    // Verify the failure flow
+    expect(mockOnLoginFailure).toHaveBeenCalled();
+  });
+
+  it('should handle login initiation', async () => {
+    const ref = React.createRef<AuthenticatorRef>();
+
+    render(
+      <MemoryRouter>
+        <OidcAuthenticator
+          childComponentType={() => <div>Child Component</div>}
+          ref={ref}
+          userConfig={{}}
+          onLoginFailure={mockOnLoginFailure}
+          onLoginSuccess={mockOnLoginSuccess}
+          onLogoutSuccess={mockOnLogoutSuccess}>
+          <div>Protected Content</div>
+        </OidcAuthenticator>
+      </MemoryRouter>
+    );
+
+    // Trigger login
+    await act(async () => {
+      ref.current?.invokeLogin();
+    });
+
+    // Verify login initialization
+    expect(mockSetIsSigningUp).toHaveBeenCalledWith(true);
+  });
+
+  it('should render SignInPage when not authenticated and not signing up', () => {
+    (useApplicationStore as unknown as jest.Mock).mockReturnValue({
+      isAuthenticated: false,
+      isSigningUp: false,
+      currentUser: {},
+      newUser: {},
+    });
+
+    render(
+      <MemoryRouter initialEntries={[ROUTES.SIGNIN]}>
+        <OidcAuthenticator
+          childComponentType={() => <div>Child Component</div>}
+          userConfig={{}}
+          onLoginFailure={mockOnLoginFailure}
+          onLoginSuccess={mockOnLoginSuccess}
+          onLogoutSuccess={mockOnLogoutSuccess}>
+          <div>Protected Content</div>
+        </OidcAuthenticator>
+      </MemoryRouter>
+    );
+
+    // Verify SignInPage is rendered
+    expect(screen.getByTestId('signin-page')).toBeInTheDocument();
+  });
+});

openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/OidcAuthenticator.tsx

@@ -115,6 +115,9 @@ const OidcAuthenticator = forwardRef<AuthenticatorRef, Props>(
       // eslint-disable-next-line no-console
       console.error(error);
 
+      // Clear the refresh token in progress flag
+      // Since refresh token request completes with a callback
+      TokenService.getInstance().clearRefreshInProgress();
       onLogoutSuccess();
       history.push(ROUTES.SIGNIN);
     };