[Connection Details] Check if user has permissions to manage own API keys #183286

vadimkibana · 2024-05-13T13:07:00Z

Summary

Exposes from security plugin ability to check if use can manage own API keys, through security.authz.getCurrentUserApiKeyPrivileges().
Checks if user has permissions, and displays "No permissions" screen, if user does not have permissions in the Connection Details flyout.

How to test

Add these Cloud settings to your config/kibana.dev.yml as a mock data:

elasticsearch.hosts: ["http://localhost:9200"]
xpack.cloud.id: my-cluster-id:dXMtZWFzdC0xLmF3cy5zdGFnaW5nLmZvdW5kaXQubm8kZjY3ZDZiZjFhM2NmNDA4ODhlODg2M2Y2Y2IyY2RjNGMkOWViYzEzYjRkOTU0NDI2NDljMzcwZTNlZjMyZWYzOGI=

This will expose the "Setup guides" button in the top nav and the "Connect to the Elasticsearch API" card:

Create a new user. The lowest level of API key permissions is manage_own_api_key. Toggle that permission on user settings to see the changes in the Connection Details flyout.

When the user does not have a permission to edit own API keys, the connection details flyout should show this message:

When user has permissions, it should allow to create a new API key:

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Flaky Test Runner was used on any tests changed
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately
[ ]

…fix'

…into connection-details-permissions

vadimkibana · 2024-05-15T10:49:38Z

@elasticmachine merge upstream

elasticmachine · 2024-05-15T10:50:54Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

elasticmachine · 2024-05-15T10:50:55Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

elasticmachine · 2024-05-15T10:50:55Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

…permissions

vadimkibana · 2024-05-16T13:48:58Z

@elasticmachine merge upstream

…fix'

vadimkibana · 2024-05-17T08:33:02Z

@elasticmachine merge upstream

packages/cloud/connection_details/kibana/kibana_connection_details_provider.tsx

elena-shostak · 2024-05-17T08:50:11Z

x-pack/packages/security/plugin_types_public/src/authorization/authorization_service.ts

