[SECURITY-1756][SECURITY-1757]

Co-authored-by: Tim Jacomb <timjacomb1@gmail.com>
Co-authored-by: Yaroslav Afenkin <yaroslavafenkin@users.noreply.github.com>

src/main/java/com/microsoft/azure/util/AzureCredentials.java

@@ -15,6 +15,7 @@
 import com.azure.resourcemanager.resources.models.Subscription;
 import com.azure.security.keyvault.secrets.SecretClient;
 import com.azure.security.keyvault.secrets.SecretClientBuilder;
+import com.cloudbees.plugins.credentials.Credentials;
 import com.cloudbees.plugins.credentials.CredentialsMatchers;
 import com.cloudbees.plugins.credentials.CredentialsProvider;
 import com.cloudbees.plugins.credentials.CredentialsScope;
@@ -35,11 +36,13 @@
 import io.jenkins.plugins.azuresdk.HttpClientRetriever;
 import java.util.List;
 import jenkins.model.Jenkins;
+import org.acegisecurity.Authentication;
 import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.verb.POST;
 
 import edu.umd.cs.findbugs.annotations.Nullable;
 import java.io.ByteArrayInputStream;
@@ -153,13 +156,15 @@ CertificateCredentialsImpl getCertificate() {
             if (StringUtils.isEmpty(certificateId)) {
                 return null;
             }
-            return CredentialsMatchers.firstOrNull(
-                    CredentialsProvider.lookupCredentials(
-                            CertificateCredentialsImpl.class,
-                            Jenkins.get(),
-                            ACL.SYSTEM,
-                            Collections.emptyList()),
-                    CredentialsMatchers.withId(certificateId));
+            CertificateCredentialsImpl certificate = getCredentials(
+                    CertificateCredentialsImpl.class,
+                    certificateId, ACL.SYSTEM);
+            if (certificate == null) {
+                return getCredentials(
+                        CertificateCredentialsImpl.class,
+                        certificateId, Jenkins.getAuthentication());
+            }
+            return null;
         }
 
         @Nullable
@@ -565,6 +570,17 @@ public static TokenCredential getSystemCredentialById(String credentialID) {
         return credential;
     }
 
