-
-
Notifications
You must be signed in to change notification settings - Fork 436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/Fix for SameSite Cookies, option to set to None during 5 minutes afte… #922
Conversation
…r order placement so that servies like 3-secure can post back to magento, and chrome+magento is able to read the session cookies so an order is created
Thanks for the PR! Does setting two cookies with identical parameters but different SameSite value cause one cookie to be overridden or two cookies? I'm guessing one cookie to be overridden because otherwise the server could only receive one value anyway due to conflicting key names. In that case this has an issue in that the cookie will either expire causing the user to be logged out, or the next cookie renew will just reset the SameSite status and it might not have accomplished it's goal yet. I would advise leaving the frontend cookie alone and instead introducing a special cookie that is only recognized by the necessary endpoints. E.g. 'frontend_adyen' so when this special cookie is given and if it is validated (e.g. with an HMAC) then the correct session could be loaded. |
for the security point of view, is there any case where the session cookie should not be none? |
NP!
Yes setting a cookie with the same name as an already existing cookie overwrites it. When magento renews the cookies when you come back it sets back the lifetime to whatever you have configured. In our case 12h.
The problem is that Magento stores the Session ID in the cookie This leads to Magento not knowing who you are, no order is being created but if everything went fine on the third party system the customer has paid for the goods but he/she won't receive any order because Magento isn't aware that any order was placed. So my simple solution is to change the cookie to SameSite=None upon order placement and change the expiry time to 5 minutes. So the customer has 5 minutes to pay for the order and return to Magento. Then the cookie is set back properly to SameSite=Lax and whatever expiry time you have configured in Magento. You could ask 3D-secure and all Banks to do GET / QueryString instead but this leaves data exposed on FE which is insecure. Best would be if all data related to the order came inside the POST data but how is 3D-secure supposed to know all data needed for Magento and I don't see this happening any time soon. You could also make this POST request redirect to another own controller with the same POST data but then you have just undone the added security this is supposed to add. |
I think I understand the issue, I'm just proposing a solution that fixes two downsides to the approach in this PR:
So my suggestion is to instead add a new cookie that is safe to expose via SameSite=None and that does not need to expire quickly but can take the place of the |
This issue is now critical for 3D secure to be maintained in Chrome. Having issues with SameSite being set, even with this added feature. Looking into it further. |
@kestraly You are probably not using the standard Magento checkout or you have a rewrite on Mage_Checkout_OnepageController:: saveOrderAction You need to make sure that whatever you have uses this same logic. |
It was the default checkout. Didn't seem to add the SameSite tag Lax or None. Will keep digging. I put the temporary fix in place for cookie path: /;Secure;SameSite=None which does mark the cookie as SameSite None. So at a bit of a loss as to why this fix doesn't mark it as Lax or None. |
Should have looked into this a bit more earlier. This method only works with PHP 7.3 onwards, otherwise the example below would need to be deployed.
|
Hello folks! Nice code you have created. What is the status? Any help needed to finish this up? |
is somebody using this patch in production? since it's 2 years old and not merged. also
|
I say close as IMO this PR does more harm than good and there is no progress. |
…r order placement so that services like 3d-Secure can post back to Magento, and Chrome+Magento is able to read the session cookies so an order is created.
Magento automatically reverts back to 'Lax' when you come back to the Success page (or any other page).
This is a major issue if you use Adyen for example with credit card iframes in the checkout.
See issue description here: