Typosquatting: finish configuration file and feature flags (#6390)

zhhyu · web-flow · commit 3b705c6bc6cd · 2018-09-06T16:14:21.000-07:00
* Typosquatting: finish configuration file and feature flags

* Update unit tests

* Update

* update unit tests
diff --git a/src/NuGetGallery/App_Data/Files/Content/Typosquatting-Configuration.json b/src/NuGetGallery/App_Data/Files/Content/Typosquatting-Configuration.json
@@ -0,0 +1,5 @@
+{
+  "packageIdChecklistLength": 20000,
+  "isCheckEnabled": false,
+  "isBlockUsersEnabled": false
+}
diff --git a/src/NuGetGallery/App_Start/OwinStartup.cs b/src/NuGetGallery/App_Start/OwinStartup.cs
@@ -63,14 +63,17 @@ public static void Configuration(IAppBuilder app)
 
             // Configure machine key for session persistence across slots
             SessionPersistence.Setup(config);
-
             // Refresh the content for the ContentObjectService to guarantee it has loaded the latest configuration on startup.
-            if (config.Current.IsHosted)
+            var contentObjectService = dependencyResolver.GetService<IContentObjectService>();
+            HostingEnvironment.QueueBackgroundWorkItem(async token =>
             {
-                var contentObjectService = dependencyResolver.GetService<IContentObjectService>();
-                HostingEnvironment.QueueBackgroundWorkItem(async cancellationToken => await contentObjectService.Refresh());
-            }
-
+                while (!token.IsCancellationRequested)
+                {
+                    await contentObjectService.Refresh();
+                    await Task.Delay(ContentObjectService.RefreshInterval, token);
+                }
+            });
+            
             // Setup telemetry
             var instrumentationKey = config.Current.AppInsightsInstrumentationKey;
             if (!string.IsNullOrEmpty(instrumentationKey))
diff --git a/src/NuGetGallery/Constants.cs b/src/NuGetGallery/Constants.cs
@@ -104,6 +104,7 @@ public static class ContentNames
             public static readonly string LoginDiscontinuationConfiguration = "Login-Discontinuation-Configuration";
             public static readonly string CertificatesConfiguration = "Certificates-Configuration";
             public static readonly string SymbolsConfiguration = "Symbols-Configuration";
+            public static readonly string TyposquattingConfiguration = "Typosquatting-Configuration";
         }
 
         public static class StatisticsDimensions
diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj
@@ -395,12 +395,14 @@
     <Compile Include="Services\BackgroundMessageService.cs" />
     <Compile Include="Services\ISymbolPackageUploadService.cs" />
     <Compile Include="Services\ISymbolPackageFileService.cs" />
+    <Compile Include="Services\ITyposquattingConfiguration.cs" />
     <Compile Include="Services\ISymbolsConfiguration.cs" />
     <Compile Include="Services\ITyposquattingCheckService.cs" />
     <Compile Include="Services\ITyposquattingUserService.cs" />
     <Compile Include="Services\SymbolPackageUploadService.cs" />
     <Compile Include="Services\SymbolPackageFileService.cs" />
     <Compile Include="Services\SymbolPackageService.cs" />
+    <Compile Include="Services\TyposquattingConfiguration.cs" />
     <Compile Include="Services\SymbolsConfiguration.cs" />
     <Compile Include="Services\CertificatesConfiguration.cs" />
     <Compile Include="Services\CertificateService.cs" />
@@ -1746,6 +1748,7 @@
     <EmbeddedResource Include="Infrastructure\Elmah.SqlServer.sql" />
     <EmbeddedResource Include="Infrastructure\AddPackageLicenseReport.sql" />
     <Content Include="Public\robots.txt" />
+    <Content Include="App_Data\Files\Content\Typosquatting-Configuration.json" />
     <None Include="Properties\PublishProfiles\nuget-staging-frontend.pubxml" />
     <Content Include="Scripts\gallery\**\*" />
     <Content Include="Web.config">
