Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract License service from CCR and Watcher into license_api_guard plugin in x-pack #95973

Merged
merged 20 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
a66cff2
Restrict CCR license-checking to API requests and localize error mess…
cjcenizal Mar 31, 2021
394ff8a
Extract License service into es_ui_shared and consume it in CCR.
cjcenizal Apr 2, 2021
ff254aa
Consume License service in Watcher.
cjcenizal Apr 2, 2021
87c0a21
Add unit tests for License service.
cjcenizal Apr 2, 2021
5d1e3cf
Merge branch 'master' into bug/ccr-watcher-license-messages
cjcenizal Apr 5, 2021
7c34572
Fix i18n and TS errors. Refactor license test.
cjcenizal Apr 5, 2021
25bc327
Fix TS errors.
cjcenizal Apr 5, 2021
f6002cc
Fix more TS errors.
cjcenizal Apr 5, 2021
818f6da
Merge branch 'master' into bug/ccr-watcher-license-messages
cjcenizal Apr 9, 2021
7d8eba5
Merge branch 'master' into bug/ccr-watcher-license-messages
cjcenizal Apr 13, 2021
974f824
Move License service from esUiShared to licenseApiGuard plugin in x-p…
cjcenizal Apr 13, 2021
efd968e
Use httpServerMock and licensingMock helpers instead of hand-rolling …
cjcenizal Apr 14, 2021
066a69f
Merge branch 'master' into bug/ccr-watcher-license-messages
cjcenizal Apr 14, 2021
7448fb1
Fix i18n error.
cjcenizal Apr 14, 2021
48293ff
Fix tsconfig errors.
cjcenizal Apr 14, 2021
a0fe425
Add plugin to i18nrc.
cjcenizal Apr 14, 2021
5701904
Update plugin list docs.
cjcenizal Apr 15, 2021
0706a25
Merge branch 'master' into bug/ccr-watcher-license-messages
cjcenizal Apr 15, 2021
3add225
Make isEsSecurityEnabled a public method.
cjcenizal Apr 15, 2021
d54f85c
Use forbidden instead of customError.
cjcenizal Apr 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plugins/es_ui_shared/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

export { isEsError, handleEsError, parseEsError } from './errors';
export { License } from './license';

