search

AsavarTzeth / docker-sftp

This project isn't being actively maintained. That also means it is not supported. It is provided for archival and academic purposes only and should never be used in a production environment.
BSD 2-Clause "Simplified" License
9 stars 10 forks source link

Removed duplicate username/uid check. #1

Closed scjudd closed 9 years ago

scjudd commented 9 years ago

This was preventing me from adding a user with UID 33. If I mount a volume that is being used by another container and all the files belong to 33, I want to be able to use this container to transfer files as user 33 (www-data).

I'm not sure if there is some issue with having duplicate UIDs in this context. I can't figure why the check was initially put in place.

AsavarTzeth commented 9 years ago

My reasoning for the duplicate sanity check is a user might set SFTP_USER or SFTP_UID, but not touching the other. The most common example I could come up with is if you add a second user, but SFTP_UID is left as default. Normally if you leave out the UID it would be random, but because of the way I scripted useradd it is technically mandatory.

This is not ideal, and I am considering a new user setup method, perhaps supporting multiple users at the same docker run.

As for putting the check there in the first place. If I remember correctly this initially became a necessity because I choose to make the UID easily customizable. If I were to remove that part from useradd the UID would be random and conflicts would not occur, but then you would not be able to use UID 33.

Thanks to you I do see the real issue though. You would use SFTP_USER=www-data and SFTP_UID=33, the script would skip user setup. You would then be unable to login because it also skipped adding your user to sftpusers. EDIT: You just realized how stupid I have been. The check is there for the addition of users, not everything else. I solving this almost the same way as you. useradd will now be the only line part of that if statement.

I will put this on my todo list and fix this when I have personally tested your setup.

I am not fully confident it is best practice to login as your webserver user, but I see how it solves some permission problems. Personally I solved this by using ACL:s and putting www-data as a default owner in the web root. It might look weird but this ensured users would not have to worry about the webserver lacking write access.

Lastly, I recommend you follow the beta branch if you are interested in knowing about future changes I might implement in master. At this time you might be interested to know I am considering adding the HPN-SSH performance patches.