From 91e7b7fbdab8aab7eaa2ce9a009be569e01b5643 Mon Sep 17 00:00:00 2001 From: philiplrb Date: Tue, 2 Apr 2019 13:23:34 -0700 Subject: [PATCH 1/2] Add the option to configure redirect uri --- .../plugins/GithubSecurityRealm.java | 52 +++++++++++++++++-- .../plugins/GithubSecurityRealm/config.jelly | 5 ++ .../webapp/help/realm/redirect-uri-help.html | 3 ++ .../GithubAccessTokenPropertySEC797Test.java | 4 +- .../GithubAccessTokenPropertyTest.java | 4 +- .../plugins/GithubSecurityRealmTest.java | 28 +++++++--- 6 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 src/main/webapp/help/realm/redirect-uri-help.html diff --git a/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java index cfef0f6d..71149723 100644 --- a/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java @@ -107,6 +107,7 @@ of this software and associated documentation files (the "Software"), to deal public class GithubSecurityRealm extends AbstractPasswordBasedSecurityRealm implements UserDetailsService { private static final String DEFAULT_WEB_URI = "https://github.com"; private static final String DEFAULT_API_URI = "https://api.github.com"; + private static final String DEFAULT_REDIRECT_URI = ""; private static final String DEFAULT_ENTERPRISE_API_SUFFIX = "/api/v3"; private static final String DEFAULT_OAUTH_SCOPES = "read:org,user:email,repo"; @@ -116,6 +117,7 @@ public class GithubSecurityRealm extends AbstractPasswordBasedSecurityRealm impl private Secret clientSecret; private String oauthScopes; private String[] myScopes; + private String redirectUri; /** * @param githubWebUri The URI to the root of the web UI for GitHub or GitHub Enterprise, @@ -125,13 +127,15 @@ public class GithubSecurityRealm extends AbstractPasswordBasedSecurityRealm impl * @param clientID The client ID for the created OAuth Application. * @param clientSecret The client secret for the created GitHub OAuth Application. * @param oauthScopes A comma separated list of OAuth Scopes to request access to. + * @param redirectUri An optional redirect URI to be used by GitHub. */ @DataBoundConstructor public GithubSecurityRealm(String githubWebUri, String githubApiUri, String clientID, String clientSecret, - String oauthScopes) { + String oauthScopes, + String redirectUri) { super(); this.githubWebUri = Util.fixEmptyAndTrim(githubWebUri); @@ -139,6 +143,7 @@ public GithubSecurityRealm(String githubWebUri, this.clientID = Util.fixEmptyAndTrim(clientID); setClientSecret(Util.fixEmptyAndTrim(clientSecret)); this.oauthScopes = Util.fixEmptyAndTrim(oauthScopes); + this.redirectUri = Util.fixEmptyAndTrim(redirectUri); } private GithubSecurityRealm() { } @@ -188,6 +193,13 @@ private void setOauthScopes(String oauthScopes) { this.oauthScopes = oauthScopes; } + /** + * @param redirectUri the redirectUri to set + */ + private void setRedirectUri(String redirectUri) { + this.redirectUri = redirectUri; + } + /** * Checks the security realm for a GitHub OAuth scope. * @param scope A scope to check for in the security realm. @@ -246,6 +258,13 @@ public void marshal(Object source, HierarchicalStreamWriter writer, writer.setValue(realm.getOauthScopes()); writer.endNode(); + writer.startNode("redirectUri"); + String redirectUriValue = DEFAULT_REDIRECT_URI; + if (null != realm.getRedirectUri()) { + redirectUriValue = realm.getRedirectUri(); + } + writer.setValue(redirectUriValue); + writer.endNode(); } public Object unmarshal(HierarchicalStreamReader reader, @@ -272,6 +291,10 @@ public Object unmarshal(HierarchicalStreamReader reader, realm.setGithubApiUri(DEFAULT_API_URI); } + if (realm.getRedirectUri() == null) { + realm.setRedirectUri(DEFAULT_REDIRECT_URI); + } + return realm; } @@ -291,6 +314,8 @@ private void setValue(GithubSecurityRealm realm, String node, realm.setGithubApiUri(value); } else if (node.toLowerCase().equals("oauthscopes")) { realm.setOauthScopes(value); + } else if (node.toLowerCase().equals("redirecturi")) { + realm.setRedirectUri(value); } else { throw new ConversionException("Invalid node value = " + node); } @@ -335,6 +360,13 @@ public String getOauthScopes() { return oauthScopes; } + /** + * @return the redirectUri + */ + public String getRedirectUri() { + return redirectUri; + } + public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter String from, @Header("Referer") final String referer) throws IOException { String redirectOnFinish; @@ -361,6 +393,10 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter Stri suffix = "&scope=" + oauthScopes; } + if (null != redirectUri && !redirectUri.isEmpty()) { + suffix += "&redirect_uri=" + redirectUri; + } + return new HttpRedirect(githubWebUri + "/login/oauth/authorize?client_id=" + clientID + suffix); } @@ -715,11 +751,20 @@ public UserDetails loadUserByUsername(String username) public boolean equals(Object object){ if(object instanceof GithubSecurityRealm) { GithubSecurityRealm obj = (GithubSecurityRealm) object; - return this.getGithubWebUri().equals(obj.getGithubWebUri()) && + String redirectUri2 = this.getRedirectUri(); + if (null == redirectUri2) { + redirectUri2 = DEFAULT_REDIRECT_URI; + } + String redirectUri3 = obj.getRedirectUri(); + if (null == redirectUri3) { + redirectUri3 = DEFAULT_REDIRECT_URI; + } + return this.getGithubWebUri().equals(obj.getGithubWebUri()) && 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()) && + redirectUri2.equals(redirectUri3); } else { return false; } @@ -733,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..6c143f55 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..7a1b1bdd --- /dev/null +++ b/src/main/webapp/help/realm/redirect-uri-help.html @@ -0,0 +1,3 @@ +
+An optional redirect URI to be used by GitHub. See https://developer.github.com/enterprise/2.10/apps/building-oauth-apps/authorization-options-for-oauth-apps/#redirect-urls. +
\ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertySEC797Test.java b/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertySEC797Test.java index 369bc228..add5b39a 100644 --- a/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertySEC797Test.java +++ b/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertySEC797Test.java @@ -270,13 +270,15 @@ private void setupRealm() { String clientID = "xxx"; String clientSecret = "yyy"; String oauthScopes = "read:org"; + String redirectUri = ""; GithubSecurityRealm githubSecurityRealm = new GithubSecurityRealm( githubWebUri, githubApiUri, clientID, clientSecret, - oauthScopes + oauthScopes, + redirectUri ); j.jenkins.setSecurityRealm(githubSecurityRealm); diff --git a/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertyTest.java b/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertyTest.java index 2ef7d9a9..7f265d0b 100644 --- a/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertyTest.java +++ b/src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertyTest.java @@ -266,13 +266,15 @@ private void setupRealm(){ String clientID = "xxx"; String clientSecret = "yyy"; String oauthScopes = "read:org"; + String redirectUri = ""; GithubSecurityRealm githubSecurityRealm = new GithubSecurityRealm( githubWebUri, githubApiUri, clientID, clientSecret, - oauthScopes + oauthScopes, + redirectUri ); j.jenkins.setSecurityRealm(githubSecurityRealm); diff --git a/src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java index 6d2390cf..d7457fa9 100644 --- a/src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java +++ b/src/test/java/org/jenkinsci/plugins/GithubSecurityRealmTest.java @@ -30,6 +30,7 @@ of this software and associated documentation files (the "Software"), to deal import org.jvnet.hudson.test.JenkinsRule; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -40,22 +41,31 @@ public class GithubSecurityRealmTest { @Test public void testEquals_true() { - 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"); + GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "https://build-dev.intuit.com/test/securityRealm/finishLogin"); + GithubSecurityRealm b = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "https://build-dev.intuit.com/test/securityRealm/finishLogin"); assertTrue(a.equals(b)); } @Test 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"); + 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", ""); + assertFalse(a.equals(b)); + assertFalse(a.equals("")); + } + + @Test + public void testEqualsRedirectUri_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", "https://build-dev.intuit.com/test/securityRealm/finishLogin"); assertFalse(a.equals(b)); assertFalse(a.equals("")); } + @Test public void testHasScope_true() { - GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email"); + GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email", ""); assertTrue(a.hasScope("user")); assertTrue(a.hasScope("read:org")); assertTrue(a.hasScope("user:email")); @@ -63,7 +73,7 @@ public void testHasScope_true() { @Test public void testHasScope_false() { - GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email"); + GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org,user,user:email", ""); assertFalse(a.hasScope("somescope")); } @@ -84,4 +94,10 @@ public void testDescriptorImplGetDefaultOauthScopes() { DescriptorImpl descriptor = new DescriptorImpl(); assertTrue("read:org,user:email,repo".equals(descriptor.getDefaultOauthScopes())); } + + @Test + public void testHashCode() { + GithubSecurityRealm a = new GithubSecurityRealm("http://jenkins.acme.com", "http://jenkins.acme.com/api/v3", "someid", "somesecret", "read:org", "https://build-dev.intuit.com/test/securityRealm/finishLogin"); + assertNotNull(a.hashCode()); + } } From 938510dc2a6a9e280ca7e3bc428203171abf5b25 Mon Sep 17 00:00:00 2001 From: philiplrb Date: Wed, 24 Apr 2019 17:13:29 -0700 Subject: [PATCH 2/2] Expand tabs to spaces --- .../plugins/GithubSecurityRealm.java | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java index 71149723..f3eaa1b0 100644 --- a/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java @@ -261,9 +261,9 @@ public void marshal(Object source, HierarchicalStreamWriter writer, writer.startNode("redirectUri"); String redirectUriValue = DEFAULT_REDIRECT_URI; if (null != realm.getRedirectUri()) { - redirectUriValue = realm.getRedirectUri(); - } - writer.setValue(redirectUriValue); + redirectUriValue = realm.getRedirectUri(); + } + writer.setValue(redirectUriValue); writer.endNode(); } @@ -292,14 +292,13 @@ public Object unmarshal(HierarchicalStreamReader reader, } if (realm.getRedirectUri() == null) { - realm.setRedirectUri(DEFAULT_REDIRECT_URI); + realm.setRedirectUri(DEFAULT_REDIRECT_URI); } return realm; } - private void setValue(GithubSecurityRealm realm, String node, - String value) { + private void setValue(GithubSecurityRealm realm, String node, String value) { if (node.toLowerCase().equals("clientid")) { realm.setClientID(value); } else if (node.toLowerCase().equals("clientsecret")) { @@ -315,7 +314,7 @@ private void setValue(GithubSecurityRealm realm, String node, } else if (node.toLowerCase().equals("oauthscopes")) { realm.setOauthScopes(value); } else if (node.toLowerCase().equals("redirecturi")) { - realm.setRedirectUri(value); + realm.setRedirectUri(value); } else { throw new ConversionException("Invalid node value = " + node); } @@ -384,9 +383,9 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter Stri for (GitHubOAuthScope s : getJenkins().getExtensionList(GitHubOAuthScope.class)) { scopes.addAll(s.getScopesToRequest()); } - String suffix=""; + String suffix = ""; if (!scopes.isEmpty()) { - suffix = "&scope="+Util.join(scopes,","); + suffix = "&scope=" + Util.join(scopes, ","); } else { // We need repo scope in order to access private repos // See https://developer.github.com/v3/oauth/#scopes @@ -394,7 +393,7 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter Stri } if (null != redirectUri && !redirectUri.isEmpty()) { - suffix += "&redirect_uri=" + redirectUri; + suffix += "&redirect_uri=" + redirectUri; } return new HttpRedirect(githubWebUri + "/login/oauth/authorize?client_id=" @@ -748,23 +747,22 @@ public UserDetails loadUserByUsername(String username) * @return true if the objects are the same instance and configuration. */ @Override - public boolean equals(Object object){ - if(object instanceof GithubSecurityRealm) { + public boolean equals(Object object) { + if (object instanceof GithubSecurityRealm) { GithubSecurityRealm obj = (GithubSecurityRealm) object; String redirectUri2 = this.getRedirectUri(); if (null == redirectUri2) { - redirectUri2 = DEFAULT_REDIRECT_URI; + redirectUri2 = DEFAULT_REDIRECT_URI; + } + String redirectUri3 = obj.getRedirectUri(); + if (null == redirectUri3) { + redirectUri3 = DEFAULT_REDIRECT_URI; } - String redirectUri3 = obj.getRedirectUri(); - if (null == redirectUri3) { - redirectUri3 = DEFAULT_REDIRECT_URI; - } - return this.getGithubWebUri().equals(obj.getGithubWebUri()) && - this.getGithubApiUri().equals(obj.getGithubApiUri()) && - this.getClientID().equals(obj.getClientID()) && - this.getClientSecret().equals(obj.getClientSecret()) && - this.getOauthScopes().equals(obj.getOauthScopes()) && - redirectUri2.equals(redirectUri3); + return this.getGithubWebUri().equals(obj.getGithubWebUri()) + && this.getGithubApiUri().equals(obj.getGithubApiUri()) + && this.getClientID().equals(obj.getClientID()) + && this.getClientSecret().equals(obj.getClientSecret()) + && this.getOauthScopes().equals(obj.getOauthScopes()) && redirectUri2.equals(redirectUri3); } else { return false; }