Skip to content

Commit

Permalink
Basic implementation of security rules for packs. Required locking do…
Browse files Browse the repository at this point in the history
…wn puzzle listing and hence some changes to daily mini querying
  • Loading branch information
mdirolf committed Jun 1, 2024
1 parent 96a81cb commit f879a2c
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 23 deletions.
160 changes: 160 additions & 0 deletions app/__tests__/packRules.test.ts
Original file line number Diff line number Diff line change
@@ -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",
],
}
`);
});
30 changes: 20 additions & 10 deletions app/firestore.rules
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
12 changes: 5 additions & 7 deletions app/lib/dailyMinis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export async function getMiniIdForDate(d: Date): Promise<string | null> {
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;
Expand All @@ -39,9 +39,10 @@ export async function getMiniIdForDate(d: Date): Promise<string | null> {
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(
Expand All @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions app/lib/dbtypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
9 changes: 9 additions & 0 deletions app/lib/pack.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as t from 'io-ts';

export const PackV = t.intersection([
t.type({
/** pack owner */
a: t.string,
}),
t.partial({}),
]);
2 changes: 2 additions & 0 deletions app/lib/prefs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof AccountPrefsV>;
33 changes: 31 additions & 2 deletions app/lib/serverOnly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
3 changes: 1 addition & 2 deletions app/pages/dailyminis/[[...slug]].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
3 changes: 1 addition & 2 deletions app/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down

0 comments on commit f879a2c

Please sign in to comment.