fix: hot fix for private route (#1381)

reapit · May 28, 2020 · 75bd498 · 75bd498
1 parent 195c099
commit 75bd498
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 66 deletions.
diff --git a/packages/cognito-auth/src/session/remove-session.test.ts b/packages/cognito-auth/src/session/remove-session.test.ts
@@ -1,28 +1,27 @@
 import { removeSession } from './remove-session'
-import hardtack from 'hardtack'
 import { COOKIE_SESSION_KEY } from '../utils/cognito'
+import { mockRefreshParams } from '../__mocks__/cognito-session'
+
+const stringifiedMock = JSON.stringify(mockRefreshParams)
 
 describe('removeSession', () => {
-  it('should remove a session cookie for a valid host', () => {
-    window.location.hostname = 'something.reapit.com'
-    hardtack.remove = jest.fn()
+  it('should remove a session cookie', () => {
+    window.localStorage.setItem(COOKIE_SESSION_KEY, stringifiedMock)
+
+    expect(window.localStorage.getItem(COOKIE_SESSION_KEY)).toEqual(stringifiedMock)
 
     removeSession()
 
-    expect(hardtack.remove).toHaveBeenCalledWith(COOKIE_SESSION_KEY, {
-      path: '/',
-      domain: window.location.hostname,
-    })
+    expect(window.localStorage.getItem(COOKIE_SESSION_KEY)).toBeFalsy()
   })
+
   it('should remove a session cookie with appEnv', () => {
-    window.location.hostname = 'something.reapit.com'
-    hardtack.remove = jest.fn()
+    window.localStorage.setItem(`development-${COOKIE_SESSION_KEY}`, stringifiedMock)
+
+    expect(window.localStorage.getItem(`development-${COOKIE_SESSION_KEY}`)).toEqual(stringifiedMock)
 
     removeSession(COOKIE_SESSION_KEY, 'development')
 
-    expect(hardtack.remove).toHaveBeenCalledWith(`development-${COOKIE_SESSION_KEY}`, {
-      path: '/',
-      domain: window.location.hostname,
-    })
+    expect(window.localStorage.getItem(COOKIE_SESSION_KEY)).toBeFalsy()
   })
 })
diff --git a/packages/cognito-auth/src/session/remove-session.ts b/packages/cognito-auth/src/session/remove-session.ts
@@ -1,10 +1,7 @@
 import { COOKIE_SESSION_KEY } from '../utils/cognito'
-import hardtack from 'hardtack'
 
 export const removeSession = (identifier: string = COOKIE_SESSION_KEY, appEnv?: string): void => {
   const identifierWithEnv = appEnv ? `${appEnv}-${identifier}` : identifier
-  hardtack.remove(identifierWithEnv, {
-    path: '/',
-    domain: window.location.hostname,
-  })
+
+  window.localStorage.removeItem(identifierWithEnv)
 }
diff --git a/packages/cognito-auth/src/utils/cognito.test.ts b/packages/cognito-auth/src/utils/cognito.test.ts
@@ -8,7 +8,6 @@ import {
   deserializeIdToken,
   tokenExpired,
   checkHasIdentityId,
-  COOKIE_EXPIRY,
   redirectToOAuth,
   redirectToLogin,
 } from './cognito'
@@ -68,21 +67,14 @@ describe('Session utils', () => {
 
       setSessionCookie(mockLoginSession, COOKIE_SESSION_KEY, 'development')
 
-      expect(hardtack.set).toHaveBeenCalledWith(
-        `development-${COOKIE_SESSION_KEY}`,
+      expect(window.localStorage.getItem(`development-${COOKIE_SESSION_KEY}`)).toEqual(
         JSON.stringify({
           refreshToken: mockLoginSession.refreshToken,
           loginType: mockLoginSession.loginType,
           userName: mockLoginSession.userName,
           mode: 'WEB',
           cognitoClientId: 'SOME_CLIENT_ID',
         }),
-        {
-          path: '/',
-          domain: 'some.host',
-          expires: COOKIE_EXPIRY,
-          samesite: 'lax',
-        },
       )
     })
   })
@@ -96,7 +88,7 @@ describe('Session utils', () => {
         userName: mockLoginSession.userName,
       })
 
-      document.cookie = `development-${COOKIE_SESSION_KEY}=${stringifiedSession}`
+      window.localStorage.setItem(`development-${COOKIE_SESSION_KEY}`, stringifiedSession)
 
       expect(getSessionCookie(COOKIE_SESSION_KEY, 'development')).toEqual(JSON.parse(stringifiedSession))
     })
@@ -109,7 +101,7 @@ describe('Session utils', () => {
         userName: mockLoginSession.userName,
       })
 
-      document.cookie = `development-${COOKIE_SESSION_KEY}=${stringifiedSession}`
+      window.localStorage.setItem(`development-${COOKIE_SESSION_KEY}`, stringifiedSession)
 
       expect(getSessionCookie(COOKIE_SESSION_KEY, 'development')).toEqual(JSON.parse(stringifiedSession))
     })
@@ -123,13 +115,13 @@ describe('Session utils', () => {
         userName: mockLoginSession.userName,
       })
 
-      document.cookie = `${COOKIE_SESSION_KEY}=${stringifiedSession}`
+      window.localStorage.setItem(`${COOKIE_SESSION_KEY}`, stringifiedSession)
 
       expect(getSessionCookie()).toEqual(JSON.parse(stringifiedSession))
     })
 
     it('should return null if no cookie', () => {
-      document.cookie = `development-${COOKIE_SESSION_KEY}=`
+      window.localStorage.clear()
 
       expect(getSessionCookie(COOKIE_SESSION_KEY, 'development')).toBeNull()
     })

diff --git a/packages/cognito-auth/src/utils/cognito.ts b/packages/cognito-auth/src/utils/cognito.ts
@@ -1,4 +1,3 @@
-import hardtack from 'hardtack'
 import jwt from 'jsonwebtoken'
 import { CognitoUserPool, CognitoUser, CognitoUserSession } from 'amazon-cognito-identity-js'
 import { LoginSession, RefreshParams, LoginType, LoginIdentity, CoginitoIdentity } from '../core/types'
@@ -32,35 +31,37 @@ export const getNewUser = (userName: string, cognitoClientId: string, userPoolId
   return new CognitoUser(userData)
 }
 
+/** TODO - deprecate these methods at the next major release
+ * Have kept the names as get and set cookie however, we are now using localstorage because the length
+ * of the Refresh token is now too big (over 4kb), to store in a cookie. That was a day wasted.
+ * Obviously we are now using localstorage, will update to reflect this later, this to solve prod bug
+ * and not break third party auth
+ */
+
 export const setSessionCookie = (
   session: LoginSession,
   identifier: string = COOKIE_SESSION_KEY,
   appEnv?: string,
 ): void => {
   const { userName, refreshToken, loginType, mode, cognitoClientId } = session
   const identifierWithEnv = appEnv ? `${appEnv}-${identifier}` : identifier
-  hardtack.set(
-    identifierWithEnv,
-    JSON.stringify({
-      refreshToken,
-      loginType,
-      userName,
-      mode,
-      cognitoClientId,
-    }),
-    {
-      path: '/',
-      domain: window.location.hostname,
-      expires: COOKIE_EXPIRY,
-      samesite: 'lax',
-    },
-  )
+
+  const cookie = JSON.stringify({
+    refreshToken,
+    loginType,
+    userName,
+    mode,
+    cognitoClientId,
+  })
+
+  window.localStorage.setItem(identifierWithEnv, cookie)
 }
 
 export const getSessionCookie = (identifier: string = COOKIE_SESSION_KEY, appEnv?: string): RefreshParams | null => {
   try {
     const identifierWithEnv = appEnv ? `${appEnv}-${identifier}` : identifier
-    const session = hardtack.get(identifierWithEnv)
+    const session = window.localStorage.getItem(identifierWithEnv)
+
     if (session) {
       const marketplaceGlobalObject = getMarketplaceGlobalsByKey()
       const mode = marketplaceGlobalObject ? 'DESKTOP' : 'WEB'

diff --git a/packages/marketplace/src/core/__tests__/__snapshots__/private-route.tsx.snap b/packages/marketplace/src/core/__tests__/__snapshots__/private-route.tsx.snap
@@ -58,13 +58,7 @@ exports[`PrivateRouter should match a snapshot 1`] = `
       <Component
         allow="CLIENT"
         component={[Function]}
-      >
-        <Route
-          render={[Function]}
-        >
-          <component />
-        </Route>
-      </Component>
+      />
     </Router>
   </MemoryRouter>
 </Provider>

diff --git a/packages/marketplace/src/core/__tests__/private-route.tsx b/packages/marketplace/src/core/__tests__/private-route.tsx
@@ -82,7 +82,7 @@ describe('PrivateRouter', () => {
         clientId: '',
         developerId: 'testDeveloperId',
       } as LoginIdentity
-      const fn = handleChangeLoginType('CLIENT', 'DEVELOPER', spyDispatch, mockLoginIdentity)
+      const fn = handleChangeLoginType('CLIENT', 'DEVELOPER', spyDispatch, mockLoginIdentity, false)
       fn()
       expect(spyDispatch).toBeCalledWith(authChangeLoginType('DEVELOPER'))
     })
@@ -91,10 +91,21 @@ describe('PrivateRouter', () => {
         clientId: 'testClient',
         developerId: '',
       } as LoginIdentity
-      const fn = handleChangeLoginType('DEVELOPER', 'CLIENT', spyDispatch, mockLoginIdentity)
+      const fn = handleChangeLoginType('DEVELOPER', 'CLIENT', spyDispatch, mockLoginIdentity, false)
       fn()
       expect(spyDispatch).toBeCalledWith(authChangeLoginType('CLIENT'))
     })
+
+    it('should dispatch authChangeLoginType to change loginType to CLIENT', () => {
+      const mockDispatch = jest.fn()
+      const mockLoginIdentity = {
+        clientId: 'testClient',
+        developerId: '',
+      } as LoginIdentity
+      const fn = handleChangeLoginType('DEVELOPER', 'CLIENT', mockDispatch, mockLoginIdentity, true)
+      fn()
+      expect(mockDispatch).not.toBeCalled()
+    })
   })
 
   describe('handleRedirectToAuthenticationPage', () => {

diff --git a/packages/marketplace/src/core/private-route.tsx b/packages/marketplace/src/core/private-route.tsx
@@ -34,16 +34,19 @@ export const handleChangeLoginType = (
   allow: LoginType | LoginType[],
   dispatch: Dispatch,
   loginIdentity?: LoginIdentity,
+  isFetchingAccessToken?: boolean,
 ) => {
   return () => {
-    if (!loginIdentity) {
+    if (!loginIdentity || isFetchingAccessToken) {
       return
     }
     if (loginType === 'CLIENT' && allow === 'DEVELOPER' && loginIdentity.developerId) {
       dispatch(authChangeLoginType('DEVELOPER'))
+      return
     }
     if (loginType === 'DEVELOPER' && allow === 'CLIENT' && loginIdentity.clientId) {
       dispatch(authChangeLoginType('CLIENT'))
+      return
     }
   }
 }
@@ -52,9 +55,10 @@ export const handleRedirectToAuthenticationPage = (
   allow: LoginType | LoginType[],
   history: History,
   loginIdentity?: LoginIdentity,
+  isFetchingAccessToken?: boolean,
 ) => {
   return () => {
-    if (!loginIdentity) {
+    if (!loginIdentity || isFetchingAccessToken) {
       return
     }
     const { clientId, developerId } = loginIdentity
@@ -69,22 +73,36 @@ export const fetchAccessToken = async () => {
 }
 
 export const PrivateRoute = ({ component, allow, fetcher = false, ...rest }: PrivateRouteProps & RouteProps) => {
+  const [isFetchingAccessToken, setFetchingAccessToken] = React.useState(true)
   const dispatch = useDispatch()
   const history = useHistory()
   const loginIdentity = useSelector(selectLoginIdentity)
   const loginType = useSelector(selectLoginType)
 
-  React.useEffect(handleChangeLoginType(loginType, allow, dispatch, loginIdentity), [
+  React.useEffect(() => {
+    fetchAccessToken().then(() => {
+      setFetchingAccessToken(false)
+    })
+  }, [])
+
+  React.useEffect(handleChangeLoginType(loginType, allow, dispatch, loginIdentity, isFetchingAccessToken), [
     allow,
     dispatch,
     loginIdentity,
     loginType,
+    isFetchingAccessToken,
   ])
 
-  React.useEffect(handleRedirectToAuthenticationPage(allow, history, loginIdentity), [loginIdentity, allow, history])
-
-  fetchAccessToken()
+  React.useEffect(handleRedirectToAuthenticationPage(allow, history, loginIdentity, isFetchingAccessToken), [
+    loginIdentity,
+    allow,
+    history,
+    isFetchingAccessToken,
+  ])
 
+  if (isFetchingAccessToken) {
+    return null
+  }
   return (
     <Route
       {...rest}

diff --git a/scripts/jest/jest.config.js b/scripts/jest/jest.config.js
@@ -19,7 +19,6 @@ module.exports = {
   moduleFileExtensions: [...defaults.moduleFileExtensions, 'ts', 'tsx', 'graphql', 'gql'],
   snapshotSerializers: ['enzyme-to-json/serializer'],
   verbose: false,
-  bail: 1,
   projects: ['<rootDir>/jest.config.js'],
   transform: {
     '^.+\\.svg$': '<rootDir>/../../scripts/jest/svg-transform.js',