Firebase's "Build Unit Tests" storage documentation is incomplete and incorrect

[lovelle-cardoso]

[REQUIRED] Describe your environment

• Operating System version: Windows 10 Home Version 20H2, Build 19042.1052
• Browser version: N/A (using mocha and @firebase/rules-unit-testing testing library)
• Firebase SDK version: @firebase/rules-unit-testing 1.3.8
• Firebase Product: storage, storage emulator, testing sdk

[REQUIRED] Describe the problem

If you attempt to write storage unit tests in the way the official documentation describes, you will encounter several errors that prevent the tests from running as intended.

Steps to reproduce:

Step 1: Read the documentation for setting up cloud storage unit tests at: https://firebase.google.com/docs/rules/unit-tests#storage

Step 2: Attempt to write unit tests following the examples in the documentation

Expected Behavior:

By following along with the documentation, you are able to write unit tests that test your storage security rules.

Actual Behavior:

You will encounter several issues which you must work around and are not stated in the documentation.

---

Issue 1: If you are writing your tests in a nodejs environment, you will encounter a "XMLHttpRequest is not defined" error when attempting to use the storage module. You must install and import xhr2 to resolve this issue.

---

Issue 2: The documentation says you may pass an auth object when initializing your testing app. But if you attempt to pass this auth object and test storage security rules, you will encounter an error that prevents your tests from running. This bug was reported a month ago here: firebase/firebase-tools#3442 but is still being actively worked on here: #5002. (I'd appreciate if this bug was noted in the documentation, considering how critical authentication is to testing security)

---

Issue 3: There is no documentation that describes how to test uploading and downloading a file. The documentation only shows examples for ref.getMetadata(). It's strange that the documentation doesn't include any examples of what most users would be interested in testing for storage (file uploads and downloads) and only includes examples for getting a file's metadata (an action which is not normally that crucial to security)

---

Issue 4: If you try to write tests using firebase.assertFails for reading, uploading, and downloading files, the getDownloadUrl(), and put() methods will throw an error that the rules-unit-testing library does not recognize. This results in you seeing oddly contradictory errors like this:

Error: Expected PERMISSION_DENIED but got unexpected error: FirebaseError: Firebase Storage: User does not have permission to access 'users/O0hAxVxGPTRDz77GC4CLwu16TmT2/18808/000'. (storage/unauthorized)
Forbidden

After some investigation in the rules-unit-testing codebase, you may notice that the assertFails is checking for the phrase "permission-denied" or "permission denied" in the error message. But because the storage permission error messages don't include this specific phrase, assertFails won't work for most storage operations.

firebase-js-sdk/packages/rules-unit-testing/src/api/index.ts

Lines 686 to 713 in e689ff6

	export function assertFails(pr: Promise<any>): any {
	return pr.then(
	(v: any) => {
	return Promise.reject(
	new Error('Expected request to fail, but it succeeded.')
	);
	},
	(err: any) => {
	const errCode = (err && err.code && err.code.toLowerCase()) || '';
	const errMessage =
	(err && err.message && err.message.toLowerCase()) || '';
	const isPermissionDenied =
	errCode === 'permission-denied' ||
	errCode === 'permission_denied' ||
	errMessage.indexOf('permission_denied') >= 0 ||
	errMessage.indexOf('permission denied') >= 0;
	if (!isPermissionDenied) {
	return Promise.reject(
	new Error(
	`Expected PERMISSION_DENIED but got unexpected error: ${err}`
	)
	);
	}
	return err;
	}
	);
	}

In order to get your tests working properly, you'll need to wrap all of your storage calls in functions like these that explicitly throw an error with the "permission denied" string that assertFails is looking for:

const readFile = async (path: string): Promise<void> => {
  const client = getClient();
  try {
    await client.ref().child(path).getDownloadURL();
  } catch (error) {
    throw new Error("permission denied");
  }
};

const uploadFile = async (
  path: string,
  data: Blob | Uint8Array | ArrayBuffer,
  metadata?: { customMetadata: Record<string, string> }
): Promise<void> => {
  const client = getClient();
  try {
    await client.ref().child(path).put(data, metadata).then();
  } catch (error) {
    throw new Error("permission denied");
  }
};

---

Issue 5: If you work around all the previous issues, you should now be able to test "get" "list", "create" and "delete" operations. However, you will probably encounter unexpected behavior when attempting to test any "update" rules. The documentation is vague on when the "update" rule is evaluated. According to this documentation, the "update" rule:

applies to writes to existing files

https://firebase.google.com/docs/storage/security/core-syntax#granular_operations.

One might interpret this to mean that the update rule will apply when a user overwrites a file by uploading a file to a path where a file already exists. However, that is not the case. The "update" rule seems to only apply when you call updateMetadata() (so when you update a file's metadata, without overwriting the file itself). When you overwrite an existing file the "create" rule is always the one that is evaluated, not the "update" rule. Furthermore, the "resource" value is always null for file overwrites, even when there is a file that already exists at that path. This means it is simply not possible to write storage security rules that prevent a user from overwriting an existing file. Note that this behavior for the update rule is very different than the behavior of the update rule in firestore security rules (in firestore it IS possible to write rules to prevent document overwrites, because the create rule is only evaluated if a document doesn't already exist at the path you are attempting to write to). But this limitation in storage security rules it is not documented anywhere in the guide or in the API reference. And in fact, the API reference never even mentions the existence of the granular "create", "update", and "delete" methods.

