From f879a2cdb80af25e5cb92dcb4b65a6de0ef5c4d2 Mon Sep 17 00:00:00 2001 From: Mike D Date: Sat, 1 Jun 2024 14:06:26 -0400 Subject: [PATCH] Basic implementation of security rules for packs. Required locking down puzzle listing and hence some changes to daily mini querying --- app/__tests__/packRules.test.ts | 160 +++++++++++++++++++++++++++ app/firestore.rules | 30 +++-- app/lib/dailyMinis.ts | 12 +- app/lib/dbtypes.ts | 2 + app/lib/pack.ts | 9 ++ app/lib/prefs.ts | 2 + app/lib/serverOnly.ts | 33 +++++- app/pages/dailyminis/[[...slug]].tsx | 3 +- app/pages/index.tsx | 3 +- 9 files changed, 231 insertions(+), 23 deletions(-) create mode 100644 app/__tests__/packRules.test.ts create mode 100644 app/lib/pack.ts diff --git a/app/__tests__/packRules.test.ts b/app/__tests__/packRules.test.ts new file mode 100644 index 00000000..963844ac --- /dev/null +++ b/app/__tests__/packRules.test.ts @@ -0,0 +1,160 @@ +/** + * @jest-environment node + */ + +import { + RulesTestEnvironment, + assertFails, + assertSucceeds, + initializeTestEnvironment, +} from '@firebase/rules-unit-testing'; +import { + collection, + doc, + getDoc, + getDocs, + limit, + query, + setDoc, + updateDoc, + where, +} from 'firebase/firestore'; +import { DBPuzzleT } from '../lib/dbtypes.js'; +import { convertTimestamps } from '../lib/firebaseWrapper.js'; +import { getMockedPuzzle } from '../lib/getMockedPuzzle.js'; + +const packPuzzleId = 'foobar'; +const unpackPuzzleId = 'foobarbaz'; +const puzzle: DBPuzzleT = convertTimestamps( + getMockedPuzzle({ cs: [], pk: 'irving' }) +) as DBPuzzleT; +const unpacked: DBPuzzleT = convertTimestamps( + getMockedPuzzle({ cs: [] }) +) as DBPuzzleT; + +const projectId = 'demo-pack-rules-testing'; +let testEnv: RulesTestEnvironment; + +beforeAll(async () => { + testEnv = await initializeTestEnvironment({ + projectId, + firestore: { host: 'localhost', port: 8080 }, + }); + + await testEnv.withSecurityRulesDisabled(async (ctxt) => { + const firestore = ctxt.firestore(); + await firestore.collection('c').doc(packPuzzleId).set(puzzle); + await firestore.collection('c').doc(unpackPuzzleId).set(unpacked); + await firestore + .collection('prefs') + .doc('buyer') + .set({ packs: ['irving'] }); + await firestore.collection('packs').doc('irving').set({ a: 'packowner' }); + }); +}); + +test('constructor can get pack puzzle', async () => { + const firestore = testEnv + .authenticatedContext('fSEwJorvqOMK5UhNMHa4mu48izl1') + .firestore(); + + await assertSucceeds(getDoc(doc(collection(firestore, 'c'), packPuzzleId))); +}); + +test('rando cannot get pack puzzle', async () => { + const firestore = testEnv.authenticatedContext('rando').firestore(); + + await assertSucceeds(getDoc(doc(collection(firestore, 'c'), unpackPuzzleId))); + await assertFails(getDoc(doc(collection(firestore, 'c'), packPuzzleId))); +}); + +test('buyer can get pack puzzle', async () => { + const firestore = testEnv.authenticatedContext('buyer').firestore(); + + await assertSucceeds(getDoc(doc(collection(firestore, 'c'), packPuzzleId))); +}); + +test('pack owner can get pack puzzle', async () => { + const firestore = testEnv.authenticatedContext('packowner').firestore(); + + await assertSucceeds(getDoc(doc(collection(firestore, 'c'), packPuzzleId))); +}); + +test('cannot list puzzles', async () => { + const firestore = testEnv.authenticatedContext('packowner').firestore(); + + await assertFails(getDocs(query(collection(firestore, 'c'), limit(2)))); +}); + +test('pack owner can list pack puzzles (for review)', async () => { + const firestore = testEnv.authenticatedContext('packowner').firestore(); + + await assertSucceeds( + getDocs(query(collection(firestore, 'c'), where('pk', '==', 'irving'))) + ); +}); + +test('rando cannot list pack puzzles', async () => { + const firestore = testEnv.authenticatedContext('rando').firestore(); + + await assertFails( + getDocs(query(collection(firestore, 'c'), where('pk', '==', 'irving'))) + ); +}); + +test('user can create prefs doc', async () => { + const firestore = testEnv.authenticatedContext('rando').firestore(); + + await assertSucceeds( + setDoc(doc(collection(firestore, 'prefs'), 'rando'), { foo: ['bar'] }) + ); +}); + +test('user cannot create prefs doc with pack', async () => { + const firestore = testEnv.authenticatedContext('rando').firestore(); + + await assertFails( + setDoc(doc(collection(firestore, 'prefs'), 'rando'), { packs: ['bar'] }) + ); +}); + +test('user can update prefs doc', async () => { + const firestore = testEnv.authenticatedContext('rando').firestore(); + + await assertSucceeds( + setDoc(doc(collection(firestore, 'prefs'), 'rando'), { foo: ['bar'] }) + ); + await assertSucceeds( + updateDoc(doc(collection(firestore, 'prefs'), 'rando'), { bam: ['baz'] }) + ); + const res = await getDoc(doc(collection(firestore, 'prefs'), 'rando')); + expect(res.data()).toMatchInlineSnapshot(` + { + "bam": [ + "baz", + ], + "foo": [ + "bar", + ], + } + `); +}); + +test('user cannot update prefs doc with pack', async () => { + const firestore = testEnv.authenticatedContext('rando').firestore(); + + await assertSucceeds( + setDoc(doc(collection(firestore, 'prefs'), 'rando'), { foo: ['bar'] }) + ); + await assertFails( + updateDoc(doc(collection(firestore, 'prefs'), 'rando'), { packs: ['baz'] }) + ); + const res = await getDoc(doc(collection(firestore, 'prefs'), 'rando')); + expect(res.data()).toMatchInlineSnapshot(` + { + "foo": [ + "bar", + ], + } + `); +}); diff --git a/app/firestore.rules b/app/firestore.rules index 54772508..0da556ea 100644 --- a/app/firestore.rules +++ b/app/firestore.rules @@ -56,15 +56,27 @@ service cloud.firestore { allow read: if isAdmin(); allow write: if isAdmin(); } - match /mail/{messageId} { - allow read: if isAdmin(); + match /packs/{packId} { + allow get: if isAdmin() || isAuthor(); allow write: if isAdmin(); } match /c/{crosswordId} { - allow get: if true; + // admin and constructor can always get + allow get: if isAdmin() || isAuthor(); + // anybody with an id can get a non-pack puz + allow get: if !resource.data.keys().hasAny(["pk"]); + // pack owner can get a pack puz + allow get: if request.auth.uid == get(/databases/$(database)/documents/packs/$(resource.data.pk)).data.a; + // anybody who has been granted access to a pack can get a pack puz + allow get: if resource.data.pk in get(/databases/$(database)/documents/prefs/$(request.auth.uid)).data.packs; - allow list: if isAdmin() || isAuthor(); - allow list: if request.query.limit <= 30; + // to support moderation queue on admin page + allow list: if isAdmin(); + // to support puzzle list on dashboard + allow list: if isAuthor() && request.query.limit <= 20; + // to support pack owner listing puzzles that have been added to the pack + allow list: if resource.data.keys().hasAny(['pk']) + && request.auth.uid == get(/databases/$(database)/documents/packs/$(resource.data.pk)).data.a; allow update: if isAdmin(); @@ -106,7 +118,9 @@ service cloud.firestore { } match /prefs/{userId} { allow read: if request.auth.uid == userId; - allow write: if isAdmin() || request.auth.uid == userId; + allow write: if isAdmin() + allow write: if request.auth.uid == userId && + !request.resource.data.keys().hasAny(["packs"]); } match /cs/{userId} { allow read: if request.auth.uid == userId || isAdmin(); @@ -150,10 +164,6 @@ service cloud.firestore { match /ds/{dateString} { allow read: if isAdmin(); } - match /categories/{category} { - allow get: if true; - allow write: if isAdmin(); - } match /cfm/{commentId} { allow write: if isAdmin(); allow write: if request.auth.uid != null diff --git a/app/lib/dailyMinis.ts b/app/lib/dailyMinis.ts index c22e64be..79f72ca1 100644 --- a/app/lib/dailyMinis.ts +++ b/app/lib/dailyMinis.ts @@ -29,7 +29,7 @@ export async function getMiniIdForDate(d: Date): Promise { if (existing === null) { return null; } - const puz = await getMiniForDate(d, true); + const puz = await getMiniForDate(d); if (puz === null) { dailyMiniIdsByDate.set(key, null); return null; @@ -39,9 +39,10 @@ export async function getMiniIdForDate(d: Date): Promise { return puz.id; } -export async function getMiniForDate( - d: Date, - allowMissing?: boolean +/* This gets used client side for loading minis to show in the UpcomingMinisCalendar component. + * The similarly named function in serverOnly.ts should only be used server side. */ +async function getMiniForDate( + d: Date ): Promise<(DBPuzzleT & { id: string }) | null> { const dbres = await getDocs( query( @@ -53,9 +54,6 @@ export async function getMiniForDate( const doc = dbres.docs[0]; if (!doc) { - if (!allowMissing) { - console.error('no dm for date ', d); - } return null; } const validationResult = DBPuzzleV.decode(doc.data()); diff --git a/app/lib/dbtypes.ts b/app/lib/dbtypes.ts index f49d77bc..470f524d 100644 --- a/app/lib/dbtypes.ts +++ b/app/lib/dbtypes.ts @@ -238,6 +238,8 @@ const DBPuzzleOptionalV = t.partial({ lk: t.array(t.string), /** ready for moderation (has necessary likes/comments/etc) */ rfm: t.boolean, + /** pack id */ + pk: t.string, }); export const DBPuzzleV = t.intersection([ diff --git a/app/lib/pack.ts b/app/lib/pack.ts new file mode 100644 index 00000000..f5d8b3bc --- /dev/null +++ b/app/lib/pack.ts @@ -0,0 +1,9 @@ +import * as t from 'io-ts'; + +export const PackV = t.intersection([ + t.type({ + /** pack owner */ + a: t.string, + }), + t.partial({}), +]); diff --git a/app/lib/prefs.ts b/app/lib/prefs.ts index 71c58edd..66bd3e7a 100644 --- a/app/lib/prefs.ts +++ b/app/lib/prefs.ts @@ -28,6 +28,8 @@ export const AccountPrefsV = t.intersection([ following: t.array(t.string), rtg: GlickoScoreV, rtgs: t.array(GlickoScoreV), + /** Packs the user owns */ + packs: t.array(t.string), }), ]); export type AccountPrefsT = t.TypeOf; diff --git a/app/lib/serverOnly.ts b/app/lib/serverOnly.ts index cb8bd954..8a8e96df 100644 --- a/app/lib/serverOnly.ts +++ b/app/lib/serverOnly.ts @@ -19,8 +19,13 @@ import { ConstructorPageV, ConstructorPageWithMarkdown, } from './constructorPage.js'; -import { getMiniForDate } from './dailyMinis.js'; -import { CommentWithRepliesT, DBPuzzleV } from './dbtypes.js'; +import { + CommentWithRepliesT, + DBPuzzleT, + DBPuzzleV, + getDateString, + prettifyDateString, +} from './dbtypes.js'; import { EmbedOptionsT, validate as validateEmbedOptions, @@ -414,3 +419,27 @@ export const getEmbedProps = async ({ } return embedOptions; }; + +/* Get a daily mini using the firebase-admin library. + * The similarly named function in dailyMinis.ts gets used client-side only. */ +export async function getMiniForDate( + d: Date +): Promise<(DBPuzzleT & { id: string }) | null> { + const dbres = await getCollection('c') + .where('dmd', '==', prettifyDateString(getDateString(d))) + .limit(1) + .get(); + + const doc = dbres.docs[0]; + if (!doc) { + console.error('no dm for date ', d); + return null; + } + const validationResult = DBPuzzleV.decode(doc.data()); + if (validationResult._tag === 'Right') { + return { ...validationResult.right, id: doc.id }; + } + console.error('invalid puzzle ', doc.id); + console.error(PathReporter.report(validationResult).join(',')); + return null; +} diff --git a/app/pages/dailyminis/[[...slug]].tsx b/app/pages/dailyminis/[[...slug]].tsx index e60a42f1..f9121d80 100644 --- a/app/pages/dailyminis/[[...slug]].tsx +++ b/app/pages/dailyminis/[[...slug]].tsx @@ -12,9 +12,8 @@ import { } from '../../components/PuzzleLink.js'; import { DefaultTopBar } from '../../components/TopBar.js'; import { ConstructorPageBase } from '../../lib/constructorPage.js'; -import { getMiniForDate } from '../../lib/dailyMinis.js'; import { isUserPatron } from '../../lib/patron.js'; -import { userIdToPage } from '../../lib/serverOnly.js'; +import { getMiniForDate, userIdToPage } from '../../lib/serverOnly.js'; import { withTranslation } from '../../lib/translation.js'; import { puzzleFromDB } from '../../lib/types.js'; import { notEmpty } from '../../lib/utils.js'; diff --git a/app/pages/index.tsx b/app/pages/index.tsx index 0f043250..c1f047ec 100644 --- a/app/pages/index.tsx +++ b/app/pages/index.tsx @@ -19,12 +19,11 @@ import { DefaultTopBar } from '../components/TopBar.js'; import { UnfinishedPuzzleList } from '../components/UnfinishedPuzzleList.js'; import { ArticleT, validate } from '../lib/article.js'; import { ConstructorPageBase } from '../lib/constructorPage.js'; -import { getMiniForDate } from '../lib/dailyMinis.js'; import { getCollection } from '../lib/firebaseAdminWrapper.js'; import { markdownToHast } from '../lib/markdown/markdown.js'; import { paginatedPuzzles } from '../lib/paginatedPuzzles.js'; import { isUserPatron } from '../lib/patron.js'; -import { userIdToPage } from '../lib/serverOnly.js'; +import { getMiniForDate, userIdToPage } from '../lib/serverOnly.js'; import { withTranslation } from '../lib/translation.js'; import { puzzleFromDB } from '../lib/types.js'; import { PAGE_SIZE } from './featured/[pageNumber].js';