👍

Thanks for the PR I'll open this for additional code review, test it to reproduce, and merge if it checks out. I'll let it be open for at least a day or two to get 24hr exposure.

If you think this applies to any Jenkins github-oauth JIRA issues, please link them as well.

Since this is considered a regression; could unit tests be added as well? This seems to be a fix for something that was fixed in the past (regression).

@samrocketman This is simple null check that should always have been there as authenticationToken.loadUser(getUsername()) can return null. Only regression this null check can do is to avoid NPE:)

Not easy to write unit tests unless there are existing mocks for GitHub authentication and authorization. If there are existing test that test this part of code, I can look at them as inspiration so please point me to them.

Doesn't look like there's tests for that specific class. However, you can find all tests for this project in https://github.com/jenkinsci/github-oauth-plugin/tree/master/src/test/java/org/jenkinsci/plugins

Regarding an example of where mocking is used; please refer to https://github.com/jenkinsci/github-oauth-plugin/blob/master/src/test/java/org/jenkinsci/plugins/GithubRequireOrganizationMembershipACLTest.java

@samrocketman Thanks, although such simple NPE fix should not mandate a unit test. Will see what I can do.

My request isn't really a mandate. Currently, this project is sorely lacking in unit tests. If you try and can't figure it out then I'll still merge. Though, to me regression indicates this is something that was fixed in the past and was broken again at some point. Without tests it is at risk of being broken again.

Point taken, I will see what I can do. thanks.

I was able to reproduce this locally. Still not really sure how to test it myself. For now, I'll merge it after testing but I think it would be great if you could submit a follow-up PR which includes tests for this scenario.

Tested, all looks good to me.

@samrocketman Can we get a release that includes this patch?

It's breaking use cases in blueocean during GitHub based pipeline creation and any other places (e.g. listing credentials in user scope) and anywhere else user.impersonate() is used with github-oauth plugin.

cc:@i386, @michaelneale

@vivek I didn't realize it was unreleased. I can perform a release.

@samrocketman Awesome, thanks!