/** dummy plugin*/
export function plugin() {
Expand Down
9 changes: 9 additions & 0 deletions src/plugins/es_ui_shared/server/license/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export { License } from './license';
105 changes: 105 additions & 0 deletions src/plugins/es_ui_shared/server/license/license.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { KibanaRequest, RequestHandlerContext } from 'src/core/server';

import { License } from './license';

/* eslint-disable @kbn/eslint/no-restricted-paths */
import type { LicenseCheckState } from '../../../../../x-pack/plugins/licensing/common/types';
/* eslint-enable @kbn/eslint/no-restricted-paths */

describe('license_pre_routing_factory', () => {
describe('#reportingFeaturePreRoutingFactory', () => {
const pluginName = 'testPlugin';
const currentLicenseType = 'currentLicenseType';

const testRoute = ({ licenseState }: { licenseState: LicenseCheckState }) => {
const license = new License();
const logger = {
warn: jest.fn(),
};

const licensingMock = {
license$: {
subscribe: (callback: (config: unknown) => {}) =>
callback({
type: currentLicenseType,
check: (): { state: LicenseCheckState } => ({ state: licenseState }),
getFeature: () => ({}),
}),
},
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to replace this duck typed implementation with a license created by the mock functionality from the licensing/server plugin:

import { licensingMock } from '../../../../../licensing/server/mocks';

...
callback(licensingMock.createLicense({ /* config */ }))
...

That way if the interface is updated this usage instance should be picked up in future refactors.

[Really minor nit]
we can use rxjs to create a sync observable.

The final result could be (did not test!):

import { of } from 'rxjs';
import { licensingMock } from '../../../../../licensing/server/mocks';
...
      const licensingMock = {
        license$: of(licensingMock.createLicense({ license: { type: currentLicenseType, status: ... });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing me to that mock! I'll try out the sync observable approach.


license.setup({ pluginName, logger });
license.start({
pluginId: 'id',
minimumLicenseType: 'basic',
licensing: licensingMock,
});

const route = jest.fn();
const guardedRoute = license.guardApiRoute(route);
const customError = jest.fn();
guardedRoute({} as RequestHandlerContext, {} as KibanaRequest, { customError });

return {
errorResponse:
customError.mock.calls.length > 0
? customError.mock.calls[customError.mock.calls.length - 1][0]
: undefined,
logMesssage:
logger.warn.mock.calls.length > 0
? logger.warn.mock.calls[logger.warn.mock.calls.length - 1][0]
: undefined,
route,
};
};

describe('valid license', () => {
it('the original route is called and nothing is logged', () => {
const { errorResponse, logMesssage, route } = testRoute({ licenseState: 'valid' });

expect(errorResponse).toBeUndefined();
expect(logMesssage).toBeUndefined();
expect(route).toHaveBeenCalled();
});
});

[
{
licenseState: 'invalid',
expectedMessage: `Your ${currentLicenseType} license does not support ${pluginName}. Please upgrade your license.`,
},
{
licenseState: 'expired',
expectedMessage: `You cannot use ${pluginName} because your ${currentLicenseType} license has expired.`,
},
{
licenseState: 'unavailable',
expectedMessage: `You cannot use ${pluginName} because license information is not available at this time.`,
},
].forEach(({ licenseState, expectedMessage }) => {
describe(`${licenseState} license`, () => {
it('replies with 403 and message and logs the message', () => {
const { errorResponse, logMesssage, route } = testRoute({ licenseState });

expect(errorResponse).toEqual({
body: {
message: expectedMessage,
},
statusCode: 403,
});

expect(logMesssage).toBe(expectedMessage);
expect(route).not.toHaveBeenCalled();
});
});
});
});
});
124 changes: 124 additions & 0 deletions src/plugins/es_ui_shared/server/license/license.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { i18n } from '@kbn/i18n';
import {
Logger,
KibanaRequest,
KibanaResponseFactory,
RequestHandler,
RequestHandlerContext,
} from 'src/core/server';

/* eslint-disable @kbn/eslint/no-restricted-paths */
Copy link
Contributor Author

@cjcenizal cjcenizal Apr 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elastic/kibana-core Do we still need this linting rule to disallow importing x-pack modules into non-x-pack?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For concrete imports, we definitely do, as it would plainly breaks on OSS distrib.

For types, it's not a 'technical' requirement, but until the basic plugins are moved out of xpack, we should stick with that rule.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK we don't release OSS distributable starting from v7.11 https://www.elastic.co/downloads/past-releases#kibana-oss

But I'd rather plugins didn't import from x-pack. Note that the code in src/ and x-pack have different licenses, so it's easy to leak commercial code outside by importing runtime code accidentally.
Regarding the current use case: why licensing code lives in src/ at all? Licensing compatible types can be declared in the current file, but maybe an x-pack plugin is a better place?

import type {
ILicense,
LicenseType,
LicenseCheckState,
} from '../../../../../x-pack/plugins/licensing/common/types';
import type { LicensingPluginStart } from '../../../../../x-pack/plugins/licensing/server/types';
/* eslint-enable @kbn/eslint/no-restricted-paths */

type LicenseLogger = Pick<Logger, 'warn'>;
type LicenseDependency = Pick<LicensingPluginStart, 'license$'>;

interface SetupSettings {
pluginName: string;
logger: LicenseLogger;
}

interface StartSettings {
pluginId: string;
minimumLicenseType: LicenseType;
licensing: LicenseDependency;
}

export class License {
private pluginName?: string;
private logger?: LicenseLogger;
private licenseCheckState: LicenseCheckState = 'unavailable';
private licenseType?: LicenseType;

private _isEsSecurityEnabled: boolean = false;

setup({ pluginName, logger }: SetupSettings) {
this.pluginName = pluginName;
this.logger = logger;
}

start({ pluginId, minimumLicenseType, licensing }: StartSettings) {
licensing.license$.subscribe((license: ILicense) => {
this.licenseType = license.type;
this.licenseCheckState = license.check(pluginId, minimumLicenseType!).state;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To testers: you can hardcode a value here, e.g. 'invalid', to test the behavior under various conditions.


// Retrieving security checks the results of GET /_xpack as well as license state,
// so we're also checking whether security is disabled in elasticsearch.yml.
this._isEsSecurityEnabled = license.getFeature('security').isEnabled;
});
}

getLicenseErrorMessage(licenseCheckState: LicenseCheckState): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit; if this does not need to be public I would mark it as private.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

switch (licenseCheckState) {
case 'invalid':
return i18n.translate('esUi.license.errorUnsupportedMessage', {
defaultMessage:
'Your {licenseType} license does not support {pluginName}. Please upgrade your license.',
values: { licenseType: this.licenseType!, pluginName: this.pluginName },
});

case 'expired':
return i18n.translate('esUi.license.errorExpiredMessage', {
defaultMessage:
'You cannot use {pluginName} because your {licenseType} license has expired.',
values: { licenseType: this.licenseType!, pluginName: this.pluginName },
});

case 'unavailable':
return i18n.translate('esUi.license.errorUnavailableMessage', {
defaultMessage:
'You cannot use {pluginName} because license information is not available at this time.',
values: { pluginName: this.pluginName },
});
}

return i18n.translate('esUi.license.genericErrorMessage', {
defaultMessage: 'You cannot use {pluginName} because the license check failed.',
values: { pluginName: this.pluginName },
});
}

guardApiRoute<Context extends RequestHandlerContext, Params, Query, Body>(
handler: RequestHandler<Params, Query, Body, Context>
) {
return (
ctx: Context,
request: KibanaRequest<Params, Query, Body>,
response: KibanaResponseFactory
) => {
// We'll only surface license errors if users attempt disallowed access to the API.
if (this.licenseCheckState !== 'valid') {
const licenseErrorMessage = this.getLicenseErrorMessage(this.licenseCheckState);
this.logger?.warn(licenseErrorMessage);

return response.customError({
body: {
message: licenseErrorMessage,
},
statusCode: 403,
});
}

return handler(ctx, request, response);
};
}

// eslint-disable-next-line @typescript-eslint/explicit-member-accessibility
get isEsSecurityEnabled() {
return this._isEsSecurityEnabled;
}
}
1 change: 1 addition & 0 deletions src/plugins/es_ui_shared/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"static/**/*"
],
"references": [
{ "path": "../../../x-pack/plugins/licensing/tsconfig.json" },
{ "path": "../../core/tsconfig.json" },
{ "path": "../data/tsconfig.json" },
]
Expand Down
37 changes: 15 additions & 22 deletions x-pack/plugins/cross_cluster_replication/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@

import { Observable } from 'rxjs';
import { first } from 'rxjs/operators';
import { i18n } from '@kbn/i18n';
import {
CoreSetup,
CoreStart,
ILegacyCustomClusterClient,
Plugin,
Logger,
Expand All @@ -19,12 +19,11 @@ import {

import { Index } from '../../index_management/server';
import { PLUGIN } from '../common/constants';
import type { Dependencies, CcrRequestHandlerContext } from './types';
import { SetupDependencies, StartDependencies, CcrRequestHandlerContext } from './types';
import { registerApiRoutes } from './routes';
import { License } from './services';
import { elasticsearchJsPlugin } from './client/elasticsearch_ccr';
import { CrossClusterReplicationConfig } from './config';
import { isEsError } from './shared_imports';
import { License, isEsError } from './shared_imports';
import { formatEsError } from './lib/format_es_error';

async function getCustomEsClient(getStartServices: CoreSetup['getStartServices']) {
Expand Down Expand Up @@ -77,7 +76,7 @@ export class CrossClusterReplicationServerPlugin implements Plugin<void, void, a

setup(
{ http, getStartServices }: CoreSetup,
{ features, licensing, indexManagement, remoteClusters }: Dependencies
{ features, licensing, indexManagement, remoteClusters }: SetupDependencies
) {
this.config$
.pipe(first())
Expand All @@ -97,22 +96,10 @@ export class CrossClusterReplicationServerPlugin implements Plugin<void, void, a
}
});

this.license.setup(
{
pluginId: PLUGIN.ID,
minimumLicenseType: PLUGIN.minimumLicenseType,
defaultErrorMessage: i18n.translate(
'xpack.crossClusterReplication.licenseCheckErrorMessage',
{
defaultMessage: 'License check failed',
}
),
},
{
licensing,
logger: this.logger,
}
);
this.license.setup({
pluginName: PLUGIN.TITLE,
logger: this.logger,
});

features.registerElasticsearchFeature({
id: 'cross_cluster_replication',
Expand Down Expand Up @@ -147,7 +134,13 @@ export class CrossClusterReplicationServerPlugin implements Plugin<void, void, a
});
}

start() {}
start(core: CoreStart, { licensing }: StartDependencies) {
this.license.start({
pluginId: PLUGIN.ID,
minimumLicenseType: PLUGIN.minimumLicenseType,
licensing,
});
}

stop() {
if (this.ccrEsClient) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
import { httpServiceMock, httpServerMock } from 'src/core/server/mocks';
import { kibanaResponseFactory, RequestHandler } from 'src/core/server';

import { isEsError } from '../../../shared_imports';
import { isEsError, License } from '../../../shared_imports';
import { formatEsError } from '../../../lib/format_es_error';
import { License } from '../../../services';
import { mockRouteContext } from '../test_lib';
import { registerCreateRoute } from './register_create_route';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
import { httpServiceMock, httpServerMock } from 'src/core/server/mocks';
import { kibanaResponseFactory, RequestHandler } from 'src/core/server';

import { isEsError } from '../../../shared_imports';
import { isEsError, License } from '../../../shared_imports';
import { formatEsError } from '../../../lib/format_es_error';
import { License } from '../../../services';
import { mockRouteContext } from '../test_lib';
import { registerDeleteRoute } from './register_delete_route';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
import { httpServiceMock, httpServerMock } from 'src/core/server/mocks';
import { kibanaResponseFactory, RequestHandler } from 'src/core/server';

import { isEsError } from '../../../shared_imports';
import { isEsError, License } from '../../../shared_imports';
import { formatEsError } from '../../../lib/format_es_error';
import { License } from '../../../services';
import { mockRouteContext } from '../test_lib';
import { registerFetchRoute } from './register_fetch_route';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
import { httpServiceMock, httpServerMock } from 'src/core/server/mocks';
import { kibanaResponseFactory, RequestHandler } from 'src/core/server';

import { isEsError } from '../../../shared_imports';
import { isEsError, License } from '../../../shared_imports';
import { formatEsError } from '../../../lib/format_es_error';
import { License } from '../../../services';
import { mockRouteContext } from '../test_lib';
import { registerGetRoute } from './register_get_route';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
import { httpServiceMock, httpServerMock } from 'src/core/server/mocks';
import { kibanaResponseFactory, RequestHandler } from 'src/core/server';

import { isEsError } from '../../../shared_imports';
import { isEsError, License } from '../../../shared_imports';
import { formatEsError } from '../../../lib/format_es_error';
import { License } from '../../../services';
import { mockRouteContext } from '../test_lib';
import { registerPauseRoute } from './register_pause_route';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
import { httpServiceMock, httpServerMock } from 'src/core/server/mocks';
import { kibanaResponseFactory, RequestHandler } from 'src/core/server';

import { isEsError } from '../../../shared_imports';
import { isEsError, License } from '../../../shared_imports';
import { formatEsError } from '../../../lib/format_es_error';
import { License } from '../../../services';
import { mockRouteContext } from '../test_lib';
import { registerResumeRoute } from './register_resume_route';

Expand Down
Loading