`err_registration_required` no longer sent on join after unsubscription

xela85 commented 1 year ago

Environment

ejabberd version: 22.10
Erlang version: 25
OS: macos
Installed from: source

Errors from error.log/crash.log

No errors

Bug description

Hello,

I am currently trying to upgrade ejabberd from 22.05 to 22.10. Our instance is using members-only rooms. In order to remove a participant from a specific MUC, a call to unsubscribe is made. In 22.05, that call leads subsequent calls to join to trigger a presence error of this kind :

%Romeo.Stanza.Presence{
        from: %Romeo.JID{
          user: visitor_name,
          full: visitor_jid,
          resource: ^visitor_nick
        },
        to: %Romeo.JID{
          resource: ^visitor_resource,
          user: ^visitor_name
        },
        xml:
          {:xmlel, "presence", [_, _, _, {"type", "error"}],
           [
             _,
             {:xmlel, "error", [{"type", "auth"}], [_, {:xmlel, "text", _, [xmlcdata: "Membership is required to enter this room"]}]}
           ]}
      }

After upgrading to 22.10, I no longer see this message when trying to join room. By looking at https://github.com/processone/ejabberd/blob/1818a29c29290ae51382f123a3d592e8710ac640/src/mod_muc_room.erl#L2283 I can see that this stanza is sent when role is none. https://github.com/processone/ejabberd/issues/3330 seems to disable setting role to none after unsubscription

Do you confirm that there is an issue with this pattern matching ?

Thanks in advance

licaon-kter commented 1 year ago

You've read the upgrade notes, yes? https://docs.ejabberd.im/admin/upgrade/from_22.05_to_22.10/

Also, why not upgrade to latest 23.04? :)

prefiks commented 1 year ago

Could you tell me whole situation? Is that you give user membership to room, and add it as mucsubscriber, then when you want to remove it, you remove mucsub entry and expect this to also remove membership affiliation?

xela85 commented 1 year ago

I create a members-only room
The user is subscribed, and it gets its affiliation set to :member: :mod_muc_room.change_item(pid, jid, :affiliation, :member, "")
I want to kick the user, so I call :mod_muc_room.unsubscribe(pid, jid) and :mod_muc_room.change_item(pid, jid, :affiliation, :none, "")
Affiliation is correctly removed, but role keeps being :participant
When trying to join room, I no longer receive Membership is required to enter this room

You've read the upgrade notes, yes? https://docs.ejabberd.im/admin/upgrade/from_22.05_to_22.10/

I have read https://www.process-one.net/blog/ejabberd-22-10/, hooks have changed, but it seems unrelated in my opinion

Also, why not upgrade to latest 23.04? :)

I prefer upgrading releases step by step to avoid breaking production

prefiks commented 1 year ago

I commited 3eecf4ae8a5cfc68164dde905ed073482167dd18 that should fix this. And looks like this was introduced as part of issue #3330

xela85 commented 1 year ago

Thanks for your quick response ! Doesn't it break the initial purpose of https://github.com/processone/ejabberd/issues/3330 (keeping role even if user is kicked) ? Cf this comment :

%% maintain the same role they had *before* they were kicked,
%% unless they were banned

prefiks commented 1 year ago

Without this you can't change role back to none so i think this is correct thing to do here.

processone / ejabberd