diff --git a/src/components/ProjectCard.tsx b/src/components/ProjectCard.tsx index 03b5783..0d122ad 100644 --- a/src/components/ProjectCard.tsx +++ b/src/components/ProjectCard.tsx @@ -331,11 +331,12 @@ const addFeature = () => { {localProject.key_learnings && localProject.key_learnings.length > 0 && (

Key Learnings

- +
)} diff --git a/src/hooks/useSectionData.ts b/src/hooks/useSectionData.ts index 0c5b28f..8862f83 100644 --- a/src/hooks/useSectionData.ts +++ b/src/hooks/useSectionData.ts @@ -20,6 +20,8 @@ export const useSectionData = (userId: string | undefined) => { id, title, description, + project_role, + key_learnings, links:project_links( id, title, @@ -49,6 +51,8 @@ export const useSectionData = (userId: string | undefined) => { id: project.id, title: project.title, description: project.description || "", + project_role: project.project_role || "", + key_learnings: project.key_learnings || [], links: (project as any).project_links?.map((link: any) => ({ id: link.id, title: link.title, diff --git a/src/hooks/useSharedPortfolio.ts b/src/hooks/useSharedPortfolio.ts index 2951c92..51c3c4e 100644 --- a/src/hooks/useSharedPortfolio.ts +++ b/src/hooks/useSharedPortfolio.ts @@ -48,36 +48,31 @@ export const useSharedPortfolio = (shareId: string | undefined) => { console.log("Sanitized shareId:", sanitizedShareId); console.log("Original shareId:", shareId); - // Make a single optimized query to get both the share record and user ID - // This eliminates one database roundtrip - const { data: shareData, error: shareError } = await supabase - .from('portfolio_shares') - .select('user_id, is_active') - .eq('share_id', sanitizedShareId) - .maybeSingle(); + // Use the secure SECURITY DEFINER function to get user_id without exposing enumeration + const { data: userId, error: userError } = await supabase + .rpc('get_user_from_share', { share_id_param: sanitizedShareId }); - console.log("Share data query result:", { shareData, shareError }); + console.log("User from share result:", { userId, userError }); - if (shareError || !shareData || !shareData.is_active) { - console.log("Share not found or not active:", { shareError, shareData }); + if (userError || !userId) { + console.log("Share not found or not active:", { userError, userId }); setNotFound(true); setIsLoading(false); return; } - const userId = shareData.user_id; - // Use Promise.all to execute queries concurrently // This significantly reduces total wait time const [profileData, sectionsData] = await Promise.all([ - // Query 1: Fetch profile including privacy settings + // Query 1: Fetch profile using the secure public_profiles view + // This view automatically filters email/phone based on privacy settings supabase - .from('profiles') + .from('public_profiles') .select('name, photo_url, email, phone, role, tagline, description, social_links, show_email, show_phone') .eq('user_id', userId) .single(), - // Query 2: Fetch sections with projects, links, and features (including project_role) + // Query 2: Fetch sections with projects, links, and features (including project_role and key_learnings) supabase .from('sections') .select(` @@ -89,6 +84,7 @@ export const useSharedPortfolio = (shareId: string | undefined) => { title, description, project_role, + key_learnings, project_links ( id, title, @@ -109,15 +105,14 @@ export const useSharedPortfolio = (shareId: string | undefined) => { console.error("Error fetching profile:", profileData.error); } else if (profileData.data) { // Apply sanitization efficiently + // Note: email and phone are already filtered by the public_profiles view based on privacy settings const data = profileData.data; - const showEmail = data.show_email ?? true; - const showPhone = data.show_phone ?? true; const sanitizedProfileData = { name: sanitizeText(data.name || ""), photo: data.photo_url || "", - email: showEmail ? sanitizeText(data.email || "") : "", - telephone: showPhone ? sanitizeText(data.phone || "") : "", + email: sanitizeText(data.email || ""), + telephone: sanitizeText(data.phone || ""), role: sanitizeText(data.role || ""), tagline: sanitizeText(data.tagline || ""), description: sanitizeText(data.description || ""), @@ -174,6 +169,9 @@ export const useSharedPortfolio = (shareId: string | undefined) => { title: sanitizeText(project.title || "Untitled Project"), description: sanitizeText(project.description || ""), project_role: project.project_role ? sanitizeText(project.project_role) : undefined, + key_learnings: Array.isArray(project.key_learnings) + ? project.key_learnings.map(learning => sanitizeText(learning)) + : [], links, features }; diff --git a/src/integrations/supabase/types.ts b/src/integrations/supabase/types.ts index 49fa2e1..c8957be 100644 --- a/src/integrations/supabase/types.ts +++ b/src/integrations/supabase/types.ts @@ -322,13 +322,58 @@ export type Database = { } } Views: { - [_ in never]: never + public_profiles: { + Row: { + description: string | null + email: string | null + name: string | null + phone: string | null + photo_url: string | null + role: string | null + show_email: boolean | null + show_phone: boolean | null + social_links: Json | null + tagline: string | null + user_id: string | null + } + Insert: { + description?: string | null + email?: never + name?: string | null + phone?: never + photo_url?: string | null + role?: string | null + show_email?: boolean | null + show_phone?: boolean | null + social_links?: Json | null + tagline?: string | null + user_id?: string | null + } + Update: { + description?: string | null + email?: never + name?: string | null + phone?: never + photo_url?: string | null + role?: string | null + show_email?: boolean | null + show_phone?: boolean | null + social_links?: Json | null + tagline?: string | null + user_id?: string | null + } + Relationships: [] + } } Functions: { delete_user: { Args: Record Returns: undefined } + get_user_from_share: { + Args: { share_id_param: string } + Returns: string + } has_role: { Args: { _role: Database["public"]["Enums"]["app_role"] diff --git a/supabase/migrations/20251016203109_ecdb4706-61ef-4f9a-9719-65f7c124a85c.sql b/supabase/migrations/20251016203109_ecdb4706-61ef-4f9a-9719-65f7c124a85c.sql new file mode 100644 index 0000000..ae7b0e6 --- /dev/null +++ b/supabase/migrations/20251016203109_ecdb4706-61ef-4f9a-9719-65f7c124a85c.sql @@ -0,0 +1,196 @@ +-- Migration to fix critical security issues: PII exposure, input validation, and user enumeration + +-- ============================================================================ +-- STEP 0: Fix existing data to prevent constraint violations +-- ============================================================================ + +-- Fix empty names in profiles +UPDATE public.profiles +SET name = 'User' +WHERE name IS NULL OR char_length(trim(name)) = 0; + +-- Fix empty titles in sections +UPDATE public.sections +SET title = 'Section' +WHERE title IS NULL OR char_length(trim(title)) = 0; + +-- Fix empty titles in projects (if any) +UPDATE public.projects +SET title = 'Untitled Project' +WHERE title IS NULL OR char_length(trim(title)) = 0; + +-- Fix empty titles in project_features (if any) +UPDATE public.project_features +SET title = 'Feature' +WHERE title IS NULL OR char_length(trim(title)) = 0; + +-- Fix empty titles in project_links (if any) +UPDATE public.project_links +SET title = 'Link' +WHERE title IS NULL OR char_length(trim(title)) = 0; + +-- ============================================================================ +-- FIX #1: Email and Phone Exposure via Public RLS +-- ============================================================================ + +-- Drop the existing public profile policy +DROP POLICY IF EXISTS "Public can view profiles with active shares" ON public.profiles; + +-- Create a secure view that respects privacy settings +CREATE OR REPLACE VIEW public.public_profiles AS +SELECT + user_id, + name, + photo_url, + role, + tagline, + description, + social_links, + CASE WHEN show_email = true THEN email ELSE NULL END as email, + CASE WHEN show_phone = true THEN phone ELSE NULL END as phone, + show_email, + show_phone +FROM public.profiles +WHERE EXISTS ( + SELECT 1 + FROM public.portfolio_shares + WHERE portfolio_shares.user_id = profiles.user_id + AND portfolio_shares.is_active = true +); + +-- Grant SELECT on the view to anonymous users +GRANT SELECT ON public.public_profiles TO anon; +GRANT SELECT ON public.public_profiles TO authenticated; + +-- Keep the original RLS policy for authenticated users on the profiles table +CREATE POLICY "Public can view profiles with active shares" +ON public.profiles +FOR SELECT +USING ( + EXISTS ( + SELECT 1 + FROM public.portfolio_shares + WHERE portfolio_shares.user_id = profiles.user_id + AND portfolio_shares.is_active = true + ) +); + +-- ============================================================================ +-- FIX #2: Server-Side Input Validation +-- ============================================================================ + +-- Add length constraints to profiles table +ALTER TABLE public.profiles + DROP CONSTRAINT IF EXISTS profiles_name_length, + DROP CONSTRAINT IF EXISTS profiles_email_length, + DROP CONSTRAINT IF EXISTS profiles_phone_length, + DROP CONSTRAINT IF EXISTS profiles_tagline_length, + DROP CONSTRAINT IF EXISTS profiles_description_length, + DROP CONSTRAINT IF EXISTS profiles_role_length; + +ALTER TABLE public.profiles + ADD CONSTRAINT profiles_name_length CHECK (name IS NULL OR (char_length(trim(name)) > 0 AND char_length(name) <= 100)), + ADD CONSTRAINT profiles_email_length CHECK (email IS NULL OR char_length(email) <= 255), + ADD CONSTRAINT profiles_phone_length CHECK (phone IS NULL OR char_length(phone) <= 50), + ADD CONSTRAINT profiles_tagline_length CHECK (tagline IS NULL OR char_length(tagline) <= 200), + ADD CONSTRAINT profiles_description_length CHECK (description IS NULL OR char_length(description) <= 1000), + ADD CONSTRAINT profiles_role_length CHECK (role IS NULL OR char_length(role) <= 100); + +-- Add validation function for profiles +CREATE OR REPLACE FUNCTION public.validate_profile_data() +RETURNS TRIGGER AS $$ +BEGIN + -- Validate email format if provided + IF NEW.email IS NOT NULL AND NEW.email !~ '^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$' THEN + RAISE EXCEPTION 'Invalid email format'; + END IF; + + -- Validate name is not just whitespace (if provided) + IF NEW.name IS NOT NULL AND char_length(trim(NEW.name)) = 0 THEN + RAISE EXCEPTION 'Name cannot be empty or whitespace only'; + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql SET search_path = public; + +-- Create trigger for profile validation +DROP TRIGGER IF EXISTS validate_profile_before_change ON public.profiles; +CREATE TRIGGER validate_profile_before_change +BEFORE INSERT OR UPDATE ON public.profiles +FOR EACH ROW EXECUTE FUNCTION public.validate_profile_data(); + +-- Add validation constraints for sections table +ALTER TABLE public.sections + DROP CONSTRAINT IF EXISTS sections_title_length, + DROP CONSTRAINT IF EXISTS sections_description_length; + +ALTER TABLE public.sections + ADD CONSTRAINT sections_title_length CHECK (title IS NULL OR (char_length(trim(title)) > 0 AND char_length(title) <= 200)), + ADD CONSTRAINT sections_description_length CHECK (description IS NULL OR char_length(description) <= 1000); + +-- Add validation constraints for projects table +ALTER TABLE public.projects + DROP CONSTRAINT IF EXISTS projects_title_length, + DROP CONSTRAINT IF EXISTS projects_description_length; + +ALTER TABLE public.projects + ADD CONSTRAINT projects_title_length CHECK (title IS NULL OR (char_length(trim(title)) > 0 AND char_length(title) <= 200)), + ADD CONSTRAINT projects_description_length CHECK (description IS NULL OR char_length(description) <= 2000); + +-- Add validation constraints for project_features table +ALTER TABLE public.project_features + DROP CONSTRAINT IF EXISTS project_features_title_length; + +ALTER TABLE public.project_features + ADD CONSTRAINT project_features_title_length CHECK (title IS NULL OR (char_length(trim(title)) > 0 AND char_length(title) <= 200)); + +-- Add validation constraints for project_links table +ALTER TABLE public.project_links + DROP CONSTRAINT IF EXISTS project_links_title_length, + DROP CONSTRAINT IF EXISTS project_links_url_length; + +ALTER TABLE public.project_links + ADD CONSTRAINT project_links_title_length CHECK (title IS NULL OR (char_length(trim(title)) > 0 AND char_length(title) <= 100)), + ADD CONSTRAINT project_links_url_length CHECK (url IS NULL OR (char_length(trim(url)) > 0 AND char_length(url) <= 1000)); + +-- ============================================================================ +-- FIX #3: User ID Enumeration via Portfolio Shares +-- ============================================================================ + +-- Drop the existing public portfolio_shares policy +DROP POLICY IF EXISTS "Public can view active shares for verification" ON public.portfolio_shares; + +-- Create a more restrictive policy that doesn't expose user_id directly +CREATE POLICY "Public can verify share existence" +ON public.portfolio_shares +FOR SELECT +USING (is_active = true); + +-- Create a SECURITY DEFINER function to safely get user_id from share_id +CREATE OR REPLACE FUNCTION public.get_user_from_share(share_id_param TEXT) +RETURNS UUID +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = public +AS $$ +DECLARE + result_user_id UUID; +BEGIN + -- Validate share_id format (alphanumeric and hyphens only) + IF share_id_param IS NULL OR share_id_param !~ '^[a-zA-Z0-9\-]+$' THEN + RAISE EXCEPTION 'Invalid share_id format'; + END IF; + + -- Get user_id for active share + SELECT user_id INTO result_user_id + FROM public.portfolio_shares + WHERE share_id = share_id_param AND is_active = true; + + RETURN result_user_id; +END; +$$; + +-- Grant execute to anonymous and authenticated users +GRANT EXECUTE ON FUNCTION public.get_user_from_share(TEXT) TO anon; +GRANT EXECUTE ON FUNCTION public.get_user_from_share(TEXT) TO authenticated; \ No newline at end of file diff --git a/supabase/migrations/20251016211004_6e2b0fb0-4e44-471b-b3c5-4793933aa2cf.sql b/supabase/migrations/20251016211004_6e2b0fb0-4e44-471b-b3c5-4793933aa2cf.sql new file mode 100644 index 0000000..35050b9 --- /dev/null +++ b/supabase/migrations/20251016211004_6e2b0fb0-4e44-471b-b3c5-4793933aa2cf.sql @@ -0,0 +1,30 @@ +-- Update get_user_from_share function to respect profile privacy settings +-- This ensures that portfolios marked as private cannot be accessed via share links + +DROP FUNCTION IF EXISTS public.get_user_from_share(text); + +CREATE OR REPLACE FUNCTION public.get_user_from_share(share_id_param text) +RETURNS uuid +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path TO 'public' +AS $function$ +DECLARE + result_user_id UUID; +BEGIN + -- Validate share_id format (alphanumeric and hyphens only) + IF share_id_param IS NULL OR share_id_param !~ '^[a-zA-Z0-9\-]+$' THEN + RAISE EXCEPTION 'Invalid share_id format'; + END IF; + + -- Get user_id for active share AND public profile + SELECT ps.user_id INTO result_user_id + FROM public.portfolio_shares ps + JOIN public.profiles p ON p.user_id = ps.user_id + WHERE ps.share_id = share_id_param + AND ps.is_active = true + AND p.is_public = true; + + RETURN result_user_id; +END; +$function$; \ No newline at end of file