search

Scout24 / c-bastion

Jump-host functionality, in the cloud.
6 stars 3 forks source link

delete user should also delete screen session infos #10

Open schlomo opened 8 years ago

schlomo commented 8 years ago

Steps to reproduce:

Using screen again does not work and fails with the following error message:

sschapiro@cbastion-eu-west-1.x.y.z:~$ screen
You are not the owner of /var/run/screen/S-sschapiro.
sschapiro@cbastion-eu-west-1.x.y.z:~$ ll /var/run/screen/S-sschapiro
ls: cannot open directory /var/run/screen/S-sschapiro: Permission denied
sschapiro@cbastion-eu-west-1.x.y.z:~$ ll -d /var/run/screen/S-sschapiro
drwx------ 2 1000 1000 4096 Mar 23 09:43 /var/run/screen/S-sschapiro/

Suggested Fix

Deleting a user should also remove all his files, especially in /var. That will also remove user cron jobs and other things that a user might leave behind.

esc commented 8 years ago

I am not even sure anymore if "delete" is even needed. If "upload" were to be a) idempotent and b) would simply concatenate the uploaded key to the authorized_keys file, this would be a non-issue.

esc commented 8 years ago

I mean, the only reason we ever added delete was so that you could re-upload your key, in case you uploaded the wrong one.

schlomo commented 8 years ago

IMHO upload should replace the existing key (who knows, maybe I upload again because my key got compromised). Besides that fine with me.

On 28 April 2016 at 09:00, Valentin Haenel notifications@github.com wrote:

I mean, the only reason we ever added delete was so that you could re-upload your key, in case you uploaded the wrong one.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/ImmobilienScout24/c-bastion/issues/10#issuecomment-215328971

esc commented 8 years ago

So, there is some more code in #17 that should simplify things, the next step would be to delete the delete operation. Note: create is now such that you can upload a new key to replace the old one.