From 6d50212f29f2ce3231097833de7686e3237359b4 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 26 May 2020 14:41:16 +0100 Subject: [PATCH] Miscellaneous fixes (#1060) * Add missing routing for PerformDirectoryLookupRequest * Tweak output * Fix some bugs in devices * Don't default to federated room joins in response to invite * Update sytest-whitelist * Update comments * Return correct room ID from PerformJoin * Fix appservice and EDU server API setup, update sytest-whitelist * Update sytest-whitelist --- appservice/appservice.go | 4 +-- appservice/query/query.go | 7 ++-- .../storage/devices/postgres/devices_table.go | 12 +++++-- .../storage/devices/sqlite3/devices_table.go | 12 +++++-- clientapi/routing/joinroom.go | 2 +- eduserver/eduserver.go | 6 +--- eduserver/input/input.go | 5 +-- federationsender/internal/api.go | 13 +++++++ internal/http/http.go | 4 +-- roomserver/api/perform.go | 1 + roomserver/internal/perform_join.go | 36 +++++++++++++------ sytest-whitelist | 1 + 12 files changed, 71 insertions(+), 32 deletions(-) diff --git a/appservice/appservice.go b/appservice/appservice.go index 76181d2a3..be5b30e2b 100644 --- a/appservice/appservice.go +++ b/appservice/appservice.go @@ -82,9 +82,7 @@ func SetupAppServiceAPIComponent( Cfg: base.Cfg, } - if base.EnableHTTPAPIs { - appserviceQueryAPI.SetupHTTP(http.DefaultServeMux) - } + appserviceQueryAPI.SetupHTTP(base.InternalAPIMux) consumer := consumers.NewOutputRoomEventConsumer( base.Cfg, base.KafkaConsumer, accountsDB, appserviceDB, diff --git a/appservice/query/query.go b/appservice/query/query.go index a61997b42..812ca9f49 100644 --- a/appservice/query/query.go +++ b/appservice/query/query.go @@ -23,6 +23,7 @@ import ( "net/url" "time" + "github.com/gorilla/mux" "github.com/matrix-org/dendrite/appservice/api" "github.com/matrix-org/dendrite/internal" "github.com/matrix-org/dendrite/internal/config" @@ -182,8 +183,8 @@ func makeHTTPClient() *http.Client { // SetupHTTP adds the AppServiceQueryPAI handlers to the http.ServeMux. This // handles and muxes incoming api requests the to internal AppServiceQueryAPI. -func (a *AppServiceQueryAPI) SetupHTTP(servMux *http.ServeMux) { - servMux.Handle( +func (a *AppServiceQueryAPI) SetupHTTP(internalAPIMux *mux.Router) { + internalAPIMux.Handle( api.AppServiceRoomAliasExistsPath, internal.MakeInternalAPI("appserviceRoomAliasExists", func(req *http.Request) util.JSONResponse { var request api.RoomAliasExistsRequest @@ -197,7 +198,7 @@ func (a *AppServiceQueryAPI) SetupHTTP(servMux *http.ServeMux) { return util.JSONResponse{Code: http.StatusOK, JSON: &response} }), ) - servMux.Handle( + internalAPIMux.Handle( api.AppServiceUserIDExistsPath, internal.MakeInternalAPI("appserviceUserIDExists", func(req *http.Request) util.JSONResponse { var request api.UserIDExistsRequest diff --git a/clientapi/auth/storage/devices/postgres/devices_table.go b/clientapi/auth/storage/devices/postgres/devices_table.go index 67d573f95..c84e83d3d 100644 --- a/clientapi/auth/storage/devices/postgres/devices_table.go +++ b/clientapi/auth/storage/devices/postgres/devices_table.go @@ -206,9 +206,8 @@ func (s *devicesStatements) selectDeviceByID( ctx context.Context, localpart, deviceID string, ) (*authtypes.Device, error) { var dev authtypes.Device - var created sql.NullInt64 stmt := s.selectDeviceByIDStmt - err := stmt.QueryRowContext(ctx, localpart, deviceID).Scan(&created) + err := stmt.QueryRowContext(ctx, localpart, deviceID).Scan(&dev.DisplayName) if err == nil { dev.ID = deviceID dev.UserID = userutil.MakeUserID(localpart, s.serverName) @@ -230,10 +229,17 @@ func (s *devicesStatements) selectDevicesByLocalpart( for rows.Next() { var dev authtypes.Device - err = rows.Scan(&dev.ID, &dev.DisplayName) + var id, displayname sql.NullString + err = rows.Scan(&id, &displayname) if err != nil { return devices, err } + if id.Valid { + dev.ID = id.String + } + if displayname.Valid { + dev.DisplayName = displayname.String + } dev.UserID = userutil.MakeUserID(localpart, s.serverName) devices = append(devices, dev) } diff --git a/clientapi/auth/storage/devices/sqlite3/devices_table.go b/clientapi/auth/storage/devices/sqlite3/devices_table.go index 6ffc1646f..49f3eaed7 100644 --- a/clientapi/auth/storage/devices/sqlite3/devices_table.go +++ b/clientapi/auth/storage/devices/sqlite3/devices_table.go @@ -208,9 +208,8 @@ func (s *devicesStatements) selectDeviceByID( ctx context.Context, localpart, deviceID string, ) (*authtypes.Device, error) { var dev authtypes.Device - var created sql.NullInt64 stmt := s.selectDeviceByIDStmt - err := stmt.QueryRowContext(ctx, localpart, deviceID).Scan(&created) + err := stmt.QueryRowContext(ctx, localpart, deviceID).Scan(&dev.DisplayName) if err == nil { dev.ID = deviceID dev.UserID = userutil.MakeUserID(localpart, s.serverName) @@ -231,10 +230,17 @@ func (s *devicesStatements) selectDevicesByLocalpart( for rows.Next() { var dev authtypes.Device - err = rows.Scan(&dev.ID, &dev.DisplayName) + var id, displayname sql.NullString + err = rows.Scan(&id, &displayname) if err != nil { return devices, err } + if id.Valid { + dev.ID = id.String + } + if displayname.Valid { + dev.DisplayName = displayname.String + } dev.UserID = userutil.MakeUserID(localpart, s.serverName) devices = append(devices, dev) } diff --git a/clientapi/routing/joinroom.go b/clientapi/routing/joinroom.go index 500df337a..a3d676532 100644 --- a/clientapi/routing/joinroom.go +++ b/clientapi/routing/joinroom.go @@ -75,6 +75,6 @@ func JoinRoomByIDOrAlias( // TODO: Put the response struct somewhere internal. JSON: struct { RoomID string `json:"room_id"` - }{joinReq.RoomIDOrAlias}, + }{joinRes.RoomID}, } } diff --git a/eduserver/eduserver.go b/eduserver/eduserver.go index cba60b8ea..14fbd3328 100644 --- a/eduserver/eduserver.go +++ b/eduserver/eduserver.go @@ -13,8 +13,6 @@ package eduserver import ( - "net/http" - "github.com/matrix-org/dendrite/eduserver/api" "github.com/matrix-org/dendrite/eduserver/cache" "github.com/matrix-org/dendrite/eduserver/input" @@ -35,9 +33,7 @@ func SetupEDUServerComponent( OutputTypingEventTopic: string(base.Cfg.Kafka.Topics.OutputTypingEvent), } - if base.EnableHTTPAPIs { - inputAPI.SetupHTTP(http.DefaultServeMux) - } + inputAPI.SetupHTTP(base.InternalAPIMux) return inputAPI } diff --git a/eduserver/input/input.go b/eduserver/input/input.go index 50837154a..73777e323 100644 --- a/eduserver/input/input.go +++ b/eduserver/input/input.go @@ -19,6 +19,7 @@ import ( "time" "github.com/Shopify/sarama" + "github.com/gorilla/mux" "github.com/matrix-org/dendrite/eduserver/api" "github.com/matrix-org/dendrite/eduserver/cache" "github.com/matrix-org/dendrite/internal" @@ -90,8 +91,8 @@ func (t *EDUServerInputAPI) sendEvent(ite *api.InputTypingEvent) error { } // SetupHTTP adds the EDUServerInputAPI handlers to the http.ServeMux. -func (t *EDUServerInputAPI) SetupHTTP(servMux *http.ServeMux) { - servMux.Handle(api.EDUServerInputTypingEventPath, +func (t *EDUServerInputAPI) SetupHTTP(internalAPIMux *mux.Router) { + internalAPIMux.Handle(api.EDUServerInputTypingEventPath, internal.MakeInternalAPI("inputTypingEvents", func(req *http.Request) util.JSONResponse { var request api.InputTypingEventRequest var response api.InputTypingEventResponse diff --git a/federationsender/internal/api.go b/federationsender/internal/api.go index 4805e7050..dd3942583 100644 --- a/federationsender/internal/api.go +++ b/federationsender/internal/api.go @@ -99,4 +99,17 @@ func (f *FederationSenderInternalAPI) SetupHTTP(internalAPIMux *mux.Router) { return util.JSONResponse{Code: http.StatusOK, JSON: &response} }), ) + internalAPIMux.Handle(api.FederationSenderPerformDirectoryLookupRequestPath, + internal.MakeInternalAPI("PerformDirectoryLookupRequest", func(req *http.Request) util.JSONResponse { + var request api.PerformDirectoryLookupRequest + var response api.PerformDirectoryLookupResponse + if err := json.NewDecoder(req.Body).Decode(&request); err != nil { + return util.MessageResponse(http.StatusBadRequest, err.Error()) + } + if err := f.PerformDirectoryLookup(req.Context(), &request, &response); err != nil { + return util.ErrorResponse(err) + } + return util.JSONResponse{Code: http.StatusOK, JSON: &response} + }), + ) } diff --git a/internal/http/http.go b/internal/http/http.go index da90b3751..2b1891403 100644 --- a/internal/http/http.go +++ b/internal/http/http.go @@ -60,9 +60,9 @@ func PostJSON( Message string `json:"message"` } if msgerr := json.NewDecoder(res.Body).Decode(&errorBody); msgerr == nil { - return fmt.Errorf("api: %d from %s: %s", res.StatusCode, apiURL, errorBody.Message) + return fmt.Errorf("Internal API: %d from %s: %s", res.StatusCode, apiURL, errorBody.Message) } - return fmt.Errorf("api: %d from %s", res.StatusCode, apiURL) + return fmt.Errorf("Internal API: %d from %s", res.StatusCode, apiURL) } return json.NewDecoder(res.Body).Decode(response) } diff --git a/roomserver/api/perform.go b/roomserver/api/perform.go index c98f91fb4..f5afd67bf 100644 --- a/roomserver/api/perform.go +++ b/roomserver/api/perform.go @@ -24,6 +24,7 @@ type PerformJoinRequest struct { } type PerformJoinResponse struct { + RoomID string `json:"room_id"` } func (h *httpRoomserverInternalAPI) PerformJoin( diff --git a/roomserver/internal/perform_join.go b/roomserver/internal/perform_join.go index dc786b296..75dfdd19c 100644 --- a/roomserver/internal/perform_join.go +++ b/roomserver/internal/perform_join.go @@ -90,6 +90,12 @@ func (r *RoomserverInternalAPI) performJoinRoomByID( req *api.PerformJoinRequest, res *api.PerformJoinResponse, // nolint:unparam ) error { + // By this point, if req.RoomIDOrAlias contained an alias, then + // it will have been overwritten with a room ID by performJoinRoomByAlias. + // We should now include this in the response so that the CS API can + // return the right room ID. + res.RoomID = req.RoomIDOrAlias + // Get the domain part of the room ID. _, domain, err := gomatrixserverlib.SplitID('!', req.RoomIDOrAlias) if err != nil { @@ -121,20 +127,30 @@ func (r *RoomserverInternalAPI) performJoinRoomByID( return fmt.Errorf("eb.SetContent: %w", err) } - // First work out if this is in response to an existing invite. - // If it is then we avoid the situation where we might think we - // know about a room in the following section but don't know the - // latest state as all of our users have left. + // First work out if this is in response to an existing invite + // from a federated server. If it is then we avoid the situation + // where we might think we know about a room in the following + // section but don't know the latest state as all of our users + // have left. isInvitePending, inviteSender, err := r.isInvitePending(ctx, req.RoomIDOrAlias, req.UserID) if err == nil && isInvitePending { - // Add the server of the person who invited us to the server list, - // as they should be a fairly good bet. - if _, inviterDomain, ierr := gomatrixserverlib.SplitID('@', inviteSender); ierr == nil { - req.ServerNames = append(req.ServerNames, inviterDomain) + // Check if there's an invite pending. + _, inviterDomain, ierr := gomatrixserverlib.SplitID('@', inviteSender) + if ierr != nil { + return fmt.Errorf("gomatrixserverlib.SplitID: %w", err) } - // Perform a federated room join. - return r.performFederatedJoinRoomByID(ctx, req, res) + // Check that the domain isn't ours. If it's local then we don't + // need to do anything as our own copy of the room state will be + // up-to-date. + if inviterDomain != r.Cfg.Matrix.ServerName { + // Add the server of the person who invited us to the server list, + // as they should be a fairly good bet. + req.ServerNames = append(req.ServerNames, inviterDomain) + + // Perform a federated room join. + return r.performFederatedJoinRoomByID(ctx, req, res) + } } // Try to construct an actual join event from the template. diff --git a/sytest-whitelist b/sytest-whitelist index 1b12aba1b..d4e6be9a4 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -288,3 +288,4 @@ New room members see their own join event Existing members see new members' join events Inbound federation can receive events Inbound federation can receive redacted events +Can logout current device