Support username casing change in Admin UI

src/NuGetGallery/Areas/Admin/Controllers/ChangeUsernameController.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Net;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
@@ -20,6 +21,7 @@ public class ChangeUsernameController : AdminControllerBase
         private readonly IEntitiesContext _entitiesContext;
         private readonly IDateTimeProvider _dateTimeProvider;
         private readonly IAuditingService _auditingService;
+        private readonly IPackageService _packageService;
 
         private readonly Regex UsernameValidationRegex = new Regex(GalleryConstants.UsernameValidationRegex);
 
@@ -28,13 +30,15 @@ public ChangeUsernameController(
             IEntityRepository<User> userRepository,
             IEntitiesContext entitiesContext,
             IDateTimeProvider dateTimeProvider,
-            IAuditingService auditingService)
+            IAuditingService auditingService,
+            IPackageService packageService)
         {
             _userService = userService ?? throw new ArgumentNullException(nameof(userService));
             _userRepository = userRepository ?? throw new ArgumentNullException(nameof(userRepository));
             _entitiesContext = entitiesContext ?? throw new ArgumentNullException(nameof(entitiesContext));
             _dateTimeProvider = dateTimeProvider ?? throw new ArgumentNullException(nameof(dateTimeProvider));
             _auditingService = auditingService ?? throw new ArgumentNullException(nameof(auditingService));
+            _packageService = packageService ?? throw new ArgumentNullException(nameof(packageService));
         }
 
         [HttpGet]
@@ -83,14 +87,28 @@ public ActionResult VerifyAccount(string accountEmailOrUsername)
         }
 
         [HttpGet]
-        public ActionResult ValidateNewUsername(string newUsername)
+        public ActionResult ValidateNewUsername(string newUsername, bool checkOwnedPackages, string oldUsername = "")
         {
             if (string.IsNullOrEmpty(newUsername))
             {
                 return Json(HttpStatusCode.BadRequest, "Username cannot be null or empty.", JsonRequestBehavior.AllowGet);
             }
 
-            var result = ValidateUsername(newUsername);
+            var result = CheckUserExist(newUsername);
+
+            if (checkOwnedPackages)
+            {
+                var oldAccount = _userService.FindByUsername(oldUsername);
+                if(oldAccount == null)
+                {
+                    return Json(HttpStatusCode.NotFound, "Old username account was not found.", JsonRequestBehavior.AllowGet);
+                }
+
+                var ownedPackages = _packageService.FindPackagesByOwner(oldAccount, includeUnlisted: true)
+                    .Where(p => p.PackageStatusKey != PackageStatus.Deleted)
+                    .Select(p => p.PackageRegistration.Id);
+                result.OwnedPackageIds = ownedPackages;
+            }
 
             return Json(result, JsonRequestBehavior.AllowGet);
         }
@@ -116,39 +134,52 @@ public async Task<ActionResult> ChangeUsername(string oldUsername, string newUse
                 return Json(HttpStatusCode.NotFound, "Old username account was not found.", JsonRequestBehavior.AllowGet);
             }
 
-            var newUsernameValidation = ValidateUsername(newUsername);
+            var newUsernameValidation = ValidateUsernameChange(account, newUsername);
 
             if (!newUsernameValidation.IsFormatValid || !newUsernameValidation.IsAvailable)
             {
                 return Json(HttpStatusCode.BadRequest, "New username validation failed.", JsonRequestBehavior.AllowGet);
             }
 
-            var newAccountForOldUsername = new User()
+            if (account.Username.Equals(newUsername, StringComparison.OrdinalIgnoreCase) == false)
             {
-                Username = account.Username,
-                EmailAllowed = false,
-                IsDeleted = true,
-                CreatedUtc = _dateTimeProvider.UtcNow
-            };
+                // We're doing a full username change and not just a casing change so we need to lock the old username
+                var newAccountForOldUsername = new User()
+                {
+                    Username = account.Username,
+                    EmailAllowed = false,
+                    IsDeleted = true,
+                    CreatedUtc = _dateTimeProvider.UtcNow
+                };
+
+                _userRepository.InsertOnCommit(newAccountForOldUsername);
+            }
 
             account.Username = newUsername;
 
             await _auditingService.SaveAuditRecordAsync(new UserAuditRecord(account, AuditedUserAction.ChangeUsername));
-
-            _userRepository.InsertOnCommit(newAccountForOldUsername);
-
             await _entitiesContext.SaveChangesAsync();
 
             return Json(HttpStatusCode.OK, "Account renamed successfully!", JsonRequestBehavior.AllowGet);
         }
 
-        private ValidateUsernameResult ValidateUsername(string username)
+        private ValidateUsernameResult CheckUserExist(string username) {
+            return new ValidateUsernameResult()
+            {
+                IsFormatValid = UsernameValidationRegex.IsMatch(username),
+                IsAvailable = _userService.FindByUsername(username, includeDeleted: true) == null
+            };
+        }
+
+        private ValidateUsernameResult ValidateUsernameChange(User requestor, string username)
         {
-            var result = new ValidateUsernameResult();
-            result.IsFormatValid = UsernameValidationRegex.IsMatch(username);
-            result.IsAvailable = _userService.FindByUsername(username, includeDeleted: true) == null;
+            var foundUser = _userService.FindByUsername(username, includeDeleted: true);
 
-            return result;
+            return new ValidateUsernameResult()
+            {
+                IsFormatValid = UsernameValidationRegex.IsMatch(username),
+                IsAvailable = foundUser == null || (requestor.Key == foundUser.Key && foundUser.Username != username) // The username check is in the event where we found a user in the DB but we're doing a cAsIng change
+            };
         }
     }
 }

src/NuGetGallery/Areas/Admin/Models/ValidateUsernameResult.cs

@@ -1,11 +1,14 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.Collections.Generic;
+
 namespace NuGetGallery.Areas.Admin.Models
 {
     public class ValidateUsernameResult
     {
         public bool IsFormatValid { get; set; }
         public bool IsAvailable { get; set; }
+        public IEnumerable<string> OwnedPackageIds { get; set; } = new List<string>();
     }
 }

src/NuGetGallery/Areas/Admin/Views/ChangeUsername/Index.cshtml

@@ -58,6 +58,9 @@
         <i>Validate if the new username is available and has a valid format.</i><br />
 
         <input type="text" placeholder="New username" autocomplete="off" autofocus style="width: 200px;" rows="1" data-bind="value: newUsername" />
+        <input type="text" placeholder="Old Username" autocomplete="off" autofocus style="width: 200px;" rows="1" data-bind="visible: checkOwnedPackages, value: oldUsername" />
+        <input type="checkbox" id="checkNonDeletedOwnedPackages" data-bind="checked: checkOwnedPackages" />
+        <label for="checkNonDeletedOwnedPackages">Check Non-Deleted Owned Packages</label>
         <button type="submit" data-bind="click: validateNewUsername">Validate</button>
         <div style="display:none" data-bind="visible: errorNewUsername">
             @ViewHelpers.AlertDanger(@<text><span data-bind="text: errorNewUsername"></span></text>)
@@ -69,6 +72,18 @@
             <div style="display:none;" data-bind="visible: validatationUsernameResult.IsAvailable()"><i class="ms-Icon ms-Icon--CompletedSolid" style="color: green"></i> Username is available.</div>
             <div style="display:none;" data-bind="visible: !validatationUsernameResult.IsAvailable()"><i class="ms-Icon ms-Icon--StatusErrorFull" style="color: red"></i> Username is already taken.</div>
         </div>
+        <table class="table" id="ownedPackages" style="display: none" data-bind="visible: ownedPackages().length > 0 && checkOwnedPackages" aria-label="owned packages">
+            <thead>
+                <tr>
+                    <th>Owned Package Id</th>
+                </tr>
+            </thead>
+            <tbody data-bind="foreach: ownedPackages">
+                <tr>
+                    <td><span data-bind="text: $data"></span></td>
+                </tr>
+            </tbody>
+        </table>
     </div>
 
     <div>
@@ -105,6 +120,9 @@
                 }
                 this.accountAdministrators = ko.observableArray([]);
                 this.newUsername = ko.observable('');
