Forbidden status and prompted to enter credentials when attempt to push new package with same ID as an existing one

[shishirx34]

Issue reported on Gallery NuGet/NuGetGallery#3218 by @AnotherSadGit:

I was attempting to push a new package to Nuget and each time was getting a Forbidden status back, and was prompted for my credentials to connect to the site. After hours of trying to figure out the problem it turned out the issue was the ID of the new package I was attempting to push duplicated the ID of an existing package from a different author.

The error message and the prompt for credentials seems misleading in this situation. I suggest an explicit error message, if possible, along the lines of "Unable to push the package, ID duplicates the ID of an existing package."

I investigated and I see that the command line nuget.exe(latest: 3.4.4) does indeed return forbidden for duplicate ID package push.

e:>nuget push basetestpackage.1.0.0.nupkg -Source https://dev.nugettest.org -ApiKey <redacted>
Pushing basetestpackage.1.0.0.nupkg to 'https://dev.nugettest.org'...
  PUT https://dev.nugettest.org/api/v2/package/
  Forbidden https://dev.nugettest.org/api/v2/package/ 804ms
Please provide credentials for: https://dev.nugettest.org
UserName: <username>
Password: <password>
  PUT https://dev.nugettest.org/api/v2/package/
  Forbidden https://dev.nugettest.org/api/v2/package/ 584ms
Please provide credentials for: https://dev.nugettest.org
UserName:

[emgarten]

@shishirx34 isn't the bug here that gallery returns a 403 forbidden? In this case the request from the client was not valid as the status code indicates, it is more of a 400 bad request, or 409 conflict.

I would expect the gallery to return a non-credential related error code for a duplicate package upload.

[shishirx34]

It is possible that it could be Gallery issue. I wasn't sure if the client masked the message hence wanted to check here first. I will open the other bug on gallery.

[shishirx34]

@emgarten - I looked at this some more and it looks like Gallery is returning a Forbidden code, however there is also the following message returned along with it.

The specified API key is invalid, has expired, or does not have permission to access the specified package.

From the code it looks like the Forbidden code was by design (@xavierdecoster @skofman1 - Thoughts?).

     // User can not push this package
     return new HttpStatusCodeWithBodyResult(HttpStatusCode.Forbidden,
            Strings.ApiKeyNotAuthorized);

Perhaps client should surface this error message(although in the case above it doesn't make it clear that there is ID collision)?

[emgarten]

The gallery may not even be getting to the id collision in that case.

From the client side prompting for a 403 is by design for scenarios where the wrong use was specified. Printing out the message may be helpful to user here.

[shishirx34]

I am sorry if I wasn't clear earlier. The API does detect the collision, it looks up the package registration by ID and if one is found, verifies if the package's owner is the current user trying to push(PUT request) the package. Which is when it detects that it is not, shown above, and returns a Forbidden result.

https://github.com/NuGet/NuGetGallery/blob/master/src/NuGetGallery/Controllers/ApiController.cs#L293

From the code it looks like this behavior is by design. @maartenba or @xavierdecoster would be able to elaborate more on if it indeed is the desired behavior.

[xavierdecoster]

The gallery checks whether the pushing user is actually an owner of the package registration. If it is not the case, then that operation is forbidden. It is not necessarily a conflict as it may concern a new version of the package being pushed. As there's a meaningful message returned in the response, it may be useful to show it in the client?

[yishaigalatzer]

The message is incorrect in this case, and the status code is also potentially incorrect.

This is not a forbidden case, as different user credentials will not allow for the request to go through, 409 seems much more appropriate (and the error message should indicate that the id/version already exists)

[rrelyea]

There was a server change. Please test the experience at that point and ensure the client is providing a good error experience.

[shishirx34]

FYI, with the change to the server to return 409 following is the response when pushing duplicate ID package for a user whose ApiKey does not match, current client behavior:

E:\Nuget\NuGetGallery>..\nuget.exe push "..\Sample Packages\BaseTestPackage.2.2.0.nupkg" -ApiKey <redacted> -Source http://nuget.localtest.me/
Pushing BaseTestPackage.2.2.0.nupkg to 'http://nuget.localtest.me/'...
  PUT http://nuget.localtest.me/api/v2/package/
  Conflict http://nuget.localtest.me/api/v2/package/ 697ms
Response status code does not indicate success: 409 (The package ID 'BaseTestPackage' is not available.).

[yishaigalatzer]

The error message is wrong, can we please correct it?

We should have the following errors for the following scenario

1. The apikey doesn't match a user that owns the id - Keep the current error message
2. The apikey does match but the version exists - The package with ID 'BaseTestPackage' and version 'XXX.XXX' already exists on the server and cannot be overriden

[shishirx34]

1. The apikey doesn't match a user that owns the id - Keep the current error message

Talked offline with Yishai. The bug addresses the above point.

2. The apikey does match but the version exists - The package with ID 'BaseTestPackage' and version
   'XXX.XXX' already exists on the server and cannot be overriden

This has been working correctly. We do have the check for ID and Version after the user permission check which throws the message described here.

[shishirx34]

The server side fix should be live in production: NuGet/NuGetGallery#3196