withCredentials flag in XHRs should default to "true"

[DanielZlotin]

Description

react-native 0.44 introduced withCredentials flag in XHRs, which, if not specified in every fetch request, defaults to false.
This change conflicts with the default behavior in native. In the iOS native SDK and the Android native SDK, when making a native HTTP request, cookies are sent by default.
We rarely have agreement between the platforms, but for the last 10 years they both agree on this security model for apps.
This greatly affects projects relying on cookies with their requests.

Additional Information

• React Native version: 0.44
• Platform: both
• Development Operating System: macOS

[talkol]

According to the commit description, the reason for this breaking change is to be

"consistent with the default behavior of XHR on web for cross-site requests".

This doesn't make much sense to me. These are native apps. Native apps don't have cross-site concerns. The JS bundle is not served from a domain like the web.

The security model for native mobile apps has been established a long time ago. The standard native API's for making HTTP requests in iOS and Android send cookies by default. Changing this behavior to conform to websites just because we're using JavaScript is strange.

In addition, there's a big problem with the override mechanism. This is a breaking change, and now we have apps in production that we cannot release due to this change.

The override mechanism according to the commit is:

"Developers can restore the previous behavior by passing true for XHR's withCredentials argument".

This makes the assumption that we can control the parameters for every request our app makes. Consider that we're using a 3rd party GraphQL client library that makes the fetch requests for us. This library is out of our control meaning we can't use the override mechanism. To support backwards compatibility for existing apps that are in production when introducing these types of changes, the minimum is to allow a global override when the app starts.

@rigdern

[rigdern]

I'm sorry that my commit is causing issues for you.

The standard native API's for making HTTP requests in iOS and Android send cookies by default.

Which Android API are you referring to?

I think there are several questions to think about here:

1. What should RN's withCredentials default to?

The answer is not obvious to me. Ignoring the web, different APIs I'm familiar with have made different choices regarding the default for sending and saving cookies:

• Libraries that disable cookies by default:
  • The request module for Node. You have to set jar to true to enable cookies.
  • OkHttp for Android. You have to provide a CookieJar to enable cookies.
• Libraries that enable cookies by default:
  • NSMutableURLRequest built into iOS. You have to set HTTPShouldHandleCookies to false to disable cookies.

I'm not familiar with the rationale behind the chosen defaults of any of these libraries. If anybody is deeply familiar with this, it would be useful if you could provide or link to an explanation. Hearing about the rationale behind withCredentials in browsers would be especially interesting because React Native copies XHR's API surface. Understanding all of this will be helpful in picking the right default for React Native.

2. Should libraries that make HTTP requests expose withCredentials?

If they don't expose withCredentials, it seems like you could run into similar problems in a web app when you're making requests to another domain. Is that correct? If so, how would you solve this problem in a web app?

3. Should RN make the default of withCredentials configurable?

Because changing the default of withCredentials was a breaking change, this might be useful to help apps adjust to the breaking change.

Disregarding the breaking change, would such an API be a good idea? I don't know. How do other HTTP APIs solve this problem? Do they give you a switch for globally enabling/disabling cookies?

Adam Comella
Microsoft Corp.

[isnifer]

It's time to follow semver :)

[itsmepetrov]

We also faced with this problem, but fortunately, we have direct access to all API calls in our app.
As a workaround, we use fetch with credentials: 'include'.

[vafada]

This broke our app too. The original fix looks like it conflicts with:

https://github.com/github/fetch/blob/08602ff819f4c41e9d9e9c2c31bfc853b1bb5bf2/fetch.js#L448-L450

It seems to me there a lot of places which sets withCredentials and each place does different things.

[rigdern]

It seems to me there a lot of places which sets withCredentials and each place does different things.

@vafada What places are you referring to? I believe the place you linked to in an implementation of fetch is fine. When you pass credentials: 'include' to fetch, it should have the same behavior as setting withCredentials to true in XMLHttpRequest. That's exactly the case the code you linked to is handling.

[vafada]

@rigdern

Sorry, I just didn't understand the code well enough:

I was doing:

fetch(url, {
   ...options
   withCredentials: true,
  credentials: 'include',
});

when I should be doing:

fetch(url, {
   ...options
  credentials: 'include', // sets    withCredentials to true
});

[itsmepetrov]

Also, what about credentials: 'same-origin'? Should it work as a fallback to 'include' or something else?

[shergin]

I personally agree with @rigdern, cookies should be disabled by default.
It is kinda standard nowadays (not only for browsers) that Cookies is opt-in feature.

We simply have to adopt new policy. Peace.

[ericvicenti]

Apologies for not taking this under more careful consideration when reviewing the pull request! I thought this would be a strict win because it brings the two platforms in alignment, but as @talkol points out, it now conflicts with the behavior of the native networking libraries.

At this point I think it may be worthwhile to keep the new behavior, because we've already switched it, it matches the behavior of JS fetch on the web, and it offers a slightly better security profile. Also, as I understand, the new behavior brings iOS in line with Android.

There are some tradeoffs here so I'd like to run a quick community poll for those paying attention to this issue. Please vote within the next 24 hours:

Vote Here!

Please "thumbs-up" this post if you want a mechanism to change the default. But we will keep the new behavior that was introduced in 0.44. (fetch default is credentials: 'omit').

To enable people to use newer versions of RN, we will add a mechanism to return the default to true. We will cherry-pick this new mechanism to 0.44 and 0.45. Pending naming, it would look like this:

fetch.setDefaultCredentials('include');

Give this comment a "thumbs-down" if you want to revert to the default credentials: 'include' behavior that we had on iOS before 0.44.

We could theoretically do this by reverting 454ab8, but it would probably be cleaner to override the default from fetch.js.

