diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/authenticator.ts b/x-pack/legacy/plugins/security/server/lib/authentication/authenticator.ts index 0c9b09902cca0..177c76b56ff63 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/authenticator.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/authenticator.ts @@ -22,6 +22,7 @@ import { DeauthenticationResult } from './deauthentication_result'; import { Session } from './session'; import { LoginAttempt } from './login_attempt'; import { AuthenticationProviderSpecificOptions } from './providers/base'; +import { Tokens } from './tokens'; interface ProviderSession { provider: string; @@ -56,11 +57,14 @@ function assertRequest(request: Legacy.Request) { */ function getProviderOptions(server: Legacy.Server) { const config = server.config(); + const client = getClient(server); + const log = server.log.bind(server); return { - client: getClient(server), - log: server.log.bind(server), + client, + log, basePath: config.get('server.basePath'), + tokens: new Tokens({ client, log }), }; } diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/base.mock.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/base.mock.ts index 85551c1219bd9..a9132258c75f1 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/base.mock.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/base.mock.ts @@ -4,16 +4,21 @@ * you may not use this file except in compliance with the Elastic License. */ -import { stub } from 'sinon'; +import { stub, createStubInstance } from 'sinon'; +import { Tokens } from '../tokens'; import { AuthenticationProviderOptions } from './base'; export function mockAuthenticationProviderOptions( - providerOptions: Partial = {} + providerOptions: Partial> = {} ) { + const client = { callWithRequest: stub(), callWithInternalUser: stub() }; + const log = stub(); + return { - client: { callWithRequest: stub(), callWithInternalUser: stub() }, - log: stub(), + client, + log, basePath: '/base-path', + tokens: createStubInstance(Tokens), ...providerOptions, }; } diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/base.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/base.ts index 2a01194b48f31..af4028185381c 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/base.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/base.ts @@ -8,6 +8,7 @@ import { Legacy } from 'kibana'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; import { LoginAttempt } from '../login_attempt'; +import { Tokens } from '../tokens'; /** * Describes a request complemented with `loginAttempt` method. @@ -23,6 +24,7 @@ export interface AuthenticationProviderOptions { basePath: string; client: Legacy.Plugins.elasticsearch.Cluster; log: (tags: string[], message: string) => void; + tokens: PublicMethodsOf; } /** diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/basic.test.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/basic.test.ts index f82531f33f0d1..88ae1d76f5b57 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/basic.test.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/basic.test.ts @@ -24,7 +24,7 @@ describe('BasicAuthenticationProvider', () => { let callWithRequest: sinon.SinonStub; beforeEach(() => { const providerOptions = mockAuthenticationProviderOptions(); - callWithRequest = providerOptions.client.callWithRequest as sinon.SinonStub; + callWithRequest = providerOptions.client.callWithRequest; provider = new BasicAuthenticationProvider(providerOptions); }); diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/kerberos.test.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/kerberos.test.ts index cb7846f4c2cec..bbfa1b9f75d0e 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/kerberos.test.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/kerberos.test.ts @@ -17,10 +17,12 @@ describe('KerberosAuthenticationProvider', () => { let provider: KerberosAuthenticationProvider; let callWithRequest: sinon.SinonStub; let callWithInternalUser: sinon.SinonStub; + let tokens: ReturnType['tokens']; beforeEach(() => { const providerOptions = mockAuthenticationProviderOptions(); - callWithRequest = providerOptions.client.callWithRequest as sinon.SinonStub; - callWithInternalUser = providerOptions.client.callWithInternalUser as sinon.SinonStub; + callWithRequest = providerOptions.client.callWithRequest; + callWithInternalUser = providerOptions.client.callWithInternalUser; + tokens = providerOptions.tokens; provider = new KerberosAuthenticationProvider(providerOptions); }); @@ -36,10 +38,12 @@ describe('KerberosAuthenticationProvider', () => { it('does not handle `authorization` header with unsupported schema even if state contains a valid token.', async () => { const request = requestFixture({ headers: { authorization: 'Basic some:credentials' } }); - - const authenticationResult = await provider.authenticate(request, { + const tokenPair = { accessToken: 'some-valid-token', - }); + refreshToken: 'some-valid-refresh-token', + }; + + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.notCalled(callWithRequest); expect(request.headers.authorization).toBe('Basic some:credentials'); @@ -48,14 +52,16 @@ describe('KerberosAuthenticationProvider', () => { it('does not handle requests with non-empty `loginAttempt`.', async () => { const request = requestFixture(); + const tokenPair = { + accessToken: 'some-valid-token', + refreshToken: 'some-valid-refresh-token', + }; const loginAttempt = new LoginAttempt(); loginAttempt.setCredentials('user', 'password'); (request.loginAttempt as sinon.SinonStub).returns(loginAttempt); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'some-valid-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.notCalled(callWithRequest); expect(authenticationResult.notHandled()).toBe(true); @@ -85,9 +91,12 @@ describe('KerberosAuthenticationProvider', () => { it('fails if state is present, but backend does not support Kerberos.', async () => { const request = requestFixture(); + const tokenPair = { accessToken: 'token', refreshToken: 'refresh-token' }; + callWithRequest.withArgs(request, 'shield.authenticate').rejects(Boom.unauthorized()); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - let authenticationResult = await provider.authenticate(request, { accessToken: 'token' }); + let authenticationResult = await provider.authenticate(request, tokenPair); expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toHaveProperty('output.statusCode', 401); expect(authenticationResult.challenges).toBeUndefined(); @@ -96,7 +105,7 @@ describe('KerberosAuthenticationProvider', () => { .withArgs(request, 'shield.authenticate') .rejects(Boom.unauthorized(null, 'Basic')); - authenticationResult = await provider.authenticate(request, { accessToken: 'token' }); + authenticationResult = await provider.authenticate(request, tokenPair); expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toHaveProperty('output.statusCode', 401); expect(authenticationResult.challenges).toBeUndefined(); @@ -126,7 +135,7 @@ describe('KerberosAuthenticationProvider', () => { expect(authenticationResult.challenges).toBeUndefined(); }); - it('gets an access token in exchange to SPNEGO one and stores it in the state.', async () => { + it('gets an token pair in exchange to SPNEGO one and stores it in the state.', async () => { const user = { username: 'user' }; const request = requestFixture({ headers: { authorization: 'negotiate spnego' } }); @@ -137,32 +146,35 @@ describe('KerberosAuthenticationProvider', () => { ) .resolves(user); - callWithRequest - .withArgs(request, 'shield.getAccessToken') - .resolves({ access_token: 'some-token' }); + callWithInternalUser + .withArgs('shield.getAccessToken') + .resolves({ access_token: 'some-token', refresh_token: 'some-refresh-token' }); const authenticationResult = await provider.authenticate(request); - sinon.assert.calledWithExactly(callWithRequest, request, 'shield.getAccessToken', { - body: { grant_type: 'client_credentials' }, + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.getAccessToken', { + body: { grant_type: '_kerberos', kerberos_ticket: 'spnego' }, }); expect(request.headers.authorization).toBe('Bearer some-token'); expect(authenticationResult.succeeded()).toBe(true); expect(authenticationResult.user).toBe(user); - expect(authenticationResult.state).toEqual({ accessToken: 'some-token' }); + expect(authenticationResult.state).toEqual({ + accessToken: 'some-token', + refreshToken: 'some-refresh-token', + }); }); it('fails if could not retrieve an access token in exchange to SPNEGO one.', async () => { const request = requestFixture({ headers: { authorization: 'negotiate spnego' } }); const failureReason = Boom.unauthorized(); - callWithRequest.withArgs(request, 'shield.getAccessToken').rejects(failureReason); + callWithInternalUser.withArgs('shield.getAccessToken').rejects(failureReason); const authenticationResult = await provider.authenticate(request); - sinon.assert.calledWithExactly(callWithRequest, request, 'shield.getAccessToken', { - body: { grant_type: 'client_credentials' }, + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.getAccessToken', { + body: { grant_type: '_kerberos', kerberos_ticket: 'spnego' }, }); expect(request.headers.authorization).toBe('negotiate spnego'); @@ -182,14 +194,14 @@ describe('KerberosAuthenticationProvider', () => { ) .rejects(failureReason); - callWithRequest - .withArgs(request, 'shield.getAccessToken') - .resolves({ access_token: 'some-token' }); + callWithInternalUser + .withArgs('shield.getAccessToken') + .resolves({ access_token: 'some-token', refresh_token: 'some-refresh-token' }); const authenticationResult = await provider.authenticate(request); - sinon.assert.calledWithExactly(callWithRequest, request, 'shield.getAccessToken', { - body: { grant_type: 'client_credentials' }, + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.getAccessToken', { + body: { grant_type: '_kerberos', kerberos_ticket: 'spnego' }, }); expect(request.headers.authorization).toBe('negotiate spnego'); @@ -201,12 +213,14 @@ describe('KerberosAuthenticationProvider', () => { it('succeeds if state contains a valid token.', async () => { const user = { username: 'user' }; const request = requestFixture(); + const tokenPair = { + accessToken: 'some-valid-token', + refreshToken: 'some-valid-refresh-token', + }; callWithRequest.withArgs(request, 'shield.authenticate').resolves(user); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'some-valid-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(request.headers.authorization).toBe('Bearer some-valid-token'); expect(authenticationResult.succeeded()).toBe(true); @@ -214,15 +228,51 @@ describe('KerberosAuthenticationProvider', () => { expect(authenticationResult.state).toBeUndefined(); }); + it('succeeds with valid session even if requiring a token refresh', async () => { + const user = { username: 'user' }; + const request = requestFixture(); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; + + callWithRequest + .withArgs( + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), + 'shield.authenticate' + ) + .rejects(Boom.unauthorized()); + + tokens.refresh + .withArgs(tokenPair.refreshToken) + .resolves({ accessToken: 'newfoo', refreshToken: 'newbar' }); + + callWithRequest + .withArgs( + sinon.match({ headers: { authorization: 'Bearer newfoo' } }), + 'shield.authenticate' + ) + .returns(user); + + const authenticationResult = await provider.authenticate(request, tokenPair); + + sinon.assert.calledTwice(callWithRequest); + sinon.assert.calledOnce(tokens.refresh); + + expect(authenticationResult.succeeded()).toBe(true); + expect(authenticationResult.user).toEqual(user); + expect(authenticationResult.state).toEqual({ accessToken: 'newfoo', refreshToken: 'newbar' }); + expect(request.headers.authorization).toEqual('Bearer newfoo'); + }); + it('fails if token from the state is rejected because of unknown reason.', async () => { const request = requestFixture(); + const tokenPair = { + accessToken: 'some-valid-token', + refreshToken: 'some-valid-refresh-token', + }; const failureReason = Boom.internal('Token is not valid!'); callWithRequest.withArgs(request, 'shield.authenticate').rejects(failureReason); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'some-invalid-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -230,25 +280,27 @@ describe('KerberosAuthenticationProvider', () => { sinon.assert.neverCalledWith(callWithRequest, 'shield.getAccessToken'); }); - it('fails with `Negotiate` challenge if token from the state is expired and backend supports Kerberos.', async () => { + it('fails with `Negotiate` challenge if both access and refresh tokens from the state are expired and backend supports Kerberos.', async () => { const request = requestFixture(); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'some-valid-refresh-token' }; + callWithRequest.rejects(Boom.unauthorized(null, 'Negotiate')); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toHaveProperty('output.statusCode', 401); expect(authenticationResult.challenges).toEqual(['Negotiate']); }); - it('fails with `Negotiate` challenge if access token document is missing and backend supports Kerberos.', async () => { + it('fails with `Negotiate` challenge if both access and refresh token documents are missing and backend supports Kerberos.', async () => { const request = requestFixture({ headers: {} }); + const tokenPair = { accessToken: 'missing-token', refreshToken: 'missing-refresh-token' }; callWithRequest .withArgs( - sinon.match({ headers: { authorization: 'Bearer expired-token' } }), + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), 'shield.authenticate' ) .rejects({ @@ -258,9 +310,9 @@ describe('KerberosAuthenticationProvider', () => { .withArgs(sinon.match({ headers: {} }), 'shield.authenticate') .rejects(Boom.unauthorized(null, 'Negotiate')); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'missing-token', - }); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); + + const authenticationResult = await provider.authenticate(request, tokenPair); expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toHaveProperty('output.statusCode', 401); @@ -296,17 +348,19 @@ describe('KerberosAuthenticationProvider', () => { it('fails if token from `authorization` header is rejected even if state contains a valid one.', async () => { const user = { username: 'user' }; const request = requestFixture({ headers: { authorization: 'Bearer some-invalid-token' } }); + const tokenPair = { + accessToken: 'some-valid-token', + refreshToken: 'some-valid-refresh-token', + }; const failureReason = { statusCode: 401 }; callWithRequest.withArgs(request, 'shield.authenticate').rejects(failureReason); callWithRequest - .withArgs(sinon.match({ headers: { authorization: 'Bearer some-valid-token' } })) + .withArgs(sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } })) .resolves(user); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'some-valid-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toBe(failureReason); @@ -314,55 +368,47 @@ describe('KerberosAuthenticationProvider', () => { }); describe('`deauthenticate` method', () => { - it('returns `notHandled` if state is not presented or does not include access token.', async () => { + it('returns `notHandled` if state is not presented.', async () => { const request = requestFixture(); let deauthenticateResult = await provider.deauthenticate(request); expect(deauthenticateResult.notHandled()).toBe(true); - deauthenticateResult = await provider.deauthenticate(request, {} as any); - expect(deauthenticateResult.notHandled()).toBe(true); - - deauthenticateResult = await provider.deauthenticate(request, { somethingElse: 'x' } as any); + deauthenticateResult = await provider.deauthenticate(request, null); expect(deauthenticateResult.notHandled()).toBe(true); - sinon.assert.notCalled(callWithInternalUser); + sinon.assert.notCalled(tokens.invalidate); }); - it('fails if `deleteAccessToken` call fails.', async () => { + it('fails if `tokens.invalidate` fails', async () => { const request = requestFixture(); - const accessToken = 'x-access-token'; + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - const failureReason = new Error('Unknown error'); - callWithInternalUser.withArgs('shield.deleteAccessToken').rejects(failureReason); + const failureReason = new Error('failed to delete token'); + tokens.invalidate.withArgs(tokenPair).rejects(failureReason); - const authenticationResult = await provider.deauthenticate(request, { - accessToken, - }); + const authenticationResult = await provider.deauthenticate(request, tokenPair); - sinon.assert.calledOnce(callWithInternalUser); - sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { - body: { token: accessToken }, - }); + sinon.assert.calledOnce(tokens.invalidate); + sinon.assert.calledWithExactly(tokens.invalidate, tokenPair); expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toBe(failureReason); }); - it('invalidates access token and redirects to `/logged_out` page.', async () => { + it('redirects to `/logged_out` page if tokens are invalidated successfully.', async () => { const request = requestFixture(); - const accessToken = 'x-access-token'; + const tokenPair = { + accessToken: 'some-valid-token', + refreshToken: 'some-valid-refresh-token', + }; - callWithInternalUser.withArgs('shield.deleteAccessToken').resolves({ invalidated_tokens: 1 }); + tokens.invalidate.withArgs(tokenPair).resolves(); - const authenticationResult = await provider.deauthenticate(request, { - accessToken, - }); + const authenticationResult = await provider.deauthenticate(request, tokenPair); - sinon.assert.calledOnce(callWithInternalUser); - sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { - body: { token: accessToken }, - }); + sinon.assert.calledOnce(tokens.invalidate); + sinon.assert.calledWithExactly(tokens.invalidate, tokenPair); expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe('/logged_out'); diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/kerberos.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/kerberos.ts index cb8ad9496f101..af6122e06e251 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/kerberos.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/kerberos.ts @@ -10,37 +10,13 @@ import { Legacy } from 'kibana'; import { getErrorStatusCode } from '../../errors'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; +import { Tokens, TokenPair } from '../tokens'; import { BaseAuthenticationProvider, RequestWithLoginAttempt } from './base'; /** * The state supported by the provider. */ -interface ProviderState { - /** - * Access token users get in exchange for SPNEGO token and that should be provided with every - * request to Elasticsearch on behalf of the authenticated user. This token will eventually expire. - */ - accessToken: string; -} - -/** - * If request with access token fails with `401 Unauthorized` then this token is no - * longer valid and we should try to refresh it. Another use case that we should - * temporarily support (until elastic/elasticsearch#38866 is fixed) is when token - * document has been removed and ES responds with `500 Internal Server Error`. - * @param err Error returned from Elasticsearch. - */ -function isAccessTokenExpiredError(err?: any) { - const errorStatusCode = getErrorStatusCode(err); - return ( - errorStatusCode === 401 || - (errorStatusCode === 500 && - err && - err.body && - err.body.error && - err.body.error.reason === 'token document is missing and must be present') - ); -} +type ProviderState = TokenPair; /** * Parses request's `Authorization` HTTP header if present and extracts authentication scheme. @@ -92,8 +68,11 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { if (state && authenticationResult.notHandled()) { authenticationResult = await this.authenticateViaState(request, state); - if (authenticationResult.failed() && isAccessTokenExpiredError(authenticationResult.error)) { - authenticationResult = AuthenticationResult.notHandled(); + if ( + authenticationResult.failed() && + Tokens.isAccessTokenExpiredError(authenticationResult.error) + ) { + authenticationResult = await this.authenticateViaRefreshToken(request, state); } } @@ -109,32 +88,18 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { * @param request Request instance. * @param state State value previously stored by the provider. */ - public async deauthenticate(request: Legacy.Request, state?: ProviderState) { + public async deauthenticate(request: Legacy.Request, state?: ProviderState | null) { this.debug(`Trying to deauthenticate user via ${request.url.path}.`); - if (!state || !state.accessToken) { + if (!state) { this.debug('There is no access token invalidate.'); return DeauthenticationResult.notHandled(); } try { - const { - invalidated_tokens: invalidatedAccessTokensCount, - } = await this.options.client.callWithInternalUser('shield.deleteAccessToken', { - body: { token: state.accessToken }, - }); - - if (invalidatedAccessTokensCount === 0) { - this.debug('User access token was already invalidated.'); - } else if (invalidatedAccessTokensCount === 1) { - this.debug('User access token has been successfully invalidated.'); - } else { - this.debug( - `${invalidatedAccessTokensCount} user access tokens were invalidated, this is unexpected.` - ); - } + await this.options.tokens.invalidate(state); } catch (err) { - this.debug(`Failed invalidating user's access token: ${err.message}`); + this.debug(`Failed invalidating access and/or refresh tokens: ${err.message}`); return DeauthenticationResult.failed(err); } @@ -149,12 +114,14 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { private async authenticateWithNegotiateScheme(request: RequestWithLoginAttempt) { this.debug('Trying to authenticate request using "Negotiate" authentication scheme.'); + const [, kerberosTicket] = request.headers.authorization.split(/\s+/); + // First attempt to exchange SPNEGO token for an access token. - let accessToken: string; + let tokens: { access_token: string; refresh_token: string }; try { - accessToken = (await this.options.client.callWithRequest(request, 'shield.getAccessToken', { - body: { grant_type: 'client_credentials' }, - })).access_token; + tokens = await this.options.client.callWithInternalUser('shield.getAccessToken', { + body: { grant_type: '_kerberos', kerberos_ticket: kerberosTicket }, + }); } catch (err) { this.debug(`Failed to exchange SPNEGO token for an access token: ${err.message}`); return AuthenticationResult.failed(err); @@ -164,14 +131,17 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { // Then attempt to query for the user details using the new token const originalAuthorizationHeader = request.headers.authorization; - request.headers.authorization = `Bearer ${accessToken}`; + request.headers.authorization = `Bearer ${tokens.access_token}`; try { const user = await this.options.client.callWithRequest(request, 'shield.authenticate'); this.debug('User has been authenticated with new access token'); - return AuthenticationResult.succeeded(user, { accessToken }); + return AuthenticationResult.succeeded(user, { + accessToken: tokens.access_token, + refreshToken: tokens.refresh_token, + }); } catch (err) { this.debug(`Failed to authenticate request via access token: ${err.message}`); @@ -245,6 +215,53 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { } } + /** + * This method is only called when authentication via access token stored in the state failed because of expired + * token. So we should use refresh token, that is also stored in the state, to extend expired access token and + * authenticate user with it. + * @param request Request instance. + * @param state State value previously stored by the provider. + */ + private async authenticateViaRefreshToken( + request: RequestWithLoginAttempt, + state: ProviderState + ) { + this.debug('Trying to refresh access token.'); + + let refreshedTokenPair: TokenPair | null; + try { + refreshedTokenPair = await this.options.tokens.refresh(state.refreshToken); + } catch (err) { + return AuthenticationResult.failed(err); + } + + // If refresh token is no longer valid, then we should clear session and renegotiate using SPNEGO. + if (refreshedTokenPair === null) { + this.debug('Both access and refresh tokens are expired. Re-initiating SPNEGO handshake.'); + return this.authenticateViaSPNEGO(request, state); + } + + try { + request.headers.authorization = `Bearer ${refreshedTokenPair.accessToken}`; + + const user = await this.options.client.callWithRequest(request, 'shield.authenticate'); + + this.debug('Request has been authenticated via refreshed token.'); + return AuthenticationResult.succeeded(user, refreshedTokenPair); + } catch (err) { + this.debug(`Failed to authenticate user using newly refreshed access token: ${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); + } + } + /** * Tries to query Elasticsearch and see if we can rely on SPNEGO to authenticate user. * @param request Request instance. diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/oidc.test.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/oidc.test.ts index 9868d068cce3a..78a2eee0e5408 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/oidc.test.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/oidc.test.ts @@ -17,11 +17,13 @@ describe('OIDCAuthenticationProvider', () => { let provider: OIDCAuthenticationProvider; let callWithRequest: sinon.SinonStub; let callWithInternalUser: sinon.SinonStub; + let tokens: ReturnType['tokens']; beforeEach(() => { const providerOptions = mockAuthenticationProviderOptions({ basePath: '/test-base-path' }); const providerSpecificOptions = { realm: 'oidc1' }; - callWithRequest = providerOptions.client.callWithRequest as sinon.SinonStub; - callWithInternalUser = providerOptions.client.callWithInternalUser as sinon.SinonStub; + callWithRequest = providerOptions.client.callWithRequest; + callWithInternalUser = providerOptions.client.callWithInternalUser; + tokens = providerOptions.tokens; provider = new OIDCAuthenticationProvider(providerOptions, providerSpecificOptions); }); @@ -327,10 +329,11 @@ describe('OIDCAuthenticationProvider', () => { it('succeeds if token from the state is expired, but has been successfully refreshed.', async () => { const user = { username: 'user' }; const request = requestFixture(); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'valid-refresh-token' }; callWithRequest .withArgs( - sinon.match({ headers: { authorization: 'Bearer expired-token' } }), + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), 'shield.authenticate' ) .rejects({ statusCode: 401 }); @@ -342,16 +345,11 @@ describe('OIDCAuthenticationProvider', () => { ) .resolves(user); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'valid-refresh-token' }, - }) - .resolves({ access_token: 'new-access-token', refresh_token: 'new-refresh-token' }); + tokens.refresh + .withArgs(tokenPair.refreshToken) + .resolves({ accessToken: 'new-access-token', refreshToken: 'new-refresh-token' }); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'valid-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(request.headers.authorization).toBe('Bearer new-access-token'); expect(authenticationResult.succeeded()).toBe(true); @@ -364,10 +362,11 @@ describe('OIDCAuthenticationProvider', () => { it('fails if token from the state is expired and refresh attempt failed too.', async () => { const request = requestFixture(); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'invalid-refresh-token' }; callWithRequest .withArgs( - sinon.match({ headers: { authorization: 'Bearer expired-token' } }), + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), 'shield.authenticate' ) .rejects({ statusCode: 401 }); @@ -376,16 +375,9 @@ describe('OIDCAuthenticationProvider', () => { statusCode: 500, message: 'Something is wrong with refresh token.', }; - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'invalid-refresh-token' }, - }) - .returns(Promise.reject(refreshFailureReason)); + tokens.refresh.withArgs(tokenPair.refreshToken).rejects(refreshFailureReason); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'invalid-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -394,6 +386,7 @@ describe('OIDCAuthenticationProvider', () => { it('redirects to OpenID Connect Provider for non-AJAX requests if refresh token is expired or already refreshed.', async () => { const request = requestFixture({ path: '/some-path', basePath: '/s/foo' }); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; callWithInternalUser.withArgs('shield.oidcPrepare').resolves({ state: 'statevalue', @@ -408,21 +401,14 @@ describe('OIDCAuthenticationProvider', () => { callWithRequest .withArgs( - sinon.match({ headers: { authorization: 'Bearer expired-token' } }), + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), 'shield.authenticate' ) .rejects({ statusCode: 401 }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'expired-refresh-token' }, - }) - .rejects({ statusCode: 400 }); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'expired-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledWithExactly(callWithInternalUser, 'shield.oidcPrepare', { body: { realm: `oidc1` }, @@ -445,29 +431,23 @@ describe('OIDCAuthenticationProvider', () => { it('fails for AJAX requests with user friendly message if refresh token is expired.', async () => { const request = requestFixture({ headers: { 'kbn-xsrf': 'xsrf' } }); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; callWithRequest .withArgs( - sinon.match({ headers: { authorization: 'Bearer expired-token' } }), + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), 'shield.authenticate' ) .rejects({ statusCode: 401 }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'expired-refresh-token' }, - }) - .rejects({ statusCode: 400 }); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'expired-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toEqual( - Boom.badRequest('Both elasticsearch access and refresh tokens are expired.') + Boom.badRequest('Both access and refresh tokens are expired.') ); }); diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/oidc.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/oidc.ts index 7d8d97335b2c1..358c0322bc3ff 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/oidc.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/oidc.ts @@ -8,9 +8,9 @@ import Boom from 'boom'; import type from 'type-detect'; import { Legacy } from 'kibana'; import { canRedirectRequest } from '../../can_redirect_request'; -import { getErrorStatusCode } from '../../errors'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; +import { Tokens, TokenPair } from '../tokens'; import { AuthenticationProviderOptions, BaseAuthenticationProvider, @@ -21,7 +21,7 @@ import { /** * The state supported by the provider (for the OpenID Connect handshake or established session). */ -interface ProviderState { +interface ProviderState extends Partial { /** * Unique identifier of the OpenID Connect request initiated the handshake used to mitigate * replay attacks. @@ -38,18 +38,6 @@ interface ProviderState { * URL to redirect user to after successful OpenID Connect handshake. */ nextURL?: string; - - /** - * Elasticsearch access token issued as the result of successful OpenID Connect handshake and that should be provided - * with every request to Elasticsearch on behalf of the authenticated user. This token will eventually expire. - */ - accessToken?: string; - - /** - * Once the elasticsearch access token expires the refresh token is used to get a new pair of access/refresh tokens - * without any user involvement. If not used this token will eventually expire as well. - */ - refreshToken?: string; } /** @@ -91,23 +79,6 @@ function isOIDCIncomingRequest(request: RequestWithLoginAttempt): request is OID ); } -/** - * Checks the error returned by Elasticsearch as the result of `authenticate` call and returns `true` if request - * has been rejected because of expired token, otherwise returns `false`. - * @param err Error returned from Elasticsearch. - */ -function isAccessTokenExpiredError(err?: any) { - const errorStatusCode = getErrorStatusCode(err); - return ( - errorStatusCode === 401 || - (errorStatusCode === 500 && - err && - err.body && - err.body.error && - err.body.error.reason === 'token document is missing and must be present') - ); -} - /** * Provider that supports authentication using an OpenID Connect realm in Elasticsearch. */ @@ -154,7 +125,10 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { if (state && authenticationResult.notHandled()) { authenticationResult = await this.authenticateViaState(request, state); - if (authenticationResult.failed() && isAccessTokenExpiredError(authenticationResult.error)) { + if ( + authenticationResult.failed() && + Tokens.isAccessTokenExpiredError(authenticationResult.error) + ) { authenticationResult = await this.authenticateViaRefreshToken(request, state); } } @@ -406,29 +380,41 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { return AuthenticationResult.notHandled(); } + let refreshedTokenPair: TokenPair | null; try { - // Token should be refreshed by the same user that obtained that token. - const { - access_token: newAccessToken, - refresh_token: newRefreshToken, - } = await this.options.client.callWithInternalUser('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: refreshToken }, - }); + refreshedTokenPair = await this.options.tokens.refresh(refreshToken); + } catch (err) { + return AuthenticationResult.failed(err); + } + + // When user has neither valid access nor refresh token, the only way to resolve this issue is to redirect + // user to OpenID Connect provider, re-initiate the authentication flow and get a new access/refresh token + // pair as result. Obviously we can't do that for AJAX requests, so we just reply with `400` and clear error + // message. There are two reasons for `400` and not `401`: Elasticsearch search responds with `400` so it + // seems logical to do the same on Kibana side and `401` would force user to logout and do full SLO if it's + // supported. + if (refreshedTokenPair === null) { + if (canRedirectRequest(request)) { + this.debug( + 'Both elasticsearch access and refresh tokens are expired. Re-initiating OpenID Connect authentication.' + ); + return this.initiateOIDCAuthentication(request, { realm: this.realm }); + } - this.debug('Elasticsearch access token has been successfully refreshed.'); + return AuthenticationResult.failed( + Boom.badRequest('Both access and refresh tokens are expired.') + ); + } - request.headers.authorization = `Bearer ${newAccessToken}`; + try { + request.headers.authorization = `Bearer ${refreshedTokenPair.accessToken}`; const user = await this.options.client.callWithRequest(request, 'shield.authenticate'); this.debug('Request has been authenticated via refreshed token.'); - - return AuthenticationResult.succeeded(user, { - accessToken: newAccessToken, - refreshToken: newRefreshToken, - }); + return AuthenticationResult.succeeded(user, refreshedTokenPair); } catch (err) { - this.debug(`Failed to refresh elasticsearch access token: ${err.message}`); + this.debug(`Failed to authenticate user using newly refreshed access token: ${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`. @@ -437,38 +423,6 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { // it's called with this request once again down the line (e.g. in the next authentication provider). delete request.headers.authorization; - // There are at least two common cases when refresh token request can fail: - // 1. Refresh token is valid only for 24 hours and if it hasn't been used it expires. - // - // 2. Refresh token is one-time use token and if it has been used already, it is treated in the same way as - // expired token. Even though it's an edge case, there are several perfectly valid scenarios when it can - // happen. E.g. when several simultaneous AJAX request has been sent to Kibana, but elasticsearch access token has - // expired already, so the first request that reaches Kibana uses refresh token to get a new elasticsearch access - // token, but the second concurrent request has no idea about that and tries to refresh access token as well. All - // ends well when first request refreshes the elasticsearch access token and updates session cookie with fresh - // access/refresh token pair. But if user navigates to another page _before_ AJAX request (the one that triggered - // token refresh)responds with updated cookie, then user will have only that old cookie with expired elasticsearch - // access token and refresh token that has been used already. - // - // When user has neither valid access nor refresh token, the only way to resolve this issue is to re-initiate the - // OpenID Connect authentication by requesting a new authentication request to send to the OpenID Connect Provider - // and exchange it's forthcoming response for a new Elasticsearch access/refresh token pair. In case this is an - // AJAX request, we just reply with `400` and clear error message. - // There are two reasons for `400` and not `401`: Elasticsearch search responds with `400` so it seems logical - // to do the same on Kibana side and `401` would force user to logout and do full SLO if it's supported. - if (getErrorStatusCode(err) === 400) { - if (canRedirectRequest(request)) { - this.debug( - 'Both elasticsearch access and refresh tokens are expired. Re-initiating OpenID Connect authentication.' - ); - return this.initiateOIDCAuthentication(request, { realm: this.realm }); - } - - return AuthenticationResult.failed( - Boom.badRequest('Both elasticsearch access and refresh tokens are expired.') - ); - } - return AuthenticationResult.failed(err); } } diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/saml.test.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/saml.test.ts index badc1d6d37af7..7029ddcc17817 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/saml.test.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/saml.test.ts @@ -17,10 +17,12 @@ describe('SAMLAuthenticationProvider', () => { let provider: SAMLAuthenticationProvider; let callWithRequest: sinon.SinonStub; let callWithInternalUser: sinon.SinonStub; + let tokens: ReturnType['tokens']; beforeEach(() => { const providerOptions = mockAuthenticationProviderOptions({ basePath: '/test-base-path' }); - callWithRequest = providerOptions.client.callWithRequest as sinon.SinonStub; - callWithInternalUser = providerOptions.client.callWithInternalUser as sinon.SinonStub; + callWithRequest = providerOptions.client.callWithRequest; + callWithInternalUser = providerOptions.client.callWithInternalUser; + tokens = providerOptions.tokens; provider = new SAMLAuthenticationProvider(providerOptions, { realm: 'test-realm' }); }); @@ -253,6 +255,7 @@ describe('SAMLAuthenticationProvider', () => { it('succeeds if token from the state is expired, but has been successfully refreshed.', async () => { const user = { username: 'user' }; const request = requestFixture(); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'valid-refresh-token' }; callWithRequest .withArgs( @@ -268,16 +271,11 @@ describe('SAMLAuthenticationProvider', () => { ) .resolves(user); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'valid-refresh-token' }, - }) - .resolves({ access_token: 'new-access-token', refresh_token: 'new-refresh-token' }); + tokens.refresh + .withArgs(tokenPair.refreshToken) + .resolves({ accessToken: 'new-access-token', refreshToken: 'new-refresh-token' }); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'valid-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(request.headers.authorization).toBe('Bearer new-access-token'); expect(authenticationResult.succeeded()).toBe(true); @@ -290,6 +288,7 @@ describe('SAMLAuthenticationProvider', () => { it('fails if token from the state is expired and refresh attempt failed with unknown reason too.', async () => { const request = requestFixture(); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'invalid-refresh-token' }; callWithRequest .withArgs( @@ -302,16 +301,9 @@ describe('SAMLAuthenticationProvider', () => { statusCode: 500, message: 'Something is wrong with refresh token.', }; - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'invalid-refresh-token' }, - }) - .rejects(refreshFailureReason); + tokens.refresh.withArgs(tokenPair.refreshToken).rejects(refreshFailureReason); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'invalid-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -320,6 +312,7 @@ describe('SAMLAuthenticationProvider', () => { it('fails for AJAX requests with user friendly message if refresh token is expired.', async () => { const request = requestFixture({ headers: { 'kbn-xsrf': 'xsrf' } }); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; callWithRequest .withArgs( @@ -328,16 +321,9 @@ describe('SAMLAuthenticationProvider', () => { ) .rejects({ statusCode: 401 }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'expired-refresh-token' }, - }) - .rejects({ statusCode: 400 }); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'expired-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -348,6 +334,7 @@ describe('SAMLAuthenticationProvider', () => { it('initiates SAML handshake for non-AJAX requests if access token document is missing.', async () => { const request = requestFixture({ path: '/some-path', basePath: '/s/foo' }); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; callWithInternalUser.withArgs('shield.samlPrepare').resolves({ id: 'some-request-id', @@ -364,16 +351,9 @@ describe('SAMLAuthenticationProvider', () => { body: { error: { reason: 'token document is missing and must be present' } }, }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'expired-refresh-token' }, - }) - .rejects({ statusCode: 400 }); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'expired-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledWithExactly(callWithInternalUser, 'shield.samlPrepare', { body: { realm: 'test-realm' }, @@ -391,6 +371,7 @@ describe('SAMLAuthenticationProvider', () => { it('initiates SAML handshake for non-AJAX requests if refresh token is expired.', async () => { const request = requestFixture({ path: '/some-path', basePath: '/s/foo' }); + const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; callWithInternalUser.withArgs('shield.samlPrepare').resolves({ id: 'some-request-id', @@ -404,16 +385,9 @@ describe('SAMLAuthenticationProvider', () => { ) .rejects({ statusCode: 401 }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'expired-refresh-token' }, - }) - .rejects({ statusCode: 400 }); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'expired-token', - refreshToken: 'expired-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledWithExactly(callWithInternalUser, 'shield.samlPrepare', { body: { realm: 'test-realm' }, @@ -538,6 +512,11 @@ describe('SAMLAuthenticationProvider', () => { it('fails if fails to invalidate existing access/refresh tokens.', async () => { const request = requestFixture({ payload: { SAMLResponse: 'saml-response-xml' } }); + const tokenPair = { + accessToken: 'existing-valid-token', + refreshToken: 'existing-valid-refresh-token', + }; + const user = { username: 'user' }; callWithRequest.withArgs(request, 'shield.authenticate').resolves(user); @@ -546,14 +525,9 @@ describe('SAMLAuthenticationProvider', () => { .resolves({ access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token' }); const failureReason = new Error('Failed to invalidate token!'); - callWithInternalUser - .withArgs('shield.deleteAccessToken', { body: { token: 'existing-valid-token' } }) - .rejects(failureReason); + tokens.invalidate.withArgs(tokenPair).rejects(failureReason); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'existing-valid-token', - refreshToken: 'existing-valid-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledWithExactly(callWithInternalUser, 'shield.samlAuthenticate', { body: { ids: [], content: 'saml-response-xml' }, @@ -565,6 +539,11 @@ describe('SAMLAuthenticationProvider', () => { it('redirects to the home page if new SAML Response is for the same user.', async () => { const request = requestFixture({ payload: { SAMLResponse: 'saml-response-xml' } }); + const tokenPair = { + accessToken: 'existing-valid-token', + refreshToken: 'existing-valid-refresh-token', + }; + const user = { username: 'user', authentication_realm: { name: 'saml1' } }; callWithRequest.withArgs(request, 'shield.authenticate').resolves(user); @@ -572,9 +551,7 @@ describe('SAMLAuthenticationProvider', () => { .withArgs('shield.samlAuthenticate') .resolves({ access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token' }); - const deleteAccessTokenStub = callWithInternalUser - .withArgs('shield.deleteAccessToken') - .resolves({ invalidated_tokens: 1 }); + tokens.invalidate.withArgs(tokenPair).resolves(); const authenticationResult = await provider.authenticate(request, { accessToken: 'existing-valid-token', @@ -585,13 +562,8 @@ describe('SAMLAuthenticationProvider', () => { body: { ids: [], content: 'saml-response-xml' }, }); - sinon.assert.calledTwice(deleteAccessTokenStub); - sinon.assert.calledWithExactly(deleteAccessTokenStub, 'shield.deleteAccessToken', { - body: { token: 'existing-valid-token' }, - }); - sinon.assert.calledWithExactly(deleteAccessTokenStub, 'shield.deleteAccessToken', { - body: { refresh_token: 'existing-valid-refresh-token' }, - }); + sinon.assert.calledOnce(tokens.invalidate); + sinon.assert.calledWithExactly(tokens.invalidate, tokenPair); expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe('/test-base-path/'); @@ -599,10 +571,15 @@ describe('SAMLAuthenticationProvider', () => { it('redirects to `overwritten_session` if new SAML Response is for the another user.', async () => { const request = requestFixture({ payload: { SAMLResponse: 'saml-response-xml' } }); + const tokenPair = { + accessToken: 'existing-valid-token', + refreshToken: 'existing-valid-refresh-token', + }; + const existingUser = { username: 'user', authentication_realm: { name: 'saml1' } }; callWithRequest .withArgs( - sinon.match({ headers: { authorization: 'Bearer existing-valid-token' } }), + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), 'shield.authenticate' ) .resolves(existingUser); @@ -619,26 +596,16 @@ describe('SAMLAuthenticationProvider', () => { .withArgs('shield.samlAuthenticate') .resolves({ access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token' }); - const deleteAccessTokenStub = callWithInternalUser - .withArgs('shield.deleteAccessToken') - .resolves({ invalidated_tokens: 1 }); + tokens.invalidate.withArgs(tokenPair).resolves(); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'existing-valid-token', - refreshToken: 'existing-valid-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledWithExactly(callWithInternalUser, 'shield.samlAuthenticate', { body: { ids: [], content: 'saml-response-xml' }, }); - sinon.assert.calledTwice(deleteAccessTokenStub); - sinon.assert.calledWithExactly(deleteAccessTokenStub, 'shield.deleteAccessToken', { - body: { token: 'existing-valid-token' }, - }); - sinon.assert.calledWithExactly(deleteAccessTokenStub, 'shield.deleteAccessToken', { - body: { refresh_token: 'existing-valid-refresh-token' }, - }); + sinon.assert.calledOnce(tokens.invalidate); + sinon.assert.calledWithExactly(tokens.invalidate, tokenPair); expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe('/test-base-path/overwritten_session'); @@ -646,6 +613,11 @@ describe('SAMLAuthenticationProvider', () => { it('redirects to `overwritten_session` if new SAML Response is for another realm.', async () => { const request = requestFixture({ payload: { SAMLResponse: 'saml-response-xml' } }); + const tokenPair = { + accessToken: 'existing-valid-token', + refreshToken: 'existing-valid-refresh-token', + }; + const existingUser = { username: 'user', authentication_realm: { name: 'saml1' } }; callWithRequest .withArgs( @@ -666,26 +638,16 @@ describe('SAMLAuthenticationProvider', () => { .withArgs('shield.samlAuthenticate') .resolves({ access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token' }); - const deleteAccessTokenStub = callWithInternalUser - .withArgs('shield.deleteAccessToken') - .resolves({ invalidated_tokens: 1 }); + tokens.invalidate.withArgs(tokenPair).resolves(); - const authenticationResult = await provider.authenticate(request, { - accessToken: 'existing-valid-token', - refreshToken: 'existing-valid-refresh-token', - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledWithExactly(callWithInternalUser, 'shield.samlAuthenticate', { body: { ids: [], content: 'saml-response-xml' }, }); - sinon.assert.calledTwice(deleteAccessTokenStub); - sinon.assert.calledWithExactly(deleteAccessTokenStub, 'shield.deleteAccessToken', { - body: { token: 'existing-valid-token' }, - }); - sinon.assert.calledWithExactly(deleteAccessTokenStub, 'shield.deleteAccessToken', { - body: { refresh_token: 'existing-valid-refresh-token' }, - }); + sinon.assert.calledOnce(tokens.invalidate); + sinon.assert.calledWithExactly(tokens.invalidate, tokenPair); expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe('/test-base-path/overwritten_session'); diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/saml.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/saml.ts index 98e85e1096190..c6209abc26bb9 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/saml.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/saml.ts @@ -7,10 +7,10 @@ import Boom from 'boom'; import { Legacy } from 'kibana'; import { canRedirectRequest } from '../../can_redirect_request'; -import { getErrorStatusCode } from '../../errors'; import { AuthenticatedUser } from '../../../../common/model'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; +import { Tokens, TokenPair } from '../tokens'; import { AuthenticationProviderOptions, BaseAuthenticationProvider, @@ -20,7 +20,7 @@ import { /** * The state supported by the provider (for the SAML handshake or established session). */ -interface ProviderState { +interface ProviderState extends Partial { /** * Unique identifier of the SAML request initiated the handshake. */ @@ -30,18 +30,6 @@ interface ProviderState { * URL to redirect user to after successful SAML handshake. */ nextURL?: string; - - /** - * Access token issued as the result of successful SAML handshake and that should be provided with - * every request to Elasticsearch on behalf of the authenticated user. This token will eventually expire. - */ - accessToken?: string; - - /** - * Once access token expires the refresh token is used to get a new pair of access/refresh tokens - * without any user involvement. If not used this token will eventually expire as well. - */ - refreshToken?: string; } /** @@ -58,25 +46,6 @@ type RequestWithSAMLPayload = RequestWithLoginAttempt & { payload: { SAMLResponse: string; RelayState?: string }; }; -/** - * If request with access token fails with `401 Unauthorized` then this token is no - * longer valid and we should try to refresh it. Another use case that we should - * temporarily support (until elastic/elasticsearch#38866 is fixed) is when token - * document has been removed and ES responds with `500 Internal Server Error`. - * @param err Error returned from Elasticsearch. - */ -function isAccessTokenExpiredError(err?: any) { - const errorStatusCode = getErrorStatusCode(err); - return ( - errorStatusCode === 401 || - (errorStatusCode === 500 && - err && - err.body && - err.body.error && - err.body.error.reason === 'token document is missing and must be present') - ); -} - /** * Checks whether request payload contains SAML response from IdP. * @param request Request instance. @@ -142,7 +111,10 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { if (state && authenticationResult.notHandled()) { authenticationResult = await this.authenticateViaState(request, state); - if (authenticationResult.failed() && isAccessTokenExpiredError(authenticationResult.error)) { + if ( + authenticationResult.failed() && + Tokens.isAccessTokenExpiredError(authenticationResult.error) + ) { authenticationResult = await this.authenticateViaRefreshToken(request, state); } } @@ -348,10 +320,11 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { // Now let's invalidate tokens from the existing session. try { - await this.performIdPInitiatedLocalLogout( - existingState.accessToken!, - existingState.refreshToken! - ); + this.debug('Perform IdP initiated local logout.'); + await this.options.tokens.invalidate({ + accessToken: existingState.accessToken!, + refreshToken: existingState.refreshToken!, + }); } catch (err) { this.debug(`Failed to perform IdP initiated local logout: ${err.message}`); return AuthenticationResult.failed(err); @@ -433,28 +406,38 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { return AuthenticationResult.notHandled(); } + let refreshedTokenPair: TokenPair | null; try { - // Token should be refreshed by the same user that obtained that token. - const { - access_token: newAccessToken, - refresh_token: newRefreshToken, - } = await this.options.client.callWithInternalUser('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: refreshToken }, - }); + refreshedTokenPair = await this.options.tokens.refresh(refreshToken); + } catch (err) { + return AuthenticationResult.failed(err); + } - this.debug('Access token has been successfully refreshed.'); + // When user has neither valid access nor refresh token, the only way to resolve this issue is to get new + // SAML LoginResponse and exchange it for a new access/refresh token pair. To do that we initiate a new SAML + // handshake. Obviously we can't do that for AJAX requests, so we just reply with `400` and clear error message. + // There are two reasons for `400` and not `401`: Elasticsearch search responds with `400` so it seems logical + // to do the same on Kibana side and `401` would force user to logout and do full SLO if it's supported. + if (refreshedTokenPair === null) { + if (canRedirectRequest(request)) { + this.debug('Both access and refresh tokens are expired. Re-initiating SAML handshake.'); + return this.authenticateViaHandshake(request); + } + + return AuthenticationResult.failed( + Boom.badRequest('Both access and refresh tokens are expired.') + ); + } - request.headers.authorization = `Bearer ${newAccessToken}`; + try { + request.headers.authorization = `Bearer ${refreshedTokenPair.accessToken}`; const user = await this.options.client.callWithRequest(request, 'shield.authenticate'); this.debug('Request has been authenticated via refreshed token.'); - return AuthenticationResult.succeeded(user, { - accessToken: newAccessToken, - refreshToken: newRefreshToken, - }); + return AuthenticationResult.succeeded(user, refreshedTokenPair); } catch (err) { - this.debug(`Failed to refresh access token: ${err.message}`); + this.debug(`Failed to authenticate user using newly refreshed access token: ${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`. @@ -463,35 +446,6 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { // it's called with this request once again down the line (e.g. in the next authentication provider). delete request.headers.authorization; - // There are at least two common cases when refresh token request can fail: - // 1. Refresh token is valid only for 24 hours and if it hasn't been used it expires. - // - // 2. Refresh token is one-time use token and if it has been used already, it is treated in the same way as - // expired token. Even though it's an edge case, there are several perfectly valid scenarios when it can - // happen. E.g. when several simultaneous AJAX request has been sent to Kibana, but access token has expired - // already, so the first request that reaches Kibana uses refresh token to get a new access token, but the - // second concurrent request has no idea about that and tries to refresh access token as well. All ends well - // when first request refreshes access token and updates session cookie with fresh access/refresh token pair. - // But if user navigates to another page _before_ AJAX request (the one that triggered token refresh) responds - // with updated cookie, then user will have only that old cookie with expired access token and refresh token - // that has been used already. - // - // When user has neither valid access nor refresh token, the only way to resolve this issue is to get new - // SAML LoginResponse and exchange it for a new access/refresh token pair. To do that we initiate a new SAML - // handshake. Obviously we can't do that for AJAX requests, so we just reply with `400` and clear error message. - // There are two reasons for `400` and not `401`: Elasticsearch search responds with `400` so it seems logical - // to do the same on Kibana side and `401` would force user to logout and do full SLO if it's supported. - if (getErrorStatusCode(err) === 400) { - if (canRedirectRequest(request)) { - this.debug('Both access and refresh tokens are expired. Re-initiating SAML handshake.'); - return this.authenticateViaHandshake(request); - } - - return AuthenticationResult.failed( - Boom.badRequest('Both access and refresh tokens are expired.') - ); - } - return AuthenticationResult.failed(err); } } @@ -530,49 +484,6 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { } } - /** - * Invalidates access and refresh tokens without calling `saml/logout`. - * @param accessToken Access token to invalidate. - * @param refreshToken Refresh token to invalidate. - */ - private async performIdPInitiatedLocalLogout(accessToken: string, refreshToken: string) { - this.debug('Local logout has been initiated by the Identity Provider.'); - - // First invalidate old access token. - const { - invalidated_tokens: invalidatedAccessTokensCount, - } = await this.options.client.callWithInternalUser('shield.deleteAccessToken', { - body: { token: accessToken }, - }); - - if (invalidatedAccessTokensCount === 0) { - this.debug('User access token was already invalidated.'); - } else if (invalidatedAccessTokensCount === 1) { - this.debug('User access token has been successfully invalidated.'); - } else { - this.debug( - `${invalidatedAccessTokensCount} user access tokens were invalidated, this is unexpected.` - ); - } - - // Then invalidate old refresh token. - const { - invalidated_tokens: invalidatedRefreshTokensCount, - } = await this.options.client.callWithInternalUser('shield.deleteAccessToken', { - body: { refresh_token: refreshToken }, - }); - - if (invalidatedRefreshTokensCount === 0) { - this.debug('User refresh token was already invalidated.'); - } else if (invalidatedRefreshTokensCount === 1) { - this.debug('User refresh token has been successfully invalidated.'); - } else { - this.debug( - `${invalidatedRefreshTokensCount} user refresh tokens were invalidated, this is unexpected.` - ); - } - } - /** * Calls `saml/logout` with access and refresh tokens and redirects user to the Identity Provider if needed. * @param accessToken Access token to invalidate. diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/token.test.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/token.test.ts index 5e9126aa0d288..4cb088aa00d0b 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/token.test.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/token.test.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import Boom from 'boom'; import { errors } from 'elasticsearch'; import sinon from 'sinon'; import { requestFixture } from '../../__tests__/__fixtures__/request'; @@ -12,18 +13,20 @@ import { mockAuthenticationProviderOptions } from './base.mock'; import { TokenAuthenticationProvider } from './token'; describe('TokenAuthenticationProvider', () => { - describe('`authenticate` method', () => { - let provider: TokenAuthenticationProvider; - let callWithRequest: sinon.SinonStub; - let callWithInternalUser: sinon.SinonStub; - beforeEach(() => { - const providerOptions = mockAuthenticationProviderOptions(); - callWithRequest = providerOptions.client.callWithRequest as sinon.SinonStub; - callWithInternalUser = providerOptions.client.callWithInternalUser as sinon.SinonStub; - - provider = new TokenAuthenticationProvider(providerOptions); - }); + let provider: TokenAuthenticationProvider; + let callWithRequest: sinon.SinonStub; + let callWithInternalUser: sinon.SinonStub; + let tokens: ReturnType['tokens']; + beforeEach(() => { + const providerOptions = mockAuthenticationProviderOptions(); + callWithRequest = providerOptions.client.callWithRequest; + callWithInternalUser = providerOptions.client.callWithInternalUser; + tokens = providerOptions.tokens; + + provider = new TokenAuthenticationProvider(providerOptions); + }); + describe('`authenticate` method', () => { it('does not redirect AJAX requests that can not be authenticated to the login page.', async () => { // Add `kbn-xsrf` header to make `can_redirect_request` think that it's AJAX request and // avoid triggering of redirect logic. @@ -47,15 +50,6 @@ describe('TokenAuthenticationProvider', () => { ); }); - it('does not handle authentication if state exists, but accessToken property is missing.', async () => { - const authenticationResult = await provider.authenticate( - requestFixture({ headers: { 'kbn-xsrf': 'xsrf' } }), - {} - ); - - expect(authenticationResult.notHandled()).toBe(true); - }); - it('succeeds with valid login attempt and stores in session', async () => { const user = { username: 'user' }; const request = requestFixture(); @@ -107,22 +101,20 @@ describe('TokenAuthenticationProvider', () => { const authenticationResult = await provider.authenticate(request); - expect(authenticationResult.state).not.toEqual({ - authorization: request.headers.authorization, - }); + expect(authenticationResult.state).toBeUndefined(); }); it('succeeds if only state is available.', async () => { const request = requestFixture(); - const accessToken = 'foo'; + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; const user = { username: 'user' }; - const authorization = `Bearer ${accessToken}`; + const authorization = `Bearer ${tokenPair.accessToken}`; callWithRequest .withArgs(sinon.match({ headers: { authorization } }), 'shield.authenticate') .resolves(user); - const authenticationResult = await provider.authenticate(request, { accessToken }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(authenticationResult.succeeded()).toBe(true); expect(authenticationResult.user).toEqual(user); @@ -133,16 +125,18 @@ describe('TokenAuthenticationProvider', () => { it('succeeds with valid session even if requiring a token refresh', async () => { const user = { username: 'user' }; const request = requestFixture(); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; callWithRequest - .withArgs(sinon.match({ headers: { authorization: 'Bearer foo' } }), 'shield.authenticate') + .withArgs( + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), + 'shield.authenticate' + ) .rejects({ statusCode: 401 }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'bar' }, - }) - .resolves({ access_token: 'newfoo', refresh_token: 'newbar' }); + tokens.refresh + .withArgs(tokenPair.refreshToken) + .resolves({ accessToken: 'newfoo', refreshToken: 'newbar' }); callWithRequest .withArgs( @@ -151,32 +145,28 @@ describe('TokenAuthenticationProvider', () => { ) .returns(user); - const accessToken = 'foo'; - const refreshToken = 'bar'; - const authenticationResult = await provider.authenticate(request, { - accessToken, - refreshToken, - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledTwice(callWithRequest); - sinon.assert.calledOnce(callWithInternalUser); + sinon.assert.calledOnce(tokens.refresh); expect(authenticationResult.succeeded()).toBe(true); expect(authenticationResult.user).toEqual(user); expect(authenticationResult.state).toEqual({ accessToken: 'newfoo', refreshToken: 'newbar' }); + expect(request.headers.authorization).toEqual('Bearer newfoo'); }); it('does not handle `authorization` header with unsupported schema even if state contains valid credentials.', async () => { const request = requestFixture({ headers: { authorization: 'Basic ***' } }); - const accessToken = 'foo'; + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; const user = { username: 'user' }; - const authorization = `Bearer ${accessToken}`; + const authorization = `Bearer ${tokenPair.accessToken}`; callWithRequest .withArgs(sinon.match({ headers: { authorization } }), 'shield.authenticate') .resolves(user); - const authenticationResult = await provider.authenticate(request, { accessToken }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.notCalled(callWithRequest); expect(request.headers.authorization).toBe('Basic ***'); @@ -184,20 +174,21 @@ describe('TokenAuthenticationProvider', () => { }); it('authenticates only via `authorization` header even if state is available.', async () => { - const accessToken = 'foo'; - const authorization = `Bearer ${accessToken}`; + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; + const authorization = `Bearer foo-from-header`; const request = requestFixture({ headers: { authorization } }); const user = { username: 'user' }; // GetUser will be called with request's `authorization` header. callWithRequest.withArgs(request, 'shield.authenticate').resolves(user); - const authenticationResult = await provider.authenticate(request, { accessToken }); + const authenticationResult = await provider.authenticate(request, tokenPair); expect(authenticationResult.succeeded()).toBe(true); expect(authenticationResult.user).toEqual(user); - expect(authenticationResult.state).not.toEqual({ accessToken }); + expect(authenticationResult.state).toBeUndefined(); sinon.assert.calledOnce(callWithRequest); + expect(request.headers.authorization).toEqual('Bearer foo-from-header'); }); it('fails if token cannot be generated during login attempt', async () => { @@ -270,18 +261,18 @@ describe('TokenAuthenticationProvider', () => { }); it('fails if authentication with token from state fails with unknown error.', async () => { - const accessToken = 'foo'; + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; const request = requestFixture(); const authenticationError = new errors.InternalServerError('something went wrong'); callWithRequest .withArgs( - sinon.match({ headers: { authorization: `Bearer ${accessToken}` } }), + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), 'shield.authenticate' ) .rejects(authenticationError); - const authenticationResult = await provider.authenticate(request, { accessToken }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledOnce(callWithRequest); @@ -294,27 +285,22 @@ describe('TokenAuthenticationProvider', () => { it('fails if token refresh is rejected with unknown error', async () => { const request = requestFixture(); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; callWithRequest - .withArgs(sinon.match({ headers: { authorization: 'Bearer foo' } }), 'shield.authenticate') + .withArgs( + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), + 'shield.authenticate' + ) .rejects({ statusCode: 401 }); const refreshError = new errors.InternalServerError('failed to refresh token'); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'bar' }, - }) - .rejects(refreshError); + tokens.refresh.withArgs(tokenPair.refreshToken).rejects(refreshError); - const accessToken = 'foo'; - const refreshToken = 'bar'; - const authenticationResult = await provider.authenticate(request, { - accessToken, - refreshToken, - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledOnce(callWithRequest); - sinon.assert.calledOnce(callWithInternalUser); + sinon.assert.calledOnce(tokens.refresh); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -325,29 +311,24 @@ describe('TokenAuthenticationProvider', () => { it('redirects non-AJAX requests to /login and clears session if token document is missing', async () => { const request = requestFixture({ path: '/some-path' }); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; callWithRequest - .withArgs(sinon.match({ headers: { authorization: 'Bearer foo' } }), 'shield.authenticate') + .withArgs( + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), + 'shield.authenticate' + ) .rejects({ statusCode: 500, body: { error: { reason: 'token document is missing and must be present' } }, }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'bar' }, - }) - .rejects(new errors.BadRequest('failed to refresh token')); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const accessToken = 'foo'; - const refreshToken = 'bar'; - const authenticationResult = await provider.authenticate(request, { - accessToken, - refreshToken, - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledOnce(callWithRequest); - sinon.assert.calledOnce(callWithInternalUser); + sinon.assert.calledOnce(tokens.refresh); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.redirected()).toBe(true); @@ -357,28 +338,23 @@ describe('TokenAuthenticationProvider', () => { expect(authenticationResult.error).toBeUndefined(); }); - it('redirects non-AJAX requests to /login and clears session if token refresh fails with 400 error', async () => { + it('redirects non-AJAX requests to /login and clears session if token cannot be refreshed', async () => { const request = requestFixture({ path: '/some-path' }); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; callWithRequest - .withArgs(sinon.match({ headers: { authorization: 'Bearer foo' } }), 'shield.authenticate') + .withArgs( + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), + 'shield.authenticate' + ) .rejects({ statusCode: 401 }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'bar' }, - }) - .rejects(new errors.BadRequest('failed to refresh token')); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const accessToken = 'foo'; - const refreshToken = 'bar'; - const authenticationResult = await provider.authenticate(request, { - accessToken, - refreshToken, - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledOnce(callWithRequest); - sinon.assert.calledOnce(callWithInternalUser); + sinon.assert.calledOnce(tokens.refresh); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.redirected()).toBe(true); @@ -388,49 +364,47 @@ describe('TokenAuthenticationProvider', () => { expect(authenticationResult.error).toBeUndefined(); }); - it('does not redirect AJAX requests if token refresh fails with 400 error', async () => { + it('does not redirect AJAX requests if token token cannot be refreshed', async () => { const request = requestFixture({ headers: { 'kbn-xsrf': 'xsrf' }, path: '/some-path' }); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; callWithRequest - .withArgs(sinon.match({ headers: { authorization: 'Bearer foo' } }), 'shield.authenticate') + .withArgs( + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), + 'shield.authenticate' + ) .rejects({ statusCode: 401 }); - const authenticationError = new errors.BadRequest('failed to refresh token'); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'bar' }, - }) - .rejects(authenticationError); + tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); - const accessToken = 'foo'; - const refreshToken = 'bar'; - const authenticationResult = await provider.authenticate(request, { - accessToken, - refreshToken, - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledOnce(callWithRequest); - sinon.assert.calledOnce(callWithInternalUser); + sinon.assert.calledOnce(tokens.refresh); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); - expect(authenticationResult.error).toBe(authenticationError); + expect(authenticationResult.error).toEqual( + Boom.badRequest('Both access and refresh tokens are expired.') + ); expect(authenticationResult.user).toBeUndefined(); expect(authenticationResult.state).toBeUndefined(); }); it('fails if new access token is rejected after successful refresh', async () => { const request = requestFixture(); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; callWithRequest - .withArgs(sinon.match({ headers: { authorization: 'Bearer foo' } }), 'shield.authenticate') + .withArgs( + sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }), + 'shield.authenticate' + ) .rejects({ statusCode: 401 }); - callWithInternalUser - .withArgs('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: 'bar' }, - }) - .resolves({ access_token: 'newfoo', refresh_token: 'newbar' }); + tokens.refresh + .withArgs(tokenPair.refreshToken) + .resolves({ accessToken: 'newfoo', refreshToken: 'newbar' }); const authenticationError = new errors.AuthenticationException('Some error'); callWithRequest @@ -440,15 +414,10 @@ describe('TokenAuthenticationProvider', () => { ) .rejects(authenticationError); - const accessToken = 'foo'; - const refreshToken = 'bar'; - const authenticationResult = await provider.authenticate(request, { - accessToken, - refreshToken, - }); + const authenticationResult = await provider.authenticate(request, tokenPair); sinon.assert.calledTwice(callWithRequest); - sinon.assert.calledOnce(callWithInternalUser); + sinon.assert.calledOnce(tokens.refresh); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -459,153 +428,66 @@ describe('TokenAuthenticationProvider', () => { }); describe('`deauthenticate` method', () => { - let provider: TokenAuthenticationProvider; - let callWithInternalUser: sinon.SinonStub; - beforeEach(() => { - const providerOptions = mockAuthenticationProviderOptions(); - callWithInternalUser = providerOptions.client.callWithInternalUser as sinon.SinonStub; - - provider = new TokenAuthenticationProvider(providerOptions); - }); - - describe('`deauthenticate` method', () => { - it('returns `notHandled` if state is not presented or does not include both access and refresh token.', async () => { - const request = requestFixture(); - const accessToken = 'foo'; - const refreshToken = 'bar'; - - let deauthenticateResult = await provider.deauthenticate(request); - expect(deauthenticateResult.notHandled()).toBe(true); - - deauthenticateResult = await provider.deauthenticate(request, {}); - expect(deauthenticateResult.notHandled()).toBe(true); - - deauthenticateResult = await provider.deauthenticate(request, { accessToken }); - expect(deauthenticateResult.notHandled()).toBe(true); - - deauthenticateResult = await provider.deauthenticate(request, { refreshToken }); - expect(deauthenticateResult.notHandled()).toBe(true); - - sinon.assert.notCalled(callWithInternalUser); - - deauthenticateResult = await provider.deauthenticate(request, { - accessToken, - refreshToken, - }); - expect(deauthenticateResult.notHandled()).toBe(false); - }); - - it('fails if call to delete access token responds with an error', async () => { - const request = requestFixture(); - const accessToken = 'foo'; - const refreshToken = 'bar'; - - const failureReason = new Error('failed to delete token'); - callWithInternalUser - .withArgs('shield.deleteAccessToken', { body: { token: accessToken } }) - .rejects(failureReason); - - const authenticationResult = await provider.deauthenticate(request, { - accessToken, - refreshToken, - }); - - sinon.assert.calledOnce(callWithInternalUser); - sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { - body: { token: accessToken }, - }); + it('returns `notHandled` if state is not presented.', async () => { + const request = requestFixture(); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - expect(authenticationResult.failed()).toBe(true); - expect(authenticationResult.error).toBe(failureReason); - }); + let deauthenticateResult = await provider.deauthenticate(request); + expect(deauthenticateResult.notHandled()).toBe(true); - it('fails if call to delete refresh token responds with an error', async () => { - const request = requestFixture(); - const accessToken = 'foo'; - const refreshToken = 'bar'; + deauthenticateResult = await provider.deauthenticate(request, null); + expect(deauthenticateResult.notHandled()).toBe(true); - callWithInternalUser - .withArgs('shield.deleteAccessToken', { body: { token: accessToken } }) - .returns({ invalidated_tokens: 1 }); + sinon.assert.notCalled(tokens.invalidate); - const failureReason = new Error('failed to delete token'); - callWithInternalUser - .withArgs('shield.deleteAccessToken', { body: { refresh_token: refreshToken } }) - .rejects(failureReason); + deauthenticateResult = await provider.deauthenticate(request, tokenPair); + expect(deauthenticateResult.notHandled()).toBe(false); + }); - const authenticationResult = await provider.deauthenticate(request, { - accessToken, - refreshToken, - }); + it('fails if `tokens.invalidate` fails', async () => { + const request = requestFixture(); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - sinon.assert.calledTwice(callWithInternalUser); - sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { - body: { refresh_token: refreshToken }, - }); + const failureReason = new Error('failed to delete token'); + tokens.invalidate.withArgs(tokenPair).rejects(failureReason); - expect(authenticationResult.failed()).toBe(true); - expect(authenticationResult.error).toBe(failureReason); - }); + const authenticationResult = await provider.deauthenticate(request, tokenPair); - it('redirects to /login if tokens are deleted successfully', async () => { - const request = requestFixture(); - const accessToken = 'foo'; - const refreshToken = 'bar'; + sinon.assert.calledOnce(tokens.invalidate); + sinon.assert.calledWithExactly(tokens.invalidate, tokenPair); - callWithInternalUser - .withArgs('shield.deleteAccessToken', { body: { token: accessToken } }) - .returns({ invalidated_tokens: 1 }); + expect(authenticationResult.failed()).toBe(true); + expect(authenticationResult.error).toBe(failureReason); + }); - callWithInternalUser - .withArgs('shield.deleteAccessToken', { body: { refresh_token: refreshToken } }) - .returns({ invalidated_tokens: 1 }); + it('redirects to /login if tokens are invalidated successfully', async () => { + const request = requestFixture(); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - const authenticationResult = await provider.deauthenticate(request, { - accessToken, - refreshToken, - }); + tokens.invalidate.withArgs(tokenPair).resolves(); - sinon.assert.calledTwice(callWithInternalUser); - sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { - body: { token: accessToken }, - }); - sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { - body: { refresh_token: refreshToken }, - }); + const authenticationResult = await provider.deauthenticate(request, tokenPair); - expect(authenticationResult.redirected()).toBe(true); - expect(authenticationResult.redirectURL).toBe('/base-path/login?msg=LOGGED_OUT'); - }); + sinon.assert.calledOnce(tokens.invalidate); + sinon.assert.calledWithExactly(tokens.invalidate, tokenPair); - it('redirects to /login with optional search parameters if tokens are deleted successfully', async () => { - const request = requestFixture({ search: '?yep' }); - const accessToken = 'foo'; - const refreshToken = 'bar'; + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe('/base-path/login?msg=LOGGED_OUT'); + }); - callWithInternalUser - .withArgs('shield.deleteAccessToken', { body: { token: accessToken } }) - .returns({ created: true }); + it('redirects to /login with optional search parameters if tokens are invalidated successfully', async () => { + const request = requestFixture({ search: '?yep' }); + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - callWithInternalUser - .withArgs('shield.deleteAccessToken', { body: { refresh_token: refreshToken } }) - .returns({ created: true }); + tokens.invalidate.withArgs(tokenPair).resolves(); - const authenticationResult = await provider.deauthenticate(request, { - accessToken, - refreshToken, - }); + const authenticationResult = await provider.deauthenticate(request, tokenPair); - sinon.assert.calledTwice(callWithInternalUser); - sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { - body: { token: accessToken }, - }); - sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { - body: { refresh_token: refreshToken }, - }); + sinon.assert.calledOnce(tokens.invalidate); + sinon.assert.calledWithExactly(tokens.invalidate, tokenPair); - expect(authenticationResult.redirected()).toBe(true); - expect(authenticationResult.redirectURL).toBe('/base-path/login?yep'); - }); + expect(authenticationResult.redirected()).toBe(true); + expect(authenticationResult.redirectURL).toBe('/base-path/login?yep'); }); }); }); diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/providers/token.ts b/x-pack/legacy/plugins/security/server/lib/authentication/providers/token.ts index 627bf764dc580..73e757b71d51d 100644 --- a/x-pack/legacy/plugins/security/server/lib/authentication/providers/token.ts +++ b/x-pack/legacy/plugins/security/server/lib/authentication/providers/token.ts @@ -4,48 +4,18 @@ * you may not use this file except in compliance with the Elastic License. */ +import Boom from 'boom'; import { Legacy } from 'kibana'; import { canRedirectRequest } from '../../can_redirect_request'; -import { getErrorStatusCode } from '../../errors'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; +import { Tokens, TokenPair } from '../tokens'; import { BaseAuthenticationProvider, RequestWithLoginAttempt } from './base'; /** * The state supported by the provider. */ -interface ProviderState { - /** - * Access token issued as the result of successful authentication and that should be provided with - * every request to Elasticsearch on behalf of the authenticated user. This token will eventually expire. - */ - accessToken?: string; - - /** - * Once access token expires the refresh token is used to get a new pair of access/refresh tokens - * without any user involvement. If not used this token will eventually expire as well. - */ - refreshToken?: string; -} - -/** - * If request with access token fails with `401 Unauthorized` then this token is no - * longer valid and we should try to refresh it. Another use case that we should - * temporarily support (until elastic/elasticsearch#38866 is fixed) is when token - * document has been removed and ES responds with `500 Internal Server Error`. - * @param err Error returned from Elasticsearch. - */ -function isAccessTokenExpiredError(err?: any) { - const errorStatusCode = getErrorStatusCode(err); - return ( - errorStatusCode === 401 || - (errorStatusCode === 500 && - err && - err.body && - err.body.error && - err.body.error.reason === 'token document is missing and must be present') - ); -} +type ProviderState = TokenPair; /** * Provider that supports token-based request authentication. @@ -77,7 +47,10 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { // if we still can't attempt auth, try authenticating via state (session token) if (authenticationResult.notHandled() && state) { authenticationResult = await this.authenticateViaState(request, state); - if (authenticationResult.failed() && isAccessTokenExpiredError(authenticationResult.error)) { + if ( + authenticationResult.failed() && + Tokens.isAccessTokenExpiredError(authenticationResult.error) + ) { authenticationResult = await this.authenticateViaRefreshToken(request, state); } } @@ -99,7 +72,7 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { public async deauthenticate(request: Legacy.Request, state?: ProviderState | null) { this.debug(`Trying to deauthenticate user via ${request.url.path}.`); - if (!state || !state.accessToken || !state.refreshToken) { + if (!state) { this.debug('There are no access and refresh tokens to invalidate.'); return DeauthenticationResult.notHandled(); } @@ -107,46 +80,14 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { this.debug('Token-based logout has been initiated by the user.'); try { - // First invalidate the access token. - const { - invalidated_tokens: invalidatedAccessTokensCount, - } = await this.options.client.callWithInternalUser('shield.deleteAccessToken', { - body: { token: state.accessToken }, - }); - - if (invalidatedAccessTokensCount === 0) { - this.debug('User access token was already invalidated.'); - } else if (invalidatedAccessTokensCount === 1) { - this.debug('User access token has been successfully invalidated.'); - } else { - this.debug( - `${invalidatedAccessTokensCount} user access tokens were invalidated, this is unexpected.` - ); - } - - // Then invalidate the refresh token. - const { - invalidated_tokens: invalidatedRefreshTokensCount, - } = await this.options.client.callWithInternalUser('shield.deleteAccessToken', { - body: { refresh_token: state.refreshToken }, - }); - - if (invalidatedRefreshTokensCount === 0) { - this.debug('User refresh token was already invalidated.'); - } else if (invalidatedRefreshTokensCount === 1) { - this.debug('User refresh token has been successfully invalidated.'); - } else { - this.debug( - `${invalidatedRefreshTokensCount} user refresh tokens were invalidated, this is unexpected.` - ); - } - - const queryString = request.url.search || `?msg=LOGGED_OUT`; - return DeauthenticationResult.redirectTo(`${this.options.basePath}/login${queryString}`); + await this.options.tokens.invalidate(state); } catch (err) { this.debug(`Failed invalidating user's access token: ${err.message}`); return DeauthenticationResult.failed(err); } + + const queryString = request.url.search || `?msg=LOGGED_OUT`; + return DeauthenticationResult.redirectTo(`${this.options.basePath}/login${queryString}`); } /** @@ -209,16 +150,6 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { this.debug('Get token API request to Elasticsearch successful'); - // We validate that both access and refresh tokens exist in the response - // so other private methods in this class can rely on them both existing. - if (!accessToken) { - throw new Error('Unexpected response from get token API - no access token present'); - } - - if (!refreshToken) { - throw new Error('Unexpected response from get token API - no refresh token present'); - } - // Then attempt to query for the user details using the new token request.headers.authorization = `Bearer ${accessToken}`; const user = await this.options.client.callWithRequest(request, 'shield.authenticate'); @@ -252,11 +183,6 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { ) { this.debug('Trying to authenticate via state.'); - if (!accessToken) { - this.debug('Access token is not found in state.'); - return AuthenticationResult.notHandled(); - } - try { request.headers.authorization = `Bearer ${accessToken}`; const user = await this.options.client.callWithRequest(request, 'shield.authenticate'); @@ -291,44 +217,36 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { ) { this.debug('Trying to refresh access token.'); - if (!refreshToken) { - this.debug('Refresh token is not found in state.'); - return AuthenticationResult.notHandled(); - } - + let refreshedTokenPair: TokenPair | null; try { - // Token must be refreshed by the same user that obtained that token, the - // kibana system user. - const { - access_token: newAccessToken, - refresh_token: newRefreshToken, - } = await this.options.client.callWithInternalUser('shield.getAccessToken', { - body: { grant_type: 'refresh_token', refresh_token: refreshToken }, - }); + refreshedTokenPair = await this.options.tokens.refresh(refreshToken); + } catch (err) { + return AuthenticationResult.failed(err); + } - this.debug(`Request to refresh token via Elasticsearch's get token API successful`); + // If refresh token is no longer valid, then we should clear session and redirect user to the + // login page to re-authenticate, or fail if redirect isn't possible. + if (refreshedTokenPair === null) { + if (canRedirectRequest(request)) { + this.debug('Clearing session since both access and refresh tokens are expired.'); - // We validate that both access and refresh tokens exist in the response - // so other private methods in this class can rely on them both existing. - if (!newAccessToken) { - throw new Error('Unexpected response from get token API - no access token present'); + // Set state to `null` to let `Authenticator` know that we want to clear current session. + return AuthenticationResult.redirectTo(this.getLoginPageURL(request), null); } - if (!newRefreshToken) { - throw new Error('Unexpected response from get token API - no refresh token present'); - } + return AuthenticationResult.failed( + Boom.badRequest('Both access and refresh tokens are expired.') + ); + } - request.headers.authorization = `Bearer ${newAccessToken}`; + try { + request.headers.authorization = `Bearer ${refreshedTokenPair.accessToken}`; const user = await this.options.client.callWithRequest(request, 'shield.authenticate'); this.debug('Request has been authenticated via refreshed token.'); - - return AuthenticationResult.succeeded(user, { - accessToken: newAccessToken, - refreshToken: newRefreshToken, - }); + return AuthenticationResult.succeeded(user, refreshedTokenPair); } catch (err) { - this.debug(`Failed to refresh access token: ${err.message}`); + this.debug(`Failed to authenticate user using newly refreshed access token: ${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`. @@ -337,15 +255,6 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { // it's called with this request once again down the line (e.g. in the next authentication provider). delete request.headers.authorization; - // If refresh fails with `400` then refresh token is no longer valid and we should clear session - // and redirect user to the login page to re-authenticate. - if (getErrorStatusCode(err) === 400 && canRedirectRequest(request)) { - this.debug('Clearing session since both access and refresh tokens are expired.'); - - // Set state to `null` to let `Authenticator` know that we want to clear current session. - return AuthenticationResult.redirectTo(this.getLoginPageURL(request), null); - } - return AuthenticationResult.failed(err); } } diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/tokens.test.ts b/x-pack/legacy/plugins/security/server/lib/authentication/tokens.test.ts new file mode 100644 index 0000000000000..9ddb1a80f4956 --- /dev/null +++ b/x-pack/legacy/plugins/security/server/lib/authentication/tokens.test.ts @@ -0,0 +1,211 @@ +/* + * 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 Boom from 'boom'; +import { errors } from 'elasticsearch'; +import sinon from 'sinon'; + +import { Tokens } from './tokens'; + +describe('Tokens', () => { + let tokens: Tokens; + let callWithInternalUser: sinon.SinonStub; + beforeEach(() => { + const client = { callWithRequest: sinon.stub(), callWithInternalUser: sinon.stub() }; + const tokensOptions = { client, log: sinon.stub() }; + callWithInternalUser = tokensOptions.client.callWithInternalUser as sinon.SinonStub; + + tokens = new Tokens(tokensOptions); + }); + + it('isAccessTokenExpiredError() returns `true` only if token expired or its document is missing', () => { + for (const error of [ + {}, + new Error(), + Boom.serverUnavailable(), + Boom.forbidden(), + new errors.InternalServerError(), + new errors.Forbidden(), + { + statusCode: 500, + body: { error: { reason: 'some unknown reason' } }, + }, + ]) { + expect(Tokens.isAccessTokenExpiredError(error)).toBe(false); + } + + for (const error of [ + { statusCode: 401 }, + Boom.unauthorized(), + new errors.AuthenticationException(), + { + statusCode: 500, + body: { error: { reason: 'token document is missing and must be present' } }, + }, + ]) { + expect(Tokens.isAccessTokenExpiredError(error)).toBe(true); + } + }); + + describe('refresh()', () => { + const refreshToken = 'some-refresh-token'; + + it('throws if API call fails with unknown reason', async () => { + const refreshFailureReason = Boom.serverUnavailable('Server is not available'); + callWithInternalUser + .withArgs('shield.getAccessToken', { + body: { grant_type: 'refresh_token', refresh_token: refreshToken }, + }) + .rejects(refreshFailureReason); + + await expect(tokens.refresh(refreshToken)).rejects.toBe(refreshFailureReason); + }); + + it('returns `null` if refresh token is not valid', async () => { + const refreshFailureReason = Boom.badRequest(); + callWithInternalUser + .withArgs('shield.getAccessToken', { + body: { grant_type: 'refresh_token', refresh_token: refreshToken }, + }) + .rejects(refreshFailureReason); + + await expect(tokens.refresh(refreshToken)).resolves.toBe(null); + }); + + it('returns token pair if refresh API call succeeds', async () => { + const tokenPair = { accessToken: 'access-token', refreshToken: 'refresh-token' }; + callWithInternalUser + .withArgs('shield.getAccessToken', { + body: { grant_type: 'refresh_token', refresh_token: refreshToken }, + }) + .resolves({ access_token: tokenPair.accessToken, refresh_token: tokenPair.refreshToken }); + + await expect(tokens.refresh(refreshToken)).resolves.toEqual(tokenPair); + }); + }); + + describe('invalidate()', () => { + it('throws if call to delete access token responds with an error', async () => { + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; + + const failureReason = new Error('failed to delete token'); + callWithInternalUser + .withArgs('shield.deleteAccessToken', { body: { token: tokenPair.accessToken } }) + .rejects(failureReason); + + callWithInternalUser + .withArgs('shield.deleteAccessToken', { body: { refresh_token: tokenPair.refreshToken } }) + .resolves({ invalidated_tokens: 1 }); + + await expect(tokens.invalidate(tokenPair)).rejects.toBe(failureReason); + + sinon.assert.calledTwice(callWithInternalUser); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { token: tokenPair.accessToken }, + }); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { refresh_token: tokenPair.refreshToken }, + }); + }); + + it('throws if call to delete refresh token responds with an error', async () => { + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; + + const failureReason = new Error('failed to delete token'); + callWithInternalUser + .withArgs('shield.deleteAccessToken', { body: { refresh_token: tokenPair.refreshToken } }) + .rejects(failureReason); + + callWithInternalUser + .withArgs('shield.deleteAccessToken', { body: { token: tokenPair.accessToken } }) + .resolves({ invalidated_tokens: 1 }); + + await expect(tokens.invalidate(tokenPair)).rejects.toBe(failureReason); + + sinon.assert.calledTwice(callWithInternalUser); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { token: tokenPair.accessToken }, + }); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { refresh_token: tokenPair.refreshToken }, + }); + }); + + it('invalidates all provided tokens', async () => { + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; + + callWithInternalUser.withArgs('shield.deleteAccessToken').resolves({ invalidated_tokens: 1 }); + + await expect(tokens.invalidate(tokenPair)).resolves.toBe(undefined); + + sinon.assert.calledTwice(callWithInternalUser); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { token: tokenPair.accessToken }, + }); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { refresh_token: tokenPair.refreshToken }, + }); + }); + + it('invalidates only access token if only access token is provided', async () => { + const tokenPair = { accessToken: 'foo' }; + + callWithInternalUser.withArgs('shield.deleteAccessToken').resolves({ invalidated_tokens: 1 }); + + await expect(tokens.invalidate(tokenPair)).resolves.toBe(undefined); + + sinon.assert.calledOnce(callWithInternalUser); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { token: tokenPair.accessToken }, + }); + }); + + it('invalidates only refresh token if only refresh token is provided', async () => { + const tokenPair = { refreshToken: 'foo' }; + + callWithInternalUser.withArgs('shield.deleteAccessToken').resolves({ invalidated_tokens: 1 }); + + await expect(tokens.invalidate(tokenPair)).resolves.toBe(undefined); + + sinon.assert.calledOnce(callWithInternalUser); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { refresh_token: tokenPair.refreshToken }, + }); + }); + + it('does not fail if none of the tokens were invalidated', async () => { + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; + + callWithInternalUser.withArgs('shield.deleteAccessToken').resolves({ invalidated_tokens: 0 }); + + await expect(tokens.invalidate(tokenPair)).resolves.toBe(undefined); + + sinon.assert.calledTwice(callWithInternalUser); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { token: tokenPair.accessToken }, + }); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { refresh_token: tokenPair.refreshToken }, + }); + }); + + it('does not fail if more than one token per access or refresh token were invalidated', async () => { + const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; + + callWithInternalUser.withArgs('shield.deleteAccessToken').resolves({ invalidated_tokens: 5 }); + + await expect(tokens.invalidate(tokenPair)).resolves.toBe(undefined); + + sinon.assert.calledTwice(callWithInternalUser); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { token: tokenPair.accessToken }, + }); + sinon.assert.calledWithExactly(callWithInternalUser, 'shield.deleteAccessToken', { + body: { refresh_token: tokenPair.refreshToken }, + }); + }); + }); +}); diff --git a/x-pack/legacy/plugins/security/server/lib/authentication/tokens.ts b/x-pack/legacy/plugins/security/server/lib/authentication/tokens.ts new file mode 100644 index 0000000000000..15702036ce6d5 --- /dev/null +++ b/x-pack/legacy/plugins/security/server/lib/authentication/tokens.ts @@ -0,0 +1,172 @@ +/* + * 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 { Legacy } from 'kibana'; +import { getErrorStatusCode } from '../errors'; + +/** + * Represents a pair of access and refresh tokens. + */ +export interface TokenPair { + /** + * Access token issued as the result of successful authentication and that should be provided with + * every request to Elasticsearch on behalf of the authenticated user. This token will eventually expire. + */ + readonly accessToken: string; + + /** + * Once access token expires the refresh token is used to get a new pair of access/refresh tokens + * without any user involvement. If not used this token will eventually expire as well. + */ + readonly refreshToken: string; +} + +/** + * Class responsible for managing access and refresh tokens (refresh, invalidate, etc.) used by + * various authentication providers. + */ +export class Tokens { + constructor( + private readonly options: Readonly<{ + client: Legacy.Plugins.elasticsearch.Cluster; + log: (tags: string[], message: string) => void; + }> + ) {} + + /** + * Tries to exchange provided refresh token to a new pair of access and refresh tokens. + * @param existingRefreshToken Refresh token to send to the refresh token API. + */ + public async refresh(existingRefreshToken: string): Promise { + try { + // Token should be refreshed by the same user that obtained that token. + const { + access_token: accessToken, + refresh_token: refreshToken, + } = await this.options.client.callWithInternalUser('shield.getAccessToken', { + body: { grant_type: 'refresh_token', refresh_token: existingRefreshToken }, + }); + + this.debug('Access token has been successfully refreshed.'); + + return { accessToken, refreshToken }; + } catch (err) { + this.debug(`Failed to refresh access token: ${err.message}`); + + // There are at least two common cases when refresh token request can fail: + // 1. Refresh token is valid only for 24 hours and if it hasn't been used it expires. + // + // 2. Refresh token is one-time use token and if it has been used already, it is treated in the same way as + // expired token. Even though it's an edge case, there are several perfectly valid scenarios when it can + // happen. E.g. when several simultaneous AJAX request has been sent to Kibana, but access token has expired + // already, so the first request that reaches Kibana uses refresh token to get a new access token, but the + // second concurrent request has no idea about that and tries to refresh access token as well. All ends well + // when first request refreshes access token and updates session cookie with fresh access/refresh token pair. + // But if user navigates to another page _before_ AJAX request (the one that triggered token refresh) responds + // with updated cookie, then user will have only that old cookie with expired access token and refresh token + // that has been used already. + // + // Even though the issue is solved to large extent by a predefined 60s window during which ES allows to use the + // same refresh token multiple times yielding the same refreshed access/refresh token pair it's still possible + // to hit the case when refresh token is no longer valid. + if (getErrorStatusCode(err) === 400) { + this.debug('Refresh token is either expired or already used.'); + return null; + } + + throw err; + } + } + + /** + * Tries to invalidate provided access and refresh token pair. At least one of the tokens should + * be specified. + * @param [accessToken] Optional access token to invalidate. + * @param [refreshToken] Optional refresh token to invalidate. + */ + public async invalidate({ accessToken, refreshToken }: Partial) { + this.debug('Invalidating access/refresh token pair.'); + + let invalidationError; + if (refreshToken) { + let invalidatedTokensCount; + try { + invalidatedTokensCount = (await this.options.client.callWithInternalUser( + 'shield.deleteAccessToken', + { body: { refresh_token: refreshToken } } + )).invalidated_tokens; + } catch (err) { + this.debug(`Failed to invalidate refresh token: ${err.message}`); + // We don't re-throw the error here to have a chance to invalidate access token if it's provided. + invalidationError = err; + } + + if (invalidatedTokensCount === 0) { + this.debug('Refresh token was already invalidated.'); + } else if (invalidatedTokensCount === 1) { + this.debug('Refresh token has been successfully invalidated.'); + } else if (invalidatedTokensCount > 1) { + this.debug( + `${invalidatedTokensCount} refresh tokens were invalidated, this is unexpected.` + ); + } + } + + if (accessToken) { + let invalidatedTokensCount; + try { + invalidatedTokensCount = (await this.options.client.callWithInternalUser( + 'shield.deleteAccessToken', + { body: { token: accessToken } } + )).invalidated_tokens; + } catch (err) { + this.debug(`Failed to invalidate access token: ${err.message}`); + invalidationError = err; + } + + if (invalidatedTokensCount === 0) { + this.debug('Access token was already invalidated.'); + } else if (invalidatedTokensCount === 1) { + this.debug('Access token has been successfully invalidated.'); + } else if (invalidatedTokensCount > 1) { + this.debug(`${invalidatedTokensCount} access tokens were invalidated, this is unexpected.`); + } + } + + if (invalidationError) { + throw invalidationError; + } + } + + /** + * Tries to determine whether specified error that occurred while trying to authenticate request + * using access token happened because access token is expired. We treat all `401 Unauthorized` + * as such. Another use case that we should temporarily support (until elastic/elasticsearch#38866 + * is fixed) is when token document has been removed and ES responds with `500 Internal Server Error`. + * @param err Error returned from Elasticsearch. + */ + public static isAccessTokenExpiredError(err?: any) { + const errorStatusCode = getErrorStatusCode(err); + return ( + errorStatusCode === 401 || + (errorStatusCode === 500 && + !!( + err && + err.body && + err.body.error && + err.body.error.reason === 'token document is missing and must be present' + )) + ); + } + + /** + * Logs message with `debug` level and tokens/security related tags. + * @param message Message to log. + */ + private debug(message: string) { + this.options.log(['debug', 'security', 'tokens'], message); + } +} diff --git a/x-pack/test/kerberos_api_integration/apis/security/kerberos_login.ts b/x-pack/test/kerberos_api_integration/apis/security/kerberos_login.ts index e6b3a9445f594..b077050fd1b0d 100644 --- a/x-pack/test/kerberos_api_integration/apis/security/kerberos_login.ts +++ b/x-pack/test/kerberos_api_integration/apis/security/kerberos_login.ts @@ -36,16 +36,10 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) { describe('Kerberos authentication', () => { before(async () => { - // HACK: remove as soon as we have a solution for https://github.com/elastic/elasticsearch/issues/41943. - await getService('esSupertest') - .post('/_security/role/krb5-user') - .send({ cluster: ['cluster:admin/xpack/security/token/create'] }) - .expect(200); - await getService('esSupertest') .post('/_security/role_mapping/krb5') .send({ - roles: ['krb5-user'], + roles: ['kibana_user'], enabled: true, rules: { field: { 'realm.name': 'kerb1' } }, }) @@ -121,7 +115,7 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) { .set('Cookie', sessionCookie.cookieString()) .expect(200, { username: 'tester@TEST.ELASTIC.CO', - roles: ['krb5-user'], + roles: ['kibana_user'], full_name: null, email: null, metadata: { @@ -275,47 +269,69 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) { checkCookieIsSet(sessionCookie); }); - it('AJAX call should initiate SPNEGO and clear existing cookie', async function() { + it('AJAX call should refresh token and update existing cookie', async function() { this.timeout(40000); // Access token expiration is set to 15s for API integration tests. // Let's wait for 20s to make sure token expires. await delay(20000); + // This api call should succeed and automatically refresh token. Returned cookie will contain + // the new access and refresh token pair. const apiResponse = await supertest .get('/api/security/v1/me') .set('kbn-xsrf', 'xxx') .set('Cookie', sessionCookie.cookieString()) - .expect(401); + .expect(200); const cookies = apiResponse.headers['set-cookie']; expect(cookies).to.have.length(1); - checkCookieIsCleared(request.cookie(cookies[0])!); - expect(apiResponse.headers['www-authenticate']).to.be('Negotiate'); + const refreshedCookie = request.cookie(cookies[0])!; + checkCookieIsSet(refreshedCookie); + + // The first new cookie with fresh pair of access and refresh tokens should work. + await supertest + .get('/api/security/v1/me') + .set('kbn-xsrf', 'xxx') + .set('Cookie', refreshedCookie.cookieString()) + .expect(200); + + expect(apiResponse.headers['www-authenticate']).to.be(undefined); }); - it('non-AJAX call should initiate SPNEGO and clear existing cookie', async function() { + it('non-AJAX call should refresh token and update existing cookie', async function() { this.timeout(40000); // Access token expiration is set to 15s for API integration tests. // Let's wait for 20s to make sure token expires. await delay(20000); + // This request should succeed and automatically refresh token. Returned cookie will contain + // the new access and refresh token pair. const nonAjaxResponse = await supertest - .get('/') + .get('/app/kibana') .set('Cookie', sessionCookie.cookieString()) - .expect(401); + .expect(200); const cookies = nonAjaxResponse.headers['set-cookie']; expect(cookies).to.have.length(1); - checkCookieIsCleared(request.cookie(cookies[0])!); - expect(nonAjaxResponse.headers['www-authenticate']).to.be('Negotiate'); + const refreshedCookie = request.cookie(cookies[0])!; + checkCookieIsSet(refreshedCookie); + + // The first new cookie with fresh pair of access and refresh tokens should work. + await supertest + .get('/api/security/v1/me') + .set('kbn-xsrf', 'xxx') + .set('Cookie', refreshedCookie.cookieString()) + .expect(200); + + expect(nonAjaxResponse.headers['www-authenticate']).to.be(undefined); }); }); - describe('API access with missing access token document.', () => { + describe('API access with missing access token document or expired refresh token.', () => { let sessionCookie: Cookie; beforeEach(async () => { @@ -331,8 +347,8 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) { checkCookieIsSet(sessionCookie); // Let's delete tokens from `.security-tokens` index directly to simulate the case when - // Elasticsearch automatically removes access token document from the index after some - // period of time. + // Elasticsearch automatically removes access/refresh token document from the index after + // some period of time. const esResponse = await getService('es').deleteByQuery({ index: '.security-tokens', q: 'doc_type:token', diff --git a/x-pack/test/oidc_api_integration/apis/security/oidc_initiate_auth.js b/x-pack/test/oidc_api_integration/apis/security/oidc_initiate_auth.js index b99cf494fe0a5..7460b1f9e238c 100644 --- a/x-pack/test/oidc_api_integration/apis/security/oidc_initiate_auth.js +++ b/x-pack/test/oidc_api_integration/apis/security/oidc_initiate_auth.js @@ -343,7 +343,7 @@ export default function ({ getService }) { expect(apiResponse.body).to.eql({ error: 'Bad Request', - message: 'Both elasticsearch access and refresh tokens are expired.', + message: 'Both access and refresh tokens are expired.', statusCode: 400 }); });