[BUG] [SOLVED] Terms of Service field not validated at all

[razor7]

Hi, in my test server I have J! 2.5.24 and VM 2.6.10.

First, when registering using VM registration form, I can register without checking the "I agree to the Terms of Service" check box! is that correct?
I expected VM to validate that check and to cancel user registration if user didn't accepted Terms of Service

Second, I have checked Must agree to Terms of Service on EVERY ORDER? and Show Terms of Service on the cart/checkout? in VM configuration, despite having that options enabled, I still can buy things without checking the Terms of Service checkbox at checkout!


My test server is here https://it.mgscreativa.com.ar/j25vm2updt/index.php?option=com_virtuemart&view=user&layout=edit

Thanks!

[razor7]

Hi, after a little debugging in the VM 2.6.10 code I found several bugs in the Terms Of Service mechanism.

First of all, I'm attaching a zip with patched files and a patch file to review the changes.

After analysing the TOS mechanism, I think no user may get registered without accepting TOS, because the agreed user field is set as a core field and can't be edited (ie: you can't set the required state for it). Until a config option like "Must agree to TOS at registration" gets implemented, agreed must be required! at registration.

So first, the "I agree to the Terms of Service" registration field should get a star indicating it's required!
administrator/components/com_virtuemart/models/userfields.php function getCoreFields line 233, agreed should be removed from the array.
Change from this

Code: [Select]

static function getCoreFields(){
return array( 'name','username', 'email', 'password', 'password2' , 'agreed','language');
}to this...

Code: [Select]

static function getCoreFields(){
return array( 'name','username', 'email', 'password', 'password2', 'language');
}
And the agreed form field should be marked as required in its HTML code (form field should be evaluated for "required" mark)
administrator/components/com_virtuemart/models/userfields.php function getUserFieldsFilled line 873
Change from this

Code: [Select]

case 'agreed':
$_return['fields'][$_fld->name]['formcode'] = '<input type="checkbox" name="'
. $_prefix.$_fld->name . '" id="' . $_prefix.$_fld->name . '_field" value="1" '
. ($_return['fields'][$_fld->name]['value'] ? 'checked="checked"' : '') .'/>';
break;to this...

Code: [Select]

case 'agreed':
$_return['fields'][$_fld->name]['formcode'] = '<input type="checkbox" name="'
. $_prefix.$_fld->name . '" id="' . $_prefix.$_fld->name . '_field" value="1" '
. ($_return['fields'][$_fld->name]['value'] ? 'checked="checked"' : '')
. ($_fld->required ? ' class="required"' : '') . ' />';
break;
Despite agreed form field being marked as required, it needs to get validated at saveData to check if no 'agreed' post variable is sent, that's the case of forced user registration using some kind of method to avoid the form javascript validation
components/com_virtuemart/controllers/user.php funciton saveData line 216
Add theese lines

Code: [Select]

if(empty($data['agreed'])) {
    $msg = JText::_('COM_VIRTUEMART_USER_FORM_BILLTO_TOS_NO');
    vmInfo($msg);
    return $this->redirect(JRoute::_('index.php?option=com_virtuemart&view=user&task=editaddresscheckout&addrtype=BT',$this->useXHTML,$this->useSSL), $msg);
}
At checkout, the config option "Must agree to Terms of Service on EVERY ORDER?" is not taken into account. Also, if you disable that option, the Agree to TOS check input is still displayed in the view and thats's not right. For that to work properly, theese are the changes that need to be done
administrator/components/com_virtuemart/models/userfields.php function getUserfield line 187 name parameter overrided because of bad logic (IE: agreed gets allways "required" = 0)
Change from this

Code: [Select]

if (empty($this->_data)) {
$this->_data = $this->getTable('userfields');
if($name !==0){
$this->_data->load($id, $name);
}
$this->_data->load($id);
}To this...

Code: [Select]