As a followup, we will need to decide what to do with the Android behavior.

[ericvicenti]

Ok, its only been an hour and we've got pretty clear signal: 13 votes to revert to the old credentials default, and 1 vote to keep the consistent behavior with override mechanism. I'll let the vote keep going for the next day, but it sounds like we should go back to the old default.

[satya164]

So current plan is to undo the breaking change. In long term, we probably want to default to not sending cookies for fetch by default (which is the for both same origin and cross origin on web), and leave XMLHttpRequest as is.

[grabbou]

I'll cherry-pick and release new versions today.

[nsisodiya]

@grabbou waiting. will it solve this issue - #14154

[talkol]

I want to return to the discussion of what is the correct behavior in the long term.

I think that the vision behind React Native is to respect the different platforms and not to force web mentality over them. React Native is not web-first. Forcing all platforms to behave like the web is what killed several competing cross-platform frameworks for native developers such as myself. We don't want to make this mistake and alienate native developers.

I know that many of the people in this thread are primarily web developers. Please make an effort to understand where the other platforms are coming from.

I asked @DanielZlotin to showcase the default behavior in (pure) native mobile in iOS and Android. Hopefully this will explain what we're used to:

Example Code

The example has Objective-C + Java code which uses default native APIs for fetching data:
https://github.com/wix/react-native-cookie-example
And a simple web service that stores a cookie and shows it:
https://stark-atoll-33661.herokuapp.com/cookie.php

Analysis of the defaults for iOS

https://github.com/wix/react-native-cookie-example/tree/master/ios/CookieExample

The default API doesn't require anything special related to cookies. Cookies are stored by default for all domains. Native code has full access to all cookies anyways so it doesn't make sense to limit them.

I tried to find the defaults in the code documentation as well:

/*!
    @method HTTPShouldHandleCookies
    @abstract Decide whether default cookie handling will happen for 
    this request.
    @param YES if cookies should be sent with and set for this request; 
    otherwise NO.
    @discussion The default is YES - in other words, cookies are sent from and 
    stored to the cookie manager by default.
    NOTE: In releases prior to 10.3, this value is ignored
*/
@property BOOL HTTPShouldHandleCookies;

Analysis of the defaults for Android

https://github.com/wix/react-native-cookie-example/tree/master/android/CookieExample

Android is more tricky because they chose to base their original HTTP API on the standard Java API. The Java API tries to make zero assumptions on platform and predated mobile, so it's hard to understand the platform state of mind from it. Newer API like okhttp conforms to the same API style.

The Java API is a very low level API with very few abstractions. You have to do everything manually, including specify your cookie storage implementation (so it's not tied to a specific one). The fact that you need to specify it IMO does not reflect that cookies are disabled.

There are 3 main cookie policies and the default policy is set by CookieManager.setDefault(new CookieManager());. As you can see, it is not ACCEPT_NONE, it is ACCEPT_ORIGINAL_SERVER.

I tried to find this also in the code documentation:

/**
     * One pre-defined policy which only accepts cookies from original server.
     */
    public static final CookiePolicy ACCEPT_ORIGINAL_SERVER  = new CookiePolicy(){
        public boolean shouldAccept(URI uri, HttpCookie cookie) {
            return HttpCookie.domainMatches(cookie.getDomain(), uri.getHost());
        }
    };

The original server policy means that as long as any HTTP server specifies their own domain on the cookies, the cookies are saved and returned. You can see this behavior in the simple example above.

My recommendations

1. Keep a constant behavior for iOS and Android. Don't change defaults between the native platforms since they are similar in spirit in this case.

2. The defaults should be based on the default security model for each platform. Websites run inside a browser sandbox. Native apps don't have a sandbox and have full access to stored cookies (you're implementing the browser yourself).

3. Allow global overrides for this behavior. Don't limit to per-call overrides. Allow to override the behavior of both XHR and fetch.

4. Keep the defaults identical between XHR and fetch to minimize confusion.

What happens if my app behaves differently in the web or as native app on device?

I think that's part of the point. If you're specifying a specific behavior, it will be respected. If you're not, you're expecting the defaults to behave correctly. If you're running in a web browser, there's no trust between the user and you and the user should be protected. If the user chose to install you natively and showed intent to have a relationship with you, there's more trust and we can provide a more intimate relationship.

In other words, it's not "write once, run anywhere", it's "learn once, write anywhere".

[shergin]

@talkol Tal,
I don't quite understand how (1) can be satisfied with (2). So, you suggest (1) to have same defaults for all platforms, (2) these defaults (many of them?) should be based on platform spirit (which is can be different). 🤔

[talkol]

@shergin I meant iOS and Android, the first two platforms, should have same defaults. Third platform is web, so if you're targeting your codebase for web (by sharing the same JS implementation) then you'll get the browser defaults naturally which can be different. And any other platforms like native desktop should have their own defaults.

[prithsharma]

I see that we are not considering another possible value - same-origin in this discussion. It is a part of the fetch API docs for Request.credentials.
Is it because there is no such thing as 'origin of the calling script' here and thus same-origin is irrelevant?

[nsisodiya]

Just to add the discuss. I have created an app using CRNA. I implemented login mechanism using cookie. Server use Set-Cookie header to put a JWT token. login mechanism is working fine but there is just one problem. iPhone app (right now playing using EXPO client) require me to login again and agian. every time I close the app, it ask for login. it means, at iPhone, when I close the app, It do not preserve the cookie. At the other hand, Even If I reboot android phone, my app do not ask for password. it means, Android app is preserving cookie.

is this problem related to this issue? I am using credentials: "include", for fetch. If anybody know workaround, let me know.

[hramos]

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

• Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
• If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
• Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.