From 6d4129c6b7db3c672207d2ba1304297865cfe712 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Fri, 5 Sep 2025 10:36:01 -0400 Subject: [PATCH 1/7] [PM-20595] Add Policy for Send access (#6282) * feat: add policy to API startup and Policies class to hold the static strings * test: add snapshot testing for constants to help with rust mappings * doc: add docs for send access --- src/Api/Startup.cs | 7 ++ src/Core/Auth/Identity/Policies.cs | 10 +++ .../SendAccess/SendAccessConstants.cs | 4 +- .../SendAccess/SendAccessGrantValidator.cs | 6 +- .../RequestValidators/SendAccess/readme.md | 66 +++++++++++++++++ .../SendAccess/SendConstantsSnapshotTests.cs | 73 +++++++++++++++++++ 6 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 src/Core/Auth/Identity/Policies.cs create mode 100644 src/Identity/IdentityServer/RequestValidators/SendAccess/readme.md create mode 100644 test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs index 2d306c4435..1d5a1609f4 100644 --- a/src/Api/Startup.cs +++ b/src/Api/Startup.cs @@ -33,6 +33,7 @@ using Bit.Core.Auth.Models.Api.Request; using Bit.Core.Dirt.Reports.ReportFeatures; using Bit.Core.Tools.SendFeatures; using Bit.Core.Auth.IdentityServer; +using Bit.Core.Auth.Identity; #if !OSS @@ -145,6 +146,12 @@ public class Startup (c.Value.Contains(ApiScopes.Api) || c.Value.Contains(ApiScopes.ApiSecrets)) )); }); + config.AddPolicy(Policies.Send, configurePolicy: policy => + { + policy.RequireAuthenticatedUser(); + policy.RequireClaim(JwtClaimTypes.Scope, ApiScopes.ApiSendAccess); + policy.RequireClaim(Claims.SendAccessClaims.SendId); + }); }); services.AddScoped(); diff --git a/src/Core/Auth/Identity/Policies.cs b/src/Core/Auth/Identity/Policies.cs new file mode 100644 index 0000000000..78d86d06a4 --- /dev/null +++ b/src/Core/Auth/Identity/Policies.cs @@ -0,0 +1,10 @@ +namespace Bit.Core.Auth.Identity; + +public static class Policies +{ + /// + /// Policy for managing access to the Send feature. + /// + public const string Send = "Send"; // [Authorize(Policy = Policies.Send)] + // TODO: migrate other existing policies to use this class +} diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs index fae7ba4215..17ec387411 100644 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs @@ -5,6 +5,8 @@ namespace Bit.Identity.IdentityServer.RequestValidators.SendAccess; /// /// String constants for the Send Access user feature +/// Most of these need to be synced with the `bitwarden-auth` crate in the SDK. +/// There is snapshot testing to help ensure this. /// public static class SendAccessConstants { @@ -41,7 +43,7 @@ public static class SendAccessConstants /// /// The sendId is missing from the request. /// - public const string MissingSendId = "send_id_required"; + public const string SendIdRequired = "send_id_required"; /// /// The sendId is invalid, does not match a known send. /// diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessGrantValidator.cs index 2ecc5a9704..d9ae946d16 100644 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessGrantValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessGrantValidator.cs @@ -23,7 +23,7 @@ public class SendAccessGrantValidator( private static readonly Dictionary _sendGrantValidatorErrorDescriptions = new() { - { SendAccessConstants.GrantValidatorResults.MissingSendId, $"{SendAccessConstants.TokenRequest.SendId} is required." }, + { SendAccessConstants.GrantValidatorResults.SendIdRequired, $"{SendAccessConstants.TokenRequest.SendId} is required." }, { SendAccessConstants.GrantValidatorResults.InvalidSendId, $"{SendAccessConstants.TokenRequest.SendId} is invalid." } }; @@ -90,7 +90,7 @@ public class SendAccessGrantValidator( // if the sendId is null then the request is the wrong shape and the request is invalid if (sendId == null) { - return (Guid.Empty, SendAccessConstants.GrantValidatorResults.MissingSendId); + return (Guid.Empty, SendAccessConstants.GrantValidatorResults.SendIdRequired); } // the send_id is not null so the request is the correct shape, so we will attempt to parse it try @@ -125,7 +125,7 @@ public class SendAccessGrantValidator( return error switch { // Request is the wrong shape - SendAccessConstants.GrantValidatorResults.MissingSendId => new GrantValidationResult( + SendAccessConstants.GrantValidatorResults.SendIdRequired => new GrantValidationResult( TokenRequestErrors.InvalidRequest, errorDescription: _sendGrantValidatorErrorDescriptions[error], customResponse), diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/readme.md b/src/Identity/IdentityServer/RequestValidators/SendAccess/readme.md new file mode 100644 index 0000000000..afab13a156 --- /dev/null +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/readme.md @@ -0,0 +1,66 @@ +Send Access Request Validation +=== + +This feature supports the ability of Tools to require specific claims for access to sends. + +In order to access Send data a user must meet the requirements laid out in these request validators. + +# ***Important: String Constants*** + +The string constants contained herein are used in conjunction with the Auth module in the SDK. Any change to these string values _must_ be intentional and _must_ have a corresponding change in the SDK. + +There is snapshot testing that will fail if the strings change to help detect unintended changes to the string constants. + +# Custom Claims + +Send access tokens contain custom claims specific to the Send the Send grant type. + +1. `send_id` - is always included in the issued access token. This is the `GUID` of the request Send. +1. `send_email` - only set when the Send requires `EmailOtp` authentication type. +1. `type` - this will always be `Send` + +# Authentication methods + +## `NeverAuthenticate` + +For a Send to be in this state two things can be true: +1. The Send has been modified and no longer allows access. +2. The Send does not exist. + +## `NotAuthenticated` + +In this scenario the Send is not protected by any added authentication or authorization and the access token is issued to the requesting user. + +## `ResourcePassword` + +In this scenario the Send is password protected and a user must supply the correct password hash to be issued an access token. + +## `EmailOtp` + +In this scenario the Send is only accessible to owners of specific email addresses. The user must submit a correct email. Once the email has been entered then ownership of the email must be established via OTP. The Otp is sent to the aforementioned email and must be supplied, along with the email, to be issued an access token. + +# Send Access Request Validation + +## Required Parameters + +### All Requests +- `send_id` - Base64 URL-encoded GUID of the send being accessed + +### Password Protected Sends +- `password_hash_b64` - client hashed Base64-encoded password. + +### Email OTP Protected Sends +- `email` - Email address associated with the send +- `otp` - One-time password (optional - if missing, OTP is generated and sent) + +## Error Responses + +All errors include a custom response field: +```json +{ + "error": "invalid_request|invalid_grant", + "error_description": "Human readable description", + "send_access_error_type": "specific_error_code" +} +``` + diff --git a/test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs b/test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs new file mode 100644 index 0000000000..95a0a6675b --- /dev/null +++ b/test/Identity.Test/IdentityServer/SendAccess/SendConstantsSnapshotTests.cs @@ -0,0 +1,73 @@ +using Bit.Identity.IdentityServer.RequestValidators.SendAccess; +using Xunit; + +namespace Bit.Identity.Test.IdentityServer.SendAccess; + +/// +/// Snapshot tests to ensure the string constants in do not change unintentionally. +/// If you change any of these values, please ensure you understand the impact and update the SDK accordingly. +/// If you intentionally change any of these values, please update the tests to reflect the new expected values. +/// +public class SendConstantsSnapshotTests +{ + [Fact] + public void SendAccessError_Constant_HasCorrectValue() + { + // Assert + Assert.Equal("send_access_error_type", SendAccessConstants.SendAccessError); + } + + [Fact] + public void TokenRequest_Constants_HaveCorrectValues() + { + // Assert + Assert.Equal("send_id", SendAccessConstants.TokenRequest.SendId); + Assert.Equal("password_hash_b64", SendAccessConstants.TokenRequest.ClientB64HashedPassword); + Assert.Equal("email", SendAccessConstants.TokenRequest.Email); + Assert.Equal("otp", SendAccessConstants.TokenRequest.Otp); + } + + [Fact] + public void GrantValidatorResults_Constants_HaveCorrectValues() + { + // Assert + Assert.Equal("valid_send_guid", SendAccessConstants.GrantValidatorResults.ValidSendGuid); + Assert.Equal("send_id_required", SendAccessConstants.GrantValidatorResults.SendIdRequired); + Assert.Equal("send_id_invalid", SendAccessConstants.GrantValidatorResults.InvalidSendId); + } + + [Fact] + public void PasswordValidatorResults_Constants_HaveCorrectValues() + { + // Assert + Assert.Equal("password_hash_b64_invalid", SendAccessConstants.PasswordValidatorResults.RequestPasswordDoesNotMatch); + Assert.Equal("password_hash_b64_required", SendAccessConstants.PasswordValidatorResults.RequestPasswordIsRequired); + } + + [Fact] + public void EmailOtpValidatorResults_Constants_HaveCorrectValues() + { + // Assert + Assert.Equal("email_invalid", SendAccessConstants.EmailOtpValidatorResults.EmailInvalid); + Assert.Equal("email_required", SendAccessConstants.EmailOtpValidatorResults.EmailRequired); + Assert.Equal("email_and_otp_required_otp_sent", SendAccessConstants.EmailOtpValidatorResults.EmailOtpSent); + Assert.Equal("otp_invalid", SendAccessConstants.EmailOtpValidatorResults.EmailOtpInvalid); + Assert.Equal("otp_generation_failed", SendAccessConstants.EmailOtpValidatorResults.OtpGenerationFailed); + } + + [Fact] + public void OtpToken_Constants_HaveCorrectValues() + { + // Assert + Assert.Equal("send_access", SendAccessConstants.OtpToken.TokenProviderName); + Assert.Equal("email_otp", SendAccessConstants.OtpToken.Purpose); + Assert.Equal("{0}_{1}", SendAccessConstants.OtpToken.TokenUniqueIdentifier); + } + + [Fact] + public void OtpEmail_Constants_HaveCorrectValues() + { + // Assert + Assert.Equal("Your Bitwarden Send verification code is {0}", SendAccessConstants.OtpEmail.Subject); + } +} From 87bc9299e6fb4c95e420d74f2af9eb4e46400ac0 Mon Sep 17 00:00:00 2001 From: Stephon Brown Date: Fri, 5 Sep 2025 11:15:01 -0400 Subject: [PATCH 2/7] [PM-23309] Admin Console Credit is not Displaying Decimals (#6280) * fix: update calculation to be decimal * fix: update record type property to decimal * tests: add tests to service and update test names --- .../Models/Responses/PaymentMethodResponse.cs | 2 +- src/Core/Billing/Models/PaymentMethod.cs | 2 +- .../Implementations/SubscriberService.cs | 2 +- .../Services/SubscriberServiceTests.cs | 225 ++++++++++++++---- 4 files changed, 183 insertions(+), 48 deletions(-) diff --git a/src/Api/Billing/Models/Responses/PaymentMethodResponse.cs b/src/Api/Billing/Models/Responses/PaymentMethodResponse.cs index fd248a0a00..a54ac0a876 100644 --- a/src/Api/Billing/Models/Responses/PaymentMethodResponse.cs +++ b/src/Api/Billing/Models/Responses/PaymentMethodResponse.cs @@ -4,7 +4,7 @@ using Bit.Core.Billing.Tax.Models; namespace Bit.Api.Billing.Models.Responses; public record PaymentMethodResponse( - long AccountCredit, + decimal AccountCredit, PaymentSource PaymentSource, string SubscriptionStatus, TaxInformation TaxInformation) diff --git a/src/Core/Billing/Models/PaymentMethod.cs b/src/Core/Billing/Models/PaymentMethod.cs index 14ee79b714..10eab97a8f 100644 --- a/src/Core/Billing/Models/PaymentMethod.cs +++ b/src/Core/Billing/Models/PaymentMethod.cs @@ -6,7 +6,7 @@ using Bit.Core.Billing.Tax.Models; namespace Bit.Core.Billing.Models; public record PaymentMethod( - long AccountCredit, + decimal AccountCredit, PaymentSource PaymentSource, string SubscriptionStatus, TaxInformation TaxInformation) diff --git a/src/Core/Billing/Services/Implementations/SubscriberService.cs b/src/Core/Billing/Services/Implementations/SubscriberService.cs index 84d41f829c..378e84f15a 100644 --- a/src/Core/Billing/Services/Implementations/SubscriberService.cs +++ b/src/Core/Billing/Services/Implementations/SubscriberService.cs @@ -345,7 +345,7 @@ public class SubscriberService( return PaymentMethod.Empty; } - var accountCredit = customer.Balance * -1 / 100; + var accountCredit = customer.Balance * -1 / 100M; var paymentMethod = await GetPaymentSourceAsync(subscriber.Id, customer); diff --git a/test/Core.Test/Billing/Services/SubscriberServiceTests.cs b/test/Core.Test/Billing/Services/SubscriberServiceTests.cs index 0df8d1bfcc..600f9d9be2 100644 --- a/test/Core.Test/Billing/Services/SubscriberServiceTests.cs +++ b/test/Core.Test/Billing/Services/SubscriberServiceTests.cs @@ -329,13 +329,165 @@ public class SubscriberServiceTests #endregion #region GetPaymentMethod + [Theory, BitAutoData] public async Task GetPaymentMethod_NullSubscriber_ThrowsArgumentNullException( SutProvider sutProvider) => await Assert.ThrowsAsync(() => sutProvider.Sut.GetPaymentSource(null)); [Theory, BitAutoData] - public async Task GetPaymentMethod_Braintree_NoDefaultPaymentMethod_ReturnsNull( + public async Task GetPaymentMethod_WithNegativeStripeAccountBalance_ReturnsCorrectAccountCreditAmount(Organization organization, + SutProvider sutProvider) + { + // Arrange + // Stripe reports balance in cents as a negative number for credit + const int stripeAccountBalance = -593; // $5.93 credit (negative cents) + const decimal creditAmount = 5.93M; // Same value in dollars + + + var customer = new Customer + { + Balance = stripeAccountBalance, + Subscriptions = new StripeList() + { + Data = + [new Subscription { Id = organization.GatewaySubscriptionId, Status = "active" }] + }, + InvoiceSettings = new CustomerInvoiceSettings + { + DefaultPaymentMethod = new PaymentMethod + { + Type = StripeConstants.PaymentMethodTypes.USBankAccount, + UsBankAccount = new PaymentMethodUsBankAccount { BankName = "Chase", Last4 = "9999" } + } + } + }; + sutProvider.GetDependency().CustomerGetAsync(organization.GatewayCustomerId, + Arg.Is(options => options.Expand.Contains("default_source") && + options.Expand.Contains("invoice_settings.default_payment_method") + && options.Expand.Contains("subscriptions") + && options.Expand.Contains("tax_ids"))) + .Returns(customer); + + // Act + var result = await sutProvider.Sut.GetPaymentMethod(organization); + + // Assert + Assert.NotNull(result); + Assert.Equal(creditAmount, result.AccountCredit); + await sutProvider.GetDependency().Received(1).CustomerGetAsync( + organization.GatewayCustomerId, + Arg.Is(options => + options.Expand.Contains("default_source") && + options.Expand.Contains("invoice_settings.default_payment_method") && + options.Expand.Contains("subscriptions") && + options.Expand.Contains("tax_ids"))); + + } + + [Theory, BitAutoData] + public async Task GetPaymentMethod_WithZeroStripeAccountBalance_ReturnsCorrectAccountCreditAmount( + Organization organization, SutProvider sutProvider) + { + // Arrange + const int stripeAccountBalance = 0; + + var customer = new Customer + { + Balance = stripeAccountBalance, + Subscriptions = new StripeList() + { + Data = + [new Subscription { Id = organization.GatewaySubscriptionId, Status = "active" }] + }, + InvoiceSettings = new CustomerInvoiceSettings + { + DefaultPaymentMethod = new PaymentMethod + { + Type = StripeConstants.PaymentMethodTypes.USBankAccount, + UsBankAccount = new PaymentMethodUsBankAccount { BankName = "Chase", Last4 = "9999" } + } + } + }; + sutProvider.GetDependency().CustomerGetAsync(organization.GatewayCustomerId, + Arg.Is(options => options.Expand.Contains("default_source") && + options.Expand.Contains("invoice_settings.default_payment_method") + && options.Expand.Contains("subscriptions") + && options.Expand.Contains("tax_ids"))) + .Returns(customer); + + // Act + var result = await sutProvider.Sut.GetPaymentMethod(organization); + + // Assert + Assert.NotNull(result); + Assert.Equal(0, result.AccountCredit); + await sutProvider.GetDependency().Received(1).CustomerGetAsync( + organization.GatewayCustomerId, + Arg.Is(options => + options.Expand.Contains("default_source") && + options.Expand.Contains("invoice_settings.default_payment_method") && + options.Expand.Contains("subscriptions") && + options.Expand.Contains("tax_ids"))); + } + + [Theory, BitAutoData] + public async Task GetPaymentMethod_WithPositiveStripeAccountBalance_ReturnsCorrectAccountCreditAmount( + Organization organization, SutProvider sutProvider) + { + // Arrange + const int stripeAccountBalance = 593; // $5.93 charge balance + const decimal accountBalance = -5.93M; // account balance + var customer = new Customer + { + Balance = stripeAccountBalance, + Subscriptions = new StripeList() + { + Data = + [new Subscription { Id = organization.GatewaySubscriptionId, Status = "active" }] + }, + InvoiceSettings = new CustomerInvoiceSettings + { + DefaultPaymentMethod = new PaymentMethod + { + Type = StripeConstants.PaymentMethodTypes.USBankAccount, + UsBankAccount = new PaymentMethodUsBankAccount { BankName = "Chase", Last4 = "9999" } + } + } + }; + sutProvider.GetDependency().CustomerGetAsync(organization.GatewayCustomerId, + Arg.Is(options => options.Expand.Contains("default_source") && + options.Expand.Contains("invoice_settings.default_payment_method") + && options.Expand.Contains("subscriptions") + && options.Expand.Contains("tax_ids"))) + .Returns(customer); + + // Act + var result = await sutProvider.Sut.GetPaymentMethod(organization); + + // Assert + Assert.NotNull(result); + Assert.Equal(accountBalance, result.AccountCredit); + await sutProvider.GetDependency().Received(1).CustomerGetAsync( + organization.GatewayCustomerId, + Arg.Is(options => + options.Expand.Contains("default_source") && + options.Expand.Contains("invoice_settings.default_payment_method") && + options.Expand.Contains("subscriptions") && + options.Expand.Contains("tax_ids"))); + + } + #endregion + + #region GetPaymentSource + + [Theory, BitAutoData] + public async Task GetPaymentSource_NullSubscriber_ThrowsArgumentNullException( + SutProvider sutProvider) => + await Assert.ThrowsAsync(() => sutProvider.Sut.GetPaymentSource(null)); + + [Theory, BitAutoData] + public async Task GetPaymentSource_Braintree_NoDefaultPaymentMethod_ReturnsNull( Provider provider, SutProvider sutProvider) { @@ -372,7 +524,7 @@ public class SubscriberServiceTests } [Theory, BitAutoData] - public async Task GetPaymentMethod_Braintree_PayPalAccount_Succeeds( + public async Task GetPaymentSource_Braintree_PayPalAccount_Succeeds( Provider provider, SutProvider sutProvider) { @@ -421,7 +573,7 @@ public class SubscriberServiceTests // TODO: Determine if we need to test Braintree.UsBankAccount [Theory, BitAutoData] - public async Task GetPaymentMethod_Stripe_BankAccountPaymentMethod_Succeeds( + public async Task GetPaymentSource_Stripe_BankAccountPaymentMethod_Succeeds( Provider provider, SutProvider sutProvider) { @@ -455,7 +607,7 @@ public class SubscriberServiceTests } [Theory, BitAutoData] - public async Task GetPaymentMethod_Stripe_CardPaymentMethod_Succeeds( + public async Task GetPaymentSource_Stripe_CardPaymentMethod_Succeeds( Provider provider, SutProvider sutProvider) { @@ -491,43 +643,37 @@ public class SubscriberServiceTests } [Theory, BitAutoData] - public async Task GetPaymentMethod_Stripe_SetupIntentForBankAccount_Succeeds( + public async Task GetPaymentSource_Stripe_SetupIntentForBankAccount_Succeeds( Provider provider, SutProvider sutProvider) { - var customer = new Customer - { - Id = provider.GatewayCustomerId - }; + var customer = new Customer { Id = provider.GatewayCustomerId }; sutProvider.GetDependency().CustomerGetAsync(provider.GatewayCustomerId, - Arg.Is( - options => options.Expand.Contains("default_source") && - options.Expand.Contains("invoice_settings.default_payment_method"))) + Arg.Is(options => options.Expand.Contains("default_source") && + options.Expand.Contains( + "invoice_settings.default_payment_method"))) .Returns(customer); var setupIntent = new SetupIntent { Id = "setup_intent_id", Status = "requires_action", - NextAction = new SetupIntentNextAction - { - VerifyWithMicrodeposits = new SetupIntentNextActionVerifyWithMicrodeposits() - }, + NextAction = + new SetupIntentNextAction + { + VerifyWithMicrodeposits = new SetupIntentNextActionVerifyWithMicrodeposits() + }, PaymentMethod = new PaymentMethod { - UsBankAccount = new PaymentMethodUsBankAccount - { - BankName = "Chase", - Last4 = "9999" - } + UsBankAccount = new PaymentMethodUsBankAccount { BankName = "Chase", Last4 = "9999" } } }; sutProvider.GetDependency().Get(provider.Id).Returns(setupIntent.Id); - sutProvider.GetDependency().SetupIntentGet(setupIntent.Id, Arg.Is( - options => options.Expand.Contains("payment_method"))).Returns(setupIntent); + sutProvider.GetDependency().SetupIntentGet(setupIntent.Id, + Arg.Is(options => options.Expand.Contains("payment_method"))).Returns(setupIntent); var paymentMethod = await sutProvider.Sut.GetPaymentSource(provider); @@ -537,24 +683,19 @@ public class SubscriberServiceTests } [Theory, BitAutoData] - public async Task GetPaymentMethod_Stripe_LegacyBankAccount_Succeeds( + public async Task GetPaymentSource_Stripe_LegacyBankAccount_Succeeds( Provider provider, SutProvider sutProvider) { var customer = new Customer { - DefaultSource = new BankAccount - { - Status = "verified", - BankName = "Chase", - Last4 = "9999" - } + DefaultSource = new BankAccount { Status = "verified", BankName = "Chase", Last4 = "9999" } }; sutProvider.GetDependency().CustomerGetAsync(provider.GatewayCustomerId, - Arg.Is( - options => options.Expand.Contains("default_source") && - options.Expand.Contains("invoice_settings.default_payment_method"))) + Arg.Is(options => options.Expand.Contains("default_source") && + options.Expand.Contains( + "invoice_settings.default_payment_method"))) .Returns(customer); var paymentMethod = await sutProvider.Sut.GetPaymentSource(provider); @@ -565,25 +706,19 @@ public class SubscriberServiceTests } [Theory, BitAutoData] - public async Task GetPaymentMethod_Stripe_LegacyCard_Succeeds( + public async Task GetPaymentSource_Stripe_LegacyCard_Succeeds( Provider provider, SutProvider sutProvider) { var customer = new Customer { - DefaultSource = new Card - { - Brand = "Visa", - Last4 = "9999", - ExpMonth = 9, - ExpYear = 2028 - } + DefaultSource = new Card { Brand = "Visa", Last4 = "9999", ExpMonth = 9, ExpYear = 2028 } }; sutProvider.GetDependency().CustomerGetAsync(provider.GatewayCustomerId, - Arg.Is( - options => options.Expand.Contains("default_source") && - options.Expand.Contains("invoice_settings.default_payment_method"))) + Arg.Is(options => options.Expand.Contains("default_source") && + options.Expand.Contains( + "invoice_settings.default_payment_method"))) .Returns(customer); var paymentMethod = await sutProvider.Sut.GetPaymentSource(provider); @@ -594,7 +729,7 @@ public class SubscriberServiceTests } [Theory, BitAutoData] - public async Task GetPaymentMethod_Stripe_LegacySourceCard_Succeeds( + public async Task GetPaymentSource_Stripe_LegacySourceCard_Succeeds( Provider provider, SutProvider sutProvider) { From 353b596a6d83eb010c0ab50cc35576943192784b Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Fri, 5 Sep 2025 10:59:36 -0500 Subject: [PATCH 3/7] [PM-25390] CORS - Password Change URI (#6287) * enable cors headers for icons program - This is needed now that browsers can hit the change-password-uri path via API call * Add absolute route for change-password-uri --- src/Icons/Controllers/ChangePasswordUriController.cs | 2 +- src/Icons/Startup.cs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Icons/Controllers/ChangePasswordUriController.cs b/src/Icons/Controllers/ChangePasswordUriController.cs index 3f2bc91cf2..935cda77df 100644 --- a/src/Icons/Controllers/ChangePasswordUriController.cs +++ b/src/Icons/Controllers/ChangePasswordUriController.cs @@ -5,7 +5,7 @@ using Microsoft.Extensions.Caching.Memory; namespace Bit.Icons.Controllers; -[Route("change-password-uri")] +[Route("~/change-password-uri")] public class ChangePasswordUriController : Controller { private readonly IMemoryCache _memoryCache; diff --git a/src/Icons/Startup.cs b/src/Icons/Startup.cs index 16bbdef553..2602dd6264 100644 --- a/src/Icons/Startup.cs +++ b/src/Icons/Startup.cs @@ -92,6 +92,9 @@ public class Startup await next(); }); + app.UseCors(policy => policy.SetIsOriginAllowed(o => CoreHelpers.IsCorsOriginAllowed(o, globalSettings)) + .AllowAnyMethod().AllowAnyHeader().AllowCredentials()); + app.UseRouting(); app.UseEndpoints(endpoints => endpoints.MapDefaultControllerRoute()); } From b7200837c3835c005ec3ae6a45b4a3ffa42d1c38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Fri, 5 Sep 2025 19:54:49 +0200 Subject: [PATCH 4/7] [PM-25182] Improve Swagger OperationIDs for Billing (#6238) * Improve Swagger OperationIDs for Billing * Fix typo --- .../OrganizationSponsorshipsController.cs | 20 +++++++++++++++++-- ...elfHostedOrganizationLicensesController.cs | 4 ++-- ...ostedOrganizationSponsorshipsController.cs | 8 +++++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs b/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs index 2d05595b2d..8c202752de 100644 --- a/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs +++ b/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs @@ -208,7 +208,6 @@ public class OrganizationSponsorshipsController : Controller [Authorize("Application")] [HttpDelete("{sponsoringOrganizationId}")] - [HttpPost("{sponsoringOrganizationId}/delete")] [SelfHosted(NotSelfHostedOnly = true)] public async Task RevokeSponsorship(Guid sponsoringOrganizationId) { @@ -225,6 +224,15 @@ public class OrganizationSponsorshipsController : Controller await _revokeSponsorshipCommand.RevokeSponsorshipAsync(existingOrgSponsorship); } + [Authorize("Application")] + [HttpPost("{sponsoringOrganizationId}/delete")] + [Obsolete("This endpoint is deprecated. Use DELETE /{sponsoringOrganizationId} instead.")] + [SelfHosted(NotSelfHostedOnly = true)] + public async Task PostRevokeSponsorship(Guid sponsoringOrganizationId) + { + await RevokeSponsorship(sponsoringOrganizationId); + } + [Authorize("Application")] [HttpDelete("{sponsoringOrgId}/{sponsoredFriendlyName}/revoke")] [SelfHosted(NotSelfHostedOnly = true)] @@ -241,7 +249,6 @@ public class OrganizationSponsorshipsController : Controller [Authorize("Application")] [HttpDelete("sponsored/{sponsoredOrgId}")] - [HttpPost("sponsored/{sponsoredOrgId}/remove")] [SelfHosted(NotSelfHostedOnly = true)] public async Task RemoveSponsorship(Guid sponsoredOrgId) { @@ -257,6 +264,15 @@ public class OrganizationSponsorshipsController : Controller await _removeSponsorshipCommand.RemoveSponsorshipAsync(existingOrgSponsorship); } + [Authorize("Application")] + [HttpPost("sponsored/{sponsoredOrgId}/remove")] + [Obsolete("This endpoint is deprecated. Use DELETE /sponsored/{sponsoredOrgId} instead.")] + [SelfHosted(NotSelfHostedOnly = true)] + public async Task PostRemoveSponsorship(Guid sponsoredOrgId) + { + await RemoveSponsorship(sponsoredOrgId); + } + [HttpGet("{sponsoringOrgId}/sync-status")] public async Task GetSyncStatus(Guid sponsoringOrgId) { diff --git a/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs b/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs index b4eecdba0f..147f2d52ee 100644 --- a/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs +++ b/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs @@ -53,7 +53,7 @@ public class SelfHostedOrganizationLicensesController : Controller } [HttpPost("")] - public async Task PostLicenseAsync(OrganizationCreateLicenseRequestModel model) + public async Task CreateLicenseAsync(OrganizationCreateLicenseRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -74,7 +74,7 @@ public class SelfHostedOrganizationLicensesController : Controller } [HttpPost("{id}")] - public async Task PostLicenseAsync(string id, LicenseRequestModel model) + public async Task UpdateLicenseAsync(string id, LicenseRequestModel model) { var orgIdGuid = new Guid(id); if (!await _currentContext.OrganizationOwner(orgIdGuid)) diff --git a/src/Api/Controllers/SelfHosted/SelfHostedOrganizationSponsorshipsController.cs b/src/Api/Controllers/SelfHosted/SelfHostedOrganizationSponsorshipsController.cs index de41a4cf10..198438201c 100644 --- a/src/Api/Controllers/SelfHosted/SelfHostedOrganizationSponsorshipsController.cs +++ b/src/Api/Controllers/SelfHosted/SelfHostedOrganizationSponsorshipsController.cs @@ -79,7 +79,6 @@ public class SelfHostedOrganizationSponsorshipsController : Controller } [HttpDelete("{sponsoringOrgId}")] - [HttpPost("{sponsoringOrgId}/delete")] public async Task RevokeSponsorship(Guid sponsoringOrgId) { var orgUser = await _organizationUserRepository.GetByOrganizationAsync(sponsoringOrgId, _currentContext.UserId ?? default); @@ -95,6 +94,13 @@ public class SelfHostedOrganizationSponsorshipsController : Controller await _revokeSponsorshipCommand.RevokeSponsorshipAsync(existingOrgSponsorship); } + [HttpPost("{sponsoringOrgId}/delete")] + [Obsolete("This endpoint is deprecated. Use DELETE /{sponsoringOrgId} instead.")] + public async Task PostRevokeSponsorship(Guid sponsoringOrgId) + { + await RevokeSponsorship(sponsoringOrgId); + } + [HttpDelete("{sponsoringOrgId}/{sponsoredFriendlyName}/revoke")] public async Task AdminInitiatedRevokeSponsorshipAsync(Guid sponsoringOrgId, string sponsoredFriendlyName) { From 2a01c804af1d4dd6e64ea08de11c05978c3e7ad9 Mon Sep 17 00:00:00 2001 From: Github Actions Date: Mon, 8 Sep 2025 10:49:00 +0000 Subject: [PATCH 5/7] Bumped version to 2025.9.0 --- Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index 3af05be0f1..66fb49300c 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -3,7 +3,7 @@ net8.0 - 2025.8.1 + 2025.9.0 Bit.$(MSBuildProjectName) enable From 7e50a46d3b315de379dfdd0c353f7f7cfd5a6c11 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Mon, 8 Sep 2025 10:12:43 -0400 Subject: [PATCH 6/7] chore(feature-flag): Remove persist-popup-view feature flag --- src/Core/Constants.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 57798204ea..69003ee253 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -199,7 +199,6 @@ public static class FeatureFlagKeys public const string SendAccess = "pm-19394-send-access-control"; /* Platform Team */ - public const string PersistPopupView = "persist-popup-view"; public const string IpcChannelFramework = "ipc-channel-framework"; public const string PushNotificationsWhenLocked = "pm-19388-push-notifications-when-locked"; public const string PushNotificationsWhenInactive = "pm-25130-receive-push-notifications-for-inactive-users"; From 0fbbb6a984a84463e2912178e815a6b07528c9df Mon Sep 17 00:00:00 2001 From: Brant DeBow <125889545+brant-livefront@users.noreply.github.com> Date: Mon, 8 Sep 2025 10:54:43 -0400 Subject: [PATCH 7/7] Event integration updates and cleanups (#6288) * Event integration updates and cleanups * Fix empty message on ArgumentException * Adjust exception message Co-authored-by: Matt Bishop --------- Co-authored-by: Matt Bishop --- .../Services/EventLoggingListenerService.cs | 4 +- .../Services/IEventMessageHandler.cs | 5 +- .../Services/IIntegrationHandler.cs | 55 +++++++++++++++++-- .../EventIntegrations/README.md | 12 +++- .../WebhookIntegrationHandler.cs | 46 ++-------------- .../Models/OrganizationIntegration.cs | 7 +-- .../OrganizationIntegrationConfiguration.cs | 7 +-- .../WebhookIntegrationHandlerTests.cs | 4 ++ 8 files changed, 76 insertions(+), 64 deletions(-) diff --git a/src/Core/AdminConsole/Services/EventLoggingListenerService.cs b/src/Core/AdminConsole/Services/EventLoggingListenerService.cs index 53ff3d4d0a..84a862ce94 100644 --- a/src/Core/AdminConsole/Services/EventLoggingListenerService.cs +++ b/src/Core/AdminConsole/Services/EventLoggingListenerService.cs @@ -28,12 +28,12 @@ public abstract class EventLoggingListenerService : BackgroundService if (root.ValueKind == JsonValueKind.Array) { var eventMessages = root.Deserialize>(); - await _handler.HandleManyEventsAsync(eventMessages); + await _handler.HandleManyEventsAsync(eventMessages ?? throw new JsonException("Deserialize returned null")); } else if (root.ValueKind == JsonValueKind.Object) { var eventMessage = root.Deserialize(); - await _handler.HandleEventAsync(eventMessage); + await _handler.HandleEventAsync(eventMessage ?? throw new JsonException("Deserialize returned null")); } else { diff --git a/src/Core/AdminConsole/Services/IEventMessageHandler.cs b/src/Core/AdminConsole/Services/IEventMessageHandler.cs index fcffb56c65..83c5e33ecb 100644 --- a/src/Core/AdminConsole/Services/IEventMessageHandler.cs +++ b/src/Core/AdminConsole/Services/IEventMessageHandler.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Models.Data; +using Bit.Core.Models.Data; namespace Bit.Core.Services; diff --git a/src/Core/AdminConsole/Services/IIntegrationHandler.cs b/src/Core/AdminConsole/Services/IIntegrationHandler.cs index 9a3edac9ec..bb10dc01b9 100644 --- a/src/Core/AdminConsole/Services/IIntegrationHandler.cs +++ b/src/Core/AdminConsole/Services/IIntegrationHandler.cs @@ -1,6 +1,5 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - +using System.Globalization; +using System.Net; using Bit.Core.AdminConsole.Models.Data.EventIntegrations; namespace Bit.Core.Services; @@ -20,8 +19,56 @@ public abstract class IntegrationHandlerBase : IIntegrationHandler public async Task HandleAsync(string json) { var message = IntegrationMessage.FromJson(json); - return await HandleAsync(message); + return await HandleAsync(message ?? throw new ArgumentException("IntegrationMessage was null when created from the provided JSON")); } public abstract Task HandleAsync(IntegrationMessage message); + + protected IntegrationHandlerResult ResultFromHttpResponse( + HttpResponseMessage response, + IntegrationMessage message, + TimeProvider timeProvider) + { + var result = new IntegrationHandlerResult(success: response.IsSuccessStatusCode, message); + + if (response.IsSuccessStatusCode) return result; + + switch (response.StatusCode) + { + case HttpStatusCode.TooManyRequests: + case HttpStatusCode.RequestTimeout: + case HttpStatusCode.InternalServerError: + case HttpStatusCode.BadGateway: + case HttpStatusCode.ServiceUnavailable: + case HttpStatusCode.GatewayTimeout: + result.Retryable = true; + result.FailureReason = response.ReasonPhrase ?? $"Failure with status code: {(int)response.StatusCode}"; + + if (response.Headers.TryGetValues("Retry-After", out var values)) + { + var value = values.FirstOrDefault(); + if (int.TryParse(value, out var seconds)) + { + // Retry-after was specified in seconds. Adjust DelayUntilDate by the requested number of seconds. + result.DelayUntilDate = timeProvider.GetUtcNow().AddSeconds(seconds).UtcDateTime; + } + else if (DateTimeOffset.TryParseExact(value, + "r", // "r" is the round-trip format: RFC1123 + CultureInfo.InvariantCulture, + DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal, + out var retryDate)) + { + // Retry-after was specified as a date. Adjust DelayUntilDate to the specified date. + result.DelayUntilDate = retryDate.UtcDateTime; + } + } + break; + default: + result.Retryable = false; + result.FailureReason = response.ReasonPhrase ?? $"Failure with status code {(int)response.StatusCode}"; + break; + } + + return result; + } } diff --git a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/README.md b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/README.md index 83b59cdec1..4092cc20ad 100644 --- a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/README.md +++ b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/README.md @@ -418,13 +418,21 @@ dependencies and integrations. For instance, `SlackIntegrationHandler` needs a ` `AddEventIntegrationServices` has a call to `AddSlackService`. Same thing for webhooks when it comes to defining a custom HttpClient by name. -1. In `AddEventIntegrationServices` create the listener configuration: +In `AddEventIntegrationServices`: + +1. Create the singleton for the handler: + +``` csharp + services.TryAddSingleton, ExampleIntegrationHandler>(); +``` + +2. Create the listener configuration: ``` csharp var exampleConfiguration = new ExampleListenerConfiguration(globalSettings); ``` -2. Add the integration to both the RabbitMQ and ASB specific declarations: +3. Add the integration to both the RabbitMQ and ASB specific declarations: ``` csharp services.AddRabbitMqIntegration(exampleConfiguration); diff --git a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/WebhookIntegrationHandler.cs b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/WebhookIntegrationHandler.cs index 99cad65efa..e0c2b66a90 100644 --- a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/WebhookIntegrationHandler.cs +++ b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/WebhookIntegrationHandler.cs @@ -1,7 +1,5 @@ #nullable enable -using System.Globalization; -using System.Net; using System.Net.Http.Headers; using System.Text; using Bit.Core.AdminConsole.Models.Data.EventIntegrations; @@ -17,7 +15,8 @@ public class WebhookIntegrationHandler( public const string HttpClientName = "WebhookIntegrationHandlerHttpClient"; - public override async Task HandleAsync(IntegrationMessage message) + public override async Task HandleAsync( + IntegrationMessage message) { var request = new HttpRequestMessage(HttpMethod.Post, message.Configuration.Uri); request.Content = new StringContent(message.RenderedTemplate, Encoding.UTF8, "application/json"); @@ -28,45 +27,8 @@ public class WebhookIntegrationHandler( parameter: message.Configuration.Token ); } + var response = await _httpClient.SendAsync(request); - var result = new IntegrationHandlerResult(success: response.IsSuccessStatusCode, message); - - switch (response.StatusCode) - { - case HttpStatusCode.TooManyRequests: - case HttpStatusCode.RequestTimeout: - case HttpStatusCode.InternalServerError: - case HttpStatusCode.BadGateway: - case HttpStatusCode.ServiceUnavailable: - case HttpStatusCode.GatewayTimeout: - result.Retryable = true; - result.FailureReason = response.ReasonPhrase ?? $"Failure with status code: {(int)response.StatusCode}"; - - if (response.Headers.TryGetValues("Retry-After", out var values)) - { - var value = values.FirstOrDefault(); - if (int.TryParse(value, out var seconds)) - { - // Retry-after was specified in seconds. Adjust DelayUntilDate by the requested number of seconds. - result.DelayUntilDate = timeProvider.GetUtcNow().AddSeconds(seconds).UtcDateTime; - } - else if (DateTimeOffset.TryParseExact(value, - "r", // "r" is the round-trip format: RFC1123 - CultureInfo.InvariantCulture, - DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal, - out var retryDate)) - { - // Retry-after was specified as a date. Adjust DelayUntilDate to the specified date. - result.DelayUntilDate = retryDate.UtcDateTime; - } - } - break; - default: - result.Retryable = false; - result.FailureReason = response.ReasonPhrase ?? $"Failure with status code {(int)response.StatusCode}"; - break; - } - - return result; + return ResultFromHttpResponse(response, message, timeProvider); } } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegration.cs b/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegration.cs index 5e5f7d4802..0f47d5947b 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegration.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegration.cs @@ -1,13 +1,10 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using AutoMapper; +using AutoMapper; namespace Bit.Infrastructure.EntityFramework.AdminConsole.Models; public class OrganizationIntegration : Core.AdminConsole.Entities.OrganizationIntegration { - public virtual Organization Organization { get; set; } + public virtual required Organization Organization { get; set; } } public class OrganizationIntegrationMapperProfile : Profile diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegrationConfiguration.cs b/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegrationConfiguration.cs index 52b8783fcf..21b282f767 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegrationConfiguration.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegrationConfiguration.cs @@ -1,13 +1,10 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using AutoMapper; +using AutoMapper; namespace Bit.Infrastructure.EntityFramework.AdminConsole.Models; public class OrganizationIntegrationConfiguration : Core.AdminConsole.Entities.OrganizationIntegrationConfiguration { - public virtual OrganizationIntegration OrganizationIntegration { get; set; } + public virtual required OrganizationIntegration OrganizationIntegration { get; set; } } public class OrganizationIntegrationConfigurationMapperProfile : Profile diff --git a/test/Core.Test/AdminConsole/Services/WebhookIntegrationHandlerTests.cs b/test/Core.Test/AdminConsole/Services/WebhookIntegrationHandlerTests.cs index bf4283243c..53a3598d47 100644 --- a/test/Core.Test/AdminConsole/Services/WebhookIntegrationHandlerTests.cs +++ b/test/Core.Test/AdminConsole/Services/WebhookIntegrationHandlerTests.cs @@ -51,6 +51,7 @@ public class WebhookIntegrationHandlerTests Assert.True(result.Success); Assert.Equal(result.Message, message); + Assert.Empty(result.FailureReason); sutProvider.GetDependency().Received(1).CreateClient( Arg.Is(AssertHelper.AssertPropertyEqual(WebhookIntegrationHandler.HttpClientName)) @@ -59,6 +60,7 @@ public class WebhookIntegrationHandlerTests Assert.Single(_handler.CapturedRequests); var request = _handler.CapturedRequests[0]; Assert.NotNull(request); + Assert.NotNull(request.Content); var returned = await request.Content.ReadAsStringAsync(); Assert.Equal(HttpMethod.Post, request.Method); @@ -77,6 +79,7 @@ public class WebhookIntegrationHandlerTests Assert.True(result.Success); Assert.Equal(result.Message, message); + Assert.Empty(result.FailureReason); sutProvider.GetDependency().Received(1).CreateClient( Arg.Is(AssertHelper.AssertPropertyEqual(WebhookIntegrationHandler.HttpClientName)) @@ -85,6 +88,7 @@ public class WebhookIntegrationHandlerTests Assert.Single(_handler.CapturedRequests); var request = _handler.CapturedRequests[0]; Assert.NotNull(request); + Assert.NotNull(request.Content); var returned = await request.Content.ReadAsStringAsync(); Assert.Equal(HttpMethod.Post, request.Method);