mod_authz_internal, and more: New iteration of role API

These changes to the API (hopefully the last) introduce a cleaner separation
between the user's primary (default) role, and their secondary (optional)
roles.

To keep the code sane and reduce complexity, a data migration is needed for
people using stored roles in 0.12. This can be performed with

  prosodyctl mod_authz_internal migrate <host>

3 files changed, 132 insertions, 38 deletions

diff --git a/plugins/mod_authz_internal.lua b/plugins/mod_authz_internal.lua
index 135c7e61..af402d3e 100644
--- a/plugins/mod_authz_internal.lua
+++ b/plugins/mod_authz_internal.lua
@@ -8,8 +8,9 @@ local roles = require "util.roles";
 local config_global_admin_jids = module:context("*"):get_option_set("admins", {}) / normalize;
 local config_admin_jids = module:get_option_inherited_set("admins", {}) / normalize;
 local host = module.host;
-local role_store = module:open_store("roles");
-local role_map_store = module:open_store("roles", "map");
+
+local role_store = module:open_store("account_roles");
+local role_map_store = module:open_store("account_roles", "map");
 
 local role_registry = {};
 
@@ -98,52 +99,96 @@ end
 
 -- Public API
 
-local config_operator_role_set = {
-	["prosody:operator"] = role_registry["prosody:operator"];
-};
-local config_admin_role_set = {
-	["prosody:admin"] = role_registry["prosody:admin"];
-};
-local default_role_set = {
-	["prosody:user"] = role_registry["prosody:user"];
-};
-
-function get_user_roles(user)
+-- Get the primary role of a user
+function get_user_role(user)
 	local bare_jid = user.."@"..host;
+
+	-- Check config first
 	if config_global_admin_jids:contains(bare_jid) then
-		return config_operator_role_set;
+		return role_registry["prosody:operator"];
 	elseif config_admin_jids:contains(bare_jid) then
-		return config_admin_role_set;
+		return role_registry["prosody:admin"];
+	end
+
+	-- Check storage
+	local stored_roles, err = role_store:get(user);
+	if not stored_roles then
+		if err then
+			-- Unable to fetch role, fail
+			return nil, err;
+		end
+		-- No role set, use default role
+		return role_registry["prosody:user"];
+	end
+	if stored_roles._default == nil then
+		-- No primary role explicitly set, return default
+		return role_registry["prosody:user"];
+	end
+	local primary_stored_role = role_registry[stored_roles._default];
+	if not primary_stored_role then
+		return nil, "unknown-role";
+	end
+	return primary_stored_role;
+end
+
+-- Set the primary role of a user
+function set_user_role(user, role_name)
+	local role = role_registry[role_name];
+	if not role then
+		return error("Cannot assign default user an unknown role: "..tostring(role_name));
+	end
+	local keys_update = {
+		_default = role_name;
+		-- Primary role cannot be secondary role
+		[role_name] = role_map_store.remove;
+	};
+	if role_name == "prosody:user" then
+		-- Don't store default
+		keys_update._default = role_map_store.remove;
+	end
+	local ok, err = role_map_store:set_keys(user, keys_update);
+	if not ok then
+		return nil, err;
 	end
-	local role_names = role_store:get(user);
-	if not role_names then return default_role_set; end
-	local user_roles = {};
-	for role_name in pairs(role_names) do
-		user_roles[role_name] = role_registry[role_name];
+	return role;
+end
+
+function add_user_secondary_role(user, role_name)
+	if not role_registry[role_name] then
+		return error("Cannot assign default user an unknown role: "..tostring(role_name));
 	end
-	return user_roles;
+	role_map_store:set(user, role_name, true);
 end
 
-function set_user_roles(user, user_roles)
-	role_store:set(user, user_roles)
-	return true;
+function remove_user_secondary_role(user, role_name)
+	role_map_store:set(user, role_name, nil);
 end
 
-function get_user_default_role(user)
-	local user_roles = get_user_roles(user);
-	if not user_roles then return nil; end
-	local default_role;
-	for role_name, role_info in pairs(user_roles) do --luacheck: ignore 213/role_name
-		if role_info.default ~= false and (not default_role or role_info.priority > default_role.priority) then
-			default_role = role_info;
+function get_user_secondary_roles(user)
+	local stored_roles, err = role_store:get(user);
+	if not stored_roles then
+		if err then
+			-- Unable to fetch role, fail
+			return nil, err;
 		end
+		-- No role set
+		return {};
+	end
+	stored_roles._default = nil;
+	for role_name in pairs(stored_roles) do
+		stored_roles[role_name] = role_registry[role_name];
 	end
