kubetail-org / edge-csrf

CSRF protection library for JavaScript that runs on the edge runtime (with Next.js, SvelteKit, Express, Node-HTTP integrations)
MIT License
146 stars 9 forks source link

Invalid csrf token in server action when used without a form #20

Closed Janhouse closed 9 months ago

Janhouse commented 9 months ago

It is possible to use server actions without using forms. https://react.dev/reference/react/use-server#calling-a-server-action-outside-of-form

In this case it still makes a POST request but when I pass csrfToken or csrf_token, csrf check fails.

Looking at the request, it makes POST request with the following body: [{"csrf_token":"AAidYJqD9YPPT5NRqWt0l+czE4APICWVe+SV7RoP"}]

But weirdly Content-Type header is set to text/plain;charset=UTF-8 for all server actions.

Tested with edge-csrf 1.0.7 and nextjs 14.0.4

Janhouse commented 9 months ago

Seems to work when only csrfToken is passed.

export async function serverAction(csrfToken: string) {}

It then ends up in payload as:

["AAg1YUx7lDpToYuW0s7vdHkne+NFJvLhRPYHHEx6"]

But passing anything else seems to fail.

type Params = {
  csrfToken: string
  somethingElse: string
}
export async function serverAction(params: Params) {}

It's sent as:

[{"csrfToken":"AAiTe2uMJZbfnS/RuvnplJeLw02Q3HC348FJrB/O","somethingElse":"test"}]
amorey commented 9 months ago

Thanks for the bug report and for triaging the problem! I think your diagnosis is correct: currently edge-csrf doesn't attempt to parse request bodies as JSON unless they specify json content in the Content-Type header so in the case of Server Actions with Content-Type: text/plain the entire request body is returned as the CSRF token: https://github.com/amorey/edge-csrf/blob/main/src/util.ts#L85

which means Server Action requests will pass CSRF checks when you pass in the token as one single argument but not when there are multiple arguments or when your single argument is an object.

Let me think about how to fix the problem and get back to you.

amorey commented 9 months ago

One potential solution is to try parsing submission requests with Content-Type: text/plain as JSON arrays (the serialized output of non-form server actions) and if they parse successfully then treat the first argument as the csrf token. Here's an example of what a server action and it's caller would look like:

// action.ts
'use server';

export default async function recordData(csrfToken: string, data: any) {
  console.log(`received data: ${JSON.stringify(data)}`);
}
// page.tsx
'use client';

import recordData from '../lib/actions';

export default function LikeButton() {
  const handleClick = async () => {
    const csrfResp = await fetch('/csrf-token');
    const { csrfToken } = await csrfResp.json();
    await recordData(csrfToken, { 'rand': Math.random() });
  };

  return (
    <>
      <button onClick={handleClick}>Click me</button>
    </>
  );
}

Here's an implementation of the solution described above: https://github.com/amorey/edge-csrf/tree/nfsa

Let me know what you think.

px-xp commented 9 months ago

I think the arguments here csrfToken:string, data: any:

export default async function recordData(csrfToken: string, data: any) {
  console.log(`received data: ${JSON.stringify(data)}`);
}

violate the Next.js API. The arguments to server actions must be serializable by React as per https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#behavior. Having multiple arguments violates this and any violates this. I think the action approach works better for forms because you can't change the content-type on actions. I haven't discovered a way to do this.

But for JSON requests I think this approach should work:

  await fetch('/api/foo', {
    method: 'POST',
    body: JSON.stringify({
      ...data,
      csrf_token: csrfToken,
    }),
    headers: {
      "Content-Type": "application/json"
    }
  })
amorey commented 9 months ago

In the example above, data: any is just a Typescript placeholder that you should replace with your own more specific schema (e.g. data: { rand: number; }). In any case, the Server Action API requirement is that each argument must be serializable at runtime; the Typescript definitions in the source code are just acting as helpers and won't be used during the actual execution. Here's an implementation of the example above that passes Typescript checks at compile time and also executes successfully without error at runtime: https://github.com/amorey/edge-csrf/tree/nfsa/examples/next14-approuter-server-action-non-form-submission

With regards to JSON requests, if you want to use fetch() instead of a server action then you should add the token as an HTTP header:

await fetch('/api/foo', {
  method: 'POST',
  headers: {
    'Content-Type': 'application/json',
    'X-CSRF-Token': csrfToken,
  },
  body: JSON.stringify(data),
});

