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

Configurable CORS and site headers #15397

Merged
merged 9 commits into from
Jul 24, 2024
Merged

Configurable CORS and site headers #15397

merged 9 commits into from
Jul 24, 2024

Conversation

timkelty
Copy link
Contributor

@timkelty timkelty commented Jul 22, 2024

Description

  • Makes minor change to allow Yii2's CORS filter to work for actions requests
  • Deprecates allowedGraphqlOrigins in favor of using CORS filter
  • Deprecates permissionsPolicyHeader in favor of using headers filter
  • Adds 2 new filters: CorsFilter, HeadersFilter
    • These are both for site-only (non-cp) requests, and can be optionally filtered by specific site

Example config: config/app.web.php

<?php

return [
    // Attach the CORS filter to the application
    'as corsFilter' => [
        'class' => \craft\filters\Cors::class,

        // optionally filter by site(s)
        'site' => ['siteA', 'siteB'],

        // CORS defaults for all non-CP requests
        'cors' => [
            'Origin' => [
                'https://craft-4-project-cp.ddev.site',
                'https://craft-4-project.ddev.site'
            ],
            'Access-Control-Request-Method' => ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS'],
            'Access-Control-Request-Headers' => ['*'],
            'Access-Control-Allow-Credentials' => true,
            'Access-Control-Max-Age' => 86400,
            'Access-Control-Expose-Headers' => [],
        ],

        // Optionally override any controller action's CORS settings.
        // This is an override only – if the setting is not present above, it will not be added.
        'actions' => [
            'graphql/api' => [
                'Origin' => ['*'],
                'Access-Control-Allow-Credentials' => false,
            ],
        ],
    ],

    // Attach the headers filter to the application
    'as headersFilter' => [
        'class' => \craft\filters\Headers::class,
        'site' => 1,
        'headers' => [
            'foo' => 'bar',
            'Permissions-Policy' => 'interest-cohort=()',
        ],
    ],
];

Related issues

src/web/Application.php Outdated Show resolved Hide resolved
@timkelty timkelty changed the base branch from 5.x to 4.x July 22, 2024 14:27
@brandonkelly brandonkelly changed the base branch from 4.x to 4.11 July 22, 2024 15:30
@timkelty timkelty marked this pull request as ready for review July 22, 2024 17:33
@timkelty timkelty changed the title Configurable CORS Configurable CORS and site headres Jul 23, 2024
@timkelty timkelty changed the title Configurable CORS and site headres Configurable CORS and site headers Jul 24, 2024
@vettndr
Copy link

vettndr commented Aug 12, 2024

Hi Brandon @brandonkelly,
I have a Nuxt.js app which is connected to a CraftCMS instance in a separated domain.

When I try to login from the Nuxt.js application using POST actions/users/login I'm getting back this error:
Response to preflight request doesn't pass access control check: It does not have HTTP ok status.

Is there a way to handle the "preflight request"?

Best

@timkelty
Copy link
Contributor Author

timkelty commented Aug 12, 2024

@vettndr The \craft\filters\Cors handles OPTIONS preflight requests, as well as Cors-related headers.
Implement the filter in your config/app.web.php with something like this:

<?php

return [
    'as corsFilter' => [
        'class' => \craft\filters\Cors::class,
        'cors' => [
            'Origin' => [
                'https://my-nuxt-app.com',
            ],
            'Access-Control-Request-Method' => ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS'],
            'Access-Control-Request-Headers' => ['*'],
            'Access-Control-Allow-Credentials' => true,
            'Access-Control-Max-Age' => 86400,
            'Access-Control-Expose-Headers' => [],
        ],
    ],
];

@vettndr
Copy link

vettndr commented Aug 12, 2024

@vettndr The \craft\filters\Cors handles OPTIONS preflight requests, as well as Cors-related headers. Implement the filter in your config/app.web.php with something like this:

<?php

return [
    'as corsFilter' => [
        'class' => \craft\filters\Cors::class,
        'cors' => [
            'Origin' => [
                'https://my-nuxt-app.com',
            ],
            'Access-Control-Request-Method' => ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS'],
            'Access-Control-Request-Headers' => ['*'],
            'Access-Control-Allow-Credentials' => true,
            'Access-Control-Max-Age' => 86400,
            'Access-Control-Expose-Headers' => [],
        ],
    ],
];

Hey @timkelty
thank you for your message!

I tried as you suggested me.... and now I'm facing a 400 Bad Request issue, this is the response:

"name": "Bad Request",
"message": "Unable to verify your data submission.",
"code": 0,
"status": 400,
"exception": "yii\\web\\BadRequestHttpException",
"file": "/var/www/html/vendor/yiisoft/yii2/web/Controller.php",
"line": 221,

I'm pretty sure the credentials I'm using are working in the backend when I try to access on it.

Thanks in advance for your support.

Best

@timkelty
Copy link
Contributor Author

@vettndr your request is failing CSRF validation.

You need to fetch a CSRF token first, then include it along with your request.
See https://craftcms.com/docs/5.x/development/forms.html#ajax for examples

@vettndr
Copy link

vettndr commented Aug 12, 2024

@vettndr your request is failing CSRF validation.

You need to fetch a CSRF token first, then include it along with your request. See https://craftcms.com/docs/5.x/development/forms.html#ajax for examples

@timkelty yes, I’m fetching the CSRF token when the app is loaded for the first time and then I’m including it in the POST request.

I don’t know why it’s triggering that 400 exception.

@timkelty
Copy link
Contributor Author

How are you making the request (fetch, axios, etc)?
A common mistake is to not send credentials along with the request (eg withCredentials, credentials: include)

@vettndr
Copy link

vettndr commented Aug 13, 2024

How are you making the request (fetch, axios, etc)? A common mistake is to not send credentials along with the request (eg withCredentials, credentials: include)

Hi @timkelty
this is the login request:

const { user } = await $fetch(
        `${envs.public.craftUrl}/${authRoutes.login}`,
        {
          method: "POST",
          headers: {
            Accept: "application/json",
            "X-CSRF-Token": csrfToken.value as string,
            "X-Requested-With": "XMLHttpRequest",
          },
          body: {
            loginName: email,
            password: password,
          },
          credentials: "include",
        }
      )

as you can see I'm setting the credentialsparameter

best

@timkelty
Copy link
Contributor Author

timkelty commented Aug 13, 2024

What are the domains involved? Are they subdomains of the same root?

I'm not seeing anything jump out from your example, so if you could, please email [email protected] with as much of the project as possible, specifically:

  • composer.json/lock files
  • config directory
  • any custom modules or plugins
  • a DB dump if possible

@vettndr
Copy link

vettndr commented Aug 14, 2024

What are the domains involved? Are they subdomains of the same root?

I'm not seeing anything jump out from your example, so if you could, please email [email protected] with as much of the project as possible, specifically:

  • composer.json/lock files
  • config directory
  • any custom modules or plugins
  • a DB dump if possible

Hi @timkelty
thank you, I will send a request to the team soon.

Anyway I did a little debugging process and I found that in the validateCsrfToken($clientSuppliedToken = null) function
the $trueToken variable is printing different values comparing with $this->getCsrfTokenFromHeader().

It seems like it's comparing the token I passed in the header with another one.

I would really appreciate to read your thoughts about it.

Best

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