Skip to content
Permalink
Browse files

[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
  • Loading branch information...
ksenia-nenasheva authored and oleg-nenashev committed Sep 3, 2017
1 parent 33799df commit 757fbe41fab0af4dd61d32464e7f4208fdbb0b25
@@ -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;
}
}
@@ -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)}
@@ -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

0 comments on commit 757fbe4

Please sign in to comment.
You can’t perform that action at this time.