From 4380354eaaa4b842b51407768462cb6fceaf63e7 Mon Sep 17 00:00:00 2001 From: ben12 Date: Sat, 22 Feb 2025 12:21:19 +0100 Subject: [PATCH 1/2] Allow to use firestore rules on attributes (#6707) --- config/firestore.json | 19 ++++++++++ config/firestore.rules | 13 +++++++ package.json | 3 +- .../replication-firestore/firestore-helper.ts | 2 +- .../replication-firestore/firestore-types.ts | 3 +- src/plugins/replication-firestore/index.ts | 14 ++++++- src/plugins/test-utils/humans-collection.ts | 36 ++++++++++++++++++ src/plugins/test-utils/schema-objects.ts | 21 ++++++++++ src/plugins/test-utils/schemas.ts | 36 ++++++++++++++++++ test/replication-firestore.test.ts | 38 ++++++++++++++++--- 10 files changed, 175 insertions(+), 10 deletions(-) create mode 100644 config/firestore.json create mode 100644 config/firestore.rules diff --git a/config/firestore.json b/config/firestore.json new file mode 100644 index 00000000000..cb6489e9007 --- /dev/null +++ b/config/firestore.json @@ -0,0 +1,19 @@ +{ + "firestore": { + "database": "(default)", + "rules": "firestore.rules" + }, + "auth": { + }, + "emulators": { + "singleProjectMode": true, + "firestore": { + "host": "localhost", + "port": 8080 + }, + "auth": { + "host": "localhost", + "port": 9099 + } + } +} diff --git a/config/firestore.rules b/config/firestore.rules new file mode 100644 index 00000000000..ad289211c63 --- /dev/null +++ b/config/firestore.rules @@ -0,0 +1,13 @@ +rules_version = '2'; +service cloud.firestore { + match /databases/{database}/documents { + match /public/{document=**} { + allow read, write: if true; + } + match /ownership/{document=**} { + allow read, delete: if request.auth != null && (resource == null || request.auth.uid == resource.data.owner); + allow create: if request.auth != null && request.auth.uid == request.resource.data.owner; + allow update: if request.auth != null && request.auth.uid == request.resource.data.owner && request.auth.uid == resource.data.owner; + } + } +} \ No newline at end of file diff --git a/package.json b/package.json index 14dc8eb3d7a..18d0e082788 100644 --- a/package.json +++ b/package.json @@ -380,7 +380,7 @@ "test:browser:remote": " npm run transpile && cross-env CI=true DEFAULT_STORAGE=remote karma start ./config/karma.conf.cjs --single-run", "test:browser:remote:loop": "npm run test:browser:remote && npm run test:browser:remote:loop", "test:browser:custom": " npm run transpile && cross-env CI=true DEFAULT_STORAGE=custom karma start ./config/karma.conf.cjs --single-run", - "test:replication-firestore": "npm run transpile && firebase emulators:exec \"cross-env DEFAULT_STORAGE=dexie mocha --expose-gc --config ./config/.mocharc.cjs ./test_tmp/replication-firestore.test.js\" --only firestore --project 'rxdb-test'", + "test:replication-firestore": "npm run transpile && firebase emulators:exec \"cross-env DEFAULT_STORAGE=dexie mocha --expose-gc --config ./config/.mocharc.cjs ./test_tmp/replication-firestore.test.js\" --only firestore --config \"config/firestore.json\" --project \"rxdb-test\"", "aaa": "firebase init firestore", "test:replication-couchdb": "npm run transpile && concurrently \"npm run couch:start\" \"cross-env NATIVE_COUCHDB=5984 DEFAULT_STORAGE=dexie mocha --config ./config/.mocharc.cjs ./test_tmp/replication-couchdb.test.js\" --success first --kill-others", "test:replication-nats": "npm run transpile && concurrently \"npm run nats:start\" \"cross-env DEFAULT_STORAGE=dexie mocha --config ./config/.mocharc.cjs ./test_tmp/replication-nats.test.js\" --success first --kill-others", @@ -495,6 +495,7 @@ "@eslint/compat": "1.2.7", "@eslint/eslintrc": "3.3.0", "@eslint/js": "9.21.0", + "@firebase/rules-unit-testing": "4.0.1", "@rollup/plugin-commonjs": "28.0.2", "@rollup/plugin-node-resolve": "16.0.0", "@stylistic/eslint-plugin": "4.0.1", diff --git a/src/plugins/replication-firestore/firestore-helper.ts b/src/plugins/replication-firestore/firestore-helper.ts index 4f96a6eaddd..b187af352e2 100644 --- a/src/plugins/replication-firestore/firestore-helper.ts +++ b/src/plugins/replication-firestore/firestore-helper.ts @@ -75,5 +75,5 @@ export function getContentByIds(ids: string[], getQuery: GetQuery content.map(i => i.docs).flat()); + return Promise.all(batches).then((content) => content.flat()); } diff --git a/src/plugins/replication-firestore/firestore-types.ts b/src/plugins/replication-firestore/firestore-types.ts index d4c004eb4eb..9543da2cc4c 100644 --- a/src/plugins/replication-firestore/firestore-types.ts +++ b/src/plugins/replication-firestore/firestore-types.ts @@ -9,6 +9,7 @@ import type { import type { CollectionReference, Firestore, + QueryDocumentSnapshot, QueryFieldFilterConstraint, QuerySnapshot } from 'firebase/firestore'; @@ -67,4 +68,4 @@ export type SyncOptionsFirestore = Omit< push?: FirestoreSyncPushOptions; }; -export type GetQuery = (ids: string[]) => Promise>; +export type GetQuery = (ids: string[]) => Promise[]>; diff --git a/src/plugins/replication-firestore/index.ts b/src/plugins/replication-firestore/index.ts index 229e033aa1f..28be3ee5755 100644 --- a/src/plugins/replication-firestore/index.ts +++ b/src/plugins/replication-firestore/index.ts @@ -15,6 +15,7 @@ import { orderBy, limit, getDocs, + getDoc, onSnapshot, runTransaction, writeBatch, @@ -257,7 +258,18 @@ export function replicateFirestore( options.firestore.collection, where(documentId(), 'in', ids) ) - ); + ) + .then(result => result.docs) + .catch(() => { + // Query may fail due to rules using 'resource' with non existing ids + // So try to get the docs one by one + return Promise.all( + ids.map( + id => getDoc(doc(options.firestore.collection, id)) + ) + ) + .then(docs => docs.filter(doc => doc.exists())); + }); }; const docsInDbResult = await getContentByIds(docIds, getQuery); diff --git a/src/plugins/test-utils/humans-collection.ts b/src/plugins/test-utils/humans-collection.ts index 355f94c6fb7..a9d89a5e0fe 100644 --- a/src/plugins/test-utils/humans-collection.ts +++ b/src/plugins/test-utils/humans-collection.ts @@ -518,3 +518,39 @@ export async function createIdAndAgeIndex( return collections.humana; } + +export async function createHumanWithOwnership( + amount = 20, + databaseName = randomToken(10), + multiInstance = true, + owner = "alice", + storage = getConfig().storage.getStorage(), + conflictHandler?: RxConflictHandler, + +): Promise> { + + const db = await createRxDatabase<{ humans: RxCollection; }>({ + name: databaseName, + storage, + multiInstance, + eventReduce: true, + ignoreDuplicate: true + }); + // setTimeout(() => db.close(), dbLifetime); + const collections = await db.addCollections({ + humans: { + conflictHandler, + schema: schemas.humanWithOwnership + } + }); + + // insert data + if (amount > 0) { + const docsData = new Array(amount) + .fill(0) + .map(() => schemaObjects.humanWithOwnershipData({}, owner)); + await collections.humans.bulkInsert(docsData); + } + + return collections.humans; +} diff --git a/src/plugins/test-utils/schema-objects.ts b/src/plugins/test-utils/schema-objects.ts index 32d4363954d..bb6544d6766 100644 --- a/src/plugins/test-utils/schema-objects.ts +++ b/src/plugins/test-utils/schema-objects.ts @@ -505,3 +505,24 @@ export function humanWithCompositePrimary(partial: Partial = {}, owner: string): HumanWithOwnershipDocumentType { + const defaultObj = { + passportId: randomStringWithSpecialChars(8, 12), + firstName: randomStringWithSpecialChars(8, 12), + lastName: randomStringWithSpecialChars(8, 12), + age: randomNumber(10, 50), + owner + }; + return Object.assign( + defaultObj, + partial + ); +} diff --git a/src/plugins/test-utils/schemas.ts b/src/plugins/test-utils/schemas.ts index a8e0fa9cfe2..e4f7d04a99d 100644 --- a/src/plugins/test-utils/schemas.ts +++ b/src/plugins/test-utils/schemas.ts @@ -1344,6 +1344,42 @@ export const humanIdAndAgeIndex: RxJsonSchema<{ id: string; name: string; age: n ] }); +export const humanWithOwnership: RxJsonSchema = overwritable.deepFreezeWhenDevMode({ + title: 'human schema', + version: 0, + description: 'describes a human being', + keyCompression: false, + primaryKey: 'passportId', + type: 'object', + properties: { + passportId: { + type: 'string', + maxLength: 100 + }, + firstName: { + type: 'string', + maxLength: 100 + }, + lastName: { + type: 'string', + maxLength: 100 + }, + age: { + description: 'age in years', + type: 'integer', + minimum: 0, + maximum: 150, + default: 20 + }, + owner: { + type: 'string', + maxLength: 128 + } + }, + indexes: [], + required: ['passportId'] +}); + export function enableKeyCompression( schema: RxJsonSchema diff --git a/test/replication-firestore.test.ts b/test/replication-firestore.test.ts index 9afd2591765..785f586e3da 100644 --- a/test/replication-firestore.test.ts +++ b/test/replication-firestore.test.ts @@ -41,7 +41,8 @@ import { where, orderBy, limit, - getDoc + getDoc, + QueryConstraint } from 'firebase/firestore'; import { FirestoreOptions, @@ -68,8 +69,8 @@ describe('replication-firestore.test.ts', function () { */ const batchSize = 5; type TestDocType = HumanWithTimestampDocumentType; - async function getAllDocsOfFirestore(firestore: FirestoreOptions): Promise { - const result = await getDocs(query(firestore.collection)); + async function getAllDocsOfFirestore
(firestore: FirestoreOptions
, ...criterias: QueryConstraint[]): Promise { + const result = await getDocs(query(firestore.collection, ...criterias)); return result.docs.map(d => { const docData = d.data(); (docData as any).id = d.id; @@ -77,15 +78,18 @@ describe('replication-firestore.test.ts', function () { }) as any; } const projectId = randomToken(10); + const ownerUid = 'owner1'; + + config.storage.init?.(); const app = firebase.initializeApp({ projectId, databaseURL: 'http://localhost:8080?ns=' + projectId }); const database = getFirestore(app); - connectFirestoreEmulator(database, 'localhost', 8080); + connectFirestoreEmulator(database, 'localhost', 8080, { mockUserToken: { user_id: ownerUid }}); - function getFirestoreState(): FirestoreOptions { - const useCollection: CollectionReference = getFirestoreCollection(database, randomToken(10)) as any; + function getFirestoreState(rootCollection = 'public'): FirestoreOptions { + const useCollection: CollectionReference = getFirestoreCollection(database, rootCollection, randomToken(10), randomToken(10)) as any; return { projectId, collection: useCollection, @@ -459,6 +463,28 @@ describe('replication-firestore.test.ts', function () { collection.database.close(); }); + it('#6707 firestore replication this owner rules', async () => { + const collection1 = await humansCollection.createHumanWithOwnership(2, undefined, false, ownerUid); + const firestoreState = getFirestoreState('ownership'); + const replicationState = replicateFirestore({ + replicationIdentifier: firestoreState.projectId, + firestore: firestoreState, + collection: collection1, + pull: { + filter: [ + where('owner', '==', ownerUid), + ], + }, + push: {}, + live: true, + autoStart: true + }); + ensureReplicationHasNoErrors(replicationState); + await replicationState.awaitInitialReplication(); + + const docsOnServer = await getAllDocsOfFirestore(firestoreState, where('owner', '==', ownerUid)); + assert.strictEqual(docsOnServer.length, 2); + }); }); }); From 9cdad1974e388abfa542a3d7ad7492b08f7b87fe Mon Sep 17 00:00:00 2001 From: ben12 Date: Sun, 23 Feb 2025 10:17:11 +0100 Subject: [PATCH 2/2] Fix spelling test + fix only for expected error (#6707) --- src/plugins/replication-firestore/index.ts | 22 +++++++++++++--------- test/replication-firestore.test.ts | 4 ++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/plugins/replication-firestore/index.ts b/src/plugins/replication-firestore/index.ts index 28be3ee5755..9632b054dda 100644 --- a/src/plugins/replication-firestore/index.ts +++ b/src/plugins/replication-firestore/index.ts @@ -22,7 +22,8 @@ import { serverTimestamp, QueryDocumentSnapshot, waitForPendingWrites, - documentId + documentId, + FirestoreError } from 'firebase/firestore'; import { RxDBLeaderElectionPlugin } from '../leader-election/index.ts'; @@ -260,15 +261,18 @@ export function replicateFirestore( ) ) .then(result => result.docs) - .catch(() => { - // Query may fail due to rules using 'resource' with non existing ids - // So try to get the docs one by one - return Promise.all( - ids.map( - id => getDoc(doc(options.firestore.collection, id)) + .catch(error => { + if (error?.code && (error as FirestoreError).code === 'permission-denied') { + // Query may fail due to rules using 'resource' with non existing ids + // So try to get the docs one by one + return Promise.all( + ids.map( + id => getDoc(doc(options.firestore.collection, id)) + ) ) - ) - .then(docs => docs.filter(doc => doc.exists())); + .then(docs => docs.filter(doc => doc.exists())); + } + throw error; }); }; diff --git a/test/replication-firestore.test.ts b/test/replication-firestore.test.ts index 785f586e3da..2f1bf0b5370 100644 --- a/test/replication-firestore.test.ts +++ b/test/replication-firestore.test.ts @@ -69,8 +69,8 @@ describe('replication-firestore.test.ts', function () { */ const batchSize = 5; type TestDocType = HumanWithTimestampDocumentType; - async function getAllDocsOfFirestore
(firestore: FirestoreOptions
, ...criterias: QueryConstraint[]): Promise { - const result = await getDocs(query(firestore.collection, ...criterias)); + async function getAllDocsOfFirestore
(firestore: FirestoreOptions
, ...criteria: QueryConstraint[]): Promise { + const result = await getDocs(query(firestore.collection, ...criteria)); return result.docs.map(d => { const docData = d.data(); (docData as any).id = d.id;