output of log action fixed

added some RFCs
added some unittests for CryptoBox config handling

pythonrewrite/bin2/CryptoBox.py

@@ -34,6 +34,11 @@ class CryptoBox:
 	put things like logging, conf and oter stuff in here,
 	that might be used by more classes, it will be passed on to them'''
 	def __init__(self, config_file=None):
+		# choose an exit function
+		if __name__ == "__main__":
+			self.errorExit = self.errorExitProg
+		else:
+			self.errorExit = self.errorExitMod
 		self.__initLogging()
 		self.__initPreferences(config_file)
 		self.__runTests()
@@ -60,7 +65,7 @@ class CryptoBox:
 			self.log.info("loggingsystem is up'n running")
 			## from now on everything can be logged via self.log...
 		except:
-			sys.errorExit("SystemError","Couldn't initialise the loggingsystem. I give up.")
+			self.errorExit("SystemError","Couldn't initialise the loggingsystem. I give up.")
 
 
 	def __initPreferences(self, config_file):
@@ -99,19 +104,24 @@ class CryptoBox:
 					self.cbxPrefs["Main"]["NameDatabase"])
 			except KeyError:
 				self.errorExit("ConfigError","could not find one of these configuration settings: [Main]->DataDir and [Main]->NameDatabase - please check your config file(%s)" % config_file)
+		except SyntaxError:
+			self.errorExit("ConfigError","Error during parsing of name database file (%s).\n" % (nameDB_file, ))
+		## create nameDB is necessary
 		if os.path.exists(nameDB_file):
 			self.nameDB = configobj.ConfigObj(nameDB_file)
 		else:
 			self.nameDB = configobj.ConfigObj(nameDB_file, create_empty=True)
-		except SyntaxError:
+		## check if nameDB file was created successfully?
-			self.errorExit("ConfigError","Error during parsing of name database file (%s).\n" % (nameDB_file, ))
+		if not os.path.exists(nameDB_file):
-		# TODO: check if nameDB file was created successfully?
+			self.errorExit("ConfigError","failed to create name database (%s)" % nameDB_file)
 		# get the loglevel
 		try:
 			log_level = self.cbxPrefs["Log"]["Level"].upper()
 			log_level_avail = ["DEBUG", "INFO", "WARN", "ERROR"]
 			if not log_level in log_level_avail:
 				self.errorExit("ConfigError","invalid log level: %s is not in %s" % (self.cbxPrefs["Log"]["Level"], log_level_avail))
+		except KeyError:
+			self.errorExit("ConfigError","could not find the configuration setting [Log]->Level in the config file (%s)" % config_file)
 		except TypeError:
 			self.errorExit("ConfigError","invalid log level: %s" % self.cbxPrefs["Log"]["Level"])
 		try:
@@ -121,8 +131,9 @@ class CryptoBox:
 				self.errorExit("ConfigError","could not find a configuration setting: [Log]->Details - please check your config file(%s)" % config_file)
 			new_handler.setFormatter(logging.Formatter('%(asctime)s %(module)s %(levelname)s: %(message)s'))
 			self.log.addHandler(new_handler)
-			# do not call parent's handlers
+			## do not call parent's handlers
 			self.log.propagate = False
+			## use 'getattr' as 'log_level' is a string
 			self.log.setLevel(getattr(logging,log_level))
 		except IOError:
 			self.errorExit("ConfigError","could not open logfile: %s" % self.cbxPrefs["Log"]["Details"])
@@ -130,6 +141,7 @@ class CryptoBox:
 
 	# do some initial checks
 	def __runTests(self):
+		## try to run 'super' with 'CryptoBoxRootActions'
 		try:
 			devnull = open(os.devnull, "w")
 		except IOError:
@@ -144,6 +156,8 @@ class CryptoBox:
 							"check"])
 		except OSError:
 			self.errorExit("ConfigError","could not find: %s" % self.cbxPrefs["Programs"]["super"])
+		except KeyError:
+			self.errorExit("ConfigError","could not find one of these configurations settings: [Programs]->super or [Programs]->CryptoBoxRootActions in the config file")
 		proc.wait()
 		if proc.returncode != 0:
 			self.errorExit("ConfigError","Could not call CryptoBoxRootActions by 'super' - maybe you did not add the appropriate line to /etc/super.tab?")
@@ -210,6 +224,31 @@ class CryptoBoxProps(CryptoBox):
 		return False
 
 
+	def getLogData(self, lines=None, maxSize=None):
+		"""get the most recent log entries of the cryptobox
+
+		the maximum number and size of these entries can be limited by 'lines' and 'maxSize'
+		"""
+		# return nothing if the currently selected log output is not a file
+		empty = [""]
+		try:
+			if self.cbxPrefs["Log"]["Destination"].upper() != "FILE": return empty
+			log_file = self.cbxPrefs["Log"]["Details"]
+		except KeyError:
+			self.log.error("could not evaluate one of the following config settings: [Log]->Destination or [Log]->Details")
+			return empty
+		try:
+			fd = open(log_file, "r")
+			if maxSize:	content = fd.readlines(maxSize)
+			else:	content = fd.readlines()
+			fd.close()
+		except IOError:
+			self.log.warn("failed to read the log file (%s)" % log_file)
+			return empty
+		if lines:	content = content[-lines:]
+		return content
+
+
 	def getContainerList(self, filterType=None, filterName=None):
 		"retrieve the list of all containers of this cryptobox"
 		try:
@@ -349,11 +388,6 @@ class CryptoBoxProps(CryptoBox):
 		except OSError:
 			return []
 
-# choose an exit function
-if __name__ == "__main__":
-	CryptoBox.errorExit = CryptoBox.errorExitProg
-else:
-	CryptoBox.errorExit = CryptoBox.errorExitMod
 
 if __name__ == "__main__":
 	cb = CryptoBox()

pythonrewrite/bin2/CryptoBoxContainer.py

@@ -1,10 +1,6 @@
 #!/usr/bin/env python2.4
 
-"""
+## check python version
-TODO: implement "getCapacity"
-"""
-
-# check python version
 import sys
 (ver_major, ver_minor, ver_sub, ver_desc, ver_subsub) = sys.version_info
 if (ver_major < 2) or ((ver_major == 2) and (ver_minor < 4)):
@@ -39,7 +35,7 @@ class CryptoBoxContainer:
 	__fsTypes = {
 		"plain":["ext3", "ext2", "vfat", "reiser"],
 		"swap":["swap"]}
-	"TODO: mehr Dateisystemtypen? / 'reiser' pruefen"
+	# TODO: more filesystem types? / check 'reiser'
 
 	__dmDir = "/dev/mapper"
 
@@ -57,7 +53,6 @@ class CryptoBoxContainer:
 
 
 	def setName(self, new_name):
-		"TODO: den Test-Code pruefen"
 		if new_name == self.name: return
 		if self.isMounted():
 			raise "VolumeIsActive", "the container must be inactive during renaming"
@@ -148,6 +143,7 @@ class CryptoBoxContainer:
 			errorMsg = "Could not add a new luks key: %s - %s" % (output.strip(), errout.strip(), )
 			self.log.error(errorMsg)
 			raise "ChangePasswordError", errorMsg
+		## retrieve the key slot we used for unlocking
 		keys_found = re.search(r'key slot (\d{1,3}) unlocked', output).groups()
 		if keys_found:
 			keyslot = int(keys_found[0])

pythonrewrite/bin2/CryptoBoxWebserver.py

@@ -5,7 +5,7 @@ import CryptoBoxWebserverSites
 try:
 	import cherrypy
 except: 
-	print "could not import cherrypy module! Try apt-get install python-cherrypy."
+	print "Could not import the cherrypy module! Try 'apt-get install python-cherrypy'."
 	sys.exit(1)
 
 class CryptoBoxWebserver:
@@ -17,6 +17,8 @@ class CryptoBoxWebserver:
 		#I currently have no idea how to cleanly extract the stylesheet path from 
 		#the config object without an extra CryptoBox.CryptoBoxProps instance.
 		#perhaps put config handling into a seperate class in CryptoBox.py?
+		# [l] why do we need to map the css manually? Shouldn't the whole
+		# www-data path be accessible anyway?
 		cherrypy.config.configMap.update(
 			{ 
 			"/cryptobox.css": { 

pythonrewrite/bin2/CryptoBoxWebserverSites.py

@@ -3,6 +3,8 @@ import CryptoBoxWebserverSettings
 import CryptoBoxWebserverRender
 website = CryptoBoxWebserverRender.CryptoBoxWebserverRender()
 
+# is it necessary to inherit these both classes?
+# for clarity they should be just instanciated - or not?
 class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettings.CryptoBoxWebserverSettings):
 	'''
 	url2func = {'index':'show_status','doc':'show_doc','logs':'show_log'}
