ispyb / py-ispyb

ISPyB backend server based on FastAPI
GNU Lesser General Public License v3.0
12 stars 14 forks source link

Add onetime tokens #214

Closed stufisher closed 1 year ago

stufisher commented 1 year ago

Adds onetime tokens to allow the client to download files from the api. Urls are signed and a unique token generated which is stored in the db. This can be used a single time to download the signed url, and otherwise expire on a short timescale (10s)

to be merged after https://github.com/ispyb/py-ispyb/pull/203

mgaonach commented 1 year ago

Also, It is possible to download files without this system.

See getObjectURL used by the ZoomImageBearer component: https://github.com/ispyb/py-ispyb-ui/blob/15-implement-ssx-fundamentals/src/components/image/zoomimage.tsx#L67

This is to display image, but could also be used the same way to download it.

antolinos commented 1 year ago

Out of curiosity, what is the use case for the onetime single token?

stufisher commented 1 year ago

Also, It is possible to download files without this system.

See getObjectURL used by the ZoomImageBearer component: https://github.com/ispyb/py-ispyb-ui/blob/15-implement-ssx-fundamentals/src/components/image/zoomimage.tsx#L67

This is to display image, but could also be used the same way to download it.

You can do this, and for images is makes sense where you want to do things like prefetch. But for normal files it forces the browser to buffer the entire file in ram (bad idea). A normal download streams the file

stufisher commented 1 year ago

Out of curiosity, what is the use case for the onetime single token?

Pretty much any time you want to download a file (that isn't an image where you might use an XHR request), say an mtz might be 50-100mb. When the browser downloads a file you can't send an authorisation header so you need to do something else. You don't want to append the JWT in the query params because this has relatively long validity and if stolen can be used to impersonate the user. These tokens can only be used once and expire on a short time scale if not used (10s, could be shorter)

mgaonach commented 1 year ago

Okay, then it makes sense to have both possibilities. But we should have some documentation page about it, stating what onetime tokens should and should not be used for.

codecov-commenter commented 1 year ago

Codecov Report

Merging #214 (3764ee9) into beamline-groups-permissions (8acad28) will increase coverage by 0.05%. The diff coverage is 79.26%.

@@                       Coverage Diff                       @@
##           beamline-groups-permissions     #214      +/-   ##
===============================================================
+ Coverage                        75.26%   75.31%   +0.05%     
===============================================================
  Files                               80       81       +1     
  Lines                             2903     2974      +71     
===============================================================
+ Hits                              2185     2240      +55     
- Misses                             718      734      +16     
Impacted Files Coverage Δ
pyispyb/app/extensions/auth/onetime.py 70.37% <70.37%> (ø)
pyispyb/app/main.py 75.00% <91.66%> (ø)
pyispyb/app/extensions/auth/bearer.py 90.24% <100.00%> (+3.75%) :arrow_up:
pyispyb/core/routes/user.py 77.41% <100.00%> (+9.23%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8acad28...3764ee9. Read the comment docs.