Skip to content

Commit

Permalink
Make the Chrome project API "protected" for the Serverless plugin (#1…
Browse files Browse the repository at this point in the history
…57307)

## Summary

Addresses
#156600 (comment)

> Let's think if there is a way to throw an error when core chrome api
are called from invalid plugins (in this cases only the serverless
plugin would be allowed.

This PR can be a starting point for discussion on the behavior we really
want. This PR has a simple goal to ensure that non-serverless plugins do
not call the `chrome.projects` API. However, it's not complete security,
as the compile-time error would be easy to override.

cc @sebelga @Dosant @clintandrewhall 

---
---


### Checklist

Delete any items that are not applicable to this PR.

- [x] Documentation was added for features that require explanation or
tutorials
Internal documentation:
https://docs.google.com/document/d/1ew8KYl6ROs_V7jeIXgeP_C9YgkYK2IPtuceo6KVF_jE/edit#

---------

Co-authored-by: Clint Andrew Hall <clint@clintandrewhall.com>
  • Loading branch information
tsullivan and clintandrewhall committed May 16, 2023
1 parent 0c88d3a commit 4805421
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,24 @@ export class ChromeService {
};

const setProjectSideNavComponent = (component: ISideNavComponent | null) => {
const chromeStyle = chromeStyle$.getValue();
if (chromeStyle !== 'project') {
// Helps ensure callers go through the serverless plugin to get here.
throw new Error(
`Invalid ChromeStyle value of "${chromeStyle}". setProjectSideNavComponent requires ChromeStyle set to "project".`
);
}
projectNavigation.setProjectSideNavComponent(component);
};

const setProjectNavigation = (config: ChromeProjectNavigation) => {
const chromeStyle = chromeStyle$.getValue();
if (chromeStyle !== 'project') {
// Helps ensure callers go through the serverless plugin to get here.
throw new Error(
`Invalid ChromeStyle value of "${chromeStyle}". setProjectNavigation requires ChromeStyle set to "project".`
);
}
projectNavigation.setProjectNavigation(config);
};

Expand Down
30 changes: 29 additions & 1 deletion packages/core/chrome/core-chrome-browser-internal/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
* Side Public License, v 1.
*/

import type {
ChromeProjectNavigation,
ChromeStart,
SideNavComponent,
} from '@kbn/core-chrome-browser';
import type { Observable } from 'rxjs';
import type { ChromeStart } from '@kbn/core-chrome-browser';

/** @internal */
export interface InternalChromeStart extends ChromeStart {
Expand All @@ -23,4 +27,28 @@ export interface InternalChromeStart extends ChromeStart {
* @internal
*/
getBodyClasses$(): Observable<string[]>;

/**
* Used only by the serverless plugin to customize project-style chrome.
* Use {@link ServerlessPluginStart.setSideNavComponent} to set serverless navigation.
*/
project: {
/**
* Sets the project navigation config to be used for rendering project navigation.
* It is used for default project sidenav, project breadcrumbs, tracking active deep link.
* @param projectNavigation The project navigation config
*
* @remarks Has no effect if the chrome style is not `project`.
*/
setNavigation(projectNavigation: ChromeProjectNavigation): void;

/**
* Set custom project sidenav component to be used instead of the default project sidenav.
* @param getter A function returning a CustomNavigationComponent.
* This component will receive Chrome navigation state as props (not yet implemented)
*
* @remarks Has no effect if the chrome style is not `project`.
*/
setSideNavComponent(component: SideNavComponent | null): void;
};
}
23 changes: 0 additions & 23 deletions packages/core/chrome/core-chrome-browser/src/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import type { ChromeHelpExtension } from './help_extension';
import type { ChromeBreadcrumb, ChromeBreadcrumbsAppendExtension } from './breadcrumb';
import type { ChromeBadge, ChromeStyle, ChromeUserBanner } from './types';
import type { ChromeGlobalHelpExtensionMenuLink } from './help_extension';
import type { ChromeProjectNavigation, SideNavComponent } from './project_navigation';

/**
* ChromeStart allows plugins to customize the global chrome header UI and
Expand Down Expand Up @@ -162,26 +161,4 @@ export interface ChromeStart {
* Get an observable of the current style type of the chrome.
*/
getChromeStyle$(): Observable<ChromeStyle>;
/**
* Configuration for serverless projects
*/
project: {
/**
* Sets the project navigation config to be used for rendering project navigation.
* It is used for default project sidenav, project breadcrumbs, tracking active deep link.
* @param projectNavigation The project navigation config
*
* @remarks Has no effect if the chrome style is not `project`.
*/
setNavigation(projectNavigation: ChromeProjectNavigation): void;

/**
* Set custom project sidenav component to be used instead of the default project sidenav.
* @param getter A function returning a CustomNavigationComponent.
* This component will receive Chrome navigation state as props (not yet implemented)
*
* @remarks Has no effect if the chrome style is not `project`.
*/
setSideNavComponent(component: SideNavComponent | null): void;
};
}
18 changes: 9 additions & 9 deletions x-pack/plugins/serverless/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,21 @@
* 2.0.
*/

import React from 'react';
import ReactDOM from 'react-dom';

import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { InternalChromeStart } from '@kbn/core-chrome-browser-internal';
import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '@kbn/core/public';
import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { ProjectSwitcher, ProjectSwitcherKibanaProvider } from '@kbn/serverless-project-switcher';
import { ProjectType } from '@kbn/serverless-types';

import React from 'react';
import ReactDOM from 'react-dom';
import { API_SWITCH_PROJECT as projectChangeAPIUrl } from '../common';
import { ServerlessConfig } from './config';
import {
ServerlessPluginSetup,
ServerlessPluginStart,
ServerlessPluginSetupDependencies,
ServerlessPluginStart,
ServerlessPluginStartDependencies,
} from './types';
import { ServerlessConfig } from './config';
import { API_SWITCH_PROJECT as projectChangeAPIUrl } from '../common';

export class ServerlessPlugin
implements
Expand Down Expand Up @@ -64,7 +63,8 @@ export class ServerlessPlugin

return {
setSideNavComponent: (sideNavigationComponent) =>
core.chrome.project.setSideNavComponent(sideNavigationComponent),
// Casting the "chrome.projects" service to an "internal" type: this is intentional to obscure the property from Typescript.
(core.chrome as InternalChromeStart).project.setSideNavComponent(sideNavigationComponent),
};
}

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/serverless/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@
"@kbn/serverless-types",
"@kbn/utils",
"@kbn/core-chrome-browser",
"@kbn/core-chrome-browser-internal",
]
}
4 changes: 2 additions & 2 deletions x-pack/plugins/serverless_search/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ export class ServerlessSearchPlugin

