Show error page when accessing unavailable app (#54656)

* display not found page instead of throwing an error when accessible unavailable app

* move types to public folder

* fix types import

* remove updater from start app

* remove unnecessary await

src/core/public/application/__snapshots__/application_service.test.ts.snap

@@ -0,0 +1,84 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`#start() getComponent returns renderable JSX tree 1`] = `
+<AppRouter
+  appStatuses$={
+    AnonymousSubject {
+      "_isScalar": false,
+      "closed": false,
+      "destination": AnonymousSubject {
+        "_isScalar": false,
+        "closed": false,
+        "destination": BehaviorSubject {
+          "_isScalar": false,
+          "_value": Map {},
+          "closed": false,
+          "hasError": false,
+          "isStopped": false,
+          "observers": Array [],
+          "thrownError": null,
+        },
+        "hasError": false,
+        "isStopped": false,
+        "observers": Array [],
+        "operator": MapOperator {
+          "project": [Function],
+          "thisArg": undefined,
+        },
+        "source": BehaviorSubject {
+          "_isScalar": false,
+          "_value": Map {},
+          "closed": false,
+          "hasError": false,
+          "isStopped": false,
+          "observers": Array [],
+          "thrownError": null,
+        },
+        "thrownError": null,
+      },
+      "hasError": false,
+      "isStopped": false,
+      "observers": Array [],
+      "operator": [Function],
+      "source": AnonymousSubject {
+        "_isScalar": false,
+        "closed": false,
+        "destination": BehaviorSubject {
+          "_isScalar": false,
+          "_value": Map {},
+          "closed": false,
+          "hasError": false,
+          "isStopped": false,
+          "observers": Array [],
+          "thrownError": null,
+        },
+        "hasError": false,
+        "isStopped": false,
+        "observers": Array [],
+        "operator": MapOperator {
+          "project": [Function],
+          "thisArg": undefined,
+        },
+        "source": BehaviorSubject {
+          "_isScalar": false,
+          "_value": Map {},
+          "closed": false,
+          "hasError": false,
+          "isStopped": false,
+          "observers": Array [],
+          "thrownError": null,
+        },
+        "thrownError": null,
+      },
+      "thrownError": null,
+    }
+  }
+  history={
+    Object {
+      "push": [MockFunction],
+    }
+  }
+  mounters={Map {}}
+  setAppLeaveHandler={[Function]}
+/>
+`;

src/core/public/application/application_service.test.ts

@@ -525,17 +525,7 @@ describe('#start()', () => {
       const { getComponent } = await service.start(startDeps);
 
       expect(() => shallow(createElement(getComponent))).not.toThrow();
-      expect(getComponent()).toMatchInlineSnapshot(`
-        <AppRouter
-          history={
-            Object {
-              "push": [MockFunction],
-            }
-          }
-          mounters={Map {}}
-          setAppLeaveHandler={[Function]}
-        />
-      `);
+      expect(getComponent()).toMatchSnapshot();
     });
 
     it('renders null when in legacy mode', async () => {

src/core/public/application/application_service.tsx

@@ -19,7 +19,7 @@
 
 import React from 'react';
 import { BehaviorSubject, Observable, Subject, Subscription } from 'rxjs';
-import { map, takeUntil } from 'rxjs/operators';
+import { map, shareReplay, takeUntil } from 'rxjs/operators';
 import { createBrowserHistory, History } from 'history';
 
 import { InjectedMetadataSetup } from '../injected_metadata';
@@ -256,6 +256,11 @@ export class ApplicationService {
       )
       .subscribe(apps => applications$.next(apps));
 
+    const applicationStatuses$ = applications$.pipe(
+      map(apps => new Map([...apps.entries()].map(([id, app]) => [id, app.status!]))),
+      shareReplay(1)
+    );
+
     return {
       applications$,
       capabilities,
@@ -264,11 +269,6 @@ export class ApplicationService {
       getUrlForApp: (appId, { path }: { path?: string } = {}) =>
         getAppUrl(availableMounters, appId, path),
       navigateToApp: async (appId, { path, state }: { path?: string; state?: any } = {}) => {
-        const app = applications$.value.get(appId);
-        if (app && app.status !== AppStatus.accessible) {
-          // should probably redirect to the error page instead
-          throw new Error(`Trying to navigate to an inaccessible application: ${appId}`);
-        }
         if (await this.shouldNavigate(overlays)) {
           this.appLeaveHandlers.delete(this.currentAppId$.value!);
           this.navigate!(getAppUrl(availableMounters, appId, path), state);
@@ -283,6 +283,7 @@ export class ApplicationService {
           <AppRouter
             history={this.history}
             mounters={availableMounters}
+            appStatuses$={applicationStatuses$}
             setAppLeaveHandler={this.setAppLeaveHandler}
           />
         );

src/core/public/application/integration_tests/router.test.tsx

@@ -18,15 +18,18 @@
  */
 
 import React from 'react';
+import { BehaviorSubject } from 'rxjs';
 import { createMemoryHistory, History, createHashHistory } from 'history';
 
 import { AppRouter, AppNotFound } from '../ui';
 import { EitherApp, MockedMounterMap, MockedMounterTuple } from '../test_types';
 import { createRenderer, createAppMounter, createLegacyAppMounter } from './utils';
+import { AppStatus } from '../types';
 
 describe('AppContainer', () => {
   let mounters: MockedMounterMap<EitherApp>;
   let history: History;
+  let appStatuses$: BehaviorSubject<Map<string, AppStatus>>;
   let update: ReturnType<typeof createRenderer>;
 
   const navigate = (path: string) => {
@@ -38,6 +41,17 @@ describe('AppContainer', () => {
     new Map([...mounters].map(([appId, { mounter }]) => [appId, mounter]));
   const setAppLeaveHandlerMock = () => undefined;
 
+  const mountersToAppStatus$ = () => {
+    return new BehaviorSubject(
+      new Map(
+        [...mounters.keys()].map(id => [
+          id,
+          id.startsWith('disabled') ? AppStatus.inaccessible : AppStatus.accessible,
+        ])
+      )
+    );
+  };
+
   beforeEach(() => {
     mounters = new Map([
       createAppMounter('app1', '<span>App 1</span>'),
@@ -45,12 +59,16 @@ describe('AppContainer', () => {
       createAppMounter('app2', '<div>App 2</div>'),
       createLegacyAppMounter('baseApp:legacyApp2', jest.fn()),
       createAppMounter('app3', '<div>App 3</div>', '/custom/path'),
+      createAppMounter('disabledApp', '<div>Disabled app</div>'),
+      createLegacyAppMounter('disabledLegacyApp', jest.fn()),
     ] as Array<MockedMounterTuple<EitherApp>>);
     history = createMemoryHistory();
+    appStatuses$ = mountersToAppStatus$();
     update = createRenderer(
       <AppRouter
         history={history}
         mounters={mockMountersToMounters()}
+        appStatuses$={appStatuses$}
         setAppLeaveHandler={setAppLeaveHandlerMock}
       />
     );
@@ -89,6 +107,7 @@ describe('AppContainer', () => {
       <AppRouter
         history={history}
         mounters={mockMountersToMounters()}
+        appStatuses$={mountersToAppStatus$()}
         setAppLeaveHandler={setAppLeaveHandlerMock}
       />
     );
@@ -107,6 +126,7 @@ describe('AppContainer', () => {
       <AppRouter
         history={history}
         mounters={mockMountersToMounters()}
+        appStatuses$={mountersToAppStatus$()}
         setAppLeaveHandler={setAppLeaveHandlerMock}
       />
     );
@@ -147,6 +167,7 @@ describe('AppContainer', () => {
       <AppRouter
         history={history}
         mounters={mockMountersToMounters()}
+        appStatuses$={mountersToAppStatus$()}
         setAppLeaveHandler={setAppLeaveHandlerMock}
       />
     );
@@ -202,4 +223,16 @@ describe('AppContainer', () => {
 
     expect(dom?.exists(AppNotFound)).toBe(true);
   });
+
+  it('displays error page if app is inaccessible', async () => {
+    const dom = await navigate('/app/disabledApp');
+
+    expect(dom?.exists(AppNotFound)).toBe(true);
+  });
+
+  it('displays error page if legacy app is inaccessible', async () => {
+    const dom = await navigate('/app/disabledLegacyApp');
+
+    expect(dom?.exists(AppNotFound)).toBe(true);
+  });
 });

src/core/public/application/ui/app_container.tsx

@@ -26,12 +26,13 @@ import React, {
   MutableRefObject,
 } from 'react';
 
-import { AppUnmount, Mounter, AppLeaveHandler } from '../types';
+import { AppLeaveHandler, AppStatus, AppUnmount, Mounter } from '../types';
 import { AppNotFound } from './app_not_found_screen';
 
 interface Props {
   appId: string;
   mounter?: Mounter;
+  appStatus: AppStatus;
   setAppLeaveHandler: (appId: string, handler: AppLeaveHandler) => void;
 }
 
@@ -39,10 +40,12 @@ export const AppContainer: FunctionComponent<Props> = ({
   mounter,
   appId,
   setAppLeaveHandler,
+  appStatus,
 }: Props) => {
   const [appNotFound, setAppNotFound] = useState(false);
   const elementRef = useRef<HTMLDivElement>(null);
   const unmountRef: MutableRefObject<AppUnmount | null> = useRef<AppUnmount>(null);
+  // const appStatus = useObservable(appStatus$);
 
   useLayoutEffect(() => {
     const unmount = () => {
@@ -52,7 +55,7 @@ export const AppContainer: FunctionComponent<Props> = ({
       }
     };
     const mount = async () => {
-      if (!mounter) {
+      if (!mounter || appStatus !== AppStatus.accessible) {
         return setAppNotFound(true);
       }
 
@@ -71,7 +74,7 @@ export const AppContainer: FunctionComponent<Props> = ({
 
     mount();
     return unmount;
-  }, [appId, mounter, setAppLeaveHandler]);
+  }, [appId, appStatus, mounter, setAppLeaveHandler]);
 
   return (
     <Fragment>

src/core/public/application/ui/app_not_found_screen.tsx

@@ -22,7 +22,7 @@ import React from 'react';
 import { FormattedMessage } from '@kbn/i18n/react';
 
 export const AppNotFound = () => (
-  <EuiPage style={{ minHeight: '100%' }}>
+  <EuiPage style={{ minHeight: '100%' }} data-test-subj="appNotFoundPageContent">
     <EuiPageBody>
       <EuiPageContent verticalPosition="center" horizontalPosition="center">
         <EuiEmptyPrompt

src/core/public/application/ui/app_router.tsx

@@ -18,15 +18,18 @@
  */
 
 import React, { FunctionComponent } from 'react';
+import { Route, RouteComponentProps, Router, Switch } from 'react-router-dom';
 import { History } from 'history';
-import { Router, Route, RouteComponentProps, Switch } from 'react-router-dom';
+import { Observable } from 'rxjs';
+import { useObservable } from 'react-use';
 
-import { Mounter, AppLeaveHandler } from '../types';
+import { AppLeaveHandler, AppStatus, Mounter } from '../types';
 import { AppContainer } from './app_container';
 
 interface Props {
   mounters: Map<string, Mounter>;
   history: History;
+  appStatuses$: Observable<Map<string, AppStatus>>;
   setAppLeaveHandler: (appId: string, handler: AppLeaveHandler) => void;
 }
 
@@ -34,45 +37,59 @@ interface Params {
   appId: string;
 }
 
-export const AppRouter: FunctionComponent<Props> = ({ history, mounters, setAppLeaveHandler }) => (
-  <Router history={history}>
-    <Switch>
-      {[...mounters].flatMap(([appId, mounter]) =>
-        // Remove /app paths from the routes as they will be handled by the
-        // "named" route parameter `:appId` below
-        mounter.appBasePath.startsWith('/app')
-          ? []
-          : [
-              <Route
-                key={mounter.appRoute}
-                path={mounter.appRoute}
-                render={() => (
-                  <AppContainer
-                    mounter={mounter}
-                    appId={appId}
-                    setAppLeaveHandler={setAppLeaveHandler}
-                  />
-                )}
-              />,
-            ]
-      )}
-      <Route
-        path="/app/:appId"
-        render={({
-          match: {
-            params: { appId },
-          },
-        }: RouteComponentProps<Params>) => {
-          // Find the mounter including legacy mounters with subapps:
-          const [id, mounter] = mounters.has(appId)
-            ? [appId, mounters.get(appId)]
-            : [...mounters].filter(([key]) => key.split(':')[0] === appId)[0] ?? [];
+export const AppRouter: FunctionComponent<Props> = ({
+  history,
+  mounters,
+  setAppLeaveHandler,
+  appStatuses$,
+}) => {
+  const appStatuses = useObservable(appStatuses$, new Map());
+  return (
+    <Router history={history}>
+      <Switch>
+        {[...mounters].flatMap(([appId, mounter]) =>
+          // Remove /app paths from the routes as they will be handled by the
+          // "named" route parameter `:appId` below
+          mounter.appBasePath.startsWith('/app')
+            ? []
+            : [
+                <Route
+                  key={mounter.appRoute}
+                  path={mounter.appRoute}
+                  render={() => (
+                    <AppContainer
+                      mounter={mounter}
+                      appId={appId}
+                      appStatus={appStatuses.get(appId) ?? AppStatus.inaccessible}
+                      setAppLeaveHandler={setAppLeaveHandler}
+                    />
+                  )}
+                />,
+              ]
+        )}
+        <Route
+          path="/app/:appId"
+          render={({
+            match: {
+              params: { appId },
+            },
+          }: RouteComponentProps<Params>) => {
+            // Find the mounter including legacy mounters with subapps:
+            const [id, mounter] = mounters.has(appId)
+              ? [appId, mounters.get(appId)]
+              : [...mounters].filter(([key]) => key.split(':')[0] === appId)[0] ?? [];
 
-          return (
-            <AppContainer mounter={mounter} appId={id} setAppLeaveHandler={setAppLeaveHandler} />
-          );
-        }}
-      />
-    </Switch>
-  </Router>
-);
+            return (
+              <AppContainer
+                mounter={mounter}
+                appId={id}
+                appStatus={appStatuses.get(id) ?? AppStatus.inaccessible}
+                setAppLeaveHandler={setAppLeaveHandler}
+              />
+            );
+          }}
+        />
+      </Switch>
+    </Router>
+  );
+};

test/plugin_functional/plugins/core_app_status/public/application.tsx

@@ -31,15 +31,15 @@ import {
   EuiTitle,
 } from '@elastic/eui';
 
-import { AppMountContext, AppMountParameters } from 'kibana/public';
+import { AppMountParameters } from 'kibana/public';
 
-const AppStatusApp = () => (
+const AppStatusApp = ({ appId }: { appId: string }) => (
   <EuiPage>
     <EuiPageBody data-test-subj="appStatusApp">
       <EuiPageHeader>
         <EuiPageHeaderSection>
           <EuiTitle size="l">
-            <h1>Welcome to App Status Test App!</h1>
+            <h1>Welcome to {appId} Test App!</h1>
           </EuiTitle>
         </EuiPageHeaderSection>
       </EuiPageHeader>
@@ -47,18 +47,18 @@ const AppStatusApp = () => (
         <EuiPageContentHeader>
           <EuiPageContentHeaderSection>
             <EuiTitle>
-              <h2>App Status Test App home page section title</h2>
+              <h2>{appId} Test App home page section title</h2>
             </EuiTitle>
           </EuiPageContentHeaderSection>
         </EuiPageContentHeader>
-        <EuiPageContentBody>App Status Test App content</EuiPageContentBody>
+        <EuiPageContentBody>{appId} Test App content</EuiPageContentBody>
       </EuiPageContent>
     </EuiPageBody>
   </EuiPage>
 );
 
-export const renderApp = (context: AppMountContext, { element }: AppMountParameters) => {
-  render(<AppStatusApp />, element);
+export const renderApp = (appId: string, { element }: AppMountParameters) => {
+  render(<AppStatusApp appId={appId} />, element);
 
   return () => unmountComponentAtNode(element);
 };

test/plugin_functional/plugins/core_app_status/public/index.ts

@@ -18,7 +18,7 @@
  */
 
 import { PluginInitializer } from 'kibana/public';
-import { CoreAppStatusPlugin, CoreAppStatusPluginSetup, CoreAppStatusPluginStart } from './plugin';
+import { CoreAppStatusPlugin, CoreAppStatusPluginStart } from './plugin';
 
-export const plugin: PluginInitializer<CoreAppStatusPluginSetup, CoreAppStatusPluginStart> = () =>
+export const plugin: PluginInitializer<{}, CoreAppStatusPluginStart> = () =>
   new CoreAppStatusPlugin();

test/plugin_functional/plugins/core_app_status/public/plugin.tsx

@@ -17,22 +17,38 @@
  * under the License.
  */
 
-import { Plugin, CoreSetup, AppUpdater, AppUpdatableFields, CoreStart } from 'kibana/public';
 import { BehaviorSubject } from 'rxjs';
+import {
+  Plugin,
+  CoreSetup,
+  AppUpdater,
+  AppUpdatableFields,
+  CoreStart,
+  AppMountParameters,
+} from 'kibana/public';
+import './types';
 
-export class CoreAppStatusPlugin
-  implements Plugin<CoreAppStatusPluginSetup, CoreAppStatusPluginStart> {
+export class CoreAppStatusPlugin implements Plugin<{}, CoreAppStatusPluginStart> {
   private appUpdater = new BehaviorSubject<AppUpdater>(() => ({}));
 
   public setup(core: CoreSetup, deps: {}) {
+    core.application.register({
+      id: 'app_status_start',
+      title: 'App Status Start Page',
+      async mount(params: AppMountParameters) {
+        const { renderApp } = await import('./application');
+        return renderApp('app_status_start', params);
+      },
+    });
+
     core.application.register({
       id: 'app_status',
       title: 'App Status',
       euiIconType: 'snowflake',
       updater$: this.appUpdater,
-      async mount(context, params) {
+      async mount(params: AppMountParameters) {
         const { renderApp } = await import('./application');
-        return renderApp(context, params);
+        return renderApp('app_status', params);
       },
     });
 
@@ -40,7 +56,7 @@ export class CoreAppStatusPlugin
   }
 
   public start(core: CoreStart) {
-    return {
+    const startContract = {
       setAppStatus: (status: Partial<AppUpdatableFields>) => {
         this.appUpdater.next(() => status);
       },
@@ -48,9 +64,10 @@ export class CoreAppStatusPlugin
         return core.application.navigateToApp(appId);
       },
     };
+    window.__coreAppStatus = startContract;
+    return startContract;
   }
   public stop() {}
 }
 
-export type CoreAppStatusPluginSetup = ReturnType<CoreAppStatusPlugin['setup']>;
 export type CoreAppStatusPluginStart = ReturnType<CoreAppStatusPlugin['start']>;

test/plugin_functional/plugins/core_app_status/public/types.ts

@@ -0,0 +1,26 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { CoreAppStatusPluginStart } from './plugin';
+
+declare global {
+  interface Window {
+    __coreAppStatus: CoreAppStatusPluginStart;
+  }
+}

test/plugin_functional/test_suites/core_plugins/application_status.ts

@@ -24,50 +24,32 @@ import {
   AppUpdatableFields,
 } from '../../../../src/core/public/application/types';
 import { PluginFunctionalProviderContext } from '../../services';
-import { CoreAppStatusPluginStart } from '../../plugins/core_app_status/public/plugin';
-import '../../plugins/core_provider_plugin/types';
+import '../../plugins/core_app_status/public/types';
 
 // eslint-disable-next-line import/no-default-export
 export default function({ getService, getPageObjects }: PluginFunctionalProviderContext) {
   const PageObjects = getPageObjects(['common']);
   const browser = getService('browser');
   const appsMenu = getService('appsMenu');
+  const testSubjects = getService('testSubjects');
 
   const setAppStatus = async (s: Partial<AppUpdatableFields>) => {
-    await browser.executeAsync(async (status: Partial<AppUpdatableFields>, cb: Function) => {
-      const plugin = window.__coreProvider.start.plugins
-        .core_app_status as CoreAppStatusPluginStart;
-      plugin.setAppStatus(status);
+    return browser.executeAsync(async (status: Partial<AppUpdatableFields>, cb: Function) => {
+      window.__coreAppStatus.setAppStatus(status);
       cb();
     }, s);
   };
 
-  const navigateToApp = async (i: string): Promise<{ error?: string }> => {
+  const navigateToApp = async (i: string) => {
     return (await browser.executeAsync(async (appId, cb: Function) => {
-      // navigating in legacy mode performs a page refresh
-      // and webdriver seems to re-execute the script after the reload
-      // as it considers it didn't end on the previous session.
-      // however when testing navigation to NP app, __coreProvider is not accessible
-      // so we need to check for existence.
-      if (!window.__coreProvider) {
-        cb({});
-      }
-      const plugin = window.__coreProvider.start.plugins
-        .core_app_status as CoreAppStatusPluginStart;
-      try {
-        await plugin.navigateToApp(appId);
-        cb({});
-      } catch (e) {
-        cb({
-          error: e.message,
-        });
-      }
+      await window.__coreAppStatus.navigateToApp(appId);
+      cb();
     }, i)) as any;
   };
 
   describe('application status management', () => {
     beforeEach(async () => {
-      await PageObjects.common.navigateToApp('settings');
+      await PageObjects.common.navigateToApp('app_status_start');
     });
 
     it('can change the navLink status at runtime', async () => {
@@ -98,10 +80,10 @@ export default function({ getService, getPageObjects }: PluginFunctionalProvider
         status: AppStatus.inaccessible,
       });
 
-      const result = await navigateToApp('app_status');
-      expect(result.error).to.contain(
-        'Trying to navigate to an inaccessible application: app_status'
-      );
+      await navigateToApp('app_status');
+
+      expect(await testSubjects.exists('appNotFoundPageContent')).to.eql(true);
+      expect(await testSubjects.exists('appStatusApp')).to.eql(false);
     });
 
     it('allows to navigate to an accessible app', async () => {
@@ -109,8 +91,35 @@ export default function({ getService, getPageObjects }: PluginFunctionalProvider
         status: AppStatus.accessible,
       });
 
-      const result = await navigateToApp('app_status');
-      expect(result.error).to.eql(undefined);
+      await navigateToApp('app_status');
+
+      expect(await testSubjects.exists('appNotFoundPageContent')).to.eql(false);
+      expect(await testSubjects.exists('appStatusApp')).to.eql(true);
+    });
+
+    it('can change the state of the currently mounted app', async () => {
+      await setAppStatus({
+        status: AppStatus.accessible,
+      });
+
+      await navigateToApp('app_status');
+
+      expect(await testSubjects.exists('appNotFoundPageContent')).to.eql(false);
+      expect(await testSubjects.exists('appStatusApp')).to.eql(true);
+
+      await setAppStatus({
+        status: AppStatus.inaccessible,
+      });
+
+      expect(await testSubjects.exists('appNotFoundPageContent')).to.eql(true);
+      expect(await testSubjects.exists('appStatusApp')).to.eql(false);
+
+      await setAppStatus({
+        status: AppStatus.accessible,
+      });
+
+      expect(await testSubjects.exists('appNotFoundPageContent')).to.eql(false);
+      expect(await testSubjects.exists('appStatusApp')).to.eql(true);
     });
   });
 }