if (empty($this->_data)) {
$this->_data = $this->getTable('userfields');
if($name !==0){
$this->_data->load($id, $name);
} else {
    $this->_data->load($id);
}
}
Config Parameter agree_to_tos_onorder (Must agree to Terms of Service on EVERY ORDER?) should be evaluated here, because if it's turned on in config by VM administrator, it doesn't matter if the user agreed to TOS, he must agree on every order because of that VM config setting.
components/com_virtuemart/helpers/cart.php function saveAddressInCart line 1255
Change from this

Code: [Select]

if(!empty($data['agreed'])){
$this->tosAccepted = $data['agreed'];
}To this...

Code: [Select]

if(!empty($data['agreed']) && !VmConfig::get('agree_to_tos_onorder',0)){
$this->tosAccepted = $data['agreed'];
}
Terms Of Service Checkbox should be visible in cart only if "Must agree to Terms of Service on EVERY ORDER?" is checked in VM configuration. For this to happen, the string COM_VIRTUEMART_CART_TOS_READ_AND_ACCEPTED should be splitted into two strings just in case the VM administrator wants to show TOS at cart and not require TOS agreement on every order. Theese are the new strings that should be placed in language/en-GB/en-GB.com_virtuemart.ini
COM_VIRTUEMART_CART_TOS_READ="Click here to read terms of service"
COM_VIRTUEMART_CART_TOS_ACCEPT="and check the box to accept them"
The cart view should be changed too
components/com_virtuemart/views/cart/tmpl/default.php arround line 182
Change from this

Code: [Select]

if ($userFieldsModel->getIfRequired ('agreed')) {
if (!class_exists ('VmHtml')) {
require(JPATH_VM_ADMINISTRATOR . DS . 'helpers' . DS . 'html.php');
}
echo VmHtml::checkbox ('tosAccepted', $this->cart->tosAccepted, 1, 0, 'class="terms-of-service"');

if (VmConfig::get ('oncheckout_show_legal_info', 1)) {
?>
<div class="terms-of-service">

<label for="tosAccepted">
<a href="<?php JRoute::_ ('index.php?option=com_virtuemart&view=vendor&layout=tos&virtuemart_vendor_id=1', FALSE) ?>" class="terms-of-service" id="terms-of-service" rel="facebox"
  target="_blank">
<span class="vmicon vm2-termsofservice-icon"></span>
<?php echo JText::_ ('COM_VIRTUEMART_CART_TOS_READ_AND_ACCEPTED'); ?>
</a>
</label>

<div id="full-tos">
<h2><?php echo JText::_ ('COM_VIRTUEMART_CART_TOS'); ?></h2>
<?php echo $this->cart->vendor->vendor_terms_of_service; ?>
</div>

</div>
<?php
}
}To this...

Code: [Select]

            if(VmConfig::get('agree_to_tos_onorder',0)) {
    if ($userFieldsModel->getIfRequired ('agreed')) {
    if (!class_exists ('VmHtml')) {
    require(JPATH_VM_ADMINISTRATOR . DS . 'helpers' . DS . 'html.php');
    }
    echo VmHtml::checkbox ('tosAccepted', $this->cart->tosAccepted, 1, 0, 'class="terms-of-service"');
    }
}
if (VmConfig::get ('oncheckout_show_legal_info', 1)) {
?>
<div class="terms-of-service">

<label for="tosAccepted">
<a href="<?php JRoute::_ ('index.php?option=com_virtuemart&view=vendor&layout=tos&virtuemart_vendor_id=1', FALSE) ?>" class="terms-of-service" id="terms-of-service" rel="facebox"
  target="_blank">
<span class="vmicon vm2-termsofservice-icon"></span>
<?php 
                            $tosText = VmConfig::get('agree_to_tos_onorder',0) ? JText::_('COM_VIRTUEMART_CART_TOS_READ').' '.JText::_('COM_VIRTUEMART_CART_TOS_ACCEPT') : JText::_('COM_VIRTUEMART_CART_TOS_READ');
echo $tosText; ?>
</a>
</label>

<div id="full-tos">
<h2><?php echo JText::_ ('COM_VIRTUEMART_CART_TOS'); ?></h2>
<?php echo $this->cart->vendor->vendor_terms_of_service; ?>
</div>

</div>
<?php
}
Well, that's all, hope it gets implemented ASAP.

