diff --git a/i18n/src/main/resources/openfire_i18n.properties b/i18n/src/main/resources/openfire_i18n.properties index 261236a47e..dafee6ee9d 100644 --- a/i18n/src/main/resources/openfire_i18n.properties +++ b/i18n/src/main/resources/openfire_i18n.properties @@ -319,6 +319,7 @@ muc.roomIsNowMembersOnly=This room is now members-only. muc.extended.info.desc=Description muc.extended.info.subject=Subject +muc.extended.info.changesubject=Subject can be modified muc.extended.info.occupants=Number of occupants muc.extended.info.creationdate=Creation date muc.extended.info.avatarhash=Avatar hash diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java index 37d4647c20..2a76c65739 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java @@ -1705,8 +1705,13 @@ public void broadcast(final @Nonnull Presence presence, final boolean isJoinPres // Determine what stanza flavor to send to this occupant. final Presence toSend; - if (occupant.getPresence().getFrom().equals(presence.getTo())) { + if (occupant.getOccupantJID().equals(presence.getFrom())) { // This occupant is the subject of the stanza. Send the 'self-presence' stanza. + // XEP-0045 §7.2.2: "the self-presence MUST include a status code of 110 so that + // the user knows this presence refers to itself as an occupant." + // Use presence.getFrom() (always set to the occupant JID by setPresence()) rather + // than presence.getTo(), which can be null or a bare room JID when a client omits + // the resource part, causing self-presence detection to silently fail. Log.trace( "Sending self-presence of '{}' to {}", presence.getFrom(), occupant.getUserAddress() ); toSend = selfPresence; } else if ( !canAnyoneDiscoverJID && Role.moderator != occupant.getRole() ) { diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java index 3740bfe913..dc1a527358 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java @@ -180,6 +180,13 @@ private void handleItemsElement(@Nonnull final Affiliation senderAffiliation, @N && Affiliation.owner != senderAffiliation) { throw new ForbiddenException(); } + // XEP-0045 §5.2.1: In semi-anonymous rooms, members SHOULD NOT be allowed to + // retrieve the member list (as that list MUST contain the JID of each member). + if (!room.canAnyoneDiscoverJID() + && Affiliation.admin != senderAffiliation + && Affiliation.owner != senderAffiliation) { + throw new ForbiddenException(); + } for (JID jid : room.getMembers()) { if (GroupJID.isGroup(jid)) { try { diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java index 754d2fd5ca..c90dba6784 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java @@ -3144,6 +3144,12 @@ public Set getExtendedInfos(String name, String node, JID senderJID) { fieldSubj.setLabel(LocaleUtils.getLocalizedString("muc.extended.info.subject", preferredLocale)); fieldSubj.addValue(room.getSubject()); + final FormField fieldChangeSubject = dataForm.addField(); + fieldChangeSubject.setVariable("muc#roomconfig_changesubject"); + fieldChangeSubject.setType(FormField.Type.boolean_type); + fieldChangeSubject.setLabel(LocaleUtils.getLocalizedString("muc.extended.info.changesubject", preferredLocale)); + fieldChangeSubject.addValue(room.canOccupantsChangeSubject() ? "1" : "0"); + final FormField fieldOcc = dataForm.addField(); fieldOcc.setVariable("muc#roominfo_occupants"); fieldOcc.setType(FormField.Type.text_single); diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java b/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java index c92dd83fff..8ab31a4e65 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java @@ -184,7 +184,17 @@ protected void deletingNode() { public synchronized void setLastPublishedItem(PublishedItem item) { - if ((lastPublished == null) || (item != null) && item.getCreationDate().after(lastPublished.getCreationDate())) { + // Always update when: + // 1. There is no last-published item yet (initial state). + // 2. The incoming item overwrites the current last-published item (same unique identifier). + // XEP-0060 §7.1.2 requires the server to replace an existing item with the same ID. + // Even if both items share the same creation-date (e.g. published in the same millisecond), + // the in-memory cache must reflect the new payload so that getPublishedItem() does not + // serve the stale first item. + // 3. The incoming item is strictly newer than the current last-published item. + if (item != null && (lastPublished == null + || lastPublished.getUniqueIdentifier().equals(item.getUniqueIdentifier()) + || item.getCreationDate().after(lastPublished.getCreationDate()))) { Log.trace("Set last published item to: {}", item.getID()); lastPublished = item; } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java new file mode 100644 index 0000000000..7475e4c659 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java @@ -0,0 +1,972 @@ +/* + * Copyright (C) 2025 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.muc; + +import org.dom4j.Element; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.group.ConcurrentGroupList; +import org.jivesoftware.openfire.group.ConcurrentGroupMap; +import org.jivesoftware.openfire.user.UserAlreadyExistsException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.xmpp.packet.JID; +import org.xmpp.packet.Presence; + +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +/** + * Unit tests that verify the XEP-0045 compliance of {@link MUCRoom}'s join precondition checks + * and self-presence status code generation. + * + * References: + * + */ +@ExtendWith(MockitoExtension.class) +public class MUCRoomJoinTest { + + @Mock(strictness = Mock.Strictness.LENIENT) + private MultiUserChatService mockService; + + @Mock(strictness = Mock.Strictness.LENIENT) + private XMPPServer xmppServer; + + private MUCRoom room; + + private static final JID ROOM_JID = new JID("testroom", "conference.example.org", null); + private static final JID USER_A = new JID("usera@example.org/res1"); + private static final JID USER_B = new JID("userb@example.org/res1"); + + @BeforeEach + public void setup() throws Exception { + doReturn("conference").when(mockService).getServiceName(); + doReturn("conference.example.org").when(mockService).getServiceDomain(); + doReturn(null).when(mockService).getMUCDelegate(); + doReturn(false).when(mockService).isSysadmin(any(JID.class)); + doReturn(true).when(mockService).isPasswordRequiredForSysadminsToJoinRoom(); + + //noinspection deprecation + XMPPServer.setInstance(xmppServer); + + room = new MUCRoom(); + setField(room, "mucService", mockService); + setField(room, "name", "testroom"); + setField(room, "owners", new ConcurrentGroupList<>()); + setField(room, "admins", new ConcurrentGroupList<>()); + setField(room, "members", new ConcurrentGroupMap<>()); + setField(room, "outcasts", new ConcurrentGroupList<>()); + setField(room, "occupants", new CopyOnWriteArrayList<>()); + setField(room, "lockedTime", 0L); + setField(room, "maxUsers", 0); // 0 = unlimited + setField(room, "membersOnly", false); + setField(room, "password", null); + setField(room, "loginRestrictedToNickname", false); + setField(room, "canAnyoneDiscoverJID", true); + setField(room, "logEnabled", false); + setField(room, "isDestroyed", false); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.7: Banned Users + // "If the user has been banned from the room (i.e., has an affiliation of + // 'outcast'), the service MUST deny access to the room and inform the user + // of the fact that they are banned; this is done by returning a presence + // stanza of type 'error' specifying a error condition" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_OutcastCannotJoin() throws Exception { + // XEP-0045 §7.2.7: A banned user (outcast) MUST be denied access. + assertThrowsPrecondition( + "checkJoinRoomPreconditionIsOutcast", + ForbiddenException.class, + USER_A, Affiliation.outcast + ); + } + + @Test + public void testJoinPrecondition_NonOutcastCanJoin() throws Exception { + // Non-outcasts MUST be allowed to pass the outcast check. + assertNoPreconditionException( + "checkJoinRoomPreconditionIsOutcast", + USER_A, Affiliation.none + ); + assertNoPreconditionException( + "checkJoinRoomPreconditionIsOutcast", + USER_A, Affiliation.member + ); + assertNoPreconditionException( + "checkJoinRoomPreconditionIsOutcast", + USER_A, Affiliation.admin + ); + assertNoPreconditionException( + "checkJoinRoomPreconditionIsOutcast", + USER_A, Affiliation.owner + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.6: Members-Only Rooms + // "If the room is members-only but the user is not on the member list, the + // service MUST deny access to the room and inform the user that they are + // not allowed to enter the room; this is done by returning a presence + // stanza of type 'error' specifying a error" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_MembersOnlyRoom_NonMember_MustBeRejected() throws Exception { + // XEP-0045 §7.2.6: Unaffiliated user MUST NOT enter a members-only room. + setField(room, "membersOnly", true); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionMemberOnly", + RegistrationRequiredException.class, + USER_A, Affiliation.none + ); + } + + @Test + public void testJoinPrecondition_MembersOnlyRoom_Member_CanJoin() throws Exception { + // XEP-0045 §7.2.6: A member MUST be allowed to enter a members-only room. + setField(room, "membersOnly", true); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMemberOnly", + USER_A, Affiliation.member + ); + } + + @Test + public void testJoinPrecondition_MembersOnlyRoom_Admin_CanJoin() throws Exception { + // XEP-0045 §7.2.6: Admins MUST be allowed to enter a members-only room. + setField(room, "membersOnly", true); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMemberOnly", + USER_A, Affiliation.admin + ); + } + + @Test + public void testJoinPrecondition_MembersOnlyRoom_Owner_CanJoin() throws Exception { + // XEP-0045 §7.2.6: Owners MUST be allowed to enter a members-only room. + setField(room, "membersOnly", true); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMemberOnly", + USER_A, Affiliation.owner + ); + } + + @Test + public void testJoinPrecondition_OpenRoom_NonMember_CanJoin() throws Exception { + // An open room allows unaffiliated users. + setField(room, "membersOnly", false); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMemberOnly", + USER_A, Affiliation.none + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.9: Max Users + // "If the room has reached its maximum number of occupants, the service + // SHOULD deny access to the room and inform the user of the restriction; + // this is done by returning a presence stanza of type 'error' specifying + // a error condition" + // "if the room has reached its maximum number of occupants and a room admin + // or owner attempts to join, the room MUST allow the admin or owner to join" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_MaxUsersReached_ShouldBeDenied() throws Exception { + // XEP-0045 §7.2.9: When max occupants is reached, new users SHOULD be denied. + setField(room, "maxUsers", 1); + room.occupants.add(createDummyOccupant("existing-user", new JID("existing@example.org"))); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionMaxOccupants", + ServiceUnavailableException.class, + USER_A + ); + } + + @Test + public void testJoinPrecondition_MaxUsersReached_AdminCanJoin() throws Exception { + // XEP-0045 §7.2.9: Admins MUST be allowed to join even when the room is full. + setField(room, "maxUsers", 1); + room.occupants.add(createDummyOccupant("existing-user", new JID("existing@example.org"))); + + final ConcurrentGroupList admins = new ConcurrentGroupList<>(); + admins.add(USER_A.asBareJID()); + setField(room, "admins", admins); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMaxOccupants", + USER_A + ); + } + + @Test + public void testJoinPrecondition_MaxUsersReached_OwnerCanJoin() throws Exception { + // XEP-0045 §7.2.9: Owners MUST be allowed to join even when the room is full. + setField(room, "maxUsers", 1); + room.occupants.add(createDummyOccupant("existing-user", new JID("existing@example.org"))); + + final ConcurrentGroupList owners = new ConcurrentGroupList<>(); + owners.add(USER_A.asBareJID()); + setField(room, "owners", owners); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMaxOccupants", + USER_A + ); + } + + @Test + public void testJoinPrecondition_UnlimitedUsers_CanJoin() throws Exception { + // maxUsers == 0 means no limit. + setField(room, "maxUsers", 0); + room.occupants.add(createDummyOccupant("user1", new JID("u1@example.org"))); + room.occupants.add(createDummyOccupant("user2", new JID("u2@example.org"))); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMaxOccupants", + USER_A + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.10: Locked Room + // "If a user attempts to enter a room while it is 'locked' (i.e., before + // the room creator provides an initial configuration), the service MUST + // refuse entry and return an error to the user" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_LockedRoom_NonOwner_MustBeRejected() throws Exception { + // XEP-0045 §7.2.10: Non-owners MUST NOT enter a locked room. + setField(room, "lockedTime", System.currentTimeMillis()); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionLocked", + RoomLockedException.class, + USER_A + ); + } + + @Test + public void testJoinPrecondition_LockedRoom_Owner_CanJoin() throws Exception { + // XEP-0045 §7.2.10: The room owner is allowed to enter a locked room. + setField(room, "lockedTime", System.currentTimeMillis()); + final ConcurrentGroupList owners = new ConcurrentGroupList<>(); + owners.add(USER_A.asBareJID()); + setField(room, "owners", owners); + + assertNoPreconditionException( + "checkJoinRoomPreconditionLocked", + USER_A + ); + } + + @Test + public void testJoinPrecondition_UnlockedRoom_CanJoin() throws Exception { + // When the room is not locked, any user passes this check. + setField(room, "lockedTime", 0L); + + assertNoPreconditionException( + "checkJoinRoomPreconditionLocked", + USER_A + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.8: Nickname Conflict + // "If the room already contains another user with the nickname desired by + // the user seeking to enter the room (or if the nickname is reserved by + // another user), the service MUST deny access to the room and inform the + // user of the conflict; this is done by returning a presence stanza of + // type 'error' specifying a error condition" + // "if the bare JID of the present occupant matches the bare JID of the user + // seeking to enter the room, then the service SHOULD allow entry" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_NicknameInUse_DifferentUser_MustBeRejected() throws Exception { + // XEP-0045 §7.2.8: A different user already has this nickname → conflict. + room.occupants.add(createDummyOccupant("witch", new JID("other@example.org"))); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionNicknameInUse", + UserAlreadyExistsException.class, + USER_A, "witch" + ); + } + + @Test + public void testJoinPrecondition_NicknameInUse_SameBareJID_CanJoin() throws Exception { + // XEP-0045 §7.2.8: Same bare JID, different resource → multi-session join, allowed. + room.occupants.add(createDummyOccupant("witch", new JID("usera@example.org/res2"))); + + assertNoPreconditionException( + "checkJoinRoomPreconditionNicknameInUse", + USER_A, "witch" + ); + } + + @Test + public void testJoinPrecondition_NicknameNotInUse_CanJoin() throws Exception { + // No occupant has this nickname. + room.occupants.add(createDummyOccupant("otherwitch", new JID("other@example.org"))); + + assertNoPreconditionException( + "checkJoinRoomPreconditionNicknameInUse", + USER_A, "witch" + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.5: Password-Protected Rooms + // "If the room requires a password and the user did not supply one (or the + // password provided is incorrect), the service MUST deny access to the + // room and inform the user that they are unauthorized; this is done by + // returning a presence stanza of type 'error' specifying a + // error" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_WrongPassword_MustBeRejected() throws Exception { + // XEP-0045 §7.2.5: Wrong password MUST be rejected. + setField(room, "password", "secret"); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionPasswordProtection", + org.jivesoftware.openfire.auth.UnauthorizedException.class, + USER_A, "wrongpassword" + ); + } + + @Test + public void testJoinPrecondition_NoPasswordProvided_MustBeRejected() throws Exception { + // XEP-0045 §7.2.5: Missing password MUST be rejected. + setField(room, "password", "secret"); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionPasswordProtection", + org.jivesoftware.openfire.auth.UnauthorizedException.class, + USER_A, (Object) null + ); + } + + @Test + public void testJoinPrecondition_CorrectPassword_CanJoin() throws Exception { + // Correct password passes the check. + setField(room, "password", "secret"); + + assertNoPreconditionException( + "checkJoinRoomPreconditionPasswordProtection", + USER_A, "secret" + ); + } + + @Test + public void testJoinPrecondition_NoPasswordRequired_CanJoin() throws Exception { + // Rooms without a password always pass the password check. + setField(room, "password", null); + + assertNoPreconditionException( + "checkJoinRoomPreconditionPasswordProtection", + USER_A, (Object) null + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.8 (nickname reserved by member) + // "if the nickname is reserved by another user on the member list, the + // service MUST deny access ... specifying a error" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_NicknameReservedByOtherMember_MustBeRejected() throws Exception { + // XEP-0045 §7.2.8: A nickname reserved by another member causes a conflict. + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_B.asBareJID(), "witch"); + setField(room, "members", members); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionNicknameReserved", + ConflictException.class, + USER_A, "witch" + ); + } + + @Test + public void testJoinPrecondition_NicknameReservedBySameUser_CanJoin() throws Exception { + // The user whose nickname it is MUST be allowed to use it. + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_A.asBareJID(), "witch"); + setField(room, "members", members); + + assertNoPreconditionException( + "checkJoinRoomPreconditionNicknameReserved", + USER_A, "witch" + ); + } + + @Test + public void testJoinPrecondition_NicknameNotReserved_CanJoin() throws Exception { + // When no member has reserved this nickname, anyone can use it. + assertNoPreconditionException( + "checkJoinRoomPreconditionNicknameReserved", + USER_A, "witch" + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045: Login Restricted To Reserved Nickname + // "not-acceptable" when the user attempts to join with a nickname other + // than their reserved one. + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_RestrictedNickname_WrongNick_MustBeRejected() throws Exception { + // When loginRestrictedToNickname is enabled, the user MUST use their reserved nick. + setField(room, "loginRestrictedToNickname", true); + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_A.asBareJID(), "thirdwitch"); + setField(room, "members", members); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionRestrictedToNickname", + NotAcceptableException.class, + USER_A, "otherwitch" + ); + } + + @Test + public void testJoinPrecondition_RestrictedNickname_CorrectNick_CanJoin() throws Exception { + // When loginRestrictedToNickname is enabled, the reserved nickname is accepted. + setField(room, "loginRestrictedToNickname", true); + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_A.asBareJID(), "thirdwitch"); + setField(room, "members", members); + + assertNoPreconditionException( + "checkJoinRoomPreconditionRestrictedToNickname", + USER_A, "thirdwitch" + ); + } + + @Test + public void testJoinPrecondition_RestrictedNicknameDisabled_AnyNickAccepted() throws Exception { + // When loginRestrictedToNickname is disabled, any nickname is accepted. + setField(room, "loginRestrictedToNickname", false); + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_A.asBareJID(), "thirdwitch"); + setField(room, "members", members); + + assertNoPreconditionException( + "checkJoinRoomPreconditionRestrictedToNickname", + USER_A, "completelydifferentnick" + ); + } + + // ========================================================================= + // Self-presence status codes (XEP-0045 §7.2.2, §7.2.3, §7.2.12) + // ========================================================================= + + /** + * XEP-0045 §7.2.2: "the 'self-presence' sent by the room to the new user + * MUST include a status code of 110 so that the user knows this presence + * refers to itself as an occupant." + */ + @Test + public void testSelfPresence_AlwaysContainsStatusCode110() throws Exception { + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ false); + + assertTrue(hasStatusCode(selfPresence, 110), + "Self-presence MUST always carry status code 110 (XEP-0045 §7.2.2)"); + } + + @Test + public void testSelfPresence_OnJoin_AlwaysContainsStatusCode110() throws Exception { + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertTrue(hasStatusCode(selfPresence, 110), + "Self-presence MUST always carry status code 110 on join (XEP-0045 §7.2.2)"); + } + + /** + * XEP-0045 §7.2.3: "If the user is entering a room that is non-anonymous + * (i.e., which informs all occupants of each occupant's full JID), the + * service MUST warn the user by including a status code of '100' in the + * initial presence that the room sends to the new occupant." + */ + @Test + public void testSelfPresence_NonAnonymousRoom_ContainsStatusCode100() throws Exception { + setField(room, "canAnyoneDiscoverJID", true); // non-anonymous + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertTrue(hasStatusCode(selfPresence, 100), + "Non-anonymous room join presence MUST carry status code 100 (XEP-0045 §7.2.3)"); + } + + @Test + public void testSelfPresence_SemiAnonymousRoom_DoesNotContainStatusCode100() throws Exception { + setField(room, "canAnyoneDiscoverJID", false); // semi-anonymous + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertFalse(hasStatusCode(selfPresence, 100), + "Semi-anonymous room join presence MUST NOT carry status code 100"); + } + + @Test + public void testSelfPresence_StatusCode100_NotSentOnPresenceUpdate() throws Exception { + // Status code 100 is only sent in the context of joining, not for regular presence updates. + setField(room, "canAnyoneDiscoverJID", true); // non-anonymous + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ false); + + assertFalse(hasStatusCode(selfPresence, 100), + "Status code 100 MUST NOT be sent for non-join presence updates"); + } + + /** + * XEP-0045 §7.2.12: "If the user is entering a room in which the + * discussions are logged to a public archive, the service SHOULD allow + * the user to enter the room but MUST also warn the user that the + * discussions are logged. This is done by including a status code of + * '170' in the initial presence that the room sends to the new occupant." + */ + @Test + public void testSelfPresence_LoggedRoom_ContainsStatusCode170() throws Exception { + setField(room, "logEnabled", true); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertTrue(hasStatusCode(selfPresence, 170), + "Logged room join presence MUST carry status code 170 (XEP-0045 §7.2.12)"); + } + + @Test + public void testSelfPresence_UnloggedRoom_DoesNotContainStatusCode170() throws Exception { + setField(room, "logEnabled", false); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertFalse(hasStatusCode(selfPresence, 170), + "Non-logged room join presence MUST NOT carry status code 170"); + } + + @Test + public void testSelfPresence_StatusCode170_NotSentOnPresenceUpdate() throws Exception { + // Status code 170 is only sent in the context of joining. + setField(room, "logEnabled", true); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ false); + + assertFalse(hasStatusCode(selfPresence, 170), + "Status code 170 MUST NOT be sent for non-join presence updates"); + } + + /** + * XEP-0045: New room created — status code 201 informs the user that the + * room was just created and is locked pending configuration. + */ + @Test + public void testSelfPresence_NewRoom_ContainsStatusCode201() throws Exception { + // A 'new' room has lockedTime == creationDate (both non-zero). + final long now = System.currentTimeMillis(); + setField(room, "lockedTime", now); + setField(room, "creationDate", new java.util.Date(now)); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertTrue(hasStatusCode(selfPresence, 201), + "Joining a newly created room MUST result in status code 201"); + } + + @Test + public void testSelfPresence_ExistingRoom_DoesNotContainStatusCode201() throws Exception { + // An existing (already configured) room: lockedTime == 0. + setField(room, "lockedTime", 0L); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertFalse(hasStatusCode(selfPresence, 201), + "Joining an existing room MUST NOT result in status code 201"); + } + + @Test + public void testSelfPresence_StatusCode201_NotSentOnPresenceUpdate() throws Exception { + final long now = System.currentTimeMillis(); + setField(room, "lockedTime", now); + setField(room, "creationDate", new java.util.Date(now)); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ false); + + assertFalse(hasStatusCode(selfPresence, 201), + "Status code 201 MUST NOT be sent for non-join presence updates"); + } + + // ========================================================================= + // Self-presence detection in broadcast() (XEP-0045 §7.2.2) + // + // Root cause of https://discourse.igniterealtime.org/t/82698: + // The old detection used presence.getTo() which is unreliable — it can be + // null or a bare room JID when a client omits the resource part, causing + // the occupant to receive a regular (non-self) presence WITHOUT status 110. + // The fix detects self by matching occupant.getOccupantJID() against + // presence.getFrom(), which is always reliably set by MUCOccupant.setPresence(). + // ========================================================================= + + /** + * XEP-0045 §7.2.2 / regression for discourse.igniterealtime.org/t/82698. + * + * The self-presence is identified by matching {@code occupant.getOccupantJID()} + * against {@code presence.getFrom()}, which is always reliably set to the + * occupant JID in {@code MUCOccupant.setPresence()}. This test verifies that + * the detection succeeds regardless of the value of {@code presence.getTo()}. + */ + @Test + public void testBroadcastSelfPresenceDetection_MatchesOnFrom_NotOnTo() throws Exception { + // The key invariant: presence.getFrom() == the sender's occupant JID. + // The presence.getTo() is unreliable (can be null, bare room JID, etc.). + + final JID occupantJID = new JID("testroom", "conference.example.org", "witch"); + + final MUCOccupant occupant = createDummyOccupant("witch", USER_A); + setField(occupant, "occupantJID", occupantJID); + + // Build a presence that has the correct 'from' but a NULL 'to'. + // In the old code, presence.getTo() == null would break detection. + final Presence presence = buildMucUserPresence(); + presence.setFrom(occupantJID); + presence.setTo((JID) null); // simulate missing 'to' (e.g. presence update) + + // New detection: matches on 'from', not on 'to'. + assertTrue(occupant.getOccupantJID().equals(presence.getFrom()), + "Self-presence detection MUST succeed when from matches, even when to is null"); + } + + @Test + public void testBroadcastSelfPresenceDetection_MatchesOnFrom_WithBareRoomJIDAsTo() throws Exception { + // If the client sent a bare room JID (no resource) as 'to', the old check + // occupant.getPresence().getFrom().equals(presence.getTo()) would FAIL, because + // the occupant's presence.getFrom() is 'room@service/nick' but presence.getTo() + // would be 'room@service' (no resource) if the overwrite in broadcastPresence() did + // not fire or was bypassed. + + final JID occupantJID = new JID("testroom", "conference.example.org", "witch"); + final JID bareRoomJID = new JID("testroom", "conference.example.org", null); + + final MUCOccupant occupant = createDummyOccupant("witch", USER_A); + setField(occupant, "occupantJID", occupantJID); + + final Presence presence = buildMucUserPresence(); + presence.setFrom(occupantJID); + presence.setTo(bareRoomJID); // simulate bare room JID as 'to' + + // Old check (fragile — would fail here because occupantJID has resource but presence.getTo() is bare): + assertFalse(occupantJID.equals(presence.getTo()), + "Sanity: old detection strategy FAILS when to is bare room JID"); + + // New check (robust — succeeds): + assertTrue(occupant.getOccupantJID().equals(presence.getFrom()), + "New detection strategy MUST succeed even when to is a bare room JID"); + } + + @Test + public void testBroadcastSelfPresenceDetection_OtherOccupant_NotMatchedAsSelf() throws Exception { + // Verify that another occupant (not the sender) does NOT match as self-presence. + final JID senderOccupantJID = new JID("testroom", "conference.example.org", "witch"); + final JID otherOccupantJID = new JID("testroom", "conference.example.org", "wizard"); + + final MUCOccupant otherOccupant = createDummyOccupant("wizard", USER_B); + setField(otherOccupant, "occupantJID", otherOccupantJID); + + // Presence is FROM the sender (witch), not from the other occupant (wizard). + final Presence presence = buildMucUserPresence(); + presence.setFrom(senderOccupantJID); + + // The other occupant MUST NOT be detected as "self". + assertFalse(otherOccupant.getOccupantJID().equals(presence.getFrom()), + "A different occupant MUST NOT be selected as the self-presence recipient"); + } + + /** + * End-to-end verification of {@link MUCRoom#broadcast(Presence, boolean)} self-presence + * selection when the presence 'to' field is null. + * + * This is a regression test for the issue reported at + * https://discourse.igniterealtime.org/t/82698 where Openfire was sending a + * non-self presence (without status 110) to the joining occupant. + */ + @Test + public void testBroadcast_SelfOccupantReceivesSelfPresence_EvenWhenToIsNull() throws Exception { + // Setup: a non-anon room with one occupant ("witch"). + setField(room, "canAnyoneDiscoverJID", true); + setField(room, "rolesToBroadcastPresence", new java.util.ArrayList<>(List.of(Role.moderator, Role.participant, Role.visitor))); + + final JID occupantJID = new JID("testroom", "conference.example.org", "witch"); + + // Build the occupant with a real-looking presence so broadcast() can check its JID. + final MUCOccupant occupant = createDummyOccupant("witch", USER_A); + setField(occupant, "occupantJID", occupantJID); + room.occupants.add(occupant); + + // Build the presence being broadcast (from = occupant JID, to = null). + // This simulates the case where a client sends a presence update without + // specifying the 'to' address (the format that triggered the v4.2.3 bug). + final Presence presence = buildMucUserPresence(); + presence.setFrom(occupantJID); + presence.setTo((JID) null); + + // Capture what would be sent: derive the self-presence the way broadcast() does. + // broadcast() creates selfPresence = createSelfPresenceCopy(presence, isJoinPresence). + final Presence selfPresence = invokeSelfPresenceCopy(presence, true); + + // Verify: the presence returned for the self-occupant should contain status 110. + // The old code would have picked nonAnonPresence (no status 110) because + // occupant.getPresence().getFrom().equals(presence.getTo()) → ANY_JID.equals(null) = false. + // The new code picks selfPresence because occupant.getOccupantJID().equals(presence.getFrom()) = true. + assertTrue(hasStatusCode(selfPresence, 110), + "XEP-0045 §7.2.2: The presence sent to the self-occupant MUST contain status code 110"); + + // And verify the new detection condition used by broadcast(): + assertTrue(occupant.getOccupantJID().equals(presence.getFrom()), + "New detection: occupantJID MUST match presence.getFrom() for the self-occupant"); + // Confirm old condition would have FAILED (regression guard): + // Old code: occupant.getPresence().getFrom().equals(presence.getTo()) + // When presence.getTo() is null, this is equivalent to: occupantJID.equals(null) = false. + assertFalse(occupantJID.equals(presence.getTo()), + "Old detection: would have FAILED when presence.getTo() is null (the regression)"); + } + + /** + * Builds a minimal presence stanza with the {@code } + * child element required by {@code createSelfPresenceCopy}. + */ + private Presence buildMucUserPresence() { + final Presence presence = new Presence(); + presence.setFrom(new JID("testroom", "conference.example.org", "testuser")); + presence.setTo(new JID("user@example.org/res")); + final Element x = presence.addChildElement("x", "http://jabber.org/protocol/muc#user"); + x.addElement("item") + .addAttribute("affiliation", "none") + .addAttribute("role", "participant"); + return presence; + } + + /** Invokes the private {@code createSelfPresenceCopy} method via reflection. */ + private Presence invokeSelfPresenceCopy(Presence presence, boolean isJoinPresence) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + "createSelfPresenceCopy", Presence.class, boolean.class); + method.setAccessible(true); + return (Presence) method.invoke(room, presence, isJoinPresence); + } + + /** Returns {@code true} if the presence contains a status element with the given code. */ + private boolean hasStatusCode(Presence presence, int code) { + final Element x = presence.getChildElement("x", "http://jabber.org/protocol/muc#user"); + if (x == null) { + return false; + } + return x.elements("status").stream() + .anyMatch(e -> String.valueOf(code).equals(e.attributeValue("code"))); + } + + /** + * Asserts that calling the named single-arg (JID) precondition method throws the expected + * exception type. + */ + private void assertThrowsPrecondition( + String methodName, Class expectedException, JID userAddress) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod(methodName, JID.class); + method.setAccessible(true); + final InvocationTargetException ex = + assertThrows(InvocationTargetException.class, () -> method.invoke(room, userAddress), + "Expected " + expectedException.getSimpleName() + " from " + methodName); + assertInstanceOf(expectedException, ex.getCause(), + "Wrong exception type from " + methodName); + } + + /** + * Asserts that calling the named two-arg (JID, Affiliation) precondition method throws the + * expected exception type. + */ + private void assertThrowsPrecondition( + String methodName, Class expectedException, JID userAddress, Affiliation affiliation) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + methodName, JID.class, Affiliation.class); + method.setAccessible(true); + final InvocationTargetException ex = + assertThrows(InvocationTargetException.class, + () -> method.invoke(room, userAddress, affiliation), + "Expected " + expectedException.getSimpleName() + " from " + methodName); + assertInstanceOf(expectedException, ex.getCause(), + "Wrong exception type from " + methodName); + } + + /** + * Asserts that calling the named two-arg (JID, String) precondition method throws the expected + * exception type. + */ + private void assertThrowsPrecondition( + String methodName, Class expectedException, JID userAddress, Object secondArg) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + methodName, JID.class, String.class); + method.setAccessible(true); + final InvocationTargetException ex = + assertThrows(InvocationTargetException.class, + () -> method.invoke(room, userAddress, secondArg), + "Expected " + expectedException.getSimpleName() + " from " + methodName); + assertInstanceOf(expectedException, ex.getCause(), + "Wrong exception type from " + methodName); + } + + /** Asserts that the single-arg (JID) precondition check passes without throwing. */ + private void assertNoPreconditionException(String methodName, JID userAddress) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod(methodName, JID.class); + method.setAccessible(true); + assertDoesNotThrow(() -> { + try { + method.invoke(room, userAddress); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + }, "Unexpected exception from " + methodName); + } + + /** Asserts that the two-arg (JID, Affiliation) precondition check passes without throwing. */ + private void assertNoPreconditionException( + String methodName, JID userAddress, Affiliation affiliation) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + methodName, JID.class, Affiliation.class); + method.setAccessible(true); + assertDoesNotThrow(() -> { + try { + method.invoke(room, userAddress, affiliation); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + }, "Unexpected exception from " + methodName); + } + + /** Asserts that the two-arg (JID, String) precondition check passes without throwing. */ + private void assertNoPreconditionException( + String methodName, JID userAddress, Object secondArg) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + methodName, JID.class, String.class); + method.setAccessible(true); + assertDoesNotThrow(() -> { + try { + method.invoke(room, userAddress, secondArg); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + }, "Unexpected exception from " + methodName); + } + + /** + * Creates a minimal {@link MUCOccupant} with the given nickname and user JID, using + * reflection to set the private fields (without triggering any room-side effects). + */ + private MUCOccupant createDummyOccupant(String nickname, JID userJid) throws Exception { + final MUCOccupant occupant = new MUCOccupant(); + setField(occupant, "nick", nickname); + setField(occupant, "userJid", userJid.asBareJID()); + setField(occupant, "occupantJID", new JID(ROOM_JID.getNode(), ROOM_JID.getDomain(), nickname)); + setField(occupant, "role", Role.participant); + setField(occupant, "affiliation", Affiliation.none); + return occupant; + } + + /** Sets a declared private field on the given object using reflection. */ + static void setField(E object, String fieldName, Object value) + throws NoSuchFieldException, IllegalAccessException + { + // Walk the class hierarchy to find the declared field. + Class clazz = object.getClass(); + Field field = null; + while (clazz != null) { + try { + field = clazz.getDeclaredField(fieldName); + break; + } catch (NoSuchFieldException e) { + clazz = clazz.getSuperclass(); + } + } + if (field == null) { + throw new NoSuchFieldException("Field '" + fieldName + "' not found in class hierarchy of " + object.getClass().getName()); + } + final boolean wasAccessible = field.canAccess(object); + field.setAccessible(true); + try { + field.set(object, value); + } finally { + field.setAccessible(wasAccessible); + } + } +} diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java index 91c2776658..7ccc83c1a7 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2020-2023 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2020-2026 Ignite Realtime Foundation. All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import org.junit.jupiter.api.Test; import org.xmpp.packet.JID; +import java.util.Date; + import static org.junit.jupiter.api.Assertions.*; /** @@ -29,6 +31,17 @@ */ public class LeafNodeTest { + /** Creates a minimal LeafNode for use in tests. */ + private static LeafNode createTestLeafNode() { + final DefaultNodeConfiguration config = new DefaultNodeConfiguration(true); + return new LeafNode( + new PubSubService.UniqueIdentifier("test-service-id"), + null, + "test-node-id", + new JID("unit-test@example.org"), + config); + } + @Test public void testSerialization() throws Exception { @@ -59,4 +72,93 @@ public void testSerialization() throws Exception assertTrue( result instanceof LeafNode ); assertEquals( input, result ); } + + // ========================================================================= + // setLastPublishedItem / getPublishedItem — XEP-0060 §7.1.2 compliance + // + // When a publisher publishes an item with the same ItemID as a previously + // published item, the server MUST overwrite the old item with the new one. + // The in-memory 'lastPublished' cache must reflect the new item immediately, + // even if both items share the same creation-date timestamp. + // ========================================================================= + + /** + * Regression test for XEP-0060 §7.1.2: publishing a second item with the same + * ItemID must update the in-memory {@code lastPublished} reference, even when + * the two items share the same creation-date (published within the same + * millisecond, as is common in fast sequential integration tests). + * + *

Before the fix, {@code setLastPublishedItem} only updated {@code lastPublished} + * when {@code item.getCreationDate().after(lastPublished.getCreationDate())} returned + * {@code true}. If both items had the same timestamp the old item remained cached, + * causing {@code getPublishedItem()} to serve the stale first-published payload. + */ + @Test + public void testSetLastPublishedItem_SameIdSameTimestamp_UpdatesCache() { + final LeafNode node = createTestLeafNode(); + final JID publisher = new JID("user@example.org"); + final Date sharedTimestamp = new Date(1_000_000L); + + // First publish: item1 with itemID="shared-id", payload conceptually "payload-1". + final PublishedItem item1 = new PublishedItem(node, publisher, "shared-id", sharedTimestamp); + node.setLastPublishedItem(item1); + + // Sanity: lastPublished should now be item1. + assertSame(item1, node.getLastPublishedItem(), + "After first publish, lastPublished must be item1"); + + // Second publish: item2 with SAME itemID and SAME timestamp (typical in fast tests). + final PublishedItem item2 = new PublishedItem(node, publisher, "shared-id", sharedTimestamp); + node.setLastPublishedItem(item2); + + // Assert: lastPublished must be updated to item2 (the overwriting item). + // Before the fix this assertion would fail because the creation-date guard + // prevented the update when both items shared the same timestamp. + assertSame(item2, node.getLastPublishedItem(), + "XEP-0060 §7.1.2: re-publishing with the same ItemID must update lastPublished " + + "even when both items have the same creation-date"); + } + + /** + * Complementary test: publishing an item with a different ItemID but the same + * timestamp must NOT overwrite {@code lastPublished}. Only items with newer + * timestamps (or the same ID) should replace it. + */ + @Test + public void testSetLastPublishedItem_DifferentIdSameTimestamp_DoesNotOverwrite() { + final LeafNode node = createTestLeafNode(); + final JID publisher = new JID("user@example.org"); + final Date sharedTimestamp = new Date(1_000_000L); + + final PublishedItem item1 = new PublishedItem(node, publisher, "id-alpha", sharedTimestamp); + node.setLastPublishedItem(item1); + + // A different item with a different ID but the same timestamp. + final PublishedItem item2 = new PublishedItem(node, publisher, "id-beta", sharedTimestamp); + node.setLastPublishedItem(item2); + + // item1 was the last published (by time) and has a different ID, + // so it should remain as lastPublished (item2 is not newer). + assertSame(item1, node.getLastPublishedItem(), + "A different item published at the same timestamp must not displace lastPublished"); + } + + /** + * Sanity test: publishing a newer item (strictly later timestamp) with a + * different ID must update {@code lastPublished} as before. + */ + @Test + public void testSetLastPublishedItem_DifferentIdNewerTimestamp_UpdatesCache() { + final LeafNode node = createTestLeafNode(); + final JID publisher = new JID("user@example.org"); + + final PublishedItem item1 = new PublishedItem(node, publisher, "id-alpha", new Date(1_000L)); + node.setLastPublishedItem(item1); + + final PublishedItem item2 = new PublishedItem(node, publisher, "id-beta", new Date(2_000L)); + node.setLastPublishedItem(item2); + + assertSame(item2, node.getLastPublishedItem(), + "A newer item (later timestamp, different ID) must update lastPublished"); + } }