diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 6d2c2a167321..f7329809fae0 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -158,8 +158,6 @@ public static class FeatureFlagKeys public const string Otp6Digits = "pm-18612-otp-6-digits"; public const string PM24579_PreventSsoOnExistingNonCompliantUsers = "pm-24579-prevent-sso-on-existing-non-compliant-users"; public const string DisableAlternateLoginMethods = "pm-22110-disable-alternate-login-methods"; - public const string PM23174ManageAccountRecoveryPermissionDrivesTheNeedToSetMasterPassword = - "pm-23174-manage-account-recovery-permission-drives-the-need-to-set-master-password"; public const string MJMLBasedEmailTemplates = "mjml-based-email-templates"; public const string MjmlWelcomeEmailTemplates = "pm-21741-mjml-welcome-email"; public const string MarketingInitiatedPremiumFlow = "pm-26140-marketing-initiated-premium-flow"; diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index fddc77c80608..56b4bb0dcf9c 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -1,5 +1,4 @@ -using Bit.Core; -using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Auth.Utilities; @@ -8,7 +7,6 @@ using Bit.Core.Enums; using Bit.Core.KeyManagement.Models.Api.Response; using Bit.Core.Repositories; -using Bit.Core.Services; using Bit.Core.Utilities; using Bit.Identity.Utilities; @@ -26,8 +24,6 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder private readonly IDeviceRepository _deviceRepository; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly ILoginApprovingClientTypes _loginApprovingClientTypes; - private readonly IFeatureService _featureService; - private UserDecryptionOptions _options = new UserDecryptionOptions(); private User _user = null!; private SsoConfig? _ssoConfig; @@ -37,15 +33,13 @@ public UserDecryptionOptionsBuilder( ICurrentContext currentContext, IDeviceRepository deviceRepository, IOrganizationUserRepository organizationUserRepository, - ILoginApprovingClientTypes loginApprovingClientTypes, - IFeatureService featureService + ILoginApprovingClientTypes loginApprovingClientTypes ) { _currentContext = currentContext; _deviceRepository = deviceRepository; _organizationUserRepository = organizationUserRepository; _loginApprovingClientTypes = loginApprovingClientTypes; - _featureService = featureService; } public IUserDecryptionOptionsBuilder ForUser(User user) @@ -145,34 +139,7 @@ private async Task BuildTrustedDeviceOptionsAsync() // In the TDE flow, the users will have been JIT-provisioned at SSO callback time, and the relationship between // user and organization user will have been codified. var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(_ssoConfig.OrganizationId, _user.Id); - var hasManageResetPasswordPermission = false; - if (_featureService.IsEnabled(FeatureFlagKeys.PM23174ManageAccountRecoveryPermissionDrivesTheNeedToSetMasterPassword)) - { - hasManageResetPasswordPermission = await EvaluateHasManageResetPasswordPermission(); - } - else - { - // TODO: PM-26065 remove use of above feature flag from the server, and remove this branching logic, which - // has been replaced by EvaluateHasManageResetPasswordPermission. - // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP. - // When removing feature flags, please also see notes and removals intended for test suite in - // Build_WhenManageResetPasswordPermissions_ShouldReturnHasManageResetPasswordPermissionTrue. - - // when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here - if (_currentContext.Organizations != null && _currentContext.Organizations.Any(o => o.Id == _ssoConfig.OrganizationId)) - { - // TDE requires single org so grabbing first org & id is fine. - hasManageResetPasswordPermission = await _currentContext.ManageResetPassword(_ssoConfig!.OrganizationId); - } - - // If sso configuration data is not null then I know for sure that ssoConfiguration isn't null - - // NOTE: Commented from original impl because the organization user repository call has been hoisted to support - // branching paths through flagging. - //organizationUser = await _organizationUserRepository.GetByOrganizationAsync(_ssoConfig.OrganizationId, _user.Id); - - hasManageResetPasswordPermission |= organizationUser != null && (organizationUser.Type == OrganizationUserType.Owner || organizationUser.Type == OrganizationUserType.Admin); - } + var hasManageResetPasswordPermission = await EvaluateHasManageResetPasswordPermission(); // They are only able to be approved by an admin if they have enrolled is reset password var hasAdminApproval = organizationUser != null && !string.IsNullOrEmpty(organizationUser.ResetPasswordKey); @@ -186,10 +153,10 @@ private async Task BuildTrustedDeviceOptionsAsync() encryptedUserKey); return; + /// Determine if the user has manage reset password permission, + /// as post-SSO logic requires it for forcing users with this permission to set a password. async Task EvaluateHasManageResetPasswordPermission() { - // PM-23174 - // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP if (organizationUser == null) { return false; diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 37e88b0ec054..01f693bee9ce 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -1,5 +1,4 @@ -using Bit.Core; -using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; using Bit.Core.Context; @@ -7,7 +6,6 @@ using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Repositories; -using Bit.Core.Services; using Bit.Identity.IdentityServer; using Bit.Identity.Test.AutoFixture; using Bit.Identity.Utilities; @@ -25,7 +23,6 @@ public class UserDecryptionOptionsBuilderTests private readonly IOrganizationUserRepository _organizationUserRepository; private readonly ILoginApprovingClientTypes _loginApprovingClientTypes; private readonly UserDecryptionOptionsBuilder _builder; - private readonly IFeatureService _featureService; public UserDecryptionOptionsBuilderTests() { @@ -33,8 +30,7 @@ public UserDecryptionOptionsBuilderTests() _deviceRepository = Substitute.For(); _organizationUserRepository = Substitute.For(); _loginApprovingClientTypes = Substitute.For(); - _featureService = Substitute.For(); - _builder = new UserDecryptionOptionsBuilder(_currentContext, _deviceRepository, _organizationUserRepository, _loginApprovingClientTypes, _featureService); + _builder = new UserDecryptionOptionsBuilder(_currentContext, _deviceRepository, _organizationUserRepository, _loginApprovingClientTypes); var user = new User(); _builder.ForUser(user); } @@ -227,43 +223,6 @@ public async Task Build_WhenHasLoginApprovingDevice_ShouldApprovingDeviceFalse( Assert.False(result.TrustedDeviceOption?.HasLoginApprovingDevice); } - /// - /// This logic has been flagged as part of PM-23174. - /// When removing the server flag, please also remove this test, and remove the FeatureService - /// dependency from this suite and the following test. - /// - /// - /// - /// - /// - /// - /// - [Theory] - [BitAutoData(OrganizationUserType.Custom)] - public async Task Build_WhenManageResetPasswordPermissions_ShouldReturnHasManageResetPasswordPermissionTrue( - OrganizationUserType organizationUserType, - SsoConfig ssoConfig, - SsoConfigurationData configurationData, - CurrentContextOrganization organization, - [OrganizationUserWithDefaultPermissions] OrganizationUser organizationUser, - User user) - { - configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; - ssoConfig.Data = configurationData.Serialize(); - ssoConfig.OrganizationId = organization.Id; - _currentContext.Organizations.Returns([organization]); - _currentContext.ManageResetPassword(organization.Id).Returns(true); - organizationUser.Type = organizationUserType; - organizationUser.OrganizationId = organization.Id; - organizationUser.UserId = user.Id; - organizationUser.SetPermissions(new Permissions() { ManageResetPassword = true }); - _organizationUserRepository.GetByOrganizationAsync(ssoConfig.OrganizationId, user.Id).Returns(organizationUser); - - var result = await _builder.ForUser(user).WithSso(ssoConfig).BuildAsync(); - - Assert.True(result.TrustedDeviceOption?.HasManageResetPasswordPermission); - } - [Theory] [BitAutoData(OrganizationUserType.Custom)] public async Task Build_WhenManageResetPasswordPermissions_ShouldFetchUserFromRepositoryAndReturnHasManageResetPasswordPermissionTrue( @@ -274,8 +233,6 @@ public async Task Build_WhenManageResetPasswordPermissions_ShouldFetchUserFromRe [OrganizationUserWithDefaultPermissions] OrganizationUser organizationUser, User user) { - _featureService.IsEnabled(FeatureFlagKeys.PM23174ManageAccountRecoveryPermissionDrivesTheNeedToSetMasterPassword) - .Returns(true); configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; ssoConfig.Data = configurationData.Serialize(); ssoConfig.OrganizationId = organization.Id;