diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java index 5bce32055..f585abd20 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java @@ -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); @@ -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 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 getApprovedClasspathEntries() { ArrayList r = new ArrayList(approvedClasspathEntries); diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly index 6a9ed46b9..90f580f1d 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly @@ -24,6 +24,30 @@ THE SOFTWARE. --> + @@ -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 && $('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 && $('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 && $('dangerousApprovedSignatures').firstChild !== null) { + $('dangerousApprovedSignatures').removeChild($('dangerousApprovedSignatures').firstChild); + } } + function approveSignature(signature, hash) { mgr.approveSignature(signature, function(r) { updateApprovedSignatures(r); @@ -75,6 +180,16 @@ THE SOFTWARE. updateApprovedSignatures(r); }); } + function clearSelectedSignatures() { + var rows = document.getElementsByClassName('selected'); + var signatures = []; + for(var i = 0; i < rows.length; i++) { + signatures.push(rows[i].firstChild.innerText); + } + mgr.clearSelectedSignatures(signatures, function(r) { + updateApprovedSignatures(r); + }); + } function renderPendingClasspathEntries(pendingClasspathEntries) { if (pendingClasspathEntries.length == 0) { @@ -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) { @@ -203,6 +327,10 @@ THE SOFTWARE. You can also remove all previous script approvals:

+

+ Remove selected signatures: + +

@@ -230,24 +358,58 @@ THE SOFTWARE.

Signatures already approved:

- + + + + + + + + +
${line}

Signatures already approved assuming permission check:

- + + + + + + + + + + +
${line}
+ + +
+ + - -

Signatures already approved which may have introduced a security vulnerability (recommend clearing):

- - +

Signatures already approved which may have introduced a security vulnerability (recommend clearing):

+ + + + + + + + + + +
${line}
+ + +
+ +

You can also remove all previous signature approvals:

+

+ Remove selected signatures: + +

Or you can just remove the dangerous ones: diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java index 99f8bb595..689272822 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java @@ -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; @@ -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)); @@ -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