From 8de766904d28da27ad707a920ade9510e86a2941 Mon Sep 17 00:00:00 2001 From: Andrea Del Rio Date: Tue, 27 Apr 2021 09:12:17 -0700 Subject: [PATCH 01/28] [K8] Small fixes (#98099) --- .../sidebar/discover_field_search.tsx | 4 +- .../public/lib/panel/_embeddable_panel.scss | 5 +- .../public/top_nav_menu/_index.scss | 10 +++- .../solution_toolbar/items/button.scss | 5 ++ .../solution_toolbar/items/quick_group.scss | 6 +++ .../home/components/filter_list_button.tsx | 46 ++++++++++--------- 6 files changed, 49 insertions(+), 27 deletions(-) diff --git a/src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx b/src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx index 67dda6dd0e9a8..e11c1716efe6b 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx @@ -233,7 +233,7 @@ export function DiscoverFieldSearch({ onChange, value, types, useNewFieldsApi }: const footer = () => { return ( - + - + {i18n.translate('discover.fieldChooser.filter.filterByTypeLabel', { defaultMessage: 'Filter by type', })} diff --git a/src/plugins/embeddable/public/lib/panel/_embeddable_panel.scss b/src/plugins/embeddable/public/lib/panel/_embeddable_panel.scss index f7ee1f3c741c4..9072c26576097 100644 --- a/src/plugins/embeddable/public/lib/panel/_embeddable_panel.scss +++ b/src/plugins/embeddable/public/lib/panel/_embeddable_panel.scss @@ -120,9 +120,10 @@ // EDITING MODE .embPanel--editing { - border-style: dashed; - border-color: $euiColorMediumShade; + border-style: dashed !important; + border-color: $euiColorMediumShade !important; transition: all $euiAnimSpeedFast $euiAnimSlightResistance; + border-width: $euiBorderWidthThin; &:hover, &:focus { diff --git a/src/plugins/navigation/public/top_nav_menu/_index.scss b/src/plugins/navigation/public/top_nav_menu/_index.scss index bc27cf061eb68..9af1bb5434bb1 100644 --- a/src/plugins/navigation/public/top_nav_menu/_index.scss +++ b/src/plugins/navigation/public/top_nav_menu/_index.scss @@ -1,5 +1,13 @@ .kbnTopNavMenu { - margin-right: $euiSizeXS; + @include kbnThemeStyle('v7') { + margin-right: $euiSizeXS; + } + + @include kbnThemeStyle('v8') { + button:last-child { + margin-right: 0; + } + } } .kbnTopNavMenu__badgeWrapper { diff --git a/src/plugins/presentation_util/public/components/solution_toolbar/items/button.scss b/src/plugins/presentation_util/public/components/solution_toolbar/items/button.scss index b8022201acf59..4fc3651ee9f73 100644 --- a/src/plugins/presentation_util/public/components/solution_toolbar/items/button.scss +++ b/src/plugins/presentation_util/public/components/solution_toolbar/items/button.scss @@ -4,4 +4,9 @@ // Lighten the border color for all states border-color: $euiBorderColor !important; // sass-lint:disable-line no-important + + @include kbnThemeStyle('v8') { + border-width: $euiBorderWidthThin; + border-style: solid; + } } diff --git a/src/plugins/presentation_util/public/components/solution_toolbar/items/quick_group.scss b/src/plugins/presentation_util/public/components/solution_toolbar/items/quick_group.scss index 870a9a945ed5d..876ee659b71d7 100644 --- a/src/plugins/presentation_util/public/components/solution_toolbar/items/quick_group.scss +++ b/src/plugins/presentation_util/public/components/solution_toolbar/items/quick_group.scss @@ -1,6 +1,12 @@ .quickButtonGroup { .quickButtonGroup__button { background-color: $euiColorEmptyShade; + @include kbnThemeStyle('v8') { + // sass-lint:disable-block no-important + border-width: $euiBorderWidthThin !important; + border-style: solid !important; + border-color: $euiBorderColor !important; + } } // Temporary fix for two tone icons to make them monochrome diff --git a/x-pack/plugins/index_management/public/application/sections/home/components/filter_list_button.tsx b/x-pack/plugins/index_management/public/application/sections/home/components/filter_list_button.tsx index 44f565f98cdb0..4bd9a01380c0e 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/components/filter_list_button.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/components/filter_list_button.tsx @@ -7,7 +7,7 @@ import React, { useState } from 'react'; import { FormattedMessage } from '@kbn/i18n/react'; -import { EuiFilterButton, EuiPopover, EuiFilterSelectItem } from '@elastic/eui'; +import { EuiFilterButton, EuiFilterGroup, EuiPopover, EuiFilterSelectItem } from '@elastic/eui'; interface Filter { name: string; @@ -65,26 +65,28 @@ export function FilterListButton({ onChange, filters }: Props< ); return ( - -
- {Object.entries(filters).map(([filter, item], index) => ( - toggleFilter(filter as T)} - data-test-subj="filterItem" - > - {(item as Filter).name} - - ))} -
-
+ + +
+ {Object.entries(filters).map(([filter, item], index) => ( + toggleFilter(filter as T)} + data-test-subj="filterItem" + > + {(item as Filter).name} + + ))} +
+
+
); } From aa281ffad7c0b1808154e00b937251f36e2ff76a Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Tue, 27 Apr 2021 09:36:27 -0700 Subject: [PATCH 02/28] [Metrics UI] Unskip Home Page Functional Test (#98085) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../test/functional/apps/infra/home_page.ts | 48 +------------------ x-pack/test/functional/apps/infra/index.ts | 20 ++++---- x-pack/test/functional/apps/infra/link_to.ts | 2 +- 3 files changed, 14 insertions(+), 56 deletions(-) diff --git a/x-pack/test/functional/apps/infra/home_page.ts b/x-pack/test/functional/apps/infra/home_page.ts index a5b8e69833fb6..1cc7c87f3a1a8 100644 --- a/x-pack/test/functional/apps/infra/home_page.ts +++ b/x-pack/test/functional/apps/infra/home_page.ts @@ -5,24 +5,17 @@ * 2.0. */ -import expect from '@kbn/expect/expect.js'; import { FtrProviderContext } from '../../ftr_provider_context'; import { DATES } from './constants'; const DATE_WITH_DATA = DATES.metricsAndLogs.hosts.withData; const DATE_WITHOUT_DATA = DATES.metricsAndLogs.hosts.withoutData; -const COMMON_REQUEST_HEADERS = { - 'kbn-xsrf': 'some-xsrf-token', -}; - export default ({ getPageObjects, getService }: FtrProviderContext) => { const esArchiver = getService('esArchiver'); const pageObjects = getPageObjects(['common', 'infraHome']); - const supertest = getService('supertest'); - // FLAKY: https://github.com/elastic/kibana/issues/75724 - describe.skip('Home page', function () { + describe('Home page', function () { this.tags('includeFirefox'); before(async () => { await esArchiver.load('empty_kibana'); @@ -54,45 +47,6 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await pageObjects.infraHome.goToTime(DATE_WITHOUT_DATA); await pageObjects.infraHome.getNoMetricsDataPrompt(); }); - - it('records telemetry for hosts', async () => { - await pageObjects.infraHome.goToTime(DATE_WITH_DATA); - await pageObjects.infraHome.getWaffleMap(); - - const resp = await supertest - .post(`/api/telemetry/v2/clusters/_stats`) - .set(COMMON_REQUEST_HEADERS) - .set('Accept', 'application/json') - .send({ - unencrypted: true, - }) - .expect(200) - .then((res: any) => res.body); - - expect( - resp[0].stack_stats.kibana.plugins.infraops.last_24_hours.hits.infraops_hosts - ).to.be.greaterThan(0); - }); - - it('records telemetry for docker', async () => { - await pageObjects.infraHome.goToTime(DATE_WITH_DATA); - await pageObjects.infraHome.getWaffleMap(); - await pageObjects.infraHome.goToDocker(); - - const resp = await supertest - .post(`/api/telemetry/v2/clusters/_stats`) - .set(COMMON_REQUEST_HEADERS) - .set('Accept', 'application/json') - .send({ - unencrypted: true, - }) - .expect(200) - .then((res: any) => res.body); - - expect( - resp[0].stack_stats.kibana.plugins.infraops.last_24_hours.hits.infraops_docker - ).to.be.greaterThan(0); - }); }); }); }; diff --git a/x-pack/test/functional/apps/infra/index.ts b/x-pack/test/functional/apps/infra/index.ts index 9c828253245d0..8cdcf6b619b26 100644 --- a/x-pack/test/functional/apps/infra/index.ts +++ b/x-pack/test/functional/apps/infra/index.ts @@ -8,15 +8,19 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default ({ loadTestFile }: FtrProviderContext) => { - describe('InfraOps app', function () { + describe('InfraOps App', function () { this.tags('ciGroup7'); - loadTestFile(require.resolve('./metrics_anomalies')); - loadTestFile(require.resolve('./home_page')); loadTestFile(require.resolve('./feature_controls')); - loadTestFile(require.resolve('./log_entry_categories_tab')); - loadTestFile(require.resolve('./log_entry_rate_tab')); - loadTestFile(require.resolve('./logs_source_configuration')); - loadTestFile(require.resolve('./metrics_source_configuration')); - loadTestFile(require.resolve('./link_to')); + describe('Metrics UI', function () { + loadTestFile(require.resolve('./home_page')); + loadTestFile(require.resolve('./metrics_source_configuration')); + loadTestFile(require.resolve('./metrics_anomalies')); + }); + describe('Logs UI', function () { + loadTestFile(require.resolve('./log_entry_categories_tab')); + loadTestFile(require.resolve('./log_entry_rate_tab')); + loadTestFile(require.resolve('./logs_source_configuration')); + loadTestFile(require.resolve('./link_to')); + }); }); }; diff --git a/x-pack/test/functional/apps/infra/link_to.ts b/x-pack/test/functional/apps/infra/link_to.ts index b7a554cea311f..3e070fb9849b1 100644 --- a/x-pack/test/functional/apps/infra/link_to.ts +++ b/x-pack/test/functional/apps/infra/link_to.ts @@ -22,7 +22,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { const traceId = '433b4651687e18be2c6c8e3b11f53d09'; - describe('Infra link-to', function () { + describe('link-to Logs', function () { it('redirects to the logs app and parses URL search params correctly', async () => { const location = { hash: '', From 52a90e3dc9a19e7c211d13fd5c672634b3f51526 Mon Sep 17 00:00:00 2001 From: Nicolas Chaulet Date: Tue, 27 Apr 2021 13:04:08 -0400 Subject: [PATCH 03/28] [Fleet] Use default port for fleet server cloud url (#98492) --- .../fleet/server/services/settings.test.ts | 18 +++++++++++++++++- .../plugins/fleet/server/services/settings.ts | 6 +++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/fleet/server/services/settings.test.ts b/x-pack/plugins/fleet/server/services/settings.test.ts index a9f9600addc39..87b3e163c1bb3 100644 --- a/x-pack/plugins/fleet/server/services/settings.test.ts +++ b/x-pack/plugins/fleet/server/services/settings.test.ts @@ -17,7 +17,7 @@ describe('getCloudFleetServersHosts', () => { expect(getCloudFleetServersHosts()).toBeUndefined(); }); - it('should return fleet server hosts if cloud is correctly setup', () => { + it('should return fleet server hosts if cloud is correctly setup with default port == 443', () => { mockedAppContextService.getCloud.mockReturnValue({ cloudId: 'dXMtZWFzdC0xLmF3cy5mb3VuZC5pbyRjZWM2ZjI2MWE3NGJmMjRjZTMzYmI4ODExYjg0Mjk0ZiRjNmMyY2E2ZDA0MjI0OWFmMGNjN2Q3YTllOTYyNTc0Mw==', @@ -32,4 +32,20 @@ describe('getCloudFleetServersHosts', () => { ] `); }); + + it('should return fleet server hosts if cloud is correctly setup with a default port', () => { + mockedAppContextService.getCloud.mockReturnValue({ + cloudId: + 'test:dGVzdC5mcjo5MjQzJGRhM2I2YjNkYWY5ZDRjODE4ZjI4ZmEzNDdjMzgzODViJDgxMmY4NWMxZjNjZTQ2YTliYjgxZjFjMWIxMzRjNmRl', + isCloudEnabled: true, + deploymentId: 'deployment-id-1', + apm: {}, + }); + + expect(getCloudFleetServersHosts()).toMatchInlineSnapshot(` + Array [ + "https://deployment-id-1.fleet.test.fr:9243", + ] + `); + }); }); diff --git a/x-pack/plugins/fleet/server/services/settings.ts b/x-pack/plugins/fleet/server/services/settings.ts index 4ef9a3a95cbd0..2046e2571c926 100644 --- a/x-pack/plugins/fleet/server/services/settings.ts +++ b/x-pack/plugins/fleet/server/services/settings.ts @@ -84,6 +84,10 @@ export function getCloudFleetServersHosts() { } // Fleet Server url are formed like this `https://.fleet. - return [`https://${cloudSetup.deploymentId}.fleet.${res.host}`]; + return [ + `https://${cloudSetup.deploymentId}.fleet.${res.host}${ + res.defaultPort !== '443' ? `:${res.defaultPort}` : '' + }`, + ]; } } From 808959e316082718c1c1081c362fbc6c91f2fdbc Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Tue, 27 Apr 2021 19:09:54 +0200 Subject: [PATCH 04/28] Handle `401 Unauthorized` errors in a more user-friendly way (#94927) --- .eslintrc.js | 4 + test/common/services/security/user.ts | 30 +- test/scripts/jenkins_xpack_build_plugins.sh | 1 + .../plugins/reporting/server/routes/jobs.ts | 2 + x-pack/plugins/security/common/constants.ts | 15 + .../capture_url/capture_url_app.test.ts | 63 ++- .../capture_url/capture_url_app.ts | 45 +- .../authentication/login/components/index.ts | 2 +- .../login/components/login_form/index.ts | 2 +- .../components/login_form/login_form.test.tsx | 6 +- .../components/login_form/login_form.tsx | 14 +- .../authentication/login/login_page.test.tsx | 9 +- .../authentication/login/login_page.tsx | 34 +- .../__snapshots__/prompt_page.test.tsx.snap | 5 + .../unauthenticated_page.test.tsx.snap | 3 + .../authentication_service.test.mocks.ts | 9 + .../authentication_service.test.ts | 458 ++++++++++++++++-- .../authentication/authentication_service.ts | 72 ++- .../authentication/authenticator.test.ts | 72 ++- .../server/authentication/authenticator.ts | 41 +- .../can_redirect_request.test.ts | 30 ++ .../authentication/can_redirect_request.ts | 7 +- .../authentication/providers/base.mock.ts | 1 + .../server/authentication/providers/base.ts | 4 + .../authentication/providers/oidc.test.ts | 76 ++- .../server/authentication/providers/oidc.ts | 54 ++- .../authentication/providers/saml.test.ts | 63 ++- .../server/authentication/providers/saml.ts | 55 ++- .../security/server/authentication/tokens.ts | 10 +- .../unauthenticated_page.test.tsx | 35 ++ .../authentication/unauthenticated_page.tsx | 55 +++ .../reset_session_page.test.tsx.snap | 2 +- .../authorization/authorization_service.tsx | 24 +- .../authorization/reset_session_page.test.tsx | 10 +- .../authorization/reset_session_page.tsx | 120 ++--- x-pack/plugins/security/server/index.ts | 1 + x-pack/plugins/security/server/plugin.ts | 7 +- .../security/server/prompt_page.test.tsx | 57 +++ .../plugins/security/server/prompt_page.tsx | 96 ++++ .../routes/authentication/common.test.ts | 6 +- .../server/routes/authentication/common.ts | 3 +- .../server/routes/authentication/oidc.ts | 17 +- .../server/routes/authentication/saml.test.ts | 7 +- .../server/routes/authentication/saml.ts | 16 +- x-pack/plugins/security/server/routes/tags.ts | 27 ++ .../server/routes/views/capture_url.test.ts | 42 +- .../server/routes/views/capture_url.ts | 6 +- .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - .../apis/security/basic_login.js | 4 +- .../test/functional/apps/security/security.ts | 2 +- .../tests/anonymous/login.ts | 20 +- .../tests/kerberos/kerberos_login.ts | 14 +- .../login_selector/basic_functionality.ts | 61 ++- .../oidc/authorization_code_flow/oidc_auth.ts | 157 +++--- .../tests/oidc/implicit_flow/oidc_auth.ts | 18 +- .../tests/pki/pki_auth.ts | 16 +- .../tests/saml/saml_login.ts | 161 +++--- .../common/test_endpoints/kibana.json | 7 + .../common/test_endpoints/public/index.ts | 9 + .../common/test_endpoints/public/plugin.tsx | 29 ++ .../common/test_endpoints/server/index.ts | 15 + .../test_endpoints/server/init_routes.ts | 38 ++ .../login_selector.config.ts | 3 + .../test/security_functional/oidc.config.ts | 3 + .../test/security_functional/saml.config.ts | 3 + .../login_selector/basic_functionality.ts | 60 +++ .../tests/oidc/url_capture.ts | 21 + .../tests/saml/url_capture.ts | 21 + 69 files changed, 1837 insertions(+), 545 deletions(-) create mode 100644 x-pack/plugins/security/server/__snapshots__/prompt_page.test.tsx.snap create mode 100644 x-pack/plugins/security/server/authentication/__snapshots__/unauthenticated_page.test.tsx.snap create mode 100644 x-pack/plugins/security/server/authentication/authentication_service.test.mocks.ts create mode 100644 x-pack/plugins/security/server/authentication/unauthenticated_page.test.tsx create mode 100644 x-pack/plugins/security/server/authentication/unauthenticated_page.tsx create mode 100644 x-pack/plugins/security/server/prompt_page.test.tsx create mode 100644 x-pack/plugins/security/server/prompt_page.tsx create mode 100644 x-pack/plugins/security/server/routes/tags.ts create mode 100644 x-pack/test/security_functional/fixtures/common/test_endpoints/kibana.json create mode 100644 x-pack/test/security_functional/fixtures/common/test_endpoints/public/index.ts create mode 100644 x-pack/test/security_functional/fixtures/common/test_endpoints/public/plugin.tsx create mode 100644 x-pack/test/security_functional/fixtures/common/test_endpoints/server/index.ts create mode 100644 x-pack/test/security_functional/fixtures/common/test_endpoints/server/init_routes.ts diff --git a/.eslintrc.js b/.eslintrc.js index 19ba7cacc3c44..0f7af42fafbfd 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1377,6 +1377,10 @@ module.exports = { ['parent', 'sibling', 'index'], ], pathGroups: [ + { + pattern: '{**,.}/*.test.mocks', + group: 'unknown', + }, { pattern: '{@kbn/**,src/**,kibana{,/**}}', group: 'internal', diff --git a/test/common/services/security/user.ts b/test/common/services/security/user.ts index 3bd31bb5ed186..d6813105ecbf6 100644 --- a/test/common/services/security/user.ts +++ b/test/common/services/security/user.ts @@ -33,7 +33,7 @@ export class User { public async delete(username: string) { this.log.debug(`deleting user ${username}`); - const { data, status, statusText } = await await this.kbnClient.request({ + const { data, status, statusText } = await this.kbnClient.request({ path: `/internal/security/users/${username}`, method: 'DELETE', }); @@ -44,4 +44,32 @@ export class User { } this.log.debug(`deleted user ${username}`); } + + public async disable(username: string) { + this.log.debug(`disabling user ${username}`); + const { data, status, statusText } = await this.kbnClient.request({ + path: `/internal/security/users/${encodeURIComponent(username)}/_disable`, + method: 'POST', + }); + if (status !== 204) { + throw new Error( + `Expected status code of 204, received ${status} ${statusText}: ${util.inspect(data)}` + ); + } + this.log.debug(`disabled user ${username}`); + } + + public async enable(username: string) { + this.log.debug(`enabling user ${username}`); + const { data, status, statusText } = await this.kbnClient.request({ + path: `/internal/security/users/${encodeURIComponent(username)}/_enable`, + method: 'POST', + }); + if (status !== 204) { + throw new Error( + `Expected status code of 204, received ${status} ${statusText}: ${util.inspect(data)}` + ); + } + this.log.debug(`enabled user ${username}`); + } } diff --git a/test/scripts/jenkins_xpack_build_plugins.sh b/test/scripts/jenkins_xpack_build_plugins.sh index 496964983cc6c..cb0b5ec1d56da 100755 --- a/test/scripts/jenkins_xpack_build_plugins.sh +++ b/test/scripts/jenkins_xpack_build_plugins.sh @@ -13,6 +13,7 @@ node scripts/build_kibana_platform_plugins \ --scan-dir "$XPACK_DIR/test/plugin_api_perf/plugins" \ --scan-dir "$XPACK_DIR/test/licensing_plugin/plugins" \ --scan-dir "$XPACK_DIR/test/usage_collection/plugins" \ + --scan-dir "$XPACK_DIR/test/security_functional/fixtures/common" \ --scan-dir "$KIBANA_DIR/examples" \ --scan-dir "$XPACK_DIR/examples" \ --workers 12 \ diff --git a/x-pack/plugins/reporting/server/routes/jobs.ts b/x-pack/plugins/reporting/server/routes/jobs.ts index 7141b1a141185..3f2a95a34224c 100644 --- a/x-pack/plugins/reporting/server/routes/jobs.ts +++ b/x-pack/plugins/reporting/server/routes/jobs.ts @@ -7,6 +7,7 @@ import { schema } from '@kbn/config-schema'; import Boom from '@hapi/boom'; +import { ROUTE_TAG_CAN_REDIRECT } from '../../../security/server'; import { ReportingCore } from '../'; import { API_BASE_URL } from '../../common/constants'; import { authorizedUserPreRoutingFactory } from './lib/authorized_user_pre_routing'; @@ -198,6 +199,7 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { docId: schema.string({ minLength: 3 }), }), }, + options: { tags: [ROUTE_TAG_CAN_REDIRECT] }, }, userHandler(async (user, context, req, res) => { // ensure the async dependencies are loaded diff --git a/x-pack/plugins/security/common/constants.ts b/x-pack/plugins/security/common/constants.ts index a205109f537e7..ef83230fc2aba 100644 --- a/x-pack/plugins/security/common/constants.ts +++ b/x-pack/plugins/security/common/constants.ts @@ -19,7 +19,22 @@ export const GLOBAL_RESOURCE = '*'; export const APPLICATION_PREFIX = 'kibana-'; export const RESERVED_PRIVILEGES_APPLICATION_WILDCARD = 'kibana-*'; +/** + * This is the key of a query parameter that contains the name of the authentication provider that should be used to + * authenticate request. It's also used while the user is being redirected during single-sign-on authentication flows. + * That query parameter is discarded after the authentication flow succeeds. See the `Authenticator`, + * `OIDCAuthenticationProvider`, and `SAMLAuthenticationProvider` classes for more information. + */ export const AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER = 'auth_provider_hint'; + +/** + * This is the key of a query parameter that contains metadata about the (client-side) URL hash while the user is being + * redirected during single-sign-on authentication flows. That query parameter is discarded after the authentication + * flow succeeds. See the `Authenticator`, `OIDCAuthenticationProvider`, and `SAMLAuthenticationProvider` classes for + * more information. + */ +export const AUTH_URL_HASH_QUERY_STRING_PARAMETER = 'auth_url_hash'; + export const LOGOUT_PROVIDER_QUERY_STRING_PARAMETER = 'provider'; export const LOGOUT_REASON_QUERY_STRING_PARAMETER = 'msg'; export const NEXT_URL_QUERY_STRING_PARAMETER = 'next'; diff --git a/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.test.ts b/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.test.ts index bf68d9f7a6e5e..44fd5ab195341 100644 --- a/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.test.ts +++ b/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.test.ts @@ -5,19 +5,29 @@ * 2.0. */ -import type { AppMount, ScopedHistory } from 'src/core/public'; -import { coreMock, scopedHistoryMock } from 'src/core/public/mocks'; +import { coreMock } from 'src/core/public/mocks'; import { captureURLApp } from './capture_url_app'; describe('captureURLApp', () => { + let mockLocationReplace: jest.Mock; beforeAll(() => { + mockLocationReplace = jest.fn(); Object.defineProperty(window, 'location', { - value: { href: 'https://some-host' }, + value: { + href: 'https://some-host', + hash: '#/?_g=()', + origin: 'https://some-host', + replace: mockLocationReplace, + }, writable: true, }); }); + beforeEach(() => { + mockLocationReplace.mockClear(); + }); + it('properly registers application', () => { const coreSetupMock = coreMock.createSetup(); @@ -42,34 +52,37 @@ describe('captureURLApp', () => { it('properly handles captured URL', async () => { window.location.href = `https://host.com/mock-base-path/internal/security/capture-url?next=${encodeURIComponent( - '/mock-base-path/app/home' - )}&providerType=saml&providerName=saml1#/?_g=()`; + '/mock-base-path/app/home?auth_provider_hint=saml1' + )}#/?_g=()`; const coreSetupMock = coreMock.createSetup(); - coreSetupMock.http.post.mockResolvedValue({ location: '/mock-base-path/app/home#/?_g=()' }); - captureURLApp.create(coreSetupMock); const [[{ mount }]] = coreSetupMock.application.register.mock.calls; - await (mount as AppMount)({ - element: document.createElement('div'), - appBasePath: '', - onAppLeave: jest.fn(), - setHeaderActionMenu: jest.fn(), - history: (scopedHistoryMock.create() as unknown) as ScopedHistory, - }); + await mount(coreMock.createAppMountParamters()); - expect(coreSetupMock.http.post).toHaveBeenCalledTimes(1); - expect(coreSetupMock.http.post).toHaveBeenCalledWith('/internal/security/login', { - body: JSON.stringify({ - providerType: 'saml', - providerName: 'saml1', - currentURL: `https://host.com/mock-base-path/internal/security/capture-url?next=${encodeURIComponent( - '/mock-base-path/app/home' - )}&providerType=saml&providerName=saml1#/?_g=()`, - }), - }); + expect(mockLocationReplace).toHaveBeenCalledTimes(1); + expect(mockLocationReplace).toHaveBeenCalledWith( + 'https://some-host/mock-base-path/app/home?auth_provider_hint=saml1&auth_url_hash=%23%2F%3F_g%3D%28%29#/?_g=()' + ); + expect(coreSetupMock.fatalErrors.add).not.toHaveBeenCalled(); + }); - expect(window.location.href).toBe('/mock-base-path/app/home#/?_g=()'); + it('properly handles open redirects', async () => { + window.location.href = `https://host.com/mock-base-path/internal/security/capture-url?next=${encodeURIComponent( + 'https://evil.com/mock-base-path/app/home?auth_provider_hint=saml1' + )}#/?_g=()`; + + const coreSetupMock = coreMock.createSetup(); + captureURLApp.create(coreSetupMock); + + const [[{ mount }]] = coreSetupMock.application.register.mock.calls; + await mount(coreMock.createAppMountParamters()); + + expect(mockLocationReplace).toHaveBeenCalledTimes(1); + expect(mockLocationReplace).toHaveBeenCalledWith( + 'https://some-host/?auth_url_hash=%23%2F%3F_g%3D%28%29' + ); + expect(coreSetupMock.fatalErrors.add).not.toHaveBeenCalled(); }); }); diff --git a/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts b/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts index 7797ce4e62102..af45314c5bacb 100644 --- a/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts +++ b/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts @@ -5,10 +5,11 @@ * 2.0. */ -import { parse } from 'url'; - import type { ApplicationSetup, FatalErrorsSetup, HttpSetup } from 'src/core/public'; +import { AUTH_URL_HASH_QUERY_STRING_PARAMETER } from '../../../common/constants'; +import { parseNext } from '../../../common/parse_next'; + interface CreateDeps { application: ApplicationSetup; http: HttpSetup; @@ -22,20 +23,17 @@ interface CreateDeps { * path segment into the `next` query string parameter (so that it's not lost during redirect). And * since browsers preserve hash fragments during redirects (assuming redirect location doesn't * specify its own hash fragment, which is true in our case) this app can capture both path and - * hash URL segments and send them back to the authentication provider via login endpoint. + * hash URL segments and re-try request sending hash fragment in a dedicated query string parameter. * * The flow can look like this: - * 1. User visits `/app/kibana#/management/elasticsearch` that initiates authentication. - * 2. Provider redirect user to `/internal/security/capture-url?next=%2Fapp%2Fkibana&providerType=saml&providerName=saml1`. - * 3. Browser preserves hash segment and users ends up at `/internal/security/capture-url?next=%2Fapp%2Fkibana&providerType=saml&providerName=saml1#/management/elasticsearch`. - * 4. The app captures full URL and sends it back as is via login endpoint: - * { - * providerType: 'saml', - * providerName: 'saml1', - * currentURL: 'https://kibana.com/internal/security/capture-url?next=%2Fapp%2Fkibana&providerType=saml&providerName=saml1#/management/elasticsearch' - * } - * 5. Login endpoint handler parses and validates `next` parameter, joins it with the hash segment - * and finally passes it to the provider that initiated capturing. + * 1. User visits `https://kibana.com/app/kibana#/management/elasticsearch` that initiates authentication. + * 2. Provider redirect user to `/internal/security/capture-url?next=%2Fapp%2Fkibana&auth_provider_hint=saml1`. + * 3. Browser preserves hash segment and users ends up at `/internal/security/capture-url?next=%2Fapp%2Fkibana&auth_provider_hint=saml1#/management/elasticsearch`. + * 4. The app reconstructs original URL, adds `auth_url_hash` query string parameter with the captured hash fragment and redirects user to: + * https://kibana.com/app/kibana?auth_provider_hint=saml1&auth_url_hash=%23%2Fmanagement%2Felasticsearch#/management/elasticsearch + * 5. Once Kibana receives this request, it immediately picks exactly the same provider to handle authentication (based on `auth_provider_hint=saml1`), + * and, since it has full URL now (original request path, query string and hash extracted from `auth_url_hash=%23%2Fmanagement%2Felasticsearch`), + * it can proceed to a proper authentication handshake. */ export const captureURLApp = Object.freeze({ id: 'security_capture_url', @@ -48,19 +46,14 @@ export const captureURLApp = Object.freeze({ appRoute: '/internal/security/capture-url', async mount() { try { - const { providerName, providerType } = parse(window.location.href, true).query ?? {}; - if (!providerName || !providerType) { - fatalErrors.add(new Error('Provider to capture URL for is not specified.')); - return () => {}; - } - - const { location } = await http.post<{ location: string }>('/internal/security/login', { - body: JSON.stringify({ providerType, providerName, currentURL: window.location.href }), - }); - - window.location.href = location; + const url = new URL( + parseNext(window.location.href, http.basePath.serverBasePath), + window.location.origin + ); + url.searchParams.append(AUTH_URL_HASH_QUERY_STRING_PARAMETER, window.location.hash); + window.location.replace(url.toString()); } catch (err) { - fatalErrors.add(new Error('Cannot login with captured URL.')); + fatalErrors.add(new Error(`Cannot parse current URL: ${err && err.message}.`)); } return () => {}; diff --git a/x-pack/plugins/security/public/authentication/login/components/index.ts b/x-pack/plugins/security/public/authentication/login/components/index.ts index 66e91a390784a..63928e4e82e37 100644 --- a/x-pack/plugins/security/public/authentication/login/components/index.ts +++ b/x-pack/plugins/security/public/authentication/login/components/index.ts @@ -5,5 +5,5 @@ * 2.0. */ -export { LoginForm } from './login_form'; +export { LoginForm, LoginFormMessageType } from './login_form'; export { DisabledLoginForm } from './disabled_login_form'; diff --git a/x-pack/plugins/security/public/authentication/login/components/login_form/index.ts b/x-pack/plugins/security/public/authentication/login/components/login_form/index.ts index 6215f4e1e5b7a..d12ea30c784cb 100644 --- a/x-pack/plugins/security/public/authentication/login/components/login_form/index.ts +++ b/x-pack/plugins/security/public/authentication/login/components/login_form/index.ts @@ -5,4 +5,4 @@ * 2.0. */ -export { LoginForm } from './login_form'; +export { LoginForm, MessageType as LoginFormMessageType } from './login_form'; diff --git a/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.test.tsx b/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.test.tsx index f58150d4580b8..e816fa032a0e5 100644 --- a/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.test.tsx +++ b/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.test.tsx @@ -14,7 +14,7 @@ import ReactMarkdown from 'react-markdown'; import { findTestSubject, mountWithIntl, nextTick, shallowWithIntl } from '@kbn/test/jest'; import { coreMock } from 'src/core/public/mocks'; -import { LoginForm, PageMode } from './login_form'; +import { LoginForm, MessageType, PageMode } from './login_form'; function expectPageMode(wrapper: ReactWrapper, mode: PageMode) { const assertions: Array<[string, boolean]> = @@ -90,7 +90,7 @@ describe('LoginForm', () => { { }); expect(wrapper.find(EuiCallOut).props().title).toEqual( - `Invalid username or password. Please try again.` + `Username or password is incorrect. Please try again.` ); }); diff --git a/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.tsx b/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.tsx index ca573ada36d22..df131e2eac133 100644 --- a/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.tsx +++ b/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.tsx @@ -40,7 +40,7 @@ interface Props { http: HttpStart; notifications: NotificationsStart; selector: LoginSelector; - infoMessage?: string; + message?: { type: MessageType.Danger | MessageType.Info; content: string }; loginAssistanceMessage: string; loginHelp?: string; authProviderHint?: string; @@ -66,7 +66,7 @@ enum LoadingStateType { AutoLogin, } -enum MessageType { +export enum MessageType { None, Info, Danger, @@ -106,9 +106,7 @@ export class LoginForm extends Component { loadingState: { type: LoadingStateType.None }, username: '', password: '', - message: this.props.infoMessage - ? { type: MessageType.Info, content: this.props.infoMessage } - : { type: MessageType.None }, + message: this.props.message || { type: MessageType.None }, mode, previousMode: mode, }; @@ -206,7 +204,7 @@ export class LoginForm extends Component { > @@ -480,8 +478,8 @@ export class LoginForm extends Component { const message = (error as IHttpFetchError).response?.status === 401 ? i18n.translate( - 'xpack.security.login.basicLoginForm.invalidUsernameOrPasswordErrorMessage', - { defaultMessage: 'Invalid username or password. Please try again.' } + 'xpack.security.login.basicLoginForm.usernameOrPasswordIsIncorrectErrorMessage', + { defaultMessage: 'Username or password is incorrect. Please try again.' } ) : i18n.translate('xpack.security.login.basicLoginForm.unknownErrorMessage', { defaultMessage: 'Oops! Error. Try again.', diff --git a/x-pack/plugins/security/public/authentication/login/login_page.test.tsx b/x-pack/plugins/security/public/authentication/login/login_page.test.tsx index a9596aff3bf0e..b3e2fac3ea2cc 100644 --- a/x-pack/plugins/security/public/authentication/login/login_page.test.tsx +++ b/x-pack/plugins/security/public/authentication/login/login_page.test.tsx @@ -14,7 +14,7 @@ import { coreMock } from 'src/core/public/mocks'; import { AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER } from '../../../common/constants'; import type { LoginState } from '../../../common/login_state'; -import { DisabledLoginForm, LoginForm } from './components'; +import { DisabledLoginForm, LoginForm, LoginFormMessageType } from './components'; import { LoginPage } from './login_page'; const createLoginState = (options?: Partial) => { @@ -228,9 +228,12 @@ describe('LoginPage', () => { resetHttpMock(); // so the calls don't show in the BasicLoginForm snapshot }); - const { authProviderHint, infoMessage } = wrapper.find(LoginForm).props(); + const { authProviderHint, message } = wrapper.find(LoginForm).props(); expect(authProviderHint).toBe('basic1'); - expect(infoMessage).toBe('Your session has timed out. Please log in again.'); + expect(message).toEqual({ + type: LoginFormMessageType.Info, + content: 'Your session has timed out. Please log in again.', + }); }); it('renders as expected when loginAssistanceMessage is set', async () => { diff --git a/x-pack/plugins/security/public/authentication/login/login_page.tsx b/x-pack/plugins/security/public/authentication/login/login_page.tsx index 562adec7918d3..40438ac1c78f3 100644 --- a/x-pack/plugins/security/public/authentication/login/login_page.tsx +++ b/x-pack/plugins/security/public/authentication/login/login_page.tsx @@ -23,7 +23,7 @@ import { LOGOUT_REASON_QUERY_STRING_PARAMETER, } from '../../../common/constants'; import type { LoginState } from '../../../common/login_state'; -import { DisabledLoginForm, LoginForm } from './components'; +import { DisabledLoginForm, LoginForm, LoginFormMessageType } from './components'; interface Props { http: HttpStart; @@ -36,18 +36,34 @@ interface State { loginState: LoginState | null; } -const infoMessageMap = new Map([ +const messageMap = new Map([ [ 'SESSION_EXPIRED', - i18n.translate('xpack.security.login.sessionExpiredDescription', { - defaultMessage: 'Your session has timed out. Please log in again.', - }), + { + type: LoginFormMessageType.Info, + content: i18n.translate('xpack.security.login.sessionExpiredDescription', { + defaultMessage: 'Your session has timed out. Please log in again.', + }), + }, ], [ 'LOGGED_OUT', - i18n.translate('xpack.security.login.loggedOutDescription', { - defaultMessage: 'You have logged out of Elastic.', - }), + { + type: LoginFormMessageType.Info, + content: i18n.translate('xpack.security.login.loggedOutDescription', { + defaultMessage: 'You have logged out of Elastic.', + }), + }, + ], + [ + 'UNAUTHENTICATED', + { + type: LoginFormMessageType.Danger, + content: i18n.translate('xpack.security.unauthenticated.errorDescription', { + defaultMessage: + "We hit an authentication error. Please check your credentials and try again. If you still can't log in, contact your system administrator.", + }), + }, ], ]); @@ -226,7 +242,7 @@ export class LoginPage extends Component { notifications={this.props.notifications} selector={selector} // @ts-expect-error Map.get is ok with getting `undefined` - infoMessage={infoMessageMap.get(query[LOGOUT_REASON_QUERY_STRING_PARAMETER]?.toString())} + message={messageMap.get(query[LOGOUT_REASON_QUERY_STRING_PARAMETER]?.toString())} loginAssistanceMessage={this.props.loginAssistanceMessage} loginHelp={loginHelp} authProviderHint={query[AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER]?.toString()} diff --git a/x-pack/plugins/security/server/__snapshots__/prompt_page.test.tsx.snap b/x-pack/plugins/security/server/__snapshots__/prompt_page.test.tsx.snap new file mode 100644 index 0000000000000..bcb97538b4f05 --- /dev/null +++ b/x-pack/plugins/security/server/__snapshots__/prompt_page.test.tsx.snap @@ -0,0 +1,5 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`PromptPage renders as expected with additional scripts 1`] = `"ElasticMockedFonts

Some Title

Some Body
Action#1
Action#2
"`; + +exports[`PromptPage renders as expected without additional scripts 1`] = `"ElasticMockedFonts

Some Title

Some Body
Action#1
Action#2
"`; diff --git a/x-pack/plugins/security/server/authentication/__snapshots__/unauthenticated_page.test.tsx.snap b/x-pack/plugins/security/server/authentication/__snapshots__/unauthenticated_page.test.tsx.snap new file mode 100644 index 0000000000000..55168401992f7 --- /dev/null +++ b/x-pack/plugins/security/server/authentication/__snapshots__/unauthenticated_page.test.tsx.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`UnauthenticatedPage renders as expected 1`] = `"ElasticMockedFonts

We couldn't log you in

We hit an authentication error. Please check your credentials and try again. If you still can't log in, contact your system administrator.

"`; diff --git a/x-pack/plugins/security/server/authentication/authentication_service.test.mocks.ts b/x-pack/plugins/security/server/authentication/authentication_service.test.mocks.ts new file mode 100644 index 0000000000000..12a63134f4ef2 --- /dev/null +++ b/x-pack/plugins/security/server/authentication/authentication_service.test.mocks.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export const mockCanRedirectRequest = jest.fn(); +jest.mock('./can_redirect_request', () => ({ canRedirectRequest: mockCanRedirectRequest })); diff --git a/x-pack/plugins/security/server/authentication/authentication_service.test.ts b/x-pack/plugins/security/server/authentication/authentication_service.test.ts index b0be9445c3fc3..d38f963a60c33 100644 --- a/x-pack/plugins/security/server/authentication/authentication_service.test.ts +++ b/x-pack/plugins/security/server/authentication/authentication_service.test.ts @@ -6,6 +6,9 @@ */ jest.mock('./authenticator'); +jest.mock('./unauthenticated_page'); + +import { mockCanRedirectRequest } from './authentication_service.test.mocks'; import Boom from '@hapi/boom'; @@ -18,6 +21,7 @@ import type { KibanaRequest, Logger, LoggerFactory, + OnPreResponseToolkit, } from 'src/core/server'; import { coreMock, @@ -37,6 +41,7 @@ import type { ConfigType } from '../config'; import { ConfigSchema, createConfig } from '../config'; import type { SecurityFeatureUsageServiceStart } from '../feature_usage'; import { securityFeatureUsageServiceMock } from '../feature_usage/index.mock'; +import { ROUTE_TAG_AUTH_FLOW } from '../routes/tags'; import type { Session } from '../session_management'; import { sessionMock } from '../session_management/session.mock'; import { AuthenticationResult } from './authentication_result'; @@ -47,15 +52,60 @@ describe('AuthenticationService', () => { let logger: jest.Mocked; let mockSetupAuthenticationParams: { http: jest.Mocked; + config: ConfigType; license: jest.Mocked; + buildNumber: number; + }; + let mockStartAuthenticationParams: { + legacyAuditLogger: jest.Mocked; + audit: jest.Mocked; + config: ConfigType; + loggers: LoggerFactory; + http: jest.Mocked; + clusterClient: ReturnType; + featureUsageService: jest.Mocked; + session: jest.Mocked>; }; beforeEach(() => { logger = loggingSystemMock.createLogger(); + const httpMock = coreMock.createSetup().http; + (httpMock.basePath.prepend as jest.Mock).mockImplementation( + (path) => `${httpMock.basePath.serverBasePath}${path}` + ); + (httpMock.basePath.get as jest.Mock).mockImplementation(() => httpMock.basePath.serverBasePath); mockSetupAuthenticationParams = { - http: coreMock.createSetup().http, + http: httpMock, + config: createConfig(ConfigSchema.validate({}), loggingSystemMock.create().get(), { + isTLSEnabled: false, + }), license: licenseMock.create(), + buildNumber: 100500, }; + mockCanRedirectRequest.mockReturnValue(false); + + const coreStart = coreMock.createStart(); + mockStartAuthenticationParams = { + legacyAuditLogger: securityAuditLoggerMock.create(), + audit: auditServiceMock.create(), + config: createConfig( + ConfigSchema.validate({ + encryptionKey: 'ab'.repeat(16), + secureCookies: true, + cookieName: 'my-sid-cookie', + }), + loggingSystemMock.create().get(), + { isTLSEnabled: false } + ), + http: coreStart.http, + clusterClient: elasticsearchServiceMock.createClusterClient(), + loggers: loggingSystemMock.create(), + featureUsageService: securityFeatureUsageServiceMock.createStartContract(), + session: sessionMock.create(), + }; + (mockStartAuthenticationParams.http.basePath.get as jest.Mock).mockImplementation( + () => mockStartAuthenticationParams.http.basePath.serverBasePath + ); service = new AuthenticationService(logger); }); @@ -71,40 +121,19 @@ describe('AuthenticationService', () => { expect.any(Function) ); }); + + it('properly registers onPreResponse handler', () => { + service.setup(mockSetupAuthenticationParams); + + expect(mockSetupAuthenticationParams.http.registerOnPreResponse).toHaveBeenCalledTimes(1); + expect(mockSetupAuthenticationParams.http.registerOnPreResponse).toHaveBeenCalledWith( + expect.any(Function) + ); + }); }); describe('#start()', () => { - let mockStartAuthenticationParams: { - legacyAuditLogger: jest.Mocked; - audit: jest.Mocked; - config: ConfigType; - loggers: LoggerFactory; - http: jest.Mocked; - clusterClient: ReturnType; - featureUsageService: jest.Mocked; - session: jest.Mocked>; - }; beforeEach(() => { - const coreStart = coreMock.createStart(); - mockStartAuthenticationParams = { - legacyAuditLogger: securityAuditLoggerMock.create(), - audit: auditServiceMock.create(), - config: createConfig( - ConfigSchema.validate({ - encryptionKey: 'ab'.repeat(16), - secureCookies: true, - cookieName: 'my-sid-cookie', - }), - loggingSystemMock.create().get(), - { isTLSEnabled: false } - ), - http: coreStart.http, - clusterClient: elasticsearchServiceMock.createClusterClient(), - loggers: loggingSystemMock.create(), - featureUsageService: securityFeatureUsageServiceMock.createStartContract(), - session: sessionMock.create(), - }; - service.setup(mockSetupAuthenticationParams); }); @@ -318,4 +347,371 @@ describe('AuthenticationService', () => { }); }); }); + + describe('onPreResponse handler', () => { + function getService({ runStart = true }: { runStart?: boolean } = {}) { + service.setup(mockSetupAuthenticationParams); + + if (runStart) { + service.start(mockStartAuthenticationParams); + } + + const onPreResponseHandler = + mockSetupAuthenticationParams.http.registerOnPreResponse.mock.calls[0][0]; + const [authenticator] = jest.requireMock('./authenticator').Authenticator.mock.instances; + + return { authenticator, onPreResponseHandler }; + } + + it('ignores responses with non-401 status code', async () => { + const mockReturnedValue = { type: 'next' as any }; + const mockOnPreResponseToolkit = httpServiceMock.createOnPreResponseToolkit(); + mockOnPreResponseToolkit.next.mockReturnValue(mockReturnedValue); + + const { onPreResponseHandler } = getService(); + for (const statusCode of [200, 400, 403, 404]) { + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest(), + { statusCode }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + } + }); + + it('ignores responses to requests that cannot handle redirects', async () => { + const mockReturnedValue = { type: 'next' as any }; + const mockOnPreResponseToolkit = httpServiceMock.createOnPreResponseToolkit(); + mockOnPreResponseToolkit.next.mockReturnValue(mockReturnedValue); + mockCanRedirectRequest.mockReturnValue(false); + + const { onPreResponseHandler } = getService(); + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest(), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + }); + + it('ignores responses if authenticator is not initialized', async () => { + // Run `setup`, but not `start` to simulate non-initialized `Authenticator`. + const { onPreResponseHandler } = getService({ runStart: false }); + + const mockReturnedValue = { type: 'next' as any }; + const mockOnPreResponseToolkit = httpServiceMock.createOnPreResponseToolkit(); + mockOnPreResponseToolkit.next.mockReturnValue(mockReturnedValue); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest(), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + }); + + describe('when login form is available', () => { + let mockReturnedValue: { type: any; body: string }; + let mockOnPreResponseToolkit: jest.Mocked; + beforeEach(() => { + mockReturnedValue = { type: 'render' as any, body: 'body' }; + mockOnPreResponseToolkit = httpServiceMock.createOnPreResponseToolkit(); + mockOnPreResponseToolkit.render.mockReturnValue(mockReturnedValue); + }); + + it('redirects to the login page when user does not have an active session', async () => { + mockCanRedirectRequest.mockReturnValue(true); + + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ path: '/app/some', query: { param: 'one two' } }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: '
', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + Refresh: + '0;url=/mock-server-basepath/login?msg=UNAUTHENTICATED&next=%2Fmock-server-basepath%2Fapp%2Fsome', + }, + }); + }); + + it('performs logout if user has an active session', async () => { + mockStartAuthenticationParams.session.getSID.mockResolvedValue('some-sid'); + + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ path: '/app/some', query: { param: 'one two' } }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: '
', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + Refresh: + '0;url=/mock-server-basepath/logout?msg=UNAUTHENTICATED&next=%2Fmock-server-basepath%2Fapp%2Fsome', + }, + }); + }); + + it('does not preserve path for the authentication flow paths', async () => { + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ + path: '/api/security/saml/callback', + query: { param: 'one two' }, + routeTags: [ROUTE_TAG_AUTH_FLOW], + }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: '
', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + Refresh: + '0;url=/mock-server-basepath/login?msg=UNAUTHENTICATED&next=%2Fmock-server-basepath%2F', + }, + }); + }); + }); + + describe('when login selector is available', () => { + let mockReturnedValue: { type: any; body: string }; + let mockOnPreResponseToolkit: jest.Mocked; + beforeEach(() => { + mockReturnedValue = { type: 'render' as any, body: 'body' }; + mockOnPreResponseToolkit = httpServiceMock.createOnPreResponseToolkit(); + mockOnPreResponseToolkit.render.mockReturnValue(mockReturnedValue); + + mockSetupAuthenticationParams.config = createConfig( + ConfigSchema.validate({ + authc: { + providers: { + saml: { saml1: { order: 0, realm: 'saml1' } }, + basic: { basic1: { order: 1 } }, + }, + }, + }), + loggingSystemMock.create().get(), + { isTLSEnabled: false } + ); + }); + + it('redirects to the login page when user does not have an active session', async () => { + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ path: '/app/some', query: { param: 'one two' } }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: '
', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + Refresh: + '0;url=/mock-server-basepath/login?msg=UNAUTHENTICATED&next=%2Fmock-server-basepath%2Fapp%2Fsome', + }, + }); + }); + + it('performs logout if user has an active session', async () => { + mockStartAuthenticationParams.session.getSID.mockResolvedValue('some-sid'); + + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ path: '/app/some', query: { param: 'one two' } }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: '
', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + Refresh: + '0;url=/mock-server-basepath/logout?msg=UNAUTHENTICATED&next=%2Fmock-server-basepath%2Fapp%2Fsome', + }, + }); + }); + + it('does not preserve path for the authentication flow paths', async () => { + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ + path: '/api/security/saml/callback', + query: { param: 'one two' }, + routeTags: [ROUTE_TAG_AUTH_FLOW], + }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: '
', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + Refresh: + '0;url=/mock-server-basepath/login?msg=UNAUTHENTICATED&next=%2Fmock-server-basepath%2F', + }, + }); + }); + }); + + describe('when neither login selector nor login form is available', () => { + let mockReturnedValue: { type: any; body: string }; + let mockOnPreResponseToolkit: jest.Mocked; + beforeEach(() => { + mockReturnedValue = { type: 'render' as any, body: 'body' }; + mockOnPreResponseToolkit = httpServiceMock.createOnPreResponseToolkit(); + mockOnPreResponseToolkit.render.mockReturnValue(mockReturnedValue); + + mockSetupAuthenticationParams.config = createConfig( + ConfigSchema.validate({ + authc: { providers: { saml: { saml1: { order: 0, realm: 'saml1' } } } }, + }), + loggingSystemMock.create().get(), + { isTLSEnabled: false } + ); + }); + + it('renders unauthenticated page if user does not have an active session', async () => { + const mockRenderUnauthorizedPage = jest + .requireMock('./unauthenticated_page') + .renderUnauthenticatedPage.mockReturnValue('rendered-view'); + + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ path: '/app/some', query: { param: 'one two' } }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: 'rendered-view', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + }, + }); + + expect(mockRenderUnauthorizedPage).toHaveBeenCalledWith({ + basePath: mockSetupAuthenticationParams.http.basePath, + buildNumber: 100500, + originalURL: '/mock-server-basepath/app/some', + }); + }); + + it('renders unauthenticated page if user has an active session', async () => { + const mockRenderUnauthorizedPage = jest + .requireMock('./unauthenticated_page') + .renderUnauthenticatedPage.mockReturnValue('rendered-view'); + mockStartAuthenticationParams.session.getSID.mockResolvedValue('some-sid'); + + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ path: '/app/some', query: { param: 'one two' } }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: 'rendered-view', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + }, + }); + + expect(mockRenderUnauthorizedPage).toHaveBeenCalledWith({ + basePath: mockSetupAuthenticationParams.http.basePath, + buildNumber: 100500, + originalURL: '/mock-server-basepath/app/some', + }); + }); + + it('does not preserve path for the authentication flow paths', async () => { + const mockRenderUnauthorizedPage = jest + .requireMock('./unauthenticated_page') + .renderUnauthenticatedPage.mockReturnValue('rendered-view'); + + const { authenticator, onPreResponseHandler } = getService(); + authenticator.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/app/some'); + mockCanRedirectRequest.mockReturnValue(true); + + await expect( + onPreResponseHandler( + httpServerMock.createKibanaRequest({ + path: '/api/security/saml/callback', + query: { param: 'one two' }, + routeTags: [ROUTE_TAG_AUTH_FLOW], + }), + { statusCode: 401 }, + mockOnPreResponseToolkit + ) + ).resolves.toBe(mockReturnedValue); + + expect(mockOnPreResponseToolkit.render).toHaveBeenCalledWith({ + body: 'rendered-view', + headers: { + 'Content-Security-Policy': `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`, + }, + }); + + expect(mockRenderUnauthorizedPage).toHaveBeenCalledWith({ + basePath: mockSetupAuthenticationParams.http.basePath, + buildNumber: 100500, + originalURL: '/mock-server-basepath/', + }); + }); + }); + }); }); diff --git a/x-pack/plugins/security/server/authentication/authentication_service.ts b/x-pack/plugins/security/server/authentication/authentication_service.ts index 7feeff7a5d8ed..e5895422e7a74 100644 --- a/x-pack/plugins/security/server/authentication/authentication_service.ts +++ b/x-pack/plugins/security/server/authentication/authentication_service.ts @@ -15,22 +15,29 @@ import type { LoggerFactory, } from 'src/core/server'; +import { NEXT_URL_QUERY_STRING_PARAMETER } from '../../common/constants'; import type { SecurityLicense } from '../../common/licensing'; import type { AuthenticatedUser } from '../../common/model'; +import { shouldProviderUseLoginForm } from '../../common/model'; import type { AuditServiceSetup, SecurityAuditLogger } from '../audit'; import type { ConfigType } from '../config'; -import { getErrorStatusCode } from '../errors'; +import { getDetailedErrorMessage, getErrorStatusCode } from '../errors'; import type { SecurityFeatureUsageServiceStart } from '../feature_usage'; +import { ROUTE_TAG_AUTH_FLOW } from '../routes/tags'; import type { Session } from '../session_management'; import { APIKeys } from './api_keys'; import type { AuthenticationResult } from './authentication_result'; import type { ProviderLoginAttempt } from './authenticator'; import { Authenticator } from './authenticator'; +import { canRedirectRequest } from './can_redirect_request'; import type { DeauthenticationResult } from './deauthentication_result'; +import { renderUnauthenticatedPage } from './unauthenticated_page'; interface AuthenticationServiceSetupParams { - http: Pick; + http: Pick; + config: ConfigType; license: SecurityLicense; + buildNumber: number; } interface AuthenticationServiceStartParams { @@ -62,12 +69,23 @@ export interface AuthenticationServiceStart { export class AuthenticationService { private license!: SecurityLicense; private authenticator?: Authenticator; + private session?: PublicMethodsOf; constructor(private readonly logger: Logger) {} - setup({ http, license }: AuthenticationServiceSetupParams) { + setup({ config, http, license, buildNumber }: AuthenticationServiceSetupParams) { this.license = license; + // If we cannot automatically authenticate users we should redirect them straight to the login + // page if possible, so that they can try other methods to log in. If not possible, we should + // render a dedicated `Unauthenticated` page from which users can explicitly trigger a new + // login attempt. There are two cases when we can redirect to the login page: + // 1. Login selector is enabled + // 2. Login selector is disabled, but the provider with the lowest `order` uses login form + const isLoginPageAvailable = + config.authc.selector.enabled || + shouldProviderUseLoginForm(config.authc.sortedProviders[0].type); + http.registerAuth(async (request, response, t) => { if (!license.isLicenseAvailable()) { this.logger.error('License is not available, authentication is not possible.'); @@ -118,8 +136,9 @@ export class AuthenticationService { } if (authenticationResult.failed()) { - this.logger.info(`Authentication attempt failed: ${authenticationResult.error!.message}`); const error = authenticationResult.error!; + this.logger.info(`Authentication attempt failed: ${getDetailedErrorMessage(error)}`); + // proxy Elasticsearch "native" errors const statusCode = getErrorStatusCode(error); if (typeof statusCode === 'number') { @@ -139,7 +158,49 @@ export class AuthenticationService { return t.notHandled(); }); - this.logger.debug('Successfully registered core authentication handler.'); + http.registerOnPreResponse(async (request, preResponse, toolkit) => { + if (preResponse.statusCode !== 401 || !canRedirectRequest(request)) { + return toolkit.next(); + } + + if (!this.authenticator) { + // Core doesn't allow returning error here. + this.logger.error('Authentication sub-system is not fully initialized yet.'); + return toolkit.next(); + } + + // If users can eventually re-login we want to redirect them directly to the page they tried + // to access initially, but we only want to do that for routes that aren't part of the various + // authentication flows that wouldn't make any sense after successful authentication. + const originalURL = !request.route.options.tags.includes(ROUTE_TAG_AUTH_FLOW) + ? this.authenticator.getRequestOriginalURL(request) + : `${http.basePath.get(request)}/`; + if (!isLoginPageAvailable) { + return toolkit.render({ + body: renderUnauthenticatedPage({ buildNumber, basePath: http.basePath, originalURL }), + headers: { 'Content-Security-Policy': http.csp.header }, + }); + } + + const needsToLogout = (await this.session?.getSID(request)) !== undefined; + if (needsToLogout) { + this.logger.warn('Could not authenticate user with the existing session. Forcing logout.'); + } + + return toolkit.render({ + body: '
', + headers: { + 'Content-Security-Policy': http.csp.header, + Refresh: `0;url=${http.basePath.prepend( + `${ + needsToLogout ? '/logout' : '/login' + }?msg=UNAUTHENTICATED&${NEXT_URL_QUERY_STRING_PARAMETER}=${encodeURIComponent( + originalURL + )}` + )}`, + }, + }); + }); } start({ @@ -161,6 +222,7 @@ export class AuthenticationService { const getCurrentUser = (request: KibanaRequest) => http.auth.get(request).state ?? null; + this.session = session; this.authenticator = new Authenticator({ legacyAuditLogger, audit, diff --git a/x-pack/plugins/security/server/authentication/authenticator.test.ts b/x-pack/plugins/security/server/authentication/authenticator.test.ts index 1bd430d0c5c98..ca33be92e9e99 100644 --- a/x-pack/plugins/security/server/authentication/authenticator.test.ts +++ b/x-pack/plugins/security/server/authentication/authenticator.test.ts @@ -20,6 +20,10 @@ import { loggingSystemMock, } from 'src/core/server/mocks'; +import { + AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + AUTH_URL_HASH_QUERY_STRING_PARAMETER, +} from '../../common/constants'; import type { SecurityLicenseFeatures } from '../../common/licensing'; import { licenseMock } from '../../common/licensing/index.mock'; import { mockAuthenticatedUser } from '../../common/model/authenticated_user.mock'; @@ -1780,13 +1784,13 @@ describe('Authenticator', () => { ); }); - it('returns `notHandled` if session does not exist.', async () => { + it('redirects to login form if session does not exist.', async () => { const request = httpServerMock.createKibanaRequest(); mockOptions.session.get.mockResolvedValue(null); mockBasicAuthenticationProvider.logout.mockResolvedValue(DeauthenticationResult.notHandled()); await expect(authenticator.logout(request)).resolves.toEqual( - DeauthenticationResult.notHandled() + DeauthenticationResult.redirectTo('/mock-server-basepath/login?msg=LOGGED_OUT') ); expect(mockOptions.session.invalidate).not.toHaveBeenCalled(); @@ -1843,12 +1847,12 @@ describe('Authenticator', () => { expect(mockOptions.session.invalidate).not.toHaveBeenCalled(); }); - it('returns `notHandled` if session does not exist and provider name is invalid', async () => { + it('redirects to login form if session does not exist and provider name is invalid', async () => { const request = httpServerMock.createKibanaRequest({ query: { provider: 'foo' } }); mockOptions.session.get.mockResolvedValue(null); await expect(authenticator.logout(request)).resolves.toEqual( - DeauthenticationResult.notHandled() + DeauthenticationResult.redirectTo('/mock-server-basepath/login?msg=LOGGED_OUT') ); expect(mockBasicAuthenticationProvider.logout).not.toHaveBeenCalled(); @@ -1937,4 +1941,64 @@ describe('Authenticator', () => { ); }); }); + + describe('`getRequestOriginalURL` method', () => { + let authenticator: Authenticator; + let mockOptions: ReturnType; + beforeEach(() => { + mockOptions = getMockOptions({ providers: { basic: { basic1: { order: 0 } } } }); + authenticator = new Authenticator(mockOptions); + }); + + it('filters out auth specific query parameters', () => { + expect(authenticator.getRequestOriginalURL(httpServerMock.createKibanaRequest())).toBe( + '/mock-server-basepath/path' + ); + + expect( + authenticator.getRequestOriginalURL( + httpServerMock.createKibanaRequest({ + query: { + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER]: 'saml1', + }, + }) + ) + ).toBe('/mock-server-basepath/path'); + + expect( + authenticator.getRequestOriginalURL( + httpServerMock.createKibanaRequest({ + query: { + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER]: 'saml1', + [AUTH_URL_HASH_QUERY_STRING_PARAMETER]: '#some-hash', + }, + }) + ) + ).toBe('/mock-server-basepath/path'); + }); + + it('allows to include additional query parameters', () => { + expect( + authenticator.getRequestOriginalURL(httpServerMock.createKibanaRequest(), [ + ['some-param', 'some-value'], + ['some-param2', 'some-value2'], + ]) + ).toBe('/mock-server-basepath/path?some-param=some-value&some-param2=some-value2'); + + expect( + authenticator.getRequestOriginalURL( + httpServerMock.createKibanaRequest({ + query: { + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER]: 'saml1', + [AUTH_URL_HASH_QUERY_STRING_PARAMETER]: '#some-hash', + }, + }), + [ + ['some-param', 'some-value'], + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, 'oidc1'], + ] + ) + ).toBe('/mock-server-basepath/path?some-param=some-value&auth_provider_hint=oidc1'); + }); + }); }); diff --git a/x-pack/plugins/security/server/authentication/authenticator.ts b/x-pack/plugins/security/server/authentication/authenticator.ts index f86ff54963da9..4eeadf23c50b2 100644 --- a/x-pack/plugins/security/server/authentication/authenticator.ts +++ b/x-pack/plugins/security/server/authentication/authenticator.ts @@ -11,6 +11,7 @@ import type { IBasePath, IClusterClient, LoggerFactory } from 'src/core/server'; import { KibanaRequest } from '../../../../../src/core/server'; import { AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + AUTH_URL_HASH_QUERY_STRING_PARAMETER, LOGOUT_PROVIDER_QUERY_STRING_PARAMETER, LOGOUT_REASON_QUERY_STRING_PARAMETER, NEXT_URL_QUERY_STRING_PARAMETER, @@ -45,6 +46,15 @@ import { } from './providers'; import { Tokens } from './tokens'; +/** + * List of query string parameters used to pass various authentication related metadata that should + * be stripped away from URL as soon as they are no longer needed. + */ +const AUTH_METADATA_QUERY_STRING_PARAMETERS = [ + AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + AUTH_URL_HASH_QUERY_STRING_PARAMETER, +]; + /** * The shape of the login attempt. */ @@ -201,6 +211,7 @@ export class Authenticator { const providerCommonOptions = { client: this.options.clusterClient, basePath: this.options.basePath, + getRequestOriginalURL: this.getRequestOriginalURL.bind(this), tokens: new Tokens({ client: this.options.clusterClient.asInternalUser, logger: this.options.loggers.get('tokens'), @@ -419,7 +430,9 @@ export class Authenticator { } } - return DeauthenticationResult.notHandled(); + // If none of the configured providers could perform a logout, we should redirect user to the + // default logout location. + return DeauthenticationResult.redirectTo(this.getLoggedOutURL(request)); } /** @@ -452,6 +465,24 @@ export class Authenticator { this.options.featureUsageService.recordPreAccessAgreementUsage(); } + getRequestOriginalURL( + request: KibanaRequest, + additionalQueryStringParameters?: Array<[string, string]> + ) { + const originalURLSearchParams = [ + ...[...request.url.searchParams.entries()].filter( + ([key]) => !AUTH_METADATA_QUERY_STRING_PARAMETERS.includes(key) + ), + ...(additionalQueryStringParameters ?? []), + ]; + + return `${this.options.basePath.get(request)}${request.url.pathname}${ + originalURLSearchParams.length > 0 + ? `?${new URLSearchParams(originalURLSearchParams).toString()}` + : '' + }`; + } + /** * Initializes HTTP Authentication provider and appends it to the end of the list of enabled * authentication providers. @@ -762,9 +793,13 @@ export class Authenticator { /** * Creates a logged out URL for the specified request and provider. * @param request Request that initiated logout. - * @param providerType Type of the provider that handles logout. + * @param providerType Type of the provider that handles logout. If not specified, then the first + * provider in the chain (default) is assumed. */ - private getLoggedOutURL(request: KibanaRequest, providerType: string) { + private getLoggedOutURL( + request: KibanaRequest, + providerType: string = this.options.config.authc.sortedProviders[0].type + ) { // The app that handles logout needs to know the reason of the logout and the URL we may need to // redirect user to once they log in again (e.g. when session expires). const searchParams = new URLSearchParams(); diff --git a/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts b/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts index 1507cd2d3a50a..805d647757ca5 100644 --- a/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts +++ b/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts @@ -7,6 +7,7 @@ import { httpServerMock } from 'src/core/server/mocks'; +import { ROUTE_TAG_API, ROUTE_TAG_CAN_REDIRECT } from '../routes/tags'; import { canRedirectRequest } from './can_redirect_request'; describe('can_redirect_request', () => { @@ -24,4 +25,33 @@ describe('can_redirect_request', () => { expect(canRedirectRequest(request)).toBe(false); }); + + it('returns false for api routes', () => { + expect( + canRedirectRequest(httpServerMock.createKibanaRequest({ path: '/api/security/some' })) + ).toBe(false); + }); + + it('returns false for internal routes', () => { + expect( + canRedirectRequest(httpServerMock.createKibanaRequest({ path: '/internal/security/some' })) + ).toBe(false); + }); + + it('returns true for the routes with the `security:canRedirect` tag', () => { + for (const request of [ + httpServerMock.createKibanaRequest({ routeTags: [ROUTE_TAG_CAN_REDIRECT] }), + httpServerMock.createKibanaRequest({ routeTags: [ROUTE_TAG_API, ROUTE_TAG_CAN_REDIRECT] }), + httpServerMock.createKibanaRequest({ + path: '/api/security/some', + routeTags: [ROUTE_TAG_CAN_REDIRECT], + }), + httpServerMock.createKibanaRequest({ + path: '/internal/security/some', + routeTags: [ROUTE_TAG_CAN_REDIRECT], + }), + ]) { + expect(canRedirectRequest(request)).toBe(true); + } + }); }); diff --git a/x-pack/plugins/security/server/authentication/can_redirect_request.ts b/x-pack/plugins/security/server/authentication/can_redirect_request.ts index 71c6365d9aea4..5a3a09f17eb86 100644 --- a/x-pack/plugins/security/server/authentication/can_redirect_request.ts +++ b/x-pack/plugins/security/server/authentication/can_redirect_request.ts @@ -7,7 +7,8 @@ import type { KibanaRequest } from 'src/core/server'; -const ROUTE_TAG_API = 'api'; +import { ROUTE_TAG_API, ROUTE_TAG_CAN_REDIRECT } from '../routes/tags'; + const KIBANA_XSRF_HEADER = 'kbn-xsrf'; const KIBANA_VERSION_HEADER = 'kbn-version'; @@ -24,9 +25,9 @@ export function canRedirectRequest(request: KibanaRequest) { const isApiRoute = route.options.tags.includes(ROUTE_TAG_API) || - (route.path.startsWith('/api/') && route.path !== '/api/security/logout') || + route.path.startsWith('/api/') || route.path.startsWith('/internal/'); const isAjaxRequest = hasVersionHeader || hasXsrfHeader; - return !isApiRoute && !isAjaxRequest; + return !isAjaxRequest && (!isApiRoute || route.options.tags.includes(ROUTE_TAG_CAN_REDIRECT)); } diff --git a/x-pack/plugins/security/server/authentication/providers/base.mock.ts b/x-pack/plugins/security/server/authentication/providers/base.mock.ts index bb78b6e963763..5d3417ae9db11 100644 --- a/x-pack/plugins/security/server/authentication/providers/base.mock.ts +++ b/x-pack/plugins/security/server/authentication/providers/base.mock.ts @@ -20,6 +20,7 @@ export function mockAuthenticationProviderOptions(options?: { name: string }) { client: elasticsearchServiceMock.createClusterClient(), logger: loggingSystemMock.create().get(), basePath: httpServiceMock.createBasePath(), + getRequestOriginalURL: jest.fn(), tokens: { refresh: jest.fn(), invalidate: jest.fn() }, name: options?.name ?? 'basic1', urls: { diff --git a/x-pack/plugins/security/server/authentication/providers/base.ts b/x-pack/plugins/security/server/authentication/providers/base.ts index 18d567a143fee..c7c0edcf1e9e1 100644 --- a/x-pack/plugins/security/server/authentication/providers/base.ts +++ b/x-pack/plugins/security/server/authentication/providers/base.ts @@ -27,6 +27,10 @@ import type { Tokens } from '../tokens'; export interface AuthenticationProviderOptions { name: string; basePath: HttpServiceSetup['basePath']; + getRequestOriginalURL: ( + request: KibanaRequest, + additionalQueryStringParameters?: Array<[string, string]> + ) => string; client: IClusterClient; logger: Logger; tokens: PublicMethodsOf; diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts index ebeca42682eb9..444a7f3e50a25 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts @@ -11,6 +11,10 @@ import Boom from '@hapi/boom'; import type { KibanaRequest } from 'src/core/server'; import { elasticsearchServiceMock, httpServerMock } from 'src/core/server/mocks'; +import { + AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + AUTH_URL_HASH_QUERY_STRING_PARAMETER, +} from '../../../common/constants'; import { mockAuthenticatedUser } from '../../../common/model/authenticated_user.mock'; import { securityMock } from '../../mocks'; import { AuthenticationResult } from '../authentication_result'; @@ -376,18 +380,78 @@ describe('OIDCAuthenticationProvider', () => { }); it('redirects non-AJAX request that can not be authenticated to the "capture URL" page.', async () => { + mockOptions.getRequestOriginalURL.mockReturnValue( + '/mock-server-basepath/s/foo/some-path?auth_provider_hint=oidc' + ); const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path' }); await expect(provider.authenticate(request, null)).resolves.toEqual( AuthenticationResult.redirectTo( - '/mock-server-basepath/internal/security/capture-url?next=%2Fmock-server-basepath%2Fs%2Ffoo%2Fsome-path&providerType=oidc&providerName=oidc', + '/mock-server-basepath/internal/security/capture-url?next=%2Fmock-server-basepath%2Fs%2Ffoo%2Fsome-path%3Fauth_provider_hint%3Doidc', { state: null } ) ); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledTimes(1); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledWith(request, [ + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, 'oidc'], + ]); + expect(mockOptions.client.asInternalUser.transport.request).not.toHaveBeenCalled(); }); + it('initiates OIDC handshake for non-AJAX request that can not be authenticated, but includes URL hash fragment.', async () => { + mockOptions.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/s/foo/some-path'); + mockOptions.client.asInternalUser.transport.request.mockResolvedValue( + securityMock.createApiResponse({ + body: { + state: 'statevalue', + nonce: 'noncevalue', + redirect: + 'https://op-host/path/login?response_type=code' + + '&scope=openid%20profile%20email' + + '&client_id=s6BhdRkqt3' + + '&state=statevalue' + + '&redirect_uri=https%3A%2F%2Ftest-hostname:1234%2Ftest-base-path%2Fapi%2Fsecurity%2Fv1%2F/oidc' + + '&login_hint=loginhint', + }, + }) + ); + + const request = httpServerMock.createKibanaRequest({ + path: '/s/foo/some-path', + query: { [AUTH_URL_HASH_QUERY_STRING_PARAMETER]: '#some-fragment' }, + }); + await expect(provider.authenticate(request)).resolves.toEqual( + AuthenticationResult.redirectTo( + 'https://op-host/path/login?response_type=code' + + '&scope=openid%20profile%20email' + + '&client_id=s6BhdRkqt3' + + '&state=statevalue' + + '&redirect_uri=https%3A%2F%2Ftest-hostname:1234%2Ftest-base-path%2Fapi%2Fsecurity%2Fv1%2F/oidc' + + '&login_hint=loginhint', + { + state: { + state: 'statevalue', + nonce: 'noncevalue', + redirectURL: '/mock-server-basepath/s/foo/some-path#some-fragment', + realm: 'oidc1', + }, + } + ) + ); + + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledTimes(1); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledWith(request); + + expect(mockOptions.client.asInternalUser.transport.request).toHaveBeenCalledTimes(1); + expect(mockOptions.client.asInternalUser.transport.request).toHaveBeenCalledWith({ + method: 'POST', + path: '/_security/oidc/prepare', + body: { realm: 'oidc1' }, + }); + }); + it('succeeds if state contains a valid token.', async () => { const request = httpServerMock.createKibanaRequest({ headers: {} }); const tokenPair = { @@ -520,6 +584,9 @@ describe('OIDCAuthenticationProvider', () => { }); it('redirects non-AJAX requests to the "capture URL" page if refresh token is expired or already refreshed.', async () => { + mockOptions.getRequestOriginalURL.mockReturnValue( + '/mock-server-basepath/s/foo/some-path?auth_provider_hint=oidc' + ); const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path', headers: {} }); const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; const authorization = `Bearer ${tokenPair.accessToken}`; @@ -534,11 +601,16 @@ describe('OIDCAuthenticationProvider', () => { provider.authenticate(request, { ...tokenPair, realm: 'oidc1' }) ).resolves.toEqual( AuthenticationResult.redirectTo( - '/mock-server-basepath/internal/security/capture-url?next=%2Fmock-server-basepath%2Fs%2Ffoo%2Fsome-path&providerType=oidc&providerName=oidc', + '/mock-server-basepath/internal/security/capture-url?next=%2Fmock-server-basepath%2Fs%2Ffoo%2Fsome-path%3Fauth_provider_hint%3Doidc', { state: null } ) ); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledTimes(1); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledWith(request, [ + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, 'oidc'], + ]); + expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(1); expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(tokenPair.refreshToken); diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.ts b/x-pack/plugins/security/server/authentication/providers/oidc.ts index 2afa49fe6e082..83f0ec50abb0d 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.ts @@ -10,8 +10,13 @@ import type from 'type-detect'; import type { KibanaRequest } from 'src/core/server'; -import { NEXT_URL_QUERY_STRING_PARAMETER } from '../../../common/constants'; +import { + AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + AUTH_URL_HASH_QUERY_STRING_PARAMETER, + NEXT_URL_QUERY_STRING_PARAMETER, +} from '../../../common/constants'; import type { AuthenticationInfo } from '../../elasticsearch'; +import { getDetailedErrorMessage } from '../../errors'; import { AuthenticationResult } from '../authentication_result'; import { canRedirectRequest } from '../can_redirect_request'; import { DeauthenticationResult } from '../deauthentication_result'; @@ -201,7 +206,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { // We might already have a state and nonce generated by Elasticsearch (from an unfinished authentication in // another tab) return authenticationResult.notHandled() && canStartNewSession(request) - ? await this.captureRedirectURL(request) + ? await this.initiateAuthenticationHandshake(request) : authenticationResult; } @@ -264,7 +269,9 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { }) ).body as any; } catch (err) { - this.logger.debug(`Failed to authenticate request via OpenID Connect: ${err.message}`); + this.logger.debug( + `Failed to authenticate request via OpenID Connect: ${getDetailedErrorMessage(err)}` + ); return AuthenticationResult.failed(err); } @@ -313,7 +320,9 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { { state: { state, nonce, redirectURL, realm: this.realm } } ); } catch (err) { - this.logger.debug(`Failed to initiate OpenID Connect authentication: ${err.message}`); + this.logger.debug( + `Failed to initiate OpenID Connect authentication: ${getDetailedErrorMessage(err)}` + ); return AuthenticationResult.failed(err); } } @@ -341,7 +350,9 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { this.logger.debug('Request has been authenticated via state.'); return AuthenticationResult.succeeded(user, { authHeaders }); } catch (err) { - this.logger.debug(`Failed to authenticate request via state: ${err.message}`); + this.logger.debug( + `Failed to authenticate request via state: ${getDetailedErrorMessage(err)}` + ); return AuthenticationResult.failed(err); } } @@ -379,7 +390,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { this.logger.debug( 'Both elasticsearch access and refresh tokens are expired. Re-initiating OpenID Connect authentication.' ); - return this.captureRedirectURL(request); + return this.initiateAuthenticationHandshake(request); } return AuthenticationResult.failed( @@ -440,7 +451,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { return DeauthenticationResult.redirectTo(redirect); } } catch (err) { - this.logger.debug(`Failed to deauthenticate user: ${err.message}`); + this.logger.debug(`Failed to deauthenticate user: ${getDetailedErrorMessage(err)}`); return DeauthenticationResult.failed(err); } } @@ -457,22 +468,29 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { } /** - * Tries to capture full redirect URL (both path and fragment) and initiate OIDC handshake. + * Tries to initiate OIDC authentication handshake. If the request already includes user URL hash fragment, we will + * initiate handshake right away, otherwise we'll redirect user to a dedicated page where we capture URL hash fragment + * first and only then initiate SAML handshake. * @param request Request instance. */ - private captureRedirectURL(request: KibanaRequest) { - const searchParams = new URLSearchParams([ - [ - NEXT_URL_QUERY_STRING_PARAMETER, - `${this.options.basePath.get(request)}${request.url.pathname}${request.url.search}`, - ], - ['providerType', this.type], - ['providerName', this.options.name], - ]); + private initiateAuthenticationHandshake(request: KibanaRequest) { + const originalURLHash = request.url.searchParams.get(AUTH_URL_HASH_QUERY_STRING_PARAMETER); + if (originalURLHash != null) { + return this.initiateOIDCAuthentication( + request, + { realm: this.realm }, + `${this.options.getRequestOriginalURL(request)}${originalURLHash}` + ); + } + return AuthenticationResult.redirectTo( `${ this.options.basePath.serverBasePath - }/internal/security/capture-url?${searchParams.toString()}`, + }/internal/security/capture-url?${NEXT_URL_QUERY_STRING_PARAMETER}=${encodeURIComponent( + this.options.getRequestOriginalURL(request, [ + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, this.options.name], + ]) + )}`, // Here we indicate that current session, if any, should be invalidated. It is a no-op for the // initial handshake, but is essential when both access and refresh tokens are expired. { state: null } 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 bd51a0f815329..dfcdb66e61c35 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.test.ts @@ -10,6 +10,10 @@ import Boom from '@hapi/boom'; import { elasticsearchServiceMock, httpServerMock } from 'src/core/server/mocks'; +import { + AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + AUTH_URL_HASH_QUERY_STRING_PARAMETER, +} from '../../../common/constants'; import { mockAuthenticatedUser } from '../../../common/model/authenticated_user.mock'; import { securityMock } from '../../mocks'; import { AuthenticationResult } from '../authentication_result'; @@ -848,18 +852,63 @@ describe('SAMLAuthenticationProvider', () => { }); it('redirects non-AJAX request that can not be authenticated to the "capture URL" page.', async () => { + mockOptions.getRequestOriginalURL.mockReturnValue( + '/mock-server-basepath/s/foo/some-path?auth_provider_hint=saml' + ); const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path' }); - await expect(provider.authenticate(request)).resolves.toEqual( AuthenticationResult.redirectTo( - '/mock-server-basepath/internal/security/capture-url?next=%2Fmock-server-basepath%2Fs%2Ffoo%2Fsome-path&providerType=saml&providerName=saml', + '/mock-server-basepath/internal/security/capture-url?next=%2Fmock-server-basepath%2Fs%2Ffoo%2Fsome-path%3Fauth_provider_hint%3Dsaml', { state: null } ) ); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledTimes(1); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledWith(request, [ + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, 'saml'], + ]); + expect(mockOptions.client.asInternalUser.transport.request).not.toHaveBeenCalled(); }); + it('initiates SAML handshake for non-AJAX request that can not be authenticated, but includes URL hash fragment.', async () => { + mockOptions.getRequestOriginalURL.mockReturnValue('/mock-server-basepath/s/foo/some-path'); + mockOptions.client.asInternalUser.transport.request.mockResolvedValue( + securityMock.createApiResponse({ + body: { + id: 'some-request-id', + redirect: 'https://idp-host/path/login?SAMLRequest=some%20request%20', + }, + }) + ); + + const request = httpServerMock.createKibanaRequest({ + path: '/s/foo/some-path', + query: { [AUTH_URL_HASH_QUERY_STRING_PARAMETER]: '#some-fragment' }, + }); + await expect(provider.authenticate(request)).resolves.toEqual( + AuthenticationResult.redirectTo( + 'https://idp-host/path/login?SAMLRequest=some%20request%20', + { + state: { + requestId: 'some-request-id', + redirectURL: '/mock-server-basepath/s/foo/some-path#some-fragment', + realm: 'test-realm', + }, + } + ) + ); + + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledTimes(1); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledWith(request); + + expect(mockOptions.client.asInternalUser.transport.request).toHaveBeenCalledWith({ + method: 'POST', + path: '/_security/saml/prepare', + body: { realm: 'test-realm' }, + }); + }); + it('succeeds if state contains a valid token.', async () => { const request = httpServerMock.createKibanaRequest({ headers: {} }); const state = { @@ -1024,6 +1073,9 @@ describe('SAMLAuthenticationProvider', () => { }); it('re-capture URL for non-AJAX requests if refresh token is expired.', async () => { + mockOptions.getRequestOriginalURL.mockReturnValue( + '/mock-server-basepath/s/foo/some-path?auth_provider_hint=saml' + ); const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path', headers: {} }); const state = { accessToken: 'expired-token', @@ -1040,11 +1092,16 @@ describe('SAMLAuthenticationProvider', () => { await expect(provider.authenticate(request, state)).resolves.toEqual( AuthenticationResult.redirectTo( - '/mock-server-basepath/internal/security/capture-url?next=%2Fmock-server-basepath%2Fs%2Ffoo%2Fsome-path&providerType=saml&providerName=saml', + '/mock-server-basepath/internal/security/capture-url?next=%2Fmock-server-basepath%2Fs%2Ffoo%2Fsome-path%3Fauth_provider_hint%3Dsaml', { state: null } ) ); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledTimes(1); + expect(mockOptions.getRequestOriginalURL).toHaveBeenCalledWith(request, [ + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, 'saml'], + ]); + expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(1); expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(state.refreshToken); diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts index 7c27e2ebeff10..ea818e5df6e12 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.ts @@ -9,9 +9,14 @@ import Boom from '@hapi/boom'; import type { KibanaRequest } from 'src/core/server'; -import { NEXT_URL_QUERY_STRING_PARAMETER } from '../../../common/constants'; +import { + AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + AUTH_URL_HASH_QUERY_STRING_PARAMETER, + NEXT_URL_QUERY_STRING_PARAMETER, +} from '../../../common/constants'; import { isInternalURL } from '../../../common/is_internal_url'; import type { AuthenticationInfo } from '../../elasticsearch'; +import { getDetailedErrorMessage } from '../../errors'; import { AuthenticationResult } from '../authentication_result'; import { canRedirectRequest } from '../can_redirect_request'; import { DeauthenticationResult } from '../deauthentication_result'; @@ -185,7 +190,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { } else { this.logger.debug( `Failed to perform a login: ${ - authenticationResult.error && authenticationResult.error.message + authenticationResult.error && getDetailedErrorMessage(authenticationResult.error) }` ); } @@ -230,7 +235,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { // 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() && canStartNewSession(request) - ? this.captureRedirectURL(request) + ? this.initiateAuthenticationHandshake(request) : authenticationResult; } @@ -283,7 +288,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { return DeauthenticationResult.redirectTo(redirect); } } catch (err) { - this.logger.debug(`Failed to deauthenticate user: ${err.message}`); + this.logger.debug(`Failed to deauthenticate user: ${getDetailedErrorMessage(err)}`); return DeauthenticationResult.failed(err); } } @@ -362,7 +367,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { }) ).body as any; } catch (err) { - this.logger.debug(`Failed to log in with SAML response: ${err.message}`); + this.logger.debug(`Failed to log in with SAML response: ${getDetailedErrorMessage(err)}`); // Since we don't know upfront what realm is targeted by the Identity Provider initiated login // there is a chance that it failed because of realm mismatch and hence we should return @@ -452,7 +457,9 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { refreshToken: existingState.refreshToken!, }); } catch (err) { - this.logger.debug(`Failed to perform IdP initiated local logout: ${err.message}`); + this.logger.debug( + `Failed to perform IdP initiated local logout: ${getDetailedErrorMessage(err)}` + ); return AuthenticationResult.failed(err); } @@ -483,7 +490,9 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { this.logger.debug('Request has been authenticated via state.'); return AuthenticationResult.succeeded(user, { authHeaders }); } catch (err) { - this.logger.debug(`Failed to authenticate request via state: ${err.message}`); + this.logger.debug( + `Failed to authenticate request via state: ${getDetailedErrorMessage(err)}` + ); return AuthenticationResult.failed(err); } } @@ -520,7 +529,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { this.logger.debug( 'Both access and refresh tokens are expired. Capturing redirect URL and re-initiating SAML handshake.' ); - return this.captureRedirectURL(request); + return this.initiateAuthenticationHandshake(request); } return AuthenticationResult.failed( @@ -569,7 +578,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { state: { requestId, redirectURL, realm: this.realm }, }); } catch (err) { - this.logger.debug(`Failed to initiate SAML handshake: ${err.message}`); + this.logger.debug(`Failed to initiate SAML handshake: ${getDetailedErrorMessage(err)}`); return AuthenticationResult.failed(err); } } @@ -629,22 +638,28 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { } /** - * Tries to capture full redirect URL (both path and fragment) and initiate SAML handshake. + * Tries to initiate SAML authentication handshake. If the request already includes user URL hash fragment, we will + * initiate handshake right away, otherwise we'll redirect user to a dedicated page where we capture URL hash fragment + * first and only then initiate SAML handshake. * @param request Request instance. */ - private captureRedirectURL(request: KibanaRequest) { - const searchParams = new URLSearchParams([ - [ - NEXT_URL_QUERY_STRING_PARAMETER, - `${this.options.basePath.get(request)}${request.url.pathname}${request.url.search}`, - ], - ['providerType', this.type], - ['providerName', this.options.name], - ]); + private initiateAuthenticationHandshake(request: KibanaRequest) { + const originalURLHash = request.url.searchParams.get(AUTH_URL_HASH_QUERY_STRING_PARAMETER); + if (originalURLHash != null) { + return this.authenticateViaHandshake( + request, + `${this.options.getRequestOriginalURL(request)}${originalURLHash}` + ); + } + return AuthenticationResult.redirectTo( `${ this.options.basePath.serverBasePath - }/internal/security/capture-url?${searchParams.toString()}`, + }/internal/security/capture-url?${NEXT_URL_QUERY_STRING_PARAMETER}=${encodeURIComponent( + this.options.getRequestOriginalURL(request, [ + [AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, this.options.name], + ]) + )}`, // Here we indicate that current session, if any, should be invalidated. It is a no-op for the // initial handshake, but is essential when both access and refresh tokens are expired. { state: null } diff --git a/x-pack/plugins/security/server/authentication/tokens.ts b/x-pack/plugins/security/server/authentication/tokens.ts index 8f6dd9275e59c..1adbb2dc66533 100644 --- a/x-pack/plugins/security/server/authentication/tokens.ts +++ b/x-pack/plugins/security/server/authentication/tokens.ts @@ -8,7 +8,7 @@ import type { ElasticsearchClient, Logger } from 'src/core/server'; import type { AuthenticationInfo } from '../elasticsearch'; -import { getErrorStatusCode } from '../errors'; +import { getDetailedErrorMessage, getErrorStatusCode } from '../errors'; /** * Represents a pair of access and refresh tokens. @@ -73,11 +73,11 @@ export class Tokens { return { accessToken, refreshToken, - // @ts-expect-error @elastic/elasticsearch decalred GetUserAccessTokenResponse.authentication: string + // @ts-expect-error @elastic/elasticsearch declared GetUserAccessTokenResponse.authentication: string authenticationInfo: authenticationInfo as AuthenticationInfo, }; } catch (err) { - this.logger.debug(`Failed to refresh access token: ${err.message}`); + this.logger.debug(`Failed to refresh access token: ${getDetailedErrorMessage(err)}`); // There are at least two common cases when refresh token request can fail: // 1. Refresh token is valid only for 24 hours and if it hasn't been used it expires. @@ -123,7 +123,7 @@ export class Tokens { }) ).body.invalidated_tokens; } catch (err) { - this.logger.debug(`Failed to invalidate refresh token: ${err.message}`); + this.logger.debug(`Failed to invalidate refresh token: ${getDetailedErrorMessage(err)}`); // When using already deleted refresh token, Elasticsearch responds with 404 and a body that // shows that no tokens were invalidated. @@ -155,7 +155,7 @@ export class Tokens { }) ).body.invalidated_tokens; } catch (err) { - this.logger.debug(`Failed to invalidate access token: ${err.message}`); + this.logger.debug(`Failed to invalidate access token: ${getDetailedErrorMessage(err)}`); // When using already deleted access token, Elasticsearch responds with 404 and a body that // shows that no tokens were invalidated. diff --git a/x-pack/plugins/security/server/authentication/unauthenticated_page.test.tsx b/x-pack/plugins/security/server/authentication/unauthenticated_page.test.tsx new file mode 100644 index 0000000000000..5cb6c899d7560 --- /dev/null +++ b/x-pack/plugins/security/server/authentication/unauthenticated_page.test.tsx @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; + +import { coreMock } from '../../../../../src/core/server/mocks'; +import { UnauthenticatedPage } from './unauthenticated_page'; + +jest.mock('src/core/server/rendering/views/fonts', () => ({ + Fonts: () => <>MockedFonts, +})); + +describe('UnauthenticatedPage', () => { + it('renders as expected', async () => { + const mockCoreSetup = coreMock.createSetup(); + (mockCoreSetup.http.basePath.prepend as jest.Mock).mockImplementation( + (path) => `/mock-basepath${path}` + ); + + const body = renderToStaticMarkup( + + ); + + expect(body).toMatchSnapshot(); + }); +}); diff --git a/x-pack/plugins/security/server/authentication/unauthenticated_page.tsx b/x-pack/plugins/security/server/authentication/unauthenticated_page.tsx new file mode 100644 index 0000000000000..48d61a72e085d --- /dev/null +++ b/x-pack/plugins/security/server/authentication/unauthenticated_page.tsx @@ -0,0 +1,55 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +// @ts-expect-error no definitions in component folder +import { EuiButton } from '@elastic/eui/lib/components/button'; +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; + +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; +import type { IBasePath } from 'src/core/server'; + +import { PromptPage } from '../prompt_page'; + +interface Props { + originalURL: string; + buildNumber: number; + basePath: IBasePath; +} + +export function UnauthenticatedPage({ basePath, originalURL, buildNumber }: Props) { + return ( + + +

+ } + actions={[ + + + , + ]} + /> + ); +} + +export function renderUnauthenticatedPage(props: Props) { + return renderToStaticMarkup(); +} diff --git a/x-pack/plugins/security/server/authorization/__snapshots__/reset_session_page.test.tsx.snap b/x-pack/plugins/security/server/authorization/__snapshots__/reset_session_page.test.tsx.snap index 785c57490e8ef..1011d82eb1f73 100644 --- a/x-pack/plugins/security/server/authorization/__snapshots__/reset_session_page.test.tsx.snap +++ b/x-pack/plugins/security/server/authorization/__snapshots__/reset_session_page.test.tsx.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ResetSessionPage renders as expected 1`] = `"MockedFonts

You do not have permission to access the requested page

Either go back to the previous page or log in as a different user.

"`; +exports[`ResetSessionPage renders as expected 1`] = `"ElasticMockedFonts

You do not have permission to access the requested page

Either go back to the previous page or log in as a different user.

"`; diff --git a/x-pack/plugins/security/server/authorization/authorization_service.tsx b/x-pack/plugins/security/server/authorization/authorization_service.tsx index db3c84477ffb1..144a8bc5fd0c4 100644 --- a/x-pack/plugins/security/server/authorization/authorization_service.tsx +++ b/x-pack/plugins/security/server/authorization/authorization_service.tsx @@ -10,7 +10,6 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import type { Observable, Subscription } from 'rxjs'; -import * as UiSharedDeps from '@kbn/ui-shared-deps'; import type { CapabilitiesSetup, HttpServiceSetup, @@ -163,25 +162,14 @@ export class AuthorizationService { http.registerOnPreResponse((request, preResponse, toolkit) => { if (preResponse.statusCode === 403 && canRedirectRequest(request)) { - const basePath = http.basePath.get(request); - const next = `${basePath}${request.url.pathname}${request.url.search}`; - const regularBundlePath = `${basePath}/${buildNumber}/bundles`; - - const logoutUrl = http.basePath.prepend( - `/api/security/logout?${querystring.stringify({ next })}` - ); - const styleSheetPaths = [ - `${regularBundlePath}/kbn-ui-shared-deps/${UiSharedDeps.baseCssDistFilename}`, - `${regularBundlePath}/kbn-ui-shared-deps/${UiSharedDeps.lightCssDistFilename}`, - `${basePath}/node_modules/@kbn/ui-framework/dist/kui_light.css`, - `${basePath}/ui/legacy_light_theme.css`, - ]; - + const next = `${http.basePath.get(request)}${request.url.pathname}${request.url.search}`; const body = renderToStaticMarkup( ); diff --git a/x-pack/plugins/security/server/authorization/reset_session_page.test.tsx b/x-pack/plugins/security/server/authorization/reset_session_page.test.tsx index e76c8ff138fcb..d5e27c9d39ffd 100644 --- a/x-pack/plugins/security/server/authorization/reset_session_page.test.tsx +++ b/x-pack/plugins/security/server/authorization/reset_session_page.test.tsx @@ -8,6 +8,7 @@ import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; +import { coreMock } from '../../../../../src/core/server/mocks'; import { ResetSessionPage } from './reset_session_page'; jest.mock('src/core/server/rendering/views/fonts', () => ({ @@ -16,11 +17,16 @@ jest.mock('src/core/server/rendering/views/fonts', () => ({ describe('ResetSessionPage', () => { it('renders as expected', async () => { + const mockCoreSetup = coreMock.createSetup(); + (mockCoreSetup.http.basePath.prepend as jest.Mock).mockImplementation( + (path) => `/mock-basepath${path}` + ); + const body = renderToStaticMarkup( ); diff --git a/x-pack/plugins/security/server/authorization/reset_session_page.tsx b/x-pack/plugins/security/server/authorization/reset_session_page.tsx index c2d43cd3dd030..4e2e6f4631287 100644 --- a/x-pack/plugins/security/server/authorization/reset_session_page.tsx +++ b/x-pack/plugins/security/server/authorization/reset_session_page.tsx @@ -7,101 +7,53 @@ // @ts-expect-error no definitions in component folder import { EuiButton, EuiButtonEmpty } from '@elastic/eui/lib/components/button'; -// @ts-expect-error no definitions in component folder -import { EuiEmptyPrompt } from '@elastic/eui/lib/components/empty_prompt'; -// @ts-expect-error no definitions in component folder -import { icon as EuiIconAlert } from '@elastic/eui/lib/components/icon/assets/alert'; -// @ts-expect-error no definitions in component folder -import { appendIconComponentCache } from '@elastic/eui/lib/components/icon/icon'; -// @ts-expect-error no definitions in component folder -import { EuiPage, EuiPageBody, EuiPageContent } from '@elastic/eui/lib/components/page'; import React from 'react'; import { i18n } from '@kbn/i18n'; -import { FormattedMessage, I18nProvider } from '@kbn/i18n/react'; - -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { Fonts } from '../../../../../src/core/server/rendering/views/fonts'; +import { FormattedMessage } from '@kbn/i18n/react'; +import type { IBasePath } from 'src/core/server'; -// Preload the alert icon used by `EuiEmptyPrompt` to ensure that it's loaded -// in advance the first time this page is rendered server-side. If not, the -// icon svg wouldn't contain any paths the first time the page was rendered. -appendIconComponentCache({ - alert: EuiIconAlert, -}); +import { PromptPage } from '../prompt_page'; export function ResetSessionPage({ logoutUrl, - styleSheetPaths, + buildNumber, basePath, }: { logoutUrl: string; - styleSheetPaths: string[]; - basePath: string; + buildNumber: number; + basePath: IBasePath; }) { - const uiPublicUrl = `${basePath}/ui`; return ( - - - {styleSheetPaths.map((path) => ( - - ))} - - {/* The alternate icon is a fallback for Safari which does not yet support SVG favicons */} - - - - - - - - - - - - - - } - body={ -

- -

- } - actions={[ - - - , - - - , - ]} - /> -
-
-
-
- - + + +

+ } + actions={[ + + + , + + + , + ]} + /> ); } diff --git a/x-pack/plugins/security/server/index.ts b/x-pack/plugins/security/server/index.ts index b66ed6e9eb7ca..087cf8f4f8ee8 100644 --- a/x-pack/plugins/security/server/index.ts +++ b/x-pack/plugins/security/server/index.ts @@ -30,6 +30,7 @@ export type { CheckPrivilegesPayload } from './authorization'; export { LegacyAuditLogger, AuditLogger, AuditEvent } from './audit'; export type { SecurityPluginSetup, SecurityPluginStart }; export type { AuthenticatedUser } from '../common/model'; +export { ROUTE_TAG_CAN_REDIRECT } from './routes/tags'; export const config: PluginConfigDescriptor> = { schema: ConfigSchema, diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts index 586707dd8c9aa..57be308525fdd 100644 --- a/x-pack/plugins/security/server/plugin.ts +++ b/x-pack/plugins/security/server/plugin.ts @@ -246,7 +246,12 @@ export class SecurityPlugin this.elasticsearchService.setup({ license, status: core.status }); this.featureUsageService.setup({ featureUsage: licensing.featureUsage }); this.sessionManagementService.setup({ config, http: core.http, taskManager }); - this.authenticationService.setup({ http: core.http, license }); + this.authenticationService.setup({ + http: core.http, + config, + license, + buildNumber: this.initializerContext.env.packageInfo.buildNum, + }); registerSecurityUsageCollector({ usageCollection, config, license }); diff --git a/x-pack/plugins/security/server/prompt_page.test.tsx b/x-pack/plugins/security/server/prompt_page.test.tsx new file mode 100644 index 0000000000000..01c4488576f57 --- /dev/null +++ b/x-pack/plugins/security/server/prompt_page.test.tsx @@ -0,0 +1,57 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; + +import { coreMock } from '../../../../src/core/server/mocks'; +import { PromptPage } from './prompt_page'; + +jest.mock('src/core/server/rendering/views/fonts', () => ({ + Fonts: () => <>MockedFonts, +})); + +describe('PromptPage', () => { + it('renders as expected without additional scripts', async () => { + const mockCoreSetup = coreMock.createSetup(); + (mockCoreSetup.http.basePath.prepend as jest.Mock).mockImplementation( + (path) => `/mock-basepath${path}` + ); + + const body = renderToStaticMarkup( + Some Body
} + actions={[Action#1, Action#2]} + /> + ); + + expect(body).toMatchSnapshot(); + }); + + it('renders as expected with additional scripts', async () => { + const mockCoreSetup = coreMock.createSetup(); + (mockCoreSetup.http.basePath.prepend as jest.Mock).mockImplementation( + (path) => `/mock-basepath${path}` + ); + + const body = renderToStaticMarkup( + Some Body
} + actions={[Action#1, Action#2]} + /> + ); + + expect(body).toMatchSnapshot(); + }); +}); diff --git a/x-pack/plugins/security/server/prompt_page.tsx b/x-pack/plugins/security/server/prompt_page.tsx new file mode 100644 index 0000000000000..338d39b29e534 --- /dev/null +++ b/x-pack/plugins/security/server/prompt_page.tsx @@ -0,0 +1,96 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +// @ts-expect-error no definitions in component folder +import { EuiEmptyPrompt } from '@elastic/eui/lib/components/empty_prompt'; +// @ts-expect-error no definitions in component folder +import { icon as EuiIconAlert } from '@elastic/eui/lib/components/icon/assets/alert'; +// @ts-expect-error no definitions in component folder +import { appendIconComponentCache } from '@elastic/eui/lib/components/icon/icon'; +// @ts-expect-error no definitions in component folder +import { EuiPage, EuiPageBody, EuiPageContent } from '@elastic/eui/lib/components/page'; +import type { ReactNode } from 'react'; +import React from 'react'; + +import { i18n } from '@kbn/i18n'; +import { I18nProvider } from '@kbn/i18n/react'; +import * as UiSharedDeps from '@kbn/ui-shared-deps'; +import type { IBasePath } from 'src/core/server'; + +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { Fonts } from '../../../../src/core/server/rendering/views/fonts'; + +// Preload the alert icon used by `EuiEmptyPrompt` to ensure that it's loaded +// in advance the first time this page is rendered server-side. If not, the +// icon svg wouldn't contain any paths the first time the page was rendered. +appendIconComponentCache({ + alert: EuiIconAlert, +}); + +interface Props { + buildNumber: number; + basePath: IBasePath; + scriptPaths?: string[]; + title: ReactNode; + body: ReactNode; + actions: ReactNode; +} + +export function PromptPage({ + basePath, + buildNumber, + scriptPaths = [], + title, + body, + actions, +}: Props) { + const uiPublicURL = `${basePath.serverBasePath}/ui`; + const regularBundlePath = `${basePath.serverBasePath}/${buildNumber}/bundles`; + const styleSheetPaths = [ + `${regularBundlePath}/kbn-ui-shared-deps/${UiSharedDeps.baseCssDistFilename}`, + `${regularBundlePath}/kbn-ui-shared-deps/${UiSharedDeps.lightCssDistFilename}`, + `${basePath.serverBasePath}/node_modules/@kbn/ui-framework/dist/kui_light.css`, + `${basePath.serverBasePath}/ui/legacy_light_theme.css`, + ]; + + return ( + + + Elastic + {styleSheetPaths.map((path) => ( + + ))} + + {/* The alternate icon is a fallback for Safari which does not yet support SVG favicons */} + + + {scriptPaths.map((path) => ( +