1
0
mirror of https://github.com/bitwarden/server.git synced 2024-11-21 12:05:42 +01:00

[PS-616] [PS-795] Fix/auto enroll master password reset without user verification (#2038)

* Fix parameter name to match entity

* Deserialize policy data in object

* Add policy with config type to fixtures

* Return policy with deserialized config

* Use CoreHelper serializers

* Add master password reset on accept request

* Simplify policy data parsing

* Linter
This commit is contained in:
Matt Gibson 2022-06-08 08:44:28 -05:00 committed by GitHub
parent 194b76c13d
commit ef403b4362
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 182 additions and 39 deletions

View File

@ -10,6 +10,7 @@ using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Microsoft.AspNetCore.Authorization;
@ -27,6 +28,7 @@ namespace Bit.Api.Controllers
private readonly ICollectionRepository _collectionRepository;
private readonly IGroupRepository _groupRepository;
private readonly IUserService _userService;
private readonly IPolicyRepository _policyRepository;
private readonly ICurrentContext _currentContext;
public OrganizationUsersController(
@ -36,6 +38,7 @@ namespace Bit.Api.Controllers
ICollectionRepository collectionRepository,
IGroupRepository groupRepository,
IUserService userService,
IPolicyRepository policyRepository,
ICurrentContext currentContext)
{
_organizationRepository = organizationRepository;
@ -44,6 +47,7 @@ namespace Bit.Api.Controllers
_collectionRepository = collectionRepository;
_groupRepository = groupRepository;
_userService = userService;
_policyRepository = policyRepository;
_currentContext = currentContext;
}
@ -169,8 +173,8 @@ namespace Bit.Api.Controllers
await _organizationService.ResendInviteAsync(orgGuidId, userId.Value, new Guid(id));
}
[HttpPost("{id}/accept")]
public async Task Accept(string orgId, string id, [FromBody] OrganizationUserAcceptRequestModel model)
[HttpPost("{organizationUserId}/accept")]
public async Task Accept(Guid orgId, Guid organizationUserId, [FromBody] OrganizationUserAcceptRequestModel model)
{
var user = await _userService.GetUserByPrincipalAsync(User);
if (user == null)
@ -178,7 +182,23 @@ namespace Bit.Api.Controllers
throw new UnauthorizedAccessException();
}
var result = await _organizationService.AcceptUserAsync(new Guid(id), user, model.Token, _userService);
var masterPasswordPolicy = await _policyRepository.GetByOrganizationIdTypeAsync<ResetPasswordDataModel>(orgId, PolicyType.MasterPassword);
var useMasterPasswordPolicy = masterPasswordPolicy != null &&
masterPasswordPolicy.Enabled &&
masterPasswordPolicy.DataModel.AutoEnrollEnabled;
if (useMasterPasswordPolicy &&
string.IsNullOrWhiteSpace(model.ResetPasswordKey))
{
throw new BadRequestException(string.Empty, "Master Password reset is required, but not provided.");
}
await _organizationService.AcceptUserAsync(organizationUserId, user, model.Token, _userService);
if (useMasterPasswordPolicy)
{
await _organizationService.UpdateUserResetPasswordEnrollmentAsync(orgId, user.Id, model.ResetPasswordKey, user.Id);
}
}
[HttpPost("{id}/confirm")]
@ -277,7 +297,7 @@ namespace Bit.Api.Controllers
throw new UnauthorizedAccessException();
}
if (!await _userService.VerifySecretAsync(user, model.Secret))
if (model.ResetPasswordKey != null && !await _userService.VerifySecretAsync(user, model.Secret))
{
await Task.Delay(2000);
throw new BadRequestException("MasterPasswordHash", "Invalid password.");

View File

@ -12,7 +12,7 @@ using Bit.Core.Context;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.OrganizationFeatures.OrganizationApiKeys.Interfaces;
using Bit.Core.Repositories;
using Bit.Core.Services;

View File

@ -40,6 +40,8 @@ namespace Bit.Api.Models.Request.Organizations
{
[Required]
public string Token { get; set; }
// Used to auto-enroll in master password reset
public string ResetPasswordKey { get; set; }
}
public class OrganizationUserConfirmRequestModel

View File

@ -1,5 +1,6 @@
using System;
using Bit.Core.Enums;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Utilities;
namespace Bit.Core.Entities
@ -18,5 +19,24 @@ namespace Bit.Core.Entities
{
Id = CoreHelpers.GenerateComb();
}
public T GetDataModel<T>() where T : IPolicyDataModel, new()
{
return CoreHelpers.LoadClassFromJsonData<T>(Data);
}
public void SetDataModel<T>(T dataModel) where T : IPolicyDataModel, new()
{
Data = CoreHelpers.ClassToJsonData(dataModel);
}
}
public class Policy<T> : Policy where T : IPolicyDataModel, new()
{
public T DataModel
{
get => GetDataModel<T>();
set => SetDataModel(value);
}
}
}

