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.

8 files changed, 277 insertions, 283 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
diff --git a/lib/authenticator.lua b/lib/authenticator.lua
index 45caec7..95eb3d7 100644
--- a/lib/authenticator.lua
+++ b/lib/authenticator.lua
@@ -21,103 +21,43 @@ usertable = "passwd"
 roletable = "roles"
 
 -- This will hold the auth structure from the database
-local authstruct
+local authstruct = {}
+local complete = false
+
+local parse_entry = function(id, entry)
+	local a
+	if id and id ~= "" and entry and entry ~= "" then
+		local fields = {}
+		for x in string.gmatch(entry or "", "([^:]*):?") do
+			fields[#fields + 1] = x
+		end
+		a = {}
+		a.userid = id
+		a.password = fields[1] or ""
+		a.username = fields[2] or ""
+		a.roles = fields[3] or ""
+		a.skin = fields[4] or ""
+		authstruct[id] = a
+	end
+	return a
+end
 
 local load_database = function(self)
-	if not authstruct then
+	if not complete then
 		local authtable = auth.read_field(self, usertable, "") or {}
 		authstruct = {}
 		for i,value in ipairs(authtable) do
-			if value.id ~= "" then
-				local fields = {}
-				for x in string.gmatch(value.entry, "([^:]*):?") do
-					fields[#fields + 1] = x
-				end
-				local a = {}
-				a.userid = value.id
-				a.password = fields[1] or ""
-				a.username = fields[2] or ""
-				a.roles = fields[3] or ""
-				a.skin = fields[4] or ""
-				table.insert(authstruct, a)
-			end
-		end
-	end
-end
-	
-local get_id = function(userid)
-	if authstruct ~= nil then
-		for x = 1,#authstruct do
-			if authstruct[x].userid == userid then
-				return authstruct[x]
-			end
-		end
-	end
-	return nil
-end
-
-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
-
-local write_settings = function(self, settings, id)
-	load_database()
-	id = id or get_id(settings.value.userid.value) or {}
-	-- Username, password, roles, skin are allowed to not exist, just leave the same
-	id.userid = settings.value.userid.value
-	if settings.value.username then id.username = settings.value.username.value end
-	if settings.value.password then id.password = md5.sumhexa(settings.value.password.value) end
-	if settings.value.roles then id.roles = table.concat(settings.value.roles.value, ",") end
-	if settings.value.skin then id.skin = settings.value.skin.value end
-
-	local success = auth.write_entry(self, usertable, "", id.userid, (id.password or "")..":"..(id.username or "")..":"..(id.roles or "")..":"..(id.skin or ""))
-
-	if success and self.sessiondata and self.sessiondata.userinfo and self.sessiondata.userinfo.userid == id.userid then
-		self.sessiondata.userinfo = {}
-		for name,value in pairs(id) do
-			self.sessiondata.userinfo[name] = value
+			parse_entry(value.id, value.entry)
 		end
+		complete = true
 	end
-
-	return success
 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
+local get_id = function(self, userid)
+	if not authstruct[userid] then
+		parse_entry(userid, auth.read_entry(self, usertable, "", userid))
 	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
+	return authstruct[userid]
 end
 
 --- public methods
@@ -130,17 +70,12 @@ authenticate = function(self, userid, password)
 	if not userid or not password then
 		errtxt = "Invalid parameter"
 	else
-		load_database(self)
-
-		if not authstruct then
-			errtxt = "Could not load authentication database"
-		else	
-			local id = get_id(userid)
-			if not id then
-				errtxt = "Userid not found"
-			elseif id.password ~= md5.sumhexa(password) then
-				errtxt = "Invalid password"
-			end
+		local id = get_id(self, userid)
+		
+		if not id then
+			errtxt = "Userid not found"
+		elseif id.password ~= md5.sumhexa(password) then
+			errtxt = "Invalid password"
 		end
 	end
 
@@ -149,133 +84,59 @@ end
 
 -- This function returns the username, roles, ...
 get_userinfo = function(self, userid)
-	load_database(self)
-	local result = {}
-	result.userid = cfe({ value=userid, label="User id" })
-	result.username = cfe({ label="Real name" })
-	local id = get_id(userid)
-	if id then
-		result.username.value = id.username
-	elseif userid then
-		result.userid.errtxt = "User does not exist"
-	end
-	result.password = cfe({ label="Password" })
-	result.password_confirm = cfe({ label="Password (confirm)" })
-	result.roles = get_userinfo_roles(self, userid)
-	result.skin = get_userinfo_skin(self, userid)
-
-	return cfe({ type="group", value=result, label="User Config" })
-end
-
-get_userinfo_roles = function(self, userid)
-	load_database(self)
-	local id = get_id(userid)
-	local roles = cfe({ type="multi", value={}, label="Roles", option={} })
+	local id = get_id(self, userid)
 	if id then
-		for x in string.gmatch(id.roles or "", "([^,]+),?") do
-			roles.value[#roles.value + 1] = x
+		-- Make a copy so roles don't get changed in the authstruct
+		local result = {}
+		for n,v in pairs(id) do
+			result[n]=v
 		end
-	elseif userid then
-		roles.errtxt = "Could not load roles"
-	end
-	local rol = require("roles")
-	if rol then
-		local avail_roles = rol.list_all_roles(self)
-		for x,role in ipairs(avail_roles) do
-			if role==rol.guest_role then
-				table.remove(avail_roles,x)
-				break
-			end
+		local tmp = {}
+		for x in string.gmatch(id.roles or "", "([^,]+),?") do
+			tmp[#tmp + 1] = x
 		end
-		roles.option = avail_roles
-	end
-	return roles
-end
-
-get_userinfo_skin = function(self, userid)
-	load_database(self)
-	local id = get_id(userid)
-	local skin = cfe({ type="select", value="", label="Skin", option={""} })
-	if id then
-		skin.value = id.skin or skin.value
-	elseif userid then
-		skin.errtxt = "Could not load skin"
-	end
-	-- Call into skins controller to get the list of skins
-	local contrl = self:new("acf-util/skins")
-	local skins = contrl:read()
-	contrl:destroy()
-	for i,s in ipairs(skins.value) do
-		skin.option[#skin.option + 1] = s.value
+		result.roles = tmp
+		return result
 	end
-	table.sort(skin.option)
-	return skin
+	return nil
 end
 
-list_users = function (self)
-	load_database(self)
-	local output = {}
-	if authstruct then
-		for k,v in pairs(authstruct) do
-			table.insert(output,v.userid)
-		end
+write_userinfo = function(self, userinfo)
+	if not userinfo or not userinfo.userid or userinfo.userid == "" then
+		return false
 	end
-	return output
-end
+	id = get_id(self, userinfo.userid) or {}
+	-- Username, password, roles, skin are allowed to not exist, just leave the same
+	id.userid = userinfo.userid
+	if userinfo.username then id.username = userinfo.username end
+	if userinfo.password then id.password = md5.sumhexa(userinfo.password) end
+	if userinfo.roles then id.roles = table.concat(userinfo.roles, ",") end
+	if userinfo.skin then id.skin = userinfo.skin end
 
--- For an existing user, change the settings that are non-nil
-change_settings = function (self, settings)
-	local success, settings = validate_settings(settings)
+	local success = auth.write_entry(self, usertable, "", id.userid, (id.password or "")..":"..(id.username or "")..":"..(id.roles or "")..":"..(id.skin or ""))
+	authstruct[userinfo.userid] = nil
+	get_id(self, id.userid)
 
-	-- Get the current user info
-	local id
-	if success then
-		load_database(self)
-		id = get_id(settings.value.userid.value)
-		if not id then
-			settings.value.userid.errtxt = "This userid does not exist!"
-			success = false
+	if success and self.sessiondata and self.sessiondata.userinfo and self.sessiondata.userinfo.userid == id.userid then
+		self.sessiondata.userinfo = {}
+		for name,value in pairs(id) do
+			self.sessiondata.userinfo[name] = value
 		end
 	end
 
-	if success then
-		success = write_settings(self, settings, id)
-	end
-
-	if not success then
-		settings.errtxt = "Failed to save settings"
-	end
-
-	return settings
+	return success
 end
-
-new_settings = function (self, settings)
-	local success, settings = validate_settings(settings)
-
-	if success then
-		load_database(self)
-		local id = get_id(settings.value.userid.value)
-		if id then
-			settings.value.userid.errtxt = "This userid already exists!"
-			success = false
-		end
-	end
-
-	if success then
-		success = write_settings(self, settings)
-	end
-
-	if not success then
-		settings.errtxt = "Failed to create new user"
+	
+list_users = function (self)
+	load_database(self)
+	local output = {}
+	for k in pairs(authstruct) do
+		table.insert(output,k)
 	end
-
-	return settings
+	return output
 end
 
 delete_user = function (self, userid)
-	local cmdresult = "Failed to delete user"
-	if auth.delete_entry(self, usertable, "", userid) then
-		cmdresult = "User deleted"
-	end
-	return cfe({ value=cmdresult, label="Delete user result" })
+	authstruct[userid] = nil	
+	return auth.delete_entry(self, usertable, "", userid)
 end
diff --git a/lib/roles.lua b/lib/roles.lua
index a07d290..e5786ba 100644
--- a/lib/roles.lua
+++ b/lib/roles.lua
@@ -8,17 +8,22 @@ module (..., package.seeall)
 
 guest_role = "GUEST"
 
+-- Global variables so we don't have to figure out all the roles multiple times
+local defined_roles, default_roles, reverseroles, roles_candidates, role_table
+
 -- returns a table of the *.roles files
 -- startdir should be the app dir
 local get_roles_candidates = function(self)
-	local list = {}
-	for p in string.gmatch(self.conf.appdir, "[^,]+") do
-		local l = fs.find_files_as_array(".*%.roles", p, true) or {}
-		for i,f in ipairs(l) do
-			list[#list+1] = f
+	if not roles_candidates then
+		roles_candidates = {}
+		for p in string.gmatch(self.conf.appdir, "[^,]+") do
+			local l = fs.find_files_as_array(".*%.roles", p, true) or {}
+			for i,f in ipairs(l) do
+				roles_candidates[#roles_candidates+1] = f
+			end
 		end
 	end
-	return list
+	return roles_candidates
 end
 
 -- Return a list of *controller.lua files
@@ -90,52 +95,61 @@ get_controllers_view = function(self,controller_info)
 end
 
 list_default_roles = function(self)
-	local default_roles = {}
-	local reverseroles = {}
+	if not default_roles then
+		default_roles = {}
+		reverseroles = {}
 
-	-- find all of the default roles files and parse them
-	local rolesfiles = get_roles_candidates(self)
+		-- find all of the default roles files and parse them
+		local rolesfiles = get_roles_candidates(self)
 
-	for x,file in ipairs(rolesfiles) do
-		f = fs.read_file_as_array(file) or {}
-		local rolefile = string.match(file, "(/[^/]+/[^/]+)%.roles$")
-		for y,line in pairs(f) do
-			local role = string.match(line,"^[%w_]+")
-			if role then
-				if not reverseroles[rolefile.."/"..role] then
-					default_roles[#default_roles+1] = rolefile.."/"..role
-					reverseroles[default_roles[#default_roles]] = #default_roles
-				end
-				if not reverseroles[role] then
-					default_roles[#default_roles+1] = role
-					reverseroles[default_roles[#default_roles]] = #default_roles
+		for x,file in ipairs(rolesfiles) do
+			f = fs.read_file_as_array(file) or {}
+			local rolefile = string.match(file, "(/[^/]+/[^/]+)%.roles$")
+			for y,line in pairs(f) do
+				local role = string.match(line,"^[%w_]+")
+				if role then
+					if not reverseroles[rolefile.."/"..role] then
+						default_roles[#default_roles+1] = rolefile.."/"..role
+						reverseroles[default_roles[#default_roles]] = #default_roles
+					end
+					if not reverseroles[role] then
+						default_roles[#default_roles+1] = role
+						reverseroles[default_roles[#default_roles]] = #default_roles
+					end
 				end
 			end
 		end
-	end
 
-	table.sort(default_roles, function(a,b)
-			if string.byte(a, 1) == 47 and string.byte(b,1) ~= 47 then return false
-			elseif string.byte(a, 1) ~= 47 and string.byte(b,1) == 47 then return true
-			else return a<b 
-			end
-		end)
+		table.sort(default_roles, function(a,b)
+				if string.byte(a, 1) == 47 and string.byte(b,1) ~= 47 then return false
+				elseif string.byte(a, 1) ~= 47 and string.byte(b,1) == 47 then return true
+				else return a<b 
+				end
+			end)
+	end
 
 	return default_roles, reverseroles
 end
 
-list_roles = function(self)
-	local defined_roles = {}
-	local default_roles, reverseroles = list_default_roles(self)
-
-	-- Open the roles file and parse for defined roles
-	local entries = authenticator.auth.read_field(self, authenticator.roletable, "") or {}
-	for x,entry in ipairs(entries) do
-		if not reverseroles[entry.id] then
-			defined_roles[#defined_roles + 1] = entry.id
+list_defined_roles = function(self)
+	if not defined_roles then
+		-- Open the roles file and parse for defined roles
+		defined_roles = {}
+		if not role_table then role_table = authenticator.auth.read_field(self, authenticator.roletable, "") or {} end
+		for x,entry in ipairs(role_table) do
+			if not reverseroles[entry.id] then
+				defined_roles[#defined_roles + 1] = entry.id
+			end
 		end
+		table.sort(defined_roles)
 	end
-	table.sort(defined_roles)
+
+	return defined_roles
+end
+
+list_roles = function(self)
+	local default_roles = list_default_roles(self)
+	local defined_roles = list_defined_roles(self)
 
 	return defined_roles, default_roles
 end
@@ -195,8 +209,8 @@ local determine_perms = function(self,roles)
 	end
 
 	-- then look in the user-editable roles
-	local entries = authenticator.auth.read_field(self, authenticator.roletable, "") or {}
-	for x,entry in ipairs(entries) do
+	if not role_table then role_table = authenticator.auth.read_field(self, authenticator.roletable, "") or {} end
+	for x,entry in ipairs(role_table) do
 		if reverseroles[entry.id] then
 			temp = format.string_to_table(entry.entry, ",")
 			for z,perm in pairs(temp) do