Add redirect_uri oauth param and configuration

src/main/java/org/jenkinsci/plugins/GithubAuthorizationStrategy.java

@@ -30,6 +30,7 @@ of this software and associated documentation files (the "Software"), to deal
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.multibranch.BranchJobProperty;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -140,6 +141,23 @@ public String getAdminUserNames() {
         return StringUtils.join(rootACL.getAdminUserNameList().iterator(), ", ");
     }
 
+    /** Set the agent username. We use a setter instead of a constructor to make this an optional field
+     *  to avoid a breaking change.
+     * @see org.jenkinsci.plugins.GithubRequireOrganizationMembershipACL#setAgentUserName(String)
+     */
+    @DataBoundSetter
+    public void setAgentUserName(String agentUserName) {
+        rootACL.setAgentUserName(agentUserName);
+    }
+
+    /**
+     * @return agentUserName
+     * @see GithubRequireOrganizationMembershipACL#getAgentUserName()
+     */
+    public String getAgentUserName() {
+        return rootACL.getAgentUserName();
+    }
+
     /**
      * @return isUseRepositoryPermissions
      * @see org.jenkinsci.plugins.GithubRequireOrganizationMembershipACL#isUseRepositoryPermissions()
@@ -208,6 +226,7 @@ public boolean equals(Object object){
             GithubAuthorizationStrategy obj = (GithubAuthorizationStrategy) object;
             return this.getOrganizationNames().equals(obj.getOrganizationNames()) &&
                 this.getAdminUserNames().equals(obj.getAdminUserNames()) &&
+                this.getAgentUserName().equals(obj.getAgentUserName()) &&
                 this.isUseRepositoryPermissions() == obj.isUseRepositoryPermissions() &&
                 this.isAuthenticatedUserCreateJobPermission() == obj.isAuthenticatedUserCreateJobPermission() &&
                 this.isAuthenticatedUserReadPermission() == obj.isAuthenticatedUserReadPermission() &&

src/main/java/org/jenkinsci/plugins/GithubRequireOrganizationMembershipACL.java

@@ -26,6 +26,7 @@ of this software and associated documentation files (the "Software"), to deal
  */
 package org.jenkinsci.plugins;
 
+import hudson.model.*;
 import org.acegisecurity.Authentication;
 import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
@@ -41,10 +42,6 @@ of this software and associated documentation files (the "Software"), to deal
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
 
-import hudson.model.AbstractItem;
-import hudson.model.AbstractProject;
-import hudson.model.Describable;
-import hudson.model.Item;
 import hudson.plugins.git.GitSCM;
 import hudson.plugins.git.UserRemoteConfig;
 import hudson.security.ACL;
