Skip to content

Allow selected scripts to be removed from approvals #202

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.

Sign up for GitHub

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

Closed
wants to merge 14 commits into from
Closed

Allow selected scripts to be removed from approvals #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8a882e7
Select specific entries to remove
EricBartusch Apr 30, 2018
e44ddfb
Use table for other approval boxes
EricBartusch Apr 30, 2018
cc452b2
Better table styling
EricBartusch Apr 30, 2018
0fcce8e
Added unit test
EricBartusch Apr 30, 2018
e460167
Update the ui dynamically
EricBartusch Apr 30, 2018
06e71a3
Removing unneeded logic
EricBartusch May 1, 2018
15e444d
Update loop structure
EricBartusch May 1, 2018
3d36adc
Adding comment
EricBartusch May 1, 2018
5aa90d5
Removing escaped ampersands
EricBartusch May 1, 2018
2c21761
Different for loop
EricBartusch May 1, 2018
0778ce1
Fixing unit test
EricBartusch May 1, 2018
d614a85
Remove unused import
EricBartusch May 1, 2018
9d94da8
Merge branch 'master' of https://github.com/jenkinsci/script-security…
EricBartusch May 24, 2019
7746bc7
Merge branch 'master' into master
bitwiseman Nov 5, 2019
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
27 changes: 26 additions & 1 deletion src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,6 @@ private String[][] reconfigure() throws IOException {
save();
}

// TODO nicer would be to allow the user to actually edit the list directly (with syntax checks)
@Restricted(NoExternalUse.class) // for use from AJAX
@JavaScriptMethod public synchronized String[][] clearApprovedSignatures() throws IOException {
Jenkins.getInstance().checkPermission(Jenkins.RUN_SCRIPTS);
Expand Down Expand Up @@ -849,6 +848,32 @@ private String[][] reconfigure() throws IOException {
return reconfigure();
}

@Restricted(NoExternalUse.class) // for use from AJAX
@JavaScriptMethod public synchronized String[][] clearSelectedSignatures(String[] signatures) throws IOException {
Jenkins.getInstance().checkPermission(Jenkins.RUN_SCRIPTS);

for(String sig : signatures) {
Iterator<String> it = approvedSignatures.iterator();
while (it.hasNext()) {
if (sig.equals(it.next())) {
it.remove();
break;
}
}

it = aclApprovedSignatures.iterator();
while (it.hasNext()) {
if (sig.equals(it.next())) {
it.remove();
break;
}
}
}

save();
return reconfigure();
}

@Restricted(NoExternalUse.class)
public synchronized List<ApprovedClasspathEntry> getApprovedClasspathEntries() {
ArrayList<ApprovedClasspathEntry> r = new ArrayList<ApprovedClasspathEntry>(approvedClasspathEntries);
Expand Down
192 changes: 177 additions & 15 deletions src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,30 @@ THE SOFTWARE.
-->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout" xmlns:f="/lib/form">
<style>
tr.selected td {
background-color: #6886b9;
color: #fff;
}
table.approvals {
border-collapse:collapse;
height:189px;

tr {
width: 100%;
display: inline-table;
table-layout: fixed;
}
}

tbody.approvals {
overflow-y:scroll;
height:189px;
position:absolute;
border:1px solid;
}

</style>
<l:layout title="In-process Script Approval" permission="${app.RUN_SCRIPTS}">
<st:include page="sidepanel.jelly" it="${app}"/>
<l:main-panel>
Expand All @@ -45,10 +69,91 @@ THE SOFTWARE.
}
function updateApprovedSignatures(r) {
var both = r.responseObject();
$('approvedSignatures').value = both[0].join('\n');
$('aclApprovedSignatures').value = both[1].join('\n');
$('dangerousApprovedSignatures').value = both[2].join('\n');

var newApprovedSignatures = document.createElement('tbody');
var oldApprovedSignatures = $('approvedSignaturesBody');
newApprovedSignatures.setAttribute('class', 'approvals');
newApprovedSignatures.setAttribute('id', 'approvedSignaturesBody');
both[0].forEach(function (approvedSig) {
var tr = document.createElement('tr');

var td = document.createElement('td');
td.setAttribute('onclick', 'selectRow(this)');
td.setAttribute('class', 'text_data');

var txt = document.createTextNode(approvedSig);

td.appendChild(txt);
tr.appendChild(td);
newApprovedSignatures.appendChild(tr);
});

//case where this is the first approvals to be added
if(oldApprovedSignatures === null) {
$('approvedSignatures').appendChild(newApprovedSignatures);
} else {
$('approvedSignatures').replaceChild(newApprovedSignatures, oldApprovedSignatures);
}
//case where the last approval has been removed
if(both[0].length === 0 &amp;&amp; $('approvedSignatures').firstChild !== null) {
$('approvedSignatures').removeChild($('approvedSignatures').firstChild);
}

var newApprovedAclSignatures = document.createElement('tbody');
var oldApprovedAclSignatures = $('aclApprovedSignaturesBody');
newApprovedAclSignatures.setAttribute('class', 'approvals');
newApprovedAclSignatures.setAttribute('id', 'aclApprovedSignaturesBody');
both[1].forEach(function (aclApprovedSig) {
var tr = document.createElement('tr');

var td = document.createElement('td');
td.setAttribute('onclick', 'selectRow(this)');
td.setAttribute('class', 'text_data');

var txt = document.createTextNode(aclApprovedSig);

td.appendChild(txt);
tr.appendChild(td);
newApprovedAclSignatures.appendChild(tr);
});

if(oldApprovedAclSignatures === null) {
$('aclApprovedSignatures').appendChild(newApprovedAclSignatures);
} else {
$('aclApprovedSignatures').replaceChild(newApprovedAclSignatures, oldApprovedAclSignatures);
}
if(both[1].length === 0 &amp;&amp; $('aclApprovedSignatures').firstChild !== null) {
$('aclApprovedSignatures').removeChild($('aclApprovedSignatures').firstChild);
}

var newApprovedDangerousSignatures = document.createElement('tbody');
var oldApprovedDangerousSignatures = $('dangerousApprovedSignaturesBody');
newApprovedDangerousSignatures.setAttribute('class', 'approvals');
newApprovedDangerousSignatures.setAttribute('id', 'dangerousApprovedSignaturesBody');
both[2].forEach(function (aclApprovedSig) {
var tr = document.createElement('tr');

var td = document.createElement('td');
td.setAttribute('onclick', 'selectRow(this)');
td.setAttribute('class', 'text_data');

var txt = document.createTextNode(aclApprovedSig);

td.appendChild(txt);
tr.appendChild(td);
newApprovedDangerousSignatures.appendChild(tr);
});

if(oldApprovedDangerousSignatures === null) {
$('dangerousApprovedSignatures').appendChild(newApprovedDangerousSignatures);
} else {
$('dangerousApprovedSignatures').replaceChild(newApprovedDangerousSignatures, oldApprovedDangerousSignatures);
}
if(both[2].length === 0 &amp;&amp; $('dangerousApprovedSignatures').firstChild !== null) {
$('dangerousApprovedSignatures').removeChild($('dangerousApprovedSignatures').firstChild);
}
}

function approveSignature(signature, hash) {
mgr.approveSignature(signature, function(r) {
updateApprovedSignatures(r);
Expand All @@ -75,6 +180,16 @@ THE SOFTWARE.
updateApprovedSignatures(r);
});
}
function clearSelectedSignatures() {
var rows = document.getElementsByClassName('selected');
var signatures = [];
for(var i = 0; i &lt; rows.length; i++) {
signatures.push(rows[i].firstChild.innerText);
}
mgr.clearSelectedSignatures(signatures, function(r) {
updateApprovedSignatures(r);
});
}

function renderPendingClasspathEntries(pendingClasspathEntries) {
if (pendingClasspathEntries.length == 0) {
Expand Down Expand Up @@ -174,6 +289,15 @@ THE SOFTWARE.
renderClasspaths(r);
});
}

