Cleanup authenticator subauths and limit authenticator-plaintext to traverse one directory level

Fix subauths to properly list fields/entries and delete from all subauths

2 files changed, 30 insertions, 19 deletions

diff --git a/lib/authenticator-plaintext.lua b/lib/authenticator-plaintext.lua
index 7a5dbea..07a8e62 100644
--- a/lib/authenticator-plaintext.lua
+++ b/lib/authenticator-plaintext.lua
@@ -20,8 +20,9 @@ mymodule.list_fields = function(self, tabl)
 
 	local fields = {}
 	for file in fs.find(".*"..format.escapemagiccharacters(tabl), self.conf.confdir) do
-		local field = string.match(file, "([^/]*)"..format.escapemagiccharacters(tabl).."$") or ""
-		if fs.is_file(file) and field ~= "" then
+		local field = string.match(file, self.conf.confdir.."(.*)"..format.escapemagiccharacters(tabl).."$")
+		-- We only allow one level of directory traversal
+		if field and fs.is_file(file) and not string.find(field, "/.*/") then
 			fields[#fields + 1] = field
 		end
 	end
@@ -29,7 +30,7 @@ mymodule.list_fields = function(self, tabl)
 end
 
 mymodule.read_field = function(self, tabl, field)
-	if not self or not tabl or tabl == "" or not field then
+	if not self or not tabl or tabl == "" or not field or string.find(field, "^..*/.*/") then
 		return nil
 	end
 
@@ -55,7 +56,7 @@ mymodule.read_field = function(self, tabl, field)
 end
 
 mymodule.delete_field = function(self, tabl, field)
-	if not self or not tabl or tabl == "" or not field then
+	if not self or not tabl or tabl == "" or not field or string.find(field, "^..*/.*/") then
 		return false
 	end
 	local passwd_path = self.conf.confdir .. field .. tabl
@@ -64,7 +65,7 @@ mymodule.delete_field = function(self, tabl, field)
 end
 
 mymodule.write_entry = function(self, tabl, field, id, entry)
-	if not self or not tabl or tabl == "" or not field or not id or not entry then
+	if not self or not tabl or tabl == "" or not field or string.find(field, "^..*/.*/") or not id or not entry then
 		return false
 	end
 
@@ -85,7 +86,7 @@ mymodule.write_entry = function(self, tabl, field, id, entry)
 end
 
 mymodule.read_entry = function(self, tabl, field, id)
-	if not self or not tabl or tabl == "" or not field or not id then
+	if not self or not tabl or tabl == "" or not field or string.find(field, "^..*/.*/") or not id then
 		return nil
 	end
 	-- Set path to passwordfile
@@ -101,7 +102,7 @@ mymodule.read_entry = function(self, tabl, field, id)
 end
 
 mymodule.delete_entry = function (self, tabl, field, id)
-	if not self or not tabl or tabl == "" or not field or not id then
+	if not self or not tabl or tabl == "" or not field or string.find(field, "^..*/.*/") or not id then
 		return false
 	end
 	local result = false
diff --git a/lib/authenticator.lua b/lib/authenticator.lua
index 975d0e6..bce2af7 100644
--- a/lib/authenticator.lua
+++ b/lib/authenticator.lua
@@ -11,6 +11,8 @@ session = require("session")
 
 -- This is the sub-authenticator
 local auth = {}
+
+-- List all fields, combining all subauths
 auth.list_fields = function(self, tabl)
 	if not auth.subauths then
 		return nil
@@ -19,41 +21,48 @@ auth.list_fields = function(self, tabl)
 	local revfields = {}
 	for i,sub in ipairs(auth.subauths) do
 		local subf = sub.list_fields(self, tabl)
-		for j,f in ipairs(subf) do
+		for j,f in ipairs(subf or {}) do
 			if not revfields[f] then
 				fields[#fields+1] = f
-				revfields[#revfields+1] = #fields
+				revfields[f] = #fields
 			end
 		end
 	end
 	return fields
 end
 
+-- Read all entries from field, combining all subauths (first entry for id takes precedence)
 auth.read_field = function(self, tabl, field)
 	if not auth.subauths then
 		return nil
 	end
+	local entries = {}
+	local reventries = {}
 	for i,sub in ipairs(auth.subauths) do
 		local f = sub.read_field(self, tabl, field)
-		if f then
-			return f
+		for j,a in ipairs(f or {}) do
+			if not reventries[a.id] then
+				entries[#entries+1] = a
+				reventries[a.id] = i
+			end
 		end
 	end
-	return nil
+	return entries
 end
 
+-- Delete all entries in all subauths for specified field
 auth.delete_field = function(self, tabl, field)
 	if not auth.subauths then
 		return nil
 	end
 	for i,sub in ipairs(auth.subauths) do
-		if sub.delete_field(self, tabl, field) then
-			return true
-		end
+		sub.delete_field(self, tabl, field)
 	end
 	return false
 end
 
+-- Should we write to the first subauth that allows writes or to subauth where entry already exists?
+-- Chose to write to first subauth that allows writes because overrides all others
 auth.write_entry = function(self, tabl, field, id, entry)
 	if not auth.subauths then
 		return nil
@@ -66,6 +75,7 @@ auth.write_entry = function(self, tabl, field, id, entry)
 	return false
 end
 
+-- Return the details from the first match from one of the subauths
 auth.read_entry = function(self, tabl, field, id)
 	if not auth.subauths then
 		return nil
@@ -79,16 +89,15 @@ auth.read_entry = function(self, tabl, field, id)
 	return nil
 end
 
+-- Delete the entry from all subauths
 auth.delete_entry = function (self, tabl, field, id)
 	if not auth.subauths then
 		return nil
 	end
 	for i,sub in ipairs(auth.subauths) do
-		if sub.delete_entry(self, tabl, field, id) then
-			return true
-		end
+		sub.delete_entry(self, tabl, field, id)
 	end
-	return false
+	return true
 end
 
 -- Publicly define the pre-defined tables
@@ -190,6 +199,7 @@ mymodule.get_subauth = function(self)
 		if self and self.conf and self.conf.authenticator and self.conf.authenticator ~= "" then
 			for a in string.gmatch(self.conf.authenticator, "[^,]+") do
 				auth.subauths[#auth.subauths+1] = require(string.gsub(a, "%.lua$", ""))
+				auth.subauths[#auth.subauths].name = a
 			end
 		else
 			auth.subauths[1] = require("authenticator-plaintext")