[Upgrade Assistant] Add permissions check to logs step

[sabarasaba]

Fixes #111978

Summary

This PR adds a deprecation logs index privilege check in the fix logs step, if the user doesn't have read privileges for the index a warning callout is shown.

How to test

1. Start elasticsearch with yarn es snapshot --license=trial
2. Start kibana with yarn start
3. Login as a superuser
4. Create a new role that only has manage cluster privileges (so it can use UA)
5. Create a new user and attach the previously created role and Kibana Admin
6. Login as that newly created user
7. Navigate to UA and enable logs collection

If you wanna test as a user with privileges skip steps 4, 5 and 6.

Screenshots

Overview of changes

// Collect logs toggle disabled

// Collect logs toggle enabled and user doesn't have privileges for reading the index

// Collect logs toggle enabled and user has privileges

[sabarasaba]

I had to change these types to be exported directly from the types file, otherwise it doesn't tree-shake the rest of the things exported from the authorization file and it causes errors if you try to use it server side (certain dependencies needed by the components exported require window object to exist).

[sabarasaba]

I patched this block of code to also accept privileges that might contain dots in its name. I extracted this part into its own function and wrote a few tests for it, I decided not to write tests for the HOC itself because it seemed a bit outside the scope of this PR.

[cjcenizal]

Awesome!!

[sabarasaba]

@elasticmachine merge upstream

[elasticmachine]

Pinging @elastic/kibana-stack-management (Team:Stack Management)

[jrodewig]

Thanks for adding this @sabarasaba. I think the warning message needs some revision.

It may be out of scope, but I also think this copy isn't quite correct:

Reset the counter after making changes and continue monitoring to verify that you are no longer using deprecated APIs.

The warnings aren't just for deprecated APIs. I'd reword to something like:

After making changes, reset the counter and continue monitoring to verify you're no longer using deprecated features.

[cjcenizal]

@sabarasaba I wasn't able to follow your testing instructions. What do you mean by "attach a new role that only has management privileges"? I wasn't able to understand what to configure in the "New role" form based on this instruction. Looking in all of the fields, I don't see anything that says management.

[cjcenizal]

Awesome!!

[cjcenizal]

Hmm, this is implicitly dependent on a guarantee that a privilege will never contain &. I understand this guarantee is unlikely to ever be broken, but I think we can be completely safe by being a bit more direct. This possibly makes the code easier to understand too.

const indexOfFirstPeriod = p.indexOf('.');
if (indexOfFirstPeriod === -1) {
  throw new Error('Required privilege must have the format "section.privilege"');
}

return [
  p.slice(0, indexOfFirstPeriod),
  p.slice(indexOfFirstPeriod + 1)
];

[sabarasaba]

Very nice suggestion, thanks for pointing that out!

[cjcenizal]

Do we have any tests that assert this isn't shown when the user has the required privileges?

[sabarasaba]

Nice call, I've just added one that checks for that also

[cjcenizal]

Thanks for adding these API integration tests!

[sabarasaba]

@cjcenizal Thanks for having a look, I've updated the instructions. I meant to write that the new role that is created for the user should have manage cluster privileges

[jrodewig]

Copy LGTM. Thanks!

[cjcenizal]

@sabarasaba I created a new user with a role that has only manage cluster privileges. When I log in as this user, I can't access Kibana:

Are there other privileges I need?

[sabarasaba]

Are there other privileges I need?

@cjcenizal When you set the privileges for the new user, besides the newly created role you should also add Kibana Admin

[alisonelizabeth]

Passing through again 😄

I have two questions:
1 - Does it make sense to update the global_upgrade_assistant role in the functional tests to have this index privilege? That way, I think we could use that role instead of superuser in the overview page tests.

2 - Will this design need to change at all once elastic/elasticsearch#76802 is completed? In other words, if the data stream is created by default when enabling deprecation log indexing, will the index privilege be required at this time?

[cjcenizal]

Thanks for helping me figure out the correct configuration for the test user! I was able to test locally and it works as expected. I had one suggestion for refining the copy a bit, since I think the user will need a little more reassurance. Great work on this!

[sabarasaba]

@alisonelizabeth Thanks for checking it out!

1 - Does it make sense to update the global_upgrade_assistant role in the functional tests to have this index privilege? That way, I think we could use that role instead of superuser in the overview page tests.

Nice suggestion! I think we can do that in a subsequent PR 🚀

2 - Will this design need to change at all once elastic/elasticsearch#76802 is completed? In other words, if the data stream is created by default when enabling deprecation log indexing, will the index privilege be required at this time?

I think it will still be needed because the fact that is created still doesn't mean that the current user will have permissions to read it.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: efe2e20
• Storybooks Preview

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
esUiShared	102	106	+4

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
upgradeAssistant	208.6KB	210.8KB	+2.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
esUiShared	1	3	+2

Page load bundle

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

id	before	after	diff
esUiShared	196.7KB	196.9KB	+207.0B
upgradeAssistant	25.0KB	25.4KB	+455.0B
total			+662.0B

Unknown metric groups

API count

id	before	after	diff
esUiShared	106	110	+4

History

• 💚 Build #154852 succeeded 602ccf9
• 💚 Build #154512 succeeded 0030b0b
• 💚 Build #154370 succeeded 65bd246
• 💚 Build #154152 succeeded b8ae396
• 💛 Build #154098 was flaky 9bc62b65a1135c47fee746b6cc88ee8d556d83d5

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

cc @sabarasaba

[alisonelizabeth]

Nice suggestion! I think we can do that in a subsequent PR 🚀

👍 sounds good

I think it will still be needed because the fact that is created still doesn't mean that the current user will have permissions to read it.

I guess I was thinking one step ahead of this - I was wondering if the user would be able to enable deprecation logs without the proper index privileges (i.e., successfully enable the toggle). Probably something we just need to test once the ES changes are made.