Fixed some old APIs timing out when auth fails

Migrated /internal/auth/consumeAccessToken and /internal/auth/personalAccessTokens to the typed router architecture.

Migrated /updateNotificationSettings to use the typed router's response logic.

Added integration tests for all three routes.

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",

packages/api/src/orbitAPI.ts

@@ -30,7 +30,7 @@ export type ValidatableSpec = {
          */
         entityID?: EntityID;
       };
-      response?: ResponseList2<Event>;
+      response?: ResponseList<Event>;
     };
 
     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<Task>;
+      response?: ResponseList<Task>;
     };
   };
 };
 
 export type Spec = RequiredSpec<ValidatableSpec>;
 
-export type ResponseList<
-  ObjectTypeString extends string,
-  IDType extends string,
-  DataType,
-> = {
-  objectType: "list";
-  hasMore: boolean;
-  data: ResponseObject<ObjectTypeString, IDType, DataType>[];
-};
-
-export type ResponseList2<ItemType> = {
+export type ResponseList<ItemType> = {
   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;
-};

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<UserMetadata> = {},
 ) {
   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<UserMetadata> = {},
+  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<UserMetadata | null> {
+  const app = getTestFirebaseAdminApp();
+  const firestore = getFirestore(app);
+  return await getUserMetadata(userID, firestore);
+}
+
 export async function setupTestOrbitAPIClient(): Promise<OrbitAPIClient> {
   await setupAuthToken("auth");
   return new OrbitAPIClient(

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();
+  });
+});

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();
+  });
+});

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 };
 }

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<InternalAPISpec>(
+    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);