Migrate config deprecations and `ShieldUser` functionality to the New Platform #53768

azasypkin · 2019-12-23T14:46:25Z

There are two changes in this PR (better to review commit by commit):

Config deprecation transformations moved to the New Platform plugin
ShieldUser is removed and replaced with getCurrentUser exposed by the New Platform plugin setup contract (unified with the same API on server side).

Dev Docs [Security Plugin]

Legacy ShieldUser angular service has been removed and replaced with the dedicated method on the Kibana platform plugin setup contract:

Before:

const currentUser = await $injector.get('ShieldUser').getCurrent().$promise;

Now:

Legacy plugin:

import { npSetup } from 'ui/new_platform';
const currentUser = await npSetup.plugins.security.authc.getCurrentUser();

Kibana platform plugin:

// manifest.json
....
[optional]requiredPlugins: ["security"],
....

// my_plugin/public/plugin.ts
public setup(core: CoreSetup, { security }: PluginSetupDependencies) {
    const currentUser = await security.authc.getCurrentUser();
}

elasticmachine · 2019-12-23T14:46:27Z

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

azasypkin · 2019-12-23T15:23:57Z

x-pack/plugins/security/public/plugin.ts

    });

    return {
-      anonymousPaths,


note: I can't think of any reason why we'd want to re-expose part of the core's own contract.

I also took a look and I agree, seems unnecessary

azasypkin · 2019-12-23T15:24:45Z

...ck/legacy/plugins/beats_management/public/lib/adapters/framework/kibana_framework_adapter.ts

@@ -11,6 +11,8 @@ import * as React from 'react';
 import * as ReactDOM from 'react-dom';
 import { UIRoutes } from 'ui/routes';
 import { isLeft } from 'fp-ts/lib/Either';
+import { npSetup } from 'ui/new_platform';


@mattapperson, I'm not sure why Beats team wasn't pinged by the GitHub automatically, but I see you were editing this file before, would you mind reviewing this single change in beats_management plugin?

Pulling and checking now

Hey @mattapperson, did you have chance to take a look at this change?

kibanamachine · 2019-12-23T16:03:42Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 2ba8d2853237004844c04e586a0e16d7877f26c3

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

kibanamachine · 2019-12-23T18:29:04Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: ec92eca66e120622a75d97126fc7c6457286e6e0

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

jportner · 2020-01-02T17:35:05Z

Reviewing now!

jportner

Good stuff. Found one problem, and had one nit.

jportner · 2020-01-02T17:56:08Z

x-pack/legacy/plugins/security/index.js

-        session: Joi.object({
-          idleTimeout: HANDLED_IN_NEW_PLATFORM,
-          lifespan: HANDLED_IN_NEW_PLATFORM,
-        }).default(),
+        session: HANDLED_IN_NEW_PLATFORM,


jportner · 2020-01-02T18:22:43Z

x-pack/legacy/plugins/security/index.js

-          legacyFallback: Joi.object({
-            enabled: Joi.boolean().default(true), // deprecated
-          }).default(),
-        }).default(),
        audit: Joi.object({


Suggested change

audit: Joi.object({

authorization: HANDLED_IN_NEW_PLATFORM,

audit: Joi.object({

Need to specify this in the legacy plugin config, otherwise Kibana will crash if you have specified xpack.security.authorization.legacyFallback.enabled:

server log [12:52:10.787] [fatal][root] { ValidationError: child "xpack" fails because [child "security" fails because ["authorization" is not allowed]] at Object.exports.process (/Users/joe/GitHub/kibana/node_modules/joi/lib/errors.js:196:19) at internals.Object._validateWithOptions (/Users/joe/GitHub/kibana/node_modules/joi/lib/types/any/index.js:675:31) at module.exports.internals.Any.validate (/Users/joe/GitHub/kibana/node_modules/joi/lib/index.js:146:23) at Config._commit (/Users/joe/GitHub/kibana/src/legacy/server/config/config.js:124:25) at Config.set (/Users/joe/GitHub/kibana/src/legacy/server/config/config.js:89:10) at Config.extendSchema (/Users/joe/GitHub/kibana/src/legacy/server/config/config.js:62:10) at extendConfigService (/Users/joe/GitHub/kibana/src/legacy/plugin_discovery/plugin_config/extend_config_service.js:36:10) name: 'ValidationError' } FATAL ValidationError: child "xpack" fails because [child "security" fails because ["authorization" is not allowed]]

Wow, good catch, I could swear it worked when I tested it at the early stages of this PR 🙈 There is a chance I'm making this up though, let me check.

I wanted to check if it's indented behavior (that NP passes non-transformed config to the LP), but Platform team is out this week. Will just use your suggestion to not be blocked!

jportner · 2020-01-02T18:37:33Z

...legacy/plugins/security/public/views/management/edit_user/components/edit_user_page.test.tsx

-  await Promise.resolve();
-  await Promise.resolve();
-  await Promise.resolve();
-  await Promise.resolve();


jportner · 2020-01-02T19:01:25Z

x-pack/plugins/security/public/plugin.ts

    });

    return {
-      anonymousPaths,


I also took a look and I agree, seems unnecessary

jportner · 2020-01-02T19:27:49Z

x-pack/plugins/security/public/nav_control/nav_control_service.tsx

 interface SetupDeps {
  securityLicense: SecurityLicense;
+  getCurrentUser: AuthenticationServiceSetup['getCurrentUser'];
 }


I'm just curious, is there any particular reason you decided to use such a specific contract?

My 2 cents is that it would have sufficed to just use AuthenticationServiceSetup as a parameter -- less verbose that way, and less TypeScript jargon / easier to understand for new developers.
I noticed that SecurityLicense is also more generic (e.g., we don't specify SecurityLicense['feature$'] here, even though we could).

I'm asking myself the same question in my next PR that is based on this one - and I totally agree that it looks weird and unnecessary. Can't recall why I did this, must be I'll think about it later, but never get back to it.... kind of thing 🙈 I'll fix it, thanks!

jportner · 2020-01-02T19:28:25Z

x-pack/plugins/security/public/session/session_timeout.tsx

-export class SessionTimeout {
+export class SessionTimeout implements ISessionTimeout {


Haha, not a big deal though - I've heard platform will be abandoning I{interface_name} convention soon (with TS 3.8 upgrade) since we used it mostly to workaround the absence of proper "private fields" support and with TS 3.8 it'd not make too much sense to create a dedicated interface if it completely replicates public part of the class definition.

… keep `authorization` in the legacy config schema.

azasypkin · 2020-01-03T11:32:28Z

@jportner thanks for review, PR is ready for another pass!

jportner

LGTM!

kibanamachine · 2020-01-06T10:13:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7f7c271

History

💚 Build #17723 succeeded e9783d4
💚 Build #17517 succeeded 9e225ef
💚 Build #17323 succeeded 2dbccd1af0e9c7bc764cbb2eaa0f5448a7cf37eb

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

azasypkin · 2020-01-06T10:43:08Z

@mattapperson I'm going to move forward and merge this PR now to unblock other migration work we have. I've tested affected pieces in Beats CM and Logstash Pipelines and everything seems to be in order, but feel free to reach out to me if you have any concerns!

* master: adds strict types to Alerting Client (elastic#53821) [Dashboard] Empty screen redesign (elastic#53681) Migrate config deprecations and `ShieldUser` functionality to the New Platform (elastic#53768)

…nsole-dependencies * 'master' of github.com:elastic/kibana: (33 commits) adds strict types to Alerting Client (elastic#53821) [Dashboard] Empty screen redesign (elastic#53681) Migrate config deprecations and `ShieldUser` functionality to the New Platform (elastic#53768) increase delay to make sure license refetched (elastic#53882) Allow custom NP plugin paths in production (elastic#53562) [Maps] show custom color ramps in legend (elastic#53780) [Lens] Expression type on document can be null (elastic#53883) [SIEM] [Detection engine] Add user permission to detection engine (elastic#53778) Update dependency @elastic/charts to v16.0.2 (elastic#52619) Set consistent EOL symbol in core API docs (elastic#53815) [Logs UI] Refactor query bar state to hooks (elastic#52656) [Maps] pass getFieldFormatter to DynamicTextProperty (elastic#53937) Invalidate alert API Key when generating a new one (elastic#53732) [Logs UI] HTTP API for log entries (elastic#53798) [kbn/pm] add caching to bootstrap (elastic#53622) adds createdAt and updatedAt fields to alerting (elastic#53793) [SR] Enable component integration tests (elastic#53893) Move index patterns: src/legacy/core_plugins/data 👉 src/plugins/data (elastic#53794) moved Task Manager server code under "server" directory (elastic#53777) Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886) ... # Conflicts: # yarn.lock

azasypkin · 2020-01-06T12:12:45Z

7.x/7.6.0: 12f4761

* master: allows Alerts to recover gracefully from Executor errors (elastic#53688) [Console] Fix OSS build (elastic#53885) migrate xsrf / version-check / custom-headers handlers to NP (elastic#53684) use NP deprecations in uiSettings (elastic#53755) adds strict types to Alerting Client (elastic#53821) [Dashboard] Empty screen redesign (elastic#53681) Migrate config deprecations and `ShieldUser` functionality to the New Platform (elastic#53768)

azasypkin added chore Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:NP Migration labels Dec 23, 2019

azasypkin requested a review from a team as a code owner December 23, 2019 14:46

azasypkin added release_note:skip Skip the PR/issue when compiling release notes v7.6.0 release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. and removed release_note:skip Skip the PR/issue when compiling release notes labels Dec 23, 2019

azasypkin commented Dec 23, 2019

View reviewed changes

azasypkin force-pushed the issue-xxx-np-security branch from ec92eca to 2dbccd1 Compare December 27, 2019 11:56

azasypkin added 2 commits January 2, 2020 09:24

Move config deprecations to the new plugin.

01d1fbc

Migrate ShieldUser functionality to a Kibana platform plugin

9e225ef

azasypkin force-pushed the issue-xxx-np-security branch from 2dbccd1 to 9e225ef Compare January 2, 2020 08:24

azasypkin mentioned this pull request Jan 2, 2020

Migrate Management views to Kibana Platform plugin #53880

Merged

azasypkin requested a review from jportner January 2, 2020 10:26

jportner reviewed Jan 2, 2020

View reviewed changes

azasypkin added 2 commits January 3, 2020 10:37

Merge branch 'master' into issue-xxx-np-security

75659fc

Review#1: pass full authc setup contract to nav control service and…

e9783d4

… keep `authorization` in the legacy config schema.

azasypkin requested review from mattapperson and jportner January 3, 2020 11:32

jportner approved these changes Jan 3, 2020

View reviewed changes

Merge branch 'master' into issue-xxx-np-security

7f7c271

azasypkin merged commit aa38fb6 into elastic:master Jan 6, 2020

azasypkin deleted the issue-xxx-np-security branch January 6, 2020 10:43

azasypkin added the backport pending label Jan 6, 2020

azasypkin mentioned this pull request Jan 6, 2020

[7.x] Migrate config deprecations and ShieldUser functionality to the New Platform (#53768) #54004

Merged

azasypkin added backported and removed backport pending labels Jan 6, 2020

jportner mentioned this pull request Jan 7, 2020

Kibana crashing when xpack.security.sessionTimeout is set to a string #54172

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate config deprecations and `ShieldUser` functionality to the New Platform #53768

Migrate config deprecations and `ShieldUser` functionality to the New Platform #53768

azasypkin commented Dec 23, 2019 •

edited

Loading

elasticmachine commented Dec 23, 2019

azasypkin Dec 23, 2019

jportner Jan 2, 2020

azasypkin Dec 23, 2019

mattapperson Dec 23, 2019

azasypkin Jan 2, 2020

kibanamachine commented Dec 23, 2019

kibanamachine commented Dec 23, 2019

jportner commented Jan 2, 2020

jportner left a comment

jportner Jan 2, 2020

jportner Jan 2, 2020

azasypkin Jan 3, 2020

azasypkin Jan 3, 2020

jportner Jan 2, 2020

jportner Jan 2, 2020

jportner Jan 2, 2020

azasypkin Jan 3, 2020 •

edited

Loading

jportner Jan 2, 2020

azasypkin Jan 3, 2020

azasypkin commented Jan 3, 2020

jportner left a comment

kibanamachine commented Jan 6, 2020

azasypkin commented Jan 6, 2020

azasypkin commented Jan 6, 2020

	audit: Joi.object({
	authorization: HANDLED_IN_NEW_PLATFORM,
	audit: Joi.object({

		export class SessionTimeout {
		export class SessionTimeout implements ISessionTimeout {

Migrate config deprecations and ShieldUser functionality to the New Platform #53768

Migrate config deprecations and ShieldUser functionality to the New Platform #53768

Conversation

azasypkin commented Dec 23, 2019 • edited Loading

Dev Docs [Security Plugin]

elasticmachine commented Dec 23, 2019

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

kibanamachine commented Dec 23, 2019

💔 Build Failed

kibanamachine commented Dec 23, 2019

💚 Build Succeeded

jportner commented Jan 2, 2020

jportner left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azasypkin Jan 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azasypkin commented Jan 3, 2020

jportner left a comment

Choose a reason for hiding this comment

kibanamachine commented Jan 6, 2020

💚 Build Succeeded

History

azasypkin commented Jan 6, 2020

azasypkin commented Jan 6, 2020

Migrate config deprecations and `ShieldUser` functionality to the New Platform #53768

Migrate config deprecations and `ShieldUser` functionality to the New Platform #53768

azasypkin commented Dec 23, 2019 •

edited

Loading

azasypkin Jan 3, 2020 •

edited

Loading