[SECURITY-916]

jenkinsci · Jun 15, 2018 · fd95d57 · fd95d57
1 parent cc2412a
commit fd95d57
Showing 1 changed file with 49 additions and 7 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/saml/SamlSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/saml/SamlSecurityRealm.java
@@ -19,6 +19,7 @@
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
+import hudson.Util;
 import hudson.security.GroupDetails;
 import hudson.security.UserMayOrMayNotExistException;
 import hudson.util.FormValidation;
@@ -44,6 +45,7 @@
 import org.pac4j.saml.profile.SAML2Profile;
 
 import javax.annotation.Nonnull;
+import javax.servlet.http.HttpSession;
 import java.io.*;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -240,9 +242,12 @@ public String getLoginUrl() {
      * @param referer  referer.
      * @return the http response.
      */
-    public HttpResponse doCommenceLogin(final StaplerRequest request, final StaplerResponse response, @Header("Referer") final String referer) {
+    public HttpResponse doCommenceLogin(final StaplerRequest request, final StaplerResponse response, @QueryParameter
+            String from, @Header("Referer") final String referer) {
         LOG.fine("SamlSecurityRealm.doCommenceLogin called. Using consumerServiceUrl " + getSamlPluginConfig().getConsumerServiceUrl());
-        request.getSession().setAttribute(REFERER_ATTRIBUTE, referer);
+
+        String redirectOnFinish = calculateSafeRedirect(from, referer);
+        request.getSession().setAttribute(REFERER_ATTRIBUTE, redirectOnFinish);
 
         RedirectAction action = new SamlRedirectActionWrapper(getSamlPluginConfig(), request, response).get();
         if (action.getType() == RedirectType.REDIRECT) {
@@ -256,6 +261,28 @@ public HttpResponse doCommenceLogin(final StaplerRequest request, final StaplerR
         }
     }
 
+    /**
+     * Check parameters "from" and "referer" to decide where is the safe URL to be redirected.
+     * @param from http request "from" parameter.
+     * @param referer referer header.
+     * @return a safe URL to be redirected.
+     */
+    private String calculateSafeRedirect(String from, String referer) {
+        String redirectURL;
+        String rootUrl = Jenkins.getInstance().getRootUrl();
+        if (from != null && Util.isSafeToRedirectTo(from)) {
+            redirectURL = from;
+        } else {
+            if (referer != null && (referer.startsWith(rootUrl) || Util.isSafeToRedirectTo(referer))) {
+                redirectURL = referer;
+            } else {
+                redirectURL = rootUrl;
+            }
+        }
+        LOG.fine("Safe URL redirection: " + redirectURL);
+        return redirectURL;
+    }
+
     /**
      * /securityRealm/finishLogin
      *
@@ -266,7 +293,12 @@ public HttpResponse doCommenceLogin(final StaplerRequest request, final StaplerR
     @RequirePOST
     public HttpResponse doFinishLogin(final StaplerRequest request, final StaplerResponse response) {
         LOG.finer("SamlSecurityRealm.doFinishLogin called");
+        String referer = (String) request.getSession().getAttribute(REFERER_ATTRIBUTE);
+        // redirect back to original page
+        String redirectUrl = referer != null ? referer : baseUrl();
+        recreateSession(request);
         logSamlResponse(request);
+
         boolean saveUser = false;
 
         SAML2Profile saml2Profile = new SamlProfileWrapper(getSamlPluginConfig(), request, response).get();
@@ -312,14 +344,24 @@ public HttpResponse doFinishLogin(final StaplerRequest request, final StaplerRes
         }
 
         SecurityListener.fireLoggedIn(userDetails.getUsername());
-
-        // redirect back to original page
-        String referer = (String) request.getSession().getAttribute(REFERER_ATTRIBUTE);
-        String redirectUrl = referer != null ? referer : baseUrl();
         return HttpResponses.redirectTo(redirectUrl);
     }
 
-
+    /**
+     * check if a request contains a session, if so, it invalidate the session and create new one to avoid session
+     * fixation.
+     * @param request request.
+     */
+    private void recreateSession(StaplerRequest request) {
+        HttpSession session = request.getSession(false);
+        if(session != null){
+            LOG.finest("Invalidate previous session");
+            // avoid session fixation
+            session.invalidate();
+        }
+        request.getSession(true);
+    }
+
     private boolean modifyUserSamlCustomAttributes(User user, SAML2Profile profile) {
         boolean saveUser = false;
         if(!getSamlCustomAttributes().isEmpty() && user != null){