Skip to content

Commit

Permalink
Merge pull request #824 from Shopify/liz/udpate-v10-migration-guide
Browse files Browse the repository at this point in the history
Update migration guide with lineItemBilling flag updates
  • Loading branch information
lizkenyon authored Apr 25, 2024
2 parents ec0a154 + ea6d7bb commit 0e36531
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 12 deletions.
7 changes: 7 additions & 0 deletions .changeset/fifty-trains-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@shopify/shopify-api": major
---

Webhook validation will now return a different `reason` value when the HMAC value is missing from the request. Instead of returning `WebhookValidationErrorReason.MissingHeaders` as it does for the other headers it validates, it will now return a new `WebhookValidationErrorReason.MissingHmac` error so this check matches other HMAC validations.

If your app doesn't explicitly check for the error after calling `webhook.validate()`, you don't need to make any changes.
5 changes: 5 additions & 0 deletions .changeset/tricky-actors-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@shopify/shopify-api": patch
---

Postponed deprecating the GraphQL clients' `query` method because they haven't been deprecated for long enough. They'll be removed when v11 is released instead.
66 changes: 66 additions & 0 deletions packages/apps/shopify-api/docs/migrating-to-v10.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,69 @@ This document outlines the changes you need to make to your app to migrate from
The `scopes` property on the config object is now optional. If your app is using the new [managed install flow](https://shopify.dev/docs/apps/auth/installation), it is now recommended you omit the `scopes` property from the config object.

Using both the `scopes` property and managed install can lead to unexpected behavior if these values are not kept in sync.

If you are directly accessing the scopes from the config object, you should update your code to handle the case where the `scopes` property is not present.

For example, but not limited to:

```js
// Before
const scopes = shopify.config.scopes.toString();

// After
const scopes = shopify.config.scopes ? shopify.config.scopes.toString() : '';
```

## v10_lineItemBilling future flag has been renamed to lineItemBilling

The `lineItemBilling` feature will **not** be enabled by default in v10. Because of this it has been renamed `lineItemBilling`. If you are using the `v10_lineItemBilling` future flag, you can optionally update your code to use the `lineItemBilling` feature flag instead.

## Webhook validation no longer returns `MissingHeaders` when HMAC header is missing

Webhook validation will now return a different `reason` value when the HMAC value is missing from the request.

Instead of returning `WebhookValidationErrorReason.MissingHeaders` as it does for the other headers it validates, it will now return a new `WebhookValidationErrorReason.MissingHmac` error so this check matches other HMAC validations.

```ts
import {type WebhookValidationErrorReason} from '@shopify/shopify-api';

const check = await shopify.webhooks.validate({
rawBody: (req as any).rawBody,
rawRequest: req,
rawResponse: res,
});

// Before
if (
!check.valid &&
check.reason === WebhookValidationErrorReason.MissingHeaders &&
check.missingHeaders.includes(ShopifyHeader.Hmac)
) {
// Handle error
}

// After
if (!check.valid && check.reason === WebhookValidationErrorReason.MissingHmac) {
// Handle error
}
```

## Internal build paths changed to introduce ESM and CJS exports

We started exporting both CJS and ESM outputs in this version, which affected how we export the files from the package internally.

While this should have no effect on most apps, if you're directly importing a file from the package, its path will have changed.

Regular imports for package files remain unchanged.

```ts
// Before
import 'node_modules/@shopify/shopify-api/lib/clients/admin/graphql/client';
import '@shopify/shopify-api/adapters/node';

// After
// Add `dist/esm|cjs/` before the file
import 'node_modules/@shopify/shopify-api/dist/esm/lib/clients/admin/graphql/client';
// Unchanged
import '@shopify/shopify-api/adapters/node';
```
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class GraphqlClient {
params: GraphqlParams,
): Promise<RequestReturn<T>> {
logger(this.graphqlClass().config).deprecated(
'10.0.0',
'11.0.0',
'The query method is deprecated, and was replaced with the request method.\n' +
'See the migration guide: https://github.com/Shopify/shopify-app-js/blob/main/packages/apps/shopify-api/docs/migrating-to-v9.md#using-the-new-clients.',
);
Expand Down
2 changes: 1 addition & 1 deletion packages/apps/shopify-api/lib/clients/storefront/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class StorefrontClient {
params: GraphqlParams,
): Promise<RequestReturn<T>> {
logger(this.storefrontClass().config).deprecated(
'10.0.0',
'11.0.0',
'The query method is deprecated, and was replaced with the request method.\n' +
'See the migration guide: https://github.com/Shopify/shopify-app-js/blob/main/packages/apps/shopify-api/docs/migrating-to-v9.md#using-the-new-clients.',
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ describe('shopify.webhooks.validate', () => {
it.each([
{headers: {apiVersion: ''}, missingHeader: ShopifyHeader.ApiVersion},
{headers: {domain: ''}, missingHeader: ShopifyHeader.Domain},
{headers: {hmac: ''}, missingHeader: ShopifyHeader.Hmac},
{headers: {topic: ''}, missingHeader: ShopifyHeader.Topic},
{headers: {webhookId: ''}, missingHeader: ShopifyHeader.WebhookId},
])(`returns false on missing header $missingHeader`, async (config) => {
Expand Down Expand Up @@ -73,6 +72,22 @@ describe('shopify.webhooks.validate', () => {
});
});

it('returns false on missing HMAC', async () => {
const shopify = shopifyApi(testConfig());
const app = getTestApp(shopify);

const response = await request(app)
.post('/webhooks')
.set(headers({hmac: ''}))
.send(rawBody)
.expect(200);

expect(response.body.data).toEqual({
valid: false,
reason: WebhookValidationErrorReason.MissingHmac,
});
});

it('returns false on invalid HMAC', async () => {
const shopify = shopifyApi(testConfig());
const app = getTestApp(shopify);
Expand Down
4 changes: 4 additions & 0 deletions packages/apps/shopify-api/lib/webhooks/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ async function handleInvalidWebhook(
response.statusCode = StatusCode.BadRequest;
response.errorMessage = 'No body was received when processing webhook';
break;
case WebhookValidationErrorReason.MissingHmac:
response.statusCode = StatusCode.BadRequest;
response.errorMessage = `Missing HMAC header in request`;
break;
case WebhookValidationErrorReason.InvalidHmac:
response.statusCode = StatusCode.Unauthorized;
response.errorMessage = `Could not validate request HMAC`;
Expand Down
9 changes: 0 additions & 9 deletions packages/apps/shopify-api/lib/webhooks/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,6 @@ export function validateFactory(config: ConfigInterface) {
});

if (!validHmacResult.valid) {
// Deprecated: this is for backwards compatibility with the old HMAC validation
// This will be removed in the next major version, and missing_hmac will be returned instead of missing_header when the hmac is missing
if (validHmacResult.reason === ValidationErrorReason.MissingHmac) {
return {
valid: false,
reason: WebhookValidationErrorReason.MissingHeaders,
missingHeaders: [ShopifyHeader.Hmac],
};
}
if (validHmacResult.reason === ValidationErrorReason.InvalidHmac) {
const log = logger(config);
await log.debug(
Expand Down

0 comments on commit 0e36531

Please sign in to comment.