Should SPC allow for a failed download of the payment instrument icon?

[stephenmcgruer]

In discussions with a partner who is using SPC and acting as the Relying Party, they stated that they would prefer for the card art icon to be a 'best effort' feature - that is, if the browser cannot successfully fetch the icon it should not fail SPC. Instead, the browser would be allowed to show a placeholder or no icon (whatever it felt was appropriate), and then the signed assertion would indicate that the icon was not shown (e.g. by excluding the field, or including it but as an empty string).

I'm interested in hearing folks thoughts on this; my personal preference is that we move towards data URLs instead (issue #101) which cannot fail, but as long as we keep supporting 'normal' URLs then this issue is an interesting question.

[rsolomakhin]

data URLs instead which cannot fail

A data URL can fail to be parsed in PNG format, so a failure case would still be good to specify in our algorithms.

[davordavidovic]

To extend on this issue a bit and note that in our case the icon can be fetched successfully (through a GET request for example) and we use it in other places in our platform, but SPC is throwing an error saying that it cannot be fetched. My assumption is that the failure is because of parsing issues. We should have a more accurate error there (eg: "failed parsing/resizing/converting icon" or something) since "icon cannot be fetched" sounds like the resource does not exist or there are missing permissions while it clearly is not the case. But long term I think whatever is causing the error on spc's side should be more flexible especially for .svg images, or we should have guidelines on the format/size of icons.

[Goosth]

I think we should consider making the Icon URL optional in the definition. This will then allow us to fail if it cannot be processed, or allow the RP not to provide it if they do not have it available?

[stephenmcgruer]

To extend on this issue a bit and note that in our case the icon can be fetched successfully (through a GET request for example) and we use it in other places in our platform, but SPC is throwing an error saying that it cannot be fetched. My assumption is that the failure is because of parsing issues.

To be clear; I consider this a Chrome bug (tracked by https://crbug.com/1246391), and not an issue for the spec. In the spec we use the fetching an image resource algorithm and I would be surprised if it doesn't support SVG! (If it doesn't, we should switch :D).

We should have a more accurate error there (eg: "failed parsing/resizing/converting icon" or something) since "icon cannot be fetched" sounds like the resource does not exist or there are missing permissions while it clearly is not the case.

I definitely agree with this. In Chrome, I consider this a bug where we're being overly sensitive about privacy (we're returning the generic WebAuthn error I think, and that's not needed here afaik. We can tell the developer the image download failed).

From the spec side, the SPC spec says to return false in this case for the steps to check if a payment can be made. This actually causes PaymentRequest to just ignore the handler, which will result in a NotSupportedError being thrown since no handlers match. That's not the best behavior, but I think @ianbjacobs would argue that it's yet-another reason we should pull SPC into its own API and let it throw whatever exceptions it wants ;)

I think we should consider making the Icon URL optional in the definition. This will then allow us to fail if it cannot be processed, or allow the RP not to provide it if they do not have it available?

This sounds (and please correct me if I misread you!) like you are not in favour of proceeding in the case that:

1. The caller provides an image URL, BUT
2. The browser is unable to fetch or unable to parse the image at that URL.

This would mean that Davor's case (which tbf appears to be a bug in Chrome) would continue to result in SPC rejecting the call because the SVG cannot be fetched. I believe Davor is arguing that they consider the icon a 'nice to have but not vital', such that they want it to show if the URL works, but they still want SPC to function if the URL is bad.

[Goosth]

