Allow organization admins to remove their organization's ownership of a package in a reserved namespace (#5984)

Scott Bommarito · web-flow · commit 62a28660a25d · 2018-05-23T12:24:31.000-07:00
diff --git a/src/NuGetGallery/Services/ActionRequiringReservedNamespacePermissions.cs b/src/NuGetGallery/Services/ActionRequiringReservedNamespacePermissions.cs
@@ -15,7 +15,9 @@ namespace NuGetGallery
     /// E.g. "JQuery.Extensions.MyCoolExtension" matches both "JQuery.*" and "JQuery.Extensions.*".
     /// </remarks>
     public class ActionRequiringReservedNamespacePermissions
-        : ActionRequiringEntityPermissions<IReadOnlyCollection<ReservedNamespace>>, IActionRequiringEntityPermissions<ActionOnNewPackageContext>
+        : ActionRequiringEntityPermissions<IReadOnlyCollection<ReservedNamespace>>, 
+        IActionRequiringEntityPermissions<ActionOnNewPackageContext>, 
+        IActionRequiringEntityPermissions<ReservedNamespace>
     {
         public PermissionsRequirement ReservedNamespacePermissionsRequirement { get; }
 
@@ -32,11 +34,21 @@ public PermissionsCheckResult CheckPermissions(User currentUser, User account, A
             return CheckPermissions(currentUser, account, GetReservedNamespaces(newPackageContext));
         }
 
+        public PermissionsCheckResult CheckPermissions(User currentUser, User account, ReservedNamespace reservedNamespace)
+        {
+            return CheckPermissions(currentUser, account, GetReservedNamespaces(reservedNamespace));
+        }
+
         public PermissionsCheckResult CheckPermissions(IPrincipal currentPrincipal, User account, ActionOnNewPackageContext newPackageContext)
         {
             return CheckPermissions(currentPrincipal, account, GetReservedNamespaces(newPackageContext));
         }
 
+        public PermissionsCheckResult CheckPermissions(IPrincipal currentPrincipal, User account, ReservedNamespace reservedNamespace)
+        {
+            return CheckPermissions(currentPrincipal, account, GetReservedNamespaces(reservedNamespace));
+        }
+
         protected override PermissionsCheckResult CheckPermissionsForEntity(User account, IReadOnlyCollection<ReservedNamespace> reservedNamespaces)
         {
             if (!reservedNamespaces.Any())
@@ -54,11 +66,21 @@ public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentU
             return CheckPermissionsOnBehalfOfAnyAccount(currentUser, GetReservedNamespaces(newPackageContext));
         }
 
+        public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, ReservedNamespace reservedNamespace)
+        {
+            return CheckPermissionsOnBehalfOfAnyAccount(currentUser, GetReservedNamespaces(reservedNamespace));
+        }
+
         public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, ActionOnNewPackageContext newPackageContext, out IEnumerable<User> accountsAllowedOnBehalfOf)
         {
             return CheckPermissionsOnBehalfOfAnyAccount(currentUser, GetReservedNamespaces(newPackageContext), out accountsAllowedOnBehalfOf);
         }
 
+        public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, ReservedNamespace reservedNamespace, out IEnumerable<User> accountsAllowedOnBehalfOf)
+        {
+            return CheckPermissionsOnBehalfOfAnyAccount(currentUser, GetReservedNamespaces(reservedNamespace), out accountsAllowedOnBehalfOf);
+        }
+
         protected override IEnumerable<User> GetOwners(IReadOnlyCollection<ReservedNamespace> reservedNamespaces)
         {
             return reservedNamespaces.Any() ? reservedNamespaces.SelectMany(rn => rn.Owners) : Enumerable.Empty<User>();
@@ -68,5 +90,10 @@ private IReadOnlyCollection<ReservedNamespace> GetReservedNamespaces(ActionOnNew
         {
             return newPackageContext.ReservedNamespaceService.GetReservedNamespacesForId(newPackageContext.PackageId);
         }
+
+        private IReadOnlyCollection<ReservedNamespace> GetReservedNamespaces(ReservedNamespace reservedNamespace)
+        {
+            return new[] { reservedNamespace };
+        }
     }
 }
diff --git a/src/NuGetGallery/Services/ActionsRequiringPermissions.cs b/src/NuGetGallery/Services/ActionsRequiringPermissions.cs
@@ -128,5 +128,21 @@ public static class ActionsRequiringPermissions
              new ActionRequiringPackagePermissions(
                 accountOnBehalfOfPermissionsRequirement: RequireOwnerOrOrganizationAdmin,
                 packageRegistrationPermissionsRequirement: RequireOwnerOrOrganizationAdmin);
+
+        /// <summary>
+        /// The action of adding a package to a reserved namespace that the package is in.
+        /// </summary>
+        public static ActionRequiringReservedNamespacePermissions AddPackageToReservedNamespace =
+            new ActionRequiringReservedNamespacePermissions(
+                accountOnBehalfOfPermissionsRequirement: PermissionsRequirement.Owner,
+                reservedNamespacePermissionsRequirement: PermissionsRequirement.Owner);
+
+        /// <summary>
+        /// The action of removing a package from a reserved namespace that the package is in.
+        /// </summary>
+        public static ActionRequiringReservedNamespacePermissions RemovePackageFromReservedNamespace =
+            new ActionRequiringReservedNamespacePermissions(
+                accountOnBehalfOfPermissionsRequirement: RequireOwnerOrSiteAdminOrOrganizationAdmin,
+                reservedNamespacePermissionsRequirement: RequireOwnerOrOrganizationAdmin);
     }
 }
diff --git a/src/NuGetGallery/Services/IPackageService.cs b/src/NuGetGallery/Services/IPackageService.cs
@@ -68,15 +68,15 @@ public interface IPackageService : ICorePackageService
         /// <returns>Awaitable task.</returns>
         Task AddPackageOwnerAsync(PackageRegistration package, User newOwner);
 
-        Task RemovePackageOwnerAsync(PackageRegistration package, User user);
+        Task RemovePackageOwnerAsync(PackageRegistration package, User user, bool commitChanges = true);
 
         Task SetLicenseReportVisibilityAsync(Package package, bool visible, bool commitChanges = true);
 
         void EnsureValid(PackageArchiveReader packageArchiveReader);
 
         Task IncrementDownloadCountAsync(string id, string version, bool commitChanges = true);
 
-        Task UpdatePackageVerifiedStatusAsync(IReadOnlyCollection<PackageRegistration> package, bool isVerified);
+        Task UpdatePackageVerifiedStatusAsync(IReadOnlyCollection<PackageRegistration> package, bool isVerified, bool commitChanges = true);
 
         /// <summary>
         /// For a package get the list of owners that are not organizations.
diff --git a/src/NuGetGallery/Services/IReservedNamespaceService.cs b/src/NuGetGallery/Services/IReservedNamespaceService.cs
@@ -57,9 +57,9 @@ public interface IReservedNamespaceService
         /// Remove the specified package registration from the reserved namespace. It is the caller's reponsibility to
         /// commit the changes to the database.
         /// </summary>
-        /// <param name="prefix">The prefix value of the reserved namespace to modify</param>
+        /// <param name="reservedNamespace">The reserved namespace to modify</param>
         /// <param name="packageRegistration">The package registration entity to be removed.</param>
-        void RemovePackageRegistrationFromNamespace(string prefix, PackageRegistration packageRegistration);
+        void RemovePackageRegistrationFromNamespace(ReservedNamespace reservedNamespace, PackageRegistration packageRegistration);
 
         /// <summary>
         /// Retrieves the first reserved namespace which matches the given prefix.
