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

Dashboard drilldown - either change the privilage definition to update the index pattern or update the dashboard app to not update the index pattern if the user is unauthorized. #76177

Closed
rashmivkulkarni opened this issue Aug 27, 2020 · 18 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Data Views Data Views code and UI - index patterns before 8.0 v8.0.0

Comments

@rashmivkulkarni
Copy link
Contributor

Kibana version: 8.0

When I was running the dashboard drilldown functional UI test, after giving specific roles to the logged in test_user ,I was still getting the following exception in the server:

  │ info [o.e.i.s.IndexShard] [mymac.local] [.kibana][0] onPreQueryPhase listener [org.elasticsearch.xpack.security.authz.SecuritySearchOperationListener@defb014] failed
   │      java.lang.NullPointerException: Cannot invoke "org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl.toString()" because "threadIndicesAccessControl" is null
   │      	at org.elasticsearch.xpack.security.authz.SecuritySearchOperationListener.ensureIndicesAccessControlForScrollThreadContext(SecuritySearchOperationListener.java:112) ~[?:?]
   │      	at org.elasticsearch.xpack.security.authz.SecuritySearchOperationListener.onPreQueryPhase(SecuritySearchOperationListener.java:101) ~[?:?]
   │      	at org.elasticsearch.index.shard.SearchOperationListener$CompositeListener.onPreQueryPhase(SearchOperationListener.java:134) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
   │      	at org.elasticsearch.search.SearchService$SearchOperationListenerExecutor.<init>(SearchService.java:1324) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
   │      	at org.elasticsearch.search.SearchService$SearchOperationListenerExecutor.<init>(SearchService.java:1313) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
   │      	at org.elasticsearch.search.SearchService.lambda$executeFetchPhase$2(SearchService.java:581) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
   │      	at org.elasticsearch.action.ActionRunnable.lambda$supply$0(ActionRunnable.java:58) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
   │      	at org.elasticsearch.action.ActionRunnable$2.doRun(ActionRunnable.java:73) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
   │      	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:706) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
   │      	at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
   │      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130) [?:?]
   │      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630) [?:?]
   │      	at java.lang.Thread.run(Thread.java:832) [?:?]

My roles assigned to the user were:

test_logstash_reader: {
          elasticsearch: {
            cluster: [],
            indices: [
              {
                names: ['logstash*', 'logstash-*'],
                privileges: ['all','view_index_metadata'],
                field_security: { grant: ['*'], except: [] },
              },
            ],
            run_as: [],
          },
          kibana: [],
        },

        global_dashboard_all: {
          kibana: [
            {
              feature: {
                dashboard: ['all'],
              },
              spaces: ['*'],
            },
          ],
        },

Screen Shot 2020-08-27 at 11 56 03 AM

From the Network Tab captured this:

{statusCode: 403, error: "Forbidden", message: "Unable to update index-pattern"}
error: "Forbidden"
message: "Unable to update index-pattern"
statusCode: 403

cc @legrego

@rashmivkulkarni rashmivkulkarni added bug Fixes for quality problems that affect the customer experience Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 labels Aug 27, 2020
@elasticmachine
Copy link
Contributor

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

@rashmivkulkarni
Copy link
Contributor Author

rashmivkulkarni commented Aug 27, 2020

http://localhost:5620/api/saved_objects/index-pattern/logstash-* - request

When a user with dashboard_all role opens the dashboard in edit mode , it causes the failure below :

Request URL: http://localhost:5620/api/saved_objects/index-pattern/logstash-*
Request Method: PUT
Status Code: 403 Forbidden

@rashmivkulkarni
Copy link
Contributor Author

When the loading is done - its still done as a super user, the lower privilege of the test_user is set just before opening the dashboard.

@kertal
Copy link
Member

kertal commented Sep 3, 2020

{statusCode: 403, error: "Forbidden", message: "Unable to update index-pattern"}
error: "Forbidden"
message: "Unable to update index-pattern"
statusCode: 403

dear @mattkime , could it be related to recent index pattern refactoring?

@rashmivkulkarni
Copy link
Contributor Author

@elastic/kibana-app bump^

@rashmivkulkarni
Copy link
Contributor Author

@legrego : would you mind taking a look at this too please if it falls under your domain.

@legrego
Copy link
Member

legrego commented Sep 10, 2020

This is a @elastic/kibana-app bug that relates to privileges. Happy to help, but they should take ownership as they understand the intended functionality better than I

@flash1293 flash1293 assigned flash1293 and unassigned flash1293 Sep 14, 2020
@rashmivkulkarni
Copy link
Contributor Author

@timroes - can you please help here . Thank you

@flash1293 flash1293 self-assigned this Sep 16, 2020
@flash1293
Copy link
Contributor

@rashmivkulkarni can you point to a branch where this happens? I'm not sure how to reproduce.

When the loading is done - its still done as a super user, the lower privilege of the test_user is set just before opening the dashboard.

You mean you load Kibana in the browser, then change the permissions of the currently logged in user? If that's the case, that was never supported. You have to reload the browser so it can pick up changed permissions (or even better, log in as a separate user)

@LeeDr
Copy link

LeeDr commented Sep 16, 2020

@flash1293 changing the roles of the logged in user is something we started doing in tests several months ago and has been very successful. The setRoles() method refreshes the browser (unless you pass false to skip that step).
And in this case, the test sets the roles including a refresh of Kibana page and then navigates to the dashboard.
And in fact, Rashmi is reproducing the issue manually.

We're seeing the error "Unable to update index pattern". We don't understand the purpose of the PUT to http://localhost:5620/api/saved_objects/index-pattern/logstash-*. Apparently dashboard all feature control doesn't have that Kibana privilege.

@flash1293
Copy link
Contributor

flash1293 commented Sep 17, 2020

Thanks for the PR @rashmivkulkarni (for reference, it's this one: #76055 )

I reproduced the problem locally and this is caused by the index pattern service trying to refresh the fields the first time the index pattern is used (

private isFieldRefreshRequired(specs?: FieldSpec[]): boolean {
returns true). I'm not familiar enough with index patterns to know why it's necessary, but it's probably because they were just loaded into Kibana and don't match the actual data indices.

To get the test running, one way is to load the dashboard once as a superuser (this will refresh the index pattern), then switch the roles and load the dashboard again as the limited user to run the tests. As the index pattern is up to date now, it will work fine. @rashmivkulkarni I recommend doing this for now. Another possible approach might be to fix the index pattern saved object in the fixture to not requiring a refresh, but @elastic/kibana-app-arch would have to help with that.

You could argue the index pattern service should handle the situation better (maybe try to work with the index pattern without refreshing first if the user doesn't have permission? Not sure whether that's possible) - so I will move this over to @elastic/kibana-app-arch (@mattkime I think you worked in that area recently).

I don't think allowing the dashboard permission to change index pattern saved objects is a good approach - this problem can happen in each app accessing index patterns in any way and is more of an edge case (an index pattern created from the UI by an admin wouldn't have the problem). Allowing write access to the index pattern for all of these apps would arguably create a security issue - at least it's very surprising for users allowing read access to a single app would also give write access to index patterns (we would need to do the same thing for read permissions to dashboard because in the current index pattern code this code path will be triggered as well, it's not just limited to "all").

This is not related to drilldowns in any way - it's a general problem with how index patterns are working internally.

@flash1293 flash1293 added Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:AppArch and removed Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 17, 2020
@mattkime
Copy link
Contributor

This flaw has existed for a long time. @flash1293 has correctly pointed out that in some circumstances the index pattern is automatically saved upon being loaded. Unfortunately this is by design. Similar problem from the
v5.5 days - #18019

The solution is to stop saving the field list to the saved object - #71787

@LeeDr
Copy link

LeeDr commented Sep 17, 2020

I reproduced the problem locally and this is caused by the index pattern service trying to refresh the fields the first time the index pattern is used

@flash1293 why doesn't that refresh just happen when the index pattern is created in the UI? We know that user has the privs to create (or update) the index pattern.

Oh, but in this case (Rashmi's test), the index pattern was created by installing the sample data, not through the index pattern UI.
And there's other cases (beats) where the index pattern is created when dashboards are loaded, which can be before there's any data in the beats index.

So for now, I think we can modify the test to first open the dashboard as the superuser.

@flash1293
Copy link
Contributor

I’m not 100% sure about these cases @LeeDr - Summoning @mattkime once again 🙂 Can you provide some more context, Matt?

@LeeDr
Copy link

LeeDr commented Sep 17, 2020

Actually, on second thought, what would happen in the UI if opening the dashboard just handled the failure to refresh the index pattern silently?
If the dashboard can work without that refresh, then the refresh could just happened the next time someone with the privs to update index patterns used it.

@mattkime
Copy link
Contributor

In the following circumstances the index pattern service determines that it needs to reload and save the field list - https://github.com/elastic/kibana/blob/master/src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts#L198

It looks like code that avoids doing a proper migration step. Thankfully there seems to be little reason to save the list at all.

@mattkime
Copy link
Contributor

mattkime commented Dec 2, 2020

The field list is no longer cached - #82223 - will be released in 7.11

@Dosant
Copy link
Contributor

Dosant commented Feb 24, 2021

Should be no longer relevant #76177 (comment)

@Dosant Dosant closed this as completed Feb 24, 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:Data Views Data Views code and UI - index patterns before 8.0 v8.0.0
Projects
None yet
Development

No branches or pull requests

8 participants