From 5afdfa6fd19d9edcf313e1237db74856511c61c1 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Wed, 4 Feb 2026 09:42:32 -0500 Subject: [PATCH] [PM-30563] Change error response on Send Access token request (#6911) * feat: remove invalid email response and instead return email and OTP required to protect against enumeration attacks. * fix: fixing tests and dotnet format --- .../SendAccess/SendAccessConstants.cs | 6 +----- .../SendEmailOtpRequestValidator.cs | 19 ++++++++++++------- .../SendNeverAuthenticateRequestValidator.cs | 4 ++-- ...EmailOtpReqestValidatorIntegrationTests.cs | 2 +- ...ndNeverAuthenticateRequestValidatorTest.cs | 6 +++--- .../SendAccess/SendConstantsSnapshotTests.cs | 3 +-- .../SendEmailOtpRequestValidatorTests.cs | 6 +++--- .../SendNeverAuthenticateValidatorTests.cs | 8 ++++---- 8 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs index 1f5bfba244..f38a4a880f 100644 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs @@ -69,13 +69,9 @@ public static class SendAccessConstants /// public const string EmailRequired = "email_required"; /// - /// Represents the error code indicating that an email address is invalid. - /// - public const string EmailInvalid = "email_invalid"; - /// /// Represents the status indicating that both email and OTP are required, and the OTP has been sent. /// - public const string EmailOtpSent = "email_and_otp_required_otp_sent"; + public const string EmailAndOtpRequired = "email_and_otp_required"; /// /// Represents the status indicating that both email and OTP are required, and the OTP is invalid. /// diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendEmailOtpRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendEmailOtpRequestValidator.cs index bd2b4f91b4..8bbeeba2c0 100644 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendEmailOtpRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendEmailOtpRequestValidator.cs @@ -22,8 +22,7 @@ public class SendEmailOtpRequestValidator( private static readonly Dictionary _sendEmailOtpValidatorErrorDescriptions = new() { { SendAccessConstants.EmailOtpValidatorResults.EmailRequired, $"{SendAccessConstants.TokenRequest.Email} is required." }, - { SendAccessConstants.EmailOtpValidatorResults.EmailOtpSent, "email otp sent." }, - { SendAccessConstants.EmailOtpValidatorResults.EmailInvalid, $"{SendAccessConstants.TokenRequest.Email} is invalid." }, + { SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired, $"{SendAccessConstants.TokenRequest.Email} and {SendAccessConstants.TokenRequest.Otp} are required." }, { SendAccessConstants.EmailOtpValidatorResults.EmailOtpInvalid, $"{SendAccessConstants.TokenRequest.Email} otp is invalid." }, }; @@ -33,7 +32,7 @@ public class SendEmailOtpRequestValidator( // get email var email = request.Get(SendAccessConstants.TokenRequest.Email); - // It is an invalid request if the email is missing which indicated bad shape. + // It is an invalid request if the email is missing. if (string.IsNullOrEmpty(email)) { // Request is the wrong shape and doesn't contain an email field. @@ -43,9 +42,16 @@ public class SendEmailOtpRequestValidator( // email hash must be in the list of email hashes in the EmailOtp array byte[] hashBytes = SHA256.HashData(Encoding.UTF8.GetBytes(email)); string hashEmailHex = Convert.ToHexString(hashBytes).ToUpperInvariant(); + /* + * This is somewhat contradictory to our process where a poor shape means invalid_request and invalid + * data is invalid_grant. + * In this case the shape is correct and the data is invalid but to protect against enumeration we treat incorrect emails + * as invalid requests. The response for a request with a correct email which needs an OTP and a request + * that has an invalid email need to be the same otherwise an attacker could enumerate until a valid email is found. + */ if (!authMethod.EmailHashes.Contains(hashEmailHex)) { - return BuildErrorResult(SendAccessConstants.EmailOtpValidatorResults.EmailInvalid); + return BuildErrorResult(SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired); } // get otp from request @@ -70,7 +76,7 @@ public class SendEmailOtpRequestValidator( token, string.Format(SendAccessConstants.OtpEmail.Subject, token)); - return BuildErrorResult(SendAccessConstants.EmailOtpValidatorResults.EmailOtpSent); + return BuildErrorResult(SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired); } // validate request otp @@ -94,7 +100,7 @@ public class SendEmailOtpRequestValidator( switch (error) { case SendAccessConstants.EmailOtpValidatorResults.EmailRequired: - case SendAccessConstants.EmailOtpValidatorResults.EmailOtpSent: + case SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired: return new GrantValidationResult(TokenRequestErrors.InvalidRequest, errorDescription: _sendEmailOtpValidatorErrorDescriptions[error], new Dictionary @@ -102,7 +108,6 @@ public class SendEmailOtpRequestValidator( { SendAccessConstants.SendAccessError, error } }); case SendAccessConstants.EmailOtpValidatorResults.EmailOtpInvalid: - case SendAccessConstants.EmailOtpValidatorResults.EmailInvalid: return new GrantValidationResult( TokenRequestErrors.InvalidGrant, errorDescription: _sendEmailOtpValidatorErrorDescriptions[error], diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendNeverAuthenticateRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendNeverAuthenticateRequestValidator.cs index 36e033360f..aabafaafd8 100644 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendNeverAuthenticateRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendNeverAuthenticateRequestValidator.cs @@ -38,7 +38,7 @@ public class SendNeverAuthenticateRequestValidator(GlobalSettings globalSettings break; case SendAccessConstants.EnumerationProtection.Email: var hasEmail = request.Get(SendAccessConstants.TokenRequest.Email) is not null; - errorType = hasEmail ? SendAccessConstants.EmailOtpValidatorResults.EmailInvalid + errorType = hasEmail ? SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired : SendAccessConstants.EmailOtpValidatorResults.EmailRequired; break; case SendAccessConstants.EnumerationProtection.Password: @@ -64,7 +64,7 @@ public class SendNeverAuthenticateRequestValidator(GlobalSettings globalSettings SendAccessConstants.EnumerationProtection.Guid => TokenRequestErrors.InvalidGrant, SendAccessConstants.PasswordValidatorResults.RequestPasswordIsRequired => TokenRequestErrors.InvalidGrant, SendAccessConstants.PasswordValidatorResults.RequestPasswordDoesNotMatch => TokenRequestErrors.InvalidRequest, - SendAccessConstants.EmailOtpValidatorResults.EmailInvalid => TokenRequestErrors.InvalidGrant, + SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired => TokenRequestErrors.InvalidRequest, SendAccessConstants.EmailOtpValidatorResults.EmailRequired => TokenRequestErrors.InvalidRequest, _ => TokenRequestErrors.InvalidGrant }; diff --git a/test/Identity.IntegrationTest/RequestValidation/SendAccess/SendEmailOtpReqestValidatorIntegrationTests.cs b/test/Identity.IntegrationTest/RequestValidation/SendAccess/SendEmailOtpReqestValidatorIntegrationTests.cs index 1c740cd448..d2f29f72b3 100644 --- a/test/Identity.IntegrationTest/RequestValidation/SendAccess/SendEmailOtpReqestValidatorIntegrationTests.cs +++ b/test/Identity.IntegrationTest/RequestValidation/SendAccess/SendEmailOtpReqestValidatorIntegrationTests.cs @@ -85,7 +85,7 @@ public class SendEmailOtpRequestValidatorIntegrationTests(IdentityApplicationFac // Assert var content = await response.Content.ReadAsStringAsync(); Assert.Contains(OidcConstants.TokenErrors.InvalidRequest, content); - Assert.Contains("email otp sent", content); + Assert.Contains("email and otp are required", content); } [Fact] diff --git a/test/Identity.IntegrationTest/RequestValidation/SendAccess/SendNeverAuthenticateRequestValidatorTest.cs b/test/Identity.IntegrationTest/RequestValidation/SendAccess/SendNeverAuthenticateRequestValidatorTest.cs index a81b01a293..4796d1f913 100644 --- a/test/Identity.IntegrationTest/RequestValidation/SendAccess/SendNeverAuthenticateRequestValidatorTest.cs +++ b/test/Identity.IntegrationTest/RequestValidation/SendAccess/SendNeverAuthenticateRequestValidatorTest.cs @@ -63,7 +63,7 @@ public class SendNeverAuthenticateRequestValidatorIntegrationTests( } [Fact] - public async Task SendAccess_NeverAuthenticateSend_WithEmail_ReturnsEmailInvalid() + public async Task SendAccess_NeverAuthenticateSend_WithEmail_ReturnsEmailAndOtpRequired() { // Arrange var email = "test@example.com"; @@ -77,10 +77,10 @@ public class SendNeverAuthenticateRequestValidatorIntegrationTests( var content = await response.Content.ReadAsStringAsync(); // should be invalid grant - Assert.Contains(OidcConstants.TokenErrors.InvalidGrant, content); + Assert.Contains(OidcConstants.TokenErrors.InvalidRequest, content); // Try to compel the invalid email error - var expectedError = SendAccessConstants.EmailOtpValidatorResults.EmailInvalid; + var expectedError = SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired; Assert.Contains(expectedError, content); } diff --git a/test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs b/test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs index 96a097a53c..76a40410c9 100644 --- a/test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs +++ b/test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs @@ -48,9 +48,8 @@ public class SendConstantsSnapshotTests public void EmailOtpValidatorResults_Constants_HaveCorrectValues() { // Assert - Assert.Equal("email_invalid", SendAccessConstants.EmailOtpValidatorResults.EmailInvalid); Assert.Equal("email_required", SendAccessConstants.EmailOtpValidatorResults.EmailRequired); - Assert.Equal("email_and_otp_required_otp_sent", SendAccessConstants.EmailOtpValidatorResults.EmailOtpSent); + Assert.Equal("email_and_otp_required", SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired); Assert.Equal("otp_invalid", SendAccessConstants.EmailOtpValidatorResults.EmailOtpInvalid); Assert.Equal("otp_generation_failed", SendAccessConstants.EmailOtpValidatorResults.OtpGenerationFailed); } diff --git a/test/Identity.Test/IdentityServer/SendAccess/SendEmailOtpRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/SendAccess/SendEmailOtpRequestValidatorTests.cs index d2c7051f69..cbf40456fb 100644 --- a/test/Identity.Test/IdentityServer/SendAccess/SendEmailOtpRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/SendAccess/SendEmailOtpRequestValidatorTests.cs @@ -68,8 +68,8 @@ public class SendEmailOtpRequestValidatorTests // Assert Assert.True(result.IsError); - Assert.Equal(OidcConstants.TokenErrors.InvalidGrant, result.Error); - Assert.Equal("email is invalid.", result.ErrorDescription); + Assert.Equal(OidcConstants.TokenErrors.InvalidRequest, result.Error); + Assert.Equal("email and otp are required.", result.ErrorDescription); // Verify no OTP generation or email sending occurred await sutProvider.GetDependency>() @@ -115,7 +115,7 @@ public class SendEmailOtpRequestValidatorTests // Assert Assert.True(result.IsError); Assert.Equal(OidcConstants.TokenErrors.InvalidRequest, result.Error); - Assert.Equal("email otp sent.", result.ErrorDescription); + Assert.Equal("email and otp are required.", result.ErrorDescription); // Verify OTP generation await sutProvider.GetDependency>() diff --git a/test/Identity.Test/IdentityServer/SendAccess/SendNeverAuthenticateValidatorTests.cs b/test/Identity.Test/IdentityServer/SendAccess/SendNeverAuthenticateValidatorTests.cs index ae0434af83..2ac4bf3255 100644 --- a/test/Identity.Test/IdentityServer/SendAccess/SendNeverAuthenticateValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/SendAccess/SendNeverAuthenticateValidatorTests.cs @@ -51,7 +51,7 @@ public class SendNeverAuthenticateRequestValidatorTests } [Theory, BitAutoData] - public async Task ValidateRequestAsync_EmailErrorSelected_HasEmail_ReturnsEmailInvalid( + public async Task ValidateRequestAsync_EmailErrorSelected_HasEmail_ReturnsEmailAndOtpRequired( SutProvider sutProvider, [AutoFixture.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, string email) @@ -69,12 +69,12 @@ public class SendNeverAuthenticateRequestValidatorTests // Assert Assert.True(result.IsError); - Assert.Equal(OidcConstants.TokenErrors.InvalidGrant, result.Error); - Assert.Equal(SendAccessConstants.EmailOtpValidatorResults.EmailInvalid, result.ErrorDescription); + Assert.Equal(OidcConstants.TokenErrors.InvalidRequest, result.Error); + Assert.Equal(SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired, result.ErrorDescription); var customResponse = result.CustomResponse as Dictionary; Assert.NotNull(customResponse); - Assert.Equal(SendAccessConstants.EmailOtpValidatorResults.EmailInvalid, customResponse[SendAccessConstants.SendAccessError]); + Assert.Equal(SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired, customResponse[SendAccessConstants.SendAccessError]); } [Theory, BitAutoData]