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

Setting up CORS properly #95

Closed
jfrelik opened this issue Jan 29, 2023 · 19 comments
Closed

Setting up CORS properly #95

jfrelik opened this issue Jan 29, 2023 · 19 comments
Labels
question Further information is requested

Comments

@jfrelik
Copy link

jfrelik commented Jan 29, 2023

Hi,

I don't know what I am doing wrong here but when trying to setup CORS with an example config like this:

corsHandler: { value: { origin: ['some.site.ever.com'], methods: ['POST'] }, route: '' }

But when I'm posting using insomnia to localhost:3000/api/test it gives me result like this, my assumption is that the access-control-allow-origin key would change but it stays at *.
Am I doing something wrong here or is it some kind of a bug?

image

@jfrelik jfrelik added the question Further information is requested label Jan 29, 2023
@Baroshem
Copy link
Collaborator

Hey,

Thanks for raising this issue. I will be back with an answer quite soon.

@Baroshem
Copy link
Collaborator

After reading the CORS documentation, it seems to me that in order to correctly handle CORS, a request that is coming to a server would need to have a header Origin set to the some.site.ever.com so that the server will then compare it with access-control-allow-origin: ['some.site.ever.com'] and if there will be a match, then in the response headers you will get access-control-allow-origin: ['some.site.ever.com']

Also another useful resource -> https://web.dev/cross-origin-resource-sharing/

@jfrelik
Copy link
Author

jfrelik commented Jan 30, 2023

I see now, thanks for sharing the docs. Maybe it's a good idea to mention that in the documentation to clarify things for other users

@Baroshem
Copy link
Collaborator

Good idea!

I will create a separate issue to include these in the docs

@Baroshem
Copy link
Collaborator

Added to #97

@Baroshem
Copy link
Collaborator

Baroshem commented Feb 1, 2023

@jfrelik

Hey, if you need some additional info on setting CORS, please let me know. Otherwise, could you please close the issue? :)

@jfrelik jfrelik closed this as completed Feb 1, 2023
@maxdzin
Copy link

maxdzin commented Mar 2, 2023

Hi @Baroshem,
first of all, thank you for such a nice and flexible solution to set up the security settings of the Nuxt app, that's helped me a lot!

I hope you can help me to understand one question because I'm a bit stuck with settings up the CORS settings correctly.
There's a situation when I need explicitly set the Access-Control-Allow-Origin header to a specific URL, regardless of whether the Origin request header is set or not.
So, when some request was made from the resource that does not correspond to a specific URL specified in Access-Control-Allow-Origin, it must be rejected.
And I expect to see such an error response if the origins are not matched (or if the Origin header isn't set in the request at all):

Access to fetch at 'https://myapp.com/' from origin 'https://example.com' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

But now, if the request Origin header isn't set, in the response an * is added to the Access-Control-Allow-Origin header, even if it was set explicitly.

So, how to handle that?

Thank you

@maxdzin
Copy link

maxdzin commented Mar 2, 2023

I also found this info about it:

If the Origin request header is not set, the server cannot determine the origin of the request. In this case, if the Access-Control-Allow-Origin header is also not set to * or to the specific origin of the request, the server should reject the request by sending an HTTP 403 Forbidden response.

This is because the Access-Control-Allow-Origin header is a crucial security mechanism that is used to protect against cross-site request forgery (CSRF) attacks. By specifying the origins that are allowed to access a resource, the server can prevent unauthorized access and protect sensitive data.

If the Access-Control-Allow-Origin header is not set, it means that the server is not explicitly allowing any origins to access the resource. In this case, if the Origin header is also not set, the server cannot determine the origin of the request and should reject it to prevent any potential security vulnerabilities.

@Baroshem
Copy link
Collaborator

Baroshem commented Mar 3, 2023

Hey @maxdzin

You have asked this question in a bit bad moment as I just yesterday deprecated the current CORS handler and will be migrating to a new one in the upcoming weeks (h3-cors package that I was using was deprecated and merged to core H3 functionality).

I will be diving into the topic more and will be able to deliver a new functionality that would allow to correctly handle CORS.
For now, you can check the H3 docs about using CORS.

Also, if you would be interested in doing a bit of research on how to implement this in Security module that would be much appreciated!

https://github.com/unjs/h3/tree/main/src/utils/cors

@maxdzin
Copy link

maxdzin commented Mar 3, 2023

Hi @Baroshem

Thank you! I saw about the deprecation and looked at how it was merged into H3.

I checked sources and I can see that the problem is in createOriginHeaders method:

export function createOriginHeaders(
  event: H3Event,
  options: H3CorsOptions
): H3AccessControlAllowOriginHeader {
  const { origin: originOption } = options;
  const origin = getRequestHeader(event, "origin");

  if (!origin || !originOption || originOption === "*") {
    return { "access-control-allow-origin": "*" };
  }

  if (typeof originOption === "string") {
    return { "access-control-allow-origin": originOption, vary: "origin" };
  }

  return isCorsOriginAllowed(origin, options)
    ? { "access-control-allow-origin": origin, vary: "origin" }
    : {};
}

That's where this check is - if the Origin request header is not set, then an "*" is set to the access-control-allow-origin is returned.

I'll check more on how it should be solved, maybe some nice idea will come to mind.
But at first glance, it seems that this method needs to be changed (maybe add some additional option for that check).

Or add some changes where defineCorsEventHandler is used.

Let's see then.

@Baroshem
Copy link
Collaborator

Baroshem commented Mar 6, 2023

Hey @maxdzin

I think that I have an answer to you. The CORS wont work when you are fetching data from the same nuxt app as it the origin of the request is the same for browser and server.

It will start working when the origin is different.

For example, you can create two nuxt applications where one is listening on port 3000 while the second on port 8000 for example. Normally the app 8000 can request the app 3000 without any issues ( because of access-control-allow-origin: *). The CORS will start working once we set configiuration origin to something, i.e.

  security: {
    corsHandler: {
      value: {
        origin: 'http://localhost:3000'
      }
    }
  }

Now, when you will try to fetch data from app 3000 in app 8000, you will get a proper CORS error because the origin header is present and has a value of 8000 while the first app is configured to only accepts request:

image

image

image

@maxdzin
Copy link

maxdzin commented Mar 8, 2023

Hi @Baroshem

OK. Let me tell you what I noticed.
Yes, I used these corsHandler settings before:

    corsHandler: {
      value: {
        origin: process.env.NUXT_APP_URL ? [process.env.NUXT_APP_URL] : '*',
        methods: ['GET', 'HEAD', 'PUT', 'PATCH', 'POST', 'DELETE'],
        preflight: {
          statusCode: 204,
        },
      },
      route: '',
    },

I passed that value as string[], i.e. [process.env.NUXT_APP_URL] since it shows a type error if string value is used instead of array.

interface CorsOptions {
    origin?: "*" | "null" | (string | RegExp)[] | ((origin: string) => boolean);
    ...

and this one merged in unjs/h3 as well:

export interface H3CorsOptions {
  origin?: "*" | "null" | (string | RegExp)[] | ((origin: string) => boolean);
  ...

So, I believe, there's some fix is needed at least on that option type.

And the problem is probably with this method:

export function createOriginHeaders(
  event: H3Event,
  options: H3CorsOptions
): H3AccessControlAllowOriginHeader {
  const { origin: originOption } = options;
  const origin = getRequestHeader(event, "origin");

  if (!origin || !originOption || originOption === "*") {
    return { "access-control-allow-origin": "*" };
  }

  if (typeof originOption === "string") {
    return { "access-control-allow-origin": originOption, vary: "origin" };
  }

  return isCorsOriginAllowed(origin, options)
    ? { "access-control-allow-origin": origin, vary: "origin" }
    : {};
}

In the last line check - if it is not isCorsOriginAllowed, then it is not set access-control-allow-origin.
But in order to set the response correctly, I think, it must return the originOption value, isn't it?
Or am I missing something?

So, when corsHandler.value.origin is set as a string, it works, but if it is set as an array of strings or an array of RegExp, it is not.

@Baroshem
Copy link
Collaborator

Baroshem commented Mar 8, 2023

Hey,

I believe it is a bug on h3-cors and now H3. Would you mind if I create an issue in H3 package with your findings and firing a pull request to them to coreect the type? :)

@maxdzin
Copy link

maxdzin commented Mar 8, 2023

@Baroshem Oh. I created an issue in H3 and just saw your message.
If you think that you should create a PR, sure, go ahead. Or I can do that.

@Baroshem
Copy link
Collaborator

Baroshem commented Mar 8, 2023

@maxdzin

I can create a PR tomorrow :)

Thanks for creating an issue!

@maxdzin
Copy link

maxdzin commented Mar 8, 2023

@Baroshem nice!

Please, take a look at these lines in the createOriginHeaders method as well:

if (!origin || !originOption || originOption === "*") {
  return { "access-control-allow-origin": "*" };
}

Probably it should be like this:

if ((!origin && !originOption) || originOption === "*") {
  return { "access-control-allow-origin": "*" };
}

Otherwise, it set the header to * when the origin value (from the request header) is empty and the originOption is set to some specific URL.

Besides, maybe that method can be even simplified if access-control-allow-header should be set by originOption if its value is different from * or null or undefined, but I'm not sure.

@Baroshem
Copy link
Collaborator

Baroshem commented Mar 9, 2023

@maxdzin

Hey,

I needed to do some fixes to the module (for some reason I couldn't load the h3 types to the app and because of that the build command was failing). To fix that, I needed to create my local CorsOptions type where I have included the different type for the CORS options -> 1b94de6

When using CORS from Nuxt security this will be now valid origin: 'http://your-page.io'

origin?: "*" | "null" | string | (string | RegExp)[] | ((origin: string) => boolean);

@maxdzin
Copy link

maxdzin commented Mar 9, 2023

@Baroshem great! That should work properly I think. I'll check it out.
Thanks a lot!

@maxdzin
Copy link

maxdzin commented Mar 9, 2023

@Baroshem It works well! Thank you 🙏

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

No branches or pull requests

3 participants