admin/server
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2026-02-09 10:23:10 +08:00
Code Issues Packages Projects Releases Wiki Activity

[PM-30563] Change error response on Send Access token request (#6911)

Browse Source 
* feat: remove invalid email response and instead return email and OTP required to protect against enumeration attacks.

* fix: fixing tests and dotnet format
This commit is contained in:
Ike
2026-02-04 09:42:32 -05:00
committed by GitHub
parent 52955d1860
commit 5afdfa6fd1
8 changed files with 27 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs
View File

@@ -69,13 +69,9 @@ public static class SendAccessConstants
/// </summary>
public const string EmailRequired = "email_required";
/// <summary>
/// Represents the error code indicating that an email address is invalid.
/// </summary>
public const string EmailInvalid = "email_invalid";
/// <summary>
/// Represents the status indicating that both email and OTP are required, and the OTP has been sent.
/// </summary>
public const string EmailOtpSent = "email_and_otp_required_otp_sent";
public const string EmailAndOtpRequired = "email_and_otp_required";
/// <summary>
/// Represents the status indicating that both email and OTP are required, and the OTP is invalid.
/// </summary>

19
src/Identity/IdentityServer/RequestValidators/SendAccess/SendEmailOtpRequestValidator.cs
View File

@@ -22,8 +22,7 @@ public class SendEmailOtpRequestValidator(
private static readonly Dictionary<string, string> _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<string, object>
@@ -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],

4
src/Identity/IdentityServer/RequestValidators/SendAccess/SendNeverAuthenticateRequestValidator.cs
View File

@@ -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
};

2
test/Identity.IntegrationTest/RequestValidation/SendAccess/SendEmailOtpReqestValidatorIntegrationTests.cs
View File

@@ -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]

6
test/Identity.IntegrationTest/RequestValidation/SendAccess/SendNeverAuthenticateRequestValidatorTest.cs
View File

@@ -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);
}

3
test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs
View File

@@ -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);
}

6
test/Identity.Test/IdentityServer/SendAccess/SendEmailOtpRequestValidatorTests.cs
View File

@@ -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<IOtpTokenProvider<DefaultOtpTokenProviderOptions>>()
@@ -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<IOtpTokenProvider<DefaultOtpTokenProviderOptions>>()

8
test/Identity.Test/IdentityServer/SendAccess/SendNeverAuthenticateValidatorTests.cs
View File

@@ -51,7 +51,7 @@ public class SendNeverAuthenticateRequestValidatorTests
}
[Theory, BitAutoData]
public async Task ValidateRequestAsync_EmailErrorSelected_HasEmail_ReturnsEmailInvalid(
public async Task ValidateRequestAsync_EmailErrorSelected_HasEmail_ReturnsEmailAndOtpRequired(
SutProvider<SendNeverAuthenticateRequestValidator> 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<string, object>;
Assert.NotNull(customResponse);
Assert.Equal(SendAccessConstants.EmailOtpValidatorResults.EmailInvalid, customResponse[SendAccessConstants.SendAccessError]);
Assert.Equal(SendAccessConstants.EmailOtpValidatorResults.EmailAndOtpRequired, customResponse[SendAccessConstants.SendAccessError]);
}
[Theory, BitAutoData]