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.

Sign up for GitHub

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

Jump to bottom

[7.x] Properly apply rename deprecation to xpack.security.authProviders. #45755

Merged
merged 1 commit into from
Sep 16, 2019
Merged

[7.x] Properly apply rename deprecation to xpack.security.authProviders. #45755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
120 changes: 94 additions & 26 deletions x-pack/plugins/security/server/config.test.ts
View file
Open in desktop
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
Open in desktop
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;
Copy link
Member Author

Choose a reason for hiding this comment

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

note: super hacky, but feels like the safest and correct for the time being.

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 };
})
);
}