fix(ui): JwtClaim should also check for "sub" claim and add fallback mechanism (#9909)

* fix(ui): JwtClaim should also check for "sub" claim and add fallback mechanism

* test: add unit tests

openmetadata-ui/src/main/resources/ui/src/components/authentication/auth-provider/AuthProvider.interface.ts

@@ -24,7 +24,7 @@ export type UserProfile = {
   name: string;
   picture: string;
   locale?: string;
-} & Pick<Profile, 'preferred_username'>;
+} & Pick<Profile, 'preferred_username' | 'sub'>;
 
 export type OidcUser = {
   id_token: string;
@@ -41,4 +41,5 @@ export interface AuthenticatorRef {
 export enum JWT_PRINCIPAL_CLAIMS {
   EMAIL = 'email',
   PREFERRED_USERNAME = 'preferred_username',
+  SUB = 'sub',
 }

openmetadata-ui/src/main/resources/ui/src/components/authentication/auth-provider/basic-auth.provider.tsx

@@ -111,6 +111,7 @@ const BasicAuthProvider = ({
               email,
               name: '',
               picture: '',
+              sub: '',
             },
             scope: '',
           });

openmetadata-ui/src/main/resources/ui/src/components/authentication/auth-provider/okta-auth-provider.tsx

@@ -59,6 +59,7 @@ export const OktaAuthProvider: FunctionComponent<Props> = ({
             // eslint-disable-next-line @typescript-eslint/no-explicit-any
             picture: (info as any).imageUrl || '',
             locale: info.locale || '',
+            sub: info.sub,
           },
         };
         onLoginSuccess(user);

openmetadata-ui/src/main/resources/ui/src/components/authentication/authenticators/MsalAuthenticator.tsx

@@ -83,6 +83,7 @@ const MsalAuthenticator = forwardRef<AuthenticatorRef, Props>(
           email: account?.username || '',
           name: account?.name || '',
           picture: '',
+          sub: '',
         },
       };
 

openmetadata-ui/src/main/resources/ui/src/components/authentication/callbacks/Auth0Callback/Auth0Callback.test.tsx

@@ -121,6 +121,7 @@ describe('Test Auth0Callback component', () => {
         name: 'test_user',
         picture: 'test_picture',
         locale: 'test_locale',
+        sub: '',
       },
       scope: '',
     });

openmetadata-ui/src/main/resources/ui/src/components/authentication/callbacks/Auth0Callback/Auth0Callback.tsx

@@ -34,6 +34,7 @@ const Auth0Callback: VFC = () => {
             name: user?.name || '',
             picture: user?.picture || '',
             locale: user?.locale || '',
+            sub: user?.sub || '',
           },
         };
         handleSuccessfulLogin(oidcUser);

openmetadata-ui/src/main/resources/ui/src/pages/signup/index.test.tsx

@@ -19,6 +19,8 @@ import AppState from '../../AppState';
 import { getImages } from '../../utils/CommonUtils';
 import { mockCreateUser } from './mocks/signup.mock';
 
+let letExpectedUserName = 'sample123';
+
 const mockChangeHandler = jest.fn();
 const mockSubmitHandler = jest.fn();
 const mockShowErrorToast = jest.fn();
@@ -84,6 +86,10 @@ jest.mock('../../utils/CommonUtils', () => ({
     ),
 }));
 
+jest.mock('utils/AuthProvider.util', () => ({
+  getNameFromUserData: jest.fn().mockImplementation(() => letExpectedUserName),
+}));
+
 describe('Signup page', () => {
   beforeEach(() => {
     jest.clearAllMocks();
@@ -222,6 +228,7 @@ describe('Signup page', () => {
 
   it('Handlers in form should work if data is empty', async () => {
     (getImages as jest.Mock).mockImplementationOnce(() => Promise.reject(''));
+    letExpectedUserName = '';
 
     AppState.newUser = {
       name: '',

openmetadata-ui/src/main/resources/ui/src/pages/signup/index.tsx

@@ -21,12 +21,12 @@ import { CookieStorage } from 'cookie-storage';
 import React, { useState } from 'react';
 import { useHistory } from 'react-router-dom';
 import { createUser } from 'rest/userAPI';
+import { getNameFromUserData } from 'utils/AuthProvider.util';
 import appState from '../../AppState';
 import { REDIRECT_PATHNAME, ROUTES } from '../../constants/constants';
 import { CreateUser } from '../../generated/api/teams/createUser';
 import { User } from '../../generated/entity/teams/user';
 import jsonData from '../../jsons/en';
-import { getNameFromUserData } from '../../utils/AuthProvider.util';
 import { getImages } from '../../utils/CommonUtils';
 import SVGIcons, { Icons } from '../../utils/SvgUtils';
 import { showErrorToast } from '../../utils/ToastUtils';

openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.ts

@@ -18,10 +18,7 @@ import {
   PopupRequest,
   PublicClientApplication,
 } from '@azure/msal-browser';
-import {
+import { UserProfile } from 'components/authentication/auth-provider/AuthProvider.interface';
-  JWT_PRINCIPAL_CLAIMS,
-  UserProfile,
-} from 'components/authentication/auth-provider/AuthProvider.interface';
 import jwtDecode, { JwtPayload } from 'jwt-decode';
 import { first, isNil } from 'lodash';
 import { WebStorageStateStore } from 'oidc-client';
@@ -212,18 +209,32 @@ export const getNameFromUserData = (
   user: UserProfile,
   jwtPrincipalClaims: AuthenticationConfiguration['jwtPrincipalClaims'] = []
 ) => {
-  // get the first claim from claim list
+  // filter and extract the present claims in user profile
-  const firstClaim = first(jwtPrincipalClaims);
+  const jwtClaims = jwtPrincipalClaims.reduce(
-  const nameFromEmail = getNameFromEmail(user.email);
+    (prev: string[], curr: string) => {
+      const currentClaim = user[curr as keyof UserProfile];
+      if (currentClaim) {
+        return [...prev, currentClaim];
+      } else {
+        return prev;
+      }
+    },
+    []
+  );
 
-  /* if first claim is preferred_username then return preferred_username
+  // get the first claim from claims list
-   * for fallback if preferred_username is not present then return the name from email
+  const firstClaim = first(jwtClaims);
-   */
+
-  if (firstClaim === JWT_PRINCIPAL_CLAIMS.PREFERRED_USERNAME) {
+  let userName = '';
-    return user.preferred_username ?? nameFromEmail;
+
+  // if claims contains the "@" then split it out otherwise assign it to username as it is
+  if (firstClaim?.includes('@')) {
+    userName = getNameFromEmail(firstClaim);
+  } else {
+    userName = firstClaim ?? '';
   }
 
-  return nameFromEmail;
+  return userName;
 };
 
 export const isProtectedRoute = (pathname: string) => {

openmetadata-ui/src/main/resources/ui/src/utils/AuthProviderUtil.test.ts

@@ -0,0 +1,94 @@
+/*
+ *  Copyright 2023 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 { getNameFromUserData } from './AuthProvider.util';
+
+const userProfile = {
+  email: 'testUser@gmail.com',
+  sub: 'i_am_sub',
+  preferred_username: 'i_am_preferred_username',
+  name: 'Test User',
+  picture: '',
+};
+
+describe('Test Auth Provider utils', () => {
+  it('getNameFromUserData should return the userName for first claim', () => {
+    // should return the preferred username
+    const userName1 = getNameFromUserData(userProfile, [
+      'preferred_username',
+      'email',
+      'sub',
+    ]);
+
+    expect(userName1).toEqual('i_am_preferred_username');
+
+    // should return the name from email
+    const userName2 = getNameFromUserData(userProfile, [
+      'email',
+      'preferred_username',
+      'sub',
+    ]);
+
+    expect(userName2).toEqual('testUser');
+
+    // should return the sub
+    const userName3 = getNameFromUserData(userProfile, [
+      'sub',
+      'email',
+      'preferred_username',
+    ]);
+
+    expect(userName3).toEqual('i_am_sub');
+  });
+
+  it('getNameFromUserData should fallback to next claim if first claim is not present', () => {
+    // should return the name from email as fallback
+    const userName1 = getNameFromUserData(
+      { ...userProfile, preferred_username: '' },
+      ['preferred_username', 'email', 'sub']
+    );
+
+    expect(userName1).toEqual('testUser');
+
+    // should return the sub as fallback
+    const userName2 = getNameFromUserData(
+      { ...userProfile, preferred_username: '' },
+      ['preferred_username', 'sub', 'email']
+    );
+
+    expect(userName2).toEqual('i_am_sub');
+
+    // should return the name from email as fallback if both 'preferred_username' and 'sub' are not present
+    const userName3 = getNameFromUserData(
+      { ...userProfile, preferred_username: '', sub: '' },
+      ['preferred_username', 'sub', 'email']
+    );
+
+    expect(userName3).toEqual('testUser');
+  });
+
+  it('getNameFromUserData should handle the claim if it contains @', () => {
+    const userName1 = getNameFromUserData(
+      { ...userProfile, preferred_username: 'test@gmail.com' },
+      ['preferred_username', 'email', 'sub']
+    );
+
+    expect(userName1).toEqual('test');
+
+    const userName2 = getNameFromUserData(
+      { ...userProfile, sub: 'test-1@gmail.com' },
+      ['sub', 'preferred_username', 'email']
+    );
+
+    expect(userName2).toEqual('test-1');
+  });
+});