[SECURITY-796][JENKINS-59069] Open redirect prevention (#17)

jenkinsci · Nov 7, 2019 · 10059a4 · 10059a4
1 parent 0f12976
commit 10059a4
Showing 1 changed file with 13 additions and 2 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/GitLabSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/GitLabSecurityRealm.java
@@ -67,6 +67,7 @@
 import org.kohsuke.stapler.HttpRedirect;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 import org.springframework.dao.DataAccessException;
 import org.springframework.dao.DataRetrievalFailureException;
@@ -264,12 +265,22 @@ public String getClientSecret() {
         return clientSecret;
     }
 
-    public HttpResponse doCommenceLogin(StaplerRequest request, @Header("Referer") final String referer) throws IOException {
+    // "from" is coming from SecurityRealm/loginLink.jelly
+    public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter String from, @Header("Referer") final String referer) throws IOException {
         // 2. Requesting authorization :
         // http://doc.gitlab.com/ce/api/oauth2.html
 
+        String redirectOnFinish;
+        if (from != null && Util.isSafeToRedirectTo(from)) {
+            redirectOnFinish = from;
+        } else if (referer != null && (referer.startsWith(Jenkins.getInstance().getRootUrl()) || Util.isSafeToRedirectTo(referer))) {
+            redirectOnFinish = referer;
+        } else {
+            redirectOnFinish = Jenkins.getInstance().getRootUrl();
+        }
+
         List<NameValuePair> parameters = new ArrayList<>();
-        parameters.add(new BasicNameValuePair("redirect_uri", buildRedirectUrl(request, referer)));
+        parameters.add(new BasicNameValuePair("redirect_uri", buildRedirectUrl(request, redirectOnFinish)));
         parameters.add(new BasicNameValuePair("response_type", "code"));
         parameters.add(new BasicNameValuePair("client_id", clientID));