Commerce Cybersource Hosted Order Pages (HOP) Payment Gateway

Welcome!

you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

Review of the 7.x-1.x branch:

• ./commerce_cybersource_hop.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function VerifyTransactionSignature($message) {
  

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

On line 115 you could watchdog an error if the file's not found.

 126   array_walk($_POST, 'check_plain');

What's the purpose of the blanket check_plain? The standard is to filter on output http://drupal.org/node/263002 - Doing a check_plain isn't magic and may even unintentionally break things. For example, the content on line 145 is getting check_plained by the use of t(@) and has already been check_plained so it might get double encoded. Ditto on line 155.

Not sure if it's a micro-optimization, but the array in commerce_cybersource_hop_reason_code could be stored in a static if there's a chance it gets called multiple times.

On line 128 I suggest watchdogging the signature failure and perhaps even adding flood control. As it is this callback could potentially be used to brute-force the site's key.

Taking a look at the project I see that:

You will need to download the HOP.php from the cybersource business center.
Under "Tools & Settings" and "Hosted Order Page" click on "Security". Then under
the "Generate Security Script" heading sleect "php" then click "Submit". The
file that is downloaded will need to be placed inside of the includes folder
in this module. Then you will need to open it up and add the following line
underneath "<?php": "namespace HOP;" then save the file.

It is generally not good practice toa configuration file like this to be added to the contributed module folder. Is it possible to place this file in sites/*/libraries/?

@megensel

Were you able to move the hop.php out of the module dir?

#7 looks like a good improvement. To be super-paranoid you could watchdog and fail out if the require_once ever fails.

There are some code review issues that should ideally be fixed and should hopefully be quick - http://ventral.org/pareview/httpgitdrupalorgsandboxmegensel1613584git-7x-1x

Hi megensel,

I am working with @Everett and @Greggles on the same project. We are trying to determine what POST-back URL to use inside the Cybersource Business Centre in order to ensure that it posts back to Drupal Commerce properly. Are you able to confirm? Is it something like domain.com/cart/checkout/complete? We have only the ability to specify a URL without tokens and for that reason I'm unsure how we can post back. The default implementation of the hosted order page just shows a link to click that takes you to your success page and of course that means that a POST isn't actually happening. The $_POST variable ends up being empty (as it should be) and therefore the desired outcome doesn't actually occur.

Thanks in advance for your thoughts.

Initially you can use http://api.drupal.org/api/drupal/modules!system!system.api.php/function/... to forece the presence of the file, but that doesn't cover the use-case of it being removed / moved post-install.

@megansel,

Thanks for your response. I've found you're correct in that the actual post-back URL is sent in the initial POST from the module and is created programmatically, but I've found that I actually don't have to specify any URL in the Cyberseource Business Centre after all. It seems to be posting to that programmatically created POST back URL no matter what (which is good) but we've run into the access problem that @greggles is discussing with you on IRC.

You need to set the status to "needs review" if you want to get a review.

We had to write a menu hook in order to expose a path that Cybersource could use to post back data. We would like to contribute back some code but we are just awaiting IP clearance from our client. You'll certainly hear back from us soon. =)

Subscribe

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

I'm sorry for the delay!
There are about 100 other applications waiting for review and only a hand full of active reviewers.
We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

// oops, too late ;D

Only a few minor issues after reviewing:

• Fix misspelling in flood event name - should be 'failed_transaction_signature'
• Add a check for product reference when building line item params in commerce_cybersource_hop_order_form(). This will ensure that you are exporting product line items.

Suggestions:

• Change setting for 'cybersource_hop_send_line_items' to have options of [0, 1] to make boolean checks easier
• Create a common utility function to resolve first and last names from an address field array. Then it could use to clean up commerce_cybersource_hop_order_form() for setting billing and shipping address

looks good

At what point will this move under drupal.org/project?
This module worked fine for me.

Cybersource added a new return field to the HOP in early September.
There is now a new "orderPage_environment" field returned back to tell whether the transaction came from a production or testing merchant gateway.
Without this someone presumably can spoof a payment by capturing the data going to the HOP and sending it through the test HOP which if enabled and configured the same way would tell the store to push through the transaction.

Here are the details from:
http://apps.cybersource.com/library/documentation/dev_guides/HOP_UG/html...

orderPage_environment

Indicator of which environment the transaction was processed on. Contact CyberSource Support to disable this field.
This field contains one of these values and prevents fraudulent users from obtaining approval for purchases that would otherwise be rejected:
■

PRODUCTION—The transaction was submitted to the production environment.
■

TEST—The transaction was submitted to the test environment.

I gave @megensel the "Git vetted user" role and asked him to help review two or three other pending applications.

Thanks for your contribution, megensel!

Damien updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.