public start(
core: CoreStart,
_startDeps: ServerlessSearchPluginStartDependencies
{ serverless }: ServerlessSearchPluginStartDependencies
): ServerlessSearchPluginStart {
core.chrome.project.setSideNavComponent(createComponent(core));
serverless.setSideNavComponent(createComponent(core));
return {};
}

Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/serverless_search/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
* 2.0.
*/

import { ManagementSetup, ManagementStart } from '@kbn/management-plugin/public';
import {
EnterpriseSearchPublicSetup,
EnterpriseSearchPublicStart,
} from '@kbn/enterprise-search-plugin/public';
import { ManagementSetup, ManagementStart } from '@kbn/management-plugin/public';
import { ServerlessPluginSetup, ServerlessPluginStart } from '@kbn/serverless/public';

// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface ServerlessSearchPluginSetup {}
Expand All @@ -20,9 +21,11 @@ export interface ServerlessSearchPluginStart {}
export interface ServerlessSearchPluginSetupDependencies {
enterpriseSearch: EnterpriseSearchPublicSetup;
management: ManagementSetup;
serverless: ServerlessPluginSetup;
}

export interface ServerlessSearchPluginStartDependencies {
enterpriseSearch: EnterpriseSearchPublicStart;
management: ManagementStart;
serverless: ServerlessPluginStart;
}
1 change: 1 addition & 0 deletions x-pack/plugins/serverless_search/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@
"@kbn/i18n",
"@kbn/kibana-react-plugin",
"@kbn/i18n-react",
"@kbn/serverless",
]
}

0 comments on commit 4805421

Please sign in to comment.