Google Pay support #10

bdewater · 2017-11-07T14:55:07Z

This PR adds support for Google Pay tokens as returned by the Google Payment API. Google's announcement is here and the technical docs are here.

A tl;dr version is that what used to be known as 'Android Pay on the web' is now known as TOKENIZED_CARD, while CARD is the credit card stored on a Google account. In the browser both go through W3C Payment Request with the https://google.com/pay payment method.

All this is versioned as ECv1 in the technical docs, old Android Pay tokens are retconned as ECv0. For decryption, an important difference between ECv1 TOKENIZED_CARD and ECv0 is the 'info' parameter in the key derivation step. Other than that the algorithms are the same.

ECv1 data is signed by Google. A Google provided Merchant ID is needed to verify it. The URLs to fetch the keys can be found in the Tink reference implementation. An application should store the keys somewhere and respect the expires header to periodically refresh them. For now I've left this as an application concern. The code in this PR takes the parsed JSON and is able to handle multiple keys when Google rotates the keys.

elfassy · 2017-11-07T15:16:54Z

lib/r2d2/google_payment_token.rb

+      message['paymentMethodDetails']
+    end
+
+    class << self


best to move classes/class methods to the top of the file to avoid them being in a private section (which will have no effect other than confuse people)

elfassy · 2017-11-07T15:38:20Z

lib/r2d2/payment_token.rb

