Skip to content

Commit

Permalink
OF-241 + OF-747 Test preparation for MUC privileges.
Browse files Browse the repository at this point in the history
  • Loading branch information
@sco0ter
sco0ter committed May 19, 2014
1 parent 06b9455 commit 7e6a1bf
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 14 deletions.
63 changes: 49 additions & 14 deletions src/java/org/jivesoftware/openfire/muc/spi/LocalMUCRoom.java
View file
Expand Up @@ -94,7 +94,7 @@
* rooms will be loaded by each cluster node when starting up. Not persistent rooms will be copied
* from the senior cluster member. All room occupants will be copied from the senior cluster member
* too.
*
*
* @author Gaston Dombiak
*/
public class LocalMUCRoom implements MUCRoom {
Expand Down Expand Up @@ -676,7 +676,7 @@ else if (outcasts.contains(bareJID)) {

/**
* Can a user join this room
*
*
* @param user the user attempting to join this room
* @return boolean
*/
Expand All @@ -688,13 +688,13 @@ private boolean canJoinRoom(LocalMUCUser user){

/**
* Does this room have an occupancy limit?
*
*
* @return boolean
*/
private boolean hasOccupancyLimit(){
return getMaxUsers() != 0;
}

/**
* Sends presence of existing occupants to new occupant.
*
Expand Down Expand Up @@ -871,7 +871,7 @@ private void removeOccupantRole(MUCRole leaveRole, boolean originator) {
leaveRole.destroy();
// Update the tables of occupants based on the bare and full JID
JID bareJID = userAddress.asBareJID();

String nickname = leaveRole.getNickname();
List<MUCRole> occupants = occupantsByNickname.get(nickname.toLowerCase());
if (occupants != null) {
Expand Down Expand Up @@ -1036,7 +1036,7 @@ else if (packet instanceof IQ) {

/**
* Checks the role of the sender and returns true if the given presence should be broadcasted
*
*
* @param presence The presence to check
* @return true if the presence should be broadcast to the rest of the room
*/
Expand Down Expand Up @@ -1280,7 +1280,7 @@ public long getChatLength() {
* @throws NotAllowedException If trying to change the moderator role to an owner or an admin or
* if trying to ban an owner or an administrator.
*/
private List<Presence> changeOccupantAffiliation(JID jid, MUCRole.Affiliation newAffiliation, MUCRole.Role newRole)
private List<Presence> changeOccupantAffiliation(MUCRole senderRole, JID jid, MUCRole.Affiliation newAffiliation, MUCRole.Role newRole)
throws NotAllowedException {
List<Presence> presences = new ArrayList<Presence>();
// Get all the roles (i.e. occupants) of this user based on his/her bare JID
Expand All @@ -1291,6 +1291,10 @@ private List<Presence> changeOccupantAffiliation(JID jid, MUCRole.Affiliation ne
}
// Collect all the updated presences of these roles
for (MUCRole role : roles) {
// TODO
// if (!isPrivilegedToChangeAffiliationAndRole(senderRole.getAffiliation(), senderRole.getRole(), role.getAffiliation(), role.getRole(), newAffiliation, newRole)) {
// throw new NotAllowedException();
// }
// Update the presence with the new affiliation and role
if (role.isLocal()) {
role.setAffiliation(newAffiliation);
Expand Down Expand Up @@ -1331,6 +1335,10 @@ private List<Presence> changeOccupantAffiliation(JID jid, MUCRole.Affiliation ne
private Presence changeOccupantRole(JID jid, MUCRole.Role newRole) throws NotAllowedException {
// Try looking the role in the bare JID list
MUCRole role = occupantsByFullJID.get(jid);
// TODO
// if (!isPrivilegedToChangeAffiliationAndRole(senderRole.getAffiliation(), senderRole.getRole(), role.getAffiliation(), role.getRole(), newAffiliation, newRole)) {
// throw new NotAllowedException();
// }
if (role != null) {
if (role.isLocal()) {
// Update the presence with the new role
Expand Down Expand Up @@ -1358,6 +1366,33 @@ private Presence changeOccupantRole(JID jid, MUCRole.Role newRole) throws NotAll
return null;
}

static boolean isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation actorAffiliation, MUCRole.Role actorRole, MUCRole.Affiliation occupantAffiliation, MUCRole.Role occupantRole, MUCRole.Affiliation newAffiliation, MUCRole.Role newRole) {
switch (actorAffiliation) {
case owner:
// An owner has all privileges
return true;
case admin:
// If affiliation has not changed
if (occupantAffiliation == newAffiliation) {
// Only check, if the admin wants to modify an owner (e.g. revoke an owner's moderator role).
return occupantAffiliation != MUCRole.Affiliation.owner;
} else {
// An admin is not allowed to modify the admin or owner list.
return occupantAffiliation != MUCRole.Affiliation.owner && newAffiliation != MUCRole.Affiliation.admin && newAffiliation != MUCRole.Affiliation.owner;
}
default:
// Every other affiliation (member, none, outcast) is not allowed to change anything, except he's a moderator and he doesn't want to change affiliations.
if (actorRole == MUCRole.Role.moderator && occupantAffiliation == newAffiliation) {
// A moderator SHOULD NOT be allowed to revoke moderation privileges from someone with a higher affiliation than themselves
// (i.e., an unaffiliated moderator SHOULD NOT be allowed to revoke moderation privileges from an admin or an owner, and an admin SHOULD NOT be allowed to revoke moderation privileges from an owner).
if (occupantRole == MUCRole.Role.moderator && newRole != MUCRole.Role.moderator) {
return occupantAffiliation != MUCRole.Affiliation.owner && occupantAffiliation != MUCRole.Affiliation.admin;
}
}
return false;
}
}

public void addFirstOwner(JID bareJID) {
owners.add( bareJID.asBareJID() );
}
Expand Down Expand Up @@ -1401,7 +1436,7 @@ else if (removeOutcast(bareJID)) {
CacheFactory.doClusterTask(new AddAffiliation(this, jid.toBareJID(), MUCRole.Affiliation.owner));
// Update the presence with the new affiliation and inform all occupants
try {
return changeOccupantAffiliation(jid, MUCRole.Affiliation.owner,
return changeOccupantAffiliation(sendRole, jid, MUCRole.Affiliation.owner,
MUCRole.Role.moderator);
}
catch (NotAllowedException e) {
Expand Down Expand Up @@ -1458,7 +1493,7 @@ else if (removeOutcast(bareJID)) {
CacheFactory.doClusterTask(new AddAffiliation(this, jid.toBareJID(), MUCRole.Affiliation.admin));
// Update the presence with the new affiliation and inform all occupants
try {
return changeOccupantAffiliation(jid, MUCRole.Affiliation.admin,
return changeOccupantAffiliation(sendRole, jid, MUCRole.Affiliation.admin,
MUCRole.Role.moderator);
}
catch (NotAllowedException e) {
Expand Down Expand Up @@ -1530,7 +1565,7 @@ else if (removeOutcast(bareJID)) {
CacheFactory.doClusterTask(new AddMember(this, jid.toBareJID(), (nickname == null ? "" : nickname)));
// Update the presence with the new affiliation and inform all occupants
try {
return changeOccupantAffiliation(jid, MUCRole.Affiliation.member,
return changeOccupantAffiliation(sendRole, jid, MUCRole.Affiliation.member,
MUCRole.Role.participant);
}
catch (NotAllowedException e) {
Expand Down Expand Up @@ -1591,7 +1626,7 @@ else if (removeMember(bareJID)) {
// Update the presence with the new affiliation and inform all occupants
// actorJID will be null if the room itself (ie. via admin console) made the request
JID actorJID = senderRole.getUserAddress();
List<Presence> updatedPresences = changeOccupantAffiliation(
List<Presence> updatedPresences = changeOccupantAffiliation(senderRole,
jid,
MUCRole.Affiliation.outcast,
MUCRole.Role.none);
Expand Down Expand Up @@ -1666,7 +1701,7 @@ else if (removeOutcast(bareJID)) {
else {
newRole = isModerated() ? MUCRole.Role.visitor : MUCRole.Role.participant;
}
updatedPresences = changeOccupantAffiliation(bareJID, MUCRole.Affiliation.none, newRole);
updatedPresences = changeOccupantAffiliation(senderRole, bareJID, MUCRole.Affiliation.none, newRole);
if (isMembersOnly() && wasMember) {
// If the room is members-only, remove the user from the room including a status
// code of 321 to indicate that the user was removed because of an affiliation
Expand Down Expand Up @@ -1753,7 +1788,7 @@ public void occupantUpdated(UpdateOccupant update) {
}
else {
Log.error(MessageFormat.format("Ignoring update of local occupant with info from a remote occupant. "
+ "Occupant nickname: {0} new role: {1} new affiliation: {2}",
+ "Occupant nickname: {0} new role: {1} new affiliation: {2}",
update.getNickname(), update.getRole(), update.getAffiliation()));
}
}
Expand Down Expand Up @@ -2070,7 +2105,7 @@ public Presence kickOccupant(JID jid, JID actorJID, String reason)

// Effectively kick the occupant from the room
kickPresence(updatedPresence, actorJID);

//Inform the other occupants that user has been kicked
broadcastPresence(updatedPresence, false);
}
Expand Down
56 changes: 56 additions & 0 deletions src/test/java/org/jivesoftware/openfire/muc/spi/MucPrivilegesTest.java
View file
@@ -0,0 +1,56 @@
package org.jivesoftware.openfire.muc.spi;

import junit.framework.Assert;
import org.jivesoftware.openfire.muc.MUCRole;
import org.junit.Test;

/**
* @author Christian Schudt
*/
public class MucPrivilegesTest {

@Test
public void ownerShouldBeAbleToDoAnything() {
Assert.assertTrue(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.owner, MUCRole.Role.moderator, MUCRole.Affiliation.owner, MUCRole.Role.moderator, MUCRole.Affiliation.none, MUCRole.Role.none));
}

@Test
public void adminShouldBeAbleToRevokeModeratorPrivilegesFromOtherAdmin() {
Assert.assertTrue(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.admin, MUCRole.Role.none, MUCRole.Affiliation.admin, MUCRole.Role.moderator, MUCRole.Affiliation.admin, MUCRole.Role.none));
}

@Test
public void adminShouldBeAbleToGrantMembership() {
Assert.assertTrue(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.admin, MUCRole.Role.none, MUCRole.Affiliation.none, MUCRole.Role.none, MUCRole.Affiliation.member, MUCRole.Role.participant));
}

@Test
public void adminModeratorShouldNotBeAbleToRevokeModeratorPrivilegesFromOwner() {
Assert.assertFalse(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.admin, MUCRole.Role.moderator, MUCRole.Affiliation.owner, MUCRole.Role.moderator, MUCRole.Affiliation.none, MUCRole.Role.none));
}

@Test
public void ownerModeratorShouldBeAbleToRevokeModeratorPrivilegesFromOwner() {
Assert.assertTrue(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.owner, MUCRole.Role.moderator, MUCRole.Affiliation.owner, MUCRole.Role.moderator, MUCRole.Affiliation.none, MUCRole.Role.none));
}

@Test
public void ownerModeratorShouldBeAbleToRevokeModeratorPrivilegesFromAdmin() {
Assert.assertTrue(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.owner, MUCRole.Role.moderator, MUCRole.Affiliation.admin, MUCRole.Role.moderator, MUCRole.Affiliation.none, MUCRole.Role.none));
}

@Test
public void memberModeratorShouldNotBeAbleToRevokeModeratorPrivilegesFromOwner() {
Assert.assertFalse(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.member, MUCRole.Role.moderator, MUCRole.Affiliation.owner, MUCRole.Role.moderator, MUCRole.Affiliation.none, MUCRole.Role.none));
}

@Test
public void memberModeratorShouldNotBeAbleToRevokeModeratorPrivilegesFromAdmin() {
Assert.assertFalse(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.member, MUCRole.Role.moderator, MUCRole.Affiliation.admin, MUCRole.Role.moderator, MUCRole.Affiliation.none, MUCRole.Role.none));
}

@Test
public void memberShouldNotBeAbleToDoAnything() {
Assert.assertFalse(LocalMUCRoom.isPrivilegedToChangeAffiliationAndRole(MUCRole.Affiliation.member, MUCRole.Role.participant, MUCRole.Affiliation.admin, MUCRole.Role.moderator, MUCRole.Affiliation.none, MUCRole.Role.none));
}
}

0 comments on commit 7e6a1bf

Please sign in to comment.