The-Commit-Company / frappe-react-sdk

React hooks for Frappe
https://frappe-react.vercel.app
MIT License
107 stars 36 forks source link

socketio throwing CORS error when using the SDK on a cross domain app #23

Closed IMS94 closed 1 year ago

IMS94 commented 1 year ago

I'm implementing an app where the frontend is hosted separately on a different domain and use frappe as the backend. Then all the requests that go to the frappe API are cross origin requests.

Let's assume my frappe site is in myapp.frappe.cloud and my react app is in app.myorg.com. I have tried setting allow_cors config in site_config.json in the frappe site. It fixed the initial CORS issue that we get when we send the login request. Now after I login with https://myapp.frappe.cloud/api/method/login, I see 2 issues:

  1. The cookies returned by the frappe site has Same-Site: Lax which is expected
  2. When I send a subsequent request to the frappe API, cookies (returned above at login success) are note sent. Tt suree client should have withCredentials: true option set for this to work.

I'm not sure if this needs to be address in this library or https://github.com/The-Commit-Company/frappe-js-sdk repo. But I believe this is a much needed thing.

I'm happy to assist in any development needed.

nikkothari22 commented 1 year ago

Hey @IMS94

The two issues you pointed out are actually linked to one another. Frappe sends cookies with a SameSite header that doesn't allow sharing cookies. Since your URL is different from your backend URL, the cookie does not get sent.

allowCredentials is set to true in frappe-js-SDK - otherwise it wouldn't work even when the React app was running on the same URL.

Can you check if cookies are being set in your browser after you login. You can check this in the application tab of Chrome Dev Tools.

AFAIK, the best way to use Frappe on another URL is to use OAuth. The library supports passing in OAuth bearer keys with every request. But you would have to implement OAuth on your frontend yourself.

IMS94 commented 1 year ago

@nikkothari22 Thanks for the quick response. And thanks for creating this awesome library.

It makes sense. This could be a case of cookie not persisting or browser not sending the cookie since Same-Site: Lax restriction. And I also see a lot of CORS header missing errors for the socket.io connection:

Access to XMLHttpRequest at 'https://app.frappe.cloud/socket.io/?EIO=4&transport=polling&t=OZ6uDf4' from origin 'http://localhost:3000' has been blocked by CORS policy: The 'Access-Control-Allow-Origin' header has a value 'https://app.frappe.cloud' that is not equal to the supplied origin.

As per https://github.com/frappe/frappe/issues/14940 we can set the device to mobile. Does the frappe-react-sdk expose https://github.com/The-Commit-Company/frappe-js-sdk/blob/main/src/auth/index.ts#L37 method?

OAuth2 Approach

I gave a try with the OAuth2 approach. I got a valid access token and used it as below (hardcoded for PoC purpose only).

root.render(
  <React.StrictMode>
    <FrappeProvider url='https://myapp.frappe.cloud'
      tokenParams={{
        useToken: true,
        token: 'FszBSL83IyiQwqMTNo7KIBT7i3KFLA',
        type: 'Bearer'
      }}
    >
      <App />
    </FrappeProvider>
  </React.StrictMode>
);

But now I get this error from the library:

2frappe-react-sdk.es.js:1337 Uncaught TypeError: t is not a function
    at Gr (frappe-react-sdk.es.js:1337:1)
    at Wi (frappe-react-sdk.es.js:1326:1)
    at new o (frappe-react-sdk.es.js:1348:1)
    at frappe-react-sdk.es.js:4059:1
    at mountMemo (react-dom.development.js:17225:1)
    at Object.useMemo (react-dom.development.js:17670:1)
    at useMemo (react.development.js:1650:1)
    at ba (frappe-react-sdk.es.js:4058:1)
    at renderWithHooks (react-dom.development.js:16305:1)
    at mountIndeterminateComponent (react-dom.development.js:20074:1)

Any idea why?

IMS94 commented 1 year ago

I tried sending the device: mobile field in the login request (via cURL) and in the response I can see that the SameSite property is not sent now.

