diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker index eb7a121c2e64b..d1fb544de733c 100755 --- a/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker +++ b/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker @@ -234,6 +234,7 @@ kibana_vars=( xpack.security.session.idleTimeout xpack.security.session.lifespan xpack.security.loginAssistanceMessage + xpack.security.loginHelp telemetry.allowChangingOptInStatus telemetry.enabled telemetry.optIn diff --git a/x-pack/plugins/security/common/login_state.ts b/x-pack/plugins/security/common/login_state.ts index 4342e82d2f90b..fd2b1cb8d1cf7 100644 --- a/x-pack/plugins/security/common/login_state.ts +++ b/x-pack/plugins/security/common/login_state.ts @@ -6,15 +6,24 @@ import { LoginLayout } from './licensing'; +export interface LoginSelectorProvider { + type: string; + name: string; + usesLoginForm: boolean; + description?: string; + hint?: string; + icon?: string; +} + export interface LoginSelector { enabled: boolean; - providers: Array<{ type: string; name: string; description?: string }>; + providers: LoginSelectorProvider[]; } export interface LoginState { layout: LoginLayout; allowLogin: boolean; - showLoginForm: boolean; requiresSecureConnection: boolean; + loginHelp?: string; selector: LoginSelector; } diff --git a/x-pack/plugins/security/public/authentication/_index.scss b/x-pack/plugins/security/public/authentication/_index.scss deleted file mode 100644 index 0a423c00f0218..0000000000000 --- a/x-pack/plugins/security/public/authentication/_index.scss +++ /dev/null @@ -1,5 +0,0 @@ -// Component styles -@import './components/index'; - -// Login styles -@import './login/index'; diff --git a/x-pack/plugins/security/public/authentication/components/_index.scss b/x-pack/plugins/security/public/authentication/components/_index.scss deleted file mode 100644 index dfa258d523c5a..0000000000000 --- a/x-pack/plugins/security/public/authentication/components/_index.scss +++ /dev/null @@ -1 +0,0 @@ -@import './authentication_state_page/index'; diff --git a/x-pack/plugins/security/public/authentication/components/authentication_state_page/_index.scss b/x-pack/plugins/security/public/authentication/components/authentication_state_page/_index.scss deleted file mode 100644 index f7cdd75143791..0000000000000 --- a/x-pack/plugins/security/public/authentication/components/authentication_state_page/_index.scss +++ /dev/null @@ -1 +0,0 @@ -@import './authentication_state_page'; diff --git a/x-pack/plugins/security/public/authentication/components/authentication_state_page/authentication_state_page.tsx b/x-pack/plugins/security/public/authentication/components/authentication_state_page/authentication_state_page.tsx index aa30661129978..66176129407cd 100644 --- a/x-pack/plugins/security/public/authentication/components/authentication_state_page/authentication_state_page.tsx +++ b/x-pack/plugins/security/public/authentication/components/authentication_state_page/authentication_state_page.tsx @@ -4,6 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ +import './_authentication_state_page.scss'; + import { EuiIcon, EuiSpacer, EuiTitle } from '@elastic/eui'; import React from 'react'; diff --git a/x-pack/plugins/security/public/authentication/login/__snapshots__/login_page.test.tsx.snap b/x-pack/plugins/security/public/authentication/login/__snapshots__/login_page.test.tsx.snap index ecbdfedac1dd3..bbc6bfa1faddc 100644 --- a/x-pack/plugins/security/public/authentication/login/__snapshots__/login_page.test.tsx.snap +++ b/x-pack/plugins/security/public/authentication/login/__snapshots__/login_page.test.tsx.snap @@ -121,10 +121,15 @@ exports[`LoginPage enabled form state renders as expected 1`] = ` selector={ Object { "enabled": false, - "providers": Array [], + "providers": Array [ + Object { + "name": "basic1", + "type": "basic", + "usesLoginForm": true, + }, + ], } } - showLoginForm={true} /> `; @@ -155,10 +160,15 @@ exports[`LoginPage enabled form state renders as expected when info message is s selector={ Object { "enabled": false, - "providers": Array [], + "providers": Array [ + Object { + "name": "basic1", + "type": "basic", + "usesLoginForm": true, + }, + ], } } - showLoginForm={true} /> `; @@ -189,10 +199,55 @@ exports[`LoginPage enabled form state renders as expected when loginAssistanceMe selector={ Object { "enabled": false, - "providers": Array [], + "providers": Array [ + Object { + "name": "basic1", + "type": "basic", + "usesLoginForm": true, + }, + ], + } + } +/> +`; + +exports[`LoginPage enabled form state renders as expected when loginHelp is set 1`] = ` + `; @@ -279,10 +334,15 @@ exports[`LoginPage page renders as expected 1`] = ` selector={ Object { "enabled": false, - "providers": Array [], + "providers": Array [ + Object { + "name": "basic1", + "type": "basic", + "usesLoginForm": true, + }, + ], } } - showLoginForm={true} /> diff --git a/x-pack/plugins/security/public/authentication/login/_index.scss b/x-pack/plugins/security/public/authentication/login/_index.scss deleted file mode 100644 index 4dd2c0cabfb5e..0000000000000 --- a/x-pack/plugins/security/public/authentication/login/_index.scss +++ /dev/null @@ -1 +0,0 @@ -@import './login_page'; diff --git a/x-pack/plugins/security/public/authentication/login/components/login_form/__snapshots__/login_form.test.tsx.snap b/x-pack/plugins/security/public/authentication/login/components/login_form/__snapshots__/login_form.test.tsx.snap index 7b8283b7bec0e..072a025aa06a0 100644 --- a/x-pack/plugins/security/public/authentication/login/components/login_form/__snapshots__/login_form.test.tsx.snap +++ b/x-pack/plugins/security/public/authentication/login/components/login_form/__snapshots__/login_form.test.tsx.snap @@ -1,170 +1,91 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`LoginForm login selector renders as expected with login form 1`] = ` - - - Login w/SAML - - - - Login w/PKI - - - login help and back: Login Help 1`] = ` + +
- ―――   - -   ――― - - - -
- - } - labelType="label" + - - - - } - labelType="label" - > - - - - - - -
- + some help + +

