Skip to content

Commit

Permalink
[7.x] Properly apply rename deprecation to `xpack.security.authProv…
Browse files Browse the repository at this point in the history 
…iders`. (#45755)
  • Loading branch information
@azasypkin
azasypkin committed Sep 16, 2019
1 parent ec4de99 commit aba8200
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 40 deletions.
120 changes: 94 additions & 26 deletions x-pack/plugins/security/server/config.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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"`);
});
});

Expand All @@ -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 {
Expand All @@ -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' } },
Expand All @@ -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"`);
});
});
});
Expand Down Expand Up @@ -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,
}
`);
});
});
32 changes: 18 additions & 14 deletions x-pack/plugins/security/server/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() }))),
Expand All @@ -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<TypeOf<typeof ConfigSchema>>().pipe(
map(config => {
Expand Down Expand Up @@ -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 };
})
);
}

0 comments on commit aba8200

Please sign in to comment.