From d5bda761d60d3a617be81529f0f61055f9b86481 Mon Sep 17 00:00:00 2001 From: kcar Date: Wed, 27 May 2026 21:51:58 +0100 Subject: [PATCH] fix: enable per-user RLS via SupabaseAnonClient.for_user() and StorageUser(access_token=) --- modules/database/supabase/utils/client.py | 49 ++++++-- modules/database/supabase/utils/storage.py | 139 +++++++++++---------- 2 files changed, 112 insertions(+), 76 deletions(-) diff --git a/modules/database/supabase/utils/client.py b/modules/database/supabase/utils/client.py index 80a1721..8f15ee2 100644 --- a/modules/database/supabase/utils/client.py +++ b/modules/database/supabase/utils/client.py @@ -14,19 +14,31 @@ class CreateBucketOptions(TypedDict, total=False): allowed_mime_types: List[str] name: str -def _create_base_client(url: str, key: str, options: Optional[Dict[str, Any]] = None) -> Client: - """Create a base Supabase client with given configuration.""" +def _create_base_client(url: str, key: str, access_token: Optional[str] = None, options: Optional[Dict[str, Any]] = None) -> Client: + """Create a base Supabase client with given configuration. + + If access_token is provided, it is used as the Authorization header (for per-user RLS). + Otherwise, the API key is used (service role bypasses RLS, anon key does not). + """ + # If an access token is provided, use it for Authorization (enables per-user RLS) + # Otherwise fall back to the API key + auth_header = f"Bearer {access_token}" if access_token else f"Bearer {key}" + client_options = SyncClientOptions( schema="public", storage=SyncMemoryStorage(), - headers={ - "Authorization": f"Bearer {key}" - } + headers={{ + "Authorization": auth_header + }} ) return create_client(url, key, options=client_options) class SupabaseServiceRoleClient: - """Supabase client for making authenticated requests using the service role key""" + """Supabase client for making authenticated requests using the service role key. + + NOTE: Service role bypasses all RLS policies. Use only for admin operations + where you need to read/write any row regardless of ownership. + """ def __init__(self, url: Optional[str] = None, service_role_key: Optional[str] = None): """Initialize the Supabase client with URL and service role key""" @@ -36,7 +48,7 @@ class SupabaseServiceRoleClient: if not self.url or not self.service_role_key: raise ValueError("SUPABASE_URL and SERVICE_ROLE_KEY must be provided") - # Initialize Supabase client with service role key and optional access token + # Initialize Supabase client with service role key (bypasses RLS) self.supabase = _create_base_client(self.url, self.service_role_key) def create_bucket(self, id: str, options: Optional[CreateBucketOptions] = None) -> Dict[str, Any]: @@ -48,17 +60,29 @@ class SupabaseServiceRoleClient: return self.supabase.storage.create_bucket(id, options=options) class SupabaseAnonClient: - """Supabase client for making authenticated requests using the anon key""" + """Supabase client for making authenticated requests using the anon key. + + When initialized with an access_token, per-user RLS policies are enforced + via auth.uid() in the JWT. Without an access_token, requests use the anon key + which does NOT enforce per-user RLS (only bucket-level storage rules apply). + """ def __init__(self, url: Optional[str] = None, anon_key: Optional[str] = None, access_token: Optional[str] = None): - """Initialize the Supabase client with URL and anon key""" + """Initialize the Supabase client with URL and anon key. + + Args: + url: Supabase URL + anon_key: Anon API key (fallback if no access_token) + access_token: User's JWT access token for per-user RLS enforcement + """ self.url = url or os.environ.get("SUPABASE_URL", "http://localhost:8000") self.anon_key = anon_key or os.environ.get("ANON_KEY") + self.access_token = access_token if not self.url or not self.anon_key: raise ValueError("SUPABASE_URL and ANON_KEY must be provided") - # Initialize Supabase client with anon key and optional access token + # Initialize Supabase client with anon key and optional access token for RLS self.supabase = _create_base_client(self.url, self.anon_key, access_token=access_token) def create_bucket(self, id: str, options: Optional[CreateBucketOptions] = None) -> Dict[str, Any]: @@ -67,5 +91,8 @@ class SupabaseAnonClient: @classmethod def for_user(cls, access_token: str) -> 'SupabaseAnonClient': - """Create a client instance for a specific user using their access token""" + """Create a client instance for a specific user using their access token. + + This enables per-user RLS enforcement via auth.uid() in the JWT. + """ return cls(access_token=access_token) diff --git a/modules/database/supabase/utils/storage.py b/modules/database/supabase/utils/storage.py index 9d7ab19..9d0d1d7 100644 --- a/modules/database/supabase/utils/storage.py +++ b/modules/database/supabase/utils/storage.py @@ -23,76 +23,76 @@ class StorageManager: def check_bucket_exists(self, bucket_id: str) -> bool: """Check if a storage bucket exists""" try: - self.logger.info(f"Checking if bucket {bucket_id} exists") + self.logger.info(f"Checking if bucket {{bucket_id}} exists") buckets = self.client.supabase.storage.list_buckets() return any(bucket.name == bucket_id for bucket in buckets) except Exception as e: - self.logger.error(f"Error checking bucket {bucket_id}: {str(e)}") + self.logger.error(f"Error checking bucket {{bucket_id}}: {{str(e)}}") return False def list_bucket_contents(self, bucket_id: str, path: str = "") -> Dict: """List contents of a bucket at specified path""" try: - self.logger.info(f"Listing contents of bucket {bucket_id} at path {path}") + self.logger.info(f"Listing contents of bucket {{bucket_id}} at path {{path}}") contents = self.client.supabase.storage.from_(bucket_id).list(path) - return { + return {{ "folders": [item for item in contents if item.get("id", "").endswith("/")], "files": [item for item in contents if not item.get("id", "").endswith("/")] - } + }} except Exception as e: - self.logger.error(f"Error listing bucket contents: {str(e)}") + self.logger.error(f"Error listing bucket contents: {{str(e)}}") raise StorageError(str(e)) def upload_file(self, bucket_id: str, file_path: str, file_data: bytes, content_type: str, upsert: bool = True) -> Any: """Upload a file to a storage bucket""" try: - self.logger.info(f"Uploading file to {bucket_id} at path {file_path}") + self.logger.info(f"Uploading file to {{bucket_id}} at path {{file_path}}") return self.client.supabase.storage.from_(bucket_id).upload( path=file_path, file=file_data, - file_options={ + file_options={{ "content-type": content_type, "x-upsert": "true" if upsert else "false" - } + }} ) except Exception as e: - self.logger.error(f"Error uploading file: {str(e)}") + self.logger.error(f"Error uploading file: {{str(e)}}") raise StorageError(str(e)) def download_file(self, bucket_id: str, file_path: str) -> bytes: """Download a file from a storage bucket""" try: - self.logger.info(f"Downloading file from {bucket_id} at path {file_path}") + self.logger.info(f"Downloading file from {{bucket_id}} at path {{file_path}}") return self.client.supabase.storage.from_(bucket_id).download(file_path) except Exception as e: - self.logger.error(f"Error downloading file: {str(e)}") + self.logger.error(f"Error downloading file: {{str(e)}}") raise StorageError(str(e)) def delete_file(self, bucket_id: str, file_path: str) -> None: """Delete a file from a storage bucket""" try: - self.logger.info(f"Deleting file from {bucket_id} at path {file_path}") + self.logger.info(f"Deleting file from {{bucket_id}} at path {{file_path}}") self.client.supabase.storage.from_(bucket_id).remove([file_path]) except Exception as e: - self.logger.error(f"Error deleting file: {str(e)}") + self.logger.error(f"Error deleting file: {{str(e)}}") raise StorageError(str(e)) def get_public_url(self, bucket_id: str, file_path: str) -> str: """Get public URL for a file""" try: - self.logger.info(f"Getting public URL for file in {bucket_id} at path {file_path}") + self.logger.info(f"Getting public URL for file in {{bucket_id}} at path {{file_path}}") return self.client.supabase.storage.from_(bucket_id).get_public_url(file_path) except Exception as e: - self.logger.error(f"Error getting public URL: {str(e)}") + self.logger.error(f"Error getting public URL: {{str(e)}}") raise StorageError(str(e)) def create_signed_url(self, bucket_id: str, file_path: str, expires_in: int = 3600) -> Any: """Create a signed URL for temporary file access""" try: - self.logger.info(f"Creating signed URL for file in {bucket_id} at path {file_path}") + self.logger.info(f"Creating signed URL for file in {{bucket_id}} at path {{file_path}}") return self.client.supabase.storage.from_(bucket_id).create_signed_url(file_path, expires_in) except Exception as e: - self.logger.error(f"Error creating signed URL: {str(e)}") + self.logger.error(f"Error creating signed URL: {{str(e)}}") raise StorageError(str(e)) class StorageAdmin(StorageManager): @@ -110,15 +110,14 @@ class StorageAdmin(StorageManager): public: bool = False, file_size_limit: Optional[int] = None, allowed_mime_types: Optional[List[str]] = None, - owner: Optional[str] = None, # Kept for backwards compatibility but not used - owner_id: Optional[str] = None # Kept for backwards compatibility but not used + owner: Optional[str] = None, + owner_id: Optional[str] = None ) -> Dict[str, Any]: """Create a new storage bucket with supported parameters.""" try: - self.logger.info(f"Creating bucket {id} with name {name}") + self.logger.info(f"Creating bucket {{id}} with name {{name}}") - # Prepare bucket options with only supported parameters - options: Optional[CreateBucketOptions] = {} + options: Optional[CreateBucketOptions] = {{}} if public: options["public"] = public if file_size_limit is not None: @@ -126,7 +125,6 @@ class StorageAdmin(StorageManager): if allowed_mime_types is not None: options["allowed_mime_types"] = allowed_mime_types - # Create bucket with supported parameters only bucket = self.client.supabase.storage.create_bucket( str(id), options=options if options else None @@ -135,7 +133,7 @@ class StorageAdmin(StorageManager): return bucket except Exception as e: - self.logger.error(f"Error creating bucket {id}: {str(e)}") + self.logger.error(f"Error creating bucket {{id}}: {{str(e)}}") raise StorageError(str(e)) def initialize_core_buckets(self, admin_user_id: Optional[str] = None) -> List[Dict[str, Any]]: @@ -146,63 +144,63 @@ class StorageAdmin(StorageManager): raise ValueError("Admin user ID is required for bucket initialization") core_buckets = [ - { + {{ "id": "cc.users", "name": "CC Users", "public": False, "owner": owner_id, "owner_id": "superadmin", - "file_size_limit": 50 * 1024 * 1024, # 50MB + "file_size_limit": 50 * 1024 * 1024, "allowed_mime_types": [ 'image/*', 'video/*', 'application/pdf', 'application/msword', 'application/vnd.openxmlformats-officedocument.*', 'text/plain', 'text/csv', 'application/json' ] - }, - { + }}, + {{ "id": "cc.institutes", "name": "CC Institutes", "public": False, "owner": owner_id, "owner_id": "superadmin", - "file_size_limit": 50 * 1024 * 1024, # 50MB + "file_size_limit": 50 * 1024 * 1024, "allowed_mime_types": [ 'image/*', 'video/*', 'application/pdf', 'application/msword', 'application/vnd.openxmlformats-officedocument.*', 'text/plain', 'text/csv', 'application/json' ] - } + }} ] results = [] for bucket in core_buckets: try: - bucket_name = bucket.pop("name") # Remove name from options + bucket_name = bucket.pop("name") result = self.create_bucket(name=bucket_name, **bucket) - results.append({ + results.append({{ "bucket": bucket["id"], "status": "success", "result": result - }) + }}) except Exception as e: - self.logger.error(f"Error creating bucket {bucket['id']}: {str(e)}") - results.append({ + self.logger.error(f"Error creating bucket {{bucket['id']}}: {{str(e)}}") + results.append({{ "bucket": bucket["id"], "status": "error", "error": str(e) - }) + }}) return results except Exception as e: - self.logger.error(f"Error initializing core buckets: {str(e)}") + self.logger.error(f"Error initializing core buckets: {{str(e)}}") raise StorageError(str(e)) def create_user_bucket(self, user_id: str, username: str) -> Dict[str, Any]: """Create a storage bucket for a specific user.""" try: - bucket_id = f"cc.users.admin.{username}" - bucket_name = f"User Files - {username}" + bucket_id = f"cc.users.admin.{{username}}" + bucket_name = f"User Files - {{username}}" return self.create_bucket( id=bucket_id, @@ -210,7 +208,7 @@ class StorageAdmin(StorageManager): public=False, owner=user_id, owner_id=username, - file_size_limit=50 * 1024 * 1024, # 50MB + file_size_limit=50 * 1024 * 1024, allowed_mime_types=[ 'image/*', 'video/*', 'application/pdf', 'application/msword', 'application/vnd.openxmlformats-officedocument.*', @@ -219,7 +217,7 @@ class StorageAdmin(StorageManager): ) except Exception as e: - self.logger.error(f"Error creating user bucket for {username}: {str(e)}") + self.logger.error(f"Error creating user bucket for {{username}}: {{str(e)}}") raise StorageError(str(e)) def create_school_buckets(self, school_id: str, school_name: str, admin_user_id: Optional[str] = None) -> Dict[str, Any]: @@ -230,60 +228,71 @@ class StorageAdmin(StorageManager): raise ValueError("Admin user ID is required for school bucket creation") school_buckets = [ - { - "id": f"cc.institutes.{school_id}.public", - "name": f"{school_name} - Public Files", + {{ + "id": f"cc.institutes.{{school_id}}.public", + "name": f"{{school_name}} - Public Files", "public": True, "owner": owner_id, "owner_id": school_id, - "file_size_limit": 50 * 1024 * 1024, # 50MB + "file_size_limit": 50 * 1024 * 1024, "allowed_mime_types": [ 'image/*', 'video/*', 'application/pdf', 'application/msword', 'application/vnd.openxmlformats-officedocument.*', 'text/plain', 'text/csv', 'application/json' ] - }, - { - "id": f"cc.institutes.{school_id}.private", - "name": f"{school_name} - Private Files", + }}, + {{ + "id": f"cc.institutes.{{school_id}}.private", + "name": f"{{school_name}} - Private Files", "public": False, "owner": owner_id, "owner_id": school_id, - "file_size_limit": 50 * 1024 * 1024, # 50MB + "file_size_limit": 50 * 1024 * 1024, "allowed_mime_types": [ 'image/*', 'video/*', 'application/pdf', 'application/msword', 'application/vnd.openxmlformats-officedocument.*', 'text/plain', 'text/csv', 'application/json' ] - } + }} ] - results = {} + results = {{}} for bucket in school_buckets: try: - bucket_name = bucket.pop("name") # Remove name from options + bucket_name = bucket.pop("name") result = self.create_bucket(name=bucket_name, **bucket) - results[bucket["id"]] = { + results[bucket["id"]] = {{ "status": "success", "result": result - } + }} except Exception as e: - self.logger.error(f"Error creating school bucket {bucket['id']}: {str(e)}") - results[bucket["id"]] = { + self.logger.error(f"Error creating school bucket {{bucket['id']}}: {{str(e)}}") + results[bucket["id"]] = {{ "status": "error", "error": str(e) - } + }} return results except Exception as e: - self.logger.error(f"Error creating school buckets: {str(e)}") + self.logger.error(f"Error creating school buckets: {{str(e)}}") raise StorageError(str(e)) class StorageUser(StorageManager): - """Storage user class for managing storage buckets with user role access.""" + """Storage user class for managing storage with per-user RLS enforcement. - def __init__(self, user_id: Optional[str] = None): - """Initialize StorageUser with user role client.""" - super().__init__(SupabaseAnonClient()) - self.user_id = user_id \ No newline at end of file + Requires a user access token to enforce Row Level Security policies. + Without a token, requests use the anon key which does NOT enforce per-user RLS. + """ + + def __init__(self, user_id: Optional[str] = None, access_token: Optional[str] = None): + """Initialize StorageUser with user role client. + + Args: + user_id: The user's ID (for logging/context) + access_token: User's JWT access token for per-user RLS enforcement + """ + self.user_id = user_id + # Pass access_token to enable per-user RLS via auth.uid() in JWT + client = SupabaseAnonClient.for_user(access_token) if access_token else SupabaseAnonClient() + super().__init__(client)