There was/is no test coverage of the GithubSecurityRealm class so I'm unsure if it's safe to modify the constructor's signature.

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

👍

Should the default scope still be "repo,read:org" ?
I think a more conservative default would be better for most installations, even if it does forbid some features.

Also, a huge thanks and kudos to @jhoblitt for taking the time and energy to figure out how to do this!

I'd default to user or user:email, but as long as it's configurable, I'm fine with any default... Just my $.1 ...

I've updated the PR so that instead of modifying the signature of the primary GithubSecurityRealm constructor, a new constructor is defined and the previous method is marked as @Deprecated. This should avoid any public API breakage.

@s0undt3ch I believe if a user's org membership and email address is public then no scope would be necessary (but I haven't tired requesting a GH oauth token without scope(s)).

I'm now wondering if read:org grants access to a user's email address or if the plugin will break for a user without a public address...

@s0undt3ch I've attempted to test the behavior of the user:email scope. I created a test user & org and deleted the user from jenkins via the delete_user() method in puppet_helper.grovvy between each test (note that Jenkins also needs to be restarted to clear caching). The user:email scope appears to have no affect on being able to retrieve a user's email address. Only changing the user's github profile to have a public email address worked. I don't know if this is due to GH's scope's, org.kohsuke.github.GHUser, or something in the plugin itself. I can say that adding user:email by default doesn't make any sense at this point.

I've opened a Jira issue for the inability to discover private email addresses. https://issues.jenkins-ci.org/browse/JENKINS-27764

We need this too! 👍

Well, user:email "Grants read access to a user’s email addresses.", so I believe that's something not being pulled by either the github-api or... dunno... should the github-oauth-plugin retrieve and update the user's email address if it has access to it?

@s0undt3ch I agree it should. However, this is a feature PR to "make Github OAuth Scopes configurable", which is why I opened https://issues.jenkins-ci.org/browse/JENKINS-27764

@jhoblitt agreed.

Voted up!

If anyone is interested in testing this PR without building it from source, I've made an hpi file available. https://s3-us-west-2.amazonaws.com/github-oauth-plugin/github-oauth.hpi

Uploaded to Jenkins but I can only restart it in about 1 or 2 hours... I'll get back to you.

Working perfectly!

When can we expect this merged and released?

👍

@mocleiri @skottler are you guys still interested in maintaining the plugin? The last project activity is Nov 2014 (merge). I'm willing to take over maintaining the plugin if you like.

Probably worth checking if anything needs updated here with regard to https://developer.github.com/changes/2015-06-24-breaking-changes-to-organization-permissions-are-now-official/

Looks like we're all good with those changes.

Personally I think this should also default to public_repo rather than repo as asking people to give complete access to all their private repos to login to Jenkins seems like a bad idea to have on by default (and is the reason I hadn't upgraded from 0.14).

@jhoblitt Thanks so much for the .hpi; it's working great for me and I've got access to my Jenkins again! You are a lifesaver ⚡

Hi! Thank you so much for working on this! We tried downloading the plugin from https://s3-us-west-2.amazonaws.com/github-oauth-plugin/github-oauth.hpi, but we got the error that starts with:

java.io.IOException: {"message":"You need at least read:org scope or user scope to list your organizations.","documentation_url":"https://developer.github.com/v3/orgs/#list-your-organizations"}
    at org.kohsuke.github.Requester.handleApiError(Requester.java:493)
    at org.kohsuke.github.Requester._to(Requester.java:245)
...

We just put the .hpi file $JENKINS/plugins. Should we have done something differently?

(Note: the github-api plugin is installed, and was the one installed via the web UI)

Update: I'm not sure what I did wrong in the first place, but manually adding <oauthScopes>read:org</oauthScopes> to the <securityRealm> section of the config.xml fixed it, and it's working great, thank you @jhoblitt!!

I think this PR has jumped up to the critical path, since without it most of us will be locked out of our Jenkins servers by the GitHub API change.

I'm also pulling down @jhoblitt's HPI file at this point, since that way I can at least add the required read:org scope.

Is the consensus that the default scope should be changed to be solely read:org?

@cyli The easiest way to solve that problem is to manually edit the ~jenkins/config.xml to include the oauthScopes setting set to read:org.
For example, if you had this:

  <securityRealm class="org.jenkinsci.plugins.GithubSecurityRealm">
    <githubWebUri>https://github.com</githubWebUri>
    <githubApiUri>https://api.github.com</githubApiUri>
    <clientID>REDACTED</clientID>
    <clientSecret>REDACTED</clientSecret>
  </securityRealm>

change it to

  <securityRealm class="org.jenkinsci.plugins.GithubSecurityRealm">
    <githubWebUri>https://github.com</githubWebUri>
    <githubApiUri>https://api.github.com</githubApiUri>
    <clientID>REDACTED</clientID>
    <clientSecret>REDACTED</clientSecret>
    <oauthScopes>read:org</oauthScopes>
  </securityRealm>

EDIT: Oh, and restart the Jenkins process afterwards, of course.

I think I already voiced support for setting the permissions to solely read:org by default.
Just in case I didn't, here I am on record stating that opinion.

@sirosen Thank you! I just figured that out a second ago too :) I appreciate the quick response!

