cors icon indicating copy to clipboard operation
cors copied to clipboard

Incorrect response when option origin is true and requestOrigin is undefined

Open yanickrochon opened this issue 2 years ago • 2 comments

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 :

https://github.com/expressjs/cors/blob/f038e7722838fd83935674aa8c5bf452766741fb/lib/index.js#L58-L67

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'
      }]);

yanickrochon avatar Dec 13 '22 14:12 yanickrochon

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!

dougwilson avatar Dec 13 '22 15:12 dougwilson

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.

yanickrochon avatar Dec 13 '22 15:12 yanickrochon