Tweaking security expressions in RecipeServiceImpl.

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);
+        Recipe recipe = this.createTestRecipe(owner);
-        this.setPrincipal(owner);
+        recipe = this.recipeService.setPublic(recipe, true);
-        final Recipe byId = this.recipeService.getById(testRecipe.getId());
+        final Recipe byId = this.recipeService.getById(recipe.getId());
-        assertThat(byId.getId(), is(testRecipe.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() {
+    public void getByIdThrowsWhenNotViewer() {
-        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 {
         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");
+        final User viewer = this.createTestUser("viewer");
-        this.setPrincipal(principal);
         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(zeroStars, containsRecipes(r0, r1, r2));
-        assertThat(oneStar, Matchers.containsRecipes(r1, r2));
+        assertThat(oneStar, containsRecipes(r1, r2));
-        assertThat(twoStars, Matchers.containsRecipes(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<Recipe> zeroStarsNoneViewable = this.recipeService.getByMinimumStars(0, viewer);
+        final List<Recipe> oneStarNoneViewable = this.recipeService.getByMinimumStars(1, viewer);
+        final List<Recipe> 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<Recipe> zeroStarsViewable = this.recipeService.getByMinimumStars(0, viewer);
+        final List<Recipe> oneStarViewable = this.recipeService.getByMinimumStars(1, viewer);
+        final List<Recipe> 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<Recipe> 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<Recipe> 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<Recipe> 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()));
     }
 

src/main/java/app/mealsmadeeasy/api/recipe/RecipeRepository.java

@@ -14,8 +14,11 @@ public interface RecipeRepository extends JpaRepository<RecipeEntity, Long> {
     List<RecipeEntity> findAllByViewersContaining(UserEntity viewer);
     List<RecipeEntity> findAllByOwner(UserEntity owner);
 
-    @Query("SELECT r FROM Recipe r WHERE size(r.stars) >= ?1")
+    @Query("SELECT r FROM Recipe r WHERE size(r.stars) >= ?1 AND r.isPublic")
-    List<RecipeEntity> findAllByStarsGreaterThanEqual(long stars);
+    List<RecipeEntity> findAllPublicByStarsGreaterThanEqual(long stars);
+
+    @Query("SELECT r FROM Recipe r WHERE size(r.stars) >= ?1 AND (r.isPublic OR ?2 MEMBER OF r.viewers)")
+    List<RecipeEntity> findAllViewableByStarsGreaterThanEqual(long stars, UserEntity viewer);
 
     @Query("SELECT r FROM Recipe r WHERE r.id = ?1")
     @EntityGraph(attributePaths = { "viewers" })

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<Recipe> getByMinimumStars(long minimumStars);
+    List<Recipe> getByMinimumStars(long minimumStars, User viewer);
+
     List<Recipe> getPublicRecipes();
     List<Recipe> getRecipesViewableBy(User user);
     List<Recipe> getRecipesOwnedBy(User user);

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<Recipe> getByMinimumStars(long minimumStars) {
-        return List.copyOf(this.recipeRepository.findAllByStarsGreaterThanEqual(minimumStars));
+        return List.copyOf(this.recipeRepository.findAllPublicByStarsGreaterThanEqual(minimumStars));
+    }
+
+    @Override
+    public List<Recipe> 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);