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

[Saved Objects] Prevent users from altering managed assets #70461

Closed
ph opened this issue Jul 1, 2020 · 41 comments
Closed

[Saved Objects] Prevent users from altering managed assets #70461

ph opened this issue Jul 1, 2020 · 41 comments
Labels
enhancement New value added to drive a business result Feature:Dashboard Dashboard related features Feature:Saved Objects impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort NeededFor:Fleet Needed for Fleet Team Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc triage_needed

Comments

@ph
Copy link
Contributor

ph commented Jul 1, 2020

Describe the feature:

Allow certain assets like dashboard to be read-only in Kibana. It should not allow the user to modify them and ask them to make a copy instead when making changes. They can manage these assets through Ingest Manager.

Describe a specific use case for the feature:

In the Ingest manager, we plan to take care of installing and upgrading assets like dashboards for the user when they activate an integration. They can also remove the integration to remove the assets. Thus, the lifecycle of the assets is controlled at the system level by Ingest Manager.

If the user is allowed to modify an installed dashboard, those changes may be overwritten when the package assets are manually or automatically upgraded. If dashboard would be set to read-only we would be free to upgrade the canonical dashboard and let people customize their own copies.

Update: Implementation
The request could be broken down into two parts:

  1. a server-side control that guarantees no changes could be made to a system managed saved object as all such changes might be lost on a package upgrade
  2. UI changes (needs to be done per application, Read-only UI behaviour for system managed dashboards installed as part of fleet packages #140364 tracks dashboard-related changes):
    1. highlight to a user that the dashboard they're viewing is system managed and
    2. a user flow where users are guided to make a copy of the system managed dashboard first, and then being allowed to make changes to their copy

Delivering (1) without (2) would be a very bad user experience. However, (2) does not depend on (1) and could be started independently.

@timroes timroes added enhancement New value added to drive a business result Feature:Dashboard Dashboard related features Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Jul 1, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@ruflin
Copy link
Member

ruflin commented Jul 3, 2020

A bit more details on the expected the behaviour. If a users wants to modify a dashboard, it would copy the dashboard and all its related assets like the visualisations / searches etc so the user has a copy. This ensures we can update / remove read only assets without breaking user dashboards. Also users should not be able to include read only visualisations in there dashboard but instead a copy of it.

@mostlyjason mostlyjason changed the title Allow to create read-only dashboard Allow creating read-only saved objects Jul 9, 2020
@mostlyjason mostlyjason changed the title Allow creating read-only saved objects Allow creating read-only assets Jul 9, 2020
@mostlyjason
Copy link
Contributor

@alexfrancoeur Are you the right PM for this request? We recently added this to the top asks spreadsheet.

@mostlyjason
Copy link
Contributor

I renamed this to read-only assets because our end goals is to also apply this to dashboards, visualizations, API keys, etc. Is it better to have separate issues for each of those? If so, I'd rename this back to dashboards.

@alexfrancoeur
Copy link

alexfrancoeur commented Jul 12, 2020

This feels like Object Level Security (OLS) as defined in #39259, so I'll tag a few more folks. At the moment, we do not have the ability to set permissions on a specific object. That being said, a dashboard is made up of multiple saved objects - one or multiple index patterns, one or multiple visualizations, one or multiple maps, etc. There are plans to simplify this in the future so a dashboard does not use this reference model but instead, persists all visualizations within the same json object. I'd imagine that will come into play with this proposal, regardless of security requirements. @mostlyjason I know this has been an issue with beats modules for quite some time, but what's the priority from Ingest Manager? Top asks is a good place to start the discussion. I do not believe we have plans to tackle this type of request in the 7.x series, but if we view it as a requirement for GA we should probably meet soon to discuss. I wonder if there are some alternative ways to support outside of a full OLS implementation.

cc: @arisonl @kobelb @legrego @AlonaNadler @VijayDoshi

@mostlyjason
Copy link
Contributor

Thanks @alexfrancoeur I think OLS could potentially work. Another option is to treat them as system objects of some kind. but I'm not sure if Kibana supports that or will sooner than OLS.

Adding @andresrc @exekias to comment on whether dashboards should be made of multiple saved objects. We are doing a clean start on packages so this might be an opportunity to reconsider how Beats modules handled it.

@kobelb
Copy link
Contributor

kobelb commented Jul 13, 2020

OLS is a significant undertaking, and it hasn't been started yet. Also, OLS is rather coupled to Kibana's authorization model, which we really shouldn't be relying upon here. Even when users aren't utilizing the Stack's security, we'd want these Dashboards, Visualization, etc. to be read-only so they can be automatically updated without fear of losing the end-users changes. This seems like a net-new request which I haven't heard from elsewhere.

@timroes
Copy link
Contributor

timroes commented Jul 14, 2020

Thanks for filing this. I agree with @kobelb here, that OLS is not really the solution here. I think if we want this, we need "machine created" saved objects, that are not editable by the user. I am not sure if this is some extension we need in the saved object core to make this happen, across all saved objects (cc @joshdover), or if we need to implement this on a per application level. Even with a central infrastructure we still need to have some parts (like copying dashboards and copying assets) on a per application level.

I just want to raise awareness here, that this is not a small request, and designing and implementing this, will take quit some time, so it's nothing you should be expecting to be around within the next minor version(s).

@arisonl
Copy link
Contributor

arisonl commented Jul 20, 2020

How do these assets play with spaces? I presume they should be space agnostic?

@kobelb
Copy link
Contributor

kobelb commented Jul 20, 2020

How do these assets play with spaces? I presume they should be space agnostic?

Unfortunately, it's not currently possible to have a subset of Dashboards, Visualizations, etc be "space agnostic". The @elastic/kibana-security team is exploring the complexities of allowing a Dashboard, Visualization, etc to be available at * spaces, but it's not possible yet. Assuming this is possible, it will also require the assets opt-in to being shareable in multiple spaces to take advantage of this.

@mostlyjason
Copy link
Contributor

mostlyjason commented Aug 10, 2020

I'll just mention there is an another proposal to implement tagging #74571. One alternative solution is to show a warning to users when they attempt to change assets with a specific tag. This is not as strong of a control as making them read-only, but it might still help prevent users from making changes to assets that will be overwritten.

@ruflin
Copy link
Member

ruflin commented Aug 11, 2020

@mostlyjason I like your idea as a "low hanging fruit" to at least indicate users something is read-only. I would assume it is just a tag, no errors / warnings show. But I don't think it will replace the feature requested here as read-only will also have some features like users can easily copy all dependencies etc.

@mostlyjason
Copy link
Contributor

@ruflin a deep copy/clone feature could be separate from marking assets read-only. Would it make sense to split into two issues?

@ruflin
Copy link
Member

ruflin commented Aug 17, 2020

@mostlyjason Happy to have it split out if needed. Agree that it might become a feature of its own.

@joshdover joshdover added Feature:Saved Objects Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc and removed Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Aug 18, 2020
@elasticmachine
Copy link
Contributor

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

@kobelb
Copy link
Contributor

kobelb commented Oct 26, 2020

Ingest Manager packages don't only contain Kibana assets, they also contain Elasticsearch assets. As such, I don't think we should be planning on implementing a Kibana-specific solution without ensuring that it's compatible with the solution to protect Elasticsearch assets.

@rudolf
Copy link
Contributor

rudolf commented Oct 29, 2020

I wonder if the problem here isn't more about the user experience and UI than real security on the API / Elasticsearch level.

If dashboards added a readonly field and adapted the UI to display a "clone and edit" button instead of "edit" that would provide a nicer user experience than editing, pressing "save" and then perhaps getting a read-only object error.

Even though users could theoretically circumvent this UI by using the Saved Objects API or even editing objects in the ES index this might be sufficient 99% of the time. To prevent data loss for the other 1% ingest manager can compare the existing dashboard to known previous versions. If it detects that a dashboard has been altered by a user it can log an error and refuse to override it, or maybe create a new dashboard object and rename the old one with "[backup] Ingest manager dashboard".

One day when we have OLS we can then come back to this again.

@roncohen
Copy link
Contributor

I wonder if the problem here isn't more about the user experience and UI than real security on the API / Elasticsearch level.

Absolutely. This is not about security. It's to protect people from accidentally editing dashboards that will then get replaced by a new version of the dashboard when the package gets upgraded. I was hoping we could do something simple for this.

@rudolf
Copy link
Contributor

rudolf commented Sep 9, 2022

In #137420 security solution doesn't want to just prevent all writes to a system managed rule, but instead give users the flexibility to customise but then revert to the default system managed rule.

@joshdover
Copy link
Contributor

One of the key areas that we're investing in on Fleet is improved debuggability and visibility into Elastic Agent health and performance. These types of problems represent the vast majority of ingest-related customer support load and are a large pain point in the product for our users. If users can't get data into the stack, they can't use anything else we're building for them.

One way we're planning to address this problem is by surfacing a lot more health and performance information for Agents in the Fleet UI. We're planning to leverage the new embeddable dashboard capabilities to embed dashboards that we include in our integration package. This allows us to re-use the same dashboards for a familiar UX and even update them outside of Stack releases (!) as we iterate on what's most useful to dispaly for identifying and troubleshooting problems.

The challenge this presents is that users could break parts of the Fleet UI if they are able to modify these dashboards in a way that makes the dashboard un-renderable. If we can ensure users can't change these objects, we'd be able to avoid this problem completely.

@ThomThomson
Copy link
Contributor

I think this will be extremely important for the adoption of Portable Dashboards more broadly, because even if the portable implementations in the solutions don't allow for the edit action to be taken, there is currently nothing stopping a user from finding the dashboard in the regular dashboards app and editing it there. There is a lot of discussion around this in this issue.

@ruflin
Copy link
Member

ruflin commented Jan 16, 2023

I can only double down on what @joshdover and @ThomThomson described above. Also in observability we start to use embedded dashboards / visualisations more and more. Not having this feature will allow our users to break these.

@thomheymann
Copy link
Contributor

@legrego / @arisonl There seems to be a need for solutions to be able to create system / read-only saved objects. These are saved objects that would be created by Kibana and cannot be modified by users.

Can you please prioritise so we know where / if this fits into our roadmap?

@legrego
Copy link
Member

legrego commented Jan 18, 2023

@thomheymann my opinion is that system/read-only saved-objects isn't a concern of the authorization service, but rather a core concern of the saved objects system. In other words, I think the core team is best suited to prioritize and design this feature.

@rudolf
Copy link
Contributor

rudolf commented Jan 19, 2023

In other words, I think the core team is best suited to prioritize and design this feature.

👍

We believe #140364 is a necessary first step. It's also a small amount of work which solves 95% of this problem. @rayafratkina Is working on prioritising this.

Once that's complete we can evaluate if it achieved the outcomes we hoped for or if we need to invest more (e.g. whether object level security or another kind of server-side read-only guarantee would be necessary).

@ThomThomson ThomThomson added loe:large Large Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Mar 23, 2023
@ThomThomson
Copy link
Contributor

Removing the presentation team tag from this for now, as I'm not sure what work will be required from our team. I will leave our tag on #140364, as we'll eventually need to hook into whatever system is created here to change our UI to hide edit functionality in these cases.

If there is a tangible todo for the presentation team here, I can re-add our tag.

@ThomThomson ThomThomson removed the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Apr 5, 2023
@drewdaemon
Copy link
Contributor

Even when these assets are read-only, we need some way to allow integration developers to edit them. Otherwise, no updates can be made to integration content. cc @P1llus

@teresaalvarezsoler
Copy link

As part of the customer interviews I'm doing for sharing dashboards across spaces project, I'm hearing this request quite a lot. When sharing a dashboard, they want to make it read-only for some of the spaces to ensure only admins can edit it.

@drewdaemon
Copy link
Contributor

Is the server-side control described in the issue still a requirement, or are UI controls good enough?

If they are, does it make sense to close this issue in favor of #172393?

@pgayvallet
Copy link
Contributor

Is the server-side control described in the issue still a requirement

That's an excellent question, and I'm not sure who would be able to answer this question today... @lukeelmers, do you have any idea of which PM would be "in charge" of read-only assets?

@teresaalvarezsoler
Copy link

@arisonl from the security team would be the owner of this initiative.

@drewdaemon
Copy link
Contributor

@teresaalvarezsoler correct me if I'm wrong but there seem to be two things going on here

  1. Users shouldn't be able to alter assets installed by the system
  2. Users want to make some of their own assets read-only based on various conditions

Both use cases involve preventing some users from making some changes, but there the similarities end. I would expect the signifiers, messaging, behavior, and affordances to differ. I suggest we separate these use cases and allow this issue to be limited to number 1. The title is a bit ambiguous, but the description clarifies that the issue is referring to system managed assets.

Given that, I would expect the Fleet team to give me an answer here. cc @kpollich

@teresaalvarezsoler
Copy link

Sorry for the confusion, you are right @drewdaemon . It would be good to replace "assets" by "managed assets" or "managed saved objects" in the issue title to avoid confusion.

@drewdaemon drewdaemon changed the title [Saved Objects] Allow creating read-only assets [Saved Objects] Prevent users from altering managed assets Feb 1, 2024
@kpollich
Copy link
Member

kpollich commented Feb 1, 2024

Is the server-side control described in the issue still a requirement, or are UI controls good enough?

I'd be in favor of allowing API requests to edit Fleet-managed saved objects like dashboards or saved searches as an escape hatch. I foresee rare cases where we might want to advise a user make a manual change to some dashboard as a stopgap fix before we can get a new release of a package shipped.

Preventing edits via the UI will cover the vast majority of cases where users make edits to managed assets too easily today, and aren't aware their changes will be discarded the next time a given integration is updated (commonly on stack upgrade). I think it'd be okay to intentionally leave a bit of a gap here on the server side treatment of managed content to give us options when working on support cases, etc.

cc @nimarezainia @jen-huang for visibility and input if you have any.

@drewdaemon
Copy link
Contributor

Okay, then I will close this in favor of #172393. Thanks for the input!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Dashboard Dashboard related features Feature:Saved Objects impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort NeededFor:Fleet Needed for Fleet Team Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc triage_needed
Projects
Status: Done (7.13)
Development

No branches or pull requests