fix(generategetobjectpresignedurl): implemented changes to restrict arbitrary access

packages/actions/test/unit/security.test.ts

@@ -1,28 +1,32 @@
 import chai, { assert, expect } from "chai"
 import chaiAsPromised from "chai-as-promised"
-import { getAuth, signInWithEmailAndPassword, signOut } from "firebase/auth"
+import { getAuth, signOut, signInWithEmailAndPassword } from "firebase/auth"
 import { where } from "firebase/firestore"
+import { fakeCeremoniesData, fakeUsersData } from "../data/samples"
 import {
     createNewFirebaseUserWithEmailAndPw,
     deleteAdminApp,
+    envType,
     generatePseudoRandomStringOfNumbers,
     initializeAdminServices,
     initializeUserServices,
     sleep,
     setCustomClaims
 } from "../utils"
-import { fakeUsersData } from "../data/samples"
-import { getCurrentFirebaseAuthUser } from "../../src"
+import { commonTerms, generateGetObjectPreSignedUrl, getCurrentFirebaseAuthUser } from "../../src"
+import { TestingEnvironment } from "../../src/types/enums"
 import { getDocumentById, queryCollection } from "../../src/helpers/database"
 
 chai.use(chaiAsPromised)
 
-describe("Security rules", () => {
-    // Init admin services.
+/**
+ * Test suite for the security rules and vulnerabilities fixes.
+ */
+describe("Security", () => {
+    // Global config
     const { adminFirestore, adminAuth } = initializeAdminServices()
-    const { userApp, userFirestore } = initializeUserServices()
+    const { userApp, userFunctions, userFirestore } = initializeUserServices()
     const userAuth = getAuth(userApp)
-
     const user1 = fakeUsersData.fakeUser1
     const user2 = fakeUsersData.fakeUser2
     const user3 = fakeUsersData.fakeUser3
@@ -58,43 +62,88 @@ describe("Security rules", () => {
         await setCustomClaims(adminAuth, user3.uid, { coordinator: true })
     })
 
-    it("should allow a user to retrieve their own data from the firestore db", async () => {
-        // login as user1
-        await signInWithEmailAndPassword(userAuth, user1.data.email, user1Pwd)
-        const userDoc = await getDocumentById(userFirestore, "users", user1.uid)
-        expect(userDoc.data()).to.not.be.null
-    })
-
-    it("should allow any authenticated user to query the ceremony collection", async () => {
-        // login as user2
-        await signInWithEmailAndPassword(userAuth, user2.data.email, user2Pwd)
-        // query the ceremonies collection
-        expect(await queryCollection(userFirestore, "ceremonies", [where("description", "!=", "")])).to.not.throw
-    })
-
-    it("should throw an error if a coordiantor tries to read another user's document", async () => {
-        // login as coordinator
-        await signInWithEmailAndPassword(userAuth, user3.data.email, user3Pwd)
-        // retrieve the document of another user
-        assert.isRejected(getDocumentById(userFirestore, "users", user1.uid))
-    })
-
-    it("should throw an error if an authenticated user tries to read another user's data", async () => {
-        // login as user2
-        await signInWithEmailAndPassword(userAuth, user2.data.email, user2Pwd)
-        // @todo debug should return the error message "Missing or insufficient permissions."
-        // below should fail because we are trying to retrieve a document from another user.
-        // expect(getDocumentById(userFirestore, "users", user1.uid)).to.be.rejectedWith(
-        //     "Missing or insufficient permissions."
-        // )
-        assert.isRejected(getDocumentById(userFirestore, "users", user1.uid))
+    describe("GeneratePreSignedURL", () => {
+        it("should throw when given a bucket name that is not used for a ceremony", async () => {
+            assert.isRejected(generateGetObjectPreSignedUrl(userFunctions, "nonExistent", "test"))
+        })
+
+        // the emulator should run without .env file thus this test would not work.
+        if (envType === TestingEnvironment.PRODUCTION) {
+            it("should return a pre-signed URL when given the bucket name for a ceremony", async () => {
+                // Create the mock data on Firestore.
+                await adminFirestore
+                    .collection(commonTerms.collections.ceremonies.name)
+                    .doc(fakeCeremoniesData.fakeCeremonyOpenedFixed.uid)
+                    .set({
+                        ...fakeCeremoniesData.fakeCeremonyOpenedFixed.data
+                    })
+
+                const url = await generateGetObjectPreSignedUrl(
+                    userFunctions,
+                    fakeCeremoniesData.fakeCeremonyOpenedFixed.data.prefix,
+                    "anObject"
+                )
+                /* eslint-disable no-useless-escape */
+                const regex =
+                    /https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_\+.~#?&//=]*)/
+                expect(url).to.match(regex)
+            })
+        }
+
+        it("should throw when called without being authenticated", async () => {
+            await signOut(getAuth(userApp))
+            assert.isRejected(generateGetObjectPreSignedUrl(userFunctions, "nonExistent", "test"))
+        })
+
+        afterAll(async () => {
+            if (envType === TestingEnvironment.PRODUCTION)
+                // Delete the ceremony.
+                await adminFirestore
+                    .collection(commonTerms.collections.ceremonies.name)
+                    .doc(fakeCeremoniesData.fakeCeremonyOpenedFixed.uid)
+                    .delete()
+        })
     })
 
-    afterEach(async () => {
-        // Make sure to sign out.
-        await signOut(userAuth)
+    describe("Security rules", () => {
+        it("should allow a user to retrieve their own data from the firestore db", async () => {
+            // login as user1
+            await signInWithEmailAndPassword(userAuth, user1.data.email, user1Pwd)
+            const userDoc = await getDocumentById(userFirestore, commonTerms.collections.users.name, user1.uid)
+            expect(userDoc.data()).to.not.be.null
+        })
+
+        it("should allow any authenticated user to query the ceremony collection", async () => {
+            // login as user2
+            await signInWithEmailAndPassword(userAuth, user2.data.email, user2Pwd)
+            // query the ceremonies collection
+            expect(
+                await queryCollection(userFirestore, commonTerms.collections.ceremonies.name, [
+                    where(commonTerms.collections.ceremonies.fields.description, "!=", "")
+                ])
+            ).to.not.throw
+        })
+
+        it("should throw an error if a coordiantor tries to read another user's document", async () => {
+            // login as coordinator
+            await signInWithEmailAndPassword(userAuth, user3.data.email, user3Pwd)
+            // retrieve the document of another user
+            assert.isRejected(getDocumentById(userFirestore, commonTerms.collections.users.name, user1.uid))
+        })
+
+        it("should throw an error if an authenticated user tries to read another user's data", async () => {
+            // login as user2
+            await signInWithEmailAndPassword(userAuth, user2.data.email, user2Pwd)
+            assert.isRejected(getDocumentById(userFirestore, commonTerms.collections.users.name, user1.uid))
+        })
+
+        afterEach(async () => {
+            // Make sure to sign out.
+            await signOut(userAuth)
+        })
     })
 
+    // general clean up after all tests
     afterAll(async () => {
         // Clean user from DB.
         await adminFirestore.collection("users").doc(user1.uid).delete()

packages/backend/.default.env

@@ -18,4 +18,6 @@ AWS_PRESIGNED_URL_EXPIRATION="900"
 # nb. right now, only one user could be a coordinator for all ceremonies deployed within the same instance.
 # Note that the email should be visible from third-party (e.g., Github public email).
 # @todo allow to use a custom domain to allow multiple coordinators.
-CUSTOM_CLAIMS_COORDINATOR_EMAIL_ADDRESS_OR_DOMAIN="YOUR-EMAIL-ADDRESS-OR-DOMAIN"
+CUSTOM_CLAIMS_COORDINATOR_EMAIL_ADDRESS_OR_DOMAIN="YOUR-EMAIL-ADDRESS-OR-DOMAIN"
+# The postfix that is used to create S3 buckets for storing the ceremonies data.
+CONFIG_CEREMONY_BUCKET_POSTFIX="-ph2-ceremony"

packages/backend/src/functions/storage.ts

@@ -106,22 +106,50 @@ export const checkIfObjectExist = functions.https.onCall(
 /**
  * Generate a new AWS S3 pre signed url to upload/download an object (GET).
  */
-export const generateGetObjectPreSignedUrl = functions.https.onCall(async (data: any): Promise<any> => {
-    if (!data.bucketName || !data.objectKey) logMsg(GENERIC_ERRORS.GENERR_MISSING_INPUT, MsgType.ERROR)
+export const generateGetObjectPreSignedUrl = functions.https.onCall(
+    async (data: any, context: functions.https.CallableContext): Promise<any> => {
+        if (!process.env.CONFIG_CEREMONY_BUCKET_POSTFIX) throw new Error(GENERIC_ERRORS.GENERR_WRONG_ENV_CONFIGURATION)
+        // requires auth
+        if (!context.auth) logMsg(GENERIC_ERRORS.GENERR_NO_AUTH_USER_FOUND, MsgType.ERROR)
+
+        if (!data.bucketName || !data.objectKey) logMsg(GENERIC_ERRORS.GENERR_MISSING_INPUT, MsgType.ERROR)
+
+        // extract the bucket name and object key from the data
+        const { objectKey, bucketName } = data
+
+        // get the firestore database
+        const firestoreDatabase = admin.firestore()
+
+        // need to get the ceremony prefix from the bucket name
+        const ceremonyPrefix = bucketName.replace(process.env.CONFIG_CEREMONY_BUCKET_POSTFIX!, "")
 
-    // Connect w/ S3.
-    const S3 = await getS3Client()
+        // query the collection
+        const ceremonyCollection = await firestoreDatabase
+            .collection(commonTerms.collections.ceremonies.name)
+            .where("prefix", "==", ceremonyPrefix)
+            .get()
 
-    // Prepare the command.
-    const command = new GetObjectCommand({ Bucket: data.bucketName, Key: data.objectKey })
+        // if there is no collection with this name then we return
+        if (ceremonyCollection.empty)
+            logMsg(
+                `Cannot get pre-signed url for this object: ${objectKey} in bucket: ${bucketName} because it does not belong to any ceremony.`,
+                MsgType.ERROR
+            )
 
-    // Get the PreSignedUrl.
-    const url = await getSignedUrl(S3, command, { expiresIn: Number(process.env.AWS_PRESIGNED_URL_EXPIRATION!) })
+        // Connect w/ S3.
+        const S3 = await getS3Client()
+
+        // Prepare the command.
+        const command = new GetObjectCommand({ Bucket: bucketName, Key: objectKey })
 
-    logMsg(`Single Pre-Signed URL ${url}`, MsgType.LOG)
+        // Get the PreSignedUrl.
+        const url = await getSignedUrl(S3, command, { expiresIn: Number(process.env.AWS_PRESIGNED_URL_EXPIRATION!) })
 
-    return url
-})
+        logMsg(`Single Pre-Signed URL ${url}`, MsgType.LOG)
+
+        return url
+    }
+)
 
 /**
  * Initiate a multi part upload for a specific object in AWS S3 bucket.