Dun-sin / Whisper

A fun Application to have a random chat with people safely
https://whisper.favour.dev/
MIT License
402 stars 375 forks source link

[FEATURE] FRONT-END: Validate loginId to be a valid uuid #716

Open ms-oh opened 1 month ago

ms-oh commented 1 month ago

Description

Code: https://github.com/Dun-sin/Whisper/blob/1c1224036887cd9981d6c389e987569c07193976/client/src/context/AuthContext.jsx#L30

Screenshots

No response

Additional information

No response

👀 Have you checked if this issue has been raised before?

🏢 Have you read the Contributing Guidelines?

Are you willing to work on this issue ?

Yes I am willing to submit a PR!

github-actions[bot] commented 1 month ago

To reduce notifications, issues are locked until they are valid/approved and to be assigned. In the meantime read the contributing guidelines -> https://github.com/Dun-sin/Whisper/blob/main/CONTRIBUTING.md

Dun-sin commented 1 month ago

Hi @ms-oh we don't use UUID anymore to generate user login, this is the code we use now

const userID = Math.random().toFixed(12).toString(36).slice(2);

you can use a function like this to fix the issue and call it, place this function in the utils folder

function validateUserID(userID) {
  const userIDPattern = /^[a-z0-9]{12}$/;

  return userIDPattern.test(userID);
}

thanks for wanting to contribute. Make sure to read the issue description carefully and ask if you have questions on the discord server. Follow the rules here, or your PR won't be accepted and will be closed. Good luck!

github-actions[bot] commented 1 month ago

The issue has been unlocked and is now ready for dev. If you would like to work on this issue, you can comment to have it assigned to you.

ms-oh commented 1 month ago

but I can see crypto module being used in node-js

https://github.com/Dun-sin/Whisper/blob/1c1224036887cd9981d6c389e987569c07193976/server/utils/helper.js#L17C1-L19C2

function generateObjectId() {
  return crypto.randomBytes(12).toString('hex');
}
Dun-sin commented 1 month ago

but I can see crypto module being used in node-js

https://github.com/Dun-sin/Whisper/blob/1c1224036887cd9981d6c389e987569c07193976/server/utils/helper.js#L17C1-L19C2

function generateObjectId() {
  return crypto.randomBytes(12).toString('hex');
}

that's the server side, your PR should be focusing on the frontend(server)

Dun-sin commented 1 month ago

@ms-oh here's a simple run down of how we create users, there's the anonymous users and the email users, when the anonymous users sign up we use the math function as shown above to create an id, but when the email users sign up we use the crypto.randomBytem in nodejs.

The function i gave you to use, focuses on the fact that using both methods, a user ID should be 12 letters long and hex.

using crypto and UUID are not the same thing.

But to be sure it works for crypto as well, you should test with an email user and an anonymous user

Dun-sin commented 2 weeks ago

@ms-oh it's been two weeks, please create a PR today, or you'll be unassigned💪🏾

Gopal-001 commented 2 days ago

@Dun-sin do we need to validate the regex for the login Id here, if that's the case i'd like to take this up.

Dun-sin commented 2 days ago

@Dun-sin do we need to validate the regex for the login Id here, if that's the case i'd like to take this up.

@Gopal-001 please read the previous conversations in this issue, i explained more there, after that you can let me know if you still want to take it up

Gopal-001 commented 2 days ago

yes I want to take this up @Dun-sin bcz as far as i can understand we need to verify the loginId which we are generating via some functions (not a uuid) using a regex expression called inside a function which will be imported from utils. just wanted to know do we need to throw an error when validation fails ? https://github.com/Dun-sin/Whisper/blob/1c1224036887cd9981d6c389e987569c07193976/client/src/context/AuthContext.jsx#L18C1-L39C3

Dun-sin commented 1 day ago

yes I want to take this up @Dun-sin bcz as far as i can understand we need to verify the loginId which we are generating via some functions (not a uuid) using a regex expression called inside a function which will be imported from utils. just wanted to know do we need to throw an error when validation fails ? https://github.com/Dun-sin/Whisper/blob/1c1224036887cd9981d6c389e987569c07193976/client/src/context/AuthContext.jsx#L18C1-L39C3

so that's what the function is for, the one i shared, i've gone in and checked both ids generated for userId and that function satifies checking for them also the past use of UUID? if you do find a mistake related to this, feel free to add it to your PR but don't forget to explain in your PR

just add the function to the utils/lib for both (server & client) or can try the shared part of it.

yes just like the code you just shared a similar error should occur and user should be logged out

it's been assigned to you @Gopal-001