From 14ee7615d6041e851658f2f46057f81738a49d69 Mon Sep 17 00:00:00 2001
From: Till <2353100+S7evinK@users.noreply.github.com>
Date: Tue, 16 Jan 2024 13:22:02 +0100
Subject: [PATCH] Fix event auth for knocking (#431)

Knocking is not allowed in `restricted` rooms.
---
 eventauth.go      | 23 ++++++++++-------------
 eventauth_test.go | 30 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/eventauth.go b/eventauth.go
index e8ecd6c2..39a41d7d 100644
--- a/eventauth.go
+++ b/eventauth.go
@@ -1155,18 +1155,7 @@ func (m *membershipAllower) membershipAllowedSelf() error { // nolint: gocyclo
 
 	switch m.newMember.Membership {
 	case spec.Knock:
-		if m.joinRule.JoinRule != spec.Knock && m.joinRule.JoinRule != spec.KnockRestricted {
-			return m.membershipFailed(
-				"join rule %q does not allow knocking", m.joinRule.JoinRule,
-			)
-		}
-		// A user that is not in the room is allowed to knock if the join
-		// rules are "knock" and they are not already joined to, invited to
-		// or banned from the room.
-		// Spec: https://spec.matrix.org/unstable/rooms/v7/
-		// MSC3787 extends this: the behaviour above is also permitted if the
-		// join rules are "knock_restricted"
-		// Spec: https://github.com/matrix-org/matrix-spec-proposals/pull/3787
+		// Check if the given roomVersionImpl allows knocking.
 		return m.roomVersionImpl.CheckKnockingAllowed(m)
 	case spec.Join:
 		if m.joinRule.JoinRule == spec.Restricted || m.joinRule.JoinRule == spec.KnockRestricted {
@@ -1244,8 +1233,16 @@ func disallowKnocking(m *membershipAllower) error {
 	)
 }
 
+// A user that is not in the room is allowed to knock if the join
+// rules are "knock" and they are not already joined to
+// or banned from the room.
+// Spec: https://spec.matrix.org/unstable/rooms/v7/
+// MSC3787 extends this: the behaviour above is also permitted if the
+// join rules are "knock_restricted"
+// Spec: https://github.com/matrix-org/matrix-spec-proposals/pull/3787
 func checkKnocking(m *membershipAllower) error {
-	supported := m.joinRule.JoinRule == spec.Knock || m.joinRule.JoinRule == spec.Restricted || m.joinRule.JoinRule == spec.KnockRestricted
+	// If the join_rule is anything other than knock or knock_restricted, reject.
+	supported := m.joinRule.JoinRule == spec.Knock || m.joinRule.JoinRule == spec.KnockRestricted
 	if !supported {
 		return m.membershipFailed(
 			"room version %q does not support knocking on rooms with join rule %q",
diff --git a/eventauth_test.go b/eventauth_test.go
index 39a4a99e..f9040aee 100644
--- a/eventauth_test.go
+++ b/eventauth_test.go
@@ -1834,6 +1834,14 @@ func TestJoinRuleKnock(t *testing.T) {
 					"state_key": "@u4:a",
 					"event_id": "$e2:a",
 					"content": {"membership": "knock"}
+				},
+				"@u5:a": {
+					"type": "m.room.member",
+					"sender": "@u5:a",
+					"room_id": "!r1:a",
+					"state_key": "@u5:a",
+					"event_id": "$e2:a",
+					"content": {"membership": "ban"}
 				}
 			}
 		},
@@ -1873,6 +1881,28 @@ func TestJoinRuleKnock(t *testing.T) {
 			"unsigned": {
 				"not_allowed": "Sender not invited or joined"
 			}
+		},
+		{
+			"type": "m.room.member",
+			"sender": "@u3:a",
+			"room_id": "!r1:a",
+			"state_key": "@u3:a",
+			"event_id": "$e2:a",
+			"content": {"membership": "knock"},
+			"unsigned": {
+				"not_allowed": "Sender is already joined"
+			}
+		},
+		{
+			"type": "m.room.member",
+			"sender": "@u5:a",
+			"room_id": "!r1:a",
+			"state_key": "@u5:a",
+			"event_id": "$e2:a",
+			"content": {"membership": "knock"},
+			"unsigned": {
+				"not_allowed": "Sender is banned"
+			}
 		}]
 	}`, RoomVersionV10)
 }