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

Return current path in callback origin #273

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

grovertb
Copy link

@grovertb grovertb commented Jul 20, 2022

The change was added because some endpoints are called without origin, but we must allow them, so a wishList was added so that they can respond without a cors error.

const wishList = [ '/path' ]
router.use(cors({
    ...
    ...
    origin        : (origin, callback, path) => {
      if(wishList.includes(path) && !origin) return callback(null, true)
      else if(/mydomain/i.test(origin))  return callback(null, origin)

      return callback(
        new Error(
          'The CORS policy for this site does not allow access from the specified Origin.'
        ),
        false
      )
    }
  }))

@dougwilson
Copy link
Contributor

Hi @grovertb thank you for your PR. Ideally the change should be backward-compatible, though. But even then, can you not use mounting points to implement this? For example, have a cors middleware that has origin: true mounted in your Express app to the paths you want any origin and then have your normal cors middleware mounted to the other paths?

@grovertb
Copy link
Author

@dougwilson
I have updated the order of the parameters, the path would return in the 3rd parameter and it no longer affects previous versions.

We need to validate with the path because we have a service that consumes an endpoint that does not send origin, so we must allow based on the path to let it access without cors errors.

I use callback(null, true) referring to the documentation example where true is passed. https://github.com/expressjs/cors#configuring-cors-asynchronously

@dougwilson
Copy link
Contributor

Hi @grovertb thank you. But I guess you still didn't answer my question: can you not use mounting points to implement this? For example, have a cors middleware that has origin: true mounted in your Express app to the paths you want any origin and then have your normal cors middleware mounted to the other paths?

But even then, if there is no origin on the request, this CORS does not come in to play. It sounds like that is a non-browser web client.

Maybe if you can provide all the details around your scenario and set up, I can help you use the existing method to vary on path this module has instead of adding a second method. Ideally we just want to have one way to do each type of thing.

@grovertb
Copy link
Author

grovertb commented Sep 8, 2022

Hi @dougwilson, I have updated the example that I had sent, maybe that was causing confusion.

We have a backend service that consumes directly from a microservice endpoint in which it does not have ORIGIN, so what we did is validate by the "path" and only that path ignores the cors validation.

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

Successfully merging this pull request may close these issues.

None yet

2 participants