LUC070 252 fix urgent pentest findings - 2nd batch

datavault-webapp/src/main/java/org/datavaultplatform/webapp/controllers/VaultsController.java

@@ -27,6 +27,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
+import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.stereotype.Controller;
@@ -308,10 +309,18 @@ private boolean canAccessVault(VaultInfo vault, Principal principal, Boolean pen
                             || (RoleUtils.isRoleInSchool(roleAssignment, vault.getGroupID()) && RoleUtils.hasPermission(roleAssignment, Permission.CAN_MANAGE_VAULTS)));
         }
     }
-    @RequestMapping(value = "/vaults/{vaultid}/{userid}", method = RequestMethod.GET)
-    public String getVault(ModelMap model, @PathVariable("vaultid") String vaultID, @PathVariable("userid") String userID) {
-    	model.addAttribute("vaults", restService.getVaultsListingAll(userID));
-
+
+    @PreAuthorize("hasRole('IS_ADMIN') or #userId == authentication.name")
+    @GetMapping(value = "/vaults/{vaultId}/{userId}", produces = MediaType.TEXT_HTML_VALUE)
+    public String getVault(ModelMap model, @PathVariable String vaultId, @PathVariable String userId, Principal principal) {
+        VaultInfo vault = restService.getVault(vaultId);
+        if (vault == null) {
+            throw new EntityNotFoundException(Vault.class, vaultId);
+        }
+        if (!canAccessVault(vault, principal)) {
+            throw new ForbiddenException();
+        }
+        model.addAttribute("vaults", restService.getVaultsListingAll(userId));
         return "vaults/userVaults";
     }
 
@@ -525,6 +534,7 @@ public String updateVaultName(ModelMap model,
         return "redirect:" + vaultUrl;
     }
 
+    @PreAuthorize("hasRole('IS_ADMIN')")
     @RequestMapping(value = "/vaults/autocompleteuun/{term}", method = RequestMethod.GET)
     @ResponseBody
     public String autocompleteUUN(@PathVariable("term") String term) {
@@ -533,6 +543,7 @@ public String autocompleteUUN(@PathVariable("term") String term) {
         return gson.toJson(result);
     }
 
+    @PreAuthorize("hasRole('IS_ADMIN')")
     @RequestMapping(value = "/vaults/isuun/{uun}", method = RequestMethod.GET)
     @ResponseBody
     public String isUUN(@PathVariable("uun") String uun) {

datavault-webapp/src/main/java/org/datavaultplatform/webapp/controllers/admin/AdminPendingVaultsController.java

@@ -33,7 +33,7 @@ public class AdminPendingVaultsController {
 
 	private static final Logger logger = LoggerFactory.getLogger(AdminPendingVaultsController.class);
 
-	private static final int MAX_RECORDS_PER_PAGE = 10;
+	protected static final int MAX_RECORDS_PER_PAGE = 10;
 
     private final RestService restService;
     private final UserLookupService userLookupService;
@@ -70,7 +70,7 @@ public String searchPendingVaults(ModelMap model,
 
     // The Admin Edit PV page
     @RequestMapping(value = "/admin/pendingVaults/edit/{vaultid}", method = RequestMethod.GET)
-    public String getPendingVault(ModelMap model, @PathVariable("vaultid") String vaultID) {
+    public String getPendingVaultForm(ModelMap model, @PathVariable("vaultid") String vaultID) {
         VaultInfo vault = restService.getPendingVault(vaultID);
         logger.info("Passed in id: '" + vaultID);
         model.addAttribute("vaultID", vaultID);
@@ -157,7 +157,7 @@ private ModelMap createModelMap(ModelMap model, int filteredRecordsTotal, Vaults
 
 
     @RequestMapping(value = "/admin/pendingVaults/summary/{pendingVaultId}", method = RequestMethod.GET)
-    public String getVault(ModelMap model, @PathVariable("pendingVaultId") String vaultID, Principal principal) {
+    public String getVaultSummary(ModelMap model, @PathVariable("pendingVaultId") String vaultID, Principal principal) {
         logger.info("VaultID:'" + vaultID + "'");
         VaultInfo pendingVault = restService.getPendingVault(vaultID);
         logger.info("pendingVault.id:'" + pendingVault.getID() + "'");
@@ -205,7 +205,7 @@ public String upgradeVault(@PathVariable("pendingVaultId") String pendingVaultID
 
 	// Process the completed 'create new vault' page
 	@RequestMapping(value = "/admin/pendingVaults/edit", method = RequestMethod.POST)
-	public String editPendingVault(@ModelAttribute CreateVault vault, ModelMap model, @RequestParam String action,
+	public String submitEditPendingVault(@ModelAttribute CreateVault vault, ModelMap model, @RequestParam String action,
 			Principal principal) {
 		// if the confirm button has been clicked save what we have if everything isn't
 		// already saved and display the summary

datavault-webapp/src/test/java/org/datavaultplatform/webapp/controllers/VaultsControllerMvcTest.java

@@ -0,0 +1,251 @@
+package org.datavaultplatform.webapp.controllers;
+
+import lombok.SneakyThrows;
+import org.datavaultplatform.common.model.RoleAssignment;
+import org.datavaultplatform.common.response.VaultInfo;
+import org.datavaultplatform.common.util.RoleUtils;
+import org.datavaultplatform.webapp.app.DataVaultWebApp;
+import org.datavaultplatform.webapp.services.RestService;
+import org.datavaultplatform.webapp.services.UserLookupService;
+import org.datavaultplatform.webapp.test.AddTestProperties;
+import org.datavaultplatform.webapp.test.ProfileDatabase;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.http.MediaType;
+import org.springframework.security.test.context.support.WithMockUser;
+import org.springframework.test.context.TestPropertySource;
+import org.springframework.test.context.bean.override.mockito.MockitoBean;
+import org.springframework.test.web.servlet.MockMvc;
+
+import java.util.List;
+
+import static org.mockito.Mockito.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+
+@SpringBootTest(classes = DataVaultWebApp.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+@ProfileDatabase
+@TestPropertySource(properties = "logging.level.org.springframework.security=DEBUG")
+@AddTestProperties
+@AutoConfigureMockMvc
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+class VaultsControllerMvcTest {
+
+    @Autowired
+    MockMvc mockMvc;
+    @Captor
+    ArgumentCaptor<RoleAssignment> roleAssignmentArg;
+    @MockitoBean
+    private RestService restService;
+
+    @MockitoBean
+    private UserLookupService userLookupService;
+
+    @Test
+    @Order(1)
+    @SneakyThrows
+    @WithMockUser(username = "super-user", roles = {"IS_ADMIN", "USER"})
+    void testGetVault_FailsIfVaultNotFoundAsSuperUser() {
+
+        when(restService.getVault("vault123")).thenReturn(null);
+
+        mockMvc.perform(get("/vaults/vault123/user123"))
+                .andDo(print()).andExpect(status().isNotFound());
+
+        verify(restService).getVault("vault123");
+        verifyNoMoreInteractions(restService, userLookupService);
+
+    }
+
+    @Test
+    @Order(2)
+    @SneakyThrows
+    @WithMockUser(username = "user123", roles = {"USER"})
+    void testGetVault_FailsIfVaultNotFoundAsVanillaUser() {
+
+        when(restService.getVault("vault123")).thenReturn(null);
+
+        mockMvc.perform(get("/vaults/vault123/user123"))
+                .andDo(print()).andExpect(status().isNotFound());
+
+        verify(restService).getVault("vault123");
+        verifyNoMoreInteractions(restService, userLookupService);
+    }
+
+    @Test
+    @Order(3)
+    @SneakyThrows
+    @WithMockUser(username = "super-user", roles = {"IS_ADMIN", "USER"})
+    void testGetVault_ForbiddenIfCannotAccessVaultAsSuperUser() {
+
+        VaultInfo vaultInfo = new VaultInfo();
+        when(restService.getVault("vault123")).thenReturn(vaultInfo);
+
+        RoleAssignment ra1 = new RoleAssignment();
+        when(restService.getRoleAssignmentsForUser("super-user")).thenReturn(List.of(ra1));
+
+        try (MockedStatic<RoleUtils> roleUtils = Mockito.mockStatic(RoleUtils.class)) {
+            roleUtils.when(() -> RoleUtils.isISAdmin(roleAssignmentArg.capture())).thenReturn(false);
+
+            mockMvc.perform(get("/vaults/vault123/user123"))
+                    .andDo(print()).andExpect(status().isForbidden());
+        }
+
+        verify(restService).getVault("vault123");
+        verify(restService).getRoleAssignmentsForUser("super-user");
+        verifyNoMoreInteractions(restService, userLookupService);
+    }
+
+    @Test
+    @Order(4)
+    @SneakyThrows
+    @WithMockUser(username = "user123", roles = {"USER"})
+    void testGetVault_ForbiddenIfCannotAccessVaultAsVanillaUser() {
+
+        VaultInfo vaultInfo = new VaultInfo();
+        when(restService.getVault("vault123")).thenReturn(vaultInfo);
+
+        RoleAssignment ra1 = new RoleAssignment();
+        when(restService.getRoleAssignmentsForUser("user123")).thenReturn(List.of(ra1));
+
+        try (MockedStatic<RoleUtils> roleUtils = Mockito.mockStatic(RoleUtils.class)) {
+            roleUtils.when(() -> RoleUtils.isISAdmin(roleAssignmentArg.capture())).thenReturn(false);
+
+            mockMvc.perform(get("/vaults/vault123/user123"))
+                    .andDo(print()).andExpect(status().isForbidden());
+        }
+
+        verify(restService).getVault("vault123");
+        verify(restService).getRoleAssignmentsForUser("user123");
+        verifyNoMoreInteractions(restService, userLookupService);
+
+    }
+
+    @Test
+    @Order(5)
+    @SneakyThrows
+    @WithMockUser(username = "super-user", roles = {"IS_ADMIN", "USER"})
+    void testGetVault_AllowedIfCanAccessVaultAsSuperUser() {
+
+        VaultInfo vaultInfo = new VaultInfo();
+        when(restService.getVault("vault123")).thenReturn(vaultInfo);
+
+        RoleAssignment ra1 = new RoleAssignment();
+        when(restService.getRoleAssignmentsForUser("super-user")).thenReturn(List.of(ra1));
+
+        VaultInfo[] vaultListing = new VaultInfo[0];
+        when(restService.getVaultsListingAll("user123")).thenReturn(vaultListing);
+
+        try (MockedStatic<RoleUtils> roleUtils = Mockito.mockStatic(RoleUtils.class)) {
+            roleUtils.when(() -> RoleUtils.isISAdmin(roleAssignmentArg.capture())).thenReturn(true);
+
+            mockMvc.perform(get("/vaults/vault123/user123"))
+                    .andDo(print())
+                    .andExpect(model().attribute("vaults", vaultListing))
+                    .andExpect(view().name("vaults/userVaults"));
+        }
+
+        verify(restService).getVault("vault123");
+        verify(restService).getRoleAssignmentsForUser("super-user");
+        verify(restService).getVaultsListingAll("user123");
+        verifyNoMoreInteractions(restService, userLookupService);
+
+    }
+
+    @Test
+    @Order(6)
+    @SneakyThrows
+    @WithMockUser(username = "user123", roles = {"USER"})
+    void testGetVault_AllowedIfCanAccessVaultAsVanillaUser() {
+
+        VaultInfo vaultInfo = new VaultInfo();
+        when(restService.getVault("vault123")).thenReturn(vaultInfo);
+
+        RoleAssignment ra1 = new RoleAssignment();
+        when(restService.getRoleAssignmentsForUser("user123")).thenReturn(List.of(ra1));
+
+        VaultInfo[] vaultListing = new VaultInfo[0];
+        when(restService.getVaultsListingAll("user123")).thenReturn(vaultListing);
+
+        try (MockedStatic<RoleUtils> roleUtils = Mockito.mockStatic(RoleUtils.class)) {
+            roleUtils.when(() -> RoleUtils.isISAdmin(roleAssignmentArg.capture())).thenReturn(true);
+
+            mockMvc.perform(get("/vaults/vault123/user123"))
+                    .andDo(print())
+                    .andExpect(model().attribute("vaults", vaultListing))
+                    .andExpect(view().name("vaults/userVaults"));
+        }
+
+        verify(restService).getVault("vault123");
+        verify(restService).getRoleAssignmentsForUser("user123");
+        verify(restService).getVaultsListingAll("user123");
+        verifyNoMoreInteractions(restService, userLookupService);
+
+    }
+
+    @Test
+    @Order(7)
+    @SneakyThrows
+    @WithMockUser(username = "vanilla-user", roles = {"USER"})
+    void testIsUUN_ForbiddenAsVanillaUser() {
+        mockMvc.perform(get("/vaults/isuun/v1dhay3")).andExpect(status().isForbidden());
+        verifyNoMoreInteractions(restService, userLookupService);
+
+    }
+
+    @Order(8)
+    @SneakyThrows
+    @ParameterizedTest
+    @ValueSource(booleans = {true, false})
+    @WithMockUser(username = "super-user", roles = {"USER","IS_ADMIN"})
+    void testIsUUN_AllowedAsSuperUser(boolean isUUN) {
+        when(userLookupService.isUUN("v1dhay3")).thenReturn(isUUN);
+        mockMvc.perform(get("/vaults/isuun/v1dhay3"))
+                .andExpect(status().isOk())
+                .andExpect(content().string(String.valueOf(isUUN)))
+                .andExpect(content().contentTypeCompatibleWith(MediaType.TEXT_PLAIN));
+
+        verify(userLookupService).isUUN("v1dhay3");
+        verifyNoMoreInteractions(restService, userLookupService);
+    }
+
+    @Test
+    @Order(9)
+    @SneakyThrows
+    @WithMockUser(username = "vanilla-user", roles = {"USER"})
+    void testAutocompleteUUN_ForbiddenAsVanillaUser() {
+        mockMvc.perform(get("/vaults/autocompleteuun/blah")).andExpect(status().isForbidden());
+        verifyNoMoreInteractions(restService, userLookupService);
+    }
+
+
+    @Test
+    @Order(10)
+    @SneakyThrows
+    @WithMockUser(username = "super-user", roles = {"USER","IS_ADMIN"})
+    void testAutocompleteUUN_AllowedAsSuperUser() {
+        when(userLookupService.getSuggestedUuns("blah")).thenReturn(List.of("blah1","blah2","blah3"));
+        mockMvc.perform(get("/vaults/autocompleteuun/blah"))
+                .andExpect(status().isOk())
+                .andExpect(content().string(
+                        """
+                        ["blah1","blah2","blah3"]"""))
+                .andExpect(content().contentTypeCompatibleWith(MediaType.TEXT_PLAIN));
+
+        verify(userLookupService).getSuggestedUuns("blah");
+        verifyNoMoreInteractions(restService, userLookupService);
+    }
+
+}