@@ -64,6 +61,7 @@ public class GithubRequireOrganizationMembershipACL extends ACL {
 
     private final List<String> organizationNameList;
     private final List<String> adminUserNameList;
+    private String agentUserName;
     private final boolean authenticatedUserReadPermission;
     private final boolean useRepositoryPermissions;
     private final boolean authenticatedUserCreateJobPermission;
@@ -102,6 +100,12 @@ public boolean hasPermission(@NonNull Authentication a, @NonNull Permission perm
                 return true;
             }
 
+            // Grant agent permissions to agent user
+            if (candidateName.equalsIgnoreCase(agentUserName) && checkAgentUserPermission(permission)) {
+                log.finest("Granting Agent Connect rights to user " + candidateName);
+                return true;
+            }
+
             // Are they trying to read?
             if (checkReadPermission(permission)) {
               // if we support authenticated read return early
@@ -153,6 +157,12 @@ else if (testBuildPermission(permission) && isInWhitelistedOrgs(authenticationTo
                 return true;
             }
 
+            // Grant agent permissions to agent user
+            if (authenticatedUserName.equalsIgnoreCase(agentUserName) && checkAgentUserPermission(permission)) {
+                log.finest("Granting Agent Connect rights to user " + authenticatedUserName);
+                return true;
+            }
+
             if (authenticatedUserName.equals("anonymous")) {
                 if (checkJobStatusPermission(permission) && allowAnonymousJobStatusPermission) {
                     return true;
@@ -239,6 +249,13 @@ private boolean checkReadPermission(@NonNull Permission permission) {
                 || id.equals("hudson.model.Item.Read"));
     }
 
+    private boolean checkAgentUserPermission(@NonNull Permission permission) {
+        return permission.equals(Hudson.READ)
+                || permission.equals(Computer.CREATE)
+                || permission.equals(Computer.CONNECT)
+                || permission.equals(Computer.CONFIGURE);
+    }
+
     private boolean checkJobStatusPermission(@NonNull Permission permission) {
         return permission.getId().equals("hudson.model.Item.ViewStatus");
     }
@@ -314,10 +331,11 @@ public GithubRequireOrganizationMembershipACL(String adminUserNames,
         }
 
         this.item = null;
+        this.agentUserName = ""; // Initially blank - populated by a setter since this field is optional
     }
 
     public GithubRequireOrganizationMembershipACL cloneForProject(AbstractItem item) {
-      return new GithubRequireOrganizationMembershipACL(
+        GithubRequireOrganizationMembershipACL acl = new GithubRequireOrganizationMembershipACL(
           this.adminUserNameList,
           this.organizationNameList,
           this.authenticatedUserReadPermission,
@@ -328,6 +346,8 @@ public GithubRequireOrganizationMembershipACL cloneForProject(AbstractItem item)
           this.allowAnonymousReadPermission,
           this.allowAnonymousJobStatusPermission,
           item);
+        acl.setAgentUserName(agentUserName);
+        return acl;
     }
 
     public GithubRequireOrganizationMembershipACL(List<String> adminUserNameList,
@@ -362,6 +382,11 @@ public List<String> getAdminUserNameList() {
         return adminUserNameList;
     }
 
+    public void setAgentUserName(String agentUserName) {
+        this.agentUserName = agentUserName;
+    }
+    public String getAgentUserName() { return agentUserName; }
+
     public boolean isUseRepositoryPermissions() {
         return useRepositoryPermissions;
     }

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();
     }
 

src/main/resources/org/jenkinsci/plugins/GithubAuthorizationStrategy/config.jelly

@@ -8,6 +8,10 @@
             <f:textbox />
         </f:entry>
 
+        <f:entry title="Agent User Name"  field="agentUserName" help="/plugin/github-oauth/help/auth/agent-user-name-help.html" >
+            <f:textbox />
+        </f:entry>
+
         <f:entry title="Participant in Organization"  field="organizationNames" help="/plugin/github-oauth/help/auth/organization-names-help.html">
             <f:textbox />
         </f:entry>

src/main/resources/org/jenkinsci/plugins/GithubSecurityRealm/config.jelly

@@ -23,5 +23,10 @@
         <f:entry title="OAuth Scope(s)" field="oauthScopes" help="/plugin/github-oauth/help/realm/oauth-scopes-help.html">
             <f:textbox default="${descriptor.getDefaultOauthScopes()}" />
         </f:entry>
+
+        <f:entry title="Redirect URI" field="redirectUri" help="/plugin/github-oauth/help/realm/redirect-uri-help.html">
+            <f:textbox default="${descriptor.getDefaultRequestUri()}" />
+        </f:entry>
+
     </f:section>
 </j:jelly>

src/main/webapp/help/auth/agent-user-name-help.html

@@ -0,0 +1,3 @@
+<div>
+If you are using inbound Jenkins agents, this is the user that is used for authenticating agents. This user will receive rights to create, connect and configure agents.
+</div>

src/main/webapp/help/realm/redirect-uri-help.html

@@ -0,0 +1,3 @@
+<div>
+An optional redirect URI to be used by GitHub.  See <a href="https://docs.github.com/en/developers/apps/building-github-apps/identifying-and-authorizing-users-for-github-apps">https://docs.github.com/en/developers/apps/building-github-apps/identifying-and-authorizing-users-for-github-apps</a>.
+</div>

src/test/java/org/jenkinsci/plugins/GithubRequireOrganizationMembershipACLTest.java

@@ -60,6 +60,7 @@ of this software and associated documentation files (the "Software"), to deal
 import java.util.Collections;
 import java.util.List;
 
+import hudson.model.Computer;
 import hudson.model.Hudson;
 import hudson.model.Item;
 import hudson.model.Messages;
@@ -130,7 +131,7 @@ private void mockJenkins(MockedStatic<Jenkins> mockedJenkins) {
             new GrantedAuthority[]{new GrantedAuthorityImpl("anonymous")});
 
     private GithubRequireOrganizationMembershipACL createACL() {
-        return new GithubRequireOrganizationMembershipACL(
+        GithubRequireOrganizationMembershipACL acl = new GithubRequireOrganizationMembershipACL(
                 "admin",
                 "myOrg",
                 authenticatedUserReadPermission,
@@ -140,6 +141,8 @@ private GithubRequireOrganizationMembershipACL createACL() {
                 allowAnonymousCCTrayPermission,
                 allowAnonymousReadPermission,
                 allowAnonymousJobStatusPermission);
+        acl.setAgentUserName("agent");
+        return acl;
     }
 
     private GithubRequireOrganizationMembershipACL aclForProject(Project project) {
@@ -554,6 +557,30 @@ public void testCannotReadRepositoryWithInvalidRepoUrl() throws IOException {
         }
     }
 
+    @Test
+    public void testAgentUserCanCreateConnectAndConfigureAgents() {
+        GithubAuthenticationToken authenticationToken = Mockito.mock(GithubAuthenticationToken.class);
+        Mockito.when(authenticationToken.isAuthenticated()).thenReturn(true);
+        Mockito.when(authenticationToken.getName()).thenReturn("agent");
+        GithubRequireOrganizationMembershipACL acl = createACL();
+
+        assertTrue(acl.hasPermission(authenticationToken, Computer.CREATE));
+        assertTrue(acl.hasPermission(authenticationToken, Computer.CONFIGURE));
+        assertTrue(acl.hasPermission(authenticationToken, Computer.CONNECT));
+    }
+
+    @Test
+    public void testAuthenticatedCanNotCreateConnectAndConfigureAgents() {
+        GithubAuthenticationToken authenticationToken = Mockito.mock(GithubAuthenticationToken.class);
+        Mockito.when(authenticationToken.isAuthenticated()).thenReturn(true);
+        Mockito.when(authenticationToken.getName()).thenReturn("authenticated");
+        GithubRequireOrganizationMembershipACL acl = createACL();
+
+        assertFalse(acl.hasPermission(authenticationToken, Computer.CREATE));
+        assertFalse(acl.hasPermission(authenticationToken, Computer.CONFIGURE));
+        assertFalse(acl.hasPermission(authenticationToken, Computer.CONNECT));
+    }
+
     @Test
     public void testAnonymousCanViewJobStatusWhenGranted() {
         this.allowAnonymousJobStatusPermission = true;