1
0
mirror of https://github.com/bitwarden/server synced 2026-01-02 00:23:40 +00:00

fix(auth-validator): [PM-22975] Client Version Validator - Took in feedback from km. Removed IsV2User in favor of checking the security version on the user.

This commit is contained in:
Patrick Pimentel
2025-12-03 09:46:00 -05:00
parent c1bc10bf40
commit 753670d26f
10 changed files with 36 additions and 125 deletions

View File

@@ -51,7 +51,7 @@ public class User : ITableObject<Guid>, IStorableSubscriber, IRevisable, ITwoFac
public string? Key { get; set; }
/// <summary>
/// The raw public key, without a signature from the user's signature key.
/// </summary>
/// </summary>
public string? PublicKey { get; set; }
/// <summary>
/// User key wrapped private key.
@@ -211,6 +211,11 @@ public class User : ITableObject<Guid>, IStorableSubscriber, IRevisable, ITwoFac
return SecurityVersion ?? 1;
}
public bool IsSecurityVersionTwo()
{
return SecurityVersion == 2;
}
/// <summary>
/// Serializes the C# object to the User.TwoFactorProviders property in JSON format.
/// </summary>

View File

@@ -2,5 +2,5 @@
public static class Constants
{
public static readonly Version MinimumClientVersionForV2Encryption = new Version("2025.11.0");
public static readonly Version MinimumClientVersionForV2Encryption = new("2025.11.0");
}

View File

@@ -26,7 +26,6 @@ public static class KeyManagementServiceCollectionExtensions
private static void AddKeyManagementQueries(this IServiceCollection services)
{
services.AddScoped<IUserAccountKeysQuery, UserAccountKeysQuery>();
services.AddScoped<IIsV2EncryptionUserQuery, IsV2EncryptionUserQuery>();
services.AddScoped<IGetMinimumClientVersionForUserQuery, GetMinimumClientVersionForUserQuery>();
}
}

View File

@@ -3,21 +3,21 @@ using Bit.Core.KeyManagement.Queries.Interfaces;
namespace Bit.Core.KeyManagement.Queries;
public class GetMinimumClientVersionForUserQuery(IIsV2EncryptionUserQuery isV2EncryptionUserQuery)
public class GetMinimumClientVersionForUserQuery()
: IGetMinimumClientVersionForUserQuery
{
public async Task<Version?> Run(User? user)
public Task<Version?> Run(User? user)
{
if (user == null)
{
return null;
return Task.FromResult<Version?>(null);
}
if (await isV2EncryptionUserQuery.Run(user))
if (user.IsSecurityVersionTwo())
{
return Constants.MinimumClientVersionForV2Encryption;
return Task.FromResult(Constants.MinimumClientVersionForV2Encryption)!;
}
return null;
return Task.FromResult<Version?>(null);
}
}

View File

@@ -1,8 +0,0 @@
using Bit.Core.Entities;
namespace Bit.Core.KeyManagement.Queries.Interfaces;
public interface IIsV2EncryptionUserQuery
{
Task<bool> Run(User user);
}

View File

@@ -1,31 +0,0 @@
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.KeyManagement.Queries.Interfaces;
using Bit.Core.KeyManagement.Repositories;
using Bit.Core.KeyManagement.Utilities;
namespace Bit.Core.KeyManagement.Queries;
public class IsV2EncryptionUserQuery(IUserSignatureKeyPairRepository userSignatureKeyPairRepository)
: IIsV2EncryptionUserQuery
{
public async Task<bool> Run(User user)
{
ArgumentNullException.ThrowIfNull(user);
var hasSignatureKeyPair = await userSignatureKeyPairRepository.GetByUserIdAsync(user.Id) != null;
var isPrivateKeyEncryptionV2 =
!string.IsNullOrWhiteSpace(user.PrivateKey) &&
EncryptionParsing.GetEncryptionType(user.PrivateKey) == EncryptionType.XChaCha20Poly1305_B64;
return hasSignatureKeyPair switch
{
// Valid v2 user
true when isPrivateKeyEncryptionV2 => true,
// Valid v1 user
false when !isPrivateKeyEncryptionV2 => false,
_ => throw new InvalidOperationException(
"User is in an invalid state for key rotation. User has a signature key pair, but the private key is not in v2 format, or vice versa.")
};
}
}

View File

@@ -7,8 +7,10 @@ public static class EncryptionParsing
/// <summary>
/// Helper method to convert an encryption type string to an enum value.
/// </summary>
public static EncryptionType GetEncryptionType(string encString)
public static EncryptionType GetEncryptionType(string? encString)
{
ArgumentNullException.ThrowIfNull(encString);
var parts = encString.Split('.');
if (parts.Length == 1)
{

View File

@@ -11,6 +11,16 @@ public interface IClientVersionValidator
Task<bool> ValidateAsync(User user, CustomValidatorRequestContext requestContext);
}
/// <summary>
/// This validator will use the Client Version on a request, which currently maps
/// to the "Bitwarden-Client-Version" header, to determine if a user meets minimum
/// required client version for issuing tokens on an old client. This is done to
/// incentivize users getting on an updated client when their password encryption
/// method has already been updated. Currently this validator looks for the version
/// defined by MinimumClientVersionForV2Encryption.
///
/// If the header is omitted, then the validator returns that this request is valid.
/// </summary>
public class ClientVersionValidator(
ICurrentContext currentContext,
IGetMinimumClientVersionForUserQuery getMinimumClientVersionForUserQuery)

View File

@@ -1,32 +1,30 @@
using Bit.Core.Entities;
using Bit.Core.KeyManagement.Queries;
using Bit.Core.KeyManagement.Queries.Interfaces;
using Xunit;
namespace Bit.Core.Test.KeyManagement.Queries;
public class GetMinimumClientVersionForUserQueryTests
{
private class FakeIsV2Query : IIsV2EncryptionUserQuery
{
private readonly bool _isV2;
public FakeIsV2Query(bool isV2) { _isV2 = isV2; }
public Task<bool> Run(User user) => Task.FromResult(_isV2);
}
[Fact]
public async Task Run_ReturnsMinVersion_ForV2User()
{
var sut = new GetMinimumClientVersionForUserQuery(new FakeIsV2Query(true));
var version = await sut.Run(new User());
var sut = new GetMinimumClientVersionForUserQuery();
var version = await sut.Run(new User
{
SecurityVersion = 2
});
Assert.Equal(Core.KeyManagement.Constants.MinimumClientVersionForV2Encryption, version);
}
[Fact]
public async Task Run_ReturnsNull_ForV1User()
{
var sut = new GetMinimumClientVersionForUserQuery(new FakeIsV2Query(false));
var version = await sut.Run(new User());
var sut = new GetMinimumClientVersionForUserQuery();
var version = await sut.Run(new User
{
SecurityVersion = 1
});
Assert.Null(version);
}
}

View File

@@ -1,64 +0,0 @@
using Bit.Core.Entities;
using Bit.Core.KeyManagement.Entities;
using Bit.Core.KeyManagement.Enums;
using Bit.Core.KeyManagement.Models.Data;
using Bit.Core.KeyManagement.Queries;
using Bit.Core.KeyManagement.Repositories;
using Bit.Core.KeyManagement.UserKey;
using Bit.Test.Common.Constants;
using Xunit;
namespace Bit.Core.Test.KeyManagement.Queries;
public class IsV2EncryptionUserQueryTests
{
private class FakeUserSignatureKeyPairRepository : IUserSignatureKeyPairRepository
{
private readonly bool _hasKeys;
public FakeUserSignatureKeyPairRepository(bool hasKeys) { _hasKeys = hasKeys; }
public Task<SignatureKeyPairData?> GetByUserIdAsync(Guid userId)
=> Task.FromResult(_hasKeys ? new SignatureKeyPairData(SignatureAlgorithm.Ed25519, TestEncryptionConstants.V2WrappedSigningKey, TestEncryptionConstants.V2VerifyingKey) : null);
// Unused in tests
public Task<IEnumerable<UserSignatureKeyPair>> GetManyAsync(IEnumerable<Guid> ids) => throw new NotImplementedException();
public Task<UserSignatureKeyPair> GetByIdAsync(Guid id) => throw new NotImplementedException();
public Task<UserSignatureKeyPair> CreateAsync(UserSignatureKeyPair obj) => throw new NotImplementedException();
public Task ReplaceAsync(UserSignatureKeyPair obj) => throw new NotImplementedException();
public Task UpsertAsync(UserSignatureKeyPair obj) => throw new NotImplementedException();
public Task DeleteAsync(UserSignatureKeyPair obj) => throw new NotImplementedException();
public Task DeleteAsync(Guid id) => throw new NotImplementedException();
public UpdateEncryptedDataForKeyRotation UpdateForKeyRotation(Guid grantorId, SignatureKeyPairData signatureKeyPair) => throw new NotImplementedException();
public UpdateEncryptedDataForKeyRotation SetUserSignatureKeyPair(Guid userId, SignatureKeyPairData signatureKeyPair) => throw new NotImplementedException();
}
[Fact]
public async Task Run_ReturnsTrue_ForV2State()
{
var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V2PrivateKey };
var sut = new IsV2EncryptionUserQuery(new FakeUserSignatureKeyPairRepository(true));
var result = await sut.Run(user);
Assert.True(result);
}
[Fact]
public async Task Run_ReturnsFalse_ForV1State()
{
var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V1EncryptedBase64 };
var sut = new IsV2EncryptionUserQuery(new FakeUserSignatureKeyPairRepository(false));
var result = await sut.Run(user);
Assert.False(result);
}
[Fact]
public async Task Run_ThrowsForInvalidMixedState()
{
var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V2PrivateKey };
var sut = new IsV2EncryptionUserQuery(new FakeUserSignatureKeyPairRepository(false));
await Assert.ThrowsAsync<InvalidOperationException>(async () => await sut.Run(user));
}
}