search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.67k stars 8.23k forks source link

[Authz] Removed security property from route.options types #201352

Closed elena-shostak closed 1 hour ago

elena-shostak commented 6 hours ago

Summary

Removed security property from route.options types, security should exist only as a top level property. Fixed routes with incorrect config accordingly.

Routes Impacted

Routes with disabled authorization (impact can be considered negligible)

/internal/entities/managed/enablement
/internal/entities/managed/enablement
/internal/entities/managed/enablement
/internal/entities/definition
/internal/entities/definition/{id}
/internal/entities/definition/{id?}
/internal/entities/definition/{id}/_reset
/internal/entities/definition/{id}
/api/streams/_enable
/api/streams/_resync
/api/streams/{id}/_fork
/api/streams/{id}
/api/streams/{id}
/api/streams/{id}
/api/streams

Routes with authorization (will be backported to 8.17.0)

/internal/product_doc_base/status
/internal/product_doc_base/install
/internal/product_doc_base/uninstall

Fixes: https://github.com/elastic/kibana/issues/201347

elena-shostak commented 6 hours ago

/ci

elena-shostak commented 6 hours ago

/ci

elena-shostak commented 5 hours ago

/ci

elena-shostak commented 5 hours ago

/ci

elena-shostak commented 5 hours ago

/ci

elena-shostak commented 4 hours ago

/ci

elasticmachine commented 4 hours ago

Pinging @elastic/kibana-security (Team:Security)

elena-shostak commented 3 hours ago

I believe you can revert the changes to the server-route-repository package because @dgieselaar has a PR that already adds similar changes to fix this for us 👍🏼

Can do, but gonna still need this line https://github.com/elastic/kibana/pull/201352/files#diff-60d59ab0267a595b6da4a6db239b07ed334cea5a76e4a6fbd28e883bc866e3adR281, otherwise type check will fail

And since I've added a runtime error, I gonna still need other changes as well

miltonhultgren commented 3 hours ago

@elena-shostak I'm not sure I understand why the type check will fail without that? Are there routes using that package that send in options.security? Then those are likely all bugs that should be using just security? But maybe we have to clean that up in a follow up PR instead.

elena-shostak commented 3 hours ago

@miltonhultgren I removed the security in options, but types in your package are referencing it here. You can see that in this failed build once I dropped security

Is that okay if we clean it up as follow up then?

And runtime error I've added to the router would not allow me to revert the changes

miltonhultgren commented 2 hours ago

Right, I see, we'll clean it up on our side after this PR is merged like Dario mentioned here 👍🏼

elasticmachine commented 1 hour ago

:yellow_heart: Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Unknown metric groups #### API count | id | [before](https://github.com/elastic/kibana/commit/11d8f0c375c8de8bb6011087ebc455190f61f5ef) | [after](https://github.com/elastic/kibana/commit/0e0140780b63da85bf7be96a52d09ef0e0b53843) | diff | | --- | --- | --- | --- | | `@kbn/core-http-server` | 568 | 567 | -1 |

History

kibanamachine commented 1 hour ago

Starting backport for target branches: 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/11976148644

kibanamachine commented 1 hour ago

💚 All backports created successfully

Status Branch Result
✅ 8.17
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation