From 7daf6cfad48260cb33f5f6b32f8f580a266d6f71 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Fri, 14 Mar 2025 11:33:24 -0400 Subject: [PATCH 01/18] [PM-18794] Allow provider payment method (#5500) * Add PaymentSource to ProviderSubscriptionResponse * Add UpdatePaymentMethod to ProviderBillingController * Add GetTaxInformation to ProviderBillingController * Add VerifyBankAccount to ProviderBillingController * Add feature flag --- .../Billing/ProviderBillingService.cs | 13 +++ .../Controllers/ProviderBillingController.cs | 83 ++++++++++++++++++- .../Responses/ProviderSubscriptionResponse.cs | 9 +- .../Services/IProviderBillingService.cs | 11 +++ src/Core/Constants.cs | 1 + 5 files changed, 113 insertions(+), 4 deletions(-) diff --git a/bitwarden_license/src/Commercial.Core/Billing/ProviderBillingService.cs b/bitwarden_license/src/Commercial.Core/Billing/ProviderBillingService.cs index 294a926022..74cfc1f916 100644 --- a/bitwarden_license/src/Commercial.Core/Billing/ProviderBillingService.cs +++ b/bitwarden_license/src/Commercial.Core/Billing/ProviderBillingService.cs @@ -628,6 +628,19 @@ public class ProviderBillingService( } } + public async Task UpdatePaymentMethod( + Provider provider, + TokenizedPaymentSource tokenizedPaymentSource, + TaxInformation taxInformation) + { + await Task.WhenAll( + subscriberService.UpdatePaymentSource(provider, tokenizedPaymentSource), + subscriberService.UpdateTaxInformation(provider, taxInformation)); + + await stripeAdapter.SubscriptionUpdateAsync(provider.GatewaySubscriptionId, + new SubscriptionUpdateOptions { CollectionMethod = StripeConstants.CollectionMethod.ChargeAutomatically }); + } + public async Task UpdateSeatMinimums(UpdateProviderSeatMinimumsCommand command) { if (command.Configuration.Any(x => x.SeatsMinimum < 0)) diff --git a/src/Api/Billing/Controllers/ProviderBillingController.cs b/src/Api/Billing/Controllers/ProviderBillingController.cs index 73c992040c..bb1fd7bb25 100644 --- a/src/Api/Billing/Controllers/ProviderBillingController.cs +++ b/src/Api/Billing/Controllers/ProviderBillingController.cs @@ -1,5 +1,6 @@ using Bit.Api.Billing.Models.Requests; using Bit.Api.Billing.Models.Responses; +using Bit.Core; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Models; using Bit.Core.Billing.Pricing; @@ -20,6 +21,7 @@ namespace Bit.Api.Billing.Controllers; [Authorize("Application")] public class ProviderBillingController( ICurrentContext currentContext, + IFeatureService featureService, ILogger logger, IPricingClient pricingClient, IProviderBillingService providerBillingService, @@ -71,6 +73,65 @@ public class ProviderBillingController( "text/csv"); } + [HttpPut("payment-method")] + public async Task UpdatePaymentMethodAsync( + [FromRoute] Guid providerId, + [FromBody] UpdatePaymentMethodRequestBody requestBody) + { + var allowProviderPaymentMethod = featureService.IsEnabled(FeatureFlagKeys.PM18794_ProviderPaymentMethod); + + if (!allowProviderPaymentMethod) + { + return TypedResults.NotFound(); + } + + var (provider, result) = await TryGetBillableProviderForAdminOperation(providerId); + + if (provider == null) + { + return result; + } + + var tokenizedPaymentSource = requestBody.PaymentSource.ToDomain(); + var taxInformation = requestBody.TaxInformation.ToDomain(); + + await providerBillingService.UpdatePaymentMethod( + provider, + tokenizedPaymentSource, + taxInformation); + + return TypedResults.Ok(); + } + + [HttpPost("payment-method/verify-bank-account")] + public async Task VerifyBankAccountAsync( + [FromRoute] Guid providerId, + [FromBody] VerifyBankAccountRequestBody requestBody) + { + var allowProviderPaymentMethod = featureService.IsEnabled(FeatureFlagKeys.PM18794_ProviderPaymentMethod); + + if (!allowProviderPaymentMethod) + { + return TypedResults.NotFound(); + } + + var (provider, result) = await TryGetBillableProviderForAdminOperation(providerId); + + if (provider == null) + { + return result; + } + + if (requestBody.DescriptorCode.Length != 6 || !requestBody.DescriptorCode.StartsWith("SM")) + { + return Error.BadRequest("Statement descriptor should be a 6-character value that starts with 'SM'"); + } + + await subscriberService.VerifyBankAccount(provider, requestBody.DescriptorCode); + + return TypedResults.Ok(); + } + [HttpGet("subscription")] public async Task GetSubscriptionAsync([FromRoute] Guid providerId) { @@ -102,12 +163,32 @@ public class ProviderBillingController( var subscriptionSuspension = await GetSubscriptionSuspensionAsync(stripeAdapter, subscription); + var paymentSource = await subscriberService.GetPaymentSource(provider); + var response = ProviderSubscriptionResponse.From( subscription, configuredProviderPlans, taxInformation, subscriptionSuspension, - provider); + provider, + paymentSource); + + return TypedResults.Ok(response); + } + + [HttpGet("tax-information")] + public async Task GetTaxInformationAsync([FromRoute] Guid providerId) + { + var (provider, result) = await TryGetBillableProviderForAdminOperation(providerId); + + if (provider == null) + { + return result; + } + + var taxInformation = await subscriberService.GetTaxInformation(provider); + + var response = TaxInformationResponse.From(taxInformation); return TypedResults.Ok(response); } diff --git a/src/Api/Billing/Models/Responses/ProviderSubscriptionResponse.cs b/src/Api/Billing/Models/Responses/ProviderSubscriptionResponse.cs index 34c3817e51..ea1479c9df 100644 --- a/src/Api/Billing/Models/Responses/ProviderSubscriptionResponse.cs +++ b/src/Api/Billing/Models/Responses/ProviderSubscriptionResponse.cs @@ -16,7 +16,8 @@ public record ProviderSubscriptionResponse( TaxInformation TaxInformation, DateTime? CancelAt, SubscriptionSuspension Suspension, - ProviderType ProviderType) + ProviderType ProviderType, + PaymentSource PaymentSource) { private const string _annualCadence = "Annual"; private const string _monthlyCadence = "Monthly"; @@ -26,7 +27,8 @@ public record ProviderSubscriptionResponse( ICollection providerPlans, TaxInformation taxInformation, SubscriptionSuspension subscriptionSuspension, - Provider provider) + Provider provider, + PaymentSource paymentSource) { var providerPlanResponses = providerPlans .Select(providerPlan => @@ -57,7 +59,8 @@ public record ProviderSubscriptionResponse( taxInformation, subscription.CancelAt, subscriptionSuspension, - provider.Type); + provider.Type, + paymentSource); } } diff --git a/src/Core/Billing/Services/IProviderBillingService.cs b/src/Core/Billing/Services/IProviderBillingService.cs index d6983da03e..64585f3361 100644 --- a/src/Core/Billing/Services/IProviderBillingService.cs +++ b/src/Core/Billing/Services/IProviderBillingService.cs @@ -95,5 +95,16 @@ public interface IProviderBillingService Task SetupSubscription( Provider provider); + /// + /// Updates the 's payment source and tax information and then sets their subscription's collection_method to be "charge_automatically". + /// + /// The to update the payment source and tax information for. + /// The tokenized payment source (ex. Credit Card) to attach to the . + /// The 's updated tax information. + Task UpdatePaymentMethod( + Provider provider, + TokenizedPaymentSource tokenizedPaymentSource, + TaxInformation taxInformation); + Task UpdateSeatMinimums(UpdateProviderSeatMinimumsCommand command); } diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 72224319fb..9cbf6b788a 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -175,6 +175,7 @@ public static class FeatureFlagKeys public const string WebPush = "web-push"; public const string AndroidImportLoginsFlow = "import-logins-flow"; public const string PM12276Breadcrumbing = "pm-12276-breadcrumbing-for-business-features"; + public const string PM18794_ProviderPaymentMethod = "pm-18794-provider-payment-method"; public static List GetAllKeys() { From 488a9847ea15d977d28582bc44da110758e46ad1 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Fri, 14 Mar 2025 12:00:58 -0500 Subject: [PATCH 02/18] Partial for CommandResult (#5482) * Example of how a partial success/failure command result would look. * Fixed code. * Added Validator and ValidationResult * Moved errors into their own files. * Fixing tests * fixed import. * Forgot mock error. --- src/Api/Utilities/CommandResultExtensions.cs | 2 +- src/Core/AdminConsole/Errors/Error.cs | 3 + .../Errors/InsufficientPermissionsError.cs | 11 ++++ .../Errors/RecordNotFoundError.cs | 11 ++++ .../Shared/Validation/IValidator.cs | 6 ++ .../Shared/Validation/ValidationResult.cs | 15 +++++ src/Core/Models/Commands/CommandResult.cs | 27 ++++++--- .../AdminConsole/Shared/IValidatorTests.cs | 58 +++++++++++++++++++ .../Models/Commands/CommandResultTests.cs | 53 +++++++++++++++++ 9 files changed, 176 insertions(+), 10 deletions(-) create mode 100644 src/Core/AdminConsole/Errors/Error.cs create mode 100644 src/Core/AdminConsole/Errors/InsufficientPermissionsError.cs create mode 100644 src/Core/AdminConsole/Errors/RecordNotFoundError.cs create mode 100644 src/Core/AdminConsole/Shared/Validation/IValidator.cs create mode 100644 src/Core/AdminConsole/Shared/Validation/ValidationResult.cs create mode 100644 test/Core.Test/AdminConsole/Shared/IValidatorTests.cs create mode 100644 test/Core.Test/Models/Commands/CommandResultTests.cs diff --git a/src/Api/Utilities/CommandResultExtensions.cs b/src/Api/Utilities/CommandResultExtensions.cs index 39104db7ff..c7315a0fa0 100644 --- a/src/Api/Utilities/CommandResultExtensions.cs +++ b/src/Api/Utilities/CommandResultExtensions.cs @@ -12,7 +12,7 @@ public static class CommandResultExtensions NoRecordFoundFailure failure => new ObjectResult(failure.ErrorMessages) { StatusCode = StatusCodes.Status404NotFound }, BadRequestFailure failure => new ObjectResult(failure.ErrorMessages) { StatusCode = StatusCodes.Status400BadRequest }, Failure failure => new ObjectResult(failure.ErrorMessages) { StatusCode = StatusCodes.Status400BadRequest }, - Success success => new ObjectResult(success.Data) { StatusCode = StatusCodes.Status200OK }, + Success success => new ObjectResult(success.Value) { StatusCode = StatusCodes.Status200OK }, _ => throw new InvalidOperationException($"Unhandled commandResult type: {commandResult.GetType().Name}") }; } diff --git a/src/Core/AdminConsole/Errors/Error.cs b/src/Core/AdminConsole/Errors/Error.cs new file mode 100644 index 0000000000..6c8eed41a4 --- /dev/null +++ b/src/Core/AdminConsole/Errors/Error.cs @@ -0,0 +1,3 @@ +namespace Bit.Core.AdminConsole.Errors; + +public record Error(string Message, T ErroredValue); diff --git a/src/Core/AdminConsole/Errors/InsufficientPermissionsError.cs b/src/Core/AdminConsole/Errors/InsufficientPermissionsError.cs new file mode 100644 index 0000000000..d04ceba7c9 --- /dev/null +++ b/src/Core/AdminConsole/Errors/InsufficientPermissionsError.cs @@ -0,0 +1,11 @@ +namespace Bit.Core.AdminConsole.Errors; + +public record InsufficientPermissionsError(string Message, T ErroredValue) : Error(Message, ErroredValue) +{ + public const string Code = "Insufficient Permissions"; + + public InsufficientPermissionsError(T ErroredValue) : this(Code, ErroredValue) + { + + } +} diff --git a/src/Core/AdminConsole/Errors/RecordNotFoundError.cs b/src/Core/AdminConsole/Errors/RecordNotFoundError.cs new file mode 100644 index 0000000000..25a169efe1 --- /dev/null +++ b/src/Core/AdminConsole/Errors/RecordNotFoundError.cs @@ -0,0 +1,11 @@ +namespace Bit.Core.AdminConsole.Errors; + +public record RecordNotFoundError(string Message, T ErroredValue) : Error(Message, ErroredValue) +{ + public const string Code = "Record Not Found"; + + public RecordNotFoundError(T ErroredValue) : this(Code, ErroredValue) + { + + } +} diff --git a/src/Core/AdminConsole/Shared/Validation/IValidator.cs b/src/Core/AdminConsole/Shared/Validation/IValidator.cs new file mode 100644 index 0000000000..d90386f00e --- /dev/null +++ b/src/Core/AdminConsole/Shared/Validation/IValidator.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.AdminConsole.Shared.Validation; + +public interface IValidator +{ + public Task> ValidateAsync(T value); +} diff --git a/src/Core/AdminConsole/Shared/Validation/ValidationResult.cs b/src/Core/AdminConsole/Shared/Validation/ValidationResult.cs new file mode 100644 index 0000000000..e25103e701 --- /dev/null +++ b/src/Core/AdminConsole/Shared/Validation/ValidationResult.cs @@ -0,0 +1,15 @@ +using Bit.Core.AdminConsole.Errors; + +namespace Bit.Core.AdminConsole.Shared.Validation; + +public abstract record ValidationResult; + +public record Valid : ValidationResult +{ + public T Value { get; init; } +} + +public record Invalid : ValidationResult +{ + public IEnumerable> Errors { get; init; } +} diff --git a/src/Core/Models/Commands/CommandResult.cs b/src/Core/Models/Commands/CommandResult.cs index ae14b7d2f9..a8ec772fc1 100644 --- a/src/Core/Models/Commands/CommandResult.cs +++ b/src/Core/Models/Commands/CommandResult.cs @@ -1,5 +1,7 @@ #nullable enable +using Bit.Core.AdminConsole.Errors; + namespace Bit.Core.Models.Commands; public class CommandResult(IEnumerable errors) @@ -9,7 +11,6 @@ public class CommandResult(IEnumerable errors) public bool Success => ErrorMessages.Count == 0; public bool HasErrors => ErrorMessages.Count > 0; public List ErrorMessages { get; } = errors.ToList(); - public CommandResult() : this(Array.Empty()) { } } @@ -29,22 +30,30 @@ public class Success : CommandResult { } -public abstract class CommandResult -{ +public abstract class CommandResult; +public class Success(T value) : CommandResult +{ + public T Value { get; } = value; } -public class Success(T data) : CommandResult +public class Failure(IEnumerable errorMessages) : CommandResult { - public T? Data { get; init; } = data; + public List ErrorMessages { get; } = errorMessages.ToList(); + + public string ErrorMessage => string.Join(" ", ErrorMessages); + + public Failure(string error) : this([error]) { } } -public class Failure(IEnumerable errorMessage) : CommandResult +public class Partial : CommandResult { - public IEnumerable ErrorMessages { get; init; } = errorMessage; + public T[] Successes { get; set; } = []; + public Error[] Failures { get; set; } = []; - public Failure(string errorMessage) : this(new[] { errorMessage }) + public Partial(IEnumerable successfulItems, IEnumerable> failedItems) { + Successes = successfulItems.ToArray(); + Failures = failedItems.ToArray(); } } - diff --git a/test/Core.Test/AdminConsole/Shared/IValidatorTests.cs b/test/Core.Test/AdminConsole/Shared/IValidatorTests.cs new file mode 100644 index 0000000000..abb49c25c6 --- /dev/null +++ b/test/Core.Test/AdminConsole/Shared/IValidatorTests.cs @@ -0,0 +1,58 @@ +using Bit.Core.AdminConsole.Errors; +using Bit.Core.AdminConsole.Shared.Validation; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.Shared; + +public class IValidatorTests +{ + public class TestClass + { + public string Name { get; set; } = string.Empty; + } + + public record InvalidRequestError(T ErroredValue) : Error(Code, ErroredValue) + { + public const string Code = "InvalidRequest"; + } + + public class TestClassValidator : IValidator + { + public Task> ValidateAsync(TestClass value) + { + if (string.IsNullOrWhiteSpace(value.Name)) + { + return Task.FromResult>(new Invalid + { + Errors = [new InvalidRequestError(value)] + }); + } + + return Task.FromResult>(new Valid { Value = value }); + } + } + + [Fact] + public async Task ValidateAsync_WhenSomethingIsInvalid_ReturnsInvalidWithError() + { + var example = new TestClass(); + + var result = await new TestClassValidator().ValidateAsync(example); + + Assert.IsType>(result); + var invalidResult = result as Invalid; + Assert.Equal(InvalidRequestError.Code, invalidResult.Errors.First().Message); + } + + [Fact] + public async Task ValidateAsync_WhenIsValid_ReturnsValid() + { + var example = new TestClass { Name = "Valid" }; + + var result = await new TestClassValidator().ValidateAsync(example); + + Assert.IsType>(result); + var validResult = result as Valid; + Assert.Equal(example.Name, validResult.Value.Name); + } +} diff --git a/test/Core.Test/Models/Commands/CommandResultTests.cs b/test/Core.Test/Models/Commands/CommandResultTests.cs new file mode 100644 index 0000000000..c500fef4f5 --- /dev/null +++ b/test/Core.Test/Models/Commands/CommandResultTests.cs @@ -0,0 +1,53 @@ +using Bit.Core.AdminConsole.Errors; +using Bit.Core.Models.Commands; +using Bit.Test.Common.AutoFixture.Attributes; +using Xunit; + +namespace Bit.Core.Test.Models.Commands; + +public class CommandResultTests +{ + public class TestItem + { + public Guid Id { get; set; } + public string Value { get; set; } + } + + public CommandResult BulkAction(IEnumerable items) + { + var itemList = items.ToList(); + var successfulItems = items.Where(x => x.Value == "SuccessfulRequest").ToArray(); + + var failedItems = itemList.Except(successfulItems).ToArray(); + + var notFound = failedItems.First(x => x.Value == "Failed due to not found"); + var invalidPermissions = failedItems.First(x => x.Value == "Failed due to invalid permissions"); + + var notFoundError = new RecordNotFoundError(notFound); + var insufficientPermissionsError = new InsufficientPermissionsError(invalidPermissions); + + return new Partial(successfulItems.ToArray(), [notFoundError, insufficientPermissionsError]); + } + + [Theory] + [BitAutoData] + public void Partial_CommandResult_BulkRequestWithSuccessAndFailures(Guid successId1, Guid failureId1, Guid failureId2) + { + var listOfRecords = new List + { + new TestItem() { Id = successId1, Value = "SuccessfulRequest" }, + new TestItem() { Id = failureId1, Value = "Failed due to not found" }, + new TestItem() { Id = failureId2, Value = "Failed due to invalid permissions" } + }; + + var result = BulkAction(listOfRecords); + + Assert.IsType>(result); + + var failures = (result as Partial).Failures.ToArray(); + var success = (result as Partial).Successes.First(); + + Assert.Equal(listOfRecords.First(), success); + Assert.Equal(2, failures.Length); + } +} From 27606e2d334f03d59571c5eb6b4c123e40f9071a Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Fri, 14 Mar 2025 13:22:22 -0400 Subject: [PATCH 03/18] [PM-3553] Feature flag: Mobile SimpleLogin self host alias generation (#5392) --- src/Core/Constants.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 9cbf6b788a..e09422871d 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -176,6 +176,7 @@ public static class FeatureFlagKeys public const string AndroidImportLoginsFlow = "import-logins-flow"; public const string PM12276Breadcrumbing = "pm-12276-breadcrumbing-for-business-features"; public const string PM18794_ProviderPaymentMethod = "pm-18794-provider-payment-method"; + public const string PM3553_MobileSimpleLoginSelfHostAlias = "simple-login-self-host-alias"; public static List GetAllKeys() { From abfdf6f5cb0f1f1504dbaaaa0e04ce9cb60faf19 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Mon, 17 Mar 2025 12:37:34 -0400 Subject: [PATCH 04/18] Revert "[PM-18944] Update error response from invalid OTP" (#5504) * Revert "[PM-18944] Update error response from invalid OTP (#5485)" This reverts commit 1b90bfe2a114e73b583100e955e57ac48b9733b9. --- .../RequestValidators/DeviceValidator.cs | 13 +++++++++---- .../IdentityServer/DeviceValidatorTests.cs | 8 ++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs b/src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs index 4744f4aca3..36a08326ab 100644 --- a/src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs @@ -250,21 +250,26 @@ public class DeviceValidator( var customResponse = new Dictionary(); switch (errorType) { + /* + * The ErrorMessage is brittle and is used to control the flow in the clients. Do not change them without updating the client as well. + * There is a backwards compatibility issue as well: if you make a change on the clients then ensure that they are backwards + * compatible. + */ case DeviceValidationResultType.InvalidUser: result.ErrorDescription = "Invalid user"; - customResponse.Add("ErrorModel", new ErrorResponseModel("Invalid user.")); + customResponse.Add("ErrorModel", new ErrorResponseModel("invalid user")); break; case DeviceValidationResultType.InvalidNewDeviceOtp: result.ErrorDescription = "Invalid New Device OTP"; - customResponse.Add("ErrorModel", new ErrorResponseModel("Invalid new device OTP. Try again.")); + customResponse.Add("ErrorModel", new ErrorResponseModel("invalid new device otp")); break; case DeviceValidationResultType.NewDeviceVerificationRequired: result.ErrorDescription = "New device verification required"; - customResponse.Add("ErrorModel", new ErrorResponseModel("New device verification required.")); + customResponse.Add("ErrorModel", new ErrorResponseModel("new device verification required")); break; case DeviceValidationResultType.NoDeviceInformationProvided: result.ErrorDescription = "No device information provided"; - customResponse.Add("ErrorModel", new ErrorResponseModel("No device information provided.")); + customResponse.Add("ErrorModel", new ErrorResponseModel("no device information provided")); break; } return (result, customResponse); diff --git a/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs b/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs index bcb357d640..b71dd6c230 100644 --- a/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs @@ -172,7 +172,7 @@ public class DeviceValidatorTests Assert.False(result); Assert.NotNull(context.CustomResponse["ErrorModel"]); - var expectedErrorModel = new ErrorResponseModel("No device information provided."); + var expectedErrorModel = new ErrorResponseModel("no device information provided"); var actualResponse = (ErrorResponseModel)context.CustomResponse["ErrorModel"]; Assert.Equal(expectedErrorModel.Message, actualResponse.Message); } @@ -418,7 +418,7 @@ public class DeviceValidatorTests Assert.False(result); Assert.NotNull(context.CustomResponse["ErrorModel"]); // PM-13340: The error message should be "invalid user" instead of "no device information provided" - var expectedErrorMessage = "No device information provided."; + var expectedErrorMessage = "no device information provided"; var actualResponse = (ErrorResponseModel)context.CustomResponse["ErrorModel"]; Assert.Equal(expectedErrorMessage, actualResponse.Message); } @@ -552,7 +552,7 @@ public class DeviceValidatorTests Assert.False(result); Assert.NotNull(context.CustomResponse["ErrorModel"]); - var expectedErrorMessage = "Invalid new device OTP. Try again."; + var expectedErrorMessage = "invalid new device otp"; var actualResponse = (ErrorResponseModel)context.CustomResponse["ErrorModel"]; Assert.Equal(expectedErrorMessage, actualResponse.Message); } @@ -604,7 +604,7 @@ public class DeviceValidatorTests Assert.False(result); Assert.NotNull(context.CustomResponse["ErrorModel"]); - var expectedErrorMessage = "New device verification required."; + var expectedErrorMessage = "new device verification required"; var actualResponse = (ErrorResponseModel)context.CustomResponse["ErrorModel"]; Assert.Equal(expectedErrorMessage, actualResponse.Message); } From d3f8a99fa6d40ea3346560eb7a5259617ed536e4 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Mon, 17 Mar 2025 16:20:51 -0400 Subject: [PATCH 05/18] [PM-18175] Remove flag check for 2FA recovery code login (#5513) * Remove server-side flagging * Linting * Linting. --- .../RequestValidators/TwoFactorAuthenticationValidator.cs | 8 ++------ .../TwoFactorAuthenticationValidatorTests.cs | 5 +---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index 856846cdd6..e733d4f410 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -1,5 +1,4 @@ using System.Text.Json; -using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Identity.TokenProviders; @@ -155,12 +154,9 @@ public class TwoFactorAuthenticationValidator( return false; } - if (_featureService.IsEnabled(FeatureFlagKeys.RecoveryCodeLogin)) + if (type is TwoFactorProviderType.RecoveryCode) { - if (type is TwoFactorProviderType.RecoveryCode) - { - return await _userService.RecoverTwoFactorAsync(user, token); - } + return await _userService.RecoverTwoFactorAsync(user, token); } // These cases we want to always return false, U2f is deprecated and OrganizationDuo diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index e59a66a9e7..fb4d7c321a 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -1,5 +1,4 @@ -using Bit.Core; -using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.Models.Business.Tokenables; @@ -464,7 +463,6 @@ public class TwoFactorAuthenticationValidatorTests user.TwoFactorRecoveryCode = token; _userService.RecoverTwoFactorAsync(Arg.Is(user), Arg.Is(token)).Returns(true); - _featureService.IsEnabled(FeatureFlagKeys.RecoveryCodeLogin).Returns(true); // Act var result = await _sut.VerifyTwoFactorAsync( @@ -486,7 +484,6 @@ public class TwoFactorAuthenticationValidatorTests user.TwoFactorRecoveryCode = token; _userService.RecoverTwoFactorAsync(Arg.Is(user), Arg.Is(token)).Returns(false); - _featureService.IsEnabled(FeatureFlagKeys.RecoveryCodeLogin).Returns(true); // Act var result = await _sut.VerifyTwoFactorAsync( From 43d0f1052b4b0aeca3f57d729a42b6a4f84b9aa9 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 18 Mar 2025 14:04:54 +0100 Subject: [PATCH 06/18] [deps] Tools: Update MailKit to 4.11.0 (#5515) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Core/Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index 8a8de3d77d..2a3edcdc00 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -36,7 +36,7 @@ - + From 87cdb923a5a3bf1b0a23aff4641728505dd9563b Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Tue, 18 Mar 2025 11:44:36 -0400 Subject: [PATCH 07/18] [PM-17901] Replaced hard-coded Bitwarden Vault URLs (#5458) * Replaced hard-coded Bitwarden Vault URLs * Jared's feedback --- ...nizationUserRevokedForSingleOrgPolicy.html.hbs | 2 +- ...nizationUserRevokedForSingleOrgPolicy.text.hbs | 2 +- .../OrganizationSeatsAutoscaled.html.hbs | 2 +- .../OrganizationSeatsMaxReached.html.hbs | 2 +- .../OrganizationSmSeatsMaxReached.html.hbs | 2 +- ...ganizationSmServiceAccountsMaxReached.html.hbs | 2 +- .../Mail/OrganizationSeatsAutoscaledViewModel.cs | 2 +- .../Mail/OrganizationSeatsMaxReachedViewModel.cs | 2 +- ...anizationServiceAccountsMaxReachedViewModel.cs | 2 +- .../Implementations/HandlebarsMailService.cs | 15 +++++++++++---- 10 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.html.hbs b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.html.hbs index d04abe86c9..5b2b1a70c5 100644 --- a/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.html.hbs +++ b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.html.hbs @@ -7,7 +7,7 @@ - To leave an organization, first log into the web app, select the three dot menu next to the organization name, and select Leave. + To leave an organization, first log into the web app, select the three dot menu next to the organization name, and select Leave. diff --git a/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.text.hbs b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.text.hbs index f933e8cf62..6a4b48006b 100644 --- a/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.text.hbs +++ b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.text.hbs @@ -1,5 +1,5 @@ {{#>BasicTextLayout}} Your user account has been revoked from the {{OrganizationName}} organization because your account is part of multiple organizations. Before you can rejoin {{OrganizationName}}, you must first leave all other organizations. -To leave an organization, first log in the web app (https://vault.bitwarden.com/#/login), select the three dot menu next to the organization name, and select Leave. +To leave an organization, first log in the web app ({{{WebVaultUrl}}}/login), select the three dot menu next to the organization name, and select Leave. {{/BasicTextLayout}} diff --git a/src/Core/MailTemplates/Handlebars/OrganizationSeatsAutoscaled.html.hbs b/src/Core/MailTemplates/Handlebars/OrganizationSeatsAutoscaled.html.hbs index 8277e3894a..6bdb982194 100644 --- a/src/Core/MailTemplates/Handlebars/OrganizationSeatsAutoscaled.html.hbs +++ b/src/Core/MailTemplates/Handlebars/OrganizationSeatsAutoscaled.html.hbs @@ -26,7 +26,7 @@ - + Manage subscription
diff --git a/src/Core/MailTemplates/Handlebars/OrganizationSeatsMaxReached.html.hbs b/src/Core/MailTemplates/Handlebars/OrganizationSeatsMaxReached.html.hbs index 6ac2ee74a5..49dbe41c72 100644 --- a/src/Core/MailTemplates/Handlebars/OrganizationSeatsMaxReached.html.hbs +++ b/src/Core/MailTemplates/Handlebars/OrganizationSeatsMaxReached.html.hbs @@ -24,7 +24,7 @@ - + Manage subscription
diff --git a/src/Core/MailTemplates/Handlebars/OrganizationSmSeatsMaxReached.html.hbs b/src/Core/MailTemplates/Handlebars/OrganizationSmSeatsMaxReached.html.hbs index a6db21effc..2ef6707f1f 100644 --- a/src/Core/MailTemplates/Handlebars/OrganizationSmSeatsMaxReached.html.hbs +++ b/src/Core/MailTemplates/Handlebars/OrganizationSmSeatsMaxReached.html.hbs @@ -24,7 +24,7 @@ - + Manage subscription
diff --git a/src/Core/MailTemplates/Handlebars/OrganizationSmServiceAccountsMaxReached.html.hbs b/src/Core/MailTemplates/Handlebars/OrganizationSmServiceAccountsMaxReached.html.hbs index 507fdc33a9..1f4300c23e 100644 --- a/src/Core/MailTemplates/Handlebars/OrganizationSmServiceAccountsMaxReached.html.hbs +++ b/src/Core/MailTemplates/Handlebars/OrganizationSmServiceAccountsMaxReached.html.hbs @@ -24,7 +24,7 @@ - + Manage subscription
diff --git a/src/Core/Models/Mail/OrganizationSeatsAutoscaledViewModel.cs b/src/Core/Models/Mail/OrganizationSeatsAutoscaledViewModel.cs index 87f87b1c69..425b853d3e 100644 --- a/src/Core/Models/Mail/OrganizationSeatsAutoscaledViewModel.cs +++ b/src/Core/Models/Mail/OrganizationSeatsAutoscaledViewModel.cs @@ -2,7 +2,7 @@ public class OrganizationSeatsAutoscaledViewModel : BaseMailModel { - public Guid OrganizationId { get; set; } public int InitialSeatCount { get; set; } public int CurrentSeatCount { get; set; } + public string VaultSubscriptionUrl { get; set; } } diff --git a/src/Core/Models/Mail/OrganizationSeatsMaxReachedViewModel.cs b/src/Core/Models/Mail/OrganizationSeatsMaxReachedViewModel.cs index cdfb57b2dc..ad9c48ab31 100644 --- a/src/Core/Models/Mail/OrganizationSeatsMaxReachedViewModel.cs +++ b/src/Core/Models/Mail/OrganizationSeatsMaxReachedViewModel.cs @@ -2,6 +2,6 @@ public class OrganizationSeatsMaxReachedViewModel : BaseMailModel { - public Guid OrganizationId { get; set; } public int MaxSeatCount { get; set; } + public string VaultSubscriptionUrl { get; set; } } diff --git a/src/Core/Models/Mail/OrganizationServiceAccountsMaxReachedViewModel.cs b/src/Core/Models/Mail/OrganizationServiceAccountsMaxReachedViewModel.cs index 1b9c925720..c814a3e564 100644 --- a/src/Core/Models/Mail/OrganizationServiceAccountsMaxReachedViewModel.cs +++ b/src/Core/Models/Mail/OrganizationServiceAccountsMaxReachedViewModel.cs @@ -2,6 +2,6 @@ public class OrganizationServiceAccountsMaxReachedViewModel { - public Guid OrganizationId { get; set; } public int MaxServiceAccountsCount { get; set; } + public string VaultSubscriptionUrl { get; set; } } diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index c598a9d432..d96f69b0a6 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -214,9 +214,9 @@ public class HandlebarsMailService : IMailService var message = CreateDefaultMessage($"{organization.DisplayName()} Seat Count Has Increased", ownerEmails); var model = new OrganizationSeatsAutoscaledViewModel { - OrganizationId = organization.Id, InitialSeatCount = initialSeatCount, CurrentSeatCount = organization.Seats.Value, + VaultSubscriptionUrl = GetCloudVaultSubscriptionUrl(organization.Id) }; await AddMessageContentAsync(message, "OrganizationSeatsAutoscaled", model); @@ -229,8 +229,8 @@ public class HandlebarsMailService : IMailService var message = CreateDefaultMessage($"{organization.DisplayName()} Seat Limit Reached", ownerEmails); var model = new OrganizationSeatsMaxReachedViewModel { - OrganizationId = organization.Id, MaxSeatCount = maxSeatCount, + VaultSubscriptionUrl = GetCloudVaultSubscriptionUrl(organization.Id) }; await AddMessageContentAsync(message, "OrganizationSeatsMaxReached", model); @@ -1103,8 +1103,8 @@ public class HandlebarsMailService : IMailService var message = CreateDefaultMessage($"{organization.DisplayName()} Secrets Manager Seat Limit Reached", ownerEmails); var model = new OrganizationSeatsMaxReachedViewModel { - OrganizationId = organization.Id, MaxSeatCount = maxSeatCount, + VaultSubscriptionUrl = GetCloudVaultSubscriptionUrl(organization.Id) }; await AddMessageContentAsync(message, "OrganizationSmSeatsMaxReached", model); @@ -1118,8 +1118,8 @@ public class HandlebarsMailService : IMailService var message = CreateDefaultMessage($"{organization.DisplayName()} Secrets Manager Machine Accounts Limit Reached", ownerEmails); var model = new OrganizationServiceAccountsMaxReachedViewModel { - OrganizationId = organization.Id, MaxServiceAccountsCount = maxSeatCount, + VaultSubscriptionUrl = GetCloudVaultSubscriptionUrl(organization.Id) }; await AddMessageContentAsync(message, "OrganizationSmServiceAccountsMaxReached", model); @@ -1223,4 +1223,11 @@ public class HandlebarsMailService : IMailService { return string.IsNullOrEmpty(userName) ? email : CoreHelpers.SanitizeForEmail(userName, false); } + + private string GetCloudVaultSubscriptionUrl(Guid organizationId) + => _globalSettings.BaseServiceUri.CloudRegion?.ToLower() switch + { + "eu" => $"https://vault.bitwarden.eu/#/organizations/{organizationId}/billing/subscription", + _ => $"https://vault.bitwarden.com/#/organizations/{organizationId}/billing/subscription" + }; } From 508bf2c9f846de1d184acd65d6dab1ccea19b9fc Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 18 Mar 2025 14:26:29 -0400 Subject: [PATCH 08/18] [deps] Vault: Update AngleSharp to 1.2.0 (#5220) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Icons/Icons.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Icons/Icons.csproj b/src/Icons/Icons.csproj index 1674e2f877..455c8b3155 100644 --- a/src/Icons/Icons.csproj +++ b/src/Icons/Icons.csproj @@ -7,7 +7,7 @@ - + From 7f0dd6d1c320bb9f8ec336cad11521ae98150fa0 Mon Sep 17 00:00:00 2001 From: Vince Grassia <593223+vgrassia@users.noreply.github.com> Date: Tue, 18 Mar 2025 13:02:39 -0700 Subject: [PATCH 09/18] Update FROM directive in Dockerfile (#5522) --- util/Attachments/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/Attachments/Dockerfile b/util/Attachments/Dockerfile index 2d99aa5911..37b23a1b95 100644 --- a/util/Attachments/Dockerfile +++ b/util/Attachments/Dockerfile @@ -1,4 +1,4 @@ -FROM bitwarden/server:latest +FROM ghcr.io/bitwarden/server LABEL com.bitwarden.product="bitwarden" From bb3ec6aca13b691120b052747f6b4198e639d97d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 19 Mar 2025 11:01:06 +0000 Subject: [PATCH 10/18] [PM-16888] Refactor OrganizationUser status update procedure to use a GuidIdArray parameter and remove JSON parsing logic (#5237) * Refactor OrganizationUser status update procedure to use a GuidIdArray parameter and remove JSON parsing logic * Fix OrganizationUser_SetStatusForUsersById procedure and bump script date * Restore OrganizationUser_SetStatusForUsersById for possible server version rollback. Add new version with the name OrganizationUser_SetStatusForUsersByGuidIdArray * Add migration script to add stored procedure OrganizationUser_SetStatusForUsersByGuidIdArray to update user status by GUID array --- .../Repositories/OrganizationUserRepository.cs | 4 ++-- ...izationUser_SetStatusForUsersByGuidIdArray.sql | 14 ++++++++++++++ ...0_AddOrgUserSetStatusForUsersByGuidIdArray.sql | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersByGuidIdArray.sql create mode 100644 util/Migrator/DbScripts/2025-03-13-00_AddOrgUserSetStatusForUsersByGuidIdArray.sql diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs index 9b77fb216e..07b55aa44a 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -563,8 +563,8 @@ public class OrganizationUserRepository : Repository, IO await using var connection = new SqlConnection(ConnectionString); await connection.ExecuteAsync( - "[dbo].[OrganizationUser_SetStatusForUsersById]", - new { OrganizationUserIds = JsonSerializer.Serialize(organizationUserIds), Status = OrganizationUserStatusType.Revoked }, + "[dbo].[OrganizationUser_SetStatusForUsersByGuidIdArray]", + new { OrganizationUserIds = organizationUserIds.ToGuidIdArrayTVP(), Status = OrganizationUserStatusType.Revoked }, commandType: CommandType.StoredProcedure); } diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersByGuidIdArray.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersByGuidIdArray.sql new file mode 100644 index 0000000000..7843748d72 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersByGuidIdArray.sql @@ -0,0 +1,14 @@ +CREATE PROCEDURE [dbo].[OrganizationUser_SetStatusForUsersByGuidIdArray] + @OrganizationUserIds AS [dbo].[GuidIdArray] READONLY, + @Status SMALLINT +AS +BEGIN + SET NOCOUNT ON + + UPDATE OU + SET OU.[Status] = @Status + FROM [dbo].[OrganizationUser] OU + INNER JOIN @OrganizationUserIds OUI ON OUI.[Id] = OU.[Id] + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIds] @OrganizationUserIds +END diff --git a/util/Migrator/DbScripts/2025-03-13-00_AddOrgUserSetStatusForUsersByGuidIdArray.sql b/util/Migrator/DbScripts/2025-03-13-00_AddOrgUserSetStatusForUsersByGuidIdArray.sql new file mode 100644 index 0000000000..e7c0477710 --- /dev/null +++ b/util/Migrator/DbScripts/2025-03-13-00_AddOrgUserSetStatusForUsersByGuidIdArray.sql @@ -0,0 +1,15 @@ +CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_SetStatusForUsersByGuidIdArray] + @OrganizationUserIds AS [dbo].[GuidIdArray] READONLY, + @Status SMALLINT +AS +BEGIN + SET NOCOUNT ON + + UPDATE OU + SET OU.[Status] = @Status + FROM [dbo].[OrganizationUser] OU + INNER JOIN @OrganizationUserIds OUI ON OUI.[Id] = OU.[Id] + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIds] @OrganizationUserIds +END +GO From fc827ed2097ca601c8933760993ce602373c3789 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Wed, 19 Mar 2025 13:49:02 -0400 Subject: [PATCH 11/18] feat(set password) [PM-17647] Add set/change password feature flags * Added flag values * Added flag values * Removed extra space * Linting --- src/Core/Constants.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index e09422871d..fb3757daab 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -177,6 +177,8 @@ public static class FeatureFlagKeys public const string PM12276Breadcrumbing = "pm-12276-breadcrumbing-for-business-features"; public const string PM18794_ProviderPaymentMethod = "pm-18794-provider-payment-method"; public const string PM3553_MobileSimpleLoginSelfHostAlias = "simple-login-self-host-alias"; + public const string SetInitialPasswordRefactor = "pm-16117-set-initial-password-refactor"; + public const string ChangeExistingPasswordRefactor = "pm-16117-change-existing-password-refactor"; public static List GetAllKeys() { From 21717ec71eb59057f3ac71dcce4c467f17d92bbc Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Wed, 19 Mar 2025 11:13:38 -0700 Subject: [PATCH 12/18] [PM-17733] - [Privilege Escalation] - Unauthorised access allows limited access user to change password of Items (#5452) * prevent view-only users from updating passwords * revert change to licensing service * add tests * check if organizationId is there * move logic to private method * move logic to private method * move logic into method * revert change to licensing service * throw exception when cipher key is created by hidden password users * fix tests * don't allow totp or passkeys changes from hidden password users * add tests * revert change to licensing service --- .../Services/Implementations/CipherService.cs | 36 ++- .../Vault/Services/CipherServiceTests.cs | 232 +++++++++++++++++- 2 files changed, 266 insertions(+), 2 deletions(-) diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index 90c03df90b..a315528e59 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -13,7 +13,9 @@ using Bit.Core.Tools.Models.Business; using Bit.Core.Tools.Services; using Bit.Core.Utilities; using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Enums; using Bit.Core.Vault.Models.Data; +using Bit.Core.Vault.Queries; using Bit.Core.Vault.Repositories; namespace Bit.Core.Vault.Services; @@ -38,6 +40,7 @@ public class CipherService : ICipherService private const long _fileSizeLeeway = 1024L * 1024L; // 1MB private readonly IReferenceEventService _referenceEventService; private readonly ICurrentContext _currentContext; + private readonly IGetCipherPermissionsForUserQuery _getCipherPermissionsForUserQuery; public CipherService( ICipherRepository cipherRepository, @@ -54,7 +57,8 @@ public class CipherService : ICipherService IPolicyService policyService, GlobalSettings globalSettings, IReferenceEventService referenceEventService, - ICurrentContext currentContext) + ICurrentContext currentContext, + IGetCipherPermissionsForUserQuery getCipherPermissionsForUserQuery) { _cipherRepository = cipherRepository; _folderRepository = folderRepository; @@ -71,6 +75,7 @@ public class CipherService : ICipherService _globalSettings = globalSettings; _referenceEventService = referenceEventService; _currentContext = currentContext; + _getCipherPermissionsForUserQuery = getCipherPermissionsForUserQuery; } public async Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnownRevisionDate, @@ -161,6 +166,7 @@ public class CipherService : ICipherService { ValidateCipherLastKnownRevisionDateAsync(cipher, lastKnownRevisionDate); cipher.RevisionDate = DateTime.UtcNow; + await ValidateViewPasswordUserAsync(cipher); await _cipherRepository.ReplaceAsync(cipher); await _eventService.LogCipherEventAsync(cipher, Bit.Core.Enums.EventType.Cipher_Updated); @@ -966,4 +972,32 @@ public class CipherService : ICipherService ValidateCipherLastKnownRevisionDateAsync(cipher, lastKnownRevisionDate); } + + private async Task ValidateViewPasswordUserAsync(Cipher cipher) + { + if (cipher.Type != CipherType.Login || cipher.Data == null || !cipher.OrganizationId.HasValue) + { + return; + } + var existingCipher = await _cipherRepository.GetByIdAsync(cipher.Id); + if (existingCipher == null) return; + + var cipherPermissions = await _getCipherPermissionsForUserQuery.GetByOrganization(cipher.OrganizationId.Value); + // Check if user is a "hidden password" user + if (!cipherPermissions.TryGetValue(cipher.Id, out var permission) || !(permission.ViewPassword && permission.Edit)) + { + // "hidden password" users may not add cipher key encryption + if (existingCipher.Key == null && cipher.Key != null) + { + throw new BadRequestException("You do not have permission to add cipher key encryption."); + } + // "hidden password" users may not change passwords, TOTP codes, or passkeys, so we need to set them back to the original values + var existingCipherData = JsonSerializer.Deserialize(existingCipher.Data); + var newCipherData = JsonSerializer.Deserialize(cipher.Data); + newCipherData.Fido2Credentials = existingCipherData.Fido2Credentials; + newCipherData.Totp = existingCipherData.Totp; + newCipherData.Password = existingCipherData.Password; + cipher.Data = JsonSerializer.Serialize(newCipherData); + } + } } diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index 1803c980c2..3ef29146c2 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -1,4 +1,5 @@ -using Bit.Core.AdminConsole.Entities; +using System.Text.Json; +using Bit.Core.AdminConsole.Entities; using Bit.Core.Billing.Enums; using Bit.Core.Entities; using Bit.Core.Enums; @@ -9,7 +10,9 @@ using Bit.Core.Services; using Bit.Core.Test.AutoFixture.CipherFixtures; using Bit.Core.Utilities; using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Enums; using Bit.Core.Vault.Models.Data; +using Bit.Core.Vault.Queries; using Bit.Core.Vault.Repositories; using Bit.Core.Vault.Services; using Bit.Test.Common.AutoFixture; @@ -797,6 +800,233 @@ public class CipherServiceTests Arg.Is>(arg => !arg.Except(ciphers).Any())); } + private class SaveDetailsAsyncDependencies + { + public CipherDetails CipherDetails { get; set; } + public SutProvider SutProvider { get; set; } + } + + private static SaveDetailsAsyncDependencies GetSaveDetailsAsyncDependencies( + SutProvider sutProvider, + string newPassword, + bool viewPassword, + bool editPermission, + string? key = null, + string? totp = null, + CipherLoginFido2CredentialData[]? passkeys = null + ) + { + var cipherDetails = new CipherDetails + { + Id = Guid.NewGuid(), + OrganizationId = Guid.NewGuid(), + Type = CipherType.Login, + UserId = Guid.NewGuid(), + RevisionDate = DateTime.UtcNow, + Key = key, + }; + + var newLoginData = new CipherLoginData { Username = "user", Password = newPassword, Totp = totp, Fido2Credentials = passkeys }; + cipherDetails.Data = JsonSerializer.Serialize(newLoginData); + + var existingCipher = new Cipher + { + Id = cipherDetails.Id, + Data = JsonSerializer.Serialize( + new CipherLoginData + { + Username = "user", + Password = "OriginalPassword", + Totp = "OriginalTotp", + Fido2Credentials = [] + } + ), + }; + + sutProvider.GetDependency() + .GetByIdAsync(cipherDetails.Id) + .Returns(existingCipher); + + sutProvider.GetDependency() + .ReplaceAsync(Arg.Any()) + .Returns(Task.CompletedTask); + + var permissions = new Dictionary + { + { cipherDetails.Id, new OrganizationCipherPermission { ViewPassword = viewPassword, Edit = editPermission } } + }; + + sutProvider.GetDependency() + .GetByOrganization(cipherDetails.OrganizationId.Value) + .Returns(permissions); + + return new SaveDetailsAsyncDependencies + { + CipherDetails = cipherDetails, + SutProvider = sutProvider, + }; + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_PasswordNotChangedWithoutViewPasswordPermission(string _, SutProvider sutProvider) + { + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: false, editPermission: true); + + await deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true); + + var updatedLoginData = JsonSerializer.Deserialize(deps.CipherDetails.Data); + Assert.Equal("OriginalPassword", updatedLoginData.Password); + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_PasswordNotChangedWithoutEditPermission(string _, SutProvider sutProvider) + { + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: false); + + await deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true); + + var updatedLoginData = JsonSerializer.Deserialize(deps.CipherDetails.Data); + Assert.Equal("OriginalPassword", updatedLoginData.Password); + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_PasswordChangedWithPermission(string _, SutProvider sutProvider) + { + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: true); + + await deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true); + + var updatedLoginData = JsonSerializer.Deserialize(deps.CipherDetails.Data); + Assert.Equal("NewPassword", updatedLoginData.Password); + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_CipherKeyChangedWithPermission(string _, SutProvider sutProvider) + { + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: true, "NewKey"); + + await deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true); + + Assert.Equal("NewKey", deps.CipherDetails.Key); + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_CipherKeyChangedWithoutPermission(string _, SutProvider sutProvider) + { + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: false, "NewKey"); + + var exception = await Assert.ThrowsAsync(() => deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true)); + + Assert.Contains("do not have permission", exception.Message); + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_TotpChangedWithoutPermission(string _, SutProvider sutProvider) + { + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: false, totp: "NewTotp"); + + await deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true); + + var updatedLoginData = JsonSerializer.Deserialize(deps.CipherDetails.Data); + Assert.Equal("OriginalTotp", updatedLoginData.Totp); + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_TotpChangedWithPermission(string _, SutProvider sutProvider) + { + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: true, totp: "NewTotp"); + + await deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true); + + var updatedLoginData = JsonSerializer.Deserialize(deps.CipherDetails.Data); + Assert.Equal("NewTotp", updatedLoginData.Totp); + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_Fido2CredentialsChangedWithoutPermission(string _, SutProvider sutProvider) + { + var passkeys = new[] + { + new CipherLoginFido2CredentialData + { + CredentialId = "CredentialId", + UserHandle = "UserHandle", + } + }; + + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: false, passkeys: passkeys); + + await deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true); + + var updatedLoginData = JsonSerializer.Deserialize(deps.CipherDetails.Data); + Assert.Empty(updatedLoginData.Fido2Credentials); + } + + [Theory, BitAutoData] + public async Task SaveDetailsAsync_Fido2CredentialsChangedWithPermission(string _, SutProvider sutProvider) + { + var passkeys = new[] + { + new CipherLoginFido2CredentialData + { + CredentialId = "CredentialId", + UserHandle = "UserHandle", + } + }; + + var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: true, passkeys: passkeys); + + await deps.SutProvider.Sut.SaveDetailsAsync( + deps.CipherDetails, + deps.CipherDetails.UserId.Value, + deps.CipherDetails.RevisionDate, + null, + true); + + var updatedLoginData = JsonSerializer.Deserialize(deps.CipherDetails.Data); + Assert.Equal(passkeys.Length, updatedLoginData.Fido2Credentials.Length); + } + [Theory] [BitAutoData] public async Task DeleteAsync_WithPersonalCipherOwner_DeletesCipher( From 481df89cf0991c02dc9ddd194de83c92cf72db14 Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Wed, 19 Mar 2025 14:24:12 -0400 Subject: [PATCH 13/18] [PM-19342] Onboarding Nudges Feature Flag (#5530) --- src/Core/Constants.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index fb3757daab..970bcb4082 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -118,6 +118,7 @@ public static class FeatureFlagKeys public const string ExportAttachments = "export-attachments"; /* Vault Team */ + public const string PM8851_BrowserOnboardingNudge = "pm-8851-browser-onboarding-nudge"; public const string PM9111ExtensionPersistAddEditForm = "pm-9111-extension-persist-add-edit-form"; public const string NewDeviceVerificationPermanentDismiss = "new-device-permanent-dismiss"; public const string NewDeviceVerificationTemporaryDismiss = "new-device-temporary-dismiss"; From 3422f4cd5033c751a0479ed91e5b15820dfdd997 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Wed, 19 Mar 2025 13:55:30 -0500 Subject: [PATCH 14/18] [PM-18971] Special Characters in Org Names (#5514) * sanitize organization name for email to avoid encoding * fix spelling mistake in variable name --- src/Core/Services/IMailService.cs | 2 +- .../Services/Implementations/HandlebarsMailService.cs | 9 +++++---- src/Core/Services/NoopImplementations/NoopMailService.cs | 2 +- .../Vault/Commands/CreateManyTaskNotificationsCommand.cs | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Core/Services/IMailService.cs b/src/Core/Services/IMailService.cs index b0b884eb3e..04b302bad9 100644 --- a/src/Core/Services/IMailService.cs +++ b/src/Core/Services/IMailService.cs @@ -99,5 +99,5 @@ public interface IMailService string organizationName); Task SendClaimedDomainUserEmailAsync(ManagedUserDomainClaimedEmails emailList); Task SendDeviceApprovalRequestedNotificationEmailAsync(IEnumerable adminEmails, Guid organizationId, string email, string userName); - Task SendBulkSecurityTaskNotificationsAsync(string orgName, IEnumerable securityTaskNotificaitons); + Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications); } diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index d96f69b0a6..588365f8c9 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -1201,21 +1201,22 @@ public class HandlebarsMailService : IMailService await _mailDeliveryService.SendEmailAsync(message); } - public async Task SendBulkSecurityTaskNotificationsAsync(string orgName, IEnumerable securityTaskNotificaitons) + public async Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications) { MailQueueMessage CreateMessage(UserSecurityTasksCount notification) { - var message = CreateDefaultMessage($"{orgName} has identified {notification.TaskCount} at-risk password{(notification.TaskCount.Equals(1) ? "" : "s")}", notification.Email); + var sanitizedOrgName = CoreHelpers.SanitizeForEmail(org.DisplayName(), false); + var message = CreateDefaultMessage($"{sanitizedOrgName} has identified {notification.TaskCount} at-risk password{(notification.TaskCount.Equals(1) ? "" : "s")}", notification.Email); var model = new SecurityTaskNotificationViewModel { - OrgName = orgName, + OrgName = CoreHelpers.SanitizeForEmail(sanitizedOrgName, false), TaskCount = notification.TaskCount, WebVaultUrl = _globalSettings.BaseServiceUri.VaultWithHash, }; message.Category = "SecurityTasksNotification"; return new MailQueueMessage(message, "SecurityTasksNotification", model); } - var messageModels = securityTaskNotificaitons.Select(CreateMessage); + var messageModels = securityTaskNotifications.Select(CreateMessage); await EnqueueMailAsync(messageModels.ToList()); } diff --git a/src/Core/Services/NoopImplementations/NoopMailService.cs b/src/Core/Services/NoopImplementations/NoopMailService.cs index 5fba545903..776dd07f19 100644 --- a/src/Core/Services/NoopImplementations/NoopMailService.cs +++ b/src/Core/Services/NoopImplementations/NoopMailService.cs @@ -324,7 +324,7 @@ public class NoopMailService : IMailService return Task.FromResult(0); } - public Task SendBulkSecurityTaskNotificationsAsync(string orgName, IEnumerable securityTaskNotificaitons) + public Task SendBulkSecurityTaskNotificationsAsync(Organization org, IEnumerable securityTaskNotifications) { return Task.FromResult(0); } diff --git a/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs b/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs index 58b5f65e0f..f939816301 100644 --- a/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs +++ b/src/Core/Vault/Commands/CreateManyTaskNotificationsCommand.cs @@ -46,7 +46,7 @@ public class CreateManyTaskNotificationsCommand : ICreateManyTaskNotificationsCo var organization = await _organizationRepository.GetByIdAsync(orgId); - await _mailService.SendBulkSecurityTaskNotificationsAsync(organization.Name, userTaskCount); + await _mailService.SendBulkSecurityTaskNotificationsAsync(organization, userTaskCount); // Break securityTaskCiphers into separate lists by user Id var securityTaskCiphersByUser = securityTaskCiphers.GroupBy(x => x.UserId) From db3151160a06daf0dac896d061e4dca2b982682c Mon Sep 17 00:00:00 2001 From: Patrick-Pimentel-Bitwarden Date: Wed, 19 Mar 2025 15:27:51 -0400 Subject: [PATCH 15/18] fix(device-approval-persistence): [PM-9112] Device Approval Persistence - Added feature flag. (#5495) --- src/Core/Constants.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 970bcb4082..0ae9f1d8d7 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -126,6 +126,9 @@ public static class FeatureFlagKeys public const string RestrictProviderAccess = "restrict-provider-access"; public const string SecurityTasks = "security-tasks"; + /* Auth Team */ + public const string PM9112DeviceApprovalPersistence = "pm-9112-device-approval-persistence"; + public const string ReturnErrorOnExistingKeypair = "return-error-on-existing-keypair"; public const string UseTreeWalkerApiForPageDetailsCollection = "use-tree-walker-api-for-page-details-collection"; public const string DuoRedirect = "duo-redirect"; From f6cc140fded360256429b24a378cbcd969e7d50a Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Thu, 20 Mar 2025 09:12:39 -0500 Subject: [PATCH 16/18] Switched from .Any to Count. Remove unreachable code. (#5519) --- src/Api/Vault/Controllers/CiphersController.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 62f07005ee..daaf8a03fb 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -127,9 +127,9 @@ public class CiphersController : Controller public async Task> Get() { var user = await _userService.GetUserByPrincipalAsync(User); - var hasOrgs = _currentContext.Organizations?.Any() ?? false; + var hasOrgs = _currentContext.Organizations.Count != 0; // TODO: Use hasOrgs proper for cipher listing here? - var ciphers = await _cipherRepository.GetManyByUserIdAsync(user.Id, withOrganizations: true || hasOrgs); + var ciphers = await _cipherRepository.GetManyByUserIdAsync(user.Id, withOrganizations: true); Dictionary> collectionCiphersGroupDict = null; if (hasOrgs) { From bb674b8990499f0c6680fb5efb009c9316a99218 Mon Sep 17 00:00:00 2001 From: Github Actions Date: Thu, 20 Mar 2025 17:14:35 +0000 Subject: [PATCH 17/18] Bumped version to 2025.3.1 --- Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index a994b2196e..082f85906d 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -3,7 +3,7 @@ net8.0 - 2025.3.0 + 2025.3.1 Bit.$(MSBuildProjectName) enable From 2d02ad3f61b939d08bc901e510007d8ed9ce2fe1 Mon Sep 17 00:00:00 2001 From: Github Actions Date: Thu, 20 Mar 2025 17:30:55 +0000 Subject: [PATCH 18/18] Bumped version to 2025.3.2 --- Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index 082f85906d..cbe9786d65 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -3,7 +3,7 @@ net8.0 - 2025.3.1 + 2025.3.2 Bit.$(MSBuildProjectName) enable