diff --git a/x-pack/legacy/server/lib/esjs_shield_plugin.js b/x-pack/legacy/server/lib/esjs_shield_plugin.js index e0a69b5ce538e..880e055c23985 100644 --- a/x-pack/legacy/server/lib/esjs_shield_plugin.js +++ b/x-pack/legacy/server/lib/esjs_shield_plugin.js @@ -300,6 +300,8 @@ * @param {Array.} ids A list of encrypted request tokens returned within SAML * preparation response. * @param {string} content SAML response returned by identity provider. + * @param {string} [realm] Optional string used to identify the name of the OpenID Connect realm + * that should be used to authenticate request. * * @returns {{username: string, access_token: string, expires_in: number}} Object that * includes name of the user, access token to use for any consequent requests that @@ -373,6 +375,8 @@ * @param {string} nonce The nonce parameter that was returned by Elasticsearch in the * preparation response. * @param {string} redirect_uri The URL to where the UA was redirected by the OpenID Connect provider. + * @param {string} [realm] Optional string used to identify the name of the OpenID Connect realm + * that should be used to authenticate request. * * @returns {{username: string, access_token: string, refresh_token; string, expires_in: number}} Object that * includes name of the user, access token to use for any consequent requests that @@ -391,7 +395,7 @@ * * @param {string} token An access token that was created by authenticating to an OpenID Connect realm and * that needs to be invalidated. - * @param {string} refres_token A refresh token that was created by authenticating to an OpenID Connect realm and + * @param {string} refresh_token A refresh token that was created by authenticating to an OpenID Connect realm and * that needs to be invalidated. * * @returns {{redirect?: string}} If the Elasticsearch OpenID Connect realm configuration and the diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts index f15ac103a81a0..7e00d8f282f62 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts @@ -105,7 +105,14 @@ describe('OIDCAuthenticationProvider', () => { sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.oidcAuthenticate', - { body: { state: 'statevalue', nonce: 'noncevalue', redirect_uri: expectedRedirectURI } } + { + body: { + state: 'statevalue', + nonce: 'noncevalue', + redirect_uri: expectedRedirectURI, + realm: 'oidc1', + }, + } ); expect(authenticationResult.redirected()).toBe(true); @@ -180,7 +187,14 @@ describe('OIDCAuthenticationProvider', () => { sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.oidcAuthenticate', - { body: { state: 'statevalue', nonce: 'noncevalue', redirect_uri: expectedRedirectURI } } + { + body: { + state: 'statevalue', + nonce: 'noncevalue', + redirect_uri: expectedRedirectURI, + realm: 'oidc1', + }, + } ); expect(authenticationResult.failed()).toBe(true); diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.ts b/x-pack/plugins/security/server/authentication/providers/oidc.ts index 27945ccd02cd1..ac8f6cc61edfa 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.ts @@ -197,7 +197,12 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { access_token: accessToken, refresh_token: refreshToken, } = await this.options.client.callAsInternalUser('shield.oidcAuthenticate', { - body: { state: stateOIDCState, nonce: stateNonce, redirect_uri: authenticationResponseURI }, + body: { + state: stateOIDCState, + nonce: stateNonce, + redirect_uri: authenticationResponseURI, + realm: this.realm, + }, }); this.logger.debug('Request has been authenticated via OpenID Connect.'); diff --git a/x-pack/plugins/security/server/authentication/providers/saml.test.ts b/x-pack/plugins/security/server/authentication/providers/saml.test.ts index 4d4fa796851d0..71e4f33609a28 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.test.ts @@ -43,9 +43,11 @@ describe('SAMLAuthenticationProvider', () => { it('gets token and redirects user to requested URL if SAML Response is valid.', async () => { const request = httpServerMock.createKibanaRequest(); - mockOptions.client.callAsInternalUser - .withArgs('shield.samlAuthenticate') - .resolves({ access_token: 'some-token', refresh_token: 'some-refresh-token' }); + mockOptions.client.callAsInternalUser.withArgs('shield.samlAuthenticate').resolves({ + username: 'user', + access_token: 'some-token', + refresh_token: 'some-refresh-token', + }); const authenticationResult = await provider.login( request, @@ -56,12 +58,13 @@ describe('SAMLAuthenticationProvider', () => { sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.samlAuthenticate', - { body: { ids: ['some-request-id'], content: 'saml-response-xml' } } + { body: { ids: ['some-request-id'], content: 'saml-response-xml', realm: 'test-realm' } } ); expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe('/test-base-path/some-path'); expect(authenticationResult.state).toEqual({ + username: 'user', accessToken: 'some-token', refreshToken: 'some-refresh-token', }); @@ -120,7 +123,7 @@ describe('SAMLAuthenticationProvider', () => { sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.samlAuthenticate', - { body: { ids: [], content: 'saml-response-xml' } } + { body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' } } ); expect(authenticationResult.redirected()).toBe(true); @@ -148,7 +151,7 @@ describe('SAMLAuthenticationProvider', () => { sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.samlAuthenticate', - { body: { ids: ['some-request-id'], content: 'saml-response-xml' } } + { body: { ids: ['some-request-id'], content: 'saml-response-xml', realm: 'test-realm' } } ); expect(authenticationResult.failed()).toBe(true); @@ -172,54 +175,17 @@ describe('SAMLAuthenticationProvider', () => { const authenticationResult = await provider.login( request, { samlResponse: 'saml-response-xml' }, - { accessToken: 'some-valid-token', refreshToken: 'some-valid-refresh-token' } - ); - - sinon.assert.calledWithExactly( - mockOptions.client.callAsInternalUser, - 'shield.samlAuthenticate', - { body: { ids: [], content: 'saml-response-xml' } } - ); - - expect(authenticationResult.failed()).toBe(true); - expect(authenticationResult.error).toBe(failureReason); - }); - - it('fails if token received in exchange to new SAML Response is rejected.', async () => { - const request = httpServerMock.createKibanaRequest(); - - // Call to `authenticate` using existing valid session. - const user = mockAuthenticatedUser(); - mockScopedClusterClient( - mockOptions.client, - sinon.match({ headers: { authorization: 'Bearer existing-valid-token' } }) - ) - .callAsCurrentUser.withArgs('shield.authenticate') - .resolves(user); - - // Call to `authenticate` with token received in exchange to new SAML payload. - const failureReason = new Error('Access token is invalid!'); - mockScopedClusterClient( - mockOptions.client, - sinon.match({ headers: { authorization: 'Bearer new-invalid-token' } }) - ) - .callAsCurrentUser.withArgs('shield.authenticate') - .rejects(failureReason); - - mockOptions.client.callAsInternalUser - .withArgs('shield.samlAuthenticate') - .resolves({ access_token: 'new-invalid-token', refresh_token: 'new-invalid-token' }); - - const authenticationResult = await provider.login( - request, - { samlResponse: 'saml-response-xml' }, - { accessToken: 'existing-valid-token', refreshToken: 'existing-valid-refresh-token' } + { + username: 'user', + accessToken: 'some-valid-token', + refreshToken: 'some-valid-refresh-token', + } ); sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.samlAuthenticate', - { body: { ids: [], content: 'saml-response-xml' } } + { body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' } } ); expect(authenticationResult.failed()).toBe(true); @@ -228,7 +194,8 @@ describe('SAMLAuthenticationProvider', () => { it('fails if fails to invalidate existing access/refresh tokens.', async () => { const request = httpServerMock.createKibanaRequest(); - const tokenPair = { + const state = { + username: 'user', accessToken: 'existing-valid-token', refreshToken: 'existing-valid-refresh-token', }; @@ -238,61 +205,75 @@ describe('SAMLAuthenticationProvider', () => { .callAsCurrentUser.withArgs('shield.authenticate') .resolves(user); - mockOptions.client.callAsInternalUser - .withArgs('shield.samlAuthenticate') - .resolves({ access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token' }); + mockOptions.client.callAsInternalUser.withArgs('shield.samlAuthenticate').resolves({ + username: 'user', + access_token: 'new-valid-token', + refresh_token: 'new-valid-refresh-token', + }); const failureReason = new Error('Failed to invalidate token!'); - mockOptions.tokens.invalidate.withArgs(tokenPair).rejects(failureReason); + mockOptions.tokens.invalidate.rejects(failureReason); const authenticationResult = await provider.login( request, { samlResponse: 'saml-response-xml' }, - tokenPair + state ); sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.samlAuthenticate', - { body: { ids: [], content: 'saml-response-xml' } } + { body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' } } ); + sinon.assert.calledOnce(mockOptions.tokens.invalidate); + sinon.assert.calledWithExactly(mockOptions.tokens.invalidate, { + accessToken: state.accessToken, + refreshToken: state.refreshToken, + }); + expect(authenticationResult.failed()).toBe(true); expect(authenticationResult.error).toBe(failureReason); }); it('redirects to the home page if new SAML Response is for the same user.', async () => { const request = httpServerMock.createKibanaRequest(); - const tokenPair = { + const state = { + username: 'user', accessToken: 'existing-valid-token', refreshToken: 'existing-valid-refresh-token', }; - const user = { username: 'user', authentication_realm: { name: 'saml1' } }; + const user = { username: 'user' }; mockScopedClusterClient(mockOptions.client) .callAsCurrentUser.withArgs('shield.authenticate') .resolves(user); - mockOptions.client.callAsInternalUser - .withArgs('shield.samlAuthenticate') - .resolves({ access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token' }); + mockOptions.client.callAsInternalUser.withArgs('shield.samlAuthenticate').resolves({ + username: 'user', + access_token: 'new-valid-token', + refresh_token: 'new-valid-refresh-token', + }); - mockOptions.tokens.invalidate.withArgs(tokenPair).resolves(); + mockOptions.tokens.invalidate.resolves(); const authenticationResult = await provider.login( request, { samlResponse: 'saml-response-xml' }, - tokenPair + state ); sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.samlAuthenticate', - { body: { ids: [], content: 'saml-response-xml' } } + { body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' } } ); sinon.assert.calledOnce(mockOptions.tokens.invalidate); - sinon.assert.calledWithExactly(mockOptions.tokens.invalidate, tokenPair); + sinon.assert.calledWithExactly(mockOptions.tokens.invalidate, { + accessToken: state.accessToken, + refreshToken: state.refreshToken, + }); expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe('/base-path/'); @@ -300,95 +281,45 @@ describe('SAMLAuthenticationProvider', () => { it('redirects to `overwritten_session` if new SAML Response is for the another user.', async () => { const request = httpServerMock.createKibanaRequest(); - const tokenPair = { - accessToken: 'existing-valid-token', - refreshToken: 'existing-valid-refresh-token', - }; - - const existingUser = { username: 'user', authentication_realm: { name: 'saml1' } }; - mockScopedClusterClient( - mockOptions.client, - sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }) - ) - .callAsCurrentUser.withArgs('shield.authenticate') - .resolves(existingUser); - - const newUser = { username: 'new-user', authentication_realm: { name: 'saml1' } }; - mockScopedClusterClient( - mockOptions.client, - sinon.match({ headers: { authorization: 'Bearer new-valid-token' } }) - ) - .callAsCurrentUser.withArgs('shield.authenticate') - .resolves(newUser); - - mockOptions.client.callAsInternalUser - .withArgs('shield.samlAuthenticate') - .resolves({ access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token' }); - - mockOptions.tokens.invalidate.withArgs(tokenPair).resolves(); - - const authenticationResult = await provider.login( - request, - { samlResponse: 'saml-response-xml' }, - tokenPair - ); - - sinon.assert.calledWithExactly( - mockOptions.client.callAsInternalUser, - 'shield.samlAuthenticate', - { body: { ids: [], content: 'saml-response-xml' } } - ); - - sinon.assert.calledOnce(mockOptions.tokens.invalidate); - sinon.assert.calledWithExactly(mockOptions.tokens.invalidate, tokenPair); - - expect(authenticationResult.redirected()).toBe(true); - expect(authenticationResult.redirectURL).toBe('/base-path/overwritten_session'); - }); - - it('redirects to `overwritten_session` if new SAML Response is for another realm.', async () => { - const request = httpServerMock.createKibanaRequest(); - const tokenPair = { + const state = { + username: 'user', accessToken: 'existing-valid-token', refreshToken: 'existing-valid-refresh-token', }; - const existingUser = { username: 'user', authentication_realm: { name: 'saml1' } }; + const existingUser = { username: 'user' }; mockScopedClusterClient( mockOptions.client, - sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }) + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) ) .callAsCurrentUser.withArgs('shield.authenticate') .resolves(existingUser); - const newUser = { username: 'user', authentication_realm: { name: 'saml2' } }; - mockScopedClusterClient( - mockOptions.client, - sinon.match({ headers: { authorization: 'Bearer new-valid-token' } }) - ) - .callAsCurrentUser.withArgs('shield.authenticate') - .resolves(newUser); - - mockOptions.client.callAsInternalUser - .withArgs('shield.samlAuthenticate') - .resolves({ access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token' }); + mockOptions.client.callAsInternalUser.withArgs('shield.samlAuthenticate').resolves({ + username: 'new-user', + access_token: 'new-valid-token', + refresh_token: 'new-valid-refresh-token', + }); - mockOptions.tokens.invalidate.withArgs(tokenPair).resolves(); + mockOptions.tokens.invalidate.resolves(); const authenticationResult = await provider.login( request, { samlResponse: 'saml-response-xml' }, - tokenPair + state ); sinon.assert.calledWithExactly( mockOptions.client.callAsInternalUser, 'shield.samlAuthenticate', - { body: { ids: [], content: 'saml-response-xml' } } + { body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' } } ); sinon.assert.calledOnce(mockOptions.tokens.invalidate); - sinon.assert.calledWithExactly(mockOptions.tokens.invalidate, tokenPair); + sinon.assert.calledWithExactly(mockOptions.tokens.invalidate, { + accessToken: state.accessToken, + refreshToken: state.refreshToken, + }); expect(authenticationResult.redirected()).toBe(true); expect(authenticationResult.redirectURL).toBe('/base-path/overwritten_session'); @@ -411,6 +342,7 @@ describe('SAMLAuthenticationProvider', () => { }); const authenticationResult = await provider.authenticate(request, { + username: 'user', accessToken: 'some-valid-token', refreshToken: 'some-valid-refresh-token', }); @@ -463,17 +395,18 @@ describe('SAMLAuthenticationProvider', () => { it('succeeds if state contains a valid token.', async () => { const user = mockAuthenticatedUser(); const request = httpServerMock.createKibanaRequest(); - const tokenPair = { + const state = { + username: 'user', accessToken: 'some-valid-token', refreshToken: 'some-valid-refresh-token', }; - const authorization = `Bearer ${tokenPair.accessToken}`; + const authorization = `Bearer ${state.accessToken}`; mockScopedClusterClient(mockOptions.client, sinon.match({ headers: { authorization } })) .callAsCurrentUser.withArgs('shield.authenticate') .resolves(user); - const authenticationResult = await provider.authenticate(request, tokenPair); + const authenticationResult = await provider.authenticate(request, state); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.succeeded()).toBe(true); @@ -484,7 +417,8 @@ describe('SAMLAuthenticationProvider', () => { it('fails if token from the state is rejected because of unknown reason.', async () => { const request = httpServerMock.createKibanaRequest(); - const tokenPair = { + const state = { + username: 'user', accessToken: 'some-valid-token', refreshToken: 'some-valid-refresh-token', }; @@ -492,12 +426,12 @@ describe('SAMLAuthenticationProvider', () => { const failureReason = { statusCode: 500, message: 'Token is not valid!' }; mockScopedClusterClient( mockOptions.client, - sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }) + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) ) .callAsCurrentUser.withArgs('shield.authenticate') .rejects(failureReason); - const authenticationResult = await provider.authenticate(request, tokenPair); + const authenticationResult = await provider.authenticate(request, state); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -507,11 +441,15 @@ describe('SAMLAuthenticationProvider', () => { it('succeeds if token from the state is expired, but has been successfully refreshed.', async () => { const user = mockAuthenticatedUser(); const request = httpServerMock.createKibanaRequest(); - const tokenPair = { accessToken: 'expired-token', refreshToken: 'valid-refresh-token' }; + const state = { + username: 'user', + accessToken: 'expired-token', + refreshToken: 'valid-refresh-token', + }; mockScopedClusterClient( mockOptions.client, - sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }) + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) ) .callAsCurrentUser.withArgs('shield.authenticate') .rejects({ statusCode: 401 }); @@ -524,10 +462,10 @@ describe('SAMLAuthenticationProvider', () => { .resolves(user); mockOptions.tokens.refresh - .withArgs(tokenPair.refreshToken) + .withArgs(state.refreshToken) .resolves({ accessToken: 'new-access-token', refreshToken: 'new-refresh-token' }); - const authenticationResult = await provider.authenticate(request, tokenPair); + const authenticationResult = await provider.authenticate(request, state); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.succeeded()).toBe(true); @@ -536,6 +474,7 @@ describe('SAMLAuthenticationProvider', () => { }); expect(authenticationResult.user).toBe(user); expect(authenticationResult.state).toEqual({ + username: 'user', accessToken: 'new-access-token', refreshToken: 'new-refresh-token', }); @@ -543,11 +482,15 @@ describe('SAMLAuthenticationProvider', () => { it('fails if token from the state is expired and refresh attempt failed with unknown reason too.', async () => { const request = httpServerMock.createKibanaRequest(); - const tokenPair = { accessToken: 'expired-token', refreshToken: 'invalid-refresh-token' }; + const state = { + username: 'user', + accessToken: 'expired-token', + refreshToken: 'invalid-refresh-token', + }; mockScopedClusterClient( mockOptions.client, - sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }) + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) ) .callAsCurrentUser.withArgs('shield.authenticate') .rejects({ statusCode: 401 }); @@ -556,9 +499,9 @@ describe('SAMLAuthenticationProvider', () => { statusCode: 500, message: 'Something is wrong with refresh token.', }; - mockOptions.tokens.refresh.withArgs(tokenPair.refreshToken).rejects(refreshFailureReason); + mockOptions.tokens.refresh.withArgs(state.refreshToken).rejects(refreshFailureReason); - const authenticationResult = await provider.authenticate(request, tokenPair); + const authenticationResult = await provider.authenticate(request, state); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -567,18 +510,22 @@ describe('SAMLAuthenticationProvider', () => { it('fails for AJAX requests with user friendly message if refresh token is expired.', async () => { const request = httpServerMock.createKibanaRequest({ headers: { 'kbn-xsrf': 'xsrf' } }); - const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; + const state = { + username: 'user', + accessToken: 'expired-token', + refreshToken: 'expired-refresh-token', + }; mockScopedClusterClient( mockOptions.client, - sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }) + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) ) .callAsCurrentUser.withArgs('shield.authenticate') .rejects({ statusCode: 401 }); - mockOptions.tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); + mockOptions.tokens.refresh.withArgs(state.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, tokenPair); + const authenticationResult = await provider.authenticate(request, state); expect(request.headers).not.toHaveProperty('authorization'); expect(authenticationResult.failed()).toBe(true); @@ -589,7 +536,11 @@ describe('SAMLAuthenticationProvider', () => { it('initiates SAML handshake for non-AJAX requests if access token document is missing.', async () => { const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path' }); - const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; + const state = { + username: 'user', + accessToken: 'expired-token', + refreshToken: 'expired-refresh-token', + }; mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ id: 'some-request-id', @@ -598,7 +549,7 @@ describe('SAMLAuthenticationProvider', () => { mockScopedClusterClient( mockOptions.client, - sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }) + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) ) .callAsCurrentUser.withArgs('shield.authenticate') .rejects({ @@ -606,9 +557,9 @@ describe('SAMLAuthenticationProvider', () => { body: { error: { reason: 'token document is missing and must be present' } }, }); - mockOptions.tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); + mockOptions.tokens.refresh.withArgs(state.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, tokenPair); + const authenticationResult = await provider.authenticate(request, state); sinon.assert.calledWithExactly(mockOptions.client.callAsInternalUser, 'shield.samlPrepare', { body: { realm: 'test-realm' }, @@ -626,7 +577,11 @@ describe('SAMLAuthenticationProvider', () => { it('initiates SAML handshake for non-AJAX requests if refresh token is expired.', async () => { const request = httpServerMock.createKibanaRequest({ path: '/s/foo/some-path' }); - const tokenPair = { accessToken: 'expired-token', refreshToken: 'expired-refresh-token' }; + const state = { + username: 'user', + accessToken: 'expired-token', + refreshToken: 'expired-refresh-token', + }; mockOptions.client.callAsInternalUser.withArgs('shield.samlPrepare').resolves({ id: 'some-request-id', @@ -635,14 +590,14 @@ describe('SAMLAuthenticationProvider', () => { mockScopedClusterClient( mockOptions.client, - sinon.match({ headers: { authorization: `Bearer ${tokenPair.accessToken}` } }) + sinon.match({ headers: { authorization: `Bearer ${state.accessToken}` } }) ) .callAsCurrentUser.withArgs('shield.authenticate') .rejects({ statusCode: 401 }); - mockOptions.tokens.refresh.withArgs(tokenPair.refreshToken).resolves(null); + mockOptions.tokens.refresh.withArgs(state.refreshToken).resolves(null); - const authenticationResult = await provider.authenticate(request, tokenPair); + const authenticationResult = await provider.authenticate(request, state); sinon.assert.calledWithExactly(mockOptions.client.callAsInternalUser, 'shield.samlPrepare', { body: { realm: 'test-realm' }, @@ -743,6 +698,7 @@ describe('SAMLAuthenticationProvider', () => { mockOptions.client.callAsInternalUser.withArgs('shield.samlLogout').rejects(failureReason); const authenticationResult = await provider.logout(request, { + username: 'user', accessToken, refreshToken, }); @@ -787,6 +743,7 @@ describe('SAMLAuthenticationProvider', () => { .resolves({ redirect: null }); const authenticationResult = await provider.logout(request, { + username: 'user', accessToken, refreshToken, }); @@ -810,6 +767,7 @@ describe('SAMLAuthenticationProvider', () => { .resolves({ redirect: undefined }); const authenticationResult = await provider.logout(request, { + username: 'user', accessToken, refreshToken, }); @@ -835,6 +793,7 @@ describe('SAMLAuthenticationProvider', () => { .resolves({ redirect: null }); const authenticationResult = await provider.logout(request, { + username: 'user', accessToken, refreshToken, }); @@ -856,6 +815,7 @@ describe('SAMLAuthenticationProvider', () => { .resolves({ redirect: null }); const authenticationResult = await provider.logout(request, { + username: 'user', accessToken: 'x-saml-token', refreshToken: 'x-saml-refresh-token', }); @@ -921,6 +881,7 @@ describe('SAMLAuthenticationProvider', () => { .resolves({ redirect: 'http://fake-idp/SLO?SAMLRequest=7zlH37H' }); const authenticationResult = await provider.logout(request, { + username: 'user', accessToken, refreshToken, }); @@ -938,6 +899,7 @@ describe('SAMLAuthenticationProvider', () => { .resolves({ redirect: 'http://fake-idp/SLO?SAMLRequest=7zlH37H' }); const authenticationResult = await provider.logout(request, { + username: 'user', accessToken: 'x-saml-token', refreshToken: 'x-saml-refresh-token', }); diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts index ecd88f511bf8c..f972ec62d1e89 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.ts @@ -6,7 +6,6 @@ import Boom from 'boom'; import { KibanaRequest } from '../../../../../../src/core/server'; -import { AuthenticatedUser } from '../../../common/model'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; import { AuthenticationProviderOptions, BaseAuthenticationProvider } from './base'; @@ -17,6 +16,11 @@ import { canRedirectRequest } from '..'; * The state supported by the provider (for the SAML handshake or established session). */ interface ProviderState extends Partial { + /** + * Username of the SAML authenticated user. + */ + username?: string; + /** * Unique identifier of the SAML request initiated the handshake. */ @@ -94,8 +98,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { return await this.loginWithNewSAMLResponse( request, samlResponse, - (authenticationResult.state || state) as ProviderState, - authenticationResult.user as AuthenticatedUser + (authenticationResult.state || state) as ProviderState ); } @@ -259,19 +262,21 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { // This operation should be performed on behalf of the user with a privilege that normal // user usually doesn't have `cluster:admin/xpack/security/saml/authenticate`. const { + username, access_token: accessToken, refresh_token: refreshToken, } = await this.options.client.callAsInternalUser('shield.samlAuthenticate', { body: { ids: stateRequestId ? [stateRequestId] : [], content: samlResponse, + realm: this.realm, }, }); this.logger.debug('Login has been performed with SAML response.'); return AuthenticationResult.redirectTo( stateRedirectURL || `${this.options.basePath.get(request)}/`, - { state: { accessToken, refreshToken } } + { state: { username, accessToken, refreshToken } } ); } catch (err) { this.logger.debug(`Failed to log in with SAML response: ${err.message}`); @@ -290,13 +295,11 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { * @param request Request instance. * @param samlResponse SAMLResponse payload string. * @param existingState State existing user session is based on. - * @param user User returned for the existing session. */ private async loginWithNewSAMLResponse( request: KibanaRequest, samlResponse: string, - existingState: ProviderState, - user: AuthenticatedUser + existingState: ProviderState ) { this.logger.debug('Trying to log in with SAML response payload and existing valid session.'); @@ -315,20 +318,6 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { const newState = payloadAuthenticationResult.state as ProviderState; - // Then use received tokens to retrieve user information. We need just username and authentication - // realm, once ES starts returning this info from `saml/authenticate` we can get rid of this call. - const newUserAuthenticationResult = await this.authenticateViaState(request, newState); - if (newUserAuthenticationResult.failed()) { - return newUserAuthenticationResult; - } - - if (newUserAuthenticationResult.user === undefined) { - // Should never happen, but if it does - it's a bug. - return AuthenticationResult.failed( - new Error('Could not retrieve user information using tokens produced for the SAML payload.') - ); - } - // Now let's invalidate tokens from the existing session. try { this.logger.debug('Perform IdP initiated local logout.'); @@ -341,10 +330,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { return AuthenticationResult.failed(err); } - if ( - newUserAuthenticationResult.user.username !== user.username || - newUserAuthenticationResult.user.authentication_realm.name !== user.authentication_realm.name - ) { + if (newState.username !== existingState.username) { this.logger.debug( 'Login initiated by Identity Provider is for a different user than currently authenticated.' ); @@ -393,7 +379,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { */ private async authenticateViaRefreshToken( request: KibanaRequest, - { refreshToken }: ProviderState + { username, refreshToken }: ProviderState ) { this.logger.debug('Trying to refresh access token.'); @@ -432,7 +418,10 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { const user = await this.getUser(request, authHeaders); this.logger.debug('Request has been authenticated via refreshed token.'); - return AuthenticationResult.succeeded(user, { authHeaders, state: refreshedTokenPair }); + return AuthenticationResult.succeeded(user, { + authHeaders, + state: { username, ...refreshedTokenPair }, + }); } catch (err) { this.logger.debug( `Failed to authenticate user using newly refreshed access token: ${err.message}`