Various tidy-up and TODOs comleted. DPM tests still needed.

Author: judgej

src/Message/DPMAuthorizeRequest.php

@@ -16,34 +16,25 @@ public function getData()
     {
         $data = parent::getData();
 
-        // If x_show_form is swet, then the form will be displayed on the Authorize.Net
-        // gateway, which acts a bit like the SIM gateway. The documentation does NOT
-        // make this clear.
-        // TODO: revisit this - maybe much of what is in the DPM can be used to enhance
-        // the SIM gateway, with very little in the DPM messages.
+        // If x_show_form is set, then the form will be displayed on the Authorize.Net
+        // gateway, in a similar way to the SIM gateway. The DPM documentation does NOT
+        // make this clear at all.
+        // Since x_show_form is set in the SIM gateway, make sure we unset it here.
 
-        //$data['x_show_form'] = 'PAYMENT_FORM';
         unset($data['x_show_form']);
 
-        // Support multiple currencies.
-        // CHECKME: should this be back-ported to SIMAuthorizeRequest and AIMAuthorizeRequest?
+        // The card details are optional.
+        // They will most likely only be used for development and testing.
+        // The card fields are still needed in the direct-post form regardless.
 
-        if ($this->getCurrency()) {
-            $data['x_currency_code'] = $this->getCurrency();
-        }
-
-        // CHECKME: x_recurring_billing is (ambiguously) listed as mandatory in the DPM docs.
-
-        // The customer ID is optional.
-        if ($this->getCustomerId()) {
-            $data['x_cust_id'] = $this->getCustomerId();
-        }
-
-        // The card details at this point are optional.
         if ($this->getCard()) {
             $data['x_card_num'] = $this->getCard()->getNumber();
             $data['x_exp_date'] = $this->getCard()->getExpiryDate('my');
             $data['x_card_code'] = $this->getCard()->getCvv();
+        } else {
+            $data['x_card_num'] = '';
+            $data['x_exp_date'] = '';
+            $data['x_card_code'] = '';
         }
 
         return $data;

src/Message/DPMAuthorizeResponse.php

@@ -21,6 +21,7 @@ class DPMAuthorizeResponse extends AbstractResponse implements RedirectResponseI
     protected $hiddenFields = array(
         'x_fp_hash',
         'x_amount',
+        'x_currency_code',
         'x_test_request',
         'x_cancel_url',
         'x_relay_url',
@@ -64,7 +65,7 @@ public function isTransparentRedirect()
     // Helpers to build the form.
 
     /**
-     * The URL the form will be posted to.
+     * The URL the form will POST to.
      */
     public function getRedirectUrl()
     {
@@ -76,10 +77,12 @@ public function getRedirectMethod()
         return "post";
     }
 
-    // CHECKME: do we still need getHiddenData()?
+    /**
+     * Data that must be included as hidden fields.
+     */
     public function getRedirectData()
     {
-        return $this->getHiddenData();
+        return array_intersect_key($this->getData(), array_flip($this->hiddenFields));
     }
 
     /**
@@ -104,18 +107,11 @@ public function unhideField($field_name)
         }
     }
 
-    /**
-     * Data that must be included as hidden fields, if they are available at all.
-     */
-    public function getHiddenData()
-    {
-        return array_intersect_key($this->getData(), array_flip($this->hiddenFields));
-    }
-
     /**
      * Data not in the hidden fields list.
      * These are not all mandatory, so you do not have to present all these
-     * to the user.
+     * to the user. You may also have custom fields you want to post, such
+     * as the merchant transactionId (if not using invoiceId for this purpose).
      */
     public function getVisibleData()
     {

src/Message/DPMCompleteRequest.php

@@ -11,30 +11,37 @@ class DPMCompleteRequest extends SIMCompleteAuthorizeRequest
 {
     public function getData()
     {
+        // The hash sent in the callback from the Authorize.Net gateway.
         $hash_posted = strtolower($this->httpRequest->request->get('x_MD5_Hash'));
+
+        // The transaction reference generated by the Authorize.Net gateway and sent in the callback.
         $posted_transaction_reference = $this->httpRequest->request->get('x_trans_id');
+
+        // The amount that the callback has authorized.
         $posted_amount = $this->httpRequest->request->get('x_amount');
+
+        // Calculate the hash locally, using the shared "hash secret" and login ID.
         $hash_calculated = $this->getDpmHash($posted_transaction_reference, $posted_amount);
 
         if ($hash_posted !== $hash_calculated) {
             // If the hash is incorrect, then we can't trust the source nor anything sent.
-            // Throwing exceptions here is a *really* bad idea. We are trying to get the data,
+            // Throwing exceptions here is probably a bad idea. We are trying to get the data,
             // and if it is invalid, then we need to be able to log that data for analysis.
             // Except we can't, baceuse the exception means we can't get to the data.
+            // For now, this is consistent with other OmniPay gateway drivers.
 
             throw new InvalidRequestException('Incorrect hash');
         }
 
         // The hashes have passed, but the amount should also be validated against the
         // amount in the stored and retrieved transaction. If the application has the
         // ability to retrieve the transaction (using the transaction_id sent as a custom
-        // form field, or perhaps as a GET parameter on the callback URL) then it will
-        // be checked here.
+        // form field, or perhaps in an otherwise unused field such as x_invoice_id.
 
         $amount = $this->getAmount();
 
         if (isset($amount) && $amount != $posted_amount) {
-            // The amounts don't match up. Someone may have been playing with the
+            // The amounts don't match. Someone may have been playing with the
             // transaction references.
 
             throw new InvalidRequestException('Incorrect amount');
@@ -45,10 +52,8 @@ public function getData()
 
     /**
      * This hash confirms the ransaction has come from the Authorize.Net gateway.
-     * It basically tests the shared hash secret is correct, but mixes in other details
-     * that will change for each transaction so the hash will be unique for each transaction.
-     * The hash secret and login ID are known to the merchent site, and the amount and transaction
-     * reference (x_amount and x_trans_id) are sent by the gatewa.
+     * It confirms the sender knows ther shared hash secret and that the amount and
+     * transaction reference has not been changed in transit.
      */
     public function getDpmHash($transaction_reference, $amount)
     {

src/Message/DPMCompleteResponse.php

@@ -9,10 +9,10 @@
  * Authorize.Net DPM Complete Authorize Response
  * This is the result of handling the callback.
  * The result will always be a HTML redirect snippet. This gets
- * returned to the gateway, displayed in the user's browser, and a GET
- * redirect is performed using JavaScript and meta refresh (belt and braces).
+ * returned to the gateway, displayed in the user's browser, and a
+ * redirect is performed using JavaScript and meta refresh (for backup).
  * We may want to return to the success page, the failed page or the retry
- * page (so the user can correct the form).
+ * page (so the user can correct the form to try again).
  */
 class DPMCompleteResponse extends SIMCompleteAuthorizeResponse implements RedirectResponseInterface
 {
@@ -35,11 +35,6 @@ public function isError()
         return isset($this->data['x_response_code']) && static::RESPONSE_CODE_ERROR === $this->data['x_response_code'];
     }
 
-    public function getMessage()
-    {
-        return parent::getReasonCode() . '|' . parent::getMessage();
-    }
-
     /**
      * We are in the callback, and we MUST return a HTML fragment to do a redirect.
      * All headers we may return are discarded by the gateway, so we cannot use
@@ -51,11 +46,10 @@ public function isRedirect()
     }
 
     /**
-    * We default here to POST because the default redirect mechanism
-    * in Omnipay Common only generates a HTML snippet for POST and not
-    * GET.
-    * TODO: We could fix that here so both GET and POST can be supported.
-    * Our fix should also include the "form data" with the URL.
+    * We set POST because the default redirect mechanism in Omnipay Common only
+    * generates a HTML snippet for POST and not for the GET method.
+    * The redirect method is actually "HTML", where a HTML page is supplied
+    * to do a redirect using any method it likes.
     */
     public function getRedirectMethod()
     {
@@ -66,7 +60,7 @@ public function getRedirectMethod()
      * We probably do not require any redirect data, if the incomplete transaction
      * is still in the user's session and we can inspect the results from the saved
      * transaction in the database. We cannot send the result through the redirect
-     * unless it is hashed in some way so the authorisation result cannot be faked.
+     * unless it is hashed so the authorisation result cannot be faked.
      */
     public function getRedirectData()
     {
@@ -75,9 +69,6 @@ public function getRedirectData()
 
     /**
      * The cancel URL is never handled here - that is a direct link from the gateway.
-     * The best approach is to have just one redirect URL, and once there, check the
-     * result of the authorisation in the database (assuming it has been saved in the
-     * callback) and take action from there.
      */
     public function getRedirectUrl()
     {

src/Message/SIMAuthorizeRequest.php

@@ -21,14 +21,20 @@ public function getData()
         $data['x_delim_data'] = 'FALSE';
         $data['x_show_form'] = 'PAYMENT_FORM';
         $data['x_relay_response'] = 'TRUE';
+
         // The returnUrl MUST be set in Authorize.net admin panel as a
         // "Response/Receipt URLs" URL, but not necessarily the default.
         $data['x_relay_url'] = $this->getReturnUrl();
         $data['x_cancel_url'] = $this->getCancelUrl();
+
         if ($this->getCustomerId() !== null) {
             $data['x_cust_id'] = $this->getCustomerId();
         }
 
+        if ($this->getCurrency() !== null) {
+            $data['x_currency_code'] = $this->getCurrency();
+        }
+
         if ($this->getTestMode()) {
             $data['x_test_request'] = 'TRUE';
         }

src/Message/SIMCompleteAuthorizeRequest.php

@@ -18,6 +18,9 @@ public function getData()
         return $this->httpRequest->request->all();
     }
 
+    /**
+     * CHECKME: DPM uses the transactionReference in the hash, not the transactionID.
+     */
     public function getHash()
     {
         return md5($this->getHashSecret().$this->getApiLoginId().$this->getTransactionId().$this->getAmount());