(See https://github.com/amorey/edge-csrf/tree/main/examples/next14-approuter-js-submission-static)

Entropy-10 commented 9 months ago

Is there a way to get this working with multipart form data? I'm currently using a server action without a form for uploading images, but with using the method above it makes working with images difficult. Currently I'm converting my images to a base64 string so I can send it through the server action, and then converting back to a blob in my server action.

create-team-form.tsx

const { data: flagData, error: imageUploadError } = await uploadImage(
    csrfToken,
    {
        blob: String(await blobToBase64(flagBlob)),
        name: data.name
     }
)

upload-image.ts

export async function uploadImage(
    csrfToken: string,
    flagForm: { blob: string; name: string }
) {
    const teamName = flagForm.name
    const flagBlob = await fetch(flagForm.blob).then((res) => res.blob())
    const image = await flagBlob.arrayBuffer()
    // ....
}
amorey commented 9 months ago

Yes, it should work with the code in the "nfsa" branch. Try edge-csrf@1.0.7-nfsa.1 and see if that works: https://www.npmjs.com/package/edge-csrf/v/1.0.7-nfsa.1

Entropy-10 commented 9 months ago

If I try to pass formData into my server action at it causes the server action to throw 403, but if I pass JSON serializable data it seems to work just fine.

I want to be able to do something like this:

const { data: flagData, error: imageUploadError } = await uploadImage(
    csrfToken,
    flagForm // type of FormData
)
amorey commented 9 months ago

Edge-CSRF already supports server actions with forms/formData in the latest release (edge-csrf@1.0.7). Take a look at the example app here: https://github.com/amorey/edge-csrf/blob/main/examples/next14-approuter-server-action-submission

And in particular at the form submission implementation here: https://github.com/amorey/edge-csrf/blob/main/examples/next14-approuter-server-action-submission/app/page.tsx

For forms, the CSRF token should be passed in as a hidden input with name csrf_token:

import { revalidatePath } from 'next/cache';
import { headers } from 'next/headers';
import { redirect } from 'next/navigation';

export default function Page() {
  const csrfToken = headers().get('X-CSRF-Token') || 'missing';

  async function myAction(formData: FormData) {
    'use server';
    console.log('passed csrf validation');
    revalidatePath('/');
    redirect('/');
  }

  return (
    <form action={myAction}>
      <legend>Form with CSRF (should succeed):</legend>
      <input type="hidden" name="csrf_token" value={csrfToken} />
      <input type="text" name="input1" />
      <button type="submit">Submit</button>
    </form>
  );
}

Let me know if that helps.

Entropy-10 commented 9 months ago

Thank you for the info. On my other server components that are using server actions directly with the forms, that is what I'm doing it and it works great. The issue I'm encountering is that I have a client side form using react-hook-forms. Within the onSubmit function I'm calling two server actions which both need the csrf token. Here's a link to the github code for my form code: https://github.com/Entropy-10/test-open-site/blob/main/app/%5Blocale%5D/register/_components/create-team-form.tsx (this is my main branch without the edge-csrf code implemented).

amorey commented 9 months ago

Ahh I see, thanks for the explanation. I just tried it out and it looks like the request type changes from text/plain to multipart/form-data when one of the arguments is a FormData instance which means the request will be treated like a server action form submission by edge-csrf. Try this:

// action.ts
'use server';

export async function uploadImage(data: FormData) {
  console.log(data);
}
// page.tsx
'use client';

import { uploadImage } from '/path/to/action';

export default function ExampleComponent() {
  const handleClick = async () => {
    const csrfResp = await fetch('/csrf-token');
    const { csrfToken } = await csrfResp.json();

    const data = new FormData();
    data.set('csrf_token', csrfToken);
    data.set('var1', 'val1');

    await uploadImage(data);
  };

  return (
    <>
      <button onClick={handleClick}>Click me</button>
    </>
  );
}
Entropy-10 commented 9 months ago

Thank you so much that worked perfectly!

amorey commented 9 months ago

Great, happy to hear it!

With regards to the questions in the beginning of this thread, I merged the server action non-form handler code into main and published a new version (v1.0.9) so now those queries should work. I also added a list of all the examples to the README to help users looking for help in the future: https://github.com/amorey/edge-csrf/tree/main