mirror of
https://github.com/pacnpal/thrilltrack-explorer.git
synced 2025-12-21 13:31:13 -05:00
Fix profile privacy and simplify code
This commit is contained in:
@@ -65,18 +65,33 @@ This document outlines all security improvements made to the account settings sy
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## ✅ Priority 3: Profile Privacy Enforcement (VERIFIED)
|
## ✅ Priority 3: Profile Privacy Enforcement (COMPLETED)
|
||||||
|
|
||||||
### Status: IMPLEMENTED & WORKING
|
### Status: IMPLEMENTED & SIMPLIFIED
|
||||||
|
|
||||||
**Verified**:
|
**Changes Made**:
|
||||||
- ✅ `get_filtered_profile()` function exists in database
|
- ✅ Updated `useProfile()` hook to use `get_filtered_profile()` RPC instead of direct table query
|
||||||
- ✅ RLS policies correctly filter profile data based on privacy settings
|
- ✅ Removed `useProfileFieldAccess` hook (no longer needed - privacy handled at database layer)
|
||||||
- ✅ Privacy levels: public, private
|
- ✅ Simplified `LocationDisplay` component (removed redundant privacy checks)
|
||||||
- ✅ Banned/deactivated users hidden from non-moderators
|
- ✅ Simplified `PersonalLocationDisplay` component (removed redundant privacy checks)
|
||||||
- ✅ Field-level privacy via `can_view_profile_field()` function
|
- ✅ Removed `canViewField()` checks from Profile page (trust filtered data from RPC)
|
||||||
|
|
||||||
**No Changes Needed**: The privacy system is already properly implemented.
|
**Architecture**:
|
||||||
|
- Privacy enforcement now happens exclusively at the database layer via `get_filtered_profile()`
|
||||||
|
- Frontend components trust that if a field is present in the response, it's viewable
|
||||||
|
- Single source of truth for privacy logic (no duplication between frontend and backend)
|
||||||
|
|
||||||
|
**Security Benefits**:
|
||||||
|
- Field-level privacy enforced by security definer function
|
||||||
|
- No possibility of frontend bypassing privacy checks
|
||||||
|
- Better performance (1 RPC call instead of 8+ field checks)
|
||||||
|
- Simpler, more maintainable code
|
||||||
|
|
||||||
|
**Files Modified**:
|
||||||
|
- `src/hooks/useProfile.tsx`
|
||||||
|
- `src/pages/Profile.tsx`
|
||||||
|
- `src/components/profile/LocationDisplay.tsx`
|
||||||
|
- `src/components/profile/PersonalLocationDisplay.tsx`
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -131,6 +146,14 @@ This document outlines all security improvements made to the account settings sy
|
|||||||
|
|
||||||
**Priority**: HIGH - Should be enabled immediately.
|
**Priority**: HIGH - Should be enabled immediately.
|
||||||
|
|
||||||
|
### 2. Extensions in Public Schema (LOW RISK)
|
||||||
|
|
||||||
|
**Issue**: Supabase linter warns that extensions are in the `public` schema.
|
||||||
|
|
||||||
|
**Action**: No action required - this is a default Supabase configuration. Moving extensions could break functionality.
|
||||||
|
|
||||||
|
**Why Safe**: Supabase manages these extensions, and they don't pose a security risk in this context.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 📊 Security Improvement Summary
|
## 📊 Security Improvement Summary
|
||||||
@@ -140,9 +163,10 @@ This document outlines all security improvements made to the account settings sy
|
|||||||
| P1 | Missing `search_path` on functions | ✅ FIXED | HIGH | Low |
|
| P1 | Missing `search_path` on functions | ✅ FIXED | HIGH | Low |
|
||||||
| P2 | Backend username validation | ✅ FIXED | HIGH | Medium |
|
| P2 | Backend username validation | ✅ FIXED | HIGH | Medium |
|
||||||
| P2.5 | Display name content filtering | ✅ FIXED | MEDIUM | Low |
|
| P2.5 | Display name content filtering | ✅ FIXED | MEDIUM | Low |
|
||||||
| P3 | Profile privacy enforcement | ✅ VERIFIED | HIGH | N/A |
|
| P3 | Profile privacy enforcement | ✅ FIXED | HIGH | Low |
|
||||||
| P4 | Disposable email blocking | ✅ FIXED | MEDIUM | Medium |
|
| P4 | Disposable email blocking | ✅ FIXED | MEDIUM | Medium |
|
||||||
| MANUAL | Leaked password protection | ⚠️ PENDING USER | HIGH | Low |
|
| MANUAL | Leaked password protection | ⚠️ PENDING USER | HIGH | Low |
|
||||||
|
| INFO | Extensions in public schema | ℹ️ DOCUMENTED | LOW | N/A |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -223,5 +247,16 @@ Before deploying to production, verify:
|
|||||||
---
|
---
|
||||||
|
|
||||||
**Last Updated**: 2025-01-14
|
**Last Updated**: 2025-01-14
|
||||||
**Implemented By**: AI Security Audit
|
**Implemented By**: AI Security Audit & Code Simplification
|
||||||
**Reviewed By**: Pending user verification
|
**Reviewed By**: Pending user verification
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 🎉 Implementation Complete
|
||||||
|
|
||||||
|
All automated security improvements have been implemented. The codebase now follows best practices:
|
||||||
|
- ✅ Database-layer privacy enforcement
|
||||||
|
- ✅ No client-side privacy bypass possible
|
||||||
|
- ✅ Simplified, maintainable code
|
||||||
|
- ✅ Single source of truth for security logic
|
||||||
|
- ✅ Better performance (fewer database calls)
|
||||||
|
|||||||
@@ -1,49 +1,15 @@
|
|||||||
import { useState, useEffect } from 'react';
|
|
||||||
import { MapPin } from 'lucide-react';
|
import { MapPin } from 'lucide-react';
|
||||||
import { supabase } from '@/integrations/supabase/client';
|
|
||||||
|
|
||||||
interface LocationDisplayProps {
|
interface LocationDisplayProps {
|
||||||
location: {
|
location: {
|
||||||
city?: string;
|
city?: string;
|
||||||
country: string;
|
country: string;
|
||||||
};
|
} | null | undefined;
|
||||||
userId: string;
|
|
||||||
isOwnProfile: boolean;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export function LocationDisplay({ location, userId, isOwnProfile }: LocationDisplayProps) {
|
export function LocationDisplay({ location }: LocationDisplayProps) {
|
||||||
const [showLocation, setShowLocation] = useState(false);
|
// get_filtered_profile() already handles privacy - if location is present, it's viewable
|
||||||
|
if (!location) {
|
||||||
useEffect(() => {
|
|
||||||
fetchLocationPrivacy();
|
|
||||||
}, [userId, isOwnProfile]);
|
|
||||||
|
|
||||||
const fetchLocationPrivacy = async () => {
|
|
||||||
try {
|
|
||||||
const { data: { user } } = await supabase.auth.getUser();
|
|
||||||
const viewerId = user?.id;
|
|
||||||
|
|
||||||
// Use the secure function to check location visibility
|
|
||||||
const { data, error } = await supabase
|
|
||||||
.rpc('can_view_user_location', {
|
|
||||||
_viewer_id: viewerId,
|
|
||||||
_profile_user_id: userId
|
|
||||||
});
|
|
||||||
|
|
||||||
if (error) {
|
|
||||||
console.error('Error checking location privacy:', error);
|
|
||||||
setShowLocation(false);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
setShowLocation(data || false);
|
|
||||||
} catch (error) {
|
|
||||||
console.error('Error checking location privacy:', error);
|
|
||||||
setShowLocation(false);
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
if (!showLocation) {
|
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,46 +1,12 @@
|
|||||||
import { useState, useEffect } from 'react';
|
|
||||||
import { MapPin } from 'lucide-react';
|
import { MapPin } from 'lucide-react';
|
||||||
import { supabase } from '@/integrations/supabase/client';
|
|
||||||
|
|
||||||
interface PersonalLocationDisplayProps {
|
interface PersonalLocationDisplayProps {
|
||||||
personalLocation: string;
|
personalLocation: string | null | undefined;
|
||||||
userId: string;
|
|
||||||
isOwnProfile: boolean;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export function PersonalLocationDisplay({ personalLocation, userId, isOwnProfile }: PersonalLocationDisplayProps) {
|
export function PersonalLocationDisplay({ personalLocation }: PersonalLocationDisplayProps) {
|
||||||
const [showLocation, setShowLocation] = useState(false);
|
// get_filtered_profile() already handles privacy - if personalLocation is present, it's viewable
|
||||||
|
if (!personalLocation) {
|
||||||
useEffect(() => {
|
|
||||||
fetchLocationPrivacy();
|
|
||||||
}, [userId, isOwnProfile]);
|
|
||||||
|
|
||||||
const fetchLocationPrivacy = async () => {
|
|
||||||
try {
|
|
||||||
const { data: { user } } = await supabase.auth.getUser();
|
|
||||||
const viewerId = user?.id;
|
|
||||||
|
|
||||||
// Use the secure function to check location visibility
|
|
||||||
const { data, error } = await supabase
|
|
||||||
.rpc('can_view_user_location', {
|
|
||||||
_viewer_id: viewerId,
|
|
||||||
_profile_user_id: userId
|
|
||||||
});
|
|
||||||
|
|
||||||
if (error) {
|
|
||||||
console.error('Error checking location privacy:', error);
|
|
||||||
setShowLocation(false);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
setShowLocation(data || false);
|
|
||||||
} catch (error) {
|
|
||||||
console.error('Error fetching location privacy:', error);
|
|
||||||
setShowLocation(false);
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
if (!showLocation || !personalLocation) {
|
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -10,25 +10,47 @@ export function useProfile(userId: string | undefined) {
|
|||||||
queryFn: async () => {
|
queryFn: async () => {
|
||||||
if (!userId) return null;
|
if (!userId) return null;
|
||||||
|
|
||||||
console.log('[useProfile] Fetching profile for userId:', userId);
|
console.log('[useProfile] Fetching filtered profile for userId:', userId);
|
||||||
|
|
||||||
const { data, error } = await supabase
|
// Get current viewer ID
|
||||||
.from('profiles')
|
const { data: { user } } = await supabase.auth.getUser();
|
||||||
.select('*, location:locations(*)')
|
const viewerId = user?.id || null;
|
||||||
.eq('user_id', userId)
|
|
||||||
.maybeSingle();
|
// Use get_filtered_profile RPC for privacy-aware field filtering
|
||||||
|
const { data, error } = await supabase.rpc('get_filtered_profile', {
|
||||||
|
_profile_user_id: userId,
|
||||||
|
_viewer_id: viewerId
|
||||||
|
});
|
||||||
|
|
||||||
if (error) {
|
if (error) {
|
||||||
console.error('[useProfile] Error:', error);
|
console.error('[useProfile] Error:', error);
|
||||||
throw error;
|
throw error;
|
||||||
}
|
}
|
||||||
|
|
||||||
console.log('[useProfile] Profile loaded:', {
|
if (!data) return null;
|
||||||
username: data?.username,
|
|
||||||
avatar_url: data?.avatar_url
|
// Type assertion since we know the structure from the RPC function
|
||||||
|
const profileData = data as unknown as Profile;
|
||||||
|
|
||||||
|
// Fetch location separately if location_id is present and visible
|
||||||
|
if (profileData.location_id) {
|
||||||
|
const { data: location } = await supabase
|
||||||
|
.from('locations')
|
||||||
|
.select('*')
|
||||||
|
.eq('id', profileData.location_id)
|
||||||
|
.single();
|
||||||
|
|
||||||
|
if (location) {
|
||||||
|
profileData.location = location;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log('[useProfile] Filtered profile loaded:', {
|
||||||
|
username: profileData.username,
|
||||||
|
has_avatar: !!profileData.avatar_url
|
||||||
});
|
});
|
||||||
|
|
||||||
return data as Profile;
|
return profileData;
|
||||||
},
|
},
|
||||||
enabled: !!userId,
|
enabled: !!userId,
|
||||||
staleTime: 5 * 60 * 1000, // 5 minutes
|
staleTime: 5 * 60 * 1000, // 5 minutes
|
||||||
|
|||||||
@@ -1,94 +0,0 @@
|
|||||||
import { useState, useEffect } from 'react';
|
|
||||||
import { supabase } from '@/integrations/supabase/client';
|
|
||||||
import { useAuth } from '@/hooks/useAuth';
|
|
||||||
|
|
||||||
interface ProfileFieldAccess {
|
|
||||||
[fieldName: string]: boolean;
|
|
||||||
}
|
|
||||||
|
|
||||||
export function useProfileFieldAccess(profileUserId: string | null | undefined) {
|
|
||||||
const { user } = useAuth();
|
|
||||||
const [fieldAccess, setFieldAccess] = useState<ProfileFieldAccess>({});
|
|
||||||
const [loading, setLoading] = useState(true);
|
|
||||||
|
|
||||||
useEffect(() => {
|
|
||||||
if (!profileUserId) {
|
|
||||||
setLoading(false);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
checkFieldAccess();
|
|
||||||
}, [profileUserId, user?.id]);
|
|
||||||
|
|
||||||
const checkFieldAccess = async () => {
|
|
||||||
if (!profileUserId || !user?.id) {
|
|
||||||
setLoading(false);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
try {
|
|
||||||
setLoading(true);
|
|
||||||
|
|
||||||
// Fields that might need privacy checking
|
|
||||||
const fieldsToCheck = [
|
|
||||||
'date_of_birth',
|
|
||||||
'personal_location',
|
|
||||||
'location_id',
|
|
||||||
'preferred_pronouns',
|
|
||||||
'home_park_id',
|
|
||||||
'bio',
|
|
||||||
'avatar_url',
|
|
||||||
'avatar_image_id'
|
|
||||||
];
|
|
||||||
|
|
||||||
const accessChecks: ProfileFieldAccess = {};
|
|
||||||
|
|
||||||
// Check each field individually using our security definer function
|
|
||||||
for (const field of fieldsToCheck) {
|
|
||||||
const { data, error } = await supabase.rpc('can_view_profile_field', {
|
|
||||||
_viewer_id: user.id,
|
|
||||||
_profile_user_id: profileUserId,
|
|
||||||
_field_name: field
|
|
||||||
});
|
|
||||||
|
|
||||||
if (error) {
|
|
||||||
console.error(`Error checking access for field ${field}:`, error);
|
|
||||||
accessChecks[field] = false;
|
|
||||||
} else {
|
|
||||||
accessChecks[field] = data === true;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
setFieldAccess(accessChecks);
|
|
||||||
} catch (error) {
|
|
||||||
console.error('Error checking field access:', error);
|
|
||||||
// Default to denying access on error
|
|
||||||
setFieldAccess({});
|
|
||||||
} finally {
|
|
||||||
setLoading(false);
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
const canViewField = (fieldName: string): boolean => {
|
|
||||||
if (!profileUserId || !user?.id) {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Users can always see their own fields
|
|
||||||
if (user.id === profileUserId) {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
return fieldAccess[fieldName] || false;
|
|
||||||
};
|
|
||||||
|
|
||||||
const refresh = () => {
|
|
||||||
checkFieldAccess();
|
|
||||||
};
|
|
||||||
|
|
||||||
return {
|
|
||||||
canViewField,
|
|
||||||
loading,
|
|
||||||
refresh
|
|
||||||
};
|
|
||||||
}
|
|
||||||
@@ -24,7 +24,6 @@ import { profileEditSchema } from '@/lib/validation';
|
|||||||
import { LocationDisplay } from '@/components/profile/LocationDisplay';
|
import { LocationDisplay } from '@/components/profile/LocationDisplay';
|
||||||
import { UserBlockButton } from '@/components/profile/UserBlockButton';
|
import { UserBlockButton } from '@/components/profile/UserBlockButton';
|
||||||
import { PersonalLocationDisplay } from '@/components/profile/PersonalLocationDisplay';
|
import { PersonalLocationDisplay } from '@/components/profile/PersonalLocationDisplay';
|
||||||
import { useProfileFieldAccess } from '@/hooks/useProfileFieldAccess';
|
|
||||||
import { useUserRole } from '@/hooks/useUserRole';
|
import { useUserRole } from '@/hooks/useUserRole';
|
||||||
|
|
||||||
export default function Profile() {
|
export default function Profile() {
|
||||||
@@ -60,9 +59,6 @@ export default function Profile() {
|
|||||||
const [recentActivity, setRecentActivity] = useState<any[]>([]);
|
const [recentActivity, setRecentActivity] = useState<any[]>([]);
|
||||||
const [activityLoading, setActivityLoading] = useState(false);
|
const [activityLoading, setActivityLoading] = useState(false);
|
||||||
|
|
||||||
// Profile field access checking
|
|
||||||
const { canViewField, loading: fieldAccessLoading } = useProfileFieldAccess(profile?.user_id);
|
|
||||||
|
|
||||||
// User role checking
|
// User role checking
|
||||||
const { isModerator, loading: rolesLoading } = useUserRole();
|
const { isModerator, loading: rolesLoading } = useUserRole();
|
||||||
|
|
||||||
@@ -489,7 +485,7 @@ export default function Profile() {
|
|||||||
variant="avatar"
|
variant="avatar"
|
||||||
maxFiles={1}
|
maxFiles={1}
|
||||||
maxSizeMB={1}
|
maxSizeMB={1}
|
||||||
existingPhotos={canViewField('avatar_url') && profile.avatar_url ? [profile.avatar_url] : []}
|
existingPhotos={profile.avatar_url ? [profile.avatar_url] : []}
|
||||||
onUploadComplete={handleAvatarUpload}
|
onUploadComplete={handleAvatarUpload}
|
||||||
currentImageId={avatarImageId}
|
currentImageId={avatarImageId}
|
||||||
onError={error => {
|
onError={error => {
|
||||||
@@ -577,7 +573,7 @@ export default function Profile() {
|
|||||||
{profile.display_name && <Badge variant="secondary" className="cursor-pointer hover:bg-secondary/80" onClick={() => navigate(`/profile/${profile.username}`)}>@{profile.username}</Badge>}
|
{profile.display_name && <Badge variant="secondary" className="cursor-pointer hover:bg-secondary/80" onClick={() => navigate(`/profile/${profile.username}`)}>@{profile.username}</Badge>}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{canViewField('bio') && profile.bio && (
|
{profile.bio && (
|
||||||
<p className="text-muted-foreground mb-4 max-w-2xl">
|
<p className="text-muted-foreground mb-4 max-w-2xl">
|
||||||
{profile.bio}
|
{profile.bio}
|
||||||
</p>
|
</p>
|
||||||
@@ -592,31 +588,23 @@ export default function Profile() {
|
|||||||
})}
|
})}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{/* Show pronouns if enabled and privacy allows */}
|
{/* Show pronouns if enabled and present (privacy already enforced by get_filtered_profile) */}
|
||||||
{profile.show_pronouns && canViewField('preferred_pronouns') && profile.preferred_pronouns && (
|
{profile.show_pronouns && profile.preferred_pronouns && (
|
||||||
<div className="flex items-center gap-1">
|
<div className="flex items-center gap-1">
|
||||||
<User className="w-4 h-4" />
|
<User className="w-4 h-4" />
|
||||||
{profile.preferred_pronouns}
|
{profile.preferred_pronouns}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{/* Show personal location if available and privacy allows */}
|
{/* Show personal location (privacy already enforced by get_filtered_profile) */}
|
||||||
{canViewField('personal_location') && profile.personal_location && (
|
|
||||||
<PersonalLocationDisplay
|
<PersonalLocationDisplay
|
||||||
personalLocation={profile.personal_location}
|
personalLocation={profile.personal_location}
|
||||||
userId={profile.user_id}
|
|
||||||
isOwnProfile={isOwnProfile}
|
|
||||||
/>
|
/>
|
||||||
)}
|
|
||||||
|
|
||||||
{/* Show location only if privacy allows */}
|
{/* Show location (privacy already enforced by get_filtered_profile) */}
|
||||||
{canViewField('location_id') && profile.location && (
|
|
||||||
<LocationDisplay
|
<LocationDisplay
|
||||||
location={profile.location}
|
location={profile.location}
|
||||||
userId={profile.user_id}
|
|
||||||
isOwnProfile={isOwnProfile}
|
|
||||||
/>
|
/>
|
||||||
)}
|
|
||||||
</div>
|
</div>
|
||||||
</div>}
|
</div>}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
Reference in New Issue
Block a user