@@ -14,9 +16,11 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		The default site to render is 'show_status'.
 		Self.settings is filled by the following methodcall
 		thus every rendered site will get actual values from the configfile.
-		After that the concrete site-method (e.g. doc) may set individual values.
+		After that the corresponding site-method (e.g. doc) may set individual values.
 		'''
 
+		# RFC: the following line somehow implicitly calls 'setSettings(self, self)'
+		# should it be that way? [l]
 		self.setSettings(self)
 		#self.settings
 		self.settings["Data.Action"] = action
@@ -29,7 +33,9 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		Every single site method has to call this before even looking
 		at url-given parameters.
 		This has to be called manually, since I don't see any other way of
-		sanitizing input automatically for all sites.'''
+		sanitizing input automatically for all sites.
+		# RFC: why shouldn't it be called in __init__? [l]
+		'''
 		#TODO: generate languages from existing hdf files
 		niceparams = { 'weblang': ('', 'de', 'en'), 
 				'loglevel': ('','info', 'warn', 'debug', 'error'),
@@ -38,7 +44,7 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		for evilkey in evilparams.keys():
 			## if the param isn't in the niceparams list, ignore it
 			if not niceparams.get(evilkey):
-				self.log.warn("irgnoring \"%s\"" % evilkey)
+				self.log.warn("ignoring \"%s\"" % evilkey)
 				return False
 				#TODO: until now only a warning message is printed
 			## if the param has no such value, set it to a default (the first in the list)
@@ -47,10 +53,12 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 				#TODO: set it to: niceparams.get(evilkey)[0]))
 		return 
 
+
 	def __check_config(self):
 		#TODO
 		pass
 
+
 	def __check_init_running(self):
 		#TODO
 		pass
@@ -61,14 +69,17 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 	def logs(self, loglevel=""):
 		'''displays a HTML version of the logfile
 
