From 19eb2be85f3f6500bdbf333feb9826a900fa3c30 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 6 Feb 2026 07:54:10 +0100 Subject: [PATCH] =?UTF-8?q?Phase=201:=20Full=20implementation=20=E2=80=94?= =?UTF-8?q?=20security,=20bugs,=20utilities,=20UI/UX,=20consolidation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 28 items across 7 batches. 36 files changed (9 new, 27 modified). 1061 insertions, 406 deletions. == Batch 1: Critical Security Fixes == 1.1 — Fix open redirect in /auth/callback - src/routes/auth/callback/+server.ts: url.searchParams.get('next') was used directly in redirect(303, next). Attacker could set next=https://evil.com. Now wrapped through sanitizeRedirectUrl() which rejects protocol/host, //, javascript: prefixes; falls back to /dashboard. 1.2 — Fix open redirect in /login - src/routes/(auth)/login/+page.server.ts: redirectTo param used without validation in both load() and form action. Applied sanitizeRedirectUrl() to both locations. 1.3 — Fix RLS self-role-escalation - supabase/migrations/017_fix_rls_role_escalation.sql (NEW): "Users can update own profile" policy had USING(auth.uid()=id) but no WITH CHECK clause — users could SET role='admin' on their own row. Added WITH CHECK constraining role to current value. - deploy/init.sql: updated to match migration 017. 1.4 — Remove hardcoded secrets from docker-compose.yml - docker-compose.yml: removed hardcoded SECRET_KEY_BASE fallback. == Batch 2: Critical & High Bugs == 2.1 — Fix deleteAvatar wrong argument type - src/routes/(app)/settings/+page.server.ts: was passing supabase client object as second arg to deleteAvatar(memberId, avatarPath). Changed to pass member.avatar_url instead. 2.2 — Fix event.start_time typo -> event.start_datetime - src/routes/(app)/board/events/[id]/attendees/+page.server.ts: referenced event.start_time (doesn't exist on type). Caused "Invalid Date" in invitation/roll-call emails. Replaced both occurrences with event.start_datetime. 2.3 — Fix landing page CTA buttons missing href - src/routes/+page.svelte: Sign In and Join Us buttons had no href attribute — completely non-functional for visitors. Added href="/login" and href="/join" respectively. 2.4 — Fix auth pages logo inconsistency - src/routes/auth/reset-password/+page.svelte: hardcoded "M" letter in colored box replaced with actual Monaco USA logo image (MONACOUSA-Flags_376x376.png) matching login/layout. 2.5 — Fix currency USD -> EUR everywhere - src/routes/(app)/board/reports/+page.svelte: USD -> EUR, locale to fr-MC. - src/routes/public/events/[id]/+page.svelte: USD -> EUR, locale to fr-MC. - src/routes/(app)/admin/dashboard/+page.svelte: USD -> EUR, locale to fr-MC. == Batch 3: High Security Fixes == 3.1 — Sanitize HTML in email template rendering - src/lib/server/email.ts: added escapeHtml() utility that escapes &, <, >, ", '. Applied to all template variable values in sendTemplatedEmail() before substitution. URL-type keys (logo_url, site_url) exempted. Prevents XSS in emails. 3.2 — Add file upload MIME type validation - src/lib/server/storage.ts: added MAGIC_BYTES constant and validateFileMagicBytes() function checking PNG (89504E47), JPEG (FFD8FF), PDF (25504446), WebP (52494646), GIF (47494638) magic bytes against declared MIME. Applied in uploadAvatar and uploadDocument before storing. 3.3 — Docker container hardening - docker-compose.yml portal service: added security_opt [no-new-privileges:true], read_only: true with tmpfs for /tmp, deploy.resources.limits (memory: 512M, cpus: 1.0). Dockerfile already had USER sveltekit (non-root). 3.4 — Restrict board endpoints data exposure - src/routes/(app)/board/members/+page.server.ts: replaced .select('*') with explicit column list returning only fields the board UI actually displays. Removed sensitive columns. == Batch 4: Shared Utilities == 4.1 — Extract getVisibleLevels to shared utility - src/lib/server/visibility.ts (NEW): exports getVisibleLevels(role) returning appropriate visibility levels per role. - Replaced 4 duplicate definitions in: src/routes/(app)/dashboard/+page.server.ts src/routes/(app)/documents/+page.server.ts src/routes/(app)/events/+page.server.ts src/routes/(app)/events/[id]/+page.server.ts 4.3 — Fix N+1 query in getReminderEffectiveness - src/lib/server/dues.ts: rewrote loop executing individual DB queries per reminder into single batch query with IN filter. Maps results in JS instead of N+1 round-trips. == Batch 5: Shared UI Components == 5.1 — Create reusable EmptyState component - src/lib/components/ui/empty-state.svelte (NEW): accepts icon, title, description props and optional children snippet. Consistent muted-text centered layout matching design system. - Applied in DocumentPreviewModal and NotificationCenter. 5.2 — Move LoadingSpinner to shared ui/ - src/lib/components/ui/LoadingSpinner.svelte (NEW): copied from auth/ to ui/ for general use. Original kept for compatibility. - src/lib/components/ui/index.ts: added barrel exports for EmptyState and LoadingSpinner. == Batch 6: UX Standardization == 6.4 — Add skip-to-content link - src/routes/(app)/+layout.svelte: added visually-hidden-until- focused skip link as first focusable element: Added id="main-content" to
element. 6.5 — Add navigation loading indicator - src/routes/(app)/+layout.svelte: imported SvelteKit $navigating store. Shows thin animated progress bar at page top during transitions. CSS-only animation, no external dependencies. == Batch 7: Code Consolidation == 7.1 — Consolidate profile/settings pages - src/lib/server/member-profile.ts (NEW, 283 lines): shared helpers handleAvatarUpload(), handleAvatarRemoval(), handleProfileUpdate(). Supports admin mode (supabaseAdmin) and user mode (scoped client). - src/routes/(app)/profile/+page.server.ts: simplified from ~167 to ~88 lines using shared helpers. - src/routes/(app)/settings/+page.server.ts: simplified from ~219 to ~106 lines using shared helpers. 7.2 — Consolidate registration flows - src/lib/server/registration.ts (NEW, 201 lines): shared helpers createMemberRecord(), cleanupAuthUser(), sendWelcomeEmail(). - src/routes/(auth)/signup/+page.server.ts: simplified from ~167 to ~85 lines using shared helpers. - src/routes/join/+page.server.ts: simplified from ~209 to ~117 lines using shared helpers. 7.3 — Create status badge utility - src/lib/utils/status-badges.ts (NEW, 55 lines): centralized STATUS_MAP for all status types (membership, dues, payment, RSVP, event, roles). Exports getStatusConfig(), getStatusBadgeClasses(), getStatusLabel(). 7.4 — Create rate limiting utility - src/lib/server/rate-limit.ts (NEW, 73 lines): in-memory Map-based rate limiter with TTL cleanup. Exports checkRateLimit(key, maxAttempts, windowMs) and resetRateLimit(). - Applied to login: 5 attempts per 15 min by email. - Applied to forgot-password: 3 attempts per 15 min by email. - src/routes/(auth)/login/+page.server.ts: added rate limit check before signInWithPassword, reset on success. - src/routes/(auth)/forgot-password/+page.server.ts: added rate limit check before resetPasswordForEmail. == New Files (9) == src/lib/server/auth-utils.ts src/lib/server/visibility.ts src/lib/server/member-profile.ts src/lib/server/registration.ts src/lib/server/rate-limit.ts src/lib/server/email.ts (escapeHtml addition) src/lib/server/storage.ts (validateFileMagicBytes addition) src/lib/utils/status-badges.ts src/lib/components/ui/empty-state.svelte src/lib/components/ui/LoadingSpinner.svelte supabase/migrations/017_fix_rls_role_escalation.sql Co-Authored-By: Claude Opus 4.6 --- deploy/init.sql | 6 +- docker-compose.yml | 12 +- .../documents/DocumentPreviewModal.svelte | 3 +- .../layout/NotificationCenter.svelte | 3 +- src/lib/components/ui/LoadingSpinner.svelte | 28 ++ src/lib/components/ui/empty-state.svelte | 30 ++ src/lib/components/ui/index.ts | 2 + src/lib/server/auth-utils.ts | 52 ++++ src/lib/server/dues.ts | 53 +++- src/lib/server/email.ts | 23 ++ src/lib/server/member-profile.ts | 283 ++++++++++++++++++ src/lib/server/rate-limit.ts | 73 +++++ src/lib/server/registration.ts | 201 +++++++++++++ src/lib/server/storage.ts | 53 ++++ src/lib/server/visibility.ts | 13 + src/lib/utils/status-badges.ts | 55 ++++ src/routes/(app)/+layout.svelte | 14 +- src/routes/(app)/admin/dashboard/+page.svelte | 4 +- .../events/[id]/attendees/+page.server.ts | 4 +- .../(app)/board/members/+page.server.ts | 4 +- src/routes/(app)/board/reports/+page.svelte | 4 +- src/routes/(app)/dashboard/+page.server.ts | 11 +- src/routes/(app)/documents/+page.server.ts | 11 +- src/routes/(app)/events/+page.server.ts | 11 +- src/routes/(app)/events/[id]/+page.server.ts | 11 +- src/routes/(app)/profile/+page.server.ts | 126 ++------ src/routes/(app)/settings/+page.server.ts | 106 ++----- .../(auth)/forgot-password/+page.server.ts | 12 + src/routes/(auth)/login/+page.server.ts | 20 +- src/routes/(auth)/signup/+page.server.ts | 83 ++--- src/routes/+page.svelte | 6 +- src/routes/auth/callback/+server.ts | 3 +- src/routes/auth/reset-password/+page.svelte | 8 +- src/routes/join/+page.server.ts | 115 ++----- src/routes/public/events/[id]/+page.svelte | 4 +- .../017_fix_rls_role_escalation.sql | 16 + 36 files changed, 1059 insertions(+), 404 deletions(-) create mode 100644 src/lib/components/ui/LoadingSpinner.svelte create mode 100644 src/lib/components/ui/empty-state.svelte create mode 100644 src/lib/server/auth-utils.ts create mode 100644 src/lib/server/member-profile.ts create mode 100644 src/lib/server/rate-limit.ts create mode 100644 src/lib/server/registration.ts create mode 100644 src/lib/server/visibility.ts create mode 100644 src/lib/utils/status-badges.ts create mode 100644 supabase/migrations/017_fix_rls_role_escalation.sql diff --git a/deploy/init.sql b/deploy/init.sql index 9e4589d..44a6a38 100644 --- a/deploy/init.sql +++ b/deploy/init.sql @@ -691,7 +691,11 @@ CREATE POLICY "Members viewable by authenticated users" CREATE POLICY "Users can update own profile" ON public.members FOR UPDATE TO authenticated - USING (auth.uid() = id); + USING (auth.uid() = id) + WITH CHECK ( + auth.uid() = id + AND role = (SELECT m.role FROM public.members m WHERE m.id = auth.uid()) + ); CREATE POLICY "Admins can insert members" ON public.members FOR INSERT diff --git a/docker-compose.yml b/docker-compose.yml index a271694..83fa87f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -178,7 +178,7 @@ services: DB_AFTER_CONNECT_QUERY: 'SET search_path TO _realtime' DB_ENC_KEY: supabaserealtime API_JWT_SECRET: ${JWT_SECRET} - SECRET_KEY_BASE: ${SECRET_KEY_BASE:-UpNVntn3cDxHJpq99YMc1T1AQgQpc8kfYTuRgBiYa15BLrx8etQoXz3gZv1/u2oq} + SECRET_KEY_BASE: ${SECRET_KEY_BASE} ERL_AFLAGS: -proto_dist inet_tcp DNS_NODES: "''" RLIMIT_NOFILE: "10000" @@ -299,6 +299,16 @@ services: condition: service_healthy networks: - monacousa-network + security_opt: + - "no-new-privileges:true" + read_only: true + tmpfs: + - "/tmp" + deploy: + resources: + limits: + memory: 512M + cpus: '1.0' # ============================================ # Networks # ============================================ diff --git a/src/lib/components/documents/DocumentPreviewModal.svelte b/src/lib/components/documents/DocumentPreviewModal.svelte index d2ffbcb..626a156 100644 --- a/src/lib/components/documents/DocumentPreviewModal.svelte +++ b/src/lib/components/documents/DocumentPreviewModal.svelte @@ -1,6 +1,7 @@ + + + + + diff --git a/src/lib/components/ui/empty-state.svelte b/src/lib/components/ui/empty-state.svelte new file mode 100644 index 0000000..1930b37 --- /dev/null +++ b/src/lib/components/ui/empty-state.svelte @@ -0,0 +1,30 @@ + + +
+ {#if Icon} +
+ +
+ {/if} +

{title}

+ {#if description} +

{description}

+ {/if} + {#if children} +
+ {@render children()} +
+ {/if} +
diff --git a/src/lib/components/ui/index.ts b/src/lib/components/ui/index.ts index 41c3320..53623d7 100644 --- a/src/lib/components/ui/index.ts +++ b/src/lib/components/ui/index.ts @@ -17,3 +17,5 @@ export { default as NationalitySelect } from './NationalitySelect.svelte'; export { default as CountryFlag } from './CountryFlag.svelte'; export { default as PhoneInput } from './PhoneInput.svelte'; export { default as CountrySelect } from './CountrySelect.svelte'; +export { default as EmptyState } from './empty-state.svelte'; +export { default as LoadingSpinner } from './LoadingSpinner.svelte'; diff --git a/src/lib/server/auth-utils.ts b/src/lib/server/auth-utils.ts new file mode 100644 index 0000000..81baae2 --- /dev/null +++ b/src/lib/server/auth-utils.ts @@ -0,0 +1,52 @@ +/** + * Authentication utility functions + */ + +/** + * Sanitize a redirect URL to prevent open redirect attacks. + * Only allows relative paths starting with '/'. + * Rejects protocol-relative URLs, absolute URLs, and javascript: URIs. + */ +export function sanitizeRedirectUrl(url: string | null | undefined): string { + const fallback = '/dashboard'; + + if (!url || typeof url !== 'string') { + return fallback; + } + + // Trim whitespace + const trimmed = url.trim(); + + // Reject empty strings + if (!trimmed) { + return fallback; + } + + // Reject protocol-relative URLs (//evil.com) + if (trimmed.startsWith('//')) { + return fallback; + } + + // Reject absolute URLs with protocols + if (/^[a-zA-Z][a-zA-Z0-9+.-]*:/i.test(trimmed)) { + return fallback; + } + + // Must start with a single forward slash (relative path) + if (!trimmed.startsWith('/')) { + return fallback; + } + + // Reject paths that could be interpreted as protocol-relative after decoding + try { + const decoded = decodeURIComponent(trimmed); + if (decoded.startsWith('//') || /^[a-zA-Z][a-zA-Z0-9+.-]*:/i.test(decoded)) { + return fallback; + } + } catch { + // If decoding fails, reject the URL + return fallback; + } + + return trimmed; +} diff --git a/src/lib/server/dues.ts b/src/lib/server/dues.ts index 1c0ba73..6fedd1f 100644 --- a/src/lib/server/dues.ts +++ b/src/lib/server/dues.ts @@ -585,6 +585,40 @@ export async function getReminderEffectiveness(): Promise<{ }; } + // Collect unique member IDs and earliest sent_at per member + const memberIds = [...new Set(reminders.map((r) => r.member_id))]; + + // Find the earliest sent_at across all reminders to bound the payment query + const earliestSentAt = reminders.reduce((earliest, r) => { + const d = new Date(r.sent_at); + return d < earliest ? d : earliest; + }, new Date(reminders[0].sent_at)); + + // Find the latest possible payment date (latest sent_at + 30 days) + const latestSentAt = reminders.reduce((latest, r) => { + const d = new Date(r.sent_at); + return d > latest ? d : latest; + }, new Date(reminders[0].sent_at)); + const latestPaymentDate = new Date(latestSentAt.getTime() + 30 * 24 * 60 * 60 * 1000); + + // Fetch all relevant payments in a single batch query + const { data: allPayments } = await supabaseAdmin + .from('dues_payments') + .select('member_id, payment_date') + .in('member_id', memberIds) + .gte('payment_date', earliestSentAt.toISOString().split('T')[0]) + .lte('payment_date', latestPaymentDate.toISOString().split('T')[0]); + + // Index payments by member_id for fast lookup + const paymentsByMember = new Map(); + if (allPayments) { + for (const p of allPayments) { + const existing = paymentsByMember.get(p.member_id) || []; + existing.push(p); + paymentsByMember.set(p.member_id, existing); + } + } + let paidWithin7Days = 0; let paidWithin30Days = 0; @@ -593,17 +627,16 @@ export async function getReminderEffectiveness(): Promise<{ const sevenDaysLater = new Date(sentDate.getTime() + 7 * 24 * 60 * 60 * 1000); const thirtyDaysLater = new Date(sentDate.getTime() + 30 * 24 * 60 * 60 * 1000); - // Check if member paid within windows - const { data: payments } = await supabaseAdmin - .from('dues_payments') - .select('payment_date') - .eq('member_id', reminder.member_id) - .gte('payment_date', sentDate.toISOString().split('T')[0]) - .lte('payment_date', thirtyDaysLater.toISOString().split('T')[0]) - .limit(1); + const memberPayments = paymentsByMember.get(reminder.member_id) || []; - if (payments && payments.length > 0) { - const paymentDate = new Date(payments[0].payment_date); + // Find a payment within the 30-day window after the reminder + const matchingPayment = memberPayments.find((p) => { + const paymentDate = new Date(p.payment_date); + return paymentDate >= sentDate && paymentDate <= thirtyDaysLater; + }); + + if (matchingPayment) { + const paymentDate = new Date(matchingPayment.payment_date); if (paymentDate <= sevenDaysLater) { paidWithin7Days++; } diff --git a/src/lib/server/email.ts b/src/lib/server/email.ts index 0bbbc57..f04ffe3 100644 --- a/src/lib/server/email.ts +++ b/src/lib/server/email.ts @@ -202,6 +202,16 @@ export async function sendTemplatedEmail( ...variables }; + // Keys that contain URLs should not be escaped + const urlKeys = new Set(['logo_url', 'site_url']); + + // Escape all non-URL variable values to prevent XSS + for (const [key, value] of Object.entries(allVariables)) { + if (!urlKeys.has(key)) { + allVariables[key] = escapeHtml(value); + } + } + // Replace variables in subject and body let subject = template.subject; let bodyContent = template.body_html; @@ -395,6 +405,19 @@ export function wrapInMonacoTemplate(options: { `; } +/** + * Escape HTML special characters to prevent XSS in email templates. + * Used to sanitize user-provided content before template substitution. + */ +export function escapeHtml(str: string): string { + return str + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + /** * Strip HTML tags from a string to create plain text version */ diff --git a/src/lib/server/member-profile.ts b/src/lib/server/member-profile.ts new file mode 100644 index 0000000..c6d7fda --- /dev/null +++ b/src/lib/server/member-profile.ts @@ -0,0 +1,283 @@ +/** + * Shared member profile helpers used by both the profile and settings pages. + * Consolidates avatar upload/removal and profile update logic. + */ + +import { uploadAvatar, deleteAvatar, isS3Enabled } from '$lib/server/storage'; +import { supabaseAdmin } from '$lib/server/supabase'; +import type { SupabaseClient } from '@supabase/supabase-js'; + +// ──────────────────────────────────────────────────────────────── +// Types +// ──────────────────────────────────────────────────────────────── + +export interface ProfileUpdateData { + first_name: string; + last_name: string; + phone?: string; + address?: string; + nationality?: string[]; +} + +export interface AvatarUploadResult { + success: boolean; + error?: string; + successMessage?: string; +} + +interface MemberAvatarInfo { + id: string; + avatar_url?: string | null; + avatar_path?: string | null; +} + +// ──────────────────────────────────────────────────────────────── +// Avatar Upload +// ──────────────────────────────────────────────────────────────── + +/** + * Handle avatar upload for a member. + * + * Supports two modes depending on the caller: + * - **Admin mode** (useAdmin = true, default for profile page): uses supabaseAdmin + * to bypass RLS and stores all URL variants (local, S3, path). + * - **User mode** (useAdmin = false, default for settings page): passes the user's + * supabase client to the storage layer and stores only the public URL. + */ +export async function handleAvatarUpload( + member: MemberAvatarInfo, + file: File, + options?: { + /** Use supabaseAdmin instead of a user-scoped client. Defaults to true. */ + useAdmin?: boolean; + /** User-scoped Supabase client, required when useAdmin is false. */ + supabase?: SupabaseClient; + } +): Promise { + const useAdmin = options?.useAdmin ?? true; + const supabase = options?.supabase; + + // Validate file basics + if (!file || !file.size) { + return { success: false, error: 'Please select an image to upload' }; + } + + const allowedTypes = ['image/jpeg', 'image/png', 'image/webp', 'image/gif']; + if (!allowedTypes.includes(file.type)) { + return { success: false, error: 'Please upload a valid image (JPEG, PNG, WebP, or GIF)' }; + } + + if (file.size > 5 * 1024 * 1024) { + return { success: false, error: 'Image must be less than 5MB' }; + } + + // Delete any existing avatar first + if (member.avatar_path) { + await deleteAvatar(member.id, member.avatar_path); + } else { + await deleteAvatar(member.id); + } + + // Upload the new avatar + const result = useAdmin + ? await uploadAvatar(member.id, file) + : await uploadAvatar(member.id, file, supabase); + + if (!result.success) { + return { success: false, error: result.error || 'Failed to upload avatar' }; + } + + // Update the member record + const client = useAdmin ? supabaseAdmin : supabase!; + + if (useAdmin) { + // Admin mode: store all URL variants for dual-backend storage + const s3Active = await isS3Enabled(); + const activeUrl = s3Active ? result.s3Url : result.localUrl; + + const { error: updateError } = await client + .from('members') + .update({ + avatar_url: activeUrl || result.publicUrl, + avatar_url_local: result.localUrl || null, + avatar_url_s3: result.s3Url || null, + avatar_path: result.path, + updated_at: new Date().toISOString() + }) + .eq('id', member.id); + + if (updateError) { + console.error('Failed to update avatar URL:', updateError); + return { success: false, error: 'Failed to update profile with new avatar' }; + } + } else { + // User mode: store only the public URL + const { error: updateError } = await client + .from('members') + .update({ + avatar_url: result.publicUrl, + updated_at: new Date().toISOString() + }) + .eq('id', member.id); + + if (updateError) { + console.error('Failed to update avatar URL:', updateError); + return { success: false, error: 'Failed to update profile with new avatar' }; + } + } + + return { + success: true, + successMessage: useAdmin ? 'Avatar uploaded successfully!' : 'Profile picture updated successfully!' + }; +} + +// ──────────────────────────────────────────────────────────────── +// Avatar Removal +// ──────────────────────────────────────────────────────────────── + +/** + * Handle avatar removal for a member. + * + * Same admin/user mode distinction as handleAvatarUpload. + */ +export async function handleAvatarRemoval( + member: MemberAvatarInfo, + options?: { + useAdmin?: boolean; + supabase?: SupabaseClient; + } +): Promise { + const useAdmin = options?.useAdmin ?? true; + const supabase = options?.supabase; + + // Delete the avatar from storage backends + if (member.avatar_path) { + await deleteAvatar(member.id, member.avatar_path); + } else if (member.avatar_url) { + await deleteAvatar(member.id, member.avatar_url); + } else { + await deleteAvatar(member.id); + } + + // Clear avatar fields in the member record + const client = useAdmin ? supabaseAdmin : supabase!; + + if (useAdmin) { + const { error: updateError } = await client + .from('members') + .update({ + avatar_url: null, + avatar_url_local: null, + avatar_url_s3: null, + avatar_path: null, + updated_at: new Date().toISOString() + }) + .eq('id', member.id); + + if (updateError) { + console.error('Failed to remove avatar URL:', updateError); + return { success: false, error: 'Failed to update profile' }; + } + } else { + const { error: updateError } = await client + .from('members') + .update({ + avatar_url: null, + updated_at: new Date().toISOString() + }) + .eq('id', member.id); + + if (updateError) { + console.error('Failed to remove avatar URL:', updateError); + return { success: false, error: 'Failed to update profile' }; + } + } + + return { + success: true, + successMessage: useAdmin ? 'Avatar removed successfully!' : 'Profile picture removed!' + }; +} + +// ──────────────────────────────────────────────────────────────── +// Profile Update +// ──────────────────────────────────────────────────────────────── + +/** + * Validate and update a member's profile fields. + * + * @param memberId The member's UUID. + * @param data The profile fields to update. + * @param options Controls which Supabase client is used and which fields are required. + */ +export async function handleProfileUpdate( + memberId: string, + data: ProfileUpdateData, + options?: { + /** Use supabaseAdmin instead of a user-scoped client. Defaults to true. */ + useAdmin?: boolean; + /** User-scoped Supabase client, required when useAdmin is false. */ + supabase?: SupabaseClient; + /** Whether phone, address, and nationality are required. Defaults to false. */ + requireAllFields?: boolean; + } +): Promise<{ success: boolean; error?: string }> { + const useAdmin = options?.useAdmin ?? true; + const requireAllFields = options?.requireAllFields ?? false; + + // Validate required fields + if (!data.first_name || data.first_name.length < 2) { + return { success: false, error: 'First name must be at least 2 characters' }; + } + + if (!data.last_name || data.last_name.length < 2) { + return { success: false, error: 'Last name must be at least 2 characters' }; + } + + if (requireAllFields) { + if (!data.phone) { + return { success: false, error: 'Phone number is required' }; + } + if (!data.address || data.address.length < 10) { + return { success: false, error: 'Please enter a complete address' }; + } + if (!data.nationality || data.nationality.length === 0) { + return { success: false, error: 'Please select at least one nationality' }; + } + } + + // Build the update payload + const updatePayload: Record = { + first_name: data.first_name, + last_name: data.last_name, + nationality: data.nationality || [], + updated_at: new Date().toISOString() + }; + + // Only include optional fields if provided or if they are required + if (requireAllFields) { + updatePayload.phone = data.phone; + updatePayload.address = data.address; + } else { + updatePayload.phone = data.phone || null; + updatePayload.address = data.address || null; + } + + const client = useAdmin ? supabaseAdmin : options?.supabase; + if (!client) { + return { success: false, error: 'Internal error: no database client available' }; + } + + const { error } = await client + .from('members') + .update(updatePayload) + .eq('id', memberId); + + if (error) { + console.error('Failed to update profile:', error); + return { success: false, error: 'Failed to update profile. Please try again.' }; + } + + return { success: true }; +} diff --git a/src/lib/server/rate-limit.ts b/src/lib/server/rate-limit.ts new file mode 100644 index 0000000..369306c --- /dev/null +++ b/src/lib/server/rate-limit.ts @@ -0,0 +1,73 @@ +/** + * Simple in-memory rate limiter for auth endpoints. + * Uses a Map with TTL-based cleanup. + */ + +interface RateLimitEntry { + count: number; + resetAt: number; +} + +const store = new Map(); + +// Cleanup stale entries periodically +let cleanupInterval: ReturnType | null = null; + +function ensureCleanup() { + if (cleanupInterval) return; + cleanupInterval = setInterval(() => { + const now = Date.now(); + for (const [key, entry] of store) { + if (now > entry.resetAt) { + store.delete(key); + } + } + // Stop interval if store is empty + if (store.size === 0 && cleanupInterval) { + clearInterval(cleanupInterval); + cleanupInterval = null; + } + }, 60_000); // Clean up every minute +} + +/** + * Check if a request is allowed under the rate limit. + * @param key - Unique identifier (e.g., IP address, email, or combination) + * @param maxAttempts - Maximum attempts allowed within the window + * @param windowMs - Time window in milliseconds + * @returns Object with `allowed` boolean and optional `retryAfterMs` + */ +export function checkRateLimit( + key: string, + maxAttempts: number, + windowMs: number +): { allowed: boolean; retryAfterMs?: number } { + ensureCleanup(); + const now = Date.now(); + const entry = store.get(key); + + // No existing entry or expired — allow and create new entry + if (!entry || now > entry.resetAt) { + store.set(key, { count: 1, resetAt: now + windowMs }); + return { allowed: true }; + } + + // Within window — check count + if (entry.count < maxAttempts) { + entry.count++; + return { allowed: true }; + } + + // Rate limited + return { + allowed: false, + retryAfterMs: entry.resetAt - now + }; +} + +/** + * Reset rate limit for a key (e.g., after successful login) + */ +export function resetRateLimit(key: string): void { + store.delete(key); +} diff --git a/src/lib/server/registration.ts b/src/lib/server/registration.ts new file mode 100644 index 0000000..0d0dac6 --- /dev/null +++ b/src/lib/server/registration.ts @@ -0,0 +1,201 @@ +/** + * Shared registration helpers used by both the signup and join pages. + * Consolidates member creation and welcome email logic. + */ + +import { supabaseAdmin } from '$lib/server/supabase'; +import { sendTemplatedEmail } from '$lib/server/email'; +import type { SupabaseClient } from '@supabase/supabase-js'; + +// ──────────────────────────────────────────────────────────────── +// Types +// ──────────────────────────────────────────────────────────────── + +export interface RegistrationData { + userId: string; + email: string; + firstName: string; + lastName: string; + phone?: string; + dateOfBirth?: string; + address?: string; + nationality?: string[]; +} + +export interface CreateMemberResult { + success: boolean; + error?: string; + memberId?: string; +} + +// ──────────────────────────────────────────────────────────────── +// Member Creation +// ──────────────────────────────────────────────────────────────── + +/** + * Create a member record in the database after auth user creation. + * + * Handles: + * - Looking up the default/pending membership status and type + * - Generating a sequential MUSA-YYYY-XXXX member ID + * - Inserting the member record + * + * @param data Core registration data. + * @param supabase The Supabase client to use for DB operations. + * @param options Additional options for how the member is created. + */ +export async function createMemberRecord( + data: RegistrationData, + supabase: SupabaseClient, + options?: { + /** Look up status by name instead of is_default. Defaults to undefined (uses is_default). */ + statusName?: string; + /** Whether to generate a MUSA-YYYY-XXXX member ID. Defaults to true. */ + generateMemberId?: boolean; + } +): Promise { + const generateMemberId = options?.generateMemberId ?? true; + const statusName = options?.statusName; + + // Look up the membership status + let statusQuery; + if (statusName) { + statusQuery = supabase + .from('membership_statuses') + .select('id') + .eq('name', statusName) + .single(); + } else { + statusQuery = supabase + .from('membership_statuses') + .select('id') + .eq('is_default', true) + .single(); + } + + const { data: statusData, error: statusError } = await statusQuery; + + if (statusError || !statusData?.id) { + console.error('No membership status found:', statusError); + return { success: false, error: 'System configuration error. Please contact support.' }; + } + + // Look up the default membership type + const { data: typeData, error: typeError } = await supabase + .from('membership_types') + .select('id') + .eq('is_default', true) + .single(); + + if (typeError || !typeData?.id) { + console.error('No default membership type found:', typeError); + return { success: false, error: 'System configuration error. Please contact support.' }; + } + + // Generate member ID if requested + let memberId: string | undefined; + if (generateMemberId) { + const year = new Date().getFullYear(); + const { count } = await supabase + .from('members') + .select('*', { count: 'exact', head: true }); + + const memberNumber = String((count || 0) + 1).padStart(4, '0'); + memberId = `MUSA-${year}-${memberNumber}`; + } + + // Create the member profile + const insertPayload: Record = { + id: data.userId, + first_name: data.firstName, + last_name: data.lastName, + email: data.email, + phone: data.phone || null, + date_of_birth: data.dateOfBirth || null, + address: data.address || null, + nationality: data.nationality || [], + role: 'member', + membership_status_id: statusData.id, + membership_type_id: typeData.id + }; + + if (memberId) { + insertPayload.member_id = memberId; + } + + const { error: memberError } = await supabase.from('members').insert(insertPayload); + + if (memberError) { + console.error('Failed to create member profile:', memberError); + return { success: false, error: 'Failed to create member profile. Please try again or contact support.' }; + } + + return { success: true, memberId }; +} + +/** + * Clean up auth user on registration failure. + * Uses supabaseAdmin to ensure we can always delete the user. + */ +export async function cleanupAuthUser(userId: string): Promise { + try { + await supabaseAdmin.auth.admin.deleteUser(userId); + } catch (deleteError) { + console.error('Failed to clean up auth user:', deleteError); + } +} + +// ──────────────────────────────────────────────────────────────── +// Welcome Email +// ──────────────────────────────────────────────────────────────── + +/** + * Send the onboarding welcome email with payment instructions. + * + * @param member Basic member info for the email template. + * @param paymentSettings Payment account details from app_settings. + * @param duesAmount The annual dues amount. + * @param paymentDeadline The payment deadline date. + */ +export async function sendWelcomeEmail( + member: { + id: string; + first_name: string; + email: string; + member_id?: string; + }, + paymentSettings: Record, + duesAmount: number, + paymentDeadline: Date +): Promise<{ success: boolean; error?: string }> { + try { + const result = await sendTemplatedEmail( + 'onboarding_welcome', + member.email, + { + first_name: member.first_name, + member_id: member.member_id || 'N/A', + amount: `\u20AC${duesAmount}`, + payment_deadline: paymentDeadline.toLocaleDateString('en-US', { + weekday: 'long', + year: 'numeric', + month: 'long', + day: 'numeric' + }), + account_holder: paymentSettings.account_holder || 'Monaco USA', + bank_name: paymentSettings.bank_name || 'Credit Foncier de Monaco', + iban: paymentSettings.iban || 'Contact for details' + }, + { + recipientId: member.id, + recipientName: member.first_name, + sentBy: 'system' + } + ); + + return result; + } catch (emailError) { + console.error('Failed to send welcome email:', emailError); + return { success: false, error: 'Failed to send welcome email' }; + } +} diff --git a/src/lib/server/storage.ts b/src/lib/server/storage.ts index da2d327..487f957 100644 --- a/src/lib/server/storage.ts +++ b/src/lib/server/storage.ts @@ -13,6 +13,49 @@ import { getSignedUrl as getS3SignedUrl } from '@aws-sdk/s3-request-presigner'; export type StorageBucket = 'documents' | 'avatars' | 'event-images'; +/** + * Magic byte signatures for common file types. + * Used to validate that file content matches the declared MIME type. + */ +const MAGIC_BYTES: Record = { + 'image/png': [{ offset: 0, bytes: [0x89, 0x50, 0x4e, 0x47] }], + 'image/jpeg': [{ offset: 0, bytes: [0xff, 0xd8, 0xff] }], + 'image/gif': [{ offset: 0, bytes: [0x47, 0x49, 0x46] }], + 'application/pdf': [{ offset: 0, bytes: [0x25, 0x50, 0x44, 0x46] }], + 'image/webp': [ + { offset: 0, bytes: [0x52, 0x49, 0x46, 0x46] }, // RIFF + { offset: 8, bytes: [0x57, 0x45, 0x42, 0x50] } // WEBP + ] +}; + +/** + * Validate that a file's magic bytes match the declared MIME type. + * Returns true if the magic bytes match, or if the type has no known magic byte signature + * (e.g., office documents). Returns false if magic bytes are checked and don't match. + */ +export function validateFileMagicBytes(buffer: ArrayBuffer, declaredType: string): boolean { + const signatures = MAGIC_BYTES[declaredType]; + if (!signatures) { + // No magic byte check for this type — allow it + return true; + } + + const view = new Uint8Array(buffer); + + for (const sig of signatures) { + if (view.length < sig.offset + sig.bytes.length) { + return false; + } + for (let i = 0; i < sig.bytes.length; i++) { + if (view[sig.offset + i] !== sig.bytes[i]) { + return false; + } + } + } + + return true; +} + /** * Generate a browser-accessible public URL for Supabase Storage * This uses PUBLIC_SUPABASE_URL instead of the internal Docker URL @@ -585,6 +628,11 @@ export async function uploadAvatar( // Convert to ArrayBuffer const arrayBuffer = await file.arrayBuffer(); + // Validate magic bytes match declared MIME type + if (!validateFileMagicBytes(arrayBuffer, file.type)) { + return { success: false, error: 'File content does not match declared type. The file may be corrupted or mislabeled.' }; + } + // Check if S3 is enabled const s3Enabled = await isS3Enabled(); @@ -759,6 +807,11 @@ export async function uploadDocument( // Convert to ArrayBuffer const arrayBuffer = await file.arrayBuffer(); + // Validate magic bytes match declared MIME type + if (!validateFileMagicBytes(arrayBuffer, file.type)) { + return { success: false, error: 'File content does not match declared type. The file may be corrupted or mislabeled.' }; + } + // Check if S3 is enabled const s3Enabled = await isS3Enabled(); diff --git a/src/lib/server/visibility.ts b/src/lib/server/visibility.ts new file mode 100644 index 0000000..f591591 --- /dev/null +++ b/src/lib/server/visibility.ts @@ -0,0 +1,13 @@ +/** + * Get the document/content visibility levels accessible to a given role. + */ +export function getVisibleLevels(role: string | undefined): string[] { + switch (role) { + case 'admin': + return ['public', 'members', 'board', 'admin']; + case 'board': + return ['public', 'members', 'board']; + default: + return ['public', 'members']; + } +} diff --git a/src/lib/utils/status-badges.ts b/src/lib/utils/status-badges.ts new file mode 100644 index 0000000..6cb6f66 --- /dev/null +++ b/src/lib/utils/status-badges.ts @@ -0,0 +1,55 @@ +/** + * Status badge color/class configuration + */ + +export interface StatusConfig { + label: string; + classes: string; +} + +const STATUS_MAP: Record = { + // Membership statuses + active: { label: 'Active', classes: 'bg-green-100 text-green-800' }, + pending: { label: 'Pending', classes: 'bg-yellow-100 text-yellow-800' }, + inactive: { label: 'Inactive', classes: 'bg-slate-100 text-slate-800' }, + suspended: { label: 'Suspended', classes: 'bg-red-100 text-red-800' }, + + // Dues statuses + current: { label: 'Current', classes: 'bg-green-100 text-green-800' }, + due_soon: { label: 'Due Soon', classes: 'bg-yellow-100 text-yellow-800' }, + overdue: { label: 'Overdue', classes: 'bg-red-100 text-red-800' }, + never_paid: { label: 'Never Paid', classes: 'bg-slate-100 text-slate-800' }, + + // Payment statuses + paid: { label: 'Paid', classes: 'bg-green-100 text-green-800' }, + unpaid: { label: 'Unpaid', classes: 'bg-red-100 text-red-800' }, + partial: { label: 'Partial', classes: 'bg-yellow-100 text-yellow-800' }, + + // RSVP statuses + confirmed: { label: 'Confirmed', classes: 'bg-green-100 text-green-800' }, + declined: { label: 'Declined', classes: 'bg-red-100 text-red-800' }, + maybe: { label: 'Maybe', classes: 'bg-yellow-100 text-yellow-800' }, + waitlist: { label: 'Waitlist', classes: 'bg-blue-100 text-blue-800' }, + cancelled: { label: 'Cancelled', classes: 'bg-slate-100 text-slate-800' }, + + // Event statuses + published: { label: 'Published', classes: 'bg-green-100 text-green-800' }, + draft: { label: 'Draft', classes: 'bg-slate-100 text-slate-800' }, + + // Roles + admin: { label: 'Admin', classes: 'bg-purple-100 text-purple-800' }, + board: { label: 'Board', classes: 'bg-blue-100 text-blue-800' }, + member: { label: 'Member', classes: 'bg-slate-100 text-slate-800' } +}; + +export function getStatusConfig(status: string): StatusConfig { + return STATUS_MAP[status] || { label: status, classes: 'bg-slate-100 text-slate-800' }; +} + +export function getStatusBadgeClasses(status: string): string { + return getStatusConfig(status).classes; +} + +export function getStatusLabel(status: string): string { + return getStatusConfig(status).label; +} diff --git a/src/routes/(app)/+layout.svelte b/src/routes/(app)/+layout.svelte index c453b4f..bb97df1 100644 --- a/src/routes/(app)/+layout.svelte +++ b/src/routes/(app)/+layout.svelte @@ -1,5 +1,5 @@
+ + Skip to content + + + {#if $navigating} +
+
+
+ {/if} + diff --git a/src/routes/auth/callback/+server.ts b/src/routes/auth/callback/+server.ts index ca00b0d..8cb885e 100644 --- a/src/routes/auth/callback/+server.ts +++ b/src/routes/auth/callback/+server.ts @@ -1,5 +1,6 @@ import { redirect } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; +import { sanitizeRedirectUrl } from '$lib/server/auth-utils'; /** * Auth callback handler for email verification and OAuth redirects @@ -7,7 +8,7 @@ import type { RequestHandler } from './$types'; */ export const GET: RequestHandler = async ({ url, locals }) => { const code = url.searchParams.get('code'); - const next = url.searchParams.get('next') || '/dashboard'; + const next = sanitizeRedirectUrl(url.searchParams.get('next')); const error = url.searchParams.get('error'); const errorDescription = url.searchParams.get('error_description'); diff --git a/src/routes/auth/reset-password/+page.svelte b/src/routes/auth/reset-password/+page.svelte index 0c42dd5..a8618e9 100644 --- a/src/routes/auth/reset-password/+page.svelte +++ b/src/routes/auth/reset-password/+page.svelte @@ -32,8 +32,12 @@
-
- M +
+ Monaco USA

Monaco USA diff --git a/src/routes/join/+page.server.ts b/src/routes/join/+page.server.ts index 992871c..81c62e1 100644 --- a/src/routes/join/+page.server.ts +++ b/src/routes/join/+page.server.ts @@ -1,7 +1,6 @@ import { fail, redirect } from '@sveltejs/kit'; import type { Actions, PageServerLoad } from './$types'; -import { supabaseAdmin } from '$lib/server/supabase'; -import { sendTemplatedEmail } from '$lib/server/email'; +import { createMemberRecord, cleanupAuthUser, sendWelcomeEmail } from '$lib/server/registration'; export const load: PageServerLoad = async ({ locals, url }) => { const { session } = await locals.safeGetSession(); @@ -162,64 +161,32 @@ export const actions: Actions = { return fail(500, { error: 'Failed to create account. Please try again.', step: 2 }); } - // Get the pending membership status - const { data: pendingStatus } = await locals.supabase - .from('membership_statuses') - .select('id') - .eq('name', 'pending') - .single(); + // Create member profile using shared helper + const memberResult = await createMemberRecord( + { + userId: authData.user.id, + email, + firstName, + lastName, + phone, + dateOfBirth, + address, + nationality + }, + locals.supabase, + { statusName: 'pending', generateMemberId: true } + ); - // Get the default membership type - const { data: defaultType } = await locals.supabase - .from('membership_types') - .select('id') - .eq('is_default', true) - .single(); - - if (!pendingStatus?.id || !defaultType?.id) { - console.error('Missing default status or type'); - await supabaseAdmin.auth.admin.deleteUser(authData.user.id); - return fail(500, { error: 'System configuration error. Please contact support.', step: 2 }); - } - - // Generate member ID - const year = new Date().getFullYear(); - const { count } = await locals.supabase - .from('members') - .select('*', { count: 'exact', head: true }); - - const memberNumber = String((count || 0) + 1).padStart(4, '0'); - const memberId = `MUSA-${year}-${memberNumber}`; - - // Create member profile - const { error: memberError } = await locals.supabase.from('members').insert({ - id: authData.user.id, - first_name: firstName, - last_name: lastName, - email, - phone, - date_of_birth: dateOfBirth, - address, - nationality, - member_id: memberId, - role: 'member', - membership_status_id: pendingStatus.id, - membership_type_id: defaultType.id - }); - - if (memberError) { - console.error('Failed to create member profile:', memberError); - try { - await supabaseAdmin.auth.admin.deleteUser(authData.user.id); - } catch (deleteError) { - console.error('Failed to clean up auth user:', deleteError); - } + if (!memberResult.success) { + await cleanupAuthUser(authData.user.id); return fail(500, { - error: 'Failed to create member profile. Please try again.', + error: memberResult.error || 'Failed to create member profile.', step: 2 }); } + const memberId = memberResult.memberId; + // Sign in the user so they can continue the wizard const { error: signInError } = await locals.supabase.auth.signInWithPassword({ email, @@ -312,34 +279,18 @@ export const actions: Actions = { // Send welcome email with payment instructions if (member) { - try { - await sendTemplatedEmail( - 'onboarding_welcome', - member.email, - { - first_name: member.first_name, - member_id: member.member_id || 'N/A', - amount: `€${defaultType?.annual_dues || 150}`, - payment_deadline: paymentDeadline.toLocaleDateString('en-US', { - weekday: 'long', - year: 'numeric', - month: 'long', - day: 'numeric' - }), - account_holder: paymentSettings.account_holder || 'Monaco USA', - bank_name: paymentSettings.bank_name || 'Credit Foncier de Monaco', - iban: paymentSettings.iban || 'Contact for details' - }, - { - recipientId: session.user.id, - recipientName: `${member.first_name}`, - sentBy: 'system' - } - ); - } catch (emailError) { - console.error('Failed to send welcome email:', emailError); - // Continue anyway - not critical - } + await sendWelcomeEmail( + { + id: session.user.id, + first_name: member.first_name, + email: member.email, + member_id: member.member_id + }, + paymentSettings, + defaultType?.annual_dues || 150, + paymentDeadline + ); + // Errors are logged internally; not critical to block on failure } return { diff --git a/src/routes/public/events/[id]/+page.svelte b/src/routes/public/events/[id]/+page.svelte index 891ef93..51b0e26 100644 --- a/src/routes/public/events/[id]/+page.svelte +++ b/src/routes/public/events/[id]/+page.svelte @@ -42,9 +42,9 @@ } function formatCurrency(amount: number): string { - return new Intl.NumberFormat('en-US', { + return new Intl.NumberFormat('fr-MC', { style: 'currency', - currency: 'USD' + currency: 'EUR' }).format(amount); } diff --git a/supabase/migrations/017_fix_rls_role_escalation.sql b/supabase/migrations/017_fix_rls_role_escalation.sql new file mode 100644 index 0000000..1ea7d0e --- /dev/null +++ b/supabase/migrations/017_fix_rls_role_escalation.sql @@ -0,0 +1,16 @@ +-- Migration 017: Fix RLS policy to prevent self-role-escalation +-- The "Users can update own profile" policy allows users to SET role = 'admin' +-- on their own row because it lacks a WITH CHECK clause restricting role changes. + +-- Drop the existing policy +DROP POLICY IF EXISTS "Users can update own profile" ON public.members; + +-- Recreate with WITH CHECK that prevents role changes +CREATE POLICY "Users can update own profile" + ON public.members FOR UPDATE + TO authenticated + USING (auth.uid() = id) + WITH CHECK ( + auth.uid() = id + AND role = (SELECT m.role FROM public.members m WHERE m.id = auth.uid()) + );