From 49c8ddbd8ff2c27dfba12bf78eb167edc5f92b32 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Sat, 30 Mar 2024 16:38:03 +0800 Subject: [PATCH 1/2] refactor(core): partially remove got --- packages/core/package.json | 5 +-- .../core/src/libraries/cloud-connection.ts | 31 ++++++++++--------- .../core/src/libraries/hook/index.test.ts | 4 ++- packages/core/src/libraries/hook/index.ts | 12 ++++--- .../core/src/libraries/hook/utils.test.ts | 6 ++-- packages/core/src/libraries/hook/utils.ts | 19 +++++++----- .../src/tests/api/hook/hook.trigger.test.ts | 4 +-- pnpm-lock.yaml | 19 ++++++++---- 8 files changed, 58 insertions(+), 42 deletions(-) diff --git a/packages/core/package.json b/packages/core/package.json index 325b7ecfdee..0183b74fa5d 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -44,6 +44,7 @@ "@logto/schemas": "workspace:^1.14.0", "@logto/shared": "workspace:^3.1.0", "@silverhand/essentials": "^2.9.0", + "@silverhand/slonik": "31.0.0-beta.2", "@simplewebauthn/server": "^9.0.0", "@withtyped/client": "^0.8.4", "camelcase": "^8.0.0", @@ -72,6 +73,7 @@ "koa-proxies": "^0.12.1", "koa-router": "^12.0.0", "koa-send": "^5.0.1", + "ky": "^1.2.3", "lru-cache": "^10.0.0", "nanoid": "^5.0.1", "oidc-provider": "^8.2.2", @@ -85,7 +87,6 @@ "roarr": "^7.11.0", "samlify": "2.8.10", "semver": "^7.3.8", - "@silverhand/slonik": "31.0.0-beta.2", "snake-case": "^4.0.0", "snakecase-keys": "^7.0.0", "zod": "^3.22.4" @@ -116,7 +117,7 @@ "jest-matcher-specific-error": "^1.0.0", "jsonc-eslint-parser": "^2.4.0", "lint-staged": "^15.0.0", - "nock": "^13.3.1", + "nock": "^14.0.0-beta.5", "node-mocks-http": "^1.12.1", "nodemon": "^3.0.0", "prettier": "^3.0.0", diff --git a/packages/core/src/libraries/cloud-connection.ts b/packages/core/src/libraries/cloud-connection.ts index e97e87d3d45..d169d64cd53 100644 --- a/packages/core/src/libraries/cloud-connection.ts +++ b/packages/core/src/libraries/cloud-connection.ts @@ -2,7 +2,7 @@ import type router from '@logto/cloud/routes'; import { cloudConnectionDataGuard, CloudScope } from '@logto/schemas'; import { appendPath } from '@silverhand/essentials'; import Client from '@withtyped/client'; -import { got } from 'got'; +import ky from 'ky'; import { z } from 'zod'; import { EnvSet } from '#src/env-set/index.js'; @@ -71,20 +71,21 @@ export class CloudConnectionLibrary { const { tokenEndpoint, appId, appSecret, resource } = await this.getCloudConnectionData(); - const httpResponse = await got.post({ - url: tokenEndpoint, - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - Authorization: `Basic ${Buffer.from(`${appId}:${appSecret}`).toString('base64')}`, - }, - form: { - grant_type: 'client_credentials', - resource, - scope: scopes.join(' '), - }, - }); - - const result = accessTokenResponseGuard.safeParse(safeParseJson(httpResponse.body)); + const text = await ky + .post(tokenEndpoint, { + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + Authorization: `Basic ${Buffer.from(`${appId}:${appSecret}`).toString('base64')}`, + }, + body: new URLSearchParams({ + grant_type: 'client_credentials', + resource, + scope: scopes.join(' '), + }), + }) + .text(); + + const result = accessTokenResponseGuard.safeParse(safeParseJson(text)); if (!result.success) { throw new Error('Unable to get access token for Cloud service'); diff --git a/packages/core/src/libraries/hook/index.test.ts b/packages/core/src/libraries/hook/index.test.ts index 5724c8f09b9..06155d02484 100644 --- a/packages/core/src/libraries/hook/index.test.ts +++ b/packages/core/src/libraries/hook/index.test.ts @@ -18,7 +18,9 @@ mockEsm('#src/utils/sign.js', () => ({ })); const { sendWebhookRequest } = mockEsm('./utils.js', () => ({ - sendWebhookRequest: jest.fn().mockResolvedValue({ statusCode: 200, body: '{"message":"ok"}' }), + sendWebhookRequest: jest + .fn() + .mockResolvedValue({ status: 200, text: async () => '{"message":"ok"}' }), generateHookTestPayload, parseResponse, })); diff --git a/packages/core/src/libraries/hook/index.ts b/packages/core/src/libraries/hook/index.ts index 7eecabd685a..0689fd7bfe4 100644 --- a/packages/core/src/libraries/hook/index.ts +++ b/packages/core/src/libraries/hook/index.ts @@ -9,7 +9,7 @@ import { } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; import { conditional, pick, trySafe } from '@silverhand/essentials'; -import { HTTPError } from 'got'; +import { HTTPError } from 'ky'; import RequestError from '#src/errors/RequestError/index.js'; import { LogEntry } from '#src/middleware/koa-audit-log.js'; @@ -106,13 +106,15 @@ export const createHookLibrary = (queries: Queries) => { }) .then(async (response) => { logEntry.append({ - response: parseResponse(response), + response: await parseResponse(response), }); }) .catch(async (error) => { logEntry.append({ result: LogResult.Error, - response: conditional(error instanceof HTTPError && parseResponse(error.response)), + response: conditional( + error instanceof HTTPError && (await parseResponse(error.response)) + ), error: conditional(error instanceof Error && String(error)), }); }); @@ -151,8 +153,8 @@ export const createHookLibrary = (queries: Queries) => { code: 'hook.endpoint_responded_with_error', }, { - responseStatus: error.response.statusCode, - responseBody: String(error.response.rawBody), + responseStatus: error.response.status, + responseBody: await error.response.text(), } satisfies HookTestErrorResponseData ); } diff --git a/packages/core/src/libraries/hook/utils.test.ts b/packages/core/src/libraries/hook/utils.test.ts index 73f3cd7f994..e59feb38472 100644 --- a/packages/core/src/libraries/hook/utils.test.ts +++ b/packages/core/src/libraries/hook/utils.test.ts @@ -1,13 +1,13 @@ import { HookEvent } from '@logto/schemas'; import { createMockUtils } from '@logto/shared/esm'; -import { got } from 'got'; +import ky from 'ky'; const { jest } = import.meta; const { mockEsm, mockEsmWithActual } = createMockUtils(jest); const post = jest - .spyOn(got, 'post') + .spyOn(ky, 'post') // @ts-expect-error .mockImplementation(jest.fn(async () => ({ statusCode: 200, body: '{"message":"ok"}' }))); @@ -44,7 +44,7 @@ describe('sendWebhookRequest', () => { }, json: testPayload, retry: { limit: 3 }, - timeout: { request: 10_000 }, + timeout: 10_000, }); }); }); diff --git a/packages/core/src/libraries/hook/utils.ts b/packages/core/src/libraries/hook/utils.ts index 79485401eff..f07f375e4c5 100644 --- a/packages/core/src/libraries/hook/utils.ts +++ b/packages/core/src/libraries/hook/utils.ts @@ -5,15 +5,18 @@ import { type HookConfig, } from '@logto/schemas'; import { conditional, trySafe } from '@silverhand/essentials'; -import { got, type Response } from 'got'; +import ky, { type KyResponse } from 'ky'; import { sign } from '#src/utils/sign.js'; -export const parseResponse = ({ statusCode, body }: Response) => ({ - statusCode, - // eslint-disable-next-line no-restricted-syntax - body: trySafe(() => JSON.parse(String(body)) as unknown) ?? String(body), -}); +export const parseResponse = async (response: KyResponse) => { + const body = await response.text(); + return { + statusCode: response.status, + // eslint-disable-next-line no-restricted-syntax + body: trySafe(() => JSON.parse(body) as unknown) ?? String(body), + }; +}; type SendWebhookRequest = { hookConfig: HookConfig; @@ -28,7 +31,7 @@ export const sendWebhookRequest = async ({ }: SendWebhookRequest) => { const { url, headers, retries } = hookConfig; - return got.post(url, { + return ky.post(url, { headers: { 'user-agent': 'Logto (https://logto.io/)', ...headers, @@ -36,7 +39,7 @@ export const sendWebhookRequest = async ({ }, json: payload, retry: { limit: retries ?? 3 }, - timeout: { request: 10_000 }, + timeout: 10_000, }); }; diff --git a/packages/integration-tests/src/tests/api/hook/hook.trigger.test.ts b/packages/integration-tests/src/tests/api/hook/hook.trigger.test.ts index 022986c04b3..08491110244 100644 --- a/packages/integration-tests/src/tests/api/hook/hook.trigger.test.ts +++ b/packages/integration-tests/src/tests/api/hook/hook.trigger.test.ts @@ -95,7 +95,7 @@ describe('trigger hooks', () => { expect( logs.some( ({ payload: { result, error } }) => - result === LogResult.Error && error === 'RequestError: Invalid URL' + result === LogResult.Error && error === 'TypeError: Failed to parse URL from not_work_url' ) ).toBeTruthy(); @@ -137,7 +137,7 @@ describe('trigger hooks', () => { // Check hook trigger log for (const [hook, expectedResult, expectedError] of [ - [hook1, LogResult.Error, 'RequestError: Invalid URL'], + [hook1, LogResult.Error, 'TypeError: Failed to parse URL from not_work_url'], [hook2, LogResult.Success, undefined], [hook3, LogResult.Success, undefined], ] satisfies Array<[Hook, LogResult, Optional]>) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ebcf70fcd8e..bb3adfdf072 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3152,6 +3152,9 @@ importers: koa-send: specifier: ^5.0.1 version: 5.0.1 + ky: + specifier: ^1.2.3 + version: 1.2.3 lru-cache: specifier: ^10.0.0 version: 10.0.0 @@ -3277,8 +3280,8 @@ importers: specifier: ^15.0.0 version: 15.0.2 nock: - specifier: ^13.3.1 - version: 13.3.1 + specifier: ^14.0.0-beta.5 + version: 14.0.0-beta.5 node-mocks-http: specifier: ^1.12.1 version: 1.12.1 @@ -16216,7 +16219,6 @@ packages: /ky@1.2.3: resolution: {integrity: sha512-2IM3VssHfG2zYz2FsHRUqIp8chhLc9uxDMcK2THxgFfv8pQhnMfN8L0ul+iW4RdBl5AglF8ooPIflRm3yNH0IA==} engines: {node: '>=18'} - dev: true /language-subtag-registry@0.3.22: resolution: {integrity: sha512-tN0MCzyWnoz/4nHS6uxdlFWoUZT7ABptwKPQ52Ea7URk6vll88bWBVhodtnlfEuCcKWNGoc+uGbw1cwa9IKh/w==} @@ -17424,6 +17426,14 @@ packages: - supports-color dev: true + /nock@14.0.0-beta.5: + resolution: {integrity: sha512-u255tf4DYvyErTlPZA9uTfXghiZZy+NflUOFONPVKZ5tP0yaHwKig28zyFOLhu8y5YcCRC+V5vDk4HHileh2iw==} + engines: {node: '>= 18'} + dependencies: + json-stringify-safe: 5.0.1 + propagate: 2.0.1 + dev: true + /node-addon-api@3.2.1: resolution: {integrity: sha512-mmcei9JghVNDYydghQmeDX8KoAm0FAiYyIcUt/N4nhyAipB17pllZQDOJD2fotxABnt4Mdz+dKTO7eftLg4d0A==} dev: true @@ -17988,9 +17998,6 @@ packages: resolution: {integrity: sha512-2GTVocFkwblV/TIg9AmT7TI2fO4xdWkyN8aFUEVtiVNWt96GTR3FgQyHFValfCbcj1k9Xf962Ws2hYXYUr9k1Q==} engines: {node: '>= 12.0.0'} hasBin: true - peerDependenciesMeta: - '@parcel/core': - optional: true dependencies: '@parcel/config-default': 2.9.3(@parcel/core@2.9.3)(postcss@8.4.31) '@parcel/core': 2.9.3 From 6046d189f992f05e466ffff9348603df945c59b6 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Sat, 30 Mar 2024 16:56:45 +0800 Subject: [PATCH 2/2] refactor: use shared form-urlencoded headers --- packages/core/src/libraries/cloud-connection.ts | 3 ++- packages/integration-tests/src/api/application.ts | 5 ++--- .../src/tests/api/oidc/refresh-token-grant.test.ts | 5 ++--- packages/shared/src/utils/fetch.ts | 4 ++++ packages/shared/src/utils/index.ts | 1 + 5 files changed, 11 insertions(+), 7 deletions(-) create mode 100644 packages/shared/src/utils/fetch.ts diff --git a/packages/core/src/libraries/cloud-connection.ts b/packages/core/src/libraries/cloud-connection.ts index d169d64cd53..f1530e3ae9d 100644 --- a/packages/core/src/libraries/cloud-connection.ts +++ b/packages/core/src/libraries/cloud-connection.ts @@ -1,5 +1,6 @@ import type router from '@logto/cloud/routes'; import { cloudConnectionDataGuard, CloudScope } from '@logto/schemas'; +import { formUrlEncodedHeaders } from '@logto/shared'; import { appendPath } from '@silverhand/essentials'; import Client from '@withtyped/client'; import ky from 'ky'; @@ -74,7 +75,7 @@ export class CloudConnectionLibrary { const text = await ky .post(tokenEndpoint, { headers: { - 'Content-Type': 'application/x-www-form-urlencoded', + ...formUrlEncodedHeaders, Authorization: `Basic ${Buffer.from(`${appId}:${appSecret}`).toString('base64')}`, }, body: new URLSearchParams({ diff --git a/packages/integration-tests/src/api/application.ts b/packages/integration-tests/src/api/application.ts index 91ba032b37a..ccb040c101c 100644 --- a/packages/integration-tests/src/api/application.ts +++ b/packages/integration-tests/src/api/application.ts @@ -6,6 +6,7 @@ import { type Role, type ProtectedAppMetadata, } from '@logto/schemas'; +import { formUrlEncodedHeaders } from '@logto/shared'; import { conditional } from '@silverhand/essentials'; import { authedAdminApi, oidcApi } from './api.js'; @@ -99,9 +100,7 @@ export const generateM2mLog = async (applicationId: string) => { // This is a token request with insufficient parameters and should fail. We make the request to generate a log for the current machine to machine app. return oidcApi.post('token', { - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, + headers: formUrlEncodedHeaders, body: new URLSearchParams({ client_id: id, client_secret: secret, diff --git a/packages/integration-tests/src/tests/api/oidc/refresh-token-grant.test.ts b/packages/integration-tests/src/tests/api/oidc/refresh-token-grant.test.ts index 3b5febf6239..6696d1f57c5 100644 --- a/packages/integration-tests/src/tests/api/oidc/refresh-token-grant.test.ts +++ b/packages/integration-tests/src/tests/api/oidc/refresh-token-grant.test.ts @@ -3,6 +3,7 @@ import assert from 'node:assert'; import { decodeAccessToken } from '@logto/js'; import { type LogtoConfig, Prompt, PersistKey } from '@logto/node'; import { GrantType, InteractionEvent, demoAppApplicationId } from '@logto/schemas'; +import { formUrlEncodedHeaders } from '@logto/shared'; import { isKeyInObject, removeUndefinedKeys } from '@silverhand/essentials'; import { createRemoteJWKSet, jwtVerify } from 'jose'; import ky, { HTTPError } from 'ky'; @@ -51,9 +52,7 @@ class MockOrganizationClient extends MockClient { try { const json = await ky .post(`${this.config.endpoint}/oidc/token`, { - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, + headers: formUrlEncodedHeaders, body: new URLSearchParams( removeUndefinedKeys({ grant_type: GrantType.RefreshToken, diff --git a/packages/shared/src/utils/fetch.ts b/packages/shared/src/utils/fetch.ts new file mode 100644 index 00000000000..4ead7edb092 --- /dev/null +++ b/packages/shared/src/utils/fetch.ts @@ -0,0 +1,4 @@ +/** The necessary headers for sending a form-urlencoded request. */ +export const formUrlEncodedHeaders = Object.freeze({ + 'content-type': 'application/x-www-form-urlencoded', +}); diff --git a/packages/shared/src/utils/index.ts b/packages/shared/src/utils/index.ts index 9861d6aa081..6a04dd7b093 100644 --- a/packages/shared/src/utils/index.ts +++ b/packages/shared/src/utils/index.ts @@ -4,3 +4,4 @@ export * from './id.js'; export * from './user-display-name.js'; export * from './phone.js'; export * from './sub-domain.js'; +export * from './fetch.js';