From 87fe34250e68eb0a2199265d9ef0329a0eb20b3a Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Wed, 3 Aug 2011 08:32:16 +0100 Subject: [PATCH] MUC: don't forget password when handling nick conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WockyMuc has a property, :password, representing the current password being used to join the MUC. GabbleMuc previously had a private variable, 'password', which was used for this. In the port to WockyMuc, setting the private variable was removed, but it was still used when re-attempting to join after a nick conflict. (I think the password should be a parameter to a hypothetical wocky_muc_join_async() which does all the nick conflict crap for you. Having this as state that kicks around on the WockyMuc for ever is bizarre—once you're in the room, you usually don't use the password, unless you're the owner, in which case you can retrieve the current password *which may be different*!) This patch expunges the zombie private variable, and ensures WockyMuc:password is only set when the user provides a password, not at every join attempt. --- src/muc-channel.c | 18 ++------ tests/twisted/Makefile.am | 1 + tests/twisted/muc/password.py | 84 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 tests/twisted/muc/password.py diff --git a/src/muc-channel.c b/src/muc-channel.c index ad91639..ebb829f 100644 --- a/src/muc-channel.c +++ b/src/muc-channel.c @@ -230,7 +230,6 @@ struct _GabbleMucChannelPrivate TpChannelPasswordFlags password_flags; DBusGMethodInvocation *password_ctx; - gchar *password; const gchar *jid; gboolean requested; @@ -803,12 +802,10 @@ create_room_identity (GabbleMucChannel *chan) } static void -send_join_request (GabbleMucChannel *gmuc, - const gchar *password) +send_join_request (GabbleMucChannel *gmuc) { GabbleMucChannelPrivate *priv = gmuc->priv; - g_object_set (priv->wmuc, "password", password, NULL); wocky_muc_join (priv->wmuc, NULL); } @@ -1194,8 +1191,6 @@ gabble_muc_channel_finalize (GObject *object) g_string_free (priv->self_jid, TRUE); } - g_free (priv->password); - if (priv->initial_channels != NULL) { g_boxed_free (TP_ARRAY_TYPE_OBJECT_PATH_LIST, priv->initial_channels); @@ -1365,10 +1360,6 @@ channel_state_changed (GabbleMucChannel *chan, priv->poll_timer_id = g_timeout_add_seconds (interval, timeout_poll, chan); - - /* no need to keep this around any longer, if it's set */ - g_free (priv->password); - priv->password = NULL; } else if (new_state == MUC_STATE_ENDED) { @@ -1518,7 +1509,7 @@ handle_nick_conflict (GabbleMucChannel *chan, tp_handle_unref (contact_repo, self_handle); priv->nick_retry_count++; - send_join_request (chan, priv->password); + send_join_request (chan); return TRUE; } @@ -2941,7 +2932,8 @@ gabble_muc_channel_provide_password (TpSvcChannelInterfacePassword *iface, } else { - send_join_request (self, password); + g_object_set (priv->wmuc, "password", password, NULL); + send_join_request (self); priv->password_ctx = context; } } @@ -3074,7 +3066,7 @@ gabble_muc_channel_add_member (GObject *obj, tp_intset_destroy (set_remote_pending); /* seek to enter the room */ - send_join_request (self, NULL); + send_join_request (self); g_object_set (obj, "state", MUC_STATE_INITIATED, NULL); /* deny adding */ diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am index 311deb7..3bd42c7 100644 --- a/tests/twisted/Makefile.am +++ b/tests/twisted/Makefile.am @@ -25,6 +25,7 @@ TWISTED_TESTS = \ muc/chat-states.py \ muc/kicked.py \ muc/name-conflict.py \ + muc/password.py \ muc/presence-before-closing.py \ muc/renamed.py \ muc/roomlist.py \ diff --git a/tests/twisted/muc/password.py b/tests/twisted/muc/password.py new file mode 100644 index 0000000..7a67d17 --- /dev/null +++ b/tests/twisted/muc/password.py @@ -0,0 +1,84 @@ +from gabbletest import exec_test, elem, request_muc_handle, make_muc_presence +from servicetest import call_async, EventPattern, wrap_channel, assertEquals +import constants as cs +import ns + +def expect_attempt(q, expected_muc_jid, expected_password): + e = q.expect('stream-presence', to=expected_muc_jid) + x = e.stanza.elements(uri=ns.MUC, name='x').next() + + p = x.firstChildElement() + assertEquals(expected_password, str(p)) + +def test(q, bus, conn, stream): + room = 'chat@conf.localhost' + handle = request_muc_handle(q, conn, stream, room) + + call_async(q, conn.Requests, 'CreateChannel', { + cs.CHANNEL_TYPE: cs.CHANNEL_TYPE_TEXT, + cs.TARGET_HANDLE_TYPE: cs.HT_ROOM, + cs.TARGET_HANDLE: handle}) + + expected_muc_jid = '%s/%s' % (room, 'test') + q.expect('stream-presence', to=expected_muc_jid) + + # tell gabble the room needs a password + denied = \ + elem('jabber:client', 'presence', from_=expected_muc_jid, + type='error')( + elem(ns.MUC, 'x'), + elem('error', type='auth')( + elem(ns.STANZA, 'not-authorized'), + ), + ) + stream.send(denied) + + cc, _, _ = q.expect_many( + EventPattern('dbus-return', method='CreateChannel'), + EventPattern('dbus-signal', signal='NewChannels'), + EventPattern('dbus-signal', signal='PasswordFlagsChanged', + args=[cs.PASSWORD_FLAG_PROVIDE, 0])) + + chan = wrap_channel(bus.get_object(conn.bus_name, cc.value[0]), 'Text', + ['Password']) + + flags = chan.Password.GetPasswordFlags() + assertEquals(cs.PASSWORD_FLAG_PROVIDE, flags) + + call_async(q, chan.Password, 'ProvidePassword', 'brand new benz') + expect_attempt(q, expected_muc_jid, 'brand new benz') + + # Try again while the first attempt is outstanding. Gabble should say no. + call_async(q, chan.Password, 'ProvidePassword', 'faster faster') + q.expect('dbus-error', method='ProvidePassword') + + # Sorry, wrong password. + stream.send(denied) + ret = q.expect('dbus-return', method='ProvidePassword') + assert not ret.value[0] + + call_async(q, chan.Password, 'ProvidePassword', 'bougie friends') + expect_attempt(q, expected_muc_jid, 'bougie friends') + + # Well, this may be the right password, but actually that nick is in use. + presence = elem('presence', from_=expected_muc_jid, type='error')( + elem(ns.MUC, 'x'), + elem('error', type='cancel')( + elem(ns.STANZA, 'conflict'), + )) + stream.send(presence) + + # Okay, so Gabble tries again, with a new JID *and the same password*. + expected_muc_jid = expected_muc_jid + '_' + expect_attempt(q, expected_muc_jid, 'bougie friends') + + # Hey this worked. + stream.send(make_muc_presence('none', 'participant', room, 'test_')) + ret, _ = q.expect_many( + EventPattern('dbus-return', method='ProvidePassword'), + EventPattern('dbus-signal', signal='PasswordFlagsChanged', + args=[0, cs.PASSWORD_FLAG_PROVIDE])) + assert ret.value[0] + +if __name__ == '__main__': + exec_test(test) -- 1.7.5.4