-
Notifications
You must be signed in to change notification settings - Fork 170
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
Log out user on token expiration #4882
Comments
@ironage, you had the misfortune to touch this code last and your name is forever engraved in the git history - perhaps you can answer the question part and then, if it turns out it's a legitimate bug, we can negotiate the fixing part 😜 |
It would make sense to me to log the user out in this situation as well. The correct thing to do to solve the refresh token being expired is to log in the user again. Looks like Jason was the one who originally added this, @jsflax can you confirm that there's no special reason to keep the user logged in here? |
Looking into this more, it appears we're handling realm-core/src/realm/object-store/sync/sync_session.cpp Lines 551 to 562 in 779c0a5
My guess is we should unify all of that handling somewhere and make sure we behave consistently regardless of whether the auth error comes through sync or http. |
What is the alternative behaviour if their refresh token has expired? I could very well be mistaken, but I thought we had to force them to re-login. |
I'm not sure I quite understand your question, but the current behaviour if the refresh token expires is that the session is closed and an error is reported to the user. If they try to open it again the same thing will happen. The developer might not know to handle this situation and log the user out. But if we change this to log the user out as well, then the app flow should be seamless the next time the session is opened because the user will be forced to log in. |
Then that sounds like the solution 😄 . This was my original intention and it somehow got corrupted along the way. The user should be logged out. |
@ironage is this something you can take further? |
Can we prioritize this. Since anonymous users have 60 day token expiration and you can't have more than one anon user logged in at the same time, it's making life really difficult for developers who chose to use anonymous auth - e.g. this forum post |
@nirinchev how does this solve that problem? Logging the user out on refresh expiration seems to be more for their convenience. |
It doesn't break their app the very least. Obviously, they'll need to redownload the data when they re-authenticate the anonymous user, but at the very least, they won't have to delete the app data or implement convoluted error handling for it. |
I'm struggling with this issue as well. On my clients phone the app stopped syncing, but otherwise was working fine. The client was unaware, that he was not getting any server updates anymore. There was no error log on the server, so I only noticed the issue by checking the clients log file:
I didn't know that I have to catch the error. The docs are not very helpful there. My solution for now would be: Kotlin:
Swift:
In Swift I am missing the errorcode! How can I handle the specific error here? I agree that the logout should be handled automatically! I'm using JWT Auth and I set the expiration date to 100 years. So I was hoping I would never have to worry about a token expiring in my lifetime ;-) . But I still came across it. Could you check if there is a bug when setting the expiration date on your end or if I messed up? There is a discussion about it in the forum: https://www.mongodb.com/community/forums/t/realm-refresh-token-expiry-and-customisation/107199/7 |
This is more of a question than a bug report, but looking at
SyncSession::handle_refresh
, I noticed we check for refresh token expiration and terminate the session in that case, however we don't log the user out:realm-core/src/realm/object-store/sync/sync_session.cpp
Lines 375 to 382 in 779c0a5
This seems a bit odd as there's no way for the developer to recover from this other than forcing the user to login. Should we change this behavior to be consistent with the handling of 401/403 error codes below:
realm-core/src/realm/object-store/sync/sync_session.cpp
Lines 394 to 396 in 779c0a5
The text was updated successfully, but these errors were encountered: