unmojang / drasl

Yggdrasil-compatible API server for Minecraft
GNU General Public License v3.0
69 stars 11 forks source link

Allow duplicate client tokens #72

Closed evan-goode closed 1 month ago

evan-goode commented 1 month ago

Allow multiple users to use the same client token without interfering with each other.

This change bumps the SQL database version from 1 to 2 and introduces a migration step.

For https://github.com/unmojang/drasl/issues/53.

IkyMax commented 1 month ago

Hey! sorry for taking too long! It works now! just idk if this is a bug, but whenever i change the username (which is linked to the drasl username) to other one, it's doesn't seem to ve a valid accessToken, that invalidates te session whenever i change and account and it won't update the username in the launcher side, /validate endpoint throws an error 403 (it seems to throw this with any user, even if it's a freshly generated accessToken, it's supposedly to give a 204 code), and the /invalidate and /refresh ones arent working only with users who changed the Minecraft Username

IkyMax commented 1 month ago

Hey! sorry for taking too long! It works now! just idk if this is a bug, but whenever i change the username (which is linked to the drasl username) to other one, it's doesn't seem to ve a valid accessToken, that invalidates te session whenever i change and account and it won't update the username in the launcher side, /validate endpoint throws an error 403 (it seems to throw this with any user, even if it's a freshly generated accessToken, it's supposedly to give a 204 code), and the /invalidate and /refresh ones arent working only with users who changed the Minecraft Username

I forgot to clarify what i'm using the normal endpoints, not the authlib-injector ones.

btw, if the mc user is kind of dettached from the actual Drasl account, it wouldn't be possible to add multiple mc accounts to a single Drasl account? also, as an improvement, it would be nice if we can ban players instead of deleting their account.

evan-goode commented 1 month ago

Hey! sorry for taking too long! It works now! just idk if this is a bug, but whenever i change the username (which is linked to the drasl username) to other one, it's doesn't seem to ve a valid accessToken, that invalidates te session whenever i change and account and it won't update the username in the launcher side, /validate endpoint throws an error 403 (it seems to throw this with any user, even if it's a freshly generated accessToken, it's supposedly to give a 204 code), and the /invalidate and /refresh ones arent working only with users who changed the Minecraft Username

Thanks for testing, I noticed a bug in the database migration code that could be related to the errors you mentioned. I fixed it and force-pushed my changes. Now I'm not seeing any issues after changing a user's username.

If you want to test again, you'll have to (1) either delete your database to get a totally new one, or restore your database from a backup from before you tested this PR, AND (2) pull the latest changes:

git fetch evan-goode
git checkout evan-goode/allow-duplicate-client-tokens
evan-goode commented 1 month ago

Oh and to answer your questions

btw, if the mc user is kind of dettached from the actual Drasl account, it wouldn't be possible to add multiple mc accounts to a single Drasl account?

That would be cool, I think it would require a lot of refactoring though, so probably a long-term goal. Maybe create an issue for it?

also, as an improvement, it would be nice if we can ban players instead of deleting their account.

In the Admin panel you can "lock" accounts which is pretty much the same as banning them. If an account is locked, that means they can't log in to the web interface or to servers.

IkyMax commented 1 month ago

Thanks for testing, I noticed a bug in the database migration code that could be related to the errors you mentioned. I fixed it and force-pushed my changes. Now I'm not seeing any issues after changing a user's username.

If you want to test again, you'll have to (1) either delete your database to get a totally new one, or restore your database from a backup from before you tested this PR, AND (2) pull the latest changes:

git fetch evan-goode
git checkout evan-goode/allow-duplicate-client-tokens

Hey! i tested it and it works now! i erased the database so im not sure if the db upgrade works, btw, idk if it was me or it has some errores because i got an error 500 in a try, but anyways, it works now, thx!