Best regards!



[attachment cleanup by admin]

[AH]

Have you put this into the SVN?

For  consideration and inclusion in next release?

[slammy]

thank you! Did test your patch and it´s working! Nice work! The only thing I noticed is that COM_VIRTUEMART_CART_TOS_READ_AND_ACCEPTED is now displayed in english language in my cart-view allthough I have COM_VIRTUEMART_CART_TOS_READ_AND_ACCEPTED in my german language file ...

[razor7]

Have you put this into the SVN?

For  consideration and inclusion in next release?

Hi Huston!, no, I don't have SVN commits access rights. It would be great that VM source get's switched to GitHub by the way!

PS: Can you tell Milbo about this patch?, I sent him a PM, but just in case...

[razor7]

thank you! Did test your patch and it´s working! Nice work! The only thing I noticed is that COM_VIRTUEMART_CART_TOS_READ_AND_ACCEPTED is now displayed in english language in my cart-view allthough I have COM_VIRTUEMART_CART_TOS_READ_AND_ACCEPTED in my german language file ...

You're welcome!

Regarding the lang string, you may double check your modified view for wrong strings and your lang files. Remember that for this patch to work, two new strings are added, to split the old string containing the TOS string.

[slammy]

Regarding the lang string, you may double check your modified view for wrong strings and your lang files. Remember that for this patch to work, two new strings are added, to split the old string containing the TOS string.

ah ok, adding the two strings in my language file and it works perfect now - sry I did overread the info in your tutorial, big thx for your fix!!!

[Milbo]

It is a nice fix, but the whole logic got overhauled for vm3 and I wont change it for vm2, because it was all the time that quirky way.

[AH]

Ok so if using VM2 we need to consider applying this patch ourselves.

[Milbo]

My problem is only, if it changes the behaviour for already running stores. Vm2.6 must be absolutly backward compatible!

[AH]

Milbo

I understand the need to retain backward compatibility

I will have to test this to see what happens in my test version and if it is anything like the report below

First, when registering using VM registration form, I can register without checking the "I agree to the Terms of Service" check box! is that correct?
I expected VM to validate that check and to cancel user registration if user didn't accepted Terms of Service

Second, I have checked Must agree to Terms of Service on EVERY ORDER? and Show Terms of Service on the cart/checkout? in VM configuration, despite having that options enabled, I still can buy things without checking the Terms of Service checkbox at checkout!

I have bigger issue with the Joomla bind user loophole in VM1.5.26 and VM 1.1.9  so it will take me a few days to get to it.

[nkamp]

Hello,

I have added this changes as well. One time just copying the files over the existing files. And one time I made changes all by my self manually in the files. But it still doesn't work.

Even the TOS link is not working at the checkout. When I click on the link of TOS, at my checkout, then it returns to the home page. I have also tried the OPC of linelabox, but the same issue.
I don't know why it is not working.

I use joomla 2.5.25 with vm 2.6.10.

Nico

[Shazam0527]

I hope this doesn't come across as stupid but how to Patch this?  Do I use the Upload Package file portion in the Extensions Manager?  When I do that it says it's missing the XML.  How do I patch as I'm having the same issue.  (Sorry I am relatively new to Virtuemart and Joomla)

Thanks in advance,

Jonathan

[GJC Web Design]

just d/L the the file
unzip
in it you will see the files you need to over write on your server

the patch is only for the devs on the SVN

use with caution

[Shazam0527]

Well crud, yeah that didn't work.  Is there a fix to this issue?  It's utterly dire that they are required to Agree to the TOS for this client.  Without it, this setup is useless.    And I am a HUGE fan of VM