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 - Updated with removal of cqrs approach in favor of static user checks. Also fixed tests

This commit is contained in:
Patrick Pimentel
2025-12-08 10:26:59 -05:00
parent d706796fc3
commit 226405609e
17 changed files with 138 additions and 160 deletions

View File

@@ -212,14 +212,31 @@ public class User : ITableObject<Guid>, IStorableSubscriber, IRevisable, ITwoFac
return SecurityVersion ?? 1;
}
public bool IsSetupForV2Encryption()
/// <summary>
/// Evaluates user state to determine if they are currently in a v2 encryption state.
/// </summary>
/// <returns>If the shape of their private key is v2 as well as has the proper security version then true, otherwise false</returns>
public bool HasV2Encryption()
{
return HasV2KeyShape() && IsSecurityVersionTwo();
}
private bool HasV2KeyShape()
{
return EncryptionParsing.GetEncryptionType(PrivateKey) == EncryptionType.XChaCha20Poly1305_B64;
if (string.IsNullOrEmpty(PrivateKey))
{
return false;
}
try
{
return EncryptionParsing.GetEncryptionType(PrivateKey) == EncryptionType.XChaCha20Poly1305_B64;
}
catch (ArgumentException)
{
// Invalid encryption string format - treat as not v2
return false;
}
}
/// <summary>

View File

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

View File

@@ -1,23 +0,0 @@
using Bit.Core.Entities;
using Bit.Core.KeyManagement.Queries.Interfaces;
namespace Bit.Core.KeyManagement.Queries;
public class GetMinimumClientVersionForUserQuery()
: IGetMinimumClientVersionForUserQuery
{
public Task<Version?> Run(User? user)
{
if (user == null)
{
return Task.FromResult<Version?>(null);
}
if (user.IsSetupForV2Encryption())
{
return Task.FromResult(Constants.MinimumClientVersionForV2Encryption)!;
}
return Task.FromResult<Version?>(null);
}
}

View File

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

View File

