Merge pull request #1512 from firebase/next

March 17 2023 release

delete-user-data/CHANGELOG.md

@@ -1,3 +1,7 @@
+## Version 0.1.16
+
+fixed - Increase UID checks on search based deletions
+
 ## Version 0.1.15
 
 feature - upgrade extensions to the latest firebase-admin sdk

delete-user-data/README.md

@@ -68,7 +68,7 @@ Here's a series of examples. To delete all the files in your default bucket with
 
 * Auto discovery search fields: If auto discovery is enabled, specify what document fields are used to associate the UID with the document. The extension will delete documents where the value for one or more of these fields matches the deleting user’s UID. If left empty, document fields will not be used in auto discovery.
 
-* Search function URL: Specify a function URL to call that will return a list of document paths to delete.
+* Search function URL: Specify a URL to call that will return a list of document paths to delete. The extension will send a `POST` request to the specified `URL`, with the `uid` of the deleted user will be provided in the body of the request. The endpoint specified should return an array of firestore paths to delete.
 
 
 

delete-user-data/extension.yaml

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 name: delete-user-data
-version: 0.1.15
+version: 0.1.16
 specVersion: v1beta
 
 displayName: Delete User Data
@@ -275,7 +275,7 @@ params:
   - param: SEARCH_FUNCTION
     label: Search function URL
     description: >-
-      Specify a function URL to call that will return a list of document paths to delete.
+      Specify a URL to call that will return a list of document paths to delete. The extension will send a `POST` request to the specified `URL`, with the `uid` of the deleted user will be provided in the body of the request. The endpoint specified should return an array of firestore paths to delete.
     example: https://us-west1-my-project-id.cloudfunctions.net/myTransformFunction
     type: string
     required: false

delete-user-data/functions/__tests__/helpers.test.ts

@@ -1,74 +1,127 @@
-import { getDatabaseUrl } from "../src/helpers";
-
-describe("Test Realtime Database URL helper function", () => {
-  test("Can return the correct url for us-central-1", () => {
-    const environment = {
-      SELECTED_DATABASE_INSTANCE: "server-name",
-      SELECTED_DATABASE_LOCATION: "us-central1",
-    };
-
-    const serverUrl = getDatabaseUrl(
-      environment.SELECTED_DATABASE_INSTANCE,
-      environment.SELECTED_DATABASE_LOCATION
-    );
-    expect(serverUrl).toBe(
-      `https://${environment.SELECTED_DATABASE_INSTANCE}.firebaseio.com`
-    );
-  });
+import * as admin from "firebase-admin";
+import { UserRecord } from "firebase-functions/v1/auth";
+import setupEnvironment from "./helpers/setupEnvironment";
 
-  test("Can return the correct url for europe-west1", () => {
-    const environment = {
-      SELECTED_DATABASE_INSTANCE: "server-name",
-      SELECTED_DATABASE_LOCATION: "europe-west1",
-    };
-
-    const serverUrl = getDatabaseUrl(
-      environment.SELECTED_DATABASE_INSTANCE,
-      environment.SELECTED_DATABASE_LOCATION
-    );
-    expect(serverUrl).toBe(
-      `https://${environment.SELECTED_DATABASE_INSTANCE}.europe-west1.firebasedatabase.app`
-    );
-  });
+import { hasValidUserPath, getDatabaseUrl } from "../src/helpers";
+import { createFirebaseUser } from "./helpers";
 
-  test("Can return the correct url for asia-southeast1", () => {
-    const environment = {
-      SELECTED_DATABASE_INSTANCE: "server-name",
-      SELECTED_DATABASE_LOCATION: "asia-southeast1",
-    };
-
-    const serverUrl = getDatabaseUrl(
-      environment.SELECTED_DATABASE_INSTANCE,
-      environment.SELECTED_DATABASE_LOCATION
-    );
-    expect(serverUrl).toBe(
-      `https://${environment.SELECTED_DATABASE_INSTANCE}.asia-southeast1.firebasedatabase.app`
-    );
-  });
+admin.initializeApp();
+setupEnvironment();
+
+const db = admin.firestore();
+const collection = db.collection("hasValidUserPath");
+let user: UserRecord;
+
+describe("helpers", () => {
+  describe("Test Realtime Database URL helper function", () => {
+    test("Can return the correct url for us-central-1", () => {
+      const environment = {
+        SELECTED_DATABASE_INSTANCE: "server-name",
+        SELECTED_DATABASE_LOCATION: "us-central1",
+      };
+
+      const serverUrl = getDatabaseUrl(
+        environment.SELECTED_DATABASE_INSTANCE,
+        environment.SELECTED_DATABASE_LOCATION
+      );
+      expect(serverUrl).toBe(
+        `https://${environment.SELECTED_DATABASE_INSTANCE}.firebaseio.com`
+      );
+    });
+
+    test("Can return the correct url for europe-west1", () => {
+      const environment = {
+        SELECTED_DATABASE_INSTANCE: "server-name",
+        SELECTED_DATABASE_LOCATION: "europe-west1",
+      };
+
+      const serverUrl = getDatabaseUrl(
+        environment.SELECTED_DATABASE_INSTANCE,
+        environment.SELECTED_DATABASE_LOCATION
+      );
+      expect(serverUrl).toBe(
+        `https://${environment.SELECTED_DATABASE_INSTANCE}.europe-west1.firebasedatabase.app`
+      );
+    });
+
+    test("Can return the correct url for asia-southeast1", () => {
+      const environment = {
+        SELECTED_DATABASE_INSTANCE: "server-name",
+        SELECTED_DATABASE_LOCATION: "asia-southeast1",
+      };
+
+      const serverUrl = getDatabaseUrl(
+        environment.SELECTED_DATABASE_INSTANCE,
+        environment.SELECTED_DATABASE_LOCATION
+      );
+      expect(serverUrl).toBe(
+        `https://${environment.SELECTED_DATABASE_INSTANCE}.asia-southeast1.firebasedatabase.app`
+      );
+    });
+
+    test("Return null if instance is undefined", () => {
+      const environment = {
+        SELECTED_DATABASE_INSTANCE: undefined,
+        SELECTED_DATABASE_LOCATION: "asia-southeast1",
+      };
 
-  test("Return null if instance is undefined", () => {
-    const environment = {
-      SELECTED_DATABASE_INSTANCE: undefined,
-      SELECTED_DATABASE_LOCATION: "asia-southeast1",
-    };
-
-    const serverUrl = getDatabaseUrl(
-      environment.SELECTED_DATABASE_INSTANCE,
-      environment.SELECTED_DATABASE_LOCATION
-    );
-    expect(serverUrl).toBe(null);
+      const serverUrl = getDatabaseUrl(
+        environment.SELECTED_DATABASE_INSTANCE,
+        environment.SELECTED_DATABASE_LOCATION
+      );
+      expect(serverUrl).toBe(null);
+    });
+
+    test("Return null if location is undefined", () => {
+      const environment = {
+        SELECTED_DATABASE_INSTANCE: "server-name",
+        SELECTED_DATABASE_LOCATION: undefined,
+      };
+
+      const serverUrl = getDatabaseUrl(
+        environment.SELECTED_DATABASE_INSTANCE,
+        environment.SELECTED_DATABASE_LOCATION
+      );
+      expect(serverUrl).toBe(null);
+    });
   });
 
-  test("Return null if location is undefined", () => {
-    const environment = {
-      SELECTED_DATABASE_INSTANCE: "server-name",
-      SELECTED_DATABASE_LOCATION: undefined,
-    };
-
-    const serverUrl = getDatabaseUrl(
-      environment.SELECTED_DATABASE_INSTANCE,
-      environment.SELECTED_DATABASE_LOCATION
-    );
-    expect(serverUrl).toBe(null);
+  describe("hasValidUserPath", () => {
+    DocumentReference: beforeAll(async () => {
+      /** create a test user */
+      user = await createFirebaseUser();
+    });
+    test("should return true if the path matches a valid string", async () => {
+      /** create a string example field value  */
+      const stringDoc = await collection.add({ field1: user.uid });
+
+      /** get the result */
+      const result = await hasValidUserPath(stringDoc, "", user.uid);
+
+      /** check the result */
+      expect(result).toBeTruthy();
+    });
+
+    test("should return true if the path matches a valid string path", async () => {
+      /** create a string example field value  */
+      const stringDoc = await collection.add({ field1: `testing/${user.uid}` });
+
+      /** get the result */
+      const result = await hasValidUserPath(stringDoc, "", user.uid);
+
+      /** check the result */
+      expect(result).toBeTruthy();
+    });
+
+    test("should return false if with a non string value", async () => {
+      /** create a string example field value  */
+      const stringDoc = await collection.add({ field1: 1234 });
+
+      /** get the result */
+      const result = await hasValidUserPath(stringDoc, "", user.uid);
+
+      /** check the result */
+      expect(result).toBeFalsy();
+    });
   });
 });

delete-user-data/functions/__tests__/runBatchPubSubDeletions.test.ts

@@ -0,0 +1,44 @@
+import * as admin from "firebase-admin";
+import { runBatchPubSubDeletions } from "../src/runBatchPubSubDeletions";
+import setupEnvironment from "./helpers/setupEnvironment";
+
+import { Paths } from "../src/types";
+
+admin.initializeApp();
+setupEnvironment();
+
+const db = admin.firestore();
+
+const generateTopLevelUserCollection = async (name) => {
+  const collection = db.collection(name);
+
+  return collection;
+};
+
+describe("runBatchPubSubDeletions", () => {
+  let rootCollection: admin.firestore.CollectionReference;
+
+  beforeEach(async () => {
+    rootCollection = await generateTopLevelUserCollection(
+      "runBatchPubSubDeletions"
+    );
+  });
+
+  test("cannot delete paths with an invalid userId", async () => {
+    /** Add a new document for testing */
+    const doc = await rootCollection.add({ testing: "testing" });
+    const invalidUserId = "invalidUserId";
+
+    const paths: Paths = { firestorePaths: [`${rootCollection.id}/${doc.id}`] };
+
+    /** Run deletion */
+    await runBatchPubSubDeletions(paths, invalidUserId);
+
+    /** Wait 10 seconds */
+    await new Promise((resolve) => setTimeout(resolve, 10000));
+
+    /** Check document still exist */
+    const documentCheck = await doc.get();
+    expect(documentCheck.exists).toBe(true);
+  }, 60000);
+});

delete-user-data/functions/__tests__/search.test.ts

@@ -64,7 +64,7 @@ describe("discovery", () => {
         .add({ field1: user.uid });
       await search(user.uid, 1);
 
-      await waitForDocumentDeletion(document);
+      await waitForDocumentDeletion(document, 60000);
     }, 60000);
 
     test("can check a document without any field values", async () => {
@@ -162,4 +162,32 @@ describe("discovery", () => {
       expect(checkExists).toBe(true);
     }, 60000);
   });
+
+  describe("does not delete documents that do not match the search criteria", () => {
+    test("can delete a document named {uid}", async () => {
+      /** Create a collection to try and delete */
+      const collection = await db.collection(generateRandomId());
+      const document = await collection.add({ testing: "should-not-delete" });
+
+      await search(collection.id, -1, document);
+
+      /** Check document still exists */
+      const checkExists = await document.get().then((doc) => doc.exists);
+      expect(checkExists).toBe(true);
+    }, 60000);
+
+    test("cannot delete a document without a valid field named {uid}", async () => {
+      const document = await db
+        .collection(generateRandomId())
+        .add({ field1: "unknown" });
+      await search(user.uid, 1);
+
+      /** Wait 10 seconds */
+      await new Promise((resolve) => setTimeout(resolve, 10000));
+
+      /** Check document still exists */
+      const checkExists = await document.get().then((doc) => doc.exists);
+      expect(checkExists).toBe(true);
+    }, 60000);
+  });
 });

delete-user-data/functions/jest.config.js

@@ -4,11 +4,12 @@ module.exports = {
   name: packageJson.name,
   displayName: packageJson.name,
   testEnvironment: "node",
-  rootDir: "./__tests__",
   preset: "ts-jest",
   testMatch: ["**/__tests__/*.test.ts"],
-  setupFilesAfterEnv: ["<rootDir>/setupTests.ts"],
+  setupFilesAfterEnv: ["<rootDir>/__tests__/setupTests.ts"],
   moduleNameMapper: {
+    "firebase-admin/firestore":
+      "<rootDir>/node_modules/firebase-admin/lib/firestore",
     "firebase-admin/auth": "<rootDir>/node_modules/firebase-admin/lib/auth",
   },
 };

delete-user-data/functions/src/helpers.ts

@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+import { DocumentReference, FieldPath } from "firebase-admin/firestore";
+import config from "./config";
+
 export const getDatabaseUrl = (
   selectedDatabaseInstance: string | undefined,
   selectedDatabaseLocation: string | undefined
@@ -25,3 +28,29 @@ export const getDatabaseUrl = (
 
   return `https://${selectedDatabaseInstance}.${selectedDatabaseLocation}.firebasedatabase.app`;
 };
+
+export const hasValidUserPath = async (
+  ref: DocumentReference,
+  path: string,
+  uid: string
+): Promise<boolean> => {
+  /** Check path for valid user id */
+  if (path.includes(uid)) return true;
+
+  /** Check to find valid field */
+  const snapshot = await ref.get();
+
+  if (snapshot.exists) {
+    for (const field of config.searchFields.split(",")) {
+      const fieldValue = snapshot.get(new FieldPath(field));
+
+      /** Return if a matching string includes the id */
+      if (typeof fieldValue === "string" && fieldValue.includes(uid)) {
+        return true;
+      }
+    }
+  }
+
+  /** Return as invalid path */
+  return false;
+};