Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-19409] use idstrategy to match role assignments #332

Merged
merged 2 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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