-
-
Notifications
You must be signed in to change notification settings - Fork 480
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
Comments
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. |
I was doing something like: cors({
origin: ['a', 'b'],
credentials: true,
}); It seemed inconsistent to send
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 The |
@alxndrsn That
|
@jub0bs the spec you've linked refers to client credentials, defined in that spec at https://fetch.spec.whatwg.org/#credentials:
I don't think those are the same as express CORS middleware's
|
For future reference, expanding on:
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 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 |
They are.
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. |
Would there be support for configuring the
Access-Control-Allow-Credentials
header dynamically? It looks like currently the only valid values for thecredentials
config option istrue
. Allowing a function similar to theorigin
option would allow for settingAccess-Control-Allow-Credentials
based on the request.The text was updated successfully, but these errors were encountered: