diff --git a/docs/officers/EVENTS_SHOP_MEMBERS.md b/docs/officers/EVENTS_SHOP_MEMBERS.md index 1423cc48..f220cf39 100644 --- a/docs/officers/EVENTS_SHOP_MEMBERS.md +++ b/docs/officers/EVENTS_SHOP_MEMBERS.md @@ -59,6 +59,33 @@ If any proof is missing, report the change as **not live**. Admin event and product editors exist in source, but their live permissions, backup, preview, and rollback behavior have not been approved. Saving can write directly to production Firestore. Officers must not use these screens as a continuity procedure yet. +### Source protection in #100 — NOT LIVE YET + +Issue [#100](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/100) narrows the source rules for these screens. It does not prove that Firebase received those rules. + +After that source change merges: + +- A browser can create only an inactive event or product draft with no live price, capacity, sale, registration, waiver, volunteer, or custom-field setup. +- A browser can edit ordinary display text and approved HTTPS image/result links. +- A browser cannot change event price, capacity, registration state, member visibility, waiver setup, volunteer setup, or collected registration fields. +- A browser cannot change product price, sale status, sizes, colors, inventory, orders, or payment state. +- A browser admin cannot directly change a member role or read stored connection secrets. + +Those protected changes need a small, reviewed server action. That action is **NOT AVAILABLE YET**. Until it exists and is tested, use [Request a change](./REQUEST_A_CHANGE.md). + +```mermaid +flowchart LR + A["Officer browser"] --> B["Display-only draft details"] + B --> C["Firestore rules"] + C --> D["Event or product draft"] + A -. "blocked" .-> E["Price, capacity, access, waiver, sale, or payment state"] + F["Scoped server action — NOT AVAILABLE YET"] -. "future approved path" .-> E +``` + +Text alternative: the officer browser may send display-only draft details through Firestore rules. Operational, access, legal, and money fields stay blocked until a future approved server action exists. + +**Proof state:** source and emulator tests may pass in #100. Firebase deployment, the live rule version, the Admin screens, and production behavior remain unproven until #105 records each state separately. + Before an officer click guide may be added, a claimed issue must prove all of the following with made-up data in an isolated staging project: 1. Only the intended officer role can open and save each screen. diff --git a/firestore.rules b/firestore.rules index 9a93446c..d4c256d3 100644 --- a/firestore.rules +++ b/firestore.rules @@ -3,41 +3,286 @@ rules_version = '2'; service cloud.firestore { match /databases/{database}/documents { - // --------------------------------------------------------------- - // Admin has full access to everything via this catch-all rule. - // Specific allow rules below EXTEND access for other roles; - // they do not restrict admin access (Firestore rules are additive). - // --------------------------------------------------------------- - match /{document=**} { - allow read, write: if request.auth != null && request.auth.token.role == 'admin'; + function isAdmin() { + return request.auth != null && request.auth.token.role == 'admin'; + } + + function isNonEmptyString(value, maxLength) { + return value is string && value.size() > 0 && value.size() <= maxLength; + } + + function isBoundedString(value, maxLength) { + return value is string && value.size() <= maxLength; + } + + function isNullableBoundedString(value, maxLength) { + return value == null || isBoundedString(value, maxLength); + } + + function isNullableHttpsUrl(value) { + return value == null + || (isBoundedString(value, 2048) + && value.matches('https://[^ ]+')); + } + + function isValidSlug(value) { + return isNonEmptyString(value, 200) + && value.matches('[a-z0-9-]+'); + } + + function isNullableTimestamp(value) { + return value == null || value is timestamp; + } + + function isNonNegativeInteger(value) { + return value is int && value >= 0; + } + + function isPublishedEventStatus(data) { + return 'status' in data + && data.status in ['open', 'closed', 'cancelled']; + } + + function hasLegacyPublishedStatus(data) { + return !('status' in data) || isPublishedEventStatus(data); + } + + function hasValidEventCreateFields(data) { + return data.keys().hasAll([ + 'slug', + 'title', + 'description', + 'startAt', + 'endAt', + 'location', + 'locationDetails', + 'capacity', + 'registeredCount', + 'status', + 'visibility', + 'pricing', + 'stripePriceIds', + 'waiverText', + 'waiverVersion', + 'customFields', + 'resultsUrl', + 'resultsText', + 'resultsPublishedAt', + 'registrationOpensAt', + 'registrationClosesAt', + 'heroImageUrl', + 'createdBy', + 'createdAt', + 'updatedAt' + ]) && data.keys().hasOnly([ + 'slug', + 'title', + 'description', + 'startAt', + 'endAt', + 'location', + 'locationDetails', + 'capacity', + 'registeredCount', + 'status', + 'visibility', + 'pricing', + 'stripePriceIds', + 'waiverText', + 'waiverVersion', + 'customFields', + 'volunteerEnabled', + 'volunteerFields', + 'resultsUrl', + 'resultsText', + 'resultsPublishedAt', + 'registrationOpensAt', + 'registrationClosesAt', + 'heroImageUrl', + 'createdBy', + 'createdAt', + 'updatedAt' + ]); + } + + function hasOnlyEventUpdateFields() { + return request.resource.data.diff(resource.data).affectedKeys().hasOnly([ + 'title', + 'description', + 'startAt', + 'endAt', + 'location', + 'locationDetails', + 'resultsUrl', + 'resultsText', + 'resultsPublishedAt', + 'heroImageUrl', + 'updatedAt' + ]); + } + + function hasValidEventPricing(pricing) { + return pricing is map + && pricing.keys().hasAll(['memberCents', 'nonMemberCents']) + && pricing.keys().hasOnly([ + 'memberCents', + 'nonMemberCents', + 'earlyBirdCents', + 'earlyBirdUntil' + ]) + && isNonNegativeInteger(pricing.memberCents) + && isNonNegativeInteger(pricing.nonMemberCents) + && (!('earlyBirdCents' in pricing) + || isNonNegativeInteger(pricing.earlyBirdCents)) + && (!('earlyBirdUntil' in pricing) + || isNullableTimestamp(pricing.earlyBirdUntil)); + } + + function hasInertEventOperationalConfig(data) { + return data.capacity == null + && data.status == 'draft' + // The current client submits visibility=public on a new form. Draft + // status keeps it unreadable and unregistrable; visibility is then + // immutable in browser updates until a server approves publication. + && data.visibility == 'public' + && data.pricing is map + && data.pricing.keys().hasAll(['memberCents', 'nonMemberCents']) + && data.pricing.keys().hasOnly(['memberCents', 'nonMemberCents']) + && data.pricing.memberCents == 0 + && data.pricing.nonMemberCents == 0 + // Volunteer signup currently takes a zero-charge comp path. Browser + // creation therefore starts with volunteering disabled, and the update + // allowlist keeps these values server-owned. + && data.volunteerEnabled == false + && data.volunteerFields is list + && data.volunteerFields.size() == 0 + && data.customFields is list + && data.customFields.size() == 0 + && data.waiverText == '' + && data.waiverVersion == '1' + && data.registrationOpensAt == null + && data.registrationClosesAt == null; + } + + function hasValidEventEditorValues(data) { + return isNonEmptyString(data.title, 200) + && isBoundedString(data.description, 20000) + && data.startAt is timestamp + && isNullableTimestamp(data.endAt) + && isBoundedString(data.location, 500) + && isBoundedString(data.locationDetails, 2000) + // `null` is the only unlimited-capacity sentinel. Zero would be + // falsey in the current checkout code and could silently disable the + // capacity check, so every configured capacity must be positive. + && (data.capacity == null + || (isNonNegativeInteger(data.capacity) && data.capacity > 0)) + && data.status in ['draft', 'open', 'closed', 'cancelled'] + && data.visibility in ['public', 'members_only', 'draft'] + && hasValidEventPricing(data.pricing) + && isBoundedString(data.waiverText, 50000) + && isNonEmptyString(data.waiverVersion, 200) + && data.customFields is list + && data.customFields.size() <= 50 + && data.volunteerEnabled is bool + && data.volunteerFields is list + && data.volunteerFields.size() <= 50 + && isNullableHttpsUrl(data.resultsUrl) + && isNullableBoundedString(data.resultsText, 10000) + && (!('resultsPublishedAt' in data) + || isNullableTimestamp(data.resultsPublishedAt)) + && isNullableTimestamp(data.registrationOpensAt) + && isNullableTimestamp(data.registrationClosesAt) + && isNullableHttpsUrl(data.heroImageUrl) + && data.updatedAt is timestamp; + } + + function hasValidProductCreateFields(data) { + return data.keys().hasAll([ + 'slug', + 'title', + 'description', + 'priceCents', + 'imageUrl', + 'sizes', + 'colors', + 'status', + 'createdBy', + 'createdAt', + 'updatedAt' + ]) && data.keys().hasOnly([ + 'slug', + 'title', + 'description', + 'priceCents', + 'imageUrl', + 'sizes', + 'colors', + 'status', + 'createdBy', + 'createdAt', + 'updatedAt' + ]); + } + + function hasOnlyProductUpdateFields() { + return request.resource.data.diff(resource.data).affectedKeys().hasOnly([ + 'title', + 'description', + 'imageUrl', + 'updatedAt' + ]); + } + + function hasValidProductEditorValues(data) { + return isNonEmptyString(data.title, 200) + && isBoundedString(data.description, 20000) + && isNonNegativeInteger(data.priceCents) + && isNullableHttpsUrl(data.imageUrl) + && data.sizes is list + && data.sizes.size() <= 100 + && data.colors is list + && data.colors.size() <= 100 + && data.status in ['draft', 'active', 'sold_out', 'archived'] + // Active products reach Stripe Checkout and therefore cannot use the + // zero-price draft sentinel. + && (data.status != 'active' || data.priceCents > 0) + && data.updatedAt is timestamp; } // Members-only legacy collection match /members_only/{document} { - allow read: if request.auth != null && request.auth.token.role == 'member'; + allow read: if request.auth != null + && (request.auth.token.role == 'member' || isAdmin()); } // A signed-in user can read and update their own member doc. // Member docs are CREATED only by the onSignUp Cloud Function (Admin SDK); // there is intentionally no client `create` rule. match /members/{uid} { - allow read: if request.auth != null && request.auth.uid == uid; + // Admin screens may enumerate profiles, but role and profile creation + // remain server-managed. In particular, an admin claim in a browser is + // not authority to change another user's role mirror. + allow read: if request.auth != null + && (request.auth.uid == uid || isAdmin()); // Self-service edits are restricted to an explicit field allowlist. // An allowlist (rather than pinning each protected field individually) // blocks BOTH tampering with managed fields — role, email, createdAt, // emailVerified, provider — AND injection of arbitrary new fields (e.g. // a planted `isAdmin` flag or a multi-MB blob). Managed fields are set - // only by Cloud Functions (Admin SDK) or by admins via the catch-all. + // only by Cloud Functions (Admin SDK). allow update: if request.auth != null && request.auth.uid == uid && request.resource.data.diff(resource.data).affectedKeys() .hasOnly(['fullName', 'phoneNumber', 'updatedAt']) + && request.resource.data.diff(resource.data).affectedKeys() + .hasAll(['updatedAt']) && (request.resource.data.fullName == null || (request.resource.data.fullName is string && request.resource.data.fullName.size() <= 200)) && request.resource.data.phoneNumber is string - && request.resource.data.phoneNumber.size() <= 40; + && request.resource.data.phoneNumber.size() <= 40 + && request.resource.data.updatedAt is timestamp; // Third-party connections (Strava, etc). Users can read *whether* // a connection exists and basic metadata, but CANNOT read tokens — @@ -63,36 +308,70 @@ service cloud.firestore { // - status: 'draft' | 'open' | 'closed' | 'cancelled' // - member_only: legacy boolean (pre-stripe schema) // - // Writes flow only through admin (catch-all) or Cloud Functions - // (Admin SDK bypasses rules). No direct client writes. + // The current admin editor reads and writes non-financial event catalog + // fields directly. Price and capacity are fixed to inert values on browser + // creation and immutable from browser updates. Registration, payment, and + // capacity configuration remains server-owned. // --------------------------------------------------------------- match /events/{eventId} { + allow read: if isAdmin(); + + // The editor initializes server-owned fields only to inert values. Any + // Stripe reference, payment/audit field, non-zero counter, or unknown + // field must be created by trusted server code instead. + allow create: if isAdmin() + && hasValidEventCreateFields(request.resource.data) + && isValidSlug(request.resource.data.slug) + && request.resource.data.slug == eventId + && request.resource.data.createdBy == request.auth.uid + && request.resource.data.createdAt is timestamp + && request.resource.data.registeredCount is int + && request.resource.data.registeredCount == 0 + && request.resource.data.stripePriceIds is map + && request.resource.data.stripePriceIds.keys().hasOnly([]) + && hasInertEventOperationalConfig(request.resource.data) + && hasValidEventEditorValues(request.resource.data); + + // Updates are limited to display-only editor fields. In particular, + // slug/ownership, status/visibility, pricing/capacity, registration and + // waiver configuration, Stripe references, payment/inventory state, and + // audit data are immutable from a browser. Delete is intentionally absent. + allow update: if isAdmin() + && hasOnlyEventUpdateFields() + && hasValidEventEditorValues(request.resource.data); + // Public read: new-schema event with public visibility and not draft allow read: if resource.data.visibility == 'public' - && resource.data.status != 'draft'; + && isPublishedEventStatus(resource.data); // Public read: legacy event (no visibility field) not flagged member_only allow read: if !('visibility' in resource.data) - && resource.data.member_only != true; + && resource.data.member_only != true + && hasLegacyPublishedStatus(resource.data); // Authenticated member/admin read: members-only events allow read: if request.auth != null && (request.auth.token.role == 'member' || request.auth.token.role == 'admin') - && (resource.data.visibility == 'members_only' || resource.data.member_only == true); + && ((resource.data.visibility == 'members_only' + && isPublishedEventStatus(resource.data)) + || (!('visibility' in resource.data) + && resource.data.member_only == true + && hasLegacyPublishedStatus(resource.data))); - // Registrations subcollection — clients CANNOT read or write directly. - // Creation, updates, refunds, and substitutions all go through - // Cloud Functions which use the Admin SDK. Admins still get access - // via the top-level catch-all rule. + // Registrations subcollection — browser admins may read rosters for the + // current admin UI, but no client may write lifecycle or financial + // fields directly. Creation, updates, refunds, and substitutions all go + // through Cloud Functions which use the Admin SDK. match /registrations/{regId} { - allow read, write: if false; + allow read: if isAdmin(); + allow write: if false; } } // --------------------------------------------------------------- - // Promo codes — admin-only via catch-all. Clients may not read, - // even to validate: validation happens in the Cloud Function - // that creates the Stripe Checkout Session. + // Promo codes — server-only. Clients may not read or mutate discount + // authority; validation happens in the Cloud Function that creates the + // Stripe Checkout Session. // --------------------------------------------------------------- match /promoCodes/{codeId} { allow read, write: if false; @@ -107,17 +386,68 @@ service cloud.firestore { } // --------------------------------------------------------------- - // Shop products — publicly readable when active or sold_out. - // Drafts and archived are hidden from clients. Writes via admin. + // Shop products — publicly readable when active or sold_out. The current + // admin editor may manage non-financial product catalog fields directly. + // Browser creation is an inert zero-price draft and browser updates cannot + // change price. Drafts and archived products remain hidden from others. // --------------------------------------------------------------- match /products/{productId} { - allow read: if resource.data.status == 'active' - || resource.data.status == 'sold_out'; + allow read: if isAdmin(); + + allow create: if isAdmin() + && hasValidProductCreateFields(request.resource.data) + && isValidSlug(request.resource.data.slug) + && request.resource.data.slug == productId + && request.resource.data.createdBy == request.auth.uid + && request.resource.data.createdAt is timestamp + && request.resource.data.status == 'draft' + && request.resource.data.priceCents == 0 + && request.resource.data.sizes is list + && request.resource.data.sizes.size() == 0 + && request.resource.data.colors is list + && request.resource.data.colors.size() == 0 + && hasValidProductEditorValues(request.resource.data); + + // Price, availability status, variants, Stripe references, inventory, + // payment fields, and audit data are not browser editor fields. Delete + // is intentionally absent. + allow update: if isAdmin() + && hasOnlyProductUpdateFields() + && hasValidProductEditorValues(request.resource.data); + + allow read: if (resource.data.status == 'active' + || resource.data.status == 'sold_out'); } - // Orders — client-inaccessible. All reads/writes via Cloud Functions. - // Admin gets access via the top-level catch-all. + // Orders contain financial state and PII. Browser admins may read them for + // the current admin UI; every mutation goes through Cloud Functions. match /orders/{orderId} { + allow read: if isAdmin(); + allow write: if false; + } + + // --------------------------------------------------------------- + // Server-owned operational boundaries. Explicit deny rules document the + // known sensitive collections; unmatched/future collections are denied by + // default because there is intentionally no recursive admin catch-all. + // --------------------------------------------------------------- + match /mail/{messageId} { + allow read, write: if false; + } + + match /stripeEvents/{stripeEventId} { + allow read, write: if false; + } + + match /checkoutRequests/{requestId} { + allow read, write: if false; + } + + match /auditEvents/{auditEventId} { + allow read, write: if false; + } + + match /retentionJobs/{jobId} { allow read, write: if false; } } diff --git a/tests/firestore-rules/events.test.js b/tests/firestore-rules/events.test.js index 7a2f287e..b52b1e9e 100644 --- a/tests/firestore-rules/events.test.js +++ b/tests/firestore-rules/events.test.js @@ -1,14 +1,19 @@ +/* eslint-env jest */ + const { clearFirestore, teardown, db, seed, assertFails, assertSucceeds, } = require('./setup'); const NOW = new Date(); +const UNSAFE_SCRIPT_URL = ['javascript', 'alert(1)'].join(':'); const PUBLIC_OPEN = { visibility: 'public', status: 'open', title: 'Public Open Race', startAt: NOW, + capacity: null, + pricing: { memberCents: 0, nonMemberCents: 0 }, }; const PUBLIC_DRAFT = { @@ -16,6 +21,8 @@ const PUBLIC_DRAFT = { status: 'draft', title: 'Draft', startAt: NOW, + capacity: null, + pricing: { memberCents: 0, nonMemberCents: 0 }, }; const MEMBERS_ONLY = { @@ -23,6 +30,14 @@ const MEMBERS_ONLY = { status: 'open', title: 'Members Run', startAt: NOW, + capacity: null, + pricing: { memberCents: 0, nonMemberCents: 0 }, +}; + +const MEMBERS_ONLY_DRAFT = { + ...MEMBERS_ONLY, + status: 'draft', + title: 'Unpublished member draft', }; const LEGACY_OPEN = { @@ -38,6 +53,95 @@ const LEGACY_MEMBERS_ONLY = { startAt: NOW, }; +const LEGACY_MEMBERS_ONLY_DRAFT = { + ...LEGACY_MEMBERS_ONLY, + status: 'draft', + title: 'Legacy unpublished member draft', +}; + +const LEGACY_DRAFT = { + // no visibility field, but a draft status must never be public + member_only: false, + status: 'draft', + title: 'Legacy draft', + startAt: NOW, +}; + +function adminEvent(slug, overrides = {}) { + return { + slug, + title: 'Admin Event', + description: 'Event description', + startAt: NOW, + endAt: null, + location: 'Clubhouse', + locationDetails: '', + capacity: null, + registeredCount: 0, + status: 'draft', + visibility: 'public', + pricing: { + memberCents: 0, + nonMemberCents: 0, + }, + stripePriceIds: {}, + waiverText: '', + waiverVersion: '1', + customFields: [], + volunteerEnabled: false, + volunteerFields: [], + resultsUrl: null, + resultsText: null, + resultsPublishedAt: null, + registrationOpensAt: null, + registrationClosesAt: null, + heroImageUrl: null, + createdBy: 'a1', + createdAt: NOW, + updatedAt: NOW, + ...overrides, + }; +} + +function editorUpdate(overrides = {}) { + return { + title: 'Renamed Event', + description: 'Updated description', + startAt: new Date(NOW.getTime() + 1000), + endAt: new Date(NOW.getTime() + 7200000), + location: 'Track', + locationDetails: 'Lane one', + capacity: null, + status: 'draft', + visibility: 'public', + pricing: { + memberCents: 0, + nonMemberCents: 0, + }, + waiverText: '', + waiverVersion: '1', + customFields: [], + volunteerEnabled: false, + volunteerFields: [], + resultsUrl: 'https://example.com/results', + resultsText: 'Official results', + resultsPublishedAt: NOW, + registrationOpensAt: null, + registrationClosesAt: null, + heroImageUrl: 'https://example.com/hero.jpg', + updatedAt: NOW, + ...overrides, + }; +} + +function withoutField(value, field) { + const copy = { ...value }; + delete copy[field]; + return copy; +} + +const INVALID_SLUGS = ['Bad-Slug', 'bad slug', 'bad?query', 'résumé']; + beforeEach(clearFirestore); afterAll(teardown); @@ -67,7 +171,45 @@ describe('events collection', () => { await assertSucceeds(member.doc('events/e1').get()); }); - test('admin CAN read draft event (via catch-all)', async () => { + test('member CANNOT read a new-schema members_only draft', async () => { + await seed('events/e1', MEMBERS_ONLY_DRAFT); + const member = await db({ uid: 'u1', role: 'member' }); + await assertFails(member.doc('events/e1').get()); + }); + + test('member CANNOT read a legacy members-only draft', async () => { + await seed('events/e1', LEGACY_MEMBERS_ONLY_DRAFT); + const member = await db({ uid: 'u1', role: 'member' }); + await assertFails(member.doc('events/e1').get()); + }); + + test('unknown event status is not treated as public or published member content', async () => { + await seed('events/public-unknown', { ...PUBLIC_OPEN, status: 'unexpected' }); + await seed('events/member-unknown', { ...MEMBERS_ONLY, status: 'unexpected' }); + await seed('events/legacy-unknown', { + ...LEGACY_MEMBERS_ONLY, + status: 'unexpected', + }); + const anon = await db(); + const member = await db({ uid: 'u1', role: 'member' }); + + await assertFails(anon.doc('events/public-unknown').get()); + await assertFails(member.doc('events/member-unknown').get()); + await assertFails(member.doc('events/legacy-unknown').get()); + }); + + test('visibility=draft remains admin-only even with an open status', async () => { + await seed('events/e1', { ...PUBLIC_OPEN, visibility: 'draft' }); + const anon = await db(); + const member = await db({ uid: 'u1', role: 'member' }); + const admin = await db({ uid: 'a1', role: 'admin' }); + + await assertFails(anon.doc('events/e1').get()); + await assertFails(member.doc('events/e1').get()); + await assertSucceeds(admin.doc('events/e1').get()); + }); + + test('admin CAN read draft event through the explicit events rule', async () => { await seed('events/e1', PUBLIC_DRAFT); const admin = await db({ uid: 'a1', role: 'admin' }); await assertSucceeds(admin.doc('events/e1').get()); @@ -85,6 +227,18 @@ describe('events collection', () => { await assertFails(anon.doc('events/e1').get()); }); + test('anonymous CANNOT read a legacy draft event', async () => { + await seed('events/e1', LEGACY_DRAFT); + const anon = await db(); + await assertFails(anon.doc('events/e1').get()); + }); + + test('admin CAN read a legacy draft event', async () => { + await seed('events/e1', LEGACY_DRAFT); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.doc('events/e1').get()); + }); + test('member CAN read legacy member_only=true event', async () => { await seed('events/e1', LEGACY_MEMBERS_ONLY); const member = await db({ uid: 'u1', role: 'member' }); @@ -134,6 +288,11 @@ describe('events collection', () => { .get(), ); }); + + test('admin CAN list all events for the current admin screen', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.collection('events').get()); + }); }); describe('writes', () => { @@ -154,19 +313,245 @@ describe('events collection', () => { test('admin CAN create an event', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('events/new').set(PUBLIC_OPEN)); + await assertSucceeds(admin.doc('events/new').set(adminEvent('new'))); }); - test('admin CAN update an event', async () => { - await seed('events/e1', PUBLIC_OPEN); + test('admin CAN create the current client payload only as a hidden inert draft', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('events/e1').update({ title: 'Renamed' })); + await assertSucceeds(admin.doc('events/free').set(adminEvent('free', { + capacity: null, + pricing: { memberCents: 0, nonMemberCents: 0 }, + }))); }); - test('admin CAN delete an event', async () => { - await seed('events/e1', PUBLIC_OPEN); + test.each(INVALID_SLUGS)('admin CANNOT create an event with invalid slug %s', async (slug) => { const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('events/e1').delete()); + await assertFails(admin.doc(`events/${slug}`).set(adminEvent(slug))); + }); + + test('admin CAN update an event with the current editor payload', async () => { + await seed('events/e1', adminEvent('e1')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.doc('events/e1').update(editorUpdate())); + }); + + test('admin CAN edit content while preserving server-owned price and capacity', async () => { + const pricing = { memberCents: 2500, nonMemberCents: 3500 }; + const registrationOpensAt = NOW; + const registrationClosesAt = new Date(NOW.getTime() + 86400000); + const customFields = [{ + key: 'pace', label: 'Pace', type: 'text', required: false, + }]; + const volunteerFields = [{ + key: 'role', label: 'Role', type: 'text', required: true, + }]; + const operational = { + capacity: 50, + pricing, + status: 'open', + visibility: 'members_only', + waiverText: 'Approved synthetic waiver', + waiverVersion: 'approved-v1', + customFields, + volunteerEnabled: true, + volunteerFields, + registrationOpensAt, + registrationClosesAt, + }; + await seed('events/e1', adminEvent('e1', operational)); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.doc('events/e1').update(editorUpdate({ + ...operational, + }))); + }); + + test('admin CANNOT delete an event directly', async () => { + await seed('events/e1', adminEvent('e1')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('events/e1').delete()); + }); + + test.each([ + ['a user without a role claim', { uid: 'u1' }], + ['an unverified user', { uid: 'u2', role: 'unverified' }], + ['a member', { uid: 'u3', role: 'member' }], + ])('%s CANNOT update or delete an event', async (_label, auth) => { + await seed('events/e1', adminEvent('e1')); + const user = await db(auth); + await assertFails(user.doc('events/e1').update({ title: 'Unauthorized' })); + await assertFails(user.doc('events/e1').delete()); + }); + + test.each([ + ['a missing title', (event) => withoutField(event, 'title')], + ['an empty title', (event) => ({ ...event, title: '' })], + ['an unknown status', (event) => ({ ...event, status: 'unexpected' })], + ['an unknown visibility', (event) => ({ ...event, visibility: 'everyone' })], + ['a string start time', (event) => ({ ...event, startAt: 'tomorrow' })], + ['an unsafe results URL', (event) => ({ + ...event, resultsUrl: UNSAFE_SCRIPT_URL, + })], + ['an insecure hero-image URL', (event) => ({ + ...event, heroImageUrl: 'http://example.com/hero.jpg', + })], + ['a negative capacity', (event) => ({ ...event, capacity: -1 })], + ['a zero capacity', (event) => ({ ...event, capacity: 0 })], + ['a fractional capacity', (event) => ({ ...event, capacity: 1.5 })], + ['a NaN capacity', (event) => ({ ...event, capacity: Number.NaN })], + ['a missing pricing field', (event) => withoutField(event, 'pricing')], + ['a missing required price tier', (event) => ({ + ...event, + pricing: { memberCents: 1000 }, + })], + ['a negative member price', (event) => ({ + ...event, + pricing: { memberCents: -1, nonMemberCents: 1000 }, + })], + ['a fractional non-member price', (event) => ({ + ...event, + pricing: { memberCents: 1000, nonMemberCents: 1000.5 }, + })], + ['a NaN non-member price', (event) => ({ + ...event, + pricing: { memberCents: 1000, nonMemberCents: Number.NaN }, + })], + ['an invalid early-bird timestamp', (event) => ({ + ...event, + pricing: { + memberCents: 1000, + nonMemberCents: 1500, + earlyBirdCents: 500, + earlyBirdUntil: 'later', + }, + })], + ['a map instead of custom-field list', (event) => ({ + ...event, + customFields: { key: 'pace' }, + })], + ['a missing creation timestamp', (event) => withoutField(event, 'createdAt')], + ])('admin CANNOT create an event with %s', async (_label, buildInvalid) => { + const admin = await db({ uid: 'a1', role: 'admin' }); + const event = buildInvalid(adminEvent('invalid')); + await assertFails(admin.doc('events/invalid').set(event)); + }); + + test.each([ + ['an unknown status', { status: 'unexpected' }], + ['an unknown visibility', { visibility: 'everyone' }], + ['a negative capacity', { capacity: -1 }], + ['a zero capacity', { capacity: 0 }], + ['a fractional capacity', { capacity: 1.5 }], + ['a negative price', { + pricing: { memberCents: -1, nonMemberCents: 1000 }, + }], + ['a NaN price', { + pricing: { memberCents: 1000, nonMemberCents: Number.NaN }, + }], + ['an invalid updatedAt', { updatedAt: 'not-a-timestamp' }], + ['an invalid custom-field value', { customFields: { key: 'pace' } }], + ['an unsafe results URL', { resultsUrl: UNSAFE_SCRIPT_URL }], + ['an insecure hero-image URL', { heroImageUrl: 'http://example.com/hero.jpg' }], + ])('admin CANNOT update an event with %s', async (_label, patch) => { + await seed('events/e1', adminEvent('e1')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('events/e1').update(patch)); + }); + + test.each([ + ['mismatched slug', { slug: 'different' }], + ['mismatched creator', { createdBy: 'attacker' }], + ['Stripe product ID', { stripeProductId: 'prod_test' }], + ['non-zero registered count', { registeredCount: 1 }], + ['configured capacity', { capacity: 50 }], + ['non-zero pricing', { + pricing: { memberCents: 2500, nonMemberCents: 3500 }, + }], + ['open status', { status: 'open' }], + ['members-only visibility', { visibility: 'members_only' }], + ['registration window', { registrationOpensAt: NOW }], + ['waiver configuration', { + waiverText: 'Unapproved text', waiverVersion: 'v1', + }], + ['registration custom fields', { + customFields: [{ + key: 'dob', label: 'Date of birth', type: 'date', required: true, + }], + }], + ['volunteer comp configuration', { + volunteerEnabled: true, + volunteerFields: [{ + key: 'role', label: 'Role', type: 'text', required: true, + }], + }], + ['Stripe price ID', { stripePriceIds: { member: 'price_test' } }], + ['payment state', { paymentStatus: 'paid' }], + ['capacity counters', { capacityCounters: { reserved: 1 } }], + ['inventory state', { inventory: { onHand: 10 } }], + ['audit data', { auditLog: [{ action: 'created' }] }], + ])('admin CANNOT create an event containing protected %s', async (_label, patch) => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails( + admin.doc('events/protected').set(adminEvent('protected', patch)), + ); + }); + + test('admin CANNOT inject Stripe fields into event pricing', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('events/protected').set(adminEvent('protected', { + pricing: { + memberCents: 2500, + nonMemberCents: 3500, + stripePriceId: 'price_test', + }, + }))); + }); + + test('admin CANNOT inject Stripe fields while updating event pricing', async () => { + await seed('events/e1', adminEvent('e1')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('events/e1').update({ + pricing: { + memberCents: 2500, + nonMemberCents: 3500, + stripePriceId: 'price_test', + }, + })); + }); + + test.each([ + ['slug', { slug: 'changed' }], + ['creator', { createdBy: 'attacker' }], + ['creation timestamp', { createdAt: new Date(0) }], + ['registered count', { registeredCount: 99 }], + ['capacity', { capacity: 50 }], + ['pricing', { pricing: { memberCents: 2500, nonMemberCents: 3500 } }], + ['status', { status: 'open' }], + ['visibility', { visibility: 'members_only' }], + ['registration window', { registrationOpensAt: NOW }], + ['waiver configuration', { + waiverText: 'Changed text', waiverVersion: 'v2', + }], + ['registration custom fields', { + customFields: [{ + key: 'dob', label: 'Date of birth', type: 'date', required: true, + }], + }], + ['volunteer comp configuration', { + volunteerEnabled: true, + volunteerFields: [{ + key: 'role', label: 'Role', type: 'text', required: true, + }], + }], + ['Stripe price IDs', { stripePriceIds: { member: 'price_test' } }], + ['Stripe product ID', { stripeProductId: 'prod_test' }], + ['payment state', { paymentStatus: 'paid' }], + ['capacity counters', { capacityCounters: { reserved: 1 } }], + ['inventory state', { inventory: { onHand: 10 } }], + ['audit data', { auditLog: [{ action: 'tampered' }] }], + ])('admin CANNOT update protected event %s', async (_label, patch) => { + await seed('events/e1', adminEvent('e1')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('events/e1').update(patch)); }); }); }); @@ -197,7 +582,7 @@ describe('registrations subcollection', () => { await assertFails(unv.doc('events/e1/registrations/r1').get()); }); - test('admin CAN read a registration (via catch-all)', async () => { + test('admin CAN read a registration through the explicit registrations rule', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); await assertSucceeds(admin.doc('events/e1/registrations/r1').get()); }); @@ -216,20 +601,45 @@ describe('registrations subcollection', () => { ); }); - test('admin CAN write a registration (via catch-all)', async () => { + test('admin CANNOT create a registration directly', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds( + await assertFails( admin.doc('events/e1/registrations/r2').set({ status: 'paid' }), ); }); - test('admin CAN list registrations across events', async () => { + test('admin CANNOT update registration financial state directly', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails( + admin.doc('events/e1/registrations/r1').update({ + status: 'refunded', + amountCents: 0, + }), + ); + }); + + test('admin CANNOT delete a registration directly', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('events/e1/registrations/r1').delete()); + }); + + test('admin CAN list registrations for a known event', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.collectionGroup('registrations').get()); + await assertSucceeds(admin.collection('events/e1/registrations').get()); }); test('member CANNOT list registrations across events', async () => { const member = await db({ uid: 'u1', role: 'member' }); await assertFails(member.collectionGroup('registrations').get()); }); + + test('admin CANNOT collectionGroup-list registrations across events', async () => { + await seed('events/e2/registrations/r2', { + eventId: 'e2', + status: 'paid', + amountCents: 5000, + }); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.collectionGroup('registrations').get()); + }); }); diff --git a/tests/firestore-rules/members-only.test.js b/tests/firestore-rules/members-only.test.js index 12fd63e4..1f7dc5bb 100644 --- a/tests/firestore-rules/members-only.test.js +++ b/tests/firestore-rules/members-only.test.js @@ -33,7 +33,7 @@ describe('members_only collection', () => { await assertSucceeds(member.doc('members_only/benefits').get()); }); - test('admins CAN read private content through the admin catch-all', async () => { + test('admins CAN read private content through the explicit collection rule', async () => { await seed('members_only/benefits', PRIVATE_CONTENT); const admin = await db({ uid: 'a1', role: 'admin' }); @@ -55,4 +55,15 @@ describe('members_only collection', () => { })); await assertFails(user.doc('members_only/benefits').delete()); }); + + test('admins CANNOT write private content from the browser', async () => { + await seed('members_only/benefits', PRIVATE_CONTENT); + const admin = await db({ uid: 'a1', role: 'admin' }); + + await assertFails(admin.doc('members_only/new-benefit').set(PRIVATE_CONTENT)); + await assertFails(admin.doc('members_only/benefits').update({ + discounts: '