+
+ `; -exports[`LoginForm login selector renders as expected without login form for providers with and without description 1`] = ` - - +
- Login w/SAML - - - + + some help + +

+
+ +`; + +exports[`LoginForm properly switches to login help: Login Help 1`] = ` + +
- - - - +

+ + some help + +

+
+ `; exports[`LoginForm renders as expected 1`] = ` - +
@@ -227,21 +148,32 @@ exports[`LoginForm renders as expected 1`] = ` value="" /> - + - - + + + + + +
diff --git a/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.scss b/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.scss new file mode 100644 index 0000000000000..6784052ef4337 --- /dev/null +++ b/x-pack/plugins/security/public/authentication/login/components/login_form/login_form.scss @@ -0,0 +1,55 @@ +.secLoginCard { + display: block; + box-shadow: none; + padding: $euiSize; + text-align: left; + width: 100%; + + &:hover { + .secLoginCard__title { + text-decoration: underline; + } + } + + &:disabled { + pointer-events: none; + } + + &:not(.secLoginCard-isLoading):disabled { + .secLoginCard__title, + .secLoginCard__hint { + color: $euiColorMediumShade; + } + } + + &:focus { + border-color: transparent; + border-radius: $euiBorderRadius; + @include euiFocusRing; + + .secLoginCard__title { + text-decoration: underline; + } + + // Make the focus ring clean and without borders + + .secLoginCard { + border-color: transparent; + } + } + + + .secLoginCard { + border-top: $euiBorderThin; + } +} + +.secLoginCard__hint { + @include euiFontSizeXS; + color: $euiColorDarkShade; + margin-top: $euiSizeXS; +} + +.secLoginAssistanceMessage { + // This tightens up the layout if message is present + margin-top: -($euiSizeXXL + $euiSizeS); + padding: 0 $euiSize; +} 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 c17c10a2c5148..4e172cdde0eed 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 @@ -5,12 +5,39 @@ */ import React from 'react'; +import ReactMarkdown from 'react-markdown'; import { act } from '@testing-library/react'; -import { EuiButton, EuiCallOut } from '@elastic/eui'; +import { EuiButton, EuiCallOut, EuiIcon } from '@elastic/eui'; import { mountWithIntl, nextTick, shallowWithIntl } from 'test_utils/enzyme_helpers'; -import { LoginForm } from './login_form'; +import { findTestSubject } from 'test_utils/find_test_subject'; +import { LoginForm, PageMode } from './login_form'; import { coreMock } from '../../../../../../../../src/core/public/mocks'; +import { ReactWrapper } from 'enzyme'; + +function expectPageMode(wrapper: ReactWrapper, mode: PageMode) { + const assertions: Array<[string, boolean]> = + mode === PageMode.Form + ? [ + ['loginForm', true], + ['loginSelector', false], + ['loginHelp', false], + ] + : mode === PageMode.Selector + ? [ + ['loginForm', false], + ['loginSelector', true], + ['loginHelp', false], + ] + : [ + ['loginForm', false], + ['loginSelector', false], + ['loginHelp', true], + ]; + for (const [selector, exists] of assertions) { + expect(findTestSubject(wrapper, selector).exists()).toBe(exists); + } +} describe('LoginForm', () => { beforeAll(() => { @@ -32,8 +59,10 @@ describe('LoginForm', () => { http={coreStartMock.http} notifications={coreStartMock.notifications} loginAssistanceMessage="" - showLoginForm={true} - selector={{ enabled: false, providers: [] }} + selector={{ + enabled: false, + providers: [{ type: 'basic', name: 'basic', usesLoginForm: true }], + }} /> ) ).toMatchSnapshot(); @@ -41,20 +70,44 @@ describe('LoginForm', () => { it('renders an info message when provided.', () => { const coreStartMock = coreMock.createStart(); - const wrapper = shallowWithIntl( + const wrapper = mountWithIntl( ); + expectPageMode(wrapper, PageMode.Form); + expect(wrapper.find(EuiCallOut).props().title).toEqual('Hey this is an info message'); }); + it('renders `Need help?` link if login help text is provided.', () => { + const coreStartMock = coreMock.createStart(); + const wrapper = mountWithIntl( + + ); + + expectPageMode(wrapper, PageMode.Form); + + expect(findTestSubject(wrapper, 'loginHelpLink').text()).toEqual('Need help?'); + }); + it('renders an invalid credentials message', async () => { const coreStartMock = coreMock.createStart({ basePath: '/some-base-path' }); coreStartMock.http.post.mockRejectedValue({ response: { status: 401 } }); @@ -64,11 +117,15 @@ describe('LoginForm', () => { http={coreStartMock.http} notifications={coreStartMock.notifications} loginAssistanceMessage="" - showLoginForm={true} - selector={{ enabled: false, providers: [] }} + selector={{ + enabled: false, + providers: [{ type: 'basic', name: 'basic', usesLoginForm: true }], + }} /> ); + expectPageMode(wrapper, PageMode.Form); + wrapper.find('input[name="username"]').simulate('change', { target: { value: 'username' } }); wrapper.find('input[name="password"]').simulate('change', { target: { value: 'password' } }); wrapper.find(EuiButton).simulate('click'); @@ -92,11 +149,15 @@ describe('LoginForm', () => { http={coreStartMock.http} notifications={coreStartMock.notifications} loginAssistanceMessage="" - showLoginForm={true} - selector={{ enabled: false, providers: [] }} + selector={{ + enabled: false, + providers: [{ type: 'basic', name: 'basic', usesLoginForm: true }], + }} /> ); + expectPageMode(wrapper, PageMode.Form); + wrapper.find('input[name="username"]').simulate('change', { target: { value: 'username' } }); wrapper.find('input[name="password"]').simulate('change', { target: { value: 'password' } }); wrapper.find(EuiButton).simulate('click'); @@ -121,11 +182,15 @@ describe('LoginForm', () => { http={coreStartMock.http} notifications={coreStartMock.notifications} loginAssistanceMessage="" - showLoginForm={true} - selector={{ enabled: false, providers: [] }} + selector={{ + enabled: false, + providers: [{ type: 'basic', name: 'basic', usesLoginForm: true }], + }} /> ); + expectPageMode(wrapper, PageMode.Form); + wrapper.find('input[name="username"]').simulate('change', { target: { value: 'username1' } }); wrapper.find('input[name="password"]').simulate('change', { target: { value: 'password1' } }); wrapper.find(EuiButton).simulate('click'); @@ -144,47 +209,125 @@ describe('LoginForm', () => { expect(wrapper.find(EuiCallOut).exists()).toBe(false); }); + it('properly switches to login help', async () => { + const coreStartMock = coreMock.createStart({ basePath: '/some-base-path' }); + const wrapper = mountWithIntl( + + ); + + expectPageMode(wrapper, PageMode.Form); + expect(findTestSubject(wrapper, 'loginBackToSelector').exists()).toBe(false); + + // Going to login help. + findTestSubject(wrapper, 'loginHelpLink').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.LoginHelp); + + expect(findTestSubject(wrapper, 'loginHelp').find(ReactMarkdown)).toMatchSnapshot('Login Help'); + + // Going back to login form. + findTestSubject(wrapper, 'loginBackToLoginLink').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.Form); + expect(findTestSubject(wrapper, 'loginBackToSelector').exists()).toBe(false); + }); + describe('login selector', () => { - it('renders as expected with login form', async () => { + it('renders as expected with providers that use login form', async () => { const coreStartMock = coreMock.createStart(); + const wrapper = mountWithIntl( + + ); + + expectPageMode(wrapper, PageMode.Selector); + expect( - shallowWithIntl( - - ) - ).toMatchSnapshot(); + wrapper.find('.secLoginCard').map(card => { + const hint = card.find('.secLoginCard__hint'); + return { + title: card.find('p.secLoginCard__title').text(), + hint: hint.exists() ? hint.text() : '', + icon: card.find(EuiIcon).props().type, + }; + }) + ).toEqual([ + { title: 'Log in with basic/basic', hint: 'Basic hint', icon: 'logoElastic' }, + { title: 'Log in w/SAML', hint: '', icon: 'empty' }, + { title: 'Log in w/PKI', hint: 'PKI hint', icon: 'empty' }, + ]); }); - it('renders as expected without login form for providers with and without description', async () => { + it('renders as expected without providers that use login form', async () => { const coreStartMock = coreMock.createStart(); + const wrapper = mountWithIntl( + + ); + + expectPageMode(wrapper, PageMode.Selector); + expect( - shallowWithIntl( - - ) - ).toMatchSnapshot(); + wrapper.find('.secLoginCard').map(card => { + const hint = card.find('.secLoginCard__hint'); + return { + title: card.find('p.secLoginCard__title').text(), + hint: hint.exists() ? hint.text() : '', + icon: card.find(EuiIcon).props().type, + }; + }) + ).toEqual([ + { title: 'Login w/SAML', hint: 'SAML hint', icon: 'empty' }, + { title: 'Log in with pki/pki1', hint: '', icon: 'some-icon' }, + ]); }); it('properly redirects after successful login', async () => { @@ -203,17 +346,19 @@ describe('LoginForm', () => { http={coreStartMock.http} notifications={coreStartMock.notifications} loginAssistanceMessage="" - showLoginForm={true} selector={{ enabled: true, providers: [ - { type: 'saml', name: 'saml1', description: 'Login w/SAML' }, - { type: 'pki', name: 'pki1', description: 'Login w/PKI' }, + { type: 'basic', name: 'basic', usesLoginForm: true }, + { type: 'saml', name: 'saml1', description: 'Login w/SAML', usesLoginForm: false }, + { type: 'pki', name: 'pki1', description: 'Login w/PKI', usesLoginForm: false }, ], }} /> ); + expectPageMode(wrapper, PageMode.Selector); + wrapper.findWhere(node => node.key() === 'saml1').simulate('click'); await act(async () => { @@ -246,11 +391,18 @@ describe('LoginForm', () => { http={coreStartMock.http} notifications={coreStartMock.notifications} loginAssistanceMessage="" - showLoginForm={true} - selector={{ enabled: true, providers: [{ type: 'saml', name: 'saml1' }] }} + selector={{ + enabled: true, + providers: [ + { type: 'basic', name: 'basic', usesLoginForm: true }, + { type: 'saml', name: 'saml1', usesLoginForm: false }, + ], + }} /> ); + expectPageMode(wrapper, PageMode.Selector); + wrapper.findWhere(node => node.key() === 'saml1').simulate('click'); await act(async () => { @@ -268,5 +420,123 @@ describe('LoginForm', () => { title: 'Could not perform login.', }); }); + + it('properly switches to login form', async () => { + const currentURL = `https://some-host/login?next=${encodeURIComponent( + '/some-base-path/app/kibana#/home?_g=()' + )}`; + + const coreStartMock = coreMock.createStart({ basePath: '/some-base-path' }); + window.location.href = currentURL; + const wrapper = mountWithIntl( + + ); + + expectPageMode(wrapper, PageMode.Selector); + + wrapper.findWhere(node => node.key() === 'basic').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.Form); + + expect(coreStartMock.http.post).not.toHaveBeenCalled(); + expect(coreStartMock.notifications.toasts.addError).not.toHaveBeenCalled(); + expect(window.location.href).toBe(currentURL); + }); + + it('properly switches to login help', async () => { + const coreStartMock = coreMock.createStart({ basePath: '/some-base-path' }); + const wrapper = mountWithIntl( + + ); + + expectPageMode(wrapper, PageMode.Selector); + + findTestSubject(wrapper, 'loginHelpLink').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.LoginHelp); + + expect(findTestSubject(wrapper, 'loginHelp').find(ReactMarkdown)).toMatchSnapshot( + 'Login Help' + ); + + // Going back to login selector. + findTestSubject(wrapper, 'loginBackToLoginLink').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.Selector); + + expect(coreStartMock.http.post).not.toHaveBeenCalled(); + expect(coreStartMock.notifications.toasts.addError).not.toHaveBeenCalled(); + }); + + it('properly switches to login form -> login help and back', async () => { + const coreStartMock = coreMock.createStart({ basePath: '/some-base-path' }); + const wrapper = mountWithIntl( + + ); + + expectPageMode(wrapper, PageMode.Selector); + + // Going to login form. + wrapper.findWhere(node => node.key() === 'basic').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.Form); + + // Going to login help. + findTestSubject(wrapper, 'loginHelpLink').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.LoginHelp); + + expect(findTestSubject(wrapper, 'loginHelp').find(ReactMarkdown)).toMatchSnapshot( + 'Login Help' + ); + + // Going back to login form. + findTestSubject(wrapper, 'loginBackToLoginLink').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.Form); + + // Going back to login selector. + findTestSubject(wrapper, 'loginBackToSelector').simulate('click'); + wrapper.update(); + expectPageMode(wrapper, PageMode.Selector); + + expect(coreStartMock.http.post).not.toHaveBeenCalled(); + expect(coreStartMock.notifications.toasts.addError).not.toHaveBeenCalled(); + }); }); }); 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 01f5c40a69aeb..460c6550085a4 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 @@ -4,10 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ +import './login_form.scss'; + import React, { ChangeEvent, Component, FormEvent, Fragment, MouseEvent } from 'react'; import ReactMarkdown from 'react-markdown'; import { EuiButton, + EuiIcon, EuiCallOut, EuiFieldPassword, EuiFieldText, @@ -15,21 +18,28 @@ import { EuiPanel, EuiSpacer, EuiText, + EuiButtonEmpty, + EuiFlexGroup, + EuiFlexItem, + EuiTitle, + EuiLoadingSpinner, + EuiLink, + EuiHorizontalRule, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { HttpStart, IHttpFetchError, NotificationsStart } from 'src/core/public'; -import { LoginValidator, LoginValidationResult } from './validate_login'; import { parseNext } from '../../../../../common/parse_next'; import { LoginSelector } from '../../../../../common/login_state'; +import { LoginValidator } from './validate_login'; interface Props { http: HttpStart; notifications: NotificationsStart; selector: LoginSelector; - showLoginForm: boolean; infoMessage?: string; loginAssistanceMessage: string; + loginHelp?: string; } interface State { @@ -42,7 +52,8 @@ interface State { message: | { type: MessageType.None } | { type: MessageType.Danger | MessageType.Info; content: string }; - formError: LoginValidationResult | null; + mode: PageMode; + previousMode: PageMode; } enum LoadingStateType { @@ -57,12 +68,21 @@ enum MessageType { Danger, } +export enum PageMode { + Selector, + Form, + LoginHelp, +} + export class LoginForm extends Component { private readonly validator: LoginValidator; constructor(props: Props) { super(props); this.validator = new LoginValidator({ shouldValidate: false }); + + const mode = this.showLoginSelector() ? PageMode.Selector : PageMode.Form; + this.state = { loadingState: { type: LoadingStateType.None }, username: '', @@ -70,7 +90,8 @@ export class LoginForm extends Component { message: this.props.infoMessage ? { type: MessageType.Info, content: this.props.infoMessage } : { type: MessageType.None }, - formError: null, + mode, + previousMode: mode, }; } @@ -79,19 +100,91 @@ export class LoginForm extends Component { {this.renderLoginAssistanceMessage()} {this.renderMessage()} - {this.renderSelector()} - {this.renderLoginForm()} + {this.renderContent()} + {this.renderPageModeSwitchLink()} ); } - private renderLoginForm = () => { - if (!this.props.showLoginForm) { + private renderLoginAssistanceMessage = () => { + if (!this.props.loginAssistanceMessage) { return null; } return ( - +
+ + + {this.props.loginAssistanceMessage} + +
+ ); + }; + + private renderMessage = () => { + const { message } = this.state; + if (message.type === MessageType.Danger) { + return ( + + + + + ); + } + + if (message.type === MessageType.Info) { + return ( + + + + + ); + } + + return null; + }; + + public renderContent() { + switch (this.state.mode) { + case PageMode.Form: + return this.renderLoginForm(); + case PageMode.Selector: + return this.renderSelector(); + case PageMode.LoginHelp: + return this.renderLoginHelp(); + } + } + + private renderLoginForm = () => { + const loginSelectorLink = this.showLoginSelector() ? ( + + this.onPageModeChange(PageMode.Selector)} + > + + + + ) : null; + + return ( +
{ /> - - - + + + + + + + + + {loginSelectorLink} +
); }; - private renderLoginAssistanceMessage = () => { - if (!this.props.loginAssistanceMessage) { - return null; - } + private renderSelector = () => { + return ( + + {this.props.selector.providers.map(provider => ( + + ))} + + ); + }; + private renderLoginHelp = () => { return ( - - - {this.props.loginAssistanceMessage} + + + {this.props.loginHelp || ''} - + ); }; - private renderMessage = () => { - const { message } = this.state; - if (message.type === MessageType.Danger) { + private renderPageModeSwitchLink = () => { + if (this.state.mode === PageMode.LoginHelp) { return ( - - + + + this.onPageModeChange(this.state.previousMode)} + > + + + ); } - if (message.type === MessageType.Info) { + if (this.props.loginHelp) { return ( - - + + + this.onPageModeChange(PageMode.LoginHelp)} + > + + + ); } @@ -205,60 +359,16 @@ export class LoginForm extends Component { return null; }; - private renderSelector = () => { - const showLoginSelector = - this.props.selector.enabled && this.props.selector.providers.length > 0; - if (!showLoginSelector) { - return null; - } - - const loginSelectorAndLoginFormSeparator = showLoginSelector && this.props.showLoginForm && ( - <> - - ―――   - -   ――― - - - - ); - - return ( - <> - {this.props.selector.providers.map((provider, index) => ( - - this.loginWithSelector(provider.type, provider.name)} - > - {provider.description ?? ( - - )} - - - - ))} - {loginSelectorAndLoginFormSeparator} - - ); - }; - private setUsernameInputRef(ref: HTMLInputElement) { if (ref) { ref.focus(); } } + private onPageModeChange = (mode: PageMode) => { + this.setState({ message: { type: MessageType.None }, mode, previousMode: this.state.mode }); + }; + private onUsernameChange = (e: ChangeEvent) => { this.setState({ username: e.target.value, @@ -279,12 +389,10 @@ export class LoginForm extends Component { this.validator.enableValidation(); const { username, password } = this.state; - const result = this.validator.validateForLogin(username, password); - if (result.isInvalid) { - this.setState({ formError: result }); - return; - } else { - this.setState({ formError: null }); + if (this.validator.validateForLogin(username, password).isInvalid) { + // Since validation is enabled now, we should ask React to re-render form and display + // validation error messages if any. + return this.forceUpdate(); } this.setState({ @@ -351,4 +459,11 @@ export class LoginForm extends Component { loadingState.type !== LoadingStateType.Selector || loadingState.providerName === providerName ); } + + private showLoginSelector() { + return ( + this.props.selector.enabled && + this.props.selector.providers.some(provider => !provider.usesLoginForm) + ); + } } 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 c4be57d8d7db7..ab107e46dfff6 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 @@ -18,8 +18,10 @@ const createLoginState = (options?: Partial) => { allowLogin: true, layout: 'form', requiresSecureConnection: false, - showLoginForm: true, - selector: { enabled: false, providers: [] }, + selector: { + enabled: false, + providers: [{ type: 'basic', name: 'basic1', usesLoginForm: true }], + }, ...options, } as LoginState; }; @@ -163,7 +165,9 @@ describe('LoginPage', () => { it('renders as expected when login is not enabled', async () => { const coreStartMock = coreMock.createStart(); - httpMock.get.mockResolvedValue(createLoginState({ showLoginForm: false })); + httpMock.get.mockResolvedValue( + createLoginState({ selector: { enabled: false, providers: [] } }) + ); const wrapper = shallow( { expect(wrapper.find(LoginForm)).toMatchSnapshot(); }); + + it('renders as expected when loginHelp is set', async () => { + const coreStartMock = coreMock.createStart(); + httpMock.get.mockResolvedValue(createLoginState({ loginHelp: '**some-help**' })); + + const wrapper = shallow( + + ); + + await act(async () => { + await nextTick(); + wrapper.update(); + resetHttpMock(); // so the calls don't show in the BasicLoginForm snapshot + }); + + expect(wrapper.find(LoginForm)).toMatchSnapshot(); + }); }); describe('API calls', () => { 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 70f8f76ee0a9c..b7ac70f2aaf89 100644 --- a/x-pack/plugins/security/public/authentication/login/login_page.tsx +++ b/x-pack/plugins/security/public/authentication/login/login_page.tsx @@ -4,6 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ +import './_login_page.scss'; + import React, { Component } from 'react'; import ReactDOM from 'react-dom'; import classNames from 'classnames'; @@ -120,10 +122,9 @@ export class LoginPage extends Component { requiresSecureConnection, isSecureConnection, selector, - showLoginForm, + loginHelp, }: LoginState & { isSecureConnection: boolean }) => { - const isLoginExplicitlyDisabled = - !showLoginForm && (!selector.enabled || selector.providers.length === 0); + const isLoginExplicitlyDisabled = selector.providers.length === 0; if (isLoginExplicitlyDisabled) { return ( { ); }; diff --git a/x-pack/plugins/security/public/index.scss b/x-pack/plugins/security/public/index.scss index 999639ba22eb7..1bdb8cc178fdf 100644 --- a/x-pack/plugins/security/public/index.scss +++ b/x-pack/plugins/security/public/index.scss @@ -1,7 +1,4 @@ $secFormWidth: 460px; -// Authentication styles -@import './authentication/index'; - // Management styles @import './management/index'; diff --git a/x-pack/plugins/security/server/config.test.ts b/x-pack/plugins/security/server/config.test.ts index 46a7ee79ee60c..9899cd688d6dd 100644 --- a/x-pack/plugins/security/server/config.test.ts +++ b/x-pack/plugins/security/server/config.test.ts @@ -29,6 +29,8 @@ describe('config schema', () => { "basic": Object { "description": undefined, "enabled": true, + "hint": undefined, + "icon": undefined, "order": 0, "showInSelector": true, }, @@ -71,6 +73,8 @@ describe('config schema', () => { "basic": Object { "description": undefined, "enabled": true, + "hint": undefined, + "icon": undefined, "order": 0, "showInSelector": true, }, @@ -113,6 +117,8 @@ describe('config schema', () => { "basic": Object { "description": undefined, "enabled": true, + "hint": undefined, + "icon": undefined, "order": 0, "showInSelector": true, }, @@ -361,20 +367,6 @@ describe('config schema', () => { `); }); - it('does not allow custom description', () => { - expect(() => - ConfigSchema.validate({ - authc: { - providers: { basic: { basic1: { order: 0, description: 'Some description' } } }, - }, - }) - ).toThrowErrorMatchingInlineSnapshot(` -"[authc.providers]: types that failed validation: -- [authc.providers.0]: expected value of type [array] but got [Object] -- [authc.providers.1.basic.basic1.description]: \`basic\` provider does not support custom description." -`); - }); - it('cannot be hidden from selector', () => { expect(() => ConfigSchema.validate({ @@ -410,7 +402,9 @@ describe('config schema', () => { Object { "basic": Object { "basic1": Object { + "description": "Log in with Elasticsearch", "enabled": true, + "icon": "logoElastic", "order": 0, "showInSelector": true, }, @@ -433,20 +427,6 @@ describe('config schema', () => { `); }); - it('does not allow custom description', () => { - expect(() => - ConfigSchema.validate({ - authc: { - providers: { token: { token1: { order: 0, description: 'Some description' } } }, - }, - }) - ).toThrowErrorMatchingInlineSnapshot(` -"[authc.providers]: types that failed validation: -- [authc.providers.0]: expected value of type [array] but got [Object] -- [authc.providers.1.token.token1.description]: \`token\` provider does not support custom description." -`); - }); - it('cannot be hidden from selector', () => { expect(() => ConfigSchema.validate({ @@ -482,7 +462,9 @@ describe('config schema', () => { Object { "token": Object { "token1": Object { + "description": "Log in with Elasticsearch", "enabled": true, + "icon": "logoElastic", "order": 0, "showInSelector": true, }, @@ -759,12 +741,16 @@ describe('config schema', () => { Object { "basic": Object { "basic1": Object { + "description": "Log in with Elasticsearch", "enabled": true, + "icon": "logoElastic", "order": 0, "showInSelector": true, }, "basic2": Object { + "description": "Log in with Elasticsearch", "enabled": false, + "icon": "logoElastic", "order": 1, "showInSelector": true, }, @@ -1043,7 +1029,7 @@ describe('createConfig()', () => { Object { "name": "basic1", "options": Object { - "description": undefined, + "description": "Log in with Elasticsearch", "order": 3, "showInSelector": true, }, diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts index 97ff7d00a4336..7fe38b05f72d6 100644 --- a/x-pack/plugins/security/server/config.ts +++ b/x-pack/plugins/security/server/config.ts @@ -6,6 +6,7 @@ import crypto from 'crypto'; import { schema, Type, TypeOf } from '@kbn/config-schema'; +import { i18n } from '@kbn/i18n'; import { Logger } from '../../../../src/core/server'; export type ConfigType = ReturnType; @@ -21,7 +22,7 @@ const providerOptionsSchema = (providerType: string, optionsSchema: Type) = ); type ProvidersCommonConfigType = Record< - 'enabled' | 'showInSelector' | 'order' | 'description', + 'enabled' | 'showInSelector' | 'order' | 'description' | 'hint' | 'icon', Type >; function getCommonProviderSchemaProperties(overrides: Partial = {}) { @@ -30,6 +31,8 @@ function getCommonProviderSchemaProperties(overrides: Partial; const providersConfigSchema = schema.object( { basic: getUniqueProviderSchema('basic', { - description: schema.maybe( - schema.any({ - validate: () => '`basic` provider does not support custom description.', - }) - ), + description: schema.string({ + defaultValue: i18n.translate('xpack.security.loginWithElasticsearchLabel', { + defaultMessage: 'Log in with Elasticsearch', + }), + }), + icon: schema.string({ defaultValue: 'logoElastic' }), showInSelector: schema.boolean({ defaultValue: true, validate: value => { @@ -68,11 +72,12 @@ const providersConfigSchema = schema.object( }), }), token: getUniqueProviderSchema('token', { - description: schema.maybe( - schema.any({ - validate: () => '`token` provider does not support custom description.', - }) - ), + description: schema.string({ + defaultValue: i18n.translate('xpack.security.loginWithElasticsearchLabel', { + defaultMessage: 'Log in with Elasticsearch', + }), + }), + icon: schema.string({ defaultValue: 'logoElastic' }), showInSelector: schema.boolean({ defaultValue: true, validate: value => { @@ -131,6 +136,7 @@ const providersConfigSchema = schema.object( export const ConfigSchema = schema.object({ enabled: schema.boolean({ defaultValue: true }), loginAssistanceMessage: schema.string({ defaultValue: '' }), + loginHelp: schema.maybe(schema.string()), cookieName: schema.string({ defaultValue: 'sid' }), encryptionKey: schema.conditional( schema.contextRef('dist'), @@ -147,7 +153,16 @@ export const ConfigSchema = schema.object({ selector: schema.object({ enabled: schema.maybe(schema.boolean()) }), providers: schema.oneOf([schema.arrayOf(schema.string()), providersConfigSchema], { defaultValue: { - basic: { basic: { enabled: true, showInSelector: true, order: 0, description: undefined } }, + basic: { + basic: { + enabled: true, + showInSelector: true, + order: 0, + description: undefined, + hint: undefined, + icon: undefined, + }, + }, token: undefined, saml: undefined, oidc: undefined, diff --git a/x-pack/plugins/security/server/routes/views/login.test.ts b/x-pack/plugins/security/server/routes/views/login.test.ts index d43319efbdfb9..8bc2bb32325fc 100644 --- a/x-pack/plugins/security/server/routes/views/login.test.ts +++ b/x-pack/plugins/security/server/routes/views/login.test.ts @@ -15,7 +15,7 @@ import { RouteConfig, } from '../../../../../../src/core/server'; import { SecurityLicense } from '../../../common/licensing'; -import { LoginState } from '../../../common/login_state'; +import { LoginSelectorProvider } from '../../../common/login_state'; import { ConfigType } from '../../config'; import { defineLoginRoutes } from './login'; @@ -141,6 +141,10 @@ describe('Login view routes', () => { }); describe('Login state route', () => { + function getAuthcConfig(authcConfig: Record = {}) { + return routeDefinitionParamsMock.create({ authc: { ...authcConfig } }).config.authc; + } + let routeHandler: RequestHandler; let routeConfig: RouteConfig; beforeEach(() => { @@ -176,9 +180,11 @@ describe('Login view routes', () => { const expectedPayload = { allowLogin: true, layout: 'error-es-unavailable', - showLoginForm: true, requiresSecureConnection: false, - selector: { enabled: false, providers: [] }, + selector: { + enabled: false, + providers: [{ name: 'basic', type: 'basic', usesLoginForm: true }], + }, }; await expect( routeHandler({ core: contextMock } as any, request, kibanaResponseFactory) @@ -198,9 +204,11 @@ describe('Login view routes', () => { const expectedPayload = { allowLogin: true, layout: 'form', - showLoginForm: true, requiresSecureConnection: false, - selector: { enabled: false, providers: [] }, + selector: { + enabled: false, + providers: [{ name: 'basic', type: 'basic', usesLoginForm: true }], + }, }; await expect( routeHandler({ core: contextMock } as any, request, kibanaResponseFactory) @@ -229,22 +237,46 @@ describe('Login view routes', () => { }); }); - it('returns `showLoginForm: true` only if either `basic` or `token` provider is enabled.', async () => { + it('returns `useLoginForm: true` for `basic` and `token` providers.', async () => { license.getFeatures.mockReturnValue({ allowLogin: true, showLogin: true } as any); const request = httpServerMock.createKibanaRequest(); const contextMock = coreMock.createRequestHandlerContext(); - const cases: Array<[boolean, ConfigType['authc']['sortedProviders']]> = [ - [false, []], - [true, [{ type: 'basic', name: 'basic1', options: { order: 0, showInSelector: true } }]], - [true, [{ type: 'token', name: 'token1', options: { order: 0, showInSelector: true } }]], + const cases: Array<[LoginSelectorProvider[], ConfigType['authc']]> = [ + [[], getAuthcConfig({ providers: { basic: { basic1: { order: 0, enabled: false } } } })], + [ + [ + { + name: 'basic1', + type: 'basic', + usesLoginForm: true, + icon: 'logoElastic', + description: 'Log in with Elasticsearch', + }, + ], + getAuthcConfig({ providers: { basic: { basic1: { order: 0 } } } }), + ], + [ + [ + { + name: 'token1', + type: 'token', + usesLoginForm: true, + icon: 'logoElastic', + description: 'Log in with Elasticsearch', + }, + ], + getAuthcConfig({ providers: { token: { token1: { order: 0 } } } }), + ], ]; - for (const [showLoginForm, sortedProviders] of cases) { - config.authc.sortedProviders = sortedProviders; + for (const [providers, authcConfig] of cases) { + config.authc = authcConfig; - const expectedPayload = expect.objectContaining({ showLoginForm }); + const expectedPayload = expect.objectContaining({ + selector: { enabled: false, providers }, + }); await expect( routeHandler({ core: contextMock } as any, request, kibanaResponseFactory) ).resolves.toEqual({ @@ -261,81 +293,142 @@ describe('Login view routes', () => { const request = httpServerMock.createKibanaRequest(); const contextMock = coreMock.createRequestHandlerContext(); - const cases: Array<[ - boolean, - ConfigType['authc']['sortedProviders'], - LoginState['selector']['providers'] - ]> = [ - // selector is disabled, providers shouldn't be returned. + const cases: Array<[ConfigType['authc'], LoginSelectorProvider[]]> = [ + // selector is disabled, multiple providers, but only basic provider should be returned. [ - false, + getAuthcConfig({ + selector: { enabled: false }, + providers: { + basic: { basic1: { order: 0 } }, + saml: { saml1: { order: 1, realm: 'realm1' } }, + }, + }), [ - { type: 'basic', name: 'basic1', options: { order: 0, showInSelector: true } }, - { type: 'saml', name: 'saml1', options: { order: 1, showInSelector: true } }, + { + name: 'basic1', + type: 'basic', + usesLoginForm: true, + icon: 'logoElastic', + description: 'Log in with Elasticsearch', + }, ], - [], ], - // selector is enabled, but only basic/token is available, providers shouldn't be returned. + // selector is enabled, but only basic/token is available and should be returned. [ - true, - [{ type: 'basic', name: 'basic1', options: { order: 0, showInSelector: true } }], - [], + getAuthcConfig({ + selector: { enabled: true }, + providers: { basic: { basic1: { order: 0 } } }, + }), + [ + { + name: 'basic1', + type: 'basic', + usesLoginForm: true, + icon: 'logoElastic', + description: 'Log in with Elasticsearch', + }, + ], ], - // selector is enabled, non-basic/token providers should be returned + // selector is enabled, all providers should be returned [ - true, + getAuthcConfig({ + selector: { enabled: true }, + providers: { + basic: { + basic1: { + order: 0, + description: 'some-desc1', + hint: 'some-hint1', + icon: 'logoElastic', + }, + }, + saml: { + saml1: { order: 1, description: 'some-desc2', realm: 'realm1', icon: 'some-icon2' }, + saml2: { order: 2, description: 'some-desc3', hint: 'some-hint3', realm: 'realm2' }, + }, + }, + }), [ { type: 'basic', name: 'basic1', - options: { order: 0, showInSelector: true, description: 'some-desc1' }, + description: 'some-desc1', + hint: 'some-hint1', + icon: 'logoElastic', + usesLoginForm: true, }, { type: 'saml', name: 'saml1', - options: { order: 1, showInSelector: true, description: 'some-desc2' }, + description: 'some-desc2', + icon: 'some-icon2', + usesLoginForm: false, }, { type: 'saml', name: 'saml2', - options: { order: 2, showInSelector: true, description: 'some-desc3' }, + description: 'some-desc3', + hint: 'some-hint3', + usesLoginForm: false, }, ], - [ - { type: 'saml', name: 'saml1', description: 'some-desc2' }, - { type: 'saml', name: 'saml2', description: 'some-desc3' }, - ], ], - // selector is enabled, only non-basic/token providers that are enabled in selector should be returned. + // selector is enabled, only providers that are enabled should be returned. [ - true, + getAuthcConfig({ + selector: { enabled: true }, + providers: { + basic: { + basic1: { + order: 0, + description: 'some-desc1', + hint: 'some-hint1', + icon: 'some-icon1', + }, + }, + saml: { + saml1: { + order: 1, + description: 'some-desc2', + realm: 'realm1', + showInSelector: false, + }, + saml2: { + order: 2, + description: 'some-desc3', + hint: 'some-hint3', + icon: 'some-icon3', + realm: 'realm2', + }, + }, + }, + }), [ { type: 'basic', name: 'basic1', - options: { order: 0, showInSelector: true, description: 'some-desc1' }, - }, - { - type: 'saml', - name: 'saml1', - options: { order: 1, showInSelector: false, description: 'some-desc2' }, + description: 'some-desc1', + hint: 'some-hint1', + icon: 'some-icon1', + usesLoginForm: true, }, { type: 'saml', name: 'saml2', - options: { order: 2, showInSelector: true, description: 'some-desc3' }, + description: 'some-desc3', + hint: 'some-hint3', + icon: 'some-icon3', + usesLoginForm: false, }, ], - [{ type: 'saml', name: 'saml2', description: 'some-desc3' }], ], ]; - for (const [selectorEnabled, sortedProviders, expectedProviders] of cases) { - config.authc.selector.enabled = selectorEnabled; - config.authc.sortedProviders = sortedProviders; + for (const [authcConfig, expectedProviders] of cases) { + config.authc = authcConfig; const expectedPayload = expect.objectContaining({ - selector: { enabled: selectorEnabled, providers: expectedProviders }, + selector: { enabled: authcConfig.selector.enabled, providers: expectedProviders }, }); await expect( routeHandler({ core: contextMock } as any, request, kibanaResponseFactory) diff --git a/x-pack/plugins/security/server/routes/views/login.ts b/x-pack/plugins/security/server/routes/views/login.ts index 4d6747de713f7..f72facb2e24cc 100644 --- a/x-pack/plugins/security/server/routes/views/login.ts +++ b/x-pack/plugins/security/server/routes/views/login.ts @@ -55,15 +55,16 @@ export function defineLoginRoutes({ const { allowLogin, layout = 'form' } = license.getFeatures(); const { sortedProviders, selector } = config.authc; - let showLoginForm = false; const providers = []; - for (const { type, name, options } of sortedProviders) { - if (options.showInSelector) { - if (type === 'basic' || type === 'token') { - showLoginForm = true; - } else if (selector.enabled) { - providers.push({ type, name, description: options.description }); - } + for (const { type, name } of sortedProviders) { + // 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]!; + + // Include provider into the list if either selector is enabled or provider uses login form. + const usesLoginForm = type === 'basic' || type === 'token'; + if (showInSelector && (usesLoginForm || selector.enabled)) { + providers.push({ type, name, usesLoginForm, description, hint, icon }); } } @@ -71,7 +72,7 @@ export function defineLoginRoutes({ allowLogin, layout, requiresSecureConnection: config.secureCookies, - showLoginForm, + loginHelp: config.loginHelp, selector: { enabled: selector.enabled, providers }, }; diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 5bb0891516517..ac074d99e9ff5 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -12651,7 +12651,6 @@ "xpack.security.loginPage.esUnavailableTitle": "Elasticsearch クラスターに接続できません", "xpack.security.loginPage.loginProviderDescription": "{providerType}/{providerName} でログイン", "xpack.security.loginPage.loginSelectorErrorMessage": "ログインを実行できませんでした。", - "xpack.security.loginPage.loginSelectorOR": "OR", "xpack.security.loginPage.noLoginMethodsAvailableMessage": "システム管理者にお問い合わせください。", "xpack.security.loginPage.noLoginMethodsAvailableTitle": "ログインが無効です。", "xpack.security.loginPage.requiresSecureConnectionMessage": "システム管理者にお問い合わせください。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index a4f4705364737..60b958623bdc7 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -12655,7 +12655,6 @@ "xpack.security.loginPage.esUnavailableTitle": "无法连接到 Elasticsearch 集群", "xpack.security.loginPage.loginProviderDescription": "使用 {providerType}/{providerName} 登录", "xpack.security.loginPage.loginSelectorErrorMessage": "无法执行登录。", - "xpack.security.loginPage.loginSelectorOR": "或", "xpack.security.loginPage.noLoginMethodsAvailableMessage": "请联系您的管理员。", "xpack.security.loginPage.noLoginMethodsAvailableTitle": "登录已禁用。", "xpack.security.loginPage.requiresSecureConnectionMessage": "请联系您的管理员。",