[ML] Adds option to Reauthorize transform in Management page #154736

qn895 · 2023-04-11T15:03:14Z

Summary

Part of #151186. Follow up of #154665. This PR adds the ability to Reauthorize transforms with a secondary authorization so that it can start operating correctly.

If the transform was originally created with insufficient permission

It will be created successfully, but fail to start, and show a warning:

It will show an option to Reauthorize transform for both individual and bulk:
Reauthorizing transform will create an API key using the user's credential, call transform/_update with the es-secondary-authorization: 'ApiKey {encoded_api_key} in the headers, and start the transform.

If the transform was originally created with sufficient permission:

No Reauthorize option will be shown:

This PR also updates the behavior of the tooltip message when a bulk action should be disabled.

WIP:

API integration test
Functional test (follow up PR)

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
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

…ment-reauthorize

…com/qn895/kibana into ml-transform-management-reauthorize

qn895 · 2023-04-12T14:06:33Z

@elasticmachine merge upstream

elasticmachine · 2023-04-12T14:07:06Z

Pinging @elastic/ml-ui (:ml)

.../plugins/transform/public/app/sections/transform_management/transform_management_section.tsx

...transform/public/app/sections/transform_management/components/transform_list/use_columns.tsx

...app/sections/transform_management/components/action_reauthorize/reauthorize_action_modal.tsx

walterra · 2023-04-13T15:05:20Z

x-pack/plugins/transform/common/utils/transform_api_key.ts