Installed the patch, changed the XML, and restarted. I keep getting this

---- Debugging information ----
class               : org.jenkinsci.plugins.GithubSecurityRealm
required-type       : org.jenkinsci.plugins.GithubSecurityRealm
converter-type      : hudson.util.XStream2$AssociatedConverterImpl
path                : /hudson/securityRealm/oauthScopes
line number         : 39
class[1]            : hudson.model.Hudson
converter-type[1]   : hudson.util.RobustReflectionConverter
version             : not available
-------------------------------

UPDATE: Finally got it working removing github-oauth.bak

Big thanks to @jhoblitt & @sirosen

@cyli I'm confused, why didn't you edit the auth scopes via the www UI?

@jhoblitt For some reason trying to use the www UI kept netting me that error.

Is the consensus that the default scope should be changed to be solely read:org?

I think it'll confusingly break if it doesn't have at least that.

Tested. Thanks. Please update the official plugin so I can back out the hotfix.

any word on this percolating to trunk?

I posted to the Jenkins dev list last week, and bumped my thread a moment ago.
I don't think a 1-2 week timeline for someone other than the plugin maintainer to step in is at all unreasonable, but I'm going to keep pushing in that channel in the hopes that we can get merging this expedited.

There's also a pending request to take over ownership of this plugin that went to the dev list earlier this week.

This is a kind of vacuous update, but I figure that several others who are watching this PR would want to know that there's listhost activity associated with this.

Thanks @sirosen!

@sirosen 🤘

Links to the relevant discussions for anyone interested:

• https://groups.google.com/forum/#!searchin/jenkinsci-dev/github-oauth-plugin|sort:date/jenkinsci-dev/ffVYBmGhM78/o-S0O7iTS_YJ
• https://groups.google.com/forum/#!searchin/jenkinsci-dev/github-oauth-plugin|sort:date/jenkinsci-dev/T4xbDk2ZGHI/HBoz1_cvKIYJ

@suryagaddipati Are you still maintaining this plugin? @sirosen asked for commit access, WDYT?

@sirosen I'd be willing to take on co-maintainership. There's some improvements to this PR that could be made I'd be willing to tackle if it looked like it was possible to get it merged. ;)

@daniel-beck i was never a maintainer of this plugin(although i do have commit rights). I believe @skottler is the maintainer AFAIK.

I personally don't have any issues with @sirosen getting commit access. More the merrier 👍

@suryagaddipati Thanks! Since you performed the last few releases I assumed you had taken over.

@skottler Do you object to getting a co-maintainer or two in this plugin?

I've currently got commit access as well. I reached out to @skottler via email and he agreed it was fine if I took it over. I posted to the dev mailing list about taking it over as well. I'm also fine with co-maintaining the plugin. I'm all for whatever improves the quality and consistency of releases. I currently maintain the slack plugin.

@samrocketman Sorry, I completely missed your emails. I made you default assignee for new issues in Jira.

Please let the dev list know if commit access for @sirosen is still desired, right now it looks like there are a few devs around to handle PRs. Feel free to kick me on IRC if that gets overlooked again.

I'll start testing and merging this coming Sunday after I get back from vacation (i.e. tomorrow).

I don't have any issues with @sirosen getting commit access though I'd still prefer development to occur with code reviews and pull requests.

I have no particular drive to get myself commit access on this project. If someone with administrative rights on the repo feels like granting them, cool beans, but I'm not going to push for it as long as there are at least two people watching PRs on this repo.

Looks like this got merged with the default scopes set to "repo,read:org"
I'm going to open a separate PR to fix that to "read:org" only, and we can argue there about what it should be.