Check for hasVerificationBeenAttempted

[joshudev]

Check for hasVerificationBeenAttempted before redirecting in require sign in wrapper

Fixes #30

[djvs]

@joshudev I'm getting this error:

Uncaught TypeError: Cannot read property 'flushGetRequests' of undefined
    at generateAuthActions (actions.js:102)
    at Object.defineProperty.value (redux-token-auth-config.js:22)
    at __webpack_require__ (bootstrap ce7b60a4d02223d3308f:19)
    at Object.<anonymous> (examinees.js:15)
    at __webpack_require__ (bootstrap ce7b60a4d02223d3308f:19)
    at Object.<anonymous> (application.js:1) [.......]

Coming from here in actions.js:

var generateAuthActions = function (config) {
    var authUrl = config.authUrl, storage = config.storage, userAttributes = config.userAttributes, userRegistrationAttributes = config.userRegistrationAttributes;
    var Storage = Boolean(storage.flushGetRequests) ? storage : AsyncLocalStorage_1.default;

"storage" isn't specified in my config as per the README.md on this repo. I see flushGetRequests refers to ... React Native async local storage? Any tips?

[djvs]

Sorry, subbed it out with storage: window.localStorage. Might be an upstream problem.

[joshudev]

@djvs - I had the same problem with master, and added the following to my config:

storage: {
  flushGetRequests: false
}

e.g:

const config = { 
  authUrl,
  userAttributes: {
    firstName: 'first_name',
    imageUrl: 'image',
  },  
  userRegistrationAttributes: {
    firstName: 'first_name',
  },  
  storage: {
    flushGetRequests: false
  }
}

[joshudev]

@kylecorbelli - can you see any problem with the approach in this pull request?

[kylecorbelli]

can we type nextProps with WrapperProps? 😃

[joshudev]

Ah yes, forgot to do that. Just pushed adc99c6

[kylecorbelli]

@joshudev can we also get a semver patch version bump?

[josephecombs]

is this getting merged any time soon? I'm encountering this exact problem - redirected prior to completing a successful token check.

[josephecombs]

when I get this working I'll add a change to the readme that includes the flushGetRequests thing

[kylecorbelli]

@josephecombs thanks a bunch!

[josephecombs]

@kylecorbelli #42

[rg-najera]

Although this was merged, I can see this has has potentially opened a bug surface area I can't quite pin-point. For me using master, and adding the storage property to my config, just leads to the persist gate not redirecting to the path I set up with generateRequireSignInWrapper. Was there a breaking change Im just not seeing?

Current State when loading the screen that is protected and should redirect.

reduxTokenAuth: {
    currentUser: {
      isSignedIn: false,
      isLoading: false,
      hasVerificationBeenAttempted: true,
      attributes: {}
    }
  },

I can verify Im getting the empty container on 9d83b38#diff-0f483cb752108ae79a3aed8f3e94098eR43

After authentication, I can see the screen fine.

[josephecombs]

I THINK I encountered behavior like this myself and this is actually a bug with devisetokenauth - see lynndylanhurley/devise_token_auth#1053 Try not changing the token on each request.

…

On Mon, Aug 6, 2018 at 1:07 PM, Rodrigo Garcia ***@***.***> wrote: Although this was merged, I can see this has has potentially opened a bug surface area I can't quite pin-point. For me using master, and adding the storage property to my config, just leads to the persist gate not redirecting to the path I set up with generateRequireSignInWrapper. Was there a breaking change Im just not seeing? Current State when loading the screen that is protected and should redirect. reduxTokenAuth: { currentUser: { isSignedIn: false, isLoading: false, hasVerificationBeenAttempted: true, attributes: {} } }, I can verify Im getting the empty container on 9d83b38#diff- 0f483cb752108ae79a3aed8f3e94098eR43 <9d83b38#diff-0f483cb752108ae79a3aed8f3e94098eR43> After authentication, I can see the screen fine. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#35 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADX2SSjrvF1SlV_XiOMeXzgDBypXgnUGks5uOKIIgaJpZM4TOMLR> .

-- *Cell: 585-319-1067* www.josephecombs.com

[rg-najera]

@josephecombs I don’t think this due to the access token not being able to be accessed. Keep in mind that a user hasn’t been authenticated yet.

Maybe I’m just not getting it. What is the premise of the change in regards to verifyToken? If the state of “hasVerificationBeenAttempted” and that is true, and the user state “isSignedIn” is false then we should be redirected to the auth screen. Which it’s not what I’m getting. Maybe it has to do with where I’m adding the gate and token verification methods.

In regards to auth token. My project needs auth to be hardened due to PCI compliance. There’s no way we can allow user token to not be changed in each request without opening up major security loopholes.

My PR, #46 takes care of verifying when an empty auth token is sent back from the server - only happens with batch requests or when the user gets click happy (i was able to reproduce very easily). I’m using my fork with and without this merge effectively. Only adds a new auth token to storage when one is returned. It is a seperate issue but thought it needed to be noted in my PR.

[rg-najera]

Thanks by the way. Don’t mean to be dry about it. Hoping to contribute more as i start understanding more about how these challenges are dealt with.

[TheStu]

I've only been playing with react for a few weeks so maybe there's something wrong with this approach, but w/r/t the issue of not being redirected, I found that I needed to replace componentWillReceiveProps in generateRequireSignInWrapper with:

public componentDidMount(): void {
  const {
    history,
    hasVerificationBeenAttempted,
    isSignedIn,
  } = this.props

  if (hasVerificationBeenAttempted && !isSignedIn) {
    history.replace(redirectPathIfNotSignedIn)
  }
}

after that everything seems to be working as expected.

[csalvato]

This was merged 2 years ago but I don't think it ever made it to a release. NPM is reporting that 0.19.0 is the latest version, so these changes aren't pulled in when doing npm install or yarn install. AM I incorrect here?