[FIXED JENKINS-41637] Users with discover permission get broken UI wh…

…en renaming job to inaccessible one (#2910) * [JENKINS-41637] - Add new check that new job name isn't used * Fixed bugs * Address comment * [JENKINS-41637] - Address comments * Add logging * Address comment
jenkinsci · Sep 3, 2017 · 757fbe4 · 757fbe4
1 parent 33799df
commit 757fbe4
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 3 deletions.
diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java
@@ -120,6 +120,9 @@
 
 import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
 import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
+import org.acegisecurity.AccessDeniedException;
+import org.acegisecurity.context.SecurityContext;
+import org.acegisecurity.context.SecurityContextHolder;
 
 /**
  * A job is an runnable entity under the monitoring of Hudson.
@@ -1606,4 +1609,58 @@ public BuildTimelineWidget getTimeline() {
     }
 
     private final static HexStringConfidentialKey SERVER_COOKIE = new HexStringConfidentialKey(Job.class,"serverCookie",16);
+
+    /**
+     * Check new name for job
+     * @param newName - New name for job
+     * @return {@code true} - if newName occupied and user has permissions for this job
+     *         {@code false} - if newName occupied and user hasn't permissions for this job
+     *         {@code null} - if newName didn't occupied
+     * 
+     * @throws Failure if the given name is not good
+     */
+    @CheckForNull
+    @Restricted(NoExternalUse.class)
+    public Boolean checkIfNameIsUsed(@Nonnull String newName) throws Failure{
+
+        Item item = null;
+        Jenkins.checkGoodName(newName);
+
+        try {
+            item = getParent().getItem(newName);
+        } catch(AccessDeniedException ex) {  
+            if (LOGGER.isLoggable(Level.FINE)) {
+                LOGGER.log(Level.FINE, "Unable to rename the job {0}: name {1} is already in use. " +
+                        "User {2} has {3} permission, but no {4} for existing job with the same name", 
+                        new Object[] {this.getFullName(), newName, User.current().getFullName(), Item.DISCOVER.name, Item.READ.name} );
+            }
+            return true;
+        }
+
+        if (item != null) {
+            // User has Read permissions for existing job with the same name
+            return true;
+        } else {
+            SecurityContext initialContext = null;
+            try {
+                initialContext = hudson.security.ACL.impersonate(ACL.SYSTEM);
+                item = getParent().getItem(newName);
+
+                if (item != null) {
+                    if (LOGGER.isLoggable(Level.FINE)) {
+                        LOGGER.log(Level.FINE, "Unable to rename the job {0}: name {1} is already in use. " +
+                                "User {2} has no {3} permission for existing job with the same name", 
+                                new Object[] {this.getFullName(), newName, initialContext.getAuthentication().getName(), Item.DISCOVER.name} );
+                    }
+                    return false;
+                }
+
+            } finally {
+                if (initialContext != null) {
+                    SecurityContextHolder.setContext(initialContext);
+                }
+            }
+        }
+        return null;
+    }
 }
diff --git a/core/src/main/resources/hudson/model/Job/rename.jelly b/core/src/main/resources/hudson/model/Job/rename.jelly
@@ -29,19 +29,28 @@ THE SOFTWARE.
     <st:include page="sidepanel.jelly" />
     <l:main-panel>
       <j:set var="newName" value="${request.getParameter('newName')}" />
-      <j:choose>
+      <j:set var="checkIfNameIsUsed" value="${it.checkIfNameIsUsed(newName)}" />
+
+      <j:choose> 
         <j:when test="${it.isBuilding()}">
           ${%noRenameWhileBuilding}
           <j:if test="${request.referer.endsWith('/configure')}">
             <br/> ${%configWasSaved}
           </j:if>
         </j:when>
-        <j:when test="${it.parent.getItem(newName)!=null and !it.name.equalsIgnoreCase(newName)}">
-          ${%newNameInUse(newName)}
+
+        <j:when test="${checkIfNameIsUsed != null and !it.name.equalsIgnoreCase(newName)}" >    
+          <j:if test="${checkIfNameIsUsed}" >
+            ${%newNameInUse(newName)}
+          </j:if>  
+          <j:if test="${!checkIfNameIsUsed}" >
+            ${%newNameNotValid(newName)}
+          </j:if>
           <j:if test="${request.referer.endsWith('/configure')}">
             <br/> ${%configWasSaved}
           </j:if>
         </j:when>
+
         <j:otherwise>
           <form method="post" action="doRename">
             ${%description(it.name, newName)}

diff --git a/core/src/main/resources/hudson/model/Job/rename.properties b/core/src/main/resources/hudson/model/Job/rename.properties
@@ -22,6 +22,7 @@
 
 noRenameWhileBuilding=Unable to rename a job while it is building.
 newNameInUse=The name {0} is already in use.
+newNameNotValid=The name {0} is not valid.
 configWasSaved=All other configuration options were saved.
 description=Are you sure about renaming {0} to {1}?
 Yes=Yes