Changed directly.

', + })); + await assertFails(admin.doc('members_only/benefits').delete()); + }); }); diff --git a/tests/firestore-rules/members.test.js b/tests/firestore-rules/members.test.js index ae74f3b3..3c407971 100644 --- a/tests/firestore-rules/members.test.js +++ b/tests/firestore-rules/members.test.js @@ -1,3 +1,5 @@ +/* eslint-env jest */ + const { clearFirestore, teardown, db, seed, assertFails, assertSucceeds, } = require('./setup'); @@ -35,12 +37,19 @@ describe('members collection', () => { await assertFails(me.doc('members/u2').get()); }); - test('admin CAN read any member doc (via catch-all)', async () => { + test('admin CAN read any member doc through the explicit members rule', async () => { await seed('members/u1', SAMPLE_MEMBER); const admin = await db({ uid: 'a1', role: 'admin' }); await assertSucceeds(admin.doc('members/u1').get()); }); + test('admin CAN list member docs for the current admin screen', async () => { + await seed('members/u1', SAMPLE_MEMBER); + await seed('members/u2', SAMPLE_MEMBER); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.collection('members').get()); + }); + test('user CANNOT list the members collection', async () => { await seed('members/u1', SAMPLE_MEMBER); await seed('members/u2', SAMPLE_MEMBER); @@ -52,20 +61,45 @@ describe('members collection', () => { }); describe('updates: self-edit allowed, immutable fields pinned', () => { - test('user CAN update their own fullName + phone', async () => { + test('user CANNOT omit updatedAt from a profile edit', async () => { await seed('members/u1', SAMPLE_MEMBER); const me = await db({ uid: 'u1', role: 'unverified' }); + await assertFails(me.doc('members/u1').update({ + fullName: 'New Name', + phoneNumber: '555-1234', + })); + }); + + test.each([ + ['a user without a role claim', { uid: 'u1' }], + ['an unverified user', { uid: 'u1', role: 'unverified' }], + ['a member', { uid: 'u1', role: 'member' }], + ['an admin editing their own profile', { uid: 'u1', role: 'admin' }], + ])('%s CAN make the exact existing-profile edit', async (_label, auth) => { + // Mirrors accountService.updateMyProfile. The role grants no extra write + // authority; ownership and the exact field/value contract are decisive. + await seed('members/u1', SAMPLE_MEMBER); + const me = await db(auth); await assertSucceeds(me.doc('members/u1').update({ fullName: 'New Name', phoneNumber: '555-1234', + updatedAt: new Date(), })); }); - test('user CAN update fullName + phoneNumber + updatedAt together', async () => { - // Mirrors the real client write (accountService.updateMyProfile). + test('profile length boundaries are accepted', async () => { await seed('members/u1', SAMPLE_MEMBER); const me = await db({ uid: 'u1', role: 'unverified' }); await assertSucceeds(me.doc('members/u1').update({ + fullName: 'n'.repeat(200), + phoneNumber: '1'.repeat(40), + updatedAt: new Date(), + })); + }); + + test('user CANNOT update a missing profile document', async () => { + const me = await db({ uid: 'u1', role: 'unverified' }); + await assertFails(me.doc('members/u1').update({ fullName: 'New Name', phoneNumber: '555-1234', updatedAt: new Date(), @@ -79,8 +113,8 @@ describe('members collection', () => { }); test('user CANNOT plant a privilege field (isAdmin)', async () => { - // The catch-all checks the auth token's role claim, not a doc field, so - // this wouldn't grant access — but the allowlist blocks it outright. + // Rules check the signed token's role claim, not a document field, so + // this would not grant access; the allowlist blocks it outright too. await seed('members/u1', SAMPLE_MEMBER); const me = await db({ uid: 'u1', role: 'unverified' }); await assertFails(me.doc('members/u1').update({ isAdmin: true })); @@ -98,13 +132,41 @@ describe('members collection', () => { test('user CANNOT set fullName to a non-string', async () => { await seed('members/u1', SAMPLE_MEMBER); const me = await db({ uid: 'u1', role: 'unverified' }); - await assertFails(me.doc('members/u1').update({ fullName: 12345 })); + await assertFails(me.doc('members/u1').update({ + fullName: 12345, + updatedAt: new Date(), + })); }); test('user CANNOT set an oversized fullName', async () => { await seed('members/u1', SAMPLE_MEMBER); const me = await db({ uid: 'u1', role: 'unverified' }); - await assertFails(me.doc('members/u1').update({ fullName: 'x'.repeat(201) })); + await assertFails(me.doc('members/u1').update({ + fullName: 'x'.repeat(201), + updatedAt: new Date(), + })); + }); + + test.each([ + ['a non-string phone number', 12345], + ['an oversized phone number', '1'.repeat(41)], + ])('user CANNOT set %s', async (_label, phoneNumber) => { + await seed('members/u1', SAMPLE_MEMBER); + const me = await db({ uid: 'u1', role: 'unverified' }); + await assertFails(me.doc('members/u1').update({ + phoneNumber, + updatedAt: new Date(), + })); + }); + + test('user CANNOT set updatedAt to a non-timestamp', async () => { + await seed('members/u1', SAMPLE_MEMBER); + const me = await db({ uid: 'u1', role: 'unverified' }); + await assertFails(me.doc('members/u1').update({ + fullName: 'New Name', + phoneNumber: '555-1234', + updatedAt: 'now', + })); }); test('user CANNOT escalate their own role', async () => { @@ -143,10 +205,24 @@ describe('members collection', () => { await assertFails(me.doc('members/u2').update({ fullName: 'Hax' })); }); - test('admin CAN promote another user\'s role (via catch-all)', async () => { + test('admin CANNOT promote another user\'s role directly', async () => { + await seed('members/u1', SAMPLE_MEMBER); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('members/u1').update({ role: 'admin' })); + }); + + test('admin CANNOT update another member profile directly', async () => { + await seed('members/u1', SAMPLE_MEMBER); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('members/u1').update({ fullName: 'Changed by admin' })); + }); + + test('user and admin CANNOT delete a member profile directly', async () => { await seed('members/u1', SAMPLE_MEMBER); + const me = await db({ uid: 'u1', role: 'member' }); const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('members/u1').update({ role: 'admin' })); + await assertFails(me.doc('members/u1').delete()); + await assertFails(admin.doc('members/u1').delete()); }); }); @@ -156,9 +232,9 @@ describe('members collection', () => { await assertFails(me.doc('members/u1').set(SAMPLE_MEMBER)); }); - test('admin CAN create a member doc', async () => { + test('admin CANNOT create a member doc directly', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('members/u9').set(SAMPLE_MEMBER)); + await assertFails(admin.doc('members/u9').set(SAMPLE_MEMBER)); }); }); }); @@ -188,11 +264,11 @@ describe('members/{uid}/connections subcollection', () => { await assertFails(me.doc('members/u1/connections/strava').set(conn)); }); - test('admin CAN read/write any connections doc (via catch-all)', async () => { + test('admin CANNOT read or write connection metadata without an explicit need', async () => { await seed('members/u1/connections/strava', conn); const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('members/u1/connections/strava').get()); - await assertSucceeds(admin.doc('members/u1/connections/strava').update({ athleteId: 99999 })); + await assertFails(admin.doc('members/u1/connections/strava').get()); + await assertFails(admin.doc('members/u1/connections/strava').update({ athleteId: 99999 })); }); test('user CANNOT collectionGroup-query all connections', async () => { @@ -204,6 +280,13 @@ describe('members/{uid}/connections subcollection', () => { const me = await db({ uid: 'u1', role: 'member' }); await assertFails(me.collectionGroup('connections').get()); }); + + test('admin CANNOT collectionGroup-query all connections', async () => { + await seed('members/u1/connections/strava', conn); + await seed('members/u2/connections/strava', conn); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.collectionGroup('connections').get()); + }); }); describe('members/{uid}/secrets subcollection', () => { @@ -230,13 +313,18 @@ describe('members/{uid}/secrets subcollection', () => { await assertFails(m.doc('members/u2/secrets/strava').get()); }); - // Note: admins CAN read secrets via the catch-all rule. This is intentional - // for token rotation/recovery — admins are highly trusted. Documenting via - // a test so the behavior is explicit and any future change is caught. - test('admin CAN read secrets (via catch-all — intentional)', async () => { + test('admin CANNOT read OAuth secrets', async () => { + await seed('members/u1/secrets/strava', secret); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('members/u1/secrets/strava').get()); + }); + + test('admin CANNOT create, update, or delete OAuth secrets', async () => { await seed('members/u1/secrets/strava', secret); const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('members/u1/secrets/strava').get()); + await assertFails(admin.doc('members/u2/secrets/strava').set(secret)); + await assertFails(admin.doc('members/u1/secrets/strava').update({ expires_at: 1 })); + await assertFails(admin.doc('members/u1/secrets/strava').delete()); }); test('member CANNOT collectionGroup-query all secrets', async () => { @@ -245,4 +333,11 @@ describe('members/{uid}/secrets subcollection', () => { const m = await db({ uid: 'u1', role: 'member' }); await assertFails(m.collectionGroup('secrets').get()); }); + + test('admin CANNOT collectionGroup-query all secrets', async () => { + await seed('members/u1/secrets/strava', secret); + await seed('members/u2/secrets/strava', secret); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.collectionGroup('secrets').get()); + }); }); diff --git a/tests/firestore-rules/shop-and-locked.test.js b/tests/firestore-rules/shop-and-locked.test.js index 33347efc..31cf3de1 100644 --- a/tests/firestore-rules/shop-and-locked.test.js +++ b/tests/firestore-rules/shop-and-locked.test.js @@ -1,3 +1,5 @@ +/* eslint-env jest */ + const { clearFirestore, teardown, db, seed, assertFails, assertSucceeds, } = require('./setup'); @@ -5,6 +7,47 @@ const { beforeEach(clearFirestore); afterAll(teardown); +const UNSAFE_SCRIPT_URL = ['javascript', 'alert(1)'].join(':'); + +function adminProduct(slug, overrides = {}) { + return { + slug, + title: 'MPRC Hat', + description: 'A lightweight running hat', + priceCents: 0, + imageUrl: null, + sizes: [], + colors: [], + status: 'draft', + createdBy: 'a1', + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + }; +} + +function productEditorUpdate(overrides = {}) { + return { + title: 'Updated MPRC Hat', + description: 'Updated description', + priceCents: 0, + imageUrl: 'https://example.com/hat.jpg', + sizes: [], + colors: [], + status: 'draft', + updatedAt: new Date(), + ...overrides, + }; +} + +function withoutField(value, field) { + const copy = { ...value }; + delete copy[field]; + return copy; +} + +const INVALID_SLUGS = ['Bad-Slug', 'bad slug', 'bad?query', 'résumé']; + describe('products collection', () => { test('anonymous CAN read active product', async () => { await seed('products/hat', { status: 'active', title: 'Hat', priceCents: 1000 }); @@ -30,7 +73,7 @@ describe('products collection', () => { await assertFails(anon.doc('products/old').get()); }); - test('admin CAN read draft product (via catch-all)', async () => { + test('admin CAN read draft product through the explicit products rule', async () => { await seed('products/secret', { status: 'draft', title: 'Secret', priceCents: 0 }); const admin = await db({ uid: 'a1', role: 'admin' }); await assertSucceeds(admin.doc('products/secret').get()); @@ -48,10 +91,143 @@ describe('products collection', () => { test('admin CAN create a product', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('products/new').set({ - status: 'active', title: 'Hat', priceCents: 1000, + await assertSucceeds(admin.doc('products/new').set(adminProduct('new'))); + }); + + test('admin CAN deliberately create a zero-price draft product', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.doc('products/free').set(adminProduct('free', { + priceCents: 0, + }))); + }); + + test.each(INVALID_SLUGS)('admin CANNOT create a product with invalid slug %s', async (slug) => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc(`products/${slug}`).set(adminProduct(slug))); + }); + + test('admin CAN update a product with the current editor payload', async () => { + await seed('products/hat', adminProduct('hat')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.doc('products/hat').update(productEditorUpdate())); + }); + + test('admin CAN edit content while preserving a server-owned product price', async () => { + const sizes = ['One size']; + const colors = ['Navy']; + await seed('products/hat', adminProduct('hat', { + priceCents: 1000, + status: 'active', + sizes, + colors, + })); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.doc('products/hat').update({ + ...productEditorUpdate({ status: 'active' }), + priceCents: 1000, + sizes, + colors, })); }); + + test('admin CANNOT delete a product directly', async () => { + await seed('products/hat', adminProduct('hat')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('products/hat').delete()); + }); + + test.each([ + ['a user without a role claim', { uid: 'u1' }], + ['an unverified user', { uid: 'u2', role: 'unverified' }], + ['a member', { uid: 'u3', role: 'member' }], + ])('%s CANNOT update or delete a product', async (_label, auth) => { + await seed('products/hat', adminProduct('hat')); + const user = await db(auth); + await assertFails(user.doc('products/hat').update({ title: 'Unauthorized' })); + await assertFails(user.doc('products/hat').delete()); + }); + + test.each([ + ['a missing title', (product) => withoutField(product, 'title')], + ['an empty title', (product) => ({ ...product, title: '' })], + ['a missing price', (product) => withoutField(product, 'priceCents')], + ['a negative price', (product) => ({ ...product, priceCents: -1 })], + ['an active zero price', (product) => ({ + ...product, + status: 'active', + priceCents: 0, + })], + ['a fractional price', (product) => ({ ...product, priceCents: 1.5 })], + ['a NaN price', (product) => ({ ...product, priceCents: Number.NaN })], + ['an unknown status', (product) => ({ ...product, status: 'unexpected' })], + ['a string size list', (product) => ({ ...product, sizes: 'S,M,L' })], + ['configured sizes', (product) => ({ ...product, sizes: ['S', 'M'] })], + ['configured colors', (product) => ({ ...product, colors: ['Navy'] })], + ['an unsafe image URL', (product) => ({ + ...product, imageUrl: UNSAFE_SCRIPT_URL, + })], + ['a missing creation timestamp', (product) => withoutField(product, 'createdAt')], + ['only slug and creator', (product) => ({ + slug: product.slug, + createdBy: product.createdBy, + })], + ])('admin CANNOT create a product with %s', async (_label, buildInvalid) => { + const admin = await db({ uid: 'a1', role: 'admin' }); + const product = buildInvalid(adminProduct('invalid')); + await assertFails(admin.doc('products/invalid').set(product)); + }); + + test.each([ + ['an empty title', { title: '' }], + ['a negative price', { priceCents: -1 }], + ['an active zero price', { status: 'active', priceCents: 0 }], + ['a fractional price', { priceCents: 1.5 }], + ['a NaN price', { priceCents: Number.NaN }], + ['an unknown status', { status: 'unexpected' }], + ['a string size list', { sizes: 'S,M,L' }], + ['configured sizes', { sizes: ['S', 'M'] }], + ['configured colors', { colors: ['Navy'] }], + ['an unsafe image URL', { imageUrl: UNSAFE_SCRIPT_URL }], + ['an invalid updatedAt', { updatedAt: 'not-a-timestamp' }], + ])('admin CANNOT update a product with %s', async (_label, patch) => { + await seed('products/hat', adminProduct('hat')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('products/hat').update(patch)); + }); + + test.each([ + ['mismatched slug', { slug: 'different' }], + ['mismatched creator', { createdBy: 'attacker' }], + ['non-zero price', { priceCents: 1000 }], + ['active status', { status: 'active', priceCents: 1000 }], + ['Stripe product ID', { stripeProductId: 'prod_test' }], + ['Stripe price ID', { stripePriceId: 'price_test' }], + ['inventory state', { inventory: { onHand: 10, reserved: 0 } }], + ['payment state', { paymentStatus: 'paid' }], + ['audit data', { auditLog: [{ action: 'created' }] }], + ])('admin CANNOT create a product containing protected %s', async (_label, patch) => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails( + admin.doc('products/protected').set(adminProduct('protected', patch)), + ); + }); + + test.each([ + ['slug', { slug: 'changed' }], + ['creator', { createdBy: 'attacker' }], + ['creation timestamp', { createdAt: new Date(0) }], + ['availability status', { status: 'active' }], + ['Stripe product ID', { stripeProductId: 'prod_test' }], + ['Stripe price ID', { stripePriceId: 'price_test' }], + ['inventory state', { inventory: { onHand: 10, reserved: 0 } }], + ['variant state', { variants: [{ sku: 'hat-navy' }] }], + ['payment state', { paymentStatus: 'paid' }], + ['audit data', { auditLog: [{ action: 'tampered' }] }], + ])('admin CANNOT update protected product %s', async (_label, patch) => { + await seed('products/hat', adminProduct('hat')); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('products/hat').update(patch)); + }); }); describe('products list queries (rules-are-not-filters)', () => { @@ -79,6 +255,11 @@ describe('products list queries (rules-are-not-filters)', () => { anon.collection('products').where('status', '==', 'archived').get(), ); }); + + test('admin CAN list all products for the current admin screen', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.collection('products').get()); + }); }); describe('orders collection', () => { @@ -101,11 +282,16 @@ describe('orders collection', () => { await assertFails(m.doc('orders/o1').get()); }); - test('admin CAN read an order (via catch-all)', async () => { + test('admin CAN read an order through the explicit orders rule', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); await assertSucceeds(admin.doc('orders/o1').get()); }); + test('admin CAN list orders for the current admin screen', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertSucceeds(admin.collection('orders').get()); + }); + test('anonymous CANNOT write an order', async () => { const anon = await db(); await assertFails(anon.doc('orders/o2').set({ status: 'paid' })); @@ -116,17 +302,37 @@ describe('orders collection', () => { await assertFails(m.doc('orders/o2').set({ status: 'paid' })); }); - test('admin CAN write an order (via catch-all)', async () => { + test('admin CANNOT create an order directly', async () => { const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('orders/o2').set({ status: 'paid' })); + await assertFails(admin.doc('orders/o2').set({ status: 'paid' })); + }); + + test('admin CANNOT update order financial state directly', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('orders/o1').update({ + status: 'refunded', + amountCents: 0, + })); + }); + + test('admin CANNOT delete an order directly', async () => { + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('orders/o1').delete()); }); }); -describe('locked-down collections (server/admin only)', () => { +describe('server-only operational collections', () => { describe.each([ - ['promoCodes/PROMO1', { discountPercent: 10 }], - ['ratelimits/checkout_ip__1.2.3.4', { count: 5 }], - ])('%s', (path_, sample) => { + ['promoCodes/PROMO1', 'promoCodes/PROMO2', { discountPercent: 10 }], + ['ratelimits/checkout_ip__1.2.3.4', 'ratelimits/checkout_ip__5.6.7.8', { count: 5 }], + ['mail/message1', 'mail/message2', { to: ['runner@example.com'] }], + ['stripeEvents/evt_test_1', 'stripeEvents/evt_test_2', { status: 'processed' }], + ['checkoutRequests/request1', 'checkoutRequests/request2', { status: 'created' }], + ['auditEvents/audit1', 'auditEvents/audit2', { action: 'refund' }], + ['retentionJobs/job1', 'retentionJobs/job2', { status: 'queued' }], + ['products/hat/variants/black-m', 'products/hat/variants/black-l', { onHand: 5 }], + ['events/e1/auditEvents/audit1', 'events/e1/auditEvents/audit2', { action: 'edit' }], + ])('%s', (path_, newPath, sample) => { test('anonymous CANNOT read', async () => { await seed(path_, sample); const anon = await db(); @@ -144,24 +350,44 @@ describe('locked-down collections (server/admin only)', () => { await assertFails(anon.doc(path_).set(sample)); }); - test('admin CAN read (via catch-all)', async () => { + test('admin CANNOT read', async () => { await seed(path_, sample); const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc(path_).get()); + await assertFails(admin.doc(path_).get()); }); - test('admin CAN write (via catch-all)', async () => { + test('admin CANNOT create, update, or delete', async () => { + await seed(path_, sample); const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc(path_).set(sample)); + await assertFails(admin.doc(newPath).set(sample)); + await assertFails(admin.doc(path_).update({ probe: true })); + await assertFails(admin.doc(path_).delete()); }); }); + + test.each([ + ['variants', 'products/hat/variants/black-m', { onHand: 5 }], + ['auditEvents', 'events/e1/auditEvents/audit1', { action: 'edit' }], + ])('admin CANNOT collectionGroup-list protected %s', async (group, path_, sample) => { + await seed(path_, sample); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.collectionGroup(group).get()); + }); }); -describe('catch-all admin rule', () => { - test('admin can read an arbitrary collection that has no specific rule', async () => { +describe('default-deny boundary', () => { + test('admin CANNOT read an arbitrary future collection', async () => { + await seed('totallyMadeUp/x', { foo: 'bar' }); + const admin = await db({ uid: 'a1', role: 'admin' }); + await assertFails(admin.doc('totallyMadeUp/x').get()); + }); + + test('admin CANNOT create, update, or delete in an arbitrary future collection', async () => { await seed('totallyMadeUp/x', { foo: 'bar' }); const admin = await db({ uid: 'a1', role: 'admin' }); - await assertSucceeds(admin.doc('totallyMadeUp/x').get()); + await assertFails(admin.doc('totallyMadeUp/new').set({ foo: 'bar' })); + await assertFails(admin.doc('totallyMadeUp/x').update({ foo: 'changed' })); + await assertFails(admin.doc('totallyMadeUp/x').delete()); }); test('non-admin CANNOT read an arbitrary collection', async () => {