Allow site admins to delete organizations they are not a part of (#7041)

Author: Scott Bommarito

src/NuGetGallery/Areas/Admin/ViewModels/DeleteAccountStatus.cs

@@ -3,7 +3,7 @@
 
 namespace NuGetGallery.Areas.Admin.ViewModels
 {
-    public class DeleteUserAccountStatus
+    public class DeleteAccountStatus
     {
         public bool Success { get; set; }
 

…teAccount/DeleteUserAccountStatus.cshtml …DeleteAccount/DeleteAccountStatus.cshtmlsrc/NuGetGallery/Areas/Admin/Views/DeleteAccount/DeleteUserAccountStatus.cshtml renamed to src/NuGetGallery/Areas/Admin/Views/DeleteAccount/DeleteAccountStatus.cshtml src/NuGetGallery/Areas/Admin/Views/DeleteAccount/DeleteUserAccountStatus.cshtml renamed to src/NuGetGallery/Areas/Admin/Views/DeleteAccount/DeleteAccountStatus.cshtml

@@ -1,5 +1,5 @@
 @using NuGetGallery
-@model NuGetGallery.Areas.Admin.ViewModels.DeleteUserAccountStatus
+@model NuGetGallery.Areas.Admin.ViewModels.DeleteAccountStatus
 @{
     ViewBag.Title = "Delete Account Status" + Model.AccountName;
     ViewBag.MdPageColumns = GalleryConstants.ColumnsFormMd;

src/NuGetGallery/Areas/Admin/Views/DeleteAccount/DeleteOrganizationAccount.cshtml

@@ -0,0 +1,59 @@
+@using NuGetGallery
+@model DeleteOrganizationViewModel
+@{
+    ViewBag.Title = "Delete Account " + Model.AccountName;
+    ViewBag.MdPageColumns = GalleryConstants.ColumnsFormMd;
+}
+
+<section role="main" class="container main-container page-delete-account">
+    <div class="form-group">
+        <div class="form-group">
+            @ViewHelpers.Breadcrumb(true, @<text><a href="@Url.User(@Model.User)">@Model.AccountName</a></text>, @<text>Delete organization</text>)
+
+            @ViewHelpers.AlertDanger(@<text>
+                <b class="keywords">Once this organization is deleted, it cannot be undone!</b> <br />
+                Deleting this organization will: <br />
+                1. Remove the organization as an owner of any child packages.<br />
+                2. Delete any API keys that are scoped to this organization.<br />
+                3. Dissociate all previously existent ID prefix reservations with this organization.<br />
+                4. Remove all of its members from being a part of the organization or owning its packages.<br />
+                5. Delete any membership requests to join this organization.<br />
+            </text>)
+
+            <div class="form-group">
+                @Html.Partial("~/Views/Users/_UserPackagesListForDeletedAccount.cshtml", Model)
+            </div>
+            <div class="form-group">
+                @{
+                    var membersCount = Model.Members.Count();
+                    var membersPlural = membersCount == 1 ? "" : "s";
+                }
+                <p>
+                    This organization has @membersCount member@(membersPlural).
+                </p>
+                @if (Model.Members.Any())
+                {
+                    @Html.Partial("_OrganizationMembersListForDeletedAccount", Model.Members)
+                }
+            </div>
+        </div>
+        <div class="form-group danger-zone">
+            @using (Html.BeginForm("Delete", "Organizations", FormMethod.Post, new { id = "delete-form" }))
+            {
+                @Html.Partial("_DeleteUserAccountForm", new DeleteAccountAsAdminViewModel(Model))
+            }
+        </div>
+    </div>
+</section>
+
+@section BottomScripts {
+    <script type="text/javascript">
+        $(function () {
+            $('#delete-form').submit(function (e) {
+                if (!confirm('Are you sure you want to continue to delete this organization?')) {
+                    e.preventDefault();
+                }
+            });
+        });
+    </script>
+}

src/NuGetGallery/Areas/Admin/Views/DeleteAccount/DeleteUserAccount.cshtml

@@ -8,15 +8,15 @@
 <section role="main" class="container main-container page-delete-account">
     <div class="form-group">
         <div class="form-group">
-            @ViewHelpers.Breadcrumb(true, @<text><a href="@Url.User(@Model.User)">@Model.AccountName</a></text>, @<text>Delete account</text>)
+            @ViewHelpers.Breadcrumb(true, @<text><a href="@Url.User(@Model.User)">@Model.AccountName</a></text>, @<text>Delete user</text>)
 
             @ViewHelpers.AlertDanger(@<text>
-                <b class="keywords">Once the account is deleted, it cannot be undone!</b> <br />
-                Deleting the account will: <br />
-                1. Revoke associated API key(s). <br />
-                2. Remove the account as an owner for any child packages.<br />
-                3. Dissociate all previously existent ID prefix reservations with this account.<br />
-                4. Remove the account as a member of any organizations.<br />
+                <b class="keywords">Once this user is deleted, it cannot be undone!</b> <br />
+                Deleting this user will: <br />
+                1. Revoke any associated API key(s). <br />
+                2. Remove the user as an owner for any child packages.<br />
+                3. Dissociate all previously existent ID prefix reservations with this user.<br />
+                4. Remove the user as a member of any organizations.<br />
             </text>)
 
             <div class="form-group">
@@ -26,15 +26,20 @@
                 @Html.Partial("_UserOrganizationsListForDeletedAccount", Model.Organizations)
             </div>
         </div>
-        @Html.Partial("_DeleteUserAccountForm", new DeleteAccountAsAdminViewModel(Model))
+        <div class="form-group danger-zone">
+            @using (Html.BeginForm("Delete", "Users", FormMethod.Post, new { id = "delete-form" }))
+            {
+                @Html.Partial("_DeleteUserAccountForm", new DeleteAccountAsAdminViewModel(Model))
+            }
+        </div>
     </div>
 </section>
 
 @section BottomScripts {
     <script type="text/javascript">
         $(function () {
             $('#delete-form').submit(function (e) {
-                if (!confirm('Are you sure you want to continue to delete this account?')) {
+                if (!confirm('Are you sure you want to continue to delete this user?')) {
                     e.preventDefault();
                 }
             });

src/NuGetGallery/Areas/Admin/Views/DeleteAccount/_DeleteUserAccountForm.cshtml

@@ -1,39 +1,34 @@
 @model DeleteAccountAsAdminViewModel
 
-<div class="form-group danger-zone">
-    @using (Html.BeginForm("Delete", "Users", FormMethod.Post, new { id = "delete-form" }))
-    {
-        @Html.HttpMethodOverride(HttpVerbs.Delete)
-        @Html.AntiForgeryToken()
-        @Html.ValidationSummary(true)
-        <div>
-            <div class="form-group">
-                @Html.LabelFor(m => m.Signature)
-                @Html.EditorFor(m => m.Signature)
-                @Html.ValidationMessageFor(m => m.Signature)
-            </div>
+@Html.HttpMethodOverride(HttpVerbs.Delete)
+@Html.AntiForgeryToken()
+@Html.ValidationSummary(true)
+<div>
+    <div class="form-group">
+        @Html.LabelFor(m => m.Signature)
+        @Html.EditorFor(m => m.Signature)
+        @Html.ValidationMessageFor(m => m.Signature)
+    </div>
 
-            @if (Model.HasPackagesThatWillBeOrphaned)
-            {
-                <div class="form-group">
-                    @Html.EditorFor(m => m.ShouldUnlist)
-                    <label class="checkbox-inline">
-                        <b>Unlist the packages without any owner.</b>
-                    </label>
-                    <p>
-                        One or more packages do not have co-owners. If you choose to proceed without fixing this issue, these packages will be orphaned and a warning message will be displayed under owners on the package page.
-                    </p>
-                </div>
-            }
+    @if (Model.HasPackagesThatWillBeOrphaned)
+    {
+        <div class="form-group">
+            @Html.EditorFor(m => m.ShouldUnlist)
+            <label class="checkbox-inline">
+                <b>Unlist the packages without any owner.</b>
+            </label>
+            <p>
+                One or more packages do not have co-owners. If you choose to proceed without fixing this issue, these packages will be orphaned and a warning message will be displayed under owners on the package page.
+            </p>
         </div>
-        <hr />
-        <p>
-            This action <strong>CANNOT</strong> be undone. You will not be able to regain access to the account <b>@Model.AccountName</b> or any of its packages.
-        </p>
-        <hr />
-        @Html.HiddenFor(m => m.Signature)
-        @Html.HiddenFor(m => m.ShouldUnlist)
-        @Html.HiddenFor(m => m.AccountName)
-        <input id="btn-delete" type="submit" class="btn btn-danger form-control" value="I understand the consequences, delete this account" title="I understand the consequences, delete this account." />
     }
-</div>
+</div>
+<hr />
+<p>
+    This action <strong>CANNOT</strong> be undone. You will not be able to regain access to the account <b>@Model.AccountName</b> or any of its packages.
+</p>
+<hr />
+@Html.HiddenFor(m => m.Signature)
+@Html.HiddenFor(m => m.ShouldUnlist)
+@Html.HiddenFor(m => m.AccountName)
+<input id="btn-delete" type="submit" class="btn btn-danger form-control" value="I understand the consequences, delete this account" title="I understand the consequences, delete this account." />

src/NuGetGallery/Controllers/AccountsController.cs

@@ -9,6 +9,7 @@
 using System.Web.Mvc;
 using NuGet.Services.Entities;
 using NuGet.Services.Messaging.Email;
+using NuGetGallery.Areas.Admin.ViewModels;
 using NuGetGallery.Authentication;
 using NuGetGallery.Filters;
 using NuGetGallery.Helpers;
@@ -48,6 +49,8 @@ public class ViewMessages
 
         public IMessageServiceConfiguration MessageServiceConfiguration { get; }
 
+        public IDeleteAccountService DeleteAccountService { get; }
+
         public AccountsController(
             AuthenticationService authenticationService,
             IPackageService packageService,
@@ -57,7 +60,8 @@ public AccountsController(
             ISecurityPolicyService securityPolicyService,
             ICertificateService certificateService,
             IContentObjectService contentObjectService,
-            IMessageServiceConfiguration messageServiceConfiguration)
+            IMessageServiceConfiguration messageServiceConfiguration,
+            IDeleteAccountService deleteAccountService)
         {
             AuthenticationService = authenticationService ?? throw new ArgumentNullException(nameof(authenticationService));
             PackageService = packageService ?? throw new ArgumentNullException(nameof(packageService));
@@ -68,6 +72,7 @@ public AccountsController(
             CertificateService = certificateService ?? throw new ArgumentNullException(nameof(certificateService));
             ContentObjectService = contentObjectService ?? throw new ArgumentNullException(nameof(contentObjectService));
             MessageServiceConfiguration = messageServiceConfiguration ?? throw new ArgumentNullException(nameof(messageServiceConfiguration));
+            DeleteAccountService = deleteAccountService ?? throw new ArgumentNullException(nameof(deleteAccountService));
         }
 
         public abstract string AccountAction { get; }
@@ -310,6 +315,48 @@ public virtual ActionResult DeleteRequest(string accountName = null)
             return View("DeleteAccount", GetDeleteAccountViewModel(accountToDelete));
         }
 
+        [HttpGet]
+        [UIAuthorize(Roles = "Admins")]
+        public virtual ActionResult Delete(string accountName)
+        {
+            var accountToDelete = UserService.FindByUsername(accountName) as TUser;
+            if (accountToDelete == null || accountToDelete.IsDeleted)
+            {
+                return HttpNotFound();
+            }
+
+            return View(GetDeleteAccountViewName(), GetDeleteAccountViewModel(accountToDelete));
+        }
+
+        [HttpDelete]
+        [UIAuthorize(Roles = "Admins")]
+        [RequiresAccountConfirmation("Delete account")]
+        [ValidateAntiForgeryToken]
+        public virtual async Task<ActionResult> Delete(DeleteAccountAsAdminViewModel model)
+        {
+            var accountToDelete = UserService.FindByUsername(model.AccountName) as TUser;
+            if (accountToDelete == null || accountToDelete.IsDeleted)
+            {
+                return View("DeleteAccountStatus", new DeleteAccountStatus()
+                {
+                    AccountName = model.AccountName,
+                    Description = $"Account {model.AccountName} not found.",
+                    Success = false
+                });
+            }
+            else
+            {
+                var admin = GetCurrentUser();
+                var status = await DeleteAccountService.DeleteAccountAsync(
+                    userToBeDeleted: accountToDelete,
+                    userToExecuteTheDelete: admin,
+                    orphanPackagePolicy: model.ShouldUnlist ? AccountDeletionOrphanPackagePolicy.UnlistOrphans : AccountDeletionOrphanPackagePolicy.KeepOrphans);
+                return View("DeleteAccountStatus", status);
+            }
+        }
+
+        protected abstract string GetDeleteAccountViewName();
+
         protected abstract DeleteAccountViewModel<TUser> GetDeleteAccountViewModel(TUser account);
 
         public abstract Task<ActionResult> RequestAccountDeletion(string accountName = null);

src/NuGetGallery/Controllers/OrganizationsController.cs

@@ -20,8 +20,6 @@ namespace NuGetGallery
     public class OrganizationsController
         : AccountsController<Organization, OrganizationAccountViewModel>
     {
-        public IDeleteAccountService DeleteAccountService { get; }
-
         public OrganizationsController(
             AuthenticationService authService,
             IMessageService messageService,
@@ -42,9 +40,9 @@ public OrganizationsController(
                   securityPolicyService,
                   certificateService,
                   contentObjectService,
-                  messageServiceConfiguration)
+                  messageServiceConfiguration,
+                  deleteAccountService)
         {
-            DeleteAccountService = deleteAccountService;
         }
 
         public override string AccountAction => nameof(ManageOrganization);
@@ -326,6 +324,8 @@ public async Task<JsonResult> DeleteMember(string accountName, string memberName
             }
         }
 
+        protected override string GetDeleteAccountViewName() => "DeleteOrganizationAccount";
+
         protected override DeleteAccountViewModel<Organization> GetDeleteAccountViewModel(Organization account)
         {
             return GetDeleteOrganizationViewModel(account);

src/NuGetGallery/Controllers/UsersController.cs

@@ -29,7 +29,6 @@ public partial class UsersController
         private readonly IPackageOwnerRequestService _packageOwnerRequestService;
         private readonly IAppConfiguration _config;
         private readonly ICredentialBuilder _credentialBuilder;
-        private readonly IDeleteAccountService _deleteAccountService;
         private readonly ISupportRequestService _supportRequestService;
 
         public UsersController(
@@ -56,12 +55,12 @@ public UsersController(
                   securityPolicyService,
                   certificateService,
                   contentObjectService,
-                  messageServiceConfiguration)
+                  messageServiceConfiguration,
+                  deleteAccountService)
         {
             _packageOwnerRequestService = packageOwnerRequestService ?? throw new ArgumentNullException(nameof(packageOwnerRequestService));
             _config = config ?? throw new ArgumentNullException(nameof(config));
             _credentialBuilder = credentialBuilder ?? throw new ArgumentNullException(nameof(credentialBuilder));
-            _deleteAccountService = deleteAccountService ?? throw new ArgumentNullException(nameof(deleteAccountService));
             _supportRequestService = supportRequestService ?? throw new ArgumentNullException(nameof(supportRequestService));
         }
 
@@ -105,6 +104,8 @@ protected override User GetAccount(string accountName)
             return null;
         }
 
+        protected override string GetDeleteAccountViewName() => "DeleteUserAccount";
+
         protected override DeleteAccountViewModel<User> GetDeleteAccountViewModel(User account)
         {
             return new DeleteUserViewModel(account, GetCurrentUser(), PackageService, _supportRequestService);
@@ -331,7 +332,7 @@ public override async Task<ActionResult> RequestAccountDeletion(string accountNa
             if (!user.Confirmed)
             {
                 // Unconfirmed users can be deleted immediately without creating a support request.
-                DeleteUserAccountStatus accountDeleteStatus = await _deleteAccountService.DeleteAccountAsync(userToBeDeleted: user,
+                DeleteAccountStatus accountDeleteStatus = await DeleteAccountService.DeleteAccountAsync(userToBeDeleted: user,
                     userToExecuteTheDelete: user,
                     orphanPackagePolicy: AccountDeletionOrphanPackagePolicy.UnlistOrphans);
                 if (!accountDeleteStatus.Success)
@@ -357,46 +358,6 @@ public override async Task<ActionResult> RequestAccountDeletion(string accountNa
             return RedirectToAction(nameof(DeleteRequest));
         }
 
-        [HttpGet]
-        [UIAuthorize(Roles = "Admins")]
-        public virtual ActionResult Delete(string accountName)
-        {
-            var user = UserService.FindByUsername(accountName);
-            if (user == null || user.IsDeleted || (user is Organization))
-            {
-                return HttpNotFound();
-            }
-
-            return View("DeleteUserAccount", GetDeleteAccountViewModel(user));
-        }
-
-        [HttpDelete]
-        [UIAuthorize(Roles = "Admins")]
-        [RequiresAccountConfirmation("Delete account")]
-        [ValidateAntiForgeryToken]
-        public virtual async Task<ActionResult> Delete(DeleteAccountAsAdminViewModel model)
-        {
-            var user = UserService.FindByUsername(model.AccountName);
-            if (user == null || user.IsDeleted)
-            {
-                return View("DeleteUserAccountStatus", new DeleteUserAccountStatus()
-                {
-                    AccountName = model.AccountName,
-                    Description = $"Account {model.AccountName} not found.",
-                    Success = false
-                });
-            }
-            else
-            {
-                var admin = GetCurrentUser();
-                var status = await _deleteAccountService.DeleteAccountAsync(
-                    userToBeDeleted: user,
-                    userToExecuteTheDelete: admin,
-                    orphanPackagePolicy: model.ShouldUnlist ? AccountDeletionOrphanPackagePolicy.UnlistOrphans : AccountDeletionOrphanPackagePolicy.KeepOrphans);
-                return View("DeleteUserAccountStatus", status);
-            }
-        }
-
         [HttpGet]
         [UIAuthorize]
         public virtual ActionResult ApiKeys()