diff --git a/handlers/authorize.go b/handlers/authorize.go new file mode 100644 index 00000000..c4edbd93 --- /dev/null +++ b/handlers/authorize.go @@ -0,0 +1,117 @@ +package handlers + +import ( + "errors" + "net/http" + + "github.com/mtlynch/screenjournal/v2/screenjournal" +) + +var errForbidden = errors.New("forbidden") + +type dbService struct { + Store + request *http.Request +} + +func (d dbService) isOwnerOrAdmin(owner screenjournal.Username) bool { + return mustGetUsernameFromContext(d.request.Context()).Equal(owner) || + isAdmin(d.request.Context()) +} + +func (d dbService) readReview(id screenjournal.ReviewID) (screenjournal.Review, error) { + return d.ReadReview(id) +} + +func (d dbService) readComment(id screenjournal.CommentID) (screenjournal.ReviewComment, error) { + return d.ReadComment(id) +} + +func (d dbService) readReaction(id screenjournal.ReactionID) (screenjournal.ReviewReaction, error) { + return d.ReadReaction(id) +} + +func (d dbService) updateReview(id screenjournal.ReviewID, updated reviewPutRequest) (screenjournal.Review, error) { + review, err := d.readReview(id) + if err != nil { + return screenjournal.Review{}, err + } + if !d.isOwnerOrAdmin(review.Owner) { + return screenjournal.Review{}, errForbidden + } + + review.Rating = updated.Rating + review.Blurb = updated.Blurb + review.Watched = updated.Watched + + if err := d.UpdateReview(review); err != nil { + return screenjournal.Review{}, err + } + + return review, nil +} + +func (d dbService) deleteReview(id screenjournal.ReviewID) error { + review, err := d.readReview(id) + if err != nil { + return err + } + if !d.isOwnerOrAdmin(review.Owner) { + return errForbidden + } + + if err := d.DeleteReview(id); err != nil { + return err + } + + return nil +} + +func (d dbService) updateComment(id screenjournal.CommentID, commentText screenjournal.CommentText) (screenjournal.ReviewComment, error) { + rc, err := d.readComment(id) + if err != nil { + return screenjournal.ReviewComment{}, err + } + if !d.isOwnerOrAdmin(rc.Owner) { + return screenjournal.ReviewComment{}, errForbidden + } + + rc.CommentText = commentText + if err := d.UpdateComment(rc); err != nil { + return screenjournal.ReviewComment{}, err + } + + return rc, nil +} + +func (d dbService) deleteComment(id screenjournal.CommentID) error { + rc, err := d.readComment(id) + if err != nil { + return err + } + if !d.isOwnerOrAdmin(rc.Owner) { + return errForbidden + } + + if err := d.DeleteComment(id); err != nil { + return err + } + + return nil +} + +func (d dbService) deleteReaction(id screenjournal.ReactionID) error { + rr, err := d.readReaction(id) + if err != nil { + return err + } + if !d.isOwnerOrAdmin(rr.Owner) { + return errForbidden + } + + if err := d.DeleteReaction(id); err != nil { + return err + } + + return nil +} diff --git a/handlers/comments.go b/handlers/comments.go index 26433307..d8312471 100644 --- a/handlers/comments.go +++ b/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.getDB(r).updateComment(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.getDB(r).deleteComment(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 diff --git a/handlers/comments_test.go b/handlers/comments_test.go index 67640fdc..bb667e5a 100644 --- a/handlers/comments_test.go +++ b/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", diff --git a/handlers/db.go b/handlers/db.go new file mode 100644 index 00000000..c12d2ac6 --- /dev/null +++ b/handlers/db.go @@ -0,0 +1,11 @@ +package handlers + +import "net/http" + +func (s Server) getDB(r *http.Request) dbService { + return s.db.dbForRequest(r) +} + +func (s Server) getAuthenticator(r *http.Request) Authenticator { + return s.db.authenticatorForRequest(r, s.authenticator) +} diff --git a/handlers/db_dev.go b/handlers/db_dev.go index 490fa27c..ed4e10b0 100644 --- a/handlers/db_dev.go +++ b/handlers/db_dev.go @@ -195,6 +195,16 @@ var sharedDBSettings = dbSettings{ tokenToDB: map[dbToken]Store{}, } +type sessionDBProvider struct { + store Store +} + +func newDBProvider(store Store) dbProvider { + return sessionDBProvider{ + store: store, + } +} + func (dbs *dbSettings) IsSessionIsolationEnabled() bool { dbs.lock.RLock() dbs.lock.RUnlock() @@ -220,9 +230,9 @@ func (dbs *dbSettings) SaveDB(token dbToken, db Store) { dbs.tokenToDB[token] = db } -func (s Server) getDB(r *http.Request) Store { +func (p sessionDBProvider) rawDB(r *http.Request) Store { if !sharedDBSettings.IsSessionIsolationEnabled() { - return s.store + return p.store } c, err := r.Cookie(dbTokenCookieName) if err != nil { @@ -231,11 +241,18 @@ func (s Server) getDB(r *http.Request) Store { return sharedDBSettings.GetDB(dbToken(c.Value)) } -func (s Server) getAuthenticator(r *http.Request) Authenticator { +func (p sessionDBProvider) dbForRequest(r *http.Request) dbService { + return dbService{ + Store: p.rawDB(r), + request: r, + } +} + +func (p sessionDBProvider) authenticatorForRequest(r *http.Request, fallback Authenticator) Authenticator { if !sharedDBSettings.IsSessionIsolationEnabled() { - return s.authenticator + return fallback } - return auth.New(s.getDB(r)) + return auth.New(p.rawDB(r)) } func dbPerSessionPost() http.HandlerFunc { diff --git a/handlers/db_prod.go b/handlers/db_prod.go index 1e04a4c0..5ad931ca 100644 --- a/handlers/db_prod.go +++ b/handlers/db_prod.go @@ -10,10 +10,23 @@ func (s *Server) addDevRoutes() { // no-op } -func (s Server) getDB(*http.Request) Store { - return s.store +type staticDBProvider struct { + store Store } -func (s Server) getAuthenticator(_ *http.Request) Authenticator { - return s.authenticator +func newDBProvider(store Store) dbProvider { + return staticDBProvider{ + store: store, + } +} + +func (p staticDBProvider) dbForRequest(r *http.Request) dbService { + return dbService{ + Store: p.store, + request: r, + } +} + +func (p staticDBProvider) authenticatorForRequest(_ *http.Request, fallback Authenticator) Authenticator { + return fallback } diff --git a/handlers/reactions.go b/handlers/reactions.go index 3d075b0f..52d88e8a 100644 --- a/handlers/reactions.go +++ b/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.getDB(r).deleteReaction(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 diff --git a/handlers/reviews.go b/handlers/reviews.go index 274515fa..fb938609 100644 --- a/handlers/reviews.go +++ b/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.getDB(r).updateReview(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.getDB(r).deleteReview(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 diff --git a/handlers/reviews_test.go b/handlers/reviews_test.go index 22272062..5ada91f1 100644 --- a/handlers/reviews_test.go +++ b/handlers/reviews_test.go @@ -844,6 +844,66 @@ func TestReviewsPut(t *testing.T) { sessionToken: "def456", expectedStatus: http.StatusForbidden, }, + { + description: "allows an admin to overwrite another user's review", + localMovies: []screenjournal.Movie{ + { + TmdbID: screenjournal.TmdbID(38), + ImdbID: screenjournal.ImdbID("tt0338013"), + Title: screenjournal.MediaTitle("Eternal Sunshine of the Spotless Mind"), + ReleaseDate: screenjournal.ReleaseDate(mustParseDate("2004-03-19")), + }, + }, + priorReviews: []screenjournal.Review{ + { + ID: screenjournal.ReviewID(1), + Owner: screenjournal.Username("userA"), + Rating: screenjournal.NewRating(5), + Watched: mustParseWatchDate("2022-10-28"), + Blurb: screenjournal.Blurb("It's my favorite movie!"), + Movie: screenjournal.Movie{ + ID: screenjournal.MovieID(1), + TmdbID: screenjournal.TmdbID(38), + ImdbID: screenjournal.ImdbID("tt0338013"), + Title: screenjournal.MediaTitle("Eternal Sunshine of the Spotless Mind"), + ReleaseDate: screenjournal.ReleaseDate(mustParseDate("2004-03-19")), + }, + }, + }, + sessions: []mockSessionEntry{ + { + token: "abc123", + session: sessions.Session{ + Username: screenjournal.Username("userA"), + }, + }, + { + token: "adm123", + session: sessions.Session{ + Username: screenjournal.Username("admin"), + IsAdmin: true, + }, + }, + }, + route: "/reviews/1", + payload: "rating=4&watch-date=2022-10-30&blurb=Admin%20updated%20this%20review", + sessionToken: "adm123", + expectedStatus: http.StatusSeeOther, + expected: screenjournal.Review{ + Owner: screenjournal.Username("userA"), + Rating: screenjournal.NewRating(4), + Watched: mustParseWatchDate("2022-10-30"), + Blurb: screenjournal.Blurb("Admin updated this review"), + Movie: screenjournal.Movie{ + ID: screenjournal.MovieID(1), + TmdbID: screenjournal.TmdbID(38), + ImdbID: screenjournal.ImdbID("tt0338013"), + Title: screenjournal.MediaTitle("Eternal Sunshine of the Spotless Mind"), + ReleaseDate: screenjournal.ReleaseDate(mustParseDate("2004-03-19")), + }, + Comments: []screenjournal.ReviewComment{}, + }, + }, } { t.Run(tt.description, func(t *testing.T) { dataStore := test_sqlite.New() @@ -913,6 +973,129 @@ func TestReviewsPut(t *testing.T) { } } +func TestReviewsDelete(t *testing.T) { + for _, tt := range []struct { + description string + sessionToken string + sessions []mockSessionEntry + expectedStatus int + expectedReviewsCount int + }{ + { + description: "allows an admin to delete another user's review", + sessionToken: "adm123", + sessions: []mockSessionEntry{ + { + token: "abc123", + session: sessions.Session{ + Username: screenjournal.Username("userA"), + }, + }, + { + token: "adm123", + session: sessions.Session{ + Username: screenjournal.Username("admin"), + IsAdmin: true, + }, + }, + }, + expectedStatus: http.StatusSeeOther, + expectedReviewsCount: 0, + }, + { + description: "prevents a non-admin user from deleting another user's review", + sessionToken: "def456", + sessions: []mockSessionEntry{ + { + token: "abc123", + session: sessions.Session{ + Username: screenjournal.Username("userA"), + }, + }, + { + token: "def456", + session: sessions.Session{ + Username: screenjournal.Username("userB"), + }, + }, + }, + expectedStatus: http.StatusForbidden, + expectedReviewsCount: 1, + }, + } { + t.Run(tt.description, func(t *testing.T) { + dataStore := test_sqlite.New() + + for _, s := range tt.sessions { + mockUser := screenjournal.User{ + Username: s.session.Username, + Email: screenjournal.Email(s.session.Username.String() + "@example.com"), + PasswordHash: screenjournal.PasswordHash("dummy-password-hash"), + } + if err := dataStore.InsertUser(mockUser); err != nil { + t.Fatalf("failed to insert mock user: %+v: %v", mockUser, err) + } + } + + movie := screenjournal.Movie{ + TmdbID: screenjournal.TmdbID(38), + ImdbID: screenjournal.ImdbID("tt0338013"), + Title: screenjournal.MediaTitle("Eternal Sunshine of the Spotless Mind"), + ReleaseDate: screenjournal.ReleaseDate(mustParseDate("2004-03-19")), + } + if _, err := dataStore.InsertMovie(movie); err != nil { + t.Fatalf("failed to insert mock movie %+v: %v", movie, err) + } + + review := screenjournal.Review{ + ID: screenjournal.ReviewID(1), + Owner: screenjournal.Username("userA"), + Rating: screenjournal.NewRating(5), + Watched: mustParseWatchDate("2022-10-28"), + Blurb: screenjournal.Blurb("It's my favorite movie!"), + Movie: screenjournal.Movie{ + ID: screenjournal.MovieID(1), + TmdbID: screenjournal.TmdbID(38), + ImdbID: screenjournal.ImdbID("tt0338013"), + Title: screenjournal.MediaTitle("Eternal Sunshine of the Spotless Mind"), + ReleaseDate: screenjournal.ReleaseDate(mustParseDate("2004-03-19")), + }, + } + if _, err := dataStore.InsertReview(review); err != nil { + t.Fatalf("failed to insert mock review %+v: %v", review, err) + } + + sessionManager := newMockSessionManager(tt.sessions) + s := handlers.New(nilAuthenticator, nilAnnouncer, &sessionManager, dataStore, mockMetadataFinder{}) + + req, err := http.NewRequest("DELETE", "/reviews/1", strings.NewReader("")) + if err != nil { + t.Fatal(err) + } + req.AddCookie(&http.Cookie{ + Name: mockSessionTokenName, + Value: tt.sessionToken, + }) + + rec := httptest.NewRecorder() + s.Router().ServeHTTP(rec, req) + res := rec.Result() + + if got, want := res.StatusCode, tt.expectedStatus; got != want { + t.Fatalf("status=%d, want=%d", got, want) + } + + reviews, err := dataStore.ReadReviews() + if err != nil { + t.Fatalf("failed to read reviews: %v", err) + } + if got, want := len(reviews), tt.expectedReviewsCount; got != want { + t.Fatalf("reviewCount=%d, want=%d", got, want) + } + }) + } +} + func clearUnpredictableReviewProperties(r *screenjournal.Review) { r.ID = screenjournal.ReviewID(0) r.Created = time.Time{} diff --git a/handlers/server.go b/handlers/server.go index 926911ce..2a549ac3 100644 --- a/handlers/server.go +++ b/handlers/server.go @@ -38,12 +38,17 @@ type ( GetTvShow(id screenjournal.TmdbID) (screenjournal.TvShow, error) } + dbProvider interface { + dbForRequest(*http.Request) dbService + authenticatorForRequest(*http.Request, Authenticator) Authenticator + } + Server struct { router *mux.Router authenticator Authenticator announcer Announcer sessionManager SessionManager - store Store + db dbProvider metadataFinder MetadataFinder } ) @@ -61,7 +66,7 @@ func New(authenticator Authenticator, announcer Announcer, sessionManager Sessio authenticator: authenticator, announcer: announcer, sessionManager: sessionManager, - store: store, + db: newDBProvider(store), metadataFinder: metadataFinder, } diff --git a/handlers/store.go b/handlers/store.go index 76f7a7bc..bf0cc4ba 100644 --- a/handlers/store.go +++ b/handlers/store.go @@ -37,11 +37,6 @@ type Store interface { ReadSignupInvitation(screenjournal.InviteCode) (screenjournal.SignupInvitation, error) ReadSignupInvitations() ([]screenjournal.SignupInvitation, error) DeleteSignupInvitation(screenjournal.InviteCode) error - ReadReviewSubscribers() ([]screenjournal.EmailSubscriber, error) - ReadCommentSubscribers( - screenjournal.ReviewID, - screenjournal.Username, - ) ([]screenjournal.EmailSubscriber, error) ReadNotificationPreferences(screenjournal.Username) (screenjournal.NotificationPreferences, error) UpdateNotificationPreferences(screenjournal.Username, screenjournal.NotificationPreferences) error InsertPasswordResetEntry(screenjournal.PasswordResetEntry) error