voting system,crud operation with integration testing and hash function to generate anonymous id updated#40
voting system,crud operation with integration testing and hash function to generate anonymous id updated#40Hrusi01 wants to merge 17 commits intop-society:mainfrom
Conversation
Summary of ChangesHello @Hrusi01, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a new mechanism for generating consistent, anonymous user identifiers within specific threads. By utilizing a SHA256 hash of the user's actual ID, the thread ID, and a server-side secret key, the system can produce unique, yet non-traceable, 'random-looking' names. This approach aims to enhance user privacy by not storing direct user identifiers in the database for these anonymous interactions, while still allowing the system to recognize a user's identity within a given thread context by re-generating the same anonymous name. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a function to generate anonymous user names using an HMAC-based approach. While the core idea is sound, there are several issues that need to be addressed. Most importantly, there is a critical security concern based on the pull request description, which suggests the secret key might be coming from the client. This would completely compromise the security of this feature. Additionally, there is a bug in an environment variable validation message that provides a confusing error, and a couple of smaller improvements are suggested for correctness and consistency in the name generation logic.
| @@ -0,0 +1,31 @@ | |||
| import crypto from "crypto"; | |||
|
|
|||
| import { getSecret } from "../secretPass"; | |||
There was a problem hiding this comment.
The pull request description states: "a secret key from front end using by the user that to be passed by post request". If the getSecret function implements this, it represents a critical security vulnerability. The secret key used for an HMAC must be kept confidential on the server and must never be exposed to or transmitted from the client. If a client can provide the secret, the entire security model is broken, as anyone could then generate names for any user, defeating the purpose of anonymity. The secret key should be securely loaded from the server's environment variables.
There was a problem hiding this comment.
then it can be bypass by the admin as he have uid thread id and key.. but we can use some method to restrict users to use multiple password in a single thread using flag but that may expose their identity in admin data base need to brain strom
| const adjIndex = parseInt(hash.substring(0, 8), 16) % adjectives.length; | ||
| const animalIndex = parseInt(hash.substring(8, 16), 16) % animals.length; |
There was a problem hiding this comment.
Using the modulo operator (%) on a large number from a hash to derive an index can introduce a slight statistical bias, known as modulo bias. This means that some names might be generated slightly more frequently than others. While this may be acceptable for this use case, it's worth noting for future reference, especially in more cryptographically sensitive contexts where a uniform distribution is required.
There was a problem hiding this comment.
bias is negligible near to 7* 10 pow -9 but yeah we can eliminate that by limit and 32 bit hash segment
| const animalIndex = parseInt(hash.substring(8, 16), 16) % animals.length; | ||
| const num = parseInt(hash.substring(16, 20), 16) % 100; | ||
|
|
||
| return `${adjectives[adjIndex]}${animals[animalIndex]}${num}`; |
There was a problem hiding this comment.
The generated number num is between 0 and 99. When this number is a single digit, the total length of the anonymous name will vary. For consistency, it's better to pad the number with a leading zero to ensure it's always two digits long.
| return `${adjectives[adjIndex]}${animals[animalIndex]}${num}`; | |
| return `${adjectives[adjIndex]}${animals[animalIndex]}${String(num).padStart(2, '0')}`; |
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
|
paggination added in post and thread |
| @@ -0,0 +1,31 @@ | |||
| import crypto from "crypto"; | |||
|
|
|||
| import { getSecret } from "../secretPass"; | |||
There was a problem hiding this comment.
@Hrusi01 where is this func? i cant find this in the PR
we can find that in post table vote column as post.vote if someone down vote or up vote final score is always calculated by the change in value relative to the previous vote if a has vote 0 |
|
@Hrusi01 the vote mapping table is just to prevent multiple upvote or downvote from a user so i think we can have downvote and upvote as boolean so a user on upvoting will be upvote as true and on downvoting downvote as true it will better to maintain |
@neoandmatrix @ujsquared have a look |
i think it mean vote table with 3 column pk, 2 columns upvote(default o yes if 1 ), down vote(default 0 yes if 1 ) but existing schema has 2 column as pk,value(-1,0,1) it will reduce space and easy while maintainance by scaning a single coloumn we can get 3 variable data |
|
@Hrusi01 fix the ci errors |
Fix the ci errors we will discuss the schema later on vc |
bhaiya apka signoff missing he |
|
|
@Hrusi01 leave dco bot Fix is the ci errors https://github.com/p-society/iiitbuzz/actions/runs/20161352795/job/57874679402#step:5:1 Its again failing |
please check if it is still failing |
|
check all mein no errors |
neoandmatrix
left a comment
There was a problem hiding this comment.
Too big of a PR with many changes at once.
My suggestions :-
- Remove all the get route functionality overlapping with #39 .
- Make changes manageable to review like one pr which adds all the schemas with dto and other one to add the logical and functionality.
CC: @iamanishx @ujsquared
@Hrusi01 remove the GET functionality overlapping with the other PR then ping us will review it |
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
7747cd6 to
d2bb11f
Compare
removed the get function and crud function |
|
@Hrusi01 please don't expose the db to the whole internet put some authentication and authorisation |
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
|
@Hrusi01 whats tanstack query doing in the package.json? there is nothing to do with this pr |
apps/web/package.json
Outdated
| "@react-router/node": "^7.6.1", | ||
| "@react-router/serve": "^7.6.1", | ||
| "@tanstack/react-form": "^1.12.0", | ||
| "@tanstack/react-query": "^5.80.5", |
There was a problem hiding this comment.
Why??? Are there any frontend changes here?
apps/web/package.json
Outdated
| "react": "19.1.0", | ||
| "react-dom": "19.1.0", | ||
| "react-router": "^7.6.1", | ||
| "react-router-dom": "^7.10.1", |
There was a problem hiding this comment.
tried to test the front end at sometime that time i installed react router dom by mistake
| ]; | ||
|
|
||
| fastify.post("/anon-name", async (request, reply) => { | ||
| const parsed = bodySchema.safeParse(request.body); |
There was a problem hiding this comment.
I think auth checks are needed here.
|
@neoandmatrix @ujsquared kal iska schema discussion karte hainn bc mein |
sure |
|
Remove apps/server/src/routers/topics.ts and post.dto,ts from the unrelated to your issue |
|
@Hrusi01 remove apps/server/src/routers/posts.ts also |
|
@Hrusi01 apps/server/src/routers/votes.ts 1st of all why is this under /routes? And again no authentication or authorisation!! |
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
Signed-off-by: Hrusi01 <hrusikeshkar01@gmail.com>
|
@iamanishx @neoandmatrix |

update 👍
Vote functionality
END POINTS:
HASH FUNCTION
this function returns random looking names that cant be traceback to original user id
Algorithm flow
uses hash of seed and secret key(provided by user) using that in sha 256
from adjectives and animal dictionary of 30 each and number 2 digit 0 to 99 we can generate unique unknown uid
3030100 =90,000
unique id per thread
even if the secret key and thread id know HASH cant be reversed and uid is not stored in data base and only known to the current state of the server
How the system will know about the person?:
when we will pass user id ,thread id and secret key in that hash it will give same random user name out put per unique thread
by that system can check which user is this
db wont store the secret key it is only known to the server
what function expects:?
a secret key from front end using by the user that to be passed by post request
@iamanishx #38