Skip to content
This repository has been archived by the owner on Sep 5, 2019. It is now read-only.

Gracefully handle a user revoking their refresh token #60

Open
Makeshift opened this issue Feb 22, 2018 · 5 comments
Open

Gracefully handle a user revoking their refresh token #60

Makeshift opened this issue Feb 22, 2018 · 5 comments
Assignees

Comments

@Makeshift
Copy link
Owner

A user revoking their refresh token while logged in kills the app when they try and do anything with the ESI. If any ESI request comes back with 'invalid token' we should forcefully log the user out and delete their session.

2018-02-22T12:58:14.585Z [error] getLocation: Error for requestNewAccessToken {"err":{"statusCode":400,"data":"{\"error\":\"invalid_token\",\"error_description\":\"The refresh token is expired.\"}"},"characterID":96099470}

@samuelgrant
Copy link
Collaborator

remove them from the waitlist as well.

@Makeshift
Copy link
Owner Author

Makeshift commented Mar 7, 2018

This seems to be happening more often now, this direly needs to be handled gracefully as the only fix currently is manually remove them from the db.

2018-03-07T16:41:08.285Z [error] getLocation: Error for requestNewAccessToken {"err":{"statusCode":400,"data":"{\"error\":\"invalid_token\",\"error_description\":\"The refresh token is expired.\"}"},"characterID":96099470}

Also, it's the same fucking person doing it.

@Makeshift
Copy link
Owner Author

I thought I fixed this but apparently I didn't - Let's maybe check the status on each token before we allow people to join the WL, so we aren't dealing with kicking people while we're in the middle of that.

@Makeshift
Copy link
Owner Author

Makeshift commented Jun 6, 2018

We now non-gracefully handle revoking by erroring a shitton instead of crashing, but that's a mild improvement. The previous method of fixing this was to simply delete the persons user account and the session would reset on next load, but due to req-flash this now isn't possible and the WL panics and crashes now.

Short explanation:

When alyss revoked his tokens earlier I deleted his user account, but I didn't delete his active session because normally they clean themselves up. When he went back to the site, his browser sent the cookie and tried to continue using the previous session, and req-flash panicked because it couldn't find his user account anymore, and tried to send an error page WHILE the waitlist was trying to redirect him, so it sent headers twice causing the server to crash

We need to gracefully handle a revoked token by using the session to log out the user (session.logout() or something - We should also check the waitlist for them, maybe a 'disabled' field?). On next login, the refresh token should be updated like normal and their account should be reactivated, with all user data left intact.

@samuelgrant samuelgrant self-assigned this Jun 7, 2018
@samuelgrant
Copy link
Collaborator

Once #47 has been completed I am going to add a check in the globalwaitlist.js to make sure users are online.

This is the script where things tend to die when people like Alyss revoke their tokens. If their token is revoked at this point I will flag them for a logout on next page refresh. This should stop people from getting stuck.. This script currently runs every 10 seconds so that should be enough.

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

No branches or pull requests

2 participants