From 02a04df2719094e28db97575b9fbecb940b6ead3 Mon Sep 17 00:00:00 2001 From: Ochenstarik Date: Tue, 18 Aug 2026 11:28:18 +0700 Subject: [PATCH] feat(auth): Phase 2.2.2 Authorization Ownership Invariant Fix - strict requireGiveawayOwner null check, Prisma non-null organizerId with onDelete Restrict, repository non-null organizer invariant, migration documentation, and regression tests --- docs/MIGRATION_OWNERSHIP_INVARIANT.md | 57 ++++++++++++++++++ prisma/schema.prisma | 4 +- src/lib/auth/auth-guard.ts | 9 ++- src/lib/repository/giveaway-repository.ts | 6 +- src/lib/repository/memory-repository.ts | 8 ++- src/lib/repository/prisma-repository.ts | 6 +- tests/auth-guard.test.ts | 60 ++++++++++++++++++- tests/concurrency-draw-100.test.ts | 2 + tests/concurrency-draw.test.ts | 1 + .../concurrency-participants-snapshot.test.ts | 2 + tests/concurrency-snapshot.test.ts | 2 + tests/payload-scalability.test.ts | 2 + tests/persistence.test.ts | 1 + tests/security.test.ts | 1 + tests/snapshot-binding.test.ts | 1 + tests/storage-driver.test.ts | 1 + 16 files changed, 150 insertions(+), 13 deletions(-) create mode 100644 docs/MIGRATION_OWNERSHIP_INVARIANT.md diff --git a/docs/MIGRATION_OWNERSHIP_INVARIANT.md b/docs/MIGRATION_OWNERSHIP_INVARIANT.md new file mode 100644 index 0000000..17caaa3 --- /dev/null +++ b/docs/MIGRATION_OWNERSHIP_INVARIANT.md @@ -0,0 +1,57 @@ +# Database Migration Strategy: Mandatory Organizer Ownership Invariant + +This document details the database schema migration strategy for enforcing mandatory, non-null `organizerId` on the `Giveaway` table in production. + +--- + +## 1. Context & Invariant + +In Randomayzer, every giveaway is owned by an authenticated Organizer (represented by the `User` model via VK ID OAuth 2.1). +To prevent broken access control and ambiguous authorization states: +- `Giveaway.organizerId` is strictly `NOT NULL`. +- Foreign key relation is defined with `onDelete: Restrict`, strictly preventing the deletion of an organizer account while owned giveaways exist. + +--- + +## 2. Prisma Schema Definition + +```prisma +model User { + id String @id @default(cuid()) + vkUserId String @unique + giveaways Giveaway[] + ... +} + +model Giveaway { + id String @id @default(cuid()) + organizerId String + organizer User @relation(fields: [organizerId], references: [id], onDelete: Restrict) + ... + @@index([organizerId]) +} +``` + +--- + +## 3. Migration Plan for Existing Records + +### A. Development & Staging Environments +Existing anonymous test records created prior to Phase 2.2 can be purged: +```sql +DELETE FROM "ParticipantSnapshot" WHERE "giveawayId" IN (SELECT "id" FROM "Giveaway" WHERE "organizerId" IS NULL); +DELETE FROM "Participant" WHERE "giveawayId" IN (SELECT "id" FROM "Giveaway" WHERE "organizerId" IS NULL); +DELETE FROM "Giveaway" WHERE "organizerId" IS NULL; +``` + +### B. Production Migration Protocol +1. **Never assign legacy records to arbitrary users**: If unassigned giveaways exist in a production database, do not automatically bind them to random organizers. +2. **Quarantine or Admin Assignment**: Migrate legacy orphan giveaways to a designated system quarantine table or assign to an explicitly verified administrative organizer. +3. **Execute SQL Migration**: +```sql +-- Step 1: Verify 0 null records remain +SELECT COUNT(*) FROM "Giveaway" WHERE "organizerId" IS NULL; + +-- Step 2: Enforce NOT NULL and Restrict constraint +ALTER TABLE "Giveaway" ALTER COLUMN "organizerId" SET NOT NULL; +``` diff --git a/prisma/schema.prisma b/prisma/schema.prisma index c1f7a44..84ef9a4 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -73,8 +73,8 @@ model Giveaway { winnersCount Int @default(1) reserveWinnersCount Int @default(0) seed String? - organizerId String? - organizer User? @relation(fields: [organizerId], references: [id], onDelete: SetNull) + organizerId String + organizer User @relation(fields: [organizerId], references: [id], onDelete: Restrict) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt drawnAt DateTime? diff --git a/src/lib/auth/auth-guard.ts b/src/lib/auth/auth-guard.ts index 5a66028..823e85f 100644 --- a/src/lib/auth/auth-guard.ts +++ b/src/lib/auth/auth-guard.ts @@ -26,6 +26,7 @@ export async function requireAuthenticatedUser(req: NextRequest): Promise = new Set(); async createGiveaway(input: CreateGiveawayInput): Promise { + if (!input.organizerId) { + throw new Error('FATAL: organizerId is strictly required to create a giveaway in repository'); + } + const id = 'gw_' + Math.random().toString(36).slice(2, 10); const now = new Date().toISOString(); @@ -38,7 +42,7 @@ export class MemoryGiveawayRepository implements IGiveawayRepository { winnersCount: input.winnersCount || 1, reserveWinnersCount: input.reserveWinnersCount || 0, seed: input.seed || null, - organizerId: input.organizerId || null, + organizerId: input.organizerId, createdAt: now, updatedAt: now, drawnAt: null, @@ -99,7 +103,7 @@ export class MemoryGiveawayRepository implements IGiveawayRepository { status: gw.status, winnersCount: gw.winnersCount, reserveWinnersCount: gw.reserveWinnersCount, - organizerId: gw.organizerId || null, + organizerId: gw.organizerId, createdAt: gw.createdAt, updatedAt: gw.updatedAt, drawnAt: gw.drawnAt, diff --git a/src/lib/repository/prisma-repository.ts b/src/lib/repository/prisma-repository.ts index 493c870..c567cfd 100644 --- a/src/lib/repository/prisma-repository.ts +++ b/src/lib/repository/prisma-repository.ts @@ -86,7 +86,7 @@ export class PrismaGiveawayRepository implements IGiveawayRepository { winnersCount: raw.winnersCount, reserveWinnersCount: raw.reserveWinnersCount, seed: raw.seed, - organizerId: raw.organizerId || null, + organizerId: raw.organizerId, createdAt: raw.createdAt.toISOString(), updatedAt: raw.updatedAt.toISOString(), drawnAt: raw.drawnAt ? raw.drawnAt.toISOString() : null, @@ -115,7 +115,7 @@ export class PrismaGiveawayRepository implements IGiveawayRepository { winnersCount: input.winnersCount || 1, reserveWinnersCount: input.reserveWinnersCount || 0, seed: input.seed, - organizerId: input.organizerId || null, + organizerId: input.organizerId, }, include: { participants: true, @@ -212,7 +212,7 @@ export class PrismaGiveawayRepository implements IGiveawayRepository { status: item.status as GiveawayStatusType, winnersCount: item.winnersCount, reserveWinnersCount: item.reserveWinnersCount, - organizerId: item.organizerId || null, + organizerId: item.organizerId, createdAt: item.createdAt.toISOString(), updatedAt: item.updatedAt.toISOString(), drawnAt: item.drawnAt ? item.drawnAt.toISOString() : null, diff --git a/tests/auth-guard.test.ts b/tests/auth-guard.test.ts index 2946921..e1034bd 100644 --- a/tests/auth-guard.test.ts +++ b/tests/auth-guard.test.ts @@ -10,7 +10,7 @@ import { POST as drawPost } from '../src/app/api/giveaways/[id]/draw/route'; import { GET as verifyGet } from '../src/app/api/giveaways/[id]/verify/route'; import { defaultSessionStore, SESSION_COOKIE_NAME } from '../src/lib/auth/session'; -describe('Phase 2.2.1 Giveaway Ownership & AuthZ Guard Security Suite', () => { +describe('Phase 2.2.2 Giveaway Ownership Invariant & AuthZ Guard Security Suite', () => { let memoryRepo: MemoryGiveawayRepository; const ownerUser = { id: 'usr_organizer_1', vkUserId: '111111', firstName: 'Alice' }; const intruderUser = { id: 'usr_intruder_2', vkUserId: '222222', firstName: 'Eve' }; @@ -215,6 +215,64 @@ describe('Phase 2.2.1 Giveaway Ownership & AuthZ Guard Security Suite', () => { expect(ownerDrawRes.status).toBe(200); }); + it('null organizer giveaway must NEVER authorize any user (fails with 403 Forbidden)', async () => { + // Manually inject a corrupted/legacy giveaway with empty organizerId + const corruptedId = 'gw_corrupted_null_owner'; + (memoryRepo as any).giveaways.set(corruptedId, { + id: corruptedId, + platform: 'VK', + sourceUrl: 'https://vk.com/wall-1_1', + platformOwnerId: '-1', + platformPostId: '1', + title: 'Corrupted', + organizerId: '', // Empty/null owner + status: 'READY', + participants: [], + filterRules: validPostData.filterRules, + winnersCount: 1, + reserveWinnersCount: 0, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + drawnAt: null, + snapshots: [], + latestSnapshot: null, + drawResult: null, + }); + + // 1. Authenticated user attempts GET -> 403 + const getReq = new NextRequest(`http://localhost:3000/api/giveaways/${corruptedId}`, { + headers: { cookie: `${SESSION_COOKIE_NAME}=${ownerSessionId}` }, + }); + const getRes = await giveawayDetailGet(getReq, { params: { id: corruptedId } }); + expect(getRes.status).toBe(403); + const getBody = await getRes.json(); + expect(getBody.error?.message).toMatch(/no valid organizer assigned/i); + + // 2. Authenticated user attempts POST participants -> 403 + const partReq = new NextRequest(`http://localhost:3000/api/giveaways/${corruptedId}/participants`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + cookie: `${SESSION_COOKIE_NAME}=${ownerSessionId}`, + }, + body: JSON.stringify({ filterRules: validPostData.filterRules }), + }); + const partRes = await participantsPost(partReq, { params: { id: corruptedId } }); + expect(partRes.status).toBe(403); + + // 3. Authenticated user attempts POST draw -> 403 + const drawReq = new NextRequest(`http://localhost:3000/api/giveaways/${corruptedId}/draw`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + cookie: `${SESSION_COOKIE_NAME}=${ownerSessionId}`, + }, + body: JSON.stringify({ winnersCount: 1, reserveWinnersCount: 0 }), + }); + const drawRes = await drawPost(drawReq, { params: { id: corruptedId } }); + expect(drawRes.status).toBe(403); + }); + it('GET /api/giveaways/[id]/verify remains public without requiring authentication', async () => { const created = await GiveawayStore.create({ sourceUrl: validPostData.sourceUrl, diff --git a/tests/concurrency-draw-100.test.ts b/tests/concurrency-draw-100.test.ts index c73df5a..ed35bdd 100644 --- a/tests/concurrency-draw-100.test.ts +++ b/tests/concurrency-draw-100.test.ts @@ -38,6 +38,7 @@ describe('100-Draw Concurrency Regression & Mixed Race', () => { filterRules: DEFAULT_FILTER_RULES, winnersCount: 3, reserveWinnersCount: 1, + organizerId: 'usr_conc_100', }); const snapshot = await repo.createAndLockSnapshot(gw.id, participants, DEFAULT_FILTER_RULES); @@ -104,6 +105,7 @@ describe('100-Draw Concurrency Regression & Mixed Race', () => { filterRules: DEFAULT_FILTER_RULES, winnersCount: 1, reserveWinnersCount: 0, + organizerId: 'usr_conc_100', }); await repo.saveParticipants(gw.id, participants); diff --git a/tests/concurrency-draw.test.ts b/tests/concurrency-draw.test.ts index 0f0f472..0947d50 100644 --- a/tests/concurrency-draw.test.ts +++ b/tests/concurrency-draw.test.ts @@ -38,6 +38,7 @@ describe('Concurrency Double Draw Protection', () => { repostsCount: 10, }, filterRules: DEFAULT_FILTER_RULES, + organizerId: 'usr_conc_draw', }); const snapshot = await repo.createAndLockSnapshot(gw.id, participants, DEFAULT_FILTER_RULES); diff --git a/tests/concurrency-participants-snapshot.test.ts b/tests/concurrency-participants-snapshot.test.ts index d34416b..4a20363 100644 --- a/tests/concurrency-participants-snapshot.test.ts +++ b/tests/concurrency-participants-snapshot.test.ts @@ -48,6 +48,7 @@ describe('Concurrency: Participants Update vs Snapshot Lock Race', () => { repostsCount: 0, }, filterRules: DEFAULT_FILTER_RULES, + organizerId: 'usr_conc_part_snap', }); await repo.saveParticipants(gw.id, initialParticipants); @@ -84,6 +85,7 @@ describe('Concurrency: Participants Update vs Snapshot Lock Race', () => { repostsCount: 0, }, filterRules: DEFAULT_FILTER_RULES, + organizerId: 'usr_conc_part_snap', }); await repo.saveParticipants(gw.id, initialParticipants); diff --git a/tests/concurrency-snapshot.test.ts b/tests/concurrency-snapshot.test.ts index 4c9d0fb..d03ea1c 100644 --- a/tests/concurrency-snapshot.test.ts +++ b/tests/concurrency-snapshot.test.ts @@ -50,6 +50,7 @@ describe('Concurrency Snapshot Locking & Participant Isolation', () => { repostsCount: 1, }, filterRules: DEFAULT_FILTER_RULES, + organizerId: 'usr_conc_snap', }); // Valid transition: READY -> SNAPSHOT_LOCKED -> DRAWN @@ -77,6 +78,7 @@ describe('Concurrency Snapshot Locking & Participant Isolation', () => { repostsCount: 1, }, filterRules: DEFAULT_FILTER_RULES, + organizerId: 'usr_conc_snap', }); await repo.createAndLockSnapshot(gw.id, participants, DEFAULT_FILTER_RULES); diff --git a/tests/payload-scalability.test.ts b/tests/payload-scalability.test.ts index 57fd4f1..e5a535f 100644 --- a/tests/payload-scalability.test.ts +++ b/tests/payload-scalability.test.ts @@ -34,6 +34,7 @@ describe('Payload Scalability & Pagination', () => { repostsCount: 10, }, filterRules: DEFAULT_FILTER_RULES, + organizerId: 'usr_payload_test', }); await repo.saveParticipants(gw.id, participants); @@ -65,6 +66,7 @@ describe('Payload Scalability & Pagination', () => { repostsCount: 10, }, filterRules: DEFAULT_FILTER_RULES, + organizerId: 'usr_payload_test', }); await repo.saveParticipants(gw.id, participants); diff --git a/tests/persistence.test.ts b/tests/persistence.test.ts index bf5b933..0df57b7 100644 --- a/tests/persistence.test.ts +++ b/tests/persistence.test.ts @@ -67,6 +67,7 @@ describe('Repository Persistence & Lifecycle Scenario', () => { filterRules: DEFAULT_FILTER_RULES, winnersCount: 1, reserveWinnersCount: 1, + organizerId: 'usr_persist_test', }); expect(gw.status).toBe('READY'); diff --git a/tests/security.test.ts b/tests/security.test.ts index 4966143..abf3cf7 100644 --- a/tests/security.test.ts +++ b/tests/security.test.ts @@ -102,6 +102,7 @@ describe('Security: VK_SERVICE_TOKEN handling', () => { excludeBlacklistedIds: [], excludeDuplicateComments: true, }, + organizerId: 'usr_security_test', }); const json = JSON.stringify(gw); diff --git a/tests/snapshot-binding.test.ts b/tests/snapshot-binding.test.ts index 572e707..d3aea9a 100644 --- a/tests/snapshot-binding.test.ts +++ b/tests/snapshot-binding.test.ts @@ -69,6 +69,7 @@ describe('DrawResult Snapshot Binding Regression Tests', () => { repostsCount: 10, }, filterRules: DEFAULT_FILTER_RULES, + organizerId: 'usr_snap_bind_test', }); // 2. Create snapshot V1 diff --git a/tests/storage-driver.test.ts b/tests/storage-driver.test.ts index b59557d..a93158f 100644 --- a/tests/storage-driver.test.ts +++ b/tests/storage-driver.test.ts @@ -65,6 +65,7 @@ describe('Storage Driver Policy & No Silent Fallback', () => { repostsCount: 2, }, filterRules: {} as any, + organizerId: 'usr_test_driver', }) ).rejects.toThrow(/Can't reach database server/);