Migrate config deprecations and `ShieldUser` functionality to the New Platform by azasypkin · Pull Request #53768 · elastic/kibana

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

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

@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

-          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

-            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

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 Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// 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 jportner and mattapperson 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

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

Conversation

azasypkin commented Dec 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dev Docs [Security Plugin]

Uh oh!

elasticmachine commented Dec 23, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Dec 23, 2019

💔 Build Failed

Uh oh!

kibanamachine commented Dec 23, 2019

💚 Build Succeeded

Uh oh!

jportner commented Jan 2, 2020

Uh oh!

jportner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azasypkin Jan 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azasypkin commented Jan 3, 2020

Uh oh!

jportner left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jan 6, 2020

💚 Build Succeeded

History

Uh oh!

azasypkin commented Jan 6, 2020

Uh oh!

azasypkin commented Jan 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

azasypkin commented Dec 23, 2019 •

edited

Loading

azasypkin Jan 3, 2020 •

edited

Loading