[FIX SECURITY-808] Prevent unauthenticated user to send out plaintext…

… credential.

src/main/java/jenkins/plugins/openstack/compute/JCloudsCloud.java

@@ -497,7 +497,7 @@ public ListBoxModel doFillCredentialIdItems(@AncestorInPath Jenkins context) {
             }
 
             List<StandardCredentials> credentials = CredentialsProvider.lookupCredentials(
-                    StandardCredentials.class, context, ACL.SYSTEM, Collections.<DomainRequirement>emptyList()
+                    StandardCredentials.class, context, ACL.SYSTEM, Collections.emptyList()
             );
             return new StandardListBoxModel()
                     .withEmptySelection()

src/main/java/jenkins/plugins/openstack/compute/auth/OpenstackCredentials.java

@@ -16,11 +16,12 @@
 public class OpenstackCredentials {
 
     public static @CheckForNull OpenstackCredential getCredential(@CheckForNull String credentialId) {
+        Jenkins.getInstance().getACL().checkPermission(Jenkins.ADMINISTER);
         if (credentialId == null) return null;
 
         List<OpenstackCredential> credentials = CredentialsProvider.lookupCredentials(
                         OpenstackCredential.class, Jenkins.getInstance(), ACL.SYSTEM,
-                        Collections.<DomainRequirement>emptyList()
+                        Collections.emptyList()
         );
         return CredentialsMatchers.firstOrNull(credentials, CredentialsMatchers.withId(credentialId));
     }

src/test/java/jenkins/plugins/openstack/compute/JCloudsCloudTest.java

@@ -3,13 +3,15 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.CALLS_REAL_METHODS;
 import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
@@ -25,7 +27,10 @@
 import com.gargoylesoftware.htmlunit.Page;
 import com.gargoylesoftware.htmlunit.html.HtmlElement;
 import com.gargoylesoftware.htmlunit.html.HtmlFormUtil;
+import hudson.ExtensionList;
 import hudson.model.Item;
+import hudson.model.UnprotectedRootAction;
+import hudson.model.User;
 import hudson.plugins.sshslaves.SSHLauncher;
 import hudson.security.ACL;
 import hudson.security.Permission;
@@ -58,14 +63,18 @@
 import jenkins.plugins.openstack.compute.JCloudsCloud.DescriptorImpl;
 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;
 import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.StaplerRequest;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 import org.openstack4j.api.OSClient;
 import org.openstack4j.openstack.compute.domain.NovaServer;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Callable;
@@ -327,6 +336,7 @@ public void doProvision() throws Exception {
         final JCloudsCloud jenkinsRead = getCloudWhereUserIsAuthorizedTo(Jenkins.READ, template);
         try {
             j.executeOnServer(new DoProvision(jenkinsRead, template));
+            fail();
         } catch (AccessDeniedException ex) {
             // Expected
         }
@@ -458,4 +468,56 @@ public AclControllingJCloudsCloud(JCloudsSlaveTemplate template, Permission auth
             };
         }
     }
+
+    @Test @Issue("SECURITY-808")
+    public void security808() throws Exception {
+        j.jenkins.setCrumbIssuer(null);
+        OpenstackCredentialv2 c = new OpenstackCredentialv2(
+                CredentialsScope.SYSTEM, "foo", "", "tenant", "username", "SHHH!"
+        );
+        OpenstackCredentials.add(c);
+        DescriptorImpl desc = j.getCloudDescriptor();
+
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        MockAuthorizationStrategy mas = new MockAuthorizationStrategy();
+        mas.grant(Jenkins.READ).everywhere().to("user");
+        mas.grant(Jenkins.ADMINISTER).everywhere().to("admin");
+        j.jenkins.setAuthorizationStrategy(mas);
+
+        final String destination = j.getURL().toExternalForm() + "security808";
+        ACL.impersonate(User.get("user").impersonate(), () -> {
+            FormValidation formValidation = desc.doTestConnection(true, c.getId(), destination, "");
+            assertEquals(0, ExtensionList.lookup(CredentialsCollectingPortal.class).get(0).reqs.size());
+            // Strange message as client does not understand the empty response
+            assertThat(formValidation.kind, equalTo(FormValidation.Kind.ERROR));
+            assertThat(formValidation.getMessage(), containsString("user is missing the Overall/Administer permission"));
+        });
+
+        ACL.impersonate(User.get("admin").impersonate(), () -> {
+            desc.doTestConnection(true, c.getId(), destination, "");
+            assertEquals(1, ExtensionList.lookup(CredentialsCollectingPortal.class).get(0).reqs.size());
+        });
+    }
+
+    @TestExtension("security808")
+    public static final class CredentialsCollectingPortal implements UnprotectedRootAction {
+
+        private List<StaplerRequest> reqs = new ArrayList<>();
+
+        @Override public String getIconFileName() {
+            return null;
+        }
+
+        @Override public String getDisplayName() {
+            return "security808";
+        }
+
+        @Override public String getUrlName() {
+            return "security808";
+        }
+
+        public void doDynamic() {
+            reqs.add(Stapler.getCurrentRequest());
+        }
+    }
 }

src/test/java/jenkins/plugins/openstack/compute/ProvisioningTest.java

@@ -425,8 +425,7 @@ public void timeoutLaunching() throws Exception {
         final SlaveOptions opts = j.defaultSlaveOptions().getBuilder().startTimeout(1000).build();
         final JCloudsCloud cloud = j.configureSlaveProvisioning(j.dummyCloud(opts, j.dummySlaveTemplate("asdf")));
         final Iterable<NodeProvisioner.PlannedNode> pns = cloud.provision(Label.get("asdf"), 1);
-        final Matcher<Iterable<NodeProvisioner.PlannedNode>> hasOnlyOneElement = iterableWithSize(1);
-        assertThat(pns, hasOnlyOneElement);
+        assertThat(pns, iterableWithSize(1));
         final PlannedNode pn = pns.iterator().next();
         final Future<Node> pnf = pn.future;