From 1e772af67f4f0c3fd6c4ff09a6fa0e0eab076a7c Mon Sep 17 00:00:00 2001 From: JesseBrault0709 <62299747+JesseBrault0709@users.noreply.github.com> Date: Mon, 8 Jul 2024 13:00:57 +0200 Subject: [PATCH] Starting to use Spring Security/SpEL on RecipeService methods. --- .../api/recipe/RecipeServiceTests.java | 98 +++++++++++++++++-- .../api/recipe/RecipeSecurity.java | 8 ++ .../api/recipe/RecipeSecurityImpl.java | 37 +++++++ .../api/recipe/RecipeService.java | 3 +- .../api/recipe/RecipeServiceImpl.java | 43 ++------ .../api/security/SecurityConfiguration.java | 2 + 6 files changed, 145 insertions(+), 46 deletions(-) create mode 100644 src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurity.java create mode 100644 src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurityImpl.java diff --git a/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java b/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java index 8b9dd1f..06e2ec8 100644 --- a/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java +++ b/src/integrationTest/java/app/mealsmadeeasy/api/recipe/RecipeServiceTests.java @@ -8,6 +8,9 @@ import app.mealsmadeeasy.api.user.UserRepository; 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; @@ -16,6 +19,7 @@ import static app.mealsmadeeasy.api.matchers.Matchers.*; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.*; @SpringBootTest public class RecipeServiceTests { @@ -38,6 +42,10 @@ 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 { @@ -61,13 +69,45 @@ public class RecipeServiceTests { @Test @DirtiesContext public void simpleGetById() throws RecipeException { - final Recipe testRecipe = this.createTestRecipe(this.createTestUser("recipeOwner")); + 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())); assertThat(byId.getTitle(), is("My Recipe")); assertThat(byId.getRawText(), is("Hello!")); } + @Test + @DirtiesContext + public void getByIdThrowsWhenPrincipalNotViewer() { + 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 { @@ -78,6 +118,36 @@ public class RecipeServiceTests { assertThat(byIdWithStars.getStars(), containsStars(star)); } + @Test + @DirtiesContext + public void getByIdWithStarsThrowsWhenPrincipalNotViewer() { + 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())); + } + + @Test + @DirtiesContext + public void getByIdWithStarsOkayWhenPublicRecipeNoPrincipal() { + final User owner = this.createTestUser("recipeOwner"); + final Recipe notYetPublicRecipe = this.createTestRecipe(owner); + final Recipe publicRecipe = this.recipeService.setPublic(notYetPublicRecipe, true); + assertDoesNotThrow(() -> this.recipeService.getByIdWithStars(publicRecipe.getId())); + } + + @Test + @DirtiesContext + public void getByIdWithStarsOkayWhenPublicRecipeWithPrincipal() { + 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.getByIdWithStars(publicRecipe.getId())); + } + @Test @DirtiesContext public void getByMinimumStars() throws RecipeException { @@ -142,7 +212,7 @@ public class RecipeServiceTests { @Test @DirtiesContext - public void getRecipesOwnedByUser() throws RecipeException { + public void getRecipesOwnedByUser() { final User owner = this.createTestUser("recipeOwner"); final Recipe r0 = this.createTestRecipe(owner); final List ownedRecipes = this.recipeService.getRecipesOwnedBy(owner); @@ -175,22 +245,22 @@ public class RecipeServiceTests { @Test @DirtiesContext - public void updateOwnerViaUsername() throws RecipeException { + public void updateOwnerViaUser() throws RecipeException { final User firstOwner = this.createTestUser("firstOwner"); final User secondOwner = this.createTestUser("secondOwner"); Recipe recipe = this.createTestRecipe(firstOwner); - recipe = this.recipeService.updateOwner(recipe, secondOwner.getUsername()); + recipe = this.recipeService.updateOwner(recipe, firstOwner, secondOwner); assertThat(recipe.getOwner(), isUser(secondOwner)); } @Test @DirtiesContext - public void updateOwnerViaUser() throws RecipeException { - final User firstOwner = this.createTestUser("firstOwner"); - final User secondOwner = this.createTestUser("secondOwner"); - Recipe recipe = this.createTestRecipe(firstOwner); - recipe = this.recipeService.updateOwner(recipe, secondOwner); - assertThat(recipe.getOwner(), isUser(secondOwner)); + public void updateOwnerViaUserThrowsIfNotOwner() { + final User actualOwner = this.createTestUser("u0"); + final User notOwner = this.createTestUser("u1"); + final User target = this.createTestUser("u2"); + final Recipe recipe = this.createTestRecipe(actualOwner); + assertThrows(AccessDeniedException.class, () -> this.recipeService.updateOwner(recipe, notOwner, target)); } @Test @@ -205,6 +275,14 @@ public class RecipeServiceTests { assertThat(star.getOwner(), isUser(starer)); } + @Test + @DirtiesContext + public void addStarWhenNotViewableThrows() { + final User notViewer = this.createTestUser("notViewer"); + final Recipe recipe = this.createTestRecipe(this.createTestUser("recipeOwner")); + assertThrows(AccessDeniedException.class, () -> this.recipeService.addStar(recipe, notViewer)); + } + @Test @DirtiesContext public void deleteStar() throws RecipeException { diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurity.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurity.java new file mode 100644 index 0000000..b24353a --- /dev/null +++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurity.java @@ -0,0 +1,8 @@ +package app.mealsmadeeasy.api.recipe; + +import app.mealsmadeeasy.api.user.User; + +public interface RecipeSecurity { + boolean isOwner(Recipe recipe, User user); + 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 new file mode 100644 index 0000000..d5e81d3 --- /dev/null +++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeSecurityImpl.java @@ -0,0 +1,37 @@ +package app.mealsmadeeasy.api.recipe; + +import app.mealsmadeeasy.api.user.User; +import org.springframework.stereotype.Component; + +import java.util.Objects; + +@Component("recipeSecurity") +public class RecipeSecurityImpl implements RecipeSecurity { + + private final RecipeRepository recipeRepository; + + public RecipeSecurityImpl(RecipeRepository recipeRepository) { + this.recipeRepository = recipeRepository; + } + + @Override + public boolean isOwner(Recipe recipe, User user) { + return recipe.getOwner() != null && recipe.getOwner().getId().equals(user.getId()); + } + + @Override + public boolean isViewableBy(Recipe recipe, User user) { + if (Objects.equals(recipe.getOwner().getId(), user.getId())) { + return true; + } else { + final RecipeEntity withViewers = this.recipeRepository.getByIdWithViewers(recipe.getId()); + for (final User viewer : withViewers.getViewers()) { + if (viewer.getId() != null && viewer.getId().equals(user.getId())) { + return true; + } + } + } + return false; + } + +} diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java index bdb9d76..ae574d5 100644 --- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java +++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeService.java @@ -23,8 +23,7 @@ public interface RecipeService { Recipe updateRawText(Recipe recipe, String newRawText); - Recipe updateOwner(Recipe recipe, String newOwnerUsername) throws RecipeException; - Recipe updateOwner(Recipe recipe, User newOwner) throws RecipeException; + Recipe updateOwner(Recipe recipe, User oldOwner, User newOwner) throws RecipeException; RecipeStar addStar(Recipe recipe, User giver) throws RecipeException; void deleteStarByUser(Recipe recipe, User giver) throws RecipeException; diff --git a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java index c395519..25a622c 100644 --- a/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java +++ b/src/main/java/app/mealsmadeeasy/api/recipe/RecipeServiceImpl.java @@ -13,15 +13,16 @@ import org.commonmark.parser.Parser; import org.commonmark.renderer.html.HtmlRenderer; import org.jsoup.Jsoup; import org.jsoup.safety.Safelist; +import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Service; import java.util.HashSet; import java.util.List; -import java.util.Objects; import java.util.Set; @Service -public final class RecipeServiceImpl implements RecipeService { +public class RecipeServiceImpl implements RecipeService { private static String renderAndCleanMarkdown(String rawText) { final var parser = Parser.builder().build(); @@ -72,6 +73,7 @@ public final class RecipeServiceImpl implements RecipeService { } @Override + @PostAuthorize("returnObject.isPublic || @recipeSecurity.isViewableBy(returnObject, principal)") public Recipe getById(long id) throws RecipeException { return this.recipeRepository.findById(id).orElseThrow(() -> new RecipeException( RecipeException.Type.INVALID_ID, @@ -80,6 +82,7 @@ public final class RecipeServiceImpl implements RecipeService { } @Override + @PostAuthorize("returnObject.isPublic || @recipeSecurity.isViewableBy(returnObject, principal)") public Recipe getByIdWithStars(long id) throws RecipeException { return this.recipeRepository.findByIdWithStars(id).orElseThrow(() -> new RecipeException( RecipeException.Type.INVALID_ID, @@ -126,44 +129,16 @@ public final class RecipeServiceImpl implements RecipeService { } @Override - public Recipe updateOwner(Recipe recipe, String newOwnerUsername) throws RecipeException { - final RecipeEntity entity = (RecipeEntity) recipe; - final UserEntity newOwner = this.userRepository.findByUsername(newOwnerUsername) - .orElseThrow(() -> new RecipeException( - RecipeException.Type.INVALID_OWNER_USERNAME, - "No such username: " + newOwnerUsername - )); - entity.setOwner(newOwner); - return this.recipeRepository.save(entity); - } - - @Override - public Recipe updateOwner(Recipe recipe, User newOwner) throws RecipeException { + @PreAuthorize("@recipeSecurity.isOwner(#recipe, #oldOwner)") + public Recipe updateOwner(Recipe recipe, User oldOwner, User newOwner) { final RecipeEntity entity = (RecipeEntity) recipe; entity.setOwner((UserEntity) newOwner); return this.recipeRepository.save(entity); } @Override - public RecipeStar addStar(Recipe recipe, User giver) throws RecipeException { - boolean viewable = false; - if (recipe.isPublic() || Objects.equals(recipe.getOwner().getId(), giver.getId())) { - viewable = true; - } else { - final RecipeEntity withViewers = this.recipeRepository.getByIdWithViewers(recipe.getId()); - for (final var viewer : withViewers.getViewers()) { - if (viewer.getId() != null && viewer.getId().equals(giver.getId())) { - viewable = true; - break; - } - } - } - if (!viewable) { - throw new RecipeException( - RecipeException.Type.NOT_VIEWABLE, - "Recipe with id " + recipe.getId() + " is not viewable by User with id " + giver.getId() - ); - } + @PreAuthorize("@recipeSecurity.isViewableBy(#recipe, #giver)") + public RecipeStar addStar(Recipe recipe, User giver) { final RecipeStarEntity star = new RecipeStarEntity(); star.setOwner((UserEntity) giver); star.setRecipe((RecipeEntity) recipe); diff --git a/src/main/java/app/mealsmadeeasy/api/security/SecurityConfiguration.java b/src/main/java/app/mealsmadeeasy/api/security/SecurityConfiguration.java index 686bbaa..29d0403 100644 --- a/src/main/java/app/mealsmadeeasy/api/security/SecurityConfiguration.java +++ b/src/main/java/app/mealsmadeeasy/api/security/SecurityConfiguration.java @@ -8,6 +8,7 @@ import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.ProviderManager; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; import org.springframework.security.config.Customizer; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityCustomizer; @@ -20,6 +21,7 @@ import org.springframework.security.web.authentication.UsernamePasswordAuthentic @Configuration @EnableWebSecurity +@EnableMethodSecurity public class SecurityConfiguration { private final JpaUserDetailsService jpaUserDetailsService;