github-oauth-plugin
github-oauth-plugin copied to clipboard
JENKINS-50122 Fix rights management for workflow jobs
Normal workflow jobs does not have the BranchJobProperty set. Only those created by multibranch pipelines have it.
However, the previous code filtered all workflow jobs which hadn't this property when searching for their declared repository (in order to match the github repository permissions to the job).
The result is the plugin GitHub Authorization Settings denies 'CANCEL' permission to non admin users for all normal workflow jobs since it can't find their repository names.
This PR is a simplified version of the (very) old PR https://github.com/jenkinsci/github-oauth-plugin/pull/99 by @masterzen. If people are interested, I can create another PR to support permission checks on every repositories declared in pipelines like he did in his PR. But I prefered to keep things as simple as possible to increase chances of a quick merge.
I just deployed it on my jenkins instance and it works fine but I would prefer to install an official version.
@samrocketman can you take a look on this when you have time?
From my point of view, this fix isn't very risky since it only take care of a specific case. Most of the code modified are unit tests.
Thanks
I haven't had a chance to fully look at this but I notice this duplicates #99
Have you tested this in your own Jenkins instance or a test instance?
I actually use it on our production jenkins instance (instance I recreated a few times from scratch with helm + JCasC). I installed the fix before creating this PR, so it has been a few months now.
It is mostly a rewrite of the old #99 PR because it wasn't working with the last changes from current master and I also took into account the reviews.
Is the wildcard import the only blocker? I'm mostly into python, but if this is the only blocker I can try to setup a java environment here to fix it.
I would really love to see this fixed 😀
I’m not as familiar with these parts of the code base so I need to test it more. I also need to dig into the blame logs to figure out why these conditionals were added to begin with. From my perspective the change makes sense I just don’t understand why they had the conditionals.
Edit: The last plugin release was basically “make this plugin no longer completely broken for everyone” so I tried limiting what went out. Now that it’s stable I can look at this again.
Is there something I can do to fix this issue? This is a big issue for me.
Is this still missing something? Can I help with something do get this merged?
I fixed what @samrocketman wanted I believe.
@emoreth, I don't think you should wait for this to be merged. You will be disappointed.
I made the fix the 14 may then I went to the jenkins mailing lists to ask for it to be merged without success. One year ago, @masterzen already proposed this fix in a PR but it was never merged either.
What I ended up doing is building the hpi with the patch and install it directly on my jenkins. You can do it by dropping it in the plugins directory and restart your jenkins or using the UI (Manage jenkins => Manage plugins => advanced tab => upload plugin).
Here is the HPI(remove the .TXT extension) with the patch if you don't want to build it: github-oauth.hpi.TXT
Well, that's bad! 😞
Thank you for your help @jpigree !
Hello, I can take a look at this again. Apologies for the delays I know you worked hard on this.
Any updates @samrocketman ? I do prefer an official release but I can use the suggested alternative if you don't have the time.
Hey @jpigree
The result is the plugin GitHub Authorization Settings denies 'CANCEL' permission to non admin users for all normal workflow jobs since it can't find their repository names.
Is this true? We have multibranch pipeline and non admin users can't cancel builds
I actually use it on our production jenkins instance (instance I recreated a few times from scratch with helm + JCasC). I installed the fix before creating this PR, so it has been a few months now.
Could you point out how can I install custom plugin in my jenkins instance?
TIA