Fix expired license error handling #1895

smrdotgg · 2023-11-30T13:03:21Z

Features and Changes

Fixes the issue where an expired license key causes an API error, preventing them from logging in and accessing the UI for license management. This issue was limited to API keys who's trials expired. Paying enterprise members who's license expired did not face this issue.
Fixes Expired License Keys Cause API Error State (#1774)

Dependencies

None

Tests

Test 1 - Login

Add an enterprise or pro key on a new account through the GrowthBook UI. (Ensure it is a trial key)
Allow the key to expire. (Can be simulated by changing your machine's date/time settings)
Ensure the license data isn't cached in server memory from when you added the key. (Memory can be flushed by restarting the dev server)
Attempt to log in using an account associated with the expired trial key.

Before this change, the login would fail.
After this change, the login succeeds and displays a banner on the top bar.

Test 2 - Unauthorized requests

Add an enterprise or pro key on a new account through the GrowthBook UI. (Ensure it is a trial key)
Allow the key to expire. (Can be simulated by changing your machine's date/time settings)
Ensure the license data isn't cached in server memory from when you added the key. (Memory can be flushed by restarting the dev server)
Log in using an account associated with the expired trial key.
Find an enterprise only feature that has been disabled. We can take exporting logs as json, which can be found in the /events route of the frontend.
The button will be disabled. Force-enable the button. For the case of the events export feature, this can be done by setting the disabled flag on the button in the EventsPage component (packages/front-end/components/Events/EventsPage/EventsPage.tsx) to false.
Once the button is enabled, press it.

Before this change, the backend would process the request and provide a valid json response eventhough the license has expired. The feature-blocking was only happening in the frontend logic. (This could have also been bypassed with curl)

After this change, the backend will do its own check to see if the license key has expired. If it has, it will not process the request for the premium feature. So in this case, it would not process the invalid request.

Screenshots (for Test 1)

Before

After

august-growthbook · 2023-11-30T14:59:03Z

Thanks for the PR! Someone from Engineering will review it.

jdorn · 2023-12-01T05:57:18Z

For paying customers (non-trial license keys), we are super lenient on the expiration date. We don't want someone to lose access to all of the Enterprise features they rely on in production, just because they are a little late renewing. For trials, however, we want to be a lot stricter since it's just meant as a way to preview Enterprise features for a short time before deciding whether or not to pay.

The current solution of throwing an Exception is bad since it stops the app from loading entirely. The change in this PR makes trials too lenient - basically allowing them to continue using Enterprise features forever without paying. We want to do something in between - downgrade the app to the Open Source version, but don't throw an Exception.

This will likely require digging a bit deeper into the code and might make sense for our engineers to help out with the implementation.

tzjames

Thanks again for doing this Semere! It looks almost perfect. I left a few comments though. Did you check out an api endpoint that has enterprise features using curl? Do they throw good error messages?

tzjames · 2023-12-05T13:08:27Z

packages/enterprise/src/license.ts

-  // The `trial` field used to be optional, force it to always be defined
-  decodedLicense.trial = !!decodedLicense.trial;


Something else might still rely on this code. Better to leave it in.

Good call, I'll leave that in.

tzjames · 2023-12-05T13:33:07Z

packages/enterprise/src/license.ts

+    // Create a date object for the expiration date
+    const expirationDate = new Date(licenseData.exp);
+
+    // Add a buffer of 2 days to the expiration date
+    const expiresWithBuffer = new Date(expirationDate);
+    expiresWithBuffer.setDate(expirationDate.getDate() + 2);
+
+    // Check if the adjusted expiration date is in the past
+    if (expiresWithBuffer < new Date()) {
+      // The license is expired
+      return true;
+    }
+  }


From my understanding before if it was a .trial license and it expired we errored immediately with no buffer. It errored too harshly to the point that the app became unusable. This change would give them the two day buffer as well, but I think we only want to have the two day grace period with enterprise customers.

Yeah, as it is, it adds the buffer for all users. I'll make sure it only does so for non-trial users.

smrdotgg · 2023-12-05T14:20:10Z

Thanks again for doing this Semere! It looks almost perfect. I left a few comments though. Did you check out an api endpoint that has enterprise features using curl? Do they throw good error messages?

I have tested the api endpoints with curl, and they only execute if the license key is valid. If its invalid, they throw appropriate errors with correct codes and proper messages.

I've also added the changes you proposed. 😃

tzjames

It looks good now. Congrats!

tzjames · 2023-12-05T20:00:54Z

packages/enterprise/src/license.ts

+    const expirationDate = new Date(licenseData.exp);
+
+    // Add a buffer of 2 days to the expiration date for non-trial users
+    if (licenseData.trial) {


I think you are missing a !

tzjames

We chatted in person about making the non-trial licenses just show the warning but not impede functionality.

…same check

tzjames

This is looking good now.

smrdotgg marked this pull request as draft November 30, 2023 13:46

smrdotgg marked this pull request as ready for review November 30, 2023 13:46

smrdotgg changed the title ~~Fix expired license error handling (#1774)~~ Fix expired license error handling Nov 30, 2023

august-growthbook requested a review from a team November 30, 2023 14:58

august-growthbook added the bug Something isn't working label Nov 30, 2023

tzjames self-requested a review December 4, 2023 18:31

tzjames requested changes Dec 5, 2023

View reviewed changes

tzjames approved these changes Dec 5, 2023

View reviewed changes

tzjames requested changes Dec 7, 2023

View reviewed changes

Semere Tereffe added 6 commits December 14, 2023 12:37

Fix expired license error handling (growthbook#1774)

4009863

Add license expiration check for trial

f18bea7

Make Premium Check Consider Trial Dates, Make License Middleware use …

c14899d

…same check

Fix Rebase Conflicts

009c24c

Minor Change: Function Rename

87a10e7

Remove expired license test case from license.test.ts

e34bfe6

smrdotgg force-pushed the bug/1774-expired-license-error branch from 207a8d9 to e34bfe6 Compare December 14, 2023 11:09

smrdotgg requested a review from tzjames December 14, 2023 11:59

tzjames approved these changes Dec 28, 2023

View reviewed changes

tzjames merged commit bb88729 into growthbook:main Dec 28, 2023
2 of 3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix expired license error handling #1895

Fix expired license error handling #1895

smrdotgg commented Nov 30, 2023 •

edited

august-growthbook commented Nov 30, 2023

jdorn commented Dec 1, 2023

tzjames left a comment

tzjames Dec 5, 2023

smrdotgg Dec 5, 2023

tzjames Dec 5, 2023

smrdotgg Dec 5, 2023

smrdotgg commented Dec 5, 2023

tzjames left a comment

tzjames Dec 5, 2023

tzjames left a comment

tzjames left a comment

		// The `trial` field used to be optional, force it to always be defined
		decodedLicense.trial = !!decodedLicense.trial;

Fix expired license error handling #1895

Fix expired license error handling #1895

Conversation

smrdotgg commented Nov 30, 2023 • edited

Features and Changes

Dependencies

Tests

Test 1 - Login

Test 2 - Unauthorized requests

Screenshots (for Test 1)

Before

After

august-growthbook commented Nov 30, 2023

jdorn commented Dec 1, 2023

tzjames left a comment

Choose a reason for hiding this comment

tzjames Dec 5, 2023

Choose a reason for hiding this comment

smrdotgg Dec 5, 2023

Choose a reason for hiding this comment

tzjames Dec 5, 2023

Choose a reason for hiding this comment

smrdotgg Dec 5, 2023

Choose a reason for hiding this comment

smrdotgg commented Dec 5, 2023

tzjames left a comment

Choose a reason for hiding this comment

tzjames Dec 5, 2023

Choose a reason for hiding this comment

tzjames left a comment

Choose a reason for hiding this comment

tzjames left a comment

Choose a reason for hiding this comment

smrdotgg commented Nov 30, 2023 •

edited