CloudAuditService preparing for secret refresh (#8486)

* Audit service registration does not require passing a configuration object anymore.
* CloudAuditingService using CloudBlobClientWrapper

Author: agr

src/NuGetGallery.Core/Auditing/CloudAuditingService.cs

@@ -21,17 +21,17 @@ public class CloudAuditingService : AuditingService, ICloudStorageStatusDependen
     {
         public static readonly string DefaultContainerName = "auditing";
 
-        private CloudBlobContainer _auditContainer;
+        private Func<ICloudBlobContainer> _auditContainerFactory;
         private Func<Task<AuditActor>> _getOnBehalfOf;
 
-        public CloudAuditingService(string storageConnectionString, bool readAccessGeoRedundant, Func<Task<AuditActor>> getOnBehalfOf)
-            : this(GetContainer(storageConnectionString, readAccessGeoRedundant), getOnBehalfOf)
+        public CloudAuditingService(Func<ICloudBlobClient> cloudBlobClientFactory, Func<Task<AuditActor>> getOnBehalfOf)
+            : this(() => GetContainer(cloudBlobClientFactory), getOnBehalfOf)
         {
         }
 
-        public CloudAuditingService(CloudBlobContainer auditContainer, Func<Task<AuditActor>> getOnBehalfOf)
+        public CloudAuditingService(Func<ICloudBlobContainer> auditContainerFactory, Func<Task<AuditActor>> getOnBehalfOf)
         {
-            _auditContainer = auditContainer;
+            _auditContainerFactory = auditContainerFactory;
             _getOnBehalfOf = getOnBehalfOf;
         }
 
@@ -52,7 +52,8 @@ protected override async Task SaveAuditRecordAsync(string auditData, string reso
                 $"{filePath.Replace(Path.DirectorySeparatorChar, '/')}/" +
                 $"{Guid.NewGuid().ToString("N")}-{action.ToLowerInvariant()}.audit.v1.json";
 
-            var blob = _auditContainer.GetBlockBlobReference(fullPath);
+            var container = _auditContainerFactory();
+            var blob = container.GetBlobReference(fullPath);
             bool retry = false;
             try
             {
@@ -74,37 +75,23 @@ protected override async Task SaveAuditRecordAsync(string auditData, string reso
             {
                 // Create the container and try again,
                 // this time we let exceptions bubble out
-                await Task.Factory.FromAsync(
-                    (cb, s) => _auditContainer.BeginCreateIfNotExists(cb, s),
-                    ar => _auditContainer.EndCreateIfNotExists(ar),
-                    null);
+                await container.CreateIfNotExistAsync(permissions: null);
                 await WriteBlob(auditData, fullPath, blob);
             }
         }
 
-        private static CloudBlobContainer GetContainer(string storageConnectionString, bool readAccessGeoRedundant)
+        private static ICloudBlobContainer GetContainer(Func<ICloudBlobClient> cloudBlobClientFactory)
         {
-            var cloudBlobClient = CloudStorageAccount.Parse(storageConnectionString).CreateCloudBlobClient();
-            if (readAccessGeoRedundant)
-            {
-                cloudBlobClient.DefaultRequestOptions.LocationMode = LocationMode.PrimaryThenSecondary;
-            }
+            var cloudBlobClient = cloudBlobClientFactory();
             return cloudBlobClient.GetContainerReference(DefaultContainerName);
         }
 
-        private static async Task WriteBlob(string auditData, string fullPath, CloudBlockBlob blob)
+        private static async Task WriteBlob(string auditData, string fullPath, ISimpleCloudBlob blob)
         {
             try
             {
-                var strm = await Task.Factory.FromAsync(
-                    (cb, s) => blob.BeginOpenWrite(
-                        AccessCondition.GenerateIfNoneMatchCondition("*"),
-                        new BlobRequestOptions(),
-                        new OperationContext(),
-                        cb, s),
-                    ar => blob.EndOpenWrite(ar),
-                    null);
-                using (var writer = new StreamWriter(strm))
+                using (var stream = await blob.OpenWriteAsync(AccessCondition.GenerateIfNoneMatchCondition("*")))
+                using (var writer = new StreamWriter(stream))
                 {
                     await writer.WriteAsync(auditData);
                 }
@@ -125,7 +112,7 @@ private static async Task WriteBlob(string auditData, string fullPath, CloudBloc
 
         public Task<bool> IsAvailableAsync(BlobRequestOptions options, OperationContext operationContext)
         {
-            return _auditContainer.ExistsAsync(options, operationContext);
+            return _auditContainerFactory().ExistsAsync(options, operationContext);
         }
 
         public override string RenderAuditEntry(AuditEntry entry)

src/NuGetGallery/App_Start/DefaultDependenciesModule.cs

@@ -78,6 +78,8 @@ public static class BindingKeys
             public const string EmailPublisherTopic = "EmailPublisherBindingKey";
 
             public const string PreviewSearchClient = "PreviewSearchClientBindingKey";
+
+            public const string AuditKey = "AuditKey";
         }
 
         public static class ParameterNames
@@ -468,24 +470,20 @@ protected override void Load(ContainerBuilder builder)
                 .As<IPrincipal>()
                 .InstancePerLifetimeScope();
 
-            IAuditingService defaultAuditingService = null;
-
             switch (configuration.Current.StorageType)
             {
                 case StorageType.FileSystem:
                 case StorageType.NotSpecified:
                     ConfigureForLocalFileSystem(builder, configuration);
-                    defaultAuditingService = GetAuditingServiceForLocalFileSystem(configuration);
                     break;
                 case StorageType.AzureStorage:
                     ConfigureForAzureStorage(builder, configuration, telemetryService);
-                    defaultAuditingService = GetAuditingServiceForAzureStorage(builder, configuration);
                     break;
             }
 
             RegisterAsynchronousValidation(builder, loggerFactory, configuration, secretInjector);
 
-            RegisterAuditingServices(builder, defaultAuditingService);
+            RegisterAuditingServices(builder, configuration.Current.StorageType);
 
             RegisterCookieComplianceService(configuration, loggerFactory);
 
@@ -1376,10 +1374,10 @@ private static void ConfigureForLocalFileSystem(ContainerBuilder builder, IGalle
                 .SingleInstance();
         }
 
-        private static IAuditingService GetAuditingServiceForLocalFileSystem(IGalleryConfigurationService configuration)
+        private static IAuditingService GetAuditingServiceForLocalFileSystem(IAppConfiguration configuration)
         {
             var auditingPath = Path.Combine(
-                FileSystemFileStorageService.ResolvePath(configuration.Current.FileStorageDirectory),
+                FileSystemFileStorageService.ResolvePath(configuration.FileStorageDirectory),
                 FileSystemAuditingService.DefaultContainerName);
 
             return new FileSystemAuditingService(auditingPath, AuditActor.GetAspNetOnBehalfOfAsync);
@@ -1449,17 +1447,6 @@ private static void ConfigureForAzureStorage(ContainerBuilder builder, IGalleryC
                 .SingleInstance();
         }
 
-        private static IAuditingService GetAuditingServiceForAzureStorage(ContainerBuilder builder, IGalleryConfigurationService configuration)
-        {
-            var service = new CloudAuditingService(configuration.Current.AzureStorage_Auditing_ConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant, AuditActor.GetAspNetOnBehalfOfAsync);
-
-            builder.RegisterInstance(service)
-                .As<ICloudStorageStatusDependency>()
-                .SingleInstance();
-
-            return service;
-        }
-
         private static IAuditingService CombineAuditingServices(IEnumerable<IAuditingService> services)
         {
             if (!services.Any())
@@ -1491,19 +1478,54 @@ private static IEnumerable<T> GetAddInServices<T>(Action<RuntimeServiceProvider>
             }
         }
 
-        private static void RegisterAuditingServices(ContainerBuilder builder, IAuditingService defaultAuditingService)
+        private static void RegisterAuditingServices(ContainerBuilder builder, string storageType)
         {
-            var auditingServices = GetAddInServices<IAuditingService>();
-            var services = new List<IAuditingService>(auditingServices);
-
-            if (defaultAuditingService != null)
+            if (storageType == StorageType.AzureStorage)
             {
-                services.Add(defaultAuditingService);
+                builder.Register(c =>
+                    {
+                        var configuration = c.Resolve<IAppConfiguration>();
+                        return new CloudBlobClientWrapper(configuration.AzureStorage_Auditing_ConnectionString, configuration.AzureStorageReadAccessGeoRedundant);
+                    })
+                    .SingleInstance()
+                    .Keyed<ICloudBlobClient>(BindingKeys.AuditKey);
+
+                builder.Register(c =>
+                    {
+                        var blobClientFactory = c.ResolveKeyed<Func<ICloudBlobClient>>(BindingKeys.AuditKey);
+                        return new CloudAuditingService(blobClientFactory, AuditActor.GetAspNetOnBehalfOfAsync);
+                    })
+                    .SingleInstance()
+                    .AsSelf()
+                    .As<ICloudStorageStatusDependency>();
             }
 
-            var service = CombineAuditingServices(services);
+            builder.Register(c =>
+                {
+                    var configuration = c.Resolve<IAppConfiguration>();
+                    IAuditingService defaultAuditingService = null;
+                    switch (storageType)
+                    {
+                        case StorageType.FileSystem:
+                        case StorageType.NotSpecified:
+                            defaultAuditingService = GetAuditingServiceForLocalFileSystem(configuration);
+                            break;
+
+                        case StorageType.AzureStorage:
+                            defaultAuditingService = c.Resolve<CloudAuditingService>();
+                            break;
+                    }
+
+                    var auditingServices = GetAddInServices<IAuditingService>();
+                    var services = new List<IAuditingService>(auditingServices);
+
+                    if (defaultAuditingService != null)
+                    {
+                        services.Add(defaultAuditingService);
+                    }
 
-            builder.RegisterInstance(service)
+                    return CombineAuditingServices(services);
+                })
                 .AsSelf()
                 .As<IAuditingService>()
                 .SingleInstance();

tests/NuGetGallery.Core.Facts/Auditing/CloudAuditingServiceTests.cs

@@ -17,8 +17,8 @@ public class CloudAuditingServiceTests
         public void CloudAuditServiceObfuscateAuditRecord()
         {
             // Arrange
-            CloudBlobContainer nullBlobContainer = null;
-            var service = new CloudAuditingService(nullBlobContainer, AuditActor.GetCurrentMachineActorAsync);
+            ICloudBlobContainer nullBlobContainer = null;
+            var service = new CloudAuditingService(() => nullBlobContainer, AuditActor.GetCurrentMachineActorAsync);
 
             AuditActor onBehalfOf = new AuditActor("machineName", "3.3.3.3", "userName1", "NoAuthentication", "someKey", DateTime.Now, null);
             AuditActor auditActor = new AuditActor("machineName", "2.2.2.2", "userName1", "NoAuthentication", "someKey", DateTime.Now, onBehalfOf);
@@ -56,8 +56,8 @@ public void CloudAuditServiceObfuscateAuditRecord()
         public void OnlyPackageAuditRecordsWillBeSaved(AuditRecord record, bool expectedResult)
         {
             // Arrange
-            CloudBlobContainer nullBlobContainer = null;
-            var service = new CloudAuditingService(nullBlobContainer, AuditActor.GetCurrentMachineActorAsync);
+            ICloudBlobContainer nullBlobContainer = null;
+            var service = new CloudAuditingService(() => nullBlobContainer, AuditActor.GetCurrentMachineActorAsync);
 
             // Act + Assert
             Assert.Equal<bool>(expectedResult, service.RecordWillBePersisted(record));