Hacks for supporting Riot iOS (#1148)

* Join room body is optional

* Support deprecated login by user/password

* Implement dummy key upload endpoint

* Make a very determinate end to /messages if we hit the create event in back-pagination

* Linting

4 files changed, 83 insertions, 41 deletions

diff --git a/clientapi/routing/joinroom.go b/clientapi/routing/joinroom.go
index e190beef..3871e4d4 100644
--- a/clientapi/routing/joinroom.go
+++ b/clientapi/routing/joinroom.go
@@ -43,9 +43,7 @@ func JoinRoomByIDOrAlias(
 	// If content was provided in the request then incude that
 	// in the request. It'll get used as a part of the membership
 	// event content.
-	if err := httputil.UnmarshalJSONRequest(req, &joinReq.Content); err != nil {
-		return *err
-	}
+	_ = httputil.UnmarshalJSONRequest(req, &joinReq.Content)
 
 	// Work out our localpart for the client profile request.
 	localpart, _, err := gomatrixserverlib.SplitID('@', device.UserID)
diff --git a/clientapi/routing/login.go b/clientapi/routing/login.go
index 1b894a56..dc0180da 100644
--- a/clientapi/routing/login.go
+++ b/clientapi/routing/login.go
@@ -47,6 +47,7 @@ type loginIdentifier struct {
 
 type passwordRequest struct {
 	Identifier loginIdentifier `json:"identifier"`
+	User       string          `json:"user"` // deprecated in favour of identifier
 	Password   string          `json:"password"`
 	// Both DeviceID and InitialDisplayName can be omitted, or empty strings ("")
 	// Thus a pointer is needed to differentiate between the two
@@ -81,6 +82,7 @@ func Login(
 	} else if req.Method == http.MethodPost {
 		var r passwordRequest
 		var acc *api.Account
+		var errJSON *util.JSONResponse
 		resErr := httputil.UnmarshalJSONRequest(req, &r)
 		if resErr != nil {
 			return *resErr
@@ -93,31 +95,23 @@ func Login(
 					JSON: jsonerror.BadJSON("'user' must be supplied."),
 				}
 			}
-
-			util.GetLogger(req.Context()).WithField("user", r.Identifier.User).Info("Processing login request")
-
-			localpart, err := userutil.ParseUsernameParam(r.Identifier.User, &cfg.Matrix.ServerName)
-			if err != nil {
-				return util.JSONResponse{
-					Code: http.StatusBadRequest,
-					JSON: jsonerror.InvalidUsername(err.Error()),
-				}
+			acc, errJSON = r.processUsernamePasswordLoginRequest(req, accountDB, cfg, r.Identifier.User)
+			if errJSON != nil {
+				return *errJSON
 			}
-
-			acc, err = accountDB.GetAccountByPassword(req.Context(), localpart, r.Password)
-			if err != nil {
-				// Technically we could tell them if the user does not exist by checking if err == sql.ErrNoRows
-				// but that would leak the existence of the user.
+		default:
+			// TODO: The below behaviour is deprecated but without it Riot iOS won't log in
+			if r.User != "" {
+				acc, errJSON = r.processUsernamePasswordLoginRequest(req, accountDB, cfg, r.User)
+				if errJSON != nil {
+					return *errJSON
+				}
+			} else {
 				return util.JSONResponse{
-					Code: http.StatusForbidden,
-					JSON: jsonerror.Forbidden("username or password was incorrect, or the account does not exist"),
+					Code: http.StatusBadRequest,
+					JSON: jsonerror.BadJSON("login identifier '" + r.Identifier.Type + "' not supported"),
 				}
 			}
-		default:
-			return util.JSONResponse{
-				Code: http.StatusBadRequest,
-				JSON: jsonerror.BadJSON("login identifier '" + r.Identifier.Type + "' not supported"),
-			}
 		}
 
 		token, err := auth.GenerateAccessToken()
@@ -163,3 +157,32 @@ func getDevice(
 	)
 	return
 }
+
+func (r *passwordRequest) processUsernamePasswordLoginRequest(
+	req *http.Request, accountDB accounts.Database,
+	cfg *config.Dendrite, username string,
+) (acc *api.Account, errJSON *util.JSONResponse) {
+	util.GetLogger(req.Context()).WithField("user", username).Info("Processing login request")
+
+	localpart, err := userutil.ParseUsernameParam(username, &cfg.Matrix.ServerName)
+	if err != nil {
+		errJSON = &util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: jsonerror.InvalidUsername(err.Error()),
+		}
+		return
+	}
+
+	acc, err = accountDB.GetAccountByPassword(req.Context(), localpart, r.Password)
+	if err != nil {
+		// Technically we could tell them if the user does not exist by checking if err == sql.ErrNoRows
+		// but that would leak the existence of the user.
+		errJSON = &util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: jsonerror.Forbidden("username or password was incorrect, or the account does not exist"),
+		}
+		return
+	}
+
+	return
+}
diff --git a/keyserver/routing/routing.go b/keyserver/routing/routing.go
index c09031d8..dba43528 100644
--- a/keyserver/routing/routing.go
+++ b/keyserver/routing/routing.go
@@ -36,9 +36,19 @@ func Setup(
 	publicAPIMux *mux.Router, cfg *config.Dendrite, userAPI userapi.UserInternalAPI,
 ) {
 	r0mux := publicAPIMux.PathPrefix(pathPrefixR0).Subrouter()
+
 	r0mux.Handle("/keys/query",
 		httputil.MakeAuthAPI("queryKeys", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
 			return QueryKeys(req)
 		}),
 	).Methods(http.MethodPost, http.MethodOptions)
+
+	r0mux.Handle("/keys/upload/{keyID}",
+		httputil.MakeAuthAPI("keys_upload", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
+			return util.JSONResponse{
+				Code: 200,
+				JSON: map[string]interface{}{},
+			}
+		}),
+	).Methods(http.MethodPost, http.MethodOptions)
 }
diff --git a/syncapi/routing/messages.go b/syncapi/routing/messages.go
index de5429db..15add1b4 100644
--- a/syncapi/routing/messages.go
+++ b/syncapi/routing/messages.go
@@ -158,6 +158,7 @@ func OnIncomingMessagesRequest(
 		util.GetLogger(req.Context()).WithError(err).Error("mreq.retrieveEvents failed")
 		return jsonerror.InternalServerError()
 	}
+
 	util.GetLogger(req.Context()).WithFields(logrus.Fields{
 		"from":         from.String(),
 		"to":           to.String(),
@@ -246,6 +247,12 @@ func (r *messagesReq) retrieveEvents() (
 	// change the way topological positions are defined (as depth isn't the most
 	// reliable way to define it), it would be easier and less troublesome to
 	// only have to change it in one place, i.e. the database.
+	start, end, err = r.getStartEnd(events)
+
+	return clientEvents, start, end, err
+}
+
+func (r *messagesReq) getStartEnd(events []gomatrixserverlib.HeaderedEvent) (start, end types.TopologyToken, err error) {
 	start, err = r.db.EventPositionInTopology(
 		r.ctx, events[0].EventID(),
 	)
@@ -253,24 +260,28 @@ func (r *messagesReq) retrieveEvents() (
 		err = fmt.Errorf("EventPositionInTopology: for start event %s: %w", events[0].EventID(), err)
 		return
 	}
-	end, err = r.db.EventPositionInTopology(
-		r.ctx, events[len(events)-1].EventID(),
-	)
-	if err != nil {
-		err = fmt.Errorf("EventPositionInTopology: for end event %s: %w", events[len(events)-1].EventID(), err)
-		return
-	}
-
-	if r.backwardOrdering {
-		// A stream/topological position is a cursor located between two events.
-		// While they are identified in the code by the event on their right (if
-		// we consider a left to right chronological order), tokens need to refer
-		// to them by the event on their left, therefore we need to decrement the
-		// end position we send in the response if we're going backward.
-		end.Decrement()
+	if r.backwardOrdering && events[len(events)-1].Type() == gomatrixserverlib.MRoomCreate {
+		// We've hit the beginning of the room so there's really nowhere else
+		// to go. This seems to fix Riot iOS from looping on /messages endlessly.
+		end = types.NewTopologyToken(0, 0)
+	} else {
+		end, err = r.db.EventPositionInTopology(
+			r.ctx, events[len(events)-1].EventID(),
+		)
+		if err != nil {
+			err = fmt.Errorf("EventPositionInTopology: for end event %s: %w", events[len(events)-1].EventID(), err)
+			return
+		}
+		if r.backwardOrdering {
+			// A stream/topological position is a cursor located between two events.
+			// While they are identified in the code by the event on their right (if
+			// we consider a left to right chronological order), tokens need to refer
+			// to them by the event on their left, therefore we need to decrement the
+			// end position we send in the response if we're going backward.
+			end.Decrement()
+		}
 	}
-
-	return clientEvents, start, end, err
+	return
 }
 
 // handleEmptyEventsSlice handles the case where the initial request to the