From 9e93d96f0c29554b5ddc6e4c3efc4d0b4609b5f0 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Fri, 8 Dec 2023 22:07:18 +0800 Subject: [PATCH 01/16] basic tests --- app/services/github-app-auth.server.ts | 9 ++- test/__snapshots__/auth.test.ts.snap | 7 ++ test/auth.test.ts | 101 +++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 test/__snapshots__/auth.test.ts.snap create mode 100644 test/auth.test.ts diff --git a/app/services/github-app-auth.server.ts b/app/services/github-app-auth.server.ts index 9b2a3fd28..80c4a5837 100644 --- a/app/services/github-app-auth.server.ts +++ b/app/services/github-app-auth.server.ts @@ -1,9 +1,10 @@ -import type { GitHubExtraParams } from "remix-auth-github"; -import { createAppAuth, createOAuthUserAuth } from "@octokit/auth-app"; -import { octokitFromConfig } from "~/octokit.server"; +import type {GitHubExtraParams} from "remix-auth-github"; +import {GitHubStrategy} from "remix-auth-github"; +import {createAppAuth, createOAuthUserAuth} from "@octokit/auth-app"; +// import createDebug from "debug"; +import {octokitFromConfig} from "~/octokit.server"; import _ from "lodash"; import getCache from "~/services/cache"; -import { GitHubStrategy } from "remix-auth-github"; function checkNonNull(name: string): NonNullable { const value = process.env[name]; diff --git a/test/__snapshots__/auth.test.ts.snap b/test/__snapshots__/auth.test.ts.snap new file mode 100644 index 000000000..92c115a42 --- /dev/null +++ b/test/__snapshots__/auth.test.ts.snap @@ -0,0 +1,7 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`auth should be able to call strategy.authenticate 1`] = ` +{ + "login": "login", +} +`; diff --git a/test/auth.test.ts b/test/auth.test.ts new file mode 100644 index 000000000..82e1991c8 --- /dev/null +++ b/test/auth.test.ts @@ -0,0 +1,101 @@ +import type { Session, SessionStorage } from "@remix-run/node"; +import { GitHubAppAuthStrategy } from "~/services/github-app-auth.server"; + +const sessionKey = "oauth2:session"; +const sessionStateKey = "oauth2:state"; +const options = { + sessionKey, + sessionStateKey, + sessionErrorKey: "", + sessionStrategyKey: "", + name: "", +}; + +Object.assign(process.env, { + GITHUB_APP_ID: "0", + GITHUB_APP_CLIENT_ID: "appClientId", + GITHUB_APP_CLIENT_SECRET: "appClientSecret", + GITHUB_APP_PRIVATE_KEY: "appPrivateKey", + KV_REST_API_URL: "https://kvdb.io/1234567890", + KV_REST_API_TOKEN: "kvdbtoken", +}); + +describe("auth", () => { + it("should be able to create a GitHubAppAuthStrategy", () => { + expect(mk()).toBeDefined(); + }); + + it("should be able to call strategy.authenticate", async () => { + const strategy = mk(); + const result = await strategy.authenticate( + makeRequest(), + makeSessionStorage({ [sessionKey]: { login: "login" } }), + options, + ); + expect(result).toMatchSnapshot(); + }); + + it("not logged in", async () => { + const strategy = mk(); + const result = await strategy.authenticate( + makeRequest("?state=state&code=code"), + makeSessionStorage({ + [sessionKey]: undefined, + [sessionStateKey]: "state", + }), + options, + ); + expect(result).toMatchSnapshot(); + }); +}); + +class DummySession implements Session { + id = "0"; + constructor(public data: Record) { + this.data = data; + } + get(key: string) { + return this.data[key]; + } + unset(key: string) { + delete this.data[key]; + } + set(name: string, value: any) { + this.data[name] = value; + } + has(name: string) { + return name in this.data; + } + flash(name: string, value: any) { + this.data[name] = value; + } +} + +const makeRequest = (extra?: string): Request => + ({ + url: "http://localhost:3000/callback" + (extra ?? ""), + headers: new Headers(), + }) satisfies Pick as unknown as Request; +const makeSessionStorage = (bucket: Record) => + ({ + async getSession(_cookieHeader, _options) { + return new DummySession(bucket); + }, + }) satisfies Pick< + SessionStorage<{}>, + "getSession" + > as unknown as SessionStorage<{}>; + +const mk = () => { + return new GitHubAppAuthStrategy( + { + clientID: "clientId", + clientSecret: "clientSecret", + callbackURL: "http://localhost:3000/callback", + scope: "scope", + }, + async (_params) => { + return {}; + }, + ); +}; From 6a014d31501feec9531c545a66c1af974386f733 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Sun, 10 Dec 2023 23:33:09 +0800 Subject: [PATCH 02/16] add not logged in test --- app/services/github-app-auth.server.ts | 14 ++++-- test/__snapshots__/auth.test.ts.snap | 51 +++++++++++++++++++++ test/auth.test.ts | 63 +++++++++++++++++++++++--- 3 files changed, 116 insertions(+), 12 deletions(-) diff --git a/app/services/github-app-auth.server.ts b/app/services/github-app-auth.server.ts index 80c4a5837..8bdfc9372 100644 --- a/app/services/github-app-auth.server.ts +++ b/app/services/github-app-auth.server.ts @@ -4,6 +4,7 @@ import {createAppAuth, createOAuthUserAuth} from "@octokit/auth-app"; // import createDebug from "debug"; import {octokitFromConfig} from "~/octokit.server"; import _ from "lodash"; +import type { RequestInterface } from "@octokit/types"; import getCache from "~/services/cache"; function checkNonNull(name: string): NonNullable { @@ -14,7 +15,7 @@ function checkNonNull(name: string): NonNullable { return value; } -export const appAuth = _.memoize(() => createAppAuth(getAuthConfig())); +export const appAuth = _.memoize((requestOverride?: RequestInterface) => createAppAuth(getAuthConfig())); export const getConfig = _.memoize(() => { return { @@ -30,7 +31,7 @@ export const getConfig = _.memoize(() => { }; }); -function getAuthConfig() { +function getAuthConfig(requestOverride?: RequestInterface) { return { appId: checkNonNull("GITHUB_APP_ID"), clientId: checkNonNull("GITHUB_APP_CLIENT_ID"), @@ -44,8 +45,9 @@ function getAuthConfig() { info: console.info.bind(console), }, cache: getCache(), + request: requestOverride, }; -} +}; export async function getAppOctokit() { return octokitFromConfig({ @@ -55,6 +57,8 @@ export async function getAppOctokit() { } export class GitHubAppAuthStrategy extends GitHubStrategy { + requestOverride?: RequestInterface; + async fetchAccessToken( code: string, params: URLSearchParams, @@ -63,7 +67,7 @@ export class GitHubAppAuthStrategy extends GitHubStrategy { extraParams: GitHubExtraParams; refreshToken: string; }> { - const authentication = await appAuth()({ + const authentication = await appAuth(this.requestOverride)({ type: "oauth-user", code: code, redirectUrl: params.get("redirect_uri")! as string, @@ -88,7 +92,7 @@ export class GitHubAppAuthStrategy extends GitHubStrategy { }); if (!("expiresAt" in token)) { - throw new Error("invalid response from GitHub"); + throw new Error("invalid response from GitHub: " + JSON.stringify(token)); } const now = Date.now(); diff --git a/test/__snapshots__/auth.test.ts.snap b/test/__snapshots__/auth.test.ts.snap index 92c115a42..355eecb3c 100644 --- a/test/__snapshots__/auth.test.ts.snap +++ b/test/__snapshots__/auth.test.ts.snap @@ -1,5 +1,56 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`auth not logged in 1`] = ` +[ + { + "accessToken": "fake-token", + "context": undefined, + "extraParams": { + "accessTokenExpiresIn": 74879019472, + "refreshTokenExpiresIn": 59010219472, + "tokenType": "oauth", + }, + "profile": { + "_json": { + "id": 690, + "login": "Mause", + "name": "Elli", + }, + "displayName": "Mause", + "emails": [ + { + "value": undefined, + }, + ], + "id": "690", + "name": { + "familyName": "Elli", + "givenName": "Elli", + "middleName": "Elli", + }, + "photos": [ + { + "value": undefined, + }, + ], + "provider": "github", + }, + "refreshToken": "fake-refresh-token", + "request": { + "headers": Headers { + Symbol(headers list): HeadersList { + "cookies": null, + Symbol(headers map): Map {}, + Symbol(headers map sorted): null, + }, + Symbol(guard): "none", + }, + "url": "http://localhost:3000/callback?state=state&code=code", + }, + }, +] +`; + exports[`auth should be able to call strategy.authenticate 1`] = ` { "login": "login", diff --git a/test/auth.test.ts b/test/auth.test.ts index 82e1991c8..fb2ceace6 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -1,14 +1,15 @@ import type { Session, SessionStorage } from "@remix-run/node"; import { GitHubAppAuthStrategy } from "~/services/github-app-auth.server"; +import type { RequestInterface } from "@octokit/types"; +import type { AuthenticateOptions } from "remix-auth"; const sessionKey = "oauth2:session"; const sessionStateKey = "oauth2:state"; -const options = { +const options: AuthenticateOptions = { sessionKey, - sessionStateKey, - sessionErrorKey: "", - sessionStrategyKey: "", - name: "", + sessionErrorKey: "sessionErrorKey", + sessionStrategyKey: "sessionStrategyKey", + name: "github-app-auth", }; Object.assign(process.env, { @@ -51,21 +52,27 @@ describe("auth", () => { class DummySession implements Session { id = "0"; + constructor(public data: Record) { this.data = data; } + get(key: string) { return this.data[key]; } + unset(key: string) { delete this.data[key]; } + set(name: string, value: any) { this.data[name] = value; } + has(name: string) { return name in this.data; } + flash(name: string, value: any) { this.data[name] = value; } @@ -86,6 +93,47 @@ const makeSessionStorage = (bucket: Record) => "getSession" > as unknown as SessionStorage<{}>; +async function request(url: string) { + if (url.includes("oauth")) { + return { + status: 200, + headers: { + date: "Mon, 26 Jul 2021 15:49:05 GMT", + }, + data: { + access_token: "fake-token", + refresh_token: "fake-refresh-token", + scope: "api", + expires_in: 28800, + refresh_token_expires_in: 15897600, + }, + }; + } else if (url.includes("user")) { + return { + status: 200, + data: { + id: 690, + login: "Mause", + name: "Elli", + }, + }; + } + throw new Error("unknown request: " + JSON.stringify({ url })); +} + +function merge() {} + +function parse() {} + +let defaults = { + baseUrl: "https://api.github.com", +}; +request.endpoint = { + DEFAULTS: defaults, + merge, + parse, +}; + const mk = () => { return new GitHubAppAuthStrategy( { @@ -94,8 +142,9 @@ const mk = () => { callbackURL: "http://localhost:3000/callback", scope: "scope", }, - async (_params) => { - return {}; + async (...args) => { + return args; }, + request as unknown as RequestInterface, ); }; From ba69def582af2df8e1777b2757fb595a73582910 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Sun, 10 Dec 2023 23:51:08 +0800 Subject: [PATCH 03/16] add basic login test --- test/__snapshots__/auth.test.ts.snap | 68 ++++++++++------------------ test/auth.test.ts | 9 ++-- 2 files changed, 30 insertions(+), 47 deletions(-) diff --git a/test/__snapshots__/auth.test.ts.snap b/test/__snapshots__/auth.test.ts.snap index 355eecb3c..7276ff137 100644 --- a/test/__snapshots__/auth.test.ts.snap +++ b/test/__snapshots__/auth.test.ts.snap @@ -1,54 +1,36 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`auth not logged in 1`] = ` -[ - { - "accessToken": "fake-token", - "context": undefined, - "extraParams": { - "accessTokenExpiresIn": 74879019472, - "refreshTokenExpiresIn": 59010219472, - "tokenType": "oauth", +{ + "accessToken": "fake-token", + "context": undefined, + "profile": { + "_json": { + "id": 690, + "login": "Mause", + "name": "Elli", }, - "profile": { - "_json": { - "id": 690, - "login": "Mause", - "name": "Elli", - }, - "displayName": "Mause", - "emails": [ - { - "value": undefined, - }, - ], - "id": "690", - "name": { - "familyName": "Elli", - "givenName": "Elli", - "middleName": "Elli", + "displayName": "Mause", + "emails": [ + { + "value": undefined, }, - "photos": [ - { - "value": undefined, - }, - ], - "provider": "github", + ], + "id": "690", + "name": { + "familyName": "Elli", + "givenName": "Elli", + "middleName": "Elli", }, - "refreshToken": "fake-refresh-token", - "request": { - "headers": Headers { - Symbol(headers list): HeadersList { - "cookies": null, - Symbol(headers map): Map {}, - Symbol(headers map sorted): null, - }, - Symbol(guard): "none", + "photos": [ + { + "value": undefined, }, - "url": "http://localhost:3000/callback?state=state&code=code", - }, + ], + "provider": "github", }, -] + "refreshToken": "fake-refresh-token", +} `; exports[`auth should be able to call strategy.authenticate 1`] = ` diff --git a/test/auth.test.ts b/test/auth.test.ts index fb2ceace6..80aee97b2 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -2,6 +2,7 @@ import type { Session, SessionStorage } from "@remix-run/node"; import { GitHubAppAuthStrategy } from "~/services/github-app-auth.server"; import type { RequestInterface } from "@octokit/types"; import type { AuthenticateOptions } from "remix-auth"; +import _ from "lodash"; const sessionKey = "oauth2:session"; const sessionStateKey = "oauth2:state"; @@ -104,8 +105,8 @@ async function request(url: string) { access_token: "fake-token", refresh_token: "fake-refresh-token", scope: "api", - expires_in: 28800, - refresh_token_expires_in: 15897600, + expires_in: 3600, + refresh_token_expires_in: 3600, }, }; } else if (url.includes("user")) { @@ -142,8 +143,8 @@ const mk = () => { callbackURL: "http://localhost:3000/callback", scope: "scope", }, - async (...args) => { - return args; + async (params) => { + return _.omit(params, ["request", "extraParams"]); }, request as unknown as RequestInterface, ); From 13a49fe7e6c6f8e84c23d1f229f6f89bf047e3ad Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 11 Dec 2023 00:19:09 +0800 Subject: [PATCH 04/16] not matching state test --- test/auth.test.ts | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/auth.test.ts b/test/auth.test.ts index 80aee97b2..63787232d 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -49,6 +49,22 @@ describe("auth", () => { ); expect(result).toMatchSnapshot(); }); + + it("non matching state", async () => { + expect.assertions(1); + const strategy = mk(); + const prom = strategy.authenticate( + makeRequest("?state=state&code=code"), + makeSessionStorage({ + [sessionKey]: undefined, + [sessionStateKey]: "state2", + }), + options, + ); + (await expectFailure(prom)).toMatchObject({ + message: "State doesn't match.", + }); + }); }); class DummySession implements Session { @@ -94,6 +110,24 @@ const makeSessionStorage = (bucket: Record) => "getSession" > as unknown as SessionStorage<{}>; +async function expectFailure( + prom: Promise, +): Promise> { + try { + await prom; + } catch (e) { + return expect(await getBody(e)); + } + throw new Error("expected failure"); +} + +async function getBody(e: unknown): Promise { + if (!(e instanceof Response)) { + throw new Error("not a response: " + JSON.stringify(e)); + } + return await e.json(); +} + async function request(url: string) { if (url.includes("oauth")) { return { From 5506578f3dd8b308a1051e3117aa8a70266a52da Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 11 Dec 2023 00:50:43 +0800 Subject: [PATCH 05/16] more tests --- test/auth.test.ts | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/auth.test.ts b/test/auth.test.ts index 63787232d..09bddd952 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -65,6 +65,35 @@ describe("auth", () => { message: "State doesn't match.", }); }); + it("missing code", async () => { + expect.assertions(1); + const strategy = mk(); + const prom = strategy.authenticate( + makeRequest("?state=state"), + makeSessionStorage({ + [sessionKey]: undefined, + [sessionStateKey]: "state", + }), + options, + ); + (await expectFailure(prom)).toMatchObject({ + message: "Missing code.", + }); + }); + it("missing state on session", async () => { + expect.assertions(1); + const strategy = mk(); + const prom = strategy.authenticate( + makeRequest("?state=state&code=code"), + makeSessionStorage({ + [sessionKey]: undefined, + }), + options, + ); + (await expectFailure(prom)).toMatchObject({ + message: "Missing state on session.", + }); + }); }); class DummySession implements Session { From 85018d3793b99d432d3874e7ea425e8d0af3b55a Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 11 Dec 2023 00:54:54 +0800 Subject: [PATCH 06/16] simplify expect failure --- test/auth.test.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/auth.test.ts b/test/auth.test.ts index 09bddd952..18747bcf3 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -61,7 +61,7 @@ describe("auth", () => { }), options, ); - (await expectFailure(prom)).toMatchObject({ + await expectFailure(prom).toMatchObject({ message: "State doesn't match.", }); }); @@ -76,7 +76,7 @@ describe("auth", () => { }), options, ); - (await expectFailure(prom)).toMatchObject({ + await expectFailure(prom).toMatchObject({ message: "Missing code.", }); }); @@ -90,7 +90,7 @@ describe("auth", () => { }), options, ); - (await expectFailure(prom)).toMatchObject({ + await expectFailure(prom).toMatchObject({ message: "Missing state on session.", }); }); @@ -139,15 +139,19 @@ const makeSessionStorage = (bucket: Record) => "getSession" > as unknown as SessionStorage<{}>; -async function expectFailure( +function expectFailure( prom: Promise, -): Promise> { - try { - await prom; - } catch (e) { - return expect(await getBody(e)); - } - throw new Error("expected failure"); +): jest.AndNot, Promise>> { + return expect( + (async () => { + try { + await prom; + } catch (e) { + return getBody(e); + } + throw new Error("expected failure"); + })(), + ).resolves; } async function getBody(e: unknown): Promise { From 159fcd82d815f6e2d5cd180e9d2454d0faaa7a14 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 11 Dec 2023 00:56:32 +0800 Subject: [PATCH 07/16] isResponse --- test/auth.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/auth.test.ts b/test/auth.test.ts index 18747bcf3..358cdf592 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -154,8 +154,12 @@ function expectFailure( ).resolves; } +function isResponse(e: unknown): e is Response { + return e instanceof Response; +} + async function getBody(e: unknown): Promise { - if (!(e instanceof Response)) { + if (!isResponse(e)) { throw new Error("not a response: " + JSON.stringify(e)); } return await e.json(); From 149bbfddccaa4983970c460824a008c826982f79 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 11 Dec 2023 00:57:35 +0800 Subject: [PATCH 08/16] extract DummySession --- test/auth.test.ts | 31 ++----------------------------- test/dummySession.ts | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 29 deletions(-) create mode 100644 test/dummySession.ts diff --git a/test/auth.test.ts b/test/auth.test.ts index 358cdf592..fcabba2fa 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -1,8 +1,9 @@ -import type { Session, SessionStorage } from "@remix-run/node"; +import type { SessionStorage } from "@remix-run/node"; import { GitHubAppAuthStrategy } from "~/services/github-app-auth.server"; import type { RequestInterface } from "@octokit/types"; import type { AuthenticateOptions } from "remix-auth"; import _ from "lodash"; +import { DummySession } from "./dummySession"; const sessionKey = "oauth2:session"; const sessionStateKey = "oauth2:state"; @@ -96,34 +97,6 @@ describe("auth", () => { }); }); -class DummySession implements Session { - id = "0"; - - constructor(public data: Record) { - this.data = data; - } - - get(key: string) { - return this.data[key]; - } - - unset(key: string) { - delete this.data[key]; - } - - set(name: string, value: any) { - this.data[name] = value; - } - - has(name: string) { - return name in this.data; - } - - flash(name: string, value: any) { - this.data[name] = value; - } -} - const makeRequest = (extra?: string): Request => ({ url: "http://localhost:3000/callback" + (extra ?? ""), diff --git a/test/dummySession.ts b/test/dummySession.ts new file mode 100644 index 000000000..14d800821 --- /dev/null +++ b/test/dummySession.ts @@ -0,0 +1,29 @@ +import type { Session } from "@remix-run/node"; + +export class DummySession implements Session { + id = "0"; + + constructor(public data: Record) { + this.data = data; + } + + get(key: string) { + return this.data[key]; + } + + unset(key: string) { + delete this.data[key]; + } + + set(name: string, value: any) { + this.data[name] = value; + } + + has(name: string) { + return name in this.data; + } + + flash(name: string, value: any) { + this.data[name] = value; + } +} From c7d12d0e000ba0530485d388f62870725db86184 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 11 Dec 2023 01:21:47 +0800 Subject: [PATCH 09/16] add redirect test --- test/__snapshots__/auth.test.ts.snap | 16 +++++++++++++ test/auth.test.ts | 36 ++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/test/__snapshots__/auth.test.ts.snap b/test/__snapshots__/auth.test.ts.snap index 7276ff137..0051eb804 100644 --- a/test/__snapshots__/auth.test.ts.snap +++ b/test/__snapshots__/auth.test.ts.snap @@ -1,5 +1,21 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`auth make login request 1`] = ` +URLSearchParams { + Symbol(query): [ + "response_type", + "code", + "client_id", + "clientId", + "redirect_uri", + "http://localhost:3000/callback", + ], + Symbol(context): "https://github.com/login/oauth/authorize?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback", +} +`; + +exports[`auth make login request 2`] = `"https://github.com/login/oauth/authorize?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback"`; + exports[`auth not logged in 1`] = ` { "accessToken": "fake-token", diff --git a/test/auth.test.ts b/test/auth.test.ts index fcabba2fa..d13a7e34e 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -1,4 +1,4 @@ -import type { SessionStorage } from "@remix-run/node"; +import type { Session, SessionStorage } from "@remix-run/node"; import { GitHubAppAuthStrategy } from "~/services/github-app-auth.server"; import type { RequestInterface } from "@octokit/types"; import type { AuthenticateOptions } from "remix-auth"; @@ -95,8 +95,37 @@ describe("auth", () => { message: "Missing state on session.", }); }); + it("make login request", async () => { + expect.assertions(5); + const strategy = mk(); + + try { + await strategy.authenticate( + makeNonCallback(), + makeSessionStorage({}), + options, + ); + } catch (e) { + expect(isResponse(e)).toBeTruthy(); + if (isResponse(e)) { + let location = new URL(e.headers.get("location")!); + expect(location.host).toEqual("github.com"); + let sp = location.searchParams; + expect(sp.has("state")).toBeTruthy(); + sp.delete("state"); + expect(sp).toMatchSnapshot(); + expect(location).toMatchSnapshot(); + } + } + }); }); +const makeNonCallback = (): Request => + ({ + url: "http://localhost/login", + headers: new Headers(), + }) as unknown as Request; + const makeRequest = (extra?: string): Request => ({ url: "http://localhost:3000/callback" + (extra ?? ""), @@ -107,9 +136,12 @@ const makeSessionStorage = (bucket: Record) => async getSession(_cookieHeader, _options) { return new DummySession(bucket); }, + async commitSession(session: Session, options): Promise { + return "serialized cookie session"; + }, }) satisfies Pick< SessionStorage<{}>, - "getSession" + "getSession" | "commitSession" > as unknown as SessionStorage<{}>; function expectFailure( From bf5d8ec07185fb4fac69d1077d82f26d40ad4cb1 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Fri, 19 Jan 2024 13:28:44 +0800 Subject: [PATCH 10/16] restore request override injection --- app/services/github-app-auth.server.ts | 33 +++++++++++++++++++++----- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/app/services/github-app-auth.server.ts b/app/services/github-app-auth.server.ts index 8bdfc9372..529090b7f 100644 --- a/app/services/github-app-auth.server.ts +++ b/app/services/github-app-auth.server.ts @@ -1,11 +1,18 @@ -import type {GitHubExtraParams} from "remix-auth-github"; -import {GitHubStrategy} from "remix-auth-github"; -import {createAppAuth, createOAuthUserAuth} from "@octokit/auth-app"; +import type { + GitHubExtraParams, + GitHubProfile, + GitHubStrategyOptions} from "remix-auth-github"; +import { + GitHubStrategy +} from "remix-auth-github"; +import { createAppAuth, createOAuthUserAuth } from "@octokit/auth-app"; // import createDebug from "debug"; -import {octokitFromConfig} from "~/octokit.server"; +import { octokitFromConfig } from "~/octokit.server"; import _ from "lodash"; import type { RequestInterface } from "@octokit/types"; import getCache from "~/services/cache"; +import type { StrategyVerifyCallback } from "remix-auth"; +import type { OAuth2StrategyVerifyParams } from "remix-auth-oauth2"; function checkNonNull(name: string): NonNullable { const value = process.env[name]; @@ -15,7 +22,9 @@ function checkNonNull(name: string): NonNullable { return value; } -export const appAuth = _.memoize((requestOverride?: RequestInterface) => createAppAuth(getAuthConfig())); +export const appAuth = _.memoize((requestOverride?: RequestInterface) => + createAppAuth(getAuthConfig(requestOverride)), +); export const getConfig = _.memoize(() => { return { @@ -47,7 +56,7 @@ function getAuthConfig(requestOverride?: RequestInterface) { cache: getCache(), request: requestOverride, }; -}; +} export async function getAppOctokit() { return octokitFromConfig({ @@ -59,6 +68,18 @@ export async function getAppOctokit() { export class GitHubAppAuthStrategy extends GitHubStrategy { requestOverride?: RequestInterface; + constructor( + options: GitHubStrategyOptions, + verify: StrategyVerifyCallback< + User, + OAuth2StrategyVerifyParams + >, + requestOverride?: RequestInterface, + ) { + super(options, verify); + this.requestOverride = requestOverride; + } + async fetchAccessToken( code: string, params: URLSearchParams, From c8b5c36caeae2488ee3bfa1fd381e6cb82495708 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Fri, 19 Jan 2024 13:31:41 +0800 Subject: [PATCH 11/16] update snapshot names --- test/__snapshots__/auth.test.ts.snap | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/__snapshots__/auth.test.ts.snap b/test/__snapshots__/auth.test.ts.snap index 0051eb804..ae4fedf61 100644 --- a/test/__snapshots__/auth.test.ts.snap +++ b/test/__snapshots__/auth.test.ts.snap @@ -1,6 +1,6 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`auth make login request 1`] = ` +exports[`auth > make login request 1`] = ` URLSearchParams { Symbol(query): [ "response_type", @@ -14,7 +14,13 @@ URLSearchParams { } `; -exports[`auth make login request 2`] = `"https://github.com/login/oauth/authorize?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback"`; +exports[`auth > make login request 2`] = `"https://github.com/login/oauth/authorize?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback"`; + +exports[`auth > should be able to call strategy.authenticate 1`] = ` +{ + "login": "login", +} +`; exports[`auth not logged in 1`] = ` { @@ -48,9 +54,3 @@ exports[`auth not logged in 1`] = ` "refreshToken": "fake-refresh-token", } `; - -exports[`auth should be able to call strategy.authenticate 1`] = ` -{ - "login": "login", -} -`; From be01b2915fc8854743d177ff51b8cca0176e4be3 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Fri, 19 Jan 2024 13:35:03 +0800 Subject: [PATCH 12/16] tweak snapshotting --- test/__snapshots__/auth.test.ts.snap | 19 +++++++++++++++---- test/auth.test.ts | 2 +- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/test/__snapshots__/auth.test.ts.snap b/test/__snapshots__/auth.test.ts.snap index ae4fedf61..adb9e55af 100644 --- a/test/__snapshots__/auth.test.ts.snap +++ b/test/__snapshots__/auth.test.ts.snap @@ -1,17 +1,28 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html exports[`auth > make login request 1`] = ` -URLSearchParams { - Symbol(query): [ +[ + [ + "scope", + "scope", + ], + [ + "allow_signup", + "true", + ], + [ "response_type", "code", + ], + [ "client_id", "clientId", + ], + [ "redirect_uri", "http://localhost:3000/callback", ], - Symbol(context): "https://github.com/login/oauth/authorize?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback", -} +] `; exports[`auth > make login request 2`] = `"https://github.com/login/oauth/authorize?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback"`; diff --git a/test/auth.test.ts b/test/auth.test.ts index d13a7e34e..6b0317893 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -113,7 +113,7 @@ describe("auth", () => { let sp = location.searchParams; expect(sp.has("state")).toBeTruthy(); sp.delete("state"); - expect(sp).toMatchSnapshot(); + expect(Array.from(sp.entries())).toMatchSnapshot(); expect(location).toMatchSnapshot(); } } From b041c95330c28b6c333fa97672585bf35e6bf55d Mon Sep 17 00:00:00 2001 From: Elliana May Date: Fri, 19 Jan 2024 13:36:05 +0800 Subject: [PATCH 13/16] update other snapshots --- test/__snapshots__/auth.test.ts.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/__snapshots__/auth.test.ts.snap b/test/__snapshots__/auth.test.ts.snap index adb9e55af..0231a0280 100644 --- a/test/__snapshots__/auth.test.ts.snap +++ b/test/__snapshots__/auth.test.ts.snap @@ -25,7 +25,7 @@ exports[`auth > make login request 1`] = ` ] `; -exports[`auth > make login request 2`] = `"https://github.com/login/oauth/authorize?response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback"`; +exports[`auth > make login request 2`] = `"https://github.com/login/oauth/authorize?scope=scope&allow_signup=true&response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback"`; exports[`auth > should be able to call strategy.authenticate 1`] = ` { From c349c1b9e9c2562bd50c3847eeed2c2abba63d91 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Fri, 19 Jan 2024 14:21:02 +0800 Subject: [PATCH 14/16] more realistic responses --- test/auth.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/auth.test.ts b/test/auth.test.ts index 6b0317893..799358d4c 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -173,10 +173,11 @@ async function getBody(e: unknown): Promise { async function request(url: string) { if (url.includes("oauth")) { return { + url, status: 200, - headers: { + headers: new Headers({ date: "Mon, 26 Jul 2021 15:49:05 GMT", - }, + }), data: { access_token: "fake-token", refresh_token: "fake-refresh-token", @@ -184,15 +185,23 @@ async function request(url: string) { expires_in: 3600, refresh_token_expires_in: 3600, }, + text: function () { + return JSON.stringify(this.data); + }, }; } else if (url.includes("user")) { return { + url, status: 200, + headers: new Headers({}), data: { id: 690, login: "Mause", name: "Elli", }, + text: function () { + return JSON.stringify(this.data); + }, }; } throw new Error("unknown request: " + JSON.stringify({ url })); From 0c1aba55d32bdf313d6d25d5a6722420cc737804 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Fri, 19 Jan 2024 15:09:44 +0800 Subject: [PATCH 15/16] isomorphic fetch? --- package.json | 1 + vitest.config.js | 5 +++++ yarn.lock | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/package.json b/package.json index f5290a342..c39d9dc2b 100644 --- a/package.json +++ b/package.json @@ -79,6 +79,7 @@ "eslint-plugin-promise": "^6.0.0", "eslint-plugin-react": "^7.32.2", "husky": ">=6", + "isomorphic-fetch": "^3.0.0", "jsdom": "^23.2.0", "jsdom-testing-mocks": "^1.11.0", "lint-staged": ">=10", diff --git a/vitest.config.js b/vitest.config.js index cfb6919e5..49c511e02 100644 --- a/vitest.config.js +++ b/vitest.config.js @@ -7,6 +7,11 @@ import tsconfigPaths from "vite-tsconfig-paths"; export default defineConfig({ plugins: [react(), tsconfigPaths()], + resolve: { + alias: { + 'node-fetch': 'isomorphic-fetch', + }, + }, test: { testTimeout: 50000, globals: true, diff --git a/yarn.lock b/yarn.lock index 991a2f26f..4be6aa271 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9519,6 +9519,7 @@ __metadata: graphql-tag: "npm:^2.12.6" humanize-duration: "npm:^3.28.0" husky: "npm:>=6" + isomorphic-fetch: "npm:^3.0.0" jsdom: "npm:^23.2.0" jsdom-testing-mocks: "npm:^1.11.0" lint-staged: "npm:>=10" @@ -10723,6 +10724,16 @@ __metadata: languageName: node linkType: hard +"isomorphic-fetch@npm:^3.0.0": + version: 3.0.0 + resolution: "isomorphic-fetch@npm:3.0.0" + dependencies: + node-fetch: "npm:^2.6.1" + whatwg-fetch: "npm:^3.4.1" + checksum: 568fe0307528c63405c44dd3873b7b6c96c0d19ff795cb15846e728b6823bdbc68cc8c97ac23324509661316f12f551e43dac2929bc7030b8bc4d6aa1158b857 + languageName: node + linkType: hard + "isomorphic-ws@npm:5.0.0, isomorphic-ws@npm:^5.0.0": version: 5.0.0 resolution: "isomorphic-ws@npm:5.0.0" @@ -16508,6 +16519,13 @@ __metadata: languageName: node linkType: hard +"whatwg-fetch@npm:^3.4.1": + version: 3.6.20 + resolution: "whatwg-fetch@npm:3.6.20" + checksum: 2b4ed92acd6a7ad4f626a6cb18b14ec982bbcaf1093e6fe903b131a9c6decd14d7f9c9ca3532663c2759d1bdf01d004c77a0adfb2716a5105465c20755a8c57c + languageName: node + linkType: hard + "whatwg-mimetype@npm:^4.0.0": version: 4.0.0 resolution: "whatwg-mimetype@npm:4.0.0" From 37046fcf32ada530930e215674c85c1f4bb5bb7d Mon Sep 17 00:00:00 2001 From: Elliana May Date: Fri, 19 Jan 2024 15:13:53 +0800 Subject: [PATCH 16/16] test mock approaches --- test/__snapshots__/mock.test.ts.snap | 58 ++++++++++++++++++++++++++++ test/auth.test.ts | 21 +++++----- test/mock.test.ts | 25 ++++++++++++ 3 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 test/__snapshots__/mock.test.ts.snap create mode 100644 test/mock.test.ts diff --git a/test/__snapshots__/mock.test.ts.snap b/test/__snapshots__/mock.test.ts.snap new file mode 100644 index 000000000..a6211ac3e --- /dev/null +++ b/test/__snapshots__/mock.test.ts.snap @@ -0,0 +1,58 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`auth > make login request 1`] = ` +[ + [ + "scope", + "scope", + ], + [ + "allow_signup", + "true", + ], + [ + "response_type", + "code", + ], + [ + "client_id", + "clientId", + ], + [ + "redirect_uri", + "http://localhost:3000/callback", + ], +] +`; + +exports[`auth > make login request 2`] = `"https://github.com/login/oauth/authorize?scope=scope&allow_signup=true&response_type=code&client_id=clientId&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback"`; + +exports[`auth > should be able to call strategy.authenticate 1`] = ` +{ + "login": "login", +} +`; + +exports[`request 1`] = ` +{ + "data": { + "id": 690, + "login": "Mause", + "name": "Elli", + }, + "headers": { + "date": "Fri, 19 Jan 2024 07:12:21 GMT", + }, + "status": 200, + "url": "https://api.github.com/user", +} +`; + +exports[`request with octokit 1`] = ` +{ + "data": "{"id":690,"login":"Mause","name":"Elli"}", + "headers": {}, + "status": 200, + "url": "https://api.github.com/user", +} +`; diff --git a/test/auth.test.ts b/test/auth.test.ts index 799358d4c..1d44db067 100644 --- a/test/auth.test.ts +++ b/test/auth.test.ts @@ -169,15 +169,17 @@ async function getBody(e: unknown): Promise { } return await e.json(); } +const now = new Date().toUTCString(); -async function request(url: string) { +export async function request(url: string) { + console.warn({ url }); if (url.includes("oauth")) { return { url, status: 200, - headers: new Headers({ - date: "Mon, 26 Jul 2021 15:49:05 GMT", - }), + headers: { + date: now, + }, data: { access_token: "fake-token", refresh_token: "fake-refresh-token", @@ -193,18 +195,19 @@ async function request(url: string) { return { url, status: 200, - headers: new Headers({}), + headers: { + date: now, + }, data: { id: 690, login: "Mause", name: "Elli", }, - text: function () { - return JSON.stringify(this.data); - }, }; } - throw new Error("unknown request: " + JSON.stringify({ url })); + const msg = "unknown request: " + JSON.stringify({ url }); + console.warn(msg); + throw new Error(msg); } function merge() {} diff --git a/test/mock.test.ts b/test/mock.test.ts new file mode 100644 index 000000000..36d8a5159 --- /dev/null +++ b/test/mock.test.ts @@ -0,0 +1,25 @@ +import { Octokit } from "@octokit/rest"; +import { request } from "./auth.test"; +import { vi } from "vitest"; + +it("request", async () => { + const resp = await request("https://api.github.com/user"); + expect(resp).toMatchSnapshot(); +}); +it("request with octokit", async () => { + globalThis.fetch = vi.fn().mockImplementation(async (url) => { + const res = await request(url); + + return { + ...res, + headers: new Headers(res.headers), + text: function () { + return JSON.stringify(this.data); + }, + }; + }); + const octokit = new Octokit(); + const resp = await octokit.request("https://api.github.com/user"); + delete resp.headers.date; + expect(resp).toMatchSnapshot(); +});