some minor fixes

jenkinsci · Aug 9, 2022 · 3b21c61 · 3b21c61
1 parent 6922951
commit 3b21c61
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 6 deletions.
diff --git a/...ain/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleBasedAuthorizationStrategy.java b/...ain/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleBasedAuthorizationStrategy.java
@@ -576,7 +576,8 @@ public void doUnassignRole(@QueryParameter(required = true) String type,
    *
    * <p>
    * Example:
-   * {@code curl -X POST localhost:8080/role-strategy/strategy/unassignUserRole --data "type=globalRoles&amp;roleName=AMD&amp;user=username"}
+   * {@code curl -X POST localhost:8080/role-strategy/strategy/unassignUserRole --data
+   *   "type=globalRoles&amp;roleName=AMD&amp;user=username"}
    *
    * @param type     (globalRoles, projectRoles, slaveRoles)
    * @param roleName unassign role with sid
@@ -603,7 +604,8 @@ public void doUnassignUserRole(@QueryParameter(required = true) String type,
    *
    * <p>
    * Example:
-   * {@code curl -X POST localhost:8080/role-strategy/strategy/unassignGroupRole --data "type=globalRoles&amp;roleName=AMD&amp;user=username"}
+   * {@code curl -X POST localhost:8080/role-strategy/strategy/unassignGroupRole --data
+   *   "type=globalRoles&amp;roleName=AMD&amp;user=username"}
    *
    * @param type     (globalRoles, projectRoles, slaveRoles)
    * @param roleName unassign role with sid

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
@@ -136,6 +136,16 @@ public boolean hasPermission(String sid, Permission permission, RoleType roleTyp
     // or a permission implying the given permission
     new RoleWalker() {
 
+      /**
+       * Checks whether the given sid is granted permission.
+       * First checks if there is a dedicated match for user/group.
+       * If not checks if there is an entry for either.
+       *
+       * @param current The current role
+       * @param sid The sid to checked
+       * @param principal If the sid is a user or a group.
+       * @return The PermissionEntry that matched or null if nothing matched.
+       */
       @CheckForNull
       private PermissionEntry hasPermission(Role current, String sid, boolean principal) {
         PermissionEntry entry = new PermissionEntry(principal ? AuthorizationType.USER : AuthorizationType.GROUP, sid);
@@ -153,7 +163,7 @@ private PermissionEntry hasPermission(Role current, String sid, boolean principa
       public void perform(Role current) {
         if (current.hasAnyPermission(permissions)) {
           PermissionEntry entry = hasPermission(current, sid, principal);
-          if (grantedRoles.get(current).contains(entry)) {
+          if (entry != null) {
             // Handle roles macro
             if (Macro.isMacro(current)) {
               Macro macro = RoleMacroExtension.getMacro(current.getName());

diff --git a/src/main/java/org/jenkinsci/plugins/rolestrategy/Settings.java b/src/main/java/org/jenkinsci/plugins/rolestrategy/Settings.java
@@ -58,7 +58,7 @@ public class Settings {
 
   /**
    * Enabling processing of User Authorities. Alters the behavior of
-   * {@link RoleMap#hasPermission(java.lang.String, hudson.security.Permission,
+   * {@link RoleMap#hasPermission(com.synopsys.arc.jenkins.plugins.rolestrategy.PermissionEntry, hudson.security.Permission,
    * com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType, hudson.security.AccessControlled)}.
    * Since 2.3.0 this value was {@code true}, but it has been switched due to the performance reasons. The behavior can be
    * reverted (even dynamically via System Groovy Script).

diff --git a/...a/org/jenkinsci/plugins/rolestrategy/casc/RoleBasedAuthorizationStrategyConfigurator.java b/...a/org/jenkinsci/plugins/rolestrategy/casc/RoleBasedAuthorizationStrategyConfigurator.java
@@ -88,7 +88,8 @@ private Function<Entry<Role, Set<PermissionEntry>>, RoleDefinition> getRoleDefin
       Role role = roleSetEntry.getKey();
       List<String> permissions = role.getPermissions().stream()
           .map(permission -> permission.group.title.toString(Locale.US) + "/" + permission.name).collect(Collectors.toList());
-      List<String> assignements = roleSetEntry.getValue().stream().map(entry -> entry.getType().toPrefix() + entry.getSid()).collect(Collectors.toList());
+      List<String> assignements = roleSetEntry.getValue().stream().map(entry -> entry.getType().toPrefix() + entry.getSid())
+          .collect(Collectors.toList());
       return new RoleDefinition(role.getName(), role.getDescription(), role.getPattern().pattern(), permissions, assignements);
     };
   }

diff --git a/src/test/java/jmh/benchmarks/PermissionBenchmark.java b/src/test/java/jmh/benchmarks/PermissionBenchmark.java
@@ -36,7 +36,7 @@ public void setup() {
       Set<String> permissionSet = Collections.singleton("hudson.model.Hudson.Administer");
       Role role = new Role("USERS", ".*", permissionSet, "description");
       RoleMap roleMap = new RoleMap(new TreeMap<>(// expects a sorted map
-          Collections.singletonMap(role, Collections.singleton(new PermissionEntry(AuthorizationType.USER,"alice")))));
+          Collections.singletonMap(role, Collections.singleton(new PermissionEntry(AuthorizationType.USER, "alice")))));
 
       jenkins.setAuthorizationStrategy(
           new RoleBasedAuthorizationStrategy(Collections.singletonMap(RoleBasedAuthorizationStrategy.GLOBAL, roleMap)));