github-oauth-plugin icon indicating copy to clipboard operation
github-oauth-plugin copied to clipboard

JENKINS-50122 Authorization should work with basic Workflow jobs

Open masterzen opened this issue 7 years ago • 8 comments

Not all Jenkins pipeline job have the BranchJob property. In that case the authorization wouldn't work. This patch makes it work for such job.

masterzen avatar Mar 13 '18 17:03 masterzen

@jenkinsci/code-reviewers please take a look at this change. @Wadeck thanks for your help in other reviews. I really appreciate your efforts.

samrocketman avatar Apr 11 '18 19:04 samrocketman

No worries @Wadeck thanks for your time.

samrocketman avatar Apr 12 '18 13:04 samrocketman

@samrocketman I just upgraded Jenkins from 2.138.1 to 2.138.2 and suddenly any jobs that didn't have a github url associated are giving 404s. I can fix that for one-off jobs to associate them with the most relevant repo, but the big breaking issue here is that GithubOrg Folders suddenly gave 404s to non-admin users. I don't know if this PR is a potential fix for that, but it looks at least related.

I'd love to file a ticket, but https://issues.jenkins-ci.org is timing out for me. I don't know what changed in Jenkins itself to trigger the bad behavior with github auth, but I'd assume the default behavior for a github org folder is to allow view/read on the folder if the user is a member of the associated org.

sgtcoolguy avatar Oct 11 '18 19:10 sgtcoolguy

I would have to look. Upgrading Jenkins doesn’t normally affect anything. Upgrading plugins is a different matter. Hard to say without knowing old plugin versions and plugin versions after upgrade. Your best bet is to file a new issue.

Edit: this patch has nothing to do with GitHub org or multi branch pipeline jobs. It is only meant for regular old pipeline jobs.

samrocketman avatar Oct 11 '18 23:10 samrocketman

@samrocketman Apologies upfront! if this is not the code of conduct for starting a discussion. This is the only way I'm authorized here to start a conversation. I had run in to this problem earlier, the normal pipeline jobs with github commit authorization strategy do not go hand in hand. The githuborgmembershipacl can only fetch permissions to multibranch pipeline jobs based off github org names, repsoitories and branch source properties. I have few questions and appreciate your time if you can answer few of them....

  1. Can this #PR be considered valid to allow authorizations for pipeline jobs with neither jobs named as Github Organizations nor include Branch Properties.
  2. If not can this be a new feature to consider and contribute to.
  3. Is it out of github oauth plugin's scope

pruthvi6767 avatar Nov 07 '18 22:11 pruthvi6767

@samrocketman @masterzen @jglick Hi. I just encountered the bug this PR is supposed to fix.

I use this plugin with the "Github commiter authorization strategy" and I have users unable to cancel builds from simple pipelines because those pipelines misses the BranchJobProperty property (confirmed it by running the problematic code in a pipeline).

I also confirm that the fix in this PR works in my case (also confirmed it by running the code in a pipeline).

What needs to be done to finish it? Or as a workaround is there a way to populate the BranchJobProperty in simple pipelines to make the current code work?

Thanks

jpigree avatar Apr 30 '19 18:04 jpigree

@jpigree ,

I was unfortunately unable to do the requested modifications, mostly because I'm really not familiar with Jenkins internals.

But, since we're using pipelines that are not multibranch and have also multiple scms, we're using my fork locally for more than 6 months without issues so far. Unfortunately this code is far from being ready to be submitted as there's no tests...

I'll try to create a PR with those changes if I can find the time in the coming weeks :)

masterzen avatar Apr 30 '19 20:04 masterzen

@masterzen.

Thank you for your quick reply. Yeah. I looked at your fork and it seems you have a few tests already, perhaps not enough.

I am mostly a Python developer but perhaps I can help you with prepping the PR? Sadly, I am not familiar with Jenkins internals too. :(

You need more tests and to take into account the comments they gave you, is that all?

I voted up your issue in their JIRA but we are kinda alone on this. Perhaps people using this feature only uses multibranch pipelines. Or they don't care to have their users not having the CANCEL button.

I wonder what is this "BranchJob" property though. Why standard pipelines do not have it?

jpigree avatar May 01 '19 00:05 jpigree