Login cycle triggered with refresh?

[christiansyoung]

Users tend to refresh their browser, which clears the redux store. As I understand it, the user data is still in the localStorage. Couldn't the middleware look in localStorage, challenge the OpenID Provider with the found data, and on success populate the store again without the user having to actively do a login cycle?

I noticed that each login cycle stores an entry in localStorage. Is this by design, and why is that? Couldn't the localStore just hold the last successful login entry?

Thank you for the library, excited about getting it up and running :)

[maxmantz]

Thank you for your kind words. It is equally exciting to help you along in getting things set up.

As I understand it, oidc-client stores user data from the OpenID Provider in session storage by default - which gets cleared after a refresh. You can look at the sample app and see that the user data is stored there after you've logged in. You can change the storage to use local storage instead by doing this:

import { WebStorageStateStore } from 'oidc-client';

const settings = {
  // your user manager settings
  store: new WebStorageStateStore({ store: localStorage }) 
};

Once again, I haven't yet tried this myself but it should work just fine. fingers crossed

The middleware checks the stored user automatically on startup. Only if the user is expired, or no user has been found (using session storage for example) the login cycle gets triggered. If a valid, existing user is found, no login should happen.

EDIT: There is a key oidc.expired in local storage set by redux-oidc when the user hasn't been found or is invalid. This is done to prevent the middleware from triggering the user check after redirection from the OpenID Provider took place. If that doesn't happen, an infinite loop of redirections would be triggered when logging in.

[christiansyoung]

I digged around in oidc-clienta bit, and it looks like localStorage is used for storing data during the authorization phase, and user data is then held in sessionStorage due to PII. sessionStorage persists through page reloads, but is cleared when quitting the browser window/tab.

The reason for my question is that on https://redux-oidc-example.herokuapp.com/, after logging in and hitting refresh, you must login again. I cloned the redux-oidc-example lib and ran it locally which worked. I can refresh the page and I don't have to login again. This does not work in my app.

Any ideas? (I have not configured silentRenew yet, I don't know if that is relevant. I just came across it here: https://github.com/IdentityModel/oidc-client-js/issues/49)

[maxmantz]

That's strange. Whenever & wherever I run my example app (either on heroku or locally) and hit refresh, I always have to log in again, because the session storage gets cleared.

Silent renew is not a requirement for this library to work, but it is recommended to avoid users having to log in after a certain period of time. It is a decision you have to make. I personally don't know how long Google's tokens are valid, but if a user has to log in every hour (the default token lifetime for IdentityServer) I prefer introducing silent renew to get a new token in the background.

[christiansyoung]

I will definitely configure silent renew in the future. My plan is to use our IdentityServer instead of Google, but for now I am just doing a PoC.

In my Chrome I can see that session storage entry persists on all the applications when I refresh. I use redux devtools, so I can see that upon refreshing, the state is cleared, so when I do a similar check to this: https://github.com/maxmantz/redux-oidc-example/blob/master/src/components/homePage/index.js#L16, the state.oidc.user is null, and thus the login page is viewed.

What I initially suggested, and what I am not sure if you have implemented is that the middleware syncs the sessionStore with the state if the state is lost due to e.g. a refresh. I believe this is how oidc-clientbehaves in the same scenario, but redux-oidc doesn't due to it relying on the redux state which can be lost.

[maxmantz]

Exactly. There is no way to persist redux state, other than introducing other libraries such as redux-persist or redux-storage. I thought about this but felt that besides adding a new dependency to the library, it also adds a lot of complexity. As such no state is persisted by default. However when you change your storage from session storage to local storage, the user data doesn't get cleared on refresh and the login screen should not appear.

There is the aforementioned key oidc.expired in local storage when an expired user has been found. This is to make sure that when redirection from the OpenID provider happens, the validation process from the middleware doesn't trigger another redirect. But that is all that is persisted so far.

[christiansyoung]

What I am suggesting though is not to try to persist the redux store, but to use the middleware to check if there already is an entry in the sessionStorage when state.oidc.user is null. If so, populate state.oidc.user based on the entry.

I think sessionStorage is a better option than localStorage, as the claims can contain somewhat personal data which are best off removed when closing a browser window.

The redux store is only cleared when a user does a refresh manually, and therefore syncing it up again when the page reloads and the middleware triggers, shouldn't have any complications towards the expiration.

Let me know if I make myself hard to understand here, I am merely trying to help out with constructive suggestions :)

[maxmantz]

I think I understand your points, but let me explain why things are the way they are right now:

As for the process of getting the user data from storage, oidc-client has the getUser() method. This is also what is called by the middleware when no existing user is present (on first load or after redirection). The problem with this however is that getUser() returns a promise. This can be resolved instantly when session storage or local storage is used, but might have some lag in its execution when other storage mechanisms are used. oidc-client has the ability to use other storage mechansims besides local and session storage. I don't want to limit the use of this library to those two storage types only. When the userManager is configured to use another storage type, but this library only checks session storage, then this would not work.

I hope this helps :)

[christiansyoung]

Thank you for the explanation. I didn't think of the interchangeable storage :)

[maxmantz]

No worries. I'm happy to explain the thought process behind this.

[christiansyoung]

Just a heads up. I solved this when using sessionStorage or localStorage by rehydrating the redux store in my login handler component if an entry is present. If you want to, I can provide some code snippets for a wiki page to help out others that face the same problem.

[maxmantz]

That would be great!

[christiansyoung]

Cool, feel free to suggest improvements. I just finished it :)

The module to check storage entries

I also wrote some tests for this, but these examples are more of a guideline on how it can be solved rather than a plugin/lib. For now, it checs for the oidc.user prefix on the last entry. If the app uses the same web storage for other entries not related to oidc (mine does not), it will probably fail since it just grabs the newest entry and checks it.

/*
 * Oidc-client stores the user object in sessionStore by default.
 * on e.g. a refresh when the redux state is cleared, we use this function
 * to fetch the stored data as a JavaScript object so that the state can
 * be rehydrated.
 * Returns a user object is found and the client_id matches, else false.
 */
const browserHasOidcEntry = (webStorage, openidProviderConfig) => {
  if(!webStorage)
    throw new Error('Storage not available');

  if(webStorage.length > 0) {
    const lastKnownStorageKey = webStorage.key(webStorage.length - 1);
    const storageKeyParts = lastKnownStorageKey.split(':');
    if(storageKeyParts[0] === 'oidc.user' && storageKeyParts[storageKeyParts.length - 1] === openidProviderConfig.client_id) {
      return JSON.parse(webStorage.getItem(lastKnownStorageKey));
    }
  }
  return false;
};

export default browserHasOidcEntry;

A login wrapper placed as a child to OidcProvider and as a parent to the App in the route config.

This is based on https://github.com/maxmantz/redux-oidc-example/blob/master/src/components/homePage/index.js

In addition to exporting the userManager as done here: https://github.com/maxmantz/redux-oidc-example/blob/master/src/utils/userManager.js I export the config as well to do a check on the client id.

When the user is in state either from rehydration or redirect loop, this.props.children which contains the app it self will render.

The first argument to browserHasOidcEntry could either be window.sessionStorage or window.localStorage`

import React, { Component, PropTypes } from 'react';
import { connect } from 'react-redux';

import { userFound as userFoundAction } from 'redux-oidc';
import { userManager, openidProviderConfig } from '../lib/oidcSetup';
import browserHasOidcEntry from '../lib/browserHasOidcEntry';

class HandleLogin extends Component {

  static propTypes = {
    user: PropTypes.object,
    dispatch: PropTypes.func.isRequired,
    children: PropTypes.oneOfType([
      PropTypes.object,
      PropTypes.array
    ])
  };

  componentDidMount () {
    const { user, dispatch }  = this.props;

    // Rehydrate user state from web storage if present
    if(!user) {
      const userFromWebStorage = browserHasOidcEntry(window.sessionStorage, openidProviderConfig);
      if (userFromSessionStorage) {
        dispatch(userFoundAction(userFromWebStorage));
      }
    }
  }

  triggerLogin = (event) => {
    event.preventDefault();
    userManager.signinRedirect({
      data: {
        redirectUrl: window.location.href
      }
    });
  };

  render() {
    const { user } = this.props;

    if (!user || user.expired) {
      return (
        <button onClick={this.triggerLogin}>Login</button>
      );
    } else {
      return (
        <div>{this.props.children}</div>
      );
    }
  }
}

const mapDispatchToProps = (dispatch) => ({
  dispatch
});

const mapStateToProps = (state) => ({
  user: state.oidc.user
});

export default connect(
  mapStateToProps,
  mapDispatchToProps
)(HandleLogin);

I could consider making the web storage check more thourough, but here is my initial approach which works fine for my project anyways.

Hope this can be of some help to anyone :)

[maxmantz]

This is a lot to take in. Thank you very much for your effort!

[christiansyoung]

Hehe, I realize that it looks like a monster, but I wanted to give you the whole context. In reality, I don't believe it's too much to grasp :)

My app is basically structured like this (simplified):

<Provider store={store} key="provider">
    <OidcProvider store={store} userManager={userManager}>
        <Route component={HandleLogin}>
            <Route path={'/'} component={App}>
                // Rest of the application
            </Route>
        </Route>
    </OidcProvider>
</Provider>

The React component HandleLogin, which I pasted in the above post, acts as a "door keeper" that checks state.oidc.user. If present, it will render App and it's children. If not, it will try to populate it if an entry is already in sessionStore, or display a "login page" that does userManager.signinRedirect() if the user wants to.

[maxmantz]

Sorry for the late reply, I only had time to look at this in more detail now. I have found the issue in my code that made your workaround necessary. The issue was that the middleware didn't send the found user to the dispatch after finding it. This issue has been fixed in the latest 2.1.0-beta.3 version. You will find that a page refresh now works without having to log in again.

Thank you for your thorough investigation into this issue.

[christiansyoung]

I'll be damned! I guess I was onto something in the start here after all :p I am glad you figured it out, and that this could be fixed in the right place. My workaround looks very tedious in comparison :)

I will test the beta.3 version later on today and hopefully get rid of some redundant code!

[keirk]

Love it when you go to github looking to see why something doesn't work and you a) find an issue exactly describing your issue & b) the solution is to update the package 💯 👍

[maxmantz]

You were right on time as well, just fixed it earlier today :+1:

[christiansyoung]

I am having trouble making this work :/

What I don't understand is how the middleware gets called after a refresh. In your example app: https://github.com/maxmantz/redux-oidc-example/blob/master/src/components/homePage/index.js how is the middleware called when state.oidc.user is null? I can't see any other actions being dispatched before this point so that the user is loaded into state by the middleware.

Is it the redux saga loading in the store file that triggers the middleware?

[maxmantz]

The redux store gets reassembled after each refresh. It starts by creating the store here.

The middleware itself then uses the userManager to find an existing user in storage here.

If a valid user is found in storage, the middleware dispatches the found user to the redux store here. This is also what was missing before the fix to beta.3.

The middleware then caches the found user object here to prevent this check from happening after every dispatch.

What triggers the middleware is simply a redux action - any action to be precise - being dispatched.

I hope this helps your understanding.

[christiansyoung]

What triggers the middleware is simply a redux action - any action to be precise - being dispatched.

That's what I thought. Is it then this that dispatches an action so the middleware runs after a refresh? My problem is that my login page that checks state.oidc.user does not render my App until user is present in state, and no actions are dispatched before that happens.

My app setup is pretty much identical hierarchically with your example app, but I don't use saga. My App component dispatches an action to fetch customers, and I did some attempts of trying to move the fetch to componentDidMount in my login handler, but that made the fetch go through before the getUser promise was resolved.

If I do it manually in HandleLogin, like I did before, but a bit cleaner using the userManager like in the middleware, it works:

componentDidMount () {
    const { user, dispatch }  = this.props;
    if(!user) {
      userManager.getUser()
        .then((userFromStore) => {
          dispatch(userFoundAction(userFromStore));
        });
    }
  }

Is it correct that for this to work, you have to either dispatch an action when the application loads, or do it manually like illustrated above?

[maxmantz]

You could try to do a manual dispatch after the store is initialized like this:

const store = createStoreWithMiddleware(oidcMiddleware, ...);

// dispatch an action to kick off the middleware
store.dispatch({ type: 'INIT_OIDC' });

This should trigger the user validation.

My sample app works similar to yours I presume. When user is null of invalid, the login page gets rendered. There the login button dispatches an action, so the validation gets triggered.

One difference to your program I could think of is that everything is contained within my <App /> component. The app component then either renders the routes or the login screen when no user is present. When your app is configured to show nothing when no user is present, then nothing will get dispatched. If you're using redux-devtools you can check your app to see whether or not anything is dispatched when the app loads. Usually react-router-redux initializes itself with action dispatches on startup.

[christiansyoung]

Got it. I will look at some alternatives. I don't use react-router-redux either :p Thank you for your time :)

[maxmantz]

Were you able to solve this issue?

[jbellmore31g]

Hello!

First of all, wanted to say thanks for creating this package it is very helpful!

I wanted to comment on this issue rather than open a new one because I am having the same, or at least a very related issue. I just updated to the beta3 package and in my app the user is indeed being loaded from local storage on refresh, however I am having an issue with the timing of the data being loaded into the redux store.

Basically, the user data is not loaded into the store until the redux-oidc/USER_FOUND action is dispatched which is unfortunately happening after my components load and I try to execute an async request to load data from the server, which requires the bearer token auth header. Because the user is missing my code is assuming the user is not logged in and redirects to the login page. This is custom code that I wrote in my async action, not the provider performing the redirect just wanted to be clear!

Am I missing something obvious here? In all honestly I am somewhat new to the react/redux world so I might just be thinking about this incorrectly but what I think I need is a way to trigger that reloading of the user into the store in a synchronous fashion if it exists in local storage.

Look forward to your thoughts on this.

Thanks again!

[maxmantz]

The initial load from storage happens on the very first action being dispatched through the middleware. You could try to dispatch an init action on the componentWillMount lifecycle function of your root app like this:

class App extends React.Component {
    componentWillMount() {
        this.props.dispatch({ type: 'INIT_APP' });
    }

    render() {
        // your app here
   }
}

// map the store's dispatch to component's props
function mapDispatchToProps(dispatch) {
    return { dispatch };
}

export default connect(null, mapDispatchToProps)(App);

This will fire the init action on app startup, which in turn causes the middleware to load the user from storage and set it in the redux state. This is my quick idea, hope it helps!

[jbellmore31g]

Thanks for the quick response!

So I actually did see that mentioned earlier so I did try that, here is the order in which the actions are being processed:

action @ 16:33:16.534 INIT_APP action @ 16:34:12.631 LOAD_CURRENT_USER_PROFILE_REQUEST action @ 16:34:12.648 redux-oidc/USER_FOUND action @ 16:34:12.676 @@router/LOCATION_CHANGE

The INIT_APP action is being created as you showed above in the root component of the app. The LOAD_CURRENT_USER_PROFILE_REQUEST action is created in the componentDidMount method of the component being loaded by the react router of the matched route (/profile).

Essentially the component being loaded by the react router is being mounted prior to the middleware creating the USER_FOUND action. The oidc user object in the store is null until the USER_FOUND action is handled by the appropriate reducer/middleware it seems.

Hopefully that explanation makes sense!

I am guessing it has to do with the asynchronous nature of the userManager.getUser?

Thanks again!

[maxmantz]

Even though the getUser function of the user manager is a promise, when used with local storage or session storage it should be almost instantaneous. Without looking at the code it is hard for me to pinpoint the reason as to why the API request is happening before the USER_FOUND action has fired. I'll think about a solution to maybe passing the user object manually to the middleware on startup to avoid triggering it via an action.

[maxmantz]

To follow this up, have you looked at the example app I've provided? It has a setup which works with the asynchronous nature of the getUser function and doesn't run into the issue that an API request is made before the user has been loaded.

I'm currently working on a solution that will enable you to pass an existing user from storage into the middleware construction function. But for this I need a PR to be merged into oidc-client-js, which may take some time, or never happen at all.

Since I don't have access to your code all I can do is guess as to what causes your issue. Your best bet for now would be to look at the example app and pinpoint the cause of your issue by comparing and contrasting. If you have any questions about the code of the example app, don't hesitate to ask.

[jbellmore31g]

Hi again!

I have been poking around with your sample app and trying to compare it to the app I am using and the main difference in the structure I am seeing is that I am currently using the thunk middleware and the sample app is using sagas. I suspect what's happening is that in the thunk middleware, the method that is returned from the action creation is invoked immediately, and that is where I start the request execution, which i before the appropriate actions are handled by the middleware.

I added some logging to the sample app and this is the order of the events I am seeing being processed:

MainPage.componentWillMount - Start index.js:12MainPage.componentWillMount - End index.js:16MainPage.componentDidMount - Start index.js:17MainPage.componentDidMount - End index.js:152 action @ 09:09:44.412 redux-oidc/USER_FOUND index.js:152 action @ 09:09:44.448 @@router/LOCATION_CHANGE

So i you dispatch an action that returns a function that is executed by the thunk middleware in either the willMount or didMount methods it seems to be executed immediately even when doing a dispatch and not just directly calling the method. This is what I am currently doing in my app in the didMount method:

this.props.dispatch(actionCreators.fetchProfileForCurrentUser());

The fechProfileForCurrentUser method is built using the pattern for async action creators outlined in the redux docs here:

http://redux.js.org/docs/advanced/AsyncActions.html

I have forked your sample app and I am working on modifying it to use the thunk middleware instead to illustrate how I have my app setup, I'll post again when I am finished with that.

For now I see two options, unless your PR gets accepted :)

First option I see is to switch to use the saga middleware, rather than thunk. It is a bit bigger of a change so not sure I will do that at the moment.

Second option I see is to put a brief timeout (few milliseconds) into the function being returned by action creator to force the method to wait until the the other actions have been processed before continuing. Sucks a bit to have to add that to each async call but won't be the end of the world.

Thanks again for your work on this repo and your help on this issue!

[maxmantz]

Have you been able to resolve this issue? If not, is it possible for you to give me access to a small repo reproducing the issue? I would like to look at the code and see if I can find the problem, in case my PR gets rejected.

[jbellmore31g]

I have finally been able to reproduce the issue after doing a lot of stare and compare between my repo and the sample repo.

The problem appears to be how I have the login and secured page split between 2 different routes. In your sample you have the homepage dynamically switch between the LoginPage and the MainPage component based on whether or not the user object is null. In my repo I have a login route and the profile route as a totally separate route.

For now in that repo you have to manually go to the /login route as I didn't do any redirect if you aren't logged in.

I have committed the changes to reproduce the issue here:

https://github.com/jbellmore31g/redux-oidc-example

The reasoning behind wanting to do this it this way was to either have a higher order component ensure the user was logged in prior to loading a component or to have the app redirect if any of the api calls returned a 401 response. Not positive which direction I was looking to go yet but wanted to do one of those two.

Hopefully that makes sense and helps you understand the issue!

Thanks again!

[maxmantz]

Thanks for the fork. The issue is clear to me now, as I guess it is for you. The <MainPage /> depends on the user to be present in its lifecycle functions. Therefore the homePage is required to make the check if the user is actually there or not in order to prevent this issue from occurring.

I can understand your reasoning behind having the login page at a separate route, as well as a possible error page. I have made a few changes solving your issue (the magic happens in the change to the componentWillMount function inside mainPage).

There is nothing preventing you to use the login page as a separate route, however let me give you the following cons this approach has:

• the user is always allowed to enter the login page via /login even when he is already logged in.
• all the other routes are also accessible even when the user isn't present (which caused your issue and can cause multiple other ones, maybe even revealing secure data to unwanted eyes!!)
• you have to check inside every component that needs the user to be present like I did in the main page, so you're definitely better off checking it inside the homePage once instead of everywhere else.
• you have to redirect to /login when you click the logout button (and remove the user), or things will break.

I've also made a little error page for you at is own route (/error) which has its own problems all related to the cons described above, but that's another topic - I guess you'll see the problems when you play around with it.

If you add me as a contributor to your fork I will push the changes. Hope this helps!

[jbellmore31g]

Cool, thanks a lot! I have added you as a contributor to that fork, look forward to seeing the changes.

Thanks again for taking the time to help me out, you rock :)

[maxmantz]

Changes pushed. Always happy to help :-)

[theirishpenguin]

Hi @maxmantz, I just wanted to let you know that the solution you outlined at https://github.com/maxmantz/redux-oidc/issues/6#issuecomment-239399073 appears to work for me, thanks. And thanks @lizter for really helping illuminate this area as I was going through the same throught process. I'm just going to post the code I used below as it might be more familiar to someone using the createStore() call with applyMiddleware() - as opposed to createStoreWithMiddleware() and includes the dispatch of the INIT_OIDC action...

const loggerMiddleware = createLogger()

// create the middleware with the userManager, null for shouldValidate, triggerAuthFlow false, and the callback route (note: not using react-router-redux in this project)
const oidcMiddleware = createOidcMiddleware(userManager, () => true, false, '/callback')

export function configureStore() {
  const store = createStore(
    rootReducer,
    compose(
      applyMiddleware(
          oidcMiddleware,
          thunkMiddleware, // lets us dispatch() functions
          loggerMiddleware // neat middleware that logs actions
        ),
      window.devToolsExtension ? window.devToolsExtension() : f => f
    )
  )

  // dispatch an action to kick off the middleware
  store.dispatch({ type: 'INIT_OIDC' })
  return store
}