I've written up more information about this undocumented behavior in my comment on this issue here: #5079 (comment)

---

NOTE: I didn't run into any problems when writing all of my firestore unit tests, but I've been struggling to work through a lot of these undocumented bugs when unit testing my storage security rules. I understand that the storage emulator and the @firebase/rules-unit-testing are very new additions to firebase, but with the amount of issues I encountered, it was surprising that the the official firebase documentation recommended starting to write unit tests for storage, considering how many problems you will encounter when attempting to write basic tests (auth being broken, assertFails not working for most storage operations, etc.).

It may be worth considering moving the storage part of the @firebase/rules-unit-testing library back into beta and removing the "storage" section from the "Build Unit Tests" documentation for the time being until all these issues can be resolved. Also, it may be helpful to have a QA tester run through the documentation (making sure the examples included in the documentation work as expected) before releasing the documentation and feature out to the public.

Thank you guys for all your hard work on the new emulators and testing features!

[mcarriere]

I was just about to create the same issue, could not have written it better 👍

[looptheloop88]

Hi @lovelle-cardoso, thanks for the feedback. We really appreciate your detailed report. We can't provide definite timelines or details, but we will try to update our documentations, code snippets and quickstart project for Firebase Storage unit testing.

[lovelle-cardoso]

@looptheloop88 Could you open a bug for Issue 2 and Issue 4 so we can track when the storage part of the testing library will be usable? It probably makes more sense to label those as bugs rather than feature requests, since they are bugs in the testing library and not just documentation issues. I could also open separate bugs for those two issues if that's easier for you guys to label.

Thanks again!

[looptheloop88]

Hi @lovelle-cardoso, I was able to replicate issue 1, 2 and 4. Issue 5 is being tracked in #5079. I've already brought this matter to the attention of our engineers here. We'll keep this thread posted for any updates.

[lovelle-cardoso]

@looptheloop88 Awesome! Thanks a bunch!

[edvomarv]

Issue 1: If you are writing your tests in a nodejs environment, you will encounter a "XMLHttpRequest is not defined" error when attempting to use the storage module. You must install and import xhr2 to resolve this issue.

I'm not clear on this workaround — merely installing xhr2 and then importing it into the test file didn't seem to change anything.

From what I can tell, an app returned by initializeAdminApp can be used successfully with the Storage Emulator, but an app returned by initializeTestApp always fails with XMLHttpRequest is not defined.

import { test, describe, afterAll, beforeAll } from '@jest/globals';
import "xhr2"; // doesn't seem to do anything?
import * as firebase from "@firebase/rules-unit-testing";

beforeAll(async () => {
  let emulatorSpec = await firebase.discoverEmulators();
  firebase.useEmulators(emulatorSpec);
})
afterAll(() => {
  Promise.all(firebase.apps().map(app => app.delete()));
});

describe("adminApp", () => {
  test("put", async (done) => {
    let adminApp = firebase.initializeAdminApp({ storageBucket: "my-bucket" });
    await adminApp.storage().bucket().file("foo").save("foo stuff");
    done!();
  });
});

describe("testApp", () => {
  test("putString", async (done) => {
    let testApp = firebase.initializeTestApp({
      storageBucket: "my-bucket",
      auth: { uid: "alice" }
    });
    // This fails with `ReferenceError: XMLHttpRequest is not defined`
    await testApp.storage().ref("bar").putString("bar stuff");
    done!();
  });
});

[looptheloop88]

Here's what I've used in my test script:

const firebase = require("@firebase/rules-unit-testing");
global.XMLHttpRequest = require('xhr2');

[lovelle-cardoso]

@edvomarv

describe("testApp", () => {
  test("putString", async (done) => {
    let testApp = firebase.initializeTestApp({
      storageBucket: "my-bucket",
      auth: { uid: "alice" }
    });
    // This fails with `ReferenceError: XMLHttpRequest is not defined`
    await testApp.storage().ref("bar").putString("bar stuff");
    done!();
  });
});

As I mentioned in Issue 2, auth is currently broken for storage. So passing an auth object into initializeTestApp and calling app.storage() will throw an error. You won't be able to write any tests that test rules that require request.auth until Issue 2 is fixed. (e.g. allow write: if request.auth.uid == uid can't be tested at the moment because the testing library doesn't currently support auth for storage, despite the documentation saying it does.)

[lovelle-cardoso]

@looptheloop88 @Feiyang1 I figured I'd throw in a PR for Issue 4 since it's a fairly straightforward fix: #5209

[lovelle-cardoso]

@looptheloop88 @Feiyang1 Any update on Issue 2 btw? That auth bug is the only one I was unable to find a workaround for.

[yuchenshi]

@lovelle-cardoso FYI #5002 has been released. FWIW, Storage JS SDK no longer depends on XHR on Node.js, so the xhr2 workaround can be dropped too.