Merge remote-tracking branch 'cert/0.29.x' into next-release

samrocketman · samrocketman · commit 8c4afe8522f3 · 2018-12-06T22:27:44.000-08:00
diff --git a/pom.xml b/pom.xml
@@ -39,7 +39,7 @@
 
     <groupId>org.jenkins-ci.plugins</groupId>
     <artifactId>github-oauth</artifactId>
-    <version>0.30-SNAPSHOT</version>
+    <version>0.31-SNAPSHOT</version>
     <name>GitHub Authentication plugin</name>
     <description>A Jenkins authentication plugin that delegates to GitHub.  We also implement an Authorization Strategy that uses the acquired OAuth token to interact with the GitHub API to determine a user's level of access to Jenkins.</description>
     <packaging>hpi</packaging>
diff --git a/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java
@@ -79,6 +79,7 @@ of this software and associated documentation files (the "Software"), to deal
 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;
@@ -93,6 +94,7 @@ of this software and associated documentation files (the "Software"), to deal
 import java.util.logging.Logger;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.servlet.http.HttpSession;
 
 /**
  *
@@ -333,9 +335,18 @@ public String getOauthScopes() {
         return oauthScopes;
     }
 
-    public HttpResponse doCommenceLogin(StaplerRequest request, @Header("Referer") final String referer)
+    public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter String from, @Header("Referer") final String referer)
             throws IOException {
-        request.getSession().setAttribute(REFERER_ATTRIBUTE,referer);
+        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();
+        }
+
+        request.getSession().setAttribute(REFERER_ATTRIBUTE, redirectOnFinish);
 
         Set<String> scopes = new HashSet<>();
         for (GitHubOAuthScope s : getJenkins().getExtensionList(GitHubOAuthScope.class)) {
@@ -361,6 +372,7 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @Header("Referer") f
     public HttpResponse doFinishLogin(StaplerRequest request)
             throws IOException {
         String code = request.getParameter("code");
+        String referer = (String)request.getSession().getAttribute(REFERER_ATTRIBUTE);
 
         if (code == null || code.trim().length() == 0) {
             Log.info("doFinishLogin: missing code.");
@@ -372,6 +384,14 @@ public HttpResponse doFinishLogin(StaplerRequest request)
         if (accessToken != null && accessToken.trim().length() > 0) {
             // only set the access token if it exists.
             GithubAuthenticationToken auth = new GithubAuthenticationToken(accessToken, getGithubApiUri());
+
+            HttpSession session = request.getSession(false);
+            if(session != null){
+                // avoid session fixation
+                session.invalidate();
+            }
+            request.getSession(true);
+
             SecurityContextHolder.getContext().setAuthentication(auth);
 
             GHMyself self = auth.getMyself();
@@ -409,7 +429,6 @@ public HttpResponse doFinishLogin(StaplerRequest request)
             Log.info("Github did not return an access token.");
         }
 
-        String referer = (String)request.getSession().getAttribute(REFERER_ATTRIBUTE);
         if (referer!=null)  return HttpResponses.redirectTo(referer);
         return HttpResponses.redirectToContextRoot();   // referer should be always there, but be defensive
     }
diff --git a/src/main/resources/org/jenkinsci/plugins/GithubSecurityRealm/config.jelly b/src/main/resources/org/jenkinsci/plugins/GithubSecurityRealm/config.jelly
@@ -17,7 +17,7 @@
         </f:entry>
 
         <f:entry title="Client Secret" field="clientSecret" help="/plugin/github-oauth/help/realm/client-secret-help.html">
-            <f:textbox />
+            <f:password />
         </f:entry>
 
         <f:entry title="OAuth Scope(s)" field="oauthScopes" help="/plugin/github-oauth/help/realm/oauth-scopes-help.html">
diff --git a/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertySEC797Test.java b/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertySEC797Test.java
