-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow disabling xsrf protection per an endpoint #58717
Changes from 11 commits
40a6419
6b3b447
b466852
263689d
a8b9f09
081527c
5d4a98b
3411fb1
5e54a83
7fcf2c8
016f664
3868f71
07d7248
e7829d9
a5c12a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<!-- Do not edit this file. It is automatically generated by API Documenter. --> | ||
|
||
[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [RouteConfigOptions](./kibana-plugin-server.routeconfigoptions.md) > [xsrfRequired](./kibana-plugin-server.routeconfigoptions.xsrfrequired.md) | ||
|
||
## RouteConfigOptions.xsrfRequired property | ||
|
||
Defines xsrf protection requirements for a route: - true. Requires an incoming request to contain `kbn-xsrf` header. - false. Disables xsrf protection. | ||
|
||
Set to true by default | ||
|
||
<b>Signature:</b> | ||
|
||
```typescript | ||
xsrfRequired?: boolean; | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 } from './router'; | ||
import { | ||
SessionStorageCookieOptions, | ||
createCookieSessionStorageFactory, | ||
|
@@ -148,15 +148,18 @@ export class HttpServer { | |
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 { authRequired = true, tags, body = {} } = route.options; | ||
const { authRequired = true, tags, body = {}, xsrfRequired = true } = route.options; | ||
const { accepts: allow, maxBytes, output, parse } = body; | ||
const kibanaRouteState: KibanaRouteState = { xsrfRequired }; | ||
|
||
this.server.route({ | ||
handler: route.handler, | ||
method: route.method, | ||
path: route.path, | ||
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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: can't this be inlined to remove the local var? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can, but I want to make sure that app: { xsrfRequired } as 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,11 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler | |
const { whitelist, disableProtection } = config.xsrf; | ||
|
||
return (request, response, toolkit) => { | ||
if (disableProtection || whitelist.includes(request.route.path)) { | ||
if ( | ||
disableProtection || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that we added |
||
whitelist.includes(request.route.path) || | ||
request.route.options.xsrfRequired === false | ||
) { | ||
return toolkit.next(); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdover what is the appropriate place to track v8 breaking changes after #40768 was closed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this doc:
docs/migration/migrate_8_0.asciidoc