Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add onetime tokens #214

Merged
merged 7 commits into from
Nov 29, 2022
Merged

Add onetime tokens #214

merged 7 commits into from
Nov 29, 2022

Conversation

stufisher
Copy link
Collaborator

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 #203

@stufisher stufisher requested a review from mgaonach September 27, 2022 13:56
@mgaonach
Copy link
Collaborator

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
Copy link
Collaborator

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

@stufisher
Copy link
Collaborator Author

stufisher commented Sep 28, 2022

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link

codecov-commenter commented Sep 30, 2022

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%) ⬆️
pyispyb/core/routes/user.py 77.41% <100.00%> (+9.23%) ⬆️

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.

@stufisher stufisher force-pushed the beamline-groups-permissions branch from 4d8e16c to cdb03e7 Compare October 26, 2022 06:56
Base automatically changed from beamline-groups-permissions to master November 3, 2022 09:02
@mgaonach mgaonach merged commit e1ce7de into master Nov 29, 2022
@mgaonach mgaonach deleted the onetime-tokens branch November 29, 2022 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants