Use standard errors instead of JSON responses in threepid

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)
 		}
 	}
 

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)

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
 }

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(