Skip to content

Commit

Permalink
[JENKINS-19409] use idstrategy to match role assignments (#332)
Browse files Browse the repository at this point in the history
The matching of users to roles was so far case sensitive. But as most
security realms work case insensitive this means that permissions are
not properly matched when someone logs in with capital letters but the
role is assigned to the user with small letters.
The plugin now falls back to the security realms idstrategy in case the
user doesn't match case sensitive.
For backward compatibility reasons, this feature can be switched off.
  • Loading branch information
mawinter69 authored Aug 23, 2023
1 parent 6a97d3b commit 731678c
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 4 deletions.
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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.
*/
Expand Down Expand Up @@ -133,6 +139,9 @@ public RoleMap(@NonNull SortedMap<Role, Set<PermissionEntry>> grantedRoles) {
public boolean hasPermission(PermissionEntry sid, Permission permission, RoleType roleType, AccessControlled controlledItem) {
final Set<Permission> 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
Expand All @@ -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<PermissionEntry> 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand Down

0 comments on commit 731678c

Please sign in to comment.