From 41348b3158c427af2c91842c8e31ae682c142fdc Mon Sep 17 00:00:00 2001 From: Ike Kottlowski Date: Tue, 27 Jan 2026 22:09:22 -0500 Subject: [PATCH] feat: remove invalid email response and instead return email and OTP required to protect against enumeration attacks. --- .../SendAccess/SendAccessConstants.cs | 6 +---- .../SendEmailOtpRequestValidator.cs | 27 +++++++++---------- .../SendNeverAuthenticateRequestValidator.cs | 7 ++--- .../SendAccess/SendConstantsSnapshotTests.cs | 3 +-- 4 files changed, 17 insertions(+), 26 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 34a7a6f6e7..baaec7168e 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,17 +32,18 @@ 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. - if (string.IsNullOrEmpty(email)) + /* + * It is an invalid request if the email is missing or is not in the list of emails in the EmailOtp array. + * This is somewhat contradictory to our process here where a poor shape means invalid_request and invalid + * data is invalid_grant. + * In this case the shape is correct but the data is invalid but to protect against enumeration we treat missing + * or 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 can enumerate until a valid email is found. + */ + if (string.IsNullOrEmpty(email) || !authMethod.Emails.Contains(email)) { // Request is the wrong shape and doesn't contain an email field. - return BuildErrorResult(SendAccessConstants.EmailOtpValidatorResults.EmailRequired); - } - - // email must be in the list of emails in the EmailOtp array - if (!authMethod.Emails.Contains(email)) - { - return BuildErrorResult(SendAccessConstants.EmailOtpValidatorResults.EmailInvalid); + return BuildErrorResult(SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired); } // get otp from request @@ -76,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 @@ -100,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 @@ -108,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..caa0160a0d 100644 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendNeverAuthenticateRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendNeverAuthenticateRequestValidator.cs @@ -37,9 +37,7 @@ public class SendNeverAuthenticateRequestValidator(GlobalSettings globalSettings errorType = SendAccessConstants.SendIdGuidValidatorResults.InvalidSendId; break; case SendAccessConstants.EnumerationProtection.Email: - var hasEmail = request.Get(SendAccessConstants.TokenRequest.Email) is not null; - errorType = hasEmail ? SendAccessConstants.EmailOtpValidatorResults.EmailInvalid - : SendAccessConstants.EmailOtpValidatorResults.EmailRequired; + errorType = SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired; break; case SendAccessConstants.EnumerationProtection.Password: var hasPassword = request.Get(SendAccessConstants.TokenRequest.ClientB64HashedPassword) is not null; @@ -64,8 +62,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.EmailRequired => TokenRequestErrors.InvalidRequest, + SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired => TokenRequestErrors.InvalidRequest, _ => TokenRequestErrors.InvalidGrant }; 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); }