maxmantz / redux-oidc

A package for managing OpenID Connect authentication in ReactJS / Redux apps
MIT License
400 stars 111 forks source link

How do I set up proper API calls, after the user refreshes the application and the user object already exists #19

Closed michaelklopf closed 7 years ago

michaelklopf commented 7 years ago

Hi,

I'm mentally stuck how to tackle my problem properly. I use redux-oidc to authorize myself, user is set up by the library and I can do my API calls with the user token in the state, like this:

// API requests - done with redux-thunk
export function fetchData() {
  return function(dispatch, getState) {
    const token = getState().oidc.user.access_token;
    if (token === '') {
      return;
    }

    dispatch(requestData());

    const request = new Request(fetchDataUrl, {
      method: 'GET',
      headers: getAuthHeader(token)
    });

    return fetch(request)
      .then(response => response.json())
      .then(json => {
        if (json.StatusCode !== 200) {
          triggerNotification(json.StatusCode, json.ResponseMessage);
        }
        return json;
      })
      .then(json => dispatch(receiveData(json)))
      .catch(() => {
        triggerNotification(900);
      });
  };
}

The problem is, when the user reloads the page, there is no user in the state, because the middleware is not yet finished with adding the user to it.

Using it like this in a component:

class Data extends Component {
  componentWillMount() {
    this.props.fetchData().then(() => {
      this.props.sortData(this.props.sort.column, this.props.sort.direction);
    });
  }
  ....

I don't know how to tackle that problem. I was thinking about the userManger.getUser().then() promise, but I wasn't able to make it work yet. Any ideas what to pay attention to?

Is there a hook in the library to only do the fetchData when the user is loaded into the state?

I hope it's ok if I ask that here.

maxmantz commented 7 years ago

Hi!

It's perfectly fine to ask here :)

I don't know the hierarchy of your app's components, but I guess that the Data component is at the root of your app. If that's the case you can run into these problems where the user doesn't exist.

To remedy that, have your app's root component be a wrapper which only renders the content when the user is present. That way you can be sure the user is loaded when your Data component makes an API request. I was thinking about something like this:

class App extends React.Component {
  //....

  render() {
    // check whether the OAuth2 / OIDC callback page is requested...
    const isCallbackRendered = location.pathname.indexOf('/callback') !== -1;

    return (
      <div>
         // check for the user here
         {this.props.user || isCallbackRendered ? <Content /> : <LoginPage />}
     </div>
    );
  }
}

Then once the middleware is done loading the user, the content will render, triggering the componentWillMount on the Data component with a guaranteed user present. I've got my example app set up in the same way. It is a neat, simple fix to the problem. Hope this helps!

michaelklopf commented 7 years ago

Here the structure of the main part:

// index.js
const routes = (
  <Route path='/' component={App}>
    <IndexRedirect to="/home" />
    <Route path='/home' component={Home} />
    <Route path='/data' component={Data} />
    ...
    <Route path='/callback' component={CallbackPage} />
  </Route>
);

render(
  <Provider store={store}>
    <OidcProvider store={store} userManager={userManager}>
      <Router history={browserHistory}>{routes}</Router>
    </OidcProvider>
  </Provider>,
  document.getElementById('root')
);

App is the root, which contains a navigation component rendered on one side and the children to render on the right side. The children is the thing that should be currently shown according to the router.

Going directly to http://localhost/data (when already logged in before) triggers the rendering of the App and the Data component. But when componentWillMount of Data is called, it won't have the user yet.

I will go through your text tomorrow again, I'm just writing this to clarify the situation.

Thank you.

maxmantz commented 7 years ago

I understand. This would work if you have your App component check the user like I suggested. You also need to check for the callback route for the API callback to work when you use this approach,

michaelklopf commented 7 years ago

Hey,

I made the necessary changes and the application works in an acceptable state, but I'm not happy that the IS3 server is called after a refresh, despite the fact that a user exists, but is not in the state yet.

With these changes, the application is rendered for a short moment (awkwardly with the right data now), then goes to the IS3, after that the callback comes back and the app is shown.

I implemented it like this:

class App extends Component {
  render() {
    if (!this.props.user || this.props.user.expired) {
      return <LoginPage />;
    } else {
      return (
    .....

And I setup a switch in the Login, but the user is not here yet too. Maybe when I add it to another React hook instead of the constructor, will try that out later.

class LoginPage extends Component {
  constructor(props, context) {
    super(props, context);
    if (!props.user || props.user.expired) {
      userManager.signinRedirect({ 
        data: {
          redirectUrl: window.location.href.split('/').pop()
        }
      });
    } else {
      context.router.push(window.location.href.split('/').pop());
    }
  }

What is your intention with

// check whether the OAuth2 / OIDC callback page is requested...
    const isCallbackRendered = location.pathname.indexOf('/callback') !== -1;

The application is not switching from the "Redirecting" of the CallbackComponent back to the content of my app when I include it.

maxmantz commented 7 years ago

It is strange that your app redirects after a refresh when a user is already present. That shouldn't happen. I'm afraid there's something strange going on with your configuration. The example app does not redirect after a refresh. It uses the same approach I suggested to you so I suggest you to check the repo out and see how my implementation differs from yours in detail.

The isCallbackRendered is required to allow the <CallbackComponent /> to be rendered even when no user is present. This is required for the OIDC token callback to be processed since the callback component is registered at a route that would otherwise not be rendered.

michaelklopf commented 7 years ago

I followed your advice and changed my Login page like this

// LoginPage.jsx
class LoginPage extends Component {
  onLoginButtonClick(event) {
    event.preventDefault();
    if (!this.props.user || this.props.user.expired) {
      userManager.signinRedirect({ 
        data: {
          redirectUrl: window.location.href.split('/').pop()
        }
      });
    }
  }

Having the redirect in the LoginPage was triggering the redirect again. Now it works similar to your example application. I think that the Login Page is rendered for a short period until the user is placed in the state by the oidc library, which results in a rerender, that subsequently renders the app content.

I wanted to make it easier for the user, so I left the login button out, but apparently it's needed here.

maxmantz commented 7 years ago

Happy to hear it worked out.

acaravia commented 6 years ago

Is the login button required? We want users to be automatically directed to the IdentityServer login screen without having to click an additional button when accessing our application. We are experiencing the user refresh as was posted by michaelklof originally.

maxmantz commented 6 years ago

When set up correctly, USER_EXPIRED will be dispatched on startup when no valid user is present. You can listen for that action and perform the login redirect then and there.

acaravia commented 6 years ago

I appreciate your response. I'm very new to Reactjs and Redux. I currently have it working with no refresh but I'm afraid I'm not doing it properly (It's not listening for any specific action). What would a correct setup look like? Here is my setup (had to remove the oidcProvider because it was performing callback twice):

<Provider store={store}>
                    <div>
                        <Navigation />
                        <div className="jumbotron">
                            <div className="container">
                                <div className="col-mg-8 col-sm-offset-2">
                                    <Router history={history}>
                                        <div>
                                            <PrivateRoute exact path="/" component={LandingPage} />
                                            <Route path="/login" component={RedirectToLogin} />
                                            <Route path="/landing" component={LandingPage} />
                                            <Route path="/callback" component={LoginCallback} />
                                            <Route path="/licensees" component={LicenseePage} />
                                            <Route path="/person" component={PersonPage} />
                                        </div>
                                    </Router>
                                </div>
                            </div>
                        </div>
                    </div>
            </Provider>
export const PrivateRoute = ({ component: Component, ...rest }) => (
    <Route {...rest} render={props => (
        authHeader
            ? <Component {...props} />
            : <Redirect to='/login' />
        )} />
)

`//authHeader.js export function authHeader(props) {

console.log("autheHeader store = ", store.getState().oidc);
if (store.getState().oidc.user === null) {
    return;
}

const token = store.getState().oidc.user.access_token;

if (token) {
    return { 'Authorization': 'Bearer ' + token };
} else {
    return { };
}

}`

Additionally, we have a problem with any component that requires a call to our API after successful login the component loads prior to the store getting populated with the user. In console I see the following:

Action type: redux-oidc/LOADING_USER Action type: @@router/LOCATION_CHANGE attempt to call api to load of component (uses authHeader.js) Action type: redux-oidc/USER_FOUND

Any guidance would be appreciated. :)

michaelklopf commented 6 years ago

@acaravia You have to use the triple back tick to get the code highlighting for multiple lines working. Please format your code first.

```
maxmantz commented 6 years ago

@acaravia Explaining how to use redux would be out of scope for discussions regarding this library. There are several ways how you can 'listen to an action' and which way you use is dependent on your project and should not be dictated by me or anyone else not involved in your project.

acaravia commented 6 years ago

Updated the code commenting. Sorry about that. I'll figure out the listening part and appreciate you taking a look. I guess right now my biggest problem is the componentDidMount firing prior to USER_FOUND action. Thanks again.

acaravia commented 6 years ago

I believe I found my issue. I'm still learning React so I didn't have full grasp of the life cycle. When componentDidMount (which has our call our to external api) was firing the oidc.user wasn't available. The component didn't have mapStateToProps adding that and moving the api call to componentWillReceiveProps solved my issue.

maxmantz commented 6 years ago

Glad you solved the issue,