diff --git a/src/NuGetGallery/Services/ContentObjectService.cs b/src/NuGetGallery/Services/ContentObjectService.cs
@@ -10,7 +10,7 @@ namespace NuGetGallery
 {
     public class ContentObjectService : IContentObjectService
     {
-        private const int RefreshIntervalMinutes = 5;
+        public static TimeSpan RefreshInterval = TimeSpan.FromMinutes(5);
 
         private readonly IContentService _contentService;
 
@@ -21,11 +21,13 @@ public ContentObjectService(IContentService contentService)
             LoginDiscontinuationConfiguration = new LoginDiscontinuationConfiguration();
             CertificatesConfiguration = new CertificatesConfiguration();
             SymbolsConfiguration = new SymbolsConfiguration();
+            TyposquattingConfiguration = new TyposquattingConfiguration();
         }
 
         public ILoginDiscontinuationConfiguration LoginDiscontinuationConfiguration { get; set; }
         public ICertificatesConfiguration CertificatesConfiguration { get; set; }
         public ISymbolsConfiguration SymbolsConfiguration { get; set; }
+        public ITyposquattingConfiguration TyposquattingConfiguration { get; set; }
 
         public async Task Refresh()
         {
@@ -40,12 +42,16 @@ await Refresh<CertificatesConfiguration>(Constants.ContentNames.CertificatesConf
             SymbolsConfiguration =
                 await Refresh<SymbolsConfiguration>(Constants.ContentNames.SymbolsConfiguration) ??
                 new SymbolsConfiguration();
+
+            TyposquattingConfiguration =
+               await Refresh<TyposquattingConfiguration>(Constants.ContentNames.TyposquattingConfiguration) ??
+               new TyposquattingConfiguration();
         }
 
         private async Task<T> Refresh<T>(string contentName) 
             where T : class
         {
-            var configString = (await _contentService.GetContentItemAsync(contentName, TimeSpan.FromMinutes(RefreshIntervalMinutes)))?.ToString();
+            var configString = (await _contentService.GetContentItemAsync(contentName, RefreshInterval))?.ToString();
             if (string.IsNullOrEmpty(configString))
             {
                 return null;
diff --git a/src/NuGetGallery/Services/IContentObjectService.cs b/src/NuGetGallery/Services/IContentObjectService.cs
@@ -11,7 +11,7 @@ public interface IContentObjectService
         ILoginDiscontinuationConfiguration LoginDiscontinuationConfiguration { get; }
         ICertificatesConfiguration CertificatesConfiguration { get; }
         ISymbolsConfiguration SymbolsConfiguration { get; }
-
+        ITyposquattingConfiguration TyposquattingConfiguration { get; }
         Task Refresh();
     }
 }
diff --git a/src/NuGetGallery/Services/ITyposquattingConfiguration.cs b/src/NuGetGallery/Services/ITyposquattingConfiguration.cs
@@ -0,0 +1,12 @@
+﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+namespace NuGetGallery.Services
+{
+    public interface ITyposquattingConfiguration
+    {
+        int PackageIdChecklistLength { get; }
+        bool IsCheckEnabled { get; }
+        bool IsBlockUsersEnabled { get; }
+    }
+}
diff --git a/src/NuGetGallery/Services/TyposquattingCheckService.cs b/src/NuGetGallery/Services/TyposquattingCheckService.cs
@@ -11,26 +11,26 @@ namespace NuGetGallery
 {
     public class TyposquattingCheckService : ITyposquattingCheckService
     {
-        // TODO: Length of checklist will be saved in the configuration file.
-        // https://github.com/NuGet/Engineering/issues/1645
-        private const int TyposquattingCheckListLength = 20000;
-
-        // TODO: Threshold parameters will be saved in the configuration file.
-        // https://github.com/NuGet/Engineering/issues/1645
         private static readonly IReadOnlyList<ThresholdInfo> ThresholdsList = new List<ThresholdInfo>
         {
             new ThresholdInfo (lowerBound: 0, upperBound: 30, threshold: 0),
             new ThresholdInfo (lowerBound: 30, upperBound: 50, threshold: 1),
-            new ThresholdInfo (lowerBound: 50, upperBound: 121, threshold: 2)
+            new ThresholdInfo (lowerBound: 50, upperBound: 129, threshold: 2)
         };
 
+        private static int TyposquattingCheckListLength;
+
         private readonly ITyposquattingUserService _userTyposquattingService;
         private readonly IEntityRepository<PackageRegistration> _packageRegistrationRepository;
+        private readonly IContentObjectService _contentObjectService;
 
-        public TyposquattingCheckService(ITyposquattingUserService typosquattingUserService, IEntityRepository<PackageRegistration> packageRegistrationRepository)
+        public TyposquattingCheckService(ITyposquattingUserService typosquattingUserService, IEntityRepository<PackageRegistration> packageRegistrationRepository, IContentObjectService contentObjectService)
         {
             _userTyposquattingService = typosquattingUserService ?? throw new ArgumentNullException(nameof(typosquattingUserService));
             _packageRegistrationRepository = packageRegistrationRepository ?? throw new ArgumentNullException(nameof(packageRegistrationRepository));
+            _contentObjectService = contentObjectService ?? throw new ArgumentNullException(nameof(contentObjectService));
+
+            TyposquattingCheckListLength = _contentObjectService.TyposquattingConfiguration.PackageIdChecklistLength;
         }
               
         public bool IsUploadedPackageIdTyposquatting(string uploadedPackageId, User uploadedPackageOwner)
diff --git a/src/NuGetGallery/Services/TyposquattingConfiguration.cs b/src/NuGetGallery/Services/TyposquattingConfiguration.cs
@@ -0,0 +1,32 @@
+﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Newtonsoft.Json;
+
+namespace NuGetGallery.Services
+{
+    public sealed class TyposquattingConfiguration : ITyposquattingConfiguration
+    {
+        private const int DefaultPackageIdCheckListLength = 20000;
+        public int PackageIdChecklistLength { get; }
+        public bool IsCheckEnabled { get; }
+        public bool IsBlockUsersEnabled { get; }
+        public TyposquattingConfiguration()
+            : this(packageIdChecklistLength: DefaultPackageIdCheckListLength,
+                  isCheckEnabled: false,
+                  isBlockUsersEnabled: false)
+        {
+        }
+
+        [JsonConstructor]
+        public TyposquattingConfiguration(
+            int packageIdChecklistLength,
+            bool isCheckEnabled,
+            bool isBlockUsersEnabled)
+        {
+            PackageIdChecklistLength = packageIdChecklistLength;
+            IsCheckEnabled = isCheckEnabled;
+            IsBlockUsersEnabled = isBlockUsersEnabled;
+        }
+    }
+}
diff --git a/tests/NuGetGallery.Facts/Services/TyposquattingCheckServiceFacts.cs b/tests/NuGetGallery.Facts/Services/TyposquattingCheckServiceFacts.cs
@@ -32,12 +32,17 @@ public class TyposquattingCheckServiceFacts
 
         private static Mock<ITyposquattingUserService> _typosquattingUserService = new Mock<ITyposquattingUserService>();
         private static Mock<IEntityRepository<PackageRegistration>> _packageRegistrationRepository = new Mock<IEntityRepository<PackageRegistration>>();
+        private static Mock<IContentObjectService> _contentObjectService = new Mock<IContentObjectService>();
 
         public TyposquattingCheckServiceFacts()
         {
             _packageRegistrationRepository
                 .Setup(x => x.GetAll())
                 .Returns(_pacakgeRegistrationsList.AsQueryable());
+
+            _contentObjectService
+                .Setup(x => x.TyposquattingConfiguration.PackageIdChecklistLength)
+                .Returns(20000);
         }
 
         [Fact]
@@ -51,7 +56,7 @@ public void CheckNotTyposquattingByDifferentOwnersTest()
                 .Setup(x => x.CanUserTyposquat(It.IsAny<string>(), It.IsAny<string>()))
                 .Returns(false);            
 
-            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object);
+            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object, _contentObjectService.Object);
 
             // Act
             var typosquattingCheckResult = newService.IsUploadedPackageIdTyposquatting(uploadedPackageId, uploadedPackageOwner);
@@ -72,7 +77,7 @@ public void CheckNotTyposquattingBySameOwnersTest()
                 .Setup(x => x.CanUserTyposquat(It.IsAny<string>(), It.IsAny<string>()))
                 .Returns(true);
            
-            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object);
+            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object, _contentObjectService.Object);
             
             // Act
             var typosquattingCheckResult = newService.IsUploadedPackageIdTyposquatting(uploadedPackageId, uploadedPackageOwner);
