Skip to content

Commit efb4c49

Browse files
joelverhagenjoelverhagen
authored
[Hotfix] Add configuration to limit the number of owners and ownership requests (#10688)
1 parent 8399004 commit efb4c49

20 files changed

Lines changed: 261 additions & 67 deletions

‎src/AccountDeleter/Configuration/GalleryConfiguration.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -118,5 +118,7 @@ public string SiteRoot
118118
public string AdminSenderUser { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
119119
public string SupportEmailSiteRoot { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
120120
public int MaxJsonLengthOverride { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
121+
public int MaxOwnerPerPackageRegistration { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
122+
public int MaxOwnerRequestsPerPackageRegistration { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
121123
}
122124
}

‎src/NuGetGallery.Services/Configuration/AppConfiguration.cs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,15 +417,27 @@ public string ExternalBrandingMessage
417417

418418
[DefaultValue(null)]
419419
public int? MinWorkerThreads { get; set; }
420+
420421
[DefaultValue(null)]
421422
public int? MaxWorkerThreads { get; set; }
423+
422424
[DefaultValue(null)]
423425
public int? MinIoThreads { get; set; }
426+
424427
[DefaultValue(null)]
425428
public int? MaxIoThreads { get; set; }
429+
426430
public string InternalMicrosoftTenantKey { get; set; }
431+
427432
public string AdminSenderUser { get; set; }
433+
428434
[DefaultValue(16 * 1024 * 1024)]
429435
public int MaxJsonLengthOverride { get; set; }
436+
437+
[DefaultValue(15)]
438+
public int MaxOwnerPerPackageRegistration { get; set; }
439+
440+
[DefaultValue(3)]
441+
public int MaxOwnerRequestsPerPackageRegistration { get; set; }
430442
}
431-
}
443+
}

‎src/NuGetGallery.Services/Configuration/IAppConfiguration.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,5 +530,15 @@ public interface IAppConfiguration : IMessageServiceConfiguration
530530
/// select places where large JSON response bodies are possible.
531531
/// </summary>
532532
int MaxJsonLengthOverride { get; set; }
533+
534+
/// <summary>
535+
/// The maximum number of owners allowed per package registration. If this limit is reached, no more owners can be added and others must be removed first.
536+
/// </summary>
537+
int MaxOwnerPerPackageRegistration { get; set; }
538+
539+
/// <summary>
540+
/// The maximum number of owner requests allowed per package registration. If this limit is reached, no more requests can be made and other requests must be removed first.
541+
/// </summary>
542+
int MaxOwnerRequestsPerPackageRegistration { get; set; }
533543
}
534-
}
544+
}

‎src/NuGetGallery.Services/PackageManagement/PackageOwnerRequestService.cs‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,28 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
55
using System.Collections.Generic;
66
using System.Data.Entity;
7+
using System.Globalization;
78
using System.Linq;
89
using System.Threading.Tasks;
910
using NuGet.Services.Entities;
11+
using NuGetGallery.Configuration;
1012

1113
namespace NuGetGallery
1214
{
1315
public class PackageOwnerRequestService : IPackageOwnerRequestService
1416
{
1517
private readonly IEntityRepository<PackageOwnerRequest> _packageOwnerRequestRepository;
18+
private readonly IAppConfiguration _appConfiguration;
1619

17-
public PackageOwnerRequestService(IEntityRepository<PackageOwnerRequest> packageOwnerRequestRepository)
20+
public PackageOwnerRequestService(
21+
IEntityRepository<PackageOwnerRequest> packageOwnerRequestRepository,
22+
IAppConfiguration appConfiguration)
1823
{
1924
_packageOwnerRequestRepository = packageOwnerRequestRepository ?? throw new ArgumentNullException(nameof(packageOwnerRequestRepository));
25+
_appConfiguration = appConfiguration ?? throw new ArgumentNullException(nameof(appConfiguration));
2026
}
2127

2228
public PackageOwnerRequest GetPackageOwnershipRequest(PackageRegistration package, User newOwner, string token)
@@ -111,10 +117,19 @@ public async Task<PackageOwnerRequest> AddPackageOwnershipRequest(PackageRegistr
111117
throw new ArgumentNullException(nameof(newOwner));
112118
}
113119

114-
var existingRequest = GetPackageOwnershipRequests(package: package, newOwner: newOwner).FirstOrDefault();
115-
if (existingRequest != null)
120+
var existingRequests = GetPackageOwnershipRequests(package: package).ToList();
121+
var duplicate = existingRequests.FirstOrDefault(x => x.NewOwnerKey == newOwner.Key);
122+
if (duplicate is not null)
116123
{
117-
return existingRequest;
124+
return duplicate;
125+
}
126+
127+
if (existingRequests.Count >= Math.Max(1, _appConfiguration.MaxOwnerRequestsPerPackageRegistration))
128+
{
129+
throw new UserSafeException(string.Format(
130+
CultureInfo.CurrentCulture,
131+
ServicesStrings.MaximumPackageOwnerRequestsReached,
132+
_appConfiguration.MaxOwnerRequestsPerPackageRegistration));
118133
}
119134

120135
var newRequest = new PackageOwnerRequest
@@ -147,4 +162,4 @@ public async Task DeletePackageOwnershipRequest(PackageOwnerRequest request, boo
147162
}
148163
}
149164
}
150-
}
165+
}

‎src/NuGetGallery.Services/PackageManagement/PackageOwnershipManagementService.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Globalization;
67
using System.Linq;
78
using System.Threading.Tasks;
89
using System.Web;
@@ -47,6 +48,16 @@ public PackageOwnershipManagementService(
4748

4849
public async Task AddPackageOwnerWithMessagesAsync(PackageRegistration packageRegistration, User user)
4950
{
51+
if (packageRegistration == null)
52+
{
53+
throw new ArgumentNullException(nameof(packageRegistration));
54+
}
55+
56+
if (packageRegistration.Owners.Count >= Math.Max(_appConfiguration.MaxOwnerPerPackageRegistration, 1))
57+
{
58+
throw new UserSafeException(string.Format(CultureInfo.CurrentCulture, ServicesStrings.MaximumPackageOwnersReached, _appConfiguration.MaxOwnerPerPackageRegistration));
59+
}
60+
5061
await AddPackageOwnerAsync(packageRegistration, user, commitChanges: true);
5162

5263
var packageUrl = _urlHelper.Package(packageRegistration.Id, version: null, relativeUrl: false, supportEmail: true);
@@ -418,4 +429,4 @@ private static bool OwnerHasPermissionsToRemoveFromNamespace(User requestingOwne
418429
return true;
419430
}
420431
}
421-
}
432+
}

‎src/NuGetGallery.Services/PackageManagement/PackageService.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using System.Linq;
99
using System.Threading;
1010
using System.Threading.Tasks;
11-
using System.Web.Helpers;
1211
using Newtonsoft.Json;
1312
using Newtonsoft.Json.Linq;
1413
using NuGet.Frameworks;

‎src/NuGetGallery.Services/ServicesStrings.Designer.cs‎

Lines changed: 28 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/NuGetGallery.Services/ServicesStrings.resx‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,4 +1145,15 @@ The {1} Team</value>
11451145
<data name="ScopeDescription_UnlistDeprecatePackage" xml:space="preserve">
11461146
<value>Unlist, relist, or deprecate package versions</value>
11471147
</data>
1148+
<data name="MaximumPackageOwnersReached" xml:space="preserve">
1149+
<value>The new package owner cannot be added. The package has the maximum number of owners ({0}). An existing owner must be removed to proceed.</value>
1150+
<comment>{0} is the maximum number of owners.</comment>
1151+
</data>
1152+
<data name="MaximumPackageOwnerRequestsReached" xml:space="preserve">
1153+
<value>The package ownership request cannot be saved. The package already has the maximum number of ownership requests ({0}). An existing ownership request must be removed to proceed.</value>
1154+
<comment>{0} is the maximum number of owners.</comment>
1155+
</data>
1156+
<data name="OwnershipRequestNotValid" xml:space="preserve">
1157+
<value>Make sure you clicked on the confirmation URL in the email we sent. It's also possible that the existing owner revoked the request to add you as an owner.</value>
1158+
</data>
11481159
</root>

src/NuGetGallery/Infrastructure/UserSafeException.cs renamed to src/NuGetGallery.Services/UserSafeException.cs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -48,21 +48,7 @@ public static string GetUserSafeMessage(this Exception self)
4848
{
4949
return uvex.UserMessage;
5050
}
51-
return Strings.DefaultUserSafeExceptionMessage;
52-
}
53-
54-
public static void Log(this Exception self)
55-
{
56-
IUserSafeException uvex = self as IUserSafeException;
57-
if (uvex != null)
58-
{
59-
// Log the exception that the User-Visible wrapper marked as to-be-logged
60-
QuietLog.LogHandledException(uvex.LoggedException);
61-
}
62-
else
63-
{
64-
QuietLog.LogHandledException(self);
65-
}
51+
return ServicesStrings.DefaultUserSafeExceptionMessage;
6652
}
6753
}
68-
}
54+
}

‎src/NuGetGallery/Controllers/JsonApiController.cs‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -136,17 +136,24 @@ public async Task<JsonResult> AddPackageOwner(AddPackageOwnerViewModel addOwnerD
136136
{
137137
var packageUrl = Url.Package(model.Package.Id, version: null, relativeUrl: false);
138138

139-
if (model.CurrentUserCanAcceptOnBehalfOfUser)
139+
try
140140
{
141-
await _packageOwnershipManagementService.AddPackageOwnerWithMessagesAsync(model.Package, model.User);
141+
if (model.CurrentUserCanAcceptOnBehalfOfUser)
142+
{
143+
await _packageOwnershipManagementService.AddPackageOwnerWithMessagesAsync(model.Package, model.User);
144+
}
145+
else
146+
{
147+
await _packageOwnershipManagementService.AddPackageOwnershipRequestWithMessagesAsync(
148+
model.Package,
149+
model.CurrentUser,
150+
model.User,
151+
message);
152+
}
142153
}
143-
else
154+
catch (UserSafeException e)
144155
{
145-
await _packageOwnershipManagementService.AddPackageOwnershipRequestWithMessagesAsync(
146-
model.Package,
147-
model.CurrentUser,
148-
model.User,
149-
message);
156+
return Json(new { success = false, message = e.Message }, JsonRequestBehavior.AllowGet);
150157
}
151158

152159
return Json(new
@@ -416,4 +423,4 @@ public ManagePackageOwnerModel(PackageRegistration package, User user, User curr
416423
public string Error { get; }
417424
}
418425
}
419-
}
426+
}

0 commit comments

Comments
 (0)