diff --git a/src/NuGetGallery/Services/PackageOwnershipManagementService.cs b/src/NuGetGallery/Services/PackageOwnershipManagementService.cs
@@ -121,13 +121,13 @@ public async Task RemovePackageOwnerAsync(PackageRegistration packageRegistratio
                     using (var strategy = new SuspendDbExecutionStrategy())
                     using (var transaction = _entitiesContext.GetDatabase().BeginTransaction())
                     {
-                        await RemovePackageOwnerImplAsync(packageRegistration, requestingOwner, ownerToBeRemoved);
+                        await RemovePackageOwnerImplAsync(packageRegistration, ownerToBeRemoved);
                         transaction.Commit();
                     }
                 }
                 else
                 {
-                    await RemovePackageOwnerImplAsync(packageRegistration, requestingOwner, ownerToBeRemoved);
+                    await RemovePackageOwnerImplAsync(packageRegistration, ownerToBeRemoved);
                 }
 
                 await _auditingService.SaveAuditRecordAsync(
@@ -139,38 +139,29 @@ await _auditingService.SaveAuditRecordAsync(
             }
         }
 
-        private async Task RemovePackageOwnerImplAsync(PackageRegistration packageRegistration, User requestingOwner, User ownerToBeRemoved)
+        private async Task RemovePackageOwnerImplAsync(PackageRegistration packageRegistration, User ownerToBeRemoved)
         {
-            // 1. Remove this package registration from the namespaces owned by this user if he is the only package owner in the set of matching namespaces
-            // 2. Remove the IsVerified flag from package registration if all the matching namespaces are owned by this user alone (no other package owner owns a matching namespace for this PR)
-            var allMatchingNamespacesForRegistration = packageRegistration.ReservedNamespaces;
-            if (allMatchingNamespacesForRegistration.Any())
+            // Remove the user from owners list of package registration
+            await _packageService.RemovePackageOwnerAsync(packageRegistration, ownerToBeRemoved, commitChanges: false);
+
+            // Remove this package registration from the namespaces owned by this user that are owned by no other package owners
+            foreach (var reservedNamespace in packageRegistration.ReservedNamespaces.ToArray())
             {
-                var allPackageOwners = packageRegistration.Owners;
-                var matchingNamespacesOwnedByUser = allMatchingNamespacesForRegistration
-                    .Where(rn => rn.Owners.Any(o => o == ownerToBeRemoved));
-                var namespacesToModify = matchingNamespacesOwnedByUser
-                    .Where(rn => rn.Owners.Intersect(allPackageOwners).Count() == 1)
-                    .ToList();
-
-                if (namespacesToModify.Any())
+                if (!packageRegistration.Owners
+                    .Any(o => ActionsRequiringPermissions.AddPackageToReservedNamespace
+                        .CheckPermissionsOnBehalfOfAnyAccount(o, reservedNamespace) == PermissionsCheckResult.Allowed))
                 {
-                    // The package will lose its 'IsVerified' flag if the user is the only package owner who owns all the namespaces that match this registration
-                    var shouldModifyIsVerified = allMatchingNamespacesForRegistration.Count() == namespacesToModify.Count();
-                    if (shouldModifyIsVerified && packageRegistration.IsVerified)
-                    {
-                        await _packageService.UpdatePackageVerifiedStatusAsync(new List<PackageRegistration> { packageRegistration }, isVerified: false);
-                    }
-
-                    namespacesToModify
-                        .ForEach(rn => _reservedNamespaceService.RemovePackageRegistrationFromNamespace(rn.Value, packageRegistration));
-
-                    await _entitiesContext.SaveChangesAsync();
+                    _reservedNamespaceService.RemovePackageRegistrationFromNamespace(reservedNamespace, packageRegistration);
                 }
             }
 
-            // Remove the user from owners list of package registration
-            await _packageService.RemovePackageOwnerAsync(packageRegistration, ownerToBeRemoved);
+            // Remove the IsVerified flag from package registration if all the matching namespaces are owned by this user alone (no other package owner owns a matching namespace for this PR)
+            if (packageRegistration.IsVerified && !packageRegistration.ReservedNamespaces.Any())
+            {
+                await _packageService.UpdatePackageVerifiedStatusAsync(new List<PackageRegistration> { packageRegistration }, isVerified: false, commitChanges: false);
+            }
+
+            await _entitiesContext.SaveChangesAsync();
         }
 
         public async Task DeletePackageOwnershipRequestAsync(PackageRegistration packageRegistration, User newOwner)