-	if not default_role then return nil; end
-	return default_role;
+	return stored_roles;
 end
 
+-- This function is *expensive*
 function get_users_with_role(role_name)
-	local storage_role_users = it.to_array(it.keys(role_map_store:get_all(role_name) or {}));
+	local function role_filter(username, default_role) --luacheck: ignore 212/username
+		return default_role == role_name;
+	end
+	local primary_role_users = set.new(it.to_array(it.filter(role_filter, pairs(role_map_store:get_all("_default") or {}))));
+	local secondary_role_users = set.new(it.to_array(it.keys(role_map_store:get_all(role_name) or {})));
+
 	local config_set;
 	if role_name == "prosody:admin" then
 		config_set = config_admin_jids;
@@ -157,9 +202,9 @@ function get_users_with_role(role_name)
 				return j_node;
 			end
 		end;
-		return it.to_array(config_admin_users + set.new(storage_role_users));
+		return it.to_array(config_admin_users + primary_role_users + secondary_role_users);
 	end
-	return storage_role_users;
+	return it.to_array(primary_role_users + secondary_role_users);
 end
 
 function get_jid_role(jid)
@@ -203,3 +248,52 @@ end
 function get_role_by_name(role_name)
 	return assert(role_registry[role_name], role_name);
 end
+
+-- COMPAT: Migrate from 0.12 role storage
+local function do_migration(migrate_host)
+	local old_role_store = assert(module:context(migrate_host):open_store("roles"));
+	local new_role_store = assert(module:context(migrate_host):open_store("account_roles"));
+
+	local migrated, failed, skipped = 0, 0, 0;
+	-- Iterate all users
+	for username in assert(old_role_store:users()) do
+		local old_roles = it.to_array(it.filter(function (k) return k:sub(1,1) ~= "_"; end, it.keys(old_role_store:get(username))));
+		if #old_roles == 1 then
+			local ok, err = new_role_store:set(username, {
+				_default = old_roles[1];
+			});
+			if ok then
+				migrated = migrated + 1;
+			else
+				failed = failed + 1;
+				print("EE: Failed to store new role info for '"..username.."': "..err);
+			end
+		else
+			print("WW: User '"..username.."' has multiple roles and cannot be automatically migrated");
+			skipped = skipped + 1;
+		end
+	end
+	return migrated, failed, skipped;
+end
+
+function module.command(arg)
+	if arg[1] == "migrate" then
+		table.remove(arg, 1);
+		local migrate_host = arg[1];
+		if not migrate_host or not prosody.hosts[migrate_host] then
+			print("EE: Please supply a valid host to migrate to the new role storage");
+			return 1;
+		end
+
+		-- Initialize storage layer
+		require "core.storagemanager".initialize_host(migrate_host);
+
+		print("II: Migrating roles...");
+		local migrated, failed, skipped = do_migration(migrate_host);
+		print(("II: %d migrated, %d failed, %d skipped"):format(migrated, failed, skipped));
+		return (failed + skipped == 0) and 0 or 1;
+	else
+		print("EE: Unknown command: "..(arg[1] or "<none given>"));
+		print("    Hint: try 'migrate'?");
+	end
+end
diff --git a/plugins/mod_c2s.lua b/plugins/mod_c2s.lua
index 8c0844ae..e8241687 100644
--- a/plugins/mod_c2s.lua
+++ b/plugins/mod_c2s.lua
@@ -259,7 +259,7 @@ local function disconnect_user_sessions(reason, leave_resource)
 end
 
 module:hook_global("user-password-changed", disconnect_user_sessions({ condition = "reset", text = "Password changed" }, true), 200);
-module:hook_global("user-roles-changed", disconnect_user_sessions({ condition = "reset", text = "Roles changed" }), 200);
+module:hook_global("user-role-changed", disconnect_user_sessions({ condition = "reset", text = "Role changed" }), 200);
 module:hook_global("user-deleted", disconnect_user_sessions({ condition = "not-authorized", text = "Account deleted" }), 200);
 
 function runner_callbacks:ready()
diff --git a/plugins/mod_tokenauth.lua b/plugins/mod_tokenauth.lua
index 6610036c..85602747 100644
--- a/plugins/mod_tokenauth.lua
+++ b/plugins/mod_tokenauth.lua
@@ -10,7 +10,7 @@ local function select_role(username, host, role)
 	if role then
 		return prosody.hosts[host].authz.get_role_by_name(role);
 	end
-	return usermanager.get_user_default_role(username, host);
+	return usermanager.get_user_role(username, host);
 end
 
 function create_jid_token(actor_jid, token_jid, token_role, token_ttl)