Allow GitHub app to be installed in multiple orgs #290

timja · 2020-03-24T14:58:57Z

Description

To test it you need multiple organisation, ideally with private repos on both as public repos can be read even if haven't granted access, install the app to both organisations and follow the steps in the documentation

Submitter checklist

[-] Link to JIRA ticket in description, if appropriate.
Change is code complete and matches issue description
[-] Automated tests have been added to exercise the changes
Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

Run the changes and verify that the change matches the issue description
Reviewed the code
Verified that the appropriate tests have been written or valid explanation given

Documentation changes

Link to jenkins.io PR, or an explanation for why no doc changes are needed
doc is in the repo

Users/aliases to notify

@jglick @bitwiseman @dwnusbaum

pom.xml

jglick · 2020-03-24T15:03:25Z

src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java

            }
+            GHAppInstallation appInstallation;
+            if (appInstallations.size() == 1) {
+                appInstallation = appInstallations.get(0);


Not verifying the owner?

I've made an assumption that most installations only work with one org, so I've made it an advanced field and by default if and only if there's one installation the plugin will just pick that.

I get that, I am just saying that in case you did specify an owner, and there is exactly one installation, we are not verifying that the installation is in fact from that owner. Which feels wrong.

jglick · 2020-03-24T15:05:00Z

docs/github-app.adoc

@@ -87,7 +87,8 @@ Fill out the form:
 - App ID: the github app ID, it can be found in the 'About' section of your GitHub app in the general tab.
 - API endpoint (optional, only required for GitHub enterprise this will only show up if a GitHub enterprise server is configured).
 - Key: click add, paste the contents of the converted private key
- Passphrase: do not fill this field, it will be ignored
+- Advanced: (optional) If you've installed your same GitHub app on multiple organisations you need the next step
+  * Owner: the name of the organisation or user, i.e. jenkinsci for https://github.com/jenkinsci


So under this approach you cannot reuse the Jenkins credentials object across orgs, and you need to duplicate the private key. Not as smooth as having Jenkins figure out which installation to work with, but better than nothing, right?

yeah I couldn't see a way to do that, as we don't have the context from the build job at all

Right, it would be a more intrusive patch since you would need to do something along the lines of defining a thread-local org and setting this from a new overload of Connector.connect which passes a repoOwner and then switching as many callers as possible to the new overload. Not sure it is worth the effort since I presume the common case is for the Jenkins installation to be used on only a single org.

Not sure it is worth the effort since I presume the common case is for the Jenkins installation to be used on only a single org.

What about for GitHub Enterprise? It doesn't seem to be possible to install an app at the Enterprise level. My Jenkins instance works on repos across many organizations in a GitHub Enterprise instance.

I do not have GHE to test against but in that case with the current patch you would need to first install the app in each such org, then create a separate Jenkins credentials item for each installation.

Yes, those were all public repos.

is there any way that we can get this owner based on the job config as it would already have the github repo information ?

The credential isn’t tied to a job it’s independent, not safely, there may be some possible hacks to do it though

My company uses Jenkins in a similar fashion as the company described by @nrayapati.

We deployed a single jenkins GitHub App that may be installed by any organization on our GHE instance. There are several hundred such organizations.

We allow our engineers to create jobs as needed within their own Jenkins folders and leverage the single GitHub App Credential.

Since any engineer can install our GitHub App on their organization, and any engineer can create an organization, we would be in the situation of needing to create a Credential for every organization as it's created.

I, too, would like to see support for a single global Credential to work for jobs across multiple organizations.

@nrayapati can you please file a Jira ticket (As this PR has already been merged and released) so that we can centralize the discussion about this use case on that ticket?

https://issues.jenkins-ci.org/browse/JENKINS-62220

Can we just use the org name in repo instead of the org name in credential config?

docs/github-app.adoc

Co-Authored-By: Liam Newman <[email protected]>

robbiemcmichael · 2020-03-25T01:21:26Z

src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java


        ) {
            GitHubAppCredentials gitHubAppCredential = new GitHubAppCredentials(
                    CredentialsScope.GLOBAL, "test-id-not-being-saved", null,
                    appID, Secret.fromString(privateKey)
            );
            gitHubAppCredential.setApiUri(apiUri);
+            gitHubAppCredential.setOwner(owner);


Commenting here since the CredentialsScope.GLOBAL part is just above this:

Do you know if it's possible to use system scope credentials with the branch source plugin? One issue we ran into with our credentials plugin was making the credentials for an org only available to the builds within that org.

It is not possible to use system scope credentials, but you can define credentials on an org folder so they are not accessible outside that folder.

bitwiseman · 2020-03-31T17:26:58Z

What the...? Not sure why these tests started failing.

pom.xml

src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java

jglick · 2020-03-31T18:12:57Z

src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java

            }
