1
0
mirror of https://github.com/bitwarden/server.git synced 2025-02-15 01:41:40 +01:00

[PM-15614] Allow Users to opt out of new device verification (#5176)

feat(NewDeviceVerification) : 
* Created database migration scripts for VerifyDevices column in [dbo].[User].
* Updated DeviceValidator to check if user has opted out of device verification.
* Added endpoint to AccountsController.cs to allow editing of new User.VerifyDevices property.
* Added tests for new methods and endpoint.
* Updating queries to track [dbo].[User].[VerifyDevices].
* Updated DeviceValidator to set `User.EmailVerified` property during the New Device Verification flow.
This commit is contained in:
Ike 2025-01-08 07:31:24 -08:00 committed by GitHub
parent 481a766cd2
commit a84ef0724c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
21 changed files with 9459 additions and 9 deletions

View File

@ -969,11 +969,28 @@ public class AccountsController : Controller
[RequireFeature(FeatureFlagKeys.NewDeviceVerification)]
[AllowAnonymous]
[HttpPost("resend-new-device-otp")]
public async Task ResendNewDeviceOtpAsync([FromBody] UnauthenticatedSecretVerificatioRequestModel request)
public async Task ResendNewDeviceOtpAsync([FromBody] UnauthenticatedSecretVerificationRequestModel request)
{
await _userService.ResendNewDeviceVerificationEmail(request.Email, request.Secret);
}
[RequireFeature(FeatureFlagKeys.NewDeviceVerification)]
[HttpPost("verify-devices")]
[HttpPut("verify-devices")]
public async Task SetUserVerifyDevicesAsync([FromBody] SetVerifyDevicesRequestModel request)
{
var user = await _userService.GetUserByPrincipalAsync(User) ?? throw new UnauthorizedAccessException();
if (!await _userService.VerifySecretAsync(user, request.Secret))
{
await Task.Delay(2000);
throw new BadRequestException(string.Empty, "User verification failed.");
}
user.VerifyDevices = request.VerifyDevices;
await _userService.SaveUserAsync(user);
}
private async Task<IEnumerable<Guid>> GetOrganizationIdsManagingUserAsync(Guid userId)
{
var organizationManagingUser = await _userService.GetOrganizationsManagingUserAsync(userId);

View File

@ -0,0 +1,9 @@
using System.ComponentModel.DataAnnotations;
namespace Bit.Api.Auth.Models.Request.Accounts;
public class SetVerifyDevicesRequestModel : SecretVerificationRequestModel
{
[Required]
public bool VerifyDevices { get; set; }
}

View File

@ -3,7 +3,7 @@ using Bit.Core.Utilities;
namespace Bit.Api.Auth.Models.Request.Accounts;
public class UnauthenticatedSecretVerificatioRequestModel : SecretVerificationRequestModel
public class UnauthenticatedSecretVerificationRequestModel : SecretVerificationRequestModel
{
[Required]
[StrictEmailAddress]

View File

@ -72,6 +72,7 @@ public class User : ITableObject<Guid>, IStorableSubscriber, IRevisable, ITwoFac
public DateTime? LastKdfChangeDate { get; set; }
public DateTime? LastKeyRotationDate { get; set; }
public DateTime? LastEmailChangeDate { get; set; }
public bool VerifyDevices { get; set; } = true;
public void SetNewId()
{

View File

@ -115,7 +115,7 @@ public class DeviceValidator(
/// </summary>
/// <param name="user">user attempting to authenticate</param>
/// <param name="ValidatedRequest">The Request is used to check for the NewDeviceOtp and for the raw device data</param>
/// <returns>returns deviceValtaionResultType</returns>
/// <returns>returns deviceValidationResultType</returns>
private async Task<DeviceValidationResultType> HandleNewDeviceVerificationAsync(User user, ValidatedRequest request)
{
// currently unreachable due to backward compatibility
@ -125,6 +125,12 @@ public class DeviceValidator(
return DeviceValidationResultType.InvalidUser;
}
// Has the User opted out of new device verification
if (!user.VerifyDevices)
{
return DeviceValidationResultType.Success;
}
// CS exception flow
// Check cache for user information
var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, user.Id.ToString());
@ -146,6 +152,12 @@ public class DeviceValidator(
var otpValid = await _userService.VerifyOTPAsync(user, newDeviceOtp);
if (otpValid)
{
// In order to get here they would have to have access to their email so we verify it if it's not already
if (!user.EmailVerified)
{
user.EmailVerified = true;
await _userService.SaveUserAsync(user);
}
return DeviceValidationResultType.Success;
}
return DeviceValidationResultType.InvalidNewDeviceOtp;

View File

@ -40,7 +40,8 @@
@LastPasswordChangeDate DATETIME2(7) = NULL,
@LastKdfChangeDate DATETIME2(7) = NULL,
@LastKeyRotationDate DATETIME2(7) = NULL,
@LastEmailChangeDate DATETIME2(7) = NULL
@LastEmailChangeDate DATETIME2(7) = NULL,
@VerifyDevices BIT = 1
AS
BEGIN
SET NOCOUNT ON
@ -88,7 +89,8 @@ BEGIN
[LastPasswordChangeDate],
[LastKdfChangeDate],
[LastKeyRotationDate],
[LastEmailChangeDate]
[LastEmailChangeDate],
[VerifyDevices]
)
VALUES
(
@ -133,6 +135,7 @@ BEGIN
@LastPasswordChangeDate,
@LastKdfChangeDate,
@LastKeyRotationDate,
@LastEmailChangeDate
@LastEmailChangeDate,
@VerifyDevices
)
END

View File

@ -40,7 +40,8 @@
@LastPasswordChangeDate DATETIME2(7) = NULL,
@LastKdfChangeDate DATETIME2(7) = NULL,
@LastKeyRotationDate DATETIME2(7) = NULL,
@LastEmailChangeDate DATETIME2(7) = NULL
@LastEmailChangeDate DATETIME2(7) = NULL,
@VerifyDevices BIT = 1
AS
BEGIN
SET NOCOUNT ON
@ -88,7 +89,8 @@ BEGIN
[LastPasswordChangeDate] = @LastPasswordChangeDate,
[LastKdfChangeDate] = @LastKdfChangeDate,
[LastKeyRotationDate] = @LastKeyRotationDate,
[LastEmailChangeDate] = @LastEmailChangeDate
[LastEmailChangeDate] = @LastEmailChangeDate,
[VerifyDevices] = @VerifyDevices
WHERE
[Id] = @Id
END

View File

@ -36,11 +36,12 @@
[UsesKeyConnector] BIT NOT NULL,
[FailedLoginCount] INT CONSTRAINT [D_User_FailedLoginCount] DEFAULT ((0)) NOT NULL,
[LastFailedLoginDate] DATETIME2 (7) NULL,
[AvatarColor] VARCHAR(7) NULL,
[AvatarColor] VARCHAR(7) NULL,
[LastPasswordChangeDate] DATETIME2 (7) NULL,
[LastKdfChangeDate] DATETIME2 (7) NULL,
[LastKeyRotationDate] DATETIME2 (7) NULL,
[LastEmailChangeDate] DATETIME2 (7) NULL,
[VerifyDevices] BIT DEFAULT ((1)) NOT NULL,
CONSTRAINT [PK_User] PRIMARY KEY CLUSTERED ([Id] ASC)
);

View File

@ -563,6 +563,49 @@ public class AccountsControllerTests : IDisposable
await _userService.Received(1).DeleteAsync(user);
}
[Theory]
[BitAutoData]
public async Task SetVerifyDevices_WhenUserDoesNotExist_ShouldThrowUnauthorizedAccessException(
SetVerifyDevicesRequestModel model)
{
// Arrange
_userService.GetUserByPrincipalAsync(Arg.Any<ClaimsPrincipal>()).Returns(Task.FromResult((User)null));
// Act & Assert
await Assert.ThrowsAsync<UnauthorizedAccessException>(() => _sut.SetUserVerifyDevicesAsync(model));
}
[Theory]
[BitAutoData]
public async Task SetVerifyDevices_WhenInvalidSecret_ShouldFail(
User user, SetVerifyDevicesRequestModel model)
{
// Arrange
_userService.GetUserByPrincipalAsync(Arg.Any<ClaimsPrincipal>()).Returns(Task.FromResult((user)));
_userService.VerifySecretAsync(user, Arg.Any<string>()).Returns(Task.FromResult(false));
// Act & Assert
await Assert.ThrowsAsync<BadRequestException>(() => _sut.SetUserVerifyDevicesAsync(model));
}
[Theory]
[BitAutoData]
public async Task SetVerifyDevices_WhenRequestValid_ShouldSucceed(
User user, SetVerifyDevicesRequestModel model)
{
// Arrange
user.VerifyDevices = false;
model.VerifyDevices = true;
_userService.GetUserByPrincipalAsync(Arg.Any<ClaimsPrincipal>()).Returns(Task.FromResult((user)));
_userService.VerifySecretAsync(user, Arg.Any<string>()).Returns(Task.FromResult(true));
// Act
await _sut.SetUserVerifyDevicesAsync(model);
await _userService.Received(1).SaveUserAsync(user);
Assert.Equal(model.VerifyDevices, user.VerifyDevices);
}
// Below are helper functions that currently belong to this
// test class, but ultimately may need to be split out into
// something greater in order to share common test steps with

View File

@ -36,6 +36,7 @@ public class CloudICloudOrganizationSignUpCommandTests
signup.PremiumAccessAddon = false;
signup.UseSecretsManager = false;
signup.IsFromSecretsManagerTrial = false;
signup.IsFromProvider = false;
var result = await sutProvider.Sut.SignUpOrganizationAsync(signup);
@ -85,6 +86,7 @@ public class CloudICloudOrganizationSignUpCommandTests
signup.PaymentMethodType = PaymentMethodType.Card;
signup.PremiumAccessAddon = false;
signup.UseSecretsManager = false;
signup.IsFromProvider = false;
// Extract orgUserId when created
Guid? orgUserId = null;
@ -128,6 +130,8 @@ public class CloudICloudOrganizationSignUpCommandTests
signup.PaymentMethodType = PaymentMethodType.Card;
signup.PremiumAccessAddon = false;
signup.IsFromSecretsManagerTrial = false;
signup.IsFromProvider = false;
var result = await sutProvider.Sut.SignUpOrganizationAsync(signup);
@ -196,6 +200,7 @@ public class CloudICloudOrganizationSignUpCommandTests
signup.PremiumAccessAddon = false;
signup.AdditionalServiceAccounts = 10;
signup.AdditionalStorageGb = 0;
signup.IsFromProvider = false;
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SignUpOrganizationAsync(signup));
@ -213,6 +218,7 @@ public class CloudICloudOrganizationSignUpCommandTests
signup.PaymentMethodType = PaymentMethodType.Card;
signup.PremiumAccessAddon = false;
signup.AdditionalServiceAccounts = 10;
signup.IsFromProvider = false;
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SignUpOrganizationAsync(signup));
@ -230,6 +236,7 @@ public class CloudICloudOrganizationSignUpCommandTests
signup.PaymentMethodType = PaymentMethodType.Card;
signup.PremiumAccessAddon = false;
signup.AdditionalServiceAccounts = -10;
signup.IsFromProvider = false;
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SignUpOrganizationAsync(signup));

View File

@ -429,6 +429,30 @@ public class DeviceValidatorTests
Assert.Equal(expectedErrorMessage, actualResponse.Message);
}
[Theory, BitAutoData]
public async void HandleNewDeviceVerificationAsync_VerifyDevicesFalse_ReturnsSuccess(
CustomValidatorRequestContext context,
[AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request)
{
// Arrange
ArrangeForHandleNewDeviceVerificationTest(context, request);
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true);
_globalSettings.EnableNewDeviceVerification = true;
context.User.VerifyDevices = false;
// 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_UserHasCacheValue_ReturnsSuccess(
CustomValidatorRequestContext context,

View File

@ -0,0 +1,252 @@
IF COL_LENGTH('[dbo].[User]', 'VerifyDevices') IS NULL
BEGIN
ALTER TABLE
[dbo].[User]
ADD
[VerifyDevices] BIT NOT NULL DEFAULT 1
END
GO
EXECUTE sp_refreshview 'dbo.UserView'
GO
CREATE OR ALTER PROCEDURE [dbo].[User_Create]
@Id UNIQUEIDENTIFIER OUTPUT,
@Name NVARCHAR(50),
@Email NVARCHAR(256),
@EmailVerified BIT,
@MasterPassword NVARCHAR(300),
@MasterPasswordHint NVARCHAR(50),
@Culture NVARCHAR(10),
@SecurityStamp NVARCHAR(50),
@TwoFactorProviders NVARCHAR(MAX),
@TwoFactorRecoveryCode NVARCHAR(32),
@EquivalentDomains NVARCHAR(MAX),
@ExcludedGlobalEquivalentDomains NVARCHAR(MAX),
@AccountRevisionDate DATETIME2(7),
@Key NVARCHAR(MAX),
@PublicKey NVARCHAR(MAX),
@PrivateKey NVARCHAR(MAX),
@Premium BIT,
@PremiumExpirationDate DATETIME2(7),
@RenewalReminderDate DATETIME2(7),
@Storage BIGINT,
@MaxStorageGb SMALLINT,
@Gateway TINYINT,
@GatewayCustomerId VARCHAR(50),
@GatewaySubscriptionId VARCHAR(50),
@ReferenceData VARCHAR(MAX),
@LicenseKey VARCHAR(100),
@Kdf TINYINT,
@KdfIterations INT,
@KdfMemory INT = NULL,
@KdfParallelism INT = NULL,
@CreationDate DATETIME2(7),
@RevisionDate DATETIME2(7),
@ApiKey VARCHAR(30),
@ForcePasswordReset BIT = 0,
@UsesKeyConnector BIT = 0,
@FailedLoginCount INT = 0,
@LastFailedLoginDate DATETIME2(7),
@AvatarColor VARCHAR(7) = NULL,
@LastPasswordChangeDate DATETIME2(7) = NULL,
@LastKdfChangeDate DATETIME2(7) = NULL,
@LastKeyRotationDate DATETIME2(7) = NULL,
@LastEmailChangeDate DATETIME2(7) = NULL,
@VerifyDevices BIT = 1
AS
BEGIN
SET NOCOUNT ON
INSERT INTO [dbo].[User]
(
[Id],
[Name],
[Email],
[EmailVerified],
[MasterPassword],
[MasterPasswordHint],
[Culture],
[SecurityStamp],
[TwoFactorProviders],
[TwoFactorRecoveryCode],
[EquivalentDomains],
[ExcludedGlobalEquivalentDomains],
[AccountRevisionDate],
[Key],
[PublicKey],
[PrivateKey],
[Premium],
[PremiumExpirationDate],
[RenewalReminderDate],
[Storage],
[MaxStorageGb],
[Gateway],
[GatewayCustomerId],
[GatewaySubscriptionId],
[ReferenceData],
[LicenseKey],
[Kdf],
[KdfIterations],
[CreationDate],
[RevisionDate],
[ApiKey],
[ForcePasswordReset],
[UsesKeyConnector],
[FailedLoginCount],
[LastFailedLoginDate],
[AvatarColor],
[KdfMemory],
[KdfParallelism],
[LastPasswordChangeDate],
[LastKdfChangeDate],
[LastKeyRotationDate],
[LastEmailChangeDate],
[VerifyDevices]
)
VALUES
(
@Id,
@Name,
@Email,
@EmailVerified,
@MasterPassword,
@MasterPasswordHint,
@Culture,
@SecurityStamp,
@TwoFactorProviders,
@TwoFactorRecoveryCode,
@EquivalentDomains,
@ExcludedGlobalEquivalentDomains,
@AccountRevisionDate,
@Key,
@PublicKey,
@PrivateKey,
@Premium,
@PremiumExpirationDate,
@RenewalReminderDate,
@Storage,
@MaxStorageGb,
@Gateway,
@GatewayCustomerId,
@GatewaySubscriptionId,
@ReferenceData,
@LicenseKey,
@Kdf,
@KdfIterations,
@CreationDate,
@RevisionDate,
@ApiKey,
@ForcePasswordReset,
@UsesKeyConnector,
@FailedLoginCount,
@LastFailedLoginDate,
@AvatarColor,
@KdfMemory,
@KdfParallelism,
@LastPasswordChangeDate,
@LastKdfChangeDate,
@LastKeyRotationDate,
@LastEmailChangeDate,
@VerifyDevices
)
END
GO
CREATE OR ALTER PROCEDURE [dbo].[User_Update]
@Id UNIQUEIDENTIFIER,
@Name NVARCHAR(50),
@Email NVARCHAR(256),
@EmailVerified BIT,
@MasterPassword NVARCHAR(300),
@MasterPasswordHint NVARCHAR(50),
@Culture NVARCHAR(10),
@SecurityStamp NVARCHAR(50),
@TwoFactorProviders NVARCHAR(MAX),
@TwoFactorRecoveryCode NVARCHAR(32),
@EquivalentDomains NVARCHAR(MAX),
@ExcludedGlobalEquivalentDomains NVARCHAR(MAX),
@AccountRevisionDate DATETIME2(7),
@Key NVARCHAR(MAX),
@PublicKey NVARCHAR(MAX),
@PrivateKey NVARCHAR(MAX),
@Premium BIT,
@PremiumExpirationDate DATETIME2(7),
@RenewalReminderDate DATETIME2(7),
@Storage BIGINT,
@MaxStorageGb SMALLINT,
@Gateway TINYINT,
@GatewayCustomerId VARCHAR(50),
@GatewaySubscriptionId VARCHAR(50),
@ReferenceData VARCHAR(MAX),
@LicenseKey VARCHAR(100),
@Kdf TINYINT,
@KdfIterations INT,
@KdfMemory INT = NULL,
@KdfParallelism INT = NULL,
@CreationDate DATETIME2(7),
@RevisionDate DATETIME2(7),
@ApiKey VARCHAR(30),
@ForcePasswordReset BIT = 0,
@UsesKeyConnector BIT = 0,
@FailedLoginCount INT,
@LastFailedLoginDate DATETIME2(7),
@AvatarColor VARCHAR(7),
@LastPasswordChangeDate DATETIME2(7) = NULL,
@LastKdfChangeDate DATETIME2(7) = NULL,
@LastKeyRotationDate DATETIME2(7) = NULL,
@LastEmailChangeDate DATETIME2(7) = NULL,
@VerifyDevices BIT = 1
AS
BEGIN
SET NOCOUNT ON
UPDATE
[dbo].[User]
SET
[Name] = @Name,
[Email] = @Email,
[EmailVerified] = @EmailVerified,
[MasterPassword] = @MasterPassword,
[MasterPasswordHint] = @MasterPasswordHint,
[Culture] = @Culture,
[SecurityStamp] = @SecurityStamp,
[TwoFactorProviders] = @TwoFactorProviders,
[TwoFactorRecoveryCode] = @TwoFactorRecoveryCode,
[EquivalentDomains] = @EquivalentDomains,
[ExcludedGlobalEquivalentDomains] = @ExcludedGlobalEquivalentDomains,
[AccountRevisionDate] = @AccountRevisionDate,
[Key] = @Key,
[PublicKey] = @PublicKey,
[PrivateKey] = @PrivateKey,
[Premium] = @Premium,
[PremiumExpirationDate] = @PremiumExpirationDate,
[RenewalReminderDate] = @RenewalReminderDate,
[Storage] = @Storage,
[MaxStorageGb] = @MaxStorageGb,
[Gateway] = @Gateway,
[GatewayCustomerId] = @GatewayCustomerId,
[GatewaySubscriptionId] = @GatewaySubscriptionId,
[ReferenceData] = @ReferenceData,
[LicenseKey] = @LicenseKey,
[Kdf] = @Kdf,
[KdfIterations] = @KdfIterations,
[KdfMemory] = @KdfMemory,
[KdfParallelism] = @KdfParallelism,
[CreationDate] = @CreationDate,
[RevisionDate] = @RevisionDate,
[ApiKey] = @ApiKey,
[ForcePasswordReset] = @ForcePasswordReset,
[UsesKeyConnector] = @UsesKeyConnector,
[FailedLoginCount] = @FailedLoginCount,
[LastFailedLoginDate] = @LastFailedLoginDate,
[AvatarColor] = @AvatarColor,
[LastPasswordChangeDate] = @LastPasswordChangeDate,
[LastKdfChangeDate] = @LastKdfChangeDate,
[LastKeyRotationDate] = @LastKeyRotationDate,
[LastEmailChangeDate] = @LastEmailChangeDate,
[VerifyDevices] = @VerifyDevices
WHERE
[Id] = @Id
END
GO

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,28 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.MySqlMigrations.Migrations;
/// <inheritdoc />
public partial class AlterUser_AddVerifyDevice : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AddColumn<bool>(
name: "VerifyDevices",
table: "User",
type: "tinyint(1)",
nullable: false,
defaultValue: true);
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "VerifyDevices",
table: "User");
}
}

