search

bgreenawald / madgab-online

1 stars 0 forks source link

ID Generation #35

Open bgreenawald opened 3 years ago

bgreenawald commented 3 years ago

Does the frontend enforce the uniqueness of game IDs? I was hoping to change the process for generating game ID's, replacing the _getnames endpoint with a _checkname endpoint (the client being able to access the name of all active games seems like a great way for people to sabotage other peoples games). Merely having an endpoint that returns True or False if a given name is already in use would be much more secure.

That being said, when looking at the frontend code, I can't find where the client checks to ensure that the new ID isn't currently in use.

JocelynYH commented 3 years ago

This is what we have on the frontend: https://github.com/bgreenawald/madgab-online/blob/6f8a72818c2fb2aaaddc4f756d5ef8dd15d208be/frontend/src/store/actions.js#L22

JocelynYH commented 3 years ago

The boolean endpoint is a great idea.

bgreenawald commented 3 years ago

I've changed the endpoints, deprecating the _/api/getnames endpoint and replacing it with _/api/checkname/\<ID> which return a payload with structure

{
    "in_use": True|False
}
bgreenawald commented 3 years ago

And maybe I'm misunderstanding the frontend code, but I'm still not sure where the client blocks ID suggestions to users that are already in use. We obviously can't prevent a user from going to an ID that is already in use and crashing another user's game, but having the client use that new endpoint to give users a warning that an ID is already in use and prompting them to try a different one would be good.

JocelynYH commented 3 years ago

Yeah, the frontend code just generates another ID if the random number generator landed on an ID already in use. It's that function you wrote - I didn't touch it really.

But yes, we don't actually have anything that'd prevent the client from deciding to copy an in-use ID.

So, from the frontend, this is what we wanna do yes? user submits an ID (provided or auto generated), then frontend hits the new API - if in_use === false, happy path, else show error?

for your viewing pleasure:

Get resp from new endpoint..
if (resp.in_use === true) //display error
else // happy path
bgreenawald commented 3 years ago

That's exactly what I'm thinking!

JocelynYH commented 3 years ago

Perfect! I'll do that then!

On Tue, Feb 2, 2021, 12:02 Benjamin Greenawald notifications@github.com wrote:

That's exactly what I'm thinking!

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/bgreenawald/madgab-online/issues/35#issuecomment-771791900, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMDHILF5GKQCM55OJYTHGDS5AV2HANCNFSM4W3TGE5Q .