+            GHAppInstallation appInstallation;
+            if (appInstallations.size() == 1) {
+                appInstallation = appInstallations.get(0);


I get that, I am just saying that in case you did specify an owner, and there is exactly one installation, we are not verifying that the installation is in fact from that owner. Which feels wrong.

...in/resources/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials/help-owner.html

…le-orgs

…bAppCredentials.java Co-Authored-By: Jesse Glick <[email protected]>

…GitHubAppCredentials/help-owner.html Co-Authored-By: Jesse Glick <[email protected]>

…ithub.com:timja/github-branch-source-plugin into allow-github-app-to-be-installed-in-multiple-orgs

oleg-nenashev

I am not sure "owner" is a correct terminology here, because it might be confused with the GitHub App owner (each app is formally owned by a GitHub account). Maybe installationTarget or targetAccount is better

oleg-nenashev · 2020-04-02T12:29:30Z

docs/github-app.adoc

@@ -87,7 +87,8 @@ Fill out the form:
 - App ID: the github app ID, it can be found in the 'About' section of your GitHub app in the general tab.
 - API endpoint (optional, only required for GitHub enterprise this will only show up if a GitHub enterprise server is configured).
 - Key: click add, paste the contents of the converted private key
- Passphrase: do not fill this field, it will be ignored
+- Advanced: (optional) If you've installed your same GitHub app on multiple organisations you need the next step
+  * Owner: the name of the organisation or user, i.e. jenkinsci for https://github.com/jenkinsci


Good enough IMHO

timja · 2020-04-02T12:32:52Z

I am not sure "owner" is a correct terminology here, because it might be confused with the GitHub App owner (each app is formally owned by a GitHub account). Maybe installationTarget or targetAccount is better

possibly but I copied the text that is used for it when configuring it in a pipeline organisation,

the help text and docs explain it better

nrayapati · 2020-04-25T23:36:43Z

Sorry for making a comment on this closed PR: but I had to make a comment here.

#290 (comment)

Let me know your thoughts, I can log a JIRA with more details. Thank you!

Support multiple orgs / users

318b60a

timja mentioned this pull request Mar 24, 2020

Add support for GitHub app authentication #269

Merged

jglick reviewed Mar 24, 2020

View reviewed changes

bitwiseman reviewed Mar 24, 2020

View reviewed changes

docs/github-app.adoc Outdated Show resolved Hide resolved

Update docs/github-app.adoc

f7ad933

Co-Authored-By: Liam Newman <[email protected]>

robbiemcmichael reviewed Mar 25, 2020

View reviewed changes

bitwiseman closed this Mar 31, 2020

bitwiseman reopened this Mar 31, 2020

jglick approved these changes Mar 31, 2020

View reviewed changes

timja and others added 4 commits March 31, 2020 19:29

Merge branch 'master' into allow-github-app-to-be-installed-in-multip…

9887b6f

…le-orgs

Update src/main/java/org/jenkinsci/plugins/github_branch_source/GitHu…

83f1d74

…bAppCredentials.java Co-Authored-By: Jesse Glick <[email protected]>

Update src/main/resources/org/jenkinsci/plugins/github_branch_source/…

aee2719

…GitHubAppCredentials/help-owner.html Co-Authored-By: Jesse Glick <[email protected]>

Merge branch 'allow-github-app-to-be-installed-in-multiple-orgs' of g…

46c5a13

…ithub.com:timja/github-branch-source-plugin into allow-github-app-to-be-installed-in-multiple-orgs

oleg-nenashev approved these changes Apr 2, 2020

View reviewed changes

bitwiseman approved these changes Apr 7, 2020

View reviewed changes

bitwiseman merged commit 28f9a64 into jenkinsci:master Apr 7, 2020

jglick mentioned this pull request Apr 7, 2020

Cache app installation token #291

Merged

8 tasks

jglick mentioned this pull request Mar 8, 2021

feat: allow one app to authenticate hub4j/github-api#1053

Closed

6 tasks

jglick mentioned this pull request Mar 23, 2022

[JENKINS-62220] Automatically select owner for GitHubAppCredentials acc. to context #527

Merged

Allow GitHub app to be installed in multiple orgs #290

Allow GitHub app to be installed in multiple orgs #290

Uh oh!

Conversation

timja commented Mar 24, 2020

Description

Submitter checklist

Reviewer checklist

Documentation changes

Users/aliases to notify

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robbiemcmichael Mar 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bitwiseman commented Mar 31, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

oleg-nenashev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timja commented Apr 2, 2020

Uh oh!

nrayapati commented Apr 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

robbiemcmichael Mar 25, 2020 •

edited

Loading

nrayapati commented Apr 25, 2020 •

edited

Loading