[JENKINS-73060] Fix missing authorities #302
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should fix JENKINS-73060.
This updates
loadByUsername2
to always use the authorities retrieved in theGithubAuthenticationToken
when constructing theGithubOAuthUserDetails
object. Previously, this plugin would try to load aGHUser
object (either from theuserByIdCache
or the Github API) and would pass an empty list of authorities when theGHUser
wasnull
.I couldn't find a logical reason for why we would need this check when we do not use the
GHUser
object to set the authorities. This check seems to be a carry over from when theGHUser
was used in thegetGrantedAuthorities
method to fetch/set authorities before 99e3d13.There's also a secondary bug here where
loadUser
will not make the API call to retrieve theGHUser
from the Github API ifgh
isnull
(which is usually the case when the token used to constructGithubAuthenticationToken
is in theusersByTokenCache
). This null check shouldn't be needed due to changes introduced in #61. However, I chose not to remove this check due to possible performance degradation as seen in #256.Testing done
We've been running this patch in our Production environment without issue since June 12. We're not seeing any performance degradation on our Jenkins instance with this patch.
Steps to Reproduce
Clone this project & execute
mvn hpi:run
Install the following additional plugins:
Authorize Project
Role-based Authorization Strategy
Configure the Github plugin as the security realm
Configure
Authorization
to use theRole-Based Strategy
Assign a Github team (i.e.
ORG*TEAMNAME
) the defaultadmin
permission in<jenkins_url>/manage/role-strategy/assign-roles
Configure
Access Control for Builds
to useProject default Build Authorization
->Run as User who Triggered Build
Create a pipeline
Example
Navigate to the build page -> Select
log out
in the top right -> Hit back button -> SelectBuild Now
Build should hang for roughly 1 minute with the following error even though the user should be a member of a Github team with
admin
permissions on the Jenkins instance.Install this patch
Navigate to the build page -> Select
log out
in the top right -> Hit back button -> SelectBuild Now
Issue should no longer occur
Submitter checklist