From 9e74fcf8968e47b254f11e05db98b20c6a63eae8 Mon Sep 17 00:00:00 2001 From: Matthew Kime Date: Mon, 19 Jun 2023 09:34:57 -0500 Subject: [PATCH] [data / saved query] BWCA all the routes (#158790) ## Summary All routes - Use versioned router - Moved to internal path - Validate responses - All responses are typed with response types, separate from internal api types. This is to help prevent unacknowledged changes to the api. - Version is included in all requests --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- src/plugins/data/common/constants.ts | 2 + .../saved_query/saved_query_service.test.ts | 20 ++- .../query/saved_query/saved_query_service.ts | 21 ++- src/plugins/data/server/query/route_types.ts | 129 +++++++++++++++++ src/plugins/data/server/query/routes.ts | 136 +++++++++++++----- .../cypress/tasks/api_calls/saved_queries.ts | 6 +- 6 files changed, 263 insertions(+), 51 deletions(-) create mode 100644 src/plugins/data/server/query/route_types.ts diff --git a/src/plugins/data/common/constants.ts b/src/plugins/data/common/constants.ts index 1c457fdb64ce2..2cc12d8b4a966 100644 --- a/src/plugins/data/common/constants.ts +++ b/src/plugins/data/common/constants.ts @@ -37,3 +37,5 @@ export const UI_SETTINGS = { DATE_FORMAT: 'dateFormat', DATEFORMAT_TZ: 'dateFormat:tz', } as const; + +export const SAVED_QUERY_BASE_URL = '/internal/saved_query'; diff --git a/src/plugins/data/public/query/saved_query/saved_query_service.test.ts b/src/plugins/data/public/query/saved_query/saved_query_service.test.ts index e7ac546226326..3a223109dbd76 100644 --- a/src/plugins/data/public/query/saved_query/saved_query_service.test.ts +++ b/src/plugins/data/public/query/saved_query/saved_query_service.test.ts @@ -9,6 +9,7 @@ import { createSavedQueryService } from './saved_query_service'; import { httpServiceMock } from '@kbn/core/public/mocks'; import type { SavedQueryAttributes } from '../../../common'; +import { SAVED_QUERY_BASE_URL } from '../../../common/constants'; const http = httpServiceMock.createStartContract(); @@ -22,6 +23,8 @@ const { getSavedQueryCount, } = createSavedQueryService(http); +const version = '1'; + const savedQueryAttributes: SavedQueryAttributes = { title: 'foo', description: 'bar', @@ -43,8 +46,9 @@ describe('saved query service', () => { it('should post the stringified given attributes', async () => { await createQuery(savedQueryAttributes); expect(http.post).toBeCalled(); - expect(http.post).toHaveBeenCalledWith('/api/saved_query/_create', { + expect(http.post).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/_create`, { body: '{"title":"foo","description":"bar","query":{"language":"kuery","query":"response:200"},"filters":[]}', + version, }); }); }); @@ -53,8 +57,9 @@ describe('saved query service', () => { it('should put the ID & stringified given attributes', async () => { await updateQuery('foo', savedQueryAttributes); expect(http.put).toBeCalled(); - expect(http.put).toHaveBeenCalledWith('/api/saved_query/foo', { + expect(http.put).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/foo`, { body: '{"title":"foo","description":"bar","query":{"language":"kuery","query":"response:200"},"filters":[]}', + version, }); }); }); @@ -67,7 +72,7 @@ describe('saved query service', () => { }); const result = await getAllSavedQueries(); expect(http.post).toBeCalled(); - expect(http.post).toHaveBeenCalledWith('/api/saved_query/_all'); + expect(http.post).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/_all`, { version }); expect(result).toEqual([{ attributes: savedQueryAttributes }]); }); }); @@ -80,8 +85,9 @@ describe('saved query service', () => { }); const result = await findSavedQueries(); expect(http.post).toBeCalled(); - expect(http.post).toHaveBeenCalledWith('/api/saved_query/_find', { + expect(http.post).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/_find`, { body: '{"page":1,"perPage":50,"search":""}', + version, }); expect(result).toEqual({ queries: [{ attributes: savedQueryAttributes }], @@ -94,7 +100,7 @@ describe('saved query service', () => { it('should get the given ID', async () => { await getSavedQuery('my_id'); expect(http.get).toBeCalled(); - expect(http.get).toHaveBeenCalledWith('/api/saved_query/my_id'); + expect(http.get).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/my_id`, { version }); }); }); @@ -102,7 +108,7 @@ describe('saved query service', () => { it('should delete the given ID', async () => { await deleteSavedQuery('my_id'); expect(http.delete).toBeCalled(); - expect(http.delete).toHaveBeenCalledWith('/api/saved_query/my_id'); + expect(http.delete).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/my_id`, { version }); }); }); @@ -110,7 +116,7 @@ describe('saved query service', () => { it('should get the total', async () => { await getSavedQueryCount(); expect(http.get).toBeCalled(); - expect(http.get).toHaveBeenCalledWith('/api/saved_query/_count'); + expect(http.get).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/_count`, { version }); }); }); }); diff --git a/src/plugins/data/public/query/saved_query/saved_query_service.ts b/src/plugins/data/public/query/saved_query/saved_query_service.ts index fd643a33dd103..09afd75470dd0 100644 --- a/src/plugins/data/public/query/saved_query/saved_query_service.ts +++ b/src/plugins/data/public/query/saved_query/saved_query_service.ts @@ -9,18 +9,23 @@ import { HttpStart } from '@kbn/core/public'; import { SavedQuery } from './types'; import type { SavedQueryAttributes } from '../../../common'; +import { SAVED_QUERY_BASE_URL } from '../../../common/constants'; + +const version = '1'; export const createSavedQueryService = (http: HttpStart) => { const createQuery = async (attributes: SavedQueryAttributes, { overwrite = false } = {}) => { - const savedQuery = await http.post('/api/saved_query/_create', { + const savedQuery = await http.post(`${SAVED_QUERY_BASE_URL}/_create`, { body: JSON.stringify(attributes), + version, }); return savedQuery; }; const updateQuery = async (id: string, attributes: SavedQueryAttributes) => { - const savedQuery = await http.put(`/api/saved_query/${id}`, { + const savedQuery = await http.put(`${SAVED_QUERY_BASE_URL}/${id}`, { body: JSON.stringify(attributes), + version, }); return savedQuery; }; @@ -28,7 +33,8 @@ export const createSavedQueryService = (http: HttpStart) => { // we have to tell the saved objects client how many to fetch, otherwise it defaults to fetching 20 per page const getAllSavedQueries = async (): Promise => { const { savedQueries } = await http.post<{ savedQueries: SavedQuery[] }>( - '/api/saved_query/_all' + `${SAVED_QUERY_BASE_URL}/_all`, + { version } ); return savedQueries; }; @@ -42,23 +48,24 @@ export const createSavedQueryService = (http: HttpStart) => { const { total, savedQueries: queries } = await http.post<{ savedQueries: SavedQuery[]; total: number; - }>('/api/saved_query/_find', { + }>(`${SAVED_QUERY_BASE_URL}/_find`, { body: JSON.stringify({ page, perPage, search }), + version, }); return { total, queries }; }; const getSavedQuery = (id: string): Promise => { - return http.get(`/api/saved_query/${id}`); + return http.get(`${SAVED_QUERY_BASE_URL}/${id}`, { version }); }; const deleteSavedQuery = (id: string) => { - return http.delete<{}>(`/api/saved_query/${id}`); + return http.delete<{}>(`${SAVED_QUERY_BASE_URL}/${id}`, { version }); }; const getSavedQueryCount = async (): Promise => { - return http.get('/api/saved_query/_count'); + return http.get(`${SAVED_QUERY_BASE_URL}/_count`, { version }); }; return { diff --git a/src/plugins/data/server/query/route_types.ts b/src/plugins/data/server/query/route_types.ts new file mode 100644 index 0000000000000..656d52dad9fa7 --- /dev/null +++ b/src/plugins/data/server/query/route_types.ts @@ -0,0 +1,129 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import type { SerializableRecord } from '@kbn/utility-types'; + +/* + * These types are used to define the shape of the response from the saved query API + * separate but similar to other types to draw attention to REST api return changes + */ + +interface MatchAllFilterMetaRestResponse extends FilterMetaRestResponse, SerializableRecord { + field: string; + formattedValue: string; +} + +type PhrasesFilterMetaRestResponse = FilterMetaRestResponse & { + params: PhraseFilterValue[]; // The unformatted values + field?: string; +}; + +interface RangeFilterParamsRestResponse extends SerializableRecord { + from?: number | string; + to?: number | string; + gt?: number | string; + lt?: number | string; + gte?: number | string; + lte?: number | string; + format?: string; +} + +type RangeFilterMetaRestResponse = FilterMetaRestResponse & { + params?: RangeFilterParamsRestResponse; + field?: string; + formattedValue?: string; + type: 'range'; +}; + +type PhraseFilterValue = string | number | boolean; + +interface PhraseFilterMetaParamsRestResponse extends SerializableRecord { + query: PhraseFilterValue; // The unformatted value +} + +type PhraseFilterMetaRestResponse = FilterMetaRestResponse & { + params?: PhraseFilterMetaParamsRestResponse; + field?: string; + index?: string; +}; + +type FilterMetaParamsRestResponse = + | FilterRestResponse + | FilterRestResponse[] + | RangeFilterMetaRestResponse + | RangeFilterParamsRestResponse + | PhraseFilterMetaRestResponse + | PhraseFilterMetaParamsRestResponse + | PhrasesFilterMetaRestResponse + | MatchAllFilterMetaRestResponse + | string + | string[] + | boolean + | boolean[] + | number + | number[]; + +interface QueryRestResponse { + query: string | { [key: string]: any }; + language: string; +} + +// eslint-disable-next-line @typescript-eslint/consistent-type-definitions +type FilterMetaRestResponse = { + alias?: string | null; + disabled?: boolean; + negate?: boolean; + // controlledBy is there to identify who owns the filter + controlledBy?: string; + // allows grouping of filters + group?: string; + // index and type are optional only because when you create a new filter, there are no defaults + index?: string; + isMultiIndex?: boolean; + type?: string; + key?: string; + params?: FilterMetaParamsRestResponse; + value?: string; +}; + +type FilterStateStoreRestResponse = 'appState' | 'globalState'; + +// eslint-disable-next-line @typescript-eslint/consistent-type-definitions +type FilterRestResponse = { + $state?: { + store: FilterStateStoreRestResponse; + }; + meta: FilterMetaRestResponse; + query?: Record; +}; + +interface RefreshIntervalRestResponse { + pause: boolean; + value: number; +} + +interface TimeRangeRestResponse { + from: string; + to: string; + mode?: 'absolute' | 'relative'; +} + +type SavedQueryTimeFilterRestResponse = TimeRangeRestResponse & { + refreshInterval: RefreshIntervalRestResponse; +}; + +export interface SavedQueryRestResponse { + id: string; + attributes: { + filters: FilterRestResponse[]; + title: string; + description: string; + query: QueryRestResponse; + timefilter?: SavedQueryTimeFilterRestResponse | undefined; + }; +} diff --git a/src/plugins/data/server/query/routes.ts b/src/plugins/data/server/query/routes.ts index 16f5d8f9955a7..a092a32d321a5 100644 --- a/src/plugins/data/server/query/routes.ts +++ b/src/plugins/data/server/query/routes.ts @@ -9,8 +9,9 @@ import { schema } from '@kbn/config-schema'; import { CoreSetup } from '@kbn/core/server'; import { SavedQueryRouteHandlerContext } from './route_handler_context'; +import { SavedQueryRestResponse } from './route_types'; +import { SAVED_QUERY_BASE_URL } from '../../common/constants'; -const SAVED_QUERY_PATH = '/api/saved_query'; const SAVED_QUERY_ID_CONFIG = schema.object({ id: schema.string(), }); @@ -25,20 +26,35 @@ const SAVED_QUERY_ATTRS_CONFIG = schema.object({ timefilter: schema.maybe(schema.any()), }); +const savedQueryResponseSchema = schema.object({ + id: schema.string(), + attributes: SAVED_QUERY_ATTRS_CONFIG, +}); + +const access = 'internal'; +const version = '1'; + export function registerSavedQueryRoutes({ http }: CoreSetup): void { const router = http.createRouter(); - router.post( + router.versioned.post({ path: `${SAVED_QUERY_BASE_URL}/_create`, access }).addVersion( { - path: `${SAVED_QUERY_PATH}/_create`, + version, validate: { - body: SAVED_QUERY_ATTRS_CONFIG, + request: { + body: SAVED_QUERY_ATTRS_CONFIG, + }, + response: { + 200: { + body: savedQueryResponseSchema, + }, + }, }, }, async (context, request, response) => { try { const savedQuery = await context.savedQuery; - const body = await savedQuery.create(request.body); + const body: SavedQueryRestResponse = await savedQuery.create(request.body); return response.ok({ body }); } catch (e) { // TODO: Handle properly @@ -47,19 +63,26 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { } ); - router.put( + router.versioned.put({ path: `${SAVED_QUERY_BASE_URL}/{id}`, access }).addVersion( { - path: `${SAVED_QUERY_PATH}/{id}`, + version, validate: { - params: SAVED_QUERY_ID_CONFIG, - body: SAVED_QUERY_ATTRS_CONFIG, + request: { + params: SAVED_QUERY_ID_CONFIG, + body: SAVED_QUERY_ATTRS_CONFIG, + }, + response: { + 200: { + body: savedQueryResponseSchema, + }, + }, }, }, async (context, request, response) => { const { id } = request.params; try { const savedQuery = await context.savedQuery; - const body = await savedQuery.update(id, request.body); + const body: SavedQueryRestResponse = await savedQuery.update(id, request.body); return response.ok({ body }); } catch (e) { // TODO: Handle properly @@ -68,18 +91,25 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { } ); - router.get( + router.versioned.get({ path: `${SAVED_QUERY_BASE_URL}/{id}`, access }).addVersion( { - path: `${SAVED_QUERY_PATH}/{id}`, + version, validate: { - params: SAVED_QUERY_ID_CONFIG, + request: { + params: SAVED_QUERY_ID_CONFIG, + }, + response: { + 200: { + body: savedQueryResponseSchema, + }, + }, }, }, async (context, request, response) => { const { id } = request.params; try { const savedQuery = await context.savedQuery; - const body = await savedQuery.get(id); + const body: SavedQueryRestResponse = await savedQuery.get(id); return response.ok({ body }); } catch (e) { // TODO: Handle properly @@ -88,15 +118,22 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { } ); - router.get( + router.versioned.get({ path: `${SAVED_QUERY_BASE_URL}/_count`, access }).addVersion( { - path: `${SAVED_QUERY_PATH}/_count`, - validate: {}, + version, + validate: { + request: {}, + response: { + 200: { + body: schema.number(), + }, + }, + }, }, async (context, request, response) => { try { const savedQuery = await context.savedQuery; - const count = await savedQuery.count(); + const count: number = await savedQuery.count(); return response.ok({ body: `${count}` }); } catch (e) { // TODO: Handle properly @@ -105,21 +142,32 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { } ); - router.post( + router.versioned.post({ path: `${SAVED_QUERY_BASE_URL}/_find`, access }).addVersion( { - path: `${SAVED_QUERY_PATH}/_find`, + version, validate: { - body: schema.object({ - search: schema.string({ defaultValue: '' }), - perPage: schema.number({ defaultValue: 50 }), - page: schema.number({ defaultValue: 1 }), - }), + request: { + body: schema.object({ + search: schema.string({ defaultValue: '' }), + perPage: schema.number({ defaultValue: 50 }), + page: schema.number({ defaultValue: 1 }), + }), + }, + response: { + 200: { + body: schema.object({ + total: schema.number(), + savedQueries: schema.arrayOf(savedQueryResponseSchema), + }), + }, + }, }, }, async (context, request, response) => { try { const savedQuery = await context.savedQuery; - const body = await savedQuery.find(request.body); + const body: { total: number; savedQueries: SavedQueryRestResponse[] } = + await savedQuery.find(request.body); return response.ok({ body }); } catch (e) { // TODO: Handle properly @@ -128,15 +176,26 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { } ); - router.post( + router.versioned.post({ path: `${SAVED_QUERY_BASE_URL}/_all`, access }).addVersion( { - path: `${SAVED_QUERY_PATH}/_all`, - validate: {}, + version, + validate: { + request: {}, + response: { + 200: { + body: schema.object({ + total: schema.number(), + savedQueries: schema.arrayOf(savedQueryResponseSchema), + }), + }, + }, + }, }, async (context, request, response) => { try { const savedQuery = await context.savedQuery; - const body = await savedQuery.getAll(); + const body: { total: number; savedQueries: SavedQueryRestResponse[] } = + await savedQuery.getAll(); return response.ok({ body }); } catch (e) { // TODO: Handle properly @@ -145,19 +204,26 @@ export function registerSavedQueryRoutes({ http }: CoreSetup): void { } ); - router.delete( + router.versioned.delete({ path: `${SAVED_QUERY_BASE_URL}/{id}`, access }).addVersion( { - path: `${SAVED_QUERY_PATH}/{id}`, + version, validate: { - params: SAVED_QUERY_ID_CONFIG, + request: { + params: SAVED_QUERY_ID_CONFIG, + }, + response: { + 200: { + body: schema.never(), + }, + }, }, }, async (context, request, response) => { const { id } = request.params; try { const savedQuery = await context.savedQuery; - const body = await savedQuery.delete(id); - return response.ok({ body }); + await savedQuery.delete(id); + return response.ok(); } catch (e) { // TODO: Handle properly return response.customError(e); diff --git a/x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts b/x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts index ddfcc543c8666..df19f9c6fffba 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts @@ -5,7 +5,9 @@ * 2.0. */ +import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common'; import type { SavedQuery } from '@kbn/data-plugin/public'; +import { SAVED_QUERY_BASE_URL } from '@kbn/data-plugin/common/constants'; import { rootRequest } from '../common'; export const createSavedQuery = ( @@ -15,7 +17,7 @@ export const createSavedQuery = ( ) => rootRequest({ method: 'POST', - url: '/api/saved_query/_create', + url: `${SAVED_QUERY_BASE_URL}/_create`, body: { title, description: '', @@ -34,7 +36,7 @@ export const createSavedQuery = ( }, ], }, - headers: { 'kbn-xsrf': 'cypress-creds' }, + headers: { 'kbn-xsrf': 'cypress-creds', [ELASTIC_HTTP_VERSION_HEADER]: '1' }, }); export const deleteSavedQueries = () => {