lona-web-org / lona

Write responsive web apps in full python
MIT License
527 stars 27 forks source link

setup buckets for file-uploads #503

Closed fscherf closed 9 months ago

fscherf commented 10 months ago

This PR adds a new subsystem called Buckets which can be used to upload files or to make files accessible via HTTP

codecov-commenter commented 10 months ago

Codecov Report

Attention: 59 lines in your changes are missing coverage. Please review.

Comparison is base (9181128) 73.04% compared to head (9361ac5) 73.19%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #503 +/- ## ========================================== + Coverage 73.04% 73.19% +0.14% ========================================== Files 85 88 +3 Lines 5869 6103 +234 Branches 1275 1326 +51 ========================================== + Hits 4287 4467 +180 - Misses 1314 1355 +41 - Partials 268 281 +13 ``` | [Files](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org) | Coverage Δ | | |---|---|---| | [lona/\_\_init\_\_.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | | | [lona/default\_settings.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS9kZWZhdWx0X3NldHRpbmdzLnB5) | `100.00% <100.00%> (ø)` | | | [lona/html/mixins.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS9odG1sL21peGlucy5weQ==) | `100.00% <100.00%> (ø)` | | | [lona/html/nodes/raw\_nodes.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS9odG1sL25vZGVzL3Jhd19ub2Rlcy5weQ==) | `100.00% <100.00%> (ø)` | | | [lona/middleware\_controller.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS9taWRkbGV3YXJlX2NvbnRyb2xsZXIucHk=) | `82.29% <100.00%> (+1.83%)` | :arrow_up: | | [lona/request.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS9yZXF1ZXN0LnB5) | `81.08% <100.00%> (+2.95%)` | :arrow_up: | | [lona/unique\_ids.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS91bmlxdWVfaWRzLnB5) | `100.00% <100.00%> (ø)` | | | [lona/server.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS9zZXJ2ZXIucHk=) | `71.19% <66.66%> (+0.76%)` | :arrow_up: | | [lona/view\_runtime.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS92aWV3X3J1bnRpbWUucHk=) | `81.86% <77.77%> (-0.44%)` | :arrow_down: | | [lona/buckets.py](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org#diff-bG9uYS9idWNrZXRzLnB5) | `86.03% <86.03%> (ø)` | | | ... and [1 more](https://app.codecov.io/gh/lona-web-org/lona/pull/503?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org) | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/lona-web-org/lona/pull/503/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lona-web-org)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fscherf commented 9 months ago

Hi @SmithChart,

nice! Thanks for testing and reviewing! :)

Both is already implemented. Buckets have the arguments max_size, which limits the size in bytes in a bucket, max_files which limits the amount of files in a bucket (can be used together), and index which enables, or disables the HTTP/HTML management interface.

lona-dropzone supports all of these arguments, and you can feed an pre-configured bucket in it.

https://github.com/lona-web-org/lona/pull/503/commits/9361ac5a2a94a530a0918b528294d260c06d55e6#diff-e6778aa47b2825caf9510d3808762cea575cd372d680e4437970f925c284e35cR62

SmithChart commented 9 months ago

Both is already implemented. Buckets have the arguments max_size, which limits the size in bytes in a bucket, max_files which limits the amount of files in a bucket (can be used together), and index which enables, or disables the HTTP/HTML management interface.

lona-dropzone supports all of these arguments, and you can feed an pre-configured bucket in it.

That's an option. But consider the following situation: Let's assume we are working on a project that is, later on, intended to be public-facing (without any authentication). For development I would like to have all buckets in debug-mode. But for production I would like to switch these features off. If this has a preset in the settings I could just add one setting to get debugging on or off. Otherwise I would need to integrate such setting into every bucket.

And: If the setting would default to "off", this would be a bit more secure by default.

fscherf commented 9 months ago

I thought about that too and I am still not sure about the security aspect of the index page. On the one hand, I agree, it feels more "secure" not to have that on in production, but on the other hand, it does not allow you anything you can't do using curl or wget. The file and size limits also apply to the index page.

fscherf commented 9 months ago

@SmithChart I thought a bit more about the security aspect and rechecked the code. I don't think that the index page is a security concern. It lets you only do what you also could do with curl. The URL, or the token in the URL rather, is the short-lived, shared secret between the view, the user, and the middleware. When you have the token, you can upload files. It is not more or less secure than the session tokens.

SmithChart commented 9 months ago

Totally right. A user could extract the token by hand and do the same. And: Since the secret is only shared with the user / client, we do not really need to take care of checking a login here, or do we?

fscherf commented 9 months ago

@SmithChart: I don't think so. I think it is reasonably secure like this

SmithChart commented 9 months ago

OK. From my side: Feel free to merge this :-)

fscherf commented 9 months ago

@SmithChart: Great! Thanks for your help and patience :)