S
SolidJS•2mo ago
ChrisThornham

Help Me Understand "use server" Security

I'm new to this topic. So please excuse me if my questions if they're obvious or don't sound well thought out. I'm simply trying to learn. Let's assume I have an app dashboard. When a user visits that dashboard, I run a createEffect() to check their access status. Inside that "client-side" createEffect() I call a server function getUserRecord() Here's a simplified example to show logic. 
  // Get the userId and email of logged in user
  const supabaseAuthContext = useSupabaseAuthContext()!;
  const userId = supabaseAuthContext.supabaseSession?.user.id;
  const userEmail = supabaseAuthContext.supabaseSession?.user.email;

  createEffect(async () => {
    if (userId && userEmail) {
      const userRecord = await getUserRecord(userId);      
      if (userRecord) {
        if (userRecord.has_app_access) {
          // let them access the app
        } else {
          // Navigate away
        }
      }
    }
  });
  // Get the userId and email of logged in user
  const supabaseAuthContext = useSupabaseAuthContext()!;
  const userId = supabaseAuthContext.supabaseSession?.user.id;
  const userEmail = supabaseAuthContext.supabaseSession?.user.email;

  createEffect(async () => {
    if (userId && userEmail) {
      const userRecord = await getUserRecord(userId);      
      if (userRecord) {
        if (userRecord.has_app_access) {
          // let them access the app
        } else {
          // Navigate away
        }
      }
    }
  });
Here's the getUserRecord() function: 
export async function getUserRecord(userId: string) {
  "use server";
  // Get Supabase Admin Client
  const supabaseAdmin = getSupabaseAdminClient();

  if (supabaseAdmin) {
    // Query supabase
    const { data, error } = await supabaseAdmin
      .from("users")
      .select()
      .eq("auth_user_id", userId);

    if (error) {
      console.log(error.message);
    }
    return data;
  }
}
export async function getUserRecord(userId: string) {
  "use server";
  // Get Supabase Admin Client
  const supabaseAdmin = getSupabaseAdminClient();

  if (supabaseAdmin) {
    // Query supabase
    const { data, error } = await supabaseAdmin
      .from("users")
      .select()
      .eq("auth_user_id", userId);

    if (error) {
      console.log(error.message);
    }
    return data;
  }
}
For context, here's the getSupabaseAdminClient() function: 
export function getSupabaseAdminClient() {
  "use server";

  // Get the environment variables
  const projectURL = process.env.SUPABASE_PROJECT_URL;
  const supabaseServiceRole = process.env.SUPABASE_SERVICE_ROLE;
  if (!projectURL || !supabaseServiceRole) {
    // Log error
    console.error("Unable to initialize Supabase Auth Admin Client");
    return;
  }

  return createClient(projectURL, supabaseServiceRole, {
    auth: {
      autoRefreshToken: false,
      persistSession: false,
    },
  });
}
export function getSupabaseAdminClient() {
  "use server";

  // Get the environment variables
  const projectURL = process.env.SUPABASE_PROJECT_URL;
  const supabaseServiceRole = process.env.SUPABASE_SERVICE_ROLE;
  if (!projectURL || !supabaseServiceRole) {
    // Log error
    console.error("Unable to initialize Supabase Auth Admin Client");
    return;
  }

  return createClient(projectURL, supabaseServiceRole, {
    auth: {
      autoRefreshToken: false,
      persistSession: false,
    },
  });
}
31 Replies
ChrisThornham
ChrisThornham•2mo ago
Here's the question: Technically, I'm calling the getUserRecord() function from the client... which leads me to believe that the getUserRecord() function is exposed to the client. Therefore, I'm wondering if it is possible for someone to repeatedly call getUserRecord() from the client? Can they open Chrome dev tools or hack into something to gain access to getUserRecord() and run it from the client with different variables? This might not be a huge concern when I'm reading data, but this could quickly become an issue if exposed a server function that wrote data to my database. Am I thinking about this correctly? If so, how do I securely call server function from the client?
Brendonovich
Brendonovich•2mo ago
Therefore, is it possible for someone to repeatedly call getUserRecord() from the client
Yes, any use server function can be called from the client
Can they open Chrome dev tools or hack into something to gain access to getUserRecord() and run it from the client with different variables?
Yes, they'll be able to supply their own userId value
If so, how do I securely call server function from the client?
Get an auth token into the server function and authenticate the user in there. Whether you do that with cookies or by passing it as an argument is up to you
ChrisThornham
ChrisThornham•2mo ago
Ok... if I pass an auth token to my server function, couldn't any hacker just sign up for an account and pass that check?
Brendonovich
Brendonovich•2mo ago
For the case of getUserRecord i assume you'd want it to get the data of the current user, so use the auth token to figure out the user id to use instead of passing it as an argument
ChrisThornham
ChrisThornham•2mo ago
Ok that makes sense. What about this: Take my original logic one step further: 
  createEffect(async () => {
    // Check to see if a user exists and whether or not they have access
    if (userId && userEmail) {
      // Query supabase for a user record.
      const userRecord = await getUserRecord(userId);      

      if (userRecord) {
        if (userRecord.length > 0) {
          // User record exist. Set access privileges
          setHasPaymentAccess(userRecord[0].has_payment_access);
          setHasSubscriptionAccess(userRecord[0].has_subscription_access);
        } else {
          // No user exists. Create a new user record.
          await createUserRecord(userId, userEmail);
        }
      }
    }
  }}:
  createEffect(async () => {
    // Check to see if a user exists and whether or not they have access
    if (userId && userEmail) {
      // Query supabase for a user record.
      const userRecord = await getUserRecord(userId);      

      if (userRecord) {
        if (userRecord.length > 0) {
          // User record exist. Set access privileges
          setHasPaymentAccess(userRecord[0].has_payment_access);
          setHasSubscriptionAccess(userRecord[0].has_subscription_access);
        } else {
          // No user exists. Create a new user record.
          await createUserRecord(userId, userEmail);
        }
      }
    }
  }}:
