Skip to content

Commit 3f692c3

Browse files
authored
AAD account checks on packages for safety reports (#9360)
1 parent 9602e0a commit 3f692c3

8 files changed

Lines changed: 385 additions & 7 deletions

File tree

src/AccountDeleter/EmptyFeatureFlagService.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,11 @@ public bool IsShowReportAbuseSafetyChangesEnabled()
276276
throw new NotImplementedException();
277277
}
278278

279+
public bool IsAllowAadContentSafetyReportsEnabled()
280+
{
281+
throw new NotImplementedException();
282+
}
283+
279284
public bool IsTyposquattingEnabled()
280285
{
281286
throw new NotImplementedException();

src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,11 @@ public bool IsShowReportAbuseSafetyChangesEnabled()
215215
throw new NotImplementedException();
216216
}
217217

218+
public bool IsAllowAadContentSafetyReportsEnabled()
219+
{
220+
throw new NotImplementedException();
221+
}
222+
218223
public bool IsPackageDependentsEnabled(User user)
219224
{
220225
throw new NotImplementedException();

src/NuGetGallery.Core/CredentialTypes.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ public static bool IsApiKey(string type)
104104
{
105105
return type?.StartsWith(ApiKey.Prefix, StringComparison.OrdinalIgnoreCase) ?? false;
106106
}
107+
107108
public static bool IsMicrosoftAccount(string type)
108109
{
109110
return type?.Equals(External.MicrosoftAccount, StringComparison.OrdinalIgnoreCase) ?? false;

src/NuGetGallery.Services/Configuration/FeatureFlagService.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public class FeatureFlagService : IFeatureFlagService
5252
private const string ImageAllowlistFlightName = GalleryPrefix + "ImageAllowlist";
5353
private const string DisplayBannerFlightName = GalleryPrefix + "Banner";
5454
private const string ShowReportAbuseSafetyChanges = GalleryPrefix + "ShowReportAbuseSafetyChanges";
55+
private const string AllowAadContentSafetyReports = GalleryPrefix + "AllowAadContentSafetyReports";
5556
private const string DisplayTargetFrameworkFeatureName = GalleryPrefix + "DisplayTargetFramework";
5657
private const string ComputeTargetFrameworkFeatureName = GalleryPrefix + "ComputeTargetFramework";
5758
private const string RecentPackagesNoIndexFeatureName = GalleryPrefix + "RecentPackagesNoIndex";
@@ -333,6 +334,11 @@ public bool IsShowReportAbuseSafetyChangesEnabled()
333334
return _client.IsEnabled(ShowReportAbuseSafetyChanges, defaultValue: false);
334335
}
335336

337+
public bool IsAllowAadContentSafetyReportsEnabled()
338+
{
339+
return _client.IsEnabled(AllowAadContentSafetyReports, defaultValue: false);
340+
}
341+
336342
public bool IsMarkdigMdRenderingEnabled()
337343
{
338344
return _client.IsEnabled(MarkdigMdRenderingFlightName, defaultValue: false);

src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,11 @@ public interface IFeatureFlagService
258258
/// </summary>
259259
bool IsShowReportAbuseSafetyChangesEnabled();
260260

261+
/// <summary>
262+
/// Whether online safety categories are available to content owned by at least one AAD-authenticated account
263+
/// </summary>
264+
bool IsAllowAadContentSafetyReportsEnabled();
265+
261266
/// <summary>
262267
/// Whether rendering Markdown content to HTML using Markdig is enabled
263268
/// </summary>

src/NuGetGallery/Controllers/PackagesController.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,6 +1385,7 @@ public virtual ActionResult ReportAbuse(string id, string version)
13851385
var model = new ReportAbuseViewModel
13861386
{
13871387
ReasonChoices = _featureFlagService.IsShowReportAbuseSafetyChangesEnabled()
1388+
&& (_featureFlagService.IsAllowAadContentSafetyReportsEnabled() || PackageHasNoAadOwners(package))
13881389
? ReportAbuseWithSafetyReasons
13891390
: ReportAbuseReasons,
13901391
PackageId = id,
@@ -2857,6 +2858,38 @@ await _auditingService.SaveAuditRecordAsync(
28572858
}
28582859
}
28592860

2861+
private static bool PackageHasNoAadOwners(Package package)
2862+
{
2863+
var owners = package?.PackageRegistration?.Owners;
2864+
if (owners == null || !owners.Any()) {
2865+
return true;
2866+
}
2867+
2868+
// First check direct owner credentials
2869+
if (owners.Any(o => o.Credentials.GetAzureActiveDirectoryCredential() != null))
2870+
{
2871+
return false;
2872+
}
2873+
2874+
// Check all members of organization owners
2875+
var orgOwners = owners.Where(o => o is Organization).Select(o => o as Organization);
2876+
foreach (var orgOwner in orgOwners)
2877+
{
2878+
if (orgOwner.Members == null)
2879+
{
2880+
continue;
2881+
}
2882+
2883+
if (orgOwner.Members.Any(m => m.Member?.Credentials != null &&
2884+
m.Member.Credentials.GetAzureActiveDirectoryCredential() != null))
2885+
{
2886+
return false;
2887+
}
2888+
}
2889+
2890+
return true;
2891+
}
2892+
28602893
private async Task DeleteUploadedFileForUser(User currentUser, Stream uploadedFileStream)
28612894
{
28622895
try

src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ public class FakeFeatureFlagService : IFeatureFlagService
103103

104104
public bool IsShowReportAbuseSafetyChangesEnabled() => throw new NotImplementedException();
105105

106+
public bool IsAllowAadContentSafetyReportsEnabled() => throw new NotImplementedException();
107+
106108
public bool IsMarkdigMdRenderingEnabled() => throw new NotImplementedException();
107109

108110
public bool IsMarkdigMdSyntaxHighlightEnabled() => throw new NotImplementedException();

0 commit comments

Comments
 (0)