[PM 29610]Update Account Storage Endpoint by cyprain-okeke · Pull Request #6750 · bitwarden/server

cyprain-okeke · 2025-12-18T13:27:49Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-29610

📔 Objective

This PR implements PM-29610: Update Account Storage Endpoint - a refactor to modernize the individual premium user storage management endpoint by migrating it to the new VNext billing architecture.

What Changed

Endpoint Migration:

FROM: POST /accounts/storage (legacy AccountsController)
TO: PUT /account/billing/vnext/storage (new AccountBillingVNextController)
Changed HTTP method to PUT to properly reflect the idempotent nature of the operation

Key Improvements:

1. Simplified Request Model:
- Old approach used delta/adjustment values (e.g., "add 5GB")
- New approach uses absolute storage values (e.g., "set to 10GB total")
- Makes the API more intuitive and truly idempotent
2. Modern Architecture:
- Follows the new command/handler pattern (UpdateStorageCommand)
- Uses BillingCommandResult<T> with OneOf union types for type-safe error handling
- Leverages the new IPricingClient for dynamic pricing configuration
3. Robust Validation:
- Premium subscription verification
- Payment method existence checks
- Storage bounds validation (minimum base storage to 100 GB maximum)
- Current usage verification (prevents setting storage below what's currently used)
- Idempotency check (no-op when storage is unchanged)
4. Extensible Design:
- Single endpoint handles both storage increases and decreases
- Future-proof for additional storage-related operations

Files Added/Modified

New Files:

src/Api/Billing/Models/Requests/Storage/StorageUpdateRequest.cs - Request model with validation
src/Core/Billing/Storage/Commands/UpdateStorageCommand.cs - Command implementation
test/Core.Test/Billing/Storage/Commands/UpdateStorageCommandTests.cs - 18 comprehensive tests
test/Api.Test/Billing/Controllers/VNext/AccountBillingVNextControllerTests.cs - 10 controller integration tests

Modified Files:

src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs - Added PUT storage endpoint
src/Core/Billing/Extensions/ServiceCollectionExtensions.cs - Registered new command in DI container

Bug Fix (Unrelated):

test/Core.Test/Billing/Payment/Commands/UpdateBillingAddressCommandTests.cs - Fixed incorrect method names that didn't match IStripeAdapter interface

📸 Screenshots

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

claude · 2025-12-18T13:33:09Z

Claude finished @cyprain-okeke's task in 2m 59s —— View job

Code Review Complete

Starting review of PR [PM 29610]Update Account Storage Endpoint #6750
Analyzing changes with bitwarden-code-reviewer

Review complete. Inline comments and summary have been posted to the PR.

github-actions · 2025-12-18T13:38:18Z

Checkmarx One – Scan Summary & Details – c8fb5075-7aa7-4ce1-981f-26596e85af5b

New Issues (1)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		CSRF	/src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs: 98	details Method at line 98 of /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs gets a parameter from a user request from user. This pa... Attack Vector

codecov · 2025-12-18T13:43:14Z

Codecov Report

❌ Patch coverage is 87.87879% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.78%. Comparing base (cc03842) to head (6a5ccb1).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...ng/Models/Requests/Storage/StorageUpdateRequest.cs	6.66%	14 Missing ⚠️
...ng/Premium/Commands/UpdatePremiumStorageCommand.cs	98.18%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6750      +/-   ##
==========================================
+ Coverage   54.74%   54.78%   +0.04%     
==========================================
  Files        1923     1925       +2     
  Lines       85291    85422     +131     
  Branches     7635     7637       +2     
==========================================
+ Hits        46693    46802     +109     
- Misses      36825    36847      +22     
  Partials     1773     1773

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

amorask-bitwarden · 2025-12-18T17:47:10Z

+using Bit.Core.Utilities;
+using Microsoft.Extensions.Logging;
+
+namespace Bit.Core.Billing.Storage.Commands;


This should live in the Premium folder and probably be named UpdatePremiumStorageCommand.

amorask-bitwarden · 2025-12-18T17:47:47Z

+{
+    public Task<BillingCommandResult<string?>> Run(User user, short storageGb) => HandleAsync<string?>(async () =>
+    {
+        if (user == null)


User should never be null if it's not marked as nullable.

amorask-bitwarden · 2025-12-18T17:48:36Z

+            return new BadRequest("No subscription found.");
+        }
+
+        var premiumPlan = await pricingClient.GetAvailablePremiumPlan();


We're going to be supporting users across multiple plans - you'll have to fetch all plans and find the one the user is on.

amorask-bitwarden · 2025-12-18T17:49:34Z

+        user.MaxStorageGb = storageGb;
+        await userService.SaveUserAsync(user);
+
+        return paymentIntentClientSecret;


This is not used and not needed.

amorask-bitwarden · 2025-12-18T17:51:22Z

+        var additionalStorage = storageGb - baseStorageGb;
+
+        // Call the payment service to adjust the subscription
+        var paymentIntentClientSecret = await paymentService.AdjustStorageAsync(


This is our chance to get away from the bloated and complicated FinalizeSubscriptionChangeAsync and the StripePaymentService; I suggest we take the opportunity.

All that would be required is that you update the premium subscription with correct storage amount and invoice a proration for it since storage is always billed annually.

amorask-bitwarden · 2025-12-18T17:52:52Z

+    /// </summary>
+    [Required]
+    [Range(1, 100)]
+    public short StorageGb { get; set; }


I don't think the user selects the total amount of storage they want in the web app. Rather, they submit how much additional storage they want to purchase. Please adjust this accordingly.

amorask-bitwarden · 2025-12-18T17:54:59Z

-    // TODO: Migrate to Command / AccountBillingVNextController as PUT /account/billing/vnext/subscription
-    [HttpPost("storage")]
-    [SelfHosted(NotSelfHostedOnly = true)]
-    public async Task<PaymentResponseModel> PostStorageAsync([FromBody] StorageRequestModel model)


This work needs to go behind a feature flag. Since it is part of the subscription page redesign, I suggest creating this flag which I'll have created for clients: pm-29594-update-individual-subscription-page

We can't remove this endpoint until that feature flag is removed or the old version of the subscription page would fail on adding storage.

Same issue still applies with applicable comments regarding FF on the command.

amorask-bitwarden · 2025-12-18T17:56:03Z

+            return new BadRequest("User does not have a premium subscription.");
+        }
+
+        if (string.IsNullOrWhiteSpace(user.GatewayCustomerId))


I think the GatewayCustomerId and GatewaySubscriptionId checks are redundant with checking if the user has premium. The user has to have a customer and subscription to have premium.

…oint

amorask-bitwarden · 2025-12-23T15:21:09Z

+
+        // Fetch all premium plans and find the one the user is on
+        var premiumPlans = await pricingClient.ListPremiumPlans();
+        var premiumPlan = premiumPlans.FirstOrDefault(p => p.Available);


❌ This isn't the plan the user is on - it's the plan that's available. The Premium user could be on an older plan. You need to match the Price ID of their password manager subscription item with the price ID from one of the premium plans.

amorask-bitwarden · 2025-12-23T15:22:10Z

+
+        var newTotalStorageGb = (short)(baseStorageGb + additionalStorageGb);
+
+        if (newTotalStorageGb > 100)


❓ Is this a business rule we have?

Yes, this 100 GB limit exists in the old BillingHelpers.AdjustStorageAsync code at /server/src/Core/Utilities/BillingHelpers.cs:38-41. The new UpdatePremiumStorageCommand copied this
validation.

amorask-bitwarden · 2025-12-23T15:22:29Z

+        }
+
+        // Check if the requested storage would fit the user's current usage
+        if (!user.MaxStorageGb.HasValue)


⛏️ This could probably be moved up in the command to prevent any unnecessary calls.

amorask-bitwarden · 2025-12-23T15:24:59Z

+        }
+
+        // Check feature flag to determine which code path to use
+        if (featureService.IsEnabled(FeatureFlagKeys.PM29594_UpdateIndividualSubscriptionPage))


❌ This feature flag controls which version of the Premium Subscription page is going to show on the Web Vault. The new version of the page is going to hit the new endpoint which hits the new command. The old version of the page will continue to hit the old endpoint with the old approach. As such, I don't think we should need to check the flag within the command.

amorask-bitwarden · 2025-12-23T15:26:29Z

+            // NEW PATH: Directly update the premium subscription with prorations
+            // This is simpler than using FinalizeSubscriptionChangeAsync since storage is always billed annually
+            var subscription = await stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId);
+            if (subscription == null)


