MSI auth support for NuGetGallery (#10059)

agr · web-flow · commit 503375867117 · 2024-07-10T12:24:35.000-07:00
* MSI auth.
* DefaultAzureCredential support for debug builds for local debugging.
* Removed last reference to WindowsAzure.Storage
* Extracting blob properties from `BlobDownloadDetails`.
diff --git a/src/AccountDeleter/Configuration/GalleryConfiguration.cs b/src/AccountDeleter/Configuration/GalleryConfiguration.cs
@@ -35,6 +35,8 @@ public string SiteRoot
         public string AzureStorage_Uploads_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
         public string AzureStorage_Revalidation_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
         public bool AzureStorageReadAccessGeoRedundant { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
+        public bool AzureStorageUseMsi { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
+        public string AzureStorageMsiClientId { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
         public TimeSpan FeatureFlagsRefreshInterval { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
         public bool AdminPanelEnabled { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
         public bool AdminPanelDatabaseAccessEnabled { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
diff --git a/src/NuGetGallery.Core/NuGetGallery.Core.csproj b/src/NuGetGallery.Core/NuGetGallery.Core.csproj
@@ -41,8 +41,6 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="WindowsAzure.Storage" Version="9.3.3" />
-
     <PackageReference Include="Azure.Core" Version="1.39.0" />
     <PackageReference Include="Azure.Identity" Version="1.11.3" />
     <PackageReference Include="Azure.Storage.Blobs" Version="12.20.0" />
diff --git a/src/NuGetGallery.Core/Services/CloudBlobClientWrapper.cs b/src/NuGetGallery.Core/Services/CloudBlobClientWrapper.cs
@@ -72,6 +72,20 @@ public static CloudBlobClientWrapper UsingServicePrincipal(
             return new CloudBlobClientWrapper(storageConnectionString, tokenCredential, readAccessGeoRedundant, requestTimeout);
         }
 
+        public static CloudBlobClientWrapper UsingDefaultAzureCredential(
+            string storageConnectionString,
+            string clientId = null,
+            bool readAccessGeoRedundant = false,
+            TimeSpan? requestTimeout = null)
+        {
+            var options = new DefaultAzureCredentialOptions
+            {
+                ManagedIdentityClientId = clientId,
+            };
+            var tokenCredential = new DefaultAzureCredential(options);
+            return new CloudBlobClientWrapper(storageConnectionString, tokenCredential, readAccessGeoRedundant, requestTimeout);
+        }
+
         public ISimpleCloudBlob GetBlobFromUri(Uri uri)
         {
             // For Azure blobs, the query string is assumed to be the SAS token.
diff --git a/src/NuGetGallery.Core/Services/CloudBlobReadOnlyProperties.cs b/src/NuGetGallery.Core/Services/CloudBlobReadOnlyProperties.cs
@@ -31,5 +31,14 @@ public CloudBlobReadOnlyProperties(BlobItem blobItem)
             CopyStatus = null;
             CopyStatusDescription = null;
         }
+
+        public CloudBlobReadOnlyProperties(BlobDownloadDetails details)
+        {
+            LastModifiedUtc = details.LastModified.UtcDateTime;
+            Length = details.ContentLength;
+            IsSnapshot = false;
+            CopyStatus = details.CopyStatus;
+            CopyStatusDescription = details.CopyStatusDescription;
+        }
     }
 }
diff --git a/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs b/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs
@@ -522,6 +522,7 @@ private void UpdateEtag(BlobDownloadDetails details)
             if (details != null)
             {
                 _lastSeenEtag = EtagToString(details.ETag);
+                BlobProperties = new CloudBlobReadOnlyProperties(details);
                 ReplaceHttpHeaders(details);
                 ReplaceMetadata(details.Metadata);
             }
diff --git a/src/NuGetGallery.Services/Configuration/AppConfiguration.cs b/src/NuGetGallery.Services/Configuration/AppConfiguration.cs
@@ -71,6 +71,10 @@ public class AppConfiguration : IAppConfiguration
         /// </summary>
         public bool AzureStorageReadAccessGeoRedundant { get; set; }
 
+        public bool AzureStorageUseMsi { get; set; } = false;
+
+        public string AzureStorageMsiClientId { get; set; } = null;
+
         public TimeSpan FeatureFlagsRefreshInterval { get; set; }
 
         [DefaultValue(true)]
diff --git a/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs b/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs
@@ -90,6 +90,20 @@ public interface IAppConfiguration : IMessageServiceConfiguration
         /// </summary>
         bool AzureStorageReadAccessGeoRedundant { get; set; }
 
+        /// <summary>
+        /// Indicates whether Managed Service Identity should be used to access Azure Storage.
+        /// If false, the presumption is that connection strings contain the necessary credentials.
+        /// If true, single MSI is going to be used for all storage connections.
+        /// </summary>
+        bool AzureStorageUseMsi { get; set; }
+
+        /// <summary>
+        /// Client ID of the MSI to use for Azure storage access.
+        /// If empty or not specified, the default MSI will be used in Release builds
+        /// and <see cref="Azure.Identity.DefaultAzureCredential"/> in Debug builds.
+        /// </summary>
+        string AzureStorageMsiClientId { get; set; }
+
         /// <summary>
         /// How frequently the feature flags should be refreshed.
         /// </summary>
diff --git a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs
@@ -237,9 +237,9 @@ protected override void Load(ContainerBuilder builder)
                 .InstancePerLifetimeScope();
 
             builder.RegisterType<EntityRepository<AccountDelete>>()
-               .AsSelf()
-               .As<IEntityRepository<AccountDelete>>()
-               .InstancePerLifetimeScope();
+                .AsSelf()
+                .As<IEntityRepository<AccountDelete>>()
+                .InstancePerLifetimeScope();
 
             builder.RegisterType<EntityRepository<Credential>>()
                 .AsSelf()
@@ -1432,18 +1432,45 @@ private static void ConfigureForAzureStorage(ContainerBuilder builder, IGalleryC
             {
                 if (completedBindingKeys.Add(dependent.BindingKey))
                 {
-                    builder.RegisterInstance(new CloudBlobClientWrapper(dependent.AzureStorageConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant))
-                       .AsSelf()
-                       .As<ICloudBlobClient>()
-                       .SingleInstance()
-                       .Keyed<ICloudBlobClient>(dependent.BindingKey);
+                    CloudBlobClientWrapper blobClient;
+                    if (!configuration.Current.AzureStorageUseMsi)
+                    {
+                        blobClient = new CloudBlobClientWrapper(dependent.AzureStorageConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant);
+                    }
+                    else
+                    {
+                        if (string.IsNullOrWhiteSpace(configuration.Current.AzureStorageMsiClientId))
+                        {
+#if DEBUG
+                            blobClient = CloudBlobClientWrapper.UsingDefaultAzureCredential(
+                                dependent.AzureStorageConnectionString,
+                                readAccessGeoRedundant: configuration.Current.AzureStorageReadAccessGeoRedundant);
+#else
+                            blobClient = CloudBlobClientWrapper.UsingMsi(
+                                dependent.AzureStorageConnectionString,
+                                readAccessGeoRedundant: configuration.Current.AzureStorageReadAccessGeoRedundant);
+#endif
+                        }
+                        else
+                        {
+                            blobClient = CloudBlobClientWrapper.UsingMsi(
+                                dependent.AzureStorageConnectionString,
+                                configuration.Current.AzureStorageMsiClientId,
+                                configuration.Current.AzureStorageReadAccessGeoRedundant);
+                        }
+                    }
+                    builder.RegisterInstance(blobClient)
+                        .AsSelf()
+                        .As<ICloudBlobClient>()
+                        .SingleInstance()
+                        .Keyed<ICloudBlobClient>(dependent.BindingKey);
 
                     // Do not register the service as ICloudStorageStatusDependency because
                     // the CloudAuditingService registers it and the gallery uses the same storage account for all the containers.
                     builder.RegisterType<CloudBlobFileStorageService>()
                         .WithParameter(new ResolvedParameter(
-                           (pi, ctx) => pi.ParameterType == typeof(ICloudBlobClient),
-                           (pi, ctx) => ctx.ResolveKeyed<ICloudBlobClient>(dependent.BindingKey)))
+                            (pi, ctx) => pi.ParameterType == typeof(ICloudBlobClient),
+                            (pi, ctx) => ctx.ResolveKeyed<ICloudBlobClient>(dependent.BindingKey)))
                         .AsSelf()
                         .As<IFileStorageService>()
                         .As<ICoreFileStorageService>()
diff --git a/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj b/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj
@@ -108,7 +108,6 @@
     <Compile Include="Services\CoreReadmeFileServiceFacts.cs" />
     <Compile Include="Services\CoreLicenseFileServiceFacts.cs" />
     <Compile Include="Services\CoreSymbolPackageServiceFacts.cs" />
-    <Compile Include="Services\FileUriPermissionsFacts.cs" />
     <Compile Include="Services\FolderNamesDataAttribute.cs" />
     <Compile Include="Services\FolderNamesDataAttributeFacts.cs" />
     <Compile Include="Services\GalleryCloudBlobContainerInformationProviderFacts.cs" />
diff --git a/tests/NuGetGallery.Core.Facts/Services/FileUriPermissionsFacts.cs b/tests/NuGetGallery.Core.Facts/Services/FileUriPermissionsFacts.cs
diff --git a/tests/NuGetGallery.Core.Facts/TestUtils/BlobStorageFixture.cs b/tests/NuGetGallery.Core.Facts/TestUtils/BlobStorageFixture.cs
@@ -2,9 +2,8 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Linq;
-using Microsoft.WindowsAzure.Storage;
-using Microsoft.WindowsAzure.Storage.Blob;
+using Azure.Storage.Blobs;
+using Azure.Storage.Blobs.Models;
 
 namespace NuGetGallery
 {
@@ -70,18 +69,17 @@ private static string GetEnvironmentVariable(string name, bool required)
 
         private void DeleteTestBlobs(string connectionString)
         {
-            var account = CloudStorageAccount.Parse(connectionString);
-            var blobClient = account.CreateCloudBlobClient();
-            var containers = blobClient.ListContainers();
-            foreach (var container in containers)
+            var client = new BlobServiceClient(connectionString);
+            var containers = client.GetBlobContainers();
+            foreach (var containerInfo in containers)
             {
-                var blobs = container.ListBlobs(
-                    prefix: TestRunId,
-                    useFlatBlobListing: true);
+                var container = client.GetBlobContainerClient(containerInfo.Name);
+                var blobs = container.GetBlobs(prefix: TestRunId);
 
-                foreach (var blob in blobs.OfType<CloudBlockBlob>())
+                foreach (var blobDetails in blobs)
                 {
-                    blob.DeleteIfExists(DeleteSnapshotsOption.IncludeSnapshots);
+                    var blob = container.GetBlobClient(blobDetails.Name);
+                    blob.DeleteIfExists(snapshotsOption: DeleteSnapshotsOption.IncludeSnapshots);
                 }
             }
         }
diff --git a/tests/NuGetGallery.Facts/Services/CloudBlobFileStorageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/CloudBlobFileStorageServiceFacts.cs
@@ -8,7 +8,6 @@
 using System.Threading.Tasks;
 using System.Web;
 using System.Web.Mvc;
-using Microsoft.WindowsAzure.Storage.Blob;
 using Moq;
 using NuGetGallery.Configuration;
 using NuGetGallery.Diagnostics;

Original file line number	Diff line number	Diff line change
`@@ -31,5 +31,14 @@ public CloudBlobReadOnlyProperties(BlobItem blobItem)`
`31`	`31`	`CopyStatus = null;`
`32`	`32`	`CopyStatusDescription = null;`
`33`	`33`	`}`
	`34`	`+`
	`35`	`+ public CloudBlobReadOnlyProperties(BlobDownloadDetails details)`
	`36`	`+ {`
	`37`	`+ LastModifiedUtc = details.LastModified.UtcDateTime;`
	`38`	`+ Length = details.ContentLength;`
	`39`	`+ IsSnapshot = false;`
	`40`	`+ CopyStatus = details.CopyStatus;`
	`41`	`+ CopyStatusDescription = details.CopyStatusDescription;`
	`42`	`+ }`
`34`	`43`	`}`
`35`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -522,6 +522,7 @@ private void UpdateEtag(BlobDownloadDetails details)`
`522`	`522`	`if (details != null)`
`523`	`523`	`{`
`524`	`524`	`_lastSeenEtag = EtagToString(details.ETag);`
	`525`	`+ BlobProperties = new CloudBlobReadOnlyProperties(details);`
`525`	`526`	`ReplaceHttpHeaders(details);`
`526`	`527`	`ReplaceMetadata(details.Metadata);`
`527`	`528`	`}`