diff --git a/apps/website/lib/blockers/Blocker.ts b/apps/website/lib/blockers/Blocker.ts new file mode 100644 index 000000000..f01ff131b --- /dev/null +++ b/apps/website/lib/blockers/Blocker.ts @@ -0,0 +1,23 @@ +/** + * Abstract base class for all Blockers. + * + * Blockers are frontend mechanisms that prevent actions to improve UX. + * They are not security mechanisms and must be reversible and best-effort. + */ +export abstract class Blocker { + /** + * Checks if the action can be executed. + * @returns true if the action can proceed, false otherwise. + */ + abstract canExecute(): boolean; + + /** + * Blocks the action from being executed. + */ + abstract block(): void; + + /** + * Releases the block, allowing future executions. + */ + abstract release(): void; +} \ No newline at end of file diff --git a/apps/website/lib/blockers/SubmitBlocker.test.ts b/apps/website/lib/blockers/SubmitBlocker.test.ts new file mode 100644 index 000000000..4079285f0 --- /dev/null +++ b/apps/website/lib/blockers/SubmitBlocker.test.ts @@ -0,0 +1,37 @@ +import { describe, it, expect } from 'vitest'; +import { SubmitBlocker } from './SubmitBlocker'; + +describe('SubmitBlocker', () => { + let blocker: SubmitBlocker; + + beforeEach(() => { + blocker = new SubmitBlocker(); + }); + + it('should allow execution initially', () => { + expect(blocker.canExecute()).toBe(true); + }); + + it('should block execution after block() is called', () => { + blocker.block(); + expect(blocker.canExecute()).toBe(false); + }); + + it('should allow execution again after release() is called', () => { + blocker.block(); + expect(blocker.canExecute()).toBe(false); + blocker.release(); + expect(blocker.canExecute()).toBe(true); + }); + + it('should handle multiple block/release cycles', () => { + blocker.block(); + expect(blocker.canExecute()).toBe(false); + blocker.release(); + expect(blocker.canExecute()).toBe(true); + blocker.block(); + expect(blocker.canExecute()).toBe(false); + blocker.release(); + expect(blocker.canExecute()).toBe(true); + }); +}); \ No newline at end of file diff --git a/apps/website/lib/blockers/SubmitBlocker.ts b/apps/website/lib/blockers/SubmitBlocker.ts new file mode 100644 index 000000000..9dbb0b6b2 --- /dev/null +++ b/apps/website/lib/blockers/SubmitBlocker.ts @@ -0,0 +1,22 @@ +import { Blocker } from './Blocker'; + +/** + * SubmitBlocker prevents multiple submissions until explicitly released. + * + * Useful for preventing duplicate form submissions or API calls. + */ +export class SubmitBlocker extends Blocker { + private blocked = false; + + canExecute(): boolean { + return !this.blocked; + } + + block(): void { + this.blocked = true; + } + + release(): void { + this.blocked = false; + } +} \ No newline at end of file diff --git a/apps/website/lib/blockers/ThrottleBlocker.test.ts b/apps/website/lib/blockers/ThrottleBlocker.test.ts new file mode 100644 index 000000000..1c7bce39b --- /dev/null +++ b/apps/website/lib/blockers/ThrottleBlocker.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect, vi } from 'vitest'; +import { ThrottleBlocker } from './ThrottleBlocker'; + +describe('ThrottleBlocker', () => { + let blocker: ThrottleBlocker; + + beforeEach(() => { + vi.useFakeTimers(); + blocker = new ThrottleBlocker(100); // 100ms delay + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should allow execution initially', () => { + expect(blocker.canExecute()).toBe(true); + }); + + it('should block execution immediately after block() is called', () => { + blocker.block(); + expect(blocker.canExecute()).toBe(false); + }); + + it('should allow execution again after the delay has passed', () => { + blocker.block(); + expect(blocker.canExecute()).toBe(false); + + // Advance time by 100ms + vi.advanceTimersByTime(100); + expect(blocker.canExecute()).toBe(true); + }); + + it('should handle multiple block calls', () => { + blocker.block(); + expect(blocker.canExecute()).toBe(false); + + vi.advanceTimersByTime(50); + expect(blocker.canExecute()).toBe(false); // Still blocked + + vi.advanceTimersByTime(50); + expect(blocker.canExecute()).toBe(true); + + blocker.block(); // Block again + expect(blocker.canExecute()).toBe(false); + + vi.advanceTimersByTime(100); + expect(blocker.canExecute()).toBe(true); + }); + + it('should work with different delay values', () => { + const fastBlocker = new ThrottleBlocker(50); + fastBlocker.block(); + expect(fastBlocker.canExecute()).toBe(false); + + vi.advanceTimersByTime(50); + expect(fastBlocker.canExecute()).toBe(true); + }); +}); \ No newline at end of file diff --git a/apps/website/lib/blockers/ThrottleBlocker.ts b/apps/website/lib/blockers/ThrottleBlocker.ts new file mode 100644 index 000000000..befe62722 --- /dev/null +++ b/apps/website/lib/blockers/ThrottleBlocker.ts @@ -0,0 +1,24 @@ +import { Blocker } from './Blocker'; + +/** + * ThrottleBlocker enforces a minimum time delay between executions. + * + * Useful for preventing rapid successive actions, such as button mashing. + */ +export class ThrottleBlocker extends Blocker { + private lastExecutionTime = 0; + + constructor(private readonly delayMs: number) {} + + canExecute(): boolean { + return Date.now() - this.lastExecutionTime >= this.delayMs; + } + + block(): void { + this.lastExecutionTime = Date.now(); + } + + release(): void { + // No-op for throttle blocker + } +} \ No newline at end of file diff --git a/apps/website/lib/blockers/index.ts b/apps/website/lib/blockers/index.ts new file mode 100644 index 000000000..57941612c --- /dev/null +++ b/apps/website/lib/blockers/index.ts @@ -0,0 +1,3 @@ +export { Blocker } from './Blocker'; +export { SubmitBlocker } from './SubmitBlocker'; +export { ThrottleBlocker } from './ThrottleBlocker'; \ No newline at end of file diff --git a/apps/website/lib/services/leagues/LeagueService.test.ts b/apps/website/lib/services/leagues/LeagueService.test.ts index 10bf54a27..b2d47a63f 100644 --- a/apps/website/lib/services/leagues/LeagueService.test.ts +++ b/apps/website/lib/services/leagues/LeagueService.test.ts @@ -200,7 +200,7 @@ describe('LeagueService', () => { }); describe('createLeague', () => { - it('should call apiClient.create and return CreateLeagueViewModel', async () => { + it('should call apiClient.create', async () => { const input: CreateLeagueInputDTO = { name: 'New League', description: 'A new league', @@ -213,31 +213,9 @@ describe('LeagueService', () => { mockApiClient.create.mockResolvedValue(mockDto); - const result = await service.createLeague(input); + await service.createLeague(input); expect(mockApiClient.create).toHaveBeenCalledWith(input); - expect(result).toBeInstanceOf(CreateLeagueViewModel); - expect(result.leagueId).toBe('new-league-id'); - expect(result.success).toBe(true); - }); - - it('should handle unsuccessful creation', async () => { - const input: CreateLeagueInputDTO = { - name: 'New League', - description: 'A new league', - }; - - const mockDto: CreateLeagueOutputDTO = { - leagueId: '', - success: false, - }; - - mockApiClient.create.mockResolvedValue(mockDto); - - const result = await service.createLeague(input); - - expect(result.success).toBe(false); - expect(result.successMessage).toBe('Failed to create league.'); }); it('should throw error when apiClient.create fails', async () => { @@ -251,6 +229,52 @@ describe('LeagueService', () => { await expect(service.createLeague(input)).rejects.toThrow('API call failed'); }); + + it('should not call apiClient.create when submitBlocker is blocked', async () => { + const input: CreateLeagueInputDTO = { + name: 'New League', + description: 'A new league', + }; + + // First call should succeed + const mockDto: CreateLeagueOutputDTO = { + leagueId: 'new-league-id', + success: true, + }; + mockApiClient.create.mockResolvedValue(mockDto); + + await service.createLeague(input); // This should block the submitBlocker + + // Reset mock to check calls + mockApiClient.create.mockClear(); + + // Second call should not call API + await service.createLeague(input); + expect(mockApiClient.create).not.toHaveBeenCalled(); + }); + + it('should not call apiClient.create when throttle is active', async () => { + const input: CreateLeagueInputDTO = { + name: 'New League', + description: 'A new league', + }; + + // First call + const mockDto: CreateLeagueOutputDTO = { + leagueId: 'new-league-id', + success: true, + }; + mockApiClient.create.mockResolvedValue(mockDto); + + await service.createLeague(input); // This blocks throttle for 500ms + + // Reset mock + mockApiClient.create.mockClear(); + + // Immediate second call should not call API due to throttle + await service.createLeague(input); + expect(mockApiClient.create).not.toHaveBeenCalled(); + }); }); describe('removeMember', () => { diff --git a/apps/website/lib/services/leagues/LeagueService.ts b/apps/website/lib/services/leagues/LeagueService.ts index 9551ec42d..421cbc0b4 100644 --- a/apps/website/lib/services/leagues/LeagueService.ts +++ b/apps/website/lib/services/leagues/LeagueService.ts @@ -8,6 +8,7 @@ import { LeagueStandingsViewModel } from "@/lib/view-models/LeagueStandingsViewM import { LeagueStatsViewModel } from "@/lib/view-models/LeagueStatsViewModel"; import { LeagueSummaryViewModel } from "@/lib/view-models/LeagueSummaryViewModel"; import { RemoveMemberViewModel } from "@/lib/view-models/RemoveMemberViewModel"; +import { SubmitBlocker, ThrottleBlocker } from "@/lib/blockers"; /** @@ -17,6 +18,9 @@ import { RemoveMemberViewModel } from "@/lib/view-models/RemoveMemberViewModel"; * All dependencies are injected via constructor. */ export class LeagueService { + private readonly submitBlocker = new SubmitBlocker(); + private readonly throttle = new ThrottleBlocker(500); + constructor( private readonly apiClient: LeaguesApiClient ) {} @@ -68,12 +72,19 @@ export class LeagueService { } /** - * Create a new league - */ - async createLeague(input: CreateLeagueInputDTO): Promise { - const dto = await this.apiClient.create(input); - return new CreateLeagueViewModel(dto); - } + * Create a new league + */ + async createLeague(input: CreateLeagueInputDTO): Promise { + if (!this.submitBlocker.canExecute() || !this.throttle.canExecute()) return; + + this.submitBlocker.block(); + this.throttle.block(); + try { + await this.apiClient.create(input); + } finally { + this.submitBlocker.release(); + } + } /** * Remove a member from league diff --git a/docs/architecture/BLOCKER_GUARDS.md b/docs/architecture/BLOCKER_GUARDS.md new file mode 100644 index 000000000..93472ab41 --- /dev/null +++ b/docs/architecture/BLOCKER_GUARDS.md @@ -0,0 +1,154 @@ +Blockers & Guards + +This document defines clear, non-overlapping responsibilities for Blockers (frontend) and Guards (backend). +The goal is to prevent semantic drift, security confusion, and inconsistent implementations. + +⸻ + +Core Principle + +Guards enforce. Blockers prevent. + • Guards protect the system. + • Blockers protect the UX. + +There are no exceptions to this rule. + +⸻ + +Backend — Guards (NestJS) + +Definition + +A Guard is a backend mechanism that enforces access or execution rules. +If a Guard denies execution, the request does not reach the application logic. + +In NestJS, Guards implement CanActivate. + +⸻ + +Responsibilities + +Guards MAY: + • block requests entirely + • return HTTP errors (401, 403, 429) + • enforce authentication and authorization + • enforce rate limits + • enforce feature availability + • protect against abuse and attacks + +Guards MUST: + • be deterministic + • be authoritative + • be security-relevant + +⸻ + +Restrictions + +Guards MUST NOT: + • depend on frontend state + • contain UI logic + • attempt to improve UX + • assume the client behaved correctly + +⸻ + +Common Backend Guards + • AuthGuard + • RolesGuard + • PermissionsGuard + • ThrottlerGuard (NestJS) + • RateLimitGuard + • CsrfGuard + • FeatureFlagGuard + +⸻ + +Summary (Backend) + • Guards decide + • Guards enforce + • Guards secure the system + +⸻ + +Frontend — Blockers + +Definition + +A Blocker is a frontend mechanism that prevents an action from being executed. +Blockers exist solely to improve UX and reduce unnecessary requests. + +Blockers are not security mechanisms. + +⸻ + +Responsibilities + +Blockers MAY: + • prevent multiple submissions + • disable actions temporarily + • debounce or throttle interactions + • hide or disable UI elements + • prevent navigation under certain conditions + +Blockers MUST: + • be reversible + • be local to the frontend + • be treated as best-effort helpers + +⸻ + +Restrictions + +Blockers MUST NOT: + • enforce security + • claim authorization + • block access permanently + • replace backend Guards + • make assumptions about backend state + +⸻ + +Common Frontend Blockers + • SubmitBlocker + • AuthBlocker + • RoleBlocker + • ThrottleBlocker + • NavigationBlocker + • FeatureBlocker + +⸻ + +Summary (Frontend) + • Blockers prevent execution + • Blockers improve UX + • Blockers reduce mistakes and load + +⸻ + +Clear Separation + +Aspect Blocker (Frontend) Guard (Backend) +Purpose Prevent execution Enforce rules +Security ❌ No ✅ Yes +Authority ❌ Best-effort ✅ Final +Reversible ✅ Yes ❌ No +Failure effect UI feedback HTTP error + + +⸻ + +Naming Rules (Hard) + • Frontend uses *Blocker + • Backend uses *Guard + • Never mix the terms + • Never implement Guards in the frontend + • Never implement Blockers in the backend + +⸻ + +Final Rule + +If it must be enforced, it is a Guard. + +If it only prevents UX mistakes, it is a Blocker. \ No newline at end of file