Bugfix. Don't remove affiliated members on probe response

by checking for the affiliation data on the probe response presence and
not on the occupant model (which might be created from a message and
therefore not yet have up to date affiliation data).
This commit is contained in:
JC Brand 2020-06-15 12:23:52 +02:00
parent 8008a2af9b
commit 6b55907ddd
2 changed files with 55 additions and 26 deletions

View File

@ -1633,23 +1633,21 @@ describe("Groupchats", function () {
async function (done, _converse) { async function (done, _converse) {
await mock.openAndEnterChatRoom(_converse, 'lounge@montague.lit', 'romeo'); await mock.openAndEnterChatRoom(_converse, 'lounge@montague.lit', 'romeo');
var view = _converse.chatboxviews.get('lounge@montague.lit'), var view = _converse.chatboxviews.get('lounge@montague.lit');
occupants = view.el.querySelector('.occupant-list'); const occupants = view.el.querySelector('.occupant-list');
var presence;
for (var i=0; i<mock.chatroom_names.length; i++) { for (var i=0; i<mock.chatroom_names.length; i++) {
const name = mock.chatroom_names[i]; const name = mock.chatroom_names[i];
const role = mock.chatroom_roles[name].role;
// See example 21 https://xmpp.org/extensions/xep-0045.html#enter-pres // See example 21 https://xmpp.org/extensions/xep-0045.html#enter-pres
presence = $pres({ const presence = $pres({
to:'romeo@montague.lit/pda', to:'romeo@montague.lit/pda',
from:'lounge@montague.lit/'+name from:'lounge@montague.lit/'+name
}).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'}) }).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'})
.c('item').attrs({ .c('item').attrs({
affiliation: 'none', affiliation: 'none',
jid: name.replace(/ /g,'.').toLowerCase() + '@montague.lit', jid: name.replace(/ /g,'.').toLowerCase() + '@montague.lit',
role: role role: 'participant'
}).up() }).up()
.c('status').attrs({code:'110'}).nodeTree; .c('status');
_converse.connection._dataRecv(mock.createRequest(presence)); _converse.connection._dataRecv(mock.createRequest(presence));
} }
@ -1667,16 +1665,16 @@ describe("Groupchats", function () {
for (i=mock.chatroom_names.length-1; i>-1; i--) { for (i=mock.chatroom_names.length-1; i>-1; i--) {
const name = mock.chatroom_names[i]; const name = mock.chatroom_names[i];
// See example 21 https://xmpp.org/extensions/xep-0045.html#enter-pres // See example 21 https://xmpp.org/extensions/xep-0045.html#enter-pres
presence = $pres({ const presence = $pres({
to:'romeo@montague.lit/pda', to:'romeo@montague.lit/pda',
from:'lounge@montague.lit/'+name, from:'lounge@montague.lit/'+name,
type: 'unavailable' type: 'unavailable'
}).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'}) }).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'})
.c('item').attrs({ .c('item').attrs({
affiliation: mock.chatroom_roles[name].affiliation, affiliation: "none",
jid: name.replace(/ /g,'.').toLowerCase() + '@montague.lit', jid: name.replace(/ /g,'.').toLowerCase() + '@montague.lit',
role: 'none' role: 'none'
}).nodeTree; });
_converse.connection._dataRecv(mock.createRequest(presence)); _converse.connection._dataRecv(mock.createRequest(presence));
expect(occupants.querySelectorAll('li').length).toBe(i+1); expect(occupants.querySelectorAll('li').length).toBe(i+1);
} }
@ -5308,7 +5306,7 @@ describe("Groupchats", function () {
const muc_jid = 'lounge@montague.lit'; const muc_jid = 'lounge@montague.lit';
await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo'); await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
const stanza = u.toStanza(` let stanza = u.toStanza(`
<message xmlns="jabber:client" to="${_converse.jid}" type="groupchat" from="${muc_jid}/ralphm"> <message xmlns="jabber:client" to="${_converse.jid}" type="groupchat" from="${muc_jid}/ralphm">
<body>This message will trigger a presence probe</body> <body>This message will trigger a presence probe</body>
</message>`); </message>`);
@ -5316,21 +5314,21 @@ describe("Groupchats", function () {
const view = _converse.chatboxviews.get(muc_jid); const view = _converse.chatboxviews.get(muc_jid);
await u.waitUntil(() => view.model.messages.length); await u.waitUntil(() => view.model.messages.length);
const occupant = view.model.messages.at(0)?.occupant; let occupant = view.model.messages.at(0)?.occupant;
expect(occupant).toBeDefined(); expect(occupant).toBeDefined();
expect(occupant.get('nick')).toBe('ralphm'); expect(occupant.get('nick')).toBe('ralphm');
expect(occupant.get('affiliation')).toBeUndefined(); expect(occupant.get('affiliation')).toBeUndefined();
expect(occupant.get('role')).toBeUndefined(); expect(occupant.get('role')).toBeUndefined();
const sent_stanzas = _converse.connection.sent_stanzas; const sent_stanzas = _converse.connection.sent_stanzas;
const probe = await u.waitUntil(() => sent_stanzas.filter(s => s.matches('presence[type="probe"]')).pop()); let probe = await u.waitUntil(() => sent_stanzas.filter(s => s.matches('presence[type="probe"]')).pop());
expect(Strophe.serialize(probe)).toBe( expect(Strophe.serialize(probe)).toBe(
`<presence to="${muc_jid}/ralphm" type="probe" xmlns="jabber:client">`+ `<presence to="${muc_jid}/ralphm" type="probe" xmlns="jabber:client">`+
`<priority>0</priority>`+ `<priority>0</priority>`+
`<c hash="sha-1" node="https://conversejs.org" ver="Hxbsr5fazs62i+O0GxIXf2OEDNs=" xmlns="http://jabber.org/protocol/caps"/>`+ `<c hash="sha-1" node="https://conversejs.org" ver="Hxbsr5fazs62i+O0GxIXf2OEDNs=" xmlns="http://jabber.org/protocol/caps"/>`+
`</presence>`); `</presence>`);
const presence = u.toStanza( let presence = u.toStanza(
`<presence xmlns="jabber:client" to="${converse.jid}" from="${muc_jid}/ralphm"> `<presence xmlns="jabber:client" to="${converse.jid}" from="${muc_jid}/ralphm">
<x xmlns="http://jabber.org/protocol/muc#user"> <x xmlns="http://jabber.org/protocol/muc#user">
<item affiliation="member" jid="ralph@example.org/Conversations.ZvLu" role="participant"/> <item affiliation="member" jid="ralph@example.org/Conversations.ZvLu" role="participant"/>
@ -5338,6 +5336,39 @@ describe("Groupchats", function () {
</presence>`); </presence>`);
_converse.connection._dataRecv(mock.createRequest(presence)); _converse.connection._dataRecv(mock.createRequest(presence));
expect(occupant.get('affiliation')).toBe('member');
expect(occupant.get('role')).toBe('participant');
// Check that unavailable but affiliated occupants don't get destroyed
stanza = u.toStanza(`
<message xmlns="jabber:client" to="${_converse.jid}" type="groupchat" from="${muc_jid}/gonePhising">
<body>This message from an unavailable user will trigger a presence probe</body>
</message>`);
_converse.connection._dataRecv(mock.createRequest(stanza));
await u.waitUntil(() => view.model.messages.length === 2);
occupant = view.model.messages.at(1)?.occupant;
expect(occupant).toBeDefined();
expect(occupant.get('nick')).toBe('gonePhising');
expect(occupant.get('affiliation')).toBeUndefined();
expect(occupant.get('role')).toBeUndefined();
probe = await u.waitUntil(() => sent_stanzas.filter(s => s.matches(`presence[to="${muc_jid}/gonePhising"]`)).pop());
expect(Strophe.serialize(probe)).toBe(
`<presence to="${muc_jid}/gonePhising" type="probe" xmlns="jabber:client">`+
`<priority>0</priority>`+
`<c hash="sha-1" node="https://conversejs.org" ver="Hxbsr5fazs62i+O0GxIXf2OEDNs=" xmlns="http://jabber.org/protocol/caps"/>`+
`</presence>`);
presence = u.toStanza(
`<presence xmlns="jabber:client" type="unavailable" to="${converse.jid}" from="${muc_jid}/gonePhising">
<x xmlns="http://jabber.org/protocol/muc#user">
<item affiliation="member" jid="gonePhishing@example.org/d34dBEEF" role="participant"/>
</x>
</presence>`);
_converse.connection._dataRecv(mock.createRequest(presence));
expect(view.model.occupants.length).toBe(3);
expect(occupant.get('affiliation')).toBe('member'); expect(occupant.get('affiliation')).toBe('member');
expect(occupant.get('role')).toBe('participant'); expect(occupant.get('role')).toBe('participant');
done(); done();

View File

@ -1612,17 +1612,15 @@ converse.plugins.add('converse-muc', {
return true; return true;
} }
const occupant = this.occupants.findOccupant(data); const occupant = this.occupants.findOccupant(data);
if (data.type === 'unavailable' && occupant) { // Destroy an unavailable occupant if this isn't a nick change operation and if they're not affiliated
if (!data.states.includes(converse.MUC_NICK_CHANGED_CODE) && !occupant.isMember()) { if (data.type === 'unavailable' && occupant &&
// We only destroy the occupant if this is not a nickname change operation. !data.states.includes(converse.MUC_NICK_CHANGED_CODE) &&
// and if they're not on the member lists. !['admin', 'owner', 'member'].includes(data['affiliation'])) {
// Before destroying we set the new data, so // Before destroying we set the new data, so that we can show the disconnection message
// that we can show the disconnection message.
occupant.set(data); occupant.set(data);
occupant.destroy(); occupant.destroy();
return; return;
} }
}
const jid = data.jid || ''; const jid = data.jid || '';
const attributes = Object.assign(data, { const attributes = Object.assign(data, {
'jid': Strophe.getBareJidFromJid(jid) || occupant?.attributes?.jid, 'jid': Strophe.getBareJidFromJid(jid) || occupant?.attributes?.jid,