firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.86k stars 894 forks source link

FR: Enable multi-tenant support in rules-unit-testing #5864

Open anothermh opened 2 years ago

anothermh commented 2 years ago

[REQUIRED] Describe your environment

[REQUIRED] Describe the problem

Firestore rules can reference request.auth properties that are not available in the RulesTestContext returned by authenticatedContext() in the rules-unit-testing package. These properties exist in the live Firestore rules environment and can be used successfully there but any attempt to evaluate those rules when using rules-unit-testing will trigger an error because the property that the rules expect to find is missing.

For example, request.auth.token.firebase.tenant is present when a user authenticates using multi-tenancy with Identity Platform, but firebase.tenant doesn't exist in the TokenOptions interface that can be passed to authenticatedContext() so any RulesTestContext returned will be missing this property.

It seems like this is the right place for this type of custom configuration given the description of TokenOptions:

More options for the mock user token to be used for testing, including developer-specfied custom claims or optional overrides for Firebase Auth token payloads.

A similar problem was brought up by someone else on the Firebase Google Group in June 2021 and a feature request was the recommended answer but I did not find one for this issue.

Steps to reproduce:

  1. Create a test suite using rules-unit-testing with an authenticated user
  2. Create Firestore rules that reference the user's tenant
  3. Run the test suite and see the error related to the missing tenant property: FirebaseError: Property tenant is undefined on object.

Relevant Code:

Create the test suite (this example uses jest) at ./tests/rules.test.ts:

import {
  assertSucceeds,
  RulesTestContext,
  initializeTestEnvironment,
  RulesTestEnvironment
} from '@firebase/rules-unit-testing';

import { readFileSync } from 'fs';

export const setup = async () => {
  return await initializeTestEnvironment({
    projectId: 'foo',
    firestore: {
      host: 'localhost',
      port: 8080,
      rules: readFileSync('firestore.rules', 'utf8')
    }
  });
};

export const teardown = async (rulesTestEnv: RulesTestEnvironment) => {
  if (!rulesTestEnv) return;

  await rulesTestEnv.clearFirestore();
  await rulesTestEnv.cleanup();
};

describe('Firestore rules', () => {
  let rulesTestEnv: RulesTestEnvironment | undefined;
  let firestore: ReturnType<RulesTestContext['firestore']> | undefined;

  beforeAll(async () => {
    rulesTestEnv = await setup();
    const alice = rulesTestEnv.authenticatedContext('alice');
    firestore = alice.firestore();
  });

  afterAll(async () => {
    if (rulesTestEnv) await teardown(rulesTestEnv);
  });

  test('permit reading collection', async () => {
    const ref = firestore?.collection('/bar') || fail('Could not create authenticatedContext');
    expect(await assertSucceeds(ref.get()));
  });
});

Create rules that check the tenant at ./firestore.rules

rules_version = '2';
service cloud.firestore {
  match /databases/{database}/documents {
    match /{document=**} {
      allow read, write: if request.auth.token.firebase.tenant == 'foobar';
    }
  }
}

Run the Firestore emulator and start the test suite:

firebase emulators:exec --only firestore jest

Observe the output:

 FAIL  tests/rules.test.ts
  Firestore rules
    ✕ permit reading collection (70 ms)

  ● Firestore rules › permit reading collection

    FirebaseError:
    Property tenant is undefined on object. for 'list' @ L6

      at fromRpcStatus (node_modules/@firebase/firestore/src/remote/serializer.ts:153:10)
      at fromWatchChange (node_modules/@firebase/firestore/src/remote/serializer.ts:477:33)
      at PersistentListenStream.onMessage (node_modules/@firebase/firestore/src/remote/persistent_stream.ts:642:25)
      at node_modules/@firebase/firestore/src/remote/persistent_stream.ts:517:21
      at node_modules/@firebase/firestore/src/remote/persistent_stream.ts:570:18
      at node_modules/@firebase/firestore/src/util/async_queue_impl.ts:135:7
      at node_modules/@firebase/firestore/src/util/async_queue_impl.ts:186:14

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        1.079 s, estimated 2 s
Ran all test suites.

Ideally, this would be resolved by being able to pass a firebase.tenant property in TokenOptions:

const alice = rulesTestEnv.authenticatedContext('alice', { firebase: { tenant: 'foobar' } });

But that presently generates an error:

Type '{ tenant: string; }' is not assignable to type '{ sign_in_provider: FirebaseSignInProvider; identities?: { email?: string[] | undefined; custom?: string[] | undefined; password?: string[] | undefined; phone?: string[] | undefined; ... 6 more ...; "apple.com"?: string[] | undefined; } | undefined; }'.
  Object literal may only specify known properties, and 'tenant' does not exist in type '{ sign_in_provider: FirebaseSignInProvider; identities?: { email?: string[] | undefined; custom?: string[] | undefined; password?: string[] | undefined; phone?: string[] | undefined; ... 6 more ...; "apple.com"?: string[] | undefined; } | undefined; }'.
argzdev commented 2 years ago

Hi @anothermh, thanks for the detailed report. Our engineers can take a look at this once they have the time.

yuchenshi commented 2 years ago

Thanks for the detailed feature request. I think this is a reasonable field to add to the typing and we welcome Pull Requests.

In the meantime rulesTestEnv.authenticatedContext('alice', { firebase: { tenant: 'foobar' } } as any) should unblock anyone who wants to test that field. (And for anyone who's using plain JS without TypeScript, you don't need to do anything special.)