-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Unauthorized route migration for routes owned by kibana-visualizations,kibana-data-discovery #198331
Unauthorized route migration for routes owned by kibana-visualizations,kibana-data-discovery #198331
Conversation
…ibana-data-discovery
Pinging @elastic/kibana-visualizations (Team:Visualizations) |
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
I guess search sessions shouldn't be excluded from authorization?
|
…a-visualizations_kibana-data-discovery
…a-visualizations_kibana-data-discovery
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.
Code changes LGTM. I also tested saved query management with a variety of privileges, and everything seemed to behave as expected 👍 Thanks for addressing this!
security: { | ||
authz: { | ||
enabled: false, | ||
reason: 'This route is opted out from authorization', |
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.
Could we please elaborate on the reasoning there?
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.
@lukasolson Maybe something along the lines of "It should be possible to collect KQL usage telemetry for all users regardless of their privileges"?
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.
but not all users are able to call this endpoint right?
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 don't want to mislead so maybe @lukasolson can confirm, but I believe they can. It's called whenever a user changes their filter language preference in Unified Search from anywhere in Kibana, which doesn't have any privileges around it:
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.
This endpoint needs to be removed. This is the sort of thing I think we were trying to catch with this PR to begin with - we are using the internal Kibana user to create a saved object when any user invokes this endpoint, which is not ideal. I'll open up a follow-up issue for this
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.
Opened #202518.
…a-visualizations_kibana-data-discovery
@@ -197,6 +198,7 @@ export class MapsPlugin implements Plugin { | |||
read: [MAP_SAVED_OBJECT_TYPE, 'index-pattern', 'query', 'tag'], | |||
}, | |||
ui: ['show'], | |||
api: ['savedQuery:read'], |
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.
Does this mean you can only access maps application if you have the role 'savedQuery:read'? I am not sure that restriction makes sense.
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.
No, it means that by having access to the maps application you are automatically granted access to APIs that require savedQuery:read
privileges.
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.
thanks for clarifying
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.
kibana-presentation changes LGTM
code review only
…a-visualizations_kibana-data-discovery
…a-visualizations_kibana-data-discovery
…a-visualizations_kibana-data-discovery
…a-visualizations_kibana-data-discovery
💚 Build Succeeded
Metrics [docs]
History
cc @lukasolson |
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/12169460505 |
…s,kibana-data-discovery (elastic#198331) ### Authz API migration for unauthorized routes This PR migrates unauthorized routes owned by your team to a new security configuration. Please refer to the documentation for more information: [Authorization API](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization) --- EDIT --- This PR also adds two privileges related to saved query APIs: `savedQuery:read` and `savedQuery:manage`. These are given by default to the same roles that already have access to the `query`-type saved objects. ### **Before migration:** ```ts router.get({ path: '/api/path', ... }, handler); ``` ### **After migration:** ```ts router.get({ path: '/api/path', security: { authz: { enabled: false, reason: 'This route is opted out from authorization because ...', }, }, ... }, handler); ``` ### What to do next? 1. Review the changes in this PR. 2. Elaborate on the reasoning to opt-out of authorization. 3. Routes without a compelling reason to opt-out of authorization should plan to introduce them as soon as possible. 2. You might need to update your tests to reflect the new security configuration: - If you have snapshot tests that include the route definition. ## Any questions? If you have any questions or need help with API authorization, please reach out to the `@elastic/kibana-security` team. --------- Co-authored-by: Lukas Olson <[email protected]> Co-authored-by: Matthias Wilhelm <[email protected]> Co-authored-by: Marta Bondyra <[email protected]> Co-authored-by: Davis McPhee <[email protected]> (cherry picked from commit 56c38bc)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…zations,kibana-data-discovery (#198331) (#203031) # Backport This will backport the following commits from `main` to `8.x`: - [Unauthorized route migration for routes owned by kibana-visualizations,kibana-data-discovery (#198331)](#198331) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kibana Machine","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-04T22:29:15Z","message":"Unauthorized route migration for routes owned by kibana-visualizations,kibana-data-discovery (#198331)\n\n### Authz API migration for unauthorized routes\r\n\r\nThis PR migrates unauthorized routes owned by your team to a new\r\nsecurity configuration.\r\nPlease refer to the documentation for more information: [Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n--- EDIT ---\r\n\r\nThis PR also adds two privileges related to saved query APIs:\r\n`savedQuery:read` and `savedQuery:manage`. These are given by default to\r\nthe same roles that already have access to the `query`-type saved\r\nobjects.\r\n\r\n### **Before migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n ...\r\n}, handler);\r\n```\r\n\r\n### **After migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n security: {\r\n authz: {\r\n enabled: false,\r\n reason: 'This route is opted out from authorization because ...',\r\n },\r\n },\r\n ...\r\n}, handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in this PR.\r\n2. Elaborate on the reasoning to opt-out of authorization.\r\n3. Routes without a compelling reason to opt-out of authorization should\r\nplan to introduce them as soon as possible.\r\n2. You might need to update your tests to reflect the new security\r\nconfiguration:\r\n - If you have snapshot tests that include the route definition.\r\n\r\n## Any questions?\r\nIf you have any questions or need help with API authorization, please\r\nreach out to the `@elastic/kibana-security` team.\r\n\r\n---------\r\n\r\nCo-authored-by: Lukas Olson <[email protected]>\r\nCo-authored-by: Matthias Wilhelm <[email protected]>\r\nCo-authored-by: Marta Bondyra <[email protected]>\r\nCo-authored-by: Davis McPhee <[email protected]>","sha":"56c38bca201ee453ee14c5eaf10baba87d6c2d8c","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["enhancement","Team:Visualizations","release_note:skip","Feature:Security/Authorization","v9.0.0","Team:DataDiscovery","backport:prev-minor","Authz: API migration"],"title":"Unauthorized route migration for routes owned by kibana-visualizations,kibana-data-discovery","number":198331,"url":"https://github.com/elastic/kibana/pull/198331","mergeCommit":{"message":"Unauthorized route migration for routes owned by kibana-visualizations,kibana-data-discovery (#198331)\n\n### Authz API migration for unauthorized routes\r\n\r\nThis PR migrates unauthorized routes owned by your team to a new\r\nsecurity configuration.\r\nPlease refer to the documentation for more information: [Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n--- EDIT ---\r\n\r\nThis PR also adds two privileges related to saved query APIs:\r\n`savedQuery:read` and `savedQuery:manage`. These are given by default to\r\nthe same roles that already have access to the `query`-type saved\r\nobjects.\r\n\r\n### **Before migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n ...\r\n}, handler);\r\n```\r\n\r\n### **After migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n security: {\r\n authz: {\r\n enabled: false,\r\n reason: 'This route is opted out from authorization because ...',\r\n },\r\n },\r\n ...\r\n}, handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in this PR.\r\n2. Elaborate on the reasoning to opt-out of authorization.\r\n3. Routes without a compelling reason to opt-out of authorization should\r\nplan to introduce them as soon as possible.\r\n2. You might need to update your tests to reflect the new security\r\nconfiguration:\r\n - If you have snapshot tests that include the route definition.\r\n\r\n## Any questions?\r\nIf you have any questions or need help with API authorization, please\r\nreach out to the `@elastic/kibana-security` team.\r\n\r\n---------\r\n\r\nCo-authored-by: Lukas Olson <[email protected]>\r\nCo-authored-by: Matthias Wilhelm <[email protected]>\r\nCo-authored-by: Marta Bondyra <[email protected]>\r\nCo-authored-by: Davis McPhee <[email protected]>","sha":"56c38bca201ee453ee14c5eaf10baba87d6c2d8c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198331","number":198331,"mergeCommit":{"message":"Unauthorized route migration for routes owned by kibana-visualizations,kibana-data-discovery (#198331)\n\n### Authz API migration for unauthorized routes\r\n\r\nThis PR migrates unauthorized routes owned by your team to a new\r\nsecurity configuration.\r\nPlease refer to the documentation for more information: [Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n--- EDIT ---\r\n\r\nThis PR also adds two privileges related to saved query APIs:\r\n`savedQuery:read` and `savedQuery:manage`. These are given by default to\r\nthe same roles that already have access to the `query`-type saved\r\nobjects.\r\n\r\n### **Before migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n ...\r\n}, handler);\r\n```\r\n\r\n### **After migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n security: {\r\n authz: {\r\n enabled: false,\r\n reason: 'This route is opted out from authorization because ...',\r\n },\r\n },\r\n ...\r\n}, handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in this PR.\r\n2. Elaborate on the reasoning to opt-out of authorization.\r\n3. Routes without a compelling reason to opt-out of authorization should\r\nplan to introduce them as soon as possible.\r\n2. You might need to update your tests to reflect the new security\r\nconfiguration:\r\n - If you have snapshot tests that include the route definition.\r\n\r\n## Any questions?\r\nIf you have any questions or need help with API authorization, please\r\nreach out to the `@elastic/kibana-security` team.\r\n\r\n---------\r\n\r\nCo-authored-by: Lukas Olson <[email protected]>\r\nCo-authored-by: Matthias Wilhelm <[email protected]>\r\nCo-authored-by: Marta Bondyra <[email protected]>\r\nCo-authored-by: Davis McPhee <[email protected]>","sha":"56c38bca201ee453ee14c5eaf10baba87d6c2d8c"}}]}] BACKPORT-->
…s,kibana-data-discovery (elastic#198331) ### Authz API migration for unauthorized routes This PR migrates unauthorized routes owned by your team to a new security configuration. Please refer to the documentation for more information: [Authorization API](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization) --- EDIT --- This PR also adds two privileges related to saved query APIs: `savedQuery:read` and `savedQuery:manage`. These are given by default to the same roles that already have access to the `query`-type saved objects. ### **Before migration:** ```ts router.get({ path: '/api/path', ... }, handler); ``` ### **After migration:** ```ts router.get({ path: '/api/path', security: { authz: { enabled: false, reason: 'This route is opted out from authorization because ...', }, }, ... }, handler); ``` ### What to do next? 1. Review the changes in this PR. 2. Elaborate on the reasoning to opt-out of authorization. 3. Routes without a compelling reason to opt-out of authorization should plan to introduce them as soon as possible. 2. You might need to update your tests to reflect the new security configuration: - If you have snapshot tests that include the route definition. ## Any questions? If you have any questions or need help with API authorization, please reach out to the `@elastic/kibana-security` team. --------- Co-authored-by: Lukas Olson <[email protected]> Co-authored-by: Matthias Wilhelm <[email protected]> Co-authored-by: Marta Bondyra <[email protected]> Co-authored-by: Davis McPhee <[email protected]>
…s,kibana-data-discovery (elastic#198331) ### Authz API migration for unauthorized routes This PR migrates unauthorized routes owned by your team to a new security configuration. Please refer to the documentation for more information: [Authorization API](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization) --- EDIT --- This PR also adds two privileges related to saved query APIs: `savedQuery:read` and `savedQuery:manage`. These are given by default to the same roles that already have access to the `query`-type saved objects. ### **Before migration:** ```ts router.get({ path: '/api/path', ... }, handler); ``` ### **After migration:** ```ts router.get({ path: '/api/path', security: { authz: { enabled: false, reason: 'This route is opted out from authorization because ...', }, }, ... }, handler); ``` ### What to do next? 1. Review the changes in this PR. 2. Elaborate on the reasoning to opt-out of authorization. 3. Routes without a compelling reason to opt-out of authorization should plan to introduce them as soon as possible. 2. You might need to update your tests to reflect the new security configuration: - If you have snapshot tests that include the route definition. ## Any questions? If you have any questions or need help with API authorization, please reach out to the `@elastic/kibana-security` team. --------- Co-authored-by: Lukas Olson <[email protected]> Co-authored-by: Matthias Wilhelm <[email protected]> Co-authored-by: Marta Bondyra <[email protected]> Co-authored-by: Davis McPhee <[email protected]>
…s,kibana-data-discovery (elastic#198331) ### Authz API migration for unauthorized routes This PR migrates unauthorized routes owned by your team to a new security configuration. Please refer to the documentation for more information: [Authorization API](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization) --- EDIT --- This PR also adds two privileges related to saved query APIs: `savedQuery:read` and `savedQuery:manage`. These are given by default to the same roles that already have access to the `query`-type saved objects. ### **Before migration:** ```ts router.get({ path: '/api/path', ... }, handler); ``` ### **After migration:** ```ts router.get({ path: '/api/path', security: { authz: { enabled: false, reason: 'This route is opted out from authorization because ...', }, }, ... }, handler); ``` ### What to do next? 1. Review the changes in this PR. 2. Elaborate on the reasoning to opt-out of authorization. 3. Routes without a compelling reason to opt-out of authorization should plan to introduce them as soon as possible. 2. You might need to update your tests to reflect the new security configuration: - If you have snapshot tests that include the route definition. ## Any questions? If you have any questions or need help with API authorization, please reach out to the `@elastic/kibana-security` team. --------- Co-authored-by: Lukas Olson <[email protected]> Co-authored-by: Matthias Wilhelm <[email protected]> Co-authored-by: Marta Bondyra <[email protected]> Co-authored-by: Davis McPhee <[email protected]>
…s,kibana-data-discovery (elastic#198331) ### Authz API migration for unauthorized routes This PR migrates unauthorized routes owned by your team to a new security configuration. Please refer to the documentation for more information: [Authorization API](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization) --- EDIT --- This PR also adds two privileges related to saved query APIs: `savedQuery:read` and `savedQuery:manage`. These are given by default to the same roles that already have access to the `query`-type saved objects. ### **Before migration:** ```ts router.get({ path: '/api/path', ... }, handler); ``` ### **After migration:** ```ts router.get({ path: '/api/path', security: { authz: { enabled: false, reason: 'This route is opted out from authorization because ...', }, }, ... }, handler); ``` ### What to do next? 1. Review the changes in this PR. 2. Elaborate on the reasoning to opt-out of authorization. 3. Routes without a compelling reason to opt-out of authorization should plan to introduce them as soon as possible. 2. You might need to update your tests to reflect the new security configuration: - If you have snapshot tests that include the route definition. ## Any questions? If you have any questions or need help with API authorization, please reach out to the `@elastic/kibana-security` team. --------- Co-authored-by: Lukas Olson <[email protected]> Co-authored-by: Matthias Wilhelm <[email protected]> Co-authored-by: Marta Bondyra <[email protected]> Co-authored-by: Davis McPhee <[email protected]>
…s,kibana-data-discovery (elastic#198331) ### Authz API migration for unauthorized routes This PR migrates unauthorized routes owned by your team to a new security configuration. Please refer to the documentation for more information: [Authorization API](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization) --- EDIT --- This PR also adds two privileges related to saved query APIs: `savedQuery:read` and `savedQuery:manage`. These are given by default to the same roles that already have access to the `query`-type saved objects. ### **Before migration:** ```ts router.get({ path: '/api/path', ... }, handler); ``` ### **After migration:** ```ts router.get({ path: '/api/path', security: { authz: { enabled: false, reason: 'This route is opted out from authorization because ...', }, }, ... }, handler); ``` ### What to do next? 1. Review the changes in this PR. 2. Elaborate on the reasoning to opt-out of authorization. 3. Routes without a compelling reason to opt-out of authorization should plan to introduce them as soon as possible. 2. You might need to update your tests to reflect the new security configuration: - If you have snapshot tests that include the route definition. ## Any questions? If you have any questions or need help with API authorization, please reach out to the `@elastic/kibana-security` team. --------- Co-authored-by: Lukas Olson <[email protected]> Co-authored-by: Matthias Wilhelm <[email protected]> Co-authored-by: Marta Bondyra <[email protected]> Co-authored-by: Davis McPhee <[email protected]>
Authz API migration for unauthorized routes
This PR migrates unauthorized routes owned by your team to a new security configuration.
Please refer to the documentation for more information: Authorization API
--- EDIT ---
This PR also adds two privileges related to saved query APIs:
savedQuery:read
andsavedQuery:manage
. These are given by default to the same roles that already have access to thequery
-type saved objects.Before migration:
After migration:
What to do next?
Any questions?
If you have any questions or need help with API authorization, please reach out to the
@elastic/kibana-security
team.