Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Token field may not be send in server-notification #15

Closed
Patabugen opened this issue Dec 9, 2013 · 4 comments
Closed

Token field may not be send in server-notification #15

Patabugen opened this issue Dec 9, 2013 · 4 comments

Comments

@Patabugen
Copy link
Contributor

Unless you request it and enable (and pay for) it on your account you don't generate 'Token' emails.

Server.php around line 240 has this line:

$this->setField('Token', $post['Token']);

which throws an error if Surcharge isn't sent.

For now I've just done

if (isset($post['Token'])) {
    $this->setField('Token', $post['Token']);
}
@Patabugen
Copy link
Contributor Author

FYI my email to SagePay asking where/what the Token was got this reply:

Thanks for your email.

The token service is an additional service we offer which is not currently on the account. I’ve included some details on it below, please let me know if you want this activated: -

The token system enables Sagepay to remember your customers details meaning when they return to purchase the process is streamline to a 1 click checkout.

  • Gives Merchant ability to offer a ‘One click’ check out – Increase conversion rates
  • Allows customers to store multiple cards – Amazon style format
  • Combatable with our Server Solution and use with iframe templates and Direct integration
  • A Token can be used multiple times up until it expires (based on card expiry). You are also only charged for stored Tokens
  • All primary payment types available with Token
  • Only get charged for stored Tokens on a monthly basis
  • Relieves PCI compliancy impact and responsibility of Capturing/Storing card numbers

Costings: -

Number of tokens

Price
0-999 stored tokens £20/month
1,000-1,999 stored tokens £40/month
2,000-2,999 stored tokens £60/month

This can be easily turned on in the admin panel of your Magento/Ebizmarts module.
If this is something you’re interested or have any further questions please just let me know and I can add this to your account.

@judgej
Copy link
Member

judgej commented Dec 9, 2013

How strange. I guess I have not seen the Token field missing because I have been testing with surcharges. I also don't have tokens enabled on my SagePay test account, but have seen a blank Token field in the notification response.

What I will do, is ensure defaults are given for all $post elements passed into the notification handler. Rather than referencing the elements of the $post array directly - an array that comes from the application and which the library has no control over generating - I'll wrap the reading of that array in a method that can provide defaults for missing elements.

The idea of tokens is great - it is an authorisation to use the card without having to ask for details of it each time, and without having to store the credit card details anywhere (like "dhoh!" Adobe did). It is a bit like an oauth key - it is given out with specific privileges for a specific period of time, but can be revoked without notice at any time. It would be used to implement something similar to Amazon's One-click purchasing. I'm not sure how many other payment processors support something similar. Whether SagePay store the credit card details, or ask the end payment processor for a token, I don't know. It may even be a mix of both, depending on who the end processor is.

I'll try and look at this this week, but also am aware I still have your pull-request to dive into.

judgej added a commit that referenced this issue Dec 10, 2013
@judgej
Copy link
Member

judgej commented Dec 10, 2013

That should fix it. Most of the $post elements are already defaulted to "". These are the elements used for tamper checking, and are flagged in the transaction metadata as "tamper: true". The few that are not a part of the tamper checks, are not assumed to be set, and will default to "" if not set. This includes Token and Surcharge. In addition, one or two fields in the transaction metadata did not have their tamper flag set, so would not have defaulted if not sent by SagePay in the notification callback.

@judgej
Copy link
Member

judgej commented Dec 8, 2015

I'm assuming the last change fixed this. Pleas reopen if I'm mistaken.

@judgej judgej closed this as completed Dec 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants