search

m2ms / fragalysis-frontend

The React, Redux frontend built by webpack
Other
1 stars 1 forks source link

Ensure no sensitive data can be extracted from closed projects #1414

Open mwinokan opened 5 months ago

mwinokan commented 5 months ago

Matteo's slack:

https://fragalysis.diamond.ac.uk/viewer/react/projects/170/161 for me to check as am not on that proposal. Looking at the developer console no download gave 403, but no structures were loaded. But I could get his email address. Majorly https://fragalysis.diamond.ac.uk/api/session-projects/170/ allowed me to download the template Iterating not logged in, from 0 to 200, this leak could be used only for these three: 'A71EV2A', 'CHIKV_Mac', 'MVMMpro'

mwinokan commented 5 months ago

The template_protein path contains sensitive strings The sequences may also be sensitive

mwinokan commented 5 months ago

@alanbchristie to go through the endpoints and make sure they are properly locked down to authenticated users only

Is it possible to lock down snapshots to authorised users as well?

@alanbchristie says that snapshots could be manipulated to extract other sensitive data too

alanbchristie commented 5 months ago

The attached CSV is an in initial summary of endpoints and whether they "might" be open. This has been a manual inspection, after visiting all the endpoint implementations (Views) to see if there were any that were "vulnerable" (OPEN). Whether they are open or not does take some investigating as some are obvious and some or more complex and obfuscated. But, yes, snapshots are open. Next step is to asses the logic required to close the important ones that are OPEN.

api.csv

mwinokan commented 5 months ago

@alanbchristie says that there is no project/proposal validation for snapshots, should be easy to patch. But dealing with all endpoints could take a lot of work.

@Waztom to please go through the endpoints and see which data is the 'riskiest' of the 36 open==TRUE endpoints

mwinokan commented 3 months ago

@Waztom has started taking a look and is going through the rest of the endpoints

phraenquex commented 2 months ago

Waiting for #1247 - test when that is done.

phraenquex commented 2 months ago

Changing to green, because a dependency on another green ticket #1247 . @mwinokan please test.

mwinokan commented 1 month ago

@alanbchristie is still concerned about some these API changes causing issues in the app. @alanbchristie is not able to fully test everything because he is unfamiliar with the frontend.

One more round of testing is required after Alan's pending deployment. @mwinokan / @Waztom

@alanbchristie also clarifies that currently only users on the proposal are able to upload even to open projects. @mwinokan, @Waztom, @phraenquex to discuss whether this is the desired behaviour. Alan also raises concerns about unauthenticated users being able to delete RHS data in open targets.

mwinokan commented 4 weeks ago

@alanbchristie suggests releasing this earlier, as we have done some testing already (even if it's not exhaustive).