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

[JENKINS-70548] Allow GitHub Webhooks to be created by users with custom roles

Open iodeslykos opened this issue 2 years ago • 6 comments

Would really like for custom roles to be used in management of Webhooks, because otherwise Jenkins needs to have admin permissions on every repository where managed Webhooks are desired. This is my attempt at making this possible.

  • Remove admin check, allowedToManageHooks() is enough.

Intended to solve https://issues.jenkins.io/browse/JENKINS-70548.

Testing done

Passing tests and building successfully. A change in tests does not appear to be required.

[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  04:14 min
[INFO] Finished at: 2024-01-24T16:22:27-07:00
[INFO] ------------------------------------------------------------------------

Submitter checklist

  • [X] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • [X] Ensure that the pull request title represents the desired changelog entry
  • [X] Please describe what you did
  • [X] Link to relevant issues in GitHub or Jira
  • [X] Link to relevant pull requests, esp. upstream and downstream changes
  • [X] Ensure you have provided tests - that demonstrates feature works or fixes the issue

iodeslykos avatar Jan 24 '24 23:01 iodeslykos

@lanwen Any chance of getting this reviewed and (hopefully) merged?

iodeslykos avatar Feb 09 '24 03:02 iodeslykos

@KostyaSha would you mind taking a look at this? This feature would be very convenient in my team's permission structure.

mcmathews avatar Feb 09 '24 22:02 mcmathews

We can try, in case of issues could be reverted back

KostyaSha avatar Feb 10 '24 10:02 KostyaSha

@KostyaSha Any ETA on when it could be tried? This is a major factor for our effort to ensure principal of least privilege.

iodeslykos avatar Feb 14 '24 15:02 iodeslykos

It would be better to move it as option. Admin check was added to identify that returned object will be able to manage hooks. Now it can return connection that will lead to errors. Also AFAIR it was impossible to make an github API check whether user can manage hook and i sent request to github support. AFAIR permissions were listed in http headers and github-api library didn't support it (now it already supports afair).

KostyaSha avatar Feb 14 '24 16:02 KostyaSha

@KostyaSha I'm not certain how you'd want this to be implemented.

For instance, it doesn't look like the library being used for the GitHub API supports custom roles (would return UNKNOWN): https://github.com/hub4j/github-api/blob/51b3a06fe364a9667d8c4e0b64abc780cff8d371/src/main/java/org/kohsuke/github/GHPermissionType.java#L9C1-L20C17

Are you saying that the admin check should still happen after the webhook permission is confirmed and simply not cause failure?

iodeslykos avatar Feb 23 '24 17:02 iodeslykos