+                this.oldUsername = ko.observable('');
+                this.checkOwnedPackages = ko.observable(false);
+                this.ownedPackages = ko.observableArray([]);
                 this.validatationUsernameResult = {
                     IsFormatValid: ko.observable(false),
                     IsAvailable: ko.observable(false)
@@ -161,14 +179,15 @@
 
                 this.validateNewUsername = function () {
                     $self.doneValidatingNewUsername(false);
+                    $self.ownedPackages([]);
                     $self.errorNewUsername('');
 
                     if (!$self.newUsername()) {
                         $self.errorNewUsername('Username cannot be null or empty.');
                         return;
                     }
 
-                    var queryParams = '?newUsername=' + encodeURIComponent($self.newUsername().trim());
+                    var queryParams = '?newUsername=' + encodeURIComponent($self.newUsername().trim()) + '&oldUsername=' + encodeURIComponent($self.oldUsername().trim()) + '&checkOwnedPackages=' + encodeURIComponent($self.checkOwnedPackages());
 
                     $.ajax({
                         url: actionUrlValidateNewUsername + queryParams,
@@ -177,6 +196,7 @@
                         success: function (data) {
                             $self.validatationUsernameResult.IsFormatValid(data.IsFormatValid);
                             $self.validatationUsernameResult.IsAvailable(data.IsAvailable);
+                            $self.ownedPackages(data.OwnedPackageIds);
                         }
                     })
                     .fail(function (jqXhr, textStatus, errorThrown) {

tests/NuGetGallery.Facts/Areas/Admin/Controllers/ChangeUsernameControllerFacts.cs

@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.Collections.Generic;
 using System.Linq;
 using System.Net;
 using System.Threading.Tasks;
@@ -104,11 +105,13 @@ public void WhenValidOrganizationAccountReturnsAccountWithAdministrators()
         public class ValidateNewUsernameMethod : FactsBase
         {
             [Theory]
-            [InlineData("")]
-            [InlineData(null)]
-            public void WhenInvalidNewUsernameReturnsBadRequestStatusCode(string newUsername)
+            [InlineData("", true)]
+            [InlineData("", false)]
+            [InlineData(null, true)]
+            [InlineData(null, false)]
+            public void WhenInvalidNewUsernameReturnsBadRequestStatusCode(string newUsername, bool checkOwnedPackages)
             {
-                var result = ChangeUsernameController.ValidateNewUsername(newUsername) as JsonResult;
+                var result = ChangeUsernameController.ValidateNewUsername(newUsername, checkOwnedPackages) as JsonResult;
 
                 Assert.Equal(((int)HttpStatusCode.BadRequest), ChangeUsernameController.Response.StatusCode);
                 Assert.Equal("Username cannot be null or empty.", result.Data);
@@ -120,13 +123,35 @@ public void WhenInvalidNewUsernameReturnsBadRequestStatusCode(string newUsername
             [InlineData("availableUsername", true, true)]
             public void WhenValidNewUsernameReturnsValidations(string newUsername, bool isFormatValid, bool isAvailable)
             {
-                var result = ChangeUsernameController.ValidateNewUsername(newUsername) as JsonResult;
+                var result = ChangeUsernameController.ValidateNewUsername(newUsername, checkOwnedPackages: false) as JsonResult;
 
                 var validations = result.Data as ValidateUsernameResult;
 
                 Assert.Equal(isFormatValid, validations.IsFormatValid);
                 Assert.Equal(isAvailable, validations.IsAvailable);
             }
+
+            [Theory]
+            [InlineData("someNewUsername", "testOrganization", false)]
+            [InlineData("someNewUsername", "testUser", true)]
+            public void WhenCheckOwnedPackagesReturnThem(string newUsername, string oldUsername, bool expectPackages)
+            {
+                var result = ChangeUsernameController.ValidateNewUsername(newUsername, checkOwnedPackages: true, oldUsername) as JsonResult;
+
+                var validations = result.Data as ValidateUsernameResult;
+
+                Assert.True(validations.IsFormatValid);
+                Assert.True(validations.IsAvailable);
+
+                if(expectPackages)
+                {
+                    Assert.NotEmpty(validations.OwnedPackageIds);
+                }
+                else
+                {
+                    Assert.Empty(validations.OwnedPackageIds);
+                }
+            }
         }
 
         public class ChangeUsernameMethod : FactsBase
@@ -211,6 +236,32 @@ public async void WhenValidAccountAndNewUsernameReturnsOkStatusCode()
                 Assert.Equal(((int)HttpStatusCode.OK), ChangeUsernameController.Response.StatusCode);
                 Assert.Equal("Account renamed successfully!", result.Data);
             }
+
+            [Fact]
+            public async void WhenValidAccountCasingChangeAndNewUsernameReturnsOkStatusCode()
+            {
+                var oldUsername = IndividualAccount.Username;
+
+                var result = await ChangeUsernameController.ChangeUsername(IndividualAccount.Username, IndividualAccount.Username.ToUpper()) as JsonResult;
+
+                GetMock<IUserService>().Verify(u => u.FindByUsername(oldUsername, false));
+                GetMock<IUserService>().Verify(u => u.FindByUsername(oldUsername.ToUpper(), true));
+                GetMock<IEntityRepository<User>>().Verify(r => r.InsertOnCommit(It.IsAny<User>()), Times.Never());
+                GetMock<IEntitiesContext>().Verify(c => c.SaveChangesAsync());
+
+                Assert.Equal(oldUsername.ToUpper(), IndividualAccount.Username);
+                Assert.Equal(((int)HttpStatusCode.OK), ChangeUsernameController.Response.StatusCode);
+                Assert.Equal("Account renamed successfully!", result.Data);
+            }
+
+            [Fact]
+            public async void FailWhenAccountCasingNotAvailable()
+            {
+                var result = await ChangeUsernameController.ChangeUsername(IndividualAccount.Username, OrganizationAccount.Username.ToUpper()) as JsonResult;
+
+                Assert.Equal(((int)HttpStatusCode.BadRequest), ChangeUsernameController.Response.StatusCode);
+                Assert.Equal("New username validation failed.", result.Data);
+            }
         }
 
         public class FactsBase : TestContainer
@@ -227,29 +278,36 @@ public FactsBase()
                 OrganizationAccount = new Fakes().Organization;
                 OrganizationAdministrator = new Fakes().OrganizationAdmin;
 
+                // FindByUsername is case-insensitive in the database, so we need to simulate that here
                 GetMock<IUserService>()
-                    .Setup(u => u.FindByUsername(IndividualAccount.Username, It.IsAny<bool>()))
+                    .Setup(u => u.FindByUsername(It.Is<string>(x => string.Equals(IndividualAccount.Username, x, System.StringComparison.OrdinalIgnoreCase)), It.IsAny<bool>()))
                     .Returns(IndividualAccount);
                 GetMock<IUserService>()
-                    .Setup(u => u.FindByUsername(IndividualAccount.EmailAddress, false))
+                    .Setup(u => u.FindByUsername(It.Is<string>(x => string.Equals(IndividualAccount.EmailAddress, x, System.StringComparison.OrdinalIgnoreCase)), false))
                     .Returns(IndividualAccount);
 
                 GetMock<IUserService>()
-                    .Setup(u => u.FindByUsername(OrganizationAccount.Username, true))
+                    .Setup(u => u.FindByUsername(It.Is<string>(x => string.Equals(OrganizationAccount.Username, x, System.StringComparison.OrdinalIgnoreCase)), true))
                     .Returns(OrganizationAccount);
                 GetMock<IUserService>()
-                    .Setup(u => u.FindByUsername(OrganizationAccount.EmailAddress, true))
+                    .Setup(u => u.FindByUsername(It.Is<string>(x => string.Equals(OrganizationAccount.EmailAddress, x, System.StringComparison.OrdinalIgnoreCase)), true))
                     .Returns(OrganizationAccount);
                 GetMock<IUserService>()
-                    .Setup(u => u.FindByUsername(AvailableUsername, true))
+                    .Setup(u => u.FindByUsername(It.Is<string>(x => string.Equals(AvailableUsername, x, System.StringComparison.OrdinalIgnoreCase)), true))
                     .ReturnsNull();
 
+                GetMock<IPackageService>().Setup(p => p.FindPackagesByOwner(It.IsAny<User>(), It.IsAny<bool>(), It.IsAny<bool>()))
+                    .Returns(new List<Package>());
+                GetMock<IPackageService>().Setup(p => p.FindPackagesByOwner(It.Is<User>(u => u.Username == "testUser"), It.IsAny<bool>(), It.IsAny<bool>()))
+                    .Returns(new List<Package>() { new Package() { PackageRegistration = new PackageRegistration() { Id = "testPackage" } }});
+
                 ChangeUsernameController = new ChangeUsernameController(
                     GetMock<IUserService>().Object,
                     GetMock<IEntityRepository<User>>().Object,
                     GetMock<IEntitiesContext>().Object,
                     GetMock<IDateTimeProvider>().Object,
-                    GetMock<IAuditingService>().Object);
+                    GetMock<IAuditingService>().Object,
+                    GetMock<IPackageService>().Object);
 
                 TestUtility.SetupHttpContextMockForUrlGeneration(new Mock<HttpContextBase>(), ChangeUsernameController);
             }

tests/NuGetGallery.Facts/Framework/Fakes.cs

@@ -31,7 +31,7 @@ public Fakes()
 
             ApiKeyV3PlaintextValue = "889e180e-335c-491a-ac26-e83c4bd31d87";
 
-            User = new User("testUser")
+            User = new User("testUser") // NOTE: Do not change the casing of this username. It will break tests for the ChangeUsername in the Admin Panel
             {
                 Key = key++,
                 EmailAddress = "[email protected]",
@@ -58,7 +58,7 @@ public Fakes()
 
             ApiKeyV4PlaintextValue = apiKeyV4PlaintextValue;
 
-            Organization = new Organization("testOrganization")
+            Organization = new Organization("testOrganization") // NOTE: Do not change the casing of this username. It will break tests for the ChangeUsername in the Admin Panel
             {
                 Key = key++,
                 EmailAddress = "[email protected]",