Extend README with usage and complete example #13
Conversation
README.md
Outdated
|
|
||
| # Validate the decrypted data; raises an Pedicel::Validator::Error if the | ||
| # format is invalid. | ||
| Pedicel::Validator.validate_token_data(data) |
There was a problem hiding this comment.
Perhaps this step becomes superfluous due to #14 ?
There was a problem hiding this comment.
Actually not, because this is a validation of the decrypted token data whereas #14 introduces automatic validation of the inputted (unencrypted) token.
There was a problem hiding this comment.
Minor: I'd use the term Payment Data somewhere in the comment, it's what Apple calls the data. As above, you could also link to what tests are being performed.
ct-clearhaus
requested review from
mt-clearhaus and
kse-clearhaus
and removed request for
kse-clearhaus
README.md
Outdated
| # Instantiate. | ||
| # Validates the format of the Apple Pay payment token; raises an | ||
| # Pedicel::Validator::Error if the format is invalid. | ||
| pedicel = Pedicel::EC.new(apple_pay_payment_token_hash) |
There was a problem hiding this comment.
Minor comment: We could mention that the rules for the check exist at https://developer.apple.com/library/content/documentation/PassKit/Reference/PaymentTokenJSON/PaymentTokenJSON.html.
We should maybe mention that it does not support using RSA.
There was a problem hiding this comment.
What section do you refer to?
If the bullets below step 6 of the "to decrypt" steps: we cannot check that currencyCode and transactionAmount is correct (because that's not the job of this gem).
If the table below "Payment Data Keys": we do not ensure that it is indeed an ISO 4217 numeric code.
If we should refer to anything, then I think it should be the code. An addition could be a checklist of things the user of this gem should check on their own.
There was a problem hiding this comment.
You're right about step 6 :)
I leave it at your discretion, but I would refer to step 1, which is essentially what the checks boil down to.
There was a problem hiding this comment.
Ahh, I misunderstood what you meant (I thought about the Validator stuff, my bad).
"Validates the format" is not exactly right. It's more like "Validates the content". I'll adjust.
There was a problem hiding this comment.
The errors have "Format" in their name:
pedicel/lib/pedicel/validator.rb
Lines 11 to 12 in 2605cb7
Should we adjust? To TokenError and TokenDataError? WDYT @kse-clearhaus @mt-clearhaus?
There was a problem hiding this comment.
The Validator stuff is more about format, so I guess it's acceptable. Otherwise, perhaps TokenSchemaError and TokenDataSchemaError?
There was a problem hiding this comment.
Hmm. It is about format, but not just, i.e. signature checking.
Maybe TokenContentError and TokenDataContentError.
But either will do.
README.md
Outdated
|
|
||
| # Validate the decrypted data; raises an Pedicel::Validator::Error if the | ||
| # format is invalid. | ||
| Pedicel::Validator.validate_token_data(data) |
There was a problem hiding this comment.
Minor: I'd use the term Payment Data somewhere in the comment, it's what Apple calls the data. As above, you could also link to what tests are being performed.
README.md
Outdated
| sk = pedicel.symmetric_key(private_key: pk, merchant_id: mid) | ||
| ``` | ||
|
|
||
| ## Complete test using `pedicel-pay` |
There was a problem hiding this comment.
Maybe we should state briefly why people would need this?
There was a problem hiding this comment.
People could use it for things we haven't thought about. It's basically just an example that one can carry out. I'll rename it to include the word "example".
README.md
Outdated
| # Decrypt using the merchant private key and merchant certificate. | ||
| # The merchant private key `pk` and merchant certificate `c` are generated | ||
| # during the Apple Pay merchant sign-up. | ||
| data = pedicel.decrypt(private_key: pk, certificate: c) |
There was a problem hiding this comment.
Apple call it Apple Pay Merchant Identity Certificate. There also the Apple Pay Payment Processing Certificate, I'm not sure what this is to be used for, yet.
There was a problem hiding this comment.
https://developer.apple.com/documentation/apple_pay_on_the_web/configuring_your_environment
Payment Processing Certificate. A certificate associated with your merchant ID, used to secure transaction data. Apple Pay servers use the certificate’s public key to encrypt payment data. You, or your payment service provider, use the private key to decrypt data to process payments. See Create a payment processing certificate for the setup steps.
Merchant Identity Certificate. A Transport Layer Security (TLS) certificate associated with your merchant ID, used to authenticate your sessions with the Apple Pay servers. The merchant identity certificate is only required for Apple Pay on the web; it isn't needed for apps. See Create a merchant identity certificate for the setup steps.
So I guess the Merchant Identity Certificate is a TLS client cert that can be handled by a merchant (which is not in scope for PCI DSS) and the Payment Processing Certificate is the cert to which they payment data is encrypted to which should thus be handled by a PCI DSS approved party (usually the PSP/gateway).
There was a problem hiding this comment.
👍
However, I agree with the suggestions put forth by @kse-clearhaus .
ct-clearhaus
dismissed stale reviews from mt-clearhaus and kse-clearhaus
via
21f991b
README.md
Outdated
| ## Complete test using `pedicel-pay` | ||
| ## Example | ||
|
|
||
| Creating keys, certificates, CSRs, do signing, etc. is a tad cumbersome, so we |
Please feel free to push improvements 🙂