From bd589b37cc676d01dec3fe1f9580bbe2ebda2567 Mon Sep 17 00:00:00 2001 From: fonfon Date: Fri, 11 Jul 2014 05:59:21 +0300 Subject: [PATCH 1/7] removed deprecated tests, and made the existing ones run again --- cfg.py | 1 + tests/actions_test.py | 38 +++++++----------- tests/auth_test.py | 67 ------------------------------- tests/user_store_test.py | 86 ---------------------------------------- 4 files changed, 16 insertions(+), 176 deletions(-) delete mode 100644 tests/auth_test.py delete mode 100644 tests/user_store_test.py diff --git a/cfg.py b/cfg.py index 2dacff987..6a705b42a 100644 --- a/cfg.py +++ b/cfg.py @@ -10,6 +10,7 @@ root = None file_root = None python_root = None data_dir = None +actions_dir = None store_file = None user_db = None status_log_file = None diff --git a/tests/actions_test.py b/tests/actions_test.py index 420ade662..0107eb58a 100644 --- a/tests/actions_test.py +++ b/tests/actions_test.py @@ -7,6 +7,11 @@ import shlex import subprocess import unittest +import cfg + +ROOT_DIR = os.path.split(os.path.abspath(os.path.split(__file__)[0]))[0] +cfg.actions_dir = os.path.join(ROOT_DIR, 'actions') + class TestPrivileged(unittest.TestCase): """Verify that privileged actions perform as expected. @@ -25,10 +30,9 @@ class TestPrivileged(unittest.TestCase): os.remove("actions/echo") os.remove("actions/id") - def test_run_as_root(self): - """1. Privileged actions run as root. - - """ + def notest_run_as_root(self): + """1. Privileged actions run as root. """ + # TODO: it's not allowed to call a symlink in the actions dir anymore self.assertEqual( "0", # user 0 is root superuser_run("id", "-ur")[0].strip()) @@ -75,45 +79,33 @@ class TestPrivileged(unittest.TestCase): for option in options: with self.assertRaises(ValueError): output = run(action, option) - - # if it somewhow doesn't error, we'd better not evaluate the - # data. + # if it somewhow doesn't error, we'd better not evaluate + # the data. self.assertFalse("2" in output[0]) def test_breakout_option_string(self): """3D. Option strings can't be used to run other actions. - Verify that shell control characters aren't interpreted. - """ action = "echo" # counting is safer than actual badness. options = "good; echo $((1+1))" - - output = run(action, options) - - self.assertFalse("2" in output) + self.assertRaises(ValueError, run, action, options) def test_breakout_option_list(self): """3D. Option lists can't be used to run other actions. - Verify that only a string of options is accepted and that we can't just tack additional shell control characters onto the list. - """ action = "echo" # counting is safer than actual badness. options = ["good", ";", "echo $((1+1))"] - - output = run(action, options) - # we'd better not evaluate the data. - self.assertFalse("2" in output) + self.assertRaises(ValueError, run, action, options) - def test_multiple_options(self): - """4. Multiple options can be provided as a list. - - """ + def notest_multiple_options(self): + """ 4. Multiple options can be provided as a list. """ + # TODO: it's not allowed to call a symlink in the actions dir anymore self.assertEqual( subprocess.check_output(shlex.split("id -ur")).strip(), run("id", ["-u" ,"-r"])[0].strip()) diff --git a/tests/auth_test.py b/tests/auth_test.py deleted file mode 100644 index 08806748b..000000000 --- a/tests/auth_test.py +++ /dev/null @@ -1,67 +0,0 @@ -#! /usr/bin/env python -# -*- mode: python; mode: auto-fill; fill-column: 80 -*- - -import auth -from logger import Logger -import cfg -import unittest -import cherrypy -import plugin_mount -import os -cfg.log = Logger() - -cherrypy.log.access_file = None - - -class Auth(unittest.TestCase): - """Test check_credentials function of auth to confirm it works as expected""" - - def setUp(self): - cfg.user_db = os.path.join(cfg.file_root, "tests/testdata/users"); - cfg.users = plugin_mount.UserStoreModule.get_plugins()[0] - - def tearDown(self): - for user in cfg.users.get_all(): - cfg.users.remove(user[0]) - cfg.users.close() - - def test_add_user(self): - self.assertIsNone(auth.add_user("test_user", "password")) - self.assertIsNotNone(auth.add_user(None, "password")) - self.assertIsNotNone(auth.add_user("test_user", None)) - self.assertIsNotNone(auth.add_user("test_user", "password")) - - def test_password_check(self): - auth.add_user("test_user", "password") - - # check_credentials returns None if there is no error, - # or returns error string - self.assertIsNone(auth.check_credentials("test_user", "password")) - self.assertIsNotNone(auth.check_credentials("test_user", "wrong")) - - def test_nonexistent_user(self): - self.assertIsNotNone(auth.check_credentials("test_user", "password")) - - def test_password_too_long(self): - password = "x" * 4097 - self.assertIsNotNone(auth.add_user("test_user", password)) - self.assertIsNotNone(auth.check_credentials("test_user", password)) - - def test_salt_is_random(self): - auth.add_user("test_user1", "password") - auth.add_user("test_user2", "password") - self.assertNotEqual( - cfg.users["test_user1"]["salt"], - cfg.users["test_user2"]["salt"] - ) - - def test_hash_is_random(self): - auth.add_user("test_user1", "password") - auth.add_user("test_user2", "password") - self.assertNotEqual( - cfg.users["test_user1"]["passphrase"], - cfg.users["test_user2"]["passphrase"] - ) - -if __name__ == "__main__": - unittest.main() diff --git a/tests/user_store_test.py b/tests/user_store_test.py deleted file mode 100644 index def0c0c93..000000000 --- a/tests/user_store_test.py +++ /dev/null @@ -1,86 +0,0 @@ -#! /usr/bin/env python -# -*- mode: python; mode: auto-fill; fill-column: 80 -*- - -from logger import Logger -import cfg -import unittest -import cherrypy -import plugin_mount -import os -from model import User -cfg.log = Logger() - -cherrypy.log.access_file = None - - -class UserStore(unittest.TestCase): - """Test each function of user_store to confirm they work as expected""" - - def setUp(self): - cfg.user_db = os.path.join(cfg.file_root, "tests/testdata/users"); - self.userstore = plugin_mount.UserStoreModule.get_plugins()[0] - - def tearDown(self): - for user in self.userstore.get_all(): - self.userstore.remove(user[0]) - self.userstore.close() - - def test_user_does_not_exist(self): - self.assertEqual(self.userstore.exists("notausername"),False) - - def test_user_does_exist(self): - self.add_user("isausername", False) - self.assertEqual(self.userstore.exists("isausername"),True) - - def test_add_user(self): - self.assertEqual(len(self.userstore.items()),0) - self.add_user("test_user", False) - self.assertEqual(len(self.userstore.items()),1) - - def test_user_is_in_expert_group(self): - self.add_user("test_user", True) - self.assertEqual(self.userstore.expert("test_user"),True) - - def test_user_is_not_in_expert_group(self): - self.add_user("test_user", False) - self.assertEqual(self.userstore.expert("test_user"),False) - - def test_user_removal(self): - self.assertEqual(len(self.userstore.items()),0) - self.add_user("test_user", False) - self.assertEqual(len(self.userstore.items()),1) - self.userstore.remove("test_user") - self.assertEqual(len(self.userstore.items()),0) - - def test_get_user_email_attribute(self): - self.add_user("test_user", False,"test@home") - self.assertEqual(self.userstore.attr("test_user","email"),"test@home") - - def test_get_user(self): - test_user = self.add_user("test_user", False) - self.assertEqual(self.userstore.get("test_user"),test_user) - - def test_get_all_users(self): - self.add_user("test_user1", False) - self.add_user("test_user2", False) - self.assertEqual(len(self.userstore.get_all()),2) - - def add_user(self, test_username, add_to_expert_group, email=''): - test_user = self.create_user(test_username, email) - if add_to_expert_group: - test_user = self.add_user_to_expert_group(test_user) - self.userstore.set(test_username,test_user) - return test_user - - def create_user(self, username, email=''): - test_user = User() - test_user["username"] = username - test_user["email"] = email - return test_user - - def add_user_to_expert_group(self, user): - user["groups"] = ["expert"] - return user - -if __name__ == "__main__": - unittest.main() From cde5f2523987e61032537125ba2357630ac11958 Mon Sep 17 00:00:00 2001 From: fonfon Date: Tue, 29 Jul 2014 17:05:29 +0300 Subject: [PATCH 2/7] bugfix: no more circular redirects to /firstboot --- modules/first_boot/middleware.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/first_boot/middleware.py b/modules/first_boot/middleware.py index 3d354b1ea..d28967d42 100644 --- a/modules/first_boot/middleware.py +++ b/modules/first_boot/middleware.py @@ -13,11 +13,15 @@ class FirstBootMiddleware: """ Forward to firstboot page if firstboot isn't finished yet """ def process_request(self, request): with sqlite_db(cfg.store_file, table='firstboot') as database: + first_boot_url = reverse('first_boot:index') if not 'state' in database: - # Permanent redirect causes the browser to cache the redirect, - # preventing the user from navigating to /plinth until the - # browser is restarted. - return HttpResponseRedirect(reverse('first_boot:index')) + if hasattr(request, 'url') and request.url != first_boot_url: + # Permanent redirect causes the browser to cache the + # redirect, preventing the user from navigating to /plinth + # until the browser is restarted. + return HttpResponseRedirect(first_boot_url) + else: + return if database['state'] < 5: LOGGER.info('First boot state - %d', database['state']) From 5b88015a284e810591d78b8903648fcef6c41d70 Mon Sep 17 00:00:00 2001 From: fonfon Date: Tue, 29 Jul 2014 18:16:54 +0300 Subject: [PATCH 3/7] improved my bugfix :> --- modules/first_boot/middleware.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/modules/first_boot/middleware.py b/modules/first_boot/middleware.py index d28967d42..d29c21a78 100644 --- a/modules/first_boot/middleware.py +++ b/modules/first_boot/middleware.py @@ -12,18 +12,18 @@ LOGGER = logging.getLogger(__name__) class FirstBootMiddleware: """ Forward to firstboot page if firstboot isn't finished yet """ def process_request(self, request): - with sqlite_db(cfg.store_file, table='firstboot') as database: - first_boot_url = reverse('first_boot:index') - if not 'state' in database: - if hasattr(request, 'url') and request.url != first_boot_url: + # TODO: there should be a better way to tell if we're at the + # firstboot module + if "firstboot" not in request.path: + with sqlite_db(cfg.store_file, table='firstboot') as database: + first_boot_url = reverse('first_boot:index') + if 'state' not in database: # Permanent redirect causes the browser to cache the # redirect, preventing the user from navigating to /plinth # until the browser is restarted. return HttpResponseRedirect(first_boot_url) - else: - return - if database['state'] < 5: - LOGGER.info('First boot state - %d', database['state']) - return HttpResponseRedirect(reverse('first_boot:state%d' % - database['state'])) + elif database['state'] < 5: + LOGGER.info('First boot state - %d', database['state']) + return HttpResponseRedirect(reverse('first_boot:state%d' % + database['state'])) From 57ba43cfc91ccaacdb60c145d4610929fd82cbcd Mon Sep 17 00:00:00 2001 From: fonfon Date: Wed, 30 Jul 2014 15:03:52 +0300 Subject: [PATCH 4/7] allow modules to have custom static directories --- module_loader.py | 3 +++ plinth.py | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/module_loader.py b/module_loader.py index b31f0e7c4..e06f2bc50 100644 --- a/module_loader.py +++ b/module_loader.py @@ -29,6 +29,8 @@ import cfg LOGGER = logging.getLogger(__name__) +loaded_modules = [] + def load_modules(): """ @@ -71,6 +73,7 @@ def load_modules(): for module_name in ordered_modules: _initialize_module(modules[module_name]) + loaded_modules.append(module_name) def _insert_modules(module_name, module, remaining_modules, ordered_modules): diff --git a/plinth.py b/plinth.py index f84d4b85a..bfe2ea520 100755 --- a/plinth.py +++ b/plinth.py @@ -104,6 +104,19 @@ def setup_server(): 'tools.staticdir.dir': '.'}} cherrypy.tree.mount(None, django.conf.settings.STATIC_URL, config) + # TODO: our modules are mimicking django apps. It'd be better to convert + # our modules to django apps instead of reinventing the wheel. + # (we'll still have to serve the static files with cherrypy though) + for module in module_loader.loaded_modules: + staticdir = os.path.join(cfg.file_root, 'modules', module, 'static') + if os.path.isdir(staticdir): + config = { + '/': {'tools.staticdir.root': staticdir, + 'tools.staticdir.on': True, + 'tools.staticdir.dir': '.'}} + urlprefix = "%s%s" % (django.conf.settings.STATIC_URL, module) + cherrypy.tree.mount(None, urlprefix, config) + if not cfg.no_daemon: Daemonizer(cherrypy.engine).subscribe() From 219109511f67a4a28ef562060a8d75752db33858 Mon Sep 17 00:00:00 2001 From: fonfon Date: Wed, 30 Jul 2014 15:16:28 +0300 Subject: [PATCH 5/7] templates: allow adding module/app specific static files when extending base.html --- templates/base.html | 1 + 1 file changed, 1 insertion(+) diff --git a/templates/base.html b/templates/base.html index b0e686e8b..5015b4c37 100644 --- a/templates/base.html +++ b/templates/base.html @@ -47,6 +47,7 @@ {{ css|safe }} + {% block head_extra %}{% endblock %} From d73c996547939ff7c915187e145154cd2513156d Mon Sep 17 00:00:00 2001 From: fonfon Date: Wed, 30 Jul 2014 15:24:18 +0300 Subject: [PATCH 6/7] renamed app-specific head block --- templates/base.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/base.html b/templates/base.html index 5015b4c37..605f98dae 100644 --- a/templates/base.html +++ b/templates/base.html @@ -47,7 +47,7 @@ {{ css|safe }} - {% block head_extra %}{% endblock %} + {% block app_head %}{% endblock %} From 8e2036928dea274973675c566dd140b02ec73801 Mon Sep 17 00:00:00 2001 From: fonfon Date: Wed, 30 Jul 2014 15:59:38 +0300 Subject: [PATCH 7/7] allow app- and page-specific head and javascript files --- templates/base.html | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/templates/base.html b/templates/base.html index 605f98dae..c5c142875 100644 --- a/templates/base.html +++ b/templates/base.html @@ -47,7 +47,8 @@ {{ css|safe }} - {% block app_head %}{% endblock %} + {% block app_head %}{% endblock %} + {% block page_head %}{% endblock %} @@ -170,5 +171,7 @@ {% block js_block %} {{ js|safe }} {% endblock %} + {% block app_js %}{% endblock %} + {% block page_js %}{% endblock %}