Skip to content

HEEDLS-958 - Refactor Promote To Admin to reactivate inactive account… #1175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 10, 2022
Merged

HEEDLS-958 - Refactor Promote To Admin to reactivate inactive account… #1175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf50575
HEEDLS-958 - Refactor Promote To Admin to reactivate inactive account…
DanBloxham-sw Jun 8, 2022
6b9fd01
HEEDLS-958 - tidy data service strings and remove comment.
DanBloxham-sw Jun 8, 2022
012ddfc
HEEDLS-958 - revoke high level permissions from reactivated admin
DanBloxham-sw Jun 9, 2022
21220da
HEEDLS-958 - review markups
DanBloxham-sw Jun 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion DigitalLearningSolutions.Data.Tests/DataServices/PasswordDataServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
public class PasswordDataServiceTests
{
private const string PasswordHashNotYetInDb = "I haven't used this password before!";
private SqlConnection connection = null!;
private PasswordDataService passwordDataService = null!;
private UserDataService userDataService = null!;
private SqlConnection connection = null!;

[SetUp]
public void Setup()
Expand Down Expand Up @@ -47,6 +47,22 @@ public void Set_password_by_candidate_number_should_set_password_correctly()
}
}

[Test]
public void SetPasswordByAdminId_should_set_password_correctly()
{
using var transaction = new TransactionScope();
// Given
const string? password = "hashedPassword";
const int adminId = 1;

// When
passwordDataService.SetPasswordByAdminId(adminId, password);
var result = userDataService.GetAdminUserById(1)!.Password;

// Then
result.Should().Be(password);
}

[Test]
public async Task Setting_password_by_email_sets_password_for_matching_admins()
{
Expand Down
26 changes: 26 additions & 0 deletions ...arningSolutions.Data.Tests/DataServices/UserDataServiceTests/AdminUserDataServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Dapper;
using DigitalLearningSolutions.Data.Tests.TestHelpers;
using FluentAssertions;
using FluentAssertions.Execution;
using NUnit.Framework;

public partial class UserDataServiceTests
Expand Down Expand Up @@ -221,6 +222,31 @@ public void DeactivateAdminUser_updates_user()
}
}

[Test]
public void ReactivateAdminUser_activates_user_and_resets_admin_permissions()
{
using var transaction = new TransactionScope();
// Given
const int adminId = 16;
connection.SetAdminToInactiveWithCentreManagerAndSuperAdminPermissions(adminId);

// When
var deactivatedAdmin = userDataService.GetAdminUserById(adminId)!;
userDataService.ReactivateAdmin(adminId);
var reactivatedAdmin = userDataService.GetAdminUserById(adminId)!;

// Then
using (new AssertionScope())
{
deactivatedAdmin.Active.Should().Be(false);
deactivatedAdmin.IsCentreManager.Should().Be(true);
deactivatedAdmin.IsUserAdmin.Should().Be(true);
reactivatedAdmin.Active.Should().Be(true);
reactivatedAdmin.IsCentreManager.Should().Be(false);
reactivatedAdmin.IsUserAdmin.Should().Be(false);
}
}

[Test]
public void DeleteAdmin_deletes_admin_record()
{
Expand Down
156 changes: 130 additions & 26 deletions DigitalLearningSolutions.Data.Tests/Services/RegistrationServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace DigitalLearningSolutions.Data.Tests.Services
using DigitalLearningSolutions.Data.Tests.TestHelpers;
using FakeItEasy;
using FluentAssertions;
using FluentAssertions.Execution;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging.Abstractions;
using NUnit.Framework;
Expand All @@ -32,7 +33,6 @@ public class RegistrationServiceTests
private ICentresDataService centresDataService = null!;
private IConfiguration config = null!;
private IEmailService emailService = null!;
private IFrameworkNotificationService frameworkNotificationService = null!;
private IPasswordDataService passwordDataService = null!;
private IPasswordResetService passwordResetService = null!;
private IRegistrationDataService registrationDataService = null!;
Expand All @@ -50,7 +50,6 @@ public void Setup()
centresDataService = A.Fake<ICentresDataService>();
config = A.Fake<IConfiguration>();
supervisorDelegateService = A.Fake<ISupervisorDelegateService>();
frameworkNotificationService = A.Fake<IFrameworkNotificationService>();
userDataService = A.Fake<IUserDataService>();

A.CallTo(() => config["CurrentSystemBaseUrl"]).Returns(OldSystemBaseUrl);
Expand All @@ -72,7 +71,6 @@ public void Setup()
centresDataService,
config,
supervisorDelegateService,
frameworkNotificationService,
userDataService,
new NullLogger<RegistrationService>()
);
Expand Down Expand Up @@ -543,7 +541,7 @@ public void RegisterDelegateByCentre_schedules_welcome_email_if_notify_date_set(
model.Email,
NewCandidateNumber,
baseUrl,
model.NotifyDate.Value,
model.NotifyDate!.Value,
"RegisterDelegateByCentre_Refactor"
)
).MustHaveHappened(1, Times.Exactly);
Expand Down Expand Up @@ -659,7 +657,8 @@ public void PromoteDelegateToAdmin_throws_AdminCreationFailedException_if_delega
}

