OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
871 stars 436 forks source link

Remove password from account_new email template #307

Closed kkrieger85 closed 4 years ago

kkrieger85 commented 7 years ago

It is not 2017 to send choosen password in email:

https://github.com/OpenMage/magento-lts/blob/1.9.3.x/app/locale/en_US/template/email/account_new.html#L25

Flyingmana commented 7 years ago

only applicable if the email authentification is forced to be always on (actually I dont know how the state is for this currently, I just know people who disabled it for conversion improvements)

But yes, Iam absolutely for this change

tomekjordan commented 7 years ago

There are many studies from UX that shows that removing plain passwords for new account or password reset mail is bad for store, because customers are not able to retrieve passwords, and they are creating new accounts and whole mess. Risk for frauds is really low. Because if pass reset email is sent to EMAIL - user must login to get access - so I would not change anything and cannot takeover smb account. Each store can remove this one line in template by themself. As I said - if customer don't have access to its email - he cannot change anything - so risk is LOW.

tomekjordan commented 7 years ago

Proof: https://www.iai-shop.com/pl/shop/blog/dlaczego-podczas-przypominania-hasla-jest-ono-wysylane-klientowi-na-e-mail-1235324199 "Why is it being sent to an email when it is being reminded of a password?

In response to the questions our clients have sent us, is there an available, one-of-a-kind way to remember passwords for online shoppers at IAI-Shop.com? This is the answer why we have not decided to lose customers' passwords irretrievably by their "hashing". The basic reason why we have not decided on this solution is that the solution chosen by us simply pays for online stores, especially the bigger one. For this reason too, we have tried not to betray these reasons as long as possible.

By doing our daily work we face thousands of technological decisions. While making them, not only do we reflect on our 12-year experience, but also on the behavioral analysis of clients and their clients. One such observation is that a typically organized reminder, previously "uploaded" password is based on sending a link email, upon which users must enter a new password. As our experience shows, a few years ago, with such an organized process, it is simply more convenient to set up a new account by a customer. And with this movement, consciously or not, all efforts are undermined, for example, in rewarding customers with prizes, rebates, points or even hindering analysis of sales of goods, or advanced marketing techniques such as recommendation systems or targeted email marketing. These and other solutions are sewn at IAI-Shop.com. So if we were to put on a client account every time a new account was created, what would we build these systems for?

Therefore, hogging the passwords of customers in the online store, in our opinion, is a bad idea and using this solution effectively reduces turnover on returning customers.

It is therefore worth to consider whether having a password has some benefit for the customer in terms of security? In our opinion yes so long as the password is needed to do something important and the customer is forced to reuse the same account. If we forced customers to recover their passwords, our online stores would lose even more money. Interestingly, some shop platforms do so. The same applies to "fast login" links. If a password or link leads to an admin panel of a store or, for example, to an account in an auction site where even a joke, the person who takes the password will click "Buy Now" will have serious consequences. In the case of an online store such consequences are absent because:

At IAI-Shop.com there is only payment information, but payments are made at other sites (eg banks or card providers). The customer account at best allows you to suspect the status of payment and the content of the order. In order to pay for your order, you have to log in again at your internet bank (without IAI) and pay your order each time. The password reminder will be located in the same customer email box where you are legally required to send a sales note, including a list of the goods and the prices at which they were purchased. So if a burglar takes control of a client box, logging in to the store will not get information that he or she no longer has when hacking into the email box. "Speedlifting" links allow faster access to your order and checks. Thanks to this the customer can once again check everything. The draft of the new law on providing electronic services for many years has even assumed the obligation to provide such an overview. If the customer does not check the revised order and there is an error in the interpretation of his expectations, the store risks a costly refund procedure, and the legislator even prepares the statutory duty to return the costs for returning the goods. Passing the password prevents it from being suspected of an SQL Injection attack. However, in this case more important is the proactive security policy, which prevents such an attack. Hiding the password in case of suspected by the attacker the contents of the database can be implemented by password encryption. In this case, the same level of security is achieved, as the attacker must also have a key to decrypt the password. This already means you have to access the application source code. If the attacker has access to the source code and database, it is better for the owner of the online store to worry about the password of the online store only. In this scenario can become everything ... It is important to recognize an account that allows you to sign in to many online stores. At IAI-Shop.com, however, every online store has its own customer database with independent logins and passwords. By analyzing the above reasons, it is easy to say that the decision to "do not quench" the passwords is conscious. We always choose scenarios strictly for a specific solution. For example, the password for the administration panels for online store operators is hashed, and there is no way to recover them or even generate a new password. For this reason, we do not recommend sending them in any way. There is not even a way to recover a lost password. It must be re-assigned by another user with administrator privileges or by IAI administrators. The administration panel is crucial and we apply adequate security to the level of security required.

Of course, as IT specialists, we are aware that it's annoying to come up with a new login and password every time, and it can lead to a single password everywhere. Just for the sake of such customers as the first store platform, we launched the ability to log in to a trusted third party account such as Facebook, Google, PayPal, OpenID. This way of login allows convenience and no need to enter the password by the shop user. Every IAI-Shop.com online store should have such a mechanism to launch what we have been advocating for many months. If such login is used, the shop will give the customer login and password automatically and randomly inform the client by e-mail with the log and password assigned. From the point of view of the attacker, the suspicion of such a password is of no value (it is random). Login and password are also randomly generated during normal store registration unless the customer completes the login or password fields.

We hope that the above arguments are enough for you to confirm that the IAI is a reliable partner who knows perfectly well not only about technology and security issues. First of all, we hope the above arguments convince you

infabo commented 7 years ago

what the...?

Flyingmana commented 7 years ago

thank you for the translation @tomekjordan , it gives a good piece of insight how this is handled by others

colinmollenhour commented 7 years ago

I think the strongest reason not to send the password via email is because email transmission is not always encrypted so the password could be sniffed from the email during transmission to the server or from the server to the client. Unfortunately many users use the same password for just about everything so this puts their potentially very valuable (but probably crappy) password at great risk. Thankfully gmail and many other ESPs have enforced encryption as a requirement but there are still many many ESPs that either don't support encryption at all for protocols like POP and IMAP or they don't strictly require them.

colinmollenhour commented 7 years ago