-        hkdf_keys = {
+      def derive_hkdf_keys(ephemeral_public_key, shared_secret, info = 'Android')
+        key_material = Base64.decode64(ephemeral_public_key) + shared_secret
+        hkdf = HKDF.new(key_material, :algorithm => 'SHA256', :info => info)


IRL: may want to have the info be a constant since only a few keys are allowed. Might also be worth having another class for android

class AndroidPaymentToken < PaymentToken HKDF_INFO = 'Android' end

hkdf = HKDF.new(key_material, :algorithm => 'SHA256', :info => self.class.const_get('HKDF_INFO')

rwdaigle · 2018-01-23T17:46:05Z

Hi @bdewater - this is great, thank you!

I'm wondering if we can isolate the Google Pay specific parts of this PR from some of the other changes you've made (upgrading the Ruby version, test framework changes, etc...)? I think we're ok with the majority of them, but just want to be able to evaluate them independently.

bdewater · 2018-01-23T20:26:10Z

Hey @rwdaigle thanks for the kind words. That should definitely be possible!

I'm thinking it would be best to pull those 'plumbing' commits out in a separate PR, get that merged to master, and then rebase this on top of it. What do you think?

rwdaigle · 2018-01-24T20:03:45Z

I'm thinking it would be best to pull those 'plumbing' commits out in a separate PR, get that merged to master, and then rebase this on top of it. What do you think?

Sure, let's get the easy stuff out of the way first 👍

Re: the ruby version change - I'd love for that not to be a thing baked into the lib itself. Perhaps we should just gitignore .ruby-version?

…yment tokens Docs are at https://developers.google.com/payments/payment-data-cryptography This commit is a quick and dirty attempt to have Google Payment API TOKENIZED_CARD working by returning the same values as Android Pay did, without breaking the existing Android Pay implementation that can still be used by non-browser clients. This is done by: - using either 'Android' or 'Google' for the info parameter of the KDF - return the parsed paymentMethodDetails attribute from the encryptedMessage of a Google Payment token, containing 'dpan' etc, to the consuming application Stuff missing in this commit for a complete implementation: - Verifying the signature (from Google) of the signedMessage - Checking the messageExpiration timestamp in the encryptedMessage

The documentation can be found on https://developers.google.com/payments/payment-data-cryptography The `to_length_value` implementation has been verified against the Tink implementation: https://github.com/google/tink/blob/516ab63d26b48077fecd1b7078a116d6dcd5d54f/apps/paymentmethodtoken/src/main/java/com/google/crypto/tink/apps/paymentmethodtoken/PaymentMethodTokenUtil.java#L64-L72

With signature verification and message expiration check, this limitation from the inital scrappy implementation can be removed. This also means the consuming application has full access to details like paymentMethod, which can be used for example with ActiveMerchant to build either a ActiveMerchant::Billing::NetworkTokenizationCreditCard or a ActiveMerchant::Billing::CreditCard object.

…e chunk string every time

bdewater · 2018-02-14T09:36:16Z

@rwdaigle I just pushed the rebase :)

bdewater · 2018-03-20T14:45:12Z

@rwdaigle we're live https://www.shopify.ca/google-pay so I think I can call this production ready now ;)

rwdaigle · 2018-03-20T18:25:41Z

@bdewater awesome (and congrats!).

I'll try to raise this a bit on my stack.

rwdaigle · 2018-03-21T15:53:02Z

@bdewater question - is Android Pay deprecated? I.e. is there any reason to support both token formats in the same lib version (would an app/service need to be able to handle both at the same time), or perhaps we can tag r2d2 as-is for Android Pay support and release a new version w/ only Google Pay support going forward?

bdewater · 2018-03-21T17:20:37Z

@rwdaigle when I started implementing this Google disabled Android Pay signups but we still saw the occasional purchase coming in, so we decided to implement Google Pay in parallel to not break anything before we actually had the alternative in place.

I'm not sure what the current state of all this is, but I can ask around. Starting from scratch would definitely clean some things up.

rwdaigle · 2018-03-21T17:41:22Z

Starting from scratch would definitely clean some things up.

It may not be viable (I'm checking on our end too), but let me know what you find. Thanks! 👍

rwdaigle · 2018-05-03T14:02:57Z

FYI @deedeelavinder and @dtykocki are taking a look at this now...

rwdaigle

@deedeelavinder just took a pass through this PR and, outside of the specific comments, have the following thought:

I'm not sure the new GooglePaymentToken class needs to be a subclass of the previous (and current Android) PaymentToken class. I looked but didn't see much shared code between the two and, conceptually, don't believe Google Pay should be thought of as a special case of Android Pay? It feels, from my high level, that GooglePaymentToken should be its own class, and a new AndroidPaymentToken should be what PaymentToken is now. If there is functionality to share it can be done via modules or a shared utility library vs. inheritance. (@bdewater may have some insight to share here that disputes this take given his deeper understanding of the implementation).

rwdaigle · 2018-05-08T17:16:09Z

lib/r2d2/google_payment_token.rb

+      end.join
+    end
+
+    def initialize(token_attrs, recipient_id:, verification_keys:)


I believe this is Ruby v2.? syntax? If this is the only place we're using this it might be good to revert to "legacy" syntax to support a greater range of Ruby versions?

Mandatory keyword args were introduced in Ruby 2.2, which was EOL'ed end of March after being released over 3 years ago. Given that we're dealing with credit card data, should we make the interface less clear to accommodate for old Ruby versions? Personally, I don't think we should.

rwdaigle · 2018-05-08T17:19:17Z

lib/r2d2/payment_token.rb

@@ -27,7 +30,8 @@ def decrypt(private_key_pem)
      # verify the tag is a valid value
      self.class.verify_mac(digest, hkdf_keys[:mac_key], encrypted_message, tag)

-      self.class.decrypt_message(encrypted_message, hkdf_keys[:symmetric_encryption_key])
+      message = self.class.decrypt_message(encrypted_message, hkdf_keys[:symmetric_encryption_key])
+      JSON.parse(message)


The current contract of r2d2 is that it tries to assume as little as possible about how the caller wants to parse the resulting message. This has the added benefit of letting the caller use their own JSON parser, or defer to a later point in processing to do so. I've found that people are quite opinionated about their chosen JSON lib :) I'd like to retain that contract here, and just return the message string itself, letting the caller decide what they do with it from there. (further background here)

GooglePaymentToken needs the parsed JSON for the message expiration check. It's logical to have both decrypt methods should be returning the same type.

The Ruby community seems to have mostly abandoned multi-json and such in libraries in favor of stdlib json; it's available on all modern Rubies for years and good enough. Call your equivalent of Oj.mimic_JSON if you really feel the need for speed, but the last time I looked at performance for this gem the Ruby HKDF was the slowest part IIRC (see ruby/openssl#172).

rwdaigle · 2018-05-08T17:22:20Z

test/fixtures/google/tokenized_card.json

@@ -0,0 +1,5 @@
+{
+  "signature": "MEUCIQDzgQkY/Lj42FoZC9wN44Uz0r6uDttc68k2c9oZCzNJPwIgSRukEx30uMjqzFswekoarAtO9p1/E221hNmVuHzDW+M\u003d",
+  "protocolVersion": "ECv1",


This payment token structure looks very different from the one we have for Android Pay. If this is a structure we can rely on, it would be interesting to provide a single entry point into r2d2 that determines which token path to use to decrypt vs. having the user have to peek into the structure to determine which token type it is.

Not sure if this is a real world scenario or not? Definitely would still keep very explicit Google vs. Android token types, just with a thin routing convenience on top if we wanted to provide this.

Android Pay was unversioned and Google's documentation says in this case to assume ECv0. A factory method would definitely be nice to abstract this concern. I can add this somewhere this week.

bdewater · 2018-05-14T19:14:43Z

Thanks for your time to review @rwdaigle and @deedeelavinder!

I'm not sure the new GooglePaymentToken class needs to be a subclass of the previous (and current Android) PaymentToken class. I looked but didn't see much shared code between the two and, conceptually, don't believe Google Pay should be thought of as a special case of Android Pay?

The innermost payload of a Google Pay TOKENIZED_CARD token is essentially the same as Android Pay (docs). The PR commits are very much an exploration of how everything fitted together at the time, and I did not really spent much time to rearrange the pieces to more logical locations after.

I agree the class structure looks weird the shared parts between the two types could be extracted to a better location.

bdewater · 2018-05-19T20:07:00Z

See #13

elfassy reviewed Nov 7, 2017

View reviewed changes

bdewater force-pushed the scrappy-google-payment-tokens branch from 14d9d0c to 418e2b6 Compare December 18, 2017 04:12

bdewater changed the title ~~Pay with Google support~~ Google Pay support Jan 9, 2018

bdewater mentioned this pull request Feb 2, 2018

Improve plumbing #11

Closed

bdewater added 9 commits February 14, 2018 10:32

Return parsed JSON hash from PaymentToken#decrypt instead of string

4635e3c

DRY Google payment token tests

3412d85

Add message expiration check

64a4230

Move HKDF info parameter to constant

5978e42

Move to_length_value class method definition up, don't un/repack th…

6690d3a

…e chunk string every time

Change merchant_id to recipient_id

cc60cd6

bdewater force-pushed the scrappy-google-payment-tokens branch from 4b2a1cc to cc60cd6 Compare February 14, 2018 09:34

rwdaigle suggested changes May 8, 2018

View reviewed changes

bdewater mentioned this pull request May 19, 2018

Google Pay support, redux #13

Closed

bdewater closed this May 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Google Pay support #10

Google Pay support #10

bdewater commented Nov 7, 2017 •

edited

Loading

elfassy Nov 7, 2017

elfassy Nov 7, 2017 •

edited

Loading

rwdaigle commented Jan 23, 2018

bdewater commented Jan 23, 2018 •

edited

Loading

rwdaigle commented Jan 24, 2018

bdewater commented Feb 14, 2018

bdewater commented Mar 20, 2018

rwdaigle commented Mar 20, 2018

rwdaigle commented Mar 21, 2018

bdewater commented Mar 21, 2018

rwdaigle commented Mar 21, 2018

rwdaigle commented May 3, 2018

rwdaigle left a comment

rwdaigle May 8, 2018

bdewater May 14, 2018

rwdaigle May 8, 2018 •

edited

Loading

bdewater May 14, 2018

rwdaigle May 8, 2018

bdewater May 14, 2018

bdewater commented May 14, 2018

bdewater commented May 19, 2018

Google Pay support #10

Google Pay support #10

Conversation

bdewater commented Nov 7, 2017 • edited Loading

elfassy Nov 7, 2017

Choose a reason for hiding this comment

elfassy Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

rwdaigle commented Jan 23, 2018

bdewater commented Jan 23, 2018 • edited Loading

rwdaigle commented Jan 24, 2018

bdewater commented Feb 14, 2018

bdewater commented Mar 20, 2018

rwdaigle commented Mar 20, 2018

rwdaigle commented Mar 21, 2018

bdewater commented Mar 21, 2018

rwdaigle commented Mar 21, 2018

rwdaigle commented May 3, 2018

rwdaigle left a comment

Choose a reason for hiding this comment

rwdaigle May 8, 2018

Choose a reason for hiding this comment

bdewater May 14, 2018

Choose a reason for hiding this comment

rwdaigle May 8, 2018 • edited Loading

Choose a reason for hiding this comment

bdewater May 14, 2018

Choose a reason for hiding this comment

rwdaigle May 8, 2018

Choose a reason for hiding this comment

bdewater May 14, 2018

Choose a reason for hiding this comment

bdewater commented May 14, 2018

bdewater commented May 19, 2018

bdewater commented Nov 7, 2017 •

edited

Loading

elfassy Nov 7, 2017 •

edited

Loading

bdewater commented Jan 23, 2018 •

edited

Loading

rwdaigle May 8, 2018 •

edited

Loading