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

minimal_all privilege for a feature with no subfeatures doesn't give any privs #111631

Closed
LeeDr opened this issue Sep 8, 2021 · 6 comments · Fixed by #115992
Closed

minimal_all privilege for a feature with no subfeatures doesn't give any privs #111631

LeeDr opened this issue Sep 8, 2021 · 6 comments · Fixed by #115992
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Security/Feature Controls Platform Security - Spaces & Role Mgmt feature controls impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@LeeDr
Copy link

LeeDr commented Sep 8, 2021

Kibana version: 7.15.0 (but also 7.14 or earlier versions)

Elasticsearch version: 7.15.0

Server OS version: n/a

Browser version: n/a

Browser OS version: n/a

Original install method (e.g. download page, yum, from source, etc.): ESS Cloud

Describe the bug: If you create a role with Kibana Canvas privilege of all or minimal_all it should allow full access to Canvas. The difference between them should impact whether you can use reporting in Canvas or not but this is also tied to the config param xpack.reporting.roles.enabled.

Expected:

  1. if xpack.reporting.roles.enabled: true reporting in Canvas is controlled by having the reporting_user role (or not having it). In this case, minimal_all and all shouldn't make a difference.
  2. if xpack.reporting.roles.enabled: false reporting in Canvas is controlled by having all priv. minimal_all means Canvas without reporting)

I made this table but not 100% sure it's correct.

Canvas privilege xpack.reporting.roles.enabled with reporting_user role? can use Canvas can use reporting in Canvas
all True (default) yes yes yes
all True (default) no yes no
minimal_all True (default) yes yes yes
minimal_all True (default) no yes no
minimal_all false n/a? yes no
all false n/a? yes yes

Actual: minimal_all with xpack.reporting.roles.enabled: true doesn't give any access to Canvas.

Steps to reproduce:

  1. Start Kibana without the xpack.reporting.roles.enabled: false that we have set in x-pack/test/functional/config.js
  2. Create a role like this. I don't think you can create this role in the UI unless xpack.reporting.roles.enabled: false ?
    So you might have to use curl to the kibana URL/api/security/role with a body like this (our tests define these roles in the config file);
        global_canvas_all: {
          kibana: [
            {
              feature: {
                canvas: ['minimal_all'],
                visualize: ['minimal_all'],
              },
              spaces: ['*'],
            },
          ],
        }
  1. Create a user and give them that role.
  2. Log in as that user. Check if Canvas is in the Nav bar. You can also change your URL to /app/canvas#/

Expected behavior: Canvas should be in the Nav bar, and should also work if you go straight to the URL.

Screenshots (if relevant):

Errors in browser console (if relevant):

Provide logs and/or server output (if relevant):

Any additional context: I think our main test/functional and x-pack/test/functional test groups should try to avoid changing any Kibana or Elasticsearch server configuration parameters for a few reasons:

  1. We should run most of our tests against the default configuration users will have (and this also means Kibana on ESS should run with those same default parameter settings)
  2. we should also have a non-default-settings config file for testing non-default values. In some cases we initially release a feature disabled but make a config parameter so users can change the setting and try it. If we have those in a different test suite, and later change the default value to enable the feature, we can just swap the tests between the two groups.
@LeeDr LeeDr added the bug Fixes for quality problems that affect the customer experience label Sep 8, 2021
@botelastic botelastic bot added the needs-team Issues missing a team label label Sep 8, 2021
@LeeDr LeeDr added Feature:Security/Feature Controls Platform Security - Spaces & Role Mgmt feature controls Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Sep 8, 2021
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot removed the needs-team Issues missing a team label label Sep 8, 2021
@tsullivan tsullivan changed the title minimal_all in combination with the default xpack.reporting.roles.enabled: true setting doesn't give any privs minimal_all privilege for a feature with no subfeatures doesn't give any privs Sep 8, 2021
@tsullivan
Copy link
Member

tsullivan commented Sep 8, 2021

Note: I couldn't figure out where minimal_all is defined for Canvas, or defined anywhere at all. I believe it is a built-in from the Security Feature Controls side. I advised Lee to file this with the Team:Security label.

To add some background to Lee's description and how it involves Canvas & Reporting: "PDF reports" are the only sub-feature for Canvas, so the xpack.reporting.roles.enabled config setting causes Canvas to either have a sub-feature defined, or no sub-features defined.

A side effect of that toggling nature, is a bug happening when the user has minimal_all privilege, they only get read & write privileges to the feature if sub-features exist for it. If a feature has no sub-feature, and the user has minimal_all privilege for it, they still should have read & write privileges, I would think.

@legrego
Copy link
Member

legrego commented Sep 8, 2021

Actual: minimal_all with xpack.reporting.roles.enabled: true doesn't give any access to Canvas.

This is actually working as designed. Features only get a minimal_all privilege if there is at least 1 sub-feature privilege defined. Otherwise, we don't bother creating this, as it is indistinguishable from the all privilege from an authorization perspective.

I think we are open to changing this decision if needed, but this isn't a bug as it stands today

@LeeDr
Copy link
Author

LeeDr commented Sep 8, 2021

I think this is a weird situation where the test creates a role via the Kibana API which has the minimal_all priv even though you couldn't do that through the UI in this case (when xpack.reporting.roles.enabled: true).

Maybe creating that role should fail?

@legrego
Copy link
Member

legrego commented Sep 9, 2021

I think this is a weird situation where the test creates a role via the Kibana API which has the minimal_all priv even though you couldn't do that through the UI in this case (when xpack.reporting.roles.enabled: true).

Maybe creating that role should fail?

Yeah I agree this is a weird situation. We could fail with an error message, but that poses some interesting backwards compatibility concerns. We haven't enforced privilege names to date, and we wouldn't want to prevent existing roles with unknown privileges from being updated. We (and ES) don't make the distinction between create and update for roles today, so it would be tricky to get this right without impacting any automated workflows that our users might have setup (e.g. terraform). I'm not opposed to exploring this further to see how feasible it'd be

@LeeDr
Copy link
Author

LeeDr commented Sep 9, 2021

I think there could be a problem if someone creates roles with xpack.reporting.roles.enabled: false and then removes the config param (or changes it to the default true). This would allow someone to initially create a Canvas role without reporting (minimal_all) and the when they change the config users with that role would no longer have access to Canvas.

I haven't tried this.

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Sep 28, 2021
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 Feature:Security/Feature Controls Platform Security - Spaces & Role Mgmt feature controls impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants