From e8bb0ac6e395343017abca803e32eb78e1dfc9c2 Mon Sep 17 00:00:00 2001 From: MiniDigger | Martin Date: Sun, 3 Sep 2023 19:10:10 +0200 Subject: [PATCH] allow disabling rate limits + add tests for pages --- .../hangar/config/hangar/HangarConfig.java | 9 ++ .../controller/api/v1/PagesController.java | 10 -- .../internal/api/requests/StringContent.java | 7 + .../ratelimit/RateLimitInterceptor.java | 9 +- backend/src/main/resources/application.yml | 1 + .../api/v1/ApiKeysControllerTest.java | 1 - .../api/v1/PagesControllerTest.java | 130 ++++++++++++++++++ .../controller/api/v1/helper/TestData.java | 9 ++ .../src/test/resources/application-test.yml | 1 + 9 files changed, 165 insertions(+), 12 deletions(-) create mode 100644 backend/src/test/java/io/papermc/hangar/controller/api/v1/PagesControllerTest.java diff --git a/backend/src/main/java/io/papermc/hangar/config/hangar/HangarConfig.java b/backend/src/main/java/io/papermc/hangar/config/hangar/HangarConfig.java index de1acee95..6205ab051 100644 --- a/backend/src/main/java/io/papermc/hangar/config/hangar/HangarConfig.java +++ b/backend/src/main/java/io/papermc/hangar/config/hangar/HangarConfig.java @@ -24,6 +24,7 @@ public class HangarConfig { private List licenses = new ArrayList<>(); private boolean allowIndexing = true; private boolean disableJGroups = false; + private boolean disableRateLimiting = false; @NestedConfigurationProperty public UpdateTasksConfig updateTasks; @@ -188,4 +189,12 @@ public boolean isDisableJGroups() { public void setDisableJGroups(final boolean disableJGroups) { this.disableJGroups = disableJGroups; } + + public boolean isDisableRateLimiting() { + return this.disableRateLimiting; + } + + public void setDisableRateLimiting(final boolean disableRateLimiting) { + this.disableRateLimiting = disableRateLimiting; + } } diff --git a/backend/src/main/java/io/papermc/hangar/controller/api/v1/PagesController.java b/backend/src/main/java/io/papermc/hangar/controller/api/v1/PagesController.java index 793974c1a..e72cf65b1 100644 --- a/backend/src/main/java/io/papermc/hangar/controller/api/v1/PagesController.java +++ b/backend/src/main/java/io/papermc/hangar/controller/api/v1/PagesController.java @@ -38,9 +38,6 @@ public PagesController(final ProjectPageService projectPageService) { @ResponseStatus(HttpStatus.OK) public String getMainPage(final String slug) { final ExtendedProjectPage projectPage = this.projectPageService.getProjectPage(slug, ""); - if (projectPage == null) { - throw new ResponseStatusException(HttpStatus.NOT_FOUND); - } return projectPage.getContents(); } @@ -51,9 +48,6 @@ public String getMainPage(final String slug) { @ResponseStatus(HttpStatus.OK) public String getPage(final String slug, final String path) { final ExtendedProjectPage projectPage = this.projectPageService.getProjectPage(slug, path); - if (projectPage == null) { - throw new ResponseStatusException(HttpStatus.NOT_FOUND); - } return projectPage.getContents(); } @@ -73,10 +67,6 @@ public void editMainPage(final String slug, final StringContent pageEditForm) { @ResponseStatus(HttpStatus.OK) public void editPage(final String slug, final PageEditForm pageEditForm) { final ExtendedProjectPage projectPage = this.projectPageService.getProjectPage(slug, pageEditForm.path()); - if (projectPage == null) { - throw new HangarApiException(HttpStatus.NOT_FOUND, "Project page not found"); - } - this.projectPageService.saveProjectPage(projectPage.getProjectId(), projectPage.getId(), pageEditForm.content()); } } diff --git a/backend/src/main/java/io/papermc/hangar/model/internal/api/requests/StringContent.java b/backend/src/main/java/io/papermc/hangar/model/internal/api/requests/StringContent.java index 2d4eabf04..59c21a093 100644 --- a/backend/src/main/java/io/papermc/hangar/model/internal/api/requests/StringContent.java +++ b/backend/src/main/java/io/papermc/hangar/model/internal/api/requests/StringContent.java @@ -10,6 +10,13 @@ public class StringContent { private @NotBlank(message = "general.error.fieldEmpty") @Schema(description = "A non-null, non-empty string") String content; + public StringContent() { + } + + public StringContent(final String content) { + this.content = content; + } + public String getContent() { return this.content; } diff --git a/backend/src/main/java/io/papermc/hangar/security/annotations/ratelimit/RateLimitInterceptor.java b/backend/src/main/java/io/papermc/hangar/security/annotations/ratelimit/RateLimitInterceptor.java index 38b3f92c1..c148e3f5b 100644 --- a/backend/src/main/java/io/papermc/hangar/security/annotations/ratelimit/RateLimitInterceptor.java +++ b/backend/src/main/java/io/papermc/hangar/security/annotations/ratelimit/RateLimitInterceptor.java @@ -1,6 +1,7 @@ package io.papermc.hangar.security.annotations.ratelimit; import io.github.bucket4j.Bucket; +import io.papermc.hangar.config.hangar.HangarConfig; import io.papermc.hangar.exceptions.HangarApiException; import io.papermc.hangar.service.internal.BucketService; import jakarta.servlet.http.HttpServletRequest; @@ -19,10 +20,12 @@ public class RateLimitInterceptor implements HandlerInterceptor { private static final Logger LOGGER = LoggerFactory.getLogger(RateLimitInterceptor.class); private final BucketService bucketService; + private final HangarConfig hangarConfig; @Autowired - public RateLimitInterceptor(final BucketService bucketService) { + public RateLimitInterceptor(final BucketService bucketService, final HangarConfig hangarConfig) { this.bucketService = bucketService; + this.hangarConfig = hangarConfig; } @Override @@ -31,6 +34,10 @@ public boolean preHandle(final @NotNull HttpServletRequest request, final @NotNu return true; } + if (this.hangarConfig.isDisableRateLimiting()) { + return true; + } + final Method method = handlerMethod.getMethod(); final RateLimit limit = method.getAnnotation(RateLimit.class); if (limit != null) { diff --git a/backend/src/main/resources/application.yml b/backend/src/main/resources/application.yml index add19ee71..161a2b57f 100644 --- a/backend/src/main/resources/application.yml +++ b/backend/src/main/resources/application.yml @@ -61,6 +61,7 @@ hangar: ga-code: "UA-38006759-9" allow-indexing: true disable-jgroups: true + disable-ratelimiting: false licenses: - "Unspecified" diff --git a/backend/src/test/java/io/papermc/hangar/controller/api/v1/ApiKeysControllerTest.java b/backend/src/test/java/io/papermc/hangar/controller/api/v1/ApiKeysControllerTest.java index 645efa658..eec8c7c2d 100644 --- a/backend/src/test/java/io/papermc/hangar/controller/api/v1/ApiKeysControllerTest.java +++ b/backend/src/test/java/io/papermc/hangar/controller/api/v1/ApiKeysControllerTest.java @@ -11,7 +11,6 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; - class ApiKeysControllerTest extends ControllerTest { @Test diff --git a/backend/src/test/java/io/papermc/hangar/controller/api/v1/PagesControllerTest.java b/backend/src/test/java/io/papermc/hangar/controller/api/v1/PagesControllerTest.java new file mode 100644 index 000000000..26b031bce --- /dev/null +++ b/backend/src/test/java/io/papermc/hangar/controller/api/v1/PagesControllerTest.java @@ -0,0 +1,130 @@ +package io.papermc.hangar.controller.api.v1; + +import io.papermc.hangar.controller.api.v1.helper.ControllerTest; +import io.papermc.hangar.controller.api.v1.helper.TestData; +import io.papermc.hangar.model.api.project.PageEditForm; +import io.papermc.hangar.model.common.NamedPermission; +import io.papermc.hangar.model.internal.api.requests.CreateAPIKeyForm; +import io.papermc.hangar.model.internal.api.requests.StringContent; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.springframework.http.MediaType; + +import static org.hamcrest.Matchers.*; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; + +class PagesControllerTest extends ControllerTest { + + @Test + void testGetMainPage() throws Exception { + this.mockMvc.perform(get("/api/v1/pages/main/" + TestData.PROJECT.getSlug()) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)) + .andExpect(content().string(startsWith("# Test"))); + } + + @Test + void testEditMainPage() throws Exception { + // edit + this.mockMvc.perform(patch("/api/v1/pages/editmain/" + TestData.PROJECT.getSlug()) + .content(this.objectMapper.writeValueAsBytes(new StringContent("# Test\nEdited"))) + .contentType(MediaType.APPLICATION_JSON) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)); + + // validate + this.mockMvc.perform(get("/api/v1/pages/main/" + TestData.PROJECT.getSlug()) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)) + .andExpect(content().string(containsString("Edited"))); + } + + @Test + void testGetOtherPage() throws Exception { + this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=" + TestData.PAGE_PARENT.getSlug()) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)) + .andExpect(content().string(startsWith("# TestParentPage"))); + } + + @Test + void testSlashes() throws Exception { + this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=/" + TestData.PAGE_PARENT.getSlug() + "/") + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)) + .andExpect(content().string(startsWith("# TestParentPage"))); + } + + @Test + void testEditOtherPage() throws Exception { + // edit + this.mockMvc.perform(patch("/api/v1/pages/edit/" + TestData.PROJECT.getSlug()) + .content(this.objectMapper.writeValueAsBytes(new PageEditForm(TestData.PAGE_PARENT.getSlug(), "# TestParentPage\nEdited"))) + .contentType(MediaType.APPLICATION_JSON) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)); + + // validate + this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=" + TestData.PAGE_PARENT.getSlug()) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)) + .andExpect(content().string(containsString("Edited"))); + } + + @Test + void testGetChildPage() throws Exception { + this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=" + TestData.PAGE_CHILD.getSlug()) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)) + .andExpect(content().string(startsWith("# TestChildPage"))); + } + + @Test + void testEditChildPage() throws Exception { + // edit + this.mockMvc.perform(patch("/api/v1/pages/edit/" + TestData.PROJECT.getSlug()) + .content(this.objectMapper.writeValueAsBytes(new PageEditForm(TestData.PAGE_CHILD.getSlug(), "# TestChildPage\nEdited"))) + .contentType(MediaType.APPLICATION_JSON) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)); + + // validate + this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=" + TestData.PAGE_CHILD.getSlug()) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)) + .andExpect(content().string(containsString("Edited"))); + } + + @Test + void testGetInvalidProject() throws Exception { + this.mockMvc.perform(get("/api/v1/pages/main/Dum") + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(404)); + } + + @Test + void testGetInvalidPage() throws Exception { + this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=Dum") + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(404)); + } + + @Test + void testEditInvalidProject() throws Exception { + this.mockMvc.perform(patch("/api/v1/pages/editmain/Dum") + .content(this.objectMapper.writeValueAsBytes(new StringContent("# Dum"))) + .contentType(MediaType.APPLICATION_JSON) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(404)); + } + + @Test + void testEditInvalidPage() throws Exception { + this.mockMvc.perform(patch("/api/v1/pages/edit/" + TestData.PROJECT.getSlug()) + .content(this.objectMapper.writeValueAsBytes(new PageEditForm(TestData.PAGE_PARENT.getSlug(), "# TestParentPage\nEdited"))) + .contentType(MediaType.APPLICATION_JSON) + .with(this.apiKey(TestData.KEY_ADMIN))) + .andExpect(status().is(200)); + } +} diff --git a/backend/src/test/java/io/papermc/hangar/controller/api/v1/helper/TestData.java b/backend/src/test/java/io/papermc/hangar/controller/api/v1/helper/TestData.java index f3747c5bf..cbf55ba4c 100644 --- a/backend/src/test/java/io/papermc/hangar/controller/api/v1/helper/TestData.java +++ b/backend/src/test/java/io/papermc/hangar/controller/api/v1/helper/TestData.java @@ -12,6 +12,7 @@ import io.papermc.hangar.model.common.roles.OrganizationRole; import io.papermc.hangar.model.db.OrganizationTable; import io.papermc.hangar.model.db.UserTable; +import io.papermc.hangar.model.db.projects.ProjectPageTable; import io.papermc.hangar.model.db.projects.ProjectTable; import io.papermc.hangar.model.db.roles.GlobalRoleTable; import io.papermc.hangar.model.internal.api.requests.CreateAPIKeyForm; @@ -22,6 +23,7 @@ import io.papermc.hangar.service.internal.perms.members.OrganizationMemberService; import io.papermc.hangar.service.internal.perms.roles.GlobalRoleService; import io.papermc.hangar.service.internal.projects.ProjectFactory; +import io.papermc.hangar.service.internal.projects.ProjectPageService; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -50,6 +52,9 @@ public class TestData { public static ProjectTable PROJECT; + public static ProjectPageTable PAGE_PARENT; + public static ProjectPageTable PAGE_CHILD; + @Autowired private AuthService authService; @Autowired @@ -62,6 +67,8 @@ public class TestData { private OrganizationMemberService organizationMemberService; @Autowired private ProjectFactory projectFactory; + @Autowired + private ProjectPageService projectPageService; @EventListener(ApplicationStartedEvent.class) public void prepare() { @@ -83,6 +90,8 @@ public void prepare() { logger.info("Creating some test projects..."); PROJECT = this.projectFactory.createProject(new NewProjectForm(new ProjectSettings(List.of(), List.of(), new ProjectLicense(null, null, "MIT"), List.of(), null), Category.CHAT, "", ORG.getUserId(), "TestProject", "# Test", null)); + PAGE_PARENT = this.projectPageService.createPage(PROJECT.getProjectId(), "TestParentPage", "testparentpage", "# TestParentPage", true, null, false); + PAGE_CHILD = this.projectPageService.createPage(PROJECT.getProjectId(), "TestChildPage", "testparentpage/testchild", "# TestChildPage", true, PAGE_PARENT.getId(), false); logger.info("Creating test api keys..."); KEY_ADMIN = this.apiKeyService.createApiKey(USER_ADMIN, new CreateAPIKeyForm("Admin", Set.of(NamedPermission.values())), Permission.All); diff --git a/backend/src/test/resources/application-test.yml b/backend/src/test/resources/application-test.yml index 53fd0a514..3c77f91ca 100644 --- a/backend/src/test/resources/application-test.yml +++ b/backend/src/test/resources/application-test.yml @@ -13,5 +13,6 @@ spring: baseline-version: 0.0 hangar: + disable-ratelimiting: true storage: type: "local"