From 834e88c945071e6bd1ed71b3b4898d34a4553831 Mon Sep 17 00:00:00 2001 From: Lauren Zugai Date: Fri, 15 Nov 2024 12:50:40 -0600 Subject: [PATCH] fix(sync): Always send firefox.fxaLogout on sign out Because: * If users are signed into the browser but they've accessed account settings in a non-Sync context, the browser holds onto the old session token, leading to a 'Session Expired' page when they try to access CAD or account settings from the browser This commit: * Removes the isSync() check around the firefox.fxaLogout call --- .../components/Settings/AppLayout/index.tsx | 6 ++-- .../DropDownAvatarMenu/index.stories.tsx | 9 ++--- .../DropDownAvatarMenu/index.test.tsx | 34 +++++-------------- .../Settings/DropDownAvatarMenu/index.tsx | 14 +++----- .../Settings/HeaderLockup/index.stories.tsx | 7 ++-- .../Settings/HeaderLockup/index.test.tsx | 9 ++--- .../Settings/HeaderLockup/index.tsx | 9 ++--- 7 files changed, 23 insertions(+), 65 deletions(-) diff --git a/packages/fxa-settings/src/components/Settings/AppLayout/index.tsx b/packages/fxa-settings/src/components/Settings/AppLayout/index.tsx index 636721e0a30..f30c29079ed 100644 --- a/packages/fxa-settings/src/components/Settings/AppLayout/index.tsx +++ b/packages/fxa-settings/src/components/Settings/AppLayout/index.tsx @@ -7,14 +7,12 @@ import HeaderLockup from '../HeaderLockup'; import ContentSkip from '../ContentSkip'; import Footer from 'fxa-react/components/Footer'; import { AlertBar } from '../AlertBar'; -import { SettingsIntegration } from '../interfaces'; type AppLayoutProps = { children: React.ReactNode; - integration: SettingsIntegration; }; -export const AppLayout = ({ children, integration }: AppLayoutProps) => { +export const AppLayout = ({ children }: AppLayoutProps) => { return (
{ >
- +
diff --git a/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.stories.tsx b/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.stories.tsx index 77f53f808f9..45a0efe7b2c 100644 --- a/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.stories.tsx +++ b/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.stories.tsx @@ -8,7 +8,6 @@ import { withLocalization } from 'fxa-react/lib/storybooks'; import DropDownAvatarMenu from '.'; import { Account, AppContext } from 'fxa-settings/src/models'; import { mockAppContext, MOCK_ACCOUNT } from 'fxa-settings/src/models/mocks'; -import { createMockSettingsIntegration } from '../mocks'; export default { title: 'Components/Settings/DropDownAvatarMenu', @@ -35,14 +34,12 @@ const accountWithoutAvatar = { }, } as unknown as Account; -const integration = createMockSettingsIntegration(); - const storyWithContext = (account: Partial) => { const context = { account: account as Account }; const story = () => ( - + ); return story; @@ -51,6 +48,4 @@ const storyWithContext = (account: Partial) => { export const DefaultNoAvatarOrDisplayName = storyWithContext(accountWithoutAvatar); -export const WithAvatarAndDisplayName = () => ( - -); +export const WithAvatarAndDisplayName = () => ; diff --git a/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.test.tsx b/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.test.tsx index 8e78bbf06bc..805c1dcee38 100644 --- a/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.test.tsx @@ -14,7 +14,6 @@ import DropDownAvatarMenu from '.'; import { logViewEvent, settingsViewName } from 'fxa-settings/src/lib/metrics'; import { Account, AppContext } from '../../../models'; import { SettingsContext } from '../../../models/contexts/SettingsContext'; -import { createMockSettingsIntegration } from '../mocks'; import firefox from '../../../lib/channels/firefox'; jest.mock('../../../models/AlertBarInfo'); @@ -48,7 +47,7 @@ describe('DropDownAvatarMenu', () => { } as unknown as Account; renderWithLocalizationProvider( - + ); @@ -75,7 +74,7 @@ describe('DropDownAvatarMenu', () => { it('renders as expected with avatar url and displayName set', () => { renderWithLocalizationProvider( - + ); fireEvent.click(screen.getByTestId('drop-down-avatar-menu-toggle')); @@ -87,7 +86,7 @@ describe('DropDownAvatarMenu', () => { it('closes on esc keypress', () => { renderWithLocalizationProvider( - + ); @@ -102,7 +101,7 @@ describe('DropDownAvatarMenu', () => {
- +
@@ -127,7 +126,7 @@ describe('DropDownAvatarMenu', () => { - + ); @@ -153,7 +152,7 @@ describe('DropDownAvatarMenu', () => { renderWithLocalizationProvider( - + ); @@ -174,14 +173,12 @@ describe('DropDownAvatarMenu', () => { afterEach(() => { jest.restoreAllMocks(); }); - it('is called integration is sync', async () => { + it('is called', async () => { renderWithLocalizationProvider( - + ); fireEvent.click(screen.getByTestId('drop-down-avatar-menu-toggle')); @@ -190,20 +187,5 @@ describe('DropDownAvatarMenu', () => { }); expect(fxaLogoutSpy).toBeCalledWith({ uid: account.uid }); }); - - it('is not called when integration is not sync', async () => { - renderWithLocalizationProvider( - - - - ); - fireEvent.click(screen.getByTestId('drop-down-avatar-menu-toggle')); - await act(async () => { - fireEvent.click(screen.getByTestId('avatar-menu-sign-out')); - }); - expect(fxaLogoutSpy).not.toBeCalled(); - }); }); }); diff --git a/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.tsx b/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.tsx index 5b6399613a2..1b50952d969 100644 --- a/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.tsx +++ b/packages/fxa-settings/src/components/Settings/DropDownAvatarMenu/index.tsx @@ -11,13 +11,8 @@ import { ReactComponent as SignOut } from './sign-out.svg'; import { logViewEvent, settingsViewName } from '../../../lib/metrics'; import { Localized, useLocalization } from '@fluent/react'; import firefox from '../../../lib/channels/firefox'; -import { SettingsIntegration } from '../interfaces'; -export const DropDownAvatarMenu = ({ - integration, -}: { - integration: SettingsIntegration; -}) => { +export const DropDownAvatarMenu = () => { const { displayName, primaryEmail, avatar, uid } = useAccount(); const session = useSession(); const [isRevealed, setRevealed] = useState(false); @@ -39,9 +34,10 @@ export const DropDownAvatarMenu = ({ try { await session.destroy(); - if (integration.isSync()) { - firefox.fxaLogout({ uid }); - } + // Send a logout event to Firefox even if the user is in a non-Sync flow. + // If the user is signed into the browser, they need to drop the now + // destroyed session token. + firefox.fxaLogout({ uid }); logViewEvent(settingsViewName, 'signout.success'); window.location.assign(window.location.origin); diff --git a/packages/fxa-settings/src/components/Settings/HeaderLockup/index.stories.tsx b/packages/fxa-settings/src/components/Settings/HeaderLockup/index.stories.tsx index 5f43eac1e34..0f9cf1d449a 100644 --- a/packages/fxa-settings/src/components/Settings/HeaderLockup/index.stories.tsx +++ b/packages/fxa-settings/src/components/Settings/HeaderLockup/index.stories.tsx @@ -8,7 +8,6 @@ import { withLocalization } from 'fxa-react/lib/storybooks'; import { HeaderLockup } from '.'; import { Account, AppContext } from '../../../models'; import { mockAppContext, MOCK_ACCOUNT } from 'fxa-settings/src/models/mocks'; -import { createMockSettingsIntegration } from '../mocks'; export default { title: 'Components/Settings/HeaderLockup', @@ -26,14 +25,12 @@ const accountWithoutAvatar = { }, } as unknown as Account; -const integration = createMockSettingsIntegration(); - const storyWithContext = (account: Partial) => { const context = { account: account as Account }; const story = () => ( - + ); return story; @@ -41,4 +38,4 @@ const storyWithContext = (account: Partial) => { export const WithDefaultAvatar = storyWithContext(accountWithoutAvatar); -export const WithCustomAvatar = () => ; +export const WithCustomAvatar = () => ; diff --git a/packages/fxa-settings/src/components/Settings/HeaderLockup/index.test.tsx b/packages/fxa-settings/src/components/Settings/HeaderLockup/index.test.tsx index cfce9c6598c..3d0d92ca358 100644 --- a/packages/fxa-settings/src/components/Settings/HeaderLockup/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/HeaderLockup/index.test.tsx @@ -6,7 +6,6 @@ import React from 'react'; import { screen } from '@testing-library/react'; import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; import HeaderLockup from '.'; -import { createMockSettingsIntegration } from '../mocks'; import { userEvent } from '@testing-library/user-event'; import GleanMetrics from '../../../lib/glean'; @@ -21,9 +20,7 @@ jest.mock('../../../lib/glean', () => ({ describe('HeaderLockup', () => { it('renders as expected', () => { - renderWithLocalizationProvider( - - ); + renderWithLocalizationProvider(); const headerMenu = screen.getByTestId('header-menu'); expect( @@ -45,9 +42,7 @@ describe('HeaderLockup', () => { }); it('emits Glean event on help link click', async () => { - renderWithLocalizationProvider( - - ); + renderWithLocalizationProvider(); await userEvent.click(screen.getByRole('link', { name: 'Help' })); expect(GleanMetrics.accountPref.help).toHaveBeenCalled(); }); diff --git a/packages/fxa-settings/src/components/Settings/HeaderLockup/index.tsx b/packages/fxa-settings/src/components/Settings/HeaderLockup/index.tsx index a2bf6fbaed6..ca9fafa9f44 100644 --- a/packages/fxa-settings/src/components/Settings/HeaderLockup/index.tsx +++ b/packages/fxa-settings/src/components/Settings/HeaderLockup/index.tsx @@ -12,15 +12,10 @@ import DropDownAvatarMenu from '../DropDownAvatarMenu'; import { ReactComponent as Help } from './help.svg'; import { ReactComponent as Menu } from './menu.svg'; import { ReactComponent as Close } from './close.svg'; -import { SettingsIntegration } from '../interfaces'; import Sidebar from '../Sidebar'; import GleanMetrics from '../../../lib/glean'; -export const HeaderLockup = ({ - integration, -}: { - integration: SettingsIntegration; -}) => { +export const HeaderLockup = () => { const [sidebarRevealedState, setNavState] = useState(false); const { l10n } = useLocalization(); const localizedHelpText = l10n.getString('header-help', null, 'Help'); @@ -87,7 +82,7 @@ export const HeaderLockup = ({ /> - + );