From d75ad491c801a357fbbd9bb2e233d0c970e2f88f Mon Sep 17 00:00:00 2001 From: Ivan Fernandez Calvo Date: Mon, 29 Aug 2022 11:57:38 +0200 Subject: [PATCH 1/3] feat: update the plugin to use Spring framework classes --- .../plugins/saml/SamlAuthenticationToken.java | 5 +- .../plugins/saml/SamlGroupAuthority.java | 43 -------- .../plugins/saml/SamlGroupDetails.java | 16 +-- .../plugins/saml/SamlSecurityRealm.java | 97 +++++++++++-------- .../plugins/saml/SamlUserDetails.java | 17 ++-- .../plugins/saml/SamlUserDetailsService.java | 30 +++--- .../saml/user/LoginDetailsProperty.java | 17 +++- .../plugins/saml/SamlSecurityRealmTest.java | 3 +- 8 files changed, 102 insertions(+), 126 deletions(-) delete mode 100644 src/main/java/org/jenkinsci/plugins/saml/SamlGroupAuthority.java diff --git a/src/main/java/org/jenkinsci/plugins/saml/SamlAuthenticationToken.java b/src/main/java/org/jenkinsci/plugins/saml/SamlAuthenticationToken.java index e0275d48..ec336136 100644 --- a/src/main/java/org/jenkinsci/plugins/saml/SamlAuthenticationToken.java +++ b/src/main/java/org/jenkinsci/plugins/saml/SamlAuthenticationToken.java @@ -17,12 +17,11 @@ package org.jenkinsci.plugins.saml; -import org.acegisecurity.providers.AbstractAuthenticationToken; - import javax.annotation.Nonnull; +import org.springframework.security.authentication.AbstractAuthenticationToken; /** - * @see org.acegisecurity.Authentication + * @see AbstractAuthenticationToken */ public final class SamlAuthenticationToken extends AbstractAuthenticationToken { diff --git a/src/main/java/org/jenkinsci/plugins/saml/SamlGroupAuthority.java b/src/main/java/org/jenkinsci/plugins/saml/SamlGroupAuthority.java deleted file mode 100644 index b5e6d287..00000000 --- a/src/main/java/org/jenkinsci/plugins/saml/SamlGroupAuthority.java +++ /dev/null @@ -1,43 +0,0 @@ -/* Licensed to Jenkins CI under one or more contributor license -agreements. See the NOTICE file distributed with this work -for additional information regarding copyright ownership. -Jenkins CI licenses this file to you under the Apache License, -Version 2.0 (the "License"); you may not use this file except -in compliance with the License. You may obtain a copy of the -License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, -software distributed under the License is distributed on an -"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -KIND, either express or implied. See the License for the -specific language governing permissions and limitations -under the License. */ - -package org.jenkinsci.plugins.saml; - -import org.acegisecurity.GrantedAuthority; - -/** - * Authority class, represents a group receved in SAML response - * - * @see org.acegisecurity.GrantedAuthority - */ -public class SamlGroupAuthority implements GrantedAuthority { - - private final String group; - - public SamlGroupAuthority(String group) { - this.group = group; - } - - public String getAuthority() { - return this.group; - } - - @Override - public String toString() { - return this.getAuthority(); - } -} diff --git a/src/main/java/org/jenkinsci/plugins/saml/SamlGroupDetails.java b/src/main/java/org/jenkinsci/plugins/saml/SamlGroupDetails.java index 49842e0a..2ae50a24 100644 --- a/src/main/java/org/jenkinsci/plugins/saml/SamlGroupDetails.java +++ b/src/main/java/org/jenkinsci/plugins/saml/SamlGroupDetails.java @@ -16,13 +16,11 @@ under the License. */ package org.jenkinsci.plugins.saml; +import java.util.HashSet; +import java.util.Set; import hudson.model.User; import hudson.security.GroupDetails; import jenkins.security.LastGrantedAuthoritiesProperty; -import org.acegisecurity.GrantedAuthority; - -import java.util.HashSet; -import java.util.Set; /** @@ -52,23 +50,19 @@ public String getDisplayName() { @Override public Set getMembers() { if (members.isEmpty()) { - for (User u : User.getAll()) { + User.getAll().forEach(u -> { LastGrantedAuthoritiesProperty prop = u.getProperty(LastGrantedAuthoritiesProperty.class); if (hasGroupOnAuthorities(prop)) { members.add(u.getId()); } - } + }); } return members; } private boolean hasGroupOnAuthorities(LastGrantedAuthoritiesProperty prop) { if (prop != null) { - for (GrantedAuthority a : prop.getAuthorities()) { - if (name.equals(a.getAuthority())) { - return true; - } - } + return prop.getAuthorities2().stream().anyMatch(a -> name.equals(a.getAuthority())); } return false; } diff --git a/src/main/java/org/jenkinsci/plugins/saml/SamlSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/saml/SamlSecurityRealm.java index 655c66b4..eb407842 100644 --- a/src/main/java/org/jenkinsci/plugins/saml/SamlSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/saml/SamlSecurityRealm.java @@ -17,20 +17,18 @@ package org.jenkinsci.plugins.saml; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nonnull; +import javax.servlet.http.HttpSession; import edu.umd.cs.findbugs.annotations.NonNull; -import hudson.Extension; -import hudson.Util; -import hudson.security.GroupDetails; -import hudson.security.UserMayOrMayNotExistException; -import hudson.util.FormValidation; -import hudson.model.Descriptor; -import hudson.model.User; -import hudson.security.SecurityRealm; -import hudson.tasks.Mailer.UserProperty; -import jenkins.model.Jenkins; -import jenkins.security.SecurityListener; -import org.acegisecurity.*; -import org.acegisecurity.context.SecurityContextHolder; +import org.acegisecurity.Authentication; import org.acegisecurity.userdetails.UsernameNotFoundException; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; @@ -39,23 +37,35 @@ import org.jenkinsci.plugins.saml.user.SamlCustomProperty; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; -import org.kohsuke.stapler.*; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.Header; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpResponses; +import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.interceptor.RequirePOST; import org.pac4j.core.redirect.RedirectAction; import org.pac4j.core.redirect.RedirectAction.RedirectType; -import org.springframework.dao.DataAccessException; import org.pac4j.saml.profile.SAML2Profile; - -import javax.annotation.Nonnull; -import javax.servlet.http.HttpSession; -import java.io.*; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.logging.Level; -import java.util.logging.Logger; - -import static org.apache.commons.codec.binary.Base64.*; +import org.springframework.dao.DataAccessException; +import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import hudson.Extension; +import hudson.Util; +import hudson.model.Descriptor; +import hudson.model.User; +import hudson.security.ACL; +import hudson.security.GroupDetails; +import hudson.security.SecurityRealm; +import hudson.security.UserMayOrMayNotExistException2; +import hudson.tasks.Mailer.UserProperty; +import hudson.util.FormValidation; +import jenkins.model.Jenkins; +import jenkins.security.SecurityListener; +import static org.apache.commons.codec.binary.Base64.decodeBase64; +import static org.apache.commons.codec.binary.Base64.isBase64; import static org.opensaml.saml.common.xml.SAMLConstants.SAML2_REDIRECT_BINDING_URI; /** @@ -215,15 +225,11 @@ public boolean allowsSignup() { @Override public SecurityComponents createSecurityComponents() { LOG.finer("createSecurityComponents"); - return new SecurityComponents(new AuthenticationManager() { - - public Authentication authenticate(Authentication authentication) throws AuthenticationException { - if (authentication instanceof SamlAuthenticationToken) { - return authentication; - } - throw new BadCredentialsException("Unexpected authentication type: " + authentication); + return new SecurityComponents(authentication -> { + if (authentication instanceof SamlAuthenticationToken) { + return authentication; } - + throw new BadCredentialsException("Unexpected authentication type: " + authentication); }, new SamlUserDetailsService()); } @@ -319,13 +325,12 @@ public HttpResponse doFinishLogin(final StaplerRequest request, final StaplerRes List authorities = loadGrantedAuthorities(saml2Profile); // create user data - SamlUserDetails userDetails = new SamlUserDetails(username, authorities.toArray(new GrantedAuthority[authorities.size()])); + SamlUserDetails userDetails = new SamlUserDetails(username, authorities); SamlAuthenticationToken samlAuthToken = new SamlAuthenticationToken(userDetails); - // initialize security context - SecurityContextHolder.getContext().setAuthentication(samlAuthToken); - SecurityListener.fireAuthenticated(userDetails); + ACL.as2(samlAuthToken); + SecurityListener.fireAuthenticated2(userDetails); User user = User.current(); saveUser |= modifyUserFullName(user, saml2Profile); @@ -491,11 +496,11 @@ List loadGrantedAuthorities(SAML2Profile saml2Profile) { // build list of authorities List authorities = new ArrayList<>(); - authorities.add(AUTHENTICATED_AUTHORITY); + authorities.add(AUTHENTICATED_AUTHORITY2); int countEmptyGroups = 0; for (String group : groups) { if (StringUtils.isNotBlank(group)) { - authorities.add(new SamlGroupAuthority(group)); + authorities.add(new SimpleGrantedAuthority(group)); } else { countEmptyGroups++; } @@ -614,16 +619,26 @@ public void doLogout(StaplerRequest req, StaplerResponse rsp) throws IOException LOG.log(Level.FINEST, "Here we could do the SAML Single Logout"); } + /** + * This method is overwritten due to SAML has no way to retrieve the members of a Group and this cause issues on + * some Authorization plugins. Because of that we have to implement SamlGroupDetails + */ + @SuppressWarnings("deprecation") @Override public GroupDetails loadGroupByGroupname(String groupname) throws UsernameNotFoundException, DataAccessException { GroupDetails dg = new SamlGroupDetails(groupname); if (dg.getMembers().isEmpty()) { - throw new UserMayOrMayNotExistException(groupname); + throw new UserMayOrMayNotExistException2(groupname); } return dg; } + /** + * This method is overwritten due to SAML has no way to retrieve the members of a Group and this cause issues on + * some Authorization plugins. Because of that we have to implement SamlGroupDetails + */ + @SuppressWarnings("deprecation") @Override public GroupDetails loadGroupByGroupname(String groupname, boolean fetchMembers) throws UsernameNotFoundException, DataAccessException { diff --git a/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java b/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java index 3480111e..a942938b 100644 --- a/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java +++ b/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java @@ -17,12 +17,13 @@ package org.jenkinsci.plugins.saml; -import org.acegisecurity.GrantedAuthority; -import org.acegisecurity.userdetails.UserDetails; - +import java.util.Collection; +import java.util.Collections; import javax.annotation.Nonnull; import java.util.Arrays; import java.util.Collections; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.userdetails.UserDetails; /** * @see UserDetails @@ -32,15 +33,15 @@ public class SamlUserDetails implements UserDetails { private static final long serialVersionUID = 2L; private final String username; - private final GrantedAuthority[] authorities; + private final Collection authorities; - public SamlUserDetails(@Nonnull String username, GrantedAuthority[] authorities) { + public SamlUserDetails(@Nonnull String username, Collection authorities) { this.username = username; - this.authorities = Arrays.copyOf(authorities, authorities.length); + this.authorities = Collections.unmodifiableCollection(authorities); } - public GrantedAuthority[] getAuthorities() { - return Arrays.copyOf(authorities, authorities.length); + public Collection getAuthorities() { + return authorities; } public String getPassword() { diff --git a/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetailsService.java b/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetailsService.java index 5e759916..ee871a0c 100644 --- a/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetailsService.java +++ b/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetailsService.java @@ -17,19 +17,19 @@ package org.jenkinsci.plugins.saml; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import javax.annotation.Nonnull; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.userdetails.UserDetailsService; import hudson.model.User; import hudson.security.SecurityRealm; import hudson.security.UserMayOrMayNotExistException2; import jenkins.model.Jenkins; import jenkins.security.LastGrantedAuthoritiesProperty; -import org.acegisecurity.Authentication; -import org.acegisecurity.GrantedAuthority; -import org.acegisecurity.userdetails.UserDetailsService; - -import javax.annotation.Nonnull; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; /** * This service is responsible for restoring UserDetails object by userId @@ -41,7 +41,7 @@ public class SamlUserDetailsService implements UserDetailsService { public SamlUserDetails loadUserByUsername(@Nonnull String username) { // try to obtain user details from current authentication details - Authentication auth = Jenkins.getAuthentication(); + Authentication auth = Jenkins.getAuthentication2(); if (username.compareTo(auth.getName()) == 0 && auth instanceof SamlAuthenticationToken) { return (SamlUserDetails) auth.getDetails(); } @@ -53,20 +53,20 @@ public SamlUserDetails loadUserByUsername(@Nonnull String username) { throw new UserMayOrMayNotExistException2(username); } - List authorities = new ArrayList<>(); - authorities.add(SecurityRealm.AUTHENTICATED_AUTHORITY); + Listauthorities = new ArrayList<>(); + authorities.add(SecurityRealm.AUTHENTICATED_AUTHORITY2); if (username.compareTo(user.getId()) == 0) { LastGrantedAuthoritiesProperty lastGranted = user.getProperty(LastGrantedAuthoritiesProperty.class); if (lastGranted != null) { - for (GrantedAuthority a : lastGranted.getAuthorities()) { - if (a != SecurityRealm.AUTHENTICATED_AUTHORITY) { - SamlGroupAuthority ga = new SamlGroupAuthority(a.getAuthority()); + for (GrantedAuthority a : lastGranted.getAuthorities2()) { + if (a != SecurityRealm.AUTHENTICATED_AUTHORITY2) { + SimpleGrantedAuthority ga = new SimpleGrantedAuthority(a.getAuthority()); authorities.add(ga); } } } } - return new SamlUserDetails(user.getId(), authorities.toArray(new GrantedAuthority[0])); + return new SamlUserDetails(user.getId(), authorities); } } diff --git a/src/main/java/org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java b/src/main/java/org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java index f150a82b..427b73a0 100644 --- a/src/main/java/org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java +++ b/src/main/java/org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java @@ -23,18 +23,29 @@ import hudson.model.UserPropertyDescriptor; import edu.umd.cs.findbugs.annotations.NonNull; import net.sf.json.JSONObject; -import org.acegisecurity.GrantedAuthority; import org.apache.commons.lang.time.FastDateFormat; import hudson.security.SecurityRealm; import jenkins.model.Jenkins; import org.jenkinsci.plugins.saml.SamlSecurityRealm; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.StaplerRequest; -import org.acegisecurity.Authentication; import java.util.Date; import java.util.logging.Level; import java.util.logging.Logger; +import edu.umd.cs.findbugs.annotations.NonNull; +import net.sf.json.JSONObject; +import org.apache.commons.lang.time.FastDateFormat; +import org.jenkinsci.plugins.saml.SamlSecurityRealm; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.StaplerRequest; +import org.springframework.security.core.Authentication; +import hudson.Extension; +import hudson.model.User; +import hudson.model.UserProperty; +import hudson.model.UserPropertyDescriptor; +import hudson.security.SecurityRealm; +import jenkins.model.Jenkins; /** * Store details about create and login processes @@ -153,7 +164,7 @@ protected void loggedIn(@javax.annotation.Nonnull String username) { o = new LoginDetailsProperty(); } u.addProperty(o); - Authentication a = Jenkins.getAuthentication(); + Authentication a = Jenkins.getAuthentication2(); if (a.getName().equals(username)) { o.update(); // just for defensive sanity checking } diff --git a/src/test/java/org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java index 4947f8e0..f2add0a0 100644 --- a/src/test/java/org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java +++ b/src/test/java/org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java @@ -27,7 +27,6 @@ import java.util.Collections; import java.util.List; import java.util.logging.LogRecord; -import org.acegisecurity.GrantedAuthority; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; @@ -47,7 +46,6 @@ import java.util.logging.Logger; import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; @@ -58,6 +56,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import org.jvnet.hudson.test.Issue; import org.pac4j.saml.profile.SAML2Profile; +import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import static org.junit.Assert.assertFalse; From 43d353c1b19e06f267489c85160d4dfb2e67e3fd Mon Sep 17 00:00:00 2001 From: Ivan Fernandez Calvo Date: Mon, 29 Aug 2022 12:08:59 +0200 Subject: [PATCH 2/3] fix: add missing change --- .../org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java index f2add0a0..5a9c92b6 100644 --- a/src/test/java/org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java +++ b/src/test/java/org/jenkinsci/plugins/saml/SamlSecurityRealmTest.java @@ -218,11 +218,11 @@ public void testGetters() throws IOException { new SamlAdvancedConfiguration(true, null, null, null).toString().contains("SamlAdvancedConfiguration")); assertTrue(new SamlAdvancedConfiguration(true, "", "", "").toString().contains("SamlAdvancedConfiguration")); - SamlGroupAuthority authority = new SamlGroupAuthority("role001"); - assertEquals(authority.toString().equals("role001"),true); + SimpleGrantedAuthority authority = new SimpleGrantedAuthority("role001"); + assertEquals("role001", authority.toString()); - SamlUserDetails userDetails = new SamlUserDetails("tesla",new GrantedAuthority[]{authority}); - assertTrue(userDetails.toString().contains("tesla") && userDetails.toString().contains("role001")); + SamlUserDetails userDetails = new SamlUserDetails("tesla", Collections.singletonList(authority)); + assertEquals(userDetails.toString().contains("tesla") && userDetails.toString().contains("role001"), true); assertThat(new SamlEncryptionData(null,null,null, null, false, false).toString(), containsString( "SamlEncryptionData")); From ccba4749566b86db7df83af6e47bb785a7a01219 Mon Sep 17 00:00:00 2001 From: Ivan Fernandez Calvo Date: Mon, 29 Aug 2022 12:21:36 +0200 Subject: [PATCH 3/3] fix: missing imports --- src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java | 1 + .../org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java b/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java index a942938b..d8fb2671 100644 --- a/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java +++ b/src/main/java/org/jenkinsci/plugins/saml/SamlUserDetails.java @@ -17,6 +17,7 @@ package org.jenkinsci.plugins.saml; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import javax.annotation.Nonnull; diff --git a/src/main/java/org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java b/src/main/java/org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java index 427b73a0..eb15af19 100644 --- a/src/main/java/org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java +++ b/src/main/java/org/jenkinsci/plugins/saml/user/LoginDetailsProperty.java @@ -33,7 +33,6 @@ import java.util.Date; import java.util.logging.Level; import java.util.logging.Logger; -import edu.umd.cs.findbugs.annotations.NonNull; import net.sf.json.JSONObject; import org.apache.commons.lang.time.FastDateFormat; import org.jenkinsci.plugins.saml.SamlSecurityRealm;