[JENKINS-34835] Authorities from team slug #124
Conversation
PR jenkinsci#116 loads the team based on the slug if it's available but the authorities list is still using the team name. This means that you can set the team in the matrix in global security but you will then be denied access because the authorities list uses the team name. You can see this if you go to /whoAmI The team name is shown against the user.
The GithubAuthenticationToken constructor now uses the github team slug if it's set. I've updated the tests so the slug is set here as well. I haven't included a separate test for having a team slug set vs not having it set but I can add that in if it's needed.
MancunianSam
changed the title
|
It will really great if this PR is merged and released with the new version of the plugin. I was facing this issue yesterday and figured out the root-cause by trial and error. |
We are using our own fork of this plugin until the change to allow teams by slug is merged into the main repo: jenkinsci/github-oauth-plugin#124 Including this plugin in the list causes the build to fail because it's been built manually.
|
@kumadee apologies for the delay. I've been on a bit of a hiatus but I'm going to start ramping up contributions to Jenkins again. I'll start by reviewing pull requests for github-oauth-plugin and try to address them (feedback or merge, etc) Thanks for contributing; this has been something on my TODO and I know users want this. |
|
@samrocketman @basil I've updated this branch with the latest from master and the checks are failing. I've looked at the logs and it looks like SIGTERM is being sent on each build and I can't see why. The Windows build has dependency errors but the dependencies it says are missing but which I can see in the repository. Do you know if there is a known issue with the builds failing like this or is this something I need to fix? |
There was a problem hiding this comment.
Thanks for the PR!
Incremental build 0.35-rc429.8f7e6fa35ad6 is available for testing. The incremental build is available from: https://repo.jenkins-ci.org/incrementals/org/jenkins-ci/plugins/github-oauth/0.35-rc429.8f7e6fa35ad6/
@MancunianSam @apemberton @kumadee Can you please test the incremental build and confirm the issue is resolved? For instructions on how to install a custom build, see: https://www.jenkins.io/doc/book/managing/plugins/#advanced-installation
|
@basil I've tested the incremental build and the issue is fixed, thanks. |
|
Released in 0.35. |
|
I know the plugin is still 0.x and I should always read changelogs, but better messaging about this change would have been appreciated (I remember some plugin updates have big warnings on in the update screen). There's a potential to lock yourself out if you already had setup the authorization to work using team names instead of slugs. |
|
Hi @thatsmydoing, thanks for the note. I don't actually use this plugin, and really only got involved with maintaining this plugin to deliver a fix required for the core Guava update, so it's possible I didn't fully understand the impact when merging and releasing this PR. Since you use this plugin, would you be willing to write a short blurb about upgrading? I would be happy to update the GitHub releases page with whatever you write. |
|
Ah, I see. I was hoping for something like BREAKING CHANGE group authorization changed from github team names to team slugs When restricting access via groups (with for example, matrix-based security), and your github team names don't match team slugs (if they contain spaces for example), make sure to add another set of grants using the github team slug prior to upgrading otherwise you could potentially lock yourself out of Jenkins. After upgrading, it should be safe to remove the old github team name grants. For example, if admin access is granted to the team "Jenkins Admin" with slug |
|
Thank you @thatsmydoing. I updated the release notes with the information you provided. |
I've put the commits against JENKINS-34835 as the issue is still open. There was a PR #116 for this which gets the team based on the slug if it's set but the authorities list, which you can see if you go to /whoAmI, is still using the team name so if I set the team using org*slug in global security, I'm no longer allowed access.