From d6e7c62b84f16905d1446a0e4df644890402f6b4 Mon Sep 17 00:00:00 2001 From: Luke aka SwissalpS <161979+SwissalpS@users.noreply.github.com> Date: Tue, 31 Dec 2024 19:45:10 +0100 Subject: [PATCH] Updated 5.9.0 (#12) * Store hashes of passwords cleartext password storage is bad practice. * Depricate factions.get_password() returns nil after first run * loaded message * Properly use the configurable admin priv in output * Don't show password, since we can't anymore * remove code that is never reached * chown: reorder to first check if player has any factions at all There is no point in checking other params if this part fails. * chown: fix command signature password is required * proper admin priv listing in help for invite * wrap fixup code in do-block variable save_needed is not used for anything else * locale: many -> multiple * locale: remove unused entry * locale: ownership rephrasing * locale: tweak and add "No factions found." * locale: exists -> exist * locale: this -> that or better also fixed a french mistake: player doesn't own these -> player owns these * locale: reuse string for missing name besides, "nil" is a valid name. This way there is no confusion. * locale: reuse "missing player name" * locale: reuse "faction x doesn't exist" * locale: faction x already exists * locale: the player -> player x * locale: some more de-Frenching * add local is_admin stash commit... * disband: allow admin - permit admin to disband a faction without having any factions himself - permit admin to skip password check (he can supply any placeholder) - permit admin to disband his own single faction - don't call get_owner or valid_password if is admin - streamline duplicate code * list: check for true first instead of using negation - check for no factions first -> simpler code - whitespace: linebreak for easier reading * info: cleanup - whitespace linebreaks for easier reading and consistancy - update helptext signiture (also for disband) to reflect actual requirements and standard - loop members into table for consistant and easier to read code * player_info: cleanup - move depricated log entry to start of get_player_faction(), no point in skipping warning. - simplify get_player_factions() - whitespace linebreaks for easier reading and consistancy - loop members into table for consistant and easier to read code - simplify get_owned_factions() - make player_name param optional, default to caller (still need to check as caller name can be missing) - loop factions into table for consistant and easier to read code (also presumpted faster) * join: cleanup - don't call get_player_factions() unless needed - use get_player_factions() instead of depricated get_player_faction() - truth check of password in valid_password() for easier understanding of code - remove explicit nil check where not needed * leave: cleanup - update help text to standard syntax - remove unnecessary param count checks - simplify leave_faction() argument checking * kick: cleanup - simplify and reduce calls of core.get_player_privs() - update help text to standard syntax - streamline duplicate code - remove unnecessary param count checks - remove explicit nil check where not needed - don't call get_owner if is admin (until needed) * passwd: cleanup - update help text to standard syntax - streamline duplicate code - remove unnecessary param count checks - remove explicit nil check where not needed - don't call get_owner if is admin * chown: cleanup and tweak - update help text to standard syntax - streamline duplicate code - remove unnecessary param count checks - remove explicit nil check where not needed - updated locale to be neutral to admin or owner - don't call get_owner or valid_password if is admin - remove core.player_exists() call since target was checked when joined faction - abort early if no target or password provided * invite: cleanup and tweaks - reduced needed indents - remove explicit nil check where not needed - use get_player_factions() instead of depricated get_player_faction() and reduce calls of it - tweaked join_faction() - adds check if player already is in that faction * more tweaks - join: check if already member - leave: checks if user is in given faction at all - kick: early abort if no player provided - create: early abort if no faction or password are provided - create: use get_player_factions() instead of get_player_faction() - create: reduce explicit nil checks - disband: early abort if missing password - disband: reduce param-count-checks and use table.getn() - info: reduce explicit nil checks and use table.getn() - passwd: early abort if no password provided - in general remove explicit nil-checks where not needed * is_admin -> not_admin for slightly easier reading and shorter lines * fix translator missing argument * some facepalm fixes and tweaks of table.getn() for consistency, here # would work just as well. * set minimum server version to 5.9.0 * another facepalm moment * add mtt support * refactor handle_command for mtt It could've been done by only exposing handle_command, but this is cleaner for future maintenance as tasks are well separated. * bundle mtt related lines * needs fakelib, not areas areas will need this mod for testing * remove unused arguments * add owner to members on cleanup * rename chat to cc also no need to expose cc directly to mtt * register the actually set priv when it is missing * label data correctly * move settings higher up where they are expected to be * consistancy with variable names use faction_name, player_name, target_name, password etc. instead of a jumble of pw, fname, name, player_name etc. * reduce needles table-copy * fail to register same named factions * no-op depricated and useless get_password * some more checks in some API methods * whitespace and comments * pass translator to mtt * bugfix cc.disband inverted password check * standardize var name and reduce looping * add get_members() api-method and use it * player_info: count empty string as no player * player_info: switch if-else to avoid negation * unreachable comments * simpler check * add mtt-checks for front and backend commands * update french locale - informal tone - adds missing entries * add Spanish locale * add German locale * whitespace cleanup * add fakelib comment * provide alternative to table.pack() * add disband hook support * remove local f == factions --- init.lua | 34 ++++++++++++++++++++------------- locale/playerfactions.de.tr | 1 - locale/playerfactions.es.tr | 1 - locale/playerfactions.fr.tr | 1 - locale/template.txt | 1 - mod.conf | 2 +- mtt.lua | 38 +++++++++++++++++++++++-------------- 7 files changed, 46 insertions(+), 32 deletions(-) diff --git a/init.lua b/init.lua index 31a318c..5031213 100644 --- a/init.lua +++ b/init.lua @@ -159,7 +159,7 @@ function factions.register_faction(faction_name, player_name, password) facts[faction_name] = { name = faction_name, owner = player_name, - password = password, + password256 = factions.hash_password(password), members = { [player_name] = true } } save_factions() @@ -178,25 +178,32 @@ function factions.disband_faction(faction_name) return true end + +function factions.hash_password(password) + return minetest.sha256(password) +end + + function factions.valid_password(faction_name, password) if not facts[faction_name] or not password then return false end - return password == facts[faction_name].password + + return factions.hash_password(password) == facts[faction_name].password256 end -function factions.get_password(faction_name) - if not facts[faction_name] then - return false - end - return facts[faction_name].password +function factions.get_password() + minetest.log("warning", "Deprecated use of factions.get_password(). " + .. "Please update to using factions.valid_password() instead.") + return nil end function factions.set_password(faction_name, password) if not (facts[faction_name] and 'string' == type(password)) then return false end - facts[faction_name].password = password + + facts[faction_name].password256 = factions.hash_password(password) save_factions() return true end @@ -284,7 +291,8 @@ function cc.list() end end -function cc.info(player_name, params, not_admin) + +function cc.info(player_name, params) local faction_name = params[2] if not faction_name then local player_factions = factions.get_player_factions(player_name) @@ -309,10 +317,6 @@ function cc.info(player_name, params, not_admin) local summary = S("Name: @1\nOwner: @2\nMembers: @3", faction_name, factions.get_owner(faction_name), table.concat(faction_members, ", ")) - if not not_admin or factions.get_owner(faction_name) == player_name then - summary = summary .. "\n" - .. S("Password: @1", factions.get_password(faction_name)) - end return true, summary end end @@ -579,6 +583,10 @@ do fact.members = { [fact.owner] = true } + end + if fact.password then + fact.password256 = factions.hash_password(fact.password) + fact.password = nil save_needed = true end end diff --git a/locale/playerfactions.de.tr b/locale/playerfactions.de.tr index af1ce6e..59c9153 100644 --- a/locale/playerfactions.de.tr +++ b/locale/playerfactions.de.tr @@ -36,7 +36,6 @@ Missing player name.=Spielername fehlt. Name: @1@nOwner: @2@nMembers: @3=Name: @1@nBesitzer: @2@nMitglieder: @3 Ownership has been transferred to @1.=Eigentum wurde auf @1 übertragen. Password has been updated.=Passwort wurde aktualisiert. -Password: @1=Passwort: @1 Permission denied: Wrong password.=Berechtigung verweigert: Falsches Passwort. Permission denied: You are not the owner of that faction, and don't have the @1 privilege.=Berechtigung verweigert: Du bist nicht der Besitzer dieser Fraktion und hast nicht das @1-Privileg. diff --git a/locale/playerfactions.es.tr b/locale/playerfactions.es.tr index f51ba17..2778adc 100644 --- a/locale/playerfactions.es.tr +++ b/locale/playerfactions.es.tr @@ -36,7 +36,6 @@ Missing player name.=Falta el nombre del jugador. Name: @1@nOwner: @2@nMembers: @3=Nombre: @1@nPropietario: @2@nMiembros: @3 Ownership has been transferred to @1.=La propiedad ha sido transferida a @1. Password has been updated.=La contraseña ha sido actualizada. -Password: @1=Contraseña: @1 Permission denied: Wrong password.=Permiso denegado: Contraseña incorrecta. Permission denied: You are not the owner of that faction, and don't have the @1 privilege.=Permiso denegado: No eres el propietario de esa facción y no tienes el privilegio @1. diff --git a/locale/playerfactions.fr.tr b/locale/playerfactions.fr.tr index 4251193..6a9b9f4 100644 --- a/locale/playerfactions.fr.tr +++ b/locale/playerfactions.fr.tr @@ -36,7 +36,6 @@ Missing player name.=Nom de joueur manquant. Name: @1@nOwner: @2@nMembers: @3=Nom : @1@nPropriétaire : @2@nMembres : @3 Ownership has been transferred to @1.=La propriété a été transférée à @1. Password has been updated.=Le mot de passe a été mis à jour. -Password: @1=Mot de passe : @1 Permission denied: Wrong password.=Permission refusée : mauvais mot de passe. Permission denied: You are not the owner of that faction, and don't have the @1 privilege.=Permission refusée : tu n'es pas le propriétaire de cette faction, et tu n'as pas le privilège @1. diff --git a/locale/template.txt b/locale/template.txt index 4386307..fc85111 100644 --- a/locale/template.txt +++ b/locale/template.txt @@ -36,7 +36,6 @@ Missing player name.= Name: @1@nOwner: @2@nMembers: @3= Ownership has been transferred to @1.= Password has been updated.= -Password: @1= Permission denied: Wrong password.= Permission denied: You are not the owner of that faction, and don't have the @1 privilege.= diff --git a/mod.conf b/mod.conf index f0a74e8..fcc0399 100644 --- a/mod.conf +++ b/mod.conf @@ -1,3 +1,3 @@ name = playerfactions -min_minetest_version = 5.0.0 +min_minetest_version = 5.9.0 optional_depends = mtt diff --git a/mtt.lua b/mtt.lua index 8751559..a543f56 100644 --- a/mtt.lua +++ b/mtt.lua @@ -43,10 +43,15 @@ local function dbChecks(callback) assert('table' == type(facts.Alberian.members)) -- make sure owners have been added as memebers assert(true == facts.Alberian.members.Albert) - -- should never fail - assert('eEe' == facts.Endorian.password) - assert('a' == facts.Alberian.password) - assert('GgG♥💩☺' == facts.Gandalfian.password) + -- hash tests, should never fail unless engine made a mistake + assert('8b2713b352c6fa2d22272a91612fba2f87d0c01885762a1522a7b4aec5592a80' + == facts.Endorian.password256) + assert('ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb' + == facts.Alberian.password256) + assert('3bfe911604e3fb079ad535a0c359a8457aea39d663bb4f21648842e3a4eaccf9' + == facts.Gandalfian.password256) + -- no more cleartext passwords (doesn't make sense in test-environement) + assert(nil == facts.Gandalfian.password) callback() end @@ -193,6 +198,12 @@ mtt.register('backend functions: disband_faction', function(callback) callback() end) +mtt.register('backend functions: hash_password', function(callback) + -- (tested in basic db checks) + + callback() +end) + mtt.register('backend functions: valid_password', function(callback) assert(false == factions.valid_password()) assert(false == factions.valid_password('Endorian')) @@ -202,9 +213,9 @@ mtt.register('backend functions: valid_password', function(callback) callback() end) -mtt.register('backend functions: get_password', function(callback) - assert(false == factions.get_password()) - assert('eEe' == factions.get_password('Endorian')) +mtt.register('backend functions: get_password (depricated)', function(callback) + assert(nil == factions.get_password()) + assert(nil == factions.get_password('Endorian')) callback() end) @@ -313,7 +324,6 @@ mtt.register('frontend functions: info', function(callback) 'Gandalfian, Endorian'), 'Gandalf', 'info')) -- SwissalpS can't be bothered to check some of these results in depth, -- so just dumping result for optical check. - -- owner sees password pd('Endor executes: /factions info', fcc('Endor', 'info')) assert(fcc('Endor', 'info')) factions.max_members_list = 1 @@ -323,9 +333,6 @@ mtt.register('frontend functions: info', function(callback) factions.max_members_list = 11 pd('Endor executes: /factions info Gandalfian', fcc('Endor', 'info Gandalfian')) assert(fcc('Endor', 'info Gandalfian')) - -- admin sees password - pd('Albert executes: /factions info Gandalfian', fcc('Albert', 'info Gandalfian')) - assert(fcc('Albert', 'info Gandalfian')) callback() end) @@ -430,13 +437,16 @@ mtt.register('frontend functions: passwd', function(callback) 'Endor', 'passwd foobar Gandalfian')) assert(fccc(true, S("Password has been updated."), 'Endor', 'passwd foobar')) - assert(factions.get_facts().Endorian.password == 'foobar') + assert(factions.get_facts().Endorian.password256 == + 'c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2') assert(fccc(true, S("Password has been updated."), 'Gandalf', 'passwd foobar Gandalfian')) - assert(factions.get_facts().Gandalfian.password == 'foobar') + assert(factions.get_facts().Gandalfian.password256 == + 'c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2') assert(fccc(true, S("Password has been updated."), 'Albert', 'passwd barf Gandalfian')) - assert(factions.get_facts().Gandalfian.password == 'barf') + assert(factions.get_facts().Gandalfian.password256 == + '8a6e40cfcd99060eb1efdfeb689fe26606e221b4fd487bb224ab79a82648ccd9') callback() end)