From 4b09bed8271f7aa8b3afc2619c52a573cd9d99c4 Mon Sep 17 00:00:00 2001
From: Sam Wedgwood <sam@wedgwood.dev>
Date: Thu, 27 Jul 2023 11:28:35 +0100
Subject: [PATCH] check for room membership in RemoveLocalAlias - otherwise
 causing 500 errors later in the pipeline, as no room membership = no sender
 ID (for pseudo ID rooms) - also add an explicit check for lack of a Sender ID

---
 clientapi/routing/directory.go | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/clientapi/routing/directory.go b/clientapi/routing/directory.go
index e914b2905..d5bd2f385 100644
--- a/clientapi/routing/directory.go
+++ b/clientapi/routing/directory.go
@@ -267,6 +267,24 @@ func RemoveLocalAlias(
 			JSON: spec.NotFound("The alias does not exist."),
 		}
 	}
+
+	// This seems like the kind of auth check that should be done in the roomserver, but
+	// if this check fails (user is not in the room), then there will be no SenderID for the user
+	// for pseudo-ID rooms - it will just return "". However, we can't use lack of a sender ID
+	// as meaning they are not in the room, since lacking a sender ID could be caused by other bugs.
+	// TODO: maybe have QuerySenderIDForUser return richer errors?
+	var queryResp roomserverAPI.QueryMembershipForUserResponse
+	rsAPI.QueryMembershipForUser(req.Context(), &roomserverAPI.QueryMembershipForUserRequest{
+		RoomID: validRoomID.String(),
+		UserID: *userID,
+	}, &queryResp)
+	if !queryResp.IsInRoom {
+		return util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: spec.Forbidden("You do not have permission to remove this alias."),
+		}
+	}
+
 	deviceSenderID, err := rsAPI.QuerySenderIDForUser(req.Context(), *validRoomID, *userID)
 	if err != nil {
 		return util.JSONResponse{
@@ -274,6 +292,13 @@ func RemoveLocalAlias(
 			JSON: spec.NotFound("The alias does not exist."),
 		}
 	}
+	// TODO: how to handle this case? missing user/room keys seem to be a whole new class of errors
+	if deviceSenderID == "" {
+		return util.JSONResponse{
+			Code: http.StatusInternalServerError,
+			JSON: spec.Unknown("internal server error"),
+		}
+	}
 
 	queryReq := roomserverAPI.RemoveRoomAliasRequest{
 		Alias:    alias,