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

Users without access to Maps should not have the option to create them #88723

Closed
legrego opened this issue Jan 19, 2021 · 14 comments · Fixed by #88830
Closed

Users without access to Maps should not have the option to create them #88723

legrego opened this issue Jan 19, 2021 · 14 comments · Fixed by #88830
Assignees
Labels
bug Fixes for quality problems that affect the customer experience [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@legrego
Copy link
Member

legrego commented Jan 19, 2021

Kibana version: tested on master

Describe the bug:

The visualize application has a link to create Maps from their create workflow. Users who aren't authorized to create Maps specifically shouldn't be given this option.

Steps to reproduce:

  1. Create a role with just All access to Visualize
  2. Create a user, and assign them the role created in step 1
  3. Login as this user, and navigate to Visualize
  4. Click the Create visualization button

Expected behavior:

I should not see an option to create Maps.

Screenshots (if relevant):

CleanShot 2021-01-19 at 13 11 34

@legrego legrego added bug Fixes for quality problems that affect the customer experience [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Jan 19, 2021
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@thomasneirynck
Copy link
Contributor

thomasneirynck commented Jan 19, 2021

@legrego this is presumably a space/role issue? Is there API in the security plugin to check whether a user has access or not?

@legrego
Copy link
Member Author

legrego commented Jan 19, 2021

@thomasneirynck the role is configured correctly, but Maps (or Visualize?) presumably isn't checking the user's privileges. The best way to check this is via UI Capabilities, which is exposed via CoreStart on the client side:
https://www.elastic.co/guide/en/kibana/current/development-security.html#_using_ui_capabilities.

Based on the UI Capabilities registered for Maps, I expect you'd need to check the save capability. I don't know where/how the Maps viz type is registered, but for some high-level pseudo-code, I'd expect something like:

class MyPublicMapsPlugin() {
   public start(core: CoreStart) {
		const canCreateMaps = core.application.capabilities.maps.save ?? false;
        if (canCreateMaps) {
          makeMapsVizTypeAvailable();
        }
   }
}

@nreese
Copy link
Contributor

nreese commented Jan 19, 2021

the role is configured correctly, but Maps (or Visualize?) presumably isn't checking the user's privileges

This is a visualize problem since that menu is created by visualize. The menu needs to check that users have maps save capabilities

@legrego
Copy link
Member Author

legrego commented Jan 19, 2021

the role is configured correctly, but Maps (or Visualize?) presumably isn't checking the user's privileges

This is a visualize problem since that menu is created by visualize. The menu needs to check that users have maps save capabilities

Thanks, this is the part I wasn't sure about. I didn't know if Maps should conditionally register itself, or if Visualize needed to conditionally render. In any case, all the relevant parties are pinged on this issue 🙂

@stratoula
Copy link
Contributor

stratoula commented Jan 19, 2021

@nreese @thomasneirynck the wizard displays the Maps card only if it is registered from the maps plugin. So the check should happen on the maps plugin which should only register the alias if it has the permissions https://github.com/elastic/kibana/blob/master/x-pack/plugins/maps/public/plugin.ts#L142.

@legrego
Copy link
Member Author

legrego commented Jan 20, 2021

@stratoula Is it possible to unregister viz types? I ask because Maps will not know the user's privileges until the start phase, but viztype registration happens during the setup phase.

So I expect that Maps would need a way to conditionally unregister or hide their viztype, rather then not register it at all.

@stratoula
Copy link
Contributor

stratoula commented Jan 20, 2021

@legrego yes, there is ;) You can see here https://github.com/elastic/kibana/blob/master/x-pack/plugins/maps/public/plugin.ts#L166 that the maps plugin unregisters the OSS alias on the start phase. (This will be removed btw as we don't need it anymore)

@nreese nreese self-assigned this Jan 20, 2021
@nreese
Copy link
Contributor

nreese commented Jan 20, 2021

thanks @legrego and @stratoula. I will put up a PR to fix this issue

@rashmivkulkarni
Copy link
Contributor

re-opening this issue as I can reproduce it in 7.12.0 BC5

@nreese
Copy link
Contributor

nreese commented Mar 22, 2021

looks like #89092 removed the ability to hide the maps card.

@stratoula
Copy link
Contributor

stratoula commented Mar 23, 2021

@nreese this removed the alias to the OSS not the ability to hide the maps card. The unregisterAlias is still there. So I don't think that this has caused the problem. Moreover I can't reproduce it on master (I haven't tested it on 7.12)

image

@stratoula
Copy link
Contributor

I am closing as I cant reproduce it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants