diff --git a/docs/user/security/authentication/index.asciidoc b/docs/user/security/authentication/index.asciidoc index 1b326c95e20b5..8a4678e051490 100644 --- a/docs/user/security/authentication/index.asciidoc +++ b/docs/user/security/authentication/index.asciidoc @@ -100,7 +100,7 @@ xpack.security.authc.saml.realm: realm-name + [source,yaml] -------------------------------------------------------------------------------- -server.xsrf.whitelist: [/api/security/v1/saml] +server.xsrf.whitelist: [/api/security/saml/callback] -------------------------------------------------------------------------------- Users will be able to log in to {kib} via SAML Single Sign-On by navigating directly to the {kib} URL. Users who aren't authenticated are redirected to the Identity Provider for login. Most Identity Providers maintain a long-lived session—users who logged in to a different application using the same Identity Provider in the same browser are automatically authenticated. An exception is if {es} or the Identity Provider is configured to force user to re-authenticate. This login scenario is called _Service Provider initiated login_. @@ -119,6 +119,21 @@ The order of `saml` and `basic` is important. Users who open {kib} will go throu Basic authentication is supported _only_ if `basic` authentication provider is explicitly declared in `xpack.security.authc.providers` setting in addition to `saml`. +[float] +===== SAML and long URLs + +At the beginning of the SAML handshake, {kib} stores the initial URL in the session cookie, so it can redirect the user back to that URL after successful SAML authentication. +If the URL is long, the session cookie might exceed the maximum size supported by the browser--typically 4KB for all cookies per domain. When this happens, the session cookie is truncated, +or dropped completely, and you might experience sporadic failures during SAML authentication. + +To remedy this issue, you can decrease the maximum +size of the URL that {kib} is allowed to store during the SAML handshake. The default value is 2KB. + +[source,yaml] +-------------------------------------------------------------------------------- +xpack.security.authc.saml.maxRedirectURLSize: 1kb +-------------------------------------------------------------------------------- + [[oidc]] ==== OpenID Connect Single Sign-On diff --git a/renovate.json5 b/renovate.json5 index 221d5b232fd21..22682fa3d1361 100644 --- a/renovate.json5 +++ b/renovate.json5 @@ -807,6 +807,22 @@ '@types/tinycolor2', ], }, + { + groupSlug: 'xml2js', + groupName: 'xml2js related packages', + packageNames: [ + 'xml2js', + '@types/xml2js', + ], + }, + { + groupSlug: 'xml-crypto', + groupName: 'xml-crypto related packages', + packageNames: [ + 'xml-crypto', + '@types/xml-crypto', + ], + }, { groupSlug: 'intl-relativeformat', groupName: 'intl-relativeformat related packages', @@ -919,14 +935,6 @@ '@types/parse-link-header', ], }, - { - groupSlug: 'xml2js', - groupName: 'xml2js related packages', - packageNames: [ - 'xml2js', - '@types/xml2js', - ], - }, { packagePatterns: [ '^@kbn/.*', diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker index e718a1a9d2209..60f94f7f38056 100755 --- a/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker +++ b/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker @@ -174,6 +174,7 @@ kibana_vars=( xpack.security.authc.providers xpack.security.authc.oidc.realm xpack.security.authc.saml.realm + xpack.security.authc.saml.maxRedirectURLSize xpack.security.cookieName xpack.security.enabled xpack.security.encryptionKey diff --git a/x-pack/legacy/plugins/security/index.js b/x-pack/legacy/plugins/security/index.js index 0626629b7c417..980af19cc8362 100644 --- a/x-pack/legacy/plugins/security/index.js +++ b/x-pack/legacy/plugins/security/index.js @@ -31,6 +31,7 @@ import { SecureSavedObjectsClientWrapper } from './server/lib/saved_objects_clie import { deepFreeze } from './server/lib/deep_freeze'; import { createOptionalPlugin } from '../../server/lib/optional_plugin'; import { KibanaRequest } from '../../../../src/core/server'; +import { createCSPRuleString } from '../../../../src/legacy/server/csp'; export const security = (kibana) => new kibana.Plugin({ id: 'security', @@ -128,6 +129,7 @@ export const security = (kibana) => new kibana.Plugin({ throw new Error('New Platform XPack Security plugin is not available.'); } + const config = server.config(); const xpackMainPlugin = server.plugins.xpack_main; const xpackInfo = xpackMainPlugin.info; securityPlugin.registerLegacyAPI({ @@ -135,10 +137,10 @@ export const security = (kibana) => new kibana.Plugin({ isSystemAPIRequest: server.plugins.kibana.systemApi.isSystemApiRequest.bind( server.plugins.kibana.systemApi ), + cspRules: createCSPRuleString(config.get('csp.rules')), }); const plugin = this; - const config = server.config(); const xpackInfoFeature = xpackInfo.feature(plugin.id); // Register a function that is called whenever the xpack info changes, diff --git a/x-pack/legacy/plugins/security/server/routes/api/v1/__tests__/authenticate.js b/x-pack/legacy/plugins/security/server/routes/api/v1/__tests__/authenticate.js index 96b47d1407bf1..5e07ec7ee0618 100644 --- a/x-pack/legacy/plugins/security/server/routes/api/v1/__tests__/authenticate.js +++ b/x-pack/legacy/plugins/security/server/routes/api/v1/__tests__/authenticate.js @@ -257,90 +257,4 @@ describe('Authentication routes', () => { expect(response).to.eql({ username: 'user' }); }); }); - - describe('SAML assertion consumer service endpoint', () => { - let samlAcsRoute; - let request; - - beforeEach(() => { - samlAcsRoute = serverStub.route - .withArgs(sinon.match({ path: '/api/security/v1/saml' })) - .firstCall - .args[0]; - - request = requestFixture({ payload: { SAMLResponse: 'saml-response-xml' } }); - }); - - it('correctly defines route.', async () => { - expect(samlAcsRoute.method).to.be('POST'); - expect(samlAcsRoute.path).to.be('/api/security/v1/saml'); - expect(samlAcsRoute.handler).to.be.a(Function); - expect(samlAcsRoute.config).to.eql({ - auth: false, - validate: { - payload: Joi.object({ - SAMLResponse: Joi.string().required(), - RelayState: Joi.string().allow('') - }) - } - }); - }); - - it('returns 500 if authentication throws unhandled exception.', async () => { - const unhandledException = new Error('Something went wrong.'); - loginStub.throws(unhandledException); - - const response = await samlAcsRoute.handler(request, hStub); - - sinon.assert.notCalled(hStub.redirect); - expect(response.isBoom).to.be(true); - expect(response.output.payload).to.eql({ - statusCode: 500, - error: 'Internal Server Error', - message: 'An internal server error occurred' - }); - }); - - it('returns 401 if authentication fails.', async () => { - const failureReason = new Error('Something went wrong.'); - loginStub.resolves(AuthenticationResult.failed(failureReason)); - - const response = await samlAcsRoute.handler(request, hStub); - - sinon.assert.notCalled(hStub.redirect); - expect(response.isBoom).to.be(true); - expect(response.message).to.be(failureReason.message); - expect(response.output.statusCode).to.be(401); - }); - - it('returns 401 if authentication is not handled.', async () => { - loginStub.resolves(AuthenticationResult.notHandled()); - - const response = await samlAcsRoute.handler(request, hStub); - - sinon.assert.notCalled(hStub.redirect); - expect(response.isBoom).to.be(true); - expect(response.message).to.be('Unauthorized'); - expect(response.output.statusCode).to.be(401); - }); - - it('returns 401 if authentication completes with unexpected result.', async () => { - loginStub.resolves(AuthenticationResult.succeeded({})); - - const response = await samlAcsRoute.handler(request, hStub); - - sinon.assert.notCalled(hStub.redirect); - expect(response.isBoom).to.be(true); - expect(response.message).to.be('Unauthorized'); - expect(response.output.statusCode).to.be(401); - }); - - it('redirects if required by the authentication process.', async () => { - loginStub.resolves(AuthenticationResult.redirectTo('http://redirect-to/path')); - - await samlAcsRoute.handler(request, hStub); - - sinon.assert.calledWithExactly(hStub.redirect, 'http://redirect-to/path'); - }); - }); }); diff --git a/x-pack/legacy/plugins/security/server/routes/api/v1/authenticate.js b/x-pack/legacy/plugins/security/server/routes/api/v1/authenticate.js index ccd4e1c3a82c6..d22cf0aef4db7 100644 --- a/x-pack/legacy/plugins/security/server/routes/api/v1/authenticate.js +++ b/x-pack/legacy/plugins/security/server/routes/api/v1/authenticate.js @@ -58,37 +58,6 @@ export function initAuthenticateApi({ authc: { login, logout }, config }, server } }); - server.route({ - method: 'POST', - path: '/api/security/v1/saml', - config: { - auth: false, - validate: { - payload: Joi.object({ - SAMLResponse: Joi.string().required(), - RelayState: Joi.string().allow('') - }) - } - }, - async handler(request, h) { - try { - // When authenticating using SAML we _expect_ to redirect to the SAML Identity provider. - const authenticationResult = await login(KibanaRequest.from(request), { - provider: 'saml', - value: { samlResponse: request.payload.SAMLResponse } - }); - - if (authenticationResult.redirected()) { - return h.redirect(authenticationResult.redirectURL); - } - - return Boom.unauthorized(authenticationResult.error); - } catch (err) { - return wrapError(err); - } - } - }); - /** * The route should be configured as a redirect URI in OP when OpenID Connect implicit flow * is used, so that we can extract authentication response from URL fragment and send it to diff --git a/x-pack/package.json b/x-pack/package.json index 33916df4d0698..acf3bc4e3553c 100644 --- a/x-pack/package.json +++ b/x-pack/package.json @@ -108,6 +108,8 @@ "@types/tar-fs": "^1.16.1", "@types/tinycolor2": "^1.4.1", "@types/uuid": "^3.4.4", + "@types/xml2js": "^0.4.5", + "@types/xml-crypto": "^1.4.0", "abab": "^1.0.4", "ansicolors": "0.3.2", "axios": "^0.19.0", diff --git a/x-pack/plugins/security/server/authentication/index.mock.ts b/x-pack/plugins/security/server/authentication/index.mock.ts new file mode 100644 index 0000000000000..dcaf26f53fe01 --- /dev/null +++ b/x-pack/plugins/security/server/authentication/index.mock.ts @@ -0,0 +1,18 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { Authentication } from '.'; + +export const authenticationMock = { + create: (): jest.Mocked => ({ + login: jest.fn(), + createAPIKey: jest.fn(), + getCurrentUser: jest.fn(), + invalidateAPIKey: jest.fn(), + isAuthenticated: jest.fn(), + logout: jest.fn(), + }), +}; diff --git a/x-pack/plugins/security/server/authentication/index.ts b/x-pack/plugins/security/server/authentication/index.ts index 59d67a18c890c..9553ddd09b2c1 100644 --- a/x-pack/plugins/security/server/authentication/index.ts +++ b/x-pack/plugins/security/server/authentication/index.ts @@ -3,6 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ +import { UnwrapPromise } from '@kbn/utility-types'; import { IClusterClient, CoreSetup, @@ -20,8 +21,13 @@ export { canRedirectRequest } from './can_redirect_request'; export { Authenticator, ProviderLoginAttempt } from './authenticator'; export { AuthenticationResult } from './authentication_result'; export { DeauthenticationResult } from './deauthentication_result'; -export { OIDCAuthenticationFlow } from './providers'; -export { CreateAPIKeyResult } from './api_keys'; +export { OIDCAuthenticationFlow, SAMLLoginStep } from './providers'; +export { + CreateAPIKeyResult, + InvalidateAPIKeyResult, + CreateAPIKeyParams, + InvalidateAPIKeyParams, +} from './api_keys'; interface SetupAuthenticationParams { core: CoreSetup; @@ -31,6 +37,8 @@ interface SetupAuthenticationParams { getLegacyAPI(): LegacyAPI; } +export type Authentication = UnwrapPromise>; + export async function setupAuthentication({ core, clusterClient, diff --git a/x-pack/plugins/security/server/authentication/providers/index.ts b/x-pack/plugins/security/server/authentication/providers/index.ts index af0b90766e859..1ec6dfb67a81d 100644 --- a/x-pack/plugins/security/server/authentication/providers/index.ts +++ b/x-pack/plugins/security/server/authentication/providers/index.ts @@ -11,7 +11,7 @@ export { } from './base'; export { BasicAuthenticationProvider } from './basic'; export { KerberosAuthenticationProvider } from './kerberos'; -export { SAMLAuthenticationProvider, isSAMLRequestQuery } from './saml'; +export { SAMLAuthenticationProvider, isSAMLRequestQuery, SAMLLoginStep } from './saml'; export { TokenAuthenticationProvider } from './token'; export { OIDCAuthenticationProvider, OIDCAuthenticationFlow } from './oidc'; export { PKIAuthenticationProvider } from './pki'; 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 71e4f33609a28..7ef1d934a7d13 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.test.ts @@ -6,6 +6,7 @@ import Boom from 'boom'; import sinon from 'sinon'; +import { ByteSizeValue } from '@kbn/config-schema'; import { httpServerMock } from '../../../../../../src/core/server/mocks'; import { mockAuthenticatedUser } from '../../../common/model/authenticated_user.mock'; @@ -15,14 +16,17 @@ import { mockScopedClusterClient, } from './base.mock'; -import { SAMLAuthenticationProvider } from './saml'; +import { SAMLAuthenticationProvider, SAMLLoginStep } from './saml'; describe('SAMLAuthenticationProvider', () => { let provider: SAMLAuthenticationProvider; let mockOptions: MockAuthenticationProviderOptions; beforeEach(() => { mockOptions = mockAuthenticationProviderOptions(); - provider = new SAMLAuthenticationProvider(mockOptions, { realm: 'test-realm' }); + provider = new SAMLAuthenticationProvider(mockOptions, { + realm: 'test-realm', + maxRedirectURLSize: new ByteSizeValue(100), + }); }); it('throws if `realm` option is not specified', () => { @@ -39,6 +43,22 @@ describe('SAMLAuthenticationProvider', () => { ); }); + it('throws if `maxRedirectURLSize` option is not specified', () => { + const providerOptions = mockAuthenticationProviderOptions(); + + expect( + () => new SAMLAuthenticationProvider(providerOptions, { realm: 'test-realm' }) + ).toThrowError('Maximum redirect URL size must be specified'); + + expect( + () => + new SAMLAuthenticationProvider(providerOptions, { + realm: 'test-realm', + maxRedirectURLSize: undefined, + }) + ).toThrowError('Maximum redirect URL size must be specified'); + }); + describe('`login` method', () => { it('gets token and redirects user to requested URL if SAML Response is valid.', async () => { const request = httpServerMock.createKibanaRequest(); @@ -51,8 +71,8 @@ describe('SAMLAuthenticationProvider', () => { const authenticationResult = await provider.login( request, - { samlResponse: 'saml-response-xml' }, - { requestId: 'some-request-id', nextURL: '/test-base-path/some-path' } + { step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' }, + { requestId: 'some-request-id', redirectURL: '/test-base-path/some-path#some-app' } ); sinon.assert.calledWithExactly( @@ -62,7 +82,7 @@ describe('SAMLAuthenticationProvider', () => { ); expect(authenticationResult.redirected()).toBe(true); - expect(authenticationResult.redirectURL).toBe('/test-base-path/some-path'); + expect(authenticationResult.redirectURL).toBe('/test-base-path/some-path#some-app'); expect(authenticationResult.state).toEqual({ username: 'user', accessToken: 'some-token', @@ -75,37 +95,44 @@ describe('SAMLAuthenticationProvider', () => { const authenticationResult = await provider.login( request, - { samlResponse: 'saml-response-xml' }, - { nextURL: '/test-base-path/some-path' } + { step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' }, + {} ); sinon.assert.notCalled(mockOptions.client.callAsInternalUser); expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toEqual( - Boom.badRequest( - 'SAML response state does not have corresponding request id or redirect URL.' - ) + Boom.badRequest('SAML response state does not have corresponding request id.') ); }); - it('fails if SAML Response payload is presented but state does not contain redirect URL.', async () => { + it('redirects to the default location if state contains empty redirect URL.', async () => { const request = httpServerMock.createKibanaRequest(); + mockOptions.client.callAsInternalUser.withArgs('shield.samlAuthenticate').resolves({ + access_token: 'user-initiated-login-token', + refresh_token: 'user-initiated-login-refresh-token', + }); + const authenticationResult = await provider.login( request, - { samlResponse: 'saml-response-xml' }, - { requestId: 'some-request-id' } + { step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' }, + { requestId: 'some-request-id', redirectURL: '' } ); - sinon.assert.notCalled(mockOptions.client.callAsInternalUser); - - expect(authenticationResult.failed()).toBe(true); - expect(authenticationResult.error).toEqual( - Boom.badRequest( - 'SAML response state does not have corresponding request id or redirect URL.' - ) + sinon.assert.calledWithExactly( + mockOptions.client.callAsInternalUser, + 'shield.samlAuthenticate', + { body: { ids: ['some-request-id'], content: 'saml-response-xml', realm: 'test-realm' } } ); + + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe('/base-path/'); + expect(authenticationResult.state).toEqual({ + accessToken: 'user-initiated-login-token', + refreshToken: 'user-initiated-login-refresh-token', + }); }); it('redirects to the default location if state is not presented.', async () => { @@ -117,6 +144,7 @@ describe('SAMLAuthenticationProvider', () => { }); const authenticationResult = await provider.login(request, { + step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml', }); @@ -144,8 +172,8 @@ describe('SAMLAuthenticationProvider', () => { const authenticationResult = await provider.login( request, - { samlResponse: 'saml-response-xml' }, - { requestId: 'some-request-id', nextURL: '/test-base-path/some-path' } + { step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' }, + { requestId: 'some-request-id', redirectURL: '/test-base-path/some-path' } ); sinon.assert.calledWithExactly( @@ -174,7 +202,7 @@ describe('SAMLAuthenticationProvider', () => { const authenticationResult = await provider.login( request, - { samlResponse: 'saml-response-xml' }, + { step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' }, { username: 'user', accessToken: 'some-valid-token', @@ -216,7 +244,7 @@ describe('SAMLAuthenticationProvider', () => { const authenticationResult = await provider.login( request, - { samlResponse: 'saml-response-xml' }, + { step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' }, state ); @@ -259,7 +287,7 @@ describe('SAMLAuthenticationProvider', () => { const authenticationResult = await provider.login( request, - { samlResponse: 'saml-response-xml' }, + { step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' }, state ); @@ -305,7 +333,7 @@ describe('SAMLAuthenticationProvider', () => { const authenticationResult = await provider.login( request, - { samlResponse: 'saml-response-xml' }, + { step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' }, state ); @@ -325,6 +353,177 @@ describe('SAMLAuthenticationProvider', () => { expect(authenticationResult.redirectURL).toBe('/base-path/overwritten_session'); }); }); + + describe('User initiated login with captured redirect URL', () => { + it('fails if state is not available', async () => { + const request = httpServerMock.createKibanaRequest(); + + const authenticationResult = await provider.login(request, { + step: SAMLLoginStep.RedirectURLFragmentCaptured, + redirectURLFragment: '#some-fragment', + }); + + sinon.assert.notCalled(mockOptions.client.callAsInternalUser); + + expect(authenticationResult.failed()).toBe(true); + expect(authenticationResult.error).toEqual( + Boom.badRequest('State does not include URL path to redirect to.') + ); + }); + + it('does not handle AJAX requests.', async () => { + const request = httpServerMock.createKibanaRequest({ headers: { 'kbn-xsrf': 'xsrf' } }); + + const authenticationResult = await provider.login( + request, + { + step: SAMLLoginStep.RedirectURLFragmentCaptured, + redirectURLFragment: '#some-fragment', + }, + { redirectURL: '/test-base-path/some-path' } + ); + + sinon.assert.notCalled(mockOptions.client.callAsInternalUser); + + expect(authenticationResult.notHandled()).toBe(true); + }); + + it('redirects non-AJAX requests to the IdP remembering combined redirect URL.', async () => { + const request = httpServerMock.createKibanaRequest(); + + mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ + id: 'some-request-id', + redirect: 'https://idp-host/path/login?SAMLRequest=some%20request%20', + }); + + const authenticationResult = await provider.login( + request, + { + step: SAMLLoginStep.RedirectURLFragmentCaptured, + redirectURLFragment: '#some-fragment', + }, + { redirectURL: '/test-base-path/some-path' } + ); + + sinon.assert.calledWithExactly( + mockOptions.client.callAsInternalUser, + 'shield.samlPrepare', + { body: { realm: 'test-realm' } } + ); + + expect(mockOptions.logger.warn).not.toHaveBeenCalled(); + + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe( + 'https://idp-host/path/login?SAMLRequest=some%20request%20' + ); + expect(authenticationResult.state).toEqual({ + requestId: 'some-request-id', + redirectURL: '/test-base-path/some-path#some-fragment', + }); + }); + + it('prepends redirect URL fragment with `#` if it does not have one.', async () => { + const request = httpServerMock.createKibanaRequest(); + + mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ + id: 'some-request-id', + redirect: 'https://idp-host/path/login?SAMLRequest=some%20request%20', + }); + + const authenticationResult = await provider.login( + request, + { + step: SAMLLoginStep.RedirectURLFragmentCaptured, + redirectURLFragment: '../some-fragment', + }, + { redirectURL: '/test-base-path/some-path' } + ); + + sinon.assert.calledWithExactly( + mockOptions.client.callAsInternalUser, + 'shield.samlPrepare', + { body: { realm: 'test-realm' } } + ); + + expect(mockOptions.logger.warn).toHaveBeenCalledTimes(1); + expect(mockOptions.logger.warn).toHaveBeenCalledWith( + 'Redirect URL fragment does not start with `#`.' + ); + + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe( + 'https://idp-host/path/login?SAMLRequest=some%20request%20' + ); + expect(authenticationResult.state).toEqual({ + requestId: 'some-request-id', + redirectURL: '/test-base-path/some-path#../some-fragment', + }); + }); + + it('redirects non-AJAX requests to the IdP remembering only redirect URL path if fragment is too large.', async () => { + const request = httpServerMock.createKibanaRequest(); + + mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ + id: 'some-request-id', + redirect: 'https://idp-host/path/login?SAMLRequest=some%20request%20', + }); + + const authenticationResult = await provider.login( + request, + { + step: SAMLLoginStep.RedirectURLFragmentCaptured, + redirectURLFragment: '#some-fragment'.repeat(10), + }, + { redirectURL: '/test-base-path/some-path' } + ); + + sinon.assert.calledWithExactly( + mockOptions.client.callAsInternalUser, + 'shield.samlPrepare', + { body: { realm: 'test-realm' } } + ); + + expect(mockOptions.logger.warn).toHaveBeenCalledTimes(1); + expect(mockOptions.logger.warn).toHaveBeenCalledWith( + 'Max URL size should not exceed 100b but it was 165b. Only URL path is captured.' + ); + + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe( + 'https://idp-host/path/login?SAMLRequest=some%20request%20' + ); + expect(authenticationResult.state).toEqual({ + requestId: 'some-request-id', + redirectURL: '/test-base-path/some-path', + }); + }); + + it('fails if SAML request preparation fails.', async () => { + const request = httpServerMock.createKibanaRequest(); + + const failureReason = new Error('Realm is misconfigured!'); + mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').rejects(failureReason); + + const authenticationResult = await provider.login( + request, + { + step: SAMLLoginStep.RedirectURLFragmentCaptured, + redirectURLFragment: '#some-fragment', + }, + { redirectURL: '/test-base-path/some-path' } + ); + + sinon.assert.calledWithExactly( + mockOptions.client.callAsInternalUser, + 'shield.samlPrepare', + { body: { realm: 'test-realm' } } + ); + + expect(authenticationResult.failed()).toBe(true); + expect(authenticationResult.error).toBe(failureReason); + }); + }); }); describe('`authenticate` method', () => { @@ -352,7 +551,7 @@ describe('SAMLAuthenticationProvider', () => { expect(authenticationResult.notHandled()).toBe(true); }); - it('redirects non-AJAX request that can not be authenticated to the IdP.', async () => { + it('redirects non-AJAX request that can not be authenticated to the "capture fragment" page.', async () => { const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path' }); mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ @@ -360,24 +559,49 @@ describe('SAMLAuthenticationProvider', () => { redirect: 'https://idp-host/path/login?SAMLRequest=some%20request%20', }); - const authenticationResult = await provider.authenticate(request, null); + const authenticationResult = await provider.authenticate(request); + + sinon.assert.notCalled(mockOptions.client.callAsInternalUser); + + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe( + '/mock-server-basepath/api/security/saml/capture-url-fragment' + ); + expect(authenticationResult.state).toEqual({ redirectURL: '/base-path/s/foo/some-path' }); + }); + + it('redirects non-AJAX request that can not be authenticated to the IdP if request path is too large.', async () => { + const request = httpServerMock.createKibanaRequest({ + path: `/s/foo/${'some-path'.repeat(10)}`, + }); + + mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ + id: 'some-request-id', + redirect: 'https://idp-host/path/login?SAMLRequest=some%20request%20', + }); + + const authenticationResult = await provider.authenticate(request); sinon.assert.calledWithExactly(mockOptions.client.callAsInternalUser, 'shield.samlPrepare', { body: { realm: 'test-realm' }, }); + expect(mockOptions.logger.warn).toHaveBeenCalledTimes(1); + expect(mockOptions.logger.warn).toHaveBeenCalledWith( + 'Max URL path size should not exceed 100b but it was 107b. URL is not captured.' + ); + expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe( 'https://idp-host/path/login?SAMLRequest=some%20request%20' ); - expect(authenticationResult.state).toEqual({ - requestId: 'some-request-id', - nextURL: `/base-path/s/foo/some-path`, - }); + expect(authenticationResult.state).toEqual({ requestId: 'some-request-id', redirectURL: '' }); }); it('fails if SAML request preparation fails.', async () => { - const request = httpServerMock.createKibanaRequest({ path: '/some-path' }); + const request = httpServerMock.createKibanaRequest({ + path: `/s/foo/${'some-path'.repeat(10)}`, + }); const failureReason = new Error('Realm is misconfigured!'); mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').rejects(failureReason); @@ -534,7 +758,7 @@ describe('SAMLAuthenticationProvider', () => { ); }); - it('initiates SAML handshake for non-AJAX requests if access token document is missing.', async () => { + it('re-capture URL for non-AJAX requests if access token document is missing.', async () => { const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path' }); const state = { username: 'user', @@ -542,6 +766,39 @@ describe('SAMLAuthenticationProvider', () => { refreshToken: 'expired-refresh-token', }; + mockScopedClusterClient( + mockOptions.client, + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) + ) + .callAsCurrentUser.withArgs('shield.authenticate') + .rejects({ + statusCode: 500, + body: { error: { reason: 'token document is missing and must be present' } }, + }); + + mockOptions.tokens.refresh.withArgs(state.refreshToken).resolves(null); + + const authenticationResult = await provider.authenticate(request, state); + + sinon.assert.notCalled(mockOptions.client.callAsInternalUser); + + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe( + '/mock-server-basepath/api/security/saml/capture-url-fragment' + ); + expect(authenticationResult.state).toEqual({ redirectURL: '/base-path/s/foo/some-path' }); + }); + + it('initiates SAML handshake for non-AJAX requests if access token document is missing and request path is too large.', async () => { + const request = httpServerMock.createKibanaRequest({ + path: `/s/foo/${'some-path'.repeat(10)}`, + }); + const state = { + username: 'user', + accessToken: 'expired-token', + refreshToken: 'expired-refresh-token', + }; + mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ id: 'some-request-id', redirect: 'https://idp-host/path/login?SAMLRequest=some%20request%20', @@ -565,17 +822,19 @@ describe('SAMLAuthenticationProvider', () => { body: { realm: 'test-realm' }, }); + expect(mockOptions.logger.warn).toHaveBeenCalledTimes(1); + expect(mockOptions.logger.warn).toHaveBeenCalledWith( + 'Max URL path size should not exceed 100b but it was 107b. URL is not captured.' + ); + expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe( 'https://idp-host/path/login?SAMLRequest=some%20request%20' ); - expect(authenticationResult.state).toEqual({ - requestId: 'some-request-id', - nextURL: `/base-path/s/foo/some-path`, - }); + expect(authenticationResult.state).toEqual({ requestId: 'some-request-id', redirectURL: '' }); }); - it('initiates SAML handshake for non-AJAX requests if refresh token is expired.', async () => { + it('re-capture URL for non-AJAX requests if refresh token is expired.', async () => { const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path' }); const state = { username: 'user', @@ -583,6 +842,36 @@ describe('SAMLAuthenticationProvider', () => { refreshToken: 'expired-refresh-token', }; + mockScopedClusterClient( + mockOptions.client, + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) + ) + .callAsCurrentUser.withArgs('shield.authenticate') + .rejects({ statusCode: 401 }); + + mockOptions.tokens.refresh.withArgs(state.refreshToken).resolves(null); + + const authenticationResult = await provider.authenticate(request, state); + + sinon.assert.notCalled(mockOptions.client.callAsInternalUser); + + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe( + '/mock-server-basepath/api/security/saml/capture-url-fragment' + ); + expect(authenticationResult.state).toEqual({ redirectURL: '/base-path/s/foo/some-path' }); + }); + + it('initiates SAML handshake for non-AJAX requests if refresh token is expired and request path is too large.', async () => { + const request = httpServerMock.createKibanaRequest({ + path: `/s/foo/${'some-path'.repeat(10)}`, + }); + const state = { + username: 'user', + accessToken: 'expired-token', + refreshToken: 'expired-refresh-token', + }; + mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ id: 'some-request-id', redirect: 'https://idp-host/path/login?SAMLRequest=some%20request%20', @@ -603,14 +892,16 @@ describe('SAMLAuthenticationProvider', () => { body: { realm: 'test-realm' }, }); + expect(mockOptions.logger.warn).toHaveBeenCalledTimes(1); + expect(mockOptions.logger.warn).toHaveBeenCalledWith( + 'Max URL path size should not exceed 100b but it was 107b. URL is not captured.' + ); + expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe( 'https://idp-host/path/login?SAMLRequest=some%20request%20' ); - expect(authenticationResult.state).toEqual({ - requestId: 'some-request-id', - nextURL: `/base-path/s/foo/some-path`, - }); + expect(authenticationResult.state).toEqual({ requestId: 'some-request-id', redirectURL: '' }); }); it('succeeds if `authorization` contains a valid token.', async () => { diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts index f972ec62d1e89..b21a23718f861 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.ts @@ -5,6 +5,7 @@ */ import Boom from 'boom'; +import { ByteSizeValue } from '@kbn/config-schema'; import { KibanaRequest } from '../../../../../../src/core/server'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; @@ -25,19 +26,33 @@ interface ProviderState extends Partial { * Unique identifier of the SAML request initiated the handshake. */ requestId?: string; + /** + * Stores path component of the URL only or in a combination with URL fragment that was used to + * initiate SAML handshake and where we should redirect user after successful authentication. + */ + redirectURL?: string; +} +/** + * Describes possible SAML Login steps. + */ +export enum SAMLLoginStep { /** - * URL to redirect user to after successful SAML handshake. + * The final login step when IdP responds with SAML Response payload. */ - nextURL?: string; + SAMLResponseReceived = 'saml-response-received', + /** + * The login step when we've captured user URL fragment and ready to start SAML handshake. + */ + RedirectURLFragmentCaptured = 'redirect-url-fragment-captured', } /** * Describes the parameters that are required by the provider to process the initial login request. */ -interface ProviderLoginAttempt { - samlResponse: string; -} +type ProviderLoginAttempt = + | { step: SAMLLoginStep.RedirectURLFragmentCaptured; redirectURLFragment: string } + | { step: SAMLLoginStep.SAMLResponseReceived; samlResponse: string }; /** * Checks whether request query includes SAML request from IdP. @@ -56,9 +71,14 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { */ private readonly realm: string; + /** + * Maximum size of the URL we store in the session during SAML handshake. + */ + private readonly maxRedirectURLSize: ByteSizeValue; + constructor( protected readonly options: Readonly, - samlOptions?: Readonly<{ realm?: string }> + samlOptions?: Readonly<{ realm?: string; maxRedirectURLSize?: ByteSizeValue }> ) { super(options); @@ -66,7 +86,12 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { throw new Error('Realm name must be specified'); } + if (!samlOptions.maxRedirectURLSize) { + throw new Error('Maximum redirect URL size must be specified'); + } + this.realm = samlOptions.realm; + this.maxRedirectURLSize = samlOptions.maxRedirectURLSize; } /** @@ -77,11 +102,39 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { */ public async login( request: KibanaRequest, - { samlResponse }: ProviderLoginAttempt, + attempt: ProviderLoginAttempt, state?: ProviderState | null ) { this.logger.debug('Trying to perform a login.'); + if (attempt.step === SAMLLoginStep.RedirectURLFragmentCaptured) { + if (!state || !state.redirectURL) { + const message = 'State does not include URL path to redirect to.'; + this.logger.debug(message); + return AuthenticationResult.failed(Boom.badRequest(message)); + } + + let redirectURLFragment = attempt.redirectURLFragment; + if (redirectURLFragment.length > 0 && !redirectURLFragment.startsWith('#')) { + this.logger.warn('Redirect URL fragment does not start with `#`.'); + redirectURLFragment = `#${redirectURLFragment}`; + } + + let redirectURL = `${state.redirectURL}${redirectURLFragment}`; + const redirectURLSize = new ByteSizeValue(Buffer.byteLength(redirectURL)); + if (this.maxRedirectURLSize.isLessThan(redirectURLSize)) { + this.logger.warn( + `Max URL size should not exceed ${this.maxRedirectURLSize.toString()} but it was ${redirectURLSize.toString()}. Only URL path is captured.` + ); + redirectURL = state.redirectURL; + } else { + this.logger.debug('Captured redirect URL.'); + } + + return this.authenticateViaHandshake(request, redirectURL); + } + + const { samlResponse } = attempt; const authenticationResult = state ? await this.authenticateViaState(request, state) : AuthenticationResult.notHandled(); @@ -142,10 +195,10 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { } } - // If we couldn't authenticate by means of all methods above, let's try to + // If we couldn't authenticate by means of all methods above, let's try to capture user URL and // initiate SAML handshake, otherwise just return authentication result we have. - return authenticationResult.notHandled() - ? await this.authenticateViaHandshake(request) + return authenticationResult.notHandled() && canRedirectRequest(request) + ? this.captureRedirectURL(request) : authenticationResult; } @@ -241,12 +294,12 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { // If we have a `SAMLResponse` and state, but state doesn't contain all the necessary information, // then something unexpected happened and we should fail. - const { requestId: stateRequestId, nextURL: stateRedirectURL } = state || { + const { requestId: stateRequestId, redirectURL: stateRedirectURL } = state || { requestId: '', - nextURL: '', + redirectURL: '', }; - if (state && (!stateRequestId || !stateRedirectURL)) { - const message = 'SAML response state does not have corresponding request id or redirect URL.'; + if (state && !stateRequestId) { + const message = 'SAML response state does not have corresponding request id.'; this.logger.debug(message); return AuthenticationResult.failed(Boom.badRequest(message)); } @@ -403,9 +456,9 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { if (refreshedTokenPair === null) { if (canRedirectRequest(request)) { this.logger.debug( - 'Both access and refresh tokens are expired. Re-initiating SAML handshake.' + 'Both access and refresh tokens are expired. Capturing redirect URL and re-initiating SAML handshake.' ); - return this.authenticateViaHandshake(request); + return this.captureRedirectURL(request); } return AuthenticationResult.failed( @@ -433,8 +486,9 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { /** * Tries to start SAML handshake and eventually receive a token. * @param request Request instance. + * @param redirectURL URL to redirect user to after successful SAML handshake. */ - private async authenticateViaHandshake(request: KibanaRequest) { + private async authenticateViaHandshake(request: KibanaRequest, redirectURL: string) { this.logger.debug('Trying to initiate SAML handshake.'); // If client can't handle redirect response, we shouldn't initiate SAML handshake. @@ -452,13 +506,9 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { ); this.logger.debug('Redirecting to Identity Provider with SAML request.'); - return AuthenticationResult.redirectTo( - redirect, - // Store request id in the state so that we can reuse it once we receive `SAMLResponse`. - { - state: { requestId, nextURL: `${this.options.basePath.get(request)}${request.url.path}` }, - } - ); + + // Store request id in the state so that we can reuse it once we receive `SAMLResponse`. + return AuthenticationResult.redirectTo(redirect, { state: { requestId, redirectURL } }); } catch (err) { this.logger.debug(`Failed to initiate SAML handshake: ${err.message}`); return AuthenticationResult.failed(err); @@ -506,4 +556,30 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { return redirect; } + + /** + * Redirects user to the client-side page that will grab URL fragment and redirect user back to Kibana + * to initiate SAML handshake. + * @param request Request instance. + */ + private captureRedirectURL(request: KibanaRequest) { + const basePath = this.options.basePath.get(request); + const redirectURL = `${basePath}${request.url.path}`; + + // If the size of the path already exceeds the maximum allowed size of the URL to store in the + // session there is no reason to try to capture URL fragment and we start handshake immediately. + // In this case user will be redirected to the Kibana home/root after successful login. + const redirectURLSize = new ByteSizeValue(Buffer.byteLength(redirectURL)); + if (this.maxRedirectURLSize.isLessThan(redirectURLSize)) { + this.logger.warn( + `Max URL path size should not exceed ${this.maxRedirectURLSize.toString()} but it was ${redirectURLSize.toString()}. URL is not captured.` + ); + return this.authenticateViaHandshake(request, ''); + } + + return AuthenticationResult.redirectTo( + `${this.options.basePath.serverBasePath}/api/security/saml/capture-url-fragment`, + { state: { redirectURL } } + ); + } } diff --git a/x-pack/plugins/security/server/config.test.ts b/x-pack/plugins/security/server/config.test.ts index 991841b4fd399..943d582bf484a 100644 --- a/x-pack/plugins/security/server/config.test.ts +++ b/x-pack/plugins/security/server/config.test.ts @@ -13,45 +13,45 @@ import { createConfig$, ConfigSchema } from './config'; describe('config schema', () => { it('generates proper defaults', () => { expect(ConfigSchema.validate({})).toMatchInlineSnapshot(` -Object { - "authc": Object { - "providers": Array [ - "basic", - ], - }, - "cookieName": "sid", - "encryptionKey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "secureCookies": false, - "sessionTimeout": null, -} -`); + Object { + "authc": Object { + "providers": Array [ + "basic", + ], + }, + "cookieName": "sid", + "encryptionKey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "secureCookies": false, + "sessionTimeout": null, + } + `); expect(ConfigSchema.validate({}, { dist: false })).toMatchInlineSnapshot(` -Object { - "authc": Object { - "providers": Array [ - "basic", - ], - }, - "cookieName": "sid", - "encryptionKey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "secureCookies": false, - "sessionTimeout": null, -} -`); + Object { + "authc": Object { + "providers": Array [ + "basic", + ], + }, + "cookieName": "sid", + "encryptionKey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "secureCookies": false, + "sessionTimeout": null, + } + `); expect(ConfigSchema.validate({}, { dist: true })).toMatchInlineSnapshot(` -Object { - "authc": Object { - "providers": Array [ - "basic", - ], - }, - "cookieName": "sid", - "secureCookies": false, - "sessionTimeout": null, -} -`); + Object { + "authc": Object { + "providers": Array [ + "basic", + ], + }, + "cookieName": "sid", + "secureCookies": false, + "sessionTimeout": null, + } + `); }); it('should throw error if xpack.security.encryptionKey is less than 32 characters', () => { @@ -89,15 +89,15 @@ Object { authc: { providers: ['oidc'], oidc: { realm: 'realm-1' } }, }).authc ).toMatchInlineSnapshot(` -Object { - "oidc": Object { - "realm": "realm-1", - }, - "providers": Array [ - "oidc", - ], -} -`); + 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 () => { @@ -114,16 +114,16 @@ Object { authc: { providers: ['oidc', 'basic'], oidc: { realm: 'realm-1' } }, }).authc ).toMatchInlineSnapshot(` -Object { - "oidc": Object { - "realm": "realm-1", - }, - "providers": Array [ - "oidc", - "basic", - ], -} -`); + Object { + "oidc": Object { + "realm": "realm-1", + }, + "providers": Array [ + "oidc", + "basic", + ], + } + `); }); it(`realm is not allowed when authc.providers is "['basic']"`, async () => { @@ -152,15 +152,18 @@ Object { authc: { providers: ['saml'], saml: { realm: 'realm-1' } }, }).authc ).toMatchInlineSnapshot(` -Object { - "providers": Array [ - "saml", - ], - "saml": Object { - "realm": "realm-1", - }, -} -`); + Object { + "providers": Array [ + "saml", + ], + "saml": Object { + "maxRedirectURLSize": ByteSizeValue { + "valueInBytes": 2048, + }, + "realm": "realm-1", + }, + } + `); }); it('`realm` is not allowed if saml provider is not enabled', async () => { @@ -168,6 +171,73 @@ Object { ConfigSchema.validate({ authc: { providers: ['basic'], saml: { realm: 'realm-1' } } }) ).toThrowErrorMatchingInlineSnapshot(`"[authc.saml]: a value wasn't expected to be present"`); }); + + it('`maxRedirectURLSize` accepts any positive value that can coerce to `ByteSizeValue`', async () => { + expect( + ConfigSchema.validate({ + authc: { providers: ['saml'], saml: { realm: 'realm-1' } }, + }).authc.saml + ).toMatchInlineSnapshot(` + Object { + "maxRedirectURLSize": ByteSizeValue { + "valueInBytes": 2048, + }, + "realm": "realm-1", + } + `); + + expect( + ConfigSchema.validate({ + authc: { providers: ['saml'], saml: { realm: 'realm-1', maxRedirectURLSize: 100 } }, + }).authc.saml + ).toMatchInlineSnapshot(` + Object { + "maxRedirectURLSize": ByteSizeValue { + "valueInBytes": 100, + }, + "realm": "realm-1", + } + `); + + expect( + ConfigSchema.validate({ + authc: { providers: ['saml'], saml: { realm: 'realm-1', maxRedirectURLSize: '1kb' } }, + }).authc.saml + ).toMatchInlineSnapshot(` + Object { + "maxRedirectURLSize": ByteSizeValue { + "valueInBytes": 1024, + }, + "realm": "realm-1", + } + `); + + expect( + ConfigSchema.validate({ + authc: { providers: ['saml'], saml: { realm: 'realm-1', maxRedirectURLSize: '100b' } }, + }).authc.saml + ).toMatchInlineSnapshot(` + Object { + "maxRedirectURLSize": ByteSizeValue { + "valueInBytes": 100, + }, + "realm": "realm-1", + } + `); + + expect( + ConfigSchema.validate({ + authc: { providers: ['saml'], saml: { realm: 'realm-1', maxRedirectURLSize: 0 } }, + }).authc.saml + ).toMatchInlineSnapshot(` + Object { + "maxRedirectURLSize": ByteSizeValue { + "valueInBytes": 0, + }, + "realm": "realm-1", + } + `); + }); }); }); @@ -183,12 +253,12 @@ describe('createConfig$()', () => { expect(config).toEqual({ encryptionKey: 'ab'.repeat(16), secureCookies: true }); expect(loggingServiceMock.collect(contextMock.logger).warn).toMatchInlineSnapshot(` -Array [ - Array [ - "Generating a random key for xpack.security.encryptionKey. To prevent sessions from being invalidated on restart, please set xpack.security.encryptionKey in kibana.yml", - ], -] -`); + Array [ + Array [ + "Generating a random key for xpack.security.encryptionKey. To prevent sessions from being invalidated on restart, please set xpack.security.encryptionKey in kibana.yml", + ], + ] + `); }); it('should log a warning if SSL is not configured', async () => { @@ -203,12 +273,12 @@ Array [ expect(config).toEqual({ encryptionKey: 'a'.repeat(32), secureCookies: false }); expect(loggingServiceMock.collect(contextMock.logger).warn).toMatchInlineSnapshot(` -Array [ - Array [ - "Session cookies will be transmitted over insecure connections. This is not recommended.", - ], -] -`); + Array [ + Array [ + "Session cookies will be transmitted over insecure connections. This is not recommended.", + ], + ] + `); }); it('should log a warning if SSL is not configured yet secure cookies are being used', async () => { @@ -223,12 +293,12 @@ Array [ expect(config).toEqual({ encryptionKey: 'a'.repeat(32), secureCookies: true }); expect(loggingServiceMock.collect(contextMock.logger).warn).toMatchInlineSnapshot(` -Array [ - Array [ - "Using secure cookies, but SSL is not enabled inside Kibana. SSL must be configured outside of Kibana to function properly.", - ], -] -`); + Array [ + Array [ + "Using secure cookies, but SSL is not enabled inside Kibana. SSL must be configured outside of Kibana to function properly.", + ], + ] + `); }); it('should set xpack.security.secureCookies if SSL is configured', async () => { diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts index 8df8641dddbed..6fe3fc73e458c 100644 --- a/x-pack/plugins/security/server/config.ts +++ b/x-pack/plugins/security/server/config.ts @@ -38,7 +38,13 @@ export const ConfigSchema = schema.object( authc: schema.object({ providers: schema.arrayOf(schema.string(), { defaultValue: ['basic'], minSize: 1 }), oidc: providerOptionsSchema('oidc', schema.maybe(schema.object({ realm: schema.string() }))), - saml: providerOptionsSchema('saml', schema.maybe(schema.object({ realm: schema.string() }))), + saml: providerOptionsSchema( + 'saml', + schema.object({ + realm: schema.string(), + maxRedirectURLSize: schema.byteSize({ defaultValue: '2kb' }), + }) + ), }), }, // This option should be removed as soon as we entirely migrate config from legacy Security plugin. diff --git a/x-pack/plugins/security/server/plugin.test.ts b/x-pack/plugins/security/server/plugin.test.ts index 1135edc658d81..7fa8f20476f90 100644 --- a/x-pack/plugins/security/server/plugin.test.ts +++ b/x-pack/plugins/security/server/plugin.test.ts @@ -6,6 +6,7 @@ import { coreMock, elasticsearchServiceMock } from '../../../../src/core/server/mocks'; +import { ByteSizeValue } from '@kbn/config-schema'; import { Plugin } from './plugin'; import { IClusterClient, CoreSetup } from '../../../../src/core/server'; @@ -18,7 +19,10 @@ describe('Security Plugin', () => { coreMock.createPluginInitializerContext({ cookieName: 'sid', sessionTimeout: 1500, - authc: { providers: ['saml', 'token'], saml: { realm: 'saml1' } }, + authc: { + providers: ['saml', 'token'], + saml: { realm: 'saml1', maxRedirectURLSize: new ByteSizeValue(2048) }, + }, }) ); diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts index 7cd742a80ac85..18717f3e132b9 100644 --- a/x-pack/plugins/security/server/plugin.ts +++ b/x-pack/plugins/security/server/plugin.ts @@ -15,15 +15,9 @@ import { } from '../../../../src/core/server'; import { deepFreeze } from '../../../../src/core/utils'; import { XPackInfo } from '../../../legacy/plugins/xpack_main/server/lib/xpack_info'; -import { AuthenticatedUser } from '../common/model'; -import { Authenticator, setupAuthentication } from './authentication'; +import { setupAuthentication, Authentication } from './authentication'; import { createConfig$ } from './config'; -import { - CreateAPIKeyParams, - CreateAPIKeyResult, - InvalidateAPIKeyParams, - InvalidateAPIKeyResult, -} from './authentication/api_keys'; +import { defineRoutes } from './routes'; /** * Describes a set of APIs that is available in the legacy platform only and required by this plugin @@ -32,26 +26,14 @@ import { export interface LegacyAPI { xpackInfo: Pick; isSystemAPIRequest: (request: KibanaRequest) => boolean; + cspRules: string; } /** * Describes public Security plugin contract returned at the `setup` stage. */ export interface PluginSetupContract { - authc: { - login: Authenticator['login']; - logout: Authenticator['logout']; - getCurrentUser: (request: KibanaRequest) => Promise; - isAuthenticated: (request: KibanaRequest) => Promise; - createAPIKey: ( - request: KibanaRequest, - params: CreateAPIKeyParams - ) => Promise; - invalidateAPIKey: ( - request: KibanaRequest, - params: InvalidateAPIKeyParams - ) => Promise; - }; + authc: Authentication; config: RecursiveReadonly<{ sessionTimeout: number | null; @@ -90,16 +72,26 @@ export class Plugin { plugins: [require('../../../legacy/server/lib/esjs_shield_plugin')], }); + const authc = await setupAuthentication({ + core, + config, + clusterClient: this.clusterClient, + loggers: this.initializerContext.logger, + getLegacyAPI: this.getLegacyAPI, + }); + + defineRoutes({ + router: core.http.createRouter(), + basePath: core.http.basePath, + logger: this.initializerContext.logger.get('routes'), + config, + authc, + getLegacyAPI: this.getLegacyAPI, + }); + return deepFreeze({ registerLegacyAPI: (legacyAPI: LegacyAPI) => (this.legacyAPI = legacyAPI), - - authc: await setupAuthentication({ - core, - config, - clusterClient: this.clusterClient, - loggers: this.initializerContext.logger, - getLegacyAPI: this.getLegacyAPI, - }), + authc, // We should stop exposing this config as soon as only new platform plugin consumes it. The only // exception may be `sessionTimeout` as other parts of the app may want to know it. diff --git a/x-pack/plugins/security/server/routes/authentication.test.ts b/x-pack/plugins/security/server/routes/authentication.test.ts new file mode 100644 index 0000000000000..67fd86587a0ce --- /dev/null +++ b/x-pack/plugins/security/server/routes/authentication.test.ts @@ -0,0 +1,217 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { Type } from '@kbn/config-schema'; +import { Authentication, AuthenticationResult, SAMLLoginStep } from '../authentication'; +import { defineAuthenticationRoutes } from './authentication'; +import { + httpServerMock, + httpServiceMock, + loggingServiceMock, +} from '../../../../../src/core/server/mocks'; +import { ConfigType } from '../config'; +import { IRouter, RequestHandler, RouteConfig } from '../../../../../src/core/server'; +import { LegacyAPI } from '../plugin'; +import { authenticationMock } from '../authentication/index.mock'; +import { mockAuthenticatedUser } from '../../common/model/authenticated_user.mock'; + +describe('SAML authentication routes', () => { + let router: jest.Mocked; + let authc: jest.Mocked; + beforeEach(() => { + router = httpServiceMock.createRouter(); + authc = authenticationMock.create(); + + defineAuthenticationRoutes({ + router, + basePath: httpServiceMock.createBasePath(), + logger: loggingServiceMock.create().get(), + config: { authc: { providers: ['saml'] } } as ConfigType, + authc, + getLegacyAPI: () => ({ cspRules: 'test-csp-rule' } as LegacyAPI), + }); + }); + + it('does not register any SAML related routes if SAML auth provider is not enabled', () => { + const testRouter = httpServiceMock.createRouter(); + defineAuthenticationRoutes({ + router: testRouter, + basePath: httpServiceMock.createBasePath(), + logger: loggingServiceMock.create().get(), + config: { authc: { providers: ['basic'] } } as ConfigType, + authc: authenticationMock.create(), + getLegacyAPI: () => ({ cspRules: 'test-csp-rule' } as LegacyAPI), + }); + + const samlRoutePathPredicate = ([{ path }]: [{ path: string }, any]) => + path.startsWith('/api/security/saml/'); + expect(testRouter.get.mock.calls.find(samlRoutePathPredicate)).toBeUndefined(); + expect(testRouter.post.mock.calls.find(samlRoutePathPredicate)).toBeUndefined(); + expect(testRouter.put.mock.calls.find(samlRoutePathPredicate)).toBeUndefined(); + expect(testRouter.delete.mock.calls.find(samlRoutePathPredicate)).toBeUndefined(); + }); + + describe('Assertion consumer service endpoint', () => { + let routeHandler: RequestHandler; + let routeConfig: RouteConfig; + beforeEach(() => { + const [acsRouteConfig, acsRouteHandler] = router.post.mock.calls.find( + ([{ path }]) => path === '/api/security/saml/callback' + )!; + + routeConfig = acsRouteConfig; + routeHandler = acsRouteHandler; + }); + + it('additionally registers BWC route', () => { + expect( + router.post.mock.calls.find(([{ path }]) => path === '/api/security/saml/callback') + ).toBeDefined(); + expect( + router.post.mock.calls.find(([{ path }]) => path === '/api/security/v1/saml') + ).toBeDefined(); + }); + + it('correctly defines route.', () => { + expect(routeConfig.options).toEqual({ authRequired: false }); + expect(routeConfig.validate).toEqual({ + body: expect.any(Type), + query: undefined, + params: undefined, + }); + + const bodyValidator = (routeConfig.validate as any).body as Type; + expect(bodyValidator.validate({ SAMLResponse: 'saml-response' })).toEqual({ + SAMLResponse: 'saml-response', + }); + + expect(bodyValidator.validate({ SAMLResponse: 'saml-response', RelayState: '' })).toEqual({ + SAMLResponse: 'saml-response', + RelayState: '', + }); + + expect( + bodyValidator.validate({ SAMLResponse: 'saml-response', RelayState: 'relay-state' }) + ).toEqual({ SAMLResponse: 'saml-response', RelayState: 'relay-state' }); + + expect(() => bodyValidator.validate({})).toThrowErrorMatchingInlineSnapshot( + `"[SAMLResponse]: expected value of type [string] but got [undefined]"` + ); + + expect(() => + bodyValidator.validate({ SAMLResponse: 'saml-response', UnknownArg: 'arg' }) + ).toThrowErrorMatchingInlineSnapshot(`"[UnknownArg]: definition for this key is missing"`); + }); + + it('returns 500 if authentication throws unhandled exception.', async () => { + const unhandledException = new Error('Something went wrong.'); + authc.login.mockRejectedValue(unhandledException); + + const internalServerErrorResponse = Symbol('error'); + const responseFactory = httpServerMock.createResponseFactory(); + responseFactory.internalError.mockReturnValue(internalServerErrorResponse as any); + + const request = httpServerMock.createKibanaRequest({ + body: { SAMLResponse: 'saml-response' }, + }); + + await expect(routeHandler({} as any, request, responseFactory)).resolves.toBe( + internalServerErrorResponse + ); + + expect(authc.login).toHaveBeenCalledWith(request, { + provider: 'saml', + value: { + step: SAMLLoginStep.SAMLResponseReceived, + samlResponse: 'saml-response', + }, + }); + }); + + it('returns 401 if authentication fails.', async () => { + const failureReason = new Error('Something went wrong.'); + authc.login.mockResolvedValue(AuthenticationResult.failed(failureReason)); + + const unauthorizedErrorResponse = Symbol('error'); + const responseFactory = httpServerMock.createResponseFactory(); + responseFactory.unauthorized.mockReturnValue(unauthorizedErrorResponse as any); + + await expect( + routeHandler( + {} as any, + httpServerMock.createKibanaRequest({ body: { SAMLResponse: 'saml-response' } }), + responseFactory + ) + ).resolves.toBe(unauthorizedErrorResponse); + + expect(responseFactory.unauthorized).toHaveBeenCalledWith({ body: failureReason }); + }); + + it('returns 401 if authentication is not handled.', async () => { + authc.login.mockResolvedValue(AuthenticationResult.notHandled()); + + const unauthorizedErrorResponse = Symbol('error'); + const responseFactory = httpServerMock.createResponseFactory(); + responseFactory.unauthorized.mockReturnValue(unauthorizedErrorResponse as any); + + await expect( + routeHandler( + {} as any, + httpServerMock.createKibanaRequest({ body: { SAMLResponse: 'saml-response' } }), + responseFactory + ) + ).resolves.toBe(unauthorizedErrorResponse); + + expect(responseFactory.unauthorized).toHaveBeenCalledWith({ body: undefined }); + }); + + it('returns 401 if authentication completes with unexpected result.', async () => { + authc.login.mockResolvedValue(AuthenticationResult.succeeded(mockAuthenticatedUser())); + + const unauthorizedErrorResponse = Symbol('error'); + const responseFactory = httpServerMock.createResponseFactory(); + responseFactory.unauthorized.mockReturnValue(unauthorizedErrorResponse as any); + + await expect( + routeHandler( + {} as any, + httpServerMock.createKibanaRequest({ body: { SAMLResponse: 'saml-response' } }), + responseFactory + ) + ).resolves.toBe(unauthorizedErrorResponse); + + expect(responseFactory.unauthorized).toHaveBeenCalledWith({ body: undefined }); + }); + + it('redirects if required by the authentication process.', async () => { + authc.login.mockResolvedValue(AuthenticationResult.redirectTo('http://redirect-to/path')); + + const redirectResponse = Symbol('error'); + const responseFactory = httpServerMock.createResponseFactory(); + responseFactory.redirected.mockReturnValue(redirectResponse as any); + + const request = httpServerMock.createKibanaRequest({ + body: { SAMLResponse: 'saml-response' }, + }); + + await expect(routeHandler({} as any, request, responseFactory)).resolves.toBe( + redirectResponse + ); + + expect(authc.login).toHaveBeenCalledWith(request, { + provider: 'saml', + value: { + step: SAMLLoginStep.SAMLResponseReceived, + samlResponse: 'saml-response', + }, + }); + + expect(responseFactory.redirected).toHaveBeenCalledWith({ + headers: { location: 'http://redirect-to/path' }, + }); + }); + }); +}); diff --git a/x-pack/plugins/security/server/routes/authentication.ts b/x-pack/plugins/security/server/routes/authentication.ts new file mode 100644 index 0000000000000..35a63e87339c4 --- /dev/null +++ b/x-pack/plugins/security/server/routes/authentication.ts @@ -0,0 +1,157 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { schema } from '@kbn/config-schema'; +import { RouteDefinitionParams } from '.'; +import { SAMLLoginStep } from '../authentication'; + +export function defineAuthenticationRoutes(params: RouteDefinitionParams) { + if (params.config.authc.providers.includes('saml')) { + defineSAMLRoutes(params); + } +} + +/** + * Defines routes required for SAML authentication. + */ +function defineSAMLRoutes({ + router, + logger, + authc, + getLegacyAPI, + basePath, +}: RouteDefinitionParams) { + function createCustomResourceResponse(body: string, contentType: string) { + return { + body, + headers: { + 'content-type': contentType, + 'cache-control': 'private, no-cache, no-store', + 'content-security-policy': getLegacyAPI().cspRules, + }, + statusCode: 200, + }; + } + + router.get( + { + path: '/api/security/saml/capture-url-fragment', + validate: false, + options: { authRequired: false }, + }, + (context, request, response) => { + // We're also preventing `favicon.ico` request since it can cause new SAML handshake. + return response.custom( + createCustomResourceResponse( + ` + + Kibana SAML Login + + + `, + 'text/html' + ) + ); + } + ); + + router.get( + { + path: '/api/security/saml/capture-url-fragment.js', + validate: false, + options: { authRequired: false }, + }, + (context, request, response) => { + return response.custom( + createCustomResourceResponse( + ` + window.location.replace( + '${basePath.serverBasePath}/api/security/saml/start?redirectURLFragment=' + encodeURIComponent(window.location.hash) + ); + `, + 'text/javascript' + ) + ); + } + ); + + router.get( + { + path: '/api/security/saml/start', + validate: { query: schema.object({ redirectURLFragment: schema.string() }) }, + options: { authRequired: false }, + }, + async (context, request, response) => { + try { + const authenticationResult = await authc.login(request, { + provider: 'saml', + value: { + step: SAMLLoginStep.RedirectURLFragmentCaptured, + redirectURLFragment: request.query.redirectURLFragment, + }, + }); + + // When authenticating using SAML we _expect_ to redirect to the SAML Identity provider. + if (authenticationResult.redirected()) { + return response.redirected({ headers: { location: authenticationResult.redirectURL! } }); + } + + return response.unauthorized(); + } catch (err) { + logger.error(err); + return response.internalError(); + } + } + ); + + // Generate two identical routes with new and deprecated URL and issue a warning if route with + // deprecated URL is ever used. + for (const path of ['/api/security/saml/callback', '/api/security/v1/saml']) { + router.post( + { + path, + validate: { + body: schema.object({ + SAMLResponse: schema.string(), + RelayState: schema.maybe(schema.string()), + }), + }, + options: { authRequired: false }, + }, + async (context, request, response) => { + try { + if (path === '/api/security/v1/saml') { + const serverBasePath = basePath.serverBasePath; + logger.warn( + `The "${serverBasePath}${path}" URL is deprecated and will stop working in the next major version, please use "${serverBasePath}/api/security/saml/callback" URL instead.`, + { tags: ['deprecation'] } + ); + } + + // When authenticating using SAML we _expect_ to redirect to the SAML Identity provider. + const authenticationResult = await authc.login(request, { + provider: 'saml', + value: { + step: SAMLLoginStep.SAMLResponseReceived, + samlResponse: request.body.SAMLResponse, + }, + }); + + if (authenticationResult.redirected()) { + return response.redirected({ + headers: { location: authenticationResult.redirectURL! }, + }); + } + + return response.unauthorized({ body: authenticationResult.error }); + } catch (err) { + logger.error(err); + return response.internalError(); + } + } + ); + } +} diff --git a/x-pack/plugins/security/server/routes/index.ts b/x-pack/plugins/security/server/routes/index.ts new file mode 100644 index 0000000000000..289f87d70b1de --- /dev/null +++ b/x-pack/plugins/security/server/routes/index.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { CoreSetup, IRouter, Logger } from '../../../../../src/core/server'; +import { Authentication } from '../authentication'; +import { ConfigType } from '../config'; +import { defineAuthenticationRoutes } from './authentication'; +import { LegacyAPI } from '../plugin'; + +/** + * Describes parameters used to define HTTP routes. + */ +export interface RouteDefinitionParams { + router: IRouter; + basePath: CoreSetup['http']['basePath']; + logger: Logger; + config: ConfigType; + authc: Authentication; + getLegacyAPI: () => LegacyAPI; +} + +export function defineRoutes(params: RouteDefinitionParams) { + defineAuthenticationRoutes(params); +} diff --git a/x-pack/scripts/functional_tests.js b/x-pack/scripts/functional_tests.js index 46bcac2fc2c67..2ac8fff6ef8ab 100644 --- a/x-pack/scripts/functional_tests.js +++ b/x-pack/scripts/functional_tests.js @@ -17,11 +17,11 @@ require('@kbn/test').runTestsCli([ require.resolve('../test/plugin_api_integration/config.js'), require.resolve('../test/kerberos_api_integration/config'), require.resolve('../test/kerberos_api_integration/anonymous_access.config'), - require.resolve('../test/saml_api_integration/config.js'), - require.resolve('../test/token_api_integration/config.js'), - require.resolve('../test/oidc_api_integration/config.ts'), - require.resolve('../test/oidc_api_integration/implicit_flow.config.ts'), - require.resolve('../test/pki_api_integration/config.ts'), + require.resolve('../test/saml_api_integration/config'), + require.resolve('../test/token_api_integration/config'), + require.resolve('../test/oidc_api_integration/config'), + require.resolve('../test/oidc_api_integration/implicit_flow.config'), + require.resolve('../test/pki_api_integration/config'), require.resolve('../test/spaces_api_integration/spaces_only/config'), require.resolve('../test/spaces_api_integration/security_and_spaces/config_trial'), require.resolve('../test/spaces_api_integration/security_and_spaces/config_basic'), diff --git a/x-pack/test/saml_api_integration/apis/index.js b/x-pack/test/saml_api_integration/apis/index.ts similarity index 66% rename from x-pack/test/saml_api_integration/apis/index.js rename to x-pack/test/saml_api_integration/apis/index.ts index ac08d2e078abf..067e3ad6f08bb 100644 --- a/x-pack/test/saml_api_integration/apis/index.js +++ b/x-pack/test/saml_api_integration/apis/index.ts @@ -4,8 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -export default function ({ loadTestFile }) { - describe('apis SAML', function () { +import { FtrProviderContext } from '../ftr_provider_context'; + +export default function({ loadTestFile }: FtrProviderContext) { + describe('apis SAML', function() { this.tags('ciGroup6'); loadTestFile(require.resolve('./security')); }); diff --git a/x-pack/test/saml_api_integration/apis/security/index.js b/x-pack/test/saml_api_integration/apis/security/index.ts similarity index 71% rename from x-pack/test/saml_api_integration/apis/security/index.js rename to x-pack/test/saml_api_integration/apis/security/index.ts index 0c6825c94c08e..ef5520b967ca3 100644 --- a/x-pack/test/saml_api_integration/apis/security/index.js +++ b/x-pack/test/saml_api_integration/apis/security/index.ts @@ -4,7 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -export default function ({ loadTestFile }) { +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function({ loadTestFile }: FtrProviderContext) { describe('security', () => { loadTestFile(require.resolve('./saml_login')); }); diff --git a/x-pack/test/saml_api_integration/apis/security/saml_login.js b/x-pack/test/saml_api_integration/apis/security/saml_login.ts similarity index 56% rename from x-pack/test/saml_api_integration/apis/security/saml_login.js rename to x-pack/test/saml_api_integration/apis/security/saml_login.ts index 401ffdea0418d..925d28b62f9bb 100644 --- a/x-pack/test/saml_api_integration/apis/security/saml_login.js +++ b/x-pack/test/saml_api_integration/apis/security/saml_login.ts @@ -7,11 +7,13 @@ import querystring from 'querystring'; import url from 'url'; import { delay } from 'bluebird'; -import { getLogoutRequest, getSAMLRequestId, getSAMLResponse } from '../../fixtures/saml_tools'; import expect from '@kbn/expect'; -import request from 'request'; +import request, { Cookie } from 'request'; +import { JSDOM } from 'jsdom'; +import { getLogoutRequest, getSAMLRequestId, getSAMLResponse } from '../../fixtures/saml_tools'; +import { FtrProviderContext } from '../../ftr_provider_context'; -export default function ({ getService }) { +export default function({ getService }: FtrProviderContext) { const chance = getService('chance'); const supertest = getService('supertestWithoutAuth'); const config = getService('config'); @@ -20,19 +22,45 @@ export default function ({ getService }) { function createSAMLResponse(options = {}) { return getSAMLResponse({ - destination: `http://localhost:${kibanaServerConfig.port}/api/security/v1/saml`, + destination: `http://localhost:${kibanaServerConfig.port}/api/security/saml/callback`, sessionIndex: chance.natural(), ...options, }); } - function createLogoutRequest(options = {}) { + function createLogoutRequest(options: { sessionIndex: string }) { return getLogoutRequest({ destination: `http://localhost:${kibanaServerConfig.port}/logout`, ...options, }); } + async function checkSessionCookie(sessionCookie: Cookie) { + expect(sessionCookie.key).to.be('sid'); + expect(sessionCookie.value).to.not.be.empty(); + expect(sessionCookie.path).to.be('/'); + expect(sessionCookie.httpOnly).to.be(true); + + const apiResponse = await supertest + .get('/api/security/v1/me') + .set('kbn-xsrf', 'xxx') + .set('Cookie', sessionCookie.cookieString()) + .expect(200); + + expect(apiResponse.body).to.only.have.keys([ + 'username', + 'full_name', + 'email', + 'roles', + 'metadata', + 'enabled', + 'authentication_realm', + 'lookup_realm', + ]); + + expect(apiResponse.body.username).to.be('a@b.c'); + } + describe('SAML authentication', () => { it('should reject API requests if client is not authenticated', async () => { await supertest @@ -55,37 +83,114 @@ export default function ({ getService }) { const { body: user } = await supertest .get('/api/security/v1/me') .set('kbn-xsrf', 'xxx') - .set('Cookie', request.cookie(cookies[0]).cookieString()) + .set('Cookie', request.cookie(cookies[0])!.cookieString()) .expect(200); expect(user.username).to.eql(username); expect(user.authentication_realm).to.eql({ name: 'reserved', type: 'reserved' }); }); + describe('capture URL fragment', () => { + it('should redirect user to a page that would capture URL fragment', async () => { + const handshakeResponse = await supertest + .get('/abc/xyz/handshake?one=two three') + .expect(302); + + // The cookie should capture current path. + const cookies = handshakeResponse.headers['set-cookie']; + expect(cookies).to.have.length(1); + + const handshakeCookie = request.cookie(cookies[0])!; + expect(handshakeCookie.key).to.be('sid'); + expect(handshakeCookie.value).to.not.be.empty(); + expect(handshakeCookie.path).to.be('/'); + expect(handshakeCookie.httpOnly).to.be(true); + + expect(handshakeResponse.headers.location).to.be('/api/security/saml/capture-url-fragment'); + }); + + it('should return an HTML page that will extract URL fragment', async () => { + const response = await supertest.get('/api/security/saml/capture-url-fragment').expect(200); + + const kibanaBaseURL = url.format({ ...config.get('servers.kibana'), auth: false }); + const dom = new JSDOM(response.text, { + url: kibanaBaseURL, + runScripts: 'dangerously', + resources: 'usable', + beforeParse(window) { + // JSDOM doesn't support changing of `window.location` and throws an exception if script + // tries to do that and we have to workaround this behaviour. We also need to wait until our + // script is loaded and executed, __isScriptExecuted__ is used exactly for that. + (window as Record).__isScriptExecuted__ = new Promise(resolve => { + Object.defineProperty(window, 'location', { + value: { + hash: '#/workpad', + href: `${kibanaBaseURL}/api/security/saml/capture-url-fragment#/workpad`, + replace(newLocation: string) { + this.href = newLocation; + resolve(); + }, + }, + }); + }); + }, + }); + + await (dom.window as Record).__isScriptExecuted__; + + // Check that proxy page is returned with proper headers. + expect(response.headers['content-type']).to.be('text/html; charset=utf-8'); + expect(response.headers['cache-control']).to.be('private, no-cache, no-store'); + expect(response.headers['content-security-policy']).to.be( + `script-src 'unsafe-eval' 'self'; worker-src blob:; child-src blob:; style-src 'unsafe-inline' 'self'` + ); + + // Check that script that forwards URL fragment worked correctly. + expect(dom.window.location.href).to.be( + '/api/security/saml/start?redirectURLFragment=%23%2Fworkpad' + ); + }); + }); + describe('initiating handshake', () => { - it('should properly set cookie and redirect user', async () => { - const handshakeResponse = await supertest.get('/abc/xyz/handshake?one=two three') + const initiateHandshakeURL = `/api/security/saml/start?redirectURLFragment=%23%2Fworkpad`; + + let captureURLCookie: Cookie; + beforeEach(async () => { + const response = await supertest.get('/abc/xyz/handshake?one=two three').expect(302); + captureURLCookie = request.cookie(response.headers['set-cookie'][0])!; + }); + + it('should properly set cookie and redirect user to IdP', async () => { + const handshakeResponse = await supertest + .get(initiateHandshakeURL) + .set('Cookie', captureURLCookie.cookieString()) .expect(302); const cookies = handshakeResponse.headers['set-cookie']; expect(cookies).to.have.length(1); - const handshakeCookie = request.cookie(cookies[0]); + const handshakeCookie = request.cookie(cookies[0])!; expect(handshakeCookie.key).to.be('sid'); expect(handshakeCookie.value).to.not.be.empty(); expect(handshakeCookie.path).to.be('/'); expect(handshakeCookie.httpOnly).to.be(true); - const redirectURL = url.parse(handshakeResponse.headers.location, true /* parseQueryString */); - expect(redirectURL.href.startsWith(`https://elastic.co/sso/saml`)).to.be(true); + const redirectURL = url.parse( + handshakeResponse.headers.location, + true /* parseQueryString */ + ); + expect(redirectURL.href!.startsWith(`https://elastic.co/sso/saml`)).to.be(true); expect(redirectURL.query.SAMLRequest).to.not.be.empty(); }); it('should not allow access to the API', async () => { - const handshakeResponse = await supertest.get('/abc/xyz/handshake?one=two three') + const handshakeResponse = await supertest + .get(initiateHandshakeURL) + .set('Cookie', captureURLCookie.cookieString()) .expect(302); - const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0]); + const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0])!; await supertest .get('/api/security/v1/me') .set('kbn-xsrf', 'xxx') @@ -94,8 +199,10 @@ export default function ({ getService }) { }); it('AJAX requests should not initiate handshake', async () => { - const ajaxResponse = await supertest.get('/abc/xyz/handshake?one=two three') + const ajaxResponse = await supertest + .get(initiateHandshakeURL) .set('kbn-xsrf', 'xxx') + .set('Cookie', captureURLCookie.cookieString()) .expect(401); expect(ajaxResponse.headers['set-cookie']).to.be(undefined); @@ -103,129 +210,92 @@ export default function ({ getService }) { }); describe('finishing handshake', () => { - let handshakeCookie; - let samlRequestId; + let handshakeCookie: Cookie; + let samlRequestId: string; beforeEach(async () => { - const handshakeResponse = await supertest.get('/abc/xyz/handshake?one=two three') + const captureURLResponse = await supertest + .get('/abc/xyz/handshake?one=two three') .expect(302); + const captureURLCookie = request.cookie(captureURLResponse.headers['set-cookie'][0])!; - handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0]); + const handshakeResponse = await supertest + .get(`/api/security/saml/start?redirectURLFragment=%23%2Fworkpad`) + .set('Cookie', captureURLCookie.cookieString()) + .expect(302); + + handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0])!; samlRequestId = await getSAMLRequestId(handshakeResponse.headers.location); }); it('should fail if SAML response is not complemented with handshake cookie', async () => { - await supertest.post('/api/security/v1/saml') + await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') - .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }, {}) + .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }) .expect(401); }); it('should succeed if both SAML response and handshake cookie are provided', async () => { - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') .set('Cookie', handshakeCookie.cookieString()) - .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }, {}) + .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }) .expect(302); // User should be redirected to the URL that initiated handshake. - expect(samlAuthenticationResponse.headers.location).to.be('/abc/xyz/handshake?one=two%20three'); + expect(samlAuthenticationResponse.headers.location).to.be( + '/abc/xyz/handshake?one=two%20three#/workpad' + ); const cookies = samlAuthenticationResponse.headers['set-cookie']; expect(cookies).to.have.length(1); - const sessionCookie = request.cookie(cookies[0]); - expect(sessionCookie.key).to.be('sid'); - expect(sessionCookie.value).to.not.be.empty(); - expect(sessionCookie.path).to.be('/'); - expect(sessionCookie.httpOnly).to.be(true); - - const apiResponse = await supertest - .get('/api/security/v1/me') - .set('kbn-xsrf', 'xxx') - .set('Cookie', sessionCookie.cookieString()) - .expect(200); - - expect(apiResponse.body).to.only.have.keys([ - 'username', - 'full_name', - 'email', - 'roles', - 'metadata', - 'enabled', - 'authentication_realm', - 'lookup_realm', - ]); - - expect(apiResponse.body.username).to.be('a@b.c'); + await checkSessionCookie(request.cookie(cookies[0])!); }); it('should succeed in case of IdP initiated login', async () => { - // Don't pass handshake cookie and don't include `inResponseTo` into SAML response - // to simulate IdP initiated login. - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + // Don't pass handshake cookie and don't include `inResponseTo` into SAML response to simulate IdP initiated login. + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') - .send({ SAMLResponse: await createSAMLResponse() }, {}) + .send({ SAMLResponse: await createSAMLResponse() }) .expect(302); - // User should be redirected to the URL that initiated handshake. + // User should be redirected to the base URL. expect(samlAuthenticationResponse.headers.location).to.be('/'); const cookies = samlAuthenticationResponse.headers['set-cookie']; expect(cookies).to.have.length(1); - const sessionCookie = request.cookie(cookies[0]); - expect(sessionCookie.key).to.be('sid'); - expect(sessionCookie.value).to.not.be.empty(); - expect(sessionCookie.path).to.be('/'); - expect(sessionCookie.httpOnly).to.be(true); - - const apiResponse = await supertest - .get('/api/security/v1/me') - .set('kbn-xsrf', 'xxx') - .set('Cookie', sessionCookie.cookieString()) - .expect(200); - - expect(apiResponse.body).to.only.have.keys([ - 'username', - 'full_name', - 'email', - 'roles', - 'metadata', - 'enabled', - 'authentication_realm', - 'lookup_realm', - ]); - - expect(apiResponse.body.username).to.be('a@b.c'); + await checkSessionCookie(request.cookie(cookies[0])!); }); it('should fail if SAML response is not valid', async () => { - await supertest.post('/api/security/v1/saml') + await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') .set('Cookie', handshakeCookie.cookieString()) - .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: 'some-invalid-request-id' }) }, {}) + .send({ + SAMLResponse: await createSAMLResponse({ inResponseTo: 'some-invalid-request-id' }), + }) .expect(401); }); }); describe('API access with active session', () => { - let sessionCookie; + let sessionCookie: Cookie; beforeEach(async () => { - const handshakeResponse = await supertest.get('/abc/xyz') - .expect(302); - - const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0]); - const samlRequestId = await getSAMLRequestId(handshakeResponse.headers.location); - - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + // Imitate IdP initiated login. + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') - .set('Cookie', handshakeCookie.cookieString()) - .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }, {}) + .send({ SAMLResponse: await createSAMLResponse() }) .expect(302); - sessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0]); + sessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0])!; }); it('should extend cookie on every successful non-system API call', async () => { @@ -236,7 +306,7 @@ export default function ({ getService }) { .expect(200); expect(apiResponseOne.headers['set-cookie']).to.not.be(undefined); - const sessionCookieOne = request.cookie(apiResponseOne.headers['set-cookie'][0]); + const sessionCookieOne = request.cookie(apiResponseOne.headers['set-cookie'][0])!; expect(sessionCookieOne.value).to.not.be.empty(); expect(sessionCookieOne.value).to.not.equal(sessionCookie.value); @@ -248,7 +318,7 @@ export default function ({ getService }) { .expect(200); expect(apiResponseTwo.headers['set-cookie']).to.not.be(undefined); - const sessionCookieTwo = request.cookie(apiResponseTwo.headers['set-cookie'][0]); + const sessionCookieTwo = request.cookie(apiResponseTwo.headers['set-cookie'][0])!; expect(sessionCookieTwo.value).to.not.be.empty(); expect(sessionCookieTwo.value).to.not.equal(sessionCookieOne.value); @@ -278,37 +348,49 @@ export default function ({ getService }) { }); describe('logging out', () => { - let sessionCookie; - let idpSessionIndex; + let sessionCookie: Cookie; + let idpSessionIndex: string; beforeEach(async () => { - const handshakeResponse = await supertest.get('/abc/xyz') + const captureURLResponse = await supertest + .get('/abc/xyz/handshake?one=two three') + .expect(302); + const captureURLCookie = request.cookie(captureURLResponse.headers['set-cookie'][0])!; + + const handshakeResponse = await supertest + .get(`/api/security/saml/start?redirectURLFragment=${encodeURIComponent('#workpad')}`) + .set('Cookie', captureURLCookie.cookieString()) .expect(302); - const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0]); + const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0])!; const samlRequestId = await getSAMLRequestId(handshakeResponse.headers.location); idpSessionIndex = chance.natural(); - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') .set('Cookie', handshakeCookie.cookieString()) .send({ - SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId, sessionIndex: idpSessionIndex }) - }, {}) + SAMLResponse: await createSAMLResponse({ + inResponseTo: samlRequestId, + sessionIndex: idpSessionIndex, + }), + }) .expect(302); - sessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0]); + sessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0])!; }); it('should redirect to IdP with SAML request to complete logout', async () => { - const logoutResponse = await supertest.get('/api/security/v1/logout') + const logoutResponse = await supertest + .get('/api/security/v1/logout') .set('Cookie', sessionCookie.cookieString()) .expect(302); const cookies = logoutResponse.headers['set-cookie']; expect(cookies).to.have.length(1); - const logoutCookie = request.cookie(cookies[0]); + const logoutCookie = request.cookie(cookies[0])!; expect(logoutCookie.key).to.be('sid'); expect(logoutCookie.value).to.be.empty(); expect(logoutCookie.path).to.be('/'); @@ -316,7 +398,7 @@ export default function ({ getService }) { expect(logoutCookie.maxAge).to.be(0); const redirectURL = url.parse(logoutResponse.headers.location, true /* parseQueryString */); - expect(redirectURL.href.startsWith(`https://elastic.co/slo/saml`)).to.be(true); + expect(redirectURL.href!.startsWith(`https://elastic.co/slo/saml`)).to.be(true); expect(redirectURL.query.SAMLRequest).to.not.be.empty(); // Tokens that were stored in the previous cookie should be invalidated as well and old @@ -330,20 +412,20 @@ export default function ({ getService }) { expect(apiResponse.body).to.eql({ error: 'Bad Request', message: 'Both access and refresh tokens are expired.', - statusCode: 400 + statusCode: 400, }); }); it('should redirect to home page if session cookie is not provided', async () => { - const logoutResponse = await supertest.get('/api/security/v1/logout') - .expect(302); + const logoutResponse = await supertest.get('/api/security/v1/logout').expect(302); expect(logoutResponse.headers['set-cookie']).to.be(undefined); expect(logoutResponse.headers.location).to.be('/'); }); it('should reject AJAX requests', async () => { - const ajaxResponse = await supertest.get('/api/security/v1/logout') + const ajaxResponse = await supertest + .get('/api/security/v1/logout') .set('kbn-xsrf', 'xxx') .set('Cookie', sessionCookie.cookieString()) .expect(400); @@ -352,20 +434,21 @@ export default function ({ getService }) { expect(ajaxResponse.body).to.eql({ error: 'Bad Request', message: 'Client should be able to process redirect response.', - statusCode: 400 + statusCode: 400, }); }); it('should invalidate access token on IdP initiated logout', async () => { const logoutRequest = await createLogoutRequest({ sessionIndex: idpSessionIndex }); - const logoutResponse = await supertest.get(`/api/security/v1/logout?${querystring.stringify(logoutRequest)}`) + const logoutResponse = await supertest + .get(`/api/security/v1/logout?${querystring.stringify(logoutRequest)}`) .set('Cookie', sessionCookie.cookieString()) .expect(302); const cookies = logoutResponse.headers['set-cookie']; expect(cookies).to.have.length(1); - const logoutCookie = request.cookie(cookies[0]); + const logoutCookie = request.cookie(cookies[0])!; expect(logoutCookie.key).to.be('sid'); expect(logoutCookie.value).to.be.empty(); expect(logoutCookie.path).to.be('/'); @@ -373,7 +456,7 @@ export default function ({ getService }) { expect(logoutCookie.maxAge).to.be(0); const redirectURL = url.parse(logoutResponse.headers.location, true /* parseQueryString */); - expect(redirectURL.href.startsWith(`https://elastic.co/slo/saml`)).to.be(true); + expect(redirectURL.href!.startsWith(`https://elastic.co/slo/saml`)).to.be(true); expect(redirectURL.query.SAMLResponse).to.not.be.empty(); // Tokens that were stored in the previous cookie should be invalidated as well and old session @@ -387,19 +470,20 @@ export default function ({ getService }) { expect(apiResponse.body).to.eql({ error: 'Bad Request', message: 'Both access and refresh tokens are expired.', - statusCode: 400 + statusCode: 400, }); }); it('should invalidate access token on IdP initiated logout even if there is no Kibana session', async () => { const logoutRequest = await createLogoutRequest({ sessionIndex: idpSessionIndex }); - const logoutResponse = await supertest.get(`/api/security/v1/logout?${querystring.stringify(logoutRequest)}`) + const logoutResponse = await supertest + .get(`/api/security/v1/logout?${querystring.stringify(logoutRequest)}`) .expect(302); expect(logoutResponse.headers['set-cookie']).to.be(undefined); const redirectURL = url.parse(logoutResponse.headers.location, true /* parseQueryString */); - expect(redirectURL.href.startsWith(`https://elastic.co/slo/saml`)).to.be(true); + expect(redirectURL.href!.startsWith(`https://elastic.co/slo/saml`)).to.be(true); expect(redirectURL.query.SAMLResponse).to.not.be.empty(); // Elasticsearch should find and invalidate access and refresh tokens that correspond to provided @@ -414,31 +498,39 @@ export default function ({ getService }) { expect(apiResponse.body).to.eql({ error: 'Bad Request', message: 'Both access and refresh tokens are expired.', - statusCode: 400 + statusCode: 400, }); }); }); describe('API access with expired access token.', () => { - let sessionCookie; + let sessionCookie: Cookie; beforeEach(async () => { - const handshakeResponse = await supertest.get('/abc/xyz') + const captureURLResponse = await supertest + .get('/abc/xyz/handshake?one=two three') + .expect(302); + const captureURLCookie = request.cookie(captureURLResponse.headers['set-cookie'][0])!; + + const handshakeResponse = await supertest + .get(`/api/security/saml/start?redirectURLFragment=${encodeURIComponent('#workpad')}`) + .set('Cookie', captureURLCookie.cookieString()) .expect(302); - const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0]); + const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0])!; const samlRequestId = await getSAMLRequestId(handshakeResponse.headers.location); - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') .set('Cookie', handshakeCookie.cookieString()) - .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }, {}) + .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }) .expect(302); - sessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0]); + sessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0])!; }); - const expectNewSessionCookie = (cookie) => { + const expectNewSessionCookie = (cookie: Cookie) => { expect(cookie.key).to.be('sid'); expect(cookie.value).to.not.be.empty(); expect(cookie.path).to.be('/'); @@ -446,7 +538,7 @@ export default function ({ getService }) { expect(cookie.value).to.not.be(sessionCookie.value); }; - it('expired access token should be automatically refreshed', async function () { + it('expired access token should be automatically refreshed', async function() { this.timeout(40000); // Access token expiration is set to 15s for API integration tests. @@ -464,7 +556,7 @@ export default function ({ getService }) { const firstResponseCookies = firstResponse.headers['set-cookie']; expect(firstResponseCookies).to.have.length(1); - const firstNewCookie = request.cookie(firstResponseCookies[0]); + const firstNewCookie = request.cookie(firstResponseCookies[0])!; expectNewSessionCookie(firstNewCookie); // Request with old cookie should reuse the same refresh token if within 60 seconds. @@ -478,7 +570,7 @@ export default function ({ getService }) { const secondResponseCookies = secondResponse.headers['set-cookie']; expect(secondResponseCookies).to.have.length(1); - const secondNewCookie = request.cookie(secondResponseCookies[0]); + const secondNewCookie = request.cookie(secondResponseCookies[0])!; expectNewSessionCookie(secondNewCookie); expect(firstNewCookie.value).not.to.eql(secondNewCookie.value); @@ -500,25 +592,33 @@ export default function ({ getService }) { }); describe('API access with missing access token document.', () => { - let sessionCookie; + let sessionCookie: Cookie; beforeEach(async () => { - const handshakeResponse = await supertest.get('/abc/xyz') + const captureURLResponse = await supertest + .get('/abc/xyz/handshake?one=two three') + .expect(302); + const captureURLCookie = request.cookie(captureURLResponse.headers['set-cookie'][0])!; + + const handshakeResponse = await supertest + .get(`/api/security/saml/start?redirectURLFragment=${encodeURIComponent('#workpad')}`) + .set('Cookie', captureURLCookie.cookieString()) .expect(302); - const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0]); + const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0])!; const samlRequestId = await getSAMLRequestId(handshakeResponse.headers.location); - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') .set('Cookie', handshakeCookie.cookieString()) - .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }, {}) + .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }) .expect(302); - sessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0]); + sessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0])!; }); - it('should properly set cookie and start new SAML handshake', async function () { + it('should properly set cookie and start new SAML handshake', async function() { // Let's delete tokens from `.security` index directly to simulate the case when // Elasticsearch automatically removes access/refresh token document from the index // after some period of time. @@ -527,56 +627,75 @@ export default function ({ getService }) { q: 'doc_type:token', refresh: true, }); - expect(esResponse).to.have.property('deleted').greaterThan(0); + expect(esResponse) + .to.have.property('deleted') + .greaterThan(0); - const handshakeResponse = await supertest.get('/abc/xyz/handshake?one=two three') + const handshakeResponse = await supertest + .get('/abc/xyz/handshake?one=two three') .set('Cookie', sessionCookie.cookieString()) .expect(302); const cookies = handshakeResponse.headers['set-cookie']; expect(cookies).to.have.length(1); - const handshakeCookie = request.cookie(cookies[0]); + const handshakeCookie = request.cookie(cookies[0])!; expect(handshakeCookie.key).to.be('sid'); expect(handshakeCookie.value).to.not.be.empty(); expect(handshakeCookie.path).to.be('/'); expect(handshakeCookie.httpOnly).to.be(true); - const redirectURL = url.parse(handshakeResponse.headers.location, true /* parseQueryString */); - expect(redirectURL.href.startsWith(`https://elastic.co/sso/saml`)).to.be(true); - expect(redirectURL.query.SAMLRequest).to.not.be.empty(); + expect(handshakeResponse.headers.location).to.be('/api/security/saml/capture-url-fragment'); }); }); describe('IdP initiated login with active session', () => { const existingUsername = 'a@b.c'; - let existingSessionCookie; + let existingSessionCookie: Cookie; beforeEach(async () => { - const handshakeResponse = await supertest.get('/abc/xyz') + const captureURLResponse = await supertest + .get('/abc/xyz/handshake?one=two three') + .expect(302); + const captureURLCookie = request.cookie(captureURLResponse.headers['set-cookie'][0])!; + + const handshakeResponse = await supertest + .get(`/api/security/saml/start?redirectURLFragment=${encodeURIComponent('#workpad')}`) + .set('Cookie', captureURLCookie.cookieString()) .expect(302); - const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0]); + const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0])!; const samlRequestId = await getSAMLRequestId(handshakeResponse.headers.location); - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') .set('Cookie', handshakeCookie.cookieString()) - .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId, username: existingUsername }) }, {}) + .send({ + SAMLResponse: await createSAMLResponse({ + inResponseTo: samlRequestId, + username: existingUsername, + }), + }) .expect(302); - existingSessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0]); + existingSessionCookie = request.cookie( + samlAuthenticationResponse.headers['set-cookie'][0] + )!; }); it('should renew session and redirect to the home page if login is for the same user', async () => { - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') .set('Cookie', existingSessionCookie.cookieString()) - .send({ SAMLResponse: await createSAMLResponse({ username: existingUsername }) }, {}) + .send({ SAMLResponse: await createSAMLResponse({ username: existingUsername }) }) .expect('location', '/') .expect(302); - const newSessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0]); + const newSessionCookie = request.cookie( + samlAuthenticationResponse.headers['set-cookie'][0] + )!; expect(newSessionCookie.value).to.not.be.empty(); expect(newSessionCookie.value).to.not.equal(existingSessionCookie.value); @@ -586,7 +705,10 @@ export default function ({ getService }) { .set('kbn-xsrf', 'xxx') .set('Cookie', existingSessionCookie.cookieString()) .expect(400); - expect(rejectedResponse.body).to.have.property('message', 'Both access and refresh tokens are expired.'); + expect(rejectedResponse.body).to.have.property( + 'message', + 'Both access and refresh tokens are expired.' + ); // Only tokens from new session are valid. const acceptedResponse = await supertest @@ -599,14 +721,17 @@ export default function ({ getService }) { it('should create a new session and redirect to the `overwritten_session` if login is for another user', async () => { const newUsername = 'c@d.e'; - const samlAuthenticationResponse = await supertest.post('/api/security/v1/saml') + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') .set('kbn-xsrf', 'xxx') .set('Cookie', existingSessionCookie.cookieString()) - .send({ SAMLResponse: await createSAMLResponse({ username: newUsername }) }, {}) + .send({ SAMLResponse: await createSAMLResponse({ username: newUsername }) }) .expect('location', '/overwritten_session') .expect(302); - const newSessionCookie = request.cookie(samlAuthenticationResponse.headers['set-cookie'][0]); + const newSessionCookie = request.cookie( + samlAuthenticationResponse.headers['set-cookie'][0] + )!; expect(newSessionCookie.value).to.not.be.empty(); expect(newSessionCookie.value).to.not.equal(existingSessionCookie.value); @@ -616,7 +741,10 @@ export default function ({ getService }) { .set('kbn-xsrf', 'xxx') .set('Cookie', existingSessionCookie.cookieString()) .expect(400); - expect(rejectedResponse.body).to.have.property('message', 'Both access and refresh tokens are expired.'); + expect(rejectedResponse.body).to.have.property( + 'message', + 'Both access and refresh tokens are expired.' + ); // Only tokens from new session are valid. const acceptedResponse = await supertest @@ -627,5 +755,83 @@ export default function ({ getService }) { expect(acceptedResponse.body).to.have.property('username', newUsername); }); }); + + describe('handshake with very long URL path or fragment', () => { + it('should not try to capture URL fragment if path is too big already', async () => { + // 1. Initiate SAML handshake. + const handshakeResponse = await supertest + .get(`/abc/xyz/${'handshake'.repeat(10)}?one=two three`) + .expect(302); + const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0])!; + const redirectURL = url.parse( + handshakeResponse.headers.location, + true /* parseQueryString */ + ); + + expect(redirectURL.href!.startsWith(`https://elastic.co/sso/saml`)).to.be(true); + expect(redirectURL.query.SAMLRequest).to.not.be.empty(); + + // 2. Finish SAML handshake + const samlRequestId = await getSAMLRequestId(handshakeResponse.headers.location); + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') + .set('kbn-xsrf', 'xxx') + .set('Cookie', handshakeCookie.cookieString()) + .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }) + .expect(302); + + // User should be redirected to the root URL since we couldn't even save URL path. + expect(samlAuthenticationResponse.headers.location).to.be('/'); + + await checkSessionCookie( + request.cookie(samlAuthenticationResponse.headers['set-cookie'][0])! + ); + }); + + it('should capture only URL path if URL fragment is too big', async () => { + // 1. Capture current path + const captureURLResponse = await supertest + .get('/abc/xyz/handshake?one=two three') + .expect(302); + const captureURLCookie = request.cookie(captureURLResponse.headers['set-cookie'][0])!; + + expect(captureURLResponse.headers.location).to.be( + '/api/security/saml/capture-url-fragment' + ); + + // 2. Initiate SAML handshake. + const handshakeResponse = await supertest + .get(`/api/security/saml/start?redirectURLFragment=%23%2F${'workpad'.repeat(10)}`) + .set('Cookie', captureURLCookie.cookieString()) + .expect(302); + + const handshakeCookie = request.cookie(handshakeResponse.headers['set-cookie'][0])!; + const redirectURL = url.parse( + handshakeResponse.headers.location, + true /* parseQueryString */ + ); + + expect(redirectURL.href!.startsWith(`https://elastic.co/sso/saml`)).to.be(true); + expect(redirectURL.query.SAMLRequest).to.not.be.empty(); + + // 3. Finish SAML handshake + const samlRequestId = await getSAMLRequestId(handshakeResponse.headers.location); + const samlAuthenticationResponse = await supertest + .post('/api/security/saml/callback') + .set('kbn-xsrf', 'xxx') + .set('Cookie', handshakeCookie.cookieString()) + .send({ SAMLResponse: await createSAMLResponse({ inResponseTo: samlRequestId }) }) + .expect(302); + + // User should be redirected to the URL path that initiated SAML handshake. + expect(samlAuthenticationResponse.headers.location).to.be( + '/abc/xyz/handshake?one=two%20three' + ); + + await checkSessionCookie( + request.cookie(samlAuthenticationResponse.headers['set-cookie'][0])! + ); + }); + }); }); } diff --git a/x-pack/test/saml_api_integration/config.js b/x-pack/test/saml_api_integration/config.ts similarity index 82% rename from x-pack/test/saml_api_integration/config.js rename to x-pack/test/saml_api_integration/config.ts index 1b262008b6d00..8fb807016afeb 100644 --- a/x-pack/test/saml_api_integration/config.js +++ b/x-pack/test/saml_api_integration/config.ts @@ -5,9 +5,12 @@ */ import { resolve } from 'path'; +import { FtrConfigProviderContext } from '@kbn/test/types/ftr'; -export default async function ({ readConfigFile }) { - const kibanaAPITestsConfig = await readConfigFile(require.resolve('../../../test/api_integration/config.js')); +export default async function({ readConfigFile }: FtrConfigProviderContext) { + const kibanaAPITestsConfig = await readConfigFile( + require.resolve('../../../test/api_integration/config.js') + ); const xPackAPITestsConfig = await readConfigFile(require.resolve('../api_integration/config.js')); const kibanaPort = xPackAPITestsConfig.get('servers.kibana.port'); @@ -36,7 +39,7 @@ export default async function ({ readConfigFile }) { 'xpack.security.authc.realms.saml.saml1.idp.entity_id=http://www.elastic.co', `xpack.security.authc.realms.saml.saml1.sp.entity_id=http://localhost:${kibanaPort}`, `xpack.security.authc.realms.saml.saml1.sp.logout=http://localhost:${kibanaPort}/logout`, - `xpack.security.authc.realms.saml.saml1.sp.acs=http://localhost:${kibanaPort}/api/security/v1/saml`, + `xpack.security.authc.realms.saml.saml1.sp.acs=http://localhost:${kibanaPort}/api/security/saml/callback`, 'xpack.security.authc.realms.saml.saml1.attributes.principal=urn:oid:0.0.7', ], }, @@ -46,9 +49,10 @@ export default async function ({ readConfigFile }) { serverArgs: [ ...xPackAPITestsConfig.get('kbnTestServer.serverArgs'), '--optimize.enabled=false', - '--server.xsrf.whitelist=[\"/api/security/v1/saml\"]', + '--server.xsrf.whitelist=["/api/security/saml/callback"]', `--xpack.security.authc.providers=${JSON.stringify(['saml', 'basic'])}`, '--xpack.security.authc.saml.realm=saml1', + '--xpack.security.authc.saml.maxRedirectURLSize=100b', ], }, }; diff --git a/x-pack/test/saml_api_integration/fixtures/saml_tools.js b/x-pack/test/saml_api_integration/fixtures/saml_tools.ts similarity index 75% rename from x-pack/test/saml_api_integration/fixtures/saml_tools.js rename to x-pack/test/saml_api_integration/fixtures/saml_tools.ts index 7515326e0a3b2..f8862e6fb209d 100644 --- a/x-pack/test/saml_api_integration/fixtures/saml_tools.js +++ b/x-pack/test/saml_api_integration/fixtures/saml_tools.ts @@ -9,7 +9,7 @@ import fs from 'fs'; import querystring from 'querystring'; import url from 'url'; import zlib from 'zlib'; -import { promisify } from 'bluebird'; +import { promisify } from 'util'; import { parseString } from 'xml2js'; import { SignedXml } from 'xml-crypto'; @@ -27,18 +27,26 @@ const parseStringAsync = promisify(parseString); const signingKey = fs.readFileSync(require.resolve('../../../../test/dev_certs/server.key')); const signatureAlgorithm = 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'; -export async function getSAMLRequestId(urlWithSAMLRequestId) { - const inflatedSAMLRequest = await inflateRawAsync( - Buffer.from(url.parse(urlWithSAMLRequestId, true /* parseQueryString */).query.SAMLRequest, 'base64') - ); +export async function getSAMLRequestId(urlWithSAMLRequestId: string) { + const inflatedSAMLRequest = (await inflateRawAsync( + Buffer.from( + url.parse(urlWithSAMLRequestId, true /* parseQueryString */).query.SAMLRequest as string, + 'base64' + ) + )) as Buffer; - const parsedSAMLRequest = await parseStringAsync(inflatedSAMLRequest.toString()); + const parsedSAMLRequest = (await parseStringAsync(inflatedSAMLRequest.toString())) as any; return parsedSAMLRequest['saml2p:AuthnRequest'].$.ID; } -export async function getSAMLResponse({ destination, inResponseTo, sessionIndex, username = 'a@b.c' } = {}) { - const issueInstant = (new Date()).toISOString(); - const notOnOrAfter = (new Date(Date.now() + 3600 * 1000)).toISOString(); +export async function getSAMLResponse({ + destination, + inResponseTo, + sessionIndex, + username = 'a@b.c', +}: { destination?: string; inResponseTo?: string; sessionIndex?: string; username?: string } = {}) { + const issueInstant = new Date().toISOString(); + const notOnOrAfter = new Date(Date.now() + 3600 * 1000).toISOString(); const samlAssertionTemplateXML = ` ${signature.getSignedXml()} - `).toString('base64'); + ` + ).toString('base64'); } -export async function getLogoutRequest({ destination, sessionIndex }) { - const issueInstant = (new Date()).toISOString(); +export async function getLogoutRequest({ + destination, + sessionIndex, +}: { + destination: string; + sessionIndex: string; +}) { + const issueInstant = new Date().toISOString(); const logoutRequestTemplateXML = ` = { SAMLRequest: deflatedLogoutRequest.toString('base64'), - SigAlg: signatureAlgorithm + SigAlg: signatureAlgorithm, }; const signer = crypto.createSign('RSA-SHA256'); signer.update(querystring.stringify(queryStringParameters)); - queryStringParameters.Signature = signer.sign(signingKey, 'base64'); + queryStringParameters.Signature = signer.sign(signingKey.toString(), 'base64'); return queryStringParameters; } diff --git a/x-pack/test/saml_api_integration/ftr_provider_context.d.ts b/x-pack/test/saml_api_integration/ftr_provider_context.d.ts new file mode 100644 index 0000000000000..e3add3748f56d --- /dev/null +++ b/x-pack/test/saml_api_integration/ftr_provider_context.d.ts @@ -0,0 +1,11 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { GenericFtrProviderContext } from '@kbn/test/types/ftr'; + +import { services } from './services'; + +export type FtrProviderContext = GenericFtrProviderContext; diff --git a/x-pack/test/saml_api_integration/services.ts b/x-pack/test/saml_api_integration/services.ts new file mode 100644 index 0000000000000..3cbe8fb37356a --- /dev/null +++ b/x-pack/test/saml_api_integration/services.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { services as apiIntegrationServices } from '../api_integration/services'; + +export const services = { + chance: apiIntegrationServices.chance, + es: apiIntegrationServices.es, + supertestWithoutAuth: apiIntegrationServices.supertestWithoutAuth, +}; diff --git a/yarn.lock b/yarn.lock index 8012490e4f8c2..e4f97a31e3386 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1118,18 +1118,6 @@ once "^1.4.0" pump "^3.0.0" -"@elastic/elasticsearch@^7.4.0": - version "7.4.0" - resolved "https://registry.yarnpkg.com/@elastic/elasticsearch/-/elasticsearch-7.4.0.tgz#57f4066acf25e9d4e9b4f6376088433aae6f25d4" - integrity sha512-HpEKHH6mHQRvea3lw4NNJw9ZUS1KmkpwWKHucaHi1svDn+/fEAwY0wD8egL1vZJo4ZmWfCQMjVqGL+Hoy1HYRw== - dependencies: - debug "^4.1.1" - decompress-response "^4.2.0" - into-stream "^5.1.0" - ms "^2.1.1" - once "^1.4.0" - pump "^3.0.0" - "@elastic/eslint-plugin-eui@0.0.2": version "0.0.2" resolved "https://registry.yarnpkg.com/@elastic/eslint-plugin-eui/-/eslint-plugin-eui-0.0.2.tgz#56b9ef03984a05cc213772ae3713ea8ef47b0314" @@ -4243,6 +4231,14 @@ "@types/events" "*" "@types/node" "*" +"@types/xml-crypto@^1.4.0": + version "1.4.0" + resolved "https://registry.yarnpkg.com/@types/xml-crypto/-/xml-crypto-1.4.0.tgz#b586e4819f6bdd0571a3faa9a8098049d5c3cc5a" + integrity sha512-F4bCSHdmXgrbLdbQn5kf77U94gb4Ifn8G6u+97BJ5wGzg3xK4uLlJUDFuOIKLf9pZrEGUSSAU/8/0d3GqVXnYQ== + dependencies: + "@types/node" "*" + xpath "0.0.27" + "@types/xml2js@^0.4.5": version "0.4.5" resolved "https://registry.yarnpkg.com/@types/xml2js/-/xml2js-0.4.5.tgz#d21759b056f282d9c7066f15bbf5c19b908f22fa"