View File

@ -0,0 +1,6 @@
namespace Bit.Core.Models.Data.Organizations.Policies
{
public interface IPolicyDataModel
{
}
}

View File

@ -1,8 +1,8 @@
using System.ComponentModel.DataAnnotations;
namespace Bit.Core.Models.Data
namespace Bit.Core.Models.Data.Organizations.Policies
{
public class ResetPasswordDataModel
public class ResetPasswordDataModel : IPolicyDataModel
{
[Display(Name = "ResetPasswordAutoEnrollCheckbox")]
public bool AutoEnrollEnabled { get; set; }

View File

@ -1,8 +1,8 @@
using System.ComponentModel.DataAnnotations;
namespace Bit.Core.Models.Data
namespace Bit.Core.Models.Data.Organizations.Policies
{
public class SendOptionsPolicyData
public class SendOptionsPolicyData : IPolicyDataModel
{
[Display(Name = "DisableHideEmail")]
public bool DisableHideEmail { get; set; }

View File

@ -3,12 +3,14 @@ using System.Collections.Generic;
using System.Threading.Tasks;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Models.Data.Organizations.Policies;
namespace Bit.Core.Repositories
{
public interface IPolicyRepository : IRepository<Policy, Guid>
{
Task<Policy> GetByOrganizationIdTypeAsync(Guid organizationId, PolicyType type);
Task<Policy<T>> GetByOrganizationIdTypeAsync<T>(Guid organizationId, PolicyType type) where T : IPolicyDataModel, new();
Task<ICollection<Policy>> GetManyByOrganizationIdAsync(Guid organizationId);
Task<ICollection<Policy>> GetManyByUserIdAsync(Guid userId);
Task<ICollection<Policy>> GetManyByTypeApplicableToUserIdAsync(Guid userId, PolicyType policyType,

View File

@ -51,7 +51,7 @@ namespace Bit.Core.Services
Task<List<Tuple<OrganizationUser, string>>> DeleteUsersAsync(Guid organizationId,
IEnumerable<Guid> organizationUserIds, Guid? deletingUserId);
Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable<Guid> groupIds, Guid? loggedInUserId);
Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid organizationUserId, string resetPasswordKey, Guid? callingUserId);
Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId);
Task<OrganizationLicense> GenerateLicenseAsync(Guid organizationId, Guid installationId);
Task<OrganizationLicense> GenerateLicenseAsync(Organization organization, Guid installationId,
int? version = null);

View File

@ -10,6 +10,7 @@ using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Repositories;
using Bit.Core.Settings;
using Bit.Core.Utilities;
@ -1751,10 +1752,10 @@ namespace Bit.Core.Services
EventType.OrganizationUser_UpdatedGroups);
}
public async Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid organizationUserId, string resetPasswordKey, Guid? callingUserId)
public async Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId)
{
// Org User must be the same as the calling user and the organization ID associated with the user must match passed org ID
var orgUser = await _organizationUserRepository.GetByOrganizationAsync(organizationId, organizationUserId);
var orgUser = await _organizationUserRepository.GetByOrganizationAsync(organizationId, userId);
if (!callingUserId.HasValue || orgUser == null || orgUser.UserId != callingUserId.Value ||
orgUser.OrganizationId != organizationId)
{

View File

@ -1,5 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Bit.Core.Entities;

View File

@ -9,6 +9,7 @@ using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Repositories;
using Bit.Core.Settings;
using Bit.Core.Utilities;
@ -291,15 +292,10 @@ namespace Bit.Core.Services
if (send.HideEmail.GetValueOrDefault())
{
var sendOptionsPolicies = await _policyRepository.GetManyByTypeApplicableToUserIdAsync(userId.Value, PolicyType.SendOptions);
foreach (var policy in sendOptionsPolicies)
{
var data = CoreHelpers.LoadClassFromJsonData<SendOptionsPolicyData>(policy.Data);
if (data?.DisableHideEmail ?? false)
if (sendOptionsPolicies.Any(p => p.GetDataModel<SendOptionsPolicyData>()?.DisableHideEmail ?? false))
{
throw new BadRequestException("Due to an Enterprise Policy, you are not allowed to hide your email address from recipients when creating or editing a Send.");
}
}
}
}

View File

@ -6,6 +6,7 @@ using System.Linq;
using System.Threading.Tasks;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Repositories;
using Bit.Core.Settings;
using Dapper;
@ -21,6 +22,9 @@ namespace Bit.Infrastructure.Dapper.Repositories
public PolicyRepository(string connectionString, string readOnlyConnectionString)
: base(connectionString, readOnlyConnectionString)
{ }
public async Task<Policy<T>> GetByOrganizationIdTypeAsync<T>(Guid organizationId, PolicyType type) where T : IPolicyDataModel, new() =>
(Policy<T>)await GetByOrganizationIdTypeAsync(organizationId, type);
public async Task<Policy> GetByOrganizationIdTypeAsync(Guid organizationId, PolicyType type)
{
using (var connection = new SqlConnection(ConnectionString))

View File

@ -4,6 +4,7 @@ using System.Linq;
using System.Threading.Tasks;
using AutoMapper;
using Bit.Core.Enums;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Repositories;
using Bit.Infrastructure.EntityFramework.Models;
using Bit.Infrastructure.EntityFramework.Repositories.Queries;
@ -18,6 +19,8 @@ namespace Bit.Infrastructure.EntityFramework.Repositories
: base(serviceScopeFactory, mapper, (DatabaseContext context) => context.Policies)
{ }
public async Task<Core.Entities.Policy<T>> GetByOrganizationIdTypeAsync<T>(Guid organizationId, PolicyType type) where T : IPolicyDataModel, new() =>
(Core.Entities.Policy<T>)await GetByOrganizationIdTypeAsync(organizationId, type);
public async Task<Core.Entities.Policy> GetByOrganizationIdTypeAsync(Guid organizationId, PolicyType type)
{
using (var scope = ServiceScopeFactory.CreateScope())

View File

@ -0,0 +1,68 @@
using System;
using System.Threading.Tasks;
using Bit.Api.Controllers;
using Bit.Api.Models.Request.Organizations;
using Bit.Api.Test.AutoFixture.Attributes;
using Bit.Core.Entities;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using NSubstitute;
using Xunit;
namespace Bit.Api.Test.Controllers
{
[ControllerCustomize(typeof(OrganizationUsersController))]
[SutProviderCustomize]
public class OrganizationUsersControllerTests
{
[Theory]
[BitAutoData]
public async Task Accept_RequiresKnownUser(Guid orgId, Guid orgUserId, OrganizationUserAcceptRequestModel model,
SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs((User)null);
await Assert.ThrowsAsync<UnauthorizedAccessException>(() => sutProvider.Sut.Accept(orgId, orgUserId, model));
}
[Theory]
[BitAutoData]
public async Task Accept_NoMasterPasswordReset(Guid orgId, Guid orgUserId,
OrganizationUserAcceptRequestModel model, User user, SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user);
await sutProvider.Sut.Accept(orgId, orgUserId, model);
await sutProvider.GetDependency<IOrganizationService>().Received(1)
.AcceptUserAsync(orgUserId, user, model.Token, sutProvider.GetDependency<IUserService>());
await sutProvider.GetDependency<IOrganizationService>().DidNotReceiveWithAnyArgs()
.UpdateUserResetPasswordEnrollmentAsync(default, default, default, default);
}
[Theory]
[BitAutoData]
public async Task Accept_RequireMasterPasswordReset(Guid orgId, Guid orgUserId,
OrganizationUserAcceptRequestModel model, User user, SutProvider<OrganizationUsersController> sutProvider)
{
var policy = new Policy<ResetPasswordDataModel>
{
Enabled = true,
DataModel = new ResetPasswordDataModel { AutoEnrollEnabled = true, },
};
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user);
sutProvider.GetDependency<IPolicyRepository>().GetByOrganizationIdTypeAsync<ResetPasswordDataModel>(orgId,
Core.Enums.PolicyType.MasterPassword).Returns(policy);
await sutProvider.Sut.Accept(orgId, orgUserId, model);
await sutProvider.GetDependency<IOrganizationService>().Received(1)
.AcceptUserAsync(orgUserId, user, model.Token, sutProvider.GetDependency<IUserService>());
await sutProvider.GetDependency<IOrganizationService>().Received(1)
.UpdateUserResetPasswordEnrollmentAsync(orgId, user.Id, model.ResetPasswordKey, user.Id);
}
}
}

View File

@ -1,10 +1,12 @@
using System;
using System.Reflection;
using System.Text.Json;
using AutoFixture;
using AutoFixture.Kernel;
using AutoFixture.Xunit2;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Test.AutoFixture.EntityFrameworkRepositoryFixtures;
using Bit.Core.Test.AutoFixture.OrganizationFixtures;
using Bit.Infrastructure.EntityFramework.Repositories;
@ -28,6 +30,16 @@ namespace Bit.Core.Test.AutoFixture.PolicyFixtures
.With(o => o.OrganizationId, Guid.NewGuid())
.With(o => o.Type, Type)
.With(o => o.Enabled, true));
fixture.Customize<Policy<ResetPasswordDataModel>>(composer => composer
.With(o => o.Data, JsonSerializer.Serialize(fixture.Create<ResetPasswordDataModel>()))
.With(o => o.OrganizationId, Guid.NewGuid())
.With(o => o.Type, Type)
.With(o => o.Enabled, true));
fixture.Customize<Policy<SendOptionsPolicyData>>(composer => composer
.With(o => o.Data, JsonSerializer.Serialize(fixture.Create<ResetPasswordDataModel>()))
.With(o => o.OrganizationId, Guid.NewGuid())
.With(o => o.Type, Type)
.With(o => o.Enabled, true));
}
}