-		The loglevel has to be set and nothing else, as we just log in
+		The loglevel has to be set and nothing else, as we just log in english.
-		english.
+		RFC: what does this mean? We still have to save the current language - or not?
-		Be awar not to name this method just "log" as it seems to be a
+
-		reserved word.'''
+		Be aware not to name this method just "log" as it seems to be a
+		reserved word.
+		# RFC: maybe it conflicts with CryptoBoxProps.log - which we inherited?
+		'''
 		self.__sanitize_input({"loglevel":loglevel})
 		self.__prepare("show_log")
 		import filehandling
-		self.settings["Data.Log"] = filehandling.read_file(self.settings["Log.Details"])
+		self.settings["Data.Log"] = "<br/>".join(self.getLogData(lines=30, maxSize=2000))
 		#TODO: give the logs a nice format for displaying as html
 		# sed s/$/<\/br>/
 		return website.render(self)
@@ -91,45 +102,51 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 			self.settings["Data.Redirect.Delay"] = "60"
 		return website.render(self)
 
+	
 	def config(self,weblang=""):
 		pass
 
+
 	def doc(self,action="",page="",weblang=""):
 		'''prints the offline wikipage
 
 		TODO: action is unnessessary, remove here and from all html
 		files in doc/html/[de|en]/*
 		'''
+		# RFC: sanitize?
 		self.__prepare("show_doc")
 		if page:
 			self.settings["Data.Doc.Page"] = page
 		if page == "":
 			self.settings["Data.Doc.Page"] ="CryptoBoxUser"
 		if not weblang == "":
+			# TODO: check if there is a 'weblang' translation available for the docs
 			self.settings["Settings.DocLang"] = weblang
 		return website.render(self)
 
-	def system(self,type=""):
+
+	def system(self,typeOfShutdown=""):
 		self.__prepare("form_system")
-		if type == "reboot":
+		if typeOfShutdown == "reboot":
 			self.settings["Data.Success"] = "ReBoot"
 			self.settings["Data.Redirect.Action"] = "show_status"
 			self.settings["Data.Redirect.Delay"] = "180"
 			self.log.info("TODO: call function for system reboot")
-		elif type == "poweroff":
+		elif typeOfShutdown == "poweroff":
 			self.settings["Data.Success"] = "PowerOff"
 			self.log.info("TODO: call function for system shutdown")
 		else:
-			self.log.warn("someone tried to shutdown the system")
+			self.log.warn("someone tried to shutdown the system in a broken way (%s)" % typeOfShutdown)
 		return website.render(self)
 
+
 	def index(self):
 		self.__prepare("show_status")
 		return website.render(self)
 		
 
 		'''
-		## DONE: this functions are pythonized
+		## DONE: these functions are pythonized
 		#################### show_log #######################
 		##################### doc ############################
 		##################### poweroff ######################
@@ -182,6 +199,7 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		#at cryptobox.pl line 568
 		'''
 
+
 	############################################################################
 	## to make the sites visible through the webserver they must be exposed here
 	index.exposed = True

pythonrewrite/bin2/unittests.CryptoBox.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python2.4
 
 import unittest
+import sys
 
 class CryptoBoxPropsDeviceTests(unittest.TestCase):
 	import CryptoBox
@@ -47,35 +48,14 @@ class CryptoBoxPropsConfigTests(unittest.TestCase):
 [Log]
 Level           = debug
 Destination        = file
-		#Details     = %s/cryptobox.log
+Details     = %s/cryptobox.log
-		Details     = /tmp/cryptobox.log
 [Programs]
 blkid			= /sbin/blkid
 cryptsetup		= /sbin/cryptsetup
 super			= /usr/bin/super
-		CryptoBoxRootActions	= CryptoBoxRootActions"""
+CryptoBoxRootActions	= CryptoBoxRootActions
+"""
 
-	configContentBroken = """
-		[Main]
-		AllowedDevices  = /dev/loop
-		DefaultVolumePrefix = "Data "
-		#DataDir         = %s
-		NameDatabase    = cryptobox_names.db
-		[System]
-		User            = 1000
-		Group           = 1000
-		MountParentDir  = %s/mnt
-		DefaultCipher   = aes-cbc-essiv:sha256
-		[Log]
-		Level           = debug
-		Destination		= file
-		#Details		= %s/cryptobox.log
-		Details			= /tmp/cryptobox.log
-		[Programs]
-		blkid			= /sbin/blkid
-		cryptsetup		= /sbin/cryptsetup
-		super			= /usr/bin/super
-		CryptoBoxRootActions	= CryptoBoxRootActions"""
 
 	def setUp(self):
 		'''generate all files in tmp and remember the names'''
@@ -84,12 +64,7 @@ class CryptoBoxPropsConfigTests(unittest.TestCase):
 		self.tmpdirname = tempfile.mkdtemp(prefix="cbox-")
 		for file in self.files.keys():
 			self.filenames[file] = os.path.join(self.tmpdirname, self.files[file])
-		cf = open(self.filenames["configFileOK"], "w")
+		self.writeConfig()
-		cf.write(self.configContentOK % (self.tmpdirname, self.tmpdirname, self.tmpdirname))
-		cf.close()
-		cf = open(self.filenames["configFileBroken"], "w")
-		cf.write(self.configContentBroken % (self.tmpdirname, self.tmpdirname, self.tmpdirname))
-		cf.close()
 
 
 	def tearDown(self):
@@ -106,15 +81,43 @@ class CryptoBoxPropsConfigTests(unittest.TestCase):
 
 	def testConfigInit(self):
 		'''Check various branches of config file loading'''
+		import os
 		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,"/invalid/path/to/config/file")
 		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,"/etc/shadow")
-		self.CryptoBox.CryptoBoxProps()
+		for a in self.CryptoBox.CONF_LOCATIONS:
+			if os.path.exists(a): self.CryptoBox.CryptoBoxProps()
+		else: self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps)
 		self.CryptoBox.CryptoBoxProps(self.filenames["configFileOK"])
 		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,[])
+	
+	def testBrokenConfigs(self):
+		"""Check various broken configurations"""
+		self.writeConfig("NameDatabase", "#out", filename=self.filenames["configFileBroken"])
+		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,self.filenames["configFileBroken"])
+		self.writeConfig("Level", "#out", filename=self.filenames["configFileBroken"])
+		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,self.filenames["configFileBroken"])
+		self.writeConfig("Details", "#out", filename=self.filenames["configFileBroken"])
+		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,self.filenames["configFileBroken"])
+		self.writeConfig("super", "super=/bin/invalid/no", filename=self.filenames["configFileBroken"])
+		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,self.filenames["configFileBroken"])
+		self.writeConfig("CryptoBoxRootActions", "#not here", filename=self.filenames["configFileBroken"])
+		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,self.filenames["configFileBroken"])
+		self.writeConfig("CryptoBoxRootActions", "CryptoBoxRootActions = /bin/false", filename=self.filenames["configFileBroken"])
 		self.assertRaises("ConfigError", self.CryptoBox.CryptoBoxProps,self.filenames["configFileBroken"])
 		# TODO: check details of different ConfigError-exceptions
-		# TODO: use different kind of broken setups ...
+	
-		self.assertTrue(1)
+
+	def writeConfig(self, replace=None, newline=None, filename=None):
+		"""write a config file and (optional) replace a line in it"""
+		import re
+		if not filename: filename = self.filenames["configFileOK"]
+		content = self.configContentOK % (self.tmpdirname, self.tmpdirname, self.tmpdirname)
+		if replace:
+			pattern = re.compile('^' + replace + '\\s*=.*$', flags=re.M)
+			content = re.sub(pattern, newline, content)
+		cf = open(filename, "w")
+		cf.write(content)
+		cf.close()
 
 
 if __name__ == "__main__":