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

Fetch Standard change: Authorization removed upon cross-origin redirects #22533

Closed
annevk opened this issue Nov 25, 2022 · 16 comments
Closed

Fetch Standard change: Authorization removed upon cross-origin redirects #22533

annevk opened this issue Nov 25, 2022 · 16 comments
Assignees
Labels
Content:HTTP HTTP docs Content:WebAPI Web API docs effort: medium This task is a medium effort. fx release archive A closed issue relating to firefox release notes for developers. help wanted If you know something about this topic, we would love your help!

Comments

@annevk
Copy link
Contributor

annevk commented Nov 25, 2022

whatwg/fetch#1544 changes the Fetch Standard to remove a web-developer-set Authorization header upon a cross-origin redirect.

This probably ought to be documented.

@github-actions github-actions bot added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Nov 25, 2022
@sideshowbarker sideshowbarker added Content:HTTP HTTP docs Content:WebAPI Web API docs help wanted If you know something about this topic, we would love your help! effort: medium This task is a medium effort. waiting for implementations Waiting for feature to be implemented in browsers and removed needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. labels Nov 27, 2022
@sideshowbarker
Copy link
Collaborator

Per https://wpt.fyi/results/fetch/api/credentials/authentication-redirection.any.html, WebKit already conforms with this spec change — but Blink and Gecko do not yet.

We may want to wait on documenting this until we have at least one other conforming implemention.

@dipikabh
Copy link
Contributor

This is related to https://bugzilla.mozilla.org/show_bug.cgi?id=1802086.

@hamishwillee
Copy link
Collaborator

Trying to work out what this "means" to users/documentation.

My understanding is that previously if the Authorization header was included with credentials, it would be passed by a server on redirect to the new server. With the change the header is stripped when the request is

So in terms of docs it looks like the place this affects is the credentials property in the options object passed to the fetch() call. Right?

So if you set credentials=same-origin and the request was redirected cross-origin the headers would now be stripped, while previously they might have been forwarded - is that right?
If you set credentials=include and there is a redirect I assume that the authorization is dropped too - because even if you allowed cross origin, you probably want to know it is an intended origin?

If that's about right we can add a note below credentials. IF not, then perhaps we should get someone who knows more about fetch() to look at the required docs.

@annevk
Copy link
Contributor Author

annevk commented Mar 3, 2023

No, this specifically changes the behavior for when the Authorization header is set by the web developer directly. When it's controlled by the browser it already depends on the request URL whether it's included or not.

@hamishwillee
Copy link
Collaborator

You mean if they call Headers.append() or set() with the Authorization header, then it will be stripped on redirect?

@annevk
Copy link
Contributor Author

annevk commented Mar 6, 2023

When it goes across origins, yes. (Also with setRequestHeader() on XMLHttpRequest.)

@hamishwillee
Copy link
Collaborator

hamishwillee commented Mar 7, 2023

There isn't any particularly great place for this information.
What I have done is:

@annevk The yaml values in https://bugzilla.mozilla.org/show_bug.cgi?id=1802086 indicate FF also strips the header from XMLHttpRequest and from any http redirect (network.http.redirect.stripAuthHeader).
So my proposal is to follow the same pattern as above - adding a feature in the browser compatibility data and adding a note in Authorization and XMLHttpRequest.

  • Does this make sense?
  • Is there a test for XMLHttpRequest that I can use to validate support? I tried https://wpt.live/xhr/xhr-authorization-redirect.any.html but it indicates FF does not support this feature - and none of the other browsers do either.
  • Is there a spec link other the fetch() spec we can use for XMLHttpRequest entry? It has its own spec and I can't find mention of this behaviour.

@annevk
Copy link
Contributor Author

annevk commented Mar 7, 2023

  1. Yes.
  2. I suspect that's a problem with wpt.live. It works fine if you run it locally.
  3. XMLHttpRequest builds upon Fetch. It doesn't define its own network logic. (If we added another API that could set headers it would also end up with this behavior.)

@hamishwillee
Copy link
Collaborator

Thanks. I think I've captured all this in #25127. Let's see how the reviews go.
FYI, the test above DOES work on wpl-live, but only for an http URL (not https).

@rhclayto
Copy link

rhclayto commented Mar 11, 2023

Where is the best place to ask a question about this new feature? I am experiencing the Authorization header being stripped in Firefox Developer Edition, even though the response is not a cross-origin redirect. This is a request from a SPA using the Axios library to an API on the same subdomain & both using HTTPS. Setting network.fetch.redirect.stripAuthHeader = false makes it work again.

@teoli2003
Copy link
Member

@schalkneethling Can you help @rhclayto ?

@hamishwillee
Copy link
Collaborator

@rhclayto While I am not an expert, that sounds like a bug. I would check FF Bugzilla to see if there is anything related. If not, I'd create a bug report with enough info for this to be reproduced and then cross link from https://bugzilla.mozilla.org/show_bug.cgi?id=1802086

@Rumyra
Copy link
Collaborator

Rumyra commented Mar 14, 2023

Pending bcd I'll close as complete 👍

@Rumyra Rumyra closed this as completed Mar 14, 2023
@rhclayto
Copy link

@rhclayto While I am not an expert, that sounds like a bug. I would check FF Bugzilla to see if there is anything related. If not, I'd create a bug report with enough info for this to be reproduced and then cross link from https://bugzilla.mozilla.org/show_bug.cgi?id=1802086

Thank you. I did file a bug & this was resolved (problem on my end). This new feature helped me discover a ecurity bug in my own code.

@kolaente
Copy link

@rhclayto Can you provide more details about the problrm you had, maybe link the bug you reported? We're hitting the same thing and I'm not quite sure what to look for in fixing this.

@rhclayto
Copy link

rhclayto commented Mar 21, 2023

@rhclayto Can you provide more details about the problrm you had, maybe link the bug you reported? We're hitting the same thing and I'm not quite sure what to look for in fixing this.

Here is the bugzilla report: https://bugzilla.mozilla.org/show_bug.cgi?id=1821881

It may be of no help to you, since soon after reporting it, I discovered that my front end code was making a request to a plain http endpoint, instead of https, & the server was doing a redirect to https, thus triggering this behavior. A typo on my part. I think an http to https upgrade will count as a redirect & cause Authorization header stripping, with the current Firefox devel implementation of the new spec. I hope you get it all sorted out without too much trouble!

@Josh-Cena Josh-Cena added fx release archive A closed issue relating to firefox release notes for developers. and removed waiting for implementations Waiting for feature to be implemented in browsers Firefox 111 labels Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTTP HTTP docs Content:WebAPI Web API docs effort: medium This task is a medium effort. fx release archive A closed issue relating to firefox release notes for developers. help wanted If you know something about this topic, we would love your help!
Projects
Archived in project
Development

No branches or pull requests

9 participants