free trials are exploitable by cancelling and resubscribing to the same trial

[thomasplevy]

[actual-saurabh]

So,I explored this a bit because of a support request and would like to take a stab at this. However, wanted to confirm if my strategy is correct and I'm not missing anything:

Refactor has_trial() (https://github.com/gocodebox/lifterlms/blob/master/includes/models/model.llms.access.plan.php#L471-L483) to check if the current user has already availed of a trial for this plan by looking at past orders where the plan_id match the current one and the order_status has already expired or cancelled. This way, user gets the standard checkout without the trial.

Since while checking out, if the user already exists, the checkout redirects with a user already exists error, this should be sufficient to take care of this instead of refactoring https://github.com/gocodebox/lifterlms/blob/master/includes/controllers/class.llms.controller.orders.php#L147-L309

Unless, I'm missing something here.

[thomasplevy]

I think that it would be better to create a new function instead of adding new logic to has_trial()

Currently the method simply checks if the access plan has a trial. It's used to check logic related to the plan having a trial and adding user-related logic might get messy here.

I think it would be better to add a new method which could be used to check very specifically if the user has already "trialed" the plan.

This would keep both functions very specific to one purpose and prevent potential issues that could arise elsewhere in the codebase from adjusting the "has_trial()" method.

I'm not sure if I'm overcomplicating this but It worries me to add user logic into this function.

[actual-saurabh]

It does make sense to add a is_trial_available_to_user() method which checks if the user has any past orders for the product (except failed & refunded ones).

However, I'm not sure if the trial is expected to behave like a one-time trial that does not care if the earlier order was for a trial or full access. @thomasplevy AFAI understand, trials shouldn't be able to repeat for a particular product, so this can check all orders for the product that gave the user access at some point. Do let me know if that's not the case and I'm missing something here.

https://github.com/actual-saurabh/lifterlms/blob/530f281e0b2ef97ea34319056984588acbaf5c88/includes/models/model.llms.access.plan.php#L495-L508

[thomasplevy]

However, I'm not sure if the trial is expected to behave like a one-time trial that does not care if the earlier order was for a trial or full access.

I'm not really certain what the "accepted" best practice here would be. I think that we could assume that a trial could be used to reengage someone who has already been a member of the course. The scenario I'm thinking is:

• Student subscribes to a course (with no trial) and then cancels after a period of time (doesn't matter what the period is, could be a year could be a day)
• Instructor improves the course with new content in an effort to re-engage cancelled students
• Instructor adds a plan with a trial (again, period doesn't work)

At this point, it's difficult to determine what the goal of this trial is. One could argue that the instructor wishes cancelled students to try the trial out to experience new content. It could also be argued that instructors may NOT want this.

Perhaps it would be best to add a trial option to allow instructors to determine the behavior of the trial however this adds another setting which may be complicated.

I think it would be best to assume that a student can trial one time per product. The availability check function can check to see if they have EVER trialed that product

A filter on the function could allow us to customize this behavior easily with a small code snippet. If we discover over time that a lot of users require customization of this behavior via the filter we could then endeavor to add an option to the UI.

I'm thinking about a filter like:

llms_is_trial_available_to_user_check_trials_only (bool)

this should, by default, return true. If false, we should include a check for ALL orders instead of only orders with trials

The returns (bool) of the function should be filterable as well to allow even further customization of the function's behavior.

[actual-saurabh]

@thomasplevy I have added a new method here: https://github.com/gocodebox/lifterlms/blob/452bf6b175546f772bded79b6f34d9603c767e85/includes/models/model.llms.access.plan.php#L695-L773

If this makes sense, I'll go ahead and modify the templates and such to use this is_trial_available_to_user() method in place of has_trial() where appropriate.

[thomasplevy]

@actual-saurabh this looks great. The only change I'd make would be that the returns of the is_trial_available_to_user() method should be filterable just to expose an entry point should anyone want to customize the behavior further... Like if they wanted to retain existing functionality they could always return true.

[nrherron92]

HS-178320

This is sort of related, the user has a free plan that expires after 7 days so this is not technically the same thing, but what we would normally do is have them set up a trial instead of a regular access plan but the same issue occurs the user can just cancel it and go back to the free trial each time.

So maybe it's more of a feature request to allow a setting to let users purchase an access plan only once, in addition to preventing users from having a free trial.

This has a work around that I gave the user so it isn't urgent :)

[nrherron92]

HS-179675