Do not try to auto refresh a token on a revoked user #4747
Conversation
| if (session->m_config.error_handler) { | ||
| auto user_facing_error = SyncError( | ||
| realm::sync::ProtocolError::permission_denied, | ||
| "Unable to refresh the user access token; has this user been disabled by an admin?", true); |
There was a problem hiding this comment.
I think the error message is a bit misleading here - the UI for revoking sessions is just that - it revokes all refresh tokens for that user but doesn't disable the user. So if they reauthenticate with their credentials, they should be able to obtain a new valid refresh token and restart sync.
There was a problem hiding this comment.
Turns out I was actually testing the disable feature and not the revoke feature, but I've updated the tests to include revoke as well which as you pointed out is different from disabling a user because logging in again will issue a new refresh token. I've also updated the message emitted here.
There was a problem hiding this comment.
I agree with @nirinchev's comments, but otherwise LGTM!
Love that the admin API stuff is getting more broadly used and that there's a real integration test.
ironage
force-pushed
the
js/handle-refresh-failure
branch
from
53b6393 to
1224637
Compare
Fixes #4745
The auto refresh would recursively try to refresh the token every 10 seconds which would loop endlessly on a user who had been revoked by an admin. Eventually this would have caused a stack overflow.
☑️ ToDos