/ps/trunk/source/network/NetServer.cpp
963	Got this one while hosting a game as elexis3 shortly after hannibal disconnected. Did we swap the strings? ERROR: Net server: lobby auth: ‎elexis3 tried joining as hannibal_barca

Imarok added inline comments.Mar 12 2018, 9:23 PM

/ps/trunk/source/network/NetServer.cpp
963	Yeah, it's swapped. But how did you get the error at all? Was that on purpose?

elexis added inline comments.Mar 13 2018, 12:36 AM

/ps/trunk/source/network/NetServer.cpp
963	But how did you get the error at all? As mentioned it occured while I was hosting, some moments after hannibal was offline. Was that on purpose? Not on my purpose.

Imarok added inline comments.Mar 13 2018, 3:30 PM

/ps/trunk/source/network/NetServer.cpp
963	Do you have mainlog & interestinglog? Can you reproduce it?

elexis added inline comments.Mar 13 2018, 3:42 PM

/ps/trunk/source/network/NetServer.cpp
963	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? Given that there is always more than one troll, Ive tried to reproduce and could reproduce the expected way, joining the lobby as elexis2 and changing to "name": "notme" in `joinSelectedGame` yields: ERROR: Net server: lobby auth: notme tried joining as elexis2 when it's the converse.

Imarok added inline comments.Mar 13 2018, 3:49 PM

/ps/trunk/source/network/NetServer.cpp
963	Sure, so that this warning is triggered is no bug, right? (Ofcourse are the names the wrong way around...)

Imarok added inline comments.Mar 13 2018, 3:51 PM

/ps/trunk/source/network/NetServer.cpp
963	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? Differently speaking did he really try to join as you, or was this warning triggered because of a bug?

Imarok mentioned this in rP21534: Fixed swapped names in rP21520.Mar 13 2018, 3:56 PM

elexis added inline comments.Mar 13 2018, 3:58 PM

/ps/trunk/source/network/NetServer.cpp
963	I don't have forensic evidence. I don't observe anything indicating a bug other than the name order. (Ofcourse are the names the wrong way around...) (So commit the fix)

Imarok added inline comments.Mar 13 2018, 4:02 PM

/ps/trunk/source/network/NetServer.cpp
963	You could ask the person in question... (too late)

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.

/ps/trunk/source/network/NetServer.cpp
872	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.
955	(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.

Thus only
(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.

elexis mentioned this in rP21877: Always require lobby authentication for lobby matches, refs #3549 / rP21520 /….Aug 25 2018, 4:35 PM

elexis added inline comments.Aug 30 2018, 6:38 PM

/ps/trunk/source/lobby/XmppClient.cpp
433	JID reconstruction + hardcoded resource (that will fail for any other non-0ad mod)
/ps/trunk/source/network/scripting/JSInterface_Network.cpp
110	JID split. Instead of receiving a JID, splitting the username, passing the username, reconstructing the JID from the username and passing the JID, it could simply receive the JID, pass the JID and pass the JID again.

elexis added inline comments.Aug 30 2018, 7:16 PM

/ps/trunk/source/network/scripting/JSInterface_Network.cpp
59	hostLobbyName can be taken directly from the primary source `g_XmppClient` rather than through JS indirection.

elexis mentioned this in rP21918: Use a banmask for multiplayer matches that have lobby-authentication enabled..Oct 25 2018, 2:00 PM

elexis mentioned this in D1666: Have mod_ipstamp insert hostJID and remove hardcoded 0ad JIDs.Nov 9 2018, 2:34 PM

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!

This commit now has outstanding concerns.Jan 13 2019, 5:35 PM

In rP21520#32227, @elexis wrote:

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.

Imarok marked an inline comment as done.Jan 13 2019, 7:29 PM

Imarok added inline comments.

/ps/trunk/source/network/NetServer.cpp
883	In rP21520#32227, @elexis wrote: Since this commit delayed the `OnAuthenticate` call which called `DeduplicatePlayerName` call, it should be this commit that broke the `duplicateplayernames` config setting. And it doesn't really break because we delay `OnAuthenticate`, but because we already set the username here in case lobby authentication is enabled.