From 2a34cdafe650e028b4b17a53508d8faad884f561 Mon Sep 17 00:00:00 2001 From: dty Date: Sat, 12 Sep 2009 08:13:00 +0000 Subject: [PATCH] [HUDSON-3677] First pass at providing read-only access to project configuration. Defined a new permission, ExtendedRead, which can be enabled by specifying true for the system property hudson.security.ExtendedReadPermission. Granting job configuration permissions implies this permission. main/core/src/main/java/model/Item.java Added plumbing for dynamic enabling and disabling of permissions at runtime. This will be the basis for future UI based toggling of the availability of a permission. main/core/src/main/java/hudson/security/Permission.java Add the enabled state of a permission to the decision about whether a principal has it or not. Only show a permission when it's enabled. main/core/src/main/java/hudson/security/GlobalMatrixAuthorizationStrategy.java Allow access to config.xml when ExtendedRead is granted. main/core/src/main/java/hudson/model/Job.java Don't show the save button if the user only has ExtendedRead. main/core/src/main/resources/hudson/model/Job/configure.jelly Change the text of the Configure link in the side panel based on whether a user has Configure or ExtendedRead permissions. main/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@21626 71c3de6d-444a-0410-be80-ed276b4c234a --- core/src/main/java/hudson/model/Item.java | 3 +- core/src/main/java/hudson/model/Job.java | 2 +- .../GlobalMatrixAuthorizationStrategy.java | 8 +- .../main/java/hudson/security/Permission.java | 29 ++++- .../model/AbstractProject/sidepanel.jelly | 9 +- .../hudson/model/Job/configure.jelly | 12 +- .../hudson/model/Messages.properties | 4 + .../security/ExtendedReadPermissionTest.java | 105 ++++++++++++++++++ .../security/ExtendedReadPermissionTest.zip | Bin 0 -> 4200 bytes 9 files changed, 158 insertions(+), 14 deletions(-) create mode 100644 test/src/test/java/hudson/security/ExtendedReadPermissionTest.java create mode 100644 test/src/test/resources/hudson/security/ExtendedReadPermissionTest.zip diff --git a/core/src/main/java/hudson/model/Item.java b/core/src/main/java/hudson/model/Item.java index 39ec2328d1..0c4e615987 100644 --- a/core/src/main/java/hudson/model/Item.java +++ b/core/src/main/java/hudson/model/Item.java @@ -1,7 +1,7 @@ /* * The MIT License * - * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi + * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Yahoo! Inc. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -196,6 +196,7 @@ public interface Item extends PersistenceRoot, SearchableModelObject, AccessCont public static final Permission DELETE = new Permission(PERMISSIONS,"Delete", Permission.DELETE); public static final Permission CONFIGURE = new Permission(PERMISSIONS,"Configure", Permission.CONFIGURE); public static final Permission READ = new Permission(PERMISSIONS,"Read", Permission.READ); + public static final Permission EXTENDED_READ = new Permission(PERMISSIONS,"ExtendedRead", Messages._AbstractProject_ExtendedReadPermission_Description(), CONFIGURE, Boolean.getBoolean("hudson.security.ExtendedReadPermission")); public static final Permission BUILD = new Permission(PERMISSIONS, "Build", Messages._AbstractProject_BuildPermission_Description(), Permission.UPDATE); public static final Permission WORKSPACE = new Permission(PERMISSIONS, "Workspace", Messages._AbstractProject_WorkspacePermission_Description(), Permission.READ); } diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java index e42812b14f..60be4f8c7e 100644 --- a/core/src/main/java/hudson/model/Job.java +++ b/core/src/main/java/hudson/model/Job.java @@ -977,7 +977,7 @@ public abstract class Job, RunT extends Run set = grantedPermissions.get(p); - if(set!=null && set.contains(sid)) + if(set!=null && set.contains(sid) && p.getEnabled()) return true; } return false; @@ -155,7 +155,7 @@ public class GlobalMatrixAuthorizationStrategy extends AuthorizationStrategy { */ public boolean hasExplicitPermission(String sid, Permission p) { Set set = grantedPermissions.get(p); - return set != null && set.contains(sid); + return set != null && set.contains(sid) && p.getEnabled(); } /** @@ -266,7 +266,7 @@ public class GlobalMatrixAuthorizationStrategy extends AuthorizationStrategy { } public boolean showPermission(Permission p) { - return true; + return p.getEnabled(); } public FormValidation doCheckName(@QueryParameter String value ) throws IOException, ServletException { diff --git a/core/src/main/java/hudson/security/Permission.java b/core/src/main/java/hudson/security/Permission.java index 5d3eaf3f56..8050a26428 100644 --- a/core/src/main/java/hudson/security/Permission.java +++ b/core/src/main/java/hudson/security/Permission.java @@ -1,7 +1,7 @@ /* * The MIT License * - * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi + * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Yahoo! Inc. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -82,6 +82,18 @@ public final class Permission { */ public final Permission impliedBy; + /** + * Whether this permission is available for use. + * + *

+ * This allows us to dynamically enable or disable the visibility of + * permissions, so administrators can control the complexitity of their + * permission matrix. + * + * @since 1.325 + */ + public boolean enabled; + /** * Defines a new permission. * @@ -106,7 +118,7 @@ public final class Permission { * @param impliedBy * See {@link #impliedBy}. */ - public Permission(PermissionGroup group, String name, Localizable description, Permission impliedBy) { + public Permission(PermissionGroup group, String name, Localizable description, Permission impliedBy, boolean enable) { if(!JSONUtils.isJavaIdentifier(name)) throw new IllegalArgumentException(name+" is not a Java identifier"); this.owner = group.owner; @@ -114,11 +126,16 @@ public final class Permission { this.name = name; this.description = description; this.impliedBy = impliedBy; + this.enabled = enable; group.add(this); ALL.add(this); } + public Permission(PermissionGroup group, String name, Localizable description, Permission impliedBy) { + this(group, name, description, impliedBy, true); + } + /** * @deprecated since 1.257. * Use {@link #Permission(PermissionGroup, String, Localizable, Permission)} @@ -171,6 +188,14 @@ public final class Permission { return "Permission["+owner+','+name+']'; } + public void setEnabled(boolean enable) { + enabled = enable; + } + + public boolean getEnabled() { + return enabled; + } + /** * Returns all the {@link Permission}s available in the system. * @return diff --git a/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly b/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly index c80319575d..4cf0be80c6 100644 --- a/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly +++ b/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly @@ -54,7 +54,14 @@ THE SOFTWARE. - + + + + + + + + diff --git a/core/src/main/resources/hudson/model/Job/configure.jelly b/core/src/main/resources/hudson/model/Job/configure.jelly index d040c8d843..c5ee2464dd 100644 --- a/core/src/main/resources/hudson/model/Job/configure.jelly +++ b/core/src/main/resources/hudson/model/Job/configure.jelly @@ -26,7 +26,7 @@ THE SOFTWARE. Config page. derived class specific entries should go to configure-entries.jsp --> - + @@ -67,10 +67,12 @@ THE SOFTWARE. - - - - + + + + + + diff --git a/core/src/main/resources/hudson/model/Messages.properties b/core/src/main/resources/hudson/model/Messages.properties index e8aa4c0bf0..05b4e947da 100644 --- a/core/src/main/resources/hudson/model/Messages.properties +++ b/core/src/main/resources/hudson/model/Messages.properties @@ -43,6 +43,10 @@ AbstractProject.WorkspacePermission.Description=\ This permission grants the ability to retrieve the contents of a workspace \ Hudson checked out for performing builds. If you don''t want an user to access \ the source code, you can do so by revoking this permission. +AbstractProject.ExtendedReadPermission.Description=\ + This permission grants read-only access to project configurations. Please be \ + aware that sensitive information in your builds, such as passwords, will be \ + exposed to a wider audience by granting this permission. Api.MultipleMatch=XPath "{0}" matched {1} nodes. \ Create XPath that only matches one, or use the "wrapper" query parameter to wrap them all under a root element. diff --git a/test/src/test/java/hudson/security/ExtendedReadPermissionTest.java b/test/src/test/java/hudson/security/ExtendedReadPermissionTest.java new file mode 100644 index 0000000000..848b159045 --- /dev/null +++ b/test/src/test/java/hudson/security/ExtendedReadPermissionTest.java @@ -0,0 +1,105 @@ +package hudson.security; + +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.html.HtmlButton; +import com.gargoylesoftware.htmlunit.html.HtmlForm; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import hudson.model.Item; +import org.jvnet.hudson.test.HudsonTestCase; +import org.jvnet.hudson.test.recipes.LocalData; + +/** + * + * @author dty + */ +public class ExtendedReadPermissionTest extends HudsonTestCase { + /** + * alice: Job/Configure+Read + * bob: Job/Read + * charlie: Job/ExtendedRead+Read + */ + + private void setPermissionEnabled(boolean enabled) throws Exception { + Item.EXTENDED_READ.setEnabled(enabled); + } + + + @LocalData + public void testReadOnlyConfigAccessWithPermissionEnabled() throws Exception { + setPermissionEnabled(true); + + AuthorizationStrategy as = hudson.getAuthorizationStrategy(); + assertTrue("Expecting GlobalMatrixAuthorizationStrategy", (as instanceof GlobalMatrixAuthorizationStrategy)); + GlobalMatrixAuthorizationStrategy gas = (GlobalMatrixAuthorizationStrategy)as; + assertTrue("Charlie should have extended read for this test", gas.hasExplicitPermission("charlie",Item.EXTENDED_READ)); + + WebClient wc = new WebClient().login("charlie","charlie"); + HtmlPage page = wc.goTo("job/a/configure"); + HtmlForm form = page.getFormByName("config"); + HtmlButton saveButton = getButtonByCaption(form,"Save"); + assertNull(saveButton); + } + + @LocalData + public void testReadOnlyConfigAccessWithPermissionDisabled() throws Exception { + setPermissionEnabled(false); + + AuthorizationStrategy as = hudson.getAuthorizationStrategy(); + assertTrue("Expecting GlobalMatrixAuthorizationStrategy", (as instanceof GlobalMatrixAuthorizationStrategy)); + GlobalMatrixAuthorizationStrategy gas = (GlobalMatrixAuthorizationStrategy)as; + assertFalse("Charlie should not have extended read for this test", gas.hasExplicitPermission("charlie",Item.EXTENDED_READ)); + + WebClient wc = new WebClient().login("charlie","charlie"); + try { + HtmlPage page = wc.goTo("job/a/configure"); + } + catch (FailingHttpStatusCodeException e) { + assertEquals(403,e.getStatusCode()); + return; + } + + fail("Charlie should not have been able to access the configuration page"); + } + + @LocalData + public void testNoConfigAccessWithPermissionEnabled() throws Exception { + setPermissionEnabled(true); + + AuthorizationStrategy as = hudson.getAuthorizationStrategy(); + assertTrue("Expecting GlobalMatrixAuthorizationStrategy", (as instanceof GlobalMatrixAuthorizationStrategy)); + GlobalMatrixAuthorizationStrategy gas = (GlobalMatrixAuthorizationStrategy)as; + assertFalse("Bob should not have extended read for this test", gas.hasExplicitPermission("bob",Item.EXTENDED_READ)); + + WebClient wc = new WebClient().login("bob","bob"); + try { + HtmlPage page = wc.goTo("job/a/configure"); + } + catch (FailingHttpStatusCodeException e) { + assertEquals(403,e.getStatusCode()); + return; + } + + fail("Bob should not have been able to access the configuration page"); + } + +/* + @LocalData + public void testConfigureLink() throws Exception { + + } + + @LocalData + public void testViewConfigurationLink() throws Exception { + + } + + @LocalData + public void testMatrixWithPermissionEnabled() throws Exception { + } + + @LocalData + public void testMatrixWithPermissionDisabled() throws Exception { + + } + */ +} diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest.zip b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest.zip new file mode 100644 index 0000000000000000000000000000000000000000..2084f20e15f5fb23935a54fc06b63c2f826b2713 GIT binary patch literal 4200 zcmbVPc{r5o8~zw0+1F&5>`NT`wAd1k$ZqWG7>q59W$a5s_M@^ZBKz7QDl%C{g;26p z_7p!-mX0-)exIX4otAU1xn{0!{&?>9exLh(pZETBH6SF60058yFG7W7mr`g7IEVp& zf)oJA0Vco}<7V&VAnNDpVrZ}*fUFFZsuzxwsvG)|00zo-K!L$?%%C`J$W%qNA1aWO z+9k!v&O)WwT!1Q8ZOE5_qp8eRYAEM=W5|UhgP2poihR7CzN^8j=f>|Hi?SdI?1~hY zhQ-}=q{V46jZ0=wAL3}BX{2&fqrp(vC-c^G61*i@B9JY}s5WjZe&1D^rlQQrKxfoJ z%e-!~^LejiqFa*6y5k4N9pYF`PILY7eg$wsr5v@3)0?v7pNJ(g9j=)CODyhv-qHb5 z`m};tdHOO6LhZYYkbM|gS*4Uyra1~>KHuWNK! zNO(r*ZA|O;231EanKJo6jW`@i<5Pbxf7*siJ4h{7I9GS0$0S;SxlBp4*oybnoY8t{ zB$cCpjreR7*TU5k0TF8H5Dz(RZ}l>vu+h?&*s^J;Y-mTxiS>8^M_H>hdI!!~#I*DM z@*+)oELE7bL2%#^tV)_$ZN~Mw0jJ4ZP9EHdkB-;$V7|gZG+uoEC0WJxL2&rF`}j+h zaz}1iY|hUg$x9kl&;`9k4tfg!&IJm~{{8Uj2MT)28Drxmwr#7bj42OdX+DseMnJc4UJ|FHMsdYwKrnmCOw$X>J3G@ghu^3s63*=LVIAzq5P!dqIUC38e3hkh{i>j`7_hlt&LlRZKf)y!VWB2?OgFBS2XmZRb$sG~63%&@ zQvFFqIL=h1{N^(q3Z*WLx;pxJE2;o3RZo+9(!xC4{kj3Avlu){d&R_!qwB|6wHB$; zUb9533OBE>b3EVb_D;|8`lUpqz@D81zgrW)3vKI(#)_Un`+t#Yce9{Pp+p365)wj6 zxPT;e#R^A`ue@knYm&V-Pdgk910?+EDsrAX4M*obwTX2@z%qt8cGSK6l`BZW^}M~% zo}X;N1&Z=(wxIq}XzPgbba6ueiaTJxKdW^BoxuPBfP>!v{KO7>Ix~A9Q9tkF0dAPE;E)rk2H`^@f?w znSho zELA|&q1%LX&K0N{kF7~be_c^4oOgorR$#Q&kWad;OeVV76zRdI4(XJKJ*Y(lD{pIE zrQWmP_&7a)YnnRk?PhN<@{$pDcr(qoclrVd=j-_F#I<(sH%t8QqkV#353gIA4dt^BH>*pT0FQl#?xU8RvRd$(fh<*9;H#DMoB0K`KT@ zF-K7a)T>XFnn2OG@rDh8jtXK&PE)kcY(3m5YU49WF+M zo16>t#_q+po9#{`=j5kGvwE|veI8!&1VkMZqS8b8(~12NwCQIf2iHIN&p6$1WHcc5 zgCmkT|2!Dj#Pa*4ZW7`T7Byp8%z4CM4?iNxSo*Ormw`i&PDd=MxG7$qd7n=8D&?O+%ih zA!8D8Av&tr$dZPf%{9S#VWQK`P6OF0#udlq$LL2x@D(`IYM6pz%wdMKsbuXpVM)*f zcN}X_8F+ur`!^)2U3hL4x@iuVTuleuUJqF*X(nB=nlYp1y1-Ht64tvvOQr!c7&*?C z3X@$>D6gSD{~n2+uekqGBfEeEhiN@?89dK8&0H^~<76J~!#wOB)eg8sZi*$M6Mf!i z1KBDy%pmm|?~dte5D_y&zV~WD1;z^i0M96pAG-ClTQBJLi-w^e0BnPNFGFsF001p8 zt7rkfwR}pHTlHY3`&s=yV0V4;=lZQ=`3=HOKH8fpcW|wKmwKr?+#v&#G z_9U-w2yM6I+W@tDOP(Oe=QsrdY{jXc^@Ms)Ps+Xk?8K^XnCxt4Z=l+}otbZde%PUZ z)|+wqg?Y*XUvufD`@*g3N{r0sKL(~cmtbKZvFc|6Eyy0 literal 0 HcmV?d00001