various minor bug fixes

jenkinsci · Aug 28, 2022 · a6eece5 · a6eece5
1 parent 8412d99
commit a6eece5
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 66 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
@@ -111,8 +111,8 @@ public class RoleBasedAuthorizationStrategy extends AuthorizationStrategy {
   private final RoleMap agentRoles;
   private final RoleMap globalRoles;
   private final RoleMap itemRoles;
-  private Set<PermissionTemplate> permissionTemplates = new HashSet<>();
-  private Set<RoleTemplate> roleTemplates = new HashSet<>();
+  private Set<PermissionTemplate> permissionTemplates;
+  private Set<RoleTemplate> roleTemplates;
 
   /**
    * Create new RoleBasedAuthorizationStrategy.
@@ -121,6 +121,8 @@ public RoleBasedAuthorizationStrategy() {
     agentRoles = new RoleMap();
     globalRoles = new RoleMap();
     itemRoles = new RoleMap();
+    permissionTemplates = new TreeSet<>();
+    roleTemplates = new TreeSet<>();
   }
 
   /**
@@ -1027,33 +1029,11 @@ public AuthorizationStrategy newInstance(StaplerRequest req, JSONObject formData
       // specifics forms, and we need to handle it.
       if (formData.has(GLOBAL) && formData.has(PROJECT) && formData.has(SLAVE) && oldStrategy instanceof RoleBasedAuthorizationStrategy) {
         strategy = new RoleBasedAuthorizationStrategy();
-
-        JSONObject globalRoles = formData.getJSONObject(GLOBAL);
-        for (Map.Entry<String, JSONObject> r : (Set<Map.Entry<String, JSONObject>>) globalRoles.getJSONObject("data").entrySet()) {
-          String roleName = r.getKey();
-          Set<Permission> permissions = new HashSet<>();
-          for (Map.Entry<String, Boolean> e : (Set<Map.Entry<String, Boolean>>) r.getValue().entrySet()) {
-            if (e.getValue()) {
-              Permission p = Permission.fromId(e.getKey());
-              permissions.add(p);
-            }
-          }
-
-          Role role = new Role(roleName, permissions);
-          strategy.addRole(RoleType.Global, role);
-          RoleMap roleMap = ((RoleBasedAuthorizationStrategy) oldStrategy).getRoleMap(RoleType.Global);
-          if (roleMap != null) {
-            Set<String> sids = roleMap.getSidsForRole(roleName);
-            if (sids != null) {
-              for (String sid : sids) {
-                strategy.assignRole(RoleType.Global, role, sid);
-              }
-            }
-          }
-        }
-
+        readRoles(formData, RoleType.Global, strategy, (RoleBasedAuthorizationStrategy) oldStrategy);
         readRoles(formData, RoleType.Project, strategy, (RoleBasedAuthorizationStrategy) oldStrategy);
         readRoles(formData, RoleType.Slave, strategy, (RoleBasedAuthorizationStrategy) oldStrategy);
+        strategy.permissionTemplates = ((RoleBasedAuthorizationStrategy) oldStrategy).permissionTemplates;
+        strategy.roleTemplates = ((RoleBasedAuthorizationStrategy) oldStrategy).roleTemplates;
       } else if (oldStrategy instanceof RoleBasedAuthorizationStrategy) {
         // When called from Hudson Manage panel, but was already on a role-based
         // strategy
@@ -1075,23 +1055,21 @@ public AuthorizationStrategy newInstance(StaplerRequest req, JSONObject formData
     private void readRoles(JSONObject formData, final RoleType roleType, RoleBasedAuthorizationStrategy targetStrategy,
         RoleBasedAuthorizationStrategy oldStrategy) {
       final String roleTypeAsString = roleType.getStringType();
-      if (!formData.has(roleTypeAsString)) {
-        assert false : "Unexistent Role type " + roleTypeAsString;
-        return;
-      }
-      JSONObject projectRoles = formData.getJSONObject(roleTypeAsString);
-      if (!projectRoles.containsKey("data")) {
+      JSONObject roles = formData.getJSONObject(roleTypeAsString);
+      if (!roles.containsKey("data")) {
         assert false : "No data at role description";
         return;
       }
 
-      for (Map.Entry<String, JSONObject> r : (Set<Map.Entry<String, JSONObject>>) projectRoles.getJSONObject("data").entrySet()) {
+      for (Map.Entry<String, JSONObject> r : (Set<Map.Entry<String, JSONObject>>) roles.getJSONObject("data").entrySet()) {
         String roleName = r.getKey();
         Set<Permission> permissions = new HashSet<>();
-        String pattern = r.getValue().getString("pattern");
-        if (pattern != null) {
+        String pattern = ".*";
+        if (r.getValue().has("pattern")) {
+          pattern = r.getValue().getString("pattern");
           r.getValue().remove("pattern");
-        } else {
+        }
+        if (pattern == null) {
           pattern = ".*";
         }
         for (Map.Entry<String, Boolean> e : (Set<Map.Entry<String, Boolean>>) r.getValue().entrySet()) {
@@ -1100,11 +1078,17 @@ private void readRoles(JSONObject formData, final RoleType roleType, RoleBasedAu
             permissions.add(p);
           }
         }
-
-        Role role = new Role(roleName, pattern, permissions);
+        RoleMap roleMap = oldStrategy.getRoleMap(roleType);
+        boolean generated = false;
+        if (roleMap != null) {
+          Role oldRole = roleMap.getRole(roleName);
+          if (oldRole != null) {
+            generated = oldRole.isGenerated();
+          }
+        }
+        Role role = new Role(roleName, Pattern.compile(pattern), permissions, "", generated);
         targetStrategy.addRole(roleType, role);
 
-        RoleMap roleMap = oldStrategy.getRoleMap(roleType);
         if (roleMap != null) {
           Set<String> sids = roleMap.getSidsForRole(roleName);
           if (sids != null) {

diff --git a/...com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/item-role-generator.jelly b/...com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/item-role-generator.jelly
@@ -31,7 +31,16 @@
   <j:set var="permissionTableid" value="permissionTemplates"/>
   <j:set var="roleTemplates" value="${it.strategy.getRoleTemplates()}"/>
   <j:set var="roleTableid" value="roleTemplates"/>
-
+
+  <j:choose>
+    <j:when test="${app.hasPermission(app.ADMINISTER)}">
+      <j:set var="readOnlyMode" value="false" />
+    </j:when>
+    <j:otherwise>
+      <j:set var="readOnlyMode" value="true" />
+    </j:otherwise>
+  </j:choose>
+
   <d:taglib uri="local">
     <d:tag name="roleRow">
       <td class="start">
@@ -44,11 +53,13 @@
       <td class="left-most" style="white-space: nowrap;">${title}</td>
       <td width="*">
         <div class="pattern-cell">
+          <j:if test="${readOnlyMode == false}">
             <l:icon class="icon-pencil icon-sm" tooltip="Edit pattern"/>
-            <span>
-              <a href="#" class="patternAnchor">&quot;${attrs.pattern}&quot;</a>
-              <input class="patternEdit" type="hidden" name="[pattern]" value="${attrs.pattern}" />
-            </span>
+          </j:if>
+          <span>
+            <a href="#" class="patternAnchor">&quot;${attrs.pattern}&quot;</a>
+            <input class="patternEdit" type="hidden" name="[pattern]" value="${attrs.pattern}" />
+          </span>
         </div>
       </td>
       <td class="stop">
@@ -110,7 +121,7 @@
 	  <f:section title="${%Permission Templates}">
 	    <f:rowSet name="permissionTemplates">
 
-  <table id="${permissionTableid}" class="center-align global-matrix-authorization-strategy-table" name="data">
+  <table id="${permissionTableid}" class="center-align global-matrix-authorization-strategy-table ${readOnlyMode ? 'read-only' : ''}" name="data">
 
       <!-- The first row will show grouping -->
     <thead>
@@ -336,7 +347,7 @@
         templateTableHighlighter = new TableHighlighter('${permissionTableid}', 2);
 
          // Show jobs matching a pattern on click
-         let projectRolesTable = document.getElementById('${permissionTableid}')
+         let projectRolesTable = document.getElementById('${roleTableid}')
          let patterns = projectRolesTable.getElementsByClassName('patternAnchor');
          for(let pattern of patterns) {
            bindListenerToPattern(pattern);

diff --git a/.../com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-agent-roles.jelly b/.../com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-agent-roles.jelly
@@ -33,7 +33,7 @@
   <j:set var="agentGrantedRoles" value="${it.strategy.getGrantedRoles(it.strategy.SLAVE)}"/>
   <j:set var="tableid" value="agentRoles"/>
 
-  <table id="${tableid}" class="center-align global-matrix-authorization-strategy-table" name="data">
+  <table id="${tableid}" class="center-align global-matrix-authorization-strategy-table ${readOnlyMode ? 'read-only' : ''}" name="data">
 
     <!-- The first row will show grouping -->
     <thead>

diff --git a/...com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-global-roles.jelly b/...com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-global-roles.jelly
@@ -38,7 +38,7 @@
       <input id="globalRoleInput" class="row-input-filter jenkins-input setting-input" data-table-id="${tableid}"/>
     </f:entry>
   </div>  
-  <table id="${tableid}" class="center-align global-matrix-authorization-strategy-table" name="data">
+  <table id="${tableid}" class="center-align global-matrix-authorization-strategy-table ${readOnlyMode ? 'read-only' : ''}" name="data">
 
       <!-- The first row shows grouping -->
     <thead>

diff --git a/...om/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-project-roles.jelly b/...om/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-project-roles.jelly
@@ -35,7 +35,7 @@
       <input id="itemRoleInput" class="row-input-filter jenkins-input setting-input" data-table-id="${tableid}"/>
     </f:entry>
   </div>  
-  <table id="${tableid}" class="center-align global-matrix-authorization-strategy-table" name="data">
+  <table id="${tableid}" class="center-align global-matrix-authorization-strategy-table ${readOnlyMode ? 'read-only' : ''}" name="data">
 
       <!-- The first row will show grouping -->
     <thead>

diff --git a/...ources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-roles.jelly b/...ources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-roles.jelly
@@ -52,16 +52,20 @@
               <input type="button" class="role-strategy-add-button" value="${%Add}" data-master-id="${id}" data-table-id="${attrs.tableid}" data-highlighter="${attrs.highlighter}"/>
             </d:tag>
             <d:tag name="roleRow">
-                <td class="start">
-                  <l:isAdmin>
-                    <j:if test="${(attrs.generated ?: false) == false}">
-                      <a href="#" class="remove">
-                        <l:icon alt="remove" class="icon-stop icon-sm"/>
-                      </a>
-                    </j:if>
-                  </l:isAdmin>
-                </td>
-              <td class="left-most" style="white-space: nowrap;">${title}</td>
+              <td class="start">
+                <l:isAdmin>
+                  <j:if test="${(attrs.generated ?: false) == false}">
+                    <a href="#" class="remove">
+                      <l:icon alt="remove" class="icon-stop icon-sm"/>
+                    </a>
+                  </j:if>
+                </l:isAdmin>
+              </td>
+              <j:set var="generated" value="${null}"/>
+              <j:if test="${attrs.generated}">
+                <j:set var="generated" value='This role is generated'/>
+              </j:if>
+              <td class="left-most" style="white-space: nowrap;" tooltip="${generated}">${title}</td>
               <j:set var="pattern" value=""/>
               <j:set var="patternTemplate" value=""/>
               <j:if test="${!attrs.global}">
@@ -70,30 +74,37 @@
                 <j:if test="${attrs.pattern != '{{PATTERN}}'}">
                   <j:set var="pattern" value="&lt;br/&gt; &lt;b&gt;Pattern&lt;/b&gt;: ${h.escape(attrs.role.pattern.toString())}"/>
                 </j:if>
-                <td width="*">
+                <td width="*" tooltip="${generated}">
                   <div class="pattern-cell">
-                      <j:if test="${(attrs.generated ?: false) == false}">
+                      <j:if test="${readOnlyMode == false &amp;&amp; (attrs.generated ?: false) == false}">
                         <l:icon class="icon-pencil icon-sm" tooltip="Edit pattern"/>
                       </j:if>
                       <span>
                         <a href="#" class="patternAnchor">&quot;${attrs.role.pattern.toString()}&quot;</a>
-                        <input class="patternEdit" type="hidden" name="[pattern]" value="${attrs.role.pattern}" />
+                        <input class="patternEdit" type="hidden" name="[pattern]" value="${attrs.role.pattern}"/>
                       </span>
                   </div>
                 </td>
               </j:if>
 
               <j:set var="tableItems" value="${it.strategy.descriptor.getGroups(attrs.type)}"/>
+              <j:set var="generated" value=""/>
+              <j:if test="${attrs.generated}">
+                <j:set var="generated" value="&lt;br/&gt;This role is generated"/>
+              </j:if>
               <j:forEach var="g" items="${tableItems}">
                 <j:forEach var="p" items="${g.permissions}">
                   <j:if test="${it.strategy.descriptor.showPermission(attrs.type, p)}">
                     <td width="*"
                         class="permissionInput"
                         data-implied-by-list="${it.strategy.descriptor.impliedByList(p)}"
                         data-permission-id="${p.id}"
-                        tooltip="&lt;b&gt;Permission&lt;/b&gt;: ${g.title}/${p.name} &lt;br/&gt; &lt;b&gt;Role&lt;/b&gt;: ${h.escape(attrs.title)} ${pattern}"
+                        tooltip="&lt;b&gt;Permission&lt;/b&gt;: ${g.title}/${p.name} &lt;br/&gt; &lt;b&gt;Role&lt;/b&gt;: ${h.escape(attrs.title)} ${pattern} ${generated}"
                         data-tooltip-template="&lt;b&gt;Permission&lt;/b&gt;: ${g.title}/${p.name}{{GRANTBYOTHER}} &lt;br/&gt; &lt;b&gt;Role&lt;/b&gt;: ${h.escape(attrs.title)} ${patternTemplate}">
-                      <f:checkbox name="[${p.id}]" checked="${attrs.role.hasPermission(p)}" readonly="${attrs.generated ?: false}"/>
+                      <j:set var="oldReadOnlyMode" value="${readOnlyMode}"/>
+                      <j:set var="readOnlyMode" value="${readOnlyMode || attrs.generated}"/>
+                      <f:checkbox name="[${p.id}]" checked="${attrs.role.hasPermission(p)}" class="${readOnlyMode ? 'read-only' : ''}"/>
+                      <j:set var="readOnlyMode" value="${oldReadOnlyMode}"/>
                     </td>
                   </j:if>
                 </j:forEach>

diff --git a/src/main/webapp/js/tableGenerate.js b/src/main/webapp/js/tableGenerate.js
@@ -201,6 +201,11 @@ Behaviour.specify(".global-matrix-authorization-strategy-table A.remove", 'RoleB
 });
 
 Behaviour.specify(".global-matrix-authorization-strategy-table td.permissionInput input", 'RoleBasedAuthorizationStrategy', 0, function(e) {
+  let table = findAncestor(e, "TABLE");
+  if (table.hasClassName('read-only')) {
+    // if this is a read-only UI (ExtendedRead / SystemRead), do not enable checkboxes
+    return;
+  }
   let row = findAncestor(e,"TR");
   let pattern = getPattern(row);
   let td = findAncestor(e,"TD");

diff --git a/src/main/webapp/js/tableManage.js b/src/main/webapp/js/tableManage.js
@@ -115,7 +115,9 @@ updateTooltip = function(tr, td, pattern) {
         input.disabled = true;
         tooltip = tooltipTemplate.replace("{{PATTERNTEMPLATE}}", doubleEscapeHTML(pattern)).replace("{{GRANTBYOTHER}}", " is granted through another permission");;
         td.setAttribute('tooltip', tooltip); // before 2.335 -- TODO remove once baseline is new enough
-        td.nextSibling.setAttribute('tooltip', tooltip); // 2.335+
+        if (td.nextSibling != null) {
+          td.nextSibling.setAttribute('tooltip', tooltip); // 2.335+
+        }
       }
     }
   }
@@ -230,6 +232,11 @@ Behaviour.specify(".global-matrix-authorization-strategy-table A.remove", 'RoleB
 });
 
 Behaviour.specify(".global-matrix-authorization-strategy-table td.permissionInput input", 'RoleBasedAuthorizationStrategy', 0, function(e) {
+  if (e.hasClassName('read-only')) {
+    // if this is a read-only UI (ExtendedRead / SystemRead), do not enable checkboxes
+    return;
+  }
+
   let row = findAncestor(e,"TR");
   let pattern = getPattern(row);
   let td = findAncestor(e,"TD");