Customer Specific code in .phtml files

srenon commented 5 years ago

/view/frontend/templates/analytics/initialize.phtml

    <script text="text/javascript">
        var _learnq = _learnq || [];
        <?php if ( $this->isLoggedIn() ): ?>
            _learnq.push(['identify', {
                $email: "<?php echo $this->getCustomerEmail() ?>",
                $first_name: "<?php echo $this->getCustomerFirstname(); ?>",
                $last_name: "<?php echo $this->getCustomerLastname(); ?>"
            }]);
        <?php endif; ?>
    </script>

You should NOT load customer specific code in phtml files because that block will get cached by FPC and then every customer will get the same content. So after cleaning cached if the first person who visits your site logged in then the there is a strong possibility that all other users who view page source will see that user info. This could become a privacy issue and also the wrong info will be sent to you. You should instead use Javascript to set this information

remstone7 commented 5 years ago

Thanks, taking a look at this one!

srenon commented 5 years ago

A quick fix would be to create a plugin for Magento\Customer\CustomerData\Customer

public function afterGetSectionData(Customer $subject, $result)
{
    if ($this->currentCustomer->getCustomerId()) {
        $result['lastname'] = $this->currentCustomer->getLastname();
        $result['email'] = $customer->getEmail();
    } else {
        $result['lastname'] = '';
        $result['email'] = '';
    }
    return $result;
}

Then move that logic to a JS files

define([
     'Magento_Customer/js/customer-data'
], function (customerData) {
     var customer = customerData.get('customer')()
     var _learnq = _learnq || [];
     if  (customer.email) {
           _learnq.push(['identify', {
            $email:  customer.email        ,
            $first_name: customer.firstname,
            $last_name: customer.lastname
        }]);
 ....

remstone7 commented 5 years ago

@srenon doing some research, it looks like a dependency injection is okay to utilize as it seems common practice? https://www.etatvasoft.com/blog/dependency-injection-in-magento-two/ & https://devdocs.magento.com/guides/v2.3/extension-dev-guide/depend-inj.html

I did some testing and doesn't appear cacheing will cause an issue as i've been able to trigger this through acting as multiple users without any issues.

Note: can add this in a future release to extend to header.container and utilize the JS method though

srenon commented 5 years ago

If you are using advance Cloudflare setting or any other caching service then the first person who visits the site page will get caches after clearing the cache.

If this was the acceptable approach why did Magento use js to load the welcome message in the header?

klaviyo / magento2-klaviyo

Customer Specific code in .phtml files #29