+
+export function isTransformApiKey(arg: any): arg is TransformAPIKey {
+  return (
+    arg &&


You could do isPopulated(arg, ['api_key', 'encoded']) && typeof arg.encoded === string here.

Updated here 3f64b1f (#154736)

walterra · 2023-04-13T15:16:48Z

x-pack/plugins/transform/server/routes/api/transforms.ts

@@ -295,6 +303,66 @@ export function registerTransformsRoutes(routeDependencies: RouteDependencies) {
    )
  );

+  /**
+   * @apiGroup Reauthorize transforms with API key generated from currently logged in suer


Type suer -> user

Updated here 3f64b1f (#154736)

walterra · 2023-04-13T15:19:01Z

x-pack/plugins/transform/server/types.ts

@@ -5,14 +5,14 @@
 * 2.0.
 */

-import type { IRouter, CoreStart } from '@kbn/core/server';


Can we keep the type exports here or is there a reason to remove them?

Updated here 3f64b1f (#154736)

…ment-reauthorize

…com/qn895/kibana into ml-transform-management-reauthorize

peteharverson · 2023-04-17T09:34:19Z

...transform/public/app/sections/transform_management/components/transform_list/use_columns.tsx

+                'xpack.transform.transformList.needsReauthorizationBadgeTooltip',
+                {
+                  defaultMessage:
+                    'This transform was created with insufficient permissions. You must have manage_transform cluster privileges to reauthorize and start it.',


@szabosteve @lcawl is it common practice to mention specific cluster privileges that are required? Wondering as in other places we aren't so specific:

After a quick check, I found that although we rarely mention specific cluster privileges in the UI, there is precedent for doing it, for example in the APM app when you don't have the correct privileges to create anomaly detection jobs. While there is precedent, I would go without mentioning the privileges specifically. The docs mention the correct privileges and we consider them as the source of truth, so it might make sense to keep only the docs up-to-date – even if the name of the privileges is unlikely to change.
@peteharverson @lcawl WDYT?

Suggested change

'This transform was created with insufficient permissions. You must have manage_transform cluster privileges to reauthorize and start it.',

'This transform was created with insufficient permissions. Contact your administrator to request the required privileges.',

+1 to your suggestion @szabosteve to not mention the privileges specifically.

Updated here (1a8969a)

Yup, that's even better, thanks!

.../plugins/transform/public/app/sections/transform_management/transform_management_section.tsx

x-pack/plugins/transform/server/routes/api/transforms.ts

…ove createTransformWithHeaders

peteharverson

Tested with your example Host Risk Score packages and overall looks good.

A couple of the warning messages probably need a further edit.

peteharverson · 2023-04-19T11:58:26Z

...transform/public/app/sections/transform_management/components/transform_list/use_columns.tsx

+                'xpack.transform.transformList.needsReauthorizationBadgeTooltip',
+                {
+                  defaultMessage:
+                    'This transform was created with insufficient permissions. Contact your administrator to request the required privileges.',


This message should be adjusted if logging in as a user with the correct permissions, otherwise it gives the impression that the current user does not have the required permissions.

Something along the lines of

This transform was installed by a user who did not have the permissions required to run it. Select Reauthorize in the Actions menu to reauthorize.

(too many 'reauthorize's! Any suggestions @szabosteve ?

How about This transform was installed by a user who did not have the permissions required to run it. Reauthorize it in the Actions menu.?

In the case of the user without proper permissions, I propose to use the following: This transform was created with insufficient permissions. Contact your administrator to request the required permissions. So we can use a single term instead of permissions and privileges.

(I don't use suggesting mode because it would be more confusing.)

Updated here 1775dd0 (#154736)

For when user has permissions:

peteharverson · 2023-04-19T11:59:53Z

.../plugins/transform/public/app/sections/transform_management/transform_management_section.tsx

+          color="warning"
+          title={i18n.translate('xpack.transform.transformList.unauthorizedTransformsCallout', {
+            defaultMessage:
+              '{unauthorizedCnt, plural, one {A transform was installed but requires more permissions to run.} other {# transforms were installed but require more permissions to run.}} Contact your administrator to request the required privileges.',


As above, this message should be adjusted if logging back in as a user with the correct permissions.

Nit - the text refers to both 'permissions' and 'privileges'. Should we use a single term here @szabosteve?

We can use the same string as above if the user has correct permissions: A transform was installed by a user who did not have the permissions required to run it. Reauthorize it in the Actions menu.

In the case of the user without proper permissions, I propose to use the following: This transform was created with insufficient permissions. Contact your administrator to request the required permissions. So we can use a single term instead of permissions and privileges.

(I don't use suggesting mode because it would be more confusing.)

Updated here 1775dd0 (#154736)

…ment-reauthorize

.../plugins/transform/public/app/sections/transform_management/transform_management_section.tsx

peteharverson

Latest text edits LGTM.

walterra

Latest changes code LGTM.

…ment-reauthorize

kibana-ci · 2023-04-20T17:45:49Z

💚 Build Succeeded

Buildkite Build
Commit: 6d9a641

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`transform`	318	325	+7

Async chunks

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

id	before	after	diff
`transform`	376.3KB	383.0KB	+6.8KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	394	397	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	474	477	+3

History

💛 Build #121618 was flaky b539864
💛 Build #121356 was flaky 5329239
💛 Build #120358 was flaky 1a8969a
💛 Build #120129 was flaky c44929b
💔 Build #120125 failed 9e73407
💛 Build #119432 was flaky 84b9ad3

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

cc @qn895

qn895 added 4 commits April 10, 2023 15:31

Update logic for needs authorization check

707b75b

Update types

ec5a456

[ML] Update reauthorization

2aa006d

[ML] Fix disabled messages for reauthorize, delete, reset

db82d60

qn895 requested review from walterra, peteharverson and lcawl April 11, 2023 15:03

qn895 self-assigned this Apr 11, 2023

qn895 added :ml Feature:Transforms ML transforms v8.8.0 labels Apr 11, 2023

qn895 added 6 commits April 11, 2023 17:58

[ML] Revert change to delete and reset action

fc6088d

Merge remote-tracking branch 'upstream/main' into ml-transform-manage…

d415f40

…ment-reauthorize

[ML] Fix i18n

f5319b2

[ML] Fix i18n and messaging

f94acda

[ML] Fix i18n and messaging

5f43e8d

Merge branch 'ml-transform-management-reauthorize' of https://github.…

970ee8f

…com/qn895/kibana into ml-transform-management-reauthorize

Merge branch 'main' into ml-transform-management-reauthorize

84b9ad3

qn895 added the release_note:enhancement label Apr 12, 2023

qn895 marked this pull request as ready for review April 12, 2023 14:07

qn895 requested a review from a team as a code owner April 12, 2023 14:07

lcawl reviewed Apr 12, 2023

View reviewed changes

lcawl added the ui-copy Review of UI copy with docs team is recommended label Apr 12, 2023

walterra reviewed Apr 13, 2023

View reviewed changes

qn895 added 4 commits April 15, 2023 18:45

Merge remote-tracking branch 'upstream/main' into ml-transform-manage…

0cea051

…ment-reauthorize

Fix messaging and is populated obj

3f64b1f

Add new reauthorize transforms tests

e8d16e7

Merge branch 'ml-transform-management-reauthorize' of https://github.…

9e73407

…com/qn895/kibana into ml-transform-management-reauthorize

Remove only

c44929b

peteharverson reviewed Apr 17, 2023

View reviewed changes

peteharverson mentioned this pull request Apr 17, 2023

[ML] Increase Test Coverage 8.8.0 #152009

Closed

7 tasks

peteharverson reviewed Apr 17, 2023

View reviewed changes

.../plugins/transform/public/app/sections/transform_management/transform_management_section.tsx Outdated Show resolved Hide resolved

x-pack/plugins/transform/server/routes/api/transforms.ts Outdated Show resolved Hide resolved

qn895 added 2 commits April 17, 2023 10:14

Refactor createTransform to have headers/deferValidation options. Rem…

bc40330

…ove createTransformWithHeaders

Update texts

1a8969a

szabosteve mentioned this pull request Apr 18, 2023

[ML] Adds secondary authorization header to Transforms in Fleet #154665

Merged

9 tasks

peteharverson reviewed Apr 19, 2023

View reviewed changes

qn895 added 3 commits April 19, 2023 10:58

Merge remote-tracking branch 'upstream/main' into ml-transform-manage…

a1b0374

…ment-reauthorize

Update messaging

1775dd0

Add order sequential install

5329239

peteharverson reviewed Apr 20, 2023

View reviewed changes

.../plugins/transform/public/app/sections/transform_management/transform_management_section.tsx Outdated Show resolved Hide resolved

Update translations

b539864

peteharverson approved these changes Apr 20, 2023

View reviewed changes

walterra approved these changes Apr 20, 2023

View reviewed changes

qn895 added 2 commits April 20, 2023 11:39

Handle case where security is not enabled

230e386

Merge remote-tracking branch 'upstream/main' into ml-transform-manage…

6d9a641

…ment-reauthorize

qn895 merged commit dd46350 into elastic:main Apr 20, 2023

qn895 deleted the ml-transform-management-reauthorize branch April 20, 2023 18:16

kibanamachine added the backport:skip This commit does not require backporting label Apr 20, 2023

szabosteve changed the title ~~[ML] Add option to Reauthorize transform in Management page~~ [ML] Adds option to Reauthorize transform in Management page Apr 21, 2023

qn895 mentioned this pull request Apr 25, 2023

[ML] Transform: support for create/update with API key for secondary credentials #151186

Closed

mgiota mentioned this pull request Aug 22, 2024

[SLOs] remove manage_transform and manage_ingest_pipeline privilege requirements #190572

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Adds option to Reauthorize transform in Management page #154736

[ML] Adds option to Reauthorize transform in Management page #154736

qn895 commented Apr 11, 2023 •

edited

Loading

qn895 commented Apr 12, 2023

elasticmachine commented Apr 12, 2023

walterra Apr 13, 2023

qn895 Apr 17, 2023

walterra Apr 13, 2023

qn895 Apr 17, 2023

walterra Apr 13, 2023

qn895 Apr 17, 2023

peteharverson Apr 17, 2023

szabosteve Apr 17, 2023 •

edited

Loading

peteharverson Apr 17, 2023

qn895 Apr 17, 2023

lcawl Apr 17, 2023

peteharverson left a comment

peteharverson Apr 19, 2023

szabosteve Apr 19, 2023 •

edited

Loading

qn895 Apr 19, 2023

peteharverson Apr 19, 2023

szabosteve Apr 19, 2023 •

edited

Loading

qn895 Apr 19, 2023

peteharverson left a comment

walterra left a comment

kibana-ci commented Apr 20, 2023

ESLint disabled line counts

Total ESLint disabled count

	'This transform was created with insufficient permissions. You must have manage_transform cluster privileges to reauthorize and start it.',
	'This transform was created with insufficient permissions. Contact your administrator to request the required privileges.',

[ML] Adds option to Reauthorize transform in Management page #154736

[ML] Adds option to Reauthorize transform in Management page #154736

Conversation

qn895 commented Apr 11, 2023 • edited Loading

Summary

If the transform was originally created with insufficient permission

If the transform was originally created with sufficient permission:

WIP:

Checklist

Risk Matrix

For maintainers

qn895 commented Apr 12, 2023

elasticmachine commented Apr 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szabosteve Apr 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szabosteve Apr 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szabosteve Apr 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 20, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

qn895 commented Apr 11, 2023 •

edited

Loading

szabosteve Apr 17, 2023 •

edited

Loading

szabosteve Apr 19, 2023 •

edited

Loading

szabosteve Apr 19, 2023 •

edited

Loading