Implement createpasswdentry and editpasswd actions

There are known problems with handling blank passwords and redirecting to viewpasswdfile

4 files changed, 249 insertions, 102 deletions

diff --git a/freeradius3-controller.lua b/freeradius3-controller.lua
index 5184ac4..ce6dfd9 100644
--- a/freeradius3-controller.lua
+++ b/freeradius3-controller.lua
@@ -38,4 +38,12 @@ function mymodule.editpasswdentry(self)
 	return self.handle_form(self, self.model.get_passwd_entry, self.model.update_passwd_entry, self.clientdata, "Save", "Edit Passwd Entry", "Entry Saved")
 end
 
+function mymodule.createpasswdentry(self)
+	return self.handle_form(self, self.model.get_new_passwd_entry, self.model.create_passwd_entry, self.clientdata, "Create", "Create Passwd Entry", "Entry Created")
+end
+
+function mymodule.editpasswd(self)
+	return self.handle_form(self, self.model.get_passwd, self.model.update_passwd, self.clientdata, "Save", "Edit Password", "Password Saved")
+end
+
 return mymodule
diff --git a/freeradius3-model.lua b/freeradius3-model.lua
index 1879437..fdc246c 100644
--- a/freeradius3-model.lua
+++ b/freeradius3-model.lua
@@ -166,6 +166,136 @@ local mksalt = function()
 	return str
 end
 
+local get_passwd_file = function(self, clientdata)
+	local retval = cfe({ type="group", value={}, label="Freeradius passwd file" })
+	retval.value.filename = cfe({ type="select", label="File name", option ={}, key=true, seq=1 })
+	local files,configs,passwdconfig
+	files,retval.errtxt,configs = get_passwd_files()
+	if files then
+		retval.value.filename.option = files
+		self.handle_clientdata(retval, clientdata)
+		if retval.value.filename.value then
+			retval.value.filename.errtxt = "Invalid selection"
+			for i,f in ipairs(files) do
+				if f == retval.value.filename.value then
+					retval.value.filename.errtxt = nil
+					retval.value.filename.readonly = true
+					passwdconfig = parse_passwd_config(configs[i])
+					break
+				end
+			end
+		end
+	end
+
+	-- return the CFE structure and the config info for this filename
+	return retval, passwdconfig
+end
+
+local get_passwd_entry_private = function(self, clientdata, create)
+	local retval,passwdconfig = get_passwd_file(self, clientdata)
+	retval.label = "Freeradius passwd entry"
+	local entry = 0
+	local entryline = {}
+	if not create then
+		retval.value.entry = cfe({ label="Entry index", key=true, seq=2 })
+		self.handle_clientdata(retval, clientdata)
+		entry = tonumber(retval.value.entry.value) or 0
+	end
+	local hash
+	if passwdconfig then
+		-- The filename is valid, and we should create the fields 
+		retval.value.fields = cfe({ type="group", value={}, label="Fields", seq=3, delimiter=passwdconfig.delimiter })
+		if not create then retval.value.entry.errtxt = "Invalid entry" end
+		local content = fs.read_file(retval.value.filename.value) or ""
+		local maxcount = 0
+		local i=1
+		for line in string.gmatch(content, "[^\n]+") do
+			local _,count = string.gsub(line, format.escapemagiccharacters(passwdconfig.delimiter), " ")
+			maxcount = math.max(maxcount, count)
+			if i == entry then
+				retval.value.entry.readonly = true
+				retval.value.entry.errtxt = nil
+				entryline = format.string_to_table(line, format.escapemagiccharacters(passwdconfig.delimiter))
+			end
+			i = i+1
+		end
+		for i=1, math.max(#passwdconfig.fields, maxcount+1), 1 do
+			local label = passwdconfig.fields[i]
+			if not label or label == "" then
+				label = "Unlabeled field "..i
+			end
+			if label == "Crypt-Password" then
+				hash = entryline[i]
+				-- We do not return the encrypted password, but will leave unchanged if blank
+				retval.value.fields.value[tostring(i)] = cfe({ type="password", label=label, seq=i })
+				-- FIXME - this leads to inconsistent handling of blank password since this function is reused
+				if not create then retval.value.fields.value[tostring(i)].descr="Leave blank to leave unchanged" end
+				retval.value.fields.value["algorithm"..i] = cfe({ type="select", value="$6$", label="Algorithm", option={{value="", label="DES"}, {value="$1$", label="MD5"}, {value="$2$", label="Blowfish"}, {value="$2a$", label="eksblowfish"}, {value="$5$", label="SHA-256"}, {value="$6$", label="SHA-512"}}, seq=i })
+				-- Hide the algorithm so user does not use insecure algorithms unless they REALLY want to
+				retval.value.fields.value["algorithm"..i].type = "hidden"
+			else
+				retval.value.fields.value[tostring(i)] = cfe({ label=label, value=entryline[i] or "", seq=i })
+			end
+		end
+	end
+	return retval, hash
+end
+
+local update_passwd_entry_private = function(self, entry, create)
+	-- The password/index fields have already been validated
+	if not entry.value.fields then
+		entry.errtxt = "Invalid passwd entry"
+	else
+		-- The only fields we can validate are the password algorithms
+		-- Don't search for 'select' cfe's because they have been changed to hidden
+		for n,v in pairs(entry.value.fields.value) do
+			if v.option and not modelfunctions.validateselect(v) then
+				entry.errtxt = "Invalid passwd entry"				
+			end
+		end
+		if not entry.errtxt then
+			-- Set the value
+			local content = fs.read_file_as_array(entry.value.filename.value) or {}
+			local values = {}
+			for n,v in pairs(entry.value.fields.value) do
+				-- FIXME - this leads to inconsistent handling of blank password since this function is reused
+				if v.type=="password" and v.value=="" then
+					-- Keep the same password
+					local line = {}
+					if not create then line = format.string_to_table(content[tonumber(entry.value.entry.value)], entry.value.fields.delimiter) or {} end
+					values[tonumber(n)] = line[tonumber(n)] or ""
+				elseif v.type=="password" then
+					local salt = entry.value.fields.value["algorithm"..n].value
+					if salt == "" then
+						salt = string.sub(mksalt(), 1, 2)
+					else
+						salt = salt..(mksalt() or "").."$"
+					end
+					local crypt,errtxt = posix.crypt(v.value, salt)
+					if crypt then
+						values[tonumber(n)] = crypt
+					else
+						v.errtxt = errtxt
+						entry.errtxt = "Invalid passwd entry"
+						break
+					end
+				elseif tonumber(n) then
+					values[tonumber(n)] = v.value
+				end
+			end
+			if not entry.errtxt then
+				if create then
+					content[#content+1] = table.concat(values, entry.value.fields.delimiter)
+				else
+					content[tonumber(entry.value.entry.value)] = table.concat(values, entry.value.fields.delimiter)
+				end
+				fs.write_file(entry.value.filename.value, table.concat(content, "\n"))
+			end
+		end
+	end
+	return entry
+end
+
 -- ################################################################################
 -- PUBLIC FUNCTIONS
 
@@ -274,31 +404,6 @@ function mymodule.list_passwd_files()
 	return cfe({ type="structure", value=retval, label="List of Freeradius passwd files", errtxt=errtxt })
 end
 
-local get_passwd_file = function(self, clientdata)
-	local retval = cfe({ type="group", value={}, label="Freeradius passwd file" })
-	retval.value.filename = cfe({ type="select", label="File name", option ={}, key=true, seq=1 })
-	local files,configs,passwdconfig
-	files,retval.errtxt,configs = get_passwd_files()
-	if files then
-		retval.value.filename.option = files
-		self.handle_clientdata(retval, clientdata)
-		if retval.value.filename.value then
-			retval.value.filename.errtxt = "Invalid selection"
-			for i,f in ipairs(files) do
-				if f == retval.value.filename.value then
-					retval.value.filename.errtxt = nil
-					retval.value.filename.readonly = true
-					passwdconfig = parse_passwd_config(configs[i])
-					break
-				end
-			end
-		end
-	end
-
-	-- return the CFE structure and the config info for this filename
-	return retval, passwdconfig
-end
-
 function mymodule.view_passwd_file(self, clientdata)
 	local retval,passwdconfig = get_passwd_file(self, clientdata)
 	if passwdconfig then
@@ -314,95 +419,116 @@ function mymodule.view_passwd_file(self, clientdata)
 end
 
 function mymodule.get_passwd_entry(self, clientdata)
+	return (get_passwd_entry_private(self, clientdata, false))
+end
+
+function mymodule.update_passwd_entry(self, entry)
+	return update_passwd_entry_private(self, entry, false)
+end
+
+function mymodule.get_new_passwd_entry(self, clientdata)
+	return (get_passwd_entry_private(self, clientdata, true))
+end
+
+function mymodule.create_passwd_entry(self, entry)
+	return update_passwd_entry_private(self, entry, true)
+end
+
+function mymodule.get_passwd(self, clientdata)
 	local retval,passwdconfig = get_passwd_file(self, clientdata)
-	retval.label = "Freeradius passwd entry"
+	retval.label = "Freeradius password"
 	retval.value.entry = cfe({ label="Entry index", key=true, seq=2 })
 	self.handle_clientdata(retval, clientdata)
 	if passwdconfig then
-		-- The filename is valid, and we should create the fields 
-		retval.value.fields = cfe({ type="group", value={}, label="Fields", seq=3, delimiter=passwdconfig.delimiter })
-		retval.value.entry.errtxt = "Invalid entry"
-		local content = fs.read_file(retval.value.filename.value) or ""
-		local maxcount = 0
-		local entry = tonumber(retval.value.entry.value) or 0
-		local entryline = ""
-		local i=1
-		for line in string.gmatch(content, "[^\n]+") do
-			local _,count = string.gsub(line, format.escapemagiccharacters(passwdconfig.delimiter), " ")
-			maxcount = math.max(maxcount, count)
-			if i == entry then
-				retval.value.entry.readonly = true
-				retval.value.entry.errtxt = nil
-				entryline = format.string_to_table(line, format.escapemagiccharacters(passwdconfig.delimiter))
+		local success = false
+		-- The filename is valid, need to validate the entry id and presense of "Crypt-Password" field
+		local passwordfield = 0
+		local usernamefield = 0
+		retval.value.filename.errtxt = "No password field present"
+		for i,f in ipairs(passwdconfig.fields) do
+			if f == "Crypt-Password" then
+				success = true
+				passwordfield = i
+				retval.value.filename.errtxt = nil
+			elseif f == "User-Name" then
+				usernamefield = i
 			end
-			i = i+1
 		end
-		for i=1, math.max(#passwdconfig.fields, maxcount+1), 1 do
-			local label = passwdconfig.fields[i]
-			if not label or label == "" then
-				label = "Unlabeled field "..i
+			
+		local contenttable = fs.read_file_as_array(retval.value.filename.value) or {}
+		local entry = tonumber(retval.value.entry.value) or 0
+		if contenttable[entry] then
+			local entryline = format.string_to_table(contenttable[entry], format.escapemagiccharacters(passwdconfig.delimiter))
+			if 0 < passwordfield then
+				if not entryline[passwordfield] or entryline[passwordfield] == "" then
+					success = false
+					retval.value.entry.errtxt = "Password access disabled for this entry"
+				else
+					retval.value.entry.readonly = true
+				end
 			end
-			if label == "Crypt-Password" then
-				-- We do not return the encrypted password, but will leave unchanged if blank
-				retval.value.fields.value[tostring(i)] = cfe({ type="password", label=label, descr="Leave blank to leave unchanged", seq=i })
-				retval.value.fields.value["algorithm"..i] = cfe({ type="select", value="$6$", label="Algorithm", option={{value="", label="DES"}, {value="$1$", label="MD5"}, {value="$2$", label="Blowfish"}, {value="$2a$", label="eksblowfish"}, {value="$5$", label="SHA-256"}, {value="$6$", label="SHA-512"}}, seq=i })
-				-- Hide the algorithm so user does not use insecure algorithms unless they REALLY want to
-				retval.value.fields.value["algorithm"..i].type = "hidden"
-			else
-				retval.value.fields.value[tostring(i)] = cfe({ label=label, value=entryline[i] or "", seq=i })
+			if 0 < usernamefield then
+				retval.value.username = cfe({ value=entryline[usernamefield] or "", label="User-Name", readonly=true, seq=3 })
 			end
+		else
+			success = false
+			retval.value.entry.errtxt = "Invalid entry"
+		end
+		if success then
+			retval.value.oldpassword = cfe({ type="password", label="Current Password", seq=4 })
+			retval.value.password = cfe({ type="password", label="New Password", seq=5 })
+			retval.value.password_confirm = cfe({ type="password", label="New Password (confirm)", seq=6 })
+			retval.value.algorithm = cfe({ type="select", value="$6$", label="Algorithm", option={{value="", label="DES"}, {value="$1$", label="MD5"}, {value="$2$", label="Blowfish"}, {value="$2a$", label="eksblowfish"}, {value="$5$", label="SHA-256"}, {value="$6$", label="SHA-512"}}, seq=i })
+			-- Hide the algorithm so user does not use insecure algorithms unless they REALLY want to
+			retval.value.algorithm.type = "hidden"
 		end
 	end
 	return retval
 end
 
-function mymodule.update_passwd_entry(self, entry)
+function mymodule.update_passwd(self, passwd)
 	-- The password/index fields have already been validated
-	if not entry.value.fields then
-		entry.errtxt = "Invalid passwd entry"
+	if not passwd.value.password then
+		passwd.errtxt = "Invalid passwd entry"
 	else
-		-- The only fields we can validate are the password algorithms
-		-- Don't search for 'select' cfe's because they have been changed to hidden
-		for n,v in pairs(entry.value.fields.value) do
-			if v.option and not modelfunctions.validateselect(v) then
-				entry.errtxt = "Invalid passwd entry"				
-			end
+		-- Get the entry form and current password hash
+		local form,pwhash = get_passwd_entry_private(self, {filename=passwd.value.filename.value, entry=passwd.value.entry.value}, false)
+
+		-- Validate the old password
+		local success = false
+		passwd.value.oldpassword.errtxt = "Incorrect password"
+		local algo_salt, hash = string.match(pwhash, "^(%$%d%$[a-zA-Z0-9./]+%$)(.*)")
+	        if algo_salt ~= nil and hash ~= nil then
+	                if (pwhash == posix.crypt(passwd.value.oldpassword.value, algo_salt)) then
+				success = true
+				passwd.value.oldpassword.errtxt = nil
+		        end
 		end
-		if not entry.errtxt then
-			-- Set the value
-			local content = fs.read_file_as_array(entry.value.filename.value) or {}
-			local values = {}
-			for n,v in pairs(entry.value.fields.value) do
-				if v.type=="password" and v.value=="" then
-					-- Keep the same password
-					local line = format.string_to_table(content[tonumber(entry.value.entry.value)], entry.value.fields.delimiter) or {}
-					values[tonumber(n)] = line[tonumber(n)] or ""
-				elseif v.type=="password" then
-					local salt = entry.value.fields.value["algorithm"..n].value
-					if salt == "" then
-						salt = string.sub(mksalt(), 1, 2)
-					else
-						salt = salt..(mksalt() or "").."$"
-					end
-					local crypt,errtxt = posix.crypt(v.value, salt)
-					if crypt then
-						values[tonumber(n)] = crypt
-					else
-						v.errtxt = errtxt
-						entry.errtxt = "Invalid passwd entry"
-						break
-					end
-				elseif tonumber(n) then
-					values[tonumber(n)] = v.value
+
+		-- Validate the new password
+		if passwd.value.password.value ~= passwd.value.password_confirm.value then
+			success = false
+			passwd.value.password_confirm.errtxt = "Must match password"
+		end
+
+		-- Validate the algorithm
+		success = modelfunctions.validateselect(passwd.value.algorithm) and success
+
+		if success then
+			for n,f in pairs(form.value.fields.value) do
+				if f.label == "Crypt-Password" then
+					f.value = passwd.value.password.value
+					form.value.fields.value["algorithm"..n].value = passwd.value.algorithm.value
+					break
 				end
 			end
-			if not entry.errtxt then
-				content[tonumber(entry.value.entry.value)] = table.concat(values, entry.value.fields.delimiter)
-				fs.write_file(entry.value.filename.value, table.concat(content, "\n"))
-			end
+			form = update_passwd_entry_private(self, form, false)
+			passwd.errtxt = form.errtxt
+		else
+			passwd.errtxt = "Failed to set password"
 		end
 	end
-	return entry
+	return passwd
 end
 
 return mymodule
diff --git a/freeradius3-viewpasswdfile-html.lsp b/freeradius3-viewpasswdfile-html.lsp
index b4e8ce5..d6cc3eb 100644
--- a/freeradius3-viewpasswdfile-html.lsp
+++ b/freeradius3-viewpasswdfile-html.lsp
@@ -21,25 +21,29 @@ html = require("acf.html")
 	});
 </script>
 
-<% htmlviewfunctions.displaycommandresults({"editpasswdentry"}, session) %>
+<% htmlviewfunctions.displaycommandresults({"editpasswdentry", "editpasswd"}, session) %>
+<% htmlviewfunctions.displaycommandresults({"createpasswdentry"}, session, true) %>
+
+<% local redir = cfe({ type="hidden", value=page_info.orig_action }) %>
+<% -- This is a hack to redirect back to viewing the same file
+redir.value = redir.value.."?filename="..html.url_encode(view.value.filename.value)
+%>
 
 <% local header_level = htmlviewfunctions.displaysectionstart(view, page_info) %>
 <% htmlviewfunctions.displayitem(view.value.filename) %>
 <% if view.value.data then %>
+<% local containspasswd = 0 %>
 <table id="list" class="tablesorter"><thead>
 	<tr>
 		<th>Action</th>
 <% for i,f in ipairs(view.value.fields.value) do %>
+		<% if f == "Crypt-Password" then containspasswd = i end %>
 		<th><%= html.html_escape(f) %></th>
 <% end %>
 	</tr>
 </thead><tbody>
 <% local filename = cfe({ type="hidden", value=view.value.filename.value }) %>
 <% local entry = cfe({ type="hidden", value="" }) %>
-<% local redir = cfe({ type="hidden", value=page_info.orig_action }) %>
-<% -- This is a hack to redirect back to viewing the same file
-redir.value = redir.value.."?filename="..html.url_encode(view.value.filename.value)
-%>
 <% for i,r in ipairs( view.value.data.value ) do %>
 	<tr>
 		<td>
@@ -47,12 +51,21 @@ redir.value = redir.value.."?filename="..html.url_encode(view.value.filename.val
 		<% if viewlibrary.check_permission("editpasswdentry") then %>
 			<% htmlviewfunctions.displayitem(cfe({type="link", value={filename=filename, entry=entry, redir=redir}, label="", option="Edit", action="editpasswdentry"}), page_info, -1) %>
 		<% end %>
+		<% if 0 < containspasswd and r[containspasswd] ~= "" and viewlibrary.check_permission("editpasswd") then %>
+			<% htmlviewfunctions.displayitem(cfe({type="link", value={filename=filename, entry=entry, redir=redir}, label="", option="Change Pass", action="editpasswd"}), page_info, -1) %>
+		<% end %>
 		</td>
 <% for j,f in ipairs(r) do %>
-		<td><%= html.html_escape(f) %></td>
+		<td><% if (j == containspasswd) and (f ~= "") then io.write("********") else io.write(html.html_escape(f)) end %></td>
 <% end %>
 	</tr>
 <% end %>
 </tbody></table>
 <% end %>
+
+<% if view.value.data and viewlibrary and viewlibrary.dispatch_component and viewlibrary.check_permission("createpasswdentry") then
+	local createform = viewlibrary.dispatch_component("createpasswdentry", {filename=view.value.filename.value, redir=redir.value}, true)
+	createform.action = page_info.script .. page_info.prefix .. page_info.controller .. "/createpasswdentry"
+	htmlviewfunctions.displayitem(createform, page_info, htmlviewfunctions.incrementheader(header_level))
+end %>
 <% htmlviewfunctions.displaysectionend(header_level) %>
diff --git a/freeradius3.roles b/freeradius3.roles
index 501d761..3871d59 100644
--- a/freeradius3.roles
+++ b/freeradius3.roles
@@ -1,3 +1,3 @@
 USER=freeradius3:status,freeradius3:startstop,freeradius3:logfile
-EXPERT=freeradius3:listfiles,freeradius3:editfile,freeradius3:createfile,freeradius3:deletefile,freeradius3:listpasswdfiles,freeradius3:viewpasswdfile,freeradius3:editpasswdentry
-ADMIN=freeradius3:status,freeradius3:startstop,freeradius3:logfile,freeradius3:listfiles,freeradius3:editfile,freeradius3:createfile,freeradius3:deletefile,freeradius3:listpasswdfiles,freeradius3:viewpasswdfile,freeradius3:editpasswdentry
+EXPERT=freeradius3:listfiles,freeradius3:editfile,freeradius3:createfile,freeradius3:deletefile,freeradius3:listpasswdfiles,freeradius3:viewpasswdfile,freeradius3:editpasswdentry,freeradius3:createpasswdentry,freeradius3:editpasswd
+ADMIN=freeradius3:status,freeradius3:startstop,freeradius3:logfile,freeradius3:listfiles,freeradius3:editfile,freeradius3:createfile,freeradius3:deletefile,freeradius3:listpasswdfiles,freeradius3:viewpasswdfile,freeradius3:editpasswdentry,freeradius3:createpasswdentry,freeradius3:editpasswd