Redirect after authentication fails to url with query parameters

[stepple]

I'm getting a 404 if the url contains a route and the wrapper redirects back. It looks like the new url is not setting correctly to the router based in the routers state pathname:"/web/myruns?test=3434" search:""

Notice that search property is empty but the pathname has the query parameter This route works (once the wrapper is removed) pathname:"/web/myruns" search:"?test=3434"

I guess I could fix that by having my own redirectAction that will set the pathname and the search correctly or by avoiding query parameters in my routes. The issue that I'm having with the first approach would be that I need to bring in the thunk middleware but I'm using sagas instead of thunks.

Am I doing something wrong or is there another solution. Thanks Steffen

[mjrussell]

@stepple what versions of the following do you have:

react-router
react-router-redux
history
redux-auth-wrapper

[stepple]

react-router 3.0.0 react-router-redux 4.0.7 history 3.2.1 redux-auth-wrapper 0.9.0

[mjrussell]

Hm I don't think those versions should be a problem.

From your original post it looks like there is an issue with either history or the react-router-redux action. Are you using a custom redirectAction?

Could you show the setup of your store, specifically the middleware setup with routing and the auth wrappers you are using?

[stepple]

I'm just using routerActions.replace I confirmed that the auth wrappers work if there are no query parameters in the url. The Urls with query parameters work without the auth-wrappers.

My wrapper for authenticated

const UserIsAuthenticated = UserAuthWrapper({
  authSelector: state => state.userInfo.accessToken, // how to get the user state
  authenticatingSelector: state => state.userInfo.loggingIn,
  redirectAction: routerActions.replace, // the redux action to dispatch for redirect
  failureRedirectPath: '/web/login',
  wrapperDisplayName: 'UserIsAuthenticated', // a nice name for this auth check
})

My wrapper for not authenticated

const UserIsNotAuthenticated = UserAuthWrapper({
  authSelector: state => state.userInfo, // how to get the user state
  redirectAction: routerActions.replace,
  wrapperDisplayName: 'UserIsNotAuthenticated',
  // Want to redirect the user when they are done loading and authenticated
  predicate: userInfo => userInfo.accessToken === null && userInfo.loggingIn === false,
  failureRedirectPath: (state, ownProps) => ownProps.location.query.redirect || '/web/login',
  allowRedirectBack: false
})

Routing

<Provider store={store}>
        <div>
            <Router history={history}>
              <Route path="/web" component={App}>
                <Route path="login" component={UserIsNotAuthenticated(Login)}/>
                <Route component={Authenticated}>
                  <Route path="requests" component={Requests}/>
                  </Route>
              </Route>
              <Route path='*' component={GenericNotFound} />
            </Router>
            {DevTools && <DevTools />}
        </div>
      </Provider>

Store setup

const sagaMiddleware = createSagaMiddleware()
const routingMiddleware = routerMiddleware(browserHistory)
const enhancer = compose(
  // Middleware you want to use in development:
  applyMiddleware(sagaMiddleware, routingMiddleware, apiClientMiddleware),
  // Required! Enable Redux DevTools with the monitors you chose
  DevTools.instrument(),
  // Optional. Lets you write ?debug_session=<key> in address bar to persist debug sessions
  persistState(getDebugSessionKey())
);

function getDebugSessionKey() {
  // You can write custom logic here!
  // By default we try to read the key from ?debug_session=<key> in the address bar
  const matches = window.location.href.match(/[?&]debug_session=([^&]+)\b/);
  return (matches && matches.length > 0)? matches[1] : null;
}

export default function configureStore(initialState, sagas) {
  const store = createStore(rootReducer, initialState, enhancer);
if (module.hot) {
    module.hot.accept('../reducers', () =>
      store.replaceReducer(require('../reducers')/*.default if you use Babel 6+ */)
    );
  }
  sagas.forEach(s => sagaMiddleware.run(s))

  return store;
}

[stepple]

Hello, any updates on this issue or do you need additional information to repro the issue?

[mjrussell]

Sorry, I've been very busy lately and been slow to respond on this. I don't see anything that jumps out at me. It could be, but it seems highly unlikely to be an issue with redux-auth-wrapper and more than likely a misconfiguration.

Ultimately a runnable example would be the easiest to diagnose.

How do you ensure the history for the Router at <Router history={history}> and const routingMiddleware = routerMiddleware(browserHistory) are the same?

[GrtDev]

I've run into the same issue as well. I think the issue is when you assign a path with a query on the failureRedirectPath property. The whole string is later used as a pathname for the redirect.

see https://github.com/mjrussell/redux-auth-wrapper/blob/master/src/index.js ( line: 27-41 )

  const createRedirect = (location, redirect, redirectPath) => {
    let query
    const canRedirect = typeof allowRedirectBack === 'function' ? allowRedirectBack(location, redirectPath) : allowRedirectBack

    if (canRedirect) {
      query = { [redirectQueryParamName]: `${location.pathname}${location.search}` }
    } else {
      query = {}
    }

    redirect({
      pathname: redirectPath,
      query
    })
  }

What could be expected is that the both the pathname and query params are extracted from the redirectPath and passed onto the redirect accordingly.

[ZwaarContrast]

Same issue. A runnable example can be seen by running the loading example included in the repository and navigating to /foo?debug=true.

You will be redirected to /login?redirect=%2Ffoo%3Fdebug%3Dtrue. After login (As an admin), you are redirected back to /foo?debug=true. At that time you get "Warning: [react-router] Location "/foo?debug=true" did not match any routes".

Might be an issue with react-router though, any ideas @mjrussell ?

[GrtDev]

You can push the new pathname directly to the browserHistory yourself to temporarily bypass this issue:

redirectAction: ( newLoc ) => ( dispatch ) => {
        browserHistory.push( newLoc.pathname );
    },

or use an appropriate action creator if you are using react-router-redux.

[peebles]

I am having either the same problem, or related and have not been able to resolve it.

So, I am not authenticated yet and attempt to go to: http://192.168.99.102/users/H1Hp_SBwx?name=NewAcct

This takes me to /login, with the following url in the browser: http://192.168.99.102/login?redirect=%2Fusers%2FH1Hp_SBwx%3Fname%3DNewAcct

I log in. The route involved looks like this: <Route path="/users(/:accountId)" component={UsersPage} />

When I log in, the browser url is http://192.168.99.102/users/H1Hp_SBwx?name=NewAcct

which looks right, but the component received: props.params = H1Hp_SBwx?name=NewAcct props.location.query = {}

what it should have received, and does if I then reload the page, is props.params = H1Hp_SBwx props.location.query = {name: 'NewAcct'}

Here is my wrapper definition:

const UserIsAuthenticated = UserAuthWrapper({
  authSelector: state => state.user, // how to get the user state
  authenticatingSelector: (state) => { return ( state.user && state.user.isLoading === true ? true : false ); },
  predicate: (user) => { return user.id ? true : false; },
  redirectAction: routerActions.replace, // the redux action to dispatch for redirect
  allowRedirectBack: (location, redirectPath) => {
    console.log( 'allow redirect:', location, redirectPath );
    return true;
  },
  wrapperDisplayName: 'UserIsAuthenticated' // a nice name for this auth check
});

I've tried to grok where in your code where you actually look at the "redirect" query parameter and actually do anything with it, so that I might reverse engineer the proper "redirectAction" function, but no luck so far.

[mjrussell]

@Sector22 @ZwaarContrast nice find. You are definitely right that there is an issue here. Its a little complicated because of the two wrappers working together with the authenticated and the unauthenticated one.

It seems like the authenticated wrapper putting the path and query url encoded into the query param is the right thing to do (otherwise your login page might accidentally mess with it) but the unauthenticated wrapper should unpack into a location object and call redirect with the location. It may be that the failureRedirectPath should instead be a failureRedirectLocation to handle this situation.

I think this would probably be a breaking change so I'm a little hesitant but will definitely make the change if thats the only option. Any better alternatives?

[peebles]

Not sure, but I also want to be able to suppress some routes from adding a redirect onto the login path too, so if change is possible that addresses both, that would be ideal.

Right now I have:

const UserIsNotAuthenticated = UserAuthWrapper({
  authSelector: state => state.user,
  redirectAction: routerActions.replace,
  wrapperDisplayName: 'UserIsNotAuthenticated',
  // Want to redirect the user when they are done loading and authenticated
  predicate: (user) => {
    let notAuthenticated = ( user.id == undefined ) || user.isLoading;
    //console.log( 'predicate:', JSON.stringify( user ), notAuthenticated );
    return notAuthenticated;
  },
  failureRedirectPath: (state, ownProps) => {
    //console.log( 'redirect decision: state:', state );
    //console.log( 'redirecting to:', ownProps.location.query.redirect || brand.homeLink );
    return ownProps.location.query.redirect || brand.homeLink;
  },
  allowRedirectBack: false
});

Right now I image you expect "failureRedirectPath" to return a string. If it returns null, then don't add a redirect path to /login (or at least ignore) ... that will solve my suppression issue. If it returns a string, do what you do now. If it returns an object, maybe, then its a location? I mean, I could parse location.query.redirect with "url" from "/users/XysCggG?name=foo" into { pathname: "/users/XysCggG", query": { name: "foo" } } and pass that back. I dunno ... if something like that works, its backward compatible.

Shoot, even as I write this I am getting confused. How does ownProps.location.query.redirect even get a value? For my suppression desire, I don't even want ?redirect=... to appear on the browser in some cases.

Also, I am not explicitly stating that some of my routes are unauthenicated. Here's my route table:

     <Provider store={store}>
        <Router history={history}>
          <Route path="/" component={EmptyPage}>
            <IndexRedirect to="/login" />
            <Route path="/login" onEnter={this.loginEnter} onLeave={this.loginLeave} component={UserIsNotAuthenticated(LoginPage)} />
            <Route path="/welcome" component={VerifyEmailPage} />
            <Route path="/forgot" onEnter={this.stopIdleTimeout} component={ResetPasswordPage} />
            <Route path="/change" component={ChangePasswordPage} />
            <Route path="/expired" component={UserIsAuthenticated(PasswordExpiredPage)} />
            <Route path="/notfound" component={Page404} />
          </Route>
          <Route component={Authenticated}>
            <Route path="/" onEnter={this.conditionalEnter} component={MasterPage}>
              <Route path="/profile" component={ProfilePage} />
              <Route path="/accounts(/:accountId)" component={AccountsPage} />
              <Route path="/users(/:accountId)" component={UsersPage} />
              <Route path="/roles/:accountId" component={RolesPage} />
            </Route>
          </Route>
          <Redirect from="*" to="/notfound" />
        </Router>
      </Provider>

[mjrussell]

@peebles

I also want to be able to suppress some routes from adding a redirect onto the login path

That is what the allowRedirectBack is for and with the release of v0.10.0 you can use a function for allowRedirectBack, returning false when you want the query param to be omitted.

[peebles]

Works like a charm. Thanks!

[peebles]

This works for me... in createRedirect():

    if ( ! canRedirect ) {
      var loc = require( 'url' ).parse( redirectPath );
      redirect({ pathname: loc.pathname, search: loc.search });
    }
    else 
      redirect({
    pathname: redirectPath,
    query: query
      });

When redirecting off of /login, canRedirect is false, and the redirectPath has the potential query string.

[mjrussell]

Just merged this in, thank you all for the comments. I misunderstood the original issue when it was first reported but I think it should be fixed now.

I released it under v1.0.0. There should be no breaking changes however. I figured this project is long overdue for v1 and seemed like a good time and will give devs more flexibility with npm version upgrades

[peebles]

Ok, looks like it works. Thanks very much for your help and dedication. This project certainly made mine easier. I am thinking that maybe a future project I might tackle is to add this auth stuff to https://github.com/davezuko/react-redux-starter-kit so that I have a nice template for myself for authenticated apps. I have my own user-management-system-as-a-service, kind of modeled after Stormpath, that tries to be HIPPA compliant and sit behind the firewall. That is what I just hooked up to with this project.

[mjrussell]

@peebles that would be great. One thing many people new to react/redux struggle with is that they clone a starter kit and try to add redux-auth-wrapper without fully understanding how higher order components or the starter kit works. So any examples would be really appreciated and I can link to them in the README

[be-next-hotdog]

Yoop, it seems as if we still need to improve this feature. Nevertheless, the redirect kept merely the first query string in case of multi query strings. For instant, ?foo1=bar1&foo2=bar2 will be transformed into 2 query strings (redirect and foo2) in nextRoute, no ?

[mjrussell]

@be-next-hotdog

Multiple query strings seems to work fine for me. On the loading example on master, If I go to http://localhost:8080/foo?bar=baz&bar2=baz2 I get redirected to http://localhost:8080/login?redirect=%2Ffoo%3Fbar%3Dbaz%26bar2%3Dbaz2. Then I enter my name, hit submit, and end up at http://localhost:8080/foo?bar=baz&bar2=baz2. The query params for both appear to be populated correctly:

[be-next-hotdog]

By the way, how the redux-auth-wrapper convert automatically characters into url coding in the redirect query string ?

In your example : http://localhost:8080/login?redirect=%2Ffoo%3Fbar%3Dbaz%26bar2%3Dbaz2 ?

[mjrussell]

@be-next-hotdog

I believe history is the one doing the URI encoding/decoding process. That assumes you are either using context.router for the replace which is using history under the covers, or dispatching a redux action to a middleware that defers to history such as react-router-redux

Did it work for you then? Can your other issue be closed?

[be-next-hotdog]

I do use react-router-redux but I wonder what would be the cleanest way to work out encoding/decoding with redux-auth-wrapper and such middleware ? Or I add a coding check feature in redux-auth-wrapper redirect although it seems little cumbersome.

[mjrussell]

I dont really understand what you are tying to accomplish. You want to use an encoding other than URI encoding? That seems a bit dangerous where you might accidentally keep characters in the URL bar which are not allowed as part of the spec. You could just use a different redirectAction that changes the query params in some way, it will receive the result of

redirect({
      pathname: redirectLoc.pathname,
      query
    })

and can change it if needed

[be-next-hotdog]

Just wanna make it work usually, now under react-router-redux location's never converted the redirect into URI encoding in routing state so that it broke out the redirect string while 'Location_change'. Such as : search : ?redirect=/foo?bar=baz&bar2=baz2 => redirect query string : /foo?bar=baz , which lost part of redirect after '&'.

Better with a state capture :

[mjrussell]

Im sorry I don't understand, above I posted that it works as I would expect it to in the loading example. I understand you are saying you are losing the bar2 but I cant reproduce that.

Can you use the loading example (on master) and include the URL at each step as I did? Otherwise please provide a repo with steps to reproduce.