Modify logon to move logic to the model, leaving redirect in the controller

2 files changed, 66 insertions, 55 deletions

diff --git a/app/acf-util/logon-controller.lua b/app/acf-util/logon-controller.lua
index bd26e10..cc4b6f2 100644
--- a/app/acf-util/logon-controller.lua
+++ b/app/acf-util/logon-controller.lua
@@ -4,7 +4,7 @@ local mymodule = {}
 
 mymodule.default_action = "status"
 
--- Logon a new user based upon id and password in clientdata
+-- If there are no users defined, default to creating new ADMIN user
 local check_users = function(self)
 	-- If there are no users defined, add privileges and dispatch password/newuser
 	local users = self.model:list_users()
@@ -13,6 +13,7 @@ local check_users = function(self)
 		self.sessiondata.permissions[self.conf.prefix].password.newuser = {"temp"}
 		self:dispatch(self.conf.prefix, "password", "newuser")
 		self.sessiondata.permissions[self.conf.prefix].password = nil
+		-- suppress the view, because the dispatch above has already handled the output
 		self.conf.suppress_view = true
 		return true
 	end
@@ -22,50 +23,45 @@ end
 
 -- Logon a new user based upon id and password in clientdata
 mymodule.logon = function(self)
-	local userid = cfe({ value=self.clientdata.userid or "", label="User ID", seq=1 })
-	local password = cfe({ type="password", label="Password", seq=2 })
-	local redir = cfe({ type="hidden", value=self.clientdata.redir, label="" })
-	local cmdresult = cfe({ type="form", value={userid=userid, password=password, redir=redir}, label="Logon", option="Logon" })
-	if self.clientdata.submit then
+	-- First, handle special case when no users are defined
+	if check_users(self) then return end
+
+	return self.handle_form(self, self.model.get_logon, function(self, cmdresult, submit)
+		-- We will handle the redirect here
+		-- The session will be cleared on a successful logon, so grab the logonredirect now
 		local logonredirect = self.sessiondata.logonredirect
-		local logon = self.model:logon(self.clientdata.userid, self.clientdata.password, self.conf.clientip, self.conf.sessiondir, self.sessiondata)
-		-- If successful logon, redirect to home or welcome page, otherwise try again
-		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)
-		if logon.value then
-			if redir.value == "" then
+		cmdresult = self.model.logon(self, cmdresult)
+		-- If successful logon, redirect to home or welcome page
+		if not cmdresult.errtxt then
+			local redir = self.clientdata.redir
+			if not redir or redir == "" then
 				if self.sessiondata.userinfo and self.sessiondata.userinfo.home and self.sessiondata.userinfo.home ~= "" then
-					redir.value = self.sessiondata.userinfo.home
+					redir = self.sessiondata.userinfo.home
 				elseif self.conf.home and self.conf.home ~= "" then
-					redir.value = self.conf.home
+					redir = self.conf.home
 				else
-					redir.value = "/acf-util/welcome/read"
+					redir = "/acf-util/welcome/read"
 				end
 			end
 			-- only copy the logonredirect if redirecting to that page
-			if logonredirect and cmdresult.value.redir.value then
-				local prefix, controller, action = self.parse_redir_string(cmdresult.value.redir.value)
+			if logonredirect and redir then
+				local prefix, controller, action = self.parse_redir_string(redir)
 				if logonredirect.action == action and logonredirect.controller == controller and logonredirect.prefix == prefix then
 					self.sessiondata.logonredirect = logonredirect
 				end
 			end
-			self:redirect(cmdresult.value.redir.value)
+			-- we always want a redirect will occur, but nothing is expecting a command result
+			-- so do the redirect here instead of in handle_form and don't pass any data
+			self:redirect(redir)
 		end
-	else
-		if check_users(self) then return end
-		cmdresult = self:redirect_to_referrer() or cmdresult
-	end
-	return cmdresult
+		return cmdresult
+	end, self.clientdata, "Logon", "Logon", "Logon Successful")
 end
 
 -- Log off current user and go to logon screen
 mymodule.logoff = function(self)
-	local logoff = self.model.logoff(self.conf.sessiondir, self.sessiondata)
+	-- This is an unusual action in that it does not require "submit" to take an action
+	local logoff = self.model.logoff(self)
 	-- We have to redirect so a new session / menu is created
 	self:redirect("logon")
 	return logoff
@@ -73,12 +69,7 @@ end
 
 -- Report the logon status
 mymodule.status = function(self)
-	local name = cfe({ label="User Name" })
-	local sessionid = cfe({ value=self.sessiondata.id or "", label="Session ID" })
-	if self.sessiondata.userinfo then
-		name.value = self.sessiondata.userinfo.username or ""
-	end
-	return cfe({ type="group", value={username=name, sessionid=sessionid}, label="Logon Status" })
+	return self.model.status(self)
 end
 
 return mymodule
diff --git a/app/acf-util/logon-model.lua b/app/acf-util/logon-model.lua
index 279e988..0cfba7f 100644
--- a/app/acf-util/logon-model.lua
+++ b/app/acf-util/logon-model.lua
@@ -8,53 +8,73 @@ fs = require ("acf.fs")
 roles = require ("roles")
 authenticator = require ("authenticator")
 
+-- Report the logon status
+mymodule.status = function(self)
+	local result = cfe({ type="group", value={}, label="Logon Status" })
+	result.value.username = cfe({ label="User Name" })
+	result.value.sessionid = cfe({ value=self.sessiondata.id or "", label="Session ID" })
+	if self.sessiondata.userinfo then
+		result.value.username.value = self.sessiondata.userinfo.username or ""
+	end
+	return result
+end
+
 -- Logoff the user by deleting session data
-mymodule.logoff = function (sessiondir, sessiondata)
+mymodule.logoff = function (self)
 	-- Unlink / delete the current session
-	local result = session.unlink_session(sessiondir, sessiondata.id)
+	local result = session.unlink_session(self.conf.sessiondir, self.sessiondata.id)
 	local success = (result ~= nil)
 	-- Clear the current session data
-	for a,b in pairs(sessiondata) do
-		sessiondata[a] = nil
+	for a,b in pairs(self.sessiondata) do
+		self.sessiondata[a] = nil
 	end
 
 	return cfe({ type="boolean", value=success, label="Logoff Success" })
 end
 
+mymodule.get_logon = function(self, clientdata)
+	local cmdresult = cfe({ type="group", value={}, label="Logon" })
+	cmdresult.value.userid = cfe({ value=self.clientdata.userid or "", label="User ID", seq=1 })
+	cmdresult.value.password = cfe({ type="password", label="Password", seq=2 })
+	cmdresult.value.redir = cfe({ type="hidden", value=self.clientdata.redir, label="" })
+	return cmdresult
+end
+
 -- Log on new user if possible and set up userinfo in session
 -- if we fail, we leave the session alone (don't log off)
-mymodule.logon = function (self, userid, password, ip_addr, sessiondir, sessiondata)
+mymodule.logon = function (self, logon)
+	logon.errtxt = "Logon Attempt Failed"
 	-- Check to see if we can log on this user id / ip addr
-	local countevent = session.count_events(sessiondir, userid, session.hash_ip_addr(ip_addr), self.conf.lockouttime, self.conf.lockouteventlimit)
+	local countevent = session.count_events(self.conf.sessiondir, logon.value.userid.value, session.hash_ip_addr(self.conf.clientip), self.conf.lockouttime, self.conf.lockouteventlimit)
 	if countevent then
-		session.record_event(sessiondir, userid, session.hash_ip_addr(ip_addr))
+		session.record_event(self.conf.sessiondir, logon.value.userid.value, session.hash_ip_addr(self.conf.clientip))
 	end
 
-	if false == countevent and userid and password then
-		if authenticator.authenticate (self, userid, password) then
+	if false == countevent then
+		if authenticator.authenticate (self, logon.value.userid.value, logon.value.password.value) then
 			-- We have a successful logon, change sessiondata
 			-- for some reason, can't call this function or it skips rest of logon
-			-- mymodule.logoff(sessiondir, sessiondata)
+			-- mymodule.logoff(self.conf.sessiondir, self.sessiondata)
 			---[[ so, do this instead
-			session.unlink_session(sessiondir, sessiondata.id)
+			session.unlink_session(self.conf.sessiondir, self.sessiondata.id)
 			-- Clear the current session data
-			for a,b in pairs(sessiondata) do
-				if a ~= "id" then sessiondata[a] = nil end
+			for a,b in pairs(self.sessiondata) do
+				if a ~= "id" then self.sessiondata[a] = nil end
 			end
 			--]]
-			sessiondata.id = session.random_hash(512)
-			local t = authenticator.get_userinfo (self, userid)
-			sessiondata.userinfo = {}
+			self.sessiondata.id = session.random_hash(512)
+			local t = authenticator.get_userinfo (self, logon.value.userid.value)
+			self.sessiondata.userinfo = {}
 			for name,value in pairs(t) do
-				sessiondata.userinfo[name] = value
+				self.sessiondata.userinfo[name] = value
 			end
-			return cfe({ type="boolean", value=true, label="Logon Success" })
+			logon.errtxt = nil
 		else
 			-- We have a bad logon, log the event
-			session.record_event(sessiondir, userid, session.hash_ip_addr(ip_addr))
+			session.record_event(self.conf.sessiondir, logon.value.userid.value, session.hash_ip_addr(self.conf.clientip))
 		end
 	end
-	return cfe({ type="boolean", value=false, label="Logon Success" })
+	return logon
 end
 
 mymodule.list_users = function(self)