Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sessionLength not being respected, ending early #379

Open
jneterer opened this issue Dec 1, 2022 · 10 comments
Open

sessionLength not being respected, ending early #379

jneterer opened this issue Dec 1, 2022 · 10 comments

Comments

@jneterer
Copy link
Contributor

jneterer commented Dec 1, 2022

When setting rememberMe: 'local' and sessionLength: 8760 (1 year), the session consistently ends early. The user's session usually lasts just a few days, max. This isn't as reproducible on web as it is in an Ionic+Capacitor mobile application where it happens every time.

It could be an issue with the Capacitor layer that allows for "local storage" access in a Web Native app, but I'm not sure. I wanted to call it out here as well in case there might be any insights as to why this is occurring.

@jneterer
Copy link
Contributor Author

jneterer commented Dec 1, 2022

May have answered my own question here: https://capacitorjs.com/docs/guides/storage.

"...your app needs to expect that the data will be lost eventually. This is because the OS will reclaim local storage from Web Views if a device is running low on space."

Although my device isn't low on storage, there could be other reasons the OS would evict the space.

@jneterer
Copy link
Contributor Author

jneterer commented Dec 6, 2022

@j-berman Looking for your thoughts here. For Capacitor applications that use Userbase, local storage is not a long-term data storage solution, as the data can get cleared and session lost. I used patch-package to prove replace local storage with this capacitor plugin (UserDefaults on iOS, SharedPreferences on Android, local storage on web). It works on all 3 platforms and successfully maintains user sessions.

The question is this, should I:

  1. Just use patch-package in my app
  2. Consider forking userbase to a new repo (i.e. @capacitor-community/userbase or something) where the only difference is capacitor support? The new project could like here with the Capacitor Community.

@j-berman
Copy link
Collaborator

j-berman commented Dec 6, 2022

Perhaps there's a simpler way to create a new Capacitor plugin that has both userbase-js and path-package as dependencies?

I did something similar for the Cordova plugin, which needed to use a custom scrypt plugin for the password hashing algorithm. The Cordova plugin overrides the password hashing algorithm used by userbase-js internally by overriding a passwordHashAlgo param in signUp, signIn, and updateUser:

var signUp = window.userbase.signUp
window.userbase.signUp = function (params) {
if (typeof params === 'object') params.passwordHashAlgo = scrypt
return signUp(params)
}
var signIn = window.userbase.signIn
window.userbase.signIn = function (params) {
if (typeof params === 'object') params.passwordHashAlgo = scrypt
return signIn(params)
}
var updateUser = window.userbase.updateUser
window.userbase.updateUser = function (params) {
if (typeof params === 'object') params.passwordHashAlgo = scrypt
return updateUser(params)
}

Maybe there's an even simpler way to do it in this case.

It might end up being more effort than it's worth if you can just use path-package in your app though. Perhaps if others express interest in a Capacitor plugin then it's worth exploring the plugin route further. Your call :)

@jneterer
Copy link
Contributor Author

jneterer commented Dec 7, 2022

@j-berman wow, for some reason I wasn't aware of the Cordova plugin, or I forgot about it! I may be able to use the Cordova plugin in place of the regular userbase-js package. I'll test it out later today.

I will say that updating to use the new storage method did not work for me. I checked the app this morning and was signed out. So my session lasted maybe 6 days. Maybe the Cordova plugin will be better!

@jneterer
Copy link
Contributor Author

jneterer commented Dec 7, 2022

@j-berman would be curious if you're using the Cordova plugin and if you've had session issues? I suppose it could have to do with my implementation, maybe a race condition with init or something, but I really don't think so.

@jneterer
Copy link
Contributor Author

jneterer commented Dec 8, 2022

I've just come to realize this is only an issue on iOS. I just picked up an old Android phone for testing, opened the app, and my session from months ago was still active and working as expected.

@jneterer
Copy link
Contributor Author

jneterer commented Dec 9, 2022

@j-berman Can you explain why expirationDate behaves this way?

When I sign in, my userbaseCurrentSession has a value of:

{
  "username": "jneterer",
  "signedIn": true,
  "sessionId": "...",
  "creationDate": "2022-12-09T05:21:46.989Z",
  "expirationDate": "2023-12-09T05:21:46.989Z"
}

However when I close/refresh the browser tab or window/app and open it again, my expirationDate is now only only 24 hours from now (instead of 365 days as set on sign in, see above). Every subsequent close+open adds 24 hours to the expiration date.

{
  "username": "jneterer",
  "signedIn": true,
  "sessionId": "...",
  "creationDate": "2022-12-09T05:21:46.989Z",
  "expirationDate": "2022-12-10T05:22:24.319Z"
}

Wondering if this has anything to do with it. Maybe not.

@j-berman
Copy link
Collaborator

j-berman commented Dec 9, 2022

Sounds like this might be the issue you're running into!

When a user resumes a session via init, Userbase will set the session to expire 24 hours from that point in time by default. You would have to set sessionLength in init in order to have the session extend your desired length.

This is odd behavior on Userbase's end. If sessionLength isn't provided in init, then it should stick with the session the user already has going (the expiration should not change). That seems more reasonable/what you were expecting too.

Separately, have you double checked to see that Capacitor is indeed clearing local storage? It sounds like the above may have been the root cause of the issue here.

@jneterer
Copy link
Contributor Author

jneterer commented Dec 9, 2022

Maybe it is! The only thing that makes me think it may not be, is this isn't an issue on Android. I just opened the app up a test device that I hadn't picked up for months and I was still signed in.

Wow, I didn't realize you had to set sessionLength on init. I'll have to try that and see if it makes a difference. I agree, if it isn't provided in init, it should respect what was set on signIn.

I'm going to have to re-verify. I switched to using the preferences capacitor plugin to test if that would persist the session (uses UserDefaults on iOS and SharedPreferences on Android), but it didn't, which might come back to the init problem. I'll have to do another test to see but without the storage modification.

@jneterer
Copy link
Contributor Author

@j-berman Results from two tests on iOS:

  1. Don't set sessionLength on init. Result - open more than 24 hours after sign in, logged out.
  2. Set sessionLength on init to 365. Result - open more than 24 hours after sign in, still logged in.

The above is also true on the web. I don't believe this is happening on Android, but it doesn't make sense why not.

I think the solution is two part:

  1. Set sessionLength on init (however should probably be updated in a PR to by default respect the value given on signIn)
  2. Use an alternative storage location to localStorage for session, like this capacitor plugin (UserDefaults on iOS, SharedPreferences on Android, local storage on web).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants