From 8a882e763dccf5c6f745b7bd544f9198cf8af9f3 Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Mon, 30 Apr 2018 10:29:00 -0500 Subject: [PATCH 01/12] Select specific entries to remove --- .../scripts/ScriptApproval.java | 27 ++++++++++++ .../scripts/ScriptApproval/index.jelly | 43 +++++++++++++++++-- 2 files changed, 67 insertions(+), 3 deletions(-) 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 c97fdea1f..7662da644 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java @@ -842,6 +842,33 @@ 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(); + it = approvedSignatures.iterator(); + 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..8e79cde41 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,16 @@ THE SOFTWARE. --> + @@ -75,6 +85,16 @@ THE SOFTWARE. updateApprovedSignatures(r); }); } + function clearSelectedSignatures() { + var rows = document.getElementsByClassName('selected'); + var signatures = []; + for(row of rows) { + signatures.push(row.innerText); + } + mgr.clearSelectedSignatures(signatures, function(r) { + updateApprovedSignatures(r); + }); + } function renderPendingClasspathEntries(pendingClasspathEntries) { if (pendingClasspathEntries.length == 0) { @@ -174,6 +194,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 +232,10 @@ THE SOFTWARE. You can also remove all previous script approvals:

+

+ Remove selected signatures: + +

@@ -230,9 +263,9 @@ THE SOFTWARE.

Signatures already approved:

- + + +
${line}

Signatures already approved assuming permission check:

+ +

Signatures already approved assuming permission check:

+ + +
${line}
+

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

- + + +
${line}

You can also remove all previous signature approvals: From cc452b2f5777e4f0acd9c278a91177aef161a088 Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Mon, 30 Apr 2018 10:54:50 -0500 Subject: [PATCH 03/12] Better table styling --- .../scripts/ScriptApproval/index.jelly | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) 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 12765c560..c89d4adda 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 @@ -30,9 +30,23 @@ THE SOFTWARE. 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; - border-collapse:collapse } + @@ -264,19 +278,25 @@ THE SOFTWARE.

Signatures already approved:

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

Signatures already approved assuming permission check:

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

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

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

From 0fcce8e1e6a74cddd6a952abdaa5989217978148 Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Mon, 30 Apr 2018 12:23:37 -0500 Subject: [PATCH 04/12] Added unit test --- .../scripts/ScriptApprovalTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) 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 b78fe6811..54b423a63 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java @@ -136,6 +136,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); + } + private Script script(String groovy) { return new Script(groovy); } From e4601675652f4142eebbe9f8688885780b4bf0a1 Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Mon, 30 Apr 2018 15:34:29 -0500 Subject: [PATCH 05/12] Update the ui dynamically --- .../scripts/ScriptApproval.java | 2 - .../scripts/ScriptApproval/index.jelly | 139 +++++++++++++++--- 2 files changed, 116 insertions(+), 25 deletions(-) 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 7662da644..33122486c 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java @@ -809,7 +809,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); @@ -859,7 +858,6 @@ private String[][] reconfigure() throws IOException { while (it.hasNext()) { if (sig.equals(it.next())) { it.remove(); - it = approvedSignatures.iterator(); break; } } 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 c89d4adda..07ddbf5c9 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 @@ -69,10 +69,93 @@ 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'); + + if(both[0].length > 0) { + 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); + }); + + if(oldApprovedSignatures === null) { + $('approvedSignatures').appendChild(newApprovedSignatures); + } else { + $('approvedSignatures').replaceChild(newApprovedSignatures, oldApprovedSignatures); + } + if(both[0].length === 0 && $('approvedSignatures').firstChild !== null) { + $('approvedSignatures').removeChild($('approvedSignatures').firstChild); + } + } + + if(both[1].length > 0) { + 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); @@ -277,28 +360,38 @@ THE SOFTWARE.

Signatures already approved:

- - - - -
${line}
- -

Signatures already approved assuming permission check:

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

Signatures already approved assuming permission check:

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

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

- - - - -
${line}
- +

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

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

You can also remove all previous signature approvals: From 06e71a3ee8734c3f6b512a78139950068846b713 Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Tue, 1 May 2018 08:51:18 -0500 Subject: [PATCH 06/12] Removing unneeded logic --- .../scripts/ScriptApproval/index.jelly | 82 +++++++++---------- 1 file changed, 39 insertions(+), 43 deletions(-) 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 07ddbf5c9..6a5d0a49e 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 @@ -70,59 +70,55 @@ THE SOFTWARE. function updateApprovedSignatures(r) { var both = r.responseObject(); - if(both[0].length > 0) { - 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 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 td = document.createElement('td'); + td.setAttribute('onclick', 'selectRow(this)'); + td.setAttribute('class', 'text_data'); - var txt = document.createTextNode(approvedSig); + var txt = document.createTextNode(approvedSig); - td.appendChild(txt); - tr.appendChild(td); - newApprovedSignatures.appendChild(tr); - }); + td.appendChild(txt); + tr.appendChild(td); + newApprovedSignatures.appendChild(tr); + }); - if(oldApprovedSignatures === null) { - $('approvedSignatures').appendChild(newApprovedSignatures); - } else { - $('approvedSignatures').replaceChild(newApprovedSignatures, oldApprovedSignatures); - } - if(both[0].length === 0 && $('approvedSignatures').firstChild !== null) { - $('approvedSignatures').removeChild($('approvedSignatures').firstChild); - } + if(oldApprovedSignatures === null) { + $('approvedSignatures').appendChild(newApprovedSignatures); + } else { + $('approvedSignatures').replaceChild(newApprovedSignatures, oldApprovedSignatures); + } + if(both[0].length === 0 && $('approvedSignatures').firstChild !== null) { + $('approvedSignatures').removeChild($('approvedSignatures').firstChild); } - if(both[1].length > 0) { - 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 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 td = document.createElement('td'); + td.setAttribute('onclick', 'selectRow(this)'); + td.setAttribute('class', 'text_data'); - var txt = document.createTextNode(aclApprovedSig); + var txt = document.createTextNode(aclApprovedSig); - td.appendChild(txt); - tr.appendChild(td); - newApprovedAclSignatures.appendChild(tr); - }); + td.appendChild(txt); + tr.appendChild(td); + newApprovedAclSignatures.appendChild(tr); + }); - if(oldApprovedAclSignatures === null) { - $('aclApprovedSignatures').appendChild(newApprovedAclSignatures); - } else { - $('aclApprovedSignatures').replaceChild(newApprovedAclSignatures, oldApprovedAclSignatures); - } + if(oldApprovedAclSignatures === null) { + $('aclApprovedSignatures').appendChild(newApprovedAclSignatures); + } else { + $('aclApprovedSignatures').replaceChild(newApprovedAclSignatures, oldApprovedAclSignatures); } if(both[1].length === 0 && $('aclApprovedSignatures').firstChild !== null) { $('aclApprovedSignatures').removeChild($('aclApprovedSignatures').firstChild); From 15e444d0134b6c2780839aa97e2a86d9ad0a06d3 Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Tue, 1 May 2018 10:10:07 -0500 Subject: [PATCH 07/12] Update loop structure --- .../scripts/ScriptApproval/index.jelly | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) 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 6a5d0a49e..accc7343b 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 @@ -358,7 +358,11 @@ THE SOFTWARE.

