Skip to content

Commit

Permalink
[HTTP] Allow for internal requests to also specify special query para…
Browse files Browse the repository at this point in the history
…m `elasticInternalOrigin` (#163796)

## Summary

Closes #163678

* Raise the notion of "internal" into `CoreKibanaRequest`. This enables
us to share this with lifecycle handlers and control validation of query
params
* Added new `isInternalRequest` alongside `isSystemRequest` and
`isFakeRequest`
* Slight simplification to existing internal restriction check
* Some other chores and minor fixes

## Test

* Start ES with `yarn es serverless` and Kibana with `yarn start
--serverless --server.restrictInternalApis=true`
* Add the service account token to `kibana.dev.yml`:
`elasticsearch.serviceAccountToken: <SAT>`
* Send a request to an internal endpoint like: `curl -XPOST
-uelastic:changeme http://localhost:5601/<base-path>/api/files/find -H
'kbn-xsrf: foo' -H 'content-type: application/json' -d '{}'`
    * Should give you a 400 result
* message like `{"statusCode":400,"error":"Bad Request","message":"uri
[http://localhost:5603/api/files/find] with method [post] exists but is
not available with the current configuration"}`
* Send the same request, but include the query param:
`elasticInternalOrigin=true`
   *  Should give you a 200 result

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
jloleysens and kibanamachine authored Aug 21, 2023
1 parent bc988f2 commit 23d3955
Show file tree
Hide file tree
Showing 16 changed files with 242 additions and 171 deletions.
1 change: 1 addition & 0 deletions packages/core/http/core-http-common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ export type { ApiVersion } from './src/versioning';
export {
ELASTIC_HTTP_VERSION_HEADER,
ELASTIC_HTTP_VERSION_QUERY_PARAM,
ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM,
X_ELASTIC_INTERNAL_ORIGIN_REQUEST,
} from './src/constants';
2 changes: 1 addition & 1 deletion packages/core/http/core-http-common/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
/** @public */
export const ELASTIC_HTTP_VERSION_HEADER = 'elastic-api-version' as const;
export const ELASTIC_HTTP_VERSION_QUERY_PARAM = 'apiVersion' as const;

export const ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM = 'elasticInternalOrigin' as const;
export const X_ELASTIC_INTERNAL_ORIGIN_REQUEST = 'x-elastic-internal-origin' as const;
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ import { hapiMocks } from '@kbn/hapi-mocks';
import type { FakeRawRequest } from '@kbn/core-http-server';
import { CoreKibanaRequest } from './request';
import { schema } from '@kbn/config-schema';
import {
ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM,
X_ELASTIC_INTERNAL_ORIGIN_REQUEST,
} from '@kbn/core-http-common';

describe('CoreKibanaRequest', () => {
describe('using real requests', () => {
Expand Down Expand Up @@ -145,6 +149,58 @@ describe('CoreKibanaRequest', () => {
});
});

describe('isInternalApiRequest property', () => {
it('is true when header is set', () => {
const request = hapiMocks.createRequest({
headers: { [X_ELASTIC_INTERNAL_ORIGIN_REQUEST]: 'true' },
});
const kibanaRequest = CoreKibanaRequest.from(request);
expect(kibanaRequest.isInternalApiRequest).toBe(true);
});
it('is true when query param is set', () => {
const request = hapiMocks.createRequest({
query: { [ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM]: 'true' },
});
const kibanaRequest = CoreKibanaRequest.from(request);
expect(kibanaRequest.isInternalApiRequest).toBe(true);
});
it('is true when both header and query param is set', () => {
const request = hapiMocks.createRequest({
headers: { [X_ELASTIC_INTERNAL_ORIGIN_REQUEST]: 'true' },
query: { [ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM]: 'true' },
});
const kibanaRequest = CoreKibanaRequest.from(request);
expect(kibanaRequest.isInternalApiRequest).toBe(true);
});
it('is false when neither header nor query param is set', () => {
const request = hapiMocks.createRequest();
const kibanaRequest = CoreKibanaRequest.from(request);
expect(kibanaRequest.isInternalApiRequest).toBe(false);
});
});

describe('sanitize input', () => {
it('does not pass the reserved query parameter to consumers', () => {
const request = hapiMocks.createRequest({
query: { [ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM]: 'true', myCoolValue: 'cool!' },
});
const kibanaRequest = CoreKibanaRequest.from(request, {
query: schema.object({ myCoolValue: schema.string() }),
});
expect(kibanaRequest.query).toEqual({ myCoolValue: 'cool!' });
});
it('pass nothing if only the reserved query param is present', () => {
const request = hapiMocks.createRequest({
query: { [ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM]: 'true' },
});
expect(() =>
CoreKibanaRequest.from(request, {
query: schema.object({}, { unknowns: 'forbid' }), // we require an empty object
})
).not.toThrow();
});
});

describe('route.options.authRequired property', () => {
it('handles required auth: undefined', () => {
const auth: RouteOptions['auth'] = undefined;
Expand Down
67 changes: 47 additions & 20 deletions packages/core/http/core-http-router-server-internal/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ import {
RawRequest,
FakeRawRequest,
} from '@kbn/core-http-server';
import {
ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM,
X_ELASTIC_INTERNAL_ORIGIN_REQUEST,
} from '@kbn/core-http-common';
import { RouteValidator } from './validator';
import { isSafeMethod } from './route';
import { KibanaSocket } from './socket';
Expand Down Expand Up @@ -59,7 +63,13 @@ export class CoreKibanaRequest<
withoutSecretHeaders: boolean = true
) {
const routeValidator = RouteValidator.from<P, Q, B>(routeSchemas);
const requestParts = CoreKibanaRequest.validate(req, routeValidator);
let requestParts: { params: P; query: Q; body: B };
if (isFakeRawRequest(req)) {
requestParts = { query: {} as Q, params: {} as P, body: {} as B };
} else {
const rawParts = CoreKibanaRequest.sanitizeRequest(req);
requestParts = CoreKibanaRequest.validate(rawParts, routeValidator);
}
return new CoreKibanaRequest(
req,
requestParts.params,
Expand All @@ -69,50 +79,65 @@ export class CoreKibanaRequest<
);
}

/**
* We have certain values that may be passed via query params that we want to
* exclude from further processing like validation. This method removes those
* internal values.
*/
private static sanitizeRequest<P, Q, B>(
req: Request
): { query: unknown; params: unknown; body: unknown } {
const { [ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM]: __, ...query } = req.query ?? {};
return {
query,
params: req.params,
body: req.payload,
};
}

/**
* Validates the different parts of a request based on the schemas defined for
* the route. Builds up the actual params, query and body object that will be
* received in the route handler.
* @internal
*/
private static validate<P, Q, B>(
req: RawRequest,
raw: { params: unknown; query: unknown; body: unknown },
routeValidator: RouteValidator<P, Q, B>
): {
params: P;
query: Q;
body: B;
} {
if (isFakeRawRequest(req)) {
return { query: {} as Q, params: {} as P, body: {} as B };
}
const params = routeValidator.getParams(req.params, 'request params');
const query = routeValidator.getQuery(req.query, 'request query');
const body = routeValidator.getBody(req.payload, 'request body');
const params = routeValidator.getParams(raw.params, 'request params');
const query = routeValidator.getQuery(raw.query, 'request query');
const body = routeValidator.getBody(raw.body, 'request body');
return { query, params, body };
}

/** {@inheritDoc IKibanaRequest.id} */
/** {@inheritDoc KibanaRequest.id} */
public readonly id: string;
/** {@inheritDoc IKibanaRequest.uuid} */
/** {@inheritDoc KibanaRequest.uuid} */
public readonly uuid: string;
/** {@inheritDoc IKibanaRequest.url} */
/** {@inheritDoc KibanaRequest.url} */
public readonly url: URL;
/** {@inheritDoc IKibanaRequest.route} */
/** {@inheritDoc KibanaRequest.route} */
public readonly route: RecursiveReadonly<KibanaRequestRoute<Method>>;
/** {@inheritDoc IKibanaRequest.headers} */
/** {@inheritDoc KibanaRequest.headers} */
public readonly headers: Headers;
/** {@inheritDoc IKibanaRequest.isSystemRequest} */
/** {@inheritDoc KibanaRequest.isSystemRequest} */
public readonly isSystemRequest: boolean;
/** {@inheritDoc IKibanaRequest.socket} */
/** {@inheritDoc KibanaRequest.socket} */
public readonly socket: IKibanaSocket;
/** {@inheritDoc IKibanaRequest.events} */
/** {@inheritDoc KibanaRequest.events} */
public readonly events: KibanaRequestEvents;
/** {@inheritDoc IKibanaRequest.auth} */
/** {@inheritDoc KibanaRequest.auth} */
public readonly auth: KibanaRequestAuth;
/** {@inheritDoc IKibanaRequest.isFakeRequest} */
/** {@inheritDoc KibanaRequest.isFakeRequest} */
public readonly isFakeRequest: boolean;
/** {@inheritDoc IKibanaRequest.rewrittenUrl} */
/** {@inheritDoc KibanaRequest.isInternalApiRequest} */
public readonly isInternalApiRequest: boolean;
/** {@inheritDoc KibanaRequest.rewrittenUrl} */
public readonly rewrittenUrl?: URL;

/** @internal */
Expand All @@ -139,7 +164,9 @@ export class CoreKibanaRequest<
this.headers = isRealRawRequest(request) ? deepFreeze({ ...request.headers }) : request.headers;
this.isSystemRequest = this.headers['kbn-system-request'] === 'true';
this.isFakeRequest = isFakeRawRequest(request);

this.isInternalApiRequest =
X_ELASTIC_INTERNAL_ORIGIN_REQUEST in this.headers ||
Boolean(this.url?.searchParams?.has(ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM));
// prevent Symbol exposure via Object.getOwnPropertySymbols()
Object.defineProperty(this, requestSymbol, {
value: request,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,21 @@ const createToolkit = (): ToolkitMock => {

const forgeRequest = ({
headers = {},
query = {},
path = '/',
method = 'get',
kibanaRouteOptions,
}: Partial<{
headers: Record<string, string>;
query: Record<string, string>;
path: string;
method: RouteMethod;
kibanaRouteOptions: KibanaRouteOptions;
}>): KibanaRequest => {
return mockRouter.createKibanaRequest({
headers,
path,
query,
method,
kibanaRouteOptions,
});
Expand Down Expand Up @@ -259,11 +262,13 @@ describe('restrictInternal post-auth handler', () => {
});
const createForgeRequest = (
access: 'internal' | 'public',
headers: Record<string, string> | undefined = {}
headers: Record<string, string> | undefined = {},
query: Record<string, string> | undefined = {}
) => {
return forgeRequest({
method: 'get',
headers,
query,
path: `/${access}/some-path`,
kibanaRouteOptions: {
xsrfRequired: false,
Expand Down Expand Up @@ -318,6 +323,24 @@ describe('restrictInternal post-auth handler', () => {
const request = createForgeRequest('public');
createForwardSuccess(handler, request);
});

it('forward the request to the next interceptor if called with internal origin query param for internal API', () => {
const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
const request = createForgeRequest('internal', undefined, { elasticInternalOrigin: 'true' });
createForwardSuccess(handler, request);
});

it('forward the request to the next interceptor if called with internal origin query param for public APIs', () => {
const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
const request = createForgeRequest('internal', undefined, { elasticInternalOrigin: 'true' });
createForwardSuccess(handler, request);
});

it('forward the request to the next interceptor if called without internal origin query param for public APIs', () => {
const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
const request = createForgeRequest('public');
createForwardSuccess(handler, request);
});
});

describe('when restriction is not enabled', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import type { OnPostAuthHandler, OnPreResponseHandler } from '@kbn/core-http-server';
import { isSafeMethod } from '@kbn/core-http-router-server-internal';
import { X_ELASTIC_INTERNAL_ORIGIN_REQUEST } from '@kbn/core-http-common/src/constants';
import { HttpConfig } from './http_config';

const VERSION_HEADER = 'kbn-version';
Expand Down Expand Up @@ -45,11 +44,7 @@ export const createRestrictInternalRoutesPostAuthHandler = (

return (request, response, toolkit) => {
const isInternalRoute = request.route.options.access === 'internal';

// only check if the header is present, not it's content.
const hasInternalKibanaRequestHeader = X_ELASTIC_INTERNAL_ORIGIN_REQUEST in request.headers;

if (isRestrictionEnabled && isInternalRoute && !hasInternalKibanaRequestHeader) {
if (isRestrictionEnabled && isInternalRoute && !request.isInternalApiRequest) {
// throw 400
return response.badRequest({
body: `uri [${request.url}] with method [${request.route.method}] exists but is not available with the current configuration`,
Expand All @@ -75,7 +70,6 @@ export const createVersionCheckPostAuthHandler = (kibanaVersion: string): OnPost
},
});
}

return toolkit.next();
};
};
Expand Down
6 changes: 6 additions & 0 deletions packages/core/http/core-http-server/src/router/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ export interface KibanaRequest<
*/
readonly isFakeRequest: boolean;

/**
* An internal request has access to internal routes.
* @note See the {@link KibanaRequestRouteOptions#access} route option.
*/
readonly isInternalApiRequest: boolean;

/**
* The socket associated with this request.
* See {@link IKibanaSocket}.
Expand Down
5 changes: 5 additions & 0 deletions packages/kbn-hapi-mocks/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ export const createRequestMock = (customization: DeepPartial<Request> = {}): Req
formatUrl(Object.assign({ pathname, path, href: path }, customization.url)),
'http://localhost'
);
if (customization.query) {
Object.entries(customization.query).forEach(([key, value]) => {
url.searchParams.set(key, value);
});
}

return merge(
{},
Expand Down
Loading

0 comments on commit 23d3955

Please sign in to comment.