@@ -191,31 +182,21 @@ public async Task DeletePackageOwnershipRequestAsync(PackageRegistration package
                 await _packageOwnerRequestService.DeletePackageOwnershipRequest(request);
             }
         }
-
-        // The requesting owner can remove other owner only if 
-        // 1. Is an admin.
-        // 2. Owns a namespace.
-        // 3. Or the other user also does not own a namespace.
+        
         private static bool OwnerHasPermissionsToRemove(User requestingOwner, User ownerToBeRemoved, PackageRegistration packageRegistration)
         {
-            if (requestingOwner.IsAdministrator)
+            var reservedNamespaces = packageRegistration.ReservedNamespaces.ToList();
+            if (ActionsRequiringPermissions.AddPackageToReservedNamespace
+                .CheckPermissionsOnBehalfOfAnyAccount(ownerToBeRemoved, reservedNamespaces) == PermissionsCheckResult.Allowed)
             {
-                return true;
+                // If the owner to be removed owns a reserved namespace that applies to this package,
+                // the requesting user must own a reserved namespace that applies to this package or be a site admin.
+                return ActionsRequiringPermissions.RemovePackageFromReservedNamespace
+                    .CheckPermissionsOnBehalfOfAnyAccount(requestingOwner, reservedNamespaces) == PermissionsCheckResult.Allowed;
             }
 
-            var requestingOwnerOwnsNamespace = IsUserAnOwnerOfPackageNamespace(packageRegistration, requestingOwner);
-            if (requestingOwnerOwnsNamespace)
-            {
-                return true;
-            }
-
-            var ownerToBeRemovedOwnsNamespace = IsUserAnOwnerOfPackageNamespace(packageRegistration, ownerToBeRemoved);
-            return !ownerToBeRemovedOwnsNamespace;
-        }
-
-        private static bool IsUserAnOwnerOfPackageNamespace(PackageRegistration packageRegistration, User user)
-        {
-            return packageRegistration.ReservedNamespaces.Any(rn => rn.Owners.Any(owner => owner == user));
+            // If the owner to be removed does not own any reserved namespaces that apply to this package, they can be removed by anyone.
+            return true;
         }
     }
 }
diff --git a/src/NuGetGallery/Services/PackageService.cs b/src/NuGetGallery/Services/PackageService.cs
@@ -361,11 +361,14 @@ public async Task AddPackageOwnerAsync(PackageRegistration package, User newOwne
             }
         }
 
-        public async Task RemovePackageOwnerAsync(PackageRegistration package, User user)
+        public async Task RemovePackageOwnerAsync(PackageRegistration package, User user, bool commitChanges = true)
         {
             // To support the delete account scenario, the admin can delete the last owner of a package.
             package.Owners.Remove(user);
-            await _packageRepository.CommitChangesAsync();
+            if (commitChanges)
+            {
+                await _packageRepository.CommitChangesAsync();
+            }
         }
 
         public async Task MarkPackageListedAsync(Package package, bool commitChanges = true)
@@ -701,7 +704,7 @@ public async Task IncrementDownloadCountAsync(string id, string version, bool co
             }
         }
 
-        public virtual async Task UpdatePackageVerifiedStatusAsync(IReadOnlyCollection<PackageRegistration> packageRegistrationList, bool isVerified)
+        public virtual async Task UpdatePackageVerifiedStatusAsync(IReadOnlyCollection<PackageRegistration> packageRegistrationList, bool isVerified, bool commitChanges = true)
         {
             var packageRegistrationIdSet = new HashSet<string>(packageRegistrationList.Select(prl => prl.Id));
             var allPackageRegistrations = _packageRegistrationRepository.GetAll();
@@ -714,7 +717,10 @@ public virtual async Task UpdatePackageVerifiedStatusAsync(IReadOnlyCollection<P
                 packageRegistrationsToUpdate
                     .ForEach(pru => pru.IsVerified = isVerified);
 
-                await _packageRegistrationRepository.CommitChangesAsync();
+                if (commitChanges)
+                {
+                    await _packageRegistrationRepository.CommitChangesAsync();
+                }
             }
         }
 
