diff --git a/src/cli/serve/serve.js b/src/cli/serve/serve.js index 976eac7f95da6..6b13d0dc32d3f 100644 --- a/src/cli/serve/serve.js +++ b/src/cli/serve/serve.js @@ -140,23 +140,12 @@ function applyConfigOverrides(rawConfig, opts, extraCliOptions) { } set('plugins.scanDirs', _.compact([].concat(get('plugins.scanDirs'), opts.pluginDir))); - set( 'plugins.paths', _.compact( [].concat( get('plugins.paths'), opts.pluginPath, - opts.runExamples - ? [ - // Ideally this would automatically include all plugins in the examples dir - fromRoot('examples/demo_search'), - fromRoot('examples/search_explorer'), - fromRoot('examples/embeddable_examples'), - fromRoot('examples/embeddable_explorer'), - ] - : [], - XPACK_INSTALLED && !opts.oss ? [XPACK_DIR] : [] ) ) @@ -253,6 +242,7 @@ export default function(program) { silent: !!opts.silent, watch: !!opts.watch, repl: !!opts.repl, + runExamples: !!opts.runExamples, // We want to run without base path when the `--run-examples` flag is given so that we can use local // links in other documentation sources, like "View this tutorial [here](http://localhost:5601/app/tutorial/xyz)". // We can tell users they only have to run with `yarn start --run-examples` to get those diff --git a/src/core/server/config/__mocks__/env.ts b/src/core/server/config/__mocks__/env.ts index 644b499ff56d8..80cfab81fb557 100644 --- a/src/core/server/config/__mocks__/env.ts +++ b/src/core/server/config/__mocks__/env.ts @@ -38,6 +38,7 @@ export function getEnvOptions(options: DeepPartial = {}): EnvOptions basePath: false, optimize: false, oss: false, + runExamples: false, ...(options.cliArgs || {}), }, isDevClusterMaster: diff --git a/src/core/server/config/__snapshots__/env.test.ts.snap b/src/core/server/config/__snapshots__/env.test.ts.snap index 1f4661283de6e..204b8a70aa877 100644 --- a/src/core/server/config/__snapshots__/env.test.ts.snap +++ b/src/core/server/config/__snapshots__/env.test.ts.snap @@ -12,6 +12,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -40,7 +41,6 @@ Env { "/test/kibanaRoot/plugins", "/test/kibanaRoot/../kibana-extra", ], - "staticFilesDir": "/test/kibanaRoot/ui", } `; @@ -56,6 +56,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -84,7 +85,6 @@ Env { "/test/kibanaRoot/plugins", "/test/kibanaRoot/../kibana-extra", ], - "staticFilesDir": "/test/kibanaRoot/ui", } `; @@ -99,6 +99,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -127,7 +128,6 @@ Env { "/test/kibanaRoot/plugins", "/test/kibanaRoot/../kibana-extra", ], - "staticFilesDir": "/test/kibanaRoot/ui", } `; @@ -142,6 +142,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -170,7 +171,6 @@ Env { "/test/kibanaRoot/plugins", "/test/kibanaRoot/../kibana-extra", ], - "staticFilesDir": "/test/kibanaRoot/ui", } `; @@ -185,6 +185,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -213,7 +214,6 @@ Env { "/test/kibanaRoot/plugins", "/test/kibanaRoot/../kibana-extra", ], - "staticFilesDir": "/test/kibanaRoot/ui", } `; @@ -228,6 +228,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -256,6 +257,5 @@ Env { "/some/home/dir/plugins", "/some/home/dir/../kibana-extra", ], - "staticFilesDir": "/some/home/dir/ui", } `; diff --git a/src/core/server/config/env.test.ts b/src/core/server/config/env.test.ts index 5812fa93cf18f..c244012e34469 100644 --- a/src/core/server/config/env.test.ts +++ b/src/core/server/config/env.test.ts @@ -152,3 +152,25 @@ test('pluginSearchPaths does not contains x-pack plugins path if --oss flag is t expect(env.pluginSearchPaths).not.toContain('/some/home/dir/x-pack/plugins'); }); + +test('pluginSearchPaths contains examples plugins path if --run-examples flag is true', () => { + const env = new Env( + '/some/home/dir', + getEnvOptions({ + cliArgs: { runExamples: true }, + }) + ); + + expect(env.pluginSearchPaths).toContain('/some/home/dir/examples'); +}); + +test('pluginSearchPaths does not contains examples plugins path if --run-examples flag is false', () => { + const env = new Env( + '/some/home/dir', + getEnvOptions({ + cliArgs: { runExamples: false }, + }) + ); + + expect(env.pluginSearchPaths).not.toContain('/some/home/dir/examples'); +}); diff --git a/src/core/server/config/env.ts b/src/core/server/config/env.ts index 460773d89db85..db363fcd4d751 100644 --- a/src/core/server/config/env.ts +++ b/src/core/server/config/env.ts @@ -43,6 +43,7 @@ export interface CliArgs { optimize: boolean; open: boolean; oss: boolean; + runExamples: boolean; } export class Env { @@ -61,8 +62,6 @@ export class Env { /** @internal */ public readonly logDir: string; /** @internal */ - public readonly staticFilesDir: string; - /** @internal */ public readonly pluginSearchPaths: readonly string[]; /** @@ -100,14 +99,14 @@ export class Env { this.configDir = resolve(this.homeDir, 'config'); this.binDir = resolve(this.homeDir, 'bin'); this.logDir = resolve(this.homeDir, 'log'); - this.staticFilesDir = resolve(this.homeDir, 'ui'); this.pluginSearchPaths = [ resolve(this.homeDir, 'src', 'plugins'), - options.cliArgs.oss ? '' : resolve(this.homeDir, 'x-pack', 'plugins'), + ...(options.cliArgs.oss ? [] : [resolve(this.homeDir, 'x-pack', 'plugins')]), resolve(this.homeDir, 'plugins'), + ...(options.cliArgs.runExamples ? [resolve(this.homeDir, 'examples')] : []), resolve(this.homeDir, '..', 'kibana-extra'), - ].filter(Boolean); + ]; this.cliArgs = Object.freeze(options.cliArgs); this.configs = Object.freeze(options.configs); diff --git a/src/core/server/http/http_config.test.ts b/src/core/server/http/http_config.test.ts index 9b6fab8f3daec..082b85ad68add 100644 --- a/src/core/server/http/http_config.test.ts +++ b/src/core/server/http/http_config.test.ts @@ -19,8 +19,6 @@ import uuid from 'uuid'; import { config, HttpConfig } from '.'; -import { Env } from '../config'; -import { getEnvOptions } from '../config/__mocks__/env'; const validHostnames = ['www.example.com', '8.8.8.8', '::1', 'localhost']; const invalidHostname = 'asdf$%^'; @@ -265,8 +263,7 @@ describe('with TLS', () => { clientAuthentication: 'none', }, }), - {} as any, - Env.createDefault(getEnvOptions()) + {} as any ); expect(httpConfig.ssl.requestCert).toBe(false); @@ -283,8 +280,7 @@ describe('with TLS', () => { clientAuthentication: 'optional', }, }), - {} as any, - Env.createDefault(getEnvOptions()) + {} as any ); expect(httpConfig.ssl.requestCert).toBe(true); @@ -301,8 +297,7 @@ describe('with TLS', () => { clientAuthentication: 'required', }, }), - {} as any, - Env.createDefault(getEnvOptions()) + {} as any ); expect(httpConfig.ssl.requestCert).toBe(true); diff --git a/src/core/server/http/http_config.ts b/src/core/server/http/http_config.ts index da90be2c8dd41..92a8d6a95b854 100644 --- a/src/core/server/http/http_config.ts +++ b/src/core/server/http/http_config.ts @@ -18,7 +18,6 @@ */ import { ByteSizeValue, schema, TypeOf } from '@kbn/config-schema'; -import { Env } from '../config'; import { CspConfigType, CspConfig, ICspConfig } from '../csp'; import { SslConfig, sslSchema } from './ssl_config'; @@ -126,7 +125,6 @@ export class HttpConfig { public maxPayload: ByteSizeValue; public basePath?: string; public rewriteBasePath: boolean; - public publicDir: string; public ssl: SslConfig; public compression: { enabled: boolean; referrerWhitelist?: string[] }; public csp: ICspConfig; @@ -134,7 +132,7 @@ export class HttpConfig { /** * @internal */ - constructor(rawHttpConfig: HttpConfigType, rawCspConfig: CspConfigType, env: Env) { + constructor(rawHttpConfig: HttpConfigType, rawCspConfig: CspConfigType) { this.autoListen = rawHttpConfig.autoListen; this.host = rawHttpConfig.host; this.port = rawHttpConfig.port; @@ -144,7 +142,6 @@ export class HttpConfig { this.keepaliveTimeout = rawHttpConfig.keepaliveTimeout; this.socketTimeout = rawHttpConfig.socketTimeout; this.rewriteBasePath = rawHttpConfig.rewriteBasePath; - this.publicDir = env.staticFilesDir; this.ssl = new SslConfig(rawHttpConfig.ssl || {}); this.compression = rawHttpConfig.compression; this.csp = new CspConfig(rawCspConfig); diff --git a/src/core/server/http/http_service.ts b/src/core/server/http/http_service.ts index d707938aab149..09982cf164a19 100644 --- a/src/core/server/http/http_service.ts +++ b/src/core/server/http/http_service.ts @@ -61,14 +61,14 @@ export class HttpService implements CoreService(httpConfig.path), configService.atPath(cspConfig.path) - ).pipe(map(([http, csp]) => new HttpConfig(http, csp, env))); + ).pipe(map(([http, csp]) => new HttpConfig(http, csp))); this.httpServer = new HttpServer(logger, 'Kibana'); this.httpsRedirectServer = new HttpsRedirectServer(logger.get('http', 'redirect', 'server')); } diff --git a/src/core/server/http/http_tools.test.ts b/src/core/server/http/http_tools.test.ts index b889ebd64971f..c1322a5aa94db 100644 --- a/src/core/server/http/http_tools.test.ts +++ b/src/core/server/http/http_tools.test.ts @@ -31,8 +31,6 @@ import { HttpConfig, config } from './http_config'; import { Router } from './router'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { ByteSizeValue } from '@kbn/config-schema'; -import { Env } from '../config'; -import { getEnvOptions } from '../config/__mocks__/env'; const emptyOutput = { statusCode: 400, @@ -122,8 +120,7 @@ describe('getServerOptions', () => { certificate: 'some-certificate-path', }, }), - {} as any, - Env.createDefault(getEnvOptions()) + {} as any ); expect(getServerOptions(httpConfig).tls).toMatchInlineSnapshot(` @@ -152,8 +149,7 @@ describe('getServerOptions', () => { clientAuthentication: 'required', }, }), - {} as any, - Env.createDefault(getEnvOptions()) + {} as any ); expect(getServerOptions(httpConfig).tls).toMatchInlineSnapshot(` diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index 2ed87f4c6d488..e17de7364ce59 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -84,7 +84,7 @@ export class LegacyService implements CoreService { private settings?: LegacyVars; constructor(private readonly coreContext: CoreContext) { - const { logger, configService, env } = coreContext; + const { logger, configService } = coreContext; this.log = logger.get('legacy-service'); this.devConfig$ = configService @@ -93,7 +93,7 @@ export class LegacyService implements CoreService { this.httpConfig$ = combineLatest( configService.atPath(httpConfig.path), configService.atPath(cspConfig.path) - ).pipe(map(([http, csp]) => new HttpConfig(http, csp, env))); + ).pipe(map(([http, csp]) => new HttpConfig(http, csp))); } public async discoverPlugins(): Promise { diff --git a/src/core/server/plugins/discovery/plugins_discovery.test.ts b/src/core/server/plugins/discovery/plugins_discovery.test.ts index bf55fc7caae4c..2902aafdbf146 100644 --- a/src/core/server/plugins/discovery/plugins_discovery.test.ts +++ b/src/core/server/plugins/discovery/plugins_discovery.test.ts @@ -18,13 +18,14 @@ */ import { mockPackage, mockReaddir, mockReadFile, mockStat } from './plugins_discovery.test.mocks'; +import { rawConfigServiceMock } from '../../config/raw_config_service.mock'; +import { loggingServiceMock } from '../../logging/logging_service.mock'; import { resolve } from 'path'; import { first, map, toArray } from 'rxjs/operators'; + import { ConfigService, Env } from '../../config'; -import { rawConfigServiceMock } from '../../config/raw_config_service.mock'; import { getEnvOptions } from '../../config/__mocks__/env'; -import { loggingServiceMock } from '../../logging/logging_service.mock'; import { PluginWrapper } from '../plugin'; import { PluginsConfig, PluginsConfigType, config } from '../plugins_config'; import { discover } from './plugins_discovery'; @@ -37,6 +38,7 @@ const TEST_PLUGIN_SEARCH_PATHS = { const TEST_EXTRA_PLUGIN_PATH = resolve(process.cwd(), 'my-extra-plugin'); const logger = loggingServiceMock.create(); + beforeEach(() => { mockReaddir.mockImplementation((path, cb) => { if (path === TEST_PLUGIN_SEARCH_PATHS.nonEmptySrcPlugins) { @@ -182,12 +184,84 @@ test('properly iterates through plugin search locations', async () => { 'kibana.json' )})`, ]); +}); + +test('logs a warning about --plugin-path when used in development', async () => { + mockPackage.raw = { + branch: 'master', + version: '1.2.3', + build: { + distributable: true, + number: 1, + sha: '', + }, + }; + + const env = Env.createDefault( + getEnvOptions({ + cliArgs: { dev: false, envName: 'development' }, + }) + ); + const configService = new ConfigService( + rawConfigServiceMock.create({ rawConfig: { plugins: { paths: [TEST_EXTRA_PLUGIN_PATH] } } }), + env, + logger + ); + await configService.setSchema(config.path, config.schema); + + const rawConfig = await configService + .atPath('plugins') + .pipe(first()) + .toPromise(); + + discover(new PluginsConfig(rawConfig, env), { + coreId: Symbol(), + configService, + env, + logger, + }); + + expect(loggingServiceMock.collect(logger).warn).toEqual([ + [ + `Explicit plugin paths [${TEST_EXTRA_PLUGIN_PATH}] should only be used in development. Relative imports may not work properly in production.`, + ], + ]); +}); + +test('does not log a warning about --plugin-path when used in production', async () => { + mockPackage.raw = { + branch: 'master', + version: '1.2.3', + build: { + distributable: true, + number: 1, + sha: '', + }, + }; + + const env = Env.createDefault( + getEnvOptions({ + cliArgs: { dev: false, envName: 'production' }, + }) + ); + const configService = new ConfigService( + rawConfigServiceMock.create({ rawConfig: { plugins: { paths: [TEST_EXTRA_PLUGIN_PATH] } } }), + env, + logger + ); + await configService.setSchema(config.path, config.schema); + + const rawConfig = await configService + .atPath('plugins') + .pipe(first()) + .toPromise(); + + discover(new PluginsConfig(rawConfig, env), { + coreId: Symbol(), + configService, + env, + logger, + }); - expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(` -Array [ - Array [ - "Explicit plugin paths [${TEST_EXTRA_PLUGIN_PATH}] are only supported in development. Relative imports will not work in production.", - ], -] -`); + expect(loggingServiceMock.collect(logger).warn).toEqual([]); }); diff --git a/src/core/server/plugins/discovery/plugins_discovery.ts b/src/core/server/plugins/discovery/plugins_discovery.ts index 521d02e487df6..79238afdf5c81 100644 --- a/src/core/server/plugins/discovery/plugins_discovery.ts +++ b/src/core/server/plugins/discovery/plugins_discovery.ts @@ -46,9 +46,9 @@ export function discover(config: PluginsConfig, coreContext: CoreContext) { const log = coreContext.logger.get('plugins-discovery'); log.debug('Discovering plugins...'); - if (config.additionalPluginPaths.length) { + if (config.additionalPluginPaths.length && coreContext.env.mode.dev) { log.warn( - `Explicit plugin paths [${config.additionalPluginPaths}] are only supported in development. Relative imports will not work in production.` + `Explicit plugin paths [${config.additionalPluginPaths}] should only be used in development. Relative imports may not work properly in production.` ); } diff --git a/src/core/server/plugins/plugins_config.test.ts b/src/core/server/plugins/plugins_config.test.ts new file mode 100644 index 0000000000000..180d6093e0404 --- /dev/null +++ b/src/core/server/plugins/plugins_config.test.ts @@ -0,0 +1,44 @@ +/* + * 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 { PluginsConfig, PluginsConfigType } from './plugins_config'; +import { Env } from '../config'; +import { getEnvOptions } from '../config/__mocks__/env'; + +describe('PluginsConfig', () => { + it('retrieves additionalPluginPaths from config.paths when in production mode', () => { + const env = Env.createDefault(getEnvOptions({ cliArgs: { dev: false } })); + const rawConfig: PluginsConfigType = { + initialize: true, + paths: ['some-path', 'another-path'], + }; + const config = new PluginsConfig(rawConfig, env); + expect(config.additionalPluginPaths).toEqual(['some-path', 'another-path']); + }); + + it('retrieves additionalPluginPaths from config.paths when in development mode', () => { + const env = Env.createDefault(getEnvOptions({ cliArgs: { dev: true } })); + const rawConfig: PluginsConfigType = { + initialize: true, + paths: ['some-path', 'another-path'], + }; + const config = new PluginsConfig(rawConfig, env); + expect(config.additionalPluginPaths).toEqual(['some-path', 'another-path']); + }); +}); diff --git a/src/core/server/plugins/plugins_config.ts b/src/core/server/plugins/plugins_config.ts index 93ae229737fae..8ebcc92672dde 100644 --- a/src/core/server/plugins/plugins_config.ts +++ b/src/core/server/plugins/plugins_config.ts @@ -29,7 +29,6 @@ export const config = { /** * Defines an array of directories where another plugin should be loaded from. - * Should only be used in a development environment. */ paths: schema.arrayOf(schema.string(), { defaultValue: [] }), }), @@ -55,7 +54,6 @@ export class PluginsConfig { constructor(rawConfig: PluginsConfigType, env: Env) { this.initialize = rawConfig.initialize; this.pluginSearchPaths = env.pluginSearchPaths; - // Only allow custom plugin paths in dev. - this.additionalPluginPaths = env.mode.dev ? rawConfig.paths : []; + this.additionalPluginPaths = rawConfig.paths; } } diff --git a/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap b/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap index edde1dee85f4f..5e6e977663bc4 100644 --- a/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap +++ b/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap @@ -18,6 +18,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -39,7 +40,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/mock-server-basepath/translations/en.json", @@ -89,6 +89,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -110,7 +111,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/mock-server-basepath/translations/en.json", @@ -160,6 +160,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -181,7 +182,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/mock-server-basepath/translations/en.json", @@ -235,6 +235,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -256,7 +257,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/translations/en.json", @@ -306,6 +306,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -327,7 +328,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/mock-server-basepath/translations/en.json", @@ -377,6 +377,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -398,7 +399,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/mock-server-basepath/translations/en.json", @@ -448,6 +448,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -469,7 +470,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/translations/en.json", @@ -519,6 +519,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -540,7 +541,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/mock-server-basepath/translations/en.json", @@ -592,6 +592,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -613,7 +614,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/mock-server-basepath/translations/en.json", @@ -663,6 +663,7 @@ Object { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -684,7 +685,6 @@ Object { "version": Any, }, "pluginSearchPaths": Any, - "staticFilesDir": Any, }, "i18n": Object { "translationsUrl": "/mock-server-basepath/translations/en.json", diff --git a/src/core/server/rendering/rendering_service.test.ts b/src/core/server/rendering/rendering_service.test.ts index 63145f2b30573..43ff4f633085c 100644 --- a/src/core/server/rendering/rendering_service.test.ts +++ b/src/core/server/rendering/rendering_service.test.ts @@ -41,7 +41,6 @@ const INJECTED_METADATA = { version: expect.any(String), }, pluginSearchPaths: expect.any(Array), - staticFilesDir: expect.any(String), }, legacyMetadata: { branch: expect.any(String), @@ -50,6 +49,7 @@ const INJECTED_METADATA = { version: expect.any(String), }, }; + const { createKibanaRequest, createRawRequest } = httpServerMock; const legacyApp = { getId: () => 'legacy' }; diff --git a/src/test_utils/kbn_server.ts b/src/test_utils/kbn_server.ts index 40700e05bcde8..43c6b4378ed27 100644 --- a/src/test_utils/kbn_server.ts +++ b/src/test_utils/kbn_server.ts @@ -76,6 +76,7 @@ export function createRootWithSettings( repl: false, basePath: false, optimize: false, + runExamples: false, oss: true, ...cliArgs, }, diff --git a/x-pack/legacy/plugins/beats_management/public/lib/adapters/framework/kibana_framework_adapter.ts b/x-pack/legacy/plugins/beats_management/public/lib/adapters/framework/kibana_framework_adapter.ts index 79ffe58d419bd..b2cfd826e6207 100644 --- a/x-pack/legacy/plugins/beats_management/public/lib/adapters/framework/kibana_framework_adapter.ts +++ b/x-pack/legacy/plugins/beats_management/public/lib/adapters/framework/kibana_framework_adapter.ts @@ -11,6 +11,8 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; import { UIRoutes } from 'ui/routes'; import { isLeft } from 'fp-ts/lib/Either'; +import { npSetup } from 'ui/new_platform'; +import { SecurityPluginSetup } from '../../../../../../../plugins/security/public'; import { BufferedKibanaServiceCall, KibanaAdapterServiceRefs, KibanaUIConfig } from '../../types'; import { FrameworkAdapter, @@ -58,7 +60,7 @@ export class KibanaFrameworkAdapter implements FrameworkAdapter { }; public async waitUntilFrameworkReady(): Promise { - const $injector = await this.onKibanaReady(); + await this.onKibanaReady(); const xpackInfo: any = this.xpackInfoService; let xpackInfoUnpacked: FrameworkInfo; @@ -95,8 +97,10 @@ export class KibanaFrameworkAdapter implements FrameworkAdapter { } this.xpackInfo = xpackInfoUnpacked; + const securitySetup = ((npSetup.plugins as unknown) as { security?: SecurityPluginSetup }) + .security; try { - this.shieldUser = await $injector.get('ShieldUser').getCurrent().$promise; + this.shieldUser = (await securitySetup?.authc.getCurrentUser()) || null; const assertUser = RuntimeFrameworkUser.decode(this.shieldUser); if (isLeft(assertUser)) { diff --git a/x-pack/legacy/plugins/logstash/public/sections/pipeline_edit/components/pipeline_edit/pipeline_edit.js b/x-pack/legacy/plugins/logstash/public/sections/pipeline_edit/components/pipeline_edit/pipeline_edit.js index aa7f88a62397c..83446278fdeca 100755 --- a/x-pack/legacy/plugins/logstash/public/sections/pipeline_edit/components/pipeline_edit/pipeline_edit.js +++ b/x-pack/legacy/plugins/logstash/public/sections/pipeline_edit/components/pipeline_edit/pipeline_edit.js @@ -8,6 +8,7 @@ import React from 'react'; import { render } from 'react-dom'; import { isEmpty } from 'lodash'; import { uiModules } from 'ui/modules'; +import { npSetup } from 'ui/new_platform'; import { toastNotifications } from 'ui/notify'; import { I18nContext } from 'ui/i18n'; import { PipelineEditor } from '../../../../components/pipeline_editor'; @@ -21,7 +22,6 @@ app.directive('pipelineEdit', function($injector) { const pipelineService = $injector.get('pipelineService'); const licenseService = $injector.get('logstashLicenseService'); const kbnUrl = $injector.get('kbnUrl'); - const shieldUser = $injector.get('ShieldUser'); const $route = $injector.get('$route'); return { @@ -32,7 +32,7 @@ app.directive('pipelineEdit', function($injector) { scope.$evalAsync(kbnUrl.change(`/management/logstash/pipelines/${id}/edit`)); const userResource = logstashSecurity.isSecurityEnabled() - ? await shieldUser.getCurrent().$promise + ? await npSetup.plugins.security.authc.getCurrentUser() : null; render( diff --git a/x-pack/legacy/plugins/security/index.js b/x-pack/legacy/plugins/security/index.js index 6ee8b5f8b2b10..bc403b803b8df 100644 --- a/x-pack/legacy/plugins/security/index.js +++ b/x-pack/legacy/plugins/security/index.js @@ -28,17 +28,10 @@ export const security = kibana => enabled: Joi.boolean().default(true), cookieName: HANDLED_IN_NEW_PLATFORM, encryptionKey: HANDLED_IN_NEW_PLATFORM, - session: Joi.object({ - idleTimeout: HANDLED_IN_NEW_PLATFORM, - lifespan: HANDLED_IN_NEW_PLATFORM, - }).default(), + session: HANDLED_IN_NEW_PLATFORM, secureCookies: HANDLED_IN_NEW_PLATFORM, loginAssistanceMessage: HANDLED_IN_NEW_PLATFORM, - authorization: Joi.object({ - legacyFallback: Joi.object({ - enabled: Joi.boolean().default(true), // deprecated - }).default(), - }).default(), + authorization: HANDLED_IN_NEW_PLATFORM, audit: Joi.object({ enabled: Joi.boolean().default(false), }).default(), @@ -46,13 +39,6 @@ export const security = kibana => }).default(); }, - deprecations: function({ rename, unused }) { - return [ - unused('authorization.legacyFallback.enabled'), - rename('sessionTimeout', 'session.idleTimeout'), - ]; - }, - uiExports: { chromeNavControls: [], managementSections: ['plugins/security/views/management'], diff --git a/x-pack/legacy/plugins/security/public/lib/api.ts b/x-pack/legacy/plugins/security/public/lib/api.ts index ffa08ca44f376..c5c6994bf4be3 100644 --- a/x-pack/legacy/plugins/security/public/lib/api.ts +++ b/x-pack/legacy/plugins/security/public/lib/api.ts @@ -5,16 +5,12 @@ */ import { kfetch } from 'ui/kfetch'; -import { AuthenticatedUser, Role, User, EditUser } from '../../common/model'; +import { Role, User, EditUser } from '../../common/model'; const usersUrl = '/internal/security/users'; const rolesUrl = '/api/security/role'; export class UserAPIClient { - public async getCurrentUser(): Promise { - return await kfetch({ pathname: `/internal/security/me` }); - } - public async getUsers(): Promise { return await kfetch({ pathname: usersUrl }); } diff --git a/x-pack/legacy/plugins/security/public/services/shield_user.js b/x-pack/legacy/plugins/security/public/services/shield_user.js deleted file mode 100644 index 14a79f267ca75..0000000000000 --- a/x-pack/legacy/plugins/security/public/services/shield_user.js +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import 'angular-resource'; -import angular from 'angular'; -import { uiModules } from 'ui/modules'; - -const module = uiModules.get('security', ['ngResource']); -module.service('ShieldUser', ($resource, chrome) => { - const baseUrl = chrome.addBasePath('/internal/security/users/:username'); - const ShieldUser = $resource( - baseUrl, - { - username: '@username', - }, - { - changePassword: { - method: 'POST', - url: `${baseUrl}/password`, - transformRequest: ({ password, newPassword }) => angular.toJson({ password, newPassword }), - }, - getCurrent: { - method: 'GET', - url: chrome.addBasePath('/internal/security/me'), - }, - } - ); - - return ShieldUser; -}); diff --git a/x-pack/legacy/plugins/security/public/views/account/account.js b/x-pack/legacy/plugins/security/public/views/account/account.js index db971bd97eab7..70a7b8dce727e 100644 --- a/x-pack/legacy/plugins/security/public/views/account/account.js +++ b/x-pack/legacy/plugins/security/public/views/account/account.js @@ -6,22 +6,13 @@ import routes from 'ui/routes'; import template from './account.html'; -import '../../services/shield_user'; import { i18n } from '@kbn/i18n'; import { I18nContext } from 'ui/i18n'; +import { npSetup } from 'ui/new_platform'; import { AccountManagementPage } from './components'; import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; -const renderReact = (elem, user) => { - render( - - - , - elem - ); -}; - routes.when('/account', { template, k7Breadcrumbs: () => [ @@ -31,13 +22,8 @@ routes.when('/account', { }), }, ], - resolve: { - user(ShieldUser) { - return ShieldUser.getCurrent().$promise; - }, - }, controllerAs: 'accountController', - controller($scope, $route) { + controller($scope) { $scope.$on('$destroy', () => { const elem = document.getElementById('userProfileReactRoot'); if (elem) { @@ -45,8 +31,12 @@ routes.when('/account', { } }); $scope.$$postDigest(() => { - const elem = document.getElementById('userProfileReactRoot'); - renderReact(elem, $route.current.locals.user); + render( + + + , + document.getElementById('userProfileReactRoot') + ); }); }, }); diff --git a/x-pack/legacy/plugins/security/public/views/account/components/account_management_page.test.tsx b/x-pack/legacy/plugins/security/public/views/account/components/account_management_page.test.tsx index 176b05f455439..366842e58e9e4 100644 --- a/x-pack/legacy/plugins/security/public/views/account/components/account_management_page.test.tsx +++ b/x-pack/legacy/plugins/security/public/views/account/components/account_management_page.test.tsx @@ -4,8 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ import React from 'react'; -import { mountWithIntl } from 'test_utils/enzyme_helpers'; +import { act } from '@testing-library/react'; +import { mountWithIntl, nextTick } from 'test_utils/enzyme_helpers'; +import { securityMock } from '../../../../../../../plugins/security/public/mocks'; import { AccountManagementPage } from './account_management_page'; +import { AuthenticatedUser } from '../../../../common/model'; jest.mock('ui/kfetch'); @@ -32,10 +35,24 @@ const createUser = ({ withFullName = true, withEmail = true, realm = 'native' }: }; }; +function getSecuritySetupMock({ currentUser }: { currentUser: AuthenticatedUser }) { + const securitySetupMock = securityMock.createSetup(); + securitySetupMock.authc.getCurrentUser.mockResolvedValue(currentUser); + return securitySetupMock; +} + describe('', () => { - it(`displays users full name, username, and email address`, () => { + it(`displays users full name, username, and email address`, async () => { const user = createUser(); - const wrapper = mountWithIntl(); + const wrapper = mountWithIntl( + + ); + + await act(async () => { + await nextTick(); + wrapper.update(); + }); + expect(wrapper.find('EuiText[data-test-subj="userDisplayName"]').text()).toEqual( user.full_name ); @@ -43,28 +60,60 @@ describe('', () => { expect(wrapper.find('[data-test-subj="email"]').text()).toEqual(user.email); }); - it(`displays username when full_name is not provided`, () => { + it(`displays username when full_name is not provided`, async () => { const user = createUser({ withFullName: false }); - const wrapper = mountWithIntl(); + const wrapper = mountWithIntl( + + ); + + await act(async () => { + await nextTick(); + wrapper.update(); + }); + expect(wrapper.find('EuiText[data-test-subj="userDisplayName"]').text()).toEqual(user.username); }); - it(`displays a placeholder when no email address is provided`, () => { + it(`displays a placeholder when no email address is provided`, async () => { const user = createUser({ withEmail: false }); - const wrapper = mountWithIntl(); + const wrapper = mountWithIntl( + + ); + + await act(async () => { + await nextTick(); + wrapper.update(); + }); + expect(wrapper.find('[data-test-subj="email"]').text()).toEqual('no email address'); }); - it(`displays change password form for users in the native realm`, () => { + it(`displays change password form for users in the native realm`, async () => { const user = createUser(); - const wrapper = mountWithIntl(); + const wrapper = mountWithIntl( + + ); + + await act(async () => { + await nextTick(); + wrapper.update(); + }); + expect(wrapper.find('EuiFieldText[data-test-subj="currentPassword"]')).toHaveLength(1); expect(wrapper.find('EuiFieldText[data-test-subj="newPassword"]')).toHaveLength(1); }); - it(`does not display change password form for users in the saml realm`, () => { + it(`does not display change password form for users in the saml realm`, async () => { const user = createUser({ realm: 'saml' }); - const wrapper = mountWithIntl(); + const wrapper = mountWithIntl( + + ); + + await act(async () => { + await nextTick(); + wrapper.update(); + }); + expect(wrapper.find('EuiFieldText[data-test-subj="currentPassword"]')).toHaveLength(0); expect(wrapper.find('EuiFieldText[data-test-subj="newPassword"]')).toHaveLength(0); }); diff --git a/x-pack/legacy/plugins/security/public/views/account/components/account_management_page.tsx b/x-pack/legacy/plugins/security/public/views/account/components/account_management_page.tsx index 2ed057ad73a12..6abee73e0b353 100644 --- a/x-pack/legacy/plugins/security/public/views/account/components/account_management_page.tsx +++ b/x-pack/legacy/plugins/security/public/views/account/components/account_management_page.tsx @@ -4,29 +4,41 @@ * you may not use this file except in compliance with the Elastic License. */ import { EuiPage, EuiPageBody, EuiPanel, EuiSpacer, EuiText } from '@elastic/eui'; -import React from 'react'; +import React, { useEffect, useState } from 'react'; +import { SecurityPluginSetup } from '../../../../../../../plugins/security/public'; import { getUserDisplayName, AuthenticatedUser } from '../../../../common/model'; import { ChangePassword } from './change_password'; import { PersonalInfo } from './personal_info'; interface Props { - user: AuthenticatedUser; + securitySetup: SecurityPluginSetup; } -export const AccountManagementPage: React.FC = props => ( - - - - -

{getUserDisplayName(props.user)}

- +export const AccountManagementPage = (props: Props) => { + const [currentUser, setCurrentUser] = useState(null); + useEffect(() => { + props.securitySetup.authc.getCurrentUser().then(setCurrentUser); + }, [props]); - + if (!currentUser) { + return null; + } - + return ( + + + + +

{getUserDisplayName(currentUser)}

+ - - - - -); + + + + + + + + + ); +}; diff --git a/x-pack/legacy/plugins/security/public/views/management/edit_role/index.js b/x-pack/legacy/plugins/security/public/views/management/edit_role/index.js index 09c612526918f..27c9beb4ba828 100644 --- a/x-pack/legacy/plugins/security/public/views/management/edit_role/index.js +++ b/x-pack/legacy/plugins/security/public/views/management/edit_role/index.js @@ -11,10 +11,10 @@ import { kfetch } from 'ui/kfetch'; import { fatalError, toastNotifications } from 'ui/notify'; import { npStart } from 'ui/new_platform'; import template from 'plugins/security/views/management/edit_role/edit_role.html'; -import 'plugins/security/services/shield_user'; import 'plugins/security/services/shield_role'; import 'plugins/security/services/shield_indices'; import { xpackInfo } from 'plugins/xpack_main/services/xpack_info'; +import { UserAPIClient } from '../../../lib/api'; import { ROLES_PATH, CLONE_ROLES_PATH, EDIT_ROLES_PATH } from '../management_urls'; import { getEditRoleBreadcrumbs, getCreateRoleBreadcrumbs } from '../breadcrumbs'; @@ -69,9 +69,8 @@ const routeDefinition = action => ({ return role.then(res => res.toJSON()); }, - users(ShieldUser) { - // $promise is used here because the result is an ngResource, not a promise itself - return ShieldUser.query().$promise.then(users => _.map(users, 'username')); + users() { + return new UserAPIClient().getUsers().then(users => _.map(users, 'username')); }, indexPatterns() { return npStart.plugins.data.indexPatterns.getTitles(); diff --git a/x-pack/legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.test.tsx b/x-pack/legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.test.tsx index 5c71d0da3954a..639646ce48e22 100644 --- a/x-pack/legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.test.tsx +++ b/x-pack/legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.test.tsx @@ -4,38 +4,42 @@ * you may not use this file except in compliance with the Elastic License. */ -import { mountWithIntl } from 'test_utils/enzyme_helpers'; +import { act } from '@testing-library/react'; +import { mountWithIntl, nextTick } from 'test_utils/enzyme_helpers'; import { EditUserPage } from './edit_user_page'; import React from 'react'; +import { securityMock } from '../../../../../../../../plugins/security/public/mocks'; import { UserAPIClient } from '../../../../lib/api'; import { User, Role } from '../../../../../common/model'; import { ReactWrapper } from 'enzyme'; +import { mockAuthenticatedUser } from '../../../../../../../../plugins/security/common/model/authenticated_user.mock'; jest.mock('ui/kfetch'); -const buildClient = () => { - const apiClient = new UserAPIClient(); +const createUser = (username: string) => { + const user: User = { + username, + full_name: 'my full name', + email: 'foo@bar.com', + roles: ['idk', 'something'], + enabled: true, + }; - const createUser = (username: string) => { - const user: User = { - username, - full_name: 'my full name', - email: 'foo@bar.com', - roles: ['idk', 'something'], - enabled: true, + if (username === 'reserved_user') { + user.metadata = { + _reserved: true, }; + } - if (username === 'reserved_user') { - user.metadata = { - _reserved: true, - }; - } + return user; +}; - return Promise.resolve(user); - }; +const buildClient = () => { + const apiClient = new UserAPIClient(); - apiClient.getUser = jest.fn().mockImplementation(createUser); - apiClient.getCurrentUser = jest.fn().mockImplementation(() => createUser('current_user')); + apiClient.getUser = jest + .fn() + .mockImplementation(async (username: string) => createUser(username)); apiClient.getRoles = jest.fn().mockImplementation(() => { return Promise.resolve([ @@ -63,6 +67,14 @@ const buildClient = () => { return apiClient; }; +function buildSecuritySetup() { + const securitySetupMock = securityMock.createSetup(); + securitySetupMock.authc.getCurrentUser.mockResolvedValue( + mockAuthenticatedUser(createUser('current_user')) + ); + return securitySetupMock; +} + function expectSaveButton(wrapper: ReactWrapper) { expect(wrapper.find('EuiButton[data-test-subj="userFormSaveButton"]')).toHaveLength(1); } @@ -74,10 +86,12 @@ function expectMissingSaveButton(wrapper: ReactWrapper) { describe('EditUserPage', () => { it('allows reserved users to be viewed', async () => { const apiClient = buildClient(); + const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( path} intl={null as any} /> @@ -86,17 +100,19 @@ describe('EditUserPage', () => { await waitForRender(wrapper); expect(apiClient.getUser).toBeCalledTimes(1); - expect(apiClient.getCurrentUser).toBeCalledTimes(1); + expect(securitySetup.authc.getCurrentUser).toBeCalledTimes(1); expectMissingSaveButton(wrapper); }); it('allows new users to be created', async () => { const apiClient = buildClient(); + const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( path} intl={null as any} /> @@ -105,17 +121,19 @@ describe('EditUserPage', () => { await waitForRender(wrapper); expect(apiClient.getUser).toBeCalledTimes(0); - expect(apiClient.getCurrentUser).toBeCalledTimes(0); + expect(securitySetup.authc.getCurrentUser).toBeCalledTimes(0); expectSaveButton(wrapper); }); it('allows existing users to be edited', async () => { const apiClient = buildClient(); + const securitySetup = buildSecuritySetup(); const wrapper = mountWithIntl( path} intl={null as any} /> @@ -124,16 +142,15 @@ describe('EditUserPage', () => { await waitForRender(wrapper); expect(apiClient.getUser).toBeCalledTimes(1); - expect(apiClient.getCurrentUser).toBeCalledTimes(1); + expect(securitySetup.authc.getCurrentUser).toBeCalledTimes(1); expectSaveButton(wrapper); }); }); async function waitForRender(wrapper: ReactWrapper) { - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - wrapper.update(); + await act(async () => { + await nextTick(); + wrapper.update(); + }); } diff --git a/x-pack/legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.tsx b/x-pack/legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.tsx index 91f5f048adc6d..bbffe07485f8d 100644 --- a/x-pack/legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.tsx +++ b/x-pack/legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.tsx @@ -28,6 +28,7 @@ import { } from '@elastic/eui'; import { toastNotifications } from 'ui/notify'; import { FormattedMessage, injectI18n, InjectedIntl } from '@kbn/i18n/react'; +import { SecurityPluginSetup } from '../../../../../../../../plugins/security/public'; import { UserValidator, UserValidationResult } from '../../../../lib/validate_user'; import { User, EditUser, Role } from '../../../../../common/model'; import { USERS_PATH } from '../../../../views/management/management_urls'; @@ -40,6 +41,7 @@ interface Props { intl: InjectedIntl; changeUrl: (path: string) => void; apiClient: UserAPIClient; + securitySetup: SecurityPluginSetup; } interface State { @@ -82,7 +84,7 @@ class EditUserPageUI extends Component { } public async componentDidMount() { - const { username, apiClient } = this.props; + const { username, apiClient, securitySetup } = this.props; let { user, currentUser } = this.state; if (username) { try { @@ -91,7 +93,7 @@ class EditUserPageUI extends Component { password: '', confirmPassword: '', }; - currentUser = await apiClient.getCurrentUser(); + currentUser = await securitySetup.authc.getCurrentUser(); } catch (err) { toastNotifications.addDanger({ title: this.props.intl.formatMessage({ diff --git a/x-pack/legacy/plugins/security/public/views/management/edit_user/edit_user.js b/x-pack/legacy/plugins/security/public/views/management/edit_user/edit_user.js index bd9d6f2b1ca35..ab218022c6ee6 100644 --- a/x-pack/legacy/plugins/security/public/views/management/edit_user/edit_user.js +++ b/x-pack/legacy/plugins/security/public/views/management/edit_user/edit_user.js @@ -7,7 +7,6 @@ import routes from 'ui/routes'; import template from 'plugins/security/views/management/edit_user/edit_user.html'; import 'angular-resource'; import 'ui/angular_ui_select'; -import 'plugins/security/services/shield_user'; import 'plugins/security/services/shield_role'; import { EDIT_USERS_PATH } from '../management_urls'; import { EditUserPage } from './components'; @@ -15,12 +14,18 @@ import { UserAPIClient } from '../../../lib/api'; import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; import { I18nContext } from 'ui/i18n'; +import { npSetup } from 'ui/new_platform'; import { getEditUserBreadcrumbs, getCreateUserBreadcrumbs } from '../breadcrumbs'; const renderReact = (elem, changeUrl, username) => { render( - + , elem ); diff --git a/x-pack/legacy/plugins/security/public/views/management/management.js b/x-pack/legacy/plugins/security/public/views/management/management.js index db2175e91c5de..59da63abbb83f 100644 --- a/x-pack/legacy/plugins/security/public/views/management/management.js +++ b/x-pack/legacy/plugins/security/public/views/management/management.js @@ -13,10 +13,10 @@ import 'plugins/security/views/management/edit_user/edit_user'; import 'plugins/security/views/management/edit_role/index'; import routes from 'ui/routes'; import { xpackInfo } from 'plugins/xpack_main/services/xpack_info'; -import '../../services/shield_user'; import { ROLES_PATH, USERS_PATH, API_KEYS_PATH } from './management_urls'; import { management } from 'ui/management'; +import { npSetup } from 'ui/new_platform'; import { i18n } from '@kbn/i18n'; import { toastNotifications } from 'ui/notify'; @@ -36,7 +36,7 @@ routes }) .defaults(/\/management/, { resolve: { - securityManagementSection: function(ShieldUser) { + securityManagementSection: function() { const showSecurityLinks = xpackInfo.get('features.security.showLinks'); function deregisterSecurity() { @@ -93,12 +93,11 @@ routes if (!showSecurityLinks) { deregisterSecurity(); } else { - // getCurrent will reject if there is no authenticated user, so we prevent them from seeing the security - // management screens - // - // $promise is used here because the result is an ngResource, not a promise itself - return ShieldUser.getCurrent() - .$promise.then(ensureSecurityRegistered) + // getCurrentUser will reject if there is no authenticated user, so we prevent them from + // seeing the security management screens. + return npSetup.plugins.security.authc + .getCurrentUser() + .then(ensureSecurityRegistered) .catch(deregisterSecurity); } }, diff --git a/x-pack/legacy/plugins/security/public/views/management/users_grid/users.js b/x-pack/legacy/plugins/security/public/views/management/users_grid/users.js index a7115f449ebfd..8d4e0526251d7 100644 --- a/x-pack/legacy/plugins/security/public/views/management/users_grid/users.js +++ b/x-pack/legacy/plugins/security/public/views/management/users_grid/users.js @@ -8,7 +8,6 @@ import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; import routes from 'ui/routes'; import template from 'plugins/security/views/management/users_grid/users.html'; -import 'plugins/security/services/shield_user'; import { SECURITY_PATH, USERS_PATH } from '../management_urls'; import { UsersListPage } from './components'; import { UserAPIClient } from '../../../lib/api'; diff --git a/x-pack/legacy/plugins/security/public/views/overwritten_session/overwritten_session.tsx b/x-pack/legacy/plugins/security/public/views/overwritten_session/overwritten_session.tsx index 76088443212b2..fb39c517e1c2c 100644 --- a/x-pack/legacy/plugins/security/public/views/overwritten_session/overwritten_session.tsx +++ b/x-pack/legacy/plugins/security/public/views/overwritten_session/overwritten_session.tsx @@ -10,36 +10,40 @@ import React from 'react'; import { render } from 'react-dom'; import chrome from 'ui/chrome'; import { I18nContext } from 'ui/i18n'; +import { npSetup } from 'ui/new_platform'; +import { SecurityPluginSetup } from '../../../../../../plugins/security/public'; import { AuthenticatedUser } from '../../../common/model'; import { AuthenticationStatePage } from '../../components/authentication_state_page'; chrome .setVisible(false) .setRootTemplate('
') - .setRootController('overwritten_session', ($scope: any, ShieldUser: any) => { + .setRootController('overwritten_session', ($scope: any) => { $scope.$$postDigest(() => { - ShieldUser.getCurrent().$promise.then((user: AuthenticatedUser) => { - const overwrittenSessionPage = ( - - - } - > - - - - - - ); - render(overwrittenSessionPage, document.getElementById('reactOverwrittenSessionRoot')); - }); + ((npSetup.plugins as unknown) as { security: SecurityPluginSetup }).security.authc + .getCurrentUser() + .then((user: AuthenticatedUser) => { + const overwrittenSessionPage = ( + + + } + > + + + + + + ); + render(overwrittenSessionPage, document.getElementById('reactOverwrittenSessionRoot')); + }); }); }); diff --git a/x-pack/plugins/security/public/authentication/authentication_service.ts b/x-pack/plugins/security/public/authentication/authentication_service.ts new file mode 100644 index 0000000000000..23c45c88e563a --- /dev/null +++ b/x-pack/plugins/security/public/authentication/authentication_service.ts @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { HttpSetup } from 'src/core/public'; +import { AuthenticatedUser } from '../../common/model'; + +interface SetupParams { + http: HttpSetup; +} + +export interface AuthenticationServiceSetup { + /** + * Returns currently authenticated user and throws if current user isn't authenticated. + */ + getCurrentUser: () => Promise; +} + +export class AuthenticationService { + public setup({ http }: SetupParams): AuthenticationServiceSetup { + return { + async getCurrentUser() { + return (await http.get('/internal/security/me', { + headers: { 'kbn-system-api': true }, + })) as AuthenticatedUser; + }, + }; + } +} diff --git a/x-pack/plugins/security/public/authentication/index.mock.ts b/x-pack/plugins/security/public/authentication/index.mock.ts new file mode 100644 index 0000000000000..c8d77a5b62c6f --- /dev/null +++ b/x-pack/plugins/security/public/authentication/index.mock.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { AuthenticationServiceSetup } from './authentication_service'; + +export const authenticationMock = { + createSetup: (): jest.Mocked => ({ + getCurrentUser: jest.fn(), + }), +}; diff --git a/x-pack/plugins/security/public/authentication/index.ts b/x-pack/plugins/security/public/authentication/index.ts new file mode 100644 index 0000000000000..a55f4d7bb95b3 --- /dev/null +++ b/x-pack/plugins/security/public/authentication/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export { AuthenticationService, AuthenticationServiceSetup } from './authentication_service'; diff --git a/x-pack/plugins/security/public/index.ts b/x-pack/plugins/security/public/index.ts index dc34fcbbe7d1e..336ec37d76a1b 100644 --- a/x-pack/plugins/security/public/index.ts +++ b/x-pack/plugins/security/public/index.ts @@ -6,7 +6,10 @@ import { PluginInitializer } from 'src/core/public'; import { SecurityPlugin, SecurityPluginSetup, SecurityPluginStart } from './plugin'; + +export { SecurityPluginSetup, SecurityPluginStart }; export { SessionInfo } from './types'; +export { AuthenticatedUser } from '../common/model'; export const plugin: PluginInitializer = () => new SecurityPlugin(); diff --git a/x-pack/plugins/security/public/mocks.ts b/x-pack/plugins/security/public/mocks.ts new file mode 100644 index 0000000000000..3c0c59d10abd1 --- /dev/null +++ b/x-pack/plugins/security/public/mocks.ts @@ -0,0 +1,19 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { authenticationMock } from './authentication/index.mock'; +import { createSessionTimeoutMock } from './session/session_timeout.mock'; + +function createSetupMock() { + return { + authc: authenticationMock.createSetup(), + sessionTimeout: createSessionTimeoutMock(), + }; +} + +export const securityMock = { + createSetup: createSetupMock, +}; diff --git a/x-pack/plugins/security/public/nav_control/nav_control_service.test.ts b/x-pack/plugins/security/public/nav_control/nav_control_service.test.ts index 3879d611d46eb..a9a89ee05f561 100644 --- a/x-pack/plugins/security/public/nav_control/nav_control_service.test.ts +++ b/x-pack/plugins/security/public/nav_control/nav_control_service.test.ts @@ -10,6 +10,8 @@ import { ILicense } from '../../../licensing/public'; import { SecurityNavControlService } from '.'; import { SecurityLicenseService } from '../../common/licensing'; import { nextTick } from 'test_utils/enzyme_helpers'; +import { securityMock } from '../mocks'; +import { mockAuthenticatedUser } from '../../common/model/authenticated_user.mock'; const validLicense = { isAvailable: true, @@ -29,13 +31,17 @@ describe('SecurityNavControlService', () => { const license$ = new BehaviorSubject(validLicense); const navControlService = new SecurityNavControlService(); + const mockSecuritySetup = securityMock.createSetup(); + mockSecuritySetup.authc.getCurrentUser.mockResolvedValue( + mockAuthenticatedUser({ username: 'some-user', full_name: undefined }) + ); navControlService.setup({ securityLicense: new SecurityLicenseService().setup({ license$ }).license, + authc: mockSecuritySetup.authc, }); const coreStart = coreMock.createStart(); coreStart.chrome.navControls.registerRight = jest.fn(); - coreStart.http.get.mockResolvedValue({ username: 'some-user' }); navControlService.start({ core: coreStart }); expect(coreStart.chrome.navControls.registerRight).toHaveBeenCalledTimes(1); @@ -93,6 +99,7 @@ describe('SecurityNavControlService', () => { const navControlService = new SecurityNavControlService(); navControlService.setup({ securityLicense: new SecurityLicenseService().setup({ license$ }).license, + authc: securityMock.createSetup().authc, }); const coreStart = coreMock.createStart(); @@ -111,6 +118,7 @@ describe('SecurityNavControlService', () => { const navControlService = new SecurityNavControlService(); navControlService.setup({ securityLicense: new SecurityLicenseService().setup({ license$ }).license, + authc: securityMock.createSetup().authc, }); const coreStart = coreMock.createStart(); @@ -126,6 +134,7 @@ describe('SecurityNavControlService', () => { const navControlService = new SecurityNavControlService(); navControlService.setup({ securityLicense: new SecurityLicenseService().setup({ license$ }).license, + authc: securityMock.createSetup().authc, }); const coreStart = coreMock.createStart(); @@ -146,6 +155,7 @@ describe('SecurityNavControlService', () => { const navControlService = new SecurityNavControlService(); navControlService.setup({ securityLicense: new SecurityLicenseService().setup({ license$ }).license, + authc: securityMock.createSetup().authc, }); const coreStart = coreMock.createStart(); diff --git a/x-pack/plugins/security/public/nav_control/nav_control_service.tsx b/x-pack/plugins/security/public/nav_control/nav_control_service.tsx index aeeb84219c937..153e7112dc95b 100644 --- a/x-pack/plugins/security/public/nav_control/nav_control_service.tsx +++ b/x-pack/plugins/security/public/nav_control/nav_control_service.tsx @@ -9,11 +9,12 @@ import { CoreStart } from 'src/core/public'; import ReactDOM from 'react-dom'; import React from 'react'; import { SecurityLicense } from '../../common/licensing'; -import { AuthenticatedUser } from '../../common/model'; import { SecurityNavControl } from './nav_control_component'; +import { AuthenticationServiceSetup } from '../authentication'; interface SetupDeps { securityLicense: SecurityLicense; + authc: AuthenticationServiceSetup; } interface StartDeps { @@ -22,13 +23,15 @@ interface StartDeps { export class SecurityNavControlService { private securityLicense!: SecurityLicense; + private authc!: AuthenticationServiceSetup; private navControlRegistered!: boolean; private securityFeaturesSubscription?: Subscription; - public setup({ securityLicense }: SetupDeps) { + public setup({ securityLicense, authc }: SetupDeps) { this.securityLicense = securityLicense; + this.authc = authc; } public start({ core }: StartDeps) { @@ -38,14 +41,8 @@ export class SecurityNavControlService { const shouldRegisterNavControl = !isAnonymousPath && showLinks && !this.navControlRegistered; - if (shouldRegisterNavControl) { - const user = core.http.get('/internal/security/me', { - headers: { - 'kbn-system-api': true, - }, - }) as Promise; - this.registerSecurityNavControl(core, user); + this.registerSecurityNavControl(core); } } ); @@ -60,16 +57,16 @@ export class SecurityNavControlService { } private registerSecurityNavControl( - core: Pick, - user: Promise + core: Pick ) { + const currentUserPromise = this.authc.getCurrentUser(); core.chrome.navControls.registerRight({ order: 2000, mount: (el: HTMLElement) => { const I18nContext = core.i18n.Context; const props = { - user, + user: currentUserPromise, editProfileUrl: core.http.basePath.prepend('/app/kibana#/account'), logoutUrl: core.http.basePath.prepend(`/logout`), }; diff --git a/x-pack/plugins/security/public/plugin.ts b/x-pack/plugins/security/public/plugin.ts index 0f10f9d89f25a..50e0b838c750f 100644 --- a/x-pack/plugins/security/public/plugin.ts +++ b/x-pack/plugins/security/public/plugin.ts @@ -9,18 +9,20 @@ import { LicensingPluginSetup } from '../../licensing/public'; import { SessionExpired, SessionTimeout, + ISessionTimeout, SessionTimeoutHttpInterceptor, UnauthorizedResponseHttpInterceptor, } from './session'; import { SecurityLicenseService } from '../common/licensing'; import { SecurityNavControlService } from './nav_control'; +import { AuthenticationService } from './authentication'; export interface PluginSetupDependencies { licensing: LicensingPluginSetup; } export class SecurityPlugin implements Plugin { - private sessionTimeout!: SessionTimeout; + private sessionTimeout!: ISessionTimeout; private navControlService!: SecurityNavControlService; @@ -43,12 +45,15 @@ export class SecurityPlugin implements Plugin; private sessionInfo?: SessionInfo; private fetchTimer?: number; diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts index b3f96497b0538..4f1c25702ae97 100644 --- a/x-pack/plugins/security/server/config.ts +++ b/x-pack/plugins/security/server/config.ts @@ -8,7 +8,6 @@ import crypto from 'crypto'; import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { schema, Type, TypeOf } from '@kbn/config-schema'; -import { duration } from 'moment'; import { PluginInitializerContext } from '../../../../src/core/server'; export type ConfigType = ReturnType extends Observable @@ -35,7 +34,6 @@ export const ConfigSchema = schema.object( schema.maybe(schema.string({ minLength: 32 })), schema.string({ minLength: 32, defaultValue: 'a'.repeat(32) }) ), - sessionTimeout: schema.maybe(schema.nullable(schema.number())), // DEPRECATED session: schema.object({ idleTimeout: schema.nullable(schema.duration()), lifespan: schema.nullable(schema.duration()), @@ -88,22 +86,11 @@ export function createConfig$(context: PluginInitializerContext, isTLSEnabled: b secureCookies = true; } - // "sessionTimeout" is deprecated and replaced with "session.idleTimeout" - // however, NP does not yet have a mechanism to automatically rename deprecated keys - // for the time being, we'll do it manually: - const deprecatedSessionTimeout = - typeof config.sessionTimeout === 'number' ? duration(config.sessionTimeout) : null; - const val = { + return { ...config, encryptionKey, secureCookies, - session: { - ...config.session, - idleTimeout: config.session.idleTimeout || deprecatedSessionTimeout, - }, }; - delete val.sessionTimeout; // DEPRECATED - return val; }) ); } diff --git a/x-pack/plugins/security/server/index.ts b/x-pack/plugins/security/server/index.ts index e189b71345ffc..33f554be5caa3 100644 --- a/x-pack/plugins/security/server/index.ts +++ b/x-pack/plugins/security/server/index.ts @@ -4,9 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ -import { PluginInitializerContext } from '../../../../src/core/server'; +import { TypeOf } from '@kbn/config-schema'; +import { + PluginConfigDescriptor, + PluginInitializer, + PluginInitializerContext, + RecursiveReadonly, +} from '../../../../src/core/server'; import { ConfigSchema } from './config'; -import { Plugin } from './plugin'; +import { Plugin, PluginSetupContract, PluginSetupDependencies } from './plugin'; // These exports are part of public Security plugin contract, any change in signature of exported // functions or removal of exports should be considered as a breaking change. @@ -17,8 +23,17 @@ export { InvalidateAPIKeyParams, InvalidateAPIKeyResult, } from './authentication'; -export { PluginSetupContract } from './plugin'; +export { PluginSetupContract }; -export const config = { schema: ConfigSchema }; -export const plugin = (initializerContext: PluginInitializerContext) => - new Plugin(initializerContext); +export const config: PluginConfigDescriptor> = { + schema: ConfigSchema, + deprecations: ({ rename, unused }) => [ + rename('sessionTimeout', 'session.idleTimeout'), + unused('authorization.legacyFallback.enabled'), + ], +}; +export const plugin: PluginInitializer< + RecursiveReadonly, + void, + PluginSetupDependencies +> = (initializerContext: PluginInitializerContext) => new Plugin(initializerContext); diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts index cdd2a024310bb..9c4b01f94ef4d 100644 --- a/x-pack/plugins/security/server/plugin.ts +++ b/x-pack/plugins/security/server/plugin.ts @@ -110,10 +110,7 @@ export class Plugin { this.logger = this.initializerContext.logger.get(); } - public async setup( - core: CoreSetup, - { features, licensing }: PluginSetupDependencies - ): Promise> { + public async setup(core: CoreSetup, { features, licensing }: PluginSetupDependencies) { const [config, legacyConfig] = await combineLatest([ createConfig$(this.initializerContext, core.http.isTlsEnabled), this.initializerContext.config.legacy.globalConfig$, @@ -169,7 +166,7 @@ export class Plugin { csp: core.http.csp, }); - return deepFreeze({ + return deepFreeze({ authc, authz: { diff --git a/x-pack/test/licensing_plugin/public/updates.ts b/x-pack/test/licensing_plugin/public/updates.ts index 6d2253fe83868..2e750dcea1a59 100644 --- a/x-pack/test/licensing_plugin/public/updates.ts +++ b/x-pack/test/licensing_plugin/public/updates.ts @@ -32,7 +32,7 @@ export default function(ftrContext: FtrProviderContext) { // this call enforces signature check to detect license update // and causes license re-fetch await setup.core.http.get('/'); - await testUtils.delay(100); + await testUtils.delay(500); const licensing: LicensingPluginSetup = setup.plugins.licensing; licensing.license$.subscribe(license => cb(license.type)); @@ -48,7 +48,7 @@ export default function(ftrContext: FtrProviderContext) { // this call enforces signature check to detect license update // and causes license re-fetch await setup.core.http.get('/'); - await testUtils.delay(100); + await testUtils.delay(500); const licensing: LicensingPluginSetup = setup.plugins.licensing; licensing.license$.subscribe(license => cb(license.type)); @@ -64,7 +64,7 @@ export default function(ftrContext: FtrProviderContext) { // this call enforces signature check to detect license update // and causes license re-fetch await setup.core.http.get('/'); - await testUtils.delay(100); + await testUtils.delay(500); const licensing: LicensingPluginSetup = setup.plugins.licensing; licensing.license$.subscribe(license => cb(license.type)); @@ -80,7 +80,7 @@ export default function(ftrContext: FtrProviderContext) { // this call enforces signature check to detect license update // and causes license re-fetch await setup.core.http.get('/'); - await testUtils.delay(100); + await testUtils.delay(500); const licensing: LicensingPluginSetup = setup.plugins.licensing; licensing.license$.subscribe(license => cb(license.type));