From 7d39efe29ff122fc9c411bb504cf7ffe34386f55 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Mon, 10 Nov 2025 08:40:40 +0100 Subject: [PATCH 1/8] [PM-27575] Add support for loading Mailer templates from disk (#6520) Adds support for overloading mail templates from disk. --- .../Mail/Mailer/HandlebarMailRenderer.cs | 59 ++++++- .../Mailer/HandlebarMailRendererTests.cs | 154 +++++++++++++++++- test/Core.Test/Platform/Mailer/MailerTest.cs | 8 +- 3 files changed, 216 insertions(+), 5 deletions(-) diff --git a/src/Core/Platform/Mail/Mailer/HandlebarMailRenderer.cs b/src/Core/Platform/Mail/Mailer/HandlebarMailRenderer.cs index 608d6d6be0..baba5b8015 100644 --- a/src/Core/Platform/Mail/Mailer/HandlebarMailRenderer.cs +++ b/src/Core/Platform/Mail/Mailer/HandlebarMailRenderer.cs @@ -1,7 +1,9 @@ #nullable enable using System.Collections.Concurrent; using System.Reflection; +using Bit.Core.Settings; using HandlebarsDotNet; +using Microsoft.Extensions.Logging; namespace Bit.Core.Platform.Mail.Mailer; public class HandlebarMailRenderer : IMailRenderer @@ -9,7 +11,7 @@ public class HandlebarMailRenderer : IMailRenderer /// /// Lazy-initialized Handlebars instance. Thread-safe and ensures initialization occurs only once. /// - private readonly Lazy> _handlebarsTask = new(InitializeHandlebarsAsync, LazyThreadSafetyMode.ExecutionAndPublication); + private readonly Lazy> _handlebarsTask; /// /// Helper function that returns the handlebar instance. @@ -21,6 +23,17 @@ public class HandlebarMailRenderer : IMailRenderer /// private readonly ConcurrentDictionary>>> _templateCache = new(); + private readonly ILogger _logger; + private readonly GlobalSettings _globalSettings; + + public HandlebarMailRenderer(ILogger logger, GlobalSettings globalSettings) + { + _logger = logger; + _globalSettings = globalSettings; + + _handlebarsTask = new Lazy>(InitializeHandlebarsAsync, LazyThreadSafetyMode.ExecutionAndPublication); + } + public async Task<(string html, string txt)> RenderAsync(BaseMailView model) { var html = await CompileTemplateAsync(model, "html"); @@ -53,19 +66,59 @@ public class HandlebarMailRenderer : IMailRenderer return handlebars.Compile(source); } - private static async Task ReadSourceAsync(Assembly assembly, string template) + private async Task ReadSourceAsync(Assembly assembly, string template) { if (assembly.GetManifestResourceNames().All(f => f != template)) { throw new FileNotFoundException("Template not found: " + template); } + var diskSource = await ReadSourceFromDiskAsync(template); + if (!string.IsNullOrWhiteSpace(diskSource)) + { + return diskSource; + } + await using var s = assembly.GetManifestResourceStream(template)!; using var sr = new StreamReader(s); return await sr.ReadToEndAsync(); } - private static async Task InitializeHandlebarsAsync() + private async Task ReadSourceFromDiskAsync(string template) + { + if (!_globalSettings.SelfHosted) + { + return null; + } + + try + { + var diskPath = Path.GetFullPath(Path.Combine(_globalSettings.MailTemplateDirectory, template)); + var baseDirectory = Path.GetFullPath(_globalSettings.MailTemplateDirectory); + + // Ensure the resolved path is within the configured directory + if (!diskPath.StartsWith(baseDirectory + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase) && + !diskPath.Equals(baseDirectory, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogWarning("Template path traversal attempt detected: {Template}", template); + return null; + } + + if (File.Exists(diskPath)) + { + var fileContents = await File.ReadAllTextAsync(diskPath); + return fileContents; + } + } + catch (Exception e) + { + _logger.LogError(e, "Failed to read mail template from disk: {TemplateName}", template); + } + + return null; + } + + private async Task InitializeHandlebarsAsync() { var handlebars = Handlebars.Create(); diff --git a/test/Core.Test/Platform/Mailer/HandlebarMailRendererTests.cs b/test/Core.Test/Platform/Mailer/HandlebarMailRendererTests.cs index 1cc7504702..2559ae2b5f 100644 --- a/test/Core.Test/Platform/Mailer/HandlebarMailRendererTests.cs +++ b/test/Core.Test/Platform/Mailer/HandlebarMailRendererTests.cs @@ -1,5 +1,8 @@ using Bit.Core.Platform.Mail.Mailer; +using Bit.Core.Settings; using Bit.Core.Test.Platform.Mailer.TestMail; +using Microsoft.Extensions.Logging; +using NSubstitute; using Xunit; namespace Bit.Core.Test.Platform.Mailer; @@ -9,7 +12,10 @@ public class HandlebarMailRendererTests [Fact] public async Task RenderAsync_ReturnsExpectedHtmlAndTxt() { - var renderer = new HandlebarMailRenderer(); + var logger = Substitute.For>(); + var globalSettings = new GlobalSettings { SelfHosted = false }; + var renderer = new HandlebarMailRenderer(logger, globalSettings); + var view = new TestMailView { Name = "John Smith" }; var (html, txt) = await renderer.RenderAsync(view); @@ -17,4 +23,150 @@ public class HandlebarMailRendererTests Assert.Equal("Hello John Smith", html.Trim()); Assert.Equal("Hello John Smith", txt.Trim()); } + + [Fact] + public async Task RenderAsync_LoadsFromDisk_WhenSelfHostedAndFileExists() + { + var logger = Substitute.For>(); + var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + + try + { + var globalSettings = new GlobalSettings + { + SelfHosted = true, + MailTemplateDirectory = tempDir + }; + + // Create test template files on disk + var htmlTemplatePath = Path.Combine(tempDir, "Bit.Core.Test.Platform.Mailer.TestMail.TestMailView.html.hbs"); + var txtTemplatePath = Path.Combine(tempDir, "Bit.Core.Test.Platform.Mailer.TestMail.TestMailView.text.hbs"); + await File.WriteAllTextAsync(htmlTemplatePath, "Custom HTML: {{Name}}"); + await File.WriteAllTextAsync(txtTemplatePath, "Custom TXT: {{Name}}"); + + var renderer = new HandlebarMailRenderer(logger, globalSettings); + var view = new TestMailView { Name = "Jane Doe" }; + + var (html, txt) = await renderer.RenderAsync(view); + + Assert.Equal("Custom HTML: Jane Doe", html.Trim()); + Assert.Equal("Custom TXT: Jane Doe", txt.Trim()); + } + finally + { + // Cleanup + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, true); + } + } + } + + [Theory] + [InlineData("../../../etc/passwd")] + [InlineData("../../../../malicious.txt")] + [InlineData("../../malicious.txt")] + [InlineData("../malicious.txt")] + public async Task ReadSourceFromDiskAsync_PrevenetsPathTraversal_WhenMaliciousPathProvided(string maliciousPath) + { + var logger = Substitute.For>(); + var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + + try + { + var globalSettings = new GlobalSettings + { + SelfHosted = true, + MailTemplateDirectory = tempDir + }; + + // Create a malicious file outside the template directory + var maliciousFile = Path.Combine(Path.GetTempPath(), "malicious.txt"); + await File.WriteAllTextAsync(maliciousFile, "Malicious Content"); + + var renderer = new HandlebarMailRenderer(logger, globalSettings); + + // Use reflection to call the private ReadSourceFromDiskAsync method + var method = typeof(HandlebarMailRenderer).GetMethod("ReadSourceFromDiskAsync", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + var task = (Task)method!.Invoke(renderer, new object[] { maliciousPath })!; + var result = await task; + + // Should return null and not load the malicious file + Assert.Null(result); + + // Verify that a warning was logged for the path traversal attempt + logger.Received(1).Log( + LogLevel.Warning, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + + // Cleanup malicious file + if (File.Exists(maliciousFile)) + { + File.Delete(maliciousFile); + } + } + finally + { + // Cleanup + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, true); + } + } + } + + [Fact] + public async Task ReadSourceFromDiskAsync_AllowsValidFileWithDifferentCase_WhenCaseInsensitiveFileSystem() + { + var logger = Substitute.For>(); + var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + + try + { + var globalSettings = new GlobalSettings + { + SelfHosted = true, + MailTemplateDirectory = tempDir + }; + + // Create a test template file + var templateFileName = "TestTemplate.hbs"; + var templatePath = Path.Combine(tempDir, templateFileName); + await File.WriteAllTextAsync(templatePath, "Test Content"); + + var renderer = new HandlebarMailRenderer(logger, globalSettings); + + // Try to read with different case (should work on case-insensitive file systems like Windows/macOS) + var method = typeof(HandlebarMailRenderer).GetMethod("ReadSourceFromDiskAsync", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + var task = (Task)method!.Invoke(renderer, new object[] { templateFileName })!; + var result = await task; + + // Should successfully read the file + Assert.Equal("Test Content", result); + + // Verify no warning was logged + logger.DidNotReceive().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + finally + { + // Cleanup + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, true); + } + } + } } diff --git a/test/Core.Test/Platform/Mailer/MailerTest.cs b/test/Core.Test/Platform/Mailer/MailerTest.cs index adaf458de0..ca9cb2a874 100644 --- a/test/Core.Test/Platform/Mailer/MailerTest.cs +++ b/test/Core.Test/Platform/Mailer/MailerTest.cs @@ -1,18 +1,24 @@ using Bit.Core.Models.Mail; using Bit.Core.Platform.Mail.Delivery; using Bit.Core.Platform.Mail.Mailer; +using Bit.Core.Settings; using Bit.Core.Test.Platform.Mailer.TestMail; +using Microsoft.Extensions.Logging; using NSubstitute; using Xunit; namespace Bit.Core.Test.Platform.Mailer; + public class MailerTest { [Fact] public async Task SendEmailAsync() { + var logger = Substitute.For>(); + var globalSettings = new GlobalSettings { SelfHosted = false }; var deliveryService = Substitute.For(); - var mailer = new Core.Platform.Mail.Mailer.Mailer(new HandlebarMailRenderer(), deliveryService); + + var mailer = new Core.Platform.Mail.Mailer.Mailer(new HandlebarMailRenderer(logger, globalSettings), deliveryService); var mail = new TestMail.TestMail() { From e7f3b6b12f67c08fc270c4f2608a9db875f5181f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Mon, 10 Nov 2025 15:27:44 +0000 Subject: [PATCH 2/8] [PM-26430] Remove Type property from PolicyRequestModel to use route parameter only (#6472) * Enhance PolicyRequestModel and SavePolicyRequest with validation for policy data and metadata. * Add integration tests for policy updates to validate handling of invalid data types in PolicyRequestModel and SavePolicyRequest. * Add missing using * Update PolicyRequestModel for null safety by making Data and ValidateAndSerializePolicyData nullable * Add integration tests for public PoliciesController to validate handling of invalid data types in policy updates. * Add PolicyDataValidator class for validating and serializing policy data and metadata based on policy type. * Refactor PolicyRequestModel, SavePolicyRequest, and PolicyUpdateRequestModel to utilize PolicyDataValidator for data validation and serialization, removing redundant methods and improving code clarity. * Update PolicyRequestModel and SavePolicyRequest to initialize Data and Metadata properties with empty dictionaries. * Refactor PolicyDataValidator to remove null checks for input data in validation methods * Rename test methods in SavePolicyRequestTests to reflect handling of empty data and metadata, and remove null assignments in test cases for improved clarity. * Remove Type property from PolicyRequestModel to use route parameter only * Run dotnet format * Enhance error handling in PolicyDataValidator to include field-specific details in BadRequestException messages. * Enhance PoliciesControllerTests to verify error messages for BadRequest responses by checking for specific field names in the response content. * refactor: Update PolicyRequestModel and SavePolicyRequest to use nullable dictionaries for Data and Metadata properties; enhance validation methods in PolicyDataValidator to handle null cases. * test: Add integration tests for handling policies with null data in PoliciesController * fix: Catch specific JsonException in PolicyDataValidator to improve error handling * test: Add unit tests for PolicyDataValidator to validate and serialize policy data and metadata * test: Remove PolicyType from PolicyRequestModel in PoliciesControllerTests * test: Update PolicyDataValidatorTests to validate organization data ownership metadata * Refactor PoliciesControllerTests to include policy type in PutVNext method calls --- .../Controllers/PoliciesController.cs | 13 ++----- .../Models/Request/PolicyRequestModel.cs | 8 ++--- .../Models/Request/SavePolicyRequest.cs | 7 ++-- .../Controllers/PoliciesControllerTests.cs | 10 ------ .../Models/Request/SavePolicyRequestTests.cs | 34 +++++++++---------- .../Controllers/PoliciesControllerTests.cs | 8 ++--- 6 files changed, 30 insertions(+), 50 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Controllers/PoliciesController.cs index 1ee6dedf89..a5272413e2 100644 --- a/src/Api/AdminConsole/Controllers/PoliciesController.cs +++ b/src/Api/AdminConsole/Controllers/PoliciesController.cs @@ -209,23 +209,17 @@ public class PoliciesController : Controller throw new NotFoundException(); } - if (type != model.Type) - { - throw new BadRequestException("Mismatched policy type"); - } - - var policyUpdate = await model.ToPolicyUpdateAsync(orgId, _currentContext); + var policyUpdate = await model.ToPolicyUpdateAsync(orgId, type, _currentContext); var policy = await _savePolicyCommand.SaveAsync(policyUpdate); return new PolicyResponseModel(policy); } - [HttpPut("{type}/vnext")] [RequireFeatureAttribute(FeatureFlagKeys.CreateDefaultLocation)] [Authorize] - public async Task PutVNext(Guid orgId, [FromBody] SavePolicyRequest model) + public async Task PutVNext(Guid orgId, PolicyType type, [FromBody] SavePolicyRequest model) { - var savePolicyRequest = await model.ToSavePolicyModelAsync(orgId, _currentContext); + var savePolicyRequest = await model.ToSavePolicyModelAsync(orgId, type, _currentContext); var policy = _featureService.IsEnabled(FeatureFlagKeys.PolicyValidatorsRefactor) ? await _vNextSavePolicyCommand.SaveAsync(savePolicyRequest) : @@ -233,5 +227,4 @@ public class PoliciesController : Controller return new PolicyResponseModel(policy); } - } diff --git a/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs b/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs index f9b9c18993..2dc7dfa7cd 100644 --- a/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs +++ b/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs @@ -9,20 +9,18 @@ namespace Bit.Api.AdminConsole.Models.Request; public class PolicyRequestModel { - [Required] - public PolicyType? Type { get; set; } [Required] public bool? Enabled { get; set; } public Dictionary? Data { get; set; } - public async Task ToPolicyUpdateAsync(Guid organizationId, ICurrentContext currentContext) + public async Task ToPolicyUpdateAsync(Guid organizationId, PolicyType type, ICurrentContext currentContext) { - var serializedData = PolicyDataValidator.ValidateAndSerialize(Data, Type!.Value); + var serializedData = PolicyDataValidator.ValidateAndSerialize(Data, type); var performedBy = new StandardUser(currentContext.UserId!.Value, await currentContext.OrganizationOwner(organizationId)); return new() { - Type = Type!.Value, + Type = type, OrganizationId = organizationId, Data = serializedData, Enabled = Enabled.GetValueOrDefault(), diff --git a/src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs b/src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs index 5c1acc1c36..2e2868a78a 100644 --- a/src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs +++ b/src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs @@ -1,4 +1,5 @@ using System.ComponentModel.DataAnnotations; +using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.AdminConsole.Utilities; @@ -13,10 +14,10 @@ public class SavePolicyRequest public Dictionary? Metadata { get; set; } - public async Task ToSavePolicyModelAsync(Guid organizationId, ICurrentContext currentContext) + public async Task ToSavePolicyModelAsync(Guid organizationId, PolicyType type, ICurrentContext currentContext) { - var policyUpdate = await Policy.ToPolicyUpdateAsync(organizationId, currentContext); - var metadata = PolicyDataValidator.ValidateAndDeserializeMetadata(Metadata, Policy.Type!.Value); + var policyUpdate = await Policy.ToPolicyUpdateAsync(organizationId, type, currentContext); + var metadata = PolicyDataValidator.ValidateAndDeserializeMetadata(Metadata, type); var performedBy = new StandardUser(currentContext.UserId!.Value, await currentContext.OrganizationOwner(organizationId)); return new SavePolicyModel(policyUpdate, performedBy, metadata); diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs index 79c31f956d..e4098ce9a9 100644 --- a/test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs @@ -67,7 +67,6 @@ public class PoliciesControllerTests : IClassFixture, IAs { Policy = new PolicyRequestModel { - Type = policyType, Enabled = true, }, Metadata = new Dictionary @@ -148,7 +147,6 @@ public class PoliciesControllerTests : IClassFixture, IAs { Policy = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = new Dictionary { @@ -218,7 +216,6 @@ public class PoliciesControllerTests : IClassFixture, IAs var policyType = PolicyType.MasterPassword; var request = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = new Dictionary { @@ -244,7 +241,6 @@ public class PoliciesControllerTests : IClassFixture, IAs var policyType = PolicyType.SendOptions; var request = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = new Dictionary { @@ -267,7 +263,6 @@ public class PoliciesControllerTests : IClassFixture, IAs var policyType = PolicyType.ResetPassword; var request = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = new Dictionary { @@ -292,7 +287,6 @@ public class PoliciesControllerTests : IClassFixture, IAs { Policy = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = new Dictionary { @@ -321,7 +315,6 @@ public class PoliciesControllerTests : IClassFixture, IAs { Policy = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = new Dictionary { @@ -347,7 +340,6 @@ public class PoliciesControllerTests : IClassFixture, IAs { Policy = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = new Dictionary { @@ -371,7 +363,6 @@ public class PoliciesControllerTests : IClassFixture, IAs var policyType = PolicyType.SingleOrg; var request = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = null }; @@ -393,7 +384,6 @@ public class PoliciesControllerTests : IClassFixture, IAs { Policy = new PolicyRequestModel { - Type = policyType, Enabled = true, Data = null }, diff --git a/test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs b/test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs index 75236fd719..163d66aeb4 100644 --- a/test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs +++ b/test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs @@ -24,11 +24,11 @@ public class SavePolicyRequestTests currentContext.OrganizationOwner(organizationId).Returns(true); var testData = new Dictionary { { "test", "value" } }; + var policyType = PolicyType.TwoFactorAuthentication; var model = new SavePolicyRequest { Policy = new PolicyRequestModel { - Type = PolicyType.TwoFactorAuthentication, Enabled = true, Data = testData }, @@ -36,7 +36,7 @@ public class SavePolicyRequestTests }; // Act - var result = await model.ToSavePolicyModelAsync(organizationId, currentContext); + var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext); // Assert Assert.Equal(PolicyType.TwoFactorAuthentication, result.PolicyUpdate.Type); @@ -63,17 +63,17 @@ public class SavePolicyRequestTests currentContext.UserId.Returns(userId); currentContext.OrganizationOwner(organizationId).Returns(false); + var policyType = PolicyType.SingleOrg; var model = new SavePolicyRequest { Policy = new PolicyRequestModel { - Type = PolicyType.SingleOrg, Enabled = false } }; // Act - var result = await model.ToSavePolicyModelAsync(organizationId, currentContext); + var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext); // Assert Assert.Null(result.PolicyUpdate.Data); @@ -93,17 +93,17 @@ public class SavePolicyRequestTests currentContext.UserId.Returns(userId); currentContext.OrganizationOwner(organizationId).Returns(true); + var policyType = PolicyType.SingleOrg; var model = new SavePolicyRequest { Policy = new PolicyRequestModel { - Type = PolicyType.SingleOrg, Enabled = false } }; // Act - var result = await model.ToSavePolicyModelAsync(organizationId, currentContext); + var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext); // Assert Assert.Null(result.PolicyUpdate.Data); @@ -124,11 +124,11 @@ public class SavePolicyRequestTests currentContext.UserId.Returns(userId); currentContext.OrganizationOwner(organizationId).Returns(true); + var policyType = PolicyType.OrganizationDataOwnership; var model = new SavePolicyRequest { Policy = new PolicyRequestModel { - Type = PolicyType.OrganizationDataOwnership, Enabled = true }, Metadata = new Dictionary @@ -138,7 +138,7 @@ public class SavePolicyRequestTests }; // Act - var result = await model.ToSavePolicyModelAsync(organizationId, currentContext); + var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext); // Assert Assert.IsType(result.Metadata); @@ -156,17 +156,17 @@ public class SavePolicyRequestTests currentContext.UserId.Returns(userId); currentContext.OrganizationOwner(organizationId).Returns(true); + var policyType = PolicyType.OrganizationDataOwnership; var model = new SavePolicyRequest { Policy = new PolicyRequestModel { - Type = PolicyType.OrganizationDataOwnership, Enabled = true } }; // Act - var result = await model.ToSavePolicyModelAsync(organizationId, currentContext); + var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext); // Assert Assert.NotNull(result); @@ -193,12 +193,11 @@ public class SavePolicyRequestTests currentContext.UserId.Returns(userId); currentContext.OrganizationOwner(organizationId).Returns(true); - + var policyType = PolicyType.ResetPassword; var model = new SavePolicyRequest { Policy = new PolicyRequestModel { - Type = PolicyType.ResetPassword, Enabled = true, Data = _complexData }, @@ -206,7 +205,7 @@ public class SavePolicyRequestTests }; // Act - var result = await model.ToSavePolicyModelAsync(organizationId, currentContext); + var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext); // Assert var deserializedData = JsonSerializer.Deserialize>(result.PolicyUpdate.Data); @@ -234,11 +233,11 @@ public class SavePolicyRequestTests currentContext.UserId.Returns(userId); currentContext.OrganizationOwner(organizationId).Returns(true); + var policyType = PolicyType.MaximumVaultTimeout; var model = new SavePolicyRequest { Policy = new PolicyRequestModel { - Type = PolicyType.MaximumVaultTimeout, Enabled = true }, Metadata = new Dictionary @@ -248,7 +247,7 @@ public class SavePolicyRequestTests }; // Act - var result = await model.ToSavePolicyModelAsync(organizationId, currentContext); + var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext); // Assert Assert.NotNull(result); @@ -266,19 +265,18 @@ public class SavePolicyRequestTests currentContext.OrganizationOwner(organizationId).Returns(true); var errorDictionary = BuildErrorDictionary(); - + var policyType = PolicyType.OrganizationDataOwnership; var model = new SavePolicyRequest { Policy = new PolicyRequestModel { - Type = PolicyType.OrganizationDataOwnership, Enabled = true }, Metadata = errorDictionary }; // Act - var result = await model.ToSavePolicyModelAsync(organizationId, currentContext); + var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext); // Assert Assert.NotNull(result); diff --git a/test/Api.Test/Controllers/PoliciesControllerTests.cs b/test/Api.Test/Controllers/PoliciesControllerTests.cs index 73cdd0fe29..89d6ddefdc 100644 --- a/test/Api.Test/Controllers/PoliciesControllerTests.cs +++ b/test/Api.Test/Controllers/PoliciesControllerTests.cs @@ -487,14 +487,14 @@ public class PoliciesControllerTests .Returns(policy); // Act - var result = await sutProvider.Sut.PutVNext(orgId, model); + var result = await sutProvider.Sut.PutVNext(orgId, policy.Type, model); // Assert await sutProvider.GetDependency() .Received(1) .SaveAsync(Arg.Is( m => m.PolicyUpdate.OrganizationId == orgId && - m.PolicyUpdate.Type == model.Policy.Type && + m.PolicyUpdate.Type == policy.Type && m.PolicyUpdate.Enabled == model.Policy.Enabled && m.PerformedBy.UserId == userId && m.PerformedBy.IsOrganizationOwnerOrProvider == true)); @@ -534,14 +534,14 @@ public class PoliciesControllerTests .Returns(policy); // Act - var result = await sutProvider.Sut.PutVNext(orgId, model); + var result = await sutProvider.Sut.PutVNext(orgId, policy.Type, model); // Assert await sutProvider.GetDependency() .Received(1) .VNextSaveAsync(Arg.Is( m => m.PolicyUpdate.OrganizationId == orgId && - m.PolicyUpdate.Type == model.Policy.Type && + m.PolicyUpdate.Type == policy.Type && m.PolicyUpdate.Enabled == model.Policy.Enabled && m.PerformedBy.UserId == userId && m.PerformedBy.IsOrganizationOwnerOrProvider == true)); From b2543b5c0f43f0fe2c9c29b7c5953c76ddb1f834 Mon Sep 17 00:00:00 2001 From: Kyle Denney <4227399+kdenney@users.noreply.github.com> Date: Mon, 10 Nov 2025 11:51:00 -0600 Subject: [PATCH 3/8] [PM-24284] - milestone 3 (#6543) * new feature flag * first pass at changes * safeguard against billing-pricing not being deployed yet * handle families pre migration plan * wrong stripe id * tests * unit tests --- .../AdminConsole/Services/ProviderService.cs | 5 +- .../Controllers/FreshsalesController.cs | 1 + src/Core/Billing/Enums/PlanType.cs | 6 +- .../Billing/Extensions/BillingExtensions.cs | 2 +- .../StaticStore/Plans/Families2025Plan.cs | 47 ++ .../Models/StaticStore/Plans/FamiliesPlan.cs | 6 +- .../Pricing/Organizations/PlanAdapter.cs | 3 +- src/Core/Billing/Pricing/PricingClient.cs | 24 +- src/Core/Constants.cs | 1 + .../Models/Business/SubscriptionUpdate.cs | 1 + src/Core/Utilities/StaticStore.cs | 1 + .../Repositories/OrganizationRepository.cs | 1 + .../ConfirmOrganizationUserCommandTests.cs | 2 + .../CloudOrganizationSignUpCommandTests.cs | 2 + .../Billing/Pricing/PricingClientTests.cs | 527 ++++++++++++++++++ .../SecretsManagerSubscriptionUpdateTests.cs | 2 +- test/Core.Test/Utilities/StaticStoreTests.cs | 6 +- 17 files changed, 621 insertions(+), 16 deletions(-) create mode 100644 src/Core/Billing/Models/StaticStore/Plans/Families2025Plan.cs create mode 100644 test/Core.Test/Billing/Pricing/PricingClientTests.cs diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs index aaf0050b63..89ef251fd6 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs @@ -35,8 +35,9 @@ public class ProviderService : IProviderService { private static readonly PlanType[] _resellerDisallowedOrganizationTypes = [ PlanType.Free, - PlanType.FamiliesAnnually, - PlanType.FamiliesAnnually2019 + PlanType.FamiliesAnnually2025, + PlanType.FamiliesAnnually2019, + PlanType.FamiliesAnnually ]; private readonly IDataProtector _dataProtector; diff --git a/src/Billing/Controllers/FreshsalesController.cs b/src/Billing/Controllers/FreshsalesController.cs index be5a9ddb16..68382fbd5d 100644 --- a/src/Billing/Controllers/FreshsalesController.cs +++ b/src/Billing/Controllers/FreshsalesController.cs @@ -158,6 +158,7 @@ public class FreshsalesController : Controller planName = "Free"; return true; case PlanType.FamiliesAnnually: + case PlanType.FamiliesAnnually2025: case PlanType.FamiliesAnnually2019: planName = "Families"; return true; diff --git a/src/Core/Billing/Enums/PlanType.cs b/src/Core/Billing/Enums/PlanType.cs index e88a73af16..0f910c4980 100644 --- a/src/Core/Billing/Enums/PlanType.cs +++ b/src/Core/Billing/Enums/PlanType.cs @@ -18,8 +18,8 @@ public enum PlanType : byte EnterpriseAnnually2019 = 5, [Display(Name = "Custom")] Custom = 6, - [Display(Name = "Families")] - FamiliesAnnually = 7, + [Display(Name = "Families 2025")] + FamiliesAnnually2025 = 7, [Display(Name = "Teams (Monthly) 2020")] TeamsMonthly2020 = 8, [Display(Name = "Teams (Annually) 2020")] @@ -48,4 +48,6 @@ public enum PlanType : byte EnterpriseAnnually = 20, [Display(Name = "Teams Starter")] TeamsStarter = 21, + [Display(Name = "Families")] + FamiliesAnnually = 22, } diff --git a/src/Core/Billing/Extensions/BillingExtensions.cs b/src/Core/Billing/Extensions/BillingExtensions.cs index 7f81bfd33f..2dae0c2025 100644 --- a/src/Core/Billing/Extensions/BillingExtensions.cs +++ b/src/Core/Billing/Extensions/BillingExtensions.cs @@ -15,7 +15,7 @@ public static class BillingExtensions => planType switch { PlanType.Custom or PlanType.Free => ProductTierType.Free, - PlanType.FamiliesAnnually or PlanType.FamiliesAnnually2019 => ProductTierType.Families, + PlanType.FamiliesAnnually or PlanType.FamiliesAnnually2025 or PlanType.FamiliesAnnually2019 => ProductTierType.Families, PlanType.TeamsStarter or PlanType.TeamsStarter2023 => ProductTierType.TeamsStarter, _ when planType.ToString().Contains("Teams") => ProductTierType.Teams, _ when planType.ToString().Contains("Enterprise") => ProductTierType.Enterprise, diff --git a/src/Core/Billing/Models/StaticStore/Plans/Families2025Plan.cs b/src/Core/Billing/Models/StaticStore/Plans/Families2025Plan.cs new file mode 100644 index 0000000000..77e238e98e --- /dev/null +++ b/src/Core/Billing/Models/StaticStore/Plans/Families2025Plan.cs @@ -0,0 +1,47 @@ +using Bit.Core.Billing.Enums; +using Bit.Core.Models.StaticStore; + +namespace Bit.Core.Billing.Models.StaticStore.Plans; + +public record Families2025Plan : Plan +{ + public Families2025Plan() + { + Type = PlanType.FamiliesAnnually2025; + ProductTier = ProductTierType.Families; + Name = "Families 2025"; + IsAnnual = true; + NameLocalizationKey = "planNameFamilies"; + DescriptionLocalizationKey = "planDescFamilies"; + + TrialPeriodDays = 7; + + HasSelfHost = true; + HasTotp = true; + UsersGetPremium = true; + + UpgradeSortOrder = 1; + DisplaySortOrder = 1; + + PasswordManager = new Families2025PasswordManagerFeatures(); + } + + private record Families2025PasswordManagerFeatures : PasswordManagerPlanFeatures + { + public Families2025PasswordManagerFeatures() + { + BaseSeats = 6; + BaseStorageGb = 1; + MaxSeats = 6; + + HasAdditionalStorageOption = true; + + StripePlanId = "2020-families-org-annually"; + StripeStoragePlanId = "personal-storage-gb-annually"; + BasePrice = 40; + AdditionalStoragePricePerGb = 4; + + AllowSeatAutoscale = false; + } + } +} diff --git a/src/Core/Billing/Models/StaticStore/Plans/FamiliesPlan.cs b/src/Core/Billing/Models/StaticStore/Plans/FamiliesPlan.cs index 8c71e50fa4..b2edc1168b 100644 --- a/src/Core/Billing/Models/StaticStore/Plans/FamiliesPlan.cs +++ b/src/Core/Billing/Models/StaticStore/Plans/FamiliesPlan.cs @@ -23,12 +23,12 @@ public record FamiliesPlan : Plan UpgradeSortOrder = 1; DisplaySortOrder = 1; - PasswordManager = new TeamsPasswordManagerFeatures(); + PasswordManager = new FamiliesPasswordManagerFeatures(); } - private record TeamsPasswordManagerFeatures : PasswordManagerPlanFeatures + private record FamiliesPasswordManagerFeatures : PasswordManagerPlanFeatures { - public TeamsPasswordManagerFeatures() + public FamiliesPasswordManagerFeatures() { BaseSeats = 6; BaseStorageGb = 1; diff --git a/src/Core/Billing/Pricing/Organizations/PlanAdapter.cs b/src/Core/Billing/Pricing/Organizations/PlanAdapter.cs index 390f7b2146..ac60411366 100644 --- a/src/Core/Billing/Pricing/Organizations/PlanAdapter.cs +++ b/src/Core/Billing/Pricing/Organizations/PlanAdapter.cs @@ -58,6 +58,7 @@ public record PlanAdapter : Core.Models.StaticStore.Plan "enterprise-monthly-2020" => PlanType.EnterpriseMonthly2020, "enterprise-monthly-2023" => PlanType.EnterpriseMonthly2023, "families" => PlanType.FamiliesAnnually, + "families-2025" => PlanType.FamiliesAnnually2025, "families-2019" => PlanType.FamiliesAnnually2019, "free" => PlanType.Free, "teams-annually" => PlanType.TeamsAnnually, @@ -77,7 +78,7 @@ public record PlanAdapter : Core.Models.StaticStore.Plan => planType switch { PlanType.Free => ProductTierType.Free, - PlanType.FamiliesAnnually or PlanType.FamiliesAnnually2019 => ProductTierType.Families, + PlanType.FamiliesAnnually or PlanType.FamiliesAnnually2025 or PlanType.FamiliesAnnually2019 => ProductTierType.Families, PlanType.TeamsStarter or PlanType.TeamsStarter2023 => ProductTierType.TeamsStarter, _ when planType.ToString().Contains("Teams") => ProductTierType.Teams, _ when planType.ToString().Contains("Enterprise") => ProductTierType.Enterprise, diff --git a/src/Core/Billing/Pricing/PricingClient.cs b/src/Core/Billing/Pricing/PricingClient.cs index 21863d03e8..0c4266665a 100644 --- a/src/Core/Billing/Pricing/PricingClient.cs +++ b/src/Core/Billing/Pricing/PricingClient.cs @@ -50,7 +50,7 @@ public class PricingClient( var plan = await response.Content.ReadFromJsonAsync(); return plan == null ? throw new BillingException(message: "Deserialization of Pricing Service response resulted in null") - : new PlanAdapter(plan); + : new PlanAdapter(PreProcessFamiliesPreMigrationPlan(plan)); } if (response.StatusCode == HttpStatusCode.NotFound) @@ -91,7 +91,7 @@ public class PricingClient( var plans = await response.Content.ReadFromJsonAsync>(); return plans == null ? throw new BillingException(message: "Deserialization of Pricing Service response resulted in null") - : plans.Select(OrganizationPlan (plan) => new PlanAdapter(plan)).ToList(); + : plans.Select(OrganizationPlan (plan) => new PlanAdapter(PreProcessFamiliesPreMigrationPlan(plan))).ToList(); } throw new BillingException( @@ -137,7 +137,7 @@ public class PricingClient( message: $"Request to the Pricing Service failed with status {response.StatusCode}"); } - private static string? GetLookupKey(PlanType planType) + private string? GetLookupKey(PlanType planType) => planType switch { PlanType.EnterpriseAnnually => "enterprise-annually", @@ -149,6 +149,10 @@ public class PricingClient( PlanType.EnterpriseMonthly2020 => "enterprise-monthly-2020", PlanType.EnterpriseMonthly2023 => "enterprise-monthly-2023", PlanType.FamiliesAnnually => "families", + PlanType.FamiliesAnnually2025 => + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3) + ? "families-2025" + : "families", PlanType.FamiliesAnnually2019 => "families-2019", PlanType.Free => "free", PlanType.TeamsAnnually => "teams-annually", @@ -164,6 +168,20 @@ public class PricingClient( _ => null }; + /// + /// Safeguard used until the feature flag is enabled. Pricing service will return the + /// 2025PreMigration plan with "families" lookup key. When that is detected and the FF + /// is still disabled, set the lookup key to families-2025 so PlanAdapter will assign + /// the correct plan. + /// + /// The plan to preprocess + private Plan PreProcessFamiliesPreMigrationPlan(Plan plan) + { + if (plan.LookupKey == "families" && !featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3)) + plan.LookupKey = "families-2025"; + return plan; + } + private static PremiumPlan CurrentPremiumPlan => new() { Name = "Premium", diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index eaa8f9163a..c5b6bbc10d 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -195,6 +195,7 @@ public static class FeatureFlagKeys public const string PremiumUpgradeNewDesign = "pm-24033-updat-premium-subscription-page"; public const string PM26793_FetchPremiumPriceFromPricingService = "pm-26793-fetch-premium-price-from-pricing-service"; public const string PM23341_Milestone_2 = "pm-23341-milestone-2"; + public const string PM26462_Milestone_3 = "pm-26462-milestone-3"; /* Key Management Team */ public const string ReturnErrorOnExistingKeypair = "return-error-on-existing-keypair"; diff --git a/src/Core/Models/Business/SubscriptionUpdate.cs b/src/Core/Models/Business/SubscriptionUpdate.cs index 028fcad80b..7c23c9b73c 100644 --- a/src/Core/Models/Business/SubscriptionUpdate.cs +++ b/src/Core/Models/Business/SubscriptionUpdate.cs @@ -50,6 +50,7 @@ public abstract class SubscriptionUpdate protected static bool IsNonSeatBasedPlan(StaticStore.Plan plan) => plan.Type is >= PlanType.FamiliesAnnually2019 and <= PlanType.EnterpriseAnnually2019 + or PlanType.FamiliesAnnually2025 or PlanType.FamiliesAnnually or PlanType.TeamsStarter2023 or PlanType.TeamsStarter; diff --git a/src/Core/Utilities/StaticStore.cs b/src/Core/Utilities/StaticStore.cs index 1ddd926569..36c4a54ae4 100644 --- a/src/Core/Utilities/StaticStore.cs +++ b/src/Core/Utilities/StaticStore.cs @@ -137,6 +137,7 @@ public static class StaticStore new Teams2019Plan(true), new Teams2019Plan(false), new Families2019Plan(), + new Families2025Plan() }.ToImmutableList(); } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs index 2238bfca76..ebc2bc6606 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs @@ -129,6 +129,7 @@ public class OrganizationRepository : Repository sutProvider) { signup.Plan = planType; @@ -65,6 +66,7 @@ public class CloudICloudOrganizationSignUpCommandTests [Theory] [BitAutoData(PlanType.FamiliesAnnually)] + [BitAutoData(PlanType.FamiliesAnnually2025)] public async Task SignUp_AssignsOwnerToDefaultCollection (PlanType planType, OrganizationSignup signup, SutProvider sutProvider) { diff --git a/test/Core.Test/Billing/Pricing/PricingClientTests.cs b/test/Core.Test/Billing/Pricing/PricingClientTests.cs new file mode 100644 index 0000000000..189df15b9c --- /dev/null +++ b/test/Core.Test/Billing/Pricing/PricingClientTests.cs @@ -0,0 +1,527 @@ +using System.Net; +using Bit.Core.Billing; +using Bit.Core.Billing.Enums; +using Bit.Core.Billing.Pricing; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.Extensions.Logging; +using NSubstitute; +using RichardSzalay.MockHttp; +using Xunit; +using GlobalSettings = Bit.Core.Settings.GlobalSettings; + +namespace Bit.Core.Test.Billing.Pricing; + +[SutProviderCustomize] +public class PricingClientTests +{ + #region GetLookupKey Tests (via GetPlan) + + [Fact] + public async Task GetPlan_WithFamiliesAnnually2025AndFeatureFlagEnabled_UsesFamilies2025LookupKey() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + var planJson = CreatePlanJson("families-2025", "Families 2025", "families", 40M, "price_id"); + + mockHttp.Expect(HttpMethod.Get, "https://test.com/plans/organization/families-2025") + .Respond("application/json", planJson); + + mockHttp.When(HttpMethod.Get, "*/plans/organization/*") + .Respond("application/json", planJson); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(true); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.GetPlan(PlanType.FamiliesAnnually2025); + + // Assert + Assert.NotNull(result); + Assert.Equal(PlanType.FamiliesAnnually2025, result.Type); + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public async Task GetPlan_WithFamiliesAnnually2025AndFeatureFlagDisabled_UsesFamiliesLookupKey() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + var planJson = CreatePlanJson("families", "Families", "families", 40M, "price_id"); + + mockHttp.Expect(HttpMethod.Get, "https://test.com/plans/organization/families") + .Respond("application/json", planJson); + + mockHttp.When(HttpMethod.Get, "*/plans/organization/*") + .Respond("application/json", planJson); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(false); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.GetPlan(PlanType.FamiliesAnnually2025); + + // Assert + Assert.NotNull(result); + // PreProcessFamiliesPreMigrationPlan should change "families" to "families-2025" when FF is disabled + Assert.Equal(PlanType.FamiliesAnnually2025, result.Type); + mockHttp.VerifyNoOutstandingExpectation(); + } + + #endregion + + #region PreProcessFamiliesPreMigrationPlan Tests (via GetPlan) + + [Fact] + public async Task GetPlan_WithFamiliesAnnually2025AndFeatureFlagDisabled_ReturnsFamiliesAnnually2025PlanType() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + // billing-pricing returns "families" lookup key because the flag is off + var planJson = CreatePlanJson("families", "Families", "families", 40M, "price_id"); + + mockHttp.When(HttpMethod.Get, "*/plans/organization/*") + .Respond("application/json", planJson); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(false); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.GetPlan(PlanType.FamiliesAnnually2025); + + // Assert + Assert.NotNull(result); + // PreProcessFamiliesPreMigrationPlan should convert the families lookup key to families-2025 + // and the PlanAdapter should assign the correct FamiliesAnnually2025 plan type + Assert.Equal(PlanType.FamiliesAnnually2025, result.Type); + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public async Task GetPlan_WithFamiliesAnnually2025AndFeatureFlagEnabled_ReturnsFamiliesAnnually2025PlanType() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + var planJson = CreatePlanJson("families-2025", "Families", "families", 40M, "price_id"); + + mockHttp.When(HttpMethod.Get, "*/plans/organization/*") + .Respond("application/json", planJson); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(true); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.GetPlan(PlanType.FamiliesAnnually2025); + + // Assert + Assert.NotNull(result); + // PreProcessFamiliesPreMigrationPlan should ignore the lookup key because the flag is on + // and the PlanAdapter should assign the correct FamiliesAnnually2025 plan type + Assert.Equal(PlanType.FamiliesAnnually2025, result.Type); + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public async Task GetPlan_WithFamiliesAnnuallyAndFeatureFlagEnabled_ReturnsFamiliesAnnuallyPlanType() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + var planJson = CreatePlanJson("families", "Families", "families", 40M, "price_id"); + + mockHttp.When(HttpMethod.Get, "*/plans/organization/*") + .Respond("application/json", planJson); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(true); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.GetPlan(PlanType.FamiliesAnnually); + + // Assert + Assert.NotNull(result); + // PreProcessFamiliesPreMigrationPlan should ignore the lookup key because the flag is on + // and the PlanAdapter should assign the correct FamiliesAnnually plan type + Assert.Equal(PlanType.FamiliesAnnually, result.Type); + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public async Task GetPlan_WithOtherLookupKey_KeepsLookupKeyUnchanged() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + var planJson = CreatePlanJson("enterprise-annually", "Enterprise", "enterprise", 144M, "price_id"); + + mockHttp.Expect(HttpMethod.Get, "https://test.com/plans/organization/enterprise-annually") + .Respond("application/json", planJson); + + mockHttp.When(HttpMethod.Get, "*/plans/organization/*") + .Respond("application/json", planJson); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(false); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.GetPlan(PlanType.EnterpriseAnnually); + + // Assert + Assert.NotNull(result); + Assert.Equal(PlanType.EnterpriseAnnually, result.Type); + mockHttp.VerifyNoOutstandingExpectation(); + } + + #endregion + + #region ListPlans Tests + + [Fact] + public async Task ListPlans_WithFeatureFlagDisabled_ReturnsListWithPreProcessing() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + // biling-pricing would return "families" because the flag is disabled + var plansJson = $@"[ + {CreatePlanJson("families", "Families", "families", 40M, "price_id")}, + {CreatePlanJson("enterprise-annually", "Enterprise", "enterprise", 144M, "price_id")} + ]"; + + mockHttp.When(HttpMethod.Get, "*/plans/organization") + .Respond("application/json", plansJson); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(false); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.ListPlans(); + + // Assert + Assert.NotNull(result); + Assert.Equal(2, result.Count); + // First plan should have been preprocessed from "families" to "families-2025" + Assert.Equal(PlanType.FamiliesAnnually2025, result[0].Type); + // Second plan should remain unchanged + Assert.Equal(PlanType.EnterpriseAnnually, result[1].Type); + mockHttp.VerifyNoOutstandingExpectation(); + } + + [Fact] + public async Task ListPlans_WithFeatureFlagEnabled_ReturnsListWithoutPreProcessing() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + var plansJson = $@"[ + {CreatePlanJson("families", "Families", "families", 40M, "price_id")} + ]"; + + mockHttp.When(HttpMethod.Get, "*/plans/organization") + .Respond("application/json", plansJson); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(true); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.ListPlans(); + + // Assert + Assert.NotNull(result); + Assert.Single(result); + // Plan should remain as FamiliesAnnually when FF is enabled + Assert.Equal(PlanType.FamiliesAnnually, result[0].Type); + mockHttp.VerifyNoOutstandingExpectation(); + } + + #endregion + + #region GetPlan - Additional Coverage + + [Theory, BitAutoData] + public async Task GetPlan_WhenSelfHosted_ReturnsNull( + SutProvider sutProvider) + { + // Arrange + var globalSettings = sutProvider.GetDependency(); + globalSettings.SelfHosted = true; + + // Act + var result = await sutProvider.Sut.GetPlan(PlanType.FamiliesAnnually2025); + + // Assert + Assert.Null(result); + } + + [Theory, BitAutoData] + public async Task GetPlan_WhenPricingServiceDisabled_ReturnsStaticStorePlan( + SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().SelfHosted = false; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.UsePricingService) + .Returns(false); + + // Act + var result = await sutProvider.Sut.GetPlan(PlanType.FamiliesAnnually); + + // Assert + Assert.NotNull(result); + Assert.Equal(PlanType.FamiliesAnnually, result.Type); + } + + [Theory, BitAutoData] + public async Task GetPlan_WhenLookupKeyNotFound_ReturnsNull( + SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.UsePricingService) + .Returns(true); + + // Act - Using PlanType that doesn't have a lookup key mapping + var result = await sutProvider.Sut.GetPlan(unchecked((PlanType)999)); + + // Assert + Assert.Null(result); + } + + [Fact] + public async Task GetPlan_WhenPricingServiceReturnsNotFound_ReturnsNull() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + mockHttp.When(HttpMethod.Get, "*/plans/organization/*") + .Respond(HttpStatusCode.NotFound); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(true); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act + var result = await pricingClient.GetPlan(PlanType.FamiliesAnnually2025); + + // Assert + Assert.Null(result); + } + + [Fact] + public async Task GetPlan_WhenPricingServiceReturnsError_ThrowsBillingException() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + mockHttp.When(HttpMethod.Get, "*/plans/organization/*") + .Respond(HttpStatusCode.InternalServerError); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.PM26462_Milestone_3).Returns(true); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act & Assert + await Assert.ThrowsAsync(() => + pricingClient.GetPlan(PlanType.FamiliesAnnually2025)); + } + + #endregion + + #region ListPlans - Additional Coverage + + [Theory, BitAutoData] + public async Task ListPlans_WhenSelfHosted_ReturnsEmptyList( + SutProvider sutProvider) + { + // Arrange + var globalSettings = sutProvider.GetDependency(); + globalSettings.SelfHosted = true; + + // Act + var result = await sutProvider.Sut.ListPlans(); + + // Assert + Assert.NotNull(result); + Assert.Empty(result); + } + + [Theory, BitAutoData] + public async Task ListPlans_WhenPricingServiceDisabled_ReturnsStaticStorePlans( + SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().SelfHosted = false; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.UsePricingService) + .Returns(false); + + // Act + var result = await sutProvider.Sut.ListPlans(); + + // Assert + Assert.NotNull(result); + Assert.NotEmpty(result); + Assert.Equal(StaticStore.Plans.Count(), result.Count); + } + + [Fact] + public async Task ListPlans_WhenPricingServiceReturnsError_ThrowsBillingException() + { + // Arrange + var mockHttp = new MockHttpMessageHandler(); + mockHttp.When(HttpMethod.Get, "*/plans/organization") + .Respond(HttpStatusCode.InternalServerError); + + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.UsePricingService).Returns(true); + + var globalSettings = new GlobalSettings { SelfHosted = false }; + + var httpClient = new HttpClient(mockHttp) + { + BaseAddress = new Uri("https://test.com/") + }; + + var logger = Substitute.For>(); + var pricingClient = new PricingClient(featureService, globalSettings, httpClient, logger); + + // Act & Assert + await Assert.ThrowsAsync(() => + pricingClient.ListPlans()); + } + + #endregion + + private static string CreatePlanJson( + string lookupKey, + string name, + string tier, + decimal seatsPrice, + string seatsStripePriceId, + int seatsQuantity = 1) + { + return $@"{{ + ""lookupKey"": ""{lookupKey}"", + ""name"": ""{name}"", + ""tier"": ""{tier}"", + ""features"": [], + ""seats"": {{ + ""type"": ""packaged"", + ""quantity"": {seatsQuantity}, + ""price"": {seatsPrice}, + ""stripePriceId"": ""{seatsStripePriceId}"" + }}, + ""canUpgradeTo"": [], + ""additionalData"": {{ + ""nameLocalizationKey"": ""{lookupKey}Name"", + ""descriptionLocalizationKey"": ""{lookupKey}Description"" + }} + }}"; + } +} diff --git a/test/Core.Test/Models/Business/SecretsManagerSubscriptionUpdateTests.cs b/test/Core.Test/Models/Business/SecretsManagerSubscriptionUpdateTests.cs index 6a411363a0..20405b07b0 100644 --- a/test/Core.Test/Models/Business/SecretsManagerSubscriptionUpdateTests.cs +++ b/test/Core.Test/Models/Business/SecretsManagerSubscriptionUpdateTests.cs @@ -22,7 +22,7 @@ public class SecretsManagerSubscriptionUpdateTests } public static TheoryData NonSmPlans => - ToPlanTheory([PlanType.Custom, PlanType.FamiliesAnnually, PlanType.FamiliesAnnually2019]); + ToPlanTheory([PlanType.Custom, PlanType.FamiliesAnnually, PlanType.FamiliesAnnually2025, PlanType.FamiliesAnnually2019]); public static TheoryData SmPlans => ToPlanTheory([ PlanType.EnterpriseAnnually2019, diff --git a/test/Core.Test/Utilities/StaticStoreTests.cs b/test/Core.Test/Utilities/StaticStoreTests.cs index 05c6d358e5..01e2ab8914 100644 --- a/test/Core.Test/Utilities/StaticStoreTests.cs +++ b/test/Core.Test/Utilities/StaticStoreTests.cs @@ -13,7 +13,7 @@ public class StaticStoreTests var plans = StaticStore.Plans.ToList(); Assert.NotNull(plans); Assert.NotEmpty(plans); - Assert.Equal(22, plans.Count); + Assert.Equal(23, plans.Count); } [Theory] @@ -34,8 +34,8 @@ public class StaticStoreTests { // Ref: https://daniel.haxx.se/blog/2025/05/16/detecting-malicious-unicode/ // URLs can contain unicode characters that to a computer would point to completely seperate domains but to the - // naked eye look completely identical. For example 'g' and 'ց' look incredibly similar but when included in a - // URL would lead you somewhere different. There is an opening for an attacker to contribute to Bitwarden with a + // naked eye look completely identical. For example 'g' and 'ց' look incredibly similar but when included in a + // URL would lead you somewhere different. There is an opening for an attacker to contribute to Bitwarden with a // url update that could be missed in code review and then if they got a user to that URL Bitwarden could // consider it equivalent with a cipher in the users vault and offer autofill when we should not. // GitHub does now show a warning on non-ascii characters but it could still be missed. From 746b413cff692c819c08a018d6790b337acfe1cc Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Mon, 10 Nov 2025 13:37:11 -0500 Subject: [PATCH 4/8] [PM-21741] MJML welcome emails (#6549) feat: Implement welcome email using MJML templating - Implement MJML templates for welcome emails (individual, family, org) - Create reusable MJML components (mj-bw-icon-row, mj-bw-learn-more-footer) - Update documentation for MJML development process --- .../Auth/SendAccessEmailOtpEmailv2.html.hbs | 383 +++++++++--------- src/Core/MailTemplates/Mjml/.mjmlconfig | 4 +- src/Core/MailTemplates/Mjml/README.md | 17 +- .../MailTemplates/Mjml/components/head.mjml | 6 - .../Mjml/components/learn-more-footer.mjml | 18 - .../Mjml/components/mj-bw-hero.js | 76 ++-- .../Mjml/components/mj-bw-icon-row.js | 100 +++++ .../components/mj-bw-learn-more-footer.js | 51 +++ .../Auth/Onboarding/welcome-family-user.mjml | 60 +++ .../Auth/Onboarding/welcome-free-user.mjml | 59 +++ .../Auth/Onboarding/welcome-org-user.mjml | 60 +++ .../Mjml/emails/Auth/send-email-otp.mjml | 2 +- .../Mjml/emails/Auth/two-factor.mjml | 4 +- .../MailTemplates/Mjml/emails/invite.mjml | 2 +- 14 files changed, 580 insertions(+), 262 deletions(-) delete mode 100644 src/Core/MailTemplates/Mjml/components/learn-more-footer.mjml create mode 100644 src/Core/MailTemplates/Mjml/components/mj-bw-icon-row.js create mode 100644 src/Core/MailTemplates/Mjml/components/mj-bw-learn-more-footer.js create mode 100644 src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-family-user.mjml create mode 100644 src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-free-user.mjml create mode 100644 src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-org-user.mjml diff --git a/src/Core/MailTemplates/Handlebars/Auth/SendAccessEmailOtpEmailv2.html.hbs b/src/Core/MailTemplates/Handlebars/Auth/SendAccessEmailOtpEmailv2.html.hbs index 095cdc82d7..fad0af840d 100644 --- a/src/Core/MailTemplates/Handlebars/Auth/SendAccessEmailOtpEmailv2.html.hbs +++ b/src/Core/MailTemplates/Handlebars/Auth/SendAccessEmailOtpEmailv2.html.hbs @@ -29,8 +29,8 @@ .mj-outlook-group-fix { width:100% !important; } - - + + - - - - + + + + - + - - + +
- + - - + +
- +
- +
- - + + - - + +
- +
- +
- + - + - + - +
- +
- + - +
- +
- +

Verify your email to access this Bitwarden Send

- +
- +
- + - +
- + - + - +
- + -
- - - + + + +
- +
- +
- +
- +
- - + + - - + +
- +
- +
- - + + - + - + - - + +
- +
- - + +
- +
- +
- +
- + - + - + - + - + - +
- +
Your verification code is:
- +
- +
{{Token}}
- +
- +
 
- +
- +
This code expires in {{Expiry}} minutes. After that, you'll need to verify your email again.
- +
- +
- +
- +
- +
- - + + - - + +
- +
- +
- +
- + - + - +
- +

Bitwarden Send transmits sensitive, temporary information to others easily and securely. Learn more about @@ -317,160 +325,160 @@ sign up to try it today.

- +
- +
- +
- +
- +
- - + +
- +
- - + + - + - + - - + +
- +
- - + +
- + -
- - + + +
- + - + - +
- -

+ +

Learn more about Bitwarden -

+

Find user guides, product documentation, and videos on the Bitwarden Help Center.
- +
- +
- - - + + +
- + - + - +
- +
- + - +
- +
- +
- +
- +
- - + +
- +
- - + + - + - + - - + +
- +
- +
- + - + - + - +
- - + + - + - + - +
@@ -485,15 +493,15 @@
- + - + - +
@@ -508,15 +516,15 @@
- + - + - +
@@ -531,15 +539,15 @@
- + - + - +
@@ -554,15 +562,15 @@
- + - + - +
@@ -577,15 +585,15 @@
- + - + - +
@@ -600,15 +608,15 @@
- + - + - +
@@ -623,20 +631,20 @@
- - + +
- +

© 2025 Bitwarden Inc. 1 N. Calle Cesar Chavez, Suite 102, Santa Barbara, CA, USA @@ -647,29 +655,28 @@ bitwarden.com | Learn why we include this

- +
- +
- +
- +
- - + + - - + +
- + - \ No newline at end of file diff --git a/src/Core/MailTemplates/Mjml/.mjmlconfig b/src/Core/MailTemplates/Mjml/.mjmlconfig index c382f10a12..92734a5f71 100644 --- a/src/Core/MailTemplates/Mjml/.mjmlconfig +++ b/src/Core/MailTemplates/Mjml/.mjmlconfig @@ -1,5 +1,7 @@ { "packages": [ - "components/mj-bw-hero" + "components/mj-bw-hero", + "components/mj-bw-icon-row", + "components/mj-bw-learn-more-footer" ] } diff --git a/src/Core/MailTemplates/Mjml/README.md b/src/Core/MailTemplates/Mjml/README.md index 7a497252d0..b9041c94f6 100644 --- a/src/Core/MailTemplates/Mjml/README.md +++ b/src/Core/MailTemplates/Mjml/README.md @@ -45,7 +45,7 @@ When using MJML templating you can use the above [commands](#building-mjml-files Not all MJML tags have the same attributes, it is highly recommended to review the documentation on the official MJML website to understand the usages of each of the tags. -### Recommended development +### Recommended development - IMailService #### Mjml email template development @@ -58,11 +58,17 @@ Not all MJML tags have the same attributes, it is highly recommended to review t After the email is developed from the [initial step](#mjml-email-template-development) make sure the email `{{variables}}` are populated properly by running it through an `IMailService` implementation. -1. run `npm run build:minify` +1. run `npm run build:hbs` 2. copy built `*.html.hbs` files from the build directory to a location the mail service can consume them + 1. all files in the `Core/MailTemplates/Mjml/out` directory can be copied to the `src/Core/MailTemplates/Handlebars/MJML` directory. If a shared component is modified it is important to copy and overwrite all files in that directory to capture + changes in the `*.html.hbs`. 3. run code that will send the email -The minified `html.hbs` artifacts are deliverables and must be placed into the correct `src/Core/MailTemplates/Handlebars/` directories in order to be used by `IMailService` implementations. +The minified `html.hbs` artifacts are deliverables and must be placed into the correct `src/Core/MailTemplates/Handlebars/` directories in order to be used by `IMailService` implementations, see 2.1 above. + +### Recommended development - IMailer + +TBD - PM-26475 ### Custom tags @@ -110,3 +116,8 @@ You are also able to reference other more static MJML templates in your MJML fil ``` + +#### `head.mjml` +Currently we include the `head.mjml` file in all MJML templates as it contains shared styling and formatting that ensures consistency across all email implementations. + +In the future we may deviate from this practice to support different layouts. At that time we will modify the docs with direction. diff --git a/src/Core/MailTemplates/Mjml/components/head.mjml b/src/Core/MailTemplates/Mjml/components/head.mjml index cf78cd6223..4cb27889eb 100644 --- a/src/Core/MailTemplates/Mjml/components/head.mjml +++ b/src/Core/MailTemplates/Mjml/components/head.mjml @@ -22,9 +22,3 @@ border-radius: 3px; } - - - -@media only screen and - (max-width: 480px) { .hide-small-img { display: none !important; } .send-bubble { padding-left: 20px; padding-right: 20px; width: 90% !important; } } - diff --git a/src/Core/MailTemplates/Mjml/components/learn-more-footer.mjml b/src/Core/MailTemplates/Mjml/components/learn-more-footer.mjml deleted file mode 100644 index 9df0614aae..0000000000 --- a/src/Core/MailTemplates/Mjml/components/learn-more-footer.mjml +++ /dev/null @@ -1,18 +0,0 @@ - - - -

- Learn more about Bitwarden -

- Find user guides, product documentation, and videos on the - Bitwarden Help Center. - - - - - - diff --git a/src/Core/MailTemplates/Mjml/components/mj-bw-hero.js b/src/Core/MailTemplates/Mjml/components/mj-bw-hero.js index d329d4ea38..c7a3b7e7ff 100644 --- a/src/Core/MailTemplates/Mjml/components/mj-bw-hero.js +++ b/src/Core/MailTemplates/Mjml/components/mj-bw-hero.js @@ -18,27 +18,19 @@ class MjBwHero extends BodyComponent { static defaultAttributes = {}; + componentHeadStyle = breakpoint => { + return ` + @media only screen and (max-width:${breakpoint}) { + .mj-bw-hero-responsive-img { + display: none !important; + } + } + ` + } + render() { - if (this.getAttribute("button-text") && this.getAttribute("button-url")) { - return this.renderMJML(` - - - - -

- ${this.getAttribute("title")} -

- - ${this.getAttribute("button-text")} - - - - - - `); - } else { - return this.renderMJML(` + >` : ""; + const subTitleElement = this.getAttribute("sub-title") ? + ` +

+ ${this.getAttribute("sub-title")} +

+ ` : ""; + + return this.renderMJML( + ` ${this.getAttribute("title")} - + ` + + subTitleElement + + ` + ` + + buttonElement + + ` + css-class="mj-bw-hero-responsive-img" + /> - `); - } + `, + ); } } diff --git a/src/Core/MailTemplates/Mjml/components/mj-bw-icon-row.js b/src/Core/MailTemplates/Mjml/components/mj-bw-icon-row.js new file mode 100644 index 0000000000..f7f402c96e --- /dev/null +++ b/src/Core/MailTemplates/Mjml/components/mj-bw-icon-row.js @@ -0,0 +1,100 @@ +const { BodyComponent } = require("mjml-core"); +class MjBwIconRow extends BodyComponent { + static dependencies = { + "mj-column": ["mj-bw-icon-row"], + "mj-wrapper": ["mj-bw-icon-row"], + "mj-bw-icon-row": [], + }; + + static allowedAttributes = { + "icon-src": "string", + "icon-alt": "string", + "head-url-text": "string", + "head-url": "string", + text: "string", + "foot-url-text": "string", + "foot-url": "string", + }; + + static defaultAttributes = {}; + + componentHeadStyle = (breakpoint) => { + return ` + @media only screen and (max-width:${breakpoint}): { + ".mj-bw-icon-row-text": { + padding-left: "5px !important", + line-height: "20px", + }, + ".mj-bw-icon-row": { + padding: "10px 15px", + width: "fit-content !important", + } + } + `; + }; + + render() { + const headAnchorElement = + this.getAttribute("head-url-text") && this.getAttribute("head-url") + ? ` + ${this.getAttribute("head-url-text")} + + External Link Icon + + ` + : ""; + + const footAnchorElement = + this.getAttribute("foot-url-text") && this.getAttribute("foot-url") + ? ` + ${this.getAttribute("foot-url-text")} + + External Link Icon + + ` + : ""; + + return this.renderMJML( + ` + + + + + + + + ` + + headAnchorElement + + ` + + + ${this.getAttribute("text")} + + + ` + + footAnchorElement + + ` + + + + + `, + ); + } +} + +module.exports = MjBwIconRow; diff --git a/src/Core/MailTemplates/Mjml/components/mj-bw-learn-more-footer.js b/src/Core/MailTemplates/Mjml/components/mj-bw-learn-more-footer.js new file mode 100644 index 0000000000..7dc2185995 --- /dev/null +++ b/src/Core/MailTemplates/Mjml/components/mj-bw-learn-more-footer.js @@ -0,0 +1,51 @@ +const { BodyComponent } = require("mjml-core"); +class MjBwLearnMoreFooter extends BodyComponent { + static dependencies = { + // Tell the validator which tags are allowed as our component's parent + "mj-column": ["mj-bw-learn-more-footer"], + "mj-wrapper": ["mj-bw-learn-more-footer"], + // Tell the validator which tags are allowed as our component's children + "mj-bw-learn-more-footer": [], + }; + + static allowedAttributes = {}; + + static defaultAttributes = {}; + + componentHeadStyle = (breakpoint) => { + return ` + @media only screen and (max-width:${breakpoint}) { + .mj-bw-learn-more-footer-responsive-img { + display: none !important; + } + } + `; + }; + + render() { + return this.renderMJML( + ` + + + +

+ Learn more about Bitwarden +

+ Find user guides, product documentation, and videos on the + Bitwarden Help Center. + + + + + + + `, + ); + } +} + +module.exports = MjBwLearnMoreFooter; diff --git a/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-family-user.mjml b/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-family-user.mjml new file mode 100644 index 0000000000..86de49016d --- /dev/null +++ b/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-family-user.mjml @@ -0,0 +1,60 @@ + + + + + + + + + + + + + + + + + An administrator from {{OrganizationName}} will approve you + before you can share passwords. While you wait for approval, get + started with Bitwarden Password Manager: + + + + + + + + + + + + + + + + + + + diff --git a/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-free-user.mjml b/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-free-user.mjml new file mode 100644 index 0000000000..e071cd26cc --- /dev/null +++ b/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-free-user.mjml @@ -0,0 +1,59 @@ + + + + + + + + + + + + + + + + + Follow these simple steps to get up and running with Bitwarden + Password Manager: + + + + + + + + + + + + + + + + + + + diff --git a/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-org-user.mjml b/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-org-user.mjml new file mode 100644 index 0000000000..39f18fce66 --- /dev/null +++ b/src/Core/MailTemplates/Mjml/emails/Auth/Onboarding/welcome-org-user.mjml @@ -0,0 +1,60 @@ + + + + + + + + + + + + + + + + + An administrator from {{OrganizationName}} will need to confirm + you before you can share passwords. Get started with Bitwarden + Password Manager: + + + + + + + + + + + + + + + + + + + diff --git a/src/Core/MailTemplates/Mjml/emails/Auth/send-email-otp.mjml b/src/Core/MailTemplates/Mjml/emails/Auth/send-email-otp.mjml index 6ccc481ff8..d3d4eb9891 100644 --- a/src/Core/MailTemplates/Mjml/emails/Auth/send-email-otp.mjml +++ b/src/Core/MailTemplates/Mjml/emails/Auth/send-email-otp.mjml @@ -55,7 +55,7 @@ - + diff --git a/src/Core/MailTemplates/Mjml/emails/Auth/two-factor.mjml b/src/Core/MailTemplates/Mjml/emails/Auth/two-factor.mjml index 3b63c278fc..73d205ba57 100644 --- a/src/Core/MailTemplates/Mjml/emails/Auth/two-factor.mjml +++ b/src/Core/MailTemplates/Mjml/emails/Auth/two-factor.mjml @@ -24,6 +24,8 @@ - + + + diff --git a/src/Core/MailTemplates/Mjml/emails/invite.mjml b/src/Core/MailTemplates/Mjml/emails/invite.mjml index cdace39c95..8e08a6753a 100644 --- a/src/Core/MailTemplates/Mjml/emails/invite.mjml +++ b/src/Core/MailTemplates/Mjml/emails/invite.mjml @@ -22,7 +22,7 @@ - + From 212f10d22ba7a47d0327c3ef93f15358ae477806 Mon Sep 17 00:00:00 2001 From: Brant DeBow <125889545+brant-livefront@users.noreply.github.com> Date: Mon, 10 Nov 2025 14:55:36 -0500 Subject: [PATCH 5/8] Extend Unit Test Coverage of Event Integrations (#6517) * Extend Unit Test Coverage of Event Integrations * Expanded SlackService error handling and tests * Cleaned up a few issues noted by Claude --- ...onIntegrationConfigurationResponseModel.cs | 4 - .../Models/Slack/SlackApiResponse.cs | 6 + .../AdminConsole/Services/ISlackService.cs | 8 +- .../SlackIntegrationHandler.cs | 33 ++++- .../EventIntegrations/SlackService.cs | 50 +++++-- .../NoopImplementations/NoopSlackService.cs | 8 +- .../OrganizationIntegrationControllerTests.cs | 23 +++ ...ntegrationsConfigurationControllerTests.cs | 129 +++++++++------- .../SlackIntegrationControllerTests.cs | 38 +++++ .../TeamsIntegrationControllerTests.cs | 44 ++++++ ...rganizationIntegrationRequestModelTests.cs | 33 +++++ .../IntegrationTemplateContextTests.cs | 14 ++ .../EventIntegrationEventWriteServiceTests.cs | 14 ++ .../Services/EventIntegrationHandlerTests.cs | 10 ++ .../Services/IntegrationFilterServiceTests.cs | 68 +++++++++ .../Services/SlackIntegrationHandlerTests.cs | 97 ++++++++++++ .../Services/SlackServiceTests.cs | 139 +++++++++++++++++- 17 files changed, 635 insertions(+), 83 deletions(-) diff --git a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationIntegrationConfigurationResponseModel.cs b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationIntegrationConfigurationResponseModel.cs index c7906318e8..d070375d88 100644 --- a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationIntegrationConfigurationResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationIntegrationConfigurationResponseModel.cs @@ -2,8 +2,6 @@ using Bit.Core.Enums; using Bit.Core.Models.Api; -#nullable enable - namespace Bit.Api.AdminConsole.Models.Response.Organizations; public class OrganizationIntegrationConfigurationResponseModel : ResponseModel @@ -11,8 +9,6 @@ public class OrganizationIntegrationConfigurationResponseModel : ResponseModel public OrganizationIntegrationConfigurationResponseModel(OrganizationIntegrationConfiguration organizationIntegrationConfiguration, string obj = "organizationIntegrationConfiguration") : base(obj) { - ArgumentNullException.ThrowIfNull(organizationIntegrationConfiguration); - Id = organizationIntegrationConfiguration.Id; Configuration = organizationIntegrationConfiguration.Configuration; CreationDate = organizationIntegrationConfiguration.CreationDate; diff --git a/src/Core/AdminConsole/Models/Slack/SlackApiResponse.cs b/src/Core/AdminConsole/Models/Slack/SlackApiResponse.cs index 70d280c428..3c811e2b28 100644 --- a/src/Core/AdminConsole/Models/Slack/SlackApiResponse.cs +++ b/src/Core/AdminConsole/Models/Slack/SlackApiResponse.cs @@ -33,6 +33,12 @@ public class SlackOAuthResponse : SlackApiResponse public SlackTeam Team { get; set; } = new(); } +public class SlackSendMessageResponse : SlackApiResponse +{ + [JsonPropertyName("channel")] + public string Channel { get; set; } = string.Empty; +} + public class SlackTeam { public string Id { get; set; } = string.Empty; diff --git a/src/Core/AdminConsole/Services/ISlackService.cs b/src/Core/AdminConsole/Services/ISlackService.cs index 0577532ac2..60d3da8af4 100644 --- a/src/Core/AdminConsole/Services/ISlackService.cs +++ b/src/Core/AdminConsole/Services/ISlackService.cs @@ -1,4 +1,6 @@ -namespace Bit.Core.Services; +using Bit.Core.Models.Slack; + +namespace Bit.Core.Services; /// Defines operations for interacting with Slack, including OAuth authentication, channel discovery, /// and sending messages. @@ -54,6 +56,6 @@ public interface ISlackService /// A valid Slack OAuth access token. /// The message text to send. /// The channel ID to send the message to. - /// A task that completes when the message has been sent. - Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId); + /// The response from Slack after sending the message. + Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId); } diff --git a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackIntegrationHandler.cs b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackIntegrationHandler.cs index 2d29494afc..16c756c8c4 100644 --- a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackIntegrationHandler.cs +++ b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackIntegrationHandler.cs @@ -6,14 +6,43 @@ public class SlackIntegrationHandler( ISlackService slackService) : IntegrationHandlerBase { + private static readonly HashSet _retryableErrors = new(StringComparer.Ordinal) + { + "internal_error", + "message_limit_exceeded", + "rate_limited", + "ratelimited", + "service_unavailable" + }; + public override async Task HandleAsync(IntegrationMessage message) { - await slackService.SendSlackMessageByChannelIdAsync( + var slackResponse = await slackService.SendSlackMessageByChannelIdAsync( message.Configuration.Token, message.RenderedTemplate, message.Configuration.ChannelId ); - return new IntegrationHandlerResult(success: true, message: message); + if (slackResponse is null) + { + return new IntegrationHandlerResult(success: false, message: message) + { + FailureReason = "Slack response was null" + }; + } + + if (slackResponse.Ok) + { + return new IntegrationHandlerResult(success: true, message: message); + } + + var result = new IntegrationHandlerResult(success: false, message: message) { FailureReason = slackResponse.Error }; + + if (_retryableErrors.Contains(slackResponse.Error)) + { + result.Retryable = true; + } + + return result; } } diff --git a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackService.cs b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackService.cs index 8b691dd4bf..7eec2ec374 100644 --- a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackService.cs +++ b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackService.cs @@ -1,5 +1,6 @@ using System.Net.Http.Headers; using System.Net.Http.Json; +using System.Text.Json; using System.Web; using Bit.Core.Models.Slack; using Bit.Core.Settings; @@ -71,7 +72,7 @@ public class SlackService( public async Task GetDmChannelByEmailAsync(string token, string email) { var userId = await GetUserIdByEmailAsync(token, email); - return await OpenDmChannel(token, userId); + return await OpenDmChannelAsync(token, userId); } public string GetRedirectUrl(string callbackUrl, string state) @@ -97,21 +98,21 @@ public class SlackService( } var tokenResponse = await _httpClient.PostAsync($"{_slackApiBaseUrl}/oauth.v2.access", - new FormUrlEncodedContent(new[] - { + new FormUrlEncodedContent([ new KeyValuePair("client_id", _clientId), new KeyValuePair("client_secret", _clientSecret), new KeyValuePair("code", code), new KeyValuePair("redirect_uri", redirectUrl) - })); + ])); SlackOAuthResponse? result; try { result = await tokenResponse.Content.ReadFromJsonAsync(); } - catch + catch (JsonException ex) { + logger.LogError(ex, "Error parsing SlackOAuthResponse: invalid JSON"); result = null; } @@ -129,14 +130,25 @@ public class SlackService( return result.AccessToken; } - public async Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId) + public async Task SendSlackMessageByChannelIdAsync(string token, string message, + string channelId) { var payload = JsonContent.Create(new { channel = channelId, text = message }); var request = new HttpRequestMessage(HttpMethod.Post, $"{_slackApiBaseUrl}/chat.postMessage"); request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); request.Content = payload; - await _httpClient.SendAsync(request); + var response = await _httpClient.SendAsync(request); + + try + { + return await response.Content.ReadFromJsonAsync(); + } + catch (JsonException ex) + { + logger.LogError(ex, "Error parsing Slack message response: invalid JSON"); + return null; + } } private async Task GetUserIdByEmailAsync(string token, string email) @@ -144,7 +156,16 @@ public class SlackService( var request = new HttpRequestMessage(HttpMethod.Get, $"{_slackApiBaseUrl}/users.lookupByEmail?email={email}"); request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); var response = await _httpClient.SendAsync(request); - var result = await response.Content.ReadFromJsonAsync(); + SlackUserResponse? result; + try + { + result = await response.Content.ReadFromJsonAsync(); + } + catch (JsonException ex) + { + logger.LogError(ex, "Error parsing SlackUserResponse: invalid JSON"); + result = null; + } if (result is null) { @@ -160,7 +181,7 @@ public class SlackService( return result.User.Id; } - private async Task OpenDmChannel(string token, string userId) + private async Task OpenDmChannelAsync(string token, string userId) { if (string.IsNullOrEmpty(userId)) return string.Empty; @@ -170,7 +191,16 @@ public class SlackService( request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); request.Content = payload; var response = await _httpClient.SendAsync(request); - var result = await response.Content.ReadFromJsonAsync(); + SlackDmResponse? result; + try + { + result = await response.Content.ReadFromJsonAsync(); + } + catch (JsonException ex) + { + logger.LogError(ex, "Error parsing SlackDmResponse: invalid JSON"); + result = null; + } if (result is null) { diff --git a/src/Core/AdminConsole/Services/NoopImplementations/NoopSlackService.cs b/src/Core/AdminConsole/Services/NoopImplementations/NoopSlackService.cs index d6c8d08c4c..a54df94814 100644 --- a/src/Core/AdminConsole/Services/NoopImplementations/NoopSlackService.cs +++ b/src/Core/AdminConsole/Services/NoopImplementations/NoopSlackService.cs @@ -1,4 +1,5 @@ -using Bit.Core.Services; +using Bit.Core.Models.Slack; +using Bit.Core.Services; namespace Bit.Core.AdminConsole.Services.NoopImplementations; @@ -24,9 +25,10 @@ public class NoopSlackService : ISlackService return string.Empty; } - public Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId) + public Task SendSlackMessageByChannelIdAsync(string token, string message, + string channelId) { - return Task.FromResult(0); + return Task.FromResult(null); } public Task ObtainTokenViaOAuth(string code, string redirectUrl) diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationControllerTests.cs index 1dd0e86f39..335859e0c4 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationControllerTests.cs @@ -133,6 +133,29 @@ public class OrganizationIntegrationControllerTests .DeleteAsync(organizationIntegration); } + [Theory, BitAutoData] + public async Task PostDeleteAsync_AllParamsProvided_Succeeds( + SutProvider sutProvider, + Guid organizationId, + OrganizationIntegration organizationIntegration) + { + organizationIntegration.OrganizationId = organizationId; + sutProvider.Sut.Url = Substitute.For(); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(organizationIntegration); + + await sutProvider.Sut.PostDeleteAsync(organizationId, organizationIntegration.Id); + + await sutProvider.GetDependency().Received(1) + .GetByIdAsync(organizationIntegration.Id); + await sutProvider.GetDependency().Received(1) + .DeleteAsync(organizationIntegration); + } + [Theory, BitAutoData] public async Task DeleteAsync_IntegrationDoesNotBelongToOrganization_ThrowsNotFound( SutProvider sutProvider, diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationsConfigurationControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationsConfigurationControllerTests.cs index 4ccfa70308..9ab626d3f0 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationsConfigurationControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationsConfigurationControllerTests.cs @@ -51,6 +51,36 @@ public class OrganizationIntegrationsConfigurationControllerTests .DeleteAsync(organizationIntegrationConfiguration); } + [Theory, BitAutoData] + public async Task PostDeleteAsync_AllParamsProvided_Succeeds( + SutProvider sutProvider, + Guid organizationId, + OrganizationIntegration organizationIntegration, + OrganizationIntegrationConfiguration organizationIntegrationConfiguration) + { + organizationIntegration.OrganizationId = organizationId; + organizationIntegrationConfiguration.OrganizationIntegrationId = organizationIntegration.Id; + sutProvider.Sut.Url = Substitute.For(); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(organizationIntegration); + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(organizationIntegrationConfiguration); + + await sutProvider.Sut.PostDeleteAsync(organizationId, organizationIntegration.Id, organizationIntegrationConfiguration.Id); + + await sutProvider.GetDependency().Received(1) + .GetByIdAsync(organizationIntegration.Id); + await sutProvider.GetDependency().Received(1) + .GetByIdAsync(organizationIntegrationConfiguration.Id); + await sutProvider.GetDependency().Received(1) + .DeleteAsync(organizationIntegrationConfiguration); + } + [Theory, BitAutoData] public async Task DeleteAsync_IntegrationConfigurationDoesNotExist_ThrowsNotFound( SutProvider sutProvider, @@ -199,27 +229,6 @@ public class OrganizationIntegrationsConfigurationControllerTests .GetManyByIntegrationAsync(organizationIntegration.Id); } - // [Theory, BitAutoData] - // public async Task GetAsync_IntegrationConfigurationDoesNotExist_ThrowsNotFound( - // SutProvider sutProvider, - // Guid organizationId, - // OrganizationIntegration organizationIntegration) - // { - // organizationIntegration.OrganizationId = organizationId; - // sutProvider.Sut.Url = Substitute.For(); - // sutProvider.GetDependency() - // .OrganizationOwner(organizationId) - // .Returns(true); - // sutProvider.GetDependency() - // .GetByIdAsync(Arg.Any()) - // .Returns(organizationIntegration); - // sutProvider.GetDependency() - // .GetByIdAsync(Arg.Any()) - // .ReturnsNull(); - // - // await Assert.ThrowsAsync(async () => await sutProvider.Sut.GetAsync(organizationId, Guid.Empty, Guid.Empty)); - // } - // [Theory, BitAutoData] public async Task GetAsync_IntegrationDoesNotExist_ThrowsNotFound( SutProvider sutProvider, @@ -293,15 +302,16 @@ public class OrganizationIntegrationsConfigurationControllerTests sutProvider.GetDependency() .CreateAsync(Arg.Any()) .Returns(organizationIntegrationConfiguration); - var requestAction = await sutProvider.Sut.CreateAsync(organizationId, organizationIntegration.Id, model); + var createResponse = await sutProvider.Sut.CreateAsync(organizationId, organizationIntegration.Id, model); await sutProvider.GetDependency().Received(1) .CreateAsync(Arg.Any()); - Assert.IsType(requestAction); - Assert.Equal(expected.Id, requestAction.Id); - Assert.Equal(expected.Configuration, requestAction.Configuration); - Assert.Equal(expected.EventType, requestAction.EventType); - Assert.Equal(expected.Template, requestAction.Template); + Assert.IsType(createResponse); + Assert.Equal(expected.Id, createResponse.Id); + Assert.Equal(expected.Configuration, createResponse.Configuration); + Assert.Equal(expected.EventType, createResponse.EventType); + Assert.Equal(expected.Filters, createResponse.Filters); + Assert.Equal(expected.Template, createResponse.Template); } [Theory, BitAutoData] @@ -331,15 +341,16 @@ public class OrganizationIntegrationsConfigurationControllerTests sutProvider.GetDependency() .CreateAsync(Arg.Any()) .Returns(organizationIntegrationConfiguration); - var requestAction = await sutProvider.Sut.CreateAsync(organizationId, organizationIntegration.Id, model); + var createResponse = await sutProvider.Sut.CreateAsync(organizationId, organizationIntegration.Id, model); await sutProvider.GetDependency().Received(1) .CreateAsync(Arg.Any()); - Assert.IsType(requestAction); - Assert.Equal(expected.Id, requestAction.Id); - Assert.Equal(expected.Configuration, requestAction.Configuration); - Assert.Equal(expected.EventType, requestAction.EventType); - Assert.Equal(expected.Template, requestAction.Template); + Assert.IsType(createResponse); + Assert.Equal(expected.Id, createResponse.Id); + Assert.Equal(expected.Configuration, createResponse.Configuration); + Assert.Equal(expected.EventType, createResponse.EventType); + Assert.Equal(expected.Filters, createResponse.Filters); + Assert.Equal(expected.Template, createResponse.Template); } [Theory, BitAutoData] @@ -369,15 +380,16 @@ public class OrganizationIntegrationsConfigurationControllerTests sutProvider.GetDependency() .CreateAsync(Arg.Any()) .Returns(organizationIntegrationConfiguration); - var requestAction = await sutProvider.Sut.CreateAsync(organizationId, organizationIntegration.Id, model); + var createResponse = await sutProvider.Sut.CreateAsync(organizationId, organizationIntegration.Id, model); await sutProvider.GetDependency().Received(1) .CreateAsync(Arg.Any()); - Assert.IsType(requestAction); - Assert.Equal(expected.Id, requestAction.Id); - Assert.Equal(expected.Configuration, requestAction.Configuration); - Assert.Equal(expected.EventType, requestAction.EventType); - Assert.Equal(expected.Template, requestAction.Template); + Assert.IsType(createResponse); + Assert.Equal(expected.Id, createResponse.Id); + Assert.Equal(expected.Configuration, createResponse.Configuration); + Assert.Equal(expected.EventType, createResponse.EventType); + Assert.Equal(expected.Filters, createResponse.Filters); + Assert.Equal(expected.Template, createResponse.Template); } [Theory, BitAutoData] @@ -575,7 +587,7 @@ public class OrganizationIntegrationsConfigurationControllerTests sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) .Returns(organizationIntegrationConfiguration); - var requestAction = await sutProvider.Sut.UpdateAsync( + var updateResponse = await sutProvider.Sut.UpdateAsync( organizationId, organizationIntegration.Id, organizationIntegrationConfiguration.Id, @@ -583,11 +595,12 @@ public class OrganizationIntegrationsConfigurationControllerTests await sutProvider.GetDependency().Received(1) .ReplaceAsync(Arg.Any()); - Assert.IsType(requestAction); - Assert.Equal(expected.Id, requestAction.Id); - Assert.Equal(expected.Configuration, requestAction.Configuration); - Assert.Equal(expected.EventType, requestAction.EventType); - Assert.Equal(expected.Template, requestAction.Template); + Assert.IsType(updateResponse); + Assert.Equal(expected.Id, updateResponse.Id); + Assert.Equal(expected.Configuration, updateResponse.Configuration); + Assert.Equal(expected.EventType, updateResponse.EventType); + Assert.Equal(expected.Filters, updateResponse.Filters); + Assert.Equal(expected.Template, updateResponse.Template); } @@ -619,7 +632,7 @@ public class OrganizationIntegrationsConfigurationControllerTests sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) .Returns(organizationIntegrationConfiguration); - var requestAction = await sutProvider.Sut.UpdateAsync( + var updateResponse = await sutProvider.Sut.UpdateAsync( organizationId, organizationIntegration.Id, organizationIntegrationConfiguration.Id, @@ -627,11 +640,12 @@ public class OrganizationIntegrationsConfigurationControllerTests await sutProvider.GetDependency().Received(1) .ReplaceAsync(Arg.Any()); - Assert.IsType(requestAction); - Assert.Equal(expected.Id, requestAction.Id); - Assert.Equal(expected.Configuration, requestAction.Configuration); - Assert.Equal(expected.EventType, requestAction.EventType); - Assert.Equal(expected.Template, requestAction.Template); + Assert.IsType(updateResponse); + Assert.Equal(expected.Id, updateResponse.Id); + Assert.Equal(expected.Configuration, updateResponse.Configuration); + Assert.Equal(expected.EventType, updateResponse.EventType); + Assert.Equal(expected.Filters, updateResponse.Filters); + Assert.Equal(expected.Template, updateResponse.Template); } [Theory, BitAutoData] @@ -662,7 +676,7 @@ public class OrganizationIntegrationsConfigurationControllerTests sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) .Returns(organizationIntegrationConfiguration); - var requestAction = await sutProvider.Sut.UpdateAsync( + var updateResponse = await sutProvider.Sut.UpdateAsync( organizationId, organizationIntegration.Id, organizationIntegrationConfiguration.Id, @@ -670,11 +684,12 @@ public class OrganizationIntegrationsConfigurationControllerTests await sutProvider.GetDependency().Received(1) .ReplaceAsync(Arg.Any()); - Assert.IsType(requestAction); - Assert.Equal(expected.Id, requestAction.Id); - Assert.Equal(expected.Configuration, requestAction.Configuration); - Assert.Equal(expected.EventType, requestAction.EventType); - Assert.Equal(expected.Template, requestAction.Template); + Assert.IsType(updateResponse); + Assert.Equal(expected.Id, updateResponse.Id); + Assert.Equal(expected.Configuration, updateResponse.Configuration); + Assert.Equal(expected.EventType, updateResponse.EventType); + Assert.Equal(expected.Filters, updateResponse.Filters); + Assert.Equal(expected.Template, updateResponse.Template); } [Theory, BitAutoData] diff --git a/test/Api.Test/AdminConsole/Controllers/SlackIntegrationControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/SlackIntegrationControllerTests.cs index 61d3486c51..c079445559 100644 --- a/test/Api.Test/AdminConsole/Controllers/SlackIntegrationControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/SlackIntegrationControllerTests.cs @@ -71,6 +71,26 @@ public class SlackIntegrationControllerTests await sutProvider.Sut.CreateAsync(string.Empty, state.ToString())); } + [Theory, BitAutoData] + public async Task CreateAsync_CallbackUrlIsEmpty_ThrowsBadRequest( + SutProvider sutProvider, + OrganizationIntegration integration) + { + integration.Type = IntegrationType.Slack; + integration.Configuration = null; + sutProvider.Sut.Url = Substitute.For(); + sutProvider.Sut.Url + .RouteUrl(Arg.Is(c => c.RouteName == "SlackIntegration_Create")) + .Returns((string?)null); + sutProvider.GetDependency() + .GetByIdAsync(integration.Id) + .Returns(integration); + var state = IntegrationOAuthState.FromIntegration(integration, sutProvider.GetDependency()); + + await Assert.ThrowsAsync(async () => + await sutProvider.Sut.CreateAsync(_validSlackCode, state.ToString())); + } + [Theory, BitAutoData] public async Task CreateAsync_SlackServiceReturnsEmpty_ThrowsBadRequest( SutProvider sutProvider, @@ -153,6 +173,8 @@ public class SlackIntegrationControllerTests OrganizationIntegration wrongOrgIntegration) { wrongOrgIntegration.Id = integration.Id; + wrongOrgIntegration.Type = IntegrationType.Slack; + wrongOrgIntegration.Configuration = null; sutProvider.Sut.Url = Substitute.For(); sutProvider.Sut.Url @@ -304,6 +326,22 @@ public class SlackIntegrationControllerTests await Assert.ThrowsAsync(async () => await sutProvider.Sut.RedirectAsync(organizationId)); } + [Theory, BitAutoData] + public async Task RedirectAsync_CallbackUrlReturnsEmpty_ThrowsBadRequest( + SutProvider sutProvider, + Guid organizationId) + { + sutProvider.Sut.Url = Substitute.For(); + sutProvider.Sut.Url + .RouteUrl(Arg.Is(c => c.RouteName == "SlackIntegration_Create")) + .Returns((string?)null); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RedirectAsync(organizationId)); + } + [Theory, BitAutoData] public async Task RedirectAsync_SlackServiceReturnsEmpty_ThrowsNotFound( SutProvider sutProvider, diff --git a/test/Api.Test/AdminConsole/Controllers/TeamsIntegrationControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/TeamsIntegrationControllerTests.cs index 3af2affdd8..3302a87372 100644 --- a/test/Api.Test/AdminConsole/Controllers/TeamsIntegrationControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/TeamsIntegrationControllerTests.cs @@ -60,6 +60,26 @@ public class TeamsIntegrationControllerTests Assert.IsType(requestAction); } + [Theory, BitAutoData] + public async Task CreateAsync_CallbackUrlIsEmpty_ThrowsBadRequest( + SutProvider sutProvider, + OrganizationIntegration integration) + { + integration.Type = IntegrationType.Teams; + integration.Configuration = null; + sutProvider.Sut.Url = Substitute.For(); + sutProvider.Sut.Url + .RouteUrl(Arg.Is(c => c.RouteName == "TeamsIntegration_Create")) + .Returns((string?)null); + sutProvider.GetDependency() + .GetByIdAsync(integration.Id) + .Returns(integration); + var state = IntegrationOAuthState.FromIntegration(integration, sutProvider.GetDependency()); + + await Assert.ThrowsAsync(async () => + await sutProvider.Sut.CreateAsync(_validTeamsCode, state.ToString())); + } + [Theory, BitAutoData] public async Task CreateAsync_CodeIsEmpty_ThrowsBadRequest( SutProvider sutProvider, @@ -315,6 +335,30 @@ public class TeamsIntegrationControllerTests sutProvider.GetDependency().Received(1).GetRedirectUrl(Arg.Any(), expectedState.ToString()); } + [Theory, BitAutoData] + public async Task RedirectAsync_CallbackUrlIsEmpty_ThrowsBadRequest( + SutProvider sutProvider, + Guid organizationId, + OrganizationIntegration integration) + { + integration.OrganizationId = organizationId; + integration.Configuration = null; + integration.Type = IntegrationType.Teams; + + sutProvider.Sut.Url = Substitute.For(); + sutProvider.Sut.Url + .RouteUrl(Arg.Is(c => c.RouteName == "TeamsIntegration_Create")) + .Returns((string?)null); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); + sutProvider.GetDependency() + .GetManyByOrganizationAsync(organizationId) + .Returns([integration]); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RedirectAsync(organizationId)); + } + [Theory, BitAutoData] public async Task RedirectAsync_IntegrationAlreadyExistsWithConfig_ThrowsBadRequest( SutProvider sutProvider, diff --git a/test/Api.Test/AdminConsole/Models/Request/Organizations/OrganizationIntegrationRequestModelTests.cs b/test/Api.Test/AdminConsole/Models/Request/Organizations/OrganizationIntegrationRequestModelTests.cs index 1303e5fe89..76e206abf4 100644 --- a/test/Api.Test/AdminConsole/Models/Request/Organizations/OrganizationIntegrationRequestModelTests.cs +++ b/test/Api.Test/AdminConsole/Models/Request/Organizations/OrganizationIntegrationRequestModelTests.cs @@ -1,14 +1,47 @@ using System.ComponentModel.DataAnnotations; using System.Text.Json; using Bit.Api.AdminConsole.Models.Request.Organizations; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Models.Data.EventIntegrations; using Bit.Core.Enums; +using Bit.Test.Common.AutoFixture.Attributes; using Xunit; namespace Bit.Api.Test.AdminConsole.Models.Request.Organizations; public class OrganizationIntegrationRequestModelTests { + [Fact] + public void ToOrganizationIntegration_CreatesNewOrganizationIntegration() + { + var model = new OrganizationIntegrationRequestModel + { + Type = IntegrationType.Hec, + Configuration = JsonSerializer.Serialize(new HecIntegration(Uri: new Uri("http://localhost"), Scheme: "Bearer", Token: "Token")) + }; + + var organizationId = Guid.NewGuid(); + var organizationIntegration = model.ToOrganizationIntegration(organizationId); + + Assert.Equal(organizationIntegration.Type, model.Type); + Assert.Equal(organizationIntegration.Configuration, model.Configuration); + Assert.Equal(organizationIntegration.OrganizationId, organizationId); + } + + [Theory, BitAutoData] + public void ToOrganizationIntegration_UpdatesExistingOrganizationIntegration(OrganizationIntegration integration) + { + var model = new OrganizationIntegrationRequestModel + { + Type = IntegrationType.Hec, + Configuration = JsonSerializer.Serialize(new HecIntegration(Uri: new Uri("http://localhost"), Scheme: "Bearer", Token: "Token")) + }; + + var organizationIntegration = model.ToOrganizationIntegration(integration); + + Assert.Equal(organizationIntegration.Configuration, model.Configuration); + } + [Fact] public void Validate_CloudBillingSync_ReturnsNotYetSupportedError() { diff --git a/test/Core.Test/AdminConsole/Models/Data/EventIntegrations/IntegrationTemplateContextTests.cs b/test/Core.Test/AdminConsole/Models/Data/EventIntegrations/IntegrationTemplateContextTests.cs index 930b04121c..cdb109e285 100644 --- a/test/Core.Test/AdminConsole/Models/Data/EventIntegrations/IntegrationTemplateContextTests.cs +++ b/test/Core.Test/AdminConsole/Models/Data/EventIntegrations/IntegrationTemplateContextTests.cs @@ -20,6 +20,20 @@ public class IntegrationTemplateContextTests Assert.Equal(expected, sut.EventMessage); } + [Theory, BitAutoData] + public void DateIso8601_ReturnsIso8601FormattedDate(EventMessage eventMessage) + { + var testDate = new DateTime(2025, 10, 27, 13, 30, 0, DateTimeKind.Utc); + eventMessage.Date = testDate; + var sut = new IntegrationTemplateContext(eventMessage); + + var result = sut.DateIso8601; + + Assert.Equal("2025-10-27T13:30:00.0000000Z", result); + // Verify it's valid ISO 8601 + Assert.True(DateTime.TryParse(result, out _)); + } + [Theory, BitAutoData] public void UserName_WhenUserIsSet_ReturnsName(EventMessage eventMessage, User user) { diff --git a/test/Core.Test/AdminConsole/Services/EventIntegrationEventWriteServiceTests.cs b/test/Core.Test/AdminConsole/Services/EventIntegrationEventWriteServiceTests.cs index 03f9c7764d..16df234004 100644 --- a/test/Core.Test/AdminConsole/Services/EventIntegrationEventWriteServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/EventIntegrationEventWriteServiceTests.cs @@ -38,6 +38,20 @@ public class EventIntegrationEventWriteServiceTests organizationId: Arg.Is(orgId => eventMessage.OrganizationId.ToString().Equals(orgId))); } + [Fact] + public async Task CreateManyAsync_EmptyList_DoesNothing() + { + await Subject.CreateManyAsync([]); + await _eventIntegrationPublisher.DidNotReceiveWithAnyArgs().PublishEventAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task DisposeAsync_DisposesEventIntegrationPublisher() + { + await Subject.DisposeAsync(); + await _eventIntegrationPublisher.Received(1).DisposeAsync(); + } + private static bool AssertJsonStringsMatch(EventMessage expected, string body) { var actual = JsonSerializer.Deserialize(body); diff --git a/test/Core.Test/AdminConsole/Services/EventIntegrationHandlerTests.cs b/test/Core.Test/AdminConsole/Services/EventIntegrationHandlerTests.cs index 89207a9d3a..1d94d58aa5 100644 --- a/test/Core.Test/AdminConsole/Services/EventIntegrationHandlerTests.cs +++ b/test/Core.Test/AdminConsole/Services/EventIntegrationHandlerTests.cs @@ -120,6 +120,16 @@ public class EventIntegrationHandlerTests Assert.Empty(_eventIntegrationPublisher.ReceivedCalls()); } + [Theory, BitAutoData] + public async Task HandleEventAsync_NoOrganizationId_DoesNothing(EventMessage eventMessage) + { + var sutProvider = GetSutProvider(OneConfiguration(_templateBase)); + eventMessage.OrganizationId = null; + + await sutProvider.Sut.HandleEventAsync(eventMessage); + Assert.Empty(_eventIntegrationPublisher.ReceivedCalls()); + } + [Theory, BitAutoData] public async Task HandleEventAsync_BaseTemplateOneConfiguration_PublishesIntegrationMessage(EventMessage eventMessage) { diff --git a/test/Core.Test/AdminConsole/Services/IntegrationFilterServiceTests.cs b/test/Core.Test/AdminConsole/Services/IntegrationFilterServiceTests.cs index 4143469a4b..fb33737c16 100644 --- a/test/Core.Test/AdminConsole/Services/IntegrationFilterServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/IntegrationFilterServiceTests.cs @@ -42,6 +42,35 @@ public class IntegrationFilterServiceTests Assert.True(_service.EvaluateFilterGroup(roundtrippedGroup, eventMessage)); } + [Theory, BitAutoData] + public void EvaluateFilterGroup_EqualsUserIdString_Matches(EventMessage eventMessage) + { + var userId = Guid.NewGuid(); + eventMessage.UserId = userId; + + var group = new IntegrationFilterGroup + { + AndOperator = true, + Rules = + [ + new() + { + Property = "UserId", + Operation = IntegrationFilterOperation.Equals, + Value = userId.ToString() + } + ] + }; + + var result = _service.EvaluateFilterGroup(group, eventMessage); + Assert.True(result); + + var jsonGroup = JsonSerializer.Serialize(group); + var roundtrippedGroup = JsonSerializer.Deserialize(jsonGroup); + Assert.NotNull(roundtrippedGroup); + Assert.True(_service.EvaluateFilterGroup(roundtrippedGroup, eventMessage)); + } + [Theory, BitAutoData] public void EvaluateFilterGroup_EqualsUserId_DoesNotMatch(EventMessage eventMessage) { @@ -281,6 +310,45 @@ public class IntegrationFilterServiceTests Assert.True(_service.EvaluateFilterGroup(roundtrippedGroup, eventMessage)); } + + [Theory, BitAutoData] + public void EvaluateFilterGroup_NestedGroups_AnyMatch(EventMessage eventMessage) + { + var id = Guid.NewGuid(); + var collectionId = Guid.NewGuid(); + eventMessage.UserId = id; + eventMessage.CollectionId = collectionId; + + var nestedGroup = new IntegrationFilterGroup + { + AndOperator = false, + Rules = + [ + new() { Property = "UserId", Operation = IntegrationFilterOperation.Equals, Value = id }, + new() + { + Property = "CollectionId", + Operation = IntegrationFilterOperation.In, + Value = new Guid?[] { Guid.NewGuid() } + } + ] + }; + + var topGroup = new IntegrationFilterGroup + { + AndOperator = false, + Groups = [nestedGroup] + }; + + var result = _service.EvaluateFilterGroup(topGroup, eventMessage); + Assert.True(result); + + var jsonGroup = JsonSerializer.Serialize(topGroup); + var roundtrippedGroup = JsonSerializer.Deserialize(jsonGroup); + Assert.NotNull(roundtrippedGroup); + Assert.True(_service.EvaluateFilterGroup(roundtrippedGroup, eventMessage)); + } + [Theory, BitAutoData] public void EvaluateFilterGroup_UnknownProperty_ReturnsFalse(EventMessage eventMessage) { diff --git a/test/Core.Test/AdminConsole/Services/SlackIntegrationHandlerTests.cs b/test/Core.Test/AdminConsole/Services/SlackIntegrationHandlerTests.cs index dab6c41b61..e2e459ceb3 100644 --- a/test/Core.Test/AdminConsole/Services/SlackIntegrationHandlerTests.cs +++ b/test/Core.Test/AdminConsole/Services/SlackIntegrationHandlerTests.cs @@ -1,4 +1,5 @@ using Bit.Core.AdminConsole.Models.Data.EventIntegrations; +using Bit.Core.Models.Slack; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -28,6 +29,9 @@ public class SlackIntegrationHandlerTests var sutProvider = GetSutProvider(); message.Configuration = new SlackIntegrationConfigurationDetails(_channelId, _token); + _slackService.SendSlackMessageByChannelIdAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new SlackSendMessageResponse() { Ok = true, Channel = _channelId }); + var result = await sutProvider.Sut.HandleAsync(message); Assert.True(result.Success); @@ -39,4 +43,97 @@ public class SlackIntegrationHandlerTests Arg.Is(AssertHelper.AssertPropertyEqual(_channelId)) ); } + + [Theory] + [InlineData("service_unavailable")] + [InlineData("ratelimited")] + [InlineData("rate_limited")] + [InlineData("internal_error")] + [InlineData("message_limit_exceeded")] + public async Task HandleAsync_FailedRetryableRequest_ReturnsFailureWithRetryable(string error) + { + var sutProvider = GetSutProvider(); + var message = new IntegrationMessage() + { + Configuration = new SlackIntegrationConfigurationDetails(_channelId, _token), + MessageId = "MessageId", + RenderedTemplate = "Test Message" + }; + + _slackService.SendSlackMessageByChannelIdAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new SlackSendMessageResponse() { Ok = false, Channel = _channelId, Error = error }); + + var result = await sutProvider.Sut.HandleAsync(message); + + Assert.False(result.Success); + Assert.True(result.Retryable); + Assert.NotNull(result.FailureReason); + Assert.Equal(result.Message, message); + + await sutProvider.GetDependency().Received(1).SendSlackMessageByChannelIdAsync( + Arg.Is(AssertHelper.AssertPropertyEqual(_token)), + Arg.Is(AssertHelper.AssertPropertyEqual(message.RenderedTemplate)), + Arg.Is(AssertHelper.AssertPropertyEqual(_channelId)) + ); + } + + [Theory] + [InlineData("access_denied")] + [InlineData("channel_not_found")] + [InlineData("token_expired")] + [InlineData("token_revoked")] + public async Task HandleAsync_FailedNonRetryableRequest_ReturnsNonRetryableFailure(string error) + { + var sutProvider = GetSutProvider(); + var message = new IntegrationMessage() + { + Configuration = new SlackIntegrationConfigurationDetails(_channelId, _token), + MessageId = "MessageId", + RenderedTemplate = "Test Message" + }; + + _slackService.SendSlackMessageByChannelIdAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new SlackSendMessageResponse() { Ok = false, Channel = _channelId, Error = error }); + + var result = await sutProvider.Sut.HandleAsync(message); + + Assert.False(result.Success); + Assert.False(result.Retryable); + Assert.NotNull(result.FailureReason); + Assert.Equal(result.Message, message); + + await sutProvider.GetDependency().Received(1).SendSlackMessageByChannelIdAsync( + Arg.Is(AssertHelper.AssertPropertyEqual(_token)), + Arg.Is(AssertHelper.AssertPropertyEqual(message.RenderedTemplate)), + Arg.Is(AssertHelper.AssertPropertyEqual(_channelId)) + ); + } + + [Fact] + public async Task HandleAsync_NullResponse_ReturnsNonRetryableFailure() + { + var sutProvider = GetSutProvider(); + var message = new IntegrationMessage() + { + Configuration = new SlackIntegrationConfigurationDetails(_channelId, _token), + MessageId = "MessageId", + RenderedTemplate = "Test Message" + }; + + _slackService.SendSlackMessageByChannelIdAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns((SlackSendMessageResponse?)null); + + var result = await sutProvider.Sut.HandleAsync(message); + + Assert.False(result.Success); + Assert.False(result.Retryable); + Assert.Equal("Slack response was null", result.FailureReason); + Assert.Equal(result.Message, message); + + await sutProvider.GetDependency().Received(1).SendSlackMessageByChannelIdAsync( + Arg.Is(AssertHelper.AssertPropertyEqual(_token)), + Arg.Is(AssertHelper.AssertPropertyEqual(message.RenderedTemplate)), + Arg.Is(AssertHelper.AssertPropertyEqual(_channelId)) + ); + } } diff --git a/test/Core.Test/AdminConsole/Services/SlackServiceTests.cs b/test/Core.Test/AdminConsole/Services/SlackServiceTests.cs index 48dd9c490e..068e5e8c82 100644 --- a/test/Core.Test/AdminConsole/Services/SlackServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/SlackServiceTests.cs @@ -146,6 +146,27 @@ public class SlackServiceTests Assert.Empty(result); } + [Fact] + public async Task GetChannelIdAsync_NoChannelFound_ReturnsEmptyResult() + { + var emptyResponse = JsonSerializer.Serialize( + new + { + ok = true, + channels = Array.Empty(), + response_metadata = new { next_cursor = "" } + }); + + _handler.When(HttpMethod.Get) + .RespondWith(HttpStatusCode.OK) + .WithContent(new StringContent(emptyResponse)); + + var sutProvider = GetSutProvider(); + var result = await sutProvider.Sut.GetChannelIdAsync(_token, "general"); + + Assert.Empty(result); + } + [Fact] public async Task GetChannelIdAsync_ReturnsCorrectChannelId() { @@ -235,6 +256,32 @@ public class SlackServiceTests Assert.Equal(string.Empty, result); } + [Fact] + public async Task GetDmChannelByEmailAsync_ApiErrorUnparsableDmResponse_ReturnsEmptyString() + { + var sutProvider = GetSutProvider(); + var email = "user@example.com"; + var userId = "U12345"; + + var userResponse = new + { + ok = true, + user = new { id = userId } + }; + + _handler.When($"https://slack.com/api/users.lookupByEmail?email={email}") + .RespondWith(HttpStatusCode.OK) + .WithContent(new StringContent(JsonSerializer.Serialize(userResponse))); + + _handler.When("https://slack.com/api/conversations.open") + .RespondWith(HttpStatusCode.OK) + .WithContent(new StringContent("NOT JSON")); + + var result = await sutProvider.Sut.GetDmChannelByEmailAsync(_token, email); + + Assert.Equal(string.Empty, result); + } + [Fact] public async Task GetDmChannelByEmailAsync_ApiErrorUserResponse_ReturnsEmptyString() { @@ -244,7 +291,7 @@ public class SlackServiceTests var userResponse = new { ok = false, - error = "An error occured" + error = "An error occurred" }; _handler.When($"https://slack.com/api/users.lookupByEmail?email={email}") @@ -256,6 +303,21 @@ public class SlackServiceTests Assert.Equal(string.Empty, result); } + [Fact] + public async Task GetDmChannelByEmailAsync_ApiErrorUnparsableUserResponse_ReturnsEmptyString() + { + var sutProvider = GetSutProvider(); + var email = "user@example.com"; + + _handler.When($"https://slack.com/api/users.lookupByEmail?email={email}") + .RespondWith(HttpStatusCode.OK) + .WithContent(new StringContent("Not JSON")); + + var result = await sutProvider.Sut.GetDmChannelByEmailAsync(_token, email); + + Assert.Equal(string.Empty, result); + } + [Fact] public void GetRedirectUrl_ReturnsCorrectUrl() { @@ -341,18 +403,29 @@ public class SlackServiceTests } [Fact] - public async Task SendSlackMessageByChannelId_Sends_Correct_Message() + public async Task SendSlackMessageByChannelId_Success_ReturnsSuccessfulResponse() { var sutProvider = GetSutProvider(); var channelId = "C12345"; var message = "Hello, Slack!"; + var jsonResponse = JsonSerializer.Serialize(new + { + ok = true, + channel = channelId, + }); + _handler.When(HttpMethod.Post) .RespondWith(HttpStatusCode.OK) - .WithContent(new StringContent(string.Empty)); + .WithContent(new StringContent(jsonResponse)); - await sutProvider.Sut.SendSlackMessageByChannelIdAsync(_token, message, channelId); + var result = await sutProvider.Sut.SendSlackMessageByChannelIdAsync(_token, message, channelId); + // Response was parsed correctly + Assert.NotNull(result); + Assert.True(result.Ok); + + // Request was sent correctly Assert.Single(_handler.CapturedRequests); var request = _handler.CapturedRequests[0]; Assert.NotNull(request); @@ -365,4 +438,62 @@ public class SlackServiceTests Assert.Equal(message, json.RootElement.GetProperty("text").GetString() ?? string.Empty); Assert.Equal(channelId, json.RootElement.GetProperty("channel").GetString() ?? string.Empty); } + + [Fact] + public async Task SendSlackMessageByChannelId_Failure_ReturnsErrorResponse() + { + var sutProvider = GetSutProvider(); + var channelId = "C12345"; + var message = "Hello, Slack!"; + + var jsonResponse = JsonSerializer.Serialize(new + { + ok = false, + channel = channelId, + error = "error" + }); + + _handler.When(HttpMethod.Post) + .RespondWith(HttpStatusCode.OK) + .WithContent(new StringContent(jsonResponse)); + + var result = await sutProvider.Sut.SendSlackMessageByChannelIdAsync(_token, message, channelId); + + // Response was parsed correctly + Assert.NotNull(result); + Assert.False(result.Ok); + Assert.NotNull(result.Error); + } + + [Fact] + public async Task SendSlackMessageByChannelIdAsync_InvalidJson_ReturnsNull() + { + var sutProvider = GetSutProvider(); + var channelId = "C12345"; + var message = "Hello world!"; + + _handler.When(HttpMethod.Post) + .RespondWith(HttpStatusCode.OK) + .WithContent(new StringContent("Not JSON")); + + var result = await sutProvider.Sut.SendSlackMessageByChannelIdAsync(_token, message, channelId); + + Assert.Null(result); + } + + [Fact] + public async Task SendSlackMessageByChannelIdAsync_HttpServerError_ReturnsNull() + { + var sutProvider = GetSutProvider(); + var channelId = "C12345"; + var message = "Hello world!"; + + _handler.When(HttpMethod.Post) + .RespondWith(HttpStatusCode.InternalServerError) + .WithContent(new StringContent(string.Empty)); + + var result = await sutProvider.Sut.SendSlackMessageByChannelIdAsync(_token, message, channelId); + + Assert.Null(result); + } } From 4fac63527270a99b614029e137ff1ee7e494067d Mon Sep 17 00:00:00 2001 From: Brant DeBow <125889545+brant-livefront@users.noreply.github.com> Date: Mon, 10 Nov 2025 14:57:04 -0500 Subject: [PATCH 6/8] Remove EventBasedOrganizationIntegrations feature flag (#6538) * Remove EventBasedOrganizationIntegrations feature flag * Remove unnecessary nullable enable * Refactored service collection extensions to follow a more direct path: ASB, RabbitMQ, Azure Queue, Repository, No-op * Use TryAdd instead of Add --- ...ationIntegrationConfigurationController.cs | 3 - .../OrganizationIntegrationController.cs | 5 -- .../Controllers/SlackIntegrationController.cs | 3 - .../Controllers/TeamsIntegrationController.cs | 3 - .../EventIntegrations/EventRouteService.cs | 34 ---------- src/Core/Constants.cs | 1 - .../Utilities/ServiceCollectionExtensions.cs | 57 +++++++--------- .../Services/EventRouteServiceTests.cs | 65 ------------------- 8 files changed, 24 insertions(+), 147 deletions(-) delete mode 100644 src/Core/AdminConsole/Services/Implementations/EventIntegrations/EventRouteService.cs delete mode 100644 test/Core.Test/AdminConsole/Services/EventRouteServiceTests.cs diff --git a/src/Api/AdminConsole/Controllers/OrganizationIntegrationConfigurationController.cs b/src/Api/AdminConsole/Controllers/OrganizationIntegrationConfigurationController.cs index ae0f91d355..0b7fe8dffe 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationIntegrationConfigurationController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationIntegrationConfigurationController.cs @@ -1,16 +1,13 @@ using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.AdminConsole.Models.Response.Organizations; -using Bit.Core; using Bit.Core.Context; using Bit.Core.Exceptions; using Bit.Core.Repositories; -using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; namespace Bit.Api.AdminConsole.Controllers; -[RequireFeature(FeatureFlagKeys.EventBasedOrganizationIntegrations)] [Route("organizations/{organizationId:guid}/integrations/{integrationId:guid}/configurations")] [Authorize("Application")] public class OrganizationIntegrationConfigurationController( diff --git a/src/Api/AdminConsole/Controllers/OrganizationIntegrationController.cs b/src/Api/AdminConsole/Controllers/OrganizationIntegrationController.cs index a12492949d..181811e892 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationIntegrationController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationIntegrationController.cs @@ -1,18 +1,13 @@ using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.AdminConsole.Models.Response.Organizations; -using Bit.Core; using Bit.Core.Context; using Bit.Core.Exceptions; using Bit.Core.Repositories; -using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; -#nullable enable - namespace Bit.Api.AdminConsole.Controllers; -[RequireFeature(FeatureFlagKeys.EventBasedOrganizationIntegrations)] [Route("organizations/{organizationId:guid}/integrations")] [Authorize("Application")] public class OrganizationIntegrationController( diff --git a/src/Api/AdminConsole/Controllers/SlackIntegrationController.cs b/src/Api/AdminConsole/Controllers/SlackIntegrationController.cs index 08635878de..7b53f73f81 100644 --- a/src/Api/AdminConsole/Controllers/SlackIntegrationController.cs +++ b/src/Api/AdminConsole/Controllers/SlackIntegrationController.cs @@ -1,6 +1,5 @@ using System.Text.Json; using Bit.Api.AdminConsole.Models.Response.Organizations; -using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Models.Data.EventIntegrations; using Bit.Core.Context; @@ -8,13 +7,11 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; namespace Bit.Api.AdminConsole.Controllers; -[RequireFeature(FeatureFlagKeys.EventBasedOrganizationIntegrations)] [Route("organizations")] [Authorize("Application")] public class SlackIntegrationController( diff --git a/src/Api/AdminConsole/Controllers/TeamsIntegrationController.cs b/src/Api/AdminConsole/Controllers/TeamsIntegrationController.cs index 8cafb6b2cf..36d107bbcc 100644 --- a/src/Api/AdminConsole/Controllers/TeamsIntegrationController.cs +++ b/src/Api/AdminConsole/Controllers/TeamsIntegrationController.cs @@ -1,6 +1,5 @@ using System.Text.Json; using Bit.Api.AdminConsole.Models.Response.Organizations; -using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Models.Data.EventIntegrations; using Bit.Core.Context; @@ -8,7 +7,6 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.Bot.Builder; @@ -16,7 +14,6 @@ using Microsoft.Bot.Builder.Integration.AspNet.Core; namespace Bit.Api.AdminConsole.Controllers; -[RequireFeature(FeatureFlagKeys.EventBasedOrganizationIntegrations)] [Route("organizations")] [Authorize("Application")] public class TeamsIntegrationController( diff --git a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/EventRouteService.cs b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/EventRouteService.cs deleted file mode 100644 index a542e75a7b..0000000000 --- a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/EventRouteService.cs +++ /dev/null @@ -1,34 +0,0 @@ -using Bit.Core.Models.Data; -using Microsoft.Extensions.DependencyInjection; - -namespace Bit.Core.Services; - -public class EventRouteService( - [FromKeyedServices("broadcast")] IEventWriteService broadcastEventWriteService, - [FromKeyedServices("storage")] IEventWriteService storageEventWriteService, - IFeatureService _featureService) : IEventWriteService -{ - public async Task CreateAsync(IEvent e) - { - if (_featureService.IsEnabled(FeatureFlagKeys.EventBasedOrganizationIntegrations)) - { - await broadcastEventWriteService.CreateAsync(e); - } - else - { - await storageEventWriteService.CreateAsync(e); - } - } - - public async Task CreateManyAsync(IEnumerable e) - { - if (_featureService.IsEnabled(FeatureFlagKeys.EventBasedOrganizationIntegrations)) - { - await broadcastEventWriteService.CreateManyAsync(e); - } - else - { - await storageEventWriteService.CreateManyAsync(e); - } - } -} diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index c5b6bbc10d..ad61d52a38 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -137,7 +137,6 @@ public static class FeatureFlagKeys /* Admin Console Team */ public const string PolicyRequirements = "pm-14439-policy-requirements"; public const string ScimInviteUserOptimization = "pm-16811-optimize-invite-user-flow-to-fail-fast"; - public const string EventBasedOrganizationIntegrations = "event-based-organization-integrations"; public const string CreateDefaultLocation = "pm-19467-create-default-location"; public const string AutomaticConfirmUsers = "pm-19934-auto-confirm-organization-users"; public const string PM23845_VNextApplicationCache = "pm-24957-refactor-memory-application-cache"; diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index ef143b042c..78b8a61015 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -524,42 +524,33 @@ public static class ServiceCollectionExtensions public static IServiceCollection AddEventWriteServices(this IServiceCollection services, GlobalSettings globalSettings) { - if (!globalSettings.SelfHosted && CoreHelpers.SettingHasValue(globalSettings.Events.ConnectionString)) + if (IsAzureServiceBusEnabled(globalSettings)) { - services.TryAddKeyedSingleton("storage"); - - if (CoreHelpers.SettingHasValue(globalSettings.EventLogging.AzureServiceBus.ConnectionString) && - CoreHelpers.SettingHasValue(globalSettings.EventLogging.AzureServiceBus.EventTopicName)) - { - services.TryAddSingleton(); - services.TryAddKeyedSingleton("broadcast"); - } - else - { - services.TryAddKeyedSingleton("broadcast"); - } - } - else if (globalSettings.SelfHosted) - { - services.TryAddKeyedSingleton("storage"); - - if (IsRabbitMqEnabled(globalSettings)) - { - services.TryAddSingleton(); - services.TryAddKeyedSingleton("broadcast"); - } - else - { - services.TryAddKeyedSingleton("broadcast"); - } - } - else - { - services.TryAddKeyedSingleton("storage"); - services.TryAddKeyedSingleton("broadcast"); + services.TryAddSingleton(); + services.TryAddSingleton(); + return services; } - services.TryAddScoped(); + if (IsRabbitMqEnabled(globalSettings)) + { + services.TryAddSingleton(); + services.TryAddSingleton(); + return services; + } + + if (CoreHelpers.SettingHasValue(globalSettings.Events.ConnectionString)) + { + services.TryAddSingleton(); + return services; + } + + if (globalSettings.SelfHosted) + { + services.TryAddSingleton(); + return services; + } + + services.TryAddSingleton(); return services; } diff --git a/test/Core.Test/AdminConsole/Services/EventRouteServiceTests.cs b/test/Core.Test/AdminConsole/Services/EventRouteServiceTests.cs deleted file mode 100644 index 1a42d846f2..0000000000 --- a/test/Core.Test/AdminConsole/Services/EventRouteServiceTests.cs +++ /dev/null @@ -1,65 +0,0 @@ -using Bit.Core.Models.Data; -using Bit.Core.Services; -using Bit.Test.Common.AutoFixture.Attributes; -using NSubstitute; -using Xunit; - -namespace Bit.Core.Test.Services; - -[SutProviderCustomize] -public class EventRouteServiceTests -{ - private readonly IEventWriteService _broadcastEventWriteService = Substitute.For(); - private readonly IEventWriteService _storageEventWriteService = Substitute.For(); - private readonly IFeatureService _featureService = Substitute.For(); - private readonly EventRouteService Subject; - - public EventRouteServiceTests() - { - Subject = new EventRouteService(_broadcastEventWriteService, _storageEventWriteService, _featureService); - } - - [Theory, BitAutoData] - public async Task CreateAsync_FlagDisabled_EventSentToStorageService(EventMessage eventMessage) - { - _featureService.IsEnabled(FeatureFlagKeys.EventBasedOrganizationIntegrations).Returns(false); - - await Subject.CreateAsync(eventMessage); - - await _broadcastEventWriteService.DidNotReceiveWithAnyArgs().CreateAsync(Arg.Any()); - await _storageEventWriteService.Received(1).CreateAsync(eventMessage); - } - - [Theory, BitAutoData] - public async Task CreateAsync_FlagEnabled_EventSentToBroadcastService(EventMessage eventMessage) - { - _featureService.IsEnabled(FeatureFlagKeys.EventBasedOrganizationIntegrations).Returns(true); - - await Subject.CreateAsync(eventMessage); - - await _broadcastEventWriteService.Received(1).CreateAsync(eventMessage); - await _storageEventWriteService.DidNotReceiveWithAnyArgs().CreateAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task CreateManyAsync_FlagDisabled_EventsSentToStorageService(IEnumerable eventMessages) - { - _featureService.IsEnabled(FeatureFlagKeys.EventBasedOrganizationIntegrations).Returns(false); - - await Subject.CreateManyAsync(eventMessages); - - await _broadcastEventWriteService.DidNotReceiveWithAnyArgs().CreateManyAsync(Arg.Any>()); - await _storageEventWriteService.Received(1).CreateManyAsync(eventMessages); - } - - [Theory, BitAutoData] - public async Task CreateManyAsync_FlagEnabled_EventsSentToBroadcastService(IEnumerable eventMessages) - { - _featureService.IsEnabled(FeatureFlagKeys.EventBasedOrganizationIntegrations).Returns(true); - - await Subject.CreateManyAsync(eventMessages); - - await _broadcastEventWriteService.Received(1).CreateManyAsync(eventMessages); - await _storageEventWriteService.DidNotReceiveWithAnyArgs().CreateManyAsync(Arg.Any>()); - } -} From 4de10c830d3e2f8eaf5192aeca8e9fcb10e7ca3d Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Mon, 10 Nov 2025 14:46:52 -0600 Subject: [PATCH 7/8] [PM-26636] Set Key when Confirming (#6550) * When confirming a uesr, we need to set the key. :face_palm: * Adding default value. --- .../OrganizationUserRepository.cs | 3 +- .../OrganizationUserRepository.cs | 5 ++-- .../OrganizationUser_ConfirmById.sql | 6 ++-- .../OrganizationUserRepositoryTests.cs | 3 ++ .../2025-11-06_00_ConfirmOrgUser_AddKey.sql | 30 +++++++++++++++++++ 5 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 util/Migrator/DbScripts/2025-11-06_00_ConfirmOrgUser_AddKey.sql diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs index dc4fc74ff8..ed5708844d 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -681,7 +681,8 @@ public class OrganizationUserRepository : Repository, IO { organizationUser.Id, organizationUser.UserId, - RevisionDate = DateTime.UtcNow.Date + RevisionDate = DateTime.UtcNow.Date, + Key = organizationUser.Key }); return rowCount > 0; diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs index b871ec44bf..e5016a20d4 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -950,8 +950,9 @@ public class OrganizationUserRepository : Repository ou.Id == organizationUser.Id && ou.Status == OrganizationUserStatusType.Accepted) - .ExecuteUpdateAsync(x => - x.SetProperty(y => y.Status, OrganizationUserStatusType.Confirmed)); + .ExecuteUpdateAsync(x => x + .SetProperty(y => y.Status, OrganizationUserStatusType.Confirmed) + .SetProperty(y => y.Key, organizationUser.Key)); if (result <= 0) { diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_ConfirmById.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_ConfirmById.sql index 004f1c93eb..7a1cd78a51 100644 --- a/src/Sql/dbo/Stored Procedures/OrganizationUser_ConfirmById.sql +++ b/src/Sql/dbo/Stored Procedures/OrganizationUser_ConfirmById.sql @@ -1,7 +1,8 @@ CREATE PROCEDURE [dbo].[OrganizationUser_ConfirmById] @Id UNIQUEIDENTIFIER, @UserId UNIQUEIDENTIFIER, - @RevisionDate DATETIME2(7) + @RevisionDate DATETIME2(7), + @Key NVARCHAR(MAX) = NULL AS BEGIN SET NOCOUNT ON @@ -12,7 +13,8 @@ BEGIN [dbo].[OrganizationUser] SET [Status] = 2, -- Set to Confirmed - [RevisionDate] = @RevisionDate + [RevisionDate] = @RevisionDate, + [Key] = @Key WHERE [Id] = @Id AND [Status] = 1 -- Only update if status is Accepted diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs index 798571df17..157d6a2589 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs @@ -1484,6 +1484,8 @@ public class OrganizationUserRepositoryTests var organization = await organizationRepository.CreateTestOrganizationAsync(); var user = await userRepository.CreateTestUserAsync(); var orgUser = await organizationUserRepository.CreateAcceptedTestOrganizationUserAsync(organization, user); + const string key = "test-key"; + orgUser.Key = key; // Act var result = await organizationUserRepository.ConfirmOrganizationUserAsync(orgUser); @@ -1493,6 +1495,7 @@ public class OrganizationUserRepositoryTests var updatedUser = await organizationUserRepository.GetByIdAsync(orgUser.Id); Assert.NotNull(updatedUser); Assert.Equal(OrganizationUserStatusType.Confirmed, updatedUser.Status); + Assert.Equal(key, updatedUser.Key); // Annul await organizationRepository.DeleteAsync(organization); diff --git a/util/Migrator/DbScripts/2025-11-06_00_ConfirmOrgUser_AddKey.sql b/util/Migrator/DbScripts/2025-11-06_00_ConfirmOrgUser_AddKey.sql new file mode 100644 index 0000000000..6cf879ee45 --- /dev/null +++ b/util/Migrator/DbScripts/2025-11-06_00_ConfirmOrgUser_AddKey.sql @@ -0,0 +1,30 @@ +CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_ConfirmById] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER, + @RevisionDate DATETIME2(7), + @Key NVARCHAR(MAX) = NULL +AS +BEGIN + SET NOCOUNT ON + + DECLARE @RowCount INT; + + UPDATE + [dbo].[OrganizationUser] + SET + [Status] = 2, -- Set to Confirmed + [RevisionDate] = @RevisionDate, + [Key] = @Key + WHERE + [Id] = @Id + AND [Status] = 1 -- Only update if status is Accepted + + SET @RowCount = @@ROWCOUNT; + + IF @RowCount > 0 + BEGIN + EXEC [dbo].[User_BumpAccountRevisionDate] @UserId + END + + SELECT @RowCount; +END From db36c52c62b7b8174d948b4c6ac78cdca66828a3 Mon Sep 17 00:00:00 2001 From: Stephon Brown Date: Mon, 10 Nov 2025 16:07:14 -0500 Subject: [PATCH 8/8] Milestone 2C Update (#6560) * fix(billing): milestone update * tests(billing): update tests --- .../Services/Implementations/UpcomingInvoiceHandler.cs | 3 ++- src/Core/Billing/Pricing/PricingClient.cs | 4 +--- test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs | 3 ++- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs b/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs index f24229f151..7a58f84cd4 100644 --- a/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs +++ b/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs @@ -177,7 +177,8 @@ public class UpcomingInvoiceHandler( Discounts = [ new SubscriptionDiscountOptions { Coupon = CouponIDs.Milestone2SubscriptionDiscount } - ] + ], + ProrationBehavior = "none" }); } catch (Exception exception) diff --git a/src/Core/Billing/Pricing/PricingClient.cs b/src/Core/Billing/Pricing/PricingClient.cs index 0c4266665a..1ec44c6496 100644 --- a/src/Core/Billing/Pricing/PricingClient.cs +++ b/src/Core/Billing/Pricing/PricingClient.cs @@ -123,9 +123,7 @@ public class PricingClient( return [CurrentPremiumPlan]; } - var milestone2Feature = featureService.IsEnabled(FeatureFlagKeys.PM23341_Milestone_2); - - var response = await httpClient.GetAsync($"plans/premium?milestone2={milestone2Feature}"); + var response = await httpClient.GetAsync("plans/premium"); if (response.IsSuccessStatusCode) { diff --git a/test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs b/test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs index 899df4ea53..913355f2db 100644 --- a/test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs +++ b/test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs @@ -268,7 +268,8 @@ public class UpcomingInvoiceHandlerTests Arg.Is("sub_123"), Arg.Is(o => o.Items[0].Id == priceSubscriptionId && - o.Items[0].Price == priceId)); + o.Items[0].Price == priceId && + o.ProrationBehavior == "none")); // Verify the updated invoice email was sent await _mailer.Received(1).SendEmail(