Signatures already approved:

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

Signatures already approved assuming permission check:

@@ -366,7 +370,11 @@ THE SOFTWARE. - + + + + +
${line}
${line}
@@ -380,7 +388,11 @@ THE SOFTWARE. - + + + + +
${line}
${line}
From 3d36adc81fc3f29b696f6b3ed5dd8a28fbee303c Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Tue, 1 May 2018 12:03:12 -0500 Subject: [PATCH 08/12] Adding comment --- .../plugins/scriptsecurity/scripts/ScriptApproval/index.jelly | 2 ++ 1 file changed, 2 insertions(+) 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 accc7343b..599b9ea2c 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 @@ -115,11 +115,13 @@ THE SOFTWARE. newApprovedAclSignatures.appendChild(tr); }); + //case where this is the first approvals to be added if(oldApprovedAclSignatures === null) { $('aclApprovedSignatures').appendChild(newApprovedAclSignatures); } else { $('aclApprovedSignatures').replaceChild(newApprovedAclSignatures, oldApprovedAclSignatures); } + //case where the last approval has been removed if(both[1].length === 0 && $('aclApprovedSignatures').firstChild !== null) { $('aclApprovedSignatures').removeChild($('aclApprovedSignatures').firstChild); } From 5aa90d5f00e4c48925ac63f42c8a8a8d31089e5b Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Tue, 1 May 2018 12:48:14 -0500 Subject: [PATCH 09/12] Removing escaped ampersands --- .../scripts/ScriptApproval/index.jelly | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) 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 599b9ea2c..d9b167d63 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 @@ -93,8 +93,10 @@ THE SOFTWARE. } else { $('approvedSignatures').replaceChild(newApprovedSignatures, oldApprovedSignatures); } - if(both[0].length === 0 && $('approvedSignatures').firstChild !== null) { - $('approvedSignatures').removeChild($('approvedSignatures').firstChild); + if(both[0].length === 0) { + if($('approvedSignatures').firstChild !== null) { + $('approvedSignatures').removeChild($('approvedSignatures').firstChild); + } } var newApprovedAclSignatures = document.createElement('tbody'); @@ -122,8 +124,10 @@ THE SOFTWARE. $('aclApprovedSignatures').replaceChild(newApprovedAclSignatures, oldApprovedAclSignatures); } //case where the last approval has been removed - if(both[1].length === 0 && $('aclApprovedSignatures').firstChild !== null) { - $('aclApprovedSignatures').removeChild($('aclApprovedSignatures').firstChild); + if(both[1].length === 0) { + if($('aclApprovedSignatures').firstChild !== null) { + $('aclApprovedSignatures').removeChild($('aclApprovedSignatures').firstChild); + } } var newApprovedDangerousSignatures = document.createElement('tbody'); @@ -149,8 +153,10 @@ THE SOFTWARE. } else { $('dangerousApprovedSignatures').replaceChild(newApprovedDangerousSignatures, oldApprovedDangerousSignatures); } - if(both[2].length === 0 && $('dangerousApprovedSignatures').firstChild !== null) { - $('dangerousApprovedSignatures').removeChild($('dangerousApprovedSignatures').firstChild); + if(both[2].length === 0) { + if($('dangerousApprovedSignatures').firstChild !== null) { + $('dangerousApprovedSignatures').removeChild($('dangerousApprovedSignatures').firstChild); + } } } From 2c2176167b786f613a0a608085f75dca3e308e7c Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Tue, 1 May 2018 13:43:54 -0500 Subject: [PATCH 10/12] Different for loop --- .../scripts/ScriptApproval/index.jelly | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) 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 d9b167d63..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 @@ -88,15 +88,15 @@ THE SOFTWARE. newApprovedSignatures.appendChild(tr); }); + //case where this is the first approvals to be added if(oldApprovedSignatures === null) { $('approvedSignatures').appendChild(newApprovedSignatures); } else { $('approvedSignatures').replaceChild(newApprovedSignatures, oldApprovedSignatures); } - if(both[0].length === 0) { - if($('approvedSignatures').firstChild !== null) { - $('approvedSignatures').removeChild($('approvedSignatures').firstChild); - } + //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'); @@ -117,17 +117,13 @@ THE SOFTWARE. newApprovedAclSignatures.appendChild(tr); }); - //case where this is the first approvals to be added if(oldApprovedAclSignatures === null) { $('aclApprovedSignatures').appendChild(newApprovedAclSignatures); } else { $('aclApprovedSignatures').replaceChild(newApprovedAclSignatures, oldApprovedAclSignatures); } - //case where the last approval has been removed - if(both[1].length === 0) { - if($('aclApprovedSignatures').firstChild !== null) { - $('aclApprovedSignatures').removeChild($('aclApprovedSignatures').firstChild); - } + if(both[1].length === 0 && $('aclApprovedSignatures').firstChild !== null) { + $('aclApprovedSignatures').removeChild($('aclApprovedSignatures').firstChild); } var newApprovedDangerousSignatures = document.createElement('tbody'); @@ -153,10 +149,8 @@ THE SOFTWARE. } else { $('dangerousApprovedSignatures').replaceChild(newApprovedDangerousSignatures, oldApprovedDangerousSignatures); } - if(both[2].length === 0) { - if($('dangerousApprovedSignatures').firstChild !== null) { - $('dangerousApprovedSignatures').removeChild($('dangerousApprovedSignatures').firstChild); - } + if(both[2].length === 0 && $('dangerousApprovedSignatures').firstChild !== null) { + $('dangerousApprovedSignatures').removeChild($('dangerousApprovedSignatures').firstChild); } } @@ -189,8 +183,8 @@ THE SOFTWARE. function clearSelectedSignatures() { var rows = document.getElementsByClassName('selected'); var signatures = []; - for(row of rows) { - signatures.push(row.innerText); + for(var i = 0; i < rows.length; i++) { + signatures.push(rows[i].firstChild.innerText); } mgr.clearSelectedSignatures(signatures, function(r) { updateApprovedSignatures(r); From 0778ce1ef3673e4fab82843758be512f3ff662e5 Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Tue, 1 May 2018 13:54:48 -0500 Subject: [PATCH 11/12] Fixing unit test --- .../plugins/scriptsecurity/scripts/ScriptApprovalTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 54b423a63..c2683acda 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.util.VersionNumber; import jenkins.model.Jenkins; @@ -99,8 +100,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)); From d614a854d4f5f2cecb46bcffde566fbd5c8cd686 Mon Sep 17 00:00:00 2001 From: Eric Bartusch Date: Tue, 1 May 2018 13:55:07 -0500 Subject: [PATCH 12/12] Remove unused import --- .../plugins/scriptsecurity/scripts/ScriptApprovalTest.java | 1 - 1 file changed, 1 deletion(-) 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 c2683acda..af821f5d7 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java @@ -26,7 +26,6 @@ import com.gargoylesoftware.htmlunit.html.HtmlPage; import com.gargoylesoftware.htmlunit.html.HtmlTable; -import com.gargoylesoftware.htmlunit.html.HtmlTextArea; import hudson.util.VersionNumber; import jenkins.model.Jenkins; import org.hamcrest.Matchers;