Imarok Mar 12 2018, 1:23 AM
- Differential Revision
- D897: Secure lobby authentication - prevent joins as a different player
- rP21519: Fix elephant archers having no animations.
- Build Status
Buildable 5511 Build 9307: Trigger Windows Autobuild Build 9306: Post-Commit Build Jenkins
Got this one while hosting a game as elexis3 shortly after hannibal disconnected. Did we swap the strings?
As mentioned it occured while I was hosting, some moments after hannibal was offline.
Not on my purpose.
Not really, but the case is clear, isn't it? He tried to join as me on my server and we need to swap these 2 lines?
when it's the converse.
Differently speaking did he really try to join as you, or was this warning triggered because of a bug?
I don't have forensic evidence. I don't observe anything indicating a bug other than the name order.
(So commit the fix)
The 3 lobby hosts and svn testers disabled the feature because they don't want to lose the ability to replace players.
It can't be so difficult to set the secure-auth flag with a boolean from the options dialog.
One can enter a session function in options.json and that can call a JSInterface_Network.cpp function.
That file can access g_NetServer directly and after the release before the dedicated server option, we can add a packet type that sets the server options such as lobby auth, password, possibly further in the future.
The use case of an ingame option is not only for replacing players,
but also allows hosts to enable it after the game started when they forgot it and vice versa.
Also it is good practice to not have to restart the game when changing an option where possible.
As mentioned it would be good to rename name to lobbyJIDNode and all other variables holding the same value, because it's so easily confused with the UDP / simulation playername. We have to lookup multiple classes, the lobby queue and whatnot to determine which of the two names it is.
(This allows Hannibal Barca to join as Hannibal Barca (The Great) and Hannibal Barca (elexis) simultaneously. Doesn't compromise it's aim however.)
The configuration entry and the options UI entry proposed were meant to be used for players that play with the vanilla 0ad code.
(1) few people, that are
((2) smart enough to determine the IP address and share it via chat were able to used it.
This meant that
(3) the impending "player replacement" was discussed within the match for some while,
(4) giving the other players plenty of time to become informed of the replacement and
(5) plenty of time to object.
But there is a mod that exploits this authentication being optional by implementing an impersonation-join UI feature, disabling all five of the mentioned preconditions of arguably legitimate use.
There is a second mod that implements joins with arbitrary nicknames, which is also in violation of the terms of the lobby.
Under these circumstances, it seems better to remove the config option.
JID reconstruction + hardcoded resource (that will fail for any other non-0ad mod)
Since this commit delayed the OnAuthenticate call which called DeduplicatePlayerName call, it should be this commit that broke the duplicateplayernames config setting. (I could not manage to verify this experimentally within 3 minutes.)
If one sets that value to true when hosting a lobbied game, all players will join with foo (2) in the gamesetup and won't be able to rejoin after a disconnect!
Hmm, bad thing.
I guess just disabling that feature in the lobby is the solution.
It doesn't make any sense anyway when lobby authentication is enabled, because there can't be duplicate names.