Allow if conflict with package from same owner (#10163)

erdembayar · web-flow · commit 5c627989d73c · 2024-08-29T09:21:37.000-07:00
diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj
@@ -352,6 +352,7 @@
     <Compile Include="RequestModels\UpdateListedRequest.cs" />
     <Compile Include="Services\ListedVerb.cs" />
     <Compile Include="Services\MissingLicenseValidationMessageV2.cs" />
+    <Compile Include="Services\NormalizedPackageIdInfo.cs" />
     <Compile Include="Services\NullTyposquattingService.cs" />
     <Compile Include="Services\UploadPackageMissingReadme.cs" />
     <Compile Include="Services\MissingLicenseValidationMessage.cs" />
diff --git a/src/NuGetGallery/Services/ITyposquattingCheckListCacheService.cs b/src/NuGetGallery/Services/ITyposquattingCheckListCacheService.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// 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 System;
@@ -17,6 +17,6 @@ public interface ITyposquattingCheckListCacheService
         /// <param name="checkListLength">The length of the checklist for typosquatting</param>
         /// <param name="checkListExpireTime">The expire time for checklist caching</param>
         /// <param name="packageService">The package service for checklist retrieval from database</param>
-        IReadOnlyCollection<string> GetTyposquattingCheckList(int checkListLength, TimeSpan checkListExpireTime, IPackageService packageService);
+        IReadOnlyCollection<NormalizedPackageIdInfo> GetTyposquattingCheckList(int checkListLength, TimeSpan checkListExpireTime, IPackageService packageService);
     }
 }
diff --git a/src/NuGetGallery/Services/NormalizedPackageIdInfo.cs b/src/NuGetGallery/Services/NormalizedPackageIdInfo.cs
@@ -0,0 +1,20 @@
+// 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 System;
+
+namespace NuGetGallery
+{
+    public sealed class NormalizedPackageIdInfo
+    {
+        public string OriginalId { get; }
+        public string NormalizedId { get; }
+
+        public NormalizedPackageIdInfo(string originalId, string normalizedId)
+        {
+            OriginalId = originalId ?? throw new ArgumentNullException(nameof(originalId));
+            NormalizedId = normalizedId ?? throw new ArgumentNullException(nameof(normalizedId));
+        }
+
+    }
+}
diff --git a/src/NuGetGallery/Services/TyposquattingCheckListCacheService.cs b/src/NuGetGallery/Services/TyposquattingCheckListCacheService.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// 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 System;
@@ -12,7 +12,7 @@ public class TyposquattingCheckListCacheService : ITyposquattingCheckListCacheSe
         private readonly object Locker = new object();
         private readonly ITyposquattingServiceHelper _typosquattingServiceHelper;
 
-        private List<string> Cache;
+        private List<NormalizedPackageIdInfo> Cache;
         private DateTime LastRefreshTime;
 
         private int TyposquattingCheckListConfiguredLength;
@@ -24,7 +24,7 @@ public TyposquattingCheckListCacheService(ITyposquattingServiceHelper typosquatt
             _typosquattingServiceHelper = typosquattingServiceHelper;
         }
 
-        public IReadOnlyCollection<string> GetTyposquattingCheckList(int checkListConfiguredLength, TimeSpan checkListExpireTime, IPackageService packageService)
+        public IReadOnlyCollection<NormalizedPackageIdInfo> GetTyposquattingCheckList(int checkListConfiguredLength, TimeSpan checkListExpireTime, IPackageService packageService)
         {
             if (packageService == null)
             {
@@ -54,7 +54,7 @@ public IReadOnlyCollection<string> GetTyposquattingCheckList(int checkListConfig
                             .ToList();
 
                         Cache = cachedPackages
-                            .Select(pr => _typosquattingServiceHelper.NormalizeString(pr))
+                            .Select(pr => new NormalizedPackageIdInfo(originalId: pr, normalizedId: _typosquattingServiceHelper.NormalizeString(pr)))
                             .ToList();
 
                         LastRefreshTime = DateTime.UtcNow;
@@ -75,4 +75,4 @@ private bool IsCheckListCacheExpired(TimeSpan checkListExpireTime)
             return DateTime.UtcNow >= LastRefreshTime.Add(checkListExpireTime);
         }
     }
-}
+}
diff --git a/src/NuGetGallery/Services/TyposquattingService.cs b/src/NuGetGallery/Services/TyposquattingService.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// 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 System;
@@ -60,20 +60,20 @@ public bool IsUploadedPackageIdTyposquatting(string uploadedPackageId, User uplo
 
             var totalTimeStopwatch = Stopwatch.StartNew();
             var checklistRetrievalStopwatch = Stopwatch.StartNew();
-            
+
             // It must be normalized during initial list creation
-            var normalizedPackageIdsCheckList = _typosquattingCheckListCacheService.GetTyposquattingCheckList(checkListConfiguredLength, checkListExpireTimeInHours, _packageService);
+            IReadOnlyCollection<NormalizedPackageIdInfo> normalizedPackageIdsCheckList = _typosquattingCheckListCacheService.GetTyposquattingCheckList(checkListConfiguredLength, checkListExpireTimeInHours, _packageService);
             checklistRetrievalStopwatch.Stop();
 
             _telemetryService.TrackMetricForTyposquattingChecklistRetrievalTime(uploadedPackageId, checklistRetrievalStopwatch.Elapsed);
 
             var algorithmProcessingStopwatch = Stopwatch.StartNew();
             var collisionIds = new ConcurrentBag<string>();
-            Parallel.ForEach(normalizedPackageIdsCheckList, (normalizedPackageId, loopState) =>
+            Parallel.ForEach(normalizedPackageIdsCheckList, (normalizedPackageIdPair, loopState) =>
             {
-                if (_typosquattingServiceHelper.IsDistanceLessThanOrEqualToThresholdWithNormalizedPackageId(uploadedPackageId, normalizedPackageId))
+                if (_typosquattingServiceHelper.IsDistanceLessThanOrEqualToThresholdWithNormalizedPackageId(uploadedPackageId, normalizedPackageIdPair.NormalizedId))
                 {
-                    collisionIds.Add(normalizedPackageId);
+                    collisionIds.Add(normalizedPackageIdPair.OriginalId);
                 }
             });
             algorithmProcessingStopwatch.Stop();
@@ -134,4 +134,4 @@ public bool IsUploadedPackageIdTyposquatting(string uploadedPackageId, User uplo
             return wasUploadBlocked;
         }
     }
-}
+}
diff --git a/tests/NuGetGallery.Facts/Services/TyposquattingCheckListCacheServiceFacts.cs b/tests/NuGetGallery.Facts/Services/TyposquattingCheckListCacheServiceFacts.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// 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 System;
@@ -44,6 +44,8 @@ public async Task WhenGettingCheckListFromMultipleThreadsItIsInitializedOnce()
                 .Returns(PacakgeRegistrationsList);
 
             var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
+            mockTyposquattingServiceHelper.Setup(helper => helper.NormalizeString(It.IsAny<string>()))
+                .Returns((string str) => str.ToLower());
 
             var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
 
@@ -76,6 +78,8 @@ public void WhenExceedExpireTimeRefreshCheckListCache()
                 .Returns(PacakgeRegistrationsList);
 
             var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
+            mockTyposquattingServiceHelper.Setup(helper => helper.NormalizeString(It.IsAny<string>()))
+                  .Returns((string str) => str.ToLower());
             var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
 
             // Act
@@ -99,6 +103,8 @@ public void WhenNotEqualConfiguredLengthRefreshCheckListCache()
                 .Returns(PacakgeRegistrationsList);
 
             var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
+            mockTyposquattingServiceHelper.Setup(helper => helper.NormalizeString(It.IsAny<string>()))
+                .Returns((string str) => str.ToLower());
             var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
 
             // Act
@@ -168,4 +174,4 @@ public void CheckNullPackageService()
             Assert.Equal("packageService", exception.ParamName);
         }
     }
-}
+}
diff --git a/tests/NuGetGallery.Facts/Services/TyposquattingServiceFacts.cs b/tests/NuGetGallery.Facts/Services/TyposquattingServiceFacts.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// 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 System;
@@ -24,7 +24,7 @@ public class TyposquattingServiceFacts
             "System.Spatial"
         };
 
