diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 3710cb4a23..6d2c2a1673 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -160,7 +160,6 @@ public static class FeatureFlagKeys public const string DisableAlternateLoginMethods = "pm-22110-disable-alternate-login-methods"; public const string PM23174ManageAccountRecoveryPermissionDrivesTheNeedToSetMasterPassword = "pm-23174-manage-account-recovery-permission-drives-the-need-to-set-master-password"; - public const string RecoveryCodeSupportForSsoRequiredUsers = "pm-21153-recovery-code-support-for-sso-required"; public const string MJMLBasedEmailTemplates = "mjml-based-email-templates"; public const string MjmlWelcomeEmailTemplates = "pm-21741-mjml-welcome-email"; public const string MarketingInitiatedPremiumFlow = "pm-26140-marketing-initiated-premium-flow"; diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 429c16a6b3..0bdf1d89c2 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -97,155 +97,16 @@ public abstract class BaseRequestValidator where T : class protected async Task ValidateAsync(T context, ValidatedTokenRequest request, CustomValidatorRequestContext validatorContext) { - if (_featureService.IsEnabled(FeatureFlagKeys.RecoveryCodeSupportForSsoRequiredUsers)) + var validators = DetermineValidationOrder(context, request, validatorContext); + var allValidationSchemesSuccessful = await ProcessValidatorsAsync(validators); + if (!allValidationSchemesSuccessful) { - var validators = DetermineValidationOrder(context, request, validatorContext); - var allValidationSchemesSuccessful = await ProcessValidatorsAsync(validators); - if (!allValidationSchemesSuccessful) - { - // Each validation task is responsible for setting its own non-success status, if applicable. - return; - } - await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.Device, - validatorContext.RememberMeRequested); + // Each validation task is responsible for setting its own non-success status, if applicable. + return; } - else - { - // 1. We need to check if the user's master password hash is correct. - var valid = await ValidateContextAsync(context, validatorContext); - var user = validatorContext.User; - if (!valid) - { - await UpdateFailedAuthDetailsAsync(user); - await BuildErrorResultAsync("Username or password is incorrect. Try again.", false, context, user); - return; - } - - // 2. Decide if this user belongs to an organization that requires SSO. - // TODO: Clean up Feature Flag: Remove this if block: PM-28281 - if (!_featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired)) - { - validatorContext.SsoRequired = await RequireSsoLoginAsync(user, request.GrantType); - if (validatorContext.SsoRequired) - { - SetSsoResult(context, - new Dictionary - { - { "ErrorModel", new ErrorResponseModel("SSO authentication is required.") } - }); - return; - } - } - else - { - var ssoValid = await _ssoRequestValidator.ValidateAsync(user, request, validatorContext); - if (!ssoValid) - { - // SSO is required - SetValidationErrorResult(context, validatorContext); - return; - } - } - - // 3. Check if 2FA is required. - (validatorContext.TwoFactorRequired, var twoFactorOrganization) = - await _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(user, request); - - // This flag is used to determine if the user wants a rememberMe token sent when - // authentication is successful. - var returnRememberMeToken = false; - - if (validatorContext.TwoFactorRequired) - { - var twoFactorToken = request.Raw["TwoFactorToken"]; - var twoFactorProvider = request.Raw["TwoFactorProvider"]; - var validTwoFactorRequest = !string.IsNullOrWhiteSpace(twoFactorToken) && - !string.IsNullOrWhiteSpace(twoFactorProvider); - - // 3a. Response for 2FA required and not provided state. - if (!validTwoFactorRequest || - !Enum.TryParse(twoFactorProvider, out TwoFactorProviderType twoFactorProviderType)) - { - var resultDict = await _twoFactorAuthenticationValidator - .BuildTwoFactorResultAsync(user, twoFactorOrganization); - if (resultDict == null) - { - await BuildErrorResultAsync("No two-step providers enabled.", false, context, user); - return; - } - - // Include Master Password Policy in 2FA response. - resultDict.Add("MasterPasswordPolicy", await GetMasterPasswordPolicyAsync(user)); - SetTwoFactorResult(context, resultDict); - return; - } - - var twoFactorTokenValid = - await _twoFactorAuthenticationValidator - .VerifyTwoFactorAsync(user, twoFactorOrganization, twoFactorProviderType, twoFactorToken); - - // 3b. Response for 2FA required but request is not valid or remember token expired state. - if (!twoFactorTokenValid) - { - // The remember me token has expired. - if (twoFactorProviderType == TwoFactorProviderType.Remember) - { - var resultDict = await _twoFactorAuthenticationValidator - .BuildTwoFactorResultAsync(user, twoFactorOrganization); - - // Include Master Password Policy in 2FA response - resultDict.Add("MasterPasswordPolicy", await GetMasterPasswordPolicyAsync(user)); - SetTwoFactorResult(context, resultDict); - } - else - { - await SendFailedTwoFactorEmail(user, twoFactorProviderType); - await UpdateFailedAuthDetailsAsync(user); - await BuildErrorResultAsync("Two-step token is invalid. Try again.", true, context, user); - } - - return; - } - - // 3c. When the 2FA authentication is successful, we can check if the user wants a - // rememberMe token. - var twoFactorRemember = request.Raw["TwoFactorRemember"] == "1"; - // Check if the user wants a rememberMe token. - if (twoFactorRemember - // if the 2FA auth was rememberMe do not send another token. - && twoFactorProviderType != TwoFactorProviderType.Remember) - { - returnRememberMeToken = true; - } - } - - // 4. Check if the user is logging in from a new device. - var deviceValid = await _deviceValidator.ValidateRequestDeviceAsync(request, validatorContext); - if (!deviceValid) - { - SetValidationErrorResult(context, validatorContext); - await LogFailedLoginEvent(validatorContext.User, EventType.User_FailedLogIn); - return; - } - - // 5. Force legacy users to the web for migration. - if (UserService.IsLegacyUser(user) && request.ClientId != "web") - { - await FailAuthForLegacyUserAsync(user, context); - return; - } - - // TODO: PM-24324 - This should be its own validator at some point. - // 6. Auth request handling - if (validatorContext.ValidatedAuthRequest != null) - { - validatorContext.ValidatedAuthRequest.AuthenticationDate = DateTime.UtcNow; - await _authRequestRepository.ReplaceAsync(validatorContext.ValidatedAuthRequest); - } - - await BuildSuccessResultAsync(user, context, validatorContext.Device, returnRememberMeToken); - } + await BuildSuccessResultAsync(validatorContext.User, context, validatorContext.Device, + validatorContext.RememberMeRequested); } protected async Task FailAuthForLegacyUserAsync(User user, T context) @@ -392,17 +253,22 @@ public abstract class BaseRequestValidator where T : class if (validatorContext.TwoFactorRequired && validatorContext.TwoFactorRecoveryRequested) { - SetSsoResult(context, new Dictionary - { - { "ErrorModel", new ErrorResponseModel("Two-factor recovery has been performed. SSO authentication is required.") } - }); + SetSsoResult(context, + new Dictionary + { + { + "ErrorModel", + new ErrorResponseModel( + "Two-factor recovery has been performed. SSO authentication is required.") + } + }); return false; } SetSsoResult(context, new Dictionary { - { "ErrorModel", new ErrorResponseModel("SSO authentication is required.") } + { "ErrorModel", new ErrorResponseModel("SSO authentication is required.") } }); return false; } @@ -683,7 +549,8 @@ public abstract class BaseRequestValidator where T : class /// user trying to login /// magic string identifying the grant type requested /// true if sso required; false if not required or already in process - [Obsolete("This method is deprecated and will be removed in future versions, PM-28281. Please use the SsoRequestValidator scheme instead.")] + [Obsolete( + "This method is deprecated and will be removed in future versions, PM-28281. Please use the SsoRequestValidator scheme instead.")] private async Task RequireSsoLoginAsync(User user, string grantType) { if (grantType == "authorization_code" || grantType == "client_credentials") diff --git a/src/Identity/IdentityServer/RequestValidators/SsoRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/SsoRequestValidator.cs index 81f8ba1c3f..145ecc8737 100644 --- a/src/Identity/IdentityServer/RequestValidators/SsoRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/SsoRequestValidator.cs @@ -48,8 +48,6 @@ public class SsoRequestValidator( // evaluated, and recovery will have been performed if requested. // We will send a descriptive message in these cases so clients can give the appropriate feedback and redirect // to /login. - // If the feature flag RecoveryCodeSupportForSsoRequiredUsers is set to false then this code is unreachable since - // Two Factor validation occurs after SSO validation in that scenario. if (context.TwoFactorRequired && context.TwoFactorRecoveryRequested) { await SetContextCustomResponseSsoErrorAsync(context, SsoConstants.RequestErrors.SsoTwoFactorRecoveryDescription); @@ -63,10 +61,10 @@ public class SsoRequestValidator( /// /// Check if the user is required to authenticate via SSO. If the user requires SSO, but they are /// logging in using an API Key (client_credentials) then they are allowed to bypass the SSO requirement. - /// If the GrantType is authorization_code or client_credentials we know the user is trying to login + /// If the GrantType is authorization_code or client_credentials we know the user is trying to log in /// using the SSO flow so they are allowed to continue. /// - /// user trying to login + /// user trying to log in /// magic string identifying the grant type requested /// true if sso required; false if not required or already in process private async Task RequireSsoAuthenticationAsync(User user, string grantType) diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 2ead26e4d2..677382b138 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -104,13 +104,6 @@ public class BaseRequestValidatorTests _userAccountKeysQuery); } - private void SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(bool recoveryCodeSupportEnabled) - { - _featureService - .IsEnabled(FeatureFlagKeys.RecoveryCodeSupportForSsoRequiredUsers) - .Returns(recoveryCodeSupportEnabled); - } - /* Logic path * ValidateAsync -> UpdateFailedAuthDetailsAsync -> _mailService.SendFailedLoginAttemptsEmailAsync * |-> BuildErrorResultAsync -> _eventService.LogUserEventAsync @@ -118,16 +111,14 @@ public class BaseRequestValidatorTests * |-> SetErrorResult */ [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_ContextNotValid_SelfHosted_ShouldBuildErrorResult_ShouldLogWarning( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); _globalSettings.SelfHosted = true; _sut.isValid = false; @@ -144,16 +135,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_DeviceNotValidated_ShouldLogError( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); // 1 -> to pass @@ -186,16 +175,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_DeviceValidated_ShouldSucceed( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); // 1 -> to pass @@ -232,16 +219,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_ValidatedAuthRequest_ConsumedOnSuccess( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); // 1 -> to pass @@ -297,16 +282,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_ValidatedAuthRequest_NotConsumed_When2faRequired( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); // 1 -> to pass @@ -329,7 +312,8 @@ public class BaseRequestValidatorTests // 2 -> will result to false with no extra configuration // 3 -> set two factor to be required - requestContext.User.TwoFactorProviders = "{\"1\":{\"Enabled\":true,\"MetaData\":{\"Email\":\"user@test.dev\"}}}"; + requestContext.User.TwoFactorProviders = + "{\"1\":{\"Enabled\":true,\"MetaData\":{\"Email\":\"user@test.dev\"}}}"; _twoFactorAuthenticationValidator .RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(true, null))); @@ -339,7 +323,7 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Dictionary { { "TwoFactorProviders", new[] { "0", "1" } }, - { "TwoFactorProviders2", new Dictionary{{"Email", null}} } + { "TwoFactorProviders2", new Dictionary { { "Email", null } } } })); // Act @@ -356,16 +340,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_TwoFactorTokenInvalid_ShouldSendFailedTwoFactorEmail( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); var user = requestContext.User; @@ -400,16 +382,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_TwoFactorRememberTokenExpired_ShouldNotSendFailedTwoFactorEmail( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); var user = requestContext.User; @@ -455,21 +435,17 @@ public class BaseRequestValidatorTests // Test grantTypes that require SSO when a user is in an organization that requires it [Theory] - [BitAutoData("password", true)] - [BitAutoData("password", false)] - [BitAutoData("webauthn", true)] - [BitAutoData("webauthn", false)] - [BitAutoData("refresh_token", true)] - [BitAutoData("refresh_token", false)] + [BitAutoData("password")] + [BitAutoData("webauthn")] + [BitAutoData("refresh_token")] public async Task ValidateAsync_GrantTypes_OrgSsoRequiredTrue_ShouldSetSsoResult( string grantType, - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -489,21 +465,17 @@ public class BaseRequestValidatorTests // Test grantTypes with RequireSsoPolicyRequirement when feature flag is enabled [Theory] - [BitAutoData("password", true)] - [BitAutoData("password", false)] - [BitAutoData("webauthn", true)] - [BitAutoData("webauthn", false)] - [BitAutoData("refresh_token", true)] - [BitAutoData("refresh_token", false)] + [BitAutoData("password")] + [BitAutoData("webauthn")] + [BitAutoData("refresh_token")] public async Task ValidateAsync_GrantTypes_WithPolicyRequirementsEnabled_OrgSsoRequiredTrue_ShouldSetSsoResult( string grantType, - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -525,21 +497,17 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData("password", true)] - [BitAutoData("password", false)] - [BitAutoData("webauthn", true)] - [BitAutoData("webauthn", false)] - [BitAutoData("refresh_token", true)] - [BitAutoData("refresh_token", false)] + [BitAutoData("password")] + [BitAutoData("webauthn")] + [BitAutoData("refresh_token")] public async Task ValidateAsync_GrantTypes_WithPolicyRequirementsEnabled_OrgSsoRequiredFalse_ShouldSucceed( string grantType, - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -574,21 +542,17 @@ public class BaseRequestValidatorTests // Test grantTypes where SSO would be required but the user is not in an // organization that requires it [Theory] - [BitAutoData("password", true)] - [BitAutoData("password", false)] - [BitAutoData("webauthn", true)] - [BitAutoData("webauthn", false)] - [BitAutoData("refresh_token", true)] - [BitAutoData("refresh_token", false)] + [BitAutoData("password")] + [BitAutoData("webauthn")] + [BitAutoData("refresh_token")] public async Task ValidateAsync_GrantTypes_OrgSsoRequiredFalse_ShouldSucceed( string grantType, - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -623,19 +587,17 @@ public class BaseRequestValidatorTests // Test the grantTypes where SSO is in progress or not relevant [Theory] - [BitAutoData("authorization_code", true)] - [BitAutoData("authorization_code", false)] - [BitAutoData("client_credentials", true)] - [BitAutoData("client_credentials", false)] + [BitAutoData("authorization_code")] + [BitAutoData("client_credentials")] + [BitAutoData("client_credentials")] public async Task ValidateAsync_GrantTypes_SsoRequiredFalse_ShouldSucceed( string grantType, - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -671,16 +633,14 @@ public class BaseRequestValidatorTests * ValidateAsync -> UserService.IsLegacyUser -> FailAuthForLegacyUserAsync */ [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_IsLegacyUser_FailAuthForLegacyUserAsync( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var context = CreateContext(tokenRequest, requestContext, grantResult); var user = context.CustomValidatorRequestContext.User; user.Key = null; @@ -705,16 +665,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_CustomResponse_NoMasterPassword_ShouldSetUserDecryptionOptions( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); _userDecryptionOptionsBuilder.ForUser(Arg.Any()).Returns(_userDecryptionOptionsBuilder); _userDecryptionOptionsBuilder.WithDevice(Arg.Any()).Returns(_userDecryptionOptionsBuilder); _userDecryptionOptionsBuilder.WithSso(Arg.Any()).Returns(_userDecryptionOptionsBuilder); @@ -755,19 +713,16 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true, KdfType.PBKDF2_SHA256, 654_321, null, null)] - [BitAutoData(false, KdfType.PBKDF2_SHA256, 654_321, null, null)] - [BitAutoData(true, KdfType.Argon2id, 11, 128, 5)] - [BitAutoData(false, KdfType.Argon2id, 11, 128, 5)] + [BitAutoData(KdfType.PBKDF2_SHA256, 654_321, null, null)] + [BitAutoData(KdfType.Argon2id, 11, 128, 5)] public async Task ValidateAsync_CustomResponse_MasterPassword_ShouldSetUserDecryptionOptions( - bool featureFlagValue, KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); _userDecryptionOptionsBuilder.ForUser(Arg.Any()).Returns(_userDecryptionOptionsBuilder); _userDecryptionOptionsBuilder.WithDevice(Arg.Any()).Returns(_userDecryptionOptionsBuilder); _userDecryptionOptionsBuilder.WithSso(Arg.Any()).Returns(_userDecryptionOptionsBuilder); @@ -826,16 +781,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_CustomResponse_ShouldIncludeAccountKeys( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var mockAccountKeys = new UserAccountKeysData { PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( @@ -908,16 +861,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_CustomResponse_AccountKeysQuery_SkippedWhenPrivateKeyIsNull( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); requestContext.User.PrivateKey = null; var context = CreateContext(tokenRequest, requestContext, grantResult); @@ -938,16 +889,14 @@ public class BaseRequestValidatorTests } [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_CustomResponse_AccountKeysQuery_CalledWithCorrectUser( - bool featureFlagValue, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagValue); var expectedUser = requestContext.User; _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData @@ -987,22 +936,20 @@ public class BaseRequestValidatorTests /// Tests the core PM-21153 feature: SSO-required users can use recovery codes to disable 2FA, /// but must then authenticate via SSO with a descriptive message about the recovery. /// This test validates: - /// 1. Validation order is changed (2FA before SSO) when recovery code is provided + /// 1. Validation order prioritizes 2FA before SSO when recovery code is provided /// 2. Recovery code successfully validates and sets TwoFactorRecoveryRequested flag /// 3. SSO validation then fails with recovery-specific message /// 4. User is NOT logged in (must authenticate via IdP) /// [Theory] - [BitAutoData(true)] // Feature flag ON - new behavior - [BitAutoData(false)] // Feature flag OFF - should fail at SSO before 2FA recovery + [BitAutoData] public async Task ValidateAsync_RecoveryCodeForSsoRequiredUser_BlocksWithDescriptiveMessage( - bool featureFlagEnabled, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagEnabled); var context = CreateContext(tokenRequest, requestContext, grantResult); var user = requestContext.User; @@ -1015,8 +962,8 @@ public class BaseRequestValidatorTests // 2. SSO is required (this user is in an org that requires SSO) _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) - .Returns(Task.FromResult(true)); + Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) + .Returns(Task.FromResult(true)); // 3. 2FA is required _twoFactorAuthenticationValidator @@ -1040,30 +987,16 @@ public class BaseRequestValidatorTests var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - if (featureFlagEnabled) - { - // NEW BEHAVIOR: Recovery succeeds, then SSO blocks with descriptive message - Assert.Equal( - "Two-factor recovery has been performed. SSO authentication is required.", - errorResponse.Message); + // Recovery succeeds, then SSO blocks with descriptive message + Assert.Equal( + "Two-factor recovery has been performed. SSO authentication is required.", + errorResponse.Message); - // Verify recovery was marked - Assert.True(requestContext.TwoFactorRecoveryRequested, - "TwoFactorRecoveryRequested flag should be set"); - } - else - { - // LEGACY BEHAVIOR: SSO blocks BEFORE recovery can happen - Assert.Equal( - "SSO authentication is required.", - errorResponse.Message); + // Verify recovery was marked + Assert.True(requestContext.TwoFactorRecoveryRequested, + "TwoFactorRecoveryRequested flag should be set"); - // Recovery never happened because SSO checked first - Assert.False(requestContext.TwoFactorRecoveryRequested, - "TwoFactorRecoveryRequested should be false (SSO blocked first)"); - } - - // In both cases: User is NOT logged in + // User is NOT logged in await _eventService.DidNotReceive().LogUserEventAsync(user.Id, EventType.User_LoggedIn); } @@ -1078,16 +1011,14 @@ public class BaseRequestValidatorTests /// 4. NOT be logged in /// [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_InvalidRecoveryCodeForSsoRequiredUser_FailsAt2FA( - bool featureFlagEnabled, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagEnabled); var context = CreateContext(tokenRequest, requestContext, grantResult); var user = requestContext.User; @@ -1096,8 +1027,8 @@ public class BaseRequestValidatorTests // 2. SSO is required _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) - .Returns(Task.FromResult(true)); + Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) + .Returns(Task.FromResult(true)); // 3. 2FA is required _twoFactorAuthenticationValidator @@ -1121,51 +1052,32 @@ public class BaseRequestValidatorTests var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - if (featureFlagEnabled) - { - // NEW BEHAVIOR: 2FA is checked first (due to recovery code request), fails with 2FA error - Assert.Equal( - "Two-step token is invalid. Try again.", - errorResponse.Message); + // 2FA is checked first (due to recovery code request), fails with 2FA error + Assert.Equal( + "Two-step token is invalid. Try again.", + errorResponse.Message); - // Recovery was attempted but failed - flag should NOT be set - Assert.False(requestContext.TwoFactorRecoveryRequested, - "TwoFactorRecoveryRequested should be false (recovery failed)"); + // Recovery was attempted but failed - flag should NOT be set + Assert.False(requestContext.TwoFactorRecoveryRequested, + "TwoFactorRecoveryRequested should be false (recovery failed)"); - // Verify failed 2FA email was sent - await _mailService.Received(1).SendFailedTwoFactorAttemptEmailAsync( - user.Email, - TwoFactorProviderType.RecoveryCode, - Arg.Any(), - Arg.Any()); + // Verify failed 2FA email was sent + await _mailService.Received(1).SendFailedTwoFactorAttemptEmailAsync( + user.Email, + TwoFactorProviderType.RecoveryCode, + Arg.Any(), + Arg.Any()); - // Verify failed login event was logged - await _eventService.Received(1).LogUserEventAsync(user.Id, EventType.User_FailedLogIn2fa); - } - else - { - // LEGACY BEHAVIOR: SSO is checked first, blocks before 2FA - Assert.Equal( - "SSO authentication is required.", - errorResponse.Message); + // Verify failed login event was logged + await _eventService.Received(1).LogUserEventAsync(user.Id, EventType.User_FailedLogIn2fa); - // 2FA validation never happened - await _mailService.DidNotReceive().SendFailedTwoFactorAttemptEmailAsync( - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any()); - } - // In both cases: User is NOT logged in + // User is NOT logged in await _eventService.DidNotReceive().LogUserEventAsync(user.Id, EventType.User_LoggedIn); // Verify user failed login count was updated (in new behavior path) - if (featureFlagEnabled) - { - await _userRepository.Received(1).ReplaceAsync(Arg.Is(u => - u.Id == user.Id && u.FailedLoginCount > 0)); - } + await _userRepository.Received(1).ReplaceAsync(Arg.Is(u => + u.Id == user.Id && u.FailedLoginCount > 0)); } /// @@ -1179,16 +1091,14 @@ public class BaseRequestValidatorTests /// This is the "happy path" for recovery code usage. /// [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_RecoveryCodeForNonSsoUser_SuccessfulLogin( - bool featureFlagEnabled, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(featureFlagEnabled); var context = CreateContext(tokenRequest, requestContext, grantResult); var user = requestContext.User; @@ -1197,8 +1107,8 @@ public class BaseRequestValidatorTests // 2. SSO is NOT required (this is a regular user, not in SSO org) _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) - .Returns(Task.FromResult(false)); + Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) + .Returns(Task.FromResult(false)); // 3. 2FA is required _twoFactorAuthenticationValidator @@ -1235,7 +1145,8 @@ public class BaseRequestValidatorTests await _sut.ValidateAsync(context); // Assert - Assert.False(context.GrantResult.IsError, "Authentication should succeed for non-SSO user with valid recovery code"); + Assert.False(context.GrantResult.IsError, + "Authentication should succeed for non-SSO user with valid recovery code"); // Verify user successfully logged in await _eventService.Received(1).LogUserEventAsync(user.Id, EventType.User_LoggedIn); @@ -1244,19 +1155,9 @@ public class BaseRequestValidatorTests await _userRepository.Received(1).ReplaceAsync(Arg.Is(u => u.Id == user.Id && u.FailedLoginCount == 0)); - if (featureFlagEnabled) - { - // NEW BEHAVIOR: Recovery flag should be set for audit purposes - Assert.True(requestContext.TwoFactorRecoveryRequested, - "TwoFactorRecoveryRequested flag should be set for audit/logging"); - } - else - { - // LEGACY BEHAVIOR: Recovery flag doesn't exist, but login still succeeds - // (SSO check happens before 2FA in legacy, but user is not SSO-required so both pass) - Assert.False(requestContext.TwoFactorRecoveryRequested, - "TwoFactorRecoveryRequested should be false in legacy mode"); - } + // Recovery flag should be set for audit purposes + Assert.True(requestContext.TwoFactorRecoveryRequested, + "TwoFactorRecoveryRequested flag should be set for audit/logging"); } /// @@ -1265,16 +1166,14 @@ public class BaseRequestValidatorTests /// is checked using the old PolicyService.AnyPoliciesApplicableToUserAsync approach. /// [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_RedirectOnSsoRequired_Disabled_UsesLegacySsoValidation( - bool recoveryCodeFeatureEnabled, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(recoveryCodeFeatureEnabled); _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(false); var context = CreateContext(tokenRequest, requestContext, grantResult); @@ -1284,7 +1183,7 @@ public class BaseRequestValidatorTests // SSO is required via legacy path _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) + Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) .Returns(Task.FromResult(true)); // Act @@ -1309,16 +1208,14 @@ public class BaseRequestValidatorTests /// instead of the legacy RequireSsoLoginAsync method. /// [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_UsesNewSsoRequestValidator( - bool recoveryCodeFeatureEnabled, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(recoveryCodeFeatureEnabled); _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); var context = CreateContext(tokenRequest, requestContext, grantResult); @@ -1328,9 +1225,9 @@ public class BaseRequestValidatorTests // Configure SsoRequestValidator to indicate SSO is required _ssoRequestValidator.ValidateAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) + Arg.Any(), + Arg.Any(), + Arg.Any()) .Returns(Task.FromResult(false)); // false = SSO required // Set up the ValidationErrorResult that SsoRequestValidator would set @@ -1367,16 +1264,14 @@ public class BaseRequestValidatorTests /// authentication continues successfully through the new validation path. /// [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_SsoNotRequired_SuccessfulLogin( - bool recoveryCodeFeatureEnabled, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(recoveryCodeFeatureEnabled); _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); var context = CreateContext(tokenRequest, requestContext, grantResult); @@ -1387,9 +1282,9 @@ public class BaseRequestValidatorTests // SsoRequestValidator returns true (SSO not required) _ssoRequestValidator.ValidateAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) + Arg.Any(), + Arg.Any(), + Arg.Any()) .Returns(Task.FromResult(true)); // No 2FA required @@ -1430,16 +1325,14 @@ public class BaseRequestValidatorTests /// (e.g., with organization identifier), that custom response is properly propagated to the result. /// [Theory] - [BitAutoData(true)] - [BitAutoData(false)] + [BitAutoData] public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_PropagatesCustomResponse( - bool recoveryCodeFeatureEnabled, [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(recoveryCodeFeatureEnabled); _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); _sut.isValid = true; @@ -1461,9 +1354,9 @@ public class BaseRequestValidatorTests var context = CreateContext(tokenRequest, requestContext, grantResult); _ssoRequestValidator.ValidateAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) + Arg.Any(), + Arg.Any(), + Arg.Any()) .Returns(Task.FromResult(false)); // Act @@ -1473,7 +1366,8 @@ public class BaseRequestValidatorTests Assert.True(context.GrantResult.IsError); Assert.NotNull(context.GrantResult.CustomResponse); Assert.Contains("SsoOrganizationIdentifier", context.CustomValidatorRequestContext.CustomResponse); - Assert.Equal("test-org-identifier", context.CustomValidatorRequestContext.CustomResponse["SsoOrganizationIdentifier"]); + Assert.Equal("test-org-identifier", + context.CustomValidatorRequestContext.CustomResponse["SsoOrganizationIdentifier"]); } /// @@ -1484,11 +1378,11 @@ public class BaseRequestValidatorTests [BitAutoData] public async Task ValidateAsync_RedirectOnSsoRequired_Disabled_RecoveryWithSso_LegacyMessage( [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(true); _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(false); var context = CreateContext(tokenRequest, requestContext, grantResult); @@ -1509,7 +1403,7 @@ public class BaseRequestValidatorTests // SSO is required (legacy check) _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) + Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) .Returns(Task.FromResult(true)); // Act @@ -1535,11 +1429,11 @@ public class BaseRequestValidatorTests [BitAutoData] public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_RecoveryWithSso_NewValidatorMessage( [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, + [AuthFixtures.CustomValidatorRequestContext] + CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - SetupRecoveryCodeSupportForSsoRequiredUsersFeatureFlag(true); _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); var context = CreateContext(tokenRequest, requestContext, grantResult); @@ -1568,13 +1462,16 @@ public class BaseRequestValidatorTests }; requestContext.CustomResponse = new Dictionary { - { "ErrorModel", new ErrorResponseModel("Two-factor recovery has been performed. SSO authentication is required.") } + { + "ErrorModel", + new ErrorResponseModel("Two-factor recovery has been performed. SSO authentication is required.") + } }; _ssoRequestValidator.ValidateAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) + Arg.Any(), + Arg.Any(), + Arg.Any()) .Returns(Task.FromResult(false)); // Act