dec0dOS / zero-ui

ZeroUI - ZeroTier Controller Web UI - is a web user interface for a self-hosted ZeroTier network controller.
GNU General Public License v3.0
946 stars 152 forks source link

feat: add file download support #101

Closed wongsyrone closed 2 years ago

wongsyrone commented 2 years ago

feat: backend: new api to download file such as planet

/PROJECT/frontend/down_folder/ http://locahost:4000/downfile/:downfilename

feat: frontend: add file download component for ease of use

Signed-off-by: Syrone Wong wong.syrone@gmail.com

Pull Request type

Please check the type of change your PR introduces:

What is the current behavior?

No download support

What is the new behavior?

Does this introduce a breaking change?

Other information

Testing docker image: https://hub.docker.com/r/wongsyrone/ztcontrollerzerouiwithplanetpatch

Example Usage:

docker run --rm -d -ti --name myztcontroller -v /root/zt-controller-docker/identity.public:/app/config/identity.public -v /root/zt-controller-docker/identity.secret:/app/config/identity.secret -v /root/zt-controller-docker/authtoken.secret:/app/config/authtoken.secret -v /root/zt-controller-docker/planet:/app/config/planet -v/root/zt-controller-docker/controller.d:/app/config/controller.d -v/root/zt-controller-docker/zero-ui-db.json:/app/backend/data/db.json -e ZU_SECURE_HEADERS=false -e ZT_PRIMARY_PORT=9993 -e ZU_CONTROLLER_ENDPOINT=http://127.0.0.1:9993/ -e ZU_DEFAULT_USERNAME=admin -e ZU_DEFAULT_PASSWORD=zero-ui -p 3000:3000 -p 4000:4000 -p 9993:9993 -p 9993:9993/udp wongsyrone/ztcontrollerzerouiwithplanetpatch:latest
lgtm-com[bot] commented 2 years ago

This pull request introduces 2 alerts when merging 17bf258ea1fe986bdcd9accba23830832cc8a6e2 into e2c651ad0579f580686dfee4c0c2007d88a3b336 - view on LGTM.com

new alerts:

dec0dOS commented 2 years ago

Hello, @wongsyrone!

I don't think that such a feature should be included at the application level. Serving user-specific files should be made at the web-server level. Setting up caddy/nginx/apache is way better solution for downloading user-specified files.

wongsyrone commented 2 years ago

@dec0dOS

Setting web servers is not out of the box and it's not easy to ensure access control.

dec0dOS commented 2 years ago

Thanks for clarifying, @wongsyrone.

Anyway, I could not accept the PR in its current state. Such functionality should be hidden under the ENV flag properly and the explanation with the use cases of this feature. Could you provide a detailed way on how you use this feature and why?

wongsyrone commented 2 years ago

I'm not familiar with the Node.js tech stack, it's the best I can do. If you cannot merge, I'll keep this as my fork.

lgtm-com[bot] commented 2 years ago

This pull request introduces 2 alerts when merging 2d4ef28d26566e3b7e1fd4c8b72dfcb1ace1854a into e2c651ad0579f580686dfee4c0c2007d88a3b336 - view on LGTM.com

new alerts: