From f7df6fdc15f0df7c7bae9ea047b56d7bdf9a75c6 Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Thu, 27 Feb 2020 12:44:11 +0100 Subject: [PATCH] Review#1: rework the way we detect which additional schemes should be supported by the HTTP authentication provider. --- .../authentication/authenticator.test.ts | 267 +++++++++++------- .../server/authentication/authenticator.ts | 39 ++- .../authentication/providers/base.mock.ts | 1 - .../server/authentication/providers/base.ts | 8 +- .../authentication/providers/basic.test.ts | 4 + .../server/authentication/providers/basic.ts | 8 + .../authentication/providers/http.test.ts | 161 +++-------- .../server/authentication/providers/http.ts | 51 +--- .../authentication/providers/kerberos.test.ts | 4 + .../authentication/providers/kerberos.ts | 8 + .../authentication/providers/oidc.test.ts | 4 + .../server/authentication/providers/oidc.ts | 8 + .../authentication/providers/pki.test.ts | 4 + .../server/authentication/providers/pki.ts | 8 + .../authentication/providers/saml.test.ts | 4 + .../server/authentication/providers/saml.ts | 8 + .../authentication/providers/token.test.ts | 4 + .../server/authentication/providers/token.ts | 8 + x-pack/plugins/security/server/config.test.ts | 50 ---- x-pack/plugins/security/server/config.ts | 46 ++- 20 files changed, 346 insertions(+), 349 deletions(-) diff --git a/x-pack/plugins/security/server/authentication/authenticator.test.ts b/x-pack/plugins/security/server/authentication/authenticator.test.ts index 16803ef8503da..af019ff10dedc 100644 --- a/x-pack/plugins/security/server/authentication/authenticator.test.ts +++ b/x-pack/plugins/security/server/authentication/authenticator.test.ts @@ -6,6 +6,7 @@ jest.mock('./providers/basic'); jest.mock('./providers/saml'); +jest.mock('./providers/http'); import Boom from 'boom'; import { duration, Duration } from 'moment'; @@ -27,9 +28,11 @@ import { BasicAuthenticationProvider } from './providers'; function getMockOptions({ session, providers, + http = {}, }: { session?: AuthenticatorOptions['config']['session']; providers?: string[]; + http?: Partial; } = {}) { return { clusterClient: elasticsearchServiceMock.createClusterClient(), @@ -41,7 +44,7 @@ function getMockOptions({ providers: providers || [], oidc: {}, saml: {}, - http: { enabled: true, autoSchemesEnabled: true, schemes: [] }, + http: { enabled: true, autoSchemesEnabled: true, schemes: ['apikey'], ...http }, }, }, sessionStorageFactory: sessionStorageMock.createFactory(), @@ -55,8 +58,13 @@ describe('Authenticator', () => { login: jest.fn(), authenticate: jest.fn(), logout: jest.fn(), + getHTTPAuthenticationScheme: jest.fn(), }; + jest.requireMock('./providers/http').HTTPAuthenticationProvider.mockImplementation(() => ({ + authenticate: jest.fn().mockResolvedValue(AuthenticationResult.notHandled()), + })); + jest .requireMock('./providers/basic') .BasicAuthenticationProvider.mockImplementation(() => mockBasicAuthenticationProvider); @@ -76,6 +84,70 @@ describe('Authenticator', () => { 'Unsupported authentication provider name: super-basic.' ); }); + + describe('HTTP authentication provider', () => { + beforeEach(() => { + jest + .requireMock('./providers/basic') + .BasicAuthenticationProvider.mockImplementation(() => ({ + getHTTPAuthenticationScheme: jest.fn().mockReturnValue('basic'), + })); + }); + + afterEach(() => jest.resetAllMocks()); + + it('enabled by default', () => { + const authenticator = new Authenticator(getMockOptions({ providers: ['basic'] })); + expect(authenticator.isProviderEnabled('basic')).toBe(true); + expect(authenticator.isProviderEnabled('http')).toBe(true); + + expect( + jest.requireMock('./providers/http').HTTPAuthenticationProvider + ).toHaveBeenCalledWith(expect.anything(), { + supportedSchemes: new Set(['apikey', 'basic']), + }); + }); + + it('includes all required schemes if `autoSchemesEnabled` is enabled', () => { + const authenticator = new Authenticator( + getMockOptions({ providers: ['basic', 'kerberos'] }) + ); + expect(authenticator.isProviderEnabled('basic')).toBe(true); + expect(authenticator.isProviderEnabled('kerberos')).toBe(true); + expect(authenticator.isProviderEnabled('http')).toBe(true); + + expect( + jest.requireMock('./providers/http').HTTPAuthenticationProvider + ).toHaveBeenCalledWith(expect.anything(), { + supportedSchemes: new Set(['apikey', 'basic', 'bearer']), + }); + }); + + it('does not include additional schemes if `autoSchemesEnabled` is disabled', () => { + const authenticator = new Authenticator( + getMockOptions({ providers: ['basic', 'kerberos'], http: { autoSchemesEnabled: false } }) + ); + expect(authenticator.isProviderEnabled('basic')).toBe(true); + expect(authenticator.isProviderEnabled('kerberos')).toBe(true); + expect(authenticator.isProviderEnabled('http')).toBe(true); + + expect( + jest.requireMock('./providers/http').HTTPAuthenticationProvider + ).toHaveBeenCalledWith(expect.anything(), { supportedSchemes: new Set(['apikey']) }); + }); + + it('disabled if explicitly disabled', () => { + const authenticator = new Authenticator( + getMockOptions({ providers: ['basic'], http: { enabled: false } }) + ); + expect(authenticator.isProviderEnabled('basic')).toBe(true); + expect(authenticator.isProviderEnabled('http')).toBe(false); + + expect( + jest.requireMock('./providers/http').HTTPAuthenticationProvider + ).not.toHaveBeenCalled(); + }); + }); }); describe('`login` method', () => { @@ -126,12 +198,9 @@ describe('Authenticator', () => { AuthenticationResult.failed(failureReason) ); - const authenticationResult = await authenticator.login(request, { - provider: 'basic', - value: {}, - }); - expect(authenticationResult.failed()).toBe(true); - expect(authenticationResult.error).toBe(failureReason); + await expect(authenticator.login(request, { provider: 'basic', value: {} })).resolves.toEqual( + AuthenticationResult.failed(failureReason) + ); }); it('returns user that authentication provider returns.', async () => { @@ -142,13 +211,9 @@ describe('Authenticator', () => { AuthenticationResult.succeeded(user, { authHeaders: { authorization: 'Basic .....' } }) ); - const authenticationResult = await authenticator.login(request, { - provider: 'basic', - value: {}, - }); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); - expect(authenticationResult.authHeaders).toEqual({ authorization: 'Basic .....' }); + await expect(authenticator.login(request, { provider: 'basic', value: {} })).resolves.toEqual( + AuthenticationResult.succeeded(user, { authHeaders: { authorization: 'Basic .....' } }) + ); }); it('creates session whenever authentication provider returns state', async () => { @@ -160,12 +225,9 @@ describe('Authenticator', () => { AuthenticationResult.succeeded(user, { state: { authorization } }) ); - const authenticationResult = await authenticator.login(request, { - provider: 'basic', - value: {}, - }); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.login(request, { provider: 'basic', value: {} })).resolves.toEqual( + AuthenticationResult.succeeded(user, { state: { authorization } }) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith({ @@ -176,11 +238,9 @@ describe('Authenticator', () => { it('returns `notHandled` if login attempt is targeted to not configured provider.', async () => { const request = httpServerMock.createKibanaRequest(); - const authenticationResult = await authenticator.login(request, { - provider: 'token', - value: {}, - }); - expect(authenticationResult.notHandled()).toBe(true); + await expect(authenticator.login(request, { provider: 'token', value: {} })).resolves.toEqual( + AuthenticationResult.notHandled() + ); }); it('clears session if it belongs to a different provider.', async () => { @@ -191,12 +251,9 @@ describe('Authenticator', () => { mockBasicAuthenticationProvider.login.mockResolvedValue(AuthenticationResult.succeeded(user)); mockSessionStorage.get.mockResolvedValue({ ...mockSessVal, provider: 'token' }); - const authenticationResult = await authenticator.login(request, { - provider: 'basic', - value: credentials, - }); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toBe(user); + await expect( + authenticator.login(request, { provider: 'basic', value: credentials }) + ).resolves.toEqual(AuthenticationResult.succeeded(user)); expect(mockBasicAuthenticationProvider.login).toHaveBeenCalledWith( request, @@ -216,12 +273,9 @@ describe('Authenticator', () => { AuthenticationResult.succeeded(user, { state: null }) ); - const authenticationResult = await authenticator.login(request, { - provider: 'basic', - value: {}, - }); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.login(request, { provider: 'basic', value: {} })).resolves.toEqual( + AuthenticationResult.succeeded(user, { state: null }) + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).toHaveBeenCalled(); @@ -277,10 +331,9 @@ describe('Authenticator', () => { AuthenticationResult.succeeded(user, { authHeaders: { authorization: 'Basic .....' } }) ); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); - expect(authenticationResult.authHeaders).toEqual({ authorization: 'Basic .....' }); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user, { authHeaders: { authorization: 'Basic .....' } }) + ); }); it('creates session whenever authentication provider returns state for system API requests', async () => { @@ -294,9 +347,9 @@ describe('Authenticator', () => { AuthenticationResult.succeeded(user, { state: { authorization } }) ); - const systemAPIAuthenticationResult = await authenticator.authenticate(request); - expect(systemAPIAuthenticationResult.succeeded()).toBe(true); - expect(systemAPIAuthenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user, { state: { authorization } }) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith({ @@ -316,9 +369,9 @@ describe('Authenticator', () => { AuthenticationResult.succeeded(user, { state: { authorization } }) ); - const systemAPIAuthenticationResult = await authenticator.authenticate(request); - expect(systemAPIAuthenticationResult.succeeded()).toBe(true); - expect(systemAPIAuthenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user, { state: { authorization } }) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith({ @@ -338,9 +391,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user) + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).not.toHaveBeenCalled(); @@ -357,9 +410,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith(mockSessVal); @@ -392,9 +445,9 @@ describe('Authenticator', () => { jest.spyOn(Date, 'now').mockImplementation(() => currentDate); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith({ @@ -437,9 +490,9 @@ describe('Authenticator', () => { jest.spyOn(Date, 'now').mockImplementation(() => currentDate); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith({ @@ -485,9 +538,9 @@ describe('Authenticator', () => { AuthenticationResult.succeeded(user) ); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith({ @@ -517,13 +570,15 @@ describe('Authenticator', () => { headers: { 'kbn-system-request': 'true' }, }); + const failureReason = new Error('some error'); mockBasicAuthenticationProvider.authenticate.mockResolvedValue( - AuthenticationResult.failed(new Error('some error')) + AuthenticationResult.failed(failureReason) ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.failed()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.failed(failureReason) + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).not.toHaveBeenCalled(); @@ -534,13 +589,15 @@ describe('Authenticator', () => { headers: { 'kbn-system-request': 'false' }, }); + const failureReason = new Error('some error'); mockBasicAuthenticationProvider.authenticate.mockResolvedValue( - AuthenticationResult.failed(new Error('some error')) + AuthenticationResult.failed(failureReason) ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.failed()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.failed(failureReason) + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).not.toHaveBeenCalled(); @@ -558,9 +615,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user, { state: newState }) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith({ @@ -582,9 +639,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual(user); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.succeeded(user, { state: newState }) + ); expect(mockSessionStorage.set).toHaveBeenCalledTimes(1); expect(mockSessionStorage.set).toHaveBeenCalledWith({ @@ -604,8 +661,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.failed()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.failed(Boom.unauthorized()) + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).toHaveBeenCalled(); @@ -621,8 +679,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.failed()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.failed(Boom.unauthorized()) + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).toHaveBeenCalled(); @@ -636,8 +695,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.redirected()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.redirectTo('some-url', { state: null }) + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).toHaveBeenCalled(); @@ -653,8 +713,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.notHandled()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.notHandled() + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).not.toHaveBeenCalled(); @@ -670,8 +731,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.notHandled()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.notHandled() + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).not.toHaveBeenCalled(); @@ -687,8 +749,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue({ ...mockSessVal, provider: 'token' }); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.notHandled()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.notHandled() + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).toHaveBeenCalled(); @@ -704,8 +767,9 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue({ ...mockSessVal, provider: 'token' }); - const authenticationResult = await authenticator.authenticate(request); - expect(authenticationResult.notHandled()).toBe(true); + await expect(authenticator.authenticate(request)).resolves.toEqual( + AuthenticationResult.notHandled() + ); expect(mockSessionStorage.set).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).toHaveBeenCalled(); @@ -742,9 +806,10 @@ describe('Authenticator', () => { const request = httpServerMock.createKibanaRequest(); mockSessionStorage.get.mockResolvedValue(null); - const deauthenticationResult = await authenticator.logout(request); + await expect(authenticator.logout(request)).resolves.toEqual( + DeauthenticationResult.notHandled() + ); - expect(deauthenticationResult.notHandled()).toBe(true); expect(mockSessionStorage.clear).not.toHaveBeenCalled(); }); @@ -755,12 +820,12 @@ describe('Authenticator', () => { ); mockSessionStorage.get.mockResolvedValue(mockSessVal); - const deauthenticationResult = await authenticator.logout(request); + await expect(authenticator.logout(request)).resolves.toEqual( + DeauthenticationResult.redirectTo('some-url') + ); expect(mockBasicAuthenticationProvider.logout).toHaveBeenCalledTimes(1); expect(mockSessionStorage.clear).toHaveBeenCalled(); - expect(deauthenticationResult.redirected()).toBe(true); - expect(deauthenticationResult.redirectURL).toBe('some-url'); }); it('if session does not exist but provider name is valid, returns whatever authentication provider returns.', async () => { @@ -771,21 +836,22 @@ describe('Authenticator', () => { DeauthenticationResult.redirectTo('some-url') ); - const deauthenticationResult = await authenticator.logout(request); + await expect(authenticator.logout(request)).resolves.toEqual( + DeauthenticationResult.redirectTo('some-url') + ); expect(mockBasicAuthenticationProvider.logout).toHaveBeenCalledTimes(1); expect(mockSessionStorage.clear).not.toHaveBeenCalled(); - expect(deauthenticationResult.redirected()).toBe(true); - expect(deauthenticationResult.redirectURL).toBe('some-url'); }); it('returns `notHandled` if session does not exist and provider name is invalid', async () => { const request = httpServerMock.createKibanaRequest({ query: { provider: 'foo' } }); mockSessionStorage.get.mockResolvedValue(null); - const deauthenticationResult = await authenticator.logout(request); + await expect(authenticator.logout(request)).resolves.toEqual( + DeauthenticationResult.notHandled() + ); - expect(deauthenticationResult.notHandled()).toBe(true); expect(mockSessionStorage.clear).not.toHaveBeenCalled(); }); @@ -794,11 +860,12 @@ describe('Authenticator', () => { const state = { authorization: 'Bearer xxx' }; mockSessionStorage.get.mockResolvedValue({ ...mockSessVal, state, provider: 'token' }); - const deauthenticationResult = await authenticator.logout(request); + await expect(authenticator.logout(request)).resolves.toEqual( + DeauthenticationResult.notHandled() + ); expect(mockBasicAuthenticationProvider.logout).not.toHaveBeenCalled(); expect(mockSessionStorage.clear).toHaveBeenCalled(); - expect(deauthenticationResult.notHandled()).toBe(true); }); }); diff --git a/x-pack/plugins/security/server/authentication/authenticator.ts b/x-pack/plugins/security/server/authentication/authenticator.ts index 5d99fe5ecbf06..4954e1b24216c 100644 --- a/x-pack/plugins/security/server/authentication/authenticator.ts +++ b/x-pack/plugins/security/server/authentication/authenticator.ts @@ -106,7 +106,6 @@ const providerMap = new Map< [TokenAuthenticationProvider.type, TokenAuthenticationProvider], [OIDCAuthenticationProvider.type, OIDCAuthenticationProvider], [PKIAuthenticationProvider.type, PKIAuthenticationProvider], - [HTTPAuthenticationProvider.type, HTTPAuthenticationProvider], ]); function assertRequest(request: KibanaRequest) { @@ -221,6 +220,17 @@ export class Authenticator { ] as [string, BaseAuthenticationProvider]; }) ); + + // For the BWC reasons we always include HTTP authentication provider unless it's explicitly disabled. + if (this.options.config.authc.http.enabled) { + this.setupHTTPAuthenticationProvider( + Object.freeze({ + ...providerCommonOptions, + logger: options.loggers.get(HTTPAuthenticationProvider.type), + }) + ); + } + this.serverBasePath = this.options.basePath.serverBasePath || '/'; this.idleTimeout = this.options.config.session.idleTimeout; @@ -398,6 +408,33 @@ export class Authenticator { return this.providers.has(providerType); } + /** + * Initializes HTTP Authentication provider and appends it to the end of the list of enabled + * authentication providers. + * @param options Common provider options. + */ + private setupHTTPAuthenticationProvider(options: AuthenticationProviderOptions) { + const supportedSchemes = new Set( + this.options.config.authc.http.schemes.map(scheme => scheme.toLowerCase()) + ); + + // If `autoSchemesEnabled` is set we should allow schemes that other providers use to + // authenticate requests with Elasticsearch. + if (this.options.config.authc.http.autoSchemesEnabled) { + for (const provider of this.providers.values()) { + const supportedScheme = provider.getHTTPAuthenticationScheme(); + if (supportedScheme) { + supportedSchemes.add(supportedScheme.toLowerCase()); + } + } + } + + this.providers.set( + HTTPAuthenticationProvider.type, + new HTTPAuthenticationProvider(options, { supportedSchemes }) + ); + } + /** * Returns provider iterator where providers are sorted in the order of priority (based on the session ownership). * @param sessionValue Current session value. diff --git a/x-pack/plugins/security/server/authentication/providers/base.mock.ts b/x-pack/plugins/security/server/authentication/providers/base.mock.ts index 5a65c95fd3269..0781608f8bc4c 100644 --- a/x-pack/plugins/security/server/authentication/providers/base.mock.ts +++ b/x-pack/plugins/security/server/authentication/providers/base.mock.ts @@ -23,6 +23,5 @@ export function mockAuthenticationProviderOptions() { logger: loggingServiceMock.create().get(), basePath, tokens: { refresh: jest.fn(), invalidate: jest.fn() }, - isProviderEnabled: jest.fn(), }; } diff --git a/x-pack/plugins/security/server/authentication/providers/base.ts b/x-pack/plugins/security/server/authentication/providers/base.ts index e98bda51178ce..300e59d9ea3da 100644 --- a/x-pack/plugins/security/server/authentication/providers/base.ts +++ b/x-pack/plugins/security/server/authentication/providers/base.ts @@ -25,7 +25,6 @@ export interface AuthenticationProviderOptions { client: IClusterClient; logger: Logger; tokens: PublicMethodsOf; - isProviderEnabled: (provider: string) => boolean; } /** @@ -85,6 +84,13 @@ export abstract class BaseAuthenticationProvider { */ abstract logout(request: KibanaRequest, state?: unknown): Promise; + /** + * Returns HTTP authentication scheme that provider uses within `Authorization` HTTP header that + * it attaches to all successfully authenticated requests to Elasticsearch or `null` in case + * provider doesn't attach any additional `Authorization` HTTP headers. + */ + abstract getHTTPAuthenticationScheme(): string | null; + /** * Queries Elasticsearch `_authenticate` endpoint to authenticate request and retrieve the user * information of authenticated user. diff --git a/x-pack/plugins/security/server/authentication/providers/basic.test.ts b/x-pack/plugins/security/server/authentication/providers/basic.test.ts index c8aaadfe6d390..b7bdff0531fc2 100644 --- a/x-pack/plugins/security/server/authentication/providers/basic.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/basic.test.ts @@ -188,4 +188,8 @@ describe('BasicAuthenticationProvider', () => { ); }); }); + + it('`getHTTPAuthenticationScheme` method', () => { + expect(provider.getHTTPAuthenticationScheme()).toBe('basic'); + }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/basic.ts b/x-pack/plugins/security/server/authentication/providers/basic.ts index 75a1439b9b9e5..ad46aff8afa51 100644 --- a/x-pack/plugins/security/server/authentication/providers/basic.ts +++ b/x-pack/plugins/security/server/authentication/providers/basic.ts @@ -110,6 +110,14 @@ export class BasicAuthenticationProvider extends BaseAuthenticationProvider { ); } + /** + * Returns HTTP authentication scheme (`Bearer`) that's used within `Authorization` HTTP header + * that provider attaches to all successfully authenticated requests to Elasticsearch. + */ + public getHTTPAuthenticationScheme() { + return 'basic'; + } + /** * Tries to extract authorization header from the state and adds it to the request before * it's forwarded to Elasticsearch backend. diff --git a/x-pack/plugins/security/server/authentication/providers/http.test.ts b/x-pack/plugins/security/server/authentication/providers/http.test.ts index 94237f9a3d538..65fbd7cd9f4ad 100644 --- a/x-pack/plugins/security/server/authentication/providers/http.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/http.test.ts @@ -38,28 +38,21 @@ describe('HTTPAuthenticationProvider', () => { it('throws if `schemes` are not specified', () => { const providerOptions = mockAuthenticationProviderOptions(); - expect(() => new HTTPAuthenticationProvider(providerOptions)).toThrowError( + expect(() => new HTTPAuthenticationProvider(providerOptions, undefined as any)).toThrowError( 'Supported schemes should be specified' ); - expect(() => new HTTPAuthenticationProvider(providerOptions, {})).toThrowError( + expect(() => new HTTPAuthenticationProvider(providerOptions, {} as any)).toThrowError( 'Supported schemes should be specified' ); - expect(() => new HTTPAuthenticationProvider(providerOptions, { schemes: [] })).toThrowError( - 'Supported schemes should be specified' - ); - expect( - () => - new HTTPAuthenticationProvider(providerOptions, { schemes: [], autoSchemesEnabled: false }) + () => new HTTPAuthenticationProvider(providerOptions, { supportedSchemes: new Set() }) ).toThrowError('Supported schemes should be specified'); }); describe('`login` method', () => { it('does not handle login', async () => { const provider = new HTTPAuthenticationProvider(mockOptions, { - enabled: true, - autoSchemesEnabled: true, - schemes: ['apikey'], + supportedSchemes: new Set(['apikey']), }); await expect(provider.login()).resolves.toEqual(AuthenticationResult.notHandled()); @@ -70,95 +63,9 @@ describe('HTTPAuthenticationProvider', () => { }); describe('`authenticate` method', () => { - const testCasesToNotHandle = [ - { - autoSchemesEnabled: false, - isProviderEnabled: () => false, - schemes: ['basic'], - header: 'Bearer xxx', - }, - { - autoSchemesEnabled: false, - isProviderEnabled: () => false, - schemes: ['bearer'], - header: 'Basic xxx', - }, - { - autoSchemesEnabled: false, - isProviderEnabled: () => false, - schemes: ['basic', 'apikey'], - header: 'Bearer xxx', - }, - { - autoSchemesEnabled: true, - isProviderEnabled: () => false, - schemes: ['basic', 'apikey'], - header: 'Bearer xxx', - }, - { - autoSchemesEnabled: true, - isProviderEnabled: (provider: string) => provider === 'basic', - schemes: ['basic'], - header: 'Bearer xxx', - }, - { - autoSchemesEnabled: true, - isProviderEnabled: () => true, - schemes: [], - header: 'ApiKey xxx', - }, - ]; - - const testCasesToHandle = [ - { - autoSchemesEnabled: false, - isProviderEnabled: () => false, - schemes: ['basic'], - header: 'Basic xxx', - }, - { - autoSchemesEnabled: false, - isProviderEnabled: () => false, - schemes: ['bearer'], - header: 'Bearer xxx', - }, - { - autoSchemesEnabled: false, - isProviderEnabled: () => false, - schemes: ['basic', 'apikey'], - header: 'ApiKey xxx', - }, - { - autoSchemesEnabled: false, - isProviderEnabled: () => false, - schemes: ['some-weird-scheme'], - header: 'some-weird-scheme xxx', - }, - ...['saml', 'oidc', 'pki', 'kerberos', 'token'].map(bearerProviderType => ({ - autoSchemesEnabled: true, - isProviderEnabled: (providerType: string) => providerType === bearerProviderType, - schemes: ['apikey'], - header: 'Bearer xxx', - })), - { - autoSchemesEnabled: true, - isProviderEnabled: (provider: string) => provider === 'basic', - schemes: ['apikey'], - header: 'Basic xxx', - }, - { - autoSchemesEnabled: true, - isProviderEnabled: () => true, - schemes: [], - header: 'Bearer xxx', - }, - ]; - it('does not handle authentication for requests without `authorization` header.', async () => { const provider = new HTTPAuthenticationProvider(mockOptions, { - enabled: true, - autoSchemesEnabled: true, - schemes: ['apikey'], + supportedSchemes: new Set(['apikey']), }); await expect(provider.authenticate(httpServerMock.createKibanaRequest())).resolves.toEqual( @@ -171,9 +78,7 @@ describe('HTTPAuthenticationProvider', () => { it('does not handle authentication for requests with empty scheme in `authorization` header.', async () => { const provider = new HTTPAuthenticationProvider(mockOptions, { - enabled: true, - autoSchemesEnabled: true, - schemes: ['apikey'], + supportedSchemes: new Set(['apikey']), }); await expect( @@ -187,19 +92,16 @@ describe('HTTPAuthenticationProvider', () => { }); it('does not handle authentication via `authorization` header if scheme is not supported.', async () => { - for (const { - isProviderEnabled, - autoSchemesEnabled, - schemes, - header, - } of testCasesToNotHandle) { + for (const { schemes, header } of [ + { schemes: ['basic'], header: 'Bearer xxx' }, + { schemes: ['bearer'], header: 'Basic xxx' }, + { schemes: ['basic', 'apikey'], header: 'Bearer xxx' }, + { schemes: ['basic', 'bearer'], header: 'ApiKey xxx' }, + ]) { const request = httpServerMock.createKibanaRequest({ headers: { authorization: header } }); - mockOptions.isProviderEnabled.mockImplementation(isProviderEnabled); const provider = new HTTPAuthenticationProvider(mockOptions, { - enabled: true, - autoSchemesEnabled, - schemes, + supportedSchemes: new Set(schemes), }); await expect(provider.authenticate(request)).resolves.toEqual( @@ -215,7 +117,13 @@ describe('HTTPAuthenticationProvider', () => { it('succeeds if authentication via `authorization` header with supported scheme succeeds.', async () => { const user = mockAuthenticatedUser(); - for (const { isProviderEnabled, autoSchemesEnabled, schemes, header } of testCasesToHandle) { + for (const { schemes, header } of [ + { schemes: ['basic'], header: 'Basic xxx' }, + { schemes: ['bearer'], header: 'Bearer xxx' }, + { schemes: ['basic', 'apikey'], header: 'ApiKey xxx' }, + { schemes: ['some-weird-scheme'], header: 'some-weird-scheme xxx' }, + { schemes: ['apikey', 'bearer'], header: 'Bearer xxx' }, + ]) { const request = httpServerMock.createKibanaRequest({ headers: { authorization: header } }); const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient(); @@ -223,11 +131,8 @@ describe('HTTPAuthenticationProvider', () => { mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); mockOptions.client.asScoped.mockClear(); - mockOptions.isProviderEnabled.mockImplementation(isProviderEnabled); const provider = new HTTPAuthenticationProvider(mockOptions, { - enabled: true, - autoSchemesEnabled, - schemes, + supportedSchemes: new Set(schemes), }); await expect(provider.authenticate(request)).resolves.toEqual( @@ -242,7 +147,13 @@ describe('HTTPAuthenticationProvider', () => { it('fails if authentication via `authorization` header with supported scheme fails.', async () => { const failureReason = ElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()); - for (const { isProviderEnabled, autoSchemesEnabled, schemes, header } of testCasesToHandle) { + for (const { schemes, header } of [ + { schemes: ['basic'], header: 'Basic xxx' }, + { schemes: ['bearer'], header: 'Bearer xxx' }, + { schemes: ['basic', 'apikey'], header: 'ApiKey xxx' }, + { schemes: ['some-weird-scheme'], header: 'some-weird-scheme xxx' }, + { schemes: ['apikey', 'bearer'], header: 'Bearer xxx' }, + ]) { const request = httpServerMock.createKibanaRequest({ headers: { authorization: header } }); const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient(); @@ -250,11 +161,8 @@ describe('HTTPAuthenticationProvider', () => { mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); mockOptions.client.asScoped.mockClear(); - mockOptions.isProviderEnabled.mockImplementation(isProviderEnabled); const provider = new HTTPAuthenticationProvider(mockOptions, { - enabled: true, - autoSchemesEnabled, - schemes, + supportedSchemes: new Set(schemes), }); await expect(provider.authenticate(request)).resolves.toEqual( @@ -271,9 +179,7 @@ describe('HTTPAuthenticationProvider', () => { describe('`logout` method', () => { it('does not handle logout', async () => { const provider = new HTTPAuthenticationProvider(mockOptions, { - enabled: true, - autoSchemesEnabled: true, - schemes: ['apikey'], + supportedSchemes: new Set(['apikey']), }); await expect(provider.logout()).resolves.toEqual(DeauthenticationResult.notHandled()); @@ -282,4 +188,11 @@ describe('HTTPAuthenticationProvider', () => { expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled(); }); }); + + it('`getHTTPAuthenticationScheme` method', () => { + const provider = new HTTPAuthenticationProvider(mockOptions, { + supportedSchemes: new Set(['apikey']), + }); + expect(provider.getHTTPAuthenticationScheme()).toBeNull(); + }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/http.ts b/x-pack/plugins/security/server/authentication/providers/http.ts index c06958ad7f350..57163bf8145b8 100644 --- a/x-pack/plugins/security/server/authentication/providers/http.ts +++ b/x-pack/plugins/security/server/authentication/providers/http.ts @@ -11,9 +11,7 @@ import { getHTTPAuthenticationScheme } from '../get_http_authentication_scheme'; import { AuthenticationProviderOptions, BaseAuthenticationProvider } from './base'; interface HTTPAuthenticationProviderOptions { - enabled: boolean; - autoSchemesEnabled: boolean; - schemes: string[]; + supportedSchemes: Set; } /** @@ -31,26 +29,16 @@ export class HTTPAuthenticationProvider extends BaseAuthenticationProvider { */ private readonly supportedSchemes: Set; - /** - * Indicates whether we should allow schemes that other providers use to authenticate with - * Elasticsearch. - */ - private readonly autoSchemesEnabled: boolean; - constructor( protected readonly options: Readonly, - httpOptions?: Readonly> + httpOptions: Readonly ) { super(options); - this.supportedSchemes = new Set( - (httpOptions?.schemes ?? []).map(scheme => scheme.toLowerCase()) - ); - this.autoSchemesEnabled = httpOptions?.autoSchemesEnabled ?? false; - - if (this.supportedSchemes.size === 0 && !this.autoSchemesEnabled) { + if ((httpOptions?.supportedSchemes?.size ?? 0) === 0) { throw new Error('Supported schemes should be specified'); } + this.supportedSchemes = httpOptions.supportedSchemes; } /** @@ -74,7 +62,7 @@ export class HTTPAuthenticationProvider extends BaseAuthenticationProvider { return AuthenticationResult.notHandled(); } - if (!this.isSchemeSupported(authenticationScheme)) { + if (!this.supportedSchemes.has(authenticationScheme)) { this.logger.debug(`Unsupported authentication scheme: ${authenticationScheme}`); return AuthenticationResult.notHandled(); } @@ -102,31 +90,10 @@ export class HTTPAuthenticationProvider extends BaseAuthenticationProvider { } /** - * Checks whether specified scheme should be supported by the provider based on the explicitly - * specified schemes in Kibana configuration or currently enabled authentication providers (if - * `xpack.security.authc.http.autoSchemesEnabled` is set to `true`). - * @param scheme + * Returns `null` since provider doesn't attach any additional `Authorization` HTTP headers to + * successfully authenticated requests to Elasticsearch. */ - private isSchemeSupported(scheme: string) { - const isSchemeSupported = this.supportedSchemes.has(scheme); - if (isSchemeSupported || !this.autoSchemesEnabled) { - return isSchemeSupported; - } - - if (scheme === 'basic') { - return this.options.isProviderEnabled('basic'); - } - - if (scheme === 'bearer') { - return ( - this.options.isProviderEnabled('saml') || - this.options.isProviderEnabled('oidc') || - this.options.isProviderEnabled('pki') || - this.options.isProviderEnabled('kerberos') || - this.options.isProviderEnabled('token') - ); - } - - return false; + public getHTTPAuthenticationScheme() { + return null; } } diff --git a/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts b/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts index 2f0431c98a295..51fb961482e83 100644 --- a/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts @@ -501,4 +501,8 @@ describe('KerberosAuthenticationProvider', () => { expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith(tokenPair); }); }); + + it('`getHTTPAuthenticationScheme` method', () => { + expect(provider.getHTTPAuthenticationScheme()).toBe('bearer'); + }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/kerberos.ts b/x-pack/plugins/security/server/authentication/providers/kerberos.ts index 9ae35cfe75901..b6474a5e1d471 100644 --- a/x-pack/plugins/security/server/authentication/providers/kerberos.ts +++ b/x-pack/plugins/security/server/authentication/providers/kerberos.ts @@ -94,6 +94,14 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { return DeauthenticationResult.redirectTo(`${this.options.basePath.serverBasePath}/logged_out`); } + /** + * Returns HTTP authentication scheme (`Bearer`) that's used within `Authorization` HTTP header + * that provider attaches to all successfully authenticated requests to Elasticsearch. + */ + public getHTTPAuthenticationScheme() { + return 'bearer'; + } + /** * Tries to authenticate request with `Negotiate ***` Authorization header by passing it to the Elasticsearch backend to * get an access token in exchange. diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts index c2fc4b8a10aff..51a25825bf985 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts @@ -603,4 +603,8 @@ describe('OIDCAuthenticationProvider', () => { }); }); }); + + it('`getHTTPAuthenticationScheme` method', () => { + expect(provider.getHTTPAuthenticationScheme()).toBe('bearer'); + }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.ts b/x-pack/plugins/security/server/authentication/providers/oidc.ts index d6f4bf37001d3..c6b504e722adf 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.ts @@ -402,4 +402,12 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { return DeauthenticationResult.failed(err); } } + + /** + * Returns HTTP authentication scheme (`Bearer`) that's used within `Authorization` HTTP header + * that provider attaches to all successfully authenticated requests to Elasticsearch. + */ + public getHTTPAuthenticationScheme() { + return 'bearer'; + } } diff --git a/x-pack/plugins/security/server/authentication/providers/pki.test.ts b/x-pack/plugins/security/server/authentication/providers/pki.test.ts index 3b5fa1bfa4d39..efc286c6c895f 100644 --- a/x-pack/plugins/security/server/authentication/providers/pki.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/pki.test.ts @@ -518,4 +518,8 @@ describe('PKIAuthenticationProvider', () => { expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({ accessToken: 'foo' }); }); }); + + it('`getHTTPAuthenticationScheme` method', () => { + expect(provider.getHTTPAuthenticationScheme()).toBe('bearer'); + }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/pki.ts b/x-pack/plugins/security/server/authentication/providers/pki.ts index 0fec62317c802..854f92a50fa9d 100644 --- a/x-pack/plugins/security/server/authentication/providers/pki.ts +++ b/x-pack/plugins/security/server/authentication/providers/pki.ts @@ -101,6 +101,14 @@ export class PKIAuthenticationProvider extends BaseAuthenticationProvider { return DeauthenticationResult.redirectTo(`${this.options.basePath.serverBasePath}/logged_out`); } + /** + * Returns HTTP authentication scheme (`Bearer`) that's used within `Authorization` HTTP header + * that provider attaches to all successfully authenticated requests to Elasticsearch. + */ + public getHTTPAuthenticationScheme() { + return 'bearer'; + } + /** * Tries to extract access token from state and adds it to the request before it's * forwarded to Elasticsearch backend. diff --git a/x-pack/plugins/security/server/authentication/providers/saml.test.ts b/x-pack/plugins/security/server/authentication/providers/saml.test.ts index cbdcfa0f0b025..d97a6c0838b86 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.test.ts @@ -1099,4 +1099,8 @@ describe('SAMLAuthenticationProvider', () => { expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); }); }); + + it('`getHTTPAuthenticationScheme` method', () => { + expect(provider.getHTTPAuthenticationScheme()).toBe('bearer'); + }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts index a7dbb14903479..1ac59d66a2235 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.ts @@ -239,6 +239,14 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { } } + /** + * Returns HTTP authentication scheme (`Bearer`) that's used within `Authorization` HTTP header + * that provider attaches to all successfully authenticated requests to Elasticsearch. + */ + public getHTTPAuthenticationScheme() { + return 'bearer'; + } + /** * Validates whether request payload contains `SAMLResponse` parameter that can be exchanged * to a proper access token. If state is presented and includes request id then it means diff --git a/x-pack/plugins/security/server/authentication/providers/token.test.ts b/x-pack/plugins/security/server/authentication/providers/token.test.ts index b1efe0a0475b4..e81d14e8bf9f3 100644 --- a/x-pack/plugins/security/server/authentication/providers/token.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/token.test.ts @@ -443,4 +443,8 @@ describe('TokenAuthenticationProvider', () => { expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith(tokenPair); }); }); + + it('`getHTTPAuthenticationScheme` method', () => { + expect(provider.getHTTPAuthenticationScheme()).toBe('bearer'); + }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/token.ts b/x-pack/plugins/security/server/authentication/providers/token.ts index 35d0aa9cc1816..fffac254ed30a 100644 --- a/x-pack/plugins/security/server/authentication/providers/token.ts +++ b/x-pack/plugins/security/server/authentication/providers/token.ts @@ -134,6 +134,14 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { ); } + /** + * Returns HTTP authentication scheme (`Bearer`) that's used within `Authorization` HTTP header + * that provider attaches to all successfully authenticated requests to Elasticsearch. + */ + public getHTTPAuthenticationScheme() { + return 'bearer'; + } + /** * Tries to extract authorization header from the state and adds it to the request before * it's forwarded to Elasticsearch backend. diff --git a/x-pack/plugins/security/server/config.test.ts b/x-pack/plugins/security/server/config.test.ts index 19110cc50da9b..64c695670fa19 100644 --- a/x-pack/plugins/security/server/config.test.ts +++ b/x-pack/plugins/security/server/config.test.ts @@ -101,20 +101,6 @@ describe('config schema', () => { ); }); - it('should throw error if `http` provider is explicitly set in xpack.security.authc.providers', () => { - expect(() => - ConfigSchema.validate({ authc: { providers: ['http'] } }) - ).toThrowErrorMatchingInlineSnapshot( - `"[authc]: \`http\` authentication provider cannot be specified in \`xpack.security.authc.providers\`. Use \`xpack.security.authc.http.enabled\` instead."` - ); - - expect(() => - ConfigSchema.validate({ authc: { providers: ['basic', 'http'] } }) - ).toThrowErrorMatchingInlineSnapshot( - `"[authc]: \`http\` authentication provider cannot be specified in \`xpack.security.authc.providers\`. Use \`xpack.security.authc.http.enabled\` instead."` - ); - }); - describe('authc.oidc', () => { it(`returns a validation error when authc.providers is "['oidc']" and realm is unspecified`, async () => { expect(() => @@ -368,40 +354,4 @@ describe('createConfig$()', () => { expect(loggingServiceMock.collect(contextMock.logger).warn).toEqual([]); }); - - it('should include `http` authentication provider by default', async () => { - let config = (await mockAndCreateConfig(true, {})).config; - expect(config.authc.providers).toEqual(['basic', 'http']); - - config = ( - await mockAndCreateConfig(true, { authc: { providers: ['saml'], saml: { realm: 'saml1' } } }) - ).config; - expect(config.authc.providers).toEqual(['saml', 'http']); - - config = ( - await mockAndCreateConfig(true, { - authc: { providers: ['saml', 'basic'], saml: { realm: 'saml1' } }, - }) - ).config; - expect(config.authc.providers).toEqual(['saml', 'basic', 'http']); - }); - - it('should not include `http` authentication provider if it is disabled', async () => { - let config = (await mockAndCreateConfig(true, { authc: { http: { enabled: false } } })).config; - expect(config.authc.providers).toEqual(['basic']); - - config = ( - await mockAndCreateConfig(true, { - authc: { providers: ['saml'], saml: { realm: 'saml1' }, http: { enabled: false } }, - }) - ).config; - expect(config.authc.providers).toEqual(['saml']); - - config = ( - await mockAndCreateConfig(true, { - authc: { providers: ['saml', 'basic'], saml: { realm: 'saml1' }, http: { enabled: false } }, - }) - ).config; - expect(config.authc.providers).toEqual(['saml', 'basic']); - }); }); diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts index f5e0914245097..8663a6e61c203 100644 --- a/x-pack/plugins/security/server/config.ts +++ b/x-pack/plugins/security/server/config.ts @@ -39,31 +39,22 @@ export const ConfigSchema = schema.object( lifespan: schema.nullable(schema.duration()), }), secureCookies: schema.boolean({ defaultValue: false }), - authc: schema.object( - { - providers: schema.arrayOf(schema.string(), { defaultValue: ['basic'], minSize: 1 }), - oidc: providerOptionsSchema('oidc', schema.object({ realm: schema.string() })), - saml: providerOptionsSchema( - 'saml', - schema.object({ - realm: schema.string(), - maxRedirectURLSize: schema.byteSize({ defaultValue: '2kb' }), - }) - ), - http: schema.object({ - enabled: schema.boolean({ defaultValue: true }), - autoSchemesEnabled: schema.boolean({ defaultValue: true }), - schemes: schema.arrayOf(schema.string(), { defaultValue: ['apikey'] }), - }), - }, - { - validate(value) { - if (value.providers.includes('http')) { - return '`http` authentication provider cannot be specified in `xpack.security.authc.providers`. Use `xpack.security.authc.http.enabled` instead.'; - } - }, - } - ), + authc: schema.object({ + providers: schema.arrayOf(schema.string(), { defaultValue: ['basic'], minSize: 1 }), + oidc: providerOptionsSchema('oidc', schema.object({ realm: schema.string() })), + saml: providerOptionsSchema( + 'saml', + schema.object({ + realm: schema.string(), + maxRedirectURLSize: schema.byteSize({ defaultValue: '2kb' }), + }) + ), + http: schema.object({ + enabled: schema.boolean({ defaultValue: true }), + autoSchemesEnabled: schema.boolean({ defaultValue: true }), + schemes: schema.arrayOf(schema.string(), { defaultValue: ['apikey'] }), + }), + }), }, // This option should be removed as soon as we entirely migrate config from legacy Security plugin. { allowUnknowns: true } @@ -100,11 +91,6 @@ export function createConfig$(context: PluginInitializerContext, isTLSEnabled: b secureCookies = true; } - // For the BWC reasons we always include HTTP authentication provider unless it's explicitly disabled. - if (config.authc.http.enabled) { - config.authc.providers.push('http'); - } - return { ...config, encryptionKey,