From a5b198c95bab555283651f4baf8ca501d8a9c6c5 Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Mon, 30 Nov 2020 16:21:25 +0100 Subject: [PATCH] Migrate security routes to a new Elasticsearch client. --- .../elasticsearch_client_plugin.ts | 254 ------------------ x-pack/plugins/security/server/plugin.ts | 1 - .../server/routes/api_keys/enabled.test.ts | 165 +++++------- .../server/routes/api_keys/get.test.ts | 27 +- .../security/server/routes/api_keys/get.ts | 10 +- .../server/routes/api_keys/invalidate.test.ts | 41 ++- .../server/routes/api_keys/invalidate.ts | 8 +- .../server/routes/api_keys/privileges.test.ts | 233 +++++----------- .../server/routes/api_keys/privileges.ts | 26 +- .../authorization/privileges/get_builtin.ts | 8 +- .../routes/authorization/roles/delete.test.ts | 30 +-- .../routes/authorization/roles/delete.ts | 4 +- .../routes/authorization/roles/get.test.ts | 30 +-- .../server/routes/authorization/roles/get.ts | 12 +- .../authorization/roles/get_all.test.ts | 27 +- .../routes/authorization/roles/get_all.ts | 10 +- .../routes/authorization/roles/put.test.ts | 182 ++++++------- .../server/routes/authorization/roles/put.ts | 21 +- .../security/server/routes/index.mock.ts | 2 - .../plugins/security/server/routes/index.ts | 9 +- .../server/routes/indices/get_fields.test.ts | 14 +- .../server/routes/indices/get_fields.ts | 16 +- .../server/routes/role_mapping/delete.test.ts | 52 ++-- .../server/routes/role_mapping/delete.ts | 14 +- .../routes/role_mapping/feature_check.test.ts | 92 +++---- .../routes/role_mapping/feature_check.ts | 46 ++-- .../server/routes/role_mapping/get.test.ts | 100 +++---- .../server/routes/role_mapping/get.ts | 12 +- .../server/routes/role_mapping/post.test.ts | 71 +++-- .../server/routes/role_mapping/post.ts | 15 +- .../routes/users/change_password.test.ts | 98 +++---- .../server/routes/users/change_password.ts | 45 ++-- .../server/routes/users/create_or_update.ts | 4 +- .../security/server/routes/users/delete.ts | 8 +- .../security/server/routes/users/get.ts | 12 +- .../security/server/routes/users/get_all.ts | 4 +- 36 files changed, 610 insertions(+), 1093 deletions(-) diff --git a/x-pack/plugins/security/server/elasticsearch/elasticsearch_client_plugin.ts b/x-pack/plugins/security/server/elasticsearch/elasticsearch_client_plugin.ts index 529e8a8aa6e9cc..0a43d8dd6973af 100644 --- a/x-pack/plugins/security/server/elasticsearch/elasticsearch_client_plugin.ts +++ b/x-pack/plugins/security/server/elasticsearch/elasticsearch_client_plugin.ts @@ -22,133 +22,6 @@ export function elasticsearchClientPlugin(Client: any, config: unknown, componen }, }); - /** - * Perform a [shield.changePassword](Change the password of a user) request - * - * @param {Object} params - An object with parameters used to carry out this action - * @param {Boolean} params.refresh - Refresh the index after performing the operation - * @param {String} params.username - The username of the user to change the password for - */ - shield.changePassword = ca({ - params: { - refresh: { - type: 'boolean', - }, - }, - urls: [ - { - fmt: '/_security/user/<%=username%>/_password', - req: { - username: { - type: 'string', - required: false, - }, - }, - }, - { - fmt: '/_security/user/_password', - }, - ], - needBody: true, - method: 'POST', - }); - - /** - * Perform a [shield.clearCachedRealms](Clears the internal user caches for specified realms) request - * - * @param {Object} params - An object with parameters used to carry out this action - * @param {String} params.usernames - Comma-separated list of usernames to clear from the cache - * @param {String} params.realms - Comma-separated list of realms to clear - */ - shield.clearCachedRealms = ca({ - params: { - usernames: { - type: 'string', - required: false, - }, - }, - url: { - fmt: '/_security/realm/<%=realms%>/_clear_cache', - req: { - realms: { - type: 'string', - required: true, - }, - }, - }, - method: 'POST', - }); - - /** - * Perform a [shield.clearCachedRoles](Clears the internal caches for specified roles) request - * - * @param {Object} params - An object with parameters used to carry out this action - * @param {String} params.name - Role name - */ - shield.clearCachedRoles = ca({ - params: {}, - url: { - fmt: '/_security/role/<%=name%>/_clear_cache', - req: { - name: { - type: 'string', - required: true, - }, - }, - }, - method: 'POST', - }); - - /** - * Perform a [shield.deleteRole](Remove a role from the native shield realm) request - * - * @param {Object} params - An object with parameters used to carry out this action - * @param {Boolean} params.refresh - Refresh the index after performing the operation - * @param {String} params.name - Role name - */ - shield.deleteRole = ca({ - params: { - refresh: { - type: 'boolean', - }, - }, - url: { - fmt: '/_security/role/<%=name%>', - req: { - name: { - type: 'string', - required: true, - }, - }, - }, - method: 'DELETE', - }); - - /** - * Perform a [shield.deleteUser](Remove a user from the native shield realm) request - * - * @param {Object} params - An object with parameters used to carry out this action - * @param {Boolean} params.refresh - Refresh the index after performing the operation - * @param {String} params.username - username - */ - shield.deleteUser = ca({ - params: { - refresh: { - type: 'boolean', - }, - }, - url: { - fmt: '/_security/user/<%=username%>', - req: { - username: { - type: 'string', - required: true, - }, - }, - }, - method: 'DELETE', - }); - /** * Perform a [shield.getRole](Retrieve one or more roles from the native shield realm) request * @@ -173,30 +46,6 @@ export function elasticsearchClientPlugin(Client: any, config: unknown, componen ], }); - /** - * Perform a [shield.getUser](Retrieve one or more users from the native shield realm) request - * - * @param {Object} params - An object with parameters used to carry out this action - * @param {String, String[], Boolean} params.username - A comma-separated list of usernames - */ - shield.getUser = ca({ - params: {}, - urls: [ - { - fmt: '/_security/user/<%=username%>', - req: { - username: { - type: 'list', - required: false, - }, - }, - }, - { - fmt: '/_security/user', - }, - ], - }); - /** * Perform a [shield.putRole](Update or create a role for the native shield realm) request * @@ -249,19 +98,6 @@ export function elasticsearchClientPlugin(Client: any, config: unknown, componen method: 'PUT', }); - /** - * Perform a [shield.getUserPrivileges](Retrieve a user's list of privileges) request - * - */ - shield.getUserPrivileges = ca({ - params: {}, - urls: [ - { - fmt: '/_security/user/_privileges', - }, - ], - }); - /** * Asks Elasticsearch to prepare SAML authentication request to be sent to * the 3rd-party SAML identity provider. @@ -489,36 +325,6 @@ export function elasticsearchClientPlugin(Client: any, config: unknown, componen }, }); - shield.getBuiltinPrivileges = ca({ - params: {}, - urls: [ - { - fmt: '/_security/privilege/_builtin', - }, - ], - }); - - /** - * Gets API keys in Elasticsearch - * @param {boolean} owner A boolean flag that can be used to query API keys owned by the currently authenticated user. - * Defaults to false. The realm_name or username parameters cannot be specified when this parameter is set to true as - * they are assumed to be the currently authenticated ones. - */ - shield.getAPIKeys = ca({ - method: 'GET', - urls: [ - { - fmt: `/_security/api_key?owner=<%=owner%>`, - req: { - owner: { - type: 'boolean', - required: true, - }, - }, - }, - ], - }); - /** * Creates an API key in Elasticsearch for the current user. * @@ -591,64 +397,4 @@ export function elasticsearchClientPlugin(Client: any, config: unknown, componen fmt: '/_security/delegate_pki', }, }); - - /** - * Retrieves all configured role mappings. - * - * @returns {{ [roleMappingName]: { enabled: boolean; roles: string[]; rules: Record} }} - */ - shield.getRoleMappings = ca({ - method: 'GET', - urls: [ - { - fmt: '/_security/role_mapping', - }, - { - fmt: '/_security/role_mapping/<%=name%>', - req: { - name: { - type: 'string', - required: true, - }, - }, - }, - ], - }); - - /** - * Saves the specified role mapping. - */ - shield.saveRoleMapping = ca({ - method: 'POST', - needBody: true, - urls: [ - { - fmt: '/_security/role_mapping/<%=name%>', - req: { - name: { - type: 'string', - required: true, - }, - }, - }, - ], - }); - - /** - * Deletes the specified role mapping. - */ - shield.deleteRoleMapping = ca({ - method: 'DELETE', - urls: [ - { - fmt: '/_security/role_mapping/<%=name%>', - req: { - name: { - type: 'string', - required: true, - }, - }, - }, - ], - }); } diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts index 17f2480026cc72..d6fe1356ce145e 100644 --- a/x-pack/plugins/security/server/plugin.ts +++ b/x-pack/plugins/security/server/plugin.ts @@ -230,7 +230,6 @@ export class Plugin { basePath: core.http.basePath, httpResources: core.http.resources, logger: this.initializerContext.logger.get('routes'), - clusterClient, config, authc: this.authc, authz, diff --git a/x-pack/plugins/security/server/routes/api_keys/enabled.test.ts b/x-pack/plugins/security/server/routes/api_keys/enabled.test.ts index 7968402cd21765..3a22b9fe003a1a 100644 --- a/x-pack/plugins/security/server/routes/api_keys/enabled.test.ts +++ b/x-pack/plugins/security/server/routes/api_keys/enabled.test.ts @@ -4,115 +4,96 @@ * you may not use this file except in compliance with the Elastic License. */ -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../src/core/server'; -import { LicenseCheck } from '../../../../licensing/server'; +import Boom from '@hapi/boom'; +import { + kibanaResponseFactory, + RequestHandler, + RequestHandlerContext, +} from '../../../../../../src/core/server'; import { httpServerMock } from '../../../../../../src/core/server/mocks'; import { routeDefinitionParamsMock } from '../index.mock'; -import Boom from '@hapi/boom'; -import { defineEnabledApiKeysRoutes } from './enabled'; -import { APIKeys } from '../../authentication/api_keys'; -interface TestOptions { - licenseCheckResult?: LicenseCheck; - apiResponse?: () => Promise; - asserts: { statusCode: number; result?: Record }; -} +import { defineEnabledApiKeysRoutes } from './enabled'; +import { Authentication } from '../../authentication'; describe('API keys enabled', () => { - const enabledApiKeysTest = ( - description: string, - { licenseCheckResult = { state: 'valid' }, apiResponse, asserts }: TestOptions - ) => { - test(description, async () => { - const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - - const apiKeys = new APIKeys({ - logger: mockRouteDefinitionParams.logger, - clusterClient: mockRouteDefinitionParams.clusterClient, - license: mockRouteDefinitionParams.license, - }); - - mockRouteDefinitionParams.authc.areAPIKeysEnabled.mockImplementation(() => - apiKeys.areAPIKeysEnabled() + function getMockContext( + licenseCheckResult: { state: string; message?: string } = { state: 'valid' } + ) { + return ({ + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, + } as unknown) as RequestHandlerContext; + } + + let routeHandler: RequestHandler; + let authc: jest.Mocked; + beforeEach(() => { + const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + authc = mockRouteDefinitionParams.authc; + + defineEnabledApiKeysRoutes(mockRouteDefinitionParams); + + const [, apiKeyRouteHandler] = mockRouteDefinitionParams.router.get.mock.calls.find( + ([{ path }]) => path === '/internal/security/api_key/_enabled' + )!; + routeHandler = apiKeyRouteHandler; + }); + + describe('failure', () => { + test('returns result of license checker', async () => { + const mockContext = getMockContext({ state: 'invalid', message: 'test forbidden message' }); + const response = await routeHandler( + mockContext, + httpServerMock.createKibanaRequest(), + kibanaResponseFactory ); - if (apiResponse) { - mockRouteDefinitionParams.clusterClient.callAsInternalUser.mockImplementation(apiResponse); - } - - defineEnabledApiKeysRoutes(mockRouteDefinitionParams); - const [[, handler]] = mockRouteDefinitionParams.router.get.mock.calls; - - const headers = { authorization: 'foo' }; - const mockRequest = httpServerMock.createKibanaRequest({ - method: 'get', - path: '/internal/security/api_key/_enabled', - headers, - }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; - - const response = await handler(mockContext, mockRequest, kibanaResponseFactory); - expect(response.status).toBe(asserts.statusCode); - expect(response.payload).toEqual(asserts.result); - - if (apiResponse) { - expect(mockRouteDefinitionParams.clusterClient.callAsInternalUser).toHaveBeenCalledWith( - 'shield.invalidateAPIKey', - { - body: { - id: expect.any(String), - }, - } - ); - } else { - expect(mockRouteDefinitionParams.clusterClient.asScoped).not.toHaveBeenCalled(); - } + expect(response.status).toBe(403); + expect(response.payload).toEqual({ message: 'test forbidden message' }); expect(mockContext.licensing.license.check).toHaveBeenCalledWith('security', 'basic'); }); - }; - describe('failure', () => { - enabledApiKeysTest('returns result of license checker', { - licenseCheckResult: { state: 'invalid', message: 'test forbidden message' }, - asserts: { statusCode: 403, result: { message: 'test forbidden message' } }, - }); + test('returns error from cluster client', async () => { + const error = Boom.notAcceptable('test not acceptable message'); + authc.areAPIKeysEnabled.mockRejectedValue(error); + + const response = await routeHandler( + getMockContext(), + httpServerMock.createKibanaRequest(), + kibanaResponseFactory + ); - const error = Boom.notAcceptable('test not acceptable message'); - enabledApiKeysTest('returns error from cluster client', { - apiResponse: async () => { - throw error; - }, - asserts: { statusCode: 406, result: error }, + expect(response.status).toBe(406); + expect(response.payload).toEqual(error); }); }); describe('success', () => { - enabledApiKeysTest('returns true if API Keys are enabled', { - apiResponse: async () => ({}), - asserts: { - statusCode: 200, - result: { - apiKeysEnabled: true, - }, - }, + test('returns true if API Keys are enabled', async () => { + authc.areAPIKeysEnabled.mockResolvedValue(true); + + const response = await routeHandler( + getMockContext(), + httpServerMock.createKibanaRequest(), + kibanaResponseFactory + ); + + expect(response.status).toBe(200); + expect(response.payload).toEqual({ apiKeysEnabled: true }); }); - enabledApiKeysTest('returns false if API Keys are disabled', { - apiResponse: async () => { - const error = new Error(); - (error as any).body = { - error: { 'disabled.feature': 'api_keys' }, - }; - throw error; - }, - asserts: { - statusCode: 200, - result: { - apiKeysEnabled: false, - }, - }, + + test('returns false if API Keys are disabled', async () => { + authc.areAPIKeysEnabled.mockResolvedValue(false); + + const response = await routeHandler( + getMockContext(), + httpServerMock.createKibanaRequest(), + kibanaResponseFactory + ); + + expect(response.status).toBe(200); + expect(response.payload).toEqual({ apiKeysEnabled: false }); }); }); }); diff --git a/x-pack/plugins/security/server/routes/api_keys/get.test.ts b/x-pack/plugins/security/server/routes/api_keys/get.test.ts index cb991fb2f5aac4..cc9e9a68e6e360 100644 --- a/x-pack/plugins/security/server/routes/api_keys/get.test.ts +++ b/x-pack/plugins/security/server/routes/api_keys/get.test.ts @@ -4,11 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../src/core/server'; +import { kibanaResponseFactory } from '../../../../../../src/core/server'; import { LicenseCheck } from '../../../../licensing/server'; import { defineGetApiKeysRoutes } from './get'; -import { elasticsearchServiceMock, httpServerMock } from '../../../../../../src/core/server/mocks'; +import { httpServerMock, coreMock } from '../../../../../../src/core/server/mocks'; import { routeDefinitionParamsMock } from '../index.mock'; import Boom from '@hapi/boom'; @@ -26,11 +26,15 @@ describe('Get API keys', () => { ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); if (apiResponse) { - mockScopedClusterClient.callAsCurrentUser.mockImplementation(apiResponse); + mockContext.core.elasticsearch.client.asCurrentUser.security.getApiKey.mockImplementation( + (async () => ({ body: await apiResponse() })) as any + ); } defineGetApiKeysRoutes(mockRouteDefinitionParams); @@ -43,22 +47,15 @@ describe('Get API keys', () => { query: { isAdmin: isAdmin.toString() }, headers, }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(asserts.statusCode); expect(response.payload).toEqual(asserts.result); if (apiResponse) { - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith( - 'shield.getAPIKeys', - { owner: !isAdmin } - ); - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.getApiKey + ).toHaveBeenCalledWith({ owner: !isAdmin }); } expect(mockContext.licensing.license.check).toHaveBeenCalledWith('security', 'basic'); }); diff --git a/x-pack/plugins/security/server/routes/api_keys/get.ts b/x-pack/plugins/security/server/routes/api_keys/get.ts index 6e98b4b098405b..b0c6ec090a0e5d 100644 --- a/x-pack/plugins/security/server/routes/api_keys/get.ts +++ b/x-pack/plugins/security/server/routes/api_keys/get.ts @@ -10,7 +10,7 @@ import { wrapIntoCustomErrorResponse } from '../../errors'; import { createLicensedRouteHandler } from '../licensed_route_handler'; import { RouteDefinitionParams } from '..'; -export function defineGetApiKeysRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineGetApiKeysRoutes({ router }: RouteDefinitionParams) { router.get( { path: '/internal/security/api_key', @@ -28,11 +28,11 @@ export function defineGetApiKeysRoutes({ router, clusterClient }: RouteDefinitio createLicensedRouteHandler(async (context, request, response) => { try { const isAdmin = request.query.isAdmin === 'true'; - const { api_keys: apiKeys } = (await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.getAPIKeys', { owner: !isAdmin })) as { api_keys: ApiKey[] }; + const apiResponse = await context.core.elasticsearch.client.asCurrentUser.security.getApiKey<{ + api_keys: ApiKey[]; + }>({ owner: !isAdmin }); - const validKeys = apiKeys.filter(({ invalidated }) => !invalidated); + const validKeys = apiResponse.body.api_keys.filter(({ invalidated }) => !invalidated); return response.ok({ body: { apiKeys: validKeys } }); } catch (error) { diff --git a/x-pack/plugins/security/server/routes/api_keys/invalidate.test.ts b/x-pack/plugins/security/server/routes/api_keys/invalidate.test.ts index 88e52f735395de..9ac41fdfa74837 100644 --- a/x-pack/plugins/security/server/routes/api_keys/invalidate.test.ts +++ b/x-pack/plugins/security/server/routes/api_keys/invalidate.test.ts @@ -6,18 +6,18 @@ import Boom from '@hapi/boom'; import { Type } from '@kbn/config-schema'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../src/core/server'; +import { kibanaResponseFactory } from '../../../../../../src/core/server'; import { LicenseCheck } from '../../../../licensing/server'; import { defineInvalidateApiKeysRoutes } from './invalidate'; -import { elasticsearchServiceMock, httpServerMock } from '../../../../../../src/core/server/mocks'; +import { coreMock, httpServerMock } from '../../../../../../src/core/server/mocks'; import { routeDefinitionParamsMock } from '../index.mock'; interface TestOptions { licenseCheckResult?: LicenseCheck; apiResponses?: Array<() => Promise>; payload?: Record; - asserts: { statusCode: number; result?: Record; apiArguments?: unknown[][] }; + asserts: { statusCode: number; result?: Record; apiArguments?: unknown[] }; } describe('Invalidate API keys', () => { @@ -27,10 +27,15 @@ describe('Invalidate API keys', () => { ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; + for (const apiResponse of apiResponses) { - mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(apiResponse); + mockContext.core.elasticsearch.client.asCurrentUser.security.invalidateApiKey.mockImplementationOnce( + (async () => ({ body: await apiResponse() })) as any + ); } defineInvalidateApiKeysRoutes(mockRouteDefinitionParams); @@ -43,9 +48,6 @@ describe('Invalidate API keys', () => { body: payload !== undefined ? (validate as any).body.validate(payload) : undefined, headers, }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(asserts.statusCode); @@ -53,13 +55,10 @@ describe('Invalidate API keys', () => { if (Array.isArray(asserts.apiArguments)) { for (const apiArguments of asserts.apiArguments) { - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith( - mockRequest - ); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith(...apiArguments); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.invalidateApiKey + ).toHaveBeenCalledWith(apiArguments); } - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); } expect(mockContext.licensing.license.check).toHaveBeenCalledWith('security', 'basic'); }); @@ -128,7 +127,7 @@ describe('Invalidate API keys', () => { isAdmin: true, }, asserts: { - apiArguments: [['shield.invalidateAPIKey', { body: { id: 'si8If24B1bKsmSLTAhJV' } }]], + apiArguments: [{ body: { id: 'si8If24B1bKsmSLTAhJV' } }], statusCode: 200, result: { itemsInvalidated: [], @@ -152,7 +151,7 @@ describe('Invalidate API keys', () => { isAdmin: true, }, asserts: { - apiArguments: [['shield.invalidateAPIKey', { body: { id: 'si8If24B1bKsmSLTAhJV' } }]], + apiArguments: [{ body: { id: 'si8If24B1bKsmSLTAhJV' } }], statusCode: 200, result: { itemsInvalidated: [{ id: 'si8If24B1bKsmSLTAhJV', name: 'my-api-key' }], @@ -168,9 +167,7 @@ describe('Invalidate API keys', () => { isAdmin: false, }, asserts: { - apiArguments: [ - ['shield.invalidateAPIKey', { body: { id: 'si8If24B1bKsmSLTAhJV', owner: true } }], - ], + apiArguments: [{ body: { id: 'si8If24B1bKsmSLTAhJV', owner: true } }], statusCode: 200, result: { itemsInvalidated: [{ id: 'si8If24B1bKsmSLTAhJV', name: 'my-api-key' }], @@ -195,8 +192,8 @@ describe('Invalidate API keys', () => { }, asserts: { apiArguments: [ - ['shield.invalidateAPIKey', { body: { id: 'si8If24B1bKsmSLTAhJV' } }], - ['shield.invalidateAPIKey', { body: { id: 'ab8If24B1bKsmSLTAhNC' } }], + { body: { id: 'si8If24B1bKsmSLTAhJV' } }, + { body: { id: 'ab8If24B1bKsmSLTAhNC' } }, ], statusCode: 200, result: { diff --git a/x-pack/plugins/security/server/routes/api_keys/invalidate.ts b/x-pack/plugins/security/server/routes/api_keys/invalidate.ts index dd472c0b60cbc1..39779541970074 100644 --- a/x-pack/plugins/security/server/routes/api_keys/invalidate.ts +++ b/x-pack/plugins/security/server/routes/api_keys/invalidate.ts @@ -15,7 +15,7 @@ interface ResponseType { errors: Array & { error: Error }>; } -export function defineInvalidateApiKeysRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineInvalidateApiKeysRoutes({ router }: RouteDefinitionParams) { router.post( { path: '/internal/security/api_key/invalidate', @@ -28,8 +28,6 @@ export function defineInvalidateApiKeysRoutes({ router, clusterClient }: RouteDe }, createLicensedRouteHandler(async (context, request, response) => { try { - const scopedClusterClient = clusterClient.asScoped(request); - // Invalidate all API keys in parallel. const invalidationResult = ( await Promise.all( @@ -41,7 +39,9 @@ export function defineInvalidateApiKeysRoutes({ router, clusterClient }: RouteDe } // Send the request to invalidate the API key and return an error if it could not be deleted. - await scopedClusterClient.callAsCurrentUser('shield.invalidateAPIKey', { body }); + await context.core.elasticsearch.client.asCurrentUser.security.invalidateApiKey({ + body, + }); return { key, error: undefined }; } catch (error) { return { key, error: wrapError(error) }; diff --git a/x-pack/plugins/security/server/routes/api_keys/privileges.test.ts b/x-pack/plugins/security/server/routes/api_keys/privileges.test.ts index ecc3d32e20aecc..b06d1329dc1dbb 100644 --- a/x-pack/plugins/security/server/routes/api_keys/privileges.test.ts +++ b/x-pack/plugins/security/server/routes/api_keys/privileges.test.ts @@ -6,23 +6,17 @@ import Boom from '@hapi/boom'; import { LicenseCheck } from '../../../../licensing/server'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../src/core/server'; +import { kibanaResponseFactory } from '../../../../../../src/core/server'; -import { elasticsearchServiceMock, httpServerMock } from '../../../../../../src/core/server/mocks'; +import { coreMock, httpServerMock } from '../../../../../../src/core/server/mocks'; import { routeDefinitionParamsMock } from '../index.mock'; import { defineCheckPrivilegesRoutes } from './privileges'; -import { APIKeys } from '../../authentication/api_keys'; interface TestOptions { licenseCheckResult?: LicenseCheck; - callAsInternalUserResponses?: Array<() => Promise>; - callAsCurrentUserResponses?: Array<() => Promise>; - asserts: { - statusCode: number; - result?: Record; - callAsInternalUserAPIArguments?: unknown[][]; - callAsCurrentUserAPIArguments?: unknown[][]; - }; + areAPIKeysEnabled?: boolean; + apiResponse?: () => Promise; + asserts: { statusCode: number; result?: Record; apiArguments?: unknown }; } describe('Check API keys privileges', () => { @@ -30,32 +24,23 @@ describe('Check API keys privileges', () => { description: string, { licenseCheckResult = { state: 'valid' }, - callAsInternalUserResponses = [], - callAsCurrentUserResponses = [], + areAPIKeysEnabled = true, + apiResponse, asserts, }: TestOptions ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; - const apiKeys = new APIKeys({ - logger: mockRouteDefinitionParams.logger, - clusterClient: mockRouteDefinitionParams.clusterClient, - license: mockRouteDefinitionParams.license, - }); - - mockRouteDefinitionParams.authc.areAPIKeysEnabled.mockImplementation(() => - apiKeys.areAPIKeysEnabled() - ); + mockRouteDefinitionParams.authc.areAPIKeysEnabled.mockResolvedValue(areAPIKeysEnabled); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - for (const apiResponse of callAsCurrentUserResponses) { - mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(apiResponse); - } - for (const apiResponse of callAsInternalUserResponses) { - mockRouteDefinitionParams.clusterClient.callAsInternalUser.mockImplementationOnce( - apiResponse + if (apiResponse) { + mockContext.core.elasticsearch.client.asCurrentUser.security.hasPrivileges.mockImplementation( + (async () => ({ body: await apiResponse() })) as any ); } @@ -68,33 +53,15 @@ describe('Check API keys privileges', () => { path: '/internal/security/api_key/privileges', headers, }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(asserts.statusCode); expect(response.payload).toEqual(asserts.result); - if (Array.isArray(asserts.callAsCurrentUserAPIArguments)) { - for (const apiArguments of asserts.callAsCurrentUserAPIArguments) { - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith( - mockRequest - ); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith(...apiArguments); - } - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); - } - - if (Array.isArray(asserts.callAsInternalUserAPIArguments)) { - for (const apiArguments of asserts.callAsInternalUserAPIArguments) { - expect(mockRouteDefinitionParams.clusterClient.callAsInternalUser).toHaveBeenCalledWith( - ...apiArguments - ); - } - } else { - expect(mockRouteDefinitionParams.clusterClient.callAsInternalUser).not.toHaveBeenCalled(); + if (asserts.apiArguments) { + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.hasPrivileges + ).toHaveBeenCalledWith(asserts.apiArguments); } expect(mockContext.licensing.license.check).toHaveBeenCalledWith('security', 'basic'); @@ -109,22 +76,13 @@ describe('Check API keys privileges', () => { const error = Boom.notAcceptable('test not acceptable message'); getPrivilegesTest('returns error from cluster client', { - callAsCurrentUserResponses: [ - async () => { - throw error; - }, - ], - callAsInternalUserResponses: [async () => {}], + apiResponse: async () => { + throw error; + }, asserts: { - callAsCurrentUserAPIArguments: [ - [ - 'shield.hasPrivileges', - { body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] } }, - ], - ], - callAsInternalUserAPIArguments: [ - ['shield.invalidateAPIKey', { body: { id: expect.any(String) } }], - ], + apiArguments: { + body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] }, + }, statusCode: 406, result: error, }, @@ -133,40 +91,17 @@ describe('Check API keys privileges', () => { describe('success', () => { getPrivilegesTest('returns areApiKeysEnabled and isAdmin', { - callAsCurrentUserResponses: [ - async () => ({ - username: 'elastic', - has_all_requested: true, - cluster: { manage_api_key: true, manage_security: true, manage_own_api_key: false }, - index: {}, - application: {}, - }), - ], - callAsInternalUserResponses: [ - async () => ({ - api_keys: [ - { - id: 'si8If24B1bKsmSLTAhJV', - name: 'my-api-key', - creation: 1574089261632, - expiration: 1574175661632, - invalidated: false, - username: 'elastic', - realm: 'reserved', - }, - ], - }), - ], + apiResponse: async () => ({ + username: 'elastic', + has_all_requested: true, + cluster: { manage_api_key: true, manage_security: true, manage_own_api_key: false }, + index: {}, + application: {}, + }), asserts: { - callAsCurrentUserAPIArguments: [ - [ - 'shield.hasPrivileges', - { body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] } }, - ], - ], - callAsInternalUserAPIArguments: [ - ['shield.invalidateAPIKey', { body: { id: expect.any(String) } }], - ], + apiArguments: { + body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] }, + }, statusCode: 200, result: { areApiKeysEnabled: true, isAdmin: true, canManage: true }, }, @@ -175,36 +110,18 @@ describe('Check API keys privileges', () => { getPrivilegesTest( 'returns areApiKeysEnabled=false when API Keys are disabled in Elasticsearch', { - callAsCurrentUserResponses: [ - async () => ({ - username: 'elastic', - has_all_requested: true, - cluster: { manage_api_key: true, manage_security: true, manage_own_api_key: true }, - index: {}, - application: {}, - }), - ], - callAsInternalUserResponses: [ - async () => { - const error = new Error(); - (error as any).body = { - error: { - 'disabled.feature': 'api_keys', - }, - }; - throw error; - }, - ], + apiResponse: async () => ({ + username: 'elastic', + has_all_requested: true, + cluster: { manage_api_key: true, manage_security: true, manage_own_api_key: true }, + index: {}, + application: {}, + }), + areAPIKeysEnabled: false, asserts: { - callAsCurrentUserAPIArguments: [ - [ - 'shield.hasPrivileges', - { body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] } }, - ], - ], - callAsInternalUserAPIArguments: [ - ['shield.invalidateAPIKey', { body: { id: expect.any(String) } }], - ], + apiArguments: { + body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] }, + }, statusCode: 200, result: { areApiKeysEnabled: false, isAdmin: true, canManage: true }, }, @@ -212,52 +129,34 @@ describe('Check API keys privileges', () => { ); getPrivilegesTest('returns isAdmin=false when user has insufficient privileges', { - callAsCurrentUserResponses: [ - async () => ({ - username: 'elastic', - has_all_requested: true, - cluster: { manage_api_key: false, manage_security: false, manage_own_api_key: false }, - index: {}, - application: {}, - }), - ], - callAsInternalUserResponses: [async () => ({})], + apiResponse: async () => ({ + username: 'elastic', + has_all_requested: true, + cluster: { manage_api_key: false, manage_security: false, manage_own_api_key: false }, + index: {}, + application: {}, + }), asserts: { - callAsCurrentUserAPIArguments: [ - [ - 'shield.hasPrivileges', - { body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] } }, - ], - ], - callAsInternalUserAPIArguments: [ - ['shield.invalidateAPIKey', { body: { id: expect.any(String) } }], - ], + apiArguments: { + body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] }, + }, statusCode: 200, result: { areApiKeysEnabled: true, isAdmin: false, canManage: false }, }, }); getPrivilegesTest('returns canManage=true when user can manage their own API Keys', { - callAsCurrentUserResponses: [ - async () => ({ - username: 'elastic', - has_all_requested: true, - cluster: { manage_api_key: false, manage_security: false, manage_own_api_key: true }, - index: {}, - application: {}, - }), - ], - callAsInternalUserResponses: [async () => ({})], + apiResponse: async () => ({ + username: 'elastic', + has_all_requested: true, + cluster: { manage_api_key: false, manage_security: false, manage_own_api_key: true }, + index: {}, + application: {}, + }), asserts: { - callAsCurrentUserAPIArguments: [ - [ - 'shield.hasPrivileges', - { body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] } }, - ], - ], - callAsInternalUserAPIArguments: [ - ['shield.invalidateAPIKey', { body: { id: expect.any(String) } }], - ], + apiArguments: { + body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] }, + }, statusCode: 200, result: { areApiKeysEnabled: true, isAdmin: false, canManage: true }, }, diff --git a/x-pack/plugins/security/server/routes/api_keys/privileges.ts b/x-pack/plugins/security/server/routes/api_keys/privileges.ts index 9cccb967527729..dd5d81060c7e53 100644 --- a/x-pack/plugins/security/server/routes/api_keys/privileges.ts +++ b/x-pack/plugins/security/server/routes/api_keys/privileges.ts @@ -8,11 +8,7 @@ import { wrapIntoCustomErrorResponse } from '../../errors'; import { createLicensedRouteHandler } from '../licensed_route_handler'; import { RouteDefinitionParams } from '..'; -export function defineCheckPrivilegesRoutes({ - router, - clusterClient, - authc, -}: RouteDefinitionParams) { +export function defineCheckPrivilegesRoutes({ router, authc }: RouteDefinitionParams) { router.get( { path: '/internal/security/api_key/privileges', @@ -20,19 +16,25 @@ export function defineCheckPrivilegesRoutes({ }, createLicensedRouteHandler(async (context, request, response) => { try { - const scopedClusterClient = clusterClient.asScoped(request); - const [ { - cluster: { - manage_security: manageSecurity, - manage_api_key: manageApiKey, - manage_own_api_key: manageOwnApiKey, + body: { + cluster: { + manage_security: manageSecurity, + manage_api_key: manageApiKey, + manage_own_api_key: manageOwnApiKey, + }, }, }, areApiKeysEnabled, ] = await Promise.all([ - scopedClusterClient.callAsCurrentUser('shield.hasPrivileges', { + context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges<{ + cluster: { + manage_security: boolean; + manage_api_key: boolean; + manage_own_api_key: boolean; + }; + }>({ body: { cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key'] }, }), authc.areAPIKeysEnabled(), diff --git a/x-pack/plugins/security/server/routes/authorization/privileges/get_builtin.ts b/x-pack/plugins/security/server/routes/authorization/privileges/get_builtin.ts index 08cd3ba487b0b5..39e6a4838d34d4 100644 --- a/x-pack/plugins/security/server/routes/authorization/privileges/get_builtin.ts +++ b/x-pack/plugins/security/server/routes/authorization/privileges/get_builtin.ts @@ -7,13 +7,13 @@ import { BuiltinESPrivileges } from '../../../../common/model'; import { RouteDefinitionParams } from '../..'; -export function defineGetBuiltinPrivilegesRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineGetBuiltinPrivilegesRoutes({ router }: RouteDefinitionParams) { router.get( { path: '/internal/security/esPrivileges/builtin', validate: false }, async (context, request, response) => { - const privileges: BuiltinESPrivileges = await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.getBuiltinPrivileges'); + const { + body: privileges, + } = await context.core.elasticsearch.client.asCurrentUser.security.getBuiltinPrivileges(); // Exclude the `none` privilege, as it doesn't make sense as an option within the Kibana UI privileges.cluster = privileges.cluster.filter((privilege) => privilege !== 'none'); diff --git a/x-pack/plugins/security/server/routes/authorization/roles/delete.test.ts b/x-pack/plugins/security/server/routes/authorization/roles/delete.test.ts index 9f5ec635f56cd1..5143b727fcb4ea 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/delete.test.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/delete.test.ts @@ -5,14 +5,11 @@ */ import Boom from '@hapi/boom'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../../src/core/server'; +import { kibanaResponseFactory } from '../../../../../../../src/core/server'; import { LicenseCheck } from '../../../../../licensing/server'; import { defineDeleteRolesRoutes } from './delete'; -import { - elasticsearchServiceMock, - httpServerMock, -} from '../../../../../../../src/core/server/mocks'; +import { coreMock, httpServerMock } from '../../../../../../../src/core/server/mocks'; import { routeDefinitionParamsMock } from '../../index.mock'; interface TestOptions { @@ -29,11 +26,15 @@ describe('DELETE role', () => { ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); if (apiResponse) { - mockScopedClusterClient.callAsCurrentUser.mockImplementation(apiResponse); + mockContext.core.elasticsearch.client.asCurrentUser.security.deleteRole.mockImplementation( + (async () => ({ body: await apiResponse() })) as any + ); } defineDeleteRolesRoutes(mockRouteDefinitionParams); @@ -46,22 +47,15 @@ describe('DELETE role', () => { params: { name }, headers, }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(asserts.statusCode); expect(response.payload).toEqual(asserts.result); if (apiResponse) { - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith( - 'shield.deleteRole', - { name } - ); - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.deleteRole + ).toHaveBeenCalledWith({ name }); } expect(mockContext.licensing.license.check).toHaveBeenCalledWith('security', 'basic'); }); diff --git a/x-pack/plugins/security/server/routes/authorization/roles/delete.ts b/x-pack/plugins/security/server/routes/authorization/roles/delete.ts index eb56143288747c..b877aaf6abd770 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/delete.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/delete.ts @@ -9,7 +9,7 @@ import { RouteDefinitionParams } from '../../index'; import { createLicensedRouteHandler } from '../../licensed_route_handler'; import { wrapIntoCustomErrorResponse } from '../../../errors'; -export function defineDeleteRolesRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineDeleteRolesRoutes({ router }: RouteDefinitionParams) { router.delete( { path: '/api/security/role/{name}', @@ -19,7 +19,7 @@ export function defineDeleteRolesRoutes({ router, clusterClient }: RouteDefiniti }, createLicensedRouteHandler(async (context, request, response) => { try { - await clusterClient.asScoped(request).callAsCurrentUser('shield.deleteRole', { + await context.core.elasticsearch.client.asCurrentUser.security.deleteRole({ name: request.params.name, }); 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 b25b13b9fc04aa..a6090ee405329b 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 @@ -4,14 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ import Boom from '@hapi/boom'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../../src/core/server'; +import { kibanaResponseFactory } from '../../../../../../../src/core/server'; import { LicenseCheck } from '../../../../../licensing/server'; import { defineGetRolesRoutes } from './get'; -import { - elasticsearchServiceMock, - httpServerMock, -} from '../../../../../../../src/core/server/mocks'; +import { coreMock, httpServerMock } from '../../../../../../../src/core/server/mocks'; import { routeDefinitionParamsMock } from '../../index.mock'; const application = 'kibana-.kibana'; @@ -32,11 +29,15 @@ describe('GET role', () => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); mockRouteDefinitionParams.authz.applicationName = application; + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); if (apiResponse) { - mockScopedClusterClient.callAsCurrentUser.mockImplementation(apiResponse); + mockContext.core.elasticsearch.client.asCurrentUser.security.getRole.mockImplementation( + (async () => ({ body: await apiResponse() })) as any + ); } defineGetRolesRoutes(mockRouteDefinitionParams); @@ -49,22 +50,17 @@ describe('GET role', () => { params: { name }, headers, }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(asserts.statusCode); expect(response.payload).toEqual(asserts.result); if (apiResponse) { - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith('shield.getRole', { - name, - }); - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.getRole + ).toHaveBeenCalledWith({ name }); } + expect(mockContext.licensing.license.check).toHaveBeenCalledWith('security', 'basic'); }); }; 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 bf1140e2e6fd10..ce4a622d30e614 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/get.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/get.ts @@ -8,9 +8,9 @@ import { schema } from '@kbn/config-schema'; import { RouteDefinitionParams } from '../..'; import { createLicensedRouteHandler } from '../../licensed_route_handler'; import { wrapIntoCustomErrorResponse } from '../../../errors'; -import { transformElasticsearchRoleToRole } from './model'; +import { ElasticsearchRole, transformElasticsearchRoleToRole } from './model'; -export function defineGetRolesRoutes({ router, authz, clusterClient }: RouteDefinitionParams) { +export function defineGetRolesRoutes({ router, authz }: RouteDefinitionParams) { router.get( { path: '/api/security/role/{name}', @@ -20,9 +20,11 @@ export function defineGetRolesRoutes({ router, authz, clusterClient }: RouteDefi }, createLicensedRouteHandler(async (context, request, response) => { try { - const elasticsearchRoles = await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.getRole', { name: request.params.name }); + const { + body: elasticsearchRoles, + } = await context.core.elasticsearch.client.asCurrentUser.security.getRole< + Record + >({ name: request.params.name }); const elasticsearchRole = elasticsearchRoles[request.params.name]; if (elasticsearchRole) { 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 30e0c52c4c443b..b3a855b2e0ae78 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 @@ -4,14 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ import Boom from '@hapi/boom'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../../src/core/server'; +import { kibanaResponseFactory } from '../../../../../../../src/core/server'; import { LicenseCheck } from '../../../../../licensing/server'; import { defineGetAllRolesRoutes } from './get_all'; -import { - elasticsearchServiceMock, - httpServerMock, -} from '../../../../../../../src/core/server/mocks'; +import { coreMock, httpServerMock } from '../../../../../../../src/core/server/mocks'; import { routeDefinitionParamsMock } from '../../index.mock'; const application = 'kibana-.kibana'; @@ -32,11 +29,15 @@ describe('GET all roles', () => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); mockRouteDefinitionParams.authz.applicationName = application; + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); if (apiResponse) { - mockScopedClusterClient.callAsCurrentUser.mockImplementation(apiResponse); + mockContext.core.elasticsearch.client.asCurrentUser.security.getRole.mockImplementation( + (async () => ({ body: await apiResponse() })) as any + ); } defineGetAllRolesRoutes(mockRouteDefinitionParams); @@ -48,19 +49,15 @@ describe('GET all roles', () => { path: '/api/security/role', headers, }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(asserts.statusCode); expect(response.payload).toEqual(asserts.result); if (apiResponse) { - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith('shield.getRole'); - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.getRole + ).toHaveBeenCalled(); } expect(mockContext.licensing.license.check).toHaveBeenCalledWith('security', 'basic'); }); 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 24be6c60e4b129..21521dd6dbae3c 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 @@ -9,14 +9,16 @@ import { createLicensedRouteHandler } from '../../licensed_route_handler'; import { wrapIntoCustomErrorResponse } from '../../../errors'; import { ElasticsearchRole, transformElasticsearchRoleToRole } from './model'; -export function defineGetAllRolesRoutes({ router, authz, clusterClient }: RouteDefinitionParams) { +export function defineGetAllRolesRoutes({ router, authz }: RouteDefinitionParams) { router.get( { path: '/api/security/role', validate: false }, createLicensedRouteHandler(async (context, request, response) => { try { - const elasticsearchRoles = (await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.getRole')) as Record; + const { + body: elasticsearchRoles, + } = await context.core.elasticsearch.client.asCurrentUser.security.getRole< + Record + >(); // Transform elasticsearch roles into Kibana roles and return in a list sorted by the role name. return response.ok({ 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 811ea080b43166..779e1a7fab177e 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 @@ -5,15 +5,12 @@ */ import { Type } from '@kbn/config-schema'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../../src/core/server'; +import { kibanaResponseFactory } from '../../../../../../../src/core/server'; import { LicenseCheck } from '../../../../../licensing/server'; import { GLOBAL_RESOURCE } from '../../../../common/constants'; import { definePutRolesRoutes } from './put'; -import { - elasticsearchServiceMock, - httpServerMock, -} from '../../../../../../../src/core/server/mocks'; +import { coreMock, httpServerMock } from '../../../../../../../src/core/server/mocks'; import { routeDefinitionParamsMock } from '../../index.mock'; import { KibanaFeature } from '../../../../../features/server'; import { securityFeatureUsageServiceMock } from '../../../feature_usage/index.mock'; @@ -47,35 +44,43 @@ const privilegeMap = { interface TestOptions { name: string; licenseCheckResult?: LicenseCheck; - apiResponses?: Array<() => Promise>; + apiResponses?: { + get: () => Promise; + put: () => Promise; + }; payload?: Record; asserts: { statusCode: number; result?: Record; - apiArguments?: unknown[][]; + apiArguments?: { get: unknown[]; put: unknown[] }; recordSubFeaturePrivilegeUsage?: boolean; }; } const putRoleTest = ( description: string, - { - name, - payload, - licenseCheckResult = { state: 'valid' }, - apiResponses = [], - asserts, - }: TestOptions + { name, payload, licenseCheckResult = { state: 'valid' }, apiResponses, asserts }: TestOptions ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); mockRouteDefinitionParams.authz.applicationName = application; mockRouteDefinitionParams.authz.privileges.get.mockReturnValue(privilegeMap); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - for (const apiResponse of apiResponses) { - mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(apiResponse); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; + + if (apiResponses?.get) { + mockContext.core.elasticsearch.client.asCurrentUser.security.getRole.mockImplementationOnce( + (async () => ({ body: await apiResponses?.get() })) as any + ); + } + + if (apiResponses?.put) { + mockContext.core.elasticsearch.client.asCurrentUser.security.putRole.mockImplementationOnce( + (async () => ({ body: await apiResponses?.put() })) as any + ); } mockRouteDefinitionParams.getFeatureUsageService.mockReturnValue( @@ -131,21 +136,20 @@ const putRoleTest = ( body: payload !== undefined ? (validate as any).body.validate(payload) : undefined, headers, }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(asserts.statusCode); expect(response.payload).toEqual(asserts.result); - if (Array.isArray(asserts.apiArguments)) { - for (const apiArguments of asserts.apiArguments) { - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith(...apiArguments); - } - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); + if (asserts.apiArguments?.get) { + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.getRole + ).toHaveBeenCalledWith(...asserts.apiArguments?.get); + } + if (asserts.apiArguments?.put) { + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.putRole + ).toHaveBeenCalledWith(...asserts.apiArguments?.put); } expect(mockContext.licensing.license.check).toHaveBeenCalledWith('security', 'basic'); @@ -208,12 +212,11 @@ describe('PUT role', () => { putRoleTest(`creates empty role`, { name: 'foo-role', payload: {}, - apiResponses: [async () => ({}), async () => {}], + apiResponses: { get: async () => ({}), put: async () => {} }, asserts: { - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -224,7 +227,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -239,12 +242,11 @@ describe('PUT role', () => { }, ], }, - apiResponses: [async () => ({}), async () => {}], + apiResponses: { get: async () => ({}), put: async () => {} }, asserts: { - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -261,7 +263,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -279,12 +281,11 @@ describe('PUT role', () => { }, ], }, - apiResponses: [async () => ({}), async () => {}], + apiResponses: { get: async () => ({}), put: async () => {} }, asserts: { - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -301,7 +302,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -317,12 +318,11 @@ describe('PUT role', () => { }, ], }, - apiResponses: [async () => ({}), async () => {}], + apiResponses: { get: async () => ({}), put: async () => {} }, asserts: { - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -339,7 +339,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -383,12 +383,11 @@ describe('PUT role', () => { }, ], }, - apiResponses: [async () => ({}), async () => {}], + apiResponses: { get: async () => ({}), put: async () => {} }, asserts: { - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -426,7 +425,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -473,8 +472,8 @@ describe('PUT role', () => { }, ], }, - apiResponses: [ - async () => ({ + apiResponses: { + get: async () => ({ 'foo-role': { metadata: { bar: 'old-metadata', @@ -504,13 +503,12 @@ describe('PUT role', () => { ], }, }), - async () => {}, - ], + put: async () => {}, + }, asserts: { - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -548,7 +546,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -577,8 +575,8 @@ describe('PUT role', () => { }, ], }, - apiResponses: [ - async () => ({ + apiResponses: { + get: async () => ({ 'foo-role': { metadata: { bar: 'old-metadata', @@ -613,13 +611,12 @@ describe('PUT role', () => { ], }, }), - async () => {}, - ], + put: async () => {}, + }, asserts: { - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -652,7 +649,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -670,13 +667,12 @@ describe('PUT role', () => { }, ], }, - apiResponses: [async () => ({}), async () => {}], + apiResponses: { get: async () => ({}), put: async () => {} }, asserts: { recordSubFeaturePrivilegeUsage: true, - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -694,7 +690,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -712,13 +708,12 @@ describe('PUT role', () => { }, ], }, - apiResponses: [async () => ({}), async () => {}], + apiResponses: { get: async () => ({}), put: async () => {} }, asserts: { recordSubFeaturePrivilegeUsage: false, - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -736,7 +731,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, @@ -754,13 +749,12 @@ describe('PUT role', () => { }, ], }, - apiResponses: [async () => ({}), async () => {}], + apiResponses: { get: async () => ({}), put: async () => {} }, asserts: { recordSubFeaturePrivilegeUsage: false, - apiArguments: [ - ['shield.getRole', { name: 'foo-role', ignore: [404] }], - [ - 'shield.putRole', + apiArguments: { + get: [{ name: 'foo-role' }, { ignore: [404] }], + put: [ { name: 'foo-role', body: { @@ -778,7 +772,7 @@ describe('PUT role', () => { }, }, ], - ], + }, statusCode: 204, result: undefined, }, 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 cdedc9ac8a5eb2..26c61b4ced15f3 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,6 @@ const roleGrantsSubFeaturePrivileges = ( export function definePutRolesRoutes({ router, authz, - clusterClient, getFeatures, getFeatureUsageService, }: RouteDefinitionParams) { @@ -64,12 +63,11 @@ export function definePutRolesRoutes({ const { name } = request.params; try { - const rawRoles: Record = await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.getRole', { - name: request.params.name, - ignore: [404], - }); + const { + body: rawRoles, + } = await context.core.elasticsearch.client.asCurrentUser.security.getRole< + Record + >({ name: request.params.name }, { ignore: [404] }); const body = transformPutPayloadToElasticsearchRole( request.body, @@ -77,11 +75,12 @@ export function definePutRolesRoutes({ rawRoles[name] ? rawRoles[name].applications : [] ); - const [features] = await Promise.all([ + const [features] = await Promise.all([ getFeatures(), - clusterClient - .asScoped(request) - .callAsCurrentUser('shield.putRole', { name: request.params.name, body }), + context.core.elasticsearch.client.asCurrentUser.security.putRole({ + name: request.params.name, + body, + }), ]); if (roleGrantsSubFeaturePrivileges(features, request.body)) { diff --git a/x-pack/plugins/security/server/routes/index.mock.ts b/x-pack/plugins/security/server/routes/index.mock.ts index fab4a71df0cb05..1df499d9816323 100644 --- a/x-pack/plugins/security/server/routes/index.mock.ts +++ b/x-pack/plugins/security/server/routes/index.mock.ts @@ -5,7 +5,6 @@ */ import { - elasticsearchServiceMock, httpServiceMock, loggingSystemMock, httpResourcesMock, @@ -25,7 +24,6 @@ export const routeDefinitionParamsMock = { basePath: httpServiceMock.createBasePath(), csp: httpServiceMock.createSetupContract().csp, logger: loggingSystemMock.create().get(), - clusterClient: elasticsearchServiceMock.createLegacyClusterClient(), config: createConfig(ConfigSchema.validate(config), loggingSystemMock.create().get(), { isTLSEnabled: false, }), diff --git a/x-pack/plugins/security/server/routes/index.ts b/x-pack/plugins/security/server/routes/index.ts index 079c9e8ab9ce70..db71b04b3e6f01 100644 --- a/x-pack/plugins/security/server/routes/index.ts +++ b/x-pack/plugins/security/server/routes/index.ts @@ -5,13 +5,7 @@ */ import type { PublicMethodsOf } from '@kbn/utility-types'; import { KibanaFeature } from '../../../features/server'; -import { - HttpResources, - IBasePath, - ILegacyClusterClient, - IRouter, - Logger, -} from '../../../../../src/core/server'; +import { HttpResources, IBasePath, IRouter, Logger } from '../../../../../src/core/server'; import { SecurityLicense } from '../../common/licensing'; import { Authentication } from '../authentication'; import { AuthorizationServiceSetup } from '../authorization'; @@ -36,7 +30,6 @@ export interface RouteDefinitionParams { basePath: IBasePath; httpResources: HttpResources; logger: Logger; - clusterClient: ILegacyClusterClient; config: ConfigType; authc: Authentication; authz: AuthorizationServiceSetup; diff --git a/x-pack/plugins/security/server/routes/indices/get_fields.test.ts b/x-pack/plugins/security/server/routes/indices/get_fields.test.ts index 4c6182e99431d5..6d3de11249a169 100644 --- a/x-pack/plugins/security/server/routes/indices/get_fields.test.ts +++ b/x-pack/plugins/security/server/routes/indices/get_fields.test.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { httpServerMock, elasticsearchServiceMock } from '../../../../../../src/core/server/mocks'; +import { httpServerMock, coreMock } from '../../../../../../src/core/server/mocks'; import { kibanaResponseFactory } from '../../../../../../src/core/server'; import { routeDefinitionParamsMock } from '../index.mock'; @@ -36,10 +36,12 @@ const mockFieldMappingResponse = { describe('GET /internal/security/fields/{query}', () => { it('returns a list of deduplicated fields, omitting empty and runtime fields', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - - const scopedClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - scopedClient.callAsCurrentUser.mockResolvedValue(mockFieldMappingResponse); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(scopedClient); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + }; + mockContext.core.elasticsearch.client.asCurrentUser.indices.getFieldMapping.mockImplementation( + (async () => ({ body: mockFieldMappingResponse })) as any + ); defineGetFieldsRoutes(mockRouteDefinitionParams); @@ -51,7 +53,7 @@ describe('GET /internal/security/fields/{query}', () => { path: `/internal/security/fields/foo`, headers, }); - const response = await handler({} as any, mockRequest, kibanaResponseFactory); + const response = await handler(mockContext as any, mockRequest, kibanaResponseFactory); expect(response.status).toBe(200); expect(response.payload).toEqual(['fooField', 'commonField', 'barField']); }); diff --git a/x-pack/plugins/security/server/routes/indices/get_fields.ts b/x-pack/plugins/security/server/routes/indices/get_fields.ts index 44b8804ed8d6e1..304e121f7fee11 100644 --- a/x-pack/plugins/security/server/routes/indices/get_fields.ts +++ b/x-pack/plugins/security/server/routes/indices/get_fields.ts @@ -22,7 +22,7 @@ interface FieldMappingResponse { }; } -export function defineGetFieldsRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineGetFieldsRoutes({ router }: RouteDefinitionParams) { router.get( { path: '/internal/security/fields/{query}', @@ -30,14 +30,16 @@ export function defineGetFieldsRoutes({ router, clusterClient }: RouteDefinition }, async (context, request, response) => { try { - const indexMappings = (await clusterClient - .asScoped(request) - .callAsCurrentUser('indices.getFieldMapping', { + const { + body: indexMappings, + } = await context.core.elasticsearch.client.asCurrentUser.indices.getFieldMapping( + { index: request.params.query, fields: '*', - allowNoIndices: false, - includeDefaults: true, - })) as FieldMappingResponse; + allow_no_indices: false, + include_defaults: true, + } + ); // The flow is the following (see response format at https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-get-field-mapping.html): // 1. Iterate over all matched indices. diff --git a/x-pack/plugins/security/server/routes/role_mapping/delete.test.ts b/x-pack/plugins/security/server/routes/role_mapping/delete.test.ts index aec0310129f6ee..33fd66f9e929d4 100644 --- a/x-pack/plugins/security/server/routes/role_mapping/delete.test.ts +++ b/x-pack/plugins/security/server/routes/role_mapping/delete.test.ts @@ -5,24 +5,26 @@ */ import { routeDefinitionParamsMock } from '../index.mock'; -import { elasticsearchServiceMock, httpServerMock } from 'src/core/server/mocks'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../src/core/server'; +import { coreMock, httpServerMock } from 'src/core/server/mocks'; +import { kibanaResponseFactory } from '../../../../../../src/core/server'; import { defineRoleMappingDeleteRoutes } from './delete'; describe('DELETE role mappings', () => { it('allows a role mapping to be deleted', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue({ acknowledged: true }); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue({ state: 'valid' }) } } as any, + }; + mockContext.core.elasticsearch.client.asCurrentUser.security.deleteRoleMapping.mockResolvedValue( + { body: { acknowledged: true } } as any + ); defineRoleMappingDeleteRoutes(mockRouteDefinitionParams); const [[, handler]] = mockRouteDefinitionParams.router.delete.mock.calls; const name = 'mapping1'; - const headers = { authorization: 'foo' }; const mockRequest = httpServerMock.createKibanaRequest({ method: 'delete', @@ -30,31 +32,35 @@ describe('DELETE role mappings', () => { params: { name }, headers, }); - const mockContext = ({ - licensing: { - license: { check: jest.fn().mockReturnValue({ state: 'valid' }) }, - }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(200); expect(response.payload).toEqual({ acknowledged: true }); - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); expect( - mockScopedClusterClient.callAsCurrentUser - ).toHaveBeenCalledWith('shield.deleteRoleMapping', { name }); + mockContext.core.elasticsearch.client.asCurrentUser.security.deleteRoleMapping + ).toHaveBeenCalledWith({ name }); }); describe('failure', () => { it('returns result of license check', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { + license: { + check: jest.fn().mockReturnValue({ + state: 'invalid', + message: 'test forbidden message', + }), + }, + } as any, + }; defineRoleMappingDeleteRoutes(mockRouteDefinitionParams); const [[, handler]] = mockRouteDefinitionParams.router.delete.mock.calls; const name = 'mapping1'; - const headers = { authorization: 'foo' }; const mockRequest = httpServerMock.createKibanaRequest({ method: 'delete', @@ -62,21 +68,13 @@ describe('DELETE role mappings', () => { params: { name }, headers, }); - const mockContext = ({ - licensing: { - license: { - check: jest.fn().mockReturnValue({ - state: 'invalid', - message: 'test forbidden message', - }), - }, - }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(403); expect(response.payload).toEqual({ message: 'test forbidden message' }); - expect(mockRouteDefinitionParams.clusterClient.asScoped).not.toHaveBeenCalled(); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.deleteRoleMapping + ).not.toHaveBeenCalled(); }); }); }); diff --git a/x-pack/plugins/security/server/routes/role_mapping/delete.ts b/x-pack/plugins/security/server/routes/role_mapping/delete.ts index dc11bcd914b35b..dbe9c5662a1f10 100644 --- a/x-pack/plugins/security/server/routes/role_mapping/delete.ts +++ b/x-pack/plugins/security/server/routes/role_mapping/delete.ts @@ -8,9 +8,7 @@ import { createLicensedRouteHandler } from '../licensed_route_handler'; import { wrapError } from '../../errors'; import { RouteDefinitionParams } from '..'; -export function defineRoleMappingDeleteRoutes(params: RouteDefinitionParams) { - const { clusterClient, router } = params; - +export function defineRoleMappingDeleteRoutes({ router }: RouteDefinitionParams) { router.delete( { path: '/internal/security/role_mapping/{name}', @@ -22,11 +20,11 @@ export function defineRoleMappingDeleteRoutes(params: RouteDefinitionParams) { }, createLicensedRouteHandler(async (context, request, response) => { try { - const deleteResponse = await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.deleteRoleMapping', { - name: request.params.name, - }); + const { + body: deleteResponse, + } = await context.core.elasticsearch.client.asCurrentUser.security.deleteRoleMapping({ + name: request.params.name, + }); return response.ok({ body: deleteResponse }); } catch (error) { const wrappedError = wrapError(error); diff --git a/x-pack/plugins/security/server/routes/role_mapping/feature_check.test.ts b/x-pack/plugins/security/server/routes/role_mapping/feature_check.test.ts index ee1d550bbe24d5..8bd9f095b0f68a 100644 --- a/x-pack/plugins/security/server/routes/role_mapping/feature_check.test.ts +++ b/x-pack/plugins/security/server/routes/role_mapping/feature_check.test.ts @@ -5,21 +5,16 @@ */ import { routeDefinitionParamsMock } from '../index.mock'; -import { elasticsearchServiceMock, httpServerMock } from 'src/core/server/mocks'; -import { - kibanaResponseFactory, - RequestHandlerContext, - ILegacyClusterClient, -} from '../../../../../../src/core/server'; +import { coreMock, httpServerMock } from 'src/core/server/mocks'; +import { kibanaResponseFactory } from '../../../../../../src/core/server'; import { LicenseCheck } from '../../../../licensing/server'; import { defineRoleMappingFeatureCheckRoute } from './feature_check'; interface TestOptions { licenseCheckResult?: LicenseCheck; canManageRoleMappings?: boolean; - nodeSettingsResponse?: Record; - xpackUsageResponse?: Record; - internalUserClusterClientImpl?: ILegacyClusterClient['callAsInternalUser']; + nodeSettingsResponse?: () => Record; + xpackUsageResponse?: () => Record; asserts: { statusCode: number; result?: Record }; } @@ -38,57 +33,34 @@ const defaultXpackUsageResponse = { }, }; -const getDefaultInternalUserClusterClientImpl = ( - nodeSettingsResponse: TestOptions['nodeSettingsResponse'], - xpackUsageResponse: TestOptions['xpackUsageResponse'] -) => - ((async (endpoint: string, clientParams: Record) => { - if (!clientParams) throw new TypeError('expected clientParams'); - - if (endpoint === 'nodes.info') { - return nodeSettingsResponse; - } - - if (endpoint === 'transport.request') { - if (clientParams.path === '/_xpack/usage') { - return xpackUsageResponse; - } - } - - throw new Error(`unexpected endpoint: ${endpoint}`); - }) as unknown) as TestOptions['internalUserClusterClientImpl']; - describe('GET role mappings feature check', () => { const getFeatureCheckTest = ( description: string, { licenseCheckResult = { state: 'valid' }, canManageRoleMappings = true, - nodeSettingsResponse = {}, - xpackUsageResponse = defaultXpackUsageResponse, - internalUserClusterClientImpl = getDefaultInternalUserClusterClientImpl( - nodeSettingsResponse, - xpackUsageResponse - ), + nodeSettingsResponse = async () => ({}), + xpackUsageResponse = async () => defaultXpackUsageResponse, asserts, }: TestOptions ) => { test(description, async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - mockRouteDefinitionParams.clusterClient.callAsInternalUser.mockImplementation( - internalUserClusterClientImpl + mockContext.core.elasticsearch.client.asInternalUser.nodes.info.mockImplementation( + (async () => ({ body: await nodeSettingsResponse() })) as any + ); + mockContext.core.elasticsearch.client.asInternalUser.transport.request.mockImplementation( + (async () => ({ body: await xpackUsageResponse() })) as any ); - mockScopedClusterClient.callAsCurrentUser.mockImplementation(async (method, payload) => { - if (method === 'shield.hasPrivileges') { - return { - has_all_requested: canManageRoleMappings, - }; - } - }); + mockContext.core.elasticsearch.client.asCurrentUser.security.hasPrivileges.mockResolvedValue({ + body: { has_all_requested: canManageRoleMappings }, + } as any); defineRoleMappingFeatureCheckRoute(mockRouteDefinitionParams); const [[, handler]] = mockRouteDefinitionParams.router.get.mock.calls; @@ -99,9 +71,6 @@ describe('GET role mappings feature check', () => { path: `/internal/security/_check_role_mapping_features`, headers, }); - const mockContext = ({ - licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(asserts.statusCode); @@ -124,7 +93,7 @@ describe('GET role mappings feature check', () => { }); getFeatureCheckTest('allows both script types when explicitly enabled', { - nodeSettingsResponse: { + nodeSettingsResponse: async () => ({ nodes: { someNodeId: { settings: { @@ -134,7 +103,7 @@ describe('GET role mappings feature check', () => { }, }, }, - }, + }), asserts: { statusCode: 200, result: { @@ -147,7 +116,7 @@ describe('GET role mappings feature check', () => { }); getFeatureCheckTest('disallows stored scripts when disabled', { - nodeSettingsResponse: { + nodeSettingsResponse: async () => ({ nodes: { someNodeId: { settings: { @@ -157,7 +126,7 @@ describe('GET role mappings feature check', () => { }, }, }, - }, + }), asserts: { statusCode: 200, result: { @@ -170,7 +139,7 @@ describe('GET role mappings feature check', () => { }); getFeatureCheckTest('disallows inline scripts when disabled', { - nodeSettingsResponse: { + nodeSettingsResponse: async () => ({ nodes: { someNodeId: { settings: { @@ -180,7 +149,7 @@ describe('GET role mappings feature check', () => { }, }, }, - }, + }), asserts: { statusCode: 200, result: { @@ -193,7 +162,7 @@ describe('GET role mappings feature check', () => { }); getFeatureCheckTest('indicates incompatible realms when only native and file are enabled', { - xpackUsageResponse: { + xpackUsageResponse: async () => ({ security: { realms: { native: { @@ -206,7 +175,7 @@ describe('GET role mappings feature check', () => { }, }, }, - }, + }), asserts: { statusCode: 200, result: { @@ -231,9 +200,12 @@ describe('GET role mappings feature check', () => { getFeatureCheckTest( 'falls back to allowing both script types if there is an error retrieving node settings', { - internalUserClusterClientImpl: (() => { - return Promise.reject(new Error('something bad happened')); - }) as TestOptions['internalUserClusterClientImpl'], + nodeSettingsResponse: async () => { + throw new Error('something bad happened'); + }, + xpackUsageResponse: async () => { + throw new Error('something bad happened'); + }, asserts: { statusCode: 200, result: { diff --git a/x-pack/plugins/security/server/routes/role_mapping/feature_check.ts b/x-pack/plugins/security/server/routes/role_mapping/feature_check.ts index 88c7f193cea34b..470039b8ae92b2 100644 --- a/x-pack/plugins/security/server/routes/role_mapping/feature_check.ts +++ b/x-pack/plugins/security/server/routes/role_mapping/feature_check.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Logger, ILegacyClusterClient } from 'src/core/server'; +import { ElasticsearchClient, Logger } from 'src/core/server'; import { createLicensedRouteHandler } from '../licensed_route_handler'; import { RouteDefinitionParams } from '..'; @@ -34,24 +34,18 @@ interface XPackUsageResponse { const INCOMPATIBLE_REALMS = ['file', 'native']; -export function defineRoleMappingFeatureCheckRoute({ - router, - clusterClient, - logger, -}: RouteDefinitionParams) { +export function defineRoleMappingFeatureCheckRoute({ router, logger }: RouteDefinitionParams) { router.get( { path: '/internal/security/_check_role_mapping_features', validate: false, }, createLicensedRouteHandler(async (context, request, response) => { - const { has_all_requested: canManageRoleMappings } = await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.hasPrivileges', { - body: { - cluster: ['manage_security'], - }, - }); + const { + body: { has_all_requested: canManageRoleMappings }, + } = await context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges<{ + has_all_requested: boolean; + }>({ body: { cluster: ['manage_security'] } }); if (!canManageRoleMappings) { return response.ok({ @@ -61,7 +55,10 @@ export function defineRoleMappingFeatureCheckRoute({ }); } - const enabledFeatures = await getEnabledRoleMappingsFeatures(clusterClient, logger); + const enabledFeatures = await getEnabledRoleMappingsFeatures( + context.core.elasticsearch.client.asInternalUser, + logger + ); return response.ok({ body: { @@ -73,13 +70,12 @@ export function defineRoleMappingFeatureCheckRoute({ ); } -async function getEnabledRoleMappingsFeatures(clusterClient: ILegacyClusterClient, logger: Logger) { +async function getEnabledRoleMappingsFeatures(esClient: ElasticsearchClient, logger: Logger) { logger.debug(`Retrieving role mappings features`); - const nodeScriptSettingsPromise: Promise = clusterClient - .callAsInternalUser('nodes.info', { - filterPath: 'nodes.*.settings.script', - }) + const nodeScriptSettingsPromise = esClient.nodes + .info({ filter_path: 'nodes.*.settings.script' }) + .then(({ body }) => body) .catch((error) => { // fall back to assuming that node settings are unset/at their default values. // this will allow the role mappings UI to permit both role template script types, @@ -88,13 +84,11 @@ async function getEnabledRoleMappingsFeatures(clusterClient: ILegacyClusterClien return {}; }); - const xpackUsagePromise: Promise = clusterClient - // `transport.request` is potentially unsafe when combined with untrusted user input. - // Do not augment with such input. - .callAsInternalUser('transport.request', { - method: 'GET', - path: '/_xpack/usage', - }) + // `transport.request` is potentially unsafe when combined with untrusted user input. + // Do not augment with such input. + const xpackUsagePromise = esClient.transport + .request({ method: 'GET', path: '/_xpack/usage' }) + .then(({ body }) => body as XPackUsageResponse) .catch((error) => { // fall back to no external realms configured. // this will cause a warning in the UI about no compatible realms being enabled, but will otherwise allow diff --git a/x-pack/plugins/security/server/routes/role_mapping/get.test.ts b/x-pack/plugins/security/server/routes/role_mapping/get.test.ts index 2519034b386bfa..625aae42a3907b 100644 --- a/x-pack/plugins/security/server/routes/role_mapping/get.test.ts +++ b/x-pack/plugins/security/server/routes/role_mapping/get.test.ts @@ -6,9 +6,9 @@ import Boom from '@hapi/boom'; import { routeDefinitionParamsMock } from '../index.mock'; -import { elasticsearchServiceMock, httpServerMock } from 'src/core/server/mocks'; +import { coreMock, httpServerMock } from 'src/core/server/mocks'; import { defineRoleMappingGetRoutes } from './get'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../src/core/server'; +import { kibanaResponseFactory } from '../../../../../../src/core/server'; const mockRoleMappingResponse = { mapping1: { @@ -49,13 +49,22 @@ const mockRoleMappingResponse = { }, }; +function getMockContext( + licenseCheckResult: { state: string; message?: string } = { state: 'valid' } +) { + return { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue(licenseCheckResult) } } as any, + }; +} + describe('GET role mappings', () => { it('returns all role mappings', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(mockRoleMappingResponse); + const mockContext = getMockContext(); + mockContext.core.elasticsearch.client.asCurrentUser.security.getRoleMapping.mockResolvedValue({ + body: mockRoleMappingResponse, + } as any); defineRoleMappingGetRoutes(mockRouteDefinitionParams); @@ -67,11 +76,6 @@ describe('GET role mappings', () => { path: `/internal/security/role_mapping`, headers, }); - const mockContext = ({ - licensing: { - license: { check: jest.fn().mockReturnValue({ state: 'valid' }) }, - }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(200); @@ -118,29 +122,27 @@ describe('GET role mappings', () => { }, ]); - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith( - 'shield.getRoleMappings', - { name: undefined } - ); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.getRoleMapping + ).toHaveBeenCalledWith({ name: undefined }); }); it('returns role mapping by name', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue({ - mapping1: { - enabled: true, - roles: ['foo', 'bar'], - rules: { - field: { - dn: 'CN=bob,OU=example,O=com', + const mockContext = getMockContext(); + mockContext.core.elasticsearch.client.asCurrentUser.security.getRoleMapping.mockResolvedValue({ + body: { + mapping1: { + enabled: true, + roles: ['foo', 'bar'], + rules: { + field: { + dn: 'CN=bob,OU=example,O=com', + }, }, }, }, - }); + } as any); defineRoleMappingGetRoutes(mockRouteDefinitionParams); @@ -155,11 +157,6 @@ describe('GET role mappings', () => { params: { name }, headers, }); - const mockContext = ({ - licensing: { - license: { check: jest.fn().mockReturnValue({ state: 'valid' }) }, - }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(200); @@ -175,16 +172,15 @@ describe('GET role mappings', () => { }, }); - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith( - 'shield.getRoleMappings', - { name } - ); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.getRoleMapping + ).toHaveBeenCalledWith({ name }); }); describe('failure', () => { it('returns result of license check', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + const mockContext = getMockContext({ state: 'invalid', message: 'test forbidden message' }); defineRoleMappingGetRoutes(mockRouteDefinitionParams); @@ -196,29 +192,19 @@ describe('GET role mappings', () => { path: `/internal/security/role_mapping`, headers, }); - const mockContext = ({ - licensing: { - license: { - check: jest.fn().mockReturnValue({ - state: 'invalid', - message: 'test forbidden message', - }), - }, - }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(403); expect(response.payload).toEqual({ message: 'test forbidden message' }); - expect(mockRouteDefinitionParams.clusterClient.asScoped).not.toHaveBeenCalled(); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.getRoleMapping + ).not.toHaveBeenCalled(); }); it('returns a 404 when the role mapping is not found', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( + const mockContext = getMockContext(); + mockContext.core.elasticsearch.client.asCurrentUser.security.getRoleMapping.mockRejectedValue( Boom.notFound('role mapping not found!') ); @@ -235,18 +221,12 @@ describe('GET role mappings', () => { params: { name }, headers, }); - const mockContext = ({ - licensing: { - license: { check: jest.fn().mockReturnValue({ state: 'valid' }) }, - }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(404); - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); expect( - mockScopedClusterClient.callAsCurrentUser - ).toHaveBeenCalledWith('shield.getRoleMappings', { name }); + mockContext.core.elasticsearch.client.asCurrentUser.security.getRoleMapping + ).toHaveBeenCalledWith({ name }); }); }); }); diff --git a/x-pack/plugins/security/server/routes/role_mapping/get.ts b/x-pack/plugins/security/server/routes/role_mapping/get.ts index 63598584b5d1b5..5ab9b1f6b4a243 100644 --- a/x-pack/plugins/security/server/routes/role_mapping/get.ts +++ b/x-pack/plugins/security/server/routes/role_mapping/get.ts @@ -14,7 +14,7 @@ interface RoleMappingsResponse { } export function defineRoleMappingGetRoutes(params: RouteDefinitionParams) { - const { clusterClient, logger, router } = params; + const { logger, router } = params; router.get( { @@ -29,13 +29,11 @@ export function defineRoleMappingGetRoutes(params: RouteDefinitionParams) { const expectSingleEntity = typeof request.params.name === 'string'; try { - const roleMappingsResponse: RoleMappingsResponse = await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.getRoleMappings', { - name: request.params.name, - }); + const roleMappingsResponse = await context.core.elasticsearch.client.asCurrentUser.security.getRoleMapping( + { name: request.params.name } + ); - const mappings = Object.entries(roleMappingsResponse).map(([name, mapping]) => { + const mappings = Object.entries(roleMappingsResponse.body).map(([name, mapping]) => { return { name, ...mapping, diff --git a/x-pack/plugins/security/server/routes/role_mapping/post.test.ts b/x-pack/plugins/security/server/routes/role_mapping/post.test.ts index 8f61d2a122f0c1..5dc7a21a02c6f0 100644 --- a/x-pack/plugins/security/server/routes/role_mapping/post.test.ts +++ b/x-pack/plugins/security/server/routes/role_mapping/post.test.ts @@ -5,17 +5,20 @@ */ import { routeDefinitionParamsMock } from '../index.mock'; -import { elasticsearchServiceMock, httpServerMock } from 'src/core/server/mocks'; -import { kibanaResponseFactory, RequestHandlerContext } from '../../../../../../src/core/server'; +import { coreMock, httpServerMock } from 'src/core/server/mocks'; +import { kibanaResponseFactory } from '../../../../../../src/core/server'; import { defineRoleMappingPostRoutes } from './post'; describe('POST role mappings', () => { it('allows a role mapping to be created', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); - - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue({ created: true }); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue({ state: 'valid' }) } } as any, + }; + mockContext.core.elasticsearch.client.asCurrentUser.security.putRoleMapping.mockResolvedValue({ + body: { created: true }, + } as any); defineRoleMappingPostRoutes(mockRouteDefinitionParams); @@ -39,37 +42,41 @@ describe('POST role mappings', () => { }, headers, }); - const mockContext = ({ - licensing: { - license: { check: jest.fn().mockReturnValue({ state: 'valid' }) }, - }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(200); expect(response.payload).toEqual({ created: true }); - expect(mockRouteDefinitionParams.clusterClient.asScoped).toHaveBeenCalledWith(mockRequest); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith( - 'shield.saveRoleMapping', - { - name, - body: { - enabled: true, - roles: ['foo', 'bar'], - rules: { - field: { - dn: 'CN=bob,OU=example,O=com', - }, + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.putRoleMapping + ).toHaveBeenCalledWith({ + name, + body: { + enabled: true, + roles: ['foo', 'bar'], + rules: { + field: { + dn: 'CN=bob,OU=example,O=com', }, }, - } - ); + }, + }); }); describe('failure', () => { it('returns result of license check', async () => { const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + const mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { + license: { + check: jest.fn().mockReturnValue({ + state: 'invalid', + message: 'test forbidden message', + }), + }, + } as any, + }; defineRoleMappingPostRoutes(mockRouteDefinitionParams); @@ -81,22 +88,14 @@ describe('POST role mappings', () => { path: `/internal/security/role_mapping`, headers, }); - const mockContext = ({ - licensing: { - license: { - check: jest.fn().mockReturnValue({ - state: 'invalid', - message: 'test forbidden message', - }), - }, - }, - } as unknown) as RequestHandlerContext; const response = await handler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(403); expect(response.payload).toEqual({ message: 'test forbidden message' }); - expect(mockRouteDefinitionParams.clusterClient.asScoped).not.toHaveBeenCalled(); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.putRoleMapping + ).not.toHaveBeenCalled(); }); }); }); diff --git a/x-pack/plugins/security/server/routes/role_mapping/post.ts b/x-pack/plugins/security/server/routes/role_mapping/post.ts index 11149f38069a7f..6c1b19dacb6015 100644 --- a/x-pack/plugins/security/server/routes/role_mapping/post.ts +++ b/x-pack/plugins/security/server/routes/role_mapping/post.ts @@ -8,9 +8,7 @@ import { createLicensedRouteHandler } from '../licensed_route_handler'; import { wrapError } from '../../errors'; import { RouteDefinitionParams } from '..'; -export function defineRoleMappingPostRoutes(params: RouteDefinitionParams) { - const { clusterClient, router } = params; - +export function defineRoleMappingPostRoutes({ router }: RouteDefinitionParams) { router.post( { path: '/internal/security/role_mapping/{name}', @@ -43,13 +41,10 @@ export function defineRoleMappingPostRoutes(params: RouteDefinitionParams) { }, createLicensedRouteHandler(async (context, request, response) => { try { - const saveResponse = await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.saveRoleMapping', { - name: request.params.name, - body: request.body, - }); - return response.ok({ body: saveResponse }); + const saveResponse = await context.core.elasticsearch.client.asCurrentUser.security.putRoleMapping( + { name: request.params.name, body: request.body } + ); + return response.ok({ body: saveResponse.body }); } catch (error) { const wrappedError = wrapError(error); return response.customError({ diff --git a/x-pack/plugins/security/server/routes/users/change_password.test.ts b/x-pack/plugins/security/server/routes/users/change_password.test.ts index c66b5f985cb33f..d98c0acb7d86d0 100644 --- a/x-pack/plugins/security/server/routes/users/change_password.test.ts +++ b/x-pack/plugins/security/server/routes/users/change_password.test.ts @@ -7,21 +7,20 @@ import { errors } from 'elasticsearch'; import { ObjectType } from '@kbn/config-schema'; import type { PublicMethodsOf } from '@kbn/utility-types'; +import type { DeeplyMockedKeys } from '@kbn/utility-types/jest'; import { - ILegacyClusterClient, + Headers, IRouter, - ILegacyScopedClusterClient, kibanaResponseFactory, RequestHandler, RequestHandlerContext, RouteConfig, - ScopeableRequest, } from '../../../../../../src/core/server'; import { Authentication, AuthenticationResult } from '../../authentication'; import { Session } from '../../session_management'; import { defineChangeUserPasswordRoutes } from './change_password'; -import { elasticsearchServiceMock, httpServerMock } from '../../../../../../src/core/server/mocks'; +import { coreMock, httpServerMock } from '../../../../../../src/core/server/mocks'; import { mockAuthenticatedUser } from '../../../common/model/authenticated_user.mock'; import { sessionMock } from '../../session_management/session.mock'; import { routeDefinitionParamsMock } from '../index.mock'; @@ -30,19 +29,19 @@ describe('Change password', () => { let router: jest.Mocked; let authc: jest.Mocked; let session: jest.Mocked>; - let mockClusterClient: jest.Mocked; - let mockScopedClusterClient: jest.Mocked; let routeHandler: RequestHandler; let routeConfig: RouteConfig; - let mockContext: RequestHandlerContext; - - function checkPasswordChangeAPICall(username: string, request: ScopeableRequest) { - expect(mockClusterClient.asScoped).toHaveBeenCalledTimes(1); - expect(mockClusterClient.asScoped).toHaveBeenCalledWith(request); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledTimes(1); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith( - 'shield.changePassword', - { username, body: { password: 'new-password' } } + let mockContext: DeeplyMockedKeys; + + function checkPasswordChangeAPICall(username: string, headers?: Headers) { + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.changePassword + ).toHaveBeenCalledTimes(1); + expect( + mockContext.core.elasticsearch.client.asCurrentUser.security.changePassword + ).toHaveBeenCalledWith( + { username, body: { password: 'new-password' } }, + headers && { headers } ); } @@ -56,15 +55,10 @@ describe('Change password', () => { authc.login.mockResolvedValue(AuthenticationResult.succeeded(mockAuthenticatedUser())); session.get.mockResolvedValue(sessionMock.createValue()); - mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockClusterClient = routeParamsMock.clusterClient; - mockClusterClient.asScoped.mockReturnValue(mockScopedClusterClient); - - mockContext = ({ - licensing: { - license: { check: jest.fn().mockReturnValue({ check: 'valid' }) }, - }, - } as unknown) as RequestHandlerContext; + mockContext = { + core: coreMock.createRequestHandlerContext(), + licensing: { license: { check: jest.fn().mockReturnValue({ state: 'valid' }) } }, + } as any; defineChangeUserPasswordRoutes(routeParamsMock); @@ -114,20 +108,18 @@ describe('Change password', () => { const changePasswordFailure = new (errors.AuthenticationException as any)('Unauthorized', { body: { error: { header: { 'WWW-Authenticate': 'Negotiate' } } }, }); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(changePasswordFailure); + mockContext.core.elasticsearch.client.asCurrentUser.security.changePassword.mockRejectedValue( + changePasswordFailure + ); const response = await routeHandler(mockContext, mockRequest, kibanaResponseFactory); expect(response.status).toBe(403); expect(response.payload).toEqual(changePasswordFailure); - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledTimes(1); - expect(mockClusterClient.asScoped).toHaveBeenCalledTimes(1); - expect(mockClusterClient.asScoped).toHaveBeenCalledWith({ - headers: { - ...mockRequest.headers, - authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, - }, + checkPasswordChangeAPICall(username, { + ...mockRequest.headers, + authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, }); }); @@ -148,16 +140,16 @@ describe('Change password', () => { expect(response.payload).toEqual(loginFailureReason); checkPasswordChangeAPICall(username, { - headers: { - ...mockRequest.headers, - authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, - }, + ...mockRequest.headers, + authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, }); }); it('returns 500 if password update request fails with non-401 error.', async () => { const failureReason = new Error('Request failed.'); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(failureReason); + mockContext.core.elasticsearch.client.asCurrentUser.security.changePassword.mockRejectedValue( + failureReason + ); const response = await routeHandler(mockContext, mockRequest, kibanaResponseFactory); @@ -165,10 +157,8 @@ describe('Change password', () => { expect(response.payload).toEqual(failureReason); checkPasswordChangeAPICall(username, { - headers: { - ...mockRequest.headers, - authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, - }, + ...mockRequest.headers, + authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, }); }); @@ -179,10 +169,8 @@ describe('Change password', () => { expect(response.payload).toBeUndefined(); checkPasswordChangeAPICall(username, { - headers: { - ...mockRequest.headers, - authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, - }, + ...mockRequest.headers, + authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, }); expect(authc.login).toHaveBeenCalledTimes(1); @@ -209,10 +197,8 @@ describe('Change password', () => { expect(response.payload).toBeUndefined(); checkPasswordChangeAPICall(username, { - headers: { - ...mockRequest.headers, - authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, - }, + ...mockRequest.headers, + authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, }); expect(authc.login).toHaveBeenCalledTimes(1); @@ -230,10 +216,8 @@ describe('Change password', () => { expect(response.payload).toBeUndefined(); checkPasswordChangeAPICall(username, { - headers: { - ...mockRequest.headers, - authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, - }, + ...mockRequest.headers, + authorization: `Basic ${Buffer.from(`${username}:old-password`).toString('base64')}`, }); expect(authc.login).not.toHaveBeenCalled(); @@ -249,7 +233,9 @@ describe('Change password', () => { it('returns 500 if password update request fails.', async () => { const failureReason = new Error('Request failed.'); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(failureReason); + mockContext.core.elasticsearch.client.asCurrentUser.security.changePassword.mockRejectedValue( + failureReason + ); const response = await routeHandler(mockContext, mockRequest, kibanaResponseFactory); @@ -257,7 +243,7 @@ describe('Change password', () => { expect(response.payload).toEqual(failureReason); expect(authc.login).not.toHaveBeenCalled(); - checkPasswordChangeAPICall(username, mockRequest); + checkPasswordChangeAPICall(username); }); it('successfully changes user password.', async () => { @@ -267,7 +253,7 @@ describe('Change password', () => { expect(response.payload).toBeUndefined(); expect(authc.login).not.toHaveBeenCalled(); - checkPasswordChangeAPICall(username, mockRequest); + checkPasswordChangeAPICall(username); }); }); }); diff --git a/x-pack/plugins/security/server/routes/users/change_password.ts b/x-pack/plugins/security/server/routes/users/change_password.ts index be868f841eeebe..66d36b4294883e 100644 --- a/x-pack/plugins/security/server/routes/users/change_password.ts +++ b/x-pack/plugins/security/server/routes/users/change_password.ts @@ -14,12 +14,7 @@ import { } from '../../authentication'; import { RouteDefinitionParams } from '..'; -export function defineChangeUserPasswordRoutes({ - authc, - session, - router, - clusterClient, -}: RouteDefinitionParams) { +export function defineChangeUserPasswordRoutes({ authc, session, router }: RouteDefinitionParams) { router.post( { path: '/internal/security/users/{username}/password', @@ -43,28 +38,26 @@ export function defineChangeUserPasswordRoutes({ // If user is changing their own password they should provide a proof of knowledge their // current password via sending it in `Authorization: Basic base64(username:current password)` // HTTP header no matter how they logged in to Kibana. - const scopedClusterClient = clusterClient.asScoped( - isUserChangingOwnPassword - ? { - headers: { - ...request.headers, - authorization: new HTTPAuthorizationHeader( - 'Basic', - new BasicHTTPAuthorizationHeaderCredentials( - username, - currentPassword || '' - ).toString() - ).toString(), - }, - } - : request - ); + const options = isUserChangingOwnPassword + ? { + headers: { + ...request.headers, + authorization: new HTTPAuthorizationHeader( + 'Basic', + new BasicHTTPAuthorizationHeaderCredentials( + username, + currentPassword || '' + ).toString() + ).toString(), + }, + } + : undefined; try { - await scopedClusterClient.callAsCurrentUser('shield.changePassword', { - username, - body: { password: newPassword }, - }); + await context.core.elasticsearch.client.asCurrentUser.security.changePassword( + { username, body: { password: newPassword } }, + options + ); } catch (error) { // This may happen only if user's credentials are rejected meaning that current password // isn't correct. diff --git a/x-pack/plugins/security/server/routes/users/create_or_update.ts b/x-pack/plugins/security/server/routes/users/create_or_update.ts index 5a3e50bb11d5c2..a98848a583500e 100644 --- a/x-pack/plugins/security/server/routes/users/create_or_update.ts +++ b/x-pack/plugins/security/server/routes/users/create_or_update.ts @@ -9,7 +9,7 @@ import { wrapIntoCustomErrorResponse } from '../../errors'; import { createLicensedRouteHandler } from '../licensed_route_handler'; import { RouteDefinitionParams } from '..'; -export function defineCreateOrUpdateUserRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineCreateOrUpdateUserRoutes({ router }: RouteDefinitionParams) { router.post( { path: '/internal/security/users/{username}', @@ -28,7 +28,7 @@ export function defineCreateOrUpdateUserRoutes({ router, clusterClient }: RouteD }, createLicensedRouteHandler(async (context, request, response) => { try { - await clusterClient.asScoped(request).callAsCurrentUser('shield.putUser', { + await context.core.elasticsearch.client.asCurrentUser.security.putUser({ username: request.params.username, // Omit `username`, `enabled` and all fields with `null` value. body: Object.fromEntries( diff --git a/x-pack/plugins/security/server/routes/users/delete.ts b/x-pack/plugins/security/server/routes/users/delete.ts index 99a8d5c18ab3dd..26a1765b4fbdf9 100644 --- a/x-pack/plugins/security/server/routes/users/delete.ts +++ b/x-pack/plugins/security/server/routes/users/delete.ts @@ -9,7 +9,7 @@ import { RouteDefinitionParams } from '../index'; import { wrapIntoCustomErrorResponse } from '../../errors'; import { createLicensedRouteHandler } from '../licensed_route_handler'; -export function defineDeleteUserRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineDeleteUserRoutes({ router }: RouteDefinitionParams) { router.delete( { path: '/internal/security/users/{username}', @@ -19,9 +19,9 @@ export function defineDeleteUserRoutes({ router, clusterClient }: RouteDefinitio }, createLicensedRouteHandler(async (context, request, response) => { try { - await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.deleteUser', { username: request.params.username }); + await context.core.elasticsearch.client.asCurrentUser.security.deleteUser({ + username: request.params.username, + }); return response.noContent(); } catch (error) { diff --git a/x-pack/plugins/security/server/routes/users/get.ts b/x-pack/plugins/security/server/routes/users/get.ts index 08679103725465..aa6a4f6be8bad7 100644 --- a/x-pack/plugins/security/server/routes/users/get.ts +++ b/x-pack/plugins/security/server/routes/users/get.ts @@ -9,7 +9,7 @@ import { wrapIntoCustomErrorResponse } from '../../errors'; import { createLicensedRouteHandler } from '../licensed_route_handler'; import { RouteDefinitionParams } from '..'; -export function defineGetUserRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineGetUserRoutes({ router }: RouteDefinitionParams) { router.get( { path: '/internal/security/users/{username}', @@ -20,9 +20,13 @@ export function defineGetUserRoutes({ router, clusterClient }: RouteDefinitionPa createLicensedRouteHandler(async (context, request, response) => { try { const username = request.params.username; - const users = (await clusterClient - .asScoped(request) - .callAsCurrentUser('shield.getUser', { username })) as Record; + const { + body: users, + } = await context.core.elasticsearch.client.asCurrentUser.security.getUser< + Record + >({ + username, + }); if (!users[username]) { return response.notFound(); diff --git a/x-pack/plugins/security/server/routes/users/get_all.ts b/x-pack/plugins/security/server/routes/users/get_all.ts index 492ab27ab27adc..3c5ca184f747c2 100644 --- a/x-pack/plugins/security/server/routes/users/get_all.ts +++ b/x-pack/plugins/security/server/routes/users/get_all.ts @@ -8,7 +8,7 @@ import { RouteDefinitionParams } from '../index'; import { wrapIntoCustomErrorResponse } from '../../errors'; import { createLicensedRouteHandler } from '../licensed_route_handler'; -export function defineGetAllUsersRoutes({ router, clusterClient }: RouteDefinitionParams) { +export function defineGetAllUsersRoutes({ router }: RouteDefinitionParams) { router.get( { path: '/internal/security/users', validate: false }, createLicensedRouteHandler(async (context, request, response) => { @@ -16,7 +16,7 @@ export function defineGetAllUsersRoutes({ router, clusterClient }: RouteDefiniti return response.ok({ // Return only values since keys (user names) are already duplicated there. body: Object.values( - await clusterClient.asScoped(request).callAsCurrentUser('shield.getUser') + (await context.core.elasticsearch.client.asCurrentUser.security.getUser()).body ), }); } catch (error) {