ori-community / wotw-rando-server

Backend server for the Ori and the Will of the Wisps Randomizer
https://wotw.orirando.com
MIT License
3 stars 2 forks source link

Discord Profile picture caching for too long #74

Closed Trip7274 closed 1 month ago

Trip7274 commented 1 month ago

Hello! I'm not sure if this is an issue, or intended behavior, but, I've changed the profile picture for my linked Discord account, and my profile picture on the rando's client UI is still my old one, it's been at least a month since I changed my profile picture, so I'm not sure if this is moreso a bug, or if the initial Discord login was just to setup an account and associate it with my Discord account, rather than my account on the rando client being by Discord account, which would need to be updated from time to time.

for further clarification, if the client uses my initial Discord login to create a new account for me, that's separate to my Discord account, then this is probably not an issue.

but if the client uses my Discord login as my account, then this seems to be a caching issue?

I haven't tried it in multiple devices to see if a different, newer install would result in my new profile picture being loaded or not, but a friend is facing the same thing after changing their profile picture, so I'm leaning into thinking this is an intended feature, than a caching issue, if not for the fact that, using the developer tools, the link for my profile picture in the rando client seems to be pointing to a link that's erroring out whenever I try to follow it using my browser.

I'd try to help or debug this myself, but I have no experience in JS or Electron, so I probably best not mess with this repository's code with low-quality code. Apologies if this was intended behavior and I just got confused, I thought it'd be nice to bring up, at least.

timoschwarzer commented 1 month ago

Current behavior is that the profile picture ID is fetched on login and then stored on the rando server. So if you log out and log in, it should update your profile picture. Since we don't store any authentication tokens, there is currently no way to keep profile pictures up to date automatically. Might change that at some point... I'll leave this issue open as a reminder.

Trip7274 commented 1 month ago

Why not store the auth token locally on the user's client, and upon boot up (or every week or so), evaluate if the photo changed or not (checksum maybe?), and if so, have the client upload the new one to the server?

A hurdle you might have is handling the client's input safely, which shouldn't be too hard since it's a .png, .jpe?g, .webp (or even a .gif file?

I'm assuming there's some form of authentication between the rando server, and the rando client, which is how the league times are handled and all, right?

timoschwarzer commented 1 month ago

It's not that simple unfortunately. The current authentication mechanism works like this:

  1. The rando server starts an OAuth2 flow against Discord with its own secret
  2. The user is redirected to the Discord OAuth2 login page.
  3. Upon successful authentication, Discord redirects back to the backend of the randomizer with a token.
  4. The backend uses that token to fetch the user ID, user name and profile picture from Discord and stores it into the rando profile. (If no profile with that user ID exists, one is created)
  5. The rando backend generates a JWT (authentication token) that is passed to the rando frontend, which it will use to authenticate against the rando server.

As you can see, the Discord token never reaches the frontend, nor is it stored on the server. Storing OAuth2 tokens unfortunately isn't as simple as saving them into the database and be done with it. These tokens expire after a rather short amount of time, hence you always get an additional token, called refresh token, which allows the server to refresh the access token. This is also the reason we cannot store it locally in the launcher, because then the access token and refresh token would expire if you don't open the application for some days.

In general, keeping the OAuth2 sessions alive for all users just to update profile pictures seems a bit overkill. I'll have to have a look at whether there exist other means to fetch basic user profile data from Discord.

Trip7274 commented 1 month ago

Oh yeah, that's fair That's a great idea though, you could most probably try handing the frontend the user's Discord User ID, and having it query the Discord API for basic user details, and update them on the server every boot through the client? I didn't know OAuth2 tokens lasted for so short, apologies!

timoschwarzer commented 1 month ago

No worries! The frontend already has the user ID, but you also need an authentication token to query the Discord API so that won't work.

Trip7274 commented 1 month ago

I think I heard that all you need is a bot to query the Discord API for a user's data So, this might be a bit janky, but you can:

  1. Front-end boots up
  2. Front-end sends a request to the back-end with its own Discord user ID
  3. Back-end sends a request to the Discord API using its own Discord Bot token, and updates the user's PFP and general data
  4. Back-end sends a request back to the front-end to either prompt it to update its cache again, or just including the user's new data

This is probably a bit over-engineered and un-scalable, but I'm just giving my own ideas, for whatever they're worth This endeavor is most probably achievable though, since there are plenty of websites (like this) that take in a User ID, and return general info of the associated user (Username, PFP, Badges, and account creation date)

timoschwarzer commented 1 month ago

Yeah something like that could work, but I'll probably skip the detour through the frontend and just let the backend update profile pictures periodically.

Trip7274 commented 1 month ago

Fair, fair I'm just concerned about unnecessary load, especially since there'll always be some amount of dead accounts, ya know? but now that I think about it, there'll be some amount of people who'll boot up the client much more often than the periodic check, and they might hammer the back-end more if the check was done every time the client boots, so that's a fair solution, yeah.

Thanks for your time with this, means a lot!