From 8bc944d526e45288f4ad5eae1fb57e1c7ee44dd3 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Tue, 24 May 2011 11:57:16 -0700 Subject: [PATCH] Prevent the PayPal standard driver from being abused. Update POD and comments in several Shop files. Fix form labels for PayPal Express checkout driver. Fix a niggling bug with updating a transaction with a cart, twice. Fixes bug #12140. commit cef8c5bde10f793db0890dcfd18cbf20b0e69c90 Author: Colin Kuskie Date: Tue May 24 11:52:48 2011 -0700 Build a Shop::Transaction. commit c386079ac29fc70c1cc63d0c2844511ce9db553e Author: Colin Kuskie Date: Tue May 24 11:34:13 2011 -0700 Pull a session out of the object. commit 59d780eb0ffcc82572491f85f08eb4ac04cab109 Author: Colin Kuskie Date: Tue May 24 11:15:05 2011 -0700 Rework PayPalStd driver to create transaction before sending the user off to PayPal. commit cd2683aa8595875f7c501f29c622abaf350e90f6 Author: Colin Kuskie Date: Tue May 24 11:14:19 2011 -0700 Fix some comments to be more cleanly readable. commit cf1fecfb916906c4d8ec8d33bd85c59b0aea3b7c Author: Colin Kuskie Date: Tue May 24 11:13:52 2011 -0700 Make the transaction optional when displaying a payment error. commit f3c949e03a18ac513938f2ed483002c5304663d5 Author: Colin Kuskie Date: Mon May 23 19:19:56 2011 -0700 Remove dead code. commit 5ddcb49f094fd054f79da38c4a95dd86c55a157f Author: Colin Kuskie Date: Thu May 19 11:31:15 2011 -0700 If a transaction is updated with a cart object, remove all transationItems from the transaction before adding new ones. commit 3315cb30a5c1fd4d583ce352cbc9022e52544404 Author: Colin Kuskie Date: Thu May 19 11:30:58 2011 -0700 Remove duplicate entries from ExpressCheckout i18n commit b46d5af528d7223d12ecbed383e798cfc376ad2a Author: Colin Kuskie Date: Mon May 23 16:07:33 2011 -0700 Fix the version number of the PayPal API we send. Add the serialized request to the log file. Fix i18n labels so it's clear which username and password to use. commit 1604d375822eb013c633b72993fa524703a72127 Author: Colin Kuskie Date: Wed May 18 15:50:42 2011 -0700 Fix copy/paste errors from other i18n modules into ExpressCheckout. commit f2c958fc7128348a18a005bfbadf83457861d6e2 Author: Colin Kuskie Date: Wed May 18 11:26:15 2011 -0700 Update out of date POD about checking out. commit 61ca80b15701733a1a7c7eae5d825b161e0c71c1 Author: Colin Kuskie Date: Mon May 23 16:05:09 2011 -0700 Fix documentation in appendCartVariables, and return the hash instead of $self. commit d3b7341c44c924f395f8594c8ae77d8187170c9f Author: Colin Kuskie Date: Mon May 23 16:03:20 2011 -0700 Remove variables that were not being used. commit 2913f96182a7630bce01998bb022d3ebf4842171 Author: Colin Kuskie Date: Mon May 16 21:05:54 2011 -0700 Pull isRecurring directly from the Cart if creating/updating a transaction from one. --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/Shop/Cart.pm | 3 - lib/WebGUI/Shop/PayDriver.pm | 30 ++-- lib/WebGUI/Shop/PayDriver/Ogone.pm | 3 +- .../Shop/PayDriver/PayPal/ExpressCheckout.pm | 3 +- lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm | 130 ++++++++++++------ lib/WebGUI/Shop/Transaction.pm | 6 + lib/WebGUI/i18n/English/PayDriver.pm | 6 + .../i18n/English/PayDriver_ExpressCheckout.pm | 17 +-- 9 files changed, 128 insertions(+), 71 deletions(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 15a57cb8d..81895273f 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -5,6 +5,7 @@ - fixed : Thingy CSV new records not updated with createdById and dateCreated and ipAddress - added : Thingy fields can now be set as unique and checked upon insert - added : Thingy max entries of thingy records added + - fixed #12140: PayPal standard can be used to to purchase items without paying for them 7.10.16 - fixed #12121: typ-o Asset_Map.templateIdEditPoint diff --git a/lib/WebGUI/Shop/Cart.pm b/lib/WebGUI/Shop/Cart.pm index 62cb80878..b31f12c0e 100644 --- a/lib/WebGUI/Shop/Cart.pm +++ b/lib/WebGUI/Shop/Cart.pm @@ -970,9 +970,6 @@ sub www_lookupPosUser { Updates the cart totals, addresses, shipping driver and payment gateway. If requested, and the cart is ready, calls the www_getCredentials method from the selected payment gateway. -If the cart total, after taxes, shipping, coupons and shop credit is zero, does the checkout -immediately without calling a payment gateway. - Otherwise, returns the user back to the cart. =cut diff --git a/lib/WebGUI/Shop/PayDriver.pm b/lib/WebGUI/Shop/PayDriver.pm index 15764b72d..e901d5a42 100644 --- a/lib/WebGUI/Shop/PayDriver.pm +++ b/lib/WebGUI/Shop/PayDriver.pm @@ -84,8 +84,13 @@ sub _buildObj { =head2 appendCartVariables ( $var ) -Append the subtotal, shipping, tax, and shop credeductions to a set of template -variables. +Append the subtotal, shipping, tax, and shop credit deductions to a set of template +variables. Returns the modified hashreference of variables. + +=head3 $var + +A hashref. Template variables will be added to it. If $var is not passed, a new +hashref is created, and that is returned. =cut @@ -103,7 +108,7 @@ sub appendCartVariables { $var->{inShopCreditAvailable} = $credit->getSum; $var->{inShopCreditDeduction} = $credit->calculateDeduction($totalPrice); $var->{totalPrice } = $cart->formatCurrency($totalPrice + $var->{inShopCreditDeduction}); - return $self; + return $var; } @@ -305,10 +310,14 @@ sub displayPaymentError { my ($self, $transaction) = @_; my $i18n = WebGUI::International->new($self->session, "PayDriver"); my $output = q{

} . $i18n->get('error processing payment') . q{

} - . q{

} . $i18n->get('error processing payment message') . q{

} - . q{

} . $transaction->get('statusMessage') . q{

} - . q{

} . $i18n->get( 'try again' ) . q{

} - ; + . q{

} . $i18n->get('error processing payment message') . q{

}; + if ($transaction) { + $output .= q{

} . $transaction->get('statusMessage') . q{

}; + } + else { + $output .= q{

} . $i18n->get('unable to finish transaction') . q{

}; + } + $output .= q{

} . $i18n->get( 'try again' ) . q{

}; return $self->session->style->userStyle($output); } @@ -645,10 +654,9 @@ sub processTransaction { # Setup dynamic transaction unless (defined $transaction) { my $transactionProperties; - $transactionProperties->{ paymentMethod } = $self; - $transactionProperties->{ cart } = $cart; - $transactionProperties->{ isRecurring } = $cart->requiresRecurringPayment; - + $transactionProperties->{ paymentMethod } = $self; + $transactionProperties->{ cart } = $cart; + # Create a transaction... $transaction = WebGUI::Shop::Transaction->create( $self->session, $transactionProperties ); } diff --git a/lib/WebGUI/Shop/PayDriver/Ogone.pm b/lib/WebGUI/Shop/PayDriver/Ogone.pm index 2f7a1743f..ed34bbd04 100644 --- a/lib/WebGUI/Shop/PayDriver/Ogone.pm +++ b/lib/WebGUI/Shop/PayDriver/Ogone.pm @@ -149,7 +149,7 @@ sub processPayment { my $self = shift; # Since we'll have to create a transaction before doing the actual tranasction, we let it fail # initially with a message that it is pending. - # Unless the transaction result with _setPaymentStatus the transaction will fail. + # Unless the transaction result is updated via _setPaymentStatus the transaction will fail. my $success = $self->{_transactionSuccessful} || 0; my $id = $self->{_ogoneId} || undef; @@ -157,7 +157,6 @@ sub processPayment { my $message = $self->{_statusMessage} || 'Waiting for checkout'; return ( $success, $id, $status, $message ); - return (0, undef, 1, 'Pending'); } #------------------------------------------------------------------- diff --git a/lib/WebGUI/Shop/PayDriver/PayPal/ExpressCheckout.pm b/lib/WebGUI/Shop/PayDriver/PayPal/ExpressCheckout.pm index 7724efce0..868540a8e 100644 --- a/lib/WebGUI/Shop/PayDriver/PayPal/ExpressCheckout.pm +++ b/lib/WebGUI/Shop/PayDriver/PayPal/ExpressCheckout.pm @@ -156,7 +156,7 @@ is a hashref, it will be modified in place. sub payPalForm { my $self = shift; my $args = ref $_[0] eq 'HASH' ? shift : {@_}; - $args->{VERSION} = '58.0'; + $args->{VERSION} = '2.3'; $args->{USER} = $self->get('user'); $args->{PWD} = $self->get('password'); $args->{SIGNATURE} = $self->get('signature'); @@ -300,6 +300,7 @@ sub www_sendToPayPal { if ($params) { unless ( $params->{ACK} =~ /^Success/ ) { my $log = sprintf "Paypal error: Request/response below: %s\n%s\n", Dumper($form), Dumper($params); + $log .= $response->request->as_string; $session->log->error($log); $error = $i18n->get('internal paypal error'); } diff --git a/lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm b/lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm index e0653f3b6..5f064592d 100644 --- a/lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm +++ b/lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm @@ -22,6 +22,7 @@ use URI; use URI::Escape; use LWP::UserAgent; use Readonly; +use WebGUI::Shop::Transaction; Readonly my $I18N => 'PayDriver_PayPalStd'; @@ -161,7 +162,9 @@ sub getButton { # All the API stuff is done in paymentVariables; we'll just turn it into # hidden form fields here - my $v = $self->paymentVariables; + my $v = $self->paymentVariables; + my $transaction = $self->processTransaction(); + $v->{custom} = $transaction->getId; my $fields = join "\n", map { WebGUI::Form::hidden( $session, { name => $_, value => $v->{$_} } ) } (keys %$v); @@ -193,6 +196,38 @@ sub getButton { #------------------------------------------------------------------- +=head2 getPayPalParams + +Using the tx form variable, dial up PayPal and ask them for details about the transaction. +Return a hashreference of name/value pairs, along with PAYPAL_TX, the transactionId and +PAYPAL_REQUEST_STATUS, the HTTP code from the response from PayPal. + +=cut + +sub getPayPalParams { + my $self = shift; + my $session = $self->session; + # instead of relying on what was passed to us. + return $self->{_params} if $self->{_params}; + my $tx = $session->form->process('tx'); + + my %form = ( + cmd => '_notify-synch', + tx => $tx, + at => $self->get('identityToken'), + ); + my $response = LWP::UserAgent->new->post($self->payPalUrl, \%form); + my ($status, @lines) = split("\n", $response->content); + my %params = map { split /=/ } + map { uri_unescape($_) } @lines; + $params{PAYPAL_REQUEST_STATUS} = $status; + $params{PAYPAL_TX} = $tx; + $self->{_params} = \%params; + return $self->{_params}; +} + +#------------------------------------------------------------------- + =head2 paymentVariables Returns a hashref of the payment variables to be used as hidden form fields @@ -274,49 +309,49 @@ passed to us. =cut sub processPayment { - my ( $self, $transaction ) = @_; - my $session = $self->session; + my ( $self ) = @_; - # To prevent a spoofed post to this url, we'll get the info from paypal - # instead of relying on what was passed to us. - my $tx = $session->form->process('tx'); + my $success = $self->{_transactionSuccessful} || 0; + my $id = $self->{_tx} || undef; + my $status = $self->{_statusCode} || undef; + my $message = $self->{_statusMessage} || 'Waiting for checkout'; - my %form = ( - cmd => '_notify-synch', - tx => $tx, - at => $self->get('identityToken'), - ); - my $response = LWP::UserAgent->new->post($self->payPalUrl, \%form); - my ($status, @lines) = split("\n", $response->content); - my %params = map { split /=/ } - map { uri_unescape($_) } @lines; + return ( $success, $id, $status, $message ); +} - if ($status =~ /FAIL/) { - my $message = ''; - foreach my $key ( keys %params ) { - $message .= ""; - } - $message .= '
FieldValue
$key$params{$key}
'; - return ( 0, $tx, $status, $message ); - } +#------------------------------------------------------------------- - # Make sure the transaction is for this cart to prevent spoofing - my $cartId = $self->getCart->getId; - if ($params{custom} ne $cartId) { - my $user = $session->user; - my $name = $user->username; - my $id = $user->userId; - $session->log->warn("SECURITY WARNING: $name (id: $id) tried to " . - "checkout cart $cartId with PayPal transaction $tx, which " . - "did not match the cart we passed ($params{custom})"); +=head2 _setPaymentStatus ( transactionSuccessful, ogoneId, statusCode, statusMessage ) - my $i18n = WebGUI::International->new( $session, $I18N ); - return ( 0, $tx, 'FAIL', $i18n->get('cart transaction mismatch') ); - } +Update the internal status of a payment, so that the next call to processPayment +returns the correct data. - $status = $params{payment_status}; - return ( 1, $tx, $status, $status, $status ); -} ## end sub processPayment +=head3 transactionSuccessful + +A boolean indicating whether or not the payment was successful. + +=head3 tx + +The PayPal issued transaction ID. + +=head3 statusCode + +The PayPal issued status code. + +=head3 statusMessage + +An updates status message + +=cut + +sub _setPaymentStatus { + my ( $self ) = @_; + + $self->{_transactionSuccessful} = shift || 0; + $self->{_tx} = shift || undef; + $self->{_statusCode} = shift || undef; + $self->{_statusMessage} = shift || undef; +} #------------------------------------------------------------------- @@ -327,9 +362,24 @@ Where paypal comes back to when a transaction has been completed. =cut sub www_completeTransaction { - my $self = shift; + my $self = shift; + my $session = $self->session; - my $transaction = $self->processTransaction; + my $params = $self->getPayPalParams; + if ($params->{PAYPAL_REQUEST_STATUS} =~ /FAIL/) { + my $message = "\n"; + foreach my $key ( keys %{ $params } ) { + $message .= sprintf "\n", $key, $params->{key}; + } + $message .= "
FieldValue
%s%s
\n"; + return ( 0, $params->{PAYPAL_TX}, $params->{PAYPAL_REQUEST_STATUS}, $message ); + } + my $transaction = eval { WebGUI::Shop::Transaction->new($session, $params->{custom}); }; + if (my $e = Exception::Class->caught) { + return $self->displayPaymentError(); + } + $self->_setPaymentStatus(1, $params->{PAYPAL_TX}, $params->{payment_status}, 'Complete'); + $self->processTransaction($transaction); return $transaction->get('isSuccessful') ? $transaction->thankYou diff --git a/lib/WebGUI/Shop/Transaction.pm b/lib/WebGUI/Shop/Transaction.pm index 1e379f0cf..034114fee 100644 --- a/lib/WebGUI/Shop/Transaction.pm +++ b/lib/WebGUI/Shop/Transaction.pm @@ -802,9 +802,15 @@ sub update { $newProperties->{paymentDriverId} = $pay->getId; $newProperties->{paymentDriverLabel} = $pay->get('label'); + ##Clear out current transaction items before adding new ones. + foreach my $item (@{$self->getItems}) { + $item->delete; + } foreach my $item (@{$cart->getItems}) { $self->addItem({item=>$item}); } + + $newProperties->{isRecurring} = $cart->requiresRecurringPayment; } if (exists $newProperties->{paymentMethod}) { my $pay = $newProperties->{paymentMethod}; diff --git a/lib/WebGUI/i18n/English/PayDriver.pm b/lib/WebGUI/i18n/English/PayDriver.pm index bc9309424..7935eb697 100644 --- a/lib/WebGUI/i18n/English/PayDriver.pm +++ b/lib/WebGUI/i18n/English/PayDriver.pm @@ -118,6 +118,12 @@ our $I18N = { context => q|Status message|, }, + 'unable to finish transaction' => { + message => q|We are unable to lookup the transaction to finish checking out.|, + lastUpdated => 0, + context => q|Error message when the transaction cannot be looked up.|, + }, + }; 1; diff --git a/lib/WebGUI/i18n/English/PayDriver_ExpressCheckout.pm b/lib/WebGUI/i18n/English/PayDriver_ExpressCheckout.pm index 7417549de..621c88884 100644 --- a/lib/WebGUI/i18n/English/PayDriver_ExpressCheckout.pm +++ b/lib/WebGUI/i18n/English/PayDriver_ExpressCheckout.pm @@ -43,7 +43,7 @@ our $I18N = { context => q{The name of the payment driver}, }, 'password' => { - message => q{Password}, + message => q{API Password}, lastUpdated => 1247254156, }, 'password help' => { @@ -88,7 +88,7 @@ our $I18N = { lastUpdated => 1247253981, }, 'user' => { - message => q{Username}, + message => q{API Username}, lastUpdated => 1247254097, }, 'user help' => { @@ -104,20 +104,9 @@ our $I18N = { 'summary template help' => { message => q|Pick a template to display the screen where the user confirms the cart summary info and agrees to pay.|, lastUpdated => 0, - context => q|Hover help for the summary template field in the configuration form of the Cash module.| + context => q|Hover help for the summary template field in the configuration form.| }, - 'password' => { - message => q|Password|, - lastUpdated => 0, - context => q|Form label in the configuration form of the iTransact module.| - }, - 'password help' => { - message => q|The password for your ITransact account.|, - lastUpdated => 0, - context => q|Hover help for the password field in the configuration form of the iTransact module.| - }, - 'Pay' => { message => q|Pay|, lastUpdated => 0,