Skip to content

Commit fd7fe13

Browse files
authored
Add restricted API for soft deleting packages (#8290)
Address NuGet/Engineering#3512
1 parent 2db92da commit fd7fe13

9 files changed

Lines changed: 327 additions & 4 deletions

File tree

src/NuGetGallery.Services/Configuration/FeatureFlagService.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public class FeatureFlagService : IFeatureFlagService
4040
private const string PackageRenamesFeatureName = GalleryPrefix + "PackageRenames";
4141
private const string EmbeddedReadmeFlightName = GalleryPrefix + "EmbeddedReadmes";
4242
private const string LicenseMdRenderingFlightName = GalleryPrefix + "LicenseMdRendering";
43+
private const string DeletePackageApiFlightName = GalleryPrefix + "DeletePackageApi";
4344

4445
private const string ODataV1GetAllNonHijackedFeatureName = GalleryPrefix + "ODataV1GetAllNonHijacked";
4546
private const string ODataV1GetAllCountNonHijackedFeatureName = GalleryPrefix + "ODataV1GetAllCountNonHijacked";
@@ -284,5 +285,10 @@ public bool IsODataV2SearchCountNonHijackedEnabled()
284285
{
285286
return _client.IsEnabled(ODataV2SearchCountNonHijackedFeatureName, defaultValue: true);
286287
}
288+
289+
public bool IsDeletePackageApiEnabled(User user)
290+
{
291+
return _client.IsEnabled(DeletePackageApiFlightName, user, defaultValue: false);
292+
}
287293
}
288294
}

src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,5 +225,10 @@ public interface IFeatureFlagService
225225
/// Whether the /Search()/$count endpoint is enabled for non-hijacked queries for the V2 OData API.
226226
/// </summary>
227227
bool IsODataV2SearchCountNonHijackedEnabled();
228+
229+
/// <summary>
230+
/// Whether or not the user can delete a package through the API.
231+
/// </summary>
232+
bool IsDeletePackageApiEnabled(User user);
228233
}
229234
}

src/NuGetGallery/App_Data/Files/Content/flags.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@
7272
"SiteAdmins": false,
7373
"Accounts": [],
7474
"Domains": []
75+
},
76+
"NuGetGallery.DeletePackageApi": {
77+
"All": false,
78+
"SiteAdmins": false,
79+
"Accounts": [],
80+
"Domains": []
7581
}
7682
}
7783
}

src/NuGetGallery/Controllers/ApiController.cs

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
using NuGetGallery.Authentication;
2626
using NuGetGallery.Configuration;
2727
using NuGetGallery.Filters;
28+
using NuGetGallery.Helpers;
2829
using NuGetGallery.Infrastructure.Authentication;
2930
using NuGetGallery.Infrastructure.Mail.Messages;
3031
using NuGetGallery.Packaging;
@@ -37,6 +38,7 @@ public partial class ApiController
3738
: AppController
3839
{
3940
private const string NuGetExeUrl = "https://dist.nuget.org/win-x86-commandline/v2.8.6/nuget.exe";
41+
private const string PackageDeleteApiReason = "Deleted via API";
4042
private readonly IAutocompletePackageIdsQuery _autocompletePackageIdsQuery;
4143
private readonly IAutocompletePackageVersionsQuery _autocompletePackageVersionsQuery;
4244

@@ -861,7 +863,7 @@ private static ActionResult BadRequestForExceptionMessage(Exception ex)
861863
[ApiAuthorize]
862864
[ApiScopeRequired(NuGetScopes.PackageUnlist)]
863865
[ActionName("DeletePackageApi")]
864-
public virtual async Task<ActionResult> DeletePackage(string id, string version)
866+
public virtual async Task<ActionResult> DeletePackage(string id, string version, DeletePackageApiRequest request = null)
865867
{
866868
var package = PackageService.FindPackageByIdAndVersionStrict(id, version);
867869
if (package == null)
@@ -870,7 +872,8 @@ public virtual async Task<ActionResult> DeletePackage(string id, string version)
870872
HttpStatusCode.NotFound, string.Format(CultureInfo.CurrentCulture, Strings.PackageWithIdAndVersionNotFound, id, version));
871873
}
872874

873-
// Check if the current user's scopes allow listing/unlisting the current package ID
875+
// Check if the current user's scopes allow listing/unlisting the current package ID. This scope is used for
876+
// both unlisting and soft deletion.
874877
var apiScopeEvaluationResult = EvaluateApiScope(ActionsRequiringPermissions.UnlistOrRelistPackage, package.PackageRegistration, NuGetScopes.PackageUnlist);
875878
if (!apiScopeEvaluationResult.IsSuccessful())
876879
{
@@ -884,7 +887,55 @@ public virtual async Task<ActionResult> DeletePackage(string id, string version)
884887
string.Format(CultureInfo.CurrentCulture, Strings.PackageIsLocked, package.PackageRegistration.Id));
885888
}
886889

887-
await PackageUpdateService.MarkPackageUnlistedAsync(package);
890+
var currentUser = GetCurrentUser();
891+
DeletePackageAction action;
892+
if (request?.Type == null)
893+
{
894+
action = DeletePackageAction.Unlist;
895+
}
896+
else if (!request.TryParseAction(out action))
897+
{
898+
return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, Strings.DeletePackage_InvalidDeleteType);
899+
}
900+
901+
switch (action)
902+
{
903+
case DeletePackageAction.Unlist:
904+
await PackageUpdateService.MarkPackageUnlistedAsync(package);
905+
break;
906+
907+
case DeletePackageAction.SoftDelete:
908+
// A user can only delete packages if there are enabled in the flight and are NOT an admin. The
909+
// admin restriction is for safety reasons. This endpoint is currently intended just for test data
910+
// clean-up which can be done with a very limited, non-admin API key.
911+
if (!FeatureFlagService.IsDeletePackageApiEnabled(currentUser)
912+
|| currentUser.IsAdministrator)
913+
{
914+
return new HttpStatusCodeWithBodyResult(HttpStatusCode.Forbidden, Strings.DeletePackage_NotAllowed);
915+
}
916+
917+
// Apply a hard stop on the deletion if there are too many downloads on this package version. We
918+
// using configuration here and use a constant to mimize the risk of misconfiguration allowing too
919+
// many deletes.
920+
const int downloadCountLimit = 1000;
921+
if (package.DownloadCount >= downloadCountLimit)
922+
{
923+
return new HttpStatusCodeWithBodyResult(
924+
HttpStatusCode.Forbidden,
925+
string.Format(CultureInfo.CurrentCulture, Strings.DeletePackage_TooManyDownloads, downloadCountLimit));
926+
}
927+
928+
await PackageDeleteService.SoftDeletePackagesAsync(
929+
new[] { package },
930+
currentUser,
931+
PackageDeleteApiReason,
932+
Strings.AutomatedPackageDeleteSignature);
933+
break;
934+
935+
default:
936+
throw new NotSupportedException($"The delete package action '{action}' is not supported.");
937+
}
938+
888939
return new EmptyResult();
889940
}
890941

src/NuGetGallery/NuGetGallery.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@
303303
<DependentUpon>202007220027197_AddEmbeddedReadmeTypeColumn.cs</DependentUpon>
304304
</Compile>
305305
<Compile Include="Modules\CookieComplianceHttpModule.cs" />
306+
<Compile Include="RequestModels\DeletePackagesApiRequest.cs" />
306307
<Compile Include="Services\IMarkdownService.cs" />
307308
<Compile Include="Services\IPackageMetadataValidationService.cs" />
308309
<Compile Include="Services\MarkdownService.cs" />
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Linq;
7+
8+
namespace NuGetGallery
9+
{
10+
public enum DeletePackageAction
11+
{
12+
Unlist,
13+
SoftDelete,
14+
}
15+
16+
public class DeletePackageApiRequest
17+
{
18+
/// <summary>
19+
/// We have a clunky string to enum mapping here because the default behavior of ASP.NET MVC model binding
20+
/// is not configurable enough for our purposes. If the model property type is an enum, an invalid incoming
21+
/// request value is set to be the default enum value, rather than rejecting this request. This behavior is too
22+
/// permissive.
23+
/// </summary>
24+
private static readonly IReadOnlyDictionary<string, DeletePackageAction> Actions = Enum
25+
.GetValues(typeof(DeletePackageAction))
26+
.Cast<DeletePackageAction>()
27+
.ToDictionary(x => x.ToString(), x => x, StringComparer.OrdinalIgnoreCase);
28+
29+
public bool TryParseAction(out DeletePackageAction action)
30+
{
31+
return DeletePackageApiRequest.Actions.TryGetValue(Type, out action);
32+
}
33+
34+
public string Type { get; set; }
35+
}
36+
}

src/NuGetGallery/Strings.Designer.cs

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/NuGetGallery/Strings.resx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1199,4 +1199,14 @@ The {1} Team</value>
11991199
<data name="UploadPackage_LearnMore_LicenseUrlDreprecation" xml:space="preserve">
12001200
<value>Learn more about license URL deprecation</value>
12011201
</data>
1202+
<data name="DeletePackage_NotAllowed" xml:space="preserve">
1203+
<value>Deleting a package through the API is not enabled for your user account.</value>
1204+
</data>
1205+
<data name="DeletePackage_InvalidDeleteType" xml:space="preserve">
1206+
<value>The provided Type in the delete request is not valid.</value>
1207+
</data>
1208+
<data name="DeletePackage_TooManyDownloads" xml:space="preserve">
1209+
<value>The provided package has more than {0} downloads and therefore cannot be deleted via API.</value>
1210+
<comment>{0} is the download count limit.</comment>
1211+
</data>
12021212
</root>

0 commit comments

Comments
 (0)