Merge pull request #13 from jenkinsci-cert/SECURITY-180

[SECURITY-180] arbitrary API Token change/leak via changeToken

core/src/main/java/jenkins/security/ApiTokenProperty.java

@@ -81,6 +81,7 @@ public class ApiTokenProperty extends UserProperty {
     }
 
     public void changeApiToken() throws IOException {
+        user.checkPermission(Jenkins.ADMINISTER);
         _changeApiToken();
         if (user!=null)
             user.save();

test/src/test/java/hudson/model/UserTest.java

@@ -28,19 +28,27 @@ import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
 import com.gargoylesoftware.htmlunit.WebAssert;
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+
 import hudson.security.AccessDeniedException2;
 import hudson.security.GlobalMatrixAuthorizationStrategy;
 import hudson.security.HudsonPrivateSecurityRealm;
 import hudson.security.Permission;
 import hudson.tasks.MailAddressResolver;
+
 import java.io.IOException;
 import java.io.PrintStream;
 import java.util.Collections;
+
 import jenkins.model.Jenkins;
+import jenkins.security.ApiTokenProperty;
+
+import org.acegisecurity.AccessDeniedException;
 import org.acegisecurity.Authentication;
 import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
+
 import static org.junit.Assert.*;
+
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Bug;
@@ -419,6 +427,42 @@ public class UserTest {
         assertTrue("User should be able to delete.", user.canDelete());
     }
 
+    @Test
+    // @Issue("SECURITY-180")
+    public void security180() throws Exception {
+        final GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy();
+        j.jenkins.setAuthorizationStrategy(auth);
+        j.jenkins.setSecurityRealm(new HudsonPrivateSecurityRealm(false));
+
+        User alice = User.get("alice");
+        User bob = User.get("bob");
+        User anonymous = User.get("anonymous");
+        User admin = User.get("admin");
+
+        auth.add(Jenkins.READ, alice.getId());
+        auth.add(Jenkins.READ, bob.getId());
+        auth.add(Jenkins.ADMINISTER, admin.getId());
+
+        SecurityContextHolder.getContext().setAuthentication(admin.impersonate());
+        // Change token by admin
+        admin.getProperty(ApiTokenProperty.class).changeApiToken();
+        alice.getProperty(ApiTokenProperty.class).changeApiToken();
+
+        SecurityContextHolder.getContext().setAuthentication(bob.impersonate());
+        // Change own token
+        bob.getProperty(ApiTokenProperty.class).changeApiToken();
+
+        try {
+            alice.getProperty(ApiTokenProperty.class).changeApiToken();
+            fail("Bob should not be authorized to change alice's token");
+        } catch (AccessDeniedException expected) { }
+
+        try {
+            anonymous.getProperty(ApiTokenProperty.class).changeApiToken();
+            fail("Anonymous should not be authorized to change alice's token");
+        } catch (AccessDeniedException expected) { }
+    }
+
     @Test
     public void testGetDynamic() {