Skip to content

Commit

Permalink
Added check for owned packages and some tests
Browse files Browse the repository at this point in the history
  • Loading branch information
RiadGahlouz committed Jan 3, 2024
1 parent 84e2929 commit fb21e0e
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
Expand All @@ -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);

Expand All @@ -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]
Expand Down Expand Up @@ -83,7 +87,7 @@ public ActionResult VerifyAccount(string accountEmailOrUsername)
}

[HttpGet]
public ActionResult ValidateNewUsername(string newUsername, bool checkOwnedPackages)
public ActionResult ValidateNewUsername(string newUsername, bool checkOwnedPackages, string oldUsername = "")
{
if (string.IsNullOrEmpty(newUsername))
{
Expand All @@ -92,6 +96,20 @@ public ActionResult ValidateNewUsername(string newUsername, bool checkOwnedPacka

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);
}

Expand Down
3 changes: 3 additions & 0 deletions src/NuGetGallery/Areas/Admin/Models/ValidateUsernameResult.cs
Original file line number Diff line number Diff line change
@@ -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>();
}
}
21 changes: 19 additions & 2 deletions src/NuGetGallery/Areas/Admin/Views/ChangeUsername/Index.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@
<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="checkbox" id="checkNonDeletedOwnedPackages" data-bind="value: checkOwnedPackages" />
<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">
Expand All @@ -71,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>
Expand Down Expand Up @@ -107,7 +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)
Expand Down Expand Up @@ -164,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()) + '&checkOwnedPackages=' + encodeURIComponent($self.checkOwnedPackages());
var queryParams = '?newUsername=' + encodeURIComponent($self.newUsername().trim()) + '&oldUsername=' + encodeURIComponent($self.oldUsername().trim()) + '&checkOwnedPackages=' + encodeURIComponent($self.checkOwnedPackages());
$.ajax({
url: actionUrlValidateNewUsername + queryParams,
Expand All @@ -180,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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/NuGetGallery.Facts/Framework/Fakes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]",
Expand All @@ -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]",
Expand Down

0 comments on commit fb21e0e

Please sign in to comment.