Check the client for permission instead of the event source

[NanoBob]

This PR changes permission checks to be done on the global client variable instead of on the event source. Since the event source is not guaranteed to be the player that triggered the event. (Think modified clients / injected Lua code)

Some detections are already in place to verify the client and source are equal, but with default configuration these are not enabled.
Furthermore there really is no reason to use source over client in this case since client is always the correct client.

[Dutchman101]

This causes a change in behavior with some commands (popular across gamemodes, including the default freeroam) that are listed in ACL

Example from chatbox: Access denied for 'repair'

[NanoBob]

There's no reason that should be the case, all this change does is check if the actual client that triggered an event has permission todo so. This would only have an impact in case the event is triggered from a client with a source other than that client's local player.

Furthermore freeroam has its own repair command which is completely unrelated to the admin resource:

mtasa-resources/[gameplay]/freeroam/fr_client.lua

Line 1502 in 2c6416a

function repairVehicle()

So unless any other gamemode were to trigger admin events, supplying a different element than the localplayer (one which does have the required admin access) as source, this change will not impact any such gamemode.

What I do see happening is that the admin panel itself triggers this event through commands (which arguably should not be done in this way, but legacy code..)
I'll add an additional update to fall back onto the event source, only if client is nil (or otherwise falsey).

[Dezash]

I think it would be better to separate the functions that are called by these event handlers so that the permission check wouldn't be needed at all if the function is called directly.
You could then replace serverside triggerEvent with function calls.

[NanoBob]

I do agree that it would indeed make far more sense for commands and events to be two separate ways to trigger the same function. (Instead of commands using the same events clients use).
However I do believe that is out of scope of this PR, since you're talking about a refactor (albeit a good one), while this is PR is about a pretty big security issue.

[patrikjuvonen]

So, did this change break other default resources or no? I expect fixes to them where needed.

[xLive]

So, did this change break other default resources or no? I expect fixes to them where needed.

No, it shouldn't affect other default resources.