From ffbede32a67d193abb3f9a6593daf45a2504e60a Mon Sep 17 00:00:00 2001 From: Damien Coraboeuf Date: Wed, 14 Jan 2015 13:35:47 +0100 Subject: [PATCH 1/5] Configurable anonymous access to the list of failure causes --- .../jenkins/plugins/bfa/CauseManagement.java | 5 +- .../jenkins/plugins/bfa/PluginImpl.java | 7 +++ .../plugins/bfa/CauseManagement/index.groovy | 58 +++++++++++-------- .../jenkins/plugins/bfa/Messages.properties | 2 + 4 files changed, 47 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java b/src/main/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java index 8b87aed0..270bd35d 100644 --- a/src/main/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java +++ b/src/main/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java @@ -115,7 +115,8 @@ public class CauseManagement extends BfaGraphAction { @Override public String getIconFileName() { - if (Hudson.getInstance().hasPermission(PluginImpl.UPDATE_PERMISSION)) { + if (Hudson.getInstance().hasPermission(PluginImpl.UPDATE_PERMISSION) || + Hudson.getInstance().hasPermission(PluginImpl.VIEW_PERMISSION)) { return PluginImpl.getDefaultIcon(); } else { return null; @@ -126,6 +127,8 @@ public String getIconFileName() { public String getDisplayName() { if (Hudson.getInstance().hasPermission(PluginImpl.UPDATE_PERMISSION)) { return Messages.CauseManagement_DisplayName(); + } else if (Hudson.getInstance().hasPermission(PluginImpl.VIEW_PERMISSION)) { + return Messages.CauseList_DisplayName(); } else { return null; } diff --git a/src/main/java/com/sonyericsson/jenkins/plugins/bfa/PluginImpl.java b/src/main/java/com/sonyericsson/jenkins/plugins/bfa/PluginImpl.java index befea244..1ca16602 100644 --- a/src/main/java/com/sonyericsson/jenkins/plugins/bfa/PluginImpl.java +++ b/src/main/java/com/sonyericsson/jenkins/plugins/bfa/PluginImpl.java @@ -86,6 +86,13 @@ public class PluginImpl extends Plugin { new Permission(PERMISSION_GROUP, "UpdateCauses", Messages._PermissionUpdate_Description(), Hudson.ADMINISTER); + /** + * Permission to view the causes. E.e. Access {@link CauseManagement}. + */ + public static final Permission VIEW_PERMISSION = + new Permission(PERMISSION_GROUP, "ViewCauses", + Messages._PermissionView_Description(), UPDATE_PERMISSION); + /** * Permission to remove causes. */ diff --git a/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/CauseManagement/index.groovy b/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/CauseManagement/index.groovy index 8018c51b..767ef9d6 100644 --- a/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/CauseManagement/index.groovy +++ b/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/CauseManagement/index.groovy @@ -32,7 +32,7 @@ def f = namespace(lib.FormTagLib) def l = namespace(lib.LayoutTagLib) def j = namespace(lib.JenkinsTagLib) -l.layout(permission: PluginImpl.UPDATE_PERMISSION) { +l.layout() { l.header(title: _("Failure Cause Management - Confirm Remove")) def management = CauseManagement.getInstance(); @@ -55,7 +55,11 @@ l.layout(permission: PluginImpl.UPDATE_PERMISSION) { + "z-index: -100;" + "background-image: url(\'" + bgImageUrl + "');") {} - h1(_("Update Failure Causes")) + if (h.hasPermission(PluginImpl.UPDATE_PERMISSION)) { + h1(_("Update Failure Causes")) + } else { + h1(_("List of Failure Causes")) + } def shallowCauses = management.getShallowCauses() if (management.isError(request)) { @@ -73,28 +77,30 @@ l.layout(permission: PluginImpl.UPDATE_PERMISSION) { } //The New Cause link - div(style: "margin-top: 10px; margin-bottom: 10px; width: 90%;") { - a(style: "font-weight: bold; " - + "font-size: larger; " - + "padding-left: 30px; " - + "min-height: 30px; " - + "padding-top: 5px; " - + "padding-bottom: 5px; " - + "background-image: url( \'" + newImageUrl + "\'); " - + "background-position: left center; " - + "background-repeat: no-repeat;", - href: "new", - alt: _("New")) {text(_("Create new"))} + if (h.hasPermission(PluginImpl.UPDATE_PERMISSION)) { + div(style: "margin-top: 10px; margin-bottom: 10px; width: 90%;") { + a(style: "font-weight: bold; " + + "font-size: larger; " + + "padding-left: 30px; " + + "min-height: 30px; " + + "padding-top: 5px; " + + "padding-bottom: 5px; " + + "background-image: url( \'" + newImageUrl + "\'); " + + "background-position: left center; " + + "background-repeat: no-repeat;", + href: "new", + alt: _("New")) { text(_("Create new")) } - if (PluginImpl.getInstance().isGraphsEnabled()) { - a(style: "font-weight: bold; " - + "font-size: larger; " - + "padding-top: 5px; " - + "padding-bottom: 5px; " - + "float: right;", - href: "detailedgraphs", - alt: _("Graphs/statistics")) {text(_("Graphs/Statistics"))} - } + if (PluginImpl.getInstance().isGraphsEnabled()) { + a(style: "font-weight: bold; " + + "font-size: larger; " + + "padding-top: 5px; " + + "padding-bottom: 5px; " + + "float: right;", + href: "detailedgraphs", + alt: _("Graphs/statistics")) { text(_("Graphs/Statistics")) } + } + } } //One time check so we don't do it for every iteration below @@ -118,7 +124,11 @@ l.layout(permission: PluginImpl.UPDATE_PERMISSION) { shallowCauses.each{ cause -> tr { td{ - a(href: cause.getId()){ text(cause.getName())} + if (h.hasPermission(PluginImpl.UPDATE_PERMISSION)) { + a(href: cause.getId()) { text(cause.getName()) } + } else { + text(cause.getName()) + } } td{ text(cause.getCategoriesAsString()) diff --git a/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/Messages.properties b/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/Messages.properties index c931640f..0d525776 100644 --- a/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/Messages.properties +++ b/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/Messages.properties @@ -1,10 +1,12 @@ #I18n messages. PermissionGroup_Title=Build Failure Analyzer +PermissionView_Description=View Failure causes. PermissionUpdate_Description=Add and Update Failure causes. PermissionRemove_Description=Remove Failure causes. BuildLogIndication_DisplayName=Build Log Indication MultilineBuildLogIndication_DisplayName=Multi-Line Build Log Indication CauseManagement_DisplayName=Failure Cause Management +CauseList_DisplayName=Failure Causes ScannerJobProperty_DisplayName=Do not Scan failed builds LocalFileKnowledgeBase_DisplayName=Jenkins Local MongoDBKnowledgeBase_DisplayName=Mongo DB From 5f3e14ed2e17665347aa64224976a3071093ff8f Mon Sep 17 00:00:00 2001 From: Damien Coraboeuf Date: Wed, 14 Jan 2015 14:22:05 +0100 Subject: [PATCH 2/5] Correction of checkstyle issue --- .../com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java b/src/main/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java index 270bd35d..33dd2843 100644 --- a/src/main/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java +++ b/src/main/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagement.java @@ -115,8 +115,8 @@ public class CauseManagement extends BfaGraphAction { @Override public String getIconFileName() { - if (Hudson.getInstance().hasPermission(PluginImpl.UPDATE_PERMISSION) || - Hudson.getInstance().hasPermission(PluginImpl.VIEW_PERMISSION)) { + if (Hudson.getInstance().hasPermission(PluginImpl.UPDATE_PERMISSION) + || Hudson.getInstance().hasPermission(PluginImpl.VIEW_PERMISSION)) { return PluginImpl.getDefaultIcon(); } else { return null; From 03f189408050bbc2d32aac376fa3622093a725d6 Mon Sep 17 00:00:00 2001 From: Damien Coraboeuf Date: Wed, 14 Jan 2015 16:18:34 +0100 Subject: [PATCH 3/5] Adding the view permission as a safety --- .../jenkins/plugins/bfa/CauseManagement/index.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/CauseManagement/index.groovy b/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/CauseManagement/index.groovy index 767ef9d6..8abc9d62 100644 --- a/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/CauseManagement/index.groovy +++ b/src/main/resources/com/sonyericsson/jenkins/plugins/bfa/CauseManagement/index.groovy @@ -32,7 +32,7 @@ def f = namespace(lib.FormTagLib) def l = namespace(lib.LayoutTagLib) def j = namespace(lib.JenkinsTagLib) -l.layout() { +l.layout(permission: PluginImpl.VIEW_PERMISSION) { l.header(title: _("Failure Cause Management - Confirm Remove")) def management = CauseManagement.getInstance(); From 46866483b9fd7e5c3ed573f99fa2021292860d54 Mon Sep 17 00:00:00 2001 From: Damien Coraboeuf Date: Wed, 14 Jan 2015 19:35:01 +0100 Subject: [PATCH 4/5] Creating unit tests for pull request #31 (work in progress) --- .../bfa/CauseManagementPermissionTest.java | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/test/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagementPermissionTest.java diff --git a/src/test/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagementPermissionTest.java b/src/test/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagementPermissionTest.java new file mode 100644 index 00000000..4d4ed6ea --- /dev/null +++ b/src/test/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagementPermissionTest.java @@ -0,0 +1,61 @@ +package com.sonyericsson.jenkins.plugins.bfa; + +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import hudson.model.Hudson; +import hudson.security.GlobalMatrixAuthorizationStrategy; +import hudson.security.SecurityRealm; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +/** + * Tests the permissions for the Cause Management. + * + * @author Damien Coraboeuf + */ +public class CauseManagementPermissionTest { + + /** + * The Jenkins Rule. + */ + @Rule + //CS IGNORE VisibilityModifier FOR NEXT 1 LINES. REASON: Jenkins Rule + public JenkinsRule j = new JenkinsRule(); + + @Before + public void jenkinsConfiguration() { + SecurityRealm securityRealm = j.createDummySecurityRealm(); + j.getInstance().setSecurityRealm(securityRealm); + + GlobalMatrixAuthorizationStrategy authorizationStrategy = new GlobalMatrixAuthorizationStrategy(); + authorizationStrategy.add(Hudson.READ, "anonymous"); + authorizationStrategy.add(PluginImpl.VIEW_PERMISSION, "view"); + authorizationStrategy.add(PluginImpl.UPDATE_PERMISSION, "update"); + authorizationStrategy.add(PluginImpl.VIEW_PERMISSION, "all"); + authorizationStrategy.add(PluginImpl.UPDATE_PERMISSION, "all"); + j.getInstance().setAuthorizationStrategy(authorizationStrategy); + } + + @Test + public void notAllowedToUpdateCausesWhenNotGrantedAnything() throws Exception { + JenkinsRule.WebClient webClient = j.createWebClient(); + // Logs in + webClient.goTo(""); + webClient.login("none"); + // Gets to the Failure Cause page + webClient.goTo("failure-cause-management"); + // FIXME Expects a failure + } + + @Test + public void allowedToUpdateCausesWhenGrantedOnlyUpdate() throws Exception { + JenkinsRule.WebClient webClient = j.createWebClient(); + // Logs in + webClient.goTo(""); + webClient.login("update"); + // Gets to the Failure Cause page + HtmlPage page = webClient.goTo("failure-cause-management"); + // FIXME Checks the "Create New" button is available + } +} From 57423130dfea35dece6e9d12cebf08038a2533d6 Mon Sep 17 00:00:00 2001 From: Damien Coraboeuf Date: Wed, 14 Jan 2015 20:00:59 +0100 Subject: [PATCH 5/5] Creating unit tests for pull request #31 --- .../bfa/CauseManagementPermissionTest.java | 76 ++++++++++++++++++- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagementPermissionTest.java b/src/test/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagementPermissionTest.java index 4d4ed6ea..94a0948c 100644 --- a/src/test/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagementPermissionTest.java +++ b/src/test/java/com/sonyericsson/jenkins/plugins/bfa/CauseManagementPermissionTest.java @@ -1,5 +1,6 @@ package com.sonyericsson.jenkins.plugins.bfa; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.html.HtmlPage; import hudson.model.Hudson; import hudson.security.GlobalMatrixAuthorizationStrategy; @@ -9,6 +10,13 @@ import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; +import javax.servlet.http.HttpServletResponse; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + /** * Tests the permissions for the Cause Management. * @@ -23,6 +31,10 @@ public class CauseManagementPermissionTest { //CS IGNORE VisibilityModifier FOR NEXT 1 LINES. REASON: Jenkins Rule public JenkinsRule j = new JenkinsRule(); + /** + * Configures Jenkins to use security and defines several users with different rights for the + * management or view of failure causes. + */ @Before public void jenkinsConfiguration() { SecurityRealm securityRealm = j.createDummySecurityRealm(); @@ -37,6 +49,11 @@ public void jenkinsConfiguration() { j.getInstance().setAuthorizationStrategy(authorizationStrategy); } + /** + * Checks that a non authorised user cannot access the failure management page at all. + * + * @throws java.lang.Exception If Jenkins cannot be accessed + */ @Test public void notAllowedToUpdateCausesWhenNotGrantedAnything() throws Exception { JenkinsRule.WebClient webClient = j.createWebClient(); @@ -44,10 +61,40 @@ public void notAllowedToUpdateCausesWhenNotGrantedAnything() throws Exception { webClient.goTo(""); webClient.login("none"); // Gets to the Failure Cause page - webClient.goTo("failure-cause-management"); - // FIXME Expects a failure + try { + webClient.goTo("failure-cause-management"); + fail("Access to the page should have failed"); + } catch (FailingHttpStatusCodeException ex) { + assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex.getStatusCode()); + } + } + + /** + * Checks that a user granted with "viewCauses" only can access the failure management page + * but not create a new failure. + * + * @throws java.lang.Exception If Jenkins cannot be accessed + */ + @Test + public void allowedToViewCausesWhenGrantedOnlyView() throws Exception { + JenkinsRule.WebClient webClient = j.createWebClient(); + // Logs in + webClient.goTo(""); + webClient.login("view"); + // Gets to the Failure Cause page + HtmlPage page = webClient.goTo("failure-cause-management"); + // Checks we are actually on the page + assertNotNull(page.selectSingleNode("//h1[.='List of Failure Causes']")); + // Checks the "Create New" button is NOT available + assertNull(page.selectSingleNode("//a[.='Create new']")); } + /** + * Checks that a user granted with "updateCauses" only can access the failure management page + * and create a new failure. + * + * @throws java.lang.Exception If Jenkins cannot be accessed + */ @Test public void allowedToUpdateCausesWhenGrantedOnlyUpdate() throws Exception { JenkinsRule.WebClient webClient = j.createWebClient(); @@ -56,6 +103,29 @@ public void allowedToUpdateCausesWhenGrantedOnlyUpdate() throws Exception { webClient.login("update"); // Gets to the Failure Cause page HtmlPage page = webClient.goTo("failure-cause-management"); - // FIXME Checks the "Create New" button is available + // Checks we are actually on the page + assertNotNull(page.selectSingleNode("//h1[.='Update Failure Causes']")); + // Checks the "Create New" button is available + assertNotNull(page.selectSingleNode("//a[.='Create new']")); + } + + /** + * Checks that a user granted with "updateCauses" and "viewCauses" only can access the failure management page + * and create a new failure. + * + * @throws java.lang.Exception If Jenkins cannot be accessed + */ + @Test + public void allowedToUpdateCausesWhenGrantedBothUpdateAndView() throws Exception { + JenkinsRule.WebClient webClient = j.createWebClient(); + // Logs in + webClient.goTo(""); + webClient.login("all"); + // Gets to the Failure Cause page + HtmlPage page = webClient.goTo("failure-cause-management"); + // Checks we are actually on the page + assertNotNull(page.selectSingleNode("//h1[.='Update Failure Causes']")); + // Checks the "Create New" button is available + assertNotNull(page.selectSingleNode("//a[.='Create new']")); } }