[Test]
public void PromoteDelegateToAdmin_throws_email_in_use_AdminCreationFailedException_if_admin_already_exists()
public void
PromoteDelegateToAdmin_throws_email_in_use_AdminCreationFailedException_if_active_admin_already_exists()
{
// Given
var delegateUser = UserTestHelper.GetDefaultDelegateUser();
Expand All @@ -674,7 +673,80 @@ public void PromoteDelegateToAdmin_throws_email_in_use_AdminCreationFailedExcept
);

// Then
result.Error.Should().Be(AdminCreationError.EmailAlreadyInUse);
using (new AssertionScope())
{
UpdateToExistingAdminAccountMustNotHaveHappened();
result.Error.Should().Be(AdminCreationError.EmailAlreadyInUse);
}
}

[Test]
public void
PromoteDelegateToAdmin_throws_email_in_use_AdminCreationFailedException_if_inactive_admin_at_different_centre_already_exists()
{
// Given
var delegateUser = UserTestHelper.GetDefaultDelegateUser();
var adminUser = UserTestHelper.GetDefaultAdminUser(centreId: 3, active: false);
var adminRoles = new AdminRoles(true, true, true, true, true, true, true);
A.CallTo(() => userDataService.GetDelegateUserById(A<int>._)).Returns(delegateUser);
A.CallTo(() => userDataService.GetAdminUserByEmailAddress(A<string>._)).Returns(adminUser);

// When
var result = Assert.Throws<AdminCreationFailedException>(
() => registrationService.PromoteDelegateToAdmin(adminRoles, 1, 1)
);

// Then
using (new AssertionScope())
{
UpdateToExistingAdminAccountMustNotHaveHappened();
result.Error.Should().Be(AdminCreationError.EmailAlreadyInUse);
}
}

[Test]
public void PromoteDelegateToAdmin_updates_existing_admin_if_inactive_admin_at_same_centre_already_exists()
{
// Given
const int categoryId = 1;
var delegateUser = UserTestHelper.GetDefaultDelegateUser();
var adminUser = UserTestHelper.GetDefaultAdminUser(active: false);
var adminRoles = new AdminRoles(true, true, true, true, true, true, true);
A.CallTo(() => userDataService.GetDelegateUserById(A<int>._)).Returns(delegateUser);
A.CallTo(() => userDataService.GetAdminUserByEmailAddress(A<string>._)).Returns(adminUser);

// When
registrationService.PromoteDelegateToAdmin(adminRoles, categoryId, 1);

// Then
using (new AssertionScope())
{
A.CallTo(() => userDataService.ReactivateAdmin(adminUser.Id)).MustHaveHappenedOnceExactly();
A.CallTo(
() => userDataService.UpdateAdminUser(
delegateUser.FirstName!,
delegateUser.LastName,
delegateUser.EmailAddress!,
delegateUser.ProfileImage,
adminUser.Id
)
).MustHaveHappenedOnceExactly();
A.CallTo(() => passwordDataService.SetPasswordByAdminId(adminUser.Id, delegateUser.Password!))
.MustHaveHappenedOnceExactly();
A.CallTo(
() => userDataService.UpdateAdminUserPermissions(
adminUser.Id,
adminRoles.IsCentreAdmin,
adminRoles.IsSupervisor,
adminRoles.IsNominatedSupervisor,
adminRoles.IsTrainer,
adminRoles.IsContentCreator,
adminRoles.IsContentManager,
adminRoles.ImportOnly,
categoryId
)
).MustHaveHappenedOnceExactly();
}
}

[Test]
Expand All @@ -690,27 +762,31 @@ public void PromoteDelegateToAdmin_calls_data_service_with_expected_value()
registrationService.PromoteDelegateToAdmin(adminRoles, 1, 1);

// Then
A.CallTo(
() => registrationDataService.RegisterAdmin(
A<AdminRegistrationModel>.That.Matches(
a =>
a.FirstName == delegateUser.FirstName &&
a.LastName == delegateUser.LastName &&
a.Email == delegateUser.EmailAddress &&
a.Centre == delegateUser.CentreId &&
a.PasswordHash == delegateUser.Password &&
a.Active &&
a.Approved &&
a.IsCentreAdmin == adminRoles.IsCentreAdmin &&
!a.IsCentreManager &&
a.IsContentManager == adminRoles.IsContentManager &&
a.ImportOnly == adminRoles.IsCmsAdministrator &&
a.IsContentCreator == adminRoles.IsContentCreator &&
a.IsTrainer == adminRoles.IsTrainer &&
a.IsSupervisor == adminRoles.IsSupervisor
using (new AssertionScope())
{
A.CallTo(
() => registrationDataService.RegisterAdmin(
A<AdminRegistrationModel>.That.Matches(
a =>
a.FirstName == delegateUser.FirstName &&
a.LastName == delegateUser.LastName &&
a.Email == delegateUser.EmailAddress &&
a.Centre == delegateUser.CentreId &&
a.PasswordHash == delegateUser.Password &&
a.Active &&
a.Approved &&
a.IsCentreAdmin == adminRoles.IsCentreAdmin &&
!a.IsCentreManager &&
a.IsContentManager == adminRoles.IsContentManager &&
a.ImportOnly == adminRoles.IsCmsAdministrator &&
a.IsContentCreator == adminRoles.IsContentCreator &&
a.IsTrainer == adminRoles.IsTrainer &&
a.IsSupervisor == adminRoles.IsSupervisor
)
)
)
).MustHaveHappened();
).MustHaveHappened();
UpdateToExistingAdminAccountMustNotHaveHappened();
}
}

