Bug 247418

Summary:	Safari reuses Authorization header on second call to 301 redirects even if the header value changed when replaying the request
Product:	WebKit	Reporter:	Santiago Saavedra <info+webkit>
Component:	Page Loading	Assignee:	youenn fablet <youennf>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	achristensen, annevk, beidson, cdumez, rreno, webkit-bug-importer, youennf
Priority:	P2	Keywords:	InRadar
Version:	Safari 16
Hardware:	All
OS:	Unspecified

Description Santiago Saavedra 2022-11-03 04:00:43 PDT

We have a server that redirects non-canonical API calls to their canonical counterpart using 301 Moved Permanently redirects.

This bug can be replicated using fetch(), with any non-canonical API call (e.g., /api/users instead of /api/users/).

If we start with a clear cache, everything works. However, if we log in onto the web app (i.e., generate a new token), perform the API call, then log out (invalidating the token on the server side) and back in again, now the API call will fail with a 401 since the token used will be the old one that has been invalidated.

The first request will use the correct Authorization header, but after the server sends the 301 redirect response, Safari will use the old Authorization header for the next request to the /api/users/ call, (which uses a token that has already been invalidated).

This is not happening on Chrome or Firefox on macOS or Windows, but is happening on Safari 16.1 for macOS and on iOS 16.0.3.

Comment 1 Radar WebKit Bug Importer 2022-11-03 15:53:32 PDT

<rdar://problem/101935060>

Comment 2 Anne van Kesteren 2022-11-14 03:24:07 PST

I strongly suspect bug 230935 is related.

Comment 3 youenn fablet 2022-11-14 04:55:19 PST

@Santiago, do you have a repro case? I guess you are using non basic authentication scheme?

Comment 4 Santiago Saavedra 2022-11-14 07:51:55 PST

@youenn the app where this is happening to us is confidential, but I have put together this test scenario on the public domain for replicability :-)

https://github.com/ssaavedra/webkit-repro-247418

Comment 5 youenn fablet 2022-11-15 01:18:24 PST

Pull request: https://github.com/WebKit/WebKit/pull/6510

Comment 6 youenn fablet 2022-11-15 01:21:02 PST

Thanks Santiago, the repro case helped a lot.

Comment 7 Santiago Saavedra 2022-11-15 04:26:14 PST

Really pleased at how much more succint you made your test case than mine, sorry I couldn't commit more time to minimizing my repro case further :)

Thank you so much for putting the PR together!


On a separate note:

I haven't tested this but would the behavior still be compliant if the header was named anything other than Authorization? (E.g., some X-API-Key-Header)

I'm just curious about whether Authorization has some specific deeper meaning in the CFNetwork/WebKit codebase than any other header/s.

Thanks again!

Comment 8 youenn fablet 2022-11-15 04:47:22 PST

> I haven't tested this but would the behavior still be compliant if the
> header was named anything other than Authorization? (E.g., some
> X-API-Key-Header)

Some headers need to be dropped. Authorisation in case of cross origin redirection.
Content-Type in case of 303 also.
@Anne, maybe that is something that should be more explicit in the fetch spec?

For other application specific headers, it is true they should probably be cloned from the original request.
We have the concept of originalRequestHeaders, these are the ones we should probably always copy.

Comment 9 youenn fablet 2022-11-15 04:49:45 PST

I filed https://bugs.webkit.org/show_bug.cgi?id=247942 to keep track of this.

Comment 10 EWS 2022-11-16 00:55:18 PST

Committed 256726@main (63145fa91bb8): <https://commits.webkit.org/256726@main>

Reviewed commits have been landed. Closing PR #6510 and removing active labels.