Skip to content
This repository has been archived by the owner on Aug 27, 2021. It is now read-only.

fix: don't redact addressLine from billingAddress #77

Merged
merged 11 commits into from
May 10, 2019

Conversation

ianbjacobs
Copy link
Contributor

@ianbjacobs ianbjacobs commented Apr 22, 2019

There should be no redaction for the billing address that is
returned once the user has accepted the payment request.

The following tasks have been completed:

Implementation commitments:

  • Chrome (link to issue)
  • Firefox
  • Edge (public signal)

Preview | Diff

Copy link
Member

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help me understand the rationale for the change?

@ianbjacobs
Copy link
Contributor Author

@marcoscaceres,

I believe this is a bug. When the payment request is done, the response data, including billing address, is returned. It was never the intention that the billing address be redacted once the user has agreed to pay with that payment instrument.

There is no reason to specify a "complete" billing address in Basic Card and then not return it under any circumstances.

Ian

@marcoscaceres
Copy link
Member

marcoscaceres commented Apr 23, 2019

Reviewing #5, I concur that "addressLine" can probably be added back in for AVS. But I'm not seeing any justification for adding "organization", "phone", "recipient" to the billing address?

@marcoscaceres
Copy link
Member

I meant #5 above.

@ianbjacobs
Copy link
Contributor Author

@marcoscaceres has convinced me that it may not be necessary to return the entirety of a (generic) PaymentAddress in a billing context. I will do some outreach to learn more whether "organization", "phone", "recipient" are required in practice. In the meantime, I have restored the redactList, but without addressLine (as Marcos indicated).

@marcoscaceres marcoscaceres changed the title Bug fix: No redaction after payment request accepted fix: don't redact addressLine Apr 25, 2019
@marcoscaceres
Copy link
Member

@zouhir, now that Microsoft has put out preview build, are you in a position to give indications of support for things related to basic card?

@marcoscaceres
Copy link
Member

Filed on bug on the Firefox side for record keeping purposes:
https://bugzilla.mozilla.org/show_bug.cgi?id=1546893

@marcoscaceres marcoscaceres changed the title fix: don't redact addressLine fix: don't redact addressLine from billingAddress Apr 25, 2019
@zouhir
Copy link
Collaborator

zouhir commented Apr 25, 2019

LGTM @ianbjacobs comment above & most recent commit changes makes total sense.

@marcoscaceres
Copy link
Member

Ok, let's run with this for now. If we need to expand this later, we can always open a new PR.

@ianbjacobs
Copy link
Contributor Author

Hi all,

I endeavored to find out whether there are some use cases for sending
the phone, organization, and recipient fields to merchants when
the PaymentAddress is used in a Basic Card billing scenario.

From my outreach I conclude that (card) information is useful both
for authorization and risk analysis.

@Krystosterone pointed to the authorize.net documentation [1] and I
similarly looked at documentation from Braintree [2], Adyen [3],
Stripe [4], and PaySafe [5]. I also spoke with Jonathan Grossar by
phone.

It seems that for authorization, the three fields are not strictly
required. However, for risk assessment (e.g., via 3DS2), I understand
that phone is a useful field.

I also concluded from discussion that "addressLine" should be returned
in the Basic Card response. We had already reached that conclusion on
GitHub.

I have thus updated the pull request so that the redactList for Basic
Card is "organization" and "recipient".

Ian

[1] https://developer.authorize.net/api/reference/index.html
[2] https://developers.braintreepayments.com/reference/response/address/ruby
[3] https://docs.adyen.com/developers/api-reference/common-api/address
[4] https://stripe.com/docs/api/cards/object
[5] https://developer.paysafe.com/en/cards/api/#/introduction/complex-json-objects/recipient

@marcoscaceres
Copy link
Member

Could you please provide a few more details about how phone plays a role? Handing out phone with the billing address seems super risky/exploitable.

@ianbjacobs
Copy link
Contributor Author

ianbjacobs commented May 2, 2019

@marcoscaceres, in cases where the merchant makes use of 3-D Secure authentication, phone is a data point that figures into that protocol.

As one person commented: "The more information provided, the better. This is due to fraud analysis being doing prior to charging the card on the PSP's side. This means that the merchant can have a transaction declined, not by the bank, but rather by the PSP itself."

Can your concern be addressed through UX? The payment handler can indicate that phone number is optional. If the user provides one, it is returned.

Ian

@marcoscaceres
Copy link
Member

Can your concern be addressed through UX?

It can. The browser wouldn’t provide the option to the user. If Firefox ever did basic card, I don’t think we would ever hand this out (or bother the user to provide it). It’s too easy for third parties to abuse.

@ianbjacobs
Copy link
Contributor Author

Should we add a privacy consideration to Basic Card at the same time? I'm happy to draft that.

@marcoscaceres
Copy link
Member

We can (we have a note about not needlessly sharing phone numbers somewhere already I think). I don’t have a horse in this race, so would be good to get opinions from other Basic Card implementers.

@rsolomakhin
Copy link
Collaborator

Privacy consideration could not hurt.

@ianbjacobs
Copy link
Contributor Author

@rsolomakhin and @marcoscaceres,

I have added a new privacy consideration paragraph; review welcome!

Copy link
Collaborator

@rsolomakhin rsolomakhin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

@marcoscaceres
Copy link
Member

Are you absolutely sure they can’t live without the phone number? I’m still very uncomfortable with this.

@ianbjacobs
Copy link
Contributor Author

@marcoscaceres, I am asking more PSPs.

Ian

@marcoscaceres
Copy link
Member

Rebased and fixed up markup.

@ianbjacobs
Copy link
Contributor Author

Hi @marcoscaceres,

Here is some additional PSP input (paraphrasing):

  1. Merchant almost always asks for phone number, especially when there are goods to be delivered.
  2. Phone not required for authorization but useful for risk assessment. Risk assessments are increasingly moving towards machine learning algorithms, so more data including phone would be useful.

I would like to propose that we address the privacy balance as follows:

Please let me know if that works for you.

Ian

@ianbjacobs ianbjacobs mentioned this pull request May 8, 2019
6 tasks
ianbjacobs and others added 11 commits May 10, 2019 12:49
returned once the user has accepted the payment request.
…on about whether all of a generic PaymentAddress is required for card billing purposes.
 - Merged two related paragraphs.
 - New paragraph on possibility of configs to share less response data.
…on about whether all of a generic PaymentAddress is required for card billing purposes.
 - Merged two related paragraphs.
 - New paragraph on possibility of configs to share less response data.
@marcoscaceres marcoscaceres merged commit bbb48fb into gh-pages May 10, 2019
@marcoscaceres marcoscaceres deleted the noredact-201904 branch May 10, 2019 02:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants