uc_proxypay3_eurobank

Hello, xaris.tsimpouris. You need to create a sandbox project, and report here the link.

I opened a few issues in the issue queue. I'm not able to actually try the module, not having a Eurobank account. I'm also not familiar enough with ubercart payment processing to be able to give a comprehensive review, so this is just a start.

The last few days I am testing the module with eurobank proxypay and with 2 changes it is working as expected.

This should be moved to a proper drupal projet module so we can get a proper realease out.

Manos
http://websynergy.gr/

Closing, feel free to re-open if this was a mistake.

Tim you marked the project as "Won't Fix", does this mean that it can't become a module?

Searching in drupal.org I still only see the application page and not the module page.

There has been no reply from the maintainer for 15 weeks, and there are still open issues: http://drupal.org/project/issues/1087750

Marking back to needs work since there is interest.

I understand. I, among others, have been using this module for some months now.

I'll see to it that we solve all issues as soon as we can together with Xaris and other people of the community.
Thank you very much!

Please remove the LICENSE.txt file. Drupal will add the appropriate version automatically during packaging so your repository should not include it.

watchdog('proxypay3_eurobank', 'Confirmation Error: Wrong password ' . $_POST['Password'] );

That's a xss vulnerability because anyone who does a post can insert javascript into the watchdog entry. It should be something more like:

watchdog('proxypay3_eurobank', 'Confirmation Error: Wrong password @password , array('@password' => $_POST['Password'] ));

That same problem is repeated a at least once more and should be fixed.

Generally speaking you don't need to check_plain database arguments, especially not if they are numeric like the %d implies it will be.
check_plain($_POST['Ref']));

This is another XSS, though harder to attack since it requires knowing the proper password. However, this should really be using the @ placeholder which will do a check_plain on the placeholder before subsituting.

watchdog('proxypay3_eurobank', 'Receiving new order notification for order !order_id.', array('!order_id' => check_plain($_POST['Ref'])));

The status messages in the validate function are not using t() to allow translation. This is both a localization problem and a security problem unless the data is being filtered on output (which is the Drupal standard).

It's not 100% clear to me what uc_proxypay3_eurobank_validate is used for, but it seems to me that the data is not being signed or verified in any way. Overall I suggest that all communication via $_POST should include system of hashing the values with a secret key so that before you process the data you can confirm that the data is actually coming from a trusted source. I know that's not within your control, but maybe you could pass the idea to the merchant gateway.

The code in uc_proxypay3_eurobank.proccess.inc doesn't match the Drupal standard coding style which makes it harder to review. I noticed some very small code style issues. Please run the coder module on "minor" setting to help catch these. I noticed things like lack of function documentation blocks using doxygen and if statements that don't use {}.

Most of those are required things to address. The code style item is a strong suggestion. The request for signing the data with a shared secret key is, of course, not a requirement for approval.

@xaris.tsimpouris has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

Should we mark this as postponed until you got time?

closing this since there is another application active from you:
#1409008: commerce_greek_banks