As the 1.9.3.x branch is the only maintained branch I closed the other pull requests. (#316 remains)

@infabo Suggested:

<strong>Password</strong>: Password you set when creating account

I like this better than removing it entirely but am OK with either. Can we get a consensus somehow?

tomekjordan commented 7 years ago

hi. it should be just commented, so if store merchant like to show the password he can easily enable it by loading and modifing template at admin area. if you remove the variables, no one will know about such "feature" to enable

colinmollenhour commented 7 years ago

Commented? Then won' the password still be in the email source?

It is still documented on line 8 even if it is removed from the body.

tomekjordan commented 7 years ago

OK. if its variable is documented you can remoce plain pass from email. thanks

damienwebdev commented 7 years ago

I feel I have a professional obligation to say this...

@tomekjordan, your reasoning and arguments are dangerously flawed. If I understand correctly, you've taken significant steps to not only falsely portray your position as a "reliable partner who knows perfectly well not only about technology and security issues", but also harm the security of your customers in the process.

I want to start with this basic premise:

As a developer, your job is to not only create useful systems, but also to design systems in a way that prevents harm to your company/client/customers.

With that said, let me break this down:

  1. We have not decided to lose customers' passwords irretrievably by their "hashing".

If I understand the translation correctly, you've decided to actually REVERT the default hashing functionality to a less secure "encrypted" password system. You've decided to add significant risk to your system in doing so. I'd advise you to read this: https://stackoverflow.com/questions/326699/difference-between-hashing-a-password-and-encrypting-it Particularly this point:

Now even if someone gets access to the database, there is no way that the passwords be reproduced or extracted using the converted-words. In this approach, there will be hardly anyway that some could know your users' top secret passwords; and this will protect the users using the same password across multiple applications. What algorithms can be used for this approach?

Instead of properly advising your client, you've decided to actively put their customers information at risk. Why would you do this?

  1. The solution chosen by us simply pays for online stores.

Clearly, this answers my previous question. You've decided to put your customers at risk for monetary benefit. You've taken the bait of a clear conflict of interest and made security-poor decision because of it.

  1. It is simply more convenient to set up a new account by a customer.

Are you trying to tell me that "Forgot Password" is uncommon or a difficult utility to optimize and make usable by your customer base? Given that it is consistent across so many different platforms, both large and small, and comes built-in to Magento, I'd argue that this is a lack of functional and clear UX work on your part.

  1. So if we were to put on a client account every time a new account was created, what would we build these (Rewards, Recommendations) systems for?

Once again, you've made a conscious decision to put client's bottom line above the security of their customer's information. There is no place, nor will there ever be, in a security discussion for the "cost" of having to do marketing. It's not a marketer's decision, they are unqualified to consider security. The have neither the training nor the knowledge.

I have difficulty understanding the paragraph that begins...

It is therefore worth to consider whether having a password has some benefit for the customer in terms of security?

so, I will skip it.

  1. At IAI-Shop.com there is only payment information, but payments are made at other sites (eg banks or card providers).

This is good security! In the case of the US, PCI-DSS standards exists lower RISK by taxing/fining/revoking access for companies who refuse to follow "minimial" security procedures. This standard is designed to PRESERVE the security of customers.

  1. The password reminder will be located in the same customer email box where you are legally required to send a sales note.

So your argument is that, if someone can hack/breach a service external to yours, they should be able to hack (I use this loosely here) into your service? This seems obviously mis-informed to me.

  1. Passing the password prevents it from being suspected of an SQL Injection attack. This sentence doesn't make sense in any context.

  2. Hiding the password in case of suspected by the attacker the contents of the database can be implemented by password encryption. In this case, the same level of security is achieved, as the attacker must also have a key to decrypt the password.

Once again, see above. Encryption is NOT THE SAME as hashing. You even somewhat make my own argument... When my application inevitably gets hacked, why risk customer password being hacked as well? With hashing, I don't have to worry about it. With hashing, I don't have to secure a "encryption key". It's one less thing for me and my customers to worry about.

  1. For example, the password for the administration panels for online store operators is hashed, and there is no way to recover them or even generate a new password.

So, you're saying once again that you value the bottom line of your company over your customer's security. You're basically sticking a middle finger to your customers and saying, "We protect ourselves, not you!"

  1. The administration panel is crucial and we apply adequate security to the level of security required.

Agreed, but clearly you don't value your customer information in the same way.

  1. Of course, as IT specialists, we are aware that it's annoying to come up with a new login and password every time, and it can lead to a single password everywhere. Just for the sake of such customers as the first store platform, we launched the ability to log in to a trusted third party account such as Facebook, Google, PayPal, OpenID.

Good on you! Assuming this is implemented properly, and you remove your own password system, this is better security. You can offload the "Identity Management" onto a vendor who's more inclined and suited to handle security than you are.

  1. If such login is used, the shop will give the customer login and password automatically and randomly inform the client by e-mail with the log and password assigned.

I take back what I said, you didn't implement it properly. You still managed to fuck it up. There's no need to manage a password for that customer any more. Remove the system entirely.

To summarize, your arguments are misinformed and frankly, dangerous. Several of things you mentioned are patently bad security and would instantly eliminate you as a vendor for any security conscious company. I'd advise you spend some time learning before you make any other arguments, and additionally, stop advising your clients on anything security related immediately.

@Flyingmana @colinmollenhour I'd strongly advise your remove the value from the email, and I would even go so far as to remove any reference to the plaintext password from the code (excluding the part where it gets hashed). We've seen Magento log plaintext passwords in system.log before and it would be a shame to have a similar kind of bug.

And, we'll wanna change this in more than one email template...

app\locale\{language}\template\email\account_new_confirmation.html
app\locale\{language}\template\email\account_new.html
app\locale\{language}\template\email\admin_password_new.html
app\locale\{language}\template\email\password_new.html
colinmollenhour commented 7 years ago

To be honest the Magento password hashing algorithm is so obsolete that it really doesn't matter much if it is hashed or not.. If you want better security you need to use password_hash function or a PBKDF2-based method like https://github.com/ikonoshirt/pbkdf2.

Nobody has perfect security because there is always a trade-off with convenience. For example, 2FA is not even supported in core and almost nobody uses it on consumer-facing ecommerce sites (the backend is a different matter where 2FA is required by PCI-DSS). PCI-DSS requirements should definitely be adhered to where applicable, but it is also quite conceivable that PCI-DSS doesn't apply for some sites (e.g. non-monetary payment forms). I'm sure Tomek has explained more times than he can count about password security to his clients but perhaps there are truly cases where the trade-off is acceptable or the client is going to ignore his advice anyway. I don't think he should be berated for it, we can just settle on a sensible default and let him do what he wants with his clients.

Thanks for taking your time to help educate the community.

On this note I've been thinking for a long time that strong password hashing and 2FA should be core features. I personally use Ikonoshirt_PBKDF2 and magento-hackathon/Magento-Two-factor-Authentication and think it would be great to have this functionality brought directly into core. Basic security protocols is one thing that really shouldn't require a third-party extension...

damienwebdev commented 7 years ago

I agree with the 2FA idea quite a bit @colinmollenhour, but I'll strongly disagree with the idea that in any world is the trade acceptable.

To your point though, Since LTS isn't adding new features, is adding 2FA in the scope of this repo?

Flyingmana commented 7 years ago

May I ask to split this into an extra ticket? And yes, the original roadmap of the LTS did include new features beginning from end of the year on. We will soon start to organize this and put this into a more official plan and roadmap with dedicated web page.

damienwebdev commented 7 years ago

@Flyingmana I'll create a new issue for the 2FA and hashing (If they don't already exist)

colinmollenhour commented 6 years ago

I would approve of:

<strong>Password</strong>: (the password you set when creating your account)
kkrieger85 commented 5 years ago

So, if I summarize correctly:

This issue should lead to changes in

app\locale\{language}\template\email\account_new_confirmation.html
app\locale\{language}\template\email\account_new.html
app\locale\{language}\template\email\admin_password_new.html
app\locale\{language}\template\email\password_new.html

To replace plaintext password with <strong>Password</strong>: (the password you set when creating your account) and also remove reference to plaintext password in email templates.

These changes should be applied to 1.9.3.x as well as 1.9.4.x branch.