From c37412bacbdbbe07027ac26cda750ba4ec9be736 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Tue, 20 Jan 2026 10:03:33 -0500 Subject: [PATCH] chore(flags): Remove pm-1632-redirect-on-sso-required feature flag * Remove feature flag. * Update test title. * Fixed some test failures. * Fixed tests * Removed method that's no longer used. * Removed unneeded directive. --- src/Core/Constants.cs | 1 - .../RequestValidators/BaseRequestValidator.cs | 91 +--- .../CustomTokenRequestValidator.cs | 11 - .../ResourceOwnerPasswordValidator.cs | 8 - .../WebAuthnGrantValidator.cs | 8 - .../BaseRequestValidatorTests.cs | 396 ++++++------------ .../BaseRequestValidatorTestWrapper.cs | 9 - 7 files changed, 144 insertions(+), 380 deletions(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 6f42778b6b..10c68ddc42 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -162,7 +162,6 @@ public static class FeatureFlagKeys public const string MjmlWelcomeEmailTemplates = "pm-21741-mjml-welcome-email"; public const string OrganizationConfirmationEmail = "pm-28402-update-confirmed-to-org-email-template"; public const string MarketingInitiatedPremiumFlow = "pm-26140-marketing-initiated-premium-flow"; - public const string RedirectOnSsoRequired = "pm-1632-redirect-on-sso-required"; public const string PrefetchPasswordPrelogin = "pm-23801-prefetch-password-prelogin"; public const string PM27086_UpdateAuthenticationApisForInputPassword = "pm-27086-update-authentication-apis-for-input-password"; diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index e07446d49f..289feebdb2 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -4,7 +4,6 @@ using System.Security.Claims; using Bit.Core; -using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Entities; @@ -233,56 +232,14 @@ public abstract class BaseRequestValidator where T : class private async Task ValidateSsoAsync(T context, ValidatedTokenRequest request, CustomValidatorRequestContext validatorContext) { - // TODO: Clean up Feature Flag: Remove this if block: PM-28281 - if (!_featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired)) + var ssoValid = await _ssoRequestValidator.ValidateAsync(validatorContext.User, request, validatorContext); + if (ssoValid) { - validatorContext.SsoRequired = await RequireSsoLoginAsync(validatorContext.User, request.GrantType); - if (!validatorContext.SsoRequired) - { - return true; - } - - // Users without SSO requirement requesting 2FA recovery will be fast-forwarded through login and are - // presented with their 2FA management area as a reminder to re-evaluate their 2FA posture after recovery and - // review their new recovery token if desired. - // SSO users cannot be assumed to be authenticated, and must prove authentication with their IdP after recovery. - // As described in validation order determination, if TwoFactorRequired, the 2FA validation scheme will have been - // 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 (validatorContext.TwoFactorRequired && - validatorContext.TwoFactorRecoveryRequested) - { - 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.") } - }); - return false; + return true; } - else - { - var ssoValid = await _ssoRequestValidator.ValidateAsync(validatorContext.User, request, validatorContext); - if (ssoValid) - { - return true; - } - SetValidationErrorResult(context, validatorContext); - return ssoValid; - } + SetValidationErrorResult(context, validatorContext); + return ssoValid; } /// @@ -521,9 +478,6 @@ public abstract class BaseRequestValidator where T : class [Obsolete("Consider using SetValidationErrorResult instead.")] protected abstract void SetTwoFactorResult(T context, Dictionary customResponse); - [Obsolete("Consider using SetValidationErrorResult instead.")] - protected abstract void SetSsoResult(T context, Dictionary customResponse); - [Obsolete("Consider using SetValidationErrorResult instead.")] protected abstract void SetErrorResult(T context, Dictionary customResponse); @@ -540,41 +494,6 @@ public abstract class BaseRequestValidator where T : class protected abstract ClaimsPrincipal GetSubject(T context); - /// - /// 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 - /// using the SSO flow so they are allowed to continue. - /// - /// 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.")] - private async Task RequireSsoLoginAsync(User user, string grantType) - { - if (grantType == "authorization_code" || grantType == "client_credentials") - { - // Already using SSO to authenticate, or logging-in via api key to skip SSO requirement - // allow to authenticate successfully - return false; - } - - // Check if user belongs to any organization with an active SSO policy - var ssoRequired = _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements) - ? (await PolicyRequirementQuery.GetAsync(user.Id)) - .SsoRequired - : await PolicyService.AnyPoliciesApplicableToUserAsync( - user.Id, PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); - if (ssoRequired) - { - return true; - } - - // Default - SSO is not required - return false; - } - private async Task ResetFailedAuthDetailsAsync(User user) { // Early escape if db hit not necessary diff --git a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs index 38a4813ecd..2412c52308 100644 --- a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs @@ -194,17 +194,6 @@ public class CustomTokenRequestValidator : BaseRequestValidator customResponse) - { - Debug.Assert(context.Result is not null); - context.Result.Error = "invalid_grant"; - context.Result.ErrorDescription = "Sso authentication required."; - context.Result.IsError = true; - context.Result.CustomResponse = customResponse; - } - [Obsolete("Consider using SetGrantValidationErrorResult instead.")] protected override void SetErrorResult(CustomTokenRequestValidationContext context, Dictionary customResponse) diff --git a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs index ea2c021f63..8bfddf24f3 100644 --- a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs @@ -152,14 +152,6 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator customResponse) - { - context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Sso authentication required.", - customResponse); - } - [Obsolete("Consider using SetGrantValidationErrorResult instead.")] protected override void SetErrorResult(ResourceOwnerPasswordValidationContext context, Dictionary customResponse) diff --git a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs index e4cd60827e..1563831b81 100644 --- a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs @@ -142,14 +142,6 @@ public class WebAuthnGrantValidator : BaseRequestValidator customResponse) - { - context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Sso authentication required.", - customResponse); - } - [Obsolete("Consider using SetValidationErrorResult instead.")] protected override void SetErrorResult(ExtensionGrantValidationContext context, Dictionary customResponse) { diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 677382b138..4b6f681096 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -18,6 +18,7 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidationConstants; using Bit.Identity.IdentityServer.RequestValidators; using Bit.Identity.Test.Wrappers; using Bit.Test.Common.AutoFixture.Attributes; @@ -130,7 +131,7 @@ public class BaseRequestValidatorTests var logs = _logger.Collector.GetSnapshot(true); Assert.Contains(logs, l => l.Level == LogLevel.Warning && l.Message == "Failed login attempt. Is2FARequest: False IpAddress: "); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; + var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; Assert.Equal("Username or password is incorrect. Try again.", errorResponse.Message); } @@ -161,7 +162,11 @@ public class BaseRequestValidatorTests .ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(false)); - // 5 -> not legacy user + // 5 -> SSO not required + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 6 -> not legacy user _userService.IsLegacyUser(Arg.Any()) .Returns(false); @@ -203,6 +208,11 @@ public class BaseRequestValidatorTests _userService.IsLegacyUser(Arg.Any()) .Returns(false); + // 6 -> SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 7 -> setup user account keys _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData { PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( @@ -262,6 +272,11 @@ public class BaseRequestValidatorTests _userService.IsLegacyUser(Arg.Any()) .Returns(false); + // 6 -> SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 7 -> setup user account keys _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData { PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( @@ -326,6 +341,9 @@ public class BaseRequestValidatorTests { "TwoFactorProviders2", new Dictionary { { "Email", null } } } })); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + // Act await _sut.ValidateAsync(context); @@ -368,6 +386,10 @@ public class BaseRequestValidatorTests .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.Email, "invalid_token") .Returns(Task.FromResult(false)); + // 5 -> set up SSO required verification to succeed + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + // Act await _sut.ValidateAsync(context); @@ -396,21 +418,25 @@ public class BaseRequestValidatorTests // 1 -> initial validation passes _sut.isValid = true; - // 2 -> set up 2FA as required + // 2 -> set up SSO required verification to succeed + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 3 -> set up 2FA as required _twoFactorAuthenticationValidator .RequiresTwoFactorAsync(Arg.Any(), tokenRequest) .Returns(Task.FromResult(new Tuple(true, null))); - // 3 -> provide invalid remember token (remember token expired) + // 4 -> provide invalid remember token (remember token expired) tokenRequest.Raw["TwoFactorToken"] = "expired_remember_token"; tokenRequest.Raw["TwoFactorProvider"] = "5"; // Remember provider - // 4 -> set up remember token verification to fail + // 5 -> set up remember token verification to fail _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.Remember, "expired_remember_token") .Returns(Task.FromResult(false)); - // 5 -> set up dummy BuildTwoFactorResultAsync + // 6 -> set up dummy BuildTwoFactorResultAsync var twoFactorResultDict = new Dictionary { { "TwoFactorProviders", new[] { "0", "1" } }, @@ -446,6 +472,19 @@ public class BaseRequestValidatorTests GrantValidationResult grantResult) { // Arrange + + // SsoRequestValidator sets custom response + requestContext.ValidationErrorResult = new ValidationResult + { + IsError = true, + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoRequiredDescription + }; + requestContext.CustomResponse = new Dictionary + { + { CustomResponseConstants.ResponseKeys.ErrorModel, new ErrorResponseModel(SsoConstants.RequestErrors.SsoRequiredDescription) }, + }; + var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -454,13 +493,17 @@ public class BaseRequestValidatorTests Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(false)); + // Act await _sut.ValidateAsync(context); // Assert Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - Assert.Equal("SSO authentication is required.", errorResponse.Message); + Assert.NotNull(context.GrantResult.CustomResponse); + var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; + Assert.Equal(SsoConstants.RequestErrors.SsoRequiredDescription, errorResponse.Message); } // Test grantTypes with RequireSsoPolicyRequirement when feature flag is enabled @@ -477,6 +520,20 @@ public class BaseRequestValidatorTests { // Arrange _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); + + // SsoRequestValidator sets custom response with organization identifier + requestContext.ValidationErrorResult = new ValidationResult + { + IsError = true, + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoRequiredDescription + }; + requestContext.CustomResponse = new Dictionary + { + { CustomResponseConstants.ResponseKeys.ErrorModel, new ErrorResponseModel(SsoConstants.RequestErrors.SsoRequiredDescription) }, + { CustomResponseConstants.ResponseKeys.SsoOrganizationIdentifier, "test-org-identifier" } + }; + var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -485,6 +542,10 @@ public class BaseRequestValidatorTests var requirement = new RequireSsoPolicyRequirement { SsoRequired = true }; _policyRequirementQuery.GetAsync(Arg.Any()).Returns(requirement); + // Mock the SSO validator to return false + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(false)); + // Act await _sut.ValidateAsync(context); @@ -492,8 +553,9 @@ public class BaseRequestValidatorTests await _policyService.DidNotReceive().AnyPoliciesApplicableToUserAsync( Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - Assert.Equal("SSO authentication is required.", errorResponse.Message); + Assert.NotNull(context.GrantResult.CustomResponse); + var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; + Assert.Equal(SsoConstants.RequestErrors.SsoRequiredDescription, errorResponse.Message); } [Theory] @@ -519,6 +581,10 @@ public class BaseRequestValidatorTests var requirement = new RequireSsoPolicyRequirement { SsoRequired = false }; _policyRequirementQuery.GetAsync(Arg.Any()).Returns(requirement); + // SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -561,6 +627,11 @@ public class BaseRequestValidatorTests _policyService.AnyPoliciesApplicableToUserAsync( Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) .Returns(Task.FromResult(false)); + + // SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -603,6 +674,10 @@ public class BaseRequestValidatorTests context.ValidatedTokenRequest.GrantType = grantType; + // SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -652,13 +727,15 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); // Assert Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; + var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; var expectedMessage = "Legacy encryption without a userkey is no longer supported. To recover your account, please contact support"; Assert.Equal(expectedMessage, errorResponse.Message); @@ -694,6 +771,10 @@ public class BaseRequestValidatorTests var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; + // SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -760,6 +841,8 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); @@ -833,6 +916,8 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); @@ -877,6 +962,8 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); @@ -921,6 +1008,8 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); @@ -950,6 +1039,19 @@ public class BaseRequestValidatorTests GrantValidationResult grantResult) { // Arrange + + // SsoRequestValidator sets custom response + requestContext.ValidationErrorResult = new ValidationResult + { + IsError = true, + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoRequiredDescription + }; + requestContext.CustomResponse = new Dictionary + { + { CustomResponseConstants.ResponseKeys.ErrorModel, new ErrorResponseModel(SsoConstants.RequestErrors.SsoRequiredDescription) }, + }; + var context = CreateContext(tokenRequest, requestContext, grantResult); var user = requestContext.User; @@ -984,12 +1086,12 @@ public class BaseRequestValidatorTests // Assert Assert.True(context.GrantResult.IsError, "Authentication should fail - SSO required after recovery"); - - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; + Assert.NotNull(context.GrantResult.CustomResponse); + var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; // Recovery succeeds, then SSO blocks with descriptive message Assert.Equal( - "Two-factor recovery has been performed. SSO authentication is required.", + SsoConstants.RequestErrors.SsoRequiredDescription, errorResponse.Message); // Verify recovery was marked @@ -1050,7 +1152,7 @@ public class BaseRequestValidatorTests // Assert Assert.True(context.GrantResult.IsError, "Authentication should fail - invalid recovery code"); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; + var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; // 2FA is checked first (due to recovery code request), fails with 2FA error Assert.Equal( @@ -1132,7 +1234,11 @@ public class BaseRequestValidatorTests _userService.IsLegacyUser(Arg.Any()) .Returns(false); - // 8. Setup user account keys for successful login response + // 8. SSO is not required + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 9. Setup user account keys for successful login response _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData { PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( @@ -1161,179 +1267,18 @@ public class BaseRequestValidatorTests } /// - /// Tests that when RedirectOnSsoRequired is DISABLED, the legacy SSO validation path is used. - /// This validates the deprecated RequireSsoLoginAsync method is called and SSO requirement - /// is checked using the old PolicyService.AnyPoliciesApplicableToUserAsync approach. + /// Tests that when SSO validation returns a custom response, (e.g., with organization identifier), + /// that custom response is properly propagated to the result. /// [Theory] [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Disabled_UsesLegacySsoValidation( + public async Task ValidateAsync_SsoRequired_PropagatesCustomResponse( [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(false); - - var context = CreateContext(tokenRequest, requestContext, grantResult); - _sut.isValid = true; - - tokenRequest.GrantType = OidcConstants.GrantTypes.Password; - - // SSO is required via legacy path - _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) - .Returns(Task.FromResult(true)); - - // Act - await _sut.ValidateAsync(context); - - // Assert - Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - Assert.Equal("SSO authentication is required.", errorResponse.Message); - - // Verify legacy path was used - await _policyService.Received(1).AnyPoliciesApplicableToUserAsync( - requestContext.User.Id, PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); - - // Verify new SsoRequestValidator was NOT called - await _ssoRequestValidator.DidNotReceive().ValidateAsync( - Arg.Any(), Arg.Any(), Arg.Any()); - } - - /// - /// Tests that when RedirectOnSsoRequired is ENABLED, the new ISsoRequestValidator is used - /// instead of the legacy RequireSsoLoginAsync method. - /// - [Theory] - [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_UsesNewSsoRequestValidator( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); - - var context = CreateContext(tokenRequest, requestContext, grantResult); - _sut.isValid = true; - - tokenRequest.GrantType = OidcConstants.GrantTypes.Password; - - // Configure SsoRequestValidator to indicate SSO is required - _ssoRequestValidator.ValidateAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) - .Returns(Task.FromResult(false)); // false = SSO required - - // Set up the ValidationErrorResult that SsoRequestValidator would set - requestContext.ValidationErrorResult = new ValidationResult - { - IsError = true, - Error = "sso_required", - ErrorDescription = "SSO authentication is required." - }; - requestContext.CustomResponse = new Dictionary - { - { "ErrorModel", new ErrorResponseModel("SSO authentication is required.") } - }; - - // Act - await _sut.ValidateAsync(context); - - // Assert - Assert.True(context.GrantResult.IsError); - - // Verify new SsoRequestValidator was called - await _ssoRequestValidator.Received(1).ValidateAsync( - requestContext.User, - tokenRequest, - requestContext); - - // Verify legacy path was NOT used - await _policyService.DidNotReceive().AnyPoliciesApplicableToUserAsync( - Arg.Any(), Arg.Any(), Arg.Any()); - } - - /// - /// Tests that when RedirectOnSsoRequired is ENABLED and SSO is NOT required, - /// authentication continues successfully through the new validation path. - /// - [Theory] - [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_SsoNotRequired_SuccessfulLogin( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); - - var context = CreateContext(tokenRequest, requestContext, grantResult); - _sut.isValid = true; - - tokenRequest.GrantType = OidcConstants.GrantTypes.Password; - tokenRequest.ClientId = "web"; - - // SsoRequestValidator returns true (SSO not required) - _ssoRequestValidator.ValidateAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) - .Returns(Task.FromResult(true)); - - // No 2FA required - _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) - .Returns(Task.FromResult(new Tuple(false, null))); - - // Device validation passes - _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) - .Returns(Task.FromResult(true)); - - // User is not legacy - _userService.IsLegacyUser(Arg.Any()).Returns(false); - - _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData - { - PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( - "test-private-key", - "test-public-key" - ) - }); - - // Act - await _sut.ValidateAsync(context); - - // Assert - Assert.False(context.GrantResult.IsError); - await _eventService.Received(1).LogUserEventAsync(requestContext.User.Id, EventType.User_LoggedIn); - - // Verify new validator was used - await _ssoRequestValidator.Received(1).ValidateAsync( - requestContext.User, - tokenRequest, - requestContext); - } - - /// - /// Tests that when RedirectOnSsoRequired is ENABLED and SSO validation returns a custom response - /// (e.g., with organization identifier), that custom response is properly propagated to the result. - /// - [Theory] - [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_PropagatesCustomResponse( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); _sut.isValid = true; tokenRequest.GrantType = OidcConstants.GrantTypes.Password; @@ -1342,13 +1287,13 @@ public class BaseRequestValidatorTests requestContext.ValidationErrorResult = new ValidationResult { IsError = true, - Error = "sso_required", - ErrorDescription = "SSO authentication is required." + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoRequiredDescription }; requestContext.CustomResponse = new Dictionary { - { "ErrorModel", new ErrorResponseModel("SSO authentication is required.") }, - { "SsoOrganizationIdentifier", "test-org-identifier" } + { CustomResponseConstants.ResponseKeys.ErrorModel, new ErrorResponseModel(SsoConstants.RequestErrors.SsoRequiredDescription) }, + { CustomResponseConstants.ResponseKeys.SsoOrganizationIdentifier, "test-org-identifier" } }; var context = CreateContext(tokenRequest, requestContext, grantResult); @@ -1365,77 +1310,24 @@ public class BaseRequestValidatorTests // Assert Assert.True(context.GrantResult.IsError); Assert.NotNull(context.GrantResult.CustomResponse); - Assert.Contains("SsoOrganizationIdentifier", context.CustomValidatorRequestContext.CustomResponse); + Assert.Contains(CustomResponseConstants.ResponseKeys.SsoOrganizationIdentifier, context.CustomValidatorRequestContext.CustomResponse); Assert.Equal("test-org-identifier", - context.CustomValidatorRequestContext.CustomResponse["SsoOrganizationIdentifier"]); + context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.SsoOrganizationIdentifier]); } /// - /// Tests that when RedirectOnSsoRequired is DISABLED and a user with 2FA recovery completes recovery, - /// but SSO is required, the legacy error message is returned (without the recovery-specific message). - /// - [Theory] - [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Disabled_RecoveryWithSso_LegacyMessage( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(false); - - var context = CreateContext(tokenRequest, requestContext, grantResult); - _sut.isValid = true; - - // Recovery code scenario - tokenRequest.Raw["TwoFactorProvider"] = ((int)TwoFactorProviderType.RecoveryCode).ToString(); - tokenRequest.Raw["TwoFactorToken"] = "valid-recovery-code"; - - // 2FA with recovery - _twoFactorAuthenticationValidator - .RequiresTwoFactorAsync(requestContext.User, tokenRequest) - .Returns(Task.FromResult(new Tuple(true, null))); - - _twoFactorAuthenticationValidator - .VerifyTwoFactorAsync(requestContext.User, null, TwoFactorProviderType.RecoveryCode, "valid-recovery-code") - .Returns(Task.FromResult(true)); - - // SSO is required (legacy check) - _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) - .Returns(Task.FromResult(true)); - - // Act - await _sut.ValidateAsync(context); - - // Assert - Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - - // Legacy behavior: recovery-specific message IS shown even without RedirectOnSsoRequired - Assert.Equal("Two-factor recovery has been performed. SSO authentication is required.", errorResponse.Message); - - // But legacy validation path was used - await _policyService.Received(1).AnyPoliciesApplicableToUserAsync( - requestContext.User.Id, PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); - } - - /// - /// Tests that when RedirectOnSsoRequired is ENABLED and recovery code is used for SSO-required user, + /// Tests that when a recovery code is used for SSO-required user, /// the SsoRequestValidator provides the recovery-specific error message. /// [Theory] [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_RecoveryWithSso_NewValidatorMessage( + public async Task ValidateAsync_RecoveryWithSso_CorrectValidatorMessage( [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); - var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -1457,14 +1349,14 @@ public class BaseRequestValidatorTests requestContext.ValidationErrorResult = new ValidationResult { IsError = true, - Error = "sso_required", - ErrorDescription = "Two-factor recovery has been performed. SSO authentication is required." + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoTwoFactorRecoveryDescription }; requestContext.CustomResponse = new Dictionary { { - "ErrorModel", - new ErrorResponseModel("Two-factor recovery has been performed. SSO authentication is required.") + CustomResponseConstants.ResponseKeys.ErrorModel, + new ErrorResponseModel(SsoConstants.RequestErrors.SsoTwoFactorRecoveryDescription) } }; @@ -1479,18 +1371,8 @@ public class BaseRequestValidatorTests // Assert Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse["ErrorModel"]; - Assert.Equal("Two-factor recovery has been performed. SSO authentication is required.", errorResponse.Message); - - // Verify new validator was used - await _ssoRequestValidator.Received(1).ValidateAsync( - requestContext.User, - tokenRequest, - Arg.Is(ctx => ctx.TwoFactorRecoveryRequested)); - - // Verify legacy path was NOT used - await _policyService.DidNotReceive().AnyPoliciesApplicableToUserAsync( - Arg.Any(), Arg.Any(), Arg.Any()); + var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; + Assert.Equal(SsoConstants.RequestErrors.SsoTwoFactorRecoveryDescription, errorResponse.Message); } private BaseRequestValidationContextFake CreateContext( diff --git a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs index b336e4c3c1..ac27c55466 100644 --- a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs +++ b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs @@ -111,15 +111,6 @@ IBaseRequestValidatorTestWrapper context.GrantResult = new GrantValidationResult(TokenRequestErrors.InvalidGrant, customResponse: customResponse); } - [Obsolete] - protected override void SetSsoResult( - BaseRequestValidationContextFake context, - Dictionary customResponse) - { - context.GrantResult = new GrantValidationResult( - TokenRequestErrors.InvalidGrant, "Sso authentication required.", customResponse); - } - protected override Task SetSuccessResult( BaseRequestValidationContextFake context, User user,