Allow disabling xsrf protection per an endpoint (#58717) (#59151)

* add xsrfRequired flag to a route definition interface

* update tests

* deprecate server.xsrf.whitelist

It meant to be used for IdP endpoints only, which we are going to refactor to disable xsrf requirement per a specific endpoint.

* update docs

* do not fail on manual KibanaRequest creation

* address comments

* update tests

* address comments

* make xsrfRequired available only for destructive methods

* update docs

* another isSafeMethod usage
This commit is contained in:
Mikhail Shustov 2020-03-03 17:47:13 +01:00 committed by GitHub
parent e099793ae5
commit 0fdb6e520a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 185 additions and 14 deletions

View file

@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
[Home](./index.md) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [DestructiveRouteMethod](./kibana-plugin-server.destructiveroutemethod.md)
## DestructiveRouteMethod type
Set of HTTP methods changing the state of the server.
<b>Signature:</b>
```typescript
export declare type DestructiveRouteMethod = 'post' | 'put' | 'delete' | 'patch';
```

View file

@ -188,6 +188,7 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
| [ConfigDeprecationLogger](./kibana-plugin-server.configdeprecationlogger.md) | Logger interface used when invoking a [ConfigDeprecation](./kibana-plugin-server.configdeprecation.md) |
| [ConfigDeprecationProvider](./kibana-plugin-server.configdeprecationprovider.md) | A provider that should returns a list of [ConfigDeprecation](./kibana-plugin-server.configdeprecation.md)<!-- -->.<!-- -->See [ConfigDeprecationFactory](./kibana-plugin-server.configdeprecationfactory.md) for more usage examples. |
| [ConfigPath](./kibana-plugin-server.configpath.md) | |
| [DestructiveRouteMethod](./kibana-plugin-server.destructiveroutemethod.md) | Set of HTTP methods changing the state of the server. |
| [ElasticsearchClientConfig](./kibana-plugin-server.elasticsearchclientconfig.md) | |
| [GetAuthHeaders](./kibana-plugin-server.getauthheaders.md) | Get headers to authenticate a user against Elasticsearch. |
| [GetAuthState](./kibana-plugin-server.getauthstate.md) | Gets authentication state for a request. Returned by <code>auth</code> interceptor. |
@ -232,6 +233,7 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
| [RouteValidationFunction](./kibana-plugin-server.routevalidationfunction.md) | The custom validation function if @<!-- -->kbn/config-schema is not a valid solution for your specific plugin requirements. |
| [RouteValidationSpec](./kibana-plugin-server.routevalidationspec.md) | Allowed property validation options: either @<!-- -->kbn/config-schema validations or custom validation functions<!-- -->See [RouteValidationFunction](./kibana-plugin-server.routevalidationfunction.md) for custom validation. |
| [RouteValidatorFullConfig](./kibana-plugin-server.routevalidatorfullconfig.md) | Route validations config and options merged into one object |
| [SafeRouteMethod](./kibana-plugin-server.saferoutemethod.md) | Set of HTTP methods not changing the state of the server. |
| [SavedObjectAttribute](./kibana-plugin-server.savedobjectattribute.md) | Type definition for a Saved Object attribute value |
| [SavedObjectAttributeSingle](./kibana-plugin-server.savedobjectattributesingle.md) | Don't use this type, it's simply a helper type for [SavedObjectAttribute](./kibana-plugin-server.savedobjectattribute.md) |
| [SavedObjectMigrationFn](./kibana-plugin-server.savedobjectmigrationfn.md) | A migration function for a [saved object type](./kibana-plugin-server.savedobjectstype.md) used to migrate it to a given version |

View file

@ -19,4 +19,5 @@ export interface RouteConfigOptions<Method extends RouteMethod>
| [authRequired](./kibana-plugin-server.routeconfigoptions.authrequired.md) | <code>boolean</code> | A flag shows that authentication for a route: <code>enabled</code> when true <code>disabled</code> when false<!-- -->Enabled by default. |
| [body](./kibana-plugin-server.routeconfigoptions.body.md) | <code>Method extends 'get' &#124; 'options' ? undefined : RouteConfigOptionsBody</code> | Additional body options [RouteConfigOptionsBody](./kibana-plugin-server.routeconfigoptionsbody.md)<!-- -->. |
| [tags](./kibana-plugin-server.routeconfigoptions.tags.md) | <code>readonly string[]</code> | Additional metadata tag strings to attach to the route. |
| [xsrfRequired](./kibana-plugin-server.routeconfigoptions.xsrfrequired.md) | <code>Method extends 'get' ? never : boolean</code> | Defines xsrf protection requirements for a route: - true. Requires an incoming POST/PUT/DELETE request to contain <code>kbn-xsrf</code> header. - false. Disables xsrf protection.<!-- -->Set to true by default |

View file

@ -0,0 +1,15 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
[Home](./index.md) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [RouteConfigOptions](./kibana-plugin-server.routeconfigoptions.md) &gt; [xsrfRequired](./kibana-plugin-server.routeconfigoptions.xsrfrequired.md)
## RouteConfigOptions.xsrfRequired property
Defines xsrf protection requirements for a route: - true. Requires an incoming POST/PUT/DELETE request to contain `kbn-xsrf` header. - false. Disables xsrf protection.
Set to true by default
<b>Signature:</b>
```typescript
xsrfRequired?: Method extends 'get' ? never : boolean;
```

View file

@ -9,5 +9,5 @@ The set of common HTTP methods supported by Kibana routing.
<b>Signature:</b>
```typescript
export declare type RouteMethod = 'get' | 'post' | 'put' | 'delete' | 'patch' | 'options';
export declare type RouteMethod = SafeRouteMethod | DestructiveRouteMethod;
```

View file

@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
[Home](./index.md) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [SafeRouteMethod](./kibana-plugin-server.saferoutemethod.md)
## SafeRouteMethod type
Set of HTTP methods not changing the state of the server.
<b>Signature:</b>
```typescript
export declare type SafeRouteMethod = 'get' | 'options';
```

View file

@ -81,6 +81,19 @@ describe('core deprecations', () => {
});
});
describe('xsrfDeprecation', () => {
it('logs a warning if server.xsrf.whitelist is set', () => {
const { messages } = applyCoreDeprecations({
server: { xsrf: { whitelist: ['/path'] } },
});
expect(messages).toMatchInlineSnapshot(`
Array [
"It is not recommended to disable xsrf protections for API endpoints via [server.xsrf.whitelist]. It will be removed in 8.0 release. Instead, supply the \\"kbn-xsrf\\" header.",
]
`);
});
});
describe('rewriteBasePath', () => {
it('logs a warning is server.basePath is set and server.rewriteBasePath is not', () => {
const { messages } = applyCoreDeprecations({

View file

@ -38,6 +38,19 @@ const dataPathDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
return settings;
};
const xsrfDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
if (
has(settings, 'server.xsrf.whitelist') &&
get<unknown[]>(settings, 'server.xsrf.whitelist').length > 0
) {
log(
'It is not recommended to disable xsrf protections for API endpoints via [server.xsrf.whitelist]. ' +
'It will be removed in 8.0 release. Instead, supply the "kbn-xsrf" header.'
);
}
return settings;
};
const rewriteBasePathDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
if (has(settings, 'server.basePath') && !has(settings, 'server.rewriteBasePath')) {
log(
@ -177,4 +190,5 @@ export const coreDeprecationProvider: ConfigDeprecationProvider = ({
rewriteBasePathDeprecation,
cspRulesDeprecation,
mapManifestServiceUrlDeprecation,
xsrfDeprecation,
];

View file

@ -29,6 +29,7 @@ import {
RouteMethod,
KibanaResponseFactory,
RouteValidationSpec,
KibanaRouteState,
} from './router';
import { OnPreResponseToolkit } from './lifecycle/on_pre_response';
import { OnPostAuthToolkit } from './lifecycle/on_post_auth';
@ -43,6 +44,7 @@ interface RequestFixtureOptions<P = any, Q = any, B = any> {
method?: RouteMethod;
socket?: Socket;
routeTags?: string[];
kibanaRouteState?: KibanaRouteState;
routeAuthRequired?: false;
validation?: {
params?: RouteValidationSpec<P>;
@ -62,6 +64,7 @@ function createKibanaRequestMock<P = any, Q = any, B = any>({
routeTags,
routeAuthRequired,
validation = {},
kibanaRouteState = { xsrfRequired: true },
}: RequestFixtureOptions<P, Q, B> = {}) {
const queryString = stringify(query, { sort: false });
@ -80,7 +83,7 @@ function createKibanaRequestMock<P = any, Q = any, B = any>({
search: queryString ? `?${queryString}` : queryString,
},
route: {
settings: { tags: routeTags, auth: routeAuthRequired },
settings: { tags: routeTags, auth: routeAuthRequired, app: kibanaRouteState },
},
raw: {
req: { socket },
@ -109,6 +112,7 @@ function createRawRequestMock(customization: DeepPartial<Request> = {}) {
return merge(
{},
{
app: { xsrfRequired: true } as any,
headers: {},
path: '/',
route: { settings: {} },

View file

@ -811,6 +811,7 @@ test('exposes route details of incoming request to a route handler', async () =>
path: '/',
options: {
authRequired: true,
xsrfRequired: false,
tags: [],
},
});
@ -923,6 +924,7 @@ test('exposes route details of incoming request to a route handler (POST + paylo
path: '/',
options: {
authRequired: true,
xsrfRequired: true,
tags: [],
body: {
parse: true, // hapi populates the default

View file

@ -27,7 +27,7 @@ import { adoptToHapiOnPostAuthFormat, OnPostAuthHandler } from './lifecycle/on_p
import { adoptToHapiOnPreAuthFormat, OnPreAuthHandler } from './lifecycle/on_pre_auth';
import { adoptToHapiOnPreResponseFormat, OnPreResponseHandler } from './lifecycle/on_pre_response';
import { IRouter } from './router';
import { IRouter, KibanaRouteState, isSafeMethod } from './router';
import {
SessionStorageCookieOptions,
createCookieSessionStorageFactory,
@ -147,9 +147,14 @@ export class HttpServer {
for (const route of router.getRoutes()) {
this.log.debug(`registering route handler for [${route.path}]`);
// Hapi does not allow payload validation to be specified for 'head' or 'get' requests
const validate = ['head', 'get'].includes(route.method) ? undefined : { payload: true };
const validate = isSafeMethod(route.method) ? undefined : { payload: true };
const { authRequired = true, tags, body = {} } = route.options;
const { accepts: allow, maxBytes, output, parse } = body;
const kibanaRouteState: KibanaRouteState = {
xsrfRequired: route.options.xsrfRequired ?? !isSafeMethod(route.method),
};
this.server.route({
handler: route.handler,
method: route.method,
@ -157,6 +162,7 @@ export class HttpServer {
options: {
// Enforcing the comparison with true because plugins could overwrite the auth strategy by doing `options: { authRequired: authStrategy as any }`
auth: authRequired === true ? undefined : false,
app: kibanaRouteState,
tags: tags ? Array.from(tags) : undefined,
// TODO: This 'validate' section can be removed once the legacy platform is completely removed.
// We are telling Hapi that NP routes can accept any payload, so that it can bypass the default

View file

@ -58,6 +58,8 @@ export {
RouteValidationError,
RouteValidatorFullConfig,
RouteValidationResultFactory,
DestructiveRouteMethod,
SafeRouteMethod,
} from './router';
export { BasePathProxyServer } from './base_path_proxy_server';
export { OnPreAuthHandler, OnPreAuthToolkit } from './lifecycle/on_pre_auth';

View file

@ -36,6 +36,7 @@ const versionHeader = 'kbn-version';
const xsrfHeader = 'kbn-xsrf';
const nameHeader = 'kbn-name';
const whitelistedTestPath = '/xsrf/test/route/whitelisted';
const xsrfDisabledTestPath = '/xsrf/test/route/disabled';
const kibanaName = 'my-kibana-name';
const setupDeps = {
context: contextServiceMock.createSetupContract(),
@ -188,6 +189,12 @@ describe('core lifecycle handlers', () => {
return res.ok({ body: 'ok' });
}
);
((router as any)[method.toLowerCase()] as RouteRegistrar<any>)<any, any, any>(
{ path: xsrfDisabledTestPath, validate: false, options: { xsrfRequired: false } },
(context, req, res) => {
return res.ok({ body: 'ok' });
}
);
});
await server.start();
@ -235,6 +242,10 @@ describe('core lifecycle handlers', () => {
it('accepts whitelisted requests without either an xsrf or version header', async () => {
await getSupertest(method.toLowerCase(), whitelistedTestPath).expect(200, 'ok');
});
it('accepts requests on a route with disabled xsrf protection', async () => {
await getSupertest(method.toLowerCase(), xsrfDisabledTestPath).expect(200, 'ok');
});
});
});
});

View file

@ -24,7 +24,7 @@ import {
} from './lifecycle_handlers';
import { httpServerMock } from './http_server.mocks';
import { HttpConfig } from './http_config';
import { KibanaRequest, RouteMethod } from './router';
import { KibanaRequest, RouteMethod, KibanaRouteState } from './router';
const createConfig = (partial: Partial<HttpConfig>): HttpConfig => partial as HttpConfig;
@ -32,12 +32,14 @@ const forgeRequest = ({
headers = {},
path = '/',
method = 'get',
kibanaRouteState,
}: Partial<{
headers: Record<string, string>;
path: string;
method: RouteMethod;
kibanaRouteState: KibanaRouteState;
}>): KibanaRequest => {
return httpServerMock.createKibanaRequest({ headers, path, method });
return httpServerMock.createKibanaRequest({ headers, path, method, kibanaRouteState });
};
describe('xsrf post-auth handler', () => {
@ -142,6 +144,29 @@ describe('xsrf post-auth handler', () => {
expect(toolkit.next).toHaveBeenCalledTimes(1);
expect(result).toEqual('next');
});
it('accepts requests if xsrf protection on a route is disabled', () => {
const config = createConfig({
xsrf: { whitelist: [], disableProtection: false },
});
const handler = createXsrfPostAuthHandler(config);
const request = forgeRequest({
method: 'post',
headers: {},
path: '/some-path',
kibanaRouteState: {
xsrfRequired: false,
},
});
toolkit.next.mockReturnValue('next' as any);
const result = handler(request, responseFactory, toolkit);
expect(responseFactory.badRequest).not.toHaveBeenCalled();
expect(toolkit.next).toHaveBeenCalledTimes(1);
expect(result).toEqual('next');
});
});
});

View file

@ -20,6 +20,7 @@
import { OnPostAuthHandler } from './lifecycle/on_post_auth';
import { OnPreResponseHandler } from './lifecycle/on_pre_response';
import { HttpConfig } from './http_config';
import { isSafeMethod } from './router';
import { Env } from '../config';
import { LifecycleRegistrar } from './http_server';
@ -31,15 +32,18 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler
const { whitelist, disableProtection } = config.xsrf;
return (request, response, toolkit) => {
if (disableProtection || whitelist.includes(request.route.path)) {
if (
disableProtection ||
whitelist.includes(request.route.path) ||
request.route.options.xsrfRequired === false
) {
return toolkit.next();
}
const isSafeMethod = request.route.method === 'get' || request.route.method === 'head';
const hasVersionHeader = VERSION_HEADER in request.headers;
const hasXsrfHeader = XSRF_HEADER in request.headers;
if (!isSafeMethod && !hasVersionHeader && !hasXsrfHeader) {
if (!isSafeMethod(request.route.method) && !hasVersionHeader && !hasXsrfHeader) {
return response.badRequest({ body: `Request must contain a ${XSRF_HEADER} header.` });
}

View file

@ -24,16 +24,20 @@ export {
KibanaRequestEvents,
KibanaRequestRoute,
KibanaRequestRouteOptions,
KibanaRouteState,
isRealRequest,
LegacyRequest,
ensureRawRequest,
} from './request';
export {
DestructiveRouteMethod,
isSafeMethod,
RouteMethod,
RouteConfig,
RouteConfigOptions,
RouteContentType,
RouteConfigOptionsBody,
SafeRouteMethod,
validBodyOutput,
} from './route';
export { HapiResponseAdapter } from './response_adapter';

View file

@ -18,18 +18,24 @@
*/
import { Url } from 'url';
import { Request } from 'hapi';
import { Request, ApplicationState } from 'hapi';
import { Observable, fromEvent, merge } from 'rxjs';
import { shareReplay, first, takeUntil } from 'rxjs/operators';
import { deepFreeze, RecursiveReadonly } from '../../../utils';
import { Headers } from './headers';
import { RouteMethod, RouteConfigOptions, validBodyOutput } from './route';
import { RouteMethod, RouteConfigOptions, validBodyOutput, isSafeMethod } from './route';
import { KibanaSocket, IKibanaSocket } from './socket';
import { RouteValidator, RouteValidatorFullConfig } from './validator';
const requestSymbol = Symbol('request');
/**
* @internal
*/
export interface KibanaRouteState extends ApplicationState {
xsrfRequired: boolean;
}
/**
* Route options: If 'GET' or 'OPTIONS' method, body options won't be returned.
* @public
@ -184,8 +190,10 @@ export class KibanaRequest<
const options = ({
authRequired: request.route.settings.auth !== false,
// some places in LP call KibanaRequest.from(request) manually. remove fallback to true before v8
xsrfRequired: (request.route.settings.app as KibanaRouteState)?.xsrfRequired ?? true,
tags: request.route.settings.tags || [],
body: ['get', 'options'].includes(method)
body: isSafeMethod(method)
? undefined
: {
parse,

View file

@ -19,11 +19,27 @@
import { RouteValidatorFullConfig } from './validator';
export function isSafeMethod(method: RouteMethod): method is SafeRouteMethod {
return method === 'get' || method === 'options';
}
/**
* Set of HTTP methods changing the state of the server.
* @public
*/
export type DestructiveRouteMethod = 'post' | 'put' | 'delete' | 'patch';
/**
* Set of HTTP methods not changing the state of the server.
* @public
*/
export type SafeRouteMethod = 'get' | 'options';
/**
* The set of common HTTP methods supported by Kibana routing.
* @public
*/
export type RouteMethod = 'get' | 'post' | 'put' | 'delete' | 'patch' | 'options';
export type RouteMethod = SafeRouteMethod | DestructiveRouteMethod;
/**
* The set of valid body.output
@ -108,6 +124,15 @@ export interface RouteConfigOptions<Method extends RouteMethod> {
*/
authRequired?: boolean;
/**
* Defines xsrf protection requirements for a route:
* - true. Requires an incoming POST/PUT/DELETE request to contain `kbn-xsrf` header.
* - false. Disables xsrf protection.
*
* Set to true by default
*/
xsrfRequired?: Method extends 'get' ? never : boolean;
/**
* Additional metadata tag strings to attach to the route.
*/

View file

@ -159,6 +159,8 @@ export {
SessionStorageCookieOptions,
SessionCookieValidationResult,
SessionStorageFactory,
DestructiveRouteMethod,
SafeRouteMethod,
} from './http';
export { RenderingServiceSetup, IRenderOptions } from './rendering';
export { Logger, LoggerFactory, LogMeta, LogRecord, LogLevel } from './logging';

View file

@ -685,6 +685,9 @@ export interface DeprecationSettings {
message: string;
}
// @public
export type DestructiveRouteMethod = 'post' | 'put' | 'delete' | 'patch';
// @public
export interface DiscoveredPlugin {
readonly configPath: ConfigPath;
@ -1459,6 +1462,7 @@ export interface RouteConfigOptions<Method extends RouteMethod> {
authRequired?: boolean;
body?: Method extends 'get' | 'options' ? undefined : RouteConfigOptionsBody;
tags?: readonly string[];
xsrfRequired?: Method extends 'get' ? never : boolean;
}
// @public
@ -1473,7 +1477,7 @@ export interface RouteConfigOptionsBody {
export type RouteContentType = 'application/json' | 'application/*+json' | 'application/octet-stream' | 'application/x-www-form-urlencoded' | 'multipart/form-data' | 'text/*';
// @public
export type RouteMethod = 'get' | 'post' | 'put' | 'delete' | 'patch' | 'options';
export type RouteMethod = SafeRouteMethod | DestructiveRouteMethod;
// @public
export type RouteRegistrar<Method extends RouteMethod> = <P, Q, B>(route: RouteConfig<P, Q, B, Method>, handler: RequestHandler<P, Q, B, Method>) => void;
@ -1526,6 +1530,9 @@ export interface RouteValidatorOptions {
};
}
// @public
export type SafeRouteMethod = 'get' | 'options';
// @public (undocumented)
export interface SavedObject<T = unknown> {
attributes: T;