Skip to content

General cleanup to improve code quality#61

Merged
samrocketman merged 1 commit into
jenkinsci:masterfrom
i386:cleanup
Oct 21, 2016
Merged

General cleanup to improve code quality#61
samrocketman merged 1 commit into
jenkinsci:masterfrom
i386:cleanup

Conversation

@i386
Copy link
Copy Markdown
Contributor

@i386 i386 commented Oct 21, 2016

  • Bump parent pom for findbugs rules
  • Fix findbugs violations
  • Pruned unneeded dependencies
  • Upgraded dependencies
  • Fixed serialization of authentication token + added test

PTAL @samrocketman

 * Bump parent pom for findbugs rules
 * Fix findbugs violations
 * Pruned unneeded dependencies
 * Upgraded dependencies
 * Fixed serialization of authentication token + added test
@i386 i386 mentioned this pull request Oct 21, 2016
@samrocketman
Copy link
Copy Markdown
Member

Thanks for the contribution. I'll look it over.

@samrocketman samrocketman merged commit 73b336e into jenkinsci:master Oct 21, 2016
thatsmydoing added a commit to thatsmydoing/github-oauth-plugin that referenced this pull request Jun 25, 2025
In jenkinsci#61, use of `gh`
was replaced by `getGitHub()` but the check for `gh != null` was not
removed. `getGitHub()` already ensures that it's present so there's no
need to do the check.

The check was causing calls to `loadUser`, etc to return `null` if a
token was restored and nothing that transitively calls `getGitHub()` has
been called yet.

In particular, when a user logs in, their authorities will be empty
because GithubSecurityRealm tries to `loadUser` in `loadUserByUsername2`
and sets authorities to empty if the user is `null`. Only later, when
something transitively calls `getGitHub()`, will the user have the
authorities set.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants