From 8e06a509d4e00efc61db0745ea3ad132de983b4a Mon Sep 17 00:00:00 2001 From: shishirh Date: Thu, 1 Sep 2016 14:26:45 -0700 Subject: [PATCH 1/2] Fix package push API to return conlfict for existing ID --- src/NuGetGallery/Controllers/ApiController.cs | 7 +++-- .../Controllers/ApiControllerFacts.cs | 31 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/NuGetGallery/Controllers/ApiController.cs b/src/NuGetGallery/Controllers/ApiController.cs index ef4ffc6538..ee1e020afc 100644 --- a/src/NuGetGallery/Controllers/ApiController.cs +++ b/src/NuGetGallery/Controllers/ApiController.cs @@ -289,9 +289,10 @@ await AuditingService.SaveAuditRecord( attemptedPackage: new AuditedPackageIdentifier( nuspec.GetId(), nuspec.GetVersion().ToNormalizedStringSafe()))); - // User can not push this package - return new HttpStatusCodeWithBodyResult(HttpStatusCode.Forbidden, - Strings.ApiKeyNotAuthorized); + // ID is taken by another user + return new HttpStatusCodeWithBodyResult(HttpStatusCode.Conflict, + String.Format(CultureInfo.CurrentCulture, Strings.PackageIdNotAvailable, + nuspec.GetId())); } // Check if a particular Id-Version combination already exists. We eventually need to remove this check. diff --git a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs index b9d8ef1896..e994e0722e 100644 --- a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs @@ -261,6 +261,37 @@ public async Task WillReturnConflictIfAPackageWithTheIdAndSameNormalizedVersionA String.Format(Strings.PackageExistsAndCannotBeModified, "theId", "1.0.42")); } + [Fact] + public async Task WillReturnConflictIfAPackageWithTheIdExistsBelongingToAnotherUser() + { + // Arrange + var user = new User { EmailAddress = "confirmed@email.com" }; + var packageRegistration = new PackageRegistration(); + packageRegistration.Id = "theId"; + var package = new Package(); + package.PackageRegistration = packageRegistration; + package.Version = "1.0.42"; + packageRegistration.Packages.Add(package); + + var controller = new TestableApiController(); + controller.SetCurrentUser(user); + controller.MockPackageService.Setup(p => p.FindPackageRegistrationById(It.IsAny())) + .Returns(packageRegistration); + + var nuGetPackage = TestPackage.CreateTestPackageStream("theId", "1.0.42"); + controller.SetCurrentUser(new User()); + controller.SetupPackageFromInputStream(nuGetPackage); + + // Act + var result = await controller.CreatePackagePut(); + + // Assert + ResultAssert.IsStatusCode( + result, + HttpStatusCode.Conflict, + String.Format(Strings.PackageIdNotAvailable, "theId")); + } + [Fact] public void WillCreateAPackageFromTheNuGetPackage() { From 89a9cf8b685737c865bc35fcd08b807c8e3fb311 Mon Sep 17 00:00:00 2001 From: shishirh Date: Tue, 6 Sep 2016 11:44:01 -0700 Subject: [PATCH 2/2] minor fixes --- src/NuGetGallery/Controllers/ApiController.cs | 2 +- tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/NuGetGallery/Controllers/ApiController.cs b/src/NuGetGallery/Controllers/ApiController.cs index ee1e020afc..b16000ba0c 100644 --- a/src/NuGetGallery/Controllers/ApiController.cs +++ b/src/NuGetGallery/Controllers/ApiController.cs @@ -289,7 +289,7 @@ await AuditingService.SaveAuditRecord( attemptedPackage: new AuditedPackageIdentifier( nuspec.GetId(), nuspec.GetVersion().ToNormalizedStringSafe()))); - // ID is taken by another user + // User cannot push a package to an ID owned by another user. return new HttpStatusCodeWithBodyResult(HttpStatusCode.Conflict, String.Format(CultureInfo.CurrentCulture, Strings.PackageIdNotAvailable, nuspec.GetId())); diff --git a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs index e994e0722e..b881772fc1 100644 --- a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs @@ -266,8 +266,9 @@ public async Task WillReturnConflictIfAPackageWithTheIdExistsBelongingToAnotherU { // Arrange var user = new User { EmailAddress = "confirmed@email.com" }; + var packageId = "theId"; var packageRegistration = new PackageRegistration(); - packageRegistration.Id = "theId"; + packageRegistration.Id = packageId; var package = new Package(); package.PackageRegistration = packageRegistration; package.Version = "1.0.42"; @@ -278,7 +279,7 @@ public async Task WillReturnConflictIfAPackageWithTheIdExistsBelongingToAnotherU controller.MockPackageService.Setup(p => p.FindPackageRegistrationById(It.IsAny())) .Returns(packageRegistration); - var nuGetPackage = TestPackage.CreateTestPackageStream("theId", "1.0.42"); + var nuGetPackage = TestPackage.CreateTestPackageStream(packageId, "1.0.42"); controller.SetCurrentUser(new User()); controller.SetupPackageFromInputStream(nuGetPackage); @@ -289,7 +290,7 @@ public async Task WillReturnConflictIfAPackageWithTheIdExistsBelongingToAnotherU ResultAssert.IsStatusCode( result, HttpStatusCode.Conflict, - String.Format(Strings.PackageIdNotAvailable, "theId")); + String.Format(Strings.PackageIdNotAvailable, packageId)); } [Fact]