diff --git a/x-pack/plugins/security/common/is_intern_url.test.ts b/x-pack/plugins/security/common/is_intern_url.test.ts deleted file mode 100644 index cee683eec963c..0000000000000 --- a/x-pack/plugins/security/common/is_intern_url.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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 { isInternalURL } from './is_internal_url'; - -describe('isInternalURL', () => { - function commonTestCases(basePath?: string) { - it('should return `true `if URL includes hash fragment', () => { - const href = `${basePath}/app/kibana#/discover/New-Saved-Search`; - expect(isInternalURL(href, basePath)).toBe(true); - }); - - it('should return `false` if URL includes a protocol/hostname', () => { - const href = `https://example.com${basePath}/app/kibana`; - expect(isInternalURL(href, basePath)).toBe(false); - }); - - it('should return `false` if URL includes a port', () => { - const href = `http://localhost:5601${basePath}/app/kibana`; - expect(isInternalURL(href, basePath)).toBe(false); - }); - - it('should return `false` if URL does not specify protocol', () => { - const hrefWithTwoSlashes = `//${basePath}/app/kibana`; - expect(isInternalURL(hrefWithTwoSlashes)).toBe(false); - - const hrefWithThreeSlashes = `///${basePath}/app/kibana`; - expect(isInternalURL(hrefWithThreeSlashes)).toBe(false); - }); - } - - describe('with basePath defined', () => { - const basePath = '/iqf'; - - commonTestCases(basePath); - - it('should return `true` if URL starts with a basepath', () => { - const href = `${basePath}/login`; - expect(isInternalURL(href, basePath)).toBe(true); - }); - - it('should return `false` if URL does not start with basePath', () => { - const href = '/notbasepath/app/kibana'; - expect(isInternalURL(href, basePath)).toBe(false); - }); - }); - - describe('without basePath defined', () => commonTestCases()); -}); diff --git a/x-pack/plugins/security/common/is_internal_url.test.ts b/x-pack/plugins/security/common/is_internal_url.test.ts new file mode 100644 index 0000000000000..7e9f63f069fd0 --- /dev/null +++ b/x-pack/plugins/security/common/is_internal_url.test.ts @@ -0,0 +1,88 @@ +/* + * 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 { isInternalURL } from './is_internal_url'; + +describe('isInternalURL', () => { + describe('with basePath defined', () => { + const basePath = '/iqf'; + + it('should return `true `if URL includes hash fragment', () => { + const href = `${basePath}/app/kibana#/discover/New-Saved-Search`; + expect(isInternalURL(href, basePath)).toBe(true); + }); + + it('should return `false` if URL includes a protocol/hostname', () => { + const href = `https://example.com${basePath}/app/kibana`; + expect(isInternalURL(href, basePath)).toBe(false); + }); + + it('should return `false` if URL includes a port', () => { + const href = `http://localhost:5601${basePath}/app/kibana`; + expect(isInternalURL(href, basePath)).toBe(false); + }); + + it('should return `false` if URL does not specify protocol', () => { + const hrefWithTwoSlashes = `/${basePath}/app/kibana`; + expect(isInternalURL(hrefWithTwoSlashes)).toBe(false); + + const hrefWithThreeSlashes = `//${basePath}/app/kibana`; + expect(isInternalURL(hrefWithThreeSlashes)).toBe(false); + }); + + it('should return `true` if URL starts with a basepath', () => { + for (const href of [basePath, `${basePath}/`, `${basePath}/login`, `${basePath}/login/`]) { + expect(isInternalURL(href, basePath)).toBe(true); + } + }); + + it('should return `false` if URL does not start with basePath', () => { + for (const href of [ + '/notbasepath/app/kibana', + `${basePath}_/login`, + basePath.slice(1), + `${basePath.slice(1)}/app/kibana`, + ]) { + expect(isInternalURL(href, basePath)).toBe(false); + } + }); + + it('should return `true` if relative path does not escape base path', () => { + const href = `${basePath}/app/kibana/../../management`; + expect(isInternalURL(href, basePath)).toBe(true); + }); + + it('should return `false` if relative path escapes base path', () => { + const href = `${basePath}/app/kibana/../../../management`; + expect(isInternalURL(href, basePath)).toBe(false); + }); + }); + + describe('without basePath defined', () => { + it('should return `true `if URL includes hash fragment', () => { + const href = '/app/kibana#/discover/New-Saved-Search'; + expect(isInternalURL(href)).toBe(true); + }); + + it('should return `false` if URL includes a protocol/hostname', () => { + const href = 'https://example.com/app/kibana'; + expect(isInternalURL(href)).toBe(false); + }); + + it('should return `false` if URL includes a port', () => { + const href = 'http://localhost:5601/app/kibana'; + expect(isInternalURL(href)).toBe(false); + }); + + it('should return `false` if URL does not specify protocol', () => { + const hrefWithTwoSlashes = `//app/kibana`; + expect(isInternalURL(hrefWithTwoSlashes)).toBe(false); + + const hrefWithThreeSlashes = `///app/kibana`; + expect(isInternalURL(hrefWithThreeSlashes)).toBe(false); + }); + }); +}); diff --git a/x-pack/plugins/security/common/is_internal_url.ts b/x-pack/plugins/security/common/is_internal_url.ts index 33a4870c676f9..7449df2e8369b 100644 --- a/x-pack/plugins/security/common/is_internal_url.ts +++ b/x-pack/plugins/security/common/is_internal_url.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { parse } from 'url'; +import { parse, URL } from 'url'; export function isInternalURL(url: string, basePath = '') { const { protocol, hostname, port, pathname } = parse( @@ -22,5 +22,17 @@ export function isInternalURL(url: string, basePath = '') { return false; } - return String(pathname).startsWith(basePath); + if (basePath) { + // Now we need to normalize URL to make sure any relative path segments (`..`) cannot escape expected + // base path. We can rely on `URL` with a localhost to automatically "normalize" the URL. + const normalizedPathname = new URL(String(pathname), 'https://localhost').pathname; + return ( + // Normalized pathname can add a leading slash, but we should also make sure it's included in + // the original URL too + pathname?.startsWith('/') && + (normalizedPathname === basePath || normalizedPathname.startsWith(`${basePath}/`)) + ); + } + + return true; } 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 b81d7e9951bf7..f8e735a658a20 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.test.ts @@ -110,6 +110,51 @@ describe('SAMLAuthenticationProvider', () => { ); }); + it('gets token and redirects user to the requested URL if SAML Response is valid ignoring Relay State.', async () => { + const request = httpServerMock.createKibanaRequest(); + + mockOptions.client.callAsInternalUser.mockResolvedValue({ + username: 'user', + access_token: 'some-token', + refresh_token: 'some-refresh-token', + }); + + provider = new SAMLAuthenticationProvider(mockOptions, { + realm: 'test-realm', + maxRedirectURLSize: new ByteSizeValue(100), + useRelayStateDeepLink: true, + }); + await expect( + provider.login( + request, + { + type: SAMLLogin.LoginWithSAMLResponse, + samlResponse: 'saml-response-xml', + relayState: `${mockOptions.basePath.serverBasePath}/app/some-app#some-deep-link`, + }, + { + requestId: 'some-request-id', + redirectURL: '/test-base-path/some-path#some-app', + realm: 'test-realm', + } + ) + ).resolves.toEqual( + AuthenticationResult.redirectTo('/test-base-path/some-path#some-app', { + state: { + username: 'user', + accessToken: 'some-token', + refreshToken: 'some-refresh-token', + realm: 'test-realm', + }, + }) + ); + + expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith( + 'shield.samlAuthenticate', + { body: { ids: ['some-request-id'], content: 'saml-response-xml', realm: 'test-realm' } } + ); + }); + it('fails if SAML Response payload is presented but state does not contain SAML Request token.', async () => { const request = httpServerMock.createKibanaRequest(); @@ -178,6 +223,45 @@ describe('SAMLAuthenticationProvider', () => { ); }); + it('redirects to the default location if state contains empty redirect URL ignoring Relay State.', async () => { + const request = httpServerMock.createKibanaRequest(); + + mockOptions.client.callAsInternalUser.mockResolvedValue({ + access_token: 'user-initiated-login-token', + refresh_token: 'user-initiated-login-refresh-token', + }); + + provider = new SAMLAuthenticationProvider(mockOptions, { + realm: 'test-realm', + maxRedirectURLSize: new ByteSizeValue(100), + useRelayStateDeepLink: true, + }); + await expect( + provider.login( + request, + { + type: SAMLLogin.LoginWithSAMLResponse, + samlResponse: 'saml-response-xml', + relayState: `${mockOptions.basePath.serverBasePath}/app/some-app#some-deep-link`, + }, + { requestId: 'some-request-id', redirectURL: '', realm: 'test-realm' } + ) + ).resolves.toEqual( + AuthenticationResult.redirectTo('/base-path/', { + state: { + accessToken: 'user-initiated-login-token', + refreshToken: 'user-initiated-login-refresh-token', + realm: 'test-realm', + }, + }) + ); + + expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith( + 'shield.samlAuthenticate', + { body: { ids: ['some-request-id'], content: 'saml-response-xml', realm: 'test-realm' } } + ); + }); + it('redirects to the default location if state is not presented.', async () => { const request = httpServerMock.createKibanaRequest(); @@ -233,7 +317,6 @@ describe('SAMLAuthenticationProvider', () => { describe('IdP initiated login', () => { beforeEach(() => { - mockOptions = mockAuthenticationProviderOptions({ name: 'saml' }); mockOptions.basePath.get.mockReturnValue(mockOptions.basePath.serverBasePath); const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient(); @@ -266,10 +349,10 @@ describe('SAMLAuthenticationProvider', () => { provider.login(httpServerMock.createKibanaRequest({ headers: {} }), { type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml', - relayState: '/mock-server-basepath/app/some-app#some-deep-link', + relayState: `${mockOptions.basePath.serverBasePath}/app/some-app#some-deep-link`, }) ).resolves.toEqual( - AuthenticationResult.redirectTo('/mock-server-basepath/', { + AuthenticationResult.redirectTo(`${mockOptions.basePath.serverBasePath}/`, { state: { username: 'user', accessToken: 'valid-token', @@ -287,7 +370,7 @@ describe('SAMLAuthenticationProvider', () => { samlResponse: 'saml-response-xml', }) ).resolves.toEqual( - AuthenticationResult.redirectTo('/mock-server-basepath/', { + AuthenticationResult.redirectTo(`${mockOptions.basePath.serverBasePath}/`, { state: { username: 'user', accessToken: 'valid-token', @@ -303,10 +386,10 @@ describe('SAMLAuthenticationProvider', () => { provider.login(httpServerMock.createKibanaRequest({ headers: {} }), { type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml', - relayState: 'https://evil.com/mock-server-basepath/app/some-app#some-deep-link', + relayState: `https://evil.com${mockOptions.basePath.serverBasePath}/app/some-app#some-deep-link`, }) ).resolves.toEqual( - AuthenticationResult.redirectTo('/mock-server-basepath/', { + AuthenticationResult.redirectTo(`${mockOptions.basePath.serverBasePath}/`, { state: { username: 'user', accessToken: 'valid-token', @@ -322,10 +405,10 @@ describe('SAMLAuthenticationProvider', () => { provider.login(httpServerMock.createKibanaRequest({ headers: {} }), { type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml', - relayState: '//mock-server-basepath/app/some-app#some-deep-link', + relayState: `//${mockOptions.basePath.serverBasePath}/app/some-app#some-deep-link`, }) ).resolves.toEqual( - AuthenticationResult.redirectTo('/mock-server-basepath/', { + AuthenticationResult.redirectTo(`${mockOptions.basePath.serverBasePath}/`, { state: { username: 'user', accessToken: 'valid-token', @@ -341,17 +424,20 @@ describe('SAMLAuthenticationProvider', () => { provider.login(httpServerMock.createKibanaRequest({ headers: {} }), { type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml', - relayState: '/mock-server-basepath/app/some-app#some-deep-link', + relayState: `${mockOptions.basePath.serverBasePath}/app/some-app#some-deep-link`, }) ).resolves.toEqual( - AuthenticationResult.redirectTo('/mock-server-basepath/app/some-app#some-deep-link', { - state: { - username: 'user', - accessToken: 'valid-token', - refreshToken: 'valid-refresh-token', - realm: 'test-realm', - }, - }) + AuthenticationResult.redirectTo( + `${mockOptions.basePath.serverBasePath}/app/some-app#some-deep-link`, + { + state: { + username: 'user', + accessToken: 'valid-token', + refreshToken: 'valid-refresh-token', + realm: 'test-realm', + }, + } + ) ); }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts index 16b316ec00937..9a4b7dd679ccb 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.ts @@ -349,16 +349,29 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { }, }); + // IdP can pass `RelayState` with the deep link in Kibana during IdP initiated login and + // depending on the configuration we may need to redirect user to this URL. + let redirectURLFromRelayState; + if (isIdPInitiatedLogin && relayState) { + if (!this.useRelayStateDeepLink) { + this.options.logger.debug( + `"RelayState" is provided, but deep links support is not enabled for "${this.type}/${this.options.name}" provider.` + ); + } else if (!isInternalURL(relayState, this.options.basePath.serverBasePath)) { + this.options.logger.debug( + `"RelayState" is provided, but it is not a valid Kibana internal URL.` + ); + } else { + this.options.logger.debug( + `User will be redirected to the Kibana internal URL specified in "RelayState".` + ); + redirectURLFromRelayState = relayState; + } + } + this.logger.debug('Login has been performed with SAML response.'); return AuthenticationResult.redirectTo( - // IdP can pass `RelayState` with the deep link in Kibana during IdP initiated login and - // depending on the configuration we may need to redirect user to this link. - isIdPInitiatedLogin && - relayState && - this.useRelayStateDeepLink && - isInternalURL(relayState, this.options.basePath.serverBasePath) - ? relayState - : stateRedirectURL || `${this.options.basePath.get(request)}/`, + redirectURLFromRelayState || stateRedirectURL || `${this.options.basePath.get(request)}/`, { state: { username, accessToken, refreshToken, realm: this.realm } } ); } catch (err) {