Noteslify icon indicating copy to clipboard operation
Noteslify copied to clipboard

Send only the required data from API to frontend.

Open Gauravdarkslayer opened this issue 2 years ago • 1 comments

What would you like to share?

There are some GET APIs which are sending unnecessary data (which are not required by the frontend), so need to fetch only those fields which are required by the frontend, it will help in user security and it also consumes less network bandwidth.

Additional information

No response

Gauravdarkslayer avatar Oct 31 '22 15:10 Gauravdarkslayer

Hey @Gauravdarkslayer, Thanks for contributing and Congrats on opening Issue :tada:

We will try to review as soon as possible and a maintainer will get back to you soon!

github-actions[bot] avatar Oct 31 '22 15:10 github-actions[bot]

I would love to work on this issue. Can you provide more info?

thevinitgupta avatar Jan 06 '23 09:01 thevinitgupta

thevinitgupta

@abhinandanwadwa Could you guide him here? Thanks!

devarshishimpi avatar Jan 06 '23 09:01 devarshishimpi

@thevinitgupta

So, basically there are various routes which are returning some useless json data, which was not supposed to be returned and was not required in the frontend. For example, look at the line below.

https://github.com/dvstechlabs/Noteslify/blob/8f08361dc59ad8839207498b4f143473f9cc59cd/apps/web/backend/routes/notes.js#L42

Instead of returning the newNote object, it's better to return a message saying, "successfully added new note!".

You can find all similar routes which are returning useless data and replace them with success text

abhinandanwadwa avatar Jan 06 '23 09:01 abhinandanwadwa

@thevinitgupta I would like to add one more point. Some routes also returning extra fields which are also not required by the frontend. So, .select() while querying can be used in this case. It will help to retrieve only the required fields from collection. It takes array of fields in string format.

Example of .select() in mongoose: https://mongoosejs.com/docs/queries.html#:~:text=%7D).%0A%20%20select(%7B-,name,-%3A%201%2C OR https://www.mongodb.com/docs/manual/tutorial/project-fields-from-query-results/#return-the-specified-fields-and-the-_id-field-only

Example where it's required: https://github.com/dvstechlabs/Noteslify/blob/8f08361dc59ad8839207498b4f143473f9cc59cd/apps/web/backend/routes/notes.js#L109-L115

Here, its returning whole array of objects which contains sensitive information like secret keys. So, try to select only required information while querying.

Gauravdarkslayer avatar Jan 06 '23 11:01 Gauravdarkslayer

I agree with @Gauravdarkslayer there are many more endpoints that are returning sensitive data along with the required data, like getting a particular note and update note,etc.

I would like to proceed with this. Could you please assign me?

thevinitgupta avatar Jan 06 '23 12:01 thevinitgupta

Sure, Here you go. @thevinitgupta

Gauravdarkslayer avatar Jan 06 '23 12:01 Gauravdarkslayer

Thanks

thevinitgupta avatar Jan 06 '23 12:01 thevinitgupta

image @Gauravdarkslayer I am getting this error on requesting getallnotes endpoint.

This issue is with the decrypt function.

thevinitgupta avatar Jan 07 '23 08:01 thevinitgupta

@thevinitgupta You can check out this link I guess It's a version issue, so try installing the exact version mentioned in the package.json.

Gauravdarkslayer avatar Jan 07 '23 09:01 Gauravdarkslayer

I tried cloning the repository again and install with the Node version specified. Also, tried the solutions in the link provided. The issue is still persisting.

Could you check if the issue is with the crypto package being used in the helper.js file?

thevinitgupta avatar Jan 07 '23 14:01 thevinitgupta

@Gauravdarkslayer Similar issues on my end, request you to look into it.

devarshishimpi avatar Jan 07 '23 14:01 devarshishimpi

Here I've created an issue and it's corresponding PR. Have a look. https://github.com/dvstechlabs/Noteslify/pull/260

Gauravdarkslayer avatar Jan 07 '23 20:01 Gauravdarkslayer

Here I've created an issue and it's corresponding PR. Have a look. #260

Merged @thevinitgupta I think the issue should be sorted now 😁

devarshishimpi avatar Jan 08 '23 05:01 devarshishimpi