nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-07-01 16:12:49 -05:00
Code Activity

chore(captcha): [PM-15162] Remove captcha enforcement and issuing of bypass token

Browse Source 
* Remove captcha enforcement and issuing/verification of bypass token

* Removed more captcha logic.

* Removed logic to enforce failed login attempts

* Linting.

* Fixed order of initialization.

* Fixed merge conflicts

* Renamed registration finish response for clarity

* Remove unnecessary mailService references.
This commit is contained in:
Todd Martin
2025-05-09 10:44:38 -04:00
committed by GitHub
parent 2918d46b62
commit 80e7a0afd6
37 changed files with 22 additions and 740 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/Identity/Controllers/AccountsController.cs
View File

@ -5,7 +5,6 @@ using Bit.Core.Auth.Enums;
using Bit.Core.Auth.Models.Api.Request.Accounts;
using Bit.Core.Auth.Models.Api.Response.Accounts;
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Auth.Services;
using Bit.Core.Auth.UserFeatures.Registration;
using Bit.Core.Auth.UserFeatures.WebAuthnLogin;
using Bit.Core.Context;
@ -37,7 +36,6 @@ public class AccountsController : Controller
private readonly ILogger<AccountsController> _logger;
private readonly IUserRepository _userRepository;
private readonly IRegisterUserCommand _registerUserCommand;
private readonly ICaptchaValidationService _captchaValidationService;
private readonly IDataProtectorTokenFactory<WebAuthnLoginAssertionOptionsTokenable> _assertionOptionsDataProtector;
private readonly IGetWebAuthnLoginCredentialAssertionOptionsCommand _getWebAuthnLoginCredentialAssertionOptionsCommand;
private readonly ISendVerificationEmailForRegistrationCommand _sendVerificationEmailForRegistrationCommand;
@ -85,7 +83,6 @@ public class AccountsController : Controller
ILogger<AccountsController> logger,
IUserRepository userRepository,
IRegisterUserCommand registerUserCommand,
ICaptchaValidationService captchaValidationService,
IDataProtectorTokenFactory<WebAuthnLoginAssertionOptionsTokenable> assertionOptionsDataProtector,
IGetWebAuthnLoginCredentialAssertionOptionsCommand getWebAuthnLoginCredentialAssertionOptionsCommand,
ISendVerificationEmailForRegistrationCommand sendVerificationEmailForRegistrationCommand,
@ -99,7 +96,6 @@ public class AccountsController : Controller
_logger = logger;
_userRepository = userRepository;
_registerUserCommand = registerUserCommand;
_captchaValidationService = captchaValidationService;
_assertionOptionsDataProtector = assertionOptionsDataProtector;
_getWebAuthnLoginCredentialAssertionOptionsCommand = getWebAuthnLoginCredentialAssertionOptionsCommand;
_sendVerificationEmailForRegistrationCommand = sendVerificationEmailForRegistrationCommand;
@ -167,7 +163,7 @@ public class AccountsController : Controller
}
[HttpPost("register/finish")]
public async Task<RegisterResponseModel> PostRegisterFinish([FromBody] RegisterFinishRequestModel model)
public async Task<RegisterFinishResponseModel> PostRegisterFinish([FromBody] RegisterFinishRequestModel model)
{
var user = model.ToUser();
@ -208,12 +204,11 @@ public class AccountsController : Controller
}
}
private RegisterResponseModel ProcessRegistrationResult(IdentityResult result, User user)
private RegisterFinishResponseModel ProcessRegistrationResult(IdentityResult result, User user)
{
if (result.Succeeded)
{
var captchaBypassToken = _captchaValidationService.GenerateCaptchaBypassToken(user);
return new RegisterResponseModel(captchaBypassToken);
return new RegisterFinishResponseModel();
}
foreach (var error in result.Errors.Where(e => e.Code != "DuplicateUserName"))

6
src/Identity/IdentityServer/CustomValidatorRequestContext.cs
View File

@ -1,5 +1,4 @@
using Bit.Core.Auth.Models.Business;
using Bit.Core.Entities;
using Bit.Core.Entities;
using Duende.IdentityServer.Validation;
namespace Bit.Identity.IdentityServer;
@ -9,7 +8,7 @@ public class CustomValidatorRequestContext
public User User { get; set; }
/// <summary>
/// This is the device that the user is using to authenticate. It can be either known or unknown.
/// We set it here since the ResourceOwnerPasswordValidator needs the device to know if CAPTCHA is required.
/// We set it here since the ResourceOwnerPasswordValidator needs the device to do device validation.
/// The option to set it here saves a trip to the database.
/// </summary>
public Device Device { get; set; }
@ -39,5 +38,4 @@ public class CustomValidatorRequestContext
/// This will be null if the authentication request is successful.
/// </summary>
public Dictionary<string, object> CustomResponse { get; set; }
public CaptchaResponse CaptchaResponse { get; set; }
}

50
src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs
View File

@ -29,7 +29,6 @@ public abstract class BaseRequestValidator<T> where T : class
private readonly IDeviceValidator _deviceValidator;
private readonly ITwoFactorAuthenticationValidator _twoFactorAuthenticationValidator;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IMailService _mailService;
private readonly ILogger _logger;
private readonly GlobalSettings _globalSettings;
private readonly IUserRepository _userRepository;
@ -49,7 +48,6 @@ public abstract class BaseRequestValidator<T> where T : class
IDeviceValidator deviceValidator,
ITwoFactorAuthenticationValidator twoFactorAuthenticationValidator,
IOrganizationUserRepository organizationUserRepository,
IMailService mailService,
ILogger logger,
ICurrentContext currentContext,
GlobalSettings globalSettings,
@ -66,7 +64,6 @@ public abstract class BaseRequestValidator<T> where T : class
_deviceValidator = deviceValidator;
_twoFactorAuthenticationValidator = twoFactorAuthenticationValidator;
_organizationUserRepository = organizationUserRepository;
_mailService = mailService;
_logger = logger;
CurrentContext = currentContext;
_globalSettings = globalSettings;
@ -81,23 +78,12 @@ public abstract class BaseRequestValidator<T> where T : class
protected async Task ValidateAsync(T context, ValidatedTokenRequest request,
CustomValidatorRequestContext validatorContext)
{
// 1. We need to check if the user is a bot and if their master password hash is correct.
var isBot = validatorContext.CaptchaResponse?.IsBot ?? false;
// 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 || isBot)
if (!valid)
{
if (isBot)
{
_logger.LogInformation(Constants.BypassFiltersEventId,
"Login attempt for {UserName} detected as a captcha bot with score {CaptchaScore}.",
request.UserName, validatorContext.CaptchaResponse.Score);
}
if (!valid)
{
await UpdateFailedAuthDetailsAsync(user, false, !validatorContext.KnownDevice);
}
await UpdateFailedAuthDetailsAsync(user);
await BuildErrorResultAsync("Username or password is incorrect. Try again.", false, context, user);
return;
@ -167,7 +153,7 @@ public abstract class BaseRequestValidator<T> where T : class
}
else
{
await UpdateFailedAuthDetailsAsync(user, true, !validatorContext.KnownDevice);
await UpdateFailedAuthDetailsAsync(user);
await BuildErrorResultAsync("Two-step token is invalid. Try again.", true, context, user);
}
return;
@ -379,7 +365,7 @@ public abstract class BaseRequestValidator<T> where T : class
await _userRepository.ReplaceAsync(user);
}
private async Task UpdateFailedAuthDetailsAsync(User user, bool twoFactorInvalid, bool unknownDevice)
private async Task UpdateFailedAuthDetailsAsync(User user)
{
if (user == null)
{
@ -390,32 +376,6 @@ public abstract class BaseRequestValidator<T> where T : class
user.FailedLoginCount = ++user.FailedLoginCount;
user.LastFailedLoginDate = user.RevisionDate = utcNow;
await _userRepository.ReplaceAsync(user);
if (ValidateFailedAuthEmailConditions(unknownDevice, user))
{
if (twoFactorInvalid)
{
await _mailService.SendFailedTwoFactorAttemptsEmailAsync(user.Email, utcNow, CurrentContext.IpAddress);
}
else
{
await _mailService.SendFailedLoginAttemptsEmailAsync(user.Email, utcNow, CurrentContext.IpAddress);
}
}
}
/// <summary>
/// checks to see if a user is trying to log into a new device
/// and has reached the maximum number of failed login attempts.
/// </summary>
/// <param name="unknownDevice">boolean</param>
/// <param name="user">current user</param>
/// <returns></returns>
private bool ValidateFailedAuthEmailConditions(bool unknownDevice, User user)
{
var failedLoginCeiling = _globalSettings.Captcha.MaximumFailedLoginAttempts;
var failedLoginCount = user?.FailedLoginCount ?? 0;
return unknownDevice && failedLoginCeiling > 0 && failedLoginCount == failedLoginCeiling;
}
private async Task<MasterPasswordPolicyResponseModel> GetMasterPasswordPolicyAsync(User user)

2
src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs
View File

@ -35,7 +35,6 @@ public class CustomTokenRequestValidator : BaseRequestValidator<CustomTokenReque
IDeviceValidator deviceValidator,
ITwoFactorAuthenticationValidator twoFactorAuthenticationValidator,
IOrganizationUserRepository organizationUserRepository,
IMailService mailService,
ILogger<CustomTokenRequestValidator> logger,
ICurrentContext currentContext,
GlobalSettings globalSettings,
@ -53,7 +52,6 @@ public class CustomTokenRequestValidator : BaseRequestValidator<CustomTokenReque
deviceValidator,
twoFactorAuthenticationValidator,
organizationUserRepository,
mailService,
logger,
currentContext,
globalSettings,

36
src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs
View File

@ -3,7 +3,6 @@ using Bit.Core;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.Repositories;
using Bit.Core.Auth.Services;
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Repositories;
@ -21,7 +20,6 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator<ResourceOwner
{
private UserManager<User> _userManager;
private readonly ICurrentContext _currentContext;
private readonly ICaptchaValidationService _captchaValidationService;
private readonly IAuthRequestRepository _authRequestRepository;
private readonly IDeviceValidator _deviceValidator;
public ResourceOwnerPasswordValidator(
@ -31,11 +29,9 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator<ResourceOwner
IDeviceValidator deviceValidator,
ITwoFactorAuthenticationValidator twoFactorAuthenticationValidator,
IOrganizationUserRepository organizationUserRepository,
IMailService mailService,
ILogger<ResourceOwnerPasswordValidator> logger,
ICurrentContext currentContext,
GlobalSettings globalSettings,
ICaptchaValidationService captchaValidationService,
IAuthRequestRepository authRequestRepository,
IUserRepository userRepository,
IPolicyService policyService,
@ -50,7 +46,6 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator<ResourceOwner
deviceValidator,
twoFactorAuthenticationValidator,
organizationUserRepository,
mailService,
logger,
currentContext,
globalSettings,
@ -63,7 +58,6 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator<ResourceOwner
{
_userManager = userManager;
_currentContext = currentContext;
_captchaValidationService = captchaValidationService;
_authRequestRepository = authRequestRepository;
_deviceValidator = deviceValidator;
}
@ -88,37 +82,7 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator<ResourceOwner
Device = knownDevice ?? requestDevice,
};
string bypassToken = null;
if (!validatorContext.KnownDevice &&
_captchaValidationService.RequireCaptchaValidation(_currentContext, user))
{
var captchaResponse = context.Request.Raw["captchaResponse"]?.ToString();
if (string.IsNullOrWhiteSpace(captchaResponse))
{
context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Captcha required.",
new Dictionary<string, object>
{
{ _captchaValidationService.SiteKeyResponseKeyName, _captchaValidationService.SiteKey },
});
return;
}
validatorContext.CaptchaResponse = await _captchaValidationService.ValidateCaptchaResponseAsync(
captchaResponse, _currentContext.IpAddress, user);
if (!validatorContext.CaptchaResponse.Success)
{
await BuildErrorResultAsync("Captcha is invalid. Please refresh and try again", false, context, null);
return;
}
bypassToken = _captchaValidationService.GenerateCaptchaBypassToken(user);
}
await ValidateAsync(context, context.Request, validatorContext);
if (context.Result.CustomResponse != null && bypassToken != null)
{
context.Result.CustomResponse["CaptchaBypassToken"] = bypassToken;
}
}
protected async override Task<bool> ValidateContextAsync(ResourceOwnerPasswordValidationContext context,

2
src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs
View File

@ -35,7 +35,6 @@ public class WebAuthnGrantValidator : BaseRequestValidator<ExtensionGrantValidat
IDeviceValidator deviceValidator,
ITwoFactorAuthenticationValidator twoFactorAuthenticationValidator,
IOrganizationUserRepository organizationUserRepository,
IMailService mailService,
ILogger<CustomTokenRequestValidator> logger,
ICurrentContext currentContext,
GlobalSettings globalSettings,
@ -54,7 +53,6 @@ public class WebAuthnGrantValidator : BaseRequestValidator<ExtensionGrantValidat
deviceValidator,
twoFactorAuthenticationValidator,
organizationUserRepository,
mailService,
logger,
currentContext,
globalSettings,

4
src/Identity/Models/Request/Accounts/RegisterRequestModel.cs
View File

@ -1,7 +1,6 @@
using System.ComponentModel.DataAnnotations;
using System.Text.Json;
using Bit.Core;
using Bit.Core.Auth.Models.Api;
using Bit.Core.Auth.Models.Api.Request.Accounts;
using Bit.Core.Entities;
using Bit.Core.Enums;
@ -9,7 +8,7 @@ using Bit.Core.Utilities;
namespace Bit.Identity.Models.Request.Accounts;
public class RegisterRequestModel : IValidatableObject, ICaptchaProtectedModel
public class RegisterRequestModel : IValidatableObject
{
[StringLength(50)]
public string Name { get; set; }
@ -22,7 +21,6 @@ public class RegisterRequestModel : IValidatableObject, ICaptchaProtectedModel
public string MasterPasswordHash { get; set; }
[StringLength(50)]
public string MasterPasswordHint { get; set; }
public string CaptchaResponse { get; set; }
public string Key { get; set; }
public KeysRequestModel Keys { get; set; }
public string Token { get; set; }

5
src/Identity/Models/Response/Accounts/ICaptchaProtectedResponseModel.cs
View File

@ -1,5 +0,0 @@
namespace Bit.Identity.Models.Response.Accounts;
public interface ICaptchaProtectedResponseModel
{
public string CaptchaBypassToken { get; set; }
}

10
src/Identity/Models/Response/Accounts/RegisterFinishResponseModel.cs Normal file
View File

@ -0,0 +1,10 @@
using Bit.Core.Models.Api;
namespace Bit.Identity.Models.Response.Accounts;
public class RegisterFinishResponseModel : ResponseModel
{
public RegisterFinishResponseModel()
: base("registerFinish")
{ }
}

14
src/Identity/Models/Response/Accounts/RegisterResponseModel.cs
View File

@ -1,14 +0,0 @@
using Bit.Core.Models.Api;
namespace Bit.Identity.Models.Response.Accounts;
public class RegisterResponseModel : ResponseModel, ICaptchaProtectedResponseModel
{
public RegisterResponseModel(string captchaBypassToken)
: base("register")
{
CaptchaBypassToken = captchaBypassToken;
}
public string CaptchaBypassToken { get; set; }
}

3
src/Identity/appsettings.Production.json
View File

@ -17,9 +17,6 @@
},
"braintree": {
"production": true
},
"captcha": {
"maximumFailedLoginAttempts": 5
}
},
"Logging": {

3
src/Identity/appsettings.SelfHosted.json
View File

@ -14,9 +14,6 @@
"internalVault": null,
"internalSso": null,
"internalScim": null
},
"captcha": {
"maximumFailedLoginAttempts": 0
}
}
}