⛏️ stripeAdapter.GetSubscriptionAsync doesn't ever return null; it throws an exception. If you want to check for null, you'd have to use the SubscriberService or catch a StripeException with a "resource_missing" error code.

amorask-bitwarden · 2025-12-23T15:28:26Z

-    // TODO: Migrate to Command / AccountBillingVNextController as PUT /account/billing/vnext/subscription
-    [HttpPost("storage")]
-    [SelfHosted(NotSelfHostedOnly = true)]
-    public async Task<PaymentResponseModel> PostStorageAsync([FromBody] StorageRequestModel model)


Same issue still applies with applicable comments regarding FF on the command.

…oint

amorask-bitwarden · 2025-12-29T14:45:35Z

+        await userService.SaveUserAsync(user);
+
+        // No payment intent needed - the subscription update will automatically create and finalize the invoice
+        return (string?)null;


❌ This return type of string serves no purpose. The command can return None.

…oint

cyprain-okeke added 2 commits December 18, 2025 13:07

update account storage endpoint

ca40b3f

Fix the failing test

e0a1e26

cyprain-okeke requested a review from a team as a code owner December 18, 2025 13:27

cyprain-okeke requested a review from kdenney December 18, 2025 13:27

amorask-bitwarden requested changes Dec 18, 2025

View reviewed changes

cyprain-okeke and others added 3 commits December 23, 2025 15:08

Added flag and refactor base on pr comments

a3e6679

Merge branch 'main' into billing/pm-29610/update-account-storage-endp…

56bf37d

…oint

fix the lint error

d58c0da

cyprain-okeke requested a review from amorask-bitwarden December 23, 2025 14:50

amorask-bitwarden requested changes Dec 23, 2025

View reviewed changes

Resolve the pr comments

505428e

cyprain-okeke requested a review from amorask-bitwarden December 23, 2025 21:43

cyprain-okeke and others added 3 commits December 23, 2025 22:49

Merge branch 'main' into billing/pm-29610/update-account-storage-endp…

c81faf1

…oint

Fix the failing test

cf6d7f7

Fix the failing test

77420e7

amorask-bitwarden requested changes Dec 29, 2025

View reviewed changes

Return none

d7d080d

cyprain-okeke force-pushed the billing/pm-29610/update-account-storage-endpoint branch from e79386e to d7d080d Compare December 29, 2025 14:58

cyprain-okeke and others added 3 commits December 29, 2025 15:59

Merge branch 'main' into billing/pm-29610/update-account-storage-endp…

13117e1

…oint

Merge branch 'main' into billing/pm-29610/update-account-storage-endp…

3e2c981

…oint

Resolve the lint error

89560de

cyprain-okeke requested a review from amorask-bitwarden December 29, 2025 16:04

amorask-bitwarden previously approved these changes Dec 29, 2025

View reviewed changes

Merge branch 'main' into billing/pm-29610/update-account-storage-endp…

bcb625c

…oint

cyprain-okeke dismissed amorask-bitwarden’s stale review via bcb625c December 31, 2025 16:50

cyprain-okeke and others added 2 commits December 31, 2025 18:09

Fix the failing test

ceee636

Merge branch 'main' into billing/pm-29610/update-account-storage-endp…

243a6c6

…oint

Add the missing test

836fff6

cyprain-okeke requested a review from amorask-bitwarden December 31, 2025 17:28

Formatting issues fixed

6a5ccb1

amorask-bitwarden approved these changes Jan 5, 2026

View reviewed changes

amorask-bitwarden merged commit 76a8f0f into main Jan 5, 2026
45 checks passed

amorask-bitwarden deleted the billing/pm-29610/update-account-storage-endpoint branch January 5, 2026 16:52


		var newTotalStorageGb = (short)(baseStorageGb + additionalStorageGb);

		if (newTotalStorageGb > 100)

Conversation

cyprain-okeke commented Dec 18, 2025

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

Uh oh!

claude Bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Complete

Uh oh!

github-actions Bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amorask-bitwarden Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Dec 18, 2025 •

edited

Loading

github-actions Bot commented Dec 18, 2025 •

edited

Loading

codecov Bot commented Dec 18, 2025 •

edited

Loading

amorask-bitwarden Dec 18, 2025 •

edited

Loading