View File

@ -1662,6 +1662,9 @@ namespace Bit.MySqlMigrations.Migrations
b.Property<bool>("UsesKeyConnector")
.HasColumnType("tinyint(1)");
b.Property<bool>("VerifyDevices")
.HasColumnType("tinyint(1)");
b.HasKey("Id");
b.HasIndex("Email")

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,28 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.PostgresMigrations.Migrations;
/// <inheritdoc />
public partial class AlterUser_AddVerifyDevice : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AddColumn<bool>(
name: "VerifyDevices",
table: "User",
type: "boolean",
nullable: false,
defaultValue: true);
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "VerifyDevices",
table: "User");
}
}

View File

@ -1668,6 +1668,9 @@ namespace Bit.PostgresMigrations.Migrations
b.Property<bool>("UsesKeyConnector")
.HasColumnType("boolean");
b.Property<bool>("VerifyDevices")
.HasColumnType("boolean");
b.HasKey("Id");
b.HasIndex("Email")

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,28 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.SqliteMigrations.Migrations;
/// <inheritdoc />
public partial class AlterUser_AddVerifyDevice : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AddColumn<bool>(
name: "VerifyDevices",
table: "User",
type: "INTEGER",
nullable: false,
defaultValue: true);
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "VerifyDevices",
table: "User");
}
}

View File

@ -1651,6 +1651,9 @@ namespace Bit.SqliteMigrations.Migrations
b.Property<bool>("UsesKeyConnector")
.HasColumnType("INTEGER");
b.Property<bool>("VerifyDevices")
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("Email")