[SECURITY-514] Restrict access to user properties via the api to admins

jenkinsci · Sep 7, 2017 · 7b1f8e9 · 7b1f8e9
1 parent 61781a6
commit 7b1f8e9
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 1 deletion.
diff --git a/core/src/main/java/hudson/model/User.java b/core/src/main/java/hudson/model/User.java
@@ -289,7 +289,11 @@ public synchronized void addProperty(@Nonnull UserProperty p) throws IOException
      */
     @Exported(name="property",inline=true)
     public List<UserProperty> getAllProperties() {
-        return Collections.unmodifiableList(properties);
+        if (hasPermission(Jenkins.ADMINISTER)) {
+            return Collections.unmodifiableList(properties);
+        }
+
+        return Collections.emptyList();
     }
 
     /**

diff --git a/test/src/test/java/hudson/model/UserTest.java b/test/src/test/java/hudson/model/UserTest.java
@@ -29,6 +29,7 @@
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 
+import hudson.security.ACL;
 import hudson.security.AbstractPasswordBasedSecurityRealm;
 import hudson.security.AccessDeniedException2;
 import hudson.security.GlobalMatrixAuthorizationStrategy;
@@ -57,15 +58,19 @@
 import org.acegisecurity.userdetails.UserDetails;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
 
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.collection.IsEmptyCollection.empty;
 import static org.junit.Assert.*;
 import static org.junit.Assume.*;
 
+import org.hamcrest.collection.IsEmptyCollection;
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.FakeChangeLogSCM;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
 import org.jvnet.hudson.test.TestExtension;
 import org.jvnet.hudson.test.recipes.LocalData;
 
@@ -681,6 +686,31 @@ public void resolveById() throws Exception {
         assertSame("'user2' should resolve to u2", u2, u);
     }
 
+    @Test
+    @Issue("SECURITY-514")
+    public void getAllPropertiesRequiresAdmin() {
+        j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
+                .grant(Jenkins.ADMINISTER).everywhere().to("admin)")
+                .grant(Jenkins.READ).everywhere().toEveryone());
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+
+        User admin = User.get("admin");
+        User alice = User.get("alice");
+        User bob = User.get("bob");
+
+        // Admin can access user properties for all users
+        ACL.impersonate(admin.impersonate());
+        assertThat(alice.getAllProperties(), not(empty()));
+        assertThat(bob.getAllProperties(), not(empty()));
+        assertThat(admin.getAllProperties(), not(empty()));
+
+        // Non admins can only view their own
+        ACL.impersonate(alice.impersonate());
+        assertThat(alice.getAllProperties(), not(empty()));
+        assertThat(bob.getAllProperties(), empty());
+        assertThat(admin.getAllProperties(), empty());
+    }
+
      public static class SomeUserProperty extends UserProperty {
 
         @TestExtension