From 72b78ed65506b7433e1a224c29fb66ebaf36f55f Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Tue, 4 Feb 2025 14:58:54 -0500 Subject: [PATCH 01/25] Update feature flag name (#5364) --- src/Core/Constants.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 6f9960919a..f58f1f7157 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -174,7 +174,7 @@ public static class FeatureFlagKeys public const string SingleTapPasskeyCreation = "single-tap-passkey-creation"; public const string SingleTapPasskeyAuthentication = "single-tap-passkey-authentication"; public const string EnablePMAuthenticatorSync = "enable-pm-bwa-sync"; - public const string P15179_AddExistingOrgsFromProviderPortal = "PM-15179-add-existing-orgs-from-provider-portal"; + public const string P15179_AddExistingOrgsFromProviderPortal = "pm-15179-add-existing-orgs-from-provider-portal"; public static List GetAllKeys() { From 90680f482a3b20b8d580056c6d136baab4cbf089 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Tue, 4 Feb 2025 15:40:17 -0500 Subject: [PATCH 02/25] Revert version from 2025.1.5 to 2025.1.4 (#5369) --- Directory.Build.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index d109303a58..88b63d156c 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -3,7 +3,7 @@ net8.0 - 2025.1.5 + 2025.1.4 Bit.$(MSBuildProjectName) enable @@ -64,4 +64,4 @@ - \ No newline at end of file + From 412c6f9849431a3d170a8009c86f2d7fa4ae3a57 Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Tue, 4 Feb 2025 15:45:24 -0500 Subject: [PATCH 03/25] [PM-11162] Assign to Collection Permission Update (#4844) Only users with Manage/Edit permissions will be allowed to Assign To Collections. If the user has Can Edit Except Password the collections dropdown will be disabled. --------- Co-authored-by: Matt Bishop Co-authored-by: kejaeger <138028972+kejaeger@users.noreply.github.com> --- .../Vault/Controllers/CiphersController.cs | 57 ++++++++- .../Vault/Repositories/CipherRepository.cs | 2 +- .../Cipher/CipherDetails_ReadByIdUserId.sql | 38 +++++- .../CollectionCipher_UpdateCollections.sql | 56 ++++----- .../Controllers/CiphersControllerTests.cs | 1 + ...0_CollectionPermissionEditExceptPWPerm.sql | 118 ++++++++++++++++++ 6 files changed, 233 insertions(+), 39 deletions(-) create mode 100644 util/Migrator/DbScripts/2025-02-04_00_CollectionPermissionEditExceptPWPerm.sql diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index c8ebb8c402..5a7d427963 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -424,6 +424,59 @@ public class CiphersController : Controller return false; } + /// + /// TODO: Move this to its own authorization handler or equivalent service - AC-2062 + /// + private async Task CanModifyCipherCollectionsAsync(Guid organizationId, IEnumerable cipherIds) + { + // If the user can edit all ciphers for the organization, just check they all belong to the org + if (await CanEditAllCiphersAsync(organizationId)) + { + // TODO: This can likely be optimized to only query the requested ciphers and then checking they belong to the org + var orgCiphers = (await _cipherRepository.GetManyByOrganizationIdAsync(organizationId)).ToDictionary(c => c.Id); + + // Ensure all requested ciphers are in orgCiphers + if (cipherIds.Any(c => !orgCiphers.ContainsKey(c))) + { + return false; + } + + return true; + } + + // The user cannot access any ciphers for the organization, we're done + if (!await CanAccessOrganizationCiphersAsync(organizationId)) + { + return false; + } + + var userId = _userService.GetProperUserId(User).Value; + // Select all editable ciphers for this user belonging to the organization + var editableOrgCipherList = (await _cipherRepository.GetManyByUserIdAsync(userId, true)) + .Where(c => c.OrganizationId == organizationId && c.UserId == null && c.Edit && c.ViewPassword).ToList(); + + // Special case for unassigned ciphers + if (await CanAccessUnassignedCiphersAsync(organizationId)) + { + var unassignedCiphers = + (await _cipherRepository.GetManyUnassignedOrganizationDetailsByOrganizationIdAsync( + organizationId)); + + // Users that can access unassigned ciphers can also edit them + editableOrgCipherList.AddRange(unassignedCiphers.Select(c => new CipherDetails(c) { Edit = true })); + } + + var editableOrgCiphers = editableOrgCipherList + .ToDictionary(c => c.Id); + + if (cipherIds.Any(c => !editableOrgCiphers.ContainsKey(c))) + { + return false; + } + + return true; + } + /// /// TODO: Move this to its own authorization handler or equivalent service - AC-2062 /// @@ -579,7 +632,7 @@ public class CiphersController : Controller var userId = _userService.GetProperUserId(User).Value; var cipher = await GetByIdAsync(id, userId); if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.OrganizationUser(cipher.OrganizationId.Value)) + !await _currentContext.OrganizationUser(cipher.OrganizationId.Value) || !cipher.ViewPassword) { throw new NotFoundException(); } @@ -634,7 +687,7 @@ public class CiphersController : Controller [HttpPost("bulk-collections")] public async Task PostBulkCollections([FromBody] CipherBulkUpdateCollectionsRequestModel model) { - if (!await CanEditCiphersAsync(model.OrganizationId, model.CipherIds) || + if (!await CanModifyCipherCollectionsAsync(model.OrganizationId, model.CipherIds) || !await CanEditItemsInCollections(model.OrganizationId, model.CollectionIds)) { throw new NotFoundException(); diff --git a/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs b/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs index 098e8299e4..b8304fbbb0 100644 --- a/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs +++ b/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs @@ -98,7 +98,7 @@ public class CipherRepository : Repository, ICipherRepository return results .GroupBy(c => c.Id) - .Select(g => g.OrderByDescending(og => og.Edit).First()) + .Select(g => g.OrderByDescending(og => og.Edit).ThenByDescending(og => og.ViewPassword).First()) .ToList(); } } diff --git a/src/Sql/Vault/dbo/Stored Procedures/Cipher/CipherDetails_ReadByIdUserId.sql b/src/Sql/Vault/dbo/Stored Procedures/Cipher/CipherDetails_ReadByIdUserId.sql index e2fb2629bd..189ad0a4a5 100644 --- a/src/Sql/Vault/dbo/Stored Procedures/Cipher/CipherDetails_ReadByIdUserId.sql +++ b/src/Sql/Vault/dbo/Stored Procedures/Cipher/CipherDetails_ReadByIdUserId.sql @@ -5,12 +5,40 @@ AS BEGIN SET NOCOUNT ON - SELECT TOP 1 - * +SELECT + [Id], + [UserId], + [OrganizationId], + [Type], + [Data], + [Attachments], + [CreationDate], + [RevisionDate], + [Favorite], + [FolderId], + [DeletedDate], + [Reprompt], + [Key], + [OrganizationUseTotp], + MAX ([Edit]) AS [Edit], + MAX ([ViewPassword]) AS [ViewPassword] FROM [dbo].[UserCipherDetails](@UserId) WHERE [Id] = @Id - ORDER BY - [Edit] DESC -END \ No newline at end of file + GROUP BY + [Id], + [UserId], + [OrganizationId], + [Type], + [Data], + [Attachments], + [CreationDate], + [RevisionDate], + [Favorite], + [FolderId], + [DeletedDate], + [Reprompt], + [Key], + [OrganizationUseTotp] +END diff --git a/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections.sql b/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections.sql index 4098ab59e2..f3a1d964b5 100644 --- a/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections.sql +++ b/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections.sql @@ -14,10 +14,9 @@ BEGIN WHERE [Id] = @CipherId ) - - ;WITH [AvailableCollectionsCTE] AS( - SELECT + SELECT C.[Id] + INTO #TempAvailableCollections FROM [dbo].[Collection] C INNER JOIN @@ -40,38 +39,33 @@ BEGIN CU.[ReadOnly] = 0 OR CG.[ReadOnly] = 0 ) - ), - [CollectionCiphersCTE] AS( - SELECT - [CollectionId], - [CipherId] - FROM - [dbo].[CollectionCipher] - WHERE - [CipherId] = @CipherId + -- Insert new collection assignments + INSERT INTO [dbo].[CollectionCipher] ( + [CollectionId], + [CipherId] ) - MERGE - [CollectionCiphersCTE] AS [Target] - USING - @CollectionIds AS [Source] - ON - [Target].[CollectionId] = [Source].[Id] - AND [Target].[CipherId] = @CipherId - WHEN NOT MATCHED BY TARGET - AND [Source].[Id] IN (SELECT [Id] FROM [AvailableCollectionsCTE]) THEN - INSERT VALUES - ( - [Source].[Id], - @CipherId - ) - WHEN NOT MATCHED BY SOURCE - AND [Target].[CipherId] = @CipherId - AND [Target].[CollectionId] IN (SELECT [Id] FROM [AvailableCollectionsCTE]) THEN - DELETE - ; + SELECT + [Id], + @CipherId + FROM @CollectionIds + WHERE [Id] IN (SELECT [Id] FROM [#TempAvailableCollections]) + AND NOT EXISTS ( + SELECT 1 + FROM [dbo].[CollectionCipher] + WHERE [CollectionId] = [@CollectionIds].[Id] + AND [CipherId] = @CipherId + ); + + -- Delete removed collection assignments + DELETE CC + FROM [dbo].[CollectionCipher] CC + WHERE CC.[CipherId] = @CipherId + AND CC.[CollectionId] IN (SELECT [Id] FROM [#TempAvailableCollections]) + AND CC.[CollectionId] NOT IN (SELECT [Id] FROM @CollectionIds); IF @OrgId IS NOT NULL BEGIN EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId END + DROP TABLE #TempAvailableCollections; END diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index e7c5cd9ef5..2afce14ac5 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -127,6 +127,7 @@ public class CiphersControllerTests UserId = userId, OrganizationId = Guid.NewGuid(), Type = CipherType.Login, + ViewPassword = true, Data = @" { ""Uris"": [ diff --git a/util/Migrator/DbScripts/2025-02-04_00_CollectionPermissionEditExceptPWPerm.sql b/util/Migrator/DbScripts/2025-02-04_00_CollectionPermissionEditExceptPWPerm.sql new file mode 100644 index 0000000000..95013afaa4 --- /dev/null +++ b/util/Migrator/DbScripts/2025-02-04_00_CollectionPermissionEditExceptPWPerm.sql @@ -0,0 +1,118 @@ +CREATE OR ALTER PROCEDURE [dbo].[CipherDetails_ReadByIdUserId] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + +SELECT + [Id], + [UserId], + [OrganizationId], + [Type], + [Data], + [Attachments], + [CreationDate], + [RevisionDate], + [Favorite], + [FolderId], + [DeletedDate], + [Reprompt], + [Key], + [OrganizationUseTotp], + MAX ([Edit]) AS [Edit], + MAX ([ViewPassword]) AS [ViewPassword] +FROM + [dbo].[UserCipherDetails](@UserId) +WHERE + [Id] = @Id +GROUP BY + [Id], + [UserId], + [OrganizationId], + [Type], + [Data], + [Attachments], + [CreationDate], + [RevisionDate], + [Favorite], + [FolderId], + [DeletedDate], + [Reprompt], + [Key], + [OrganizationUseTotp] +END +GO + +CREATE OR ALTER PROCEDURE [dbo].[CollectionCipher_UpdateCollections] + @CipherId UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER, + @CollectionIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DECLARE @OrgId UNIQUEIDENTIFIER = ( + SELECT TOP 1 + [OrganizationId] + FROM + [dbo].[Cipher] + WHERE + [Id] = @CipherId + ) + SELECT + C.[Id] + INTO #TempAvailableCollections + FROM + [dbo].[Collection] C + INNER JOIN + [Organization] O ON O.[Id] = C.[OrganizationId] + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[OrganizationId] = O.[Id] AND OU.[UserId] = @UserId + LEFT JOIN + [dbo].[CollectionUser] CU ON CU.[CollectionId] = C.[Id] AND CU.[OrganizationUserId] = OU.[Id] + LEFT JOIN + [dbo].[GroupUser] GU ON CU.[CollectionId] IS NULL AND GU.[OrganizationUserId] = OU.[Id] + LEFT JOIN + [dbo].[Group] G ON G.[Id] = GU.[GroupId] + LEFT JOIN + [dbo].[CollectionGroup] CG ON CG.[CollectionId] = C.[Id] AND CG.[GroupId] = GU.[GroupId] + WHERE + O.[Id] = @OrgId + AND O.[Enabled] = 1 + AND OU.[Status] = 2 -- Confirmed + AND ( + CU.[ReadOnly] = 0 + OR CG.[ReadOnly] = 0 + ) + -- Insert new collection assignments + INSERT INTO [dbo].[CollectionCipher] ( + [CollectionId], + [CipherId] + ) + SELECT + [Id], + @CipherId + FROM @CollectionIds + WHERE [Id] IN (SELECT [Id] FROM [#TempAvailableCollections]) + AND NOT EXISTS ( + SELECT 1 + FROM [dbo].[CollectionCipher] + WHERE [CollectionId] = [@CollectionIds].[Id] + AND [CipherId] = @CipherId + ); + + -- Delete removed collection assignments + DELETE CC + FROM [dbo].[CollectionCipher] CC + WHERE CC.[CipherId] = @CipherId + AND CC.[CollectionId] IN (SELECT [Id] FROM [#TempAvailableCollections]) + AND CC.[CollectionId] NOT IN (SELECT [Id] FROM @CollectionIds); + + IF @OrgId IS NOT NULL + BEGIN + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId + END + DROP TABLE #TempAvailableCollections; +END +GO From a8a08a0c8f21b9893b4a5831f61cfce1ad131d3c Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Wed, 5 Feb 2025 09:18:23 +0100 Subject: [PATCH 04/25] Remove the feature flag (#5331) --- .../Billing/Controllers/OrganizationSponsorshipsController.cs | 3 +-- src/Core/Constants.cs | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs b/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs index a7a4c39054..42263aa88b 100644 --- a/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs +++ b/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs @@ -1,6 +1,5 @@ using Bit.Api.Models.Request.Organizations; using Bit.Api.Models.Response.Organizations; -using Bit.Core; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationConnections.Interfaces; using Bit.Core.AdminConsole.Repositories; @@ -107,7 +106,7 @@ public class OrganizationSponsorshipsController : Controller { var isFreeFamilyPolicyEnabled = false; var (isValid, sponsorship) = await _validateRedemptionTokenCommand.ValidateRedemptionTokenAsync(sponsorshipToken, (await CurrentUser).Email); - if (isValid && _featureService.IsEnabled(FeatureFlagKeys.DisableFreeFamiliesSponsorship) && sponsorship.SponsoringOrganizationId.HasValue) + if (isValid && sponsorship.SponsoringOrganizationId.HasValue) { var policy = await _policyRepository.GetByOrganizationIdTypeAsync(sponsorship.SponsoringOrganizationId.Value, PolicyType.FreeFamiliesSponsorshipPolicy); diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index f58f1f7157..cba146c959 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -155,7 +155,6 @@ public static class FeatureFlagKeys public const string NewDeviceVerificationTemporaryDismiss = "new-device-temporary-dismiss"; public const string NewDeviceVerificationPermanentDismiss = "new-device-permanent-dismiss"; public const string SecurityTasks = "security-tasks"; - public const string DisableFreeFamiliesSponsorship = "PM-12274-disable-free-families-sponsorship"; public const string MacOsNativeCredentialSync = "macos-native-credential-sync"; public const string PM9111ExtensionPersistAddEditForm = "pm-9111-extension-persist-add-edit-form"; public const string InlineMenuTotp = "inline-menu-totp"; From 617bb5015fdde37a0e78d43a8086918409de41fb Mon Sep 17 00:00:00 2001 From: Tom <144813356+ttalty@users.noreply.github.com> Date: Wed, 5 Feb 2025 04:57:19 -0500 Subject: [PATCH 05/25] Removing the member access feature flag from the server (#5368) --- src/Core/Constants.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index cba146c959..03d6a193a7 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -112,7 +112,6 @@ public static class FeatureFlagKeys /* Tools Team */ public const string ItemShare = "item-share"; public const string GeneratorToolsModernization = "generator-tools-modernization"; - public const string MemberAccessReport = "ac-2059-member-access-report"; public const string RiskInsightsCriticalApplication = "pm-14466-risk-insights-critical-application"; public const string EnableRiskInsightsNotifications = "enable-risk-insights-notifications"; From 03c390de7405b6e2a2c0463d28ad406de6b66210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 5 Feb 2025 14:47:06 +0000 Subject: [PATCH 06/25] =?UTF-8?q?[PM-15637]=20Notify=20Custom=20Users=20wi?= =?UTF-8?q?th=20=E2=80=9CManage=20Account=20Recovery=E2=80=9D=20permission?= =?UTF-8?q?=20for=20Device=20Approval=20Requests=20(#5359)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add stored procedure to read organization user details by role * Add OrganizationUserRepository method to retrieve OrganizationUser details by role * Enhance AuthRequestService to send notifications to custom users with ManageResetPassword permission * Enhance AuthRequestServiceTests to include custom user permissions and validate notification email recipients --- .../IOrganizationUserRepository.cs | 8 +++++ .../Implementations/AuthRequestService.cs | 30 +++++++++++++++-- .../OrganizationUserRepository.cs | 13 ++++++++ .../OrganizationUserRepository.cs | 21 ++++++++++++ ...OrganizationUser_ReadManyDetailsByRole.sql | 16 ++++++++++ .../Auth/Services/AuthRequestServiceTests.cs | 32 +++++++++++++++++-- ...-02-03_00_OrgUserReadManyDetailsByRole.sql | 16 ++++++++++ 7 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 src/Sql/dbo/Stored Procedures/OrganizationUser_ReadManyDetailsByRole.sql create mode 100644 util/Migrator/DbScripts/2025-02-03_00_OrgUserReadManyDetailsByRole.sql diff --git a/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs b/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs index 516b4614af..8825f9722a 100644 --- a/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs +++ b/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs @@ -60,4 +60,12 @@ public interface IOrganizationUserRepository : IRepository> GetManyByOrganizationWithClaimedDomainsAsync(Guid organizationId); Task RevokeManyByIdAsync(IEnumerable organizationUserIds); + + /// + /// Returns a list of OrganizationUsersUserDetails with the specified role. + /// + /// The organization to search within + /// The role to search for + /// A list of OrganizationUsersUserDetails with the specified role + Task> GetManyDetailsByRoleAsync(Guid organizationId, OrganizationUserType role); } diff --git a/src/Core/Auth/Services/Implementations/AuthRequestService.cs b/src/Core/Auth/Services/Implementations/AuthRequestService.cs index 5e41e3a679..b70a690338 100644 --- a/src/Core/Auth/Services/Implementations/AuthRequestService.cs +++ b/src/Core/Auth/Services/Implementations/AuthRequestService.cs @@ -297,10 +297,34 @@ public class AuthRequestService : IAuthRequestService return; } - var admins = await _organizationUserRepository.GetManyByMinimumRoleAsync( + var adminEmails = await GetAdminAndAccountRecoveryEmailsAsync(organizationUser.OrganizationId); + + await _mailService.SendDeviceApprovalRequestedNotificationEmailAsync( + adminEmails, organizationUser.OrganizationId, + user.Email, + user.Name); + } + + /// + /// Returns a list of emails for admins and custom users with the ManageResetPassword permission. + /// + /// The organization to search within + private async Task> GetAdminAndAccountRecoveryEmailsAsync(Guid organizationId) + { + var admins = await _organizationUserRepository.GetManyByMinimumRoleAsync( + organizationId, OrganizationUserType.Admin); - var adminEmails = admins.Select(a => a.Email).Distinct().ToList(); - await _mailService.SendDeviceApprovalRequestedNotificationEmailAsync(adminEmails, organizationUser.OrganizationId, user.Email, user.Name); + + var customUsers = await _organizationUserRepository.GetManyDetailsByRoleAsync( + organizationId, + OrganizationUserType.Custom); + + return admins.Select(a => a.Email) + .Concat(customUsers + .Where(a => a.GetPermissions().ManageResetPassword) + .Select(a => a.Email)) + .Distinct() + .ToList(); } } diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs index 42f79852f3..9b77fb216e 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -567,4 +567,17 @@ public class OrganizationUserRepository : Repository, IO new { OrganizationUserIds = JsonSerializer.Serialize(organizationUserIds), Status = OrganizationUserStatusType.Revoked }, commandType: CommandType.StoredProcedure); } + + public async Task> GetManyDetailsByRoleAsync(Guid organizationId, OrganizationUserType role) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + "[dbo].[OrganizationUser_ReadManyDetailsByRole]", + new { OrganizationId = organizationId, Role = role }, + commandType: CommandType.StoredProcedure); + + return results.ToList(); + } + } } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs index 007ff1a7ff..ef6460df0e 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -733,4 +733,25 @@ public class OrganizationUserRepository : Repository> GetManyDetailsByRoleAsync(Guid organizationId, OrganizationUserType role) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var query = from ou in dbContext.OrganizationUsers + join u in dbContext.Users + on ou.UserId equals u.Id + where ou.OrganizationId == organizationId && + ou.Type == role && + ou.Status == OrganizationUserStatusType.Confirmed + select new OrganizationUserUserDetails + { + Id = ou.Id, + Email = ou.Email ?? u.Email, + Permissions = ou.Permissions + }; + return await query.ToListAsync(); + } + } } diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadManyDetailsByRole.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadManyDetailsByRole.sql new file mode 100644 index 0000000000..e8bf8bb701 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadManyDetailsByRole.sql @@ -0,0 +1,16 @@ +CREATE PROCEDURE [dbo].[OrganizationUser_ReadManyDetailsByRole] + @OrganizationId UNIQUEIDENTIFIER, + @Role TINYINT +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[OrganizationUserUserDetailsView] + WHERE + OrganizationId = @OrganizationId + AND Status = 2 -- 2 = Confirmed + AND [Type] = @Role +END diff --git a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs index 3894ac90a8..8feef2facc 100644 --- a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs +++ b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs @@ -7,11 +7,13 @@ using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; +using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; @@ -347,14 +349,24 @@ public class AuthRequestServiceTests User user, OrganizationUser organizationUser1, OrganizationUserUserDetails admin1, + OrganizationUserUserDetails customUser1, OrganizationUser organizationUser2, OrganizationUserUserDetails admin2, - OrganizationUserUserDetails admin3) + OrganizationUserUserDetails admin3, + OrganizationUserUserDetails customUser2) { createModel.Type = AuthRequestType.AdminApproval; user.Email = createModel.Email; organizationUser1.UserId = user.Id; organizationUser2.UserId = user.Id; + customUser1.Permissions = CoreHelpers.ClassToJsonData(new Permissions + { + ManageResetPassword = false, + }); + customUser2.Permissions = CoreHelpers.ClassToJsonData(new Permissions + { + ManageResetPassword = true, + }); sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.DeviceApprovalRequestAdminNotifications) @@ -392,6 +404,13 @@ public class AuthRequestServiceTests admin1, ]); + sutProvider.GetDependency() + .GetManyDetailsByRoleAsync(organizationUser1.OrganizationId, OrganizationUserType.Custom) + .Returns( + [ + customUser1, + ]); + sutProvider.GetDependency() .GetManyByMinimumRoleAsync(organizationUser2.OrganizationId, OrganizationUserType.Admin) .Returns( @@ -400,6 +419,13 @@ public class AuthRequestServiceTests admin3, ]); + sutProvider.GetDependency() + .GetManyDetailsByRoleAsync(organizationUser2.OrganizationId, OrganizationUserType.Custom) + .Returns( + [ + customUser2, + ]); + sutProvider.GetDependency() .CreateAsync(Arg.Any()) .Returns(c => c.ArgAt(0)); @@ -435,7 +461,9 @@ public class AuthRequestServiceTests await sutProvider.GetDependency() .Received(1) .SendDeviceApprovalRequestedNotificationEmailAsync( - Arg.Is>(emails => emails.Count() == 2 && emails.Contains(admin2.Email) && emails.Contains(admin3.Email)), + Arg.Is>(emails => emails.Count() == 3 && + emails.Contains(admin2.Email) && emails.Contains(admin3.Email) && + emails.Contains(customUser2.Email)), organizationUser2.OrganizationId, user.Email, user.Name); diff --git a/util/Migrator/DbScripts/2025-02-03_00_OrgUserReadManyDetailsByRole.sql b/util/Migrator/DbScripts/2025-02-03_00_OrgUserReadManyDetailsByRole.sql new file mode 100644 index 0000000000..4d687f0bb1 --- /dev/null +++ b/util/Migrator/DbScripts/2025-02-03_00_OrgUserReadManyDetailsByRole.sql @@ -0,0 +1,16 @@ +CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_ReadManyDetailsByRole] + @OrganizationId UNIQUEIDENTIFIER, + @Role TINYINT +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[OrganizationUserUserDetailsView] + WHERE + OrganizationId = @OrganizationId + AND Status = 2 -- 2 = Confirmed + AND [Type] = @Role +END From 77364549fa9dd756a02c0bea04761b7f01beaa76 Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Wed, 5 Feb 2025 10:03:13 -0500 Subject: [PATCH 07/25] [PM-16157] Add feature flag for mTLS support in Android client (#5335) Add a feature flag to control support for selecting a mutual TLS client certificate within the Android client. --- src/Core/Constants.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 03d6a193a7..ba3b8b0795 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -173,6 +173,7 @@ public static class FeatureFlagKeys public const string SingleTapPasskeyAuthentication = "single-tap-passkey-authentication"; public const string EnablePMAuthenticatorSync = "enable-pm-bwa-sync"; public const string P15179_AddExistingOrgsFromProviderPortal = "pm-15179-add-existing-orgs-from-provider-portal"; + public const string AndroidMutualTls = "mutual-tls"; public static List GetAllKeys() { From a971a18719b4a9a8cf3af77270b1799ed6942083 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Wed, 5 Feb 2025 15:32:27 -0500 Subject: [PATCH 08/25] [PM-17957] Pin Transitive Deps (#5371) * Remove duplicate quartz reference * Pin Core packages * Pin Notifications packages --- src/Core/Core.csproj | 6 +++++- src/Notifications/Notifications.csproj | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index 7b319e56c9..a2d4660194 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -51,7 +51,6 @@ - @@ -73,6 +72,11 @@ + + + + + diff --git a/src/Notifications/Notifications.csproj b/src/Notifications/Notifications.csproj index 68ae96963e..4d19f7faf9 100644 --- a/src/Notifications/Notifications.csproj +++ b/src/Notifications/Notifications.csproj @@ -11,6 +11,10 @@ + + + + From 46004b9c6849f0ca7c8ddbfce930cbb2f72a9c0a Mon Sep 17 00:00:00 2001 From: SmithThe4th Date: Wed, 5 Feb 2025 16:56:01 -0500 Subject: [PATCH 09/25] [PM-14381] Add POST /tasks/bulk-create endpoint (#5188) * [PM-14378] Introduce GetCipherPermissionsForOrganization query for Dapper CipherRepository * [PM-14378] Introduce GetCipherPermissionsForOrganization method for Entity Framework * [PM-14378] Add integration tests for new repository method * [PM-14378] Introduce IGetCipherPermissionsForUserQuery CQRS query * [PM-14378] Introduce SecurityTaskOperationRequirement * [PM-14378] Introduce SecurityTaskAuthorizationHandler.cs * [PM-14378] Introduce SecurityTaskOrganizationAuthorizationHandler.cs * [PM-14378] Register new authorization handlers * [PM-14378] Formatting * [PM-14378] Add unit tests for GetCipherPermissionsForUserQuery * [PM-15378] Cleanup SecurityTaskAuthorizationHandler and add tests * [PM-14378] Add tests for SecurityTaskOrganizationAuthorizationHandler * [PM-14378] Formatting * [PM-14378] Update date in migration file * [PM-14378] Add missing awaits * Added bulk create request model * Created sproc to create bulk security tasks * Renamed tasks to SecurityTasksInput * Added create many implementation for sqlserver and ef core * removed trailing comma * created ef implementatin for create many and added integration test * Refactored request model * Refactored request model * created create many tasks command interface and class * added security authorization handler work temp * Added the implementation for the create manys tasks command * Added comment * Changed return to return list of created security tasks * Registered command * Completed bulk create action * Added unit tests for the command * removed hard coded table name * Fixed lint issue * Added JsonConverter attribute to allow enum value to be passed as string * Removed makshift security task operations * Fixed references * Removed old migration * Rebased * [PM-14378] Introduce GetCipherPermissionsForOrganization query for Dapper CipherRepository * [PM-14378] Introduce GetCipherPermissionsForOrganization method for Entity Framework * [PM-14378] Add unit tests for GetCipherPermissionsForUserQuery * Completed bulk create action * bumped migration version * Fixed lint issue * Removed complex sql data type in favour of json string * Register IGetTasksForOrganizationQuery * Fixed lint issue * Removed tasks grouping * Fixed linting * Removed unused code * Removed unused code * Aligned with client change * Fixed linting --------- Co-authored-by: Shane Melton --- .../Controllers/SecurityTaskController.cs | 21 ++++- .../BulkCreateSecurityTasksRequestModel.cs | 8 ++ .../Authorization/SecurityTaskOperations.cs | 16 ---- .../Vault/Commands/CreateManyTasksCommand.cs | 65 ++++++++++++++ .../Interfaces/ICreateManyTasksCommand.cs | 17 ++++ .../Commands/MarkTaskAsCompletedCommand.cs | 2 +- .../Models/Api/SecurityTaskCreateRequest.cs | 9 ++ .../Repositories/ISecurityTaskRepository.cs | 7 ++ .../Vault/VaultServiceCollectionExtensions.cs | 1 + src/Infrastructure.Dapper/DapperHelpers.cs | 2 +- .../Repositories/SecurityTaskRepository.cs | 26 ++++++ .../Repositories/SecurityTaskRepository.cs | 24 ++++++ .../SecurityTask/SecurityTask_CreateMany.sql | 55 ++++++++++++ .../Commands/CreateManyTasksCommandTest.cs | 85 +++++++++++++++++++ .../MarkTaskAsCompletedCommandTest.cs | 2 +- .../SecurityTaskRepositoryTests.cs | 43 ++++++++++ .../2025-01-22_00_SecurityTaskCreateMany.sql | 55 ++++++++++++ 17 files changed, 418 insertions(+), 20 deletions(-) create mode 100644 src/Api/Vault/Models/Request/BulkCreateSecurityTasksRequestModel.cs delete mode 100644 src/Core/Vault/Authorization/SecurityTaskOperations.cs create mode 100644 src/Core/Vault/Commands/CreateManyTasksCommand.cs create mode 100644 src/Core/Vault/Commands/Interfaces/ICreateManyTasksCommand.cs create mode 100644 src/Core/Vault/Models/Api/SecurityTaskCreateRequest.cs create mode 100644 src/Sql/Vault/dbo/Stored Procedures/SecurityTask/SecurityTask_CreateMany.sql create mode 100644 test/Core.Test/Vault/Commands/CreateManyTasksCommandTest.cs create mode 100644 util/Migrator/DbScripts/2025-01-22_00_SecurityTaskCreateMany.sql diff --git a/src/Api/Vault/Controllers/SecurityTaskController.cs b/src/Api/Vault/Controllers/SecurityTaskController.cs index 14ef0e5e4e..88b7aed9c6 100644 --- a/src/Api/Vault/Controllers/SecurityTaskController.cs +++ b/src/Api/Vault/Controllers/SecurityTaskController.cs @@ -1,4 +1,5 @@ using Bit.Api.Models.Response; +using Bit.Api.Vault.Models.Request; using Bit.Api.Vault.Models.Response; using Bit.Core; using Bit.Core.Services; @@ -20,17 +21,20 @@ public class SecurityTaskController : Controller private readonly IGetTaskDetailsForUserQuery _getTaskDetailsForUserQuery; private readonly IMarkTaskAsCompleteCommand _markTaskAsCompleteCommand; private readonly IGetTasksForOrganizationQuery _getTasksForOrganizationQuery; + private readonly ICreateManyTasksCommand _createManyTasksCommand; public SecurityTaskController( IUserService userService, IGetTaskDetailsForUserQuery getTaskDetailsForUserQuery, IMarkTaskAsCompleteCommand markTaskAsCompleteCommand, - IGetTasksForOrganizationQuery getTasksForOrganizationQuery) + IGetTasksForOrganizationQuery getTasksForOrganizationQuery, + ICreateManyTasksCommand createManyTasksCommand) { _userService = userService; _getTaskDetailsForUserQuery = getTaskDetailsForUserQuery; _markTaskAsCompleteCommand = markTaskAsCompleteCommand; _getTasksForOrganizationQuery = getTasksForOrganizationQuery; + _createManyTasksCommand = createManyTasksCommand; } /// @@ -71,4 +75,19 @@ public class SecurityTaskController : Controller var response = securityTasks.Select(x => new SecurityTasksResponseModel(x)).ToList(); return new ListResponseModel(response); } + + /// + /// Bulk create security tasks for an organization. + /// + /// + /// + /// A list response model containing the security tasks created for the organization. + [HttpPost("{orgId:guid}/bulk-create")] + public async Task> BulkCreateTasks(Guid orgId, + [FromBody] BulkCreateSecurityTasksRequestModel model) + { + var securityTasks = await _createManyTasksCommand.CreateAsync(orgId, model.Tasks); + var response = securityTasks.Select(x => new SecurityTasksResponseModel(x)).ToList(); + return new ListResponseModel(response); + } } diff --git a/src/Api/Vault/Models/Request/BulkCreateSecurityTasksRequestModel.cs b/src/Api/Vault/Models/Request/BulkCreateSecurityTasksRequestModel.cs new file mode 100644 index 0000000000..6c8c7e03b3 --- /dev/null +++ b/src/Api/Vault/Models/Request/BulkCreateSecurityTasksRequestModel.cs @@ -0,0 +1,8 @@ +using Bit.Core.Vault.Models.Api; + +namespace Bit.Api.Vault.Models.Request; + +public class BulkCreateSecurityTasksRequestModel +{ + public IEnumerable Tasks { get; set; } +} diff --git a/src/Core/Vault/Authorization/SecurityTaskOperations.cs b/src/Core/Vault/Authorization/SecurityTaskOperations.cs deleted file mode 100644 index 77b504723f..0000000000 --- a/src/Core/Vault/Authorization/SecurityTaskOperations.cs +++ /dev/null @@ -1,16 +0,0 @@ -using Microsoft.AspNetCore.Authorization.Infrastructure; - -namespace Bit.Core.Vault.Authorization; - -public class SecurityTaskOperationRequirement : OperationAuthorizationRequirement -{ - public SecurityTaskOperationRequirement(string name) - { - Name = name; - } -} - -public static class SecurityTaskOperations -{ - public static readonly SecurityTaskOperationRequirement Update = new(nameof(Update)); -} diff --git a/src/Core/Vault/Commands/CreateManyTasksCommand.cs b/src/Core/Vault/Commands/CreateManyTasksCommand.cs new file mode 100644 index 0000000000..1b21f202eb --- /dev/null +++ b/src/Core/Vault/Commands/CreateManyTasksCommand.cs @@ -0,0 +1,65 @@ +using Bit.Core.Context; +using Bit.Core.Exceptions; +using Bit.Core.Utilities; +using Bit.Core.Vault.Authorization.SecurityTasks; +using Bit.Core.Vault.Commands.Interfaces; +using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Enums; +using Bit.Core.Vault.Models.Api; +using Bit.Core.Vault.Repositories; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Core.Vault.Commands; + +public class CreateManyTasksCommand : ICreateManyTasksCommand +{ + private readonly IAuthorizationService _authorizationService; + private readonly ICurrentContext _currentContext; + private readonly ISecurityTaskRepository _securityTaskRepository; + + public CreateManyTasksCommand( + ISecurityTaskRepository securityTaskRepository, + IAuthorizationService authorizationService, + ICurrentContext currentContext) + { + _securityTaskRepository = securityTaskRepository; + _authorizationService = authorizationService; + _currentContext = currentContext; + } + + /// + public async Task> CreateAsync(Guid organizationId, + IEnumerable tasks) + { + if (!_currentContext.UserId.HasValue) + { + throw new NotFoundException(); + } + + var tasksList = tasks?.ToList(); + + if (tasksList is null || tasksList.Count == 0) + { + throw new BadRequestException("No tasks provided."); + } + + var securityTasks = tasksList.Select(t => new SecurityTask + { + OrganizationId = organizationId, + CipherId = t.CipherId, + Type = t.Type, + Status = SecurityTaskStatus.Pending + }).ToList(); + + // Verify authorization for each task + foreach (var task in securityTasks) + { + await _authorizationService.AuthorizeOrThrowAsync( + _currentContext.HttpContext.User, + task, + SecurityTaskOperations.Create); + } + + return await _securityTaskRepository.CreateManyAsync(securityTasks); + } +} diff --git a/src/Core/Vault/Commands/Interfaces/ICreateManyTasksCommand.cs b/src/Core/Vault/Commands/Interfaces/ICreateManyTasksCommand.cs new file mode 100644 index 0000000000..3aa0f85070 --- /dev/null +++ b/src/Core/Vault/Commands/Interfaces/ICreateManyTasksCommand.cs @@ -0,0 +1,17 @@ +using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Models.Api; + +namespace Bit.Core.Vault.Commands.Interfaces; + +public interface ICreateManyTasksCommand +{ + /// + /// Creates multiple security tasks for an organization. + /// Each task must be authorized and the user must have the Create permission + /// and associated ciphers must belong to the organization. + /// + /// The + /// + /// Collection of created security tasks + Task> CreateAsync(Guid organizationId, IEnumerable tasks); +} diff --git a/src/Core/Vault/Commands/MarkTaskAsCompletedCommand.cs b/src/Core/Vault/Commands/MarkTaskAsCompletedCommand.cs index b46fb0cecb..77b8a8625c 100644 --- a/src/Core/Vault/Commands/MarkTaskAsCompletedCommand.cs +++ b/src/Core/Vault/Commands/MarkTaskAsCompletedCommand.cs @@ -1,7 +1,7 @@ using Bit.Core.Context; using Bit.Core.Exceptions; using Bit.Core.Utilities; -using Bit.Core.Vault.Authorization; +using Bit.Core.Vault.Authorization.SecurityTasks; using Bit.Core.Vault.Commands.Interfaces; using Bit.Core.Vault.Enums; using Bit.Core.Vault.Repositories; diff --git a/src/Core/Vault/Models/Api/SecurityTaskCreateRequest.cs b/src/Core/Vault/Models/Api/SecurityTaskCreateRequest.cs new file mode 100644 index 0000000000..f865871380 --- /dev/null +++ b/src/Core/Vault/Models/Api/SecurityTaskCreateRequest.cs @@ -0,0 +1,9 @@ +using Bit.Core.Vault.Enums; + +namespace Bit.Core.Vault.Models.Api; + +public class SecurityTaskCreateRequest +{ + public SecurityTaskType Type { get; set; } + public Guid? CipherId { get; set; } +} diff --git a/src/Core/Vault/Repositories/ISecurityTaskRepository.cs b/src/Core/Vault/Repositories/ISecurityTaskRepository.cs index c236172533..cc8303345d 100644 --- a/src/Core/Vault/Repositories/ISecurityTaskRepository.cs +++ b/src/Core/Vault/Repositories/ISecurityTaskRepository.cs @@ -21,4 +21,11 @@ public interface ISecurityTaskRepository : IRepository /// Optional filter for task status. If not provided, returns tasks of all statuses /// Task> GetManyByOrganizationIdStatusAsync(Guid organizationId, SecurityTaskStatus? status = null); + + /// + /// Creates bulk security tasks for an organization. + /// + /// Collection of tasks to create + /// Collection of created security tasks + Task> CreateManyAsync(IEnumerable tasks); } diff --git a/src/Core/Vault/VaultServiceCollectionExtensions.cs b/src/Core/Vault/VaultServiceCollectionExtensions.cs index 169a62d12d..fcb9259135 100644 --- a/src/Core/Vault/VaultServiceCollectionExtensions.cs +++ b/src/Core/Vault/VaultServiceCollectionExtensions.cs @@ -21,5 +21,6 @@ public static class VaultServiceCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); } } diff --git a/src/Infrastructure.Dapper/DapperHelpers.cs b/src/Infrastructure.Dapper/DapperHelpers.cs index c256612447..9a67af3a93 100644 --- a/src/Infrastructure.Dapper/DapperHelpers.cs +++ b/src/Infrastructure.Dapper/DapperHelpers.cs @@ -81,7 +81,7 @@ public class DataTableBuilder return true; } - // Value type properties will implicitly box into the object so + // Value type properties will implicitly box into the object so // we need to look past the Convert expression // i => (System.Object?)i.Id if ( diff --git a/src/Infrastructure.Dapper/Vault/Repositories/SecurityTaskRepository.cs b/src/Infrastructure.Dapper/Vault/Repositories/SecurityTaskRepository.cs index 35dace9a9e..f7a5f3b878 100644 --- a/src/Infrastructure.Dapper/Vault/Repositories/SecurityTaskRepository.cs +++ b/src/Infrastructure.Dapper/Vault/Repositories/SecurityTaskRepository.cs @@ -1,4 +1,5 @@ using System.Data; +using System.Text.Json; using Bit.Core.Settings; using Bit.Core.Vault.Entities; using Bit.Core.Vault.Enums; @@ -46,4 +47,29 @@ public class SecurityTaskRepository : Repository, ISecurityT return results.ToList(); } + + /// + public async Task> CreateManyAsync(IEnumerable tasks) + { + var tasksList = tasks?.ToList(); + if (tasksList is null || tasksList.Count == 0) + { + return Array.Empty(); + } + + foreach (var task in tasksList) + { + task.SetNewId(); + } + + var tasksJson = JsonSerializer.Serialize(tasksList); + + await using var connection = new SqlConnection(ConnectionString); + await connection.ExecuteAsync( + $"[{Schema}].[{Table}_CreateMany]", + new { SecurityTasksJson = tasksJson }, + commandType: CommandType.StoredProcedure); + + return tasksList; + } } diff --git a/src/Infrastructure.EntityFramework/Vault/Repositories/SecurityTaskRepository.cs b/src/Infrastructure.EntityFramework/Vault/Repositories/SecurityTaskRepository.cs index 5adfdc4c76..a3ba2632fe 100644 --- a/src/Infrastructure.EntityFramework/Vault/Repositories/SecurityTaskRepository.cs +++ b/src/Infrastructure.EntityFramework/Vault/Repositories/SecurityTaskRepository.cs @@ -52,4 +52,28 @@ public class SecurityTaskRepository : Repository st.CreationDate).ToListAsync(); } + + /// + public async Task> CreateManyAsync( + IEnumerable tasks) + { + var tasksList = tasks?.ToList(); + if (tasksList is null || tasksList.Count == 0) + { + return Array.Empty(); + } + + foreach (var task in tasksList) + { + task.SetNewId(); + } + + using var scope = ServiceScopeFactory.CreateScope(); + var dbContext = GetDatabaseContext(scope); + var entities = Mapper.Map>(tasksList); + await dbContext.AddRangeAsync(entities); + await dbContext.SaveChangesAsync(); + + return tasksList; + } } diff --git a/src/Sql/Vault/dbo/Stored Procedures/SecurityTask/SecurityTask_CreateMany.sql b/src/Sql/Vault/dbo/Stored Procedures/SecurityTask/SecurityTask_CreateMany.sql new file mode 100644 index 0000000000..9e60f2ad1b --- /dev/null +++ b/src/Sql/Vault/dbo/Stored Procedures/SecurityTask/SecurityTask_CreateMany.sql @@ -0,0 +1,55 @@ +CREATE PROCEDURE [dbo].[SecurityTask_CreateMany] + @SecurityTasksJson NVARCHAR(MAX) +AS +BEGIN + SET NOCOUNT ON + + CREATE TABLE #TempSecurityTasks + ( + [Id] UNIQUEIDENTIFIER, + [OrganizationId] UNIQUEIDENTIFIER, + [CipherId] UNIQUEIDENTIFIER, + [Type] TINYINT, + [Status] TINYINT, + [CreationDate] DATETIME2(7), + [RevisionDate] DATETIME2(7) + ) + + INSERT INTO #TempSecurityTasks + ([Id], + [OrganizationId], + [CipherId], + [Type], + [Status], + [CreationDate], + [RevisionDate]) + SELECT CAST(JSON_VALUE([value], '$.Id') AS UNIQUEIDENTIFIER), + CAST(JSON_VALUE([value], '$.OrganizationId') AS UNIQUEIDENTIFIER), + CAST(JSON_VALUE([value], '$.CipherId') AS UNIQUEIDENTIFIER), + CAST(JSON_VALUE([value], '$.Type') AS TINYINT), + CAST(JSON_VALUE([value], '$.Status') AS TINYINT), + CAST(JSON_VALUE([value], '$.CreationDate') AS DATETIME2(7)), + CAST(JSON_VALUE([value], '$.RevisionDate') AS DATETIME2(7)) + FROM OPENJSON(@SecurityTasksJson) ST + + INSERT INTO [dbo].[SecurityTask] + ( + [Id], + [OrganizationId], + [CipherId], + [Type], + [Status], + [CreationDate], + [RevisionDate] + ) + SELECT [Id], + [OrganizationId], + [CipherId], + [Type], + [Status], + [CreationDate], + [RevisionDate] + FROM #TempSecurityTasks + + DROP TABLE #TempSecurityTasks +END diff --git a/test/Core.Test/Vault/Commands/CreateManyTasksCommandTest.cs b/test/Core.Test/Vault/Commands/CreateManyTasksCommandTest.cs new file mode 100644 index 0000000000..23e92965f2 --- /dev/null +++ b/test/Core.Test/Vault/Commands/CreateManyTasksCommandTest.cs @@ -0,0 +1,85 @@ +using System.Security.Claims; +using Bit.Core.Context; +using Bit.Core.Exceptions; +using Bit.Core.Test.Vault.AutoFixture; +using Bit.Core.Vault.Authorization.SecurityTasks; +using Bit.Core.Vault.Commands; +using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Models.Api; +using Bit.Core.Vault.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Authorization; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.Vault.Commands; + +[SutProviderCustomize] +[SecurityTaskCustomize] +public class CreateManyTasksCommandTest +{ + private static void Setup(SutProvider sutProvider, Guid? userId, + bool authorizedCreate = false) + { + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Is>(reqs => + reqs.Contains(SecurityTaskOperations.Create))) + .Returns(authorizedCreate ? AuthorizationResult.Success() : AuthorizationResult.Failed()); + } + + [Theory] + [BitAutoData] + public async Task CreateAsync_NotLoggedIn_NotFoundException( + SutProvider sutProvider, + Guid organizationId, + IEnumerable tasks) + { + Setup(sutProvider, null, true); + + await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(organizationId, tasks)); + } + + [Theory] + [BitAutoData] + public async Task CreateAsync_NoTasksProvided_BadRequestException( + SutProvider sutProvider, + Guid organizationId) + { + Setup(sutProvider, Guid.NewGuid()); + + await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(organizationId, null)); + } + + [Theory] + [BitAutoData] + public async Task CreateAsync_AuthorizationFailed_NotFoundException( + SutProvider sutProvider, + Guid organizationId, + IEnumerable tasks) + { + Setup(sutProvider, Guid.NewGuid()); + + await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(organizationId, tasks)); + } + + [Theory] + [BitAutoData] + public async Task CreateAsync_AuthorizationSucceeded_ReturnsSecurityTasks( + SutProvider sutProvider, + Guid organizationId, + IEnumerable tasks, + ICollection securityTasks) + { + Setup(sutProvider, Guid.NewGuid(), true); + sutProvider.GetDependency() + .CreateManyAsync(Arg.Any>()) + .Returns(securityTasks); + + var result = await sutProvider.Sut.CreateAsync(organizationId, tasks); + + Assert.Equal(securityTasks, result); + } +} diff --git a/test/Core.Test/Vault/Commands/MarkTaskAsCompletedCommandTest.cs b/test/Core.Test/Vault/Commands/MarkTaskAsCompletedCommandTest.cs index 82550df48d..ca9a42cdb3 100644 --- a/test/Core.Test/Vault/Commands/MarkTaskAsCompletedCommandTest.cs +++ b/test/Core.Test/Vault/Commands/MarkTaskAsCompletedCommandTest.cs @@ -3,7 +3,7 @@ using System.Security.Claims; using Bit.Core.Context; using Bit.Core.Exceptions; using Bit.Core.Test.Vault.AutoFixture; -using Bit.Core.Vault.Authorization; +using Bit.Core.Vault.Authorization.SecurityTasks; using Bit.Core.Vault.Commands; using Bit.Core.Vault.Entities; using Bit.Core.Vault.Repositories; diff --git a/test/Infrastructure.IntegrationTest/Vault/Repositories/SecurityTaskRepositoryTests.cs b/test/Infrastructure.IntegrationTest/Vault/Repositories/SecurityTaskRepositoryTests.cs index 2010c90a5e..eb5a310db3 100644 --- a/test/Infrastructure.IntegrationTest/Vault/Repositories/SecurityTaskRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/Vault/Repositories/SecurityTaskRepositoryTests.cs @@ -223,4 +223,47 @@ public class SecurityTaskRepositoryTests Assert.DoesNotContain(task1, completedTasks, new SecurityTaskComparer()); Assert.DoesNotContain(task3, completedTasks, new SecurityTaskComparer()); } + + [DatabaseTheory, DatabaseData] + public async Task CreateManyAsync( + IOrganizationRepository organizationRepository, + ICipherRepository cipherRepository, + ISecurityTaskRepository securityTaskRepository) + { + var organization = await organizationRepository.CreateAsync(new Organization + { + Name = "Test Org", + PlanType = PlanType.EnterpriseAnnually, + Plan = "Test Plan", + BillingEmail = "" + }); + + var cipher1 = new Cipher { Type = CipherType.Login, OrganizationId = organization.Id, Data = "", }; + await cipherRepository.CreateAsync(cipher1); + + var cipher2 = new Cipher { Type = CipherType.Login, OrganizationId = organization.Id, Data = "", }; + await cipherRepository.CreateAsync(cipher2); + + var tasks = new List + { + new() + { + OrganizationId = organization.Id, + CipherId = cipher1.Id, + Status = SecurityTaskStatus.Pending, + Type = SecurityTaskType.UpdateAtRiskCredential, + }, + new() + { + OrganizationId = organization.Id, + CipherId = cipher2.Id, + Status = SecurityTaskStatus.Completed, + Type = SecurityTaskType.UpdateAtRiskCredential, + } + }; + + var taskIds = await securityTaskRepository.CreateManyAsync(tasks); + + Assert.Equal(2, taskIds.Count); + } } diff --git a/util/Migrator/DbScripts/2025-01-22_00_SecurityTaskCreateMany.sql b/util/Migrator/DbScripts/2025-01-22_00_SecurityTaskCreateMany.sql new file mode 100644 index 0000000000..6bf797eccd --- /dev/null +++ b/util/Migrator/DbScripts/2025-01-22_00_SecurityTaskCreateMany.sql @@ -0,0 +1,55 @@ +-- SecurityTask_CreateMany +CREATE OR ALTER PROCEDURE [dbo].[SecurityTask_CreateMany] + @SecurityTasksJson NVARCHAR(MAX) +AS +BEGIN + SET NOCOUNT ON + + CREATE TABLE #TempSecurityTasks + ( + [Id] UNIQUEIDENTIFIER, + [OrganizationId] UNIQUEIDENTIFIER, + [CipherId] UNIQUEIDENTIFIER, + [Type] TINYINT, + [Status] TINYINT, + [CreationDate] DATETIME2(7), + [RevisionDate] DATETIME2(7) + ) + + INSERT INTO #TempSecurityTasks + ([Id], + [OrganizationId], + [CipherId], + [Type], + [Status], + [CreationDate], + [RevisionDate]) + SELECT CAST(JSON_VALUE([value], '$.Id') AS UNIQUEIDENTIFIER), + CAST(JSON_VALUE([value], '$.OrganizationId') AS UNIQUEIDENTIFIER), + CAST(JSON_VALUE([value], '$.CipherId') AS UNIQUEIDENTIFIER), + CAST(JSON_VALUE([value], '$.Type') AS TINYINT), + CAST(JSON_VALUE([value], '$.Status') AS TINYINT), + CAST(JSON_VALUE([value], '$.CreationDate') AS DATETIME2(7)), + CAST(JSON_VALUE([value], '$.RevisionDate') AS DATETIME2(7)) + FROM OPENJSON(@SecurityTasksJson) ST + + INSERT INTO [dbo].[SecurityTask] + ([Id], + [OrganizationId], + [CipherId], + [Type], + [Status], + [CreationDate], + [RevisionDate]) + SELECT [Id], + [OrganizationId], + [CipherId], + [Type], + [Status], + [CreationDate], + [RevisionDate] + FROM #TempSecurityTasks + + DROP TABLE #TempSecurityTasks +END +GO From daf2696a813fba07a704e9b552b5e5151f64bd4f Mon Sep 17 00:00:00 2001 From: Graham Walker Date: Wed, 5 Feb 2025 16:36:18 -0600 Subject: [PATCH 10/25] PM-16085 - Increase import limitations (#5275) * PM-16261 move ImportCiphersAsync to the tools team and create services using CQRS design pattern * PM-16261 fix renaming methods and add unit tests for succes and bad request exception * PM-16261 clean up old code from test * make import limits configurable via appsettings * PM-16085 fix issue with appSettings converting to globalSettings for new cipher import limits --- src/Api/Tools/Controllers/ImportCiphersController.cs | 5 +++-- src/Api/appsettings.json | 5 +++++ src/Core/Settings/GlobalSettings.cs | 8 ++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Api/Tools/Controllers/ImportCiphersController.cs b/src/Api/Tools/Controllers/ImportCiphersController.cs index 4f4e76f6e3..c268500f71 100644 --- a/src/Api/Tools/Controllers/ImportCiphersController.cs +++ b/src/Api/Tools/Controllers/ImportCiphersController.cs @@ -64,8 +64,9 @@ public class ImportCiphersController : Controller [FromBody] ImportOrganizationCiphersRequestModel model) { if (!_globalSettings.SelfHosted && - (model.Ciphers.Count() > 7000 || model.CollectionRelationships.Count() > 14000 || - model.Collections.Count() > 2000)) + (model.Ciphers.Count() > _globalSettings.ImportCiphersLimitation.CiphersLimit || + model.CollectionRelationships.Count() > _globalSettings.ImportCiphersLimitation.CollectionRelationshipsLimit || + model.Collections.Count() > _globalSettings.ImportCiphersLimitation.CollectionsLimit)) { throw new BadRequestException("You cannot import this much data at once."); } diff --git a/src/Api/appsettings.json b/src/Api/appsettings.json index c04539a9fe..98b210cb1e 100644 --- a/src/Api/appsettings.json +++ b/src/Api/appsettings.json @@ -56,6 +56,11 @@ "publicKey": "SECRET", "privateKey": "SECRET" }, + "importCiphersLimitation": { + "ciphersLimit": 40000, + "collectionRelationshipsLimit": 80000, + "collectionsLimit": 2000 + }, "bitPay": { "production": false, "token": "SECRET", diff --git a/src/Core/Settings/GlobalSettings.cs b/src/Core/Settings/GlobalSettings.cs index d039102eb9..a63a36c1c0 100644 --- a/src/Core/Settings/GlobalSettings.cs +++ b/src/Core/Settings/GlobalSettings.cs @@ -70,6 +70,7 @@ public class GlobalSettings : IGlobalSettings public virtual YubicoSettings Yubico { get; set; } = new YubicoSettings(); public virtual DuoSettings Duo { get; set; } = new DuoSettings(); public virtual BraintreeSettings Braintree { get; set; } = new BraintreeSettings(); + public virtual ImportCiphersLimitationSettings ImportCiphersLimitation { get; set; } = new ImportCiphersLimitationSettings(); public virtual BitPaySettings BitPay { get; set; } = new BitPaySettings(); public virtual AmazonSettings Amazon { get; set; } = new AmazonSettings(); public virtual ServiceBusSettings ServiceBus { get; set; } = new ServiceBusSettings(); @@ -521,6 +522,13 @@ public class GlobalSettings : IGlobalSettings public string PrivateKey { get; set; } } + public class ImportCiphersLimitationSettings + { + public int CiphersLimit { get; set; } + public int CollectionRelationshipsLimit { get; set; } + public int CollectionsLimit { get; set; } + } + public class BitPaySettings { public bool Production { get; set; } From 1c3ea1151c69aece541810819b8c1b6a31452f9b Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Thu, 6 Feb 2025 09:22:16 +0100 Subject: [PATCH 11/25] [PM-16482]NullReferenceException in CustomerUpdatedHandler due to uninitialized dependency (#5349) * Changes to throw exact errors * Add some logging to each error state Signed-off-by: Cy Okeke --------- Signed-off-by: Cy Okeke --- .../Implementations/CustomerUpdatedHandler.cs | 62 +++++++++++++++++-- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/src/Billing/Services/Implementations/CustomerUpdatedHandler.cs b/src/Billing/Services/Implementations/CustomerUpdatedHandler.cs index ec70697c01..6deb0bc330 100644 --- a/src/Billing/Services/Implementations/CustomerUpdatedHandler.cs +++ b/src/Billing/Services/Implementations/CustomerUpdatedHandler.cs @@ -14,19 +14,22 @@ public class CustomerUpdatedHandler : ICustomerUpdatedHandler private readonly ICurrentContext _currentContext; private readonly IStripeEventService _stripeEventService; private readonly IStripeEventUtilityService _stripeEventUtilityService; + private readonly ILogger _logger; public CustomerUpdatedHandler( IOrganizationRepository organizationRepository, IReferenceEventService referenceEventService, ICurrentContext currentContext, IStripeEventService stripeEventService, - IStripeEventUtilityService stripeEventUtilityService) + IStripeEventUtilityService stripeEventUtilityService, + ILogger logger) { - _organizationRepository = organizationRepository; + _organizationRepository = organizationRepository ?? throw new ArgumentNullException(nameof(organizationRepository)); _referenceEventService = referenceEventService; _currentContext = currentContext; _stripeEventService = stripeEventService; _stripeEventUtilityService = stripeEventUtilityService; + _logger = logger; } /// @@ -35,25 +38,76 @@ public class CustomerUpdatedHandler : ICustomerUpdatedHandler /// public async Task HandleAsync(Event parsedEvent) { - var customer = await _stripeEventService.GetCustomer(parsedEvent, true, ["subscriptions"]); - if (customer.Subscriptions == null || !customer.Subscriptions.Any()) + if (parsedEvent == null) { + _logger.LogError("Parsed event was null in CustomerUpdatedHandler"); + throw new ArgumentNullException(nameof(parsedEvent)); + } + + if (_stripeEventService == null) + { + _logger.LogError("StripeEventService was not initialized in CustomerUpdatedHandler"); + throw new InvalidOperationException($"{nameof(_stripeEventService)} is not initialized"); + } + + var customer = await _stripeEventService.GetCustomer(parsedEvent, true, ["subscriptions"]); + if (customer?.Subscriptions == null || !customer.Subscriptions.Any()) + { + _logger.LogWarning("Customer or subscriptions were null or empty in CustomerUpdatedHandler. Customer ID: {CustomerId}", customer?.Id); return; } var subscription = customer.Subscriptions.First(); + if (subscription.Metadata == null) + { + _logger.LogWarning("Subscription metadata was null in CustomerUpdatedHandler. Subscription ID: {SubscriptionId}", subscription.Id); + return; + } + + if (_stripeEventUtilityService == null) + { + _logger.LogError("StripeEventUtilityService was not initialized in CustomerUpdatedHandler"); + throw new InvalidOperationException($"{nameof(_stripeEventUtilityService)} is not initialized"); + } + var (organizationId, _, providerId) = _stripeEventUtilityService.GetIdsFromMetadata(subscription.Metadata); if (!organizationId.HasValue) { + _logger.LogWarning("Organization ID was not found in subscription metadata. Subscription ID: {SubscriptionId}", subscription.Id); return; } + if (_organizationRepository == null) + { + _logger.LogError("OrganizationRepository was not initialized in CustomerUpdatedHandler"); + throw new InvalidOperationException($"{nameof(_organizationRepository)} is not initialized"); + } + var organization = await _organizationRepository.GetByIdAsync(organizationId.Value); + + if (organization == null) + { + _logger.LogWarning("Organization not found. Organization ID: {OrganizationId}", organizationId.Value); + return; + } + organization.BillingEmail = customer.Email; await _organizationRepository.ReplaceAsync(organization); + if (_referenceEventService == null) + { + _logger.LogError("ReferenceEventService was not initialized in CustomerUpdatedHandler"); + throw new InvalidOperationException($"{nameof(_referenceEventService)} is not initialized"); + } + + if (_currentContext == null) + { + _logger.LogError("CurrentContext was not initialized in CustomerUpdatedHandler"); + throw new InvalidOperationException($"{nameof(_currentContext)} is not initialized"); + } + await _referenceEventService.RaiseEventAsync( new ReferenceEvent(ReferenceEventType.OrganizationEditedInStripe, organization, _currentContext)); } From a12b61cc9ea48cf8132f2427747f6c4b11f0eb58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Thu, 6 Feb 2025 10:28:12 +0000 Subject: [PATCH 12/25] [PM-17168] Sync organization user revoked/restored status immediately via push notification (#5330) * [PM-17168] Add push notification for revoked and restored organization users * Add feature flag for push notification on user revoke/restore actions * Add tests for user revocation and restoration with push sync feature flag enabled --- .../Implementations/OrganizationService.cs | 28 ++ src/Core/Constants.cs | 1 + .../Services/OrganizationServiceTests.cs | 267 +++++++++++++----- 3 files changed, 230 insertions(+), 66 deletions(-) diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 70a3227a71..6194aea6c7 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -1951,6 +1951,11 @@ public class OrganizationService : IOrganizationService await RepositoryRevokeUserAsync(organizationUser); await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked); + + if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) + { + await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } } public async Task RevokeUserAsync(OrganizationUser organizationUser, @@ -1958,6 +1963,11 @@ public class OrganizationService : IOrganizationService { await RepositoryRevokeUserAsync(organizationUser); await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked, systemUser); + + if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) + { + await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } } private async Task RepositoryRevokeUserAsync(OrganizationUser organizationUser) @@ -2023,6 +2033,10 @@ public class OrganizationService : IOrganizationService await _organizationUserRepository.RevokeAsync(organizationUser.Id); organizationUser.Status = OrganizationUserStatusType.Revoked; await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked); + if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) + { + await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } result.Add(Tuple.Create(organizationUser, "")); } @@ -2050,12 +2064,22 @@ public class OrganizationService : IOrganizationService await RepositoryRestoreUserAsync(organizationUser); await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + + if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) + { + await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } } public async Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser) { await RepositoryRestoreUserAsync(organizationUser); await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, systemUser); + + if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) + { + await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } } private async Task RepositoryRestoreUserAsync(OrganizationUser organizationUser) @@ -2147,6 +2171,10 @@ public class OrganizationService : IOrganizationService await _organizationUserRepository.RestoreAsync(organizationUser.Id, status); organizationUser.Status = status; await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) + { + await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } result.Add(Tuple.Create(organizationUser, "")); } diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index ba3b8b0795..3e59a9390b 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -108,6 +108,7 @@ public static class FeatureFlagKeys public const string IntegrationPage = "pm-14505-admin-console-integration-page"; public const string DeviceApprovalRequestAdminNotifications = "pm-15637-device-approval-request-admin-notifications"; public const string LimitItemDeletion = "pm-15493-restrict-item-deletion-to-can-manage-permission"; + public const string PushSyncOrgKeysOnRevokeRestore = "pm-17168-push-sync-org-keys-on-revoke-restore"; /* Tools Team */ public const string ItemShare = "item-share"; diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index cd680f2ef0..f4440a65a3 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -19,6 +19,7 @@ using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Models.Mail; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; +using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; @@ -1450,76 +1451,174 @@ OrganizationUserInvite invite, SutProvider sutProvider) [OrganizationUser] OrganizationUser organizationUser, SutProvider sutProvider) { RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); await sutProvider.Sut.RevokeUserAsync(organizationUser, owner.Id); - await organizationUserRepository.Received().RevokeAsync(organizationUser.Id); - await eventService.Received() + await sutProvider.GetDependency() + .Received(1) + .RevokeAsync(organizationUser.Id); + await sutProvider.GetDependency() + .Received(1) .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked); } + [Theory, BitAutoData] + public async Task RevokeUser_WithPushSyncOrgKeysOnRevokeRestoreEnabled_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser] OrganizationUser organizationUser, SutProvider sutProvider) + { + RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) + .Returns(true); + + await sutProvider.Sut.RevokeUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RevokeAsync(organizationUser.Id); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked); + await sutProvider.GetDependency() + .Received(1) + .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); + } + [Theory, BitAutoData] public async Task RevokeUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) { RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); await sutProvider.Sut.RevokeUserAsync(organizationUser, eventSystemUser); - await organizationUserRepository.Received().RevokeAsync(organizationUser.Id); - await eventService.Received() + await sutProvider.GetDependency() + .Received(1) + .RevokeAsync(organizationUser.Id); + await sutProvider.GetDependency() + .Received(1) .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked, eventSystemUser); } + [Theory, BitAutoData] + public async Task RevokeUser_WithEventSystemUser_WithPushSyncOrgKeysOnRevokeRestoreEnabled_Success(Organization organization, [OrganizationUser] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) + { + RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) + .Returns(true); + + await sutProvider.Sut.RevokeUserAsync(organizationUser, eventSystemUser); + + await sutProvider.GetDependency() + .Received(1) + .RevokeAsync(organizationUser.Id); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked, eventSystemUser); + await sutProvider.GetDependency() + .Received(1) + .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); + } + [Theory, BitAutoData] public async Task RestoreUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) { RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); - await organizationUserRepository.Received().RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); - await eventService.Received() + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); } + [Theory, BitAutoData] + public async Task RestoreUser_WithPushSyncOrgKeysOnRevokeRestoreEnabled_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) + { + RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) + .Returns(true); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + await sutProvider.GetDependency() + .Received(1) + .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); + } + [Theory, BitAutoData] public async Task RestoreUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) { RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); await sutProvider.Sut.RestoreUserAsync(organizationUser, eventSystemUser); - await organizationUserRepository.Received().RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); - await eventService.Received() + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, eventSystemUser); } + [Theory, BitAutoData] + public async Task RestoreUser_WithEventSystemUser_WithPushSyncOrgKeysOnRevokeRestoreEnabled_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) + { + RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) + .Returns(true); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, eventSystemUser); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, eventSystemUser); + await sutProvider.GetDependency() + .Received(1) + .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); + } + [Theory, BitAutoData] public async Task RestoreUser_RestoreThemselves_Fails(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) { organizationUser.UserId = owner.Id; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("you cannot restore yourself", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory] @@ -1531,17 +1630,21 @@ OrganizationUserInvite invite, SutProvider sutProvider) { restoringUser.Type = restoringUserType; RestoreRevokeUser_Setup(organization, restoringUser, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.RestoreUserAsync(organizationUser, restoringUser.Id)); Assert.Contains("only owners can restore other owners", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory] @@ -1553,17 +1656,21 @@ OrganizationUserInvite invite, SutProvider sutProvider) { organizationUser.Status = userStatus; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("already active", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory, BitAutoData] @@ -1575,8 +1682,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) { organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); sutProvider.GetDependency() .AnyPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) @@ -1591,9 +1696,15 @@ OrganizationUserInvite invite, SutProvider sutProvider) Assert.Contains("test@bitwarden.com belongs to an organization that doesn't allow them to join multiple organizations", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory, BitAutoData] @@ -1610,8 +1721,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, false) }); RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) @@ -1626,9 +1735,15 @@ OrganizationUserInvite invite, SutProvider sutProvider) Assert.Contains("test@bitwarden.com is not compliant with the two-step login policy", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory, BitAutoData] @@ -1640,8 +1755,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) { organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) @@ -1652,8 +1765,11 @@ OrganizationUserInvite invite, SutProvider sutProvider) await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); - await organizationUserRepository.Received().RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); - await eventService.Received() + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); + await sutProvider.GetDependency() + .Received(1) .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); } @@ -1668,10 +1784,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke secondOrganizationUser.UserId = organizationUser.UserId; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); - organizationUserRepository.GetManyByUserAsync(organizationUser.UserId.Value).Returns(new[] { organizationUser, secondOrganizationUser }); + sutProvider.GetDependency() + .GetManyByUserAsync(organizationUser.UserId.Value) + .Returns(new[] { organizationUser, secondOrganizationUser }); sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) .Returns(new[] @@ -1688,9 +1804,15 @@ OrganizationUserInvite invite, SutProvider sutProvider) Assert.Contains("test@bitwarden.com is not compliant with the single organization policy", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory, BitAutoData] @@ -1704,11 +1826,8 @@ OrganizationUserInvite invite, SutProvider sutProvider) organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke secondOrganizationUser.UserId = organizationUser.UserId; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); - var twoFactorIsEnabledQuery = sutProvider.GetDependency(); - twoFactorIsEnabledQuery + sutProvider.GetDependency() .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); @@ -1725,9 +1844,15 @@ OrganizationUserInvite invite, SutProvider sutProvider) Assert.Contains("test@bitwarden.com belongs to an organization that doesn't allow them to join multiple organizations", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory, BitAutoData] @@ -1741,10 +1866,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke secondOrganizationUser.UserId = organizationUser.UserId; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); - organizationUserRepository.GetManyByUserAsync(organizationUser.UserId.Value).Returns(new[] { organizationUser, secondOrganizationUser }); + sutProvider.GetDependency() + .GetManyByUserAsync(organizationUser.UserId.Value) + .Returns(new[] { organizationUser, secondOrganizationUser }); sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) .Returns(new[] @@ -1768,9 +1893,15 @@ OrganizationUserInvite invite, SutProvider sutProvider) Assert.Contains("test@bitwarden.com is not compliant with the single organization and two-step login polciy", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory, BitAutoData] @@ -1783,8 +1914,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) organizationUser.Email = null; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) @@ -1799,9 +1928,15 @@ OrganizationUserInvite invite, SutProvider sutProvider) Assert.Contains("test@bitwarden.com is not compliant with the two-step login policy", exception.Message.ToLowerInvariant()); - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); } [Theory, BitAutoData] @@ -1813,22 +1948,22 @@ OrganizationUserInvite invite, SutProvider sutProvider) { organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); - var twoFactorIsEnabledQuery = sutProvider.GetDependency(); sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); - twoFactorIsEnabledQuery + sutProvider.GetDependency() .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); - await organizationUserRepository.Received().RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); - await eventService.Received() + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); + await sutProvider.GetDependency() + .Received(1) .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); } From 17f5c97891ad983687455b3a46006d15c8a29c7a Mon Sep 17 00:00:00 2001 From: Vijay Oommen Date: Thu, 6 Feb 2025 08:13:17 -0600 Subject: [PATCH 13/25] PM-6939 - Onyx Integration into freshdesk controller (#5365) --- src/Billing/Billing.csproj | 3 + src/Billing/BillingSettings.cs | 7 + src/Billing/Controllers/BitPayController.cs | 1 + .../Controllers/FreshdeskController.cs | 137 +++++++++++++++ .../Models/FreshdeskViewTicketModel.cs | 44 +++++ .../OnyxAnswerWithCitationRequestModel.cs | 54 ++++++ .../OnyxAnswerWithCitationResponseModel.cs | 30 ++++ src/Billing/Startup.cs | 15 ++ src/Billing/appsettings.json | 6 +- .../Controllers/FreshdeskControllerTests.cs | 156 +++++++++++++++++- 10 files changed, 451 insertions(+), 2 deletions(-) create mode 100644 src/Billing/Models/FreshdeskViewTicketModel.cs create mode 100644 src/Billing/Models/OnyxAnswerWithCitationRequestModel.cs create mode 100644 src/Billing/Models/OnyxAnswerWithCitationResponseModel.cs diff --git a/src/Billing/Billing.csproj b/src/Billing/Billing.csproj index b30d987a95..f32eccfe8c 100644 --- a/src/Billing/Billing.csproj +++ b/src/Billing/Billing.csproj @@ -10,5 +10,8 @@ + + + diff --git a/src/Billing/BillingSettings.cs b/src/Billing/BillingSettings.cs index 91ea8f1221..ffe73808d4 100644 --- a/src/Billing/BillingSettings.cs +++ b/src/Billing/BillingSettings.cs @@ -12,6 +12,7 @@ public class BillingSettings public virtual FreshDeskSettings FreshDesk { get; set; } = new FreshDeskSettings(); public virtual string FreshsalesApiKey { get; set; } public virtual PayPalSettings PayPal { get; set; } = new PayPalSettings(); + public virtual OnyxSettings Onyx { get; set; } = new OnyxSettings(); public class PayPalSettings { @@ -31,4 +32,10 @@ public class BillingSettings public virtual string UserFieldName { get; set; } public virtual string OrgFieldName { get; set; } } + + public class OnyxSettings + { + public virtual string ApiKey { get; set; } + public virtual string BaseUrl { get; set; } + } } diff --git a/src/Billing/Controllers/BitPayController.cs b/src/Billing/Controllers/BitPayController.cs index 026909aed1..4caf57aa20 100644 --- a/src/Billing/Controllers/BitPayController.cs +++ b/src/Billing/Controllers/BitPayController.cs @@ -13,6 +13,7 @@ using Microsoft.Extensions.Options; namespace Bit.Billing.Controllers; [Route("bitpay")] +[ApiExplorerSettings(IgnoreApi = true)] public class BitPayController : Controller { private readonly BillingSettings _billingSettings; diff --git a/src/Billing/Controllers/FreshdeskController.cs b/src/Billing/Controllers/FreshdeskController.cs index 7aeb60a67f..4bf6b7bad4 100644 --- a/src/Billing/Controllers/FreshdeskController.cs +++ b/src/Billing/Controllers/FreshdeskController.cs @@ -1,6 +1,8 @@ using System.ComponentModel.DataAnnotations; +using System.Net.Http.Headers; using System.Reflection; using System.Text; +using System.Text.Json; using System.Web; using Bit.Billing.Models; using Bit.Core.Repositories; @@ -142,6 +144,121 @@ public class FreshdeskController : Controller } } + [HttpPost("webhook-onyx-ai")] + public async Task PostWebhookOnyxAi([FromQuery, Required] string key, + [FromBody, Required] FreshdeskWebhookModel model) + { + // ensure that the key is from Freshdesk + if (!IsValidRequestFromFreshdesk(key)) + { + return new BadRequestResult(); + } + + // get ticket info from Freshdesk + var getTicketRequest = new HttpRequestMessage(HttpMethod.Get, + string.Format("https://bitwarden.freshdesk.com/api/v2/tickets/{0}", model.TicketId)); + var getTicketResponse = await CallFreshdeskApiAsync(getTicketRequest); + + // check if we have a valid response from freshdesk + if (getTicketResponse.StatusCode != System.Net.HttpStatusCode.OK) + { + _logger.LogError("Error getting ticket info from Freshdesk. Ticket Id: {0}. Status code: {1}", + model.TicketId, getTicketResponse.StatusCode); + return BadRequest("Failed to retrieve ticket info from Freshdesk"); + } + + // extract info from the response + var ticketInfo = await ExtractTicketInfoFromResponse(getTicketResponse); + if (ticketInfo == null) + { + return BadRequest("Failed to extract ticket info from Freshdesk response"); + } + + // create the onyx `answer-with-citation` request + var onyxRequestModel = new OnyxAnswerWithCitationRequestModel(ticketInfo.DescriptionText); + var onyxRequest = new HttpRequestMessage(HttpMethod.Post, + string.Format("{0}/query/answer-with-citation", _billingSettings.Onyx.BaseUrl)) + { + Content = JsonContent.Create(onyxRequestModel, mediaType: new MediaTypeHeaderValue("application/json")), + }; + var (_, onyxJsonResponse) = await CallOnyxApi(onyxRequest); + + // the CallOnyxApi will return a null if we have an error response + if (onyxJsonResponse?.Answer == null || !string.IsNullOrEmpty(onyxJsonResponse?.ErrorMsg)) + { + return BadRequest( + string.Format("Failed to get a valid response from Onyx API. Response: {0}", + JsonSerializer.Serialize(onyxJsonResponse ?? new OnyxAnswerWithCitationResponseModel()))); + } + + // add the answer as a note to the ticket + await AddAnswerNoteToTicketAsync(onyxJsonResponse.Answer, model.TicketId); + + return Ok(); + } + + private bool IsValidRequestFromFreshdesk(string key) + { + if (string.IsNullOrWhiteSpace(key) + || !CoreHelpers.FixedTimeEquals(key, _billingSettings.FreshDesk.WebhookKey)) + { + return false; + } + + return true; + } + + private async Task AddAnswerNoteToTicketAsync(string note, string ticketId) + { + // if there is no content, then we don't need to add a note + if (string.IsNullOrWhiteSpace(note)) + { + return; + } + + var noteBody = new Dictionary + { + { "body", $"Onyx AI:
    {note}
" }, + { "private", true } + }; + + var noteRequest = new HttpRequestMessage(HttpMethod.Post, + string.Format("https://bitwarden.freshdesk.com/api/v2/tickets/{0}/notes", ticketId)) + { + Content = JsonContent.Create(noteBody), + }; + + var addNoteResponse = await CallFreshdeskApiAsync(noteRequest); + if (addNoteResponse.StatusCode != System.Net.HttpStatusCode.Created) + { + _logger.LogError("Error adding note to Freshdesk ticket. Ticket Id: {0}. Status: {1}", + ticketId, addNoteResponse.ToString()); + } + } + + private async Task ExtractTicketInfoFromResponse(HttpResponseMessage getTicketResponse) + { + var responseString = string.Empty; + try + { + responseString = await getTicketResponse.Content.ReadAsStringAsync(); + var ticketInfo = JsonSerializer.Deserialize(responseString, + options: new System.Text.Json.JsonSerializerOptions + { + PropertyNameCaseInsensitive = true, + }); + + return ticketInfo; + } + catch (System.Exception ex) + { + _logger.LogError("Error deserializing ticket info from Freshdesk response. Response: {0}. Exception {1}", + responseString, ex.ToString()); + } + + return null; + } + private async Task CallFreshdeskApiAsync(HttpRequestMessage request, int retriedCount = 0) { try @@ -166,6 +283,26 @@ public class FreshdeskController : Controller return await CallFreshdeskApiAsync(request, retriedCount++); } + private async Task<(HttpResponseMessage, T)> CallOnyxApi(HttpRequestMessage request) + { + var httpClient = _httpClientFactory.CreateClient("OnyxApi"); + var response = await httpClient.SendAsync(request); + + if (response.StatusCode != System.Net.HttpStatusCode.OK) + { + _logger.LogError("Error calling Onyx AI API. Status code: {0}. Response {1}", + response.StatusCode, JsonSerializer.Serialize(response)); + return (null, default); + } + var responseStr = await response.Content.ReadAsStringAsync(); + var responseJson = JsonSerializer.Deserialize(responseStr, options: new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true, + }); + + return (response, responseJson); + } + private TAttribute GetAttribute(Enum enumValue) where TAttribute : Attribute { return enumValue.GetType().GetMember(enumValue.ToString()).First().GetCustomAttribute(); diff --git a/src/Billing/Models/FreshdeskViewTicketModel.cs b/src/Billing/Models/FreshdeskViewTicketModel.cs new file mode 100644 index 0000000000..2aa6eff94d --- /dev/null +++ b/src/Billing/Models/FreshdeskViewTicketModel.cs @@ -0,0 +1,44 @@ +namespace Bit.Billing.Models; + +using System; +using System.Collections.Generic; +using System.Text.Json.Serialization; + +public class FreshdeskViewTicketModel +{ + [JsonPropertyName("spam")] + public bool? Spam { get; set; } + + [JsonPropertyName("priority")] + public int? Priority { get; set; } + + [JsonPropertyName("source")] + public int? Source { get; set; } + + [JsonPropertyName("status")] + public int? Status { get; set; } + + [JsonPropertyName("subject")] + public string Subject { get; set; } + + [JsonPropertyName("support_email")] + public string SupportEmail { get; set; } + + [JsonPropertyName("id")] + public int Id { get; set; } + + [JsonPropertyName("description")] + public string Description { get; set; } + + [JsonPropertyName("description_text")] + public string DescriptionText { get; set; } + + [JsonPropertyName("created_at")] + public DateTime CreatedAt { get; set; } + + [JsonPropertyName("updated_at")] + public DateTime UpdatedAt { get; set; } + + [JsonPropertyName("tags")] + public List Tags { get; set; } +} diff --git a/src/Billing/Models/OnyxAnswerWithCitationRequestModel.cs b/src/Billing/Models/OnyxAnswerWithCitationRequestModel.cs new file mode 100644 index 0000000000..e7bd29b2f5 --- /dev/null +++ b/src/Billing/Models/OnyxAnswerWithCitationRequestModel.cs @@ -0,0 +1,54 @@ + +using System.Text.Json.Serialization; + +namespace Bit.Billing.Models; + +public class OnyxAnswerWithCitationRequestModel +{ + [JsonPropertyName("messages")] + public List Messages { get; set; } + + [JsonPropertyName("persona_id")] + public int PersonaId { get; set; } = 1; + + [JsonPropertyName("prompt_id")] + public int PromptId { get; set; } = 1; + + [JsonPropertyName("retrieval_options")] + public RetrievalOptions RetrievalOptions { get; set; } + + public OnyxAnswerWithCitationRequestModel(string message) + { + message = message.Replace(Environment.NewLine, " ").Replace('\r', ' ').Replace('\n', ' '); + Messages = new List() { new Message() { MessageText = message } }; + RetrievalOptions = new RetrievalOptions(); + } +} + +public class Message +{ + [JsonPropertyName("message")] + public string MessageText { get; set; } + + [JsonPropertyName("sender")] + public string Sender { get; set; } = "user"; +} + +public class RetrievalOptions +{ + [JsonPropertyName("run_search")] + public string RunSearch { get; set; } = RetrievalOptionsRunSearch.Auto; + + [JsonPropertyName("real_time")] + public bool RealTime { get; set; } = true; + + [JsonPropertyName("limit")] + public int? Limit { get; set; } = 3; +} + +public class RetrievalOptionsRunSearch +{ + public const string Always = "always"; + public const string Never = "never"; + public const string Auto = "auto"; +} diff --git a/src/Billing/Models/OnyxAnswerWithCitationResponseModel.cs b/src/Billing/Models/OnyxAnswerWithCitationResponseModel.cs new file mode 100644 index 0000000000..e85ee9a674 --- /dev/null +++ b/src/Billing/Models/OnyxAnswerWithCitationResponseModel.cs @@ -0,0 +1,30 @@ +using System.Text.Json.Serialization; + +namespace Bit.Billing.Models; + +public class OnyxAnswerWithCitationResponseModel +{ + [JsonPropertyName("answer")] + public string Answer { get; set; } + + [JsonPropertyName("rephrase")] + public string Rephrase { get; set; } + + [JsonPropertyName("citations")] + public List Citations { get; set; } + + [JsonPropertyName("llm_selected_doc_indices")] + public List LlmSelectedDocIndices { get; set; } + + [JsonPropertyName("error_msg")] + public string ErrorMsg { get; set; } +} + +public class Citation +{ + [JsonPropertyName("citation_num")] + public int CitationNum { get; set; } + + [JsonPropertyName("document_id")] + public string DocumentId { get; set; } +} diff --git a/src/Billing/Startup.cs b/src/Billing/Startup.cs index 2d2f109e77..e9f2f53488 100644 --- a/src/Billing/Startup.cs +++ b/src/Billing/Startup.cs @@ -1,4 +1,5 @@ using System.Globalization; +using System.Net.Http.Headers; using Bit.Billing.Services; using Bit.Billing.Services.Implementations; using Bit.Core.Billing.Extensions; @@ -34,6 +35,7 @@ public class Startup // Settings var globalSettings = services.AddGlobalSettingsServices(Configuration, Environment); services.Configure(Configuration.GetSection("BillingSettings")); + var billingSettings = Configuration.GetSection("BillingSettings").Get(); // Stripe Billing StripeConfiguration.ApiKey = globalSettings.Stripe.ApiKey; @@ -97,6 +99,10 @@ public class Startup // Set up HttpClients services.AddHttpClient("FreshdeskApi"); + services.AddHttpClient("OnyxApi", client => + { + client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", billingSettings.Onyx.ApiKey); + }); services.AddScoped(); services.AddScoped(); @@ -112,6 +118,10 @@ public class Startup // Jobs service Jobs.JobsHostedService.AddJobsServices(services); services.AddHostedService(); + + // Swagger + services.AddEndpointsApiExplorer(); + services.AddSwaggerGen(); } public void Configure( @@ -128,6 +138,11 @@ public class Startup if (env.IsDevelopment()) { app.UseDeveloperExceptionPage(); + app.UseSwagger(); + app.UseSwaggerUI(c => + { + c.SwaggerEndpoint("/swagger/v1/swagger.json", "Billing API V1"); + }); } app.UseStaticFiles(); diff --git a/src/Billing/appsettings.json b/src/Billing/appsettings.json index 84a67434f5..2a2864b246 100644 --- a/src/Billing/appsettings.json +++ b/src/Billing/appsettings.json @@ -73,6 +73,10 @@ "region": "US", "userFieldName": "cf_user", "orgFieldName": "cf_org" - } + }, + "onyx": { + "apiKey": "SECRET", + "baseUrl": "https://cloud.onyx.app/api" + } } } diff --git a/test/Billing.Test/Controllers/FreshdeskControllerTests.cs b/test/Billing.Test/Controllers/FreshdeskControllerTests.cs index f07c64dad9..26ce310b9c 100644 --- a/test/Billing.Test/Controllers/FreshdeskControllerTests.cs +++ b/test/Billing.Test/Controllers/FreshdeskControllerTests.cs @@ -1,4 +1,5 @@ -using Bit.Billing.Controllers; +using System.Text.Json; +using Bit.Billing.Controllers; using Bit.Billing.Models; using Bit.Core.AdminConsole.Entities; using Bit.Core.Entities; @@ -70,6 +71,159 @@ public class FreshdeskControllerTests _ = mockHttpMessageHandler.Received(1).Send(Arg.Is(m => m.Method == HttpMethod.Post && m.RequestUri.ToString().EndsWith($"{model.TicketId}/notes")), Arg.Any()); } + [Theory] + [BitAutoData((string)null, null)] + [BitAutoData((string)null)] + [BitAutoData(WebhookKey, null)] + public async Task PostWebhookOnyxAi_InvalidWebhookKey_results_in_BadRequest( + string freshdeskWebhookKey, FreshdeskWebhookModel model, + BillingSettings billingSettings, SutProvider sutProvider) + { + sutProvider.GetDependency>() + .Value.FreshDesk.WebhookKey.Returns(billingSettings.FreshDesk.WebhookKey); + + var response = await sutProvider.Sut.PostWebhookOnyxAi(freshdeskWebhookKey, model); + + var statusCodeResult = Assert.IsAssignableFrom(response); + Assert.Equal(StatusCodes.Status400BadRequest, statusCodeResult.StatusCode); + } + + [Theory] + [BitAutoData(WebhookKey)] + public async Task PostWebhookOnyxAi_invalid_ticketid_results_in_BadRequest( + string freshdeskWebhookKey, FreshdeskWebhookModel model, SutProvider sutProvider) + { + sutProvider.GetDependency>() + .Value.FreshDesk.WebhookKey.Returns(freshdeskWebhookKey); + + var mockHttpMessageHandler = Substitute.ForPartsOf(); + var mockResponse = new HttpResponseMessage(System.Net.HttpStatusCode.BadRequest); + mockHttpMessageHandler.Send(Arg.Any(), Arg.Any()) + .Returns(mockResponse); + var httpClient = new HttpClient(mockHttpMessageHandler); + + sutProvider.GetDependency().CreateClient("FreshdeskApi").Returns(httpClient); + + var response = await sutProvider.Sut.PostWebhookOnyxAi(freshdeskWebhookKey, model); + + var result = Assert.IsAssignableFrom(response); + Assert.Equal(StatusCodes.Status400BadRequest, result.StatusCode); + } + + [Theory] + [BitAutoData(WebhookKey)] + public async Task PostWebhookOnyxAi_invalid_freshdesk_response_results_in_BadRequest( + string freshdeskWebhookKey, FreshdeskWebhookModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency>() + .Value.FreshDesk.WebhookKey.Returns(freshdeskWebhookKey); + + var mockHttpMessageHandler = Substitute.ForPartsOf(); + var mockResponse = new HttpResponseMessage(System.Net.HttpStatusCode.OK) + { + Content = new StringContent("non json content. expect json deserializer to throw error") + }; + mockHttpMessageHandler.Send(Arg.Any(), Arg.Any()) + .Returns(mockResponse); + var httpClient = new HttpClient(mockHttpMessageHandler); + + sutProvider.GetDependency().CreateClient("FreshdeskApi").Returns(httpClient); + + var response = await sutProvider.Sut.PostWebhookOnyxAi(freshdeskWebhookKey, model); + + var result = Assert.IsAssignableFrom(response); + Assert.Equal(StatusCodes.Status400BadRequest, result.StatusCode); + } + + [Theory] + [BitAutoData(WebhookKey)] + public async Task PostWebhookOnyxAi_invalid_onyx_response_results_in_BadRequest( + string freshdeskWebhookKey, FreshdeskWebhookModel model, + FreshdeskViewTicketModel freshdeskTicketInfo, SutProvider sutProvider) + { + var billingSettings = sutProvider.GetDependency>().Value; + billingSettings.FreshDesk.WebhookKey.Returns(freshdeskWebhookKey); + billingSettings.Onyx.BaseUrl.Returns("http://simulate-onyx-api.com/api"); + + // mocking freshdesk Api request for ticket info + var mockFreshdeskHttpMessageHandler = Substitute.ForPartsOf(); + var mockFreshdeskResponse = new HttpResponseMessage(System.Net.HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(freshdeskTicketInfo)) + }; + mockFreshdeskHttpMessageHandler.Send(Arg.Any(), Arg.Any()) + .Returns(mockFreshdeskResponse); + var freshdeskHttpClient = new HttpClient(mockFreshdeskHttpMessageHandler); + + // mocking Onyx api response given a ticket description + var mockOnyxHttpMessageHandler = Substitute.ForPartsOf(); + var mockOnyxResponse = new HttpResponseMessage(System.Net.HttpStatusCode.BadRequest); + mockOnyxHttpMessageHandler.Send(Arg.Any(), Arg.Any()) + .Returns(mockOnyxResponse); + var onyxHttpClient = new HttpClient(mockOnyxHttpMessageHandler); + + sutProvider.GetDependency().CreateClient("FreshdeskApi").Returns(freshdeskHttpClient); + sutProvider.GetDependency().CreateClient("OnyxApi").Returns(onyxHttpClient); + + var response = await sutProvider.Sut.PostWebhookOnyxAi(freshdeskWebhookKey, model); + + var result = Assert.IsAssignableFrom(response); + Assert.Equal(StatusCodes.Status400BadRequest, result.StatusCode); + } + + [Theory] + [BitAutoData(WebhookKey)] + public async Task PostWebhookOnyxAi_success( + string freshdeskWebhookKey, FreshdeskWebhookModel model, + FreshdeskViewTicketModel freshdeskTicketInfo, + OnyxAnswerWithCitationResponseModel onyxResponse, + SutProvider sutProvider) + { + var billingSettings = sutProvider.GetDependency>().Value; + billingSettings.FreshDesk.WebhookKey.Returns(freshdeskWebhookKey); + billingSettings.Onyx.BaseUrl.Returns("http://simulate-onyx-api.com/api"); + + // mocking freshdesk Api request for ticket info (GET) + var mockFreshdeskHttpMessageHandler = Substitute.ForPartsOf(); + var mockFreshdeskResponse = new HttpResponseMessage(System.Net.HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(freshdeskTicketInfo)) + }; + mockFreshdeskHttpMessageHandler.Send( + Arg.Is(_ => _.Method == HttpMethod.Get), + Arg.Any()) + .Returns(mockFreshdeskResponse); + + // mocking freshdesk api add note request (POST) + var mockFreshdeskAddNoteResponse = new HttpResponseMessage(System.Net.HttpStatusCode.BadRequest); + mockFreshdeskHttpMessageHandler.Send( + Arg.Is(_ => _.Method == HttpMethod.Post), + Arg.Any()) + .Returns(mockFreshdeskAddNoteResponse); + var freshdeskHttpClient = new HttpClient(mockFreshdeskHttpMessageHandler); + + + // mocking Onyx api response given a ticket description + var mockOnyxHttpMessageHandler = Substitute.ForPartsOf(); + onyxResponse.ErrorMsg = string.Empty; + var mockOnyxResponse = new HttpResponseMessage(System.Net.HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(onyxResponse)) + }; + mockOnyxHttpMessageHandler.Send(Arg.Any(), Arg.Any()) + .Returns(mockOnyxResponse); + var onyxHttpClient = new HttpClient(mockOnyxHttpMessageHandler); + + sutProvider.GetDependency().CreateClient("FreshdeskApi").Returns(freshdeskHttpClient); + sutProvider.GetDependency().CreateClient("OnyxApi").Returns(onyxHttpClient); + + var response = await sutProvider.Sut.PostWebhookOnyxAi(freshdeskWebhookKey, model); + + var result = Assert.IsAssignableFrom(response); + Assert.Equal(StatusCodes.Status200OK, result.StatusCode); + } + public class MockHttpMessageHandler : HttpMessageHandler { protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) From bc27ec2b9b800c7dbb5ffd9e6454d44df144ef3c Mon Sep 17 00:00:00 2001 From: Jonas Hendrickx Date: Thu, 6 Feb 2025 15:15:36 +0100 Subject: [PATCH 14/25] =?UTF-8?q?[PM-12765]=20Change=20error=20message=20w?= =?UTF-8?q?hen=20subscription=20canceled=20and=20attemp=E2=80=A6=20(#5346)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Services/Implementations/OrganizationService.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 6194aea6c7..6f4aba4882 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -1294,6 +1294,12 @@ public class OrganizationService : IOrganizationService } } + var subscription = await _paymentService.GetSubscriptionAsync(organization); + if (subscription?.Subscription?.Status == StripeConstants.SubscriptionStatus.Canceled) + { + return (false, "You do not have an active subscription. Reinstate your subscription to make changes"); + } + if (organization.Seats.HasValue && organization.MaxAutoscaleSeats.HasValue && organization.MaxAutoscaleSeats.Value < organization.Seats.Value + seatsToAdd) @@ -1301,12 +1307,6 @@ public class OrganizationService : IOrganizationService return (false, $"Seat limit has been reached."); } - var subscription = await _paymentService.GetSubscriptionAsync(organization); - if (subscription?.Subscription?.Status == StripeConstants.SubscriptionStatus.Canceled) - { - return (false, "Cannot autoscale with a canceled subscription."); - } - return (true, failureReason); } From 678d5d5d632447ac3431781d8232971eab713edc Mon Sep 17 00:00:00 2001 From: Jonas Hendrickx Date: Thu, 6 Feb 2025 16:34:22 +0100 Subject: [PATCH 15/25] [PM-18028] Attempting to enable automatic tax on customer with invalid location (#5374) --- .../Implementations/UpcomingInvoiceHandler.cs | 13 +++--- .../Billing/Extensions/CustomerExtensions.cs | 16 +++++++ .../SubscriptionCreateOptionsExtensions.cs | 26 +++++++++++ .../SubscriptionUpdateOptionsExtensions.cs | 35 +++++++++++++++ .../UpcomingInvoiceOptionsExtensions.cs | 35 +++++++++++++++ .../PremiumUserBillingService.cs | 2 +- .../Implementations/SubscriberService.cs | 20 ++++++--- .../Implementations/StripePaymentService.cs | 43 +++++++++---------- 8 files changed, 155 insertions(+), 35 deletions(-) create mode 100644 src/Core/Billing/Extensions/CustomerExtensions.cs create mode 100644 src/Core/Billing/Extensions/SubscriptionCreateOptionsExtensions.cs create mode 100644 src/Core/Billing/Extensions/SubscriptionUpdateOptionsExtensions.cs create mode 100644 src/Core/Billing/Extensions/UpcomingInvoiceOptionsExtensions.cs diff --git a/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs b/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs index c52c03b6aa..409bd0d18b 100644 --- a/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs +++ b/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs @@ -1,6 +1,7 @@ using Bit.Billing.Constants; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Billing.Extensions; using Bit.Core.OrganizationFeatures.OrganizationSponsorships.FamiliesForEnterprise.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; @@ -160,16 +161,16 @@ public class UpcomingInvoiceHandler : IUpcomingInvoiceHandler private async Task TryEnableAutomaticTaxAsync(Subscription subscription) { - if (subscription.AutomaticTax.Enabled) + var customerGetOptions = new CustomerGetOptions { Expand = ["tax"] }; + var customer = await _stripeFacade.GetCustomer(subscription.CustomerId, customerGetOptions); + + var subscriptionUpdateOptions = new SubscriptionUpdateOptions(); + + if (!subscriptionUpdateOptions.EnableAutomaticTax(customer, subscription)) { return subscription; } - var subscriptionUpdateOptions = new SubscriptionUpdateOptions - { - AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true } - }; - return await _stripeFacade.UpdateSubscription(subscription.Id, subscriptionUpdateOptions); } diff --git a/src/Core/Billing/Extensions/CustomerExtensions.cs b/src/Core/Billing/Extensions/CustomerExtensions.cs new file mode 100644 index 0000000000..62f1a5055c --- /dev/null +++ b/src/Core/Billing/Extensions/CustomerExtensions.cs @@ -0,0 +1,16 @@ +using Bit.Core.Billing.Constants; +using Stripe; + +namespace Bit.Core.Billing.Extensions; + +public static class CustomerExtensions +{ + + /// + /// Determines if a Stripe customer supports automatic tax + /// + /// + /// + public static bool HasTaxLocationVerified(this Customer customer) => + customer?.Tax?.AutomaticTax == StripeConstants.AutomaticTaxStatus.Supported; +} diff --git a/src/Core/Billing/Extensions/SubscriptionCreateOptionsExtensions.cs b/src/Core/Billing/Extensions/SubscriptionCreateOptionsExtensions.cs new file mode 100644 index 0000000000..d76a0553a3 --- /dev/null +++ b/src/Core/Billing/Extensions/SubscriptionCreateOptionsExtensions.cs @@ -0,0 +1,26 @@ +using Stripe; + +namespace Bit.Core.Billing.Extensions; + +public static class SubscriptionCreateOptionsExtensions +{ + /// + /// Attempts to enable automatic tax for given new subscription options. + /// + /// + /// The existing customer. + /// Returns true when successful, false when conditions are not met. + public static bool EnableAutomaticTax(this SubscriptionCreateOptions options, Customer customer) + { + // We might only need to check the automatic tax status. + if (!customer.HasTaxLocationVerified() && string.IsNullOrWhiteSpace(customer.Address?.Country)) + { + return false; + } + + options.DefaultTaxRates = []; + options.AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }; + + return true; + } +} diff --git a/src/Core/Billing/Extensions/SubscriptionUpdateOptionsExtensions.cs b/src/Core/Billing/Extensions/SubscriptionUpdateOptionsExtensions.cs new file mode 100644 index 0000000000..d70af78fa8 --- /dev/null +++ b/src/Core/Billing/Extensions/SubscriptionUpdateOptionsExtensions.cs @@ -0,0 +1,35 @@ +using Stripe; + +namespace Bit.Core.Billing.Extensions; + +public static class SubscriptionUpdateOptionsExtensions +{ + /// + /// Attempts to enable automatic tax for given subscription options. + /// + /// + /// The existing customer to which the subscription belongs. + /// The existing subscription. + /// Returns true when successful, false when conditions are not met. + public static bool EnableAutomaticTax( + this SubscriptionUpdateOptions options, + Customer customer, + Subscription subscription) + { + if (subscription.AutomaticTax.Enabled) + { + return false; + } + + // We might only need to check the automatic tax status. + if (!customer.HasTaxLocationVerified() && string.IsNullOrWhiteSpace(customer.Address?.Country)) + { + return false; + } + + options.DefaultTaxRates = []; + options.AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }; + + return true; + } +} diff --git a/src/Core/Billing/Extensions/UpcomingInvoiceOptionsExtensions.cs b/src/Core/Billing/Extensions/UpcomingInvoiceOptionsExtensions.cs new file mode 100644 index 0000000000..88df5638c9 --- /dev/null +++ b/src/Core/Billing/Extensions/UpcomingInvoiceOptionsExtensions.cs @@ -0,0 +1,35 @@ +using Stripe; + +namespace Bit.Core.Billing.Extensions; + +public static class UpcomingInvoiceOptionsExtensions +{ + /// + /// Attempts to enable automatic tax for given upcoming invoice options. + /// + /// + /// The existing customer to which the upcoming invoice belongs. + /// The existing subscription to which the upcoming invoice belongs. + /// Returns true when successful, false when conditions are not met. + public static bool EnableAutomaticTax( + this UpcomingInvoiceOptions options, + Customer customer, + Subscription subscription) + { + if (subscription != null && subscription.AutomaticTax.Enabled) + { + return false; + } + + // We might only need to check the automatic tax status. + if (!customer.HasTaxLocationVerified() && string.IsNullOrWhiteSpace(customer.Address?.Country)) + { + return false; + } + + options.AutomaticTax = new InvoiceAutomaticTaxOptions { Enabled = true }; + options.SubscriptionDefaultTaxRates = []; + + return true; + } +} diff --git a/src/Core/Billing/Services/Implementations/PremiumUserBillingService.cs b/src/Core/Billing/Services/Implementations/PremiumUserBillingService.cs index ed841c9576..99815c0557 100644 --- a/src/Core/Billing/Services/Implementations/PremiumUserBillingService.cs +++ b/src/Core/Billing/Services/Implementations/PremiumUserBillingService.cs @@ -258,7 +258,7 @@ public class PremiumUserBillingService( { AutomaticTax = new SubscriptionAutomaticTaxOptions { - Enabled = true + Enabled = customer.Tax?.AutomaticTax == StripeConstants.AutomaticTaxStatus.Supported, }, CollectionMethod = StripeConstants.CollectionMethod.ChargeAutomatically, Customer = customer.Id, diff --git a/src/Core/Billing/Services/Implementations/SubscriberService.cs b/src/Core/Billing/Services/Implementations/SubscriberService.cs index f4cf22ac19..b2dca19e80 100644 --- a/src/Core/Billing/Services/Implementations/SubscriberService.cs +++ b/src/Core/Billing/Services/Implementations/SubscriberService.cs @@ -661,11 +661,21 @@ public class SubscriberService( } } - await stripeAdapter.SubscriptionUpdateAsync(subscriber.GatewaySubscriptionId, - new SubscriptionUpdateOptions - { - AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }, - }); + if (SubscriberIsEligibleForAutomaticTax(subscriber, customer)) + { + await stripeAdapter.SubscriptionUpdateAsync(subscriber.GatewaySubscriptionId, + new SubscriptionUpdateOptions + { + AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true } + }); + } + + return; + + bool SubscriberIsEligibleForAutomaticTax(ISubscriber localSubscriber, Customer localCustomer) + => !string.IsNullOrEmpty(localSubscriber.GatewaySubscriptionId) && + (localCustomer.Subscriptions?.Any(sub => sub.Id == localSubscriber.GatewaySubscriptionId && !sub.AutomaticTax.Enabled) ?? false) && + localCustomer.Tax?.AutomaticTax == StripeConstants.AutomaticTaxStatus.Supported; } public async Task VerifyBankAccount( diff --git a/src/Core/Services/Implementations/StripePaymentService.cs b/src/Core/Services/Implementations/StripePaymentService.cs index fb5c7364a5..553c1c65a8 100644 --- a/src/Core/Services/Implementations/StripePaymentService.cs +++ b/src/Core/Services/Implementations/StripePaymentService.cs @@ -177,7 +177,7 @@ public class StripePaymentService : IPaymentService customer = await _stripeAdapter.CustomerCreateAsync(customerCreateOptions); subCreateOptions.AddExpand("latest_invoice.payment_intent"); subCreateOptions.Customer = customer.Id; - subCreateOptions.AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }; + subCreateOptions.EnableAutomaticTax(customer); subscription = await _stripeAdapter.SubscriptionCreateAsync(subCreateOptions); if (subscription.Status == "incomplete" && subscription.LatestInvoice?.PaymentIntent != null) @@ -358,10 +358,9 @@ public class StripePaymentService : IPaymentService customer = await _stripeAdapter.CustomerUpdateAsync(org.GatewayCustomerId, customerUpdateOptions); } - var subCreateOptions = new OrganizationUpgradeSubscriptionOptions(customer.Id, org, plan, upgrade) - { - AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true } - }; + var subCreateOptions = new OrganizationUpgradeSubscriptionOptions(customer.Id, org, plan, upgrade); + + subCreateOptions.EnableAutomaticTax(customer); var (stripePaymentMethod, paymentMethodType) = IdentifyPaymentMethod(customer, subCreateOptions); @@ -520,10 +519,6 @@ public class StripePaymentService : IPaymentService var customerCreateOptions = new CustomerCreateOptions { - Tax = new CustomerTaxOptions - { - ValidateLocation = StripeConstants.ValidateTaxLocationTiming.Immediately - }, Description = user.Name, Email = user.Email, Metadata = stripeCustomerMetadata, @@ -561,7 +556,6 @@ public class StripePaymentService : IPaymentService var subCreateOptions = new SubscriptionCreateOptions { - AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }, Customer = customer.Id, Items = [], Metadata = new Dictionary @@ -581,10 +575,12 @@ public class StripePaymentService : IPaymentService subCreateOptions.Items.Add(new SubscriptionItemOptions { Plan = StoragePlanId, - Quantity = additionalStorageGb, + Quantity = additionalStorageGb }); } + subCreateOptions.EnableAutomaticTax(customer); + var subscription = await ChargeForNewSubscriptionAsync(user, customer, createdStripeCustomer, stripePaymentMethod, paymentMethodType, subCreateOptions, braintreeCustomer); @@ -622,7 +618,10 @@ public class StripePaymentService : IPaymentService SubscriptionItems = ToInvoiceSubscriptionItemOptions(subCreateOptions.Items) }); - previewInvoice.AutomaticTax = new InvoiceAutomaticTax { Enabled = true }; + if (customer.HasTaxLocationVerified()) + { + previewInvoice.AutomaticTax = new InvoiceAutomaticTax { Enabled = true }; + } if (previewInvoice.AmountDue > 0) { @@ -680,12 +679,10 @@ public class StripePaymentService : IPaymentService Customer = customer.Id, SubscriptionItems = ToInvoiceSubscriptionItemOptions(subCreateOptions.Items), SubscriptionDefaultTaxRates = subCreateOptions.DefaultTaxRates, - AutomaticTax = new InvoiceAutomaticTaxOptions - { - Enabled = true - } }; + upcomingInvoiceOptions.EnableAutomaticTax(customer, null); + var previewInvoice = await _stripeAdapter.InvoiceUpcomingAsync(upcomingInvoiceOptions); if (previewInvoice.AmountDue > 0) @@ -804,11 +801,7 @@ public class StripePaymentService : IPaymentService Items = updatedItemOptions, ProrationBehavior = invoiceNow ? Constants.AlwaysInvoice : Constants.CreateProrations, DaysUntilDue = daysUntilDue ?? 1, - CollectionMethod = "send_invoice", - AutomaticTax = new SubscriptionAutomaticTaxOptions - { - Enabled = true - } + CollectionMethod = "send_invoice" }; if (!invoiceNow && isAnnualPlan && sub.Status.Trim() != "trialing") { @@ -816,6 +809,8 @@ public class StripePaymentService : IPaymentService new SubscriptionPendingInvoiceItemIntervalOptions { Interval = "month" }; } + subUpdateOptions.EnableAutomaticTax(sub.Customer, sub); + if (!subscriptionUpdate.UpdateNeeded(sub)) { // No need to update subscription, quantity matches @@ -1500,11 +1495,13 @@ public class StripePaymentService : IPaymentService if (!string.IsNullOrEmpty(subscriber.GatewaySubscriptionId) && customer.Subscriptions.Any(sub => sub.Id == subscriber.GatewaySubscriptionId && - !sub.AutomaticTax.Enabled)) + !sub.AutomaticTax.Enabled) && + customer.HasTaxLocationVerified()) { var subscriptionUpdateOptions = new SubscriptionUpdateOptions { - AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true } + AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }, + DefaultTaxRates = [] }; _ = await _stripeAdapter.SubscriptionUpdateAsync( From a1ef07ea69ad39a92931ada2c3c8963b18237020 Mon Sep 17 00:00:00 2001 From: Jonas Hendrickx Date: Thu, 6 Feb 2025 17:11:20 +0100 Subject: [PATCH 16/25] =?UTF-8?q?Revert=20"[PM-18028]=20Attempting=20to=20?= =?UTF-8?q?enable=20automatic=20tax=20on=20customer=20with=20invali?= =?UTF-8?q?=E2=80=A6"=20(#5375)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 678d5d5d632447ac3431781d8232971eab713edc. --- .../Implementations/UpcomingInvoiceHandler.cs | 13 +++--- .../Billing/Extensions/CustomerExtensions.cs | 16 ------- .../SubscriptionCreateOptionsExtensions.cs | 26 ----------- .../SubscriptionUpdateOptionsExtensions.cs | 35 --------------- .../UpcomingInvoiceOptionsExtensions.cs | 35 --------------- .../PremiumUserBillingService.cs | 2 +- .../Implementations/SubscriberService.cs | 20 +++------ .../Implementations/StripePaymentService.cs | 43 ++++++++++--------- 8 files changed, 35 insertions(+), 155 deletions(-) delete mode 100644 src/Core/Billing/Extensions/CustomerExtensions.cs delete mode 100644 src/Core/Billing/Extensions/SubscriptionCreateOptionsExtensions.cs delete mode 100644 src/Core/Billing/Extensions/SubscriptionUpdateOptionsExtensions.cs delete mode 100644 src/Core/Billing/Extensions/UpcomingInvoiceOptionsExtensions.cs diff --git a/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs b/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs index 409bd0d18b..c52c03b6aa 100644 --- a/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs +++ b/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs @@ -1,7 +1,6 @@ using Bit.Billing.Constants; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.Billing.Extensions; using Bit.Core.OrganizationFeatures.OrganizationSponsorships.FamiliesForEnterprise.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; @@ -161,16 +160,16 @@ public class UpcomingInvoiceHandler : IUpcomingInvoiceHandler private async Task TryEnableAutomaticTaxAsync(Subscription subscription) { - var customerGetOptions = new CustomerGetOptions { Expand = ["tax"] }; - var customer = await _stripeFacade.GetCustomer(subscription.CustomerId, customerGetOptions); - - var subscriptionUpdateOptions = new SubscriptionUpdateOptions(); - - if (!subscriptionUpdateOptions.EnableAutomaticTax(customer, subscription)) + if (subscription.AutomaticTax.Enabled) { return subscription; } + var subscriptionUpdateOptions = new SubscriptionUpdateOptions + { + AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true } + }; + return await _stripeFacade.UpdateSubscription(subscription.Id, subscriptionUpdateOptions); } diff --git a/src/Core/Billing/Extensions/CustomerExtensions.cs b/src/Core/Billing/Extensions/CustomerExtensions.cs deleted file mode 100644 index 62f1a5055c..0000000000 --- a/src/Core/Billing/Extensions/CustomerExtensions.cs +++ /dev/null @@ -1,16 +0,0 @@ -using Bit.Core.Billing.Constants; -using Stripe; - -namespace Bit.Core.Billing.Extensions; - -public static class CustomerExtensions -{ - - /// - /// Determines if a Stripe customer supports automatic tax - /// - /// - /// - public static bool HasTaxLocationVerified(this Customer customer) => - customer?.Tax?.AutomaticTax == StripeConstants.AutomaticTaxStatus.Supported; -} diff --git a/src/Core/Billing/Extensions/SubscriptionCreateOptionsExtensions.cs b/src/Core/Billing/Extensions/SubscriptionCreateOptionsExtensions.cs deleted file mode 100644 index d76a0553a3..0000000000 --- a/src/Core/Billing/Extensions/SubscriptionCreateOptionsExtensions.cs +++ /dev/null @@ -1,26 +0,0 @@ -using Stripe; - -namespace Bit.Core.Billing.Extensions; - -public static class SubscriptionCreateOptionsExtensions -{ - /// - /// Attempts to enable automatic tax for given new subscription options. - /// - /// - /// The existing customer. - /// Returns true when successful, false when conditions are not met. - public static bool EnableAutomaticTax(this SubscriptionCreateOptions options, Customer customer) - { - // We might only need to check the automatic tax status. - if (!customer.HasTaxLocationVerified() && string.IsNullOrWhiteSpace(customer.Address?.Country)) - { - return false; - } - - options.DefaultTaxRates = []; - options.AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }; - - return true; - } -} diff --git a/src/Core/Billing/Extensions/SubscriptionUpdateOptionsExtensions.cs b/src/Core/Billing/Extensions/SubscriptionUpdateOptionsExtensions.cs deleted file mode 100644 index d70af78fa8..0000000000 --- a/src/Core/Billing/Extensions/SubscriptionUpdateOptionsExtensions.cs +++ /dev/null @@ -1,35 +0,0 @@ -using Stripe; - -namespace Bit.Core.Billing.Extensions; - -public static class SubscriptionUpdateOptionsExtensions -{ - /// - /// Attempts to enable automatic tax for given subscription options. - /// - /// - /// The existing customer to which the subscription belongs. - /// The existing subscription. - /// Returns true when successful, false when conditions are not met. - public static bool EnableAutomaticTax( - this SubscriptionUpdateOptions options, - Customer customer, - Subscription subscription) - { - if (subscription.AutomaticTax.Enabled) - { - return false; - } - - // We might only need to check the automatic tax status. - if (!customer.HasTaxLocationVerified() && string.IsNullOrWhiteSpace(customer.Address?.Country)) - { - return false; - } - - options.DefaultTaxRates = []; - options.AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }; - - return true; - } -} diff --git a/src/Core/Billing/Extensions/UpcomingInvoiceOptionsExtensions.cs b/src/Core/Billing/Extensions/UpcomingInvoiceOptionsExtensions.cs deleted file mode 100644 index 88df5638c9..0000000000 --- a/src/Core/Billing/Extensions/UpcomingInvoiceOptionsExtensions.cs +++ /dev/null @@ -1,35 +0,0 @@ -using Stripe; - -namespace Bit.Core.Billing.Extensions; - -public static class UpcomingInvoiceOptionsExtensions -{ - /// - /// Attempts to enable automatic tax for given upcoming invoice options. - /// - /// - /// The existing customer to which the upcoming invoice belongs. - /// The existing subscription to which the upcoming invoice belongs. - /// Returns true when successful, false when conditions are not met. - public static bool EnableAutomaticTax( - this UpcomingInvoiceOptions options, - Customer customer, - Subscription subscription) - { - if (subscription != null && subscription.AutomaticTax.Enabled) - { - return false; - } - - // We might only need to check the automatic tax status. - if (!customer.HasTaxLocationVerified() && string.IsNullOrWhiteSpace(customer.Address?.Country)) - { - return false; - } - - options.AutomaticTax = new InvoiceAutomaticTaxOptions { Enabled = true }; - options.SubscriptionDefaultTaxRates = []; - - return true; - } -} diff --git a/src/Core/Billing/Services/Implementations/PremiumUserBillingService.cs b/src/Core/Billing/Services/Implementations/PremiumUserBillingService.cs index 99815c0557..ed841c9576 100644 --- a/src/Core/Billing/Services/Implementations/PremiumUserBillingService.cs +++ b/src/Core/Billing/Services/Implementations/PremiumUserBillingService.cs @@ -258,7 +258,7 @@ public class PremiumUserBillingService( { AutomaticTax = new SubscriptionAutomaticTaxOptions { - Enabled = customer.Tax?.AutomaticTax == StripeConstants.AutomaticTaxStatus.Supported, + Enabled = true }, CollectionMethod = StripeConstants.CollectionMethod.ChargeAutomatically, Customer = customer.Id, diff --git a/src/Core/Billing/Services/Implementations/SubscriberService.cs b/src/Core/Billing/Services/Implementations/SubscriberService.cs index b2dca19e80..f4cf22ac19 100644 --- a/src/Core/Billing/Services/Implementations/SubscriberService.cs +++ b/src/Core/Billing/Services/Implementations/SubscriberService.cs @@ -661,21 +661,11 @@ public class SubscriberService( } } - if (SubscriberIsEligibleForAutomaticTax(subscriber, customer)) - { - await stripeAdapter.SubscriptionUpdateAsync(subscriber.GatewaySubscriptionId, - new SubscriptionUpdateOptions - { - AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true } - }); - } - - return; - - bool SubscriberIsEligibleForAutomaticTax(ISubscriber localSubscriber, Customer localCustomer) - => !string.IsNullOrEmpty(localSubscriber.GatewaySubscriptionId) && - (localCustomer.Subscriptions?.Any(sub => sub.Id == localSubscriber.GatewaySubscriptionId && !sub.AutomaticTax.Enabled) ?? false) && - localCustomer.Tax?.AutomaticTax == StripeConstants.AutomaticTaxStatus.Supported; + await stripeAdapter.SubscriptionUpdateAsync(subscriber.GatewaySubscriptionId, + new SubscriptionUpdateOptions + { + AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }, + }); } public async Task VerifyBankAccount( diff --git a/src/Core/Services/Implementations/StripePaymentService.cs b/src/Core/Services/Implementations/StripePaymentService.cs index 553c1c65a8..fb5c7364a5 100644 --- a/src/Core/Services/Implementations/StripePaymentService.cs +++ b/src/Core/Services/Implementations/StripePaymentService.cs @@ -177,7 +177,7 @@ public class StripePaymentService : IPaymentService customer = await _stripeAdapter.CustomerCreateAsync(customerCreateOptions); subCreateOptions.AddExpand("latest_invoice.payment_intent"); subCreateOptions.Customer = customer.Id; - subCreateOptions.EnableAutomaticTax(customer); + subCreateOptions.AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }; subscription = await _stripeAdapter.SubscriptionCreateAsync(subCreateOptions); if (subscription.Status == "incomplete" && subscription.LatestInvoice?.PaymentIntent != null) @@ -358,9 +358,10 @@ public class StripePaymentService : IPaymentService customer = await _stripeAdapter.CustomerUpdateAsync(org.GatewayCustomerId, customerUpdateOptions); } - var subCreateOptions = new OrganizationUpgradeSubscriptionOptions(customer.Id, org, plan, upgrade); - - subCreateOptions.EnableAutomaticTax(customer); + var subCreateOptions = new OrganizationUpgradeSubscriptionOptions(customer.Id, org, plan, upgrade) + { + AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true } + }; var (stripePaymentMethod, paymentMethodType) = IdentifyPaymentMethod(customer, subCreateOptions); @@ -519,6 +520,10 @@ public class StripePaymentService : IPaymentService var customerCreateOptions = new CustomerCreateOptions { + Tax = new CustomerTaxOptions + { + ValidateLocation = StripeConstants.ValidateTaxLocationTiming.Immediately + }, Description = user.Name, Email = user.Email, Metadata = stripeCustomerMetadata, @@ -556,6 +561,7 @@ public class StripePaymentService : IPaymentService var subCreateOptions = new SubscriptionCreateOptions { + AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }, Customer = customer.Id, Items = [], Metadata = new Dictionary @@ -575,12 +581,10 @@ public class StripePaymentService : IPaymentService subCreateOptions.Items.Add(new SubscriptionItemOptions { Plan = StoragePlanId, - Quantity = additionalStorageGb + Quantity = additionalStorageGb, }); } - subCreateOptions.EnableAutomaticTax(customer); - var subscription = await ChargeForNewSubscriptionAsync(user, customer, createdStripeCustomer, stripePaymentMethod, paymentMethodType, subCreateOptions, braintreeCustomer); @@ -618,10 +622,7 @@ public class StripePaymentService : IPaymentService SubscriptionItems = ToInvoiceSubscriptionItemOptions(subCreateOptions.Items) }); - if (customer.HasTaxLocationVerified()) - { - previewInvoice.AutomaticTax = new InvoiceAutomaticTax { Enabled = true }; - } + previewInvoice.AutomaticTax = new InvoiceAutomaticTax { Enabled = true }; if (previewInvoice.AmountDue > 0) { @@ -679,10 +680,12 @@ public class StripePaymentService : IPaymentService Customer = customer.Id, SubscriptionItems = ToInvoiceSubscriptionItemOptions(subCreateOptions.Items), SubscriptionDefaultTaxRates = subCreateOptions.DefaultTaxRates, + AutomaticTax = new InvoiceAutomaticTaxOptions + { + Enabled = true + } }; - upcomingInvoiceOptions.EnableAutomaticTax(customer, null); - var previewInvoice = await _stripeAdapter.InvoiceUpcomingAsync(upcomingInvoiceOptions); if (previewInvoice.AmountDue > 0) @@ -801,7 +804,11 @@ public class StripePaymentService : IPaymentService Items = updatedItemOptions, ProrationBehavior = invoiceNow ? Constants.AlwaysInvoice : Constants.CreateProrations, DaysUntilDue = daysUntilDue ?? 1, - CollectionMethod = "send_invoice" + CollectionMethod = "send_invoice", + AutomaticTax = new SubscriptionAutomaticTaxOptions + { + Enabled = true + } }; if (!invoiceNow && isAnnualPlan && sub.Status.Trim() != "trialing") { @@ -809,8 +816,6 @@ public class StripePaymentService : IPaymentService new SubscriptionPendingInvoiceItemIntervalOptions { Interval = "month" }; } - subUpdateOptions.EnableAutomaticTax(sub.Customer, sub); - if (!subscriptionUpdate.UpdateNeeded(sub)) { // No need to update subscription, quantity matches @@ -1495,13 +1500,11 @@ public class StripePaymentService : IPaymentService if (!string.IsNullOrEmpty(subscriber.GatewaySubscriptionId) && customer.Subscriptions.Any(sub => sub.Id == subscriber.GatewaySubscriptionId && - !sub.AutomaticTax.Enabled) && - customer.HasTaxLocationVerified()) + !sub.AutomaticTax.Enabled)) { var subscriptionUpdateOptions = new SubscriptionUpdateOptions { - AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true }, - DefaultTaxRates = [] + AutomaticTax = new SubscriptionAutomaticTaxOptions { Enabled = true } }; _ = await _stripeAdapter.SubscriptionUpdateAsync( From f7d882d760cb8cc1d283d1c1297202f4583f0e09 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:37:15 -0500 Subject: [PATCH 17/25] Remove feature flag from endpoint (#5342) --- src/Api/Auth/Controllers/AccountsController.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 7990a5a18a..3f460a3b90 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -974,7 +974,6 @@ public class AccountsController : Controller await _userService.ResendNewDeviceVerificationEmail(request.Email, request.Secret); } - [RequireFeature(FeatureFlagKeys.NewDeviceVerification)] [HttpPost("verify-devices")] [HttpPut("verify-devices")] public async Task SetUserVerifyDevicesAsync([FromBody] SetVerifyDevicesRequestModel request) From 58d2a7ddaaa6d9e78b0ebca5cdfeb8a4ab3f6121 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 6 Feb 2025 21:38:50 +0100 Subject: [PATCH 18/25] [PM-17210] Prevent unintentionally corrupting private keys (#5285) * Prevent unintentionally corrupting private keys * Deny key update only when replacing existing keys * Fix incorrect use of existing user public/encrypted private key * Fix test * Fix tests * Re-add test * Pass through error for set-password * Fix test * Increase test coverage and simplify checks --- .../Auth/Controllers/AccountsController.cs | 12 +- .../Api/Request/Accounts/KeysRequestModel.cs | 24 +++- .../Controllers/AccountsControllerTests.cs | 127 +++++++++++------- 3 files changed, 108 insertions(+), 55 deletions(-) diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 3f460a3b90..1cd9292386 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -266,8 +266,18 @@ public class AccountsController : Controller throw new UnauthorizedAccessException(); } + try + { + user = model.ToUser(user); + } + catch (Exception e) + { + ModelState.AddModelError(string.Empty, e.Message); + throw new BadRequestException(ModelState); + } + var result = await _setInitialMasterPasswordCommand.SetInitialMasterPasswordAsync( - model.ToUser(user), + user, model.MasterPasswordHash, model.Key, model.OrgIdentifier); diff --git a/src/Core/Auth/Models/Api/Request/Accounts/KeysRequestModel.cs b/src/Core/Auth/Models/Api/Request/Accounts/KeysRequestModel.cs index 93832542de..0964fe1a1d 100644 --- a/src/Core/Auth/Models/Api/Request/Accounts/KeysRequestModel.cs +++ b/src/Core/Auth/Models/Api/Request/Accounts/KeysRequestModel.cs @@ -1,26 +1,36 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Entities; +using Bit.Core.Utilities; namespace Bit.Core.Auth.Models.Api.Request.Accounts; public class KeysRequestModel { + [Required] public string PublicKey { get; set; } [Required] public string EncryptedPrivateKey { get; set; } public User ToUser(User existingUser) { - if (string.IsNullOrWhiteSpace(existingUser.PublicKey) && !string.IsNullOrWhiteSpace(PublicKey)) + if (string.IsNullOrWhiteSpace(PublicKey) || string.IsNullOrWhiteSpace(EncryptedPrivateKey)) + { + throw new InvalidOperationException("Public and private keys are required."); + } + + if (string.IsNullOrWhiteSpace(existingUser.PublicKey) && string.IsNullOrWhiteSpace(existingUser.PrivateKey)) { existingUser.PublicKey = PublicKey; - } - - if (string.IsNullOrWhiteSpace(existingUser.PrivateKey)) - { existingUser.PrivateKey = EncryptedPrivateKey; + return existingUser; + } + else if (PublicKey == existingUser.PublicKey && CoreHelpers.FixedTimeEquals(EncryptedPrivateKey, existingUser.PrivateKey)) + { + return existingUser; + } + else + { + throw new InvalidOperationException("Cannot replace existing key(s) with new key(s)."); } - - return existingUser; } } diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index 1b8c040789..33b7e764d4 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -419,22 +419,32 @@ public class AccountsControllerTests : IDisposable [Theory] - [BitAutoData(true, false)] // User has PublicKey and PrivateKey, and Keys in request are NOT null - [BitAutoData(true, true)] // User has PublicKey and PrivateKey, and Keys in request are null - [BitAutoData(false, false)] // User has neither PublicKey nor PrivateKey, and Keys in request are NOT null - [BitAutoData(false, true)] // User has neither PublicKey nor PrivateKey, and Keys in request are null + [BitAutoData(true, "existingPrivateKey", "existingPublicKey", true)] // allow providing existing keys in the request + [BitAutoData(true, null, null, true)] // allow not setting the public key when the user already has a key + [BitAutoData(false, "newPrivateKey", "newPublicKey", true)] // allow setting new keys when the user has no keys + [BitAutoData(false, null, null, true)] // allow not setting the public key when the user has no keys + // do not allow single key + [BitAutoData(false, "existingPrivateKey", null, false)] + [BitAutoData(false, null, "existingPublicKey", false)] + [BitAutoData(false, "newPrivateKey", null, false)] + [BitAutoData(false, null, "newPublicKey", false)] + [BitAutoData(true, "existingPrivateKey", null, false)] + [BitAutoData(true, null, "existingPublicKey", false)] + [BitAutoData(true, "newPrivateKey", null, false)] + [BitAutoData(true, null, "newPublicKey", false)] + // reject overwriting existing keys + [BitAutoData(true, "newPrivateKey", "newPublicKey", false)] public async Task PostSetPasswordAsync_WhenUserExistsAndSettingPasswordSucceeds_ShouldHandleKeysCorrectlyAndReturn( - bool hasExistingKeys, - bool shouldSetKeysToNull, - User user, - SetPasswordRequestModel setPasswordRequestModel) + bool hasExistingKeys, + string requestPrivateKey, + string requestPublicKey, + bool shouldSucceed, + User user, + SetPasswordRequestModel setPasswordRequestModel) { // Arrange const string existingPublicKey = "existingPublicKey"; - const string existingEncryptedPrivateKey = "existingEncryptedPrivateKey"; - - const string newPublicKey = "newPublicKey"; - const string newEncryptedPrivateKey = "newEncryptedPrivateKey"; + const string existingEncryptedPrivateKey = "existingPrivateKey"; if (hasExistingKeys) { @@ -447,16 +457,16 @@ public class AccountsControllerTests : IDisposable user.PrivateKey = null; } - if (shouldSetKeysToNull) + if (requestPrivateKey == null && requestPublicKey == null) { setPasswordRequestModel.Keys = null; } else { - setPasswordRequestModel.Keys = new KeysRequestModel() + setPasswordRequestModel.Keys = new KeysRequestModel { - PublicKey = newPublicKey, - EncryptedPrivateKey = newEncryptedPrivateKey + EncryptedPrivateKey = requestPrivateKey, + PublicKey = requestPublicKey }; } @@ -469,44 +479,66 @@ public class AccountsControllerTests : IDisposable .Returns(Task.FromResult(IdentityResult.Success)); // Act - await _sut.PostSetPasswordAsync(setPasswordRequestModel); - - // Assert - await _setInitialMasterPasswordCommand.Received(1) - .SetInitialMasterPasswordAsync( - Arg.Is(u => u == user), - Arg.Is(s => s == setPasswordRequestModel.MasterPasswordHash), - Arg.Is(s => s == setPasswordRequestModel.Key), - Arg.Is(s => s == setPasswordRequestModel.OrgIdentifier)); - - // Additional Assertions for User object modifications - Assert.Equal(setPasswordRequestModel.MasterPasswordHint, user.MasterPasswordHint); - Assert.Equal(setPasswordRequestModel.Kdf, user.Kdf); - Assert.Equal(setPasswordRequestModel.KdfIterations, user.KdfIterations); - Assert.Equal(setPasswordRequestModel.KdfMemory, user.KdfMemory); - Assert.Equal(setPasswordRequestModel.KdfParallelism, user.KdfParallelism); - Assert.Equal(setPasswordRequestModel.Key, user.Key); - - if (hasExistingKeys) + if (shouldSucceed) { - // User Keys should not be modified - Assert.Equal(existingPublicKey, user.PublicKey); - Assert.Equal(existingEncryptedPrivateKey, user.PrivateKey); - } - else if (!shouldSetKeysToNull) - { - // User had no keys so they should be set to the request model's keys - Assert.Equal(setPasswordRequestModel.Keys.PublicKey, user.PublicKey); - Assert.Equal(setPasswordRequestModel.Keys.EncryptedPrivateKey, user.PrivateKey); + await _sut.PostSetPasswordAsync(setPasswordRequestModel); + // Assert + await _setInitialMasterPasswordCommand.Received(1) + .SetInitialMasterPasswordAsync( + Arg.Is(u => u == user), + Arg.Is(s => s == setPasswordRequestModel.MasterPasswordHash), + Arg.Is(s => s == setPasswordRequestModel.Key), + Arg.Is(s => s == setPasswordRequestModel.OrgIdentifier)); + + // Additional Assertions for User object modifications + Assert.Equal(setPasswordRequestModel.MasterPasswordHint, user.MasterPasswordHint); + Assert.Equal(setPasswordRequestModel.Kdf, user.Kdf); + Assert.Equal(setPasswordRequestModel.KdfIterations, user.KdfIterations); + Assert.Equal(setPasswordRequestModel.KdfMemory, user.KdfMemory); + Assert.Equal(setPasswordRequestModel.KdfParallelism, user.KdfParallelism); + Assert.Equal(setPasswordRequestModel.Key, user.Key); } else { - // User had no keys and the request model's keys were null, so they should be set to null - Assert.Null(user.PublicKey); - Assert.Null(user.PrivateKey); + await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setPasswordRequestModel)); } } + [Theory] + [BitAutoData] + public async Task PostSetPasswordAsync_WhenUserExistsAndHasKeysAndKeysAreUpdated_ShouldThrowAsync( + User user, + SetPasswordRequestModel setPasswordRequestModel) + { + // Arrange + const string existingPublicKey = "existingPublicKey"; + const string existingEncryptedPrivateKey = "existingEncryptedPrivateKey"; + + const string newPublicKey = "newPublicKey"; + const string newEncryptedPrivateKey = "newEncryptedPrivateKey"; + + user.PublicKey = existingPublicKey; + user.PrivateKey = existingEncryptedPrivateKey; + + setPasswordRequestModel.Keys = new KeysRequestModel() + { + PublicKey = newPublicKey, + EncryptedPrivateKey = newEncryptedPrivateKey + }; + + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); + _setInitialMasterPasswordCommand.SetInitialMasterPasswordAsync( + user, + setPasswordRequestModel.MasterPasswordHash, + setPasswordRequestModel.Key, + setPasswordRequestModel.OrgIdentifier) + .Returns(Task.FromResult(IdentityResult.Success)); + + // Act & Assert + await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setPasswordRequestModel)); + } + + [Theory] [BitAutoData] public async Task PostSetPasswordAsync_WhenUserDoesNotExist_ShouldThrowUnauthorizedAccessException( @@ -525,6 +557,7 @@ public class AccountsControllerTests : IDisposable User user, SetPasswordRequestModel model) { + model.Keys = null; // Arrange _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); _setInitialMasterPasswordCommand.SetInitialMasterPasswordAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) From f8b65e047751b01cfdfc44ea17a029270863707e Mon Sep 17 00:00:00 2001 From: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Date: Thu, 6 Feb 2025 16:46:23 -0500 Subject: [PATCH 19/25] Removed all usages of FluentAssertions (#5378) --- .github/renovate.json | 1 - test/Billing.Test/Billing.Test.csproj | 1 - .../Controllers/PayPalControllerTests.cs | 9 +-- .../Services/StripeEventServiceTests.cs | 79 ++++++++----------- 4 files changed, 38 insertions(+), 52 deletions(-) diff --git a/.github/renovate.json b/.github/renovate.json index affa29bea9..31d78a4d4e 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -64,7 +64,6 @@ "Braintree", "coverlet.collector", "CsvHelper", - "FluentAssertions", "Kralizek.AutoFixture.Extensions.MockHttp", "Microsoft.AspNetCore.Mvc.Testing", "Microsoft.Extensions.Logging", diff --git a/test/Billing.Test/Billing.Test.csproj b/test/Billing.Test/Billing.Test.csproj index 4d71425681..b4ea2938f6 100644 --- a/test/Billing.Test/Billing.Test.csproj +++ b/test/Billing.Test/Billing.Test.csproj @@ -6,7 +6,6 @@ - diff --git a/test/Billing.Test/Controllers/PayPalControllerTests.cs b/test/Billing.Test/Controllers/PayPalControllerTests.cs index 3c9edd2220..a059207c76 100644 --- a/test/Billing.Test/Controllers/PayPalControllerTests.cs +++ b/test/Billing.Test/Controllers/PayPalControllerTests.cs @@ -8,7 +8,6 @@ using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Core.Services; using Divergic.Logging.Xunit; -using FluentAssertions; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Infrastructure; @@ -577,14 +576,14 @@ public class PayPalControllerTests { var statusCodeActionResult = (IStatusCodeActionResult)result; - statusCodeActionResult.StatusCode.Should().Be(statusCode); + Assert.Equal(statusCode, statusCodeActionResult.StatusCode); } private static void Logged(ICacheLogger logger, LogLevel logLevel, string message) { - logger.Last.Should().NotBeNull(); - logger.Last!.LogLevel.Should().Be(logLevel); - logger.Last!.Message.Should().Be(message); + Assert.NotNull(logger.Last); + Assert.Equal(logLevel, logger.Last!.LogLevel); + Assert.Equal(message, logger.Last!.Message); } private static void LoggedError(ICacheLogger logger, string message) diff --git a/test/Billing.Test/Services/StripeEventServiceTests.cs b/test/Billing.Test/Services/StripeEventServiceTests.cs index 15aa5c7234..b40e8b9408 100644 --- a/test/Billing.Test/Services/StripeEventServiceTests.cs +++ b/test/Billing.Test/Services/StripeEventServiceTests.cs @@ -2,7 +2,6 @@ using Bit.Billing.Services.Implementations; using Bit.Billing.Test.Utilities; using Bit.Core.Settings; -using FluentAssertions; using Microsoft.Extensions.Logging; using NSubstitute; using Stripe; @@ -36,10 +35,8 @@ public class StripeEventServiceTests var function = async () => await _stripeEventService.GetCharge(stripeEvent); // Assert - await function - .Should() - .ThrowAsync() - .WithMessage($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(Charge)}'"); + var exception = await Assert.ThrowsAsync(function); + Assert.Equal($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(Charge)}'", exception.Message); await _stripeFacade.DidNotReceiveWithAnyArgs().GetCharge( Arg.Any(), @@ -58,7 +55,7 @@ public class StripeEventServiceTests var charge = await _stripeEventService.GetCharge(stripeEvent); // Assert - charge.Should().BeEquivalentTo(stripeEvent.Data.Object as Charge); + Assert.Equivalent(stripeEvent.Data.Object as Charge, charge, true); await _stripeFacade.DidNotReceiveWithAnyArgs().GetCharge( Arg.Any(), @@ -88,8 +85,8 @@ public class StripeEventServiceTests var charge = await _stripeEventService.GetCharge(stripeEvent, true, expand); // Assert - charge.Should().Be(apiCharge); - charge.Should().NotBeSameAs(eventCharge); + Assert.Equal(apiCharge, charge); + Assert.NotSame(eventCharge, charge); await _stripeFacade.Received().GetCharge( apiCharge.Id, @@ -110,10 +107,8 @@ public class StripeEventServiceTests var function = async () => await _stripeEventService.GetCustomer(stripeEvent); // Assert - await function - .Should() - .ThrowAsync() - .WithMessage($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(Customer)}'"); + var exception = await Assert.ThrowsAsync(function); + Assert.Equal($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(Customer)}'", exception.Message); await _stripeFacade.DidNotReceiveWithAnyArgs().GetCustomer( Arg.Any(), @@ -132,7 +127,7 @@ public class StripeEventServiceTests var customer = await _stripeEventService.GetCustomer(stripeEvent); // Assert - customer.Should().BeEquivalentTo(stripeEvent.Data.Object as Customer); + Assert.Equivalent(stripeEvent.Data.Object as Customer, customer, true); await _stripeFacade.DidNotReceiveWithAnyArgs().GetCustomer( Arg.Any(), @@ -162,8 +157,8 @@ public class StripeEventServiceTests var customer = await _stripeEventService.GetCustomer(stripeEvent, true, expand); // Assert - customer.Should().Be(apiCustomer); - customer.Should().NotBeSameAs(eventCustomer); + Assert.Equal(apiCustomer, customer); + Assert.NotSame(eventCustomer, customer); await _stripeFacade.Received().GetCustomer( apiCustomer.Id, @@ -184,10 +179,8 @@ public class StripeEventServiceTests var function = async () => await _stripeEventService.GetInvoice(stripeEvent); // Assert - await function - .Should() - .ThrowAsync() - .WithMessage($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(Invoice)}'"); + var exception = await Assert.ThrowsAsync(function); + Assert.Equal($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(Invoice)}'", exception.Message); await _stripeFacade.DidNotReceiveWithAnyArgs().GetInvoice( Arg.Any(), @@ -206,7 +199,7 @@ public class StripeEventServiceTests var invoice = await _stripeEventService.GetInvoice(stripeEvent); // Assert - invoice.Should().BeEquivalentTo(stripeEvent.Data.Object as Invoice); + Assert.Equivalent(stripeEvent.Data.Object as Invoice, invoice, true); await _stripeFacade.DidNotReceiveWithAnyArgs().GetInvoice( Arg.Any(), @@ -236,8 +229,8 @@ public class StripeEventServiceTests var invoice = await _stripeEventService.GetInvoice(stripeEvent, true, expand); // Assert - invoice.Should().Be(apiInvoice); - invoice.Should().NotBeSameAs(eventInvoice); + Assert.Equal(apiInvoice, invoice); + Assert.NotSame(eventInvoice, invoice); await _stripeFacade.Received().GetInvoice( apiInvoice.Id, @@ -258,10 +251,8 @@ public class StripeEventServiceTests var function = async () => await _stripeEventService.GetPaymentMethod(stripeEvent); // Assert - await function - .Should() - .ThrowAsync() - .WithMessage($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(PaymentMethod)}'"); + var exception = await Assert.ThrowsAsync(function); + Assert.Equal($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(PaymentMethod)}'", exception.Message); await _stripeFacade.DidNotReceiveWithAnyArgs().GetPaymentMethod( Arg.Any(), @@ -280,7 +271,7 @@ public class StripeEventServiceTests var paymentMethod = await _stripeEventService.GetPaymentMethod(stripeEvent); // Assert - paymentMethod.Should().BeEquivalentTo(stripeEvent.Data.Object as PaymentMethod); + Assert.Equivalent(stripeEvent.Data.Object as PaymentMethod, paymentMethod, true); await _stripeFacade.DidNotReceiveWithAnyArgs().GetPaymentMethod( Arg.Any(), @@ -310,8 +301,8 @@ public class StripeEventServiceTests var paymentMethod = await _stripeEventService.GetPaymentMethod(stripeEvent, true, expand); // Assert - paymentMethod.Should().Be(apiPaymentMethod); - paymentMethod.Should().NotBeSameAs(eventPaymentMethod); + Assert.Equal(apiPaymentMethod, paymentMethod); + Assert.NotSame(eventPaymentMethod, paymentMethod); await _stripeFacade.Received().GetPaymentMethod( apiPaymentMethod.Id, @@ -332,10 +323,8 @@ public class StripeEventServiceTests var function = async () => await _stripeEventService.GetSubscription(stripeEvent); // Assert - await function - .Should() - .ThrowAsync() - .WithMessage($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(Subscription)}'"); + var exception = await Assert.ThrowsAsync(function); + Assert.Equal($"Stripe event with ID '{stripeEvent.Id}' does not have object matching type '{nameof(Subscription)}'", exception.Message); await _stripeFacade.DidNotReceiveWithAnyArgs().GetSubscription( Arg.Any(), @@ -354,7 +343,7 @@ public class StripeEventServiceTests var subscription = await _stripeEventService.GetSubscription(stripeEvent); // Assert - subscription.Should().BeEquivalentTo(stripeEvent.Data.Object as Subscription); + Assert.Equivalent(stripeEvent.Data.Object as Subscription, subscription, true); await _stripeFacade.DidNotReceiveWithAnyArgs().GetSubscription( Arg.Any(), @@ -384,8 +373,8 @@ public class StripeEventServiceTests var subscription = await _stripeEventService.GetSubscription(stripeEvent, true, expand); // Assert - subscription.Should().Be(apiSubscription); - subscription.Should().NotBeSameAs(eventSubscription); + Assert.Equal(apiSubscription, subscription); + Assert.NotSame(eventSubscription, subscription); await _stripeFacade.Received().GetSubscription( apiSubscription.Id, @@ -417,7 +406,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeTrue(); + Assert.True(cloudRegionValid); await _stripeFacade.Received(1).GetSubscription( subscription.Id, @@ -447,7 +436,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeTrue(); + Assert.True(cloudRegionValid); await _stripeFacade.Received(1).GetCharge( charge.Id, @@ -475,7 +464,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeTrue(); + Assert.True(cloudRegionValid); await _stripeFacade.Received(1).GetCustomer( invoice.CustomerId, @@ -505,7 +494,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeTrue(); + Assert.True(cloudRegionValid); await _stripeFacade.Received(1).GetInvoice( invoice.Id, @@ -535,7 +524,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeTrue(); + Assert.True(cloudRegionValid); await _stripeFacade.Received(1).GetPaymentMethod( paymentMethod.Id, @@ -561,7 +550,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeTrue(); + Assert.True(cloudRegionValid); await _stripeFacade.Received(1).GetCustomer( customer.Id, @@ -592,7 +581,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeFalse(); + Assert.False(cloudRegionValid); await _stripeFacade.Received(1).GetSubscription( subscription.Id, @@ -623,7 +612,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeTrue(); + Assert.True(cloudRegionValid); await _stripeFacade.Received(1).GetSubscription( subscription.Id, @@ -657,7 +646,7 @@ public class StripeEventServiceTests var cloudRegionValid = await _stripeEventService.ValidateCloudRegion(stripeEvent); // Assert - cloudRegionValid.Should().BeTrue(); + Assert.True(cloudRegionValid); await _stripeFacade.Received(1).GetSubscription( subscription.Id, From af07dffa6f4e1be0e3fb94fa84e67e753d38d059 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Thu, 6 Feb 2025 17:07:43 -0500 Subject: [PATCH 20/25] Relax nullable in test projects (#5379) * Relax nullable in test projects * Fix xUnit Warnings * More xUnit fixes --- Directory.Build.props | 5 +++++ .../Controllers/v2/GroupsControllerTests.cs | 2 +- .../Controllers/v2/UsersControllerTests.cs | 2 +- test/Api.IntegrationTest/Api.IntegrationTest.csproj | 1 - .../AdminConsole/Services/OrganizationServiceTests.cs | 2 -- .../Business/Tokenables/OrgUserInviteTokenableTests.cs | 2 +- test/Core.Test/Utilities/CoreHelpersTests.cs | 6 +++--- test/Core.Test/Utilities/EncryptedStringAttributeTests.cs | 2 +- .../Utilities/StrictEmailAddressAttributeTests.cs | 2 +- .../Utilities/StrictEmailAddressListAttributeTests.cs | 2 +- test/Events.IntegrationTest/Events.IntegrationTest.csproj | 7 +++---- test/Icons.Test/Models/IconLinkTests.cs | 2 +- test/Identity.Test/Identity.Test.csproj | 3 +-- .../Infrastructure.Dapper.Test.csproj | 6 ++---- .../Infrastructure.IntegrationTest.csproj | 1 - 15 files changed, 21 insertions(+), 24 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 88b63d156c..71650fc16c 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -8,6 +8,11 @@ Bit.$(MSBuildProjectName) enable false + + true + annotations + +