@@ -359,12 +359,11 @@ public abstract class BaseRequestValidator<T> where T : class
/// <summary>
/// Validates whether the client version is compatible for the user attempting to authenticate.
/// New authentications only; refresh/device grants are handled elsewhere.
/// </summary>
/// <returns>true if the scheme successfully passed validation, otherwise false.</returns>
private async Task<bool> ValidateClientVersionAsync(T context, CustomValidatorRequestContext validatorContext)
{
var ok = await _clientVersionValidator.ValidateAsync(validatorContext.User, validatorContext);
var ok = _clientVersionValidator.ValidateAsync(validatorContext.User, validatorContext);
if (ok)
{
return true;

View File

@@ -1,6 +1,6 @@
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.KeyManagement.Queries.Interfaces;
using Bit.Core.KeyManagement;
using Bit.Core.Models.Api;
using Duende.IdentityServer.Validation;
@@ -8,7 +8,7 @@ namespace Bit.Identity.IdentityServer.RequestValidators;
public interface IClientVersionValidator
{
Task<bool> ValidateAsync(User user, CustomValidatorRequestContext requestContext);
bool ValidateAsync(User user, CustomValidatorRequestContext requestContext);
}
/// <summary>
@@ -22,24 +22,34 @@ public interface IClientVersionValidator
/// If the header is omitted, then the validator returns that this request is valid.
/// </summary>
public class ClientVersionValidator(
ICurrentContext currentContext,
IGetMinimumClientVersionForUserQuery getMinimumClientVersionForUserQuery)
ICurrentContext currentContext)
: IClientVersionValidator
{
private const string _upgradeMessage = "Please update your app to continue using Bitwarden";
private const string _noUserMessage = "No user found while trying to validate client version";
public async Task<bool> ValidateAsync(User? user, CustomValidatorRequestContext requestContext)
public bool ValidateAsync(User? user, CustomValidatorRequestContext requestContext)
{
// Do this nullish check because the base request validator currently is not
// strict null checking. Once that gets fixed then we can see about making
// the user not nullish checked. If they are null then the validator should fail.
if (user == null)
{
requestContext.ValidationErrorResult = new ValidationResult
{
Error = "no_user",
ErrorDescription = _noUserMessage,
IsError = true
};
requestContext.CustomResponse = new Dictionary<string, object>
{
{ "ErrorModel", new ErrorResponseModel(_noUserMessage) }
};
return false;
}
Version? clientVersion = currentContext.ClientVersion;
Version? minVersion = await getMinimumClientVersionForUserQuery.Run(user);
Version? minVersion = user.HasV2Encryption() ? Constants.MinimumClientVersionForV2Encryption : null;
// Allow through if headers are missing.
// The minVersion should never be null because of where this validator is run. The user would

View File

@@ -68,9 +68,9 @@ public class ApiApplicationFactory : WebApplicationFactoryBase<Startup>
UserAsymmetricKeys = new KeysRequestModel()
{
PublicKey = TestEncryptionConstants.PublicKey,
EncryptedPrivateKey = TestEncryptionConstants.V1EncryptedBase64
EncryptedPrivateKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring
},
UserSymmetricKey = TestEncryptionConstants.V1EncryptedBase64,
UserSymmetricKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring,
});
return await _identityApplicationFactory.TokenFromPasswordAsync(email, masterPasswordHash);

View File

@@ -2,8 +2,8 @@
public static class TestEncryptionConstants
{
// V1-style encrypted strings (AES-CBC-HMAC formats) accepted by validators
public const string V1EncryptedBase64 = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==";
// Intended for use as a V1 encrypted string, accepted by validators
public const string AES256_CBC_HMAC_Encstring = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==";
// Public key test placeholder
public const string PublicKey = "pk_test";
@@ -15,5 +15,3 @@ public static class TestEncryptionConstants
public const string V2WrappedSigningKey = "7.cose_signing";
public const string V2VerifyingKey = "vk";
}

View File

@@ -1,72 +0,0 @@
using Bit.Core.Entities;
using Bit.Core.KeyManagement.Queries;
using Bit.Test.Common.Constants;
using Xunit;
namespace Bit.Core.Test.KeyManagement.Queries;
public class GetMinimumClientVersionForUserQueryTests
{
[Fact]
public async Task Run_ReturnsMinVersion_ForV2User()
{
var sut = new GetMinimumClientVersionForUserQuery();
var version = await sut.Run(new User
{
SecurityVersion = 2,
PrivateKey = TestEncryptionConstants.V2PrivateKey,
});
Assert.Equal(Core.KeyManagement.Constants.MinimumClientVersionForV2Encryption, version);
}
[Fact]
public async Task Run_ReturnsNull_ForV1User()
{
var sut = new GetMinimumClientVersionForUserQuery();
var version = await sut.Run(new User
{
SecurityVersion = 1,
PrivateKey = TestEncryptionConstants.V1EncryptedBase64,
});
Assert.Null(version);
}
[Fact]
public async Task Run_ReturnsNull_ForSecurityVersion1ButPrivateKeyV2User()
{
var sut = new GetMinimumClientVersionForUserQuery();
var version = await sut.Run(new User
{
SecurityVersion = 1,
PrivateKey = TestEncryptionConstants.V2PrivateKey,
});
Assert.Null(version);
}
[Fact]
public async Task Run_ReturnsNull_ForPrivateKeyV1ButSecurityVersion2User()
{
var sut = new GetMinimumClientVersionForUserQuery();
var version = await sut.Run(new User
{
SecurityVersion = 2,
PrivateKey = TestEncryptionConstants.V1EncryptedBase64,
});
Assert.Null(version);
}
[Fact]
public async Task Run_ReturnsNull_ForV1UserWithNull()
{
var sut = new GetMinimumClientVersionForUserQuery();
var version = await sut.Run(new User
{
SecurityVersion = null,
PrivateKey = TestEncryptionConstants.V2PrivateKey,
});
Assert.Null(version);
}
}

View File

@@ -60,9 +60,9 @@ public class EventsApplicationFactory : WebApplicationFactoryBase<Startup>
UserAsymmetricKeys = new KeysRequestModel()
{
PublicKey = TestEncryptionConstants.PublicKey,
EncryptedPrivateKey = TestEncryptionConstants.V1EncryptedBase64
EncryptedPrivateKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring
},
UserSymmetricKey = TestEncryptionConstants.V1EncryptedBase64,
UserSymmetricKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring,
});
return await _identityApplicationFactory.TokenFromPasswordAsync(email, masterPasswordHash);

View File

