billing/pm-24665/license-file-generation-should-fail-for-unpaid-subscription#7444
Conversation
|
New Issues (4)Checkmarx found the following issues in this Pull Request
Fixed Issues (4)Great job! The following issues were fixed in this Pull Request
|
…uld-fail-for-unpaid-subscription
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7444 +/- ##
==========================================
+ Coverage 58.75% 58.77% +0.02%
==========================================
Files 2071 2071
Lines 91252 91270 +18
Branches 8130 8134 +4
==========================================
+ Hits 53611 53646 +35
+ Misses 35726 35721 -5
+ Partials 1915 1903 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
amorask-bitwarden
left a comment
amorask-bitwarden
left a comment
There was a problem hiding this comment.
What about premium?
…uld-fail-for-unpaid-subscription
|
|
||
| public static class SubscriptionLicenseValidator | ||
| { | ||
| public static void ValidateSubscriptionForLicenseGeneration(SubscriptionInfo subscriptionInfo) |
There was a problem hiding this comment.
❌ I don't think there's any reason for this to take a SubscriptionInfo when we can just pass in Subscription.
| { | ||
| public async Task<UserLicense> Run(User user) | ||
| { | ||
| var subscriptionInfo = await paymentService.GetSubscriptionAsync(user); |
There was a problem hiding this comment.
❌ This work should be done in the query itself and it should just use the StripeAdapter to get the subscription per the comment here: https://github.com/bitwarden/server/pull/7444/changes#r3081694476
| } | ||
|
|
||
| var subscriptionInfo = await GetSubscriptionAsync(organization); | ||
| SubscriptionLicenseValidator.ValidateSubscriptionForLicenseGeneration(subscriptionInfo); |
There was a problem hiding this comment.
See comment here: https://github.com/bitwarden/server/pull/7444/changes#r3081694476
…uld-fail-for-unpaid-subscription
…uld-fail-for-unpaid-subscription
amorask-bitwarden
left a comment
amorask-bitwarden
left a comment
There was a problem hiding this comment.
Looking at this further, I'm not sure an entirely new validator is required for this work, which is evidenced by fact we're only checking the subscription status to make sure the subscription isn't terminal. Is there any reason we can't just check the subscription status in each license query? That way you don't have to double fetch the subscription in the org license query nor did you need an entirely new file.
…uld-fail-for-unpaid-subscription
|
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-24665
📔 Objective
The PR adds validation to the license generation endpoint that blocks license file downloads when an organization's subscription is in any of these statuses:
Technical Implementation
Location: src/Core/Billing/Organizations/Queries/GetCloudOrganizationLicenseQuery.cs
Changes:
Test Coverage:
📸 Screenshots