Now I'm exposing createUserRecord() to the client. 
export async function createUserRecord(userId: string, email: string) {
  "use server";
  // Create a user Data Object
  const userData = {
    auth_user_id: userId,
    customer_email: email,
  };

  // Get Supabase Admin Client
  const supabaseAdmin = getSupabaseAdminClient();
  if (supabaseAdmin) {
    // Add User Record
    const { error } = await supabaseAdmin.from("users").insert(userData);
    if (error) {
      throw new Error(`Failed to insert user record: ${error.message}`);
    }
  }
}
export async function createUserRecord(userId: string, email: string) {
  "use server";
  // Create a user Data Object
  const userData = {
    auth_user_id: userId,
    customer_email: email,
  };

  // Get Supabase Admin Client
  const supabaseAdmin = getSupabaseAdminClient();
  if (supabaseAdmin) {
    // Add User Record
    const { error } = await supabaseAdmin.from("users").insert(userData);
    if (error) {
      throw new Error(`Failed to insert user record: ${error.message}`);
    }
  }
}
If this function is exposed to the client, what prevents a hacker from continually writing to my DB? Would I add a "unique" clause to my user table to prevent multiple rows with the same userId?
Brendonovich
Brendonovich•2mo ago
You'd probably want to make emails unique and personally i'd generate user IDs on the server instead of client Since signups are a public thing you'll probably want rate limiting at some point And you'll probably want an email verification step at some point userId should also be unique though yeah
ChrisThornham
ChrisThornham•2mo ago
Boy... apps get complicated the further you go, don't they? You think you have something all figured out... then you start thinking and realize there's another hole somewhere. I've got some homework to do. Last question... Let's say I'm using a cache() and load() function from the router like this: 
// CACHE ============================================================
const getOrders = cache(async (): Promise<Order[] | undefined> => {
  "use server";
  try {
    // Get Supabase Admin Client
    const supabase = getSupabaseAdminClient();
    if (supabase) {
      const { data, error } = await supabase.from("orders").select();

      if (error) {
        console.log("Error fetching orders:", error.message);
        return undefined;
      }

      if (!data) {
        console.log("No orders found");
        return undefined;
      }
      return data as Order[];
    }
  } catch (err) {
    console.log("Unexpected error fetching orders:", err);
    return undefined;
  }
}, "orders");

// LOADER ===========================================================
export const route = {
  load() {
    void getOrders();
  },
};

export default function OrdersPage() {
  const orders = createAsync(() => getOrders());
  return (
    <>
      <AdminDashboardLayout>
        <h4>Orders</h4>
        <hr class="my-10" />
        <Show when={orders()} fallback="No orders found.">
          {(orders) => <OrdersTable orders={orders} />}
        </Show>
      </AdminDashboardLayout>
    </>
  );
}
// CACHE ============================================================
const getOrders = cache(async (): Promise<Order[] | undefined> => {
  "use server";
  try {
    // Get Supabase Admin Client
    const supabase = getSupabaseAdminClient();
    if (supabase) {
      const { data, error } = await supabase.from("orders").select();

      if (error) {
        console.log("Error fetching orders:", error.message);
        return undefined;
      }

      if (!data) {
        console.log("No orders found");
        return undefined;
      }
      return data as Order[];
    }
  } catch (err) {
    console.log("Unexpected error fetching orders:", err);
    return undefined;
  }
}, "orders");

// LOADER ===========================================================
export const route = {
  load() {
    void getOrders();
  },
};

export default function OrdersPage() {
  const orders = createAsync(() => getOrders());
  return (
    <>
      <AdminDashboardLayout>
        <h4>Orders</h4>
        <hr class="my-10" />
        <Show when={orders()} fallback="No orders found.">
          {(orders) => <OrdersTable orders={orders} />}
        </Show>
      </AdminDashboardLayout>
    </>
  );
}
Is getOrders() exposed to the client, even if it's driven by the solid router? I think it is, right?
Brendonovich
Brendonovich•2mo ago
Yes since there's a use server there Solid router has basically no knowledge of server functions, it'll just use what you give it
ChrisThornham
ChrisThornham•2mo ago
So a hacker get simply call getOrders() can get all of the orders?
Brendonovich
Brendonovich•2mo ago
Yeah
ChrisThornham
ChrisThornham•2mo ago
So how do people get order securely?
Brendonovich
Brendonovich•2mo ago
Auth token
ChrisThornham
ChrisThornham•2mo ago
Can you point me in a direction? Where can I read about this?
Brendonovich
Brendonovich•2mo ago
I assume supabase would have some examples, but the basic premise is that you get the auth token into the function, whether with cookies or as an argument or otherwise, and then make sure the auth token belongs to someone who is an admin Whether that's by fetching the user info from your database or validatin the auth token itself depends on the implementation
ChrisThornham
ChrisThornham•2mo ago
Ok... That's a great start. Thank you for all of the help with this. I see a full day or two ahead of me. Have a great night.
Want results from more Discord servers?
Add your server