Add an oAuth HMAC authentication

[humancopy]

This adds the ability to authenticate the request from Shopify using the HMAC param. I also modified the readme to reflect this change.

[Ninigi]

As always, thank you for contributing 👍

Could you move the HMAC auth into its own module?

There is a problem with authenticating hmac in plug based applications, because some plugs (json parse for example) consume the original request body, and the params map is not in the same order as the original params, so the hmac comes out different. This is no problem with query params, but validating webhooks can be tricky to say the least. You can try sorting the params, but iirc if you receive a list, or nested params, it is almost impossible to sort them back into the original format.
So your best bet would be to read the original body - which requires the plug to be called before any other plug that could consume the body - and put the original body back into the plug after verification.

This is fine for a first draft, but if you want to address the issue I just mentioned, feel free to take some time. If not, let me know and I will take a closer look at the rest 😄

[humancopy]

Yes I actually ran into a problem while using this with parameters coming in for order ids. So I'll smooth that out :)

Where did you have in mind to put the HMAC auth validation? I thought it actually fits in the oAuth module because it's only for oAuth, they have a different authentication scheme for Webhooks.

[Ninigi]

Yes I actually ran into a problem while using this with parameters coming in for order ids. So I'll smooth that out :)

👍

Where did you have in mind to put the HMAC auth validation? I thought it actually fits in the oAuth module because it's only for oAuth, they have a different authentication scheme for Webhooks.

Maybe something like HMACAuth and then we could pattern match on the different schemes? Just for separation of concerns, I think the hmac validation logic should have it's own module. Does not have to be exactly this, but:

HMACAuth.check(params, :oauth)
# and
HMACAuth.check(params, :webhook)

[humancopy]

Ok cool, I'll work out some module for the auth ;)

As for the ids parameter, I found that it's a specific issue with bulk menu items, and there is a suggested solution though I haven't managed to make it work validate ... It seems Shopify is quite vague about this scenario:

https://community.shopify.com/c/Shopify-APIs-SDKs/HMAC-calculation-vs-ids-arrays/m-p/261154

[humancopy]

I fixed the issue with the ids parameter in 1ed38f8

It's crazy Shopify don't URI encode it 😮

[nsweeting]

I would maybe try and keep the PR focused on a simple hmac authentication method. One that could be applied against both oauth and webhooks.

Something akin to Shopify.HMAC.authenticate(secret, raw_body) - or something like that. The logic surrounding the details for auth'ing oauth vs webhooks can be kept in their respective modules.

One thing to be aware of is that we need to prevent timing attacks. I've typically done this with Plug.Crypto.secure_compare/2. But we could probably just copy the code to remove the need to add the dependency.

Typically I've been using the ueberauth_shopify library for performing oauth. Ueberuath has become the elixir standard for oauth. Couple thoughts:

1. We could add ueberauth support in this library. The actual hmac authentication could be performed within that context then (using the HMAC module).
2. A PR could be made on that library, as I dont believe it performs hmac auth at the moment...

[humancopy]

I don't know if it's a good idea to add a dependency for ueberauth_shopify just to make the authentication. Unless I'm missing something why it's good to use that instead of the code already in the lib I'd be happy to understand the benefits :)

As for HMAC, it's for authenticating all Shopify requests and not just the preliminary auth one so I think it's better to implement it in the library. Again, please enlighten me if I missed something ;)

I'll try to implement the module Shopify.HMAC.authenticate this week, if times allows me. Maybe it's better I open a new PR for it?