From af0bb705fbe4dfbb48f96f02ff5959e5a07e5830 Mon Sep 17 00:00:00 2001 From: Court Ewing Date: Wed, 12 Dec 2018 20:18:39 -0500 Subject: [PATCH] [security] Support alternate auth providers for login (#26979) (#27097) Login is no longer coupled directly to our basic auth provider, so alternative auth providers can now be used with our standard login flow. The LoginAttempt request service is the mechanism for auth providers to integrate with the login flow. --- .../lib/__tests__/__fixtures__/request.js | 8 ++ .../lib/__tests__/__fixtures__/server.js | 1 + .../authentication/__tests__/authenticator.js | 111 +++++++++++------ .../authentication/__tests__/login_attempt.js | 38 ++++++ .../lib/authentication/authenticator.js | 10 ++ .../lib/authentication/login_attempt.js | 48 ++++++++ .../providers/__tests__/basic.js | 37 +++++- .../lib/authentication/providers/basic.js | 115 ++++++++++++------ .../routes/api/v1/__tests__/authenticate.js | 6 +- .../server/routes/api/v1/__tests__/users.js | 11 +- .../server/routes/api/v1/authenticate.js | 6 +- .../security/server/routes/api/v1/users.js | 5 +- 12 files changed, 302 insertions(+), 94 deletions(-) create mode 100644 x-pack/plugins/security/server/lib/authentication/__tests__/login_attempt.js create mode 100644 x-pack/plugins/security/server/lib/authentication/login_attempt.js diff --git a/x-pack/plugins/security/server/lib/__tests__/__fixtures__/request.js b/x-pack/plugins/security/server/lib/__tests__/__fixtures__/request.js index fbd088dcc958..fdcb14011041 100644 --- a/x-pack/plugins/security/server/lib/__tests__/__fixtures__/request.js +++ b/x-pack/plugins/security/server/lib/__tests__/__fixtures__/request.js @@ -5,8 +5,13 @@ */ import url from 'url'; +import { stub } from 'sinon'; +import { LoginAttempt } from '../../authentication/login_attempt'; + export function requestFixture({ headers = { accept: 'something/html' }, + auth = undefined, + params = undefined, path = '/wat', basePath = '', search = '', @@ -14,9 +19,12 @@ export function requestFixture({ } = {}) { return { raw: { req: { headers } }, + auth, headers, + params, url: { path, search }, getBasePath: () => basePath, + loginAttempt: stub().returns(new LoginAttempt()), query: search ? url.parse(search, { parseQueryString: true }).query : {}, payload, state: { user: 'these are the contents of the user client cookie' } diff --git a/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js b/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js index 3814c282a911..4886c0cf89c8 100644 --- a/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js +++ b/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js @@ -13,6 +13,7 @@ export function serverFixture() { expose: stub(), log: stub(), route: stub(), + decorate: stub(), info: { protocol: 'protocol' diff --git a/x-pack/plugins/security/server/lib/authentication/__tests__/authenticator.js b/x-pack/plugins/security/server/lib/authentication/__tests__/authenticator.js index af786bd5faaa..de747f626ca2 100644 --- a/x-pack/plugins/security/server/lib/authentication/__tests__/authenticator.js +++ b/x-pack/plugins/security/server/lib/authentication/__tests__/authenticator.js @@ -12,6 +12,7 @@ import { serverFixture } from '../../__tests__/__fixtures__/server'; import { requestFixture } from '../../__tests__/__fixtures__/request'; import { Session } from '../session'; import { AuthScopeService } from '../../auth_scope_service'; +import { LoginAttempt } from '../login_attempt'; import { initAuthenticator } from '../authenticator'; import * as ClientShield from '../../../../../../server/lib/get_client_shield'; @@ -149,40 +150,56 @@ describe('Authenticator', () => { sinon.assert.calledWith(authorizationMode.initialize, request); }); - it('creates session whenever authentication provider returns state to store.', async () => { + it('creates session whenever authentication provider returns state for system API requests', async () => { const user = { username: 'user' }; - const systemAPIRequest = requestFixture({ headers: { authorization: 'Basic xxx' } }); - const notSystemAPIRequest = requestFixture({ headers: { authorization: 'Basic yyy' } }); + const request = requestFixture(); + const loginAttempt = new LoginAttempt(); + const authorization = `Basic ${Buffer.from('foo:bar').toString('base64')}`; + loginAttempt.setCredentials('foo', 'bar'); + request.loginAttempt.returns(loginAttempt); server.plugins.kibana.systemApi.isSystemApiRequest - .withArgs(systemAPIRequest).returns(true) - .withArgs(notSystemAPIRequest).returns(false); + .withArgs(request).returns(true); cluster.callWithRequest - .withArgs(systemAPIRequest).returns(Promise.resolve(user)) - .withArgs(notSystemAPIRequest).returns(Promise.resolve(user)); + .withArgs(request).returns(Promise.resolve(user)); - const systemAPIAuthenticationResult = await authenticate(systemAPIRequest); + const systemAPIAuthenticationResult = await authenticate(request); expect(systemAPIAuthenticationResult.succeeded()).to.be(true); expect(systemAPIAuthenticationResult.user).to.be.eql({ ...user, scope: [] }); sinon.assert.calledOnce(session.set); - sinon.assert.calledWithExactly(session.set, systemAPIRequest, { - state: { authorization: systemAPIRequest.headers.authorization }, + sinon.assert.calledWithExactly(session.set, request, { + state: { authorization }, provider: 'basic' }); + }); - const notSystemAPIAuthenticationResult = await authenticate(notSystemAPIRequest); + it('creates session whenever authentication provider returns state for non-system API requests', async () => { + const user = { username: 'user' }; + const request = requestFixture(); + const loginAttempt = new LoginAttempt(); + const authorization = `Basic ${Buffer.from('foo:bar').toString('base64')}`; + loginAttempt.setCredentials('foo', 'bar'); + request.loginAttempt.returns(loginAttempt); + + server.plugins.kibana.systemApi.isSystemApiRequest + .withArgs(request).returns(false); + + cluster.callWithRequest + .withArgs(request).returns(Promise.resolve(user)); + + const notSystemAPIAuthenticationResult = await authenticate(request); expect(notSystemAPIAuthenticationResult.succeeded()).to.be(true); expect(notSystemAPIAuthenticationResult.user).to.be.eql({ ...user, scope: [] }); - sinon.assert.calledTwice(session.set); - sinon.assert.calledWithExactly(session.set, notSystemAPIRequest, { - state: { authorization: notSystemAPIRequest.headers.authorization }, + sinon.assert.calledOnce(session.set); + sinon.assert.calledWithExactly(session.set, request, { + state: { authorization }, provider: 'basic' }); }); @@ -263,50 +280,66 @@ describe('Authenticator', () => { sinon.assert.notCalled(session.set); }); - it('replaces existing session with the one returned by authentication provider.', async () => { + it('replaces existing session with the one returned by authentication provider for system API requests', async () => { const user = { username: 'user' }; - const systemAPIRequest = requestFixture({ headers: { authorization: 'Basic xxx-new' } }); - const notSystemAPIRequest = requestFixture({ headers: { authorization: 'Basic yyy-new' } }); + const authorization = `Basic ${Buffer.from('foo:bar').toString('base64')}`; + const request = requestFixture(); + const loginAttempt = new LoginAttempt(); + loginAttempt.setCredentials('foo', 'bar'); + request.loginAttempt.returns(loginAttempt); - session.get.withArgs(systemAPIRequest).returns(Promise.resolve({ - state: { authorization: 'Basic xxx-old' }, - provider: 'basic' - })); - - session.get.withArgs(notSystemAPIRequest).returns(Promise.resolve({ - state: { authorization: 'Basic yyy-old' }, + session.get.withArgs(request).returns(Promise.resolve({ + state: { authorization: 'Basic some-old-token' }, provider: 'basic' })); server.plugins.kibana.systemApi.isSystemApiRequest - .withArgs(systemAPIRequest).returns(true) - .withArgs(notSystemAPIRequest).returns(false); + .withArgs(request).returns(true); cluster.callWithRequest - .withArgs(systemAPIRequest).returns(Promise.resolve(user)) - .withArgs(notSystemAPIRequest).returns(Promise.resolve(user)); + .withArgs(request).returns(Promise.resolve(user)); - const systemAPIAuthenticationResult = await authenticate(systemAPIRequest); - expect(systemAPIAuthenticationResult.succeeded()).to.be(true); - expect(systemAPIAuthenticationResult.user).to.be.eql({ + const authenticationResult = await authenticate(request); + expect(authenticationResult.succeeded()).to.be(true); + expect(authenticationResult.user).to.be.eql({ ...user, scope: [] }); sinon.assert.calledOnce(session.set); - sinon.assert.calledWithExactly(session.set, systemAPIRequest, { - state: { authorization: 'Basic xxx-new' }, + sinon.assert.calledWithExactly(session.set, request, { + state: { authorization }, provider: 'basic' }); + }); - const notSystemAPIAuthenticationResult = await authenticate(notSystemAPIRequest); - expect(notSystemAPIAuthenticationResult.succeeded()).to.be(true); - expect(notSystemAPIAuthenticationResult.user).to.be.eql({ + it('replaces existing session with the one returned by authentication provider for non-system API requests', async () => { + const user = { username: 'user' }; + const authorization = `Basic ${Buffer.from('foo:bar').toString('base64')}`; + const request = requestFixture(); + const loginAttempt = new LoginAttempt(); + loginAttempt.setCredentials('foo', 'bar'); + request.loginAttempt.returns(loginAttempt); + + session.get.withArgs(request).returns(Promise.resolve({ + state: { authorization: 'Basic some-old-token' }, + provider: 'basic' + })); + + server.plugins.kibana.systemApi.isSystemApiRequest + .withArgs(request).returns(false); + + cluster.callWithRequest + .withArgs(request).returns(Promise.resolve(user)); + + const authenticationResult = await authenticate(request); + expect(authenticationResult.succeeded()).to.be(true); + expect(authenticationResult.user).to.be.eql({ ...user, scope: [] }); - sinon.assert.calledTwice(session.set); - sinon.assert.calledWithExactly(session.set, notSystemAPIRequest, { - state: { authorization: 'Basic yyy-new' }, + sinon.assert.calledOnce(session.set); + sinon.assert.calledWithExactly(session.set, request, { + state: { authorization }, provider: 'basic' }); }); diff --git a/x-pack/plugins/security/server/lib/authentication/__tests__/login_attempt.js b/x-pack/plugins/security/server/lib/authentication/__tests__/login_attempt.js new file mode 100644 index 000000000000..36e362a547be --- /dev/null +++ b/x-pack/plugins/security/server/lib/authentication/__tests__/login_attempt.js @@ -0,0 +1,38 @@ +/* + * 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 expect from 'expect.js'; + +import { LoginAttempt } from '../login_attempt'; + +describe('LoginAttempt', () => { + describe('getCredentials()', () => { + it('returns null by default', () => { + const attempt = new LoginAttempt(); + expect(attempt.getCredentials()).to.be(null); + }); + + it('returns a credentials object after credentials are set', () => { + const attempt = new LoginAttempt(); + attempt.setCredentials('foo', 'bar'); + expect(attempt.getCredentials()).to.eql({ username: 'foo', password: 'bar' }); + }); + }); + + describe('setCredentials()', () => { + it('sets the credentials for this login attempt', () => { + const attempt = new LoginAttempt(); + attempt.setCredentials('foo', 'bar'); + expect(attempt.getCredentials()).to.eql({ username: 'foo', password: 'bar' }); + }); + + it('throws if credentials have already been set', () => { + const attempt = new LoginAttempt(); + attempt.setCredentials('foo', 'bar'); + expect(() => attempt.setCredentials()).to.throwError('Credentials for login attempt have already been set'); + }); + }); +}); diff --git a/x-pack/plugins/security/server/lib/authentication/authenticator.js b/x-pack/plugins/security/server/lib/authentication/authenticator.js index 398f187930cf..8da537ae2f64 100644 --- a/x-pack/plugins/security/server/lib/authentication/authenticator.js +++ b/x-pack/plugins/security/server/lib/authentication/authenticator.js @@ -11,6 +11,7 @@ import { SAMLAuthenticationProvider } from './providers/saml'; import { AuthenticationResult } from './authentication_result'; import { DeauthenticationResult } from './deauthentication_result'; import { Session } from './session'; +import { LoginAttempt } from './login_attempt'; // Mapping between provider key defined in the config and authentication // provider class that can handle specific authentication mechanism. @@ -282,6 +283,15 @@ export async function initAuthenticator(server, authorizationMode) { const authScope = new AuthScopeService(); const authenticator = new Authenticator(server, authScope, session, authorizationMode); + const loginAttempts = new WeakMap(); + server.decorate('request', 'loginAttempt', function () { + const request = this; + if (!loginAttempts.has(request)) { + loginAttempts.set(request, new LoginAttempt()); + } + return loginAttempts.get(request); + }); + server.expose('authenticate', (request) => authenticator.authenticate(request)); server.expose('deauthenticate', (request) => authenticator.deauthenticate(request)); server.expose('registerAuthScopeGetter', (scopeExtender) => authScope.registerGetter(scopeExtender)); diff --git a/x-pack/plugins/security/server/lib/authentication/login_attempt.js b/x-pack/plugins/security/server/lib/authentication/login_attempt.js new file mode 100644 index 000000000000..d09ce5738458 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authentication/login_attempt.js @@ -0,0 +1,48 @@ +/* + * 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. + */ + +/** + * Object that represents login credentials + * @typedef {{ + * username: string, + * password: string + * }} LoginCredentials + */ + +/** + * A LoginAttempt represents a single attempt to provide login credentials. + * Once credentials are set, they cannot be changed. + */ +export class LoginAttempt { + /** + * Username and password for login + * @type {?LoginCredentials} + * @protected + */ + _credentials = null; + + /** + * Gets the username and password for this login + * @returns {LoginCredentials} + */ + getCredentials() { + return this._credentials; + } + + /** + * Sets the username and password for this login + * @param {string} username + * @param {string} password + * @returns {LoginCredentials} + */ + setCredentials(username, password) { + if (this._credentials) { + throw new Error('Credentials for login attempt have already been set'); + } + + this._credentials = { username, password }; + } +} diff --git a/x-pack/plugins/security/server/lib/authentication/providers/__tests__/basic.js b/x-pack/plugins/security/server/lib/authentication/providers/__tests__/basic.js index dc0170264aa6..0226c9b36b5c 100644 --- a/x-pack/plugins/security/server/lib/authentication/providers/__tests__/basic.js +++ b/x-pack/plugins/security/server/lib/authentication/providers/__tests__/basic.js @@ -7,6 +7,7 @@ import expect from 'expect.js'; import sinon from 'sinon'; import { requestFixture } from '../../../__tests__/__fixtures__/request'; +import { LoginAttempt } from '../../login_attempt'; import { BasicAuthenticationProvider, BasicCredentials } from '../basic'; function generateAuthorizationHeader(username, password) { @@ -63,6 +64,26 @@ describe('BasicAuthenticationProvider', () => { expect(authenticationResult.notHandled()).to.be(true); }); + it('succeeds with valid login attempt and stores in session', async () => { + const user = { username: 'user' }; + const authorization = generateAuthorizationHeader('user', 'password'); + const request = requestFixture(); + const loginAttempt = new LoginAttempt(); + loginAttempt.setCredentials('user', 'password'); + request.loginAttempt.returns(loginAttempt); + + callWithRequest + .withArgs(request, 'shield.authenticate') + .returns(Promise.resolve(user)); + + const authenticationResult = await provider.authenticate(request); + + expect(authenticationResult.succeeded()).to.be(true); + expect(authenticationResult.user).to.be.eql(user); + expect(authenticationResult.state).to.be.eql({ authorization }); + sinon.assert.calledOnce(callWithRequest); + }); + it('succeeds if only `authorization` header is available.', async () => { const request = BasicCredentials.decorateRequest(requestFixture(), 'user', 'password'); const user = { username: 'user' }; @@ -75,10 +96,22 @@ describe('BasicAuthenticationProvider', () => { expect(authenticationResult.succeeded()).to.be(true); expect(authenticationResult.user).to.be.eql(user); - expect(authenticationResult.state).to.be.eql({ authorization: request.headers.authorization }); sinon.assert.calledOnce(callWithRequest); }); + it('does not return session state for header-based auth', async () => { + const request = BasicCredentials.decorateRequest(requestFixture(), 'user', 'password'); + const user = { username: 'user' }; + + callWithRequest + .withArgs(request, 'shield.authenticate') + .returns(Promise.resolve(user)); + + const authenticationResult = await provider.authenticate(request); + + expect(authenticationResult.state).not.to.eql({ authorization: request.headers.authorization }); + }); + it('succeeds if only state is available.', async () => { const request = requestFixture(); const user = { username: 'user' }; @@ -138,7 +171,7 @@ describe('BasicAuthenticationProvider', () => { expect(authenticationResult.succeeded()).to.be(true); expect(authenticationResult.user).to.be.eql(user); - expect(authenticationResult.state).to.be.eql({ authorization: request.headers.authorization }); + expect(authenticationResult.state).not.to.eql({ authorization: request.headers.authorization }); sinon.assert.calledOnce(callWithRequest); }); }); diff --git a/x-pack/plugins/security/server/lib/authentication/providers/basic.js b/x-pack/plugins/security/server/lib/authentication/providers/basic.js index 7526582dff3e..bd8eb7a3820d 100644 --- a/x-pack/plugins/security/server/lib/authentication/providers/basic.js +++ b/x-pack/plugins/security/server/lib/authentication/providers/basic.js @@ -8,6 +8,37 @@ import { canRedirectRequest } from '../../can_redirect_request'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; +/** + * Utility class that knows how to decorate request with proper Basic authentication headers. + */ +export class BasicCredentials { + /** + * Takes provided `username` and `password`, transforms them into proper `Basic ***` authorization + * header and decorates passed request with it. + * @param {Hapi.Request} request HapiJS request instance. + * @param {string} username User name. + * @param {string} password User password. + * @returns {Hapi.Request} HapiJS request instance decorated with the proper header. + */ + static decorateRequest(request, username, password) { + if (!request || typeof request !== 'object') { + throw new Error('Request should be a valid object.'); + } + + if (!username || typeof username !== 'string') { + throw new Error('Username should be a valid non-empty string.'); + } + + if (!password || typeof password !== 'string') { + throw new Error('Password should be a valid non-empty string.'); + } + + const basicCredentials = Buffer.from(`${username}:${password}`).toString('base64'); + request.headers.authorization = `Basic ${basicCredentials}`; + return request; + } +} + /** * Object that represents available provider options. * @typedef {{ @@ -56,12 +87,19 @@ export class BasicAuthenticationProvider { async authenticate(request, state) { this._options.log(['debug', 'security', 'basic'], `Trying to authenticate user request to ${request.url.path}.`); - let { - authenticationResult, - headerNotRecognized, // eslint-disable-line prefer-const - } = await this._authenticateViaHeader(request); - if (headerNotRecognized) { - return authenticationResult; + // first try from login payload + let authenticationResult = await this._authenticateViaLoginAttempt(request); + + // if there isn't a payload, try header-based auth + if (authenticationResult.notHandled()) { + const { + authenticationResult: headerAuthResult, + headerNotRecognized, + } = await this._authenticateViaHeader(request); + if (headerNotRecognized) { + return headerAuthResult; + } + authenticationResult = headerAuthResult; } if (authenticationResult.notHandled() && state) { @@ -90,6 +128,38 @@ export class BasicAuthenticationProvider { ); } + /** + * Validates whether request contains a login payload and authenticates the + * user if necessary. + * @param {Hapi.Request} request HapiJS request instance. + * @returns {Promise.} + * @private + */ + async _authenticateViaLoginAttempt(request) { + this._options.log(['debug', 'security', 'basic'], 'Trying to authenticate via login attempt.'); + const credentials = request.loginAttempt(request).getCredentials(); + if (!credentials) { + this._options.log(['debug', 'security', 'basic'], 'Username and password not found in payload.'); + return AuthenticationResult.notHandled(); + } + try { + const { username, password } = credentials; + BasicCredentials.decorateRequest(request, username, password); + const user = await this._options.client.callWithRequest(request, 'shield.authenticate'); + this._options.log(['debug', 'security', 'basic'], 'Request has been authenticated via login attempt.'); + return AuthenticationResult.succeeded(user, { authorization: request.headers.authorization }); + } catch(err) { + this._options.log(['debug', 'security', 'basic'], `Failed to authenticate request via login attempt: ${err.message}`); + // Reset `Authorization` header we've just set. We know for sure that it hasn't been defined before, + // otherwise it would have been used or completely rejected by the `authenticateViaHeader`. + // We can't just set `authorization` to `undefined` or `null`, we should remove this property + // entirely, otherwise `authorization` header without value will cause `callWithRequest` to fail if + // it's called with this request once again down the line (e.g. in the next authentication provider). + delete request.headers.authorization; + return AuthenticationResult.failed(err); + } + } + /** * Validates whether request contains `Basic ***` Authorization header and just passes it * forward to Elasticsearch backend. @@ -123,7 +193,7 @@ export class BasicAuthenticationProvider { this._options.log(['debug', 'security', 'basic'], 'Request has been authenticated via header.'); return { - authenticationResult: AuthenticationResult.succeeded(user, { authorization }) + authenticationResult: AuthenticationResult.succeeded(user) }; } catch(err) { this._options.log(['debug', 'security', 'basic'], `Failed to authenticate request via header: ${err.message}`); @@ -171,34 +241,3 @@ export class BasicAuthenticationProvider { } } } - -/** - * Utility class that knows how to decorate request with proper Basic authentication headers. - */ -export class BasicCredentials { - /** - * Takes provided `username` and `password`, transforms them into proper `Basic ***` authorization - * header and decorates passed request with it. - * @param {Hapi.Request} request HapiJS request instance. - * @param {string} username User name. - * @param {string} password User password. - * @returns {Hapi.Request} HapiJS request instance decorated with the proper header. - */ - static decorateRequest(request, username, password) { - if (!request || typeof request !== 'object') { - throw new Error('Request should be a valid object.'); - } - - if (!username || typeof username !== 'string') { - throw new Error('Username should be a valid non-empty string.'); - } - - if (!password || typeof password !== 'string') { - throw new Error('Password should be a valid non-empty string.'); - } - - const basicCredentials = Buffer.from(`${username}:${password}`).toString('base64'); - request.headers.authorization = `Basic ${basicCredentials}`; - return request; - } -} diff --git a/x-pack/plugins/security/server/routes/api/v1/__tests__/authenticate.js b/x-pack/plugins/security/server/routes/api/v1/__tests__/authenticate.js index 2b2c1ecb6434..3da07f04dd31 100644 --- a/x-pack/plugins/security/server/routes/api/v1/__tests__/authenticate.js +++ b/x-pack/plugins/security/server/routes/api/v1/__tests__/authenticate.js @@ -44,13 +44,13 @@ describe('Authentication routes', () => { .firstCall .args[0]; - request = { + request = requestFixture({ headers: {}, payload: { username: 'user', password: 'password' } - }; + }); authenticateStub = serverStub.plugins.security.authenticate.withArgs( - sinon.match(BasicCredentials.decorateRequest({ headers: {} }, 'user', 'password')) + sinon.match(BasicCredentials.decorateRequest(request, 'user', 'password')) ); authorizationModeStub = serverStub.plugins.security.authorization.mode; }); diff --git a/x-pack/plugins/security/server/routes/api/v1/__tests__/users.js b/x-pack/plugins/security/server/routes/api/v1/__tests__/users.js index aa5dfe3480a9..39ab7d7627bd 100644 --- a/x-pack/plugins/security/server/routes/api/v1/__tests__/users.js +++ b/x-pack/plugins/security/server/routes/api/v1/__tests__/users.js @@ -9,6 +9,7 @@ import Joi from 'joi'; import sinon from 'sinon'; import { serverFixture } from '../../../../lib/__tests__/__fixtures__/server'; +import { requestFixture } from '../../../../lib/__tests__/__fixtures__/request'; import { AuthenticationResult } from '../../../../../server/lib/authentication/authentication_result'; import { BasicCredentials } from '../../../../../server/lib/authentication/providers/basic'; import { initUsersApi } from '../users'; @@ -43,12 +44,12 @@ describe('User routes', () => { .firstCall .args[0]; - request = { + request = requestFixture({ headers: {}, auth: { credentials: { username: 'user' } }, params: { username: 'target-user' }, payload: { password: 'old-password', newPassword: 'new-password' } - }; + }); }); it('correctly defines route.', async () => { @@ -74,7 +75,7 @@ describe('User routes', () => { getUserStub = serverStub.plugins.security.getUser .withArgs( - sinon.match(BasicCredentials.decorateRequest({ headers: {} }, 'user', 'old-password')) + sinon.match(BasicCredentials.decorateRequest(request, 'user', 'old-password')) ); }); @@ -99,7 +100,7 @@ describe('User routes', () => { serverStub.plugins.security.authenticate .withArgs( - sinon.match(BasicCredentials.decorateRequest({ headers: {} }, 'user', 'new-password')) + sinon.match(BasicCredentials.decorateRequest(request, 'user', 'new-password')) ) .returns( Promise.resolve(AuthenticationResult.failed(new Error('Something went wrong.'))) @@ -151,7 +152,7 @@ describe('User routes', () => { serverStub.plugins.security.authenticate .withArgs( - sinon.match(BasicCredentials.decorateRequest({ headers: {} }, 'user', 'new-password')) + sinon.match(BasicCredentials.decorateRequest(request, 'user', 'new-password')) ) .returns( Promise.resolve(AuthenticationResult.succeeded({})) diff --git a/x-pack/plugins/security/server/routes/api/v1/authenticate.js b/x-pack/plugins/security/server/routes/api/v1/authenticate.js index 61abc38d3b0d..0469a242aa1b 100644 --- a/x-pack/plugins/security/server/routes/api/v1/authenticate.js +++ b/x-pack/plugins/security/server/routes/api/v1/authenticate.js @@ -7,7 +7,6 @@ import Boom from 'boom'; import Joi from 'joi'; import { wrapError } from '../../../lib/errors'; -import { BasicCredentials } from '../../../../server/lib/authentication/providers/basic'; import { canRedirectRequest } from '../../../lib/can_redirect_request'; export function initAuthenticateApi(server) { @@ -31,9 +30,8 @@ export function initAuthenticateApi(server) { const { username, password } = request.payload; try { - const authenticationResult = await server.plugins.security.authenticate( - BasicCredentials.decorateRequest(request, username, password) - ); + request.loginAttempt().setCredentials(username, password); + const authenticationResult = await server.plugins.security.authenticate(request); if (!authenticationResult.succeeded()) { throw Boom.unauthorized(authenticationResult.error); diff --git a/x-pack/plugins/security/server/routes/api/v1/users.js b/x-pack/plugins/security/server/routes/api/v1/users.js index 17b0cfe7e556..0e2719e87707 100644 --- a/x-pack/plugins/security/server/routes/api/v1/users.js +++ b/x-pack/plugins/security/server/routes/api/v1/users.js @@ -105,9 +105,8 @@ export function initUsersApi(server) { // Now we authenticate user with the new password again updating current session if any. if (isCurrentUser) { - const authenticationResult = await server.plugins.security.authenticate( - BasicCredentials.decorateRequest(request, username, newPassword) - ); + request.loginAttempt().setCredentials(username, newPassword); + const authenticationResult = await server.plugins.security.authenticate(request); if (!authenticationResult.succeeded()) { throw Boom.unauthorized((authenticationResult.error));