Resolved comments on PR; changes in design

clientapi/routing/room_tagging.go

@@ -23,6 +23,8 @@ import (
 	"github.com/matrix-org/gomatrix"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
+	"github.com/matrix-org/dendrite/clientapi/jsonerror"
+	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 )
 
 // newTag creates and returns a new Tag type
@@ -36,14 +38,21 @@ func newTag() gomatrix.TagContent {
 func GetTag(
 	req *http.Request,
 	accountDB *accounts.Database,
+	device *authtypes.Device,
 	userID string,
 	roomID string,
 ) util.JSONResponse {
-	Tag := newTag()
+
+	if device.UserID != userID {
+		return util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: jsonerror.Forbidden("Cannot set another user's typing state"),
+		}
+	}
 
 	localpart, _, err := gomatrixserverlib.SplitID('@', userID)
 	if err != nil {
-		httputil.LogThenError(req, err)
+		return httputil.LogThenError(req, err)
 	}
 
 	data, err := accountDB.GetAccountDataByType(
@@ -51,17 +60,19 @@ func GetTag(
 	)
 
 	if err != nil {
-		httputil.LogThenError(req, err)
+		return httputil.LogThenError(req, err)
 	}
 
 	dataByte, err := json.Marshal(data)
 	if err != nil {
-		httputil.LogThenError(req, err)
+		return httputil.LogThenError(req, err)
 	}
+
+	Tag := newTag()
 	err = json.Unmarshal(dataByte, &Tag)
 
 	if err != nil {
-		httputil.LogThenError(req, err)
+		return httputil.LogThenError(req, err)
 	}
 
 	return util.JSONResponse{
@@ -74,10 +85,19 @@ func GetTag(
 func PutTag(
 	req *http.Request,
 	accountDB *accounts.Database,
+	device *authtypes.Device,
 	userID string,
 	roomID string,
 	tag string,
 ) util.JSONResponse {
+
+	if device.UserID != userID {
+		return util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: jsonerror.Forbidden("Cannot set another user's typing state"),
+		}
+	}
+
 	localpart, data, err := obtainSavedTags(req, userID, roomID, accountDB)
 
 	if err != nil {
@@ -93,14 +113,17 @@ func PutTag(
 	if len(data) > 0 {
 		dataByte, err := json.Marshal(data)
 		if err != nil {
-			httputil.LogThenError(req, err)
+			return httputil.LogThenError(req, err)
 		}
 		if err = json.Unmarshal(dataByte, &Tag); err != nil {
 			return httputil.LogThenError(req, err)
 		}
 	}
 	Tag.Tags[tag] = properties
-	addDataToDB(req, localpart, roomID, accountDB, Tag)
+	err = saveTagData(req, localpart, roomID, accountDB, Tag)
+	if err != nil {
+		return httputil.LogThenError(req, err)
+	}
 
 	return util.JSONResponse{
 		Code: http.StatusOK,
@@ -112,10 +135,19 @@ func PutTag(
 func DeleteTag(
 	req *http.Request,
 	accountDB *accounts.Database,
+	device *authtypes.Device,
 	userID string,
 	roomID string,
 	tag string,
 ) util.JSONResponse {
+
+	if device.UserID != userID {
+		return util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: jsonerror.Forbidden("Cannot set another user's typing state"),
+		}
+	}
+
 	localpart, data, err := obtainSavedTags(req, userID, roomID, accountDB)
 
 	if err != nil {
@@ -123,15 +155,8 @@ func DeleteTag(
 	}
 	Tag := newTag()
 
-	if len(data) > 0 {
+	// If there are no tags in the database, exit.
-		dataByte, err := json.Marshal(data)
+	if len(data) == 0 {
-		if err != nil {
-			httputil.LogThenError(req, err)
-		}
-		if err := json.Unmarshal(dataByte, &Tag); err != nil {
-			return httputil.LogThenError(req, err)
-		}
-	} else {
 		//Synapse returns a 200 OK response on finding no Tags, same policy is followed here.
 		return util.JSONResponse{
 			Code: http.StatusOK,
@@ -139,6 +164,14 @@ func DeleteTag(
 		}
 	}
 
+	dataByte, err := json.Marshal(data)
+	if err != nil {
+		return httputil.LogThenError(req, err)
+	}
+	if err := json.Unmarshal(dataByte, &Tag); err != nil {
+		return httputil.LogThenError(req, err)
+	}
+
 	// Check whether the Tag to be deleted exists
 	if _, ok := Tag.Tags[tag]; ok {
 		delete(Tag.Tags, tag)
@@ -149,7 +182,11 @@ func DeleteTag(
 			JSON: struct{}{},
 		}
 	}
-	addDataToDB(req, localpart, roomID, accountDB, Tag)
+	err = saveTagData(req, localpart, roomID, accountDB, Tag)
+
+	if err != nil {
+		return httputil.LogThenError(req, err)
+	}
 
 	return util.JSONResponse{
 		Code: http.StatusOK,
@@ -176,31 +213,32 @@ func obtainSavedTags(
 		return "", []gomatrixserverlib.RawJSON{}, err
 	}
 
-	return localpart, getContentFromData(data), nil
+	return localpart, extractEventContents(data), nil
 }
 
-// addDataToDB is a utility function to save the tag data into the DB
+// saveTagData is a utility function to save the tag data into the DB
-func addDataToDB(
+func saveTagData(
 	req *http.Request,
 	localpart string,
 	roomID string,
 	accountDB *accounts.Database,
 	Tag gomatrix.TagContent,
-) {
+) error {
 	newTagData, err := json.Marshal(Tag)
 	if err != nil {
-		httputil.LogThenError(req, err)
+		return err
 	}
 	if err = accountDB.SaveAccountData(
 		req.Context(), localpart, roomID, "tag", string(newTagData),
 	); err != nil {
-		httputil.LogThenError(req, err)
+		return err
 	}
+	return nil
 }
 
-// getContentFromData is an utility function to obtain "content" from the ClientEvent
+// extractEventContents is an utility function to obtain "content" from the ClientEvent
-func getContentFromData(data []gomatrixserverlib.ClientEvent) []gomatrixserverlib.RawJSON {
+func extractEventContents(data []gomatrixserverlib.ClientEvent) []gomatrixserverlib.RawJSON {
-	var contentData []gomatrixserverlib.RawJSON
+	contentData := make([]gomatrixserverlib.RawJSON, 0, len(data))
 	for i := 0; i < len(data); i++ {
 		contentData = append(contentData, data[i].Content)
 	}

clientapi/routing/routing.go

@@ -486,22 +486,31 @@ func Setup(
 
 	r0mux.Handle("/user/{userId}/rooms/{roomId}/tags",
 		common.MakeAuthAPI("get_tag", authData, func(req *http.Request, device *authtypes.Device) util.JSONResponse {
-			vars := mux.Vars(req)
+			vars, err := common.URLDecodeMapValues(mux.Vars(req))
-			return GetTag(req, accountDB, vars["userId"], vars["roomId"])
+			if err != nil {
+				return util.ErrorResponse(err)
+			}
+			return GetTag(req, accountDB, device, vars["userId"], vars["roomId"])
 		}),
 	).Methods(http.MethodGet, http.MethodOptions)
 
 	r0mux.Handle("/user/{userId}/rooms/{roomId}/tags/{tag}",
 		common.MakeAuthAPI("put_tag", authData, func(req *http.Request, device *authtypes.Device) util.JSONResponse {
-			vars := mux.Vars(req)
+			vars, err := common.URLDecodeMapValues(mux.Vars(req))
-			return PutTag(req, accountDB, vars["userId"], vars["roomId"], vars["tag"])
+			if err != nil {
+				return util.ErrorResponse(err)
+			}
+			return PutTag(req, accountDB, device, vars["userId"], vars["roomId"], vars["tag"])
 		}),
 	).Methods(http.MethodPut, http.MethodOptions)
 
 	r0mux.Handle("/user/{userId}/rooms/{roomId}/tags/{tag}",
 		common.MakeAuthAPI("delete_tag", authData, func(req *http.Request, device *authtypes.Device) util.JSONResponse {
-			vars := mux.Vars(req)
+			vars, err := common.URLDecodeMapValues(mux.Vars(req))
-			return DeleteTag(req, accountDB, vars["userId"], vars["roomId"], vars["tag"])
+			if err != nil {
+				return util.ErrorResponse(err)
+			}
+			return DeleteTag(req, accountDB, device, vars["userId"], vars["roomId"], vars["tag"])
 		}),
 	).Methods(http.MethodDelete, http.MethodOptions)
 }

go.mod

@@ -22,13 +22,8 @@ require (
 	github.com/klauspost/crc32 v0.0.0-20161016154125-cb6bfca970f6
 	github.com/lib/pq v0.0.0-20170918175043-23da1db4f16d
 	github.com/matrix-org/dugong v0.0.0-20171220115018-ea0a4690a0d5
-<<<<<<< HEAD
 	github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26
-	github.com/matrix-org/gomatrixserverlib v0.0.0-20181109104322-1c2cbc0872f0
-=======
-	github.com/matrix-org/gomatrix v0.0.0-20190130130140-385f072fe9af
 	github.com/matrix-org/gomatrixserverlib v0.0.0-20190619132215-178ed5e3b8e2
->>>>>>> e2251199a49ab0bb846c02ba37e1cd437a7f725b
 	github.com/matrix-org/naffka v0.0.0-20171115094957-662bfd0841d0
 	github.com/matrix-org/util v0.0.0-20171127121716-2e2df66af2f5
 	github.com/matttproud/golang_protobuf_extensions v1.0.1

go.sum

@@ -45,13 +45,10 @@ github.com/matrix-org/dugong v0.0.0-20171220115018-ea0a4690a0d5 h1:nMX2t7hbGF0NY
 github.com/matrix-org/dugong v0.0.0-20171220115018-ea0a4690a0d5/go.mod h1:NgPCr+UavRGH6n5jmdX8DuqFZ4JiCWIJoZiuhTRLSUg=
 github.com/matrix-org/gomatrix v0.0.0-20171003113848-a7fc80c8060c h1:aZap604NyBGhAUE0CyNHz6+Pryye5A5mHnYyO4KPPW8=
 github.com/matrix-org/gomatrix v0.0.0-20171003113848-a7fc80c8060c/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0=
-<<<<<<< HEAD
-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/gomatrix v0.0.0-20190130130140-385f072fe9af h1:piaIBNQGIHnni27xRB7VKkEwoWCgAmeuYf8pxAyG0bI=
 github.com/matrix-org/gomatrix v0.0.0-20190130130140-385f072fe9af/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0=
->>>>>>> e2251199a49ab0bb846c02ba37e1cd437a7f725b
+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-20181109104322-1c2cbc0872f0 h1:3UzhmERBbis4ZaB3imEbZwtDjGz/oVRC2cLLEajCzJA=
 github.com/matrix-org/gomatrixserverlib v0.0.0-20181109104322-1c2cbc0872f0/go.mod h1:YHyhIQUmuXyKtoVfDUMk/DyU93Taamlu6nPZkij/JtA=
 github.com/matrix-org/gomatrixserverlib v0.0.0-20190619132215-178ed5e3b8e2 h1:pYajAEdi3sowj4iSunqctchhcMNW3rDjeeH0T4uDkMY=