From d3a532fb12c33c9dfaf75bf085d86d5259ddf7a9 Mon Sep 17 00:00:00 2001 From: JesseBrault0709 <62299747+JesseBrault0709@users.noreply.github.com> Date: Mon, 8 Jul 2024 18:50:22 +0200 Subject: [PATCH] Tweaking security expressions in RecipeServiceImpl. --- .../api/recipe/RecipeServiceTests.java | 151 ++++++++++++------ .../api/recipe/RecipeRepository.java | 7 +- .../api/recipe/RecipeService.java | 5 + .../api/recipe/RecipeServiceImpl.java | 33 +++- 4 files changed, 138 insertions(+), 58 deletions(-) diff --git a/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java b/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java index 06e2ec8..f8096ca 100644 --- a/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java +++ b/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java @@ -1,6 +1,5 @@ package app.mealsmadeeasy.api.recipe; -import app.mealsmadeeasy.api.matchers.Matchers; import app.mealsmadeeasy.api.recipe.star.RecipeStar; import app.mealsmadeeasy.api.user.User; import app.mealsmadeeasy.api.user.UserEntity; @@ -9,8 +8,6 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.security.access.AccessDeniedException; -import org.springframework.security.authentication.TestingAuthenticationToken; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.test.annotation.DirtiesContext; import java.util.List; @@ -42,10 +39,6 @@ public class RecipeServiceTests { return this.recipeService.create(owner, "My Recipe" , "Hello!"); } - private void setPrincipal(User principal) { - SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken(principal, null)); - } - @Test @DirtiesContext public void createViaUsername() throws RecipeException { @@ -68,51 +61,58 @@ public class RecipeServiceTests { @Test @DirtiesContext - public void simpleGetById() throws RecipeException { + public void getByIdPublic() throws RecipeException { final User owner = this.createTestUser("recipeOwner"); - final Recipe testRecipe = this.createTestRecipe(owner); - this.setPrincipal(owner); - final Recipe byId = this.recipeService.getById(testRecipe.getId()); - assertThat(byId.getId(), is(testRecipe.getId())); + Recipe recipe = this.createTestRecipe(owner); + recipe = this.recipeService.setPublic(recipe, true); + final Recipe byId = this.recipeService.getById(recipe.getId()); + assertThat(byId.getId(), is(recipe.getId())); assertThat(byId.getTitle(), is("My Recipe")); assertThat(byId.getRawText(), is("Hello!")); } @Test @DirtiesContext - public void getByIdThrowsWhenPrincipalNotViewer() { + public void getByIdThrowsWhenNotPublic() { final User owner = this.createTestUser("recipeOwner"); - final User notViewer = this.createTestUser("notViewer"); - this.setPrincipal(notViewer); final Recipe recipe = this.createTestRecipe(owner); assertThrows(AccessDeniedException.class, () -> this.recipeService.getById(recipe.getId())); } @Test @DirtiesContext - public void getByIdOkayWhenPublicRecipeNoPrincipal() { - final User owner = this.createTestUser("recipeOwner"); - final Recipe notYetPublicRecipe = this.createTestRecipe(owner); - final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true); - assertDoesNotThrow(() -> this.recipeService.getById(publicRecipe.getId())); - } - - @Test - @DirtiesContext - public void getByIdOkayWhenPublicRecipeWithPrincipal() { - final User owner = this.createTestUser("recipeOwner"); - final User principal = this.createTestUser("principal"); - this.setPrincipal(principal); - final Recipe notYetPublicRecipe = this.createTestRecipe(owner); - final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true); - assertDoesNotThrow(() -> this.recipeService.getById(publicRecipe.getId())); - } - - @Test - @DirtiesContext - public void getByIdWithStars() throws RecipeException { + public void getByIdThrowsWhenNotViewer() { final User owner = this.createTestUser("recipeOwner"); + final User notViewer = this.createTestUser("notViewer"); final Recipe recipe = this.createTestRecipe(owner); + assertThrows(AccessDeniedException.class, () -> this.recipeService.getById(recipe.getId(), notViewer)); + } + + @Test + @DirtiesContext + public void getByIdOkayWhenPublic() { + final User owner = this.createTestUser("recipeOwner"); + final Recipe notYetPublicRecipe = this.createTestRecipe(owner); + final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true); + assertDoesNotThrow(() -> this.recipeService.getById(publicRecipe.getId())); + } + + @Test + @DirtiesContext + public void getByIdOkayWhenPublicRecipeWithViewer() { + 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); + assertDoesNotThrow(() -> this.recipeService.getById(publicRecipe.getId(), viewer)); + } + + @Test + @DirtiesContext + public void getByIdWithStarsPublic() throws RecipeException { + final User owner = this.createTestUser("recipeOwner"); + Recipe recipe = this.createTestRecipe(owner); + recipe = this.recipeService.setPublic(recipe, true); final RecipeStar star = this.recipeService.addStar(recipe, owner); final Recipe byIdWithStars = this.recipeService.getByIdWithStars(recipe.getId()); assertThat(byIdWithStars.getStars(), containsStars(star)); @@ -120,17 +120,16 @@ public class RecipeServiceTests { @Test @DirtiesContext - public void getByIdWithStarsThrowsWhenPrincipalNotViewer() { + public void getByIdWithStarsThrowsWhenNotViewer() { final User owner = this.createTestUser("recipeOwner"); final User notViewer = this.createTestUser("notViewer"); - this.setPrincipal(notViewer); final Recipe recipe = this.createTestRecipe(owner); - assertThrows(AccessDeniedException.class, () -> this.recipeService.getByIdWithStars(recipe.getId())); + assertThrows(AccessDeniedException.class, () -> this.recipeService.getByIdWithStars(recipe.getId(), notViewer)); } @Test @DirtiesContext - public void getByIdWithStarsOkayWhenPublicRecipeNoPrincipal() { + public void getByIdWithStarsOkayWhenPublic() { final User owner = this.createTestUser("recipeOwner"); final Recipe notYetPublicRecipe = this.createTestRecipe(owner); final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true); @@ -139,18 +138,17 @@ public class RecipeServiceTests { @Test @DirtiesContext - public void getByIdWithStarsOkayWhenPublicRecipeWithPrincipal() { + public void getByIdWithStarsOkayWhenPublicRecipeWithViewer() { final User owner = this.createTestUser("recipeOwner"); - final User principal = this.createTestUser("principal"); - this.setPrincipal(principal); + final User viewer = this.createTestUser("viewer"); final Recipe notYetPublicRecipe = this.createTestRecipe(owner); final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true); - assertDoesNotThrow(() -> this.recipeService.getByIdWithStars(publicRecipe.getId())); + assertDoesNotThrow(() -> this.recipeService.getByIdWithStars(publicRecipe.getId(), viewer)); } @Test @DirtiesContext - public void getByMinimumStars() throws RecipeException { + public void getByMinimumStarsAllPublic() throws RecipeException { final User owner = this.createTestUser("recipeOwner"); final User u0 = this.createTestUser("u0"); final User u1 = this.createTestUser("u1"); @@ -176,9 +174,58 @@ public class RecipeServiceTests { assertThat(oneStar.size(), is(2)); assertThat(twoStars.size(), is(1)); - assertThat(zeroStars, Matchers.containsRecipes(r0, r1, r2)); - assertThat(oneStar, Matchers.containsRecipes(r1, r2)); - assertThat(twoStars, Matchers.containsRecipes(r2)); + assertThat(zeroStars, containsRecipes(r0, r1, r2)); + assertThat(oneStar, containsRecipes(r1, r2)); + assertThat(twoStars, containsRecipes(r2)); + } + + @Test + @DirtiesContext + public void getByMinimumStarsOnlySomeViewable() throws RecipeException { + final User owner = this.createTestUser("recipeOwner"); + final User u0 = this.createTestUser("u0"); + final User u1 = this.createTestUser("u1"); + final User viewer = this.createTestUser("recipeViewer"); + + Recipe r0 = this.createTestRecipe(owner); + Recipe r1 = this.createTestRecipe(owner); + Recipe r2 = this.createTestRecipe(owner); + + for (final User starer : List.of(u0, u1)) { + r0 = this.recipeService.addViewer(r0, starer); + r1 = this.recipeService.addViewer(r1, starer); + r2 = this.recipeService.addViewer(r2, starer); + } + + // r0.stars = 0, r1.stars = 1, r2.stars = 2 + this.recipeService.addStar(r1, u0); + this.recipeService.addStar(r2, u0); + this.recipeService.addStar(r2, u1); + + final List zeroStarsNoneViewable = this.recipeService.getByMinimumStars(0, viewer); + final List oneStarNoneViewable = this.recipeService.getByMinimumStars(1, viewer); + final List twoStarsNoneViewable = this.recipeService.getByMinimumStars(2, viewer); + + assertThat(zeroStarsNoneViewable.size(), is(0)); + assertThat(oneStarNoneViewable.size(), is(0)); + assertThat(twoStarsNoneViewable.size(), is(0)); + + // Now make them viewable + r0 = this.recipeService.addViewer(r0, viewer); + r1 = this.recipeService.addViewer(r1, viewer); + r2 = this.recipeService.addViewer(r2, viewer); + + final List zeroStarsViewable = this.recipeService.getByMinimumStars(0, viewer); + final List oneStarViewable = this.recipeService.getByMinimumStars(1, viewer); + final List twoStarsViewable = this.recipeService.getByMinimumStars(2, viewer); + + assertThat(zeroStarsViewable.size(), is(3)); + assertThat(oneStarViewable.size(), is(2)); + assertThat(twoStarsViewable.size(), is (1)); + + assertThat(zeroStarsViewable, containsRecipes(r0, r1, r2)); + assertThat(oneStarViewable, containsRecipes(r1, r2)); + assertThat(twoStarsViewable, containsRecipes(r2)); } @Test @@ -194,7 +241,7 @@ public class RecipeServiceTests { final List publicRecipes = this.recipeService.getPublicRecipes(); assertThat(publicRecipes.size(), is(2)); - assertThat(publicRecipes, Matchers.containsRecipes(r0, r1)); + assertThat(publicRecipes, containsRecipes(r0, r1)); } @Test @@ -207,7 +254,7 @@ public class RecipeServiceTests { r0 = this.recipeService.addViewer(r0, viewer); final List viewableRecipes = this.recipeService.getRecipesViewableBy(viewer); assertThat(viewableRecipes.size(), is(1)); - assertThat(viewableRecipes, Matchers.containsRecipes(r0)); + assertThat(viewableRecipes, containsRecipes(r0)); } @Test @@ -217,7 +264,7 @@ public class RecipeServiceTests { final Recipe r0 = this.createTestRecipe(owner); final List ownedRecipes = this.recipeService.getRecipesOwnedBy(owner); assertThat(ownedRecipes.size(), is(1)); - assertThat(ownedRecipes, Matchers.containsRecipes(r0)); + assertThat(ownedRecipes, containsRecipes(r0)); } @Test @@ -292,7 +339,7 @@ public class RecipeServiceTests { recipe = this.recipeService.addViewer(recipe, starer); final RecipeStar star = this.recipeService.addStar(recipe, starer); this.recipeService.deleteStar(star); - recipe = this.recipeService.getByIdWithStars(recipe.getId()); + recipe = this.recipeService.getByIdWithStars(recipe.getId(), owner); assertThat(recipe.getStars(), is(empty())); } diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeRepository.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeRepository.java index c5dcd3b..dcfbc8f 100644 --- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeRepository.java +++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeRepository.java @@ -14,8 +14,11 @@ public interface RecipeRepository extends JpaRepository { List findAllByViewersContaining(UserEntity viewer); List findAllByOwner(UserEntity owner); - @Query("SELECT r FROM Recipe r WHERE size(r.stars) >= ?1") - List findAllByStarsGreaterThanEqual(long stars); + @Query("SELECT r FROM Recipe r WHERE size(r.stars) >= ?1 AND r.isPublic") + List findAllPublicByStarsGreaterThanEqual(long stars); + + @Query("SELECT r FROM Recipe r WHERE size(r.stars) >= ?1 AND (r.isPublic OR ?2 MEMBER OF r.viewers)") + List findAllViewableByStarsGreaterThanEqual(long stars, UserEntity viewer); @Query("SELECT r FROM Recipe r WHERE r.id = ?1") @EntityGraph(attributePaths = { "viewers" }) diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java index ae574d5..de9fdff 100644 --- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java +++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java @@ -12,9 +12,14 @@ public interface RecipeService { Recipe create(User user, String title, String rawText); Recipe getById(long id) throws RecipeException; + Recipe getById(long id, User viewer) throws RecipeException; + Recipe getByIdWithStars(long id) throws RecipeException; + Recipe getByIdWithStars(long id, User viewer) throws RecipeException; List getByMinimumStars(long minimumStars); + List getByMinimumStars(long minimumStars, User viewer); + List getPublicRecipes(); List getRecipesViewableBy(User user); List getRecipesOwnedBy(User user); diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java index 25a622c..73d2c99 100644 --- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java +++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java @@ -73,7 +73,7 @@ public class RecipeServiceImpl implements RecipeService { } @Override - @PostAuthorize("returnObject.isPublic || @recipeSecurity.isViewableBy(returnObject, principal)") + @PostAuthorize("returnObject.isPublic") public Recipe getById(long id) throws RecipeException { return this.recipeRepository.findById(id).orElseThrow(() -> new RecipeException( RecipeException.Type.INVALID_ID, @@ -82,7 +82,16 @@ public class RecipeServiceImpl implements RecipeService { } @Override - @PostAuthorize("returnObject.isPublic || @recipeSecurity.isViewableBy(returnObject, principal)") + @PostAuthorize("returnObject.isPublic || @recipeSecurity.isViewableBy(returnObject, #viewer)") + public Recipe getById(long id, User viewer) throws RecipeException { + return this.recipeRepository.findById(id).orElseThrow(() -> new RecipeException( + RecipeException.Type.INVALID_ID, + "No such recipe for id " + id + )); + } + + @Override + @PostAuthorize("returnObject.isPublic") public Recipe getByIdWithStars(long id) throws RecipeException { return this.recipeRepository.findByIdWithStars(id).orElseThrow(() -> new RecipeException( RecipeException.Type.INVALID_ID, @@ -90,9 +99,25 @@ public class RecipeServiceImpl implements RecipeService { )); } + @Override + @PostAuthorize("returnObject.isPublic || @recipeSecurity.isViewableBy(returnObject, #viewer)") + public Recipe getByIdWithStars(long id, User viewer) throws RecipeException { + return this.recipeRepository.findByIdWithStars(id).orElseThrow(() -> new RecipeException( + RecipeException.Type.INVALID_ID, + "No such recipe for id " + id + )); + } + @Override public List getByMinimumStars(long minimumStars) { - return List.copyOf(this.recipeRepository.findAllByStarsGreaterThanEqual(minimumStars)); + return List.copyOf(this.recipeRepository.findAllPublicByStarsGreaterThanEqual(minimumStars)); + } + + @Override + public List getByMinimumStars(long minimumStars, User viewer) { + return List.copyOf( + this.recipeRepository.findAllViewableByStarsGreaterThanEqual(minimumStars, (UserEntity) viewer) + ); } @Override @@ -137,7 +162,7 @@ public class RecipeServiceImpl implements RecipeService { } @Override - @PreAuthorize("@recipeSecurity.isViewableBy(#recipe, #giver)") + @PreAuthorize("#recipe.isPublic || @recipeSecurity.isViewableBy(#recipe, #giver)") public RecipeStar addStar(Recipe recipe, User giver) { final RecipeStarEntity star = new RecipeStarEntity(); star.setOwner((UserEntity) giver);