[Spec] Correctly validate payeeOrigin

[stephenmcgruer]

Fixes #149

---

Preview | Diff

[stephenmcgruer]

@annevk - PTAL (and apologies for the delay!). I wasn't sure if we should be doing the same 3rd step as postMessage does, which is:

Set targetOrigin to parsedURL's origin.

I assume this is for normalization, but I'm unclear if its desirable for SPC as it seems to be a tuple rather than a string then?

[annevk]

If you don't have an actual origin, how would you do a same origin check? Internally you also want origins to be "objects". If you need to output it at some point you can serialize it then.

[stephenmcgruer]

how would you do a same origin check

I may not be following you; nowhere in the spec is payeeOrigin subject to a same-origin check, I believe. The payeeOrigin is a claim by the SPC caller as to which origin is receiving payment, which is displayed to the user by the browser and signed into the cryptogram. It may or may not be the same as the origin that SPC was called from.

For example, in a redirect flow, merchant.com will redirect to psp.com, who may then call SPC with payeeOrigin: 'https://merchant.com'. In this flow, the browser has no way to enforce that payeeOrigin is anything in particular, but that's ok - we don't have the ability to enforce that (e.g.) the transaction amount is correct either. The browser's job is to clearly indicate to the user what transaction information they are agreeing to, and then sign that for later validation/verification by the Relying Party (e.g., the bank).

[annevk]

Okay, so you essentially use a serialized form of the origin for signing (which would still end up being compared I suspect at some other point by another party?).

In that case you do still want to extract the origin from URL. Then you probably want to check that it's not an opaque origin (and fail if it is one). And I guess you might even want to enforce that the origin's scheme is https? Then you want to serialize the origin and use the resulting value for signing.

And yeah, normalization is important here. E.g., if the supplied value is https://EXAMPLE.example/test, you want to sign with https://example.example, right?

[stephenmcgruer]

Ah yes, very good points. I'm embarrassed - I hadn't even considered that (I now assume) the URL parser would accept https://foo.com/this/is/a/path, nevermind things like opaque origins >_<.

Thanks, I'll update the PR and ping for re-review. I appreciate your help!

[stephenmcgruer]

@annevk - PTAAL. I now check the scheme is https (which I believe rules out opaque origins as well), and then overwrite the input with the normalization. It's a little dodgy to do the overwrite (since it makes the Steps to check if a payment can be made non-idempotent), but in practice it 'works' and long-term I expect us to move SPC away from PaymentRequest and to its own API shape (see #65 (comment))

[annevk]

Yeah this works I think. Modifying data is a little weird, but it's a copy so it should be okay.

[annevk]

This looks pretty solid, thanks.

(By the way, if you ever need to add something from a WHATWG specification to an anchors block, please file an issue. To a large extent they are code smell.)

[stephenmcgruer]

(By the way, if you ever need to add something from a WHATWG specification to an anchors block, please file an issue. To a large extent they are code smell.)

Ack, thanks (and thanks for the review!)