dualcube / moodle-enrol_stripepayment

Moodle Stripe Payment Collector
16 stars 27 forks source link

Upgrading from version 2.0 -> 3.2.1 fails any purchase made by existing customer #62

Closed bit9labs closed 3 years ago

bit9labs commented 3 years ago

I hope not many people experience this but I made the move from Moodle 3.8 -> 3.10+ so I figured it was time to upgrade my enrollment plugin.

Version 2.0 stores the $USER->id in the receiver_id column of enrol_stripepayment Version 3.2.1 stores the stripe customer id in the same column (cus_XXXXX). It does not check for the validity of the existing data and there is no migration strategy to remove or reformat the existing data.

$checkcustomer = $DB->get_records('enrol_stripepayment',
    array('receiver_email' => $data->stripeEmail));
    foreach ($checkcustomer as $keydata => $valuedata) {
        $checkcustomer = $valuedata;
    }

    if (!$checkcustomer) {
        $customerarray = array("email" => $data->stripeEmail,
        "description" => get_string('charge_description1', 'enrol_stripepayment'));
        if ($iscoupon) {
            $customerarray["coupon"] = $data->coupon_id;
        }
            $charge1 = \Stripe\Customer::create($customerarray);
            $data->receiver_id = $charge1->id;
    } else {
        if ($iscoupon) {
            $cu = \Stripe\Customer::retrieve($checkcustomer->receiver_id);
            $cu->coupon = $data->coupon_id;
            $cu->save();
        } else {
            $cu = \Stripe\Customer::retrieve($checkcustomer->receiver_id);
            $cu->coupon = null;
            $cu->save();
        }
        $data->receiver_id = $checkcustomer->receiver_id;
    }

\Stripe\Customer::retrieve($checkcustomer->receiver_id); throws an exception and the user will get: 'Something else happened, completely unrelated to Stripe' Stripe will bill the customer and the user will not get enrolled. This is very dangerous and leads to poor user experience.

bit9labs commented 3 years ago

Proposed fix is to check the format of the receiver_id

if (!$checkcustomer || strpos($checkcustomer->receiver_id, "cus_") === false) {

RTrave commented 3 years ago

Hi, is there a workaround ? modifying charge.php don't fix the trouble here. thanks

RTrave commented 3 years ago

Proposed fix is to check the format of the receiver_id

if (!$checkcustomer || strpos($checkcustomer->receiver_id, "cus_") === false) {

finally, changes in freeenrol.php and charge.php are ok, but with a different prefix here, not cus, but pi_, surely for payment indent. note: tests are ok, problem only appears with production keys.

ty for your great job on this plugin.

moumitahalder commented 3 years ago

Hi @bit9labs, please update the plugin to the latest version and check.