From 4100b52431ebd6fe07cd76f5cde31a0b2733bc12 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Sun, 6 Feb 2022 19:04:37 +0200 Subject: [PATCH 01/13] Deprecate endpoints --- x-pack/plugins/cases/server/routes/api/cases/get_case.ts | 6 ++++++ .../cases/server/routes/api/comments/get_all_comment.ts | 8 +++++++- .../routes/api/user_actions/get_all_user_actions.ts | 8 +++++++- x-pack/plugins/cases/server/routes/api/utils.ts | 9 ++++++++- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts index f8b85313482f4..f0143b6b49c47 100644 --- a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts +++ b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts @@ -20,6 +20,9 @@ export function initGetCaseApi({ router, logger }: RouteDeps) { case_id: schema.string(), }), query: schema.object({ + /** + * @deprecated since version 8.1.0 + */ includeComments: schema.boolean({ defaultValue: true }), }), }, @@ -30,6 +33,9 @@ export function initGetCaseApi({ router, logger }: RouteDeps) { const id = request.params.case_id; return response.ok({ + headers: { + ...getWarningHeader('', 'Deprecated query parameter includeComments'), + }, body: await casesClient.cases.get({ id, includeComments: request.query.includeComments, diff --git a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts index d7f95d503c394..d54e76b2975e8 100644 --- a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts +++ b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts @@ -8,9 +8,12 @@ import { schema } from '@kbn/config-schema'; import { RouteDeps } from '../types'; -import { wrapError } from '../utils'; +import { wrapError, getWarningHeader } from '../utils'; import { CASE_COMMENTS_URL } from '../../../../common/constants'; +/** + * @deprecated since version 8.1.0 + */ export function initGetAllCommentsApi({ router, logger }: RouteDeps) { router.get( { @@ -26,6 +29,9 @@ export function initGetAllCommentsApi({ router, logger }: RouteDeps) { const client = await context.cases.getCasesClient(); return response.ok({ + headers: { + ...getWarningHeader(), + }, body: await client.attachments.getAll({ caseID: request.params.case_id, }), diff --git a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts index cf258c284568b..2d48a1f3b1bde 100644 --- a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts +++ b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts @@ -8,9 +8,12 @@ import { schema } from '@kbn/config-schema'; import { RouteDeps } from '../types'; -import { wrapError } from '../utils'; +import { getWarningHeader, wrapError } from '../utils'; import { CASE_USER_ACTIONS_URL } from '../../../../common/constants'; +/** + * @deprecated since version 8.1.0 + */ export function initGetAllCaseUserActionsApi({ router, logger }: RouteDeps) { router.get( { @@ -31,6 +34,9 @@ export function initGetAllCaseUserActionsApi({ router, logger }: RouteDeps) { const caseId = request.params.case_id; return response.ok({ + headers: { + ...getWarningHeader(), + }, body: await casesClient.userActions.getAll({ caseId }), }); } catch (error) { diff --git a/x-pack/plugins/cases/server/routes/api/utils.ts b/x-pack/plugins/cases/server/routes/api/utils.ts index a09fd4cc9c746..4548d9016cf0f 100644 --- a/x-pack/plugins/cases/server/routes/api/utils.ts +++ b/x-pack/plugins/cases/server/routes/api/utils.ts @@ -5,8 +5,8 @@ * 2.0. */ +import { IncomingHttpHeaders } from 'http'; import { Boom, boomify, isBoom } from '@hapi/boom'; - import { schema } from '@kbn/config-schema'; import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; import { CaseError, isCaseError, HTTPError, isHTTPError } from '../../common/error'; @@ -35,3 +35,10 @@ export function wrapError( } export const escapeHatch = schema.object({}, { unknowns: 'allow' }); + +export const getWarningHeader = ( + kibanaVersion: string, + msg: string | undefined = 'Deprecated endpoint' +): { warning: string } => ({ + warning: `299 Kibana-${kibanaVersion} "${msg}"`, +}); From 0bdefb601805381f8fb54e381f7ad10d24b6b243 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 7 Feb 2022 12:05:59 +0200 Subject: [PATCH 02/13] Pass kibana version --- x-pack/plugins/cases/server/client/client.ts | 9 +++++++++ x-pack/plugins/cases/server/client/factory.ts | 3 +++ x-pack/plugins/cases/server/client/types.ts | 3 ++- x-pack/plugins/cases/server/plugin.ts | 3 +++ x-pack/plugins/cases/server/routes/api/cases/get_case.ts | 7 +++++-- .../cases/server/routes/api/comments/get_all_comment.ts | 2 +- .../routes/api/user_actions/get_all_user_actions.ts | 2 +- x-pack/plugins/cases/server/routes/api/utils.ts | 1 - 8 files changed, 24 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/cases/server/client/client.ts b/x-pack/plugins/cases/server/client/client.ts index fb111f267ddac..9f2e7ff10b399 100644 --- a/x-pack/plugins/cases/server/client/client.ts +++ b/x-pack/plugins/cases/server/client/client.ts @@ -18,6 +18,7 @@ import { createMetricsSubClient, MetricsSubClient } from './metrics/client'; * Client wrapper that contains accessor methods for individual entities within the cases system. */ export class CasesClient { + private readonly _kibanaVersion: CasesClientArgs['kibanaVersion']; private readonly _casesClientInternal: CasesClientInternal; private readonly _cases: CasesSubClient; private readonly _attachments: AttachmentsSubClient; @@ -27,6 +28,7 @@ export class CasesClient { private readonly _metrics: MetricsSubClient; constructor(args: CasesClientArgs) { + this._kibanaVersion = args.kibanaVersion; this._casesClientInternal = createCasesClientInternal(args); this._cases = createCasesSubClient(args, this, this._casesClientInternal); this._attachments = createAttachmentsSubClient(args, this, this._casesClientInternal); @@ -36,6 +38,13 @@ export class CasesClient { this._metrics = createMetricsSubClient(args, this); } + /** + * Retrieves the current kibana version + */ + public get kibanaVersion() { + return this._kibanaVersion; + } + /** * Retrieves an interface for interacting with cases entities. */ diff --git a/x-pack/plugins/cases/server/client/factory.ts b/x-pack/plugins/cases/server/client/factory.ts index d657f1a3f4f48..958de25fbb34e 100644 --- a/x-pack/plugins/cases/server/client/factory.ts +++ b/x-pack/plugins/cases/server/client/factory.ts @@ -10,6 +10,7 @@ import { SavedObjectsServiceStart, Logger, ElasticsearchClient, + PluginInitializerContext, } from 'kibana/server'; import { SecurityPluginSetup, SecurityPluginStart } from '../../../security/server'; import { SAVED_OBJECT_TYPES } from '../../common/constants'; @@ -31,6 +32,7 @@ import { AuthorizationAuditLogger } from '../authorization'; import { CasesClient, createCasesClient } from '.'; interface CasesClientFactoryArgs { + kibanaVersion: PluginInitializerContext['env']['packageInfo']['version']; securityPluginSetup?: SecurityPluginSetup; securityPluginStart?: SecurityPluginStart; getSpace: GetSpaceFn; @@ -113,6 +115,7 @@ export class CasesClientFactory { lensEmbeddableFactory: this.options.lensEmbeddableFactory, authorization: auth, actionsClient: await this.options.actionsPluginStart.getActionsClientWithRequest(request), + kibanaVersion: this.options.kibanaVersion, }); } } diff --git a/x-pack/plugins/cases/server/client/types.ts b/x-pack/plugins/cases/server/client/types.ts index e3d7b8a541b9d..5412ccde57134 100644 --- a/x-pack/plugins/cases/server/client/types.ts +++ b/x-pack/plugins/cases/server/client/types.ts @@ -6,7 +6,7 @@ */ import type { PublicMethodsOf } from '@kbn/utility-types'; -import { SavedObjectsClientContract, Logger } from 'kibana/server'; +import { SavedObjectsClientContract, Logger, PluginInitializerContext } from 'kibana/server'; import { User } from '../../common/api'; import { Authorization } from '../authorization/authorization'; import { @@ -24,6 +24,7 @@ import { LensServerPluginSetup } from '../../../lens/server'; * Parameters for initializing a cases client */ export interface CasesClientArgs { + readonly kibanaVersion: PluginInitializerContext['env']['packageInfo']['version']; readonly caseConfigureService: CaseConfigureService; readonly caseService: CasesService; readonly connectorMappingsService: ConnectorMappingsService; diff --git a/x-pack/plugins/cases/server/plugin.ts b/x-pack/plugins/cases/server/plugin.ts index 8d2582d9c5857..2bf9c876e6f9c 100644 --- a/x-pack/plugins/cases/server/plugin.ts +++ b/x-pack/plugins/cases/server/plugin.ts @@ -59,11 +59,13 @@ export interface PluginStartContract { export class CasePlugin { private readonly log: Logger; + private readonly kibanaVersion: PluginInitializerContext['env']['packageInfo']['version']; private clientFactory: CasesClientFactory; private securityPluginSetup?: SecurityPluginSetup; private lensEmbeddableFactory?: LensServerPluginSetup['lensEmbeddableFactory']; constructor(private readonly initializerContext: PluginInitializerContext) { + this.kibanaVersion = initializerContext.env.packageInfo.version; this.log = this.initializerContext.logger.get(); this.clientFactory = new CasesClientFactory(this.log); } @@ -121,6 +123,7 @@ export class CasePlugin { */ // eslint-disable-next-line @typescript-eslint/no-non-null-assertion lensEmbeddableFactory: this.lensEmbeddableFactory!, + kibanaVersion: this.kibanaVersion, }); const client = core.elasticsearch.client; diff --git a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts index f0143b6b49c47..2b87636a491f6 100644 --- a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts +++ b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts @@ -8,7 +8,7 @@ import { schema } from '@kbn/config-schema'; import { RouteDeps } from '../types'; -import { wrapError } from '../utils'; +import { getWarningHeader, wrapError } from '../utils'; import { CASE_DETAILS_URL } from '../../../../common/constants'; export function initGetCaseApi({ router, logger }: RouteDeps) { @@ -34,7 +34,10 @@ export function initGetCaseApi({ router, logger }: RouteDeps) { return response.ok({ headers: { - ...getWarningHeader('', 'Deprecated query parameter includeComments'), + ...getWarningHeader( + casesClient.kibanaVersion, + 'Deprecated query parameter includeComments' + ), }, body: await casesClient.cases.get({ id, diff --git a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts index d54e76b2975e8..d154c400345e8 100644 --- a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts +++ b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts @@ -30,7 +30,7 @@ export function initGetAllCommentsApi({ router, logger }: RouteDeps) { return response.ok({ headers: { - ...getWarningHeader(), + ...getWarningHeader(client.kibanaVersion), }, body: await client.attachments.getAll({ caseID: request.params.case_id, diff --git a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts index 2d48a1f3b1bde..655d65430ddb9 100644 --- a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts +++ b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts @@ -35,7 +35,7 @@ export function initGetAllCaseUserActionsApi({ router, logger }: RouteDeps) { return response.ok({ headers: { - ...getWarningHeader(), + ...getWarningHeader(casesClient.kibanaVersion), }, body: await casesClient.userActions.getAll({ caseId }), }); diff --git a/x-pack/plugins/cases/server/routes/api/utils.ts b/x-pack/plugins/cases/server/routes/api/utils.ts index 4548d9016cf0f..5386518a50e92 100644 --- a/x-pack/plugins/cases/server/routes/api/utils.ts +++ b/x-pack/plugins/cases/server/routes/api/utils.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { IncomingHttpHeaders } from 'http'; import { Boom, boomify, isBoom } from '@hapi/boom'; import { schema } from '@kbn/config-schema'; import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; From 85d0e0c1b7916642ce97cbe925dc262262d4760b Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 7 Feb 2022 13:23:08 +0200 Subject: [PATCH 03/13] Add tests --- .../plugins/cases/server/routes/api/utils.ts | 5 ++++ .../cases_api_integration/common/lib/utils.ts | 25 +++++++++++++++++++ .../tests/common/cases/get_case.ts | 17 ++++++++++++- .../tests/common/comments/get_all_comments.ts | 16 ++++++++++++ .../user_actions/get_all_user_actions.ts | 16 ++++++++++++ 5 files changed, 78 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/cases/server/routes/api/utils.ts b/x-pack/plugins/cases/server/routes/api/utils.ts index 5386518a50e92..8a824b8679ffd 100644 --- a/x-pack/plugins/cases/server/routes/api/utils.ts +++ b/x-pack/plugins/cases/server/routes/api/utils.ts @@ -35,6 +35,11 @@ export function wrapError( export const escapeHatch = schema.object({}, { unknowns: 'allow' }); +/** + * Creates a warning header with a message formatted according to RFC7234. + * We follow the same formatting as Elasticsearch + * https://github.com/elastic/elasticsearch/blob/5baabff6670a8ed49297488ca8cac8ec12a2078d/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java#L55 + */ export const getWarningHeader = ( kibanaVersion: string, msg: string | undefined = 'Deprecated endpoint' diff --git a/x-pack/test/cases_api_integration/common/lib/utils.ts b/x-pack/test/cases_api_integration/common/lib/utils.ts index de27836a9559e..cda5d6dcbeaa1 100644 --- a/x-pack/test/cases_api_integration/common/lib/utils.ts +++ b/x-pack/test/cases_api_integration/common/lib/utils.ts @@ -1131,3 +1131,28 @@ export const getServiceNowSimulationServer = async (): Promise<{ return { server, url }; }; + +/** + * + * @param warningHeader + * @returns + */ +export const extractWarningValueFromWarningHeader = (warningHeader: string) => { + const firstQuote = warningHeader.indexOf('"'); + const lastQuote = warningHeader.length - 1; + const warningValue = warningHeader.substring(firstQuote + 1, lastQuote); + return warningValue; +}; + +/** + * Regular expression to test if a string matches the RFC7234 specification (without warn-date) for warning headers. This pattern assumes that the warn code + * is always 299. Further, this pattern assumes that the warn agent represents a version of Kibana. + * + * Example: 299 Kibana-8.2.0 "Deprecated endpoint" + */ +const WARNING_HEADER_REGEX = /299 Kibana-\d+.\d+.\d+ \".+\"/g; + +export const assertWarningHeader = (warningHeader: string) => { + const res = warningHeader.match(WARNING_HEADER_REGEX); + expect(res).not.to.be(null); +}; diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts index 8df044baf813e..aff1caff022fc 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts @@ -8,7 +8,7 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; -import { AttributesTypeUser } from '../../../../../../plugins/cases/common/api'; +import { AttributesTypeUser, getCaseDetailsUrl } from '../../../../../../plugins/cases/common/api'; import { CASES_URL } from '../../../../../../plugins/cases/common/constants'; import { defaultUser, @@ -24,6 +24,8 @@ import { createComment, removeServerGeneratedPropertiesFromCase, removeServerGeneratedPropertiesFromSavedObject, + assertWarningHeader, + extractWarningValueFromWarningHeader, } from '../../../../common/lib/utils'; import { secOnly, @@ -191,5 +193,18 @@ export default ({ getService }: FtrProviderContext): void => { }); }); }); + + describe('deprecations', () => { + it('should return a warning header', async () => { + const theCase = await createCase(supertest, postCaseReq); + const res = await supertest.get(getCaseDetailsUrl(theCase.id)).expect(200); + const warningHeader = res.header.warning; + + assertWarningHeader(warningHeader); + + const warningValue = extractWarningValueFromWarningHeader(warningHeader); + expect(warningValue).to.be('Deprecated query parameter includeComments'); + }); + }); }); }; diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/get_all_comments.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/get_all_comments.ts index e0ee2576ef610..0a5dd64bc0dfc 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/get_all_comments.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/get_all_comments.ts @@ -15,6 +15,8 @@ import { createComment, getAllComments, superUserSpace1Auth, + assertWarningHeader, + extractWarningValueFromWarningHeader, } from '../../../../common/lib/utils'; import { globalRead, @@ -27,6 +29,7 @@ import { secOnlyRead, superUser, } from '../../../../common/lib/authentication/users'; +import { getCaseCommentsUrl } from '../../../../../../plugins/cases/common/api'; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { @@ -148,5 +151,18 @@ export default ({ getService }: FtrProviderContext): void => { }); }); }); + + describe('deprecations', () => { + it('should return a warning header', async () => { + const theCase = await createCase(supertest, postCaseReq); + const res = await supertest.get(getCaseCommentsUrl(theCase.id)).expect(200); + const warningHeader = res.header.warning; + + assertWarningHeader(warningHeader); + + const warningValue = extractWarningValueFromWarningHeader(warningHeader); + expect(warningValue).to.be('Deprecated endpoint'); + }); + }); }); }; diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/get_all_user_actions.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/get_all_user_actions.ts index 17863b282bf05..bf45e7e384217 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/get_all_user_actions.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/get_all_user_actions.ts @@ -13,6 +13,7 @@ import { CaseStatuses, CommentType, ConnectorTypes, + getCaseUserActionUrl, } from '../../../../../../plugins/cases/common/api'; import { CreateCaseUserAction } from '../../../../../../plugins/cases/common/api/cases/user_actions/create_case'; import { postCaseReq, postCommentUserReq, getPostCaseRequest } from '../../../../common/lib/mock'; @@ -26,6 +27,8 @@ import { createComment, updateComment, deleteComment, + assertWarningHeader, + extractWarningValueFromWarningHeader, } from '../../../../common/lib/utils'; import { globalRead, @@ -354,5 +357,18 @@ export default ({ getService }: FtrProviderContext): void => { }); } }); + + describe('deprecations', () => { + it('should return a warning header', async () => { + const theCase = await createCase(supertest, postCaseReq); + const res = await supertest.get(getCaseUserActionUrl(theCase.id)).expect(200); + const warningHeader = res.header.warning; + + assertWarningHeader(warningHeader); + + const warningValue = extractWarningValueFromWarningHeader(warningHeader); + expect(warningValue).to.be('Deprecated endpoint'); + }); + }); }); }; From 15bb9d9b2ac39376ba3d605fc873afeaf145636a Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 7 Feb 2022 13:25:44 +0200 Subject: [PATCH 04/13] Fix types --- x-pack/plugins/cases/server/client/mocks.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/cases/server/client/mocks.ts b/x-pack/plugins/cases/server/client/mocks.ts index 1d0cf000018cb..236082be2ef60 100644 --- a/x-pack/plugins/cases/server/client/mocks.ts +++ b/x-pack/plugins/cases/server/client/mocks.ts @@ -91,6 +91,7 @@ export interface CasesClientMock extends CasesClient { export const createCasesClientMock = (): CasesClientMock => { const client: PublicContract = { + kibanaVersion: '8.2.0', cases: createCasesSubClientMock(), attachments: createAttachmentsSubClientMock(), userActions: createUserActionsSubClientMock(), From d4d8fe2c0cd274960ddaf97c2747d85f2a4f1f9a Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 7 Feb 2022 17:48:58 +0200 Subject: [PATCH 05/13] Move kibana version to route deps --- x-pack/plugins/cases/server/client/client.ts | 9 --------- x-pack/plugins/cases/server/client/factory.ts | 3 --- x-pack/plugins/cases/server/client/mocks.ts | 1 - x-pack/plugins/cases/server/client/types.ts | 3 +-- x-pack/plugins/cases/server/plugin.ts | 2 +- x-pack/plugins/cases/server/routes/api/cases/get_case.ts | 7 ++----- .../cases/server/routes/api/comments/get_all_comment.ts | 4 ++-- x-pack/plugins/cases/server/routes/api/types.ts | 3 ++- .../routes/api/user_actions/get_all_user_actions.ts | 4 ++-- x-pack/test/cases_api_integration/common/lib/utils.ts | 4 ++-- 10 files changed, 12 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/cases/server/client/client.ts b/x-pack/plugins/cases/server/client/client.ts index 8b351be229379..266c988212cdf 100644 --- a/x-pack/plugins/cases/server/client/client.ts +++ b/x-pack/plugins/cases/server/client/client.ts @@ -17,7 +17,6 @@ import { createMetricsSubClient, MetricsSubClient } from './metrics/client'; * Client wrapper that contains accessor methods for individual entities within the cases system. */ export class CasesClient { - private readonly _kibanaVersion: CasesClientArgs['kibanaVersion']; private readonly _casesClientInternal: CasesClientInternal; private readonly _cases: CasesSubClient; private readonly _attachments: AttachmentsSubClient; @@ -26,7 +25,6 @@ export class CasesClient { private readonly _metrics: MetricsSubClient; constructor(args: CasesClientArgs) { - this._kibanaVersion = args.kibanaVersion; this._casesClientInternal = createCasesClientInternal(args); this._cases = createCasesSubClient(args, this, this._casesClientInternal); this._attachments = createAttachmentsSubClient(args, this, this._casesClientInternal); @@ -35,13 +33,6 @@ export class CasesClient { this._metrics = createMetricsSubClient(args, this); } - /** - * Retrieves the current kibana version - */ - public get kibanaVersion() { - return this._kibanaVersion; - } - /** * Retrieves an interface for interacting with cases entities. */ diff --git a/x-pack/plugins/cases/server/client/factory.ts b/x-pack/plugins/cases/server/client/factory.ts index b3d711949dfb9..3cbcee62d8c09 100644 --- a/x-pack/plugins/cases/server/client/factory.ts +++ b/x-pack/plugins/cases/server/client/factory.ts @@ -10,7 +10,6 @@ import { SavedObjectsServiceStart, Logger, ElasticsearchClient, - PluginInitializerContext, } from 'kibana/server'; import { SecurityPluginSetup, SecurityPluginStart } from '../../../security/server'; import { SAVED_OBJECT_TYPES } from '../../common/constants'; @@ -32,7 +31,6 @@ import { AuthorizationAuditLogger } from '../authorization'; import { CasesClient, createCasesClient } from '.'; interface CasesClientFactoryArgs { - kibanaVersion: PluginInitializerContext['env']['packageInfo']['version']; securityPluginSetup?: SecurityPluginSetup; securityPluginStart?: SecurityPluginStart; getSpace: GetSpaceFn; @@ -123,7 +121,6 @@ export class CasesClientFactory { lensEmbeddableFactory: this.options.lensEmbeddableFactory, authorization: auth, actionsClient: await this.options.actionsPluginStart.getActionsClientWithRequest(request), - kibanaVersion: this.options.kibanaVersion, }); } } diff --git a/x-pack/plugins/cases/server/client/mocks.ts b/x-pack/plugins/cases/server/client/mocks.ts index 0a9119fdff227..ecedc7cb05071 100644 --- a/x-pack/plugins/cases/server/client/mocks.ts +++ b/x-pack/plugins/cases/server/client/mocks.ts @@ -83,7 +83,6 @@ export interface CasesClientMock extends CasesClient { export const createCasesClientMock = (): CasesClientMock => { const client: PublicContract = { - kibanaVersion: '8.2.0', cases: createCasesSubClientMock(), attachments: createAttachmentsSubClientMock(), userActions: createUserActionsSubClientMock(), diff --git a/x-pack/plugins/cases/server/client/types.ts b/x-pack/plugins/cases/server/client/types.ts index 5412ccde57134..e3d7b8a541b9d 100644 --- a/x-pack/plugins/cases/server/client/types.ts +++ b/x-pack/plugins/cases/server/client/types.ts @@ -6,7 +6,7 @@ */ import type { PublicMethodsOf } from '@kbn/utility-types'; -import { SavedObjectsClientContract, Logger, PluginInitializerContext } from 'kibana/server'; +import { SavedObjectsClientContract, Logger } from 'kibana/server'; import { User } from '../../common/api'; import { Authorization } from '../authorization/authorization'; import { @@ -24,7 +24,6 @@ import { LensServerPluginSetup } from '../../../lens/server'; * Parameters for initializing a cases client */ export interface CasesClientArgs { - readonly kibanaVersion: PluginInitializerContext['env']['packageInfo']['version']; readonly caseConfigureService: CaseConfigureService; readonly caseService: CasesService; readonly connectorMappingsService: ConnectorMappingsService; diff --git a/x-pack/plugins/cases/server/plugin.ts b/x-pack/plugins/cases/server/plugin.ts index 2bf9c876e6f9c..5881b7b7633be 100644 --- a/x-pack/plugins/cases/server/plugin.ts +++ b/x-pack/plugins/cases/server/plugin.ts @@ -103,6 +103,7 @@ export class CasePlugin { initCaseApi({ logger: this.log, router, + kibanaVersion: this.kibanaVersion, }); } @@ -123,7 +124,6 @@ export class CasePlugin { */ // eslint-disable-next-line @typescript-eslint/no-non-null-assertion lensEmbeddableFactory: this.lensEmbeddableFactory!, - kibanaVersion: this.kibanaVersion, }); const client = core.elasticsearch.client; diff --git a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts index 2b87636a491f6..5e45b30f626f7 100644 --- a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts +++ b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts @@ -11,7 +11,7 @@ import { RouteDeps } from '../types'; import { getWarningHeader, wrapError } from '../utils'; import { CASE_DETAILS_URL } from '../../../../common/constants'; -export function initGetCaseApi({ router, logger }: RouteDeps) { +export function initGetCaseApi({ router, logger, kibanaVersion }: RouteDeps) { router.get( { path: CASE_DETAILS_URL, @@ -34,10 +34,7 @@ export function initGetCaseApi({ router, logger }: RouteDeps) { return response.ok({ headers: { - ...getWarningHeader( - casesClient.kibanaVersion, - 'Deprecated query parameter includeComments' - ), + ...getWarningHeader(kibanaVersion, 'Deprecated query parameter includeComments'), }, body: await casesClient.cases.get({ id, diff --git a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts index d154c400345e8..9b311013ca2fa 100644 --- a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts +++ b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts @@ -14,7 +14,7 @@ import { CASE_COMMENTS_URL } from '../../../../common/constants'; /** * @deprecated since version 8.1.0 */ -export function initGetAllCommentsApi({ router, logger }: RouteDeps) { +export function initGetAllCommentsApi({ router, logger, kibanaVersion }: RouteDeps) { router.get( { path: CASE_COMMENTS_URL, @@ -30,7 +30,7 @@ export function initGetAllCommentsApi({ router, logger }: RouteDeps) { return response.ok({ headers: { - ...getWarningHeader(client.kibanaVersion), + ...getWarningHeader(kibanaVersion), }, body: await client.attachments.getAll({ caseID: request.params.case_id, diff --git a/x-pack/plugins/cases/server/routes/api/types.ts b/x-pack/plugins/cases/server/routes/api/types.ts index 9211aee5606a6..e3aa6e0e970fa 100644 --- a/x-pack/plugins/cases/server/routes/api/types.ts +++ b/x-pack/plugins/cases/server/routes/api/types.ts @@ -5,13 +5,14 @@ * 2.0. */ -import type { Logger } from 'kibana/server'; +import type { Logger, PluginInitializerContext } from 'kibana/server'; import type { CasesRouter } from '../../types'; export interface RouteDeps { router: CasesRouter; logger: Logger; + kibanaVersion: PluginInitializerContext['env']['packageInfo']['version']; } export interface TotalCommentByCase { diff --git a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts index 655d65430ddb9..113596e560ab1 100644 --- a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts +++ b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts @@ -14,7 +14,7 @@ import { CASE_USER_ACTIONS_URL } from '../../../../common/constants'; /** * @deprecated since version 8.1.0 */ -export function initGetAllCaseUserActionsApi({ router, logger }: RouteDeps) { +export function initGetAllCaseUserActionsApi({ router, logger, kibanaVersion }: RouteDeps) { router.get( { path: CASE_USER_ACTIONS_URL, @@ -35,7 +35,7 @@ export function initGetAllCaseUserActionsApi({ router, logger }: RouteDeps) { return response.ok({ headers: { - ...getWarningHeader(casesClient.kibanaVersion), + ...getWarningHeader(kibanaVersion), }, body: await casesClient.userActions.getAll({ caseId }), }); diff --git a/x-pack/test/cases_api_integration/common/lib/utils.ts b/x-pack/test/cases_api_integration/common/lib/utils.ts index cda5d6dcbeaa1..10ccac3c12139 100644 --- a/x-pack/test/cases_api_integration/common/lib/utils.ts +++ b/x-pack/test/cases_api_integration/common/lib/utils.ts @@ -1133,9 +1133,9 @@ export const getServiceNowSimulationServer = async (): Promise<{ }; /** + * Extracts the warning value a warning header that is formatted according to RFC 7234. + * For example for the string 299 Kibana-8.1.0 "Deprecation endpoint", the return value is Deprecation endpoint. * - * @param warningHeader - * @returns */ export const extractWarningValueFromWarningHeader = (warningHeader: string) => { const firstQuote = warningHeader.indexOf('"'); From d59f1b988cfd76308c242670b7014d6deaad9da6 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 7 Feb 2022 18:08:35 +0200 Subject: [PATCH 06/13] Move assertWarningHeader to validations --- .../test/cases_api_integration/common/lib/utils.ts | 13 ------------- .../cases_api_integration/common/lib/validation.ts | 13 +++++++++++++ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/test/cases_api_integration/common/lib/utils.ts b/x-pack/test/cases_api_integration/common/lib/utils.ts index 10ccac3c12139..9c56db80e45fc 100644 --- a/x-pack/test/cases_api_integration/common/lib/utils.ts +++ b/x-pack/test/cases_api_integration/common/lib/utils.ts @@ -1143,16 +1143,3 @@ export const extractWarningValueFromWarningHeader = (warningHeader: string) => { const warningValue = warningHeader.substring(firstQuote + 1, lastQuote); return warningValue; }; - -/** - * Regular expression to test if a string matches the RFC7234 specification (without warn-date) for warning headers. This pattern assumes that the warn code - * is always 299. Further, this pattern assumes that the warn agent represents a version of Kibana. - * - * Example: 299 Kibana-8.2.0 "Deprecated endpoint" - */ -const WARNING_HEADER_REGEX = /299 Kibana-\d+.\d+.\d+ \".+\"/g; - -export const assertWarningHeader = (warningHeader: string) => { - const res = warningHeader.match(WARNING_HEADER_REGEX); - expect(res).not.to.be(null); -}; diff --git a/x-pack/test/cases_api_integration/common/lib/validation.ts b/x-pack/test/cases_api_integration/common/lib/validation.ts index 220074f22db86..d03c9494d6b99 100644 --- a/x-pack/test/cases_api_integration/common/lib/validation.ts +++ b/x-pack/test/cases_api_integration/common/lib/validation.ts @@ -37,3 +37,16 @@ export function arraysToEqual(array1?: object[], array2?: object[]) { const array1AsSet = new Set(array1); return array2.every((item) => array1AsSet.has(item)); } + +/** + * Regular expression to test if a string matches the RFC7234 specification (without warn-date) for warning headers. This pattern assumes that the warn code + * is always 299. Further, this pattern assumes that the warn agent represents a version of Kibana. + * + * Example: 299 Kibana-8.2.0 "Deprecated endpoint" + */ +const WARNING_HEADER_REGEX = /299 Kibana-\d+.\d+.\d+ \".+\"/g; + +export const assertWarningHeader = (warningHeader: string) => { + const res = warningHeader.match(WARNING_HEADER_REGEX); + expect(res).not.to.be(null); +}; From 7b6cf4f1647ba8705b87a15891a52cc7c1e5e0fe Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Feb 2022 15:51:29 +0200 Subject: [PATCH 07/13] Deprecate status endpoint --- .../cases/server/routes/api/stats/get_status.ts | 10 ++++++++-- .../tests/common/cases/status/get_status.ts | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/cases/server/routes/api/stats/get_status.ts b/x-pack/plugins/cases/server/routes/api/stats/get_status.ts index ffbc101a75dc4..4be7bf6472da1 100644 --- a/x-pack/plugins/cases/server/routes/api/stats/get_status.ts +++ b/x-pack/plugins/cases/server/routes/api/stats/get_status.ts @@ -6,12 +6,15 @@ */ import { RouteDeps } from '../types'; -import { escapeHatch, wrapError } from '../utils'; +import { escapeHatch, wrapError, getWarningHeader } from '../utils'; import { CasesStatusRequest } from '../../../../common/api'; import { CASE_STATUS_URL } from '../../../../common/constants'; -export function initGetCasesStatusApi({ router, logger }: RouteDeps) { +/** + * @deprecated since version 8.1.0 + */ +export function initGetCasesStatusApi({ router, logger, kibanaVersion }: RouteDeps) { router.get( { path: CASE_STATUS_URL, @@ -21,6 +24,9 @@ export function initGetCasesStatusApi({ router, logger }: RouteDeps) { try { const client = await context.cases.getCasesClient(); return response.ok({ + headers: { + ...getWarningHeader(kibanaVersion), + }, body: await client.metrics.getStatusTotalsByType(request.query as CasesStatusRequest), }); } catch (error) { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts index 02ace7077a20a..c3fd98737c261 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts @@ -16,6 +16,8 @@ import { getAllCasesStatuses, deleteAllCaseItems, superUserSpace1Auth, + assertWarningHeader, + extractWarningValueFromWarningHeader, } from '../../../../../common/lib/utils'; import { globalRead, @@ -26,6 +28,7 @@ import { secOnlyRead, superUser, } from '../../../../../common/lib/authentication/users'; +import { CASE_STATUS_URL } from '../../../../../../../plugins/cases/common/constants'; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { @@ -181,5 +184,18 @@ export default ({ getService }: FtrProviderContext): void => { }); } }); + + describe('deprecations', () => { + it('should return a warning header', async () => { + await createCase(supertest, postCaseReq); + const res = await supertest.get(CASE_STATUS_URL).expect(200); + const warningHeader = res.header.warning; + + assertWarningHeader(warningHeader); + + const warningValue = extractWarningValueFromWarningHeader(warningHeader); + expect(warningValue).to.be('Deprecated endpoint'); + }); + }); }); }; From 84ef3b4cd685896a74c62eadc9974aafe04e5372 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Feb 2022 17:21:47 +0200 Subject: [PATCH 08/13] Fix validation --- x-pack/test/cases_api_integration/common/lib/validation.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/cases_api_integration/common/lib/validation.ts b/x-pack/test/cases_api_integration/common/lib/validation.ts index d03c9494d6b99..826ac7e2600e1 100644 --- a/x-pack/test/cases_api_integration/common/lib/validation.ts +++ b/x-pack/test/cases_api_integration/common/lib/validation.ts @@ -44,7 +44,8 @@ export function arraysToEqual(array1?: object[], array2?: object[]) { * * Example: 299 Kibana-8.2.0 "Deprecated endpoint" */ -const WARNING_HEADER_REGEX = /299 Kibana-\d+.\d+.\d+ \".+\"/g; +const WARNING_HEADER_REGEX = + /299 Kibana-\d+.\d+.\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)? \".+\"/g; export const assertWarningHeader = (warningHeader: string) => { const res = warningHeader.match(WARNING_HEADER_REGEX); From e63e3cfaa3acdf634b19004e9908fb38ab8f28ee Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Feb 2022 17:22:17 +0200 Subject: [PATCH 09/13] Integration test for status API --- .../security_and_spaces/tests/common/cases/status/get_status.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts index c3fd98737c261..c170dc0ff3ccd 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts @@ -16,7 +16,6 @@ import { getAllCasesStatuses, deleteAllCaseItems, superUserSpace1Auth, - assertWarningHeader, extractWarningValueFromWarningHeader, } from '../../../../../common/lib/utils'; import { @@ -29,6 +28,7 @@ import { superUser, } from '../../../../../common/lib/authentication/users'; import { CASE_STATUS_URL } from '../../../../../../../plugins/cases/common/constants'; +import { assertWarningHeader } from '../../../../../common/lib/validation'; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { From 3bebd6b1ab76b1374dfd18bfd0bbbe8abcdba319 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Feb 2022 17:22:29 +0200 Subject: [PATCH 10/13] Fix types --- .../security_and_spaces/tests/common/cases/get_case.ts | 2 +- .../tests/common/comments/get_all_comments.ts | 2 +- .../tests/common/user_actions/get_all_user_actions.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts index aff1caff022fc..4932f8ccb7784 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts @@ -24,7 +24,6 @@ import { createComment, removeServerGeneratedPropertiesFromCase, removeServerGeneratedPropertiesFromSavedObject, - assertWarningHeader, extractWarningValueFromWarningHeader, } from '../../../../common/lib/utils'; import { @@ -39,6 +38,7 @@ import { obsSec, } from '../../../../common/lib/authentication/users'; import { getUserInfo } from '../../../../common/lib/authentication'; +import { assertWarningHeader } from '../../../../common/lib/validation'; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/get_all_comments.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/get_all_comments.ts index 0a5dd64bc0dfc..d6e52b6577fef 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/get_all_comments.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/get_all_comments.ts @@ -15,7 +15,6 @@ import { createComment, getAllComments, superUserSpace1Auth, - assertWarningHeader, extractWarningValueFromWarningHeader, } from '../../../../common/lib/utils'; import { @@ -30,6 +29,7 @@ import { superUser, } from '../../../../common/lib/authentication/users'; import { getCaseCommentsUrl } from '../../../../../../plugins/cases/common/api'; +import { assertWarningHeader } from '../../../../common/lib/validation'; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/get_all_user_actions.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/get_all_user_actions.ts index bf45e7e384217..b09059966a16b 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/get_all_user_actions.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/user_actions/get_all_user_actions.ts @@ -27,7 +27,6 @@ import { createComment, updateComment, deleteComment, - assertWarningHeader, extractWarningValueFromWarningHeader, } from '../../../../common/lib/utils'; import { @@ -39,6 +38,7 @@ import { secOnlyRead, superUser, } from '../../../../common/lib/authentication/users'; +import { assertWarningHeader } from '../../../../common/lib/validation'; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { From eb3c26a45e8abda691fe20acee023731f6c0d8b8 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Feb 2022 18:58:37 +0200 Subject: [PATCH 11/13] Log warnings about deprecated endpoints --- x-pack/plugins/cases/server/routes/api/cases/get_case.ts | 4 ++++ .../cases/server/routes/api/comments/get_all_comment.ts | 1 + x-pack/plugins/cases/server/routes/api/stats/get_status.ts | 2 ++ .../server/routes/api/user_actions/get_all_user_actions.ts | 2 ++ 4 files changed, 9 insertions(+) diff --git a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts index 5e45b30f626f7..4f121a66b29f7 100644 --- a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts +++ b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts @@ -29,6 +29,10 @@ export function initGetCaseApi({ router, logger, kibanaVersion }: RouteDeps) { }, async (context, request, response) => { try { + logger.warn( + `The query parameter 'includeComments' of the get case API '${CASE_DETAILS_URL}' is deprecated` + ); + const casesClient = await context.cases.getCasesClient(); const id = request.params.case_id; diff --git a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts index 9b311013ca2fa..7b739005fcd2f 100644 --- a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts +++ b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts @@ -26,6 +26,7 @@ export function initGetAllCommentsApi({ router, logger, kibanaVersion }: RouteDe }, async (context, request, response) => { try { + logger.warn(`The get all cases comments API '${CASE_COMMENTS_URL}' is deprecated.`); const client = await context.cases.getCasesClient(); return response.ok({ diff --git a/x-pack/plugins/cases/server/routes/api/stats/get_status.ts b/x-pack/plugins/cases/server/routes/api/stats/get_status.ts index 4be7bf6472da1..bf5f9206ab28b 100644 --- a/x-pack/plugins/cases/server/routes/api/stats/get_status.ts +++ b/x-pack/plugins/cases/server/routes/api/stats/get_status.ts @@ -22,6 +22,8 @@ export function initGetCasesStatusApi({ router, logger, kibanaVersion }: RouteDe }, async (context, request, response) => { try { + logger.warn(`The get cases status API '${CASE_STATUS_URL}' is deprecated.`); + const client = await context.cases.getCasesClient(); return response.ok({ headers: { diff --git a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts index 113596e560ab1..bde1c818e0547 100644 --- a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts +++ b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts @@ -30,6 +30,8 @@ export function initGetAllCaseUserActionsApi({ router, logger, kibanaVersion }: return response.badRequest({ body: 'RouteHandlerContext is not registered for cases' }); } + logger.warn(`The get all cases user actions API '${CASE_USER_ACTIONS_URL}' is deprecated.`); + const casesClient = await context.cases.getCasesClient(); const caseId = request.params.case_id; From 4d293ccb9e844d97a8645f8c93cd69fdb408853e Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 9 Feb 2022 13:19:55 +0200 Subject: [PATCH 12/13] Warn user only if provided the includeComment query param --- .../cases/server/routes/api/cases/get_case.ts | 19 ++++++++++----- .../tests/common/cases/get_case.ts | 23 ++++++++++++++----- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts index 4f121a66b29f7..dcd92c8160f63 100644 --- a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts +++ b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts @@ -29,17 +29,24 @@ export function initGetCaseApi({ router, logger, kibanaVersion }: RouteDeps) { }, async (context, request, response) => { try { - logger.warn( - `The query parameter 'includeComments' of the get case API '${CASE_DETAILS_URL}' is deprecated` - ); + const isIncludeCommentsParamProvidedByTheUser = + request.url.searchParams.has('includeComments'); + + if (isIncludeCommentsParamProvidedByTheUser) { + logger.warn( + `The query parameter 'includeComments' of the get case API '${CASE_DETAILS_URL}' is deprecated` + ); + } const casesClient = await context.cases.getCasesClient(); const id = request.params.case_id; return response.ok({ - headers: { - ...getWarningHeader(kibanaVersion, 'Deprecated query parameter includeComments'), - }, + ...(isIncludeCommentsParamProvidedByTheUser && { + headers: { + ...getWarningHeader(kibanaVersion, 'Deprecated query parameter includeComments'), + }, + }), body: await casesClient.cases.get({ id, includeComments: request.query.includeComments, diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts index 4932f8ccb7784..abafc514e56f3 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/get_case.ts @@ -195,15 +195,26 @@ export default ({ getService }: FtrProviderContext): void => { }); describe('deprecations', () => { - it('should return a warning header', async () => { + for (const paramValue of [true, false]) { + it(`should return a warning header if includeComments=${paramValue}`, async () => { + const theCase = await createCase(supertest, postCaseReq); + const res = await supertest + .get(`${getCaseDetailsUrl(theCase.id)}?includeComments=${paramValue}`) + .expect(200); + const warningHeader = res.header.warning; + + assertWarningHeader(warningHeader); + + const warningValue = extractWarningValueFromWarningHeader(warningHeader); + expect(warningValue).to.be('Deprecated query parameter includeComments'); + }); + } + + it('should not return a warning header if includeComments is not provided', async () => { const theCase = await createCase(supertest, postCaseReq); const res = await supertest.get(getCaseDetailsUrl(theCase.id)).expect(200); const warningHeader = res.header.warning; - - assertWarningHeader(warningHeader); - - const warningValue = extractWarningValueFromWarningHeader(warningHeader); - expect(warningValue).to.be('Deprecated query parameter includeComments'); + expect(warningHeader).to.be(undefined); }); }); }); From 07598ab98f6fd33fcdb321f8ba1e75efcb3c6984 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 10 Feb 2022 18:54:55 +0200 Subject: [PATCH 13/13] Log only request from external clients --- .../cases/server/routes/api/cases/get_case.ts | 6 +++-- .../routes/api/comments/get_all_comment.ts | 9 ++++++-- .../server/routes/api/stats/get_status.ts | 8 +++++-- .../api/user_actions/get_all_user_actions.ts | 8 +++++-- .../cases/server/routes/api/utils.test.ts | 22 ++++++++++++++++++- .../plugins/cases/server/routes/api/utils.ts | 19 +++++++++++++++- 6 files changed, 62 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts index dcd92c8160f63..c8558d09e5c5f 100644 --- a/x-pack/plugins/cases/server/routes/api/cases/get_case.ts +++ b/x-pack/plugins/cases/server/routes/api/cases/get_case.ts @@ -8,7 +8,7 @@ import { schema } from '@kbn/config-schema'; import { RouteDeps } from '../types'; -import { getWarningHeader, wrapError } from '../utils'; +import { getWarningHeader, logDeprecatedEndpoint, wrapError } from '../utils'; import { CASE_DETAILS_URL } from '../../../../common/constants'; export function initGetCaseApi({ router, logger, kibanaVersion }: RouteDeps) { @@ -33,7 +33,9 @@ export function initGetCaseApi({ router, logger, kibanaVersion }: RouteDeps) { request.url.searchParams.has('includeComments'); if (isIncludeCommentsParamProvidedByTheUser) { - logger.warn( + logDeprecatedEndpoint( + logger, + request.headers, `The query parameter 'includeComments' of the get case API '${CASE_DETAILS_URL}' is deprecated` ); } diff --git a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts index 7b739005fcd2f..e94b19cdd9a1c 100644 --- a/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts +++ b/x-pack/plugins/cases/server/routes/api/comments/get_all_comment.ts @@ -8,7 +8,7 @@ import { schema } from '@kbn/config-schema'; import { RouteDeps } from '../types'; -import { wrapError, getWarningHeader } from '../utils'; +import { wrapError, getWarningHeader, logDeprecatedEndpoint } from '../utils'; import { CASE_COMMENTS_URL } from '../../../../common/constants'; /** @@ -26,7 +26,12 @@ export function initGetAllCommentsApi({ router, logger, kibanaVersion }: RouteDe }, async (context, request, response) => { try { - logger.warn(`The get all cases comments API '${CASE_COMMENTS_URL}' is deprecated.`); + logDeprecatedEndpoint( + logger, + request.headers, + `The get all cases comments API '${CASE_COMMENTS_URL}' is deprecated.` + ); + const client = await context.cases.getCasesClient(); return response.ok({ diff --git a/x-pack/plugins/cases/server/routes/api/stats/get_status.ts b/x-pack/plugins/cases/server/routes/api/stats/get_status.ts index bf5f9206ab28b..90044c9516b3a 100644 --- a/x-pack/plugins/cases/server/routes/api/stats/get_status.ts +++ b/x-pack/plugins/cases/server/routes/api/stats/get_status.ts @@ -6,7 +6,7 @@ */ import { RouteDeps } from '../types'; -import { escapeHatch, wrapError, getWarningHeader } from '../utils'; +import { escapeHatch, wrapError, getWarningHeader, logDeprecatedEndpoint } from '../utils'; import { CasesStatusRequest } from '../../../../common/api'; import { CASE_STATUS_URL } from '../../../../common/constants'; @@ -22,7 +22,11 @@ export function initGetCasesStatusApi({ router, logger, kibanaVersion }: RouteDe }, async (context, request, response) => { try { - logger.warn(`The get cases status API '${CASE_STATUS_URL}' is deprecated.`); + logDeprecatedEndpoint( + logger, + request.headers, + `The get cases status API '${CASE_STATUS_URL}' is deprecated.` + ); const client = await context.cases.getCasesClient(); return response.ok({ diff --git a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts index bde1c818e0547..2e38ac8b4ebc7 100644 --- a/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts +++ b/x-pack/plugins/cases/server/routes/api/user_actions/get_all_user_actions.ts @@ -8,7 +8,7 @@ import { schema } from '@kbn/config-schema'; import { RouteDeps } from '../types'; -import { getWarningHeader, wrapError } from '../utils'; +import { getWarningHeader, logDeprecatedEndpoint, wrapError } from '../utils'; import { CASE_USER_ACTIONS_URL } from '../../../../common/constants'; /** @@ -30,7 +30,11 @@ export function initGetAllCaseUserActionsApi({ router, logger, kibanaVersion }: return response.badRequest({ body: 'RouteHandlerContext is not registered for cases' }); } - logger.warn(`The get all cases user actions API '${CASE_USER_ACTIONS_URL}' is deprecated.`); + logDeprecatedEndpoint( + logger, + request.headers, + `The get all cases user actions API '${CASE_USER_ACTIONS_URL}' is deprecated.` + ); const casesClient = await context.cases.getCasesClient(); const caseId = request.params.case_id; diff --git a/x-pack/plugins/cases/server/routes/api/utils.test.ts b/x-pack/plugins/cases/server/routes/api/utils.test.ts index c2cff04f56a49..2614536e3242b 100644 --- a/x-pack/plugins/cases/server/routes/api/utils.test.ts +++ b/x-pack/plugins/cases/server/routes/api/utils.test.ts @@ -6,8 +6,9 @@ */ import { isBoom, boomify } from '@hapi/boom'; +import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; import { HTTPError } from '../../common/error'; -import { wrapError } from './utils'; +import { logDeprecatedEndpoint, wrapError } from './utils'; describe('Utils', () => { describe('wrapError', () => { @@ -55,4 +56,23 @@ describe('Utils', () => { expect(res.headers).toEqual({}); }); }); + + describe('logDeprecatedEndpoint', () => { + const logger = loggingSystemMock.createLogger(); + const kibanaHeader = { 'kbn-version': '8.1.0', referer: 'test' }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('does NOT log when the request is from the kibana client', () => { + logDeprecatedEndpoint(logger, kibanaHeader, 'test'); + expect(logger.warn).not.toHaveBeenCalledWith('test'); + }); + + it('does log when the request is NOT from the kibana client', () => { + logDeprecatedEndpoint(logger, {}, 'test'); + expect(logger.warn).toHaveBeenCalledWith('test'); + }); + }); }); diff --git a/x-pack/plugins/cases/server/routes/api/utils.ts b/x-pack/plugins/cases/server/routes/api/utils.ts index 8a824b8679ffd..532a316e9a7b8 100644 --- a/x-pack/plugins/cases/server/routes/api/utils.ts +++ b/x-pack/plugins/cases/server/routes/api/utils.ts @@ -7,7 +7,7 @@ import { Boom, boomify, isBoom } from '@hapi/boom'; import { schema } from '@kbn/config-schema'; -import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; +import type { CustomHttpResponseOptions, ResponseError, Headers, Logger } from 'kibana/server'; import { CaseError, isCaseError, HTTPError, isHTTPError } from '../../common/error'; /** @@ -46,3 +46,20 @@ export const getWarningHeader = ( ): { warning: string } => ({ warning: `299 Kibana-${kibanaVersion} "${msg}"`, }); + +/** + * Taken from + * https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 + */ + +const getIsKibanaRequest = (headers?: Headers) => { + // The presence of these two request headers gives us a good indication that this is a first-party request from the Kibana client. + // We can't be 100% certain, but this is a reasonable attempt. + return headers && headers['kbn-version'] && headers.referer; +}; + +export const logDeprecatedEndpoint = (logger: Logger, headers: Headers, msg: string) => { + if (!getIsKibanaRequest(headers)) { + logger.warn(msg); + } +};