From 7046aecfd581eb0bbf896bb1d784a071baf1262d Mon Sep 17 00:00:00 2001 From: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Date: Wed, 9 Mar 2022 12:07:06 -0600 Subject: [PATCH] [Captcha] BUG Add null checks | Make ceiling default to zero (#1903) * [Captcha] BUG Add null checks | Make ceiling default to zero * Formatting --- src/Core/IdentityServer/BaseRequestValidator.cs | 7 ++++++- src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs | 2 +- src/Core/Services/ICaptchaValidationService.cs | 2 +- .../Services/Implementations/HCaptchaValidationService.cs | 8 ++++---- .../NoopImplementations/NoopCaptchaValidationService.cs | 2 +- src/Core/Settings/GlobalSettings.cs | 2 +- src/Identity/appsettings.SelfHosted.json | 2 +- 7 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Core/IdentityServer/BaseRequestValidator.cs b/src/Core/IdentityServer/BaseRequestValidator.cs index 04fae0fbc2..8503f78c8d 100644 --- a/src/Core/IdentityServer/BaseRequestValidator.cs +++ b/src/Core/IdentityServer/BaseRequestValidator.cs @@ -515,7 +515,7 @@ namespace Bit.Core.IdentityServer private async Task ResetFailedAuthDetailsAsync(User user) { // Early escape if db hit not necessary - if (user.FailedLoginCount == 0) + if (user == null || user.FailedLoginCount == 0) { return; } @@ -527,6 +527,11 @@ namespace Bit.Core.IdentityServer private async Task UpdateFailedAuthDetailsAsync(User user, bool twoFactorInvalid, bool unknownDevice) { + if (user == null) + { + return; + } + var utcNow = DateTime.UtcNow; user.FailedLoginCount = ++user.FailedLoginCount; user.LastFailedLoginDate = user.RevisionDate = utcNow; diff --git a/src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs b/src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs index 458f313be1..dca11723b4 100644 --- a/src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs +++ b/src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs @@ -62,7 +62,7 @@ namespace Bit.Core.IdentityServer string bypassToken = null; var user = await _userManager.FindByEmailAsync(context.UserName.ToLowerInvariant()); var unknownDevice = !await KnownDeviceAsync(user, context.Request); - if (unknownDevice && _captchaValidationService.RequireCaptchaValidation(_currentContext, user.FailedLoginCount)) + if (unknownDevice && _captchaValidationService.RequireCaptchaValidation(_currentContext, user?.FailedLoginCount ?? 0)) { var captchaResponse = context.Request.Raw["captchaResponse"]?.ToString(); diff --git a/src/Core/Services/ICaptchaValidationService.cs b/src/Core/Services/ICaptchaValidationService.cs index 421dce8eb6..8b983ebedf 100644 --- a/src/Core/Services/ICaptchaValidationService.cs +++ b/src/Core/Services/ICaptchaValidationService.cs @@ -8,7 +8,7 @@ namespace Bit.Core.Services { string SiteKey { get; } string SiteKeyResponseKeyName { get; } - bool RequireCaptchaValidation(ICurrentContext currentContext, int? failedLoginCount = null); + bool RequireCaptchaValidation(ICurrentContext currentContext, int failedLoginCount = 0); Task ValidateCaptchaResponseAsync(string captchResponse, string clientIpAddress); string GenerateCaptchaBypassToken(User user); bool ValidateCaptchaBypassToken(string encryptedToken, User user); diff --git a/src/Core/Services/Implementations/HCaptchaValidationService.cs b/src/Core/Services/Implementations/HCaptchaValidationService.cs index 3216efdf7b..e066d62c83 100644 --- a/src/Core/Services/Implementations/HCaptchaValidationService.cs +++ b/src/Core/Services/Implementations/HCaptchaValidationService.cs @@ -83,17 +83,17 @@ namespace Bit.Core.Services return root.GetProperty("success").GetBoolean(); } - public bool RequireCaptchaValidation(ICurrentContext currentContext, int? failedLoginCount = null) + public bool RequireCaptchaValidation(ICurrentContext currentContext, int failedLoginCount = 0) { - var failedLoginCeiling = _globalSettings.Captcha.MaximumFailedLoginAttempts.GetValueOrDefault(); + var failedLoginCeiling = _globalSettings.Captcha.MaximumFailedLoginAttempts; return currentContext.IsBot || _globalSettings.Captcha.ForceCaptchaRequired || - failedLoginCeiling > 0 && failedLoginCount.GetValueOrDefault() >= failedLoginCeiling; + failedLoginCeiling > 0 && failedLoginCount >= failedLoginCeiling; } public bool ValidateFailedAuthEmailConditions(bool unknownDevice, int failedLoginCount) { - var failedLoginCeiling = _globalSettings.Captcha.MaximumFailedLoginAttempts.GetValueOrDefault(); + var failedLoginCeiling = _globalSettings.Captcha.MaximumFailedLoginAttempts; return unknownDevice && failedLoginCeiling > 0 && failedLoginCount == failedLoginCeiling; } diff --git a/src/Core/Services/NoopImplementations/NoopCaptchaValidationService.cs b/src/Core/Services/NoopImplementations/NoopCaptchaValidationService.cs index c6e9ea93df..bf62368ec3 100644 --- a/src/Core/Services/NoopImplementations/NoopCaptchaValidationService.cs +++ b/src/Core/Services/NoopImplementations/NoopCaptchaValidationService.cs @@ -8,7 +8,7 @@ namespace Bit.Core.Services { public string SiteKeyResponseKeyName => null; public string SiteKey => null; - public bool RequireCaptchaValidation(ICurrentContext currentContext, int? failedLoginCount) => false; + public bool RequireCaptchaValidation(ICurrentContext currentContext, int failedLoginCount = 0) => false; public bool ValidateFailedAuthEmailConditions(bool unknownDevice, int failedLoginCount) => false; public string GenerateCaptchaBypassToken(User user) => ""; public bool ValidateCaptchaBypassToken(string encryptedToken, User user) => false; diff --git a/src/Core/Settings/GlobalSettings.cs b/src/Core/Settings/GlobalSettings.cs index f9da63e6d6..5259c75643 100644 --- a/src/Core/Settings/GlobalSettings.cs +++ b/src/Core/Settings/GlobalSettings.cs @@ -463,7 +463,7 @@ namespace Bit.Core.Settings public bool ForceCaptchaRequired { get; set; } = false; public string HCaptchaSecretKey { get; set; } public string HCaptchaSiteKey { get; set; } - public int? MaximumFailedLoginAttempts { get; set; } + public int MaximumFailedLoginAttempts { get; set; } } public class StripeSettings diff --git a/src/Identity/appsettings.SelfHosted.json b/src/Identity/appsettings.SelfHosted.json index 9d765a6771..5efd8d6518 100644 --- a/src/Identity/appsettings.SelfHosted.json +++ b/src/Identity/appsettings.SelfHosted.json @@ -15,7 +15,7 @@ "internalSso": null }, "captcha": { - "maximumFailedLoginAttempts": null + "maximumFailedLoginAttempts": 0 } } }