fix: Reseting server auth state during login cancellation (#20546)

* WIP * Canceling in progress login mutation during reset. Updating state handling for dual launchpad/app LoginModal presentation. * Updating reset logic to better support error cases. Updating existing tests. * Tweaking test, extra commit should get us better percy snapshots * Reverting change to network policy for HeaderBar query, I don't think it's necessary * Addressing PR feedback * Throwing instead * Updating tests (subscription updates are much faster!)
2026-01-26 00:49:05 -06:00 · 2022-03-14 19:39:10 -05:00
parent b0ab66e123
commit efa2b88f69
9 changed files with 213 additions and 154 deletions
--- a/packages/app/cypress/e2e/top-nav.cy.ts
+++ b/packages/app/cypress/e2e/top-nav.cy.ts
@@ -1,5 +1,5 @@
+import type { SinonStub } from 'sinon'
 import defaultMessages from '@packages/frontend-shared/src/locales/en-US.json'
-import type { AuthStateShape } from '@packages/data-context/src/data'

 const pkg = require('@packages/root')

@@ -366,12 +366,14 @@ describe('App Top Nav Workflows', () => {
      const mockLogInActionsForUser = (user) => {
        cy.withCtx((ctx, options) => {
          options.sinon.stub(ctx._apis.authApi, 'logIn').callsFake(async (onMessage) => {
-            onMessage({ browserOpened: true } as AuthStateShape)
+            setTimeout(() => {
+              onMessage({ browserOpened: true })
+            }, 500)

            return new Promise((resolve) => {
              setTimeout(() => {
                resolve(options.user)
-              }, 2000) // timeout ensures full auth browser lifecycle is testable
+              }, 1000)
            })
          })
        }, { user })
@@ -445,12 +447,16 @@ describe('App Top Nav Workflows', () => {
      })

      it('shows correct error when browser cannot launch', () => {
-        cy.withCtx((ctx) => {
-          ctx.coreData.authState = {
-            name: 'AUTH_COULD_NOT_LAUNCH_BROWSER',
-            message: 'http://127.0.0.1:0000/redirect-to-auth',
-            browserOpened: false,
-          }
+        cy.withCtx((ctx, o) => {
+          o.sinon.stub(ctx._apis.authApi, 'logIn').callsFake(async (onMessage) => {
+            onMessage({
+              name: 'AUTH_COULD_NOT_LAUNCH_BROWSER',
+              message: 'http://127.0.0.1:0000/redirect-to-auth',
+              browserOpened: false,
+            })
+
+            throw new Error()
+          })
        })

        cy.findByTestId('app-header-bar').within(() => {
@@ -458,23 +464,31 @@ describe('App Top Nav Workflows', () => {
          cy.findByRole('button', { name: 'Log In' }).click()
        })

-        cy.contains('http://127.0.0.1:0000/redirect-to-auth').should('be.visible')
-        cy.contains(loginText.titleBrowserError).should('be.visible')
-        cy.contains(loginText.bodyBrowserError).should('be.visible')
-        cy.contains(loginText.bodyBrowserErrorDetails).should('be.visible')
+        cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
+          cy.findByRole('button', { name: 'Log In' }).click()

-        // in this state, there is no retry UI, we ask the user to visit the auth url on their own
-        cy.contains('button', loginText.actionTryAgain).should('not.exist')
-        cy.contains('button', loginText.actionCancel).should('not.exist')
+          cy.contains('http://127.0.0.1:0000/redirect-to-auth').should('be.visible')
+          cy.contains(loginText.titleBrowserError).should('be.visible')
+          cy.contains(loginText.bodyBrowserError).should('be.visible')
+          cy.contains(loginText.bodyBrowserErrorDetails).should('be.visible')
+
+          // in this state, there is no retry UI, we ask the user to visit the auth url on their own
+          cy.contains('button', loginText.actionTryAgain).should('not.be.visible')
+          cy.contains('button', loginText.actionCancel).should('not.be.visible')
+        })
      })

      it('shows correct error when error other than browser-launch happens', () => {
-        cy.withCtx((ctx) => {
-          ctx.coreData.authState = {
-            name: 'AUTH_ERROR_DURING_LOGIN',
-            message: 'An unexpected error occurred',
-            browserOpened: false,
-          }
+        cy.withCtx((ctx, o) => {
+          o.sinon.stub(ctx._apis.authApi, 'logIn').callsFake(async (onMessage) => {
+            onMessage({
+              name: 'AUTH_ERROR_DURING_LOGIN',
+              message: 'An unexpected error occurred',
+              browserOpened: false,
+            })
+
+            throw new Error()
+          })
        })

        cy.findByTestId('app-header-bar').within(() => {
@@ -482,34 +496,49 @@ describe('App Top Nav Workflows', () => {
          cy.findByRole('button', { name: 'Log In' }).click()
        })

-        cy.contains(loginText.titleFailed).should('be.visible')
-        cy.contains(loginText.bodyError).should('be.visible')
-        cy.contains('An unexpected error occurred').should('be.visible')
+        cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
+          cy.findByRole('button', { name: 'Log In' }).click()

-        cy.contains('button', loginText.actionTryAgain).should('be.visible').as('tryAgain')
-        cy.contains('button', loginText.actionCancel).should('be.visible')
+          cy.contains(loginText.titleFailed).should('be.visible')
+          cy.contains(loginText.bodyError).should('be.visible')
+          cy.contains('An unexpected error occurred').should('be.visible')
+
+          cy.contains('button', loginText.actionTryAgain).should('be.visible').as('tryAgain')
+          cy.contains('button', loginText.actionCancel).should('be.visible')
+        })

        cy.percySnapshot()

        cy.withCtx((ctx) => {
-          ctx.coreData.authState = {
-            name: 'AUTH_BROWSER_LAUNCHED',
-            message: '',
-            browserOpened: true,
-          }
+          (ctx._apis.authApi.logIn as SinonStub).callsFake(async (onMessage) => {
+            onMessage({
+              name: 'AUTH_BROWSER_LAUNCHED',
+              message: '',
+              browserOpened: true,
+            })
+
+            return Promise.resolve()
+          })
        })

        cy.get('@tryAgain').click()
-        cy.contains(loginText.titleInitial).should('be.visible')
+
+        cy.findByRole('dialog', { name: loginText.titleInitial }).within(() => {
+          cy.contains(loginText.actionWaiting).should('be.visible')
+        })
      })

      it('cancel button correctly clears error state', () => {
-        cy.withCtx((ctx) => {
-          ctx.coreData.authState = {
-            name: 'AUTH_ERROR_DURING_LOGIN',
-            message: 'An unexpected error occurred',
-            browserOpened: false,
-          }
+        cy.withCtx((ctx, o) => {
+          o.sinon.stub(ctx._apis.authApi, 'logIn').callsFake(async (onMessage) => {
+            onMessage({
+              name: 'AUTH_ERROR_DURING_LOGIN',
+              message: 'An unexpected error occurred',
+              browserOpened: false,
+            })
+
+            throw new Error()
+          })
        })

        cy.findByTestId('app-header-bar').within(() => {
@@ -517,26 +546,36 @@ describe('App Top Nav Workflows', () => {
          cy.findByRole('button', { name: 'Log In' }).as('loginButton').click()
        })

-        cy.contains(loginText.titleFailed).should('be.visible')
-        cy.contains(loginText.bodyError).should('be.visible')
-        cy.contains('An unexpected error occurred').should('be.visible')
+        cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
+          cy.findByRole('button', { name: 'Log In' }).click()
+
+          cy.contains(loginText.titleFailed).should('be.visible')
+          cy.contains(loginText.bodyError).should('be.visible')
+          cy.contains('An unexpected error occurred').should('be.visible')
+        })

        cy.percySnapshot()

-        cy.contains('button', loginText.actionTryAgain).should('be.visible')
-        cy.contains('button', loginText.actionCancel).click()
+        cy.findByRole('dialog', { name: loginText.titleFailed }).within(() => {
+          cy.contains('button', loginText.actionTryAgain).should('be.visible')
+          cy.contains('button', loginText.actionCancel).click()
+        })

        cy.get('@loginButton').click()
        cy.contains(loginText.titleInitial).should('be.visible')
      })

      it('closing modal correctly clears error state', () => {
-        cy.withCtx((ctx) => {
-          ctx.coreData.authState = {
-            name: 'AUTH_ERROR_DURING_LOGIN',
-            message: 'An unexpected error occurred',
-            browserOpened: false,
-          }
+        cy.withCtx((ctx, o) => {
+          o.sinon.stub(ctx._apis.authApi, 'logIn').callsFake(async (onMessage) => {
+            onMessage({
+              name: 'AUTH_ERROR_DURING_LOGIN',
+              message: 'An unexpected error occurred',
+              browserOpened: false,
+            })
+
+            throw new Error()
+          })
        })

        cy.findByTestId('app-header-bar').within(() => {
@@ -544,11 +583,14 @@ describe('App Top Nav Workflows', () => {
          cy.findByRole('button', { name: 'Log In' }).as('loginButton').click()
        })

-        cy.contains(loginText.titleFailed).should('be.visible')
-        cy.contains(loginText.bodyError).should('be.visible')
-        cy.contains('An unexpected error occurred').should('be.visible')
+        cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
+          cy.findByRole('button', { name: 'Log In' }).click()
+          cy.contains(loginText.titleFailed).should('be.visible')
+          cy.contains(loginText.bodyError).should('be.visible')
+          cy.contains('An unexpected error occurred').should('be.visible')

-        cy.findByLabelText(defaultMessages.actions.close).click()
+          cy.findByLabelText(defaultMessages.actions.close).click()
+        })

        cy.get('@loginButton').click()
        cy.contains(loginText.titleInitial).should('be.visible')
--- a/packages/data-context/src/actions/AuthActions.ts
+++ b/packages/data-context/src/actions/AuthActions.ts
@@ -5,7 +5,7 @@ export interface AuthApiShape {
  getUser(): Promise<Partial<AuthenticatedUserShape>>
  logIn(onMessage: (message: AuthStateShape) => void): Promise<AuthenticatedUserShape>
  logOut(): Promise<void>
-  resetAuthState(): Promise<void>
+  resetAuthState(): void
 }

 export class AuthActions {
@@ -44,19 +44,62 @@ export class AuthActions {
  }

  async login () {
-    this.setAuthenticatedUser(await this.authApi.logIn((authState) => {
+    const loginPromise = new Promise<AuthenticatedUserShape | null>((resolve, reject) => {
+      // A resolver is exposed to the instance so that we can
+      // resolve this promise and the original mutation promise
+      // if a reset occurs
      this.ctx.update((coreData) => {
-        coreData.authState = authState
+        coreData.cancelActiveLogin = () => resolve(null)
      })
-    }))

-    this.ctx.emitter.authChange()
+      this.authApi.logIn((authState) => {
+        this.ctx.update((coreData) => {
+          coreData.authState = authState
+        })
+
+        // Ensure auth state changes during the login lifecycle
+        // are propagated to the clients
+        this.ctx.emitter.authChange()
+      }).then(resolve, reject)
+    })
+
+    const user = await loginPromise
+
+    if (!user) {
+      // if the user is null, this promise is resolving due to a
+      // login mutation cancellation. the state should already
+      // be reset, so abort early.
+      return
+    }
+
+    this.setAuthenticatedUser(user as AuthenticatedUserShape)
+
+    this.ctx.update((coreData) => {
+      coreData.cancelActiveLogin = null
+    })
+
+    this.resetAuthState()
  }

  resetAuthState () {
+    // closes the express server opened during login, if it's still open
+    this.authApi.resetAuthState()
+
+    // if a login mutation is still in progress, we
+    // forcefully resolve it so that the mutation does not persist
+    if (this.ctx.coreData.cancelActiveLogin) {
+      this.ctx.coreData.cancelActiveLogin()
+
+      this.ctx.update((coreData) => {
+        coreData.cancelActiveLogin = null
+      })
+    }
+
    this.ctx.update((coreData) => {
      coreData.authState = { browserOpened: false }
    })
+
+    this.ctx.emitter.authChange()
  }

  async logout () {
@@ -76,7 +119,9 @@ export class AuthActions {
  }

  private setAuthenticatedUser (authUser: AuthenticatedUserShape | null) {
-    this.ctx.coreData.user = authUser
+    this.ctx.update((coreData) => {
+      coreData.user = authUser
+    })

    return this
  }
--- a/packages/data-context/src/data/coreDataShape.ts
+++ b/packages/data-context/src/data/coreDataShape.ts
@@ -138,6 +138,7 @@ export interface CoreDataShape {
  warnings: ErrorWrapperSource[]
  packageManager: typeof PACKAGE_MANAGERS[number]
  forceReconfigureProject: ForceReconfigureProjectDataShape | null
+  cancelActiveLogin: (() => void) | null
 }

 /**
@@ -209,5 +210,6 @@ export function makeCoreData (modeOptions: Partial<AllModeOptions> = {}): CoreDa
    scaffoldedFiles: null,
    packageManager: 'npm',
    forceReconfigureProject: null,
+    cancelActiveLogin: null,
  }
 }
--- a/packages/frontend-shared/src/gql-components/Auth.vue
+++ b/packages/frontend-shared/src/gql-components/Auth.vue
@@ -34,37 +34,45 @@
  </div>
  <div v-else>
    <Button
-      ref="loginButtonRef"
+      v-if="loginMutationIsPending"
      size="lg"
-      :variant="buttonVariant"
-      :disabled="isLoggingIn || !isOnline"
+      variant="pending"
      aria-live="polite"
-      @click="handleAuth"
+      :disabled="true"
    >
      <template
-        v-if="isLoggingIn"
        #prefix
      >
        <i-cy-loading_x16
          class="animate-spin icon-dark-white icon-light-gray-400"
        />
      </template>
-      {{ buttonMessage }}
+      {{ browserOpened ? t('topNav.login.actionWaiting') : t('topNav.login.actionOpening') }}
+    </Button>
+    <Button
+      v-else
+      ref="loginButtonRef"
+      size="lg"
+      variant="primary"
+      aria-live="polite"
+      :disabled="!cloudViewer && !isOnline"
+      @click="handleLoginOrContinue"
+    >
+      {{ cloudViewer ? t('topNav.login.actionContinue') : t('topNav.login.actionLogin') }}
    </Button>
  </div>
 </template>

 <script lang="ts" setup>
-import { computed, ref, onMounted } from 'vue'
+import { computed, ref, onMounted, onBeforeUnmount } from 'vue'
 import { gql } from '@urql/core'
-import { useMutation, useQuery } from '@urql/vue'
+import { useMutation } from '@urql/vue'
 import { useOnline } from '@vueuse/core'

 import {
  Auth_LoginDocument,
  Auth_LogoutDocument,
  Auth_ResetAuthStateDocument,
-  Auth_BrowserOpenedDocument,
 } from '../generated/graphql'
 import type {
  AuthFragment,
@@ -119,99 +127,76 @@ mutation Auth_ResetAuthState {
 }
 `

-gql`
-query Auth_BrowserOpened {
-  authState {
-    browserOpened
-    name
-    message
-  }
-}
-`
-
 const login = useMutation(Auth_LoginDocument)
 const logout = useMutation(Auth_LogoutDocument)
 const reset = useMutation(Auth_ResetAuthStateDocument)
+
 const loginButtonRef = ref(Button)
+const loginInitiated = ref(false)

 onMounted(() => {
  loginButtonRef?.value?.$el?.focus()
 })

-const clickedOnce = ref(false)
+onBeforeUnmount(() => {
+  // If a log in was initiated from this component instance, then the auth state
+  // may be polluted, due to the mutation still being fetched or due to
+  // errors returned during the login process. So a reset occurs when
+  // this instance unmounts to cover all scenarios where the LoginModal may be dismissed.
+  //
+  // We only perform the reset for the component that triggered a login
+  // to prevent state conflicts when LoginModals are presented within the launchpad
+  // and app simultaneously.
+  if (loginInitiated.value) {
+    reset.executeMutation({})
+  }
+})

 const emit = defineEmits<{
  (event: 'continue', value: boolean): void
 }>()

-const viewer = computed(() => props.gql.cloudViewer)
-const isBrowserOpened = computed(() => props.gql.authState.browserOpened)
-const isLoggingIn = computed(() => clickedOnce.value && !viewer.value)
-
-const query = useQuery({
-  query: Auth_BrowserOpenedDocument,
-  requestPolicy: 'cache-and-network',
+const cloudViewer = computed(() => {
+  return props.gql.cloudViewer
 })

-const handleAuth = async () => {
-  if (viewer.value) {
+const browserOpened = computed(() => {
+  return props.gql.authState.browserOpened
+})
+
+// We determine that a login is pending if there is no current cloudViewer and
+// either a login has been initiated from this component, or the browser has been
+// successfully opened.
+//
+// It is possible for the browser to be open but not due to actions by this component,
+// particularly when LoginModals are presented in both the launchpad and app simultaneously.
+const loginMutationIsPending = computed(() => {
+  return !cloudViewer.value && (loginInitiated.value || browserOpened.value)
+})
+
+const handleLoginOrContinue = async () => {
+  if (cloudViewer.value) {
    emit('continue', true)

    return
  }

-  clickedOnce.value = true
+  loginInitiated.value = true

-  const browserCheckInterval = setInterval(async () => {
-    await query.executeQuery({})
-    if (isBrowserOpened.value) {
-      clearInterval(browserCheckInterval)
-    }
-  }, 1500)
-
-  await login.executeMutation({})
+  login.executeMutation({})
 }

-const handleLogout = async () => {
-  await logout.executeMutation({})
+const handleLogout = () => {
+  logout.executeMutation({})
 }

-const buttonMessage = computed(() => {
-  if (!isBrowserOpened.value && isLoggingIn.value) {
-    return t('topNav.login.actionOpening')
-  }
+const handleTryAgain = async () => {
+  await reset.executeMutation({})

-  if (!clickedOnce.value && !viewer.value) {
-    return t('topNav.login.actionLogin')
-  }
-
-  if (isLoggingIn.value) {
-    return t('topNav.login.actionWaiting')
-  }
-
-  if (viewer.value) {
-    return t('topNav.login.actionContinue')
-  }
-
-  // default
-  return t('topNav.login.actionLogin')
-})
-
-const buttonVariant = computed(() => {
-  if (clickedOnce.value && !viewer.value) {
-    return 'pending'
-  }
-
-  return 'primary'
-})
-
-const handleTryAgain = () => {
-  reset.executeMutation({})
  login.executeMutation({})
 }

 const handleCancel = () => {
-  reset.executeMutation({})
  emit('continue', true)
 }

--- a/packages/frontend-shared/src/gql-components/topnav/LoginModal.cy.tsx
+++ b/packages/frontend-shared/src/gql-components/topnav/LoginModal.cy.tsx
@@ -62,8 +62,8 @@ describe('<LoginModal />', { viewportWidth: 1000, viewportHeight: 750 }, () => {
        },
      })

-      cy.findByRole('button', { name: text.login.actionLogin }).click()
-
+      // The LoginModal immediately shows the "Waiting..." button
+      // if the browser already opened
      cy.findByRole('button', { name: text.login.actionWaiting })
      .should('be.visible')
      .and('be.disabled')
@@ -93,8 +93,8 @@ describe('<LoginModal />', { viewportWidth: 1000, viewportHeight: 750 }, () => {
          </div>),
      })

-      cy.contains('button', text.login.actionTryAgain).should('not.exist')
-      cy.contains('button', text.login.actionCancel).should('not.exist')
+      cy.contains('button', text.login.actionTryAgain).should('not.be.visible')
+      cy.contains('button', text.login.actionCancel).should('not.be.visible')
      cy.contains(text.login.titleBrowserError).should('be.visible')
      cy.contains(text.login.bodyBrowserError).should('be.visible')
      cy.contains(text.login.bodyBrowserErrorDetails).should('be.visible')
--- a/packages/frontend-shared/src/gql-components/topnav/LoginModal.vue
+++ b/packages/frontend-shared/src/gql-components/topnav/LoginModal.vue
@@ -72,10 +72,9 @@
            </div>
          </div>
        </DialogDescription>
-
        <div
-          v-if="showFooter"
          class="bg-gray-50 border-t-1px py-16px px-24px"
+          :class="{ 'hidden': !showFooter }"
        >
          <Auth
            :gql="props.gql"
@@ -97,8 +96,6 @@ import { useOnline } from '@vueuse/core'
 import NoInternetConnection from '../../components/NoInternetConnection.vue'
 import CopyText from '@cy/components/CopyText.vue'
 import StandardModalHeader from '@cy/components/StandardModalHeader.vue'
-import { useMutation } from '@urql/vue'
-import { LoginModal_ResetAuthStateDocument } from '../../generated/graphql'

 import {
  Dialog,
@@ -125,21 +122,8 @@ fragment LoginModal on Query {
 }
 `

-gql`
-mutation LoginModal_ResetAuthState {
-  resetAuthState {
-    ...Auth
-  }
-}
-`
-
-const resetAuth = useMutation(LoginModal_ResetAuthStateDocument)
-
 const setIsOpen = (value: boolean) => {
  emit('update:modelValue', value)
-  if (!value) {
-    resetAuth.executeMutation({})
-  }
 }
 const { t } = useI18n()

--- a/packages/graphql/src/schemaTypes/objectTypes/gql-Mutation.ts
+++ b/packages/graphql/src/schemaTypes/objectTypes/gql-Mutation.ts
@@ -346,7 +346,7 @@ export const mutation = mutationType({
      resolve (_, args, ctx) {
        ctx.actions.auth.resetAuthState()

-        return ctx.appData
+        return {}
      },
    })

--- a/packages/server/lib/gui/auth.ts
+++ b/packages/server/lib/gui/auth.ts
@@ -228,5 +228,6 @@ const start = (onMessage, utmCode, onLoginFlowComplete) => {

 export = {
  start,
+  stopServer,
  _internal,
 }
--- a/packages/server/lib/makeDataContext.ts
+++ b/packages/server/lib/makeDataContext.ts
@@ -92,7 +92,7 @@ export function makeDataContext (options: MakeDataContextOptions): DataContext {
        return user.logOut()
      },
      resetAuthState () {
-        return ctx.actions.auth.resetAuthState()
+        auth.stopServer()
      },
    },
    projectApi: {