From dd2ea41b7484010431142859d8121d7f3fe5113b Mon Sep 17 00:00:00 2001 From: Patrick-Pimentel-Bitwarden Date: Tue, 13 May 2025 15:43:11 -0400 Subject: [PATCH] Auth/pm 17111/add browser to list of approving clients (#5792) * feat(update-auth-approving-clients): [PM-17111] Add Browser to List of Approving Clients - Initial changes. * feat(update-auth-approving-clients): [PM-17111] Add Browser to List of Approving Clients - Updated tests. * test(update-auth-approving-clients): [PM-17111] Add Browser to List of Approving Clients - Strengthened tests. --- .../UserDecryptionOptionsBuilder.cs | 8 +-- .../Utilities/LoginApprovingClientTypes.cs | 22 ++++++++ .../Utilities/LoginApprovingDeviceTypes.cs | 20 ------- .../UserDecryptionOptionsBuilderTests.cs | 52 +++++++++++++++++-- 4 files changed, 74 insertions(+), 28 deletions(-) create mode 100644 src/Identity/Utilities/LoginApprovingClientTypes.cs delete mode 100644 src/Identity/Utilities/LoginApprovingDeviceTypes.cs diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 2dc1f2926b..34a9d6c573 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -6,6 +6,7 @@ using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Repositories; +using Bit.Core.Utilities; using Bit.Identity.Utilities; namespace Bit.Identity.IdentityServer; @@ -24,7 +25,7 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder private UserDecryptionOptions _options = new UserDecryptionOptions(); private User? _user; - private Core.Auth.Entities.SsoConfig? _ssoConfig; + private SsoConfig? _ssoConfig; private Device? _device; public UserDecryptionOptionsBuilder( @@ -45,7 +46,7 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder return this; } - public IUserDecryptionOptionsBuilder WithSso(Core.Auth.Entities.SsoConfig ssoConfig) + public IUserDecryptionOptionsBuilder WithSso(SsoConfig ssoConfig) { _ssoConfig = ssoConfig; return this; @@ -119,8 +120,7 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder // 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 - .Where(d => d.Identifier != _device.Identifier && LoginApprovingDeviceTypes.Types.Contains(d.Type)) - .Any(); + .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 diff --git a/src/Identity/Utilities/LoginApprovingClientTypes.cs b/src/Identity/Utilities/LoginApprovingClientTypes.cs new file mode 100644 index 0000000000..dd27a87550 --- /dev/null +++ b/src/Identity/Utilities/LoginApprovingClientTypes.cs @@ -0,0 +1,22 @@ +using Bit.Core.Enums; + +namespace Bit.Identity.Utilities; + +public static class LoginApprovingClientTypes +{ + private static readonly IReadOnlyCollection _clientTypesThatCanApprove; + + static LoginApprovingClientTypes() + { + var clientTypes = new List + { + ClientType.Desktop, + ClientType.Mobile, + ClientType.Web, + ClientType.Browser, + }; + _clientTypesThatCanApprove = clientTypes.AsReadOnly(); + } + + public static IReadOnlyCollection TypesThatCanApprove => _clientTypesThatCanApprove; +} diff --git a/src/Identity/Utilities/LoginApprovingDeviceTypes.cs b/src/Identity/Utilities/LoginApprovingDeviceTypes.cs deleted file mode 100644 index b8b11a4d19..0000000000 --- a/src/Identity/Utilities/LoginApprovingDeviceTypes.cs +++ /dev/null @@ -1,20 +0,0 @@ -using Bit.Core.Enums; -using Bit.Core.Utilities; - -namespace Bit.Identity.Utilities; - -public static class LoginApprovingDeviceTypes -{ - private static readonly IReadOnlyCollection _deviceTypes; - - static LoginApprovingDeviceTypes() - { - var deviceTypes = new List(); - deviceTypes.AddRange(DeviceTypes.DesktopTypes); - deviceTypes.AddRange(DeviceTypes.MobileTypes); - deviceTypes.AddRange(DeviceTypes.BrowserTypes); - _deviceTypes = deviceTypes.AsReadOnly(); - } - - public static IReadOnlyCollection Types => _deviceTypes; -} diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 89940275b0..0a6346fe9e 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -6,7 +6,6 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Identity.IdentityServer; -using Bit.Identity.Utilities; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; @@ -102,12 +101,39 @@ public class UserDecryptionOptionsBuilderTests Assert.Equal(device.EncryptedUserKey, result.TrustedDeviceOption?.EncryptedUserKey); } - [Theory, BitAutoData] - public async Task Build_WhenHasLoginApprovingDevice_ShouldApprovingDeviceTrue(SsoConfig ssoConfig, SsoConfigurationData configurationData, User user, Device device, Device approvingDevice) + [Theory] + // Desktop + [BitAutoData(DeviceType.LinuxDesktop)] + [BitAutoData(DeviceType.MacOsDesktop)] + [BitAutoData(DeviceType.WindowsDesktop)] + [BitAutoData(DeviceType.UWP)] + // Mobile + [BitAutoData(DeviceType.Android)] + [BitAutoData(DeviceType.iOS)] + [BitAutoData(DeviceType.AndroidAmazon)] + // Web + [BitAutoData(DeviceType.ChromeBrowser)] + [BitAutoData(DeviceType.FirefoxBrowser)] + [BitAutoData(DeviceType.OperaBrowser)] + [BitAutoData(DeviceType.EdgeBrowser)] + [BitAutoData(DeviceType.IEBrowser)] + [BitAutoData(DeviceType.SafariBrowser)] + [BitAutoData(DeviceType.VivaldiBrowser)] + [BitAutoData(DeviceType.UnknownBrowser)] + // Extension + [BitAutoData(DeviceType.ChromeExtension)] + [BitAutoData(DeviceType.FirefoxExtension)] + [BitAutoData(DeviceType.OperaExtension)] + [BitAutoData(DeviceType.EdgeExtension)] + [BitAutoData(DeviceType.VivaldiExtension)] + [BitAutoData(DeviceType.SafariExtension)] + public async Task Build_WhenHasLoginApprovingDevice_ShouldApprovingDeviceTrue( + DeviceType deviceType, + SsoConfig ssoConfig, SsoConfigurationData configurationData, User user, Device device, Device approvingDevice) { configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; ssoConfig.Data = configurationData.Serialize(); - approvingDevice.Type = LoginApprovingDeviceTypes.Types.First(); + approvingDevice.Type = deviceType; _deviceRepository.GetManyByUserIdAsync(user.Id).Returns(new Device[] { approvingDevice }); var result = await _builder.ForUser(user).WithSso(ssoConfig).WithDevice(device).BuildAsync(); @@ -115,6 +141,24 @@ public class UserDecryptionOptionsBuilderTests Assert.True(result.TrustedDeviceOption?.HasLoginApprovingDevice); } + [Theory] + [BitAutoData(DeviceType.WindowsCLI)] + [BitAutoData(DeviceType.MacOsCLI)] + [BitAutoData(DeviceType.LinuxCLI)] + public async Task Build_WhenHasLoginApprovingDevice_ShouldApprovingDeviceFalse( + DeviceType deviceType, + SsoConfig ssoConfig, SsoConfigurationData configurationData, User user, Device device, Device approvingDevice) + { + configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; + ssoConfig.Data = configurationData.Serialize(); + approvingDevice.Type = deviceType; + _deviceRepository.GetManyByUserIdAsync(user.Id).Returns(new Device[] { approvingDevice }); + + var result = await _builder.ForUser(user).WithSso(ssoConfig).WithDevice(device).BuildAsync(); + + Assert.False(result.TrustedDeviceOption?.HasLoginApprovingDevice); + } + [Theory, BitAutoData] public async Task Build_WhenManageResetPasswordPermissions_ShouldReturnHasManageResetPasswordPermissionTrue( SsoConfig ssoConfig,