diff --git a/src/NuGetGallery/Services/ReservedNamespaceService.cs b/src/NuGetGallery/Services/ReservedNamespaceService.cs
diff --git a/tests/NuGetGallery.Facts/Services/PackageOwnershipManagementServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageOwnershipManagementServiceFacts.cs
diff --git a/tests/NuGetGallery.Facts/Services/ReservedNamespaceServiceFacts.cs b/tests/NuGetGallery.Facts/Services/ReservedNamespaceServiceFacts.cs

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,9 @@ namespace NuGetGallery`
`15`	`15`	`/// E.g. "JQuery.Extensions.MyCoolExtension" matches both "JQuery." and "JQuery.Extensions.".`
`16`	`16`	`/// </remarks>`
`17`	`17`	`public class ActionRequiringReservedNamespacePermissions`
`18`		`- : ActionRequiringEntityPermissions<IReadOnlyCollection<ReservedNamespace>>, IActionRequiringEntityPermissions<ActionOnNewPackageContext>`
	`18`	`+ : ActionRequiringEntityPermissions<IReadOnlyCollection<ReservedNamespace>>,`
	`19`	`+ IActionRequiringEntityPermissions<ActionOnNewPackageContext>,`
	`20`	`+ IActionRequiringEntityPermissions<ReservedNamespace>`
`19`	`21`	`{`
`20`	`22`	`public PermissionsRequirement ReservedNamespacePermissionsRequirement { get; }`
`21`	`23`
`@@ -32,11 +34,21 @@ public PermissionsCheckResult CheckPermissions(User currentUser, User account, A`
`32`	`34`	`return CheckPermissions(currentUser, account, GetReservedNamespaces(newPackageContext));`
`33`	`35`	`}`
`34`	`36`
	`37`	`+ public PermissionsCheckResult CheckPermissions(User currentUser, User account, ReservedNamespace reservedNamespace)`
	`38`	`+ {`
	`39`	`+ return CheckPermissions(currentUser, account, GetReservedNamespaces(reservedNamespace));`
	`40`	`+ }`
	`41`	`+`
`35`	`42`	`public PermissionsCheckResult CheckPermissions(IPrincipal currentPrincipal, User account, ActionOnNewPackageContext newPackageContext)`
`36`	`43`	`{`
`37`	`44`	`return CheckPermissions(currentPrincipal, account, GetReservedNamespaces(newPackageContext));`
`38`	`45`	`}`
`39`	`46`
	`47`	`+ public PermissionsCheckResult CheckPermissions(IPrincipal currentPrincipal, User account, ReservedNamespace reservedNamespace)`
	`48`	`+ {`
	`49`	`+ return CheckPermissions(currentPrincipal, account, GetReservedNamespaces(reservedNamespace));`
	`50`	`+ }`
	`51`	`+`
`40`	`52`	`protected override PermissionsCheckResult CheckPermissionsForEntity(User account, IReadOnlyCollection<ReservedNamespace> reservedNamespaces)`
`41`	`53`	`{`
`42`	`54`	`if (!reservedNamespaces.Any())`
`@@ -54,11 +66,21 @@ public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentU`
`54`	`66`	`return CheckPermissionsOnBehalfOfAnyAccount(currentUser, GetReservedNamespaces(newPackageContext));`
`55`	`67`	`}`
`56`	`68`
	`69`	`+ public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, ReservedNamespace reservedNamespace)`
	`70`	`+ {`
	`71`	`+ return CheckPermissionsOnBehalfOfAnyAccount(currentUser, GetReservedNamespaces(reservedNamespace));`
	`72`	`+ }`
	`73`	`+`
`57`	`74`	`public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, ActionOnNewPackageContext newPackageContext, out IEnumerable<User> accountsAllowedOnBehalfOf)`
`58`	`75`	`{`
`59`	`76`	`return CheckPermissionsOnBehalfOfAnyAccount(currentUser, GetReservedNamespaces(newPackageContext), out accountsAllowedOnBehalfOf);`
`60`	`77`	`}`
`61`	`78`
	`79`	`+ public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, ReservedNamespace reservedNamespace, out IEnumerable<User> accountsAllowedOnBehalfOf)`
	`80`	`+ {`
	`81`	`+ return CheckPermissionsOnBehalfOfAnyAccount(currentUser, GetReservedNamespaces(reservedNamespace), out accountsAllowedOnBehalfOf);`
	`82`	`+ }`
	`83`	`+`
