Shopify / quilt

[⚠️ Deprecated] A loosely related set of packages for JavaScript/TypeScript projects at Shopify
MIT License
1.7k stars 220 forks source link

GDPR Webhook Integration #2177

Open akeans-mgs opened 2 years ago

akeans-mgs commented 2 years ago

Our app was rejected by Shopify's automated submission test immediately after submission with the same reason of

App must verify the authenticity of the request from Shopify. Expected HTTP 401 (Unauthorized), but got HTTP 405 from https://8235cf20c428.ngrok.io/webhook/gdpr/shop_redact. Your app's HTTPS webhook endpoints must validate the HMAC digest of each request, and return an HTTP 401 (Unauthorized) response when rejecting a request that has an invalid digest.

Error I received

TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined in
at Hmac.update (internal/crypto/hash.js:84:11)
at receiveWebhookMiddleware( \node_modules@shopify\koa-shopify-webhooks\build\cjs\receive.js:32:63 )
at dispatch( \node_modules@shopify\koa-shopify-webhooks\node_modules\koa-compose\index.js:42:32 )
at bodyParser \node_modules\koa-bodyparser\index.js:95:11)
at processTicksAndRejections (internal/process/task_queues.js:95:5)
at \node_modules\koa-mount\index.js:58:5

You can check the code here. https://github.com/akeans-mgs/mgs_testing

BPScott commented 2 years ago

This line appears to map to passing the secret into createHmac. - The error being that the value is undefined rather than a string. That value comes from what what you set as secret when calling receiveWebhook({secret}).

In your code that seems to be the value process.env.SHOPIFY_API_SECRET. Are you sure your're setting that environment variable correctly? I suspect you're not, and you're doing receiveWebhook({secret: undefined}), which is an error.

akeans-mgs commented 2 years ago

@BPScott Hi, Thanks for your quick replay. I am sure that, environment variables are added correctly.

https://github.com/akeans-mgs/mgs_testing/blob/main/server/server.js#L22 in this line also, i am using process.env.SHOPIFY_API_SECRET to initialize shopify context.

And My App GDPR Webhook endpoints are like below gdpr-webhooks

akeans-mgs commented 2 years ago

@BPScott Thanks for reviewing. You can clone the repo : https://github.com/akeans-mgs/mgs_testing.git And check it out too.

BPScott commented 2 years ago

You can clone the repo

This wouldn't help. I don't know what value you set the SHOPIFY_API_SECRET environment variable to (and neither should you tell me), thus I wouldn't be able to test this.

What happens if you change the line https://github.com/akeans-mgs/mgs_testing/blob/main/server/server.js#L85:

- secret: process.env.SHOPIFY_API_SECRET,
+ secret: process.env.SHOPIFY_API_SECRET || '123456',

Do you get the same or a different error?

What is output if you do console.log({secret: process.env.SHOPIFY_API_SECRET}) at some point in your code?

I remain convinced that this is erroring because you're passing undefined into receiveWebhook's secret key.

akeans-mgs commented 2 years ago

@BPScott

I have placed my .env file values in .env.example for your reference.

I have tried to output the console.log({secret: process.env.SHOPIFY_API_SECRET}) at some point in server.js file. it returns the value of SHOPIFY_API_SECRET in .env

I have changed the line as you said https://github.com/akeans-mgs/mgs_testing/blob/main/server/server.js#L85

I received the same error.

TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined at Hmac.update (internal/crypto/hash.js:84:11) at receiveWebhookMiddleware (D:\xampp\htdocs\2021\shopify-app\mgs-testing\mgs_testing\node_modules\@shopify\koa-shopify-webhooks\build\cjs\receive.js:32:63) at dispatch (D:\xampp\htdocs\2021\shopify-app\mgs-testing\mgs_testing\node_modules\@shopify\koa-shopify-webhooks\node_modules\koa-compose\index.js:42:32) at bodyParser (D:\xampp\htdocs\2021\shopify-app\mgs-testing\mgs_testing\node_modules\koa-bodyparser\index.js:95:11) at processTicksAndRejections (internal/process/task_queues.js:95:5) at D:\xampp\htdocs\2021\shopify-app\mgs-testing\mgs_testing\node_modules\koa-mount\index.js:58:5

I asked you to clone the repo and create a dummy shopify app from your end to test the webhook code.

akeans-mgs commented 2 years ago

@BPScott Any Update.

akeans-mgs commented 2 years ago

Hi All, Just waiting for your response. kindly give me any update.

TrevPennington commented 2 years ago

+1

ryanwilsonperkin commented 2 years ago

It looks like the error is caused by the cleartext rawBody being passed to the HMAC digest rather than the key itself. When creating the HMAC with an undefined cleartext we get the same message

> createHmac('sha256', '123456').update(undefined, 'utf8').digest('base64')
Uncaught:
TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined
    at __node_internal_captureLargerStackTrace (node:internal/errors:464:5)
    at new NodeError (node:internal/errors:371:5)
    at Hmac.update (node:internal/crypto/hash:105:11) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Whereas if we specify an undefined key we get a slightly different error message:

> createHmac('sha256', undefined).update('asdf', 'utf8').digest('base64')
Uncaught:
TypeError [ERR_INVALID_ARG_TYPE]: The "key" argument must be of type string or an instance of ArrayBuffer, Buffer, TypedArray, DataView, KeyObject, or CryptoKey. Received undefined
    at __node_internal_captureLargerStackTrace (node:internal/errors:464:5)
    at new NodeError (node:internal/errors:371:5)
    at prepareSecretKey (node:internal/crypto/keys:570:11)
    at new Hmac (node:internal/crypto/hash:132:9)
    at createHmac (node:crypto:162:10) {
  code: 'ERR_INVALID_ARG_TYPE'
}

This would suggest that the API key is likely being set correctly, and instead the rawBody value which should be accessible on the request from koa-bodyparser is not being properly set.

ryanwilsonperkin commented 2 years ago

The parsed body value is only available if the content type that's provided specifies that the request is either json or form. Can you try adding a console.log to output the value of ctx.request.type before passing it into the receiveWebhook and let us know what it says? I can see from the stack trace that you're running this application behind XAMPP and it seems possible that the configuration of that web server is stripping or modifying the content-type header.

qiqqq commented 2 years ago

I have the same problem and this is not only for GDPR webhook but for all types of webhooks.

I noticed that receiveWebhook function from koa-shopify-webhooks uses rawBody from ctx.request to generate Hmac and this undefined and causes this error:

const {rawBody} = (ctx as ParsedContext).request;

const generatedHash = createHmac('sha256', secret)
   .update(rawBody, 'utf8')
   .digest('base64');

I am using koa-better-body npm package and I checked ctx.request.type and it is application/json .

Does someone resolved this problem?

Update: Problem is in koa-better-body package. Changing it to koa-bodyparser resolves it.