From 43cddfe00f53e3a2df4769be31c66cd818a2966e Mon Sep 17 00:00:00 2001 From: Kegsay Date: Thu, 25 Jun 2020 15:04:48 +0100 Subject: [PATCH] Return remote errors from FS.PerformJoin (#1164) * Return remote errors from FS.PerformJoin Follows the same pattern as PerformJoin on roomserver (no error return). Also return the right format for incompatible room version errors. Makes a bunch of tests pass! * Handle network errors better when returning remote HTTP errors * Linting * Fix tests * Update whitelist, pass network errors through in API=1 mode --- clientapi/jsonerror/jsonerror.go | 14 ++++++++++++-- federationsender/api/api.go | 4 +++- federationsender/internal/perform.go | 28 +++++++++++++++++++++++----- federationsender/inthttp/client.go | 12 ++++++++++-- federationsender/inthttp/server.go | 4 +--- go.mod | 2 +- go.sum | 4 ++-- roomserver/api/perform.go | 19 +++++++++++++++++-- roomserver/internal/perform_join.go | 10 +++++++--- sytest-whitelist | 7 +++++++ 10 files changed, 83 insertions(+), 21 deletions(-) diff --git a/clientapi/jsonerror/jsonerror.go b/clientapi/jsonerror/jsonerror.go index 85e887ae..7f8f264b 100644 --- a/clientapi/jsonerror/jsonerror.go +++ b/clientapi/jsonerror/jsonerror.go @@ -125,10 +125,20 @@ func GuestAccessForbidden(msg string) *MatrixError { return &MatrixError{"M_GUEST_ACCESS_FORBIDDEN", msg} } +type IncompatibleRoomVersionError struct { + RoomVersion string `json:"room_version"` + Error string `json:"error"` + Code string `json:"errcode"` +} + // IncompatibleRoomVersion is an error which is returned when the client // requests a room with a version that is unsupported. -func IncompatibleRoomVersion(roomVersion gomatrixserverlib.RoomVersion) *MatrixError { - return &MatrixError{"M_INCOMPATIBLE_ROOM_VERSION", string(roomVersion)} +func IncompatibleRoomVersion(roomVersion gomatrixserverlib.RoomVersion) *IncompatibleRoomVersionError { + return &IncompatibleRoomVersionError{ + Code: "M_INCOMPATIBLE_ROOM_VERSION", + RoomVersion: string(roomVersion), + Error: "Your homeserver does not support the features required to join this room", + } } // UnsupportedRoomVersion is an error which is returned when the client diff --git a/federationsender/api/api.go b/federationsender/api/api.go index 02c76258..d90ffd29 100644 --- a/federationsender/api/api.go +++ b/federationsender/api/api.go @@ -4,6 +4,7 @@ import ( "context" "github.com/matrix-org/dendrite/federationsender/types" + "github.com/matrix-org/gomatrix" "github.com/matrix-org/gomatrixserverlib" ) @@ -28,7 +29,7 @@ type FederationSenderInternalAPI interface { ctx context.Context, request *PerformJoinRequest, response *PerformJoinResponse, - ) error + ) // Handle an instruction to make_leave & send_leave with a remote server. PerformLeave( ctx context.Context, @@ -62,6 +63,7 @@ type PerformJoinRequest struct { } type PerformJoinResponse struct { + LastError *gomatrix.HTTPError } type PerformLeaveRequest struct { diff --git a/federationsender/internal/perform.go b/federationsender/internal/perform.go index 7ced4af8..96b1149d 100644 --- a/federationsender/internal/perform.go +++ b/federationsender/internal/perform.go @@ -2,6 +2,7 @@ package internal import ( "context" + "errors" "fmt" "time" @@ -9,6 +10,7 @@ import ( "github.com/matrix-org/dendrite/federationsender/internal/perform" roomserverAPI "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/roomserver/version" + "github.com/matrix-org/gomatrix" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" "github.com/sirupsen/logrus" @@ -40,7 +42,7 @@ func (r *FederationSenderInternalAPI) PerformJoin( ctx context.Context, request *api.PerformJoinRequest, response *api.PerformJoinResponse, -) (err error) { +) { // Look up the supported room versions. var supportedVersions []gomatrixserverlib.RoomVersion for version := range version.SupportedRoomVersions() { @@ -63,6 +65,7 @@ func (r *FederationSenderInternalAPI) PerformJoin( // Try each server that we were provided until we land on one that // successfully completes the make-join send-join dance. + var lastErr error for _, serverName := range request.ServerNames { if err := r.performJoinUsingServer( ctx, @@ -76,17 +79,32 @@ func (r *FederationSenderInternalAPI) PerformJoin( "server_name": serverName, "room_id": request.RoomID, }).Warnf("Failed to join room through server") + lastErr = err continue } // We're all good. - return nil + return } // If we reach here then we didn't complete a join for some reason. - return fmt.Errorf( - "failed to join user %q to room %q through %d server(s)", - request.UserID, request.RoomID, len(request.ServerNames), + var httpErr gomatrix.HTTPError + if ok := errors.As(lastErr, &httpErr); ok { + httpErr.Message = string(httpErr.Contents) + // Clear the wrapped error, else serialising to JSON (in polylith mode) will fail + httpErr.WrappedError = nil + response.LastError = &httpErr + } else { + response.LastError = &gomatrix.HTTPError{ + Code: 0, + WrappedError: nil, + Message: lastErr.Error(), + } + } + + logrus.Errorf( + "failed to join user %q to room %q through %d server(s): last error %s", + request.UserID, request.RoomID, len(request.ServerNames), lastErr, ) } diff --git a/federationsender/inthttp/client.go b/federationsender/inthttp/client.go index 5da4b35f..25de99cc 100644 --- a/federationsender/inthttp/client.go +++ b/federationsender/inthttp/client.go @@ -7,6 +7,7 @@ import ( "github.com/matrix-org/dendrite/federationsender/api" "github.com/matrix-org/dendrite/internal/httputil" + "github.com/matrix-org/gomatrix" "github.com/opentracing/opentracing-go" ) @@ -77,12 +78,19 @@ func (h *httpFederationSenderInternalAPI) PerformJoin( ctx context.Context, request *api.PerformJoinRequest, response *api.PerformJoinResponse, -) error { +) { span, ctx := opentracing.StartSpanFromContext(ctx, "PerformJoinRequest") defer span.Finish() apiURL := h.federationSenderURL + FederationSenderPerformJoinRequestPath - return httputil.PostJSON(ctx, span, h.httpClient, apiURL, request, response) + err := httputil.PostJSON(ctx, span, h.httpClient, apiURL, request, response) + if err != nil { + response.LastError = &gomatrix.HTTPError{ + Message: err.Error(), + Code: 0, + WrappedError: err, + } + } } // Handle an instruction to make_join & send_join with a remote server. diff --git a/federationsender/inthttp/server.go b/federationsender/inthttp/server.go index babd3ae1..a4f3d63d 100644 --- a/federationsender/inthttp/server.go +++ b/federationsender/inthttp/server.go @@ -33,9 +33,7 @@ func AddRoutes(intAPI api.FederationSenderInternalAPI, internalAPIMux *mux.Route if err := json.NewDecoder(req.Body).Decode(&request); err != nil { return util.MessageResponse(http.StatusBadRequest, err.Error()) } - if err := intAPI.PerformJoin(req.Context(), &request, &response); err != nil { - return util.ErrorResponse(err) - } + intAPI.PerformJoin(req.Context(), &request, &response) return util.JSONResponse{Code: http.StatusOK, JSON: &response} }), ) diff --git a/go.mod b/go.mod index 6bfce844..57cdb909 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/matrix-org/go-http-js-libp2p v0.0.0-20200518170932-783164aeeda4 github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26 - github.com/matrix-org/gomatrixserverlib v0.0.0-20200623103809-13ff8109e137 + github.com/matrix-org/gomatrixserverlib v0.0.0-20200625121044-e5d892cd30c1 github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7 github.com/mattn/go-sqlite3 v2.0.2+incompatible diff --git a/go.sum b/go.sum index 6178f152..973b003c 100644 --- a/go.sum +++ b/go.sum @@ -371,8 +371,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3 h1:Yb+Wlf github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3/go.mod h1:e+cg2q7C7yE5QnAXgzo512tgFh1RbQLC0+jozuegKgo= github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26 h1:Hr3zjRsq2bhrnp3Ky1qgx/fzCtCALOoGYylh2tpS9K4= github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0= -github.com/matrix-org/gomatrixserverlib v0.0.0-20200623103809-13ff8109e137 h1:+eBh4L04+08IslvFM071TNrQTggU317GsQKzZ1SGEVo= -github.com/matrix-org/gomatrixserverlib v0.0.0-20200623103809-13ff8109e137/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU= +github.com/matrix-org/gomatrixserverlib v0.0.0-20200625121044-e5d892cd30c1 h1:3yS6hw01X72jpJuAPGVOY+QFD9cpAETR/6Hq2WYKbpU= +github.com/matrix-org/gomatrixserverlib v0.0.0-20200625121044-e5d892cd30c1/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU= github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f h1:pRz4VTiRCO4zPlEMc3ESdUOcW4PXHH4Kj+YDz1XyE+Y= github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f/go.mod h1:y0oDTjZDv5SM9a2rp3bl+CU+bvTRINQsdb7YlDql5Go= github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7 h1:ntrLa/8xVzeSs8vHFHK25k0C+NV74sYMJnNSg5NoSRo= diff --git a/roomserver/api/perform.go b/roomserver/api/perform.go index 12ba1516..5d8d88a5 100644 --- a/roomserver/api/perform.go +++ b/roomserver/api/perform.go @@ -1,6 +1,7 @@ package api import ( + "encoding/json" "fmt" "net/http" @@ -12,8 +13,9 @@ import ( type PerformErrorCode int type PerformError struct { - Msg string - Code PerformErrorCode + Msg string + RemoteCode int // remote HTTP status code, for PerformErrRemote + Code PerformErrorCode } func (p *PerformError) Error() string { @@ -43,6 +45,17 @@ func (p *PerformError) JSONResponse() util.JSONResponse { Code: http.StatusForbidden, JSON: jsonerror.Forbidden(p.Msg), } + case PerformErrRemote: + // if the code is 0 then something bad happened and it isn't + // a remote HTTP error being encapsulated, e.g network error to remote. + if p.RemoteCode == 0 { + return util.ErrorResponse(fmt.Errorf("%s", p.Msg)) + } + return util.JSONResponse{ + Code: p.RemoteCode, + // TODO: Should we assert this is in fact JSON? E.g gjson parse? + JSON: json.RawMessage(p.Msg), + } default: return util.ErrorResponse(p) } @@ -57,6 +70,8 @@ const ( PerformErrorNoRoom PerformErrorCode = 3 // PerformErrorNoOperation means that the request resulted in nothing happening e.g invite->invite or leave->leave. PerformErrorNoOperation PerformErrorCode = 4 + // PerformErrRemote means that the request failed and the PerformError.Msg is the raw remote JSON error response + PerformErrRemote PerformErrorCode = 5 ) type PerformJoinRequest struct { diff --git a/roomserver/internal/perform_join.go b/roomserver/internal/perform_join.go index d409b684..b594c2d8 100644 --- a/roomserver/internal/perform_join.go +++ b/roomserver/internal/perform_join.go @@ -270,9 +270,13 @@ func (r *RoomserverInternalAPI) performFederatedJoinRoomByID( Content: req.Content, // the membership event content } fedRes := fsAPI.PerformJoinResponse{} - if err := r.fsAPI.PerformJoin(ctx, &fedReq, &fedRes); err != nil { - return fmt.Errorf("Error joining federated room: %q", err) + r.fsAPI.PerformJoin(ctx, &fedReq, &fedRes) + if fedRes.LastError != nil { + return &api.PerformError{ + Code: api.PerformErrRemote, + Msg: fedRes.LastError.Message, + RemoteCode: fedRes.LastError.Code, + } } - return nil } diff --git a/sytest-whitelist b/sytest-whitelist index ce97e8af..eb28898a 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -366,3 +366,10 @@ Typing notification sent to local room members Typing notifications also sent to remote room members Typing can be explicitly stopped Banned user is kicked and may not rejoin until unbanned +Inbound federation rejects attempts to join v1 rooms from servers without v1 support +Inbound federation rejects attempts to join v2 rooms from servers lacking version support +Inbound federation rejects attempts to join v2 rooms from servers only supporting v1 +Outbound federation passes make_join failures through to the client +Outbound federation correctly handles unsupported room versions +Remote users may not join unfederated rooms +Guest users denied access over federation if guest access prohibited