`62`	`84`	`protected override IEnumerable<User> GetOwners(IReadOnlyCollection<ReservedNamespace> reservedNamespaces)`
`63`	`85`	`{`
`64`	`86`	`return reservedNamespaces.Any() ? reservedNamespaces.SelectMany(rn => rn.Owners) : Enumerable.Empty<User>();`
`@@ -68,5 +90,10 @@ private IReadOnlyCollection<ReservedNamespace> GetReservedNamespaces(ActionOnNew`
`68`	`90`	`{`
`69`	`91`	`return newPackageContext.ReservedNamespaceService.GetReservedNamespacesForId(newPackageContext.PackageId);`
`70`	`92`	`}`
	`93`	`+`
	`94`	`+ private IReadOnlyCollection<ReservedNamespace> GetReservedNamespaces(ReservedNamespace reservedNamespace)`
	`95`	`+ {`
	`96`	`+ return new[] { reservedNamespace };`
	`97`	`+ }`
`71`	`98`	`}`
`72`	`99`	`}`
Original file line number	Diff line number	Diff line change
`@@ -121,13 +121,13 @@ public async Task RemovePackageOwnerAsync(PackageRegistration packageRegistratio`
`121`	`121`	`using (var strategy = new SuspendDbExecutionStrategy())`
`122`	`122`	`using (var transaction = _entitiesContext.GetDatabase().BeginTransaction())`
`123`	`123`	`{`
`124`		`- await RemovePackageOwnerImplAsync(packageRegistration, requestingOwner, ownerToBeRemoved);`
	`124`	`+ await RemovePackageOwnerImplAsync(packageRegistration, ownerToBeRemoved);`
`125`	`125`	`transaction.Commit();`
`126`	`126`	`}`
`127`	`127`	`}`
`128`	`128`	`else`
`129`	`129`	`{`
`130`		`- await RemovePackageOwnerImplAsync(packageRegistration, requestingOwner, ownerToBeRemoved);`
	`130`	`+ await RemovePackageOwnerImplAsync(packageRegistration, ownerToBeRemoved);`
`131`	`131`	`}`
`132`	`132`
`133`	`133`	`await _auditingService.SaveAuditRecordAsync(`
`@@ -139,38 +139,29 @@ await _auditingService.SaveAuditRecordAsync(`
`139`	`139`	`}`
`140`	`140`	`}`
`141`	`141`
`142`		`- private async Task RemovePackageOwnerImplAsync(PackageRegistration packageRegistration, User requestingOwner, User ownerToBeRemoved)`
	`142`	`+ private async Task RemovePackageOwnerImplAsync(PackageRegistration packageRegistration, User ownerToBeRemoved)`
`143`	`143`	`{`
`144`		`- // 1. Remove this package registration from the namespaces owned by this user if he is the only package owner in the set of matching namespaces`
`145`		`- // 2. Remove the IsVerified flag from package registration if all the matching namespaces are owned by this user alone (no other package owner owns a matching namespace for this PR)`
`146`		`- var allMatchingNamespacesForRegistration = packageRegistration.ReservedNamespaces;`
`147`		`- if (allMatchingNamespacesForRegistration.Any())`
	`144`	`+ // Remove the user from owners list of package registration`
	`145`	`+ await _packageService.RemovePackageOwnerAsync(packageRegistration, ownerToBeRemoved, commitChanges: false);`
	`146`	`+`
	`147`	`+ // Remove this package registration from the namespaces owned by this user that are owned by no other package owners`
	`148`	`+ foreach (var reservedNamespace in packageRegistration.ReservedNamespaces.ToArray())`
