🐛 Only allow to search *owned* files in storage search endpoint

[bisgaard-itis]

What do these changes do?

• The result in File uploads using the new-style api are getting errors related to search for filename #5729 shows that searching for a file with read-access using its sha256 checksum is almost 200x slower than searching for it if you consider it a file with write-access (of exact factor depends on the amount of data the user has in the db etc). As pointed out to me by @sanderegg the write-access search is in fact buggy. What's really searched are owned files. To make this explicit I remove the access_rights query parameter from the endpoint

POST /v0/simcore-s3/files/metadata:search

and instead introduce the compulsory query parameter kind which must equal owned to clearly indicate to the user (of storage) that only owned files are searched using this endpoint.

• Update api-server to use this corrected endpoint

Related issue/s

• resolves Only search write access files in api-server #5771
• hopefully fixes the issues in File uploads using the new-style api are getting errors related to search for filename #5729 🤞🏻

How to test

Dev-ops checklist

• No ENV changes or I properly updated ENV (read the instruction)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.6%. Comparing base (cafbf96) to head (e00d85c).
Report is 175 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master   #5772      +/-   ##
=========================================
- Coverage    84.5%   67.6%   -17.0%     
=========================================
  Files          10     666     +656     
  Lines         214   32725   +32511     
  Branches       25     205     +180     
=========================================
+ Hits          181   22133   +21952     
- Misses         23   10540   +10517     
- Partials       10      52      +42     

Flag	Coverage Δ
integrationtests	63.9% <ø> (?)
unittests	89.9% <100.0%> (+5.3%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...src/simcore_service_api_server/api/routes/files.py	78.1% <100.0%> (ø)
...vice_api_server/api/routes/solvers_jobs_getters.py	92.9% <100.0%> (ø)
...src/simcore_service_api_server/services/storage.py	68.7% <100.0%> (ø)
...api_server/services/study_job_models_converters.py	95.6% <100.0%> (ø)
...src/simcore_service_storage/handlers_simcore_s3.py	100.0% <100.0%> (ø)
...ices/storage/src/simcore_service_storage/models.py	95.7% <100.0%> (ø)
...rage/src/simcore_service_storage/simcore_s3_dsm.py	94.7% <ø> (ø)

... and 666 files with indirect coverage changes

[sanderegg]

Thanks a lot for checking. But tbh I don't really understand the solution. Here you are changing the logic in order to improve performance. But actually write access is a completely different thing than read access.
As far as I remember, @pcrespov made the "file access rights" actually reads the projects access rights which are a JSON column.
So that means that a project that is shared with READ access rights means that the files are also readable. If the project is shared with WRITE access rights, then you have also WRITE access to the files. Therefore I am not sure your change makes sense.
Now the projects access rights are a JSON column, which is for sure not efficient. Maybe we should discuss that also together with @matusdrobuliak66 as we talked about moving the JSON column to its own table, which could also vastly improve that speed.
But a change like that might have large implications here.

[bisgaard-itis]

Thanks a lot for checking. But tbh I don't really understand the solution. Here you are changing the logic in order to improve performance. But actually write access is a completely different thing than read access. As far as I remember, @pcrespov made the "file access rights" actually reads the projects access rights which are a JSON column. So that means that a project that is shared with READ access rights means that the files are also readable. If the project is shared with WRITE access rights, then you have also WRITE access to the files. Therefore I am not sure your change makes sense. Now the projects access rights are a JSON column, which is for sure not efficient. Maybe we should discuss that also together with @matusdrobuliak66 as we talked about moving the JSON column to its own table, which could also vastly improve that speed. But a change like that might have large implications here.

I see your point. The reason I think it probably makes sense to change to write access here is that files which are accessed through the api-server are (or at least that's what I believe) uploaded via the api-server. Meaning the user has write access to them anyway. But I am not 💯 if this is correct, so that's why I asked the question above.

[pcrespov]

Thanks a lot for checking. But tbh I don't really understand the solution. Here you are changing the logic in order to improve performance. But actually write access is a completely different thing than read access. As far as I remember, @pcrespov made the "file access rights" actually reads the projects access rights which are a JSON column. So that means that a project that is shared with READ access rights means that the files are also readable. If the project is shared with WRITE access rights, then you have also WRITE access to the files. Therefore I am not sure your change makes sense. Now the projects access rights are a JSON column, which is for sure not efficient. Maybe we should discuss that also together with @matusdrobuliak66 as we talked about moving the JSON column to its own table, which could also vastly improve that speed. But a change like that might have large implications here.

I see your point. The reason I think it probably makes sense to change to write access here is that files which are accessed through the api-server are (or at least that's what I believe) uploaded via the api-server. Meaning the user has write access to them anyway. But I am not 💯 if this is correct, so that's why I asked the question above.

It also makes sense to me.

[pcrespov]

thx

[matusdrobuliak66]

Reviewed in person

[bisgaard-itis]

@pcrespov @sanderegg I rerequest reviews because I now changed this PR to also contain changes to storage

[sanderegg]

very nice. Consider my comments regarding adding the assertion and/or the test

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud