Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Next.js server actions and components 1st-class Sanctum citizens #58

Closed
wants to merge 8 commits into from

Conversation

jared-cannon
Copy link

@jared-cannon jared-cannon commented Jan 18, 2025

The Problem

  1. Laravel Breeze/Sanctum does not have a clear way to use Next.js server actions, components, or route handlers. This leads to suboptimal implementations with security vulnerabilities and complex session management logic (more on this later).
  2. The composition of this starter kit de-opts every page out of SSR because the of its client component layout.

Why this matters

Next.js server actions, components, and route handlers run on a node js server. Cookies, therefore, are not automatically included in server-side requests, which breaks Sanctum's session management and CSRF protection.

There are questions throughout Stack Overflow and Reddit asking for help navigating this issue, and most developers do not have a deep enough understanding to effectively solve it. I have seen workarounds such as:

Unsafe Workarounds Seen on the Web

Generate a Sanctum PAT, store it as a cookie on the user's browser, then use it as a Bearer Token for all requests

OWASP nightmare, incredibly insecure.

Generate a Sanctum PAT, send it to the node js server, place it in an encrypted JWT, then send to the browser in an http-only cookie

More secure than the previous example, but still vulnerable to CSRF for non-get requests. This could technically be solved by a second CSRF token sent between the node js server and frontend, but this becomes tedious and error-prone. This also forces the user to manage two separate states for two different servers. It can be done, but it is a far cry from the simplicity promised by Sanctum.

Proposed Solution

Provide developers with a clear way to implement server-side functionality, and provide a component that handles authentication client-side without de-opting the app from SSR.

The new serverFetch server function forwards all cookies to the API, which maintains the user's session and CSRF protection provided by Sanctum.

The <UserRefresh /> client component is added outside of the (app) layout's children component tree, allowing developers to choose when to opt out of SSR while maintaining the user's session cookies.

Final Thoughts

This is a substantial and opinionated change. If this solution does not align with how Laravel wants to approach Next.js SSR, I understand. But I do believe this is a problem that needs to be solved to prevent the continued implementation of unsafe workarounds by the community.

…braries, add user check to email verification redirect, add serverFetch, and add User Refresh client component
@Willem-Jaap
Copy link
Contributor

Great to see this being addressed, I fully agree there are a lot of very unsafe examples of implementing Next.js with external API's. However I don't think is necessary to forward all data fetching to the server. Client side data fetching is very common e.g. with filtering and other highly interactive actions.

{A6F892FC-3D1A-4490-A2FA-7E36EE9DB25A}

I've previously moved all data fetching/mutations to server actions which then called a Laravel API and the latency was definely noticable.

I'm curious if you have any experience in this and how you solve this?

@jared-cannon
Copy link
Author

@Willem-Jaap

I agree, adding a second request layer is not ideal, and I think apps using this stack should generally stick to client requests sent directly to their Laravel API. However, there are times when it is beneficial to utilize Next.js server rendering at the expense of slightly higher latency. Vercel's documentation also pushes devs towards this paradigm, so adding clarity on how to do that with breeze/sanctum (while also documenting the drawbacks you've brought up here) would be beneficial.

Anecdotally, I use this approach for a couple lightweight fetch operations that I want available on first paint to avoid flickering or a loading state. The rest of my requests are sent directly to the API.

@Willem-Jaap
Copy link
Contributor

Part of my data fetching logic looks like this currently, can be used on client and server as it conditionally imports next/headers

const fetchRequest = async (route: string, config: Config = {}): Promise<globalThis.Response> => {
    const options = {
        ...config,
        headers: {
            ...DEFAULT_HEADERS,
            ...(await getHeaders()),
            ...config.headers,
        },
    };

    const response = await fetch(`${env.NEXT_PUBLIC_API_URL}/${route}`, options);
    return response;
};

const getHeaders = async () => {
    // 💡 If running on server, include the headers of the current request.
    return typeof window === 'undefined' ? getServerHeaders() : {};
};

const getServerHeaders = async () => {
    const { headers } = await import('next/headers');
    const headerStore = await headers();
    const Referer = headerStore.get('host');
    const cookie = headerStore.get('cookie');

    return {
        ...(Referer && { Referer }),
        ...(cookie && { cookie }),
    };
};

I think that works quite well, haven't thoroughly tested security for this

@jared-cannon
Copy link
Author

jared-cannon commented Jan 18, 2025

I like your approach - I'm assuming DEFAULT_HEADERS includes x-xsrf-token, etc. after testing this, I think we still need to extract the csrf token from the cookieStore within getServerHeaders() and add that header to the request. The other option would be adding it to the request body as a _token input value, but the result is the same.

This still removes the need for some of the additional .env variables at the expense of not exiting early if the session cookie doesn't exist. Do you mind if I steal your implementation and add it to this PR?

@Willem-Jaap
Copy link
Contributor

Yeah, you can definitely use it. I look forward to having a fully featured, standardized implementation of Next with Breeze

…ponse. This gives us the option to add schemas/type conformance with libraries like zod in conjunction with typescript in the future.
@taylorotwell
Copy link
Member

So I totally agree this needs official documentation and guidance and we have it on our schedule for this summer to revisit. Since we're rebuilding our starter kits now I'll probably table this for now until we can revisit it internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants