Creating a global banning system

I’ve tried to create a global banning script:

bannedPlayers = {}
function checkPlayer(player)
	for _, players in pairs(game.Players:GetPlayers()) do
		if player == players.UserId then
			return true
		end
	end
end

moderation.OnServerEvent:Connect(function(player, reason, badPlayer)
	if badPlayer then
		if reason == 'Kick' then
			badPlayer:Kick('A member of staff has kicked you')
		elseif reason == 'Ban' then	
			local bannedPlayer = game.Players:GetUserIdFromNameAsync(badPlayer)
			if bannedPlayer then
				print('banned player ID found')
				local myData = bannedPlayers[bannedPlayer]
				print('myData checked')
				if myData then
					print('myData passed')
					myData.banned = true
					bannedData:SetAsync(bannedPlayer, myData)
					if checkPlayer() then
						badPlayer:Kick('A member of staff has banned you')
					end
				end
			end
		end
	end
end)

game.Players.PlayerAdded:Connect(function(player)
	local myData = bannedData:GetAsync(player.userId) or {}

	myData.banned = myData.banned or false
	
	bannedPlayers[player.userId] = myData
	
	if myData.banned then
		player:Kick('You have been banned from the server by a member of staff')
	end
end)

The problem I am having is when you enter a player whose NOT in the servers user name. It still gets their UserId from :GetUserIdFromNameAsync(badPlayer) but won’t go into the datastore to add them to the banned list. Only time I can ban them is if they are in the server, which is extremely unreliable as reports of hacking/general bad behaviour couldn’t be solved as it would be unlikely I’d be able to get into a server with the perpetrators.

EDIT Half the script is missing, but it’s irrelevant stuff (variables like defining the datastore and what not)

EDIT2 Put in prints, output prints the first 2 prints. So it’s stopping at ‘if myData then’

3 Likes

From your code it seems you are trying to pass a player object to :GetUserIdFromNameAsync. This won’t work, as a player object isn’t a string.

You probably forgot to add a .Name in there :wink:

1 Like

No, because ‘badPlayer’ is a string. Basically I enter a players name into a TextBox and click ban and the textbox.Text is what badPlayer is. The problem is that it doesn’t work per say. If I enter a players name who is in the server, they get kicked and banned and it works. It’s that I want it to work so the player doesnt have to be on the server. I want to just be able add them to the datastore automatically. And if they try to join they get kicked because they are banned

1 Like

Screenshot_687

badPlayer is not a string.

To do it without them being in game, the code doesn’t really need to change much. Just enter their exact name, and it can be plugged into :GetUserIdFromNameAsync.

Oh sorry, there’s a function the localscript that if you click kick it will go through all the players in the server and find the player with the same name as textbox.Text and then return the player. But since I want to ban players who arent in the server I made it so it just sends the text. So badPlayer is a player if they click Kick, if they click ban it’s a string (the players name)

2 Likes

That’s not good behaviour. You should clean up that up.

Either way, what you want is possible using just the name string.

1 Like

I editted my script with prints to see where it’s stopping. It’s stopping on the line ‘if myData then’

Well debug it more. Their UserId is not in the banned table – why, and is it the right UserId in the first place?

Print those all out.

Well that’s the problem, for them to be put into the myData table they need to join (scroll down to the playeradded event to see) I’m not sure how to change it.

Create a new myData table for them then :wink:

There is a pretty big flaw in this script, it trusts client way too much. Right now you assume that this remote event will only get fired by localscripts that you control. But when you’re dealing with security, you have to always assume that client will lie whenever it gets a chance.

In case of your code, you probably expect that this event will be fired only by, for example, a GUI administration tool, that only whitelisted people can acquire. It might seem secure enough having a server script that checks if a user has admin rights and only then copies the admin GUI to the user, which is true. But the actual administration interface is this very remote you have here, not the GUI. That means anyone, even those without the GUI, can fire this event(with use of hacking tools like memory editors and custom code injectors). Since your server code doesn’t check if the player who fired this event has permission to ban people, any determined person will be able to exploit this vulnerability and end up banning all players from the server.

Filtering is just a tool. You can imagine it as a fortified wall that can only be passed through few, highly secured gateways. What you do here is smash a hole in this wall, which allows anyone who stumbled upon it, pass through and access whatever you had locked away behind it.

In this particular case, all you really had to do, is check if the object held by player argument has permission to ban others.

6 Likes

Yeah had someone else message me about it :+1: added an admin list to the server script now and checking to see if the event is fired by someone in that list.