From 1c4fd6ca24b503dfd5091927a62c8cf0fc9b5d6e Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 17 Nov 2025 15:46:02 -0500 Subject: [PATCH 01/40] feat(auth-validator): [PM-22975] Client Version Validator - initial implementation --- src/Core/KeyManagement/Constants.cs | 6 + ...eyManagementServiceCollectionExtensions.cs | 2 + .../GetMinimumClientVersionForUserQuery.cs | 25 ++ .../IGetMinimumClientVersionForUserQuery.cs | 10 + .../Interfaces/IIsV2EncryptionUserQuery.cs | 10 + .../Queries/IsV2EncryptionUserQuery.cs | 33 +++ .../RotateUserAccountKeysCommand.cs | 265 ++++++++++++++++++ .../RotateUserAccountkeysCommand.cs | 29 +- .../Utilities/EncryptionParsing.cs | 46 +++ .../RequestValidators/BaseRequestValidator.cs | 25 +- .../ClientVersionValidator.cs | 55 ++++ .../CustomTokenRequestValidator.cs | 6 +- .../ResourceOwnerPasswordValidator.cs | 6 +- .../WebAuthnGrantValidator.cs | 6 +- .../Utilities/ServiceCollectionExtensions.cs | 1 + .../Utilities/ServiceCollectionExtensions.cs | 2 +- ...etMinimumClientVersionForUserQueryTests.cs | 34 +++ .../Queries/IsV2EncryptionUserQueryTests.cs | 65 +++++ .../Login/ClientVersionGateTests.cs | 119 ++++++++ .../ClientVersionValidatorTests.cs | 55 ++++ 20 files changed, 769 insertions(+), 31 deletions(-) create mode 100644 src/Core/KeyManagement/Constants.cs create mode 100644 src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs create mode 100644 src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs create mode 100644 src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs create mode 100644 src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs create mode 100644 src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs create mode 100644 src/Core/KeyManagement/Utilities/EncryptionParsing.cs create mode 100644 src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs create mode 100644 test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs create mode 100644 test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs create mode 100644 test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs create mode 100644 test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs diff --git a/src/Core/KeyManagement/Constants.cs b/src/Core/KeyManagement/Constants.cs new file mode 100644 index 000000000000..2bc44134be6e --- /dev/null +++ b/src/Core/KeyManagement/Constants.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.KeyManagement; + +public static class Constants +{ + public static readonly Version MinimumClientVersion = new Version("2025.11.0"); +} diff --git a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs index 0e551c5d0e20..9b63dfdaf9a3 100644 --- a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs +++ b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs @@ -26,5 +26,7 @@ private static void AddKeyManagementCommands(this IServiceCollection services) private static void AddKeyManagementQueries(this IServiceCollection services) { services.AddScoped(); + services.AddScoped(); + services.AddScoped(); } } diff --git a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs new file mode 100644 index 000000000000..49cc46a54c41 --- /dev/null +++ b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs @@ -0,0 +1,25 @@ +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Queries.Interfaces; + +namespace Bit.Core.KeyManagement.Queries; + +public class GetMinimumClientVersionForUserQuery(IIsV2EncryptionUserQuery isV2EncryptionUserQuery) + : IGetMinimumClientVersionForUserQuery +{ + public async Task Run(User? user) + { + if (user == null) + { + return null; + } + + if (await isV2EncryptionUserQuery.Run(user)) + { + return Constants.MinimumClientVersion; + } + + return null; + } +} + + diff --git a/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs new file mode 100644 index 000000000000..896a473d6942 --- /dev/null +++ b/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs @@ -0,0 +1,10 @@ +using Bit.Core.Entities; + +namespace Bit.Core.KeyManagement.Queries.Interfaces; + +public interface IGetMinimumClientVersionForUserQuery +{ + Task Run(User? user); +} + + diff --git a/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs b/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs new file mode 100644 index 000000000000..6b644a2dc7d3 --- /dev/null +++ b/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs @@ -0,0 +1,10 @@ +using Bit.Core.Entities; + +namespace Bit.Core.KeyManagement.Queries.Interfaces; + +public interface IIsV2EncryptionUserQuery +{ + Task Run(User user); +} + + diff --git a/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs b/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs new file mode 100644 index 000000000000..74a203004c1f --- /dev/null +++ b/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs @@ -0,0 +1,33 @@ +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 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.") + }; + } +} + + diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs new file mode 100644 index 000000000000..6e5708f667d8 --- /dev/null +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs @@ -0,0 +1,265 @@ +// FIXME: Update this file to be null safe and then delete the line below +#nullable disable + +using Bit.Core.Auth.Repositories; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.Repositories; +using Bit.Core.KeyManagement.Utilities; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Tools.Repositories; +using Bit.Core.Vault.Repositories; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.KeyManagement.UserKey.Implementations; + +/// +public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand +{ + private readonly IUserService _userService; + private readonly IUserRepository _userRepository; + private readonly ICipherRepository _cipherRepository; + private readonly IFolderRepository _folderRepository; + private readonly ISendRepository _sendRepository; + private readonly IEmergencyAccessRepository _emergencyAccessRepository; + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IDeviceRepository _deviceRepository; + private readonly IPushNotificationService _pushService; + private readonly IdentityErrorDescriber _identityErrorDescriber; + private readonly IWebAuthnCredentialRepository _credentialRepository; + private readonly IPasswordHasher _passwordHasher; + private readonly IUserSignatureKeyPairRepository _userSignatureKeyPairRepository; + private readonly IFeatureService _featureService; + + /// + /// Instantiates a new + /// + /// Master password hash validation + /// Updates user keys and re-encrypted data if needed + /// Provides a method to update re-encrypted cipher data + /// Provides a method to update re-encrypted folder data + /// Provides a method to update re-encrypted send data + /// Provides a method to update re-encrypted emergency access data + /// Provides a method to update re-encrypted organization user data + /// Provides a method to update re-encrypted device keys + /// Hashes the new master password + /// Logs out user from other devices after successful rotation + /// Provides a password mismatch error if master password hash validation fails + /// Provides a method to update re-encrypted WebAuthn keys + /// Provides a method to update re-encrypted signature keys + public RotateUserAccountKeysCommand(IUserService userService, IUserRepository userRepository, + ICipherRepository cipherRepository, IFolderRepository folderRepository, ISendRepository sendRepository, + IEmergencyAccessRepository emergencyAccessRepository, IOrganizationUserRepository organizationUserRepository, + IDeviceRepository deviceRepository, + IPasswordHasher passwordHasher, + IPushNotificationService pushService, IdentityErrorDescriber errors, IWebAuthnCredentialRepository credentialRepository, + IUserSignatureKeyPairRepository userSignatureKeyPairRepository, + IFeatureService featureService) + { + _userService = userService; + _userRepository = userRepository; + _cipherRepository = cipherRepository; + _folderRepository = folderRepository; + _sendRepository = sendRepository; + _emergencyAccessRepository = emergencyAccessRepository; + _organizationUserRepository = organizationUserRepository; + _deviceRepository = deviceRepository; + _pushService = pushService; + _identityErrorDescriber = errors; + _credentialRepository = credentialRepository; + _passwordHasher = passwordHasher; + _userSignatureKeyPairRepository = userSignatureKeyPairRepository; + _featureService = featureService; + } + + /// + public async Task RotateUserAccountKeysAsync(User user, RotateUserAccountKeysData model) + { + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + + if (!await _userService.CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash)) + { + return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); + } + + var now = DateTime.UtcNow; + user.RevisionDate = user.AccountRevisionDate = now; + user.LastKeyRotationDate = now; + user.SecurityStamp = Guid.NewGuid().ToString(); + + List saveEncryptedDataActions = []; + + await UpdateAccountKeysAsync(model, user, saveEncryptedDataActions); + UpdateUnlockMethods(model, user, saveEncryptedDataActions); + UpdateUserData(model, user, saveEncryptedDataActions); + + await _userRepository.UpdateUserKeyAndEncryptedDataV2Async(user, saveEncryptedDataActions); + await _pushService.PushLogOutAsync(user.Id); + return IdentityResult.Success; + } + + public async Task RotateV2AccountKeysAsync(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) + { + ValidateV2Encryption(model); + await ValidateVerifyingKeyUnchangedAsync(model, user); + + saveEncryptedDataActions.Add(_userSignatureKeyPairRepository.UpdateForKeyRotation(user.Id, model.AccountKeys.SignatureKeyPairData)); + user.SignedPublicKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey; + user.SecurityState = model.AccountKeys.SecurityStateData!.SecurityState; + user.SecurityVersion = model.AccountKeys.SecurityStateData.SecurityVersion; + } + + public void UpgradeV1ToV2Keys(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) + { + ValidateV2Encryption(model); + saveEncryptedDataActions.Add(_userSignatureKeyPairRepository.SetUserSignatureKeyPair(user.Id, model.AccountKeys.SignatureKeyPairData)); + user.SignedPublicKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey; + user.SecurityState = model.AccountKeys.SecurityStateData!.SecurityState; + user.SecurityVersion = model.AccountKeys.SecurityStateData.SecurityVersion; + } + + public async Task UpdateAccountKeysAsync(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) + { + ValidatePublicKeyEncryptionKeyPairUnchanged(model, user); + + if (IsV2EncryptionUserAsync(user)) + { + await RotateV2AccountKeysAsync(model, user, saveEncryptedDataActions); + } + else if (model.AccountKeys.SignatureKeyPairData != null) + { + UpgradeV1ToV2Keys(model, user, saveEncryptedDataActions); + } + else + { + if (EncryptionParsing.GetEncryptionType(model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey) != EncryptionType.AesCbc256_HmacSha256_B64) + { + throw new InvalidOperationException("The provided account private key was not wrapped with AES-256-CBC-HMAC"); + } + // V1 user to V1 user rotation needs to further changes, the private key was re-encrypted. + } + + // Private key is re-wrapped with new user key by client + user.PrivateKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey; + } + + public void UpdateUserData(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) + { + // The revision date has to be updated so that de-synced clients don't accidentally post over the re-encrypted data + // with an old-user key-encrypted copy + var now = DateTime.UtcNow; + + if (model.Ciphers.Any()) + { + var ciphersWithUpdatedDate = model.Ciphers.ToList().Select(c => { c.RevisionDate = now; return c; }); + saveEncryptedDataActions.Add(_cipherRepository.UpdateForKeyRotation(user.Id, ciphersWithUpdatedDate)); + } + + if (model.Folders.Any()) + { + var foldersWithUpdatedDate = model.Folders.ToList().Select(f => { f.RevisionDate = now; return f; }); + saveEncryptedDataActions.Add(_folderRepository.UpdateForKeyRotation(user.Id, foldersWithUpdatedDate)); + } + + if (model.Sends.Any()) + { + var sendsWithUpdatedDate = model.Sends.ToList().Select(s => { s.RevisionDate = now; return s; }); + saveEncryptedDataActions.Add(_sendRepository.UpdateForKeyRotation(user.Id, sendsWithUpdatedDate)); + } + } + + void UpdateUnlockMethods(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) + { + if (!model.MasterPasswordUnlockData.ValidateForUser(user)) + { + throw new InvalidOperationException("The provided master password unlock data is not valid for this user."); + } + // Update master password authentication & unlock + user.Key = model.MasterPasswordUnlockData.MasterKeyEncryptedUserKey; + user.MasterPassword = _passwordHasher.HashPassword(user, model.MasterPasswordUnlockData.MasterKeyAuthenticationHash); + user.MasterPasswordHint = model.MasterPasswordUnlockData.MasterPasswordHint; + + if (model.EmergencyAccesses.Any()) + { + saveEncryptedDataActions.Add(_emergencyAccessRepository.UpdateForKeyRotation(user.Id, model.EmergencyAccesses)); + } + + if (model.OrganizationUsers.Any()) + { + saveEncryptedDataActions.Add(_organizationUserRepository.UpdateForKeyRotation(user.Id, model.OrganizationUsers)); + } + + if (model.WebAuthnKeys.Any()) + { + saveEncryptedDataActions.Add(_credentialRepository.UpdateKeysForRotationAsync(user.Id, model.WebAuthnKeys)); + } + + if (model.DeviceKeys.Any()) + { + saveEncryptedDataActions.Add(_deviceRepository.UpdateKeysForRotationAsync(user.Id, model.DeviceKeys)); + } + } + + private bool IsV2EncryptionUserAsync(User user) + { + // Returns whether the user is a V2 user based on the private key's encryption type. + ArgumentNullException.ThrowIfNull(user); + var isPrivateKeyEncryptionV2 = EncryptionParsing.GetEncryptionType(user.PrivateKey) == EncryptionType.XChaCha20Poly1305_B64; + return isPrivateKeyEncryptionV2; + } + + private async Task ValidateVerifyingKeyUnchangedAsync(RotateUserAccountKeysData model, User user) + { + var currentSignatureKeyPair = await _userSignatureKeyPairRepository.GetByUserIdAsync(user.Id) ?? throw new InvalidOperationException("User does not have a signature key pair."); + if (model.AccountKeys.SignatureKeyPairData.VerifyingKey != currentSignatureKeyPair!.VerifyingKey) + { + throw new InvalidOperationException("The provided verifying key does not match the user's current verifying key."); + } + } + + private static void ValidatePublicKeyEncryptionKeyPairUnchanged(RotateUserAccountKeysData model, User user) + { + var publicKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.PublicKey; + if (publicKey != user.PublicKey) + { + throw new InvalidOperationException("The provided account public key does not match the user's current public key, and changing the account asymmetric key pair is currently not supported during key rotation."); + } + } + + private static void ValidateV2Encryption(RotateUserAccountKeysData model) + { + if (model.AccountKeys.SignatureKeyPairData == null) + { + throw new InvalidOperationException("Signature key pair data is required for V2 encryption."); + } + if (EncryptionParsing.GetEncryptionType(model.AccountKeys.SignatureKeyPairData.WrappedSigningKey) != EncryptionType.XChaCha20Poly1305_B64) + { + throw new InvalidOperationException("The provided signing key data is not wrapped with XChaCha20-Poly1305."); + } + if (string.IsNullOrEmpty(model.AccountKeys.SignatureKeyPairData.VerifyingKey)) + { + throw new InvalidOperationException("The provided signature key pair data does not contain a valid verifying key."); + } + + if (EncryptionParsing.GetEncryptionType(model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey) != EncryptionType.XChaCha20Poly1305_B64) + { + throw new InvalidOperationException("The provided private key encryption key is not wrapped with XChaCha20-Poly1305."); + } + if (string.IsNullOrEmpty(model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey)) + { + throw new InvalidOperationException("No signed public key provided, but the user already has a signature key pair."); + } + if (model.AccountKeys.SecurityStateData == null || string.IsNullOrEmpty(model.AccountKeys.SecurityStateData.SecurityState)) + { + throw new InvalidOperationException("No signed security state provider for V2 user"); + } + } + + // Parsing moved to Bit.Core.KeyManagement.Utilities.EncryptionParsing +} diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs index c1e7905d7857..6e5708f667d8 100644 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs @@ -6,6 +6,7 @@ using Bit.Core.Enums; using Bit.Core.KeyManagement.Models.Data; using Bit.Core.KeyManagement.Repositories; +using Bit.Core.KeyManagement.Utilities; using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Services; @@ -137,7 +138,7 @@ public async Task UpdateAccountKeysAsync(RotateUserAccountKeysData model, User u } else { - if (GetEncryptionType(model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey) != EncryptionType.AesCbc256_HmacSha256_B64) + if (EncryptionParsing.GetEncryptionType(model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey) != EncryptionType.AesCbc256_HmacSha256_B64) { throw new InvalidOperationException("The provided account private key was not wrapped with AES-256-CBC-HMAC"); } @@ -209,7 +210,7 @@ private bool IsV2EncryptionUserAsync(User user) { // Returns whether the user is a V2 user based on the private key's encryption type. ArgumentNullException.ThrowIfNull(user); - var isPrivateKeyEncryptionV2 = GetEncryptionType(user.PrivateKey) == EncryptionType.XChaCha20Poly1305_B64; + var isPrivateKeyEncryptionV2 = EncryptionParsing.GetEncryptionType(user.PrivateKey) == EncryptionType.XChaCha20Poly1305_B64; return isPrivateKeyEncryptionV2; } @@ -237,7 +238,7 @@ private static void ValidateV2Encryption(RotateUserAccountKeysData model) { throw new InvalidOperationException("Signature key pair data is required for V2 encryption."); } - if (GetEncryptionType(model.AccountKeys.SignatureKeyPairData.WrappedSigningKey) != EncryptionType.XChaCha20Poly1305_B64) + if (EncryptionParsing.GetEncryptionType(model.AccountKeys.SignatureKeyPairData.WrappedSigningKey) != EncryptionType.XChaCha20Poly1305_B64) { throw new InvalidOperationException("The provided signing key data is not wrapped with XChaCha20-Poly1305."); } @@ -246,7 +247,7 @@ private static void ValidateV2Encryption(RotateUserAccountKeysData model) throw new InvalidOperationException("The provided signature key pair data does not contain a valid verifying key."); } - if (GetEncryptionType(model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey) != EncryptionType.XChaCha20Poly1305_B64) + if (EncryptionParsing.GetEncryptionType(model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey) != EncryptionType.XChaCha20Poly1305_B64) { throw new InvalidOperationException("The provided private key encryption key is not wrapped with XChaCha20-Poly1305."); } @@ -260,23 +261,5 @@ private static void ValidateV2Encryption(RotateUserAccountKeysData model) } } - /// - /// Helper method to convert an encryption type string to an enum value. - /// - private static EncryptionType GetEncryptionType(string encString) - { - var parts = encString.Split('.'); - if (parts.Length == 1) - { - throw new ArgumentException("Invalid encryption type string."); - } - if (byte.TryParse(parts[0], out var encryptionTypeNumber)) - { - if (Enum.IsDefined(typeof(EncryptionType), encryptionTypeNumber)) - { - return (EncryptionType)encryptionTypeNumber; - } - } - throw new ArgumentException("Invalid encryption type string."); - } + // Parsing moved to Bit.Core.KeyManagement.Utilities.EncryptionParsing } diff --git a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs new file mode 100644 index 000000000000..4658f4cf592e --- /dev/null +++ b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs @@ -0,0 +1,46 @@ +using Bit.Core.Enums; + +namespace Bit.Core.KeyManagement.Utilities; + +public static class EncryptionParsing +{ + /// + /// Helper method to convert an encryption type string to an enum value. + /// Accepts formats like "Header.iv|ct|mac" or "Header" COSE format. + /// + public static EncryptionType GetEncryptionType(string encString) + { + if (string.IsNullOrWhiteSpace(encString)) + { + throw new ArgumentException("Encrypted string cannot be null or empty.", nameof(encString)); + } + + var parts = encString.Split('.'); + if (parts.Length == 1) + { + // No header detected; assume AES CBC variants based on number of pieces + var splitParts = encString.Split('|'); + if (splitParts.Length == 3) + { + return EncryptionType.AesCbc128_HmacSha256_B64; + } + + return EncryptionType.AesCbc256_B64; + } + + // Try parse header as numeric, then as enum name, else fail + if (byte.TryParse(parts[0], out var encryptionTypeNumber)) + { + return (EncryptionType)encryptionTypeNumber; + } + + if (Enum.TryParse(parts[0], out EncryptionType parsed)) + { + return parsed; + } + + throw new ArgumentException("Invalid encryption type header.", nameof(encString)); + } +} + + diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 224c7a1866ab..4226098b4e83 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -40,6 +40,7 @@ public abstract class BaseRequestValidator where T : class private readonly IUserRepository _userRepository; private readonly IAuthRequestRepository _authRequestRepository; private readonly IMailService _mailService; + private readonly IClientVersionValidator _clientVersionValidator; protected ICurrentContext CurrentContext { get; } protected IPolicyService PolicyService { get; } @@ -68,7 +69,8 @@ public BaseRequestValidator( IPolicyRequirementQuery policyRequirementQuery, IAuthRequestRepository authRequestRepository, IMailService mailService, - IUserAccountKeysQuery userAccountKeysQuery + IUserAccountKeysQuery userAccountKeysQuery, + IClientVersionValidator clientVersionValidator ) { _userManager = userManager; @@ -89,6 +91,7 @@ IUserAccountKeysQuery userAccountKeysQuery _authRequestRepository = authRequestRepository; _mailService = mailService; _accountKeysQuery = userAccountKeysQuery; + _clientVersionValidator = clientVersionValidator; } protected async Task ValidateAsync(T context, ValidatedTokenRequest request, @@ -259,6 +262,7 @@ private Func>[] DetermineValidationOrder(T context, ValidatedTokenReq return [ () => ValidateMasterPasswordAsync(context, validatorContext), + () => ValidateClientVersionAsync(context, validatorContext), () => ValidateTwoFactorAsync(context, request, validatorContext), () => ValidateSsoAsync(context, request, validatorContext), () => ValidateNewDeviceAsync(context, request, validatorContext), @@ -272,6 +276,7 @@ private Func>[] DetermineValidationOrder(T context, ValidatedTokenReq return [ () => ValidateMasterPasswordAsync(context, validatorContext), + () => ValidateClientVersionAsync(context, validatorContext), () => ValidateSsoAsync(context, request, validatorContext), () => ValidateTwoFactorAsync(context, request, validatorContext), () => ValidateNewDeviceAsync(context, request, validatorContext), @@ -323,6 +328,24 @@ private static async Task ProcessValidatorsAsync(params Func>[] return true; } + /// + /// Validates whether the client version is compatible for the user attempting to authenticate. + /// New authentications only; refresh/device grants are handled elsewhere. + /// + /// true if the scheme successfully passed validation, otherwise false. + private async Task ValidateClientVersionAsync(T context, CustomValidatorRequestContext validatorContext) + { + var ok = await _clientVersionValidator.ValidateAsync(validatorContext.User, validatorContext); + if (ok) + { + return true; + } + + SetValidationErrorResult(context, validatorContext); + await LogFailedLoginEvent(validatorContext.User, EventType.User_FailedLogIn); + return false; + } + /// /// Validates the user's Master Password hash. /// diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs new file mode 100644 index 000000000000..3d35db7f172b --- /dev/null +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -0,0 +1,55 @@ +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Queries.Interfaces; +using Bit.Core.Models.Api; +using Duende.IdentityServer.Validation; + +namespace Bit.Identity.IdentityServer.RequestValidators; + +public interface IClientVersionValidator +{ + Task ValidateAsync(User user, CustomValidatorRequestContext requestContext); +} + +public class ClientVersionValidator(ICurrentContext currentContext, + IGetMinimumClientVersionForUserQuery getMinimumClientVersionForUserQuery) + : IClientVersionValidator +{ + private static readonly string UpgradeMessage = "Please update your app to continue using Bitwarden"; + + public async Task ValidateAsync(User? user, CustomValidatorRequestContext requestContext) + { + if (user == null) + { + return true; + } + + var clientVersion = currentContext.ClientVersion; + var minVersion = await getMinimumClientVersionForUserQuery.Run(user); + + // Fail-open if headers are missing or no restriction + if (minVersion == null) + { + return true; + } + + if (clientVersion < minVersion) + { + requestContext.ValidationErrorResult = new ValidationResult + { + Error = "invalid_grant", + ErrorDescription = UpgradeMessage, + IsError = true + }; + requestContext.CustomResponse = new Dictionary + { + { "ErrorModel", new ErrorResponseModel(UpgradeMessage) } + }; + return false; + } + + return true; + } +} + + diff --git a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs index 64156ea5f3a0..e7fc4b6498c2 100644 --- a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs @@ -49,7 +49,8 @@ public CustomTokenRequestValidator( IPolicyRequirementQuery policyRequirementQuery, IAuthRequestRepository authRequestRepository, IMailService mailService, - IUserAccountKeysQuery userAccountKeysQuery) + IUserAccountKeysQuery userAccountKeysQuery, + IClientVersionValidator clientVersionValidator) : base( userManager, userService, @@ -68,7 +69,8 @@ public CustomTokenRequestValidator( policyRequirementQuery, authRequestRepository, mailService, - userAccountKeysQuery) + userAccountKeysQuery, + clientVersionValidator) { _userManager = userManager; _updateInstallationCommand = updateInstallationCommand; diff --git a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs index d69d521ef7a3..a9952c26f250 100644 --- a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs @@ -43,7 +43,8 @@ public ResourceOwnerPasswordValidator( IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder, IPolicyRequirementQuery policyRequirementQuery, IMailService mailService, - IUserAccountKeysQuery userAccountKeysQuery) + IUserAccountKeysQuery userAccountKeysQuery, + IClientVersionValidator clientVersionValidator) : base( userManager, userService, @@ -62,7 +63,8 @@ public ResourceOwnerPasswordValidator( policyRequirementQuery, authRequestRepository, mailService, - userAccountKeysQuery) + userAccountKeysQuery, + clientVersionValidator) { _userManager = userManager; _currentContext = currentContext; diff --git a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs index 294df1c18d37..d0290106388e 100644 --- a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs @@ -52,7 +52,8 @@ public WebAuthnGrantValidator( IPolicyRequirementQuery policyRequirementQuery, IAuthRequestRepository authRequestRepository, IMailService mailService, - IUserAccountKeysQuery userAccountKeysQuery) + IUserAccountKeysQuery userAccountKeysQuery, + IClientVersionValidator clientVersionValidator) : base( userManager, userService, @@ -71,7 +72,8 @@ public WebAuthnGrantValidator( policyRequirementQuery, authRequestRepository, mailService, - userAccountKeysQuery) + userAccountKeysQuery, + clientVersionValidator) { _assertionOptionsDataProtector = assertionOptionsDataProtector; _assertWebAuthnLoginCredentialCommand = assertWebAuthnLoginCredentialCommand; diff --git a/src/Identity/Utilities/ServiceCollectionExtensions.cs b/src/Identity/Utilities/ServiceCollectionExtensions.cs index e9056d030e70..9a5188db25dd 100644 --- a/src/Identity/Utilities/ServiceCollectionExtensions.cs +++ b/src/Identity/Utilities/ServiceCollectionExtensions.cs @@ -25,6 +25,7 @@ public static IIdentityServerBuilder AddCustomIdentityServerServices(this IServi services.AddTransient(); services.AddTransient(); services.AddTransient(); + services.AddTransient(); services.AddTransient(); services.AddTransient(); services.AddTransient, SendPasswordRequestValidator>(); diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index ef143b042cb2..1243a3ee604b 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -7,7 +7,6 @@ using System.Security.Cryptography.X509Certificates; using AspNetCoreRateLimit; using Azure.Messaging.ServiceBus; -using Bit.Core; using Bit.Core.AdminConsole.AbilitiesCache; using Bit.Core.AdminConsole.Models.Business.Tokenables; using Bit.Core.AdminConsole.Models.Data.EventIntegrations; @@ -85,6 +84,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using StackExchange.Redis; +using Constants = Bit.Core.Constants; using NoopRepos = Bit.Core.Repositories.Noop; using Role = Bit.Core.Entities.Role; using TableStorageRepos = Bit.Core.Repositories.TableStorage; diff --git a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs new file mode 100644 index 000000000000..1a04e60ca536 --- /dev/null +++ b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs @@ -0,0 +1,34 @@ +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 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()); + Assert.Equal(Core.KeyManagement.Constants.MinimumClientVersion, version); + } + + [Fact] + public async Task Run_ReturnsNull_ForV1User() + { + var sut = new GetMinimumClientVersionForUserQuery(new FakeIsV2Query(false)); + var version = await sut.Run(new User()); + Assert.Null(version); + } +} + + diff --git a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs new file mode 100644 index 000000000000..67c3601b81a9 --- /dev/null +++ b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs @@ -0,0 +1,65 @@ +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 Xunit; + +namespace Bit.Core.Test.KeyManagement.Queries; + +public class IsV2EncryptionUserQueryTests +{ + private class FakeSigRepo : IUserSignatureKeyPairRepository + { + private readonly bool _hasKeys; + public FakeSigRepo(bool hasKeys) { _hasKeys = hasKeys; } + public Task GetByUserIdAsync(Guid userId) + => Task.FromResult(_hasKeys ? new SignatureKeyPairData(SignatureAlgorithm.Ed25519, "7.cose_signing", "vk") : null); + + // Unused in tests + public Task> GetManyAsync(IEnumerable ids) => throw new NotImplementedException(); + public Task GetByIdAsync(Guid id) => throw new NotImplementedException(); + public Task 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 = "7.cose" }; + var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(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 = "2.iv|ct|mac" }; + var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(false)); + + var result = await sut.Run(user); + + Assert.False(result); + } + + [Fact] + public async Task Run_ThrowsForInvalidMixedState() + { + var user = new User { Id = Guid.NewGuid(), PrivateKey = "7.cose" }; + var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(false)); + + await Assert.ThrowsAsync(async () => await sut.Run(user)); + } +} + + diff --git a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs new file mode 100644 index 000000000000..54f005688e98 --- /dev/null +++ b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs @@ -0,0 +1,119 @@ +using System.Text.Json; +using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.KeyManagement.Enums; +using Bit.Core.Test.Auth.AutoFixture; +using Bit.IntegrationTestCommon.Factories; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.EntityFrameworkCore; +using Xunit; + +namespace Bit.Identity.IntegrationTest.Login; + +public class ClientVersionGateTests : IClassFixture +{ + private readonly IdentityApplicationFactory _factory; + + public ClientVersionGateTests(IdentityApplicationFactory factory) + { + _factory = factory; + ReinitializeDbForTests(_factory); + } + + [Theory, BitAutoData, RegisterFinishRequestModelCustomize] + public async Task TokenEndpoint_GrantTypePassword_V2User_OnOldClientVersion_Blocked(RegisterFinishRequestModel requestModel) + { + var localFactory = new IdentityApplicationFactory(); + var server = localFactory.Server; + var user = await localFactory.RegisterNewIdentityFactoryUserAsync(requestModel); + + // Make user V2: set private key to COSE and add signature key pair + var db = localFactory.GetDatabaseContext(); + var efUser = await db.Users.FirstAsync(u => u.Email == user.Email); + efUser.PrivateKey = "7.cose"; + db.UserSignatureKeyPairs.Add(new Bit.Infrastructure.EntityFramework.Models.UserSignatureKeyPair + { + Id = Core.Utilities.CoreHelpers.GenerateComb(), + UserId = efUser.Id, + SignatureAlgorithm = SignatureAlgorithm.Ed25519, + SigningKey = "7.cose_signing", + VerifyingKey = "vk" + }); + await db.SaveChangesAsync(); + + var context = await server.PostAsync("/connect/token", + new FormUrlEncodedContent(new Dictionary + { + { "scope", "api offline_access" }, + { "client_id", "web" }, + { "deviceType", "2" }, + { "deviceIdentifier", IdentityApplicationFactory.DefaultDeviceIdentifier }, + { "deviceName", "firefox" }, + { "grant_type", "password" }, + { "username", user.Email }, + { "password", requestModel.MasterPasswordHash }, + }), + http => + { + http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); + }); + + Assert.Equal(StatusCodes.Status400BadRequest, context.Response.StatusCode); + var errorBody = await Bit.Test.Common.Helpers.AssertHelper.AssertResponseTypeIs(context); + var error = Bit.Test.Common.Helpers.AssertHelper.AssertJsonProperty(errorBody.RootElement, "ErrorModel", JsonValueKind.Object); + var message = Bit.Test.Common.Helpers.AssertHelper.AssertJsonProperty(error, "Message", JsonValueKind.String).GetString(); + Assert.Equal("Please update your app to continue using Bitwarden", message); + } + + [Theory, BitAutoData, RegisterFinishRequestModelCustomize] + public async Task TokenEndpoint_GrantTypePassword_V2User_OnMinClientVersion_Succeeds(RegisterFinishRequestModel requestModel) + { + var localFactory = new IdentityApplicationFactory(); + var server = localFactory.Server; + var user = await localFactory.RegisterNewIdentityFactoryUserAsync(requestModel); + + // Make user V2 + var db = localFactory.GetDatabaseContext(); + var efUser = await db.Users.FirstAsync(u => u.Email == user.Email); + efUser.PrivateKey = "7.cose"; + db.UserSignatureKeyPairs.Add(new Bit.Infrastructure.EntityFramework.Models.UserSignatureKeyPair + { + Id = Core.Utilities.CoreHelpers.GenerateComb(), + UserId = efUser.Id, + SignatureAlgorithm = SignatureAlgorithm.Ed25519, + SigningKey = "7.cose_signing", + VerifyingKey = "vk" + }); + await db.SaveChangesAsync(); + + var context = await server.PostAsync("/connect/token", + new FormUrlEncodedContent(new Dictionary + { + { "scope", "api offline_access" }, + { "client_id", "web" }, + { "deviceType", "2" }, + { "deviceIdentifier", IdentityApplicationFactory.DefaultDeviceIdentifier }, + { "deviceName", "firefox" }, + { "grant_type", "password" }, + { "username", user.Email }, + { "password", requestModel.MasterPasswordHash }, + }), + http => + { + http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); + }); + + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + } + + private void ReinitializeDbForTests(IdentityApplicationFactory factory) + { + var databaseContext = factory.GetDatabaseContext(); + databaseContext.Policies.RemoveRange(databaseContext.Policies); + databaseContext.OrganizationUsers.RemoveRange(databaseContext.OrganizationUsers); + databaseContext.Organizations.RemoveRange(databaseContext.Organizations); + databaseContext.Users.RemoveRange(databaseContext.Users); + databaseContext.SaveChanges(); + } +} + + diff --git a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs new file mode 100644 index 000000000000..dc491596fe59 --- /dev/null +++ b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs @@ -0,0 +1,55 @@ +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Queries.Interfaces; +using Bit.Identity.IdentityServer.RequestValidators; +using NSubstitute; +using Xunit; + +namespace Bit.Identity.Test.IdentityServer.RequestValidators; + +public class ClientVersionValidatorTests +{ + private static ICurrentContext MakeContext(Version version) + { + var ctx = Substitute.For(); + ctx.ClientVersion = version; + return ctx; + } + + private static IGetMinimumClientVersionForUserQuery MakeMinQuery(Version? v) + { + var q = Substitute.For(); + q.Run(Arg.Any()).Returns(Task.FromResult(v)); + return q; + } + + [Fact] + public async Task Allows_When_NoMinVersion() + { + var sut = new ClientVersionValidator(MakeContext(new Version("2025.1.0")), MakeMinQuery(null)); + var ok = await sut.ValidateAsync(new User(), new Bit.Identity.IdentityServer.CustomValidatorRequestContext()); + Assert.True(ok); + } + + [Fact] + public async Task Blocks_When_ClientTooOld() + { + var sut = new ClientVersionValidator(MakeContext(new Version("2025.10.0")), MakeMinQuery(new Version("2025.11.0"))); + var ctx = new Bit.Identity.IdentityServer.CustomValidatorRequestContext(); + var ok = await sut.ValidateAsync(new User(), ctx); + Assert.False(ok); + Assert.NotNull(ctx.ValidationErrorResult); + Assert.True(ctx.ValidationErrorResult.IsError); + Assert.Equal("invalid_grant", ctx.ValidationErrorResult.Error); + } + + [Fact] + public async Task Allows_When_ClientMeetsMin() + { + 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()); + Assert.True(ok); + } +} + + From 22e9e5bc5d4ea86772f9d9dd74514f9439b82210 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 17 Nov 2025 15:46:40 -0500 Subject: [PATCH 02/40] mend --- .../Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs index ec3e791d5b41..dc6d80a730e6 100644 --- a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs +++ b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs @@ -66,6 +66,7 @@ public BaseRequestValidatorTestWrapper( IPolicyRequirementQuery policyRequirementQuery, IAuthRequestRepository authRequestRepository, IMailService mailService, + IClientVersionValidator clientVersionValidator, IUserAccountKeysQuery userAccountKeysQuery) : base( userManager, @@ -85,7 +86,8 @@ public BaseRequestValidatorTestWrapper( policyRequirementQuery, authRequestRepository, mailService, - userAccountKeysQuery) + userAccountKeysQuery, + clientVersionValidator) { } From 47a26bb204a81ea555f30607df1ea7ef8b632c09 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 20 Nov 2025 10:30:18 -0500 Subject: [PATCH 03/40] fix(auth-validator): [PM-22975] Client Version Validator - Minor touchups to baserequest validator. --- .../Utilities/EncryptionParsing.cs | 29 ++++--------------- .../RequestValidators/BaseRequestValidator.cs | 4 +++ .../ClientVersionValidator.cs | 5 ++-- .../CustomTokenRequestValidator.cs | 3 -- 4 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs index 4658f4cf592e..f288cfd99a37 100644 --- a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs +++ b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs @@ -10,36 +10,19 @@ public static class EncryptionParsing /// public static EncryptionType GetEncryptionType(string encString) { - if (string.IsNullOrWhiteSpace(encString)) - { - throw new ArgumentException("Encrypted string cannot be null or empty.", nameof(encString)); - } - var parts = encString.Split('.'); if (parts.Length == 1) { - // No header detected; assume AES CBC variants based on number of pieces - var splitParts = encString.Split('|'); - if (splitParts.Length == 3) - { - return EncryptionType.AesCbc128_HmacSha256_B64; - } - - return EncryptionType.AesCbc256_B64; + throw new ArgumentException("Invalid encryption type string."); } - - // Try parse header as numeric, then as enum name, else fail if (byte.TryParse(parts[0], out var encryptionTypeNumber)) { - return (EncryptionType)encryptionTypeNumber; - } - - if (Enum.TryParse(parts[0], out EncryptionType parsed)) - { - return parsed; + if (Enum.IsDefined(typeof(EncryptionType), encryptionTypeNumber)) + { + return (EncryptionType)encryptionTypeNumber; + } } - - throw new ArgumentException("Invalid encryption type header.", nameof(encString)); + throw new ArgumentException("Invalid encryption type string."); } } diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 4226098b4e83..ae6aaed2bda9 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -122,6 +122,10 @@ await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.D return; } + // 1.5 We need to check now the version number + await ValidateClientVersionAsync(context, validatorContext); + + // 2. Decide if this user belongs to an organization that requires SSO. validatorContext.SsoRequired = await RequireSsoLoginAsync(user, request.GrantType); if (validatorContext.SsoRequired) diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index 3d35db7f172b..5896061e1312 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -11,7 +11,8 @@ public interface IClientVersionValidator Task ValidateAsync(User user, CustomValidatorRequestContext requestContext); } -public class ClientVersionValidator(ICurrentContext currentContext, +public class ClientVersionValidator( + ICurrentContext currentContext, IGetMinimumClientVersionForUserQuery getMinimumClientVersionForUserQuery) : IClientVersionValidator { @@ -37,7 +38,7 @@ public async Task ValidateAsync(User? user, CustomValidatorRequestContext { requestContext.ValidationErrorResult = new ValidationResult { - Error = "invalid_grant", + Error = "invalid_client_version", ErrorDescription = UpgradeMessage, IsError = true }; diff --git a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs index e7fc4b6498c2..4a5befc42bdf 100644 --- a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs @@ -16,11 +16,8 @@ using Duende.IdentityModel; using Duende.IdentityServer.Extensions; using Duende.IdentityServer.Validation; -using HandlebarsDotNet; using Microsoft.AspNetCore.Identity; -#nullable enable - namespace Bit.Identity.IdentityServer.RequestValidators; public class CustomTokenRequestValidator : BaseRequestValidator, From a82b31c65f4d4fec7351d2d49dfc8bdd822c7456 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 20 Nov 2025 13:28:33 -0500 Subject: [PATCH 04/40] fix(auth-validator): [PM-22975] Client Version Validator - Fixing some white spaces as well as the renaming of a file --- .../GetMinimumClientVersionForUserQuery.cs | 2 - .../IGetMinimumClientVersionForUserQuery.cs | 2 - .../Interfaces/IIsV2EncryptionUserQuery.cs | 2 - .../Queries/IsV2EncryptionUserQuery.cs | 2 - .../RotateUserAccountkeysCommand.cs | 265 ------------------ ...serAccountKeysCommand.cs => tmp_Rotate.cs} | 0 .../Utilities/EncryptionParsing.cs | 2 - .../RequestValidators/BaseRequestValidator.cs | 7 +- .../Queries/IsV2EncryptionUserQueryTests.cs | 2 - .../Login/ClientVersionGateTests.cs | 2 - .../ClientVersionValidatorTests.cs | 2 - 11 files changed, 4 insertions(+), 284 deletions(-) delete mode 100644 src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs rename src/Core/KeyManagement/UserKey/Implementations/{RotateUserAccountKeysCommand.cs => tmp_Rotate.cs} (100%) diff --git a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs index 49cc46a54c41..fe6020ab5a91 100644 --- a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs +++ b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs @@ -21,5 +21,3 @@ public class GetMinimumClientVersionForUserQuery(IIsV2EncryptionUserQuery isV2En return null; } } - - diff --git a/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs index 896a473d6942..01deb460f198 100644 --- a/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs +++ b/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs @@ -6,5 +6,3 @@ public interface IGetMinimumClientVersionForUserQuery { Task Run(User? user); } - - diff --git a/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs b/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs index 6b644a2dc7d3..38c0e10b4404 100644 --- a/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs +++ b/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs @@ -6,5 +6,3 @@ public interface IIsV2EncryptionUserQuery { Task Run(User user); } - - diff --git a/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs b/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs index 74a203004c1f..ea64d5a20aa5 100644 --- a/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs +++ b/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs @@ -29,5 +29,3 @@ public async Task Run(User user) }; } } - - diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs deleted file mode 100644 index 6e5708f667d8..000000000000 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs +++ /dev/null @@ -1,265 +0,0 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Auth.Repositories; -using Bit.Core.Entities; -using Bit.Core.Enums; -using Bit.Core.KeyManagement.Models.Data; -using Bit.Core.KeyManagement.Repositories; -using Bit.Core.KeyManagement.Utilities; -using Bit.Core.Platform.Push; -using Bit.Core.Repositories; -using Bit.Core.Services; -using Bit.Core.Tools.Repositories; -using Bit.Core.Vault.Repositories; -using Microsoft.AspNetCore.Identity; - -namespace Bit.Core.KeyManagement.UserKey.Implementations; - -/// -public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand -{ - private readonly IUserService _userService; - private readonly IUserRepository _userRepository; - private readonly ICipherRepository _cipherRepository; - private readonly IFolderRepository _folderRepository; - private readonly ISendRepository _sendRepository; - private readonly IEmergencyAccessRepository _emergencyAccessRepository; - private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IDeviceRepository _deviceRepository; - private readonly IPushNotificationService _pushService; - private readonly IdentityErrorDescriber _identityErrorDescriber; - private readonly IWebAuthnCredentialRepository _credentialRepository; - private readonly IPasswordHasher _passwordHasher; - private readonly IUserSignatureKeyPairRepository _userSignatureKeyPairRepository; - private readonly IFeatureService _featureService; - - /// - /// Instantiates a new - /// - /// Master password hash validation - /// Updates user keys and re-encrypted data if needed - /// Provides a method to update re-encrypted cipher data - /// Provides a method to update re-encrypted folder data - /// Provides a method to update re-encrypted send data - /// Provides a method to update re-encrypted emergency access data - /// Provides a method to update re-encrypted organization user data - /// Provides a method to update re-encrypted device keys - /// Hashes the new master password - /// Logs out user from other devices after successful rotation - /// Provides a password mismatch error if master password hash validation fails - /// Provides a method to update re-encrypted WebAuthn keys - /// Provides a method to update re-encrypted signature keys - public RotateUserAccountKeysCommand(IUserService userService, IUserRepository userRepository, - ICipherRepository cipherRepository, IFolderRepository folderRepository, ISendRepository sendRepository, - IEmergencyAccessRepository emergencyAccessRepository, IOrganizationUserRepository organizationUserRepository, - IDeviceRepository deviceRepository, - IPasswordHasher passwordHasher, - IPushNotificationService pushService, IdentityErrorDescriber errors, IWebAuthnCredentialRepository credentialRepository, - IUserSignatureKeyPairRepository userSignatureKeyPairRepository, - IFeatureService featureService) - { - _userService = userService; - _userRepository = userRepository; - _cipherRepository = cipherRepository; - _folderRepository = folderRepository; - _sendRepository = sendRepository; - _emergencyAccessRepository = emergencyAccessRepository; - _organizationUserRepository = organizationUserRepository; - _deviceRepository = deviceRepository; - _pushService = pushService; - _identityErrorDescriber = errors; - _credentialRepository = credentialRepository; - _passwordHasher = passwordHasher; - _userSignatureKeyPairRepository = userSignatureKeyPairRepository; - _featureService = featureService; - } - - /// - public async Task RotateUserAccountKeysAsync(User user, RotateUserAccountKeysData model) - { - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } - - if (!await _userService.CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash)) - { - return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); - } - - var now = DateTime.UtcNow; - user.RevisionDate = user.AccountRevisionDate = now; - user.LastKeyRotationDate = now; - user.SecurityStamp = Guid.NewGuid().ToString(); - - List saveEncryptedDataActions = []; - - await UpdateAccountKeysAsync(model, user, saveEncryptedDataActions); - UpdateUnlockMethods(model, user, saveEncryptedDataActions); - UpdateUserData(model, user, saveEncryptedDataActions); - - await _userRepository.UpdateUserKeyAndEncryptedDataV2Async(user, saveEncryptedDataActions); - await _pushService.PushLogOutAsync(user.Id); - return IdentityResult.Success; - } - - public async Task RotateV2AccountKeysAsync(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) - { - ValidateV2Encryption(model); - await ValidateVerifyingKeyUnchangedAsync(model, user); - - saveEncryptedDataActions.Add(_userSignatureKeyPairRepository.UpdateForKeyRotation(user.Id, model.AccountKeys.SignatureKeyPairData)); - user.SignedPublicKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey; - user.SecurityState = model.AccountKeys.SecurityStateData!.SecurityState; - user.SecurityVersion = model.AccountKeys.SecurityStateData.SecurityVersion; - } - - public void UpgradeV1ToV2Keys(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) - { - ValidateV2Encryption(model); - saveEncryptedDataActions.Add(_userSignatureKeyPairRepository.SetUserSignatureKeyPair(user.Id, model.AccountKeys.SignatureKeyPairData)); - user.SignedPublicKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey; - user.SecurityState = model.AccountKeys.SecurityStateData!.SecurityState; - user.SecurityVersion = model.AccountKeys.SecurityStateData.SecurityVersion; - } - - public async Task UpdateAccountKeysAsync(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) - { - ValidatePublicKeyEncryptionKeyPairUnchanged(model, user); - - if (IsV2EncryptionUserAsync(user)) - { - await RotateV2AccountKeysAsync(model, user, saveEncryptedDataActions); - } - else if (model.AccountKeys.SignatureKeyPairData != null) - { - UpgradeV1ToV2Keys(model, user, saveEncryptedDataActions); - } - else - { - if (EncryptionParsing.GetEncryptionType(model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey) != EncryptionType.AesCbc256_HmacSha256_B64) - { - throw new InvalidOperationException("The provided account private key was not wrapped with AES-256-CBC-HMAC"); - } - // V1 user to V1 user rotation needs to further changes, the private key was re-encrypted. - } - - // Private key is re-wrapped with new user key by client - user.PrivateKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey; - } - - public void UpdateUserData(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) - { - // The revision date has to be updated so that de-synced clients don't accidentally post over the re-encrypted data - // with an old-user key-encrypted copy - var now = DateTime.UtcNow; - - if (model.Ciphers.Any()) - { - var ciphersWithUpdatedDate = model.Ciphers.ToList().Select(c => { c.RevisionDate = now; return c; }); - saveEncryptedDataActions.Add(_cipherRepository.UpdateForKeyRotation(user.Id, ciphersWithUpdatedDate)); - } - - if (model.Folders.Any()) - { - var foldersWithUpdatedDate = model.Folders.ToList().Select(f => { f.RevisionDate = now; return f; }); - saveEncryptedDataActions.Add(_folderRepository.UpdateForKeyRotation(user.Id, foldersWithUpdatedDate)); - } - - if (model.Sends.Any()) - { - var sendsWithUpdatedDate = model.Sends.ToList().Select(s => { s.RevisionDate = now; return s; }); - saveEncryptedDataActions.Add(_sendRepository.UpdateForKeyRotation(user.Id, sendsWithUpdatedDate)); - } - } - - void UpdateUnlockMethods(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) - { - if (!model.MasterPasswordUnlockData.ValidateForUser(user)) - { - throw new InvalidOperationException("The provided master password unlock data is not valid for this user."); - } - // Update master password authentication & unlock - user.Key = model.MasterPasswordUnlockData.MasterKeyEncryptedUserKey; - user.MasterPassword = _passwordHasher.HashPassword(user, model.MasterPasswordUnlockData.MasterKeyAuthenticationHash); - user.MasterPasswordHint = model.MasterPasswordUnlockData.MasterPasswordHint; - - if (model.EmergencyAccesses.Any()) - { - saveEncryptedDataActions.Add(_emergencyAccessRepository.UpdateForKeyRotation(user.Id, model.EmergencyAccesses)); - } - - if (model.OrganizationUsers.Any()) - { - saveEncryptedDataActions.Add(_organizationUserRepository.UpdateForKeyRotation(user.Id, model.OrganizationUsers)); - } - - if (model.WebAuthnKeys.Any()) - { - saveEncryptedDataActions.Add(_credentialRepository.UpdateKeysForRotationAsync(user.Id, model.WebAuthnKeys)); - } - - if (model.DeviceKeys.Any()) - { - saveEncryptedDataActions.Add(_deviceRepository.UpdateKeysForRotationAsync(user.Id, model.DeviceKeys)); - } - } - - private bool IsV2EncryptionUserAsync(User user) - { - // Returns whether the user is a V2 user based on the private key's encryption type. - ArgumentNullException.ThrowIfNull(user); - var isPrivateKeyEncryptionV2 = EncryptionParsing.GetEncryptionType(user.PrivateKey) == EncryptionType.XChaCha20Poly1305_B64; - return isPrivateKeyEncryptionV2; - } - - private async Task ValidateVerifyingKeyUnchangedAsync(RotateUserAccountKeysData model, User user) - { - var currentSignatureKeyPair = await _userSignatureKeyPairRepository.GetByUserIdAsync(user.Id) ?? throw new InvalidOperationException("User does not have a signature key pair."); - if (model.AccountKeys.SignatureKeyPairData.VerifyingKey != currentSignatureKeyPair!.VerifyingKey) - { - throw new InvalidOperationException("The provided verifying key does not match the user's current verifying key."); - } - } - - private static void ValidatePublicKeyEncryptionKeyPairUnchanged(RotateUserAccountKeysData model, User user) - { - var publicKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.PublicKey; - if (publicKey != user.PublicKey) - { - throw new InvalidOperationException("The provided account public key does not match the user's current public key, and changing the account asymmetric key pair is currently not supported during key rotation."); - } - } - - private static void ValidateV2Encryption(RotateUserAccountKeysData model) - { - if (model.AccountKeys.SignatureKeyPairData == null) - { - throw new InvalidOperationException("Signature key pair data is required for V2 encryption."); - } - if (EncryptionParsing.GetEncryptionType(model.AccountKeys.SignatureKeyPairData.WrappedSigningKey) != EncryptionType.XChaCha20Poly1305_B64) - { - throw new InvalidOperationException("The provided signing key data is not wrapped with XChaCha20-Poly1305."); - } - if (string.IsNullOrEmpty(model.AccountKeys.SignatureKeyPairData.VerifyingKey)) - { - throw new InvalidOperationException("The provided signature key pair data does not contain a valid verifying key."); - } - - if (EncryptionParsing.GetEncryptionType(model.AccountKeys.PublicKeyEncryptionKeyPairData.WrappedPrivateKey) != EncryptionType.XChaCha20Poly1305_B64) - { - throw new InvalidOperationException("The provided private key encryption key is not wrapped with XChaCha20-Poly1305."); - } - if (string.IsNullOrEmpty(model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey)) - { - throw new InvalidOperationException("No signed public key provided, but the user already has a signature key pair."); - } - if (model.AccountKeys.SecurityStateData == null || string.IsNullOrEmpty(model.AccountKeys.SecurityStateData.SecurityState)) - { - throw new InvalidOperationException("No signed security state provider for V2 user"); - } - } - - // Parsing moved to Bit.Core.KeyManagement.Utilities.EncryptionParsing -} diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/tmp_Rotate.cs similarity index 100% rename from src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs rename to src/Core/KeyManagement/UserKey/Implementations/tmp_Rotate.cs diff --git a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs index f288cfd99a37..269ff5122828 100644 --- a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs +++ b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs @@ -25,5 +25,3 @@ public static EncryptionType GetEncryptionType(string encString) throw new ArgumentException("Invalid encryption type string."); } } - - diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index ae6aaed2bda9..54e432de84bb 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -111,7 +111,7 @@ await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.D } else { - // 1. We need to check if the user's master password hash is correct. + // 1. We need to check if the user is legitimate via the appropriate mechanism through. var valid = await ValidateContextAsync(context, validatorContext); var user = validatorContext.User; if (!valid) @@ -122,10 +122,11 @@ await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.D return; } - // 1.5 We need to check now the version number + // 1.5 Now check the version number of the client. Do this after ValidateContextAsync so that + // we prevent account enumeration. If we were to do this before we would validate that a given user + // could exist await ValidateClientVersionAsync(context, validatorContext); - // 2. Decide if this user belongs to an organization that requires SSO. validatorContext.SsoRequired = await RequireSsoLoginAsync(user, request.GrantType); if (validatorContext.SsoRequired) diff --git a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs index 67c3601b81a9..a3e91bb6efec 100644 --- a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs +++ b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs @@ -61,5 +61,3 @@ public async Task Run_ThrowsForInvalidMixedState() await Assert.ThrowsAsync(async () => await sut.Run(user)); } } - - diff --git a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs index 54f005688e98..642148b685d1 100644 --- a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs +++ b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs @@ -115,5 +115,3 @@ private void ReinitializeDbForTests(IdentityApplicationFactory factory) databaseContext.SaveChanges(); } } - - diff --git a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs index dc491596fe59..45fd26169a2e 100644 --- a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs @@ -51,5 +51,3 @@ public async Task Allows_When_ClientMeetsMin() Assert.True(ok); } } - - From 7d71ee2eec3d35940bdbe463dd90fb69dca47950 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 20 Nov 2025 13:29:50 -0500 Subject: [PATCH 05/40] fix(auth-validator): [PM-22975] Client Version Validator - Finished fixing the rename to the correct file. --- .../{tmp_Rotate.cs => RotateUserAccountKeysCommand.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/Core/KeyManagement/UserKey/Implementations/{tmp_Rotate.cs => RotateUserAccountKeysCommand.cs} (100%) diff --git a/src/Core/KeyManagement/UserKey/Implementations/tmp_Rotate.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs similarity index 100% rename from src/Core/KeyManagement/UserKey/Implementations/tmp_Rotate.cs rename to src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs From 851f963be64f35a80b5af2161e1571f108a96b92 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 20 Nov 2025 13:54:14 -0500 Subject: [PATCH 06/40] test(auth-validator): [PM-22975] Client Version Validator - Fixed tests a little. --- src/Core/KeyManagement/Utilities/EncryptionParsing.cs | 1 - .../RequestValidators/BaseRequestValidator.cs | 9 ++++++--- .../IdentityServer/BaseRequestValidatorTests.cs | 5 ++++- .../RequestValidators/ClientVersionValidatorTests.cs | 2 +- .../Wrappers/BaseRequestValidatorTestWrapper.cs | 4 ++-- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs index 269ff5122828..ffe8cb3134fb 100644 --- a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs +++ b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs @@ -6,7 +6,6 @@ public static class EncryptionParsing { /// /// Helper method to convert an encryption type string to an enum value. - /// Accepts formats like "Header.iv|ct|mac" or "Header" COSE format. /// public static EncryptionType GetEncryptionType(string encString) { diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 54e432de84bb..b1bff766a40b 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -111,7 +111,8 @@ await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.D } else { - // 1. We need to check if the user is legitimate via the appropriate mechanism through. + // 1. We need to check if the user is legitimate via the contextually appropriate mechanism + // (webauthn, password, custom token, etc.). var valid = await ValidateContextAsync(context, validatorContext); var user = validatorContext.User; if (!valid) @@ -123,8 +124,10 @@ await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.D } // 1.5 Now check the version number of the client. Do this after ValidateContextAsync so that - // we prevent account enumeration. If we were to do this before we would validate that a given user - // could exist + // we prevent account enumeration. If we were to do this before ValidateContextAsync, then attackers + // could use a known invalid client version and make a request for a user (before we know if they have + // demonstrated ownership of the account via correct credentials) and identify if they exist by getting + // an error response back from the validator saying the user is not compatible with the client. await ValidateClientVersionAsync(context, validatorContext); // 2. Decide if this user belongs to an organization that requires SSO. diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index e78c7d161c09..9a3d4dd71131 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -55,6 +55,7 @@ public class BaseRequestValidatorTests private readonly IAuthRequestRepository _authRequestRepository; private readonly IMailService _mailService; private readonly IUserAccountKeysQuery _userAccountKeysQuery; + private readonly IClientVersionValidator _clientVersionValidator; private readonly BaseRequestValidatorTestWrapper _sut; @@ -78,6 +79,7 @@ public BaseRequestValidatorTests() _authRequestRepository = Substitute.For(); _mailService = Substitute.For(); _userAccountKeysQuery = Substitute.For(); + _clientVersionValidator = Substitute.For(); _sut = new BaseRequestValidatorTestWrapper( _userManager, @@ -97,7 +99,8 @@ public BaseRequestValidatorTests() _policyRequirementQuery, _authRequestRepository, _mailService, - _userAccountKeysQuery); + _userAccountKeysQuery, + _clientVersionValidator); } private void SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(bool recoveryCodeSupportEnabled) diff --git a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs index 45fd26169a2e..65a99042464d 100644 --- a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs @@ -40,7 +40,7 @@ public async Task Blocks_When_ClientTooOld() Assert.False(ok); Assert.NotNull(ctx.ValidationErrorResult); Assert.True(ctx.ValidationErrorResult.IsError); - Assert.Equal("invalid_grant", ctx.ValidationErrorResult.Error); + Assert.Equal("invalid_client_version", ctx.ValidationErrorResult.Error); } [Fact] diff --git a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs index dc6d80a730e6..bd8fcd4bda1e 100644 --- a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs +++ b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs @@ -66,8 +66,8 @@ public BaseRequestValidatorTestWrapper( IPolicyRequirementQuery policyRequirementQuery, IAuthRequestRepository authRequestRepository, IMailService mailService, - IClientVersionValidator clientVersionValidator, - IUserAccountKeysQuery userAccountKeysQuery) : + IUserAccountKeysQuery userAccountKeysQuery, + IClientVersionValidator clientVersionValidator) : base( userManager, userService, From 756ae5e8910bf4e234e00f7f56a1c4ae7a53b9cf Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 20 Nov 2025 14:07:05 -0500 Subject: [PATCH 07/40] fix(auth-validator): [PM-22975] Client Version Validator - Fixed naming a little to be more clear. --- .../RequestValidators/BaseRequestValidator.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index b1bff766a40b..0b010be38a15 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -269,7 +269,7 @@ private Func>[] DetermineValidationOrder(T context, ValidatedTokenReq // validation to perform the recovery as part of scheme validation based on the request. return [ - () => ValidateMasterPasswordAsync(context, validatorContext), + () => ValidateUserViaAppropriateValidatorContextAsync(context, validatorContext), () => ValidateClientVersionAsync(context, validatorContext), () => ValidateTwoFactorAsync(context, request, validatorContext), () => ValidateSsoAsync(context, request, validatorContext), @@ -283,7 +283,7 @@ private Func>[] DetermineValidationOrder(T context, ValidatedTokenReq // The typical validation scenario. return [ - () => ValidateMasterPasswordAsync(context, validatorContext), + () => ValidateUserViaAppropriateValidatorContextAsync(context, validatorContext), () => ValidateClientVersionAsync(context, validatorContext), () => ValidateSsoAsync(context, request, validatorContext), () => ValidateTwoFactorAsync(context, request, validatorContext), @@ -355,12 +355,12 @@ private async Task ValidateClientVersionAsync(T context, CustomValidatorRe } /// - /// Validates the user's Master Password hash. + /// Validates the user's master password, webauthen, or custom token request via the appropriate context validator. /// /// The current request context. /// /// true if the scheme successfully passed validation, otherwise false. - private async Task ValidateMasterPasswordAsync(T context, CustomValidatorRequestContext validatorContext) + private async Task ValidateUserViaAppropriateValidatorContextAsync(T context, CustomValidatorRequestContext validatorContext) { var valid = await ValidateContextAsync(context, validatorContext); var user = validatorContext.User; From 91af02b9d28381430b1d617b94498a9e7d8d3ff1 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 20 Nov 2025 14:54:33 -0500 Subject: [PATCH 08/40] test(auth-validator): [PM-22975] Client Version Validator - Fixed tests. --- .../RequestValidators/BaseRequestValidator.cs | 6 ++- .../BaseRequestValidatorTests.cs | 40 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 0b010be38a15..92258c9289de 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -128,7 +128,11 @@ await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.D // could use a known invalid client version and make a request for a user (before we know if they have // demonstrated ownership of the account via correct credentials) and identify if they exist by getting // an error response back from the validator saying the user is not compatible with the client. - await ValidateClientVersionAsync(context, validatorContext); + var clientVersionValid = await ValidateClientVersionAsync(context, validatorContext); + if (!clientVersionValid) + { + return; + } // 2. Decide if this user belongs to an organization that requires SSO. validatorContext.SsoRequired = await RequireSsoLoginAsync(user, request.GrantType); diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 9a3d4dd71131..6ef2df23013e 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -101,6 +101,11 @@ public BaseRequestValidatorTests() _mailService, _userAccountKeysQuery, _clientVersionValidator); + + // Default client version validator behavior: allow to pass unless a test overrides. + _clientVersionValidator + .ValidateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); } private void SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(bool recoveryCodeSupportEnabled) @@ -1246,6 +1251,41 @@ await _userRepository.Received(1).ReplaceAsync(Arg.Is(u => } } + [Theory] + [BitAutoData(true)] + [BitAutoData(false)] + public async Task ValidateAsync_ClientVersionValidator_IsInvoked_ForFeatureFlagStates( + bool featureFlagValue, + [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, + [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + GrantValidationResult grantResult) + { + // Arrange + SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); + var context = CreateContext(tokenRequest, requestContext, grantResult); + _sut.isValid = true; // ensure initial context validation passes + + // Force a grant type that will evaluate SSO after client version validation + context.ValidatedTokenRequest.GrantType = "password"; + + // Make client version validation succeed but ensure it's invoked + _clientVersionValidator + .ValidateAsync(requestContext.User, requestContext) + .Returns(Task.FromResult(true)); + + // Ensure SSO requirement triggers an early stop after version validation to avoid success path setup + _policyService.AnyPoliciesApplicableToUserAsync( + Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) + .Returns(Task.FromResult(true)); + + // Act + await _sut.ValidateAsync(context); + + // Assert + await _clientVersionValidator.Received(1) + .ValidateAsync(requestContext.User, requestContext); + } + private BaseRequestValidationContextFake CreateContext( ValidatedTokenRequest tokenRequest, CustomValidatorRequestContext requestContext, From 789748555ae166254a15b156a80f420ea71a64b1 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Fri, 21 Nov 2025 15:09:28 -0500 Subject: [PATCH 09/40] fix(auth-validator): [PM-22975] Client Version Validator - Renamed validator to make more sense. --- .../RequestValidators/BaseRequestValidator.cs | 6 +++--- .../RequestValidators/ResourceOwnerPasswordValidator.cs | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 92258c9289de..3a49c0fdb8c5 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -273,7 +273,7 @@ private Func>[] DetermineValidationOrder(T context, ValidatedTokenReq // validation to perform the recovery as part of scheme validation based on the request. return [ - () => ValidateUserViaAppropriateValidatorContextAsync(context, validatorContext), + () => ValidateGrantSpecificContext(context, validatorContext), () => ValidateClientVersionAsync(context, validatorContext), () => ValidateTwoFactorAsync(context, request, validatorContext), () => ValidateSsoAsync(context, request, validatorContext), @@ -287,7 +287,7 @@ private Func>[] DetermineValidationOrder(T context, ValidatedTokenReq // The typical validation scenario. return [ - () => ValidateUserViaAppropriateValidatorContextAsync(context, validatorContext), + () => ValidateGrantSpecificContext(context, validatorContext), () => ValidateClientVersionAsync(context, validatorContext), () => ValidateSsoAsync(context, request, validatorContext), () => ValidateTwoFactorAsync(context, request, validatorContext), @@ -364,7 +364,7 @@ private async Task ValidateClientVersionAsync(T context, CustomValidatorRe /// The current request context. /// /// true if the scheme successfully passed validation, otherwise false. - private async Task ValidateUserViaAppropriateValidatorContextAsync(T context, CustomValidatorRequestContext validatorContext) + private async Task ValidateGrantSpecificContext(T context, CustomValidatorRequestContext validatorContext) { var valid = await ValidateContextAsync(context, validatorContext); var user = validatorContext.User; diff --git a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs index a9952c26f250..c8e9b13c8ce4 100644 --- a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs @@ -74,7 +74,6 @@ public ResourceOwnerPasswordValidator( public async Task ValidateAsync(ResourceOwnerPasswordValidationContext context) { - var user = await _userManager.FindByEmailAsync(context.UserName.ToLowerInvariant()); // We want to keep this device around incase the device is new for the user var requestDevice = DeviceValidator.GetDeviceFromRequest(context.Request); From d69842d668bea573a3643537ef2c5bf4386f8dcf Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 24 Nov 2025 12:59:03 -0500 Subject: [PATCH 10/40] fix(auth-validator): [PM-22975] Client Version Validator - Added header to all tests within the webapplication factory extensions. --- .../Factories/WebApplicationFactoryExtensions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs b/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs index 3f5bf49dd9d7..37c0cb2e980b 100644 --- a/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs +++ b/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs @@ -36,6 +36,8 @@ private static async Task SendAsync(this TestServer server, httpContext.Request.Body = content.ReadAsStream(); } + httpContext.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); + extraConfiguration?.Invoke(httpContext); }); } From 1681703eea0c5a28e4f93e08f7acaaba7b2f3ea5 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 24 Nov 2025 15:10:46 -0500 Subject: [PATCH 11/40] fix(auth-validator): [PM-22975] Client Version Validator - Trying to fix the error occuring in the build process. --- test/Common/Helpers/AssertHelper.cs | 32 ++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/test/Common/Helpers/AssertHelper.cs b/test/Common/Helpers/AssertHelper.cs index 5e9c3a5aba53..1a9ad8ee1628 100644 --- a/test/Common/Helpers/AssertHelper.cs +++ b/test/Common/Helpers/AssertHelper.cs @@ -225,7 +225,37 @@ private static void AssertEqualJsonArray(JsonElement a, JsonElement b) public async static Task AssertResponseTypeIs(HttpContext context) { - return await JsonSerializer.DeserializeAsync(context.Response.Body); + try + { + if (context.Response.Body.CanSeek) + { + context.Response.Body.Position = 0; + } + + return await JsonSerializer.DeserializeAsync(context.Response.Body); + } + catch (JsonException ex) + { + string bodyText = ""; + try + { + if (context.Response.Body.CanSeek) + { + context.Response.Body.Position = 0; + } + + using var sr = new StreamReader(context.Response.Body, leaveOpen: true); + bodyText = await sr.ReadToEndAsync(); + } + catch + { + // ignore read errors + } + + throw new Xunit.Sdk.XunitException( + $"Failed to deserialize response to {typeof(T).Name}. " + + $"StatusCode: {context.Response.StatusCode}. Body:\n{bodyText}\nException: {ex}"); + } } public static TimeSpan AssertRecent(DateTime dateTime, int skewSeconds = 2) From cb146fcbc8649691f7fabe2cfb3265f224e000fc Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 24 Nov 2025 15:42:54 -0500 Subject: [PATCH 12/40] fix(auth-validator): [PM-22975] Client Version Validator - Fixed tests --- .../Factories/ApiApplicationFactory.cs | 6 ++-- test/Common/Helpers/AssertHelper.cs | 32 +------------------ .../WebApplicationFactoryExtensions.cs | 2 -- 3 files changed, 4 insertions(+), 36 deletions(-) diff --git a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs index 173580ad8c5e..08bd60253700 100644 --- a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs +++ b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs @@ -66,10 +66,10 @@ await _identityApplicationFactory.RegisterNewIdentityFactoryUserAsync( KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, UserAsymmetricKeys = new KeysRequestModel() { - PublicKey = "public_key", - EncryptedPrivateKey = "private_key" + PublicKey = "pk_test", + EncryptedPrivateKey = "2.iv|ct|mac" // v1-format so parsing succeeds and user is treated as v1 }, - UserSymmetricKey = "sym_key", + UserSymmetricKey = "2.iv|ct|mac", }); return await _identityApplicationFactory.TokenFromPasswordAsync(email, masterPasswordHash); diff --git a/test/Common/Helpers/AssertHelper.cs b/test/Common/Helpers/AssertHelper.cs index 1a9ad8ee1628..5e9c3a5aba53 100644 --- a/test/Common/Helpers/AssertHelper.cs +++ b/test/Common/Helpers/AssertHelper.cs @@ -225,37 +225,7 @@ private static void AssertEqualJsonArray(JsonElement a, JsonElement b) public async static Task AssertResponseTypeIs(HttpContext context) { - try - { - if (context.Response.Body.CanSeek) - { - context.Response.Body.Position = 0; - } - - return await JsonSerializer.DeserializeAsync(context.Response.Body); - } - catch (JsonException ex) - { - string bodyText = ""; - try - { - if (context.Response.Body.CanSeek) - { - context.Response.Body.Position = 0; - } - - using var sr = new StreamReader(context.Response.Body, leaveOpen: true); - bodyText = await sr.ReadToEndAsync(); - } - catch - { - // ignore read errors - } - - throw new Xunit.Sdk.XunitException( - $"Failed to deserialize response to {typeof(T).Name}. " + - $"StatusCode: {context.Response.StatusCode}. Body:\n{bodyText}\nException: {ex}"); - } + return await JsonSerializer.DeserializeAsync(context.Response.Body); } public static TimeSpan AssertRecent(DateTime dateTime, int skewSeconds = 2) diff --git a/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs b/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs index 37c0cb2e980b..3f5bf49dd9d7 100644 --- a/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs +++ b/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs @@ -36,8 +36,6 @@ private static async Task SendAsync(this TestServer server, httpContext.Request.Body = content.ReadAsStream(); } - httpContext.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); - extraConfiguration?.Invoke(httpContext); }); } From b3b1b9b91dc15bb4958fb4676a12db49f6915b13 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 1 Dec 2025 17:49:09 -0500 Subject: [PATCH 13/40] fix(auth-validator): [PM-22975] Client Version Validator - misc changes, trying to get things to work --- .../Factories/ApiApplicationFactory.cs | 7 +- .../Constants/TestEncryptionConstants.cs | 23 +++++ .../AutoFixture/GlobalSettingsFixtures.cs | 5 +- .../Queries/IsV2EncryptionUserQueryTests.cs | 9 +- .../Endpoints/IdentityServerSsoTests.cs | 96 ++++++++++++++++--- .../WebApplicationFactoryExtensions.cs | 3 + 6 files changed, 119 insertions(+), 24 deletions(-) create mode 100644 test/Common/Constants/TestEncryptionConstants.cs diff --git a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs index 08bd60253700..93ebacf726a4 100644 --- a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs +++ b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs @@ -3,6 +3,7 @@ using Bit.Core.Enums; using Bit.IntegrationTestCommon; using Bit.IntegrationTestCommon.Factories; +using Bit.Test.Common.Constants; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.TestHost; using Xunit; @@ -66,10 +67,10 @@ await _identityApplicationFactory.RegisterNewIdentityFactoryUserAsync( KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, UserAsymmetricKeys = new KeysRequestModel() { - PublicKey = "pk_test", - EncryptedPrivateKey = "2.iv|ct|mac" // v1-format so parsing succeeds and user is treated as v1 + PublicKey = TestEncryptionConstants.PublicKey, + EncryptedPrivateKey = TestEncryptionConstants.V1EncryptedBase64 // v1-format so parsing succeeds and user is treated as v1 }, - UserSymmetricKey = "2.iv|ct|mac", + UserSymmetricKey = TestEncryptionConstants.V1EncryptedBase64, }); return await _identityApplicationFactory.TokenFromPasswordAsync(email, masterPasswordHash); diff --git a/test/Common/Constants/TestEncryptionConstants.cs b/test/Common/Constants/TestEncryptionConstants.cs new file mode 100644 index 000000000000..40ce984f237d --- /dev/null +++ b/test/Common/Constants/TestEncryptionConstants.cs @@ -0,0 +1,23 @@ +namespace Bit.Test.Common.Constants; + +public static class TestEncryptionConstants +{ + // V1-style encrypted strings (AES-CBC-HMAC formats) accepted by validators + public const string V1EncryptedBase64 = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; + + // Public key test placeholder + public const string PublicKey = "pk_test"; + + // V2-style values used across tests + // Private key indicating v2 (used in multiple tests to mark v2 state) + public const string V2PrivateKey = "7.cose"; + // Wrapped signing key and verifying key values from real tests + public const string V2WrappedSigningKey = "test-wrapped-signing-key"; + public const string V2VerifyingKey = "test-verifying-key"; + // Additional related v2 values used in tests + public const string V2PublicKey = "test-public-key"; + public const string V2WrappedPrivateKey = "test-private-key"; + public const string V2SignedPublicKey = "test-signed-public-key"; +} + + diff --git a/test/Core.Test/AutoFixture/GlobalSettingsFixtures.cs b/test/Core.Test/AutoFixture/GlobalSettingsFixtures.cs index 020b097077e2..e3d36fdc7152 100644 --- a/test/Core.Test/AutoFixture/GlobalSettingsFixtures.cs +++ b/test/Core.Test/AutoFixture/GlobalSettingsFixtures.cs @@ -3,7 +3,6 @@ using AutoFixture; using AutoFixture.Kernel; using AutoFixture.Xunit2; -using Bit.Core; using Bit.Core.Test.Helpers.Factories; using Microsoft.AspNetCore.DataProtection; using NSubstitute; @@ -36,11 +35,11 @@ public object Create(object request, ISpecimenContext context) var dataProtector = Substitute.For(); dataProtector.Unprotect(Arg.Any()) .Returns(data => - Encoding.UTF8.GetBytes(Constants.DatabaseFieldProtectedPrefix + + Encoding.UTF8.GetBytes(Core.Constants.DatabaseFieldProtectedPrefix + Encoding.UTF8.GetString((byte[])data[0]))); var dataProtectionProvider = Substitute.For(); - dataProtectionProvider.CreateProtector(Constants.DatabaseFieldProtectorPurpose) + dataProtectionProvider.CreateProtector(Core.Constants.DatabaseFieldProtectorPurpose) .Returns(dataProtector); return dataProtectionProvider; diff --git a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs index a3e91bb6efec..8dc74100331e 100644 --- a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs +++ b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs @@ -5,6 +5,7 @@ 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; @@ -16,7 +17,7 @@ private class FakeSigRepo : IUserSignatureKeyPairRepository private readonly bool _hasKeys; public FakeSigRepo(bool hasKeys) { _hasKeys = hasKeys; } public Task GetByUserIdAsync(Guid userId) - => Task.FromResult(_hasKeys ? new SignatureKeyPairData(SignatureAlgorithm.Ed25519, "7.cose_signing", "vk") : null); + => Task.FromResult(_hasKeys ? new SignatureKeyPairData(SignatureAlgorithm.Ed25519, TestEncryptionConstants.V2WrappedSigningKey, TestEncryptionConstants.V2VerifyingKey) : null); // Unused in tests public Task> GetManyAsync(IEnumerable ids) => throw new NotImplementedException(); @@ -33,7 +34,7 @@ private class FakeSigRepo : IUserSignatureKeyPairRepository [Fact] public async Task Run_ReturnsTrue_ForV2State() { - var user = new User { Id = Guid.NewGuid(), PrivateKey = "7.cose" }; + var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V2PrivateKey }; var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(true)); var result = await sut.Run(user); @@ -44,7 +45,7 @@ public async Task Run_ReturnsTrue_ForV2State() [Fact] public async Task Run_ReturnsFalse_ForV1State() { - var user = new User { Id = Guid.NewGuid(), PrivateKey = "2.iv|ct|mac" }; + var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V1EncryptedBase64 }; var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(false)); var result = await sut.Run(user); @@ -55,7 +56,7 @@ public async Task Run_ReturnsFalse_ForV1State() [Fact] public async Task Run_ThrowsForInvalidMixedState() { - var user = new User { Id = Guid.NewGuid(), PrivateKey = "7.cose" }; + var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V2PrivateKey }; var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(false)); await Assert.ThrowsAsync(async () => await sut.Run(user)); diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs index 920d3b0ad320..22274288245f 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs @@ -15,7 +15,10 @@ using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Utilities; +using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidators; using Bit.IntegrationTestCommon.Factories; +using Bit.Test.Common.Constants; using Bit.Test.Common.Helpers; using Duende.IdentityModel; using Duende.IdentityServer.Models; @@ -310,8 +313,8 @@ public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_DeviceAlre var user = await factory.Services.GetRequiredService().GetByEmailAsync(TestEmail); Assert.NotNull(user); - const string expectedPrivateKey = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; - const string expectedUserKey = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; + const string expectedPrivateKey = TestEncryptionConstants.V1EncryptedBase64; + const string expectedUserKey = TestEncryptionConstants.V1EncryptedBase64; var device = await deviceRepository.CreateAsync(new Device { @@ -320,7 +323,7 @@ public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_DeviceAlre Name = "Thing", UserId = user.Id, EncryptedPrivateKey = expectedPrivateKey, - EncryptedPublicKey = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", + EncryptedPublicKey = TestEncryptionConstants.V1EncryptedBase64, EncryptedUserKey = expectedUserKey, }); @@ -339,7 +342,8 @@ public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_DeviceAlre { "code", "test_code" }, { "code_verifier", challenge }, { "redirect_uri", "https://localhost:8080/sso-connector.html" } - })); + }), + http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); }); // Assert // If the organization has selected TrustedDeviceEncryption but the user still has their master password @@ -408,7 +412,12 @@ public async Task SsoLogin_TrustedDeviceEncryption_UserHasManageResetPasswordPer { "code", "test_code" }, { "code_verifier", challenge }, { "redirect_uri", "https://localhost:8080/sso-connector.html" } - })); + }), + http => + { + http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); + http.Request.Headers.Append("Accept", "application/json"); + }); // Assert // If the organization has selected TrustedDeviceEncryption but the user still has their master password @@ -481,7 +490,8 @@ await providerOrganizationRepository.CreateAsync(new ProviderOrganization { "code", "test_code" }, { "code_verifier", challenge }, { "redirect_uri", "https://localhost:8080/sso-connector.html" } - })); + }), + http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); }); Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); using var responseBody = await AssertHelper.AssertResponseTypeIs(context); @@ -558,10 +568,56 @@ private static async Task RunSuccessTestAsync(Func { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); }); - // Only calls that result in a 200 OK should call this helper - Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + // If this fails, surface detailed error information to aid debugging + if (context.Response.StatusCode != StatusCodes.Status200OK) + { + string contentType = context.Response.ContentType ?? string.Empty; + string rawBody = ""; + try + { + if (context.Response.Body.CanSeek) + { + context.Response.Body.Position = 0; + } + using var reader = new StreamReader(context.Response.Body, leaveOpen: true); + rawBody = await reader.ReadToEndAsync(); + } + catch + { + // leave rawBody as unreadable + } + + string? error = null; + string? errorDesc = null; + string? errorModelMsg = null; + try + { + using var doc = JsonDocument.Parse(rawBody); + var root = doc.RootElement; + if (root.TryGetProperty("error", out var e)) error = e.GetString(); + if (root.TryGetProperty("error_description", out var ed)) errorDesc = ed.GetString(); + if (root.TryGetProperty("ErrorModel", out var em) && em.ValueKind == JsonValueKind.Object) + { + if (em.TryGetProperty("Message", out var msg) && msg.ValueKind == JsonValueKind.String) + { + errorModelMsg = msg.GetString(); + } + } + } + catch + { + // Not JSON, continue with raw body + } + + var message = + $"Unexpected status {context.Response.StatusCode}." + + $" error='{error}' error_description='{errorDesc}' ErrorModel.Message='{errorModelMsg}'" + + $" ContentType='{contentType}' RawBody='{rawBody}'"; + Assert.Fail(message); + } return await AssertHelper.AssertResponseTypeIs(context); } @@ -574,6 +630,18 @@ private static async Task CreateFactoryAsync( { var factory = new IdentityApplicationFactory(); + // Bypass client version gating to isolate SSO test behavior + factory.SubstituteService(svc => + { + svc.ValidateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + }); + + // Compute PKCE S256 code challenge explicitly (base64url of SHA256) + var challengeBytes = System.Text.Encoding.ASCII.GetBytes(challenge); + var hash = System.Security.Cryptography.SHA256.HashData(challengeBytes); + var codeChallenge = Duende.IdentityModel.Base64Url.Encode(hash); + var authorizationCode = new AuthorizationCode { ClientId = "web", @@ -581,8 +649,8 @@ private static async Task CreateFactoryAsync( Lifetime = (int)TimeSpan.FromMinutes(5).TotalSeconds, RedirectUri = "https://localhost:8080/sso-connector.html", RequestedScopes = ["api", "offline_access"], - CodeChallenge = challenge.Sha256(), - CodeChallengeMethod = "plain", + CodeChallenge = codeChallenge, + CodeChallengeMethod = "S256", Subject = null!, // Temporarily set it to null }; @@ -601,10 +669,10 @@ private static async Task CreateFactoryAsync( KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, UserAsymmetricKeys = new KeysRequestModel() { - PublicKey = "public_key", - EncryptedPrivateKey = "private_key" + PublicKey = TestEncryptionConstants.PublicKey, + EncryptedPrivateKey = TestEncryptionConstants.V1EncryptedBase64 // v1-format so parsing succeeds and user is treated as v1 }, - UserSymmetricKey = "sym_key", + UserSymmetricKey = TestEncryptionConstants.V1EncryptedBase64, }); var organizationRepository = factory.Services.GetRequiredService(); diff --git a/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs b/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs index 3f5bf49dd9d7..5613f2e683cc 100644 --- a/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs +++ b/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs @@ -23,6 +23,9 @@ private static async Task SendAsync(this TestServer server, // it runs after this so it will take precedence. httpContext.Connection.RemoteIpAddress = IPAddress.Parse(FactoryConstants.WhitelistedIp); + // Ensure response body is bufferable and seekable for tests to read later + httpContext.Response.Body = new MemoryStream(); + httpContext.Request.Path = new PathString(requestUri); httpContext.Request.Method = method.Method; From 8b8694e5899307cd65d19b99f5dc813ac7043d09 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 2 Dec 2025 13:46:23 -0500 Subject: [PATCH 14/40] test(auth-validator): [PM-22975] Client Version Validator - WIP changes. --- src/Core/KeyManagement/Constants.cs | 2 +- .../GetMinimumClientVersionForUserQuery.cs | 2 +- .../RotateUserAccountKeysCommand.cs | 2 -- .../ClientVersionValidator.cs | 12 +++++----- .../Factories/ApiApplicationFactory.cs | 2 +- ...etMinimumClientVersionForUserQueryTests.cs | 2 +- .../EventsApplicationFactory.cs | 7 +++--- .../Endpoints/IdentityServerSsoTests.cs | 13 ++++++---- .../Endpoints/IdentityServerTwoFactorTests.cs | 12 +++++----- .../ResourceOwnerPasswordValidatorTests.cs | 6 ++--- .../ClientVersionValidatorTests.cs | 13 ++++++++++ .../Factories/IdentityApplicationFactory.cs | 24 ++++++++++++++++--- .../WebApplicationFactoryExtensions.cs | 3 --- 13 files changed, 65 insertions(+), 35 deletions(-) diff --git a/src/Core/KeyManagement/Constants.cs b/src/Core/KeyManagement/Constants.cs index 2bc44134be6e..f5976752fbf5 100644 --- a/src/Core/KeyManagement/Constants.cs +++ b/src/Core/KeyManagement/Constants.cs @@ -2,5 +2,5 @@ public static class Constants { - public static readonly Version MinimumClientVersion = new Version("2025.11.0"); + public static readonly Version MinimumClientVersionForV2Encryption = new Version("2025.11.0"); } diff --git a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs index fe6020ab5a91..4b59a4f3d76f 100644 --- a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs +++ b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs @@ -15,7 +15,7 @@ public class GetMinimumClientVersionForUserQuery(IIsV2EncryptionUserQuery isV2En if (await isV2EncryptionUserQuery.Run(user)) { - return Constants.MinimumClientVersion; + return Constants.MinimumClientVersionForV2Encryption; } return null; diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs index 6e5708f667d8..3e392e6c3702 100644 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs @@ -260,6 +260,4 @@ private static void ValidateV2Encryption(RotateUserAccountKeysData model) throw new InvalidOperationException("No signed security state provider for V2 user"); } } - - // Parsing moved to Bit.Core.KeyManagement.Utilities.EncryptionParsing } diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index 5896061e1312..d8b9e5c3db11 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -16,7 +16,7 @@ public class ClientVersionValidator( IGetMinimumClientVersionForUserQuery getMinimumClientVersionForUserQuery) : IClientVersionValidator { - private static readonly string UpgradeMessage = "Please update your app to continue using Bitwarden"; + private const string _upgradeMessage = "Please update your app to continue using Bitwarden"; public async Task ValidateAsync(User? user, CustomValidatorRequestContext requestContext) { @@ -25,10 +25,10 @@ public async Task ValidateAsync(User? user, CustomValidatorRequestContext return true; } - var clientVersion = currentContext.ClientVersion; - var minVersion = await getMinimumClientVersionForUserQuery.Run(user); + Version clientVersion = currentContext.ClientVersion; + Version? minVersion = await getMinimumClientVersionForUserQuery.Run(user); - // Fail-open if headers are missing or no restriction + // Allow through if headers are missing. if (minVersion == null) { return true; @@ -39,12 +39,12 @@ public async Task ValidateAsync(User? user, CustomValidatorRequestContext requestContext.ValidationErrorResult = new ValidationResult { Error = "invalid_client_version", - ErrorDescription = UpgradeMessage, + ErrorDescription = _upgradeMessage, IsError = true }; requestContext.CustomResponse = new Dictionary { - { "ErrorModel", new ErrorResponseModel(UpgradeMessage) } + { "ErrorModel", new ErrorResponseModel(_upgradeMessage) } }; return false; } diff --git a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs index 93ebacf726a4..fe6fe7d46355 100644 --- a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs +++ b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs @@ -68,7 +68,7 @@ await _identityApplicationFactory.RegisterNewIdentityFactoryUserAsync( UserAsymmetricKeys = new KeysRequestModel() { PublicKey = TestEncryptionConstants.PublicKey, - EncryptedPrivateKey = TestEncryptionConstants.V1EncryptedBase64 // v1-format so parsing succeeds and user is treated as v1 + EncryptedPrivateKey = TestEncryptionConstants.V1EncryptedBase64 }, UserSymmetricKey = TestEncryptionConstants.V1EncryptedBase64, }); diff --git a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs index 1a04e60ca536..77410c25a083 100644 --- a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs +++ b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs @@ -19,7 +19,7 @@ public async Task Run_ReturnsMinVersion_ForV2User() { var sut = new GetMinimumClientVersionForUserQuery(new FakeIsV2Query(true)); var version = await sut.Run(new User()); - Assert.Equal(Core.KeyManagement.Constants.MinimumClientVersion, version); + Assert.Equal(Core.KeyManagement.Constants.MinimumClientVersionForV2Encryption, version); } [Fact] diff --git a/test/Events.IntegrationTest/EventsApplicationFactory.cs b/test/Events.IntegrationTest/EventsApplicationFactory.cs index 7d692c442a6b..45e89f7dc66d 100644 --- a/test/Events.IntegrationTest/EventsApplicationFactory.cs +++ b/test/Events.IntegrationTest/EventsApplicationFactory.cs @@ -3,6 +3,7 @@ using Bit.Core.Enums; using Bit.IntegrationTestCommon; using Bit.IntegrationTestCommon.Factories; +using Bit.Test.Common.Constants; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.TestHost; @@ -58,10 +59,10 @@ await _identityApplicationFactory.RegisterNewIdentityFactoryUserAsync( KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, UserAsymmetricKeys = new KeysRequestModel() { - PublicKey = "public_key", - EncryptedPrivateKey = "private_key" + PublicKey = TestEncryptionConstants.PublicKey, + EncryptedPrivateKey = TestEncryptionConstants.V1EncryptedBase64 }, - UserSymmetricKey = "sym_key", + UserSymmetricKey = TestEncryptionConstants.V1EncryptedBase64, }); return await _identityApplicationFactory.TokenFromPasswordAsync(email, masterPasswordHash); diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs index 22274288245f..8846214b7802 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs @@ -343,7 +343,7 @@ public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_DeviceAlre { "code_verifier", challenge }, { "redirect_uri", "https://localhost:8080/sso-connector.html" } }), - http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); }); + http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); }); // Assert // If the organization has selected TrustedDeviceEncryption but the user still has their master password @@ -415,7 +415,7 @@ public async Task SsoLogin_TrustedDeviceEncryption_UserHasManageResetPasswordPer }), http => { - http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); + http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); http.Request.Headers.Append("Accept", "application/json"); }); @@ -491,7 +491,7 @@ await providerOrganizationRepository.CreateAsync(new ProviderOrganization { "code_verifier", challenge }, { "redirect_uri", "https://localhost:8080/sso-connector.html" } }), - http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); }); + http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); }); Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); using var responseBody = await AssertHelper.AssertResponseTypeIs(context); @@ -569,7 +569,7 @@ private static async Task RunSuccessTestAsync(Func { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); }); + http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); }); // If this fails, surface detailed error information to aid debugging if (context.Response.StatusCode != StatusCodes.Status200OK) @@ -656,8 +656,11 @@ private static async Task CreateFactoryAsync( factory.SubstituteService(service => { - service.GetAuthorizationCodeAsync("test_code") + // Return our pre-built authorization code regardless of handle representation + service.GetAuthorizationCodeAsync(Arg.Any()) .Returns(authorizationCode); + service.RemoveAuthorizationCodeAsync(Arg.Any()) + .Returns(Task.CompletedTask); }); var user = await factory.RegisterNewIdentityFactoryUserAsync( diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs index a04b8acf1908..0e6f9931e6a9 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs @@ -387,10 +387,10 @@ private async Task CreateUserAsync( KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, UserAsymmetricKeys = new KeysRequestModel() { - PublicKey = "public_key", - EncryptedPrivateKey = "private_key" + PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey, + EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64 }, - UserSymmetricKey = "sym_key", + UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64, }); Assert.NotNull(user); @@ -441,10 +441,10 @@ private async Task CreateSsoOrganizationAndUserAsync KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, UserAsymmetricKeys = new KeysRequestModel() { - PublicKey = "public_key", - EncryptedPrivateKey = "private_key" + PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey, + EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64 }, - UserSymmetricKey = "sym_key", + UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64, }); var userService = factory.GetService(); diff --git a/test/Identity.IntegrationTest/RequestValidation/VaultAccess/ResourceOwnerPasswordValidatorTests.cs b/test/Identity.IntegrationTest/RequestValidation/VaultAccess/ResourceOwnerPasswordValidatorTests.cs index 91123b3a60e9..90391ff69931 100644 --- a/test/Identity.IntegrationTest/RequestValidation/VaultAccess/ResourceOwnerPasswordValidatorTests.cs +++ b/test/Identity.IntegrationTest/RequestValidation/VaultAccess/ResourceOwnerPasswordValidatorTests.cs @@ -613,10 +613,10 @@ await factory.RegisterNewIdentityFactoryUserAsync( KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, UserAsymmetricKeys = new KeysRequestModel { - PublicKey = "public_key", - EncryptedPrivateKey = "private_key" + PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey, + EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64 }, - UserSymmetricKey = "sym_key", + UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.V1EncryptedBase64, }); } diff --git a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs index 65a99042464d..8410a4eb75d9 100644 --- a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs @@ -50,4 +50,17 @@ public async Task Allows_When_ClientMeetsMin() var ok = await sut.ValidateAsync(new User(), new Bit.Identity.IdentityServer.CustomValidatorRequestContext()); Assert.True(ok); } + + [Fact] + public async Task Allows_When_ClientVersionHeaderMissing() + { + // Do not set ClientVersion on the context (remains null) and ensure we fail open + var ctx = Substitute.For(); + var minQuery = MakeMinQuery(new Version("2025.11.0")); + var sut = new ClientVersionValidator(ctx, minQuery); + + var ok = await sut.ValidateAsync(new User(), new Bit.Identity.IdentityServer.CustomValidatorRequestContext()); + + Assert.True(ok); + } } diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index 3c0b55190864..529f8459fdf9 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -75,8 +75,20 @@ public async Task PostRegisterVerificationEmailClicked(RegisterVeri var context = await ContextFromPasswordAsync( username, password, deviceIdentifier, clientId, deviceType, deviceName); - using var body = await AssertHelper.AssertResponseTypeIs(context); - var root = body.RootElement; + // Provide clearer diagnostics on failure + if (context.Response.StatusCode != StatusCodes.Status200OK) + { + var contentType = context.Response.ContentType ?? string.Empty; + if (context.Response.Body.CanSeek) + { + context.Response.Body.Position = 0; + } + string rawBody = await new StreamReader(context.Response.Body).ReadToEndAsync(); + throw new Xunit.Sdk.XunitException($"Login failed: status={context.Response.StatusCode}, contentType='{contentType}', body='{rawBody}'"); + } + + using var jsonDoc = await AssertHelper.AssertResponseTypeIs(context); + var root = jsonDoc.RootElement; return (root.GetProperty("access_token").GetString(), root.GetProperty("refresh_token").GetString()); } @@ -99,7 +111,13 @@ public async Task ContextFromPasswordAsync( { "grant_type", "password" }, { "username", username }, { "password", password }, - })); + }), + http => + { + // Ensure JSON content negotiation for errors and set a sane client version + http.Request.Headers.Append("Accept", "application/json"); + http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); + }); return context; } diff --git a/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs b/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs index 5613f2e683cc..3f5bf49dd9d7 100644 --- a/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs +++ b/test/IntegrationTestCommon/Factories/WebApplicationFactoryExtensions.cs @@ -23,9 +23,6 @@ private static async Task SendAsync(this TestServer server, // it runs after this so it will take precedence. httpContext.Connection.RemoteIpAddress = IPAddress.Parse(FactoryConstants.WhitelistedIp); - // Ensure response body is bufferable and seekable for tests to read later - httpContext.Response.Body = new MemoryStream(); - httpContext.Request.Path = new PathString(requestUri); httpContext.Request.Method = method.Method; From ed89cf816108738aded1e58a4ae74eea2892c94a Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 2 Dec 2025 14:22:17 -0500 Subject: [PATCH 15/40] fix(auth-validator): [PM-22975] Client Version Validator - Made enough changes so that it's ready for review by KM --- src/Core/Context/ICurrentContext.cs | 2 +- .../ClientVersionValidator.cs | 7 ++- .../Constants/TestEncryptionConstants.cs | 8 +--- .../Queries/IsV2EncryptionUserQueryTests.cs | 10 ++--- .../Endpoints/IdentityServerSsoTests.cs | 45 ++++++++----------- .../Login/ClientVersionGateTests.cs | 13 +++--- .../ClientVersionValidatorTests.cs | 2 +- 7 files changed, 40 insertions(+), 47 deletions(-) diff --git a/src/Core/Context/ICurrentContext.cs b/src/Core/Context/ICurrentContext.cs index d527cdd36382..3de09f944181 100644 --- a/src/Core/Context/ICurrentContext.cs +++ b/src/Core/Context/ICurrentContext.cs @@ -32,7 +32,7 @@ public interface ICurrentContext Guid? OrganizationId { get; set; } IdentityClientType IdentityClientType { get; set; } string ClientId { get; set; } - Version ClientVersion { get; set; } + Version? ClientVersion { get; set; } bool ClientVersionIsPrerelease { get; set; } Task BuildAsync(HttpContext httpContext, GlobalSettings globalSettings); diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index d8b9e5c3db11..a9de0550cc65 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -25,11 +25,14 @@ public async Task ValidateAsync(User? user, CustomValidatorRequestContext return true; } - Version clientVersion = currentContext.ClientVersion; + Version? clientVersion = currentContext.ClientVersion; Version? minVersion = await getMinimumClientVersionForUserQuery.Run(user); // Allow through if headers are missing. - if (minVersion == null) + // The minVersion should never be null because of where this validator is run. The user would + // have been determined to be null prior to reaching this point, but it is defensive programming + // to check for nullish values in case validators were to ever be reordered. + if (clientVersion == null || minVersion == null) { return true; } diff --git a/test/Common/Constants/TestEncryptionConstants.cs b/test/Common/Constants/TestEncryptionConstants.cs index 40ce984f237d..45d3281865c2 100644 --- a/test/Common/Constants/TestEncryptionConstants.cs +++ b/test/Common/Constants/TestEncryptionConstants.cs @@ -12,12 +12,8 @@ public static class TestEncryptionConstants // Private key indicating v2 (used in multiple tests to mark v2 state) public const string V2PrivateKey = "7.cose"; // Wrapped signing key and verifying key values from real tests - public const string V2WrappedSigningKey = "test-wrapped-signing-key"; - public const string V2VerifyingKey = "test-verifying-key"; - // Additional related v2 values used in tests - public const string V2PublicKey = "test-public-key"; - public const string V2WrappedPrivateKey = "test-private-key"; - public const string V2SignedPublicKey = "test-signed-public-key"; + public const string V2WrappedSigningKey = "7.cose_signing"; + public const string V2VerifyingKey = "vk"; } diff --git a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs index 8dc74100331e..30ec47c2fe07 100644 --- a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs +++ b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs @@ -12,10 +12,10 @@ namespace Bit.Core.Test.KeyManagement.Queries; public class IsV2EncryptionUserQueryTests { - private class FakeSigRepo : IUserSignatureKeyPairRepository + private class FakeUserSignatureKeyPairRepository : IUserSignatureKeyPairRepository { private readonly bool _hasKeys; - public FakeSigRepo(bool hasKeys) { _hasKeys = hasKeys; } + public FakeUserSignatureKeyPairRepository(bool hasKeys) { _hasKeys = hasKeys; } public Task GetByUserIdAsync(Guid userId) => Task.FromResult(_hasKeys ? new SignatureKeyPairData(SignatureAlgorithm.Ed25519, TestEncryptionConstants.V2WrappedSigningKey, TestEncryptionConstants.V2VerifyingKey) : null); @@ -35,7 +35,7 @@ private class FakeSigRepo : IUserSignatureKeyPairRepository public async Task Run_ReturnsTrue_ForV2State() { var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V2PrivateKey }; - var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(true)); + var sut = new IsV2EncryptionUserQuery(new FakeUserSignatureKeyPairRepository(true)); var result = await sut.Run(user); @@ -46,7 +46,7 @@ public async Task Run_ReturnsTrue_ForV2State() public async Task Run_ReturnsFalse_ForV1State() { var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V1EncryptedBase64 }; - var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(false)); + var sut = new IsV2EncryptionUserQuery(new FakeUserSignatureKeyPairRepository(false)); var result = await sut.Run(user); @@ -57,7 +57,7 @@ public async Task Run_ReturnsFalse_ForV1State() public async Task Run_ThrowsForInvalidMixedState() { var user = new User { Id = Guid.NewGuid(), PrivateKey = TestEncryptionConstants.V2PrivateKey }; - var sut = new IsV2EncryptionUserQuery(new FakeSigRepo(false)); + var sut = new IsV2EncryptionUserQuery(new FakeUserSignatureKeyPairRepository(false)); await Assert.ThrowsAsync(async () => await sut.Run(user)); } diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs index 8846214b7802..22447593b47c 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs @@ -342,8 +342,7 @@ public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_DeviceAlre { "code", "test_code" }, { "code_verifier", challenge }, { "redirect_uri", "https://localhost:8080/sso-connector.html" } - }), - http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); }); + })); // Assert // If the organization has selected TrustedDeviceEncryption but the user still has their master password @@ -412,12 +411,7 @@ public async Task SsoLogin_TrustedDeviceEncryption_UserHasManageResetPasswordPer { "code", "test_code" }, { "code_verifier", challenge }, { "redirect_uri", "https://localhost:8080/sso-connector.html" } - }), - http => - { - http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); - http.Request.Headers.Append("Accept", "application/json"); - }); + })); // Assert // If the organization has selected TrustedDeviceEncryption but the user still has their master password @@ -490,8 +484,7 @@ await providerOrganizationRepository.CreateAsync(new ProviderOrganization { "code", "test_code" }, { "code_verifier", challenge }, { "redirect_uri", "https://localhost:8080/sso-connector.html" } - }), - http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); }); + })); Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); using var responseBody = await AssertHelper.AssertResponseTypeIs(context); @@ -554,22 +547,22 @@ private static async Task RunSuccessTestAsync(Func - { - { "scope", "api offline_access" }, - { "client_id", "web" }, - { "deviceType", "10" }, - { "deviceIdentifier", "test_id" }, - { "deviceName", "firefox" }, - { "twoFactorToken", "TEST"}, - { "twoFactorProvider", "5" }, // RememberMe Provider - { "twoFactorRemember", "0" }, - { "grant_type", "authorization_code" }, - { "code", "test_code" }, - { "code_verifier", challenge }, - { "redirect_uri", "https://localhost:8080/sso-connector.html" } - }), - http => { http.Request.Headers.Append("Bitwarden-Client-Version", "2025.10.0"); }); + var context = await factory.Server.PostAsync("/connect/token", new FormUrlEncodedContent( + new Dictionary + { + { "scope", "api offline_access" }, + { "client_id", "web" }, + { "deviceType", "10" }, + { "deviceIdentifier", "test_id" }, + { "deviceName", "firefox" }, + { "twoFactorToken", "TEST" }, + { "twoFactorProvider", "5" }, // RememberMe Provider + { "twoFactorRemember", "0" }, + { "grant_type", "authorization_code" }, + { "code", "test_code" }, + { "code_verifier", challenge }, + { "redirect_uri", "https://localhost:8080/sso-connector.html" } + })); // If this fails, surface detailed error information to aid debugging if (context.Response.StatusCode != StatusCodes.Status200OK) diff --git a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs index 642148b685d1..02eca826abf0 100644 --- a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs +++ b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs @@ -4,6 +4,7 @@ using Bit.Core.Test.Auth.AutoFixture; using Bit.IntegrationTestCommon.Factories; using Bit.Test.Common.AutoFixture.Attributes; +using Bit.Test.Common.Constants; using Microsoft.EntityFrameworkCore; using Xunit; @@ -29,14 +30,14 @@ public async Task TokenEndpoint_GrantTypePassword_V2User_OnOldClientVersion_Bloc // Make user V2: set private key to COSE and add signature key pair var db = localFactory.GetDatabaseContext(); var efUser = await db.Users.FirstAsync(u => u.Email == user.Email); - efUser.PrivateKey = "7.cose"; + efUser.PrivateKey = TestEncryptionConstants.V2PrivateKey; db.UserSignatureKeyPairs.Add(new Bit.Infrastructure.EntityFramework.Models.UserSignatureKeyPair { Id = Core.Utilities.CoreHelpers.GenerateComb(), UserId = efUser.Id, SignatureAlgorithm = SignatureAlgorithm.Ed25519, - SigningKey = "7.cose_signing", - VerifyingKey = "vk" + SigningKey = TestEncryptionConstants.V2WrappedSigningKey, + VerifyingKey = TestEncryptionConstants.V2VerifyingKey, }); await db.SaveChangesAsync(); @@ -74,14 +75,14 @@ public async Task TokenEndpoint_GrantTypePassword_V2User_OnMinClientVersion_Succ // Make user V2 var db = localFactory.GetDatabaseContext(); var efUser = await db.Users.FirstAsync(u => u.Email == user.Email); - efUser.PrivateKey = "7.cose"; + efUser.PrivateKey = TestEncryptionConstants.V2PrivateKey; db.UserSignatureKeyPairs.Add(new Bit.Infrastructure.EntityFramework.Models.UserSignatureKeyPair { Id = Core.Utilities.CoreHelpers.GenerateComb(), UserId = efUser.Id, SignatureAlgorithm = SignatureAlgorithm.Ed25519, - SigningKey = "7.cose_signing", - VerifyingKey = "vk" + SigningKey = TestEncryptionConstants.V2WrappedSigningKey, + VerifyingKey = TestEncryptionConstants.V2VerifyingKey, }); await db.SaveChangesAsync(); diff --git a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs index 8410a4eb75d9..494fb79d25d6 100644 --- a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs @@ -54,7 +54,7 @@ public async Task Allows_When_ClientMeetsMin() [Fact] public async Task Allows_When_ClientVersionHeaderMissing() { - // Do not set ClientVersion on the context (remains null) and ensure we fail open + // Do not set ClientVersion on the context (remains null) and ensure var ctx = Substitute.For(); var minQuery = MakeMinQuery(new Version("2025.11.0")); var sut = new ClientVersionValidator(ctx, minQuery); From 6696104e9dd8b4f7ffa3d49ee2b486028631b6eb Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 2 Dec 2025 16:13:26 -0500 Subject: [PATCH 16/40] fix(auth-validator): [PM-22975] Client Version Validator - Fixed more tests. Checking in with CI to see how it's looking. --- .../Endpoints/IdentityServerSsoTests.cs | 9 ++------- .../Endpoints/IdentityServerTests.cs | 11 +++++++++++ .../Factories/IdentityApplicationFactory.cs | 9 +++++++++ .../Factories/WebApplicationFactoryBase.cs | 4 ++++ 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs index 22447593b47c..0ef761a11206 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs @@ -630,11 +630,6 @@ private static async Task CreateFactoryAsync( .Returns(Task.FromResult(true)); }); - // Compute PKCE S256 code challenge explicitly (base64url of SHA256) - var challengeBytes = System.Text.Encoding.ASCII.GetBytes(challenge); - var hash = System.Security.Cryptography.SHA256.HashData(challengeBytes); - var codeChallenge = Duende.IdentityModel.Base64Url.Encode(hash); - var authorizationCode = new AuthorizationCode { ClientId = "web", @@ -642,8 +637,8 @@ private static async Task CreateFactoryAsync( Lifetime = (int)TimeSpan.FromMinutes(5).TotalSeconds, RedirectUri = "https://localhost:8080/sso-connector.html", RequestedScopes = ["api", "offline_access"], - CodeChallenge = codeChallenge, - CodeChallengeMethod = "S256", + CodeChallenge = challenge.Sha256(), + CodeChallengeMethod = "plain", Subject = null!, // Temporarily set it to null }; diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs index 6f10f22002be..fb298b9677e6 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs @@ -9,11 +9,14 @@ using Bit.Core.Platform.Installations; using Bit.Core.Repositories; using Bit.Core.Test.Auth.AutoFixture; +using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidators; using Bit.IntegrationTestCommon.Factories; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; using Microsoft.AspNetCore.TestHost; using Microsoft.EntityFrameworkCore; +using NSubstitute; using Xunit; namespace Bit.Identity.IntegrationTest.Endpoints; @@ -29,6 +32,14 @@ public class IdentityServerTests : IClassFixture public IdentityServerTests(IdentityApplicationFactory factory) { _factory = factory; + + // Bypass client version gating to isolate SSO test behavior + _factory.SubstituteService(svc => + { + svc.ValidateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + }); + ReinitializeDbForTests(_factory); } diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index 529f8459fdf9..b8642cc49e7a 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -9,6 +9,8 @@ using Bit.Core.Enums; using Bit.Core.Services; using Bit.Identity; +using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidators; using Bit.Test.Common.Helpers; using LinqToDB; using Microsoft.AspNetCore.Hosting; @@ -46,6 +48,13 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) }); }); + // Bypass client version gating to isolate tests from client version behavior + SubstituteService(svc => + { + svc.ValidateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + }); + base.ConfigureWebHost(builder); } diff --git a/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs b/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs index a41cd4392360..983e5f6102db 100644 --- a/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs +++ b/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs @@ -131,6 +131,10 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) { "globalSettings:databaseProvider", "postgres" }, { "globalSettings:postgreSql:connectionString", "Host=localhost;Username=test;Password=test;Database=test" }, + // Ensure base service URIs are defined for tests (used for client redirect URIs) + { "globalSettings:baseServiceUri:vault", "https://localhost:8080" }, + { "globalSettings:baseServiceUri:internalVault", "https://localhost:8080" }, + // Clear the redis connection string for distributed caching, forcing an in-memory implementation { "globalSettings:redis:connectionString", "" }, From aa4f8ab96e9bc0e6e10ad9d57923ca706128613d Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 2 Dec 2025 16:42:09 -0500 Subject: [PATCH 17/40] test(auth-validator): [PM-22975] Client Version Validator - Fixed the last test. --- .../Login/ClientVersionGateTests.cs | 10 ++++++++-- .../Factories/IdentityApplicationFactory.cs | 14 +++++++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs index 02eca826abf0..501d39754df6 100644 --- a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs +++ b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs @@ -23,7 +23,10 @@ public ClientVersionGateTests(IdentityApplicationFactory factory) [Theory, BitAutoData, RegisterFinishRequestModelCustomize] public async Task TokenEndpoint_GrantTypePassword_V2User_OnOldClientVersion_Blocked(RegisterFinishRequestModel requestModel) { - var localFactory = new IdentityApplicationFactory(); + var localFactory = new IdentityApplicationFactory + { + UseMockClientVersionValidator = false + }; var server = localFactory.Server; var user = await localFactory.RegisterNewIdentityFactoryUserAsync(requestModel); @@ -68,7 +71,10 @@ public async Task TokenEndpoint_GrantTypePassword_V2User_OnOldClientVersion_Bloc [Theory, BitAutoData, RegisterFinishRequestModelCustomize] public async Task TokenEndpoint_GrantTypePassword_V2User_OnMinClientVersion_Succeeds(RegisterFinishRequestModel requestModel) { - var localFactory = new IdentityApplicationFactory(); + var localFactory = new IdentityApplicationFactory + { + UseMockClientVersionValidator = false + }; var server = localFactory.Server; var user = await localFactory.RegisterNewIdentityFactoryUserAsync(requestModel); diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index b8642cc49e7a..d228ed38e365 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -25,6 +25,7 @@ public class IdentityApplicationFactory : WebApplicationFactoryBase public const string DefaultDeviceIdentifier = "92b9d953-b9b6-4eaf-9d3e-11d57144dfeb"; public const string DefaultUserEmail = "DefaultEmail@bitwarden.com"; public const string DefaultUserPasswordHash = "default_password_hash"; + public bool UseMockClientVersionValidator { get; set; } = true; /// /// A dictionary to store registration tokens for email verification. We cannot substitute the IMailService more than once, so @@ -48,12 +49,15 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) }); }); - // Bypass client version gating to isolate tests from client version behavior - SubstituteService(svc => + if (UseMockClientVersionValidator) { - svc.ValidateAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); - }); + // Bypass client version gating to isolate tests from client version behavior + SubstituteService(svc => + { + svc.ValidateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + }); + } base.ConfigureWebHost(builder); } From 86bca816447b02582479abd99537e57420d2e89f Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 2 Dec 2025 16:59:52 -0500 Subject: [PATCH 18/40] fix(auth-validator): [PM-22975] Client Version Validator - Changed some minor things in identity server sso tests. --- .../Endpoints/IdentityServerSsoTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs index 0ef761a11206..010ac70d2710 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs @@ -612,6 +612,9 @@ private static async Task RunSuccessTestAsync(Func(context); } @@ -645,10 +648,8 @@ private static async Task CreateFactoryAsync( factory.SubstituteService(service => { // Return our pre-built authorization code regardless of handle representation - service.GetAuthorizationCodeAsync(Arg.Any()) + service.GetAuthorizationCodeAsync("test_code") .Returns(authorizationCode); - service.RemoveAuthorizationCodeAsync(Arg.Any()) - .Returns(Task.CompletedTask); }); var user = await factory.RegisterNewIdentityFactoryUserAsync( From c1bc10bf4084b6a114bd763bc79593c151a13c4e Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 2 Dec 2025 17:07:14 -0500 Subject: [PATCH 19/40] fix(auth-validator): [PM-22975] Client Version Validator - Removed unneded code. --- .../Factories/WebApplicationFactoryBase.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs b/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs index 983e5f6102db..a41cd4392360 100644 --- a/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs +++ b/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs @@ -131,10 +131,6 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) { "globalSettings:databaseProvider", "postgres" }, { "globalSettings:postgreSql:connectionString", "Host=localhost;Username=test;Password=test;Database=test" }, - // Ensure base service URIs are defined for tests (used for client redirect URIs) - { "globalSettings:baseServiceUri:vault", "https://localhost:8080" }, - { "globalSettings:baseServiceUri:internalVault", "https://localhost:8080" }, - // Clear the redis connection string for distributed caching, forcing an in-memory implementation { "globalSettings:redis:connectionString", "" }, From 753670d26fee9314f7502b70ccf8324ee480168e Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Wed, 3 Dec 2025 09:46:00 -0500 Subject: [PATCH 20/40] 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. --- src/Core/Entities/User.cs | 7 +- src/Core/KeyManagement/Constants.cs | 2 +- ...eyManagementServiceCollectionExtensions.cs | 1 - .../GetMinimumClientVersionForUserQuery.cs | 12 ++-- .../Interfaces/IIsV2EncryptionUserQuery.cs | 8 --- .../Queries/IsV2EncryptionUserQuery.cs | 31 --------- .../Utilities/EncryptionParsing.cs | 4 +- .../ClientVersionValidator.cs | 10 +++ ...etMinimumClientVersionForUserQueryTests.cs | 22 +++---- .../Queries/IsV2EncryptionUserQueryTests.cs | 64 ------------------- 10 files changed, 36 insertions(+), 125 deletions(-) delete mode 100644 src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs delete mode 100644 src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs delete mode 100644 test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index fec9b80d8e2c..5d687efe168c 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -51,7 +51,7 @@ public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFac public string? Key { get; set; } /// /// The raw public key, without a signature from the user's signature key. - /// + /// public string? PublicKey { get; set; } /// /// User key wrapped private key. @@ -211,6 +211,11 @@ public int GetSecurityVersion() return SecurityVersion ?? 1; } + public bool IsSecurityVersionTwo() + { + return SecurityVersion == 2; + } + /// /// Serializes the C# object to the User.TwoFactorProviders property in JSON format. /// diff --git a/src/Core/KeyManagement/Constants.cs b/src/Core/KeyManagement/Constants.cs index f5976752fbf5..f1e3e1a2685c 100644 --- a/src/Core/KeyManagement/Constants.cs +++ b/src/Core/KeyManagement/Constants.cs @@ -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"); } diff --git a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs index 9b63dfdaf9a3..f18d1c73ba38 100644 --- a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs +++ b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs @@ -26,7 +26,6 @@ private static void AddKeyManagementCommands(this IServiceCollection services) private static void AddKeyManagementQueries(this IServiceCollection services) { services.AddScoped(); - services.AddScoped(); services.AddScoped(); } } diff --git a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs index 4b59a4f3d76f..b39fa11320d8 100644 --- a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs +++ b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs @@ -3,21 +3,21 @@ namespace Bit.Core.KeyManagement.Queries; -public class GetMinimumClientVersionForUserQuery(IIsV2EncryptionUserQuery isV2EncryptionUserQuery) +public class GetMinimumClientVersionForUserQuery() : IGetMinimumClientVersionForUserQuery { - public async Task Run(User? user) + public Task Run(User? user) { if (user == null) { - return null; + return Task.FromResult(null); } - if (await isV2EncryptionUserQuery.Run(user)) + if (user.IsSecurityVersionTwo()) { - return Constants.MinimumClientVersionForV2Encryption; + return Task.FromResult(Constants.MinimumClientVersionForV2Encryption)!; } - return null; + return Task.FromResult(null); } } diff --git a/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs b/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs deleted file mode 100644 index 38c0e10b4404..000000000000 --- a/src/Core/KeyManagement/Queries/Interfaces/IIsV2EncryptionUserQuery.cs +++ /dev/null @@ -1,8 +0,0 @@ -using Bit.Core.Entities; - -namespace Bit.Core.KeyManagement.Queries.Interfaces; - -public interface IIsV2EncryptionUserQuery -{ - Task Run(User user); -} diff --git a/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs b/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs deleted file mode 100644 index ea64d5a20aa5..000000000000 --- a/src/Core/KeyManagement/Queries/IsV2EncryptionUserQuery.cs +++ /dev/null @@ -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 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.") - }; - } -} diff --git a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs index ffe8cb3134fb..96a3117cf78f 100644 --- a/src/Core/KeyManagement/Utilities/EncryptionParsing.cs +++ b/src/Core/KeyManagement/Utilities/EncryptionParsing.cs @@ -7,8 +7,10 @@ public static class EncryptionParsing /// /// Helper method to convert an encryption type string to an enum value. /// - public static EncryptionType GetEncryptionType(string encString) + public static EncryptionType GetEncryptionType(string? encString) { + ArgumentNullException.ThrowIfNull(encString); + var parts = encString.Split('.'); if (parts.Length == 1) { diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index a9de0550cc65..558ad041c2cc 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -11,6 +11,16 @@ public interface IClientVersionValidator Task ValidateAsync(User user, CustomValidatorRequestContext requestContext); } +/// +/// 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. +/// public class ClientVersionValidator( ICurrentContext currentContext, IGetMinimumClientVersionForUserQuery getMinimumClientVersionForUserQuery) diff --git a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs index 77410c25a083..db8a76e06b93 100644 --- a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs +++ b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs @@ -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 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); } } diff --git a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs deleted file mode 100644 index 30ec47c2fe07..000000000000 --- a/test/Core.Test/KeyManagement/Queries/IsV2EncryptionUserQueryTests.cs +++ /dev/null @@ -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 GetByUserIdAsync(Guid userId) - => Task.FromResult(_hasKeys ? new SignatureKeyPairData(SignatureAlgorithm.Ed25519, TestEncryptionConstants.V2WrappedSigningKey, TestEncryptionConstants.V2VerifyingKey) : null); - - // Unused in tests - public Task> GetManyAsync(IEnumerable ids) => throw new NotImplementedException(); - public Task GetByIdAsync(Guid id) => throw new NotImplementedException(); - public Task 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(async () => await sut.Run(user)); - } -} From 544965e0bdd98812a3daaa36d837cc9739cefb11 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Wed, 3 Dec 2025 10:00:11 -0500 Subject: [PATCH 21/40] test(auth-validator): [PM-22975] Client Version Validator - Fixed test --- test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs index 501d39754df6..55db8cf82ad6 100644 --- a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs +++ b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs @@ -34,6 +34,7 @@ public async Task TokenEndpoint_GrantTypePassword_V2User_OnOldClientVersion_Bloc var db = localFactory.GetDatabaseContext(); var efUser = await db.Users.FirstAsync(u => u.Email == user.Email); efUser.PrivateKey = TestEncryptionConstants.V2PrivateKey; + efUser.SecurityVersion = 2; db.UserSignatureKeyPairs.Add(new Bit.Infrastructure.EntityFramework.Models.UserSignatureKeyPair { Id = Core.Utilities.CoreHelpers.GenerateComb(), @@ -82,6 +83,7 @@ public async Task TokenEndpoint_GrantTypePassword_V2User_OnMinClientVersion_Succ var db = localFactory.GetDatabaseContext(); var efUser = await db.Users.FirstAsync(u => u.Email == user.Email); efUser.PrivateKey = TestEncryptionConstants.V2PrivateKey; + efUser.SecurityVersion = 2; db.UserSignatureKeyPairs.Add(new Bit.Infrastructure.EntityFramework.Models.UserSignatureKeyPair { Id = Core.Utilities.CoreHelpers.GenerateComb(), From f719763a85a489951dfb3a5ad4d20d5cb7138823 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Wed, 3 Dec 2025 14:44:33 -0500 Subject: [PATCH 22/40] fix(auth-validator): [PM-22975] Client Version Validator - Took in team feedback. --- src/Core/Entities/User.cs | 13 ++++++++++++- .../Queries/GetMinimumClientVersionForUserQuery.cs | 2 +- .../GetMinimumClientVersionForUserQueryTests.cs | 7 +++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index 5d687efe168c..8b3b242c8f38 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -4,6 +4,7 @@ using Bit.Core.Auth.Models; using Bit.Core.Enums; using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.Utilities; using Bit.Core.Utilities; using Microsoft.AspNetCore.Identity; @@ -211,7 +212,17 @@ public int GetSecurityVersion() return SecurityVersion ?? 1; } - public bool IsSecurityVersionTwo() + public bool IsSetupForV2Encryption() + { + return HasV2KeyShape() && IsSecurityVersionTwo(); + } + + private bool HasV2KeyShape() + { + return EncryptionParsing.GetEncryptionType(PrivateKey) == EncryptionType.XChaCha20Poly1305_B64; + } + + private bool IsSecurityVersionTwo() { return SecurityVersion == 2; } diff --git a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs index b39fa11320d8..f6fc64a4f768 100644 --- a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs +++ b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs @@ -13,7 +13,7 @@ public class GetMinimumClientVersionForUserQuery() return Task.FromResult(null); } - if (user.IsSecurityVersionTwo()) + if (user.IsSetupForV2Encryption()) { return Task.FromResult(Constants.MinimumClientVersionForV2Encryption)!; } diff --git a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs index db8a76e06b93..b9bbbcd60bba 100644 --- a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs +++ b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs @@ -1,5 +1,6 @@ using Bit.Core.Entities; using Bit.Core.KeyManagement.Queries; +using Bit.Test.Common.Constants; using Xunit; namespace Bit.Core.Test.KeyManagement.Queries; @@ -12,7 +13,8 @@ public async Task Run_ReturnsMinVersion_ForV2User() var sut = new GetMinimumClientVersionForUserQuery(); var version = await sut.Run(new User { - SecurityVersion = 2 + SecurityVersion = 2, + PrivateKey = TestEncryptionConstants.V2PrivateKey, }); Assert.Equal(Core.KeyManagement.Constants.MinimumClientVersionForV2Encryption, version); } @@ -23,7 +25,8 @@ public async Task Run_ReturnsNull_ForV1User() var sut = new GetMinimumClientVersionForUserQuery(); var version = await sut.Run(new User { - SecurityVersion = 1 + SecurityVersion = 1, + PrivateKey = TestEncryptionConstants.V2PrivateKey, }); Assert.Null(version); } From cff2f5df6dd50bf1acbb77e1d971df97108424bb Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 4 Dec 2025 09:24:27 -0500 Subject: [PATCH 23/40] fix(auth-validator): [PM-22975] Client Version Validator - Added more tests and added comment. --- src/Core/Entities/User.cs | 5 +++ ...etMinimumClientVersionForUserQueryTests.cs | 37 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index 8b3b242c8f38..ff0ec2f31e88 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -222,6 +222,11 @@ private bool HasV2KeyShape() return EncryptionParsing.GetEncryptionType(PrivateKey) == EncryptionType.XChaCha20Poly1305_B64; } + /// + /// This technically is correct but all versions after 1 are considered v2 encryption. Leaving for now with + /// KM's blessing that when a new version comes along they will handle migration. + /// + /// private bool IsSecurityVersionTwo() { return SecurityVersion == 2; diff --git a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs index b9bbbcd60bba..010eb2358672 100644 --- a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs +++ b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs @@ -26,6 +26,43 @@ public async Task Run_ReturnsNull_ForV1User() 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); From 2ed458d5d40f8d4864257d9e4e189f5e78d296b2 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 4 Dec 2025 09:27:10 -0500 Subject: [PATCH 24/40] fix(auth-validator): [PM-22975] Client Version Validator - Removed one line --- src/Core/Entities/User.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index ff0ec2f31e88..510455886cde 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -226,7 +226,6 @@ private bool HasV2KeyShape() /// This technically is correct but all versions after 1 are considered v2 encryption. Leaving for now with /// KM's blessing that when a new version comes along they will handle migration. /// - /// private bool IsSecurityVersionTwo() { return SecurityVersion == 2; From d706796fc33da31f5c1129fca0a246bb4b9ecce5 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 4 Dec 2025 16:54:45 -0500 Subject: [PATCH 25/40] fix(auth-validator): [PM-22975] Client Version Validator - Updated validator to return false on null. --- .../RequestValidators/ClientVersionValidator.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index 558ad041c2cc..378b272a6229 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -30,9 +30,12 @@ public class ClientVersionValidator( public async Task 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) { - return true; + return false; } Version? clientVersion = currentContext.ClientVersion; From 226405609e4e5f935eb0c819170c07dc94d7fe9d Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 10:26:59 -0500 Subject: [PATCH 26/40] fix(auth-validator): [PM-22975] Client Version Validator - Updated with removal of cqrs approach in favor of static user checks. Also fixed tests --- src/Core/Entities/User.cs | 21 +++- ...eyManagementServiceCollectionExtensions.cs | 1 - .../GetMinimumClientVersionForUserQuery.cs | 23 ---- .../IGetMinimumClientVersionForUserQuery.cs | 8 -- .../RequestValidators/BaseRequestValidator.cs | 3 +- .../ClientVersionValidator.cs | 22 ++-- .../Factories/ApiApplicationFactory.cs | 4 +- .../Constants/TestEncryptionConstants.cs | 6 +- ...etMinimumClientVersionForUserQueryTests.cs | 72 ------------- .../EventsApplicationFactory.cs | 4 +- .../Endpoints/IdentityServerSsoTests.cs | 12 +-- .../Endpoints/IdentityServerTests.cs | 2 +- .../Endpoints/IdentityServerTwoFactorTests.cs | 8 +- .../ResourceOwnerPasswordValidatorTests.cs | 4 +- .../BaseRequestValidatorTests.cs | 6 +- .../ClientVersionValidatorTests.cs | 100 ++++++++++++++---- .../Factories/IdentityApplicationFactory.cs | 2 +- 17 files changed, 138 insertions(+), 160 deletions(-) delete mode 100644 src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs delete mode 100644 src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs delete mode 100644 test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index 510455886cde..8045e7eaf14b 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -212,14 +212,31 @@ public int GetSecurityVersion() return SecurityVersion ?? 1; } - public bool IsSetupForV2Encryption() + /// + /// Evaluates user state to determine if they are currently in a v2 encryption state. + /// + /// If the shape of their private key is v2 as well as has the proper security version then true, otherwise false + 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; + } } /// diff --git a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs index f18d1c73ba38..0e551c5d0e20 100644 --- a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs +++ b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs @@ -26,6 +26,5 @@ private static void AddKeyManagementCommands(this IServiceCollection services) private static void AddKeyManagementQueries(this IServiceCollection services) { services.AddScoped(); - services.AddScoped(); } } diff --git a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs deleted file mode 100644 index f6fc64a4f768..000000000000 --- a/src/Core/KeyManagement/Queries/GetMinimumClientVersionForUserQuery.cs +++ /dev/null @@ -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 Run(User? user) - { - if (user == null) - { - return Task.FromResult(null); - } - - if (user.IsSetupForV2Encryption()) - { - return Task.FromResult(Constants.MinimumClientVersionForV2Encryption)!; - } - - return Task.FromResult(null); - } -} diff --git a/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs b/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs deleted file mode 100644 index 01deb460f198..000000000000 --- a/src/Core/KeyManagement/Queries/Interfaces/IGetMinimumClientVersionForUserQuery.cs +++ /dev/null @@ -1,8 +0,0 @@ -using Bit.Core.Entities; - -namespace Bit.Core.KeyManagement.Queries.Interfaces; - -public interface IGetMinimumClientVersionForUserQuery -{ - Task Run(User? user); -} diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index e18b7165be85..7c81b3b777b0 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -359,12 +359,11 @@ private static async Task ProcessValidatorsAsync(params Func>[] /// /// Validates whether the client version is compatible for the user attempting to authenticate. - /// New authentications only; refresh/device grants are handled elsewhere. /// /// true if the scheme successfully passed validation, otherwise false. private async Task ValidateClientVersionAsync(T context, CustomValidatorRequestContext validatorContext) { - var ok = await _clientVersionValidator.ValidateAsync(validatorContext.User, validatorContext); + var ok = _clientVersionValidator.ValidateAsync(validatorContext.User, validatorContext); if (ok) { return true; diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index 378b272a6229..c160d81f1111 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -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 ValidateAsync(User user, CustomValidatorRequestContext requestContext); + bool ValidateAsync(User user, CustomValidatorRequestContext requestContext); } /// @@ -22,24 +22,34 @@ public interface IClientVersionValidator /// If the header is omitted, then the validator returns that this request is valid. /// 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 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 + { + { "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 diff --git a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs index fe6fe7d46355..7cc827bc6eea 100644 --- a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs +++ b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs @@ -68,9 +68,9 @@ await _identityApplicationFactory.RegisterNewIdentityFactoryUserAsync( 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); diff --git a/test/Common/Constants/TestEncryptionConstants.cs b/test/Common/Constants/TestEncryptionConstants.cs index 45d3281865c2..aa30b2060952 100644 --- a/test/Common/Constants/TestEncryptionConstants.cs +++ b/test/Common/Constants/TestEncryptionConstants.cs @@ -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"; } - - diff --git a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs b/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs deleted file mode 100644 index 010eb2358672..000000000000 --- a/test/Core.Test/KeyManagement/Queries/GetMinimumClientVersionForUserQueryTests.cs +++ /dev/null @@ -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); - } -} - - diff --git a/test/Events.IntegrationTest/EventsApplicationFactory.cs b/test/Events.IntegrationTest/EventsApplicationFactory.cs index 45e89f7dc66d..3530d0aa6cb3 100644 --- a/test/Events.IntegrationTest/EventsApplicationFactory.cs +++ b/test/Events.IntegrationTest/EventsApplicationFactory.cs @@ -60,9 +60,9 @@ await _identityApplicationFactory.RegisterNewIdentityFactoryUserAsync( 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); diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs index 010ac70d2710..2effe8cb432c 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs @@ -313,8 +313,8 @@ public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_DeviceAlre var user = await factory.Services.GetRequiredService().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 async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_DeviceAlre Name = "Thing", UserId = user.Id, EncryptedPrivateKey = expectedPrivateKey, - EncryptedPublicKey = TestEncryptionConstants.V1EncryptedBase64, + EncryptedPublicKey = TestEncryptionConstants.AES256_CBC_HMAC_Encstring, EncryptedUserKey = expectedUserKey, }); @@ -630,7 +630,7 @@ private static async Task CreateFactoryAsync( factory.SubstituteService(svc => { svc.ValidateAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(true); }); var authorizationCode = new AuthorizationCode @@ -662,9 +662,9 @@ private static async Task CreateFactoryAsync( 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(); diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs index fb298b9677e6..7b42f7464e4d 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs @@ -37,7 +37,7 @@ public IdentityServerTests(IdentityApplicationFactory factory) _factory.SubstituteService(svc => { svc.ValidateAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(true); }); ReinitializeDbForTests(_factory); diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs index 0e6f9931e6a9..d0d35f5d7bfe 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs @@ -388,9 +388,9 @@ private async Task CreateUserAsync( 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 @@ private async Task CreateSsoOrganizationAndUserAsync 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(); diff --git a/test/Identity.IntegrationTest/RequestValidation/VaultAccess/ResourceOwnerPasswordValidatorTests.cs b/test/Identity.IntegrationTest/RequestValidation/VaultAccess/ResourceOwnerPasswordValidatorTests.cs index 90391ff69931..54fb80eed04c 100644 --- a/test/Identity.IntegrationTest/RequestValidation/VaultAccess/ResourceOwnerPasswordValidatorTests.cs +++ b/test/Identity.IntegrationTest/RequestValidation/VaultAccess/ResourceOwnerPasswordValidatorTests.cs @@ -614,9 +614,9 @@ await factory.RegisterNewIdentityFactoryUserAsync( 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, }); } diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index e92a04ec155d..040da40da2a3 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -109,7 +109,7 @@ public BaseRequestValidatorTests() // Default client version validator behavior: allow to pass unless a test overrides. _clientVersionValidator .ValidateAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(true); } private void SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(bool recoveryCodeSupportEnabled) @@ -1296,7 +1296,7 @@ public async Task ValidateAsync_ClientVersionValidator_IsInvoked_ForFeatureFlagS // 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 async Task ValidateAsync_ClientVersionValidator_IsInvoked_ForFeatureFlagS await _sut.ValidateAsync(context); // Assert - await _clientVersionValidator.Received(1) + _clientVersionValidator.Received(1) .ValidateAsync(requestContext.User, requestContext); } diff --git a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs index 494fb79d25d6..6adc997f79e3 100644 --- a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs @@ -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(); ctx.ClientVersion = version; return ctx; } - private static IGetMinimumClientVersionForUserQuery MakeMinQuery(Version? v) + private static User MakeValidV2User() { - var q = Substitute.For(); - q.Run(Arg.Any()).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 async Task Blocks_When_ClientTooOld() } [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(); - 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 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(); - var ok = await sut.ValidateAsync(new User(), new Bit.Identity.IdentityServer.CustomValidatorRequestContext()); + // Act + var ok = sut.ValidateAsync(user, ctx); + // Assert Assert.True(ok); } } diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index d228ed38e365..1304d66f67a9 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -55,7 +55,7 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) SubstituteService(svc => { svc.ValidateAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(true); }); } From 55bfb71bef5a981f15b2153afcd437f886b834c0 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 10:35:34 -0500 Subject: [PATCH 27/40] test(auth-validator): [PM-22975] Client Version Validator - Added enccryption parsing tests --- .../Utilities/EncryptionParsingTests.cs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs diff --git a/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs b/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs new file mode 100644 index 000000000000..69bb6d08beda --- /dev/null +++ b/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs @@ -0,0 +1,41 @@ +using Bit.Core.Enums; +using Bit.Core.KeyManagement.Utilities; +using Xunit; + +namespace Bit.Core.Test.KeyManagement.Utilities; + +public class EncryptionParsingTests +{ + [Fact] + public void GetEncryptionType_WithNull_ThrowsArgumentNullException() + { + Assert.Throws(() => EncryptionParsing.GetEncryptionType(null)); + } + + [Theory] + [InlineData("2")] // missing '.' separator + [InlineData("abc.def")] // non-numeric prefix + [InlineData("8.any")] // undefined enum value + [InlineData("255.any")] // out of defined enum range + public void GetEncryptionType_WithInvalidString_ThrowsArgumentException(string input) + { + Assert.Throws(() => EncryptionParsing.GetEncryptionType(input)); + } + + [Theory] + [InlineData("0.foo", EncryptionType.AesCbc256_B64)] + [InlineData("1.bar", EncryptionType.AesCbc128_HmacSha256_B64)] + [InlineData("2.qux", EncryptionType.AesCbc256_HmacSha256_B64)] + [InlineData("3.any", EncryptionType.Rsa2048_OaepSha256_B64)] + [InlineData("4.any", EncryptionType.Rsa2048_OaepSha1_B64)] + [InlineData("5.any", EncryptionType.Rsa2048_OaepSha256_HmacSha256_B64)] + [InlineData("6.any", EncryptionType.Rsa2048_OaepSha1_HmacSha256_B64)] + [InlineData("7.any", EncryptionType.XChaCha20Poly1305_B64)] + [InlineData("2.", EncryptionType.AesCbc256_HmacSha256_B64)] // empty suffix still valid + public void GetEncryptionType_WithValidString_ReturnsExpected(string input, EncryptionType expected) + { + var result = EncryptionParsing.GetEncryptionType(input); + Assert.Equal(expected, result); + } +} + From 36e7b1c65ec21ef5b4b541fa493c91ef7d7fda3f Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 10:41:35 -0500 Subject: [PATCH 28/40] test(auth-validator): [PM-22975] Client Version Validator - Added stubs and updated test for encryption parsing tests. --- .../Constants/TestEncryptionConstants.cs | 9 +++++++++ .../Utilities/EncryptionParsingTests.cs | 20 ++++++++++--------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/test/Common/Constants/TestEncryptionConstants.cs b/test/Common/Constants/TestEncryptionConstants.cs index aa30b2060952..7bb3b3bb8479 100644 --- a/test/Common/Constants/TestEncryptionConstants.cs +++ b/test/Common/Constants/TestEncryptionConstants.cs @@ -5,6 +5,15 @@ public static class TestEncryptionConstants // Intended for use as a V1 encrypted string, accepted by validators public const string AES256_CBC_HMAC_Encstring = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; + // Simple stubs for other encstring versions used by parsing tests + public const string AES256_CBC_B64_Encstring = "0.stub"; + public const string AES128_CBC_HMACSHA256_B64_Encstring = "1.stub"; + public const string AES256_CBC_HMAC_EmptySuffix = "2."; + public const string RSA2048_OAEPSHA256_B64_Encstring = "3.stub"; + public const string RSA2048_OAEPSHA1_B64_Encstring = "4.stub"; + public const string RSA2048_OAEPSHA256_HMACSHA256_B64_Encstring = "5.stub"; + public const string RSA2048_OAEPSHA1_HMACSHA256_B64_Encstring = "6.stub"; + // Public key test placeholder public const string PublicKey = "pk_test"; diff --git a/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs b/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs index 69bb6d08beda..c68aa8a7a37b 100644 --- a/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs +++ b/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs @@ -1,5 +1,6 @@ using Bit.Core.Enums; using Bit.Core.KeyManagement.Utilities; +using Bit.Test.Common.Constants; using Xunit; namespace Bit.Core.Test.KeyManagement.Utilities; @@ -23,15 +24,16 @@ public void GetEncryptionType_WithInvalidString_ThrowsArgumentException(string i } [Theory] - [InlineData("0.foo", EncryptionType.AesCbc256_B64)] - [InlineData("1.bar", EncryptionType.AesCbc128_HmacSha256_B64)] - [InlineData("2.qux", EncryptionType.AesCbc256_HmacSha256_B64)] - [InlineData("3.any", EncryptionType.Rsa2048_OaepSha256_B64)] - [InlineData("4.any", EncryptionType.Rsa2048_OaepSha1_B64)] - [InlineData("5.any", EncryptionType.Rsa2048_OaepSha256_HmacSha256_B64)] - [InlineData("6.any", EncryptionType.Rsa2048_OaepSha1_HmacSha256_B64)] - [InlineData("7.any", EncryptionType.XChaCha20Poly1305_B64)] - [InlineData("2.", EncryptionType.AesCbc256_HmacSha256_B64)] // empty suffix still valid + [InlineData(TestEncryptionConstants.AES256_CBC_B64_Encstring, EncryptionType.AesCbc256_B64)] + [InlineData(TestEncryptionConstants.AES128_CBC_HMACSHA256_B64_Encstring, EncryptionType.AesCbc128_HmacSha256_B64)] + [InlineData(TestEncryptionConstants.AES256_CBC_HMAC_Encstring, EncryptionType.AesCbc256_HmacSha256_B64)] + [InlineData(TestEncryptionConstants.RSA2048_OAEPSHA256_B64_Encstring, EncryptionType.Rsa2048_OaepSha256_B64)] + [InlineData(TestEncryptionConstants.RSA2048_OAEPSHA1_B64_Encstring, EncryptionType.Rsa2048_OaepSha1_B64)] + [InlineData(TestEncryptionConstants.RSA2048_OAEPSHA256_HMACSHA256_B64_Encstring, EncryptionType.Rsa2048_OaepSha256_HmacSha256_B64)] + [InlineData(TestEncryptionConstants.RSA2048_OAEPSHA1_HMACSHA256_B64_Encstring, EncryptionType.Rsa2048_OaepSha1_HmacSha256_B64)] + [InlineData(TestEncryptionConstants.V2PrivateKey, EncryptionType.XChaCha20Poly1305_B64)] + [InlineData(TestEncryptionConstants.V2WrappedSigningKey, EncryptionType.XChaCha20Poly1305_B64)] + [InlineData(TestEncryptionConstants.AES256_CBC_HMAC_EmptySuffix, EncryptionType.AesCbc256_HmacSha256_B64)] // empty suffix still valid public void GetEncryptionType_WithValidString_ReturnsExpected(string input, EncryptionType expected) { var result = EncryptionParsing.GetEncryptionType(input); From 1f8be3b05ca113ab215fa287180e9b156998f2ab Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 10:48:22 -0500 Subject: [PATCH 29/40] docs(auth-validator): [PM-22975] Client Version Validator - Updated comment to make more sense. --- .../RequestValidators/ClientVersionValidator.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index c160d81f1111..32af1dac5298 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -15,11 +15,13 @@ public interface IClientVersionValidator /// 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. +/// incentivize users to get on an updated client when their password encryption +/// method has already been updated. /// /// If the header is omitted, then the validator returns that this request is valid. +/// We do this because clients can always just put whatever they want in the header, +/// and all we can do is try to prevent legitimate clients from ending up in a scenario +/// where they cannot log in due to stale encryption versions and newer client architecture. /// public class ClientVersionValidator( ICurrentContext currentContext) From 998aeb14826cb9a237f50242190b0e6a2819950d Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 13:28:37 -0500 Subject: [PATCH 30/40] fix(auth-validator): [PM-22975] Client Version Validator - Not having the header present now blocks users from validating --- .../ClientVersionValidator.cs | 35 ++++++++++++++----- .../ClientVersionValidatorTests.cs | 7 ++-- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index 32af1dac5298..89a1f7769c85 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -18,10 +18,7 @@ public interface IClientVersionValidator /// incentivize users to get on an updated client when their password encryption /// method has already been updated. /// -/// If the header is omitted, then the validator returns that this request is valid. -/// We do this because clients can always just put whatever they want in the header, -/// and all we can do is try to prevent legitimate clients from ending up in a scenario -/// where they cannot log in due to stale encryption versions and newer client architecture. +/// If the header is omitted, then the validator returns that this request is invalid. /// public class ClientVersionValidator( ICurrentContext currentContext) @@ -29,6 +26,7 @@ public class ClientVersionValidator( { 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"; + private const string _versionHeaderMissing = "No client version header found, required to prevent encryption errors. Please confirm your client is supplying the header: \"Bitwarden-Client-Version\""; public bool ValidateAsync(User? user, CustomValidatorRequestContext requestContext) { @@ -51,13 +49,32 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte } Version? clientVersion = currentContext.ClientVersion; + + // Determine the minimum version client that a user needs. If no V2 encryption detected then + // no validation needs to occur, which is why min version number can be null. 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 - // have been determined to be null prior to reaching this point, but it is defensive programming - // to check for nullish values in case validators were to ever be reordered. - if (clientVersion == null || minVersion == null) + // Deny access if the client version headers are missing. + // We want to establish a contract with clients that if they omit this heading that they + // will be susceptible to encryption failures. + if (clientVersion == null) + { + requestContext.ValidationErrorResult = new ValidationResult + { + Error = "version_header_missing", + ErrorDescription = _versionHeaderMissing, + IsError = true + }; + requestContext.CustomResponse = new Dictionary + { + { "ErrorModel", new ErrorResponseModel(_versionHeaderMissing) } + }; + return false; + } + + // If min version is null then we know that the user had an encryption + // configuration that doesn't require a minimum version. Allowing through. + if (minVersion == null) { return true; } diff --git a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs index 6adc997f79e3..3af33d0123e6 100644 --- a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs @@ -108,7 +108,7 @@ public void Allows_When_NoSecurityVersion() } [Fact] - public void Allows_When_ClientVersionHeaderMissing() + public void Blocks_When_ClientVersionHeaderMissing() { // Arrange var sut = new ClientVersionValidator(MakeContext(null)); @@ -119,6 +119,9 @@ public void Allows_When_ClientVersionHeaderMissing() var ok = sut.ValidateAsync(user, ctx); // Assert - Assert.True(ok); + Assert.False(ok); + Assert.NotNull(ctx.ValidationErrorResult); + Assert.True(ctx.ValidationErrorResult.IsError); + Assert.Equal("version_header_missing", ctx.ValidationErrorResult.Error); } } From fb83df353c972cce981d1866f6df1c2ef701adf6 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 13:38:16 -0500 Subject: [PATCH 31/40] fix(auth-validator): [PM-22975] Client Version Validator - Minor touchups. --- .../ClientVersionValidator.cs | 55 ++++++++----------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index 89a1f7769c85..b598eb175aff 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -35,16 +35,7 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte // 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 - { - { "ErrorModel", new ErrorResponseModel(_noUserMessage) } - }; + FillContextWithErrorData(requestContext, "no_user", _noUserMessage); return false; } @@ -55,20 +46,12 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte Version? minVersion = user.HasV2Encryption() ? Constants.MinimumClientVersionForV2Encryption : null; // Deny access if the client version headers are missing. - // We want to establish a contract with clients that if they omit this heading that they - // will be susceptible to encryption failures. + // We want to establish a strict contract with clients that if they omit this header, + // then the server cannot guarantee that a client won't do harm to a user's data + // with stale encryption architecture. if (clientVersion == null) { - requestContext.ValidationErrorResult = new ValidationResult - { - Error = "version_header_missing", - ErrorDescription = _versionHeaderMissing, - IsError = true - }; - requestContext.CustomResponse = new Dictionary - { - { "ErrorModel", new ErrorResponseModel(_versionHeaderMissing) } - }; + FillContextWithErrorData(requestContext, "version_header_missing", _versionHeaderMissing); return false; } @@ -81,21 +64,29 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte if (clientVersion < minVersion) { - requestContext.ValidationErrorResult = new ValidationResult - { - Error = "invalid_client_version", - ErrorDescription = _upgradeMessage, - IsError = true - }; - requestContext.CustomResponse = new Dictionary - { - { "ErrorModel", new ErrorResponseModel(_upgradeMessage) } - }; + FillContextWithErrorData(requestContext, "invalid_client_version", _upgradeMessage); return false; } return true; } + + private void FillContextWithErrorData( + CustomValidatorRequestContext requestContext, + string errorId, + string errorMessage) + { + requestContext.ValidationErrorResult = new ValidationResult + { + Error = errorId, + ErrorDescription = errorMessage, + IsError = true + }; + requestContext.CustomResponse = new Dictionary + { + { "ErrorModel", new ErrorResponseModel(errorMessage) } + }; + } } From 10cbfd836c7e7770e9df0226568e967be7d3ae68 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 13:39:40 -0500 Subject: [PATCH 32/40] fix(auth-validator): [PM-22975] Client Version Validator - Minor function name change. --- .../RequestValidators/ClientVersionValidator.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index b598eb175aff..d34890475f2e 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -35,7 +35,7 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte // the user not nullish checked. If they are null then the validator should fail. if (user == null) { - FillContextWithErrorData(requestContext, "no_user", _noUserMessage); + FillRequestContextWithErrorData(requestContext, "no_user", _noUserMessage); return false; } @@ -51,7 +51,7 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte // with stale encryption architecture. if (clientVersion == null) { - FillContextWithErrorData(requestContext, "version_header_missing", _versionHeaderMissing); + FillRequestContextWithErrorData(requestContext, "version_header_missing", _versionHeaderMissing); return false; } @@ -64,14 +64,14 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte if (clientVersion < minVersion) { - FillContextWithErrorData(requestContext, "invalid_client_version", _upgradeMessage); + FillRequestContextWithErrorData(requestContext, "invalid_client_version", _upgradeMessage); return false; } return true; } - private void FillContextWithErrorData( + private void FillRequestContextWithErrorData( CustomValidatorRequestContext requestContext, string errorId, string errorMessage) From 79256d73ee355229ed1e9ecec4885402877449d6 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 14:34:11 -0500 Subject: [PATCH 33/40] fix(auth-validator): [PM-22975] Client Version Validator - Minor changes to test encryption constants. --- test/Common/Constants/TestEncryptionConstants.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/Common/Constants/TestEncryptionConstants.cs b/test/Common/Constants/TestEncryptionConstants.cs index 7bb3b3bb8479..c6fb7fac7873 100644 --- a/test/Common/Constants/TestEncryptionConstants.cs +++ b/test/Common/Constants/TestEncryptionConstants.cs @@ -2,17 +2,18 @@ public static class TestEncryptionConstants { - // Intended for use as a V1 encrypted string, accepted by validators - public const string AES256_CBC_HMAC_Encstring = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; - // Simple stubs for other encstring versions used by parsing tests + // Simple stubs for different encrypted string versions public const string AES256_CBC_B64_Encstring = "0.stub"; public const string AES128_CBC_HMACSHA256_B64_Encstring = "1.stub"; public const string AES256_CBC_HMAC_EmptySuffix = "2."; + // Intended for use as a V1 encrypted string, accepted by validators + public const string AES256_CBC_HMAC_Encstring = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; public const string RSA2048_OAEPSHA256_B64_Encstring = "3.stub"; public const string RSA2048_OAEPSHA1_B64_Encstring = "4.stub"; public const string RSA2048_OAEPSHA256_HMACSHA256_B64_Encstring = "5.stub"; public const string RSA2048_OAEPSHA1_HMACSHA256_B64_Encstring = "6.stub"; + public const string XCHACHA20POLY1305_B64_Encstring = "7.stub"; // Public key test placeholder public const string PublicKey = "pk_test"; From 865e76f6202453a26b2d6419607bd0af9e3a8363 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 8 Dec 2025 15:00:14 -0500 Subject: [PATCH 34/40] fix(auth-validator): [PM-22975] Client Version Validator - Reorder of client version validation. --- .../RequestValidators/ClientVersionValidator.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index d34890475f2e..7f332c899746 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -41,10 +41,6 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte Version? clientVersion = currentContext.ClientVersion; - // Determine the minimum version client that a user needs. If no V2 encryption detected then - // no validation needs to occur, which is why min version number can be null. - Version? minVersion = user.HasV2Encryption() ? Constants.MinimumClientVersionForV2Encryption : null; - // Deny access if the client version headers are missing. // We want to establish a strict contract with clients that if they omit this header, // then the server cannot guarantee that a client won't do harm to a user's data @@ -55,6 +51,10 @@ public bool ValidateAsync(User? user, CustomValidatorRequestContext requestConte return false; } + // Determine the minimum version client that a user needs. If no V2 encryption detected then + // no validation needs to occur, which is why min version number can be null. + Version? minVersion = user.HasV2Encryption() ? Constants.MinimumClientVersionForV2Encryption : null; + // If min version is null then we know that the user had an encryption // configuration that doesn't require a minimum version. Allowing through. if (minVersion == null) From 50298fbbce4227bbd5c9f90d28fcf4ae30d3edab Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 11 Dec 2025 13:07:28 -0500 Subject: [PATCH 35/40] fix(auth-validator): [PM-22975] Client Version Validator - Fixed tests and made versions of the sha obsolete --- src/Core/Enums/EncryptionType.cs | 3 +++ test/Common/Constants/TestEncryptionConstants.cs | 5 +---- .../KeyManagement/Utilities/EncryptionParsingTests.cs | 4 ---- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Core/Enums/EncryptionType.cs b/src/Core/Enums/EncryptionType.cs index 52231e047cf7..02de7c71acda 100644 --- a/src/Core/Enums/EncryptionType.cs +++ b/src/Core/Enums/EncryptionType.cs @@ -11,8 +11,11 @@ public enum EncryptionType : byte XChaCha20Poly1305_B64 = 7, // asymmetric + [Obsolete("Should probably be removed as it is not known to exist in the real world")] Rsa2048_OaepSha256_B64 = 3, Rsa2048_OaepSha1_B64 = 4, + [Obsolete("Should probably be removed as it is not known to exist in the real world")] Rsa2048_OaepSha256_HmacSha256_B64 = 5, + [Obsolete("Should probably be removed as it is not known to exist in the real world")] Rsa2048_OaepSha1_HmacSha256_B64 = 6 } diff --git a/test/Common/Constants/TestEncryptionConstants.cs b/test/Common/Constants/TestEncryptionConstants.cs index c6fb7fac7873..08022fa83d5e 100644 --- a/test/Common/Constants/TestEncryptionConstants.cs +++ b/test/Common/Constants/TestEncryptionConstants.cs @@ -4,15 +4,12 @@ public static class TestEncryptionConstants { // Simple stubs for different encrypted string versions + [Obsolete] public const string AES256_CBC_B64_Encstring = "0.stub"; - public const string AES128_CBC_HMACSHA256_B64_Encstring = "1.stub"; public const string AES256_CBC_HMAC_EmptySuffix = "2."; // Intended for use as a V1 encrypted string, accepted by validators public const string AES256_CBC_HMAC_Encstring = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; - public const string RSA2048_OAEPSHA256_B64_Encstring = "3.stub"; public const string RSA2048_OAEPSHA1_B64_Encstring = "4.stub"; - public const string RSA2048_OAEPSHA256_HMACSHA256_B64_Encstring = "5.stub"; - public const string RSA2048_OAEPSHA1_HMACSHA256_B64_Encstring = "6.stub"; public const string XCHACHA20POLY1305_B64_Encstring = "7.stub"; // Public key test placeholder diff --git a/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs b/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs index c68aa8a7a37b..9647a6ca6881 100644 --- a/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs +++ b/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs @@ -25,12 +25,8 @@ public void GetEncryptionType_WithInvalidString_ThrowsArgumentException(string i [Theory] [InlineData(TestEncryptionConstants.AES256_CBC_B64_Encstring, EncryptionType.AesCbc256_B64)] - [InlineData(TestEncryptionConstants.AES128_CBC_HMACSHA256_B64_Encstring, EncryptionType.AesCbc128_HmacSha256_B64)] [InlineData(TestEncryptionConstants.AES256_CBC_HMAC_Encstring, EncryptionType.AesCbc256_HmacSha256_B64)] - [InlineData(TestEncryptionConstants.RSA2048_OAEPSHA256_B64_Encstring, EncryptionType.Rsa2048_OaepSha256_B64)] [InlineData(TestEncryptionConstants.RSA2048_OAEPSHA1_B64_Encstring, EncryptionType.Rsa2048_OaepSha1_B64)] - [InlineData(TestEncryptionConstants.RSA2048_OAEPSHA256_HMACSHA256_B64_Encstring, EncryptionType.Rsa2048_OaepSha256_HmacSha256_B64)] - [InlineData(TestEncryptionConstants.RSA2048_OAEPSHA1_HMACSHA256_B64_Encstring, EncryptionType.Rsa2048_OaepSha1_HmacSha256_B64)] [InlineData(TestEncryptionConstants.V2PrivateKey, EncryptionType.XChaCha20Poly1305_B64)] [InlineData(TestEncryptionConstants.V2WrappedSigningKey, EncryptionType.XChaCha20Poly1305_B64)] [InlineData(TestEncryptionConstants.AES256_CBC_HMAC_EmptySuffix, EncryptionType.AesCbc256_HmacSha256_B64)] // empty suffix still valid From ad683124329bc478295b32405d4aafc058e3e329 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 11 Dec 2025 13:12:49 -0500 Subject: [PATCH 36/40] fix(auth-validator): [PM-22975] Client Version Validator - Added tickt to obsolete remarks --- src/Core/Enums/EncryptionType.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Core/Enums/EncryptionType.cs b/src/Core/Enums/EncryptionType.cs index 02de7c71acda..d910e22e56c0 100644 --- a/src/Core/Enums/EncryptionType.cs +++ b/src/Core/Enums/EncryptionType.cs @@ -11,11 +11,11 @@ public enum EncryptionType : byte XChaCha20Poly1305_B64 = 7, // asymmetric - [Obsolete("Should probably be removed as it is not known to exist in the real world")] + [Obsolete("PM-29656 - Should probably be removed as it is not known to exist in the real world")] Rsa2048_OaepSha256_B64 = 3, Rsa2048_OaepSha1_B64 = 4, - [Obsolete("Should probably be removed as it is not known to exist in the real world")] + [Obsolete("PM-29656 - Should probably be removed as it is not known to exist in the real world")] Rsa2048_OaepSha256_HmacSha256_B64 = 5, - [Obsolete("Should probably be removed as it is not known to exist in the real world")] + [Obsolete("PM-29656 - Should probably be removed as it is not known to exist in the real world")] Rsa2048_OaepSha1_HmacSha256_B64 = 6 } From 3f0d7d2b552eca2d6f6a80ec9143ebc898ed4f39 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 11 Dec 2025 13:17:05 -0500 Subject: [PATCH 37/40] test(auth-validator): [PM-22975] Client Version Validator - added one more test --- test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs b/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs index 9647a6ca6881..a4837c91d012 100644 --- a/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs +++ b/test/Core.Test/KeyManagement/Utilities/EncryptionParsingTests.cs @@ -30,6 +30,7 @@ public void GetEncryptionType_WithInvalidString_ThrowsArgumentException(string i [InlineData(TestEncryptionConstants.V2PrivateKey, EncryptionType.XChaCha20Poly1305_B64)] [InlineData(TestEncryptionConstants.V2WrappedSigningKey, EncryptionType.XChaCha20Poly1305_B64)] [InlineData(TestEncryptionConstants.AES256_CBC_HMAC_EmptySuffix, EncryptionType.AesCbc256_HmacSha256_B64)] // empty suffix still valid + [InlineData(TestEncryptionConstants.XCHACHA20POLY1305_B64_Encstring, EncryptionType.XChaCha20Poly1305_B64)] public void GetEncryptionType_WithValidString_ReturnsExpected(string input, EncryptionType expected) { var result = EncryptionParsing.GetEncryptionType(input); From 6a5518a037d30be11a6752eae5890f468bd2f803 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 15 Dec 2025 12:00:42 -0500 Subject: [PATCH 38/40] fix(auth-validator): [PM-22975] Client Version Validator - Rename function --- .../RequestValidators/BaseRequestValidator.cs | 2 +- .../RequestValidators/ClientVersionValidator.cs | 4 ++-- .../Endpoints/IdentityServerSsoTests.cs | 2 +- .../Endpoints/IdentityServerTests.cs | 2 +- .../IdentityServer/BaseRequestValidatorTests.cs | 6 +++--- .../RequestValidators/ClientVersionValidatorTests.cs | 12 ++++++------ .../Factories/IdentityApplicationFactory.cs | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 7d807d432b41..babf681c0400 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -222,7 +222,7 @@ private static async Task ProcessValidatorsAsync(params Func>[] /// true if the scheme successfully passed validation, otherwise false. private async Task ValidateClientVersionAsync(T context, CustomValidatorRequestContext validatorContext) { - var ok = _clientVersionValidator.ValidateAsync(validatorContext.User, validatorContext); + var ok = _clientVersionValidator.Validate(validatorContext.User, validatorContext); if (ok) { return true; diff --git a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs index 7f332c899746..0936b4e8ff14 100644 --- a/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs @@ -8,7 +8,7 @@ namespace Bit.Identity.IdentityServer.RequestValidators; public interface IClientVersionValidator { - bool ValidateAsync(User user, CustomValidatorRequestContext requestContext); + bool Validate(User user, CustomValidatorRequestContext requestContext); } /// @@ -28,7 +28,7 @@ public class ClientVersionValidator( private const string _noUserMessage = "No user found while trying to validate client version"; private const string _versionHeaderMissing = "No client version header found, required to prevent encryption errors. Please confirm your client is supplying the header: \"Bitwarden-Client-Version\""; - public bool ValidateAsync(User? user, CustomValidatorRequestContext requestContext) + public bool Validate(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 diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs index ade0410e70d2..1c7b035874ad 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs @@ -625,7 +625,7 @@ private static async Task CreateFactoryAsync( // Bypass client version gating to isolate SSO test behavior factory.SubstituteService(svc => { - svc.ValidateAsync(Arg.Any(), Arg.Any()) + svc.Validate(Arg.Any(), Arg.Any()) .Returns(true); }); diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs index 7b42f7464e4d..8e6079c0364d 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs @@ -36,7 +36,7 @@ public IdentityServerTests(IdentityApplicationFactory factory) // Bypass client version gating to isolate SSO test behavior _factory.SubstituteService(svc => { - svc.ValidateAsync(Arg.Any(), Arg.Any()) + svc.Validate(Arg.Any(), Arg.Any()) .Returns(true); }); diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 370ffd34aa5a..738e5527da25 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -108,7 +108,7 @@ public BaseRequestValidatorTests() // Default client version validator behavior: allow to pass unless a test overrides. _clientVersionValidator - .ValidateAsync(Arg.Any(), Arg.Any()) + .Validate(Arg.Any(), Arg.Any()) .Returns(true); } @@ -1187,7 +1187,7 @@ public async Task ValidateAsync_ClientVersionValidator_IsInvoked_ForFeatureFlagS // Make client version validation succeed but ensure it's invoked _clientVersionValidator - .ValidateAsync(requestContext.User, requestContext) + .Validate(requestContext.User, requestContext) .Returns(true); // Ensure SSO requirement triggers an early stop after version validation to avoid success path setup @@ -1200,7 +1200,7 @@ public async Task ValidateAsync_ClientVersionValidator_IsInvoked_ForFeatureFlagS // Assert _clientVersionValidator.Received(1) - .ValidateAsync(requestContext.User, requestContext); + .Validate(requestContext.User, requestContext); } /// diff --git a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs index 3af33d0123e6..51b1c73a73cc 100644 --- a/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/RequestValidators/ClientVersionValidatorTests.cs @@ -34,7 +34,7 @@ public void Allows_When_ClientMeetsMinimumVersion() var user = MakeValidV2User(); // Act - var ok = sut.ValidateAsync(user, ctx); + var ok = sut.Validate(user, ctx); // Assert Assert.True(ok); @@ -49,7 +49,7 @@ public void Blocks_When_ClientTooOld() var user = MakeValidV2User(); // Act - var ok = sut.ValidateAsync(user, ctx); + var ok = sut.Validate(user, ctx); // Assert Assert.False(ok); @@ -67,7 +67,7 @@ public void Blocks_When_NullUser() User? user = null; // Act - var ok = sut.ValidateAsync(user, ctx); + var ok = sut.Validate(user, ctx); // Assert Assert.False(ok); @@ -86,7 +86,7 @@ public void Allows_When_NoPrivateKey() user.PrivateKey = null; // Act - var ok = sut.ValidateAsync(user, ctx); + var ok = sut.Validate(user, ctx); // Assert Assert.True(ok); @@ -102,7 +102,7 @@ public void Allows_When_NoSecurityVersion() var user = MakeValidV2User(); user.SecurityVersion = null; // Act - var ok = sut.ValidateAsync(user, ctx); + var ok = sut.Validate(user, ctx); // Assert Assert.True(ok); } @@ -116,7 +116,7 @@ public void Blocks_When_ClientVersionHeaderMissing() var user = MakeValidV2User(); // Act - var ok = sut.ValidateAsync(user, ctx); + var ok = sut.Validate(user, ctx); // Assert Assert.False(ok); diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index 1304d66f67a9..fc6829e45131 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -54,7 +54,7 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) // Bypass client version gating to isolate tests from client version behavior SubstituteService(svc => { - svc.ValidateAsync(Arg.Any(), Arg.Any()) + svc.Validate(Arg.Any(), Arg.Any()) .Returns(true); }); } From 3bd54e5d2d15c18d16e982362c229d0a067141ed Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 22 Dec 2025 22:11:10 -0500 Subject: [PATCH 39/40] test(auth-validator): [PM-22975] Client Version Validator - Fixed tests. --- test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 738e5527da25..156e675a3371 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -1178,7 +1178,7 @@ public async Task ValidateAsync_ClientVersionValidator_IsInvoked_ForFeatureFlagS GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); + _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; // ensure initial context validation passes From 9a1f91f2dd2cef8a332ec456a9929b4c12022c45 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Fri, 2 Jan 2026 16:44:58 -0500 Subject: [PATCH 40/40] fix(auth-validator): [PM-22975] Client Version Validator - Fixed tests. --- .../Login/ClientVersionGateTests.cs | 23 +++++++ .../Factories/IdentityApplicationFactory.cs | 61 ++++++++++++------- 2 files changed, 61 insertions(+), 23 deletions(-) diff --git a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs index 55db8cf82ad6..a05e187c895a 100644 --- a/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs +++ b/test/Identity.IntegrationTest/Login/ClientVersionGateTests.cs @@ -13,6 +13,7 @@ namespace Bit.Identity.IntegrationTest.Login; public class ClientVersionGateTests : IClassFixture { private readonly IdentityApplicationFactory _factory; + private const string DefaultEncryptedString = "2.3Uk+WNBIoU5xzmVFNcoWzz==|1MsPIYuRfdOHfu/0uY6H2Q==|/98sp4wb6pHP1VTZ9JcNCYgQjEUMFPlqJgCwRk1YXKg="; public ClientVersionGateTests(IdentityApplicationFactory factory) { @@ -23,6 +24,7 @@ public ClientVersionGateTests(IdentityApplicationFactory factory) [Theory, BitAutoData, RegisterFinishRequestModelCustomize] public async Task TokenEndpoint_GrantTypePassword_V2User_OnOldClientVersion_Blocked(RegisterFinishRequestModel requestModel) { + NormalizeEncryptedStrings(requestModel); var localFactory = new IdentityApplicationFactory { UseMockClientVersionValidator = false @@ -72,6 +74,7 @@ public async Task TokenEndpoint_GrantTypePassword_V2User_OnOldClientVersion_Bloc [Theory, BitAutoData, RegisterFinishRequestModelCustomize] public async Task TokenEndpoint_GrantTypePassword_V2User_OnMinClientVersion_Succeeds(RegisterFinishRequestModel requestModel) { + NormalizeEncryptedStrings(requestModel); var localFactory = new IdentityApplicationFactory { UseMockClientVersionValidator = false @@ -123,4 +126,24 @@ private void ReinitializeDbForTests(IdentityApplicationFactory factory) databaseContext.Users.RemoveRange(databaseContext.Users); databaseContext.SaveChanges(); } + + private static void NormalizeEncryptedStrings(RegisterFinishRequestModel requestModel) + { + var accountKeys = requestModel.UserAsymmetricKeys.AccountKeys; + if (accountKeys == null) + { + return; + } + + accountKeys.UserKeyEncryptedAccountPrivateKey = DefaultEncryptedString; + if (accountKeys.PublicKeyEncryptionKeyPair != null) + { + accountKeys.PublicKeyEncryptionKeyPair.WrappedPrivateKey = DefaultEncryptedString; + } + if (accountKeys.SignatureKeyPair != null) + { + accountKeys.SignatureKeyPair.WrappedSigningKey = DefaultEncryptedString; + accountKeys.SignatureKeyPair.SignatureAlgorithm = "ed25519"; + } + } } diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index fc6829e45131..298fe7e4add6 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -3,6 +3,7 @@ using System.Collections.Concurrent; using System.Net.Http.Json; +using System.Text; using System.Text.Json; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Entities; @@ -25,6 +26,7 @@ public class IdentityApplicationFactory : WebApplicationFactoryBase public const string DefaultDeviceIdentifier = "92b9d953-b9b6-4eaf-9d3e-11d57144dfeb"; public const string DefaultUserEmail = "DefaultEmail@bitwarden.com"; public const string DefaultUserPasswordHash = "default_password_hash"; + private const string DefaultEncryptedString = "2.3Uk+WNBIoU5xzmVFNcoWzz==|1MsPIYuRfdOHfu/0uY6H2Q==|/98sp4wb6pHP1VTZ9JcNCYgQjEUMFPlqJgCwRk1YXKg="; public bool UseMockClientVersionValidator { get; set; } = true; /// @@ -88,20 +90,8 @@ public async Task PostRegisterVerificationEmailClicked(RegisterVeri var context = await ContextFromPasswordAsync( username, password, deviceIdentifier, clientId, deviceType, deviceName); - // Provide clearer diagnostics on failure - if (context.Response.StatusCode != StatusCodes.Status200OK) - { - var contentType = context.Response.ContentType ?? string.Empty; - if (context.Response.Body.CanSeek) - { - context.Response.Body.Position = 0; - } - string rawBody = await new StreamReader(context.Response.Body).ReadToEndAsync(); - throw new Xunit.Sdk.XunitException($"Login failed: status={context.Response.StatusCode}, contentType='{contentType}', body='{rawBody}'"); - } - - using var jsonDoc = await AssertHelper.AssertResponseTypeIs(context); - var root = jsonDoc.RootElement; + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; return (root.GetProperty("access_token").GetString(), root.GetProperty("refresh_token").GetString()); } @@ -124,13 +114,7 @@ public async Task ContextFromPasswordAsync( { "grant_type", "password" }, { "username", username }, { "password", password }, - }), - http => - { - // Ensure JSON content negotiation for errors and set a sane client version - http.Request.Headers.Append("Accept", "application/json"); - http.Request.Headers.Append("Bitwarden-Client-Version", "2025.11.0"); - }); + })); return context; } @@ -242,8 +226,11 @@ public async Task RegisterNewIdentityFactoryUserAsync( requestModel.EmailVerificationToken = RegistrationTokens[requestModel.Email]; var postRegisterFinishHttpContext = await PostRegisterFinishAsync(requestModel); - - Assert.Equal(StatusCodes.Status200OK, postRegisterFinishHttpContext.Response.StatusCode); + if (postRegisterFinishHttpContext.Response.StatusCode != StatusCodes.Status200OK) + { + var body = await ReadResponseBodyAsync(postRegisterFinishHttpContext); + Assert.Fail($"register/finish failed (status {postRegisterFinishHttpContext.Response.StatusCode}). Body: {body}"); + } var database = GetDatabaseContext(); var user = await database.Users @@ -253,4 +240,32 @@ public async Task RegisterNewIdentityFactoryUserAsync( return user; } + + private static async Task ReadResponseBodyAsync(HttpContext ctx) + { + try + { + if (ctx?.Response.Body == null) + { + return ""; + } + var stream = ctx.Response.Body; + if (stream.CanSeek) + { + stream.Seek(0, SeekOrigin.Begin); + } + using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: false, leaveOpen: true); + var text = await reader.ReadToEndAsync(); + if (stream.CanSeek) + { + stream.Seek(0, SeekOrigin.Begin); + } + return string.IsNullOrWhiteSpace(text) ? "" : text; + } + catch (Exception ex) + { + return $""; + } + } + }