Skip to content

[Fix • Issue #41] - Automatically assign commands permission groups based on ranks / roles #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Mophyr wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[Fix • Issue #41] - Automatically assign commands permission groups based on ranks / roles #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
def0ca4
Automatically assign commands permission groups based on ranks / roles
Mophyr Nov 27, 2025
2ffd59a
Remove debug warn
Mophyr Nov 27, 2025
66f2ef5
Merge branch 'main' into pr/42
Raild3x Dec 1, 2025
2b7ddda
Add server-only assertions to permission methods
Raild3x Dec 1, 2025
64d783b
Remove server assertion from GetPlayerPermissionGroups
Raild3x Dec 1, 2025
9626975
Merge branch 'main' into pr/42
Raild3x Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions lib/cmdrhandler/src/Server/init.luau
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,41 @@ function CmdrServer:Init()
end

-- Sync initial player permissions to client
--[[
[⚠️ • NOTE]: Player:GetRoleInGroup() still returns the player rank,
so this should not be used until Roblox fixes it.

[⚠️ • NOTE]: When getting back to this, check again this implementation.
A player might have multiple roles with different permissions, and :GetRoleInGroup()
assumes only one role per player.

local groupRolePermissionGroups = Client.GroupRolePermissionGroups:Get()
for groupId, roleData in groupRolePermissionGroups do
if player:IsInGroup(groupId) then
for role, permissionGroups in roleData do
if player:GetRoleInGroup(groupId) == role then
CmdrServer.PermissionsHandler:GivePlayerPermissionGroups(player, permissionGroups)
end
end
end
end
--]]

local groupRankPermissionGroups = Client.GroupRankPermissionGroups:Get()
for groupId, rankData in groupRankPermissionGroups do
if player:IsInGroup(groupId) then
for _, rankPermissionsData in rankData do
local playerRank = player:GetRankInGroup(groupId)
if playerRank >= rankPermissionsData.Ranks.Min and playerRank <= rankPermissionsData.Ranks.Max then
CmdrServer.PermissionsHandler:GivePlayerPermissionGroups(
player,
rankPermissionsData.Permissions
)
end
end
end
end

local playerPerms = CmdrServer.PermissionsHandler:_getPlayerPermissionGroupsData()[player]
Client.Permissions:SetFor(player, playerPerms)
end)
Expand Down
93 changes: 61 additions & 32 deletions lib/cmdrhandler/src/Shared/CmdrUtil.luau
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,61 +6,69 @@ local GroupService = game:GetService("GroupService")
local Players = game:GetService("Players")

--// Imports //--
local GroupCache = (setmetatable({}, {__mode = "k"}) :: any) :: {[Player]: {{
Name: string,
Id: number,
Rank: number,
Role: string,
IsPrimary: boolean,
}}}
local GroupCache = (setmetatable({}, { __mode = "k" }) :: any) :: {
[Player]: {
{
Name: string,
Id: number,
Rank: number,
Role: string,
IsPrimary: boolean,
}
},
}

local function RefreshGroupCache(plr: Player)
task.spawn(function()
local groups = GroupService:GetGroupsAsync(plr.UserId)
GroupCache[plr] = groups
-- if RunService:IsStudio() then
-- print("[Cmdr] Cached Groups for ", plr.Name, ":", groups)
-- end
end)
task.spawn(function()
local groups = GroupService:GetGroupsAsync(plr.UserId)
GroupCache[plr] = groups
-- if RunService:IsStudio() then
-- print("[Cmdr] Cached Groups for ", plr.Name, ":", groups)
-- end
end)
end

Players.PlayerAdded:Connect(RefreshGroupCache)
for _, plr in ipairs(Players:GetPlayers()) do
RefreshGroupCache(plr)
RefreshGroupCache(plr)
end


--------------------------------------------------------------------------------
--// Class //--
--// Class //--
--------------------------------------------------------------------------------

local Util = {}

function Util.getPlayerPermissions(cmdrWrapper, plr: Player, rawPlayerPerms: {string}): {string}
local playerPerms = table.clone(rawPlayerPerms)
function Util.getPlayerPermissions(cmdrWrapper, plr: Player, rawPlayerPerms: { string }): { string }
local playerPerms = table.clone(rawPlayerPerms)

for _, permissionGroup: string in playerPerms do
for _, permissionGroup: string in playerPerms do
for _, inheritedPerm in cmdrWrapper:GetPermissionInheritance(permissionGroup) do
if not table.find(playerPerms, inheritedPerm) then
table.insert(playerPerms, inheritedPerm)
end
end
end

