From f7f3ea11f2eb558aa5d8580f87a6b3bbf787558b Mon Sep 17 00:00:00 2001 From: Jimmy Vo Date: Wed, 6 Nov 2024 09:35:47 -0500 Subject: [PATCH 1/3] PM-13763 Move ResetPasswordEnrolled to response model to adhere to Liskov Substitution Principle. Ensures request models inherit only relevant properties. --- src/Api/AdminConsole/Public/Models/MemberBaseModel.cs | 8 +------- .../Public/Models/Response/MemberResponseModel.cs | 8 ++++++++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Api/AdminConsole/Public/Models/MemberBaseModel.cs b/src/Api/AdminConsole/Public/Models/MemberBaseModel.cs index 931f63741d7f..c56117ae71d7 100644 --- a/src/Api/AdminConsole/Public/Models/MemberBaseModel.cs +++ b/src/Api/AdminConsole/Public/Models/MemberBaseModel.cs @@ -18,7 +18,6 @@ public MemberBaseModel(OrganizationUser user) Type = user.Type; ExternalId = user.ExternalId; - ResetPasswordEnrolled = user.ResetPasswordKey != null; if (Type == OrganizationUserType.Custom) { @@ -35,7 +34,6 @@ public MemberBaseModel(OrganizationUserUserDetails user) Type = user.Type; ExternalId = user.ExternalId; - ResetPasswordEnrolled = user.ResetPasswordKey != null; if (Type == OrganizationUserType.Custom) { @@ -55,11 +53,7 @@ public MemberBaseModel(OrganizationUserUserDetails user) /// external_id_123456 [StringLength(300)] public string ExternalId { get; set; } - /// - /// Returns true if the member has enrolled in Password Reset assistance within the organization - /// - [Required] - public bool ResetPasswordEnrolled { get; set; } + /// /// The member's custom permissions if the member has a Custom role. If not supplied, all custom permissions will /// default to false. diff --git a/src/Api/AdminConsole/Public/Models/Response/MemberResponseModel.cs b/src/Api/AdminConsole/Public/Models/Response/MemberResponseModel.cs index 6f73532ad4cb..ab6ecbca4497 100644 --- a/src/Api/AdminConsole/Public/Models/Response/MemberResponseModel.cs +++ b/src/Api/AdminConsole/Public/Models/Response/MemberResponseModel.cs @@ -28,6 +28,7 @@ public MemberResponseModel(OrganizationUser user, IEnumerable new AssociationWithPermissionsResponseModel(c)); + ResetPasswordEnrolled = user.ResetPasswordKey != null; } public MemberResponseModel(OrganizationUserUserDetails user, bool twoFactorEnabled, @@ -45,6 +46,7 @@ public MemberResponseModel(OrganizationUserUserDetails user, bool twoFactorEnabl TwoFactorEnabled = twoFactorEnabled; Status = user.Status; Collections = collections?.Select(c => new AssociationWithPermissionsResponseModel(c)); + ResetPasswordEnrolled = user.ResetPasswordKey != null; } /// @@ -93,4 +95,10 @@ public MemberResponseModel(OrganizationUserUserDetails user, bool twoFactorEnabl /// The associated collections that this member can access. /// public IEnumerable Collections { get; set; } + + /// + /// Returns true if the member has enrolled in Password Reset assistance within the organization + /// + [Required] + public bool ResetPasswordEnrolled { get; } } From 6feafcd033466d7935d324126a3ffa2c95774ee3 Mon Sep 17 00:00:00 2001 From: Jimmy Vo Date: Wed, 6 Nov 2024 17:03:41 -0500 Subject: [PATCH 2/3] Add unit tests --- .../Response/MemberResponseModelTests.cs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/Api.Test/AdminConsole/Public/Models/Response/MemberResponseModelTests.cs diff --git a/test/Api.Test/AdminConsole/Public/Models/Response/MemberResponseModelTests.cs b/test/Api.Test/AdminConsole/Public/Models/Response/MemberResponseModelTests.cs new file mode 100644 index 000000000000..52c148b6662f --- /dev/null +++ b/test/Api.Test/AdminConsole/Public/Models/Response/MemberResponseModelTests.cs @@ -0,0 +1,41 @@ +using Bit.Api.AdminConsole.Public.Models.Response; +using Bit.Core.Entities; +using Bit.Core.Models.Data; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.AdminConsole.Public.Models.Response; + + +public class MemberResponseModelTests +{ + [Fact] + public void ResetPasswordEnrolled_ShouldBeTrue_WhenUserHasResetPasswordKey() + { + // Arrange + var user = Substitute.For(); + var collections = Substitute.For>(); + user.ResetPasswordKey = "none-empty"; + + + // Act + var sut = new MemberResponseModel(user, collections); + + // Assert + Assert.True(sut.ResetPasswordEnrolled); + } + + [Fact] + public void ResetPasswordEnrolled_ShouldBeFalse_WhenUserHasResetPasswordKey() + { + // Arrange + var user = Substitute.For(); + var collections = Substitute.For>(); + + // Act + var sut = new MemberResponseModel(user, collections); + + // Assert + Assert.False(sut.ResetPasswordEnrolled); + } +} From 1329a1d510ce88787ef9bf16a3e7d2041956faf8 Mon Sep 17 00:00:00 2001 From: Jimmy Vo Date: Thu, 7 Nov 2024 14:15:47 -0500 Subject: [PATCH 3/3] Fix unit test name --- .../Public/Models/Response/MemberResponseModelTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Api.Test/AdminConsole/Public/Models/Response/MemberResponseModelTests.cs b/test/Api.Test/AdminConsole/Public/Models/Response/MemberResponseModelTests.cs index 52c148b6662f..a9193258b819 100644 --- a/test/Api.Test/AdminConsole/Public/Models/Response/MemberResponseModelTests.cs +++ b/test/Api.Test/AdminConsole/Public/Models/Response/MemberResponseModelTests.cs @@ -26,7 +26,7 @@ public void ResetPasswordEnrolled_ShouldBeTrue_WhenUserHasResetPasswordKey() } [Fact] - public void ResetPasswordEnrolled_ShouldBeFalse_WhenUserHasResetPasswordKey() + public void ResetPasswordEnrolled_ShouldBeFalse_WhenUserDoesNotHaveResetPasswordKey() { // Arrange var user = Substitute.For();