`148`	`149`	`{`
`149`		`- var allPackageOwners = packageRegistration.Owners;`
`150`		`- var matchingNamespacesOwnedByUser = allMatchingNamespacesForRegistration`
`151`		`- .Where(rn => rn.Owners.Any(o => o == ownerToBeRemoved));`
`152`		`- var namespacesToModify = matchingNamespacesOwnedByUser`
`153`		`- .Where(rn => rn.Owners.Intersect(allPackageOwners).Count() == 1)`
`154`		`- .ToList();`
`155`		`-`
`156`		`- if (namespacesToModify.Any())`
	`150`	`+ if (!packageRegistration.Owners`
	`151`	`+ .Any(o => ActionsRequiringPermissions.AddPackageToReservedNamespace`
	`152`	`+ .CheckPermissionsOnBehalfOfAnyAccount(o, reservedNamespace) == PermissionsCheckResult.Allowed))`
`157`	`153`	`{`
`158`		`- // The package will lose its 'IsVerified' flag if the user is the only package owner who owns all the namespaces that match this registration`
`159`		`- var shouldModifyIsVerified = allMatchingNamespacesForRegistration.Count() == namespacesToModify.Count();`
`160`		`- if (shouldModifyIsVerified && packageRegistration.IsVerified)`
`161`		`- {`
`162`		`- await _packageService.UpdatePackageVerifiedStatusAsync(new List<PackageRegistration> { packageRegistration }, isVerified: false);`
`163`		`- }`
`164`		`-`
`165`		`- namespacesToModify`
`166`		`- .ForEach(rn => _reservedNamespaceService.RemovePackageRegistrationFromNamespace(rn.Value, packageRegistration));`
`167`		`-`
`168`		`- await _entitiesContext.SaveChangesAsync();`
	`154`	`+ _reservedNamespaceService.RemovePackageRegistrationFromNamespace(reservedNamespace, packageRegistration);`
`169`	`155`	`}`
`170`	`156`	`}`
`171`	`157`
`172`		`- // Remove the user from owners list of package registration`
`173`		`- await _packageService.RemovePackageOwnerAsync(packageRegistration, ownerToBeRemoved);`
	`158`	`+ // Remove the IsVerified flag from package registration if all the matching namespaces are owned by this user alone (no other package owner owns a matching namespace for this PR)`
	`159`	`+ if (packageRegistration.IsVerified && !packageRegistration.ReservedNamespaces.Any())`
	`160`	`+ {`
	`161`	`+ await _packageService.UpdatePackageVerifiedStatusAsync(new List<PackageRegistration> { packageRegistration }, isVerified: false, commitChanges: false);`
	`162`	`+ }`
	`163`	`+`
	`164`	`+ await _entitiesContext.SaveChangesAsync();`
`174`	`165`	`}`
`175`	`166`
`176`	`167`	`public async Task DeletePackageOwnershipRequestAsync(PackageRegistration packageRegistration, User newOwner)`
`@@ -191,31 +182,21 @@ public async Task DeletePackageOwnershipRequestAsync(PackageRegistration package`
`191`	`182`	`await _packageOwnerRequestService.DeletePackageOwnershipRequest(request);`
`192`	`183`	`}`
`193`	`184`	`}`
`194`		`-`
`195`		`- // The requesting owner can remove other owner only if`
`196`		`- // 1. Is an admin.`
`197`		`- // 2. Owns a namespace.`
`198`		`- // 3. Or the other user also does not own a namespace.`
	`185`	`+`
`199`	`186`	`private static bool OwnerHasPermissionsToRemove(User requestingOwner, User ownerToBeRemoved, PackageRegistration packageRegistration)`
`200`	`187`	`{`
`201`		`- if (requestingOwner.IsAdministrator)`
	`188`	`+ var reservedNamespaces = packageRegistration.ReservedNamespaces.ToList();`
	`189`	`+ if (ActionsRequiringPermissions.AddPackageToReservedNamespace`
	`190`	`+ .CheckPermissionsOnBehalfOfAnyAccount(ownerToBeRemoved, reservedNamespaces) == PermissionsCheckResult.Allowed)`
