OF-2089 XEP-0045 7.2.13 - ofrom adresses in message stanza

[nsobadzhiev]

Introduction

In non-anonymous rooms, participants can/should see the others' real JID. That's usually done via presences. However, according to XEP-0045, section 7.2.13 a server MAY send the real JID also as an "addresses" stanza with type "ofrom" within the message.

This PR implements that. I opted to add the field if canAnyoneDiscoverJID is set to true for the room. Ofc, a more sophisticated implementation might include other cases based on the participant's role.

Testing

I wanted to write some tests for this, but LocalMUCRoom had some difficult to mock dependencies to singleton classes (namely the MUCPersistenceManager that is used in the constructor).

[akrherz]

Thanks, this change seems innocuous to me, @guusdk you have thoughts for 4.6.0 inclusion or punt it to 4.7.0 or ?

[guusdk]

I wouldn't mind adding this to 4.6.0. However, I would like to see a property that can be used to disable this functionality. My reasoning is that this change, although perfectly compliant with specification, might break a third-party client implementation. I'd like to offer them the ability to switch off the addition of the element introduced in this PR.

[nsobadzhiev]

You mean a system property?
I think since it's just adding properties to the message XML, it shouldn't break existing implementations, but you never know. I'll add it if you think it's needed. What name would you like? Should it be a system property via JiveGlobals?

[akrherz]

I am sort of wondering how this could break a client as well since it is purely additional?

[guusdk]

Ideally a system property using JiveGlobals, yes. I'm not to worried by the name of the property. If you make an effort to keep things roughly the same with the (rather loose) conventions that we use on property naming elsewhere in the code, that'll be more than enough.

I don't expect this change to introduce code breakage in Openfire directly. It'll be fine. However:

• I've seen weirder things in third party code that break under ... questionable ... circumstances.
• This touches on privacy-sensitive information.

Even though I think it would be fine to not having any, implementing a 'kill switch' adds some flexibility for the above. It doesn't really come at a cost for us (it's pretty easy to do, and adds next to no complexity), so I'm not seeing a downside to adding it.

[guusdk]

This is far from a hill for me to die on, though. If (sightly) pressed, I'm happy to accept this PR as-is.

[akrherz]

• This touches on privacy-sensitive information

Agreed, but this code is protected by the canAnyoneDiscoverJID boolean already, so a privacy minded server admin would already have that set.

[guusdk]

Not a hill for me to die on.

[guusdk]

Upon closer inspection, I wonder if the provided fix is quite correct. The XEP mentions that this attribute can be added to messages, but specifies this only when messages are sent as part of message archive (the attribute is not defined for ‘live’ messsages).

For live messages, the real JID can be discovered via presence information that was already sent by occupants joining the room.

I’m thinking that adding ‘ofrom’ to all messages, as was done by this fix, is at best redundant. I’m not sure if this introduces (privacy/security?) issues.

[guusdk]

Also, to be consistent, I think we want this added to private messages shared through MUC. I've fixed that in #1768