@@ -313,8 +313,8 @@ public class IdentityServerSsoTests
var user = await factory.Services.GetRequiredService<IUserRepository>().GetByEmailAsync(TestEmail);
Assert.NotNull(user);
const string expectedPrivateKey = TestEncryptionConstants.V1EncryptedBase64;
const string expectedUserKey = TestEncryptionConstants.V1EncryptedBase64;
const string expectedPrivateKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring;
const string expectedUserKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring;
var device = await deviceRepository.CreateAsync(new Device
{
@@ -323,7 +323,7 @@ public class IdentityServerSsoTests
Name = "Thing",
UserId = user.Id,
EncryptedPrivateKey = expectedPrivateKey,
EncryptedPublicKey = TestEncryptionConstants.V1EncryptedBase64,
EncryptedPublicKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring,
EncryptedUserKey = expectedUserKey,
});
@@ -630,7 +630,7 @@ public class IdentityServerSsoTests
factory.SubstituteService<IClientVersionValidator>(svc =>
{
svc.ValidateAsync(Arg.Any<User>(), Arg.Any<CustomValidatorRequestContext>())
.Returns(Task.FromResult(true));
.Returns(true);
});
var authorizationCode = new AuthorizationCode
@@ -662,9 +662,9 @@ public class IdentityServerSsoTests
UserAsymmetricKeys = new KeysRequestModel()
{
PublicKey = TestEncryptionConstants.PublicKey,
EncryptedPrivateKey = TestEncryptionConstants.V1EncryptedBase64 // v1-format so parsing succeeds and user is treated as v1
EncryptedPrivateKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring // v1-format so parsing succeeds and user is treated as v1
},
UserSymmetricKey = TestEncryptionConstants.V1EncryptedBase64,
UserSymmetricKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring,
});
var organizationRepository = factory.Services.GetRequiredService<IOrganizationRepository>();

View File

@@ -37,7 +37,7 @@ public class IdentityServerTests : IClassFixture<IdentityApplicationFactory>
_factory.SubstituteService<IClientVersionValidator>(svc =>
{
svc.ValidateAsync(Arg.Any<User>(), Arg.Any<CustomValidatorRequestContext>())
.Returns(Task.FromResult(true));
.Returns(true);
});
ReinitializeDbForTests(_factory);

View File

@@ -388,9 +388,9 @@ public class IdentityServerTwoFactorTests : IClassFixture<IdentityApplicationFac
UserAsymmetricKeys = new KeysRequestModel()
{
PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey,
EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64
EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring
},
UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64,
UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring,
});
Assert.NotNull(user);
@@ -442,9 +442,9 @@ public class IdentityServerTwoFactorTests : IClassFixture<IdentityApplicationFac
UserAsymmetricKeys = new KeysRequestModel()
{
PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey,
EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64
EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring
},
UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64,
UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring,
});
var userService = factory.GetService<IUserService>();

View File

@@ -614,9 +614,9 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture<IdentityApplica
UserAsymmetricKeys = new KeysRequestModel
{
PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey,
EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64
EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring
},
UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64,
UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring,
});
}

View File

@@ -109,7 +109,7 @@ public class BaseRequestValidatorTests
// Default client version validator behavior: allow to pass unless a test overrides.
_clientVersionValidator
.ValidateAsync(Arg.Any<User>(), Arg.Any<CustomValidatorRequestContext>())
.Returns(Task.FromResult(true));
.Returns(true);
}
private void SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(bool recoveryCodeSupportEnabled)
@@ -1296,7 +1296,7 @@ public class BaseRequestValidatorTests
// Make client version validation succeed but ensure it's invoked
_clientVersionValidator
.ValidateAsync(requestContext.User, requestContext)
.Returns(Task.FromResult(true));
.Returns(true);
// Ensure SSO requirement triggers an early stop after version validation to avoid success path setup
_policyService.AnyPoliciesApplicableToUserAsync(
@@ -1307,7 +1307,7 @@ public class BaseRequestValidatorTests
await _sut.ValidateAsync(context);
// Assert
await _clientVersionValidator.Received(1)
_clientVersionValidator.Received(1)
.ValidateAsync(requestContext.User, requestContext);
}

View File

