From 56ee1ba4a382b14903c52c4a690aec0099458cf6 Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 2 Oct 2014 15:58:27 +0000 Subject: [PATCH] systemusers functional tests passing --- orchestra/apps/accounts/admin.py | 1 + orchestra/apps/accounts/forms.py | 10 +- orchestra/apps/orchestration/manager.py | 5 +- orchestra/apps/orchestration/methods.py | 6 +- orchestra/apps/systemusers/backends.py | 11 +- orchestra/apps/systemusers/forms.py | 12 +- orchestra/apps/systemusers/models.py | 23 +- orchestra/apps/systemusers/settings.py | 6 +- .../tests/functional_tests/tests.py | 198 ++++++++++++++++-- orchestra/utils/system.py | 7 +- orchestra/utils/tests.py | 13 +- scripts/services/rssh.md | 17 ++ scripts/services/vsftpd.md | 5 +- 13 files changed, 258 insertions(+), 56 deletions(-) create mode 100644 scripts/services/rssh.md diff --git a/orchestra/apps/accounts/admin.py b/orchestra/apps/accounts/admin.py index 2960fd33..665fa1fa 100644 --- a/orchestra/apps/accounts/admin.py +++ b/orchestra/apps/accounts/admin.py @@ -52,6 +52,7 @@ class AccountAdmin(auth.UserAdmin, ExtendedModelAdmin): add_form = AccountCreationForm form = AccountChangeForm filter_horizontal = () + change_readonly_fields = ('username',) change_form_template = 'admin/accounts/account/change_form.html' def formfield_for_dbfield(self, db_field, **kwargs): diff --git a/orchestra/apps/accounts/forms.py b/orchestra/apps/accounts/forms.py index 126f44e4..24005b7d 100644 --- a/orchestra/apps/accounts/forms.py +++ b/orchestra/apps/accounts/forms.py @@ -24,21 +24,13 @@ class AccountCreationForm(auth.forms.UserCreationForm): class AccountChangeForm(forms.ModelForm): - username = forms.CharField(required=False) password = auth.forms.ReadOnlyPasswordHashField(label=_("Password"), help_text=_("Raw passwords are not stored, so there is no way to see " "this user's password, but you can change the password " "using this form.")) - def __init__(self, *args, **kwargs): - super(AccountChangeForm, self).__init__(*args, **kwargs) - account = kwargs.get('instance') - username = '%s' % account.username - self.fields['username'].widget = ReadOnlyWidget(username) - self.fields['password'].initial = account.password - def clean_password(self): # Regardless of what the user provides, return the initial value. # This is done here, rather than on the field, because the # field does not have access to the initial value - return self.fields['password'].initial + return self.initial["password"] diff --git a/orchestra/apps/orchestration/manager.py b/orchestra/apps/orchestration/manager.py index 1dec0709..80b5254e 100644 --- a/orchestra/apps/orchestration/manager.py +++ b/orchestra/apps/orchestration/manager.py @@ -41,7 +41,7 @@ def execute(operations): scripts = {} cache = {} for operation in operations: - logger.info("Queued %s" % str(operation)) + logger.debug("Queued %s" % str(operation)) servers = router.get_servers(operation, cache=cache) for server in servers: key = (server, operation.backend) @@ -72,6 +72,7 @@ def execute(operations): logger.info("Executed %s" % str(operation)) operation.log = execution.log operation.save() - logger.info(execution.log.stderr) + logger.debug(execution.log.stdout) + logger.debug(execution.log.stderr) logs.append(execution.log) return logs diff --git a/orchestra/apps/orchestration/methods.py b/orchestra/apps/orchestration/methods.py index d9dead2b..056c7570 100644 --- a/orchestra/apps/orchestration/methods.py +++ b/orchestra/apps/orchestration/methods.py @@ -37,8 +37,6 @@ def BashSSH(backend, log, server, cmds): log.save(update_fields=['state']) return transport = ssh.get_transport() - channel = transport.open_session() - sftp = paramiko.SFTPClient.from_transport(transport) sftp.put(path, "%s.remote" % path) sftp.close() @@ -51,9 +49,11 @@ def BashSSH(backend, log, server, cmds): cmd = ( "[[ $(md5sum %(path)s|awk {'print $1'}) == %(digest)s ]] && bash %(path)s\n" "RETURN_CODE=$?\n" -# "rm -fr %(path)s\n" +# TODO "rm -fr %(path)s\n" "exit $RETURN_CODE" % context ) + + channel = transport.open_session() channel.exec_command(cmd) if True: # TODO if not async log.stdout += channel.makefile('rb', -1).read().decode('utf-8') diff --git a/orchestra/apps/systemusers/backends.py b/orchestra/apps/systemusers/backends.py index 276b36e7..633c55fc 100644 --- a/orchestra/apps/systemusers/backends.py +++ b/orchestra/apps/systemusers/backends.py @@ -22,7 +22,7 @@ class SystemUserBackend(ServiceController): if [[ $( id %(username)s ) ]]; then usermod %(username)s --password '%(password)s' --shell %(shell)s %(groups_arg)s else - useradd %(username)s --password '%(password)s' --shell %(shell)s %(groups_arg)s + useradd %(username)s --home %(home)s --password '%(password)s' --shell %(shell)s %(groups_arg)s usermod -a -G %(username)s %(mainusername)s fi mkdir -p %(home)s @@ -35,10 +35,14 @@ class SystemUserBackend(ServiceController): self.append("killall -u %(username)s || true" % context) self.append("userdel %(username)s || true" % context) self.append("groupdel %(username)s || true" % context) + if user.is_main: + # TODO delete instead of this shit + context['deleted'] = context['home'][:-1]+'.deleted' + self.append("mv %(home)s %(deleted)s" % context) def get_groups(self, user): if user.is_main: - return user.account.systemusers.exclude(id=user.id).values_list('username', flat=True) + return user.account.systemusers.exclude(username=user.username).values_list('username', flat=True) groups = list(user.groups.values_list('username', flat=True)) return groups @@ -48,9 +52,8 @@ class SystemUserBackend(ServiceController): 'password': user.password if user.active else '*%s' % user.password, 'shell': user.shell, 'mainusername': user.username if user.is_main else user.account.username, + 'home': user.get_home() } - basehome = settings.SYSTEMUSERS_HOME % context - context['home'] = os.path.join(basehome, user.home) return context diff --git a/orchestra/apps/systemusers/forms.py b/orchestra/apps/systemusers/forms.py index a18bacf2..0ffd8dce 100644 --- a/orchestra/apps/systemusers/forms.py +++ b/orchestra/apps/systemusers/forms.py @@ -5,6 +5,8 @@ from django.utils.translation import ugettext, ugettext_lazy as _ from orchestra.apps.accounts.models import Account from orchestra.core.validators import validate_password +from .models import SystemUser + # TODO orchestra.UserCretionForm class UserCreationForm(auth.forms.UserCreationForm): @@ -16,10 +18,12 @@ class UserCreationForm(auth.forms.UserCreationForm): # Since model.clean() will check this, this is redundant, # but it sets a nicer error message than the ORM and avoids conflicts with contrib.auth username = self.cleaned_data["username"] - account_model = self._meta.model.account.field.rel.to - if account_model.objects.filter(username=username).exists(): - raise forms.ValidationError(self.error_messages['duplicate_username']) - return username + try: + SystemUser._default_manager.get(username=username) + except SystemUser.DoesNotExist: + return username + raise forms.ValidationError(self.error_messages['duplicate_username']) + # TODO orchestra.UserCretionForm diff --git a/orchestra/apps/systemusers/models.py b/orchestra/apps/systemusers/models.py index ba31fef9..e43e68d0 100644 --- a/orchestra/apps/systemusers/models.py +++ b/orchestra/apps/systemusers/models.py @@ -1,3 +1,5 @@ +import os + from django.contrib.auth.hashers import make_password from django.core import validators from django.core.mail import send_mail @@ -49,7 +51,26 @@ class SystemUser(models.Model): @cached_property def active(self): - return self.is_active and self.account.is_active + a = type(self).account.field.model + try: + return self.is_active and self.account.is_active + except type(self).account.field.rel.to.DoesNotExist: + return self.is_active + + def get_home(self): + if self.is_main: + context = { + 'username': self.username, + } + basehome = settings.SYSTEMUSERS_HOME % context + else: + basehome = self.account.systemusers.get(is_main=True).get_home() + basehome = basehome.replace('/./', '/') + home = os.path.join(basehome, self.home) + # Chrooting + home = home.split('/') + home.insert(-2, '.') + return '/'.join(home) ## TODO user deletion and group handling. diff --git a/orchestra/apps/systemusers/settings.py b/orchestra/apps/systemusers/settings.py index 723c61ae..eac6fb4f 100644 --- a/orchestra/apps/systemusers/settings.py +++ b/orchestra/apps/systemusers/settings.py @@ -4,13 +4,13 @@ from django.utils.translation import ugettext, ugettext_lazy as _ SYSTEMUSERS_SHELLS = getattr(settings, 'SYSTEMUSERS_SHELLS', ( - ('/bin/false', _("No shell, FTP only")), - ('/bin/rsync', _("No shell, SFTP/RSYNC only")), + ('/dev/null', _("No shell, FTP only")), + ('/bin/rssh', _("No shell, SFTP/RSYNC only")), ('/bin/bash', "/bin/bash"), ('/bin/sh', "/bin/sh"), )) -SYSTEMUSERS_DEFAULT_SHELL = getattr(settings, 'SYSTEMUSERS_DEFAULT_SHELL', '/bin/false') +SYSTEMUSERS_DEFAULT_SHELL = getattr(settings, 'SYSTEMUSERS_DEFAULT_SHELL', '/dev/null') SYSTEMUSERS_HOME = getattr(settings, 'SYSTEMUSERS_HOME', '/home/%(username)s') diff --git a/orchestra/apps/systemusers/tests/functional_tests/tests.py b/orchestra/apps/systemusers/tests/functional_tests/tests.py index 6d76e90c..0e8b7888 100644 --- a/orchestra/apps/systemusers/tests/functional_tests/tests.py +++ b/orchestra/apps/systemusers/tests/functional_tests/tests.py @@ -1,6 +1,10 @@ +import ftplib +import re from functools import partial -from django.conf import settings +import paramiko +from django.conf import settings as djsettings +from django.core.management.base import CommandError from django.core.urlresolvers import reverse from selenium.webdriver.support.select import Select @@ -9,7 +13,7 @@ from orchestra.apps.orchestration.models import Server, Route from orchestra.utils.system import run from orchestra.utils.tests import BaseLiveServerTestCase, random_ascii -from ... import backends +from ... import backends, settings from ...models import SystemUser @@ -27,12 +31,16 @@ class SystemUserMixin(object): def setUp(self): super(SystemUserMixin, self).setUp() self.add_route() + djsettings.DEBUG = True def add_route(self): master = Server.objects.create(name=self.MASTER_ADDR) backend = backends.SystemUserBackend.get_name() Route.objects.create(backend=backend, match=True, host=master) + def save(self): + raise NotImplementedError + def add(self): raise NotImplementedError @@ -45,54 +53,154 @@ class SystemUserMixin(object): def disable(self): raise NotImplementedError + def add_group(self, username, groupname): + raise NotImplementedError + + def validate_user(self, username): + idcmd = r("id %s" % username) + self.assertEqual(0, idcmd.return_code) + user = SystemUser.objects.get(username=username) + groups = list(user.groups.values_list('username', flat=True)) + groups.append(user.username) + idgroups = idcmd.stdout.strip().split(' ')[2] + idgroups = re.findall(r'\d+\((\w+)\)', idgroups) + self.assertEqual(set(groups), set(idgroups)) + + def validate_delete(self, username): + self.assertRaises(SystemUser.DoesNotExist, SystemUser.objects.get, username=username) + self.assertRaises(CommandError, run, 'id %s' % username, display=False) + self.assertRaises(CommandError, run, 'grep "^%s:" /etc/groups' % username, display=False) + self.assertRaises(CommandError, run, 'grep "^%s:" /etc/passwd' % username, display=False) + self.assertRaises(CommandError, run, 'grep "^%s:" /etc/shadow' % username, display=False) + + def validate_ftp(self, username, password): + connection = ftplib.FTP(self.MASTER_ADDR) + connection.login(user=username, passwd=password) + connection.close() + + def validate_sftp(self, username, password): + transport = paramiko.Transport((self.MASTER_ADDR, 22)) + transport.connect(username=username, password=password) + sftp = paramiko.SFTPClient.from_transport(transport) + sftp.listdir() + sftp.close() + + def validate_ssh(self, username, password): + ssh = paramiko.SSHClient() + ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) + ssh.connect(self.MASTER_ADDR, username=username, password=password) + transport = ssh.get_transport() + channel = transport.open_session() + channel.exec_command('ls') + self.assertEqual(0, channel.recv_exit_status()) + channel.close() + def test_create_systemuser(self): username = '%s_systemuser' % random_ascii(10) password = '@!?%spppP001' % random_ascii(5) self.add(username, password) self.addCleanup(partial(self.delete, username)) - self.assertEqual(0, r("id %s" % username).return_code) - # TODO test group membership and everything + self.validate_user(username) + + def test_ftp(self): + username = '%s_systemuser' % random_ascii(10) + password = '@!?%spppP001' % random_ascii(5) + self.add(username, password, shell='/dev/null') + self.addCleanup(partial(self.delete, username)) + self.assertRaises(paramiko.AuthenticationException, self.validate_sftp, username, password) + self.assertRaises(paramiko.AuthenticationException, self.validate_ssh, username, password) + + def test_sftp(self): + username = '%s_systemuser' % random_ascii(10) + password = '@!?%spppP001' % random_ascii(5) + self.add(username, password, shell='/bin/rssh') + self.addCleanup(partial(self.delete, username)) + self.validate_sftp(username, password) + self.assertRaises(AssertionError, self.validate_ssh, username, password) + + def test_ssh(self): + username = '%s_systemuser' % random_ascii(10) + password = '@!?%spppP001' % random_ascii(5) + self.add(username, password, shell='/bin/bash') + self.addCleanup(partial(self.delete, username)) + self.validate_ssh(username, password) def test_delete_systemuser(self): username = '%s_systemuser' % random_ascii(10) password = '@!?%sppppP001' % random_ascii(5) self.add(username, password) - self.assertEqual(0, r("id %s" % username).return_code) + self.validate_user(username) self.delete(username) - self.assertEqual(1, r("id %s" % username, error_codes=[0,1]).return_code) + self.validate_delete(username) - def test_update_systemuser(self): - pass - # TODO + def test_add_group_systemuser(self): + username = '%s_systemuser' % random_ascii(10) + password = '@!?%spppP001' % random_ascii(5) + self.add(username, password) + self.addCleanup(partial(self.delete, username)) + self.validate_user(username) + username2 = '%s_systemuser' % random_ascii(10) + password2 = '@!?%spppP001' % random_ascii(5) + self.add(username2, password2) + self.addCleanup(partial(self.delete, username2)) + self.validate_user(username2) + self.add_group(username, username2) + user = SystemUser.objects.get(username=username) + groups = list(user.groups.values_list('username', flat=True)) + self.assertIn(username2, groups) + self.validate_user(username) def test_disable_systemuser(self): - pass - # TODO + username = '%s_systemuser' % random_ascii(10) + password = '@!?%spppP001' % random_ascii(5) + self.add(username, password, shell='/dev/null') + self.addCleanup(partial(self.delete, username)) + self.validate_ftp(username, password) + self.disable(username) + self.validate_user(username) + self.assertRaises(ftplib.error_perm, self.validate_ftp, username, password) -# TODO test with ftp and ssh clients? class RESTSystemUserMixin(SystemUserMixin): def setUp(self): super(RESTSystemUserMixin, self).setUp() self.rest_login() + # create main user + self.save(self.account.username) + self.addCleanup(partial(self.delete, self.account.username)) - def add(self, username, password): - self.rest.systemusers.create(username=username, password=password) + def add(self, username, password, shell='/dev/null'): + self.rest.systemusers.create(username=username, password=password, shell=shell) def delete(self, username): user = self.rest.systemusers.retrieve(username=username).get() user.delete() - def update(self): - pass + def add_group(self, username, groupname): + user = self.rest.systemusers.retrieve(username=username).get() + group = self.rest.systemusers.retrieve(username=groupname).get() + user.groups.append(group) # TODO how to do it with the api? + user.save() + + def disable(self, username): + user = self.rest.systemusers.retrieve(username=username).get() + user.is_active = False + user.save() + + def save(self, username): + user = self.rest.systemusers.retrieve(username=username).get() + user.save() class AdminSystemUserMixin(SystemUserMixin): def setUp(self): super(AdminSystemUserMixin, self).setUp() self.admin_login() + # create main user + self.save(self.account.username) + self.addCleanup(partial(self.delete, self.account.username)) - def add(self, username, password): + def add(self, username, password, shell='/dev/null'): url = self.live_server_url + reverse('admin:systemusers_systemuser_add') self.selenium.get(url) @@ -108,21 +216,52 @@ class AdminSystemUserMixin(SystemUserMixin): account_select = Select(account_input) account_select.select_by_value(str(self.account.pk)) + shell_input = self.selenium.find_element_by_id('id_shell') + shell_select = Select(shell_input) + shell_select.select_by_value(shell) + username_field.submit() self.assertNotEqual(url, self.selenium.current_url) def delete(self, username): user = SystemUser.objects.get(username=username) - url = self.live_server_url + reverse('admin:systemusers_systemuser_delete', args=(user.pk,)) + delete = reverse('admin:systemusers_systemuser_delete', args=(user.pk,)) + url = self.live_server_url + delete self.selenium.get(url) confirmation = self.selenium.find_element_by_name('post') confirmation.submit() + self.assertNotEqual(url, self.selenium.current_url) def disable(self, username): - pass + user = SystemUser.objects.get(username=username) + change = reverse('admin:systemusers_systemuser_change', args=(user.pk,)) + url = self.live_server_url + change + self.selenium.get(url) + is_active = self.selenium.find_element_by_id('id_is_active') + is_active.click() + save = self.selenium.find_element_by_name('_save') + save.submit() + self.assertNotEqual(url, self.selenium.current_url) - def update(self): - pass + def add_group(self, username, groupname): + user = SystemUser.objects.get(username=username) + change = reverse('admin:systemusers_systemuser_change', args=(user.pk,)) + url = self.live_server_url + change + self.selenium.get(url) + groups = self.selenium.find_element_by_id('id_groups_add_all_link') + groups.click() + save = self.selenium.find_element_by_name('_save') + save.submit() + self.assertNotEqual(url, self.selenium.current_url) + + def save(self, username): + user = SystemUser.objects.get(username=username) + change = reverse('admin:systemusers_systemuser_change', args=(user.pk,)) + url = self.live_server_url + change + self.selenium.get(url) + save = self.selenium.find_element_by_name('_save') + save.submit() + self.assertNotEqual(url, self.selenium.current_url) class RESTSystemUserTest(RESTSystemUserMixin, BaseLiveServerTestCase): @@ -160,3 +299,20 @@ class AdminSystemUserTest(AdminSystemUserMixin, BaseLiveServerTestCase): self.addCleanup(account.delete) self.assertNotEqual(url, self.selenium.current_url) self.assertEqual(0, r("id %s" % account.username).return_code) + + def test_delete_account(self): + home = self.account.systemusers.get(is_main=True).get_home() + + delete = reverse('admin:accounts_account_delete', args=(self.account.pk,)) + url = self.live_server_url + delete + self.selenium.get(url) + confirmation = self.selenium.find_element_by_name('post') + confirmation.submit() + self.assertNotEqual(url, self.selenium.current_url) + + self.assertRaises(CommandError, run, 'ls %s' % home, display=False) + + # Recreate a fucking fake account for test cleanup + self.account = self.create_account(username=self.account.username, superuser=True) + self.selenium.delete_all_cookies() + self.admin_login() diff --git a/orchestra/utils/system.py b/orchestra/utils/system.py index 64ad2176..121a5fa9 100644 --- a/orchestra/utils/system.py +++ b/orchestra/utils/system.py @@ -46,7 +46,7 @@ def read_async(fd): return '' -def run(command, display=True, error_codes=[0], silent=True, stdin=''): +def run(command, display=True, error_codes=[0], silent=False, stdin=''): """ Subprocess wrapper for running commands """ if display: sys.stderr.write("\n\033[1m $ %s\033[0m\n" % command) @@ -96,9 +96,10 @@ def run(command, display=True, error_codes=[0], silent=True, stdin=''): out.failed = True msg = "\nrun() encountered an error (return code %s) while executing '%s'\n" msg = msg % (p.returncode, command) - sys.stderr.write("\n\033[1;31mCommandError: %s %s\033[m\n" % (msg, err)) + if display: + sys.stderr.write("\n\033[1;31mCommandError: %s %s\033[m\n" % (msg, err)) if not silent: - raise CommandError("\n%s\n %s\n" % (msg, err)) + raise CommandError("%s %s" % (msg, err)) out.succeeded = not out.failed return out diff --git a/orchestra/utils/tests.py b/orchestra/utils/tests.py index f0093339..64060a88 100644 --- a/orchestra/utils/tests.py +++ b/orchestra/utils/tests.py @@ -53,8 +53,9 @@ class AppDependencyMixin(object): class BaseTestCase(TestCase, AppDependencyMixin): - def create_account(self, superuser=False): - username = '%s_superaccount' % random_ascii(5) + def create_account(self, username='', superuser=False): + if not username: + username = '%s_superaccount' % random_ascii(5) password = 'orchestra' if superuser: return Account.objects.create_superuser(username, password=password, email='orchestra@orchestra.org') @@ -75,9 +76,11 @@ class BaseLiveServerTestCase(AppDependencyMixin, LiveServerTestCase): cls.vdisplay.stop() super(BaseLiveServerTestCase, cls).tearDownClass() - def create_account(self, superuser=False): - username = '%s_superaccount' % random_ascii(5) + def create_account(self, username='', superuser=False): + if not username: + username = '%s_superaccount' % random_ascii(5) password = 'orchestra' + self.account_password = password if superuser: return Account.objects.create_superuser(username, password=password, email='orchestra@orchestra.org') return Account.objects.create_user(username, password=password, email='orchestra@orchestra.org') @@ -101,4 +104,4 @@ class BaseLiveServerTestCase(AppDependencyMixin, LiveServerTestCase): )) def rest_login(self): - self.rest.login(username=self.ACCOUNT_USERNAME, password=self.ACCOUNT_PASSWORD) + self.rest.login(username=self.account.username, password=self.account_password) diff --git a/scripts/services/rssh.md b/scripts/services/rssh.md new file mode 100644 index 00000000..9cc2065c --- /dev/null +++ b/scripts/services/rssh.md @@ -0,0 +1,17 @@ +Restricted Shell for SCP and Rsync +================================== + +1. apt-get install rssh + +2. Enable desired programs + ```bash + sed -i "s/^#allowscp/allowscp/" /etc/rssh.conf + sed -i "s/^#allowrsync/allowrsync/" /etc/rssh.conf + sed -i "s/^#allowsftp/allowsftp/" /etc/rssh.conf + ``` + +2. Enable the shell + ```bash + ln -s /usr/local/bin/rssh /bin/rssh + echo /bin/rssh >> /etc/shells + ``` diff --git a/scripts/services/vsftpd.md b/scripts/services/vsftpd.md index 6bde4a11..3c3b0d6b 100644 --- a/scripts/services/vsftpd.md +++ b/scripts/services/vsftpd.md @@ -15,9 +15,12 @@ VsFTPd with System Users sed -i "s/#write_enable=YES/write_enable=YES" /etc/vsftpd.conf sed -i "s/#chroot_local_user=YES/chroot_local_user=YES/" /etc/vsftpd.conf - echo '/bin/false' >> /etc/shells + echo '/dev/null' >> /etc/shells ``` +# TODO https://www.benscobie.com/fixing-500-oops-vsftpd-refusing-to-run-with-writable-root-inside-chroot/#comment-2051 +Define option passwd_chroot_enable=yes in configuration file and change in /etc/passwd file user home directory from «/home/user» to «/home/./user» (w/o quotes). + 3. Apply changes ```bash