I've identified and verified in the code the following logic bug:

Inside the file uc_credit.module, inside the the routine uc_credit_settings_form_validate(), the following logic controls if a credit card encryption key will be created:

  $dir = variable_get('uc_credit_encryption_path', t('Not configured, see below.'));
  $filename = rtrim($dir, '/\\') .'/uc_credit.key';

  if ($form_state['values']['uc_credit_encryption_path'] !== variable_get('uc_credit_encryption_path', t('Not configured, see below.'))) 
  {
     validation logic insuring that the specified encryption path is valid and writable
  }

The above "if" conditional contains a subtle, and practical logic bug that occurs when an eCommerce site is ported from one host to another, or from one version of Drupal to another...

Consider the following:
1) Web site's eCommerce operates fine, but for one reason or another the site is rebuilt or moved to another host
2) the web site's database is backed up, along with the credit card encryption key filepath
3) the web site is recreated on a new host, or recreated from scratch for a new version of Drupal (D5->D6, D6->D7)
4) the backed up database is restored (or migrated in the case of a new version of Drupal)
5) site admins proceed to verify that all the eCommerce settings are configured, including the credit card number encryption key filepath... assuming that the old filepath was fine, they create the same path on their new server...

logic bug:

6) when the form at admin/store/settings/payment/edit/methods is submitted, the uc_credit_settings_form_validate() code I show above will compare the previous (working) filepath against the new (but same) filepath and assume that the encryption key file already exists, and no new (and needed) encryption key file is created.

I suggest different logic that does not assume if the path is the same, the encryption key already exists.

Comments

vadym.kononenko’s picture

StatusFileSize
new821 bytes

I think this is a bug. Try the attached patch to fix it.

tr’s picture

Status: Active » Needs review
tr’s picture

Version: 6.x-2.2 » 6.x-2.x-dev
Status: Needs review » Needs work

Let's sic the testbot on this one. It's going to fail due to the age of the patch, but it's a start ...

tr’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, uc_credit.module.validation_ignore_fix.patch, failed testing.

tr’s picture

Assigned: Unassigned » tr
Status: Needs work » Patch (to be ported)

Fixed in 7.x-3.x by commit ebcb007c. I took the opportunity to rewrite the validation and submit handlers for the uc_credit encryption key filepath. I verified that the new code doesn't have the problem described above. I will *probably* backport this to Ubercart 6.x-2.x.

bsenftner’s picture

@TR: please do backport it. There are a lot of D6 Ubercart sites out there, including several of mine!

longwave’s picture

Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.