diff --git a/src/main/java/com/cloudbees/hudson/plugins/folder/properties/AuthorizationMatrixProperty.java b/src/main/java/com/cloudbees/hudson/plugins/folder/properties/AuthorizationMatrixProperty.java index 7b0f9332..54b3f72f 100644 --- a/src/main/java/com/cloudbees/hudson/plugins/folder/properties/AuthorizationMatrixProperty.java +++ b/src/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 { @@ -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 index 00b8f8e6..51168bcc 100644 --- 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 index da838b27..e655bb75 100644 --- 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 new file mode 100644 index 00000000..1b1604be --- /dev/null +++ b/src/test/resources/hudson/security/ProjectMatrixAuthorizationStrategyTest/JENKINS-58703.xml @@ -0,0 +1,21 @@ + + + unused + false + + + + hudson.model.Item.Read:authenticated + + + + true + false + false + false + + false + + + + \ No newline at end of file