Address set_player_privs footgun (#14297)

---------

Co-authored-by: grorp <gregor.parzefall@posteo.de>

builtin/game/auth.lua

@@ -92,8 +92,16 @@ core.builtin_auth_handler = {
 
 		core_auth.save(auth_entry)
 
-		-- Run grant callbacks
+		for priv, value in pairs(privileges) do
-		for priv, _ in pairs(privileges) do
+			-- Warnings for improper API usage
+			if value == false then
+				core.log('deprecated', "`false` value given to `minetest.set_player_privs`, "..
+						"this is almost certainly a bug, "..
+						"granting a privilege rather than revoking it")
+			elseif value ~= true then
+				core.log('deprecated', "non-`true` value given to `minetest.set_player_privs`")
+			end
+			-- Run grant callbacks
 			if not prev_privs[priv] then
 				core.run_priv_callbacks(name, priv, nil, "grant")
 			end
@@ -180,6 +188,20 @@ core.set_player_privs    = auth_pass("set_privileges")
 core.remove_player_auth  = auth_pass("delete_auth")
 core.auth_reload         = auth_pass("reload")
 
+function core.change_player_privs(name, changes)
+	local privs = core.get_player_privs(name)
+	for priv, change in pairs(changes) do
+		if change == true then
+			privs[priv] = true
+		elseif change == false then
+			privs[priv] = nil
+		else
+			error("non-bool value given to `minetest.change_player_privs`")
+		end
+	end
+	core.set_player_privs(name, privs)
+end
+
 local record_login = auth_pass("record_login")
 core.register_on_joinplayer(function(player)
 	record_login(player:get_player_name())

doc/lua_api.md

@@ -5841,8 +5841,20 @@ Authentication
     * `name`: string; if omitted, all auth data should be considered modified
 * `minetest.set_player_password(name, password_hash)`: Set password hash of
   player `name`.
-* `minetest.set_player_privs(name, {priv1=true,...})`: Set privileges of player
+* `minetest.set_player_privs(name, privs)`: Set privileges of player `name`.
-  `name`.
+  * `privs` is a **set** of privileges:
+    A table where the keys are names of privileges and the values are `true`.
+  * Example: `minetest.set_player_privs("singleplayer", {interact = true, fly = true})`.
+    This **sets** the player privileges to `interact` and `fly`;
+    `singleplayer` will only have these two privileges afterwards.
+* `minetest.change_player_privs(name, changes)`: Helper to grant or revoke privileges.
+  * `changes`: Table of changes to make.
+    A field `[privname] = true` grants a privilege,
+    whereas `[privname] = false` revokes a privilege.
+  * Example: `minetest.change_player_privs("singleplayer", {interact = true, fly = false})`
+    will grant singleplayer the `interact` privilege
+    and revoke singleplayer's `fly` privilege.
+    All other privileges will remain unchanged.
 * `minetest.auth_reload()`
     * See `reload()` in authentication handler definition
 
@@ -10745,8 +10757,8 @@ Used by `minetest.register_authentication_handler`.
 
     set_privileges = function(name, privileges),
     -- Set privileges of player `name`.
-    -- `privileges` is in table form, auth data should be created if not
+    -- `privileges` is in table form: keys are privilege names, values are `true`;
-    -- present.
+    -- auth data should be created if not present.
 
     reload = function(),
     -- Reload authentication data from the storage location.