[SOR] Intersect allowed and authorized types by afharo · Pull Request #244967 · elastic/kibana

afharo · 2025-12-02T17:38:41Z

Summary

We identified some scenarios where clients authorized to a partial list of SO types would circumvent the Saved Objects Repository's allowed types (it could list "hidden" SO types).

This PR addresses this issue by unifying the flow for partially and fully authorized clients, and applying the intersection of the allowed and authorized lists.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
Review the backport guidelines and apply applicable backport:* labels.

Identify risks

Some APIs might have been abusing this bug. We need to validate through CI that they work as intended, and send an internal note to all Kibana contributors to raise awareness of potential failures.

afharo · 2025-12-03T15:36:17Z

src/core/packages/saved-objects/api-server-internal/src/lib/apis/find.ts

    }
-    if (authorizationResult?.status === 'partially_authorized') {
+    if (
+      authorizationResult?.status === 'fully_authorized' ||


This only ensures the same path is followed for both authorization levels, although, functionally, it achieves the same result if we don't add this clause in the if.

Happy to remove this line if we think that performance might be a problem.

I'd appreciate @azasypkin's thoughts on this.

Would it be more defensive to express this as if (authorizationResult?.status) {?

I'd like @elastic/kibana-security's input to help me answer that.

I might be missing some other context here, but if you are concerned with having the overhead of additional conditional checks, then this can just check that authorizationResult is not undefined or that authorizationResult.status is not unauthorized. Though, just before this we're returning if unauthorized. By this point in the execution path, the status must be either partially or fully authorized.

Apologies for not being clear: what I meant is that, in the current code, the fully_authorized doesn't go through this path, so typeToNamespacesMap is not defined, and only allowedTypes is used.

Maybe it's not worth spending CPU cycles to calculate the intersection of the authorized and the allowed types because the intersection will always equal allowedTypes.

I only added it because it made the code consistent before the fix. But happy to revert this change if we think that the extra CPU cycles are not necessary.

Ah, I see. Thanks for clarifying. You are correct, and I think it is fine to skip this logic for fully_authorized if there is any concern about performance here.

I added a code comment for future us to know that's safe to be removed.

I left it for consistency, and because, while there are more CPU cycles used... I'd expect that, typically, users are partially_authorized, so we're just removing that branch for a low percentage of the users.

afharo · 2025-12-03T15:38:48Z

src/core/packages/saved-objects/api-server-internal/src/lib/apis/find.ts

+        // Discard the types that the SO repository doesn't know about (typically hidden objects).
+        if (!allowedTypes.includes(objType)) continue;


Mixing the user's authorization and the client's scope here (which can be odd).

However, typeToNamespacesMap doesn't really imply that it's bound to only one.

Part of the issue/complexity for me is that we have types and typeToNamespacesMap and these can get out-of-sync. In that context, code like this looks a bit weird to me:

kibana/src/core/packages/saved-objects/api-server-internal/src/lib/search/search_dsl/query_params.ts

Line 205 in d70608e

typeToNamespacesMap ? Array.from(typeToNamespacesMap.keys()) : type

Shouldn't our type: string | string[] | undefined be the source of truth for the types we are building queries for? Would another fix be to just use type as types when building our query (just on that line Iinked)? Or does that break something else?

IIUC, this would break the authorization part: if we only look at the requested type, users who don't have access to the type will be able to read it. Also, in the line that you shared, type can be undefined, leading to getTypes to return all registered types (no matter if the user has access or not).

The bug here is that type is "cleaned up" from the hidden SOs that the SO Client isn't allowed to read, but we don't apply such filter to typeToNamespacesMap.

I see, thanks for taking a closer look @afharo ! We also have a comment talking directly to the issue I expressed:

typeToNamespacesMap, // If defined, this takes precedence over the `type` and `namespaces` fields

Still a bit odd, but I think that's OK!

afharo · 2025-12-03T15:40:13Z

...ore/packages/saved-objects/api-server-internal/src/lib/repository.security_extension.test.ts

+        {
+          type: [
+            type,
+            'foo',


I had to add foo to the requested type so that it is kept in the auth map.

afharo · 2025-12-03T15:40:49Z

...ore/packages/saved-objects/api-server-internal/src/lib/repository.security_extension.test.ts

+      expect(arrayMapsAreEqual(actualMap, expectedMap)).toBeTruthy();
+    });
+
+    test(`uses the authorization map when fully authorized`, async () => {


just repeating the test above, but for fully authorized.

afharo · 2025-12-03T15:42:35Z

src/platform/plugins/shared/data_views/server/rest_api_routes/public/swap_references.ts

+              // Make sure to search through all the hidden types as well.
+              includedHiddenTypes: types.filter((t) => t.hidden).map((t) => t.name),


I assume that we want to search references to the Data View across all types. Is my assumption correct?

Nice, tested locally and this fixes the bug from #243432, thanks!

And your assumption is correct, although we're intending to get approval from existing hidden SO owners before changing the swap_references API to make sure everyone is on the same page. I've pinged all owning teams now and am tracking an approvals list in #243432.

I'm hoping to get all approvals quickly, but if you'd like to get this PR merged ahead of that, please feel free to drop these changes for now and we can reintroduce them in a separate PR after, as well as adding some integration tests for it.

Thank you, @davismcphee.

We agreed in the Core team that we'll send a heads-up to all other teams and hold off on merging until next week to give enough time to test and react. So happy to wait for your approval.

As agreed, I reverted the changes here

Thanks for the heads up @afharo, I think this makes sense. Looks like the swap_references case is a bit more complicated than anticipated, and we'll have some additional work to do there.

elasticmachine · 2025-12-04T00:08:22Z

Pinging @elastic/kibana-core (Team:Core)

elasticmachine · 2025-12-04T00:08:24Z

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

elasticmachine · 2025-12-04T00:09:42Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

…nd-authorized-types

elasticmachine · 2025-12-16T16:31:51Z

💔 Build Failed

Buildkite Build
Commit: ece637f

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #8 / serverless observability UI - Cases and Rules Serverless Observability Cases Cases list row actions Severity to critical

History

💔 Build #373756 failed fcc7200
💔 Build #373258 failed c42d401
💚 Build #372970 succeeded 3b11a37
💛 Build #372260 was flaky 0d61a99
💔 Build #372138 failed 92b6022

cc @afharo

…nd-authorized-types

kibanamachine · 2025-12-16T19:00:13Z

Starting backport for target branches: 8.19, 9.1, 9.2

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

## Summary We identified some scenarios where clients authorized to a partial list of SO types would circumvent the Saved Objects Repository's allowed types (it could list "hidden" SO types). This PR addresses this issue by unifying the flow for partially and fully authorized clients, and applying the intersection of the allowed and authorized lists. ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks - [x] Some APIs might have been abusing this bug. We need to validate through CI that they work as intended, and send an internal note to all Kibana contributors to raise awareness of potential failures. --------- Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co> (cherry picked from commit 28fc5b9)

kibanamachine · 2025-12-16T19:08:56Z

💚 All backports created successfully

Status	Branch	Result
✅	8.19
✅	9.1
✅	9.2

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

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `9.2`: - [[SOR] Intersect allowed and authorized types (#244967)](#244967)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Alejandro Fernández Haro <alejandro.haro@elastic.co> Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>

# Backport This will backport the following commits from `main` to `8.19`: - [[SOR] Intersect allowed and authorized types (#244967)](#244967)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Alejandro Fernández Haro <alejandro.haro@elastic.co> Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>

# Backport This will backport the following commits from `main` to `9.1`: - [[SOR] Intersect allowed and authorized types (#244967)](#244967)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Alejandro Fernández Haro <alejandro.haro@elastic.co> Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>

afharo self-assigned this Dec 2, 2025

afharo added 2 commits December 3, 2025 15:17

[SOR] Intersect allowed and authorized types

6f51d1e

Fix tests + swap_references API

3a28421

afharo force-pushed the saved-objects-repository/intersect-allowed-and-authorized-types branch from 60c94ce to 3a28421 Compare December 3, 2025 14:17

afharo commented Dec 3, 2025

View reviewed changes

Fix more tests

3ac4e78

afharo marked this pull request as ready for review December 4, 2025 00:08

afharo requested review from a team as code owners December 4, 2025 00:08

afharo requested a review from a team December 4, 2025 00:09

afharo added the Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// label Dec 4, 2025

afharo added 2 commits December 4, 2025 12:39

Merge branch 'main' into saved-objects-repository/intersect-allowed-a…

d3c4c4a

…nd-authorized-types

Merge branch 'main' into saved-objects-repository/intersect-allowed-a…

fdb0612

…nd-authorized-types

Merge branch 'main' into saved-objects-repository/intersect-allowed-a…

ece637f

…nd-authorized-types

Merge branch 'main' into saved-objects-repository/intersect-allowed-a…

3d77a91

…nd-authorized-types

afharo merged commit 28fc5b9 into elastic:main Dec 16, 2025
14 checks passed

afharo deleted the saved-objects-repository/intersect-allowed-and-authorized-types branch December 16, 2025 18:59

kibanamachine added the v9.3.0 label Dec 16, 2025

kibanamachine mentioned this pull request Dec 16, 2025

[8.19] [SOR] Intersect allowed and authorized types (#244967) #246625

Merged

kibanamachine mentioned this pull request Dec 16, 2025

[9.1] [SOR] Intersect allowed and authorized types (#244967) #246626

Merged

kibanamachine mentioned this pull request Dec 16, 2025

[9.2] [SOR] Intersect allowed and authorized types (#244967) #246627

Merged

kibanamachine added the v9.2.3 label Dec 16, 2025

kibanamachine added the v8.19.9 label Dec 16, 2025

kibanamachine added the v9.1.9 label Dec 17, 2025

mistic added v9.1.10 v9.2.4 and removed v9.1.9 v9.2.3 labels Dec 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SOR] Intersect allowed and authorized types#244967

[SOR] Intersect allowed and authorized types#244967
afharo merged 16 commits intoelastic:mainfrom
afharo:saved-objects-repository/intersect-allowed-and-authorized-types

afharo commented Dec 2, 2025 •

edited by kibanamachine

Loading

afharo Dec 3, 2025 •

edited

Loading

jloleysens Dec 9, 2025

afharo Dec 9, 2025

jeramysoucy Dec 12, 2025

afharo Dec 15, 2025

jeramysoucy Dec 15, 2025

afharo Dec 15, 2025

afharo Dec 3, 2025

jloleysens Dec 5, 2025 •

edited

Loading

afharo Dec 9, 2025

jloleysens Dec 9, 2025

afharo Dec 3, 2025

afharo Dec 3, 2025

afharo Dec 3, 2025

davismcphee Dec 4, 2025

afharo Dec 5, 2025

afharo Dec 11, 2025

davismcphee Dec 12, 2025

elasticmachine commented Dec 4, 2025

elasticmachine commented Dec 4, 2025

elasticmachine commented Dec 4, 2025

elasticmachine commented Dec 16, 2025

Uh oh!

kibanamachine commented Dec 16, 2025

kibanamachine commented Dec 16, 2025

Labels

7 participants

		// Discard the types that the SO repository doesn't know about (typically hidden objects).
		if (!allowedTypes.includes(objType)) continue;

		// Make sure to search through all the hidden types as well.
		includedHiddenTypes: types.filter((t) => t.hidden).map((t) => t.name),

Conversation

afharo commented Dec 2, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Identify risks

afharo Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jloleysens Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Dec 4, 2025

elasticmachine commented Dec 4, 2025

elasticmachine commented Dec 4, 2025

elasticmachine commented Dec 16, 2025

💔 Build Failed

Failed CI Steps

Test Failures

History

Uh oh!

kibanamachine commented Dec 16, 2025

kibanamachine commented Dec 16, 2025

💚 All backports created successfully

Questions ?

Labels

7 participants

afharo commented Dec 2, 2025 •

edited by kibanamachine

Loading

afharo Dec 3, 2025 •

edited

Loading

jloleysens Dec 5, 2025 •

edited

Loading