From 35ef2aa039843eb442f7eb29a3c381d36eb8399e Mon Sep 17 00:00:00 2001
From: JesseBrault0709 <62299747+JesseBrault0709@users.noreply.github.com>
Date: Tue, 9 Jul 2024 09:24:36 +0200
Subject: [PATCH] More RecipeService security and tests.

---
 .../api/recipe/RecipeServiceTests.java        | 65 +++++++++++++++----
 .../api/recipe/RecipeSecurity.java            |  1 +
 .../api/recipe/RecipeSecurityImpl.java        |  9 +++
 .../api/recipe/RecipeService.java             | 10 +--
 .../api/recipe/RecipeServiceImpl.java         | 15 +++--
 5 files changed, 77 insertions(+), 23 deletions(-)

diff --git a/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java b/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java
index f8096ca..517ba3f 100644
--- a/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java
+++ b/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java
@@ -64,7 +64,7 @@ public class RecipeServiceTests {
     public void getByIdPublic() throws RecipeException {
         final User owner = this.createTestUser("recipeOwner");
         Recipe recipe = this.createTestRecipe(owner);
-        recipe = this.recipeService.setPublic(recipe, true);
+        recipe = this.recipeService.setPublic(recipe, owner, true);
         final Recipe byId = this.recipeService.getById(recipe.getId());
         assertThat(byId.getId(), is(recipe.getId()));
         assertThat(byId.getTitle(), is("My Recipe"));
@@ -93,7 +93,7 @@ public class RecipeServiceTests {
     public void getByIdOkayWhenPublic() {
         final User owner = this.createTestUser("recipeOwner");
         final Recipe notYetPublicRecipe = this.createTestRecipe(owner);
-        final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true);
+        final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, owner, true);
         assertDoesNotThrow(() -> this.recipeService.getById(publicRecipe.getId()));
     }
 
@@ -103,7 +103,7 @@ public class RecipeServiceTests {
         final User owner = this.createTestUser("recipeOwner");
         final User viewer = this.createTestUser("viewer");
         final Recipe notYetPublicRecipe = this.createTestRecipe(owner);
-        final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true);
+        final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, owner, true);
         assertDoesNotThrow(() -> this.recipeService.getById(publicRecipe.getId(), viewer));
     }
 
@@ -112,7 +112,7 @@ public class RecipeServiceTests {
     public void getByIdWithStarsPublic() throws RecipeException {
         final User owner = this.createTestUser("recipeOwner");
         Recipe recipe = this.createTestRecipe(owner);
-        recipe = this.recipeService.setPublic(recipe, true);
+        recipe = this.recipeService.setPublic(recipe, owner, true);
         final RecipeStar star = this.recipeService.addStar(recipe, owner);
         final Recipe byIdWithStars = this.recipeService.getByIdWithStars(recipe.getId());
         assertThat(byIdWithStars.getStars(), containsStars(star));
@@ -132,7 +132,7 @@ public class RecipeServiceTests {
     public void getByIdWithStarsOkayWhenPublic() {
         final User owner = this.createTestUser("recipeOwner");
         final Recipe notYetPublicRecipe = this.createTestRecipe(owner);
-        final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true);
+        final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, owner, true);
         assertDoesNotThrow(() -> this.recipeService.getByIdWithStars(publicRecipe.getId()));
     }
 
@@ -142,7 +142,7 @@ public class RecipeServiceTests {
         final User owner = this.createTestUser("recipeOwner");
         final User viewer = this.createTestUser("viewer");
         final Recipe notYetPublicRecipe = this.createTestRecipe(owner);
-        final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true);
+        final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, owner, true);
         assertDoesNotThrow(() -> this.recipeService.getByIdWithStars(publicRecipe.getId(), viewer));
     }
 
