adambrgmn / react-oauth-flow

An OAuth2 flow for React apps
MIT License
94 stars 37 forks source link

Allow for custom fetch function in OauthReceiver #28

Closed aguscha333 closed 6 years ago

aguscha333 commented 6 years ago

Hi, I am developing a website in react with rails backend. I am using your library to connect the user with their Shopify account.

We are using Devise on the backend for authentication handling and the user is already logged in when the Shopify connection happens. Because of this I need to send some headers to the backend on the fetch call that the OauthReceiver does. I saw that that was fixed in the latest version that was released a couple of days ago.

My problem is that in the response of the backend there are some headers that I need to save, mainly the access-token, they are a one time use so when I use one for a call to the backend, in the response it already sends me a new one to use next.

I see that in your code the access-token comes inside the body of the response and not the headers so that's why you are able to get it and I am not.

As a quick fix I copied the OauthReceiver component to my code and made the modifications I needed so instead of using the fetch2 I am using my own api service fetch function which already parses the headers and saves them with redux-react-session.

I think adding the possibility to use your own fetch function is a great addition, it just gives us more flexibility and I think more often than not we already have an api service that communicates with our backend in a specific way.

Here are the small changes I made:

 api.post(url)
  .then((response) => {
    if (typeof onAuthSuccess === 'function') {
      onAuthSuccess(response);
    }

    this.setState(() => ({ processing: false }));
  })
  .catch((err) => {
    this.handleError(err);
    this.setState(() => ({ processing: false }));
  });

I just changes the fist line to use my fetch and then the onAuthSuccess line as well since the access-token is already being taken care of by the api.post and I don't need the state.

If you like the idea of adding the possibility to have a custom fetch function I can work on a more generic solution that not only accommodates my custom fetch but from others as well.

adambrgmn commented 6 years ago

Hi @aguscha333! Thanks for reaching out!

As you might have understood, so far this library have been suited solely to my needs, and therefore I understand that it's quite narrow regarding use-cases.

But your idea sounds reasonable, and I would really appreciate if you can work out something general that could benefit others.

Just on the top of my head I think that a prop called something like tokenFn, just api, or similar, would work, where the user can choose to supply their own function to fetch the token. It should probably accept the same kind of args as my fetch-function.

I'm looking forward to see if you can work something out!

aguscha333 commented 6 years ago

Yeah, that's the solution I though too. I already made the changes to the code, edited the documentation accordingly and created some tests. I will test it on Monday on my project at work and submit a PR if everything works as expected.

adambrgmn commented 6 years ago

:tada: This issue has been resolved in version 1.2.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: