From 8dee365bda57a324d18fbd9405a965f74f1c047f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Tue, 13 Aug 2024 17:00:08 +0100 Subject: [PATCH] [Stateful sidenav] Don't fetch active space on unauthenticated routes (#190408) --- src/plugins/navigation/public/plugin.test.ts | 25 ++++++++++++ src/plugins/navigation/public/plugin.tsx | 38 +++++++++++++++---- .../navigation/server/ui_settings.test.ts | 20 +++++++++- src/plugins/navigation/server/ui_settings.ts | 2 + 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/plugins/navigation/public/plugin.test.ts b/src/plugins/navigation/public/plugin.test.ts index 1d9abb6f85dbc..e1f9cd9ea3991 100644 --- a/src/plugins/navigation/public/plugin.test.ts +++ b/src/plugins/navigation/public/plugin.test.ts @@ -69,6 +69,31 @@ describe('Navigation Plugin', () => { expect(coreStart.chrome.project.changeActiveSolutionNavigation).toHaveBeenCalledWith('es'); }); + it('should not load the active space on non authenticated pages', async () => { + const { plugin, coreStart, unifiedSearch, cloud, spaces } = setup(); + + coreStart.http.anonymousPaths.isAnonymous.mockReturnValue(true); + + const activeSpace$ = of({ solution: 'es' } as Pick); + activeSpace$.pipe = jest.fn().mockReturnValue(activeSpace$); + activeSpace$.subscribe = jest.fn().mockReturnValue(activeSpace$); + spaces.getActiveSpace$ = jest.fn().mockReturnValue(activeSpace$); + + plugin.start(coreStart, { unifiedSearch, cloud, spaces }); + await new Promise((resolve) => setTimeout(resolve)); + + expect(activeSpace$.pipe).not.toHaveBeenCalled(); + expect(activeSpace$.subscribe).not.toHaveBeenCalled(); + + // Test that the activeSpace$ observable is accessed when not an anonymous path + coreStart.http.anonymousPaths.isAnonymous.mockReturnValue(false); + plugin.start(coreStart, { unifiedSearch, cloud, spaces }); + await new Promise((resolve) => setTimeout(resolve)); + + expect(activeSpace$.pipe).toHaveBeenCalled(); + expect(activeSpace$.subscribe).toHaveBeenCalled(); + }); + describe('addSolutionNavigation()', () => { it('should update the solution navigation definitions', async () => { const { plugin, coreStart, unifiedSearch, cloud } = setup(); diff --git a/src/plugins/navigation/public/plugin.tsx b/src/plugins/navigation/public/plugin.tsx index 66ef8cb808720..73dc93609bcf7 100644 --- a/src/plugins/navigation/public/plugin.tsx +++ b/src/plugins/navigation/public/plugin.tsx @@ -6,8 +6,14 @@ * Side Public License, v 1. */ import React from 'react'; -import { of, ReplaySubject, take, map, Observable } from 'rxjs'; -import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from '@kbn/core/public'; +import { of, ReplaySubject, take, map, Observable, switchMap } from 'rxjs'; +import { + PluginInitializerContext, + CoreSetup, + CoreStart, + Plugin, + HttpStart, +} from '@kbn/core/public'; import type { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public'; import type { Space } from '@kbn/spaces-plugin/public'; import type { SolutionNavigationDefinition } from '@kbn/core-chrome-browser'; @@ -89,8 +95,7 @@ export class NavigationPublicPlugin return createTopNav(customUnifiedSearch ?? unifiedSearch, customExtensions ?? extensions); }; - // Initialize the solution navigation if it is enabled - activeSpace$.pipe(take(1)).subscribe((activeSpace) => { + const initSolutionNavigation = (activeSpace?: Space) => { this.initiateChromeStyleAndSideNav(chrome, { isServerless, activeSpace, @@ -99,7 +104,14 @@ export class NavigationPublicPlugin if (!this.isSolutionNavEnabled) return; chrome.project.setCloudUrls(cloud!); - }); + }; + + if (this.getIsUnauthenticated(core.http)) { + // Don't fetch the active space if the user is not authenticated + initSolutionNavigation(); + } else { + activeSpace$.pipe(take(1)).subscribe(initSolutionNavigation); + } return { ui: { @@ -111,9 +123,14 @@ export class NavigationPublicPlugin if (!this.isSolutionNavEnabled) return; this.addSolutionNavigation(solutionNavigation); }, - isSolutionNavEnabled$: activeSpace$.pipe( - map((activeSpace) => { - return this.isSolutionNavEnabled && getIsProjectNav(activeSpace?.solution); + isSolutionNavEnabled$: of(this.getIsUnauthenticated(core.http)).pipe( + switchMap((isUnauthenticated) => { + if (isUnauthenticated) return of(false); + return activeSpace$.pipe( + map((activeSpace) => { + return this.isSolutionNavEnabled && getIsProjectNav(activeSpace?.solution); + }) + ); }) ), }; @@ -172,6 +189,11 @@ export class NavigationPublicPlugin chrome.project.changeActiveSolutionNavigation(solutionView!); } } + + private getIsUnauthenticated(http: HttpStart) { + const { anonymousPaths } = http; + return anonymousPaths.isAnonymous(window.location.pathname); + } } function getIsProjectNav(solutionView?: string) { diff --git a/src/plugins/navigation/server/ui_settings.test.ts b/src/plugins/navigation/server/ui_settings.test.ts index 1d8431b359109..6d8b12d41cbf9 100644 --- a/src/plugins/navigation/server/ui_settings.test.ts +++ b/src/plugins/navigation/server/ui_settings.test.ts @@ -41,6 +41,21 @@ describe('ui settings', () => { await expect(defaultRoute.getValue!()).resolves.toBe(DEFAULT_ROUTES.classic); }); + it('should return classic when accessing a non authenticated route', async () => { + const spaces = spacesMock.createStart(); + const mockSpace: Pick = { solution: 'es' }; + spaces.spacesService.getActiveSpace.mockResolvedValue(mockSpace as Space); + core.getStartServices.mockResolvedValue([{} as any, { spaces }, {} as any]); + + const { defaultRoute } = getUiSettings(core, logger); + const requestMock = { + auth: { isAuthenticated: false }, + }; + await expect(defaultRoute.getValue!({ request: requestMock as any })).resolves.toBe( + DEFAULT_ROUTES.classic + ); + }); + it('should return the route based on the active space', async () => { const spaces = spacesMock.createStart(); @@ -50,7 +65,10 @@ describe('ui settings', () => { core.getStartServices.mockResolvedValue([{} as any, { spaces }, {} as any]); const { defaultRoute } = getUiSettings(core, logger); - await expect(defaultRoute.getValue!({ request: {} as any })).resolves.toBe( + const requestMock = { + auth: { isAuthenticated: true }, + }; + await expect(defaultRoute.getValue!({ request: requestMock as any })).resolves.toBe( DEFAULT_ROUTES[solution] ); } diff --git a/src/plugins/navigation/server/ui_settings.ts b/src/plugins/navigation/server/ui_settings.ts index bcf128f2d348a..be216e5d5175f 100644 --- a/src/plugins/navigation/server/ui_settings.ts +++ b/src/plugins/navigation/server/ui_settings.ts @@ -34,6 +34,8 @@ export const getUiSettings = ( } try { + if (!request.auth.isAuthenticated) return DEFAULT_ROUTES.classic; + const activeSpace = await spaces.spacesService.getActiveSpace(request); const solution = activeSpace?.solution ?? 'classic';