@@ -157,9 +157,9 @@ public class RecipeServiceTests {
         Recipe r1 = this.createTestRecipe(owner);
         Recipe r2 = this.createTestRecipe(owner);
 
-        r0 = this.recipeService.setPublic(r0, true);
-        r1 = this.recipeService.setPublic(r1, true);
-        r2 = this.recipeService.setPublic(r2, true);
+        r0 = this.recipeService.setPublic(r0, owner, true);
+        r1 = this.recipeService.setPublic(r1, owner, true);
+        r2 = this.recipeService.setPublic(r2, owner, true);
 
         // r0.stars = 0, r1.stars = 1, r2.stars = 2
         this.recipeService.addStar(r1, u0);
@@ -236,8 +236,8 @@ public class RecipeServiceTests {
         Recipe r0 = this.createTestRecipe(owner);
         Recipe r1 = this.createTestRecipe(owner);
 
-        r0 = this.recipeService.setPublic(r0, true);
-        r1 = this.recipeService.setPublic(r1, true);
+        r0 = this.recipeService.setPublic(r0, owner, true);
+        r1 = this.recipeService.setPublic(r1, owner, true);
 
         final List<Recipe> publicRecipes = this.recipeService.getPublicRecipes();
         assertThat(publicRecipes.size(), is(2));
@@ -274,10 +274,19 @@ public class RecipeServiceTests {
         final Recipe recipe = this.recipeService.create(
                 owner, "My Recipe", "# A Heading"
         );
-        final String rendered = this.recipeService.getRenderedMarkdown(recipe);
+        final String rendered = this.recipeService.getRenderedMarkdown(recipe, owner);
         assertThat(rendered, is("<h1>A Heading</h1>"));
     }
 
+    @Test
+    @DirtiesContext
+    public void getRenderedMarkThrowsIfNotViewable() {
+        final User owner = this.createTestUser("recipeOwner");
+        final User notViewer = this.createTestUser("notViewer");
+        final Recipe recipe = this.createTestRecipe(owner);
+        assertThrows(AccessDeniedException.class, () -> this.recipeService.getRenderedMarkdown(recipe, notViewer));
+    }
+
     @Test
     @DirtiesContext
     public void updateRawText() {
@@ -286,10 +295,22 @@ public class RecipeServiceTests {
                 owner, "My Recipe", "# A Heading"
         );
         final String newRawText = "# A Heading\n## A Subheading";
-        recipe = this.recipeService.updateRawText(recipe, newRawText);
+        recipe = this.recipeService.updateRawText(recipe, owner, newRawText);
         assertThat(recipe.getRawText(), is(newRawText));
     }
 
+    @Test
+    @DirtiesContext
+    public void updateRawTextThrowsIfNotOwner() {
+        final User owner = this.createTestUser("recipeOwner");
+        final User notOwner = this.createTestUser("notOwner");
+        final Recipe recipe = this.createTestRecipe(owner);
+        assertThrows(
+                AccessDeniedException.class,
+                () -> this.recipeService.updateRawText(recipe, notOwner, "should fail")
+        );
+    }
+
     @Test
     @DirtiesContext
     public void updateOwnerViaUser() throws RecipeException {
@@ -343,4 +364,22 @@ public class RecipeServiceTests {
         assertThat(recipe.getStars(), is(empty()));
     }
 
+    @Test
+    @DirtiesContext
+    public void deleteRecipe() {
+        final User owner = this.createTestUser("recipeOwner");
+        final Recipe toDelete = this.createTestRecipe(owner);
+        this.recipeService.deleteRecipe(toDelete, owner);
+        assertThrows(RecipeException.class, () -> this.recipeService.getById(toDelete.getId(), owner));
+    }
+
+    @Test
+    @DirtiesContext
+    public void deleteRecipeThrowsIfNotOwner() {
+        final User owner = this.createTestUser("recipeOwner");
+        final User notOwner = this.createTestUser("notOwner");
+        final Recipe toDelete = this.createTestRecipe(owner);
+        assertThrows(AccessDeniedException.class, () -> this.recipeService.deleteRecipe(toDelete, notOwner));
+    }
+
 }
diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurity.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurity.java
index b24353a..52eedc8 100644
--- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurity.java
+++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurity.java
@@ -4,5 +4,6 @@ import app.mealsmadeeasy.api.user.User;
 
 public interface RecipeSecurity {
     boolean isOwner(Recipe recipe, User user);
+    boolean isOwner(long recipeId, User user) throws RecipeException;
     boolean isViewableBy(Recipe recipe, User user);
 }
diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurityImpl.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurityImpl.java
index d5e81d3..ea9a9c3 100644
--- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurityImpl.java
+++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurityImpl.java
@@ -19,6 +19,15 @@ public class RecipeSecurityImpl implements RecipeSecurity {
         return recipe.getOwner() != null && recipe.getOwner().getId().equals(user.getId());
     }
 
+    @Override
+    public boolean isOwner(long recipeId, User user) throws RecipeException {
+        final Recipe recipe = this.recipeRepository.findById(recipeId).orElseThrow(() -> new RecipeException(
+                RecipeException.Type.INVALID_ID,
+                "No such Recipe with id " + recipeId
+        ));
+        return this.isOwner(recipe, user);
+    }
+
     @Override
     public boolean isViewableBy(Recipe recipe, User user) {
         if (Objects.equals(recipe.getOwner().getId(), user.getId())) {
diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java
index de9fdff..fbedba8 100644
--- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java
+++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java
@@ -24,9 +24,9 @@ public interface RecipeService {
     List<Recipe> getRecipesViewableBy(User user);
     List<Recipe> getRecipesOwnedBy(User user);
 
-    String getRenderedMarkdown(Recipe recipe);
+    String getRenderedMarkdown(Recipe recipe, User viewer);
 
-    Recipe updateRawText(Recipe recipe, String newRawText);
+    Recipe updateRawText(Recipe recipe, User owner, String newRawText);
 
     Recipe updateOwner(Recipe recipe, User oldOwner, User newOwner) throws RecipeException;
 
@@ -34,7 +34,7 @@ public interface RecipeService {
     void deleteStarByUser(Recipe recipe, User giver) throws RecipeException;
     void deleteStar(RecipeStar recipeStar);
 
-    Recipe setPublic(Recipe recipe, boolean isPublic);
+    Recipe setPublic(Recipe recipe, User owner, boolean isPublic);
 
     Recipe addViewer(Recipe recipe, User user);
     Recipe removeViewer(Recipe recipe, User user);
@@ -47,7 +47,7 @@ public interface RecipeService {
     void deleteComment(RecipeComment comment);
     Recipe clearComments(Recipe recipe);
 
-    void deleteRecipe(Recipe recipe);
-    void deleteById(long id);
+    void deleteRecipe(Recipe recipe, User owner);
+    void deleteById(long id, User owner);
 
 }
diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java
index 73d2c99..e9aa255 100644
--- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java
+++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java
@@ -136,7 +136,8 @@ public class RecipeServiceImpl implements RecipeService {
     }
 
     @Override
-    public String getRenderedMarkdown(Recipe recipe) {
+    @PreAuthorize("#recipe.isPublic || @recipeSecurity.isViewableBy(#recipe, #viewer)")
+    public String getRenderedMarkdown(Recipe recipe, User viewer) {
         RecipeEntity entity = (RecipeEntity) recipe;
         if (entity.getCachedRenderedText() == null) {
             entity.setCachedRenderedText(renderAndCleanMarkdown(entity.getRawText()));
@@ -146,7 +147,8 @@ public class RecipeServiceImpl implements RecipeService {
     }
 
     @Override
-    public Recipe updateRawText(Recipe recipe, String newRawText) {
+    @PreAuthorize("@recipeSecurity.isOwner(#recipe, #owner)")
+    public Recipe updateRawText(Recipe recipe, User owner, String newRawText) {
         final RecipeEntity entity = (RecipeEntity) recipe;
         entity.setCachedRenderedText(null);
         entity.setRawText(newRawText);
@@ -188,7 +190,8 @@ public class RecipeServiceImpl implements RecipeService {
     }
 
     @Override
-    public Recipe setPublic(Recipe recipe, boolean isPublic) {
+    @PreAuthorize("@recipeSecurity.isOwner(#recipe, #owner)")
+    public Recipe setPublic(Recipe recipe, User owner, boolean isPublic) {
         final RecipeEntity entity = (RecipeEntity) recipe;
         entity.setPublic(isPublic);
         return this.recipeRepository.save(entity);
@@ -266,12 +269,14 @@ public class RecipeServiceImpl implements RecipeService {
     }
 
     @Override
-    public void deleteRecipe(Recipe recipe) {
+    @PreAuthorize("@recipeSecurity.isOwner(#recipe, #owner)")
+    public void deleteRecipe(Recipe recipe, User owner) {
         this.recipeRepository.delete((RecipeEntity) recipe);
     }
 
     @Override
-    public void deleteById(long id) {
+    @PreAuthorize("@recipeSecurity.isOwner(#id, #owner)")
+    public void deleteById(long id, User owner) {
         this.recipeRepository.deleteById(id);
     }