private void GivenNoPendingSupervisorDelegateRecordsForEmail()
Expand All @@ -735,5 +811,33 @@ private void GivenPendingSupervisorDelegateIdsForEmailAre(IEnumerable<int> super
)
.Returns(supervisorDelegates);
}

private void UpdateToExistingAdminAccountMustNotHaveHappened()
{
A.CallTo(() => userDataService.ReactivateAdmin(A<int>._)).MustNotHaveHappened();
A.CallTo(
() => userDataService.UpdateAdminUser(
A<string>._,
A<string>._,
A<string>._,
A<byte[]>._,
A<int>._
)
).MustNotHaveHappened();
A.CallTo(() => passwordDataService.SetPasswordByAdminId(A<int>._, A<string>._)).MustNotHaveHappened();
A.CallTo(
() => userDataService.UpdateAdminUserPermissions(
A<int>._,
A<bool>._,
A<bool>._,
A<bool>._,
A<bool>._,
A<bool>._,
A<bool>._,
A<bool>._,
A<int>._
)
).MustNotHaveHappened();
}
}
}
12 changes: 12 additions & 0 deletions DigitalLearningSolutions.Data.Tests/TestHelpers/UserTestHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,18 @@ public static CentreAnswersData GetDefaultCentreAnswersData(
return new CentreAnswersData(centreId, jobGroupId, answer1, answer2, answer3, answer4, answer5, answer6);
}

public static void SetAdminToInactiveWithCentreManagerAndSuperAdminPermissions(this DbConnection connection, int adminId)
{
connection.Execute(
@"UPDATE AdminUsers SET
Active = 0,
IsCentreManager = 1,
UserAdmin = 1
WHERE AdminID = @adminId",
new { adminId }
);
}

public static void GivenDelegateUserIsInDatabase(DelegateUser user, SqlConnection sqlConnection)
{
sqlConnection.Execute(
Expand Down
14 changes: 14 additions & 0 deletions DigitalLearningSolutions.Data/DataServices/PasswordDataService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
public interface IPasswordDataService
{
void SetPasswordByCandidateNumber(string candidateNumber, string passwordHash);

void SetPasswordByAdminId(int adminId, string passwordHash);

Task SetPasswordByEmailAsync(string email, string passwordHash);

Task SetPasswordForUsersAsync(IEnumerable<UserReference> users, string passwordHash);
}

Expand All @@ -34,6 +38,16 @@ public void SetPasswordByCandidateNumber(string candidateNumber, string password
);
}

public void SetPasswordByAdminId(int adminId, string passwordHash)
{
connection.Query(
@"UPDATE AdminUsers SET
Password = @passwordHash
WHERE AdminID = @adminID",
new { passwordHash, adminId }
);
}

public async Task SetPasswordByEmailAsync(
string email,
string passwordHash
Expand Down
25 changes: 21 additions & 4 deletions DigitalLearningSolutions.Data/DataServices/UserDataService/AdminUserDataService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,20 +143,20 @@ int categoryId
isContentManager,
importOnly,
categoryId,
adminId
adminId,
}
);
}

public void UpdateAdminUserFailedLoginCount(int adminId, int updatedCount)
{
connection.Execute(
@"UPDATE AdminUsers
@"UPDATE AdminUsers
SET
FailedLoginCount = @updatedCount
WHERE AdminID = @adminId",
new { adminId, updatedCount}
);
new { adminId, updatedCount }
);
}

public void DeactivateAdmin(int adminId)
Expand All @@ -170,6 +170,23 @@ public void DeactivateAdmin(int adminId)
);
}

/// <summary>
/// When we reactivate an admin, we must ensure the admin permissions are not
/// greater than basic levels. Otherwise, a basic admin would be able to
/// "create" admins with more permissions than themselves.
/// </summary>
public void ReactivateAdmin(int adminId)
{
connection.Execute(
@"UPDATE AdminUsers SET
Active = 1,
IsCentreManager = 0,
UserAdmin = 0
WHERE AdminID = @adminId",
new { adminId }
);
}

public void DeleteAdminUser(int adminId)
{
connection.Execute(
Expand Down
2 changes: 2 additions & 0 deletions DigitalLearningSolutions.Data/DataServices/UserDataService/UserDataService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ void UpdateDelegateUserCentrePrompts(

void DeactivateAdmin(int adminId);

void ReactivateAdmin(int adminId);

void ActivateDelegateUser(int delegateId);

int? GetDelegateUserLearningHubAuthId(int delegateId);
Expand Down
Loading