diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 97c1399719..ba8c1a84cd 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -144,6 +144,8 @@ public static class FeatureFlagKeys public const string Otp6Digits = "pm-18612-otp-6-digits"; public const string FailedTwoFactorEmail = "pm-24425-send-2fa-failed-email"; 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"; /* Autofill Team */ public const string IdpAutoSubmitLogin = "idp-auto-submit-login"; diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index dc27842210..136c3f7298 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -1,4 +1,5 @@ -using Bit.Core.Auth.Entities; +using Bit.Core; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Auth.Utilities; @@ -7,6 +8,7 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.KeyManagement.Models.Response; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Core.Utilities; using Bit.Identity.Utilities; @@ -24,6 +26,7 @@ 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!; @@ -34,13 +37,15 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder ICurrentContext currentContext, IDeviceRepository deviceRepository, IOrganizationUserRepository organizationUserRepository, - ILoginApprovingClientTypes loginApprovingClientTypes + ILoginApprovingClientTypes loginApprovingClientTypes, + IFeatureService featureService ) { _currentContext = currentContext; _deviceRepository = deviceRepository; _organizationUserRepository = organizationUserRepository; _loginApprovingClientTypes = loginApprovingClientTypes; + _featureService = featureService; } public IUserDecryptionOptionsBuilder ForUser(User user) @@ -65,8 +70,10 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder { if (credential.GetPrfStatus() == WebAuthnPrfStatus.Enabled) { - _options.WebAuthnPrfOption = new WebAuthnPrfDecryptionOption(credential.EncryptedPrivateKey, credential.EncryptedUserKey); + _options.WebAuthnPrfOption = + new WebAuthnPrfDecryptionOption(credential.EncryptedPrivateKey, credential.EncryptedUserKey); } + return this; } @@ -74,7 +81,7 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder { BuildMasterPasswordUnlock(); BuildKeyConnectorOptions(); - await BuildTrustedDeviceOptions(); + await BuildTrustedDeviceOptionsAsync(); return _options; } @@ -87,13 +94,14 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder } var ssoConfigurationData = _ssoConfig.GetData(); - if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) + if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && + !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) { _options.KeyConnectorOption = new KeyConnectorUserDecryptionOption(ssoConfigurationData.KeyConnectorUrl); } } - private async Task BuildTrustedDeviceOptions() + private async Task BuildTrustedDeviceOptionsAsync() { // TrustedDeviceEncryption only exists for SSO, if that changes then these guards should change if (_ssoConfig == null) @@ -101,7 +109,8 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder return; } - var isTdeActive = _ssoConfig.GetData() is { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption }; + var isTdeActive = _ssoConfig.GetData() is + { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption }; var isTdeOffboarding = !_user.HasMasterPassword() && _device != null && _device.IsTrusted() && !isTdeActive; if (!isTdeActive && !isTdeOffboarding) { @@ -120,25 +129,51 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder if (_device != null) { var allDevices = await _deviceRepository.GetManyByUserIdAsync(_user.Id); - // Checks if the current user has any devices that are capable of approving login with device requests except for - // their current device. - // NOTE: this doesn't check for if the users have configured the devices to be capable of approving requests as that is a client side setting. - hasLoginApprovingDevice = allDevices.Any(d => d.Identifier != _device.Identifier && _loginApprovingClientTypes.TypesThatCanApprove.Contains(DeviceTypes.ToClientType(d.Type))); + // Checks if the current user has any devices that are capable of approving login with device requests + // except for their current device. + hasLoginApprovingDevice = allDevices.Any(d => + d.Identifier != _device.Identifier && + _loginApprovingClientTypes.TypesThatCanApprove.Contains(DeviceTypes.ToClientType(d.Type))); } - // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP - var hasManageResetPasswordPermission = false; - // 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 + // Just-in-time-provisioned users, which can include users invited to a TDE organization with SSO and granted + // the Admin/Owner role or Custom user role with ManageResetPassword permission, will not have claims available + // in context to reflect this permission if granted as part of an invite for the current organization. + // Therefore, as written today, CurrentContext will not surface those permissions for those users. + // In order to make this check accurate at first login for all applicable cases, we have to go back to the + // database record. + // 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); + } - hasManageResetPasswordPermission |= organizationUser != null && (organizationUser.Type == OrganizationUserType.Owner || organizationUser.Type == OrganizationUserType.Admin); // 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); @@ -149,6 +184,31 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder isTdeOffboarding, encryptedPrivateKey, encryptedUserKey); + return; + + 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; + } + + var organizationUserHasResetPasswordPermission = + // The repository will pull users in all statuses, so we also need to ensure that revoked-status users do not have + // permissions sent down. + organizationUser.Status is OrganizationUserStatusType.Invited or OrganizationUserStatusType.Accepted or + OrganizationUserStatusType.Confirmed && + // Admins and owners get ManageResetPassword functionally "for free" through their role. + (organizationUser.Type is OrganizationUserType.Admin or OrganizationUserType.Owner || + // Custom users can have the ManagePasswordReset permission assigned directly. + organizationUser.GetPermissions() is { ManageResetPassword: true }); + + return organizationUserHasResetPasswordPermission || + // A provider user for the given organization gets ManageResetPassword through that relationship. + await _currentContext.ProviderUserForOrgAsync(_ssoConfig.OrganizationId); + } } private void BuildMasterPasswordUnlock() diff --git a/test/Identity.Test/AutoFixture/OrganizationUserFixtures.cs b/test/Identity.Test/AutoFixture/OrganizationUserFixtures.cs new file mode 100644 index 0000000000..e4d40601c5 --- /dev/null +++ b/test/Identity.Test/AutoFixture/OrganizationUserFixtures.cs @@ -0,0 +1,26 @@ +using System.Reflection; +using AutoFixture; +using AutoFixture.Xunit2; +using Bit.Core.Entities; +using Bit.Core.Models.Data; +using Bit.Core.Utilities; + +namespace Bit.Identity.Test.AutoFixture; + +internal class OrganizationUserWithDefaultPermissionsCustomization : ICustomization +{ + public void Customize(IFixture fixture) + { + fixture.Customize(composer => composer + // On OrganizationUser, Permissions can be JSON data (as string) or sometimes null. + // Entity APIs should prevent it from being anything else. + // An un-modified fixture for OrganizationUser will return a bare string Permissions{guid} + // in the member, throwing a JsonException on deserialization of a bare string. + .With(organizationUser => organizationUser.Permissions, CoreHelpers.ClassToJsonData(new Permissions()))); + } +} + +public class OrganizationUserWithDefaultPermissionsAttribute : CustomizeAttribute +{ + public override ICustomization GetCustomization(ParameterInfo parameter) => new OrganizationUserWithDefaultPermissionsCustomization(); +} diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index b44dfe8d5f..37e88b0ec0 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -1,15 +1,20 @@ -using Bit.Core.Auth.Entities; +using Bit.Core; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; using Bit.Core.Context; using Bit.Core.Entities; 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; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; +using User = Bit.Core.Entities.User; namespace Bit.Identity.Test.IdentityServer; @@ -20,6 +25,7 @@ public class UserDecryptionOptionsBuilderTests private readonly IOrganizationUserRepository _organizationUserRepository; private readonly ILoginApprovingClientTypes _loginApprovingClientTypes; private readonly UserDecryptionOptionsBuilder _builder; + private readonly IFeatureService _featureService; public UserDecryptionOptionsBuilderTests() { @@ -27,7 +33,8 @@ public class UserDecryptionOptionsBuilderTests _deviceRepository = Substitute.For(); _organizationUserRepository = Substitute.For(); _loginApprovingClientTypes = Substitute.For(); - _builder = new UserDecryptionOptionsBuilder(_currentContext, _deviceRepository, _organizationUserRepository, _loginApprovingClientTypes); + _featureService = Substitute.For(); + _builder = new UserDecryptionOptionsBuilder(_currentContext, _deviceRepository, _organizationUserRepository, _loginApprovingClientTypes, _featureService); var user = new User(); _builder.ForUser(user); } @@ -220,19 +227,65 @@ public class UserDecryptionOptionsBuilderTests Assert.False(result.TrustedDeviceOption?.HasLoginApprovingDevice); } - [Theory, BitAutoData] + /// + /// 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) + CurrentContextOrganization organization, + [OrganizationUserWithDefaultPermissions] OrganizationUser organizationUser, + User user) { configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; ssoConfig.Data = configurationData.Serialize(); ssoConfig.OrganizationId = organization.Id; - _currentContext.Organizations.Returns(new List(new CurrentContextOrganization[] { organization })); + _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.WithSso(ssoConfig).BuildAsync(); + var result = await _builder.ForUser(user).WithSso(ssoConfig).BuildAsync(); + + Assert.True(result.TrustedDeviceOption?.HasManageResetPasswordPermission); + } + + [Theory] + [BitAutoData(OrganizationUserType.Custom)] + public async Task Build_WhenManageResetPasswordPermissions_ShouldFetchUserFromRepositoryAndReturnHasManageResetPasswordPermissionTrue( + OrganizationUserType organizationUserType, + SsoConfig ssoConfig, + SsoConfigurationData configurationData, + CurrentContextOrganization organization, + [OrganizationUserWithDefaultPermissions] OrganizationUser organizationUser, + User user) + { + _featureService.IsEnabled(FeatureFlagKeys.PM23174ManageAccountRecoveryPermissionDrivesTheNeedToSetMasterPassword) + .Returns(true); + configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; + ssoConfig.Data = configurationData.Serialize(); + ssoConfig.OrganizationId = organization.Id; + 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); } @@ -241,7 +294,7 @@ public class UserDecryptionOptionsBuilderTests public async Task Build_WhenIsOwnerInvite_ShouldReturnHasManageResetPasswordPermissionTrue( SsoConfig ssoConfig, SsoConfigurationData configurationData, - OrganizationUser organizationUser, + [OrganizationUserWithDefaultPermissions] OrganizationUser organizationUser, User user) { configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; @@ -258,7 +311,7 @@ public class UserDecryptionOptionsBuilderTests public async Task Build_WhenIsAdminInvite_ShouldReturnHasManageResetPasswordPermissionTrue( SsoConfig ssoConfig, SsoConfigurationData configurationData, - OrganizationUser organizationUser, + [OrganizationUserWithDefaultPermissions] OrganizationUser organizationUser, User user) { configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; @@ -275,7 +328,7 @@ public class UserDecryptionOptionsBuilderTests public async Task Build_WhenUserHasEnrolledIntoPasswordReset_ShouldReturnHasAdminApprovalTrue( SsoConfig ssoConfig, SsoConfigurationData configurationData, - OrganizationUser organizationUser, + [OrganizationUserWithDefaultPermissions] OrganizationUser organizationUser, User user) { configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption;