From 2ce2524721c411ffdf47417bb88c1de4c8bbbc66 Mon Sep 17 00:00:00 2001 From: Tyler Biethman Date: Mon, 10 Jan 2022 12:30:03 -0600 Subject: [PATCH] test(unify): Additional navigation tests for log in workflows (#19574) * test(unify): Additional navigation tests for log in workflows * Couple tweaks for better coverage and dryer tests * Adding unit test coverage for window focusing after auth completion. * Thank you tslint * Updating LoginModal component test * Adding PR feedback --- .../e2e/integration/navigation.spec.ts | 20 --- .../cypress/e2e/integration/top-nav.spec.ts | 105 +++++++++++++-- .../gql-components/topnav/LoginModal.spec.tsx | 24 +++- .../src/gql-components/topnav/LoginModal.vue | 2 +- .../src/gql-components/topnav/UserAvatar.vue | 1 + packages/server/lib/gui/auth.ts | 121 ++++++++++-------- packages/server/test/unit/gui/auth_spec.js | 62 +++++++-- 7 files changed, 234 insertions(+), 101 deletions(-) delete mode 100644 packages/app/cypress/e2e/integration/navigation.spec.ts diff --git a/packages/app/cypress/e2e/integration/navigation.spec.ts b/packages/app/cypress/e2e/integration/navigation.spec.ts deleted file mode 100644 index 8c6b3a6af9..0000000000 --- a/packages/app/cypress/e2e/integration/navigation.spec.ts +++ /dev/null @@ -1,20 +0,0 @@ -import defaultMessages from '@packages/frontend-shared/src/locales/en-US.json' - -describe('Navigation', () => { - before(() => { - cy.scaffoldProject('todos') - }) - - it('External links trigger mutation to open in a new browser', () => { - cy.openProject('todos') - cy.startAppServer() - cy.visitApp() - - cy.contains('button', defaultMessages.topNav.docsMenu.docsHeading).click() - - cy.validateExternalLink({ - name: defaultMessages.topNav.docsMenu.firstTest, - href: 'https://on.cypress.io/writing-first-test?utm_medium=Docs+Menu&utm_content=First+Test', - }) - }) -}) diff --git a/packages/app/cypress/e2e/integration/top-nav.spec.ts b/packages/app/cypress/e2e/integration/top-nav.spec.ts index a64714de58..47d7e300bb 100644 --- a/packages/app/cypress/e2e/integration/top-nav.spec.ts +++ b/packages/app/cypress/e2e/integration/top-nav.spec.ts @@ -268,7 +268,7 @@ describe('App Top Nav Workflows', () => { }) describe('Login', () => { - context('user logged in', () => { + context('user logged in at launch', () => { beforeEach(() => { cy.findBrowsers() cy.openProject('launchpad') @@ -289,31 +289,118 @@ describe('App Top Nav Workflows', () => { name: 'Profile Settings', href: 'https://on.cypress.io/dashboard/profile', }) + }) + + it('replaces user avatar after logout', () => { + cy.get('@logInButton').click() + + cy.withCtx((ctx) => { + sinon.stub(ctx._apis.authApi, 'logOut').callsFake(async () => { + // resolves + }) + }) cy.intercept('mutation-Logout').as('logout') - cy.findByRole('button', { name: 'Log Out' }).should('be.visible').click() + cy.findByRole('button', { name: 'Log Out' }).click() cy.wait('@logout') + + cy.findByTestId('app-header-bar').findByText('Log In').should('be.visible') }) }) context('user not logged in', () => { + const mockUser = { + authToken: 'test1', + email: 'test_user_a@example.com', + name: 'Test User A', + } + + const mockUserNoName = { + authToken: 'test22', + email: 'test_user_b@example.com', + } + + const mockLogInActionsForUser = (user) => { + cy.withCtx((ctx, options) => { + sinon.stub(ctx._apis.authApi, 'logIn').callsFake(async (onMessage) => { + onMessage({ browserOpened: true }) + + return new Promise((resolve) => { + setTimeout(() => { + resolve(options.user) + }, 2000) // timeout ensures full auth browser lifecycle is testable + }) + }) + }, { user }) + } + beforeEach(() => { cy.findBrowsers() cy.openProject('launchpad') cy.startAppServer() cy.visitApp() - - cy.findByTestId('app-header-bar').findByRole('button', { name: 'Log In' }).as('logInButton') }) - it('shows log in modal when button is pressed', () => { - cy.get('@logInButton').click() + it('shows log in modal workflow for user with name and email', () => { + mockLogInActionsForUser(mockUser) - cy.findByRole('dialog', { name: 'Log in to Cypress' }).as('logInModal') - cy.get('@logInModal').findByRole('button', { name: 'Log In' }) - cy.get('@logInModal').findByRole('button', { name: 'Close' }).click() + cy.findByTestId('app-header-bar').within(() => { + cy.findByTestId('user-avatar').should('not.exist') + cy.findByRole('button', { name: 'Log In' }).click() + }) + + cy.findByRole('dialog', { name: 'Log in to Cypress' }).as('logInModal').within(() => { + cy.findByRole('button', { name: 'Log In' }).click() + + // The Log In button transitions through a few states as the browser launch lifecycle completes + cy.findByRole('button', { name: 'Opening Browser' }).should('be.visible').and('be.disabled') + cy.findByRole('button', { name: 'Waiting for you to log in' }).should('be.visible').and('be.disabled') + }) + + cy.findByRole('dialog', { name: 'Login Successful' }).within(() => { + cy.findByText('You are now logged in as', { exact: false }).should('be.visible') + cy.validateExternalLink({ name: mockUser.name, href: 'https://on.cypress.io/dashboard/profile' }) + + // The dialog can be closed at this point by either the header close button or the Continue button + // The Continue button is tested here + cy.findByRole('button', { name: 'Close' }).should('be.visible').and('not.be.disabled') + cy.findByRole('button', { name: 'Continue' }).click() + }) + + cy.get('@logInModal').should('not.exist') + cy.findByTestId('app-header-bar').findByTestId('user-avatar').should('be.visible') + }) + + it('shows log in modal workflow for user with only email', () => { + mockLogInActionsForUser(mockUserNoName) + + cy.findByTestId('app-header-bar').within(() => { + cy.findByTestId('user-avatar').should('not.exist') + cy.findByRole('button', { name: 'Log In' }).click() + }) + + cy.findByRole('dialog', { name: 'Log in to Cypress' }).as('logInModal').within(() => { + cy.findByRole('button', { name: 'Log In' }).click() + + // The Log In button transitions through a few states as the browser launch lifecycle completes + cy.findByRole('button', { name: 'Opening Browser' }).should('be.visible').and('be.disabled') + cy.findByRole('button', { name: 'Waiting for you to log in' }).should('be.visible').and('be.disabled') + }) + + cy.findByRole('dialog', { name: 'Login Successful' }).within(() => { + cy.findByText('You are now logged in as', { exact: false }).should('be.visible') + cy.validateExternalLink({ name: mockUserNoName.email, href: 'https://on.cypress.io/dashboard/profile' }) + + // The dialog can be closed at this point by either the header close button or the Continue button + // The close button is tested here + cy.findByRole('button', { name: 'Continue' }).should('be.visible').and('not.be.disabled') + cy.findByRole('button', { name: 'Close' }).click() + }) + + cy.get('@logInModal').should('not.exist') + cy.findByTestId('app-header-bar').findByTestId('user-avatar').should('be.visible') }) }) }) diff --git a/packages/frontend-shared/src/gql-components/topnav/LoginModal.spec.tsx b/packages/frontend-shared/src/gql-components/topnav/LoginModal.spec.tsx index e7e879a66b..1e81416556 100644 --- a/packages/frontend-shared/src/gql-components/topnav/LoginModal.spec.tsx +++ b/packages/frontend-shared/src/gql-components/topnav/LoginModal.spec.tsx @@ -10,12 +10,25 @@ const cloudViewer = { fullName: 'Tester Test', } -const mountSuccess = () => { +const cloudViewerNoName = { + id: '2', + email: 'no.name@test.test', + fullName: null, +} + +type TestCloudViewer = { + __typename?: 'CloudUser' | undefined + id: string + email: string | null + fullName: string | null +} + +const mountSuccess = (viewer: TestCloudViewer = cloudViewer) => { cy.mountFragment(LoginModalFragmentDoc, { onResult: (result) => { result.__typename = 'Query' result.isAuthBrowserOpened = true - result.cloudViewer = cloudViewer + result.cloudViewer = viewer result.cloudViewer.__typename = 'CloudUser' }, render: (gqlVal) =>
, @@ -62,6 +75,13 @@ describe('', { viewportWidth: 1000, viewportHeight: 750 }, () => { cy.contains('a', cloudViewer.fullName).should('have.attr', 'href', 'https://on.cypress.io/dashboard/profile') }) + it('shows successful login status with email if name not provided', () => { + mountSuccess(cloudViewerNoName) + cy.contains('h2', text.login.titleSuccess).should('be.visible') + cy.contains(text.login.bodySuccess.replace('{0}', cloudViewerNoName.email)).should('be.visible') + cy.contains('a', cloudViewerNoName.email).should('have.attr', 'href', 'https://on.cypress.io/dashboard/profile') + }) + it('emits an event to close the modal when "Continue" button is clicked', () => { mountSuccess() cy.findByRole('button', { name: text.login.actionContinue }).click().then(() => { diff --git a/packages/frontend-shared/src/gql-components/topnav/LoginModal.vue b/packages/frontend-shared/src/gql-components/topnav/LoginModal.vue index e3254d2dfe..f4704f695d 100644 --- a/packages/frontend-shared/src/gql-components/topnav/LoginModal.vue +++ b/packages/frontend-shared/src/gql-components/topnav/LoginModal.vue @@ -42,7 +42,7 @@ href="https://on.cypress.io/dashboard/profile" class="font-medium text-indigo-500" > - {{ viewer.fullName }} + {{ viewer.fullName || viewer.email }} diff --git a/packages/frontend-shared/src/gql-components/topnav/UserAvatar.vue b/packages/frontend-shared/src/gql-components/topnav/UserAvatar.vue index dacf6b3d7b..c277d4a9ec 100644 --- a/packages/frontend-shared/src/gql-components/topnav/UserAvatar.vue +++ b/packages/frontend-shared/src/gql-components/topnav/UserAvatar.vue @@ -2,6 +2,7 @@
diff --git a/packages/server/lib/gui/auth.ts b/packages/server/lib/gui/auth.ts index 2aa76eb2a9..a5c826d217 100644 --- a/packages/server/lib/gui/auth.ts +++ b/packages/server/lib/gui/auth.ts @@ -19,13 +19,13 @@ let openExternalAttempted = false let authRedirectReached = false let server -const _buildLoginRedirectUrl = (server) => { +const buildLoginRedirectUrl = (server) => { const { port } = server.address() return `http://127.0.0.1:${port}/redirect-to-auth` } -const _buildFullLoginUrl = (baseLoginUrl, server, utmCode) => { +const buildFullLoginUrl = (baseLoginUrl, server, utmCode) => { const { port } = server.address() if (!authState) { @@ -57,60 +57,19 @@ const _buildFullLoginUrl = (baseLoginUrl, server, utmCode) => { }) } -const _getOriginFromUrl = (originalUrl) => { +const getOriginFromUrl = (originalUrl) => { const parsedUrl = url.parse(originalUrl) return url.format(_.pick(parsedUrl, ['protocol', 'slashes', 'hostname', 'port'])) } -/** - * @returns a promise that is resolved with a user when auth is complete or rejected when it fails - */ -const start = (onMessage, utmCode) => { - function sendMessage (type, name, arg1) { - onMessage({ - type, - name, - message: errors.getMsgByType(name, arg1), - browserOpened: authRedirectReached, - }) - } - - authRedirectReached = false - - return user.getBaseLoginUrl() - .then((baseLoginUrl) => { - return _launchServer(baseLoginUrl, sendMessage, utmCode) - }) - .then(() => { - return _buildLoginRedirectUrl(server) - }) - .then((loginRedirectUrl) => { - debug('Trying to open native auth to URL ', loginRedirectUrl) - - return _launchNativeAuth(loginRedirectUrl, sendMessage) - .then(() => { - debug('openExternal completed') - }) - }) - .then(() => { - return Promise.fromCallback((cb) => { - authCallback = cb - }) - }) - .finally(() => { - _stopServer() - require('./windows').focusMainWindow() - }) -} - /** * @returns the currently running auth server instance, launches one if there is not one */ -const _launchServer = (baseLoginUrl, sendMessage, utmCode) => { +const launchServer = (baseLoginUrl, sendMessage, utmCode) => { if (!server) { // launch an express server to listen for the auth callback from dashboard - const origin = _getOriginFromUrl(baseLoginUrl) + const origin = getOriginFromUrl(baseLoginUrl) debug('Launching auth server with origin', origin) app = express() @@ -118,7 +77,7 @@ const _launchServer = (baseLoginUrl, sendMessage, utmCode) => { app.get('/redirect-to-auth', (req, res) => { authRedirectReached = true - _buildFullLoginUrl(baseLoginUrl, server, utmCode) + buildFullLoginUrl(baseLoginUrl, server, utmCode) .then((fullLoginUrl) => { debug('Received GET to /redirect-to-auth, redirecting: %o', { fullLoginUrl }) @@ -179,7 +138,7 @@ const _launchServer = (baseLoginUrl, sendMessage, utmCode) => { return Promise.resolve() } -const _stopServer = () => { +const stopServer = () => { debug('Closing auth server') if (server) { server.close() @@ -193,7 +152,7 @@ const _stopServer = () => { authRedirectReached = false } -const _launchNativeAuth = Promise.method((loginUrl, sendMessage) => { +const launchNativeAuth = Promise.method((loginUrl, sendMessage) => { const warnCouldNotLaunch = () => { if (openExternalAttempted && !authRedirectReached) { sendMessage('warning', 'AUTH_COULD_NOT_LAUNCH_BROWSER', loginUrl) @@ -213,11 +172,61 @@ const _launchNativeAuth = Promise.method((loginUrl, sendMessage) => { }) }) -export = { - _buildFullLoginUrl, - _getOriginFromUrl, - _launchServer, - _launchNativeAuth, - _stopServer, - start, +/** + * Grouping internal APIs under separate export to allow for stubbing + * in public API tests. + */ +const _internal = { + buildLoginRedirectUrl, + buildFullLoginUrl, + getOriginFromUrl, + launchServer, + stopServer, + launchNativeAuth, +} + +/** + * @returns a promise that is resolved with a user when auth is complete or rejected when it fails + */ +const start = (onMessage, utmCode) => { + function sendMessage (type, name, arg1) { + onMessage({ + type, + name, + message: errors.getMsgByType(name, arg1), + browserOpened: authRedirectReached, + }) + } + + authRedirectReached = false + + return user.getBaseLoginUrl() + .then((baseLoginUrl) => { + return _internal.launchServer(baseLoginUrl, sendMessage, utmCode) + }) + .then(() => { + return _internal.buildLoginRedirectUrl(server) + }) + .then((loginRedirectUrl) => { + debug('Trying to open native auth to URL %s', loginRedirectUrl) + + return _internal.launchNativeAuth(loginRedirectUrl, sendMessage) + .then(() => { + debug('successfully opened native auth url') + }) + }) + .then(() => { + return Promise.fromCallback((cb) => { + authCallback = cb + }) + }) + .finally(() => { + _internal.stopServer() + require('./windows').focusMainWindow() + }) +} + +export = { + start, + _internal, } diff --git a/packages/server/test/unit/gui/auth_spec.js b/packages/server/test/unit/gui/auth_spec.js index bab8690933..9f5789b90c 100644 --- a/packages/server/test/unit/gui/auth_spec.js +++ b/packages/server/test/unit/gui/auth_spec.js @@ -1,10 +1,14 @@ require('../../spec_helper') const auth = require(`../../../lib/gui/auth`) +const windows = require(`../../../lib/gui/windows`) +const user = require(`../../../lib/user`) + const electron = require('electron') const machineId = require(`../../../lib/util/machine_id`) const os = require('os') const pkg = require('@packages/root') +const Promise = require('bluebird') const random = require(`../../../lib/util/random`) const BASE_URL = 'https://foo.invalid/login.html' @@ -21,24 +25,24 @@ describe('lib/gui/auth', function () { }) afterEach(function () { - auth._stopServer() + auth._internal.stopServer() }) - context('._getOriginFromUrl', function () { + context('_internal.getOriginFromUrl', function () { it('given an https URL, returns the origin', function () { - const origin = auth._getOriginFromUrl(FULL_LOGIN_URL) + const origin = auth._internal.getOriginFromUrl(FULL_LOGIN_URL) expect(origin).to.eq('https://foo.invalid') }) it('given an http URL, returns the origin', function () { - const origin = auth._getOriginFromUrl('http://foo.invalid/login.html?abc=123&foo=bar') + const origin = auth._internal.getOriginFromUrl('http://foo.invalid/login.html?abc=123&foo=bar') expect(origin).to.eq('http://foo.invalid') }) }) - context('._buildFullLoginUrl', function () { + context('_internal.buildFullLoginUrl', function () { beforeEach(function () { sinon.stub(random, 'id').returns(RANDOM_STRING) this.server = { @@ -49,7 +53,7 @@ describe('lib/gui/auth', function () { }) it('uses random and server.port to form a URL along with environment info', function () { - return auth._buildFullLoginUrl(BASE_URL, this.server) + return auth._internal.buildFullLoginUrl(BASE_URL, this.server) .then((url) => { expect(url).to.eq(FULL_LOGIN_URL) expect(random.id).to.be.calledWith(32) @@ -58,9 +62,9 @@ describe('lib/gui/auth', function () { }) it('does not regenerate the state code', function () { - return auth._buildFullLoginUrl(BASE_URL, this.server) + return auth._internal.buildFullLoginUrl(BASE_URL, this.server) .then(() => { - return auth._buildFullLoginUrl(BASE_URL, this.server) + return auth._internal.buildFullLoginUrl(BASE_URL, this.server) }) .then(() => { expect(random.id).to.be.calledOnce @@ -68,16 +72,16 @@ describe('lib/gui/auth', function () { }) it('uses utm code to form a trackable URL', function () { - return auth._buildFullLoginUrl(BASE_URL, this.server, 'GUI Tab') + return auth._internal.buildFullLoginUrl(BASE_URL, this.server, 'GUI Tab') .then((url) => { expect(url).to.eq(FULL_LOGIN_URL_UTM) }) }) }) - context('._launchNativeAuth', function () { + context('_internal.launchNativeAuth', function () { it('is catchable if `shell` does not exist', function () { - return auth._launchNativeAuth(REDIRECT_URL) + return auth._internal.launchNativeAuth(REDIRECT_URL) .then(() => { throw new Error('This should not succeed') }) @@ -98,7 +102,7 @@ describe('lib/gui/auth', function () { sinon.stub(electron.shell, 'openExternal').resolves() const sendWarning = sinon.stub() - return auth._launchNativeAuth(REDIRECT_URL, sendWarning) + return auth._internal.launchNativeAuth(REDIRECT_URL, sendWarning) .then(() => { expect(electron.shell.openExternal).to.be.calledWithMatch(REDIRECT_URL) expect(sendWarning).to.not.be.called @@ -109,7 +113,7 @@ describe('lib/gui/auth', function () { sinon.stub(electron.shell, 'openExternal').rejects() const sendWarning = sinon.stub() - return auth._launchNativeAuth(REDIRECT_URL, sendWarning) + return auth._internal.launchNativeAuth(REDIRECT_URL, sendWarning) .then(() => { expect(electron.shell.openExternal).to.be.calledWithMatch(REDIRECT_URL) expect(sendWarning).to.be.calledWithMatch('warning', 'AUTH_COULD_NOT_LAUNCH_BROWSER', REDIRECT_URL) @@ -117,4 +121,36 @@ describe('lib/gui/auth', function () { }) }) }) + + context('.start', () => { + it('focuses main window upon successful auth', async () => { + sinon.stub(user, 'getBaseLoginUrl').resolves('www.foo.bar') + sinon.stub(Promise, 'fromCallback').resolves() + sinon.stub(auth._internal, 'launchServer').resolves() + sinon.stub(auth._internal, 'buildLoginRedirectUrl').resolves('www.redirect.url') + sinon.stub(auth._internal, 'launchNativeAuth').resolves() + sinon.stub(auth._internal, 'stopServer') + sinon.stub(windows, 'focusMainWindow').callsFake(() => {}) + + await auth.start(() => {}, 'code') + + expect(auth._internal.stopServer).to.be.calledOnce + expect(windows.focusMainWindow).to.be.calledOnce + }) + + it('focuses main window when auth fails', async () => { + sinon.stub(user, 'getBaseLoginUrl').rejects(new Error('test error')) + sinon.stub(auth._internal, 'stopServer') + sinon.stub(windows, 'focusMainWindow').callsFake(() => {}) + + try { + await auth.start(() => {}, 'code') + } catch (e) { + expect(e.message).to.eql('test error') + } + + expect(auth._internal.stopServer).to.be.calledOnce + expect(windows.focusMainWindow).to.be.calledOnce + }) + }) })