Skip to content

Commit 44f5236

Browse files
basilbasil
authored
Migrate from Acegi compatibility layer to Spring Security 6.x (#285)
1 parent a377277 commit 44f5236

7 files changed

+171
-197
lines changed

‎src/main/java/org/jenkinsci/plugins/GithubAuthenticationToken.java

+11-11
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ of this software and associated documentation files (the "Software"), to deal
5050
import java.util.logging.Logger;
5151
import jenkins.model.Jenkins;
5252
import okhttp3.OkHttpClient;
53-
import org.acegisecurity.GrantedAuthority;
54-
import org.acegisecurity.GrantedAuthorityImpl;
55-
import org.acegisecurity.providers.AbstractAuthenticationToken;
56-
import org.acegisecurity.userdetails.UsernameNotFoundException;
5753
import org.kohsuke.github.GHMyself;
5854
import org.kohsuke.github.GHOrganization;
5955
import org.kohsuke.github.GHRepository;
@@ -63,6 +59,10 @@ of this software and associated documentation files (the "Software"), to deal
6359
import org.kohsuke.github.GitHubBuilder;
6460
import org.kohsuke.github.RateLimitHandler;
6561
import org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector;
62+
import org.springframework.security.authentication.AbstractAuthenticationToken;
63+
import org.springframework.security.core.GrantedAuthority;
64+
import org.springframework.security.core.authority.SimpleGrantedAuthority;
65+
import org.springframework.security.core.userdetails.UsernameNotFoundException;
6666

6767
/**
6868
* @author mocleiri
@@ -190,7 +190,7 @@ public GithubAuthenticationToken(final String accessToken, final String githubSe
190190

191191
@SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE")
192192
public GithubAuthenticationToken(final String accessToken, final String githubServer, final boolean clearUserCache) throws IOException {
193-
super(new GrantedAuthority[] {});
193+
super(List.of());
194194

195195
this.accessToken = accessToken;
196196
this.githubServer = githubServer;
@@ -209,7 +209,7 @@ public GithubAuthenticationToken(final String accessToken, final String githubSe
209209
// may have changed due to SSO [JENKINS-60200].
210210
clearCacheForUser(this.userName);
211211
}
212-
authorities.add(SecurityRealm.AUTHENTICATED_AUTHORITY);
212+
authorities.add(SecurityRealm.AUTHENTICATED_AUTHORITY2);
213213

214214
// This stuff only really seems useful if *not* using GithubAuthorizationStrategy
215215
// but instead using matrix so org/team can be granted rights
@@ -242,11 +242,11 @@ public GithubAuthenticationToken(final String accessToken, final String githubSe
242242
for (Map.Entry<String, Set<GHTeam>> teamEntry : myTeams.entrySet()) {
243243
String orgLogin = teamEntry.getKey();
244244
LOGGER.log(Level.FINE, "Fetch teams for user " + userName + " in organization " + orgLogin);
245-
authorities.add(new GrantedAuthorityImpl(orgLogin));
245+
authorities.add(new SimpleGrantedAuthority(orgLogin));
246246
for (GHTeam team : teamEntry.getValue()) {
247247
String teamIdentifier = team.getSlug() == null ? team.getName() : team.getSlug();
248248

249-
authorities.add(new GrantedAuthorityImpl(orgLogin + GithubOAuthGroupDetails.ORG_TEAM_SEPARATOR
249+
authorities.add(new SimpleGrantedAuthority(orgLogin + GithubOAuthGroupDetails.ORG_TEAM_SEPARATOR
250250
+ teamIdentifier));
251251
}
252252
}
@@ -340,8 +340,8 @@ private static Proxy getProxy(@NonNull String host) {
340340
}
341341

342342
@Override
343-
public GrantedAuthority[] getAuthorities() {
344-
return authorities.toArray(new GrantedAuthority[0]);
343+
public Collection<GrantedAuthority> getAuthorities() {
344+
return authorities;
345345
}
346346

347347
@Override
@@ -572,7 +572,7 @@ GHTeam loadTeam(@NonNull String organization, @NonNull String team) {
572572
GithubOAuthUserDetails getUserDetails(@NonNull String username) throws IOException {
573573
GHUser user = loadUser(username);
574574
if (user != null) {
575-
return new GithubOAuthUserDetails(user.getLogin(), this);
575+
return new GithubOAuthUserDetails(user.getLogin(), getAuthorities());
576576
}
577577
return null;
578578
}

‎src/main/java/org/jenkinsci/plugins/GithubOAuthUserDetails.java

+5-32
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@
55

66
import edu.umd.cs.findbugs.annotations.NonNull;
77
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
8-
import java.io.IOException;
9-
import org.acegisecurity.GrantedAuthority;
10-
import org.acegisecurity.userdetails.User;
11-
import org.acegisecurity.userdetails.UserDetails;
12-
import org.kohsuke.github.GHUser;
8+
import java.util.Collection;
9+
import org.springframework.security.core.GrantedAuthority;
10+
import org.springframework.security.core.userdetails.User;
11+
import org.springframework.security.core.userdetails.UserDetails;
1312

1413
/**
1514
* @author Mike
@@ -20,34 +19,8 @@ public class GithubOAuthUserDetails extends User implements UserDetails {
2019

2120
private static final long serialVersionUID = 1L;
2221

23-
private boolean hasGrantedAuthorities;
24-
25-
private final GithubAuthenticationToken authenticationToken;
26-
27-
public GithubOAuthUserDetails(@NonNull String login, @NonNull GrantedAuthority[] authorities) {
22+
public GithubOAuthUserDetails(@NonNull String login, @NonNull Collection<? extends GrantedAuthority> authorities) {
2823
super(login, "", true, true, true, true, authorities);
29-
this.authenticationToken = null;
30-
this.hasGrantedAuthorities = true;
3124
}
3225

33-
public GithubOAuthUserDetails(@NonNull String login, @NonNull GithubAuthenticationToken authenticationToken) {
34-
super(login, "", true, true, true, true, new GrantedAuthority[0]);
35-
this.authenticationToken = authenticationToken;
36-
this.hasGrantedAuthorities = false;
37-
}
38-
39-
@Override
40-
public GrantedAuthority[] getAuthorities() {
41-
if (!hasGrantedAuthorities) {
42-
try {
43-
GHUser user = authenticationToken.loadUser(getUsername());
44-
if(user != null) {
45-
setAuthorities(authenticationToken.getAuthorities());
46-
}
47-
} catch (IOException e) {
48-
throw new RuntimeException(e);
49-
}
50-
}
51-
return super.getAuthorities();
52-
}
5326
}

‎src/main/java/org/jenkinsci/plugins/GithubRequireOrganizationMembershipACL.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ of this software and associated documentation files (the "Software"), to deal
4545
import jenkins.branch.MultiBranchProject;
4646
import jenkins.model.Jenkins;
4747
import jenkins.scm.api.SCMSource;
48-
import org.acegisecurity.Authentication;
4948
import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource;
5049
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
5150
import org.jenkinsci.plugins.workflow.multibranch.BranchJobProperty;
5251
import org.kohsuke.stapler.Stapler;
53-
import org.kohsuke.stapler.StaplerRequest;
52+
import org.kohsuke.stapler.StaplerRequest2;
53+
import org.springframework.security.core.Authentication;
5454

5555
/**
5656
* @author Mike
@@ -76,11 +76,11 @@ public class GithubRequireOrganizationMembershipACL extends ACL {
7676
/*
7777
* (non-Javadoc)
7878
*
79-
* @see hudson.security.ACL#hasPermission(org.acegisecurity.Authentication,
79+
* @see hudson.security.ACL#hasPermission(org.springframework.security.core.Authentication,
8080
* hudson.security.Permission)
8181
*/
8282
@Override
83-
public boolean hasPermission(@NonNull Authentication a, @NonNull Permission permission) {
83+
public boolean hasPermission2(@NonNull Authentication a, @NonNull Permission permission) {
8484
if (a instanceof GithubAuthenticationToken) {
8585
if (!a.isAuthenticated())
8686
return false;
@@ -153,7 +153,7 @@ else if (testBuildPermission(permission) && isInWhitelistedOrgs(authenticationTo
153153
throw new IllegalArgumentException("Authentication must have a valid name");
154154
}
155155

156-
if (authenticatedUserName.equals(SYSTEM.getPrincipal())) {
156+
if (authenticatedUserName.equals(SYSTEM2.getPrincipal())) {
157157
// give system user full access
158158
log.finest("Granting Full rights to SYSTEM user.");
159159
return true;
@@ -233,7 +233,7 @@ private boolean currentUriPathEndsWithSegment( String segment ) {
233233

234234
@Nullable
235235
private String requestURI() {
236-
StaplerRequest currentRequest = Stapler.getCurrentRequest();
236+
StaplerRequest2 currentRequest = Stapler.getCurrentRequest2();
237237
return (currentRequest == null) ? null : currentRequest.getOriginalRequestURI();
238238
}
239239

‎src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java

+48-33
Original file line numberDiff line numberDiff line change
@@ -42,30 +42,24 @@ of this software and associated documentation files (the "Software"), to deal
4242
import hudson.security.AbstractPasswordBasedSecurityRealm;
4343
import hudson.security.GroupDetails;
4444
import hudson.security.SecurityRealm;
45-
import hudson.security.UserMayOrMayNotExistException;
45+
import hudson.security.UserMayOrMayNotExistException2;
4646
import hudson.tasks.Mailer;
4747
import hudson.util.Secret;
48+
import jakarta.servlet.http.HttpSession;
4849
import java.io.IOException;
50+
import java.io.UncheckedIOException;
4951
import java.net.InetSocketAddress;
5052
import java.net.Proxy;
5153
import java.security.SecureRandom;
5254
import java.util.Arrays;
55+
import java.util.Collection;
5356
import java.util.HashSet;
57+
import java.util.List;
5458
import java.util.Set;
5559
import java.util.logging.Level;
5660
import java.util.logging.Logger;
57-
import javax.servlet.http.HttpSession;
5861
import jenkins.model.Jenkins;
5962
import jenkins.security.SecurityListener;
60-
import org.acegisecurity.Authentication;
61-
import org.acegisecurity.AuthenticationException;
62-
import org.acegisecurity.AuthenticationManager;
63-
import org.acegisecurity.BadCredentialsException;
64-
import org.acegisecurity.context.SecurityContextHolder;
65-
import org.acegisecurity.providers.UsernamePasswordAuthenticationToken;
66-
import org.acegisecurity.userdetails.UserDetails;
67-
import org.acegisecurity.userdetails.UserDetailsService;
68-
import org.acegisecurity.userdetails.UsernameNotFoundException;
6963
import org.apache.commons.lang.builder.HashCodeBuilder;
7064
import org.apache.http.HttpEntity;
7165
import org.apache.http.HttpHost;
@@ -83,15 +77,24 @@ of this software and associated documentation files (the "Software"), to deal
8377
import org.kohsuke.github.GHMyself;
8478
import org.kohsuke.github.GHOrganization;
8579
import org.kohsuke.github.GHTeam;
80+
import org.kohsuke.github.GHUser;
8681
import org.kohsuke.stapler.DataBoundConstructor;
8782
import org.kohsuke.stapler.Header;
8883
import org.kohsuke.stapler.HttpRedirect;
8984
import org.kohsuke.stapler.HttpResponse;
9085
import org.kohsuke.stapler.HttpResponses;
9186
import org.kohsuke.stapler.QueryParameter;
92-
import org.kohsuke.stapler.StaplerRequest;
93-
import org.springframework.dao.DataAccessException;
94-
import org.springframework.dao.DataRetrievalFailureException;
87+
import org.kohsuke.stapler.StaplerRequest2;
88+
import org.springframework.security.authentication.AuthenticationManager;
89+
import org.springframework.security.authentication.AuthenticationServiceException;
90+
import org.springframework.security.authentication.BadCredentialsException;
91+
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
92+
import org.springframework.security.core.Authentication;
93+
import org.springframework.security.core.AuthenticationException;
94+
import org.springframework.security.core.GrantedAuthority;
95+
import org.springframework.security.core.context.SecurityContextHolder;
96+
import org.springframework.security.core.userdetails.UserDetails;
97+
import org.springframework.security.core.userdetails.UsernameNotFoundException;
9598

9699
/**
97100
*
@@ -101,7 +104,7 @@ of this software and associated documentation files (the "Software"), to deal
101104
* This is based on the MySQLSecurityRealm from the mysql-auth-plugin written by
102105
* Alex Ackerman.
103106
*/
104-
public class GithubSecurityRealm extends AbstractPasswordBasedSecurityRealm implements UserDetailsService {
107+
public class GithubSecurityRealm extends AbstractPasswordBasedSecurityRealm {
105108
private static final String DEFAULT_WEB_URI = "https://github.com";
106109
private static final String DEFAULT_API_URI = "https://api.github.com";
107110
private static final String DEFAULT_ENTERPRISE_API_SUFFIX = "/api/v3";
@@ -332,7 +335,7 @@ public String getOauthScopes() {
332335
return oauthScopes;
333336
}
334337

335-
public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter String from, @Header("Referer") final String referer)
338+
public HttpResponse doCommenceLogin(StaplerRequest2 request, @QueryParameter String from, @Header("Referer") final String referer)
336339
throws IOException {
337340
// https://tools.ietf.org/html/rfc6749#section-10.10 dictates that probability that an attacker guesses the string
338341
// SHOULD be less than or equal to 2^(-160) and our Strings consist of 65 chars. (65^27 ~= 2^160)
@@ -355,7 +358,7 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter Stri
355358
}
356359
String suffix="";
357360
if (!scopes.isEmpty()) {
358-
suffix = "&scope="+Util.join(scopes,",")+"&state="+state;
361+
suffix = "&scope=" + String.join(",", scopes) + "&state=" + state;
359362
} else {
360363
// We need repo scope in order to access private repos
361364
// See https://developer.github.com/v3/oauth/#scopes
@@ -370,7 +373,7 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter Stri
370373
* This is where the user comes back to at the end of the OAuth redirect
371374
* ping-pong.
372375
*/
373-
public HttpResponse doFinishLogin(StaplerRequest request)
376+
public HttpResponse doFinishLogin(StaplerRequest2 request)
374377
throws IOException {
375378
String code = request.getParameter("code");
376379
String state = request.getParameter(STATE_ATTRIBUTE);
@@ -435,7 +438,7 @@ public HttpResponse doFinishLogin(StaplerRequest request)
435438
}
436439
}
437440

438-
SecurityListener.fireAuthenticated(new GithubOAuthUserDetails(self.getLogin(), auth.getAuthorities()));
441+
SecurityListener.fireAuthenticated2(new GithubOAuthUserDetails(self.getLogin(), auth.getAuthorities()));
439442

440443
// While LastGrantedAuthorities are triggered by that event, we cannot trigger it there
441444
// or modifications in organizations will be not reflected when using API Token, due to that caching
@@ -561,7 +564,7 @@ public Authentication authenticate(Authentication authentication)
561564
GithubSecretStorage.put(user, token.getCredentials().toString());
562565
}
563566

564-
SecurityListener.fireAuthenticated(new GithubOAuthUserDetails(token.getName(), github.getAuthorities()));
567+
SecurityListener.fireAuthenticated2(new GithubOAuthUserDetails(token.getName(), github.getAuthorities()));
565568

566569
return github;
567570
} catch (IOException e) {
@@ -570,11 +573,11 @@ public Authentication authenticate(Authentication authentication)
570573
throw new BadCredentialsException(
571574
"Unexpected authentication type: " + authentication);
572575
}
573-
}, GithubSecurityRealm.this::loadUserByUsername);
576+
}, GithubSecurityRealm.this::loadUserByUsername2);
574577
}
575578

576579
@Override
577-
protected GithubOAuthUserDetails authenticate(String username, String password) throws AuthenticationException {
580+
protected GithubOAuthUserDetails authenticate2(String username, String password) throws AuthenticationException {
578581
try {
579582
GithubAuthenticationToken github = new GithubAuthenticationToken(password, getGithubApiUri());
580583
if(username.equals(github.getPrincipal())) {
@@ -593,12 +596,12 @@ public String getLoginUrl() {
593596
}
594597

595598
@Override
596-
protected String getPostLogOutUrl(StaplerRequest req, Authentication auth) {
599+
protected String getPostLogOutUrl2(StaplerRequest2 req, Authentication auth) {
597600
// if we just redirect to the root and anonymous does not have Overall read then we will start a login all over again.
598601
// we are actually anonymous here as the security context has been cleared
599602
Jenkins j = Jenkins.get();
600603
if (j.hasPermission(Jenkins.READ)) {
601-
return super.getPostLogOutUrl(req, auth);
604+
return super.getPostLogOutUrl2(req, auth);
602605
}
603606
return req.getContextPath()+ "/" + GithubLogoutAction.POST_LOGOUT_URL;
604607
}
@@ -654,8 +657,8 @@ public DescriptorImpl getDescriptor() {
654657
* @return userDetails
655658
*/
656659
@Override
657-
public UserDetails loadUserByUsername(String username)
658-
throws UsernameNotFoundException, DataAccessException {
660+
public UserDetails loadUserByUsername2(String username)
661+
throws UsernameNotFoundException {
659662
//username is in org*team format
660663
if(username.contains(GithubOAuthGroupDetails.ORG_TEAM_SEPARATOR)) {
661664
throw new UsernameNotFoundException("Using org*team format instead of username: " + username);
@@ -672,7 +675,7 @@ public UserDetails loadUserByUsername(String username)
672675
}
673676
} catch(IOException | UsernameNotFoundException e) {
674677
if(e instanceof IOException) {
675-
throw new UserMayOrMayNotExistException("Could not connect to GitHub API server, target URL = " + getGithubApiUri(), e);
678+
throw new UserMayOrMayNotExistException2("Could not connect to GitHub API server, target URL = " + getGithubApiUri(), e);
676679
} else {
677680
// user not found so continuing normally re-using the current context holder
678681
LOGGER.log(Level.FINE, "Attempted to impersonate " + username + " but token in user property was invalid.");
@@ -684,15 +687,27 @@ public UserDetails loadUserByUsername(String username)
684687
if (token instanceof GithubAuthenticationToken) {
685688
authToken = (GithubAuthenticationToken) token;
686689
} else {
687-
throw new UserMayOrMayNotExistException("Unexpected authentication type: " + token);
690+
throw new UserMayOrMayNotExistException2("Unexpected authentication type: " + token);
688691
}
689692

690693
/**
691694
* Always lookup the local user first. If we can't resolve it then we can burn an API request to Github for this user
692695
* Taken from hudson.security.HudsonPrivateSecurityRealm#loadUserByUsername(java.lang.String)
693696
*/
694697
if (localUser != null) {
695-
return new GithubOAuthUserDetails(username, authToken);
698+
GHUser user;
699+
try {
700+
user = authToken.loadUser(username);
701+
} catch (IOException e) {
702+
throw new UncheckedIOException(e);
703+
}
704+
Collection<GrantedAuthority> authorities;
705+
if (user != null) {
706+
authorities = authToken.getAuthorities();
707+
} else {
708+
authorities = List.of();
709+
}
710+
return new GithubOAuthUserDetails(username, authorities);
696711
}
697712

698713
try {
@@ -709,7 +724,7 @@ public UserDetails loadUserByUsername(String username)
709724

710725
return userDetails;
711726
} catch (IOException | Error e) {
712-
throw new DataRetrievalFailureException("loadUserByUsername (username=" + username +")", e);
727+
throw new AuthenticationServiceException("loadUserByUsername (username=" + username +")", e);
713728
}
714729
}
715730

@@ -749,8 +764,8 @@ public int hashCode() {
749764
* @return groupDetails
750765
*/
751766
@Override
752-
public GroupDetails loadGroupByGroupname(String groupName)
753-
throws UsernameNotFoundException, DataAccessException {
767+
public GroupDetails loadGroupByGroupname2(String groupName, boolean fetchMembers)
768+
throws UsernameNotFoundException {
754769
GithubAuthenticationToken authToken = (GithubAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
755770

756771
if(authToken == null)
@@ -776,7 +791,7 @@ public GroupDetails loadGroupByGroupname(String groupName)
776791
return new GithubOAuthGroupDetails(ghOrg);
777792
}
778793
} catch (Error e) {
779-
throw new DataRetrievalFailureException("loadGroupByGroupname (groupname=" + groupName + ")", e);
794+
throw new AuthenticationServiceException("loadGroupByGroupname (groupname=" + groupName + ")", e);
780795
}
781796
}
782797

0 commit comments

Comments
 (0)