Made major changes to authenticator and roles to improve efficiency.

Changed authenticator.lua to request individual users, rather than always loading everyone.
Modified get_userinfo and removed get_userinfo_roles and get_userinfo_skin to remove unnecessary cfe processing.
This was especially bad when retrieving the cfe options for skins and roles.
This does break the interface used by other modules like acf-tinydns.
Validation and cfe processing were moved into password-model, where they belong.
Added global variables to authenticator and roles libraries so they can reuse data that was already generated.
Modified logon-controller to only check for zero users if logon fails.

6 files changed, 150 insertions, 31 deletions

diff --git a/app/acf-util/logon-controller.lua b/app/acf-util/logon-controller.lua
index 165bedb..2a88528 100644
--- a/app/acf-util/logon-controller.lua
+++ b/app/acf-util/logon-controller.lua
@@ -5,7 +5,7 @@ module (..., package.seeall)
 default_action = "status"
 
 -- Logon a new user based upon id and password in clientdata
-logon = function(self)
+local check_users = function(self)
 	-- If there are no users defined, add privileges and dispatch password/newuser
 	local users = self.model:list_users()
 	if #users.value == 0 then
@@ -14,9 +14,14 @@ logon = function(self)
 		self:dispatch(self.conf.prefix, "password", "newuser")
 		self.sessiondata.permissions[self.conf.prefix].password = nil
 		self.conf.suppress_view = true
-		return
+		return true
 	end
 
+	return false
+end
+
+-- Logon a new user based upon id and password in clientdata
+logon = function(self)
 	local userid = cfe({ value=clientdata.userid or "", label="User ID" })
 	local password = cfe({ label="Password" })
 	local redir = cfe({ value=clientdata.redir or "welcome/read", label="" })
@@ -28,6 +33,7 @@ logon = function(self)
 		if logon.value then
 			cmdresult.descr = "Logon Successful"
 		else
+			if check_users(self) then return end
 			cmdresult.errtxt = "Logon Attempt Failed"
 		end
 		cmdresult = self:redirect_to_referrer(cmdresult)
@@ -42,6 +48,7 @@ logon = function(self)
 			redirect(self, cmdresult.value.redir.value)
 		end
 	else
+		if check_users(self) then return end
 		cmdresult = self:redirect_to_referrer() or cmdresult
 	end
 	return cmdresult
diff --git a/app/acf-util/logon-model.lua b/app/acf-util/logon-model.lua
index d84e5e9..41cc0e0 100644
--- a/app/acf-util/logon-model.lua
+++ b/app/acf-util/logon-model.lua
@@ -45,8 +45,8 @@ logon = function (self, userid, password, ip_addr, sessiondir, sessiondata)
 			sessiondata.id = session.random_hash(512)
 			local t = authenticator.get_userinfo (self, userid)
 			sessiondata.userinfo = {}
-			for name,value in pairs(t.value) do
-				sessiondata.userinfo[name] = value.value
+			for name,value in pairs(t) do
+				sessiondata.userinfo[name] = value
 			end
 			return cfe({ type="boolean", value=true, label="Logon Success" })
 		else
diff --git a/app/acf-util/password-model.lua b/app/acf-util/password-model.lua
index fb06fae..b7e9ebe 100644
--- a/app/acf-util/password-model.lua
+++ b/app/acf-util/password-model.lua
@@ -1,43 +1,159 @@
 module(..., package.seeall)
 
 require("authenticator")
+require("roles")
 
