[JENKINS-58703]

jenkinsci · Aug 1, 2019 · 80515b9 · 80515b9
1 parent 2115018
commit 80515b9
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 5 deletions.
diff --git a/...main/java/com/cloudbees/hudson/plugins/folder/properties/AuthorizationMatrixProperty.java b/...main/java/com/cloudbees/hudson/plugins/folder/properties/AuthorizationMatrixProperty.java
@@ -196,7 +196,7 @@ public InheritanceStrategy getInheritanceStrategy() {
 
     /**
      * Persist {@link ProjectMatrixAuthorizationStrategy} as a list of IDs that
-     * represent {@link ProjectMatrixAuthorizationStrategy#grantedPermissions}.
+     * represent ProjectMatrixAuthorizationStrategy#grantedPermissions.
      */
     @Restricted(DoNotUse.class)
     public static final class ConverterImpl extends AbstractAuthorizationPropertyConverter<AuthorizationMatrixProperty> {
@@ -226,7 +226,8 @@ public void onCreated(Item item) {
                 if (item instanceof AbstractFolder) {
                     AbstractFolder<?> folder = (AbstractFolder<?>) item;
                     AuthorizationMatrixProperty prop = folder.getProperties().get(AuthorizationMatrixProperty.class);
-                    if (prop == null) {
+                    boolean propIsNew = prop == null;
+                    if (propIsNew) {
                         prop = new AuthorizationMatrixProperty();
                     }
 
@@ -241,7 +242,11 @@ public void onCreated(Item item) {
                     }
                     if (prop.getGrantedPermissions().size() > 0) {
                         try {
-                            folder.addProperty(prop);
+                            if (propIsNew) {
+                                folder.addProperty(prop);
+                            } else {
+                                folder.save();
+                            }
                         } catch (IOException ex) {
                             LOGGER.log(Level.WARNING, "Failed to grant creator permissions on folder " + item.getFullName(), ex);
                         }

diff --git a/src/main/java/hudson/security/AuthorizationMatrixProperty.java b/src/main/java/hudson/security/AuthorizationMatrixProperty.java
@@ -244,7 +244,8 @@ public void onCreated(Item item) {
                 if (item instanceof Job) {
                     Job<?, ?> job = (Job<?, ?>) item;
                     AuthorizationMatrixProperty prop = job.getProperty(AuthorizationMatrixProperty.class);
-                    if (prop == null) {
+                    boolean propIsNew = prop == null;
+                    if (propIsNew) {
                         prop = new AuthorizationMatrixProperty();
                     }
 
@@ -259,7 +260,11 @@ public void onCreated(Item item) {
                     }
                     if (prop.getGrantedPermissions().size() > 0) {
                         try {
-                            job.addProperty(prop);
+                            if (propIsNew) {
+                                job.addProperty(prop);
+                            } else {
+                                job.save();
+                            }
                         } catch (IOException ex) {
                             LOGGER.log(Level.WARNING, "Failed to grant creator permissions on job " + item.getFullName(), ex);
                         }

diff --git a/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java b/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java
@@ -49,6 +49,34 @@ public void ensureCreatorHasPermissions() throws Exception {
         Assert.assertTrue(job.getACL().hasPermission(User.get("alice", false, Collections.emptyMap()).impersonate(), Item.CONFIGURE));
     }
 
+    @Test
+    @Issue("JENKINS-58703")
+    public void ensureNoJobPropertyDuplication() throws Exception {
+        HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null);
+        realm.createAccount("alice","alice");
+        realm.createAccount("bob","bob");
+        r.jenkins.setSecurityRealm(realm);
+
+        ProjectMatrixAuthorizationStrategy authorizationStrategy = new ProjectMatrixAuthorizationStrategy();
+        authorizationStrategy.add(Item.CREATE, "alice");
+        authorizationStrategy.add(Jenkins.READ, "alice");
+        authorizationStrategy.add(Jenkins.READ, "bob");
+        r.jenkins.setAuthorizationStrategy(authorizationStrategy);
+
+        Job<?, ?> job;
+        try (ACLContext ignored = ACL.as(User.get("alice", false, Collections.emptyMap()))) {
+            r.jenkins.createProjectFromXML("job", getClass().getResourceAsStream(getClass().getSimpleName() + "/JENKINS-58703.xml"));
+            job = r.jenkins.getItem("job", r.jenkins, Job.class);
+        }
+
+        Assert.assertNotNull(job.getProperty(AuthorizationMatrixProperty.class));
+        Assert.assertTrue(job.getACL().hasPermission(User.get("alice", false, Collections.emptyMap()).impersonate(), Item.READ));
+        Assert.assertTrue(job.getACL().hasPermission(User.get("bob", false, Collections.emptyMap()).impersonate(), Item.READ));
+        Assert.assertTrue(job.getACL().hasPermission(User.get("alice", false, Collections.emptyMap()).impersonate(), Item.CONFIGURE));
+
+        Assert.assertEquals("one property", 1, job.getAllProperties().size());
+    }
+
     @Test
     public void submitEmptyPropertyEnsuresPermissionsForSubmitter() throws Exception {
         HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null);

diff --git a/src/test/resources/hudson/security/ProjectMatrixAuthorizationStrategyTest/JENKINS-58703.xml b/src/test/resources/hudson/security/ProjectMatrixAuthorizationStrategyTest/JENKINS-58703.xml
@@ -0,0 +1,21 @@
+<?xml version='1.1' encoding='UTF-8'?>
+<project>
+    <description>unused</description>
+    <keepDependencies>false</keepDependencies>
+    <properties>
+        <hudson.security.AuthorizationMatrixProperty>
+            <inheritanceStrategy class="org.jenkinsci.plugins.matrixauth.inheritance.NonInheritingStrategy"/>
+            <permission>hudson.model.Item.Read:authenticated</permission>
+        </hudson.security.AuthorizationMatrixProperty>
+    </properties>
+    <scm class="hudson.scm.NullSCM"/>
+    <canRoam>true</canRoam>
+    <disabled>false</disabled>
+    <blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
+    <blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
+    <triggers/>
+    <concurrentBuild>false</concurrentBuild>
+    <builders/>
+    <publishers/>
+    <buildWrappers/>
+</project>