magshimim-next / cv-next

A simple cv upload and peer review system
7 stars 2 forks source link

Optimize usage of ids for APIs and link #161

Open Aradon267 opened 1 month ago

Aradon267 commented 1 month ago

A while back @YechiamW3 asked me about using any other form of recognition(not the full ID) for API calls, and @TheGreenOak made a point about the links to the user page being website.com/profile/superlong-generatedUID-thatlookslike-badphishing So maybe these 2 can be combined?

YechiamW3 commented 1 week ago

Yeah I think we should auto-generate usernames (and make them editable from the Profile page, with validation for unique), and use them for all user-based interactions.

YechiamW3 commented 1 week ago

Did a little something locally, I'll open a PR for this (I think this should be of a moderate priority). Generating username upon login (if one does not exists already), with a bit of hashing to preserve uniqueness: image

I'll open a PR only for implementing this mechanism, but more steps are required: 1.a. Provide a pop-up on first-login (can we notice this? not sure if supabase has this functionality when signing-in using social provider) to change username. 1.b. Add uniqueness validation when updating username (can build upon code I'll put up)

  1. Move all user-based API calls/routings to username when available, otherwise fallback to user id (if for some reason username generation fails).

@Aradon267 are there any more steps you think are needed?

Aradon267 commented 1 week ago

I don't think we have a way to detect the first login because we do need to remember that the "first login" of a user is based on when we change that user's perms

The problem that I foresee is that I'm not sure we have a reliable way to get the generated username - we have a way to do this with the ID(getUser of Supabase) - where will the name be stored? User metadata is the only thing that I can think that is maybe possible

YechiamW3 commented 1 week ago

I don't think we have a way to detect the first login because we do need to remember that the "first login" of a user is based on when we change that user's perms

The problem that I foresee is that I'm not sure we have a reliable way to get the generated username - we have a way to do this with the ID(getUser of Supabase) - where will the name be stored? User metadata is the only thing that I can think that is maybe possible

Regarding the "first login" - yeah thought that much. Perhaps we can do a weird shenanigans, do we have a "created at" timestamp for the user? We can try to compare against that. Weird workaround, but might work.

Regarding the problem you mentioned - not sure what you mean. We can simply select by username, no? According to the logic I made, we make sure the username is unique*, and if we fail we simply fallback to id-based control.

*now that I think about it, I need to check if we can lock the user table for the few ms when it attempts generation. It retries 10 times, but doesn't retrieve the usernames data on each retry (it'd be bad), so update-lock to the table would be good for the whole operation.

Aradon267 commented 1 week ago

Even the created at won't really work because we have a buffer between the creation and the perms update, maybe we can add like a bool for that or something but I'll look into ways to see first login or measure it

When we do the requests, will the username be stored in a cookie or changed in the oAuth metadata itself? metadata might work, cookies makes it super vulnerable

Update lock is interesting

YechiamW3 commented 1 week ago

You mean because the oAuth includes the id as part of its claims? I'm pretty sure we can add a claim for the username.

Aradon267 commented 1 week ago

Yes that's the "metadata" that we can add