1
0
mirror of https://github.com/bitwarden/server.git synced 2025-02-16 01:51:21 +01:00

[AC-1070] Enforce master password policy on login (#2714)

* [EC-1070] Add API endpoint to retrieve all policies for the current user

The additional API endpoint is required to avoid forcing a full sync call before every login for master password policy enforcement on login.

* [EC-1070] Add MasterPasswordPolicyData model

* [EC-1070] Move PolicyResponseModel to Core project

The response model is used by both the Identity and Api projects.

* [EC-1070] Supply master password polices as a custom identity token response

* [EC-1070] Include master password policies in 2FA token response

* [EC-1070] Add response model to verify-password endpoint that includes master password policies

* [AC-1070] Introduce MasterPasswordPolicyResponseModel

* [AC-1070] Add policy service method to retrieve a user's master password policy

* [AC-1070] User new policy service method

- Update BaseRequestValidator
- Update AccountsController for /verify-password endpoint
- Update VerifyMasterPasswordResponseModel to accept MasterPasswordPolicyData

* [AC-1070] Cleanup new policy service method

- Use User object instead of Guid
- Remove TODO message
- Use `PolicyRepository.GetManyByTypeApplicableToUserIdAsync` instead of filtering locally

* [AC-1070] Cleanup MasterPasswordPolicy models

- Remove default values from both models
- Add missing `RequireLower`
- Fix mismatched properties in `CombineWith` method
- Make properties nullable in response model

* [AC-1070] Remove now un-used GET /policies endpoint

* [AC-1070] Update policy service method to use GetManyByUserIdAsync

* [AC-1070] Ensure existing value is not null before comparison

* [AC-1070] Remove redundant VerifyMasterPasswordResponse model

* [AC-1070] Fix service typo in constructor
This commit is contained in:
Shane Melton 2023-04-17 07:35:47 -07:00 committed by GitHub
parent 972a500745
commit f2fad5513d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 149 additions and 12 deletions

View File

@ -6,6 +6,7 @@ using Bit.Api.Vault.Models.Response;
using Bit.Core.Auth.Services; using Bit.Core.Auth.Services;
using Bit.Core.Entities; using Bit.Core.Entities;
using Bit.Core.Exceptions; using Bit.Core.Exceptions;
using Bit.Core.Models.Api.Response;
using Bit.Core.Repositories; using Bit.Core.Repositories;
using Bit.Core.Services; using Bit.Core.Services;
using Bit.Core.Settings; using Bit.Core.Settings;

View File

@ -12,6 +12,7 @@ using Bit.Core.Entities;
using Bit.Core.Enums; using Bit.Core.Enums;
using Bit.Core.Enums.Provider; using Bit.Core.Enums.Provider;
using Bit.Core.Exceptions; using Bit.Core.Exceptions;
using Bit.Core.Models.Api.Response;
using Bit.Core.Models.Business; using Bit.Core.Models.Business;
using Bit.Core.Models.Data; using Bit.Core.Models.Data;
using Bit.Core.Repositories; using Bit.Core.Repositories;
@ -41,6 +42,7 @@ public class AccountsController : Controller
private readonly ISendRepository _sendRepository; private readonly ISendRepository _sendRepository;
private readonly ISendService _sendService; private readonly ISendService _sendService;
private readonly ICaptchaValidationService _captchaValidationService; private readonly ICaptchaValidationService _captchaValidationService;
private readonly IPolicyService _policyService;
public AccountsController( public AccountsController(
GlobalSettings globalSettings, GlobalSettings globalSettings,
@ -54,7 +56,8 @@ public class AccountsController : Controller
IUserService userService, IUserService userService,
ISendRepository sendRepository, ISendRepository sendRepository,
ISendService sendService, ISendService sendService,
ICaptchaValidationService captchaValidationService) ICaptchaValidationService captchaValidationService,
IPolicyService policyService)
{ {
_cipherRepository = cipherRepository; _cipherRepository = cipherRepository;
_folderRepository = folderRepository; _folderRepository = folderRepository;
@ -68,6 +71,7 @@ public class AccountsController : Controller
_sendRepository = sendRepository; _sendRepository = sendRepository;
_sendService = sendService; _sendService = sendService;
_captchaValidationService = captchaValidationService; _captchaValidationService = captchaValidationService;
_policyService = policyService;
} }
#region DEPRECATED (Moved to Identity Service) #region DEPRECATED (Moved to Identity Service)
@ -261,7 +265,7 @@ public class AccountsController : Controller
} }
[HttpPost("verify-password")] [HttpPost("verify-password")]
public async Task PostVerifyPassword([FromBody] SecretVerificationRequestModel model) public async Task<MasterPasswordPolicyResponseModel> PostVerifyPassword([FromBody] SecretVerificationRequestModel model)
{ {
var user = await _userService.GetUserByPrincipalAsync(User); var user = await _userService.GetUserByPrincipalAsync(User);
if (user == null) if (user == null)
@ -271,7 +275,9 @@ public class AccountsController : Controller
if (await _userService.CheckPasswordAsync(user, model.MasterPasswordHash)) if (await _userService.CheckPasswordAsync(user, model.MasterPasswordHash))
{ {
return; var policyData = await _policyService.GetMasterPasswordPolicyForUserAsync(user);
return new MasterPasswordPolicyResponseModel(policyData);
} }
ModelState.AddModelError(nameof(model.MasterPasswordHash), "Invalid password."); ModelState.AddModelError(nameof(model.MasterPasswordHash), "Invalid password.");

View File

@ -3,6 +3,7 @@ using Bit.Api.Models.Response;
using Bit.Core.Context; using Bit.Core.Context;
using Bit.Core.Enums; using Bit.Core.Enums;
using Bit.Core.Exceptions; using Bit.Core.Exceptions;
using Bit.Core.Models.Api.Response;
using Bit.Core.Repositories; using Bit.Core.Repositories;
using Bit.Core.Services; using Bit.Core.Services;
using Bit.Core.Settings; using Bit.Core.Settings;

View File

@ -1,6 +1,7 @@
using Bit.Api.Models.Response; using Bit.Api.Models.Response;
using Bit.Core.Entities; using Bit.Core.Entities;
using Bit.Core.Models.Api; using Bit.Core.Models.Api;
using Bit.Core.Models.Api.Response;
using Bit.Core.Models.Data; using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Settings; using Bit.Core.Settings;

View File

@ -0,0 +1,36 @@
using Bit.Core.Models.Data.Organizations.Policies;
namespace Bit.Core.Models.Api.Response;
public class MasterPasswordPolicyResponseModel : ResponseModel
{
public MasterPasswordPolicyResponseModel(MasterPasswordPolicyData data) : base("masterPasswordPolicy")
{
if (data == null)
{
return;
}
MinComplexity = data.MinComplexity;
MinLength = data.MinLength;
RequireLower = data.RequireLower;
RequireUpper = data.RequireUpper;
RequireNumbers = data.RequireNumbers;
RequireSpecial = data.RequireSpecial;
EnforceOnLogin = data.EnforceOnLogin;
}
public int? MinComplexity { get; set; }
public int? MinLength { get; set; }
public bool? RequireLower { get; set; }
public bool? RequireUpper { get; set; }
public bool? RequireNumbers { get; set; }
public bool? RequireSpecial { get; set; }
public bool? EnforceOnLogin { get; set; }
}

View File

@ -1,9 +1,8 @@
using System.Text.Json; using System.Text.Json;
using Bit.Core.Entities; using Bit.Core.Entities;
using Bit.Core.Enums; using Bit.Core.Enums;
using Bit.Core.Models.Api;
namespace Bit.Api.Models.Response; namespace Bit.Core.Models.Api.Response;
public class PolicyResponseModel : ResponseModel public class PolicyResponseModel : ResponseModel
{ {

View File

@ -0,0 +1,40 @@
namespace Bit.Core.Models.Data.Organizations.Policies;
public class MasterPasswordPolicyData : IPolicyDataModel
{
public int? MinComplexity { get; set; }
public int? MinLength { get; set; }
public bool? RequireLower { get; set; }
public bool? RequireUpper { get; set; }
public bool? RequireNumbers { get; set; }
public bool? RequireSpecial { get; set; }
public bool? EnforceOnLogin { get; set; }
/// <summary>
/// Combine the other policy data with this instance, taking the most secure options
/// </summary>
/// <param name="other">The other policy instance to combine with this</param>
public void CombineWith(MasterPasswordPolicyData other)
{
if (other == null)
{
return;
}
if (other.MinComplexity.HasValue && (!MinComplexity.HasValue || other.MinComplexity > MinComplexity))
{
MinComplexity = other.MinComplexity;
}
if (other.MinLength.HasValue && (!MinLength.HasValue || other.MinLength > MinLength))
{
MinLength = other.MinLength;
}
RequireLower = (other.RequireLower ?? false) || (RequireLower ?? false);
RequireUpper = (other.RequireUpper ?? false) || (RequireUpper ?? false);
RequireNumbers = (other.RequireNumbers ?? false) || (RequireNumbers ?? false);
RequireSpecial = (other.RequireSpecial ?? false) || (RequireSpecial ?? false);
EnforceOnLogin = (other.EnforceOnLogin ?? false) || (EnforceOnLogin ?? false);
}
}

View File

@ -1,4 +1,5 @@
using Bit.Core.Entities; using Bit.Core.Entities;
using Bit.Core.Models.Data.Organizations.Policies;
namespace Bit.Core.Services; namespace Bit.Core.Services;
@ -6,4 +7,9 @@ public interface IPolicyService
{ {
Task SaveAsync(Policy policy, IUserService userService, IOrganizationService organizationService, Task SaveAsync(Policy policy, IUserService userService, IOrganizationService organizationService,
Guid? savingUserId); Guid? savingUserId);
/// <summary>
/// Get the combined master password policy options for the specified user.
/// </summary>
Task<MasterPasswordPolicyData> GetMasterPasswordPolicyForUserAsync(User user);
} }

View File

@ -2,6 +2,7 @@
using Bit.Core.Entities; using Bit.Core.Entities;
using Bit.Core.Enums; using Bit.Core.Enums;
using Bit.Core.Exceptions; using Bit.Core.Exceptions;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Repositories; using Bit.Core.Repositories;
namespace Bit.Core.Services; namespace Bit.Core.Services;
@ -141,6 +142,27 @@ public class PolicyService : IPolicyService
await _eventService.LogPolicyEventAsync(policy, Enums.EventType.Policy_Updated); await _eventService.LogPolicyEventAsync(policy, Enums.EventType.Policy_Updated);
} }
public async Task<MasterPasswordPolicyData> GetMasterPasswordPolicyForUserAsync(User user)
{
var policies = (await _policyRepository.GetManyByUserIdAsync(user.Id))
.Where(p => p.Type == PolicyType.MasterPassword && p.Enabled)
.ToList();
if (!policies.Any())
{
return null;
}
var enforcedOptions = new MasterPasswordPolicyData();
foreach (var policy in policies)
{
enforcedOptions.CombineWith(policy.GetDataModel<MasterPasswordPolicyData>());
}
return enforcedOptions;
}
private async Task DependsOnSingleOrgAsync(Organization org) private async Task DependsOnSingleOrgAsync(Organization org)
{ {
var singleOrg = await _policyRepository.GetByOrganizationIdTypeAsync(org.Id, PolicyType.SingleOrg); var singleOrg = await _policyRepository.GetByOrganizationIdTypeAsync(org.Id, PolicyType.SingleOrg);

View File

@ -11,6 +11,7 @@ using Bit.Core.Entities;
using Bit.Core.Enums; using Bit.Core.Enums;
using Bit.Core.Identity; using Bit.Core.Identity;
using Bit.Core.Models.Api; using Bit.Core.Models.Api;
using Bit.Core.Models.Api.Response;
using Bit.Core.Models.Data.Organizations; using Bit.Core.Models.Data.Organizations;
using Bit.Core.Repositories; using Bit.Core.Repositories;
using Bit.Core.Services; using Bit.Core.Services;
@ -38,6 +39,7 @@ public abstract class BaseRequestValidator<T> where T : class
private readonly GlobalSettings _globalSettings; private readonly GlobalSettings _globalSettings;
private readonly IPolicyRepository _policyRepository; private readonly IPolicyRepository _policyRepository;
private readonly IUserRepository _userRepository; private readonly IUserRepository _userRepository;
private readonly IPolicyService _policyService;
public BaseRequestValidator( public BaseRequestValidator(
UserManager<User> userManager, UserManager<User> userManager,
@ -54,7 +56,8 @@ public abstract class BaseRequestValidator<T> where T : class
ICurrentContext currentContext, ICurrentContext currentContext,
GlobalSettings globalSettings, GlobalSettings globalSettings,
IPolicyRepository policyRepository, IPolicyRepository policyRepository,
IUserRepository userRepository) IUserRepository userRepository,
IPolicyService policyService)
{ {
_userManager = userManager; _userManager = userManager;
_deviceRepository = deviceRepository; _deviceRepository = deviceRepository;
@ -71,6 +74,7 @@ public abstract class BaseRequestValidator<T> where T : class
_globalSettings = globalSettings; _globalSettings = globalSettings;
_policyRepository = policyRepository; _policyRepository = policyRepository;
_userRepository = userRepository; _userRepository = userRepository;
_policyService = policyService;
} }
protected async Task ValidateAsync(T context, ValidatedTokenRequest request, protected async Task ValidateAsync(T context, ValidatedTokenRequest request,
@ -181,6 +185,7 @@ public abstract class BaseRequestValidator<T> where T : class
customResponse.Add("Key", user.Key); customResponse.Add("Key", user.Key);
} }
customResponse.Add("MasterPasswordPolicy", await GetMasterPasswordPolicy(user));
customResponse.Add("ForcePasswordReset", user.ForcePasswordReset); customResponse.Add("ForcePasswordReset", user.ForcePasswordReset);
customResponse.Add("ResetMasterPassword", string.IsNullOrWhiteSpace(user.MasterPassword)); customResponse.Add("ResetMasterPassword", string.IsNullOrWhiteSpace(user.MasterPassword));
customResponse.Add("Kdf", (byte)user.Kdf); customResponse.Add("Kdf", (byte)user.Kdf);
@ -239,7 +244,8 @@ public abstract class BaseRequestValidator<T> where T : class
new Dictionary<string, object> new Dictionary<string, object>
{ {
{ "TwoFactorProviders", providers.Keys }, { "TwoFactorProviders", providers.Keys },
{ "TwoFactorProviders2", providers } { "TwoFactorProviders2", providers },
{ "MasterPasswordPolicy", await GetMasterPasswordPolicy(user) }
}); });
if (enabledProviders.Count() == 1 && enabledProviders.First().Key == TwoFactorProviderType.Email) if (enabledProviders.Count() == 1 && enabledProviders.First().Key == TwoFactorProviderType.Email)
@ -568,4 +574,18 @@ public abstract class BaseRequestValidator<T> where T : class
var failedLoginCount = user?.FailedLoginCount ?? 0; var failedLoginCount = user?.FailedLoginCount ?? 0;
return unknownDevice && failedLoginCeiling > 0 && failedLoginCount == failedLoginCeiling; return unknownDevice && failedLoginCeiling > 0 && failedLoginCount == failedLoginCeiling;
} }
private async Task<MasterPasswordPolicyResponseModel> GetMasterPasswordPolicy(User user)
{
// Check current context/cache to see if user is in any organizations, avoids extra DB call if not
var orgs = (await _currentContext.OrganizationMembershipAsync(_organizationUserRepository, user.Id))
.ToList();
if (!orgs.Any())
{
return null;
}
return new MasterPasswordPolicyResponseModel(await _policyService.GetMasterPasswordPolicyForUserAsync(user));
}
} }

View File

@ -36,11 +36,12 @@ public class CustomTokenRequestValidator : BaseRequestValidator<CustomTokenReque
GlobalSettings globalSettings, GlobalSettings globalSettings,
IPolicyRepository policyRepository, IPolicyRepository policyRepository,
ISsoConfigRepository ssoConfigRepository, ISsoConfigRepository ssoConfigRepository,
IUserRepository userRepository) IUserRepository userRepository,
IPolicyService policyService)
: base(userManager, deviceRepository, deviceService, userService, eventService, : base(userManager, deviceRepository, deviceService, userService, eventService,
organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository,
applicationCacheService, mailService, logger, currentContext, globalSettings, policyRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, policyRepository,
userRepository) userRepository, policyService)
{ {
_userManager = userManager; _userManager = userManager;
_ssoConfigRepository = ssoConfigRepository; _ssoConfigRepository = ssoConfigRepository;

View File

@ -38,11 +38,12 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator<ResourceOwner
IPolicyRepository policyRepository, IPolicyRepository policyRepository,
ICaptchaValidationService captchaValidationService, ICaptchaValidationService captchaValidationService,
IAuthRequestRepository authRequestRepository, IAuthRequestRepository authRequestRepository,
IUserRepository userRepository) IUserRepository userRepository,
IPolicyService policyService)
: base(userManager, deviceRepository, deviceService, userService, eventService, : base(userManager, deviceRepository, deviceService, userService, eventService,
organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository,
applicationCacheService, mailService, logger, currentContext, globalSettings, policyRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, policyRepository,
userRepository) userRepository, policyService)
{ {
_userManager = userManager; _userManager = userManager;
_userService = userService; _userService = userService;

View File

@ -33,6 +33,7 @@ public class AccountsControllerTests : IDisposable
private readonly ISendService _sendService; private readonly ISendService _sendService;
private readonly IProviderUserRepository _providerUserRepository; private readonly IProviderUserRepository _providerUserRepository;
private readonly ICaptchaValidationService _captchaValidationService; private readonly ICaptchaValidationService _captchaValidationService;
private readonly IPolicyService _policyService;
public AccountsControllerTests() public AccountsControllerTests()
{ {
@ -48,6 +49,7 @@ public class AccountsControllerTests : IDisposable
_sendRepository = Substitute.For<ISendRepository>(); _sendRepository = Substitute.For<ISendRepository>();
_sendService = Substitute.For<ISendService>(); _sendService = Substitute.For<ISendService>();
_captchaValidationService = Substitute.For<ICaptchaValidationService>(); _captchaValidationService = Substitute.For<ICaptchaValidationService>();
_policyService = Substitute.For<IPolicyService>();
_sut = new AccountsController( _sut = new AccountsController(
_globalSettings, _globalSettings,
_cipherRepository, _cipherRepository,
@ -60,7 +62,8 @@ public class AccountsControllerTests : IDisposable
_userService, _userService,
_sendRepository, _sendRepository,
_sendService, _sendService,
_captchaValidationService _captchaValidationService,
_policyService
); );
} }