Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support XEP-0421: Anonymous unique occupant identifiers for MUCs #3397

Closed
karimhm opened this issue Sep 22, 2020 · 8 comments
Closed

Support XEP-0421: Anonymous unique occupant identifiers for MUCs #3397

karimhm opened this issue Sep 22, 2020 · 8 comments
Assignees

Comments

@karimhm
Copy link

karimhm commented Sep 22, 2020

ejabberd doesn't seem to support XEP-0421: Anonymous unique occupant identifiers for MUCs. It would be nice to have support for it.

@lovetox
Copy link

lovetox commented Jun 14, 2023

Hi, i want to bump this issue.

This XEP solves a lot of problems which exist with anonymous MUCs.

  • It allows to do message correction better (you dont have to track leave and joins)
  • It allows to discover messages sent by another device of the user in a MUC
  • https://xmpp.org/extensions/xep-0424.html depends on it (Retractions)
  • It allows to track users across nick changes

Please consider adding this.

For Reference other Server Implementations like for example prosody already support this.

@mremond mremond added this to the ejabberd 23.xx milestone Jul 11, 2023
@badlop badlop self-assigned this Aug 2, 2023
@badlop
Copy link
Member

badlop commented Aug 2, 2023

I've implemented this feature in a new module. You can compile ejabberd from git, or download the installers or the container, then add to ejabberd.yml

modules:
  mod_muc_occupantid: {}
  ...

It would be great if you can try this in real world and report any problem before the next release.

@lovetox
Copy link

lovetox commented Aug 2, 2023

Great to hear

Some pointers about the implementation that clients may expect which are not directly stated in the XEP

  • occupant-id should be always attached to message and presence, it should not depend on the anonymous setting of the MUC, this is mostly because it makes implementation for client much easier, rather than switching between real jid and occupant-id depending on the MUC setting
  • occupant-id needs to be attached to all MAM messages
  • occupant-id needs to be attached to the MUC subject, if ejabberd supports that the MUC subject comes from a occupant resource (so signal who is the author of the subject) -> i guess clients could live without this depending on how hard it is to implement this, but i just wanted to note it.

i try and test this, thanks

@lovetox
Copy link

lovetox commented Aug 2, 2023

I looked at your implementation of the secret

and

calculate_occupantid(From, RoomJid) ->
    Term = {jid:remove_resource(From), RoomJid, erlang:get_cookie()},

get_cookie() sounds like not a good idea.
the secret must be stable, forever, across new installations, restore from backup, migrating to other machines etc

the occupant-id in best case should never change. With probably the exception, if the server operator loses his whole database, its probably ok to regenerate the occupant-ids, but then its on him for not having proper backups.

Ideas i heard were:

  • generate a secret per room and store it in the room config (hidden of course, users should not be able to see or edit it)
  • Define a secret in ejabberd config that the admin has to set, maybe provide means of generating the secret

@badlop
Copy link
Member

badlop commented Aug 4, 2023

  • occupant-id should be always attached to message and presence, it should not depend on the anonymous setting of the MUC ➡️ done
  • occupant-id needs to be attached to all MAM messages ➡️ hopefully done. Please test yourself
  • occupant-id needs to be attached to the MUC subject ➡️ done
  • the secret must be stable, forever, across new installations ➡️ i've implemented as the XEP and prosody do

I've implemented your suggestions. As they are still experimental, I've committed in a branch in my fork, see master...badlop:ejabberd:3397c

If the tests succeed, there will be installers in https://github.com/badlop/ejabberd/actions/runs/5764017825 and container in https://github.com/badlop/ejabberd/pkgs/container/ejabberd

@lovetox
Copy link

lovetox commented Aug 5, 2023

i did now do a basic test

Attaching occupant-id to MAM, subject, messages independent of muc config, seems to work

I did not do indepth tests if the ID generation is now stable, i have to trust your impl here.

What i found is that for moderated messages the occupant id should also be added

<message xmlns="jabber:client" to="lovetox@temptatio.dev/gajim.B937Y3JD" from="ipijoq@conference.temptatio.dev" type="groupchat">
  <apply-to id="1691239200105665" xmlns="urn:xmpp:fasten:0">
    <moderated by="ipijoq@conference.temptatio.dev/lovetox" xmlns="urn:xmpp:message-moderate:0">
      <reason>Spam</reason>
      <retract xmlns="urn:xmpp:message-retract:0" />
</moderated>
</apply-to>
</message>

See https://xmpp.org/extensions/xep-0425.html#tombstones

It should be added to the tombstone, but also to the live message

Are there any other messages that are issue by MUCs? nothing comes to mind currently.

@badlop
Copy link
Member

badlop commented Aug 7, 2023

for moderated messages the occupant id should also be added

Ok, I added that for live messages. ejabberd does not support tombstones. BTW, there was a bug in message moderation, fixed now.

badlop added a commit that referenced this issue Aug 16, 2023
… subject (#3397)

When changing the room subject, store the original author JID,
so later it can be provided in the hook and mod_room_occupantid
can use it to calculate and provide the occupant id

This is noticeable when a new occupant joins an existing room,
and receives the room subject.
@badlop
Copy link
Member

badlop commented Aug 16, 2023

I've merged all those fixes. It would be great if you can give it another test

@badlop badlop closed this as completed Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants