Repository: react-native
Issue: iOS - Duplicate HTTP requests occur when an authorisation challenge is returned (401 with www-authenticate header)
Labels: Platform: iOS,🌐Networking,Needs: Triage :mag:,Never gets stale

### Description

RN's iOS code is not working with authentication challenges in the way the Foundation framework expects and this results in extraneous, duplicate requests to login when bad credentials are presented to the server. It seems this was first noticed by someone in 2015 but didn't go anywhere: https://github.com/facebook/react-native/issues/2266

In our instance, this is causing us issues with server-side brute force protection on login as iOS users get half the number of real-world attempts to log in, compared to Android users, before they hit our brute force limit.

The documentation for `NSURLRequest` states that, among others, `Authorization` is a reserved header and that:

>If you set a value for one of these reserved headers, the system may ignore the value you set, or overwrite it with its own value, or simply not send it. Moreover, the exact behaviour may change over time. To avoid confusing problems like this, do not set these headers directly. 

(see https://developer.apple.com/documentation/foundation/nsurlrequest?language=objc#1776617)

So, this header should not be set directly by RN but currently is.

### Current code path

If an Authorization header is set in JS, the JS layer passes this on to `RCTNetworking.sendRequest`: https://github.com/facebook/react-native/blob/v0.70.2/Libraries/Network/XMLHttpRequest.js#L584

`sendRequest` then calls `buildRequest` and this copies all the headers onto the `NSURLRequest` (including all the reserved headers): https://github.com/facebook/react-native/blob/b9c297537a5eda68621ba77ead272cfd8b4c6e2c/Libraries/Network/RCTNetworking.mm#L302

This `NSURLRequest` makes its way down to an `NSURLSessionTask` in `RCTHTTPRequestHandler.mm` and that task is eventually run: https://github.com/facebook/react-native/blob/b9c297537a5eda68621ba77ead272cfd8b4c6e2c/Libraries/Network/RCTHTTPRequestHandler.mm#L108

`RCTHTTPRequestHandler` is an `NSURLSessionDataDelegate` but doesn't implement the authentication challenge delegate methods so the `NSURLSession` runs its default handling which results in the duplicate 401 request (seemingly because Foundation did not expect the Authorization header to have been set directly on the NSURLRequest).

### Foundation's expected code path

What Foundation expects you to do is documented here: https://developer.apple.com/documentation/foundation/url_loading_system/handling_an_authentication_challenge?language=objc

Essentially:

1. The reserved `Authorization` header (and other reserved headers) should **not** be populated on the `NSURLRequest` directly.
2. The `NSURLSessionTask` runs and requests the specified URL with no credentials, it received a 401 response from the server.
3. `RCTHTTPRequestHandler` (or another `NSURLSessionDelegate`) picks up the authentication challenge via `URLSession:task:didReceiveChallenge:completionHandler:` and provides an `NSURLCredential` for the completion handler to use.
4. If this succeeds, all is good. If it still 401s then the `URLSession:task:didReceiveChallenge:completionHandler:` handler is called again but this time it has the context of `challenge.proposedCredential` and `challenge.previousFailureCount` so can detect that the `NSURLCredential` was denied and change its behaviour as a result (the handler does not get the context of a previous failure if a raw `Authorization` header was added).

### Naive patch for this issue

I've implemented a naive patch in the PoC mentioned below: https://github.com/liamjones/RNDuplicateRequestsBug/blob/main/patches/react-native%2B0.70.2.patch

I'm pretty rusty on Objective-C so I took the simplest route (to me) to play with ways of fixing this. In summary the patch:
1. Renames the `Authorization` header to `Was-Authorization` in `buildRequest` so it doesn't trigger a 401 with credentials on the server (it instead generates a 401 with no credentials - this stops it from recording a strike against our account brute force protection counter)
2. Implements `URLSession:task:didReceiveChallenge:completionHandler:` in `RCTHTTPRequestHandler` to deal with the 401. This then goes through checking that we're dealing with a Basic auth request and, if we've not dealt with it already, grabs the `Was-Authorization` header off the request, base-64 decodes and splits it to retrieve the username and password in plaintext so it can use them to construct an `NSURLCredential`.

If `URLSession:task:didReceiveChallenge:completionHandler:` gets called a second time with the `challenge.proposedCredential` and `challenge.previousFailureCount` it uses these as a signal to abort because the credentials are wrong. It feels like what _should_ happen is I call `completionHandler(NSURLSessionAuthChallengeCancelAuthenticationChallenge, nil);` but, with the current RN setup that results in a 'Network request failure' promise rejection in the JS layer. If instead, I say `completionHandler(NSURLSessionAuthChallengeRejectProtectionSpace, nil);` - essentially, we can't do Basic anymore - the 401 does correctly make its way up to the JS layer. A side effect is that we end up with 3 401 requests now:
1. 401 without credentials
2. 401 with the NSURLCredential
3. 401 without credentials again - I assume this one comes from the fact we've said "we can't do Basic auth anymore" and the NSURLSession is trying to work out if there's another auth challenge method that could be used (such as NTLM).

This obviously isn't ideal (using the renamed header to pass info around is clunky and this renamed header will still be sent to the server, etc) but fixing this properly is beyond my rusty Objective-C skills/the time I have available currently.

There's one extra wrinkle with this solution;

in the 0.70.2 PoC, it's working fine with correct credentials that result in a 200. You get the initial 401 with no credentials, then the 200 with credentials, and this 200 response is returned up to the JS layer.

However, it's _not_ working correctly with our real-world 0.66.3 app. Upon a successful 200 response, it's the original 401 without credentials that ends up in the JS layer.

I'm guessing this is either a race condition (PoC = light and fast, our app = heavier and slower?) or something that's changed between 0.66.3 and 0.70.2 which has changed behaviour. Upgrading our app to 0.70.2 is something I want to do but is not simple due to various native dependency conflicts/upgrades I need to sort out.

I had a quick look at what had changed in `Libraries/Network` between 0.66.3 and 0.70.2 but the only commit that looked potentially relevant was this: https://github.com/facebook/react-native/commit/5ed6ac1f25b0b793c74ffc23005a82530618eb55 I backported the change to 0.66.3 but it still didn't resolve the 200 issue in our app. So it's either a race condition or a change somewhere else in RN which has altered behaviour between those versions.

### Version

0.70.2 & 0.66.3

### Output of `npx react-native info`

System:
    OS: macOS 12.6
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 205.13 MB / 32.00 GB
    Shell: 5.9 - /usr/local/bin/zsh
  Binaries:
    Node: 18.10.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.19.2 - /usr/local/bin/npm
    Watchman: 2022.10.03.00 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.11.3 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.4, iOS 16.0, macOS 12.3, tvOS 16.0, watchOS 9.0
    Android SDK:
      API Levels: 23, 25, 26, 27, 28, 29, 30, 31, 32
      Build Tools: 27.0.3, 28.0.3, 29.0.2, 29.0.3, 30.0.1, 30.0.2, 30.0.3, 31.0.0, 31.0.0
      System Images: android-24 | Google APIs Intel x86 Atom, android-24 | Google Play Intel x86 Atom, android-25 | Google APIs Intel x86 Atom, android-25 | Google Play Intel x86 Atom, android-26 | Google Play Intel x86 Atom, android-27 | Google Play Intel x86 Atom, android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom, android-29 | Google APIs Intel x86 Atom, android-30 | Google APIs Intel x86 Atom, android-30 | Google Play Intel x86 Atom, android-31 | Google APIs Intel x86 Atom_64, android-31 | Google Play Intel x86 Atom_64, android-32 | Google Play Intel x86 Atom_64, android-33 | Google Play Intel x86 Atom_64, android-S | Google Play Intel x86 Atom_64, android-Tiramisu | Google Play Intel x86 Atom_64
      Android NDK: Not Found
  IDEs:
    Android Studio: Dolphin 2021.3.1 Dolphin 2021.3.1
    Xcode: 14.0.1/14A400 - /usr/bin/xcodebuild
  Languages:
    Java: 11.0.15 - /Users/liam.jones/.sdkman/candidates/java/11.0.15-tem/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.1.0 => 18.1.0
    react-native: 0.70.2 => 0.70.2
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

### Steps to reproduce

1. Issue a fetch request with an `Authorization: Basic ...` header to a server endpoint which will return with a `401` and `WWW-Authenticate: Basic` header.
2. Observe network logs in some way, see 2 401s with the credentials were sent to the server rather than 1

This only happens on iOS, not Android.

Example code:

```typescript
  fetch('https://mockbin.org/bin/3275aeb4-a08d-43cd-9ed1-7c2410277124', {
    headers: {
      Authorization: `Basic ${new Buffer('bad@example.org:password').toString('base64')}`,
    },
  })
```

### Snack, code example, screenshot, or link to a repository

https://github.com/liamjones/RNDuplicateRequestsBug

`yarn`, `yarn start`, `yarn ios` and tap the buttons in the UI. The 401 button will result in 2 network requests. Tapping the 200 button will result in 1 request. The Mockbin `/view` URLs in the `App.tsx` can be used to see the requests (or you can reconfigure the URLs to point somewhere else **if you don't want your request IP on a public page**).

If changing the URLs; the important thing is that the 401 URL returns a 401 with a `WWW-Authenticate: Basic` with each request (to simulate the first request's credentials being rejected).

Running `yarn patch-package` followed by `yarn ios` again will apply a naive patch to RN which stops the duplicate requests with credentials against the 401 endpoint (but does result in an extra request to the endpoint without the credentials) - explanation of the patch is in the description above.