@@ -92,7 +97,7 @@ public void CheckTyposquattingByDifferentOwnersTest()
                 .Setup(x => x.CanUserTyposquat(It.IsAny<string>(), It.IsAny<string>()))
                 .Returns(false);
            
-            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object);
+            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object, _contentObjectService.Object);
 
             // Act
             var typosquattingCheckResult = newService.IsUploadedPackageIdTyposquatting(uploadedPackageId, uploadedPackageOwner);
@@ -108,7 +113,7 @@ public void CheckTyposquattingNullUploadedPackageId()
             var uploadedPackageOwner = new User();
             string uploadedPackageId = null;
 
-            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object);
+            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object, _contentObjectService.Object);
             
             // Act
             var exception = Assert.Throws<ArgumentNullException>(
@@ -125,7 +130,7 @@ public void CheckTyposquattingNullUploadedPackageOwner()
             User uploadedPackageOwner = null;
             var uploadedPackageId = "microsoft_netframework_v1";
 
-            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object);
+            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object, _contentObjectService.Object);
   
             // Act
             var exception = Assert.Throws<ArgumentNullException>(
@@ -142,7 +147,7 @@ public void CheckTyposquattingEmptyUploadedPackageId()
             var uploadedPackageOwner = new User();
             var uploadedPackageId = "";
 
-            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object);
+            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object, _contentObjectService.Object);
 
             // Act
             var typosquattingCheckResult = newService.IsUploadedPackageIdTyposquatting(uploadedPackageId, uploadedPackageOwner);
