Skip to content

Conditional credentials? #336

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

Closed
alxndrsn opened this issue Nov 13, 2024 · 6 comments · May be fixed by #337
Closed

Conditional credentials? #336

alxndrsn opened this issue Nov 13, 2024 · 6 comments · May be fixed by #337
Labels

Comments

@alxndrsn
Copy link

Would there be support for configuring the Access-Control-Allow-Credentials header dynamically? It looks like currently the only valid values for the credentials config option is true. Allowing a function similar to the origin option would allow for setting Access-Control-Allow-Credentials based on the request.

alxndrsn pushed a commit to alxndrsn/express-cors that referenced this issue Nov 19, 2024
@jonchurch
Copy link
Member

jonchurch commented Nov 27, 2024

Do you have example code you can share for what you're trying to do in your app?

You can configure CORS options on a per request basis by passing in a function when setting the middleware up. https://github.com/expressjs/cors#configuring-cors-asynchronously

Each request that comes in can get a chance to determine the CORS options which should be used.

I can see the need for a function for credentials if you don't have the information you need until further down in the handler logic.

It would be simpler for sure. But want to understand the usecase here before making every option either a value or a function. credentials is a good candidate for being a function, but still want to better understand your usecase.

@alxndrsn
Copy link
Author

alxndrsn commented Nov 28, 2024

Do you have example code you can share for what you're trying to do in your app?

I was doing something like:

cors({
  origin: ['a', 'b'],
  credentials: true,
});

It seemed inconsistent to send Access-Control-Allow-Credentials but not Access-Control-Allow-Origin when the Origin was not found in the configured array.

You can configure CORS options on a per request basis by passing in a function when setting the middleware up. https://github.com/expressjs/cors#configuring-cors-asynchronously

Thanks, this seems adequate. I've ended up with something like:

app.use(() => {
  const wrapped = cors((req, callback) => {
     try {
       const { allow, credentials } = getOptions(req);
       if(allow) {
         callback(null, {
           origin: true,
           credentials,
           methods: 'GET,POST', // keep it simple
           maxAge: 86400, // max(firefox, chrome) - see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Max-Age
         });
       } else {
         callback(null, { origin:false }); 
       }
     } catch(err) {
       callback(err);
     }   
  }); 
  return (req, res, next) => {
    wrapped(req, res, () => {
      res.vary(varyHeaderFor(req));
      next();
    }); 
  };  
});

function varyHeaderFor({ method }) {
  switch(method) {
    case 'OPTIONS': return 'Origin, Access-Control-Request-Headers';
    default:        return 'Origin';
  }
}

where getOptions(req) is app-specific code.

The Vary header also seemed to need additional manual handling.

@jub0bs
Copy link

jub0bs commented Apr 5, 2025

@alxndrsn That getOptions function that you propose cannot work in the general case, per the Fetch standard:

A request’s credentials mode is not necessarily observable on the server; only when credentials exist for a request can it be observed by virtue of the credentials being included. Note that even so, a CORS-preflight request never includes credentials.

@alxndrsn
Copy link
Author

alxndrsn commented Apr 6, 2025

per the Fetch standard

@jub0bs the spec you've linked refers to client credentials, defined in that spec at https://fetch.spec.whatwg.org/#credentials:

Credentials are HTTP cookies, TLS client certificates, and authentication entries (for HTTP authentication).

I don't think those are the same as express CORS middleware's credentials config option, described at https://github.com/expressjs/cors?tab=readme-ov-file#configuration-options as:

credentials: Configures the Access-Control-Allow-Credentials CORS header. Set to true to pass the header, otherwise it is omitted.

@alxndrsn
Copy link
Author

alxndrsn commented Apr 6, 2025

For future reference, expanding on:

It seemed inconsistent to send Access-Control-Allow-Credentials but not Access-Control-Allow-Origin when the Origin was not found in the configured array.

Here's a worked example:

Given a server:

app = require('express')();
app.use(require('cors')({ credentials:true, origin:['a'] }));
app.listen(1234);

and a function for making an OPTIONS request and logging the response headers:

makeReq = origin => {
  http.request(
    'http://localhost:1234',
    { method:'OPTIONS', headers:{ origin } },
    res => console.log({
      origin,
      responseHeaders: Object.entries(res.headers)
                             .filter(([k]) => k.startsWith('access-control')),
     })
  ).end();
};

CORS header response to an allowed origin:

> makeReq('a')
undefined
> {
  origin: 'a',
  responseHeaders: [
    [ 'access-control-allow-origin', 'a' ],
    [ 'access-control-allow-credentials', 'true' ],
    [ 'access-control-allow-methods', 'GET,HEAD,PUT,PATCH,POST,DELETE' ]
  ]
}

And do a disallowed origin:

> makeReq('b')
undefined
> {
  origin: 'b',
  responseHeaders: [
    [ 'access-control-allow-credentials', 'true' ],
    [ 'access-control-allow-methods', 'GET,HEAD,PUT,PATCH,POST,DELETE' ]
  ]
}

Should any CORS headers be included when Origin is b?

@jub0bs
Copy link

jub0bs commented Apr 6, 2025

@alxndrsn

I don't think those are the same as express CORS middleware's credentials config option [...]

They are.

It seemed inconsistent to send Access-Control-Allow-Credentials but not Access-Control-Allow-Origin when the Origin was not found in the configured array.

There is indeed an argument for omitting all CORS headers in responses to requests whose origin isn't allowed; I'm not denying that. But your proposed option cannot work, since the server cannot (in general) determine whether the client issued the request with credentials or not.

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

Successfully merging a pull request may close this issue.

3 participants