From 2d8bf2160701601a949f02df8b7aa9a92a784e83 Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Wed, 11 Dec 2019 10:13:26 +0100 Subject: [PATCH] [7.x] Decouple Authorization subsystem from Legacy API. (#52719) --- x-pack/legacy/plugins/security/index.js | 1 - .../authorization/check_privileges.test.ts | 6 +++--- .../server/authorization/check_privileges.ts | 11 ++++++----- .../security/server/authorization/index.mock.ts | 7 +++++-- .../security/server/authorization/index.test.ts | 7 +++---- .../security/server/authorization/index.ts | 16 ++++++++-------- x-pack/plugins/security/server/plugin.ts | 10 ++++++---- .../routes/authorization/roles/get.test.ts | 2 +- .../server/routes/authorization/roles/get.ts | 2 +- .../routes/authorization/roles/get_all.test.ts | 2 +- .../server/routes/authorization/roles/get_all.ts | 6 +----- .../routes/authorization/roles/put.test.ts | 2 +- .../server/routes/authorization/roles/put.ts | 2 +- 13 files changed, 37 insertions(+), 37 deletions(-) diff --git a/x-pack/legacy/plugins/security/index.js b/x-pack/legacy/plugins/security/index.js index 7ec21d43eeadc..0f4940da8f59d 100644 --- a/x-pack/legacy/plugins/security/index.js +++ b/x-pack/legacy/plugins/security/index.js @@ -151,7 +151,6 @@ export const security = (kibana) => new kibana.Plugin({ server.plugins.kibana.systemApi ), cspRules: createCSPRuleString(config.get('csp.rules')), - kibanaIndexName: config.get('kibana.index'), }); // Legacy xPack Info endpoint returns whatever we return in a callback for `registerLicenseCheckResultsGenerator` diff --git a/x-pack/plugins/security/server/authorization/check_privileges.test.ts b/x-pack/plugins/security/server/authorization/check_privileges.test.ts index b1cb78008da00..8c1241937892e 100644 --- a/x-pack/plugins/security/server/authorization/check_privileges.test.ts +++ b/x-pack/plugins/security/server/authorization/check_privileges.test.ts @@ -48,7 +48,7 @@ describe('#atSpace', () => { const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory( mockActions, mockClusterClient, - () => application + application ); const request = httpServerMock.createKibanaRequest(); const checkPrivileges = checkPrivilegesWithRequest(request); @@ -291,7 +291,7 @@ describe('#atSpaces', () => { const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory( mockActions, mockClusterClient, - () => application + application ); const request = httpServerMock.createKibanaRequest(); const checkPrivileges = checkPrivilegesWithRequest(request); @@ -772,7 +772,7 @@ describe('#globally', () => { const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory( mockActions, mockClusterClient, - () => application + application ); const request = httpServerMock.createKibanaRequest(); const checkPrivileges = checkPrivilegesWithRequest(request); diff --git a/x-pack/plugins/security/server/authorization/check_privileges.ts b/x-pack/plugins/security/server/authorization/check_privileges.ts index 5bc3ce075452d..3ef7a8f29a0bf 100644 --- a/x-pack/plugins/security/server/authorization/check_privileges.ts +++ b/x-pack/plugins/security/server/authorization/check_privileges.ts @@ -61,7 +61,7 @@ export interface CheckPrivileges { export function checkPrivilegesWithRequestFactory( actions: CheckPrivilegesActions, clusterClient: IClusterClient, - getApplicationName: () => string + applicationName: string ) { const hasIncompatibleVersion = ( applicationPrivilegesResponse: HasPrivilegesResponseApplication @@ -81,23 +81,24 @@ export function checkPrivilegesWithRequestFactory( : [privilegeOrPrivileges]; const allApplicationPrivileges = uniq([actions.version, actions.login, ...privileges]); - const application = getApplicationName(); const hasPrivilegesResponse = (await clusterClient .asScoped(request) .callAsCurrentUser('shield.hasPrivileges', { body: { - applications: [{ application, resources, privileges: allApplicationPrivileges }], + applications: [ + { application: applicationName, resources, privileges: allApplicationPrivileges }, + ], }, })) as HasPrivilegesResponse; validateEsPrivilegeResponse( hasPrivilegesResponse, - application, + applicationName, allApplicationPrivileges, resources ); - const applicationPrivilegesResponse = hasPrivilegesResponse.application[application]; + const applicationPrivilegesResponse = hasPrivilegesResponse.application[applicationName]; if (hasIncompatibleVersion(applicationPrivilegesResponse)) { throw new Error( diff --git a/x-pack/plugins/security/server/authorization/index.mock.ts b/x-pack/plugins/security/server/authorization/index.mock.ts index 2e700745c69dc..930ede4157723 100644 --- a/x-pack/plugins/security/server/authorization/index.mock.ts +++ b/x-pack/plugins/security/server/authorization/index.mock.ts @@ -8,12 +8,15 @@ import { Actions } from '.'; import { AuthorizationMode } from './mode'; export const authorizationMock = { - create: ({ version = 'mock-version' }: { version?: string } = {}) => ({ + create: ({ + version = 'mock-version', + applicationName = 'mock-application', + }: { version?: string; applicationName?: string } = {}) => ({ actions: new Actions(version), checkPrivilegesWithRequest: jest.fn(), checkPrivilegesDynamicallyWithRequest: jest.fn(), checkSavedObjectsPrivilegesWithRequest: jest.fn(), - getApplicationName: jest.fn().mockReturnValue('mock-application'), + applicationName, mode: { useRbacForRequest: jest.fn() } as jest.Mocked, privileges: { get: jest.fn() }, registerPrivilegesWithCluster: jest.fn(), diff --git a/x-pack/plugins/security/server/authorization/index.test.ts b/x-pack/plugins/security/server/authorization/index.test.ts index 24179e062230a..34b9efea77165 100644 --- a/x-pack/plugins/security/server/authorization/index.test.ts +++ b/x-pack/plugins/security/server/authorization/index.test.ts @@ -53,7 +53,6 @@ test(`returns exposed services`, () => { .fn() .mockReturnValue({ getSpaceId: jest.fn(), namespaceToSpaceId: jest.fn() }); const mockFeaturesService = { getFeatures: () => [] }; - const mockGetLegacyAPI = () => ({ kibanaIndexName }); const mockLicense = licenseMock.create(); const authz = setupAuthorization({ @@ -61,20 +60,20 @@ test(`returns exposed services`, () => { clusterClient: mockClusterClient, license: mockLicense, loggers: loggingServiceMock.create(), - getLegacyAPI: mockGetLegacyAPI, + kibanaIndexName, packageVersion: 'some-version', featuresService: mockFeaturesService, getSpacesService: mockGetSpacesService, }); expect(authz.actions.version).toBe('version:some-version'); - expect(authz.getApplicationName()).toBe(application); + expect(authz.applicationName).toBe(application); expect(authz.checkPrivilegesWithRequest).toBe(mockCheckPrivilegesWithRequest); expect(checkPrivilegesWithRequestFactory).toHaveBeenCalledWith( authz.actions, mockClusterClient, - authz.getApplicationName + authz.applicationName ); expect(authz.checkPrivilegesDynamicallyWithRequest).toBe( diff --git a/x-pack/plugins/security/server/authorization/index.ts b/x-pack/plugins/security/server/authorization/index.ts index b5f9efadbd8d0..41e6d12eb8f36 100644 --- a/x-pack/plugins/security/server/authorization/index.ts +++ b/x-pack/plugins/security/server/authorization/index.ts @@ -12,7 +12,7 @@ import { IClusterClient, } from '../../../../../src/core/server'; -import { FeaturesService, LegacyAPI, SpacesService } from '../plugin'; +import { FeaturesService, SpacesService } from '../plugin'; import { Actions } from './actions'; import { CheckPrivilegesWithRequest, checkPrivilegesWithRequestFactory } from './check_privileges'; import { @@ -43,7 +43,7 @@ interface SetupAuthorizationParams { license: SecurityLicense; loggers: LoggerFactory; featuresService: FeaturesService; - getLegacyAPI(): Pick; + kibanaIndexName: string; getSpacesService(): SpacesService | undefined; } @@ -52,7 +52,7 @@ export interface Authorization { checkPrivilegesWithRequest: CheckPrivilegesWithRequest; checkPrivilegesDynamicallyWithRequest: CheckPrivilegesDynamicallyWithRequest; checkSavedObjectsPrivilegesWithRequest: CheckSavedObjectsPrivilegesWithRequest; - getApplicationName: () => string; + applicationName: string; mode: AuthorizationMode; privileges: PrivilegesService; disableUnauthorizedCapabilities: ( @@ -69,23 +69,23 @@ export function setupAuthorization({ license, loggers, featuresService, - getLegacyAPI, + kibanaIndexName, getSpacesService, }: SetupAuthorizationParams): Authorization { const actions = new Actions(packageVersion); const mode = authorizationModeFactory(license); - const getApplicationName = () => `${APPLICATION_PREFIX}${getLegacyAPI().kibanaIndexName}`; + const applicationName = `${APPLICATION_PREFIX}${kibanaIndexName}`; const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory( actions, clusterClient, - getApplicationName + applicationName ); const privileges = privilegesFactory(actions, featuresService); const logger = loggers.get('authorization'); const authz = { actions, - getApplicationName, + applicationName, checkPrivilegesWithRequest, checkPrivilegesDynamicallyWithRequest: checkPrivilegesDynamicallyWithRequestFactory( checkPrivilegesWithRequest, @@ -123,7 +123,7 @@ export function setupAuthorization({ registerPrivilegesWithCluster: async () => { validateFeaturePrivileges(actions, featuresService.getFeatures()); - await registerPrivilegesWithCluster(logger, privileges, getApplicationName(), clusterClient); + await registerPrivilegesWithCluster(logger, privileges, applicationName, clusterClient); }, }; diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts index dd057e8c6d295..b1e46e1346c61 100644 --- a/x-pack/plugins/security/server/plugin.ts +++ b/x-pack/plugins/security/server/plugin.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Subscription } from 'rxjs'; +import { Subscription, combineLatest } from 'rxjs'; import { first } from 'rxjs/operators'; import { IClusterClient, @@ -43,7 +43,6 @@ export type FeaturesService = Pick; export interface LegacyAPI { serverConfig: { protocol: string; hostname: string; port: number }; isSystemAPIRequest: (request: KibanaRequest) => boolean; - kibanaIndexName: string; cspRules: string; savedObjects: SavedObjectsLegacyService; auditLogger: { @@ -122,7 +121,10 @@ export class Plugin { core: CoreSetup, { features, licensing }: PluginSetupDependencies ): Promise> { - const config = await createConfig$(this.initializerContext, core.http.isTlsEnabled) + const [config, legacyConfig] = await combineLatest([ + createConfig$(this.initializerContext, core.http.isTlsEnabled), + this.initializerContext.config.legacy.globalConfig$, + ]) .pipe(first()) .toPromise(); @@ -149,7 +151,7 @@ export class Plugin { clusterClient: this.clusterClient, license, loggers: this.initializerContext.logger, - getLegacyAPI: this.getLegacyAPI, + kibanaIndexName: legacyConfig.kibana.index, packageVersion: this.initializerContext.env.packageInfo.version, getSpacesService: this.getSpacesService, featuresService: features, diff --git a/x-pack/plugins/security/server/routes/authorization/roles/get.test.ts b/x-pack/plugins/security/server/routes/authorization/roles/get.test.ts index 1cfc1ae416ae4..447d890605cc9 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/get.test.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/get.test.ts @@ -36,7 +36,7 @@ describe('GET role', () => { ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - mockRouteDefinitionParams.authz.getApplicationName.mockReturnValue(application); + mockRouteDefinitionParams.authz.applicationName = application; const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient(); mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); diff --git a/x-pack/plugins/security/server/routes/authorization/roles/get.ts b/x-pack/plugins/security/server/routes/authorization/roles/get.ts index be69e222dd093..1173d37cba64f 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/get.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/get.ts @@ -28,7 +28,7 @@ export function defineGetRolesRoutes({ router, authz, clusterClient }: RouteDefi body: transformElasticsearchRoleToRole( elasticsearchRole, request.params.name, - authz.getApplicationName() + authz.applicationName ), }); } diff --git a/x-pack/plugins/security/server/routes/authorization/roles/get_all.test.ts b/x-pack/plugins/security/server/routes/authorization/roles/get_all.test.ts index 76ce6a272e285..3bd85122c95d1 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/get_all.test.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/get_all.test.ts @@ -31,7 +31,7 @@ describe('GET all roles', () => { ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - mockRouteDefinitionParams.authz.getApplicationName.mockReturnValue(application); + mockRouteDefinitionParams.authz.applicationName = application; const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient(); mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); diff --git a/x-pack/plugins/security/server/routes/authorization/roles/get_all.ts b/x-pack/plugins/security/server/routes/authorization/roles/get_all.ts index f5d2d51280fc4..6ff431f0f8b6a 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/get_all.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/get_all.ts @@ -22,11 +22,7 @@ export function defineGetAllRolesRoutes({ router, authz, clusterClient }: RouteD return response.ok({ body: Object.entries(elasticsearchRoles) .map(([roleName, elasticsearchRole]) => - transformElasticsearchRoleToRole( - elasticsearchRole, - roleName, - authz.getApplicationName() - ) + transformElasticsearchRoleToRole(elasticsearchRole, roleName, authz.applicationName) ) .sort((roleA, roleB) => { if (roleA.name < roleB.name) { diff --git a/x-pack/plugins/security/server/routes/authorization/roles/put.test.ts b/x-pack/plugins/security/server/routes/authorization/roles/put.test.ts index 31963987c2efb..cb80549df8417 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/put.test.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/put.test.ts @@ -62,7 +62,7 @@ const putRoleTest = ( ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - mockRouteDefinitionParams.authz.getApplicationName.mockReturnValue(application); + mockRouteDefinitionParams.authz.applicationName = application; mockRouteDefinitionParams.authz.privileges.get.mockReturnValue(privilegeMap); const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient(); diff --git a/x-pack/plugins/security/server/routes/authorization/roles/put.ts b/x-pack/plugins/security/server/routes/authorization/roles/put.ts index 92c940132e660..e0245e7260446 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/put.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/put.ts @@ -42,7 +42,7 @@ export function definePutRolesRoutes({ router, authz, clusterClient }: RouteDefi const body = transformPutPayloadToElasticsearchRole( request.body, - authz.getApplicationName(), + authz.applicationName, rawRoles[name] ? rawRoles[name].applications : [] );