for _, groupData in GroupCache[plr] or {} do
for _, groupPerm in Util.getGroupRankPermissions(cmdrWrapper, groupData.Id, groupData.Rank) do
if not table.find(playerPerms, groupPerm) then
table.insert(playerPerms, groupPerm)
end
end
end
for _, groupData in GroupCache[plr] or {} do
for _, groupPerm in Util.getGroupRankPermissions(cmdrWrapper, groupData.Id, groupData.Rank) do
if not table.find(playerPerms, groupPerm) then
table.insert(playerPerms, groupPerm)
end
end
for _, groupPerm in Util.getGroupRolePermissions(cmdrWrapper, groupData.Id, groupData.Role) do
if not table.find(playerPerms, groupPerm) then
table.insert(playerPerms, groupPerm)
end
end
Comment on lines +59 to +63
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Util.getPlayerPermissions, the added loop calling Util.getGroupRolePermissions grants additional permissions to every group member, because getGroupRolePermissions (as implemented) returns all permissions for the group regardless of role. This causes privilege escalation where low-rank members inherit permissions intended only for high ranks/roles. Fix by removing this loop until role-based permissions are correctly implemented, or by ensuring getGroupRolePermissions strictly returns permissions for the player’s actual role and that underlying data is truly role-scoped.

Suggested change
for _, groupPerm in Util.getGroupRolePermissions(cmdrWrapper, groupData.Id, groupData.Role) do
if not table.find(playerPerms, groupPerm) then
table.insert(playerPerms, groupPerm)
end
end

Copilot uses AI. Check for mistakes.
end

return playerPerms
return playerPerms
end

function Util.getGroupRankPermissions(cmdrWrapper, groupId: number, rank: number): {string}
local groupData = cmdrWrapper:_getRawGroupPerms(groupId)
local rankPerms = {}
function Util.getGroupRankPermissions(cmdrWrapper, groupId: number, rank: number): { string }
local groupData = cmdrWrapper:_getRawGroupPerms(groupId)
local rankPerms = {}

