Database inconsistency errors during in-band registration

[haael]

Describe the bug

A client may leave the server database in inconsistent state.

1. If the database url doesn't have 'autoCreateUser=true', in-band creation will not work at all, but the server still returns a success response.

2. If the database url has 'autoCreateUser=true', the behaviour is as follows:

• on the first attempt, a user entry will be created in tig_users, but password will be unset
• on the next attempts, the password entry will be set / changed.

The server returns a success response in every case.

To Reproduce

import asyncio
import slixmpp

class Connection(slixmpp.ClientXMPP):
    def __init__(self, jid, password):
        self.__username = jid.split('@')[0]
        self.__password = password
        self.__login_failed = False

        super().__init__(jid, password)

        self.register_plugin('xep_0077')
        self.add_event_handler('connected', self.on_connected)
        self.add_event_handler('disconnected', self.on_disconnected)
        self.add_event_handler('failed_auth', self.on_failed_auth)
        self.add_event_handler('session_start', self.on_session_start)
        self.add_event_handler('session_end', self.on_session_end)

    def exception(self, exception):
        print("exception", exception)
        raise exception

    def on_connected(self, event):
        print("connected", event)

    def on_disconnected(self, event):
        print("disconnected", event)

    def on_failed_auth(self, event):
        print("failed_auth", event)
        self.__login_failed = True

    async def on_session_start(self, event):
        print("session_start", event)
        if self.__login_failed:
            print(" login failed, registering")

            iq = await (self['xep_0077'].get_registration())
            print(" recv:", iq)
            registration_form = iq['register']

            registration_request = self['xep_0077'].stanza.Register()
            registration_request.set_fields(registration_form.get_fields())
            for field in registration_request.get_fields():
                if field == 'username':
                    registration_request[field] = self.__username
                elif field == 'password':
                    registration_request[field] = self.__password
                elif field == 'email':
                    registration_request[field] = 'bobby@example.net'
                else:
                    print(" unsupported field:", field)
            iq = self['xep_0077'].xmpp.make_iq_set(registration_request)
            print(" send:", iq)
            print(" recv:", (await iq.send()))
            self.disconnect() # disconnect

        else:
            self.send_presence()
            self.get_roster()

            print(" login successful, unregistering")

            iq = await (self['xep_0077'].get_registration())
            print(" recv:", iq)
            registration_form = iq['register']

            registration_request = self['xep_0077'].stanza.Register()
            registration_request.set_fields(['remove'])
            iq = self['xep_0077'].xmpp.make_iq_set(registration_request)
            print(" send:", iq)
            await iq.send()
            # the server will disconnect

    def on_session_end(self, event):
        print("session_end", event)
        quit()

if __debug__ and __name__ == '__main__':
    connection = Connection('bobby@tigase.example.net/x12345', 'abcdef') # use tigase address server here, change username

    connection.connect()
    try:
        connection.process()
    except KeyboardInterrupt:
        connection.disconnect()
        connection.process(timeout=5)

Expected behavior

In-band registration works in a single step, the database stays consistent even when the client violates protocol.

Details (please complete the following information):

• Tigase version: [e.g. 8.0.0]
• JVM flavour and version [e.g. OpenJDK11]
• Operating system/distribution/version [e.g. Linux Ubuntu 20.04]

Additional context

I believe the problem lays in the file JabberIqRegister.java line 689:

if (session.isAuthorized()) {

The server treats the first registration as password change, not account creation.

Proposed workaround:

if (session.isAuthorized() && ! session.isAnonymous()) {

[woj-tek]

First of all:

even when the client violates protocol.

We can't guarantee that things work well if user violates the specification...

ad rem: could you share more details? 1) which exact tigase-server version do you use? 2) is it your deployment or some public server? 3) could you share complete configuration file (obfuscated)? 4) do you receive any error during registration? 5) what do you mean by "inconsistent state"?

[haael]

• We can't guarantee that things work well if user violates the specification...

The server still has to return the right error code and maintain the database in the right state.

1. which exact tigase-server version do you use?

8.0.0

2. is it your deployment or some public server?

My private deployment.

3. could you share complete configuration file (obfuscated)?

admins = [
    'admin@localhost'
]
'config-type' = 'default'
debug = [ 'server' ]
'default-virtual-host' = 'localhost'
dataSource () {
    default () {
        uri = 'jdbc:postgresql://localhost/tigase?user=tigase&password=2938hkjd230scvsd&useSSL=false&autoCreateUser=true'
    }
}
'ext-man' () {}
http () {}
pubsub () {
    trusted = [ 'http@{clusterNode}' ]
}
s2s (active: false) {}
'sess-man' () {
    'presence-offline' () {}
}
socks5 () {}
upload () {}

I also tried the database url without autoCreateUser=true.

4. do you receive any error during registration?

Without the autoCreateUser=true option:

2021-03-22 13:11:36.167 [jabber:iq:register Queue Worker 1]  RepositoryAccess.setRegistration()  WARNING: Problem accessing reposiotry: 
tigase.db.UserNotFoundException: User does not exist: ziom_3@localnet
    at tigase.db.jdbc.JDBCRepository.getUserUID(JDBCRepository.java:1174)
    at tigase.db.jdbc.JDBCRepository.setData(JDBCRepository.java:587)
    at tigase.db.jdbc.JDBCRepository.setData(JDBCRepository.java:637)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at tigase.stats.StatisticsInvocationHandler.invoke(StatisticsInvocationHandler.java:75)
    at com.sun.proxy.$Proxy35.setData(Unknown Source)
    at tigase.db.UserRepositoryMDImpl.setData(UserRepositoryMDImpl.java:320)
    at tigase.xmpp.RepositoryAccess.setRegistration(RepositoryAccess.java:721)
    at tigase.xmpp.impl.JabberIqRegister.doRegisterNewAccount(JabberIqRegister.java:686)
    at tigase.xmpp.impl.JabberIqRegister.process(JabberIqRegister.java:339)
    at tigase.server.xmppsession.SessionManager$ProcessorWorkerThread.process(SessionManager.java:2587)
    at tigase.util.processing.WorkerThread.run(WorkerThread.java:68)

• what do you mean by "inconsistent state"?

If I add the autoCreateUser=true option:

• Upon the first attempt of registration there is a user entry created in the table tig_users, but no corresponding password entry in the table tig_user_credentials.

• After the second registration attempt the credentials get created, probably because it is interpreted as password change request.

• After user unregistration the entries from tig_user_credentials are deleted, but the entry in tig_users stays there. Consequently, another attempt at registration works, because it is interpreted as password change.

I believe the problem lies in the check session.isAuthorized(), because it returns true even when the password check failed.

[woj-tek]

Thank you for all the details. Could you try reproduce it with the latest stable: 8.1.x?

[haael]

The last version uses data forms instead of plain fields, so please give me some time to write the test.

[haael]

I can confirm the bug is present in the last version. The server returns the success response but the user is not created. No error is printed on the console.

Expected behavior: user is created and the server returns success, or user is not created and the server returns error.