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

Access-Control-Max-Age not effective in preflight request caching when Authorization header is programatically defined #1278

Open
mfbx9da4 opened this issue Aug 4, 2021 · 8 comments
Labels
needs tests Moving the issue forward requires someone to write tests topic: cors

Comments

@mfbx9da4
Copy link

mfbx9da4 commented Aug 4, 2021

Expected behaviour

Access-Control-Max-Age: 10s header should cache preflight requests for 10s when the Authorization header is set programmatically e.g. using a value from localStorage.

Reproduce

Response headers:

"Access-Control-Allow-Methods": "*",
"Access-Control-Allow-Headers": "*",
"Access-Control-Allow-Origin": "*",
"Access-Control-Max-Age": "10"

Execute this fetch request twice from a different origin. Separate executing each request by about 6s.

const url = "https://prairie-bright-earl.glitch.me/";
const opts = {
  headers: {
    "Content-Type": "application/json",
    Authorization: "asdf",
  },
  method: "POST",
  body: JSON.stringify({ message: "ping" }),
};
fetch(url, opts);

Observe that:

  • Every single request produces a preflight request.
  • Things can be "fixed", in the sense that Access-Control-Max-Age will be respected by doing one of the following:
    • Removing the Authorization header
    • Enabling credentials mode. ie Adding fetch request header: credentials: include and setting the response headers Access-Control-Allow-Headers: Authorization, Content-Type and Origin: ${origin}.
    • Setting Access-Control-Allow-Headers: Authorization, *.

Links to minimal failing demo:

Tested in chrome and firefox, they both exhibit this behaviour. Preflight requests are not visible in safari.

According to @jakearchibald this could be a bug? From reading the spec, at least to me, it's not clear whether programatically setting Authorization header should require credentials mode. Regardless, it seems like an unnecessary precaution to require setting origin if I know as the developer that I don't use cookies and therefore I am not susceptible to CORS attacks?

Also the workarounds seem totally unintuitive to me.

Disclaimer: I am a spec issue contributor virgin. 👋

@mfbx9da4 mfbx9da4 changed the title Preflight request not cached when Authorization header is programatically defined Access-Control-Max-Age not effective in preflight request caching when Authorization header is programatically defined Aug 4, 2021
@annevk
Copy link
Member

annevk commented Sep 29, 2021

There's multiple things going on here.

  1. Implementations have a bug in that they do not enforce listing Authorization explicitly. This is being worked on by @yutakahirano, see https://groups.google.com/a/chromium.org/g/blink-dev/c/jEV2VMVjMt8/m/rogFnr6xAgAJ.
  2. It seems however that they might enforce this for the preflight cache? I think that would explain the results you are seeing above. Enabling credentials mode does not really matter I think as the side effect of that is that you have to list the header, which I suspect is what matters here.

@annevk annevk added the needs tests Moving the issue forward requires someone to write tests label Sep 29, 2021
@yutakahirano
Copy link
Member

I tried to ship the proposed behavior, but it's been rejected because the number of affected users is larger than expected.

I can make more efforts to reduce the number of sites that depend on the current behavior (as discussed in the intent thread), but before that I want to re-check people's opinions on this. Mozilla failed to change the behavior, Chrome failed to change the behavior. Do we still want to change the behavior?

@mfbx9da4
Copy link
Author

How might a site depend on this behaviour?

@annevk
Copy link
Member

annevk commented Jan 21, 2022

I think ideally we'd still change this as otherwise it makes websites more easily vulnerable to dictionary attacks.

@javiervisiedo
Copy link

I'm currently trying to better understand in which way the shipment of this behavior impacts affected sites. In most cases, this policy will block calls to 1P and 3P APIs, and the vast majority doesn't seem to have Ux impacts. These are typically calls to backend services such as personalization, analytics, etc.

I've been reaching out to sites where I observed impacts on the functionality of their pages, e.g. login not working, some widgets not showing, etc. and most took action. However this has become a game of wack-a-mole. Configuring a server to return wildcard for ACAH is probably the path of lowest friction today, therefore the usage is slowly growing, and it is slightly higher that what we'd be comfortable with for shipping.

I see 2 possible ways forward. 1) ship it in a coordinated way with other browsers, or 2) discuss changing the directive if no browser is planning to comply. I'm personally inclined to prioritize (1) if there is an interest

@annevk
Copy link
Member

annevk commented Apr 27, 2023

Adding some other people to see if there is interest in requiring Authorization to be explicitly listed (as already required by the standard) in a coordinated fashion across implementations.

cc @youennf @sysrqb @mozfreddyb @dveditz

@mozfreddyb
Copy link
Collaborator

Looks like this spun off into an email thread and the issue here did not see the response it required.

Firefox has this change implemented behind a preference starting version 115. Previous looks at observed behavior via https://chromestatus.com/metrics/feature/timeline/popularity/3873 seem to indicate that this anti-pattern has rather grown in popularity, which I find a bit worrying.
In summary, Firefox is happy to follow if this ends up being shipped in a coordinated way.

@jub0bs
Copy link
Contributor

jub0bs commented Sep 24, 2023

@mozfreddyb

this anti-pattern has rather grown in popularity, which I find a bit worrying.

An interesting manifestation of Hyrum's law!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests Moving the issue forward requires someone to write tests topic: cors
Development

No branches or pull requests

6 participants