From 3ef3d291fcd64b1a9f78d364bcd7105fc92d6618 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Oct 2025 20:40:53 +0000 Subject: [PATCH 1/2] Initial plan From 8ea17702015112c94583ffaf4d59f6a9ba68ab7b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Oct 2025 20:53:33 +0000 Subject: [PATCH 2/2] Fix: Disallow read access to nested user collections for disabled users Co-authored-by: pauljohanneskraft <15239005+pauljohanneskraft@users.noreply.github.com> --- firestore.rules | 2 +- .../src/tests/rules/userCollections.test.ts | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/firestore.rules b/firestore.rules index e7ad3390..10d50883 100644 --- a/firestore.rules +++ b/firestore.rules @@ -163,7 +163,7 @@ service cloud.firestore { } allow read: if isAdmin() - || (request.auth != null && request.auth.uid == userId) + || isUser(userId) || isOwnerOrClinicianOfSameOrganization(); allow write: if isAdmin() diff --git a/functions/src/tests/rules/userCollections.test.ts b/functions/src/tests/rules/userCollections.test.ts index 99bbb58f..cdc5dc4f 100644 --- a/functions/src/tests/rules/userCollections.test.ts +++ b/functions/src/tests/rules/userCollections.test.ts @@ -34,6 +34,7 @@ describe("firestore.rules: users/{userId}/{collectionName}/{documentId}", () => const clinicianId = "mockClinician"; const patientId = "mockPatient"; const userId = "mockUser"; + const disabledUserId = "disabledMockUser"; let testEnvironment: RulesTestEnvironment; let adminFirestore: firebase.firestore.Firestore; @@ -41,6 +42,7 @@ describe("firestore.rules: users/{userId}/{collectionName}/{documentId}", () => let clinicianFirestore: firebase.firestore.Firestore; let patientFirestore: firebase.firestore.Firestore; let userFirestore: firebase.firestore.Firestore; + let disabledUserFirestore: firebase.firestore.Firestore; beforeAll(async () => { testEnvironment = await initializeTestEnvironment({ @@ -80,6 +82,14 @@ describe("firestore.rules: users/{userId}/{collectionName}/{documentId}", () => userFirestore = testEnvironment .authenticatedContext(userId, {}) .firestore(); + + disabledUserFirestore = testEnvironment + .authenticatedContext(disabledUserId, { + type: UserType.patient, + organization: organizationId, + disabled: true, + }) + .firestore(); }); beforeEach(async () => { @@ -99,6 +109,11 @@ describe("firestore.rules: users/{userId}/{collectionName}/{documentId}", () => .doc(`users/${patientId}`) .set({ type: UserType.patient, organization: organizationId }); await firestore.doc(`users/${userId}`).set({}); + await firestore.doc(`users/${disabledUserId}`).set({ + type: UserType.patient, + organization: organizationId, + disabled: true, + }); }); }); @@ -143,4 +158,18 @@ describe("firestore.rules: users/{userId}/{collectionName}/{documentId}", () => await assertFails(userFirestore.collection(patientPath).get()); await assertSucceeds(userFirestore.collection(userPath).get()); }); + + it("disabled users cannot access their own nested collections", async () => { + const disabledUserPath = `users/${disabledUserId}/allergyIntolerances`; + + // Disabled users should not be able to read their own nested collections + await assertFails(disabledUserFirestore.collection(disabledUserPath).get()); + + // Admin can still access disabled user's collections + await assertSucceeds(adminFirestore.collection(disabledUserPath).get()); + + // Owners and clinicians can still access disabled user's collections + await assertSucceeds(ownerFirestore.collection(disabledUserPath).get()); + await assertSucceeds(clinicianFirestore.collection(disabledUserPath).get()); + }); });