From ba221c41c57d5003a974ed98ebfcfdd716b54ae6 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 7 Sep 2017 16:55:59 +0100 Subject: [PATCH] Use standard errors instead of JSON responses in threepid --- .../dendrite/clientapi/readers/threepid.go | 28 +++++++-- .../dendrite/clientapi/threepid/invites.go | 60 ++++--------------- .../dendrite/clientapi/threepid/threepid.go | 3 +- .../dendrite/clientapi/writers/membership.go | 32 +++++++++- 4 files changed, 65 insertions(+), 58 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/clientapi/readers/threepid.go b/src/github.com/matrix-org/dendrite/clientapi/readers/threepid.go index eeb31b8b0..b0d792875 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/readers/threepid.go +++ b/src/github.com/matrix-org/dendrite/clientapi/readers/threepid.go @@ -65,8 +65,13 @@ func RequestEmailToken(req *http.Request, accountDB *accounts.Database, cfg conf } resp.SID, err = threepid.CreateSession(body, cfg) - if err != nil { - return threepid.ProcessIDServerError(req, err) + if err == threepid.ErrNotTrusted { + return util.JSONResponse{ + Code: 400, + JSON: jsonerror.NotTrusted(body.IDServer), + } + } else if err != nil { + return httputil.LogThenError(req, err) } return util.JSONResponse{ @@ -87,8 +92,13 @@ func CheckAndSave3PIDAssociation( // Check if the association has been validated verified, address, medium, err := threepid.CheckAssociation(body.Creds, cfg) - if err != nil { - return threepid.ProcessIDServerError(req, err) + if err == threepid.ErrNotTrusted { + return util.JSONResponse{ + Code: 400, + JSON: jsonerror.NotTrusted(body.Creds.IDServer), + } + } else if err != nil { + return httputil.LogThenError(req, err) } if !verified { @@ -103,8 +113,14 @@ func CheckAndSave3PIDAssociation( if body.Bind { // Publish the association on the identity server if requested - if err = threepid.PublishAssociation(body.Creds, device.UserID, cfg); err != nil { - return threepid.ProcessIDServerError(req, err) + err = threepid.PublishAssociation(body.Creds, device.UserID, cfg) + if err == threepid.ErrNotTrusted { + return util.JSONResponse{ + Code: 400, + JSON: jsonerror.NotTrusted(body.Creds.IDServer), + } + } else if err != nil { + return httputil.LogThenError(req, err) } } diff --git a/src/github.com/matrix-org/dendrite/clientapi/threepid/invites.go b/src/github.com/matrix-org/dendrite/clientapi/threepid/invites.go index 4a8686820..339999ad9 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/threepid/invites.go +++ b/src/github.com/matrix-org/dendrite/clientapi/threepid/invites.go @@ -26,15 +26,11 @@ import ( "github.com/matrix-org/dendrite/clientapi/auth/authtypes" "github.com/matrix-org/dendrite/clientapi/auth/storage/accounts" "github.com/matrix-org/dendrite/clientapi/events" - "github.com/matrix-org/dendrite/clientapi/httputil" - "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/clientapi/producers" "github.com/matrix-org/dendrite/common" "github.com/matrix-org/dendrite/common/config" "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/gomatrixserverlib" - - "github.com/matrix-org/util" ) // MembershipRequest represents the body of an incoming POST request @@ -66,22 +62,10 @@ type idServerStoreInviteResponse struct { PublicKeys []common.PublicKey `json:"public_keys"` } -// ProcessIDServerError differentiate errors caused by the absence of the identity -// server in the list of trusted servers in the configuration file from normal -// errors caused by a failure in a process. -// Returns the representation of a 400 error in the former case (without logging), -// and the one of a 500 error in the latter (with logging of the error). -func ProcessIDServerError(req *http.Request, err error) util.JSONResponse { - // Check whether this is an - mErr, ok := err.(*jsonerror.MatrixError) - if !ok || mErr.ErrCode != "M_SERVER_NOT_TRUSTED" { - return httputil.LogThenError(req, err) - } - return util.JSONResponse{ - Code: 400, - JSON: mErr, - } -} +var ( + ErrMissingParameter = errors.New("'address', 'id_server' and 'medium' must all be supplied") + ErrNotTrusted = errors.New("Untrusted server") +) // CheckAndProcessInvite analyses the body of an incoming membership request. // If the fields relative to a third-party-invite are all supplied, lookups the @@ -100,24 +84,21 @@ func CheckAndProcessInvite( req *http.Request, device *authtypes.Device, body *MembershipRequest, cfg config.Dendrite, queryAPI api.RoomserverQueryAPI, db *accounts.Database, producer *producers.RoomserverProducer, membership string, roomID string, -) *util.JSONResponse { +) (inviteStoredOnIDServer bool, err error) { if membership != "invite" || (body.Address == "" && body.IDServer == "" && body.Medium == "") { // If none of the 3PID-specific fields are supplied, it's a standard invite // so return nil for it to be processed as such - return nil + return } else if body.Address == "" || body.IDServer == "" || body.Medium == "" { // If at least one of the 3PID-specific fields is supplied but not all // of them, return an error - return &util.JSONResponse{ - Code: 400, - JSON: jsonerror.BadJSON("'address', 'id_server' and 'medium' must all be supplied"), - } + err = ErrMissingParameter + return } lookupRes, storeInviteRes, err := queryIDServer(req, db, cfg, device, body, roomID) if err != nil { - resErr := ProcessIDServerError(req, err) - return &resErr + return } if lookupRes.MXID == "" { @@ -125,28 +106,16 @@ func CheckAndProcessInvite( // "m.room.third_party_invite" have to be emitted from the data in // storeInviteRes. err = emit3PIDInviteEvent(body, storeInviteRes, device, roomID, cfg, queryAPI, producer) - if err == events.ErrRoomNoExists { - return &util.JSONResponse{ - Code: 404, - JSON: jsonerror.NotFound(err.Error()), - } - } else if err != nil { - resErr := httputil.LogThenError(req, err) - return &resErr - } + inviteStoredOnIDServer = err == nil - // If everything went well, returns with an empty response. - return &util.JSONResponse{ - Code: 200, - JSON: struct{}{}, - } + return } // A Matrix ID have been found: set it in the body request and let the process // continue to create a "m.room.member" event with an "invite" membership body.UserID = lookupRes.MXID - return nil + return } // queryIDServer handles all the requests to the identity server, starting by @@ -163,7 +132,7 @@ func queryIDServer( device *authtypes.Device, body *MembershipRequest, roomID string, ) (lookupRes *idServerLookupResponse, storeInviteRes *idServerStoreInviteResponse, err error) { if err = isTrusted(body.IDServer, cfg); err != nil { - return nil, nil, err + return } // Lookup the 3PID @@ -270,7 +239,6 @@ func queryIDServerStoreInvite( } if resp.StatusCode != http.StatusOK { - // TODO: Log the error supplied with the identity server? errMsg := fmt.Sprintf("Identity server %s responded with a %d error code", body.IDServer, resp.StatusCode) return nil, errors.New(errMsg) } @@ -296,7 +264,6 @@ func queryIDServerPubKey(idServerName string, keyID string) ([]byte, error) { } if resp.StatusCode != http.StatusOK { - // TODO: Log the error supplied with the identity server? errMsg := fmt.Sprintf("Couldn't retrieve key %s from server %s", keyID, idServerName) return nil, errors.New(errMsg) } @@ -318,7 +285,6 @@ func checkIDServerSignatures(body *MembershipRequest, res *idServerLookupRespons return err } - // TODO: Check if the domain is part of a list of trusted ID servers signatures, ok := res.Signatures[body.IDServer] if !ok { return errors.New("No signature for domain " + body.IDServer) diff --git a/src/github.com/matrix-org/dendrite/clientapi/threepid/threepid.go b/src/github.com/matrix-org/dendrite/clientapi/threepid/threepid.go index 8e6485d0c..f07a3a144 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/threepid/threepid.go +++ b/src/github.com/matrix-org/dendrite/clientapi/threepid/threepid.go @@ -23,7 +23,6 @@ import ( "strconv" "strings" - "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/common/config" ) @@ -175,5 +174,5 @@ func isTrusted(idServer string, cfg config.Dendrite) error { return nil } } - return jsonerror.NotTrusted(idServer) + return ErrNotTrusted } diff --git a/src/github.com/matrix-org/dendrite/clientapi/writers/membership.go b/src/github.com/matrix-org/dendrite/clientapi/writers/membership.go index 27404c03c..7a1ad10bb 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/writers/membership.go +++ b/src/github.com/matrix-org/dendrite/clientapi/writers/membership.go @@ -47,10 +47,36 @@ func SendMembership( return *reqErr } - if res := threepid.CheckAndProcessInvite( + inviteStored, err := threepid.CheckAndProcessInvite( req, device, &body, cfg, queryAPI, accountDB, producer, membership, roomID, - ); res != nil { - return *res + ) + if err == threepid.ErrMissingParameter { + return util.JSONResponse{ + Code: 400, + JSON: jsonerror.BadJSON(err.Error()), + } + } else if err == threepid.ErrNotTrusted { + return util.JSONResponse{ + Code: 400, + JSON: jsonerror.NotTrusted(body.IDServer), + } + } else if err == events.ErrRoomNoExists { + return util.JSONResponse{ + Code: 404, + JSON: jsonerror.NotFound(err.Error()), + } + } else if err != nil { + return httputil.LogThenError(req, err) + } + + // If an invite has been stored on an identity server, it means that a + // m.room.third_party_invite event has been emitted and that we shouldn't + // emit a m.room.member one. + if inviteStored { + return util.JSONResponse{ + Code: 200, + JSON: struct{}{}, + } } event, err := buildMembershipEvent(