diff --git a/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java
index 803fd1c5..c6c706d5 100644
--- a/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java
+++ b/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java
@@ -62,7 +62,7 @@ of this software and associated documentation files (the "Software"), to deal
import org.apache.http.client.CredentialsProvider;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpPost;
-import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.client.utils.URIBuilder;
import org.apache.http.impl.client.BasicCredentialsProvider;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
@@ -73,6 +73,7 @@ of this software and associated documentation files (the "Software"), to deal
import org.kohsuke.github.GHOrganization;
import org.kohsuke.github.GHTeam;
import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.Header;
import org.kohsuke.stapler.HttpRedirect;
import org.kohsuke.stapler.HttpResponse;
@@ -85,6 +86,8 @@ of this software and associated documentation files (the "Software"), to deal
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.Proxy;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
import java.util.Arrays;
import java.util.HashSet;
@@ -115,6 +118,8 @@ public class GithubSecurityRealm extends AbstractPasswordBasedSecurityRealm impl
private Secret clientSecret;
private String oauthScopes;
private String[] myScopes;
+ @NonNull
+ private String redirectUri = "";
/**
* @param githubWebUri The URI to the root of the web UI for GitHub or GitHub Enterprise,
@@ -187,6 +192,15 @@ private void setOauthScopes(String oauthScopes) {
this.oauthScopes = oauthScopes;
}
+ /**
+ * @param redirectUri the redirectUri to set
+ */
+ @DataBoundSetter
+ public void setRedirectUri(String redirectUri) {
+ if (null == redirectUri) redirectUri = "";
+ this.redirectUri = redirectUri;
+ }
+
/**
* Checks the security realm for a GitHub OAuth scope.
* @param scope A scope to check for in the security realm.
@@ -245,6 +259,10 @@ public void marshal(Object source, HierarchicalStreamWriter writer,
writer.setValue(realm.getOauthScopes());
writer.endNode();
+ writer.startNode("redirectUri");
+ writer.setValue(realm.getRedirectUri());
+ writer.endNode();
+
}
public Object unmarshal(HierarchicalStreamReader reader,
@@ -274,8 +292,7 @@ public Object unmarshal(HierarchicalStreamReader reader,
return realm;
}
- private void setValue(GithubSecurityRealm realm, String node,
- String value) {
+ private void setValue(GithubSecurityRealm realm, String node, String value) {
if (node.equalsIgnoreCase("clientid")) {
realm.setClientID(value);
} else if (node.equalsIgnoreCase("clientsecret")) {
@@ -290,6 +307,8 @@ private void setValue(GithubSecurityRealm realm, String node,
realm.setGithubApiUri(value);
} else if (node.equalsIgnoreCase("oauthscopes")) {
realm.setOauthScopes(value);
+ } else if (node.equalsIgnoreCase("redirecturi")) {
+ realm.setRedirectUri(value);
} else {
throw new ConversionException("Invalid node value = " + node);
}
@@ -334,11 +353,21 @@ public String getOauthScopes() {
return oauthScopes;
}
+ /**
+ * @return the redirectUri
+ */
+ @NonNull
+ public String getRedirectUri() {
+ return redirectUri;
+ }
+
public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter String from, @Header("Referer") final String referer)
- throws IOException {
+ throws IOException, URISyntaxException {
// https://tools.ietf.org/html/rfc6749#section-10.10 dictates that probability that an attacker guesses the string
// SHOULD be less than or equal to 2^(-160) and our Strings consist of 65 chars. (65^27 ~= 2^160)
final String state = getSecureRandomString(27);
+
+ // This is to go back to the current page after login, not the oauth callback
String redirectOnFinish;
if (from != null && Util.isSafeToRedirectTo(from)) {
redirectOnFinish = from;
@@ -355,17 +384,17 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter Stri
for (GitHubOAuthScope s : Jenkins.get().getExtensionList(GitHubOAuthScope.class)) {
scopes.addAll(s.getScopesToRequest());
}
- String suffix="";
- if (!scopes.isEmpty()) {
- suffix = "&scope="+Util.join(scopes,",")+"&state="+state;
- } else {
- // We need repo scope in order to access private repos
- // See https://developer.github.com/v3/oauth/#scopes
- suffix = "&scope=" + oauthScopes +"&state="+state;
+
+ URIBuilder builder = new URIBuilder(githubWebUri, StandardCharsets.UTF_8);
+ builder.setPath("/login/oauth/authorize");
+ builder.setParameter("client_id", clientID);
+ builder.setParameter("scope", scopes.isEmpty() ? oauthScopes : Util.join(scopes,","));
+ builder.setParameter("state", state);
+ if (!redirectUri.isEmpty()) {
+ builder.setParameter("redirect_uri", redirectUri);
}
- return new HttpRedirect(githubWebUri + "/login/oauth/authorize?client_id="
- + clientID + suffix);
+ return new HttpRedirect(builder.toString());
}
/**
@@ -630,6 +659,12 @@ public String getDefaultOauthScopes() {
return DEFAULT_OAUTH_SCOPES;
}
+ public String getDefaultRequestUri() {
+ // Intentionally making this default in UI and not in code
+ // to preserve behaviour for existing groovy init & JCasc setups
+ return Jenkins.get().getRootUrl() + "securityRealm/finishLogin";
+ }
+
public DescriptorImpl() {
super();
// TODO Auto-generated constructor stub
@@ -728,7 +763,8 @@ public boolean equals(Object object){
this.getGithubApiUri().equals(obj.getGithubApiUri()) &&
this.getClientID().equals(obj.getClientID()) &&
this.getClientSecret().equals(obj.getClientSecret()) &&
- this.getOauthScopes().equals(obj.getOauthScopes());
+ this.getOauthScopes().equals(obj.getOauthScopes()) &&
+ this.getRedirectUri().equals(obj.getRedirectUri());
} else {
return false;
}
@@ -742,6 +778,7 @@ public int hashCode() {
.append(this.getClientID())
.append(this.getClientSecret())
.append(this.getOauthScopes())
+ .append(this.getRedirectUri())
.toHashCode();
}
diff --git a/src/main/resources/org/jenkinsci/plugins/GithubSecurityRealm/config.jelly b/src/main/resources/org/jenkinsci/plugins/GithubSecurityRealm/config.jelly
index 5008251d..9cd44860 100644
--- a/src/main/resources/org/jenkinsci/plugins/GithubSecurityRealm/config.jelly
+++ b/src/main/resources/org/jenkinsci/plugins/GithubSecurityRealm/config.jelly
@@ -23,5 +23,10 @@
+
+
+
+
+
diff --git a/src/main/webapp/help/realm/redirect-uri-help.html b/src/main/webapp/help/realm/redirect-uri-help.html
new file mode 100644
index 00000000..c779283d
--- /dev/null
+++ b/src/main/webapp/help/realm/redirect-uri-help.html
@@ -0,0 +1,3 @@
+
diff --git a/src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java
index e6cd935a..1938cc78 100644
--- a/src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java
+++ b/src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java
@@ -52,7 +52,24 @@ public void testEquals_false() {
GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
GithubSecurityRealm b = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,repo");
assertNotEquals(a, b);
- assertNotEquals("", a);
+ }
+
+ @Test
+ public void testEqualsRedirectUri() {
+ GithubSecurityRealm _default = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
+ GithubSecurityRealm empty = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
+ empty.setRedirectUri("");
+ GithubSecurityRealm sameValue1 = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
+ sameValue1.setRedirectUri("http://jenkins1.awesomecorp.com");
+ GithubSecurityRealm sameValue2 = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
+ sameValue2.setRedirectUri("http://jenkins1.awesomecorp.com");
+ GithubSecurityRealm otherValue = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org");
+ otherValue.setRedirectUri("http://jenkins1.notsogoodcorp.com");
+ assertEquals(_default, empty);
+ assertEquals(sameValue1, sameValue2);
+ assertNotEquals(sameValue1, _default);
+ assertNotEquals(sameValue1, empty);
+ assertNotEquals(sameValue1, otherValue);
}
@Test