[http] Make http headers required for internal and when in dev mode (#158667)

## Summary

Updates the versioned router behaviour to require the setting of version
on requests when either requesting against:

(1) internal endpoints
or
(2) all endpoints when Kibana is in dev mode

The idea is that when calling our versioned endpoints we should always
be requesting a specific version to avoid possible inconsistent
behaviour if our defaults resolution changes onprem (oldest version) vs
serverless (newest version).

Partially addresses https://github.com/elastic/kibana/issues/158722

### Follow up
* We should update our dev docs to point out this behaviour


### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
This commit is contained in:
Jean-Louis Leysens 2023-06-02 16:16:35 +02:00 committed by GitHub
parent 40c75ed1a2
commit de6d8ca33f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 105 additions and 27 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

@ -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),
});