+    private static <T extends Credentials> T getCredentials(
+            Class<T> type, String certificateId, Authentication authentication) {
+        return CredentialsMatchers.firstOrNull(
+                CredentialsProvider.lookupCredentials(
+                        type,
+                        Jenkins.get(),
+                        authentication,
+                        Collections.emptyList()),
+                CredentialsMatchers.withId(certificateId));
+    }
+
 
     public static TokenCredential getTokenCredential(AzureBaseCredentials credentials) {
         if (credentials instanceof AzureCredentials) {
@@ -769,7 +785,10 @@ public String getDisplayName() {
             return "Azure Service Principal";
         }
 
+
+        @POST
         public FormValidation doVerifyConfiguration(
+                @AncestorInPath Item owner,
                 @QueryParameter String subscriptionId,
                 @QueryParameter String clientId,
                 @QueryParameter String clientSecret,
@@ -780,6 +799,11 @@ public FormValidation doVerifyConfiguration(
                 @QueryParameter String authenticationEndpoint,
                 @QueryParameter String resourceManagerEndpoint,
                 @QueryParameter String graphEndpoint) {
+            if (owner == null) {
+                Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+            } else {
+                owner.checkPermission(Item.CONFIGURE);
+            }
 
             AzureCredentials.ServicePrincipal servicePrincipal
                     = new AzureCredentials.ServicePrincipal(subscriptionId, clientId, Secret.fromString(clientSecret));
@@ -799,11 +823,27 @@ public FormValidation doVerifyConfiguration(
             return FormValidation.ok(Messages.Azure_Config_Success());
         }
 
-        public ListBoxModel doFillCertificateIdItems(@AncestorInPath Item owner) {
+        public ListBoxModel doFillCertificateIdItems(
+                @AncestorInPath Item owner,
+                @QueryParameter("certificateId") String certificateId) {
             StandardListBoxModel model = new StandardListBoxModel();
             model.add(Messages.Azure_Credentials_Select(), "");
-            model.includeAs(ACL.SYSTEM, owner, CertificateCredentialsImpl.class);
-            return model;
+            if (owner == null) {
+                if (!Jenkins.get().hasPermission(CredentialsProvider.CREATE)
+                        && !Jenkins.get().hasPermission(CredentialsProvider.UPDATE)) {
+                    return model.includeCurrentValue(certificateId);
+                }
+            } else {
+                if (!owner.hasPermission(CredentialsProvider.CREATE)
+                        && !owner.hasPermission(CredentialsProvider.UPDATE)) {
+                    return model.includeCurrentValue(certificateId);
+                }
+            }
+
+            return model
+                    .includeCurrentValue(certificateId)
+                    .includeAs(Jenkins.getAuthentication(), owner, CertificateCredentialsImpl.class)
+                    .includeAs(ACL.SYSTEM, owner, CertificateCredentialsImpl.class);
         }
 
         public ListBoxModel doFillAzureEnvironmentNameItems() {

src/main/java/com/microsoft/azure/util/AzureImdsCredentials.java

@@ -1,5 +1,7 @@
 package com.microsoft.azure.util;
 
+import com.azure.core.http.policy.FixedDelay;
+import com.azure.core.http.policy.RetryPolicy;
 import com.azure.core.http.rest.PagedIterable;
 import com.azure.core.management.profile.AzureProfile;
 import com.azure.identity.ManagedIdentityCredentialBuilder;
@@ -8,14 +10,20 @@
 import com.cloudbees.plugins.credentials.CredentialsScope;
 import com.cloudbees.plugins.credentials.impl.BaseStandardCredentials;
 import hudson.Extension;
+import hudson.Main;
 import hudson.Util;
+import hudson.model.Item;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
 import io.jenkins.plugins.azuresdk.HttpClientRetriever;
+import java.time.Duration;
+import jenkins.model.Jenkins;
 import org.apache.commons.lang.StringUtils;
+import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.verb.POST;
 
 public class AzureImdsCredentials extends AbstractManagedIdentitiesCredentials {
 
@@ -65,6 +73,7 @@ public boolean validate() throws AzureCredentials.ValidationException {
 
             AzureResourceManager azure = AzureResourceManager
                     .configure()
+                    .withRetryPolicy(getRetryPolicy())
                     .withHttpClient(HttpClientRetriever.get())
                     .authenticate(credentialBuilder.build(), profile)
                     .withSubscription(credentialSubscriptionId);
@@ -86,6 +95,10 @@ public boolean validate() throws AzureCredentials.ValidationException {
         throw new AzureCredentials.ValidationException(Messages.Azure_Invalid_SubscriptionId());
     }
 
+    private static RetryPolicy getRetryPolicy() {
+        return Main.isUnitTest ? new RetryPolicy(new FixedDelay(0, Duration.ZERO)) : new RetryPolicy();
+    }
+
 
     @Extension
     public static class DescriptorImpl
@@ -104,10 +117,17 @@ public ListBoxModel doFillAzureEnvNameItems() {
             return model;
         }
 
+        @POST
         public FormValidation doVerifyConfiguration(
+                @AncestorInPath Item owner,
                 @QueryParameter String subscriptionId,
                 @QueryParameter String clientId,
                 @QueryParameter String azureEnvironmentName) {
+            if (owner == null) {
+                Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+            } else {
+                owner.checkPermission(Item.CONFIGURE);
+            }
 
             AzureImdsCredentials imdsCredentials = new AzureImdsCredentials(null, null, null, azureEnvironmentName);
             if (StringUtils.isNotBlank(subscriptionId)) {

src/main/java/com/microsoft/jenkins/keyvault/BaseSecretCredentials.java

@@ -7,6 +7,7 @@
 
 import com.azure.security.keyvault.secrets.SecretClient;
 import com.azure.security.keyvault.secrets.models.KeyVaultSecret;
+import com.cloudbees.plugins.credentials.CredentialsProvider;
 import com.cloudbees.plugins.credentials.CredentialsScope;
 import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
 import com.cloudbees.plugins.credentials.impl.BaseStandardCredentials;
@@ -15,6 +16,10 @@
 import hudson.model.Item;
 import hudson.security.ACL;
 import hudson.util.ListBoxModel;
+import jenkins.model.Jenkins;
+import org.kohsuke.stapler.AncestorInPath;
+import org.kohsuke.stapler.QueryParameter;
+
 import java.net.MalformedURLException;
 import java.net.URL;
 
@@ -94,10 +99,26 @@ interface SecretGetter {
 
     protected abstract static class DescriptorImpl extends BaseStandardCredentialsDescriptor {
 
-        public ListBoxModel doFillServicePrincipalIdItems(Item owner) {
-            return new StandardListBoxModel()
-                .includeEmptyValue()
-                .includeAs(ACL.SYSTEM, owner, AzureCredentials.class);
+        public ListBoxModel doFillServicePrincipalIdItems(
+                @AncestorInPath Item owner,
+                @QueryParameter("servicePrincipalId") String servicePrincipalId) {
+            StandardListBoxModel model = new StandardListBoxModel();
+            model.includeEmptyValue();
+            if (owner == null) {
+                if (!Jenkins.get().hasPermission(CredentialsProvider.CREATE)
+                        && !Jenkins.get().hasPermission(CredentialsProvider.UPDATE)) {
+                    return model.includeCurrentValue(servicePrincipalId);
+                }
+            } else {
+                if (!owner.hasPermission(CredentialsProvider.CREATE)
+                        && !owner.hasPermission(CredentialsProvider.UPDATE)) {
+                    return model.includeCurrentValue(servicePrincipalId);
+                }
+            }
+            return model
+                    .includeCurrentValue(servicePrincipalId)
+                    .includeAs(Jenkins.getAuthentication(), owner, AzureCredentials.class)
+                    .includeAs(ACL.SYSTEM, owner, AzureCredentials.class);
         }
     }
 }

src/main/java/com/microsoft/jenkins/keyvault/SecretCertificateCredentials.java

@@ -11,11 +11,15 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
 import hudson.Util;
+import hudson.model.Item;
 import hudson.util.FormValidation;
 import hudson.util.Secret;
+import jenkins.model.Jenkins;
 import org.apache.commons.codec.binary.Base64;
+import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.verb.POST;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
@@ -102,10 +106,17 @@ public String getDisplayName() {
             return Messages.Certificate_Credentials_Display_Name();
         }
 
+        @POST
         public FormValidation doVerifyConfiguration(
+                @AncestorInPath Item owner,
                 @QueryParameter String servicePrincipalId,
                 @QueryParameter String secretIdentifier,
                 @QueryParameter Secret password) {
+            if (owner == null) {
+                Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+            } else {
+                owner.checkPermission(Item.CONFIGURE);
+            }
 
             final SecretCertificateCredentials credentials = new SecretCertificateCredentials(
                     CredentialsScope.SYSTEM, "", "", servicePrincipalId, secretIdentifier, password);

src/main/java/com/microsoft/jenkins/keyvault/SecretStringCredentials.java

@@ -7,11 +7,15 @@
 import com.azure.security.keyvault.secrets.models.KeyVaultSecret;
 import com.cloudbees.plugins.credentials.CredentialsScope;
 import hudson.Extension;
+import hudson.model.Item;
 import hudson.util.FormValidation;
 import hudson.util.Secret;
+import jenkins.model.Jenkins;
 import org.jenkinsci.plugins.plaincredentials.StringCredentials;
+import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.verb.POST;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 
@@ -43,9 +47,17 @@ public String getDisplayName() {
             return Messages.String_Credentials_Diaplay_Name();
         }
 
+
+        @POST
         public FormValidation doVerifyConfiguration(
+                @AncestorInPath Item owner,
                 @QueryParameter String servicePrincipalId,
                 @QueryParameter String secretIdentifier) {
+            if (owner == null) {
+                Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+            } else {
+                owner.checkPermission(Item.CONFIGURE);
+            }
 
             final SecretStringCredentials credentials = new SecretStringCredentials(
                     CredentialsScope.SYSTEM, "", "", servicePrincipalId, secretIdentifier);

src/main/resources/com/microsoft/azure/util/AzureCredentials/credentials.jelly

@@ -13,7 +13,7 @@
         <f:password/>
     </f:entry>
     <f:entry title="${%CertificateId}" field="certificateId" help="/plugin/azure-credentials/help-certificateId.html">
-        <c:select expressionAllowed="false" />
+        <c:select expressionAllowed="false" checkMethod="post" />
     </f:entry>
     <f:entry title="${%Tenant}" field="tenant" help="/plugin/azure-credentials/help-tenant.html">
         <f:textbox/>

src/main/resources/com/microsoft/jenkins/keyvault/SecretCertificateCredentials/credentials.jelly

@@ -6,7 +6,7 @@
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler" xmlns:c="/lib/credentials">
     <f:entry title="${%ServicePrincipal}" field="servicePrincipalId">
-        <c:select expressionAllowed="false"/>
+        <c:select expressionAllowed="false" checkMethod="post"/>
     </f:entry>
 
     <f:entry title="${%SecretIdentifier}" field="secretIdentifier">

src/main/resources/com/microsoft/jenkins/keyvault/SecretStringCredentials/credentials.jelly

@@ -6,7 +6,7 @@
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler" xmlns:c="/lib/credentials">
     <f:entry title="${%ServicePrincipal}" field="servicePrincipalId">
-        <c:select expressionAllowed="false"/>
+        <c:select expressionAllowed="false" checkMethod="post" />
     </f:entry>
 
     <f:entry title="${%SecretIdentifier}" field="secretIdentifier">

src/test/java/com/microsoft/azure/util/AzureImdsCredentialsTest.java

@@ -0,0 +1,64 @@
+package com.microsoft.azure.util;
+
+import com.cloudbees.hudson.plugins.folder.Folder;
+import hudson.model.Item;
+import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.ACLContext;
+import hudson.security.AccessDeniedException3;
+import hudson.util.FormValidation;
+import jenkins.model.Jenkins;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+public class AzureImdsCredentialsTest {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Test
+    public void descriptorVerifyConfigurationAsAdmin() {
+        // No security realm, anonymous has Overall/Administer
+        final AzureImdsCredentials.DescriptorImpl descriptor = new AzureImdsCredentials.DescriptorImpl();
+
+        FormValidation result = descriptor.doVerifyConfiguration(null,"", "", "");
+        Assert.assertEquals(FormValidation.Kind.ERROR, result.kind);
+    }
+
+    @Test
+    public void descriptorVerifyConfigurationWithAncestorAsAuthorizedUser() throws Exception {
+        Folder folder = j.jenkins.createProject(Folder.class, "folder");
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        MockAuthorizationStrategy authorizationStrategy = new MockAuthorizationStrategy();
+        authorizationStrategy.grant(Jenkins.READ).everywhere().to("user");
+        authorizationStrategy.grant(Item.CONFIGURE).onFolders(folder).to("user");
+        j.jenkins.setAuthorizationStrategy(authorizationStrategy);
+
+        final AzureImdsCredentials.DescriptorImpl descriptor = new AzureImdsCredentials.DescriptorImpl();
+
+        try (ACLContext ctx = ACL.as(User.getOrCreateByIdOrFullName("user"))) {
+            FormValidation result = descriptor.doVerifyConfiguration(folder, "", "", "");
+            // we aren't looking up an actual secret so this fails with missing protocol
+            // TODO mock secrets retrieval so we can test the happy case here properly
+            Assert.assertEquals(FormValidation.Kind.ERROR, result.kind);
+        }
+    }
+
+    @Test
+    public void descriptorVerifyConfigurationWithAncestorAsUnauthorizedUser() throws Exception {
+        Folder folder = j.jenkins.createProject(Folder.class, "folder");
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        MockAuthorizationStrategy authorizationStrategy = new MockAuthorizationStrategy();
+        authorizationStrategy.grant(Jenkins.READ).everywhere().to("user");
+        j.jenkins.setAuthorizationStrategy(authorizationStrategy);
+
+        final AzureImdsCredentials.DescriptorImpl descriptor = new AzureImdsCredentials.DescriptorImpl();
+
+        try (ACLContext ctx = ACL.as(User.getOrCreateByIdOrFullName("user"))) {
+            Assert.assertThrows(AccessDeniedException3.class, () -> descriptor.doVerifyConfiguration(folder, "", "", ""));
+        }
+    }
+}