delay / sveltekit-auth-starter

This is a Sveltekit auth starter project. It utilizes Lucia for authentication, Skeleton for ui elements, Prisma for database connectivity and type safety, Lucide for icons, inlang for translation, Zod and Superforms to handle forms and validation and Sveltekit.
https://sveltekit-auth-starter.vercel.app
MIT License
215 stars 31 forks source link

Serverside data load on protected route while logged out #5

Closed RobinMaas95 closed 1 year ago

RobinMaas95 commented 1 year ago

tldr: #6 should fix this issue

I'm not sure if this is a security concern because - at least from what I can see - the loaded data are not returned to the client-side so they are safe, but at least it opens the door for kind of unnecessary data loads and potential load on your internal systems or costs for usage of external apis.

From my understanding it's the kinda the issue that is discussed in this issue. The problem is, that all load functions run in parallel. So at long as you keep se simple structure of the current protected route, everything is fine.

But when you add an +page.server.ts file inside of the protected route where you access some external data sources in the load function, this access run in parallel to the load function inside of +layout.server.ts. In the end the data are not returned to the client (at least I didn't found them client side) because the auth check prevents the user from accessing the +page.svelte file where the loaded data are passed into via prop. But the fetch call still happens (which can be annoying in case of system load or costly in case of external apis with usage fees per call).

To have an example of the issue, you simple can add the file src/routes/protected/+page.server.ts with the following code (simple fetch and a log message we can see on the server side):

import type { PageServerLoad } from './$types';

export const load: PageServerLoad = async () => {
    const getCustomers = async () => {
        console.log('Fetched customers from database');
        const res = await fetch(
            'https://dummyjson.com/users?limit=10&skip-10&select=firstName,lastName,id,email'
        );

        const customers = await res.json();
        return customers.users;
    };

    const customers = await getCustomers();

    return {
        props: {
            customers: customers
        }
    };
};

To see the date in case of an authorised user, you also can change src/routes/protected/+page.svelte so that you accept PageData inside the script block:

import type { PageData } from './$types';
export let data: PageData;

and display them in the body:

{#if data}
<pre>{JSON.stringify(data.props.customers, null, 2)}</pre>
{:else}
// This should not be reachable
<p>There is no data.</p>
{/if}

With this in place, data are loaded as soon as you hover over the Protected link inside of the menu. You can see the log "Fetched customers from database" (server side). This gif shows the behaviour:

To fix this problem, we can move the redirects from the layout.svelte.ts inside of the protected route into hooks.server.ts. See #6 for the implementation details.

delay commented 1 year ago

Thanks so much for the detailed write up! When I originally wrote the +hooks.server.ts, it didn't really sit right that it was the best way to handle the protection. I had actually seen that github post you mentioned in the past when working on a custom auth system and forgot to consider it when using lucia auth in this project. I was following their recommendations for how to protect the route.

You are completely right and thanks for the example and recommended corrections! I will approve the pull request but I will remove the example exploit code you added. Thanks so much!