diff --git a/bun.lockb b/bun.lockb index 768690a2..3fd0045b 100755 Binary files a/bun.lockb and b/bun.lockb differ diff --git a/packages/.eslintrc.js b/packages/.eslintrc.js index 14544708..b178b529 100644 --- a/packages/.eslintrc.js +++ b/packages/.eslintrc.js @@ -11,7 +11,7 @@ module.exports = { "no-unused-vars": "off", "@typescript-eslint/no-unused-vars": [ "error", - { varsIgnorePattern: "_^", argsIgnorePattern: "_^" }, + { varsIgnorePattern: "^_", argsIgnorePattern: "^_" }, ], "@typescript-eslint/ban-ts-comment": "off", "@typescript-eslint/explicit-function-return-type": "off", diff --git a/packages/api/src/orbitAPI.ts b/packages/api/src/orbitAPI.ts index f3c92204..bea25701 100644 --- a/packages/api/src/orbitAPI.ts +++ b/packages/api/src/orbitAPI.ts @@ -30,7 +30,7 @@ export type ValidatableSpec = { */ entityID?: EntityID; }; - response?: ResponseList2; + response?: ResponseList; }; PATCH?: { @@ -54,7 +54,6 @@ export type ValidatableSpec = { /** * encode with multipart/form-data, with the file in part named "file" * make sure to include Content-Type heading for your attachment - * returns application/json encoded ResponseObject<"attachmentIDReference", AttachmentID, AttachmentIDReference> */ POST?: { // NOTE: Content-type must use regex to be validated since additional data, @@ -92,38 +91,15 @@ export type ValidatableSpec = { POST?: { contentType: "application/json"; body: TaskID[]; - response?: ResponseList2; + response?: ResponseList; }; }; }; export type Spec = RequiredSpec; -export type ResponseList< - ObjectTypeString extends string, - IDType extends string, - DataType, -> = { - objectType: "list"; - hasMore: boolean; - data: ResponseObject[]; -}; - -export type ResponseList2 = { +export type ResponseList = { type: "list"; hasMore: boolean; items: ItemType[]; }; - -export type ResponseObject< - ObjectType extends string, - IDType extends string, - DataType, -> = { - objectType: ObjectType; - /** - * @TJS-type string - */ - id: IDType; - data: DataType; -}; diff --git a/packages/backend/src/__tests__/firebaseTesting.ts b/packages/backend/src/__tests__/firebaseTesting.ts index f740ecff..579893f9 100644 --- a/packages/backend/src/__tests__/firebaseTesting.ts +++ b/packages/backend/src/__tests__/firebaseTesting.ts @@ -1,6 +1,7 @@ import OrbitAPIClient, { emulatorAPIConfig } from "@withorbit/api-client"; import { App, deleteApp, initializeApp } from "firebase-admin/app"; -import { getFirestore } from "firebase-admin/firestore"; +import { getFirestore, Timestamp } from "firebase-admin/firestore"; +import { getUserMetadata } from "../db/firebaseAccountData.js"; import { UserMetadata } from "../db/userMetadata.js"; const projectID = "metabook-system"; @@ -47,25 +48,60 @@ export async function clearFirestoreData() { } export async function setupAuthToken( - name: string, + authToken: string, + userID = "testUserID", userMetadata: Partial = {}, ) { const app = getTestFirebaseAdminApp(); const firestore = getFirestore(app); await firestore .collection("users") - .doc("WvLvv9uDtFha1jVTyxObVl00gPFN") + .doc(userID) .set({ registrationTimestampMillis: 1615510817519, ...userMetadata, }); - await firestore.collection("accessCodes").doc(name).set({ + await firestore.collection("accessCodes").doc(authToken).set({ type: "personalAccessToken", - userID: "WvLvv9uDtFha1jVTyxObVl00gPFN", + userID, }); } +export async function setupAccessCode( + accessCode: string, + userID = "testUserID", + userMetadata: Partial = {}, + expirationDate: Date = new Date(Date.now() + 1000 * 60), +) { + const app = getTestFirebaseAdminApp(); + const firestore = getFirestore(app); + await firestore + .collection("users") + .doc(userID) + .set({ + registrationTimestampMillis: 1615510817519, + ...userMetadata, + }); + + await firestore + .collection("accessCodes") + .doc(accessCode) + .set({ + type: "oneTime", + userID, + expirationTimestamp: Timestamp.fromDate(expirationDate), + }); +} + +export async function getTestUserMetadata( + userID: string, +): Promise { + const app = getTestFirebaseAdminApp(); + const firestore = getFirestore(app); + return await getUserMetadata(userID, firestore); +} + export async function setupTestOrbitAPIClient(): Promise { await setupAuthToken("auth"); return new OrbitAPIClient( diff --git a/packages/backend/src/__tests__/integration/auth.test.ts b/packages/backend/src/__tests__/integration/auth.test.ts new file mode 100644 index 00000000..c80fc775 --- /dev/null +++ b/packages/backend/src/__tests__/integration/auth.test.ts @@ -0,0 +1,103 @@ +import { resetLocalEmulators } from "../emulators.js"; +import { setupAccessCode } from "../firebaseTesting.js"; +import { fetchRoute } from "./utils/fetchRoute.js"; + +beforeEach(async () => { + await resetLocalEmulators(); + await setupAccessCode("test"); +}); + +async function consumeAccessCode(accessCode: string) { + return await fetchRoute(`/api/internal/auth/consumeAccessCode`, { + method: "GET", + authorization: { token: accessCode }, + }); +} + +describe("/internal/auth/consumeAccessCode", () => { + test("returns a login token for valid access code", async () => { + const { status, body } = await consumeAccessCode("test"); + expect(status).toBe(200); + expect(body).toMatch(/.+/); // i.e. to be a non-empty string + }); + + test("fails when no authorization is provided", async () => { + const { status, body } = await fetchRoute( + `/api/internal/auth/consumeAccessCode`, + { + method: "GET", + }, + ); + expect(status).toBe(401); + expect(body).toBeUndefined(); + }); + + test("fails when consuming an already-consumed code", async () => { + await consumeAccessCode("test"); + const { status, body } = await consumeAccessCode("test"); + expect(status).toBe(401); + expect(body).toBeUndefined(); + }); + + test("fails when consuming a non-existent code", async () => { + const { status, body } = await consumeAccessCode("test2"); + expect(status).toBe(401); + expect(body).toBeUndefined(); + }); + + test("fails when consuming an expired code", async () => { + await setupAccessCode( + "testExpired", + "testUserID", + {}, + new Date(Date.now() - 1000), + ); + const { status, body } = await consumeAccessCode("testExpired"); + expect(status).toBe(401); + expect(body).toBeUndefined(); + }); +}); + +describe("/internal/auth/personalAccessTokens", () => { + test("creates PAT from access code", async () => { + const { status, body } = await fetchRoute( + `/api/internal/auth/personalAccessTokens`, + { + method: "POST", + json: {}, + authorization: { token: "test" }, + }, + ); + expect(status).toBe(200); + expect(body.token).toMatch(/.+/); // i.e. to be a non-empty string + + // Personal access tokens can be used repeatedly. + expect((await consumeAccessCode(body.token)).status).toBe(200); + expect((await consumeAccessCode(body.token)).status).toBe(200); + }); + + test("fails without auth", async () => { + const { status, body } = await fetchRoute( + `/api/internal/auth/personalAccessTokens`, + { + method: "POST", + json: {}, + }, + ); + expect(status).toBe(401); + expect(body).toBeUndefined(); + }); + + test("fails with invalid auth", async () => { + const { status, body } = await fetchRoute( + `/api/internal/auth/personalAccessTokens`, + { + method: "POST", + json: {}, + authorization: { token: "invalid" }, + }, + ); + expect(status).toBe(401); + expect(body).toBeUndefined(); + }); +}); diff --git a/packages/backend/src/__tests__/integration/updateNotificationSettings.test.ts b/packages/backend/src/__tests__/integration/updateNotificationSettings.test.ts new file mode 100644 index 00000000..d688df2c --- /dev/null +++ b/packages/backend/src/__tests__/integration/updateNotificationSettings.test.ts @@ -0,0 +1,72 @@ +import { UpdateNotificationSettingsAction } from "../../firebaseFunctions/updateNotificationSettings.js"; +import { resetLocalEmulators } from "../emulators.js"; +import { getTestUserMetadata, setupAuthToken } from "../firebaseTesting.js"; +import { fetchRoute } from "./utils/fetchRoute.js"; + +beforeEach(async () => { + await resetLocalEmulators(); +}); + +async function attemptUpdate( + action: UpdateNotificationSettingsAction, + token: string | null, +) { + return await fetchRoute(`/updateNotificationSettings?action=${action}`, { + method: "GET", + authorization: token ? { token } : undefined, + followRedirects: false, + }); +} + +describe("/updateNotificationSettings", () => { + test("fails without auth", async () => { + const { status, body } = await attemptUpdate("unsubscribe", null); + expect(status).toBe(401); + expect(body).toBeUndefined(); + }); + + test("fails with invalid auth", async () => { + const { status, body } = await attemptUpdate("unsubscribe", "invalid"); + expect(status).toBe(401); + expect(body).toBeUndefined(); + }); + + test("unsubscribes", async () => { + await setupAuthToken("test", "testUserID"); + const { status, body } = await attemptUpdate("unsubscribe", "test"); + expect(status).toBe(302); + expect(body).toBeUndefined(); + + const metadata = await getTestUserMetadata("testUserID"); + expect(metadata!.isUnsubscribedFromSessionNotifications).toBe(true); + }); + + test("snoozes", async () => { + await setupAuthToken("test", "testUserID"); + const { status, body } = await attemptUpdate("snooze1Week", "test"); + expect(status).toBe(302); + expect(body).toBeUndefined(); + + const metadata = await getTestUserMetadata("testUserID"); + expect(metadata!.isUnsubscribedFromSessionNotifications).toBe(false); + expect( + metadata!.snoozeSessionNotificationsUntilTimestampMillis, + ).toBeGreaterThan(Date.now()); + }); + + test("unsubscribing then snoozing resubscribes", async () => { + await setupAuthToken("test", "testUserID"); + await attemptUpdate("unsubscribe", "test"); + await attemptUpdate("snooze1Week", "test"); + const metadata = await getTestUserMetadata("testUserID"); + expect(metadata!.isUnsubscribedFromSessionNotifications).toBe(false); + }); + + test("fails for unknown action", async () => { + await setupAuthToken("test"); + // @ts-ignore + const { status, body } = await attemptUpdate("invalid", "test"); + expect(status).toBe(400); + expect(body).toBeUndefined(); + }); +}); diff --git a/packages/backend/src/__tests__/integration/utils/fetchRoute.ts b/packages/backend/src/__tests__/integration/utils/fetchRoute.ts index 7e8f921f..1239d163 100644 --- a/packages/backend/src/__tests__/integration/utils/fetchRoute.ts +++ b/packages/backend/src/__tests__/integration/utils/fetchRoute.ts @@ -24,9 +24,18 @@ export async function fetchRoute( redirect: args.followRedirects === false ? "manual" : "follow", }); // helper to avoid having this boilerplate unwrapping within the tests - let body: any = undefined; - try { - body = await result.json(); - } catch {} + let body: any; + const contentType = result.headers.get("Content-Type"); + if (contentType === null) { + body = undefined; + } else if (contentType?.startsWith("application/json")) { + try { + body = await result.json(); + } catch {} + } else if (contentType?.startsWith("text/plain")) { + body = await result.text(); + } else { + throw new Error(`Unsupported response content type ${contentType}`); + } return { status: result.status, body, headers: result.headers }; } diff --git a/packages/backend/src/api.ts b/packages/backend/src/api.ts index dc0c6f66..802604a5 100644 --- a/packages/backend/src/api.ts +++ b/packages/backend/src/api.ts @@ -2,18 +2,19 @@ import { OrbitAPI, OrbitAPIValidator } from "@withorbit/api"; import cookieParser from "cookie-parser"; import express from "express"; import morganBody from "morgan-body"; -import { listEvents, storeEvents } from "./api/events.js"; -import { bulkGetTasks } from "./api/tasks.js"; import { getAttachment, ingestAttachmentsFromURLs, storeAttachment, } from "./api/attachments.js"; +import { listEvents, storeEvents } from "./api/events.js"; import { consumeAccessCode } from "./api/internal/auth/consumeAccessCode.js"; import { createLoginToken } from "./api/internal/auth/createLoginToken.js"; import { personalAccessTokens } from "./api/internal/auth/personalAccessTokens.js"; import { refreshSessionCookie } from "./api/internal/auth/refreshSessionCookie.js"; import { recordPageView } from "./api/internal/recordPageView.js"; +import { InternalAPISpec } from "./api/internalSpec.js"; +import { bulkGetTasks } from "./api/tasks.js"; import corsHandler from "./api/util/corsHandler.js"; import createTypedRouter from "./api/util/typedRouter.js"; @@ -57,11 +58,21 @@ export function createAPIApp(): express.Application { }, }); - app.post("/internal/auth/personalAccessTokens", personalAccessTokens); + createTypedRouter( + app, + { + // TODO validate internal API calls + validateRequest: () => true, + validateResponse: () => true, + }, + { + "/internal/auth/consumeAccessCode": { GET: consumeAccessCode }, + "/internal/auth/personalAccessTokens": { POST: personalAccessTokens }, + }, + ); // These older auth APIs need some rethinking... app.get("/internal/auth/createLoginToken", createLoginToken); - app.get("/internal/auth/consumeAccessCode", consumeAccessCode); app.get("/internal/auth/refreshSessionCookie", refreshSessionCookie); app.post("/internal/recordPageView", recordPageView); diff --git a/packages/backend/src/api/internal/auth/consumeAccessCode.ts b/packages/backend/src/api/internal/auth/consumeAccessCode.ts index ae04f92d..13fed135 100644 --- a/packages/backend/src/api/internal/auth/consumeAccessCode.ts +++ b/packages/backend/src/api/internal/auth/consumeAccessCode.ts @@ -1,15 +1,19 @@ -import express from "express"; import { sharedServerDatabase } from "../../../db/index.js"; -import { authenticateRequest } from "../../util/authenticateRequest.js"; +import { InternalAPISpec } from "../../internalSpec.js"; +import { authenticatedRequestHandler } from "../../util/authenticateRequest.js"; +import { CachePolicy, TypedRouteHandler } from "../../util/typedRouter.js"; -export async function consumeAccessCode( - request: express.Request, - response: express.Response, -) { - await authenticateRequest(request, response, async (userID) => { - const loginToken = await sharedServerDatabase().auth.createCustomLoginToken( - userID, - ); - response.send(loginToken); - }); -} +export const consumeAccessCode: TypedRouteHandler< + InternalAPISpec, + "/internal/auth/consumeAccessCode", + "GET" +> = authenticatedRequestHandler(async (request, userID) => { + const loginToken = + await sharedServerDatabase().auth.createCustomLoginToken(userID); + + return { + status: 200, + text: loginToken, + cachePolicy: CachePolicy.NoStore, + }; +}); diff --git a/packages/backend/src/api/internal/auth/personalAccessTokens.ts b/packages/backend/src/api/internal/auth/personalAccessTokens.ts index 38d040f9..18746ebe 100644 --- a/packages/backend/src/api/internal/auth/personalAccessTokens.ts +++ b/packages/backend/src/api/internal/auth/personalAccessTokens.ts @@ -1,23 +1,20 @@ -import express from "express"; import { sharedServerDatabase } from "../../../db/index.js"; -import { authenticateRequest } from "../../util/authenticateRequest.js"; +import { InternalAPISpec } from "../../internalSpec.js"; +import { authenticatedRequestHandler } from "../../util/authenticateRequest.js"; +import { CachePolicy, TypedRouteHandler } from "../../util/typedRouter.js"; -interface CreatePersonalAccessTokenResponse { - token: string; -} - -export async function personalAccessTokens( - request: express.Request, - response: express.Response, -) { - await authenticateRequest(request, response, async (userID) => { - const token = await sharedServerDatabase().auth.createPersonalAccessToken( - userID, - ); - - const responseJSON: CreatePersonalAccessTokenResponse = { +export const personalAccessTokens: TypedRouteHandler< + InternalAPISpec, + "/internal/auth/personalAccessTokens", + "POST" +> = authenticatedRequestHandler(async (request, userID) => { + const token = + await sharedServerDatabase().auth.createPersonalAccessToken(userID); + return { + status: 200, + cachePolicy: CachePolicy.NoStore, + json: { token, - }; - response.json(responseJSON); - }); -} + }, + }; +}); diff --git a/packages/backend/src/api/internalSpec.ts b/packages/backend/src/api/internalSpec.ts new file mode 100644 index 00000000..05fe5e51 --- /dev/null +++ b/packages/backend/src/api/internalSpec.ts @@ -0,0 +1,12 @@ +export type InternalAPISpec = { + "/internal/auth/consumeAccessCode": { + GET: { + response: string; + }; + }; + "/internal/auth/personalAccessTokens": { + POST: { + response: { token: string }; + }; + }; +}; diff --git a/packages/backend/src/api/util/authenticateRequest.ts b/packages/backend/src/api/util/authenticateRequest.ts index 90727546..7477ffb0 100644 --- a/packages/backend/src/api/util/authenticateRequest.ts +++ b/packages/backend/src/api/util/authenticateRequest.ts @@ -1,25 +1,11 @@ import { API as ApiType } from "@withorbit/api"; -import express from "express"; import { sharedServerDatabase } from "../../db/index.js"; import { - CachePolicy, TypedRequest, TypedResponse, TypedRouteHandler, } from "./typedRouter.js"; -export async function authenticateRequest( - request: express.Request, - response: express.Response, - next: (userID: string) => void, -): Promise { - await authenticateTypedRequest(request, async (userID) => { - next(userID); - // HACK Not actually used: - return { status: 200, json: undefined, cachePolicy: CachePolicy.NoStore }; - }); -} - export async function authenticateTypedRequest< API extends ApiType.Spec, Path extends Extract, diff --git a/packages/backend/src/api/util/typedRouter.ts b/packages/backend/src/api/util/typedRouter.ts index 4370b342..dfc68c4f 100644 --- a/packages/backend/src/api/util/typedRouter.ts +++ b/packages/backend/src/api/util/typedRouter.ts @@ -36,13 +36,11 @@ export type TypedRouteHandler< ) => Promise>>; export type TypedResponse = ( - | { status: 200 | 201; json: Data; cachePolicy: CachePolicy } - | { - status: 200 | 201; - data: Uint8Array; - mimeType: string; - cachePolicy: CachePolicy; - } + | ({ status: 200 | 201; cachePolicy: CachePolicy } & ( + | { json: Data } + | { data: Uint8Array; mimeType: string } + | { text: string } + )) | { status: 204 } | { status: 301 | 302; redirectURL: string; cachePolicy: CachePolicy } | { status: 400 | 401 | 404 } @@ -103,30 +101,7 @@ export default function createTypedRouter( request as TypedRequest>, ) .then((result) => { - response.status(result.status); - - for (const [name, value] of Object.entries(result.headers ?? {})) { - response.setHeader(name, value); - } - - if (result.status === 301 || result.status === 302) { - response.header("Location", result.redirectURL); - } - - if ("cachePolicy" in result) { - response.header("Cache-Control", result.cachePolicy); - } - - if (result.status === 200) { - if ("json" in result) { - response.json(result.json); - } else { - response.contentType(result.mimeType); - response.send(result.data); - } - } else { - response.send(); - } + sendStructuredResponse(response, result); }) .catch((err) => { console.error("Error executing handler: ", err.name, err.message); @@ -232,3 +207,45 @@ function validateURLPathParams( } return { status: "success" }; } + +export function sendStructuredResponse< + API extends API.Spec, + Path extends Extract, + Method extends Extract, +>( + response: express.Response, + result: TypedResponse>, +) { + response.status(result.status); + + for (const [name, value] of Object.entries(result.headers ?? {})) { + response.setHeader(name, value); + } + + if (result.status === 301 || result.status === 302) { + response.header("Location", result.redirectURL); + } + + if ("cachePolicy" in result) { + response.header("Cache-Control", result.cachePolicy); + } + + if (result.status === 200) { + if ("json" in result) { + response.json(result.json); + } else if ("data" in result) { + response.contentType(result.mimeType); + response.send(result.data); + } else if ("text" in result) { + response.contentType("text/plain"); + response.send(result.text); + } else { + function unreachable(_: never): never { + throw new Error("unreachable"); + } + unreachable(result); + } + } else { + response.end(); + } +} diff --git a/packages/backend/src/db/firebaseAccountData.ts b/packages/backend/src/db/firebaseAccountData.ts index 7bbd8664..ef98988f 100644 --- a/packages/backend/src/db/firebaseAccountData.ts +++ b/packages/backend/src/db/firebaseAccountData.ts @@ -15,8 +15,9 @@ function getUserMetadataReference( export async function getUserMetadata( userID: string, + database: firebase.firestore.Firestore = getDatabase(), ): Promise { - const snapshot = await getUserMetadataReference(getDatabase(), userID).get(); + const snapshot = await getUserMetadataReference(database, userID).get(); return snapshot.data() ?? null; } diff --git a/packages/backend/src/firebaseFunctions/updateNotificationSettings.ts b/packages/backend/src/firebaseFunctions/updateNotificationSettings.ts index 54220cb8..727081e7 100644 --- a/packages/backend/src/firebaseFunctions/updateNotificationSettings.ts +++ b/packages/backend/src/firebaseFunctions/updateNotificationSettings.ts @@ -1,10 +1,14 @@ import * as dateFns from "date-fns"; import express from "express"; import functions from "firebase-functions"; +import { authenticateTypedRequest } from "../api/util/authenticateRequest.js"; +import { + CachePolicy, + sendStructuredResponse, +} from "../api/util/typedRouter.js"; import { sharedServerDatabase } from "../db/index.js"; -import serviceConfig from "../serviceConfig.js"; -import { authenticateRequest } from "../api/util/authenticateRequest.js"; import { UserMetadata } from "../db/userMetadata.js"; +import serviceConfig from "../serviceConfig.js"; export type UpdateNotificationSettingsAction = "unsubscribe" | "snooze1Week"; @@ -42,22 +46,35 @@ function getAction( return actionTable[action] ? action : null; } +// TODO: rewrite this as an internal API endpoint which the page calls onload const app = express(); app.use(async (request, response) => { - await authenticateRequest(request, response, async (userID) => { - const action = getAction(request); - if (action) { - const metadataUpdate = actionTable[action](Date.now()); - await sharedServerDatabase().accounts.updateUserMetadata( - userID, - metadataUpdate, - ); - } else { - console.log("Missing or unknown action", action); - } - - response.redirect(getRedirectURL(action)); - }); + try { + const result = await authenticateTypedRequest(request, async (userID) => { + const action = getAction(request); + if (action) { + const metadataUpdate = actionTable[action](Date.now()); + await sharedServerDatabase().accounts.updateUserMetadata( + userID, + metadataUpdate, + ); + } else { + console.log("Missing or unknown action", action); + return { + status: 400, + }; + } + return { + status: 302, + redirectURL: getRedirectURL(action), + cachePolicy: CachePolicy.NoStore, + }; + }); + sendStructuredResponse(response, result); + } catch (e) { + console.error("Unhandled error while updating notification settings", e); + response.status(500).send(); + } }); export const updateNotificationSettings = functions.https.onRequest(app);