Skip to content
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

[Legacy] Route payload must be set to 'parse' when payload validation enabled #57777

Closed
afharo opened this issue Feb 17, 2020 · 14 comments
Closed
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@afharo
Copy link
Member

afharo commented Feb 17, 2020

Kibana version: 7.6.0

Elasticsearch version: 7.6.0

Server OS version: Any

Browser version: Any

Browser OS version: Any

Original install method (e.g. download page, yum, from source, etc.): Any

Describe the bug:
This error is happens on startup when registering the linked plugin

AssertionError [ERR_ASSERTION]: Route payload must be set to 'parse' when payload validation enabled: POST /api/elastalert/{path*}

bitsensor/elastalert-kibana-plugin#141

Steps to reproduce:

  1. Register an api like https://github.com/bitsensor/elastalert-kibana-plugin/blob/master/server/routes/elastalert.js in the legacy world
  2. Start the kibana and see the crash

Expected behavior:
It should work as it was working back in 7.5.x (maybe this is the change that broke this behaviour?): #48753

Screenshots (if relevant):
Not relevant

Errors in browser console (if relevant):
Not relevant

Provide logs and/or server output (if relevant):

{"type":"log","@timestamp":"2020-02-12T20:52:54Z","tags":["fatal","root"],"pid":18,"message":"{ AssertionError [ERR_ASSERTION]: Route payload must be set to 'parse' when payload validation enabled: POST /api/elastalert/{path*} at new AssertionError (internal/assert.js:269:11) at Object.exports.assert (/usr/share/kibana/node_modules/hoek/lib/index.js:736:11) at new module.exports.internals.Route (/usr/share/kibana/node_modules/hapi/lib/route.js:166:14) at internals.Server._addRoute (/usr/share/kibana/node_modules/hapi/lib/server.js:463:23) at internals.Server.route (/usr/share/kibana/node_modules/hapi/lib/server.js:452:26) at _default (/usr/share/kibana/plugins/elastalert-kibana-plugin/server/routes/elastalert.js:5:10) at Plugin.init [as externalInit] (/usr/share/kibana/plugins/elastalert-kibana-plugin/index.js:28:7) at Object.register (/usr/share/kibana/src/legacy/server/plugins/lib/plugin.js:96:20) at internals.Server.register (/usr/share/kibana/node_modules/hapi/lib/server.js:431:35) at Plugin.init (/usr/share/kibana/src/legacy/server/plugins/lib/plugin.js:100:28) at Plugin.init (/usr/share/kibana/node_modules/lodash/ind ex.js:7411:25) at callPluginHook (/usr/share/kibana/src/legacy/server/plugins/lib/call_plugin_hook.js:53:25) generatedMessage: false, name: 'AssertionError [ERR_ASSERTION]', code: 'ERR_ASSERTION', actual: false, expected: true, operator: '==' }"}

Any additional context:
@pgayvallet and @legrego had an initial discussion about this.

@afharo afharo added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Feb 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@aristenio
Copy link

I'm with the same problem to use the ElastAlert plugin on Kibana 7.6.0.

@nerddelphi
Copy link

Hi @pgayvallet and @afharo .

Is there any update in that issue? Thanks.

@mmguero
Copy link

mmguero commented Mar 20, 2020

👍 same problem, any update?

@malcolm061990
Copy link

👍 same problem, any update?

@maoqin
Copy link

maoqin commented Apr 3, 2020

👍 same problem, any update?

@pgayvallet
Copy link
Contributor

Hello,

First of all, my apologies for the lack of answer on this issue.

Due to security concerns, default payload validation has been enabled for every route registered on the kibana hapi server, legacy routes included. I can confirm that this was introduced in #48753.

I will be honest, after a few tests, I'm not really sure how/why it broke routes registered from some 3rd parties such as elastalert-kibana-plugin, as no 1st party legacy plugins seems to be impacted. I initially thought it was cause by the route registering multiple methods at the same time (no internal routes are registered that way), but even when editing elastalert-kibana-plugin/server/routes/elastalert.js to only register POST, the error is still present.

Only solution I can think of for now is to manually disable payload validation on 'faulty' routes. This has to be done on the plugin's side.

to take the elastalert-kibana-plugin example:

server.route({
    path: '/api/elastalert/{path*}',
    method: ['GET', 'POST', 'DELETE'],
    handler: {
      proxy: {
        mapUri: request => {
          return { uri: `${baseUri}/${request.params.path || ''}` };
        }
      }
    }
  });

would have to be adapted to something like:

const handler = {
    proxy: {
      mapUri: request => {
        return { uri: `${baseUri}/${request.params.path || ''}` };
      }
}

['GET', 'POST', 'DELETE'].forEach(method => {
   server.route({
     path: '/api/elastalert/{path*}',
     method,
     handler,
     config:
       method === 'GET'
         ? undefined
         : {
             validate: { payload: null },
             payload: { parse: false },
           },
    });
});

By testing this locally, the error goes away.

Please note that as this is not impacting any internal plugins, and as 3rd party plugins support is an experimental API, this is not considered breaking.

However if someone with some hapi knowledge wants to tackle this without removing the security validation introduced in #48753, this would be very welcome.

@mmguero
Copy link

mmguero commented Apr 3, 2020

I found a commit in the fork of @aristenio where he adds Kibana 7.6.0 support this way.

@nerddelphi
Copy link

@mmguero Have you tested this fork/commit? If yes, you could release one zip for us.

@mmguero
Copy link

mmguero commented Apr 3, 2020

testing it out now, i'll let you know

@mmguero
Copy link

mmguero commented Apr 3, 2020

So today I learned I had no idea how to package one of these up into a zip file for distribution. I did some research and build myself a docker build environment (based on https://github.com/blacktop/kibana-plugin-builder) and npm run build does create a .zip file but none of the dependencies are present (no node_modules directory). Anyway I'll keep playing with it and let you know when i figure it out.

@maoqin
Copy link

maoqin commented Apr 4, 2020

@pgayvallet thank you so much for your time!
You have lost a closing parenthesis on line 6, after fixing that, it works fine!

@mmguero
Copy link

mmguero commented Apr 4, 2020

@pgayvallet Thank you, I tested your example in the bitsensor elastalert kibana plugin with good results, as well.

@joshdover
Copy link
Contributor

This issue only affected legacy plugins which are no longer supported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

9 participants