`202`	`191`	`{`
`203`		`- return true;`
	`192`	`+ // If the owner to be removed owns a reserved namespace that applies to this package,`
	`193`	`+ // the requesting user must own a reserved namespace that applies to this package or be a site admin.`
	`194`	`+ return ActionsRequiringPermissions.RemovePackageFromReservedNamespace`
	`195`	`+ .CheckPermissionsOnBehalfOfAnyAccount(requestingOwner, reservedNamespaces) == PermissionsCheckResult.Allowed;`
`204`	`196`	`}`
`205`	`197`
`206`		`- var requestingOwnerOwnsNamespace = IsUserAnOwnerOfPackageNamespace(packageRegistration, requestingOwner);`
`207`		`- if (requestingOwnerOwnsNamespace)`
`208`		`- {`
`209`		`- return true;`
`210`		`- }`
`211`		`-`
`212`		`- var ownerToBeRemovedOwnsNamespace = IsUserAnOwnerOfPackageNamespace(packageRegistration, ownerToBeRemoved);`
`213`		`- return !ownerToBeRemovedOwnsNamespace;`
`214`		`- }`
`215`		`-`
`216`		`- private static bool IsUserAnOwnerOfPackageNamespace(PackageRegistration packageRegistration, User user)`
`217`		`- {`
`218`		`- return packageRegistration.ReservedNamespaces.Any(rn => rn.Owners.Any(owner => owner == user));`
	`198`	`+ // If the owner to be removed does not own any reserved namespaces that apply to this package, they can be removed by anyone.`
	`199`	`+ return true;`
`219`	`200`	`}`
`220`	`201`	`}`
`221`	`202`	`}`
Original file line number	Diff line number	Diff line change
`@@ -361,11 +361,14 @@ public async Task AddPackageOwnerAsync(PackageRegistration package, User newOwne`
`361`	`361`	`}`
`362`	`362`	`}`
`363`	`363`
`364`		`- public async Task RemovePackageOwnerAsync(PackageRegistration package, User user)`
	`364`	`+ public async Task RemovePackageOwnerAsync(PackageRegistration package, User user, bool commitChanges = true)`
`365`	`365`	`{`
`366`	`366`	`// To support the delete account scenario, the admin can delete the last owner of a package.`
`367`	`367`	`package.Owners.Remove(user);`
`368`		`- await _packageRepository.CommitChangesAsync();`
	`368`	`+ if (commitChanges)`
	`369`	`+ {`
	`370`	`+ await _packageRepository.CommitChangesAsync();`
	`371`	`+ }`
`369`	`372`	`}`
`370`	`373`
`371`	`374`	`public async Task MarkPackageListedAsync(Package package, bool commitChanges = true)`
`@@ -701,7 +704,7 @@ public async Task IncrementDownloadCountAsync(string id, string version, bool co`
`701`	`704`	`}`
`702`	`705`	`}`
`703`	`706`
`704`		`- public virtual async Task UpdatePackageVerifiedStatusAsync(IReadOnlyCollection<PackageRegistration> packageRegistrationList, bool isVerified)`
	`707`	`+ public virtual async Task UpdatePackageVerifiedStatusAsync(IReadOnlyCollection<PackageRegistration> packageRegistrationList, bool isVerified, bool commitChanges = true)`
`705`	`708`	`{`
`706`	`709`	`var packageRegistrationIdSet = new HashSet<string>(packageRegistrationList.Select(prl => prl.Id));`
`707`	`710`	`var allPackageRegistrations = _packageRegistrationRepository.GetAll();`
`@@ -714,7 +717,10 @@ public virtual async Task UpdatePackageVerifiedStatusAsync(IReadOnlyCollection<P`
`714`	`717`	`packageRegistrationsToUpdate`
`715`	`718`	`.ForEach(pru => pru.IsVerified = isVerified);`
`716`	`719`
`717`		`- await _packageRegistrationRepository.CommitChangesAsync();`
	`720`	`+ if (commitChanges)`
	`721`	`+ {`
	`722`	`+ await _packageRegistrationRepository.CommitChangesAsync();`
	`723`	`+ }`
`718`	`724`	`}`
`719`	`725`	`}`
`720`	`726`