From d22b8c14b6926acd0042754df80e8f5dc5622202 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Thu, 4 Dec 2025 10:59:23 -0500 Subject: [PATCH 1/7] fix(two-factor-controller) [PM-24211]: Update send email validation to use auth request's IsValidForAuthentication. --- src/Api/Auth/Controllers/TwoFactorController.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Api/Auth/Controllers/TwoFactorController.cs b/src/Api/Auth/Controllers/TwoFactorController.cs index 0af46fb57c19..15e942c7194f 100644 --- a/src/Api/Auth/Controllers/TwoFactorController.cs +++ b/src/Api/Auth/Controllers/TwoFactorController.cs @@ -9,7 +9,6 @@ using Bit.Core.Auth.Enums; using Bit.Core.Auth.Identity; using Bit.Core.Auth.Identity.TokenProviders; -using Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Services; using Bit.Core.Context; @@ -35,7 +34,7 @@ public class TwoFactorController : Controller private readonly IOrganizationService _organizationService; private readonly UserManager _userManager; private readonly ICurrentContext _currentContext; - private readonly IVerifyAuthRequestCommand _verifyAuthRequestCommand; + private readonly IAuthRequestRepository _authRequestRepository; private readonly IDuoUniversalTokenService _duoUniversalTokenService; private readonly IDataProtectorTokenFactory _twoFactorAuthenticatorDataProtector; private readonly IDataProtectorTokenFactory _ssoEmailTwoFactorSessionDataProtector; @@ -47,7 +46,7 @@ public TwoFactorController( IOrganizationService organizationService, UserManager userManager, ICurrentContext currentContext, - IVerifyAuthRequestCommand verifyAuthRequestCommand, + IAuthRequestRepository authRequestRepository, IDuoUniversalTokenService duoUniversalConfigService, IDataProtectorTokenFactory twoFactorAuthenticatorDataProtector, IDataProtectorTokenFactory ssoEmailTwoFactorSessionDataProtector, @@ -58,7 +57,7 @@ public TwoFactorController( _organizationService = organizationService; _userManager = userManager; _currentContext = currentContext; - _verifyAuthRequestCommand = verifyAuthRequestCommand; + _authRequestRepository = authRequestRepository; _duoUniversalTokenService = duoUniversalConfigService; _twoFactorAuthenticatorDataProtector = twoFactorAuthenticatorDataProtector; _ssoEmailTwoFactorSessionDataProtector = ssoEmailTwoFactorSessionDataProtector; @@ -353,9 +352,9 @@ public async Task SendEmailLoginAsync([FromBody] TwoFactorEmailRequestModel requ // Check if 2FA email is from Passwordless. if (!string.IsNullOrEmpty(requestModel.AuthRequestAccessCode)) { - if (await _verifyAuthRequestCommand - .VerifyAuthRequestAsync(new Guid(requestModel.AuthRequestId), - requestModel.AuthRequestAccessCode)) + var authRequest = await _authRequestRepository.GetByIdAsync(new Guid(requestModel.AuthRequestId)); + if (authRequest != null && + authRequest.IsValidForAuthentication(user.Id, requestModel.AuthRequestAccessCode)) { await _twoFactorEmailService.SendTwoFactorEmailAsync(user); } From d32f5cf7c8d0145d32f752b680190c668b5b3c9c Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Thu, 4 Dec 2025 11:45:01 -0500 Subject: [PATCH 2/7] refactor(login-features) [PM-24211]: Remove Core.LoginFeatures as no longer used; AuthRequest.IsValidForAuthentication should be used for any applicable use cases. --- .../LoginServiceCollectionExtensions.cs | 14 ----------- .../Interfaces/IVerifyAuthRequest.cs | 6 ----- .../PasswordlessLogin/VerifyAuthRequest.cs | 25 ------------------- .../Utilities/ServiceCollectionExtensions.cs | 2 -- 4 files changed, 47 deletions(-) delete mode 100644 src/Core/Auth/LoginFeatures/LoginServiceCollectionExtensions.cs delete mode 100644 src/Core/Auth/LoginFeatures/PasswordlessLogin/Interfaces/IVerifyAuthRequest.cs delete mode 100644 src/Core/Auth/LoginFeatures/PasswordlessLogin/VerifyAuthRequest.cs diff --git a/src/Core/Auth/LoginFeatures/LoginServiceCollectionExtensions.cs b/src/Core/Auth/LoginFeatures/LoginServiceCollectionExtensions.cs deleted file mode 100644 index f8caad448b32..000000000000 --- a/src/Core/Auth/LoginFeatures/LoginServiceCollectionExtensions.cs +++ /dev/null @@ -1,14 +0,0 @@ -using Bit.Core.Auth.LoginFeatures.PasswordlessLogin; -using Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces; -using Microsoft.Extensions.DependencyInjection; - -namespace Bit.Core.Auth.LoginFeatures; - -public static class LoginServiceCollectionExtensions -{ - public static void AddLoginServices(this IServiceCollection services) - { - services.AddScoped(); - } -} - diff --git a/src/Core/Auth/LoginFeatures/PasswordlessLogin/Interfaces/IVerifyAuthRequest.cs b/src/Core/Auth/LoginFeatures/PasswordlessLogin/Interfaces/IVerifyAuthRequest.cs deleted file mode 100644 index e5da1b06d822..000000000000 --- a/src/Core/Auth/LoginFeatures/PasswordlessLogin/Interfaces/IVerifyAuthRequest.cs +++ /dev/null @@ -1,6 +0,0 @@ -namespace Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces; - -public interface IVerifyAuthRequestCommand -{ - Task VerifyAuthRequestAsync(Guid authRequestId, string accessCode); -} diff --git a/src/Core/Auth/LoginFeatures/PasswordlessLogin/VerifyAuthRequest.cs b/src/Core/Auth/LoginFeatures/PasswordlessLogin/VerifyAuthRequest.cs deleted file mode 100644 index 7def7fea7683..000000000000 --- a/src/Core/Auth/LoginFeatures/PasswordlessLogin/VerifyAuthRequest.cs +++ /dev/null @@ -1,25 +0,0 @@ -using Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces; -using Bit.Core.Repositories; -using Bit.Core.Utilities; - -namespace Bit.Core.Auth.LoginFeatures.PasswordlessLogin; - -public class VerifyAuthRequestCommand : IVerifyAuthRequestCommand -{ - private readonly IAuthRequestRepository _authRequestRepository; - - public VerifyAuthRequestCommand(IAuthRequestRepository authRequestRepository) - { - _authRequestRepository = authRequestRepository; - } - - public async Task VerifyAuthRequestAsync(Guid authRequestId, string accessCode) - { - var authRequest = await _authRequestRepository.GetByIdAsync(authRequestId); - if (authRequest == null || !CoreHelpers.FixedTimeEquals(authRequest.AccessCode, accessCode)) - { - return false; - } - return true; - } -} diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 587ddb65a4a1..3d9a79a2e15c 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -21,7 +21,6 @@ using Bit.Core.Auth.Identity; using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.IdentityServer; -using Bit.Core.Auth.LoginFeatures; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.Services; @@ -140,7 +139,6 @@ public static void AddBaseServices(this IServiceCollection services, IGlobalSett services.AddScoped(); services.AddScoped(); services.AddScoped(); - services.AddLoginServices(); services.AddScoped(); services.AddVaultServices(); services.AddReportingServices(); From 1f85df17b7995d45bfbbfdc9a7bf2c48d4959759 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Thu, 4 Dec 2025 12:10:04 -0500 Subject: [PATCH 3/7] feat(auth-request) [PM-24211]: Add tests for AuthRequest.IsValidForAuthentication. --- .../Auth/Entities/AuthRequestTests.cs | 224 ++++++++++++++++++ 1 file changed, 224 insertions(+) create mode 100644 test/Core.Test/Auth/Entities/AuthRequestTests.cs diff --git a/test/Core.Test/Auth/Entities/AuthRequestTests.cs b/test/Core.Test/Auth/Entities/AuthRequestTests.cs new file mode 100644 index 000000000000..9efeb1ded1c4 --- /dev/null +++ b/test/Core.Test/Auth/Entities/AuthRequestTests.cs @@ -0,0 +1,224 @@ +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Enums; +using Xunit; + +namespace Bit.Core.Test.Auth.Entities; + +public class AuthRequestTests +{ + [Fact] + public void IsValidForAuthentication_WithValidRequest_ReturnsTrue() + { + // Arrange + var userId = Guid.NewGuid(); + var accessCode = "test-access-code"; + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.AuthenticateAndUnlock, + ResponseDate = DateTime.UtcNow, + Approved = true, + CreationDate = DateTime.UtcNow, + AuthenticationDate = null, + AccessCode = accessCode + }; + + // Act + var result = authRequest.IsValidForAuthentication(userId, accessCode); + + // Assert + Assert.True(result); + } + + [Fact] + public void IsValidForAuthentication_WithWrongUserId_ReturnsFalse() + { + // Arrange + var userId = Guid.NewGuid(); + var differentUserId = Guid.NewGuid(); + var accessCode = "test-access-code"; + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.AuthenticateAndUnlock, + ResponseDate = DateTime.UtcNow, + Approved = true, + CreationDate = DateTime.UtcNow, + AuthenticationDate = null, + AccessCode = accessCode + }; + + // Act + var result = authRequest.IsValidForAuthentication(differentUserId, accessCode); + + // Assert + Assert.False(result, "Auth request should not validate for a different user"); + } + + [Fact] + public void IsValidForAuthentication_WithWrongAccessCode_ReturnsFalse() + { + // Arrange + var userId = Guid.NewGuid(); + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.AuthenticateAndUnlock, + ResponseDate = DateTime.UtcNow, + Approved = true, + CreationDate = DateTime.UtcNow, + AuthenticationDate = null, + AccessCode = "correct-code" + }; + + // Act + var result = authRequest.IsValidForAuthentication(userId, "wrong-code"); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsValidForAuthentication_WithoutResponseDate_ReturnsFalse() + { + // Arrange + var userId = Guid.NewGuid(); + var accessCode = "test-access-code"; + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.AuthenticateAndUnlock, + ResponseDate = null, // Not responded to + Approved = true, + CreationDate = DateTime.UtcNow, + AuthenticationDate = null, + AccessCode = accessCode + }; + + // Act + var result = authRequest.IsValidForAuthentication(userId, accessCode); + + // Assert + Assert.False(result, "Unanswered auth requests should not be valid"); + } + + [Fact] + public void IsValidForAuthentication_WithApprovedFalse_ReturnsFalse() + { + // Arrange + var userId = Guid.NewGuid(); + var accessCode = "test-access-code"; + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.AuthenticateAndUnlock, + ResponseDate = DateTime.UtcNow, + Approved = false, // Denied + CreationDate = DateTime.UtcNow, + AuthenticationDate = null, + AccessCode = accessCode + }; + + // Act + var result = authRequest.IsValidForAuthentication(userId, accessCode); + + // Assert + Assert.False(result, "Denied auth requests should not be valid"); + } + + [Fact] + public void IsValidForAuthentication_WithApprovedNull_ReturnsFalse() + { + // Arrange + var userId = Guid.NewGuid(); + var accessCode = "test-access-code"; + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.AuthenticateAndUnlock, + ResponseDate = DateTime.UtcNow, + Approved = null, // Pending + CreationDate = DateTime.UtcNow, + AuthenticationDate = null, + AccessCode = accessCode + }; + + // Act + var result = authRequest.IsValidForAuthentication(userId, accessCode); + + // Assert + Assert.False(result, "Pending auth requests should not be valid"); + } + + [Fact] + public void IsValidForAuthentication_WithExpiredRequest_ReturnsFalse() + { + // Arrange + var userId = Guid.NewGuid(); + var accessCode = "test-access-code"; + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.AuthenticateAndUnlock, + ResponseDate = DateTime.UtcNow, + Approved = true, + CreationDate = DateTime.UtcNow.AddMinutes(-20), // Expired (15 min timeout) + AuthenticationDate = null, + AccessCode = accessCode + }; + + // Act + var result = authRequest.IsValidForAuthentication(userId, accessCode); + + // Assert + Assert.False(result, "Expired auth requests should not be valid"); + } + + [Fact] + public void IsValidForAuthentication_WithWrongType_ReturnsFalse() + { + // Arrange + var userId = Guid.NewGuid(); + var accessCode = "test-access-code"; + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.Unlock, // Wrong type + ResponseDate = DateTime.UtcNow, + Approved = true, + CreationDate = DateTime.UtcNow, + AuthenticationDate = null, + AccessCode = accessCode + }; + + // Act + var result = authRequest.IsValidForAuthentication(userId, accessCode); + + // Assert + Assert.False(result, "Only AuthenticateAndUnlock type should be valid"); + } + + [Fact] + public void IsValidForAuthentication_WithAlreadyUsed_ReturnsFalse() + { + // Arrange + var userId = Guid.NewGuid(); + var accessCode = "test-access-code"; + var authRequest = new AuthRequest + { + UserId = userId, + Type = AuthRequestType.AuthenticateAndUnlock, + ResponseDate = DateTime.UtcNow, + Approved = true, + CreationDate = DateTime.UtcNow, + AuthenticationDate = DateTime.UtcNow, // Already used + AccessCode = accessCode + }; + + // Act + var result = authRequest.IsValidForAuthentication(userId, accessCode); + + // Assert + Assert.False(result, "Auth requests should only be valid for one-time use"); + } +} From ca1a8c7cd0a107d66a1d38f29531ccf50bdfe130 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Fri, 5 Dec 2025 11:43:37 -0500 Subject: [PATCH 4/7] fix(two-factor-controller) [PM-24211]: Branching logic should return on successful send. --- src/Api/Auth/Controllers/TwoFactorController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Api/Auth/Controllers/TwoFactorController.cs b/src/Api/Auth/Controllers/TwoFactorController.cs index 15e942c7194f..8ed4cd3b472d 100644 --- a/src/Api/Auth/Controllers/TwoFactorController.cs +++ b/src/Api/Auth/Controllers/TwoFactorController.cs @@ -357,6 +357,7 @@ public async Task SendEmailLoginAsync([FromBody] TwoFactorEmailRequestModel requ authRequest.IsValidForAuthentication(user.Id, requestModel.AuthRequestAccessCode)) { await _twoFactorEmailService.SendTwoFactorEmailAsync(user); + return; } } else if (!string.IsNullOrEmpty(requestModel.SsoEmail2FaSessionToken)) From 09de3e83337786c170d287a2c91800eacd0bfb10 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Fri, 5 Dec 2025 14:12:24 -0500 Subject: [PATCH 5/7] chore(auth-request) [PM-24211]: Remove some old comments (solved-for). --- src/Core/Auth/Entities/AuthRequest.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Core/Auth/Entities/AuthRequest.cs b/src/Core/Auth/Entities/AuthRequest.cs index 2117c575c0fc..38dc0534c1e3 100644 --- a/src/Core/Auth/Entities/AuthRequest.cs +++ b/src/Core/Auth/Entities/AuthRequest.cs @@ -49,11 +49,9 @@ public bool IsSpent() public bool IsExpired() { - // TODO: PM-24252 - consider using TimeProvider for better mocking in tests return GetExpirationDate() < DateTime.UtcNow; } - // TODO: PM-24252 - this probably belongs in a service. public bool IsValidForAuthentication(Guid userId, string password) { From 427a0cfeebc8305803890544f49d9963c1115e50 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Fri, 5 Dec 2025 15:12:29 -0500 Subject: [PATCH 6/7] fix(two-factor-controller) [PM-24211]: Update some comments (clarification/naming). --- src/Api/Auth/Controllers/TwoFactorController.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Api/Auth/Controllers/TwoFactorController.cs b/src/Api/Auth/Controllers/TwoFactorController.cs index 8ed4cd3b472d..7d66eb835d8d 100644 --- a/src/Api/Auth/Controllers/TwoFactorController.cs +++ b/src/Api/Auth/Controllers/TwoFactorController.cs @@ -349,7 +349,8 @@ public async Task SendEmailLoginAsync([FromBody] TwoFactorEmailRequestModel requ if (user != null) { - // Check if 2FA email is from Passwordless. + // Check if 2FA email is from a device approval ("Log in with device") scenario. + // 2FA is required for an unknown device. if (!string.IsNullOrEmpty(requestModel.AuthRequestAccessCode)) { var authRequest = await _authRequestRepository.GetByIdAsync(new Guid(requestModel.AuthRequestId)); From 77234c17412fa509287b308ee8051cb715fabd79 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Fri, 5 Dec 2025 16:54:35 -0500 Subject: [PATCH 7/7] fix(two-factor-controller) [PM-24211]: Rephrase a comment (accuracy). --- src/Api/Auth/Controllers/TwoFactorController.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Api/Auth/Controllers/TwoFactorController.cs b/src/Api/Auth/Controllers/TwoFactorController.cs index 7d66eb835d8d..ba6cf6685996 100644 --- a/src/Api/Auth/Controllers/TwoFactorController.cs +++ b/src/Api/Auth/Controllers/TwoFactorController.cs @@ -350,7 +350,6 @@ public async Task SendEmailLoginAsync([FromBody] TwoFactorEmailRequestModel requ if (user != null) { // Check if 2FA email is from a device approval ("Log in with device") scenario. - // 2FA is required for an unknown device. if (!string.IsNullOrEmpty(requestModel.AuthRequestAccessCode)) { var authRequest = await _authRequestRepository.GetByIdAsync(new Guid(requestModel.AuthRequestId));