From 08c6f23fedecfafdb1430d7ed35185c165fbc943 Mon Sep 17 00:00:00 2001
From: Till Faelligen <2353100+S7evinK@users.noreply.github.com>
Date: Fri, 9 Dec 2022 09:07:21 +0100
Subject: [PATCH] Review comments

---
 roomserver/internal/api.go                | 18 +++++++++---------
 roomserver/internal/input/input_events.go | 16 +++++++---------
 roomserver/roomserver_test.go             |  3 ++-
 userapi/inthttp/server.go                 |  2 +-
 4 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/roomserver/internal/api.go b/roomserver/internal/api.go
index e0d3d9b33..451b37696 100644
--- a/roomserver/internal/api.go
+++ b/roomserver/internal/api.go
@@ -4,6 +4,10 @@ import (
 	"context"
 
 	"github.com/getsentry/sentry-go"
+	"github.com/matrix-org/gomatrixserverlib"
+	"github.com/nats-io/nats.go"
+	"github.com/sirupsen/logrus"
+
 	asAPI "github.com/matrix-org/dendrite/appservice/api"
 	fsAPI "github.com/matrix-org/dendrite/federationapi/api"
 	"github.com/matrix-org/dendrite/internal/caching"
@@ -19,9 +23,6 @@ import (
 	"github.com/matrix-org/dendrite/setup/jetstream"
 	"github.com/matrix-org/dendrite/setup/process"
 	userapi "github.com/matrix-org/dendrite/userapi/api"
-	"github.com/matrix-org/gomatrixserverlib"
-	"github.com/nats-io/nats.go"
-	"github.com/sirupsen/logrus"
 )
 
 // RoomserverInternalAPI is an implementation of api.RoomserverInternalAPI
@@ -104,9 +105,8 @@ func (r *RoomserverInternalAPI) SetFederationAPI(fsAPI fsAPI.RoomserverFederatio
 	r.fsAPI = fsAPI
 	r.KeyRing = keyRing
 
-	identity, err := r.Cfg.Matrix.SigningIdentityFor(r.Cfg.Matrix.ServerName)
-	// If federation is enabled, but we don't have a signing key, bail.
-	if err != nil && !r.Cfg.Matrix.DisableFederation {
+	identity, err := r.Cfg.Matrix.SigningIdentityFor(r.ServerName)
+	if err != nil {
 		logrus.Panic(err)
 	}
 
@@ -120,7 +120,7 @@ func (r *RoomserverInternalAPI) SetFederationAPI(fsAPI fsAPI.RoomserverFederatio
 		JetStream:           r.JetStream,
 		NATSClient:          r.NATSClient,
 		Durable:             nats.Durable(r.Durable),
-		ServerName:          r.Cfg.Matrix.ServerName,
+		ServerName:          r.ServerName,
 		SigningIdentity:     identity,
 		FSAPI:               fsAPI,
 		KeyRing:             keyRing,
@@ -142,7 +142,7 @@ func (r *RoomserverInternalAPI) SetFederationAPI(fsAPI fsAPI.RoomserverFederatio
 		Queryer: r.Queryer,
 	}
 	r.Peeker = &perform.Peeker{
-		ServerName: r.Cfg.Matrix.ServerName,
+		ServerName: r.ServerName,
 		Cfg:        r.Cfg,
 		DB:         r.DB,
 		FSAPI:      r.fsAPI,
@@ -153,7 +153,7 @@ func (r *RoomserverInternalAPI) SetFederationAPI(fsAPI fsAPI.RoomserverFederatio
 		Inputer: r.Inputer,
 	}
 	r.Unpeeker = &perform.Unpeeker{
-		ServerName: r.Cfg.Matrix.ServerName,
+		ServerName: r.ServerName,
 		Cfg:        r.Cfg,
 		DB:         r.DB,
 		FSAPI:      r.fsAPI,
diff --git a/roomserver/internal/input/input_events.go b/roomserver/internal/input/input_events.go
index 3be67d4dc..8e9e39d03 100644
--- a/roomserver/internal/input/input_events.go
+++ b/roomserver/internal/input/input_events.go
@@ -26,13 +26,14 @@ import (
 
 	"github.com/tidwall/gjson"
 
-	userAPI "github.com/matrix-org/dendrite/userapi/api"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
 	"github.com/opentracing/opentracing-go"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/sirupsen/logrus"
 
+	userAPI "github.com/matrix-org/dendrite/userapi/api"
+
 	fedapi "github.com/matrix-org/dendrite/federationapi/api"
 	"github.com/matrix-org/dendrite/internal"
 	"github.com/matrix-org/dendrite/internal/eventutil"
@@ -442,8 +443,11 @@ func (r *Inputer) processRoomEvent(
 		}
 	}
 
-	if err = r.kickGuests(ctx, event, roomInfo); err != nil {
-		logrus.WithError(err).Error("failed to kick guest users on m.room.guest_access revocation")
+	// If guest_access changed and is not can_join, kick all guest users.
+	if event.Type() == gomatrixserverlib.MRoomGuestAccess && gjson.GetBytes(event.Content(), "guest_access").String() != "can_join" {
+		if err = r.kickGuests(ctx, event, roomInfo); err != nil {
+			logrus.WithError(err).Error("failed to kick guest users on m.room.guest_access revocation")
+		}
 	}
 
 	// Everything was OK — the latest events updater didn't error and
@@ -738,12 +742,6 @@ func (r *Inputer) calculateAndSetState(
 
 // kickGuests kicks guests users from m.room.guest_access rooms, if guest access is now prohibited.
 func (r *Inputer) kickGuests(ctx context.Context, event *gomatrixserverlib.Event, roomInfo *types.RoomInfo) error {
-	if event.Type() != gomatrixserverlib.MRoomGuestAccess {
-		return nil
-	}
-	if gjson.GetBytes(event.Content(), "guest_access").String() == "can_join" {
-		return nil
-	}
 	membershipNIDs, err := r.DB.GetMembershipEventNIDsForRoom(ctx, roomInfo.RoomNID, true, true)
 	if err != nil {
 		return err
diff --git a/roomserver/roomserver_test.go b/roomserver/roomserver_test.go
index 9e6e235ab..562a40800 100644
--- a/roomserver/roomserver_test.go
+++ b/roomserver/roomserver_test.go
@@ -10,11 +10,12 @@ import (
 
 	userAPI "github.com/matrix-org/dendrite/userapi/api"
 
+	"github.com/matrix-org/gomatrixserverlib"
+
 	"github.com/matrix-org/dendrite/roomserver"
 	"github.com/matrix-org/dendrite/roomserver/api"
 	"github.com/matrix-org/dendrite/test"
 	"github.com/matrix-org/dendrite/test/testrig"
-	"github.com/matrix-org/gomatrixserverlib"
 )
 
 func TestUsers(t *testing.T) {
diff --git a/userapi/inthttp/server.go b/userapi/inthttp/server.go
index 9c69e51a6..b40b507c2 100644
--- a/userapi/inthttp/server.go
+++ b/userapi/inthttp/server.go
@@ -192,6 +192,6 @@ func AddRoutes(internalAPIMux *mux.Router, s api.UserInternalAPI, enableMetrics
 
 	internalAPIMux.Handle(
 		QueryAccountByLocalpartPath,
-		httputil.MakeInternalRPCAPI("AccountByLocalpart", s.QueryAccountByLocalpart),
+		httputil.MakeInternalRPCAPI("AccountByLocalpart", enableMetrics, s.QueryAccountByLocalpart),
 	)
 }