-        private static IQueryable<PackageRegistration> PacakgeRegistrationsList = Enumerable.Range(0, _packageIds.Count()).Select(i =>
+        private static IQueryable<PackageRegistration> PackageRegistrationsList = Enumerable.Range(0, _packageIds.Count()).Select(i =>
                 new PackageRegistration()
                 {
                     Id = _packageIds[i],
@@ -48,7 +48,7 @@ private static ITyposquattingService CreateService(
                 packageService = new Mock<IPackageService>();
                 packageService
                     .Setup(x => x.GetAllPackageRegistrations())
-                    .Returns(PacakgeRegistrationsList);
+                    .Returns(PackageRegistrationsList);
             }
 
             if (contentObjectService == null)
@@ -88,10 +88,15 @@ private static ITyposquattingService CreateService(
 
             if (typosquattingCheckListCacheService == null)
             {
+                List<NormalizedPackageIdInfo> normalizedPackageIdInfos = PackageRegistrationsList
+                    .ToList()
+                    .Select(pr => new NormalizedPackageIdInfo(pr.Id, pr.Id))
+                    .ToList();
+
                 typosquattingCheckListCacheService = new Mock<ITyposquattingCheckListCacheService>();
                 typosquattingCheckListCacheService
                     .Setup(x => x.GetTyposquattingCheckList(It.IsAny<int>(), It.IsAny<TimeSpan>(), It.IsAny<IPackageService>()))
-                    .Returns(PacakgeRegistrationsList.Select(pr => pr.Id).ToList());
+                    .Returns(normalizedPackageIdInfos);
             }
 
             return new TyposquattingService(
@@ -220,7 +225,7 @@ public void CheckTyposquattingEmptyChecklist()
             var mockTyposquattingCheckListCacheService = new Mock<ITyposquattingCheckListCacheService>();
             mockTyposquattingCheckListCacheService
                 .Setup(x => x.GetTyposquattingCheckList(It.IsAny<int>(), It.IsAny<TimeSpan>(), It.IsAny<IPackageService>()))
-                .Returns(new List<string>());
+                .Returns(new List<NormalizedPackageIdInfo>());
 
             var newService = CreateService(packageService: mockPackageService, typosquattingCheckListCacheService: mockTyposquattingCheckListCacheService);
 
@@ -315,6 +320,83 @@ public void CheckIsTyposquattingBlockUserNotEnabled()
                 .Verify(f => f.IsTyposquattingEnabled(_uploadedPackageOwner), Times.Once);
         }
 
+        [Fact]
+        public void CheckIsTyposquattingBlockDifferentOwnersUploadBlocked()
+        {
+            // Arrange
+            var uploadedPackageId = "Microsoft_NetFramework_v1";
+            string conflictingPackageId = "microsoft_netframework_v1";
+            User conflictingPackageOwner = PackageRegistrationsList
+                .Single(pr => pr.Id == conflictingPackageId)
+                .Owners
+                .First();
+
+            // Make sure they have different owners.
+            Assert.NotEqual(_uploadedPackageOwner.Key, conflictingPackageOwner.Key);
+
+            var featureFlagService = new Mock<IFeatureFlagService>();
+            featureFlagService
+                .Setup(f => f.IsTyposquattingEnabled())
+                .Returns(true);
+            featureFlagService
+                .Setup(f => f.IsTyposquattingEnabled(_uploadedPackageOwner))
+                .Returns(true);
+
+            var newService = CreateService(featureFlagService: featureFlagService);
+
+            // Act
+
+            var typosquattingCheckResult = newService.IsUploadedPackageIdTyposquatting(uploadedPackageId, _uploadedPackageOwner, out List<string> typosquattingCheckCollisionIds);
+
+            // Assert
+            Assert.True(typosquattingCheckResult);
+            Assert.Single(typosquattingCheckCollisionIds);
+            Assert.Equal(conflictingPackageId, typosquattingCheckCollisionIds[0]);
+
+            featureFlagService
+                .Verify(f => f.IsTyposquattingEnabled(), Times.Once);
+            featureFlagService
+                .Verify(f => f.IsTyposquattingEnabled(_uploadedPackageOwner), Times.Once);
+        }
+
+        [Fact]
+        public void CheckIsTyposquattingBlockSameOwnerUploadNotBlocked()
+        {
+            // Arrange
+            var uploadedPackageId = "Microsoft_NetFramework_v1";
+            string conflictingPackageId = "microsoft_netframework_v1";
+
+            var featureFlagService = new Mock<IFeatureFlagService>();
+
+            // Use same owner for both packages.
+            User samePackageOwner =  PackageRegistrationsList
+                .Single(pr => pr.Id == conflictingPackageId)
+                .Owners
+                .First();
+
+            featureFlagService
+                .Setup(f => f.IsTyposquattingEnabled())
+                .Returns(true);
+            featureFlagService
+                .Setup(f => f.IsTyposquattingEnabled(samePackageOwner))
+                .Returns(true);
+
+            var newService = CreateService(featureFlagService: featureFlagService);
+
+            // Act
+
+            var typosquattingCheckResult = newService.IsUploadedPackageIdTyposquatting(uploadedPackageId, samePackageOwner, out List<string> typosquattingCheckCollisionIds);
+
+            // Assert
+            Assert.False(typosquattingCheckResult);
+            Assert.Empty(typosquattingCheckCollisionIds);
+
+            featureFlagService
+                .Verify(f => f.IsTyposquattingEnabled(), Times.Once);
+            featureFlagService
+                .Verify(f => f.IsTyposquattingEnabled(samePackageOwner), Times.Once);
+        }
+
         [Fact]
         public void CheckTelemetryServiceLogOriginalUploadedPackageId()
         {
@@ -347,4 +429,4 @@ public void CheckTelemetryServiceLogOriginalUploadedPackageId()
                 Times.Once);
         }
     }
-}
+}

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// Copyright (c) .NET Foundation. All rights reserved.`
	`1`	`+// Copyright (c) .NET Foundation. All rights reserved.`
`2`	`2`	`// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.`
`3`	`3`
`4`	`4`	`using System;`
`@@ -17,6 +17,6 @@ public interface ITyposquattingCheckListCacheService`
`17`	`17`	`/// <param name="checkListLength">The length of the checklist for typosquatting</param>`
`18`	`18`	`/// <param name="checkListExpireTime">The expire time for checklist caching</param>`
`19`	`19`	`/// <param name="packageService">The package service for checklist retrieval from database</param>`
`20`		`- IReadOnlyCollection<string> GetTyposquattingCheckList(int checkListLength, TimeSpan checkListExpireTime, IPackageService packageService);`
	`20`	`+ IReadOnlyCollection<NormalizedPackageIdInfo> GetTyposquattingCheckList(int checkListLength, TimeSpan checkListExpireTime, IPackageService packageService);`
`21`	`21`	`}`
`22`	`22`	`}`