< set-cookie: sid=5f614deb0c768c01fc86e6f58efa8bc4a7718a11fd7c4689734533fc; Expires=Tue, 20 Jun 2023 01:55:24 GMT; Secure; HttpOnly; Path=/
< set-cookie: system_user=yes; Secure; Path=/
< set-cookie: full_name=Imesha%20Sudasingha; Secure; Path=/
< set-cookie: user_id=imesha%40example.com; Secure; Path=/
< set-cookie: user_image=; Secure; Path=/

But unfortunately, browsers interpret as SameSite: Lax by default. Do we have a workaround for that?

nikkothari22 commented 1 year ago

Device mobile has now been deprecated by Frappe in version 14. Hence it won't help.

To use a token, you need to pass a function to the token field and not a string. We designed it this way because in most cases you would want to load the latest token from either a third party auth provider library like Firebase or Auth0, or would want to load it from localStorage.

So instead of passing a string, just pass a function that returns the string.

IMS94 commented 1 year ago

@nikkothari22 Passing a function worked for the OAuth2 approach.

However, still the socket IO requests are getting CORS errors. I have opened https://github.com/frappe/frappe/issues/21413 to request the allow_cors site config to be applied to the socket being created too. Without socket IO, I can't listen for doc changes and listening for doc changes is a very valuable feature in general.

Do you have an alternative suggestion for CORS issue in socket IO?

nikkothari22 commented 1 year ago

I haven't tried listening to socket events from a different domain yet, so I didn't even know about this bug. It would be a bug with Frappe though.

Are you facing the issue on production or your local dev environment?

IMS94 commented 1 year ago

@nikkothari22 I face this in both cases. And yes, this is an issue with frappe IMO. I have created https://github.com/frappe/frappe/issues/21413 to track that.

IMS94 commented 1 year ago

@nikkothari22 can we give an option in frappe-react-sdk to disable socket.io client creation? This will be useful in cross domain scenarios.

nikkothari22 commented 1 year ago

@IMS94 We are working with the Frappe team to add support for Socket for Non-native environments like React Native.

SocketIO should ideally work in cross domain, but I guess since it's cookie based, it's not working as of now.

Regardless, I'll add an option to disable Socket in the library.

IMS94 commented 1 year ago

@nikkothari22 thanks for the prompt response and addressing this.

chechani commented 1 year ago

After today's update, I am also getting socket io cors errors. on a continuous basis, though research further I will do understand the case

nikkothari22 commented 1 year ago

@chechani - are you using the library cross domain - so your backend is hosted on another domain or subdomain?

nikkothari22 commented 1 year ago

@nikkothari22 thanks for the prompt response and addressing this.

Let me know if the solution worked. You need to pass false in the Frappe Provider for the enable socket flag.

chechani commented 1 year ago

Sorry, After the update, socket io cors errors have been resolved.

asharamseervi commented 1 year ago

We are working with the Frappe team to add support for Socket for Non-native environments like React Native.

Hi, @nikkothari22, Thanks for your wonderful contribution. I've a concern regarding React Native compatibility; if you would like to update-

Thanks in advance.

nikkothari22 commented 1 year ago

We are working with the Frappe team to add support for Socket for Non-native environments like React Native.

Hi, @nikkothari22, Thanks for your wonderful contribution. I've a concern regarding React Native compatibility; if you would like to update-

  • I was wondering if there is any progress for React Native.

  • Is this React SDK fit with React Native development?

Thanks in advance.

Hey @asharamseervi ,

Two things won't work with React Native - cookie based authentication and Socket events. For authentication, you'll have to to use token based auth - which is supported by the SDK. All other hooks will work since it's using Axios under the hood.

Hussain Nagaria has a Build With Hussain episode on YouTube wherein he uses frappe-js-sdk to build a React Native app with Frappe.

We tried using frappe-React-SDK for Raven's mobile app using React Native, but since socket events were critical for Raven, we decided to not continue with RN (among other reasons). If you don't want real-time events, you can use the SDK. If you want real-time events, you can try contributing to the Frappe core so that it supports socket events with token based auth.

Thanks!