Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions x-pack/legacy/plugins/security/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,31 +28,16 @@ export const security = kibana =>
enabled: Joi.boolean().default(true),
cookieName: HANDLED_IN_NEW_PLATFORM,
encryptionKey: HANDLED_IN_NEW_PLATFORM,
session: Joi.object({
idleTimeout: HANDLED_IN_NEW_PLATFORM,
lifespan: HANDLED_IN_NEW_PLATFORM,
}).default(),
session: HANDLED_IN_NEW_PLATFORM,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

secureCookies: HANDLED_IN_NEW_PLATFORM,
loginAssistanceMessage: HANDLED_IN_NEW_PLATFORM,
authorization: Joi.object({
legacyFallback: Joi.object({
enabled: Joi.boolean().default(true), // deprecated
}).default(),
}).default(),
audit: Joi.object({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

enabled: Joi.boolean().default(false),
}).default(),
authc: HANDLED_IN_NEW_PLATFORM,
}).default();
},

deprecations: function({ rename, unused }) {
return [
unused('authorization.legacyFallback.enabled'),
rename('sessionTimeout', 'session.idleTimeout'),
];
},

uiExports: {
chromeNavControls: [],
managementSections: ['plugins/security/views/management'],
Expand Down
15 changes: 1 addition & 14 deletions x-pack/plugins/security/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import crypto from 'crypto';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { schema, Type, TypeOf } from '@kbn/config-schema';
import { duration } from 'moment';
import { PluginInitializerContext } from '../../../../src/core/server';

export type ConfigType = ReturnType<typeof createConfig$> extends Observable<infer P>
Expand All @@ -35,7 +34,6 @@ export const ConfigSchema = schema.object(
schema.maybe(schema.string({ minLength: 32 })),
schema.string({ minLength: 32, defaultValue: 'a'.repeat(32) })
),
sessionTimeout: schema.maybe(schema.nullable(schema.number())), // DEPRECATED
session: schema.object({
idleTimeout: schema.nullable(schema.duration()),
lifespan: schema.nullable(schema.duration()),
Expand Down Expand Up @@ -88,22 +86,11 @@ export function createConfig$(context: PluginInitializerContext, isTLSEnabled: b
secureCookies = true;
}

// "sessionTimeout" is deprecated and replaced with "session.idleTimeout"
// however, NP does not yet have a mechanism to automatically rename deprecated keys
// for the time being, we'll do it manually:
const deprecatedSessionTimeout =
typeof config.sessionTimeout === 'number' ? duration(config.sessionTimeout) : null;
const val = {
return {
...config,
encryptionKey,
secureCookies,
session: {
...config.session,
idleTimeout: config.session.idleTimeout || deprecatedSessionTimeout,
},
};
delete val.sessionTimeout; // DEPRECATED
return val;
})
);
}
27 changes: 21 additions & 6 deletions x-pack/plugins/security/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,30 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { PluginInitializerContext } from '../../../../src/core/server';
import { TypeOf } from '@kbn/config-schema';
import {
PluginConfigDescriptor,
PluginInitializer,
PluginInitializerContext,
RecursiveReadonly,
} from '../../../../src/core/server';
import { ConfigSchema } from './config';
import { Plugin } from './plugin';
import { Plugin, PluginSetupContract, PluginSetupDependencies } from './plugin';

// These exports are part of public Security plugin contract, any change in signature of exported
// functions or removal of exports should be considered as a breaking change.
export { AuthenticationResult, DeauthenticationResult, CreateAPIKeyResult } from './authentication';
export { PluginSetupContract } from './plugin';
export { PluginSetupContract };

export const config = { schema: ConfigSchema };
export const plugin = (initializerContext: PluginInitializerContext) =>
new Plugin(initializerContext);
export const config: PluginConfigDescriptor<TypeOf<typeof ConfigSchema>> = {
schema: ConfigSchema,
deprecations: ({ rename, unused }) => [
rename('sessionTimeout', 'session.idleTimeout'),
unused('authorization.legacyFallback.enabled'),
],
};
export const plugin: PluginInitializer<
RecursiveReadonly<PluginSetupContract>,
void,
PluginSetupDependencies
> = (initializerContext: PluginInitializerContext) => new Plugin(initializerContext);
7 changes: 2 additions & 5 deletions x-pack/plugins/security/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,7 @@ export class Plugin {
this.logger = this.initializerContext.logger.get();
}

public async setup(
core: CoreSetup,
{ features, licensing }: PluginSetupDependencies
): Promise<RecursiveReadonly<PluginSetupContract>> {
public async setup(core: CoreSetup, { features, licensing }: PluginSetupDependencies) {
const [config, legacyConfig] = await combineLatest([
createConfig$(this.initializerContext, core.http.isTlsEnabled),
this.initializerContext.config.legacy.globalConfig$,
Expand Down Expand Up @@ -169,7 +166,7 @@ export class Plugin {
csp: core.http.csp,
});

return deepFreeze({
return deepFreeze<PluginSetupContract>({
authc,

authz: {
Expand Down