[PM-10742] Pull Device verification into testable service (#4851)

* initial device removal

* Unit Testing

* Added unit tests fixed validator null checks

* Finalized tests

* formatting

* fixed test

* lint

* addressing review notes

* comments

test/Identity.IntegrationTest/RequestValidation/ResourceOwnerPasswordValidatorTests.cs

@@ -4,11 +4,15 @@ using Bit.Core.Auth.Enums;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
 using Bit.Core.Repositories;
+using Bit.Core.Services;
+using Bit.Identity.IdentityServer;
 using Bit.Identity.Models.Request.Accounts;
 using Bit.IntegrationTestCommon.Factories;
 using Bit.Test.Common.AutoFixture.Attributes;
 using Bit.Test.Common.Helpers;
+using Duende.IdentityServer.Validation;
 using Microsoft.AspNetCore.Identity;
+using NSubstitute;
 using Xunit;
 
 namespace Bit.Identity.IntegrationTest.RequestValidation;
@@ -21,6 +25,7 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
     private readonly IdentityApplicationFactory _factory;
     private readonly UserManager<User> _userManager;
     private readonly IAuthRequestRepository _authRequestRepository;
+    private readonly IDeviceService _deviceService;
 
     public ResourceOwnerPasswordValidatorTests(IdentityApplicationFactory factory)
     {
@@ -28,13 +33,13 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
 
         _userManager = _factory.GetService<UserManager<User>>();
         _authRequestRepository = _factory.GetService<IAuthRequestRepository>();
-
+        _deviceService = _factory.GetService<IDeviceService>();
     }
 
     [Fact]
     public async Task ValidateAsync_Success()
     {
-        // Arrange        
+        // Arrange
         await EnsureUserCreatedAsync();
 
         // Act
@@ -53,7 +58,7 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
     [Fact]
     public async Task ValidateAsync_AuthEmailHeaderInvalid_InvalidGrantResponse()
     {
-        // Arrange        
+        // Arrange
         await EnsureUserCreatedAsync();
 
         // Act
@@ -88,12 +93,12 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
     }
 
     /// <summary>
-    /// I would have liked to spy into the IUserService but by spying into the IUserService it 
-    /// creates a Singleton that is not available to the UserManager<User> thus causing the 
+    /// I would have liked to spy into the IUserService but by spying into the IUserService it
+    /// creates a Singleton that is not available to the UserManager<User> thus causing the
     /// RegisterAsync() to create a the user in a different UserStore than the one the
-    /// UserManager<User> has access to. This is an assumption made from observing the behavior while
-    /// writing theses tests. I could be wrong.
-    /// 
+    /// UserManager<User> has access to (This is an assumption made from observing the behavior while
+    /// writing theses tests, I could be wrong).
+    ///
     /// For the time being, verifying that the user is not null confirms that the failure is due to
     /// a bad password.
     /// </summary>
@@ -102,10 +107,11 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
     [Theory, BitAutoData]
     public async Task ValidateAsync_BadPassword_Failure(string badPassword)
     {
-        // Arrange        
+        // Arrange
         await EnsureUserCreatedAsync();
 
         // Verify the User is not null to ensure the failure is due to bad password
+        Assert.NotNull(await _userManager.FindByEmailAsync(DefaultUsername));
 
         // Act
         var context = await _factory.Server.PostAsync("/connect/token",
@@ -113,8 +119,6 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
             context => context.SetAuthEmail(DefaultUsername));
 
         // Assert
-        Assert.NotNull(await _userManager.FindByEmailAsync(DefaultUsername));
-
         var body = await AssertHelper.AssertResponseTypeIs<JsonDocument>(context);
         var root = body.RootElement;
 
@@ -200,8 +204,8 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
         // Assert
 
         /*
-        An improvement on the current failure flow would be to document which part of 
-        the flow failed since all of the failures are basically the same. 
+        An improvement on the current failure flow would be to document which part of
+        the flow failed since all of the failures are basically the same.
         This doesn't build confidence in the tests.
         */
 
@@ -213,6 +217,43 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
         Assert.Equal("Username or password is incorrect. Try again.", errorMessage);
     }
 
+    [Fact]
+    public async Task ValidateAsync_DeviceSaveAsync_ReturnsNullDevice_ErrorResult()
+    {
+        // Arrange
+        var factory = new IdentityApplicationFactory();
+
+        // Stub DeviceValidator
+        factory.SubstituteService<IDeviceValidator>(sub =>
+        {
+            sub.SaveDeviceAsync(Arg.Any<User>(), Arg.Any<ValidatedTokenRequest>())
+                .Returns(null as Device);
+        });
+
+        // Add User
+        await factory.RegisterAsync(new RegisterRequestModel
+        {
+            Email = DefaultUsername,
+            MasterPasswordHash = DefaultPassword
+        });
+        var userManager = factory.GetService<UserManager<User>>();
+        var user = await userManager.FindByEmailAsync(DefaultUsername);
+        Assert.NotNull(user);
+
+        // Act
+        var context = await factory.Server.PostAsync("/connect/token",
+            GetFormUrlEncodedContent(),
+            context => context.SetAuthEmail(DefaultUsername));
+
+        // Assert
+        var body = await AssertHelper.AssertResponseTypeIs<JsonDocument>(context);
+        var root = body.RootElement;
+
+        var errorModel = AssertHelper.AssertJsonProperty(root, "ErrorModel", JsonValueKind.Object);
+        var errorMessage = AssertHelper.AssertJsonProperty(errorModel, "Message", JsonValueKind.String).GetString();
+        Assert.Equal("No device information provided.", errorMessage);
+    }
+
     private async Task EnsureUserCreatedAsync(IdentityApplicationFactory factory = null)
     {
         factory ??= _factory;

test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs

@@ -29,10 +29,9 @@ namespace Bit.Identity.Test.IdentityServer;
 public class BaseRequestValidatorTests
 {
     private UserManager<User> _userManager;
-    private readonly IDeviceRepository _deviceRepository;
-    private readonly IDeviceService _deviceService;
     private readonly IUserService _userService;
     private readonly IEventService _eventService;
+    private readonly IDeviceValidator _deviceValidator;
     private readonly IOrganizationDuoWebTokenProvider _organizationDuoWebTokenProvider;
     private readonly ITemporaryDuoWebV4SDKService _duoWebV4SDKService;
     private readonly IOrganizationRepository _organizationRepository;
@@ -53,10 +52,9 @@ public class BaseRequestValidatorTests
 
     public BaseRequestValidatorTests()
     {
-        _deviceRepository = Substitute.For<IDeviceRepository>();
-        _deviceService = Substitute.For<IDeviceService>();
         _userService = Substitute.For<IUserService>();
         _eventService = Substitute.For<IEventService>();
+        _deviceValidator = Substitute.For<IDeviceValidator>();
         _organizationDuoWebTokenProvider = Substitute.For<IOrganizationDuoWebTokenProvider>();
         _duoWebV4SDKService = Substitute.For<ITemporaryDuoWebV4SDKService>();
         _organizationRepository = Substitute.For<IOrganizationRepository>();
@@ -76,10 +74,9 @@ public class BaseRequestValidatorTests
 
         _sut = new BaseRequestValidatorTestWrapper(
             _userManager,
-            _deviceRepository,
-            _deviceService,
             _userService,
             _eventService,
+            _deviceValidator,
             _organizationDuoWebTokenProvider,
             _duoWebV4SDKService,
             _organizationRepository,
@@ -228,7 +225,8 @@ public class BaseRequestValidatorTests
     public async Task ValidateAsync_ClientCredentialsGrantType_ShouldSucceed(
         [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest,
         CustomValidatorRequestContext requestContext,
-        GrantValidationResult grantResult)
+        GrantValidationResult grantResult,
+        Device device)
     {
         // Arrange
         var context = CreateContext(tokenRequest, requestContext, grantResult);
@@ -240,18 +238,13 @@ public class BaseRequestValidatorTests
         _globalSettings.DisableEmailNewDevice = false;
 
         context.ValidatedTokenRequest.GrantType = "client_credentials"; // This || AuthCode will allow process to continue to get device 
-        context.ValidatedTokenRequest.Raw["DeviceIdentifier"] = "DeviceIdentifier";
-        context.ValidatedTokenRequest.Raw["DeviceType"] = "Android"; // This needs to be an actual Type
-        context.ValidatedTokenRequest.Raw["DeviceName"] = "DeviceName";
-        context.ValidatedTokenRequest.Raw["DevicePushToken"] = "DevicePushToken";
 
+        _deviceValidator.SaveDeviceAsync(Arg.Any<User>(), Arg.Any<ValidatedTokenRequest>())
+                         .Returns(device);
         // Act
         await _sut.ValidateAsync(context);
 
         // Assert
-        await _mailService.Received(1).SendNewDeviceLoggedInEmail(
-            context.CustomValidatorRequestContext.User.Email, "Android", Arg.Any<DateTime>(), Arg.Any<string>()
-        );
         Assert.False(context.GrantResult.IsError);
     }
 
@@ -262,7 +255,8 @@ public class BaseRequestValidatorTests
     public async Task ValidateAsync_ClientCredentialsGrantType_ExistingDevice_ShouldSucceed(
         [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest,
         CustomValidatorRequestContext requestContext,
-        GrantValidationResult grantResult)
+        GrantValidationResult grantResult,
+        Device device)
     {
         // Arrange
         var context = CreateContext(tokenRequest, requestContext, grantResult);
@@ -274,13 +268,9 @@ public class BaseRequestValidatorTests
         _globalSettings.DisableEmailNewDevice = false;
 
         context.ValidatedTokenRequest.GrantType = "client_credentials"; // This || AuthCode will allow process to continue to get device 
-        context.ValidatedTokenRequest.Raw["DeviceIdentifier"] = "DeviceIdentifier";
-        context.ValidatedTokenRequest.Raw["DeviceType"] = "Android"; // This needs to be an actual Type
-        context.ValidatedTokenRequest.Raw["DeviceName"] = "DeviceName";
-        context.ValidatedTokenRequest.Raw["DevicePushToken"] = "DevicePushToken";
 
-        _deviceRepository.GetByIdentifierAsync("DeviceIdentifier", Arg.Any<Guid>())
-                         .Returns(new Device() { Identifier = "DeviceIdentifier" });
+        _deviceValidator.SaveDeviceAsync(Arg.Any<User>(), Arg.Any<ValidatedTokenRequest>())
+                         .Returns(device);
         // Act
         await _sut.ValidateAsync(context);
 

test/Identity.Test/IdentityServer/DeviceValidatorTests.cs

@@ -0,0 +1,247 @@
+using Bit.Core.Context;
+using Bit.Core.Entities;
+using Bit.Core.Enums;
+using Bit.Core.Repositories;
+using Bit.Core.Services;
+using Bit.Core.Settings;
+using Bit.Identity.IdentityServer;
+using Bit.Test.Common.AutoFixture.Attributes;
+using Duende.IdentityServer.Validation;
+using NSubstitute;
+using Xunit;
+using AuthFixtures = Bit.Identity.Test.AutoFixture;
+
+namespace Bit.Identity.Test.IdentityServer;
+
+public class DeviceValidatorTests
+{
+    private readonly IDeviceService _deviceService;
+    private readonly IDeviceRepository _deviceRepository;
+    private readonly GlobalSettings _globalSettings;
+    private readonly IMailService _mailService;
+    private readonly ICurrentContext _currentContext;
+    private readonly DeviceValidator _sut;
+
+    public DeviceValidatorTests()
+    {
+        _deviceService = Substitute.For<IDeviceService>();
+        _deviceRepository = Substitute.For<IDeviceRepository>();
+        _globalSettings = new GlobalSettings();
+        _mailService = Substitute.For<IMailService>();
+        _currentContext = Substitute.For<ICurrentContext>();
+        _sut = new DeviceValidator(
+            _deviceService,
+            _deviceRepository,
+            _globalSettings,
+            _mailService,
+            _currentContext);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void SaveDeviceAsync_DeviceNull_ShouldReturnNull(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request,
+        User user)
+    {
+        // Arrange
+        request.Raw["DeviceIdentifier"] = null;
+
+        // Act
+        var device = await _sut.SaveDeviceAsync(user, request);
+
+        // Assert
+        Assert.Null(device);
+        await _mailService.DidNotReceive().SendNewDeviceLoggedInEmail(
+            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<DateTime>(), Arg.Any<string>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void SaveDeviceAsync_UserIsNull_ShouldReturnNull(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request)
+    {
+        // Arrange
+        request = AddValidDeviceToRequest(request);
+
+        // Act
+        var device = await _sut.SaveDeviceAsync(null, request);
+
+        // Assert
+        Assert.Null(device);
+        await _mailService.DidNotReceive().SendNewDeviceLoggedInEmail(
+            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<DateTime>(), Arg.Any<string>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void SaveDeviceAsync_ExistingUser_NewDevice_ReturnsDevice_SendsEmail(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request,
+        User user)
+    {
+        // Arrange
+        request = AddValidDeviceToRequest(request);
+
+        user.CreationDate = DateTime.UtcNow - TimeSpan.FromMinutes(11);
+        _globalSettings.DisableEmailNewDevice = false;
+
+        // Act
+        var device = await _sut.SaveDeviceAsync(user, request);
+
+        // Assert
+        Assert.NotNull(device);
+        Assert.Equal(user.Id, device.UserId);
+        Assert.Equal("DeviceIdentifier", device.Identifier);
+        Assert.Equal(DeviceType.Android, device.Type);
+        await _mailService.Received(1).SendNewDeviceLoggedInEmail(
+            user.Email, "Android", Arg.Any<DateTime>(), Arg.Any<string>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void SaveDeviceAsync_ExistingUser_NewDevice_ReturnsDevice_SendEmailFalse(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request,
+        User user)
+    {
+        // Arrange
+        request = AddValidDeviceToRequest(request);
+
+        user.CreationDate = DateTime.UtcNow - TimeSpan.FromMinutes(11);
+        _globalSettings.DisableEmailNewDevice = true;
+
+        // Act
+        var device = await _sut.SaveDeviceAsync(user, request);
+
+        // Assert
+        Assert.NotNull(device);
+        Assert.Equal(user.Id, device.UserId);
+        Assert.Equal("DeviceIdentifier", device.Identifier);
+        Assert.Equal(DeviceType.Android, device.Type);
+        await _mailService.DidNotReceive().SendNewDeviceLoggedInEmail(
+            user.Email, "Android", Arg.Any<DateTime>(), Arg.Any<string>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void SaveDeviceAsync_DeviceIsKnown_ShouldReturnDevice(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request,
+        User user,
+        Device device)
+    {
+        // Arrange
+        request = AddValidDeviceToRequest(request);
+
+        device.UserId = user.Id;
+        device.Identifier = "DeviceIdentifier";
+        device.Type = DeviceType.Android;
+        device.Name = "DeviceName";
+        device.PushToken = "DevicePushToken";
+        _deviceRepository.GetByIdentifierAsync(device.Identifier, user.Id).Returns(device);
+
+        // Act
+        var resultDevice = await _sut.SaveDeviceAsync(user, request);
+
+        // Assert
+        Assert.Equal(device, resultDevice);
+        await _mailService.DidNotReceive().SendNewDeviceLoggedInEmail(
+            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<DateTime>(), Arg.Any<string>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void SaveDeviceAsync_NewUser_DeviceUnknown_ShouldSaveDevice_NoEmail(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request,
+        User user)
+    {
+        // Arrange
+        request = AddValidDeviceToRequest(request);
+        user.CreationDate = DateTime.UtcNow;
+        _deviceRepository.GetByIdentifierAsync(Arg.Any<string>(), Arg.Any<Guid>()).Returns(null as Device);
+
+        // Act
+        var device = await _sut.SaveDeviceAsync(user, request);
+
+        // Assert
+        Assert.NotNull(device);
+        Assert.Equal(user.Id, device.UserId);
+        Assert.Equal("DeviceIdentifier", device.Identifier);
+        Assert.Equal(DeviceType.Android, device.Type);
+        await _deviceService.Received(1).SaveAsync(device);
+        await _mailService.DidNotReceive().SendNewDeviceLoggedInEmail(
+            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<DateTime>(), Arg.Any<string>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void KnownDeviceAsync_UserNull_ReturnsFalse(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request)
+    {
+        // Arrange
+        request = AddValidDeviceToRequest(request);
+
+        // Act
+        var result = await _sut.KnownDeviceAsync(null, request);
+
+        // Assert
+        Assert.False(result);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void KnownDeviceAsync_DeviceNull_ReturnsFalse(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request,
+        User user)
+    {
+        // Arrange
+        // Device raw data is null which will cause the device to be null
+
+        // Act
+        var result = await _sut.KnownDeviceAsync(user, request);
+
+        // Assert
+        Assert.False(result);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void KnownDeviceAsync_DeviceNotInDatabase_ReturnsFalse(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request,
+        User user)
+    {
+        // Arrange
+        request = AddValidDeviceToRequest(request);
+        _deviceRepository.GetByIdentifierAsync(Arg.Any<string>(), Arg.Any<Guid>())
+                         .Returns(null as Device);
+        // Act
+        var result = await _sut.KnownDeviceAsync(user, request);
+
+        // Assert
+        Assert.False(result);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void KnownDeviceAsync_UserAndDeviceValid_ReturnsTrue(
+        [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request,
+        User user,
+        Device device)
+    {
+        // Arrange
+        request = AddValidDeviceToRequest(request);
+        _deviceRepository.GetByIdentifierAsync(Arg.Any<string>(), Arg.Any<Guid>())
+                         .Returns(device);
+        // Act
+        var result = await _sut.KnownDeviceAsync(user, request);
+
+        // Assert
+        Assert.True(result);
+    }
+
+    private ValidatedTokenRequest AddValidDeviceToRequest(ValidatedTokenRequest request)
+    {
+        request.Raw["DeviceIdentifier"] = "DeviceIdentifier";
+        request.Raw["DeviceType"] = "Android";
+        request.Raw["DeviceName"] = "DeviceName";
+        request.Raw["DevicePushToken"] = "DevicePushToken";
+        return request;
+    }
+}

test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs

@@ -51,10 +51,9 @@ IBaseRequestValidatorTestWrapper
     public bool isValid { get; set; }
     public BaseRequestValidatorTestWrapper(
         UserManager<User> userManager,
-        IDeviceRepository deviceRepository,
-        IDeviceService deviceService,
         IUserService userService,
         IEventService eventService,
+        IDeviceValidator deviceValidator,
         IOrganizationDuoWebTokenProvider organizationDuoWebTokenProvider,
         ITemporaryDuoWebV4SDKService duoWebV4SDKService,
         IOrganizationRepository organizationRepository,
@@ -72,10 +71,9 @@ IBaseRequestValidatorTestWrapper
         IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder) :
         base(
             userManager,
-            deviceRepository,
-            deviceService,
             userService,
             eventService,
+            deviceValidator,
             organizationDuoWebTokenProvider,
             duoWebV4SDKService,
             organizationRepository,

test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs

@@ -145,7 +145,10 @@ public abstract class WebApplicationFactoryBase<T> : WebApplicationFactory<T>
                 // Email Verification
                 { "globalSettings:enableEmailVerification", "true" },
                 { "globalSettings:disableUserRegistration", "false" },
-                { "globalSettings:launchDarkly:flagValues:email-verification", "true" }
+                { "globalSettings:launchDarkly:flagValues:email-verification", "true" },
+
+                // New Device Verification
+                { "globalSettings:disableEmailNewDevice", "false" },
             });
         });