-
-
Notifications
You must be signed in to change notification settings - Fork 462
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
In app file transfer #221
In app file transfer #221
Conversation
Thanks! I'll take a look at this over the weekend. |
I am writing my comments here, if you would like to apply them, feel free. Alternatively I'll apply them over the weekend.
I would not allow admin to toggle filetransfer off or on. If I decide, I don't want to use filetransfer at all (because of security reason) in env, it should not be visible in neko at all. Admin could enable/disable it for users. For this I would reuse locks that we already have (for locking control/room). They would just lock file transfer. Since there will be multiple features implemented in the future, i am thinking about adding project-wide some soft of feature gate, that can include/exclude part of code as needed.
For this, I would suggest using filepath.Clean and similarly how container paths are implemented in neko-rooms. |
I agree with your thoughts. I can work on the necessary changes starting tomorrow. I'll be busy Sunday but if I don't finish tomorrow, I'll push what I have and either pick it up later or, if you'd like to work on it, I'll leave it up to you. |
I added error handling to transfers, made some lint fixes, moved
I listen only to
I added |
Looks like you beat me to it :) I like your changes. For the throttling of fsnotify write events, I can work on this some over the coming week or weekend. For now though, is that everything or are there other issues that needs resolving? |
No additional issues, this can be merged. Maybe there will be some smaller enhancements over time, but first version is good to go. Thanks for your contribution! |
Can this be used with google chrome docker image as well? Not sure if it will work, probably some policies would need to be changed in policy file. |
It is available for all neko images. But yes, to be able to read local files from google chrome or download them, policies needs to be setup. Maybe you want to AllowFileSelectionDialogs or set up DownloadRestrictions and you might want to have file:// accessbile (you can just add |
Do you mean by overwriting supervisord.conf for the --disable-file-system (same like with policies,json)? |
Ok so I did this in policy file:
It looks good, but the window where I need to browse and select files that I want to upload is behind Chrome window. So I need to double click on chrome to resize the window. It looks like Chrome window is set to be on top. I tested it on wetransfer.com site. |
In openbox.xml we need to target only main browser window, not popups. |
At the moment you cannot bring popup window to be on top at all, so you need to move main windows around to be able to select files for upload. In case of downloading, it is working fine. Very strange. |
Running Clicking on popup:
Clicking on browser:
Current match in openbox is But it still is paced under the brwoser window and you cannot bring it to foreground, even when removing whole openbox configuration. |
@alanmilinovic fixed in latest version. Tested on all browsers, only on opera it does not seem to work. They have maybe own file dialogs that are not compatible. |
Cool, tnx man. |
This adds uploads to and downloads from a specified directory when enabled by the server owner when launching the app or an admin in the app's settings.
Demo
NEKO_FILE_TRANSFER
env variable. An admin can toggle admin only file transfer in the app's settings panel.NEKO_UNPRIV_FILE_TRANSFER
env is also set, or if an admin toggles file transfer for all users. This setting has no effect if the setting above is not set.NEKO_FILE_TRANSFER_PATH
.Issues
I've tested this with large files locally (see demo) and smaller files across the Internet.