Addressing PR comments, moved stateless signIn functions to their own…

… module, simplified RedirectFromOAuth REBASE
DataBiosphere · Jun 27, 2024 · ede346f · ede346f
1 parent c3cb893
commit ede346f
Show file tree

Hide file tree

Showing 10 changed files with 149 additions and 149 deletions.
diff --git a/src/App.jsx b/src/App.jsx
@@ -3,7 +3,6 @@ import ReactGA from 'react-ga4';
 import Modal from 'react-modal';
 import './App.css';
 import {Config} from './libs/config';
-import { Auth } from './libs/auth/auth';
 import DuosFooter from './components/DuosFooter';
 import DuosHeader from './components/DuosHeader';
 import {useHistory, useLocation} from 'react-router-dom';

diff --git a/src/components/DuosHeader.jsx b/src/components/DuosHeader.jsx
@@ -233,7 +233,7 @@ const NavigationTabsComponent = (props) => {
           <div
             style={{ display: 'flex', alignItems: 'center', flexDirection: orientation === 'vertical' ? 'column' : 'row' }}
           >
-            <SignInButton customStyle={{ whiteSpace: 'nowrap',  }}></SignInButton>
+            <SignInButton style={{ whiteSpace: 'nowrap', color: 'black' }}></SignInButton>
           </div>
         )}
         {isLogged && (

diff --git a/src/components/SignInButton.tsx b/src/components/SignInButton.tsx
@@ -1,21 +1,21 @@
 import React, { useState } from 'react';
-import { isEmpty, isNil } from 'lodash/fp';
 import { Alert } from './Alert';
-import { ToS } from '../libs/ajax/ToS';
-import { DuosUser, User } from '../libs/ajax/User';
-import { Metrics } from '../libs/ajax/Metrics';
 import { Storage } from '../libs/storage';
-import { Navigation, setUserRoleStatuses } from '../libs/utils';
 import loadingImage from '../images/loading-indicator.svg';
 import { Auth } from '../libs/auth/auth';
-import eventList from '../libs/events';
-import { StackdriverReporter } from '../libs/stackdriverReporter';
 import CSS from 'csstype';
 import { useHistory } from 'react-router';
-import { OidcUser } from 'src/libs/auth/oidcBroker';
+import { OidcUser } from '../libs/auth/oidcBroker';
+import {
+  attemptSignInCheckToSAndRedirect,
+  getRedirectTo,
+  handleConflictError,
+  registerAndRedirectNewUser,
+  shouldRedirectTo,
+} from '../libs/signIn';
 
 interface SignInButtonProps {
-  customStyle: CSS.Properties | undefined;
+  style: CSS.Properties | undefined;
 }
 
 interface ErrorInfo {
@@ -33,93 +33,29 @@ export const SignInButton = (props: SignInButtonProps) => {
   const [errorInfo, setErrorInfo] = useState<ErrorInfo>({});
   const [isLoading, setIsLoading] = useState<boolean>(false);
   const [showError, setShowError] = useState<boolean>(false);
-  const { customStyle } = props;
+  const { style } = props;
   const history = useHistory();
 
-  // Utility function called in the normal success case and in the undocumented 409 case
-  // Check for ToS Acceptance - redirect user if not set.
-  const checkToSAndRedirect = async (redirectPath: string | null) => {
-    // Check if the user has accepted ToS yet or not:
-    const user: DuosUser = await User.getMe();
-    if (!user.roles) {
-      await StackdriverReporter.report(
-        'roles not found for user: ' + user.email
-      );
-    }
-    setUserRoleStatuses(user, Storage);
-    const userStatus = await ToS.getStatus();
-    const { tosAccepted } = userStatus;
-    if (!isEmpty(userStatus) && !tosAccepted) {
-      Storage.setUserIsLogged(false);
-      if (isNil(redirectPath)) {
-        history.push(`/tos_acceptance`);
-      } else {
-        history.push(`/tos_acceptance?redirectTo=${redirectPath}`);
-      }
-    } else {
-      if (isNil(redirectPath)) {
-        Navigation.back(user, history);
-      } else {
-        history.push(redirectPath);
-      }
-    }
-  };
-
   const onSuccess = async (response: OidcUser) => {
 
     const redirectTo = getRedirectTo();
     const shouldRedirect = shouldRedirectTo(redirectTo);
     Storage.setAnonymousId();
     try {
-      await attemptSignInCheckToSAndRedirect(redirectTo, shouldRedirect);
-    } catch (error: unknown) {
-      await handleRegistration(redirectTo, shouldRedirect);
-    }
-  };
-
-  const getRedirectTo = (): string => {
-    const queryParams = new URLSearchParams(window.location.search);
-    return queryParams.get('redirectTo') || window.location.pathname;
-  };
-
-  const shouldRedirectTo = (page: string): boolean =>
-    page !== '/' && page !== '/home';
-
-  const attemptSignInCheckToSAndRedirect = async (
-    redirectTo: string,
-    shouldRedirect: boolean
-  ) => {
-    await checkToSAndRedirect(shouldRedirect ? redirectTo : null);
-    Metrics.identify(Storage.getAnonymousId());
-    Metrics.syncProfile();
-    Metrics.captureEvent(eventList.userSignIn);
-  };
-
-  const handleRegistration = async (
-    redirectTo: string,
-    shouldRedirect: boolean
-  ) => {
-    try {
-      await registerAndRedirectNewUser(redirectTo, shouldRedirect);
+      await attemptSignInCheckToSAndRedirect(
+        redirectTo,
+        shouldRedirect,
+        history
+      );
     } catch (error: unknown) {
-      await handleErrors(error as HttpError, redirectTo, shouldRedirect);
+      await registerAndRedirectNewUser(
+        redirectTo,
+        shouldRedirect,
+        history
+      ).catch((reason) => handleErrors(reason, redirectTo, shouldRedirect));
     }
   };
 
-  const registerAndRedirectNewUser = async (
-    redirectTo: string,
-    shouldRedirect: boolean
-  ) => {
-    const registeredUser = await User.registerUser();
-    setUserRoleStatuses(registeredUser, Storage);
-    Metrics.identify(Storage.getAnonymousId());
-    Metrics.syncProfile();
-    Metrics.captureEvent(eventList.userRegister);
-    history.push(
-      `/tos_acceptance${shouldRedirect ? `?redirectTo=${redirectTo}` : ''}`
-    );
-  };
-
   const handleErrors = async (
     error: HttpError,
     redirectTo: string,
@@ -136,7 +72,7 @@ export const SignInButton = (props: SignInButtonProps) => {
         });
         break;
       case 409:
-        handleConflictError(redirectTo, shouldRedirect);
+        handleConflictError(redirectTo, shouldRedirect, history);
         break;
       default:
         setShowError(true);
@@ -148,16 +84,8 @@ export const SignInButton = (props: SignInButtonProps) => {
     }
   };
 
-  const handleConflictError = async (redirectTo: string, shouldRedirect: boolean) => {
-    try {
-      await checkToSAndRedirect(shouldRedirect ? redirectTo : null);
-    } catch (error) {
-      Storage.clearStorage();
-    }
-  };
-
   const onFailure = (error: Error) => {
-    Storage.clearStorage();
+    Auth.signOut();
     setIsLoading(false);
     if (!error.message.includes('Popup closed by user')) {
       setShowError(true);
@@ -180,10 +108,10 @@ export const SignInButton = (props: SignInButtonProps) => {
     return (
       <button
         className={'btn-secondary'}
-        style={customStyle}
+        style={style}
         onClick={async () => {
           setIsLoading(true);
-          await Auth.signIn(true, onSuccess, onFailure);
+          await Auth.signIn(true).then(onSuccess, onFailure);
           setIsLoading(false);
         }}
         disabled={isLoading}
@@ -206,7 +134,7 @@ export const SignInButton = (props: SignInButtonProps) => {
     );
   };
 
-  return <div>{!showError ? signInButton() : errorAlert(errorInfo)}</div>;
+  return <div>{showError ? errorAlert(errorInfo) : signInButton()}</div>;
 };
 
 export default SignInButton;
diff --git a/src/index.ts b/src/index.ts
@@ -12,7 +12,7 @@ const load = async (): Promise<void> => {
   unregister();
   await Auth.initialize();
   window.location.pathname.startsWith('/redirect-from-oauth')
-    ? import('./libs/auth/oauth-redirect-loader')
+    ? import('./libs/auth/RedirectFromOAuth')
     : import('./appLoader');
 };
 

diff --git a/src/libs/auth/RedirectFromOAuth.ts b/src/libs/auth/RedirectFromOAuth.ts
diff --git a/src/libs/auth/RedirectFromOAuth.tsx b/src/libs/auth/RedirectFromOAuth.tsx
@@ -0,0 +1,23 @@
+import React, { useState } from 'react';
+import { createRoot } from 'react-dom/client';
+import { OidcBroker } from './oidcBroker';
+import { Spinner } from '../../components/Spinner';
+import { UserManager } from 'oidc-client-ts';
+
+const userManager: UserManager = new UserManager(
+  OidcBroker.getUserManagerSettings()
+);
+const url = window.location.href;
+const isSilent = window.location.pathname.startsWith(
+  '/redirect-from-oauth-silent'
+);
+
+if (isSilent) {
+  userManager.signinSilentCallback(url);
+} else {
+  userManager.signinPopupCallback(url);
+}
+
+const rootElement = document.getElementById('root');
+const root = createRoot(rootElement!);
+root.render(<Spinner />);
diff --git a/src/libs/auth/auth.ts b/src/libs/auth/auth.ts
@@ -3,19 +3,17 @@
     and implement DUOS specific auth login (signIn, signOut, etc.)
 */
 
-
 import { OidcBroker, OidcUser } from './oidcBroker';
 import { Storage } from './../storage';
 import { UserManager } from 'oidc-client-ts';
 
-
 export const Auth = {
   getToken: (): string => {
     // In a future ticket, it would be better to make get Token an async function and call
     // the UserManager.getUser to get the token. Since authOpts depends on getToken being synchonous
     // it would be alot of places to change the uses of authOpts.
     const oidcUser: OidcUser | null = OidcBroker.getUserSync();
-    return oidcUser !== null ? oidcUser.access_token : "token";
+    return oidcUser !== null ? oidcUser.access_token : 'token';
   },
   initialize: async (): Promise<void> => {
     await OidcBroker.initialize();
@@ -24,15 +22,14 @@ export const Auth = {
     // UserManager events.
     // For details of each event, see https://authts.github.io/oidc-client-ts/classes/UserManagerEvents.html
     um.events.addUserLoaded((user: OidcUser) => {
-      //TODO: Add metrics for user loaded
+      //TODO: DUOS-3072 Add metrics for user loaded
     });
     um.events.addAccessTokenExpiring((): void => {
-      //TODO: Add an alert that session will expire soon
-      console.log('accessTokenExpiring');
+      //TODO: DUOS-3082 Add an alert that session will expire soon
     });
     um.events.addAccessTokenExpired((): void => {
       Auth.signOut();
-      //TODO: Add an alert that session has expired
+      //TODO: DUOS-3082 Add an alert that session has expired
     });
     if (oidcUser !== null) {
       Storage.setUserIsLogged(true);
@@ -41,21 +38,13 @@ export const Auth = {
     }
   },
 
-  signIn: async (
-    popup: boolean,
-    onSuccess?: (response: OidcUser) => Promise<void> | void,
-    onFailure?: (response: any) => Promise<void> | void
-  ): Promise<void> => {
-    try {
-      const user: OidcUser | null = await OidcBroker.signIn(popup);
-      if (user === null) {
-        throw new Error('signInSilent called before signInPopup');
-      }
-      await onSuccess?.(user);
-      Storage.setUserIsLogged(true);
-    } catch (err) {
-      onFailure?.(err);
+  signIn: async (popup: boolean): Promise<OidcUser> => {
+    const user: OidcUser | null = await OidcBroker.signIn(popup);
+    if (user === null) {
+      throw new Error('signInSilent called before signInPopup');
     }
+    Storage.setUserIsLogged(true);
+    return user;
   },
 
   signOut: async () => {

diff --git a/src/libs/auth/oauth-redirect-loader.ts b/src/libs/auth/oauth-redirect-loader.ts
diff --git a/src/libs/auth/oidcBroker.ts b/src/libs/auth/oidcBroker.ts
@@ -88,6 +88,8 @@ export const OidcBroker = {
   },
 
   signOut: async (): Promise<void> => {
-    await OidcBroker.getUserManager().removeUser();
+    const um: UserManager = OidcBroker.getUserManager();
+    await um.removeUser();
+    await um.clearStaleState();
   } 
 };