[data views] Rely on Content Management / Saved Object client for API security, don't use capabilities API

[mattkime]

Summary

The capabilities api should only be used for UI features and specifically not REST CRUD operations.

In particular, it should be possible to perform CRUD operations even without access to a particular space.

Closes #188569

Functional test is based on #187540

[elasticmachine]

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

[davismcphee]

In general the code changes look good, but I'm not exactly sure how/what to test. Could you provide instructions on how to test the differences? Or maybe we could add a functional or API integration test to validate the impact of these changes?

[davismcphee]

Is this description still accurate for this test if we no longer rely on capabilities API?

[mattkime]

The advanced settings and therefore the default data view rely on the capabilites API, therefore the test is still relevant.

[kertal]

@elasticmachine merge upstream @mattkime ,seems this was somehow paused, will you iterate on it?

[jbudz]

.buildkite/ftr_platform_stateful_configs.yml

[mattkime]

/ci

[mattkime]

/ci

[mattkime]

/ci

[davismcphee]

Code changes look good, and I confirmed I was able to create, update, and delete a data view in a space where indexPatterns was disabled. It looks like there are just a couple of test failures to look into, then I'll approve for merge.

[davismcphee]

Can getCanSave be dropped entirely? It doesn't look like it's used anywhere anymore.

[mattkime]

Its used externally, particularly in UIs

[davismcphee]

Are you sure about this? I did a global search across the codebase and the only remaining references I've found are related to its initialization within the data views plugin:

Possibly I'm missing something here though about how this is used. Not a blocker on my end regardless, just something to consider.

[mattkime]

@davismcphee - you're correct, I was confusing it with the sync version of the function. I might as well remove it, pending the conversation with Larry below.

[mattkime]

/ci

[kibana-ci]

💔 Build Failed

• Buildkite Build
• Commit: d3a50bf
• Interpreting CI Failures

Failed CI Steps

• Investigations - Security Solution Cypress Tests #3

Test Failures

• [job] [logs] Investigations - Security Solution Cypress Tests #3 / Sourcerer permissions role Hunter No actions shows error when user does not have permissions role Hunter No actions shows error when user does not have permissions

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
dataViews	62.3KB	62.5KB	+204.0B

History

• 💔 Build #236828 failed d3a50bf
• 💔 Build #236816 failed d3a50bf
• 💔 Build #236525 failed ed24ca5
• 💔 Build #236524 failed ed24ca5
• 💔 Build #236519 failed be02506

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @mattkime

[logeekal]

@mattkime , I took a look at the investigations Cypress tests that is failing. So below is the summary of test.

1. User logs as username hunter_no_actions with below privs
   
   

2. This results in below error
   

Now in this PR the error is gone with the same priveleges. Is it expected from your side? This seems to be a bug. A user without Saved object Management should be getting 403 error as in main.

I am trying to find out what privs affect the saved_object_tagging api but it is taking time. Do you have any opinion on this?

[mattkime]

@legrego We had discussed how the data views api should rely on the saved object client for auth and didn't need to check the capabilities API. The saved objects tagging api is apparently affected by spaces (and therefore the capabilities api) - is there a contradiction here?

[legrego]

@mattkime thanks for the ping.

In particular, it should be possible to perform CRUD operations even without access to a particular space.

That's not entirely accurate. A user still needs to be authorized to perform a specific action against a type of saved object within their space. This authorization check is automatically performed for you when using the scoped saved objects client. Are you referring to the feature visibility toggles that are present on the Spaces Management UI? I'm not trying to be 🤓 pedantic, I just want to make sure we're talking about the same thing.

The saved objects tagging api is apparently affected by spaces (and therefore the capabilities api) - is there a contradiction here?

Where is tagging using the capabilities API? I'm also confused by the test failure -- how/why is the tagging API impacted by this change to the data views API?

---

In general:

UI Capabilities should only be used to inform the front-end if a particular action is currently available. An action could be unavailable for several reasons:
1/ The feature that enables this action is "hidden" in the space
2/ The current user is not authorized to perform this action
3/ The plugin that owns this feature is disabled or has disabled the feature.

You capabilities API will not tell you why a feature isn't available, as it's a simple true/false flag. This is why we tell folks not to use these flags to make authorization decisions, but instead use these to curate the UX, and use our other security mechanisms to enforce authorization.

Protecting access to saved objects should be done through the SO privilege declarations during feature registration (e.g., Discover grants read access to data views:

kibana/x-pack/plugins/features/server/oss_features.ts

Lines 36 to 45 in 64e1116

	all: {
	app: ['discover', 'kibana'],
	api: ['fileUpload:analyzeFile'],
	catalogue: ['discover'],
	savedObject: {
	all: ['search', 'query'],
	read: ['index-pattern'],
	},
	ui: ['show', 'save', 'saveQuery'],
	},

). This is what gives you the automatic authorization checks I mentioned above.

[mattkime]

Where is tagging using the capabilities API? I'm also confused by the test failure -- how/why is the tagging API impacted by this change to the data views API?

Its not, its brought up for comparison.

---

Frankly I'm getting a little lost. It seems clear that my removal of the capabilities check was the right move - it seems this should be accompanied by a behavior change otherwise this would have never come to my attention. Here's the issue created by the customer - #187540

At this point its unclear to me what should be happening and how it should be implemented.

[legrego]

It seems clear that my removal of the capabilities check was the right move

I agree.

it seems this should be accompanied by a behavior change otherwise this would have never come to my attention. Here's the issue created by the customer - #187540
At this point its unclear to me what should be happening and how it should be implemented.

I think the desired behavior change is illustrated by your new API test in x-pack/test/api_integration/apis/data_views/index.ts, but perhaps you're looking for something else? I'm beginning to think I'm equally lost. I'm happy to jump on a call later this week to help sort things out if that'd be helpful.

[mattkime]

@logeekal Lets find some time to talk about the privileges provided to the user in that test and how the error is being used.

[logeekal]

Sure @mattkime , sent you an invite for Wednesday. It was the earliest time I could find. feel free to suggest me any other time.

[logeekal]

Hey @mattkime, @michaelolo24

I can confirm that error is coming in main branch because we are trying to create a new DataView as soon as the Security solution plugin initiates. Below is how call stack looks like

Security -> DataViews createDataView -> DataViews createSavedObject -> raise DataViewInsufficientAccessError

But this Error is not raised with the new code here, although it should.

@legrego We had discussed how the data views api should rely on the saved object client for auth

According to your above statement @mattkime, looks like savedObjectClient is not checking auth. savedObjectClient lets user create a DataView, event though does not have that privelege. Do you think it is an issue with saved Objects client and we should use scopedSavedObject client?