-- This is terribly unoptimized, feel free to optimize it
for _, groupPermData in groupData do
Expand All @@ -81,4 +89,25 @@ function Util.getGroupRankPermissions(cmdrWrapper, groupId: number, rank: number
return rankPerms
end

return Util
function Util.getGroupRolePermissions(cmdrWrapper, groupId: number, role: string): { string }
local groupData = cmdrWrapper:_getRawGroupPerms(groupId)
local rolePerms = {}

-- This is terribly unoptimized, feel free to optimize it
for _, groupPermData in groupData do
for _, permissionGroup: string in groupPermData.Permissions do
if not table.find(rolePerms, permissionGroup) then
table.insert(rolePerms, permissionGroup)
end
for _, inheritedPerm in cmdrWrapper:GetPermissionInheritance(permissionGroup) do
if not table.find(rolePerms, inheritedPerm) then
table.insert(rolePerms, inheritedPerm)
end
Comment on lines +96 to +105
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Util.getGroupRolePermissions ignores the role parameter and returns the union of ALL permission groups configured for the Roblox group (from _getRawGroupPerms), effectively bypassing rank/role restrictions. Any player in the group will be granted permissions intended for higher ranks/roles because this function does not filter by role (or any constraint) before appending permissions. Fix by either (a) removing this function until role-based data is properly stored and retrieved, or (b) fetching role-specific mappings (e.g., via a dedicated _getGroupRolePermissionGroupsData() path) and returning only permissions for the matching role (with inheritance applied), never reading rank-based entries here.

Suggested change
-- This is terribly unoptimized, feel free to optimize it
for _, groupPermData in groupData do
for _, permissionGroup: string in groupPermData.Permissions do
if not table.find(rolePerms, permissionGroup) then
table.insert(rolePerms, permissionGroup)
end
for _, inheritedPerm in cmdrWrapper:GetPermissionInheritance(permissionGroup) do
if not table.find(rolePerms, inheritedPerm) then
table.insert(rolePerms, inheritedPerm)
end
-- Only include permissions for the specified role
for _, groupPermData in groupData do
if groupPermData.Role == role then
for _, permissionGroup: string in groupPermData.Permissions do
if not table.find(rolePerms, permissionGroup) then
table.insert(rolePerms, permissionGroup)
end
for _, inheritedPerm in cmdrWrapper:GetPermissionInheritance(permissionGroup) do
if not table.find(rolePerms, inheritedPerm) then
table.insert(rolePerms, inheritedPerm)
end
end

Copilot uses AI. Check for mistakes.
end
end
end

return rolePerms
end

return Util
35 changes: 28 additions & 7 deletions lib/cmdrhandler/src/Shared/PermissionsHandler.luau
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type CmdrRegistry = {

local Packages = script.Parent.Parent.Parent
local Signal = require(Packages.Signal) :: any
local IS_SERVER = RunService:IsServer()

-- Default permission inheritance structure (shared constant)
local DEFAULT_PERMISSION_INHERITANCE: { [string]: { string } } = {
Expand Down Expand Up @@ -115,6 +116,7 @@ end
@param permissions -- The permission group(s) to set
]=]
function PermissionsHandler:SetPlayerPermissionGroups(plr: Player, permissions: string | { string }): ()
assert(IS_SERVER, "PermissionsHandler:SetPlayerPermissionGroups can only be called on the server")
if typeof(permissions) == "string" then
permissions = { permissions }
end
Expand All @@ -128,6 +130,7 @@ end
@param permissionGroups -- The permission groups to grant
]=]
function PermissionsHandler:GivePlayerPermissionGroups(plr: Player, permissionGroups: string | { string }): ()
assert(IS_SERVER, "PermissionsHandler:GivePlayerPermissionGroups can only be called on the server")
if typeof(permissionGroups) == "string" then
permissionGroups = { permissionGroups }
end
Expand Down Expand Up @@ -192,9 +195,10 @@ function PermissionsHandler:GiveRobloxGroupRankPermissionGroups(

if RunService:IsStudio() then
print(
`[PermissionsHandler] - Granted group [{robloxGroupId}] permissions [{table.concat(permissionGroups :: any, ", ")}] for ranks [{(
ranks :: NumberRange
).Min}, {(ranks :: NumberRange).Max}]`
`[PermissionsHandler] - Granted group [{robloxGroupId}] permissions [{table.concat(
permissionGroups :: any,
", "
)}] for ranks [{(ranks :: NumberRange).Min}, {(ranks :: NumberRange).Max}]`
)
end

Expand All @@ -208,6 +212,9 @@ function PermissionsHandler:GiveRobloxGroupRankPermissionGroups(
end

--[=[
@ignore
@unreleased

Grants specified roles in a Roblox Group permission to use commands under a given PermissionGroup.
@param robloxGroupId -- The Roblox group id to grant permissions to
@param role -- The role name to grant permissions to
Expand All @@ -219,6 +226,9 @@ function PermissionsHandler:GiveRobloxGroupRolePermissionGroups(
role: string,
permissionGroups: string | { string }
): () -> ()
error(
"Using group roles for permissions does not work properly yet since Player:GetRoleInGroup() returns the player rank instead of the role. Use :GiveRobloxGroupRankPermissionGroups() instead."
)
assert(typeof(robloxGroupId) == "number", "Bad robloxGroupId")
assert(typeof(role) == "string", "Bad role")
assert(typeof(permissionGroups) == "string" or typeof(permissionGroups) == "table", "Bad permissions")
Expand Down Expand Up @@ -250,7 +260,10 @@ function PermissionsHandler:GiveRobloxGroupRolePermissionGroups(

if RunService:IsStudio() then
print(
`[PermissionsHandler] - Granted group [{robloxGroupId}] permissions [{table.concat(permissionGroups :: any, ", ")}] for role [{role}]`
`[PermissionsHandler] - Granted group [{robloxGroupId}] permissions [{table.concat(
permissionGroups :: any,
", "
)}] for role [{role}]`
)
end

Expand All @@ -271,7 +284,11 @@ end
@param permissionGroup -- The permission group to set the inheritance for
@param inheritedGroups -- The group(s) to inherit permissions from
]=]
function PermissionsHandler:SetPermissionGroupInheritance(permissionGroup: string, inheritedGroups: string | { string }): ()
function PermissionsHandler:SetPermissionGroupInheritance(
permissionGroup: string,
inheritedGroups: string | { string }
): ()
assert(IS_SERVER, "PermissionsHandler:SetPermissionGroupInheritance can only be called on the server")
assert(typeof(permissionGroup) == "string", "Bad permissionGroup")
if typeof(inheritedGroups) == "string" then
inheritedGroups = { inheritedGroups }
Expand Down Expand Up @@ -331,7 +348,9 @@ function PermissionsHandler:_getPlayerPermissionGroupsData(): { [Player]: { stri
return playerPermissionGroups
end

function PermissionsHandler:_getGroupRankPermissionGroupsData(): { [GroupId]: { { Ranks: NumberRange, Permissions: { string } } } }
function PermissionsHandler:_getGroupRankPermissionGroupsData(): {
[GroupId]: { { Ranks: NumberRange, Permissions: { string } } },
}
return groupRankPermissionGroups
end

Expand All @@ -352,7 +371,9 @@ function PermissionsHandler:_setPlayerPermissionGroupsData(data: { [Player]: { s
playerPermissionGroups = data or {}
end

function PermissionsHandler:_setGroupRankPermissionGroupsData(data: { [GroupId]: { { Ranks: NumberRange, Permissions: { string } } } }?): ()
function PermissionsHandler:_setGroupRankPermissionGroupsData(data: {
[GroupId]: { { Ranks: NumberRange, Permissions: { string } } },
}?): ()
groupRankPermissionGroups = data or {}
end

Expand Down