[http] Require version headers for internal endpoints (#159009)

## Summary

Makes version headers required for internal endpoints. We also require
version headers for public endpoints when in dev mode.

### Note to reviewers

This PR is a re-revert of the original
https://github.com/elastic/kibana/pull/158667 with some minor additions
(see comments).

The original was reverted due to failing Cypress tests blocking Kibana
promotion for 8.8.1 (CC @stephmilovic,
https://github.com/elastic/kibana/pull/158961)

Not sending headers to versioned, internal endpoints will return 400!
Due to the somewhat sensitive nature of this change, I went through all
of the existing `.versioned` endpoints and tried to ensure that for
_internal_ endpoints we send through a version as this is now
**required**.

I would greatly appreciate it if code owners could check their code,
think of any existing consumers of your versioned endpoints and ensure
they are sending a version.

Closes https://github.com/elastic/kibana/issues/158722

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com>
This commit is contained in:
Jean-Louis Leysens 2023-06-06 19:51:06 +02:00 committed by GitHub
parent f48b742be1
commit 6cf0c8c564
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 119 additions and 35 deletions

View file

@ -6,7 +6,7 @@
* Side Public License, v 1.
*/
/** @internal */
/** @public */
export const ELASTIC_HTTP_VERSION_HEADER = 'elastic-api-version' as const;
export const X_ELASTIC_INTERNAL_ORIGIN_REQUEST = 'x-elastic-internal-origin' as const;

View file

@ -62,6 +62,7 @@ export class CoreVersionedRoute implements VersionedRoute {
}
private isPublic: boolean;
private isInternal: boolean;
private constructor(
private readonly router: CoreVersionedRouter,
public readonly method: Method,
@ -69,6 +70,7 @@ export class CoreVersionedRoute implements VersionedRoute {
public readonly options: VersionedRouteConfig<Method>
) {
this.isPublic = this.options.access === 'public';
this.isInternal = !this.isPublic;
this.router.router[this.method](
{
path: this.path,
@ -91,11 +93,8 @@ export class CoreVersionedRoute implements VersionedRoute {
return resolvers[this.router.defaultHandlerResolutionStrategy]([...this.handlers.keys()]);
}
private getAvailableVersionsMessage(): string {
const versions = [...this.handlers.keys()];
return `Available versions are: ${
versions.length ? '[' + [...versions].join(', ') + ']' : '<none>'
}`;
private versionsToString(): string {
return this.handlers.size ? '[' + [...this.handlers.keys()].join(', ') + ']' : '<none>';
}
private requestHandler = async (
@ -109,6 +108,13 @@ export class CoreVersionedRoute implements VersionedRoute {
body: `No handlers registered for [${this.method}] [${this.path}].`,
});
}
if (!this.hasVersion(req) && (this.isInternal || this.router.isDev)) {
return res.badRequest({
body: `Please specify a version. Available versions: ${this.versionsToString()}`,
});
}
const version = this.getVersion(req);
const invalidVersionMessage = isValidRouteVersion(this.isPublic, version);
@ -121,7 +127,7 @@ export class CoreVersionedRoute implements VersionedRoute {
return res.badRequest({
body: `No version "${version}" available for [${this.method}] [${
this.path
}]. ${this.getAvailableVersionsMessage()}`,
}]. Available versions are: ${this.versionsToString()}`,
});
}
@ -179,6 +185,10 @@ export class CoreVersionedRoute implements VersionedRoute {
);
};
private hasVersion(request: KibanaRequest): boolean {
return ELASTIC_HTTP_VERSION_HEADER in request.headers;
}
private getVersion(request: KibanaRequest): ApiVersion {
const versions = request.headers?.[ELASTIC_HTTP_VERSION_HEADER];
return Array.isArray(versions) ? versions[0] : versions ?? this.getDefaultVersion();

View file

@ -80,6 +80,7 @@ describe('Routing versioned requests', () => {
});
it('handles missing version header (defaults to oldest)', async () => {
await setupServer({ dev: false });
router.versioned
.get({ path: '/my-path', access: 'public' })
.addVersion({ validate: false, version: '2020-02-02' }, async (ctx, req, res) => {
@ -222,6 +223,56 @@ describe('Routing versioned requests', () => {
).resolves.toEqual('1');
});
it('requires version headers to be set for internal HTTP APIs', async () => {
await setupServer({ dev: false });
router.versioned
.get({ path: '/my-path', access: 'internal' })
.addVersion(
{ version: '1', validate: { response: { 200: { body: schema.number() } } } },
async (ctx, req, res) => res.ok()
)
.addVersion(
{ version: '2', validate: { response: { 200: { body: schema.number() } } } },
async (ctx, req, res) => res.ok()
);
await server.start();
await expect(
supertest
.get('/my-path')
.unset('Elastic-Api-Version')
.expect(400)
.then(({ body }) => body.message)
).resolves.toMatch(/Please specify.+version/);
});
it.each([
['public', '2022-02-02', '2022-02-03'],
['internal', '1', '2'],
])('requires version headers to be set %p when in dev', async (access, v1, v2) => {
await setupServer({ dev: true });
router.versioned
.get({ path: '/my-path', access: access as 'internal' | 'public' })
.addVersion(
{ version: v1, validate: { response: { 200: { body: schema.number() } } } },
async (ctx, req, res) => res.ok()
)
.addVersion(
{ version: v2, validate: { response: { 200: { body: schema.number() } } } },
async (ctx, req, res) => res.ok()
);
await server.start();
await expect(
supertest
.get('/my-path')
.unset('Elastic-Api-Version')
.expect(400)
.then(({ body }) => body.message)
).resolves.toMatch(/Please specify.+version/);
});
it('errors when no handler could be found', async () => {
router.versioned.get({ path: '/my-path', access: 'public' });
@ -239,7 +290,7 @@ describe('Routing versioned requests', () => {
});
it('resolves the newest handler on serverless', async () => {
await setupServer({ serverless: true });
await setupServer({ serverless: true, dev: false });
router.versioned
.get({ path: '/my-path', access: 'public' })
@ -261,7 +312,7 @@ describe('Routing versioned requests', () => {
});
it('resolves the oldest handler on anything other than serverless', async () => {
await setupServer({ serverless: false });
await setupServer({ serverless: false, dev: false });
router.versioned
.get({ path: '/my-path', access: 'public' })

View file

@ -54,5 +54,6 @@ export async function getTimeFieldRange(options: GetTimeFieldRangeOptions) {
path,
method: 'POST',
body: JSON.stringify(body),
version: '1',
});
}

View file

@ -27,6 +27,7 @@ export const useFindCspRuleTemplates = (
() => {
return http.get<GetCspRuleTemplateResponse>(FIND_CSP_RULE_TEMPLATE_ROUTE_PATH, {
query: { packagePolicyId, page, perPage },
version: '1',
});
}
);

View file

@ -95,7 +95,7 @@ describe('Alert Event Details', () => {
});
});
describe('Response actions', () => {
describe.skip('Response actions', () => {
let multiQueryPackId: string;
let multiQueryPackName: string;
let ruleId: string;

View file

@ -4,7 +4,7 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common';
import { TRANSFORMS_URL } from '../../urls/risk_score';
import { RiskScoreEntity } from './common';
import { getLatestTransformIndex, getPivotTransformIndex } from './indices';
@ -35,7 +35,7 @@ export const getTransformState = (transformId: string) => {
return cy.request<{ transforms: Array<{ id: string; state: string }>; count: number }>({
method: 'get',
url: `${TRANSFORMS_URL}/transforms/${transformId}/_stats`,
headers: { 'kbn-xsrf': 'cypress-creds-via-config' },
headers: { 'kbn-xsrf': 'cypress-creds-via-config', [ELASTIC_HTTP_VERSION_HEADER]: '1' },
});
};
@ -43,7 +43,7 @@ export const startTransforms = (transformIds: string[]) => {
return cy.request({
method: 'post',
url: `${TRANSFORMS_URL}/start_transforms`,
headers: { 'kbn-xsrf': 'cypress-creds-via-config' },
headers: { 'kbn-xsrf': 'cypress-creds-via-config', [ELASTIC_HTTP_VERSION_HEADER]: '1' },
body: transformIds.map((id) => ({
id,
})),
@ -57,7 +57,7 @@ const stopTransform = (state: {
return cy.request({
method: 'post',
url: `${TRANSFORMS_URL}/stop_transforms`,
headers: { 'kbn-xsrf': 'cypress-creds-via-config' },
headers: { 'kbn-xsrf': 'cypress-creds-via-config', [ELASTIC_HTTP_VERSION_HEADER]: '1' },
body:
state != null && state.transforms.length > 0
? [
@ -74,7 +74,7 @@ export const createTransform = (transformId: string, options: string | Record<st
return cy.request({
method: 'put',
url: `${TRANSFORMS_URL}/transforms/${transformId}`,
headers: { 'kbn-xsrf': 'cypress-creds-via-config' },
headers: { 'kbn-xsrf': 'cypress-creds-via-config', [ELASTIC_HTTP_VERSION_HEADER]: '1' },
body: options,
});
};
@ -83,7 +83,7 @@ const deleteTransform = (transformId: string) => {
return cy.request({
method: 'post',
url: `${TRANSFORMS_URL}/delete_transforms`,
headers: { 'kbn-xsrf': 'cypress-creds-via-config' },
headers: { 'kbn-xsrf': 'cypress-creds-via-config', [ELASTIC_HTTP_VERSION_HEADER]: '1' },
failOnStatusCode: false,
body: {
transformsInfo: [

View file

@ -34,6 +34,7 @@
"@kbn/alerting-plugin",
"@kbn/securitysolution-data-table",
"@kbn/cases-plugin",
"@kbn/data-plugin"
"@kbn/data-plugin",
"@kbn/core-http-common"
]
}

View file

@ -49,6 +49,7 @@ export async function createTransform({
const res = await http
.put<CreateTransformResult>(`${TRANSFORM_API_BASE_PATH}/transforms/${transformId}`, {
body: JSON.stringify(options),
version: '1',
signal,
})
.then((result) => {
@ -100,6 +101,7 @@ export async function startTransforms({
id,
}))
),
version: '1',
signal,
})
.then((result) => {
@ -146,6 +148,7 @@ export async function getTransformState({
.get<{ transforms: Array<{ id: string; state: string }>; count: number }>(
`${TRANSFORM_API_BASE_PATH}/transforms/${transformId}/_stats`,
{
version: '1',
signal,
}
)
@ -206,6 +209,7 @@ export async function stopTransforms({
const states = await getTransformsState({ http, signal, transformIds });
const res = await http
.post<StopTransformsResult>(`${TRANSFORM_API_BASE_PATH}/stop_transforms`, {
version: '1',
body: JSON.stringify(
states.reduce(
(acc, state) =>
@ -270,6 +274,7 @@ export async function deleteTransforms({
await stopTransforms({ http, signal, transformIds });
const res = await http
.post<DeleteTransformsResult>(`${TRANSFORM_API_BASE_PATH}/delete_transforms`, {
version: '1',
body: JSON.stringify({
transformsInfo: transformIds.map((id) => ({
id,

View file

@ -24,11 +24,11 @@ import {
import { getJobPrefix, getMLJobId } from '../../../../common/lib/ml';
export const getMLCapabilities = async (): Promise<MlCapabilitiesResponse> => {
return await apiService.get(API_URLS.ML_CAPABILITIES);
return await apiService.get(API_URLS.ML_CAPABILITIES, { version: '1' });
};
export const getExistingJobs = async (): Promise<JobExistResult> => {
return await apiService.get(API_URLS.ML_MODULE_JOBS + ML_MODULE_ID);
return await apiService.get(API_URLS.ML_MODULE_JOBS + ML_MODULE_ID, { version: '1' });
};
export const createMLJob = async ({
@ -54,7 +54,9 @@ export const createMLJob = async ({
},
};
const response: DataRecognizerConfigResponse = await apiService.post(url, data);
const response: DataRecognizerConfigResponse = await apiService.post(url, data, undefined, {
version: '1',
});
if (response?.jobs?.[0]?.id === getMLJobId(monitorId)) {
const jobResponse = response.jobs[0];
const datafeedResponse = response.datafeeds[0];
@ -76,7 +78,7 @@ export const createMLJob = async ({
export const deleteMLJob = async ({ monitorId }: MonitorIdParam): Promise<DeleteJobResults> => {
const data = { jobIds: [getMLJobId(monitorId)] };
return await apiService.post(API_URLS.ML_DELETE_JOB, data);
return await apiService.post(API_URLS.ML_DELETE_JOB, data, undefined, { version: '1' });
};
export const fetchAnomalyRecords = async ({
@ -97,5 +99,5 @@ export const fetchAnomalyRecords = async ({
maxRecords: 500,
maxExamples: 10,
};
return apiService.post(API_URLS.ML_ANOMALIES_RESULT, data);
return apiService.post(API_URLS.ML_ANOMALIES_RESULT, data, undefined, { version: '1' });
};

View file

@ -10,6 +10,7 @@ import { formatErrors } from '@kbn/securitysolution-io-ts-utils';
import { HttpFetchQuery, HttpSetup } from '@kbn/core/public';
import { FETCH_STATUS, AddInspectorRequest } from '@kbn/observability-shared-plugin/public';
type Params = HttpFetchQuery & { version?: string };
class ApiService {
private static instance: ApiService;
private _http!: HttpSetup;
@ -41,16 +42,13 @@ class ApiService {
return ApiService.instance;
}
public async get<T>(
apiUrl: string,
params?: HttpFetchQuery,
decodeType?: any,
asResponse = false
) {
public async get<T>(apiUrl: string, params: Params = {}, decodeType?: any, asResponse = false) {
const { version, ...queryParams } = params;
const response = await this._http!.fetch<T>({
path: apiUrl,
query: params,
query: queryParams,
asResponse,
version,
});
this.addInspectorRequest?.({ data: response, status: FETCH_STATUS.SUCCESS, loading: false });
@ -73,11 +71,13 @@ class ApiService {
return response;
}
public async post<T>(apiUrl: string, data?: any, decodeType?: any, params?: HttpFetchQuery) {
public async post<T>(apiUrl: string, data?: any, decodeType?: any, params: Params = {}) {
const { version, ...queryParams } = params;
const response = await this._http!.post<T>(apiUrl, {
method: 'POST',
body: JSON.stringify(data),
query: params,
query: queryParams,
version,
});
this.addInspectorRequest?.({ data: response, status: FETCH_STATUS.SUCCESS, loading: false });
@ -96,11 +96,13 @@ class ApiService {
return response;
}
public async put<T>(apiUrl: string, data?: any, decodeType?: any, params?: HttpFetchQuery) {
public async put<T>(apiUrl: string, data?: any, decodeType?: any, params: Params = {}) {
const { version, ...queryParams } = params;
const response = await this._http!.put<T>(apiUrl, {
method: 'PUT',
body: JSON.stringify(data),
query: params,
query: queryParams,
version,
});
if (decodeType) {
@ -117,8 +119,8 @@ class ApiService {
return response;
}
public async delete<T>(apiUrl: string) {
const response = await this._http!.delete<T>(apiUrl);
public async delete<T>(apiUrl: string, { version }: { version?: string } = {}) {
const response = await this._http!.delete<T>(apiUrl, { version });
if (response instanceof Error) {
throw response;
}

View file

@ -5,6 +5,7 @@
* 2.0.
*/
import expect from '@kbn/expect';
import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common';
import type { GetCspRuleTemplateResponse } from '@kbn/cloud-security-posture-plugin/common/types';
import type { SuperTest, Test } from 'supertest';
import { CspRuleTemplate } from '@kbn/cloud-security-posture-plugin/common/schemas';
@ -50,6 +51,7 @@ export default function ({ getService }: FtrProviderContext) {
const { body }: { body: { message: string } } = await supertest
.get(`/internal/cloud_security_posture/rules/_find`)
.set(ELASTIC_HTTP_VERSION_HEADER, '1')
.set('kbn-xsrf', 'xxxx')
.expect(500);
@ -70,6 +72,7 @@ export default function ({ getService }: FtrProviderContext) {
const { body }: { body: { message: string } } = await supertest
.get(`/internal/cloud_security_posture/rules/_find`)
.set(ELASTIC_HTTP_VERSION_HEADER, '1')
.set('kbn-xsrf', 'xxxx')
.query({
packagePolicyId: 'your-package-policy-id',
@ -85,6 +88,7 @@ export default function ({ getService }: FtrProviderContext) {
it(`Should return 404 status code when the package policy ID does not exist`, async () => {
const { body }: { body: { statusCode: number; error: string } } = await supertest
.get(`/internal/cloud_security_posture/rules/_find`)
.set(ELASTIC_HTTP_VERSION_HEADER, '1')
.set('kbn-xsrf', 'xxxx')
.query({
packagePolicyId: 'non-existing-packagePolicy-id',
@ -107,6 +111,7 @@ export default function ({ getService }: FtrProviderContext) {
const { body }: { body: GetCspRuleTemplateResponse } = await supertest
.get(`/internal/cloud_security_posture/rules/_find`)
.set(ELASTIC_HTTP_VERSION_HEADER, '1')
.set('kbn-xsrf', 'xxxx')
.query({
benchmarkId: 'cis_k8s',
@ -134,6 +139,7 @@ export default function ({ getService }: FtrProviderContext) {
const { body }: { body: GetCspRuleTemplateResponse } = await supertest
.get(`/internal/cloud_security_posture/rules/_find`)
.set(ELASTIC_HTTP_VERSION_HEADER, '1')
.set('kbn-xsrf', 'xxxx')
.query({
benchmarkId: 'cis_k8s',
@ -166,6 +172,7 @@ export default function ({ getService }: FtrProviderContext) {
const { body }: { body: GetCspRuleTemplateResponse } = await supertest
.get(`/internal/cloud_security_posture/rules/_find`)
.set(ELASTIC_HTTP_VERSION_HEADER, '1')
.set('kbn-xsrf', 'xxxx')
.query({
benchmarkId: 'cis_k8s',
@ -198,6 +205,7 @@ export default function ({ getService }: FtrProviderContext) {
const { body }: { body: GetCspRuleTemplateResponse } = await supertest
.get(`/internal/cloud_security_posture/rules/_find`)
.set(ELASTIC_HTTP_VERSION_HEADER, '1')
.set('kbn-xsrf', 'xxxx')
.query({
benchmarkId: 'cis_k8s',

View file

@ -7,6 +7,7 @@
import fetch from 'node-fetch';
import { format as formatUrl } from 'url';
import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common';
import expect from '@kbn/expect';
@ -36,6 +37,7 @@ export default ({ getService }: FtrProviderContext) => {
await supertest
.post(`/internal/aiops/explain_log_rate_spikes`)
.set('kbn-xsrf', 'kibana')
.set(ELASTIC_HTTP_VERSION_HEADER, '1')
.send(requestBody)
.expect(403);
});
@ -46,6 +48,7 @@ export default ({ getService }: FtrProviderContext) => {
headers: {
'Content-Type': 'application/json',
'kbn-xsrf': 'stream',
[ELASTIC_HTTP_VERSION_HEADER]: '1',
},
body: JSON.stringify(requestBody),
});