From aba82005331261b9e644f20c21aea7f9e045c8e4 Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Mon, 16 Sep 2019 15:51:26 +0200 Subject: [PATCH] [7.x] Properly apply `rename` deprecation to `xpack.security.authProviders`. (#45755) --- x-pack/plugins/security/server/config.test.ts | 120 ++++++++++++++---- x-pack/plugins/security/server/config.ts | 32 +++-- 2 files changed, 112 insertions(+), 40 deletions(-) diff --git a/x-pack/plugins/security/server/config.test.ts b/x-pack/plugins/security/server/config.test.ts index 1207d9bb0def72..4e09bb8be78398 100644 --- a/x-pack/plugins/security/server/config.test.ts +++ b/x-pack/plugins/security/server/config.test.ts @@ -182,11 +182,23 @@ describe('config schema', () => { `"[authc.oidc.realm]: expected value of type [string] but got [undefined]"` ); + expect(() => + ConfigSchema.validate({ authProviders: ['oidc'] }) + ).toThrowErrorMatchingInlineSnapshot( + `"[authc.oidc.realm]: expected value of type [string] but got [undefined]"` + ); + expect(() => ConfigSchema.validate({ authc: { providers: ['oidc'], oidc: {} } }) ).toThrowErrorMatchingInlineSnapshot( `"[authc.oidc.realm]: expected value of type [string] but got [undefined]"` ); + + expect(() => + ConfigSchema.validate({ authProviders: ['oidc'], authc: { oidc: {} } }) + ).toThrowErrorMatchingInlineSnapshot( + `"[authc.oidc.realm]: expected value of type [string] but got [undefined]"` + ); }); it(`is valid when authc.providers is "['oidc']" and realm is specified`, async () => { @@ -204,6 +216,22 @@ describe('config schema', () => { ], } `); + + expect( + ConfigSchema.validate({ + authProviders: ['oidc'], + authc: { oidc: { realm: 'realm-1' } }, + }).authc + ).toMatchInlineSnapshot(` + Object { + "oidc": Object { + "realm": "realm-1", + }, + "providers": Array [ + "oidc", + ], + } + `); }); it(`returns a validation error when authc.providers is "['oidc', 'basic']" and realm is unspecified`, async () => { @@ -212,6 +240,12 @@ describe('config schema', () => { ).toThrowErrorMatchingInlineSnapshot( `"[authc.oidc.realm]: expected value of type [string] but got [undefined]"` ); + + expect(() => + ConfigSchema.validate({ authProviders: ['oidc', 'basic'] }) + ).toThrowErrorMatchingInlineSnapshot( + `"[authc.oidc.realm]: expected value of type [string] but got [undefined]"` + ); }); it(`is valid when authc.providers is "['oidc', 'basic']" and realm is specified`, async () => { @@ -230,12 +264,33 @@ describe('config schema', () => { ], } `); + + expect( + ConfigSchema.validate({ + authProviders: ['oidc', 'basic'], + authc: { oidc: { realm: 'realm-1' } }, + }).authc + ).toMatchInlineSnapshot(` + Object { + "oidc": Object { + "realm": "realm-1", + }, + "providers": Array [ + "oidc", + "basic", + ], + } + `); }); it(`realm is not allowed when authc.providers is "['basic']"`, async () => { expect(() => ConfigSchema.validate({ authc: { providers: ['basic'], oidc: { realm: 'realm-1' } } }) ).toThrowErrorMatchingInlineSnapshot(`"[authc.oidc]: a value wasn't expected to be present"`); + + expect(() => + ConfigSchema.validate({ authProviders: ['basic'], authc: { oidc: { realm: 'realm-1' } } }) + ).toThrowErrorMatchingInlineSnapshot(`"[authc.oidc]: a value wasn't expected to be present"`); }); }); @@ -251,6 +306,15 @@ describe('config schema', () => { } `); + expect(ConfigSchema.validate({ authProviders: ['saml'] }).authc).toMatchInlineSnapshot(` + Object { + "providers": Array [ + "saml", + ], + "saml": Object {}, + } + `); + expect(ConfigSchema.validate({ authc: { providers: ['saml'], saml: {} } }).authc) .toMatchInlineSnapshot(` Object { @@ -261,6 +325,16 @@ describe('config schema', () => { } `); + expect(ConfigSchema.validate({ authProviders: ['saml'], authc: { saml: {} } }).authc) + .toMatchInlineSnapshot(` + Object { + "providers": Array [ + "saml", + ], + "saml": Object {}, + } + `); + expect( ConfigSchema.validate({ authc: { providers: ['saml'], saml: { realm: 'realm-1' } }, @@ -275,12 +349,32 @@ describe('config schema', () => { }, } `); + + expect( + ConfigSchema.validate({ + authProviders: ['saml'], + authc: { saml: { realm: 'realm-1' } }, + }).authc + ).toMatchInlineSnapshot(` + Object { + "providers": Array [ + "saml", + ], + "saml": Object { + "realm": "realm-1", + }, + } + `); }); it('`realm` is not allowed if saml provider is not enabled', async () => { expect(() => ConfigSchema.validate({ authc: { providers: ['basic'], saml: { realm: 'realm-1' } } }) ).toThrowErrorMatchingInlineSnapshot(`"[authc.saml]: a value wasn't expected to be present"`); + + expect(() => + ConfigSchema.validate({ authProviders: ['basic'], authc: { saml: { realm: 'realm-1' } } }) + ).toThrowErrorMatchingInlineSnapshot(`"[authc.saml]: a value wasn't expected to be present"`); }); }); }); @@ -403,30 +497,4 @@ describe('createConfig$()', () => { expect(loggingServiceMock.collect(contextMock.logger).warn).toEqual([]); }); - - it('should set authProviders to authc.providers', async () => { - const contextMock = coreMock.createPluginInitializerContext({ - authProviders: ['saml', 'basic'], - }); - - const config = await createConfig$(contextMock, true) - .pipe(first()) - .toPromise(); - expect(config).toMatchInlineSnapshot(` - Object { - "authProviders": Array [ - "saml", - "basic", - ], - "authc": Object { - "providers": Array [ - "saml", - "basic", - ], - }, - "encryptionKey": "abababababababababababababababab", - "secureCookies": true, - } - `); - }); }); diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts index b9b6d2b77d3880..9a379e2263021c 100644 --- a/x-pack/plugins/security/server/config.ts +++ b/x-pack/plugins/security/server/config.ts @@ -40,9 +40,6 @@ export const ConfigSchema = schema.object( hostname: schema.maybe(schema.string({ hostname: true })), port: schema.maybe(schema.number({ min: 0, max: 65535 })), }), - // This property is deprecated, but we include it here since new platform doesn't support config deprecation - // transformations yet (e.g. `rename`), so we handle it manually for now. - authProviders: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1 })), authc: schema.object({ providers: schema.arrayOf(schema.string(), { defaultValue: ['basic'], minSize: 1 }), oidc: providerOptionsSchema('oidc', schema.maybe(schema.object({ realm: schema.string() }))), @@ -56,6 +53,23 @@ export const ConfigSchema = schema.object( { allowUnknowns: true } ); +// HACK: Since new platform doesn't support config deprecation transformations yet (e.g. `rename`), we have to handle +// them manually here for the time being. Legacy platform config will log corresponding deprecation warnings. +const origValidate = ConfigSchema.validate; +ConfigSchema.validate = (value, context, namespace) => { + // Rename deprecated `xpack.security.authProviders` to `xpack.security.authc.providers`. + if (value && value.authProviders) { + value.authc = { + ...(value.authc || {}), + providers: value.authProviders, + }; + + delete value.authProviders; + } + + return origValidate.call(ConfigSchema, value, context, namespace); +}; + export function createConfig$(context: PluginInitializerContext, isTLSEnabled: boolean) { return context.config.create>().pipe( map(config => { @@ -87,17 +101,7 @@ export function createConfig$(context: PluginInitializerContext, isTLSEnabled: b secureCookies = true; } - return { - ...config, - encryptionKey, - secureCookies, - authc: { - ...config.authc, - // If deprecated `authProviders` is specified that most likely means that `authc.providers` isn't, but if it's - // specified then this case should be caught by the config deprecation subsystem in the legacy platform. - providers: config.authProviders || config.authc.providers, - }, - }; + return { ...config, encryptionKey, secureCookies }; }) ); }