Don't ignore previous headers when beginning OAuth

Author: paulomarg

CHANGELOG.md

@@ -7,6 +7,7 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
 
+- [Patch] Don't ignore previous headers when beginning OAuth [#652](https://github.com/Shopify/shopify-api-js/pull/652)
 - [Patch] Export missing client types from package [#648](https://github.com/Shopify/shopify-api-js/pull/648)
 - [Patch] Add an info-level log of API library version and runtime environment string during initialization, to aid in troubleshooting [650](https://github.com/Shopify/shopify-api-js/pull/650)
 

adapters/node/adapter.ts

@@ -29,6 +29,18 @@ export async function nodeConvertRequest(
   };
 }
 
+export async function nodeConvertIncomingResponse(
+  adapterArgs: NodeAdapterArgs,
+): Promise<NormalizedResponse> {
+  return {
+    statusCode: adapterArgs.rawResponse.statusCode,
+    statusText: adapterArgs.rawResponse.statusMessage,
+    headers: canonicalizeHeaders(
+      adapterArgs.rawResponse.getHeaders() as any as Headers,
+    ),
+  } as NormalizedResponse;
+}
+
 export async function nodeConvertAndSendResponse(
   response: NormalizedResponse,
   adapterArgs: NodeAdapterArgs,

adapters/node/index.ts

@@ -3,6 +3,7 @@ import crypto from 'crypto';
 import {
   setAbstractFetchFunc,
   setAbstractConvertRequestFunc,
+  setAbstractConvertIncomingResponseFunc,
   setAbstractConvertResponseFunc,
   setAbstractConvertHeadersFunc,
   setAbstractRuntimeString,
@@ -12,13 +13,15 @@ import {
 import {
   nodeFetch,
   nodeConvertRequest,
+  nodeConvertIncomingResponse,
   nodeConvertAndSendResponse,
   nodeConvertAndSetHeaders,
   nodeRuntimeString,
 } from './adapter';
 
 setAbstractFetchFunc(nodeFetch);
 setAbstractConvertRequestFunc(nodeConvertRequest);
+setAbstractConvertIncomingResponseFunc(nodeConvertIncomingResponse);
 setAbstractConvertResponseFunc(nodeConvertAndSendResponse);
 setAbstractConvertHeadersFunc(nodeConvertAndSetHeaders);
 setAbstractRuntimeString(nodeRuntimeString);

docs/guides/runtimes.md

@@ -18,6 +18,10 @@ These are the functions you'll need to create:
 - `setAbstractRuntimeString`
 - `setCrypto`
 
+For runtimes that pass in a response object, such as Node.js, you'll also need to create:
+
+- `setAbstractConvertIncomingResponseFunc`
+
 Below is a _very_ simplified example with some functions:
 
 ```ts

lib/auth/oauth/oauth.ts

@@ -11,6 +11,7 @@ import {httpClientClass} from '../../clients/http_client/http_client';
 import {DataType, RequestReturn} from '../../clients/http_client/types';
 import {
   abstractConvertRequest,
+  abstractConvertIncomingResponse,
   abstractConvertResponse,
   abstractConvertHeaders,
   AdapterResponse,
@@ -55,8 +56,9 @@ export function begin(config: ConfigInterface) {
 
     const cleanShop = sanitizeShop(config)(shop, true)!;
     const request = await abstractConvertRequest(adapterArgs);
+    const response = await abstractConvertIncomingResponse(adapterArgs);
 
-    const cookies = new Cookies(request, {} as NormalizedResponse, {
+    const cookies = new Cookies(request, response, {
       keys: [config.apiSecretKey],
       secure: true,
     });
@@ -81,13 +83,12 @@ export function begin(config: ConfigInterface) {
     processedQuery.putAll(query);
 
     const redirectUrl = `https://${cleanShop}/admin/oauth/authorize${processedQuery.stringify()}`;
-    const response: NormalizedResponse = {
-      statusCode: 302,
-      statusText: 'Found',
-      headers: {
-        ...cookies.response.headers!,
-        Location: redirectUrl,
-      },
+    response.statusCode = 302;
+    response.statusText = 'Found';
+    response.headers = {
+      ...response.headers,
+      ...cookies.response.headers!,
+      Location: redirectUrl,
     };
 
     log.debug(`OAuth started, redirecting to ${redirectUrl}`, {shop, isOnline});

runtime/http/index.ts

@@ -1,6 +1,7 @@
 import type {
   AbstractFetchFunc,
   AbstractConvertRequestFunc,
+  AbstractConvertIncomingResponseFunc,
   AbstractConvertResponseFunc,
   NormalizedResponse,
   AbstractConvertHeadersFunc,
@@ -41,6 +42,16 @@ export function setAbstractConvertRequestFunc(
   abstractConvertRequest = func;
 }
 
+// By default we just return an empty NormalizedResponse because not all adapters will need to convert an incoming response
+// eslint-disable-next-line import/no-mutable-exports
+export let abstractConvertIncomingResponse: AbstractConvertIncomingResponseFunc =
+  () => Promise.resolve({} as NormalizedResponse);
+export function setAbstractConvertIncomingResponseFunc(
+  func: AbstractConvertIncomingResponseFunc,
+) {
+  abstractConvertIncomingResponse = func;
+}
+
 // eslint-disable-next-line import/no-mutable-exports
 export let abstractConvertResponse: AbstractConvertResponseFunc = () => {
   throw new Error(

runtime/http/types.ts

@@ -32,6 +32,10 @@ export type AbstractConvertRequestFunc = (
   adapterArgs: AdapterArgs,
 ) => Promise<NormalizedRequest>;
 
+export type AbstractConvertIncomingResponseFunc = (
+  adapterArgs: AdapterArgs,
+) => Promise<NormalizedResponse>;
+
 export type AbstractConvertResponseFunc = (
   response: NormalizedResponse,
   adapterArgs: AdapterArgs,