From fe570cb6c8b380226709810ee099b68ca4638a33 Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Mon, 17 Jul 2023 11:00:32 -0700 Subject: [PATCH 1/3] [AC-1487] Update queries to use [User] table instead of [OrganizationUser] for email address (#3083) --- .../AuthRequest_ReadAdminApprovalsByIds.sql | 4 +- ...uthRequest_ReadPendingByOrganizationId.sql | 4 +- ...2023-07-10_00_FixTdeAdminApprovalEmail.sql | 46 +++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 util/Migrator/DbScripts/2023-07-10_00_FixTdeAdminApprovalEmail.sql diff --git a/src/Sql/Auth/dbo/Stored Procedures/AuthRequest_ReadAdminApprovalsByIds.sql b/src/Sql/Auth/dbo/Stored Procedures/AuthRequest_ReadAdminApprovalsByIds.sql index 6aff364318..74515f9d38 100644 --- a/src/Sql/Auth/dbo/Stored Procedures/AuthRequest_ReadAdminApprovalsByIds.sql +++ b/src/Sql/Auth/dbo/Stored Procedures/AuthRequest_ReadAdminApprovalsByIds.sql @@ -6,11 +6,13 @@ BEGIN SET NOCOUNT ON SELECT - ar.*, ou.[Email], ou.[Id] AS [OrganizationUserId] + ar.*, u.[Email], ou.[Id] AS [OrganizationUserId] FROM [dbo].[AuthRequestView] ar INNER JOIN [dbo].[OrganizationUser] ou ON ou.[UserId] = ar.[UserId] AND ou.[OrganizationId] = ar.[OrganizationId] + INNER JOIN + [dbo].[User] u ON u.[Id] = ar.[UserId] WHERE ar.[OrganizationId] = @OrganizationId AND diff --git a/src/Sql/Auth/dbo/Stored Procedures/AuthRequest_ReadPendingByOrganizationId.sql b/src/Sql/Auth/dbo/Stored Procedures/AuthRequest_ReadPendingByOrganizationId.sql index d3a2f8c581..56c1d1f15c 100644 --- a/src/Sql/Auth/dbo/Stored Procedures/AuthRequest_ReadPendingByOrganizationId.sql +++ b/src/Sql/Auth/dbo/Stored Procedures/AuthRequest_ReadPendingByOrganizationId.sql @@ -5,11 +5,13 @@ BEGIN SET NOCOUNT ON SELECT - ar.*, ou.[Email], ou.[OrganizationId], ou.[Id] AS [OrganizationUserId] + ar.*, u.[Email], ou.[Id] AS [OrganizationUserId] FROM [dbo].[AuthRequestView] ar INNER JOIN [dbo].[OrganizationUser] ou ON ou.[UserId] = ar.[UserId] AND ou.[OrganizationId] = ar.[OrganizationId] + INNER JOIN + [dbo].[User] u ON u.[Id] = ar.[UserId] WHERE ar.[OrganizationId] = @OrganizationId AND diff --git a/util/Migrator/DbScripts/2023-07-10_00_FixTdeAdminApprovalEmail.sql b/util/Migrator/DbScripts/2023-07-10_00_FixTdeAdminApprovalEmail.sql new file mode 100644 index 0000000000..e2aec0ecaa --- /dev/null +++ b/util/Migrator/DbScripts/2023-07-10_00_FixTdeAdminApprovalEmail.sql @@ -0,0 +1,46 @@ +CREATE OR ALTER PROCEDURE [dbo].[AuthRequest_ReadAdminApprovalsByIds] + @OrganizationId UNIQUEIDENTIFIER, + @Ids AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + +SELECT + ar.*, u.[Email], ou.[Id] AS [OrganizationUserId] +FROM + [dbo].[AuthRequestView] ar + INNER JOIN + [dbo].[OrganizationUser] ou ON ou.[UserId] = ar.[UserId] AND ou.[OrganizationId] = ar.[OrganizationId] + INNER JOIN + [dbo].[User] u ON u.[Id] = ar.[UserId] + WHERE + ar.[OrganizationId] = @OrganizationId + AND + ar.[Type] = 2 -- AdminApproval + AND + ar.[Id] IN (SELECT [Id] FROM @Ids) +END +GO + +CREATE OR ALTER PROCEDURE [dbo].[AuthRequest_ReadPendingByOrganizationId] + @OrganizationId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + +SELECT + ar.*, u.[Email], ou.[Id] AS [OrganizationUserId] +FROM + [dbo].[AuthRequestView] ar + INNER JOIN + [dbo].[OrganizationUser] ou ON ou.[UserId] = ar.[UserId] AND ou.[OrganizationId] = ar.[OrganizationId] + INNER JOIN + [dbo].[User] u ON u.[Id] = ar.[UserId] + WHERE + ar.[OrganizationId] = @OrganizationId + AND + ar.[ResponseDate] IS NULL + AND + ar.[Type] = 2 -- AdminApproval +END +GO \ No newline at end of file From a095e02e86a36abb0713db9d4c032eef1ea2127a Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Tue, 18 Jul 2023 08:00:49 -0700 Subject: [PATCH 2/3] [AC-1435] Single Organization policy prerequisite for Account Recovery policy (#3082) * [AC-1435] Automatically enable Single Org policy when selecting TDE * [AC-1435] Add test for automatic policy enablement * [AC-1435] Prevent disabling single org when account recovery is enabled * [AC-1435] Require Single Org policy when enabling Account recovery * [AC-1435] Add unit test to check for account recovery policy when attempting to disable single org * [AC-1435] Add test to verify single org policy is enabled for account recovery policy * [AC-1435] Fix failing test --- .../Implementations/SsoConfigService.cs | 9 ++- .../Services/Implementations/PolicyService.cs | 15 ++++ .../Auth/Services/SsoConfigServiceTests.cs | 38 +++++++++++ test/Core.Test/Services/PolicyServiceTests.cs | 68 +++++++++++++++++++ 4 files changed, 129 insertions(+), 1 deletion(-) diff --git a/src/Core/Auth/Services/Implementations/SsoConfigService.cs b/src/Core/Auth/Services/Implementations/SsoConfigService.cs index fb90d5d8d4..a2cb906f6f 100644 --- a/src/Core/Auth/Services/Implementations/SsoConfigService.cs +++ b/src/Core/Auth/Services/Implementations/SsoConfigService.cs @@ -63,9 +63,16 @@ public class SsoConfigService : ISsoConfigService throw new BadRequestException("Key Connector cannot be disabled at this moment."); } - // Automatically enable reset password policy if trusted device encryption is selected + // Automatically enable account recovery and single org policies if trusted device encryption is selected if (config.GetData().MemberDecryptionType == MemberDecryptionType.TrustedDeviceEncryption) { + var singleOrgPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.SingleOrg) ?? + new Policy { OrganizationId = config.OrganizationId, Type = PolicyType.SingleOrg }; + + singleOrgPolicy.Enabled = true; + + await _policyService.SaveAsync(singleOrgPolicy, _userService, _organizationService, null); + var resetPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.ResetPassword) ?? new Policy { OrganizationId = config.OrganizationId, Type = PolicyType.ResetPassword, }; diff --git a/src/Core/Services/Implementations/PolicyService.cs b/src/Core/Services/Implementations/PolicyService.cs index a8411e4198..6b1009093c 100644 --- a/src/Core/Services/Implementations/PolicyService.cs +++ b/src/Core/Services/Implementations/PolicyService.cs @@ -61,6 +61,7 @@ public class PolicyService : IPolicyService await RequiredBySsoAsync(org); await RequiredByVaultTimeoutAsync(org); await RequiredByKeyConnectorAsync(org); + await RequiredByAccountRecoveryAsync(org); } break; @@ -80,6 +81,11 @@ public class PolicyService : IPolicyService { await RequiredBySsoTrustedDeviceEncryptionAsync(org); } + + if (policy.Enabled) + { + await DependsOnSingleOrgAsync(org); + } break; case PolicyType.MaximumVaultTimeout: @@ -244,6 +250,15 @@ public class PolicyService : IPolicyService } } + private async Task RequiredByAccountRecoveryAsync(Organization org) + { + var requireSso = await _policyRepository.GetByOrganizationIdTypeAsync(org.Id, PolicyType.ResetPassword); + if (requireSso?.Enabled == true) + { + throw new BadRequestException("Account recovery policy is enabled."); + } + } + private async Task RequiredByVaultTimeoutAsync(Organization org) { var vaultTimeout = await _policyRepository.GetByOrganizationIdTypeAsync(org.Id, PolicyType.MaximumVaultTimeout); diff --git a/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs b/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs index fdc8217ba8..7886d6f9e8 100644 --- a/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs +++ b/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs @@ -6,7 +6,9 @@ using Bit.Core.Auth.Services; using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Models.Data.Organizations.OrganizationUsers; +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; @@ -317,4 +319,40 @@ public class SsoConfigServiceTests await sutProvider.GetDependency().ReceivedWithAnyArgs() .UpsertAsync(default); } + + [Theory, BitAutoData] + public async Task SaveAsync_Tde_Enable_Required_Policies(SutProvider sutProvider, Organization organization) + { + var ssoConfig = new SsoConfig + { + Id = default, + Data = new SsoConfigurationData + { + MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption, + }.Serialize(), + Enabled = true, + OrganizationId = organization.Id, + }; + + await sutProvider.Sut.SaveAsync(ssoConfig, organization); + + await sutProvider.GetDependency().Received(1) + .SaveAsync( + Arg.Is(t => t.Type == Enums.PolicyType.SingleOrg), + Arg.Any(), + Arg.Any(), + null + ); + + await sutProvider.GetDependency().Received(1) + .SaveAsync( + Arg.Is(t => t.Type == Enums.PolicyType.ResetPassword && t.GetDataModel().AutoEnrollEnabled), + Arg.Any(), + Arg.Any(), + null + ); + + await sutProvider.GetDependency().ReceivedWithAnyArgs() + .UpsertAsync(default); + } } diff --git a/test/Core.Test/Services/PolicyServiceTests.cs b/test/Core.Test/Services/PolicyServiceTests.cs index 3b4d9b1b80..6c1218c84a 100644 --- a/test/Core.Test/Services/PolicyServiceTests.cs +++ b/test/Core.Test/Services/PolicyServiceTests.cs @@ -217,6 +217,10 @@ public class PolicyServiceTests UsePolicies = true, }); + sutProvider.GetDependency() + .GetByOrganizationIdTypeAsync(policy.OrganizationId, Enums.PolicyType.SingleOrg) + .Returns(Task.FromResult(new Policy { Enabled = true })); + var utcNow = DateTime.UtcNow; await sutProvider.Sut.SaveAsync(policy, Substitute.For(), Substitute.For(), Guid.NewGuid()); @@ -444,6 +448,70 @@ public class PolicyServiceTests .LogPolicyEventAsync(default, default, default); } + [Theory, BitAutoData] + public async Task SaveAsync_PolicyRequiredForAccountRecovery_NotEnabled_ThrowsBadRequestAsync( + [PolicyFixtures.Policy(Enums.PolicyType.ResetPassword)] Policy policy, SutProvider sutProvider) + { + policy.Enabled = true; + policy.SetDataModel(new ResetPasswordDataModel()); + + SetupOrg(sutProvider, policy.OrganizationId, new Organization + { + Id = policy.OrganizationId, + UsePolicies = true, + }); + + sutProvider.GetDependency() + .GetByOrganizationIdTypeAsync(policy.OrganizationId, PolicyType.SingleOrg) + .Returns(Task.FromResult(new Policy { Enabled = false })); + + var badRequestException = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveAsync(policy, + Substitute.For(), + Substitute.For(), + Guid.NewGuid())); + + Assert.Contains("Single Organization policy not enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpsertAsync(default); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogPolicyEventAsync(default, default, default); + } + + + [Theory, BitAutoData] + public async Task SaveAsync_SingleOrg_AccountRecoveryEnabled_ThrowsBadRequest( + [PolicyFixtures.Policy(Enums.PolicyType.SingleOrg)] Policy policy, SutProvider sutProvider) + { + policy.Enabled = false; + + SetupOrg(sutProvider, policy.OrganizationId, new Organization + { + Id = policy.OrganizationId, + UsePolicies = true, + }); + + sutProvider.GetDependency() + .GetByOrganizationIdTypeAsync(policy.OrganizationId, Enums.PolicyType.ResetPassword) + .Returns(new Policy { Enabled = true }); + + var badRequestException = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveAsync(policy, + Substitute.For(), + Substitute.For(), + Guid.NewGuid())); + + Assert.Contains("Account recovery policy is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpsertAsync(default); + } + [Theory, BitAutoData] public async Task GetPoliciesApplicableToUserAsync_WithRequireSsoTypeFilter_WithDefaultOrganizationUserStatusFilter_ReturnsNoPolicies(Guid userId, SutProvider sutProvider) { From 1fe2f0fb5738d69e4f214e4f6b6cec49df1dd836 Mon Sep 17 00:00:00 2001 From: Colton Hurst Date: Tue, 18 Jul 2023 15:32:47 -0400 Subject: [PATCH 3/3] SM-503: Add EmptySecretsManagerTrashJob (#2863) * SM-503: Add EmptySecretsManagerJob * SM-503: Fix date logic and refactor a few lines * SM-503: Add logging * SM-503: Move EmptySecretsManagerTrashJob to src/Api/SecretsManager/Jobs * SM-503: Update trigger time for EmptySecretsManagerTrashJob * SM-503: Switch to scope on one line * SM-768: Update EFCore and related packages to >= 7.0 * SM-768: Update more packages for the EF 7 upgrade * SM-768: Update the PostgreSQL package * SM-768: Run dotnet restore --force-evaluate * SM-768: Revert package upgrades for 3 projects * SM-768: Update the dotnet-ef tool * SM-503: Switch to using ExecuteDeleteAsync and fix param name * SM-503: Rename trigger to smTrashCleanupTrigger * SM-503: Fix OSS job issue * SM-503: Only add trigger if not OSS for SM Trash Job --- .../Repositories/SecretRepository.cs | 10 ++++++++ src/Api/Jobs/JobsHostedService.cs | 14 +++++++++++ .../Jobs/EmptySecretsManagerTrashJob.cs | 23 +++++++++++++++++++ src/Api/Startup.cs | 1 + .../Repositories/ISecretRepository.cs | 1 + 5 files changed, 49 insertions(+) create mode 100644 src/Api/SecretsManager/Jobs/EmptySecretsManagerTrashJob.cs diff --git a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs index 6720c7116d..078147f3ab 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs @@ -293,6 +293,16 @@ public class SecretRepository : Repository s.DeletedDate != null && s.DeletedDate < currentDate.AddDays(-deleteAfterThisNumberOfDays)).ExecuteDeleteAsync(); + + await dbContext.SaveChangesAsync(); + } + private IQueryable SecretToPermissionDetails(IQueryable query, Guid userId, AccessClientType accessType) { var secrets = accessType switch diff --git a/src/Api/Jobs/JobsHostedService.cs b/src/Api/Jobs/JobsHostedService.cs index 29cd4842da..acd95a0213 100644 --- a/src/Api/Jobs/JobsHostedService.cs +++ b/src/Api/Jobs/JobsHostedService.cs @@ -41,6 +41,11 @@ public class JobsHostedService : BaseJobsHostedService .StartNow() .WithCronSchedule("0 30 */12 * * ?") .Build(); + var smTrashCleanupTrigger = TriggerBuilder.Create() + .WithIdentity("SMTrashCleanupTrigger") + .StartNow() + .WithCronSchedule("0 0 22 * * ?") + .Build(); var randomDailySponsorshipSyncTrigger = TriggerBuilder.Create() .WithIdentity("RandomDailySponsorshipSyncTrigger") .StartAt(DateBuilder.FutureDate(new Random().Next(24), IntervalUnit.Hour)) @@ -70,6 +75,10 @@ public class JobsHostedService : BaseJobsHostedService jobs.Add(new Tuple(typeof(SelfHostedSponsorshipSyncJob), randomDailySponsorshipSyncTrigger)); } +#if !OSS + jobs.Add(new Tuple(typeof(EmptySecretsManagerTrashJob), smTrashCleanupTrigger)); +#endif + Jobs = jobs; await base.StartAsync(cancellationToken); @@ -88,4 +97,9 @@ public class JobsHostedService : BaseJobsHostedService services.AddTransient(); services.AddTransient(); } + + public static void AddCommercialSecretsManagerJobServices(IServiceCollection services) + { + services.AddTransient(); + } } diff --git a/src/Api/SecretsManager/Jobs/EmptySecretsManagerTrashJob.cs b/src/Api/SecretsManager/Jobs/EmptySecretsManagerTrashJob.cs new file mode 100644 index 0000000000..f8668e8e53 --- /dev/null +++ b/src/Api/SecretsManager/Jobs/EmptySecretsManagerTrashJob.cs @@ -0,0 +1,23 @@ +using Bit.Core.Jobs; +using Bit.Core.SecretsManager.Repositories; +using Quartz; + +namespace Bit.Api.Jobs; + +public class EmptySecretsManagerTrashJob : BaseJob +{ + private ISecretRepository _secretRepository; + private const uint DeleteAfterThisNumberOfDays = 30; + + public EmptySecretsManagerTrashJob(ISecretRepository secretRepository, ILogger logger) : base(logger) + { + _secretRepository = secretRepository; + } + + protected override async Task ExecuteJobAsync(IJobExecutionContext context) + { + _logger.LogInformation("Execute job task: EmptySecretsManagerTrashJob: Start"); + await _secretRepository.EmptyTrash(DateTime.UtcNow, DeleteAfterThisNumberOfDays); + _logger.LogInformation("Execute job task: EmptySecretsManagerTrashJob: End"); + } +} diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs index 6070a91b29..de47794932 100644 --- a/src/Api/Startup.cs +++ b/src/Api/Startup.cs @@ -147,6 +147,7 @@ public class Startup services.AddCommercialCoreServices(); services.AddCommercialSecretsManagerServices(); services.AddSecretsManagerEfRepositories(); + Jobs.JobsHostedService.AddCommercialSecretsManagerJobServices(services); #endif // MVC diff --git a/src/Core/SecretsManager/Repositories/ISecretRepository.cs b/src/Core/SecretsManager/Repositories/ISecretRepository.cs index 5b40a33278..68422cb44c 100644 --- a/src/Core/SecretsManager/Repositories/ISecretRepository.cs +++ b/src/Core/SecretsManager/Repositories/ISecretRepository.cs @@ -20,4 +20,5 @@ public interface ISecretRepository Task> ImportAsync(IEnumerable secrets); Task UpdateRevisionDates(IEnumerable ids); Task<(bool Read, bool Write)> AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType); + Task EmptyTrash(DateTime nowTime, uint deleteAfterThisNumberOfDays); }