MUC: don't forget password when handling nick conflicts

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. It adds a test for this case, and some of the basic
functionality of Password (which subsumes some incidental testing of the
Password interface in muc/presence-before-closing: the only test that
touched Password at all!).

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=39790
Reviewed-by: Jonny Lamb <jonny.lamb@collabora.co.uk>

3 files changed, 28 insertions, 37 deletions

diff --git a/src/muc-channel.c b/src/muc-channel.c
index ad916392..ebb829f1 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 311deb77..3bd42c7a 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/presence-before-closing.py b/tests/twisted/muc/presence-before-closing.py
index c221ca20..b89abef4 100644
--- a/tests/twisted/muc/presence-before-closing.py
+++ b/tests/twisted/muc/presence-before-closing.py
@@ -6,7 +6,9 @@ import dbus
 
 from twisted.words.xish import domish
 
-from gabbletest import exec_test, make_result_iq, request_muc_handle, wrap_channel
+from gabbletest import (
+    exec_test, make_result_iq, request_muc_handle, wrap_channel, elem,
+)
 from servicetest import (EventPattern, assertEquals, assertLength,
         assertContains, sync_dbus, call_async)
 import constants as cs
@@ -111,31 +113,27 @@ def test_with_password(q, bus, conn, stream):
             cs.TARGET_HANDLE_TYPE: cs.HT_ROOM,
             cs.TARGET_HANDLE: handle})
 
-    q.expect('stream-presence', to='chat@conf.localhost/test')
+    expected_muc_jid = '%s/%s' % (room, 'test')
+    q.expect('stream-presence', to=expected_muc_jid)
 
     # tell gabble the room needs a password
-    presence = domish.Element(('jabber:client', 'presence'))
-    presence['from'] = '%s/%s' % (room, 'test')
-    presence['type'] = 'error'
-    x = presence.addElement((ns.MUC, 'x'))
-    error = presence.addElement('error')
-    error['type'] = 'auth'
-    not_authorized = error.addElement((ns.STANZA, 'not-authorized'))
-    stream.send(presence)
-
-    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'])
-
-    # ensure gabble knows we need a password
-    flags = chan.Password.GetPasswordFlags()
-    assertEquals(cs.PASSWORD_FLAG_PROVIDE, flags)
-
-    forbidden = [EventPattern('stream-presence', to=room + '/test')]
+    stream.send(
+        elem('jabber:client', 'presence', from_=expected_muc_jid,
+            type='error')(
+          elem(ns.MUC, 'x'),
+          elem('error', type='auth')(
+            elem(ns.STANZA, 'not-authorized'),
+          ),
+        ))
+
+    cc, _ = q.expect_many(
+        EventPattern('dbus-return', method='CreateChannel'),
+        EventPattern('dbus-signal', signal='PasswordFlagsChanged',
+            args=[cs.PASSWORD_FLAG_PROVIDE, 0]))
+
+    chan = wrap_channel(bus.get_object(conn.bus_name, cc.value[0]), 'Text')
+
+    forbidden = [EventPattern('stream-presence', to=expected_muc_jid)]
     q.forbid_events(forbidden)
 
     # we call Close...