Skip to content

Commit

Permalink
Review#2: new unit tests, return response headers from login_with if …
Browse files Browse the repository at this point in the history
…provided by the authentication result, make `redirectURLPath` required in user initiated OIDC login attempt, handle part of the review feedback.
  • Loading branch information
azasypkin committed Mar 19, 2020
1 parent d6d4c2d commit a572c52
Show file tree
Hide file tree
Showing 17 changed files with 1,859 additions and 258 deletions.
6 changes: 1 addition & 5 deletions x-pack/plugins/security/common/login_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ import { LoginLayout } from './licensing';

interface LoginSelector {
enabled: boolean;
providers: Array<{
type: string;
name: string;
options: { description?: string; order: number };
}>;
providers: Array<{ type: string; name: string; description?: string }>;
}

export interface LoginState {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@ describe('LoginPage', () => {
selector: {
enabled: true,
providers: [
{ type: 'saml', name: 'saml1', options: { description: 'Login w/SAML', order: 0 } },
{ type: 'pki', name: 'pki1', options: { description: 'Login w/PKI', order: 1 } },
{ type: 'saml', name: 'saml1', description: 'Login w/SAML' },
{ type: 'pki', name: 'pki1', description: 'Login w/PKI' },
],
},
})
Expand Down Expand Up @@ -294,8 +294,8 @@ describe('LoginPage', () => {
selector: {
enabled: true,
providers: [
{ type: 'saml', name: 'saml1', options: { description: 'Login w/SAML', order: 0 } },
{ type: 'pki', name: 'pki1', options: { description: 'Login w/PKI', order: 1 } },
{ type: 'saml', name: 'saml1', description: 'Login w/SAML' },
{ type: 'pki', name: 'pki1', description: 'Login w/PKI' },
],
},
})
Expand Down
34 changes: 18 additions & 16 deletions x-pack/plugins/security/public/authentication/login/login_page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,7 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import {
CoreStart,
FatalErrorsStart,
HttpStart,
IHttpFetchError,
NotificationsStart,
} from 'src/core/public';
import { CoreStart, FatalErrorsStart, HttpStart, NotificationsStart } from 'src/core/public';
import { LoginState } from '../../../common/login_state';
import { BasicLoginForm, DisabledLoginForm } from './components';

Expand Down Expand Up @@ -225,7 +219,7 @@ export class LoginPage extends Component<Props, State> {
message={
<FormattedMessage
id="xpack.security.loginPage.unknownLayoutMessage"
defaultMessage="Refer to the Kibana logs for more details and refresh to try again."
defaultMessage="See the Kibana logs for details and try reloading the page."
/>
}
/>
Expand All @@ -242,7 +236,16 @@ export class LoginPage extends Component<Props, State> {
fullWidth={true}
onClick={() => this.login(provider.type, provider.name)}
>
{provider.options.description ?? `${provider.type}/${provider.name}`}
{provider.description ?? (
<FormattedMessage
id="xpack.security.loginPage.loginProviderDescription"
defaultMessage="Login with {providerType}/{providerName}"
values={{
providerType: provider.type,
providerName: provider.name,
}}
/>
)}
</EuiButton>
));

Expand Down Expand Up @@ -277,18 +280,17 @@ export class LoginPage extends Component<Props, State> {
private login = async (providerType: string, providerName: string) => {
try {
const { location } = await this.props.http.post<{ location: string }>(
`${this.props.http.basePath.serverBasePath}/internal/security/login_with`,
'/internal/security/login_with',
{ body: JSON.stringify({ providerType, providerName, currentURL: window.location.href }) }
);

window.location.href = location;
} catch (err) {
this.props.notifications.toasts.addDanger(
i18n.translate('xpack.security.loginPage.loginSelectorErrorMessage', {
defaultMessage: 'Could not perform login: {message}. Contact your system administrator.',
values: { message: (err as IHttpFetchError).message },
})
);
this.props.notifications.toasts.addError(err, {
title: i18n.translate('xpack.security.loginPage.loginSelectorErrorMessage', {
defaultMessage: 'Could not perform login.',
}),
});
}
};
}
Expand Down
Loading

0 comments on commit a572c52

Please sign in to comment.