card number shouldn't go to drupal

Does anyone have a response to this? I'm considering using Stripe, but if the current Drupal module nullifies what makes Stripe so special, then what's the point? I would love to hear some additional feedback...
-JB

Pushed the following to 7.x-1.x-dev:

- Removed "name" attributes from Stripe related input fields to prevent card data to be sent to Drupal server (see https://stripe.com/docs/tutorials/forms): "The only thing to note is how input fields representing sensitive card data (number, CVC, expiration month and year) do not have a "name" attribute. This prevents them from hitting your server when the form is submitted. "

- Made Stripe related credit card fields not required to avoid validation errors when submitting the form

Other fixes:

- The click event on credit card form worked only once, fixed by changing .click(function(event) to .live('click', function(event)

- Adding "auth-processing" css class only when Stripe payment method is selected

- Saving unserialize-safe transaction payload data to avoid spl_autoload warnings (now saving json-data instead of Stripe objects)

TODO: Prevent expiration date from being sent to Drupal server

Would appreciate some testing effort, if someone has time :)

Small change still: Removing name attributes from Stripe inputs in click event instead of removing them in server side code, since there are problems when offering multiple payment methods and when switching between them using ajax (form caching, the name stays removed for all methods :()

This really needs to be rolled back. Stripe has the capability to not send the card info through the web application, but it's certainly not a requirement. This completely breaks any possibility of using commerce_cardonfile + commerce_recurring at all (since the card information needs to actually be processed by Drupal in order to store the card information and allow for recurring billing).

If you really want this feature, it should be an option and NOT a requirement to use this module.

I propose that this is rolled back until a better (more optional) solution is reached.

I'm going to have to disagree with @cweagans in #10 here.

Obviously there are conflicting needs here. It's great to be able to download and install a Drupal module to handle recurring billing or cardonfile. That's a specific use-case that some people have, but...

Many of us have chosen Stripe because it simplifies PCI compliance.

Anyone who processes credit cards are contractually obligated to meet the PCI-DSS standard. While there's a heated debate, on this, my sense from Commerce Guys is that there isn't really a PCI compliant way for Drupal to handle a credit card. Even if there was, the steps to properly secure a server to store a credit card require more time and resources most than most people have to spend configuring their servers.

Ultimately, this is a question of risk and risk aversion. The Credit Card companies can essentially shut you down if there's a breach and you don't meet PCI standards.
They can take away your ability to process credit cards, and fine you tens of thousand dollars.

So, for almost anyone using this module, this patch is exactly what we need.
For the few people who want to either disregard PCI or who believe they actually have the resources to secure their servers to this level, let's have a separate module that captures card numbers, stores them and allows recurring/card on file transactions.

The whole idea behind Stripe is that you don't store any credit card information on your own server so that you can achieve PCI compliance in an easy way. So the correct way is to save the customer data to Stripe and then save the customer reference to Drupal https://stripe.com/docs/tutorials/charges#saving-credit-card-details-for.... This way no credit card information is saved on your server but you can handle subscriptions/recurring payments in Commerce with Stripe. So if commerce_cardonfile + commerce_recurring does not support this method the correct way is to patch those modules (maybe just patching commerce_recurring is enough?) so that we can still achieve PCI compliance.

Sorry to be a pest, but there's a few counterpoints:

1) Even if the card number doesn't pass through Drupal, if your server is compromised, it won't take much for a hacker to make that not the case. This patch does not automatically secure against this.
2) If card data DOES go to Drupal, by default, we're not actually storing the data in Drupal (only the last 4)
3) With commerce_cardonfile, there is a strict requirement to use card storage methods provided by the payment provider - for instance, authorize.net's customer information manager or Stripe's storage mechanism.
4) Drupal sites have been PCI compliant before. What makes Stripe so special that we have to change how card data is handled to continue having PCI compliance? Does this mean that all Drupal sites should switch to Stripe to be compliant (since, as far as I know, Stripe is the only payment gateway doing something like this)?

The entire point, though, is that the card number must go through Drupal because the Stripe storage mechanism depends on using that API (Which can't be used from the client side). We have to be able to call that API from PHP and store the customer ID in the database, and we can't do that with the token from stripe.js.

For the few people who want to either disregard PCI or who believe they actually have the resources to secure their servers to this level, let's have a separate module that captures card numbers, stores them and allows recurring/card on file transactions.

Recurring transactions in a PCI compliant way is easy to do with Stripe, but it does require that the card number makes it to the server so that we can call the API I mentioned before.

And again, I'm not saying remove this feature, as it's obviously important to some people. I'm saying make it optional to cover all bases.

Re: #12, the commerce_cardonfile support patch at https://drupal.org/node/1541074 doesn't remove the protections mentioned in #4 above (removal of name attributes in Stripe form elements). It's still using the Stripe token for the initial Server-side communication with Stripe. It is storing some of the response from Stripe (e.g. last four digits, expiry date, Stripe card id, etc.) with commerce_cardonfile.

@cweagans - maybe i am missing something, but it looks to me like this is a non-issue.

Stripe has the capability to not send the card info through the web application, but it's certainly not a requirement. This completely breaks any possibility of using commerce_cardonfile + commerce_recurring at all (since the card information needs to actually be processed by Drupal in order to store the card information and allow for recurring billing).

The patch provided in https://drupal.org/node/1541074 looks to me like it enables Card On File support (and by extension, Commerce Recurring support) while still avoiding sending any sensitive card data to drupal. The last 4 digits, name, and exp date are stored in Card On File along with a Stripe remote ID for the card that allows you to run future charges via API. What is the problem with this?

I guess another way of asking the question would be:

Assuming we can make Card On File/Recurring work without having the card data ever hit drupal, would that address your concerns?

The entire point, though, is that the card number must go through Drupal because the Stripe storage mechanism depends on using that API (Which can't be used from the client side). We have to be able to call that API from PHP and store the customer ID in the database, and we can't do that with the token from stripe.js.

This is just plain not true. You use Stripe.js to create the token, you post the token to Drupal, Drupal passes the token to Stripe using Stripe's PHP library, and Stripe returns a permanent ID that can be used to call that same card back up in their API. The card data never touches Drupal at any point in this process (except last 4, exp data, name), and it totally works with Card On File.

Okay. The Stripe API used to not let you use tokens for stored card data. So as a compromise, I'd be happy with that.

However, I still don't see the point of using stripe.js in the first place. It's no more secure than just sending the card data over SSL, because if your server is compromised, a hacker can just capture the tokens and comment out the line where it's sent to Stripe, then proceed to use those tokens for pretty much anything (such as storing the card data using the customer API, and charging a small amount of money on the card every month).

Also, how does this perform if Javascript is disabled?

If your server is compromised to the point where they can change your codebase in arbitrary ways without you knowing, then you are utterly screwed. If that is your standard of comparison, then you should just never worry about security ever, because nothing you can ever do will possibly save you if that type of breach has already occurred.

Using Stripe.js just adds an extra little layer of security, mainly because it prevents people from doing stupid things. If the data is being passed to Drupal, it could potentially be stored in Drupal, and that is undeniably a bad thing. This can happen in unexpected ways. For example, it could inadvertently be stored in a log somewhere. Given that there is zero downside to it, i dont see what could possibly be a valid objection to using it.

If javascript is disabled, the checkout form breaks. I am pretty sure that is the case anyway, as several core features (like switching countries in the address, for example) depend on ajax. I do not see that as a problem.

If you would like to write a patch that turns this in to an option instead, please feel free to do so, although it would merit some more community discussion before accepting. I would personally be opposed to accepting such a patch, because it serves no positive purpose that i can see, and introduces some potential security issues (however minor they may be).

The point here is not really about security at all, you're right, if your server is hacked, and codebase changed the gig is up. This is about the arbitrary standards that the PCI council have identified as necessary and required if a full credit card number is every submitted/stored to your server.

The equation is pretty simple.
You signed a contract (you read those terms of service right?) with Stripe or with your bank includes agreements with all the major credit card brands. Your contract with card brands says, if a card number is on your server, you will take these 200+ steps to secure it. (It includes having an encryption key for your encryption keys - seriously).
For most of us, we're small enough that, if you ignore that contract with the major card companies, it's not a big deal, nobody checks, you probably sign something from Stripe or your bank saying you're compliant, no big deal.
But, let's say some algorithm at one of the card companies sees that a handful of cards you processed, ended up stolen.
The card company decides to audit you. You get to (at your expense) hire a firm to audit your security. If you come up lacking in any of those 200+ criteria, you get to pay a major penalty, a five figure "fee" - (Remember the card companies are not governments, these are fees, not fines). Let's say you had a file sitting on your server with a few CCV numbers - you get to pay a "fee" for every single CCV number you had on file. If your setup is bad enough, let's say you didn't hash the card numbers, or you used unsalted MD5 they take away your ability to process cards. Period. Ever. (You didn't really need that did you?)

In other words, if you're not compliant, the prospect of getting a card number stolen off your server isn't necessary what you should worry about, you should worry more about being audited by the card companies if you're not compliant. But then of course who else should pay for credit card fraud? It's not like the credit card companies have any money or anything? It's a lot better for small merchants to get shut down or crippled paying for it.

To expand to selfuntitled's point, i think PCI was intentially designed to be an impossible standard. There is no "if you are not complaint." You are not complaint, ever.

wikipedia:

According to Visa Chief Enterprise Risk Officer, Ellen Richey, "...no compromised entity has yet been found to be in compliance with PCI DSS at the time of a breach."

So basically the way it works is that a breach occurs, they audit you, they find you out of compliance (every time), and they pass the liability off to you. Totally crappy and unfair, but so it goes.

Cross-posting from https://drupal.org/comment/8387841#comment-8387841:

...

Rather than manipulating a new DomDocument with an #after_build callback, a #post_render callback that unsets the #name property should suffice.

This works:

$credit_card_field['#after_build'][] = '_commerce_stripe_credit_card_field_remove_name';

...

function _commerce_stripe_credit_card_field_remove_name($element, &$form_state) {
  unset($element['#name']);
  return $element;
}

thanks MEsch, i've made that change in my repo.

After a bit more testing, it turns out that the approach suggested in #24 has a side effect of also removing the class name from the wrapper div around each input/label pair (for example form-item-commerce-payment-payment-details-credit-card-owner). I am guessing that this is not an acceptable side effect.

On top of that, it turns out that the DOMDocument approach i was originally taking requires php 5.4, so i have to revise that a bit to make it work on php 5.2.5, which is the minimum for D7.

Unless someone has a better suggestion, i am going back to (a slight variation on) the #post_render / DOMDocument approach.

actually, just doing regex string operations on the html in #post_render seems to be the best way. the DOMDocument just complicates things.

ok, my current version looks like this:

<?php
$credit_card_field['#post_render'][] = '_commerce_stripe_credit_card_field_remove_name'; 
...
function _commerce_stripe_credit_card_field_remove_name($content, $element) {
  $name_pattern = '/\sname\s*=\s*[\'"]?' . preg_quote($element['#name']) . '[\'"]?/';
  return preg_replace($name_pattern, '', $content);
}
?>

All this patches on top of patches on top of patches stuff is getting to be more than my feeble mind can handle, so i am just committing stuff to my sandbox at this point. If anyone wants to cherry-pick any of the commits on there, i will be glad to help.

Can we safely assume that card data is not stored and not passed back to Drupal in the clear - unless we want it?

@Jons - In the most recent release, all card data except the name, exp date, and last 4 digits go directly to Stripe, and never touch Drupal. You would have to modify the module to change this behavior.

thanks.
I'm not passing address info in, and I noticed that the country is being set in payload - the default: 'US'.
I'm not in US. Can this be null or should it be set to UK etc?
Payload extract (from transaction record)

....
 1392375950] => {
    "id": "ch_103UjF2AGXMjWmxxxxx",
    "object": "charge",
    "created": 1392375952,
    "livemode": false,
    "paid": true,
    "amount": 19900,
    "currency": "gbp",
    "refunded": false,
    "card": {
        "id": "card_103UjF2AGXMjWxxxxx",
        "object": "card",
        "last4": "4242",
        "type": "Visa",
        "exp_month": 2,
        "exp_year": 2014,
        "fingerprint": "Z21UwVYxxxxx",
        "customer": null,
        "country": "US",
        "name": "S ssss1",
        "address_line1": "undefined",
        "address_line2": "undefined",
        "address_city": null,
        "address_state": "unde ....
...

Jons - i started a new issue for your question: https://drupal.org/node/2196473

Please restrict this thread to discussion about the feature, and start other threads for bug reports/support requests.

Just looked at the module and it does appear that Stripe.JS functionality is included. Line 161 of commerce_stripe.module:

  // Include the stripe.js from stripe.com.
  drupal_add_js('https://js.stripe.com/v1/', 'external');

This is awesome!

I just wanted to point out a couple of things:

1. Stripe.js is now at v2: https://stripe.com/docs/stripe.js
Any chance the module could be updated to use it? I can't seem to find a changelog for the Stripe.JS library, though.

2. It'd be great if "Stripe.JS" was mentioned in the Commerce Stripe documentation somewhere, preferably right on the main project page, since it's a major feature that enables full PCI compliance. (Right now the page just mentions "PCI Compliance" but as a developer, knowing that the module does, in fact, utilize the StripeJS library would be a big help.)

Thanks for the great work on this module.

Hi torgosPizza, could you please create a new issue proposing the switch to v2? Otherwise it might get lost in this thread.

Done - thanks! #2249225: Upgrade Stripe.JS integration to V2