@@ -165,7 +170,7 @@ public void CheckTyposquattingEmptyChecklist()
                 .Setup(x => x.GetAll())
                 .Returns(new List<PackageRegistration>().AsQueryable());
 
-            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object);
+            var newService = new TyposquattingCheckService(_typosquattingUserService.Object, _packageRegistrationRepository.Object, _contentObjectService.Object);
 
             // Act
             var typosquattingCheckResult = newService.IsUploadedPackageIdTyposquatting(uploadedPackageId, uploadedPackageOwner);

Original file line number	Diff line number	Diff line change
`@@ -104,6 +104,7 @@ public static class ContentNames`
`104`	`104`	`public static readonly string LoginDiscontinuationConfiguration = "Login-Discontinuation-Configuration";`
`105`	`105`	`public static readonly string CertificatesConfiguration = "Certificates-Configuration";`
`106`	`106`	`public static readonly string SymbolsConfiguration = "Symbols-Configuration";`
	`107`	`+ public static readonly string TyposquattingConfiguration = "Typosquatting-Configuration";`
`107`	`108`	`}`
`108`	`109`
`109`	`110`	`public static class StatisticsDimensions`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ namespace NuGetGallery`
`10`	`10`	`{`
`11`	`11`	`public class ContentObjectService : IContentObjectService`
`12`	`12`	`{`
`13`		`- private const int RefreshIntervalMinutes = 5;`
	`13`	`+ public static TimeSpan RefreshInterval = TimeSpan.FromMinutes(5);`
`14`	`14`
`15`	`15`	`private readonly IContentService _contentService;`
`16`	`16`
`@@ -21,11 +21,13 @@ public ContentObjectService(IContentService contentService)`
`21`	`21`	`LoginDiscontinuationConfiguration = new LoginDiscontinuationConfiguration();`
`22`	`22`	`CertificatesConfiguration = new CertificatesConfiguration();`
`23`	`23`	`SymbolsConfiguration = new SymbolsConfiguration();`
	`24`	`+ TyposquattingConfiguration = new TyposquattingConfiguration();`
`24`	`25`	`}`
`25`	`26`
`26`	`27`	`public ILoginDiscontinuationConfiguration LoginDiscontinuationConfiguration { get; set; }`
`27`	`28`	`public ICertificatesConfiguration CertificatesConfiguration { get; set; }`
`28`	`29`	`public ISymbolsConfiguration SymbolsConfiguration { get; set; }`
	`30`	`+ public ITyposquattingConfiguration TyposquattingConfiguration { get; set; }`
`29`	`31`
`30`	`32`	`public async Task Refresh()`
`31`	`33`	`{`
`@@ -40,12 +42,16 @@ await Refresh<CertificatesConfiguration>(Constants.ContentNames.CertificatesConf`
`40`	`42`	`SymbolsConfiguration =`
`41`	`43`	`await Refresh<SymbolsConfiguration>(Constants.ContentNames.SymbolsConfiguration) ??`
`42`	`44`	`new SymbolsConfiguration();`
	`45`	`+`
	`46`	`+ TyposquattingConfiguration =`
	`47`	`+ await Refresh<TyposquattingConfiguration>(Constants.ContentNames.TyposquattingConfiguration) ??`
	`48`	`+ new TyposquattingConfiguration();`
`43`	`49`	`}`
`44`	`50`
`45`	`51`	`private async Task<T> Refresh<T>(string contentName)`
`46`	`52`	`where T : class`
`47`	`53`	`{`
`48`		`- var configString = (await _contentService.GetContentItemAsync(contentName, TimeSpan.FromMinutes(RefreshIntervalMinutes)))?.ToString();`
	`54`	`+ var configString = (await _contentService.GetContentItemAsync(contentName, RefreshInterval))?.ToString();`
`49`	`55`	`if (string.IsNullOrEmpty(configString))`
`50`	`56`	`{`
`51`	`57`	`return null;`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ public interface IContentObjectService`
`11`	`11`	`ILoginDiscontinuationConfiguration LoginDiscontinuationConfiguration { get; }`
`12`	`12`	`ICertificatesConfiguration CertificatesConfiguration { get; }`
`13`	`13`	`ISymbolsConfiguration SymbolsConfiguration { get; }`
`14`		`-`
	`14`	`+ ITyposquattingConfiguration TyposquattingConfiguration { get; }`
`15`	`15`	`Task Refresh();`
`16`	`16`	`}`
`17`	`17`	`}`