commit e7ad8d8c60c04ce70d0c8bf011bd4551bbd4b04d
author Urs Keller <urs@revault.ch> Thu Dec 10 17:51:08 2015 +0100
committer Luca Milanesio <luca.milanesio@gmail.com> Mon Feb 11 16:06:37 2019 +0000
tree fec10fb1e245185a7c693d73e12f75796f644557
parent 20e11e232eac78c3978bc05ea5b14ca56317de6b

Destroy GerritAccount Cookie on logout

Old behavior:
* the filter detects logout URL and cleans its own attribute
  from the session
* the filter redirects to the configured auth.logoutUrl not
  letting Gerrit the chance to clean up

New behavior:

* the filter detects logout URL and cleans its own attribute
  from the session
* It then passes the request on to Gerrit (continuing in the filter chain)
* Gerrit will do its cleanup and then redirect to the configured
  auth.logoutUrl

PR: https://github.com/thesamet/gerrit-saml-plugin/pull/4
Change-Id: I5a0fcc33af7a4833cfa548d064a054d997fd58fd

src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java

diff --git a/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java b/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java
index 1717bf4..9149a8b 100644
--- a/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java
+++ b/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java
@@ -19,7 +19,6 @@
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
-import com.google.inject.Injector;
 import com.google.inject.Singleton;
 import org.eclipse.jgit.lib.Config;
 import org.pac4j.core.context.J2EContext;
@@ -52,7 +51,6 @@
     private static final String SESSION_ATTR_USER = "Gerrit-Saml-User";
 
     static final Logger log = LoggerFactory.getLogger(SamlWebFilter.class);
-    private final Injector injector;
     private final SAML2Client saml2Client;
     private final SamlConfig samlConfig;
     private final String httpUserNameHeader;
@@ -60,7 +58,6 @@
     private final String httpEmailHeader;
     private final String httpExternalIdHeader;
     private final HashSet<String> authHeaders;
-    private final String logoutUrl;
 
     private String getHeaderFromConfig(Config gerritConfig, String name) {
         String s = gerritConfig.getString("auth", null, name);
@@ -68,8 +65,7 @@
     }
 
     @Inject
-    SamlWebFilter(Injector injector, @GerritServerConfig Config gerritConfig, SamlConfig samlConfig) {
-        this.injector = injector;
+    SamlWebFilter(@GerritServerConfig Config gerritConfig, SamlConfig samlConfig) {
         this.samlConfig = samlConfig;
         saml2Client =
                 new SAML2Client(new SAML2ClientConfiguration(
@@ -93,7 +89,6 @@
                     "httpDisplaynameHeader, httpEmailHeader and httpExternalIdHeader " +
                     "are required.");
         }
-        logoutUrl = gerritConfig.getString("auth", null, "logoutUrl");
 
         saml2Client.setCallbackUrl(callbackUrl);
     }
@@ -132,17 +127,9 @@
                 redirectUri = "/";
             }
             context.getResponse().sendRedirect(context.getRequest().getContextPath() + redirectUri);
-        } else {
-            signout(context.getRequest(), context.getResponse());
         }
     }
 
-    private void signout(HttpServletRequest request, HttpServletResponse response) throws IOException {
-        HttpSession s = request.getSession();
-        s.removeAttribute(SESSION_ATTR_USER);
-        response.sendRedirect(logoutUrl);
-    }
-
     @Override
     public void doFilter(ServletRequest incomingRequest, ServletResponse response,
                          FilterChain chain) throws IOException, ServletException {
@@ -167,7 +154,8 @@
                     chain.doFilter(req, response);
                 }
             } else if (isGerritLogout(httpRequest)) {
-                signout(httpRequest, httpResponse);
+                httpRequest.getSession().removeAttribute(SESSION_ATTR_USER);
+                chain.doFilter(httpRequest, httpResponse);
             } else {
                 chain.doFilter(httpRequest, httpResponse);
             }