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

Guest Upload V2 #178

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Guest Upload V2 #178

wants to merge 14 commits into from

Conversation

Kwonunn
Copy link
Contributor

@Kwonunn Kwonunn commented Jun 15, 2024

hehe sorry for the long wait :3

i've restarted development on 1.8.4, will rebase onto master when the PR is finished.

i've rewritten the entire feature from scratch and i'm taking more of a minimum viable product approach this time. first version is going to be quite limited, but that should make adding more features later a lot easier.

right now I still have to do the following things:

  • un-break e2e encryption (might need some help with this but will ask)
  • do a good security check to see if i haven't made any holes
  • delete the token once it's been successfully used
  • make a nicer result page when the upload succeeded

@Forceu
Copy link
Owner

Forceu commented Jun 16, 2024

Thanks a lot for your hard work! Let me know when it is ready for review or if you need help!

@Kwonunn
Copy link
Contributor Author

Kwonunn commented Jun 16, 2024

after reading some more about how it works, i think i'm not going to do e2e encryption support for guest uploads (at least for now). it would require sending the e2e encryption key to anyone with a guest upload token which seems really insecure and a bad idea.

right now, if e2e encryption is enabled, guest uploads work just fine but won't be e2e encrypted. i think this is pretty good behaviour, but some warnings that guest uploads aren't encrypted in the docs and setup would be nice i think.

@Forceu
Copy link
Owner

Forceu commented Jun 16, 2024

In theory it would be possible to create a new key for each upload request, but to be honest I am not sure if it would be worth it. Maybe it can be implemented at some point in the future, but I don't see it as a high priority either.

@Kwonunn
Copy link
Contributor Author

Kwonunn commented Jun 16, 2024

oki, i've finished everything for the first version

  • E2E is now disabled for guest uploads, there's notices about this in the setup and the docs
  • guest tokens are now deleted after they've been used
  • after uploading the link is shown in a nicer interface

i'd like to submit the code for review now, but i would also appreciate a little help with making the guest upload result page look a little nicer. i'm not good with bootstrap/frontend so i just slapped some stuff together.

@Kwonunn Kwonunn changed the title WIP: Guest Upload V2 Guest Upload V2 Jun 16, 2024
@Forceu
Copy link
Owner

Forceu commented Jun 16, 2024

Thanks, I might have some time to review it next week. Would you mind resolving the merge conflicts as well? That would make it a little bit easier

@Kwonunn
Copy link
Contributor Author

Kwonunn commented Jun 16, 2024

finished the rebase, take your time with the review!

@Forceu
Copy link
Owner

Forceu commented Jun 19, 2024

Would you mind giving me write access to the branch? I would like to add a function for upgrading the database, otherwise the server crashes if started with an already existing configuration

@Kwonunn
Copy link
Contributor Author

Kwonunn commented Jun 19, 2024

Done!

@Forceu
Copy link
Owner

Forceu commented Jun 19, 2024

Thank you. I am not sure if I can review the rest this week, I will probably have more time next week. Also I think it would be a better idea, if the uploaded files are listed in the upload request tab, instead of the main tab, so no confusion exists. In addition it is probably a good idea if multiple files can be uploaded at once and then are grouped together. I will look into it as well however.

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.

None yet

2 participants