BanID Targetting

[ghost]

Steps to reproduce

1. Ban a user with banid (steamid) who is a higher rank in terms of cantarget

Expected behavior

An error of some kind saying you cannot target this user

Actual behavior

Bans the user

Error(s) in server console, if any

Error(s) in player's console, if any

Version

ULib v2.61d (10/26/16)
ULX v3.71d (12/03/16)

[viral32111]

Surely if they are a higher rank in terms of cantarget they wouldn't be able to be banned by a lower rank using any type of ban?

[ghost]

Yep, that should be the case but it is not.

[eggrolls-repu]

That's just poor configuration on your part. Not using the "can target" feature.

[JoshuaLeivers]

That's just poor configuration on your part. Not using the "can target" feature.

This is actually not the case. Running !ban $STEAM_BLAH will check if the (online) player is allowed to be targetted by the player, and then decide based on that whether to run it. !banid STEAM_BLAH does not do this checking at all, and will allow you to target anyone.

[JamminR]

This exact issue has been discussed for many years (even in our forums circa 2014).
Reference #105
A re-write of the command is not as easy as one would think, as banid allows for both offline and online bans. Due to the sensitivity of this command and it's potential for abuse, it was added to superadmin instead of the admin group in default ULX.
Superadmin was never intended for the level of access many server owners grant it.
Superadmin grants full console access to your server.
If you grant superadmin level access, server hosts have more than banid to be concerned with.

Though we're not saying never, I'll give the general same answer -
Don't give superadmin commands to those who should not be trusted with full server access

[JoshuaLeivers]

I'm currently working on a change to make ulx banid actually check who you are allowed to target. You are right that this isn't easy, but so far what I have seems potentially promising; I just need to put some more work into testing it and tweaking it before I'm ready to push it up and create a PR.

[JamminR]

Ensure to see #105 that was denied so as to not create duplicate work.
Reasons for denial - #105 (comment)

[JoshuaLeivers]

Ensure to see #105 that was denied so as to not create duplicate work.
Reasons for denial - #105 (comment)

Ah, thanks for pointing that out. The way I was going about it on my end was quite different to what I can see in that PR, though isn't quite the suggested route of creating a new parameter type.

What I currently have is essentially a new helper function to check whether the caller is allowed to target that SteamID by looking them up in the users table and checking things based on the caller's can_target and the like. I believe the current functionality should also be possible to use in other places mentioned as having the same issue.

Based on the comments here and in the denied PR, I'm going to continue testing and refining my current implementation to ensure that the logic is sound, and then once I'm happy with that, I will look into trying to create an actual param type to make this more accessible.