[sysconfig] Refactor ldapauth and adauth classes; show mainpage warning; improve config module handling

12 files changed, 240 insertions, 233 deletions

diff --git a/config.php.example b/config.php.example
index 14978897..6e760f1c 100644
--- a/config.php.example
+++ b/config.php.example
@@ -27,7 +27,8 @@ define('CONFIG_VMSTORE_DIR', '/srv/openslx/nfs');
 define('CONFIG_PROXY_CONF', '/opt/openslx/proxy/config');
 
 /* for the dozmod API proxy cache */ 
-define('CONFIG_DOZMOD_EXPIRE', 60*60); // 1 Minute
+define('CONFIG_DOZMOD_URL', 'http://127.0.0.1:9080');
+define('CONFIG_DOZMOD_EXPIRE', 60);
 
 // Sort order for menu - optional, if missing, order will be alphabetically
 $MENU_CAT_SORT_ORDER = array('main.content' => 0, 'main.settings-client' => 1, 'main.settings-server' => 2, 'main.status' => 3, 'main.users' => 4);
diff --git a/inc/trigger.inc.php b/inc/trigger.inc.php
index 353d6d69..db4a2148 100644
--- a/inc/trigger.inc.php
+++ b/inc/trigger.inc.php
@@ -86,19 +86,16 @@ class Trigger
 	public static function ldadp($exclude = NULL, $parent = NULL)
 	{
 		// TODO: Fetch list from ConfigModule_AdAuth (call loadDb first)
-		$res = Database::simpleQuery("SELECT moduleid, configtgz.filepath FROM configtgz_module"
+		$res = Database::simpleQuery("SELECT DISTINCT moduleid FROM configtgz_module"
 				. " INNER JOIN configtgz_x_module USING (moduleid)"
 				. " INNER JOIN configtgz USING (configid)"
+				. " INNER JOIN configtgz_location USING (configid)"
 				. " WHERE moduletype IN ('AdAuth', 'LdapAuth')");
-		// TODO: Multiconfig support
 		$id = array();
 		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
-			if (readlink('/srv/openslx/www/boot/default/config.tgz') === $row['filepath']) {
-				if (!is_null($exclude) && (int)$row['moduleid'] === (int)$exclude)
-					continue;
-				$id[] = (int)$row['moduleid'];
-				break;
-			}
+			if (!is_null($exclude) && (int)$row['moduleid'] === (int)$exclude)
+				continue;
+			$id[] = (int)$row['moduleid'];
 		}
 		$task = Taskmanager::submit('LdadpLauncher', array(
 				'ids' => $id,
diff --git a/modules-available/sysconfig/hooks/main-warning.inc.php b/modules-available/sysconfig/hooks/main-warning.inc.php
index e5bc592f..9c5a4f3f 100644
--- a/modules-available/sysconfig/hooks/main-warning.inc.php
+++ b/modules-available/sysconfig/hooks/main-warning.inc.php
@@ -1,6 +1,6 @@
 <?php
 
-if (!file_exists(CONFIG_HTTP_DIR . '/default/config.tgz')) {
+if (false === Database::queryFirst("SELECT locationid FROM configtgz_location WHERE locationid = 0")) {
 	Message::addError('sysconfig.no-noconfig-active', true);
 	$needSetup = true;
-}
\ No newline at end of file
+}
diff --git a/modules-available/sysconfig/inc/configmodule.inc.php b/modules-available/sysconfig/inc/configmodule.inc.php
index 9fc3db21..ca40094a 100644
--- a/modules-available/sysconfig/inc/configmodule.inc.php
+++ b/modules-available/sysconfig/inc/configmodule.inc.php
@@ -14,6 +14,7 @@ abstract class ConfigModule
 	private $moduleId = 0;
 	private $moduleArchive = false;
 	private $moduleTitle = false;
+	private $moduleStatus = false;
 	private $currentVersion = 0;
 	protected $moduleData = false;
 	
@@ -28,6 +29,7 @@ abstract class ConfigModule
 		if (self::$moduleTypes !== false)
 			return;
 		self::$moduleTypes = array();
+		Module::isAvailable('sysconfig');
 		foreach (glob(dirname(__FILE__) . '/configmodule/*.inc.php', GLOB_NOSORT) as $file) {
 			require_once $file;
 		}
@@ -58,14 +60,17 @@ abstract class ConfigModule
 	 */
 	public static function registerModule($id, $title, $description, $group, $unique, $sortOrder = 0)
 	{
-		if (isset(self::$moduleTypes[$id]))
+		if (isset(self::$moduleTypes[$id])) {
 			Util::traceError("Config Module $id already registered!");
+		}
 		$moduleClass = 'ConfigModule_' . $id;
 		$wizardClass = $id . '_Start';
-		if (!class_exists($moduleClass))
+		if (!class_exists($moduleClass)) {
 			Util::traceError("Class $moduleClass does not exist!");
-		if (get_parent_class($moduleClass) !== 'ConfigModule')
+		}
+		if (!is_subclass_of($moduleClass, 'ConfigModule')) {
 			Util::traceError("$moduleClass does not have ConfigModule as its parent!");
+		}
 		self::$moduleTypes[$id] = array(
 			'title' => $title,
 			'description' => $description,
@@ -86,11 +91,28 @@ abstract class ConfigModule
 	public static function getInstance($moduleType)
 	{
 		self::loadDb();
-		if (!isset(self::$moduleTypes[$moduleType]))
+		if (!isset(self::$moduleTypes[$moduleType])) {
+			error_log('Unknown module type: ' . $moduleType);
 			return false;
+		}
 		return new self::$moduleTypes[$moduleType]['moduleClass'];
 	}
 
+	public static function instanceFromDbRow($dbRow)
+	{
+		$instance = self::getInstance($dbRow['moduletype']);
+		$instance->currentVersion = $dbRow['version'];
+		$instance->moduleArchive = $dbRow['filepath'];
+		$instance->moduleData = json_decode($dbRow['contents'], true);
+		$instance->moduleId = $dbRow['moduleid'];
+		$instance->moduleTitle = $dbRow['title'];
+		$instance->moduleStatus = $dbRow['status'];
+		if ($instance->moduleVersion() > $instance->currentVersion) {
+			$instance->markFailed();
+		}
+		return $instance;
+	}
+
 	/**
 	 * Get module instance from id.
 	 *
@@ -99,19 +121,11 @@ abstract class ConfigModule
 	 */
 	public static function get($moduleId)
 	{
-		$ret = Database::queryFirst("SELECT title, moduletype, filepath, contents, version FROM configtgz_module "
+		$ret = Database::queryFirst("SELECT moduleid, title, moduletype, filepath, contents, version, status FROM configtgz_module "
 				. " WHERE moduleid = :moduleid LIMIT 1", array('moduleid' => $moduleId));
 		if ($ret === false)
 			return false;
-		$instance = self::getInstance($ret['moduletype']);
-		if ($instance === false)
-			return false;
-		$instance->currentVersion = $ret['version'];
-		$instance->moduleArchive = $ret['filepath'];
-		$instance->moduleData = json_decode($ret['contents'], true);
-		$instance->moduleId = $moduleId;
-		$instance->moduleTitle = $ret['title'];
-		return $instance;
+		return self::instanceFromDbRow($ret);
 	}
 
 	/**
@@ -123,23 +137,18 @@ abstract class ConfigModule
 	public static function getAll($moduleType = false)
 	{
 		if ($moduleType === false) {
-			$ret = Database::simpleQuery("SELECT moduleid, title, moduletype, filepath, contents, version FROM configtgz_module");
+			$ret = Database::simpleQuery("SELECT moduleid, title, moduletype, filepath, contents, version, status FROM configtgz_module");
 		} else {
-			$ret = Database::simpleQuery("SELECT moduleid, title, moduletype, filepath, contents, version FROM configtgz_module "
+			$ret = Database::simpleQuery("SELECT moduleid, title, moduletype, filepath, contents, version, status FROM configtgz_module "
 					. " WHERE moduletype = :moduletype", array('moduletype' => $moduleType));
 		}
 		if ($ret === false)
 			return false;
 		$list = array();
 		while ($row = $ret->fetch(PDO::FETCH_ASSOC)) {
-			$instance = self::getInstance($row['moduletype']);
+			$instance = self::instanceFromDbRow($row);
 			if ($instance === false)
-				return false;
-			$instance->currentVersion = $row['version'];
-			$instance->moduleArchive = $row['filepath'];
-			$instance->moduleData = json_decode($row['contents'], true);
-			$instance->moduleId = $row['moduleid'];
-			$instance->moduleTitle = $row['title'];
+				continue;
 			$list[] = $instance;
 		}
 		return $list;
@@ -200,6 +209,16 @@ abstract class ConfigModule
 		return CONFIG_TGZ_LIST_DIR . '/modules/'
 			. $this->moduleType() . '_id-' . $this->moduleId . '__' . mt_rand() . '-' . time() . '.tgz';
 	}
+
+	public function allowDownload()
+	{
+		return false;
+	}
+
+	public function needRebuild()
+	{
+		return $this->moduleStatus !== 'OK' || $this->currentVersion < $this->moduleVersion();
+	}
 	
 	/**
 	 * Get module id (in db)
@@ -230,6 +249,11 @@ abstract class ConfigModule
 	{
 		return $this->moduleArchive;
 	}
+
+	public final function status()
+	{
+		return $this->moduleStatus;
+	}
 	
 	/**
 	 * Get the module type.
diff --git a/modules-available/sysconfig/inc/configmodule/adauth.inc.php b/modules-available/sysconfig/inc/configmodule/adauth.inc.php
index 180ac717..db06a4a4 100644
--- a/modules-available/sysconfig/inc/configmodule/adauth.inc.php
+++ b/modules-available/sysconfig/inc/configmodule/adauth.inc.php
@@ -1,5 +1,12 @@
 <?php
 
+class ConfigModule_AdAuth extends ConfigModuleBaseLdap
+{
+
+	const MODID = 'AdAuth';
+
+}
+
 ConfigModule::registerModule(
 	ConfigModule_AdAuth::MODID, // ID
 	Dictionary::translateFileModule('sysconfig', 'config-module', 'adAuth_title'), // Title
@@ -7,69 +14,3 @@ ConfigModule::registerModule(
 	Dictionary::translateFileModule('sysconfig', 'config-module', 'group_authentication'), // Group
 	true // Only one per config?
 );
-
-class ConfigModule_AdAuth extends ConfigModule
-{
-
-	const MODID = 'AdAuth';
-	const VERSION = 1;
-
-	private static $REQUIRED_FIELDS = array('server', 'searchbase', 'binddn');
-	private static $OPTIONAL_FIELDS = array('bindpw', 'home', 'ssl', 'fingerprint', 'certificate', 'homeattr');
-
-	protected function generateInternal($tgz, $parent)
-	{
-		Trigger::ldadp($this->id(), $parent);
-		$config = $this->moduleData;
-		if (isset($config['certificate']) && !is_string($config['certificate'])) {
-			unset($config['certificate']);
-		}
-		if (preg_match('/^([^\:]+)\:(\d+)$/', $config['server'], $out)) {
-			$config['server'] = $out[1];
-			$config['adport'] = $out[2];
-		} else {
-			if (isset($config['certificate'])) {
-				$config['adport'] = 636;
-			} else {
-				$config['adport'] = 389;
-			}
-		}
-		$config['parentTask'] = $parent;
-		$config['failOnParentFail'] = false;
-		$config['proxyip'] = Property::getServerIp();
-		$config['proxyport'] = 3100 + $this->id();
-		$config['filename'] = $tgz;
-		$config['moduleid'] = $this->id();
-		return Taskmanager::submit('CreateLdapConfig', $config);
-	}
-
-	protected function moduleVersion()
-	{
-		return self::VERSION;
-	}
-
-	protected function validateConfig()
-	{
-		// Check if required fields are filled
-		return Util::hasAllKeys($this->moduleData, self::$REQUIRED_FIELDS);
-	}
-
-	public function setData($key, $value)
-	{
-		if (!in_array($key, self::$REQUIRED_FIELDS) && !in_array($key, self::$OPTIONAL_FIELDS))
-			return false;
-		$this->moduleData[$key] = $value;
-		return true;
-	}
-
-	// ############## Callbacks #############################
-
-	/**
-	 * Server IP changed - rebuild all AD modules.
-	 */
-	public function event_serverIpChanged()
-	{
-		$this->generate(false);
-	}
-
-}
diff --git a/modules-available/sysconfig/inc/configmodule/branding.inc.php b/modules-available/sysconfig/inc/configmodule/branding.inc.php
index 4a9718d6..fd11dade 100644
--- a/modules-available/sysconfig/inc/configmodule/branding.inc.php
+++ b/modules-available/sysconfig/inc/configmodule/branding.inc.php
@@ -53,4 +53,9 @@ class ConfigModule_Branding extends ConfigModule
 		return false;
 	}
 
+	public function allowDownload()
+	{
+		return true;
+	}
+
 }
diff --git a/modules-available/sysconfig/inc/configmodule/customodule.inc.php b/modules-available/sysconfig/inc/configmodule/customodule.inc.php
index 8d1b6bf0..336d794f 100644
--- a/modules-available/sysconfig/inc/configmodule/customodule.inc.php
+++ b/modules-available/sysconfig/inc/configmodule/customodule.inc.php
@@ -53,4 +53,9 @@ class ConfigModule_CustomModule extends ConfigModule
 		return false;
 	}
 
+	public function allowDownload()
+	{
+		return true;
+	}
+
 }
diff --git a/modules-available/sysconfig/inc/configmodule/ldapauth.inc.php b/modules-available/sysconfig/inc/configmodule/ldapauth.inc.php
index ed1a47c3..1a706234 100644
--- a/modules-available/sysconfig/inc/configmodule/ldapauth.inc.php
+++ b/modules-available/sysconfig/inc/configmodule/ldapauth.inc.php
@@ -1,77 +1,23 @@
 <?php
 
-ConfigModule::registerModule(
-	ConfigModule_LdapAuth::MODID, // ID
-	Dictionary::translateFileModule('sysconfig', 'config-module', 'ldapAuth_title'), // Title
-	Dictionary::translateFileModule('sysconfig', 'config-module', 'ldapAuth_description'), // Description
-	Dictionary::translateFileModule('sysconfig', 'config-module', 'group_authentication'), // Group
-	true // Only one per config?
-);
-
-class ConfigModule_LdapAuth extends ConfigModule
+class ConfigModule_LdapAuth extends ConfigModuleBaseLdap
 {
 
 	const MODID = 'LdapAuth';
-	const VERSION = 1;
 
-	private static $REQUIRED_FIELDS = array('server', 'searchbase');
-	private static $OPTIONAL_FIELDS = array('binddn', 'bindpw', 'home', 'ssl', 'fingerprint', 'certificate');
-
-	protected function generateInternal($tgz, $parent)
+	protected function preTaskmanagerHook(&$config)
 	{
-		Trigger::ldadp($this->id(), $parent);
-		$config = $this->moduleData;
-		if (isset($config['certificate']) && !is_string($config['certificate'])) {
-			unset($config['certificate']);
-		}
-		if (preg_match('/^([^\:]+)\:(\d+)$/', $config['server'], $out)) {
-			$config['server'] = $out[1];
-			$config['adport'] = $out[2]; // sic!
-		} else {
-			if (isset($config['certificate'])) {
-				$config['adport'] = 636;
-			} else {
-				$config['adport'] = 389;
-			}
-		}
-		$config['parentTask'] = $parent;
-		$config['failOnParentFail'] = false;
-		$config['proxyip'] = Property::getServerIp();
-		$config['proxyport'] = 3100 + $this->id();
-		$config['filename'] = $tgz;
-		$config['moduleid'] = $this->id();
+		// Just set the flag so the taskmanager job knows we're dealing with a normal ldap server,
+		// not AD scheme
 		$config['plainldap'] = true;
-		return Taskmanager::submit('CreateLdapConfig', $config);
-	}
-
-	protected function moduleVersion()
-	{
-		return self::VERSION;
-	}
-
-	protected function validateConfig()
-	{
-		// Check if required fields are filled
-		return Util::hasAllKeys($this->moduleData, self::$REQUIRED_FIELDS);
-	}
-
-	public function setData($key, $value)
-	{
-		if (!in_array($key, self::$REQUIRED_FIELDS) && !in_array($key, self::$OPTIONAL_FIELDS))
-			return false;
-		$this->moduleData[$key] = $value;
-		return true;
-	}
-
-	// ############## Callbacks #############################
-
-	/**
-	 * Server IP changed - rebuild all LDAP modules.
-	 */
-	public function event_serverIpChanged()
-	{
-		error_log('Calling generate on ' . $this->title());
-		$this->generate(false);
 	}
 
 }
+
+ConfigModule::registerModule(
+	ConfigModule_LdapAuth::MODID, // ID
+	Dictionary::translateFileModule('sysconfig', 'config-module', 'ldapAuth_title'), // Title
+	Dictionary::translateFileModule('sysconfig', 'config-module', 'ldapAuth_description'), // Description
+	Dictionary::translateFileModule('sysconfig', 'config-module', 'group_authentication'), // Group
+	true // Only one per config?
+);
diff --git a/modules-available/sysconfig/inc/configmodulebaseldap.inc.php b/modules-available/sysconfig/inc/configmodulebaseldap.inc.php
new file mode 100644
index 00000000..760593e1
--- /dev/null
+++ b/modules-available/sysconfig/inc/configmodulebaseldap.inc.php
@@ -0,0 +1,85 @@
+<?php
+
+abstract class ConfigModuleBaseLdap extends ConfigModule
+{
+
+	const VERSION = 2;
+
+	private static $REQUIRED_FIELDS = array('server', 'searchbase');
+	private static $OPTIONAL_FIELDS = array('binddn', 'bindpw', 'home', 'ssl', 'fingerprint', 'certificate', 'homeattr',
+		'shareRemapMode', 'shareRemapCreate', 'shareDocuments', 'shareDownloads', 'shareDesktop', 'shareMedia', 'shareOther', 'shareHomeDrive');
+
+	protected function generateInternal($tgz, $parent)
+	{
+		Trigger::ldadp($this->id(), $parent);
+		$config = $this->moduleData;
+		if (isset($config['certificate']) && !is_string($config['certificate'])) {
+			unset($config['certificate']);
+		}
+		if (preg_match('/^([^\:]+)\:(\d+)$/', $config['server'], $out)) {
+			$config['server'] = $out[1];
+			$config['adport'] = $out[2];
+		} else {
+			if (isset($config['certificate'])) {
+				$config['adport'] = 636;
+			} else {
+				$config['adport'] = 389;
+			}
+		}
+		$config['parentTask'] = $parent;
+		$config['failOnParentFail'] = false;
+		$config['proxyip'] = Property::getServerIp();
+		$config['proxyport'] = 3100 + $this->id();
+		$config['filename'] = $tgz;
+		$config['moduleid'] = $this->id();
+		if (!isset($config['shareRemapMode'])) {
+			$config['shareRemapMode'] = 3;
+		}
+		if (!isset($config['shareHomeDrive'])) {
+			$config['shareHomeDrive'] = 'H:';
+		}
+		$this->preTaskmanagerHook($config);
+		return Taskmanager::submit('CreateLdapConfig', $config);
+	}
+
+	/**
+	 * Hook called before running CreateLdapConfig task with the
+	 * configuration to be passed to the task. Passed by reference
+	 * so it can be modified.
+	 *
+	 * @param array $config
+	 */
+	protected function preTaskmanagerHook(&$config)
+	{
+	}
+
+	protected function moduleVersion()
+	{
+		return self::VERSION;
+	}
+
+	protected function validateConfig()
+	{
+		// Check if required fields are filled
+		return Util::hasAllKeys($this->moduleData, self::$REQUIRED_FIELDS);
+	}
+
+	public function setData($key, $value)
+	{
+		if (!in_array($key, self::$REQUIRED_FIELDS) && !in_array($key, self::$OPTIONAL_FIELDS))
+			return false;
+		$this->moduleData[$key] = $value;
+		return true;
+	}
+
+	// ############## Callbacks #############################
+
+	/**
+	 * Server IP changed - rebuild all AD modules.
+	 */
+	public function event_serverIpChanged()
+	{
+		$this->generate(false);
+	}
+
+}
diff --git a/modules-available/sysconfig/install.inc.php b/modules-available/sysconfig/install.inc.php
index 0cde39c2..91f282dd 100644
--- a/modules-available/sysconfig/install.inc.php
+++ b/modules-available/sysconfig/install.inc.php
@@ -39,10 +39,17 @@ $res[] = tableCreate('configtgz_location', "
 
 // Constraints
 if (in_array(UPDATE_DONE, $res)) {
-	Database::exec("ALTER TABLE `configtgz_x_module`
-		ADD CONSTRAINT `configtgz_x_module_ibfk_1` FOREIGN KEY (`configid`) REFERENCES `configtgz` (`configid`) ON DELETE CASCADE");
-	Database::exec("ALTER TABLE `configtgz_x_module`
-		ADD CONSTRAINT `configtgz_x_module_ibfk_2` FOREIGN KEY (`moduleid`) REFERENCES `configtgz_module` (`moduleid`)");
+	$ret = Database::exec("ALTER TABLE `configtgz_x_module`
+		ADD CONSTRAINT `configtgz_x_module_ibfk_1` FOREIGN KEY (`configid`) REFERENCES `configtgz` (`configid`)
+		ON DELETE CASCADE");
+	$ret = Database::exec("ALTER TABLE `configtgz_x_module`
+		ADD CONSTRAINT `configtgz_x_module_ibfk_2` FOREIGN KEY (`moduleid`) REFERENCES `configtgz_module` (`moduleid`)") || $ret;
+	$ret = Database::exec("ALTER TABLE `configtgz_location`
+		ADD CONSTRAINT `configtgz_location_fk_configid` FOREIGN KEY ( `configid` ) REFERENCES `openslx`.`configtgz` (`configid`)
+		ON DELETE CASCADE ON UPDATE CASCADE") || $ret;
+	if ($ret) {
+		$res[] = UPDATE_DONE;
+	}
 }
 
 // Update path
@@ -77,14 +84,16 @@ if (!tableHasColumn('configtgz', 'status')) {
 }
 
 // ----- rebuild AD configs ------
-// TEMPORARY HACK; Rebuild AD configs.. move somewhere else
+// TEMPORARY HACK; Rebuild configs.. move somewhere else?
 Module::isAvailable('sysconfig');
-$list = array_merge(ConfigModule::getAll('AdAuth'), ConfigModule::getAll('LdapAuth'));
+$list = ConfigModule::getAll();
 if ($list === false) {
 	EventLog::warning('Could not regenerate AD/LDAP configs - please do so manually');
 } else {
 	foreach ($list as $ad) {
-		$ad->generate(false);
+		if ($ad->needRebuild()) {
+			$ad->generate(false);
+		}
 	}
 }
 
diff --git a/modules-available/sysconfig/page.inc.php b/modules-available/sysconfig/page.inc.php
index 9bf9cacd..a3c0c93f 100644
--- a/modules-available/sysconfig/page.inc.php
+++ b/modules-available/sysconfig/page.inc.php
@@ -5,6 +5,7 @@ class Page_SysConfig extends Page
 
 	/**
 	 * Holds all the known configuration modules, with title, description, start class for their wizard, etc.
+	 *
 	 * @var array
 	 */
 	protected static $moduleTypes = array();
@@ -46,7 +47,7 @@ class Page_SysConfig extends Page
 	}
 
 	/**
-	 * 
+	 *
 	 * @return array All registered module types
 	 */
 	public static function getModuleTypes()
@@ -136,35 +137,35 @@ class Page_SysConfig extends Page
 	{
 		$action = Request::any('action', 'list');
 		switch ($action) {
-			case 'addmodule':
-				AddModule_Base::render();
-				return;
-			case 'addconfig':
-				AddConfig_Base::render();
+		case 'addmodule':
+			AddModule_Base::render();
+			return;
+		case 'addconfig':
+			AddConfig_Base::render();
+			return;
+		case 'list':
+			Render::openTag('div', array('class' => 'row'));
+			$this->listConfigs();
+			if ($this->currentLoc === 0) {
+				$this->listModules();
+			}
+			Render::closeTag('div');
+			Render::addTemplate('list-legend', array('showLocationBadge' => $this->haveOverriddenLocations));
+			return;
+		case 'module':
+			$listid = Request::post('list');
+			if ($listid !== false) {
+				$this->listModuleContents($listid);
 				return;
-			case 'list':
-				Render::openTag('div', array('class' => 'row'));
-				$this->listConfigs();
-				if ($this->currentLoc === 0) {
-					$this->listModules();
-				}
-				Render::closeTag('div');
-				Render::addTemplate('list-legend', array('showLocationBadge' => $this->haveOverriddenLocations));
+			}
+			break;
+		case 'config':
+			$listid = Request::post('list');
+			if ($listid !== false) {
+				$this->listConfigContents($listid);
 				return;
-			case 'module':
-				$listid = Request::post('list');
-				if ($listid !== false) {
-					$this->listModuleContents($listid);
-					return;
-				}
-				break;
-			case 'config':
-				$listid = Request::post('list');
-				if ($listid !== false) {
-					$this->listConfigContents($listid);
-					return;
-				}
-				break;
+			}
+			break;
 		}
 		Message::addError('invalid-action', $action, 'main');
 	}
@@ -243,17 +244,10 @@ class Page_SysConfig extends Page
 	private function listModules()
 	{
 		// Config modules
-		$res = Database::simpleQuery("SELECT moduleid, title, moduletype, status FROM configtgz_module ORDER BY moduletype ASC, title ASC");
-		$modules = array();
-		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
-			$modules[] = array(
-				'moduleid' => $row['moduleid'],
-				'moduletype' => $row['moduletype'],
-				'module' => $row['title'],
-				'iscustom' => ($row['moduletype'] === 'CustomModule' || $row['moduletype'] === 'Branding'),
-				'needrebuild' => ($row['status'] !== 'OK')
-			);
-		}
+		$modules = ConfigModule::getAll();
+		$types = array_map(function ($mod) { return $mod->moduleType(); }, $modules);
+		$titles = array_map(function ($mod) { return $mod->title(); }, $modules);
+		array_multisort($types, SORT_ASC, $titles, SORT_ASC, $modules);
 		Render::addTemplate('list-modules', array(
 			'modules' => $modules,
 			'havemodules' => (count($modules) > 0)
@@ -271,7 +265,7 @@ class Page_SysConfig extends Page
 
 		// find files in that archive
 		$status = Taskmanager::submit('ListArchive', array(
-				'file' => $row['filepath']
+			'file' => $row['filepath']
 		));
 		if (isset($status['id']))
 			$status = Taskmanager::waitComplete($status, 4000);
@@ -306,7 +300,7 @@ class Page_SysConfig extends Page
 			'files' => $list,
 		));
 	}
-	
+
 	private function listConfigContents($configid)
 	{
 		// get config name
@@ -321,7 +315,7 @@ class Page_SysConfig extends Page
 			. " INNER JOIN configtgz_x_module USING (moduleid)"
 			. " WHERE configtgz_x_module.configid = :configid"
 			. " ORDER BY module.title ASC", array('configid' => $configid));
-		
+
 		$modules = array();
 		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
 			$modules[] = array(
@@ -329,7 +323,7 @@ class Page_SysConfig extends Page
 				'moduleid' => $row['moduleid']
 			);
 		}
-		
+
 		// render the template
 		Render::addDialog(Dictionary::translate('lang_contentOf') . ' ' . $config['title'], false, 'config-module-list', array(
 			'modules' => $modules
@@ -364,7 +358,7 @@ class Page_SysConfig extends Page
 			Message::addError('config-invalid', $configid);
 			Util::redirect('?do=sysconfig&locationid=' . $this->currentLoc);
 		}
-		$ret = $config->generate(false, 350); // TODO
+		$ret = $config->generate(false, 500); // TODO
 		if ($ret === true)
 			Message::addSuccess('module-rebuilt', $config->title());
 		elseif ($ret === false)
@@ -383,14 +377,14 @@ class Page_SysConfig extends Page
 			Util::redirect('?do=sysconfig');
 		}
 		$existing = Database::queryFirst("SELECT title FROM configtgz_x_module"
-				. " INNER JOIN configtgz USING (configid)"
-				. " WHERE moduleid = :moduleid LIMIT 1", array('moduleid' => $moduleid));
+			. " INNER JOIN configtgz USING (configid)"
+			. " WHERE moduleid = :moduleid LIMIT 1", array('moduleid' => $moduleid));
 		if ($existing !== false) {
 			Message::addError('module-in-use', $row['title'], $existing['title']);
 			Util::redirect('?do=sysconfig');
 		}
 		$task = Taskmanager::submit('DeleteFile', array(
-				'file' => $row['filepath']
+			'file' => $row['filepath']
 		));
 		if (isset($task['statusCode']) && $task['statusCode'] === TASK_WAITING) {
 			$task = Taskmanager::waitComplete($task['id']);
diff --git a/modules-available/sysconfig/templates/list-modules.html b/modules-available/sysconfig/templates/list-modules.html
index c3e2d736..ba9ce425 100644
--- a/modules-available/sysconfig/templates/list-modules.html
+++ b/modules-available/sysconfig/templates/list-modules.html
@@ -11,25 +11,25 @@
 				<table id="modtable" class="slx-table" style="max-width:100px !important">
 					{{#modules}}
 					<tr>
-						<td class="badge slx-nowrap">{{moduletype}}</td>
-						<td data-id="{{moduleid}}" class="modrow slx-width-ignore slx-nowrap" width="100%"><div class="slx-dyn-ellipsis">{{module}}</div></td>
+						<td class="badge slx-nowrap">{{moduleType}}</td>
+						<td data-id="{{id}}" class="modrow slx-width-ignore slx-nowrap" width="100%"><div class="slx-dyn-ellipsis">{{title}}</div></td>
 						<td class="slx-nowrap">
-							{{#iscustom}}
-							<button class="btn btn-default btn-xs" name="list" value="{{moduleid}}" title="{{lang_show}}"><span class="glyphicon glyphicon-eye-open"></span></button>
-							<button class="btn btn-default btn-xs" name="download" value="{{moduleid}}" title="{{lang_download}}"><span class="glyphicon glyphicon-download-alt"></span></button>
-							{{/iscustom}}
+							{{#allowDownload}}
+							<button class="btn btn-default btn-xs" name="list" value="{{id}}" title="{{lang_show}}"><span class="glyphicon glyphicon-eye-open"></span></button>
+							<button class="btn btn-default btn-xs" name="download" value="{{id}}" title="{{lang_download}}"><span class="glyphicon glyphicon-download-alt"></span></button>
+							{{/allowDownload}}
 						</td>
 						<td class="slx-nowrap">
 							<button
-									{{#needrebuild}}
+									{{#needRebuild}}
 									class="refmod btn btn-primary btn-xs"
-									{{/needrebuild}}
-							{{^needrebuild}}
+									{{/needRebuild}}
+							{{^needRebuild}}
 							class="refmod btn btn-default btn-xs"
-							{{/needrebuild}}
-							name="rebuild" value="{{moduleid}}" title="{{lang_rebuild}}"><span class="glyphicon glyphicon-refresh"></span></button>
-							<a class="btn btn-success btn-xs" href="?do=SysConfig&amp;action=addmodule&amp;step={{moduletype}}_Start&amp;edit={{moduleid}}" title="{{lang_edit}}"><span class="glyphicon glyphicon-edit"></span></a>
-							<button class="btn btn-danger btn-xs" name="del" value="{{moduleid}}" title="{{lang_delete}}"><span class="glyphicon glyphicon-trash"></span></button>
+							{{/needRebuild}}
+							name="rebuild" value="{{id}}" title="{{lang_rebuild}}"><span class="glyphicon glyphicon-refresh"></span></button>
+							<a class="btn btn-success btn-xs" href="?do=SysConfig&amp;action=addmodule&amp;step={{moduleType}}_Start&amp;edit={{id}}" title="{{lang_edit}}"><span class="glyphicon glyphicon-edit"></span></a>
+							<button class="btn btn-danger btn-xs" name="del" value="{{id}}" title="{{lang_delete}}"><span class="glyphicon glyphicon-trash"></span></button>
 						</td>
 					</tr>
 					{{/modules}}