From 89a92fd05e0a079ddfe6258f013dee2ebc1730ed Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 13 Dec 2023 18:46:11 -0700 Subject: [PATCH] Tests --- .../security/plugin_types_server/index.ts | 1 - .../src/authentication/api_keys/api_keys.ts | 14 -- .../src/authentication/api_keys/index.ts | 1 - .../src/authentication/index.ts | 1 - .../authentication/api_keys/api_keys.ts | 27 ---- .../authentication/authentication_service.ts | 2 - .../server/routes/api_keys/has.test.ts | 125 ++++++++++++++++++ .../security/server/routes/api_keys/has.ts | 65 +++++++++ .../security/server/routes/api_keys/index.ts | 2 + .../api_integration/apis/security/api_keys.ts | 14 ++ .../common/platform_security/api_keys.ts | 29 ++++ 11 files changed, 235 insertions(+), 46 deletions(-) create mode 100644 x-pack/plugins/security/server/routes/api_keys/has.test.ts create mode 100644 x-pack/plugins/security/server/routes/api_keys/has.ts diff --git a/x-pack/packages/security/plugin_types_server/index.ts b/x-pack/packages/security/plugin_types_server/index.ts index ad1a298d0898..2d697dd0187a 100644 --- a/x-pack/packages/security/plugin_types_server/index.ts +++ b/x-pack/packages/security/plugin_types_server/index.ts @@ -18,7 +18,6 @@ export type { CreateAPIKeyResult, CreateRestAPIKeyParams, GrantAPIKeyResult, - HasApiKeysOptions, InvalidateAPIKeysParams, ValidateAPIKeyParams, CreateRestAPIKeyWithKibanaPrivilegesParams, diff --git a/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/api_keys.ts b/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/api_keys.ts index 09efa8297788..1cbf13a4ad45 100644 --- a/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/api_keys.ts +++ b/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/api_keys.ts @@ -17,12 +17,6 @@ export interface APIKeys { */ areAPIKeysEnabled(): Promise; - /** - * Determines if the currently logged in user has created API keys - * @param apiKeyPrams ValidateAPIKeyParams. - */ - hasApiKeys(request: KibanaRequest, options: HasApiKeysOptions): Promise; - /** * Determines if Cross-Cluster API Keys are enabled in Elasticsearch. */ @@ -122,14 +116,6 @@ export interface ValidateAPIKeyParams { api_key: string; } -/** - * - */ -export interface HasApiKeysOptions { - ownerOnly: boolean; - validOnly: boolean; -} - /** * Represents the params for invalidating multiple API keys */ diff --git a/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/index.ts b/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/index.ts index 02af946dd1fb..dbad1344d1d2 100644 --- a/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/index.ts +++ b/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/index.ts @@ -8,7 +8,6 @@ export type { CreateAPIKeyParams, CreateAPIKeyResult, - HasApiKeysOptions, InvalidateAPIKeyResult, InvalidateAPIKeysParams, ValidateAPIKeyParams, diff --git a/x-pack/packages/security/plugin_types_server/src/authentication/index.ts b/x-pack/packages/security/plugin_types_server/src/authentication/index.ts index 4a9f7b888e92..04e4a820fb4d 100644 --- a/x-pack/packages/security/plugin_types_server/src/authentication/index.ts +++ b/x-pack/packages/security/plugin_types_server/src/authentication/index.ts @@ -11,7 +11,6 @@ export type { CreateRestAPIKeyParams, CreateRestAPIKeyWithKibanaPrivilegesParams, CreateCrossClusterAPIKeyParams, - HasApiKeysOptions, InvalidateAPIKeyResult, InvalidateAPIKeysParams, ValidateAPIKeyParams, diff --git a/x-pack/plugins/security/server/authentication/api_keys/api_keys.ts b/x-pack/plugins/security/server/authentication/api_keys/api_keys.ts index f56a447d76b4..36a3bfeee4f7 100644 --- a/x-pack/plugins/security/server/authentication/api_keys/api_keys.ts +++ b/x-pack/plugins/security/server/authentication/api_keys/api_keys.ts @@ -16,7 +16,6 @@ import type { CreateRestAPIKeyParams, CreateRestAPIKeyWithKibanaPrivilegesParams, GrantAPIKeyResult, - HasApiKeysOptions, InvalidateAPIKeyResult, InvalidateAPIKeysParams, ValidateAPIKeyParams, @@ -84,32 +83,6 @@ export class APIKeys implements APIKeysType { this.kibanaFeatures = kibanaFeatures; } - /** - * Determines if currently-logged-in user has created any API Keys. - * NOTE: The current user may not have privileges to call the requred Elasticsearch API. - */ - async hasApiKeys(request: KibanaRequest, options: HasApiKeysOptions): Promise { - const { ownerOnly, validOnly } = options; - - try { - const scopedClusterClient = this.clusterClient.asScoped(request); - const result = await scopedClusterClient.asCurrentUser.security.getApiKey({ - owner: ownerOnly, - }); - const { api_keys: apiKeys } = result; - - let countedKeys = apiKeys; - if (validOnly) { - countedKeys = countedKeys.filter((key) => !key.invalidated); - } - - return countedKeys.length > 0; - } catch (e) { - this.logger.error(`Failed to determine if user has created any API keys`); - return false; - } - } - /** * Determines if API Keys are enabled in Elasticsearch. */ diff --git a/x-pack/plugins/security/server/authentication/authentication_service.ts b/x-pack/plugins/security/server/authentication/authentication_service.ts index 9ff741480214..d6f955b8b455 100644 --- a/x-pack/plugins/security/server/authentication/authentication_service.ts +++ b/x-pack/plugins/security/server/authentication/authentication_service.ts @@ -69,7 +69,6 @@ export interface InternalAuthenticationServiceStart extends AuthenticationServic | 'areAPIKeysEnabled' | 'areCrossClusterAPIKeysEnabled' | 'create' - | 'hasApiKeys' | 'update' | 'invalidate' | 'validate' @@ -367,7 +366,6 @@ export class AuthenticationService { create: apiKeys.create.bind(apiKeys), update: apiKeys.update.bind(apiKeys), grantAsInternalUser: apiKeys.grantAsInternalUser.bind(apiKeys), - hasApiKeys: apiKeys.hasApiKeys.bind(apiKeys), invalidate: apiKeys.invalidate.bind(apiKeys), validate: apiKeys.validate.bind(apiKeys), invalidateAsInternalUser: apiKeys.invalidateAsInternalUser.bind(apiKeys), diff --git a/x-pack/plugins/security/server/routes/api_keys/has.test.ts b/x-pack/plugins/security/server/routes/api_keys/has.test.ts new file mode 100644 index 000000000000..7f944bcf5b88 --- /dev/null +++ b/x-pack/plugins/security/server/routes/api_keys/has.test.ts @@ -0,0 +1,125 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import Boom from '@hapi/boom'; + +import { kibanaResponseFactory } from '@kbn/core/server'; +import type { RequestHandler } from '@kbn/core/server'; +import type { CustomRequestHandlerMock, ScopedClusterClientMock } from '@kbn/core/server/mocks'; +import { coreMock, httpServerMock } from '@kbn/core/server/mocks'; +import { licensingMock } from '@kbn/licensing-plugin/server/mocks'; +import type { DeeplyMockedKeys } from '@kbn/utility-types-jest'; + +import { defineHasApiKeysRoutes } from './has'; +import type { InternalAuthenticationServiceStart } from '../../authentication'; +import { authenticationServiceMock } from '../../authentication/authentication_service.mock'; +import { routeDefinitionParamsMock } from '../index.mock'; + +describe('Has API Keys route', () => { + let routeHandler: RequestHandler; + let authc: DeeplyMockedKeys; + let esClientMock: ScopedClusterClientMock; + let mockContext: CustomRequestHandlerMock; + + beforeEach(async () => { + const mockRouteDefinitionParams = routeDefinitionParamsMock.create(); + authc = authenticationServiceMock.createStart(); + mockRouteDefinitionParams.getAuthenticationService.mockReturnValue(authc); + defineHasApiKeysRoutes(mockRouteDefinitionParams); + [[, routeHandler]] = mockRouteDefinitionParams.router.get.mock.calls; + mockContext = coreMock.createCustomRequestHandlerContext({ + core: coreMock.createRequestHandlerContext(), + licensing: licensingMock.createRequestHandlerContext(), + }); + + esClientMock = (await mockContext.core).elasticsearch.client; + + authc.apiKeys.areAPIKeysEnabled.mockResolvedValue(true); + authc.apiKeys.areCrossClusterAPIKeysEnabled.mockResolvedValue(true); + + esClientMock.asCurrentUser.security.getApiKey.mockResponse({ + api_keys: [ + { id: '123', invalidated: false }, + { id: '456', invalidated: true }, + ], + } as any); + }); + + it('should calculate when user has API keys', async () => { + const response = await routeHandler( + mockContext, + httpServerMock.createKibanaRequest(), + kibanaResponseFactory + ); + + expect(response.payload).toEqual( + expect.objectContaining({ + hasApiKeys: true, + }) + ); + }); + + it('should calculate when user does not have API keys', async () => { + esClientMock.asCurrentUser.security.getApiKey.mockResponse({ + api_keys: [], + }); + + const response = await routeHandler( + mockContext, + httpServerMock.createKibanaRequest(), + kibanaResponseFactory + ); + + expect(response.payload).toEqual( + expect.objectContaining({ + hasApiKeys: false, + }) + ); + }); + + it('should filter out invalidated API keys', async () => { + const response = await routeHandler( + mockContext, + httpServerMock.createKibanaRequest(), + kibanaResponseFactory + ); + + expect(response.status).toBe(200); + expect(response.payload?.hasApiKeys).toBe(true); + }); + + it('should return `404` if API keys are disabled', async () => { + authc.apiKeys.areAPIKeysEnabled.mockResolvedValue(false); + + const response = await routeHandler( + mockContext, + httpServerMock.createKibanaRequest(), + kibanaResponseFactory + ); + + expect(response.status).toBe(404); + expect(response.payload).toEqual({ + message: + "API keys are disabled in Elasticsearch. To use API keys enable 'xpack.security.authc.api_key.enabled' setting.", + }); + }); + + it('should forward error from Elasticsearch GET API keys endpoint', async () => { + const error = Boom.forbidden('test not acceptable message'); + esClientMock.asCurrentUser.security.getApiKey.mockResponseImplementation(() => { + throw error; + }); + + const response = await routeHandler( + mockContext, + httpServerMock.createKibanaRequest(), + kibanaResponseFactory + ); + + expect(response.status).toBe(403); + expect(response.payload).toEqual(error); + }); +}); diff --git a/x-pack/plugins/security/server/routes/api_keys/has.ts b/x-pack/plugins/security/server/routes/api_keys/has.ts new file mode 100644 index 000000000000..504fb7869f08 --- /dev/null +++ b/x-pack/plugins/security/server/routes/api_keys/has.ts @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { RouteDefinitionParams } from '..'; +import { wrapIntoCustomErrorResponse } from '../../errors'; +import { createLicensedRouteHandler } from '../licensed_route_handler'; + +/** + * Response of Kibana Has API keys endpoint. + */ +export interface HasAPIKeysResult { + hasApiKeys: boolean; +} + +export function defineHasApiKeysRoutes({ + router, + getAuthenticationService, +}: RouteDefinitionParams) { + router.get( + { + path: '/internal/security/has_api_keys', + validate: false, + options: { + access: 'internal', + }, + }, + createLicensedRouteHandler(async (context, _request, response) => { + try { + // copied logic from get.ts + const esClient = (await context.core).elasticsearch.client; + const authenticationService = getAuthenticationService(); + + const areApiKeysEnabled = await authenticationService.apiKeys.areAPIKeysEnabled(); + + if (!areApiKeysEnabled) { + return response.notFound({ + body: { + message: + "API keys are disabled in Elasticsearch. To use API keys enable 'xpack.security.authc.api_key.enabled' setting.", + }, + }); + } + + const apiResponse = await esClient.asCurrentUser.security.getApiKey({ + owner: true, + }); + + const validKeys = apiResponse.api_keys.filter(({ invalidated }) => !invalidated); + + // simply return true if the result array is non-empty + return response.ok({ + body: { + hasApiKeys: validKeys.length > 0, + }, + }); + } catch (error) { + return response.customError(wrapIntoCustomErrorResponse(error)); + } + }) + ); +} diff --git a/x-pack/plugins/security/server/routes/api_keys/index.ts b/x-pack/plugins/security/server/routes/api_keys/index.ts index 9855d94923c3..15be8f7d05c4 100644 --- a/x-pack/plugins/security/server/routes/api_keys/index.ts +++ b/x-pack/plugins/security/server/routes/api_keys/index.ts @@ -8,6 +8,7 @@ import { defineCreateApiKeyRoutes } from './create'; import { defineEnabledApiKeysRoutes } from './enabled'; import { defineGetApiKeysRoutes } from './get'; +import { defineHasApiKeysRoutes } from './has'; import { defineInvalidateApiKeysRoutes } from './invalidate'; import { defineUpdateApiKeyRoutes } from './update'; import type { RouteDefinitionParams } from '..'; @@ -24,6 +25,7 @@ export type { GetAPIKeysResult } from './get'; export function defineApiKeysRoutes(params: RouteDefinitionParams) { defineEnabledApiKeysRoutes(params); defineGetApiKeysRoutes(params); + defineHasApiKeysRoutes(params); defineCreateApiKeyRoutes(params); defineUpdateApiKeyRoutes(params); defineInvalidateApiKeysRoutes(params); diff --git a/x-pack/test/api_integration/apis/security/api_keys.ts b/x-pack/test/api_integration/apis/security/api_keys.ts index 84307f3a11d0..295a27c8f23d 100644 --- a/x-pack/test/api_integration/apis/security/api_keys.ts +++ b/x-pack/test/api_integration/apis/security/api_keys.ts @@ -137,5 +137,19 @@ export default function ({ getService }: FtrProviderContext) { }); }); }); + + describe('GET /internal/security/has_api_keys', () => { + it('should return false by default', async () => { + await supertest + .get('/internal/security/has_api_keys') + .set('kbn-xsrf', 'xxx') + .send() + .expect(200) + .then((response: Record) => { + const payload = response.body; + expect(payload).to.eql({ hasApiKeys: false }); + }); + }); + }); }); } diff --git a/x-pack/test_serverless/api_integration/test_suites/common/platform_security/api_keys.ts b/x-pack/test_serverless/api_integration/test_suites/common/platform_security/api_keys.ts index 256a0fcfe32a..ecfa9cb6e54f 100644 --- a/x-pack/test_serverless/api_integration/test_suites/common/platform_security/api_keys.ts +++ b/x-pack/test_serverless/api_integration/test_suites/common/platform_security/api_keys.ts @@ -163,6 +163,35 @@ export default function ({ getService }: FtrProviderContext) { expect(status).toBe(200); }); + it('has_api_keys', async () => { + let body: unknown; + let status: number; + + ({ body, status } = await supertest + .get('/internal/security/has_api_keys') + .set(svlCommonApi.getCommonRequestHeader())); + // expect a rejection because we're not using the internal header + expect(body).toEqual({ + statusCode: 400, + error: 'Bad Request', + message: expect.stringContaining( + 'method [get] exists but is not available with the current configuration' + ), + }); + expect(status).toBe(400); + + ({ body, status } = await supertest + .get('/internal/security/has_api_keys') + .set(svlCommonApi.getInternalRequestHeader())); + // expect success because we're using the internal header + expect(body).toEqual( + expect.objectContaining({ + apiKeys: expect.arrayContaining([expect.objectContaining({ id: roleMapping.id })]), + }) + ); + expect(status).toBe(200); + }); + it('invalidate', async () => { let body: unknown; let status: number;