From 4b7e8680de10ef92da541a6acfc5f2a9aadedafa Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Wed, 2 Dec 2020 13:55:15 +0100 Subject: [PATCH] [7.x] Make all providers to preserve original URL when session expires. (#84743) --- x-pack/plugins/security/common/constants.ts | 3 + .../common/model/authenticated_user.ts | 3 +- .../model/authentication_provider.test.ts | 21 ++++ .../common/model/authentication_provider.ts | 21 ++++ x-pack/plugins/security/common/model/index.ts | 1 + x-pack/plugins/security/common/parse_next.ts | 12 ++- x-pack/plugins/security/common/types.ts | 8 +- .../logged_out/logged_out_page.test.tsx | 39 +++++++ .../logged_out/logged_out_page.tsx | 3 +- .../authentication/login/login_page.tsx | 7 +- .../public/session/session_expired.ts | 16 ++- .../server/audit/security_audit_logger.ts | 2 +- .../authentication/authenticator.test.ts | 101 +++++++++++++----- .../server/authentication/authenticator.ts | 71 ++++++++---- .../authentication/providers/anonymous.ts | 2 +- .../authentication/providers/base.mock.ts | 2 +- .../server/authentication/providers/base.ts | 2 +- .../authentication/providers/basic.test.ts | 27 ++--- .../server/authentication/providers/basic.ts | 10 +- .../authentication/providers/kerberos.test.ts | 4 +- .../authentication/providers/kerberos.ts | 2 +- .../authentication/providers/oidc.test.ts | 6 +- .../server/authentication/providers/oidc.ts | 17 +-- .../authentication/providers/pki.test.ts | 4 +- .../server/authentication/providers/pki.ts | 2 +- .../authentication/providers/saml.test.ts | 18 ++-- .../server/authentication/providers/saml.ts | 17 +-- .../authentication/providers/token.test.ts | 28 ++--- .../server/authentication/providers/token.ts | 10 +- x-pack/plugins/security/server/config.ts | 2 +- .../routes/views/access_agreement.test.ts | 2 +- .../security/server/routes/views/login.ts | 11 +- .../server/session_management/session.ts | 2 +- .../session_management/session_index.ts | 2 +- .../tests/anonymous/login.ts | 2 +- .../tests/kerberos/kerberos_login.ts | 2 +- .../login_selector/basic_functionality.ts | 2 +- .../tests/pki/pki_auth.ts | 2 +- .../tests/session_idle/cleanup.ts | 2 +- .../tests/session_lifespan/cleanup.ts | 2 +- 40 files changed, 322 insertions(+), 168 deletions(-) create mode 100644 x-pack/plugins/security/common/model/authentication_provider.test.ts create mode 100644 x-pack/plugins/security/common/model/authentication_provider.ts create mode 100644 x-pack/plugins/security/public/authentication/logged_out/logged_out_page.test.tsx diff --git a/x-pack/plugins/security/common/constants.ts b/x-pack/plugins/security/common/constants.ts index 07e6ab6c72cb9..f53b5ca6d56ca 100644 --- a/x-pack/plugins/security/common/constants.ts +++ b/x-pack/plugins/security/common/constants.ts @@ -19,3 +19,6 @@ export const APPLICATION_PREFIX = 'kibana-'; export const RESERVED_PRIVILEGES_APPLICATION_WILDCARD = 'kibana-*'; export const AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER = 'auth_provider_hint'; +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/common/model/authenticated_user.ts b/x-pack/plugins/security/common/model/authenticated_user.ts index c22c5fc4ef0da..491ceb6845e28 100644 --- a/x-pack/plugins/security/common/model/authenticated_user.ts +++ b/x-pack/plugins/security/common/model/authenticated_user.ts @@ -4,8 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import type { AuthenticationProvider } from '../types'; -import { User } from './user'; +import type { AuthenticationProvider, User } from '.'; const REALMS_ELIGIBLE_FOR_PASSWORD_CHANGE = ['reserved', 'native']; diff --git a/x-pack/plugins/security/common/model/authentication_provider.test.ts b/x-pack/plugins/security/common/model/authentication_provider.test.ts new file mode 100644 index 0000000000000..fc32d3108be08 --- /dev/null +++ b/x-pack/plugins/security/common/model/authentication_provider.test.ts @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { shouldProviderUseLoginForm } from './authentication_provider'; + +describe('#shouldProviderUseLoginForm', () => { + ['basic', 'token'].forEach((providerType) => { + it(`returns "true" for "${providerType}" provider`, () => { + expect(shouldProviderUseLoginForm(providerType)).toEqual(true); + }); + }); + + ['anonymous', 'http', 'kerberos', 'oidc', 'pki', 'saml'].forEach((providerType) => { + it(`returns "false" for "${providerType}" provider`, () => { + expect(shouldProviderUseLoginForm(providerType)).toEqual(false); + }); + }); +}); diff --git a/x-pack/plugins/security/common/model/authentication_provider.ts b/x-pack/plugins/security/common/model/authentication_provider.ts new file mode 100644 index 0000000000000..1b34fbc9da29a --- /dev/null +++ b/x-pack/plugins/security/common/model/authentication_provider.ts @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +/** + * Type and name tuple to identify provider used to authenticate user. + */ +export interface AuthenticationProvider { + type: string; + name: string; +} + +/** + * Checks whether authentication provider with the specified type uses Kibana's native login form. + * @param providerType Type of the authentication provider. + */ +export function shouldProviderUseLoginForm(providerType: string) { + return providerType === 'basic' || providerType === 'token'; +} diff --git a/x-pack/plugins/security/common/model/index.ts b/x-pack/plugins/security/common/model/index.ts index 59d4908c67ffb..ee1dcffd4a794 100644 --- a/x-pack/plugins/security/common/model/index.ts +++ b/x-pack/plugins/security/common/model/index.ts @@ -7,6 +7,7 @@ export { ApiKey, ApiKeyToInvalidate } from './api_key'; export { User, EditUser, getUserDisplayName } from './user'; export { AuthenticatedUser, canUserChangePassword } from './authenticated_user'; +export { AuthenticationProvider, shouldProviderUseLoginForm } from './authentication_provider'; export { BuiltinESPrivileges } from './builtin_es_privileges'; export { RawKibanaPrivileges, RawKibanaFeaturePrivileges } from './raw_kibana_privileges'; export { FeaturesPrivileges } from './features_privileges'; diff --git a/x-pack/plugins/security/common/parse_next.ts b/x-pack/plugins/security/common/parse_next.ts index 7ce0de05ad526..68903baeb05b8 100644 --- a/x-pack/plugins/security/common/parse_next.ts +++ b/x-pack/plugins/security/common/parse_next.ts @@ -5,19 +5,21 @@ */ import { parse } from 'url'; +import { NEXT_URL_QUERY_STRING_PARAMETER } from './constants'; import { isInternalURL } from './is_internal_url'; export function parseNext(href: string, basePath = '') { const { query, hash } = parse(href, true); - if (!query.next) { + + let next = query[NEXT_URL_QUERY_STRING_PARAMETER]; + if (!next) { return `${basePath}/`; } - let next: string; - if (Array.isArray(query.next) && query.next.length > 0) { - next = query.next[0]; + if (Array.isArray(next) && next.length > 0) { + next = next[0]; } else { - next = query.next as string; + next = next as string; } // validate that `next` is not attempting a redirect to somewhere diff --git a/x-pack/plugins/security/common/types.ts b/x-pack/plugins/security/common/types.ts index c668c6ccf71d1..33e2875acefef 100644 --- a/x-pack/plugins/security/common/types.ts +++ b/x-pack/plugins/security/common/types.ts @@ -4,13 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -/** - * Type and name tuple to identify provider used to authenticate user. - */ -export interface AuthenticationProvider { - type: string; - name: string; -} +import type { AuthenticationProvider } from './model'; export interface SessionInfo { now: number; diff --git a/x-pack/plugins/security/public/authentication/logged_out/logged_out_page.test.tsx b/x-pack/plugins/security/public/authentication/logged_out/logged_out_page.test.tsx new file mode 100644 index 0000000000000..89d622e086b38 --- /dev/null +++ b/x-pack/plugins/security/public/authentication/logged_out/logged_out_page.test.tsx @@ -0,0 +1,39 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { EuiButton } from '@elastic/eui'; +import { mountWithIntl } from '@kbn/test/jest'; +import { LoggedOutPage } from './logged_out_page'; + +import { coreMock } from '../../../../../../src/core/public/mocks'; + +describe('LoggedOutPage', () => { + beforeAll(() => { + Object.defineProperty(window, 'location', { + value: { href: 'https://some-host' }, + writable: true, + }); + }); + + it('points to a base path if `next` parameter is not provided', async () => { + const basePathMock = coreMock.createStart({ basePath: '/mock-base-path' }).http.basePath; + const wrapper = mountWithIntl(); + + expect(wrapper.find(EuiButton).prop('href')).toBe('/mock-base-path/'); + }); + + it('properly parses `next` parameter', async () => { + window.location.href = `https://host.com/mock-base-path/security/logged_out?next=${encodeURIComponent( + '/mock-base-path/app/home#/?_g=()' + )}`; + + const basePathMock = coreMock.createStart({ basePath: '/mock-base-path' }).http.basePath; + const wrapper = mountWithIntl(); + + expect(wrapper.find(EuiButton).prop('href')).toBe('/mock-base-path/app/home#/?_g=()'); + }); +}); diff --git a/x-pack/plugins/security/public/authentication/logged_out/logged_out_page.tsx b/x-pack/plugins/security/public/authentication/logged_out/logged_out_page.tsx index a708931c3fa95..5498b8ef3644c 100644 --- a/x-pack/plugins/security/public/authentication/logged_out/logged_out_page.tsx +++ b/x-pack/plugins/security/public/authentication/logged_out/logged_out_page.tsx @@ -9,6 +9,7 @@ import ReactDOM from 'react-dom'; import { EuiButton } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; import { CoreStart, IBasePath } from 'src/core/public'; +import { parseNext } from '../../../common/parse_next'; import { AuthenticationStatePage } from '../components'; interface Props { @@ -25,7 +26,7 @@ export function LoggedOutPage({ basePath }: Props) { /> } > - + 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 0646962684284..35703212762fd 100644 --- a/x-pack/plugins/security/public/authentication/login/login_page.tsx +++ b/x-pack/plugins/security/public/authentication/login/login_page.tsx @@ -15,7 +15,10 @@ import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiSpacer, EuiTitle } from '@elasti import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { CoreStart, FatalErrorsStart, HttpStart, NotificationsStart } from 'src/core/public'; -import { AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER } from '../../../common/constants'; +import { + AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + LOGOUT_REASON_QUERY_STRING_PARAMETER, +} from '../../../common/constants'; import { LoginState } from '../../../common/login_state'; import { LoginForm, DisabledLoginForm } from './components'; @@ -219,7 +222,7 @@ export class LoginPage extends Component { http={this.props.http} notifications={this.props.notifications} selector={selector} - infoMessage={infoMessageMap.get(query.msg?.toString())} + infoMessage={infoMessageMap.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/public/session/session_expired.ts b/x-pack/plugins/security/public/session/session_expired.ts index 5866526b8851e..52ba37c242d09 100644 --- a/x-pack/plugins/security/public/session/session_expired.ts +++ b/x-pack/plugins/security/public/session/session_expired.ts @@ -4,6 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ +import { + LOGOUT_PROVIDER_QUERY_STRING_PARAMETER, + LOGOUT_REASON_QUERY_STRING_PARAMETER, + NEXT_URL_QUERY_STRING_PARAMETER, +} from '../../common/constants'; + export interface ISessionExpired { logout(): void; } @@ -11,13 +17,15 @@ export interface ISessionExpired { const getNextParameter = () => { const { location } = window; const next = encodeURIComponent(`${location.pathname}${location.search}${location.hash}`); - return `&next=${next}`; + return `&${NEXT_URL_QUERY_STRING_PARAMETER}=${next}`; }; const getProviderParameter = (tenant: string) => { const key = `${tenant}/session_provider`; const providerName = sessionStorage.getItem(key); - return providerName ? `&provider=${encodeURIComponent(providerName)}` : ''; + return providerName + ? `&${LOGOUT_PROVIDER_QUERY_STRING_PARAMETER}=${encodeURIComponent(providerName)}` + : ''; }; export class SessionExpired { @@ -26,6 +34,8 @@ export class SessionExpired { logout() { const next = getNextParameter(); const provider = getProviderParameter(this.tenant); - window.location.assign(`${this.logoutUrl}?msg=SESSION_EXPIRED${next}${provider}`); + window.location.assign( + `${this.logoutUrl}?${LOGOUT_REASON_QUERY_STRING_PARAMETER}=SESSION_EXPIRED${next}${provider}` + ); } } diff --git a/x-pack/plugins/security/server/audit/security_audit_logger.ts b/x-pack/plugins/security/server/audit/security_audit_logger.ts index ee81f5f330f44..c0d431b3c2fa2 100644 --- a/x-pack/plugins/security/server/audit/security_audit_logger.ts +++ b/x-pack/plugins/security/server/audit/security_audit_logger.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { AuthenticationProvider } from '../../common/types'; +import type { AuthenticationProvider } from '../../common/model'; import { LegacyAuditLogger } from './audit_service'; /** diff --git a/x-pack/plugins/security/server/authentication/authenticator.test.ts b/x-pack/plugins/security/server/authentication/authenticator.test.ts index e954cc204ea94..a7bb462d8f83a 100644 --- a/x-pack/plugins/security/server/authentication/authenticator.test.ts +++ b/x-pack/plugins/security/server/authentication/authenticator.test.ts @@ -112,31 +112,78 @@ describe('Authenticator', () => { ).toThrowError('Provider name "__http__" is reserved.'); }); - it('properly sets `loggedOut` URL.', () => { - const basicAuthenticationProviderMock = jest.requireMock('./providers/basic') - .BasicAuthenticationProvider; + describe('#options.urls.loggedOut', () => { + it('points to /login if provider requires login form', () => { + const authenticationProviderMock = jest.requireMock(`./providers/basic`) + .BasicAuthenticationProvider; + authenticationProviderMock.mockClear(); + new Authenticator(getMockOptions()); + const getLoggedOutURL = authenticationProviderMock.mock.calls[0][0].urls.loggedOut; - basicAuthenticationProviderMock.mockClear(); - new Authenticator(getMockOptions()); - expect(basicAuthenticationProviderMock).toHaveBeenCalledWith( - expect.objectContaining({ - urls: { - loggedOut: '/mock-server-basepath/security/logged_out', - }, - }), - expect.anything() - ); + expect(getLoggedOutURL(httpServerMock.createKibanaRequest())).toBe( + '/mock-server-basepath/login?msg=LOGGED_OUT' + ); - basicAuthenticationProviderMock.mockClear(); - new Authenticator(getMockOptions({ selector: { enabled: true } })); - expect(basicAuthenticationProviderMock).toHaveBeenCalledWith( - expect.objectContaining({ - urls: { - loggedOut: `/mock-server-basepath/login?msg=LOGGED_OUT`, - }, - }), - expect.anything() - ); + expect( + getLoggedOutURL( + httpServerMock.createKibanaRequest({ + query: { next: '/app/ml/encode me', msg: 'SESSION_EXPIRED' }, + }) + ) + ).toBe('/mock-server-basepath/login?next=%2Fapp%2Fml%2Fencode+me&msg=SESSION_EXPIRED'); + }); + + it('points to /login if login selector is enabled', () => { + const authenticationProviderMock = jest.requireMock(`./providers/saml`) + .SAMLAuthenticationProvider; + authenticationProviderMock.mockClear(); + new Authenticator( + getMockOptions({ + selector: { enabled: true }, + providers: { saml: { saml1: { order: 0, realm: 'realm' } } }, + }) + ); + const getLoggedOutURL = authenticationProviderMock.mock.calls[0][0].urls.loggedOut; + + expect(getLoggedOutURL(httpServerMock.createKibanaRequest())).toBe( + '/mock-server-basepath/login?msg=LOGGED_OUT' + ); + + expect( + getLoggedOutURL( + httpServerMock.createKibanaRequest({ + query: { next: '/app/ml/encode me', msg: 'SESSION_EXPIRED' }, + }) + ) + ).toBe('/mock-server-basepath/login?next=%2Fapp%2Fml%2Fencode+me&msg=SESSION_EXPIRED'); + }); + + it('points to /security/logged_out if login selector is NOT enabled', () => { + const authenticationProviderMock = jest.requireMock(`./providers/saml`) + .SAMLAuthenticationProvider; + authenticationProviderMock.mockClear(); + new Authenticator( + getMockOptions({ + selector: { enabled: false }, + providers: { saml: { saml1: { order: 0, realm: 'realm' } } }, + }) + ); + const getLoggedOutURL = authenticationProviderMock.mock.calls[0][0].urls.loggedOut; + + expect(getLoggedOutURL(httpServerMock.createKibanaRequest())).toBe( + '/mock-server-basepath/security/logged_out?msg=LOGGED_OUT' + ); + + expect( + getLoggedOutURL( + httpServerMock.createKibanaRequest({ + query: { next: '/app/ml/encode me', msg: 'SESSION_EXPIRED' }, + }) + ) + ).toBe( + '/mock-server-basepath/security/logged_out?next=%2Fapp%2Fml%2Fencode+me&msg=SESSION_EXPIRED' + ); + }); }); describe('HTTP authentication provider', () => { @@ -1770,7 +1817,9 @@ describe('Authenticator', () => { }); it('if session does not exist but provider name is valid, returns whatever authentication provider returns.', async () => { - const request = httpServerMock.createKibanaRequest({ query: { provider: 'basic1' } }); + const request = httpServerMock.createKibanaRequest({ + query: { provider: 'basic1' }, + }); mockOptions.session.get.mockResolvedValue(null); mockBasicAuthenticationProvider.logout.mockResolvedValue( @@ -1783,7 +1832,7 @@ describe('Authenticator', () => { expect(mockBasicAuthenticationProvider.logout).toHaveBeenCalledTimes(1); expect(mockBasicAuthenticationProvider.logout).toHaveBeenCalledWith(request, null); - expect(mockOptions.session.clear).not.toHaveBeenCalled(); + expect(mockOptions.session.clear).toHaveBeenCalled(); }); it('if session does not exist and provider name is not available, returns whatever authentication provider returns.', async () => { @@ -1812,7 +1861,7 @@ describe('Authenticator', () => { ); expect(mockBasicAuthenticationProvider.logout).not.toHaveBeenCalled(); - expect(mockOptions.session.clear).not.toHaveBeenCalled(); + expect(mockOptions.session.clear).toHaveBeenCalled(); }); }); diff --git a/x-pack/plugins/security/server/authentication/authenticator.ts b/x-pack/plugins/security/server/authentication/authenticator.ts index 28122fcd2c567..ccdf7f87526b1 100644 --- a/x-pack/plugins/security/server/authentication/authenticator.ts +++ b/x-pack/plugins/security/server/authentication/authenticator.ts @@ -10,10 +10,15 @@ import { ILegacyClusterClient, IBasePath, } from '../../../../../src/core/server'; -import { AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER } from '../../common/constants'; +import { + AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER, + LOGOUT_PROVIDER_QUERY_STRING_PARAMETER, + LOGOUT_REASON_QUERY_STRING_PARAMETER, + NEXT_URL_QUERY_STRING_PARAMETER, +} from '../../common/constants'; import type { SecurityLicense } from '../../common/licensing'; -import type { AuthenticatedUser } from '../../common/model'; -import type { AuthenticationProvider } from '../../common/types'; +import type { AuthenticatedUser, AuthenticationProvider } from '../../common/model'; +import { shouldProviderUseLoginForm } from '../../common/model'; import { SecurityAuditLogger, AuditServiceSetup, userLoginEvent } from '../audit'; import type { ConfigType } from '../config'; import { getErrorStatusCode } from '../errors'; @@ -201,11 +206,6 @@ export class Authenticator { logger: this.options.loggers.get('tokens'), }), getServerBaseURL: this.options.getServerBaseURL, - urls: { - loggedOut: options.config.authc.selector.enabled - ? `${options.basePath.serverBasePath}/login?msg=LOGGED_OUT` - : `${options.basePath.serverBasePath}/security/logged_out`, - }, }; this.providers = new Map( @@ -220,6 +220,7 @@ export class Authenticator { ...providerCommonOptions, name, logger: options.loggers.get(type, name), + urls: { loggedOut: (request) => this.getLoggedOutURL(request, type) }, }), this.options.config.authc.providers[type]?.[name] ), @@ -234,6 +235,9 @@ export class Authenticator { ...providerCommonOptions, name: '__http__', logger: options.loggers.get(HTTPAuthenticationProvider.type), + urls: { + loggedOut: (request) => this.getLoggedOutURL(request, HTTPAuthenticationProvider.type), + }, }) ); } @@ -340,7 +344,9 @@ export class Authenticator { if (this.shouldRedirectToLoginSelector(request, existingSessionValue)) { this.logger.debug('Redirecting request to Login Selector.'); return AuthenticationResult.redirectTo( - `${this.options.basePath.serverBasePath}/login?next=${encodeURIComponent( + `${ + this.options.basePath.serverBasePath + }/login?${NEXT_URL_QUERY_STRING_PARAMETER}=${encodeURIComponent( `${this.options.basePath.get(request)}${request.url.pathname}${request.url.search}` )}${ suggestedProviderName && !existingSessionValue @@ -387,20 +393,17 @@ export class Authenticator { assertRequest(request); const sessionValue = await this.getSessionValue(request); - if (sessionValue) { + const suggestedProviderName = + sessionValue?.provider.name ?? + request.url.searchParams.get(LOGOUT_PROVIDER_QUERY_STRING_PARAMETER); + if (suggestedProviderName) { await this.session.clear(request); - return this.providers - .get(sessionValue.provider.name)! - .logout(request, sessionValue.state ?? null); - } - const queryStringProviderName = (request.query as Record)?.provider; - if (queryStringProviderName) { - // provider name is passed in a query param and sourced from the browser's local storage; - // hence, we can't assume that this provider exists, so we have to check it - const provider = this.providers.get(queryStringProviderName); + // Provider name may be passed in a query param and sourced from the browser's local storage; + // hence, we can't assume that this provider exists, so we have to check it. + const provider = this.providers.get(suggestedProviderName); if (provider) { - return provider.logout(request, null); + return provider.logout(request, sessionValue?.state ?? null); } } else { // In case logout is called and we cannot figure out what provider is supposed to handle it, @@ -739,7 +742,7 @@ export class Authenticator { // redirect URL in the `next` parameter. Redirect URL provided in authentication result, if any, // always takes precedence over what is specified in `redirectURL` parameter. if (preAccessRedirectURL) { - preAccessRedirectURL = `${preAccessRedirectURL}?next=${encodeURIComponent( + preAccessRedirectURL = `${preAccessRedirectURL}?${NEXT_URL_QUERY_STRING_PARAMETER}=${encodeURIComponent( authenticationResult.redirectURL || redirectURL || `${this.options.basePath.get(request)}${request.url.pathname}${request.url.search}` @@ -756,4 +759,30 @@ export class Authenticator { }) : authenticationResult; } + + /** + * 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. + */ + private getLoggedOutURL(request: KibanaRequest, providerType: string) { + // 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(); + for (const [key, defaultValue] of [ + [NEXT_URL_QUERY_STRING_PARAMETER, null], + [LOGOUT_REASON_QUERY_STRING_PARAMETER, 'LOGGED_OUT'], + ] as Array<[string, string | null]>) { + const value = request.url.searchParams.get(key) || defaultValue; + if (value) { + searchParams.append(key, value); + } + } + + // Query string may contain the path where logout has been called or + // logout reason that login page may need to know. + return this.options.config.authc.selector.enabled || shouldProviderUseLoginForm(providerType) + ? `${this.options.basePath.serverBasePath}/login?${searchParams.toString()}` + : `${this.options.basePath.serverBasePath}/security/logged_out?${searchParams.toString()}`; + } } diff --git a/x-pack/plugins/security/server/authentication/providers/anonymous.ts b/x-pack/plugins/security/server/authentication/providers/anonymous.ts index 6d62d3a909e55..4d1b5f4a74b2f 100644 --- a/x-pack/plugins/security/server/authentication/providers/anonymous.ts +++ b/x-pack/plugins/security/server/authentication/providers/anonymous.ts @@ -162,7 +162,7 @@ export class AnonymousAuthenticationProvider extends BaseAuthenticationProvider return DeauthenticationResult.notHandled(); } - return DeauthenticationResult.redirectTo(this.options.urls.loggedOut); + return DeauthenticationResult.redirectTo(this.options.urls.loggedOut(request)); } /** 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 9bf9bb8bee70c..269dbb648640e 100644 --- a/x-pack/plugins/security/server/authentication/providers/base.mock.ts +++ b/x-pack/plugins/security/server/authentication/providers/base.mock.ts @@ -23,7 +23,7 @@ export function mockAuthenticationProviderOptions(options?: { name: string }) { tokens: { refresh: jest.fn(), invalidate: jest.fn() }, name: options?.name ?? 'basic1', urls: { - loggedOut: '/mock-server-basepath/security/logged_out', + loggedOut: jest.fn().mockReturnValue('/mock-server-basepath/security/logged_out'), }, }; } diff --git a/x-pack/plugins/security/server/authentication/providers/base.ts b/x-pack/plugins/security/server/authentication/providers/base.ts index 9d26761060dd3..19b351a0c49d8 100644 --- a/x-pack/plugins/security/server/authentication/providers/base.ts +++ b/x-pack/plugins/security/server/authentication/providers/base.ts @@ -30,7 +30,7 @@ export interface AuthenticationProviderOptions { logger: Logger; tokens: PublicMethodsOf; urls: { - loggedOut: string; + loggedOut: (request: KibanaRequest) => string; }; } diff --git a/x-pack/plugins/security/server/authentication/providers/basic.test.ts b/x-pack/plugins/security/server/authentication/providers/basic.test.ts index 87002ebed5672..4f93e2327da06 100644 --- a/x-pack/plugins/security/server/authentication/providers/basic.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/basic.test.ts @@ -34,6 +34,8 @@ describe('BasicAuthenticationProvider', () => { let mockOptions: ReturnType; beforeEach(() => { mockOptions = mockAuthenticationProviderOptions(); + mockOptions.urls.loggedOut.mockReturnValue('/some-logged-out-page'); + provider = new BasicAuthenticationProvider(mockOptions); }); @@ -184,30 +186,13 @@ describe('BasicAuthenticationProvider', () => { ); }); - it('redirects to login view if state is `null`.', async () => { - await expect(provider.logout(httpServerMock.createKibanaRequest(), null)).resolves.toEqual( - DeauthenticationResult.redirectTo('/mock-server-basepath/login?msg=LOGGED_OUT') - ); - }); - - it('always redirects to the login page.', async () => { + it('redirects to the logged out URL.', async () => { await expect(provider.logout(httpServerMock.createKibanaRequest(), {})).resolves.toEqual( - DeauthenticationResult.redirectTo('/mock-server-basepath/login?msg=LOGGED_OUT') + DeauthenticationResult.redirectTo('/some-logged-out-page') ); - }); - it('passes query string parameters to the login page.', async () => { - await expect( - provider.logout( - httpServerMock.createKibanaRequest({ - query: { next: '/app/ml', msg: 'SESSION_EXPIRED' }, - }), - {} - ) - ).resolves.toEqual( - DeauthenticationResult.redirectTo( - '/mock-server-basepath/login?next=%2Fapp%2Fml&msg=SESSION_EXPIRED' - ) + await expect(provider.logout(httpServerMock.createKibanaRequest(), null)).resolves.toEqual( + DeauthenticationResult.redirectTo('/some-logged-out-page') ); }); }); diff --git a/x-pack/plugins/security/server/authentication/providers/basic.ts b/x-pack/plugins/security/server/authentication/providers/basic.ts index 28b671346ee7f..6a5ae29dfd832 100644 --- a/x-pack/plugins/security/server/authentication/providers/basic.ts +++ b/x-pack/plugins/security/server/authentication/providers/basic.ts @@ -5,6 +5,7 @@ */ import { KibanaRequest } from '../../../../../../src/core/server'; +import { NEXT_URL_QUERY_STRING_PARAMETER } from '../../../common/constants'; import { canRedirectRequest } from '../can_redirect_request'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; @@ -108,7 +109,7 @@ export class BasicAuthenticationProvider extends BaseAuthenticationProvider { this.logger.debug('Redirecting request to Login page.'); const basePath = this.options.basePath.get(request); return AuthenticationResult.redirectTo( - `${basePath}/login?next=${encodeURIComponent( + `${basePath}/login?${NEXT_URL_QUERY_STRING_PARAMETER}=${encodeURIComponent( `${basePath}${request.url.pathname}${request.url.search}` )}` ); @@ -131,12 +132,7 @@ export class BasicAuthenticationProvider extends BaseAuthenticationProvider { return DeauthenticationResult.notHandled(); } - // Query string may contain the path where logout has been called or - // logout reason that login page may need to know. - const queryString = request.url.search || `?msg=LOGGED_OUT`; - return DeauthenticationResult.redirectTo( - `${this.options.basePath.get(request)}/login${queryString}` - ); + return DeauthenticationResult.redirectTo(this.options.urls.loggedOut(request)); } /** diff --git a/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts b/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts index eb4ac8f4dcbed..d368bf90cf360 100644 --- a/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts @@ -470,7 +470,7 @@ describe('KerberosAuthenticationProvider', () => { const request = httpServerMock.createKibanaRequest(); await expect(provider.logout(request, null)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.tokens.invalidate).not.toHaveBeenCalled(); @@ -501,7 +501,7 @@ describe('KerberosAuthenticationProvider', () => { mockOptions.tokens.invalidate.mockResolvedValue(undefined); await expect(provider.logout(request, tokenPair)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1); diff --git a/x-pack/plugins/security/server/authentication/providers/kerberos.ts b/x-pack/plugins/security/server/authentication/providers/kerberos.ts index 2e15893d0845f..9bf419c7dacaa 100644 --- a/x-pack/plugins/security/server/authentication/providers/kerberos.ts +++ b/x-pack/plugins/security/server/authentication/providers/kerberos.ts @@ -124,7 +124,7 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { } } - return DeauthenticationResult.redirectTo(this.options.urls.loggedOut); + return DeauthenticationResult.redirectTo(this.options.urls.loggedOut(request)); } /** 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 126306c885e53..9988ddd99c395 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts @@ -611,10 +611,10 @@ describe('OIDCAuthenticationProvider', () => { const request = httpServerMock.createKibanaRequest(); await expect(provider.logout(request, null)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); await expect(provider.logout(request, { nonce: 'x', realm: 'oidc1' })).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled(); @@ -647,7 +647,7 @@ describe('OIDCAuthenticationProvider', () => { await expect( provider.logout(request, { accessToken, refreshToken, realm: 'oidc1' }) - ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut)); + ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request))); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.oidcLogout', { diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.ts b/x-pack/plugins/security/server/authentication/providers/oidc.ts index 250641d1cf174..c46ea37f144e9 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.ts @@ -7,6 +7,7 @@ import Boom from '@hapi/boom'; import type from 'type-detect'; import { KibanaRequest } from '../../../../../../src/core/server'; +import { NEXT_URL_QUERY_STRING_PARAMETER } from '../../../common/constants'; import type { AuthenticationInfo } from '../../elasticsearch'; import { AuthenticationResult } from '../authentication_result'; import { canRedirectRequest } from '../can_redirect_request'; @@ -434,7 +435,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { } } - return DeauthenticationResult.redirectTo(this.options.urls.loggedOut); + return DeauthenticationResult.redirectTo(this.options.urls.loggedOut(request)); } /** @@ -450,14 +451,18 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { * @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], + ]); return AuthenticationResult.redirectTo( `${ this.options.basePath.serverBasePath - }/internal/security/capture-url?next=${encodeURIComponent( - `${this.options.basePath.get(request)}${request.url.pathname}${request.url.search}` - )}&providerType=${encodeURIComponent(this.type)}&providerName=${encodeURIComponent( - this.options.name - )}`, + }/internal/security/capture-url?${searchParams.toString()}`, // 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/pki.test.ts b/x-pack/plugins/security/server/authentication/providers/pki.test.ts index aa85b8a43af4d..763231f7fd0df 100644 --- a/x-pack/plugins/security/server/authentication/providers/pki.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/pki.test.ts @@ -544,7 +544,7 @@ describe('PKIAuthenticationProvider', () => { const request = httpServerMock.createKibanaRequest(); await expect(provider.logout(request, null)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.tokens.invalidate).not.toHaveBeenCalled(); @@ -572,7 +572,7 @@ describe('PKIAuthenticationProvider', () => { mockOptions.tokens.invalidate.mockResolvedValue(undefined); await expect(provider.logout(request, state)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1); diff --git a/x-pack/plugins/security/server/authentication/providers/pki.ts b/x-pack/plugins/security/server/authentication/providers/pki.ts index 974a838127e1d..4bb0ddaa4ee65 100644 --- a/x-pack/plugins/security/server/authentication/providers/pki.ts +++ b/x-pack/plugins/security/server/authentication/providers/pki.ts @@ -128,7 +128,7 @@ export class PKIAuthenticationProvider extends BaseAuthenticationProvider { } } - return DeauthenticationResult.redirectTo(this.options.urls.loggedOut); + return DeauthenticationResult.redirectTo(this.options.urls.loggedOut(request)); } /** 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 674bf2d61a2eb..5e14d8abeab23 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.test.ts @@ -1124,10 +1124,10 @@ describe('SAMLAuthenticationProvider', () => { const request = httpServerMock.createKibanaRequest(); await expect(provider.logout(request, null)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); await expect(provider.logout(request, { somethingElse: 'x' } as any)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled(); @@ -1187,7 +1187,7 @@ describe('SAMLAuthenticationProvider', () => { refreshToken, realm: 'test-realm', }) - ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut)); + ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request))); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.samlLogout', { @@ -1208,7 +1208,7 @@ describe('SAMLAuthenticationProvider', () => { refreshToken, realm: 'test-realm', }) - ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut)); + ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request))); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.samlLogout', { @@ -1231,7 +1231,7 @@ describe('SAMLAuthenticationProvider', () => { refreshToken, realm: 'test-realm', }) - ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut)); + ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request))); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.samlLogout', { @@ -1250,7 +1250,7 @@ describe('SAMLAuthenticationProvider', () => { refreshToken: 'x-saml-refresh-token', realm: 'test-realm', }) - ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut)); + ).resolves.toEqual(DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request))); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.samlInvalidate', { @@ -1264,7 +1264,7 @@ describe('SAMLAuthenticationProvider', () => { mockOptions.client.callAsInternalUser.mockResolvedValue({ redirect: null }); await expect(provider.logout(request)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); @@ -1282,7 +1282,7 @@ describe('SAMLAuthenticationProvider', () => { mockOptions.client.callAsInternalUser.mockResolvedValue({ redirect: undefined }); await expect(provider.logout(request)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); @@ -1298,7 +1298,7 @@ describe('SAMLAuthenticationProvider', () => { const request = httpServerMock.createKibanaRequest({ query: { SAMLResponse: 'xxx yyy' } }); await expect(provider.logout(request)).resolves.toEqual( - DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut) + DeauthenticationResult.redirectTo(mockOptions.urls.loggedOut(request)) ); expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled(); diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts index c2e82bb3df3e9..09ec62e503da1 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.ts @@ -7,6 +7,7 @@ import Boom from '@hapi/boom'; import { KibanaRequest } from '../../../../../../src/core/server'; import { isInternalURL } from '../../../common/is_internal_url'; +import { NEXT_URL_QUERY_STRING_PARAMETER } from '../../../common/constants'; import type { AuthenticationInfo } from '../../elasticsearch'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; @@ -280,7 +281,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { } } - return DeauthenticationResult.redirectTo(this.options.urls.loggedOut); + return DeauthenticationResult.redirectTo(this.options.urls.loggedOut(request)); } /** @@ -632,14 +633,18 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { * @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], + ]); return AuthenticationResult.redirectTo( `${ this.options.basePath.serverBasePath - }/internal/security/capture-url?next=${encodeURIComponent( - `${this.options.basePath.get(request)}${request.url.pathname}${request.url.search}` - )}&providerType=${encodeURIComponent(this.type)}&providerName=${encodeURIComponent( - this.options.name - )}`, + }/internal/security/capture-url?${searchParams.toString()}`, // 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/token.test.ts b/x-pack/plugins/security/server/authentication/providers/token.test.ts index e09400e9bb44a..5a600461ef467 100644 --- a/x-pack/plugins/security/server/authentication/providers/token.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/token.test.ts @@ -37,6 +37,8 @@ describe('TokenAuthenticationProvider', () => { let mockOptions: MockAuthenticationProviderOptions; beforeEach(() => { mockOptions = mockAuthenticationProviderOptions({ name: 'token' }); + mockOptions.urls.loggedOut.mockReturnValue('/some-logged-out-page'); + provider = new TokenAuthenticationProvider(mockOptions); }); @@ -347,11 +349,9 @@ describe('TokenAuthenticationProvider', () => { expect(mockOptions.tokens.invalidate).not.toHaveBeenCalled(); }); - it('redirects to login view if state is `null`.', async () => { - const request = httpServerMock.createKibanaRequest(); - - await expect(provider.logout(request, null)).resolves.toEqual( - DeauthenticationResult.redirectTo('/mock-server-basepath/login?msg=LOGGED_OUT') + it('redirects to the logged out URL if state is `null`.', async () => { + await expect(provider.logout(httpServerMock.createKibanaRequest(), null)).resolves.toEqual( + DeauthenticationResult.redirectTo('/some-logged-out-page') ); expect(mockOptions.tokens.invalidate).not.toHaveBeenCalled(); @@ -372,28 +372,14 @@ describe('TokenAuthenticationProvider', () => { expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith(tokenPair); }); - it('redirects to /login if tokens are invalidated successfully', async () => { + it('redirects to the logged out URL if tokens are invalidated successfully.', async () => { const request = httpServerMock.createKibanaRequest(); const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; mockOptions.tokens.invalidate.mockResolvedValue(undefined); await expect(provider.logout(request, tokenPair)).resolves.toEqual( - DeauthenticationResult.redirectTo('/mock-server-basepath/login?msg=LOGGED_OUT') - ); - - expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1); - expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith(tokenPair); - }); - - it('redirects to /login with optional search parameters if tokens are invalidated successfully', async () => { - const request = httpServerMock.createKibanaRequest({ query: { yep: 'nope' } }); - const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - - mockOptions.tokens.invalidate.mockResolvedValue(undefined); - - await expect(provider.logout(request, tokenPair)).resolves.toEqual( - DeauthenticationResult.redirectTo('/mock-server-basepath/login?yep=nope') + DeauthenticationResult.redirectTo('/some-logged-out-page') ); expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1); diff --git a/x-pack/plugins/security/server/authentication/providers/token.ts b/x-pack/plugins/security/server/authentication/providers/token.ts index 2032db4b0a8f2..67c2d244e75a2 100644 --- a/x-pack/plugins/security/server/authentication/providers/token.ts +++ b/x-pack/plugins/security/server/authentication/providers/token.ts @@ -6,6 +6,7 @@ import Boom from '@hapi/boom'; import { KibanaRequest } from '../../../../../../src/core/server'; +import { NEXT_URL_QUERY_STRING_PARAMETER } from '../../../common/constants'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; import { canRedirectRequest } from '../can_redirect_request'; @@ -145,10 +146,7 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { } } - const queryString = request.url.search || `?msg=LOGGED_OUT`; - return DeauthenticationResult.redirectTo( - `${this.options.basePath.get(request)}/login${queryString}` - ); + return DeauthenticationResult.redirectTo(this.options.urls.loggedOut(request)); } /** @@ -235,6 +233,8 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { const nextURL = encodeURIComponent( `${this.options.basePath.get(request)}${request.url.pathname}${request.url.search}` ); - return `${this.options.basePath.get(request)}/login?next=${nextURL}`; + return `${this.options.basePath.get( + request + )}/login?${NEXT_URL_QUERY_STRING_PARAMETER}=${nextURL}`; } } diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts index 9408147791c3d..62f36c400180f 100644 --- a/x-pack/plugins/security/server/config.ts +++ b/x-pack/plugins/security/server/config.ts @@ -9,7 +9,7 @@ import type { Duration } from 'moment'; import { schema, Type, TypeOf } from '@kbn/config-schema'; import { i18n } from '@kbn/i18n'; import { Logger, config as coreConfig } from '../../../../src/core/server'; -import type { AuthenticationProvider } from '../common/types'; +import type { AuthenticationProvider } from '../common/model'; export type ConfigType = ReturnType; type RawConfigType = TypeOf; diff --git a/x-pack/plugins/security/server/routes/views/access_agreement.test.ts b/x-pack/plugins/security/server/routes/views/access_agreement.test.ts index b513230b3ba6f..dfe5faa95ae15 100644 --- a/x-pack/plugins/security/server/routes/views/access_agreement.test.ts +++ b/x-pack/plugins/security/server/routes/views/access_agreement.test.ts @@ -14,7 +14,7 @@ import { RequestHandlerContext, } from '../../../../../../src/core/server'; import { SecurityLicense, SecurityLicenseFeatures } from '../../../common/licensing'; -import { AuthenticationProvider } from '../../../common/types'; +import type { AuthenticationProvider } from '../../../common/model'; import { ConfigType } from '../../config'; import { Session } from '../../session_management'; import { defineAccessAgreementRoutes } from './access_agreement'; diff --git a/x-pack/plugins/security/server/routes/views/login.ts b/x-pack/plugins/security/server/routes/views/login.ts index 93d43d04a86ca..68becb48f596e 100644 --- a/x-pack/plugins/security/server/routes/views/login.ts +++ b/x-pack/plugins/security/server/routes/views/login.ts @@ -7,6 +7,11 @@ import { schema } from '@kbn/config-schema'; import { parseNext } from '../../../common/parse_next'; import { LoginState } from '../../../common/login_state'; +import { shouldProviderUseLoginForm } from '../../../common/model'; +import { + LOGOUT_REASON_QUERY_STRING_PARAMETER, + NEXT_URL_QUERY_STRING_PARAMETER, +} from '../../../common/constants'; import { RouteDefinitionParams } from '..'; /** @@ -26,8 +31,8 @@ export function defineLoginRoutes({ validate: { query: schema.object( { - next: schema.maybe(schema.string()), - msg: schema.maybe(schema.string()), + [NEXT_URL_QUERY_STRING_PARAMETER]: schema.maybe(schema.string()), + [LOGOUT_REASON_QUERY_STRING_PARAMETER]: schema.maybe(schema.string()), }, { unknowns: 'allow' } ), @@ -59,7 +64,7 @@ export function defineLoginRoutes({ // Since `config.authc.sortedProviders` is based on `config.authc.providers` config we can // be sure that config is present for every provider in `config.authc.sortedProviders`. const { showInSelector, description, hint, icon } = config.authc.providers[type]?.[name]!; - const usesLoginForm = type === 'basic' || type === 'token'; + const usesLoginForm = shouldProviderUseLoginForm(type); return { type, name, diff --git a/x-pack/plugins/security/server/session_management/session.ts b/x-pack/plugins/security/server/session_management/session.ts index 757b1aaeddcbc..4dc83a1abe4af 100644 --- a/x-pack/plugins/security/server/session_management/session.ts +++ b/x-pack/plugins/security/server/session_management/session.ts @@ -9,7 +9,7 @@ import { randomBytes, createHash } from 'crypto'; import nodeCrypto, { Crypto } from '@elastic/node-crypto'; import type { PublicMethodsOf } from '@kbn/utility-types'; import type { KibanaRequest, Logger } from '../../../../../src/core/server'; -import type { AuthenticationProvider } from '../../common/types'; +import type { AuthenticationProvider } from '../../common/model'; import type { ConfigType } from '../config'; import type { SessionIndex, SessionIndexValue } from './session_index'; import type { SessionCookie } from './session_cookie'; diff --git a/x-pack/plugins/security/server/session_management/session_index.ts b/x-pack/plugins/security/server/session_management/session_index.ts index 96fff41d57503..45b2f4489c195 100644 --- a/x-pack/plugins/security/server/session_management/session_index.ts +++ b/x-pack/plugins/security/server/session_management/session_index.ts @@ -5,7 +5,7 @@ */ import type { ILegacyClusterClient, Logger } from '../../../../../src/core/server'; -import type { AuthenticationProvider } from '../../common/types'; +import type { AuthenticationProvider } from '../../common/model'; import type { ConfigType } from '../config'; export interface SessionIndexOptions { diff --git a/x-pack/test/security_api_integration/tests/anonymous/login.ts b/x-pack/test/security_api_integration/tests/anonymous/login.ts index 559dc7cc78036..eaf999c509741 100644 --- a/x-pack/test/security_api_integration/tests/anonymous/login.ts +++ b/x-pack/test/security_api_integration/tests/anonymous/login.ts @@ -182,7 +182,7 @@ export default function ({ getService }: FtrProviderContext) { expect(cookies).to.have.length(1); checkCookieIsCleared(request.cookie(cookies[0])!); - expect(logoutResponse.headers.location).to.be('/security/logged_out'); + expect(logoutResponse.headers.location).to.be('/security/logged_out?msg=LOGGED_OUT'); // Old cookie should be invalidated and not allow API access. const apiResponse = await supertest diff --git a/x-pack/test/security_api_integration/tests/kerberos/kerberos_login.ts b/x-pack/test/security_api_integration/tests/kerberos/kerberos_login.ts index e63f8cd2ebe32..26edc36563e1c 100644 --- a/x-pack/test/security_api_integration/tests/kerberos/kerberos_login.ts +++ b/x-pack/test/security_api_integration/tests/kerberos/kerberos_login.ts @@ -259,7 +259,7 @@ export default function ({ getService }: FtrProviderContext) { expect(cookies).to.have.length(1); checkCookieIsCleared(request.cookie(cookies[0])!); - expect(logoutResponse.headers.location).to.be('/security/logged_out'); + expect(logoutResponse.headers.location).to.be('/security/logged_out?msg=LOGGED_OUT'); // Token that was stored in the previous cookie should be invalidated as well and old // session cookie should not allow API access. diff --git a/x-pack/test/security_api_integration/tests/login_selector/basic_functionality.ts b/x-pack/test/security_api_integration/tests/login_selector/basic_functionality.ts index edcc1b5744fe3..a9e83ff7dadce 100644 --- a/x-pack/test/security_api_integration/tests/login_selector/basic_functionality.ts +++ b/x-pack/test/security_api_integration/tests/login_selector/basic_functionality.ts @@ -10,7 +10,7 @@ import { resolve } from 'path'; import url from 'url'; import { CA_CERT_PATH } from '@kbn/dev-utils'; import expect from '@kbn/expect'; -import type { AuthenticationProvider } from '../../../../plugins/security/common/types'; +import type { AuthenticationProvider } from '../../../../plugins/security/common/model'; import { getStateAndNonce } from '../../fixtures/oidc/oidc_tools'; import { getMutualAuthenticationResponseToken, diff --git a/x-pack/test/security_api_integration/tests/pki/pki_auth.ts b/x-pack/test/security_api_integration/tests/pki/pki_auth.ts index 93eabe33dc687..0d630dab51cf7 100644 --- a/x-pack/test/security_api_integration/tests/pki/pki_auth.ts +++ b/x-pack/test/security_api_integration/tests/pki/pki_auth.ts @@ -307,7 +307,7 @@ export default function ({ getService }: FtrProviderContext) { expect(cookies).to.have.length(1); checkCookieIsCleared(request.cookie(cookies[0])!); - expect(logoutResponse.headers.location).to.be('/security/logged_out'); + expect(logoutResponse.headers.location).to.be('/security/logged_out?msg=LOGGED_OUT'); }); it('should redirect to home page if session cookie is not provided', async () => { diff --git a/x-pack/test/security_api_integration/tests/session_idle/cleanup.ts b/x-pack/test/security_api_integration/tests/session_idle/cleanup.ts index c1e8bb9938986..8251ca3419ac8 100644 --- a/x-pack/test/security_api_integration/tests/session_idle/cleanup.ts +++ b/x-pack/test/security_api_integration/tests/session_idle/cleanup.ts @@ -7,7 +7,7 @@ import request, { Cookie } from 'request'; import { delay } from 'bluebird'; import expect from '@kbn/expect'; -import type { AuthenticationProvider } from '../../../../plugins/security/common/types'; +import type { AuthenticationProvider } from '../../../../plugins/security/common/model'; import { getSAMLRequestId, getSAMLResponse } from '../../fixtures/saml/saml_tools'; import { FtrProviderContext } from '../../ftr_provider_context'; diff --git a/x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts b/x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts index 59e8c746a6d07..134c9e9b1ad82 100644 --- a/x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts +++ b/x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts @@ -7,7 +7,7 @@ import request, { Cookie } from 'request'; import { delay } from 'bluebird'; import expect from '@kbn/expect'; -import type { AuthenticationProvider } from '../../../../plugins/security/common/types'; +import type { AuthenticationProvider } from '../../../../plugins/security/common/model'; import { getSAMLRequestId, getSAMLResponse } from '../../fixtures/saml/saml_tools'; import { FtrProviderContext } from '../../ftr_provider_context';