From 3077534f0ecb9a6a233bfd128e1bc13d92d25cec Mon Sep 17 00:00:00 2001 From: kobelb Date: Fri, 24 Jan 2020 13:06:24 -0800 Subject: [PATCH 1/3] Specifying valid licenses for the Graph feature --- x-pack/legacy/plugins/graph/index.ts | 1 + x-pack/plugins/features/common/feature.ts | 2 +- x-pack/plugins/features/server/feature_schema.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/legacy/plugins/graph/index.ts b/x-pack/legacy/plugins/graph/index.ts index f798fa5e9f39d4..143d07cfdbd579 100644 --- a/x-pack/legacy/plugins/graph/index.ts +++ b/x-pack/legacy/plugins/graph/index.ts @@ -61,6 +61,7 @@ export const graph: LegacyPluginInitializer = kibana => { navLinkId: 'graph', app: ['graph', 'kibana'], catalogue: ['graph'], + validLicenses: ['platinum', 'enterprise', 'trial'], privileges: { all: { savedObject: { diff --git a/x-pack/plugins/features/common/feature.ts b/x-pack/plugins/features/common/feature.ts index 423fe1eb997040..748076b95ad77f 100644 --- a/x-pack/plugins/features/common/feature.ts +++ b/x-pack/plugins/features/common/feature.ts @@ -43,7 +43,7 @@ export interface Feature< * This does not restrict access to your feature based on license. * Its only purpose is to inform the space and roles UIs on which features to display. */ - validLicenses?: Array<'basic' | 'standard' | 'gold' | 'platinum' | 'enterprise'>; + validLicenses?: Array<'basic' | 'standard' | 'gold' | 'platinum' | 'enterprise' | 'trial'>; /** * An optional EUI Icon to be used when displaying your feature. diff --git a/x-pack/plugins/features/server/feature_schema.ts b/x-pack/plugins/features/server/feature_schema.ts index 7732686db5ee14..cc12ea1b78dce0 100644 --- a/x-pack/plugins/features/server/feature_schema.ts +++ b/x-pack/plugins/features/server/feature_schema.ts @@ -51,7 +51,7 @@ const schema = Joi.object({ name: Joi.string().required(), excludeFromBasePrivileges: Joi.boolean(), validLicenses: Joi.array().items( - Joi.string().valid('basic', 'standard', 'gold', 'platinum', 'enterprise') + Joi.string().valid('basic', 'standard', 'gold', 'platinum', 'enterprise', 'trial') ), icon: Joi.string(), description: Joi.string(), From 10147779f6b86c630aa8433fd127b9dbc8faacb8 Mon Sep 17 00:00:00 2001 From: kobelb Date: Fri, 24 Jan 2020 15:31:40 -0800 Subject: [PATCH 2/3] Adding option to /api/features to ignore valid licenses This allow us to take advantage of the /api/featues endpoint within our tests to disable all features, including those which are disabled by the current license. The ui capabilities don't take into considerating the license at the moment, so they're separate entirely separeate mechanisms at this point in time. --- .../features/server/routes/index.test.ts | 65 ++++++++++++++++++- .../plugins/features/server/routes/index.ts | 8 ++- .../common/services/features.ts | 6 +- .../spaces_only/tests/index.ts | 3 +- 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/features/server/routes/index.test.ts b/x-pack/plugins/features/server/routes/index.test.ts index 98a23a61d542c8..b0f8417b7175da 100644 --- a/x-pack/plugins/features/server/routes/index.test.ts +++ b/x-pack/plugins/features/server/routes/index.test.ts @@ -53,7 +53,7 @@ describe('GET /api/features', () => { it('returns a list of available features', async () => { const mockResponse = httpServerMock.createResponseFactory(); - routeHandler(undefined as any, undefined as any, mockResponse); + routeHandler(undefined as any, { query: {} } as any, mockResponse); expect(mockResponse.ok.mock.calls).toMatchInlineSnapshot(` Array [ @@ -84,11 +84,11 @@ describe('GET /api/features', () => { `); }); - it(`does not return features that arent allowed by current license`, async () => { + it(`by default does not return features that arent allowed by current license`, async () => { currentLicenseLevel = 'basic'; const mockResponse = httpServerMock.createResponseFactory(); - routeHandler(undefined as any, undefined as any, mockResponse); + routeHandler(undefined as any, { query: {} } as any, mockResponse); expect(mockResponse.ok.mock.calls).toMatchInlineSnapshot(` Array [ @@ -107,4 +107,63 @@ describe('GET /api/features', () => { ] `); }); + + it(`ignoreValidLicenses=false does not return features that arent allowed by current license`, async () => { + currentLicenseLevel = 'basic'; + + const mockResponse = httpServerMock.createResponseFactory(); + routeHandler(undefined as any, { query: { ignoreValidLicenses: false } } as any, mockResponse); + + expect(mockResponse.ok.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "body": Array [ + Object { + "app": Array [], + "id": "feature_1", + "name": "Feature 1", + "privileges": Object {}, + }, + ], + }, + ], + ] + `); + }); + + it(`ignoreValidLicenses=true returns features that arent allowed by current license`, async () => { + currentLicenseLevel = 'basic'; + + const mockResponse = httpServerMock.createResponseFactory(); + routeHandler(undefined as any, { query: { ignoreValidLicenses: true } } as any, mockResponse); + + expect(mockResponse.ok.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "body": Array [ + Object { + "app": Array [], + "id": "feature_1", + "name": "Feature 1", + "privileges": Object {}, + }, + Object { + "app": Array [ + "bar-app", + ], + "id": "licensed_feature", + "name": "Licensed Feature", + "privileges": Object {}, + "validLicenses": Array [ + "gold", + ], + }, + ], + }, + ], + ] + `); + }); }); diff --git a/x-pack/plugins/features/server/routes/index.ts b/x-pack/plugins/features/server/routes/index.ts index 51869c39cf83c1..49446a8a360bf2 100644 --- a/x-pack/plugins/features/server/routes/index.ts +++ b/x-pack/plugins/features/server/routes/index.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { schema } from '@kbn/config-schema'; import { IRouter } from '../../../../../src/core/server'; import { LegacyAPI } from '../plugin'; import { FeatureRegistry } from '../feature_registry'; @@ -19,13 +20,18 @@ export interface RouteDefinitionParams { export function defineRoutes({ router, featureRegistry, getLegacyAPI }: RouteDefinitionParams) { router.get( - { path: '/api/features', options: { tags: ['access:features'] }, validate: false }, + { + path: '/api/features', + options: { tags: ['access:features'] }, + validate: { query: schema.object({ ignoreValidLicenses: schema.maybe(schema.boolean()) }) }, + }, (context, request, response) => { const allFeatures = featureRegistry.getAll(); return response.ok({ body: allFeatures.filter( feature => + request.query.ignoreValidLicenses === true || !feature.validLicenses || !feature.validLicenses.length || getLegacyAPI().xpackInfo.license.isOneOf(feature.validLicenses) diff --git a/x-pack/test/ui_capabilities/common/services/features.ts b/x-pack/test/ui_capabilities/common/services/features.ts index 9f644fd6d0f6e6..830c9a556d2e1c 100644 --- a/x-pack/test/ui_capabilities/common/services/features.ts +++ b/x-pack/test/ui_capabilities/common/services/features.ts @@ -22,9 +22,11 @@ export class FeaturesService { }); } - public async get(): Promise { + public async get({ ignoreValidLicenses } = { ignoreValidLicenses: false }): Promise { this.log.debug('requesting /api/features to get the features'); - const response = await this.axios.get('/api/features'); + const response = await this.axios.get( + `/api/features${ignoreValidLicenses ? '?ignoreValidLicenses=true' : ''}` + ); if (response.status !== 200) { throw new Error( diff --git a/x-pack/test/ui_capabilities/spaces_only/tests/index.ts b/x-pack/test/ui_capabilities/spaces_only/tests/index.ts index 0b40f9716dcb4e..a25838ac4f76d3 100644 --- a/x-pack/test/ui_capabilities/spaces_only/tests/index.ts +++ b/x-pack/test/ui_capabilities/spaces_only/tests/index.ts @@ -16,7 +16,8 @@ export default function uiCapabilitesTests({ loadTestFile, getService }: FtrProv this.tags('ciGroup9'); before(async () => { - const features = await featuresService.get(); + // we're using a basic license, so if we want to disable all features, we have to ignore the valid licenses + const features = await featuresService.get({ ignoreValidLicenses: true }); for (const space of SpaceScenarios) { const disabledFeatures = space.disabledFeatures === '*' ? Object.keys(features) : space.disabledFeatures; From f34f9a6857c8675f7ea904dd427f6e2df81b747d Mon Sep 17 00:00:00 2001 From: kobelb Date: Mon, 27 Jan 2020 06:33:14 -0800 Subject: [PATCH 3/3] Addressing PR comments --- x-pack/plugins/features/server/routes/index.ts | 6 ++++-- x-pack/test/ui_capabilities/common/services/features.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/features/server/routes/index.ts b/x-pack/plugins/features/server/routes/index.ts index 49446a8a360bf2..cf4d61ccac88b4 100644 --- a/x-pack/plugins/features/server/routes/index.ts +++ b/x-pack/plugins/features/server/routes/index.ts @@ -23,7 +23,9 @@ export function defineRoutes({ router, featureRegistry, getLegacyAPI }: RouteDef { path: '/api/features', options: { tags: ['access:features'] }, - validate: { query: schema.object({ ignoreValidLicenses: schema.maybe(schema.boolean()) }) }, + validate: { + query: schema.object({ ignoreValidLicenses: schema.boolean({ defaultValue: false }) }), + }, }, (context, request, response) => { const allFeatures = featureRegistry.getAll(); @@ -31,7 +33,7 @@ export function defineRoutes({ router, featureRegistry, getLegacyAPI }: RouteDef return response.ok({ body: allFeatures.filter( feature => - request.query.ignoreValidLicenses === true || + request.query.ignoreValidLicenses || !feature.validLicenses || !feature.validLicenses.length || getLegacyAPI().xpackInfo.license.isOneOf(feature.validLicenses) diff --git a/x-pack/test/ui_capabilities/common/services/features.ts b/x-pack/test/ui_capabilities/common/services/features.ts index 830c9a556d2e1c..0f796c1d0a0cc0 100644 --- a/x-pack/test/ui_capabilities/common/services/features.ts +++ b/x-pack/test/ui_capabilities/common/services/features.ts @@ -25,7 +25,7 @@ export class FeaturesService { public async get({ ignoreValidLicenses } = { ignoreValidLicenses: false }): Promise { this.log.debug('requesting /api/features to get the features'); const response = await this.axios.get( - `/api/features${ignoreValidLicenses ? '?ignoreValidLicenses=true' : ''}` + `/api/features?ignoreValidLicenses=${ignoreValidLicenses}` ); if (response.status !== 200) {