mirror of
https://github.com/gristlabs/grist-core.git
synced 2026-03-02 04:09:24 +00:00
(core) Allow requests from untrusted origins but without credentials
Summary: Allow requests from untrusted origins instead of returning an error, but don't allow credentials (Cookie header) or API keys (Authorization header). Allow setting the header `Content-type: application/json` as an alternative to `X-Requested-With: XMLHttpRequest` to make it easier for clients to make POST/PUT/PATCH/DELETE requests without authentication. Discussion: https://grist.slack.com/archives/C0234CPPXPA/p1666355281535479 Test Plan: Added and updated DocApi tests. Tested manually how this affects requests made from a browser. Reviewers: paulfitz, dsagal Reviewed By: paulfitz, dsagal Differential Revision: https://phab.getgrist.com/D3678
This commit is contained in:
@@ -210,13 +210,23 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer
|
||||
}
|
||||
|
||||
// If we haven't already been authenticated, and this is not a GET/HEAD/OPTIONS, then
|
||||
// require that the X-Requested-With header field be set to XMLHttpRequest.
|
||||
// require a header that would trigger a CORS pre-flight request, either:
|
||||
// - X-Requested-With: XMLHttpRequest
|
||||
// - https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#use-of-custom-request-headers
|
||||
// - https://markitzeroday.com/x-requested-with/cors/2017/06/29/csrf-mitigation-for-ajax-requests.html
|
||||
// - Content-Type: application/json
|
||||
// - https://www.directdefense.com/csrf-in-the-age-of-json/
|
||||
// This is trivial for legitimate web clients to do, and an obstacle to
|
||||
// nefarious ones.
|
||||
// https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#use-of-custom-request-headers
|
||||
// https://markitzeroday.com/x-requested-with/cors/2017/06/29/csrf-mitigation-for-ajax-requests.html
|
||||
if (!mreq.userId && !mreq.xhr && !['GET', 'HEAD', 'OPTIONS'].includes(mreq.method)) {
|
||||
return res.status(401).json({error: 'Bad request (missing header)'});
|
||||
if (
|
||||
!mreq.userId &&
|
||||
!(mreq.xhr || mreq.get("content-type") === "application/json") &&
|
||||
!['GET', 'HEAD', 'OPTIONS'].includes(mreq.method)
|
||||
) {
|
||||
return res.status(401).json({
|
||||
error: "Unauthenticated requests require one of the headers" +
|
||||
"'Content-Type: application/json' or 'X-Requested-With: XMLHttpRequest'"
|
||||
});
|
||||
}
|
||||
|
||||
// For some configurations, the user profile can be determined from the request.
|
||||
|
||||
@@ -1752,12 +1752,22 @@ function allowTestLogin() {
|
||||
// Check OPTIONS requests for allowed origins, and return heads to allow the browser to proceed
|
||||
// with a POST (or other method) request.
|
||||
function trustOriginHandler(req: express.Request, res: express.Response, next: express.NextFunction) {
|
||||
res.header("Access-Control-Allow-Methods", "GET, PATCH, PUT, POST, DELETE, OPTIONS");
|
||||
if (trustOrigin(req, res)) {
|
||||
res.header("Access-Control-Allow-Credentials", "true");
|
||||
res.header("Access-Control-Allow-Methods", "GET, PATCH, PUT, POST, DELETE, OPTIONS");
|
||||
res.header("Access-Control-Allow-Headers", "Authorization, Content-Type, X-Requested-With");
|
||||
} else {
|
||||
throw new ApiError('Unrecognized origin', 403);
|
||||
// Any origin is allowed, but if it isn't trusted, then we don't allow credentials,
|
||||
// i.e. no Cookie or Authorization header.
|
||||
res.header("Access-Control-Allow-Origin", "*");
|
||||
res.header("Access-Control-Allow-Headers", "Content-Type, X-Requested-With");
|
||||
if (req.get("Cookie") || req.get("Authorization")) {
|
||||
// In practice we don't expect to actually reach this point,
|
||||
// as the browser should not include credentials in preflight (OPTIONS) requests,
|
||||
// and should block real requests with credentials based on the preflight response.
|
||||
// But having this means not having to rely on our understanding of browsers and CORS too much.
|
||||
throw new ApiError("Credentials not supported for cross-origin requests", 403);
|
||||
}
|
||||
}
|
||||
if ('OPTIONS' === req.method) {
|
||||
res.sendStatus(200);
|
||||
|
||||
Reference in New Issue
Block a user