Extract read-then-authorize boilerplate into helper methods

handlers/authorize.go

@@ -0,0 +1,119 @@
+package handlers
+
+import (
+	"errors"
+	"net/http"
+
+	"github.com/mtlynch/screenjournal/v2/screenjournal"
+)
+
+var errForbidden = errors.New("forbidden")
+
+func (s Server) isOwnerOrAdmin(r *http.Request, owner screenjournal.Username) bool {
+	return mustGetUsernameFromContext(r.Context()).Equal(owner) || isAdmin(r.Context())
+}
+
+func (s Server) readReview(r *http.Request, id screenjournal.ReviewID) (screenjournal.Review, error) {
+	return s.getDB(r).ReadReview(id)
+}
+
+func (s Server) readComment(r *http.Request, id screenjournal.CommentID) (screenjournal.ReviewComment, error) {
+	return s.getDB(r).ReadComment(id)
+}
+
+func (s Server) readReaction(r *http.Request, id screenjournal.ReactionID) (screenjournal.ReviewReaction, error) {
+	return s.getDB(r).ReadReaction(id)
+}
+
+func (s Server) updateReview(
+	r *http.Request,
+	id screenjournal.ReviewID,
+	updated reviewPutRequest,
+) (screenjournal.Review, error) {
+	review, err := s.readReview(r, id)
+	if err != nil {
+		return screenjournal.Review{}, err
+	}
+	if !s.isOwnerOrAdmin(r, review.Owner) {
+		return screenjournal.Review{}, errForbidden
+	}
+
+	review.Rating = updated.Rating
+	review.Blurb = updated.Blurb
+	review.Watched = updated.Watched
+
+	if err := s.getDB(r).UpdateReview(review); err != nil {
+		return screenjournal.Review{}, err
+	}
+
+	return review, nil
+}
+
+func (s Server) deleteReview(r *http.Request, id screenjournal.ReviewID) error {
+	review, err := s.readReview(r, id)
+	if err != nil {
+		return err
+	}
+	if !s.isOwnerOrAdmin(r, review.Owner) {
+		return errForbidden
+	}
+
+	if err := s.getDB(r).DeleteReview(id); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (s Server) updateComment(
+	r *http.Request,
+	id screenjournal.CommentID,
+	commentText screenjournal.CommentText,
+) (screenjournal.ReviewComment, error) {
+	rc, err := s.readComment(r, id)
+	if err != nil {
+		return screenjournal.ReviewComment{}, err
+	}
+	if !s.isOwnerOrAdmin(r, rc.Owner) {
+		return screenjournal.ReviewComment{}, errForbidden
+	}
+
+	rc.CommentText = commentText
+	if err := s.getDB(r).UpdateComment(rc); err != nil {
+		return screenjournal.ReviewComment{}, err
+	}
+
+	return rc, nil
+}
+
+func (s Server) deleteComment(r *http.Request, id screenjournal.CommentID) error {
+	rc, err := s.readComment(r, id)
+	if err != nil {
+		return err
+	}
+	if !s.isOwnerOrAdmin(r, rc.Owner) {
+		return errForbidden
+	}
+
+	if err := s.getDB(r).DeleteComment(id); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (s Server) deleteReaction(r *http.Request, id screenjournal.ReactionID) error {
+	rr, err := s.readReaction(r, id)
+	if err != nil {
+		return err
+	}
+	if !s.isOwnerOrAdmin(r, rr.Owner) {
+		return errForbidden
+	}
+
+	if err := s.getDB(r).DeleteReaction(id); err != nil {
+		return err
+	}
+
+	return nil
+}

handlers/comments.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"errors"
 	"fmt"
 	"html/template"
 	"log"
@@ -197,23 +198,14 @@ func (s Server) commentsPut() http.HandlerFunc {
 			return
 		}
 
-		rc, err := s.getDB(r).ReadComment(req.CommentID)
+		rc, err := s.updateComment(r, req.CommentID, req.CommentText)
 		if err == store.ErrCommentNotFound {
 			http.Error(w, "Comment not found", http.StatusNotFound)
 			return
-		} else if err != nil {
-			log.Printf("failed to read comment: %v", err)
-			http.Error(w, fmt.Sprintf("Failed to read comment: %v", err), http.StatusInternalServerError)
-			return
-		}
-
-		if !mustGetUsernameFromContext(r.Context()).Equal(rc.Owner) {
+		} else if errors.Is(err, errForbidden) {
 			http.Error(w, "Can't edit another user's comment", http.StatusForbidden)
 			return
-		}
-
-		rc.CommentText = req.CommentText
-		if err := s.getDB(r).UpdateComment(rc); err != nil {
+		} else if err != nil {
 			log.Printf("failed to update comment: %v", err)
 			http.Error(w, fmt.Sprintf("Failed to update comment: %v", err), http.StatusInternalServerError)
 			return
@@ -241,22 +233,13 @@ func (s Server) commentsDelete() http.HandlerFunc {
 			return
 		}
 
-		rc, err := s.getDB(r).ReadComment(cid)
-		if err == store.ErrCommentNotFound {
+		if err := s.deleteComment(r, cid); err == store.ErrCommentNotFound {
 			http.Error(w, "Comment not found", http.StatusNotFound)
 			return
-		} else if err != nil {
-			log.Printf("failed to read comment: %v", err)
-			http.Error(w, fmt.Sprintf("Failed to read comment: %v", err), http.StatusInternalServerError)
-			return
-		}
-
-		if !mustGetUsernameFromContext(r.Context()).Equal(rc.Owner) {
+		} else if errors.Is(err, errForbidden) {
 			http.Error(w, "Can't delete another user's comment", http.StatusForbidden)
 			return
-		}
-
-		if err := s.getDB(r).DeleteComment(cid); err != nil {
+		} else if err != nil {
 			log.Printf("failed to delete comment id=%v: %v", cid, err)
 			http.Error(w, "Failed to delete comment: %v", http.StatusInternalServerError)
 			return

handlers/comments_test.go

@@ -403,6 +403,40 @@ func TestCommentsPut(t *testing.T) {
 			},
 			status: http.StatusForbidden,
 		},
+		{
+			description:  "allows an admin to update another user's comment",
+			route:        "/api/comments/1",
+			payload:      "comment=Admin%20updated%20this%20comment",
+			sessionToken: "adm123",
+			sessions: []mockSessionEntry{
+				makeCommentsTestData().sessions.userA,
+				makeCommentsTestData().sessions.userB,
+				{
+					token: "adm123",
+					session: sessions.Session{
+						Username: screenjournal.Username("admin"),
+						IsAdmin:  true,
+					},
+				},
+			},
+			comments: []screenjournal.ReviewComment{
+				{
+					ID:          screenjournal.CommentID(1),
+					Owner:       makeCommentsTestData().sessions.userA.session.Username,
+					CommentText: screenjournal.CommentText("Good insights!"),
+					Review:      makeCommentsTestData().reviews.userBTheWaterBoy,
+				},
+			},
+			status: http.StatusOK,
+			expectedComments: []screenjournal.ReviewComment{
+				{
+					ID:          screenjournal.CommentID(1),
+					Owner:       makeCommentsTestData().sessions.userA.session.Username,
+					CommentText: screenjournal.CommentText("Admin updated this comment"),
+					Review:      makeCommentsTestData().reviews.userBTheWaterBoy,
+				},
+			},
+		},
 		{
 			description:  "prevents an unauthenticated user from updating any comment",
 			route:        "/api/comments/1",
@@ -573,6 +607,32 @@ func TestCommentsDelete(t *testing.T) {
 			},
 			status: http.StatusForbidden,
 		},
+		{
+			description:  "allows an admin to delete another user's comment",
+			route:        "/api/comments/1",
+			sessionToken: "adm123",
+			sessions: []mockSessionEntry{
+				makeCommentsTestData().sessions.userA,
+				makeCommentsTestData().sessions.userB,
+				{
+					token: "adm123",
+					session: sessions.Session{
+						Username: screenjournal.Username("admin"),
+						IsAdmin:  true,
+					},
+				},
+			},
+			comments: []screenjournal.ReviewComment{
+				{
+					ID:          screenjournal.CommentID(1),
+					Owner:       makeCommentsTestData().sessions.userA.session.Username,
+					CommentText: screenjournal.CommentText("Good insights!"),
+					Review:      makeCommentsTestData().reviews.userBTheWaterBoy,
+				},
+			},
+			status:           http.StatusNoContent,
+			expectedComments: []screenjournal.ReviewComment{},
+		},
 		{
 			description:  "prevents an unauthenticated user from deleting any comment",
 			route:        "/api/comments/1",

handlers/reactions.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"errors"
 	"fmt"
 	"html/template"
 	"log"
@@ -119,23 +120,13 @@ func (s Server) reactionsDelete() http.HandlerFunc {
 			return
 		}
 
-		rr, err := s.getDB(r).ReadReaction(rid)
-		if err == store.ErrReactionNotFound {
+		if err := s.deleteReaction(r, rid); err == store.ErrReactionNotFound {
 			http.Error(w, "Reaction not found", http.StatusNotFound)
 			return
-		} else if err != nil {
-			log.Printf("failed to read reaction: %v", err)
-			http.Error(w, fmt.Sprintf("Failed to read reaction: %v", err), http.StatusInternalServerError)
-			return
-		}
-
-		loggedInUsername := mustGetUsernameFromContext(r.Context())
-		if !loggedInUsername.Equal(rr.Owner) && !isAdmin(r.Context()) {
+		} else if errors.Is(err, errForbidden) {
 			http.Error(w, "Can't delete another user's reaction", http.StatusForbidden)
 			return
-		}
-
-		if err := s.getDB(r).DeleteReaction(rid); err != nil {
+		} else if err != nil {
 			log.Printf("failed to delete reaction id=%v: %v", rid, err)
 			http.Error(w, "Failed to delete reaction", http.StatusInternalServerError)
 			return

handlers/reviews.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"net/http"
@@ -91,32 +92,20 @@ func (s Server) reviewsPut() http.HandlerFunc {
 			return
 		}
 
-		review, err := s.getDB(r).ReadReview(id)
-		if err == store.ErrReviewNotFound {
-			http.Error(w, "Review not found", http.StatusNotFound)
-			return
-		} else if err != nil {
-			http.Error(w, fmt.Sprintf("Failed to read review: %v", err), http.StatusInternalServerError)
-			return
-		}
-
-		loggedInUsername := mustGetUsernameFromContext(r.Context())
-		if !review.Owner.Equal(loggedInUsername) {
-			http.Error(w, "You can't edit another user's review", http.StatusForbidden)
-			return
-		}
-
 		parsedRequest, err := parseReviewPutRequest(r)
 		if err != nil {
 			http.Error(w, fmt.Sprintf("Invalid request: %v", err), http.StatusBadRequest)
 			return
 		}
 
-		review.Rating = parsedRequest.Rating
-		review.Blurb = parsedRequest.Blurb
-		review.Watched = parsedRequest.Watched
-
-		if err := s.getDB(r).UpdateReview(review); err != nil {
+		review, err := s.updateReview(r, id, parsedRequest)
+		if err == store.ErrReviewNotFound {
+			http.Error(w, "Review not found", http.StatusNotFound)
+			return
+		} else if errors.Is(err, errForbidden) {
+			http.Error(w, "You can't edit another user's review", http.StatusForbidden)
+			return
+		} else if err != nil {
 			log.Printf("failed to update review: %v", err)
 			http.Error(w, fmt.Sprintf("Failed to update review: %v", err), http.StatusInternalServerError)
 			return
@@ -140,22 +129,13 @@ func (s Server) reviewsDelete() http.HandlerFunc {
 			return
 		}
 
-		review, err := s.getDB(r).ReadReview(id)
-		if err == store.ErrReviewNotFound {
+		if err := s.deleteReview(r, id); err == store.ErrReviewNotFound {
 			http.Error(w, "Review not found", http.StatusNotFound)
 			return
-		} else if err != nil {
-			http.Error(w, fmt.Sprintf("Failed to read review: %v", err), http.StatusInternalServerError)
-			return
-		}
-
-		loggedInUsername := mustGetUsernameFromContext(r.Context())
-		if !review.Owner.Equal(loggedInUsername) {
+		} else if errors.Is(err, errForbidden) {
 			http.Error(w, "You can't delete another user's review", http.StatusForbidden)
 			return
-		}
-
-		if err := s.getDB(r).DeleteReview(id); err != nil {
+		} else if err != nil {
 			log.Printf("failed to delete review: %v", err)
 			http.Error(w, fmt.Sprintf("Failed to delete review: %v", err), http.StatusInternalServerError)
 			return