diff --git a/eventauth.go b/eventauth.go index c02ee6f5..f2fdbc4e 100644 --- a/eventauth.go +++ b/eventauth.go @@ -620,38 +620,21 @@ func checkEventLevels(senderLevel int64, oldPowerLevels, newPowerLevels PowerLev // checkUserLevels checks that the changes in user levels are allowed. func checkUserLevels(senderLevel int64, senderID string, oldPowerLevels, newPowerLevels PowerLevelContent) error { type levelPair struct { - old int64 - new int64 - userID string + old int64 + new int64 } // Build a list of user levels to check. - // This differs slightly in behaviour from the code in synapse because it will use the - // default value if a level is not present in one of the old or new events. - - // First add the user default level. - userLevelChecks := []levelPair{ - {oldPowerLevels.UsersDefault, newPowerLevels.UsersDefault, ""}, - } - - // Then add checks for each user key in the new levels. + userLevelChecks := map[string]levelPair{} for userID := range newPowerLevels.Users { - userLevelChecks = append(userLevelChecks, levelPair{ - oldPowerLevels.UserLevel(userID), newPowerLevels.UserLevel(userID), userID, - }) - } - - // Then add checks for each user key in the old levels. - // Some of these will be duplicates of the ones added using the keys from - // the new levels. But it doesn't hurt to run the checks twice for the same level. - for userID := range oldPowerLevels.Users { - userLevelChecks = append(userLevelChecks, levelPair{ - oldPowerLevels.UserLevel(userID), newPowerLevels.UserLevel(userID), userID, - }) + userLevelChecks[userID] = levelPair{ + old: oldPowerLevels.Users[userID], + new: newPowerLevels.Users[userID], + } } // Check each of the levels in the list. - for _, level := range userLevelChecks { + for userID, level := range userLevelChecks { // Check if the level is being changed. if level.old == level.new { // Levels are always allowed to stay the same. @@ -669,14 +652,14 @@ func checkUserLevels(senderLevel int64, senderID string, oldPowerLevels, newPowe // Check if the user is trying to set any of the levels to above their own. if senderLevel < level.new { return errorf( - "sender with level %d is not allowed change user level from %d to %d"+ + "sender %q with level %d is not allowed change user %q level from %d to %d"+ " because the new level is above the level of the sender", - senderLevel, level.old, level.new, + senderID, senderLevel, userID, level.old, level.new, ) } // Check if the user is changing their own user level. - if level.userID == senderID { + if userID == senderID { // Users are always allowed to reduce their own user level. // We know that the user is reducing their level because of the previous checks. continue @@ -685,9 +668,9 @@ func checkUserLevels(senderLevel int64, senderID string, oldPowerLevels, newPowe // Check if the user is changing the level that was above or the same as their own. if senderLevel <= level.old { return errorf( - "sender with level %d is not allowed to change user level from %d to %d"+ + "sender %q with level %d is not allowed to change user %q level from %d to %d"+ " because the old level is equal to or above the level of the sender", - senderLevel, level.old, level.new, + senderID, senderLevel, userID, level.old, level.new, ) } } @@ -1261,6 +1244,7 @@ func (m *membershipAllower) membershipAllowedOther() error { // nolint: gocyclo } return nil } + return m.membershipFailed( "sender has insufficient power to invite (sender level %d, target level %d, invite level %d)", senderLevel, targetLevel, m.powerLevels.Invite, diff --git a/eventauth_test.go b/eventauth_test.go index 7647c072..baf8d772 100644 --- a/eventauth_test.go +++ b/eventauth_test.go @@ -1040,6 +1040,95 @@ func TestAuthEvents(t *testing.T) { } } +var powerLevelTestRoom = &testAuthEvents{ + CreateJSON: json.RawMessage(`{ + "type": "m.room.create", + "state_key": "", + "sender": "@u1:a", + "room_id": "!r1:a", + "event_id": "$e1:a", + "content": { + "room_version": "1" + } + }`), + PowerLevelsJSON: json.RawMessage(`{ + "type": "m.room.power_levels", + "state_key": "", + "sender": "@u1:a", + "room_id": "!r1:a", + "event_id": "$e3:a", + "content": { + "users_default": 100, + "users": { + "@u1:a": 100 + }, + "redact": 100 + } + }`), + MemberJSON: map[string]json.RawMessage{ + "@u1:a": json.RawMessage(`{ + "type": "m.room.member", + "state_key": "@u1:a", + "sender": "@u1:a", + "room_id": "!r1:a", + "event_id": "$e2:a", + "content": { + "membership": "join" + } + }`), + }, +} + +func TestDemoteUserDefaultPowerLevelBelowOwn(t *testing.T) { + // User should be able to demote the user default level + // below their own effective level. + powerChangeShouldSucceed, err := NewEventFromTrustedJSON(RawJSON(`{ + "type": "m.room.power_levels", + "state_key": "", + "sender": "@u1:a", + "room_id": "!r1:a", + "event_id": "$e5:a", + "content": { + "users_default": 50, + "users": { + "@u1:a": 100 + }, + "redact": 100 + } + }`), false, RoomVersionV1) + if err != nil { + t.Fatal(err) + } + if err = Allowed(powerChangeShouldSucceed, powerLevelTestRoom); err != nil { + t.Error("TestDemoteUserDefaultPowerLevel should have succeeded but it didn't:", err) + } +} + +func TestPromoteUserDefaultLevelAboveOwn(t *testing.T) { + // User shouldn't be able to promote the user default + // level above their own effective level. + powerChangeShouldFail, err := NewEventFromTrustedJSON(RawJSON(`{ + "type": "m.room.power_levels", + "state_key": "", + "sender": "@u2:a", + "room_id": "!r1:a", + "event_id": "$e5:a", + "content": { + "users_default": 500, + "users": { + "@u1:a": 100 + }, + "redact": 100 + } + }`), false, RoomVersionV1) + if err != nil { + t.Fatal(err) + } + if err = Allowed(powerChangeShouldFail, powerLevelTestRoom); err == nil { + t.Error("TestPromoteUserDefaultLevelAboveOwn event should have failed but it didn't") + } +} + func newMemberContent( membership string, thirdPartyInvite *MemberThirdPartyInvite, ) MemberContent {