[6.8] Redirect to Logged Out UI on SAML Logout Response.

x-pack/plugins/security/server/lib/authentication/authenticator.js

@@ -202,13 +202,20 @@ class Authenticator {
       return this._providers.get(sessionValue.provider).deauthenticate(request, sessionValue.state);
     }
 
-    // Normally when there is no active session in Kibana, `deauthenticate` method shouldn't do anything
-    // and user will eventually be redirected to the home page to log in. But if SAML is supported there
-    // is a special case when logout is initiated by the IdP or another SP, then IdP will request _every_
-    // SP associated with the current user session to do the logout. So if Kibana (without active session)
-    // receives such a request it shouldn't redirect user to the home page, but rather redirect back to IdP
-    // with correct logout response and only Elasticsearch knows how to do that.
-    if (this._isSAMLRequest(request) && this._providers.has('saml')) {
+    // Normally when there is no active session in Kibana, `deauthenticate` method shouldn't do
+    // anything and user will eventually be redirected to the home page to log in. But when SAML SLO
+    // is supported there are two special cases that we need to handle even if there is no active
+    // Kibana session:
+    //
+    // 1. When IdP or another SP initiates logout, then IdP will request _every_ SP associated with
+    // the current user session to do the logout. So if Kibana receives such request it shouldn't
+    // redirect user to the home page, but rather redirect back to IdP with correct logout response
+    // and only Elasticsearch knows how to do that.
+    //
+    // 2. When Kibana initiates logout, then IdP may eventually respond with the logout response. So
+    // if Kibana receives such response it shouldn't redirect user to the home page, but rather
+    // redirect to the `loggedOut` URL instead.
+    if ((this._isSAMLRequestQuery(request) || this._isSAMLResponseQuery(request)) && this._providers.has('saml')) {
       return this._providers.get('saml').deauthenticate(request);
     }
 
@@ -241,7 +248,7 @@ class Authenticator {
     // If there is no way to predict which provider to use first, let's use the order providers are configured in.
     // Otherwise return provider that either owns session or can handle 3rd-party login request first, and only then
     // the rest of providers.
-    const shouldHandleSAMLResponse = this._isSAMLResponse(request) && this._providers.has('saml');
+    const shouldHandleSAMLResponse = this._isSAMLResponsePayload(request) && this._providers.has('saml');
     if (!sessionValue && !shouldHandleSAMLResponse) {
       yield* this._providers;
     } else {
@@ -278,22 +285,32 @@ class Authenticator {
   }
 
   /**
-   * Checks whether specified request represents SAML Request.
+   * Checks whether specified request has SAML Request in the query.
    * @param {Hapi.Request} request HapiJS request instance.
    * @returns {boolean}
    * @private
    */
-  _isSAMLRequest(request) {
+  _isSAMLRequestQuery(request) {
     return !!(request.query && request.query.SAMLRequest);
   }
 
   /**
-   * Checks whether specified request represents SAML Response.
+   * Checks whether specified request has SAML Response in the query.
    * @param {Hapi.Request} request HapiJS request instance.
    * @returns {boolean}
    * @private
    */
-  _isSAMLResponse(request) {
+  _isSAMLResponseQuery(request) {
+    return !!(request.query && request.query.SAMLResponse);
+  }
+
+  /**
+   * Checks whether specified request has SAML Response in the payload.
+   * @param {Hapi.Request} request HapiJS request instance.
+   * @returns {boolean}
+   * @private
+   */
+  _isSAMLResponsePayload(request) {
     return !!(request.payload && request.payload.SAMLResponse)
       && request.path === '/api/security/v1/saml';
   }

x-pack/plugins/security/server/lib/authentication/providers/__tests__/saml.js

@@ -559,7 +559,7 @@ describe('SAMLAuthenticationProvider', () => {
       );
 
       expect(authenticationResult.redirected()).to.be(true);
-      expect(authenticationResult.redirectURL).to.be('/logged_out');
+      expect(authenticationResult.redirectURL).to.be('/test-base-path/logged_out');
     });
 
     it('redirects to /logged_out if `redirect` field in SAML logout response is not defined.', async () => {
@@ -581,11 +581,11 @@ describe('SAMLAuthenticationProvider', () => {
       );
 
       expect(authenticationResult.redirected()).to.be(true);
-      expect(authenticationResult.redirectURL).to.be('/logged_out');
+      expect(authenticationResult.redirectURL).to.be('/test-base-path/logged_out');
     });
 
     it('relies on SAML logout if query string is not empty, but does not include SAMLRequest.', async () => {
-      const request = requestFixture({ search: '?Whatever=something%20unrelated' });
+      const request = requestFixture({ search: '?Whatever=something%20unrelated&SAMLResponse=xxx%20yyy' });
       const accessToken = 'x-saml-token';
       const refreshToken = 'x-saml-refresh-token';
 
@@ -603,7 +603,7 @@ describe('SAMLAuthenticationProvider', () => {
       );
 
       expect(authenticationResult.redirected()).to.be(true);
-      expect(authenticationResult.redirectURL).to.be('/logged_out');
+      expect(authenticationResult.redirectURL).to.be('/test-base-path/logged_out');
     });
 
     it('relies SAML invalidate call even if access token is presented.', async () => {
@@ -631,7 +631,7 @@ describe('SAMLAuthenticationProvider', () => {
       );
 
       expect(authenticationResult.redirected()).to.be(true);
-      expect(authenticationResult.redirectURL).to.be('/logged_out');
+      expect(authenticationResult.redirectURL).to.be('/test-base-path/logged_out');
     });
 
     it('redirects to /logged_out if `redirect` field in SAML invalidate response is null.', async () => {
@@ -656,7 +656,7 @@ describe('SAMLAuthenticationProvider', () => {
       );
 
       expect(authenticationResult.redirected()).to.be(true);
-      expect(authenticationResult.redirectURL).to.be('/logged_out');
+      expect(authenticationResult.redirectURL).to.be('/test-base-path/logged_out');
     });
 
     it('redirects to /logged_out if `redirect` field in SAML invalidate response is not defined.', async () => {
@@ -681,7 +681,18 @@ describe('SAMLAuthenticationProvider', () => {
       );
 
       expect(authenticationResult.redirected()).to.be(true);
-      expect(authenticationResult.redirectURL).to.be('/logged_out');
+      expect(authenticationResult.redirectURL).to.be('/test-base-path/logged_out');
+    });
+
+    it('redirects to /logged_out if SAML logout response is received.', async () => {
+      const request = requestFixture({ search: '?SAMLResponse=xxx%20yyy' });
+
+      const authenticationResult = await provider.deauthenticate(request);
+
+      sinon.assert.notCalled(callWithInternalUser);
+
+      expect(authenticationResult.redirected()).to.be(true);
+      expect(authenticationResult.redirectURL).to.be('/test-base-path/logged_out');
     });
 
     it('redirects user to the IdP if SLO is supported by IdP in case of SP initiated logout.', async () => {

x-pack/plugins/security/server/lib/authentication/providers/saml.js

@@ -387,20 +387,22 @@ export class SAMLAuthenticationProvider {
   async deauthenticate(request, state) {
     this._options.log(['debug', 'security', 'saml'], `Trying to deauthenticate user via ${request.url.path}.`);
 
-    if ((!state || !state.accessToken) && !request.query.SAMLRequest) {
+    const isIdPInitiatedSLORequest = !!(request.query && request.query.SAMLRequest);
+    const isSPInitiatedSLOResponse = !!(request.query && request.query.SAMLResponse);
+    if ((!state || !state.accessToken) && !isIdPInitiatedSLORequest && !isSPInitiatedSLOResponse) {
       this._options.log(['debug', 'security', 'saml'], 'There is neither access token nor SAML session to invalidate.');
       return DeauthenticationResult.notHandled();
     }
 
     let logoutArgs;
-    if (request.query.SAMLRequest) {
+    if (isIdPInitiatedSLORequest) {
       this._options.log(['debug', 'security', 'saml'], 'Logout has been initiated by the Identity Provider.');
       logoutArgs = [
         'shield.samlInvalidate',
         // Elasticsearch expects `queryString` without leading `?`, so we should strip it with `slice`.
         { body: { queryString: request.url.search ? request.url.search.slice(1) : '', acs: this._getACS() } }
       ];
-    } else {
+    } else if (state) {
       this._options.log(['debug', 'security', 'saml'], 'Logout has been initiated by the user.');
       logoutArgs = [
         'shield.samlLogout',
@@ -409,9 +411,15 @@ export class SAMLAuthenticationProvider {
     }
 
     try {
-      // 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/logout (invalidate)`.
-      const { redirect } = await this._options.client.callWithInternalUser(...logoutArgs);
+      // It may _theoretically_ (highly unlikely in practice though) happen that when user receives
+      // logout response they may already have a new SAML session (isSPInitiatedSLOResponse == true
+      // and state !== undefined). In this case case it'd be safer to trigger SP initiated logout
+      // for the new session as well.
+      const redirect = logoutArgs
+        // 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/logout (invalidate)`.
+        ? (await this._options.client.callWithInternalUser(...logoutArgs)).redirect
+        : null;
 
       this._options.log(['debug', 'security', 'saml'], 'User session has been successfully invalidated.');
 
@@ -423,7 +431,7 @@ export class SAMLAuthenticationProvider {
         return DeauthenticationResult.redirectTo(redirect);
       }
 
-      return DeauthenticationResult.redirectTo('/logged_out');
+      return DeauthenticationResult.redirectTo(`${this._options.basePath}/logged_out`);
     } catch(err) {
       this._options.log(['debug', 'security', 'saml'], `Failed to deauthenticate user: ${err.message}`);
       return DeauthenticationResult.failed(err);