From 7a0e73f6df57fe378cb0ea84f475ea0aaa32ab8f Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:51:05 -0500 Subject: [PATCH 01/19] [PM-40673 Part 2] update validators --- .../RequestValidators/BaseRequestValidator.cs | 16 +- .../CustomTokenRequestValidator.cs | 7 +- .../ITwoFactorAuthenticationValidator.cs | 21 ++- .../ResourceOwnerPasswordValidator.cs | 7 +- .../TwoFactorAuthenticationValidator.cs | 41 +++-- .../WebAuthnGrantValidator.cs | 7 +- .../Endpoints/RememberedDeviceLoginTests.cs | 151 ++++++++++++++++++ .../BaseRequestValidatorTests.cs | 18 ++- .../CustomTokenRequestValidatorTests.cs | 8 +- .../TwoFactorAuthenticationValidatorTests.cs | 62 +++++-- .../BaseRequestValidatorTestWrapper.cs | 7 +- .../Factories/IdentityApplicationFactory.cs | 66 +++++++- 12 files changed, 354 insertions(+), 57 deletions(-) create mode 100644 test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 6fafe1453c01..81b63f8603b1 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -12,6 +12,7 @@ using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -42,6 +43,7 @@ public abstract class BaseRequestValidator where T : class private readonly IMailService _mailService; private readonly IClientVersionValidator _clientVersionValidator; protected readonly IUpdateDeviceLastActivityCommand _updateDeviceLastActivityCommand; + private readonly IIssueTwoFactorRememberTokenCommand _issueTwoFactorRememberTokenCommand; protected ICurrentContext CurrentContext { get; } protected Bitwarden.Server.Sdk.Features.IFeatureService _featureService { get; } @@ -70,7 +72,8 @@ public BaseRequestValidator( IMailService mailService, IUserAccountKeysQuery userAccountKeysQuery, IClientVersionValidator clientVersionValidator, - IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand + IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand, + IIssueTwoFactorRememberTokenCommand issueTwoFactorRememberTokenCommand ) { _userManager = userManager; @@ -92,6 +95,7 @@ IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand _accountKeysQuery = userAccountKeysQuery; _clientVersionValidator = clientVersionValidator; _updateDeviceLastActivityCommand = updateDeviceLastActivityCommand; + _issueTwoFactorRememberTokenCommand = issueTwoFactorRememberTokenCommand; } // NOTE: Feature flags with progressive rollout (device-keyed or user-keyed) cannot be @@ -321,10 +325,11 @@ private async Task ValidateTwoFactorAsync(T context, ValidatedTokenRequest return false; } - var twoFactorTokenValid = + var twoFactorResult = await _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(validatorContext.User, twoFactorOrganization, twoFactorProviderType, twoFactorToken, request.Raw["DeviceIdentifier"]); + var twoFactorTokenValid = twoFactorResult.Succeeded; // 3b. Response for 2FA required but request is not valid or remember token expired state. if (!twoFactorTokenValid) @@ -654,9 +659,10 @@ private async Task> BuildCustomResponse(User user, T if (sendRememberToken) { - var token = await _userManager.GenerateTwoFactorTokenAsync(user, - CoreHelpers.CustomProviderName(TwoFactorProviderType.Remember)); - customResponse.Add("TwoFactorToken", token); + // The device is non-null here even though the signature allows null: this runs only + // after device validation succeeded, and DeviceValidator assigns (and saves, for a new + // device) the row on every success path. + customResponse.Add("TwoFactorToken", await _issueTwoFactorRememberTokenCommand.IssueAsync(user, device)); } return customResponse; diff --git a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs index 5f6dd5aa9622..7be0a0a25653 100644 --- a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs @@ -3,6 +3,7 @@ using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.Auth.Identity; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.IdentityServer; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; @@ -47,7 +48,8 @@ public CustomTokenRequestValidator( IMailService mailService, IUserAccountKeysQuery userAccountKeysQuery, IClientVersionValidator clientVersionValidator, - IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand) + IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand, + IIssueTwoFactorRememberTokenCommand issueTwoFactorRememberTokenCommand) : base( userManager, userService, @@ -67,7 +69,8 @@ public CustomTokenRequestValidator( mailService, userAccountKeysQuery, clientVersionValidator, - updateDeviceLastActivityCommand) + updateDeviceLastActivityCommand, + issueTwoFactorRememberTokenCommand) { _userManager = userManager; _updateInstallationCommand = updateInstallationCommand; diff --git a/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs index 29dda3570f2b..44a4b8b2bd2d 100644 --- a/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs @@ -34,9 +34,22 @@ public interface ITwoFactorAuthenticationValidator /// Two Factor Provider to use to verify the token /// secret passed from the user and consumed by the two-factor provider's verify method /// - /// identifier of the device making the request; an email code only verifies for the device it was issued to + /// The client-generated device identifier from the raw request. An email code only verifies for the + /// device it was issued to, and the Remember provider binds its token to a device. It is read from + /// the request rather than from CurrentContext or the validated device: device validation runs + /// after this point, so neither is populated yet. /// - /// boolean - Task VerifyTwoFactorAsync(User user, Organization organization, TwoFactorProviderType twoFactorProviderType, - string token, string deviceIdentifier); + Task VerifyTwoFactorAsync( + User user, + Organization organization, + TwoFactorProviderType twoFactorProviderType, + string token, + string deviceIdentifier); } + +/// Whether the presented second factor was accepted. +/// +/// Set when a remember token in the pre-TwoFactorRememberTokenable format was accepted, so +/// the caller knows to issue a replacement in the current format on this response. +/// +public record TwoFactorVerificationResult(bool Succeeded, bool LegacyRememberUpgradeRequired = false); diff --git a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs index 03055079f424..c77f7df69756 100644 --- a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs @@ -5,6 +5,7 @@ using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.Auth.Repositories; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; using Bit.Core.Context; using Bit.Core.Entities; @@ -44,7 +45,8 @@ public ResourceOwnerPasswordValidator( IMailService mailService, IUserAccountKeysQuery userAccountKeysQuery, IClientVersionValidator clientVersionValidator, - IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand) + IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand, + IIssueTwoFactorRememberTokenCommand issueTwoFactorRememberTokenCommand) : base( userManager, userService, @@ -64,7 +66,8 @@ public ResourceOwnerPasswordValidator( mailService, userAccountKeysQuery, clientVersionValidator, - updateDeviceLastActivityCommand) + updateDeviceLastActivityCommand, + issueTwoFactorRememberTokenCommand) { _userManager = userManager; _currentContext = currentContext; diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index 0ddbd1a7c18e..957f0ef2e2bd 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -9,6 +9,7 @@ using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Services; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Context; using Bit.Core.Entities; @@ -32,8 +33,12 @@ public class TwoFactorAuthenticationValidator( IDataProtectorTokenFactory ssoEmail2faSessionTokeFactory, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, ICurrentContext currentContext, - ITwoFactorEmailService twoFactorEmailService) : ITwoFactorAuthenticationValidator + ITwoFactorEmailService twoFactorEmailService, + IValidateTwoFactorRememberTokenQuery validateTwoFactorRememberTokenQuery) : ITwoFactorAuthenticationValidator { + private readonly IValidateTwoFactorRememberTokenQuery _validateTwoFactorRememberTokenQuery = + validateTwoFactorRememberTokenQuery; + private readonly IUserService _userService = userService; private readonly UserManager _userManager = userManager; private readonly IOrganizationDuoUniversalTokenProvider _organizationDuoUniversalTokenProvider = organizationDuoWebTokenProvider; @@ -120,7 +125,7 @@ public async Task> BuildTwoFactorResultAsync(User use return twoFactorResultDict; } - public async Task VerifyTwoFactorAsync( + public async Task VerifyTwoFactorAsync( User user, Organization organization, TwoFactorProviderType type, @@ -131,14 +136,15 @@ public async Task VerifyTwoFactorAsync( { if (organization.TwoFactorProviderIsEnabled(type)) { - return await _organizationDuoUniversalTokenProvider.ValidateAsync(token, organization, user); + return new TwoFactorVerificationResult( + await _organizationDuoUniversalTokenProvider.ValidateAsync(token, organization, user)); } - return false; + return new TwoFactorVerificationResult(false); } if (type is TwoFactorProviderType.RecoveryCode) { - return await _userService.RecoverTwoFactorAsync(user, token); + return new TwoFactorVerificationResult(await _userService.RecoverTwoFactorAsync(user, token)); } // These cases we want to always return false, U2f is deprecated and OrganizationDuo @@ -147,31 +153,36 @@ public async Task VerifyTwoFactorAsync( // provider flow. See IOrganizationDuoUniversalTokenProvider.cs if (type is TwoFactorProviderType.U2f or TwoFactorProviderType.OrganizationDuo) { - return false; + return new TwoFactorVerificationResult(false); + } + + // Remember is not a provider a user configures, so it has no entry to look up in the checks + // below. It carries its own server-side state and is validated against that instead. + if (type is TwoFactorProviderType.Remember) + { + return new TwoFactorVerificationResult( + await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, deviceIdentifier, token)); } // Now we are concerning the rest of the Two Factor Provider Types // The intent of this check is to make sure that the user is using a 2FA provider that // is enabled and allowed by their premium status. - // The exception for Remember is because it is a "special" 2FA type that isn't ever explicitly - // enabled by a user, so we can't check the user's 2FA providers to see if they're - // enabled. We just have to check if the token is valid. - if (type != TwoFactorProviderType.Remember && - user.GetTwoFactorProvider(type) == null) + if (user.GetTwoFactorProvider(type) == null) { - return false; + return new TwoFactorVerificationResult(false); } // Email codes are bound to the device they were issued to, so they are verified with that device. if (type == TwoFactorProviderType.Email) { - return await _twoFactorEmailService.VerifyTwoFactorLoginTokenAsync(user, deviceIdentifier, token); + return new TwoFactorVerificationResult( + await _twoFactorEmailService.VerifyTwoFactorLoginTokenAsync(user, deviceIdentifier, token)); } // Finally, verify the token based on the provider type. - return await _userManager.VerifyTwoFactorTokenAsync( - user, CoreHelpers.CustomProviderName(type), token); + return new TwoFactorVerificationResult( + await _userManager.VerifyTwoFactorTokenAsync(user, CoreHelpers.CustomProviderName(type), token)); } private async Task>> GetEnabledTwoFactorProvidersAsync( diff --git a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs index 8c03677d2dd3..df55f6caf1bf 100644 --- a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs @@ -3,6 +3,7 @@ using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.Auth.Enums; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; @@ -50,7 +51,8 @@ public WebAuthnGrantValidator( IMailService mailService, IUserAccountKeysQuery userAccountKeysQuery, IClientVersionValidator clientVersionValidator, - IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand) + IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand, + IIssueTwoFactorRememberTokenCommand issueTwoFactorRememberTokenCommand) : base( userManager, userService, @@ -70,7 +72,8 @@ public WebAuthnGrantValidator( mailService, userAccountKeysQuery, clientVersionValidator, - updateDeviceLastActivityCommand) + updateDeviceLastActivityCommand, + issueTwoFactorRememberTokenCommand) { _assertionOptionsDataProtector = assertionOptionsDataProtector; _assertWebAuthnLoginCredentialCommand = assertWebAuthnLoginCredentialCommand; diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs new file mode 100644 index 000000000000..ed36912e1a63 --- /dev/null +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs @@ -0,0 +1,151 @@ +using System.Text; +using System.Text.Json; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.KeyManagement.Kdf; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Bit.IntegrationTestCommon.Factories; +using Bit.Test.Common.Helpers; +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; +using Xunit; + +namespace Bit.Identity.IntegrationTest.Endpoints; + +/// +/// End-to-end coverage for the "remember this device" flow: a token issued by one login is honored +/// by the next, and is refused when it should not be. +/// +public class RememberedDeviceLoginTests +{ + private const string _testEmail = "test+rememberdevice@email.com"; + private const string _testPassword = "master_password_hash"; + private const string _emailToken = "12345678"; + private const string _userEmailTwoFactor = + """{"1": { "Enabled": true, "MetaData": { "Email": "test+rememberdevice@email.com"}}}"""; + + /// + /// The numeric value of . Sent as a number rather + /// than the name because the validator-order selection parses it with int.TryParse. + /// + private const string _rememberProvider = "5"; + + private const string _emailProvider = "1"; + + private static async Task<(IdentityApplicationFactory Factory, User User)> CreateFactoryWithTwoFactorUserAsync() + { + var factory = new IdentityApplicationFactory(); + + // The Email provider reads its one-time code from the distributed cache; pinning it lets the + // test complete a real two-factor login. + factory.SubstituteService(distCache => + { + distCache.GetAsync(Arg.Is(s => s.StartsWith("EmailToken_"))) + .Returns(Task.FromResult(Encoding.UTF8.GetBytes(_emailToken))); + }); + + var user = await factory.RegisterNewIdentityFactoryUserAsync( + new RegisterFinishRequestModel + { + Email = _testEmail, + MasterPasswordHash = _testPassword, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = KdfConstants.PBKDF2_ITERATIONS.Default, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey, + EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring + }, + UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring, + }); + + var userRepository = factory.Services.GetRequiredService(); + var userService = factory.GetService(); + user.TwoFactorProviders = _userEmailTwoFactor; + await userRepository.ReplaceAsync(user); + await userService.GetUserByIdAsync(user.Id); + + return (factory, user); + } + + /// + /// H1 — a two-factor login that asks to be remembered gets a token back. + /// + [Fact] + public async Task RememberRequested_ResponseCarriesRememberToken() + { + var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + + var (accessToken, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + + Assert.NotNull(accessToken); + Assert.False(string.IsNullOrEmpty(rememberToken)); + } + + /// + /// H2 — the token is honored on a later login, with no second factor supplied. This is the + /// behavior the whole feature exists to preserve; it must stay green through every change to the + /// token format. + /// + [Fact] + public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() + { + var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + + var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + Assert.False(string.IsNullOrEmpty(rememberToken)); + + // Clients send remember=0 when re-authenticating with a stored token. + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: rememberToken!, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + var accessToken = AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String).GetString(); + Assert.False(string.IsNullOrEmpty(accessToken)); + } + + /// + /// H3 — the token is bound to the device it was issued to, so presenting it from another device + /// falls back to a two-factor challenge. + /// + [Fact] + public async Task RememberToken_PresentedFromDifferentDevice_IsRefused() + { + var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + + var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + Assert.False(string.IsNullOrEmpty(rememberToken)); + + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + deviceIdentifier: Guid.NewGuid().ToString(), + twoFactorProviderType: _rememberProvider, + twoFactorToken: rememberToken!, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + // A refused remember token falls back to a challenge rather than a generic auth failure: + // the password was fine, the device just is not trusted. + Assert.False(root.TryGetProperty("access_token", out _)); + var error = AssertHelper.AssertJsonProperty(root, "error_description", JsonValueKind.String).GetString(); + Assert.Equal("Two factor required.", error); + AssertHelper.AssertJsonProperty(root, "TwoFactorProviders2", JsonValueKind.Object); + } +} diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index f9bff31a9736..921449badb77 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -1,4 +1,5 @@ using Bit.Core; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; @@ -60,6 +61,7 @@ public class BaseRequestValidatorTests private readonly IUserAccountKeysQuery _userAccountKeysQuery; private readonly IClientVersionValidator _clientVersionValidator; private readonly IUpdateDeviceLastActivityCommand _updateDeviceLastActivityCommand; + private readonly IIssueTwoFactorRememberTokenCommand _issueTwoFactorRememberTokenCommand; private readonly BaseRequestValidatorTestWrapper _sut; @@ -84,6 +86,7 @@ public BaseRequestValidatorTests() _userAccountKeysQuery = Substitute.For(); _clientVersionValidator = Substitute.For(); _updateDeviceLastActivityCommand = Substitute.For(); + _issueTwoFactorRememberTokenCommand = Substitute.For(); _sut = new BaseRequestValidatorTestWrapper( _userManager, @@ -104,7 +107,8 @@ public BaseRequestValidatorTests() _mailService, _userAccountKeysQuery, _clientVersionValidator, - _updateDeviceLastActivityCommand); + _updateDeviceLastActivityCommand, + _issueTwoFactorRememberTokenCommand); // Default client version validator behavior: allow to pass unless a test overrides. _clientVersionValidator @@ -397,7 +401,7 @@ public async Task ValidateAsync_TwoFactorTokenInvalid_ShouldSendFailedTwoFactorE // 4 -> set up 2FA verification to fail _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.Email, "invalid_token", "requesting-device") - .Returns(Task.FromResult(false)); + .Returns(Task.FromResult(new TwoFactorVerificationResult(false))); // 5 -> set up SSO required verification to succeed _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) @@ -450,7 +454,7 @@ public async Task ValidateAsync_TwoFactorRememberTokenExpired_ShouldNotSendFaile // 5 -> set up remember token verification to fail _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.Remember, "expired_remember_token", Arg.Any()) - .Returns(Task.FromResult(false)); + .Returns(Task.FromResult(new TwoFactorVerificationResult(false))); // 6 -> set up dummy BuildTwoFactorResultAsync var twoFactorResultDict = new Dictionary @@ -987,7 +991,7 @@ public async Task ValidateAsync_RecoveryCodeForSsoRequiredUser_BlocksWithDescrip // 5. Recovery code is valid (UserService.RecoverTwoFactorAsync will be called internally) _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.RecoveryCode, "valid-recovery-code-12345", Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(new TwoFactorVerificationResult(true))); // Act await _sut.ValidateAsync(context); @@ -1051,7 +1055,7 @@ public async Task ValidateAsync_InvalidRecoveryCodeForSsoRequiredUser_FailsAt2FA // 5. Recovery code is INVALID _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.RecoveryCode, "INVALID-recovery-code", Arg.Any()) - .Returns(Task.FromResult(false)); + .Returns(Task.FromResult(new TwoFactorVerificationResult(false))); // Act await _sut.ValidateAsync(context); @@ -1130,7 +1134,7 @@ public async Task ValidateAsync_RecoveryCodeForNonSsoUser_SuccessfulLogin( // 5. Recovery code is valid _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.RecoveryCode, "valid-recovery-code-67890", Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(new TwoFactorVerificationResult(true))); // 6. Device validation passes _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -1275,7 +1279,7 @@ public async Task ValidateAsync_RecoveryWithSso_CorrectValidatorMessage( _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(requestContext.User, null, TwoFactorProviderType.RecoveryCode, "valid-recovery-code", Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(new TwoFactorVerificationResult(true))); // SsoRequestValidator handles the recovery + SSO scenario requestContext.TwoFactorRecoveryRequested = true; diff --git a/test/Identity.Test/IdentityServer/CustomTokenRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/CustomTokenRequestValidatorTests.cs index 4220c9d20798..27ba6672c1dc 100644 --- a/test/Identity.Test/IdentityServer/CustomTokenRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/CustomTokenRequestValidatorTests.cs @@ -2,6 +2,7 @@ using System.Security.Claims; using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.Identity; using Bit.Core.Auth.IdentityServer; using Bit.Core.Auth.Repositories; @@ -77,7 +78,8 @@ public CustomTokenRequestValidatorTests() Substitute.For(), Substitute.For(), Substitute.For(), - _updateDeviceLastActivityCommand); + _updateDeviceLastActivityCommand, + Substitute.For()); } private CustomTokenRequestValidationContext CreateRefreshTokenContext(ClaimsPrincipal subject) @@ -261,6 +263,7 @@ public async Task TryUpdateDeviceLastActivityForRefreshAsync_PassesClientVersion Substitute.For>>()); var updateCmd = Substitute.For(); + var issueRememberCmd = Substitute.For(); var sut = new CustomTokenRequestValidator( userManager, @@ -282,7 +285,8 @@ public async Task TryUpdateDeviceLastActivityForRefreshAsync_PassesClientVersion Substitute.For(), Substitute.For(), Substitute.For(), - updateCmd); + updateCmd, + issueRememberCmd); var subject = new ClaimsPrincipal(new ClaimsIdentity( [ diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index e49cc6b0bfce..80f0567dc134 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -5,6 +5,7 @@ using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Services; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Context; using Bit.Core.Entities; @@ -29,6 +30,7 @@ namespace Bit.Identity.Test.IdentityServer; public class TwoFactorAuthenticationValidatorTests { + private const string DeviceIdentifier = "test-device-identifier"; private readonly IUserService _userService; private readonly UserManagerTestWrapper _userManager; private readonly IOrganizationDuoUniversalTokenProvider _organizationDuoUniversalTokenProvider; @@ -37,12 +39,11 @@ public class TwoFactorAuthenticationValidatorTests private readonly IOrganizationRepository _organizationRepository; private readonly IDataProtectorTokenFactory _ssoEmail2faSessionTokenable; private readonly ITwoFactorIsEnabledQuery _twoFactorEnabledQuery; + private readonly IValidateTwoFactorRememberTokenQuery _validateTwoFactorRememberTokenQuery; private readonly ICurrentContext _currentContext; private readonly ITwoFactorEmailService _twoFactorEmailService; private readonly TwoFactorAuthenticationValidator _sut; - private const string DeviceIdentifier = "device-identifier"; - public TwoFactorAuthenticationValidatorTests() { _userService = Substitute.For(); @@ -53,6 +54,7 @@ public TwoFactorAuthenticationValidatorTests() _organizationRepository = Substitute.For(); _ssoEmail2faSessionTokenable = Substitute.For>(); _twoFactorEnabledQuery = Substitute.For(); + _validateTwoFactorRememberTokenQuery = Substitute.For(); _currentContext = Substitute.For(); _twoFactorEmailService = Substitute.For(); @@ -66,7 +68,8 @@ public TwoFactorAuthenticationValidatorTests() _ssoEmail2faSessionTokenable, _twoFactorEnabledQuery, _currentContext, - _twoFactorEmailService); + _twoFactorEmailService, + _validateTwoFactorRememberTokenQuery); } [Theory] @@ -411,7 +414,7 @@ public async void VerifyTwoFactorAsync_Individual_TypeNull_ReturnsFalse( user, null, TwoFactorProviderType.U2f, token, DeviceIdentifier); // Assert - Assert.False(result); + Assert.False(result.Succeeded); } [Theory] @@ -429,7 +432,7 @@ public async void VerifyTwoFactorAsync_Individual_NotEnabled_ReturnsFalse( user, null, TwoFactorProviderType.Email, token, DeviceIdentifier); // Assert - Assert.False(result); + Assert.False(result.Succeeded); await _twoFactorEmailService.DidNotReceiveWithAnyArgs().VerifyTwoFactorLoginTokenAsync(default, default, default); } @@ -447,14 +450,13 @@ public async void VerifyTwoFactorAsync_Organization_NotEnabled_ReturnsFalse( user, null, TwoFactorProviderType.OrganizationDuo, token, DeviceIdentifier); // Assert - Assert.False(result); + Assert.False(result.Succeeded); } [Theory] [BitAutoData(TwoFactorProviderType.Duo)] [BitAutoData(TwoFactorProviderType.WebAuthn)] [BitAutoData(TwoFactorProviderType.YubiKey)] - [BitAutoData(TwoFactorProviderType.Remember)] public async void VerifyTwoFactorAsync_Individual_ValidToken_ReturnsTrue( TwoFactorProviderType providerType, User user, @@ -469,14 +471,46 @@ public async void VerifyTwoFactorAsync_Individual_ValidToken_ReturnsTrue( var result = await _sut.VerifyTwoFactorAsync(user, null, providerType, token, DeviceIdentifier); // Assert - Assert.True(result); + Assert.True(result.Succeeded); + } + + /// + /// Remember is not a provider a user configures, so it does not go through the provider-enabled + /// checks or UserManager. It is validated against its own server-side state instead. + /// + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(User user, string token) + { + // Arrange + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, null, TwoFactorProviderType.Remember, token, DeviceIdentifier); + + // Assert + Assert.True(result.Succeeded); + await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, DeviceIdentifier, token); + } + + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_Remember_QueryRejects_ReturnsFalse(User user, string token) + { + // Arrange + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(false); + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, null, TwoFactorProviderType.Remember, token, DeviceIdentifier); + + // Assert + Assert.False(result.Succeeded); } [Theory] [BitAutoData(TwoFactorProviderType.Duo)] [BitAutoData(TwoFactorProviderType.WebAuthn)] [BitAutoData(TwoFactorProviderType.YubiKey)] - [BitAutoData(TwoFactorProviderType.Remember)] public async void VerifyTwoFactorAsync_Individual_InvalidToken_ReturnsFalse( TwoFactorProviderType providerType, User user, @@ -491,7 +525,7 @@ public async void VerifyTwoFactorAsync_Individual_InvalidToken_ReturnsFalse( var result = await _sut.VerifyTwoFactorAsync(user, null, providerType, token, DeviceIdentifier); // Assert - Assert.False(result); + Assert.False(result.Succeeded); } /// @@ -517,7 +551,7 @@ public async void VerifyTwoFactorAsync_Email_ReturnsEmailServiceResultForRequest user, null, TwoFactorProviderType.Email, token, DeviceIdentifier); // Assert - Assert.Equal(emailServiceResult, result); + Assert.Equal(emailServiceResult, result.Succeeded); } [Theory] @@ -544,7 +578,7 @@ public async void VerifyTwoFactorAsync_Organization_ValidToken_ReturnsTrue( user, organization, providerType, token, DeviceIdentifier); // Assert - Assert.True(result); + Assert.True(result.Succeeded); } [Theory] @@ -564,7 +598,7 @@ public async void VerifyTwoFactorAsync_RecoveryCode_ValidToken_ReturnsTrue( user, organization, providerType, token, DeviceIdentifier); // Assert - Assert.True(result); + Assert.True(result.Succeeded); } [Theory] @@ -585,7 +619,7 @@ public async void VerifyTwoFactorAsync_RecoveryCode_InvalidToken_ReturnsFalse( user, organization, providerType, token, DeviceIdentifier); // Assert - Assert.False(result); + Assert.False(result.Succeeded); } private static UserManagerTestWrapper SubstituteUserManager() diff --git a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs index 310c98d14301..bde53c92c171 100644 --- a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs +++ b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs @@ -1,5 +1,6 @@ using System.Security.Claims; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; using Bit.Core.Context; @@ -67,7 +68,8 @@ public BaseRequestValidatorTestWrapper( IMailService mailService, IUserAccountKeysQuery userAccountKeysQuery, IClientVersionValidator clientVersionValidator, - IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand) : + IUpdateDeviceLastActivityCommand updateDeviceLastActivityCommand, + IIssueTwoFactorRememberTokenCommand issueTwoFactorRememberTokenCommand) : base( userManager, userService, @@ -87,7 +89,8 @@ public BaseRequestValidatorTestWrapper( mailService, userAccountKeysQuery, clientVersionValidator, - updateDeviceLastActivityCommand) + updateDeviceLastActivityCommand, + issueTwoFactorRememberTokenCommand) { } diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index 45cd74dec7e3..2b4fdd0f4d65 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -4,12 +4,14 @@ using System.Net.Http.Json; using System.Text; using System.Text.Json; +using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.KeyManagement.Kdf; using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Core.Services; +using Bit.Core.Utilities; using Bit.Identity; using Bit.Identity.IdentityServer; using Bit.Identity.IdentityServer.RequestValidators; @@ -17,6 +19,7 @@ using Bit.Test.Common.Helpers; using Core.Auth.Enums; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Http; using Microsoft.EntityFrameworkCore; using NSubstitute; @@ -150,6 +153,16 @@ public async Task ContextFromPasswordAsync( return context; } + /// + /// Send the numeric provider value rather than the enum name where the distinction matters: + /// selects the validator order with + /// int.TryParse, while ValidateTwoFactorAsync parses the same value with + /// Enum.TryParse. A name therefore authenticates but silently takes the ordinary order. + /// + /// + /// "1" asks the server to remember the device. Pass "0" to mirror a client re-authenticating + /// with a stored remember token, which is what clients actually send in that case. + /// public async Task ContextFromPasswordWithTwoFactorAsync( string username, string password, @@ -158,7 +171,8 @@ public async Task ContextFromPasswordWithTwoFactorAsync( DeviceType deviceType = DeviceType.FirefoxBrowser, string deviceName = "firefox", string twoFactorProviderType = "Email", - string twoFactorToken = "two-factor-token") + string twoFactorToken = "two-factor-token", + string twoFactorRemember = "1") { var context = await Server.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary { @@ -172,12 +186,60 @@ public async Task ContextFromPasswordWithTwoFactorAsync( { "password", password }, { "TwoFactorToken", twoFactorToken }, { "TwoFactorProvider", twoFactorProviderType }, - { "TwoFactorRemember", "1" }, + { "TwoFactorRemember", twoFactorRemember }, })); return context; } + /// + /// Completes a two-factor login and returns the access token alongside the TwoFactorToken + /// response field, which is the remember token when the server issued one. + /// + /// + /// RememberToken is null when the response carried no TwoFactorToken field at all — + /// a meaningful outcome in its own right, not just an absent value. + /// + public async Task<(string AccessToken, string? RememberToken)> TokensFromPasswordWithTwoFactorAsync( + string username, + string password, + string deviceIdentifier = DefaultDeviceIdentifier, + string clientId = "web", + DeviceType deviceType = DeviceType.FirefoxBrowser, + string deviceName = "firefox", + string twoFactorProviderType = "Email", + string twoFactorToken = "two-factor-token", + string twoFactorRemember = "1") + { + var context = await ContextFromPasswordWithTwoFactorAsync( + username, password, deviceIdentifier, clientId, deviceType, deviceName, + twoFactorProviderType, twoFactorToken, twoFactorRemember); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + Debug.Assert(root.TryGetProperty("access_token", out var accessToken)); + var accessTokenString = accessToken.GetString(); + Debug.Assert(accessTokenString != null); + + var rememberToken = root.TryGetProperty("TwoFactorToken", out var twoFactorTokenElement) + ? twoFactorTokenElement.GetString() + : null; + + return (accessTokenString, rememberToken); + } + + /// + /// Mints a remember token the way the server did before this feature existed, for tests that + /// need to present one issued by a prior release. + /// + public async Task GenerateLegacyRememberTokenAsync(User user) + { + var userManager = GetService>(); + return await userManager.GenerateTwoFactorTokenAsync( + user, CoreHelpers.CustomProviderName(TwoFactorProviderType.Remember)); + } + public async Task TokenFromAccessTokenAsync(Guid clientId, string clientSecret, DeviceType deviceType = DeviceType.SDK) { From ba3e8c6ef97a31c68c322c92463974ccd6f29bc3 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:02:59 -0500 Subject: [PATCH 02/19] [PM-40673 Part 2] accept old tokens and automatically upgrade to new tokens --- .../CustomValidatorRequestContext.cs | 10 ++ .../RequestValidators/BaseRequestValidator.cs | 7 +- .../TwoFactorAuthenticationValidator.cs | 24 +++- .../Endpoints/RememberedDeviceLoginTests.cs | 91 +++++++++++++++ .../TwoFactorAuthenticationValidatorTests.cs | 106 +++++++++++++++++- 5 files changed, 233 insertions(+), 5 deletions(-) diff --git a/src/Identity/IdentityServer/CustomValidatorRequestContext.cs b/src/Identity/IdentityServer/CustomValidatorRequestContext.cs index e16c8ad69553..feb83f624e48 100644 --- a/src/Identity/IdentityServer/CustomValidatorRequestContext.cs +++ b/src/Identity/IdentityServer/CustomValidatorRequestContext.cs @@ -57,4 +57,14 @@ public class CustomValidatorRequestContext /// Whether the user has requested a Remember Me token for their current device. /// public bool RememberMeRequested { get; set; } = false; + /// + /// Whether the Remember Me token presented on this request was in the previous format and should + /// be replaced with one in the current format on this response. + /// + /// + /// Distinct from on purpose. It is set only when a + /// previous-format token was accepted, never merely because the request used the Remember + /// provider — which is what keeps a remember-login from extending its own lifetime. + /// + public bool RememberUpgradeRequired { get; set; } = false; } diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 81b63f8603b1..1b8cef57bcda 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -117,8 +117,12 @@ protected async Task ValidateAsync(T context, ValidatedTokenRequest request, return; } + // A remember token is issued either because the user asked for one, or because they + // presented one in the previous format and it is being replaced. The second condition is + // set only on a successful previous-format validation — never merely because the request + // used the Remember provider, which would let an active user's remember-me renew forever. await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.Device, - validatorContext.RememberMeRequested); + validatorContext.RememberMeRequested || validatorContext.RememberUpgradeRequired); } protected async Task FailAuthForLegacyUserAsync(User user, T context) @@ -330,6 +334,7 @@ await _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(validatorContext.User, twoFactorOrganization, twoFactorProviderType, twoFactorToken, request.Raw["DeviceIdentifier"]); var twoFactorTokenValid = twoFactorResult.Succeeded; + validatorContext.RememberUpgradeRequired = twoFactorResult.LegacyRememberUpgradeRequired; // 3b. Response for 2FA required but request is not valid or remember token expired state. if (!twoFactorTokenValid) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index 957f0ef2e2bd..5b31dc2c27c0 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -160,8 +160,28 @@ public async Task VerifyTwoFactorAsync( // below. It carries its own server-side state and is validated against that instead. if (type is TwoFactorProviderType.Remember) { - return new TwoFactorVerificationResult( - await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, deviceIdentifier, token)); + // A remember token stands in for a second factor, so it cannot be honored by an account + // that has none configured. Applied ahead of the format branch so it covers both. + if (!await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) + { + return new TwoFactorVerificationResult(false); + } + + if (token?.StartsWith(TwoFactorRememberTokenable.ClearTextPrefix, StringComparison.Ordinal) == true) + { + return new TwoFactorVerificationResult( + await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, deviceIdentifier, token)); + } + + // TODO: PM-XXXXX - Remove the legacy token path below once every token issued before this + // release has expired. Both formats validate until then. The token lifespan bounds that at + // 30 days from the release date, so no token predating it can validate after that point. + var legacyTokenValid = await _userManager.VerifyTwoFactorTokenAsync( + user, CoreHelpers.CustomProviderName(type), token); + + // Accepting one earns a replacement in the current format on this same response, so the + // device moves over on this login rather than waiting out the old token's lifetime. + return new TwoFactorVerificationResult(legacyTokenValid, legacyTokenValid); } // Now we are concerning the rest of the Two Factor Provider Types diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs index ed36912e1a63..859a967ca151 100644 --- a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs @@ -2,6 +2,7 @@ using System.Text.Json; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.KeyManagement.Kdf; @@ -117,6 +118,96 @@ public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() Assert.False(string.IsNullOrEmpty(accessToken)); } + /// + /// H6 — a login that presents a remember token must not be issued another one. + /// + /// + /// This is what bounds the remember-me lifetime. The token's lifespan is an absolute cap only + /// because the server refuses to re-issue on a remember-login; if it did re-issue, an active + /// user's remember-me would be extended on every login and never expire. Asserted over HTTP + /// deliberately — it inspects the same response field a client would. + /// + [Fact] + public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken() + { + var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + + var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + Assert.False(string.IsNullOrEmpty(rememberToken)); + + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: rememberToken!, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); + Assert.False(root.TryGetProperty("TwoFactorToken", out _)); + } + + /// + /// H4 — a token minted the way a previous release minted them is still accepted, so nobody is + /// challenged again purely because this release shipped. + /// + [Fact] + public async Task LegacyRememberToken_IsStillAccepted() + { + var (factory, user) = await CreateFactoryWithTwoFactorUserAsync(); + var legacyToken = await factory.GenerateLegacyRememberTokenAsync(user); + + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: legacyToken, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); + } + + /// + /// H5 — accepting a legacy token also replaces it, so each device moves to the current format on + /// its next login rather than waiting for its token to expire. + /// + [Fact] + public async Task LegacyRememberToken_IsUpgradedToCurrentFormat() + { + var (factory, user) = await CreateFactoryWithTwoFactorUserAsync(); + var legacyToken = await factory.GenerateLegacyRememberTokenAsync(user); + + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: legacyToken, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + var upgraded = AssertHelper.AssertJsonProperty(root, "TwoFactorToken", JsonValueKind.String).GetString(); + Assert.StartsWith(TwoFactorRememberTokenable.ClearTextPrefix, upgraded); + + // And the replacement works on the login after that. + var replayContext = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: upgraded!, + twoFactorRemember: "0"); + + using var replayBody = await AssertHelper.AssertResponseTypeIs(replayContext); + AssertHelper.AssertJsonProperty(replayBody.RootElement, "access_token", JsonValueKind.String); + } + /// /// H3 — the token is bound to the device it was issued to, so presenting it from another device /// falls back to a two-factor challenge. diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index 80f0567dc134..d9c8e4056365 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -479,9 +479,11 @@ public async void VerifyTwoFactorAsync_Individual_ValidToken_ReturnsTrue( /// checks or UserManager. It is validated against its own server-side state instead. /// [Theory, BitAutoData] - public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(User user, string token) + public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(User user, string tokenBody) { // Arrange + var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); // Act @@ -493,10 +495,110 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, DeviceIdentifier, token); } + // --------------------------------------------------------------------------------------- + // Tokens issued before the current format existed. This whole region goes away with the + // legacy branch once no such token can still be within its lifetime. + // --------------------------------------------------------------------------------------- + + /// L1 — a token in the previous format is still honored. + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_LegacyRemember_ValidToken_ReturnsTrue(User user) + { + // Arrange + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); + _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, null, TwoFactorProviderType.Remember, "legacy-format-token", DeviceIdentifier); + + // Assert + Assert.True(result.Succeeded); + } + + /// + /// L2 — the previous format embeds the user's security stamp and the framework provider compares + /// it, so a rotated stamp still rejects these tokens during the transition. + /// + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_LegacyRemember_ProviderRejects_ReturnsFalse(User user) + { + // Arrange + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); + _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, null, TwoFactorProviderType.Remember, "legacy-format-token", DeviceIdentifier); + + // Assert + Assert.False(result.Succeeded); + Assert.False(result.LegacyRememberUpgradeRequired); + } + + /// + /// L3 — the two-factor-enabled gate covers the previous format too, so an un-replaced token is + /// inert for as long as the account has no second factor configured. + /// + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_LegacyRemember_NoTwoFactorEnabled_ReturnsFalse(User user) + { + // Arrange + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); + _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, null, TwoFactorProviderType.Remember, "legacy-format-token", DeviceIdentifier); + + // Assert + Assert.False(result.Succeeded); + } + + /// L4 — accepting one signals that a replacement should be issued on this response. + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_LegacyRemember_Accepted_SignalsUpgrade(User user) + { + // Arrange + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); + _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, null, TwoFactorProviderType.Remember, "legacy-format-token", DeviceIdentifier); + + // Assert + Assert.True(result.LegacyRememberUpgradeRequired); + } + + /// + /// L7 — a current-format token never reaches the legacy branch, which is what makes removing + /// that branch a no-op for anyone already migrated. + /// + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_CurrentFormat_SkipsLegacyBranchAndSignalsNoUpgrade(User user) + { + // Arrange + var token = TwoFactorRememberTokenable.ClearTextPrefix + "current-format-token"; + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); + _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, null, TwoFactorProviderType.Remember, token, DeviceIdentifier); + + // Assert + Assert.True(result.Succeeded); + Assert.False(result.LegacyRememberUpgradeRequired); + } + [Theory, BitAutoData] - public async void VerifyTwoFactorAsync_Remember_QueryRejects_ReturnsFalse(User user, string token) + public async void VerifyTwoFactorAsync_Remember_QueryRejects_ReturnsFalse(User user, string tokenBody) { // Arrange + var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(false); // Act From d3eab66213e09f673a352b9a218b404410fd23e1 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:14:46 -0500 Subject: [PATCH 03/19] [PM-40673 Part 2] split line into 2 lines, update comment --- .../RequestValidators/BaseRequestValidator.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 1b8cef57bcda..8108d3c48edd 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -664,10 +664,9 @@ private async Task> BuildCustomResponse(User user, T if (sendRememberToken) { - // The device is non-null here even though the signature allows null: this runs only - // after device validation succeeded, and DeviceValidator assigns (and saves, for a new - // device) the row on every success path. - customResponse.Add("TwoFactorToken", await _issueTwoFactorRememberTokenCommand.IssueAsync(user, device)); + // The device is non-null here because this runs only after DeviceValidator succeeded + var rememberToken = await _issueTwoFactorRememberTokenCommand.IssueAsync(user, device); + customResponse.Add("TwoFactorToken", rememberToken); } return customResponse; From a736aee3f3e14c2643f2e8a1c1b9f9a005b85c00 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:36:43 -0500 Subject: [PATCH 04/19] [PM-40673 Part 2] simplify comment --- .../RequestValidators/ITwoFactorAuthenticationValidator.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs index 44a4b8b2bd2d..008686bd96e2 100644 --- a/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs @@ -35,9 +35,7 @@ public interface ITwoFactorAuthenticationValidator /// secret passed from the user and consumed by the two-factor provider's verify method /// /// The client-generated device identifier from the raw request. An email code only verifies for the - /// device it was issued to, and the Remember provider binds its token to a device. It is read from - /// the request rather than from CurrentContext or the validated device: device validation runs - /// after this point, so neither is populated yet. + /// device it was issued to, and a remember token is bound to its device. /// Task VerifyTwoFactorAsync( User user, From f90b553443a69a84a3a4831c535e48e7c9daadef Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:47:08 -0500 Subject: [PATCH 05/19] [PM-40673 Part 2] update TODO comment to point to specific Jira ticket --- .../RequestValidators/TwoFactorAuthenticationValidator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index 5b31dc2c27c0..be895a98d935 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -173,7 +173,7 @@ public async Task VerifyTwoFactorAsync( await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, deviceIdentifier, token)); } - // TODO: PM-XXXXX - Remove the legacy token path below once every token issued before this + // TODO: PM-44171 - Remove the legacy token path below once every token issued before this // release has expired. Both formats validate until then. The token lifespan bounds that at // 30 days from the release date, so no token predating it can validate after that point. var legacyTokenValid = await _userManager.VerifyTwoFactorTokenAsync( From a2e0b3fb8759d0e087c7cd4fd727345dfec1f867 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:53:01 -0500 Subject: [PATCH 06/19] [PM-40673 Part 2] update 2FA remember branch comment for clarity --- .../RequestValidators/TwoFactorAuthenticationValidator.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index be895a98d935..c2a9c967a174 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -156,8 +156,8 @@ public async Task VerifyTwoFactorAsync( return new TwoFactorVerificationResult(false); } - // Remember is not a provider a user configures, so it has no entry to look up in the checks - // below. It carries its own server-side state and is validated against that instead. + // Remember is not a provider a user configures, so user.GetTwoFactorProvider(Remember) is always + // null and the lookup below would reject it. It is validated on its own terms here instead. if (type is TwoFactorProviderType.Remember) { // A remember token stands in for a second factor, so it cannot be honored by an account From 236fdf38d103980946e2f8b573c5deb18cb1dfa8 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:17:17 -0500 Subject: [PATCH 07/19] [PM-40673 Part 2] remove test summary comment prefixes --- .../TwoFactorAuthenticationValidatorTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index d9c8e4056365..1edaf7a47e07 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -500,7 +500,7 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us // legacy branch once no such token can still be within its lifetime. // --------------------------------------------------------------------------------------- - /// L1 — a token in the previous format is still honored. + /// A token in the previous format is still honored. [Theory, BitAutoData] public async void VerifyTwoFactorAsync_LegacyRemember_ValidToken_ReturnsTrue(User user) { @@ -517,7 +517,7 @@ public async void VerifyTwoFactorAsync_LegacyRemember_ValidToken_ReturnsTrue(Use } /// - /// L2 — the previous format embeds the user's security stamp and the framework provider compares + /// The previous format embeds the user's security stamp and the framework provider compares /// it, so a rotated stamp still rejects these tokens during the transition. /// [Theory, BitAutoData] @@ -537,7 +537,7 @@ public async void VerifyTwoFactorAsync_LegacyRemember_ProviderRejects_ReturnsFal } /// - /// L3 — the two-factor-enabled gate covers the previous format too, so an un-replaced token is + /// The two-factor-enabled gate covers the previous format too, so an un-replaced token is /// inert for as long as the account has no second factor configured. /// [Theory, BitAutoData] @@ -555,7 +555,7 @@ public async void VerifyTwoFactorAsync_LegacyRemember_NoTwoFactorEnabled_Returns Assert.False(result.Succeeded); } - /// L4 — accepting one signals that a replacement should be issued on this response. + /// Accepting one signals that a replacement should be issued on this response. [Theory, BitAutoData] public async void VerifyTwoFactorAsync_LegacyRemember_Accepted_SignalsUpgrade(User user) { @@ -572,7 +572,7 @@ public async void VerifyTwoFactorAsync_LegacyRemember_Accepted_SignalsUpgrade(Us } /// - /// L7 — a current-format token never reaches the legacy branch, which is what makes removing + /// A current-format token never reaches the legacy branch, which is what makes removing /// that branch a no-op for anyone already migrated. /// [Theory, BitAutoData] From 908bbc6d23973527a6065ee78d983f0f5f3e6e5a Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:34:41 -0500 Subject: [PATCH 08/19] [PM-40673 Part 2] remove test summary comment prefixes from RememberedDeviceLoginTests.cs --- .../Endpoints/RememberedDeviceLoginTests.cs | 12 +- .../Endpoints/RememberedDeviceLoginTests.cs-E | 242 ++++++++++++++++++ 2 files changed, 248 insertions(+), 6 deletions(-) create mode 100644 test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs-E diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs index 859a967ca151..dd7e3a0f0095 100644 --- a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs @@ -75,7 +75,7 @@ public class RememberedDeviceLoginTests } /// - /// H1 — a two-factor login that asks to be remembered gets a token back. + /// A two-factor login that asks to be remembered gets a token back. /// [Fact] public async Task RememberRequested_ResponseCarriesRememberToken() @@ -90,7 +90,7 @@ public async Task RememberRequested_ResponseCarriesRememberToken() } /// - /// H2 — the token is honored on a later login, with no second factor supplied. This is the + /// The token is honored on a later login, with no second factor supplied. This is the /// behavior the whole feature exists to preserve; it must stay green through every change to the /// token format. /// @@ -119,7 +119,7 @@ public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() } /// - /// H6 — a login that presents a remember token must not be issued another one. + /// A login that presents a remember token must not be issued another one. /// /// /// This is what bounds the remember-me lifetime. The token's lifespan is an absolute cap only @@ -151,7 +151,7 @@ public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken() } /// - /// H4 — a token minted the way a previous release minted them is still accepted, so nobody is + /// A token minted the way a previous release minted them is still accepted, so nobody is /// challenged again purely because this release shipped. /// [Fact] @@ -174,7 +174,7 @@ public async Task LegacyRememberToken_IsStillAccepted() } /// - /// H5 — accepting a legacy token also replaces it, so each device moves to the current format on + /// Accepting a legacy token also replaces it, so each device moves to the current format on /// its next login rather than waiting for its token to expire. /// [Fact] @@ -209,7 +209,7 @@ public async Task LegacyRememberToken_IsUpgradedToCurrentFormat() } /// - /// H3 — the token is bound to the device it was issued to, so presenting it from another device + /// The token is bound to the device it was issued to, so presenting it from another device /// falls back to a two-factor challenge. /// [Fact] diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs-E b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs-E new file mode 100644 index 000000000000..859a967ca151 --- /dev/null +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs-E @@ -0,0 +1,242 @@ +using System.Text; +using System.Text.Json; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.KeyManagement.Kdf; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Bit.IntegrationTestCommon.Factories; +using Bit.Test.Common.Helpers; +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; +using Xunit; + +namespace Bit.Identity.IntegrationTest.Endpoints; + +/// +/// End-to-end coverage for the "remember this device" flow: a token issued by one login is honored +/// by the next, and is refused when it should not be. +/// +public class RememberedDeviceLoginTests +{ + private const string _testEmail = "test+rememberdevice@email.com"; + private const string _testPassword = "master_password_hash"; + private const string _emailToken = "12345678"; + private const string _userEmailTwoFactor = + """{"1": { "Enabled": true, "MetaData": { "Email": "test+rememberdevice@email.com"}}}"""; + + /// + /// The numeric value of . Sent as a number rather + /// than the name because the validator-order selection parses it with int.TryParse. + /// + private const string _rememberProvider = "5"; + + private const string _emailProvider = "1"; + + private static async Task<(IdentityApplicationFactory Factory, User User)> CreateFactoryWithTwoFactorUserAsync() + { + var factory = new IdentityApplicationFactory(); + + // The Email provider reads its one-time code from the distributed cache; pinning it lets the + // test complete a real two-factor login. + factory.SubstituteService(distCache => + { + distCache.GetAsync(Arg.Is(s => s.StartsWith("EmailToken_"))) + .Returns(Task.FromResult(Encoding.UTF8.GetBytes(_emailToken))); + }); + + var user = await factory.RegisterNewIdentityFactoryUserAsync( + new RegisterFinishRequestModel + { + Email = _testEmail, + MasterPasswordHash = _testPassword, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = KdfConstants.PBKDF2_ITERATIONS.Default, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey, + EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring + }, + UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring, + }); + + var userRepository = factory.Services.GetRequiredService(); + var userService = factory.GetService(); + user.TwoFactorProviders = _userEmailTwoFactor; + await userRepository.ReplaceAsync(user); + await userService.GetUserByIdAsync(user.Id); + + return (factory, user); + } + + /// + /// H1 — a two-factor login that asks to be remembered gets a token back. + /// + [Fact] + public async Task RememberRequested_ResponseCarriesRememberToken() + { + var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + + var (accessToken, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + + Assert.NotNull(accessToken); + Assert.False(string.IsNullOrEmpty(rememberToken)); + } + + /// + /// H2 — the token is honored on a later login, with no second factor supplied. This is the + /// behavior the whole feature exists to preserve; it must stay green through every change to the + /// token format. + /// + [Fact] + public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() + { + var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + + var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + Assert.False(string.IsNullOrEmpty(rememberToken)); + + // Clients send remember=0 when re-authenticating with a stored token. + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: rememberToken!, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + var accessToken = AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String).GetString(); + Assert.False(string.IsNullOrEmpty(accessToken)); + } + + /// + /// H6 — a login that presents a remember token must not be issued another one. + /// + /// + /// This is what bounds the remember-me lifetime. The token's lifespan is an absolute cap only + /// because the server refuses to re-issue on a remember-login; if it did re-issue, an active + /// user's remember-me would be extended on every login and never expire. Asserted over HTTP + /// deliberately — it inspects the same response field a client would. + /// + [Fact] + public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken() + { + var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + + var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + Assert.False(string.IsNullOrEmpty(rememberToken)); + + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: rememberToken!, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); + Assert.False(root.TryGetProperty("TwoFactorToken", out _)); + } + + /// + /// H4 — a token minted the way a previous release minted them is still accepted, so nobody is + /// challenged again purely because this release shipped. + /// + [Fact] + public async Task LegacyRememberToken_IsStillAccepted() + { + var (factory, user) = await CreateFactoryWithTwoFactorUserAsync(); + var legacyToken = await factory.GenerateLegacyRememberTokenAsync(user); + + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: legacyToken, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); + } + + /// + /// H5 — accepting a legacy token also replaces it, so each device moves to the current format on + /// its next login rather than waiting for its token to expire. + /// + [Fact] + public async Task LegacyRememberToken_IsUpgradedToCurrentFormat() + { + var (factory, user) = await CreateFactoryWithTwoFactorUserAsync(); + var legacyToken = await factory.GenerateLegacyRememberTokenAsync(user); + + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: legacyToken, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + var upgraded = AssertHelper.AssertJsonProperty(root, "TwoFactorToken", JsonValueKind.String).GetString(); + Assert.StartsWith(TwoFactorRememberTokenable.ClearTextPrefix, upgraded); + + // And the replacement works on the login after that. + var replayContext = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + twoFactorProviderType: _rememberProvider, + twoFactorToken: upgraded!, + twoFactorRemember: "0"); + + using var replayBody = await AssertHelper.AssertResponseTypeIs(replayContext); + AssertHelper.AssertJsonProperty(replayBody.RootElement, "access_token", JsonValueKind.String); + } + + /// + /// H3 — the token is bound to the device it was issued to, so presenting it from another device + /// falls back to a two-factor challenge. + /// + [Fact] + public async Task RememberToken_PresentedFromDifferentDevice_IsRefused() + { + var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + + var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + Assert.False(string.IsNullOrEmpty(rememberToken)); + + var context = await factory.ContextFromPasswordWithTwoFactorAsync( + _testEmail, + _testPassword, + deviceIdentifier: Guid.NewGuid().ToString(), + twoFactorProviderType: _rememberProvider, + twoFactorToken: rememberToken!, + twoFactorRemember: "0"); + + using var body = await AssertHelper.AssertResponseTypeIs(context); + var root = body.RootElement; + + // A refused remember token falls back to a challenge rather than a generic auth failure: + // the password was fine, the device just is not trusted. + Assert.False(root.TryGetProperty("access_token", out _)); + var error = AssertHelper.AssertJsonProperty(root, "error_description", JsonValueKind.String).GetString(); + Assert.Equal("Two factor required.", error); + AssertHelper.AssertJsonProperty(root, "TwoFactorProviders2", JsonValueKind.Object); + } +} From 8c3d919e9bd2afb888f7ce8310c80cb8a26a0e81 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:36:42 -0500 Subject: [PATCH 09/19] [PM-40673 Part 2] remove unnecessary usings --- .../Endpoints/RememberedDeviceLoginTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs index dd7e3a0f0095..ba0b259bba12 100644 --- a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs @@ -8,11 +8,9 @@ using Bit.Core.KeyManagement.Kdf; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Utilities; using Bit.IntegrationTestCommon.Factories; using Bit.Test.Common.Helpers; using Microsoft.Extensions.Caching.Distributed; -using Microsoft.Extensions.DependencyInjection; using NSubstitute; using Xunit; From 5d61a99b99b95cddd798997871ad1537a31587c2 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:32:40 -0500 Subject: [PATCH 10/19] [PM-40673 Part 2] simplify comments on IdentityApplicationFactory.cs and RememberedDeviceLoginTests.cs --- .../Endpoints/RememberedDeviceLoginTests.cs | 20 ++++--------------- .../Factories/IdentityApplicationFactory.cs | 10 ---------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs index ba0b259bba12..efa4dc6a3aaa 100644 --- a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs @@ -29,8 +29,7 @@ public class RememberedDeviceLoginTests """{"1": { "Enabled": true, "MetaData": { "Email": "test+rememberdevice@email.com"}}}"""; /// - /// The numeric value of . Sent as a number rather - /// than the name because the validator-order selection parses it with int.TryParse. + /// The numeric value of . /// private const string _rememberProvider = "5"; @@ -88,9 +87,7 @@ public async Task RememberRequested_ResponseCarriesRememberToken() } /// - /// The token is honored on a later login, with no second factor supplied. This is the - /// behavior the whole feature exists to preserve; it must stay green through every change to the - /// token format. + /// The token is honored on a later login, with no second factor supplied. /// [Fact] public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() @@ -117,14 +114,9 @@ public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() } /// - /// A login that presents a remember token must not be issued another one. + /// A login that presents a remember token must not be issued another one, or an active user's + /// remember-me would renew forever. /// - /// - /// This is what bounds the remember-me lifetime. The token's lifespan is an absolute cap only - /// because the server refuses to re-issue on a remember-login; if it did re-issue, an active - /// user's remember-me would be extended on every login and never expire. Asserted over HTTP - /// deliberately — it inspects the same response field a client would. - /// [Fact] public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken() { @@ -148,10 +140,6 @@ public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken() Assert.False(root.TryGetProperty("TwoFactorToken", out _)); } - /// - /// A token minted the way a previous release minted them is still accepted, so nobody is - /// challenged again purely because this release shipped. - /// [Fact] public async Task LegacyRememberToken_IsStillAccepted() { diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index 2b4fdd0f4d65..9d5a829a44b0 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -153,12 +153,6 @@ public async Task ContextFromPasswordAsync( return context; } - /// - /// Send the numeric provider value rather than the enum name where the distinction matters: - /// selects the validator order with - /// int.TryParse, while ValidateTwoFactorAsync parses the same value with - /// Enum.TryParse. A name therefore authenticates but silently takes the ordinary order. - /// /// /// "1" asks the server to remember the device. Pass "0" to mirror a client re-authenticating /// with a stored remember token, which is what clients actually send in that case. @@ -229,10 +223,6 @@ public async Task ContextFromPasswordWithTwoFactorAsync( return (accessTokenString, rememberToken); } - /// - /// Mints a remember token the way the server did before this feature existed, for tests that - /// need to present one issued by a prior release. - /// public async Task GenerateLegacyRememberTokenAsync(User user) { var userManager = GetService>(); From 9563523a4e0b9a1c2781f73403174c5a94243f48 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:58:55 -0500 Subject: [PATCH 11/19] [PM-40673 Part 2] remove old file --- .../Endpoints/RememberedDeviceLoginTests.cs-E | 242 ------------------ 1 file changed, 242 deletions(-) delete mode 100644 test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs-E diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs-E b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs-E deleted file mode 100644 index 859a967ca151..000000000000 --- a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs-E +++ /dev/null @@ -1,242 +0,0 @@ -using System.Text; -using System.Text.Json; -using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Models.Api.Request.Accounts; -using Bit.Core.Auth.Models.Business.Tokenables; -using Bit.Core.Entities; -using Bit.Core.Enums; -using Bit.Core.KeyManagement.Kdf; -using Bit.Core.Repositories; -using Bit.Core.Services; -using Bit.Core.Utilities; -using Bit.IntegrationTestCommon.Factories; -using Bit.Test.Common.Helpers; -using Microsoft.Extensions.Caching.Distributed; -using Microsoft.Extensions.DependencyInjection; -using NSubstitute; -using Xunit; - -namespace Bit.Identity.IntegrationTest.Endpoints; - -/// -/// End-to-end coverage for the "remember this device" flow: a token issued by one login is honored -/// by the next, and is refused when it should not be. -/// -public class RememberedDeviceLoginTests -{ - private const string _testEmail = "test+rememberdevice@email.com"; - private const string _testPassword = "master_password_hash"; - private const string _emailToken = "12345678"; - private const string _userEmailTwoFactor = - """{"1": { "Enabled": true, "MetaData": { "Email": "test+rememberdevice@email.com"}}}"""; - - /// - /// The numeric value of . Sent as a number rather - /// than the name because the validator-order selection parses it with int.TryParse. - /// - private const string _rememberProvider = "5"; - - private const string _emailProvider = "1"; - - private static async Task<(IdentityApplicationFactory Factory, User User)> CreateFactoryWithTwoFactorUserAsync() - { - var factory = new IdentityApplicationFactory(); - - // The Email provider reads its one-time code from the distributed cache; pinning it lets the - // test complete a real two-factor login. - factory.SubstituteService(distCache => - { - distCache.GetAsync(Arg.Is(s => s.StartsWith("EmailToken_"))) - .Returns(Task.FromResult(Encoding.UTF8.GetBytes(_emailToken))); - }); - - var user = await factory.RegisterNewIdentityFactoryUserAsync( - new RegisterFinishRequestModel - { - Email = _testEmail, - MasterPasswordHash = _testPassword, - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = KdfConstants.PBKDF2_ITERATIONS.Default, - UserAsymmetricKeys = new KeysRequestModel - { - PublicKey = Bit.Test.Common.Constants.TestEncryptionConstants.PublicKey, - EncryptedPrivateKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring - }, - UserSymmetricKey = Bit.Test.Common.Constants.TestEncryptionConstants.AES256_CBC_HMAC_Encstring, - }); - - var userRepository = factory.Services.GetRequiredService(); - var userService = factory.GetService(); - user.TwoFactorProviders = _userEmailTwoFactor; - await userRepository.ReplaceAsync(user); - await userService.GetUserByIdAsync(user.Id); - - return (factory, user); - } - - /// - /// H1 — a two-factor login that asks to be remembered gets a token back. - /// - [Fact] - public async Task RememberRequested_ResponseCarriesRememberToken() - { - var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); - - var (accessToken, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( - _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); - - Assert.NotNull(accessToken); - Assert.False(string.IsNullOrEmpty(rememberToken)); - } - - /// - /// H2 — the token is honored on a later login, with no second factor supplied. This is the - /// behavior the whole feature exists to preserve; it must stay green through every change to the - /// token format. - /// - [Fact] - public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() - { - var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); - - var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( - _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); - Assert.False(string.IsNullOrEmpty(rememberToken)); - - // Clients send remember=0 when re-authenticating with a stored token. - var context = await factory.ContextFromPasswordWithTwoFactorAsync( - _testEmail, - _testPassword, - twoFactorProviderType: _rememberProvider, - twoFactorToken: rememberToken!, - twoFactorRemember: "0"); - - using var body = await AssertHelper.AssertResponseTypeIs(context); - var root = body.RootElement; - - var accessToken = AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String).GetString(); - Assert.False(string.IsNullOrEmpty(accessToken)); - } - - /// - /// H6 — a login that presents a remember token must not be issued another one. - /// - /// - /// This is what bounds the remember-me lifetime. The token's lifespan is an absolute cap only - /// because the server refuses to re-issue on a remember-login; if it did re-issue, an active - /// user's remember-me would be extended on every login and never expire. Asserted over HTTP - /// deliberately — it inspects the same response field a client would. - /// - [Fact] - public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken() - { - var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); - - var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( - _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); - Assert.False(string.IsNullOrEmpty(rememberToken)); - - var context = await factory.ContextFromPasswordWithTwoFactorAsync( - _testEmail, - _testPassword, - twoFactorProviderType: _rememberProvider, - twoFactorToken: rememberToken!, - twoFactorRemember: "0"); - - using var body = await AssertHelper.AssertResponseTypeIs(context); - var root = body.RootElement; - - AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); - Assert.False(root.TryGetProperty("TwoFactorToken", out _)); - } - - /// - /// H4 — a token minted the way a previous release minted them is still accepted, so nobody is - /// challenged again purely because this release shipped. - /// - [Fact] - public async Task LegacyRememberToken_IsStillAccepted() - { - var (factory, user) = await CreateFactoryWithTwoFactorUserAsync(); - var legacyToken = await factory.GenerateLegacyRememberTokenAsync(user); - - var context = await factory.ContextFromPasswordWithTwoFactorAsync( - _testEmail, - _testPassword, - twoFactorProviderType: _rememberProvider, - twoFactorToken: legacyToken, - twoFactorRemember: "0"); - - using var body = await AssertHelper.AssertResponseTypeIs(context); - var root = body.RootElement; - - AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); - } - - /// - /// H5 — accepting a legacy token also replaces it, so each device moves to the current format on - /// its next login rather than waiting for its token to expire. - /// - [Fact] - public async Task LegacyRememberToken_IsUpgradedToCurrentFormat() - { - var (factory, user) = await CreateFactoryWithTwoFactorUserAsync(); - var legacyToken = await factory.GenerateLegacyRememberTokenAsync(user); - - var context = await factory.ContextFromPasswordWithTwoFactorAsync( - _testEmail, - _testPassword, - twoFactorProviderType: _rememberProvider, - twoFactorToken: legacyToken, - twoFactorRemember: "0"); - - using var body = await AssertHelper.AssertResponseTypeIs(context); - var root = body.RootElement; - - var upgraded = AssertHelper.AssertJsonProperty(root, "TwoFactorToken", JsonValueKind.String).GetString(); - Assert.StartsWith(TwoFactorRememberTokenable.ClearTextPrefix, upgraded); - - // And the replacement works on the login after that. - var replayContext = await factory.ContextFromPasswordWithTwoFactorAsync( - _testEmail, - _testPassword, - twoFactorProviderType: _rememberProvider, - twoFactorToken: upgraded!, - twoFactorRemember: "0"); - - using var replayBody = await AssertHelper.AssertResponseTypeIs(replayContext); - AssertHelper.AssertJsonProperty(replayBody.RootElement, "access_token", JsonValueKind.String); - } - - /// - /// H3 — the token is bound to the device it was issued to, so presenting it from another device - /// falls back to a two-factor challenge. - /// - [Fact] - public async Task RememberToken_PresentedFromDifferentDevice_IsRefused() - { - var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); - - var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( - _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); - Assert.False(string.IsNullOrEmpty(rememberToken)); - - var context = await factory.ContextFromPasswordWithTwoFactorAsync( - _testEmail, - _testPassword, - deviceIdentifier: Guid.NewGuid().ToString(), - twoFactorProviderType: _rememberProvider, - twoFactorToken: rememberToken!, - twoFactorRemember: "0"); - - using var body = await AssertHelper.AssertResponseTypeIs(context); - var root = body.RootElement; - - // A refused remember token falls back to a challenge rather than a generic auth failure: - // the password was fine, the device just is not trusted. - Assert.False(root.TryGetProperty("access_token", out _)); - var error = AssertHelper.AssertJsonProperty(root, "error_description", JsonValueKind.String).GetString(); - Assert.Equal("Two factor required.", error); - AssertHelper.AssertJsonProperty(root, "TwoFactorProviders2", JsonValueKind.Object); - } -} From 989b5518bea78d791ca0762f0b5cfc3607e8ef14 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:12:18 -0500 Subject: [PATCH 12/19] [PM-40673 Part 2] add check for org-required-2FA when checking if 2FA is enabled for user --- .../TwoFactorAuthenticationValidator.cs | 6 +- .../TwoFactorAuthenticationValidatorTests.cs | 58 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index c2a9c967a174..576045b5527f 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -161,8 +161,10 @@ public async Task VerifyTwoFactorAsync( if (type is TwoFactorProviderType.Remember) { // A remember token stands in for a second factor, so it cannot be honored by an account - // that has none configured. Applied ahead of the format branch so it covers both. - if (!await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) + // that has none, whether configured by the user or enforced by their organization. + // Applied ahead of the format branch so it covers both. + if (organization?.TwoFactorIsEnabled() != true && + !await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) { return new TwoFactorVerificationResult(false); } diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index 1edaf7a47e07..3a32f84d1bf6 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -495,6 +495,45 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, DeviceIdentifier, token); } + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_Remember_OrgEnforcesTwoFactor_NoPersonalTwoFactor_DelegatesToRememberTokenQuery( + User user, Organization organization, string tokenBody) + { + // Arrange + organization.Use2fa = true; + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, organization, TwoFactorProviderType.Remember, token, DeviceIdentifier); + + // Assert + Assert.True(result.Succeeded); + } + + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_Remember_OrgDoesNotEnforceTwoFactor_NoPersonalTwoFactor_ReturnsFalse( + User user, Organization organization, string tokenBody) + { + // Arrange + organization.Use2fa = false; + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, organization, TwoFactorProviderType.Remember, token, DeviceIdentifier); + + // Assert + Assert.False(result.Succeeded); + await _validateTwoFactorRememberTokenQuery.DidNotReceiveWithAnyArgs().ValidateAsync(default!, default!, default!); + } + // --------------------------------------------------------------------------------------- // Tokens issued before the current format existed. This whole region goes away with the // legacy branch once no such token can still be within its lifetime. @@ -555,6 +594,25 @@ public async void VerifyTwoFactorAsync_LegacyRemember_NoTwoFactorEnabled_Returns Assert.False(result.Succeeded); } + [Theory, BitAutoData] + public async void VerifyTwoFactorAsync_LegacyRemember_OrgEnforcesTwoFactor_NoPersonalTwoFactor_ReturnsTrue( + User user, Organization organization) + { + // Arrange + organization.Use2fa = true; + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); + _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; + + // Act + var result = await _sut.VerifyTwoFactorAsync( + user, organization, TwoFactorProviderType.Remember, "legacy-format-token", DeviceIdentifier); + + // Assert + Assert.True(result.Succeeded); + Assert.True(result.LegacyRememberUpgradeRequired); + } + /// Accepting one signals that a replacement should be issued on this response. [Theory, BitAutoData] public async void VerifyTwoFactorAsync_LegacyRemember_Accepted_SignalsUpgrade(User user) From 5ca46f0f75618715cd4e343d59c3e07c03d53f42 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:59:23 -0500 Subject: [PATCH 13/19] [PM-40673 Part 2] add organization to arguments --- .../TwoFactorAuthenticationValidator.cs | 2 +- .../TwoFactorAuthenticationValidatorTests.cs | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index 576045b5527f..d1943ca8dbbe 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -172,7 +172,7 @@ public async Task VerifyTwoFactorAsync( if (token?.StartsWith(TwoFactorRememberTokenable.ClearTextPrefix, StringComparison.Ordinal) == true) { return new TwoFactorVerificationResult( - await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, deviceIdentifier, token)); + await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, deviceIdentifier, token)); } // TODO: PM-44171 - Remove the legacy token path below once every token issued before this diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index 3a32f84d1bf6..447b9c298a81 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -484,7 +484,7 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(true); // Act var result = await _sut.VerifyTwoFactorAsync( @@ -492,7 +492,7 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us // Assert Assert.True(result.Succeeded); - await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, DeviceIdentifier, token); + await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, null, DeviceIdentifier, token); } [Theory, BitAutoData] @@ -504,7 +504,7 @@ public async void VerifyTwoFactorAsync_Remember_OrgEnforcesTwoFactor_NoPersonalT organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, DeviceIdentifier, token).Returns(true); // Act var result = await _sut.VerifyTwoFactorAsync( @@ -512,6 +512,8 @@ public async void VerifyTwoFactorAsync_Remember_OrgEnforcesTwoFactor_NoPersonalT // Assert Assert.True(result.Succeeded); + // The query repeats the check, so it has to be handed the organization too. + await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, organization, DeviceIdentifier, token); } [Theory, BitAutoData] @@ -523,7 +525,7 @@ public async void VerifyTwoFactorAsync_Remember_OrgDoesNotEnforceTwoFactor_NoPer organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, DeviceIdentifier, token).Returns(true); // Act var result = await _sut.VerifyTwoFactorAsync( @@ -531,7 +533,7 @@ public async void VerifyTwoFactorAsync_Remember_OrgDoesNotEnforceTwoFactor_NoPer // Assert Assert.False(result.Succeeded); - await _validateTwoFactorRememberTokenQuery.DidNotReceiveWithAnyArgs().ValidateAsync(default!, default!, default!); + await _validateTwoFactorRememberTokenQuery.DidNotReceiveWithAnyArgs().ValidateAsync(default!, default!, default!, default!); } // --------------------------------------------------------------------------------------- @@ -639,7 +641,7 @@ public async void VerifyTwoFactorAsync_CurrentFormat_SkipsLegacyBranchAndSignals // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + "current-format-token"; _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(true); _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; // Act @@ -657,7 +659,7 @@ public async void VerifyTwoFactorAsync_Remember_QueryRejects_ReturnsFalse(User u // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(false); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(false); // Act var result = await _sut.VerifyTwoFactorAsync( From b4a5685fd16f66406d819969ff6754a053ca3551 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 9 Oct 2026 17:04:52 -0500 Subject: [PATCH 14/19] [PM-40673 Part 2] run dotnet format --- .../IdentityServer/RequestValidators/BaseRequestValidator.cs | 1 - .../RequestValidators/CustomTokenRequestValidator.cs | 2 +- .../RequestValidators/ResourceOwnerPasswordValidator.cs | 2 +- .../IdentityServer/RequestValidators/WebAuthnGrantValidator.cs | 2 +- .../Endpoints/RememberedDeviceLoginTests.cs | 2 +- test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs | 2 +- .../IdentityServer/CustomTokenRequestValidatorTests.cs | 2 +- test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs | 2 +- .../Factories/IdentityApplicationFactory.cs | 2 +- 9 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 8108d3c48edd..8ce108913b5a 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -23,7 +23,6 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Utilities; using Duende.IdentityServer.Validation; using Microsoft.AspNetCore.Identity; diff --git a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs index 7be0a0a25653..d4446c91590d 100644 --- a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs @@ -3,10 +3,10 @@ using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.Auth.Identity; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.IdentityServer; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.KeyManagement.Queries.Interfaces; diff --git a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs index c77f7df69756..d677bdf65d90 100644 --- a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs @@ -5,8 +5,8 @@ using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.Auth.Repositories; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.KeyManagement.Queries.Interfaces; diff --git a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs index df55f6caf1bf..2aac9a4d809d 100644 --- a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs @@ -3,10 +3,10 @@ using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.Auth.Enums; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.UserFeatures.WebAuthnLogin; using Bit.Core.Context; using Bit.Core.Entities; diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs index efa4dc6a3aaa..3403db852551 100644 --- a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs @@ -1,4 +1,4 @@ -using System.Text; +using System.Text; using System.Text.Json; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Request.Accounts; diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 921449badb77..f829d82c7f19 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -1,5 +1,4 @@ using Bit.Core; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; @@ -9,6 +8,7 @@ using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; diff --git a/test/Identity.Test/IdentityServer/CustomTokenRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/CustomTokenRequestValidatorTests.cs index 27ba6672c1dc..a5044ad2c6c5 100644 --- a/test/Identity.Test/IdentityServer/CustomTokenRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/CustomTokenRequestValidatorTests.cs @@ -2,11 +2,11 @@ using System.Security.Claims; using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.Identity; using Bit.Core.Auth.IdentityServer; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; diff --git a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs index bde53c92c171..98e1d998a929 100644 --- a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs +++ b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs @@ -1,8 +1,8 @@ using System.Security.Claims; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Devices.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.KeyManagement.Queries.Interfaces; diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index 9d5a829a44b0..d173ce8f626c 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -19,8 +19,8 @@ using Bit.Test.Common.Helpers; using Core.Auth.Enums; using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Identity; using Microsoft.EntityFrameworkCore; using NSubstitute; using Xunit; From 17585fcdea66b238e868b88b0ae7b06cd80a1ceb Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Wed, 14 Oct 2026 08:18:36 -0500 Subject: [PATCH 15/19] [PM-40673 Part 2] remove 2fa-enabled check from VerifyTwoFactorAsync --- .../TwoFactorAuthenticationValidator.cs | 11 +--- .../TwoFactorAuthenticationValidatorTests.cs | 58 ++----------------- 2 files changed, 8 insertions(+), 61 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index d1943ca8dbbe..61872d15b93f 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -160,17 +160,10 @@ public async Task VerifyTwoFactorAsync( // null and the lookup below would reject it. It is validated on its own terms here instead. if (type is TwoFactorProviderType.Remember) { - // A remember token stands in for a second factor, so it cannot be honored by an account - // that has none, whether configured by the user or enforced by their organization. - // Applied ahead of the format branch so it covers both. - if (organization?.TwoFactorIsEnabled() != true && - !await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) - { - return new TwoFactorVerificationResult(false); - } - if (token?.StartsWith(TwoFactorRememberTokenable.ClearTextPrefix, StringComparison.Ordinal) == true) { + // The query checks that the user has a second factor, personal or enforced by their + // organization, so it needs the organization too. return new TwoFactorVerificationResult( await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, deviceIdentifier, token)); } diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index 447b9c298a81..a75cbb0d22e4 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -483,7 +483,6 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us { // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(true); // Act @@ -495,15 +494,16 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, null, DeviceIdentifier, token); } + /// + /// The query decides whether the user, or their organization, supplies the second factor, so the + /// organization has to reach it. + /// [Theory, BitAutoData] - public async void VerifyTwoFactorAsync_Remember_OrgEnforcesTwoFactor_NoPersonalTwoFactor_DelegatesToRememberTokenQuery( + public async void VerifyTwoFactorAsync_Remember_PassesOrganizationToRememberTokenQuery( User user, Organization organization, string tokenBody) { // Arrange - organization.Use2fa = true; - organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, DeviceIdentifier, token).Returns(true); // Act @@ -512,30 +512,9 @@ public async void VerifyTwoFactorAsync_Remember_OrgEnforcesTwoFactor_NoPersonalT // Assert Assert.True(result.Succeeded); - // The query repeats the check, so it has to be handed the organization too. await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, organization, DeviceIdentifier, token); } - [Theory, BitAutoData] - public async void VerifyTwoFactorAsync_Remember_OrgDoesNotEnforceTwoFactor_NoPersonalTwoFactor_ReturnsFalse( - User user, Organization organization, string tokenBody) - { - // Arrange - organization.Use2fa = false; - organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); - var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, DeviceIdentifier, token).Returns(true); - - // Act - var result = await _sut.VerifyTwoFactorAsync( - user, organization, TwoFactorProviderType.Remember, token, DeviceIdentifier); - - // Assert - Assert.False(result.Succeeded); - await _validateTwoFactorRememberTokenQuery.DidNotReceiveWithAnyArgs().ValidateAsync(default!, default!, default!, default!); - } - // --------------------------------------------------------------------------------------- // Tokens issued before the current format existed. This whole region goes away with the // legacy branch once no such token can still be within its lifetime. @@ -546,7 +525,6 @@ public async void VerifyTwoFactorAsync_Remember_OrgDoesNotEnforceTwoFactor_NoPer public async void VerifyTwoFactorAsync_LegacyRemember_ValidToken_ReturnsTrue(User user) { // Arrange - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; // Act @@ -565,7 +543,6 @@ public async void VerifyTwoFactorAsync_LegacyRemember_ValidToken_ReturnsTrue(Use public async void VerifyTwoFactorAsync_LegacyRemember_ProviderRejects_ReturnsFalse(User user) { // Arrange - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; // Act @@ -577,33 +554,13 @@ public async void VerifyTwoFactorAsync_LegacyRemember_ProviderRejects_ReturnsFal Assert.False(result.LegacyRememberUpgradeRequired); } - /// - /// The two-factor-enabled gate covers the previous format too, so an un-replaced token is - /// inert for as long as the account has no second factor configured. - /// - [Theory, BitAutoData] - public async void VerifyTwoFactorAsync_LegacyRemember_NoTwoFactorEnabled_ReturnsFalse(User user) - { - // Arrange - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); - _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; - - // Act - var result = await _sut.VerifyTwoFactorAsync( - user, null, TwoFactorProviderType.Remember, "legacy-format-token", DeviceIdentifier); - - // Assert - Assert.False(result.Succeeded); - } - [Theory, BitAutoData] - public async void VerifyTwoFactorAsync_LegacyRemember_OrgEnforcesTwoFactor_NoPersonalTwoFactor_ReturnsTrue( + public async void VerifyTwoFactorAsync_LegacyRemember_OrgEnforcesTwoFactor_NoPersonalTwoFactor_ReturnsTrueAndSignalsUpgrade( User user, Organization organization) { // Arrange organization.Use2fa = true; organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; // Act @@ -620,7 +577,6 @@ public async void VerifyTwoFactorAsync_LegacyRemember_OrgEnforcesTwoFactor_NoPer public async void VerifyTwoFactorAsync_LegacyRemember_Accepted_SignalsUpgrade(User user) { // Arrange - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; // Act @@ -640,7 +596,6 @@ public async void VerifyTwoFactorAsync_CurrentFormat_SkipsLegacyBranchAndSignals { // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + "current-format-token"; - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(true); _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; @@ -658,7 +613,6 @@ public async void VerifyTwoFactorAsync_Remember_QueryRejects_ReturnsFalse(User u { // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; - _twoFactorEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(true); _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(false); // Act From c8c5e31725c1d7c1da989a43a785028bf7b5e9d2 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Sat, 17 Oct 2026 07:35:20 -0500 Subject: [PATCH 16/19] [PM-40673 Part 2] refactor legacy format check to make legacy path removal easier in future --- .../TwoFactorAuthenticationValidator.cs | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index 61872d15b93f..eab5a511c085 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -160,23 +160,25 @@ public async Task VerifyTwoFactorAsync( // null and the lookup below would reject it. It is validated on its own terms here instead. if (type is TwoFactorProviderType.Remember) { - if (token?.StartsWith(TwoFactorRememberTokenable.ClearTextPrefix, StringComparison.Ordinal) == true) + // TODO: PM-44171 - Remove the legacy token path. PM-40673 introduced a new remember token format and stopped + // issuing legacy format tokens. Legacy tokens have a 30 day expiration, so we must wait until more than 30 days + // have passed after the release of PM-40673 before we remove this legacy path. + var isLegacyFormat = token is null || !token.StartsWith(TwoFactorRememberTokenable.ClearTextPrefix, StringComparison.Ordinal); + + if (isLegacyFormat) { - // The query checks that the user has a second factor, personal or enforced by their - // organization, so it needs the organization too. - return new TwoFactorVerificationResult( - await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, deviceIdentifier, token)); - } + var legacyTokenValid = await _userManager.VerifyTwoFactorTokenAsync( + user, CoreHelpers.CustomProviderName(type), token); - // TODO: PM-44171 - Remove the legacy token path below once every token issued before this - // release has expired. Both formats validate until then. The token lifespan bounds that at - // 30 days from the release date, so no token predating it can validate after that point. - var legacyTokenValid = await _userManager.VerifyTwoFactorTokenAsync( - user, CoreHelpers.CustomProviderName(type), token); + // Accepting a legacy format token earns a replacement in the new format on this same response, + // so the device migrates over on this login rather than waiting out the legacy token's lifetime. + return new TwoFactorVerificationResult(legacyTokenValid, legacyTokenValid); + } - // Accepting one earns a replacement in the current format on this same response, so the - // device moves over on this login rather than waiting out the old token's lifetime. - return new TwoFactorVerificationResult(legacyTokenValid, legacyTokenValid); + // The query checks that the user has a second factor, personal or enforced by their + // organization, so it needs the organization too. + return new TwoFactorVerificationResult( + await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, deviceIdentifier, token)); } // Now we are concerning the rest of the Two Factor Provider Types From 0ec9aaa81c64468a92b2e369596131e89ba12ab1 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Sat, 17 Oct 2026 07:39:14 -0500 Subject: [PATCH 17/19] [PM-40673 Part 2] change remember token query call back to three arguments, drop organization --- .../TwoFactorAuthenticationValidator.cs | 4 +-- .../TwoFactorAuthenticationValidatorTests.cs | 29 +++---------------- 2 files changed, 5 insertions(+), 28 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index eab5a511c085..039446118e61 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -175,10 +175,8 @@ public async Task VerifyTwoFactorAsync( return new TwoFactorVerificationResult(legacyTokenValid, legacyTokenValid); } - // The query checks that the user has a second factor, personal or enforced by their - // organization, so it needs the organization too. return new TwoFactorVerificationResult( - await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, deviceIdentifier, token)); + await _validateTwoFactorRememberTokenQuery.ValidateAsync(user, deviceIdentifier, token)); } // Now we are concerning the rest of the Two Factor Provider Types diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index a75cbb0d22e4..5a7c2b0a5df2 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -483,7 +483,7 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us { // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(true); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); // Act var result = await _sut.VerifyTwoFactorAsync( @@ -491,28 +491,7 @@ public async void VerifyTwoFactorAsync_Remember_DelegatesToRememberTokenQuery(Us // Assert Assert.True(result.Succeeded); - await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, null, DeviceIdentifier, token); - } - - /// - /// The query decides whether the user, or their organization, supplies the second factor, so the - /// organization has to reach it. - /// - [Theory, BitAutoData] - public async void VerifyTwoFactorAsync_Remember_PassesOrganizationToRememberTokenQuery( - User user, Organization organization, string tokenBody) - { - // Arrange - var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, organization, DeviceIdentifier, token).Returns(true); - - // Act - var result = await _sut.VerifyTwoFactorAsync( - user, organization, TwoFactorProviderType.Remember, token, DeviceIdentifier); - - // Assert - Assert.True(result.Succeeded); - await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, organization, DeviceIdentifier, token); + await _validateTwoFactorRememberTokenQuery.Received(1).ValidateAsync(user, DeviceIdentifier, token); } // --------------------------------------------------------------------------------------- @@ -596,7 +575,7 @@ public async void VerifyTwoFactorAsync_CurrentFormat_SkipsLegacyBranchAndSignals { // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + "current-format-token"; - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(true); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(true); _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; // Act @@ -613,7 +592,7 @@ public async void VerifyTwoFactorAsync_Remember_QueryRejects_ReturnsFalse(User u { // Arrange var token = TwoFactorRememberTokenable.ClearTextPrefix + tokenBody; - _validateTwoFactorRememberTokenQuery.ValidateAsync(user, null, DeviceIdentifier, token).Returns(false); + _validateTwoFactorRememberTokenQuery.ValidateAsync(user, DeviceIdentifier, token).Returns(false); // Act var result = await _sut.VerifyTwoFactorAsync( From 9212d29ec5a06db8c746ed6801ae787258ef9f6e Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:25:01 -0500 Subject: [PATCH 18/19] [PM-40673 Part 2] cover TwoFactorRemember=1 in the no-reissue remember token test. --- .../Endpoints/RememberedDeviceLoginTests.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs index 3403db852551..4d4880378237 100644 --- a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs @@ -115,10 +115,13 @@ public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() /// /// A login that presents a remember token must not be issued another one, or an active user's - /// remember-me would renew forever. + /// remember-me would renew forever. Replayed with and without asking to be remembered again, since + /// a client can send TwoFactorRemember=1 regardless. /// - [Fact] - public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken() + [Theory] + [InlineData("0")] + [InlineData("1")] + public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken(string twoFactorRemember) { var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); @@ -131,7 +134,7 @@ public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken() _testPassword, twoFactorProviderType: _rememberProvider, twoFactorToken: rememberToken!, - twoFactorRemember: "0"); + twoFactorRemember: twoFactorRemember); using var body = await AssertHelper.AssertResponseTypeIs(context); var root = body.RootElement; From 89faab1319e4b884491e5d82bc07f45c75cd6824 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Wed, 28 Oct 2026 08:53:16 -0500 Subject: [PATCH 19/19] [PM-40673 Part 2] fix remembered device tests for device-bound email 2FA codes --- .../Endpoints/RememberedDeviceLoginTests.cs | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs index 4d4880378237..f175298b3fbe 100644 --- a/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/RememberedDeviceLoginTests.cs @@ -1,8 +1,8 @@ -using System.Text; -using System.Text.Json; +using System.Text.Json; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Services; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.KeyManagement.Kdf; @@ -10,8 +10,6 @@ using Bit.Core.Services; using Bit.IntegrationTestCommon.Factories; using Bit.Test.Common.Helpers; -using Microsoft.Extensions.Caching.Distributed; -using NSubstitute; using Xunit; namespace Bit.Identity.IntegrationTest.Endpoints; @@ -24,7 +22,6 @@ public class RememberedDeviceLoginTests { private const string _testEmail = "test+rememberdevice@email.com"; private const string _testPassword = "master_password_hash"; - private const string _emailToken = "12345678"; private const string _userEmailTwoFactor = """{"1": { "Enabled": true, "MetaData": { "Email": "test+rememberdevice@email.com"}}}"""; @@ -39,14 +36,6 @@ public class RememberedDeviceLoginTests { var factory = new IdentityApplicationFactory(); - // The Email provider reads its one-time code from the distributed cache; pinning it lets the - // test complete a real two-factor login. - factory.SubstituteService(distCache => - { - distCache.GetAsync(Arg.Is(s => s.StartsWith("EmailToken_"))) - .Returns(Task.FromResult(Encoding.UTF8.GetBytes(_emailToken))); - }); - var user = await factory.RegisterNewIdentityFactoryUserAsync( new RegisterFinishRequestModel { @@ -71,6 +60,18 @@ public class RememberedDeviceLoginTests return (factory, user); } + /// + /// Issues a real email two-factor login code bound to the device and returns it, as the factory's mail + /// substitute captured it. + /// + private static async Task EmailLoginCodeAsync( + IdentityApplicationFactory factory, string email, string deviceIdentifier) + { + var user = await factory.GetService().GetByEmailAsync(email); + await factory.GetService().SendTwoFactorLoginEmailAsync(user, deviceIdentifier); + return factory.TwoFactorEmailCodes[email]; + } + /// /// A two-factor login that asks to be remembered gets a token back. /// @@ -79,8 +80,10 @@ public async Task RememberRequested_ResponseCarriesRememberToken() { var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + var emailCode = await EmailLoginCodeAsync( + factory, _testEmail, IdentityApplicationFactory.DefaultDeviceIdentifier); var (accessToken, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( - _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: emailCode); Assert.NotNull(accessToken); Assert.False(string.IsNullOrEmpty(rememberToken)); @@ -94,8 +97,10 @@ public async Task RememberToken_ReplayedOnNextLogin_SkipsTwoFactorChallenge() { var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + var emailCode = await EmailLoginCodeAsync( + factory, _testEmail, IdentityApplicationFactory.DefaultDeviceIdentifier); var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( - _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: emailCode); Assert.False(string.IsNullOrEmpty(rememberToken)); // Clients send remember=0 when re-authenticating with a stored token. @@ -125,8 +130,10 @@ public async Task RememberToken_ReplayedOnNextLogin_DoesNotIssueAnotherToken(str { var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + var emailCode = await EmailLoginCodeAsync( + factory, _testEmail, IdentityApplicationFactory.DefaultDeviceIdentifier); var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( - _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: emailCode); Assert.False(string.IsNullOrEmpty(rememberToken)); var context = await factory.ContextFromPasswordWithTwoFactorAsync( @@ -206,8 +213,10 @@ public async Task RememberToken_PresentedFromDifferentDevice_IsRefused() { var (factory, _) = await CreateFactoryWithTwoFactorUserAsync(); + var emailCode = await EmailLoginCodeAsync( + factory, _testEmail, IdentityApplicationFactory.DefaultDeviceIdentifier); var (_, rememberToken) = await factory.TokensFromPasswordWithTwoFactorAsync( - _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: _emailToken); + _testEmail, _testPassword, twoFactorProviderType: _emailProvider, twoFactorToken: emailCode); Assert.False(string.IsNullOrEmpty(rememberToken)); var context = await factory.ContextFromPasswordWithTwoFactorAsync(