-function create_user(self, userdata)
-	return authenticator.new_settings(self, userdata)
+avail_roles, avail_skins = nil
+
+local weak_password = function(password)
+	-- If password is too short, return false
+	if (#password < 4) then
+		return true, "Password is too short!"
+	end	
+	if (tonumber(password)) then
+		return true, "Password can't contain only numbers!"
+	end	
+
+	return false, nil
+end
+
+-- validate the settings (ignore password if it's nil)
+local validate_settings = function(settings)
+	-- Username, password, roles, and skin are allowed to not exist, just leave the same
+	-- Set errtxt when entering invalid values
+	if (#settings.value.userid.value == 0) then settings.value.userid.errtxt = "You need to enter a valid userid!" end
+	if string.find(settings.value.userid.value, "[^%w_]") then settings.value.userid.errtxt = "Can only contain letters, numbers, and '_'" end
+	if settings.value.username and string.find(settings.value.username.value, "%p") then settings.value.username.errtxt = "Cannot contain punctuation" end
+	if settings.value.password then
+		if (#settings.value.password.value == 0) then
+			settings.value.password.errtxt = "Password cannot be blank!"
+		elseif (not settings.value.password_confirm) or (settings.value.password.value ~= settings.value.password_confirm.value) then
+			settings.value.password.errtxt = "You entered wrong password/confirmation"
+		else
+			local weak_password_result, weak_password_errormessage = weak_password(settings.value.password.value)
+			if (weak_password_result) then settings.value.password.errtxt = weak_password_errormessage end
+		end
+	end
+	if settings.value.roles then modelfunctions.validatemulti(settings.value.roles) end
+	if settings.value.skin then modelfunctions.validateselect(settings.value.skin) end
+
+	-- Return false if any errormessages are set
+	for name,value in pairs(settings.value) do
+		if value.errtxt then
+			return false, settings
+		end
+	end
+
+	return true, settings
+end
+
+function create_user(self, settings)
+	return update_user(self, settings, true)
+end
+
+function update_user(self, settings, create)
+	local success, settings = validate_settings(settings)
+
+	if success then
+		local userinfo = authenticator.get_userinfo(self, settings.value.userid.value)
+		if userinfo and create then
+			settings.value.userid.errtxt = "This userid already exists!"
+			success = false
+		elseif not userinfo and not create then
+			settings.value.userid.errtxt = "This userid does not exist!"
+			success = false
+		end
+	end
+
+	if success then
+		local userinfo = {}
+		for name,val in pairs(settings.value) do
+			userinfo[name] = val.value
+		end
+		success = authenticator.write_userinfo(self, userinfo)
+	end
+
+	if not success then
+		if create then
+			settings.errtxt = "Failed to create new user"
+		else
+			settings.errtxt = "Failed to save settings"
+		end
+	end
+
+	return settings
 end
 
+
 function read_user(self, user)
-	local retval = authenticator.get_userinfo(self, user)
+	local result = {}
+        result.userid = cfe({ value=user, label="User id" })
+	
+	local userinfo = {}
 	if not user then
 		local userlist = authenticator.list_users(self)
 		if #userlist == 0 then
 			-- There are no users yet, suggest some values
-			retval.value.userid.value = "root"
-			retval.value.username.value = "Admin account"
-			retval.value.roles.value = {"ADMIN"}
+			result.userid.value = "root"
+			userinfo = { userid="root", username="Admin account", roles={"ADMIN"} }
+		end
+	else
+		userinfo = authenticator.get_userinfo(self, user)
+		if not userinfo then
+			result.userid.errtxt = "User does not exist"
+			userinfo = {}
 		end
 	end
-	return retval
-end
 
-function update_user(self, userdata)
-	return authenticator.change_settings(self, userdata)
+	if not avail_roles then
+		avail_roles = roles.list_all_roles(self)
+		for x,role in ipairs(avail_roles) do
+			if role==roles.guest_role then
+				table.remove(avail_roles,x)
+				break
+			end
+		end
+	end
+	
+	-- Call into skins controller to get the list of skins
+	if not avail_skins then
+		avail_skins = {""}
+		local contrl = self:new("acf-util/skins")
+		skins = contrl:read()
+		contrl:destroy()
+		for i,s in ipairs(skins.value) do
+			avail_skins[#avail_skins + 1] = s.value
+		end
+	end
+
+	-- Passwords are set to empty string
+	result.username = cfe({ value=userinfo.username or "", label="Real name" })
+	result.password = cfe({ value="", label="Password" })
+	result.password_confirm = cfe({ value="", label="Password (confirm)" })
+	result.roles = cfe({ type="multi", value=userinfo.roles or {}, label="Roles", option=avail_roles or {} })
+	result.skin = cfe({ type="select", value=userinfo.skin or "", label="Skin", option=avail_skins or {""} })
+
+	return cfe({ type="group", value=result, label="User Config" })
 end
 
 function get_users(self)
 	--List all users and their userinfo
 	local users = {}
 	local userlist = authenticator.list_users(self)
+	table.sort(userlist)
 	
 	for x,user in pairs(userlist) do
-		users[user] = read_user(self, user)
-		users[user].value.password = nil
-		users[user].value.password_confirm = nil
+		users[#users+1] = read_user(self, user)
 	end
 
 	return cfe({ type="group", value=users, label="User Configs" })
 end
 
 function delete_user(self, userid)
-	return authenticator.delete_user(self, userid)
+	result = cfe({ label="Delete user result", errtxt="Failed to delete user"})
+	if authenticator.delete_user(self, userid) then
+		result.value = "User deleted"
+		result.errtxt = nil
+	end
+	return result
 end
diff --git a/app/acf-util/password-status-html.lsp b/app/acf-util/password-status-html.lsp
index 90672ad..01eafe7 100644
--- a/app/acf-util/password-status-html.lsp
+++ b/app/acf-util/password-status-html.lsp
@@ -11,14 +11,9 @@
 </form>
 <H2>Existing account</H2>
 <DL>
-<% local users = {}
-for name,user in pairs(form.value) do
-	users[#users+1] = name
-end
-table.sort(users)
-for i,name in ipairs(users) do
-	user = form.value[name] %>
-	<DT><IMG SRC='<%= html.html_escape(page_info.wwwprefix..page_info.staticdir) %>/tango/16x16/apps/system-users.png' HEIGHT='16' WIDTH='16'> <%= html.html_escape(name) %></DT>
+<% for i,user in ipairs(form.value) do
+	local name = html.html_escape(user.value.userid.value) %>
+	<DT><IMG SRC='<%= html.html_escape(page_info.wwwprefix..page_info.staticdir) %>/tango/16x16/apps/system-users.png' HEIGHT='16' WIDTH='16'> <%= name %></DT>
 	<DD><TABLE>
 		<TR>
 			<TD STYLE='border:none;'><B><%= html.html_escape(user.value.userid.label) %></B></TD>
@@ -32,9 +27,9 @@ for i,name in ipairs(users) do
 		</TR><TR>
 			<TD STYLE='border:none;'><B>Option</B></TD>
 			<TD STYLE='border:none;'>
-			[<A HREF='edituser?userid=<%= html.html_escape(name) %>&redir=<%= html.html_escape(page_info.orig_action) %>'>Edit this account</A>]
-			[<A HREF='deleteuser?userid=<%= html.html_escape(name) %>'>Delete this account</A>]
-			[<A HREF='<%= html.html_escape(page_info.script) %>/acf-util/roles/viewuserroles?userid=<%= html.html_escape(name) %>'>View roles for this account</A>]
+			[<A HREF='edituser?userid=<%= name %>&redir=<%= html.html_escape(page_info.orig_action) %>'>Edit this account</A>]
+			[<A HREF='deleteuser?userid=<%= name %>'>Delete this account</A>]
+			[<A HREF='<%= html.html_escape(page_info.script) %>/acf-util/roles/viewuserroles?userid=<%= name %>'>View roles for this account</A>]
 			</TD>
 		</TR>
 	</TABLE></DD>
diff --git a/app/acf-util/roles-model.lua b/app/acf-util/roles-model.lua
index 6b12dd9..e12d49a 100644
--- a/app/acf-util/roles-model.lua
+++ b/app/acf-util/roles-model.lua
@@ -36,7 +36,8 @@ end
 
 -- Return roles/permissions for specified user
 get_user_roles = function(self, userid)
-	rls = cfe({ type="list", value=authenticator.get_userinfo_roles(self, userid).value, label="Roles" })
+	local userinfo = authenticator.get_userinfo(self, userid) or {}
+	rls = cfe({ type="list", value=userinfo.roles or {}, label="Roles" })
 	permissions = cfe({ type="table", value=roles.get_roles_perm(self, rls.value), label="Permissions" })
 	return cfe({ type="group", value={roles=rls, permissions=permissions} })
 end
diff --git a/app/acf_www-controller.lua b/app/acf_www-controller.lua
index 7d9d2d5..25f4c09 100644
--- a/app/acf_www-controller.lua
+++ b/app/acf_www-controller.lua
@@ -464,7 +464,7 @@ dispatch = function (self, userprefix, userctlr, useraction)
 			controller.worker.mvc.pre_exec ( controller )
 		end
 
-		-- run the action		
+		-- run the action
 		viewtable = controller.worker[self.conf.action](controller)
 
 		-- run the post_exec code