+  getCurrentUserApiKeyPrivileges: () => Promise<AuthorizationCurrentUserApiKeyPrivilegesResponse>;
+}
+
+export interface AuthorizationCurrentUserApiKeyPrivilegesResponse {


Can't we use GetAPIKeysResult interface here?

This code is in a package, but GetAPIKeysResult is the server-side plugin. I'm thinking this would create a circular dependency problem, no?

The GetAPIKeysResult would need to be moved to shared package to be reused? Happy to move it somewhere, if you could please tell me what is the right place in the security code.

We have plugin_types_common, but I would probably ask for second opinion also before moving things around.
cc @jeramysoucy

Thanks for the ping! I added my feedback here

packages/cloud/connection_details/kibana/kibana_connection_details_provider.tsx

…ails_provider.tsx Co-authored-by: elena-shostak <165678770+elena-shostak@users.noreply.github.com>

…tion_details_provider.tsx" This reverts commit 9b66af5.

…fix'

kibana-ci · 2024-05-17T11:02:47Z

💚 Build Succeeded

Buildkite Build
Commit: da49c22

Metrics [docs]

Async chunks

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

id	before	after	diff
`home`	149.4KB	149.6KB	+221.0B

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
`@kbn/security-plugin-types-public`	0	1	+1

Page load bundle

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

id	before	after	diff
`cloudLinks`	26.7KB	26.9KB	+221.0B
`security`	69.1KB	69.3KB	+262.0B
total			+483.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/security-plugin-types-public`	39	40	+1
`security`	413	414	+1
total			+2

History

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

jeramysoucy · 2024-05-17T15:07:39Z

x-pack/plugins/security/public/authorization/authorization_service.ts

+    const getCurrentUserApiKeyPrivileges = async () => {
+      const { canManageApiKeys, canManageCrossClusterApiKeys, canManageOwnApiKeys } =
+        (await http.get(
+          '/internal/security/api_key'
+        )) as AuthorizationCurrentUserApiKeyPrivilegesResponse;
+
+      return { canManageApiKeys, canManageCrossClusterApiKeys, canManageOwnApiKeys };
+    };
+
+    return { isRoleManagementEnabled, getCurrentUserApiKeyPrivileges };


GET /internal/security/api_key returns all API keys, which if there are many could make this slow. Additionally, this endpoint is being dropped and replaced with a query endpoint. See #168970

I'd suggest instead augmenting our authentication service to include the functionality that you want. This is where we already expose API key functions. See

kibana/x-pack/plugins/security/public/authentication/authentication_service.ts

Line 44 in bcbd550

const areAPIKeysEnabled = async () =>

You could add a getCurrentUserApiKeyPrivileges function in the authentication service that calls a new internal endpoint that ONLY returns the three flags you are looking for (canManageApiKeys, canManageCrossClusterApiKeys, canManageOwnApiKeys), e.g. GET /internal/security/api_key/_check_privileges

Additionally, this would be exposed on the server side as well here:

kibana/x-pack/plugins/security/server/authentication/authentication_service.ts

Line 387 in ba8234e

apiKeys: {

pgayvallet

see comment

pgayvallet · 2024-05-22T06:57:49Z

packages/cloud/tsconfig.json

@@ -22,5 +22,7 @@
    "@kbn/cloud-plugin",
    "@kbn/share-plugin",
    "@kbn/security-plugin-types-server",
+    "@kbn/security-plugin-types-public",
+    "@kbn/core",


Please do not import from core's "plugin entry point" from package code. Imports from @kbn/core/server|public should be replaced with imports from core's corresponding domain packages instead.

Looking at the changes in this package, it seems this was added because of this import:

import { coreMock } from '@kbn/core/public/mocks';

used for

coreMock.createStart(),

Please use

import { coreLifecycleMock } from '@kbn/core-lifecycle-browser-mocks'; //... coreLifecycleMock.createCoreStart()

instead

vadimkibana added 2 commits May 13, 2024 15:03

expose ability to check for security key privileges

7c80f73

wire in security key privilege check into conn details flyout

0051b6b

vadimkibana requested review from a team as code owners May 13, 2024 13:07

kibanamachine and others added 3 commits May 13, 2024 13:13

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

04d5865

…fix'

handle case when fetching permissions is forbidden

e491f22

Merge remote-tracking branch 'origin/connection-details-permissions' …

d95075d

…into connection-details-permissions

Merge branch 'main' into connection-details-permissions

2f163c1

vadimkibana changed the title ~~[wip] Connection details permissions~~ [Connection Details] Check if user has permissions to manage own API keys May 15, 2024

vadimkibana added review release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.15.0 labels May 15, 2024

jeramysoucy self-requested a review May 15, 2024 11:04

vadimkibana mentioned this pull request May 15, 2024

[WIP] [Connection Details] Link to API keys management, when no permissions #183296

Closed

vadimkibana added 3 commits May 16, 2024 10:49

Merge remote-tracking branch 'upstream/main' into connection-details-…

ee4258c

…permissions

remove import from security plugin

63aa379

import security type from public

b8303ab

kibanamachine and others added 4 commits May 16, 2024 09:49

Merge branch 'main' into connection-details-permissions

e9e4942

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

a54b758

…fix'

add new api method mocks

114db76

update test snapshots

e99527b

Merge branch 'main' into connection-details-permissions

5fb7a0d

elena-shostak requested changes May 17, 2024

View reviewed changes

vadimkibana and others added 4 commits May 17, 2024 11:13

Update packages/cloud/connection_details/kibana/kibana_connection_det…

9b66af5

…ails_provider.tsx Co-authored-by: elena-shostak <165678770+elena-shostak@users.noreply.github.com>

Revert "Update packages/cloud/connection_details/kibana/kibana_connec…

e9a7b1d

…tion_details_provider.tsx" This reverts commit 9b66af5.

add permissions check test

7141325

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

da49c22

…fix'

jeramysoucy reviewed May 17, 2024

View reviewed changes

pgayvallet reviewed May 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Connection Details] Check if user has permissions to manage own API keys #183286

[Connection Details] Check if user has permissions to manage own API keys #183286

vadimkibana commented May 13, 2024 •

edited

vadimkibana commented May 15, 2024

elasticmachine commented May 15, 2024

elasticmachine commented May 15, 2024

elasticmachine commented May 15, 2024

vadimkibana commented May 16, 2024

vadimkibana commented May 17, 2024

elena-shostak May 17, 2024

vadimkibana May 17, 2024 •

edited

elena-shostak May 17, 2024

jeramysoucy May 17, 2024

kibana-ci commented May 17, 2024

API count

jeramysoucy May 17, 2024

pgayvallet left a comment

pgayvallet May 22, 2024

[Connection Details] Check if user has permissions to manage own API keys #183286

Are you sure you want to change the base?

[Connection Details] Check if user has permissions to manage own API keys #183286

Conversation

vadimkibana commented May 13, 2024 • edited

Summary

How to test

Checklist

Risk Matrix

For maintainers

vadimkibana commented May 15, 2024

elasticmachine commented May 15, 2024

elasticmachine commented May 15, 2024

elasticmachine commented May 15, 2024

vadimkibana commented May 16, 2024

vadimkibana commented May 17, 2024

elena-shostak May 17, 2024

Choose a reason for hiding this comment

vadimkibana May 17, 2024 • edited

Choose a reason for hiding this comment

elena-shostak May 17, 2024

Choose a reason for hiding this comment

jeramysoucy May 17, 2024

Choose a reason for hiding this comment

kibana-ci commented May 17, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

Public APIs missing exports

Page load bundle

API count

History

jeramysoucy May 17, 2024

Choose a reason for hiding this comment

pgayvallet left a comment

Choose a reason for hiding this comment

pgayvallet May 22, 2024

Choose a reason for hiding this comment

vadimkibana commented May 13, 2024 •

edited

vadimkibana May 17, 2024 •

edited