kylecorbelli / redux-token-auth

Redux actions and reducers to integrate easily with Devise Token Auth
MIT License
154 stars 80 forks source link

Redirect to route after successful sign in #21

Open epaillous opened 6 years ago

epaillous commented 6 years ago

I have a route A protected by requireSignIn. It works perfectly (it redirects to sign in route if I am not signed in), but I would like to redirect to the route A if sign in is successful.

How can I do that ?

Thank you for this awesome package !

kylecorbelli commented 6 years ago

Hello @epaillous! Thanks for checking out this repo! When you dispatch the action to sign in, can you redirect once the promise resolves? Assuming you're using react-router-dom, something along these lines:

class SignInScreen extends Component {
  submitForm = async (event) => {
    event.preventDefault()
    const { history, signInUser } = this.props
    const { email, password } = this.state
    try {
      await signInUser({ email, password })
      history.push('/welcome')
    } catch (error) { console.error(error) }
  }

  render () {
    return <form onSubmit={this.submitForm}>...</form>
  }
}

Is this kind of what you're thinking?

kylecorbelli commented 6 years ago

Or do you mean you want some pages to be inaccessible once the user is signed in?

wtanna commented 6 years ago

I've been struggling with a similar issue. My code currently is:

// index.js
import React from 'react';
import ReactDOM from 'react-dom';
import { Provider } from 'react-redux'
import store from 'store'
import { verifyCredentials } from 'actions/redux-token-auth-config'
import Router from 'router'
import './index.css';

verifyCredentials(store)

ReactDOM.render(
  <Provider store={store}>
    <Router />
  </Provider>,
  document.getElementById('root'),
)
// router.js
import * as React from 'react'
import { ConnectedRouter } from 'react-router-redux'
import { Route } from 'react-router-dom'
import { history } from 'store'
import HomePage from 'containers/home'
import UserIndex from 'containers/users/index'
import SignInPage from 'components/auth/sign-in'
import { generateRequireSignInWrapper } from 'redux-token-auth'

const requireSignIn = generateRequireSignInWrapper({
  redirectPathIfNotSignedIn: '/signin'
})

export default () => (
  <ConnectedRouter history={history}>
    <div>
      <Route exact={true} path="/" component={HomePage} />
      <Route path="/users" component={requireSignIn(UserIndex)} />
      <Route path="/signin" component={SignInPage} />
    </div>
  </ConnectedRouter>
)

What seems to be happening is when a user is not logged in it goes to the /signin page, which is great! But when I hard refresh the web page in /users it seems like the verifyCredentials is taking a tad too long, so when requireSignIn gets called it sees the isSignedIn state as false because the verifyCredentials hasn't retrieved the info from the validate_token url yet.

This is what my Redux actions look like:

redux-token-auth/VERIFY_TOKEN_REQUEST_SENT
@@router/LOCATION_CHANGE // it is trying to go to `/users`
@@router/LOCATION_CHANGE // The `requireSignIn` saw that the `isSignedIn` is false and is redirecting to `/signin`
redux-token-auth/VERIFY_TOKEN_REQUEST_SUCCEEDED // `isSignedIn` set to true

I was hoping that using the requireSignIn would mean that I wouldn't have to create a custom protected Route HOC, but that might be an easier solution than figuring out how to make the verifyCredentials a synchronous action that the router will wait for before proceeding.

I'm curious if I am missing something fundamental about this package, or if going the ProtectedRoute is more of what I need to do when a user refreshes the page?

lksalbq commented 6 years ago

I am having the same issue that @wtanna described...anyone else? I'm stucked on this and nothing yeat...

wtanna commented 6 years ago

After sleeping on this, I think one solution would be an update to the function verifyCredentials.

Instead of grabbing the access-token first and doing an API call to validate. It should first grab the expiry and check with the current date.

If it is valid then it would dispatch an action that states that and change the stores isSignedIn value to true.

If it has expired, or there is an error, it would be an invalid token and the isSignedIn redux store value would stay to false, making the user have to re-login. Saves from having to do an API call, and if something is borked in the local storage it might be best to just sign in again and get all the info anyway.

Once that check is successful it can dispatch an action saying so, then continue to do work to getUserAttributes in a different action / reducer to set the user data. At least this way the verification won't be held up by getting data from the server.

I'm thinking something like: (sorry for it not being typescript and for it being pseudocode)

function verifyCredentials () {
  return async function (dispatch) {
    try {
      await dispatch(verifyLocalToken())
    } catch (e) {
      dispatch(setHasVerificationBeenAttempted(true))
    }
  }
}

function verifyLocalToken () {
  return async function (dispatch) {
    const expiry = await Storage.getItem('expiry')

    if (expiry <= new Date().getTime()) {
      dispatch(validToken())
    } else {
      dispatch(invalidToken())
    }
  }
}

function validToken () {
  return async function (dispatch) {
    dispatch({ type: 'VALID_TOKEN', isSignedIn: true })
    dispatch(getUserAttributes())
  }
}

function invalidToken () {
  return { type: 'INVALID_TOKEN', isSignedIn: false }
}

It seems like even with this update though there is going to always be a race condition. There is a chance that the route will change too fast before even doing local validation. Although I do think checking local storage will help a lot, I think the most full proof plan is to make the ProtectedRoute that can at least look at the isSignedIn value or read from Local Storage itself that thankfully, redux-token-auth, provides all that leg work when users sign in!

perchard commented 6 years ago

I also saw this race condition between verifyCredentials and the generateRequireSignInWrapper redirect on hard refresh.

I ended up using redux-persist to hydrate the store with the authenticated state. I think a similar approach to that package's persistGate could be used here to delay rendering until credential verification is complete.

jdarnok commented 5 years ago

The hard refresh problem still occurs. Is using HOC or custom route component an answer here? in current state, this functionality is kinda useless

anduscheung commented 5 years ago

I am encountering the same problem as @wtanna & @epaillous and manage to come up with a solution. Instead of using generateRequireSignInWrapper, I use the props in reduxTokenAuth.

routeSwitch() {
  let {reduxTokenAuth} = this.props;

  if (!reduxTokenAuth.currentUser.hasVerificationBeenAttempted) {
    return (
        <div></div>
    );
  } else {
    if (reduxTokenAuth.currentUser.isSignedIn) {
      return (
        <Switch>
            <Route strict path="/protected_1" component={Protected1}/>
            <Route strict path="/protected_2" component={Protected2}/>
            <Route strict path="/protected_3" component={Protected3}/>
            <Redirect to="/protected_1"/>
        </Switch>
      );
    } else {
      return (
        <Switch>
            <Route strict path="/login" component={Login}/>
            <Redirect to="/login"/>
        </Switch>
      )
    }
  }
}

render() {
  return (
    <Router id="Main" history={this.props.history}>
      {this.routeSwitch()}
    </Router>
  );
}
StefanDorresteijn commented 4 years ago

I also saw this race condition between verifyCredentials and the generateRequireSignInWrapper redirect on hard refresh.

I ended up using redux-persist to hydrate the store with the authenticated state. I think a similar approach to that package's persistGate could be used here to delay rendering until credential verification is complete.

This is absolutely the easiest solution, and works consistently.