From aa33a67aeeeea602dbe96a483e1f69a94744fce4 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 16 Jan 2026 10:33:17 -0500 Subject: [PATCH 01/51] [PM-30858] Fix excessive logs (#6860) * Add tests showing issue & workaround - `AddSerilogFileLogging_LegacyConfig_InfoLogs_DoNotFillUpFile` fails - `AddSerilogFileLogging_LegacyConfig_WithLevelCustomization_InfoLogs_DoNotFillUpFile` fails - `AddSerilogFileLogging_NewConfig_InfoLogs_DoNotFillUpFile` fails - `AddSerilogFileLogging_NewConfig_WithLevelCustomization_InfoLogs_DoNotFillUpFile` works * Allow customization of LogLevel with legacy path format config * Lower default logging levels * Delete tests now that log levels have been customized --- .../src/Scim/appsettings.Production.json | 6 +- src/Admin/appsettings.Production.json | 6 +- src/Api/appsettings.Production.json | 6 +- src/Core/Utilities/LoggerFactoryExtensions.cs | 38 +++++++----- src/Events/appsettings.Production.json | 6 +- .../appsettings.Production.json | 6 +- src/Icons/appsettings.Production.json | 6 +- src/Identity/appsettings.Production.json | 6 +- src/Notifications/appsettings.Production.json | 6 +- .../Utilities/LoggerFactoryExtensionsTests.cs | 59 ++++++++++++++++++- 10 files changed, 96 insertions(+), 49 deletions(-) diff --git a/bitwarden_license/src/Scim/appsettings.Production.json b/bitwarden_license/src/Scim/appsettings.Production.json index d9efbcda12..a6578c08dc 100644 --- a/bitwarden_license/src/Scim/appsettings.Production.json +++ b/bitwarden_license/src/Scim/appsettings.Production.json @@ -23,11 +23,9 @@ } }, "Logging": { - "IncludeScopes": false, "LogLevel": { - "Default": "Debug", - "System": "Information", - "Microsoft": "Information" + "Default": "Information", + "Microsoft.AspNetCore": "Warning" }, "Console": { "IncludeScopes": true, diff --git a/src/Admin/appsettings.Production.json b/src/Admin/appsettings.Production.json index 9f797f3111..1d852abfed 100644 --- a/src/Admin/appsettings.Production.json +++ b/src/Admin/appsettings.Production.json @@ -20,11 +20,9 @@ } }, "Logging": { - "IncludeScopes": false, "LogLevel": { - "Default": "Debug", - "System": "Information", - "Microsoft": "Information" + "Default": "Information", + "Microsoft.AspNetCore": "Warning" }, "Console": { "IncludeScopes": true, diff --git a/src/Api/appsettings.Production.json b/src/Api/appsettings.Production.json index d9efbcda12..a6578c08dc 100644 --- a/src/Api/appsettings.Production.json +++ b/src/Api/appsettings.Production.json @@ -23,11 +23,9 @@ } }, "Logging": { - "IncludeScopes": false, "LogLevel": { - "Default": "Debug", - "System": "Information", - "Microsoft": "Information" + "Default": "Information", + "Microsoft.AspNetCore": "Warning" }, "Console": { "IncludeScopes": true, diff --git a/src/Core/Utilities/LoggerFactoryExtensions.cs b/src/Core/Utilities/LoggerFactoryExtensions.cs index b950e30d5d..f3330f0792 100644 --- a/src/Core/Utilities/LoggerFactoryExtensions.cs +++ b/src/Core/Utilities/LoggerFactoryExtensions.cs @@ -1,4 +1,5 @@ -using Microsoft.AspNetCore.Hosting; +using System.Globalization; +using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -8,7 +9,7 @@ namespace Bit.Core.Utilities; public static class LoggerFactoryExtensions { /// - /// + /// /// /// /// @@ -21,10 +22,12 @@ public static class LoggerFactoryExtensions return; } + IConfiguration loggingConfiguration; + // If they have begun using the new settings location, use that if (!string.IsNullOrEmpty(context.Configuration["Logging:PathFormat"])) { - logging.AddFile(context.Configuration.GetSection("Logging")); + loggingConfiguration = context.Configuration.GetSection("Logging"); } else { @@ -40,28 +43,35 @@ public static class LoggerFactoryExtensions var projectName = loggingOptions.ProjectName ?? context.HostingEnvironment.ApplicationName; + string pathFormat; + if (loggingOptions.LogRollBySizeLimit.HasValue) { - var pathFormat = loggingOptions.LogDirectoryByProject + pathFormat = loggingOptions.LogDirectoryByProject ? Path.Combine(loggingOptions.LogDirectory, projectName, "log.txt") : Path.Combine(loggingOptions.LogDirectory, $"{projectName.ToLowerInvariant()}.log"); - - logging.AddFile( - pathFormat: pathFormat, - fileSizeLimitBytes: loggingOptions.LogRollBySizeLimit.Value - ); } else { - var pathFormat = loggingOptions.LogDirectoryByProject + pathFormat = loggingOptions.LogDirectoryByProject ? Path.Combine(loggingOptions.LogDirectory, projectName, "{Date}.txt") : Path.Combine(loggingOptions.LogDirectory, $"{projectName.ToLowerInvariant()}_{{Date}}.log"); - - logging.AddFile( - pathFormat: pathFormat - ); } + + // We want to rely on Serilog using the configuration section to have customization of the log levels + // so we make a custom configuration source for them based on the legacy values and allow overrides from + // the new location. + loggingConfiguration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + {"PathFormat", pathFormat}, + {"FileSizeLimitBytes", loggingOptions.LogRollBySizeLimit?.ToString(CultureInfo.InvariantCulture)} + }) + .AddConfiguration(context.Configuration.GetSection("Logging")) + .Build(); } + + logging.AddFile(loggingConfiguration); }); } diff --git a/src/Events/appsettings.Production.json b/src/Events/appsettings.Production.json index 010f02f8cd..9a10621264 100644 --- a/src/Events/appsettings.Production.json +++ b/src/Events/appsettings.Production.json @@ -17,11 +17,9 @@ } }, "Logging": { - "IncludeScopes": false, "LogLevel": { - "Default": "Debug", - "System": "Information", - "Microsoft": "Information" + "Default": "Information", + "Microsoft.AspNetCore": "Warning" }, "Console": { "IncludeScopes": true, diff --git a/src/EventsProcessor/appsettings.Production.json b/src/EventsProcessor/appsettings.Production.json index 1cce4a9ed3..d57bf98b55 100644 --- a/src/EventsProcessor/appsettings.Production.json +++ b/src/EventsProcessor/appsettings.Production.json @@ -1,10 +1,8 @@ { "Logging": { - "IncludeScopes": false, "LogLevel": { - "Default": "Debug", - "System": "Information", - "Microsoft": "Information" + "Default": "Information", + "Microsoft.AspNetCore": "Warning" }, "Console": { "IncludeScopes": true, diff --git a/src/Icons/appsettings.Production.json b/src/Icons/appsettings.Production.json index 828e8c61cc..19d21f7260 100644 --- a/src/Icons/appsettings.Production.json +++ b/src/Icons/appsettings.Production.json @@ -17,11 +17,9 @@ } }, "Logging": { - "IncludeScopes": false, "LogLevel": { - "Default": "Debug", - "System": "Information", - "Microsoft": "Information" + "Default": "Information", + "Microsoft.AspNetCore": "Warning" }, "Console": { "IncludeScopes": true, diff --git a/src/Identity/appsettings.Production.json b/src/Identity/appsettings.Production.json index 4897a7d8b1..14471b5fb6 100644 --- a/src/Identity/appsettings.Production.json +++ b/src/Identity/appsettings.Production.json @@ -20,11 +20,9 @@ } }, "Logging": { - "IncludeScopes": false, "LogLevel": { - "Default": "Debug", - "System": "Information", - "Microsoft": "Information" + "Default": "Information", + "Microsoft.AspNetCore": "Warning" }, "Console": { "IncludeScopes": true, diff --git a/src/Notifications/appsettings.Production.json b/src/Notifications/appsettings.Production.json index 010f02f8cd..735c70e481 100644 --- a/src/Notifications/appsettings.Production.json +++ b/src/Notifications/appsettings.Production.json @@ -17,11 +17,9 @@ } }, "Logging": { - "IncludeScopes": false, "LogLevel": { - "Default": "Debug", - "System": "Information", - "Microsoft": "Information" + "Default": "Information", + "Microsoft": "Warning" }, "Console": { "IncludeScopes": true, diff --git a/test/Core.Test/Utilities/LoggerFactoryExtensionsTests.cs b/test/Core.Test/Utilities/LoggerFactoryExtensionsTests.cs index 81311cb802..ffeb3fa2e7 100644 --- a/test/Core.Test/Utilities/LoggerFactoryExtensionsTests.cs +++ b/test/Core.Test/Utilities/LoggerFactoryExtensionsTests.cs @@ -74,8 +74,7 @@ public class LoggerFactoryExtensionsTests logger.LogWarning("This is a test"); - // Writing to the file is buffered, give it a little time to flush - await Task.Delay(5); + await provider.DisposeAsync(); var logFile = Assert.Single(tempDir.EnumerateFiles("Logs/*.log")); @@ -90,13 +89,67 @@ public class LoggerFactoryExtensionsTests logFileContents ); } + + [Fact] + public async Task AddSerilogFileLogging_LegacyConfig_WithLevelCustomization_InfoLogs_DoNotFillUpFile() + { + await AssertSmallFileAsync((tempDir, config) => + { + config["GlobalSettings:LogDirectory"] = tempDir; + config["Logging:LogLevel:Microsoft.AspNetCore"] = "Warning"; + }); + } + + [Fact] + public async Task AddSerilogFileLogging_NewConfig_WithLevelCustomization_InfoLogs_DoNotFillUpFile() + { + await AssertSmallFileAsync((tempDir, config) => + { + config["Logging:PathFormat"] = Path.Combine(tempDir, "log.txt"); + config["Logging:LogLevel:Microsoft.AspNetCore"] = "Warning"; + }); + } + + private static async Task AssertSmallFileAsync(Action> configure) + { + using var tempDir = new TempDirectory(); + var config = new Dictionary(); + + configure(tempDir.Directory, config); + + var provider = GetServiceProvider(config, "Production"); + + var loggerFactory = provider.GetRequiredService(); + var microsoftLogger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Testing"); + + for (var i = 0; i < 100; i++) + { + microsoftLogger.LogInformation("Tons of useless information"); + } + + var otherLogger = loggerFactory.CreateLogger("Bitwarden"); + + for (var i = 0; i < 5; i++) + { + otherLogger.LogInformation("Mildly more useful information but not as frequent."); + } + + await provider.DisposeAsync(); + + var logFiles = Directory.EnumerateFiles(tempDir.Directory, "*.txt", SearchOption.AllDirectories); + var logFile = Assert.Single(logFiles); + + using var fr = File.OpenRead(logFile); + Assert.InRange(fr.Length, 0, 1024); + } + private static IEnumerable GetProviders(Dictionary initialData, string environment = "Production") { var provider = GetServiceProvider(initialData, environment); return provider.GetServices(); } - private static IServiceProvider GetServiceProvider(Dictionary initialData, string environment) + private static ServiceProvider GetServiceProvider(Dictionary initialData, string environment) { var config = new ConfigurationBuilder() .AddInMemoryCollection(initialData) From 8d30fbcc8abe424e604073e721ceaeb365c4fba1 Mon Sep 17 00:00:00 2001 From: Stephon Brown Date: Fri, 16 Jan 2026 18:13:57 -0500 Subject: [PATCH 02/51] Billing/pm 30882/defect pm coupon removed on upgrade (#6863) * fix(billing): update coupon check logic * tests(billing): update tests and add plan check test --- .../SubscriptionUpdatedHandler.cs | 11 ++- .../SubscriptionUpdatedHandlerTests.cs | 89 +++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs b/src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs index c10368d8c0..9e20bd3191 100644 --- a/src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs +++ b/src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs @@ -275,17 +275,24 @@ public class SubscriptionUpdatedHandler : ISubscriptionUpdatedHandler .PreviousAttributes .ToObject() as Subscription; + // Get all plan IDs that include Secrets Manager support to check if the organization has secret manager in the + // previous and/or current subscriptions. + var planIdsOfPlansWithSecretManager = (await _pricingClient.ListPlans()) + .Where(orgPlan => orgPlan.SupportsSecretsManager && orgPlan.SecretsManager.StripeSeatPlanId != null) + .Select(orgPlan => orgPlan.SecretsManager.StripeSeatPlanId) + .ToHashSet(); + // This being false doesn't necessarily mean that the organization doesn't subscribe to Secrets Manager. // If there are changes to any subscription item, Stripe sends every item in the subscription, both // changed and unchanged. var previousSubscriptionHasSecretsManager = previousSubscription?.Items is not null && previousSubscription.Items.Any( - previousSubscriptionItem => previousSubscriptionItem.Plan.Id == plan.SecretsManager.StripeSeatPlanId); + previousSubscriptionItem => planIdsOfPlansWithSecretManager.Contains(previousSubscriptionItem.Plan.Id)); var currentSubscriptionHasSecretsManager = subscription.Items.Any( - currentSubscriptionItem => currentSubscriptionItem.Plan.Id == plan.SecretsManager.StripeSeatPlanId); + currentSubscriptionItem => planIdsOfPlansWithSecretManager.Contains(currentSubscriptionItem.Plan.Id)); if (!previousSubscriptionHasSecretsManager || currentSubscriptionHasSecretsManager) { diff --git a/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs b/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs index 182f09e163..2259d846b7 100644 --- a/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs +++ b/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs @@ -11,6 +11,7 @@ using Bit.Core.Billing.Pricing; using Bit.Core.OrganizationFeatures.OrganizationSponsorships.FamiliesForEnterprise.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Test.Billing.Mocks; using Bit.Core.Test.Billing.Mocks.Plans; using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; @@ -654,6 +655,8 @@ public class SubscriptionUpdatedHandlerTests var plan = new Enterprise2023Plan(true); _pricingClient.GetPlanOrThrow(organization.PlanType) .Returns(plan); + _pricingClient.ListPlans() + .Returns(MockPlans.Plans); var parsedEvent = new Event { @@ -693,6 +696,92 @@ public class SubscriptionUpdatedHandlerTests await _stripeFacade.Received(1).DeleteCustomerDiscount(subscription.CustomerId); await _stripeFacade.Received(1).DeleteSubscriptionDiscount(subscription.Id); } + [Fact] + public async Task + HandleAsync_WhenUpgradingPlan_AndPreviousPlanHasSecretsManagerTrial_AndCurrentPlanHasSecretsManagerTrial_DoesNotRemovePasswordManagerCoupon() + { + // Arrange + var organizationId = Guid.NewGuid(); + var subscription = new Subscription + { + Id = "sub_123", + Status = StripeSubscriptionStatus.Active, + CustomerId = "cus_123", + Items = new StripeList + { + Data = + [ + new SubscriptionItem + { + CurrentPeriodEnd = DateTime.UtcNow.AddDays(10), + Plan = new Plan { Id = "2023-enterprise-org-seat-annually" } + }, + new SubscriptionItem + { + CurrentPeriodEnd = DateTime.UtcNow.AddDays(10), + Plan = new Plan { Id = "secrets-manager-enterprise-seat-annually" } + } + ] + }, + Customer = new Customer + { + Balance = 0, + Discount = new Discount { Coupon = new Coupon { Id = "sm-standalone" } } + }, + Discounts = [new Discount { Coupon = new Coupon { Id = "sm-standalone" } }], + Metadata = new Dictionary { { "organizationId", organizationId.ToString() } } + }; + + // Note: The organization plan is still the previous plan because the subscription is updated before the organization is updated + var organization = new Organization { Id = organizationId, PlanType = PlanType.TeamsAnnually2023 }; + + var plan = new Teams2023Plan(true); + _pricingClient.GetPlanOrThrow(organization.PlanType) + .Returns(plan); + _pricingClient.ListPlans() + .Returns(MockPlans.Plans); + + var parsedEvent = new Event + { + Data = new EventData + { + Object = subscription, + PreviousAttributes = JObject.FromObject(new + { + items = new + { + data = new[] + { + new { plan = new { id = "secrets-manager-teams-seat-annually" } }, + } + }, + Items = new StripeList + { + Data = + [ + new SubscriptionItem { Plan = new Stripe.Plan { Id = "secrets-manager-teams-seat-annually" } }, + ] + } + }) + } + }; + + _stripeEventService.GetSubscription(Arg.Any(), Arg.Any(), Arg.Any>()) + .Returns(subscription); + + _stripeEventUtilityService.GetIdsFromMetadata(Arg.Any>()) + .Returns(Tuple.Create(organizationId, null, null)); + + _organizationRepository.GetByIdAsync(organizationId) + .Returns(organization); + + // Act + await _sut.HandleAsync(parsedEvent); + + // Assert + await _stripeFacade.DidNotReceive().DeleteCustomerDiscount(subscription.CustomerId); + await _stripeFacade.DidNotReceive().DeleteSubscriptionDiscount(subscription.Id); + } [Theory] [MemberData(nameof(GetNonActiveSubscriptions))] From ad19efcff7a4dacb3d538689479867dd780e14eb Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Sat, 17 Jan 2026 10:47:21 +1000 Subject: [PATCH 03/51] [PM-22236] Fix invited accounts stuck in intermediate claimed status (#6810) * Exclude invited users from claimed domain checks. These users should be excluded by the JOIN on UserId, but it's a known issue that some invited users have this FK set. --- .../Repositories/IOrganizationRepository.cs | 4 +- .../Repositories/OrganizationRepository.cs | 3 +- ...erReadByClaimedOrganizationDomainsQuery.cs | 2 + ...dByOrganizationIdWithClaimedDomains_V2.sql | 3 +- ...anization_ReadByClaimedUserEmailDomain.sql | 3 +- .../GetByVerifiedUserEmailDomainAsyncTests.cs | 335 ++++++++++++++++++ .../OrganizationRepositoryTests.cs | 267 +------------- ...rganizationWithClaimedDomainsAsyncTests.cs | 197 ++++++++++ .../OrganizationUserRepositoryTests.cs | 194 ---------- ...0_ExcludeInvitedUsersFromClaimedDomain.sql | 24 ++ ...cludeInvitedUsersFromClaimedDomains_V2.sql | 29 ++ 11 files changed, 606 insertions(+), 455 deletions(-) create mode 100644 test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepository/GetByVerifiedUserEmailDomainAsyncTests.cs create mode 100644 test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/GetManyByOrganizationWithClaimedDomainsAsyncTests.cs create mode 100644 util/Migrator/DbScripts/2026-01-14_00_ExcludeInvitedUsersFromClaimedDomain.sql create mode 100644 util/Migrator/DbScripts/2026-01-14_01_ExcludeInvitedUsersFromClaimedDomains_V2.sql diff --git a/src/Core/AdminConsole/Repositories/IOrganizationRepository.cs b/src/Core/AdminConsole/Repositories/IOrganizationRepository.cs index da7a77000b..d79923fdd1 100644 --- a/src/Core/AdminConsole/Repositories/IOrganizationRepository.cs +++ b/src/Core/AdminConsole/Repositories/IOrganizationRepository.cs @@ -21,7 +21,9 @@ public interface IOrganizationRepository : IRepository Task> GetOwnerEmailAddressesById(Guid organizationId); /// - /// Gets the organizations that have a verified domain matching the user's email domain. + /// Gets the organizations that have claimed the user's account. Currently, only one organization may claim a user. + /// This requires that the organization has claimed the user's domain and the user is an organization member. + /// It excludes invited members. /// Task> GetByVerifiedUserEmailDomainAsync(Guid userId); diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs index 88410facf5..93c8cd304c 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs @@ -325,7 +325,8 @@ public class OrganizationRepository : Repository od.OrganizationId == _organizationId && od.VerifiedDate != null && diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2.sql index 64f3d81e08..4f781d2cc9 100644 --- a/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2.sql +++ b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2.sql @@ -8,13 +8,14 @@ BEGIN SELECT * FROM [dbo].[OrganizationUserView] WHERE [OrganizationId] = @OrganizationId + AND [Status] != 0 -- Exclude invited users ), UserDomains AS ( SELECT U.[Id], U.[EmailDomain] FROM [dbo].[UserEmailDomainView] U WHERE EXISTS ( SELECT 1 - FROM [dbo].[OrganizationDomainView] OD + FROM [dbo].[OrganizationDomainView] OD WHERE OD.[OrganizationId] = @OrganizationId AND OD.[VerifiedDate] IS NOT NULL AND OD.[DomainName] = U.[EmailDomain] diff --git a/src/Sql/dbo/Stored Procedures/Organization_ReadByClaimedUserEmailDomain.sql b/src/Sql/dbo/Stored Procedures/Organization_ReadByClaimedUserEmailDomain.sql index 583f548c8b..ee14c2c52a 100644 --- a/src/Sql/dbo/Stored Procedures/Organization_ReadByClaimedUserEmailDomain.sql +++ b/src/Sql/dbo/Stored Procedures/Organization_ReadByClaimedUserEmailDomain.sql @@ -6,7 +6,7 @@ BEGIN WITH CTE_User AS ( SELECT - U.*, + U.[Id], SUBSTRING(U.Email, CHARINDEX('@', U.Email) + 1, LEN(U.Email)) AS EmailDomain FROM dbo.[UserView] U WHERE U.[Id] = @UserId @@ -19,4 +19,5 @@ BEGIN WHERE OD.[VerifiedDate] IS NOT NULL AND CU.EmailDomain = OD.[DomainName] AND O.[Enabled] = 1 + AND OU.[Status] != 0 -- Exclude invited users END diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepository/GetByVerifiedUserEmailDomainAsyncTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepository/GetByVerifiedUserEmailDomainAsyncTests.cs new file mode 100644 index 0000000000..6dd7aafca4 --- /dev/null +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepository/GetByVerifiedUserEmailDomainAsyncTests.cs @@ -0,0 +1,335 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Xunit; + +namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.OrganizationRepository; + +public class GetByVerifiedUserEmailDomainAsyncTests +{ + [Theory, DatabaseData] + public async Task GetByClaimedUserDomainAsync_WithVerifiedDomain_Success( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var user1 = await userRepository.CreateAsync(new User + { + Name = "Test User 1", + Email = $"test+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var user2 = await userRepository.CreateAsync(new User + { + Name = "Test User 2", + Email = $"test+{id}@x-{domainName}", // Different domain + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var user3 = await userRepository.CreateAsync(new User + { + Name = "Test User 2", + Email = $"test+{id}@{domainName}.example.com", // Different domain + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization = await organizationRepository.CreateTestOrganizationAsync(); + + var organizationDomain = new OrganizationDomain + { + OrganizationId = organization.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain.SetVerifiedDate(); + organizationDomain.SetNextRunDate(12); + organizationDomain.SetJobRunCount(); + await organizationDomainRepository.CreateAsync(organizationDomain); + + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, user1); + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, user2); + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, user3); + + var user1Response = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user1.Id); + var user2Response = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user2.Id); + var user3Response = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user3.Id); + + Assert.NotEmpty(user1Response); + Assert.Equal(organization.Id, user1Response.First().Id); + Assert.Empty(user2Response); + Assert.Empty(user3Response); + } + + [Theory, DatabaseData] + public async Task GetByVerifiedUserEmailDomainAsync_WithUnverifiedDomains_ReturnsEmpty( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization = await organizationRepository.CreateTestOrganizationAsync(); + + var organizationDomain = new OrganizationDomain + { + OrganizationId = organization.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain.SetNextRunDate(12); + organizationDomain.SetJobRunCount(); + await organizationDomainRepository.CreateAsync(organizationDomain); + + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, user); + + var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user.Id); + + Assert.Empty(result); + } + + [Theory, DatabaseData] + public async Task GetByVerifiedUserEmailDomainAsync_WithMultipleVerifiedDomains_ReturnsAllMatchingOrganizations( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization1 = await organizationRepository.CreateTestOrganizationAsync(); + var organization2 = await organizationRepository.CreateTestOrganizationAsync(); + + var organizationDomain1 = new OrganizationDomain + { + OrganizationId = organization1.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain1.SetNextRunDate(12); + organizationDomain1.SetJobRunCount(); + organizationDomain1.SetVerifiedDate(); + await organizationDomainRepository.CreateAsync(organizationDomain1); + + var organizationDomain2 = new OrganizationDomain + { + OrganizationId = organization2.Id, + DomainName = domainName, + Txt = "btw+67890", + }; + organizationDomain2.SetNextRunDate(12); + organizationDomain2.SetJobRunCount(); + organizationDomain2.SetVerifiedDate(); + await organizationDomainRepository.CreateAsync(organizationDomain2); + + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization1, user); + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization2, user); + + var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user.Id); + + Assert.Equal(2, result.Count); + Assert.Contains(result, org => org.Id == organization1.Id); + Assert.Contains(result, org => org.Id == organization2.Id); + } + + [Theory, DatabaseData] + public async Task GetByVerifiedUserEmailDomainAsync_WithNonExistentUser_ReturnsEmpty( + IOrganizationRepository organizationRepository) + { + var nonExistentUserId = Guid.NewGuid(); + + var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(nonExistentUserId); + + Assert.Empty(result); + } + + /// + /// Tests an edge case where some invited users are created linked to a UserId. + /// This is defective behavior, but will take longer to fix - for now, we are defensive and expressly + /// exclude such users from the results without relying on the inner join only. + /// Invited-revoked users linked to a UserId remain intentionally unhandled for now as they have not caused + /// any issues to date and we want to minimize edge cases. + /// We will fix the underlying issue going forward: https://bitwarden.atlassian.net/browse/PM-22405 + /// + [Theory, DatabaseData] + public async Task GetByVerifiedUserEmailDomainAsync_WithInvitedUserWithUserId_ReturnsEmpty( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization = await organizationRepository.CreateTestOrganizationAsync(); + + var organizationDomain = new OrganizationDomain + { + OrganizationId = organization.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain.SetVerifiedDate(); + organizationDomain.SetNextRunDate(12); + organizationDomain.SetJobRunCount(); + await organizationDomainRepository.CreateAsync(organizationDomain); + + // Create invited user with matching email domain but UserId set (edge case) + await organizationUserRepository.CreateAsync(new OrganizationUser + { + OrganizationId = organization.Id, + UserId = user.Id, + Email = user.Email, + Status = OrganizationUserStatusType.Invited, + }); + + var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user.Id); + + // Invited users should be excluded even if they have UserId set + Assert.Empty(result); + } + + [Theory, DatabaseData] + public async Task GetByVerifiedUserEmailDomainAsync_WithAcceptedUser_ReturnsOrganization( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization = await organizationRepository.CreateTestOrganizationAsync(); + + var organizationDomain = new OrganizationDomain + { + OrganizationId = organization.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain.SetVerifiedDate(); + organizationDomain.SetNextRunDate(12); + organizationDomain.SetJobRunCount(); + await organizationDomainRepository.CreateAsync(organizationDomain); + + await organizationUserRepository.CreateAcceptedTestOrganizationUserAsync(organization, user); + + var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user.Id); + + Assert.NotEmpty(result); + Assert.Equal(organization.Id, result.First().Id); + } + + [Theory, DatabaseData] + public async Task GetByVerifiedUserEmailDomainAsync_WithRevokedUser_ReturnsOrganization( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization = await organizationRepository.CreateTestOrganizationAsync(); + + var organizationDomain = new OrganizationDomain + { + OrganizationId = organization.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain.SetVerifiedDate(); + organizationDomain.SetNextRunDate(12); + organizationDomain.SetJobRunCount(); + await organizationDomainRepository.CreateAsync(organizationDomain); + + await organizationUserRepository.CreateRevokedTestOrganizationUserAsync(organization, user); + + var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user.Id); + + Assert.NotEmpty(result); + Assert.Equal(organization.Id, result.First().Id); + } +} diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs index 67e2c1910b..52b1e7484b 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs @@ -8,254 +8,7 @@ namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories; public class OrganizationRepositoryTests { - [DatabaseTheory, DatabaseData] - public async Task GetByClaimedUserDomainAsync_WithVerifiedDomain_Success( - IUserRepository userRepository, - IOrganizationRepository organizationRepository, - IOrganizationUserRepository organizationUserRepository, - IOrganizationDomainRepository organizationDomainRepository) - { - var id = Guid.NewGuid(); - var domainName = $"{id}.example.com"; - - var user1 = await userRepository.CreateAsync(new User - { - Name = "Test User 1", - Email = $"test+{id}@{domainName}", - ApiKey = "TEST", - SecurityStamp = "stamp", - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 1, - KdfMemory = 2, - KdfParallelism = 3 - }); - - var user2 = await userRepository.CreateAsync(new User - { - Name = "Test User 2", - Email = $"test+{id}@x-{domainName}", // Different domain - ApiKey = "TEST", - SecurityStamp = "stamp", - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 1, - KdfMemory = 2, - KdfParallelism = 3 - }); - - var user3 = await userRepository.CreateAsync(new User - { - Name = "Test User 2", - Email = $"test+{id}@{domainName}.example.com", // Different domain - ApiKey = "TEST", - SecurityStamp = "stamp", - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 1, - KdfMemory = 2, - KdfParallelism = 3 - }); - - var organization = await organizationRepository.CreateAsync(new Organization - { - Name = $"Test Org {id}", - BillingEmail = user1.Email, // TODO: EF does not enforce this being NOT NULL - Plan = "Test", // TODO: EF does not enforce this being NOT NULL - PrivateKey = "privatekey", - }); - - var organizationDomain = new OrganizationDomain - { - OrganizationId = organization.Id, - DomainName = domainName, - Txt = "btw+12345", - }; - organizationDomain.SetVerifiedDate(); - organizationDomain.SetNextRunDate(12); - organizationDomain.SetJobRunCount(); - await organizationDomainRepository.CreateAsync(organizationDomain); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - OrganizationId = organization.Id, - UserId = user1.Id, - Status = OrganizationUserStatusType.Confirmed, - ResetPasswordKey = "resetpasswordkey1", - }); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - OrganizationId = organization.Id, - UserId = user2.Id, - Status = OrganizationUserStatusType.Confirmed, - ResetPasswordKey = "resetpasswordkey1", - }); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - OrganizationId = organization.Id, - UserId = user3.Id, - Status = OrganizationUserStatusType.Confirmed, - ResetPasswordKey = "resetpasswordkey1", - }); - - var user1Response = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user1.Id); - var user2Response = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user2.Id); - var user3Response = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user3.Id); - - Assert.NotEmpty(user1Response); - Assert.Equal(organization.Id, user1Response.First().Id); - Assert.Empty(user2Response); - Assert.Empty(user3Response); - } - - [DatabaseTheory, DatabaseData] - public async Task GetByVerifiedUserEmailDomainAsync_WithUnverifiedDomains_ReturnsEmpty( - IUserRepository userRepository, - IOrganizationRepository organizationRepository, - IOrganizationUserRepository organizationUserRepository, - IOrganizationDomainRepository organizationDomainRepository) - { - var id = Guid.NewGuid(); - var domainName = $"{id}.example.com"; - - var user = await userRepository.CreateAsync(new User - { - Name = "Test User", - Email = $"test+{id}@{domainName}", - ApiKey = "TEST", - SecurityStamp = "stamp", - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 1, - KdfMemory = 2, - KdfParallelism = 3 - }); - - var organization = await organizationRepository.CreateAsync(new Organization - { - Name = $"Test Org {id}", - BillingEmail = user.Email, - Plan = "Test", - PrivateKey = "privatekey", - }); - - var organizationDomain = new OrganizationDomain - { - OrganizationId = organization.Id, - DomainName = domainName, - Txt = "btw+12345", - }; - organizationDomain.SetNextRunDate(12); - organizationDomain.SetJobRunCount(); - await organizationDomainRepository.CreateAsync(organizationDomain); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - OrganizationId = organization.Id, - UserId = user.Id, - Status = OrganizationUserStatusType.Confirmed, - ResetPasswordKey = "resetpasswordkey", - }); - - var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user.Id); - - Assert.Empty(result); - } - - [DatabaseTheory, DatabaseData] - public async Task GetByVerifiedUserEmailDomainAsync_WithMultipleVerifiedDomains_ReturnsAllMatchingOrganizations( - IUserRepository userRepository, - IOrganizationRepository organizationRepository, - IOrganizationUserRepository organizationUserRepository, - IOrganizationDomainRepository organizationDomainRepository) - { - var id = Guid.NewGuid(); - var domainName = $"{id}.example.com"; - - var user = await userRepository.CreateAsync(new User - { - Name = "Test User", - Email = $"test+{id}@{domainName}", - ApiKey = "TEST", - SecurityStamp = "stamp", - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 1, - KdfMemory = 2, - KdfParallelism = 3 - }); - - var organization1 = await organizationRepository.CreateAsync(new Organization - { - Name = $"Test Org 1 {id}", - BillingEmail = user.Email, - Plan = "Test", - PrivateKey = "privatekey1", - }); - - var organization2 = await organizationRepository.CreateAsync(new Organization - { - Name = $"Test Org 2 {id}", - BillingEmail = user.Email, - Plan = "Test", - PrivateKey = "privatekey2", - }); - - var organizationDomain1 = new OrganizationDomain - { - OrganizationId = organization1.Id, - DomainName = domainName, - Txt = "btw+12345", - }; - organizationDomain1.SetNextRunDate(12); - organizationDomain1.SetJobRunCount(); - organizationDomain1.SetVerifiedDate(); - await organizationDomainRepository.CreateAsync(organizationDomain1); - - var organizationDomain2 = new OrganizationDomain - { - OrganizationId = organization2.Id, - DomainName = domainName, - Txt = "btw+67890", - }; - organizationDomain2.SetNextRunDate(12); - organizationDomain2.SetJobRunCount(); - organizationDomain2.SetVerifiedDate(); - await organizationDomainRepository.CreateAsync(organizationDomain2); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - OrganizationId = organization1.Id, - UserId = user.Id, - Status = OrganizationUserStatusType.Confirmed, - ResetPasswordKey = "resetpasswordkey1", - }); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - OrganizationId = organization2.Id, - UserId = user.Id, - Status = OrganizationUserStatusType.Confirmed, - ResetPasswordKey = "resetpasswordkey2", - }); - - var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user.Id); - - Assert.Equal(2, result.Count); - Assert.Contains(result, org => org.Id == organization1.Id); - Assert.Contains(result, org => org.Id == organization2.Id); - } - - [DatabaseTheory, DatabaseData] - public async Task GetByVerifiedUserEmailDomainAsync_WithNonExistentUser_ReturnsEmpty( - IOrganizationRepository organizationRepository) - { - var nonExistentUserId = Guid.NewGuid(); - - var result = await organizationRepository.GetByVerifiedUserEmailDomainAsync(nonExistentUserId); - - Assert.Empty(result); - } - - - [DatabaseTheory, DatabaseData] + [Theory, DatabaseData] public async Task GetManyByIdsAsync_ExistingOrganizations_ReturnsOrganizations(IOrganizationRepository organizationRepository) { var email = "test@email.com"; @@ -287,7 +40,7 @@ public class OrganizationRepositoryTests await organizationRepository.DeleteAsync(organization2); } - [DatabaseTheory, DatabaseData] + [Theory, DatabaseData] public async Task GetOccupiedSeatCountByOrganizationIdAsync_WithUsersAndSponsorships_ReturnsCorrectCounts( IUserRepository userRepository, IOrganizationRepository organizationRepository, @@ -356,7 +109,7 @@ public class OrganizationRepositoryTests Assert.Equal(4, result.Total); // Total occupied seats } - [DatabaseTheory, DatabaseData] + [Theory, DatabaseData] public async Task GetOccupiedSeatCountByOrganizationIdAsync_WithNoUsersOrSponsorships_ReturnsZero( IOrganizationRepository organizationRepository) { @@ -372,7 +125,7 @@ public class OrganizationRepositoryTests Assert.Equal(0, result.Total); } - [DatabaseTheory, DatabaseData] + [Theory, DatabaseData] public async Task GetOccupiedSeatCountByOrganizationIdAsync_WithOnlyRevokedUsers_ReturnsZero( IUserRepository userRepository, IOrganizationRepository organizationRepository, @@ -399,7 +152,7 @@ public class OrganizationRepositoryTests Assert.Equal(0, result.Total); } - [DatabaseTheory, DatabaseData] + [Theory, DatabaseData] public async Task GetOccupiedSeatCountByOrganizationIdAsync_WithOnlyExpiredSponsorships_ReturnsZero( IOrganizationRepository organizationRepository, IOrganizationSponsorshipRepository organizationSponsorshipRepository) @@ -424,7 +177,7 @@ public class OrganizationRepositoryTests Assert.Equal(0, result.Total); } - [DatabaseTheory, DatabaseData] + [Theory, DatabaseData] public async Task IncrementSeatCountAsync_IncrementsSeatCount(IOrganizationRepository organizationRepository) { var organization = await organizationRepository.CreateTestOrganizationAsync(); @@ -438,7 +191,7 @@ public class OrganizationRepositoryTests Assert.Equal(8, result.Seats); } - [DatabaseData, DatabaseTheory] + [DatabaseData, Theory] public async Task IncrementSeatCountAsync_GivenOrganizationHasNotChangedSeatCountBefore_WhenUpdatingOrgSeats_ThenSubscriptionUpdateIsSaved( IOrganizationRepository sutRepository) { @@ -462,7 +215,7 @@ public class OrganizationRepositoryTests await sutRepository.DeleteAsync(organization); } - [DatabaseData, DatabaseTheory] + [DatabaseData, Theory] public async Task IncrementSeatCountAsync_GivenOrganizationHasChangedSeatCountBeforeAndRecordExists_WhenUpdatingOrgSeats_ThenSubscriptionUpdateIsSaved( IOrganizationRepository sutRepository) { @@ -487,7 +240,7 @@ public class OrganizationRepositoryTests await sutRepository.DeleteAsync(organization); } - [DatabaseData, DatabaseTheory] + [DatabaseData, Theory] public async Task GetOrganizationsForSubscriptionSyncAsync_GivenOrganizationHasChangedSeatCount_WhenGettingOrgsToUpdate_ThenReturnsOrgSubscriptionUpdate( IOrganizationRepository sutRepository) { @@ -510,7 +263,7 @@ public class OrganizationRepositoryTests await sutRepository.DeleteAsync(organization); } - [DatabaseData, DatabaseTheory] + [DatabaseData, Theory] public async Task UpdateSuccessfulOrganizationSyncStatusAsync_GivenOrganizationHasChangedSeatCount_WhenUpdatingStatus_ThenSuccessfullyUpdatesOrgSoItDoesntSync( IOrganizationRepository sutRepository) { diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/GetManyByOrganizationWithClaimedDomainsAsyncTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/GetManyByOrganizationWithClaimedDomainsAsyncTests.cs new file mode 100644 index 0000000000..6fa395751b --- /dev/null +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/GetManyByOrganizationWithClaimedDomainsAsyncTests.cs @@ -0,0 +1,197 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Xunit; + +namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.OrganizationUserRepository; + +public class GetManyByOrganizationWithClaimedDomainsAsyncTests +{ + [Theory, DatabaseData] + public async Task WithVerifiedDomain_WithOneMatchingEmailDomain_ReturnsSingle( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var user1 = await userRepository.CreateAsync(new User + { + Name = "Test User 1", + Email = $"test+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var user2 = await userRepository.CreateAsync(new User + { + Name = "Test User 2", + Email = $"test+{id}@x-{domainName}", // Different domain + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var user3 = await userRepository.CreateAsync(new User + { + Name = "Test User 3", + Email = $"test+{id}@{domainName}.example.com", // Different domain + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization = await organizationRepository.CreateTestOrganizationAsync(); + + var organizationDomain = new OrganizationDomain + { + OrganizationId = organization.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain.SetVerifiedDate(); + organizationDomain.SetNextRunDate(12); + organizationDomain.SetJobRunCount(); + await organizationDomainRepository.CreateAsync(organizationDomain); + + var orgUser1 = await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, user1); + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, user2); + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, user3); + + var result = await organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync(organization.Id); + + Assert.NotNull(result); + Assert.Single(result); + Assert.Equal(orgUser1.Id, result.Single().Id); + } + + [Theory, DatabaseData] + public async Task WithNoVerifiedDomain_ReturnsEmpty( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization = await organizationRepository.CreateTestOrganizationAsync(); + + // Create domain but do NOT verify it + var organizationDomain = new OrganizationDomain + { + OrganizationId = organization.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain.SetNextRunDate(12); + // Note: NOT calling SetVerifiedDate() + await organizationDomainRepository.CreateAsync(organizationDomain); + + await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, user); + + var result = await organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync(organization.Id); + + Assert.NotNull(result); + Assert.Empty(result); + } + + /// + /// Tests an edge case where some invited users are created linked to a UserId. + /// This is defective behavior, but will take longer to fix - for now, we are defensive and expressly + /// exclude such users from the results without relying on the inner join only. + /// Invited-revoked users linked to a UserId remain intentionally unhandled for now as they have not caused + /// any issues to date and we want to minimize edge cases. + /// We will fix the underlying issue going forward: https://bitwarden.atlassian.net/browse/PM-22405 + /// + [Theory, DatabaseData] + public async Task WithVerifiedDomain_ExcludesInvitedUsers( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationDomainRepository organizationDomainRepository) + { + var id = Guid.NewGuid(); + var domainName = $"{id}.example.com"; + + var invitedUser = await userRepository.CreateAsync(new User + { + Name = "Invited User", + Email = $"invited+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var confirmedUser = await userRepository.CreateAsync(new User + { + Name = "Confirmed User", + Email = $"confirmed+{id}@{domainName}", + ApiKey = "TEST", + SecurityStamp = "stamp", + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 1, + KdfMemory = 2, + KdfParallelism = 3 + }); + + var organization = await organizationRepository.CreateTestOrganizationAsync(); + + var organizationDomain = new OrganizationDomain + { + OrganizationId = organization.Id, + DomainName = domainName, + Txt = "btw+12345", + }; + organizationDomain.SetVerifiedDate(); + organizationDomain.SetNextRunDate(12); + organizationDomain.SetJobRunCount(); + await organizationDomainRepository.CreateAsync(organizationDomain); + + // Create invited user with UserId set (edge case - should be excluded even with UserId linked) + var invitedOrgUser = await organizationUserRepository.CreateAsync(new OrganizationUser + { + OrganizationId = organization.Id, + UserId = invitedUser.Id, // Edge case: invited user with UserId set + Email = invitedUser.Email, + Status = OrganizationUserStatusType.Invited, + Type = OrganizationUserType.User + }); + + // Create confirmed user linked by UserId only (no Email field set) + var confirmedOrgUser = await organizationUserRepository.CreateConfirmedTestOrganizationUserAsync(organization, confirmedUser); + + var result = await organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync(organization.Id); + + Assert.NotNull(result); + var claimedUser = Assert.Single(result); + Assert.Equal(confirmedOrgUser.Id, claimedUser.Id); + } +} diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs index 1c433d0e6e..b77406abf5 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs @@ -599,136 +599,6 @@ public class OrganizationUserRepositoryTests Assert.Null(orgWithoutSsoDetails.SsoConfig); } - [DatabaseTheory, DatabaseData] - public async Task GetManyByOrganizationWithClaimedDomainsAsync_WithVerifiedDomain_WithOneMatchingEmailDomain_ReturnsSingle( - IUserRepository userRepository, - IOrganizationRepository organizationRepository, - IOrganizationUserRepository organizationUserRepository, - IOrganizationDomainRepository organizationDomainRepository) - { - var id = Guid.NewGuid(); - var domainName = $"{id}.example.com"; - - var user1 = await userRepository.CreateAsync(new User - { - Name = "Test User 1", - Email = $"test+{id}@{domainName}", - ApiKey = "TEST", - SecurityStamp = "stamp", - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 1, - KdfMemory = 2, - KdfParallelism = 3 - }); - - var user2 = await userRepository.CreateAsync(new User - { - Name = "Test User 2", - Email = $"test+{id}@x-{domainName}", // Different domain - ApiKey = "TEST", - SecurityStamp = "stamp", - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 1, - KdfMemory = 2, - KdfParallelism = 3 - }); - - var user3 = await userRepository.CreateAsync(new User - { - Name = "Test User 2", - Email = $"test+{id}@{domainName}.example.com", // Different domain - ApiKey = "TEST", - SecurityStamp = "stamp", - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 1, - KdfMemory = 2, - KdfParallelism = 3 - }); - - var organization = await organizationRepository.CreateAsync(new Organization - { - Name = $"Test Org {id}", - BillingEmail = user1.Email, // TODO: EF does not enforce this being NOT NULL - Plan = "Test", // TODO: EF does not enforce this being NOT NULL - PrivateKey = "privatekey", - UsePolicies = false, - UseSso = false, - UseKeyConnector = false, - UseScim = false, - UseGroups = false, - UseDirectory = false, - UseEvents = false, - UseTotp = false, - Use2fa = false, - UseApi = false, - UseResetPassword = false, - UseSecretsManager = false, - SelfHost = false, - UsersGetPremium = false, - UseCustomPermissions = false, - Enabled = true, - UsePasswordManager = false, - LimitCollectionCreation = false, - LimitCollectionDeletion = false, - LimitItemDeletion = false, - AllowAdminAccessToAllCollectionItems = false, - UseRiskInsights = false, - UseAdminSponsoredFamilies = false, - UsePhishingBlocker = false, - UseDisableSmAdsForUsers = false, - }); - - var organizationDomain = new OrganizationDomain - { - OrganizationId = organization.Id, - DomainName = domainName, - Txt = "btw+12345", - }; - organizationDomain.SetVerifiedDate(); - organizationDomain.SetNextRunDate(12); - organizationDomain.SetJobRunCount(); - await organizationDomainRepository.CreateAsync(organizationDomain); - - var orgUser1 = await organizationUserRepository.CreateAsync(new OrganizationUser - { - Id = CoreHelpers.GenerateComb(), - OrganizationId = organization.Id, - UserId = user1.Id, - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.Owner, - ResetPasswordKey = "resetpasswordkey1", - AccessSecretsManager = false - }); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - Id = CoreHelpers.GenerateComb(), - OrganizationId = organization.Id, - UserId = user2.Id, - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.User, - ResetPasswordKey = "resetpasswordkey1", - AccessSecretsManager = false - }); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - Id = CoreHelpers.GenerateComb(), - OrganizationId = organization.Id, - UserId = user3.Id, - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.User, - ResetPasswordKey = "resetpasswordkey1", - AccessSecretsManager = false - }); - - var responseModel = await organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync(organization.Id); - - Assert.NotNull(responseModel); - Assert.Single(responseModel); - Assert.Equal(orgUser1.Id, responseModel.Single().Id); - } - [DatabaseTheory, DatabaseData] public async Task CreateManyAsync_NoId_Works(IOrganizationRepository organizationRepository, IUserRepository userRepository, @@ -1237,70 +1107,6 @@ public class OrganizationUserRepositoryTests Assert.DoesNotContain(user1Result.Collections, c => c.Id == defaultUserCollection.Id); } - [DatabaseTheory, DatabaseData] - public async Task GetManyByOrganizationWithClaimedDomainsAsync_WithNoVerifiedDomain_ReturnsEmpty( - IUserRepository userRepository, - IOrganizationRepository organizationRepository, - IOrganizationUserRepository organizationUserRepository, - IOrganizationDomainRepository organizationDomainRepository) - { - var id = Guid.NewGuid(); - var domainName = $"{id}.example.com"; - var requestTime = DateTime.UtcNow; - - var user1 = await userRepository.CreateAsync(new User - { - Id = CoreHelpers.GenerateComb(), - Name = "Test User 1", - Email = $"test+{id}@{domainName}", - ApiKey = "TEST", - SecurityStamp = "stamp", - CreationDate = requestTime, - RevisionDate = requestTime, - AccountRevisionDate = requestTime - }); - - var organization = await organizationRepository.CreateAsync(new Organization - { - Id = CoreHelpers.GenerateComb(), - Name = $"Test Org {id}", - BillingEmail = user1.Email, - Plan = "Test", - Enabled = true, - CreationDate = requestTime, - RevisionDate = requestTime - }); - - // Create domain but do NOT verify it - var organizationDomain = new OrganizationDomain - { - Id = CoreHelpers.GenerateComb(), - OrganizationId = organization.Id, - DomainName = domainName, - Txt = "btw+12345", - CreationDate = requestTime - }; - organizationDomain.SetNextRunDate(12); - // Note: NOT calling SetVerifiedDate() - await organizationDomainRepository.CreateAsync(organizationDomain); - - await organizationUserRepository.CreateAsync(new OrganizationUser - { - Id = CoreHelpers.GenerateComb(), - OrganizationId = organization.Id, - UserId = user1.Id, - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.Owner, - CreationDate = requestTime, - RevisionDate = requestTime - }); - - var responseModel = await organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync(organization.Id); - - Assert.NotNull(responseModel); - Assert.Empty(responseModel); - } - [DatabaseTheory, DatabaseData] public async Task DeleteAsync_WithNullEmail_DoesNotSetDefaultUserCollectionEmail(IUserRepository userRepository, ICollectionRepository collectionRepository, diff --git a/util/Migrator/DbScripts/2026-01-14_00_ExcludeInvitedUsersFromClaimedDomain.sql b/util/Migrator/DbScripts/2026-01-14_00_ExcludeInvitedUsersFromClaimedDomain.sql new file mode 100644 index 0000000000..788fa02b7c --- /dev/null +++ b/util/Migrator/DbScripts/2026-01-14_00_ExcludeInvitedUsersFromClaimedDomain.sql @@ -0,0 +1,24 @@ +CREATE OR ALTER PROCEDURE [dbo].[Organization_ReadByClaimedUserEmailDomain] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON; + + WITH CTE_User AS ( + SELECT + U.[Id], + SUBSTRING(U.Email, CHARINDEX('@', U.Email) + 1, LEN(U.Email)) AS EmailDomain + FROM dbo.[UserView] U + WHERE U.[Id] = @UserId + ) + SELECT O.* + FROM CTE_User CU + INNER JOIN dbo.[OrganizationUserView] OU ON CU.[Id] = OU.[UserId] + INNER JOIN dbo.[OrganizationView] O ON OU.[OrganizationId] = O.[Id] + INNER JOIN dbo.[OrganizationDomainView] OD ON OU.[OrganizationId] = OD.[OrganizationId] + WHERE OD.[VerifiedDate] IS NOT NULL + AND CU.EmailDomain = OD.[DomainName] + AND O.[Enabled] = 1 + AND OU.[Status] != 0 -- Exclude invited users +END +GO diff --git a/util/Migrator/DbScripts/2026-01-14_01_ExcludeInvitedUsersFromClaimedDomains_V2.sql b/util/Migrator/DbScripts/2026-01-14_01_ExcludeInvitedUsersFromClaimedDomains_V2.sql new file mode 100644 index 0000000000..b7be5fd0e0 --- /dev/null +++ b/util/Migrator/DbScripts/2026-01-14_01_ExcludeInvitedUsersFromClaimedDomains_V2.sql @@ -0,0 +1,29 @@ +CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2] + @OrganizationId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON; + + WITH OrgUsers AS ( + SELECT * + FROM [dbo].[OrganizationUserView] + WHERE [OrganizationId] = @OrganizationId + AND [Status] != 0 -- Exclude invited users + ), + UserDomains AS ( + SELECT U.[Id], U.[EmailDomain] + FROM [dbo].[UserEmailDomainView] U + WHERE EXISTS ( + SELECT 1 + FROM [dbo].[OrganizationDomainView] OD + WHERE OD.[OrganizationId] = @OrganizationId + AND OD.[VerifiedDate] IS NOT NULL + AND OD.[DomainName] = U.[EmailDomain] + ) + ) + SELECT OU.* + FROM OrgUsers OU + JOIN UserDomains UD ON OU.[UserId] = UD.[Id] + OPTION (RECOMPILE); +END +GO From c37412bacbdbbe07027ac26cda750ba4ec9be736 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Tue, 20 Jan 2026 10:03:33 -0500 Subject: [PATCH 04/51] chore(flags): Remove pm-1632-redirect-on-sso-required feature flag * Remove feature flag. * Update test title. * Fixed some test failures. * Fixed tests * Removed method that's no longer used. * Removed unneeded directive. --- src/Core/Constants.cs | 1 - .../RequestValidators/BaseRequestValidator.cs | 91 +--- .../CustomTokenRequestValidator.cs | 11 - .../ResourceOwnerPasswordValidator.cs | 8 - .../WebAuthnGrantValidator.cs | 8 - .../BaseRequestValidatorTests.cs | 396 ++++++------------ .../BaseRequestValidatorTestWrapper.cs | 9 - 7 files changed, 144 insertions(+), 380 deletions(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 6f42778b6b..10c68ddc42 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -162,7 +162,6 @@ public static class FeatureFlagKeys public const string MjmlWelcomeEmailTemplates = "pm-21741-mjml-welcome-email"; public const string OrganizationConfirmationEmail = "pm-28402-update-confirmed-to-org-email-template"; public const string MarketingInitiatedPremiumFlow = "pm-26140-marketing-initiated-premium-flow"; - public const string RedirectOnSsoRequired = "pm-1632-redirect-on-sso-required"; public const string PrefetchPasswordPrelogin = "pm-23801-prefetch-password-prelogin"; public const string PM27086_UpdateAuthenticationApisForInputPassword = "pm-27086-update-authentication-apis-for-input-password"; diff --git a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index e07446d49f..289feebdb2 100644 --- a/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -4,7 +4,6 @@ using System.Security.Claims; using Bit.Core; -using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Entities; @@ -233,56 +232,14 @@ public abstract class BaseRequestValidator where T : class private async Task ValidateSsoAsync(T context, ValidatedTokenRequest request, CustomValidatorRequestContext validatorContext) { - // TODO: Clean up Feature Flag: Remove this if block: PM-28281 - if (!_featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired)) + var ssoValid = await _ssoRequestValidator.ValidateAsync(validatorContext.User, request, validatorContext); + if (ssoValid) { - validatorContext.SsoRequired = await RequireSsoLoginAsync(validatorContext.User, request.GrantType); - if (!validatorContext.SsoRequired) - { - return true; - } - - // Users without SSO requirement requesting 2FA recovery will be fast-forwarded through login and are - // presented with their 2FA management area as a reminder to re-evaluate their 2FA posture after recovery and - // review their new recovery token if desired. - // SSO users cannot be assumed to be authenticated, and must prove authentication with their IdP after recovery. - // As described in validation order determination, if TwoFactorRequired, the 2FA validation scheme will have been - // evaluated, and recovery will have been performed if requested. - // We will send a descriptive message in these cases so clients can give the appropriate feedback and redirect - // to /login. - if (validatorContext.TwoFactorRequired && - validatorContext.TwoFactorRecoveryRequested) - { - SetSsoResult(context, - new Dictionary - { - { - "ErrorModel", - new ErrorResponseModel( - "Two-factor recovery has been performed. SSO authentication is required.") - } - }); - return false; - } - - SetSsoResult(context, - new Dictionary - { - { "ErrorModel", new ErrorResponseModel("SSO authentication is required.") } - }); - return false; + return true; } - else - { - var ssoValid = await _ssoRequestValidator.ValidateAsync(validatorContext.User, request, validatorContext); - if (ssoValid) - { - return true; - } - SetValidationErrorResult(context, validatorContext); - return ssoValid; - } + SetValidationErrorResult(context, validatorContext); + return ssoValid; } /// @@ -521,9 +478,6 @@ public abstract class BaseRequestValidator where T : class [Obsolete("Consider using SetValidationErrorResult instead.")] protected abstract void SetTwoFactorResult(T context, Dictionary customResponse); - [Obsolete("Consider using SetValidationErrorResult instead.")] - protected abstract void SetSsoResult(T context, Dictionary customResponse); - [Obsolete("Consider using SetValidationErrorResult instead.")] protected abstract void SetErrorResult(T context, Dictionary customResponse); @@ -540,41 +494,6 @@ public abstract class BaseRequestValidator where T : class protected abstract ClaimsPrincipal GetSubject(T context); - /// - /// Check if the user is required to authenticate via SSO. If the user requires SSO, but they are - /// logging in using an API Key (client_credentials) then they are allowed to bypass the SSO requirement. - /// If the GrantType is authorization_code or client_credentials we know the user is trying to login - /// using the SSO flow so they are allowed to continue. - /// - /// user trying to login - /// magic string identifying the grant type requested - /// true if sso required; false if not required or already in process - [Obsolete( - "This method is deprecated and will be removed in future versions, PM-28281. Please use the SsoRequestValidator scheme instead.")] - private async Task RequireSsoLoginAsync(User user, string grantType) - { - if (grantType == "authorization_code" || grantType == "client_credentials") - { - // Already using SSO to authenticate, or logging-in via api key to skip SSO requirement - // allow to authenticate successfully - return false; - } - - // Check if user belongs to any organization with an active SSO policy - var ssoRequired = _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements) - ? (await PolicyRequirementQuery.GetAsync(user.Id)) - .SsoRequired - : await PolicyService.AnyPoliciesApplicableToUserAsync( - user.Id, PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); - if (ssoRequired) - { - return true; - } - - // Default - SSO is not required - return false; - } - private async Task ResetFailedAuthDetailsAsync(User user) { // Early escape if db hit not necessary diff --git a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs index 38a4813ecd..2412c52308 100644 --- a/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs @@ -194,17 +194,6 @@ public class CustomTokenRequestValidator : BaseRequestValidator customResponse) - { - Debug.Assert(context.Result is not null); - context.Result.Error = "invalid_grant"; - context.Result.ErrorDescription = "Sso authentication required."; - context.Result.IsError = true; - context.Result.CustomResponse = customResponse; - } - [Obsolete("Consider using SetGrantValidationErrorResult instead.")] protected override void SetErrorResult(CustomTokenRequestValidationContext context, Dictionary customResponse) diff --git a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs index ea2c021f63..8bfddf24f3 100644 --- a/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs @@ -152,14 +152,6 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator customResponse) - { - context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Sso authentication required.", - customResponse); - } - [Obsolete("Consider using SetGrantValidationErrorResult instead.")] protected override void SetErrorResult(ResourceOwnerPasswordValidationContext context, Dictionary customResponse) diff --git a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs index e4cd60827e..1563831b81 100644 --- a/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs @@ -142,14 +142,6 @@ public class WebAuthnGrantValidator : BaseRequestValidator customResponse) - { - context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Sso authentication required.", - customResponse); - } - [Obsolete("Consider using SetValidationErrorResult instead.")] protected override void SetErrorResult(ExtensionGrantValidationContext context, Dictionary customResponse) { diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 677382b138..4b6f681096 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -18,6 +18,7 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidationConstants; using Bit.Identity.IdentityServer.RequestValidators; using Bit.Identity.Test.Wrappers; using Bit.Test.Common.AutoFixture.Attributes; @@ -130,7 +131,7 @@ public class BaseRequestValidatorTests var logs = _logger.Collector.GetSnapshot(true); Assert.Contains(logs, l => l.Level == LogLevel.Warning && l.Message == "Failed login attempt. Is2FARequest: False IpAddress: "); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; + var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; Assert.Equal("Username or password is incorrect. Try again.", errorResponse.Message); } @@ -161,7 +162,11 @@ public class BaseRequestValidatorTests .ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(false)); - // 5 -> not legacy user + // 5 -> SSO not required + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 6 -> not legacy user _userService.IsLegacyUser(Arg.Any()) .Returns(false); @@ -203,6 +208,11 @@ public class BaseRequestValidatorTests _userService.IsLegacyUser(Arg.Any()) .Returns(false); + // 6 -> SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 7 -> setup user account keys _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData { PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( @@ -262,6 +272,11 @@ public class BaseRequestValidatorTests _userService.IsLegacyUser(Arg.Any()) .Returns(false); + // 6 -> SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 7 -> setup user account keys _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData { PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( @@ -326,6 +341,9 @@ public class BaseRequestValidatorTests { "TwoFactorProviders2", new Dictionary { { "Email", null } } } })); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + // Act await _sut.ValidateAsync(context); @@ -368,6 +386,10 @@ public class BaseRequestValidatorTests .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.Email, "invalid_token") .Returns(Task.FromResult(false)); + // 5 -> set up SSO required verification to succeed + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + // Act await _sut.ValidateAsync(context); @@ -396,21 +418,25 @@ public class BaseRequestValidatorTests // 1 -> initial validation passes _sut.isValid = true; - // 2 -> set up 2FA as required + // 2 -> set up SSO required verification to succeed + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 3 -> set up 2FA as required _twoFactorAuthenticationValidator .RequiresTwoFactorAsync(Arg.Any(), tokenRequest) .Returns(Task.FromResult(new Tuple(true, null))); - // 3 -> provide invalid remember token (remember token expired) + // 4 -> provide invalid remember token (remember token expired) tokenRequest.Raw["TwoFactorToken"] = "expired_remember_token"; tokenRequest.Raw["TwoFactorProvider"] = "5"; // Remember provider - // 4 -> set up remember token verification to fail + // 5 -> set up remember token verification to fail _twoFactorAuthenticationValidator .VerifyTwoFactorAsync(user, null, TwoFactorProviderType.Remember, "expired_remember_token") .Returns(Task.FromResult(false)); - // 5 -> set up dummy BuildTwoFactorResultAsync + // 6 -> set up dummy BuildTwoFactorResultAsync var twoFactorResultDict = new Dictionary { { "TwoFactorProviders", new[] { "0", "1" } }, @@ -446,6 +472,19 @@ public class BaseRequestValidatorTests GrantValidationResult grantResult) { // Arrange + + // SsoRequestValidator sets custom response + requestContext.ValidationErrorResult = new ValidationResult + { + IsError = true, + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoRequiredDescription + }; + requestContext.CustomResponse = new Dictionary + { + { CustomResponseConstants.ResponseKeys.ErrorModel, new ErrorResponseModel(SsoConstants.RequestErrors.SsoRequiredDescription) }, + }; + var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -454,13 +493,17 @@ public class BaseRequestValidatorTests Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(false)); + // Act await _sut.ValidateAsync(context); // Assert Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - Assert.Equal("SSO authentication is required.", errorResponse.Message); + Assert.NotNull(context.GrantResult.CustomResponse); + var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; + Assert.Equal(SsoConstants.RequestErrors.SsoRequiredDescription, errorResponse.Message); } // Test grantTypes with RequireSsoPolicyRequirement when feature flag is enabled @@ -477,6 +520,20 @@ public class BaseRequestValidatorTests { // Arrange _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); + + // SsoRequestValidator sets custom response with organization identifier + requestContext.ValidationErrorResult = new ValidationResult + { + IsError = true, + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoRequiredDescription + }; + requestContext.CustomResponse = new Dictionary + { + { CustomResponseConstants.ResponseKeys.ErrorModel, new ErrorResponseModel(SsoConstants.RequestErrors.SsoRequiredDescription) }, + { CustomResponseConstants.ResponseKeys.SsoOrganizationIdentifier, "test-org-identifier" } + }; + var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -485,6 +542,10 @@ public class BaseRequestValidatorTests var requirement = new RequireSsoPolicyRequirement { SsoRequired = true }; _policyRequirementQuery.GetAsync(Arg.Any()).Returns(requirement); + // Mock the SSO validator to return false + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(false)); + // Act await _sut.ValidateAsync(context); @@ -492,8 +553,9 @@ public class BaseRequestValidatorTests await _policyService.DidNotReceive().AnyPoliciesApplicableToUserAsync( Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - Assert.Equal("SSO authentication is required.", errorResponse.Message); + Assert.NotNull(context.GrantResult.CustomResponse); + var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; + Assert.Equal(SsoConstants.RequestErrors.SsoRequiredDescription, errorResponse.Message); } [Theory] @@ -519,6 +581,10 @@ public class BaseRequestValidatorTests var requirement = new RequireSsoPolicyRequirement { SsoRequired = false }; _policyRequirementQuery.GetAsync(Arg.Any()).Returns(requirement); + // SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -561,6 +627,11 @@ public class BaseRequestValidatorTests _policyService.AnyPoliciesApplicableToUserAsync( Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) .Returns(Task.FromResult(false)); + + // SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -603,6 +674,10 @@ public class BaseRequestValidatorTests context.ValidatedTokenRequest.GrantType = grantType; + // SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -652,13 +727,15 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); // Assert Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; + var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; var expectedMessage = "Legacy encryption without a userkey is no longer supported. To recover your account, please contact support"; Assert.Equal(expectedMessage, errorResponse.Message); @@ -694,6 +771,10 @@ public class BaseRequestValidatorTests var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; + // SSO validation passes + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) @@ -760,6 +841,8 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); @@ -833,6 +916,8 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); @@ -877,6 +962,8 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); @@ -921,6 +1008,8 @@ public class BaseRequestValidatorTests .Returns(Task.FromResult(new Tuple(false, null))); _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) .Returns(Task.FromResult(true)); + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); // Act await _sut.ValidateAsync(context); @@ -950,6 +1039,19 @@ public class BaseRequestValidatorTests GrantValidationResult grantResult) { // Arrange + + // SsoRequestValidator sets custom response + requestContext.ValidationErrorResult = new ValidationResult + { + IsError = true, + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoRequiredDescription + }; + requestContext.CustomResponse = new Dictionary + { + { CustomResponseConstants.ResponseKeys.ErrorModel, new ErrorResponseModel(SsoConstants.RequestErrors.SsoRequiredDescription) }, + }; + var context = CreateContext(tokenRequest, requestContext, grantResult); var user = requestContext.User; @@ -984,12 +1086,12 @@ public class BaseRequestValidatorTests // Assert Assert.True(context.GrantResult.IsError, "Authentication should fail - SSO required after recovery"); - - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; + Assert.NotNull(context.GrantResult.CustomResponse); + var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; // Recovery succeeds, then SSO blocks with descriptive message Assert.Equal( - "Two-factor recovery has been performed. SSO authentication is required.", + SsoConstants.RequestErrors.SsoRequiredDescription, errorResponse.Message); // Verify recovery was marked @@ -1050,7 +1152,7 @@ public class BaseRequestValidatorTests // Assert Assert.True(context.GrantResult.IsError, "Authentication should fail - invalid recovery code"); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; + var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; // 2FA is checked first (due to recovery code request), fails with 2FA error Assert.Equal( @@ -1132,7 +1234,11 @@ public class BaseRequestValidatorTests _userService.IsLegacyUser(Arg.Any()) .Returns(false); - // 8. Setup user account keys for successful login response + // 8. SSO is not required + _ssoRequestValidator.ValidateAsync(requestContext.User, tokenRequest, requestContext) + .Returns(Task.FromResult(true)); + + // 9. Setup user account keys for successful login response _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData { PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( @@ -1161,179 +1267,18 @@ public class BaseRequestValidatorTests } /// - /// Tests that when RedirectOnSsoRequired is DISABLED, the legacy SSO validation path is used. - /// This validates the deprecated RequireSsoLoginAsync method is called and SSO requirement - /// is checked using the old PolicyService.AnyPoliciesApplicableToUserAsync approach. + /// Tests that when SSO validation returns a custom response, (e.g., with organization identifier), + /// that custom response is properly propagated to the result. /// [Theory] [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Disabled_UsesLegacySsoValidation( + public async Task ValidateAsync_SsoRequired_PropagatesCustomResponse( [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(false); - - var context = CreateContext(tokenRequest, requestContext, grantResult); - _sut.isValid = true; - - tokenRequest.GrantType = OidcConstants.GrantTypes.Password; - - // SSO is required via legacy path - _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) - .Returns(Task.FromResult(true)); - - // Act - await _sut.ValidateAsync(context); - - // Assert - Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - Assert.Equal("SSO authentication is required.", errorResponse.Message); - - // Verify legacy path was used - await _policyService.Received(1).AnyPoliciesApplicableToUserAsync( - requestContext.User.Id, PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); - - // Verify new SsoRequestValidator was NOT called - await _ssoRequestValidator.DidNotReceive().ValidateAsync( - Arg.Any(), Arg.Any(), Arg.Any()); - } - - /// - /// Tests that when RedirectOnSsoRequired is ENABLED, the new ISsoRequestValidator is used - /// instead of the legacy RequireSsoLoginAsync method. - /// - [Theory] - [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_UsesNewSsoRequestValidator( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); - - var context = CreateContext(tokenRequest, requestContext, grantResult); - _sut.isValid = true; - - tokenRequest.GrantType = OidcConstants.GrantTypes.Password; - - // Configure SsoRequestValidator to indicate SSO is required - _ssoRequestValidator.ValidateAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) - .Returns(Task.FromResult(false)); // false = SSO required - - // Set up the ValidationErrorResult that SsoRequestValidator would set - requestContext.ValidationErrorResult = new ValidationResult - { - IsError = true, - Error = "sso_required", - ErrorDescription = "SSO authentication is required." - }; - requestContext.CustomResponse = new Dictionary - { - { "ErrorModel", new ErrorResponseModel("SSO authentication is required.") } - }; - - // Act - await _sut.ValidateAsync(context); - - // Assert - Assert.True(context.GrantResult.IsError); - - // Verify new SsoRequestValidator was called - await _ssoRequestValidator.Received(1).ValidateAsync( - requestContext.User, - tokenRequest, - requestContext); - - // Verify legacy path was NOT used - await _policyService.DidNotReceive().AnyPoliciesApplicableToUserAsync( - Arg.Any(), Arg.Any(), Arg.Any()); - } - - /// - /// Tests that when RedirectOnSsoRequired is ENABLED and SSO is NOT required, - /// authentication continues successfully through the new validation path. - /// - [Theory] - [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_SsoNotRequired_SuccessfulLogin( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); - - var context = CreateContext(tokenRequest, requestContext, grantResult); - _sut.isValid = true; - - tokenRequest.GrantType = OidcConstants.GrantTypes.Password; - tokenRequest.ClientId = "web"; - - // SsoRequestValidator returns true (SSO not required) - _ssoRequestValidator.ValidateAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) - .Returns(Task.FromResult(true)); - - // No 2FA required - _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(requestContext.User, tokenRequest) - .Returns(Task.FromResult(new Tuple(false, null))); - - // Device validation passes - _deviceValidator.ValidateRequestDeviceAsync(tokenRequest, requestContext) - .Returns(Task.FromResult(true)); - - // User is not legacy - _userService.IsLegacyUser(Arg.Any()).Returns(false); - - _userAccountKeysQuery.Run(Arg.Any()).Returns(new UserAccountKeysData - { - PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData( - "test-private-key", - "test-public-key" - ) - }); - - // Act - await _sut.ValidateAsync(context); - - // Assert - Assert.False(context.GrantResult.IsError); - await _eventService.Received(1).LogUserEventAsync(requestContext.User.Id, EventType.User_LoggedIn); - - // Verify new validator was used - await _ssoRequestValidator.Received(1).ValidateAsync( - requestContext.User, - tokenRequest, - requestContext); - } - - /// - /// Tests that when RedirectOnSsoRequired is ENABLED and SSO validation returns a custom response - /// (e.g., with organization identifier), that custom response is properly propagated to the result. - /// - [Theory] - [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_PropagatesCustomResponse( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); _sut.isValid = true; tokenRequest.GrantType = OidcConstants.GrantTypes.Password; @@ -1342,13 +1287,13 @@ public class BaseRequestValidatorTests requestContext.ValidationErrorResult = new ValidationResult { IsError = true, - Error = "sso_required", - ErrorDescription = "SSO authentication is required." + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoRequiredDescription }; requestContext.CustomResponse = new Dictionary { - { "ErrorModel", new ErrorResponseModel("SSO authentication is required.") }, - { "SsoOrganizationIdentifier", "test-org-identifier" } + { CustomResponseConstants.ResponseKeys.ErrorModel, new ErrorResponseModel(SsoConstants.RequestErrors.SsoRequiredDescription) }, + { CustomResponseConstants.ResponseKeys.SsoOrganizationIdentifier, "test-org-identifier" } }; var context = CreateContext(tokenRequest, requestContext, grantResult); @@ -1365,77 +1310,24 @@ public class BaseRequestValidatorTests // Assert Assert.True(context.GrantResult.IsError); Assert.NotNull(context.GrantResult.CustomResponse); - Assert.Contains("SsoOrganizationIdentifier", context.CustomValidatorRequestContext.CustomResponse); + Assert.Contains(CustomResponseConstants.ResponseKeys.SsoOrganizationIdentifier, context.CustomValidatorRequestContext.CustomResponse); Assert.Equal("test-org-identifier", - context.CustomValidatorRequestContext.CustomResponse["SsoOrganizationIdentifier"]); + context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.SsoOrganizationIdentifier]); } /// - /// Tests that when RedirectOnSsoRequired is DISABLED and a user with 2FA recovery completes recovery, - /// but SSO is required, the legacy error message is returned (without the recovery-specific message). - /// - [Theory] - [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Disabled_RecoveryWithSso_LegacyMessage( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - [AuthFixtures.CustomValidatorRequestContext] - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(false); - - var context = CreateContext(tokenRequest, requestContext, grantResult); - _sut.isValid = true; - - // Recovery code scenario - tokenRequest.Raw["TwoFactorProvider"] = ((int)TwoFactorProviderType.RecoveryCode).ToString(); - tokenRequest.Raw["TwoFactorToken"] = "valid-recovery-code"; - - // 2FA with recovery - _twoFactorAuthenticationValidator - .RequiresTwoFactorAsync(requestContext.User, tokenRequest) - .Returns(Task.FromResult(new Tuple(true, null))); - - _twoFactorAuthenticationValidator - .VerifyTwoFactorAsync(requestContext.User, null, TwoFactorProviderType.RecoveryCode, "valid-recovery-code") - .Returns(Task.FromResult(true)); - - // SSO is required (legacy check) - _policyService.AnyPoliciesApplicableToUserAsync( - Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) - .Returns(Task.FromResult(true)); - - // Act - await _sut.ValidateAsync(context); - - // Assert - Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - - // Legacy behavior: recovery-specific message IS shown even without RedirectOnSsoRequired - Assert.Equal("Two-factor recovery has been performed. SSO authentication is required.", errorResponse.Message); - - // But legacy validation path was used - await _policyService.Received(1).AnyPoliciesApplicableToUserAsync( - requestContext.User.Id, PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); - } - - /// - /// Tests that when RedirectOnSsoRequired is ENABLED and recovery code is used for SSO-required user, + /// Tests that when a recovery code is used for SSO-required user, /// the SsoRequestValidator provides the recovery-specific error message. /// [Theory] [BitAutoData] - public async Task ValidateAsync_RedirectOnSsoRequired_Enabled_RecoveryWithSso_NewValidatorMessage( + public async Task ValidateAsync_RecoveryWithSso_CorrectValidatorMessage( [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, [AuthFixtures.CustomValidatorRequestContext] CustomValidatorRequestContext requestContext, GrantValidationResult grantResult) { // Arrange - _featureService.IsEnabled(FeatureFlagKeys.RedirectOnSsoRequired).Returns(true); - var context = CreateContext(tokenRequest, requestContext, grantResult); _sut.isValid = true; @@ -1457,14 +1349,14 @@ public class BaseRequestValidatorTests requestContext.ValidationErrorResult = new ValidationResult { IsError = true, - Error = "sso_required", - ErrorDescription = "Two-factor recovery has been performed. SSO authentication is required." + Error = SsoConstants.RequestErrors.SsoRequired, + ErrorDescription = SsoConstants.RequestErrors.SsoTwoFactorRecoveryDescription }; requestContext.CustomResponse = new Dictionary { { - "ErrorModel", - new ErrorResponseModel("Two-factor recovery has been performed. SSO authentication is required.") + CustomResponseConstants.ResponseKeys.ErrorModel, + new ErrorResponseModel(SsoConstants.RequestErrors.SsoTwoFactorRecoveryDescription) } }; @@ -1479,18 +1371,8 @@ public class BaseRequestValidatorTests // Assert Assert.True(context.GrantResult.IsError); - var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse["ErrorModel"]; - Assert.Equal("Two-factor recovery has been performed. SSO authentication is required.", errorResponse.Message); - - // Verify new validator was used - await _ssoRequestValidator.Received(1).ValidateAsync( - requestContext.User, - tokenRequest, - Arg.Is(ctx => ctx.TwoFactorRecoveryRequested)); - - // Verify legacy path was NOT used - await _policyService.DidNotReceive().AnyPoliciesApplicableToUserAsync( - Arg.Any(), Arg.Any(), Arg.Any()); + var errorResponse = (ErrorResponseModel)context.CustomValidatorRequestContext.CustomResponse[CustomResponseConstants.ResponseKeys.ErrorModel]; + Assert.Equal(SsoConstants.RequestErrors.SsoTwoFactorRecoveryDescription, errorResponse.Message); } private BaseRequestValidationContextFake CreateContext( diff --git a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs index b336e4c3c1..ac27c55466 100644 --- a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs +++ b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs @@ -111,15 +111,6 @@ IBaseRequestValidatorTestWrapper context.GrantResult = new GrantValidationResult(TokenRequestErrors.InvalidGrant, customResponse: customResponse); } - [Obsolete] - protected override void SetSsoResult( - BaseRequestValidationContextFake context, - Dictionary customResponse) - { - context.GrantResult = new GrantValidationResult( - TokenRequestErrors.InvalidGrant, "Sso authentication required.", customResponse); - } - protected override Task SetSuccessResult( BaseRequestValidationContextFake context, User user, From 2e4dd061e313caf96e321da374ea5be61d9865d0 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Tue, 20 Jan 2026 09:18:27 -0600 Subject: [PATCH 05/51] [PM-30855] Pay prorated storage adjustment immediately with Braintree for Premium PayPal users (#6850) * fix: Pay prorated storage invoice immediately with Braintree for PayPal users * Run dotnet format --- .../Commands/UpdatePremiumStorageCommand.cs | 47 +++- src/Core/Billing/Services/IStripeAdapter.cs | 1 + .../Services/Implementations/StripeAdapter.cs | 3 + .../UpdatePremiumStorageCommandTests.cs | 230 +++++++++++++++++- 4 files changed, 263 insertions(+), 18 deletions(-) diff --git a/src/Core/Billing/Premium/Commands/UpdatePremiumStorageCommand.cs b/src/Core/Billing/Premium/Commands/UpdatePremiumStorageCommand.cs index 176c77bf57..219f450f1d 100644 --- a/src/Core/Billing/Premium/Commands/UpdatePremiumStorageCommand.cs +++ b/src/Core/Billing/Premium/Commands/UpdatePremiumStorageCommand.cs @@ -2,6 +2,7 @@ using Bit.Core.Billing.Constants; using Bit.Core.Billing.Pricing; using Bit.Core.Billing.Services; +using Bit.Core.Billing.Subscriptions.Models; using Bit.Core.Entities; using Bit.Core.Services; using Bit.Core.Utilities; @@ -29,6 +30,7 @@ public interface IUpdatePremiumStorageCommand } public class UpdatePremiumStorageCommand( + IBraintreeService braintreeService, IStripeAdapter stripeAdapter, IUserService userService, IPricingClient pricingClient, @@ -49,7 +51,10 @@ public class UpdatePremiumStorageCommand( // Fetch all premium plans and the user's subscription to find which plan they're on var premiumPlans = await pricingClient.ListPremiumPlans(); - var subscription = await stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId); + var subscription = await stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId, new SubscriptionGetOptions + { + Expand = ["customer"] + }); // Find the password manager subscription item (seat, not storage) and match it to a plan var passwordManagerItem = subscription.Items.Data.FirstOrDefault(i => @@ -127,13 +132,41 @@ public class UpdatePremiumStorageCommand( }); } - var subscriptionUpdateOptions = new SubscriptionUpdateOptions - { - Items = subscriptionItemOptions, - ProrationBehavior = ProrationBehavior.AlwaysInvoice - }; + var usingPayPal = subscription.Customer.Metadata.ContainsKey(MetadataKeys.BraintreeCustomerId); - await stripeAdapter.UpdateSubscriptionAsync(subscription.Id, subscriptionUpdateOptions); + if (usingPayPal) + { + var options = new SubscriptionUpdateOptions + { + Items = subscriptionItemOptions, + ProrationBehavior = ProrationBehavior.CreateProrations + }; + + await stripeAdapter.UpdateSubscriptionAsync(subscription.Id, options); + + var draftInvoice = await stripeAdapter.CreateInvoiceAsync(new InvoiceCreateOptions + { + Customer = subscription.CustomerId, + Subscription = subscription.Id, + AutoAdvance = false, + CollectionMethod = CollectionMethod.ChargeAutomatically + }); + + var finalizedInvoice = await stripeAdapter.FinalizeInvoiceAsync(draftInvoice.Id, + new InvoiceFinalizeOptions { AutoAdvance = false, Expand = ["customer"] }); + + await braintreeService.PayInvoice(new UserId(user.Id), finalizedInvoice); + } + else + { + var options = new SubscriptionUpdateOptions + { + Items = subscriptionItemOptions, + ProrationBehavior = ProrationBehavior.AlwaysInvoice + }; + + await stripeAdapter.UpdateSubscriptionAsync(subscription.Id, options); + } // Update the user's max storage user.MaxStorageGb = maxStorageGb; diff --git a/src/Core/Billing/Services/IStripeAdapter.cs b/src/Core/Billing/Services/IStripeAdapter.cs index 5ec732920e..12ea3d5a7c 100644 --- a/src/Core/Billing/Services/IStripeAdapter.cs +++ b/src/Core/Billing/Services/IStripeAdapter.cs @@ -24,6 +24,7 @@ public interface IStripeAdapter Task CancelSubscriptionAsync(string id, SubscriptionCancelOptions options = null); Task GetInvoiceAsync(string id, InvoiceGetOptions options); Task> ListInvoicesAsync(StripeInvoiceListOptions options); + Task CreateInvoiceAsync(InvoiceCreateOptions options); Task CreateInvoicePreviewAsync(InvoiceCreatePreviewOptions options); Task> SearchInvoiceAsync(InvoiceSearchOptions options); Task UpdateInvoiceAsync(string id, InvoiceUpdateOptions options); diff --git a/src/Core/Billing/Services/Implementations/StripeAdapter.cs b/src/Core/Billing/Services/Implementations/StripeAdapter.cs index cdc7645042..5b90500021 100644 --- a/src/Core/Billing/Services/Implementations/StripeAdapter.cs +++ b/src/Core/Billing/Services/Implementations/StripeAdapter.cs @@ -116,6 +116,9 @@ public class StripeAdapter : IStripeAdapter return invoices; } + public Task CreateInvoiceAsync(InvoiceCreateOptions options) => + _invoiceService.CreateAsync(options); + public Task CreateInvoicePreviewAsync(InvoiceCreatePreviewOptions options) => _invoiceService.CreatePreviewAsync(options); diff --git a/test/Core.Test/Billing/Premium/Commands/UpdatePremiumStorageCommandTests.cs b/test/Core.Test/Billing/Premium/Commands/UpdatePremiumStorageCommandTests.cs index 7b9b68c757..cd9b323f9d 100644 --- a/test/Core.Test/Billing/Premium/Commands/UpdatePremiumStorageCommandTests.cs +++ b/test/Core.Test/Billing/Premium/Commands/UpdatePremiumStorageCommandTests.cs @@ -1,6 +1,7 @@ using Bit.Core.Billing.Premium.Commands; using Bit.Core.Billing.Pricing; using Bit.Core.Billing.Services; +using Bit.Core.Billing.Subscriptions.Models; using Bit.Core.Entities; using Bit.Core.Services; using Bit.Test.Common.AutoFixture.Attributes; @@ -8,6 +9,7 @@ using Microsoft.Extensions.Logging; using NSubstitute; using Stripe; using Xunit; +using static Bit.Core.Billing.Constants.StripeConstants; using PremiumPlan = Bit.Core.Billing.Pricing.Premium.Plan; using PremiumPurchasable = Bit.Core.Billing.Pricing.Premium.Purchasable; @@ -15,6 +17,7 @@ namespace Bit.Core.Test.Billing.Premium.Commands; public class UpdatePremiumStorageCommandTests { + private readonly IBraintreeService _braintreeService = Substitute.For(); private readonly IStripeAdapter _stripeAdapter = Substitute.For(); private readonly IUserService _userService = Substitute.For(); private readonly IPricingClient _pricingClient = Substitute.For(); @@ -33,13 +36,14 @@ public class UpdatePremiumStorageCommandTests _pricingClient.ListPremiumPlans().Returns([premiumPlan]); _command = new UpdatePremiumStorageCommand( + _braintreeService, _stripeAdapter, _userService, _pricingClient, Substitute.For>()); } - private Subscription CreateMockSubscription(string subscriptionId, int? storageQuantity = null) + private Subscription CreateMockSubscription(string subscriptionId, int? storageQuantity = null, bool isPayPal = false) { var items = new List { @@ -63,9 +67,17 @@ public class UpdatePremiumStorageCommandTests }); } + var customer = new Customer + { + Id = "cus_123", + Metadata = isPayPal ? new Dictionary { { MetadataKeys.BraintreeCustomerId, "braintree_123" } } : new Dictionary() + }; + return new Subscription { Id = subscriptionId, + CustomerId = "cus_123", + Customer = customer, Items = new StripeList { Data = items @@ -97,7 +109,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123", 4); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, -5); @@ -117,7 +129,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123", 4); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, 100); @@ -154,7 +166,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123", 9); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, 0); @@ -176,7 +188,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123", 4); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, 4); @@ -185,7 +197,7 @@ public class UpdatePremiumStorageCommandTests Assert.True(result.IsT0); // Verify subscription was fetched but NOT updated - await _stripeAdapter.Received(1).GetSubscriptionAsync("sub_123"); + await _stripeAdapter.Received(1).GetSubscriptionAsync("sub_123", Arg.Any()); await _stripeAdapter.DidNotReceive().UpdateSubscriptionAsync(Arg.Any(), Arg.Any()); await _userService.DidNotReceive().SaveUserAsync(Arg.Any()); } @@ -200,7 +212,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123", 4); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, 9); @@ -233,7 +245,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123"); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, 9); @@ -262,7 +274,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123", 9); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, 2); @@ -291,7 +303,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123", 9); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, 0); @@ -320,7 +332,7 @@ public class UpdatePremiumStorageCommandTests user.GatewaySubscriptionId = "sub_123"; var subscription = CreateMockSubscription("sub_123", 4); - _stripeAdapter.GetSubscriptionAsync("sub_123").Returns(subscription); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); // Act var result = await _command.Run(user, 99); @@ -335,4 +347,200 @@ public class UpdatePremiumStorageCommandTests await _userService.Received(1).SaveUserAsync(Arg.Is(u => u.MaxStorageGb == 100)); } + + [Theory, BitAutoData] + public async Task Run_IncreaseStorage_PayPal_Success(User user) + { + // Arrange + user.Premium = true; + user.MaxStorageGb = 5; + user.Storage = 2L * 1024 * 1024 * 1024; + user.GatewaySubscriptionId = "sub_123"; + + var subscription = CreateMockSubscription("sub_123", 4, isPayPal: true); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); + + var draftInvoice = new Invoice { Id = "in_draft" }; + _stripeAdapter.CreateInvoiceAsync(Arg.Any()).Returns(draftInvoice); + + var finalizedInvoice = new Invoice + { + Id = "in_finalized", + Customer = new Customer { Id = "cus_123" } + }; + _stripeAdapter.FinalizeInvoiceAsync("in_draft", Arg.Any()).Returns(finalizedInvoice); + + // Act + var result = await _command.Run(user, 9); + + // Assert + Assert.True(result.IsT0); + + // Verify subscription was updated with CreateProrations + await _stripeAdapter.Received(1).UpdateSubscriptionAsync( + "sub_123", + Arg.Is(opts => + opts.Items.Count == 1 && + opts.Items[0].Id == "si_storage" && + opts.Items[0].Quantity == 9 && + opts.ProrationBehavior == "create_prorations")); + + // Verify draft invoice was created + await _stripeAdapter.Received(1).CreateInvoiceAsync( + Arg.Is(opts => + opts.Customer == "cus_123" && + opts.Subscription == "sub_123" && + opts.AutoAdvance == false && + opts.CollectionMethod == "charge_automatically")); + + // Verify invoice was finalized + await _stripeAdapter.Received(1).FinalizeInvoiceAsync( + "in_draft", + Arg.Is(opts => + opts.AutoAdvance == false && + opts.Expand.Contains("customer"))); + + // Verify Braintree payment was processed + await _braintreeService.Received(1).PayInvoice(Arg.Any(), finalizedInvoice); + + // Verify user was saved + await _userService.Received(1).SaveUserAsync(Arg.Is(u => + u.Id == user.Id && + u.MaxStorageGb == 10)); + } + + [Theory, BitAutoData] + public async Task Run_AddStorageFromZero_PayPal_Success(User user) + { + // Arrange + user.Premium = true; + user.MaxStorageGb = 1; + user.Storage = 500L * 1024 * 1024; + user.GatewaySubscriptionId = "sub_123"; + + var subscription = CreateMockSubscription("sub_123", isPayPal: true); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); + + var draftInvoice = new Invoice { Id = "in_draft" }; + _stripeAdapter.CreateInvoiceAsync(Arg.Any()).Returns(draftInvoice); + + var finalizedInvoice = new Invoice + { + Id = "in_finalized", + Customer = new Customer { Id = "cus_123" } + }; + _stripeAdapter.FinalizeInvoiceAsync("in_draft", Arg.Any()).Returns(finalizedInvoice); + + // Act + var result = await _command.Run(user, 9); + + // Assert + Assert.True(result.IsT0); + + // Verify subscription was updated with new storage item + await _stripeAdapter.Received(1).UpdateSubscriptionAsync( + "sub_123", + Arg.Is(opts => + opts.Items.Count == 1 && + opts.Items[0].Price == "price_storage" && + opts.Items[0].Quantity == 9 && + opts.ProrationBehavior == "create_prorations")); + + // Verify invoice creation and payment flow + await _stripeAdapter.Received(1).CreateInvoiceAsync(Arg.Any()); + await _stripeAdapter.Received(1).FinalizeInvoiceAsync("in_draft", Arg.Any()); + await _braintreeService.Received(1).PayInvoice(Arg.Any(), finalizedInvoice); + + await _userService.Received(1).SaveUserAsync(Arg.Is(u => u.MaxStorageGb == 10)); + } + + [Theory, BitAutoData] + public async Task Run_DecreaseStorage_PayPal_Success(User user) + { + // Arrange + user.Premium = true; + user.MaxStorageGb = 10; + user.Storage = 2L * 1024 * 1024 * 1024; + user.GatewaySubscriptionId = "sub_123"; + + var subscription = CreateMockSubscription("sub_123", 9, isPayPal: true); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); + + var draftInvoice = new Invoice { Id = "in_draft" }; + _stripeAdapter.CreateInvoiceAsync(Arg.Any()).Returns(draftInvoice); + + var finalizedInvoice = new Invoice + { + Id = "in_finalized", + Customer = new Customer { Id = "cus_123" } + }; + _stripeAdapter.FinalizeInvoiceAsync("in_draft", Arg.Any()).Returns(finalizedInvoice); + + // Act + var result = await _command.Run(user, 2); + + // Assert + Assert.True(result.IsT0); + + // Verify subscription was updated + await _stripeAdapter.Received(1).UpdateSubscriptionAsync( + "sub_123", + Arg.Is(opts => + opts.Items.Count == 1 && + opts.Items[0].Id == "si_storage" && + opts.Items[0].Quantity == 2 && + opts.ProrationBehavior == "create_prorations")); + + // Verify invoice creation and payment flow + await _stripeAdapter.Received(1).CreateInvoiceAsync(Arg.Any()); + await _stripeAdapter.Received(1).FinalizeInvoiceAsync("in_draft", Arg.Any()); + await _braintreeService.Received(1).PayInvoice(Arg.Any(), finalizedInvoice); + + await _userService.Received(1).SaveUserAsync(Arg.Is(u => u.MaxStorageGb == 3)); + } + + [Theory, BitAutoData] + public async Task Run_RemoveAllAdditionalStorage_PayPal_Success(User user) + { + // Arrange + user.Premium = true; + user.MaxStorageGb = 10; + user.Storage = 500L * 1024 * 1024; + user.GatewaySubscriptionId = "sub_123"; + + var subscription = CreateMockSubscription("sub_123", 9, isPayPal: true); + _stripeAdapter.GetSubscriptionAsync("sub_123", Arg.Any()).Returns(subscription); + + var draftInvoice = new Invoice { Id = "in_draft" }; + _stripeAdapter.CreateInvoiceAsync(Arg.Any()).Returns(draftInvoice); + + var finalizedInvoice = new Invoice + { + Id = "in_finalized", + Customer = new Customer { Id = "cus_123" } + }; + _stripeAdapter.FinalizeInvoiceAsync("in_draft", Arg.Any()).Returns(finalizedInvoice); + + // Act + var result = await _command.Run(user, 0); + + // Assert + Assert.True(result.IsT0); + + // Verify subscription item was deleted + await _stripeAdapter.Received(1).UpdateSubscriptionAsync( + "sub_123", + Arg.Is(opts => + opts.Items.Count == 1 && + opts.Items[0].Id == "si_storage" && + opts.Items[0].Deleted == true && + opts.ProrationBehavior == "create_prorations")); + + // Verify invoice creation and payment flow + await _stripeAdapter.Received(1).CreateInvoiceAsync(Arg.Any()); + await _stripeAdapter.Received(1).FinalizeInvoiceAsync("in_draft", Arg.Any()); + await _braintreeService.Received(1).PayInvoice(Arg.Any(), finalizedInvoice); + + await _userService.Received(1).SaveUserAsync(Arg.Is(u => u.MaxStorageGb == 1)); + } } From 439485fc1687692cec131cc413ce4e5a78058cec Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Tue, 20 Jan 2026 09:29:49 -0600 Subject: [PATCH 06/51] Update renewal email copy (#6862) --- .../Renewals/families-2019-renewal.mjml | 2 +- .../Billing/Renewals/premium-renewal.mjml | 2 +- .../Families2019RenewalMailView.html.hbs | 74 +++++++++---------- .../Families2019RenewalMailView.text.hbs | 2 +- .../Premium/PremiumRenewalMailView.html.hbs | 74 +++++++++---------- .../Premium/PremiumRenewalMailView.text.hbs | 2 +- 6 files changed, 78 insertions(+), 78 deletions(-) diff --git a/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/families-2019-renewal.mjml b/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/families-2019-renewal.mjml index 092ae303de..11d82e2039 100644 --- a/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/families-2019-renewal.mjml +++ b/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/families-2019-renewal.mjml @@ -19,7 +19,7 @@ As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. - This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. + This year's renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. Questions? Contact diff --git a/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/premium-renewal.mjml b/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/premium-renewal.mjml index a460442a7c..1fe48c9ba9 100644 --- a/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/premium-renewal.mjml +++ b/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/premium-renewal.mjml @@ -18,7 +18,7 @@ As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. - This renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually. + This year's renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually. Questions? Contact diff --git a/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.html.hbs b/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.html.hbs index 227613999b..0befde11b5 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.html.hbs +++ b/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.html.hbs @@ -203,7 +203,7 @@
As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. - This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax.
+ This year's renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. @@ -271,12 +271,12 @@ - + -
+
- +
@@ -364,8 +364,8 @@ - -
- + +
@@ -381,13 +381,13 @@
- +
+ - @@ -404,13 +404,13 @@ -
+ - +
- +
+ - @@ -427,13 +427,13 @@ -
+ - +
- +
+ - @@ -450,13 +450,13 @@ -
+ - +
- +
+ - @@ -473,13 +473,13 @@ -
+ - +
- +
+ - @@ -496,13 +496,13 @@ -
+ - +
- +
+ - @@ -519,13 +519,13 @@ -
+ - +
- +
+ - @@ -546,15 +546,15 @@ diff --git a/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.text.hbs b/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.text.hbs index 88d64f9acf..7178548772 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.text.hbs +++ b/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.text.hbs @@ -2,6 +2,6 @@ at {{BaseAnnualRenewalPrice}} + tax. As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. -This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. +This year's renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. Questions? Contact support@bitwarden.com diff --git a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs index a6b2fda0f7..9ce45ef7fe 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs +++ b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs @@ -202,7 +202,7 @@ @@ -270,12 +270,12 @@
+ - +
-

+

© 2025 Bitwarden Inc. 1 N. Calle Cesar Chavez, Suite 102, Santa Barbara, CA, USA

Always confirm you are on a trusted Bitwarden domain before logging in:
- bitwarden.com | - Learn why we include this + bitwarden.com | + Learn why we include this

As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. - This renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually.
+ This year's renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually.
- + -
+
- +
@@ -363,8 +363,8 @@ - -
- + +
@@ -380,13 +380,13 @@
- +
+ - @@ -403,13 +403,13 @@ -
+ - +
- +
+ - @@ -426,13 +426,13 @@ -
+ - +
- +
+ - @@ -449,13 +449,13 @@ -
+ - +
- +
+ - @@ -472,13 +472,13 @@ -
+ - +
- +
+ - @@ -495,13 +495,13 @@ -
+ - +
- +
+ - @@ -518,13 +518,13 @@ -
+ - +
- +
+ - @@ -545,15 +545,15 @@ diff --git a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs index 41300d0f96..15ad530a07 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs +++ b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs @@ -1,6 +1,6 @@ Your Bitwarden Premium subscription renews in 15 days. The price is updating to {{BaseMonthlyRenewalPrice}}/month, billed annually. As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. -This renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually. +This year's renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually. Questions? Contact support@bitwarden.com From 7fb2822e05114f61c54ea2f6f0795b7c7425df3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 21 Jan 2026 11:27:24 +0000 Subject: [PATCH 07/51] =?UTF-8?q?[PM-28023]=C2=A0Fix=20restoring=20revoked?= =?UTF-8?q?=20invited=20users=20in=20Free=20Organizations=20(#6861)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix null reference when restoring invited users in Free orgs Add null check before querying for other free org ownership. Invited users don't have a UserId yet, causing NullReferenceException. * Add regression test for restoring revoked invited users with null UserId. --- .../v1/RestoreOrganizationUserCommand.cs | 2 +- .../RestoreOrganizationUserCommandTests.cs | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs index ec42c8b402..c5b7314730 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -93,7 +93,7 @@ public class RestoreOrganizationUserCommand( .twoFactorIsEnabled; } - if (organization.PlanType == PlanType.Free) + if (organization.PlanType == PlanType.Free && organizationUser.UserId.HasValue) { await CheckUserForOtherFreeOrganizationOwnershipAsync(organizationUser); } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs index 4fa5e92abe..a75345a05d 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs @@ -715,6 +715,39 @@ public class RestoreOrganizationUserCommandTests Arg.Is(x => x != OrganizationUserStatusType.Revoked)); } + [Theory, BitAutoData] + public async Task RestoreUser_InvitedUserInFreeOrganization_Success( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + organization.PlanType = PlanType.Free; + organizationUser.UserId = null; + organizationUser.Key = null; + organizationUser.Status = OrganizationUserStatusType.Revoked; + + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + sutProvider.GetDependency() + .GetOccupiedSeatCountByOrganizationIdAsync(organization.Id).Returns(new OrganizationSeatCounts + { + Sponsored = 0, + Users = 1 + }); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + [Theory, BitAutoData] public async Task RestoreUsers_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, From 75a857055e74c452cfdc96cd4c1143439a3d2f8e Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Wed, 21 Jan 2026 11:52:36 -0600 Subject: [PATCH 08/51] [PM-30697] [PM-30698] Renewal email copy updates (#6875) * feat(families-renewal): Update copy * feat(premium-renewal): Add new var, update copy --- .../Services/Implementations/UpcomingInvoiceHandler.cs | 2 +- .../Mjml/emails/Billing/Renewals/families-2019-renewal.mjml | 4 ++-- .../Mjml/emails/Billing/Renewals/premium-renewal.mjml | 4 ++-- .../Families2019Renewal/Families2019RenewalMailView.html.hbs | 4 ++-- .../Families2019Renewal/Families2019RenewalMailView.text.hbs | 4 ++-- .../Mail/Billing/Renewal/Premium/PremiumRenewalMailView.cs | 2 +- .../Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs | 4 ++-- .../Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs | 4 ++-- test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs | 4 ++-- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs b/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs index 004828dc48..ae2a76a7ce 100644 --- a/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs +++ b/src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs @@ -627,7 +627,7 @@ public class UpcomingInvoiceHandler( { BaseMonthlyRenewalPrice = (premiumPlan.Seat.Price / 12).ToString("C", new CultureInfo("en-US")), DiscountAmount = $"{coupon.PercentOff}%", - DiscountedMonthlyRenewalPrice = (discountedAnnualRenewalPrice / 12).ToString("C", new CultureInfo("en-US")) + DiscountedAnnualRenewalPrice = discountedAnnualRenewalPrice.ToString("C", new CultureInfo("en-US")) } }; diff --git a/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/families-2019-renewal.mjml b/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/families-2019-renewal.mjml index 11d82e2039..06f60e7724 100644 --- a/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/families-2019-renewal.mjml +++ b/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/families-2019-renewal.mjml @@ -18,8 +18,8 @@ at {{BaseAnnualRenewalPrice}} + tax. - As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. - This year's renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. + As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this year's renewal. + This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. Questions? Contact diff --git a/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/premium-renewal.mjml b/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/premium-renewal.mjml index 1fe48c9ba9..defec91f0e 100644 --- a/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/premium-renewal.mjml +++ b/src/Core/MailTemplates/Mjml/emails/Billing/Renewals/premium-renewal.mjml @@ -17,8 +17,8 @@ Your Bitwarden Premium subscription renews in 15 days. The price is updating to {{BaseMonthlyRenewalPrice}}/month, billed annually. - As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. - This year's renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually. + As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this year's renewal. + This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. Questions? Contact diff --git a/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.html.hbs b/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.html.hbs index 0befde11b5..2d7c9edf35 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.html.hbs +++ b/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.html.hbs @@ -202,8 +202,8 @@ diff --git a/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.text.hbs b/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.text.hbs index 7178548772..9f40c88329 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.text.hbs +++ b/src/Core/Models/Mail/Billing/Renewal/Families2019Renewal/Families2019RenewalMailView.text.hbs @@ -1,7 +1,7 @@ Your Bitwarden Families subscription renews in 15 days. The price is updating to {{BaseMonthlyRenewalPrice}}/month, billed annually at {{BaseAnnualRenewalPrice}} + tax. -As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. -This year's renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. +As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this year's renewal. +This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. Questions? Contact support@bitwarden.com diff --git a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.cs b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.cs index 4006c92a63..0798c7dbc8 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.cs +++ b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.cs @@ -5,7 +5,7 @@ namespace Bit.Core.Models.Mail.Billing.Renewal.Premium; public class PremiumRenewalMailView : BaseMailView { public required string BaseMonthlyRenewalPrice { get; set; } - public required string DiscountedMonthlyRenewalPrice { get; set; } + public required string DiscountedAnnualRenewalPrice { get; set; } public required string DiscountAmount { get; set; } } diff --git a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs index 9ce45ef7fe..db76520eed 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs +++ b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.html.hbs @@ -201,8 +201,8 @@ diff --git a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs index 15ad530a07..4b79826f71 100644 --- a/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs +++ b/src/Core/Models/Mail/Billing/Renewal/Premium/PremiumRenewalMailView.text.hbs @@ -1,6 +1,6 @@ Your Bitwarden Premium subscription renews in 15 days. The price is updating to {{BaseMonthlyRenewalPrice}}/month, billed annually. -As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. -This year's renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually. +As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this year's renewal. +This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax. Questions? Contact support@bitwarden.com diff --git a/test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs b/test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs index 3b133c7d37..82d6c8acfd 100644 --- a/test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs +++ b/test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs @@ -280,7 +280,7 @@ public class UpcomingInvoiceHandlerTests email.ToEmails.Contains("user@example.com") && email.Subject == "Your Bitwarden Premium renewal is updating" && email.View.BaseMonthlyRenewalPrice == (plan.Seat.Price / 12).ToString("C", new CultureInfo("en-US")) && - email.View.DiscountedMonthlyRenewalPrice == (discountedPrice / 12).ToString("C", new CultureInfo("en-US")) && + email.View.DiscountedAnnualRenewalPrice == discountedPrice.ToString("C", new CultureInfo("en-US")) && email.View.DiscountAmount == $"{coupon.PercentOff}%" )); } @@ -2436,7 +2436,7 @@ public class UpcomingInvoiceHandlerTests email.Subject == "Your Bitwarden Premium renewal is updating" && email.View.BaseMonthlyRenewalPrice == (plan.Seat.Price / 12).ToString("C", new CultureInfo("en-US")) && email.View.DiscountAmount == "30%" && - email.View.DiscountedMonthlyRenewalPrice == (expectedDiscountedPrice / 12).ToString("C", new CultureInfo("en-US")) + email.View.DiscountedAnnualRenewalPrice == expectedDiscountedPrice.ToString("C", new CultureInfo("en-US")) )); await _mailService.DidNotReceive().SendInvoiceUpcoming( From b686da18dcdeecd93d2774fa348d16960cc4c959 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Thu, 22 Jan 2026 09:01:06 -0600 Subject: [PATCH 09/51] [PM-30626] Fetch provided storage from Pricing Service when determining storage limit (#6845) * Fetch provided storage from Pricing Service * Run dotnet format * Gbubemi's feedback --- .../Services/SendValidationService.cs | 22 ++- .../Services/Implementations/CipherService.cs | 23 ++- .../Services/SendValidationServiceTests.cs | 120 +++++++++++ .../Vault/Services/CipherServiceTests.cs | 186 +++++++++++++++++- 4 files changed, 337 insertions(+), 14 deletions(-) create mode 100644 test/Core.Test/Tools/Services/SendValidationServiceTests.cs diff --git a/src/Core/Tools/SendFeatures/Services/SendValidationService.cs b/src/Core/Tools/SendFeatures/Services/SendValidationService.cs index c545c8b35f..bd987bb396 100644 --- a/src/Core/Tools/SendFeatures/Services/SendValidationService.cs +++ b/src/Core/Tools/SendFeatures/Services/SendValidationService.cs @@ -6,6 +6,7 @@ using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; +using Bit.Core.Billing.Pricing; using Bit.Core.Context; using Bit.Core.Exceptions; using Bit.Core.Repositories; @@ -27,6 +28,7 @@ public class SendValidationService : ISendValidationService private readonly GlobalSettings _globalSettings; private readonly ICurrentContext _currentContext; private readonly IPolicyRequirementQuery _policyRequirementQuery; + private readonly IPricingClient _pricingClient; @@ -38,7 +40,7 @@ public class SendValidationService : ISendValidationService IUserService userService, IPolicyRequirementQuery policyRequirementQuery, GlobalSettings globalSettings, - + IPricingClient pricingClient, ICurrentContext currentContext) { _userRepository = userRepository; @@ -48,6 +50,7 @@ public class SendValidationService : ISendValidationService _userService = userService; _policyRequirementQuery = policyRequirementQuery; _globalSettings = globalSettings; + _pricingClient = pricingClient; _currentContext = currentContext; } @@ -123,10 +126,19 @@ public class SendValidationService : ISendValidationService } else { - // Users that get access to file storage/premium from their organization get the default - // 1 GB max storage. - short limit = _globalSettings.SelfHosted ? Constants.SelfHostedMaxStorageGb : (short)1; - storageBytesRemaining = user.StorageBytesRemaining(limit); + // Users that get access to file storage/premium from their organization get storage + // based on the current premium plan from the pricing service + short provided; + if (_globalSettings.SelfHosted) + { + provided = Constants.SelfHostedMaxStorageGb; + } + else + { + var premiumPlan = await _pricingClient.GetAvailablePremiumPlan(); + provided = (short)premiumPlan.Storage.Provided; + } + storageBytesRemaining = user.StorageBytesRemaining(provided); } } else if (send.OrganizationId.HasValue) diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index fa2cfbb209..140399a37a 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -7,6 +7,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; +using Bit.Core.Billing.Pricing; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Platform.Push; @@ -46,6 +47,7 @@ public class CipherService : ICipherService private readonly IPolicyRequirementQuery _policyRequirementQuery; private readonly IApplicationCacheService _applicationCacheService; private readonly IFeatureService _featureService; + private readonly IPricingClient _pricingClient; public CipherService( ICipherRepository cipherRepository, @@ -65,7 +67,8 @@ public class CipherService : ICipherService IGetCipherPermissionsForUserQuery getCipherPermissionsForUserQuery, IPolicyRequirementQuery policyRequirementQuery, IApplicationCacheService applicationCacheService, - IFeatureService featureService) + IFeatureService featureService, + IPricingClient pricingClient) { _cipherRepository = cipherRepository; _folderRepository = folderRepository; @@ -85,6 +88,7 @@ public class CipherService : ICipherService _policyRequirementQuery = policyRequirementQuery; _applicationCacheService = applicationCacheService; _featureService = featureService; + _pricingClient = pricingClient; } public async Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnownRevisionDate, @@ -943,10 +947,19 @@ public class CipherService : ICipherService } else { - // Users that get access to file storage/premium from their organization get the default - // 1 GB max storage. - storageBytesRemaining = user.StorageBytesRemaining( - _globalSettings.SelfHosted ? Constants.SelfHostedMaxStorageGb : (short)1); + // Users that get access to file storage/premium from their organization get storage + // based on the current premium plan from the pricing service + short provided; + if (_globalSettings.SelfHosted) + { + provided = Constants.SelfHostedMaxStorageGb; + } + else + { + var premiumPlan = await _pricingClient.GetAvailablePremiumPlan(); + provided = (short)premiumPlan.Storage.Provided; + } + storageBytesRemaining = user.StorageBytesRemaining(provided); } } else if (cipher.OrganizationId.HasValue) diff --git a/test/Core.Test/Tools/Services/SendValidationServiceTests.cs b/test/Core.Test/Tools/Services/SendValidationServiceTests.cs new file mode 100644 index 0000000000..8adce1a29f --- /dev/null +++ b/test/Core.Test/Tools/Services/SendValidationServiceTests.cs @@ -0,0 +1,120 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Billing.Pricing; +using Bit.Core.Billing.Pricing.Premium; +using Bit.Core.Entities; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Tools.Entities; +using Bit.Core.Tools.Enums; +using Bit.Core.Tools.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.Tools.Services; + +[SutProviderCustomize] +public class SendValidationServiceTests +{ + [Theory, BitAutoData] + public async Task StorageRemainingForSendAsync_OrgGrantedPremiumUser_UsesPricingService( + SutProvider sutProvider, + Send send, + User user) + { + // Arrange + send.UserId = user.Id; + send.OrganizationId = null; + send.Type = SendType.File; + user.Premium = false; + user.Storage = 1024L * 1024L * 1024L; // 1 GB used + user.EmailVerified = true; + + sutProvider.GetDependency().SelfHosted = false; + sutProvider.GetDependency().GetByIdAsync(user.Id).Returns(user); + sutProvider.GetDependency().CanAccessPremium(user).Returns(true); + + var premiumPlan = new Plan + { + Storage = new Purchasable { Provided = 5 } + }; + sutProvider.GetDependency().GetAvailablePremiumPlan().Returns(premiumPlan); + + // Act + var result = await sutProvider.Sut.StorageRemainingForSendAsync(send); + + // Assert + await sutProvider.GetDependency().Received(1).GetAvailablePremiumPlan(); + Assert.True(result > 0); + } + + [Theory, BitAutoData] + public async Task StorageRemainingForSendAsync_IndividualPremium_DoesNotCallPricingService( + SutProvider sutProvider, + Send send, + User user) + { + // Arrange + send.UserId = user.Id; + send.OrganizationId = null; + send.Type = SendType.File; + user.Premium = true; + user.MaxStorageGb = 10; + user.EmailVerified = true; + + sutProvider.GetDependency().GetByIdAsync(user.Id).Returns(user); + sutProvider.GetDependency().CanAccessPremium(user).Returns(true); + + // Act + var result = await sutProvider.Sut.StorageRemainingForSendAsync(send); + + // Assert - should NOT call pricing service for individual premium users + await sutProvider.GetDependency().DidNotReceive().GetAvailablePremiumPlan(); + } + + [Theory, BitAutoData] + public async Task StorageRemainingForSendAsync_SelfHosted_DoesNotCallPricingService( + SutProvider sutProvider, + Send send, + User user) + { + // Arrange + send.UserId = user.Id; + send.OrganizationId = null; + send.Type = SendType.File; + user.Premium = false; + user.EmailVerified = true; + + sutProvider.GetDependency().SelfHosted = true; + sutProvider.GetDependency().GetByIdAsync(user.Id).Returns(user); + sutProvider.GetDependency().CanAccessPremium(user).Returns(true); + + // Act + var result = await sutProvider.Sut.StorageRemainingForSendAsync(send); + + // Assert - should NOT call pricing service for self-hosted + await sutProvider.GetDependency().DidNotReceive().GetAvailablePremiumPlan(); + } + + [Theory, BitAutoData] + public async Task StorageRemainingForSendAsync_OrgSend_DoesNotCallPricingService( + SutProvider sutProvider, + Send send, + Organization org) + { + // Arrange + send.UserId = null; + send.OrganizationId = org.Id; + send.Type = SendType.File; + org.MaxStorageGb = 100; + + sutProvider.GetDependency().GetByIdAsync(org.Id).Returns(org); + + // Act + var result = await sutProvider.Sut.StorageRemainingForSendAsync(send); + + // Assert - should NOT call pricing service for org sends + await sutProvider.GetDependency().DidNotReceive().GetAvailablePremiumPlan(); + } +} diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index 058c6f68ab..5fc92a9d39 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -6,6 +6,8 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; using Bit.Core.Billing.Enums; +using Bit.Core.Billing.Pricing; +using Bit.Core.Billing.Pricing.Premium; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -2228,10 +2230,6 @@ public class CipherServiceTests .PushSyncCiphersAsync(deletingUserId); } - - - - [Theory] [OrganizationCipherCustomize] [BitAutoData] @@ -2387,6 +2385,186 @@ public class CipherServiceTests ids.Count() == cipherIds.Length && ids.All(id => cipherIds.Contains(id)))); } + [Theory, BitAutoData] + public async Task CreateAttachmentAsync_UserWithOrgGrantedPremium_UsesStorageFromPricingClient( + SutProvider sutProvider, CipherDetails cipher, Guid savingUserId) + { + var stream = new MemoryStream(new byte[100]); + var fileName = "test.txt"; + var key = "test-key"; + + // Setup cipher with user ownership + cipher.UserId = savingUserId; + cipher.OrganizationId = null; + + // Setup user WITHOUT personal premium (Premium = false), but with org-granted premium access + var user = new User + { + Id = savingUserId, + Premium = false, // User does not have personal premium + MaxStorageGb = null, // No personal storage allocation + Storage = 0 // No storage used yet + }; + + sutProvider.GetDependency() + .GetByIdAsync(savingUserId) + .Returns(user); + + // User has premium access through their organization + sutProvider.GetDependency() + .CanAccessPremium(user) + .Returns(true); + + // Mock GlobalSettings to indicate cloud (not self-hosted) + sutProvider.GetDependency().SelfHosted = false; + + // Mock the PricingClient to return a premium plan with 1 GB of storage + var premiumPlan = new Plan + { + Name = "Premium", + Available = true, + Seat = new Purchasable { StripePriceId = "price_123", Price = 10, Provided = 1 }, + Storage = new Purchasable { StripePriceId = "price_456", Price = 4, Provided = 1 } + }; + sutProvider.GetDependency() + .GetAvailablePremiumPlan() + .Returns(premiumPlan); + + sutProvider.GetDependency() + .UploadNewAttachmentAsync(Arg.Any(), cipher, Arg.Any()) + .Returns(Task.CompletedTask); + + sutProvider.GetDependency() + .ValidateFileAsync(cipher, Arg.Any(), Arg.Any()) + .Returns((true, 100L)); + + sutProvider.GetDependency() + .UpdateAttachmentAsync(Arg.Any()) + .Returns(Task.CompletedTask); + + sutProvider.GetDependency() + .ReplaceAsync(Arg.Any()) + .Returns(Task.CompletedTask); + + // Act + await sutProvider.Sut.CreateAttachmentAsync(cipher, stream, fileName, key, 100, savingUserId, false, cipher.RevisionDate); + + // Assert - PricingClient was called to get the premium plan storage + await sutProvider.GetDependency().Received(1).GetAvailablePremiumPlan(); + + // Assert - Attachment was uploaded successfully + await sutProvider.GetDependency().Received(1) + .UploadNewAttachmentAsync(Arg.Any(), cipher, Arg.Any()); + } + + [Theory, BitAutoData] + public async Task CreateAttachmentAsync_UserWithOrgGrantedPremium_ExceedsStorage_ThrowsBadRequest( + SutProvider sutProvider, CipherDetails cipher, Guid savingUserId) + { + var stream = new MemoryStream(new byte[100]); + var fileName = "test.txt"; + var key = "test-key"; + + // Setup cipher with user ownership + cipher.UserId = savingUserId; + cipher.OrganizationId = null; + + // Setup user WITHOUT personal premium, with org-granted access, but storage is full + var user = new User + { + Id = savingUserId, + Premium = false, + MaxStorageGb = null, + Storage = 1073741824 // 1 GB already used (equals the provided storage) + }; + + sutProvider.GetDependency() + .GetByIdAsync(savingUserId) + .Returns(user); + + sutProvider.GetDependency() + .CanAccessPremium(user) + .Returns(true); + + sutProvider.GetDependency().SelfHosted = false; + + // Premium plan provides 1 GB of storage + var premiumPlan = new Plan + { + Name = "Premium", + Available = true, + Seat = new Purchasable { StripePriceId = "price_123", Price = 10, Provided = 1 }, + Storage = new Purchasable { StripePriceId = "price_456", Price = 4, Provided = 1 } + }; + sutProvider.GetDependency() + .GetAvailablePremiumPlan() + .Returns(premiumPlan); + + // Act & Assert - Should throw because storage is full + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.CreateAttachmentAsync(cipher, stream, fileName, key, 100, savingUserId, false, cipher.RevisionDate)); + Assert.Contains("Not enough storage available", exception.Message); + } + + [Theory, BitAutoData] + public async Task CreateAttachmentAsync_UserWithOrgGrantedPremium_SelfHosted_UsesConstantStorage( + SutProvider sutProvider, CipherDetails cipher, Guid savingUserId) + { + var stream = new MemoryStream(new byte[100]); + var fileName = "test.txt"; + var key = "test-key"; + + // Setup cipher with user ownership + cipher.UserId = savingUserId; + cipher.OrganizationId = null; + + // Setup user WITHOUT personal premium, but with org-granted premium access + var user = new User + { + Id = savingUserId, + Premium = false, + MaxStorageGb = null, + Storage = 0 + }; + + sutProvider.GetDependency() + .GetByIdAsync(savingUserId) + .Returns(user); + + sutProvider.GetDependency() + .CanAccessPremium(user) + .Returns(true); + + // Mock GlobalSettings to indicate self-hosted + sutProvider.GetDependency().SelfHosted = true; + + sutProvider.GetDependency() + .UploadNewAttachmentAsync(Arg.Any(), cipher, Arg.Any()) + .Returns(Task.CompletedTask); + + sutProvider.GetDependency() + .ValidateFileAsync(cipher, Arg.Any(), Arg.Any()) + .Returns((true, 100L)); + + sutProvider.GetDependency() + .UpdateAttachmentAsync(Arg.Any()) + .Returns(Task.CompletedTask); + + sutProvider.GetDependency() + .ReplaceAsync(Arg.Any()) + .Returns(Task.CompletedTask); + + // Act + await sutProvider.Sut.CreateAttachmentAsync(cipher, stream, fileName, key, 100, savingUserId, false, cipher.RevisionDate); + + // Assert - PricingClient should NOT be called for self-hosted + await sutProvider.GetDependency().DidNotReceive().GetAvailablePremiumPlan(); + + // Assert - Attachment was uploaded successfully + await sutProvider.GetDependency().Received(1) + .UploadNewAttachmentAsync(Arg.Any(), cipher, Arg.Any()); + } + private async Task AssertNoActionsAsync(SutProvider sutProvider) { await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyOrganizationDetailsByOrganizationIdAsync(default); From bab4750caa472449b82681bbc71f14be5db26cf9 Mon Sep 17 00:00:00 2001 From: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Date: Thu, 22 Jan 2026 11:23:18 -0600 Subject: [PATCH 10/51] chore: add feature flag definition, refs PM-26463 (#6882) --- src/Core/Constants.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 10c68ddc42..9ffe199f1d 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -143,6 +143,7 @@ public static class FeatureFlagKeys public const string BlockClaimedDomainAccountCreation = "pm-28297-block-uninvited-claimed-domain-registration"; public const string IncreaseBulkReinviteLimitForCloud = "pm-28251-increase-bulk-reinvite-limit-for-cloud"; public const string PremiumAccessQuery = "pm-29495-refactor-premium-interface"; + public const string RefactorMembersComponent = "pm-29503-refactor-members-inheritance"; /* Architecture */ public const string DesktopMigrationMilestone1 = "desktop-ui-migration-milestone-1"; From 415821f173dcd2756e471a36d36f1b615b948142 Mon Sep 17 00:00:00 2001 From: Derek Nance Date: Thu, 22 Jan 2026 15:20:38 -0600 Subject: [PATCH 11/51] [PM-29142] Config for SSO cookie vending (#6880) This config may be used when a load balancer in front of Bitwarden is first verifying an auth cookie issued by an IdP before proxying the request to Bitwarden. --- dev/secrets.json.example | 10 +++++++++- src/Core/Settings/GlobalSettings.cs | 15 ++++++++++++++- src/Core/Settings/ICommunicationSettings.cs | 7 +++++++ src/Core/Settings/IGlobalSettings.cs | 1 + src/Core/Settings/ISsoCookieVendorSettings.cs | 8 ++++++++ 5 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 src/Core/Settings/ICommunicationSettings.cs create mode 100644 src/Core/Settings/ISsoCookieVendorSettings.cs diff --git a/dev/secrets.json.example b/dev/secrets.json.example index 0d4213aec1..7bf753e938 100644 --- a/dev/secrets.json.example +++ b/dev/secrets.json.example @@ -39,6 +39,14 @@ }, "licenseDirectory": "", "enableNewDeviceVerification": true, - "enableEmailVerification": true + "enableEmailVerification": true, + "communication": { + "bootstrap": "none", + "ssoCookieVendor": { + "idpLoginUrl": "", + "cookieName": "", + "cookieDomain": "" + } + } } } diff --git a/src/Core/Settings/GlobalSettings.cs b/src/Core/Settings/GlobalSettings.cs index 1f4fa6104b..6ccbd1ee85 100644 --- a/src/Core/Settings/GlobalSettings.cs +++ b/src/Core/Settings/GlobalSettings.cs @@ -83,7 +83,6 @@ public class GlobalSettings : IGlobalSettings public virtual ILaunchDarklySettings LaunchDarkly { get; set; } = new LaunchDarklySettings(); public virtual string DevelopmentDirectory { get; set; } public virtual IWebPushSettings WebPush { get; set; } = new WebPushSettings(); - public virtual int SendAccessTokenLifetimeInMinutes { get; set; } = 5; public virtual bool EnableEmailVerification { get; set; } public virtual string KdfDefaultHashKey { get; set; } @@ -93,6 +92,7 @@ public class GlobalSettings : IGlobalSettings public virtual string SendDefaultHashKey { get; set; } public virtual string PricingUri { get; set; } public virtual Fido2Settings Fido2 { get; set; } = new Fido2Settings(); + public virtual ICommunicationSettings Communication { get; set; } = new CommunicationSettings(); public string BuildExternalUri(string explicitValue, string name) { @@ -776,4 +776,17 @@ public class GlobalSettings : IGlobalSettings { public HashSet Origins { get; set; } } + + public class CommunicationSettings : ICommunicationSettings + { + public string Bootstrap { get; set; } = "none"; + public ISsoCookieVendorSettings SsoCookieVendor { get; set; } = new SsoCookieVendorSettings(); + } + + public class SsoCookieVendorSettings : ISsoCookieVendorSettings + { + public string IdpLoginUrl { get; set; } + public string CookieName { get; set; } + public string CookieDomain { get; set; } + } } diff --git a/src/Core/Settings/ICommunicationSettings.cs b/src/Core/Settings/ICommunicationSettings.cs new file mode 100644 index 0000000000..26259a8448 --- /dev/null +++ b/src/Core/Settings/ICommunicationSettings.cs @@ -0,0 +1,7 @@ +namespace Bit.Core.Settings; + +public interface ICommunicationSettings +{ + string Bootstrap { get; set; } + ISsoCookieVendorSettings SsoCookieVendor { get; set; } +} diff --git a/src/Core/Settings/IGlobalSettings.cs b/src/Core/Settings/IGlobalSettings.cs index c316836d09..7f5323fac0 100644 --- a/src/Core/Settings/IGlobalSettings.cs +++ b/src/Core/Settings/IGlobalSettings.cs @@ -29,4 +29,5 @@ public interface IGlobalSettings IWebPushSettings WebPush { get; set; } GlobalSettings.EventLoggingSettings EventLogging { get; set; } GlobalSettings.WebAuthnSettings WebAuthn { get; set; } + ICommunicationSettings Communication { get; set; } } diff --git a/src/Core/Settings/ISsoCookieVendorSettings.cs b/src/Core/Settings/ISsoCookieVendorSettings.cs new file mode 100644 index 0000000000..a9f2169b13 --- /dev/null +++ b/src/Core/Settings/ISsoCookieVendorSettings.cs @@ -0,0 +1,8 @@ +namespace Bit.Core.Settings; + +public interface ISsoCookieVendorSettings +{ + string IdpLoginUrl { get; set; } + string CookieName { get; set; } + string CookieDomain { get; set; } +} From 0cc72127d7fe50f9033931658a112762345141fb Mon Sep 17 00:00:00 2001 From: Mike Amirault Date: Thu, 22 Jan 2026 20:11:56 -0500 Subject: [PATCH 12/51] [PM-26405] Fix cipher favorite info being saved incorrectly on import (#6776) --- .../ImportFeatures/ImportCiphersCommand.cs | 2 +- .../ImportCiphersAsyncCommandTests.cs | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs b/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs index fa558f5963..9300e3c4bb 100644 --- a/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs +++ b/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs @@ -74,7 +74,7 @@ public class ImportCiphersCommand : IImportCiphersCommand if (cipher.UserId.HasValue && cipher.Favorite) { - cipher.Favorites = $"{{\"{cipher.UserId.ToString().ToUpperInvariant()}\":\"true\"}}"; + cipher.Favorites = $"{{\"{cipher.UserId.ToString().ToUpperInvariant()}\":true}}"; } } diff --git a/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs b/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs index b92477e73d..aea06f39a8 100644 --- a/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs +++ b/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs @@ -135,6 +135,43 @@ public class ImportCiphersAsyncCommandTests Assert.Equal("You cannot import items into your personal vault because you are a member of an organization which forbids it.", exception.Message); } + [Theory, BitAutoData] + public async Task ImportIntoIndividualVaultAsync_FavoriteCiphers_PersistsFavoriteInfo( + Guid importingUserId, + List ciphers, + SutProvider sutProvider + ) + { + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + sutProvider.GetDependency() + .GetAsync(importingUserId) + .Returns(new OrganizationDataOwnershipPolicyRequirement( + OrganizationDataOwnershipState.Disabled, + [])); + + sutProvider.GetDependency() + .GetManyByUserIdAsync(importingUserId) + .Returns(new List()); + + var folders = new List(); + var folderRelationships = new List>(); + + ciphers.ForEach(c => + { + c.UserId = importingUserId; + c.Favorite = true; + }); + + await sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships, importingUserId); + + await sutProvider.GetDependency() + .Received(1) + .CreateAsync(importingUserId, Arg.Is>(ciphers => ciphers.All(c => c.Favorites == $"{{\"{importingUserId.ToString().ToUpperInvariant()}\":true}}")), Arg.Any>()); + } + [Theory, BitAutoData] public async Task ImportIntoOrganizationalVaultAsync_Success( Organization organization, From 93e2c971df7e58920f87ee40d468e5a6abcb38dd Mon Sep 17 00:00:00 2001 From: Dave <3836813+enmande@users.noreply.github.com> Date: Thu, 22 Jan 2026 20:24:15 -0500 Subject: [PATCH 13/51] feat(emergency-access) [PM-29584] Create Email for Emergency Access Removal (#6793) * feat(emergency-access) [PM-29584]: Add email template. * refactor(emergency-access) [PM-29584]: Move Emergency Access to Auth/UserFeatures. * refactor(emergency-access) [PM-29584]: Move EmergencyAccess tests to UserFeatures space. * feat(emergency-access) [PM-29584]: Add compiled EmergencyAccess templates. * test(emergency-access) [PM-29584]: Add mailer-specific tests. * refactor(emergency-access) [PM-29584]: Move mail to UserFeatures area. * feat(emergency-access) [PM-29584]: Update link for help pages, not web vault. * test(emergency-access) [PM-29584]: Update mail tests for new URL and single responsibility. * refactor(emergency-access) [PM-29584]: Add comments for added test. --- .../Controllers/EmergencyAccessController.cs | 2 +- .../Jobs/EmergencyAccessNotificationJob.cs | 2 +- .../Auth/Jobs/EmergencyAccessTimeoutJob.cs | 2 +- .../EmergencyAccess/EmergencyAccessService.cs | 19 +- .../IEmergencyAccessService.cs | 13 +- .../EmergencyAccessRemoveGranteesMailView.cs | 14 + ...gencyAccessRemoveGranteesMailView.html.hbs | 499 ++++++++++++++++++ ...gencyAccessRemoveGranteesMailView.text.hbs | 7 + .../EmergencyAccess/readme.md | 0 .../emergency-access-remove-grantees.mjml | 31 ++ .../Utilities/ServiceCollectionExtensions.cs | 1 + .../EmergencyAccessMailTests.cs | 153 ++++++ .../EmergencyAccessServiceTests.cs | 151 +++--- .../Registration/RegisterUserCommandTests.cs | 8 +- 14 files changed, 802 insertions(+), 100 deletions(-) rename src/Core/Auth/{Services => UserFeatures}/EmergencyAccess/EmergencyAccessService.cs (95%) rename src/Core/Auth/{Services => UserFeatures}/EmergencyAccess/IEmergencyAccessService.cs (93%) create mode 100644 src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.cs create mode 100644 src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.html.hbs create mode 100644 src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.text.hbs rename src/Core/Auth/{Services => UserFeatures}/EmergencyAccess/readme.md (100%) create mode 100644 src/Core/MailTemplates/Mjml/emails/Auth/UserFeatures/EmergencyAccess/emergency-access-remove-grantees.mjml create mode 100644 test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessMailTests.cs rename test/Core.Test/Auth/{Services => UserFeatures/EmergencyAccess}/EmergencyAccessServiceTests.cs (92%) diff --git a/src/Api/Auth/Controllers/EmergencyAccessController.cs b/src/Api/Auth/Controllers/EmergencyAccessController.cs index 016cd82fe2..bd87e82c8a 100644 --- a/src/Api/Auth/Controllers/EmergencyAccessController.cs +++ b/src/Api/Auth/Controllers/EmergencyAccessController.cs @@ -7,7 +7,7 @@ using Bit.Api.Auth.Models.Request; using Bit.Api.Auth.Models.Response; using Bit.Api.Models.Response; using Bit.Api.Vault.Models.Response; -using Bit.Core.Auth.Services; +using Bit.Core.Auth.UserFeatures.EmergencyAccess; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; diff --git a/src/Api/Auth/Jobs/EmergencyAccessNotificationJob.cs b/src/Api/Auth/Jobs/EmergencyAccessNotificationJob.cs index c67cb9db3f..f58eaafaab 100644 --- a/src/Api/Auth/Jobs/EmergencyAccessNotificationJob.cs +++ b/src/Api/Auth/Jobs/EmergencyAccessNotificationJob.cs @@ -1,7 +1,7 @@ // FIXME: Update this file to be null safe and then delete the line below #nullable disable -using Bit.Core.Auth.Services; +using Bit.Core.Auth.UserFeatures.EmergencyAccess; using Bit.Core.Jobs; using Quartz; diff --git a/src/Api/Auth/Jobs/EmergencyAccessTimeoutJob.cs b/src/Api/Auth/Jobs/EmergencyAccessTimeoutJob.cs index f23774f060..63b861d920 100644 --- a/src/Api/Auth/Jobs/EmergencyAccessTimeoutJob.cs +++ b/src/Api/Auth/Jobs/EmergencyAccessTimeoutJob.cs @@ -1,7 +1,7 @@ // FIXME: Update this file to be null safe and then delete the line below #nullable disable -using Bit.Core.Auth.Services; +using Bit.Core.Auth.UserFeatures.EmergencyAccess; using Bit.Core.Jobs; using Quartz; diff --git a/src/Core/Auth/Services/EmergencyAccess/EmergencyAccessService.cs b/src/Core/Auth/UserFeatures/EmergencyAccess/EmergencyAccessService.cs similarity index 95% rename from src/Core/Auth/Services/EmergencyAccess/EmergencyAccessService.cs rename to src/Core/Auth/UserFeatures/EmergencyAccess/EmergencyAccessService.cs index 0072f85e61..6552f4bc69 100644 --- a/src/Core/Auth/Services/EmergencyAccess/EmergencyAccessService.cs +++ b/src/Core/Auth/UserFeatures/EmergencyAccess/EmergencyAccessService.cs @@ -4,7 +4,6 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Models.Data; @@ -19,7 +18,7 @@ using Bit.Core.Vault.Models.Data; using Bit.Core.Vault.Repositories; using Bit.Core.Vault.Services; -namespace Bit.Core.Auth.Services; +namespace Bit.Core.Auth.UserFeatures.EmergencyAccess; public class EmergencyAccessService : IEmergencyAccessService { @@ -61,7 +60,7 @@ public class EmergencyAccessService : IEmergencyAccessService _removeOrganizationUserCommand = removeOrganizationUserCommand; } - public async Task InviteAsync(User grantorUser, string emergencyContactEmail, EmergencyAccessType accessType, int waitTime) + public async Task InviteAsync(User grantorUser, string emergencyContactEmail, EmergencyAccessType accessType, int waitTime) { if (!await _userService.CanAccessPremium(grantorUser)) { @@ -73,7 +72,7 @@ public class EmergencyAccessService : IEmergencyAccessService throw new BadRequestException("You cannot use Emergency Access Takeover because you are using Key Connector."); } - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Entities.EmergencyAccess { GrantorId = grantorUser.Id, Email = emergencyContactEmail.ToLowerInvariant(), @@ -113,7 +112,7 @@ public class EmergencyAccessService : IEmergencyAccessService await SendInviteAsync(emergencyAccess, NameOrEmail(grantorUser)); } - public async Task AcceptUserAsync(Guid emergencyAccessId, User granteeUser, string token, IUserService userService) + public async Task AcceptUserAsync(Guid emergencyAccessId, User granteeUser, string token, IUserService userService) { var emergencyAccess = await _emergencyAccessRepository.GetByIdAsync(emergencyAccessId); if (emergencyAccess == null) @@ -175,7 +174,7 @@ public class EmergencyAccessService : IEmergencyAccessService await _emergencyAccessRepository.DeleteAsync(emergencyAccess); } - public async Task ConfirmUserAsync(Guid emergencyAccessId, string key, Guid grantorId) + public async Task ConfirmUserAsync(Guid emergencyAccessId, string key, Guid grantorId) { var emergencyAccess = await _emergencyAccessRepository.GetByIdAsync(emergencyAccessId); if (emergencyAccess == null || emergencyAccess.Status != EmergencyAccessStatusType.Accepted || @@ -201,7 +200,7 @@ public class EmergencyAccessService : IEmergencyAccessService return emergencyAccess; } - public async Task SaveAsync(EmergencyAccess emergencyAccess, User grantorUser) + public async Task SaveAsync(Entities.EmergencyAccess emergencyAccess, User grantorUser) { if (!await _userService.CanAccessPremium(grantorUser)) { @@ -311,7 +310,7 @@ public class EmergencyAccessService : IEmergencyAccessService } // TODO PM-21687: rename this to something like InitiateRecoveryTakeoverAsync - public async Task<(EmergencyAccess, User)> TakeoverAsync(Guid emergencyAccessId, User granteeUser) + public async Task<(Entities.EmergencyAccess, User)> TakeoverAsync(Guid emergencyAccessId, User granteeUser) { var emergencyAccess = await _emergencyAccessRepository.GetByIdAsync(emergencyAccessId); @@ -429,7 +428,7 @@ public class EmergencyAccessService : IEmergencyAccessService return await _cipherService.GetAttachmentDownloadDataAsync(cipher, attachmentId); } - private async Task SendInviteAsync(EmergencyAccess emergencyAccess, string invitingUsersName) + private async Task SendInviteAsync(Entities.EmergencyAccess emergencyAccess, string invitingUsersName) { var token = _dataProtectorTokenizer.Protect(new EmergencyAccessInviteTokenable(emergencyAccess, _globalSettings.OrganizationInviteExpirationHours)); await _mailService.SendEmergencyAccessInviteEmailAsync(emergencyAccess, invitingUsersName, token); @@ -449,7 +448,7 @@ public class EmergencyAccessService : IEmergencyAccessService */ //TODO PM-21687: this IsValidRequest() checks the validity based on the granteeUser. There should be a complementary method for the grantorUser private static bool IsValidRequest( - EmergencyAccess availableAccess, + Entities.EmergencyAccess availableAccess, User requestingUser, EmergencyAccessType requestedAccessType) { diff --git a/src/Core/Auth/Services/EmergencyAccess/IEmergencyAccessService.cs b/src/Core/Auth/UserFeatures/EmergencyAccess/IEmergencyAccessService.cs similarity index 93% rename from src/Core/Auth/Services/EmergencyAccess/IEmergencyAccessService.cs rename to src/Core/Auth/UserFeatures/EmergencyAccess/IEmergencyAccessService.cs index de695bbd7d..860ae8bfb6 100644 --- a/src/Core/Auth/Services/EmergencyAccess/IEmergencyAccessService.cs +++ b/src/Core/Auth/UserFeatures/EmergencyAccess/IEmergencyAccessService.cs @@ -1,5 +1,4 @@ using Bit.Core.AdminConsole.Entities; -using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; using Bit.Core.Entities; @@ -7,7 +6,7 @@ using Bit.Core.Enums; using Bit.Core.Services; using Bit.Core.Vault.Models.Data; -namespace Bit.Core.Auth.Services; +namespace Bit.Core.Auth.UserFeatures.EmergencyAccess; public interface IEmergencyAccessService { @@ -20,7 +19,7 @@ public interface IEmergencyAccessService /// Type of emergency access allowed to the emergency contact /// The amount of time to pass before the invite is auto confirmed /// a new Emergency Access object - Task InviteAsync(User grantorUser, string emergencyContactEmail, EmergencyAccessType accessType, int waitTime); + Task InviteAsync(User grantorUser, string emergencyContactEmail, EmergencyAccessType accessType, int waitTime); /// /// Sends an invite to the emergency contact associated with the emergency access id. /// @@ -37,7 +36,7 @@ public interface IEmergencyAccessService /// the tokenable that was sent via email /// service dependency /// void - Task AcceptUserAsync(Guid emergencyAccessId, User granteeUser, string token, IUserService userService); + Task AcceptUserAsync(Guid emergencyAccessId, User granteeUser, string token, IUserService userService); /// /// The creator of the emergency access request can delete the request. /// @@ -53,7 +52,7 @@ public interface IEmergencyAccessService /// The grantor user key encrypted by the grantee public key; grantee.PubicKey(grantor.User.Key) /// Id of grantor user /// emergency access object associated with the Id passed in - Task ConfirmUserAsync(Guid emergencyAccessId, string key, Guid grantorId); + Task ConfirmUserAsync(Guid emergencyAccessId, string key, Guid grantorId); /// /// Fetches an emergency access object. The grantor user must own the object being fetched. /// @@ -67,7 +66,7 @@ public interface IEmergencyAccessService /// emergency access entity being updated /// grantor user /// void - Task SaveAsync(EmergencyAccess emergencyAccess, User grantorUser); + Task SaveAsync(Entities.EmergencyAccess emergencyAccess, User grantorUser); /// /// Initiates the recovery process. For either Takeover or view. Will send an email to the Grantor User notifying of the initiation. /// @@ -107,7 +106,7 @@ public interface IEmergencyAccessService /// Id of entity being accessed /// grantee user of the emergency access entity /// emergency access entity and the grantorUser - Task<(EmergencyAccess, User)> TakeoverAsync(Guid emergencyAccessId, User granteeUser); + Task<(Entities.EmergencyAccess, User)> TakeoverAsync(Guid emergencyAccessId, User granteeUser); /// /// Updates the grantor's password hash and updates the key for the EmergencyAccess entity. /// diff --git a/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.cs b/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.cs new file mode 100644 index 0000000000..4d60556785 --- /dev/null +++ b/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.cs @@ -0,0 +1,14 @@ +using Bit.Core.Platform.Mail.Mailer; + +namespace Bit.Core.Auth.UserFeatures.EmergencyAccess.Mail; + +public class EmergencyAccessRemoveGranteesMailView : BaseMailView +{ + public required IEnumerable RemovedGranteeNames { get; set; } + public string EmergencyAccessHelpPageUrl => "https://bitwarden.com/help/emergency-access/"; +} + +public class EmergencyAccessRemoveGranteesMail : BaseMail +{ + public override string Subject { get; set; } = "Emergency contacts removed"; +} diff --git a/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.html.hbs b/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.html.hbs new file mode 100644 index 0000000000..405f2744bd --- /dev/null +++ b/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.html.hbs @@ -0,0 +1,499 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + +
+ +
+ - +
-

+

© 2025 Bitwarden Inc. 1 N. Calle Cesar Chavez, Suite 102, Santa Barbara, CA, USA

Always confirm you are on a trusted Bitwarden domain before logging in:
- bitwarden.com | - Learn why we include this + bitwarden.com | + Learn why we include this

-
As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. - This year's renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax.
+
As a long time Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this year's renewal. + This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax.
-
As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this renewal. - This year's renewal now will be {{DiscountedMonthlyRenewalPrice}}/month, billed annually.
+
As an existing Bitwarden customer, you will receive a one-time {{DiscountAmount}} loyalty discount for this year's renewal. + This renewal will now be billed annually at {{DiscountedAnnualRenewalPrice}} + tax.
+ + + + + +
+ + + + + + + + +
+ + + + + +
+ + + + + + + +
+ + +
+ + + + + + + + + + + + + +
+ + + + + + + +
+ + + +
+ +
+ +

+ +

+ +
+ +
+ + + +
+ + + + + + + + + +
+ + + + + + + +
+ + + +
+ +
+ +
+ + +
+ +
+ + + + + +
+ + +
+ + + + + + + + + + + +
+ + + + + + + +
+ + + +
+ + + + + + + +
+ + +
+ + + + + + + + + +
+ +
The following emergency contacts have been removed from your account: +
    + {{#each RemovedGranteeNames}} +
  • {{this}}
    • + {{/each}} +
+ Learn more about emergency access.
+ +
+ +
+ + +
+ +
+ + + +
+ +
+ + + + + + + + + +
+ + + + + + + +
+ + +
+ + + + + + + + + + + + + +
+ + + + + + + + + + + + +
+ + + + + + +
+ + + +
+
+ + + + + + + + + + +
+ + + + + + +
+ + + +
+
+ + + + + + + + + + +
+ + + + + + +
+ + + +
+
+ + + + + + + + + + +
+ + + + + + +
+ + + +
+
+ + + + + + + + + + +
+ + + + + + +
+ + + +
+
+ + + + + + + + + + +
+ + + + + + +
+ + + +
+
+ + + + + + + + + + +
+ + + + + + +
+ + + +
+
+ + + +
+ +

+ © 2025 Bitwarden Inc. 1 N. Calle Cesar Chavez, Suite 102, Santa + Barbara, CA, USA +

+

+ Always confirm you are on a trusted Bitwarden domain before logging + in:
+ bitwarden.com | + Learn why we include this +

+ +
+ +
+ + +
+ +
+ + + + + + + + + + \ No newline at end of file diff --git a/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.text.hbs b/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.text.hbs new file mode 100644 index 0000000000..3c17274f35 --- /dev/null +++ b/src/Core/Auth/UserFeatures/EmergencyAccess/Mail/EmergencyAccessRemoveGranteesMailView.text.hbs @@ -0,0 +1,7 @@ +The following emergency contacts have been removed from your account: + +{{#each RemovedGranteeNames}} + {{this}} +{{/each}} + +Learn more about emergency access at {{EmergencyAccessHelpPageUrl}} diff --git a/src/Core/Auth/Services/EmergencyAccess/readme.md b/src/Core/Auth/UserFeatures/EmergencyAccess/readme.md similarity index 100% rename from src/Core/Auth/Services/EmergencyAccess/readme.md rename to src/Core/Auth/UserFeatures/EmergencyAccess/readme.md diff --git a/src/Core/MailTemplates/Mjml/emails/Auth/UserFeatures/EmergencyAccess/emergency-access-remove-grantees.mjml b/src/Core/MailTemplates/Mjml/emails/Auth/UserFeatures/EmergencyAccess/emergency-access-remove-grantees.mjml new file mode 100644 index 0000000000..3af29a4414 --- /dev/null +++ b/src/Core/MailTemplates/Mjml/emails/Auth/UserFeatures/EmergencyAccess/emergency-access-remove-grantees.mjml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + The following emergency contacts have been removed from your account: +
    + {{#each RemovedGranteeNames}} +
  • {{this}}
    • + {{/each}} +
+ Learn more about emergency access. + + + + + + + + + diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 1bb9cb6c7a..5234a257cf 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -22,6 +22,7 @@ using Bit.Core.Auth.Repositories; using Bit.Core.Auth.Services; using Bit.Core.Auth.Services.Implementations; using Bit.Core.Auth.UserFeatures; +using Bit.Core.Auth.UserFeatures.EmergencyAccess; using Bit.Core.Auth.UserFeatures.PasswordValidation; using Bit.Core.Billing.Services; using Bit.Core.Billing.Services.Implementations; diff --git a/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessMailTests.cs b/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessMailTests.cs new file mode 100644 index 0000000000..8cb6c2c2fe --- /dev/null +++ b/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessMailTests.cs @@ -0,0 +1,153 @@ +using Bit.Core.Auth.UserFeatures.EmergencyAccess.Mail; +using Bit.Core.Models.Mail; +using Bit.Core.Platform.Mail.Delivery; +using Bit.Core.Platform.Mail.Mailer; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; +using GlobalSettings = Bit.Core.Settings.GlobalSettings; + +namespace Bit.Core.Test.Auth.UserFeatures.EmergencyAccess; + +[SutProviderCustomize] +public class EmergencyAccessMailTests +{ + // Constant values for all Emergency Access emails + private const string _emergencyAccessHelpUrl = "https://bitwarden.com/help/emergency-access/"; + private const string _emergencyAccessMailSubject = "Emergency contacts removed"; + + /// + /// Documents how to construct and send the emergency access removal email. + /// 1. Inject IMailer into their command/service + /// 2. Construct EmergencyAccessRemoveGranteesMail as shown below + /// 3. Call mailer.SendEmail(mail) + /// + [Theory, BitAutoData] + public async Task SendEmergencyAccessRemoveGranteesEmail_SingleGrantee_Success( + string grantorEmail, + string granteeName) + { + // Arrange + var logger = Substitute.For>(); + var globalSettings = new GlobalSettings { SelfHosted = false }; + var deliveryService = Substitute.For(); + var mailer = new Mailer( + new HandlebarMailRenderer(logger, globalSettings), + deliveryService); + + var mail = new EmergencyAccessRemoveGranteesMail + { + ToEmails = [grantorEmail], + View = new EmergencyAccessRemoveGranteesMailView + { + RemovedGranteeNames = [granteeName] + } + }; + + MailMessage sentMessage = null; + await deliveryService.SendEmailAsync(Arg.Do(message => + sentMessage = message + )); + + // Act + await mailer.SendEmail(mail); + + // Assert + Assert.NotNull(sentMessage); + Assert.Contains(grantorEmail, sentMessage.ToEmails); + + // Verify the content contains the grantee name + Assert.Contains(granteeName, sentMessage.TextContent); + Assert.Contains(granteeName, sentMessage.HtmlContent); + } + + /// + /// Documents handling multiple removed grantees in a single email. + /// + [Theory, BitAutoData] + public async Task SendEmergencyAccessRemoveGranteesEmail_MultipleGrantees_RendersAllNames( + string grantorEmail) + { + // Arrange + var logger = Substitute.For>(); + var globalSettings = new GlobalSettings { SelfHosted = false }; + var deliveryService = Substitute.For(); + var mailer = new Mailer( + new HandlebarMailRenderer(logger, globalSettings), + deliveryService); + + var granteeNames = new[] { "Alice", "Bob", "Carol" }; + + var mail = new EmergencyAccessRemoveGranteesMail + { + ToEmails = [grantorEmail], + View = new EmergencyAccessRemoveGranteesMailView + { + RemovedGranteeNames = granteeNames + } + }; + + MailMessage sentMessage = null; + await deliveryService.SendEmailAsync(Arg.Do(message => + sentMessage = message + )); + + // Act + await mailer.SendEmail(mail); + + // Assert - All grantee names should appear in the email + Assert.NotNull(sentMessage); + foreach (var granteeName in granteeNames) + { + Assert.Contains(granteeName, sentMessage.TextContent); + Assert.Contains(granteeName, sentMessage.HtmlContent); + } + } + + /// + /// Validates the required GranteeNames for the email view model. + /// + [Theory, BitAutoData] + public void EmergencyAccessRemoveGranteesMailView_GranteeNames_AreRequired( + string grantorEmail) + { + // Arrange - Shows the minimum required to construct the email + var mail = new EmergencyAccessRemoveGranteesMail + { + ToEmails = [grantorEmail], // Required: who to send to + View = new EmergencyAccessRemoveGranteesMailView + { + // Required: at least one removed grantee name + RemovedGranteeNames = ["Example Grantee"] + } + }; + + // Assert + Assert.NotNull(mail); + Assert.NotNull(mail.View); + Assert.NotEmpty(mail.View.RemovedGranteeNames); + } + + /// + /// Ensure consistency with help pages link and email subject. + /// + /// + /// + [Theory, BitAutoData] + public void EmergencyAccessRemoveGranteesMailView_SubjectAndHelpLink_MatchesExpectedValues(string grantorEmail, string granteeName) + { + // Arrange + var mail = new EmergencyAccessRemoveGranteesMail + { + ToEmails = [grantorEmail], + View = new EmergencyAccessRemoveGranteesMailView { RemovedGranteeNames = [granteeName] } + }; + + // Assert + Assert.NotNull(mail); + Assert.NotNull(mail.View); + Assert.Equal(_emergencyAccessMailSubject, mail.Subject); + Assert.Equal(_emergencyAccessHelpUrl, mail.View.EmergencyAccessHelpPageUrl); + } +} diff --git a/test/Core.Test/Auth/Services/EmergencyAccessServiceTests.cs b/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessServiceTests.cs similarity index 92% rename from test/Core.Test/Auth/Services/EmergencyAccessServiceTests.cs rename to test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessServiceTests.cs index 006515aafd..83585e6667 100644 --- a/test/Core.Test/Auth/Services/EmergencyAccessServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessServiceTests.cs @@ -1,11 +1,10 @@ using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Models.Data; -using Bit.Core.Auth.Services; +using Bit.Core.Auth.UserFeatures.EmergencyAccess; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -17,7 +16,7 @@ using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; -namespace Bit.Core.Test.Auth.Services; +namespace Bit.Core.Test.Auth.UserFeatures.EmergencyAccess; [SutProviderCustomize] public class EmergencyAccessServiceTests @@ -68,13 +67,13 @@ public class EmergencyAccessServiceTests Assert.Equal(EmergencyAccessStatusType.Invited, result.Status); await sutProvider.GetDependency() .Received(1) - .CreateAsync(Arg.Any()); + .CreateAsync(Arg.Any()); sutProvider.GetDependency>() .Received(1) .Protect(Arg.Any()); await sutProvider.GetDependency() .Received(1) - .SendEmergencyAccessInviteEmailAsync(Arg.Any(), Arg.Any(), Arg.Any()); + .SendEmergencyAccessInviteEmailAsync(Arg.Any(), Arg.Any(), Arg.Any()); } [Theory, BitAutoData] @@ -98,7 +97,7 @@ public class EmergencyAccessServiceTests User invitingUser, Guid emergencyAccessId) { - EmergencyAccess emergencyAccess = null; + Core.Auth.Entities.EmergencyAccess emergencyAccess = null; sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) @@ -119,7 +118,7 @@ public class EmergencyAccessServiceTests User invitingUser, Guid emergencyAccessId) { - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Status = EmergencyAccessStatusType.Invited, GrantorId = Guid.NewGuid(), @@ -148,7 +147,7 @@ public class EmergencyAccessServiceTests User invitingUser, Guid emergencyAccessId) { - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Status = statusType, GrantorId = invitingUser.Id, @@ -172,7 +171,7 @@ public class EmergencyAccessServiceTests User invitingUser, Guid emergencyAccessId) { - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Status = EmergencyAccessStatusType.Invited, GrantorId = invitingUser.Id, @@ -194,7 +193,7 @@ public class EmergencyAccessServiceTests public async Task AcceptUserAsync_EmergencyAccessNull_ThrowsBadRequest( SutProvider sutProvider, User acceptingUser, string token) { - EmergencyAccess emergencyAccess = null; + Core.Auth.Entities.EmergencyAccess emergencyAccess = null; sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) .Returns(emergencyAccess); @@ -209,7 +208,7 @@ public class EmergencyAccessServiceTests public async Task AcceptUserAsync_CannotUnprotectToken_ThrowsBadRequest( SutProvider sutProvider, User acceptingUser, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string token) { sutProvider.GetDependency() @@ -230,8 +229,8 @@ public class EmergencyAccessServiceTests public async Task AcceptUserAsync_TokenDataInvalid_ThrowsBadRequest( SutProvider sutProvider, User acceptingUser, - EmergencyAccess emergencyAccess, - EmergencyAccess wrongEmergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess wrongEmergencyAccess, string token) { sutProvider.GetDependency() @@ -257,7 +256,7 @@ public class EmergencyAccessServiceTests public async Task AcceptUserAsync_AcceptedStatus_ThrowsBadRequest( SutProvider sutProvider, User acceptingUser, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string token) { emergencyAccess.Status = EmergencyAccessStatusType.Accepted; @@ -284,7 +283,7 @@ public class EmergencyAccessServiceTests public async Task AcceptUserAsync_NotInvitedStatus_ThrowsBadRequest( SutProvider sutProvider, User acceptingUser, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string token) { emergencyAccess.Status = EmergencyAccessStatusType.Confirmed; @@ -311,7 +310,7 @@ public class EmergencyAccessServiceTests public async Task AcceptUserAsync_EmergencyAccessEmailDoesNotMatch_ThrowsBadRequest( SutProvider sutProvider, User acceptingUser, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string token) { emergencyAccess.Status = EmergencyAccessStatusType.Invited; @@ -339,7 +338,7 @@ public class EmergencyAccessServiceTests SutProvider sutProvider, User acceptingUser, User invitingUser, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string token) { emergencyAccess.Status = EmergencyAccessStatusType.Invited; @@ -364,7 +363,7 @@ public class EmergencyAccessServiceTests await sutProvider.GetDependency() .Received(1) - .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.Accepted)); + .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.Accepted)); await sutProvider.GetDependency() .Received(1) @@ -375,11 +374,11 @@ public class EmergencyAccessServiceTests public async Task DeleteAsync_EmergencyAccessNull_ThrowsBadRequest( SutProvider sutProvider, User invitingUser, - EmergencyAccess emergencyAccess) + Core.Auth.Entities.EmergencyAccess emergencyAccess) { sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) - .Returns((EmergencyAccess)null); + .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.DeleteAsync(emergencyAccess.Id, invitingUser.Id)); @@ -391,7 +390,7 @@ public class EmergencyAccessServiceTests public async Task DeleteAsync_EmergencyAccessGrantorIdNotEqual_ThrowsBadRequest( SutProvider sutProvider, User invitingUser, - EmergencyAccess emergencyAccess) + Core.Auth.Entities.EmergencyAccess emergencyAccess) { emergencyAccess.GrantorId = Guid.NewGuid(); sutProvider.GetDependency() @@ -408,7 +407,7 @@ public class EmergencyAccessServiceTests public async Task DeleteAsync_EmergencyAccessGranteeIdNotEqual_ThrowsBadRequest( SutProvider sutProvider, User invitingUser, - EmergencyAccess emergencyAccess) + Core.Auth.Entities.EmergencyAccess emergencyAccess) { emergencyAccess.GranteeId = Guid.NewGuid(); sutProvider.GetDependency() @@ -425,7 +424,7 @@ public class EmergencyAccessServiceTests public async Task DeleteAsync_EmergencyAccessIsDeleted_Success( SutProvider sutProvider, User user, - EmergencyAccess emergencyAccess) + Core.Auth.Entities.EmergencyAccess emergencyAccess) { emergencyAccess.GranteeId = user.Id; emergencyAccess.GrantorId = user.Id; @@ -443,7 +442,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task ConfirmUserAsync_EmergencyAccessNull_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string key, User grantorUser) { @@ -451,7 +450,7 @@ public class EmergencyAccessServiceTests emergencyAccess.Status = EmergencyAccessStatusType.RecoveryInitiated; sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) - .Returns((EmergencyAccess)null); + .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.ConfirmUserAsync(emergencyAccess.Id, key, grantorUser.Id)); @@ -463,7 +462,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task ConfirmUserAsync_EmergencyAccessStatusIsNotAccepted_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string key, User grantorUser) { @@ -484,7 +483,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task ConfirmUserAsync_EmergencyAccessGrantorIdNotEqualToConfirmingUserId_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string key, User grantorUser) { @@ -505,7 +504,7 @@ public class EmergencyAccessServiceTests SutProvider sutProvider, User confirmingUser, string key) { confirmingUser.UsesKeyConnector = true; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Status = EmergencyAccessStatusType.Accepted, GrantorId = confirmingUser.Id, @@ -530,7 +529,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task ConfirmUserAsync_ConfirmsAndReplacesEmergencyAccess_Success( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, string key, User grantorUser, User granteeUser) @@ -553,7 +552,7 @@ public class EmergencyAccessServiceTests await sutProvider.GetDependency() .Received(1) - .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.Confirmed)); + .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.Confirmed)); await sutProvider.GetDependency() .Received(1) @@ -564,7 +563,7 @@ public class EmergencyAccessServiceTests public async Task SaveAsync_PremiumCannotUpdate_ThrowsBadRequest( SutProvider sutProvider, User savingUser) { - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Type = EmergencyAccessType.Takeover, GrantorId = savingUser.Id, @@ -586,7 +585,7 @@ public class EmergencyAccessServiceTests SutProvider sutProvider, User savingUser) { savingUser.Premium = true; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Type = EmergencyAccessType.Takeover, GrantorId = new Guid(), @@ -611,7 +610,7 @@ public class EmergencyAccessServiceTests SutProvider sutProvider, User grantorUser) { grantorUser.UsesKeyConnector = true; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Type = EmergencyAccessType.Takeover, GrantorId = grantorUser.Id, @@ -633,7 +632,7 @@ public class EmergencyAccessServiceTests SutProvider sutProvider, User grantorUser) { grantorUser.UsesKeyConnector = true; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Type = EmergencyAccessType.View, GrantorId = grantorUser.Id, @@ -655,7 +654,7 @@ public class EmergencyAccessServiceTests SutProvider sutProvider, User grantorUser) { grantorUser.UsesKeyConnector = false; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Type = EmergencyAccessType.Takeover, GrantorId = grantorUser.Id, @@ -678,7 +677,7 @@ public class EmergencyAccessServiceTests { sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) - .Returns((EmergencyAccess)null); + .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.InitiateAsync(new Guid(), initiatingUser)); @@ -692,7 +691,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task InitiateAsync_EmergencyAccessGranteeIdNotEqual_ThrowBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User initiatingUser) { emergencyAccess.GranteeId = new Guid(); @@ -712,7 +711,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task InitiateAsync_EmergencyAccessStatusIsNotConfirmed_ThrowBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User initiatingUser) { emergencyAccess.GranteeId = initiatingUser.Id; @@ -735,7 +734,7 @@ public class EmergencyAccessServiceTests SutProvider sutProvider, User initiatingUser, User grantor) { grantor.UsesKeyConnector = true; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Status = EmergencyAccessStatusType.Confirmed, GranteeId = initiatingUser.Id, @@ -764,7 +763,7 @@ public class EmergencyAccessServiceTests SutProvider sutProvider, User initiatingUser, User grantor) { grantor.UsesKeyConnector = true; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Status = EmergencyAccessStatusType.Confirmed, GranteeId = initiatingUser.Id, @@ -783,14 +782,14 @@ public class EmergencyAccessServiceTests await sutProvider.GetDependency() .Received(1) - .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.RecoveryInitiated)); + .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.RecoveryInitiated)); } [Theory, BitAutoData] public async Task InitiateAsync_RequestIsCorrect_Success( SutProvider sutProvider, User initiatingUser, User grantor) { - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { Status = EmergencyAccessStatusType.Confirmed, GranteeId = initiatingUser.Id, @@ -809,7 +808,7 @@ public class EmergencyAccessServiceTests await sutProvider.GetDependency() .Received(1) - .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.RecoveryInitiated)); + .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.RecoveryInitiated)); } [Theory, BitAutoData] @@ -818,7 +817,7 @@ public class EmergencyAccessServiceTests { sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) - .Returns((EmergencyAccess)null); + .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.ApproveAsync(new Guid(), null)); @@ -829,7 +828,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task ApproveAsync_EmergencyAccessGrantorIdNotEquatToApproving_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User grantorUser) { emergencyAccess.Status = EmergencyAccessStatusType.RecoveryInitiated; @@ -851,7 +850,7 @@ public class EmergencyAccessServiceTests public async Task ApproveAsync_EmergencyAccessStatusNotRecoveryInitiated_ThrowsBadRequest( EmergencyAccessStatusType statusType, SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User grantorUser) { emergencyAccess.GrantorId = grantorUser.Id; @@ -869,7 +868,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task ApproveAsync_Success( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User grantorUser, User granteeUser) { @@ -885,20 +884,20 @@ public class EmergencyAccessServiceTests await sutProvider.Sut.ApproveAsync(emergencyAccess.Id, grantorUser); await sutProvider.GetDependency() .Received(1) - .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.RecoveryApproved)); + .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.RecoveryApproved)); } [Theory, BitAutoData] public async Task RejectAsync_EmergencyAccessIdNull_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User GrantorUser) { emergencyAccess.GrantorId = GrantorUser.Id; emergencyAccess.Status = EmergencyAccessStatusType.Accepted; sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) - .Returns((EmergencyAccess)null); + .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.RejectAsync(emergencyAccess.Id, GrantorUser)); @@ -909,7 +908,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task RejectAsync_EmergencyAccessGrantorIdNotEqualToRequestUser_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User GrantorUser) { emergencyAccess.Status = EmergencyAccessStatusType.Accepted; @@ -930,7 +929,7 @@ public class EmergencyAccessServiceTests public async Task RejectAsync_EmergencyAccessStatusNotValid_ThrowsBadRequest( EmergencyAccessStatusType statusType, SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User GrantorUser) { emergencyAccess.GrantorId = GrantorUser.Id; @@ -951,7 +950,7 @@ public class EmergencyAccessServiceTests public async Task RejectAsync_Success( EmergencyAccessStatusType statusType, SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User GrantorUser, User GranteeUser) { @@ -968,7 +967,7 @@ public class EmergencyAccessServiceTests await sutProvider.GetDependency() .Received(1) - .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.Confirmed)); + .ReplaceAsync(Arg.Is(x => x.Status == EmergencyAccessStatusType.Confirmed)); } [Theory, BitAutoData] @@ -977,7 +976,7 @@ public class EmergencyAccessServiceTests { sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) - .Returns((EmergencyAccess)null); + .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.GetPoliciesAsync(default, default)); @@ -992,7 +991,7 @@ public class EmergencyAccessServiceTests public async Task GetPoliciesAsync_RequestNotValidStatusType_ThrowsBadRequest( EmergencyAccessStatusType statusType, SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.GranteeId = granteeUser.Id; @@ -1010,7 +1009,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task GetPoliciesAsync_RequestNotValidType_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.GranteeId = granteeUser.Id; @@ -1032,7 +1031,7 @@ public class EmergencyAccessServiceTests public async Task GetPoliciesAsync_OrganizationUserTypeNotOwner_ReturnsNull( OrganizationUserType userType, SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser, User grantorUser, OrganizationUser grantorOrganizationUser) @@ -1062,7 +1061,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task GetPoliciesAsync_OrganizationUserEmpty_ReturnsNull( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser, User grantorUser) { @@ -1090,7 +1089,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task GetPoliciesAsync_ReturnsNotNull( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser, User grantorUser, OrganizationUser grantorOrganizationUser) @@ -1127,7 +1126,7 @@ public class EmergencyAccessServiceTests { sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) - .Returns((EmergencyAccess)null); + .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.TakeoverAsync(default, default)); @@ -1138,7 +1137,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task TakeoverAsync_RequestNotValid_GranteeNotEqualToRequestingUser_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.Status = EmergencyAccessStatusType.RecoveryApproved; @@ -1161,7 +1160,7 @@ public class EmergencyAccessServiceTests public async Task TakeoverAsync_RequestNotValid_StatusType_ThrowsBadRequest( EmergencyAccessStatusType statusType, SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.GranteeId = granteeUser.Id; @@ -1180,7 +1179,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task TakeoverAsync_RequestNotValid_TypeIsView_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.GranteeId = granteeUser.Id; @@ -1203,7 +1202,7 @@ public class EmergencyAccessServiceTests User grantor) { grantor.UsesKeyConnector = true; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { GrantorId = grantor.Id, GranteeId = granteeUser.Id, @@ -1232,7 +1231,7 @@ public class EmergencyAccessServiceTests User grantor) { grantor.UsesKeyConnector = false; - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { GrantorId = grantor.Id, GranteeId = granteeUser.Id, @@ -1260,7 +1259,7 @@ public class EmergencyAccessServiceTests { sutProvider.GetDependency() .GetByIdAsync(Arg.Any()) - .Returns((EmergencyAccess)null); + .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.PasswordAsync(default, default, default, default)); @@ -1271,7 +1270,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task PasswordAsync_RequestNotValid_GranteeNotEqualToRequestingUser_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.Status = EmergencyAccessStatusType.RecoveryApproved; @@ -1294,7 +1293,7 @@ public class EmergencyAccessServiceTests public async Task PasswordAsync_RequestNotValid_StatusType_ThrowsBadRequest( EmergencyAccessStatusType statusType, SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.GranteeId = granteeUser.Id; @@ -1313,7 +1312,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task PasswordAsync_RequestNotValid_TypeIsView_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.GranteeId = granteeUser.Id; @@ -1332,7 +1331,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task PasswordAsync_NonOrgUser_Success( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser, User grantorUser, string key, @@ -1367,7 +1366,7 @@ public class EmergencyAccessServiceTests public async Task PasswordAsync_OrgUser_NotOrganizationOwner_RemovedFromOrganization_Success( OrganizationUserType userType, SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser, User grantorUser, OrganizationUser organizationUser, @@ -1408,7 +1407,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task PasswordAsync_OrgUser_IsOrganizationOwner_NotRemovedFromOrganization_Success( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser, User grantorUser, OrganizationUser organizationUser, @@ -1459,7 +1458,7 @@ public class EmergencyAccessServiceTests Enabled = true } }); - var emergencyAccess = new EmergencyAccess + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess { GrantorId = grantor.Id, GranteeId = requestingUser.Id, @@ -1484,7 +1483,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task ViewAsync_EmergencyAccessTypeNotView_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.GranteeId = granteeUser.Id; @@ -1500,7 +1499,7 @@ public class EmergencyAccessServiceTests [Theory, BitAutoData] public async Task GetAttachmentDownloadAsync_EmergencyAccessTypeNotView_ThrowsBadRequest( SutProvider sutProvider, - EmergencyAccess emergencyAccess, + Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) { emergencyAccess.GranteeId = granteeUser.Id; diff --git a/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs b/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs index ae669398c5..b67bfaa131 100644 --- a/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs @@ -2,7 +2,6 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Business.Tokenables; @@ -23,6 +22,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.WebUtilities; using NSubstitute; using Xunit; +using EmergencyAccessEntity = Bit.Core.Auth.Entities.EmergencyAccess; namespace Bit.Core.Test.Auth.UserFeatures.Registration; @@ -726,7 +726,7 @@ public class RegisterUserCommandTests [BitAutoData] public async Task RegisterUserViaAcceptEmergencyAccessInviteToken_Succeeds( SutProvider sutProvider, User user, string masterPasswordHash, - EmergencyAccess emergencyAccess, string acceptEmergencyAccessInviteToken, Guid acceptEmergencyAccessId) + EmergencyAccessEntity emergencyAccess, string acceptEmergencyAccessInviteToken, Guid acceptEmergencyAccessId) { // Arrange user.Email = $"test+{Guid.NewGuid()}@example.com"; @@ -767,7 +767,7 @@ public class RegisterUserCommandTests [Theory] [BitAutoData] public async Task RegisterUserViaAcceptEmergencyAccessInviteToken_InvalidToken_ThrowsBadRequestException(SutProvider sutProvider, User user, - string masterPasswordHash, EmergencyAccess emergencyAccess, string acceptEmergencyAccessInviteToken, Guid acceptEmergencyAccessId) + string masterPasswordHash, EmergencyAccessEntity emergencyAccess, string acceptEmergencyAccessInviteToken, Guid acceptEmergencyAccessId) { // Arrange user.Email = $"test+{Guid.NewGuid()}@example.com"; @@ -1112,7 +1112,7 @@ public class RegisterUserCommandTests [BitAutoData] public async Task RegisterUserViaAcceptEmergencyAccessInviteToken_BlockedDomain_ThrowsBadRequestException( SutProvider sutProvider, User user, string masterPasswordHash, - EmergencyAccess emergencyAccess, string acceptEmergencyAccessInviteToken, Guid acceptEmergencyAccessId) + EmergencyAccessEntity emergencyAccess, string acceptEmergencyAccessInviteToken, Guid acceptEmergencyAccessId) { // Arrange user.Email = "user@blocked-domain.com"; From bfe2e7717d32b2de3ca6128934b1e5496362ca29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Fri, 23 Jan 2026 11:07:56 +0000 Subject: [PATCH 14/51] [PM-30615] Fix Public API List Collections returning Default Collections (#6841) --- .../Controllers/CollectionsController.cs | 5 +- .../Public/CollectionsControllerTests.cs | 61 +++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/Api/Public/Controllers/CollectionsController.cs b/src/Api/Public/Controllers/CollectionsController.cs index a567062a5e..28de4dc16d 100644 --- a/src/Api/Public/Controllers/CollectionsController.cs +++ b/src/Api/Public/Controllers/CollectionsController.cs @@ -67,8 +67,9 @@ public class CollectionsController : Controller { var collections = await _collectionRepository.GetManyByOrganizationIdWithAccessAsync(_currentContext.OrganizationId.Value); - var collectionResponses = collections.Select(c => - new CollectionResponseModel(c.Item1, c.Item2.Groups)); + var collectionResponses = collections + .Where(c => c.Item1.Type != CollectionType.DefaultUserCollection) + .Select(c => new CollectionResponseModel(c.Item1, c.Item2.Groups)); var response = new ListResponseModel(collectionResponses); return new JsonResult(response); diff --git a/test/Api.IntegrationTest/Controllers/Public/CollectionsControllerTests.cs b/test/Api.IntegrationTest/Controllers/Public/CollectionsControllerTests.cs index a729abb849..3551ed4efa 100644 --- a/test/Api.IntegrationTest/Controllers/Public/CollectionsControllerTests.cs +++ b/test/Api.IntegrationTest/Controllers/Public/CollectionsControllerTests.cs @@ -6,6 +6,7 @@ using Bit.Api.Models.Public.Response; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Enums; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Platform.Push; @@ -114,4 +115,64 @@ public class CollectionsControllerTests : IClassFixture, Assert.NotEmpty(result.Item2.Groups); Assert.NotEmpty(result.Item2.Users); } + + [Fact] + public async Task List_ExcludesDefaultUserCollections_IncludesGroupsAndUsers() + { + // Arrange + var collectionRepository = _factory.GetService(); + var groupRepository = _factory.GetService(); + + var defaultCollection = new Collection + { + OrganizationId = _organization.Id, + Name = "My Items", + Type = CollectionType.DefaultUserCollection + }; + await collectionRepository.CreateAsync(defaultCollection, null, null); + + var group = await groupRepository.CreateAsync(new Group + { + OrganizationId = _organization.Id, + Name = "Test Group", + ExternalId = $"test-group-{Guid.NewGuid()}", + }); + + var (_, user) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, + _organization.Id, + OrganizationUserType.User); + + var sharedCollection = await OrganizationTestHelpers.CreateCollectionAsync( + _factory, + _organization.Id, + "Shared Collection with Access", + externalId: "shared-collection-with-access", + groups: + [ + new CollectionAccessSelection { Id = group.Id, ReadOnly = false, HidePasswords = false, Manage = true } + ], + users: + [ + new CollectionAccessSelection { Id = user.Id, ReadOnly = true, HidePasswords = true, Manage = false } + ]); + + // Act + var response = await _client.GetFromJsonAsync>("public/collections"); + + // Assert + Assert.NotNull(response); + + Assert.DoesNotContain(response.Data, c => c.Id == defaultCollection.Id); + + var collectionResponse = response.Data.First(c => c.Id == sharedCollection.Id); + Assert.NotNull(collectionResponse.Groups); + Assert.Single(collectionResponse.Groups); + + var groupResponse = collectionResponse.Groups.First(); + Assert.Equal(group.Id, groupResponse.Id); + Assert.False(groupResponse.ReadOnly); + Assert.False(groupResponse.HidePasswords); + Assert.True(groupResponse.Manage); + } } From b360d6a00a547e88e4c524aea63c7bfaa25274fb Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 23 Jan 2026 11:43:05 +0000 Subject: [PATCH 15/51] [deps]: Update github-action minor (#6868) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> --- .github/workflows/build.yml | 10 +++++----- .github/workflows/test-database.yml | 6 +++--- .github/workflows/test.yml | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a7717be4e8..f3cc279a58 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -31,7 +31,7 @@ jobs: persist-credentials: false - name: Set up .NET - uses: actions/setup-dotnet@2016bd2012dba4e32de620c46fe006a3ac9f0602 # v5.0.1 + uses: actions/setup-dotnet@baa11fbfe1d6520db94683bd5c7a3818018e4309 # v5.1.0 - name: Verify format run: dotnet format --verify-no-changes @@ -119,10 +119,10 @@ jobs: fi - name: Set up .NET - uses: actions/setup-dotnet@2016bd2012dba4e32de620c46fe006a3ac9f0602 # v5.0.1 + uses: actions/setup-dotnet@baa11fbfe1d6520db94683bd5c7a3818018e4309 # v5.1.0 - name: Set up Node - uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # v6.1.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: "npm" cache-dependency-path: "**/package-lock.json" @@ -294,7 +294,7 @@ jobs: persist-credentials: false - name: Set up .NET - uses: actions/setup-dotnet@2016bd2012dba4e32de620c46fe006a3ac9f0602 # v5.0.1 + uses: actions/setup-dotnet@baa11fbfe1d6520db94683bd5c7a3818018e4309 # v5.1.0 - name: Log in to Azure uses: bitwarden/gh-actions/azure-login@main @@ -420,7 +420,7 @@ jobs: persist-credentials: false - name: Set up .NET - uses: actions/setup-dotnet@2016bd2012dba4e32de620c46fe006a3ac9f0602 # v5.0.1 + uses: actions/setup-dotnet@baa11fbfe1d6520db94683bd5c7a3818018e4309 # v5.1.0 - name: Print environment run: | diff --git a/.github/workflows/test-database.yml b/.github/workflows/test-database.yml index 4630c18e40..25ff9d0488 100644 --- a/.github/workflows/test-database.yml +++ b/.github/workflows/test-database.yml @@ -49,7 +49,7 @@ jobs: persist-credentials: false - name: Set up .NET - uses: actions/setup-dotnet@2016bd2012dba4e32de620c46fe006a3ac9f0602 # v5.0.1 + uses: actions/setup-dotnet@baa11fbfe1d6520db94683bd5c7a3818018e4309 # v5.1.0 - name: Restore tools run: dotnet tool restore @@ -156,7 +156,7 @@ jobs: run: 'docker logs "$(docker ps --quiet --filter "name=mssql")"' - name: Report test results - uses: dorny/test-reporter@fe45e9537387dac839af0d33ba56eed8e24189e8 # v2.3.0 + uses: dorny/test-reporter@b082adf0eced0765477756c2a610396589b8c637 # v2.5.0 if: ${{ github.event.pull_request.head.repo.full_name == github.repository && !cancelled() }} with: name: Test Results @@ -183,7 +183,7 @@ jobs: persist-credentials: false - name: Set up .NET - uses: actions/setup-dotnet@2016bd2012dba4e32de620c46fe006a3ac9f0602 # v5.0.1 + uses: actions/setup-dotnet@baa11fbfe1d6520db94683bd5c7a3818018e4309 # v5.1.0 - name: Print environment run: | diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a6d07bb650..12b5355c33 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: persist-credentials: false - name: Set up .NET - uses: actions/setup-dotnet@2016bd2012dba4e32de620c46fe006a3ac9f0602 # v5.0.1 + uses: actions/setup-dotnet@baa11fbfe1d6520db94683bd5c7a3818018e4309 # v5.1.0 - name: Install rust uses: dtolnay/rust-toolchain@f7ccc83f9ed1e5b9c81d8a67d7ad1a747e22a561 # stable @@ -59,7 +59,7 @@ jobs: run: dotnet test ./bitwarden_license/test --configuration Debug --logger "trx;LogFileName=bw-test-results.trx" /p:CoverletOutputFormatter="cobertura" --collect:"XPlat Code Coverage" - name: Report test results - uses: dorny/test-reporter@fe45e9537387dac839af0d33ba56eed8e24189e8 # v2.3.0 + uses: dorny/test-reporter@b082adf0eced0765477756c2a610396589b8c637 # v2.5.0 if: ${{ github.event.pull_request.head.repo.full_name == github.repository && !cancelled() }} with: name: Test Results From 867e61694bf31722992c4f779e9326cc48cc01e3 Mon Sep 17 00:00:00 2001 From: Robyn MacCallum Date: Fri, 23 Jan 2026 09:05:58 -0500 Subject: [PATCH 16/51] Add NotificationUndeterminedCipherScenarioLogic feature flag (#6884) * Add NotificationUndeterminedCipherScenarioLogic feature flag * Remove whitespace --- src/Core/Constants.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 9ffe199f1d..c3c5cb5e5b 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -174,6 +174,7 @@ public static class FeatureFlagKeys public const string MacOsNativeCredentialSync = "macos-native-credential-sync"; public const string WindowsDesktopAutotype = "windows-desktop-autotype"; public const string WindowsDesktopAutotypeGA = "windows-desktop-autotype-ga"; + public const string NotificationUndeterminedCipherScenarioLogic = "undetermined-cipher-scenario-logic"; /* Billing Team */ public const string TrialPayment = "PM-8163-trial-payment"; From b623e381b4c52b993c83db7f31c1b4cfd82431c0 Mon Sep 17 00:00:00 2001 From: Vijay Oommen Date: Fri, 23 Jan 2026 08:34:19 -0600 Subject: [PATCH 17/51] PM-30799 added validation for DomainName (#6856) --- .../Request/OrganizationDomainRequestModel.cs | 2 + src/Core/Utilities/DomainNameAttribute.cs | 64 ++++++++++++++ .../Utilities/DomainNameAttributeTests.cs | 84 +++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 src/Core/Utilities/DomainNameAttribute.cs create mode 100644 test/Core.Test/Utilities/DomainNameAttributeTests.cs diff --git a/src/Api/AdminConsole/Models/Request/OrganizationDomainRequestModel.cs b/src/Api/AdminConsole/Models/Request/OrganizationDomainRequestModel.cs index 46b253da31..3a2ada719f 100644 --- a/src/Api/AdminConsole/Models/Request/OrganizationDomainRequestModel.cs +++ b/src/Api/AdminConsole/Models/Request/OrganizationDomainRequestModel.cs @@ -2,11 +2,13 @@ #nullable disable using System.ComponentModel.DataAnnotations; +using Bit.Core.Utilities; namespace Bit.Api.AdminConsole.Models.Request; public class OrganizationDomainRequestModel { [Required] + [DomainNameValidator] public string DomainName { get; set; } } diff --git a/src/Core/Utilities/DomainNameAttribute.cs b/src/Core/Utilities/DomainNameAttribute.cs new file mode 100644 index 0000000000..9b571e96d7 --- /dev/null +++ b/src/Core/Utilities/DomainNameAttribute.cs @@ -0,0 +1,64 @@ +using System.ComponentModel.DataAnnotations; +using System.Text.RegularExpressions; + +namespace Bit.Core.Utilities; + +/// +/// https://bitwarden.atlassian.net/browse/VULN-376 +/// Domain names are vulnerable to XSS attacks if not properly validated. +/// Domain names can contain letters, numbers, dots, and hyphens. +/// Domain names maybe internationalized (IDN) and contain unicode characters. +/// +public class DomainNameValidatorAttribute : ValidationAttribute +{ + // RFC 1123 compliant domain name regex + // - Allows alphanumeric characters and hyphens + // - Cannot start or end with a hyphen + // - Each label (part between dots) must be 1-63 characters + // - Total length should not exceed 253 characters + // - Supports internationalized domain names (IDN) - which is why this regex includes unicode ranges + private static readonly Regex _domainNameRegex = new( + @"^(?:[a-zA-Z0-9\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF](?:[a-zA-Z0-9\-\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF]{0,61}[a-zA-Z0-9\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])?\.)*[a-zA-Z0-9\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF](?:[a-zA-Z0-9\-\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF]{0,61}[a-zA-Z0-9\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])?$", + RegexOptions.Compiled | RegexOptions.IgnoreCase + ); + + public DomainNameValidatorAttribute() + : base("The {0} field is not a valid domain name.") + { } + + public override bool IsValid(object? value) + { + if (value == null) + { + return true; // Use [Required] for null checks + } + + var domainName = value.ToString(); + + if (string.IsNullOrWhiteSpace(domainName)) + { + return false; + } + + // Reject if contains any whitespace (including leading/trailing spaces, tabs, newlines) + if (domainName.Any(char.IsWhiteSpace)) + { + return false; + } + + // Check length constraints + if (domainName.Length > 253) + { + return false; + } + + // Check for control characters or other dangerous characters + if (domainName.Any(c => char.IsControl(c) || c == '<' || c == '>' || c == '"' || c == '\'' || c == '&')) + { + return false; + } + + // Validate against domain name regex + return _domainNameRegex.IsMatch(domainName); + } +} diff --git a/test/Core.Test/Utilities/DomainNameAttributeTests.cs b/test/Core.Test/Utilities/DomainNameAttributeTests.cs new file mode 100644 index 0000000000..3f3190c9a1 --- /dev/null +++ b/test/Core.Test/Utilities/DomainNameAttributeTests.cs @@ -0,0 +1,84 @@ +using Bit.Core.Utilities; +using Xunit; + +namespace Bit.Core.Test.Utilities; + +public class DomainNameValidatorAttributeTests +{ + [Theory] + [InlineData("example.com")] // basic domain + [InlineData("sub.example.com")] // subdomain + [InlineData("sub.sub2.example.com")] // multiple subdomains + [InlineData("example-dash.com")] // domain with dash + [InlineData("123example.com")] // domain starting with number + [InlineData("example123.com")] // domain with numbers + [InlineData("e.com")] // short domain + [InlineData("very-long-subdomain-name.example.com")] // long subdomain + [InlineData("wörldé.com")] // unicode domain (IDN) + public void IsValid_ReturnsTrueWhenValid(string domainName) + { + var sut = new DomainNameValidatorAttribute(); + + var actual = sut.IsValid(domainName); + + Assert.True(actual); + } + + [Theory] + [InlineData("")] // XSS attempt + [InlineData("example.com