From 7e0637fc85c0d690d318a8b50a48f3b9d375bc19 Mon Sep 17 00:00:00 2001 From: JohnAlbin Date: Sat, 24 Feb 2024 12:27:39 +0800 Subject: [PATCH] refactor(drupal-next): do not pass withAuth to init param of fetcher The `withAuth` option belongs to the DrupalClient.fetch() method but was mistakenly being passed to the init param of the global fetch and custom fetcher functions. --- packages/next-drupal/src/client.ts | 29 ++++---- .../fetch-related-methods.test.ts | 39 +++++++---- .../DrupalClient/pages-router-methods.test.ts | 70 ++++++++++++------- .../next-drupal/tests/utils/mocks/data.ts | 9 ++- 4 files changed, 93 insertions(+), 54 deletions(-) diff --git a/packages/next-drupal/src/client.ts b/packages/next-drupal/src/client.ts index c64df2d1..e8645702 100644 --- a/packages/next-drupal/src/client.ts +++ b/packages/next-drupal/src/client.ts @@ -233,7 +233,10 @@ export class DrupalClient { this.tokenExpiresOn = Date.now() + token.expires_in * 1000 } - async fetch(input: RequestInfo, init?: FetchOptions): Promise { + async fetch( + input: RequestInfo, + { withAuth, ...init }: FetchOptions = {} + ): Promise { init = { ...init, credentials: "include", @@ -245,10 +248,10 @@ export class DrupalClient { // Using the auth set on the client. // TODO: Abstract this to a re-usable. - if (init?.withAuth) { + if (withAuth) { this.debug(`Using authenticated request.`) - if (init.withAuth === true) { + if (withAuth === true) { if (typeof this._auth === "undefined") { throw new Error( "auth is not configured. See https://next-drupal.org/docs/client/auth" @@ -289,32 +292,32 @@ export class DrupalClient { `${this._auth.token_type} ${this._auth.access_token}` } } - } else if (typeof init.withAuth === "string") { + } else if (typeof withAuth === "string") { this.debug(`Using custom authorization header.`) - init["headers"]["Authorization"] = init.withAuth - } /* c8 ignore next 4 */ else if (typeof init.withAuth === "function") { + init["headers"]["Authorization"] = withAuth + } /* c8 ignore next 4 */ else if (typeof withAuth === "function") { this.debug(`Using custom authorization callback.`) - init["headers"]["Authorization"] = init.withAuth() - } else if (isBasicAuth(init.withAuth)) { + init["headers"]["Authorization"] = withAuth() + } else if (isBasicAuth(withAuth)) { this.debug(`Using basic authorization header.`) const basic = Buffer.from( - `${init.withAuth.username}:${init.withAuth.password}` + `${withAuth.username}:${withAuth.password}` ).toString("base64") init["headers"]["Authorization"] = `Basic ${basic}` - } else if (isClientIdSecretAuth(init.withAuth)) { + } else if (isClientIdSecretAuth(withAuth)) { // Fetch an access token and add it to the request. // Access token can be fetched from cache or using a custom auth method. - const token = await this.getAccessToken(init.withAuth) + const token = await this.getAccessToken(withAuth) if (token) { init["headers"]["Authorization"] = `Bearer ${token.access_token}` } - } /* c8 ignore next 4 */ else if (isAccessTokenAuth(init.withAuth)) { + } /* c8 ignore next 4 */ else if (isAccessTokenAuth(withAuth)) { init["headers"]["Authorization"] = - `${init.withAuth.token_type} ${init.withAuth.access_token}` + `${withAuth.token_type} ${withAuth.access_token}` } } diff --git a/packages/next-drupal/tests/DrupalClient/fetch-related-methods.test.ts b/packages/next-drupal/tests/DrupalClient/fetch-related-methods.test.ts index 3d90b1a9..104dbdd5 100644 --- a/packages/next-drupal/tests/DrupalClient/fetch-related-methods.test.ts +++ b/packages/next-drupal/tests/DrupalClient/fetch-related-methods.test.ts @@ -105,7 +105,6 @@ describe("fetch()", () => { Accept: "application/vnd.api+json", Authorization: "Basic YWRtaW46cGFzc3dvcmQ=", }, - withAuth: true, }) ) }) @@ -133,7 +132,6 @@ describe("fetch()", () => { Accept: "application/vnd.api+json", Authorization: "Basic YXJzaGFkQG5leHQtZHJ1cGFsLm9yZzphYmMxMjM=", }, - withAuth: true, }) ) }) @@ -227,7 +225,6 @@ describe("fetch()", () => { foo: "bar", Authorization: "Basic YXJzaGFkQG5leHQtZHJ1cGFsLm9yZzphYmMxMjM=", }, - withAuth: true, }) ) }) @@ -464,7 +461,9 @@ describe("getMenu()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) @@ -653,7 +652,9 @@ describe("getResource()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) @@ -803,7 +804,9 @@ describe("getResourceByPath()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.not.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: expect.anything(), + }), }) ) expect(getAccessTokenSpy).not.toHaveBeenCalled() @@ -814,7 +817,9 @@ describe("getResourceByPath()", () => { auth: mocks.auth.clientIdSecret, }) const fetchSpy = spyOnFetch() - const getAccessTokenSpy = jest.spyOn(client, "getAccessToken") + const getAccessTokenSpy = jest + .spyOn(client, "getAccessToken") + .mockImplementation(async () => mocks.auth.accessToken) await client.getResourceByPath( "/recipes/deep-mediterranean-quiche", @@ -826,7 +831,9 @@ describe("getResourceByPath()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: `${mocks.auth.accessToken.token_type} ${mocks.auth.accessToken.access_token}`, + }), }) ) expect(getAccessTokenSpy).toHaveBeenCalled() @@ -930,7 +937,9 @@ describe("getResourceCollection()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) @@ -1029,7 +1038,9 @@ describe("getSearchIndex()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) @@ -1114,7 +1125,9 @@ describe("getView()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) @@ -1179,7 +1192,9 @@ describe("translatePath()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) diff --git a/packages/next-drupal/tests/DrupalClient/pages-router-methods.test.ts b/packages/next-drupal/tests/DrupalClient/pages-router-methods.test.ts index fa4679b5..5751e69a 100644 --- a/packages/next-drupal/tests/DrupalClient/pages-router-methods.test.ts +++ b/packages/next-drupal/tests/DrupalClient/pages-router-methods.test.ts @@ -232,13 +232,16 @@ describe("buildStaticPathsParamsFromPaths()", () => { describe("getAuthFromContextAndOptions()", () => { const clientIdSecret = mocks.auth.clientIdSecret + const accessToken = mocks.auth.accessToken test("should use the withAuth option if provided and NOT in preview", async () => { const client = new DrupalClient(BASE_URL, { auth: clientIdSecret, }) const fetchSpy = spyOnFetch() - jest.spyOn(client, "getAccessToken") + jest + .spyOn(client, "getAccessToken") + .mockImplementation(async () => accessToken) await client.getResourceFromContext( "node--article", @@ -253,7 +256,9 @@ describe("getAuthFromContextAndOptions()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: `${accessToken.token_type} ${accessToken.access_token}`, + }), }) ) @@ -274,11 +279,9 @@ describe("getAuthFromContextAndOptions()", () => { expect(fetchSpy).toHaveBeenLastCalledWith( expect.anything(), expect.objectContaining({ - withAuth: { - clientId: "foo", - clientSecret: "bar", - scope: "baz", - }, + headers: expect.objectContaining({ + Authorization: `${accessToken.token_type} ${accessToken.access_token}`, + }), }) ) }) @@ -296,7 +299,9 @@ describe("getAuthFromContextAndOptions()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: false, + headers: expect.not.objectContaining({ + Authorization: expect.anything(), + }), }) ) @@ -304,11 +309,9 @@ describe("getAuthFromContextAndOptions()", () => { auth: clientIdSecret, withAuth: true, }) - jest.spyOn(client2, "getAccessToken").mockImplementation(async () => ({ - token_type: "", - expires_in: 0, - access_token: "", - })) + jest + .spyOn(client2, "getAccessToken") + .mockImplementation(async () => accessToken) await client2.getResourceFromContext("node--article", { preview: false, @@ -317,7 +320,9 @@ describe("getAuthFromContextAndOptions()", () => { expect(fetchSpy).toHaveBeenLastCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: `${accessToken.token_type} ${accessToken.access_token}`, + }), }) ) }) @@ -327,7 +332,8 @@ describe("getAuthFromContextAndOptions()", () => { auth: clientIdSecret, withAuth: true, }) - const fetchSpy = spyOnFetch() + const fetchSpy = jest.spyOn(client, "fetch") + spyOnFetch() await client.getResourceFromContext("node--article", { preview: true, @@ -345,7 +351,8 @@ describe("getAuthFromContextAndOptions()", () => { const client = new DrupalClient(BASE_URL, { auth: clientIdSecret, }) - const fetchSpy = spyOnFetch() + const fetchSpy = jest.spyOn(client, "fetch") + spyOnFetch() await client.getResourceFromContext("node--article", { preview: true, @@ -375,7 +382,8 @@ describe("getAuthFromContextAndOptions()", () => { }, withAuth: true, }) - const fetchSpy = spyOnFetch() + const fetchSpy = jest.spyOn(client, "fetch") + spyOnFetch() await client.getResourceFromContext("node--article", { preview: true, @@ -414,7 +422,9 @@ describe("getAuthFromContextAndOptions()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: `Bearer example-token`, + headers: expect.objectContaining({ + Authorization: `Bearer example-token`, + }), }) ) }) @@ -440,7 +450,9 @@ describe("getAuthFromContextAndOptions()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: `Bearer example-token`, + headers: expect.objectContaining({ + Authorization: `Bearer example-token`, + }), }) ) }) @@ -775,7 +787,9 @@ describe("getResourceCollectionFromContext()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) @@ -981,7 +995,9 @@ describe("getResourceFromContext()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) @@ -1010,7 +1026,9 @@ describe("getResourceFromContext()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: `Bearer sample-token`, + headers: expect.objectContaining({ + Authorization: `Bearer sample-token`, + }), }) ) }) @@ -1148,7 +1166,9 @@ describe("getStaticPathsFromContext()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) @@ -1245,7 +1265,9 @@ describe("translatePathFromContext()", () => { expect(fetchSpy).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - withAuth: true, + headers: expect.objectContaining({ + Authorization: "Bearer sample-token", + }), }) ) }) diff --git a/packages/next-drupal/tests/utils/mocks/data.ts b/packages/next-drupal/tests/utils/mocks/data.ts index 95ec1895..d1caf896 100644 --- a/packages/next-drupal/tests/utils/mocks/data.ts +++ b/packages/next-drupal/tests/utils/mocks/data.ts @@ -16,18 +16,17 @@ const auth = { } as DrupalClientAuthUsernamePassword, accessToken: { access_token: "ECYM594IlARGc3S8KgBHvTpki0rDtWx6", - token_type: "bearer", + token_type: "Bearer", expires_in: 3600, } as DrupalClientAuthAccessToken, clientIdSecret: { clientId: "7795065e-8ad0-45eb-a64d-73d9f3a5e943", clientSecret: "d92Fm^ds", } as DrupalClientAuthClientIdSecret, - function: function authFunction() { + callback: function authFunction() { return "custom Authentication header from authFunction" - } as DrupalClientAuth, - customAuthenticationHeader: - "custom Authentication header from string" as DrupalClientAuth, + }, + customAuthenticationHeader: "custom Authentication header from string", } const resources = {