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

Incorrect response when option origin is true and requestOrigin is undefined #295

Open
yanickrochon opened this issue Dec 13, 2022 · 2 comments

Comments

@yanickrochon
Copy link

For an app requesting data using the relative URL /api/data using this method :

const response = await fetch(`/api/data?locale=${locale}`, {
   credentials: 'same-origin',
   method: 'GET',
   headers: {
      'Accept': 'application/json',
   }
});
const data = await response.json();

And an API route handled this way :

export default apiMiddleware(async (req, res) => {
   // ... code here
});

With the apiMiddleware defined this way :

import Cors from 'cors';

const originWhitelist = [ /* ... list of valid origins */ ];

// Initializing the cors middleware
const cors = Cors({
   methods: ['GET', 'POST', 'HEAD'],
   allowedHeaders: 'X-CSRF-Token, X-Requested-With, Accept, Accept-Version, Content-Length, Content-MD5, Content-Type, Date, X-Api-Version, X-Api-Authorize, X-Authorize, Origin',
   credentials: true,
   origin: (origin, callback) => {
      // NOTE : origin may be undefined for relative URLs
      if (!origin || originWhitelist.indexOf(origin) !== -1) {
         callback(null, true);
      } else {
         callback(new Error('Not allowed by CORS'))
      }
   },
   optionsSuccessStatus: 200 // some legacy browsers (IE11, various SmartTVs) choke on 204
});

export const apiMiddleware = handler => async (req, res) => {
   await cors(req, res, (result) => {
      if (result instanceof Error) {
         reject(result);
      } else {
         resolve();
      }
   });

   // ... extend req with utils

   return handler(req, res);
};

The origin function will receive undefined as request origin, so even if the returned value is true (i.e. accept the request), the resposne will be handled like this :

cors/lib/index.js

Lines 58 to 67 in f038e77

isAllowed = isOriginAllowed(requestOrigin, options.origin);
// reflect origin
headers.push([{
key: 'Access-Control-Allow-Origin',
value: isAllowed ? requestOrigin : false
}]);
headers.push([{
key: 'Vary',
value: 'Origin'
}]);

The line 62 will actually resolve like this :

      isAllowed = isOriginAllowed(requestOrigin, options.origin);
      // reflect origin
      headers.push([{
        key: 'Access-Control-Allow-Origin',
        value: isAllowed ? undefined : false              // <---- undefined
      }]);
      headers.push([{
        key: 'Vary',
        value: 'Origin'
      }]);

Which will not send the Access-Control-Allow-Origin header to the client.

A fix could be to send "*" as default origin :

      isAllowed = isOriginAllowed(requestOrigin, options.origin);
      // reflect origin
      headers.push([{
        key: 'Access-Control-Allow-Origin',
        value: isAllowed ? requestOrigin || "*" : false         // <---- defaults to "*" if undefined
      }]);
      headers.push([{
        key: 'Vary',
        value: 'Origin'
      }]);
@dougwilson
Copy link
Contributor

Sorry, I may have misread your issue. I will need to check it out. Can you please include complete code and complete way to reproduce? I was not sure how to assemble the procided code snipplets into a running app to reproduce your issue. Ideally please provide the following so I can take a look:

  1. Complete server code I can copy and paste and run without modification
  2. Complete client side code for the same
  3. Versions of things like node.js ans this module
  4. Instructions for how to reproduce

Thank you and sorry about the initial response!

@yanickrochon
Copy link
Author

Thank you for the consideration. I will have to collect more data myself because I get these reports from users but cannot reproduce myself. However this seems to be the source of the problem; a missing Access-Control-Allow-Origin header :

image

I will do my best to provide you with either a reproducible example, or a withdrawal of this issue.

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

No branches or pull requests

2 participants