[PATCH] Display a nice error message when trying to register too many lobby accounts

[vincent]

We should display a nice error message when someone tries to register too many lobby accounts within a short period of time, as nobody knows about this limitation and wonders why the registration doesn't work.

At the moment it just displays "Unknown error" refs #3047 Fixed by workaround. Should be fixed properly, when gloox catches this error correctly (​​https://bugs.camaya.net/ticket/?id=267)

[Stan]

That's likely because you tried to create two accounts in a short while.

[Imarok]

A very ugly approach to fix it

[Imarok]

Change "Unknown Error"

[Vladislav Belov]

Shouldn't we modify the lobby bot to return normal error, instead of hack?

[Imarok]

I already discussed this with scythetwirler: There is no way to catch this error without rewriting gloox. The first patch shouldn't be committed because it's an ugly hack. The second patch is meant for review/commit and just adds a hint to "Unknown Error".

[elexis]

• Did scythe say why we'd have to rewrite gloox? (Why) Is it impossible/infeasible to change the "one account per ip per hour" rule of the lobbybot from an insta-disconnect to a sending of the dc reason + disconnect?

• What if we decide to make that 2 or 24 hours?

• What if someone else wants to use the engine without the public mod? There are many hardcoded 0ad logic things in the engine, but adding more won't improve that. (lobby.js would have to be moved to mods/mod/ eventually)

• It's not necessarily the same person but everyone with the same IP, the usual case is that two people on the same router both want an account.

• Assuming this default case is always hit when someone can't establish a connection, so it would be true if the server is down or if someone is IP-banned, right?

• Would it be better to return an empty string and add the custom explanation to lobby.js?

[Vladislav Belov]

Replying to Imarok:

I already discussed this with scythetwirler: There is no way to catch this error without rewriting gloox.

Rewriting or patching? If it's a small patch, we could do this or write to authors of gloox.

[Imarok]

I discussed that with scythe and he said, it won't be worth the effort catching the error.

[elexis]

• The word you try to use as a comparative in that sentence is actually an adverb
• s/Would it be/It is - at least as long as we describe a scenario in that string that is not true for every xmpp server, thus allowing future contributors and modders to change the string without having to recompile the entire thing and not add to the "public mod specific logic in cpp" problem.
• All of the translated strings (StanzaErrorToString, ConnectionErrorToString) could be moved to prelobby.js by adding a function that translates the (only in cpp well defined) enum to a string that can be easily parsed in JS, like GetPresenceString or GetRoleString. Should likely not be done as it would make it less mod friendly (requiring the modded game to copy the english strings defined in JS to the mod and either lose the translations from transifex or having to include all public mod translations in the mod or copying manually cutting out the strings from the public mod translation files (*.po)) and since those strings are true for all xmpp servers (as they derive from the protocol, not from our server policy)

• Hence prelobby.js: Engine.GetGUIObjectByName("feedback").caption = message.text || translate("...")) I guess, potentially an \n or moving it to g_UnknownDisconnectReason if too long

[Imarok]

Replying to elexis:

• Assuming this default case is always hit when someone can't establish a connection, so it would be true if the server is down or if someone is IP-banned, right?

If 0ad cannot establish the connection to the server it just freezes, so this is no issue (I think there was a ticket with this issue) The default case is just hit, if the server responds with an unknown error.

[elexis]

About the specs: Server: The XMPP server is an instance of ejabberd. We observe that ejabberd implements the XEP-0077 specification (​https://docs.ejabberd.im/admin/configuration/#modregister)

mod_register In-Band Registration (XEP-0077)

The registration limit is a configuration documented at ​https://docs.ejabberd.im/admin/configuration/:

This module reads also another option defined globally for the server: registration_timeout: Timeout. This option limits the frequency of registration from a given IP or username. So, a user that tries to register a new account from the same IP address or JID during this number of seconds after his previous registration will receive an error resource-constraint with the explanation: “Users are not allowed to register accounts so quickly”. The timeout is expressed in seconds, and it must be an integer. To disable this limitation, instead of an integer put a word like: infinity. Default value: 600 seconds.

Client: Our XmppClient uses gloox. We can verify that it's registration also implemented XEP-0077 and is documented at ​https://camaya.net/api/gloox/classgloox_1_1Registration.html

This class is an implementation of ​ XEP-0077 (In-Band Registration).

As linked from ​ xep-0077.html #errors, error codes are defined by ​ XEP-0086 and also include the mentioned resource-constraint type.

According to our ​ BuildInstructions , we require gloox 1.0.9 or later. Current gloox stable is 1.0.18: ​https://camaya.net/gloox/. If we download the 1.0.9 source, ​http://camaya.net/download/gloox-1.0.9.tar.bz2, we can see that StanzaErrorResourceConstraint is already supported.

gloox Registration::handleIqID of registration.cpp of the gloox code upstream ( ​https://camaya.net/api/gloox/registration_8cpp_source.html ) simply doesn't check for that error and ends up in the "default" case.

About the proper Solution: Updating to the latest stable wouldn't help, as that also doesn't provide the check we need.

If we could modify gloox, we could add one case catching that StanzaErrorResourceConstraint and add a new case to the enum RegistrationResult, we could gracefully handle the registration limit case.

Our current string for the case that is never triggered for registrations is:

DEBUG_CASE(StanzaErrorResourceConstraint, "The recipient is unable to process the message due to resource constraints");

Once we can catch that case, we can also catch the Text property sent within the Stanza and stop worrying about providing custom strings: Users are not allowed to register accounts so quickly. Perhaps the type property could be used, it is wait in our case.

Also refs ​https://www.ietf.org/rfc/rfc3920.txt

[elexis]

as we can see the data is actually sent by the lobby bot

[elexis]

Also the message we are looking for is hardcoded in src/mod_register.erl of ejabberd ( ​https://github.com/processone/ejabberd/blob/master/src/mod_register.erl#L356 ):

		      ErrText =
			  <<"Users are not allowed to register accounts "
			    "so quickly">>,
		      {error, xmpp:err_resource_constraint(ErrText, Lang)}

So even if we had it, we couldn't display how long the users have to wait.

[elexis]

Review part of the discussion moved to ​https://code.wildfiregames.com/D87 See ​https://code.wildfiregames.com/D87#3591 for a tested gloox and 0 A.D. patch that solve the issue correctly.

Gloox patch proposed at ​https://bugs.camaya.net/ticket/?id=267

[Imarok]

In 19205:

Display a nice error message when trying to register too many lobby accounts
(This is a workaround that will be removed when gloox has fixed the issue.)

Reviewed by: elexis
Refs #3771
Differential Revision: ​https://code.wildfiregames.com/D87

[elexis]

People should be able to create two accounts before the timeout takes effect: #4509

[Itms]

In 19608:

Update precompiled win32 gloox lib to 1.0.20 and rebuild glooxwrapper, fixes #4564, refs #3004.

This gloox version includes a change that would improve the user experience when registrations are disabled or limited, refs #3771.

Reviewed By: vladislavbelov

Differential Revision: ​https://code.wildfiregames.com/D483