View File

@ -15,10 +15,12 @@ using PolicyFixtures = Bit.Core.Test.AutoFixture.PolicyFixtures;
namespace Bit.Core.Test.Services
{
[SutProviderCustomize]
public class PolicyServiceTests
{
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
public async Task SaveAsync_OrganizationDoesNotExist_ThrowsBadRequest([PolicyFixtures.Policy(PolicyType.DisableSend)] Policy policy, SutProvider<PolicyService> sutProvider)
[Theory, BitAutoData]
public async Task SaveAsync_OrganizationDoesNotExist_ThrowsBadRequest(
[PolicyFixtures.Policy(PolicyType.DisableSend)] Policy policy, SutProvider<PolicyService> sutProvider)
{
SetupOrg(sutProvider, policy.OrganizationId, null);
@ -39,8 +41,9 @@ namespace Bit.Core.Test.Services
.LogPolicyEventAsync(default, default, default);
}
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
public async Task SaveAsync_OrganizationCannotUsePolicies_ThrowsBadRequest([PolicyFixtures.Policy(PolicyType.DisableSend)] Policy policy, SutProvider<PolicyService> sutProvider)
[Theory, BitAutoData]
public async Task SaveAsync_OrganizationCannotUsePolicies_ThrowsBadRequest(
[PolicyFixtures.Policy(PolicyType.DisableSend)] Policy policy, SutProvider<PolicyService> sutProvider)
{
var orgId = Guid.NewGuid();
@ -66,8 +69,9 @@ namespace Bit.Core.Test.Services
.LogPolicyEventAsync(default, default, default);
}
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
public async Task SaveAsync_SingleOrg_RequireSsoEnabled_ThrowsBadRequest([PolicyFixtures.Policy(PolicyType.SingleOrg)] Policy policy, SutProvider<PolicyService> sutProvider)
[Theory, BitAutoData]
public async Task SaveAsync_SingleOrg_RequireSsoEnabled_ThrowsBadRequest(
[PolicyFixtures.Policy(PolicyType.SingleOrg)] Policy policy, SutProvider<PolicyService> sutProvider)
{
policy.Enabled = false;
@ -98,7 +102,7 @@ namespace Bit.Core.Test.Services
.LogPolicyEventAsync(default, default, default);
}
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
[Theory, BitAutoData]
public async Task SaveAsync_SingleOrg_VaultTimeoutEnabled_ThrowsBadRequest([PolicyFixtures.Policy(Enums.PolicyType.SingleOrg)] Policy policy, SutProvider<PolicyService> sutProvider)
{
policy.Enabled = false;
@ -127,8 +131,8 @@ namespace Bit.Core.Test.Services
}
[Theory]
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, Enums.PolicyType.SingleOrg)]
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, Enums.PolicyType.RequireSso)]
[BitAutoData(PolicyType.SingleOrg)]
[BitAutoData(PolicyType.RequireSso)]
public async Task SaveAsync_PolicyRequiredByKeyConnector_DisablePolicy_ThrowsBadRequest(
Enums.PolicyType policyType,
Policy policy,
@ -164,8 +168,9 @@ namespace Bit.Core.Test.Services
.UpsertAsync(default);
}
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
public async Task SaveAsync_RequireSsoPolicy_NotEnabled_ThrowsBadRequestAsync([PolicyFixtures.Policy(Enums.PolicyType.RequireSso)] Policy policy, SutProvider<PolicyService> sutProvider)
[Theory, BitAutoData]
public async Task SaveAsync_RequireSsoPolicy_NotEnabled_ThrowsBadRequestAsync(
[PolicyFixtures.Policy(Enums.PolicyType.RequireSso)] Policy policy, SutProvider<PolicyService> sutProvider)
{
policy.Enabled = true;
@ -196,8 +201,9 @@ namespace Bit.Core.Test.Services
.LogPolicyEventAsync(default, default, default);
}
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
public async Task SaveAsync_NewPolicy_Created([PolicyFixtures.Policy(PolicyType.MasterPassword)] Policy policy, SutProvider<PolicyService> sutProvider)
[Theory, BitAutoData]
public async Task SaveAsync_NewPolicy_Created(
[PolicyFixtures.Policy(PolicyType.MasterPassword)] Policy policy, SutProvider<PolicyService> sutProvider)
{
policy.Id = default;
@ -221,8 +227,9 @@ namespace Bit.Core.Test.Services
Assert.True(policy.RevisionDate - utcNow < TimeSpan.FromSeconds(1));
}
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
public async Task SaveAsync_VaultTimeoutPolicy_NotEnabled_ThrowsBadRequestAsync([PolicyFixtures.Policy(Enums.PolicyType.MaximumVaultTimeout)] Policy policy, SutProvider<PolicyService> sutProvider)
[Theory, BitAutoData]
public async Task SaveAsync_VaultTimeoutPolicy_NotEnabled_ThrowsBadRequestAsync(
[PolicyFixtures.Policy(PolicyType.MaximumVaultTimeout)] Policy policy, SutProvider<PolicyService> sutProvider)
{
policy.Enabled = true;
@ -253,8 +260,9 @@ namespace Bit.Core.Test.Services
.LogPolicyEventAsync(default, default, default);
}
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
public async Task SaveAsync_ExistingPolicy_UpdateTwoFactor([PolicyFixtures.Policy(PolicyType.TwoFactorAuthentication)] Policy policy, SutProvider<PolicyService> sutProvider)
[Theory, BitAutoData]
public async Task SaveAsync_ExistingPolicy_UpdateTwoFactor(
[PolicyFixtures.Policy(PolicyType.TwoFactorAuthentication)] Policy policy, SutProvider<PolicyService> sutProvider)
{
// If the policy that this is updating isn't enabled then do some work now that the current one is enabled
@ -322,8 +330,9 @@ namespace Bit.Core.Test.Services
Assert.True(policy.RevisionDate - utcNow < TimeSpan.FromSeconds(1));
}
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
public async Task SaveAsync_ExistingPolicy_UpdateSingleOrg([PolicyFixtures.Policy(PolicyType.TwoFactorAuthentication)] Policy policy, SutProvider<PolicyService> sutProvider)
[Theory, BitAutoData]
public async Task SaveAsync_ExistingPolicy_UpdateSingleOrg(
[PolicyFixtures.Policy(PolicyType.TwoFactorAuthentication)] Policy policy, SutProvider<PolicyService> sutProvider)
{
// If the policy that this is updating isn't enabled then do some work now that the current one is enabled

View File

@ -7,6 +7,7 @@ using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.Policies;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Core.Test.AutoFixture.SendFixtures;