diff --git a/README.md b/README.md index 9fe1c548..9a4b6467 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,16 @@ jenkins.setAuthorizationStrategy(rbas) jenkins.save() ``` + +### Case sensitive mode +In previous versions of this plugin, role assignments where always matched case-sensitive, even when the security realm +works case-insensitive (as do most of them). As of version 685 the plugin will use the strategy given by the security realm +to match assigned roles. If for some reason you need the old behaviour, set the property `com.michelin.cio.hudson.plugins.rolestrategy.RoleMap.FORCE_CASE_SENSITIVE` +via command line `jenkins -Dcom.michelin.cio.hudson.plugins.rolestrategy.RoleMap.FORCE_CASE_SENSITIVE=true -war jenkins.war`, set it via the script console or via +an init hook script. + + + ## License [MIT License](./LICENSE.md) diff --git a/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java b/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java index ee41c965..9cf3db9a 100644 --- a/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java +++ b/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java @@ -41,6 +41,7 @@ import hudson.security.AccessControlled; import hudson.security.AuthorizationStrategy; import hudson.security.Permission; +import hudson.security.SecurityRealm; import hudson.security.SidACL; import java.util.ArrayList; import java.util.Collections; @@ -60,6 +61,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import jenkins.model.IdStrategy; import jenkins.model.Jenkins; import jenkins.model.ProjectNamingStrategy; import org.acegisecurity.acls.sid.PrincipalSid; @@ -83,6 +85,10 @@ */ public class RoleMap { + @SuppressFBWarnings("MS_SHOULD_BE_FINAL") // Used to allow old behaviour + @Restricted(NoExternalUse.class) + public static boolean FORCE_CASE_SENSITIVE = Boolean.getBoolean(RoleMap.class.getName() + ".FORCE_CASE_SENSITIVE"); + /** * Map associating each {@link Role} with the concerned {@link User}s/groups. */ @@ -133,6 +139,9 @@ public RoleMap(@NonNull SortedMap> grantedRoles) { public boolean hasPermission(PermissionEntry sid, Permission permission, RoleType roleType, AccessControlled controlledItem) { final Set permissions = getImplyingPermissions(permission); final boolean[] hasPermission = { false }; + final SecurityRealm securityRealm = Jenkins.get().getSecurityRealm(); + final boolean principal = sid.getType() == AuthorizationType.USER; + final IdStrategy strategy = principal ? securityRealm.getUserIdStrategy() : securityRealm.getGroupIdStrategy(); // Walk through the roles, and only add the roles having the given permission, // or a permission implying the given permission @@ -149,12 +158,20 @@ public boolean hasPermission(PermissionEntry sid, Permission permission, RoleTyp */ @CheckForNull private PermissionEntry hasPermission(Role current, PermissionEntry entry) { - if (grantedRoles.get(current).contains(entry)) { + Set entries = grantedRoles.get(current); + if (entries.contains(entry)) { return entry; } - entry = new PermissionEntry(AuthorizationType.EITHER, entry.getSid()); - if (grantedRoles.get(current).contains(entry)) { - return entry; + PermissionEntry eitherEntry = new PermissionEntry(AuthorizationType.EITHER, entry.getSid()); + if (entries.contains(eitherEntry)) { + return eitherEntry; + } + if (!FORCE_CASE_SENSITIVE) { + for (PermissionEntry pe : entries) { + if (pe.isApplicable(principal) && strategy.equals(pe.getSid(), entry.getSid())) { + return pe; + } + } } return null; } diff --git a/src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java b/src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java index 196b4617..7255ac2f 100644 --- a/src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java +++ b/src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java @@ -2,9 +2,11 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import com.michelin.cio.hudson.plugins.rolestrategy.RoleBasedAuthorizationStrategy; +import com.michelin.cio.hudson.plugins.rolestrategy.RoleMap; import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType; import hudson.PluginManager; import hudson.model.User; @@ -31,11 +33,39 @@ public void initSecurityRealm() { @LocalData @Test public void testRoleAssignment() { + RoleMap.FORCE_CASE_SENSITIVE = false; try (ACLContext c = ACL.as(User.getById("alice", true))) { assertTrue(jenkinsRule.jenkins.hasPermission(Permission.READ)); } } + @LocalData + @Test + public void testRoleAssignmentCaseInsensitiveNoMatchSucceeds() { + RoleMap.FORCE_CASE_SENSITIVE = false; + try (ACLContext c = ACL.as(User.getById("Alice", true))) { + assertTrue(jenkinsRule.jenkins.hasPermission(Permission.READ)); + } + } + + @LocalData + @Test + public void testRoleAssignmentCaseSensitiveMatch() { + RoleMap.FORCE_CASE_SENSITIVE = true; + try (ACLContext c = ACL.as(User.getById("alice", true))) { + assertTrue(jenkinsRule.jenkins.hasPermission(Permission.READ)); + } + } + + @LocalData + @Test + public void testRoleAssignmentCaseSensitiveNoMatchFails() { + RoleMap.FORCE_CASE_SENSITIVE = true; + try (ACLContext c = ACL.as(User.getById("Alice", true))) { + assertFalse(jenkinsRule.jenkins.hasPermission(Permission.READ)); + } + } + @LocalData @Test public void dangerousPermissionsAreIgnored() { diff --git a/src/test/resources/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest/testRoleAssignment/config.xml b/src/test/resources/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest/config.xml similarity index 100% rename from src/test/resources/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest/testRoleAssignment/config.xml rename to src/test/resources/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest/config.xml