@@ -1,7 +1,7 @@
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.KeyManagement.Queries.Interfaces;
using Bit.Identity.IdentityServer.RequestValidators;
using Bit.Test.Common.Constants;
using NSubstitute;
using Xunit;
@@ -9,34 +9,49 @@ namespace Bit.Identity.Test.IdentityServer.RequestValidators;
public class ClientVersionValidatorTests
{
private static ICurrentContext MakeContext(Version version)
private static ICurrentContext MakeContext(Version? version)
{
var ctx = Substitute.For<ICurrentContext>();
ctx.ClientVersion = version;
return ctx;
}
private static IGetMinimumClientVersionForUserQuery MakeMinQuery(Version? v)
private static User MakeValidV2User()
{
var q = Substitute.For<IGetMinimumClientVersionForUserQuery>();
q.Run(Arg.Any<User>()).Returns(Task.FromResult(v));
return q;
return new User
{
PrivateKey = TestEncryptionConstants.V2PrivateKey,
SecurityVersion = 2
};
}
[Fact]
public async Task Allows_When_NoMinVersion()
public void Allows_When_ClientMeetsMinimumVersion()
{
var sut = new ClientVersionValidator(MakeContext(new Version("2025.1.0")), MakeMinQuery(null));
var ok = await sut.ValidateAsync(new User(), new Bit.Identity.IdentityServer.CustomValidatorRequestContext());
// Arrange
var sut = new ClientVersionValidator(MakeContext(new Version("2025.11.0")));
var ctx = new Bit.Identity.IdentityServer.CustomValidatorRequestContext();
var user = MakeValidV2User();
// Act
var ok = sut.ValidateAsync(user, ctx);
// Assert
Assert.True(ok);
}
[Fact]
public async Task Blocks_When_ClientTooOld()
public void Blocks_When_ClientTooOld()
{
var sut = new ClientVersionValidator(MakeContext(new Version("2025.10.0")), MakeMinQuery(new Version("2025.11.0")));
// Arrange
var sut = new ClientVersionValidator(MakeContext(new Version("2025.10.0")));
var ctx = new Bit.Identity.IdentityServer.CustomValidatorRequestContext();
var ok = await sut.ValidateAsync(new User(), ctx);
var user = MakeValidV2User();
// Act
var ok = sut.ValidateAsync(user, ctx);
// Assert
Assert.False(ok);
Assert.NotNull(ctx.ValidationErrorResult);
Assert.True(ctx.ValidationErrorResult.IsError);
@@ -44,23 +59,66 @@ public class ClientVersionValidatorTests
}
[Fact]
public async Task Allows_When_ClientMeetsMin()
public void Blocks_When_NullUser()
{
var sut = new ClientVersionValidator(MakeContext(new Version("2025.11.0")), MakeMinQuery(new Version("2025.11.0")));
var ok = await sut.ValidateAsync(new User(), new Bit.Identity.IdentityServer.CustomValidatorRequestContext());
// Arrange
var sut = new ClientVersionValidator(MakeContext(new Version("2025.11.0")));
var ctx = new Bit.Identity.IdentityServer.CustomValidatorRequestContext();
User? user = null;
// Act
var ok = sut.ValidateAsync(user, ctx);
// Assert
Assert.False(ok);
Assert.NotNull(ctx.ValidationErrorResult);
Assert.True(ctx.ValidationErrorResult.IsError);
Assert.Equal("no_user", ctx.ValidationErrorResult.Error);
}
[Fact]
public void Allows_When_NoPrivateKey()
{
// Arrange
var sut = new ClientVersionValidator(MakeContext(new Version("2025.11.0")));
var ctx = new Bit.Identity.IdentityServer.CustomValidatorRequestContext();
var user = MakeValidV2User();
user.PrivateKey = null;
// Act
var ok = sut.ValidateAsync(user, ctx);
// Assert
Assert.True(ok);
}
[Fact]
public async Task Allows_When_ClientVersionHeaderMissing()
public void Allows_When_NoSecurityVersion()
{
// Do not set ClientVersion on the context (remains null) and ensure
var ctx = Substitute.For<ICurrentContext>();
var minQuery = MakeMinQuery(new Version("2025.11.0"));
var sut = new ClientVersionValidator(ctx, minQuery);
// Arrange
var sut = new ClientVersionValidator(MakeContext(new Version("2025.11.0")));
var ctx = new Bit.Identity.IdentityServer.CustomValidatorRequestContext();
var ok = await sut.ValidateAsync(new User(), new Bit.Identity.IdentityServer.CustomValidatorRequestContext());
var user = MakeValidV2User();
user.SecurityVersion = null;
// Act
var ok = sut.ValidateAsync(user, ctx);
// Assert
Assert.True(ok);
}
[Fact]
public void Allows_When_ClientVersionHeaderMissing()
{
// Arrange
var sut = new ClientVersionValidator(MakeContext(null));
var ctx = new Bit.Identity.IdentityServer.CustomValidatorRequestContext();
var user = MakeValidV2User();
// Act
var ok = sut.ValidateAsync(user, ctx);
// Assert
Assert.True(ok);
}
}

View File

@@ -55,7 +55,7 @@ public class IdentityApplicationFactory : WebApplicationFactoryBase<Startup>
SubstituteService<IClientVersionValidator>(svc =>
{
svc.ValidateAsync(Arg.Any<User>(), Arg.Any<CustomValidatorRequestContext>())
.Returns(Task.FromResult(true));
.Returns(true);
});
}