If an image was provided in the call, there's a fair expectation that it should be shown. If an RP does not have an image available, they don't have to make one up. By throwing an accurate image-not-found exception, the merchant would be able to make a 'second call' without the image (since it's optional), if they have a mandate from the RP to do that.

[rsolomakhin]

the merchant would be able to make a 'second call' without the image

Please keep in mind that a user gesture should be required for this call (even if Chrome's implementation may not require it for now due to historical reasons). It may be awkward for the website to ask the user to click the [ Confirm ] button twice.

[stephenmcgruer]

To give a direction for the upcoming TPAC discussion on this issue: Given lack of interest, I propose that we drop this conversation and continue to let a failed image download/decode blocking

[adrianhopebailie]

My proposal during discussion at TPAC was that the assertion (clientData JSON) should contain an optional bit that indicates the instrument image was not shown. RPs can choose to consider this in their risk evaluation.

@stephenmcgruer points out that this means the failure could occur late (i.e. the user authenticates but the tx is rejected by the RP) and if the merchant knows it will fail it might wish to do a different form of authentication instead to reduce friction.

So my updated proposal would be to also have a way for the caller to know that the image was not shown, perhaps through an event that can be caught by the caller. If the caller doesn't listen for this event then the browser simply continues.

[adrianhopebailie]

Concrete proposal (modified example from spec):

if (!window.PaymentRequest) { /* PaymentRequest not available; merchant should fallback to traditional flows */ }

const request = new PaymentRequest([{
  supportedMethods: "secure-payment-confirmation",
  data: {
    // List of credential IDs obtained from the bank.
    credentialIds,

    // The challenge is also obtained from the bank.
    challenge: new Uint8Array([21,31,105 /* 29 more random bytes generated by the bank */]),

    instrument: {
      displayName: "Fancy Card ****1234",
      icon: "https://fancybank.com/card-art.png",
    },

    payeeOrigin: "https://merchant.com",

    timeout: 360000,  // 6 minutes
  }], {
    total: {
      label: "Total",
      amount: {
        currency: "USD",
        value: "5.00",
      },
    },
  });

try {
  const canMakePayment = await request.canMakePayment();
  if (!canMakePayment) { throw new Error('Cannot make payment'); }

  request.on("imageDisplayError", () => {
    // Handle the fact that SPC couldn't show the instrument image
  })

  const response = await request.show();
  await response.complete('success');

  // response.data is a PublicKeyCredential, with a clientDataJSON that
  // contains the transaction data for verification by the issuing bank.

  /* send response.data to the issuing bank for verification */
} catch (err) {
  /* SPC cannot be used; merchant should fallback to traditional flows */
}

Response could include:

PublicKeyCredential {
    id: 'ADSUllKQmbqdGtpu4sjseh4cg2TxSvrbcHDTBsv4NSSX9...',
    rawId: [],
    response: AuthenticatorAssertionResponse {
        authenticatorData: [],
        clientDataJSON: {
            "instrumentDisplay" : "labelOnly"
        },
        signature: [],
        userHandle: [],
    },
    type: 'public-key'
}

Here, instrumentDisplay is an optional property which defaults to ImageAndLabel but can contain imageOnly or labelOnly too.

[davordavidovic]

Since svg support was fixed in the meantime, I think we should not have many cases where the icon is downloadable, but cannot be parsed by SPC. However, I still think the error should explain why authentication failed in the case of a problem with the icon, for monitoring purposes.

[stephenmcgruer]

However, I still think the error should explain why authentication failed in the case of a problem with the icon, for monitoring purposes.

Agree; I have filed https://crbug.com/1263500 now

[stephenmcgruer]

Agree; I have filed https://crbug.com/1263500 now

This bug is fixed; as of Chrome M98 (in canary now, planned schedule is beta in January and stable in February), the show() promise will be rejected without showing UX if the image cannot be downloaded or decoded.

[stephenmcgruer]

So my updated proposal would be to also have a way for the caller to know that the image was not shown, perhaps through an event that can be caught by the caller. If the caller doesn't listen for this event then the browser simply continues.

When would the event fire, and what would you expect the caller to do when it does?

Note that I think for privacy we would have to fire the event irregardless of what UX shows (transaction UX or no-matching-creds UX), probably just based on when the image download/decode fails.

And now for my idea: if we're going to support this (and that's not clear to me still :p), how about an option in the input data? For example:

dictionary PaymentCredentialInstrument {
    required DOMString displayName;
    required USVString icon;
    required bool iconMustBeShown = true;
};

We'd sign over iconMustBeShown too, so that the RP knows what the merchant asked for.

[stephenmcgruer]

Hey folks; FYI there is now a pull request (#171) for our proposal on how to resolve this issue, as we've heard partner feedback that this is useful to them.

The proposal is roughly aligned with what I posted above:

1. A new optional dictionary member, iconMustBeShown:

dictionary PaymentCredentialInstrument {
    required DOMString displayName;
    required USVString icon;
    boolean iconMustBeShown = true;
};

2. If set to false, the browser will ignore errors whilst fetching/decoding the icon, and will clear the icon URL to be empty. This clearing is how the RP can detect that the icon wasn't shown (along with iconMustBeShown being included in the assertion to indicate that the caller said the icon wasn't required).

I'd be interested in feedback from folks on whether this seems reasonable, particularly @jcemer-stripe . One consideration is whether we want to encode the information different in the assertion. An more complex alternative would be to:

1. Turn PaymentCredentialInstrument into three dictionaries: PaymentCredentialInstrumentInput, PaymentCredentialInstrumentOutput, and a core PaymentCredentialInstrument that they extend.
   1. Add iconMustBeShown to PaymentCredentialInstrumentInput
   2. Add iconWasShown to PaymentCredentialInstrumentOutput.
   3. Change SecurePaymentConfirmationRequest to have a PaymentCredentialInstrumentInput.
   4. Change AuthenticationExtensionsPaymentInput and CollectedClientAdditionalPaymentData to have a PaymentCredentialInstrumentOutput.
   5. In the steps to respond to a payment request, step 2, set instrument to be a PaymentCredentialInstrumentOutput whose iconWasShown field is set according to whether or not the icon was shown or not.

That felt like overkill to me, but if folks feel the 'clearing' approach has problems we could consider this alternative.

[jcemer-stripe]

The iconMustBeShown flag is clever, it makes total sense to me. It is also great that it is true by default which allow more advanced use cases to turn it off if required.

Regarding the clientDataJSON, it makes sense to me to clear (even better remove) the icon in instrument in case of a failure. I don't think that iconMustBeShown needs to be added because it won't make any difference in case the icon was displayed for example. It will also for sure be false when icon isn't present. My understanding is that clientDataJSON should keep information of what was displayed to the user combined with origins (and rp).

[stephenmcgruer]

Thanks @jcemer-stripe , I appreciate the input.

Regarding the clientDataJSON, it makes sense to me to clear (even better remove) the icon in instrument in case of a failure. I don't think that iconMustBeShown needs to be added because it won't make any difference in case the icon was displayed for example. It will also for sure be false when icon isn't present. My understanding is that clientDataJSON should keep information of what was displayed to the user combined with origins (and rp).

This is interesting; I'm not sure if its actually feasible in WebIDL to 'remove' a dictionary field in this way. @domfarolino - do you know? The idea would be to (in certain cases) remove the icon member of PaymentCredentialInstrument before passing it to WebAuthn in this alg.

[domfarolino]

It is technically possible to remove a field yes, but I think making the field nullable (with a ?) is probably recommended? https://webidl.spec.whatwg.org/#idl-nullable-type