diff --git a/src/Api/Controllers/TwoFactorController.cs b/src/Api/Controllers/TwoFactorController.cs index ab00b0be5..7b834dd47 100644 --- a/src/Api/Controllers/TwoFactorController.cs +++ b/src/Api/Controllers/TwoFactorController.cs @@ -289,7 +289,16 @@ namespace Bit.Api.Controllers { if (await _userService.VerifySecretAsync(user, model.Secret)) { - await _userService.SendTwoFactorEmailAsync(user); + var isBecauseNewDeviceLogin = false; + if (user.GetTwoFactorProvider(TwoFactorProviderType.Email) is null + && + await _userService.Needs2FABecauseNewDeviceAsync(user, model.DeviceIdentifier, null)) + { + model.ToUser(user); + isBecauseNewDeviceLogin = true; + } + + await _userService.SendTwoFactorEmailAsync(user, isBecauseNewDeviceLogin); return; } } diff --git a/src/Api/Models/Request/TwoFactorRequestModels.cs b/src/Api/Models/Request/TwoFactorRequestModels.cs index 8bf1c3f3f..846a76c89 100644 --- a/src/Api/Models/Request/TwoFactorRequestModels.cs +++ b/src/Api/Models/Request/TwoFactorRequestModels.cs @@ -203,6 +203,8 @@ namespace Bit.Api.Models.Request [StringLength(256)] public string Email { get; set; } + public string DeviceIdentifier { get; set; } + public User ToUser(User extistingUser) { var providers = extistingUser.GetTwoFactorProviders(); diff --git a/src/Core/IdentityServer/BaseRequestValidator.cs b/src/Core/IdentityServer/BaseRequestValidator.cs index c7e159598..63dd15e74 100644 --- a/src/Core/IdentityServer/BaseRequestValidator.cs +++ b/src/Core/IdentityServer/BaseRequestValidator.cs @@ -319,9 +319,11 @@ namespace Bit.Core.IdentityServer var requires2FA = individualRequired || firstEnabledOrg != null; var requires2FABecauseNewDevice = !requires2FA - && user.EmailVerified - && request.GrantType != "authorization_code" - && await IsNewDeviceAndNotTheFirstOneAsync(user, request); + && + await _userService.Needs2FABecauseNewDeviceAsync( + user, + GetDeviceFromRequest(request)?.Identifier, + request.GrantType); requires2FA = requires2FA || requires2FABecauseNewDevice; @@ -536,22 +538,6 @@ namespace Bit.Core.IdentityServer return await _deviceRepository.GetByIdentifierAsync(GetDeviceFromRequest(request).Identifier, user.Id); } - protected async Task IsNewDeviceAndNotTheFirstOneAsync(User user, ValidatedTokenRequest request) - { - if (user == null) - { - return default; - } - - var devices = await _deviceRepository.GetManyByUserIdAsync(user.Id); - if (!devices.Any()) - { - return false; - } - - return !devices.Any(d => d.Identifier == GetDeviceFromRequest(request)?.Identifier); - } - private async Task SaveDeviceAsync(User user, ValidatedTokenRequest request) { var device = GetDeviceFromRequest(request); diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index f2b2da5f9..c1fce0965 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -78,5 +78,6 @@ namespace Bit.Core.Services Task SendOTPAsync(User user); Task VerifyOTPAsync(User user, string token); Task VerifySecretAsync(User user, string secret); + Task Needs2FABecauseNewDeviceAsync(User user, string deviceIdentifier, string grantType); } } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 0401a19f5..e6ad3b562 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -17,7 +17,9 @@ using Bit.Core.Utilities; using Fido2NetLib; using Fido2NetLib.Objects; using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using File = System.IO.File; @@ -51,6 +53,8 @@ namespace Bit.Core.Services private readonly GlobalSettings _globalSettings; private readonly IOrganizationService _organizationService; private readonly IProviderUserRepository _providerUserRepository; + private readonly IDeviceRepository _deviceRepository; + private readonly IWebHostEnvironment _environment; public UserService( IUserRepository userRepository, @@ -79,7 +83,9 @@ namespace Bit.Core.Services ICurrentContext currentContext, GlobalSettings globalSettings, IOrganizationService organizationService, - IProviderUserRepository providerUserRepository) + IProviderUserRepository providerUserRepository, + IDeviceRepository deviceRepository, + IWebHostEnvironment environment) : base( store, optionsAccessor, @@ -114,6 +120,8 @@ namespace Bit.Core.Services _globalSettings = globalSettings; _organizationService = organizationService; _providerUserRepository = providerUserRepository; + _deviceRepository = deviceRepository; + _environment = environment; } public Guid? GetProperUserId(ClaimsPrincipal principal) @@ -1408,5 +1416,29 @@ namespace Bit.Core.Services ? await VerifyOTPAsync(user, secret) : await CheckPasswordAsync(user, secret); } + + public async Task Needs2FABecauseNewDeviceAsync(User user, string deviceIdentifier, string grantType) + { + return user.EmailVerified + && grantType != "authorization_code" + && !_environment.IsDevelopment() + && await IsNewDeviceAndNotTheFirstOneAsync(user, deviceIdentifier); + } + + private async Task IsNewDeviceAndNotTheFirstOneAsync(User user, string deviceIdentifier) + { + if (user == null) + { + return default; + } + + var devices = await _deviceRepository.GetManyByUserIdAsync(user.Id); + if (!devices.Any()) + { + return false; + } + + return !devices.Any(d => d.Identifier == deviceIdentifier); + } } } diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 7237c865f..6b4bd3a03 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -7,11 +7,14 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Models; using Bit.Core.Models.Business; +using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Hosting; using NSubstitute; using NSubstitute.ReceivedExtensions; using Xunit; @@ -156,5 +159,112 @@ namespace Bit.Core.Test.Services await Assert.ThrowsAsync("No email.", () => sutProvider.Sut.SendTwoFactorEmailAsync(user)); } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task Needs2FABecauseNewDeviceAsync_ReturnsTrue(SutProvider sutProvider, User user) + { + user.Id = Guid.NewGuid(); + user.EmailVerified = true; + const string deviceIdToCheck = "7b01b586-b210-499f-8d52-0c3fdaa646fc"; + const string deviceIdInRepo = "ea29126c-91b7-4cc4-8ce6-00105b37f64a"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(user.Id) + .Returns(Task.FromResult>(new List + { + new Device { Identifier = deviceIdInRepo } + })); + + Assert.True(await sutProvider.Sut.Needs2FABecauseNewDeviceAsync(user, deviceIdToCheck, "password")); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task Needs2FABecauseNewDeviceAsync_ReturnsFalse_When_GranType_Is_AuthorizationCode(SutProvider sutProvider, User user) + { + user.Id = Guid.NewGuid(); + user.EmailVerified = true; + const string deviceIdToCheck = "7b01b586-b210-499f-8d52-0c3fdaa646fc"; + const string deviceIdInRepo = "ea29126c-91b7-4cc4-8ce6-00105b37f64a"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(user.Id) + .Returns(Task.FromResult>(new List + { + new Device { Identifier = deviceIdInRepo } + })); + + Assert.False(await sutProvider.Sut.Needs2FABecauseNewDeviceAsync(user, deviceIdToCheck, "authorization_code")); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task Needs2FABecauseNewDeviceAsync_ReturnsFalse_When_Email_Is_Not_Verified(SutProvider sutProvider, User user) + { + user.Id = Guid.NewGuid(); + user.EmailVerified = false; + const string deviceIdToCheck = "7b01b586-b210-499f-8d52-0c3fdaa646fc"; + const string deviceIdInRepo = "ea29126c-91b7-4cc4-8ce6-00105b37f64a"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(user.Id) + .Returns(Task.FromResult>(new List + { + new Device { Identifier = deviceIdInRepo } + })); + + Assert.False(await sutProvider.Sut.Needs2FABecauseNewDeviceAsync(user, deviceIdToCheck, "password")); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task Needs2FABecauseNewDeviceAsync_ReturnsFalse_When_Is_The_First_Device(SutProvider sutProvider, User user) + { + user.Id = Guid.NewGuid(); + user.EmailVerified = true; + const string deviceIdToCheck = "7b01b586-b210-499f-8d52-0c3fdaa646fc"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(user.Id) + .Returns(Task.FromResult>(new List())); + + Assert.False(await sutProvider.Sut.Needs2FABecauseNewDeviceAsync(user, deviceIdToCheck, "password")); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task Needs2FABecauseNewDeviceAsync_ReturnsFalse_When_DeviceId_Is_Already_In_Repo(SutProvider sutProvider, User user) + { + user.Id = Guid.NewGuid(); + user.EmailVerified = true; + const string deviceIdToCheck = "7b01b586-b210-499f-8d52-0c3fdaa646fc"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(user.Id) + .Returns(Task.FromResult>(new List + { + new Device { Identifier = deviceIdToCheck } + })); + + Assert.False(await sutProvider.Sut.Needs2FABecauseNewDeviceAsync(user, deviceIdToCheck, "password")); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task Needs2FABecauseNewDeviceAsync_ReturnsFalse_When_Environment_Is_Development(SutProvider sutProvider, User user) + { + user.Id = Guid.NewGuid(); + user.EmailVerified = true; + const string deviceIdToCheck = "7b01b586-b210-499f-8d52-0c3fdaa646fc"; + const string deviceIdInRepo = "ea29126c-91b7-4cc4-8ce6-00105b37f64a"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(user.Id) + .Returns(Task.FromResult>(new List + { + new Device { Identifier = deviceIdInRepo } + })); + + sutProvider.GetDependency() + .EnvironmentName + .Returns(Environments.Development); + + Assert.False(await sutProvider.Sut.Needs2FABecauseNewDeviceAsync(user, deviceIdToCheck, "password")); + } } }