Skip to content

Commit ac9e62d

Browse files
xavierdecosterxavierdecoster
authored
Restore auto-add co-owner feature (#6289)
1 parent a9379ee commit ac9e62d

45 files changed

Lines changed: 1633 additions & 229 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/NuGetGallery.Core/Services/CoreMessageService.cs‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,24 @@ public CoreMessageService(IMailSender mailSender, ICoreMessageServiceConfigurati
2828
public IMailSender MailSender { get; protected set; }
2929
public ICoreMessageServiceConfiguration CoreConfiguration { get; protected set; }
3030

31-
public void SendPackageAddedNotice(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl)
31+
public void SendPackageAddedNotice(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl, IEnumerable<string> warningMessages = null)
3232
{
33-
string subject = $"[{CoreConfiguration.GalleryOwner.DisplayName}] Package published - {package.PackageRegistration.Id} {package.Version}";
33+
bool hasWarnings = warningMessages != null && warningMessages.Any();
34+
35+
string subject;
36+
var warningMessagesPlaceholder = string.Empty;
37+
if (hasWarnings)
38+
{
39+
subject = $"[{CoreConfiguration.GalleryOwner.DisplayName}] Package published with warnings - {package.PackageRegistration.Id} {package.Version}";
40+
warningMessagesPlaceholder = Environment.NewLine + string.Join(Environment.NewLine, warningMessages);
41+
}
42+
else
43+
{
44+
subject = $"[{CoreConfiguration.GalleryOwner.DisplayName}] Package published - {package.PackageRegistration.Id} {package.Version}";
45+
}
46+
3447
string body = $@"The package [{package.PackageRegistration.Id} {package.Version}]({packageUrl}) was recently published on {CoreConfiguration.GalleryOwner.DisplayName} by {package.User.Username}. If this was not intended, please [contact support]({packageSupportUrl}).
48+
{warningMessagesPlaceholder}
3549
3650
-----------------------------------------------
3751
<em style=""font-size: 0.8em;"">
@@ -54,6 +68,30 @@ [change your email notification settings]({emailSettingsUrl}).
5468
}
5569
}
5670

71+
public void SendPackageAddedWithWarningsNotice(Package package, string packageUrl, string packageSupportUrl, IEnumerable<string> warningMessages)
72+
{
73+
var subject = $"[{CoreConfiguration.GalleryOwner.DisplayName}] Package pushed with warnings - {package.PackageRegistration.Id} {package.Version}";
74+
var warningMessagesPlaceholder = Environment.NewLine + string.Join(Environment.NewLine, warningMessages);
75+
76+
string body = $@"The package [{package.PackageRegistration.Id} {package.Version}]({packageUrl}) was recently pushed to {CoreConfiguration.GalleryOwner.DisplayName} by {package.User.Username}. If this was not intended, please [contact support]({packageSupportUrl}).
77+
{warningMessagesPlaceholder}
78+
";
79+
80+
using (var mailMessage = new MailMessage())
81+
{
82+
mailMessage.Subject = subject;
83+
mailMessage.Body = body;
84+
mailMessage.From = CoreConfiguration.GalleryNoReplyAddress;
85+
86+
AddOwnersSubscribedToPackagePushedNotification(package.PackageRegistration, mailMessage);
87+
88+
if (mailMessage.To.Any())
89+
{
90+
SendMessage(mailMessage);
91+
}
92+
}
93+
}
94+
5795
public void SendPackageValidationFailedNotice(Package package, PackageValidationSet validationSet, string packageUrl, string packageSupportUrl, string announcementsUrl, string twitterUrl)
5896
{
5997
var validationIssues = validationSet.GetValidationIssues();

‎src/NuGetGallery.Core/Services/ICoreMessageService.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
// 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

4+
using System.Collections.Generic;
45
using NuGet.Services.Validation;
56

67
namespace NuGetGallery.Services
78
{
89
public interface ICoreMessageService
910
{
10-
void SendPackageAddedNotice(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl);
11+
void SendPackageAddedNotice(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl, IEnumerable<string> warningMessages = null);
12+
void SendPackageAddedWithWarningsNotice(Package package, string packageUrl, string packageSupportUrl, IEnumerable<string> warningMessages);
1113
void SendPackageValidationFailedNotice(Package package, PackageValidationSet validationSet, string packageUrl, string packageSupportUrl, string announcementsUrl, string twitterUrl);
1214
void SendValidationTakingTooLongNotice(Package package, string packageUrl);
1315
}

‎src/NuGetGallery/App_Start/DefaultDependenciesModule.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,10 @@ protected override void Load(ContainerBuilder builder)
395395

396396
RegisterCookieComplianceService(builder, configuration, diagnosticsService);
397397

398+
builder.RegisterType<MicrosoftTeamSubscription>()
399+
.AsSelf()
400+
.InstancePerLifetimeScope();
401+
398402
// todo: bind all package curators by convention
399403
builder.RegisterType<WebMatrixPackageCurator>()
400404
.AsSelf()

‎src/NuGetGallery/Controllers/ApiController.cs‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,13 @@ public async virtual Task<ActionResult> CreatePackageVerificationKeyAsync(string
274274
[ActionName("VerifyPackageKey")]
275275
public async virtual Task<ActionResult> VerifyPackageKeyAsync(string id, string version)
276276
{
277-
var policyResult = await SecurityPolicyService.EvaluateUserPoliciesAsync(SecurityPolicyAction.PackageVerify, HttpContext);
277+
var user = GetCurrentUser();
278+
var policyResult = await SecurityPolicyService.EvaluateUserPoliciesAsync(SecurityPolicyAction.PackageVerify, user, HttpContext);
278279
if (!policyResult.Success)
279280
{
280281
return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, policyResult.ErrorMessage);
281282
}
282283

283-
var user = GetCurrentUser();
284284
var credential = user.GetCurrentApiKeyCredential(User.Identity);
285285

286286
var result = await VerifyPackageKeyInternalAsync(user, credential, id, version);
@@ -506,15 +506,17 @@ private async Task<ActionResult> CreatePackageInternal()
506506

507507
try
508508
{
509-
var policyResult = await SecurityPolicyService.EvaluateUserPoliciesAsync(SecurityPolicyAction.PackagePush, HttpContext);
509+
var securityPolicyAction = SecurityPolicyAction.PackagePush;
510+
511+
// Get the user
512+
var currentUser = GetCurrentUser();
513+
514+
var policyResult = await SecurityPolicyService.EvaluateUserPoliciesAsync(securityPolicyAction, currentUser, HttpContext);
510515
if (!policyResult.Success)
511516
{
512517
return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, policyResult.ErrorMessage);
513518
}
514519

515-
// Get the user
516-
var currentUser = GetCurrentUser();
517-
518520
using (var packageStream = ReadPackageFromRequest())
519521
{
520522
try
@@ -654,6 +656,18 @@ await PackageDeleteService.HardDeletePackagesAsync(
654656
packageStreamMetadata,
655657
owner,
656658
currentUser);
659+
660+
var packagePolicyResult = await SecurityPolicyService.EvaluatePackagePoliciesAsync(
661+
securityPolicyAction,
662+
package,
663+
currentUser,
664+
owner,
665+
HttpContext);
666+
667+
if (!packagePolicyResult.Success)
668+
{
669+
return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, packagePolicyResult.ErrorMessage);
670+
}
657671

658672
// Perform validations that require the package already being in the entity context.
659673
var afterValidationResult = await PackageUploadService.ValidateAfterGeneratePackageAsync(
@@ -702,7 +716,17 @@ await AuditingService.SaveAuditRecordAsync(
702716
MessageService.SendPackageAddedNotice(package,
703717
Url.Package(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false),
704718
Url.ReportPackage(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false),
705-
Url.AccountSettings(relativeUrl: false));
719+
Url.AccountSettings(relativeUrl: false),
720+
packagePolicyResult.WarningMessages);
721+
}
722+
// Emit warning messages if any
723+
else if (packagePolicyResult.HasWarnings)
724+
{
725+
// Notify user of push unless async validation in blocking mode is used
726+
MessageService.SendPackageAddedWithWarningsNotice(package,
727+
Url.Package(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false),
728+
Url.ReportPackage(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false),
729+
packagePolicyResult.WarningMessages);
706730
}
707731

708732
TelemetryService.TrackPackagePushEvent(package, currentUser, User.Identity);

‎src/NuGetGallery/Controllers/PackagesController.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,6 +1617,18 @@ public virtual async Task<JsonResult> VerifyPackage(VerifyPackageRequest formDat
16171617

16181618
return Json(HttpStatusCode.BadRequest, new[] { ex.Message });
16191619
}
1620+
1621+
var packagePolicyResult = await _securityPolicyService.EvaluatePackagePoliciesAsync(
1622+
SecurityPolicyAction.PackagePush,
1623+
package,
1624+
currentUser,
1625+
owner,
1626+
HttpContext);
1627+
1628+
if (!packagePolicyResult.Success)
1629+
{
1630+
return Json(HttpStatusCode.BadRequest, new[] { packagePolicyResult.ErrorMessage });
1631+
}
16201632

16211633
// Perform validations that require the package already being in the entity context.
16221634
var afterValidationResult = await _packageUploadService.ValidateAfterGeneratePackageAsync(

‎src/NuGetGallery/NuGetGallery.csproj‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,13 @@
376376
</Compile>
377377
<Compile Include="Security\AutomaticOverwriteRequiredSignerPolicy.cs" />
378378
<Compile Include="Security\ControlRequiredSignerPolicy.cs" />
379+
<Compile Include="Security\MicrosoftTeamSubscription.cs" />
380+
<Compile Include="Security\PackageSecurityPolicyEvaluationContext.cs" />
381+
<Compile Include="Security\PackageSecurityPolicyHandler.cs" />
379382
<Compile Include="Security\RequiredSignerPolicy.cs" />
383+
<Compile Include="Security\RequirePackageMetadataCompliancePolicy.cs" />
380384
<Compile Include="Security\RequireOrganizationTenantPolicy.cs" />
385+
<Compile Include="Security\SecurityPolicyHandler.cs" />
381386
<Compile Include="Services\AccountDeletionOrphanPackagePolicy.cs" />
382387
<Compile Include="Services\ActionOnNewPackageContext.cs" />
383388
<Compile Include="Services\ActionRequiringAccountPermissions.cs" />

‎src/NuGetGallery/Security/ISecurityPolicyService.cs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,12 @@ public interface ISecurityPolicyService
5656
/// Evaluate any security policies that may apply to the current context.
5757
/// </summary>
5858
/// <param name="action">Security policy action.</param>
59+
/// <param name="user">The user for which to evaluate security policies.</param>
5960
/// <param name="httpContext">Http context.</param>
6061
/// <returns>A task that represents the asynchronous operation.
6162
/// The task result (<see cref="Task{TResult}.Result" />) returns a <see cref="SecurityPolicyResult" />
6263
/// instance.</returns>
63-
Task<SecurityPolicyResult> EvaluateUserPoliciesAsync(SecurityPolicyAction action, HttpContextBase httpContext);
64+
Task<SecurityPolicyResult> EvaluateUserPoliciesAsync(SecurityPolicyAction action, User user, HttpContextBase httpContext);
6465

6566
/// <summary>
6667
/// Evaluate any organization security policies for the specified account.
@@ -72,5 +73,18 @@ public interface ISecurityPolicyService
7273
/// The task result (<see cref="Task{TResult}.Result" />) returns a <see cref="SecurityPolicyResult" />
7374
/// instance.</returns>
7475
Task<SecurityPolicyResult> EvaluateOrganizationPoliciesAsync(SecurityPolicyAction action, Organization organization, User account);
76+
77+
/// <summary>
78+
/// Evaluate any package security policies that may apply to the current context.
79+
/// </summary>
80+
/// <param name="action">Security policy action.</param>
81+
/// <param name="package">The package to evaluate.</param>
82+
/// <param name="currentUser">The current user.</param>
83+
/// <param name="owner">The package owner.</param>
84+
/// <param name="httpContext">Http context.</param>
85+
/// <returns>A task that represents the asynchronous operation.
86+
/// The task result (<see cref="Task{TResult}.Result" />) returns a <see cref="SecurityPolicyResult"/>
87+
/// instance.</returns>
88+
Task<SecurityPolicyResult> EvaluatePackagePoliciesAsync(SecurityPolicyAction action, Package package, User currentUser, User owner, HttpContextBase httpContext);
7589
}
7690
}

‎src/NuGetGallery/Security/MicrosoftTeamSubscription.cs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
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.Threading.Tasks;
7+
8+
namespace NuGetGallery.Security
9+
{
10+
public class MicrosoftTeamSubscription : IUserSecurityPolicySubscription
11+
{
12+
private Lazy<List<UserSecurityPolicy>> _policies = new Lazy<List<UserSecurityPolicy>>(InitializePoliciesList, isThreadSafe: true);
13+
14+
internal const string MicrosoftUsername = "Microsoft";
15+
internal const string Name = "MicrosoftTeamSubscription";
16+
17+
public string SubscriptionName => Name;
18+
19+
public MicrosoftTeamSubscription()
20+
{
21+
}
22+
23+
public IEnumerable<UserSecurityPolicy> Policies => _policies.Value;
24+
25+
public Task OnSubscribeAsync(UserSecurityPolicySubscriptionContext context)
26+
{
27+
// Todo:
28+
// Maybe we should enumerate through the user's packages and add Microsoft as a package owner if the package passes the metadata requirements when a user is onboarded to this policy.
29+
// We should also unlock the package if it is locked as part of adding Microsoft as co-owner.
30+
return Task.CompletedTask;
31+
}
32+
33+
public Task OnUnsubscribeAsync(UserSecurityPolicySubscriptionContext context)
34+
{
35+
return Task.CompletedTask;
36+
}
37+
38+
private static List<UserSecurityPolicy> InitializePoliciesList()
39+
{
40+
return new List<UserSecurityPolicy>()
41+
{
42+
RequirePackageMetadataCompliancePolicy.CreatePolicy(
43+
Name,
44+
MicrosoftUsername,
45+
allowedCopyrightNotices: new string[]
46+
{
47+
"(c) Microsoft Corporation. All rights reserved.",
48+
"© Microsoft Corporation. All rights reserved."
49+
},
50+
isLicenseUrlRequired: true,
51+
isProjectUrlRequired: true,
52+
errorMessageFormat: Strings.SecurityPolicy_RequireMicrosoftPackageMetadataComplianceForPush)
53+
};
54+
}
55+
}
56+
}

‎src/NuGetGallery/Security/PackageSecurityPolicyEvaluationContext.cs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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.Web;
7+
8+
namespace NuGetGallery.Security
9+
{
10+
public class PackageSecurityPolicyEvaluationContext : UserSecurityPolicyEvaluationContext
11+
{
12+
public PackageSecurityPolicyEvaluationContext(
13+
IUserService userService,
14+
IPackageOwnershipManagementService packageOwnershipManagementService,
15+
IEnumerable<UserSecurityPolicy> policies,
16+
Package package,
17+
HttpContextBase httpContext)
18+
: base(policies, httpContext)
19+
{
20+
Package = package ?? throw new ArgumentNullException(nameof(package));
21+
UserService = userService ?? throw new ArgumentNullException(nameof(userService));
22+
PackageOwnershipManagementService = packageOwnershipManagementService ?? throw new ArgumentNullException(nameof(packageOwnershipManagementService));
23+
}
24+
25+
public PackageSecurityPolicyEvaluationContext(
26+
IUserService userService,
27+
IPackageOwnershipManagementService packageOwnershipManagementService,
28+
IEnumerable<UserSecurityPolicy> policies,
29+
Package package,
30+
User sourceAccount,
31+
User targetAccount,
32+
HttpContextBase httpContext = null)
33+
: base(policies, sourceAccount, targetAccount, httpContext)
34+
{
35+
Package = package ?? throw new ArgumentNullException(nameof(package));
36+
UserService = userService ?? throw new ArgumentNullException(nameof(userService));
37+
PackageOwnershipManagementService = packageOwnershipManagementService ?? throw new ArgumentNullException(nameof(packageOwnershipManagementService));
38+
}
39+
40+
/// <summary>
41+
/// Package under evaluation.
42+
/// </summary>
43+
public Package Package { get; }
44+
45+
public IUserService UserService { get; }
46+
47+
public IPackageOwnershipManagementService PackageOwnershipManagementService { get; }
48+
}
49+
}

‎src/NuGetGallery/Security/PackageSecurityPolicyHandler.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
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+
namespace NuGetGallery.Security
5+
{
6+
/// <summary>
7+
/// Policy handler that defines behavior for specific user policy types requiring package policy evaluation.
8+
/// </summary>
9+
public abstract class PackageSecurityPolicyHandler : SecurityPolicyHandler<PackageSecurityPolicyEvaluationContext>
10+
{
11+
public PackageSecurityPolicyHandler(string name, SecurityPolicyAction action)
12+
: base(name, action)
13+
{
14+
}
15+
}
16+
}

0 commit comments

Comments
 (0)