eShop
eShop copied to clipboard
Storing credit card information in the clear is a problem
Nice work on eshop/Aspire! I was looking through the code to get a better handle on Aspire and other things when I came across this migration:
https://github.com/dotnet-architecture/eShopOnAzure/blob/master/src/Services/Ordering/Ordering.API/Infrastructure/Migrations/20170208181933_Initial.cs#L91
It looks like a table is created in the database with columns to store card number, expiration date, and security code - all in the clear. Looking at the command/model for saving this information:
https://github.com/dotnet/eShop/blob/main/src/Ordering.API/Application/Commands/CreateOrderCommand.cs#L78
It doesn't look like there's any encryption or attempt to conceal private information for the card or the user. Running the app and placing an order confirms this:
This is kind of a Big Deal when it comes to PCI Compliance which many companies are held to. Moreover it's just a good idea to not store credit card information in the clear in a database :). Given that we refer to this as a "reference app" it might be a good idea to do the needful for people so they don't accidentally do Bad Things.
Recommendations
One idea that we could do here would be to call out, explicitly, in the API that we're not logging/storing the credit card information. At that point you could return a FakePayment object, for instance, with the information the app needs.
Another idea would be to stub out a Stripe Webhook Receiver with Stripe Checkout integration. Easy to do and I would be more than happy to PR that!
Hi @robconery, thank you for pointing this out - I agree with you that this should be fixed, since this sample should not guide people down the wrong path.
In this case, I think faking the payment call is the right approach (your first suggestion), but we should ultimately adopt an API pattern which is similar to Stripe's, completing payment via another site, with a Webhook to call back into the app.
Cheers @ReubenBond I'm hoping to help on this as I can.
Thank you, @robconery! If you are willing to open a PR, it would be greatly appreciated!
@SrushtiPasari94 Are you working on this one?
Raised PR https://github.com/dotnet/eShop/pull/326
Close Srushti's PR for now, talking with Rob looks like we will look into something else.
Srushti verified that right now if you don't add card details then it will break our VerifyOrAddPaymentMethod so that would have to be adjustred or something new