Merge pull request #74 from marmelab/sales-policies

fzaninotto · web-flow · commit c2bcf5135d48 · 2024-11-08T13:12:17.000+01:00
Add more secure RLS policies on the sales table
diff --git a/src/providers/supabase/dataProvider.ts b/src/providers/supabase/dataProvider.ts
@@ -147,7 +147,14 @@ const dataProviderWithCustomMethods = {
         id: Identifier,
         data: Partial<Omit<SalesFormData, 'password'>>
     ) {
-        const { email, first_name, last_name, administrator, disabled } = data;
+        const {
+            email,
+            first_name,
+            last_name,
+            administrator,
+            avatar,
+            disabled,
+        } = data;
 
         const { data: sale, error } = await supabase.functions.invoke<Sale>(
             'users',
@@ -160,6 +167,7 @@ const dataProviderWithCustomMethods = {
                     last_name,
                     administrator,
                     disabled,
+                    avatar,
                 },
             }
         );
diff --git a/src/sales/SalesList.tsx b/src/sales/SalesList.tsx
@@ -52,7 +52,7 @@ export function SalesList() {
                 actions={<SalesListActions />}
                 sort={{ field: 'first_name', order: 'ASC' }}
             >
-                <DatagridConfigurable rowClick="edit">
+                <DatagridConfigurable rowClick="edit" bulkActionButtons={false}>
                     <TextField source="first_name" />
                     <TextField source="last_name" />
                     <TextField source="email" />
diff --git a/src/settings/SettingsPage.tsx b/src/settings/SettingsPage.tsx
@@ -20,12 +20,12 @@ import {
     useGetIdentity,
     useGetOne,
     useNotify,
-    useUpdate,
+    useRecordContext,
 } from 'react-admin';
 import { useFormState } from 'react-hook-form';
 import ImageEditorField from '../misc/ImageEditorField';
 import { CrmDataProvider } from '../providers/types';
-import { SalesFormData } from '../types';
+import { Sale, SalesFormData } from '../types';
 import { useMutation } from '@tanstack/react-query';
 import ContentCopyIcon from '@mui/icons-material/ContentCopy';
 
@@ -84,13 +84,13 @@ const SettingsForm = ({
     isEditMode: boolean;
     setEditMode: (value: boolean) => void;
 }) => {
-    const [update] = useUpdate();
     const notify = useNotify();
+    const record = useRecordContext<Sale>();
     const { identity, refetch } = useGetIdentity();
     const { isDirty } = useFormState();
     const dataProvider = useDataProvider<CrmDataProvider>();
 
-    const { mutate } = useMutation({
+    const { mutate: updatePassword } = useMutation({
         mutationKey: ['updatePassword'],
         mutationFn: async () => {
             if (!identity) {
@@ -110,33 +110,30 @@ const SettingsForm = ({
         },
     });
 
+    const { mutate: mutateSale } = useMutation({
+        mutationKey: ['signup'],
+        mutationFn: async (data: SalesFormData) => {
+            if (!record) {
+                throw new Error('Record not found');
+            }
+            return dataProvider.salesUpdate(record.id, data);
+        },
+        onSuccess: () => {
+            refetch();
+            notify('Your profile has been updated');
+        },
+        onError: () => {
+            notify('An error occurred. Please try again.');
+        },
+    });
     if (!identity) return null;
 
     const handleClickOpenPasswordChange = () => {
-        mutate();
+        updatePassword();
     };
 
     const handleAvatarUpdate = async (values: any) => {
-        await update(
-            'sales',
-            {
-                id: identity.id,
-                data: values,
-                previousData: identity,
-            },
-            {
-                onSuccess: () => {
-                    refetch();
-                    setEditMode(false);
-                    notify('Your profile has been updated');
-                },
-                onError: _ => {
-                    notify('An error occurred. Please try again', {
-                        type: 'error',
-                    });
-                },
-            }
-        );
+        mutateSale(values);
     };
 
     return (
diff --git a/src/types.ts b/src/types.ts
@@ -16,6 +16,7 @@ export type SignUpData = {
 };
 
 export type SalesFormData = {
+    avatar: string;
     email: string;
     password: string;
     first_name: string;
diff --git a/supabase/functions/users/index.ts b/supabase/functions/users/index.ts
@@ -21,16 +21,34 @@ async function updateSaleAdministrator(
         .select('*');
 
     if (!sales?.length || salesError) {
-        console.error('Error inviting user:', salesError);
+        console.error('Error updating user:', salesError);
         throw salesError ?? new Error('Failed to update sale');
     }
     return sales.at(0);
 }
 
-async function inviteUser(req: Request) {
+async function updateSaleAvatar(user_id: string, avatar: string) {
+    const { data: sales, error: salesError } = await supabaseAdmin
+        .from('sales')
+        .update({ avatar })
+        .eq('user_id', user_id)
+        .select('*');
+
+    if (!sales?.length || salesError) {
+        console.error('Error updating user:', salesError);
+        throw salesError ?? new Error('Failed to update sale');
+    }
+    return sales.at(0);
+}
+
+async function inviteUser(req: Request, currentUserSale: any) {
     const { email, password, first_name, last_name, disabled, administrator } =
         await req.json();
 
+    if (!currentUserSale.administrator) {
+        return createErrorResponse(401, 'Not Authorized');
+    }
+
     const { data, error: userError } =
         await supabaseAdmin.auth.admin.createUser({
             email,
@@ -69,9 +87,16 @@ async function inviteUser(req: Request) {
     }
 }
 
-async function patchUser(req: Request) {
-    const { sales_id, email, first_name, last_name, administrator, disabled } =
-        await req.json();
+async function patchUser(req: Request, currentUserSale: any) {
+    const {
+        sales_id,
+        email,
+        first_name,
+        last_name,
+        avatar,
+        administrator,
+        disabled,
+    } = await req.json();
     const { data: sale } = await supabaseAdmin
         .from('sales')
         .select('*')
@@ -82,6 +107,11 @@ async function patchUser(req: Request) {
         return createErrorResponse(404, 'Not Found');
     }
 
+    // Users can only update their own profile unless they are an administrator
+    if (!currentUserSale.administrator && currentUserSale.id !== sale.id) {
+        return createErrorResponse(401, 'Not Authorized');
+    }
+
     const { data, error: userError } =
         await supabaseAdmin.auth.admin.updateUserById(sale.user_id, {
             email,
@@ -94,16 +124,42 @@ async function patchUser(req: Request) {
         return createErrorResponse(500, 'Internal Server Error');
     }
 
+    if (avatar) {
+        await updateSaleAvatar(data.user.id, avatar);
+    }
+
+    // Only administrators can update the administrator and disabled status
+    if (!currentUserSale.administrator) {
+        const { data: new_sale } = await supabaseAdmin
+            .from('sales')
+            .select('*')
+            .eq('id', sales_id)
+            .single();
+        return new Response(
+            JSON.stringify({
+                data: new_sale,
+            }),
+            {
+                headers: {
+                    'Content-Type': 'application/json',
+                    ...corsHeaders,
+                },
+            }
+        );
+    }
+
     try {
         await updateSaleDisabled(data.user.id, disabled);
         const sale = await updateSaleAdministrator(data.user.id, administrator);
-
         return new Response(
             JSON.stringify({
                 data: sale,
             }),
             {
-                headers: { 'Content-Type': 'application/json', ...corsHeaders },
+                headers: {
+                    'Content-Type': 'application/json',
+                    ...corsHeaders,
+                },
             }
         );
     } catch (e) {
@@ -126,18 +182,25 @@ Deno.serve(async (req: Request) => {
         Deno.env.get('SUPABASE_ANON_KEY') ?? '',
         { global: { headers: { Authorization: authHeader } } }
     );
-
     const { data } = await localClient.auth.getUser();
     if (!data?.user) {
         return createErrorResponse(401, 'Unauthorized');
     }
+    const currentUserSale = await supabaseAdmin
+        .from('sales')
+        .select('*')
+        .eq('user_id', data.user.id)
+        .single();
 
+    if (!currentUserSale?.data) {
+        return createErrorResponse(401, 'Unauthorized');
+    }
     if (req.method === 'POST') {
-        return inviteUser(req);
+        return inviteUser(req, currentUserSale.data);
     }
 
     if (req.method === 'PATCH') {
-        return patchUser(req);
+        return patchUser(req, currentUserSale.data);
     }
 
     return createErrorResponse(405, 'Method Not Allowed');
diff --git a/supabase/migrations/20241104153231_sales_policies.sql b/supabase/migrations/20241104153231_sales_policies.sql
@@ -0,0 +1,7 @@
+create schema if not exists "private";
+
+set check_function_bodies = off;
+
+drop policy "Enable insert for authenticated users only" on "public"."sales";
+
+drop policy "Enable update for authenticated users only" on "public"."sales";

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ export function SalesList() {`
`52`	`52`	`actions={<SalesListActions />}`
`53`	`53`	`sort={{ field: 'first_name', order: 'ASC' }}`
`54`	`54`	`>`
`55`		`- <DatagridConfigurable rowClick="edit">`
	`55`	`+ <DatagridConfigurable rowClick="edit" bulkActionButtons={false}>`
`56`	`56`	`<TextField source="first_name" />`
`57`	`57`	`<TextField source="last_name" />`
`58`	`58`	`<TextField source="email" />`