1
0
mirror of https://github.com/bitwarden/server.git synced 2025-05-20 02:54:32 -05:00

feat(NewDeviceVerification) : (#5153)

feat(NewDeviceVerification) :
Added constat for the cache key in Bit.Core because the cache key format needs to be shared between the Identity Server and the MVC Admin project.
Updated DeviceValidator class to handle checking cache for user information to allow pass through.
Updated and Added tests to handle new flow.
This commit is contained in:
Ike 2024-12-17 08:59:39 -08:00 committed by GitHub
parent b75c63c2c6
commit 2e8f2df942
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 55 additions and 3 deletions

View File

@ -57,7 +57,7 @@ public static class AuthConstants
public static readonly RangeConstant ARGON2_ITERATIONS = new(2, 10, 3);
public static readonly RangeConstant ARGON2_MEMORY = new(15, 1024, 64);
public static readonly RangeConstant ARGON2_PARALLELISM = new(1, 16, 4);
public static readonly string NewDeviceVerificationExceptionCacheKeyFormat = "NewDeviceVerificationException_{0}";
}
public class RangeConstant

View File

@ -10,6 +10,7 @@ using Bit.Core.Services;
using Bit.Core.Settings;
using Bit.Identity.IdentityServer.Enums;
using Duende.IdentityServer.Validation;
using Microsoft.Extensions.Caching.Distributed;
namespace Bit.Identity.IdentityServer.RequestValidators;
@ -20,6 +21,8 @@ public class DeviceValidator(
IMailService mailService,
ICurrentContext currentContext,
IUserService userService,
IDistributedCache distributedCache,
ILogger<DeviceValidator> logger,
IFeatureService featureService) : IDeviceValidator
{
private readonly IDeviceService _deviceService = deviceService;
@ -28,6 +31,8 @@ public class DeviceValidator(
private readonly IMailService _mailService = mailService;
private readonly ICurrentContext _currentContext = currentContext;
private readonly IUserService _userService = userService;
private readonly IDistributedCache distributedCache = distributedCache;
private readonly ILogger<DeviceValidator> _logger = logger;
private readonly IFeatureService _featureService = featureService;
public async Task<bool> ValidateRequestDeviceAsync(ValidatedTokenRequest request, CustomValidatorRequestContext context)
@ -67,7 +72,6 @@ public class DeviceValidator(
!context.SsoRequired &&
_globalSettings.EnableNewDeviceVerification)
{
// We only want to return early if the device is invalid or there is an error
var validationResult = await HandleNewDeviceVerificationAsync(context.User, request);
if (validationResult != DeviceValidationResultType.Success)
{
@ -121,6 +125,18 @@ public class DeviceValidator(
return DeviceValidationResultType.InvalidUser;
}
// CS exception flow
// Check cache for user information
var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, user.Id.ToString());
var cacheValue = await distributedCache.GetAsync(cacheKey);
if (cacheValue != null)
{
// if found in cache return success result and remove from cache
await distributedCache.RemoveAsync(cacheKey);
_logger.LogInformation("New device verification exception for user {UserId} found in cache", user.Id);
return DeviceValidationResultType.Success;
}
// parse request for NewDeviceOtp to validate
var newDeviceOtp = request.Raw["NewDeviceOtp"]?.ToString();
// we only check null here since an empty OTP will be considered an incorrect OTP

View File

@ -10,6 +10,8 @@ using Bit.Identity.IdentityServer;
using Bit.Identity.IdentityServer.RequestValidators;
using Bit.Test.Common.AutoFixture.Attributes;
using Duende.IdentityServer.Validation;
using Microsoft.Extensions.Caching.Distributed;
using Microsoft.Extensions.Logging;
using NSubstitute;
using Xunit;
using AuthFixtures = Bit.Identity.Test.AutoFixture;
@ -24,6 +26,8 @@ public class DeviceValidatorTests
private readonly IMailService _mailService;
private readonly ICurrentContext _currentContext;
private readonly IUserService _userService;
private readonly IDistributedCache _distributedCache;
private readonly Logger<DeviceValidator> _logger;
private readonly IFeatureService _featureService;
private readonly DeviceValidator _sut;
@ -35,6 +39,8 @@ public class DeviceValidatorTests
_mailService = Substitute.For<IMailService>();
_currentContext = Substitute.For<ICurrentContext>();
_userService = Substitute.For<IUserService>();
_distributedCache = Substitute.For<IDistributedCache>();
_logger = new Logger<DeviceValidator>(Substitute.For<ILoggerFactory>());
_featureService = Substitute.For<IFeatureService>();
_sut = new DeviceValidator(
_deviceService,
@ -43,6 +49,8 @@ public class DeviceValidatorTests
_mailService,
_currentContext,
_userService,
_distributedCache,
_logger,
_featureService);
}
@ -51,7 +59,7 @@ public class DeviceValidatorTests
Device device)
{
// Arrange
// AutoData arrages
// AutoData arranges
// Act
var result = await _sut.GetKnownDeviceAsync(null, device);
@ -421,6 +429,30 @@ public class DeviceValidatorTests
Assert.Equal(expectedErrorMessage, actualResponse.Message);
}
[Theory, BitAutoData]
public async void HandleNewDeviceVerificationAsync_UserHasCacheValue_ReturnsSuccess(
CustomValidatorRequestContext context,
[AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request)
{
// Arrange
ArrangeForHandleNewDeviceVerificationTest(context, request);
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true);
_globalSettings.EnableNewDeviceVerification = true;
_distributedCache.GetAsync(Arg.Any<string>()).Returns([1]);
// Act
var result = await _sut.ValidateRequestDeviceAsync(request, context);
// Assert
await _userService.Received(0).SendOTPAsync(context.User);
await _deviceService.Received(1).SaveAsync(Arg.Any<Device>());
Assert.True(result);
Assert.False(context.CustomResponse.ContainsKey("ErrorModel"));
Assert.Equal(context.User.Id, context.Device.UserId);
Assert.NotNull(context.Device);
}
[Theory, BitAutoData]
public async void HandleNewDeviceVerificationAsync_NewDeviceOtpValid_ReturnsSuccess(
CustomValidatorRequestContext context,
@ -430,6 +462,7 @@ public class DeviceValidatorTests
ArrangeForHandleNewDeviceVerificationTest(context, request);
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true);
_globalSettings.EnableNewDeviceVerification = true;
_distributedCache.GetAsync(Arg.Any<string>()).Returns(null as byte[]);
var newDeviceOtp = "123456";
request.Raw.Add("NewDeviceOtp", newDeviceOtp);
@ -461,6 +494,7 @@ public class DeviceValidatorTests
ArrangeForHandleNewDeviceVerificationTest(context, request);
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true);
_globalSettings.EnableNewDeviceVerification = true;
_distributedCache.GetAsync(Arg.Any<string>()).Returns(null as byte[]);
request.Raw.Add("NewDeviceOtp", newDeviceOtp);
@ -489,6 +523,7 @@ public class DeviceValidatorTests
ArrangeForHandleNewDeviceVerificationTest(context, request);
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true);
_globalSettings.EnableNewDeviceVerification = true;
_distributedCache.GetAsync(Arg.Any<string>()).Returns([1]);
_deviceRepository.GetManyByUserIdAsync(context.User.Id).Returns([]);
// Act
@ -515,6 +550,7 @@ public class DeviceValidatorTests
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true);
_globalSettings.EnableNewDeviceVerification = true;
_deviceRepository.GetManyByUserIdAsync(context.User.Id).Returns([new Device()]);
_distributedCache.GetAsync(Arg.Any<string>()).Returns(null as byte[]);
// Act
var result = await _sut.ValidateRequestDeviceAsync(request, context);