function selectRow(row) {
var tr = row.parentElement
if(tr.hasClassName("selected")) {
tr.removeClassName("selected");
} else {
tr.addClassName("selected");
}
}

Event.observe(window, "load", function(){
mgr.getClasspathRenderInfo(function(r) {
Expand Down Expand Up @@ -203,6 +327,10 @@ THE SOFTWARE.
You can also remove all previous script approvals:
<button onclick="if (confirm('Really delete all approvals? Any existing scripts will need to be requeued and reapproved.')) {mgr.clearApprovedScripts()}">Clear Approvals</button>
</p>
<p>
Remove selected signatures:
<button onclick="clearSelectedSignatures()">Clear selected</button>
</p>
<hr/>
<j:choose>
<j:when test="${it.pendingSignatures.isEmpty()}">
Expand Down Expand Up @@ -230,24 +358,58 @@ THE SOFTWARE.
</j:otherwise>
</j:choose>
<p>Signatures already approved:</p>
<textarea readonly="readonly" id="approvedSignatures" rows="10" cols="80">
<j:forEach var="line" items="${it.approvedSignatures}">${line}<st:out value="&#10;"/></j:forEach>
</textarea>
<table class="approvals" id="approvedSignatures">
<tbody class="approvals" id="approvedSignaturesBody">
<j:forEach var="line" items="${it.approvedSignatures}">
<tr>
<td onclick="selectRow(this)" class="text_data">${line}</td>
</tr>
</j:forEach>
</tbody>
</table>
<p>Signatures already approved assuming permission check:</p>
<textarea readonly="readonly" id="aclApprovedSignatures" rows="10" cols="80">
<j:forEach var="line" items="${it.aclApprovedSignatures}">${line}<st:out value="&#10;"/></j:forEach>
</textarea>
<j:choose>
<j:when test="${!empty(aclApprovedSignatures)}">
<table class="approvals" id="aclApprovedSignatures">
<tbody class="approvals" id="aclApprovedSignaturesBody">
<j:forEach var="line" items="${it.aclApprovedSignatures}">
<tr>
<td onclick="selectRow(this)" class="text_data">${line}</td>
</tr>
</j:forEach>
</tbody>
</table>
</j:when>
<j:otherwise>
<table class="approvals" id="aclApprovedSignatures"></table>
</j:otherwise>
</j:choose>
<j:set var="dangerousApprovedSignatures" value="${it.dangerousApprovedSignatures}"/>
<j:if test="${!empty(dangerousApprovedSignatures)}">
<p>Signatures already approved which <strong><font color="red">may have introduced a security vulnerability</font></strong> (recommend clearing):</p>
<textarea readonly="readonly" id="dangerousApprovedSignatures" rows="10" cols="80">
<j:forEach var="line" items="${dangerousApprovedSignatures}">${line}<st:out value="&#10;"/></j:forEach>
</textarea>
</j:if>
<p>Signatures already approved which <strong><font color="red">may have introduced a security vulnerability</font></strong> (recommend clearing):</p>
<j:choose>
<j:when test="${!empty(dangerousApprovedSignatures)}">
<table class="approvals" id="dangerousApprovedSignatures">
<tbody class="approvals" id="dangerousApprovedSignaturesBody">
<j:forEach var="line" items="${it.dangerousApprovedSignatures}">
<tr>
<td onclick="selectRow(this)" class="text_data">${line}</td>
</tr>
</j:forEach>
</tbody>
</table>
</j:when>
<j:otherwise>
<table class="approvals" id="dangerousApprovedSignatures"></table>
</j:otherwise>
</j:choose>
<p>
You can also remove all previous signature approvals:
<button onclick="if (confirm('Really delete all approvals? Any existing scripts will need to be rerun and signatures reapproved.')) {clearApprovedSignatures()}">Clear Approvals</button>
</p>
<p>
Remove selected signatures:
<button onclick="clearSelectedSignatures()">Clear selected</button>
</p>
<j:if test="${!empty(dangerousApprovedSignatures)}">
Or you can just remove the dangerous ones:
<button onclick="clearDangerousApprovedSignatures()">Clear only dangerous Approvals</button>
Expand Down
26 changes: 24 additions & 2 deletions src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
package org.jenkinsci.plugins.scriptsecurity.scripts;

import com.gargoylesoftware.htmlunit.html.HtmlPage;
import com.gargoylesoftware.htmlunit.html.HtmlTable;
import com.gargoylesoftware.htmlunit.html.HtmlTextArea;
import hudson.model.FreeStyleProject;
import hudson.model.Result;
Expand Down Expand Up @@ -103,8 +104,8 @@ public void malformedScriptApproval() throws Exception {
assertThat(managePageBodyText, Matchers.containsString("1 dangerous signatures previously approved which ought not have been."));

HtmlPage scriptApprovalPage = managePage.getAnchorByHref("scriptApproval").click();
HtmlTextArea approvedTextArea = scriptApprovalPage.getHtmlElementById("approvedSignatures");
HtmlTextArea dangerousTextArea = scriptApprovalPage.getHtmlElementById("dangerousApprovedSignatures");
HtmlTable approvedTextArea = scriptApprovalPage.getHtmlElementById("approvedSignatures");
HtmlTable dangerousTextArea = scriptApprovalPage.getHtmlElementById("dangerousApprovedSignatures");

assertThat(approvedTextArea.getTextContent(), Matchers.containsString(DANGEROUS_SIGNATURE));
assertThat(dangerousTextArea.getTextContent(), Matchers.containsString(DANGEROUS_SIGNATURE));
Expand Down Expand Up @@ -140,6 +141,27 @@ public void malformedScriptApproval() throws Exception {
assertEquals(0, sa.getDangerousApprovedSignatures().length);
}

@Test public void clearSelectedMethodLifeCycle() throws Exception {
ScriptApproval sa = ScriptApproval.get();

String signature1 = "method java.io.Writer write java.lang.String";
String signature2 = "method java.lang.AutoCloseable close";

sa.approveSignature(WHITELISTED_SIGNATURE);
sa.approveSignature(DANGEROUS_SIGNATURE);
sa.approveSignature(signature1);
sa.approveSignature(signature2);
assertEquals(4, sa.getApprovedSignatures().length);

String[] toBeRemoved = {WHITELISTED_SIGNATURE};
sa.clearSelectedSignatures(toBeRemoved);
assertEquals(3, sa.getApprovedSignatures().length);

toBeRemoved = new String[]{signature1, signature2};
sa.clearSelectedSignatures(toBeRemoved);
assertEquals(1, sa.getApprovedSignatures().length);
}

@Issue("JENKINS-57563")
@LocalData // Just a scriptApproval.xml that whitelists 'staticMethod jenkins.model.Jenkins getInstance'
@Test
Expand Down